Files
multica/server/internal/daemon/prompt_test.go
Bohan Jiang 7f9e4e829d feat(comments): thread-internal --tail pagination + reply cursor (MUL-2421) (#2846)
* feat(comments): thread-internal pagination via --tail + reply cursor (MUL-2421)

Long threads inside a single issue still forced agents to read every reply
once they used --thread, even after MUL-2387 fixed cross-thread noise. This
adds reply-level paging so a 200-reply thread can be navigated tail-first
without dragging the whole conversation into prompt context.

- New SQL query ListThreadCommentsForIssuePaged: same recursive root walk
  as the legacy thread query, but caps reply count and supports an
  (created_at, id) composite cursor. Root is unconditional — even tail=0
  emits it so the reader keeps the "what is this thread about" context.
- Handler ListComments: parses `tail` (non-negative, ThreadTailSet flag
  preserves the tail=0 intent), threads it through to the paged query,
  and re-uses X-Multica-Next-Before / X-Multica-Next-Before-Id for the
  reply cursor. Cursor's meaning is now context-dependent: thread cursor
  under --recent, reply cursor under --thread + --tail.
- CLI: new --tail flag (only valid with --thread; mutually exclusive
  with --recent), reply-cursor semantics for --before / --before-id when
  paired with --thread + --tail, stderr label flips to "Next reply cursor"
  so an operator copy-pasting the cursor knows which scope it scrolls.
- Tests cover the new contract: tail=N keeps newest N + root, tail=0 is
  root-only, anchor on a nested reply still walks up, reply cursor
  scrolls older replies page-by-page, since combined with tail filters
  after the cut, and the negative-flag-combination matrix.

Out of scope: prompt template update to hint at `--thread <id> --tail 30`
on long threads — separate follow-up per the issue.

Co-authored-by: multica-agent <github@multica.ai>

* fix(comments): only emit reply cursor when older reply exists (MUL-2421)

The thread-tail path emitted `X-Multica-Next-Before` whenever the page
filled to exactly the requested reply count, even when there was nothing
older to scroll to. So `--thread <root> --tail 3` on a thread with
exactly 3 replies sent a cursor that, when followed, returned just the
root — a wasted round-trip that surfaced as a phantom "older replies"
affordance in the agent prompt.

Switch to a `reply_limit + 1` probe: ask the SQL for one extra row, trim
the oldest overflow before responding, and only emit the cursor when an
older reply actually existed. The exact-boundary case (replyCount ==
tail with no overflow) now returns no cursor.

Also documents `--thread/--tail/--recent/--before` and the cursor
semantics in CLI_AND_DAEMON.md, which was the second must-fix in the
MUL-2421 review.

Co-authored-by: multica-agent <github@multica.ai>

* fix(comments): suppress reply cursor when --since covers older replies (MUL-2421)

In the thread + tail + since path the server still emitted a reply cursor
whenever there was an older reply on disk, regardless of `since`. If the
oldest retained reply on the page was already `<= since`, every older
reply was guaranteed to be filtered out too, so the next page only ever
returned the root — wasting round-trips until the agent walked the whole
pre-`since` history. Mirror the recent + since suppression: when
`replies[0].CreatedAt <= since`, drop the cursor.

Test covers the exact case from Elon's review: tail=2 overflow, body
keeps a fresher reply, but the cursor target (oldest retained reply) is
already past `since` — header must be empty.

Co-authored-by: multica-agent <github@multica.ai>

* feat(prompt): default comment-trigger reads to --thread --tail 30 (MUL-2421)

Comment-triggered agents previously defaulted the trigger-thread read to
the unbounded `--thread <id> --output json`, which dumps the full thread
into the prompt — exactly the kind of context bloat MUL-2387 fixed at the
cross-thread layer but never bounded inside a single thread.

Use the new `--tail` flag landed earlier in this PR (server + CLI) as the
default for both the per-turn prompt and the runtime-config Workflow:

- `--thread <trigger-id> --tail 30 --output json` is the new default.
  Root is always included so "what is this about" context survives.
- If 30 replies aren't enough, the prompt now spells out the reply
  cursor: re-feed the stderr `Next reply cursor: --before <ts>
  --before-id <reply-id>` pair back to walk older replies.
- `--recent 20` stays as the cross-thread background fallback, with an
  explicit callout that the same `--before` / `--before-id` flags walk
  *threads* (not replies) in that mode.
- Available Commands core line now surfaces `--tail N` and both stderr
  cursor labels so non-workflow callers also discover the flag.
- `--since` callouts reflect the post-MUL-2421 combinable mode names
  (`--thread --tail` / `--recent`).

Tests (`prompt_test.go`, `execenv_test.go`) pin the new defaults and add
a regression guard against the unbounded `--thread` recipe sneaking back
in.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: multica-agent <github@multica.ai>
2026-05-21 13:43:15 +08:00

313 lines
13 KiB
Go

package daemon
import (
"strings"
"testing"
)
// TestBuildQuickCreatePromptRules locks in the rules that govern how the
// quick-create agent is allowed to translate raw user input into the issue
// description body. Each substring corresponds to a concrete failure mode
// observed in production output:
// - meta-instructions ("create an issue", "cc @X") leaking into the body
// - the Context section being misused as an apology log when no external
// references were actually fetched
// - hard-line rules being silently dropped on prompt rewrites
func TestBuildQuickCreatePromptRules(t *testing.T) {
out := buildQuickCreatePrompt(Task{QuickCreatePrompt: "fix the login button color"})
mustContain := []string{
// high-fidelity invariant
"Faithfully restate what the user wants",
"Preserve specific names, identifiers, file paths",
// strip non-spec material: verbal routing wrappers + conversational fillers
"verbal routing wrappers about creating the issue",
"pure conversational fillers",
// cc routing must survive: mention link stays in description so the
// auto-subscribe path fires (multica issue create has no --subscriber flag)
"CC exception",
"auto-subscribes members",
// context section is conditional and must not be an apology log
"include ONLY when the input cited external resources",
"never use it as an apology log",
// output/reporting must be workspace-prefix agnostic. Workspaces can
// use custom issue prefixes, so a successful issue creation should
// not look failed merely because the identifier does not match one
// fixed prefix.
"multica issue create --output json",
"JSON response",
"identifier",
"Do not scrape human output",
"do not assume any workspace issue prefix",
"Created <identifier-or-id>: <title>",
// hard rules
"never invent requirements",
"never reduce multi-sentence input",
}
for _, s := range mustContain {
if !strings.Contains(out, s) {
t.Errorf("buildQuickCreatePrompt output missing required rule: %q", s)
}
}
}
// TestBuildQuickCreatePromptAssigneeIncludesSquads locks in the MUL-2165
// fix: the assignee-resolution rules must tell the agent to consult the
// squad list alongside members and agents. Before this, a quick-create
// input like "assign to <SquadName>" silently fell through to
// "Unrecognized assignee" because squads were never queried.
func TestBuildQuickCreatePromptAssigneeIncludesSquads(t *testing.T) {
out := buildQuickCreatePrompt(Task{QuickCreatePrompt: "fix the login button color"})
mustContain := []string{
"multica squad list",
"Squads are first-class assignees",
"Treat bare @-routing as an assignee directive",
"让 @独立团 review 这个 PR",
"pass the squad's `id` as `--assignee-id`",
}
for _, s := range mustContain {
if !strings.Contains(out, s) {
t.Errorf("buildQuickCreatePrompt assignee block missing %q\n--- output ---\n%s", s, out)
}
}
}
// TestBuildQuickCreatePromptSquadDefaultsToSquad locks in the MUL-2203
// fix: when the picker was a squad, the task runs on the squad's leader
// agent, but the default assignee for issues created by this run must
// point at the SQUAD's UUID — not the leader agent's UUID. The previous
// "default to YOURSELF" instruction made squad-created issues land under
// the leader, hiding them from the squad's delegation flow.
func TestBuildQuickCreatePromptSquadDefaultsToSquad(t *testing.T) {
const (
squadID = "aaaa1111-2222-3333-4444-555555555555"
squadName = "独立团"
leaderID = "bbbb1111-2222-3333-4444-666666666666"
)
out := buildQuickCreatePrompt(Task{
QuickCreatePrompt: "fix the login button color",
Agent: &AgentData{ID: leaderID, Name: "leader-agent"},
SquadID: squadID,
SquadName: squadName,
})
// The default-assignee instruction must point at the squad UUID.
if !strings.Contains(out, "--assignee-id \""+squadID+"\"") {
t.Errorf("buildQuickCreatePrompt with SquadID must default to the squad's UUID, got:\n%s", out)
}
// And it must NOT tell the agent to default to itself (the leader).
if strings.Contains(out, "--assignee-id \""+leaderID+"\"") {
t.Errorf("buildQuickCreatePrompt with SquadID must NOT default to the leader agent's UUID, got:\n%s", out)
}
// The squad name should appear in the instruction so the agent has
// human-readable context for the routing decision.
if !strings.Contains(out, squadName) {
t.Errorf("buildQuickCreatePrompt with SquadID should mention the squad name %q, got:\n%s", squadName, out)
}
// And the prompt must explicitly call out the squad-vs-leader rule
// so the agent does not silently regress to "default to YOURSELF".
mustContain := []string{
"picker SQUAD",
"running on the squad's behalf",
"do not assign it to your own agent UUID",
}
for _, s := range mustContain {
if !strings.Contains(out, s) {
t.Errorf("buildQuickCreatePrompt with SquadID missing %q\n--- output ---\n%s", s, out)
}
}
}
// TestBuildQuickCreatePromptProjectPinning verifies that when the user
// pins a project in the quick-create modal, the prompt instructs the agent
// to pass `--project <uuid>` exactly. Without this, the agent would re-read
// the workspace default and silently drop the user's selection — the same
// "I have to retype 'in project X' every time" failure mode the modal
// addition was meant to fix.
func TestBuildQuickCreatePromptProjectPinning(t *testing.T) {
const projectID = "11111111-2222-3333-4444-555555555555"
out := buildQuickCreatePrompt(Task{
QuickCreatePrompt: "fix the login button color",
ProjectID: projectID,
ProjectTitle: "Web App",
})
mustContain := []string{
"--project \"" + projectID + "\"",
"Web App",
"modal selection is authoritative",
}
for _, s := range mustContain {
if !strings.Contains(out, s) {
t.Errorf("buildQuickCreatePrompt with project missing %q\n--- output ---\n%s", s, out)
}
}
// Without a project, the prompt must keep the legacy "omit" instruction
// so the agent doesn't accidentally start passing --project on plain
// quick-create runs.
plain := buildQuickCreatePrompt(Task{QuickCreatePrompt: "fix the login button color"})
if !strings.Contains(plain, "**project**: omit") {
t.Errorf("buildQuickCreatePrompt without project must keep the omit instruction, got:\n%s", plain)
}
if strings.Contains(plain, "--project") {
t.Errorf("buildQuickCreatePrompt without project must NOT mention --project, got:\n%s", plain)
}
}
// TestBuildPromptSquadLeaderNoActionForMemberTrigger verifies that the
// squad leader no_action prohibition is injected in the per-turn prompt
// regardless of whether the triggering comment was posted by an agent or
// a member. This was the root cause of the "LGTM is a pure acknowledgment
// — no reply needed. Exiting silently." noise comment: the prohibition
// only fired for agent-triggered comments, so member-triggered ones
// (like "LGTM") bypassed it.
func TestBuildPromptSquadLeaderNoActionForMemberTrigger(t *testing.T) {
task := Task{
IssueID: "issue-123",
TriggerCommentID: "comment-456",
TriggerCommentContent: "LGTM",
TriggerAuthorType: "member",
TriggerAuthorName: "Bohan",
Agent: &AgentData{
Instructions: "Some instructions\n\n## Squad Operating Protocol\n\nYou are the LEADER...",
},
}
out := BuildPrompt(task, "claude")
if !strings.Contains(out, "Squad leader no_action rule") {
t.Errorf("buildCommentPrompt must inject squad leader no_action rule for member-triggered comments, got:\n%s", out)
}
if !strings.Contains(out, "DO NOT post any comment") {
t.Errorf("buildCommentPrompt must contain DO NOT post prohibition for member-triggered squad leader, got:\n%s", out)
}
}
// TestBuildPromptSquadLeaderNoActionForAgentTrigger verifies the rule also
// fires for agent-triggered comments (the original path that already worked).
func TestBuildPromptSquadLeaderNoActionForAgentTrigger(t *testing.T) {
task := Task{
IssueID: "issue-123",
TriggerCommentID: "comment-456",
TriggerCommentContent: "Deploy complete.",
TriggerAuthorType: "agent",
TriggerAuthorName: "deploy-boy",
Agent: &AgentData{
Instructions: "Some instructions\n\n## Squad Operating Protocol\n\nYou are the LEADER...",
},
}
out := BuildPrompt(task, "claude")
if !strings.Contains(out, "Squad leader no_action rule") {
t.Errorf("buildCommentPrompt must inject squad leader no_action rule for agent-triggered comments, got:\n%s", out)
}
}
// TestBuildPromptCommentTriggerPromotesThreadReads pins MUL-2387 + MUL-2421:
// the per-turn prompt for a comment-triggered task must default the trigger
// thread read to `--thread <id> --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 <reply-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
// to the flat dump, even though it cannot anchor a --thread without a
// trigger comment id.
func TestBuildPromptDefaultMentionsRecent(t *testing.T) {
out := BuildPrompt(Task{IssueID: "issue-default-1"}, "claude")
for _, s := range []string{
"--recent 20 --output json",
"Next thread cursor:",
"--since",
} {
if !strings.Contains(out, s) {
t.Errorf("default BuildPrompt missing %q\n--- output ---\n%s", s, out)
}
}
// And the default path must NOT inject a --thread example, because there
// is no trigger comment id to anchor on.
if strings.Contains(out, "--thread") {
t.Errorf("default BuildPrompt should NOT mention --thread (no trigger comment to anchor on)\n--- output ---\n%s", out)
}
// The legacy "If you need comment history" soft phrasing conflicts with
// the assignment-trigger runtime workflow, which treats reading comments
// as mandatory. Guard against it sneaking back in.
if strings.Contains(out, "If you need comment history") {
t.Errorf("default BuildPrompt still carries the legacy 'If you need' soft phrasing that conflicts with the mandatory workflow\n--- output ---\n%s", out)
}
}
// TestBuildPromptNonSquadLeaderNoRule verifies that non-squad-leader agents
// do NOT get the squad leader no_action rule injected.
func TestBuildPromptNonSquadLeaderNoRule(t *testing.T) {
task := Task{
IssueID: "issue-123",
TriggerCommentID: "comment-456",
TriggerCommentContent: "LGTM",
TriggerAuthorType: "member",
TriggerAuthorName: "Bohan",
Agent: &AgentData{
Instructions: "Some instructions without the squad marker",
},
}
out := BuildPrompt(task, "claude")
if strings.Contains(out, "Squad leader no_action rule") {
t.Errorf("buildCommentPrompt must NOT inject squad leader no_action rule for non-squad-leader agents, got:\n%s", out)
}
}