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

packages/codesurf-daemon/bin/chat-jobs.mjs

@@ -700,20 +700,24 @@ export function buildCodexExecArgs(request, workspaceDir, instructionPrompt = ''
   // Multi-turn continuity: when the request carries the Codex thread id from a
   // prior turn (emitted as a `thread.started` session event and echoed back by
   // the client as request.sessionId), resume that thread so the model keeps the
-  // full conversation — `codex exec resume <threadId> ...`. This mirrors the
-  // runtime builder (src/main/chat/providers/agent-mode-payloads.ts
-  // buildCodexSpawnArgs), which places `resume <id>` immediately after `exec`.
+  // full conversation — `codex exec [OPTIONS] resume <threadId> [PROMPT]`.
+  // Codex's CLI grammar requires every exec-level OPTION (--json, --model,
+  // --skip-git-repo-check, -C <dir>, sandbox/approval flags) to precede the
+  // `resume` subcommand; only SESSION_ID and PROMPT follow it. Placing `resume`
+  // before the options makes codex reject the trailing flags (e.g.
+  // `error: unexpected argument '-C' found`). Mirrors the runtime builder
+  // (src/main/chat/providers/agent-mode-payloads.ts buildCodexSpawnArgs).
   // First turn (no sessionId) starts a fresh thread (unchanged behavior).
   const resumeArgs = request.sessionId ? ['resume', request.sessionId] : []
   const codexArgs = [
     'exec',
-    ...resumeArgs,
     '--json',
     '--model',
     request.model,
     '--skip-git-repo-check',
     ...(workspaceDir ? ['-C', workspaceDir] : []),
     ...sandboxApprovalFlags,
+    ...resumeArgs,
   ]
   codexArgs.push(buildCodexPrompt(
     lastUserMsg?.content ?? '',

packages/codesurf-daemon/test/codex-exec-args.test.mjs

@@ -0,0 +1,73 @@
+import assert from 'node:assert/strict'
+import test from 'node:test'
+
+import { buildCodexExecArgs } from '../bin/chat-jobs.mjs'
+
+// Codex's CLI grammar is `codex exec [OPTIONS] resume <SESSION_ID> [PROMPT]`.
+// Every exec-level OPTION (--json, --model, sandbox/approval flags,
+// --skip-git-repo-check, -C <dir>) must precede the `resume` subcommand; only
+// SESSION_ID and PROMPT follow it. The original builder pushed `resume <id>`
+// immediately after `exec`, so codex rejected the trailing `-C` with
+// `error: unexpected argument '-C' found`, breaking multi-turn resume.
+
+test('buildCodexExecArgs places exec-level options before resume', () => {
+  const args = buildCodexExecArgs({
+    provider: 'codex',
+    model: 'o3',
+    mode: 'default',
+    sessionId: 'session-1',
+    messages: [{ role: 'user', content: 'continue' }],
+  }, '/tmp/workspace')
+
+  assert.deepEqual(args.slice(0, 12), [
+    'exec',
+    '--json',
+    '--model',
+    'o3',
+    '--skip-git-repo-check',
+    '-C',
+    '/tmp/workspace',
+    '-s',
+    'workspace-write',
+    '-c',
+    'approval_policy=on-request',
+    'resume',
+  ])
+  assert.equal(args[12], 'session-1')
+  assert.match(args.at(-1), /continue/)
+})
+
+test('buildCodexExecArgs never emits -C (or any flag) after resume', () => {
+  const args = buildCodexExecArgs({
+    provider: 'codex',
+    model: 'o3',
+    mode: 'default',
+    sessionId: 'session-1',
+    messages: [{ role: 'user', content: 'continue' }],
+  }, '/tmp/workspace')
+
+  const resumeIdx = args.indexOf('resume')
+  assert.ok(resumeIdx > -1, 'resume subcommand must be present on a continuation turn')
+  // `-C` and every other exec-level flag must live BEFORE `resume`.
+  assert.ok(args.lastIndexOf('-C') < resumeIdx, '`-C` must never follow `resume`')
+  for (const flag of ['--json', '--model', '--skip-git-repo-check', '-s', '-c']) {
+    assert.ok(args.indexOf(flag) > -1 && args.indexOf(flag) < resumeIdx, `${flag} must precede resume`)
+  }
+  // Only the session id and prompt follow `resume`.
+  assert.equal(resumeIdx, args.length - 3, '`resume` must be third-from-last: resume, <id>, <prompt>')
+  assert.equal(args[resumeIdx + 1], 'session-1', 'the session id immediately follows `resume`')
+})
+
+test('buildCodexExecArgs keeps fresh turns on codex exec without resume', () => {
+  const args = buildCodexExecArgs({
+    provider: 'codex',
+    model: 'o3',
+    mode: 'default',
+    messages: [{ role: 'user', content: 'start' }],
+  }, '/tmp/workspace')
+
+  assert.equal(args[0], 'exec')
+  assert.equal(args[1], '--json', 'fresh turns keep the flag order immediately after exec')
+  assert.equal(args.includes('resume'), false)
+  assert.equal(args.at(-1).includes('start'), true)
+})

src/main/chat/providers/agent-mode-payloads.ts

@@ -130,13 +130,20 @@ export function buildCodexSpawnArgs(input: CodexSpawnInput): string[] {
     agentPersona,
   )
 
+  // Codex CLI grammar is `codex exec [OPTIONS] resume <SESSION_ID> [PROMPT]`:
+  // every exec-level OPTION (--json, --model, sandbox/approval flags,
+  // --ignore-user-config, --skip-git-repo-check, -C <dir>) MUST precede the
+  // `resume` subcommand — only SESSION_ID and PROMPT follow it. Emitting
+  // `resume` before the options makes codex reject the trailing flags with
+  // `error: unexpected argument '-C' found`, breaking multi-turn resume. So push
+  // all options first, THEN (when resuming) `resume <threadId>`, THEN the prompt.
   const args: string[] = ['exec']
-  if (input.resumeThreadId) args.push('resume', input.resumeThreadId)
   args.push('--json', '--model', input.model)
   args.push(...sandboxApprovalFlags)
   args.push('--ignore-user-config')
   if (input.workspaceDir) args.push('--skip-git-repo-check', '-C', input.workspaceDir)
   else args.push('--skip-git-repo-check')
+  if (input.resumeThreadId) args.push('resume', input.resumeThreadId)
   args.push(promptText)
   return args
 }

test/daemon/chat-jobs-agent-mode.test.mjs

@@ -500,24 +500,37 @@ test('daemon Codex: AgentMode.systemPrompt is injected into the prompt arg', ()
 
 // ─── daemon Codex: multi-turn continuity (resume) ────────────────────────────
 
-test('daemon Codex: a request with sessionId resumes the thread (`exec resume <id>`), first turn does NOT', () => {
+test('daemon Codex: a request with sessionId resumes the thread (`exec [OPTIONS] resume <id>`), first turn does NOT', () => {
   const base = { provider: 'codex', model: 'gpt-test', mode: 'default', messages: [{ role: 'user', content: 'do it' }] }
 
   // Continuation turn: the Codex thread id from a prior turn is echoed back as
-  // request.sessionId. The argv must resume that thread so the model keeps the
-  // full conversation — `codex exec resume <id> ...`, with `resume <id>`
-  // immediately after `exec` (mirrors the runtime buildCodexSpawnArgs shape).
+  // request.sessionId. Codex's CLI grammar is `codex exec [OPTIONS] resume
+  // <SESSION_ID> [PROMPT]` — ALL exec-level options (--json, --model,
+  // --skip-git-repo-check, -C <dir>, sandbox/approval flags) MUST precede the
+  // `resume` subcommand; only the SESSION_ID and PROMPT follow it. Emitting
+  // `resume` before the options makes codex reject the trailing flags
+  // (`error: unexpected argument '-C' found`), which broke multi-turn resume.
   const resumed = buildCodexExecArgs({ ...base, sessionId: 'thread-1' }, '/ws')
   const execIdx = resumed.indexOf('exec')
-  assert.equal(resumed[execIdx + 1], 'resume', '`resume` must immediately follow `exec`')
-  assert.equal(resumed[execIdx + 2], 'thread-1', 'the thread id must follow `resume`')
-  // (a plain includes('resume thread-1') would be false — they are two argv elements.)
-  assert.ok(resumed.includes('resume') && resumed.includes('thread-1'))
+  const resumeIdx = resumed.indexOf('resume')
+  // `resume` must NOT sit immediately after `exec` — it comes AFTER the options.
+  assert.notEqual(resumed[execIdx + 1], 'resume', '`resume` must NOT immediately follow `exec` (options come first)')
+  assert.equal(resumed[execIdx + 1], '--json', 'exec-level options lead the argv')
+  assert.equal(resumed[resumeIdx + 1], 'thread-1', 'the thread id must immediately follow `resume`')
+  // The thread id and the prompt are the only args after `resume`.
+  assert.equal(resumeIdx, resumed.length - 3, '`resume` must be third-from-last (resume, <id>, <prompt>)')
+  assert.match(resumed.at(-1), /do it/, 'the prompt is the final positional arg')
+  // Every exec-level option/flag must appear BEFORE `resume`.
+  for (const flag of ['--json', '--model', 'gpt-test', '--skip-git-repo-check', '-C', '/ws', '-s', 'workspace-write', '-c', 'approval_policy=on-request']) {
+    assert.ok(resumed.indexOf(flag) > -1 && resumed.indexOf(flag) < resumeIdx, `${flag} must precede resume`)
+  }
+  // `-C <dir>` must never appear after `resume` (the exact bug codex rejected).
+  assert.ok(resumed.lastIndexOf('-C') < resumeIdx, '`-C` must never follow `resume`')
 
   // First turn (no sessionId): a fresh thread, NO resume subcommand.
   const fresh = buildCodexExecArgs({ ...base }, '/ws')
   assert.ok(!fresh.includes('resume'), 'first turn (no sessionId) must NOT resume')
-  assert.equal(fresh[fresh.indexOf('exec') + 1], '--json', 'first turn keeps the unchanged flag order after exec')
+  assert.equal(fresh[fresh.indexOf('exec') + 1], '--json', 'first turn keeps the flag order after exec')
 
   // Empty-string sessionId is falsy → treated as a first turn (no resume).
   assert.ok(!buildCodexExecArgs({ ...base, sessionId: '' }, '/ws').includes('resume'))
@@ -527,6 +540,38 @@ test('daemon Codex: a request with sessionId resumes the thread (`exec resume <i
   assert.deepEqual(codexSandboxAndApproval(resumed), { sandbox: 'workspace-write', approval: 'on-request' })
 })
 
+// ─── runtime Codex: multi-turn continuity (resume) — buildCodexSpawnArgs ──────
+
+test('runtime Codex: buildCodexSpawnArgs places exec-level options before `resume <id>`, first turn does NOT resume', () => {
+  const base = { agentMode: null, mode: 'default', model: 'gpt-test', userContent: 'do it', workspaceDir: '/ws' }
+
+  // Same Codex CLI grammar invariant as the daemon builder: `codex exec
+  // [OPTIONS] resume <SESSION_ID> [PROMPT]`. buildCodexSpawnArgs is the function
+  // chatCodex actually spawns (the runtime path); BUG 1 was that it pushed
+  // `resume <id>` before the option flags, so codex rejected `-C` on resume.
+  const resumed = buildCodexSpawnArgs({ ...base, resumeThreadId: 'thread-9' })
+  const execIdx = resumed.indexOf('exec')
+  const resumeIdx = resumed.indexOf('resume')
+  assert.equal(resumed[execIdx + 1], '--json', 'exec-level options must lead the argv (not `resume`)')
+  assert.notEqual(resumed[execIdx + 1], 'resume', '`resume` must NOT immediately follow `exec`')
+  assert.equal(resumed[resumeIdx + 1], 'thread-9', 'the thread id must immediately follow `resume`')
+  assert.equal(resumeIdx, resumed.length - 3, '`resume` must be third-from-last (resume, <id>, <prompt>)')
+  // Prompt is the final positional; `resume <id>` are the two args before it.
+  assert.equal(resumed[resumed.length - 2], 'thread-9', '<id> precedes the prompt')
+  assert.match(resumed.at(-1), /do it/, 'the prompt is the final positional arg')
+  // Every exec-level option/flag must precede `resume`, and `-C` must not follow it.
+  for (const flag of ['--json', '--model', 'gpt-test', '--ignore-user-config', '--skip-git-repo-check', '-C', '/ws', '-s', 'workspace-write', '-c', 'approval_policy=on-request']) {
+    assert.ok(resumed.indexOf(flag) > -1 && resumed.indexOf(flag) < resumeIdx, `${flag} must precede resume`)
+  }
+  assert.ok(resumed.lastIndexOf('-C') < resumeIdx, '`-C` must never follow `resume`')
+
+  // First turn (no resumeThreadId): no resume subcommand, prompt stays last.
+  const fresh = buildCodexSpawnArgs({ ...base })
+  assert.ok(!fresh.includes('resume'), 'first turn must NOT resume')
+  assert.equal(fresh[fresh.indexOf('exec') + 1], '--json', 'first turn keeps the flag order after exec')
+  assert.match(fresh.at(-1), /do it/, 'prompt is last on a fresh turn too')
+})
+
 // ─── daemon Codex SDK provider: opt-in mapping + resume ──────────────────────
 
 test('daemon Codex SDK: each UI mode maps to SDK sandboxMode + approvalPolicy', () => {