From 394eaaeae20675a3fe50a74a4e17d0de4e75b38b Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 27 Jul 2026 10:31:18 +0000 Subject: [PATCH] fix(mcp): filter project scope before dedup Applied the project-scope filter before connection-equivalence deduplication so a project server can no longer shadow a differently-named but equivalent user server and then be dropped, leaving none. Fixes #6786 --- packages/coding-agent/src/capability/index.ts | 4 + packages/coding-agent/src/capability/types.ts | 6 ++ packages/coding-agent/src/mcp/config.ts | 15 ++-- .../test/mcp-config-scope-dedup.test.ts | 73 +++++++++++++++++++ 4 files changed, 91 insertions(+), 7 deletions(-) create mode 100644 packages/coding-agent/test/mcp-config-scope-dedup.test.ts diff --git a/packages/coding-agent/src/capability/index.ts b/packages/coding-agent/src/capability/index.ts index 938602218..3c1403cb2 100644 --- a/packages/coding-agent/src/capability/index.ts +++ b/packages/coding-agent/src/capability/index.ts @@ -154,6 +154,10 @@ async function loadImpl( continue; } + if (options.filter && !options.filter(itemWithSource)) { + continue; + } + itemWithSource._source.providerName = provider.displayName; allItems.push(itemWithSource as T & { _source: SourceMeta; _shadowed?: boolean }); contributedItemCount += 1; diff --git a/packages/coding-agent/src/capability/types.ts b/packages/coding-agent/src/capability/types.ts index 5f06c08d2..661eb4d9e 100644 --- a/packages/coding-agent/src/capability/types.ts +++ b/packages/coding-agent/src/capability/types.ts @@ -72,6 +72,12 @@ export interface LoadOptions { includeDisabled?: boolean; /** Explicit disabled extension IDs to apply instead of settings. */ disabledExtensions?: string[]; + /** + * Drop items before deduplication. Use when a downstream scope filter must + * precede equivalence collapse, so a to-be-filtered item cannot shadow a + * differently-keyed but equivalent survivor. Receives the item's `_source`. + */ + filter?(item: { _source: SourceMeta }): boolean; } /** diff --git a/packages/coding-agent/src/mcp/config.ts b/packages/coding-agent/src/mcp/config.ts index 39872005b..34d77db1b 100644 --- a/packages/coding-agent/src/mcp/config.ts +++ b/packages/coding-agent/src/mcp/config.ts @@ -97,13 +97,14 @@ export async function loadAllMCPConfigs(cwd: string, options?: LoadMCPConfigsOpt const filterExa = options?.filterExa ?? true; const filterBrowser = options?.filterBrowser ?? false; - // Load MCP servers via capability system - const result = await loadCapability(mcpCapability.id, { cwd }); - - // Filter out project-level configs if disabled - const servers = enableProjectConfig - ? result.items - : result.items.filter(server => server._source.level !== "project"); + // Filter project-scoped entries BEFORE the capability layer's equivalence + // deduplication so a project server cannot shadow a differently-named but + // connection-equivalent user server and then be dropped here, leaving none. + const result = await loadCapability(mcpCapability.id, { + cwd, + ...(enableProjectConfig ? {} : { filter: server => server._source.level !== "project" }), + }); + const servers = result.items; // Load user-level disable/force-enable lists. The denylist always wins; the // allowlist overrides a non-writable source config's `enabled: false`. diff --git a/packages/coding-agent/test/mcp-config-scope-dedup.test.ts b/packages/coding-agent/test/mcp-config-scope-dedup.test.ts new file mode 100644 index 000000000..1146b9bee --- /dev/null +++ b/packages/coding-agent/test/mcp-config-scope-dedup.test.ts @@ -0,0 +1,73 @@ +/** + * Regression: project-scope filtering must run BEFORE MCP connection-equivalence + * deduplication. The native provider orders project entries before user entries, + * so a project server can shadow a differently-named but connection-equivalent + * user server during dedup. When `enableProjectConfig` is false the project entry + * is then removed, and without pre-dedup scope filtering no server would survive. + */ +import { afterEach, beforeEach, describe, expect, test, vi } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { clearCache as clearFsCache } from "@oh-my-pi/pi-coding-agent/capability/fs"; +import { loadAllMCPConfigs } from "@oh-my-pi/pi-coding-agent/mcp/config"; +import { getConfigRootDir, removeWithRetries, setAgentDir } from "@oh-my-pi/pi-utils"; +import "@oh-my-pi/pi-coding-agent/discovery/builtin"; + +const originalAgentDirEnv = process.env.PI_CODING_AGENT_DIR; +const fallbackAgentDir = path.join(getConfigRootDir(), "agent"); +const CONNECTION = { type: "http", url: "https://mcp.example/mcp" } as const; + +async function writeMcpJson(dir: string, servers: Record): Promise { + await fs.mkdir(dir, { recursive: true }); + await fs.writeFile(path.join(dir, "mcp.json"), JSON.stringify({ mcpServers: servers }, null, 2)); +} + +describe("MCP scope filtering precedes connection-equivalence deduplication", () => { + let tempHome = ""; + let projectDir = ""; + let userAgentDir = ""; + let originalHome: string | undefined; + + beforeEach(async () => { + originalHome = process.env.HOME; + tempHome = await fs.mkdtemp(path.join(os.tmpdir(), "omp-mcp-scope-home-")); + projectDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-mcp-scope-project-")); + userAgentDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-mcp-scope-agent-")); + process.env.HOME = tempHome; + vi.spyOn(os, "homedir").mockReturnValue(tempHome); + setAgentDir(userAgentDir); + clearFsCache(); + // Same connection identity under two distinct names, one per scope. + await writeMcpJson(path.join(projectDir, ".omp"), { projcontext: CONNECTION }); + await writeMcpJson(userAgentDir, { usercontext: CONNECTION }); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + clearFsCache(); + if (originalAgentDirEnv) { + setAgentDir(originalAgentDirEnv); + } else { + setAgentDir(fallbackAgentDir); + delete process.env.PI_CODING_AGENT_DIR; + } + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + await removeWithRetries(tempHome); + await removeWithRetries(projectDir); + await removeWithRetries(userAgentDir); + }); + + test("keeps the user server when project config is disabled", async () => { + const result = await loadAllMCPConfigs(projectDir, { enableProjectConfig: false, filterExa: false }); + expect(Object.keys(result.configs)).toEqual(["usercontext"]); + expect(result.sources.usercontext?.level).toBe("user"); + }); + + test("collapses the equivalent alias to the higher-priority project name when enabled", async () => { + const result = await loadAllMCPConfigs(projectDir, { enableProjectConfig: true, filterExa: false }); + expect(Object.keys(result.configs)).toEqual(["projcontext"]); + expect(result.sources.projcontext?.level).toBe("project"); + }); +});