From 7c535daf3558e138fd957427e504e4e74b57611d Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Wed, 3 Jun 2026 14:13:08 +0200 Subject: [PATCH] fix: Sanitize ACP MCP server names for Codex We ran into this issue on our end where codex doesnt' allow mcp servers with whitespace. --- src/CodexAcpClient.ts | 9 ++- src/CodexAcpServer.ts | 7 ++- src/McpServerName.ts | 5 ++ .../CodexACPAgent/CodexAcpClient.test.ts | 61 +++++++++++++++++++ 4 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 src/McpServerName.ts diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 22cde0da..d76e0269 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -21,6 +21,7 @@ import {ModelId} from "./ModelId"; import {AgentMode} from "./AgentMode"; import path from "node:path"; import {logger} from "./Logger"; +import {sanitizeMcpServerName} from "./McpServerName"; import type { AccountLoginCompletedNotification, AccountUpdatedNotification, @@ -282,14 +283,18 @@ export class CodexAcpClient { // Deduplicates new servers against existing config to prevent Codex from deep-merging // incompatible field types (e.g., mixing url and stdio schemas). const existingNames = await this.getConfigMcpServerNames(projectPath); - const uniqueServers = mcpServers.filter(mcp => !existingNames.has(mcp.name)); + const requestedServers = mcpServers.map(mcp => ({ + name: sanitizeMcpServerName(mcp.name), + server: mcp, + })); + const uniqueServers = requestedServers.filter(mcp => !existingNames.has(mcp.name)); if (uniqueServers.length === 0) { return mergedConfig; } return { ...mergedConfig, - "mcp_servers": Object.fromEntries(uniqueServers.map(mcp => [mcp.name, this.createMcpSeverConfig(mcp)])), + "mcp_servers": Object.fromEntries(uniqueServers.map(mcp => [mcp.name, this.createMcpSeverConfig(mcp.server)])), }; } diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 0865c984..0af30418 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -25,6 +25,7 @@ import {toPromptUsage} from "./TokenCount"; import {CodexCommands} from "./CodexCommands"; import type {QuotaMeta} from "./QuotaMeta"; import {logger} from "./Logger"; +import {sanitizeMcpServerName} from "./McpServerName"; import {isExtMethodRequest} from "./AcpExtensions"; import { createCommandExecutionUpdate, @@ -209,7 +210,7 @@ export class CodexAcpServer implements acp.Agent { if (requestedMcpServers.length > 0 && mcpServerStartupVersion !== null) { this.pendingMcpStartupSessions.set(sessionId, { - requestedServers: new Set(requestedMcpServers.map(server => server.name)), + requestedServers: new Set(getRequestedMcpServerNames(requestedMcpServers)), afterVersion: mcpServerStartupVersion, }); this.publishMcpStartupStatusAsync(sessionId); @@ -476,7 +477,7 @@ export class CodexAcpServer implements acp.Agent { if (requestedMcpServers.length > 0 && mcpServerStartupVersion !== null) { this.pendingMcpStartupSessions.set(sessionId, { - requestedServers: new Set(requestedMcpServers.map(server => server.name)), + requestedServers: new Set(getRequestedMcpServerNames(requestedMcpServers)), afterVersion: mcpServerStartupVersion, }); this.publishMcpStartupStatusAsync(sessionId); @@ -957,5 +958,5 @@ export class CodexAcpServer implements acp.Agent { } function getRequestedMcpServerNames(mcpServers: Array): Array { - return Array.from(new Set(mcpServers.map(server => server.name))); + return Array.from(new Set(mcpServers.map(server => sanitizeMcpServerName(server.name)))); } diff --git a/src/McpServerName.ts b/src/McpServerName.ts new file mode 100644 index 00000000..d7a89aa8 --- /dev/null +++ b/src/McpServerName.ts @@ -0,0 +1,5 @@ +const MCP_SERVER_NAME_WHITESPACE = /\p{White_Space}/gu; + +export function sanitizeMcpServerName(name: string): string { + return name.replace(MCP_SERVER_NAME_WHITESPACE, "_"); +} diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index a66a8276..804d2a08 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -6,6 +6,7 @@ import type * as acp from "@agentclientprotocol/sdk"; import { createCodexMockTestFixture, createTestFixture, + createTestModel, createTestSessionState, type TestFixture } from "../acp-test-utils"; @@ -269,6 +270,66 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(listSkillsSpy.mock.invocationCallOrder[0]!).toBeLessThan(threadStartSpy.mock.invocationCallOrder[0]!); }); + it('sanitizes whitespace in ACP MCP server names before adding them to Codex config', async () => { + const mockFixture = createCodexMockTestFixture(); + const codexAcpClient = mockFixture.getCodexAcpClient(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + + vi.spyOn(codexAppServerClient, "listSkills").mockResolvedValue({data: []}); + vi.spyOn(codexAppServerClient, "configRead").mockResolvedValue({ + config: { + mcp_servers: { + shared_mcp: { + url: "https://example.com/mcp", + }, + }, + }, + } as any); + const threadStartSpy = vi.spyOn(codexAppServerClient, "threadStart").mockResolvedValue({ + thread: {id: "thread-id"} as any, + model: "gpt-5", + reasoningEffort: "medium", + serviceTier: null, + } as any); + vi.spyOn(codexAppServerClient, "listModels").mockResolvedValue({ + data: [createTestModel({id: "gpt-5"})], + nextCursor: null, + }); + + await codexAcpClient.newSession({ + cwd: "/workspace", + mcpServers: [{ + name: "shared mcp", + command: "npx", + args: ["shared"], + env: [], + }, { + name: "stdio server\tone", + command: "npx", + args: ["stdio"], + env: [{name: "EXAMPLE", value: "1"}], + }, { + type: "http", + name: "http\nserver\u00a0two", + url: "https://example.com/http", + headers: [{name: "Authorization", value: "Bearer token"}], + }], + }); + + const threadStartRequest = threadStartSpy.mock.calls[0]![0]; + expect(threadStartRequest.config?.["mcp_servers"]).toEqual({ + stdio_server_one: { + command: "npx", + args: ["stdio"], + env: {EXAMPLE: "1"}, + }, + http_server_two: { + url: "https://example.com/http", + http_headers: {Authorization: "Bearer token"}, + }, + }); + }); + it('waits for typed mcp startup status updates and returns terminal states', async () => { const mockFixture = createCodexMockTestFixture(); const codexAcpClient = mockFixture.getCodexAcpClient();