diff --git a/server/internal/daemon/daemon.go b/server/internal/daemon/daemon.go index 22ebb98fe..40d92514e 100644 --- a/server/internal/daemon/daemon.go +++ b/server/internal/daemon/daemon.go @@ -1621,7 +1621,7 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i // the same (agent, issue) pair. The work_dir path is stored in DB on // task completion and passed back via PriorWorkDir on the next claim. - prompt := BuildPrompt(task) + prompt := BuildPrompt(task, provider) // Pass the daemon's auth credentials and context so the spawned agent CLI // can call the Multica API and the local daemon (e.g. `multica repo checkout`). diff --git a/server/internal/daemon/daemon_test.go b/server/internal/daemon/daemon_test.go index 31d78018c..486f0cfe9 100644 --- a/server/internal/daemon/daemon_test.go +++ b/server/internal/daemon/daemon_test.go @@ -101,7 +101,7 @@ func TestBuildPromptContainsIssueID(t *testing.T) { {Name: "Concise", Content: "Be concise."}, }, }, - }) + }, "claude") // Prompt should contain the issue ID and CLI hint. for _, want := range []string{ @@ -127,7 +127,7 @@ func TestBuildPromptNoIssueDetails(t *testing.T) { prompt := BuildPrompt(Task{ IssueID: "test-id", Agent: &AgentData{Name: "Test"}, - }) + }, "claude") // Prompt should not contain issue title/description (agent fetches via CLI). for _, absent := range []string{"**Issue:**", "**Summary:**"} { @@ -146,7 +146,7 @@ func TestBuildPromptAutopilotRunOnly(t *testing.T) { AutopilotTitle: "Daily dependency check", AutopilotDescription: "Check dependencies and report outdated packages.", AutopilotSource: "manual", - }) + }, "claude") for _, want := range []string{ "run-only mode", @@ -178,7 +178,7 @@ func TestBuildPromptCommentTriggered(t *testing.T) { TriggerCommentID: commentID, TriggerCommentContent: commentContent, Agent: &AgentData{Name: "Test"}, - }) + }, "claude") // Prompt should contain the comment content, the trigger comment id, and // the full reply command with --parent. Re-emitting --parent on every turn @@ -223,7 +223,7 @@ func TestBuildPromptCommentTriggeredByAgent(t *testing.T) { TriggerAuthorType: "agent", TriggerAuthorName: "Atlas", Agent: &AgentData{Name: "Test"}, - }) + }, "claude") for _, want := range []string{ "Another agent (Atlas)", @@ -249,7 +249,7 @@ func TestBuildPromptCommentTriggeredByMember(t *testing.T) { TriggerAuthorType: "member", TriggerAuthorName: "Alice", Agent: &AgentData{Name: "Test"}, - }) + }, "claude") if !strings.Contains(prompt, "A user just left a new comment") { t.Fatalf("member-triggered prompt should label the author as a user\n---\n%s", prompt) @@ -278,7 +278,7 @@ func TestBuildPromptCommentTriggeredNoContent(t *testing.T) { IssueID: "test-id", TriggerCommentID: "comment-id", Agent: &AgentData{Name: "Test"}, - }) + }, "claude") if !strings.Contains(prompt, "multica issue get") { t.Fatal("prompt missing CLI hint") diff --git a/server/internal/daemon/execenv/execenv_test.go b/server/internal/daemon/execenv/execenv_test.go index fb9416aaf..41f5cf2df 100644 --- a/server/internal/daemon/execenv/execenv_test.go +++ b/server/internal/daemon/execenv/execenv_test.go @@ -982,59 +982,96 @@ func TestInjectRuntimeConfigRequiresExplicitCommentPost(t *testing.T) { } } -// TestInjectRuntimeConfigDirectsMultiLineWritesToStdin pins the guidance that -// any multi-line content for `multica issue comment add` must go through -// `--content-stdin` + a HEREDOC. Agents that reached for the inline -// `--content "...\n\n..."` form ended up with literal 4-char `\n` sequences -// in stored comments because bash does not expand backslash escapes inside -// double quotes; see MUL-1467. This test prevents the multi-line guidance -// from silently regressing back into a "for special characters" footnote. +// TestInjectRuntimeConfigAvailableCommandsIsNeutral pins that the global +// Available Commands section lists the three input modes neutrally for +// every provider, with no "MUST pipe via stdin" mandate. +// +// Background: #1795 / #1851 introduced "MUST pipe via stdin" / +// `--description-stdin` directives in the global section to fix Codex's +// habit of emitting literal `\n` inside `--content "..."` (MUL-1467). +// That mandate landed in the all-provider section and ended up steering +// every provider at stdin — which then broke non-ASCII bytes on Windows +// shells (#2198 / #2236). This rollback keeps the strong Codex-specific +// mandate in the Codex-Specific section (pinned by +// TestInjectRuntimeConfigCodexEmphasizesStdinForFormattedComments) and +// leaves the global section neutral. Pinning the neutrality here so a +// future refactor can't accidentally re-introduce the over-spread. // -// Pins runtimeGOOS to "linux" so the test is deterministic regardless of -// the host OS the test runner is on; the Windows branch has a separate -// file-first contract pinned by TestInjectRuntimeConfigWindowsRecommendsContentFile. // Not parallel: mutates the package-level runtimeGOOS. -func TestInjectRuntimeConfigDirectsMultiLineWritesToStdin(t *testing.T) { +func TestInjectRuntimeConfigAvailableCommandsIsNeutral(t *testing.T) { saved := runtimeGOOS t.Cleanup(func() { runtimeGOOS = saved }) - runtimeGOOS = "linux" - dir := t.TempDir() - if err := InjectRuntimeConfig(dir, "claude", TaskContextForEnv{IssueID: "issue-1"}); err != nil { - t.Fatalf("InjectRuntimeConfig failed: %v", err) - } - data, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md")) - if err != nil { - t.Fatalf("read CLAUDE.md: %v", err) - } - s := string(data) + for _, host := range []string{"linux", "darwin", "windows"} { + for _, provider := range []string{"claude", "opencode", "openclaw", "hermes", "kimi", "kiro", "cursor", "gemini"} { + t.Run(provider+"/"+host, func(t *testing.T) { + runtimeGOOS = host + dir := t.TempDir() + if err := InjectRuntimeConfig(dir, provider, TaskContextForEnv{IssueID: "issue-1"}); err != nil { + t.Fatalf("InjectRuntimeConfig failed: %v", err) + } - for _, want := range []string{ - "multi-line content", - "MUST pipe via stdin", - "--content-stdin", - "<<'COMMENT'", - "`--description`", - "--description-stdin", - } { - if !strings.Contains(s, want) { - t.Errorf("CLAUDE.md missing multi-line guidance %q\n---\n%s", want, s) + configFile := "CLAUDE.md" + if provider != "claude" { + configFile = "AGENTS.md" + } + if provider == "gemini" { + configFile = "GEMINI.md" + } + data, err := os.ReadFile(filepath.Join(dir, configFile)) + if err != nil { + t.Fatalf("read %s: %v", configFile, err) + } + s := string(data) + + // Available Commands lists all three input modes as fact. + for _, want := range []string{ + "`--content \"...\"`", + "`--content-stdin`", + "`--content-file `", + "`--description-stdin`", + "`--description-file `", + } { + if !strings.Contains(s, want) { + t.Errorf("%s missing flag mention %q\n---\n%s", configFile, want, s) + } + } + + // "MUST pipe via stdin" must NOT appear in any non-Codex + // provider's runtime config: it was the over-spread of + // the Codex-specific fix. + for _, banned := range []string{ + "MUST pipe via stdin", + "Agent-authored comments should always pipe content via stdin", + "use `--description-stdin` and pipe a HEREDOC", + } { + if strings.Contains(s, banned) { + t.Errorf("%s carries over-spread Codex mandate %q for non-Codex provider %s\n---\n%s", configFile, banned, provider, s) + } + } + }) } } } -// TestInjectRuntimeConfigWindowsRecommendsContentFile pins the Windows-specific -// rewrite added after issues #2198 / #2236: agent-authored comments arrive -// as `?` because Windows PowerShell 5.1 and cmd.exe re-encode piped HEREDOC -// bytes through the active console codepage, dropping non-ASCII bytes -// before they reach `multica`. The Windows comment/description block is -// rewritten file-first — there is no preceding "MUST pipe via stdin" / -// `--description-stdin` directive that could override it. -func TestInjectRuntimeConfigWindowsRecommendsContentFile(t *testing.T) { +// TestInjectRuntimeConfigCodexWindowsRecommendsContentFile pins the +// Windows-specific Codex carve-out: on Windows the Codex-Specific section +// directs the agent at `--content-file` instead of `--content-stdin`, +// because Windows PowerShell 5.1 / cmd.exe re-encode piped HEREDOC bytes +// through the active console codepage and silently drop non-ASCII as `?` +// before reaching `multica.exe` (#2198 / #2236). On non-Windows hosts the +// Codex section keeps the canonical stdin/HEREDOC mandate. +// +// The Available Commands section is provider-neutral and lists all three +// input modes regardless of host — its neutrality is pinned separately by +// TestInjectRuntimeConfigAvailableCommandsIsNeutral. +// +// Not parallel: mutates the package-level runtimeGOOS. +func TestInjectRuntimeConfigCodexWindowsRecommendsContentFile(t *testing.T) { saved := runtimeGOOS t.Cleanup(func() { runtimeGOOS = saved }) - t.Run("windows host emits the file-first block", func(t *testing.T) { + t.Run("codex/windows points at --content-file", func(t *testing.T) { runtimeGOOS = "windows" dir := t.TempDir() if err := InjectRuntimeConfig(dir, "codex", TaskContextForEnv{IssueID: "issue-1"}); err != nil { @@ -1046,39 +1083,31 @@ func TestInjectRuntimeConfigWindowsRecommendsContentFile(t *testing.T) { } s := string(data) for _, want := range []string{ - "On this Windows host", + "On Windows, **always write the comment body to a UTF-8 file", "console codepage", "--content-file", - "--description-file", "silently drop non-ASCII characters as `?`", } { if !strings.Contains(s, want) { - t.Errorf("AGENTS.md missing Windows file-first guidance %q\n---\n%s", want, s) + t.Errorf("AGENTS.md missing Codex/Windows file-first guidance %q\n---\n%s", want, s) } } - // The Windows block must NOT carry over any prescriptive - // "use stdin" / "pipe a HEREDOC" directive: GPT-Boy's review noted - // that even a Windows caveat appended after MUST-stdin lines sets - // up a conflicting instruction the agent can latch onto. The - // Windows branch is now file-first; stdin appears only in - // anti-prescriptive prose like "do NOT pipe via `--content-stdin`", - // so the assertions below pin the prescriptive phrasings, not the - // bare flag names. + // On Windows the Codex section must NOT prescribe stdin — that's + // the exact path the Windows console codepage mangles. Pin the + // prescriptive phrasings (sentence-level), not bare flag names, + // so anti-prescriptive prose like "do NOT pipe via + // `--content-stdin`" doesn't trip the ban. for _, banned := range []string{ - "MUST pipe via stdin", - "use `--description-stdin` and pipe a HEREDOC", - "<<'COMMENT'", - "Agent-authored comments should always pipe content via stdin", - "always use `--content-stdin`", + "always use `--content-stdin` with a HEREDOC, even for short single-line replies", } { if strings.Contains(s, banned) { - t.Errorf("AGENTS.md still carries non-Windows stdin directive %q on Windows\n---\n%s", banned, s) + t.Errorf("AGENTS.md still carries Codex stdin mandate %q on Windows\n---\n%s", banned, s) } } }) - t.Run("non-windows host keeps the stdin-first block", func(t *testing.T) { + t.Run("codex/linux keeps the stdin-first Codex section", func(t *testing.T) { runtimeGOOS = "linux" dir := t.TempDir() if err := InjectRuntimeConfig(dir, "codex", TaskContextForEnv{IssueID: "issue-1"}); err != nil { @@ -1089,21 +1118,20 @@ func TestInjectRuntimeConfigWindowsRecommendsContentFile(t *testing.T) { t.Fatalf("read AGENTS.md: %v", err) } s := string(data) - if strings.Contains(s, "On this Windows host") { - t.Errorf("AGENTS.md should not surface Windows guidance when host is %s", runtimeGOOS) - } - // On Linux the stdin/HEREDOC contract is the canonical multi-line - // guidance — pin that it is still present so we can't accidentally - // kill it for non-Windows hosts in a future refactor. + // On Linux the Codex section keeps the canonical stdin/HEREDOC + // mandate — pin it so a future refactor can't accidentally drop + // the protection that originally fixed MUL-1467. for _, want := range []string{ - "MUST pipe via stdin", - "--content-stdin", - "--description-stdin", + "always use `--content-stdin` with a HEREDOC", + "Never use inline `--content` for agent-authored comments", } { if !strings.Contains(s, want) { - t.Errorf("AGENTS.md missing Linux stdin guidance %q\n---\n%s", want, s) + t.Errorf("AGENTS.md missing Codex/Linux stdin mandate %q\n---\n%s", want, s) } } + if strings.Contains(s, "On Windows, **always write the comment body to a UTF-8 file") { + t.Errorf("AGENTS.md should not surface Windows codex guidance on linux host\n---\n%s", s) + } }) } diff --git a/server/internal/daemon/execenv/reply_instructions.go b/server/internal/daemon/execenv/reply_instructions.go index ab18fbdbc..86e84f4a3 100644 --- a/server/internal/daemon/execenv/reply_instructions.go +++ b/server/internal/daemon/execenv/reply_instructions.go @@ -12,42 +12,68 @@ import "fmt" // because resumed Claude sessions keep prior turns' tool calls in context // and will otherwise copy the old --parent UUID forward. // -// On Windows the template switches to `--content-file` because piping a -// HEREDOC through PowerShell 5.1 / cmd.exe re-encodes the bytes via the -// active console codepage, dropping non-ASCII characters as `?` before they -// reach `multica.exe` — see issues #2198 / #2236. Reading the body off disk -// preserves UTF-8 verbatim. -func BuildCommentReplyInstructions(issueID, triggerCommentID string) string { +// The template is provider-aware. The strong "use stdin / use file" mandate +// originated from #1795 / #1851 to fix Codex's habit of emitting literal +// `\n` escapes inside `--content "..."`. Other providers handle inline +// escaping correctly (the CLI's `util.UnescapeBackslashEscapes` decodes +// `\n` server-side anyway), so they get the original lightweight inline +// template that worked on every platform — including Windows non-ASCII, +// where argv goes through CreateProcessW UTF-16 and bytes survive intact. +// +// Codex on Windows must use `--content-file` because piping a HEREDOC +// through PowerShell 5.1 / cmd.exe re-encodes bytes via the active console +// codepage and drops non-ASCII as `?` before reaching `multica.exe` — +// see issues #2198 / #2236. +func BuildCommentReplyInstructions(provider, issueID, triggerCommentID string) string { if triggerCommentID == "" { return "" } - if runtimeGOOS == "windows" { + if provider == "codex" { + if runtimeGOOS == "windows" { + 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, then post it with `--content-file`. "+ + "Do NOT pipe via `--content-stdin` — Windows PowerShell 5.1 and cmd.exe re-encode piped bytes through the active console codepage and silently drop non-ASCII characters as `?`. "+ + "Do NOT use inline `--content`; it is easy to lose formatting or accidentally compress a structured reply into one line.\n\n"+ + "Use this form, preserving the same issue ID and --parent value:\n\n"+ + " # 1. Write the reply body to a UTF-8 file (e.g. reply.md) with your file-write tool.\n"+ + " # 2. Then run:\n"+ + " multica issue comment add %s --parent %s --content-file ./reply.md\n\n"+ + "Do NOT write literal `\\n` escapes to simulate line breaks; the file preserves real newlines.\n", + issueID, triggerCommentID, + ) + } 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, then post it with `--content-file`. "+ - "Do NOT pipe via `--content-stdin` — Windows PowerShell 5.1 and cmd.exe re-encode piped bytes through the active console codepage and silently drop non-ASCII characters as `?`. "+ + "Always use `--content-stdin` with a HEREDOC for agent-authored issue comments, even when the reply is a single line. "+ "Do NOT use inline `--content`; it is easy to lose formatting or accidentally compress a structured reply into one line.\n\n"+ "Use this form, preserving the same issue ID and --parent value:\n\n"+ - " # 1. Write the reply body to a UTF-8 file (e.g. reply.md) with your file-write tool.\n"+ - " # 2. Then run:\n"+ - " multica issue comment add %s --parent %s --content-file ./reply.md\n\n"+ - "Do NOT write literal `\\n` escapes to simulate line breaks; the file preserves real newlines.\n", + " cat <<'COMMENT' | multica issue comment add %s --parent %s --content-stdin\n"+ + " First paragraph.\n"+ + "\n"+ + " Second paragraph.\n"+ + " COMMENT\n\n"+ + "Do NOT write literal `\\n` escapes to simulate line breaks; the HEREDOC preserves real newlines.\n", issueID, triggerCommentID, ) } + // Non-Codex providers: lightweight inline template, no platform branch. + // Pre-#1795 default, restored after we found that #1795 / #1851 had + // expanded a Codex-specific fix into a global mandate that broke + // Windows non-ASCII for every provider. The CLI decodes `\n` etc. + // server-side, so escaped multi-line is fine; for richer formatting + // the agent can still reach for `--content-stdin` (works on Linux / + // macOS) or `--content-file ` (works on every platform), both + // listed in Available Commands above. 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"+ - "Always use `--content-stdin` with a HEREDOC for agent-authored issue comments, even when the reply is a single line. "+ - "Do NOT use inline `--content`; it is easy to lose formatting or accidentally compress a structured reply into one line.\n\n"+ "Use this form, preserving the same issue ID and --parent value:\n\n"+ - " cat <<'COMMENT' | multica issue comment add %s --parent %s --content-stdin\n"+ - " First paragraph.\n"+ - "\n"+ - " Second paragraph.\n"+ - " COMMENT\n\n"+ - "Do NOT write literal `\\n` escapes to simulate line breaks; the HEREDOC preserves real newlines.\n", + " multica issue comment add %s --parent %s --content \"...\"\n\n"+ + "For multi-line bodies, code blocks, or content with quotes/backticks, prefer `--content-stdin` "+ + "(pipe a HEREDOC) or `--content-file ` (read a UTF-8 file). See Available Commands above for the full menu.\n", issueID, triggerCommentID, ) } diff --git a/server/internal/daemon/execenv/reply_instructions_test.go b/server/internal/daemon/execenv/reply_instructions_test.go index d8b608088..67ea7129a 100644 --- a/server/internal/daemon/execenv/reply_instructions_test.go +++ b/server/internal/daemon/execenv/reply_instructions_test.go @@ -7,10 +7,14 @@ import ( "testing" ) -// Pins runtimeGOOS to "linux": the Windows reply template has its own -// dedicated test (TestBuildCommentReplyInstructionsWindowsUsesContentFile). +// TestBuildCommentReplyInstructionsCodexLinux pins that the strong +// "MUST use --content-stdin + HEREDOC" mandate is alive for Codex on +// non-Windows hosts. Codex's habit of emitting literal `\n` inside +// `--content "..."` is the original reason this mandate exists +// (#1795 / #1851); on Linux/macOS stdin is the right answer. +// // Not parallel: mutates the package-level runtimeGOOS. -func TestBuildCommentReplyInstructionsIncludesTriggerID(t *testing.T) { +func TestBuildCommentReplyInstructionsCodexLinux(t *testing.T) { saved := runtimeGOOS t.Cleanup(func() { runtimeGOOS = saved }) runtimeGOOS = "linux" @@ -18,37 +22,93 @@ func TestBuildCommentReplyInstructionsIncludesTriggerID(t *testing.T) { issueID := "11111111-1111-1111-1111-111111111111" triggerID := "22222222-2222-2222-2222-222222222222" - got := BuildCommentReplyInstructions(issueID, triggerID) + got := BuildCommentReplyInstructions("codex", issueID, triggerID) for _, want := range []string{ - "multica issue comment add " + issueID + " --parent " + triggerID, + "multica issue comment add " + issueID + " --parent " + triggerID + " --content-stdin", "Always use `--content-stdin`", "even when the reply is a single line", - "--content-stdin", "<<'COMMENT'", "Do NOT write literal `\\n` escapes to simulate line breaks", "do NOT reuse --parent values from previous turns", } { if !strings.Contains(got, want) { - t.Fatalf("reply instructions missing %q\n---\n%s", want, got) + t.Fatalf("codex/linux reply instructions missing %q\n---\n%s", want, got) } } if strings.Contains(got, "--content \"...\"") { - t.Fatalf("reply instructions should not offer inline --content form\n---\n%s", got) + t.Fatalf("codex reply instructions should not offer inline --content form\n---\n%s", got) + } +} + +// TestBuildCommentReplyInstructionsNonCodexUsesInline pins that every +// non-Codex provider gets the lightweight pre-#1795 inline template, +// regardless of host OS. The "MUST stdin" mandate was originally a +// Codex-specific fix that #1795 / #1851 accidentally spread to every +// provider — and on Windows that spread broke non-ASCII bytes via the +// console codepage (#2198 / #2236). Non-Codex providers handle inline +// escaping correctly and the CLI server-decodes `\n` etc., so the +// inline template works on every platform including Windows non-ASCII +// (argv goes through CreateProcessW UTF-16). +// +// Not parallel: mutates the package-level runtimeGOOS. +func TestBuildCommentReplyInstructionsNonCodexUsesInline(t *testing.T) { + saved := runtimeGOOS + t.Cleanup(func() { runtimeGOOS = saved }) + + issueID := "11111111-1111-1111-1111-111111111111" + triggerID := "22222222-2222-2222-2222-222222222222" + + for _, host := range []string{"linux", "darwin", "windows"} { + for _, provider := range []string{"claude", "opencode", "openclaw", "hermes", "kimi", "kiro", "cursor", "gemini"} { + name := provider + "/" + host + t.Run(name, func(t *testing.T) { + runtimeGOOS = host + got := BuildCommentReplyInstructions(provider, issueID, triggerID) + + for _, want := range []string{ + "multica issue comment add " + issueID + " --parent " + triggerID + " --content \"...\"", + "do NOT reuse --parent values from previous turns", + "If you decide to reply", + } { + if !strings.Contains(got, want) { + t.Errorf("%s reply instructions missing %q\n---\n%s", name, want, got) + } + } + + // Non-Codex providers must NOT receive the Codex-specific + // "MUST stdin" mandate or its HEREDOC template, even on + // Linux/macOS — that was the over-spread of #1795 / #1851. + for _, banned := range []string{ + "Always use `--content-stdin`", + "<<'COMMENT'", + "--parent " + triggerID + " --content-stdin", + "--parent " + triggerID + " --content-file", + } { + if strings.Contains(got, banned) { + t.Errorf("%s reply instructions still steers at codex template: %q\n---\n%s", name, banned, got) + } + } + }) + } } } func TestBuildCommentReplyInstructionsEmptyWhenNoTrigger(t *testing.T) { t.Parallel() - if got := BuildCommentReplyInstructions("issue-id", ""); got != "" { + if got := BuildCommentReplyInstructions("codex", "issue-id", ""); got != "" { + t.Fatalf("expected empty string when triggerCommentID is empty, got %q", got) + } + if got := BuildCommentReplyInstructions("claude", "issue-id", ""); got != "" { t.Fatalf("expected empty string when triggerCommentID is empty, got %q", got) } } // Pins runtimeGOOS to "linux" so the helper output is deterministic. -// Not parallel: mutates the package-level runtimeGOOS. +// Provider is "claude" — exercises the non-codex inline path through +// InjectRuntimeConfig end-to-end. Not parallel: mutates runtimeGOOS. func TestInjectRuntimeConfigCommentTriggerUsesHelper(t *testing.T) { saved := runtimeGOOS t.Cleanup(func() { runtimeGOOS = saved }) @@ -84,73 +144,55 @@ func TestInjectRuntimeConfigCommentTriggerUsesHelper(t *testing.T) { } } -// TestBuildCommentReplyInstructionsWindowsUsesContentFile pins that on Windows -// hosts every per-turn reply prompt — and the workflow block in CLAUDE.md / -// AGENTS.md — points agents at `--content-file` instead of `--content-stdin`. -// Without this, the Windows console codepage re-encodes piped HEREDOC bytes -// and silently drops non-ASCII characters as `?` before they reach -// `multica.exe` (issues #2198, #2236). +// TestBuildCommentReplyInstructionsCodexWindowsUsesContentFile pins that on +// Windows hosts the Codex per-turn reply template points at +// `--content-file` instead of `--content-stdin`. PowerShell 5.1 / cmd.exe +// re-encode piped HEREDOC bytes through the active console codepage and +// silently drop non-ASCII characters as `?` before they reach +// `multica.exe` (issues #2198 / #2236). // -// Not parallel: mutates the package-level `runtimeGOOS`. Restores it via -// t.Cleanup so any subsequent `t.Parallel()` tests see the original value. -func TestBuildCommentReplyInstructionsWindowsUsesContentFile(t *testing.T) { +// Not parallel: mutates the package-level runtimeGOOS. +func TestBuildCommentReplyInstructionsCodexWindowsUsesContentFile(t *testing.T) { saved := runtimeGOOS t.Cleanup(func() { runtimeGOOS = saved }) issueID := "11111111-1111-1111-1111-111111111111" triggerID := "22222222-2222-2222-2222-222222222222" - t.Run("windows host points at --content-file", func(t *testing.T) { + t.Run("codex/windows points at --content-file", func(t *testing.T) { runtimeGOOS = "windows" - got := BuildCommentReplyInstructions(issueID, triggerID) + got := BuildCommentReplyInstructions("codex", issueID, triggerID) for _, want := range []string{ "multica issue comment add " + issueID + " --parent " + triggerID + " --content-file", - "--content-file", "On Windows, write the reply body to a UTF-8 file", "Do NOT pipe via `--content-stdin`", "silently drop non-ASCII characters as `?`", } { if !strings.Contains(got, want) { - t.Errorf("Windows reply instructions missing %q\n---\n%s", want, got) + t.Errorf("codex/windows reply instructions missing %q\n---\n%s", want, got) } } for _, banned := range []string{ "<<'COMMENT'", - "--content-stdin\n", + "--parent " + triggerID + " --content-stdin", "cat <<", } { if strings.Contains(got, banned) { - t.Errorf("Windows reply instructions should not contain %q\n---\n%s", banned, got) + t.Errorf("codex/windows reply instructions should not contain %q\n---\n%s", banned, got) } } }) - - t.Run("non-windows host keeps --content-stdin HEREDOC", func(t *testing.T) { - runtimeGOOS = "linux" - got := BuildCommentReplyInstructions(issueID, triggerID) - for _, want := range []string{ - "multica issue comment add " + issueID + " --parent " + triggerID + " --content-stdin", - "<<'COMMENT'", - } { - if !strings.Contains(got, want) { - t.Errorf("Linux reply instructions missing %q\n---\n%s", want, got) - } - } - if strings.Contains(got, "--content-file") { - t.Errorf("Linux reply instructions should not push --content-file\n---\n%s", got) - } - }) } -// TestInjectRuntimeConfigWindowsCommentTriggerHasNoStdin asserts the -// end-to-end CLAUDE.md / AGENTS.md surface for a comment-triggered task on a -// Windows daemon: the Available Commands section, the Codex-specific -// paragraph, AND the per-turn reply template all line up on `--content-file`, -// with no remaining `--content-stdin` directive that would override the -// Windows fallback. Pins the bug GPT-Boy flagged on PR #2247: the original -// fix only patched Available Commands, leaving the Codex section and per-turn -// prompt still mandating stdin. -func TestInjectRuntimeConfigWindowsCommentTriggerHasNoStdin(t *testing.T) { +// TestInjectRuntimeConfigCodexWindowsCommentTriggerHasNoStdin asserts the +// end-to-end AGENTS.md surface for a Codex comment-triggered task on a +// Windows daemon: the Codex-Specific paragraph + the per-turn reply +// template are file-first, with no remaining `--content-stdin` directive +// that would override the Windows file mandate. The Available Commands +// section is now neutral on every platform (post-rollback of #1795 / +// #1851), so it is allowed to mention `--content-stdin` as one of the +// three input modes. +func TestInjectRuntimeConfigCodexWindowsCommentTriggerHasNoStdin(t *testing.T) { saved := runtimeGOOS t.Cleanup(func() { runtimeGOOS = saved }) runtimeGOOS = "windows" @@ -162,56 +204,36 @@ func TestInjectRuntimeConfigWindowsCommentTriggerHasNoStdin(t *testing.T) { TriggerCommentID: triggerID, } - for _, provider := range []string{"claude", "codex"} { - t.Run(provider, func(t *testing.T) { - dir := t.TempDir() - if err := InjectRuntimeConfig(dir, provider, ctx); err != nil { - t.Fatalf("InjectRuntimeConfig failed: %v", err) - } - fileName := "CLAUDE.md" - if provider != "claude" { - fileName = "AGENTS.md" - } - data, err := os.ReadFile(filepath.Join(dir, fileName)) - if err != nil { - t.Fatalf("read %s: %v", fileName, err) - } - s := string(data) + dir := t.TempDir() + if err := InjectRuntimeConfig(dir, "codex", ctx); err != nil { + t.Fatalf("InjectRuntimeConfig failed: %v", err) + } + data, err := os.ReadFile(filepath.Join(dir, "AGENTS.md")) + if err != nil { + t.Fatalf("read AGENTS.md: %v", err) + } + s := string(data) - for _, want := range []string{ - "multica issue comment add " + issueID + " --parent " + triggerID + " --content-file", - "--content-file", - "--description-file", - "On this Windows host", - } { - if !strings.Contains(s, want) { - t.Errorf("%s missing %q\n---\n%s", fileName, want, s) - } - } + for _, want := range []string{ + "multica issue comment add " + issueID + " --parent " + triggerID + " --content-file", + "--content-file", + "--description-file", + } { + if !strings.Contains(s, want) { + t.Errorf("AGENTS.md missing %q\n---\n%s", want, s) + } + } - // The Available Commands section, the Codex paragraph, and the - // per-turn reply template must NOT end up prescriptively - // directing the agent at stdin — that is the exact pattern - // Windows shells mangle. Covers GPT-Boy's second blocker on - // PR #2247: the original Windows-only fallback was appended - // *after* unconditional "MUST pipe via stdin" / - // `--description-stdin` lines, leaving agents with - // conflicting instructions. We pin the prescriptive - // phrasings (not bare flag names) so anti-prescriptive prose - // like "do NOT pipe via `--content-stdin`" doesn't trip the - // ban. - for _, banned := range []string{ - "--parent " + triggerID + " --content-stdin", - "always use `--content-stdin` with a HEREDOC, even for short single-line replies", - "MUST pipe via stdin", - "use `--description-stdin` and pipe a HEREDOC", - "<<'COMMENT'", - "Agent-authored comments should always pipe content via stdin", - } { - if strings.Contains(s, banned) { - t.Errorf("%s still steers agent at stdin: %q\n---\n%s", fileName, banned, s) - } - } - }) + // The per-turn reply template and the Codex-specific paragraph must + // not direct the agent at stdin on Windows. Pin prescriptive + // substrings rather than bare flag names so anti-prescriptive prose + // like "do NOT pipe via `--content-stdin`" doesn't trip the ban. + for _, banned := range []string{ + "--parent " + triggerID + " --content-stdin", + "always use `--content-stdin` with a HEREDOC, even for short single-line replies", + } { + if strings.Contains(s, banned) { + t.Errorf("AGENTS.md still steers codex at stdin on Windows: %q\n---\n%s", banned, s) + } } } diff --git a/server/internal/daemon/execenv/runtime_config.go b/server/internal/daemon/execenv/runtime_config.go index 0055fa0ef..cd006bc49 100644 --- a/server/internal/daemon/execenv/runtime_config.go +++ b/server/internal/daemon/execenv/runtime_config.go @@ -139,38 +139,22 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string { b.WriteString("- `multica issue label remove ` — Detach a label from an issue\n") b.WriteString("- `multica issue subscriber add [--user |--user-id ]` — Subscribe a member or agent to issue updates (defaults to the caller when neither flag is set; the two flags are mutually exclusive)\n") b.WriteString("- `multica issue subscriber remove [--user |--user-id ]` — Unsubscribe a member or agent\n") - if runtimeGOOS == "windows" { - // Windows shells (PowerShell 5.1, cmd.exe) re-encode piped HEREDOC - // bytes through the active console codepage before they reach - // `multica.exe`, silently replacing characters the codepage cannot - // represent with `?`. Steer the agent at `--content-file` / - // `--description-file` from the start so there is no contradicting - // "MUST pipe via stdin" line for the agent to latch onto. See - // issues #2198 / #2236. - b.WriteString("- `multica issue comment add --content-file [--parent ] [--attachment ]` — Post a comment. **On this Windows host, write the body to a UTF-8 file with your file-write tool first, then pass the path via `--content-file` — do NOT pipe via `--content-stdin`.** PowerShell 5.1 and `cmd.exe` re-encode piped bytes through the active console codepage and silently drop non-ASCII characters as `?`, so `中文` arrives as `??`. Use `--parent` to reply to a specific comment; `--attachment` may be repeated.\n") - b.WriteString(" - **For comment content, write a UTF-8 file then pass `--content-file `; this preserves multi-line content and non-ASCII bytes (Chinese, Japanese, accents, emoji) verbatim.** Do not use inline `--content` and do not write `\\n` escapes. Example:\n") - b.WriteString("\n") - b.WriteString(" ```\n") - b.WriteString(" # 1. Write the comment body to a UTF-8 file using your file-write tool.\n") - b.WriteString(" # 2. Then run:\n") - b.WriteString(" multica issue comment add --content-file ./comment.md\n") - b.WriteString(" ```\n") - b.WriteString("\n") - b.WriteString(" - The same rule applies to `--description` on `multica issue create` and `multica issue update` — write the body to a UTF-8 file with your file-write tool and pass `--description-file `. The inline `--description \"...\"` form is acceptable only for ASCII-only single-line text. Do NOT use `--description-stdin` on this host.\n") - } else { - b.WriteString("- `multica issue comment add --content-stdin [--parent ] [--attachment ]` — Post a comment. Agent-authored comments should always pipe content via stdin, even for short single-line replies. Use `--parent` to reply to a specific comment; `--attachment` may be repeated.\n") - b.WriteString(" - **For comment content, you MUST pipe via stdin; this is mandatory for multi-line content (anything with line breaks, paragraphs, code blocks, backticks, or quotes).** Do not use inline `--content` and do not write `\\n` escapes. Use a HEREDOC instead:\n") - b.WriteString("\n") - b.WriteString(" ```\n") - b.WriteString(" cat <<'COMMENT' | multica issue comment add --content-stdin\n") - b.WriteString(" First paragraph.\n") - b.WriteString("\n") - b.WriteString(" Second paragraph with `code` and \"quotes\".\n") - b.WriteString(" COMMENT\n") - b.WriteString(" ```\n") - b.WriteString("\n") - b.WriteString(" - The same rule applies to `--description` on `multica issue create` and `multica issue update` — use `--description-stdin` and pipe a HEREDOC for any multi-line description; the inline `--description \"...\"` form is for short single-line text only.\n") - } + // Available Commands lists `multica issue comment add` and the + // description flags neutrally — three input modes, pick what fits. + // The previous "MUST pipe via stdin" mandate (#1795 / #1851) was + // originally a Codex-specific fix for codex emitting literal `\n` + // escapes inside `--content "..."`, but it landed in this global + // section and ended up steering every provider at stdin, which then + // burned non-ASCII bytes on Windows shells (issues #2198 / #2236). + // Strong "MUST" wording lives in the Codex-Specific section below + // where it actually belongs; non-Codex providers handle inline + // escaping correctly and can pick whichever flag suits their content. + b.WriteString("- `multica issue comment add [--content \"...\" | --content-stdin | --content-file ] [--parent ] [--attachment ]` — Post a comment. Three input modes, pick whichever fits the content:\n") + b.WriteString(" - `--content \"...\"` for short single-line text. The CLI decodes `\\n`, `\\r`, `\\t`, `\\\\` so escaped multi-line is OK; do not embed raw newlines in the argument.\n") + b.WriteString(" - `--content-stdin` to pipe the body via HEREDOC. Preserves multi-line and special characters verbatim. Cleanest in `bash` / `zsh`.\n") + b.WriteString(" - `--content-file ` to read a UTF-8 file off disk. Preserves bytes verbatim regardless of the shell — use this on Windows when stdin would re-encode non-ASCII (Chinese, Japanese, accents, emoji) through the console codepage and drop them as `?`.\n") + b.WriteString(" - Use `--parent` to reply to a specific comment; `--attachment` may be repeated.\n") + b.WriteString("- `multica issue create` / `multica issue update` accept the same three modes for `--description`: `--description \"...\"`, `--description-stdin`, or `--description-file `.\n") b.WriteString("- `multica issue comment delete ` — Delete a comment\n") b.WriteString("- `multica label create --name \"...\" --color \"#hex\"` — Define a new workspace label (use this only when the label you need does not exist yet; reuse existing labels via `multica label list` first)\n") b.WriteString("- `multica autopilot create --title \"...\" --agent --mode create_issue [--description \"...\"]` — Create an autopilot\n") @@ -285,7 +269,7 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string { b.WriteString("4. **Decide whether a reply is warranted.** If you produced actual work this turn (investigated, fixed, answered a real question), post the result via step 6 — that is a normal reply, not a noise comment. If the triggering comment was a pure acknowledgment / thanks / sign-off from another agent AND you produced no work this turn, do NOT post a reply — and do NOT post a comment saying 'No reply needed' or similar. Simply exit with no output. Silence is a valid and preferred way to end agent-to-agent conversations.\n") b.WriteString("5. If a reply IS warranted: do any requested work first, then **decide whether to include any `@mention` link.** The default is NO mention. Only mention when you are escalating to a human owner who is not yet involved, delegating a concrete new sub-task to another agent for the first time, or the user explicitly asked you to loop someone in. Never @mention the agent you are replying to as a thank-you or sign-off.\n") b.WriteString("6. **If you reply, post it as a comment — this step is mandatory when you reply.** Text in your terminal or run logs is NOT delivered to the user. ") - b.WriteString(BuildCommentReplyInstructions(ctx.IssueID, ctx.TriggerCommentID)) + b.WriteString(BuildCommentReplyInstructions(provider, ctx.IssueID, ctx.TriggerCommentID)) b.WriteString("7. Do NOT change the issue status unless the comment explicitly asks for it\n\n") } else { // Assignment-triggered: defer to agent Skills for workflow specifics. diff --git a/server/internal/daemon/prompt.go b/server/internal/daemon/prompt.go index a199b17a3..574591068 100644 --- a/server/internal/daemon/prompt.go +++ b/server/internal/daemon/prompt.go @@ -9,13 +9,16 @@ import ( // BuildPrompt constructs the task prompt for an agent CLI. // Keep this minimal — detailed instructions live in CLAUDE.md / AGENTS.md -// injected by execenv.InjectRuntimeConfig. -func BuildPrompt(task Task) string { +// injected by execenv.InjectRuntimeConfig. The provider string is used by +// comment-triggered tasks: Codex's per-turn reply template needs the +// platform-aware "stdin or file" variant, every other provider gets a +// lightweight inline template. +func BuildPrompt(task Task, provider string) string { if task.ChatSessionID != "" { return buildChatPrompt(task) } if task.TriggerCommentID != "" { - return buildCommentPrompt(task) + return buildCommentPrompt(task, provider) } if task.AutopilotRunID != "" { return buildAutopilotPrompt(task) @@ -96,7 +99,7 @@ func buildQuickCreatePrompt(task Task) string { // The reply instructions (including the current TriggerCommentID as --parent) // are re-emitted on every turn so resumed sessions cannot carry forward a // previous turn's --parent UUID. -func buildCommentPrompt(task Task) string { +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) @@ -117,7 +120,7 @@ func buildCommentPrompt(task Task) string { } fmt.Fprintf(&b, "Start by running `multica issue get %s --output json` to understand your task, then decide how to proceed.\n\n", task.IssueID) fmt.Fprintf(&b, "If you need comment history, `multica issue comment list %s` returns the latest 50 by default — pass --limit or --since to scope older windows. Long issues can have thousands of comments; do not fetch everything blindly.\n\n", task.IssueID) - b.WriteString(execenv.BuildCommentReplyInstructions(task.IssueID, task.TriggerCommentID)) + b.WriteString(execenv.BuildCommentReplyInstructions(provider, task.IssueID, task.TriggerCommentID)) return b.String() }