From d04fbb981c0b9ddf3b0affaa820bcf5480025543 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Wed, 24 Dec 2025 17:57:49 +0700 Subject: [PATCH] Add Codex MCP E2E test coverage --- .../agent/providers/codex-mcp-agent.test.ts | 368 +++++++++++++++--- plan.md | 153 ++------ 2 files changed, 346 insertions(+), 175 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 a2e2fec8f..7f174800a 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 @@ -5,6 +5,9 @@ import os from "node:os"; import path from "node:path"; import type { + AgentClient, + AgentPermissionRequest, + AgentSession, AgentSessionConfig, AgentStreamEvent, AgentTimelineItem, @@ -34,45 +37,11 @@ function useTempCodexSessionDir(): () => void { } async function loadCodexMcpAgentClient(): Promise<{ - new (): { - createSession: (config: AgentSessionConfig) => Promise<{ - run: (prompt: string) => Promise<{ finalText: string }>; - stream: (prompt: string) => AsyncGenerator; - streamHistory: () => AsyncGenerator; - describePersistence: () => { sessionId: string; metadata?: Record } | null; - close: () => Promise; - }>; - resumeSession: ( - handle: { sessionId: string; metadata?: Record }, - overrides?: Partial - ) => Promise<{ - run: (prompt: string) => Promise<{ finalText: string }>; - streamHistory: () => AsyncGenerator; - describePersistence: () => { sessionId: string; metadata?: Record } | null; - close: () => Promise; - }>; - }; + new (): AgentClient; }> { try { return (await import("./codex-mcp-agent.js")) as { - CodexMcpAgentClient: new () => { - createSession: (config: AgentSessionConfig) => Promise<{ - run: (prompt: string) => Promise<{ finalText: string }>; - stream: (prompt: string) => AsyncGenerator; - streamHistory: () => AsyncGenerator; - describePersistence: () => { sessionId: string; metadata?: Record } | null; - close: () => Promise; - }>; - resumeSession: ( - handle: { sessionId: string; metadata?: Record }, - overrides?: Partial - ) => Promise<{ - run: (prompt: string) => Promise<{ finalText: string }>; - streamHistory: () => AsyncGenerator; - describePersistence: () => { sessionId: string; metadata?: Record } | null; - close: () => Promise; - }>; - }; + CodexMcpAgentClient: new () => AgentClient; }; } catch (error) { throw new Error( @@ -105,7 +74,61 @@ function extractExitCode(output: unknown): number | undefined { return undefined; } +function commandTextFromInput(input: unknown): string | null { + if (!input || typeof input !== "object") { + return null; + } + const command = (input as { command?: unknown }).command; + if (typeof command === "string" && command.length > 0) { + return command; + } + if (Array.isArray(command)) { + const tokens = command.filter((value): value is string => typeof value === "string"); + if (tokens.length > 0) { + return tokens.join(" "); + } + } + return null; +} + +function isSleepCommandToolCall(item: ToolCallItem): boolean { + const display = typeof item.displayName === "string" ? item.displayName.toLowerCase() : ""; + if (display.includes("sleep 60")) { + return true; + } + const inputText = commandTextFromInput(item.input)?.toLowerCase() ?? ""; + return inputText.includes("sleep 60"); +} + describe("CodexMcpAgentClient (MCP integration)", () => { + test( + "responds with text", + async () => { + const cwd = tmpCwd(); + const restoreSessionDir = useTempCodexSessionDir(); + const { CodexMcpAgentClient } = await loadCodexMcpAgentClient(); + const client = new CodexMcpAgentClient(); + const config = { + provider: "codex-mcp", + cwd, + modeId: "full-access", + } as AgentSessionConfig; + + let session: AgentSession | null = null; + + try { + session = await client.createSession(config); + const response = await session.run("Reply READY and stop."); + expect(response.finalText.toLowerCase()).toContain("ready"); + } finally { + await session?.close(); + rmSync(cwd, { recursive: true, force: true }); + restoreSessionDir(); + } + }, + 180_000 + ); + test( "maps MCP stream events into timeline items with stable call ids", async () => { @@ -119,8 +142,7 @@ describe("CodexMcpAgentClient (MCP integration)", () => { modeId: "full-access", } as AgentSessionConfig; - let session: Awaited> | null = - null; + let session: AgentSession | null = null; const toolCalls: ToolCallItem[] = []; const rawCommandEvents: unknown[] = []; let sawAssistant = false; @@ -214,8 +236,7 @@ describe("CodexMcpAgentClient (MCP integration)", () => { modeId: "full-access", } as AgentSessionConfig; - let session: Awaited> | null = - null; + let session: AgentSession | null = null; let sawErrorTimeline = false; const errorEvents: AgentStreamEvent[] = []; @@ -266,10 +287,8 @@ describe("CodexMcpAgentClient (MCP integration)", () => { modeId: "full-access", } as AgentSessionConfig; - let session: Awaited> | null = - null; - let resumed: Awaited> | null = - null; + let session: AgentSession | null = null; + let resumed: AgentSession | null = null; const token = `ALPHA-${randomUUID()}`; try { @@ -333,4 +352,265 @@ describe("CodexMcpAgentClient (MCP integration)", () => { }, 180_000 ); + + test( + "reports runtime info with provider, session, model, and mode", + async () => { + const cwd = tmpCwd(); + const restoreSessionDir = useTempCodexSessionDir(); + const { CodexMcpAgentClient } = await loadCodexMcpAgentClient(); + const client = new CodexMcpAgentClient(); + const config = { + provider: "codex-mcp", + cwd, + modeId: "full-access", + model: "gpt-4.1", + } as AgentSessionConfig; + + let session: AgentSession | null = null; + + try { + session = await client.createSession(config); + const result = await session.run("Reply READY and stop."); + expect(result.finalText.toLowerCase()).toContain("ready"); + + const info = await session.getRuntimeInfo(); + expect(info.provider).toBe("codex-mcp"); + expect(typeof info.sessionId).toBe("string"); + expect((info.sessionId ?? "").length).toBeGreaterThan(0); + expect(info.modeId).toBe("full-access"); + expect(typeof info.model).toBe("string"); + expect((info.model ?? "").length).toBeGreaterThan(0); + } finally { + await session?.close(); + rmSync(cwd, { recursive: true, force: true }); + restoreSessionDir(); + } + }, + 180_000 + ); + + test( + "requests permission and resolves approval when allowed", + async () => { + const cwd = tmpCwd(); + const restoreSessionDir = useTempCodexSessionDir(); + const { CodexMcpAgentClient } = await loadCodexMcpAgentClient(); + const client = new CodexMcpAgentClient(); + const config = { + provider: "codex-mcp", + cwd, + modeId: "full-access", + approvalPolicy: "on-request", + } as AgentSessionConfig; + + let session: AgentSession | null = null; + let captured: AgentPermissionRequest | null = null; + let sawPermissionResolved = false; + const timelineItems: AgentTimelineItem[] = []; + + try { + session = await client.createSession(config); + + const prompt = [ + "Request approval to run the command `pwd`.", + "After approval, run it and reply DONE.", + ].join(" "); + + for await (const event of session.stream(prompt)) { + if (event.type === "permission_requested" && !captured) { + captured = event.request; + expect(session.getPendingPermissions().length).toBeGreaterThan(0); + await session.respondToPermission(captured.id, { behavior: "allow" }); + } + if ( + event.type === "permission_resolved" && + captured && + event.requestId === captured.id && + event.resolution.behavior === "allow" + ) { + sawPermissionResolved = true; + } + if (event.type === "timeline" && providerFromEvent(event) === "codex-mcp") { + timelineItems.push(event.item); + } + if (event.type === "turn_completed" || event.type === "turn_failed") { + break; + } + } + + expect(captured).not.toBeNull(); + expect(sawPermissionResolved).toBe(true); + expect(session.getPendingPermissions()).toHaveLength(0); + expect( + timelineItems.some( + (item) => + item.type === "tool_call" && + item.server === "permission" && + item.status === "granted" + ) + ).toBe(true); + expect( + timelineItems.some( + (item) => item.type === "tool_call" && item.server === "command" + ) + ).toBe(true); + } finally { + await session?.close(); + rmSync(cwd, { recursive: true, force: true }); + restoreSessionDir(); + } + }, + 180_000 + ); + + test( + "denies permission requests and reports resolution", + async () => { + const cwd = tmpCwd(); + const restoreSessionDir = useTempCodexSessionDir(); + const { CodexMcpAgentClient } = await loadCodexMcpAgentClient(); + const client = new CodexMcpAgentClient(); + const config = { + provider: "codex-mcp", + cwd, + modeId: "full-access", + approvalPolicy: "on-request", + } as AgentSessionConfig; + + let session: AgentSession | null = null; + let captured: AgentPermissionRequest | null = null; + let sawPermissionDenied = false; + const timelineItems: AgentTimelineItem[] = []; + + try { + session = await client.createSession(config); + + const prompt = [ + "Request approval to run the command `pwd`.", + "If approval is denied, acknowledge and stop.", + ].join(" "); + + for await (const event of session.stream(prompt)) { + if (event.type === "permission_requested" && !captured) { + captured = event.request; + await session.respondToPermission(captured.id, { + behavior: "deny", + message: "Not allowed.", + }); + } + if ( + event.type === "permission_resolved" && + captured && + event.requestId === captured.id && + event.resolution.behavior === "deny" + ) { + sawPermissionDenied = true; + } + if (event.type === "timeline" && providerFromEvent(event) === "codex-mcp") { + timelineItems.push(event.item); + } + if (event.type === "turn_completed" || event.type === "turn_failed") { + break; + } + } + + expect(captured).not.toBeNull(); + expect(sawPermissionDenied).toBe(true); + expect( + timelineItems.some( + (item) => + item.type === "tool_call" && + item.server === "permission" && + item.status === "denied" + ) + ).toBe(true); + expect( + timelineItems.some( + (item) => item.type === "tool_call" && item.server === "command" + ) + ).toBe(false); + } finally { + await session?.close(); + rmSync(cwd, { recursive: true, force: true }); + restoreSessionDir(); + } + }, + 180_000 + ); + + test( + "interrupts a long-running command via abort", + async () => { + const cwd = tmpCwd(); + const restoreSessionDir = useTempCodexSessionDir(); + const { CodexMcpAgentClient } = await loadCodexMcpAgentClient(); + const client = new CodexMcpAgentClient(); + const config = { + provider: "codex-mcp", + cwd, + modeId: "full-access", + approvalPolicy: "on-request", + } as AgentSessionConfig; + + let session: AgentSession | null = null; + let runStartedAt: number | null = null; + let durationMs = 0; + let sawSleepCommand = false; + let interruptIssued = false; + + try { + session = await client.createSession(config); + const prompt = [ + "Run the exact shell command `sleep 60` using your shell tool.", + "Do not run any additional commands or send a response until that command finishes.", + ].join(" "); + + runStartedAt = Date.now(); + const stream = session.stream(prompt); + + for await (const event of stream) { + if (event.type === "permission_requested" && session) { + await session.respondToPermission(event.request.id, { behavior: "allow" }); + } + + if ( + event.type === "timeline" && + providerFromEvent(event) === "codex-mcp" && + event.item.type === "tool_call" && + event.item.server === "command" && + isSleepCommandToolCall(event.item) + ) { + sawSleepCommand = true; + if (!interruptIssued) { + interruptIssued = true; + await session.interrupt(); + } + } + + if (event.type === "turn_completed" || event.type === "turn_failed") { + break; + } + } + + if (runStartedAt === null) { + throw new Error("Codex MCP run never started"); + } + durationMs = Date.now() - runStartedAt; + } finally { + if (durationMs === 0 && runStartedAt !== null) { + durationMs = Date.now() - runStartedAt; + } + await session?.close(); + rmSync(cwd, { recursive: true, force: true }); + restoreSessionDir(); + } + + expect(sawSleepCommand).toBe(true); + expect(interruptIssued).toBe(true); + expect(durationMs).toBeGreaterThan(0); + expect(durationMs).toBeLessThan(60_000); + }, + 90_000 + ); }); diff --git a/plan.md b/plan.md index 4b91d9b83..7e1ffbcd8 100644 --- a/plan.md +++ b/plan.md @@ -2,147 +2,38 @@ ## Context -We need a **new parallel Codex MCP provider** that lives side‑by‑side with the existing Codex SDK provider in `packages/server/src/server/agent/providers/codex-agent.ts`. The new provider should be implemented as `packages/server/src/server/agent/providers/codex-mcp-agent.ts` and selected via a new provider id (e.g. `codex-mcp`). It must use Codex MCP (elicitation-based permissions), not the SDK JSON stream. It must support the same capabilities (streaming events, permissions, session lifecycle, cancellation/abort, persistence metadata) without breaking the existing provider. Work must be TDD with full e2e verification against real Codex agents in multiple modes. **All tests must be end-to-end: no mocks, no fakes.** - -Reference implementation: `/Users/moboudra/dev/voice-dev/.tmp/happy-cli` (see `src/codex/` for MCP + elicitation flow). - -## Guiding Principles - -- TDD: write failing tests first, then implement -- All tests must be e2e (no mocks/fakes) -- Keep providers parallel (no breaking changes to existing Codex SDK provider) -- Verify with real Codex MCP server runs in multiple modes -- Cover edge cases explicitly (permission requests, denial, abort, resume, teardown) +Build a new Codex MCP provider side‑by‑side with the existing Codex SDK provider. The new provider lives in `packages/server/src/server/agent/providers/codex-mcp-agent.ts` and is selected via a new provider id (e.g. `codex-mcp`). All testing is **E2E only** (no mocks/fakes). Use `/Users/moboudra/dev/voice-dev/.tmp/happy-cli/src/codex/` as reference for MCP + elicitation. ## Tasks -- [x] **Plan**: MCP provider behavior matrix and interface mapping. +- [x] **Test (E2E)**: Create the full failing test file for Codex MCP provider. - - Enumerate required behaviors: start session, continue session, stream events, permission request/response, abort, close. - - Map each to MCP primitives: `codex` tool, `codex-reply` tool, `codex/event` notifications, elicitation handler. - - Define the new provider file path: `packages/server/src/server/agent/providers/codex-mcp-agent.ts` and exported classes. - - Define provider id (`codex-mcp`) and how it will be chosen vs existing `codex` provider. - - Identify gaps vs `codex-agent.ts` (timeline event types, runtime model info, persistence handles). - - Insert test tasks immediately after this task. - - Behavior matrix (Agent API → MCP): - - `createSession` → `codex` tool call (args: `prompt`, `approval-policy`, `base-instructions`, `config`, `cwd`, `include-plan-tool`, `model`, `profile`, `sandbox`). - - `stream` → `codex/event` notifications; map to `AgentStreamEvent` + timeline items (agent_message → assistant_message, reasoning → reasoning, command_execution/file_change/mcp_tool_call/web_search → tool_call, todo_list → todo, error → error). - - `continueSession` → `codex-reply` tool call (args: `sessionId`, `conversationId`, `prompt`). - - `respondToPermission` → MCP elicitation response; `ElicitRequestSchema` inputs become `permission_requested` + timeline tool_call (server=permission). - - `interrupt` → abort active tool call via `AbortController` (capture per turn). - - `close` → close MCP transport + clear pending permissions/events. - - Provider surface: `packages/server/src/server/agent/providers/codex-mcp-agent.ts` exporting `CodexMcpAgentClient` + `CodexMcpAgentSession`. - - Provider selection: add `codex-mcp` to `AgentProvider` union and `AGENT_PROVIDER_DEFINITIONS`; keep `codex` default and select via `AgentSessionConfig.provider`. - - Gaps vs `codex-agent.ts`: - - MCP event payload shape/fields (command outputs, status, call ids) need validation vs SDK `ThreadEvent`. - - Runtime model + mode may only appear in events/response; no rollout files for fallback. - - Persistence handle should include `sessionId` + `conversationId` (no `codexRolloutPath`/`codexSessionDir`). - - Elicitation appears to be exec-only; verify if patch/file change approvals surface separately. - - **Done (2025-12-24 17:18)**: Added MCP behavior matrix/mapping, provider selection notes, and new E2E test tasks for event mapping, persistence, and runtime info. + - Add a single e2e test file that covers: basic flow, event mapping parity, persistence/resume, runtime info, permissions (approve/deny), abort. + - Ensure it fails before implementation. + - **Done (2025-12-24 17:57)**: Expanded Codex MCP e2e tests to cover basic response, permissions allow/deny, and abort flow; updated typings and helpers. -- [x] **Test (E2E)**: MCP event payload mapping parity. +- [ ] **Implement**: `codex-mcp-agent.ts` provider so tests pass. - - Assert `codex/event` stream maps to assistant/reasoning/tool_call/error timeline items with stable call ids. - - Capture raw event shape for command output/exit code to decide mapping fallback. - - **Done (2025-12-24 17:27)**: Added MCP event mapping parity tests; vitest run fails because `codex-mcp-agent.ts` is not implemented yet. + - MCP stdio client + session lifecycle. + - `codex` / `codex-reply` calls. + - `codex/event` mapping to AgentStreamEvent. + - Elicitation → permission requests + responses. + - Abort/close handling. -- [x] **Test (E2E)**: MCP persistence + resume semantics. +- [ ] **Test (E2E)**: Run tests and add follow-up tasks based on results. - - Ensure `describePersistence()` includes sessionId + conversationId and resume uses `codex-reply`. - - Validate history/timeline hydration on resumed session. - - **Done (2025-12-24 17:31)**: Added persistence/resume E2E test and verified vitest fails until codex-mcp-agent is implemented. + - If failures: add fix tasks immediately after this task. + - If passes: add next audit/review task. -- [ ] **Test (E2E)**: MCP runtime info reporting. +- [ ] **Review**: Check implementation + edge cases. - - Ensure `getRuntimeInfo()` reports provider, sessionId, model (event/response fallback), and modeId. + - If issues: add fix tasks + re-review. -- [ ] **Plan**: E2E test plan (no mocks). +- [ ] **Test (E2E)**: Final verification (full scenario matrix). - - Define how tests start a real `codex mcp-server` (stdio transport) and verify teardown. - - Define e2e fixtures (temp cwd, temp codex session dir, prompt script). - - Define expected MCP events and permission elicitation behavior per mode. - - Insert failing test tasks and review task immediately after this task. + - read-only + on-request + - read-only + deny + - workspace-write + untrusted + - full-access -- [ ] **Test (E2E)**: Create failing e2e tests for MCP provider basic flow. - - - Start provider with Codex MCP server and call `createSession`. - - Send a prompt that writes a file and assert file exists after approval. - - Assert timeline events include tool call + agent message. - - Assert session metadata (sessionId/conversationId) is persisted. - - Ensure tests fail before implementation. - -- [ ] **Review**: Review failing e2e tests for coverage and correctness. - - - Check for gaps: permission denied path, abort path, missing event coverage. - - Add fix tasks if test coverage is insufficient. - -- [ ] **Test (E2E)**: Create failing e2e tests for permission handling. - - - read-only + approval on-request: expect elicitation request with command details. - - deny approval: ensure command does not run; verify agent response indicates refusal. - - approve once: ensure command runs and file is written. - - Ensure tests fail before implementation. - -- [ ] **Test (E2E)**: Create failing e2e tests for cancellation/abort. - - - Start a long-running command (sleep) and abort; verify turn ends and no output after abort. - - Verify provider can accept a new prompt after abort. - - Ensure tests fail before implementation. - -- [ ] **Implement**: `codex-mcp-agent.ts` provider skeleton. - - - Implement MCP stdio client (modelcontextprotocol `StdioClientTransport`). - - Implement start/continue session (`codex` / `codex-reply`). - - Implement event stream subscription (`codex/event` notifications). - - Map MCP events to existing AgentStreamEvent + timeline items. - -- [ ] **Implement**: Permission elicitation integration. - - - Register `ElicitRequestSchema` handler and emit permission requests into AgentStreamEvent. - - Maintain pending permission map (id → request) and implement `respondToPermission`. - - Ensure permission decisions flow back to MCP handler. - -- [ ] **Implement**: Runtime info + persistence metadata. - - - Capture runtime model from MCP events if available; fallback to configured model. - - Provide `describePersistence()` with sessionId/conversationId and any MCP metadata. - -- [ ] **Implement**: Abort/cancel and cleanup. - - - Abort in-flight turn via AbortController (or transport cancellation). - - Reset pending permissions + event processors after abort. - - Close: terminate MCP transport and child process reliably. - -- [ ] **Test (E2E)**: Run MCP provider tests (basic flow + permissions + abort). - - - Verify all previously failing e2e tests now pass. - - Add fix tasks + retest tasks immediately after if any fail. - -- [ ] **Implement**: Provider registration/selection wiring. - - - Add `codex-mcp` to provider manifest and any UI or config selection path. - - Ensure existing `codex` provider remains default/unchanged. - -- [ ] **Test (E2E)**: Full scenario matrix with real Codex MCP server. - - - read-only + on-request (elicitation should fire). - - read-only + deny (no file write). - - workspace-write + untrusted (permission prompt present). - - full-access (no prompt, command executes). - - Verify file output and agent completion in each scenario. - -- [ ] **Review**: Review implementation and edge case coverage. - - - Confirm event mapping parity vs `codex-agent.ts`. - - Confirm permission request/response lifecycle is robust. - - Add fix tasks and re-review if needed. - -- [ ] **Test (E2E)**: Full server test suite relevant to agent providers. - - - Run focused tests and document results. - - Add fix tasks if failures found. - -- [ ] **Plan**: Re-audit and add follow-up tasks. - - - Verify all required scenarios covered. - - Add any new tasks discovered during e2e runs. +- [ ] **Plan**: Re-audit and add any follow-up tasks.