From 8b854598f1649c49f501c37ee76787558ab9c270 Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 3 Aug 2026 16:38:09 +0000 Subject: [PATCH 1/3] fix(discovery): honored disabled codex mcp servers - Skipped Codex config.toml MCP entries explicitly marked enabled = false. - Added discovery regression coverage for disabled and enabled entries. Fixes #7538 --- packages/coding-agent/CHANGELOG.md | 4 ++++ packages/coding-agent/src/discovery/codex.ts | 3 +++ .../test/discovery/codex-mcp-cwd.test.ts | 23 ++++++++++++++++++- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 46d565c01..24f39d47c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed Codex `config.toml` discovery importing MCP servers with `enabled = false` ([#7538](https://github.com/can1357/oh-my-pi/issues/7538)). + ## [17.2.6] - 2026-08-03 ### Added diff --git a/packages/coding-agent/src/discovery/codex.ts b/packages/coding-agent/src/discovery/codex.ts index cd993c989..a723071c4 100644 --- a/packages/coding-agent/src/discovery/codex.ts +++ b/packages/coding-agent/src/discovery/codex.ts @@ -125,6 +125,7 @@ async function loadTomlConfig(_ctx: LoadContext, path: string): Promise; @@ -153,6 +154,8 @@ function extractMCPServersFromToml( const result: Record> = {}; for (const [name, config] of Object.entries(codexServers)) { + if (config.enabled === false) continue; + // Root relative cwd/command against the Codex config directory. Codex // spawns the process with the resolved cwd, so a relative command is // resolved by the OS from there — pass "cwd" so e.g. cwd="server", diff --git a/packages/coding-agent/test/discovery/codex-mcp-cwd.test.ts b/packages/coding-agent/test/discovery/codex-mcp-cwd.test.ts index 15ab602ab..ccd99370b 100644 --- a/packages/coding-agent/test/discovery/codex-mcp-cwd.test.ts +++ b/packages/coding-agent/test/discovery/codex-mcp-cwd.test.ts @@ -1,5 +1,5 @@ /** - * Regression tests for #5561. + * Regression tests for Codex MCP config discovery (#5561, #7538). * * The Codex `config.toml` MCP importer in `packages/coding-agent/src/discovery/codex.ts` * used to copy only `command`/`args`/`url` into the returned `MCPServer`, dropping @@ -48,6 +48,27 @@ async function loadCodexServers(): Promise { return result.items; } +test("disabled Codex MCP servers are not discovered (#7538)", async () => { + const codexDir = path.join(tempHome, ".codex"); + await fs.writeFile( + path.join(codexDir, "config.toml"), + [ + "[mcp_servers.computer-use]", + 'command = "./Codex Computer Use.app/Contents/MacOS/SkyComputerUseClient"', + "enabled = false", + "", + "[mcp_servers.context7]", + 'command = "npx"', + "enabled = true", + "", + ].join("\n"), + ); + + const servers = await loadCodexServers(); + expect(servers.find(server => server.name === "computer-use")).toBeUndefined(); + expect(servers.find(server => server.name === "context7")?.command).toBe("npx"); +}); + test("relative path-like command and cwd resolve against the Codex config directory (#5561)", async () => { const codexDir = path.join(tempHome, ".codex"); await fs.writeFile( From 012836d99ee5e9acfa2deabf8d8e00c5de3c1a11 Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 3 Aug 2026 16:45:31 +0000 Subject: [PATCH 2/3] fix(discovery): tag disabled codex mcp servers instead of dropping Carry enabled = false through discovery so loadAllMCPConfigs' suppress path can claim the dedupe key (keeping a same-named lower-priority source disabled) and honor the user force-enable allowlist. Dropping the entry outright defeated both. Fixes #7538 --- packages/coding-agent/src/discovery/codex.ts | 8 ++++++-- .../test/discovery/codex-mcp-cwd.test.ts | 14 +++++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/packages/coding-agent/src/discovery/codex.ts b/packages/coding-agent/src/discovery/codex.ts index a723071c4..402e6051b 100644 --- a/packages/coding-agent/src/discovery/codex.ts +++ b/packages/coding-agent/src/discovery/codex.ts @@ -154,14 +154,18 @@ function extractMCPServersFromToml( const result: Record> = {}; for (const [name, config] of Object.entries(codexServers)) { - if (config.enabled === false) continue; - // Root relative cwd/command against the Codex config directory. Codex // spawns the process with the resolved cwd, so a relative command is // resolved by the OS from there — pass "cwd" so e.g. cwd="server", // command="./bin/mcp" resolves to /server/bin/mcp. const rooted = resolvePluginStdioPaths({ command: config.command, cwd: config.cwd }, configDir, "cwd"); const server: Partial = { + // Carry `enabled: false` through rather than dropping the entry: the + // central MCP loader (`loadAllMCPConfigs`) suppresses disabled servers + // so they still claim their dedupe key (keeping a same-named, + // lower-priority source disabled) and remain overridable via the user + // force-enable allowlist. Dropping here would defeat both. + ...(config.enabled === false && { enabled: false }), ...(rooted.command !== undefined && { command: rooted.command }), args: config.args, url: config.url, diff --git a/packages/coding-agent/test/discovery/codex-mcp-cwd.test.ts b/packages/coding-agent/test/discovery/codex-mcp-cwd.test.ts index ccd99370b..dca7be0c9 100644 --- a/packages/coding-agent/test/discovery/codex-mcp-cwd.test.ts +++ b/packages/coding-agent/test/discovery/codex-mcp-cwd.test.ts @@ -48,7 +48,7 @@ async function loadCodexServers(): Promise { return result.items; } -test("disabled Codex MCP servers are not discovered (#7538)", async () => { +test("disabled Codex MCP servers survive discovery tagged enabled: false (#7538)", async () => { const codexDir = path.join(tempHome, ".codex"); await fs.writeFile( path.join(codexDir, "config.toml"), @@ -65,8 +65,16 @@ test("disabled Codex MCP servers are not discovered (#7538)", async () => { ); const servers = await loadCodexServers(); - expect(servers.find(server => server.name === "computer-use")).toBeUndefined(); - expect(servers.find(server => server.name === "context7")?.command).toBe("npx"); + const cu = servers.find(server => server.name === "computer-use"); + const context7 = servers.find(server => server.name === "context7"); + + // The disabled entry is NOT dropped: it stays so loadAllMCPConfigs' suppress + // path can claim its dedupe key (keeping a same-named, lower-priority source + // disabled) and honor the user force-enable allowlist. `enabled: true` is the + // default, so it is not persisted onto the enabled sibling. + expect(cu?.enabled).toBe(false); + expect(context7?.command).toBe("npx"); + expect(context7?.enabled).toBeUndefined(); }); test("relative path-like command and cwd resolve against the Codex config directory (#5561)", async () => { From 6f7600cd89832436f814239f9097405fa5871ddd Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 3 Aug 2026 16:52:54 +0000 Subject: [PATCH 3/3] fix(discovery): prioritized project codex mcp entries Load project Codex MCP entries before user entries so a disabled project server claims its dedupe key before a same-named user server can survive. Added regression coverage for project-over-user disable precedence. Fixes #7538 --- packages/coding-agent/src/discovery/codex.ts | 30 +++++++++++-------- .../test/discovery/codex-mcp-cwd.test.ts | 24 +++++++++++++++ 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/packages/coding-agent/src/discovery/codex.ts b/packages/coding-agent/src/discovery/codex.ts index 402e6051b..d5328371f 100644 --- a/packages/coding-agent/src/discovery/codex.ts +++ b/packages/coding-agent/src/discovery/codex.ts @@ -87,19 +87,13 @@ async function loadMCPServers(ctx: LoadContext): Promise> ]); const items: MCPServer[] = []; - if (userConfig) { - const servers = extractMCPServersFromToml(userConfig, path.dirname(userConfigPath)); - for (const [name, config] of Object.entries(servers)) { - items.push({ - name, - ...config, - _source: createSourceMeta(PROVIDER_ID, userConfigPath, "user"), - }); - } - } + // Capability dedupe is first-wins, including suppressed items claiming their + // key. Load project entries first so a project `enabled = false` keeps a + // same-named user server disabled. if (projectConfig) { const servers = extractMCPServersFromToml(projectConfig, path.dirname(projectConfigPath)); - for (const [name, config] of Object.entries(servers)) { + for (const name in servers) { + const config = servers[name]; items.push({ name, ...config, @@ -107,6 +101,17 @@ async function loadMCPServers(ctx: LoadContext): Promise> }); } } + if (userConfig) { + const servers = extractMCPServersFromToml(userConfig, path.dirname(userConfigPath)); + for (const name in servers) { + const config = servers[name]; + items.push({ + name, + ...config, + _source: createSourceMeta(PROVIDER_ID, userConfigPath, "user"), + }); + } + } return { items, warnings }; } @@ -153,7 +158,8 @@ function extractMCPServersFromToml( const codexServers = toml.mcp_servers as Record; const result: Record> = {}; - for (const [name, config] of Object.entries(codexServers)) { + for (const name in codexServers) { + const config = codexServers[name]; // Root relative cwd/command against the Codex config directory. Codex // spawns the process with the resolved cwd, so a relative command is // resolved by the OS from there — pass "cwd" so e.g. cwd="server", diff --git a/packages/coding-agent/test/discovery/codex-mcp-cwd.test.ts b/packages/coding-agent/test/discovery/codex-mcp-cwd.test.ts index dca7be0c9..06aeef700 100644 --- a/packages/coding-agent/test/discovery/codex-mcp-cwd.test.ts +++ b/packages/coding-agent/test/discovery/codex-mcp-cwd.test.ts @@ -77,6 +77,30 @@ test("disabled Codex MCP servers survive discovery tagged enabled: false (#7538) expect(context7?.enabled).toBeUndefined(); }); +test("project Codex disable suppresses a same-named user server (#7538)", async () => { + const userCodexDir = path.join(tempHome, ".codex"); + const projectCodexDir = path.join(tempCwd, ".codex"); + await fs.mkdir(projectCodexDir, { recursive: true }); + await Promise.all([ + fs.writeFile( + path.join(userCodexDir, "config.toml"), + ["[mcp_servers.shared]", 'command = "user-server"', ""].join("\n"), + ), + fs.writeFile( + path.join(projectCodexDir, "config.toml"), + ["[mcp_servers.shared]", 'command = "project-server"', "enabled = false", ""].join("\n"), + ), + ]); + + const result = await loadCapability(mcpCapability.id, { + cwd: tempCwd, + providers: ["codex"], + suppress: server => server.enabled === false, + }); + + expect(result.items.find(server => server.name === "shared")).toBeUndefined(); +}); + test("relative path-like command and cwd resolve against the Codex config directory (#5561)", async () => { const codexDir = path.join(tempHome, ".codex"); await fs.writeFile(