fix(mcp): user config overrides Claude Code .mcp.json with collision warning

Previously, Claude Code's .mcp.json would silently override OpenCode user
config when MCP server names collided. This was unexpected behavior since
users expect their explicit OpenCode configuration to take precedence.

Changes:
1. Swapped merge order: Claude Code .mcp.json is now merged BEFORE user
   config, so user config wins on collision
2. Added warning log when user config overrides a Claude Code MCP server:
   'warning: MCP server X from user config overrides Claude Code .mcp.json'
3. Added comprehensive tests for collision scenarios

Fixes #2946
This commit is contained in:
YeonGyu-Kim
2026-04-02 13:32:11 +09:00
parent 51d9685571
commit 649a83d046
3 changed files with 143 additions and 1 deletions
@@ -0,0 +1,132 @@
/// <reference types="bun-types" />
import { describe, test, expect, spyOn, beforeEach, afterEach } from "bun:test"
import type { OhMyOpenCodeConfig } from "../config"
import * as mcpLoader from "../features/claude-code-mcp-loader"
import * as mcpModule from "../mcp"
import * as shared from "../shared"
let loadMcpConfigsSpy: ReturnType<typeof spyOn>
let createBuiltinMcpsSpy: ReturnType<typeof spyOn>
let logSpy: ReturnType<typeof spyOn>
beforeEach(() => {
loadMcpConfigsSpy = spyOn(mcpLoader, "loadMcpConfigs").mockResolvedValue({
servers: {},
loadedServers: [],
})
createBuiltinMcpsSpy = spyOn(mcpModule, "createBuiltinMcps").mockReturnValue({})
logSpy = spyOn(shared, "log").mockImplementation(() => {})
})
afterEach(() => {
loadMcpConfigsSpy.mockRestore()
createBuiltinMcpsSpy.mockRestore()
logSpy.mockRestore()
})
function createPluginConfig(overrides: Partial<OhMyOpenCodeConfig> = {}): OhMyOpenCodeConfig {
return {
disabled_mcps: [],
...overrides,
} as OhMyOpenCodeConfig
}
const EMPTY_PLUGIN_COMPONENTS = {
commands: {},
skills: {},
agents: {},
mcpServers: {},
hooksConfigs: [],
plugins: [],
errors: [],
}
describe("applyMcpConfig collision handling", () => {
test("merges without collision when names are unique", async () => {
//#given
const userMcp = {
userServer: { type: "remote", url: "https://user.example.com", enabled: true },
}
loadMcpConfigsSpy.mockResolvedValue({
servers: {
claudeServer: { type: "remote", url: "https://claude.example.com", enabled: true },
},
loadedServers: [],
})
const config: Record<string, unknown> = { mcp: userMcp }
const pluginConfig = createPluginConfig()
//#when
const { applyMcpConfig } = await import("./mcp-config-handler")
await applyMcpConfig({ config, pluginConfig, pluginComponents: EMPTY_PLUGIN_COMPONENTS })
//#then
const mergedMcp = config.mcp as Record<string, Record<string, unknown>>
expect(mergedMcp).toHaveProperty("userServer")
expect(mergedMcp).toHaveProperty("claudeServer")
expect(mergedMcp.userServer.enabled).toBe(true)
expect(mergedMcp.claudeServer.enabled).toBe(true)
expect(logSpy).not.toHaveBeenCalledWith(expect.stringContaining("overrides Claude Code"))
})
test("user config wins on collision with Claude Code and logs warning", async () => {
//#given
const userMcp = {
sharedServer: { type: "remote", url: "https://user.example.com", enabled: true },
}
loadMcpConfigsSpy.mockResolvedValue({
servers: {
sharedServer: { type: "remote", url: "https://claude.example.com", enabled: true },
},
loadedServers: [],
})
const config: Record<string, unknown> = { mcp: userMcp }
const pluginConfig = createPluginConfig()
//#when
const { applyMcpConfig } = await import("./mcp-config-handler")
await applyMcpConfig({ config, pluginConfig, pluginComponents: EMPTY_PLUGIN_COMPONENTS })
//#then
const mergedMcp = config.mcp as Record<string, Record<string, unknown>>
expect(mergedMcp.sharedServer.url).toBe("https://user.example.com")
expect(logSpy).toHaveBeenCalledWith(
'warning: MCP server "sharedServer" from user config overrides Claude Code .mcp.json'
)
})
test("preserves enabled:false from user config after collision with Claude Code", async () => {
//#given
const userMcp = {
sharedServer: { type: "remote", url: "https://user.example.com", enabled: false },
}
loadMcpConfigsSpy.mockResolvedValue({
servers: {
sharedServer: { type: "remote", url: "https://claude.example.com", enabled: true },
},
loadedServers: [],
})
const config: Record<string, unknown> = { mcp: userMcp }
const pluginConfig = createPluginConfig()
//#when
const { applyMcpConfig } = await import("./mcp-config-handler")
await applyMcpConfig({ config, pluginConfig, pluginComponents: EMPTY_PLUGIN_COMPONENTS })
//#then
const mergedMcp = config.mcp as Record<string, Record<string, unknown>>
expect(mergedMcp.sharedServer.enabled).toBe(false)
expect(mergedMcp.sharedServer.url).toBe("https://user.example.com")
expect(logSpy).toHaveBeenCalledWith(
'warning: MCP server "sharedServer" from user config overrides Claude Code .mcp.json'
)
})
})
@@ -164,4 +164,5 @@ describe("applyMcpConfig", () => {
const mergedMcp = config.mcp as Record<string, Record<string, unknown>>
expect(mergedMcp).not.toHaveProperty("plugin:custom")
})
})
+10 -1
View File
@@ -2,6 +2,7 @@ import type { OhMyOpenCodeConfig } from "../config";
import { loadMcpConfigs } from "../features/claude-code-mcp-loader";
import { createBuiltinMcps } from "../mcp";
import type { PluginComponents } from "./plugin-components-loader";
import { log } from "../shared";
type McpEntry = Record<string, unknown>;
@@ -38,10 +39,18 @@ export async function applyMcpConfig(params: {
? await loadMcpConfigs(disabledMcps)
: { servers: {} };
if (userMcp) {
for (const name of Object.keys(userMcp)) {
if (name in mcpResult.servers) {
log(`warning: MCP server "${name}" from user config overrides Claude Code .mcp.json`);
}
}
}
const merged = {
...createBuiltinMcps(disabledMcps, params.pluginConfig),
...(userMcp ?? {}),
...mcpResult.servers,
...(userMcp ?? {}),
...params.pluginComponents.mcpServers,
} as Record<string, McpEntry>;