fix(daemon): route the issue workflow on an explicit turn-mode marker

Review follow-up on the mode router. The brief said Reply mode applies when the per-turn message "opens with a [NEW COMMENT] block", but buildCommentPrompt writes two paragraphs before that block and only emits it when TriggerCommentContent is non-empty. Two ways to get it wrong: - The message never literally opens with the block, so the router's own wording did not match the prompt it describes. - A comment-triggered run with an empty comment body — or an older server that does not send one — emitted no block at all. An agent following the brief would fall through to Ownership mode and change the issue status on a turn whose rule is "do NOT change the issue status". BuildPrompt now emits an unconditional `**Turn mode: Reply.**` / `**Turn mode: Ownership.**` line from the same branches it uses to pick a code path, and the brief routes on that marker. Brief and prompt can no longer disagree about the mode, because the value that selects the path also states it. The router also names a safe fallback (treat an unlabelled turn as Reply mode and leave the status alone). Tests: TestTurnModeMarkerAlwaysPresent covers comment-triggered with and without comment content, plus both assignment shapes; TestTurnModeMarkerAbsentOnIssuelessKinds keeps the marker off chat / quick-create / autopilot; TestBriefModeRouterMatchesPromptMarkers fails if the brief ever describes a marker the prompt does not emit. Co-authored-by: multica-agent <[email protected]>
2026-11-08 01:16:45 +08:00 · 2026-07-28 15:23:07 +08:00
parent 43e425a5bb
commit 1ea3cf1d70
4 changed files with 135 additions and 5 deletions
@@ -198,7 +198,8 @@ func TestInjectRuntimeConfigKeepsTriggerCommentOutOfBrief(t *testing.T) {
 	}
 	for _, want := range []string{
 		"Mode router",
-		"`[NEW COMMENT]` block",
+		"`Turn mode: Reply.`",
+		"`Turn mode: Ownership.`",
 		"Use the `--parent` value the per-turn user message gives you for this turn",
 		"do NOT reuse a `--parent` from an earlier turn in this session",
 	} {
@@ -458,10 +458,10 @@ func writeWorkflowAutopilot(b *strings.Builder, ctx TaskContextForEnv) {
 // ctx.IsSquadLeader is agent configuration, not per-run state, so branching
 // on it does not break byte-stability across runs of one session.
 func writeWorkflowIssue(b *strings.Builder, ctx TaskContextForEnv) {
-	b.WriteString("**Mode router — read this before acting.** This file is identical on every run, so it cannot tell you what triggered THIS turn. The user message at the start of this turn does:\n\n")
-	b.WriteString("- It opens with a `[NEW COMMENT]` block → **Reply mode**. That message carries the triggering comment's id, its content, and the exact `--parent` value for your reply.\n")
-	b.WriteString("- It does not → **Ownership mode** (an assignment or status change started this run).\n\n")
-	b.WriteString("Steps 1–6 below are the same in both modes. The mode blocks after them differ, and they differ on issue status in particular — **apply exactly one mode block, the one the user message selected. Never apply both.**\n\n")
+	b.WriteString("**Mode router — read this before acting.** This file is identical on every run, so it cannot tell you what triggered THIS turn. The user message for this turn names its mode on a line of its own:\n\n")
+	b.WriteString("- `Turn mode: Reply.` → **Reply mode**. That message also carries the triggering comment's id, the exact `--parent` value for your reply, and the comment's content when the platform supplied it.\n")
+	b.WriteString("- `Turn mode: Ownership.` → **Ownership mode** (an assignment or status change started this run).\n\n")
+	b.WriteString("Steps 1–6 below are the same in both modes. The mode blocks after them differ, and they differ on issue status in particular — **apply exactly one mode block, the one the user message named. Never apply both.** If neither line is present, treat the turn as Reply mode and do not change the issue status.\n\n")

 	b.WriteString("**Steps 1–6 — both modes**\n\n")
 	fmt.Fprintf(b, "1. Run `multica issue get %s --output json` to understand the issue context\n", ctx.IssueID)
@@ -21,6 +21,20 @@ func freshSessionRetryPrompt(prompt string) string {
 	return notice + prompt
 }

+// Turn-mode markers consumed by the runtime brief's mode router
+// (execenv.writeWorkflowIssue). The brief is byte-identical on every run and
+// therefore cannot say what triggered this turn; these lines do, and they are
+// emitted unconditionally from the same branches BuildPrompt uses to pick a
+// path, so the two can never disagree.
+//
+// Reply mode = respond to the triggering comment, do not touch issue status.
+// Ownership mode = an assignment/status change started this run; own the
+// status arc. Applying the wrong one silently changes issue status.
+const (
+	turnModeReply     = "**Turn mode: Reply.** Follow the Reply-mode block in your runtime workflow file for this turn; the Ownership-mode status steps do not apply.\n\n"
+	turnModeOwnership = "**Turn mode: Ownership.** Follow the Ownership-mode block in your runtime workflow file for this turn; the Reply-mode rules do not apply.\n\n"
+)
+
 // perTurnContextBlocks renders the run-scoped context blocks that used to live
 // in the runtime brief (CLAUDE.md / AGENTS.md).
 //
@@ -81,6 +95,7 @@ func buildPromptBody(task Task, provider string) string {
 	var b strings.Builder
 	b.WriteString("You are running as a local coding agent for a Multica workspace.\n\n")
 	fmt.Fprintf(&b, "Your assigned issue ID is: %s\n\n", task.IssueID)
+	b.WriteString(turnModeOwnership)
 	// Assignment handoff (MUL-3375): a free-text instruction the person who
 	// assigned/promoted this issue left for you. Frame it as a handoff, not a
 	// comment to reply to — there is no comment thread to answer here.
@@ -207,6 +222,13 @@ func buildCommentPrompt(task Task, provider string) string {
 	var b strings.Builder
 	b.WriteString("You are running as a local coding agent for a Multica workspace.\n\n")
 	fmt.Fprintf(&b, "Your assigned issue ID is: %s\n\n", task.IssueID)
+	// Mode marker for the brief's router. Emitted unconditionally from the same
+	// branch that selects this code path, so the brief and the prompt can never
+	// disagree about which mode this turn is in. It must NOT be gated on
+	// TriggerCommentContent: an empty comment body (or an older server that
+	// doesn't send one) would otherwise leave the turn unlabelled, and the
+	// agent would fall through to Ownership mode and change the issue status.
+	b.WriteString(turnModeReply)
 	if task.TriggerCommentContent != "" {
 		authorLabel := "A user"
 		if task.TriggerAuthorType == "agent" {
@@ -1100,3 +1100,110 @@ func TestPerTurnContextBlocksOnAssignmentPath(t *testing.T) {
 		t.Errorf("assignment-triggered prompt lost the initiator block\n---\n%s", prompt)
 	}
 }
+
+// TestTurnModeMarkerAlwaysPresent is the regression guard for the review
+// finding on #6021: the brief's mode router keys off an explicit marker in the
+// per-turn prompt, so that marker must be emitted unconditionally from the same
+// branch that selects the code path.
+//
+// The dangerous case is a comment-triggered run whose comment body is empty (or
+// an older server that doesn't send one). Before this guard the prompt emitted
+// no `[NEW COMMENT]` block at all, the brief fell through to Ownership mode,
+// and the agent would change the issue status on a turn that must not.
+func TestTurnModeMarkerAlwaysPresent(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name string
+		task Task
+		want string
+		deny string
+	}{
+		{
+			name: "comment-triggered with content",
+			task: Task{IssueID: "issue-1", TriggerCommentID: "c-1", TriggerCommentContent: "please look"},
+			want: "**Turn mode: Reply.**",
+			deny: "**Turn mode: Ownership.**",
+		},
+		{
+			name: "comment-triggered with EMPTY content",
+			task: Task{IssueID: "issue-1", TriggerCommentID: "c-1"},
+			want: "**Turn mode: Reply.**",
+			deny: "**Turn mode: Ownership.**",
+		},
+		{
+			name: "assignment-triggered",
+			task: Task{IssueID: "issue-1"},
+			want: "**Turn mode: Ownership.**",
+			deny: "**Turn mode: Reply.**",
+		},
+		{
+			name: "assignment-triggered with handoff note",
+			task: Task{IssueID: "issue-1", HandoffNote: "start with the API"},
+			want: "**Turn mode: Ownership.**",
+			deny: "**Turn mode: Reply.**",
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			prompt := BuildPrompt(tc.task, "claude")
+			if !strings.Contains(prompt, tc.want) {
+				t.Errorf("prompt missing turn-mode marker %q\n---\n%s", tc.want, prompt)
+			}
+			if strings.Contains(prompt, tc.deny) {
+				t.Errorf("prompt carries the wrong turn-mode marker %q\n---\n%s", tc.deny, prompt)
+			}
+		})
+	}
+}
+
+// The mode marker only makes sense for the two issue paths — the issue-less
+// kinds have no Reply/Ownership distinction and no issue status to protect.
+func TestTurnModeMarkerAbsentOnIssuelessKinds(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name string
+		task Task
+	}{
+		{"chat", Task{ChatSessionID: "chat-1"}},
+		{"quick-create", Task{QuickCreatePrompt: "make an issue"}},
+		{"autopilot", Task{AutopilotRunID: "run-1"}},
+	} {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			prompt := BuildPrompt(tc.task, "claude")
+			for _, banned := range []string{"**Turn mode: Reply.**", "**Turn mode: Ownership.**"} {
+				if strings.Contains(prompt, banned) {
+					t.Errorf("%s prompt must not carry %q\n---\n%s", tc.name, banned, prompt)
+				}
+			}
+		})
+	}
+}
+
+// The brief's router must describe the markers the prompt actually emits.
+// A drift here is exactly the bug this pair of changes fixes, and it is
+// invisible at runtime until an agent silently picks the wrong mode.
+func TestBriefModeRouterMatchesPromptMarkers(t *testing.T) {
+	t.Parallel()
+
+	brief, err := execenv.InjectRuntimeConfig(t.TempDir(), "claude", execenv.TaskContextForEnv{IssueID: "issue-1"})
+	if err != nil {
+		t.Fatalf("InjectRuntimeConfig: %v", err)
+	}
+	for _, want := range []string{"`Turn mode: Reply.`", "`Turn mode: Ownership.`"} {
+		if !strings.Contains(brief, want) {
+			t.Errorf("brief mode router does not name %s\n---\n%s", want, brief)
+		}
+	}
+	// The retired wording keyed off the prompt's first line, which was never
+	// actually the [NEW COMMENT] block.
+	if strings.Contains(brief, "It opens with a `[NEW COMMENT]` block") {
+		t.Error("brief still routes on the prompt's opening line; it must route on the explicit marker")
+	}
+}