From 85af83203d1092bea99b53353634cc9f96ff53bb Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Wed, 24 Dec 2025 20:33:53 +0700 Subject: [PATCH] Fix exit code parsing for Codex MCP --- .../server/agent/providers/codex-mcp-agent.ts | 26 ++++++++++++++++--- plan.md | 20 +++++++++++++- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/packages/server/src/server/agent/providers/codex-mcp-agent.ts b/packages/server/src/server/agent/providers/codex-mcp-agent.ts index e6691122f..f8b9968f8 100644 --- a/packages/server/src/server/agent/providers/codex-mcp-agent.ts +++ b/packages/server/src/server/agent/providers/codex-mcp-agent.ts @@ -139,6 +139,22 @@ function normalizeThreadEventType(type: string): string { return type; } +function normalizeExitCode(value: unknown): number | undefined { + if (typeof value === "number" && Number.isFinite(value)) { + return value; + } + if (typeof value === "string") { + const trimmed = value.trim(); + if (trimmed.length) { + const parsed = Number(trimmed); + if (Number.isFinite(parsed)) { + return parsed; + } + } + } + return undefined; +} + function buildCommandDisplayName(command?: unknown): string { if (typeof command === "string") { const trimmed = command.trim(); @@ -994,7 +1010,7 @@ class CodexMcpAgentSession implements AgentSession { const cwd = (event as { cwd?: string }).cwd; const exitCodeRaw = (event as { exit_code?: unknown; exitCode?: unknown }).exit_code ?? (event as { exitCode?: unknown }).exitCode; - const exitCode = typeof exitCodeRaw === "number" ? exitCodeRaw : undefined; + const exitCode = normalizeExitCode(exitCodeRaw); const output = (event as { output?: unknown; stdout?: unknown }).output ?? (event as { stdout?: unknown }).stdout ?? (event as { stderr?: unknown }).stderr; @@ -1155,7 +1171,7 @@ class CodexMcpAgentSession implements AgentSession { }); } if (item.type === "command_execution") { - const exitCode = item.exit_code ?? item.exitCode; + const exitCode = normalizeExitCode(item.exit_code ?? item.exitCode); if (typeof exitCode === "number" && exitCode !== 0) { this.turnState && (this.turnState.sawError = true); this.emitEvent({ @@ -1200,8 +1216,10 @@ class CodexMcpAgentSession implements AgentSession { return { type: "user_message", text: item.text as string }; case "command_execution": { const aggregatedOutput = (item as { aggregated_output?: unknown }).aggregated_output; - const exitCode = (item as { exit_code?: unknown; exitCode?: unknown }).exit_code ?? - (item as { exitCode?: unknown }).exitCode; + const exitCode = normalizeExitCode( + (item as { exit_code?: unknown; exitCode?: unknown }).exit_code ?? + (item as { exitCode?: unknown }).exitCode + ); const cwd = (item as { cwd?: string }).cwd; const commandValue = item.command; const command = diff --git a/plan.md b/plan.md index faa815361..e2b428644 100644 --- a/plan.md +++ b/plan.md @@ -262,7 +262,8 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov - [x] **Fix**: Codex SDK persisted shell_command hydration still missing completed status. - **Done (2025-12-24 20:31)**: Mapped shell_command custom_tool_call entries to command tool calls and normalized output/status during rollout hydration. -- [ ] **Fix**: Codex MCP command output should include exit codes for command tool calls (missing in timeline mapping). +- [x] **Fix**: Codex MCP command output should include exit codes for command tool calls (missing in timeline mapping). + - **Done (2025-12-24 20:33)**: Normalized exit code parsing so numeric strings are captured in timeline output. - [ ] **Fix**: Codex MCP thread/item event mapping for file_change, mcp_tool_call, web_search, and todo_list still failing. @@ -274,6 +275,23 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov - [ ] **Fix**: Investigate `agent-mcp.e2e.test.ts` hang (Claude agent flow) and add timeout/skip conditions as needed. +- [ ] **Test (E2E) CRITICAL**: Interruption/abort latency for Codex MCP provider. + + - **Requirement**: Interrupting a long-running operation must stop within 1 second + - Test setup: + - Ask Codex to run a long command (e.g., `sleep 300`, `for i in {1..1000000}; do echo $i; done`, or similar) + - Wait for the command to start executing (tool_call event received) + - Call `session.interrupt()` or equivalent abort signal + - Measure time from interrupt call to session fully stopped + - Pass criteria: + - Interrupt completes in < 1 second + - No zombie processes left running + - Session state is clean (can start new session) + - Test BOTH: + - Codex MCP provider interruption + - Codex SDK provider interruption (if applicable) + - This is critical for user experience - users expect immediate response to cancel + - [ ] **Test (E2E)**: Permission flow parity - test both Codex MCP and Claude providers. - Create/update E2E tests that verify permissions work for BOTH providers