diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index ec77b5e..ee76c03 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -326,21 +326,23 @@ export class CodexAcpClient { return configWithWorkspaceRoots; } - // 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 requestedServers = mcpServers.map(mcp => ({ name: sanitizeMcpServerName(mcp.name), server: mcp, })); - const uniqueServers = requestedServers.filter(mcp => !existingNames.has(mcp.name)); - if (uniqueServers.length === 0) { + let serversToConfigure = requestedServers; + if (shouldDeduplicateMcpConflicts()) { + // Prevents Codex from deep-merging incompatible field types, such as url and stdio schemas. + const existingNames = await this.getConfigMcpServerNames(projectPath); + serversToConfigure = requestedServers.filter(mcp => !existingNames.has(mcp.name)); + } + if (serversToConfigure.length === 0) { return configWithWorkspaceRoots; } return { ...configWithWorkspaceRoots, - "mcp_servers": Object.fromEntries(uniqueServers.map(mcp => [mcp.name, this.createMcpSeverConfig(mcp.server)])), + "mcp_servers": Object.fromEntries(serversToConfigure.map(mcp => [mcp.name, this.createMcpSeverConfig(mcp.server)])), }; } @@ -735,6 +737,11 @@ function formatUriAsLink(name: string | null | undefined, uri: string): string { return uri; } +function shouldDeduplicateMcpConflicts(): boolean { + const disabledByEnv = process.env["DISABLE_MCP_CONFIG_FILTERING"] === "true"; + return !disabledByEnv; +} + interface GatewayConfig { modelProvider: string; config: { diff --git a/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts b/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts index ab54b8e..f2cb510 100644 --- a/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts +++ b/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts @@ -35,6 +35,7 @@ url = "https://example.com/mcp" }); afterEach(() => { + vi.unstubAllEnvs(); removeDirectoryWithRetry(codexHome); }); @@ -66,4 +67,24 @@ url = "https://example.com/mcp" expect(transportDump).contain("Configured MCP servers:"); expect(transportDump).contain("- shared-mcp"); }); + + it('should not filter the conflicting ACP MCP when config filtering is disabled', async () => { + vi.stubEnv("DISABLE_MCP_CONFIG_FILTERING", "true"); + const codexAcpAgent = fixture.getCodexAcpAgent(); + await codexAcpAgent.initialize({protocolVersion: 1}); + + fixture.getCodexAcpClient().authRequired = vi.fn().mockResolvedValue(false); + + const conflictingMcp: McpServerStdio = { + name: "shared-mcp", + command: "./node_modules/.bin/mcp-hello-world", + args: ["example"], + env: [{name: "example", value: "example"}], + }; + + await expect(codexAcpAgent.newSession({ + cwd: "", + mcpServers: [conflictingMcp], + })).rejects.toThrow("url is not supported for stdio"); + }); });