diff --git a/server/internal/daemon/execenv/execenv_test.go b/server/internal/daemon/execenv/execenv_test.go index 0f3896fc2..10d2fd38f 100644 --- a/server/internal/daemon/execenv/execenv_test.go +++ b/server/internal/daemon/execenv/execenv_test.go @@ -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 --tail 30 --output json", + "expand only the threads that matter (`--thread --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. diff --git a/server/internal/daemon/execenv/reply_instructions.go b/server/internal/daemon/execenv/reply_instructions.go index b9a30f630..cf382974c 100644 --- a/server/internal/daemon/execenv/reply_instructions.go +++ b/server/internal/daemon/execenv/reply_instructions.go @@ -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", diff --git a/server/internal/daemon/execenv/reply_instructions_test.go b/server/internal/daemon/execenv/reply_instructions_test.go index 7e8d43089..b585b890f 100644 --- a/server/internal/daemon/execenv/reply_instructions_test.go +++ b/server/internal/daemon/execenv/reply_instructions_test.go @@ -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) diff --git a/server/internal/daemon/execenv/runtime_config_sections.go b/server/internal/daemon/execenv/runtime_config_sections.go index e06f7a053..c3b6a080d 100644 --- a/server/internal/daemon/execenv/runtime_config_sections.go +++ b/server/internal/daemon/execenv/runtime_config_sections.go @@ -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 --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 --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 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 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 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 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 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") diff --git a/server/internal/daemon/execenv/runtime_config_test.go b/server/internal/daemon/execenv/runtime_config_test.go index 68f96201d..9de32cb9d 100644 --- a/server/internal/daemon/execenv/runtime_config_test.go +++ b/server/internal/daemon/execenv/runtime_config_test.go @@ -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 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 in_review`.", + "If blocked, run `multica issue status 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 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 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"} {