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
This commit is contained in:
@@ -154,6 +154,10 @@ async function loadImpl<T>(
|
||||
continue;
|
||||
}
|
||||
|
||||
if (options.filter && !options.filter(itemWithSource)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
itemWithSource._source.providerName = provider.displayName;
|
||||
allItems.push(itemWithSource as T & { _source: SourceMeta; _shadowed?: boolean });
|
||||
contributedItemCount += 1;
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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<MCPServer>(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<MCPServer>(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`.
|
||||
|
||||
@@ -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<string, unknown>): Promise<void> {
|
||||
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");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user