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.