From 322c50e5a9785ec4a55f50fac543fee92071944e Mon Sep 17 00:00:00 2001 From: weishu Date: Tue, 23 Dec 2025 18:42:25 +0800 Subject: [PATCH] refactor(codex): extract config building and support CLI overrides in remote mode This refactoring introduces CLI argument overrides for sandbox and approval policy settings in remote mode, allowing users to specify security constraints via `--sandbox` and `--ask-for-approval` flags. Changes: - Add `CodexCliOverrides` type and `parseCodexCliOverrides()` utility to parse CLI flags like `--sandbox`, `-s`, `--ask-for-approval`, `-a`, along with convenience flags (`--full-auto`, `--dangerously-bypass-approvals-and-sandbox`) - Extract complex start config building logic into `buildCodexStartConfig()` function with proper approval policy and sandbox resolution based on permission mode - Thread `codexCliOverrides` through the session/loop/launcher chain and apply overrides only when permission mode is 'default' - Update `codexRemoteLauncher` to use the new config builder and display appropriate warnings based on whether overrides are present - Add comprehensive tests for both parsing and config building functions --- cli/src/codex/codexRemoteLauncher.ts | 45 ++++------ cli/src/codex/loop.ts | 5 +- cli/src/codex/runCodex.ts | 4 + cli/src/codex/session.ts | 4 + cli/src/codex/utils/codexCliOverrides.test.ts | 48 +++++++++++ cli/src/codex/utils/codexCliOverrides.ts | 82 +++++++++++++++++++ cli/src/codex/utils/codexStartConfig.test.ts | 44 ++++++++++ cli/src/codex/utils/codexStartConfig.ts | 59 +++++++++++++ 8 files changed, 261 insertions(+), 30 deletions(-) create mode 100644 cli/src/codex/utils/codexCliOverrides.test.ts create mode 100644 cli/src/codex/utils/codexCliOverrides.ts create mode 100644 cli/src/codex/utils/codexStartConfig.test.ts create mode 100644 cli/src/codex/utils/codexStartConfig.ts diff --git a/cli/src/codex/codexRemoteLauncher.ts b/cli/src/codex/codexRemoteLauncher.ts index 46d53ac7..e0d57063 100644 --- a/cli/src/codex/codexRemoteLauncher.ts +++ b/cli/src/codex/codexRemoteLauncher.ts @@ -12,7 +12,6 @@ import { DiffProcessor } from './utils/diffProcessor'; import { logger } from '@/ui/logger'; import { MessageBuffer } from '@/ui/ink/messageBuffer'; import { CodexDisplay } from '@/ui/ink/CodexDisplay'; -import { trimIdent } from '@/utils/trimIdent'; import type { CodexSessionConfig } from './types'; import { getHappyCliCommand } from '@/utils/spawnHappyCLI'; import { startHappyServer } from '@/claude/utils/startHappyServer'; @@ -20,12 +19,19 @@ import { emitReadyIfIdle } from './utils/emitReadyIfIdle'; import type { CodexSession } from './session'; import type { EnhancedMode } from './loop'; import { restoreTerminalState } from '@/ui/terminalState'; +import { hasCodexCliOverrides } from './utils/codexCliOverrides'; +import { buildCodexStartConfig } from './utils/codexStartConfig'; export async function codexRemoteLauncher(session: CodexSession): Promise<'switch' | 'exit'> { // Warn if CLI args were passed that won't apply in remote mode if (session.codexArgs && session.codexArgs.length > 0) { - logger.debug(`[codex-remote] Warning: CLI args [${session.codexArgs.join(', ')}] are ignored in remote mode. ` + - `Remote mode uses message-based configuration (model/sandbox set via web interface).`); + if (hasCodexCliOverrides(session.codexCliOverrides)) { + logger.debug(`[codex-remote] CLI args include sandbox/approval overrides; other args ` + + `are ignored in remote mode.`); + } else { + logger.debug(`[codex-remote] Warning: CLI args [${session.codexArgs.join(', ')}] are ignored in remote mode. ` + + `Remote mode uses message-based configuration (model/sandbox set via web interface).`); + } } const hasTTY = process.stdout.isTTY && process.stdin.isTTY; @@ -373,33 +379,14 @@ export async function codexRemoteLauncher(session: CodexSession): Promise<'switc currentModeHash = message.hash; try { - const approvalPolicy = (() => { - switch (message.mode.permissionMode) { - case 'default': return 'untrusted' as const; - case 'read-only': return 'never' as const; - case 'safe-yolo': return 'on-failure' as const; - case 'yolo': return 'on-failure' as const; - } - })(); - const sandbox = (() => { - switch (message.mode.permissionMode) { - case 'default': return 'workspace-write' as const; - case 'read-only': return 'read-only' as const; - case 'safe-yolo': return 'workspace-write' as const; - case 'yolo': return 'danger-full-access' as const; - } - })(); - if (!wasCreated) { - const startConfig: CodexSessionConfig = { - prompt: first ? message.message + '\n\n' + trimIdent(`Based on this message, call functions.hapi__change_title to change chat session title that would represent the current task. If chat idea would change dramatically - call this function again to update the title.`) : message.message, - sandbox, - 'approval-policy': approvalPolicy, - config: { mcp_servers: mcpServers } - }; - if (message.mode.model) { - startConfig.model = message.mode.model; - } + const startConfig: CodexSessionConfig = buildCodexStartConfig({ + message: message.message, + mode: message.mode, + first, + mcpServers, + cliOverrides: session.codexCliOverrides + }); let resumeFile: string | null = null; if (nextExperimentalResume) { diff --git a/cli/src/codex/loop.ts b/cli/src/codex/loop.ts index 878423ff..39121361 100644 --- a/cli/src/codex/loop.ts +++ b/cli/src/codex/loop.ts @@ -5,6 +5,7 @@ import { CodexSession } from './session'; import { codexLocalLauncher } from './codexLocalLauncher'; import { codexRemoteLauncher } from './codexRemoteLauncher'; import { ApiClient, ApiSessionClient } from '@/lib'; +import type { CodexCliOverrides } from './utils/codexCliOverrides'; export type PermissionMode = 'default' | 'read-only' | 'safe-yolo' | 'yolo'; @@ -21,6 +22,7 @@ interface LoopOptions { session: ApiSessionClient; api: ApiClient; codexArgs?: string[]; + codexCliOverrides?: CodexCliOverrides; onSessionReady?: (session: CodexSession) => void; } @@ -35,7 +37,8 @@ export async function loop(opts: LoopOptions): Promise { messageQueue: opts.messageQueue, onModeChange: opts.onModeChange, mode: opts.startingMode ?? 'local', - codexArgs: opts.codexArgs + codexArgs: opts.codexArgs, + codexCliOverrides: opts.codexCliOverrides }); if (opts.onSessionReady) { diff --git a/cli/src/codex/runCodex.ts b/cli/src/codex/runCodex.ts index aef6344c..406c2a3c 100644 --- a/cli/src/codex/runCodex.ts +++ b/cli/src/codex/runCodex.ts @@ -17,6 +17,7 @@ import type { AgentState, Metadata } from '@/api/types'; import packageJson from '../../package.json'; import { runtimePath } from '@/projectPath'; import type { CodexSession } from './session'; +import { parseCodexCliOverrides } from './utils/codexCliOverrides'; export { emitReadyIfIdle } from './utils/emitReadyIfIdle'; @@ -93,6 +94,8 @@ export async function runCodex(opts: { model: mode.model })); + const codexCliOverrides = parseCodexCliOverrides(opts.codexArgs); + let currentPermissionMode: PermissionMode | undefined = undefined; let currentModel: string | undefined = undefined; @@ -190,6 +193,7 @@ export async function runCodex(opts: { api, session, codexArgs: opts.codexArgs, + codexCliOverrides, onModeChange: (newMode) => { session.sendSessionEvent({ type: 'switch', mode: newMode }); session.updateAgentState((currentState) => ({ diff --git a/cli/src/codex/session.ts b/cli/src/codex/session.ts index a15e3282..e1dd4106 100644 --- a/cli/src/codex/session.ts +++ b/cli/src/codex/session.ts @@ -2,9 +2,11 @@ import { ApiClient, ApiSessionClient } from '@/lib'; import { MessageQueue2 } from '@/utils/MessageQueue2'; import { AgentSessionBase } from '@/agent/sessionBase'; import type { EnhancedMode } from './loop'; +import type { CodexCliOverrides } from './utils/codexCliOverrides'; export class CodexSession extends AgentSessionBase { readonly codexArgs?: string[]; + readonly codexCliOverrides?: CodexCliOverrides; constructor(opts: { api: ApiClient; @@ -16,6 +18,7 @@ export class CodexSession extends AgentSessionBase { onModeChange: (mode: 'local' | 'remote') => void; mode?: 'local' | 'remote'; codexArgs?: string[]; + codexCliOverrides?: CodexCliOverrides; }) { super({ api: opts.api, @@ -35,6 +38,7 @@ export class CodexSession extends AgentSessionBase { }); this.codexArgs = opts.codexArgs; + this.codexCliOverrides = opts.codexCliOverrides; } sendCodexMessage = (message: unknown): void => { diff --git a/cli/src/codex/utils/codexCliOverrides.test.ts b/cli/src/codex/utils/codexCliOverrides.test.ts new file mode 100644 index 00000000..ad349515 --- /dev/null +++ b/cli/src/codex/utils/codexCliOverrides.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest'; +import { parseCodexCliOverrides } from './codexCliOverrides'; + +describe('parseCodexCliOverrides', () => { + it('parses sandbox and approval flags', () => { + expect(parseCodexCliOverrides(['-s', 'read-only', '-a', 'on-request'])).toEqual({ + sandbox: 'read-only', + approvalPolicy: 'on-request' + }); + }); + + it('parses long flags with equals syntax', () => { + expect(parseCodexCliOverrides(['--sandbox=workspace-write', '--ask-for-approval=never'])).toEqual({ + sandbox: 'workspace-write', + approvalPolicy: 'never' + }); + }); + + it('parses convenience flags', () => { + expect(parseCodexCliOverrides(['--full-auto'])).toEqual({ + sandbox: 'workspace-write', + approvalPolicy: 'on-request' + }); + + expect(parseCodexCliOverrides(['--dangerously-bypass-approvals-and-sandbox'])).toEqual({ + sandbox: 'danger-full-access', + approvalPolicy: 'never' + }); + }); + + it('uses last value when flags repeat', () => { + expect(parseCodexCliOverrides(['--sandbox', 'read-only', '--sandbox', 'danger-full-access'])).toEqual({ + sandbox: 'danger-full-access' + }); + + expect(parseCodexCliOverrides(['-a', 'untrusted', '-a', 'on-failure'])).toEqual({ + approvalPolicy: 'on-failure' + }); + }); + + it('ignores invalid values and stops at terminator', () => { + expect(parseCodexCliOverrides(['--sandbox', 'nope', '--ask-for-approval', 'bad'])).toEqual({}); + + expect(parseCodexCliOverrides(['-s', 'read-only', '--', '-a', 'never'])).toEqual({ + sandbox: 'read-only' + }); + }); +}); diff --git a/cli/src/codex/utils/codexCliOverrides.ts b/cli/src/codex/utils/codexCliOverrides.ts new file mode 100644 index 00000000..61ed4d1d --- /dev/null +++ b/cli/src/codex/utils/codexCliOverrides.ts @@ -0,0 +1,82 @@ +export type CodexCliOverrides = { + sandbox?: 'read-only' | 'workspace-write' | 'danger-full-access'; + approvalPolicy?: 'untrusted' | 'on-failure' | 'on-request' | 'never'; +}; + +const SANDBOX_VALUES = new Set([ + 'read-only', + 'workspace-write', + 'danger-full-access' +]); + +const APPROVAL_POLICY_VALUES = new Set([ + 'untrusted', + 'on-failure', + 'on-request', + 'never' +]); + +export function parseCodexCliOverrides(args?: string[]): CodexCliOverrides { + const overrides: CodexCliOverrides = {}; + if (!args || args.length === 0) { + return overrides; + } + + for (let i = 0; i < args.length; i++) { + const arg = args[i]; + if (arg === '--') { + break; + } + + if (arg === '--full-auto') { + overrides.approvalPolicy = 'on-request'; + overrides.sandbox = 'workspace-write'; + continue; + } + + if (arg === '--dangerously-bypass-approvals-and-sandbox') { + overrides.approvalPolicy = 'never'; + overrides.sandbox = 'danger-full-access'; + continue; + } + + if (arg === '-s' || arg === '--sandbox') { + const value = args[i + 1]; + if (SANDBOX_VALUES.has(value as CodexCliOverrides['sandbox'])) { + overrides.sandbox = value as CodexCliOverrides['sandbox']; + i += 1; + } + continue; + } + + if (arg.startsWith('--sandbox=')) { + const value = arg.slice('--sandbox='.length); + if (SANDBOX_VALUES.has(value as CodexCliOverrides['sandbox'])) { + overrides.sandbox = value as CodexCliOverrides['sandbox']; + } + continue; + } + + if (arg === '-a' || arg === '--ask-for-approval') { + const value = args[i + 1]; + if (APPROVAL_POLICY_VALUES.has(value as CodexCliOverrides['approvalPolicy'])) { + overrides.approvalPolicy = value as CodexCliOverrides['approvalPolicy']; + i += 1; + } + continue; + } + + if (arg.startsWith('--ask-for-approval=')) { + const value = arg.slice('--ask-for-approval='.length); + if (APPROVAL_POLICY_VALUES.has(value as CodexCliOverrides['approvalPolicy'])) { + overrides.approvalPolicy = value as CodexCliOverrides['approvalPolicy']; + } + } + } + + return overrides; +} + +export function hasCodexCliOverrides(overrides?: CodexCliOverrides): boolean { + return Boolean(overrides?.sandbox || overrides?.approvalPolicy); +} diff --git a/cli/src/codex/utils/codexStartConfig.test.ts b/cli/src/codex/utils/codexStartConfig.test.ts new file mode 100644 index 00000000..c09d8815 --- /dev/null +++ b/cli/src/codex/utils/codexStartConfig.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest'; +import { buildCodexStartConfig } from './codexStartConfig'; + +describe('buildCodexStartConfig', () => { + const mcpServers = { hapi: { command: 'node', args: ['mcp'] } }; + + it('applies CLI overrides when permission mode is default', () => { + const config = buildCodexStartConfig({ + message: 'hello', + mode: { permissionMode: 'default' }, + first: true, + mcpServers, + cliOverrides: { sandbox: 'danger-full-access', approvalPolicy: 'never' } + }); + + expect(config.sandbox).toBe('danger-full-access'); + expect(config['approval-policy']).toBe('never'); + expect(config.config).toEqual({ mcp_servers: mcpServers }); + }); + + it('ignores CLI overrides when permission mode is not default', () => { + const config = buildCodexStartConfig({ + message: 'hello', + mode: { permissionMode: 'yolo' }, + first: false, + mcpServers, + cliOverrides: { sandbox: 'read-only', approvalPolicy: 'never' } + }); + + expect(config.sandbox).toBe('danger-full-access'); + expect(config['approval-policy']).toBe('on-failure'); + }); + + it('passes model when provided', () => { + const config = buildCodexStartConfig({ + message: 'hello', + mode: { permissionMode: 'default', model: 'o3' }, + first: false, + mcpServers + }); + + expect(config.model).toBe('o3'); + }); +}); diff --git a/cli/src/codex/utils/codexStartConfig.ts b/cli/src/codex/utils/codexStartConfig.ts new file mode 100644 index 00000000..758bd8ee --- /dev/null +++ b/cli/src/codex/utils/codexStartConfig.ts @@ -0,0 +1,59 @@ +import { trimIdent } from '@/utils/trimIdent'; +import type { CodexSessionConfig } from '../types'; +import type { EnhancedMode } from '../loop'; +import type { CodexCliOverrides } from './codexCliOverrides'; + +const TITLE_INSTRUCTION = trimIdent(`Based on this message, call functions.hapi__change_title to change chat session title that would represent the current task. If chat idea would change dramatically - call this function again to update the title.`); + +function resolveApprovalPolicy(mode: EnhancedMode): CodexSessionConfig['approval-policy'] { + switch (mode.permissionMode) { + case 'default': return 'untrusted'; + case 'read-only': return 'never'; + case 'safe-yolo': return 'on-failure'; + case 'yolo': return 'on-failure'; + default: { + throw new Error(`Unknown permission mode: ${mode.permissionMode}`); + } + } +} + +function resolveSandbox(mode: EnhancedMode): CodexSessionConfig['sandbox'] { + switch (mode.permissionMode) { + case 'default': return 'workspace-write'; + case 'read-only': return 'read-only'; + case 'safe-yolo': return 'workspace-write'; + case 'yolo': return 'danger-full-access'; + default: { + throw new Error(`Unknown permission mode: ${mode.permissionMode}`); + } + } +} + +export function buildCodexStartConfig(args: { + message: string; + mode: EnhancedMode; + first: boolean; + mcpServers: Record; + cliOverrides?: CodexCliOverrides; +}): CodexSessionConfig { + const approvalPolicy = resolveApprovalPolicy(args.mode); + const sandbox = resolveSandbox(args.mode); + const allowCliOverrides = args.mode.permissionMode === 'default'; + const cliOverrides = allowCliOverrides ? args.cliOverrides : undefined; + const resolvedApprovalPolicy = cliOverrides?.approvalPolicy ?? approvalPolicy; + const resolvedSandbox = cliOverrides?.sandbox ?? sandbox; + + const prompt = args.first ? `${args.message}\n\n${TITLE_INSTRUCTION}` : args.message; + const startConfig: CodexSessionConfig = { + prompt, + sandbox: resolvedSandbox, + 'approval-policy': resolvedApprovalPolicy, + config: { mcp_servers: args.mcpServers } + }; + + if (args.mode.model) { + startConfig.model = args.mode.model; + } + + return startConfig; +}