diff --git a/cli/src/codex/codexAppServerClient.ts b/cli/src/codex/codexAppServerClient.ts index 758b5605..f0f8510f 100644 --- a/cli/src/codex/codexAppServerClient.ts +++ b/cli/src/codex/codexAppServerClient.ts @@ -77,10 +77,15 @@ export class CodexAppServerClient { private readonly pending = new Map(); private readonly requestHandlers = new Map(); private notificationHandler: ((method: string, params: unknown) => void) | null = null; + private stderrHandler: ((text: string) => void) | null = null; private protocolError: Error | null = null; static readonly DEFAULT_TIMEOUT_MS = 14 * 24 * 60 * 60 * 1000; + setStderrHandler(handler: ((text: string) => void) | null): void { + this.stderrHandler = handler; + } + async connect(): Promise { if (this.connected) { return; @@ -105,6 +110,7 @@ export class CodexAppServerClient { const text = chunk.toString().trim(); if (text.length > 0) { logger.debug(`[CodexAppServer][stderr] ${text}`); + this.stderrHandler?.(text); } }); diff --git a/cli/src/codex/codexRemoteLauncher.test.ts b/cli/src/codex/codexRemoteLauncher.test.ts index 7e703a3f..21b4f64f 100644 --- a/cli/src/codex/codexRemoteLauncher.test.ts +++ b/cli/src/codex/codexRemoteLauncher.test.ts @@ -45,6 +45,9 @@ const harness = vi.hoisted(() => ({ emitParentTitleChange: false, emitParentSpawnFailureWithoutAgentId: false, emitParentSpawnStartWithoutEnd: false, + emitParentSpawnRouterStderrError: false, + emitChildTaskStartedAfterParentSpawnStart: false, + emitSecondParentSpawnStartWithoutEnd: false, emitParentSendInputFailure: false, emitParentResumeSuccess: false, emitRunningChildTurnBeforeSuppressedParent: false, @@ -56,6 +59,7 @@ const harness = vi.hoisted(() => ({ vi.mock('./codexAppServerClient', () => { class MockCodexAppServerClient { private notificationHandler: ((method: string, params: unknown) => void) | null = null; + private stderrHandler: ((text: string) => void) | null = null; async connect(): Promise {} @@ -68,6 +72,10 @@ vi.mock('./codexAppServerClient', () => { this.notificationHandler = handler; } + setStderrHandler(handler: ((text: string) => void) | null): void { + this.stderrHandler = handler; + } + async listCollaborationModes(): Promise { harness.listCollaborationModeCalls += 1; if (harness.failListCollaborationModes) { @@ -310,6 +318,42 @@ vi.mock('./codexAppServerClient', () => { harness.notifications.push({ method: 'item/started', params: spawnStart }); this.notificationHandler?.('item/started', spawnStart); + if (harness.emitSecondParentSpawnStartWithoutEnd) { + const secondSpawnStart = { + item: { + id: 'second-spawn', + type: 'collabAgentToolCall', + tool: 'spawnAgent', + prompt: 'do other side work', + senderThreadId: threadId, + receiverThreadIds: [] + }, + threadId, + turnId + }; + harness.notifications.push({ method: 'item/started', params: secondSpawnStart }); + this.notificationHandler?.('item/started', secondSpawnStart); + } + + if (harness.emitParentSpawnRouterStderrError) { + this.stderrHandler?.( + 'codex_core::tools::router: error=Full-history forked agents inherit the parent agent type, model, and reasoning effort; ' + + 'omit agent_type, model, and reasoning_effort, or spawn without a full-history fork.' + ); + } + + if (harness.emitChildTaskStartedAfterParentSpawnStart) { + const childStarted = { + msg: { + type: 'task_started', + thread_id: 'child-thread', + turn_id: 'child-turn' + } + }; + harness.notifications.push({ method: 'codex/event/task_started', params: childStarted }); + this.notificationHandler?.('codex/event/task_started', childStarted); + } + if (harness.emitParentSpawnFailureWithoutAgentId) { const spawnCompleted = { item: { @@ -888,6 +932,9 @@ describe('codexRemoteLauncher', () => { harness.emitParentTitleChange = false; harness.emitParentSpawnFailureWithoutAgentId = false; harness.emitParentSpawnStartWithoutEnd = false; + harness.emitParentSpawnRouterStderrError = false; + harness.emitChildTaskStartedAfterParentSpawnStart = false; + harness.emitSecondParentSpawnStartWithoutEnd = false; harness.emitParentSendInputFailure = false; harness.emitParentResumeSuccess = false; harness.emitRunningChildTurnBeforeSuppressedParent = false; @@ -1743,6 +1790,83 @@ describe('codexRemoteLauncher', () => { })); }); + it('marks pending spawn_agent cards failed with the Codex router argument error from stderr', async () => { + harness.emitParentSpawnStartWithoutEnd = true; + harness.emitParentSpawnRouterStderrError = true; + const { session, codexMessages } = createSessionStub(); + + await codexRemoteLauncher(session as never); + + expect(codexMessages).toContainEqual(expect.objectContaining({ + type: 'agent-run-update', + agentId: 'spawn-error:failed-spawn', + cardId: 'failed-spawn', + status: 'failed', + statusText: 'Failed to start', + activityKind: 'failed', + error: 'Full-history forked agents inherit the parent agent type, model, and reasoning effort; ' + + 'omit agent_type, model, and reasoning_effort, or spawn without a full-history fork.' + })); + expect(codexMessages).not.toContainEqual(expect.objectContaining({ + type: 'agent-run-update', + agentId: 'spawn-error:failed-spawn', + cardId: 'failed-spawn', + error: 'spawn_agent did not return an agent id before the Codex session ended' + })); + }); + + it('links a lone pending spawn_agent card from the child task_started event', async () => { + harness.emitParentSpawnStartWithoutEnd = true; + harness.emitChildTaskStartedAfterParentSpawnStart = true; + const { session, codexMessages } = createSessionStub(); + + await codexRemoteLauncher(session as never); + + expect(codexMessages).toContainEqual(expect.objectContaining({ + type: 'agent-run-update', + agentId: 'child-thread', + cardId: 'failed-spawn', + status: 'running', + activity: 'Started', + activityKind: 'running' + })); + expect(codexMessages).not.toContainEqual(expect.objectContaining({ + type: 'agent-run-update', + agentId: 'spawn-error:failed-spawn', + cardId: 'failed-spawn' + })); + }); + + it('does not guess a child task_started card when multiple spawn_agent starts are pending', async () => { + harness.emitParentSpawnStartWithoutEnd = true; + harness.emitSecondParentSpawnStartWithoutEnd = true; + harness.emitChildTaskStartedAfterParentSpawnStart = true; + const { session, codexMessages } = createSessionStub(); + + await codexRemoteLauncher(session as never); + + expect(codexMessages).not.toContainEqual(expect.objectContaining({ + type: 'agent-run-update', + agentId: 'child-thread', + cardId: 'failed-spawn' + })); + expect(codexMessages).not.toContainEqual(expect.objectContaining({ + type: 'agent-run-update', + agentId: 'child-thread', + cardId: 'second-spawn' + })); + expect(codexMessages).toContainEqual(expect.objectContaining({ + type: 'agent-run-update', + agentId: 'spawn-error:failed-spawn', + cardId: 'failed-spawn' + })); + expect(codexMessages).toContainEqual(expect.objectContaining({ + type: 'agent-run-update', + agentId: 'spawn-error:second-spawn', + cardId: 'second-spawn' + })); + }); + it('marks pending spawn_agent cards failed when the session ends before a result', async () => { harness.emitParentSpawnStartWithoutEnd = true; const { session, codexMessages } = createSessionStub(); diff --git a/cli/src/codex/codexRemoteLauncher.ts b/cli/src/codex/codexRemoteLauncher.ts index 005c7d0e..01c2a459 100644 --- a/cli/src/codex/codexRemoteLauncher.ts +++ b/cli/src/codex/codexRemoteLauncher.ts @@ -46,6 +46,9 @@ type ChildAgentRuntime = { const AGENT_RUN_UPDATE_THROTTLE_MS = 300; const AGENT_RUN_START_TIMEOUT_MS = 30 * 1000; const THROTTLED_AGENT_RUN_ACTIVITY_KINDS = new Set(['thinking']); +const CODEX_SPAWN_AGENT_FULL_HISTORY_ARGUMENT_ERROR = + 'Full-history forked agents inherit the parent agent type, model, and reasoning effort; ' + + 'omit agent_type, model, and reasoning_effort, or spawn without a full-history fork.'; const SAME_THREAD_RETRYABLE_ERROR_PATTERNS = [ 'selected model is at capacity', @@ -103,6 +106,10 @@ function formatGoalUsage(goal: ThreadGoal): string { return parts.join(' ยท '); } +function stripAnsi(value: string): string { + return value.replace(/\u001b\[[0-9;]*m/g, ''); +} + class CodexRemoteLauncher extends RemoteLauncherBase { private readonly session: CodexSession; private readonly appServerClient: CodexAppServerClient; @@ -254,6 +261,13 @@ class CodexRemoteLauncher extends RemoteLauncherBase { return error instanceof Error ? error.message : String(error); }; + const extractSpawnAgentStartErrorFromStderr = (text: string): string | null => { + const cleanText = stripAnsi(text); + return cleanText.includes(CODEX_SPAWN_AGENT_FULL_HISTORY_ARGUMENT_ERROR) + ? CODEX_SPAWN_AGENT_FULL_HISTORY_ARGUMENT_ERROR + : null; + }; + const isExitPlanModeTool = (toolName: string): boolean => { return toolName === 'exit_plan_mode' || toolName === 'ExitPlanMode'; }; @@ -714,6 +728,38 @@ class CodexRemoteLauncher extends RemoteLauncherBase { flushPendingAgentTraces(agentId); }; + const getOnlyPendingAgentStartCardId = (): string | null => { + if (pendingAgentStartCardIds.size !== 1) return null; + return pendingAgentStartCardIds.values().next().value ?? null; + }; + + const linkPendingAgentStartFromChildTask = (agentId: string): void => { + if (agentCardByAgentId.has(agentId)) { + return; + } + + const cardId = getOnlyPendingAgentStartCardId(); + if (!cardId) { + if (pendingAgentStartCardIds.size > 1) { + logger.debug( + `[Codex] Child task_started while ${pendingAgentStartCardIds.size} spawn_agent cards are pending; ` + + `not linking automatically; agentId=${agentId}` + ); + } + return; + } + + logger.debug(`[Codex] Linking pending spawn_agent card from child task_started; cardId=${cardId}, agentId=${agentId}`); + linkAgentToCard(agentId, cardId); + emitAgentRunUpdate(agentId, { + status: 'running', + statusText: 'Running', + activity: 'Started', + activityKind: 'running' + }, cardId); + flushPendingAgentUpdates(agentId); + }; + const flushPendingAgentUpdates = (agentId: string): void => { const updates = pendingAgentUpdatesByAgentId.get(agentId); if (!updates || updates.length === 0) return; @@ -917,6 +963,56 @@ class CodexRemoteLauncher extends RemoteLauncherBase { } }; + const isPresentSpawnOption = (value: unknown): boolean => { + if (value === undefined || value === null) return false; + return typeof value !== 'string' || value.trim().length > 0; + }; + + const isFullHistorySpawnWithInheritedOverrides = (input: unknown): boolean => { + const record = asRecord(input); + if (!record) return false; + + const forkContext = record.fork_context ?? record.forkContext; + if (forkContext === false) { + return false; + } + + return [ + record.agent_type, + record.agentType, + record.subagent_type, + record.subagentType, + record.model, + record.reasoning_effort, + record.reasoningEffort + ].some(isPresentSpawnOption); + }; + + const failPendingAgentStartsForSpawnArgumentError = (error: unknown): void => { + const matchingCardIds = Array.from(pendingAgentStartCardIds).filter((cardId) => { + return isFullHistorySpawnWithInheritedOverrides( + pendingAgentToolInputByCallId.get(cardId)?.input + ); + }); + + if (matchingCardIds.length > 0) { + for (const cardId of matchingCardIds) { + failAgentStartCard(cardId, error); + } + return; + } + + if (pendingAgentStartCardIds.size === 1) { + failPendingAgentStarts(error); + return; + } + + logger.debug( + `[Codex] Ignoring spawn_agent argument stderr error for ${pendingAgentStartCardIds.size} ` + + 'pending starts because none have detectable inherited-override args' + ); + }; + const emitAgentRunTraceMessage = (agentId: string, message: unknown): void => { const cardId = agentCardByAgentId.get(agentId); if (!cardId) { @@ -1763,6 +1859,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { } else { logger.debug(`[Codex] Child task_started missing turn id; threadId=${eventThreadId}`); } + linkPendingAgentStartFromChildTask(eventThreadId); } else if (isTerminalEvent) { this.activeChildTurns.delete(eventThreadId); } @@ -2261,6 +2358,18 @@ class CodexRemoteLauncher extends RemoteLauncherBase { } }); + appServerClient.setStderrHandler((text) => { + const spawnAgentError = extractSpawnAgentStartErrorFromStderr(text); + if (!spawnAgentError || pendingAgentStartCardIds.size === 0) { + return; + } + logger.debug( + `[Codex] Failing ${pendingAgentStartCardIds.size} pending spawn_agent start(s) ` + + `from app-server stderr: ${spawnAgentError}` + ); + failPendingAgentStartsForSpawnArgumentError(spawnAgentError); + }); + const { server: happyServer, mcpServers } = await buildHapiMcpBridge(session.client, { // In app-server/collab mode, child agents share this MCP bridge. // If the MCP handler writes the title directly, child title calls @@ -2859,6 +2968,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { protected async cleanup(): Promise { logger.debug('[codex-remote]: cleanup start'); + this.appServerClient.setStderrHandler(null); try { await this.appServerClient.disconnect(); } catch (error) { diff --git a/cli/src/codex/utils/appServerConfig.test.ts b/cli/src/codex/utils/appServerConfig.test.ts index f06d272e..26e1cc5c 100644 --- a/cli/src/codex/utils/appServerConfig.test.ts +++ b/cli/src/codex/utils/appServerConfig.test.ts @@ -273,9 +273,9 @@ describe('appServerConfig', () => { }); const instructions = params.collaborationMode?.settings.developer_instructions; - expect(instructions).toContain('If you call spawn_agent with fork_context: true'); + expect(instructions).toContain('Treat omitted fork_context the same as fork_context: true'); expect(instructions).toContain('do not set agent_type, model, or reasoning_effort'); - expect(instructions).toContain('omit fork_context or set fork_context: false'); + expect(instructions).toContain('set fork_context: false'); expect(instructions).toContain('Do not rely on parent turn reasoning settings for spawned agents'); }); diff --git a/cli/src/codex/utils/appServerConfig.ts b/cli/src/codex/utils/appServerConfig.ts index 375d5a80..4a2e4f7e 100644 --- a/cli/src/codex/utils/appServerConfig.ts +++ b/cli/src/codex/utils/appServerConfig.ts @@ -13,8 +13,9 @@ import { resolveCodexPermissionModeConfig } from './permissionModeConfig'; export const codexCollaborationSpawnAgentInstructions = [ 'Codex sub-agent spawning rules:', - '- If you call spawn_agent with fork_context: true, do not set agent_type, model, or reasoning_effort; full-history forked agents inherit these values from the parent.', - '- If you need a specific agent_type, model, or reasoning_effort, omit fork_context or set fork_context: false, and include only the necessary context in the message.', + '- Treat omitted fork_context the same as fork_context: true: a full-history fork inherits the parent agent type, model, and reasoning effort.', + '- If you call spawn_agent with fork_context omitted or true, do not set agent_type, model, or reasoning_effort.', + '- If you need a specific agent_type, model, or reasoning_effort, set fork_context: false and include only the necessary context in the message.', '- Do not rely on parent turn reasoning settings for spawned agents; only set reasoning_effort on spawn_agent when the chosen child model supports it.' ].join('\n');