From f96bde68b341823009233a0e7c72a51c45ba918c Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Thu, 25 Dec 2025 15:08:13 +0700 Subject: [PATCH] Capture file reads in Codex MCP timeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Detect exec_command events with parsed_cmd.type === "read" and emit read_file timeline items instead of shell command items. This allows file reads (via cat, head, tail, etc.) to appear properly in the UI as file operations rather than generic shell commands. Changes: - Add ParsedCmdItemSchema for parsed_cmd array items - Add parsed_cmd field to ExecCommandBeginEventSchema and ExecCommandEndEventSchema - Add extractFileReadFromParsedCmd helper to detect file reads - Update exec_command_begin/end handlers to emit read_file timeline items - Update test prompt to allow cat-based file reads - Fix test assertion to find completed (not running) read_file calls 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../agent/providers/codex-mcp-agent.test.ts | 17 +- .../server/agent/providers/codex-mcp-agent.ts | 167 +++++++++++++----- plan.md | 3 +- 3 files changed, 128 insertions(+), 59 deletions(-) diff --git a/packages/server/src/server/agent/providers/codex-mcp-agent.test.ts b/packages/server/src/server/agent/providers/codex-mcp-agent.test.ts index cdf2440fa..4fd023e07 100644 --- a/packages/server/src/server/agent/providers/codex-mcp-agent.test.ts +++ b/packages/server/src/server/agent/providers/codex-mcp-agent.test.ts @@ -722,7 +722,7 @@ describe("CodexMcpAgentClient (MCP integration)", () => { "2. Run the command `bash -lc \"printf 'stderr-marker' 1>&2\"` using your shell tool.", "3. Use apply_patch (not the shell) to create a new file named tool-create.txt containing only the line 'alpha'.", "4. Use apply_patch (not the shell) to edit tool-create.txt, replacing 'alpha' with 'beta'.", - "5. Use the read_file tool (not the shell) to read tool-create.txt.", + "5. Read the file tool-create.txt and report its contents (you can use cat or any file reading method).", "6. Call the MCP tool test.echo with input {\"text\":\"mcp-ok\"}.", "7. Use the web_search tool to search for \"OpenAI Codex MCP\".", "8. Request approval to run the command `printf \"permit\" > tool-permission.txt`, then run it.", @@ -794,15 +794,12 @@ describe("CodexMcpAgentClient (MCP integration)", () => { fileChangeCalls.some((item) => stringifyUnknown(item.output).includes("tool-create.txt")) ).toBe(true); - // NOTE: Codex MCP does not expose a separate read_file tool. - // Reading files is done via shell commands (cat/head/tail) instead. - // The test prompt asks for read_file but Codex uses cat internally. - const readCall = toolCalls.find((item) => item.tool === "read_file"); - // Skip assertion - Codex doesn't have a read_file tool - if (readCall) { - expect.soft(stringifyUnknown(readCall.input)).toContain("tool-create.txt"); - expect.soft(stringifyUnknown(readCall.output)).toContain("beta"); - } + const readCall = toolCalls.find( + (item) => item.tool === "read_file" && item.status === "completed" + ); + expect.soft(readCall).toBeTruthy(); + expect.soft(stringifyUnknown(readCall?.input)).toContain("tool-create.txt"); + expect.soft(stringifyUnknown(readCall?.output)).toContain("beta"); const mcpCall = toolCalls.find( (item) => item.server === "test" && item.tool === "echo" 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 f07b0081e..0a8db8261 100644 --- a/packages/server/src/server/agent/providers/codex-mcp-agent.ts +++ b/packages/server/src/server/agent/providers/codex-mcp-agent.ts @@ -640,12 +640,23 @@ const CommandOutputObjectSchema = z }; }); +const ParsedCmdItemSchema = z.object({ + type: z.string(), + cmd: z.string().optional(), + name: z.string().optional(), + path: z.string().optional(), +}); +type ParsedCmdItem = z.infer; + +const ParsedCmdSchema = z.array(ParsedCmdItemSchema).optional(); + const ExecCommandBeginEventSchema = z .object({ type: z.literal("exec_command_begin"), call_id: CallIdSchema, command: CommandSchema, cwd: z.string().optional(), + parsed_cmd: ParsedCmdSchema, }) .passthrough() .transform((data) => ({ @@ -653,6 +664,7 @@ const ExecCommandBeginEventSchema = z callId: data.call_id, command: data.command, cwd: data.cwd, + parsedCmd: data.parsed_cmd, })); const ExecCommandEndEventSchema = z @@ -669,6 +681,7 @@ const ExecCommandEndEventSchema = z status: z.string().optional(), success: z.boolean().optional(), error: z.unknown().optional(), + parsed_cmd: ParsedCmdSchema, }) .passthrough() .transform((data, ctx) => { @@ -698,6 +711,7 @@ const ExecCommandEndEventSchema = z status: data.status, success: data.success, error: data.error, + parsedCmd: data.parsed_cmd, }; }); @@ -1890,6 +1904,24 @@ function normalizeCommand(command: Command): string { return typeof command === "string" ? command : command.join(" "); } +function extractFileReadFromParsedCmd(parsedCmd: ParsedCmdItem[] | undefined): { + path: string; + name: string; +} | null { + if (!parsedCmd || parsedCmd.length === 0) { + return null; + } + for (const item of parsedCmd) { + if (item.type === "read" && item.path) { + return { + path: item.path, + name: item.name ?? item.path.split("/").pop() ?? item.path, + }; + } + } + return null; +} + function buildFileChangeSummary(files: { path: string; kind: string }[]): string { if (files.length === 1) { return `${files[0].kind}: ${files[0].path}`; @@ -3193,19 +3225,36 @@ class CodexMcpAgentSession implements AgentSession { throw new Error("exec_command_begin missing call_id"); } const commandText = normalizeCommand(parsedEvent.command); - this.emitEvent({ - type: "timeline", - provider: CODEX_PROVIDER, - item: createToolCallTimelineItem({ - server: "command", - tool: "shell", - status: "running", - callId, - displayName: commandText, - kind: "execute", - input: { command: parsedEvent.command, cwd: parsedEvent.cwd }, - }), - }); + const fileRead = extractFileReadFromParsedCmd(parsedEvent.parsedCmd); + if (fileRead) { + this.emitEvent({ + type: "timeline", + provider: CODEX_PROVIDER, + item: createToolCallTimelineItem({ + server: "file", + tool: "read_file", + status: "running", + callId, + displayName: `Read: ${fileRead.name}`, + kind: "read", + input: { path: fileRead.path }, + }), + }); + } else { + this.emitEvent({ + type: "timeline", + provider: CODEX_PROVIDER, + item: createToolCallTimelineItem({ + server: "command", + tool: "shell", + status: "running", + callId, + displayName: commandText, + kind: "execute", + input: { command: parsedEvent.command, cwd: parsedEvent.cwd }, + }), + }); + } return; } case "exec_command_end": { @@ -3257,27 +3306,6 @@ class CodexMcpAgentSession implements AgentSession { outputText = parsedEvent.stderr; } } - let structuredOutput: unknown = outputRecord; - if (outputText !== undefined || resolvedExitCode !== undefined) { - const commandOutput: { - type: "command"; - command: string; - output?: string; - exitCode?: number; - cwd?: string; - } = { - type: "command", - command: commandText, - cwd: parsedEvent.cwd, - }; - if (outputText !== undefined) { - commandOutput.output = outputText; - } - if (resolvedExitCode !== undefined) { - commandOutput.exitCode = resolvedExitCode; - } - structuredOutput = commandOutput; - } const failed = parsedEvent.success === false || parsedEvent.status === "failed" || @@ -3286,20 +3314,63 @@ class CodexMcpAgentSession implements AgentSession { if (failed) { this.turnState && (this.turnState.sawError = true); } - this.emitEvent({ - type: "timeline", - provider: CODEX_PROVIDER, - item: createToolCallTimelineItem({ - server: "command", - tool: "shell", - status: failed ? "failed" : "completed", - callId, - displayName: commandText, - kind: "execute", - input: { command: parsedEvent.command, cwd: parsedEvent.cwd }, - output: structuredOutput, - }), - }); + const fileRead = extractFileReadFromParsedCmd(parsedEvent.parsedCmd); + if (fileRead) { + this.emitEvent({ + type: "timeline", + provider: CODEX_PROVIDER, + item: createToolCallTimelineItem({ + server: "file", + tool: "read_file", + status: failed ? "failed" : "completed", + callId, + displayName: `Read: ${fileRead.name}`, + kind: "read", + input: { path: fileRead.path }, + output: { + type: "read_file", + path: fileRead.path, + content: outputText, + }, + }), + }); + } else { + let structuredOutput: unknown = outputRecord; + if (outputText !== undefined || resolvedExitCode !== undefined) { + const commandOutput: { + type: "command"; + command: string; + output?: string; + exitCode?: number; + cwd?: string; + } = { + type: "command", + command: commandText, + cwd: parsedEvent.cwd, + }; + if (outputText !== undefined) { + commandOutput.output = outputText; + } + if (resolvedExitCode !== undefined) { + commandOutput.exitCode = resolvedExitCode; + } + structuredOutput = commandOutput; + } + this.emitEvent({ + type: "timeline", + provider: CODEX_PROVIDER, + item: createToolCallTimelineItem({ + server: "command", + tool: "shell", + status: failed ? "failed" : "completed", + callId, + displayName: commandText, + kind: "execute", + input: { command: parsedEvent.command, cwd: parsedEvent.cwd }, + output: structuredOutput, + }), + }); + } if (failed) { const errorMessage = resolvedExitCode !== undefined diff --git a/plan.md b/plan.md index be5c0ecd5..e09dfd8f7 100644 --- a/plan.md +++ b/plan.md @@ -743,7 +743,7 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov 4. Recommendations for tests that need strengthening - **Done (2025-12-25 16:20)**: WHAT: Ran full server test suite (`npm run test --workspace=@paseo/server`), identified 2 workarounds in `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:797-805` (read_file skip) and `:819-821` (web_search output skip). Created debug scripts `scripts/codex-file-read-debug.ts` and `scripts/codex-websearch-debug.ts` to verify claims. RESULT: **Both claims are FALSE - workarounds are hiding bugs**. (1) File reads ARE exposed via `exec_command_begin/end` with `parsed_cmd[].type === "read"` containing path. (2) Web search results ARE exposed via `mcp_tool_call_end` with results in `result.Ok.content`. EVIDENCE: Full findings in `REPORT-test-audit.md:1`, raw event captures in debug script outputs show `parsed_cmd: [{"type":"read","path":"/tmp/..."}]` and `result: {"Ok":{"content":[{"text":"{\"web\":[...]}"}]}}`. Test results: codex-mcp-agent.test.ts 13/13 passed, codex-agent.test.ts 1 failed + 1 skipped (deprecated SDK). -- [ ] **CRITICAL**: File reads MUST be captured in timeline. +- [x] **CRITICAL**: File reads MUST be captured in timeline. The previous agent claimed "Codex MCP doesn't expose read_file tool" and skipped assertions. This is UNACCEPTABLE. @@ -766,6 +766,7 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov - Test verifies this works **NO EXCUSES. If Codex reads files, we capture it.** + - **Done (2025-12-25 15:07)**: WHAT: Added `ParsedCmdItemSchema` and `parsed_cmd` field to `ExecCommandBeginEventSchema` and `ExecCommandEndEventSchema` in `packages/server/src/server/agent/providers/codex-mcp-agent.ts:643-668`. Added `extractFileReadFromParsedCmd` helper at `packages/server/src/server/agent/providers/codex-mcp-agent.ts:1907-1923` to detect `parsed_cmd.type === "read"` events. Updated `exec_command_begin` handler at `packages/server/src/server/agent/providers/codex-mcp-agent.ts:3222-3258` and `exec_command_end` handler at `packages/server/src/server/agent/providers/codex-mcp-agent.ts:3260-3386` to emit `read_file` timeline items with path input and content output when file reads are detected. Updated test prompt at `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:725` to allow cat-based file reads, and fixed test assertion at `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:797-802` to find completed read_file calls. RESULT: File reads now appear as `read_file` timeline items with `server: "file"`, `tool: "read_file"`, `kind: "read"`, file path in input, and file content in output. EVIDENCE: `npm run test --workspace=@paseo/server -- codex-mcp-agent.test.ts` (13 passed, 0 failed), `npm run typecheck --workspace=@paseo/server` (0 errors). - [ ] **E2E**: Test Codex MCP in the app using Playwright.