Skip to content

Commit f245b9d

Browse files
Merge pull request #1 from agentclientprotocol/main
Update from upstream main
2 parents edaface + 87383d9 commit f245b9d

4 files changed

Lines changed: 95 additions & 8 deletions

File tree

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"publishConfig": {
44
"access": "public"
55
},
6-
"version": "0.0.43",
6+
"version": "0.0.44",
77
"description": "",
88
"main": "dist/index.js",
99
"bin": {

src/CodexAcpClient.ts

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ export class CodexAcpClient {
207207
approvalPolicy: null,
208208
sandbox: null,
209209
baseInstructions: null,
210-
config: this.createSessionConfig(request.cwd, request.mcpServers ?? []),
210+
config: await this.createSessionConfig(request.cwd, request.mcpServers ?? []),
211211
cwd: request.cwd,
212212
developerInstructions: null,
213213
model: null,
@@ -229,7 +229,7 @@ export class CodexAcpClient {
229229
approvalPolicy: null,
230230
sandbox: null,
231231
baseInstructions: null,
232-
config: this.createSessionConfig(request.cwd, request.mcpServers ?? []),
232+
config: await this.createSessionConfig(request.cwd, request.mcpServers ?? []),
233233
cwd: request.cwd,
234234
developerInstructions: null,
235235
model: null,
@@ -251,7 +251,7 @@ export class CodexAcpClient {
251251
await this.refreshSkills(request.cwd, request._meta);
252252

253253
const response = await this.codexClient.threadStart({
254-
config: this.createSessionConfig(request.cwd, request.mcpServers),
254+
config: await this.createSessionConfig(request.cwd, request.mcpServers),
255255
modelProvider: this.getModelProvider(),
256256
model: null,
257257
cwd: request.cwd,
@@ -283,7 +283,7 @@ export class CodexAcpClient {
283283
return this.codexClient.getMcpServerStartupVersion();
284284
}
285285

286-
private createSessionConfig(projectPath: string, mcpServers: Array<McpServer>): JsonObject {
286+
private async createSessionConfig(projectPath: string, mcpServers: Array<McpServer>): Promise<JsonObject> {
287287
const mergedConfig = {
288288
...mergeGatewayConfig(this.config, this.gatewayConfig),
289289
projects: {
@@ -295,10 +295,28 @@ export class CodexAcpClient {
295295
if (mcpServers.length === 0) {
296296
return mergedConfig;
297297
}
298+
299+
// Deduplicates new servers against existing config to prevent Codex from deep-merging
300+
// incompatible field types (e.g., mixing url and stdio schemas).
301+
const existingNames = await this.getConfigMcpServerNames(projectPath);
302+
const uniqueServers = mcpServers.filter(mcp => !existingNames.has(mcp.name));
303+
if (uniqueServers.length === 0) {
304+
return mergedConfig;
305+
}
306+
298307
return {
299308
...mergedConfig,
300-
"mcp_servers": Object.fromEntries(mcpServers.map(mcp => [mcp.name, this.createMcpSeverConfig(mcp)]))
309+
"mcp_servers": Object.fromEntries(uniqueServers.map(mcp => [mcp.name, this.createMcpSeverConfig(mcp)])),
310+
};
311+
}
312+
313+
private async getConfigMcpServerNames(projectPath: string): Promise<Set<string>> {
314+
const response = await this.codexClient.configRead({ includeLayers: true, cwd: projectPath });
315+
const mcpServers = response?.config?.["mcp_servers"];
316+
if (!mcpServers || typeof mcpServers !== "object" || Array.isArray(mcpServers)) {
317+
return new Set();
301318
}
319+
return new Set(Object.keys(mcpServers));
302320
}
303321

304322
getModelProvider(): string | null {
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// noinspection ES6RedundantAwait
2+
3+
import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest';
4+
import path from "node:path";
5+
import fs from "node:fs";
6+
import os from "node:os";
7+
import type {McpServerStdio} from "@agentclientprotocol/sdk";
8+
import {startCodexConnection} from "../../CodexJsonRpcConnection";
9+
import {createBaseTestFixture, removeDirectoryWithRetry, type TestFixture} from "../acp-test-utils";
10+
11+
describe('MCP config merge across global config and ACP request', { timeout: 40_000 }, () => {
12+
13+
let codexHome: string;
14+
let fixture: TestFixture;
15+
16+
beforeEach(() => {
17+
vi.clearAllMocks();
18+
19+
const configToml = `
20+
[mcp_servers.shared-mcp]
21+
url = "https://example.com/mcp"
22+
`;
23+
codexHome = fs.mkdtempSync(path.join(os.tmpdir(), "codex-acp-mcp-merge-"));
24+
fs.writeFileSync(path.join(codexHome, "config.toml"), configToml, "utf8");
25+
26+
const codexConnection = startCodexConnection(undefined, {
27+
...process.env,
28+
CODEX_HOME: codexHome,
29+
});
30+
31+
fixture = createBaseTestFixture({
32+
connection: codexConnection.connection,
33+
getExitCode: () => codexConnection.process.exitCode,
34+
});
35+
});
36+
37+
afterEach(() => {
38+
removeDirectoryWithRetry(codexHome);
39+
});
40+
41+
it('should preserve the global url-based MCP when ACP passes a command-type MCP with the same name', async () => {
42+
const codexAcpAgent = fixture.getCodexAcpAgent();
43+
await codexAcpAgent.initialize({protocolVersion: 1});
44+
45+
fixture.getCodexAcpClient().authRequired = vi.fn().mockResolvedValue(false);
46+
47+
const conflictingMcp: McpServerStdio = {
48+
name: "shared-mcp",
49+
command: "./node_modules/.bin/mcp-hello-world",
50+
args: ["example"],
51+
env: [{name: "example", value: "example"}],
52+
};
53+
54+
const newSessionResponse = await codexAcpAgent.newSession({
55+
cwd: "",
56+
mcpServers: [conflictingMcp],
57+
});
58+
fixture.clearAcpConnectionDump();
59+
60+
await codexAcpAgent.prompt({
61+
sessionId: newSessionResponse.sessionId,
62+
prompt: [{type: "text", text: "/mcp"}],
63+
});
64+
65+
const transportDump = fixture.getAcpConnectionDump([]);
66+
expect(transportDump).contain("Configured MCP servers:");
67+
expect(transportDump).contain("- shared-mcp");
68+
});
69+
});

0 commit comments

Comments
 (0)