diff --git a/package.json b/package.json index c7f83dad5..fd99a34e9 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,7 @@ "copy-themes": "node scripts/copy-themes.cjs", "copy-export-html": "node scripts/copy-export-html.cjs", "test:compile": "node scripts/compile-tests.mjs", - "test:unit": "npm run test:compile && node --import ./scripts/dist-test-resolve.mjs --experimental-test-isolation=process --test-reporter=./scripts/test-reporter-compact.mjs --test 'dist-test/src/tests/*.test.js' 'dist-test/src/resources/extensions/gsd/tests/*.test.js' 'dist-test/src/resources/extensions/gsd/tests/*.test.mjs' 'dist-test/src/resources/extensions/shared/tests/*.test.js' 'dist-test/src/resources/extensions/claude-code-cli/tests/*.test.js' 'dist-test/src/resources/extensions/github-sync/tests/*.test.js' 'dist-test/src/resources/extensions/universal-config/tests/*.test.js' 'dist-test/src/resources/extensions/voice/tests/*.test.js'", + "test:unit": "npm run test:compile && node --import ./scripts/dist-test-resolve.mjs --experimental-test-isolation=process --test-reporter=./scripts/test-reporter-compact.mjs --test 'dist-test/src/tests/*.test.js' 'dist-test/src/resources/extensions/gsd/tests/*.test.js' 'dist-test/src/resources/extensions/gsd/tests/*.test.mjs' 'dist-test/src/resources/extensions/shared/tests/*.test.js' 'dist-test/src/resources/extensions/claude-code-cli/tests/*.test.js' 'dist-test/src/resources/extensions/github-sync/tests/*.test.js' 'dist-test/src/resources/extensions/universal-config/tests/*.test.js' 'dist-test/src/resources/extensions/voice/tests/*.test.js' 'dist-test/src/resources/extensions/mcp-client/tests/*.test.js'", "test:packages": "node --test packages/pi-coding-agent/dist/core/*.test.js", "test:marketplace": "GSD_TEST_CLONE_MARKETPLACES=1 node --import ./src/resources/extensions/gsd/tests/resolve-ts.mjs --experimental-strip-types --test src/resources/extensions/gsd/tests/claude-import-tui.test.ts src/resources/extensions/gsd/tests/plugin-importer-live.test.ts src/tests/marketplace-discovery.test.ts", "test:coverage": "c8 --reporter=text --reporter=lcov --exclude='src/resources/extensions/gsd/tests/**' --exclude='src/tests/**' --exclude='scripts/**' --exclude='native/**' --exclude='node_modules/**' --check-coverage --statements=40 --lines=40 --branches=20 --functions=20 node --import ./src/resources/extensions/gsd/tests/resolve-ts.mjs --experimental-strip-types --experimental-test-isolation=process --test src/resources/extensions/gsd/tests/*.test.ts src/resources/extensions/gsd/tests/*.test.mjs src/tests/*.test.ts src/resources/extensions/shared/tests/*.test.ts", diff --git a/src/resources/extensions/mcp-client/index.ts b/src/resources/extensions/mcp-client/index.ts index 38d001aa1..f62173455 100644 --- a/src/resources/extensions/mcp-client/index.ts +++ b/src/resources/extensions/mcp-client/index.ts @@ -111,7 +111,11 @@ function readConfigs(): McpServerConfig[] { } function getServerConfig(name: string): McpServerConfig | undefined { - return readConfigs().find((s) => s.name === name); + const trimmed = name.trim(); + return readConfigs().find((s) => + s.name === trimmed || + s.name.toLowerCase() === trimmed.toLowerCase(), + ); } /** Resolve ${VAR} references in env values against process.env. */ @@ -131,12 +135,14 @@ function resolveEnv(env: Record): Record { } async function getOrConnect(name: string, signal?: AbortSignal): Promise { - const existing = connections.get(name); - if (existing) return existing.client; - const config = getServerConfig(name); if (!config) throw new Error(`Unknown MCP server: "${name}". Use mcp_servers to list available servers.`); + // Always use config.name as the canonical cache key so that variant + // casing / whitespace still hits the same connection. + const existing = connections.get(config.name); + if (existing) return existing.client; + const client = new Client({ name: "gsd", version: "1.0.0" }); let transport: StdioClientTransport | StreamableHTTPClientTransport; @@ -151,15 +157,15 @@ async function getOrConnect(name: string, signal?: AbortSignal): Promise } else if (config.transport === "http" && config.url) { const resolvedUrl = config.url.replace( /\$\{([^}]+)\}/g, - (_, name) => process.env[name] ?? "", + (_, varName) => process.env[varName] ?? "", ); transport = new StreamableHTTPClientTransport(new URL(resolvedUrl)); } else { - throw new Error(`Server "${name}" has unsupported transport: ${config.transport}`); + throw new Error(`Server "${config.name}" has unsupported transport: ${config.transport}`); } await client.connect(transport, { signal, timeout: 30000 }); - connections.set(name, { client, transport }); + connections.set(config.name, { client, transport }); return client; } diff --git a/src/resources/extensions/mcp-client/tests/server-name-spaces.test.ts b/src/resources/extensions/mcp-client/tests/server-name-spaces.test.ts new file mode 100644 index 000000000..1cdb30f6e --- /dev/null +++ b/src/resources/extensions/mcp-client/tests/server-name-spaces.test.ts @@ -0,0 +1,55 @@ +/** + * Regression test for #3029 — mcp_discover fails for server names with spaces. + * + * The getServerConfig lookup must handle: + * 1. Exact match (already works) + * 2. Names with leading/trailing whitespace (trimming) + * 3. Case-insensitive matching (e.g. "Langgraph code" vs "langgraph Code") + * + * We test at the source level since getServerConfig is not exported. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +const source = readFileSync(join(__dirname, "..", "index.ts"), "utf-8"); + +test("#3029: getServerConfig trims whitespace from input name", () => { + assert.ok( + source.includes(".trim()"), + "getServerConfig should trim the input name before comparison", + ); +}); + +test("#3029: getServerConfig performs case-insensitive matching", () => { + assert.ok( + source.includes(".toLowerCase()"), + "getServerConfig should compare names case-insensitively", + ); +}); + +test("#3029: getOrConnect normalizes name for connection cache lookup", () => { + // The connections Map key must use the canonical (config) name, not the + // raw user input, so that subsequent lookups hit the cache even when the + // user's casing differs. + const getOrConnectMatch = source.match( + /async function getOrConnect\(name: string[\s\S]*?const existing = connections\.get\(/, + ); + assert.ok( + getOrConnectMatch, + "getOrConnect function should exist", + ); + // After the fix, getOrConnect should normalize the name via getServerConfig + // or use config.name as the canonical cache key. + assert.ok( + source.includes("connections.get(config.name") || + source.includes("connections.set(config.name"), + "getOrConnect should use config.name (canonical) as the connections cache key", + ); +});