diff --git a/server/internal/daemon/daemon.go b/server/internal/daemon/daemon.go index da2f3fafc5..66b6e41068 100644 --- a/server/internal/daemon/daemon.go +++ b/server/internal/daemon/daemon.go @@ -2488,8 +2488,6 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i taskCtx := execenv.TaskContextForEnv{ IssueID: task.IssueID, TriggerCommentID: task.TriggerCommentID, - NewCommentCount: task.NewCommentCount, - NewCommentsSince: task.NewCommentsSince, AgentID: agentID, AgentName: agentName, AgentInstructions: instructions, diff --git a/server/internal/daemon/execenv/execenv.go b/server/internal/daemon/execenv/execenv.go index b89c04ded9..5d51e86bc5 100644 --- a/server/internal/daemon/execenv/execenv.go +++ b/server/internal/daemon/execenv/execenv.go @@ -54,8 +54,6 @@ type PrepareParams struct { type TaskContextForEnv struct { IssueID string TriggerCommentID string // comment that triggered this task (empty for on_assign) - NewCommentCount int // comments created since this agent's last run on the issue (excludes its own) - NewCommentsSince string // RFC3339 anchor (last run's started_at) the count is measured from; empty on cold start AgentID string // unique ID of the dispatched agent AgentName string AgentInstructions string // agent identity/persona instructions, injected into CLAUDE.md diff --git a/server/internal/daemon/execenv/execenv_test.go b/server/internal/daemon/execenv/execenv_test.go index 51f9335a40..df91e4110b 100644 --- a/server/internal/daemon/execenv/execenv_test.go +++ b/server/internal/daemon/execenv/execenv_test.go @@ -3345,13 +3345,14 @@ func TestBuildMetaSkillContentOmitsRequestingUserWhenEmpty(t *testing.T) { } } -// TestInjectRuntimeConfigCommentTriggerColdStartRead checks the -// comment-triggered Workflow on cold start (no prior run): it falls back to a -// plain catch-up read with no since-delta hint, while the Available Commands -// core line still surfaces the thread/recent/cursor flags so they remain -// discoverable for CLI use even though the verbose cursor walkthrough was -// dropped from the workflow steps. -func TestInjectRuntimeConfigCommentTriggerColdStartRead(t *testing.T) { +// TestInjectRuntimeConfigCommentTriggerThreadFirstReads locks in +// MUL-2387 + MUL-2421: the runtime config's comment-triggered Workflow +// section must steer the agent at thread-aware reads first, default the +// trigger thread to `--thread --tail 30` (bounded), and explain the +// reply-cursor walk for older replies. `--recent N` stays as the +// cross-thread fallback. The Available Commands core line also has to +// surface the `--tail` flag so the agent has a single place to discover it. +func TestInjectRuntimeConfigCommentTriggerThreadFirstReads(t *testing.T) { t.Parallel() const ( @@ -3372,18 +3373,34 @@ func TestInjectRuntimeConfigCommentTriggerColdStartRead(t *testing.T) { } s := string(data) - // Cold start (no prior run) → plain catch-up line, no since-delta hint. + // Workflow step 2 must read the trigger's thread with --thread anchored + // on the exact trigger comment id from this task, bounded to --tail 30. for _, want := range []string{ - "Catch up on comments", - "multica issue comment list " + issueID + " --output json", + "--thread " + triggerID, + "--tail 30", + "multica issue comment list " + issueID + " --thread " + triggerID + " --tail 30 --output json", + // Reply cursor walks older replies inside the same thread. + "Next reply cursor:", + "--before-id ", + // --recent fallback at the documented default N=20 for cross-thread context. + "multica issue comment list " + issueID + " --recent 20 --output json", + // Cursor walks via the stderr line the CLI emits, not invented flags. + "Next thread cursor", + "--before", + "--before-id", + // --since is still available and combinable (now scoped to the + // post-MUL-2421 mode names). + "--since", + "may combine with `--thread --tail` or `--recent`", + // Explicit pushback on the legacy full-dump recipe so the model has + // no reason to fall back to it on long issues. + "Avoid the unfiltered", + "wastes context", } { if !strings.Contains(s, want) { - t.Errorf("comment-triggered Workflow missing cold-start read %q\n---\n%s", want, s) + t.Errorf("comment-triggered Workflow missing %q\n---\n%s", want, s) } } - if strings.Contains(s, "new comment(s) since your last run") { - t.Errorf("cold-start workflow must not render the since-delta hint\n---\n%s", s) - } // Available Commands core line must surface the new flags (this is the // single discovery point for non-workflow CLI use cases). diff --git a/server/internal/daemon/execenv/reply_instructions.go b/server/internal/daemon/execenv/reply_instructions.go index 965dad6b18..d9a371e201 100644 --- a/server/internal/daemon/execenv/reply_instructions.go +++ b/server/internal/daemon/execenv/reply_instructions.go @@ -2,30 +2,6 @@ package execenv import "fmt" -// BuildNewCommentsHint returns a one-line pointer telling a comment-triggered -// agent how many comments arrived since its last run on this issue, and the -// exact `--since` invocation to fetch just those. It ships only the COUNT and -// the cursor — never the comment bodies — so the server stays cheap and the -// agent pulls details on demand. -// -// Both the per-turn prompt (daemon.buildCommentPrompt) and the CLAUDE.md -// workflow (InjectRuntimeConfig) call this so the two surfaces cannot drift -// (hard requirement from PR #2816). -// -// Renders nothing on cold start (no prior run → newCommentsSince empty) or when -// there are no new comments (newCommentCount <= 0) or issueID is empty. In -// those cases the caller falls back to a plain "read the discussion" line. -func BuildNewCommentsHint(issueID, newCommentsSince string, newCommentCount int) string { - if newCommentCount <= 0 || newCommentsSince == "" || issueID == "" { - return "" - } - return fmt.Sprintf( - "%d new comment(s) since your last run. Catch up: "+ - "`multica issue comment list %s --since %s --output json`.\n\n", - newCommentCount, issueID, newCommentsSince, - ) -} - // BuildCommentReplyInstructions returns the canonical block telling an agent // how to post its reply for a comment-triggered task. Both the per-turn // prompt (daemon.buildCommentPrompt) and the CLAUDE.md workflow diff --git a/server/internal/daemon/execenv/runtime_config.go b/server/internal/daemon/execenv/runtime_config.go index 9d8fd0e520..9d5ae2bb99 100644 --- a/server/internal/daemon/execenv/runtime_config.go +++ b/server/internal/daemon/execenv/runtime_config.go @@ -574,12 +574,11 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string { b.WriteString("**This task was triggered by a NEW comment.** Your primary job is to respond to THIS specific comment, even if you have handled similar requests before in this session.\n\n") fmt.Fprintf(&b, "1. Run `multica issue get %s --output json` to understand the issue context\n", ctx.IssueID) fmt.Fprintf(&b, "2. Run `multica issue metadata list %s --output json` to see what prior agents pinned — best-effort, empty `{}` and CLI failures are normal. See the `## Issue Metadata` section above for what to look for.\n", ctx.IssueID) - if hint := BuildNewCommentsHint(ctx.IssueID, ctx.NewCommentsSince, ctx.NewCommentCount); hint != "" { - b.WriteString("3. " + hint) - } else { - fmt.Fprintf(&b, "3. Catch up on comments — read with `multica issue comment list %s --output json` (long issue? `--thread %s` for the triggering line, or `--recent 20`).\n", ctx.IssueID, ctx.TriggerCommentID) - } - fmt.Fprintf(&b, "4. Find the triggering comment (ID: `%s`) and understand what is being asked — do NOT confuse it with previous comments\n", ctx.TriggerCommentID) + fmt.Fprintf(&b, "3. Read the triggering thread first — that is what this comment is actually about. Default to the 30 most recent replies in that thread: `multica issue comment list %s --thread %s --tail 30 --output json` returns the root + the 30 newest replies (root is always included, even at `--tail 0`).\n", ctx.IssueID, ctx.TriggerCommentID) + b.WriteString(" - If 30 replies aren't enough, walk older replies in the same thread one page at a time using the stderr `Next reply cursor: --before --before-id ` line — pass the same pair back as `--before --before-id ` on the next call. Under `--thread --tail` the cursor walks older *replies*, not older threads.\n") + fmt.Fprintf(&b, " - If you also need cross-thread background, pull the most recently active threads on the issue: `multica issue comment list %s --recent 20 --output json`. Under `--recent` the same `--before` / `--before-id` flags walk older *threads* instead of older replies, and the stderr line is `Next thread cursor: --before --before-id `. Pass the pair back to scroll to older threads when 20 still isn't enough.\n", ctx.IssueID) + b.WriteString(" - Avoid the unfiltered `multica issue comment list --output json` form on long-running issues — it dumps the entire flat timeline (cap 2000) and wastes context on chatter unrelated to the trigger. `--since ` is still available for incremental polling against a known cursor and may combine with `--thread --tail` or `--recent`.\n") + fmt.Fprintf(&b, "4. Find the triggering comment (ID: `%s`) inside the thread you just read and understand what is being asked — do NOT confuse it with previous comments\n", ctx.TriggerCommentID) if ctx.IsSquadLeader { b.WriteString("5. **Decide whether a reply is warranted.** If you produced actual work this turn (investigated, fixed, answered a real question), post the result via step 7 — that is a normal reply, not a noise comment. If the triggering comment was a pure acknowledgment / thanks / sign-off from another agent AND you produced no work this turn, do NOT post a reply — and do NOT post a comment saying 'No reply needed' or similar. Simply exit with no output. Silence is a valid and preferred way to end agent-to-agent conversations.\n") fmt.Fprintf(&b, " - **Squad leader rule:** If your evaluation outcome is `no_action`, call `multica squad activity %s no_action --reason \"...\"` and then EXIT IMMEDIATELY. DO NOT post any comment whose only purpose is to announce that you are taking no action, exiting silently, or acknowledging another agent. A comment like \"No action needed\" or \"Exiting silently\" is noise — the `squad activity` call already records your decision in the timeline.\n", ctx.IssueID) diff --git a/server/internal/daemon/execenv/runtime_config_test.go b/server/internal/daemon/execenv/runtime_config_test.go index 118e15346d..470c1e7e99 100644 --- a/server/internal/daemon/execenv/runtime_config_test.go +++ b/server/internal/daemon/execenv/runtime_config_test.go @@ -156,54 +156,6 @@ func TestCommentTriggeredProtocolDoesNotForceInReview(t *testing.T) { } } -// The CLAUDE.md workflow surface must carry the same since-delta new-comment -// hint as the per-turn prompt. PR #2816 requires the two surfaces stay in sync, -// so this pins the count-driven `--since` hint into the comment-triggered brief. -func TestCommentTriggeredBriefCarriesNewCommentsHint(t *testing.T) { - t.Parallel() - const ( - issueID = "55555555-6666-7777-8888-999999999999" - since = "2026-05-28T11:00:00Z" - ) - ctx := TaskContextForEnv{ - IssueID: issueID, - TriggerCommentID: "reply-abc", - NewCommentCount: 4, - NewCommentsSince: since, - } - out := buildMetaSkillContent("claude", ctx) - - if !strings.Contains(out, "4 new comment(s) since your last run") { - t.Errorf("comment brief must report the new-comment count, got:\n%s", out) - } - if !strings.Contains(out, "--since "+since+" --output json") { - t.Errorf("comment brief must point at the --since catch-up read, got:\n%s", out) - } - // The removed resolve step must not reappear. - if strings.Contains(out, "multica comment resolve") { - t.Errorf("comment brief must not carry the dropped resolve step, got:\n%s", out) - } -} - -// Cold start (no prior run → no since anchor) must fall back to the plain read -// line instead of the since-delta hint. -func TestCommentTriggeredBriefColdStartNoHint(t *testing.T) { - t.Parallel() - ctx := TaskContextForEnv{ - IssueID: "55555555-6666-7777-8888-999999999999", - TriggerCommentID: "trigger-1", - NewCommentCount: 0, - NewCommentsSince: "", - } - out := buildMetaSkillContent("claude", ctx) - if strings.Contains(out, "new comment(s) since your last run") { - t.Errorf("no since-delta hint should render on cold start, got:\n%s", out) - } - if !strings.Contains(out, "Catch up on comments") { - t.Errorf("cold start must fall back to the plain catch-up line, got:\n%s", out) - } -} - // Assignment-triggered briefs are the inverse boundary: when the agent // owns the issue lifecycle, the brief AS A WHOLE must still tell it to // flip to in_review on completion. The flip lives in the diff --git a/server/internal/daemon/prompt.go b/server/internal/daemon/prompt.go index a770e7001c..196f9ee0a0 100644 --- a/server/internal/daemon/prompt.go +++ b/server/internal/daemon/prompt.go @@ -159,13 +159,7 @@ func buildCommentPrompt(task Task, provider string) string { } } fmt.Fprintf(&b, "Start by running `multica issue get %s --output json` to understand your task, then decide how to proceed.\n\n", task.IssueID) - // New-comment hint when we know the agent ran on this issue before; otherwise - // (cold start) point it at the discussion with a single plain read. - if hint := execenv.BuildNewCommentsHint(task.IssueID, task.NewCommentsSince, task.NewCommentCount); hint != "" { - b.WriteString(hint) - } else { - fmt.Fprintf(&b, "Read the discussion: `multica issue comment list %s --output json` (long issue? use `--recent 20`).\n\n", task.IssueID) - } + fmt.Fprintf(&b, "For comment history, read the triggering thread first: `multica issue comment list %s --thread %s --tail 30 --output json` returns the root + the 30 most recent replies in that thread (root is always included, even at `--tail 0`, so you keep the \"what is this about\" context without dragging hundreds of replies into your prompt). If 30 replies aren't enough, walk older replies in the same thread one page at a time by passing the stderr `Next reply cursor: --before --before-id ` line back as `--before --before-id ` on the next call. If you also need cross-thread background, `multica issue comment list %s --recent 20 --output json` pulls the 20 most recently active threads on the issue; under `--recent` the same `--before` / `--before-id` flags walk older *threads* (stderr label: `Next thread cursor`) instead of older replies. Avoid the unfiltered `--output json` form on long-running issues; it dumps the full flat timeline (cap 2000) and wastes context. `--since ` is still available for incremental polling and may combine with `--thread --tail` or `--recent`.\n\n", task.IssueID, task.TriggerCommentID, task.IssueID) b.WriteString(execenv.BuildCommentReplyInstructions(provider, task.IssueID, task.TriggerCommentID)) return b.String() } diff --git a/server/internal/daemon/prompt_test.go b/server/internal/daemon/prompt_test.go index 0a88bfd2f6..06e433d632 100644 --- a/server/internal/daemon/prompt_test.go +++ b/server/internal/daemon/prompt_test.go @@ -249,6 +249,69 @@ func TestBuildPromptSquadLeaderNoActionForAgentTrigger(t *testing.T) { } } +// TestBuildPromptCommentTriggerPromotesThreadReads pins MUL-2387 + MUL-2421: +// the per-turn prompt for a comment-triggered task must default the trigger +// thread read to `--thread --tail 30` (so long threads don't dump +// hundreds of replies into the agent's context) and explain reply-cursor +// pagination for older replies. --recent N stays as the cross-thread +// fallback. Locking this in test stops the guidance from decaying back to +// either the legacy full-flat-dump or the unbounded `--thread` recipe. +func TestBuildPromptCommentTriggerPromotesThreadReads(t *testing.T) { + const ( + issueID = "issue-thread-1" + triggerID = "trigger-comment-1" + ) + task := Task{ + IssueID: issueID, + TriggerCommentID: triggerID, + TriggerCommentContent: "anything", + TriggerAuthorType: "member", + TriggerAuthorName: "Bohan", + } + out := BuildPrompt(task, "claude") + + mustContain := []string{ + // Thread-first read pinned by trigger comment id, capped via --tail 30. + "--thread " + triggerID, + "--tail 30", + "`multica issue comment list " + issueID + " --thread " + triggerID + " --tail 30 --output json`", + // Reply cursor walks older replies inside the same thread. + "Next reply cursor:", + "--before-id ", + // --recent stays as the cross-thread background fallback. + "--recent 20 --output json", + // Cursor walks via the stderr line the CLI emits, not invented flags. + "Next thread cursor", + "--before", + "--before-id", + // --since is preserved as an additional, combinable knob (now scoped + // to the post-MUL-2421 mode names). + "--since", + "may combine with `--thread --tail` or `--recent`", + // Discourage the unfiltered full dump on long-running issues. + "Avoid the unfiltered", + "wastes context", + } + for _, s := range mustContain { + if !strings.Contains(out, s) { + t.Errorf("buildCommentPrompt missing thread-first guidance %q\n--- output ---\n%s", s, out) + } + } + + // The old "dump everything via --output json alone" prose is exactly the + // pattern this PR is replacing — guard against the legacy phrasing + // sneaking back in. + if strings.Contains(out, "returns all comments for the issue (server caps at 2000)") { + t.Errorf("buildCommentPrompt still carries the legacy full-dump phrasing") + } + // The pre-MUL-2421 unbounded `--thread` recipe (no --tail) is also a + // regression target: it dumps the entire thread on long threads, which + // is exactly what --tail 30 is meant to bound. + if strings.Contains(out, "--thread "+triggerID+" --output json") { + t.Errorf("buildCommentPrompt regressed to unbounded --thread recipe (no --tail) — long threads will overflow context\n--- output ---\n%s", out) + } +} + // TestBuildPromptDefaultMentionsRecent pins that the catch-all fallback // prompt (no trigger comment, no chat, no autopilot, no quick-create) also // teaches the agent about --recent as the long-issue-friendly alternative @@ -296,57 +359,3 @@ func TestBuildPromptNonSquadLeaderNoRule(t *testing.T) { t.Errorf("buildCommentPrompt must NOT inject squad leader no_action rule for non-squad-leader agents, got:\n%s", out) } } - -// TestBuildPromptNewCommentsHint pins that a comment-triggered task whose agent -// ran before on this issue (NewCommentsSince set, NewCommentCount > 0) gets the -// one-line since-delta hint pointing at `--since`, so the agent catches up on -// exactly the comments that arrived since its last run instead of re-reading -// everything or walking threads blind. -func TestBuildPromptNewCommentsHint(t *testing.T) { - const ( - issueID = "issue-new-1" - since = "2026-05-28T11:00:00Z" - ) - task := Task{ - IssueID: issueID, - TriggerCommentID: "trigger-1", - TriggerCommentContent: "please look", - TriggerAuthorType: "member", - NewCommentCount: 3, - NewCommentsSince: since, - } - out := BuildPrompt(task, "claude") - - if !strings.Contains(out, "3 new comment(s) since your last run") { - t.Errorf("hint must report the new-comment count, got:\n%s", out) - } - if !strings.Contains(out, "multica issue comment list "+issueID+" --since "+since+" --output json") { - t.Errorf("hint must point at the --since catch-up read, got:\n%s", out) - } - // The old cursor-heavy paragraph must be gone. - if strings.Contains(out, "Next reply cursor") || strings.Contains(out, "--before-id") { - t.Errorf("the old cursor-pagination paragraph must not render, got:\n%s", out) - } -} - -// TestBuildPromptColdStartNoHint pins the cold-start case: no prior run means no -// since anchor (NewCommentsSince empty), so we suppress the delta hint and fall -// back to a plain "read the discussion" line. -func TestBuildPromptColdStartNoHint(t *testing.T) { - const issueID = "issue-cold-1" - task := Task{ - IssueID: issueID, - TriggerCommentID: "trigger-1", - TriggerCommentContent: "hi", - TriggerAuthorType: "member", - NewCommentCount: 0, - NewCommentsSince: "", - } - out := BuildPrompt(task, "claude") - if strings.Contains(out, "new comment(s) since your last run") { - t.Errorf("no since-delta hint should render on cold start, got:\n%s", out) - } - if !strings.Contains(out, "Read the discussion: `multica issue comment list "+issueID+" --output json`") { - t.Errorf("cold start must fall back to the plain read line, got:\n%s", out) - } -} diff --git a/server/internal/daemon/types.go b/server/internal/daemon/types.go index 5b6ea3c205..e1d66c54d8 100644 --- a/server/internal/daemon/types.go +++ b/server/internal/daemon/types.go @@ -55,8 +55,6 @@ type Task struct { TriggerCommentContent string `json:"trigger_comment_content,omitempty"` // content of the triggering comment TriggerAuthorType string `json:"trigger_author_type,omitempty"` // "agent" or "member" — author kind for the triggering comment TriggerAuthorName string `json:"trigger_author_name,omitempty"` // display name of the triggering comment author - NewCommentCount int `json:"new_comment_count,omitempty"` // comments on the issue created since this agent's last run (excludes its own); 0/omitted for old daemons or cold start - NewCommentsSince string `json:"new_comments_since,omitempty"` // RFC3339 anchor (last run's started_at) the count is measured from; empty on cold start ChatSessionID string `json:"chat_session_id,omitempty"` // non-empty for chat tasks ChatMessage string `json:"chat_message,omitempty"` // user message content for chat tasks ChatMessageAttachments []ChatAttachmentMeta `json:"chat_message_attachments,omitempty"` // attachments linked to the chat message; agent uses these to `multica attachment download ` diff --git a/server/internal/handler/agent.go b/server/internal/handler/agent.go index c0e63e59e9..d7817db1f9 100644 --- a/server/internal/handler/agent.go +++ b/server/internal/handler/agent.go @@ -202,8 +202,6 @@ type AgentTaskResponse struct { TriggerSummary *string `json:"trigger_summary,omitempty"` // canonical short description snapshot — comment text / autopilot title — taken at task creation; survives source edits/deletes TriggerAuthorType string `json:"trigger_author_type,omitempty"` // "agent" or "member" — author kind of the triggering comment TriggerAuthorName string `json:"trigger_author_name,omitempty"` // display name of the triggering comment author - NewCommentCount int `json:"new_comment_count,omitempty"` // comments created since this agent's last run on the issue (excludes its own); omitempty so old daemons ignore it - NewCommentsSince string `json:"new_comments_since,omitempty"` // RFC3339 anchor (last run's started_at) the count is measured from; omitempty so old daemons ignore it ChatSessionID string `json:"chat_session_id,omitempty"` // non-empty for chat tasks ChatMessage string `json:"chat_message,omitempty"` // user message for chat tasks ChatMessageAttachments []ChatAttachmentMeta `json:"chat_message_attachments,omitempty"` // attachments on the user message — agent calls `multica attachment download ` per entry diff --git a/server/internal/handler/comment_list_test.go b/server/internal/handler/comment_list_test.go index 0e31c7cc3c..c30cec1154 100644 --- a/server/internal/handler/comment_list_test.go +++ b/server/internal/handler/comment_list_test.go @@ -11,8 +11,6 @@ import ( "time" "github.com/google/uuid" - "github.com/jackc/pgx/v5/pgtype" - db "github.com/multica-ai/multica/server/pkg/db/generated" ) // cursorQuery builds a properly URL-encoded query string for the recent + @@ -1015,67 +1013,3 @@ func TestListComments_ThreadTailNotFoundReturns404(t *testing.T) { } _ = fx } - -// resolveCommentRow marks a comment resolved directly in the DB (test helper — -// the public path goes through ResolveComment, but for list-filter tests we -// just need the column set). -func resolveCommentRow(t *testing.T, commentID string) { - t.Helper() - if _, err := testPool.Exec(context.Background(), - `UPDATE comment SET resolved_at = now(), resolved_by_type = 'member', resolved_by_id = $2 WHERE id = $1`, - commentID, testUserID, - ); err != nil { - t.Fatalf("resolve comment row: %v", err) - } -} - -// TestCountNewCommentsSince_ExcludesAgentOwn pins the claim-side count query: it -// counts comments created after the given anchor and excludes the agent's own, -// so a chatty agent does not inflate its own new-comment count. -func TestCountNewCommentsSince_ExcludesAgentOwn(t *testing.T) { - if testHandler == nil || testPool == nil { - t.Skip("database not available") - } - fx := newCommentListFixture(t) - agentID := createHandlerTestAgent(t, "count-agent", []byte("[]")) - - // Two agent-authored comments — must NOT be counted. - for _, body := range []string{"agent reply 1", "agent reply 2"} { - if _, err := testPool.Exec(context.Background(), ` - INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type, parent_id) - VALUES ($1, $2, 'agent', $3, $4, 'comment', $5) - `, fx.IssueID, testWorkspaceID, agentID, body, fx.Root2); err != nil { - t.Fatalf("insert agent comment: %v", err) - } - } - - ctx := context.Background() - - // Anchor in the far past: all 7 member comments are newer; agent's 2 excluded. - got, err := testHandler.Queries.CountNewCommentsSince(ctx, db.CountNewCommentsSinceParams{ - IssueID: parseUUID(fx.IssueID), - WorkspaceID: parseUUID(testWorkspaceID), - Since: pgtype.Timestamptz{Time: time.Unix(0, 0), Valid: true}, - AuthorID: parseUUID(agentID), - }) - if err != nil { - t.Fatalf("count new since epoch: %v", err) - } - if got != 7 { - t.Fatalf("expected 7 new member comments since epoch (agent's own excluded), got %d", got) - } - - // Anchor in the future: nothing is newer. - got, err = testHandler.Queries.CountNewCommentsSince(ctx, db.CountNewCommentsSinceParams{ - IssueID: parseUUID(fx.IssueID), - WorkspaceID: parseUUID(testWorkspaceID), - Since: pgtype.Timestamptz{Time: time.Now().Add(time.Hour), Valid: true}, - AuthorID: parseUUID(agentID), - }) - if err != nil { - t.Fatalf("count new since future: %v", err) - } - if got != 0 { - t.Fatalf("expected 0 new comments after a future anchor, got %d", got) - } -} diff --git a/server/internal/handler/daemon.go b/server/internal/handler/daemon.go index 6591baa279..f5e4e34c17 100644 --- a/server/internal/handler/daemon.go +++ b/server/internal/handler/daemon.go @@ -1269,27 +1269,6 @@ func (h *Handler) ClaimTaskByRuntime(w http.ResponseWriter, r *http.Request) { } } } - // Count comments that arrived since this agent's last run on the - // issue, so the daemon can tell it up front instead of relying on - // it to self-fetch. Anchor = the prior task's started_at (never - // completed_at: a long run would miss comments posted while it - // ran). Cold start (no prior task) → no anchor → no hint. - // Excludes the agent's own comments. Best-effort: any DB error or - // zero count leaves the hint suppressed. - if startedAt, err := h.Queries.GetLastTaskStartedAtForIssueAndAgent(r.Context(), db.GetLastTaskStartedAtForIssueAndAgentParams{ - AgentID: task.AgentID, - IssueID: comment.IssueID, - }); err == nil && startedAt.Valid { - if cnt, err := h.Queries.CountNewCommentsSince(r.Context(), db.CountNewCommentsSinceParams{ - IssueID: comment.IssueID, - WorkspaceID: comment.WorkspaceID, - Since: startedAt, - AuthorID: task.AgentID, - }); err == nil && cnt > 0 { - resp.NewCommentCount = int(cnt) - resp.NewCommentsSince = startedAt.Time.UTC().Format(time.RFC3339) - } - } } } @@ -1299,19 +1278,18 @@ func (h *Handler) ClaimTaskByRuntime(w http.ResponseWriter, r *http.Request) { // Skip all prior state when the task was flagged as a manual rerun: // the user just judged the prior output bad, so the daemon must start a // fresh agent session in a fresh workdir instead of resuming anything - // from the same conversation that produced that output. + // from the same conversation that produced that output. For + // comment-triggered follow-ups, skip only the session resume: resumed + // issue conversations often inherit the prior final assistant message + // (for example "Done.") and answer a new human comment with that stale + // completion marker instead of the comment itself. Keep reusing the + // workdir for comment follow-ups so the agent still sees the same checkout. if !task.ForceFreshSession { if prior, err := h.Queries.GetLastTaskSession(r.Context(), db.GetLastTaskSessionParams{ AgentID: task.AgentID, IssueID: task.IssueID, }); err == nil && prior.SessionID.Valid { - // Resume the prior session when it ran on the same runtime — - // including comment-triggered follow-ups, so the agent keeps the - // issue's conversation context across turns. The "Focus on THIS - // comment" guard in prompt.go defends against inheriting the prior - // turn's "Done." marker, and GetLastTaskSession already excludes - // poisoned sessions. - if prior.RuntimeID == task.RuntimeID { + if !task.TriggerCommentID.Valid && prior.RuntimeID == task.RuntimeID { resp.PriorSessionID = prior.SessionID.String } if prior.WorkDir.Valid { diff --git a/server/internal/handler/daemon_test.go b/server/internal/handler/daemon_test.go index d734b789f7..51c1237c4f 100644 --- a/server/internal/handler/daemon_test.go +++ b/server/internal/handler/daemon_test.go @@ -2718,10 +2718,8 @@ func TestClaimTask_IssuePriorSessionRuntimeGuard(t *testing.T) { } task = claimTaskForRuntimeGuard(t, runtimeID, daemonID) - // Comment-triggered tasks now resume the prior session by default (same - // runtime), so the agent keeps the issue's conversation context across turns. - if task.PriorSessionID != "comment-prior-session" { - t.Fatalf("comment trigger: expected PriorSessionID='comment-prior-session' (resume default-on), got %q", task.PriorSessionID) + if task.PriorSessionID != "" { + t.Fatalf("comment trigger: expected empty PriorSessionID, got %q", task.PriorSessionID) } if task.PriorWorkDir != "/tmp/comment-prior-workdir" { t.Fatalf("comment trigger: expected PriorWorkDir='/tmp/comment-prior-workdir', got %q", task.PriorWorkDir) @@ -3440,128 +3438,3 @@ func TestMembershipCache_InvalidatedOnDeleteWorkspace(t *testing.T) { t.Fatal("DeleteWorkspace handler did not invalidate extra-member cache entry") } } - -// createCommentTriggeredClaimTask seeds a queued comment-triggered task whose -// trigger comment is rooted under parentID (nil → trigger is itself a root). -// Returns the task id and the trigger comment id. -func createCommentTriggeredClaimTask(t *testing.T, ctx context.Context, agentID, runtimeID, issueID string, parentID *string) (string, string) { - t.Helper() - var commentID string - if err := testPool.QueryRow(ctx, ` - INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type, parent_id) - VALUES ($1, $2, 'member', $3, 'trigger comment', 'comment', $4) - RETURNING id - `, issueID, testWorkspaceID, testUserID, parentID).Scan(&commentID); err != nil { - t.Fatalf("insert trigger comment: %v", err) - } - t.Cleanup(func() { testPool.Exec(ctx, `DELETE FROM comment WHERE id = $1`, commentID) }) - - var taskID string - if err := testPool.QueryRow(ctx, ` - INSERT INTO agent_task_queue (agent_id, runtime_id, issue_id, status, priority, trigger_comment_id) - VALUES ($1, $2, $3, 'queued', 0, $4) - RETURNING id - `, agentID, runtimeID, issueID, commentID).Scan(&taskID); err != nil { - t.Fatalf("insert comment-triggered task: %v", err) - } - t.Cleanup(func() { testPool.Exec(ctx, `DELETE FROM agent_task_queue WHERE id = $1`, taskID) }) - return taskID, commentID -} - -type claimCommentTaskResp struct { - Task *struct { - ID string `json:"id"` - PriorSessionID string `json:"prior_session_id"` - TriggerCommentID string `json:"trigger_comment_id"` - NewCommentCount int `json:"new_comment_count"` - NewCommentsSince string `json:"new_comments_since"` - } `json:"task"` -} - -func claimCommentTask(t *testing.T, runtimeID, daemonID string) claimCommentTaskResp { - t.Helper() - w := httptest.NewRecorder() - req := newDaemonTokenRequest("POST", "/api/daemon/runtimes/"+runtimeID+"/tasks/claim", nil, testWorkspaceID, daemonID) - req = withURLParam(req, "runtimeId", runtimeID) - testHandler.ClaimTaskByRuntime(w, req) - if w.Code != http.StatusOK { - t.Fatalf("ClaimTaskByRuntime: expected 200, got %d: %s", w.Code, w.Body.String()) - } - var resp claimCommentTaskResp - if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { - t.Fatalf("decode claim response: %v", err) - } - if resp.Task == nil { - t.Fatalf("expected a claimed task, got nil: %s", w.Body.String()) - } - return resp -} - -// TestClaimTaskByRuntime_CommentTaskPopulatesNewCommentCount verifies the claim -// response carries new_comment_count + new_comments_since for a comment task -// when the agent ran on this issue before: the count is comments created after -// the prior run's started_at, and the since anchor is that started_at. -func TestClaimTaskByRuntime_CommentTaskPopulatesNewCommentCount(t *testing.T) { - if testHandler == nil || testPool == nil { - t.Skip("database not available") - } - ctx := context.Background() - runtimeID := createClaimReclaimRuntime(t, ctx, "Comment newcount runtime") - agentID, issueID := createClaimReclaimAgentAndIssue(t, ctx, runtimeID, "Comment newcount agent") - - // A prior run establishes the "since" anchor (its started_at, in the past). - var priorTaskID string - if err := testPool.QueryRow(ctx, ` - INSERT INTO agent_task_queue (agent_id, runtime_id, issue_id, status, priority, started_at, completed_at) - VALUES ($1, $2, $3, 'completed', 0, now() - interval '1 hour', now() - interval '50 minutes') - RETURNING id - `, agentID, runtimeID, issueID).Scan(&priorTaskID); err != nil { - t.Fatalf("insert prior task: %v", err) - } - t.Cleanup(func() { testPool.Exec(ctx, `DELETE FROM agent_task_queue WHERE id = $1`, priorTaskID) }) - - // The trigger comment (member-authored, created now) lands after the anchor. - _, triggerID := createCommentTriggeredClaimTask(t, ctx, agentID, runtimeID, issueID, nil) - - resp := claimCommentTask(t, runtimeID, "comment-newcount-claim") - if resp.Task.TriggerCommentID != triggerID { - t.Fatalf("trigger_comment_id = %s, want %s", resp.Task.TriggerCommentID, triggerID) - } - if resp.Task.NewCommentsSince == "" { - t.Errorf("new_comments_since must be set when a prior run exists, got empty") - } - if resp.Task.NewCommentCount < 1 { - t.Errorf("new_comment_count = %d, want >= 1 (the trigger comment is newer than the anchor)", resp.Task.NewCommentCount) - } -} - -// TestClaimTaskByRuntime_CommentResumeDefaultOn verifies comment-triggered tasks -// resume the prior session by default (no env flag), as long as the prior -// session ran on the same runtime. -func TestClaimTaskByRuntime_CommentResumeDefaultOn(t *testing.T) { - if testHandler == nil || testPool == nil { - t.Skip("database not available") - } - ctx := context.Background() - runtimeID := createClaimReclaimRuntime(t, ctx, "Comment resume runtime") - agentID, issueID := createClaimReclaimAgentAndIssue(t, ctx, runtimeID, "Comment resume agent") - - // A prior completed task on the same (agent, issue, runtime) with a session. - const priorSession = "sess-prior-123" - var priorTaskID string - if err := testPool.QueryRow(ctx, ` - INSERT INTO agent_task_queue (agent_id, runtime_id, issue_id, status, priority, session_id, completed_at) - VALUES ($1, $2, $3, 'completed', 0, $4, now()) - RETURNING id - `, agentID, runtimeID, issueID, priorSession).Scan(&priorTaskID); err != nil { - t.Fatalf("insert prior completed task: %v", err) - } - t.Cleanup(func() { testPool.Exec(ctx, `DELETE FROM agent_task_queue WHERE id = $1`, priorTaskID) }) - - createCommentTriggeredClaimTask(t, ctx, agentID, runtimeID, issueID, nil) - - resp := claimCommentTask(t, runtimeID, "comment-resume-default") - if resp.Task.PriorSessionID != priorSession { - t.Errorf("prior_session_id = %q, want %q (comment resume is default-on)", resp.Task.PriorSessionID, priorSession) - } -} diff --git a/server/pkg/db/generated/agent.sql.go b/server/pkg/db/generated/agent.sql.go index 56a495d3d7..09385aae93 100644 --- a/server/pkg/db/generated/agent.sql.go +++ b/server/pkg/db/generated/agent.sql.go @@ -1403,31 +1403,6 @@ func (q *Queries) GetLastTaskSession(ctx context.Context, arg GetLastTaskSession return i, err } -const getLastTaskStartedAtForIssueAndAgent = `-- name: GetLastTaskStartedAtForIssueAndAgent :one -SELECT started_at FROM agent_task_queue -WHERE agent_id = $1 AND issue_id = $2 AND started_at IS NOT NULL -ORDER BY started_at DESC -LIMIT 1 -` - -type GetLastTaskStartedAtForIssueAndAgentParams struct { - AgentID pgtype.UUID `json:"agent_id"` - IssueID pgtype.UUID `json:"issue_id"` -} - -// Returns the started_at of the most recent prior task for this (agent, issue) -// pair, used as the "since" anchor for counting comments that arrived since the -// agent's last run. Any terminal state counts as "a run happened". Tasks with -// no started_at (never dispatched / the just-claimed current task) are excluded, -// so this never returns the current claim's own row. MUST use started_at, never -// completed_at: a long run would otherwise miss comments posted while it ran. -func (q *Queries) GetLastTaskStartedAtForIssueAndAgent(ctx context.Context, arg GetLastTaskStartedAtForIssueAndAgentParams) (pgtype.Timestamptz, error) { - row := q.db.QueryRow(ctx, getLastTaskStartedAtForIssueAndAgent, arg.AgentID, arg.IssueID) - var started_at pgtype.Timestamptz - err := row.Scan(&started_at) - return started_at, err -} - const getLatestTaskIsLeaderForIssueAndAgent = `-- name: GetLatestTaskIsLeaderForIssueAndAgent :one SELECT is_leader_task FROM agent_task_queue WHERE issue_id = $1 AND agent_id = $2 diff --git a/server/pkg/db/generated/comment.sql.go b/server/pkg/db/generated/comment.sql.go index 47b071fecc..897f79ee1e 100644 --- a/server/pkg/db/generated/comment.sql.go +++ b/server/pkg/db/generated/comment.sql.go @@ -28,37 +28,6 @@ func (q *Queries) CountComments(ctx context.Context, arg CountCommentsParams) (i return count, err } -const countNewCommentsSince = `-- name: CountNewCommentsSince :one -SELECT count(*) FROM comment -WHERE issue_id = $1 - AND workspace_id = $2 - AND created_at > $3 - AND NOT (author_type = 'agent' AND author_id = $4) -` - -type CountNewCommentsSinceParams struct { - IssueID pgtype.UUID `json:"issue_id"` - WorkspaceID pgtype.UUID `json:"workspace_id"` - Since pgtype.Timestamptz `json:"since"` - AuthorID pgtype.UUID `json:"author_id"` -} - -// Counts comments on an issue created strictly after @since, excluding any -// authored by the given agent (@author_id). Feeds the daemon claim response so -// a comment-triggered task can tell the agent how many comments arrived since -// its last run on this issue, without shipping their bodies. -func (q *Queries) CountNewCommentsSince(ctx context.Context, arg CountNewCommentsSinceParams) (int64, error) { - row := q.db.QueryRow(ctx, countNewCommentsSince, - arg.IssueID, - arg.WorkspaceID, - arg.Since, - arg.AuthorID, - ) - var count int64 - err := row.Scan(&count) - return count, err -} - const createComment = `-- name: CreateComment :one INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type, parent_id) VALUES ($1, $2, $3, $4, $5, $6, $7) diff --git a/server/pkg/db/queries/agent.sql b/server/pkg/db/queries/agent.sql index 606792c381..07649b11ba 100644 --- a/server/pkg/db/queries/agent.sql +++ b/server/pkg/db/queries/agent.sql @@ -389,18 +389,6 @@ WHERE agent_id = $1 AND issue_id = $2 ORDER BY COALESCE(completed_at, started_at, dispatched_at, created_at) DESC LIMIT 1; --- name: GetLastTaskStartedAtForIssueAndAgent :one --- Returns the started_at of the most recent prior task for this (agent, issue) --- pair, used as the "since" anchor for counting comments that arrived since the --- agent's last run. Any terminal state counts as "a run happened". Tasks with --- no started_at (never dispatched / the just-claimed current task) are excluded, --- so this never returns the current claim's own row. MUST use started_at, never --- completed_at: a long run would otherwise miss comments posted while it ran. -SELECT started_at FROM agent_task_queue -WHERE agent_id = $1 AND issue_id = $2 AND started_at IS NOT NULL -ORDER BY started_at DESC -LIMIT 1; - -- name: FailAgentTask :one -- Marks a task as failed. session_id and work_dir are merged via COALESCE so -- if the agent already established a real session before failing (e.g. it diff --git a/server/pkg/db/queries/comment.sql b/server/pkg/db/queries/comment.sql index e33770468f..8e9367a602 100644 --- a/server/pkg/db/queries/comment.sql +++ b/server/pkg/db/queries/comment.sql @@ -201,17 +201,6 @@ ORDER BY p.last_activity_at ASC, p.root_id ASC, c.created_at ASC, c.id ASC; SELECT count(*) FROM comment WHERE issue_id = $1 AND workspace_id = $2; --- name: CountNewCommentsSince :one --- Counts comments on an issue created strictly after @since, excluding any --- authored by the given agent (@author_id). Feeds the daemon claim response so --- a comment-triggered task can tell the agent how many comments arrived since --- its last run on this issue, without shipping their bodies. -SELECT count(*) FROM comment -WHERE issue_id = @issue_id - AND workspace_id = @workspace_id - AND created_at > @since - AND NOT (author_type = 'agent' AND author_id = @author_id); - -- name: GetComment :one SELECT * FROM comment WHERE id = $1;