From 4f63e4e5218a933f04e34764566c8a429654cd48 Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 28 Jul 2026 07:24:49 +0000 Subject: [PATCH] fix(discovery): honored inline plugin MCP manifest maps Treated an object-valued mcpServers manifest field as the server map, rooting relative stdio paths at the plugin root, so plugins declaring servers inline no longer fell through to .mcp.json. Fixes #6871 --- packages/coding-agent/CHANGELOG.md | 2 +- .../src/discovery/claude-plugins.ts | 130 +++++++++++------- .../test/discovery/claude-plugins.test.ts | 41 ++++++ 3 files changed, 126 insertions(+), 47 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6a66e85bf..e2a4a7ed4 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed marketplace plugin MCP discovery ignoring `mcpServers` file pointers in `.omp-plugin/plugin.json` and `.claude-plugin/plugin.json`; OMP-specific manifests now take precedence before the conventional root `.mcp.json` fallback ([#6871](https://github.com/can1357/oh-my-pi/issues/6871)). +- Fixed marketplace plugin MCP discovery ignoring the `mcpServers` manifest field in `.omp-plugin/plugin.json` and `.claude-plugin/plugin.json`; both an inline server-map object and a file-path pointer are now honored (OMP manifest first) before the conventional root `.mcp.json` fallback ([#6871](https://github.com/can1357/oh-my-pi/issues/6871)). ## [17.1.7] - 2026-07-27 diff --git a/packages/coding-agent/src/discovery/claude-plugins.ts b/packages/coding-agent/src/discovery/claude-plugins.ts index 127c00925..2d33562b4 100644 --- a/packages/coding-agent/src/discovery/claude-plugins.ts +++ b/packages/coding-agent/src/discovery/claude-plugins.ts @@ -42,7 +42,14 @@ interface ResolvedPluginDir { } interface ResolvedMCPConfig { + /** On-disk config file to read, or null when servers are inline or nothing applies. */ path: string | null; + /** Server map declared inline in the plugin manifest, or null when the source is a file. */ + inlineServers: Record | null; + /** Path recorded as each discovered server's capability source. */ + sourcePath: string; + /** Directory that relative stdio `command`/`cwd` values resolve against. */ + baseDir: string; warnings: string[]; } @@ -380,7 +387,30 @@ async function loadTools(ctx: LoadContext): Promise> { // MCP Servers // ============================================================================= +/** + * Unwrap a parsed MCP config file to its server map. Supports the nested + * `{ mcpServers: { … } }` project shape and the flat `{ name: cfg, … }` + * marketplace-plugin shape. Returns null when a `mcpServers` field is present + * but not an object map (malformed) so the caller skips the file. + */ +function extractServerMap(obj: Record): Record | null { + if (isRecord(obj.mcpServers)) return obj.mcpServers; + if (!("mcpServers" in obj)) return obj; + return null; +} + +/** + * Resolve where a plugin's MCP servers come from, honoring the manifest's + * `mcpServers` field before the conventional root `.mcp.json`. + * + * `.omp-plugin/plugin.json` takes precedence over `.claude-plugin/plugin.json`. + * The field may be an inline object (the server map itself) or a string path to + * a config file within the plugin root; a path escaping the root is rejected + * with a warning. When no manifest declares the field, `/.mcp.json` is the + * fallback source. + */ async function resolvePluginMCPConfig(root: ClaudePluginRoot): Promise { + const fallback = path.join(root.path, ".mcp.json"); for (const manifestDir of [".omp-plugin", ".claude-plugin"]) { const manifestPath = path.join(root.path, manifestDir, "plugin.json"); const raw = await readFile(manifestPath); @@ -392,21 +422,40 @@ async function resolvePluginMCPConfig(root: ClaudePluginRoot): Promise> { @@ -419,43 +468,35 @@ async function loadMCPServers(ctx: LoadContext): Promise> for (const root of roots) { const resolved = await resolvePluginMCPConfig(root); warnings.push(...resolved.warnings); - if (resolved.path === null) continue; - const mcpPath = resolved.path; - const raw = await readFile(mcpPath); - if (raw === null) continue; // file absent — skip silently - let parsed: unknown; - try { - parsed = JSON.parse(raw); - } catch { - warnings.push(`[claude-plugins] Invalid JSON in ${mcpPath}`); - logger.warn(`[claude-plugins] Invalid JSON in ${mcpPath}`); - continue; - } + let servers: Record | null; + if (resolved.inlineServers) { + servers = resolved.inlineServers; + } else if (resolved.path !== null) { + const raw = await readFile(resolved.path); + if (raw === null) continue; // file absent — skip silently - if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) continue; - const obj = parsed as Record; - - // Two shapes are supported: - // nested: { "mcpServers": { name: cfg, ... } } (OMP/Claude Code project shape) - // flat: { name: cfg, ... } (Claude marketplace plugin shape) - // If "mcpServers" is present and an object, treat it as the canonical map. - // Otherwise, treat the whole object as the server map. - let servers: Record; - if ( - obj.mcpServers !== undefined && - obj.mcpServers !== null && - typeof obj.mcpServers === "object" && - !Array.isArray(obj.mcpServers) - ) { - servers = obj.mcpServers as Record; - } else if (!("mcpServers" in obj)) { - servers = obj; + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + warnings.push(`[claude-plugins] Invalid JSON in ${resolved.path}`); + logger.warn(`[claude-plugins] Invalid JSON in ${resolved.path}`); + continue; + } + // Two file shapes are supported: + // nested: { "mcpServers": { name: cfg, ... } } (OMP/Claude Code project shape) + // flat: { name: cfg, ... } (Claude marketplace plugin shape) + if (!isRecord(parsed)) continue; + servers = extractServerMap(parsed); } else { continue; } + if (servers === null) continue; - for (const [serverName, serverCfg] of Object.entries(servers)) { + const { sourcePath, baseDir } = resolved; + for (const serverName in servers) { + const serverCfg = servers[serverName]; if (!serverCfg || typeof serverCfg !== "object" || Array.isArray(serverCfg)) continue; const raw = serverCfg as { enabled?: boolean; @@ -474,7 +515,7 @@ async function loadMCPServers(ctx: LoadContext): Promise> // occasionally ship .mcp.json entries with neither, which would register a useless // server and surface as a connection error at runtime. if (typeof raw.command !== "string" && typeof raw.url !== "string") { - warnings.push(`[claude-plugins] Skipping MCP server "${serverName}" in ${mcpPath}: missing command or url`); + warnings.push(`[claude-plugins] Skipping MCP server "${serverName}" in ${sourcePath}: missing command or url`); continue; } const namespacedName = root.plugin ? `${root.plugin}:${serverName}` : serverName; @@ -483,10 +524,7 @@ async function loadMCPServers(ctx: LoadContext): Promise> const substitutedCwd = raw.cwd !== undefined ? substitutePluginRoot(raw.cwd, root.path) : undefined; // Root relative command/cwd at the plugin's config directory, not the // session cwd (MCP stdio spawning resolves relative values there). - const rooted = resolvePluginStdioPaths( - { command: substitutedCommand, cwd: substitutedCwd }, - path.dirname(mcpPath), - ); + const rooted = resolvePluginStdioPaths({ command: substitutedCommand, cwd: substitutedCwd }, baseDir); const server: MCPServer = { name: namespacedName, ...(raw.enabled !== undefined && { enabled: raw.enabled }), @@ -500,7 +538,7 @@ async function loadMCPServers(ctx: LoadContext): Promise> ...(raw.auth !== undefined && { auth: raw.auth }), ...(raw.oauth !== undefined && { oauth: raw.oauth }), ...(raw.type !== undefined && { transport: raw.type as MCPServer["transport"] }), - _source: createSourceMeta(PROVIDER_ID, mcpPath, root.scope), + _source: createSourceMeta(PROVIDER_ID, sourcePath, root.scope), }; items.push(server); } diff --git a/packages/coding-agent/test/discovery/claude-plugins.test.ts b/packages/coding-agent/test/discovery/claude-plugins.test.ts index 45bb58284..01153d4d4 100644 --- a/packages/coding-agent/test/discovery/claude-plugins.test.ts +++ b/packages/coding-agent/test/discovery/claude-plugins.test.ts @@ -565,6 +565,47 @@ describe("listClaudePluginRoots", () => { ]); }); + test("loads inline manifest mcpServers object and roots relative stdio at plugin root", async () => { + const pluginsDir = path.join(tempDir, ".claude", "plugins"); + const pluginPath = path.join(tempDir, "plugins", "inline-mcp"); + await fs.mkdir(pluginsDir, { recursive: true }); + await fs.mkdir(path.join(pluginPath, ".omp-plugin"), { recursive: true }); + await fs.writeFile( + path.join(pluginsDir, "installed_plugins.json"), + JSON.stringify({ + version: 2, + plugins: { + "inline-mcp@market": [ + { + scope: "user", + installPath: pluginPath, + version: "1.0.0", + installedAt: "2026-07-28T00:00:00Z", + lastUpdated: "2026-07-28T00:00:00Z", + }, + ], + }, + }), + ); + // Inline object form: the manifest carries the server map directly, and no + // root .mcp.json exists, so the pre-fix fallback would register nothing. + await fs.writeFile( + path.join(pluginPath, ".omp-plugin", "plugin.json"), + JSON.stringify({ mcpServers: { local: { command: "./bin/server", args: ["run"] } } }), + ); + + const result = await loadCapability(mcpCapability.id, { + cwd: tempDir, + providers: ["claude-plugins"], + }); + + expect(result.warnings).toEqual([]); + const server = result.all.find(item => item.name === "inline-mcp:local"); + expect(server).toBeDefined(); + expect(server?.command).toBe(path.join(pluginPath, "bin", "server")); + expect(server?.args).toEqual(["run"]); + }); + test("deduplicates a plugin alias of a directly configured MCP connection", async () => { const pluginsDir = path.join(tempDir, ".claude", "plugins"); const pluginPath = path.join(tempDir, "plugins", "context7");