Skip to content

Commit e698674

Browse files
authored
fix: Sanitize ACP MCP server names for Codex (#172)
We ran into this issue on our end where codex doesnt' allow mcp servers with whitespace.
1 parent 04b7d03 commit e698674

4 files changed

Lines changed: 77 additions & 5 deletions

File tree

src/CodexAcpClient.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {ModelId} from "./ModelId";
2121
import {AgentMode} from "./AgentMode";
2222
import path from "node:path";
2323
import {logger} from "./Logger";
24+
import {sanitizeMcpServerName} from "./McpServerName";
2425
import type {
2526
AccountLoginCompletedNotification,
2627
AccountUpdatedNotification,
@@ -292,14 +293,18 @@ export class CodexAcpClient {
292293
// Deduplicates new servers against existing config to prevent Codex from deep-merging
293294
// incompatible field types (e.g., mixing url and stdio schemas).
294295
const existingNames = await this.getConfigMcpServerNames(projectPath);
295-
const uniqueServers = mcpServers.filter(mcp => !existingNames.has(mcp.name));
296+
const requestedServers = mcpServers.map(mcp => ({
297+
name: sanitizeMcpServerName(mcp.name),
298+
server: mcp,
299+
}));
300+
const uniqueServers = requestedServers.filter(mcp => !existingNames.has(mcp.name));
296301
if (uniqueServers.length === 0) {
297302
return mergedConfig;
298303
}
299304

300305
return {
301306
...mergedConfig,
302-
"mcp_servers": Object.fromEntries(uniqueServers.map(mcp => [mcp.name, this.createMcpSeverConfig(mcp)])),
307+
"mcp_servers": Object.fromEntries(uniqueServers.map(mcp => [mcp.name, this.createMcpSeverConfig(mcp.server)])),
303308
};
304309
}
305310

src/CodexAcpServer.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {toPromptUsage} from "./TokenCount";
2525
import {CodexCommands} from "./CodexCommands";
2626
import type {QuotaMeta} from "./QuotaMeta";
2727
import {logger} from "./Logger";
28+
import {sanitizeMcpServerName} from "./McpServerName";
2829
import {isExtMethodRequest} from "./AcpExtensions";
2930
import {
3031
createCommandExecutionUpdate,
@@ -341,7 +342,7 @@ export class CodexAcpServer implements acp.Agent {
341342

342343
if (requestedMcpServers.length > 0 && mcpServerStartupVersion !== null) {
343344
this.pendingMcpStartupSessions.set(sessionId, {
344-
requestedServers: new Set(requestedMcpServers.map(server => server.name)),
345+
requestedServers: new Set(getRequestedMcpServerNames(requestedMcpServers)),
345346
afterVersion: mcpServerStartupVersion,
346347
});
347348
this.publishMcpStartupStatusAsync(sessionId);
@@ -667,7 +668,7 @@ export class CodexAcpServer implements acp.Agent {
667668

668669
if (requestedMcpServers.length > 0 && mcpServerStartupVersion !== null) {
669670
this.pendingMcpStartupSessions.set(sessionId, {
670-
requestedServers: new Set(requestedMcpServers.map(server => server.name)),
671+
requestedServers: new Set(getRequestedMcpServerNames(requestedMcpServers)),
671672
afterVersion: mcpServerStartupVersion,
672673
});
673674
this.publishMcpStartupStatusAsync(sessionId);
@@ -1322,5 +1323,5 @@ export class CodexAcpServer implements acp.Agent {
13221323
}
13231324

13241325
function getRequestedMcpServerNames(mcpServers: Array<acp.McpServer>): Array<string> {
1325-
return Array.from(new Set(mcpServers.map(server => server.name)));
1326+
return Array.from(new Set(mcpServers.map(server => sanitizeMcpServerName(server.name))));
13261327
}

src/McpServerName.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
const MCP_SERVER_NAME_WHITESPACE = /\p{White_Space}/gu;
2+
3+
export function sanitizeMcpServerName(name: string): string {
4+
return name.replace(MCP_SERVER_NAME_WHITESPACE, "_");
5+
}

src/__tests__/CodexACPAgent/CodexAcpClient.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type * as acp from "@agentclientprotocol/sdk";
66
import {
77
createCodexMockTestFixture,
88
createTestFixture,
9+
createTestModel,
910
createTestSessionState,
1011
type TestFixture
1112
} from "../acp-test-utils";
@@ -269,6 +270,66 @@ describe('ACP server test', { timeout: 40_000 }, () => {
269270
expect(listSkillsSpy.mock.invocationCallOrder[0]!).toBeLessThan(threadStartSpy.mock.invocationCallOrder[0]!);
270271
});
271272

273+
it('sanitizes whitespace in ACP MCP server names before adding them to Codex config', async () => {
274+
const mockFixture = createCodexMockTestFixture();
275+
const codexAcpClient = mockFixture.getCodexAcpClient();
276+
const codexAppServerClient = mockFixture.getCodexAppServerClient();
277+
278+
vi.spyOn(codexAppServerClient, "listSkills").mockResolvedValue({data: []});
279+
vi.spyOn(codexAppServerClient, "configRead").mockResolvedValue({
280+
config: {
281+
mcp_servers: {
282+
shared_mcp: {
283+
url: "https://example.com/mcp",
284+
},
285+
},
286+
},
287+
} as any);
288+
const threadStartSpy = vi.spyOn(codexAppServerClient, "threadStart").mockResolvedValue({
289+
thread: {id: "thread-id"} as any,
290+
model: "gpt-5",
291+
reasoningEffort: "medium",
292+
serviceTier: null,
293+
} as any);
294+
vi.spyOn(codexAppServerClient, "listModels").mockResolvedValue({
295+
data: [createTestModel({id: "gpt-5"})],
296+
nextCursor: null,
297+
});
298+
299+
await codexAcpClient.newSession({
300+
cwd: "/workspace",
301+
mcpServers: [{
302+
name: "shared mcp",
303+
command: "npx",
304+
args: ["shared"],
305+
env: [],
306+
}, {
307+
name: "stdio server\tone",
308+
command: "npx",
309+
args: ["stdio"],
310+
env: [{name: "EXAMPLE", value: "1"}],
311+
}, {
312+
type: "http",
313+
name: "http\nserver\u00a0two",
314+
url: "https://example.com/http",
315+
headers: [{name: "Authorization", value: "Bearer token"}],
316+
}],
317+
});
318+
319+
const threadStartRequest = threadStartSpy.mock.calls[0]![0];
320+
expect(threadStartRequest.config?.["mcp_servers"]).toEqual({
321+
stdio_server_one: {
322+
command: "npx",
323+
args: ["stdio"],
324+
env: {EXAMPLE: "1"},
325+
},
326+
http_server_two: {
327+
url: "https://example.com/http",
328+
http_headers: {Authorization: "Bearer token"},
329+
},
330+
});
331+
});
332+
272333
it('waits for typed mcp startup status updates and returns terminal states', async () => {
273334
const mockFixture = createCodexMockTestFixture();
274335
const codexAcpClient = mockFixture.getCodexAcpClient();

0 commit comments

Comments
 (0)