diff --git a/apps/server/src/agents/manager.ts b/apps/server/src/agents/manager.ts index 03f77c33..e65b147c 100644 --- a/apps/server/src/agents/manager.ts +++ b/apps/server/src/agents/manager.ts @@ -933,6 +933,17 @@ export class AgentManager { agentId: id, payload: { kind: "agent-command", command: agentCommand }, }); + // The tmux runtime writes agentCommand to /tmp/dispatch_setup_.sh + // and runs `bash ` (see prepareLaunch) rather than embedding it + // inline, so a restart doesn't hit tmux's ~16KB argv limit. Fresh + // launches clean up their equivalent script in completeSetup once the + // setup script's own callback fires; a restart has no such callback, + // so clean up here instead — by the time launch() resolves, the + // fast-fail check has already confirmed the pane didn't die, meaning + // bash is already past reading the script into the long-running CLI + // process. Left otherwise, every restart of a persona/review agent + // would leave its full identity/task prompt sitting in /tmp. + await unlink(`/tmp/dispatch_setup_${id}.sh`).catch(() => {}); await this.setAgentStatus(id, "running", null, tmuxSession); // Re-populate gitContext on every restart so existing agents that diff --git a/apps/server/src/agents/tmux/command-builder.ts b/apps/server/src/agents/tmux/command-builder.ts index 3a389360..a5e59fa0 100644 --- a/apps/server/src/agents/tmux/command-builder.ts +++ b/apps/server/src/agents/tmux/command-builder.ts @@ -25,6 +25,16 @@ const CLI_BY_AGENT_TYPE: Record< const DISPATCH_API_URL_ENV = "DISPATCH_API_URL"; const DISPATCH_RELEASE_UPDATE_TOKEN_ENV = "DISPATCH_RELEASE_UPDATE_TOKEN"; +/** + * Appended after a persona/review agent's identity/task when Codex resumes a + * stopped session, so it doesn't blindly redo work it already finished. + */ +const CODEX_RESUME_NOTE = + "Session resumed. Continue based on your role and task above using your " + + "existing progress and conversation history — do not restart or redo " + + "work you already completed (for example, do not resubmit a review that " + + "was already submitted)."; + /** * Pull a `--append-system-prompt ` pair out of an arg list (codex / * opencode put system prompts in their own flag). Claude doesn't need this @@ -486,11 +496,33 @@ export function buildAgentCommand( ].join(" "); const codexEnvPrefix = `${envPrefix} ${codexDispatchAuthEnv}=${shellEscape(dispatchMcpToken)}`; const modelFlag = model ? `--model ${shellEscape(model)}` : ""; - // Codex resume: `codex resume [OPTIONS] `. Options go before the - // session id so it always binds to SESSION_ID and never slides into the - // trailing [PROMPT] positional. Passthrough args are re-applied here too — - // they carry `--dangerously-bypass-approvals-and-sandbox` for full-access - // agents, which a resumed session would otherwise silently lose. + // Codex resume: `codex resume [OPTIONS] [PROMPT]`. Options go + // before the session id so it always binds to SESSION_ID and never slides + // into the trailing [PROMPT] positional. Passthrough args are re-applied + // here too — they carry `--dangerously-bypass-approvals-and-sandbox` for + // full-access agents, which a resumed session would otherwise silently + // lose. + // + // The trailing [PROMPT] on resume is deliberately NOT the same startupPrompt + // used on fresh launch below. A resumed session already has launchGuidance + // and personalityPrompt in its history from when it first launched — + // resending them would silently submit a new turn on every ordinary + // restart, burning an unattended turn re-processing guidance it already + // has (and for an autoReview agent, plausibly re-triggering "commit, push, + // open a PR, launch a reviewer" on an agent that already finished that + // flow). So a plain sub-agent gets nothing on resume, exactly as before + // this fix. + // + // Only a `role === "review"` agent (a persona/review sub-agent — the case + // this fix targets) gets a prompt at all, and it's just its own identity/ + // task (appendedSystemPrompt) plus a short resume note telling it not to + // redo already-finished work — otherwise it comes back with a live session + // but nothing telling it what to do, and just sits at its input line + // forever. The gate is on role, not on "has appendedSystemPrompt": job + // agents also carry their prompt via --append-system-prompt in agentArgs + // (role "standard"), and re-sending a job's prompt on resume would re-run + // it against a stale job run id with none of the job's MCP tools threaded + // through — resume must stay a no-op for those, same as before this fix. if (resume && cliSessionId) { const resumeFlags = [ codexMcpFlags, @@ -499,7 +531,10 @@ export function buildAgentCommand( ] .filter(Boolean) .join(" "); - return `${codexEnvPrefix} ${shellEscape(cliBin)} resume ${resumeFlags} ${shellEscape(cliSessionId)}`; + const resumeCommand = `${codexEnvPrefix} ${shellEscape(cliBin)} resume ${resumeFlags} ${shellEscape(cliSessionId)}`; + if (role !== "review" || !appendedSystemPrompt) return resumeCommand; + const resumePrompt = [appendedSystemPrompt, CODEX_RESUME_NOTE].join("\n\n"); + return `${resumeCommand} ${shellEscape(resumePrompt)}`; } const codexPromptParts = [ launchGuidance, diff --git a/apps/server/src/agents/tmux/runtime.ts b/apps/server/src/agents/tmux/runtime.ts index 2f634cf6..8735e3f5 100644 --- a/apps/server/src/agents/tmux/runtime.ts +++ b/apps/server/src/agents/tmux/runtime.ts @@ -250,16 +250,25 @@ export function createTmuxRuntime(logger: FastifyBaseLogger): AgentRuntime { * per-session exit file — so the reconciler can read both via * `readSetupLogTail` and `readExitInfo` regardless of how the session * was launched. + * + * Both kinds are also written to disk and run via `bash ` rather + * than embedded inline in the outer `bash -c '...'` string. `tmux + * new-session` hands its arguments to the kernel as a single argv entry + * per arg, and imsg-backed platforms (macOS, some Linux configs) reject + * anything past ~16KB with "command too long" — an `agent-command` + * carrying a large startup prompt (e.g. a persona/review agent's full + * identity + diff, re-sent on resume) can exceed that. Writing to disk + * removes the size limit entirely, matching how `setup-script` already + * avoided it. */ async function prepareLaunch(input: LaunchInput): Promise { - let inner: string; - if (input.payload.kind === "setup-script") { - const scriptPath = setupScriptPath(input.agentId); - await writeFile(scriptPath, input.payload.scriptContent, { mode: 0o755 }); - inner = `bash ${scriptPath}`; - } else { - inner = input.payload.command; - } + const scriptContent = + input.payload.kind === "setup-script" + ? input.payload.scriptContent + : input.payload.command; + const scriptPath = setupScriptPath(input.agentId); + await writeFile(scriptPath, scriptContent, { mode: 0o755 }); + const inner = `bash ${scriptPath}`; const logFile = setupLogPath(input.agentId); const exitFile = exitFilePath(input.sessionName); diff --git a/apps/server/test/agent-runtime-tmux.test.ts b/apps/server/test/agent-runtime-tmux.test.ts index a2cce939..726b07cb 100644 --- a/apps/server/test/agent-runtime-tmux.test.ts +++ b/apps/server/test/agent-runtime-tmux.test.ts @@ -448,7 +448,13 @@ describe("TmuxRuntime — launch (setup-script payload)", () => { }); describe("TmuxRuntime — launch (agent-command payload)", () => { - it("wraps the inline command with stderr-tee and exit-code capture (paths runtime-internal)", async () => { + afterEach(async () => { + await unlink("/tmp/dispatch_setup_agt_y.sh").catch(() => {}); + await unlink("/tmp/dispatch_setup_agt_z.sh").catch(() => {}); + await unlink("/tmp/dispatch_setup_agt_big.sh").catch(() => {}); + }); + + it("writes the command to disk and runs `bash ` — same as setup-script, not embedded inline", async () => { vi.mocked(runCommand).mockImplementation(async (_cmd, args) => { if (args[0] === "has-session") return ok(); return ok(); @@ -462,6 +468,10 @@ describe("TmuxRuntime — launch (agent-command payload)", () => { payload: { kind: "agent-command", command: "/opt/claude --foo" }, }); + const expectedScriptPath = "/tmp/dispatch_setup_agt_y.sh"; + const written = await readFile(expectedScriptPath, "utf-8"); + expect(written).toBe("/opt/claude --foo"); + const newSessionCall = vi .mocked(runCommand) .mock.calls.find(([, a]) => a.includes("new-session")); @@ -470,20 +480,22 @@ describe("TmuxRuntime — launch (agent-command payload)", () => { ] as string; // The wrapper bakes in: stderr tee to the agent's setup log file - // (path derived from agentId), the agent command itself, and - // EXIT:$? to the per-session exit file (path derived from - // sessionName). The manager doesn't supply or know either path. + // (path derived from agentId), `bash ` (not the raw + // command), and EXIT:$? to the per-session exit file (path derived + // from sessionName). The manager doesn't supply or know either path. expect(wrappedCommand).toContain(`tee "/tmp/dispatch_setup_agt_y.log"`); - expect(wrappedCommand).toContain("/opt/claude --foo"); + expect(wrappedCommand).toContain(`bash ${expectedScriptPath}`); + expect(wrappedCommand).not.toContain("/opt/claude --foo"); expect(wrappedCommand).toContain( `echo "EXIT:$?" > /tmp/dispatch_dispatch_agt_y.exit` ); }); - it("escapes embedded single quotes in the command (security regression check)", async () => { - // The wrapper interpolates the command into a single-quoted bash - // string. An attacker-controlled value with `'` in it must not - // break out of the wrapping. + it("preserves embedded single quotes in the written script verbatim (no shell-interpolation mangling)", async () => { + // Since the command now goes to disk via writeFile rather than being + // interpolated into a single-quoted bash string, a `'` in the command + // (e.g. inside a persona prompt) must survive untouched, not get the + // classic '\'' escape treatment. vi.mocked(runCommand).mockImplementation(async (_cmd, args) => { if (args[0] === "has-session") return ok(); return ok(); @@ -500,13 +512,43 @@ describe("TmuxRuntime — launch (agent-command payload)", () => { }, }); + const written = await readFile("/tmp/dispatch_setup_agt_z.sh", "utf-8"); + expect(written).toBe(`echo 'inner-quote'`); + }); + + it("regression: a resume command carrying a large (>16KB) persona/review prompt still launches — tmux argv has a ~16KB limit that inline commands used to hit", async () => { + // Before this file-indirection fix, `agent-command` embedded the whole + // wrapped command as a single tmux argv entry, which tmux/imsg rejects + // past ~16KB — exactly the size a persona/review agent's re-sent + // identity+task prompt can reach on resume. Routing through disk (same + // as setup-script) removes the size limit entirely; this test proves a + // command well past that threshold reaches `runCommand` as a short, + // fixed-size argv regardless of payload size. + vi.mocked(runCommand).mockImplementation(async (_cmd, args) => { + if (args[0] === "has-session") return ok(); + return ok(); + }); + + const largeCommand = `codex resume --model gpt "${"x".repeat(20_000)}"`; + const runtime = createTmuxRuntime(noopLogger); + await runtime.launch({ + sessionName: "dispatch_agt_big", + cwd: "/tmp", + agentId: "agt_big", + payload: { kind: "agent-command", command: largeCommand }, + }); + + const written = await readFile("/tmp/dispatch_setup_agt_big.sh", "utf-8"); + expect(written).toBe(largeCommand); + const newSessionCall = vi .mocked(runCommand) .mock.calls.find(([, a]) => a.includes("new-session")); const wrappedCommand = newSessionCall?.[1]?.[ newSessionCall[1].length - 1 ] as string; - // The classic '\'' escape must appear in place of the embedded `'`. - expect(wrappedCommand).toContain(`echo '\\''inner-quote'\\''`); + // The argv entry tmux actually receives stays small — well under the + // ~16KB limit — no matter how large the underlying command is. + expect(wrappedCommand.length).toBeLessThan(1000); }); }); diff --git a/apps/server/test/db/agent-manager.test.ts b/apps/server/test/db/agent-manager.test.ts index 25cb34bd..28af4faa 100644 --- a/apps/server/test/db/agent-manager.test.ts +++ b/apps/server/test/db/agent-manager.test.ts @@ -1483,20 +1483,31 @@ describe("AgentManager", () => { }); const newSessionArgs: string[][] = []; + // The restart command is written to disk and run via `bash ` + // (same file-backed launch as a fresh session), not embedded inline + // in the tmux argv — and startAgent unlinks that script right after a + // successful launch (see manager.ts), so capture its content here, + // inside the mock, while it's still guaranteed to exist on disk. + let launchCommand = ""; vi.mocked(runCommand).mockImplementation(async (_cmd, args) => { if (args[0] === "has-session") { if (newSessionArgs.length === 0) return { exitCode: 1, stdout: "", stderr: "" }; return { exitCode: 0, stdout: "", stderr: "" }; } - if (args.includes("new-session")) newSessionArgs.push(args); + if (args.includes("new-session")) { + newSessionArgs.push(args); + launchCommand = await readFile( + `/tmp/dispatch_setup_${agent.id}.sh`, + "utf-8" + ); + } return { exitCode: 0, stdout: "", stderr: "" }; }); await manager.startAgent(agent.id); expect(newSessionArgs.length).toBe(1); - const launchCommand = newSessionArgs[0]!.join(" "); expect(launchCommand).toContain("--resume"); expect(launchCommand).toContain(sessionId); }); @@ -1513,20 +1524,26 @@ describe("AgentManager", () => { ); const newSessionArgs: string[][] = []; + let launchCommand = ""; vi.mocked(runCommand).mockImplementation(async (_cmd, args) => { if (args[0] === "has-session") { if (newSessionArgs.length === 0) return { exitCode: 1, stdout: "", stderr: "" }; return { exitCode: 0, stdout: "", stderr: "" }; } - if (args.includes("new-session")) newSessionArgs.push(args); + if (args.includes("new-session")) { + newSessionArgs.push(args); + launchCommand = await readFile( + `/tmp/dispatch_setup_${agent.id}.sh`, + "utf-8" + ); + } return { exitCode: 0, stdout: "", stderr: "" }; }); await manager.startAgent(agent.id); expect(newSessionArgs.length).toBe(1); - const launchCommand = newSessionArgs[0]!.join(" "); expect(launchCommand).not.toContain("--resume"); }); }); diff --git a/apps/server/test/tmux-command-builder.test.ts b/apps/server/test/tmux-command-builder.test.ts index bed5c2d2..8a61408f 100644 --- a/apps/server/test/tmux-command-builder.test.ts +++ b/apps/server/test/tmux-command-builder.test.ts @@ -320,7 +320,7 @@ describe("buildAgentCommand", () => { expect(cmd).toContain("DISPATCH_AUTH_TOKEN="); }); - it("for codex resume, emits 'codex resume ' with the session id last", () => { + it("for codex resume with no persona/review identity, emits 'codex resume ' with nothing trailing", () => { const cmd = buildAgentCommand( baseConfig, "codex", @@ -333,9 +333,12 @@ describe("buildAgentCommand", () => { ); expect(cmd).toContain("'/opt/codex' resume "); expect(cmd).toContain("mcp_servers.dispatch.url="); - // Session id must be the trailing positional so codex binds it to - // SESSION_ID rather than the optional PROMPT argument. + // A plain sub-agent (no --append-system-prompt in agentArgs) gets no + // trailing prompt on resume, exactly as before this fix — re-sending + // launchGuidance/personality on every ordinary restart would silently + // submit a new turn. Session id stays the trailing positional. expect(cmd.endsWith("'codex-session'")).toBe(true); + expect(cmd).not.toContain("Dispatch startup rules"); }); it("for codex resume, re-applies passthrough args so full access survives a restart", () => { @@ -368,23 +371,57 @@ describe("buildAgentCommand", () => { expect(cmd.indexOf("--model")).toBeLessThan(cmd.indexOf("'codex-session'")); }); - it("for codex resume, does not re-send the startup prompt", () => { + it("for codex resume, re-sends only the persona/review identity plus a resume note — not launch guidance, personality, or initialPrompt", () => { const cmd = buildAgentCommand( baseConfig, "codex", - "standard", - [], + "review", + ["--append-system-prompt", "You are the security-review persona."], "/tmp/media", SESSION, false, { cliSessionId: "codex-session", resume: true, + // initialPrompt is only ever supplied at original creation in + // practice (manager.ts doesn't pass it to startAgent's rebuild), + // but exercise it here to prove resume deliberately excludes it. initialPrompt: "do the thing", + personalityPrompt: "Be extra terse.", } ); - expect(cmd).not.toContain("do the thing"); + expect(cmd).toContain("You are the security-review persona."); + expect(cmd).toContain("Session resumed."); + expect(cmd).toContain("do not resubmit a review"); expect(cmd).not.toContain("Dispatch startup rules"); + expect(cmd).not.toContain("Be extra terse."); + expect(cmd).not.toContain("do the thing"); + // Prompt must trail the session id, per `codex resume [OPTIONS] [PROMPT]`. + expect(cmd.indexOf("'codex-session'")).toBeLessThan( + cmd.indexOf("Session resumed.") + ); + }); + + it("for codex resume, does NOT re-send the prompt for a standard-role agent even when agentArgs carries --append-system-prompt (job agents)", () => { + // Job agents are role "standard" but, like persona/review agents, store + // their entire prompt via --append-system-prompt in agentArgs (see + // jobs/service.ts's buildJobPrompt). The resume gate must key off role, + // not "does appendedSystemPrompt exist" — otherwise a resumed job agent + // re-runs its job against a stale run id with no job MCP tools threaded + // through the resumed session. + const cmd = buildAgentCommand( + baseConfig, + "codex", + "standard", + ["--append-system-prompt", "You are running as a Dispatch Job agent."], + "/tmp/media", + SESSION, + false, + { cliSessionId: "codex-session", resume: true } + ); + expect(cmd).not.toContain("You are running as a Dispatch Job agent."); + expect(cmd).not.toContain("Session resumed."); + expect(cmd.endsWith("'codex-session'")).toBe(true); }); it("for opencode with fullAccess=true, sets OPENCODE_PERMISSION env", () => { diff --git a/apps/web/src/components/app/docs-sections/personalities.tsx b/apps/web/src/components/app/docs-sections/personalities.tsx index 19328de5..8c1148f4 100644 --- a/apps/web/src/components/app/docs-sections/personalities.tsx +++ b/apps/web/src/components/app/docs-sections/personalities.tsx @@ -65,10 +65,11 @@ export function PersonalitiesContent() { launches or resumes: it goes into Claude's{" "} --append-system-prompt flag and into the launch prompt for Codex, Cursor, and OpenCode. The one exception is Codex on resume - — codex resume continues the existing session without a - new prompt, so a resumed Codex agent keeps whatever personality it - launched with. Terminal agents have no CLI to inject into, so the - personality is silently skipped. + — resuming re-sends only a persona/review agent's own identity and + task (never a plain personality or launch guidance), so an ordinary + restart doesn't silently submit a new turn. Cursor and OpenCode still + resend the personality on resume. Terminal agents have no CLI to + inject into, so the personality is silently skipped.

Three flows intentionally don't get the personality, since