Merge PR #7540: fix(discovery): honor disabled codex mcp servers (@roboomp)
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -87,19 +87,13 @@ async function loadMCPServers(ctx: LoadContext): Promise<LoadResult<MCPServer>>
|
||||
]);
|
||||
|
||||
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<LoadResult<MCPServer>>
|
||||
});
|
||||
}
|
||||
}
|
||||
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<Record<s
|
||||
|
||||
/** Codex MCP server config format (from config.toml) */
|
||||
interface CodexMCPConfig {
|
||||
enabled?: boolean;
|
||||
command?: string;
|
||||
args?: string[];
|
||||
env?: Record<string, string>;
|
||||
@@ -152,13 +158,20 @@ function extractMCPServersFromToml(
|
||||
const codexServers = toml.mcp_servers as Record<string, CodexMCPConfig>;
|
||||
const result: Record<string, Partial<MCPServer>> = {};
|
||||
|
||||
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 <configDir>/server/bin/mcp.
|
||||
const rooted = resolvePluginStdioPaths({ command: config.command, cwd: config.cwd }, configDir, "cwd");
|
||||
const server: Partial<MCPServer> = {
|
||||
// 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,
|
||||
|
||||
@@ -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<MCPServer[]> {
|
||||
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<MCPServer>(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(
|
||||
|
||||
Reference in New Issue
Block a user