feat(claude-code-mcp-loader): auto-disable builtin skills with overlapping MCP servers
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)
This commit is contained in:
YeonGyu-Kim
162
src/features/claude-code-mcp-loader/loader.test.ts
Normal file
162
src/features/claude-code-mcp-loader/loader.test.ts
Normal file
@@ -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)
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -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<string> {
|
||||
const names = new Set<string>()
|
||||
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<McpLoadResult> {
|
||||
const servers: McpLoadResult["servers"] = {}
|
||||
const loadedServers: LoadedMcpServer[] = []
|
||||
|
||||
120
src/index.ts
120
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<Record<string, unknown>>(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,
|
||||
|
||||
Reference in New Issue
Block a user