fix(mcp): preserve user's enabled:false and apply disabled_mcps to all MCP sources
Commit 598a4389 refactored config-handler into separate modules but
dropped the disabledMcps parameter from loadMcpConfigs() and did not
handle the spread-order overwrite where .mcp.json MCPs (hardcoded
enabled:true) overwrote user's enabled:false from opencode.json.
Changes:
- Re-add disabledMcps parameter to loadMcpConfigs() in loader.ts
- Capture user's enabled:false MCPs before merge, restore after
- Pass disabled_mcps to loadMcpConfigs for .mcp.json filtering
- Delete disabled_mcps entries from final merged result
- Add 8 new tests covering both fixes
This commit is contained in:
@@ -229,5 +229,109 @@ describe("getSystemMcpServerNames", () => {
|
||||
} finally {
|
||||
process.chdir(originalCwd)
|
||||
}
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("loadMcpConfigs", () => {
|
||||
beforeEach(() => {
|
||||
mkdirSync(TEST_DIR, { recursive: true })
|
||||
mkdirSync(TEST_HOME, { recursive: true })
|
||||
mock.module("os", () => ({
|
||||
homedir: () => TEST_HOME,
|
||||
tmpdir,
|
||||
}))
|
||||
mock.module("../../shared", () => ({
|
||||
getClaudeConfigDir: () => join(TEST_HOME, ".claude"),
|
||||
}))
|
||||
mock.module("../../shared/logger", () => ({
|
||||
log: () => {},
|
||||
}))
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
mock.restore()
|
||||
rmSync(TEST_DIR, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it("should skip MCPs in disabledMcps list", async () => {
|
||||
//#given
|
||||
const mcpConfig = {
|
||||
mcpServers: {
|
||||
playwright: { command: "npx", args: ["@playwright/mcp@latest"] },
|
||||
sqlite: { command: "uvx", args: ["mcp-server-sqlite"] },
|
||||
active: { command: "npx", args: ["some-mcp"] },
|
||||
},
|
||||
}
|
||||
writeFileSync(join(TEST_DIR, ".mcp.json"), JSON.stringify(mcpConfig))
|
||||
|
||||
const originalCwd = process.cwd()
|
||||
process.chdir(TEST_DIR)
|
||||
|
||||
try {
|
||||
//#when
|
||||
const { loadMcpConfigs } = await import("./loader")
|
||||
const result = await loadMcpConfigs(["playwright", "sqlite"])
|
||||
|
||||
//#then
|
||||
expect(result.servers).not.toHaveProperty("playwright")
|
||||
expect(result.servers).not.toHaveProperty("sqlite")
|
||||
expect(result.servers).toHaveProperty("active")
|
||||
expect(result.loadedServers.find((s) => s.name === "playwright")).toBeUndefined()
|
||||
expect(result.loadedServers.find((s) => s.name === "sqlite")).toBeUndefined()
|
||||
expect(result.loadedServers.find((s) => s.name === "active")).toBeDefined()
|
||||
} finally {
|
||||
process.chdir(originalCwd)
|
||||
}
|
||||
})
|
||||
|
||||
it("should load all MCPs when disabledMcps is empty", async () => {
|
||||
//#given
|
||||
const mcpConfig = {
|
||||
mcpServers: {
|
||||
playwright: { command: "npx", args: ["@playwright/mcp@latest"] },
|
||||
active: { command: "npx", args: ["some-mcp"] },
|
||||
},
|
||||
}
|
||||
writeFileSync(join(TEST_DIR, ".mcp.json"), JSON.stringify(mcpConfig))
|
||||
|
||||
const originalCwd = process.cwd()
|
||||
process.chdir(TEST_DIR)
|
||||
|
||||
try {
|
||||
//#when
|
||||
const { loadMcpConfigs } = await import("./loader")
|
||||
const result = await loadMcpConfigs([])
|
||||
|
||||
//#then
|
||||
expect(result.servers).toHaveProperty("playwright")
|
||||
expect(result.servers).toHaveProperty("active")
|
||||
} finally {
|
||||
process.chdir(originalCwd)
|
||||
}
|
||||
})
|
||||
|
||||
it("should load all MCPs when disabledMcps is not provided", async () => {
|
||||
//#given
|
||||
const mcpConfig = {
|
||||
mcpServers: {
|
||||
playwright: { command: "npx", args: ["@playwright/mcp@latest"] },
|
||||
},
|
||||
}
|
||||
writeFileSync(join(TEST_DIR, ".mcp.json"), JSON.stringify(mcpConfig))
|
||||
|
||||
const originalCwd = process.cwd()
|
||||
process.chdir(TEST_DIR)
|
||||
|
||||
try {
|
||||
//#when
|
||||
const { loadMcpConfigs } = await import("./loader")
|
||||
const result = await loadMcpConfigs()
|
||||
|
||||
//#then
|
||||
expect(result.servers).toHaveProperty("playwright")
|
||||
} finally {
|
||||
process.chdir(originalCwd)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -68,16 +68,24 @@ export function getSystemMcpServerNames(): Set<string> {
|
||||
return names
|
||||
}
|
||||
|
||||
export async function loadMcpConfigs(): Promise<McpLoadResult> {
|
||||
export async function loadMcpConfigs(
|
||||
disabledMcps: string[] = []
|
||||
): Promise<McpLoadResult> {
|
||||
const servers: McpLoadResult["servers"] = {}
|
||||
const loadedServers: LoadedMcpServer[] = []
|
||||
const paths = getMcpConfigPaths()
|
||||
const disabledSet = new Set(disabledMcps)
|
||||
|
||||
for (const { path, scope } of paths) {
|
||||
const config = await loadMcpConfigFile(path)
|
||||
if (!config?.mcpServers) continue
|
||||
|
||||
for (const [name, serverConfig] of Object.entries(config.mcpServers)) {
|
||||
if (disabledSet.has(name)) {
|
||||
log(`Skipping MCP "${name}" (in disabled_mcps)`, { path })
|
||||
continue
|
||||
}
|
||||
|
||||
if (serverConfig.disabled) {
|
||||
log(`Disabling MCP server "${name}"`, { path })
|
||||
delete servers[name]
|
||||
|
||||
Reference in New Issue
Block a user