diff --git a/docs/evidence/issue-642/README.md b/docs/evidence/issue-642/README.md new file mode 100644 index 000000000..3d252a393 --- /dev/null +++ b/docs/evidence/issue-642/README.md @@ -0,0 +1,15 @@ +# Codex launch evidence for issue #642 + +The PNGs render captured stdout from the same launch-argument regression, against origin/main (before) and the fix (after). They are test-output captures, not screenshots of the application or proof of mobile connectivity. + +The regression checks that an Auto Mode resume retains its session ID, approval policy, sandbox and writable hive directory without selecting remote transport. The old helper attaches `--remote`; the new launch planner selects `--no-daemon`. + +To repeat from the repository root: + +```sh +git show origin/main:src/shared/codexRemote.ts > /tmp/md-codex-642-before.ts +CODEX_TEST_SOURCE=/tmp/md-codex-642-before.ts node --test --test-reporter=spec docs/evidence/issue-642/check-launch.cjs +node --test --test-reporter=spec docs/evidence/issue-642/check-launch.cjs +``` + +The first run should exit 1, and the second should exit 0. Paths with spaces are deliberately included. No Codex model turn is run. diff --git a/docs/evidence/issue-642/after.png b/docs/evidence/issue-642/after.png new file mode 100644 index 000000000..3aa7531f1 Binary files /dev/null and b/docs/evidence/issue-642/after.png differ diff --git a/docs/evidence/issue-642/before.png b/docs/evidence/issue-642/before.png new file mode 100644 index 000000000..7afd5fc35 Binary files /dev/null and b/docs/evidence/issue-642/before.png differ diff --git a/docs/evidence/issue-642/check-launch.cjs b/docs/evidence/issue-642/check-launch.cjs new file mode 100644 index 000000000..ff606522c --- /dev/null +++ b/docs/evidence/issue-642/check-launch.cjs @@ -0,0 +1,21 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const loadTs = require(process.cwd() + '/test/load-ts.cjs'); +const codex = loadTs(process.env.CODEX_TEST_SOURCE || 'src/shared/codexRemote.ts'); +test('Auto Mode resume retains permissions without remote transport', () => { + const args = ['resume', 'session-id', '-a', 'never', '-s', 'workspace-write', '--add-dir', '/tmp/hive with spaces']; + const endpoint = 'unix:///tmp/agent.sock'; + let launch; + if (codex.planCodexLaunch) { + const plan = codex.planCodexLaunch(args); + launch = plan.managedRemote ? codex.withCodexRemoteArgs(plan.args, endpoint) : plan.args; + } else { + launch = codex.withCodexRemoteArgs(args, endpoint); + } + console.log('Codex argv:', JSON.stringify(launch)); + assert.equal(launch.includes('--remote'), false, 'Remote resume must not receive permission overrides'); + assert.ok(launch.includes('session-id')); + assert.ok(launch.includes('never')); + assert.ok(launch.includes('workspace-write')); + assert.ok(launch.includes('/tmp/hive with spaces')); +}); diff --git a/src/main/index.ts b/src/main/index.ts index 0cc017c8d..9d09937f3 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -90,6 +90,7 @@ import { codexRemoteAliasPath, codexRemoteEndpoint, codexRemoteSocketFits, + planCodexLaunch, withCodexRemoteArgs } from '../shared/codexRemote'; @@ -160,6 +161,12 @@ async function enableCodexRemoteForSpawn( opts: SpawnOptions & { hive?: AgentMeta }, agentId: string ): Promise { + const launch = planCodexLaunch(opts.args ?? []); + if (!launch.managedRemote) { + opts.args = launch.args; + if (launch.localReason) console.log('[codex-remote] starting local TUI:', launch.localReason); + return false; + } if (process.platform === 'win32') return false; const realHome = opts.env?.CODEX_HOME; if (!realHome) return false; @@ -216,7 +223,7 @@ async function enableCodexRemoteForSpawn( return false; } opts.env = { ...(opts.env ?? {}), CODEX_HOME: alias }; - opts.args = withCodexRemoteArgs(opts.args ?? [], codexRemoteEndpoint(alias)); + opts.args = withCodexRemoteArgs(launch.args, codexRemoteEndpoint(alias)); return true; } catch (e) { console.warn('[codex-remote] setup failed; starting local TUI:', diff --git a/src/shared/codexRemote.ts b/src/shared/codexRemote.ts index a58daed48..1a0e6bd14 100644 --- a/src/shared/codexRemote.ts +++ b/src/shared/codexRemote.ts @@ -42,6 +42,75 @@ export function codexRemoteEndpoint(shortHome: string): string { return `unix://${join(shortHome, CODEX_REMOTE_SOCKET_RELATIVE)}`; } +/** Choose the managed remote transport only when Codex supports the launch. + * Remote resume cannot override permissions, and remote launches cannot accept + * --add-dir. Keep those options intact in a local TUI rather than weakening the + * requested permissions or losing the hive's writable directories. */ +export function planCodexLaunch(args: string[]): { + args: string[]; + managedRemote: boolean; + localReason?: string; +} { + // Respect an explicitly selected transport; never redirect an external server + // to an agent's local home or start a managed daemon for --no-daemon. + if (args.some(a => a === '--remote' || a.startsWith('--remote=') || a === '--no-daemon')) { + return { args, managedRemote: false }; + } + + const valueOptions = new Set([ + '--model', '-m', '--config', '-c', '--profile', '-p', '--cd', '-C', + '--image', '-i', '--enable', '--disable', '--local-provider', + '--ask-for-approval', '-a', '--sandbox', '-s', '--add-dir', + '--permission-profile', '-P' + ]); + const permissionOptions = new Set([ + '--ask-for-approval', '-a', '--sandbox', '-s', '--profile', '-p', + '--permission-profile', '-P', '--full-auto', '--approve-for-me', + '--dangerously-bypass-approvals-and-sandbox', '--yolo' + ]); + let resume = false; + let positionalSeen = false; + let permissions = false; + let extraDirs = false; + let bypass = false; + const directoryArgIndices = new Set(); + for (let i = 0; i < args.length; i++) { + const arg = args[i]; + if (arg === '--') break; // remaining tokens are literal positional text + const option = arg.split('=', 1)[0]; + if (!arg.startsWith('-')) { + if (!positionalSeen) resume = arg === 'resume' || arg === 'fork'; + positionalSeen = true; + continue; + } + if (permissionOptions.has(option) || /^-[asPp].+/.test(arg)) permissions = true; + if (option === '--dangerously-bypass-approvals-and-sandbox' || option === '--yolo') bypass = true; + if (option === '--add-dir') { + extraDirs = true; + directoryArgIndices.add(i); + if (arg === option && i + 1 < args.length) directoryArgIndices.add(i + 1); + } + if (option === '--config' || option === '-c' || arg.startsWith('-c')) { + const config = arg === option && (option === '-c' || option === '--config') + ? args[i + 1] ?? '' : arg.slice(option === '--config' ? '--config='.length : 2).replace(/^=/, ''); + if (/^(approval_policy|sandbox_mode|sandbox_workspace_write|permissions|default_permissions)([.=]|$)/.test(config)) { + permissions = true; + } + } + if (valueOptions.has(option) && arg === option) i++; + } + + // Full bypass already permits access to the entire filesystem. These roots + // are redundant only in that explicit mode; retain them for sandboxed runs. + const launchArgs = bypass ? args.filter((_, i) => !directoryArgIndices.has(i)) : args; + const localReason = resume && (permissions || extraDirs) + ? 'remote resume does not support permission overrides' + : extraDirs && !bypass ? 'remote launches do not support --add-dir' : undefined; + return localReason + ? { args: ['--no-daemon', ...args], managedRemote: false, localReason } + : { args: launchArgs, managedRemote: true }; +} + /** Global options must precede `resume`, so prepend the endpoint in all cases. */ export function withCodexRemoteArgs(args: string[], endpoint: string): string[] { if (args.includes('--remote')) return args; diff --git a/test/codex-remote.test.cjs b/test/codex-remote.test.cjs index c6b333bf7..51a675403 100644 --- a/test/codex-remote.test.cjs +++ b/test/codex-remote.test.cjs @@ -9,6 +9,7 @@ const { codexRemoteEndpoint, codexRemoteSocketFits, withCodexRemoteArgs, + planCodexLaunch, CODEX_REMOTE_SOCKET_MAX, CODEX_REMOTE_SOCKET_RELATIVE } = loadTs('src/shared/codexRemote.ts'); @@ -25,6 +26,75 @@ test('Codex remote uses a short stable per-agent home alias', { assert.match(codexRemoteEndpoint(first), /^unix:\/\/\/tmp\//); }); +test('Auto Mode resume keeps its permissions, hive roots and prompt in a local TUI', () => { + const args = ['resume', 'session-id', '-a', 'never', '-s', 'workspace-write', + '--dangerously-bypass-hook-trust', '--add-dir', '/hive/agent', 'Drain the inbox']; + const original = [...args]; + const plan = planCodexLaunch(args); + assert.equal(plan.managedRemote, false); + assert.match(plan.localReason, /resume/); + assert.deepEqual(plan.args, ['--no-daemon', ...original]); + assert.deepEqual(args, original); +}); + +test('fresh sandboxed launches retain --add-dir and use local execution', () => { + for (const args of [ + ['-a', 'never', '-s', 'workspace-write', '--add-dir', '/hive', 'hello'], + ['--add-dir=/hive', 'hello'] + ]) { + const plan = planCodexLaunch(args); + assert.equal(plan.managedRemote, false); + assert.deepEqual(plan.args, ['--no-daemon', ...args]); + assert.match(plan.localReason, /--add-dir/); + } +}); + +test('fresh full-bypass launches can use remote without redundant directory grants', () => { + for (const bypass of ['--dangerously-bypass-approvals-and-sandbox', '--yolo']) { + const plan = planCodexLaunch([bypass, '--add-dir', '/hive', '--add-dir=/shared', + '--dangerously-bypass-hook-trust', 'hello']); + assert.equal(plan.managedRemote, true); + assert.deepEqual(plan.args, [bypass, '--dangerously-bypass-hook-trust', 'hello']); + } +}); + +test('compatible fresh launches and resumes retain remote support', () => { + for (const args of [ + ['-a', 'never', '-s', 'workspace-write', 'hello'], + ['resume', 'session-id', '--model', 'gpt-5.6-sol', 'hello'], + ['--model', 'resume', 'hello'], // an option value is not a subcommand + ['resume', 'session-id', '--', '--add-dir'] // literal prompt text + ]) { + assert.deepEqual(planCodexLaunch(args), { args, managedRemote: true }); + } +}); + +test('permission overrides on resume select local execution across CLI spellings', () => { + for (const override of [ + ['--ask-for-approval=never'], ['--sandbox=workspace-write'], ['-anever'], + ['-sworkspace-write'], ['--approve-for-me'], ['--yolo'], + ['--profile', 'custom'], ['--permission-profile', 'custom'], + ['-c', 'approval_policy="never"'], ['--config=sandbox_mode="workspace-write"'], + ['-csandbox_workspace_write.writable_roots=["/hive"]'], + ['-c=default_permissions="custom"'] + ]) { + const args = [...override, 'resume', 'session-id']; + assert.deepEqual(planCodexLaunch(args).args, ['--no-daemon', ...args]); + assert.equal(planCodexLaunch(args).managedRemote, false); + } + assert.equal(planCodexLaunch(['fork', 'session-id', '-a', 'never']).managedRemote, false); +}); + +test('explicit transports are preserved without starting the managed remote daemon', () => { + for (const args of [ + ['--no-daemon', 'resume', 'session-id', '-a', 'never'], + ['--remote', 'wss://custom.example', 'resume', 'session-id'], + ['--remote=unix:///custom.sock', 'hello'] + ]) { + assert.deepEqual(planCodexLaunch(args), { args, managedRemote: false }); + } +}); + test('the default alias root yields a socket within sun_path', () => { // The real hive home that failed with "path must be shorter than SUN_LEN". const realHome =