diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index cf610969c..7884b94b7 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -15,6 +15,9 @@ ### Fixed - Fixed `install.sh` reporting success (exit 0) for a musl binary that cannot start: the installer now smoke-runs the downloaded binary and, on failure, prints the captured error plus the `apk add libstdc++ libgcc` remediation for musl targets and exits non-zero. Documented the Alpine/musl runtime requirement in the README ([#7545](https://github.com/can1357/oh-my-pi/issues/7545)). +### 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 diff --git a/packages/coding-agent/src/discovery/codex.ts b/packages/coding-agent/src/discovery/codex.ts index cd993c989..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 }; } @@ -125,6 +130,7 @@ async function loadTomlConfig(_ctx: LoadContext, path: string): Promise; @@ -152,13 +158,20 @@ 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", // 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 15ab602ab..06aeef700 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,59 @@ async function loadCodexServers(): Promise { return result.items; } +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"), + [ + "[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(); + 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("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(