diff --git a/packages/server/src/server/agent/providers/codex-agent.ts b/packages/server/src/server/agent/providers/codex-agent.ts index 3e28d1d3f..1c362054b 100644 --- a/packages/server/src/server/agent/providers/codex-agent.ts +++ b/packages/server/src/server/agent/providers/codex-agent.ts @@ -1660,23 +1660,39 @@ function finalizeRolloutFunctionCall( return; } const result = parseJsonOrObject(payload.output); - const exitCode = result?.metadata?.exit_code; + const parsedOutput = + typeof payload.output === "string" + ? parseCommandOutputText(payload.output) + : null; + const exitCode = result?.metadata?.exit_code ?? parsedOutput?.exitCode; const status = exitCode === undefined || exitCode === 0 ? "completed" : "failed"; + const stdout = + result?.stdout ?? + (typeof result?.output === "string" ? result.output : undefined) ?? + parsedOutput?.stdout; + const metadata = + result?.metadata ?? + (typeof exitCode === "number" ? { exit_code: exitCode } : undefined); // Build structured command output - const output = result?.stdout - ? { - type: "command" as const, - command: command.command, - output: result.stdout, - exitCode, - cwd: command.cwd, - metadata: - result?.metadata ?? - (typeof exitCode === "number" ? { exit_code: exitCode } : undefined), - } - : result; + let output: unknown; + if (stdout !== undefined) { + output = { + type: "command" as const, + command: command.command, + output: stdout, + exitCode, + cwd: command.cwd, + metadata, + }; + } else if (result && typeof result === "object") { + output = { ...result, metadata: metadata ?? result.metadata }; + } else if (metadata) { + output = { metadata }; + } else { + output = result; + } events.push({ type: "timeline", @@ -2116,6 +2132,32 @@ function parseJsonOrObject(value: unknown): T | null { return null; } +function parseCommandOutputText( + output: string +): { exitCode?: number; stdout?: string } | null { + if (!output.trim()) { + return null; + } + + const exitCodeMatch = output.match(/Exit code:\s*(\d+)/i); + const exitCode = + exitCodeMatch && exitCodeMatch[1] + ? Number.parseInt(exitCodeMatch[1], 10) + : undefined; + + const outputMatch = output.match(/\nOutput:\s*\n([\s\S]*)$/); + const stdout = outputMatch ? outputMatch[1].replace(/\s+$/, "") : undefined; + + if (exitCode === undefined && stdout === undefined) { + return null; + } + + return { + exitCode: Number.isNaN(exitCode as number) ? undefined : exitCode, + stdout, + }; +} + function formatCommand(args: unknown): string | null { if (!args || typeof args !== "object") { return null; diff --git a/plan.md b/plan.md index 9db0572fd..ca07ea08c 100644 --- a/plan.md +++ b/plan.md @@ -175,29 +175,29 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov - Decide: keep MCP for tools, or drop it and use SDK only? - **Done (2025-12-24 20:04)**: Concluded MCP provider is only necessary for external MCP tool integration; for permissions and core workflows, SDK provider is simpler and already supports approvals. Recommend dropping/parking MCP unless external MCP tool usage is a requirement. -- [ ] **CRITICAL FINDING**: `codex exec` (SDK) is non-interactive - cannot prompt! - - - Ran `scripts/codex-sdk-permission-test.mjs` with workspace-write + untrusted - - NO approval events fire - commands just run or refuse - - `codex exec --experimental-json` mode is NON-INTERACTIVE by design - - It either refuses ("approvals are disabled") or runs in sandbox - - The interactive `codex` CLI is what prompts - but SDK uses `codex exec` - - This explains why SDK never emits `exec_approval_request` events - - MCP server (`codex mcp-server`) may work differently via MCP elicitation - - The SDK provider CANNOT support interactive approvals by design - - [x] **Plan**: Re-audit based on test results. - **Done (2025-12-24 20:06)**: Reviewed latest E2E failures/hang; added fix tasks for Codex SDK hydration and Codex MCP abort hang, plus a retest task. -- [ ] **Fix**: Codex SDK persistence hydration should emit completed shell_command tool entries. +- [x] **Fix**: Codex SDK persistence hydration should emit completed shell_command tool entries. - Capture the failing rollout entry and ensure hydrated tool calls include completed status + exit code metadata. + - **Done (2025-12-24 20:12)**: Parsed rollout command output strings to extract exit codes/stdout and attach metadata to hydrated shell tool results. - [ ] **Fix**: Codex MCP E2E hang in long-running command abort test. - Add deterministic abort/timeout handling and ensure the session closes even if the sleep tool call is never surfaced. +- [ ] **CRITICAL FINDING (VERIFIED IN SOURCE)**: `codex exec` IGNORES approval events! + + - In `codex-rs/exec/src/event_processor_with_human_output.rs:568`: + - `ExecApprovalRequest` and `ApplyPatchApprovalRequest` are in ignore match arm `=> {}` + - `codex exec` receives approval events but DOESN'T emit them as JSON + - SDK uses `codex exec` → CANNOT support approvals BY DESIGN + - MCP server DOES handle them → sends `ElicitRequest` (see `exec_approval.rs:107`) + - CONCLUSION: MCP provider is the ONLY path to real permissions, not SDK + - The agent's earlier conclusion to "park MCP" was WRONG + - [ ] **Test (E2E)**: Rerun server vitest after fixes. - If failures: add follow-up fix tasks immediately after this item.