diff --git a/src/cli.ts b/src/cli.ts index 08e1e0452..00aae55f5 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -546,6 +546,17 @@ if (isPrintMode) { if (mode === 'mcp') { printStartupTimings() const { startMcpServer } = await import('./mcp-server.js') + + // Activate every registered tool before starting the MCP transport. + // `session.agent.state.tools` is the *active* subset, not the full + // registry — if we expose only the active set, extension-registered + // tools (gsd workflow, browser-tools, mac-tools, search-the-web, …) + // are invisible to MCP clients. Flipping the active set to every + // known tool name makes `state.tools` mirror the full registry for + // this MCP session, which is what an external client expects. + const allToolNames = session.getAllTools().map((t) => t.name) + session.setActiveToolsByName(allToolNames) + await startMcpServer({ tools: session.agent.state.tools ?? [], version: process.env.GSD_VERSION || '0.0.0', diff --git a/src/mcp-server.ts b/src/mcp-server.ts index af83c9594..6db605dc9 100644 --- a/src/mcp-server.ts +++ b/src/mcp-server.ts @@ -16,13 +16,18 @@ export interface McpToolDef { }> } -// MCP SDK subpath imports use wildcard exports (./*) that NodeNext resolves -// at runtime but TypeScript cannot statically type-check. We construct the -// specifiers dynamically so tsc treats them as `any`. +// MCP SDK subpath imports use wildcard exports (./*) in @modelcontextprotocol/sdk's +// package.json export map. The wildcard maps "./foo" → "./dist/cjs/foo" (no .js +// suffix), so bare subpath specifiers like `${MCP_PKG}/server/stdio` resolve to +// a non-existent file. Historically the workaround (#3603) used createRequire so +// the CJS resolver could auto-append `.js`; that no longer works with current +// Node + SDK releases (#3914) — `_require.resolve` also fails with +// "Cannot find module .../dist/cjs/server/stdio". // -// Use explicit .js subpaths for modules that are loaded dynamically at runtime. -// Recent Node / SDK combinations do not reliably resolve the extensionless -// wildcard targets for `server/stdio` and `types` (#3914). +// The reliable convention (matching packages/mcp-server/{server,cli}.ts) is to +// write the `.js` suffix explicitly on every wildcard subpath. Specifiers are +// built via a template string so TypeScript's NodeNext resolver treats them as +// `any` and skips static checking. const MCP_PKG = '@modelcontextprotocol/sdk' /** @@ -45,7 +50,7 @@ export async function startMcpServer(options: { }): Promise { const { tools, version = '0.0.0' } = options - const serverMod = await import(`${MCP_PKG}/server`) + const serverMod = await import(`${MCP_PKG}/server/index.js`) const stdioMod = await import(`${MCP_PKG}/server/stdio.js`) const typesMod = await import(`${MCP_PKG}/types.js`) diff --git a/src/tests/mcp-createRequire.test.ts b/src/tests/mcp-createRequire.test.ts index d16ebacd6..63c12af0c 100644 --- a/src/tests/mcp-createRequire.test.ts +++ b/src/tests/mcp-createRequire.test.ts @@ -1,9 +1,17 @@ /** - * Regression test for #3914 — MCP server uses explicit .js SDK subpaths. + * Regression test for #3603 / #3914 — MCP server subpath imports. * - * Extensionless wildcard exports for `server/stdio` and `types` do not resolve - * reliably across current Node / SDK combinations. The runtime import strings - * must include `.js`. + * @modelcontextprotocol/sdk's package.json exports map uses a wildcard + * `./*` → `./dist/cjs/*` with no `.js` suffix, so bare subpath specifiers + * like `@modelcontextprotocol/sdk/server/stdio` resolve to a file that + * doesn't exist. Historically the workaround used `createRequire` so the + * CJS resolver auto-appended `.js`; that no longer works with current + * Node + SDK versions (#3914). + * + * The reliable convention (used in packages/mcp-server/{server,cli}.ts) + * is to write the `.js` suffix explicitly on every subpath import. This + * test locks that convention in so regressions can't silently reintroduce + * the bare subpath form or the broken createRequire-based resolution. */ import { describe, test } from 'node:test'; @@ -17,19 +25,31 @@ const __dirname = dirname(__filename); const source = readFileSync(join(__dirname, '..', 'mcp-server.ts'), 'utf-8'); -describe('MCP server SDK subpath imports (#3914)', () => { - test('server/stdio import uses explicit .js subpath', () => { - assert.match(source, /await import\(`\$\{MCP_PKG\}\/server\/stdio\.js`\)/, - 'server/stdio import should include the .js suffix'); +describe('MCP server SDK subpath imports (#3603 / #3914)', () => { + test('server/index.js subpath is imported with explicit .js suffix', () => { + assert.match(source, /await import\(`\$\{MCP_PKG\}\/server\/index\.js`\)/, + 'server import must use `${MCP_PKG}/server/index.js` to satisfy the wildcard export map'); }); - test('types import uses explicit .js subpath', () => { + test('server/stdio.js subpath is imported with explicit .js suffix', () => { + assert.match(source, /await import\(`\$\{MCP_PKG\}\/server\/stdio\.js`\)/, + 'stdio import must use `${MCP_PKG}/server/stdio.js`'); + }); + + test('types.js subpath is imported with explicit .js suffix', () => { assert.match(source, /await import\(`\$\{MCP_PKG\}\/types\.js`\)/, - 'types import should include the .js suffix'); + 'types import must use `${MCP_PKG}/types.js`'); }); test('legacy createRequire-based resolution is gone', () => { - assert.doesNotMatch(source, /createRequire|_require\.resolve/, - 'legacy createRequire-based subpath resolution should not remain'); + // Only flag actual code, not the comment that explains the history. + // The import statement, variable declaration, and `_require.resolve(` call + // sites are the real regression surfaces. + assert.doesNotMatch(source, /^\s*import\s*\{\s*createRequire\s*\}\s*from/m, + 'createRequire should not be imported from node:module'); + assert.doesNotMatch(source, /^\s*const\s+_require\s*=\s*createRequire/m, + '_require helper should not be created'); + assert.doesNotMatch(source, /_require\.resolve\(/, + '_require.resolve should not be used for subpath resolution'); }); });