From b6f8c77a1816bdbe83552d8670d41d2f64ffd21b Mon Sep 17 00:00:00 2001 From: Ilia Brovarnik Date: Tue, 16 Jun 2026 22:07:17 +0900 Subject: [PATCH 1/6] Allow disabling MCP config filtering --- src/CodexAcpClient.ts | 8 ++++++-- src/index.ts | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index ec77b5e2..ab9ae7fc 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -48,6 +48,7 @@ export class CodexAcpClient { private readonly codexClient: CodexAppServerClient; private readonly config: JsonObject; private readonly modelProvider: string | null; + private readonly disableMcpConfigFiltering: boolean; private gatewayConfig: GatewayConfig | null; private pendingLoginCompleted: Promise | null = null; private pendingAccountUpdated: Promise | null = null; @@ -55,10 +56,11 @@ export class CodexAcpClient { private skillExtraRoots: string[] = []; - constructor(codexClient: CodexAppServerClient, codexConfig?: JsonObject, modelProvider?: string) { + constructor(codexClient: CodexAppServerClient, codexConfig?: JsonObject, modelProvider?: string, disableMcpConfigFiltering = false) { this.codexClient = codexClient; this.config = codexConfig ?? {}; this.modelProvider = modelProvider ?? null; + this.disableMcpConfigFiltering = disableMcpConfigFiltering; this.gatewayConfig = null; } @@ -328,7 +330,9 @@ 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 existingNames = this.disableMcpConfigFiltering + ? new Set() + : await this.getConfigMcpServerNames(projectPath); const requestedServers = mcpServers.map(mcp => ({ name: sanitizeMcpServerName(mcp.name), server: mcp, diff --git a/src/index.ts b/src/index.ts index c892310f..7af8a4be 100644 --- a/src/index.ts +++ b/src/index.ts @@ -42,6 +42,7 @@ function startAcpServer() { const configString = process.env["CODEX_CONFIG"]; const authRequestString = process.env["DEFAULT_AUTH_REQUEST"]; const modelProvider = process.env["MODEL_PROVIDER"]; + const disableMcpConfigFiltering = process.env["DISABLE_MCP_CONFIG_FILTERING"] === "true"; const config = configString ? JSON.parse(configString) : undefined; const parsedAuthRequest = authRequestString ? JSON.parse(authRequestString) : undefined; const defaultAuthRequest = parsedAuthRequest && isCodexAuthRequest(parsedAuthRequest) ? parsedAuthRequest : undefined; @@ -72,7 +73,7 @@ function startAcpServer() { function createAgent(connection: acp.AgentSideConnection): CodexAcpServer { const appServerClient = new CodexAppServerClient(codexConnection.connection); - const codexClient = new CodexAcpClient(appServerClient, config, modelProvider); + const codexClient = new CodexAcpClient(appServerClient, config, modelProvider, disableMcpConfigFiltering); return new CodexAcpServer(connection, codexClient, defaultAuthRequest, () => codexConnection.process.exitCode); } From 7b2aecfdbb09eefce51482f7f47a20482d4bda46 Mon Sep 17 00:00:00 2001 From: Ilia Brovarnik Date: Wed, 17 Jun 2026 15:04:33 +0900 Subject: [PATCH 2/6] Refine MCP config conflict deduplication toggle --- src/CodexAcpClient.ts | 25 ++++++++++++++----------- src/index.ts | 3 +-- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index ab9ae7fc..ee76c036 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -48,7 +48,6 @@ export class CodexAcpClient { private readonly codexClient: CodexAppServerClient; private readonly config: JsonObject; private readonly modelProvider: string | null; - private readonly disableMcpConfigFiltering: boolean; private gatewayConfig: GatewayConfig | null; private pendingLoginCompleted: Promise | null = null; private pendingAccountUpdated: Promise | null = null; @@ -56,11 +55,10 @@ export class CodexAcpClient { private skillExtraRoots: string[] = []; - constructor(codexClient: CodexAppServerClient, codexConfig?: JsonObject, modelProvider?: string, disableMcpConfigFiltering = false) { + constructor(codexClient: CodexAppServerClient, codexConfig?: JsonObject, modelProvider?: string) { this.codexClient = codexClient; this.config = codexConfig ?? {}; this.modelProvider = modelProvider ?? null; - this.disableMcpConfigFiltering = disableMcpConfigFiltering; this.gatewayConfig = null; } @@ -328,23 +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 = this.disableMcpConfigFiltering - ? new Set() - : 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)])), }; } @@ -739,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/index.ts b/src/index.ts index 7af8a4be..c892310f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -42,7 +42,6 @@ function startAcpServer() { const configString = process.env["CODEX_CONFIG"]; const authRequestString = process.env["DEFAULT_AUTH_REQUEST"]; const modelProvider = process.env["MODEL_PROVIDER"]; - const disableMcpConfigFiltering = process.env["DISABLE_MCP_CONFIG_FILTERING"] === "true"; const config = configString ? JSON.parse(configString) : undefined; const parsedAuthRequest = authRequestString ? JSON.parse(authRequestString) : undefined; const defaultAuthRequest = parsedAuthRequest && isCodexAuthRequest(parsedAuthRequest) ? parsedAuthRequest : undefined; @@ -73,7 +72,7 @@ function startAcpServer() { function createAgent(connection: acp.AgentSideConnection): CodexAcpServer { const appServerClient = new CodexAppServerClient(codexConnection.connection); - const codexClient = new CodexAcpClient(appServerClient, config, modelProvider, disableMcpConfigFiltering); + const codexClient = new CodexAcpClient(appServerClient, config, modelProvider); return new CodexAcpServer(connection, codexClient, defaultAuthRequest, () => codexConnection.process.exitCode); } From 900accd4b80ac4a1d07b9470852f1d241e4432eb Mon Sep 17 00:00:00 2001 From: Ilia Brovarnik Date: Wed, 17 Jun 2026 15:27:33 +0900 Subject: [PATCH 3/6] Test disabled MCP config filtering --- .../CodexACPAgent/CodexAcpClient.test.ts | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 7474c3f4..ec29b120 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -439,6 +439,58 @@ describe('ACP server test', { timeout: 40_000 }, () => { }); }); + it('passes conflicting ACP MCP servers through when config filtering is disabled', async () => { + vi.stubEnv("DISABLE_MCP_CONFIG_FILTERING", "true"); + try { + const mockFixture = createCodexMockTestFixture(); + const codexAcpClient = mockFixture.getCodexAcpClient(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + + vi.spyOn(codexAppServerClient, "listSkills").mockResolvedValue({data: []}); + const configReadSpy = 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: [], + }], + }); + + const threadStartRequest = threadStartSpy.mock.calls[0]![0]; + expect(configReadSpy).not.toHaveBeenCalled(); + expect(threadStartRequest.config?.["mcp_servers"]).toEqual({ + shared_mcp: { + command: "npx", + args: ["shared"], + env: {}, + }, + }); + } finally { + vi.unstubAllEnvs(); + } + }); + it('waits for typed mcp startup status updates and returns terminal states', async () => { const mockFixture = createCodexMockTestFixture(); const codexAcpClient = mockFixture.getCodexAcpClient(); From 0b89e93009c2a506638cf13e35cb407853e32567 Mon Sep 17 00:00:00 2001 From: Ilia Brovarnik Date: Wed, 17 Jun 2026 17:17:26 +0900 Subject: [PATCH 4/6] Address MCP config filtering review comments --- .../CodexACPAgent/CodexAcpClient.test.ts | 52 ------------------- .../CodexACPAgent/mcp-config-merge.test.ts | 37 +++++++++++++ 2 files changed, 37 insertions(+), 52 deletions(-) diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index ec29b120..7474c3f4 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -439,58 +439,6 @@ describe('ACP server test', { timeout: 40_000 }, () => { }); }); - it('passes conflicting ACP MCP servers through when config filtering is disabled', async () => { - vi.stubEnv("DISABLE_MCP_CONFIG_FILTERING", "true"); - try { - const mockFixture = createCodexMockTestFixture(); - const codexAcpClient = mockFixture.getCodexAcpClient(); - const codexAppServerClient = mockFixture.getCodexAppServerClient(); - - vi.spyOn(codexAppServerClient, "listSkills").mockResolvedValue({data: []}); - const configReadSpy = 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: [], - }], - }); - - const threadStartRequest = threadStartSpy.mock.calls[0]![0]; - expect(configReadSpy).not.toHaveBeenCalled(); - expect(threadStartRequest.config?.["mcp_servers"]).toEqual({ - shared_mcp: { - command: "npx", - args: ["shared"], - env: {}, - }, - }); - } finally { - vi.unstubAllEnvs(); - } - }); - it('waits for typed mcp startup status updates and returns terminal states', async () => { const mockFixture = createCodexMockTestFixture(); const codexAcpClient = mockFixture.getCodexAcpClient(); diff --git a/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts b/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts index ab54b8ea..d59ec89c 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,40 @@ url = "https://example.com/mcp" expect(transportDump).contain("Configured MCP servers:"); expect(transportDump).contain("- shared-mcp"); }); + + it('should pass the conflicting ACP MCP through 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 codexAcpAgent.newSession({ + cwd: "", + mcpServers: [conflictingMcp], + }); + + const threadStartRequest = fixture.getCodexConnectionEvents([]) + .find(event => event.eventType === "request" && event.method === "thread/start"); + expect(threadStartRequest).toMatchObject({ + params: { + config: { + mcp_servers: { + "shared-mcp": { + command: "./node_modules/.bin/mcp-hello-world", + args: ["example"], + env: {example: "example"}, + }, + }, + }, + }, + }); + }); }); From eadc50ac38a3ad41f6626b0dc22cf5d16f20a783 Mon Sep 17 00:00:00 2001 From: Ilia Brovarnik Date: Wed, 17 Jun 2026 17:34:35 +0900 Subject: [PATCH 5/6] Expect MCP merge conflict when filtering disabled --- src/__tests__/CodexACPAgent/mcp-config-merge.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts b/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts index d59ec89c..08f83051 100644 --- a/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts +++ b/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts @@ -68,7 +68,7 @@ url = "https://example.com/mcp" expect(transportDump).contain("- shared-mcp"); }); - it('should pass the conflicting ACP MCP through when config filtering is disabled', async () => { + 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}); @@ -82,10 +82,10 @@ url = "https://example.com/mcp" env: [{name: "example", value: "example"}], }; - await codexAcpAgent.newSession({ + await expect(codexAcpAgent.newSession({ cwd: "", mcpServers: [conflictingMcp], - }); + })).rejects.toThrow("url is not supported for stdio"); const threadStartRequest = fixture.getCodexConnectionEvents([]) .find(event => event.eventType === "request" && event.method === "thread/start"); From 042d286ccf1109b958ea2c72fc7282428ddc3fab Mon Sep 17 00:00:00 2001 From: Alexandr Suhinin Date: Wed, 17 Jun 2026 13:27:32 +0300 Subject: [PATCH 6/6] test: remove redundant assertion --- .../CodexACPAgent/mcp-config-merge.test.ts | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts b/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts index 08f83051..f2cb510c 100644 --- a/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts +++ b/src/__tests__/CodexACPAgent/mcp-config-merge.test.ts @@ -86,21 +86,5 @@ url = "https://example.com/mcp" cwd: "", mcpServers: [conflictingMcp], })).rejects.toThrow("url is not supported for stdio"); - - const threadStartRequest = fixture.getCodexConnectionEvents([]) - .find(event => event.eventType === "request" && event.method === "thread/start"); - expect(threadStartRequest).toMatchObject({ - params: { - config: { - mcp_servers: { - "shared-mcp": { - command: "./node_modules/.bin/mcp-hello-world", - args: ["example"], - env: {example: "example"}, - }, - }, - }, - }, - }); }); });