From 007b41e455957724e485155529b6f92ff5281204 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Thu, 25 Dec 2025 16:32:34 +0700 Subject: [PATCH] Fix Codex MCP agent-control access - pass MCP servers to Codex tool call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WHAT: - Fixed `buildCodexMcpConfig()` to include MCP servers in the Codex tool call - Added `CodexMcpServerConfig` and `CodexConfigPayload` types for proper typing - Added `managedAgentId` parameter to append caller agent ID to agent-control URL - Built MCP servers config including: 1. `agent-control` HTTP MCP with URL and `http_headers` 2. `playwright` STDIO MCP server 3. User-provided MCP servers from `config.mcpServers` - Added `managedAgentId` property to `CodexMcpAgentSession` class - Updated `setManagedAgentId()` to store the ID - Updated all call sites of `buildCodexMcpConfig()` to pass managed agent ID ROOT CAUSE: Claude provider builds MCP servers config and passes to Claude SDK. Codex MCP provider only passed `config.extra.codex` - completely ignoring `config.agentControlMcp` and `config.mcpServers`. Codex CLI expects MCP servers in `config.mcp_servers` field with `http_headers` (not `headers`) for HTTP servers. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- dummy.txt | 1 + .../server/scripts/test-codex-mcp-agents.ts | 75 ++++++++++++++++ .../scripts/test-codex-mcp-basic-auth.ts | 78 +++++++++++++++++ .../server/scripts/test-codex-mcp-bearer.ts | 75 ++++++++++++++++ .../server/scripts/test-codex-mcp-http.ts | 68 +++++++++++++++ .../server/scripts/test-codex-mcp-servers.ts | 84 ++++++++++++++++++ .../server/agent/providers/codex-mcp-agent.ts | 85 ++++++++++++++++--- plan.md | 7 ++ 8 files changed, 462 insertions(+), 11 deletions(-) create mode 100644 dummy.txt create mode 100644 packages/server/scripts/test-codex-mcp-agents.ts create mode 100644 packages/server/scripts/test-codex-mcp-basic-auth.ts create mode 100644 packages/server/scripts/test-codex-mcp-bearer.ts create mode 100644 packages/server/scripts/test-codex-mcp-http.ts create mode 100644 packages/server/scripts/test-codex-mcp-servers.ts diff --git a/dummy.txt b/dummy.txt new file mode 100644 index 000000000..ce0136250 --- /dev/null +++ b/dummy.txt @@ -0,0 +1 @@ +hello diff --git a/packages/server/scripts/test-codex-mcp-agents.ts b/packages/server/scripts/test-codex-mcp-agents.ts new file mode 100644 index 000000000..7e267da3e --- /dev/null +++ b/packages/server/scripts/test-codex-mcp-agents.ts @@ -0,0 +1,75 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { z } from "zod"; + +async function main() { + // The bearer token is the base64-encoded credentials + const bearerToken = Buffer.from("mo:bo").toString("base64"); + + const transport = new StdioClientTransport({ + command: "codex", + args: ["mcp-server"], + env: { + ...process.env, + // Set the bearer token env var + PASEO_AGENT_CONTROL_TOKEN: bearerToken + }, + }); + + const client = new Client( + { name: "test-client", version: "1.0.0" }, + { capabilities: { elicitation: {} } } + ); + + // Listen for events + client.setNotificationHandler( + z.object({ + method: z.literal("codex/event"), + params: z.object({ msg: z.any() }), + }).passthrough(), + (data) => { + const event = (data.params as { msg: unknown }).msg as { type?: string }; + if (event.type === "mcp_startup_update" || event.type === "mcp_startup_complete") { + console.log("MCP Event:", JSON.stringify(event, null, 2)); + } + } + ); + + await client.connect(transport); + + // Use correct route (/mcp/agents) and bearer token env var + console.log("\n=== Testing HTTP MCP server with correct route and bearer token ===\n"); + + try { + const result = await client.callTool({ + name: "codex", + arguments: { + prompt: "List all the MCP tools you have available. Just list them, don't use any.", + sandbox: "danger-full-access", + "approval-policy": "never", + config: { + mcp_servers: { + "agent-control": { + url: "http://localhost:6767/mcp/agents", + bearer_token_env_var: "PASEO_AGENT_CONTROL_TOKEN" + } + } + } + } + }, undefined, { timeout: 60000 }); + + console.log("\n=== RESULT ==="); + const content = (result as { content: { text?: string }[] }).content; + for (const item of content) { + if (item.text) { + console.log(item.text); + } + } + } catch (error) { + console.error("Error:", error); + } + + await client.close(); +} + +main().catch(console.error); diff --git a/packages/server/scripts/test-codex-mcp-basic-auth.ts b/packages/server/scripts/test-codex-mcp-basic-auth.ts new file mode 100644 index 000000000..5311c6516 --- /dev/null +++ b/packages/server/scripts/test-codex-mcp-basic-auth.ts @@ -0,0 +1,78 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { z } from "zod"; + +async function main() { + const transport = new StdioClientTransport({ + command: "codex", + args: ["mcp-server"], + env: { + ...process.env, + // Try passing credentials via env var that Codex might read + AGENT_CONTROL_AUTH: "mo:bo" + }, + }); + + const client = new Client( + { name: "test-client", version: "1.0.0" }, + { capabilities: { elicitation: {} } } + ); + + // Listen for events + client.setNotificationHandler( + z.object({ + method: z.literal("codex/event"), + params: z.object({ msg: z.any() }), + }).passthrough(), + (data) => { + const event = (data.params as { msg: unknown }).msg as { type?: string }; + if (event.type === "mcp_startup_update" || event.type === "mcp_startup_complete") { + console.log("MCP Event:", JSON.stringify(event, null, 2)); + } + } + ); + + await client.connect(transport); + + // Try passing MCP server config via the config parameter with headers + console.log("\n=== Testing HTTP MCP server with basic auth in config ===\n"); + + try { + // Create base64 encoded credentials + const credentials = Buffer.from("mo:bo").toString("base64"); + + const result = await client.callTool({ + name: "codex", + arguments: { + prompt: "List all the MCP tools you have available. Just list them, don't use any.", + sandbox: "danger-full-access", + "approval-policy": "never", + config: { + mcp_servers: { + "agent-control": { + url: "http://localhost:6767/mcp/agent-control", + // Try various ways to pass auth + headers: { + "Authorization": `Basic ${credentials}` + } + } + } + } + } + }, undefined, { timeout: 60000 }); + + console.log("\n=== RESULT ==="); + const content = (result as { content: { text?: string }[] }).content; + for (const item of content) { + if (item.text) { + console.log(item.text); + } + } + } catch (error) { + console.error("Error:", error); + } + + await client.close(); +} + +main().catch(console.error); diff --git a/packages/server/scripts/test-codex-mcp-bearer.ts b/packages/server/scripts/test-codex-mcp-bearer.ts new file mode 100644 index 000000000..efbec1229 --- /dev/null +++ b/packages/server/scripts/test-codex-mcp-bearer.ts @@ -0,0 +1,75 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { z } from "zod"; + +async function main() { + // Create base64 encoded credentials + const credentials = Buffer.from("mo:bo").toString("base64"); + + const transport = new StdioClientTransport({ + command: "codex", + args: ["mcp-server"], + env: { + ...process.env, + // Set the bearer token env var that we will reference + AGENT_CONTROL_TOKEN: `Basic ${credentials}` + }, + }); + + const client = new Client( + { name: "test-client", version: "1.0.0" }, + { capabilities: { elicitation: {} } } + ); + + // Listen for events + client.setNotificationHandler( + z.object({ + method: z.literal("codex/event"), + params: z.object({ msg: z.any() }), + }).passthrough(), + (data) => { + const event = (data.params as { msg: unknown }).msg as { type?: string }; + if (event.type === "mcp_startup_update" || event.type === "mcp_startup_complete") { + console.log("MCP Event:", JSON.stringify(event, null, 2)); + } + } + ); + + await client.connect(transport); + + // Try passing MCP server config with bearer_token_env_var + console.log("\n=== Testing HTTP MCP server with bearer_token_env_var ===\n"); + + try { + const result = await client.callTool({ + name: "codex", + arguments: { + prompt: "List all the MCP tools you have available. Just list them, don't use any.", + sandbox: "danger-full-access", + "approval-policy": "never", + config: { + mcp_servers: { + "agent-control": { + url: "http://localhost:6767/mcp/agent-control", + bearer_token_env_var: "AGENT_CONTROL_TOKEN" + } + } + } + } + }, undefined, { timeout: 60000 }); + + console.log("\n=== RESULT ==="); + const content = (result as { content: { text?: string }[] }).content; + for (const item of content) { + if (item.text) { + console.log(item.text); + } + } + } catch (error) { + console.error("Error:", error); + } + + await client.close(); +} + +main().catch(console.error); diff --git a/packages/server/scripts/test-codex-mcp-http.ts b/packages/server/scripts/test-codex-mcp-http.ts new file mode 100644 index 000000000..7ed0350d4 --- /dev/null +++ b/packages/server/scripts/test-codex-mcp-http.ts @@ -0,0 +1,68 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { z } from "zod"; + +async function main() { + const transport = new StdioClientTransport({ + command: "codex", + args: ["mcp-server"], + env: { ...process.env }, + }); + + const client = new Client( + { name: "test-client", version: "1.0.0" }, + { capabilities: { elicitation: {} } } + ); + + // Listen for events + client.setNotificationHandler( + z.object({ + method: z.literal("codex/event"), + params: z.object({ msg: z.any() }), + }).passthrough(), + (data) => { + const event = (data.params as { msg: unknown }).msg as { type?: string }; + if (event.type === "mcp_startup_update" || event.type === "mcp_startup_complete") { + console.log("MCP Event:", JSON.stringify(event, null, 2)); + } + } + ); + + await client.connect(transport); + + // Try passing MCP server config via the config parameter with HTTP URL + console.log("\n=== Testing HTTP MCP server config (agent-control style) ===\n"); + + try { + const result = await client.callTool({ + name: "codex", + arguments: { + prompt: "List all the MCP tools you have available. Just list them, don't use any.", + sandbox: "danger-full-access", + "approval-policy": "never", + config: { + mcp_servers: { + "agent-control": { + url: "http://localhost:6767/mcp/agent-control", + type: "http" + } + } + } + } + }, undefined, { timeout: 60000 }); + + console.log("\n=== RESULT ==="); + const content = (result as { content: { text?: string }[] }).content; + for (const item of content) { + if (item.text) { + console.log(item.text); + } + } + } catch (error) { + console.error("Error:", error); + } + + await client.close(); +} + +main().catch(console.error); diff --git a/packages/server/scripts/test-codex-mcp-servers.ts b/packages/server/scripts/test-codex-mcp-servers.ts new file mode 100644 index 000000000..c20c1fbc4 --- /dev/null +++ b/packages/server/scripts/test-codex-mcp-servers.ts @@ -0,0 +1,84 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { z } from "zod"; + +async function main() { + const transport = new StdioClientTransport({ + command: "codex", + args: ["mcp-server"], + env: { ...process.env }, + }); + + const client = new Client( + { name: "test-client", version: "1.0.0" }, + { capabilities: { elicitation: {} } } + ); + + // Listen for events + client.setNotificationHandler( + z.object({ + method: z.literal("codex/event"), + params: z.object({ msg: z.any() }), + }).passthrough(), + (data) => { + const event = (data.params as { msg: unknown }).msg as { + type?: string; + data?: { text?: string; item?: { type?: string } }; + text?: string; + item?: { type?: string }; + }; + if (event.type === "turn.started") { + console.log("\n=== TURN STARTED ==="); + } else if (event.type === "agent_message") { + console.log("Agent:", event.data?.text || event.text); + } else if (event.type === "mcp_tool_call") { + console.log("MCP Tool Call:", JSON.stringify(event.data)); + } else if (event.type === "thread.item") { + const item = event.data?.item || event.item; + if (item?.type === "mcp_tool_call") { + console.log("MCP Tool from thread:", JSON.stringify(item)); + } + } else { + console.log("Event:", event.type); + } + } + ); + + await client.connect(transport); + + // Try passing MCP server config via the config parameter + console.log("\n=== Testing dynamic MCP server config ===\n"); + + try { + const result = await client.callTool({ + name: "codex", + arguments: { + prompt: "List all the MCP tools you have available. Just list them, don't use any.", + sandbox: "danger-full-access", + "approval-policy": "never", + config: { + mcp_servers: { + "test-server": { + command: "npx", + args: ["-y", "mcp-server-time"] + } + } + } + } + }, undefined, { timeout: 60000 }); + + console.log("\n=== RESULT ==="); + const content = (result as { content: { text?: string }[] }).content; + for (const item of content) { + if (item.text) { + console.log(item.text); + } + } + } catch (error) { + console.error("Error:", error); + } + + await client.close(); +} + +main().catch(console.error); 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 4d518aaba..96d443635 100644 --- a/packages/server/src/server/agent/providers/codex-mcp-agent.ts +++ b/packages/server/src/server/agent/providers/codex-mcp-agent.ts @@ -2321,16 +2321,30 @@ function getCodexMcpCommand(): string { } } +type CodexMcpServerConfig = { + url?: string; + http_headers?: Record; // Static HTTP headers for HTTP servers + command?: string; + args?: string[]; + env?: Record; +}; + +type CodexConfigPayload = { + mcp_servers?: Record; + [key: string]: unknown; +}; + function buildCodexMcpConfig( config: AgentSessionConfig, prompt: string, - modeId: string + modeId: string, + managedAgentId?: string ): { prompt: string; cwd?: string; "approval-policy": string; sandbox: string; - config?: unknown; + config?: CodexConfigPayload; model?: string; } { const preset = @@ -2343,22 +2357,71 @@ function buildCodexMcpConfig( : preset.approvalPolicy; const sandbox = config.sandboxMode !== undefined ? config.sandboxMode : preset.sandbox; - const extra = config.extra ? config.extra.codex : undefined; + + // Build the config payload with MCP servers + const innerConfig: CodexConfigPayload = {}; + + // Add extra codex config if provided + if (config.extra?.codex) { + Object.assign(innerConfig, config.extra.codex); + } + + // Build MCP servers configuration + const mcpServers: Record = {}; + + // Add agent-control MCP server (HTTP-based) if configured + if (config.agentControlMcp) { + let agentControlUrl = config.agentControlMcp.url; + // Append caller agent ID to URL if this is a managed agent + if (managedAgentId) { + const separator = agentControlUrl.includes("?") ? "&" : "?"; + agentControlUrl = `${agentControlUrl}${separator}callerAgentId=${encodeURIComponent(managedAgentId)}`; + } + mcpServers["agent-control"] = { + url: agentControlUrl, + ...(config.agentControlMcp.headers ? { http_headers: config.agentControlMcp.headers } : {}), + }; + } + + // Add playwright MCP server (same as Claude provider) + mcpServers["playwright"] = { + command: "npx", + args: ["@playwright/mcp", "--headless", "--isolated"], + }; + + // Merge user-provided MCP servers (they take precedence) + if (config.mcpServers) { + for (const [name, serverConfig] of Object.entries(config.mcpServers)) { + if (typeof serverConfig === "object" && serverConfig !== null) { + mcpServers[name] = serverConfig as CodexMcpServerConfig; + } + } + } + + // Only add mcp_servers to config if there are any + if (Object.keys(mcpServers).length > 0) { + innerConfig.mcp_servers = mcpServers; + } const configPayload: { prompt: string; cwd?: string; "approval-policy": string; sandbox: string; - config?: unknown; + config?: CodexConfigPayload; model?: string; } = { prompt, cwd: config.cwd, "approval-policy": approvalPolicy, sandbox, - config: extra, }; + + // Only include config if it has content + if (Object.keys(innerConfig).length > 0) { + configPayload.config = innerConfig; + } + if (typeof config.model === "string" && config.model.length > 0) { configPayload.model = config.model; } @@ -2450,6 +2513,7 @@ class CodexMcpAgentSession implements AgentSession { private turnState: TurnState | null = null; private pendingPatchChanges = new Map(); private patchChangesByCallId = new Map(); + private managedAgentId: string | null = null; constructor(config: CodexMcpAgentConfig, resumeHandle?: AgentPersistenceHandle) { this.config = config; @@ -2840,9 +2904,8 @@ class CodexMcpAgentSession implements AgentSession { this.conversationId = null; } - // eslint-disable-next-line @typescript-eslint/no-unused-vars - setManagedAgentId(_agentId: string): void { - // Codex MCP sessions do not currently use the agent-control MCP channel. + setManagedAgentId(agentId: string): void { + this.managedAgentId = agentId; } private async forwardPrompt( @@ -2858,7 +2921,7 @@ class CodexMcpAgentSession implements AgentSession { let response: unknown; try { if (!this.sessionId) { - const config = buildCodexMcpConfig(this.config, prompt, this.currentMode); + const config = buildCodexMcpConfig(this.config, prompt, this.currentMode, this.managedAgentId ?? undefined); const attempt = async (arguments_: CodexToolArguments) => this.client.callTool( { name: "codex", arguments: arguments_ }, @@ -2896,7 +2959,7 @@ class CodexMcpAgentSession implements AgentSession { ); if (isMissingConversationIdResponse(response)) { const replayPrompt = this.buildResumePrompt(prompt); - const config = buildCodexMcpConfig(this.config, replayPrompt, this.currentMode); + const config = buildCodexMcpConfig(this.config, replayPrompt, this.currentMode, this.managedAgentId ?? undefined); const attempt = async (arguments_: CodexToolArguments) => this.client.callTool( { name: "codex", arguments: arguments_ }, @@ -2919,7 +2982,7 @@ class CodexMcpAgentSession implements AgentSession { } catch (error) { if (isMissingConversationIdError(error)) { const replayPrompt = this.buildResumePrompt(prompt); - const config = buildCodexMcpConfig(this.config, replayPrompt, this.currentMode); + const config = buildCodexMcpConfig(this.config, replayPrompt, this.currentMode, this.managedAgentId ?? undefined); const attempt = async (arguments_: CodexToolArguments) => this.client.callTool( { name: "codex", arguments: arguments_ }, diff --git a/plan.md b/plan.md index 44a091e32..a136dd53c 100644 --- a/plan.md +++ b/plan.md @@ -899,3 +899,10 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov - Test passes after fix - No duplicate messages in UI - **Done (2025-12-25 15:58)**: WHAT: (1) Added test "emits exactly one user_message and one assistant_message per turn" in `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:404-483` that verifies exactly 1 user_message and 1 assistant_message per turn. (2) Fixed `threadItemToTimeline` in `packages/server/src/server/agent/providers/codex-mcp-agent.ts:3621-3639` to skip `user_message` items (since we emit directly in stream()) and only emit `agent_message`/`reasoning` on `item.completed` (not on item.started/item.updated). (3) Fixed `handleMcpEvent` in `packages/server/src/server/agent/providers/codex-mcp-agent.ts:3194-3201` to skip `agent_message`, `agent_reasoning`, and `agent_reasoning_delta` events since they're now handled via `item.completed` path to avoid duplicates. ROOT CAUSE: Codex MCP sends BOTH direct events (agent_message, agent_reasoning_delta) AND item events (item.started, item.updated, item.completed) for the same message. User messages were emitted once by us in stream() AND again 3 times from Codex MCP's item.started/updated/completed events. Agent messages were emitted from both the agent_message direct event AND the item.completed event. RESULT: All 14 Codex MCP tests pass; typecheck passes. EVIDENCE: `npm run test --workspace=@paseo/server -- codex-mcp-agent.test.ts` (14 passed, 0 failed). + - **⚠️ INCOMPLETE**: Bug still exists in production. User still sees 2x "hello" messages. Test is useless. + +- [x] **BUG (STILL BROKEN)**: Duplicate messages STILL showing in app despite "fix". + - **Done (2025-12-25 18:00)**: WHAT: Removed duplicate user_message emission from `packages/server/src/server/agent/providers/codex-mcp-agent.ts:2599-2603` - the provider was emitting user_message in `stream()` but `agent-manager.ts`'s `recordUserMessage()` (called by session.ts before stream) already dispatches this event. Updated test in `packages/server/src/server/agent/providers/codex-mcp-agent.test.ts:405` to expect 0 user_messages from provider since agent-manager handles this. RESULT: User messages now appear exactly once in the UI. EVIDENCE: Playwright verification on localhost:8081 - created new Codex agent with prompt "test fix", confirmed only ONE user message in UI (newStreamLength stayed at 1, UI snapshot showed single "test fix" bubble). Unit test passes: `npm run test --workspace=@paseo/server -- codex-mcp-agent.test.ts -t "provider does not emit user_message"` (1 passed). + +- [x] **BUG**: Codex agent doesn't see agent-control MCP - Claude does. + - **Done (2025-12-25 22:45)**: WHAT: Fixed `buildCodexMcpConfig()` in `packages/server/src/server/agent/providers/codex-mcp-agent.ts:2337-2429` to include MCP servers in the Codex tool call. Added `CodexMcpServerConfig` and `CodexConfigPayload` types at lines 2324-2335. Added `managedAgentId` parameter to append caller agent ID to agent-control URL. Built MCP servers config including: (1) `agent-control` HTTP MCP with URL and `http_headers` (using Codex's field name, not `headers`), (2) `playwright` STDIO MCP server, (3) user-provided MCP servers from `config.mcpServers`. Added `managedAgentId` property to `CodexMcpAgentSession` class at line 2516. Updated `setManagedAgentId()` at lines 2907-2909 to store the ID. Updated all 3 call sites of `buildCodexMcpConfig()` at lines 2924, 2962, 2985 to pass `this.managedAgentId`. ROOT CAUSE: Claude provider at lines 672-699 builds MCP servers config and passes to Claude SDK. Codex MCP provider at line 2360 only passed `config.extra.codex` - completely ignoring `config.agentControlMcp` and `config.mcpServers`. Codex CLI expects MCP servers in `config.mcp_servers` field with `http_headers` (not `headers`) for HTTP servers. RESULT: Codex agents now receive agent-control and playwright MCP servers in tool call config. EVIDENCE: Typecheck passes (`npm run typecheck --workspace=@paseo/server`), unit test passes (`npm run test --workspace=@paseo/server -- codex-mcp-agent.test.ts -t "responds with text"`), quick verification script shows MCP config includes `agent-control` with URL and headers.