test: add codex mcp tool call coverage

This commit is contained in:
Mohamed Boudra
2025-12-24 23:44:56 +07:00
parent 6c48b50442
commit c8988b80d6
3 changed files with 277 additions and 41 deletions
+48
View File
@@ -0,0 +1,48 @@
# Codex MCP Tool Call Coverage E2E Failures
## Test Command
`npm run test --workspace=@paseo/server -- codex-mcp-agent.test.ts -t "captures tool call inputs/outputs"`
## Failing Assertions
Test: `CodexMcpAgentClient (MCP integration) > captures tool call inputs/outputs for commands, file changes, file reads, MCP tools, and web search`
- `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:532`
- `fileChangeCalls.some((item) => stringifyUnknown(item.output).includes("tool-create.txt"))`
- Output does not contain the file path; indicates `file_change` tool output is missing file metadata.
- `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:535`
- `const readCall = toolCalls.find((item) => item.tool === "read_file")`
- No `read_file` tool call captured.
- `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:536`
- `stringifyUnknown(readCall?.input)` did not include `tool-create.txt`.
- `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:537`
- `stringifyUnknown(readCall?.output)` did not include `beta`.
- `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:542`
- `const mcpCall = toolCalls.find((item) => item.server === "test" && item.tool === "echo")`
- No MCP tool call captured.
- `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:543`
- `stringifyUnknown(mcpCall?.input)` did not include `mcp-ok`.
- `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:544`
- `stringifyUnknown(mcpCall?.output)` did not include `mcp-ok`.
- `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:549`
- `const webSearchCall = toolCalls.find((item) => item.server === "web_search" && item.tool === "web_search")`
- No web search tool call captured.
- `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:550`
- `stringifyUnknown(webSearchCall?.input)` did not include `OpenAI Codex MCP`.
- `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:551`
- `webSearchCall?.output` was undefined; web search results not captured.
## Hypotheses
- The Codex MCP provider may not be emitting `tool_call` timeline items for `read_file`, MCP tool calls, and `web_search` in this scenario, or the model is not executing the requested tools despite explicit prompts.
- File change timeline output appears to omit file metadata (e.g., `{ files: [{ path, kind }] }`), so the apply_patch completion output is not surfaced in timeline items.
@@ -1,7 +1,7 @@
import { describe, expect, test } from "vitest";
import { execFileSync } from "node:child_process";
import { randomUUID } from "node:crypto";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
import os from "node:os";
import path from "node:path";
@@ -150,6 +150,35 @@ function commandTextFromInput(input: unknown): string | null {
return null;
}
function commandOutputText(output: unknown): string | null {
if (typeof output === "string") {
return output;
}
if (!output || typeof output !== "object") {
return null;
}
const record = output as Record<string, unknown>;
const outputText = record.output;
if (typeof outputText === "string") {
return outputText;
}
if (typeof record.stdout === "string") {
return record.stdout;
}
if (typeof record.stderr === "string") {
return record.stderr;
}
return null;
}
function stringifyUnknown(value: unknown): string {
try {
return JSON.stringify(value) ?? "";
} catch {
return "";
}
}
function isSleepCommandToolCall(item: ToolCallItem): boolean {
const display = typeof item.displayName === "string" ? item.displayName.toLowerCase() : "";
if (display.includes("sleep 60")) {
@@ -258,9 +287,13 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
expect((toolCall.callId ?? "").trim().length).toBeGreaterThan(0);
}
const commandToolCall = toolCalls.find(
(item) => item.server === "command"
);
const commandToolCall = toolCalls
.slice()
.reverse()
.find(
(item) =>
item.server === "command" && item.status !== "running"
);
expect(commandToolCall).toBeTruthy();
const exitCode = extractExitCode(commandToolCall?.output);
@@ -393,6 +426,168 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
180_000
);
test(
"captures tool call inputs/outputs for commands, file changes, file reads, MCP tools, and web search",
async () => {
const cwd = tmpCwd();
const restoreSessionDir = useTempCodexSessionDir();
const mcpServerScript = writeTestMcpServerScript(cwd);
const { CodexMcpAgentClient } = await loadCodexMcpAgentClient();
const client = new CodexMcpAgentClient();
const config = {
provider: "codex-mcp",
cwd,
modeId: "auto",
approvalPolicy: "on-request",
networkAccess: true,
extra: {
codex: {
mcp_servers: {
test: {
command: process.execPath,
args: [mcpServerScript],
},
},
},
},
} as AgentSessionConfig;
let session: AgentSession | null = null;
let permissionRequest: AgentPermissionRequest | null = null;
let permissionResolved = false;
const toolCalls: ToolCallItem[] = [];
try {
session = await client.createSession(config);
const prompt = [
"1. Run the command `bash -lc \"printf 'stdout-marker'\"` using your shell tool.",
"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.",
"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.",
"9. After all tools finish, reply DONE and stop.",
].join("\n");
for await (const event of session.stream(prompt)) {
if (event.type === "permission_requested" && !permissionRequest) {
permissionRequest = event.request;
await session.respondToPermission(permissionRequest.id, { behavior: "allow" });
}
if (
event.type === "permission_resolved" &&
permissionRequest &&
event.requestId === permissionRequest.id &&
event.resolution.behavior === "allow"
) {
permissionResolved = true;
}
if (event.type === "timeline" && providerFromEvent(event) === "codex-mcp") {
if (event.item.type === "tool_call" && event.item.server !== "permission") {
toolCalls.push(event.item);
}
}
if (event.type === "turn_completed" || event.type === "turn_failed") {
break;
}
}
expect.soft(permissionRequest).not.toBeNull();
expect.soft(permissionResolved).toBe(true);
const commandCalls = toolCalls.filter(
(item) => item.server === "command" && item.status === "completed"
);
expect.soft(commandCalls.length).toBeGreaterThanOrEqual(2);
const stdoutCall = commandCalls.find((item) =>
(commandOutputText(item.output) ?? "").includes("stdout-marker")
);
const stderrCall = commandCalls.find((item) =>
(commandOutputText(item.output) ?? "").includes("stderr-marker")
);
expect.soft(stdoutCall).toBeTruthy();
expect.soft(stderrCall).toBeTruthy();
expect.soft(extractExitCode(stdoutCall?.output)).toBe(0);
expect.soft(extractExitCode(stderrCall?.output)).toBe(0);
const fileChangeCalls = toolCalls.filter(
(item) => item.server === "file_change" && item.tool === "apply_patch"
);
expect.soft(fileChangeCalls.length).toBeGreaterThanOrEqual(2);
expect.soft(
fileChangeCalls.some((item) => stringifyUnknown(item.input).includes("tool-create.txt"))
).toBe(true);
expect.soft(
fileChangeCalls.some((item) => stringifyUnknown(item.input).includes("alpha"))
).toBe(true);
expect.soft(
fileChangeCalls.some((item) => stringifyUnknown(item.input).includes("beta"))
).toBe(true);
expect.soft(
fileChangeCalls.some((item) => stringifyUnknown(item.output).includes("tool-create.txt"))
).toBe(true);
const readCall = toolCalls.find((item) => item.tool === "read_file");
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"
);
expect.soft(mcpCall).toBeTruthy();
expect.soft(stringifyUnknown(mcpCall?.input)).toContain("mcp-ok");
expect.soft(stringifyUnknown(mcpCall?.output)).toContain("mcp-ok");
const webSearchCall = toolCalls.find(
(item) => item.server === "web_search" && item.tool === "web_search"
);
expect.soft(webSearchCall).toBeTruthy();
expect.soft(stringifyUnknown(webSearchCall?.input)).toContain("OpenAI Codex MCP");
expect.soft(webSearchCall?.output).toBeTruthy();
const callIdStatuses = new Map<string, Set<string>>();
for (const toolCall of toolCalls) {
if (!toolCall.callId) {
continue;
}
if (!callIdStatuses.has(toolCall.callId)) {
callIdStatuses.set(toolCall.callId, new Set());
}
if (typeof toolCall.status === "string") {
callIdStatuses.get(toolCall.callId)!.add(toolCall.status);
}
}
const commandCallIds = toolCalls
.filter((item) => item.server === "command" && item.callId)
.map((item) => item.callId as string);
const fileChangeCallIds = toolCalls
.filter((item) => item.server === "file_change" && item.callId)
.map((item) => item.callId as string);
const hasCommandLifecycle = commandCallIds.some((callId) => {
const statuses = callIdStatuses.get(callId);
return statuses?.has("running") && statuses?.has("completed");
});
const hasFileChangeLifecycle = fileChangeCallIds.some((callId) => {
const statuses = callIdStatuses.get(callId);
return statuses?.has("running") && statuses?.has("completed");
});
expect.soft(hasCommandLifecycle).toBe(true);
expect.soft(hasFileChangeLifecycle).toBe(true);
} finally {
await session?.close();
rmSync(cwd, { recursive: true, force: true });
restoreSessionDir();
}
},
240_000
);
test(
"emits an error timeline item for failed MCP turns",
async () => {
@@ -569,9 +764,10 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
const config = {
provider: "codex-mcp",
cwd,
modeId: "read-only",
modeId: "auto",
approvalPolicy: "on-request",
} as AgentSessionConfig;
const filePath = path.join(cwd, "permission.txt");
let session: AgentSession | null = null;
let captured: AgentPermissionRequest | null = null;
@@ -624,6 +820,8 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
(item) => item.type === "tool_call" && item.server === "command"
)
).toBe(true);
expect(existsSync(filePath)).toBe(true);
expect(readFileSync(filePath, "utf8")).toContain("ok");
} finally {
await session?.close();
rmSync(cwd, { recursive: true, force: true });
@@ -700,9 +898,10 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
const config = {
provider: "codex-mcp",
cwd,
modeId: "read-only",
modeId: "auto",
approvalPolicy: "on-request",
} as AgentSessionConfig;
const filePath = path.join(cwd, "permission.txt");
let session: AgentSession | null = null;
let captured: AgentPermissionRequest | null = null;
@@ -751,6 +950,7 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
item.status === "denied"
)
).toBe(true);
expect(existsSync(filePath)).toBe(false);
} finally {
await session?.close();
rmSync(cwd, { recursive: true, force: true });
@@ -770,9 +970,10 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
const config = {
provider: "codex-mcp",
cwd,
modeId: "read-only",
modeId: "auto",
approvalPolicy: "on-request",
} as AgentSessionConfig;
const filePath = path.join(cwd, "permission.txt");
let session: AgentSession | null = null;
let captured: AgentPermissionRequest | null = null;
@@ -832,6 +1033,7 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
).toBe(true);
expect(sawTurnFailed).toBe(true);
expect(failureMessage ?? "").toMatch(/aborted|interrupted/i);
expect(existsSync(filePath)).toBe(false);
} finally {
await session?.close();
rmSync(cwd, { recursive: true, force: true });
@@ -860,7 +1062,6 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
let durationMs = 0;
let sawSleepCommand = false;
let interruptIssued = false;
let interruptTimer: ReturnType<typeof setTimeout> | null = null;
try {
session = await client.createSession(config);
@@ -871,12 +1072,6 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
runStartedAt = Date.now();
const stream = session.stream(prompt);
interruptTimer = setTimeout(() => {
if (!interruptIssued) {
interruptIssued = true;
void session?.interrupt();
}
}, 10_000);
for await (const event of stream) {
if (event.type === "permission_requested" && session) {
@@ -893,10 +1088,6 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
sawSleepCommand = true;
if (!interruptIssued) {
interruptIssued = true;
if (interruptTimer) {
clearTimeout(interruptTimer);
interruptTimer = null;
}
await session.interrupt();
}
}
@@ -914,10 +1105,6 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
if (durationMs === 0 && runStartedAt !== null) {
durationMs = Date.now() - runStartedAt;
}
if (interruptTimer) {
clearTimeout(interruptTimer);
interruptTimer = null;
}
await session?.close();
rmSync(cwd, { recursive: true, force: true });
restoreSessionDir();
@@ -950,9 +1137,7 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
let sawCommand = false;
let interruptAt: number | null = null;
let stoppedAt: number | null = null;
let interruptReason: "tool-call" | "timeout" | null = null;
const marker = `codex-mcp-abort-${randomUUID()}`;
let interruptTimer: ReturnType<typeof setTimeout> | null = null;
try {
session = await client.createSession(config);
@@ -962,13 +1147,6 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
].join(" ");
const stream = session.stream(prompt);
interruptTimer = setTimeout(() => {
if (!interruptAt) {
interruptReason = "timeout";
interruptAt = Date.now();
void session?.interrupt();
}
}, 15_000);
for await (const event of stream) {
if (event.type === "permission_requested" && session) {
@@ -985,12 +1163,7 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
if (commandText.includes(marker)) {
sawCommand = true;
if (!interruptAt) {
interruptReason = "tool-call";
interruptAt = Date.now();
if (interruptTimer) {
clearTimeout(interruptTimer);
interruptTimer = null;
}
await session.interrupt();
}
}
@@ -1011,7 +1184,6 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
const latencyMs = stoppedAt - interruptAt;
expect(sawCommand).toBe(true);
expect(interruptReason).toBe("tool-call");
expect(latencyMs).toBeGreaterThanOrEqual(0);
expect(latencyMs).toBeLessThan(1_000);
@@ -1025,9 +1197,6 @@ describe("CodexMcpAgentClient (MCP integration)", () => {
const followup = await followupSession.run("Reply OK and stop.");
expect(followup.finalText.toLowerCase()).toContain("ok");
} finally {
if (interruptTimer) {
clearTimeout(interruptTimer);
}
await session?.close();
await followupSession?.close();
rmSync(cwd, { recursive: true, force: true });
+20 -1
View File
@@ -386,7 +386,7 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov
- Mode switching
- **Done (2025-12-24 23:35)**: WHAT: authored parity audit in `REPORT-codex-mcp-claude-parity.md:1` covering capabilities, gaps, and evidence lines; updated `plan.md:200` status. RESULT: Codex MCP vs Claude feature parity checklist documented with concrete gaps (modes, persistence, MCP servers, permissions). EVIDENCE: `REPORT-codex-mcp-claude-parity.md` contents summarizing code references.
- [ ] **Test (E2E)**: Comprehensive tool call coverage for Codex MCP provider.
- [x] **Test (E2E)**: Comprehensive tool call coverage for Codex MCP provider.
- All tool call types must be tested and emit proper timeline events:
- **Command runs**: `shell_command` / `exec_command` → exit code, stdout, stderr
@@ -400,6 +400,25 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov
- Tool `input` and `output` are captured
- `callId` is consistent across events
- Permission flow triggers when expected (for unsafe operations)
- **Done (2025-12-24 23:44)**: WHAT: added tool-call coverage helpers and E2E test in `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:153` and `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:430`, plus documented failures in `REPORT-codex-mcp-tool-call-coverage.md:1`. RESULT: test fails because file_change outputs omit file metadata and no read_file/mcp_tool_call/web_search timeline items or outputs are captured. EVIDENCE: `npm run test --workspace=@paseo/server -- codex-mcp-agent.test.ts -t "captures tool call inputs/outputs"` (fails with missing file_change output at `codex-mcp-agent.test.ts:532`, missing read_file at `codex-mcp-agent.test.ts:535`, missing mcp tool call at `codex-mcp-agent.test.ts:542`, missing web_search at `codex-mcp-agent.test.ts:549`).
- [ ] **Fix**: Codex MCP apply_patch tool_call output should include file metadata (path/kind) and before/after content.
- Ensure `patch_apply_end` timeline output includes file path info and the patch content needed for before/after validation.
- Align with `REPORT-codex-mcp-tool-call-coverage.md`.
- [ ] **Fix**: Codex MCP should emit read_file tool_call timeline items with input/output content.
- Map Codex MCP read_file events into timeline items (tool name, file path, content snippet).
- Ensure the E2E coverage test can find `tool: "read_file"` with content.
- [ ] **Fix**: Codex MCP should emit external MCP tool calls (mcp_tool_call) with input/output in timeline items.
- Ensure MCP server tool calls surface `server`, `tool`, `input`, and `output` fields.
- [ ] **Fix**: Codex MCP web_search timeline items should include query input and results output.
- Ensure web_search tool calls emit a timeline item with query and results.
- [ ] **CRITICAL REFACTOR**: Eliminate ALL type casting and defensive coding in Codex MCP provider.