diff --git a/cli/src/codex/codexLocal.test.ts b/cli/src/codex/codexLocal.test.ts index 989581fe..d4608d5e 100644 --- a/cli/src/codex/codexLocal.test.ts +++ b/cli/src/codex/codexLocal.test.ts @@ -48,13 +48,43 @@ describe('filterResumeSubcommand', () => { .toEqual(['--model', 'gpt-4']); }); - it('does not filter resume when it appears as flag value', () => { - expect(filterResumeSubcommand(['--name', 'resume'])).toEqual(['--name', 'resume']); + it('does not filter resume when it appears as an option value', () => { + expect(filterResumeSubcommand(['--model', 'resume'])).toEqual(['--model', 'resume']); }); - it('does not filter resume in middle of args', () => { + it('filters resume after global options', () => { expect(filterResumeSubcommand(['--model', 'gpt-4', 'resume', '123'])) - .toEqual(['--model', 'gpt-4', 'resume', '123']); + .toEqual(['--model', 'gpt-4']); + expect(filterResumeSubcommand(['--yolo', 'resume', '--last'])) + .toEqual(['--yolo']); + expect(filterResumeSubcommand(['--config', 'model="resume"', 'resume', '--last'])) + .toEqual(['--config', 'model="resume"']); + }); + + it('preserves an optional prompt after the resume selector', () => { + expect(filterResumeSubcommand(['resume', 'abc-123', 'continue here'])) + .toEqual(['continue here']); + expect(filterResumeSubcommand(['resume', 'abc-123', 'resume'])) + .toEqual(['resume']); + expect(filterResumeSubcommand(['resume', '--last', 'continue here'])) + .toEqual(['continue here']); + }); + + it('does not filter resume after a prompt or option terminator', () => { + expect(filterResumeSubcommand(['start here', 'resume', '--last'])) + .toEqual(['start here', 'resume', '--last']); + expect(filterResumeSubcommand(['--', 'resume', '--last'])) + .toEqual(['--', 'resume', '--last']); + }); + + it('does not treat variadic image values as a resume subcommand', () => { + expect(filterResumeSubcommand(['--image', 'one.png', 'resume', '--last'])) + .toEqual(['--image', 'one.png', 'resume', '--last']); + }); + + it('preserves arguments after an option terminator in resume mode', () => { + expect(filterResumeSubcommand(['resume', 'abc-123', '--', '--last'])) + .toEqual(['--', '--last']); }); }); @@ -138,4 +168,46 @@ describe('codexLocal', () => { expect(spawnOptions.args).toContain('model_reasoning_effort="high"'); expect(spawnOptions.args).not.toContain('--model-reasoning-effort'); }); + + it('passes resume --last through while Codex is resolving the initial session', async () => { + const controller = new AbortController(); + + await codexLocal({ + abort: controller.signal, + sessionId: null, + path: workspacePath, + onSessionFound: vi.fn(), + codexArgs: ['--yolo', 'resume', '--last'] + }); + + const spawnOptions = spawnWithTerminalGuardMock.mock.calls[0][0] as { + args: string[]; + }; + expect(spawnOptions.args.slice(-3)).toEqual(['--yolo', 'resume', '--last']); + }); + + it('replaces resume --last with the resolved session ID during local handoff', async () => { + const controller = new AbortController(); + const sessionId = '11111111-1111-4111-8111-111111111111'; + + await codexLocal({ + abort: controller.signal, + sessionId, + path: workspacePath, + onSessionFound: vi.fn(), + codexArgs: [ + '--ask-for-approval', 'never', + '--sandbox', 'danger-full-access', + 'resume', '--last' + ] + }); + + const spawnOptions = spawnWithTerminalGuardMock.mock.calls[0][0] as { + args: string[]; + }; + expect(spawnOptions.args.filter((arg) => arg === 'resume')).toEqual(['resume']); + expect(spawnOptions.args).toContain(sessionId); + expect(spawnOptions.args).not.toContain('--last'); + expect(spawnOptions.args).toContain('danger-full-access'); + }); }); diff --git a/cli/src/codex/codexLocal.ts b/cli/src/codex/codexLocal.ts index 92bab0f9..ea14ee55 100644 --- a/cli/src/codex/codexLocal.ts +++ b/cli/src/codex/codexLocal.ts @@ -11,23 +11,102 @@ import type { ReasoningEffort } from './appServerTypes'; import { resolveCodexCommand } from './utils/codexExecutable'; import type { McpServersConfig } from './utils/buildHapiMcpBridge'; +const CODEX_OPTIONS_WITH_VALUE = new Set([ + '-a', + '--add-dir', + '--ask-for-approval', + '-C', + '--cd', + '-c', + '--config', + '--disable', + '--enable', + '--local-provider', + '-m', + '--model', + '-p', + '--profile', + '--remote', + '--remote-auth-token-env', + '-s', + '--sandbox' +]); +// -i/--image is intentionally omitted because it accepts a variable number of files. + +function findResumeSubcommandIndex(args: string[]): number { + for (let i = 0; i < args.length; i++) { + const arg = args[i]; + + if (arg === '--') { + return -1; + } + if (arg === 'resume') { + return i; + } + if (!arg.startsWith('-')) { + return -1; + } + if (!arg.includes('=') && CODEX_OPTIONS_WITH_VALUE.has(arg)) { + i += 1; + } + } + + return -1; +} + +function findResumeSessionIdIndex(args: string[], resumeIndex: number): number { + for (let i = resumeIndex + 1; i < args.length; i++) { + const arg = args[i]; + + if (arg === '--') { + return -1; + } + if (!arg.startsWith('-')) { + return i; + } + if (!arg.includes('=') && CODEX_OPTIONS_WITH_VALUE.has(arg)) { + i += 1; + } + } + + return -1; +} + /** - * Filter out 'resume' subcommand which is managed internally by hapi. - * Codex CLI format is `codex resume `, so subcommand is always first. + * Filter out the Codex resume selector which is managed internally by hapi. + * Codex accepts global options before the subcommand, for example + * `codex --sandbox danger-full-access resume --last`. */ export function filterResumeSubcommand(args: string[]): string[] { - if (args.length === 0 || args[0] !== 'resume') { + const resumeIndex = findResumeSubcommandIndex(args); + if (resumeIndex === -1) { return args; } - // First arg is 'resume', filter it and optional session ID - if (args.length > 1 && !args[1].startsWith('-')) { - logger.debug(`[CodexLocal] Filtered 'resume ${args[1]}' - session managed by hapi`); - return args.slice(2); + const optionTerminatorIndex = args.indexOf('--', resumeIndex + 1); + const lastIndex = args.findIndex((arg, index) => ( + arg === '--last' + && index > resumeIndex + && (optionTerminatorIndex === -1 || index < optionTerminatorIndex) + )); + const sessionIdIndex = lastIndex === -1 + ? findResumeSessionIdIndex(args, resumeIndex) + : -1; + const filtered = args.filter((_, index) => ( + index !== resumeIndex + && index !== lastIndex + && index !== sessionIdIndex + )); + + if (lastIndex !== -1) { + logger.debug("[CodexLocal] Filtered 'resume --last' - session managed by hapi"); + } else if (sessionIdIndex !== -1) { + logger.debug(`[CodexLocal] Filtered 'resume ${args[sessionIdIndex]}' - session managed by hapi`); + } else { + logger.debug("[CodexLocal] Filtered 'resume' - session managed by hapi"); } - logger.debug(`[CodexLocal] Filtered 'resume' - session managed by hapi`); - return args.slice(1); + return filtered; } export async function codexLocal(opts: { @@ -77,7 +156,12 @@ export async function codexLocal(opts: { args.push(...buildDeveloperInstructionsArg(codexSystemPrompt)); if (opts.codexArgs) { - const safeArgs = filterResumeSubcommand(opts.codexArgs); + // Before the first launch, Codex still needs the user's selector (for + // example `resume --last`). Once hapi has the concrete session ID, it + // prepends that ID above and removes the original selector here. + const safeArgs = opts.sessionId + ? filterResumeSubcommand(opts.codexArgs) + : opts.codexArgs; args.push(...safeArgs); } diff --git a/cli/src/commands/codex.test.ts b/cli/src/commands/codex.test.ts index 058b2420..ee3df8e2 100644 --- a/cli/src/commands/codex.test.ts +++ b/cli/src/commands/codex.test.ts @@ -85,6 +85,15 @@ describe('codexCommand', () => { }) }) + it('passes native resume selectors through for Codex to resolve', async () => { + await codexCommand.run(createCommandContext(['resume', '--last', 'continue here'])) + + expect(assertCodexLocalSupportedMock).toHaveBeenCalledOnce() + expect(runCodexMock).toHaveBeenCalledWith({ + codexArgs: ['resume', '--last', 'continue here'] + }) + }) + it('skips the local version check for runner-started sessions', async () => { await codexCommand.run(createCommandContext(['--started-by', 'runner'])) diff --git a/cli/src/commands/codex.ts b/cli/src/commands/codex.ts index 85e89f6d..5eba254d 100644 --- a/cli/src/commands/codex.ts +++ b/cli/src/commands/codex.ts @@ -44,7 +44,8 @@ export const codexCommand: CommandDefinition = { if (i === 0 && arg === 'resume') { const candidate = commandArgs[i + 1] if (!candidate || candidate.startsWith('-')) { - throw new Error('resume requires a session id') + unknownArgs.push(arg) + continue } options.resumeSessionId = candidate i += 1