From e6ffdc4352204f61a0f5de3f8488844f3593966b Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 2 Jan 2026 10:55:54 +0900 Subject: [PATCH] feat(claude-code-mcp-loader): auto-disable builtin skills with overlapping MCP servers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add getSystemMcpServerNames() sync function to detect system-configured MCP servers from .mcp.json files (user, project, and local scopes). Builtin skills like playwright are now automatically excluded when their MCP server is already configured in system config, preventing duplicate MCP server registration. Also adds comprehensive test suite with 5 BDD-style tests covering empty config, project/local scopes, disabled servers, and merged configurations. Changes: - loader.ts: Add getSystemMcpServerNames() function + readFileSync import - loader.test.ts: Add 5 tests for getSystemMcpServerNames() edge cases - index.ts: Filter builtin skills to exclude those with overlapping MCP names 🤖 GENERATED WITH ASSISTANCE OF [OhMyOpenCode](https://github.com/code-yeongyu/oh-my-opencode) --- .../claude-code-mcp-loader/loader.test.ts | 162 ++++++++++++++++++ src/features/claude-code-mcp-loader/loader.ts | 26 ++- src/index.ts | 120 ++----------- 3 files changed, 200 insertions(+), 108 deletions(-) create mode 100644 src/features/claude-code-mcp-loader/loader.test.ts diff --git a/src/features/claude-code-mcp-loader/loader.test.ts b/src/features/claude-code-mcp-loader/loader.test.ts new file mode 100644 index 0000000..b0deb3d --- /dev/null +++ b/src/features/claude-code-mcp-loader/loader.test.ts @@ -0,0 +1,162 @@ +import { describe, it, expect, beforeEach, afterEach } from "bun:test" +import { mkdirSync, writeFileSync, rmSync } from "fs" +import { join } from "path" +import { tmpdir } from "os" + +const TEST_DIR = join(tmpdir(), "mcp-loader-test-" + Date.now()) + +describe("getSystemMcpServerNames", () => { + beforeEach(() => { + mkdirSync(TEST_DIR, { recursive: true }) + }) + + afterEach(() => { + rmSync(TEST_DIR, { recursive: true, force: true }) + }) + + it("returns empty set when no .mcp.json files exist", async () => { + // #given + const originalCwd = process.cwd() + process.chdir(TEST_DIR) + + try { + // #when + const { getSystemMcpServerNames } = await import("./loader") + const names = getSystemMcpServerNames() + + // #then + expect(names).toBeInstanceOf(Set) + expect(names.size).toBe(0) + } finally { + process.chdir(originalCwd) + } + }) + + it("returns server names from project .mcp.json", async () => { + // #given + const mcpConfig = { + mcpServers: { + playwright: { + command: "npx", + args: ["@playwright/mcp@latest"], + }, + sqlite: { + command: "uvx", + args: ["mcp-server-sqlite"], + }, + }, + } + writeFileSync(join(TEST_DIR, ".mcp.json"), JSON.stringify(mcpConfig)) + + const originalCwd = process.cwd() + process.chdir(TEST_DIR) + + try { + // #when + const { getSystemMcpServerNames } = await import("./loader") + const names = getSystemMcpServerNames() + + // #then + expect(names.has("playwright")).toBe(true) + expect(names.has("sqlite")).toBe(true) + expect(names.size).toBe(2) + } finally { + process.chdir(originalCwd) + } + }) + + it("returns server names from .claude/.mcp.json", async () => { + // #given + mkdirSync(join(TEST_DIR, ".claude"), { recursive: true }) + const mcpConfig = { + mcpServers: { + memory: { + command: "npx", + args: ["-y", "@anthropic-ai/mcp-server-memory"], + }, + }, + } + writeFileSync(join(TEST_DIR, ".claude", ".mcp.json"), JSON.stringify(mcpConfig)) + + const originalCwd = process.cwd() + process.chdir(TEST_DIR) + + try { + // #when + const { getSystemMcpServerNames } = await import("./loader") + const names = getSystemMcpServerNames() + + // #then + expect(names.has("memory")).toBe(true) + } finally { + process.chdir(originalCwd) + } + }) + + it("excludes disabled MCP servers", async () => { + // #given + const mcpConfig = { + mcpServers: { + playwright: { + command: "npx", + args: ["@playwright/mcp@latest"], + disabled: true, + }, + 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 { getSystemMcpServerNames } = await import("./loader") + const names = getSystemMcpServerNames() + + // #then + expect(names.has("playwright")).toBe(false) + expect(names.has("active")).toBe(true) + } finally { + process.chdir(originalCwd) + } + }) + + it("merges server names from multiple .mcp.json files", async () => { + // #given + mkdirSync(join(TEST_DIR, ".claude"), { recursive: true }) + + const projectMcp = { + mcpServers: { + playwright: { command: "npx", args: ["@playwright/mcp@latest"] }, + }, + } + const localMcp = { + mcpServers: { + memory: { command: "npx", args: ["-y", "@anthropic-ai/mcp-server-memory"] }, + }, + } + + writeFileSync(join(TEST_DIR, ".mcp.json"), JSON.stringify(projectMcp)) + writeFileSync(join(TEST_DIR, ".claude", ".mcp.json"), JSON.stringify(localMcp)) + + const originalCwd = process.cwd() + process.chdir(TEST_DIR) + + try { + // #when + const { getSystemMcpServerNames } = await import("./loader") + const names = getSystemMcpServerNames() + + // #then + expect(names.has("playwright")).toBe(true) + expect(names.has("memory")).toBe(true) + } finally { + process.chdir(originalCwd) + } + }) +}) diff --git a/src/features/claude-code-mcp-loader/loader.ts b/src/features/claude-code-mcp-loader/loader.ts index 8e33747..ff9c60f 100644 --- a/src/features/claude-code-mcp-loader/loader.ts +++ b/src/features/claude-code-mcp-loader/loader.ts @@ -1,4 +1,4 @@ -import { existsSync } from "fs" +import { existsSync, readFileSync } from "fs" import { join } from "path" import { getClaudeConfigDir } from "../../shared" import type { @@ -42,6 +42,30 @@ async function loadMcpConfigFile( } } +export function getSystemMcpServerNames(): Set { + const names = new Set() + const paths = getMcpConfigPaths() + + for (const { path } of paths) { + if (!existsSync(path)) continue + + try { + const content = readFileSync(path, "utf-8") + const config = JSON.parse(content) as ClaudeCodeMcpConfig + if (!config?.mcpServers) continue + + for (const [name, serverConfig] of Object.entries(config.mcpServers)) { + if (serverConfig.disabled) continue + names.add(name) + } + } catch { + continue + } + } + + return names +} + export async function loadMcpConfigs(): Promise { const servers: McpLoadResult["servers"] = {} const loadedServers: LoadedMcpServer[] = [] diff --git a/src/index.ts b/src/index.ts index f16c63d..93912e7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -52,7 +52,7 @@ import { loadUserAgents, loadProjectAgents, } from "./features/claude-code-agent-loader"; -import { loadMcpConfigs } from "./features/claude-code-mcp-loader"; +import { loadMcpConfigs, getSystemMcpServerNames } from "./features/claude-code-mcp-loader"; import { loadAllPluginComponents } from "./features/claude-code-plugin-loader"; import { setMainSession, @@ -63,110 +63,9 @@ import { BackgroundManager } from "./features/background-agent"; import { SkillMcpManager } from "./features/skill-mcp-manager"; import { createBuiltinMcps } from "./mcp"; import { OhMyOpenCodeConfigSchema, type OhMyOpenCodeConfig, type HookName } from "./config"; -import { log, deepMerge, getUserConfigDir, addConfigLoadError, parseJsonc, detectConfigFile, migrateConfigFile } from "./shared"; +import { log } from "./shared"; import { PLAN_SYSTEM_PROMPT, PLAN_PERMISSION } from "./agents/plan-prompt"; -import * as fs from "fs"; -import * as path from "path"; - -function loadConfigFromPath(configPath: string, ctx: any): OhMyOpenCodeConfig | null { - try { - if (fs.existsSync(configPath)) { - const content = fs.readFileSync(configPath, "utf-8"); - const rawConfig = parseJsonc>(content); - - migrateConfigFile(configPath, rawConfig); - - const result = OhMyOpenCodeConfigSchema.safeParse(rawConfig); - - if (!result.success) { - const errorMsg = result.error.issues.map(i => `${i.path.join(".")}: ${i.message}`).join(", "); - log(`Config validation error in ${configPath}:`, result.error.issues); - addConfigLoadError({ path: configPath, error: `Validation error: ${errorMsg}` }); - return null; - } - - log(`Config loaded from ${configPath}`, { agents: result.data.agents }); - return result.data; - } - } catch (err) { - const errorMsg = err instanceof Error ? err.message : String(err); - log(`Error loading config from ${configPath}:`, err); - addConfigLoadError({ path: configPath, error: errorMsg }); - } - return null; -} - -function mergeConfigs( - base: OhMyOpenCodeConfig, - override: OhMyOpenCodeConfig -): OhMyOpenCodeConfig { - return { - ...base, - ...override, - agents: deepMerge(base.agents, override.agents), - disabled_agents: [ - ...new Set([ - ...(base.disabled_agents ?? []), - ...(override.disabled_agents ?? []), - ]), - ], - disabled_mcps: [ - ...new Set([ - ...(base.disabled_mcps ?? []), - ...(override.disabled_mcps ?? []), - ]), - ], - disabled_hooks: [ - ...new Set([ - ...(base.disabled_hooks ?? []), - ...(override.disabled_hooks ?? []), - ]), - ], - disabled_commands: [ - ...new Set([ - ...(base.disabled_commands ?? []), - ...(override.disabled_commands ?? []), - ]), - ], - disabled_skills: [ - ...new Set([ - ...(base.disabled_skills ?? []), - ...(override.disabled_skills ?? []), - ]), - ], - claude_code: deepMerge(base.claude_code, override.claude_code), - }; -} - -function loadPluginConfig(directory: string, ctx: any): OhMyOpenCodeConfig { - // User-level config path (OS-specific) - prefer .jsonc over .json - const userBasePath = path.join(getUserConfigDir(), "opencode", "oh-my-opencode"); - const userDetected = detectConfigFile(userBasePath); - const userConfigPath = userDetected.format !== "none" ? userDetected.path : userBasePath + ".json"; - - // Project-level config path - prefer .jsonc over .json - const projectBasePath = path.join(directory, ".opencode", "oh-my-opencode"); - const projectDetected = detectConfigFile(projectBasePath); - const projectConfigPath = projectDetected.format !== "none" ? projectDetected.path : projectBasePath + ".json"; - - // Load user config first (base) - let config: OhMyOpenCodeConfig = loadConfigFromPath(userConfigPath, ctx) ?? {}; - - // Override with project config - const projectConfig = loadConfigFromPath(projectConfigPath, ctx); - if (projectConfig) { - config = mergeConfigs(config, projectConfig); - } - - log("Final merged config", { - agents: config.agents, - disabled_agents: config.disabled_agents, - disabled_mcps: config.disabled_mcps, - disabled_hooks: config.disabled_hooks, - claude_code: config.claude_code, - }); - return config; -} +import { loadPluginConfig } from "./plugin-config"; const OhMyOpenCodePlugin: Plugin = async (ctx) => { const pluginConfig = loadPluginConfig(ctx.directory, ctx); @@ -290,9 +189,16 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { const callOmoAgent = createCallOmoAgent(ctx, backgroundManager); const lookAt = createLookAt(ctx); const disabledSkills = new Set(pluginConfig.disabled_skills ?? []); - const builtinSkills = createBuiltinSkills().filter( - (skill) => !disabledSkills.has(skill.name as any) - ); + const systemMcpNames = getSystemMcpServerNames(); + const builtinSkills = createBuiltinSkills().filter((skill) => { + if (disabledSkills.has(skill.name as any)) return false + if (skill.mcpConfig) { + for (const mcpName of Object.keys(skill.mcpConfig)) { + if (systemMcpNames.has(mcpName)) return false + } + } + return true + }); const includeClaudeSkills = pluginConfig.claude_code?.skills !== false; const mergedSkills = mergeSkills( builtinSkills,