fix(chat): codex exec OPTIONS must precede resume subcommand (multi-turn resume)

[jasonkneen]

Problem

Resuming a multi-turn codex chat was broken. Codex's CLI grammar is codex exec [OPTIONS] resume <SESSION_ID> [PROMPT] (confirmed via codex exec --help, codex-cli 0.139.0) — resume is a subcommand of exec, so every exec-level OPTION (--json, --model, sandbox/approval flags, --ignore-user-config, --skip-git-repo-check, -C <dir>) must come before resume; only SESSION_ID and PROMPT follow it.

Both codex argv builders emitted resume <id> immediately after exec, before the flags. On a resume turn codex matched the resume subcommand first and rejected the trailing options against the resume subparser:

error: unexpected argument '-C' found
Usage: codex exec [OPTIONS] resume [SESSION_ID] [PROMPT]

Codex argv (resume case) — before → after

# BEFORE (broken)
exec resume <id> --json --model X -s workspace-write -c approval_policy=on-request --skip-git-repo-check -C <dir> <prompt>
                 ^^^^^^^^^^^^^^^^^ options after `resume` → codex rejects -C

# AFTER (fixed, buildCodexExecArgs / daemon)
exec --json --model X --skip-git-repo-check -C <dir> -s workspace-write -c approval_policy=on-request resume <id> <prompt>

# AFTER (fixed, buildCodexSpawnArgs / runtime)
exec --json --model X -s workspace-write -c approval_policy=on-request --ignore-user-config --skip-git-repo-check -C <dir> resume <id> <prompt>

First-turn (non-resume) argv is functionally unchanged.

Fixes

• src/main/chat/providers/agent-mode-payloads.ts (buildCodexSpawnArgs) — runtime path chatCodex spawns. Options first, then resume <threadId>, then prompt.
• packages/codesurf-daemon/bin/chat-jobs.mjs (buildCodexExecArgs) — daemon path. Same reorder. (This builder introduced the defect in 64980c8.)

Audit (same bug class)

No other harness resume site repeats the subcommand-before-options defect:

Site	Verdict
relay/provider-executor.ts runCodexTurn	clean — no resume path
session-sources.ts:1384, session-index.mjs:923	clean — resumeBin/resumeArgs metadata for the interactive codex resume <id> TUI (not codex exec); top-level, order-independent
hermes (--resume <id>), claude (--resume / SDK options.resume), opencode (--session)	clean — flag-form, order-independent

Claude resume (BUG 2) — investigated, verified correct (no fix)

claude.ts:725 options.resume = existingSessionId is not the same bug class — resume is a named SDK option, immune to positional ordering. Verified empirically with a live 2-turn SDK spike (haiku, claude-opus-4-7, and with the named-agent options chatClaude sets): resume carries context every time, session id stable. The client round-trip is intact (useChatStreamHandler captures the session event → useChatTileMessaging sends sessionId on the next turn), and the desktop "claude" provider dispatches to chatClaude (chat.ts:1025). Field-name, model-resolution, threading, and CLI-ordering candidates were all ruled out. No provider-level Claude fix is warranted.

Tests

• packages/codesurf-daemon/test/codex-exec-args.test.mjs (new) — asserts buildCodexExecArgs puts all options before resume, -C never follows resume, and fresh turns omit resume.
• test/daemon/chat-jobs-agent-mode.test.mjs — corrected the prior test that asserted the buggy resume-after-exec order; added direct coverage for buildCodexSpawnArgs.

Gates

• node --test packages/codesurf-daemon/test/codex-exec-args.test.mjs → 3/3 pass
• cd packages/codesurf-daemon && npm test → 14/14 pass
• npm run test:daemon → 219 pass / 3 fail; the 3 failures (persist detached background mode, persisted chat job timeline replay, composer pills disabled) are pre-existing on origin/main (verified) and unrelated to codex args.
• npm run typecheck:go → edited TS file (agent-mode-payloads.ts) is clean; the 16 remaining tsgo errors are pre-existing baseline in unrelated files.

Summary by CodeRabbit

• Bug Fixes

  • Fixed command-line argument ordering for resuming previous chat conversations to comply with CLI grammar requirements and prevent "unexpected argument" errors during multi-turn interactions.

• Tests

  • Added comprehensive test coverage for chat session resumption argument ordering and consistency validation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dafb7ee0-2fa3-4385-b149-5a3234350ad8

📥 Commits

Reviewing files that changed from the base of the PR and between 00df0c2 and ef90b90.

📒 Files selected for processing (4)

• packages/codesurf-daemon/bin/chat-jobs.mjs
• packages/codesurf-daemon/test/codex-exec-args.test.mjs
• src/main/chat/providers/agent-mode-payloads.ts
• test/daemon/chat-jobs-agent-mode.test.mjs

---

📝 Walkthrough

Walkthrough

Both buildCodexExecArgs (daemon) and buildCodexSpawnArgs (runtime) are updated to emit all exec-level options (--json, --model, -C, sandbox/approval flags) before the resume <threadId> subcommand arguments. New and updated tests enforce this ordering invariant and verify related placement and edge-case constraints.

Changes

Codex argv ordering fix

Layer / File(s)	Summary
Reorder exec options before resume in both argv builders packages/codesurf-daemon/bin/chat-jobs.mjs, src/main/chat/providers/agent-mode-payloads.ts	Both buildCodexExecArgs and buildCodexSpawnArgs are changed so exec-level flags (--json, --model, -C, sandbox/approval flags) are pushed before resume <threadId/sessionId>, with added comments explaining the Codex CLI grammar requirement.
Tests asserting option-before-resume ordering contract packages/codesurf-daemon/test/codex-exec-args.test.mjs, test/daemon/chat-jobs-agent-mode.test.mjs	New codex-exec-args.test.mjs covers daemon argv ordering in three cases; existing daemon and new runtime tests in chat-jobs-agent-mode.test.mjs are updated/added to assert exec-level options precede resume, -C never follows resume, empty sessionId skips resume, and prompt is always last.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• jasonkneen/codesurf#9: Modifies the same buildCodexExecArgs function in packages/codesurf-daemon/bin/chat-jobs.mjs and the same Codex resume argv handling in the daemon test suite, directly touching the code paths this PR corrects.

Poem

🐇 Hop, hop, the flags must lead the way,
Before resume gets its say!
-C and --model march ahead,
So Codex parses with no dread.
Order matters, says the bunny — hooray! 🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately describes the main change: fixing the ordering of command-line options to precede the resume subcommand in Codex exec calls for multi-turn resumption.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch polly/daemon-resume-argfix

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

Fixes broken multi-turn resume for the Codex CLI provider by ensuring codex exec options are emitted before the resume subcommand, matching Codex’s grammar (codex exec [OPTIONS] resume <SESSION_ID> [PROMPT]). This restores continuity on resumed turns while keeping first-turn behavior effectively unchanged.

Changes:

• Reordered Codex argv construction so exec-level flags precede resume <id> in both runtime and daemon builders.
• Updated/added tests to assert resume placement and prevent regressions (especially ensuring -C <dir> never appears after resume).

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File	Description
src/main/chat/providers/agent-mode-payloads.ts	Reorders buildCodexSpawnArgs to place all exec options before resume <threadId> (runtime spawn path).
packages/codesurf-daemon/bin/chat-jobs.mjs	Reorders buildCodexExecArgs to place all exec options before resume <sessionId> (daemon exec path).
test/daemon/chat-jobs-agent-mode.test.mjs	Updates daemon resume test expectations and adds runtime buildCodexSpawnArgs ordering coverage.
packages/codesurf-daemon/test/codex-exec-args.test.mjs	New unit tests specifically guarding daemon buildCodexExecArgs resume ordering invariants.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.