diff --git a/server/internal/daemon/prompt.go b/server/internal/daemon/prompt.go index dd63a23976..36d67cabdc 100644 --- a/server/internal/daemon/prompt.go +++ b/server/internal/daemon/prompt.go @@ -275,8 +275,40 @@ func buildCommentPrompt(task Task, provider string) string { } fmt.Fprintf(&b, "\nIf you need the surrounding discussion for any of them, fetch its thread with `multica issue comment list %s --thread --tail 30 --output json` using the thread id shown above.\n\n", task.IssueID) } else if len(task.CoalescedCommentIDs) > 0 { - fmt.Fprintf(&b, "This run also covers %d earlier comment(s) posted before it started — you must read and address them too, not just the one above: %s. These may be in DIFFERENT threads, so do not assume they share the triggering thread; fetch each by pulling the issue-wide discussion with `multica issue comment list %s --recent 30 --output json` (expand with `--full` if a thread is folded) and locate the ids above.\n\n", - len(task.CoalescedCommentIDs), strings.Join(task.CoalescedCommentIDs, ", "), task.IssueID) + // MUL-5442: this fallback used to send the agent at `--recent 30`. + // That flag caps THREADS, not comments, and every returned thread + // carries all of its descendants — so on an issue with fewer than 30 + // root threads it returned the entire comment history to locate a + // handful of ids. It also contradicted the brief's own catch-up step, + // which tells the agent to read in two bounded steps and never make + // one bulk pull (MUL-5372): the platform was recommending exactly the + // shape it forbids elsewhere. + // + // The replacement is a per-id lookup, which is what makes it + // deterministic: `--thread` accepts ANY comment id, reply or root, and + // the server resolves it to the containing thread. So each id can be + // fetched directly and bounded, without knowing its thread and without + // guessing which threads look recent. + // + // `--since` is only a prefetch, never the guarantee. Two ways it can + // miss an id, so the per-id pass below is unconditional: + // - A retry inherits the previous attempt's coalesced_comment_ids + // verbatim (queries/agent.sql RetryTask), while the anchor is + // recomputed from the last STARTED task's started_at + // (GetLastTaskStartedAtForIssueAndAgent). An inherited id can + // therefore predate the anchor. + // - The anchor is only populated when some comment landed after it, + // which is independent of where these ids sit. + // It is also not a precise fetch in the other direction: the window + // carries the trigger comment and unrelated comments too. + fmt.Fprintf(&b, "This run also covers %d earlier comment(s) posted before it started — you must read and address every one of them, not just the one above: %s. They may be in DIFFERENT threads, so do not assume they share the triggering thread.\n\n", + len(task.CoalescedCommentIDs), strings.Join(task.CoalescedCommentIDs, ", ")) + if task.NewCommentsSince != "" { + fmt.Fprintf(&b, "Start with `multica issue comment list %s --since %s --output json`. Treat that as a candidate window, not a guarantee — it also carries unrelated comments, and a retried run can carry ids older than the window. Check every id above against the result.\n\n", + task.IssueID, task.NewCommentsSince) + } + fmt.Fprintf(&b, "Fetch each id you still need directly: `multica issue comment list %s --thread --tail 30 --output json`. `--thread` accepts a reply id, not just a thread root, so you do not need to know which thread the comment lives in. If it is older than those 30 replies, page back with the `Next reply cursor` values (`--before` / `--before-id`) until it appears. Do not finish this turn until every id above is accounted for.\n\n", + task.IssueID) } if task.TriggerAuthorType == "agent" { b.WriteString("⚠️ The triggering comment was posted by another agent. Decide whether a reply is warranted. If you produced actual work this turn (investigated, fixed something, answered a real question), post the result as a normal reply — that is NOT a noise comment, and the standard rule that final results must be delivered via comment still applies. If the triggering comment was a pure acknowledgment, thanks, or sign-off AND you produced no work this turn, do NOT reply — and do NOT post a comment saying 'No reply needed' or similar. Simply exit with no output. Silence is the preferred way to end agent-to-agent threads. If you do reply, do not @mention the other agent as a sign-off (that re-triggers them and starts a loop).\n\n") diff --git a/server/internal/daemon/prompt_test.go b/server/internal/daemon/prompt_test.go index f2e91cfa7c..2efde9f749 100644 --- a/server/internal/daemon/prompt_test.go +++ b/server/internal/daemon/prompt_test.go @@ -841,9 +841,14 @@ func TestBuildCommentPromptCoalescedCrossThread(t *testing.T) { // TestBuildCommentPromptCoalescedIDsOnlyFallback pins the old-server fallback: // when only coalesced ids are shipped (no embedded detail), the prompt must -// still NOT assume a shared thread and must point at an issue-wide fetch. +// still NOT assume a shared thread, and must reach the ids through a BOUNDED +// read rather than an issue-wide bulk pull (MUL-5442). +// +// The bulk pull is the regression this guards: `--recent N` caps threads, not +// comments, so on a small issue it returns the whole history — and the brief's +// own catch-up step forbids exactly that shape. func TestBuildCommentPromptCoalescedIDsOnlyFallback(t *testing.T) { - task := Task{ + base := Task{ IssueID: "issue-fallback-1", TriggerCommentID: "trigger-newest", TriggerThreadID: "thread-root-A", @@ -851,13 +856,74 @@ func TestBuildCommentPromptCoalescedIDsOnlyFallback(t *testing.T) { TriggerAuthorType: "member", CoalescedCommentIDs: []string{"c-old-1", "c-old-2"}, } - out := BuildPrompt(task, "claude") + t.Run("with since anchor", func(t *testing.T) { + task := base + task.NewCommentsSince = "2026-08-03T06:00:00Z" + out := BuildPrompt(task, "claude") + + want := "multica issue comment list issue-fallback-1 --since 2026-08-03T06:00:00Z --output json" + if !strings.Contains(out, want) { + t.Errorf("id-only fallback should prefetch the window with %q, got:\n%s", want, out) + } + // The window is a prefetch, never the guarantee: a retry inherits the + // prior attempt's coalesced ids verbatim while the anchor is recomputed + // from the last started task, so an inherited id can predate the window. + // The prompt must say so and must not promise an exact fetch. + for _, want := range []string{"candidate window, not a guarantee", "can carry ids older than the window"} { + if !strings.Contains(out, want) { + t.Errorf("anchored fallback must not present --since as complete, missing %q, got:\n%s", want, out) + } + } + for _, banned := range []string{"returns exactly the comments", "precisely"} { + if strings.Contains(out, banned) { + t.Errorf("anchored fallback must not overpromise the window (%q), got:\n%s", banned, out) + } + } + assertBoundedIDOnlyFallback(t, out) + }) + + t.Run("without since anchor", func(t *testing.T) { + // No prior run on this issue, so the server sent no anchor. The per-id + // lookup below is the whole contract here. + out := BuildPrompt(base, "claude") + + if strings.Contains(out, "--since") { + t.Errorf("anchorless fallback must not emit a --since read, got:\n%s", out) + } + // No heuristics: the agent must not be asked to guess which threads look + // recent enough to hold the ids (MUL-5442 review). + if strings.Contains(out, "last_activity_at") { + t.Errorf("anchorless fallback must not rely on a recency heuristic, got:\n%s", out) + } + assertBoundedIDOnlyFallback(t, out) + }) +} + +// assertBoundedIDOnlyFallback holds the completeness contract both fallback +// shapes must satisfy: every listed id is reachable deterministically, through +// bounded reads, without a bulk pull. +func assertBoundedIDOnlyFallback(t *testing.T, out string) { + t.Helper() if strings.Contains(out, "they are in the triggering thread") { t.Errorf("id-only fallback must not assume a shared thread, got:\n%s", out) } - if !strings.Contains(out, "--recent 30") { - t.Errorf("id-only fallback must point at an issue-wide fetch (--recent 30), got:\n%s", out) + if strings.Contains(out, "--recent") { + t.Errorf("id-only fallback must not send the agent at an issue-wide --recent pull (MUL-5442), got:\n%s", out) + } + // The deterministic per-id lookup. `--thread` resolves ANY comment id, so an + // id is reachable without knowing its thread; paging keeps it reachable even + // when it is older than the tail window. + for _, want := range []string{ + "multica issue comment list issue-fallback-1 --thread --tail 30 --output json", + "accepts a reply id", + "Next reply cursor", + "--before-id", + "Do not finish this turn until every id above is accounted for", + } { + if !strings.Contains(out, want) { + t.Errorf("id-only fallback missing per-id completeness guarantee %q, got:\n%s", want, out) + } } for _, id := range []string{"c-old-1", "c-old-2"} { if !strings.Contains(out, id) {