refactor(daemon): cross-channel command dedup — brief states the loop, per-turn carries the commands (MUL-5442) (#6421)

* refactor(daemon): move the workflow's command templates to the per-turn channel (MUL-5442) Cross-channel dedup, brief side. MUL-5721 measured that every issue-turn variant of the per-turn message already carries the ready-to-run commands with real ids (issue-get line, reading hints, reply cookbook), fresher than the brief's static copies. The six workflow steps now state the loop shape — command names and flag mnemonics only; full templates and every embedded issue UUID leave the brief. The Ownership status commands and the squad-activity call switch to <issue-id> placeholder form. Doctrine pins stay on the brief side (mandatory catch-up, scan-first order, both motivation anchors); command-template pins re-anchor to names and mnemonics. Two design-consequence test updates: the static-catch-up assertion repoints at the doctrine (the full command now lives in every per-turn variant), and the byte-identity non-vacuity guard varies agent identity instead of issue id — because the brief is now deliberately issue-id-independent, which opens the cross-issue prefix-cache door. Co-authored-by: multica-agent <[email protected]> * refactor(daemon): point the reply cookbook at Comment Formatting instead of restating it (MUL-5442) Cross-channel dedup, per-turn side. The cookbook's prose restated the brief's Comment Formatting rules (file-first rationale, inline-content and HEREDOC hazards, the full Windows $OutputEncoding mechanics) around the command that already demonstrates the shape. It now keeps the file-first order, the ready-to-run command, the literal-newline rule, and the pointer; the hazard mechanics live once, in the brief section the pointer names. Default cookbook: 639 -> 494 bytes (-145 per reply turn, uncacheable channel); the Windows variant sheds ~200 more. Pins re-anchor from the deleted prose to the surviving anchors (file-first order, the pointer, the command form); the MUL-2904/#4182 banned-shape negative guards are untouched. Co-authored-by: multica-agent <[email protected]> * fix(daemon): narrow the per-turn coverage claim, test the cross-issue invariant, strip the command shape from the guest-visible prohibition (MUL-5442) Review catches by Elon on #6421 plus the CI failure, all three related: 1. The steps header overstated what the per-turn message carries — it ships the issue id and ready-to-run context-read commands; other calls are assembled from Available Commands. Reworded to say exactly that (a factual claim in a prompt cannot rely on the reader discovering it is wrong). 2. The cross-issue byte-equality this PR claims as a design benefit is now asserted directly per provider (same stable inputs, different 36-char issue UUIDs, byte-identical brief) — a Contains-based negative cannot catch truncated/transformed/conditional id use. 3. CI: the squad guest-leader contract test bans any runnable 'issue status <issue-id> in_review' shape in guest-visible text; the placeholder rewrite made the leader dispatch rule's NEGATIVE sentence match that ban. The prohibition now states itself without a command form ('do NOT move it to in_review or done on this turn') — negative sentences should not carry copy-pasteable command shapes at all. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Bohan-J <[email protected]> Co-authored-by: multica-agent <[email protected]>
2026-11-10 10:26:46 +08:00 · 2026-08-05 14:29:01 +08:00
parent b98f760f1f
commit 2bb13667c6
5 changed files with 79 additions and 44 deletions
@@ -1913,7 +1913,10 @@ func TestInjectRuntimeConfigRequiresExplicitCommentPost(t *testing.T) {
 			// The workflow must contain an explicit `multica issue comment add`
 			// invocation for this issue — not just a prose mention of posting.
 			mustContain := []string{
-				"multica issue comment add issue-1",
+				// MUL-5442 cross-channel dedup: the brief states the loop shape; the
+				// ready-to-run commands with real ids live in the per-turn message.
+				// Pin the command NAME and the flag mnemonics, not full templates.
+				"post it with `multica issue comment add` using",
 				"mandatory",
 			}
 			for _, want := range mustContain {
@@ -5373,8 +5376,14 @@ func TestInjectRuntimeConfigBriefKeepsStaticCatchUpRead(t *testing.T) {
 	}
 	s := string(data)

-	if !strings.Contains(s, "multica issue comment list "+issueID+" --roots-only --summary --output json") {
-		t.Errorf("brief must keep the static catch-up read\n---\n%s", s)
+	// MUL-5442 cross-channel dedup: the full command with the real issue id
+	// moved to the per-turn message (every issue variant carries it); the
+	// brief keeps the doctrine and the flag mnemonics.
+	if !strings.Contains(s, "scan every thread cheaply (`--roots-only --summary`)") {
+		t.Errorf("brief must keep the bounded catch-up doctrine\n---\n%s", s)
+	}
+	if strings.Contains(s, issueID) {
+		t.Errorf("workflow steps must not embed the issue id anymore (MUL-5442)\n---\n%s", s)
 	}
 	if strings.Contains(s, "--recent 20") {
 		t.Errorf("brief still uses recent 20\n---\n%s", s)
@@ -5467,7 +5476,7 @@ func TestInjectRuntimeConfigAssignmentTriggerScansRootsFirst(t *testing.T) {
 	// Mandatory comment catch-up must stay, but the required first read is
 	// bounded to recent active threads instead of the full flat timeline.
 	for _, want := range []string{
-		"multica issue comment list issue-1 --roots-only --summary --output json",
+		"scan every thread cheaply (`--roots-only --summary`)",
 		"this is mandatory, not optional",
 		"Skipping this step is the most common cause",
 	} {
@@ -5531,10 +5540,12 @@ func TestInjectRuntimeConfigCatchUpScansRootsFirst(t *testing.T) {
 	s := string(data)

 	for _, want := range []string{
-		// The cheap scan is the first thing step 3 asks for.
-		"multica issue comment list issue-1 --roots-only --summary --output json",
+		// The cheap scan is the first thing step 3 asks for; the full
+		// command with real ids arrives in the per-turn message (MUL-5442
+		// cross-channel dedup), so the brief pins the flag mnemonics.
+		"scan every thread cheaply (`--roots-only --summary`)",
 		// ...followed by an explicit, bounded drill-down.
-		"multica issue comment list issue-1 --thread <thread-id> --tail 30 --output json",
+		"expand only the threads that matter (`--thread <id> --tail 30`)",
 		// The headline saturation warning stays in the flag reference; the
 		// deep semantics (per-thread cap, root-thread saturation) moved to the
 		// CLI's own --help (MUL-5442) and are pinned there
@@ -5661,7 +5672,7 @@ func TestInjectRuntimeConfigIssueMetadataSectionScope(t *testing.T) {
 			provider: "claude",
 			filename: "CLAUDE.md",
 			workflowStepPresent: []string{
-				"multica issue metadata list issue-md-1 --output json",
+				"Read the metadata bag (`multica issue metadata list`)",
 				// Platform failure semantics, not tool mechanics: a failed
 				// metadata read must never block the main task (MUL-5442
 				// stage-1 review).
@@ -5686,7 +5697,7 @@ func TestInjectRuntimeConfigIssueMetadataSectionScope(t *testing.T) {
 			provider: "claude",
 			filename: "CLAUDE.md",
 			workflowStepPresent: []string{
-				"multica issue metadata list issue-md-2 --output json",
+				"Read the metadata bag (`multica issue metadata list`)",
 				"What to look for: `## Issue Metadata`",
 				"the bar in `## Issue Metadata`",
 				"multica issue metadata set",
@@ -5806,7 +5817,7 @@ func TestInjectRuntimeConfigIssueMetadataCodexFormattingUnchanged(t *testing.T)
 		if !strings.Contains(s, "## Issue Metadata") {
 			t.Fatalf("Issue Metadata section missing\n---\n%s", s)
 		}
-		if !strings.Contains(s, "multica issue metadata list issue-md-codex --output json") {
+		if !strings.Contains(s, "Read the metadata bag (`multica issue metadata list`)") {
 			t.Fatalf("metadata list step missing\n---\n%s", s)
 		}
 		// ...AND the post-#4182 file-first rule is still emitted on Linux.
@@ -187,9 +187,7 @@ func buildCommentReplyInstructionsSlim(provider, issueID, triggerCommentID strin
 		return fmt.Sprintf(
 			"If you decide to reply, post it as a comment — always use the trigger comment ID below, "+
 				"do NOT reuse --parent values from previous turns in this session.\n\n"+
-				"On Windows, write the reply body to a UTF-8 file with your file-write tool first, then post with `--content-file`. "+
-				"Do NOT pipe via `--content-stdin` — PowerShell 5.1's `$OutputEncoding` defaults to ASCIIEncoding when piping to native commands and silently drops non-ASCII (Chinese, Japanese, Cyrillic, accents, emoji) as `?` before bytes reach `multica.exe`. "+
-				"See ## Comment Formatting above for the full rule:\n\n"+
+				"Write the body file first — never pipe via `--content-stdin` (PowerShell drops non-ASCII; full rules: ## Comment Formatting above):\n\n"+
 				"    multica issue comment add %s --parent %s --content-file ./reply.md\n"+
 				"    Remove-Item ./reply.md\n\n"+
 				"Do NOT write literal `\\n` escapes to simulate line breaks; the file preserves real newlines.\n",
@@ -199,8 +197,7 @@ func buildCommentReplyInstructionsSlim(provider, issueID, triggerCommentID strin
 	return fmt.Sprintf(
 		"If you decide to reply, post it as a comment — always use the trigger comment ID below, "+
 			"do NOT reuse --parent values from previous turns in this session.\n\n"+
-			"Write the reply body to a UTF-8 file with your file-write tool first, then post it with `--content-file` "+
-			"(see ## Comment Formatting above for why inline `--content` and `--content-stdin` HEREDOCs are unsafe — MUL-2904 / #4182):\n\n"+
+			"Write the body file first (rules: ## Comment Formatting above — MUL-2904 / #4182):\n\n"+
 			"    multica issue comment add %s --parent %s --content-file ./reply.md\n"+
 			"    rm ./reply.md\n\n"+
 			"Do NOT write literal `\\n` escapes to simulate line breaks; the file preserves real newlines.\n",
@@ -30,8 +30,8 @@ func TestBuildCommentReplyInstructionsCodexLinux(t *testing.T) {

 	for _, want := range []string{
 		"multica issue comment add " + issueID + " --parent " + triggerID + " --content-file ./reply.md",
-		"Write the reply body to a UTF-8 file",
-		"`--content-file`",
+		"Write the body file first",
+		"--content-file ./reply.md",
 		"#4182",
 		"rm ./reply.md",
 		"Do NOT write literal `\\n` escapes to simulate line breaks",
@@ -86,8 +86,11 @@ func TestBuildCommentReplyInstructionsNonCodexLinux(t *testing.T) {

 				for _, want := range []string{
 					"multica issue comment add " + issueID + " --parent " + triggerID + " --content-file ./reply.md",
-					"Write the reply body to a UTF-8 file",
-					"`--content-file`",
+					// MUL-5442 cross-channel dedup: shell-hazard mechanics live in
+					// the brief's Comment Formatting; the cookbook keeps the
+					// file-first order, the command, and the pointer.
+					"Write the body file first",
+					"## Comment Formatting",
 					"#4182",
 					"rm ./reply.md",
 					"do NOT reuse --parent values from previous turns",
@@ -138,10 +141,14 @@ func TestBuildCommentReplyInstructionsWindowsUsesContentFile(t *testing.T) {
 			got := BuildCommentReplyInstructions(provider, issueID, triggerID)
 			for _, want := range []string{
 				"multica issue comment add " + issueID + " --parent " + triggerID + " --content-file",
-				"On Windows, write the reply body to a UTF-8 file",
-				"Do NOT pipe via `--content-stdin`",
-				"silently drops non-ASCII",
-				"$OutputEncoding",
+				// MUL-5442 cross-channel dedup: the $OutputEncoding trap's
+				// full mechanics live once, in the brief's Windows Comment
+				// Formatting variant; the per-turn cookbook keeps the ban,
+				// the one-line consequence, and the pointer.
+				"Write the body file first",
+				"never pipe via `--content-stdin`",
+				"PowerShell drops non-ASCII",
+				"## Comment Formatting",
 			} {
 				if !strings.Contains(got, want) {
 					t.Errorf("%s reply instructions missing %q\n---\n%s", provider, want, got)
@@ -510,32 +510,32 @@ func writeWorkflowAutopilot(b *strings.Builder, ctx TaskContextForEnv) {
 func writeWorkflowIssue(b *strings.Builder, ctx TaskContextForEnv) {
 	b.WriteString("**Turn mode.** The per-turn user message names this run's mode on a line of its own: `Turn mode: Reply.` (respond to the comment that message carries — it brings the triggering comment's id and your `--parent` value) or `Turn mode: Ownership.` (an assignment or status change started this run). Steps 1–6 are shared; then **apply exactly one mode block, the one the user message named** — they differ on issue status. No mode line → Reply mode, 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)
-	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. What to look for: `## Issue Metadata`.\n", ctx.IssueID)
-	fmt.Fprintf(b, "3. Catch up on the comment history — this is mandatory, not optional — in two bounded reads, never one bulk pull: scan every thread cheaply with `multica issue comment list %s --roots-only --summary --output json`, then expand only the threads that matter with `multica issue comment list %s --thread <thread-id> --tail 30 --output json`. Earlier comments often carry context the issue body lacks. Skipping this step is the most common cause of agents acting on stale or incomplete instructions — so always run the scan, even when the trigger looks self-contained. In Reply mode the per-turn user message names the thread to expand first; the scan is how you decide whether any OTHER thread is also relevant.\n", ctx.IssueID, ctx.IssueID)
+	b.WriteString("**Steps 1–6 — both modes** (the per-turn user message carries this issue's real id and ready-to-run context-read commands; assemble other calls from `## Available Commands`)\n\n")
+	b.WriteString("1. Read the issue (`multica issue get`) to understand the context.\n")
+	b.WriteString("2. Read the metadata bag (`multica issue metadata list`) — best-effort, empty `{}` and CLI failures are normal. What to look for: `## Issue Metadata`.\n")
+	b.WriteString("3. Catch up on the comment history — this is mandatory, not optional — in two bounded reads, never one bulk pull: scan every thread cheaply (`--roots-only --summary`), then expand only the threads that matter (`--thread <id> --tail 30`). Earlier comments often carry context the issue body lacks. Skipping this step is the most common cause of agents acting on stale or incomplete instructions — so always run the scan, even when the trigger looks self-contained. In Reply mode the per-turn user message names the thread to expand first; the scan is how you decide whether any OTHER thread is also relevant.\n")
 	b.WriteString("4. Complete the task within your Agent Identity boundaries (`## Instruction Precedence` lists the actions Agent Identity can forbid). If your role is delegation-only, perform the allowed delegation work and stop once that outcome is delivered.\n")
 	if ctx.IsSquadLeader {
-		fmt.Fprintf(b, "5. **Post your final results as a comment** (unless your outcome is `no_action` — in that case, calling `multica squad activity %s no_action --reason \"...\"` alone is sufficient; you MUST exit without posting any comment. DO NOT post a comment announcing no_action or saying you are exiting silently): post it with `multica issue comment add %s` using the platform-correct non-inline mode from ## Comment Formatting (never inline `--content`). Your results are only visible to the user if posted via this CLI call; text in your terminal or run logs is NOT delivered.\n", ctx.IssueID, ctx.IssueID)
+		b.WriteString("5. **Post your final results as a comment** (unless your outcome is `no_action` — in that case, calling `multica squad activity <issue-id> no_action --reason \"...\"` alone is sufficient; you MUST exit without posting any comment. DO NOT post a comment announcing no_action or saying you are exiting silently): post it with `multica issue comment add` using the platform-correct non-inline mode from ## Comment Formatting (never inline `--content`). Your results are only visible to the user if posted via this CLI call; text in your terminal or run logs is NOT delivered.\n")
 	} else {
-		fmt.Fprintf(b, "5. **Post your final results as a comment — this step is mandatory**: post it with `multica issue comment add %s` using the platform-correct non-inline mode from ## Comment Formatting (never inline `--content`). `## Output` states why this call is the only delivery channel. In Reply mode this step is conditional on the reply rule below.\n", ctx.IssueID)
+		b.WriteString("5. **Post your final results as a comment — this step is mandatory**: post it with `multica issue comment add` using the platform-correct non-inline mode from ## Comment Formatting (never inline `--content`). `## Output` states why this call is the only delivery channel. In Reply mode this step is conditional on the reply rule below.\n")
 	}
 	b.WriteString("6. Before exiting, pin or clear a metadata key via `multica issue metadata set`/`delete` only if it clears the bar in `## Issue Metadata`. Most runs write nothing here — that is the expected outcome, not a gap. When in doubt, do not write.\n\n")

 	b.WriteString("**Ownership mode only — you own the issue status this run** (skip any status call below that your Agent Identity forbids)\n\n")
-	fmt.Fprintf(b, "- Before step 4, run `multica issue status %s in_progress`.\n", ctx.IssueID)
+	b.WriteString("- Before step 4, run `multica issue status <issue-id> in_progress`.\n")
 	if ctx.IsSquadLeader {
-		fmt.Fprintf(b, "- After this initial dispatch, leave the parent issue `in_progress` — do NOT run `multica issue status %s in_review` or `done` on this turn. Dispatching members is not completion. You will be re-triggered when members post updates or a stage closes; only then, if the overall goal is met, move the parent to `in_review`.\n", ctx.IssueID)
+		b.WriteString("- After this initial dispatch, leave the parent issue `in_progress` — do NOT move it to `in_review` or `done` on this turn. Dispatching members is not completion. You will be re-triggered when members post updates or a stage closes; only then, if the overall goal is met, move the parent to `in_review`.\n")
 	} else {
-		fmt.Fprintf(b, "- When done, run `multica issue status %s in_review`.\n", ctx.IssueID)
+		b.WriteString("- When done, run `multica issue status <issue-id> in_review`.\n")
 	}
-	fmt.Fprintf(b, "- If blocked, run `multica issue status %s blocked`, and post a comment explaining the blocker unless your Agent Identity forbids issue comments.\n\n", ctx.IssueID)
+	b.WriteString("- If blocked, run `multica issue status <issue-id> blocked`, and post a comment explaining the blocker unless your Agent Identity forbids issue comments.\n\n")

 	b.WriteString("**Reply mode only — respond to the comment in the user message**\n\n")
 	b.WriteString("- Respond to THAT specific comment; take its id from the user message, never from this file or from an earlier turn.\n")
 	b.WriteString("- **Decide whether a reply is warranted.** If you produced actual work this turn, post the result via step 5. If the triggering comment was a pure acknowledgment / thanks / sign-off from another agent AND you produced no work, do NOT reply — not even a 'No reply needed' comment; exit with no output. Silence is a valid and preferred way to end agent-to-agent conversations.\n")
 	if ctx.IsSquadLeader {
-		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)
+		b.WriteString("- **Squad leader rule:** If your evaluation outcome is `no_action`, call `multica squad activity <issue-id> 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")
 	}
 	b.WriteString("- If a reply IS warranted: do any requested work first, then **decide whether to include any `@mention` link.** The default is NO mention; `## Mentions` states when one is warranted.\n")
 	b.WriteString("- **If you reply, posting it as a comment is mandatory** (`## Output`). 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. When that message lists more than one thread to answer, post one reply per thread instead of merging them.\n")
@@ -338,13 +338,13 @@ func TestIssueWorkflowHonorsAgentIdentity(t *testing.T) {
 		// MUL-5442: the forbids-clause is stated once on the Ownership-mode
 		// header instead of once per status bullet.
 		"skip any status call below that your Agent Identity forbids",
-		"Before step 4, run `multica issue status " + issueID + " in_progress`.",
+		"Before step 4, run `multica issue status <issue-id> in_progress`.",
 		"Complete the task within your Agent Identity boundaries",
 		// Step 4 keeps only what the enumeration cannot express: a
 		// delegation-only role stops once the delegation is delivered.
 		"If your role is delegation-only, perform the allowed delegation work and stop once that outcome is delivered",
-		"When done, run `multica issue status " + issueID + " in_review`.",
-		"If blocked, run `multica issue status " + issueID + " blocked`, and post a comment explaining the blocker unless your Agent Identity forbids issue comments.",
+		"When done, run `multica issue status <issue-id> in_review`.",
+		"If blocked, run `multica issue status <issue-id> blocked`, and post a comment explaining the blocker unless your Agent Identity forbids issue comments.",
 	} {
 		if !strings.Contains(out, want) {
 			t.Errorf("issue brief missing identity-bound workflow text %q\n---\n%s", want, out)
@@ -373,9 +373,12 @@ func TestSquadLeaderIssueWorkflowKeepsParentInProgress(t *testing.T) {
 	})

 	for _, want := range []string{
-		"Before step 4, run `multica issue status " + issueID + " in_progress`.",
+		"Before step 4, run `multica issue status <issue-id> in_progress`.",
 		"After this initial dispatch, leave the parent issue `in_progress`",
-		"do NOT run `multica issue status " + issueID + " in_review` or `done` on this turn",
+		// The guest-leader contract test (handler side) bans any runnable
+		// in_review command shape from reaching a guest — the dispatch rule
+		// therefore states the prohibition without a command form.
+		"do NOT move it to `in_review` or `done` on this turn",
 		"only then, if the overall goal is met, move the parent to `in_review`",
 	} {
 		if !strings.Contains(out, want) {
@@ -383,7 +386,7 @@ func TestSquadLeaderIssueWorkflowKeepsParentInProgress(t *testing.T) {
 		}
 	}

-	if strings.Contains(out, "When done, run `multica issue status "+issueID+" in_review`") {
+	if strings.Contains(out, "When done, run `multica issue status <issue-id> in_review`") {
 		t.Errorf("squad-leader issue brief must not contain the ordinary-agent completion step\n---\n%s", out)
 	}
 }
@@ -1632,10 +1635,27 @@ func TestInjectRuntimeConfigByteIdenticalAcrossTriggers(t *testing.T) {

 	// Non-vacuity guard: the brief must still depend on its stable inputs, or
 	// this whole test would pass on a function that ignores ctx entirely.
-	otherIssue := base
-	otherIssue.IssueID = "99999999-8888-7777-6666-555555555555"
-	if buildMetaSkillContent("claude", base) == buildMetaSkillContent("claude", otherIssue) {
-		t.Fatal("brief does not vary with issue id — byte-identity assertions below would be vacuous")
+	// Since MUL-5442's cross-channel dedup the brief is deliberately
+	// issue-id-independent (the per-turn message carries the ids), so the
+	// guard now varies a different stable input: the agent identity.
+	otherAgent := base
+	otherAgent.AgentName = "Someone Else"
+	if buildMetaSkillContent("claude", base) == buildMetaSkillContent("claude", otherAgent) {
+		t.Fatal("brief does not vary with agent identity — byte-identity assertions below would be vacuous")
+	}
+
+	// The stronger MUL-5442 invariant this PR claims as a design benefit:
+	// with identical stable inputs, two DIFFERENT issue ids must render the
+	// byte-identical brief — this is what makes a cross-issue shared cache
+	// prefix possible. Asserted directly, per provider, so a truncated,
+	// transformed, or id-conditional use of the issue id cannot slip past
+	// the Contains-based negative check.
+	for _, provider := range []string{"claude", "codex"} {
+		otherIssue := base
+		otherIssue.IssueID = "99999999-8888-7777-6666-555555555555"
+		if buildMetaSkillContent(provider, base) != buildMetaSkillContent(provider, otherIssue) {
+			t.Fatalf("%s brief differs across issue ids — the cross-issue cache invariant is broken", provider)
+		}
 	}

 	for _, provider := range []string{"claude", "codex"} {