From 8e417e0bfb299b995fc4bc7597365a1dd2bbedd8 Mon Sep 17 00:00:00 2001 From: JF Date: Mon, 9 Nov 2026 20:43:22 -0500 Subject: [PATCH 1/3] feat(stdio): serve both protocol eras through the SDK's serveStdio (#1025) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stdio transport is now served by `serveStdio(factory, { transport })` instead of a hand-wired `Server.connect(new StdioServerTransport())`. The entry owns the wire, answers a client's `server/discover` probe and pins the connection to the era its opening message chose: a probing client (Claude Code's v2 runtime, which until now recorded mcp-debugger as a legacy peer on every connect) lands on 2026-07-28 — no initialize, `subscriptions/listen` for resource notifications, routed from the pinned facade's own `sendResourceUpdated`/`sendResourceListChanged` by the SDK — and a client that opens with `initialize` gets the 2025 era exactly as before. The factory builds `createProtocolServer(core, { era, deliver: 'instance' })` per connection (and once for a probe instance a client may abandon). Shutdown closes the serving handle first (a graceful end for an open listen stream), then the core, under the existing 5 s guard; the stdin watchdog and the MCP_CONTAINER rule are unchanged. The command no longer overwrites the transport's onclose/onerror after connect, which had kept the SDK's own teardown from running. Tests: the stdio unit suite drives the injected `serveStdio` double (factory per era, options, close order, a refusing handle); an in-process suite serves the facade through the real `serveStdio` over the in-memory transport (a pinned 2026-07-28 client hears list changes on its listen stream, a 2025 client keeps resources/subscribe with coalesced pings, the handle's close detaches the sink); `tests/integration/cli/stdio-era-negotiation.test.ts` against `dist/index.js stdio` — `auto` lands on 2026-07-28 with 29 tools, a pinned client drives a full mock debug session and hears list changes, `legacy` still gets 2025-11-25 with `subscribeResource`, and a raw `server/discover` line names 2026-07-28. Gates: typecheck 0, lint, build, integration cli + self-debug 45/45, Python and HTTP e2e smokes 7/7. The npm bundle grows 4,205,542 → 4,347,620 bytes (the serving entry and its listen router). Docs: README transports bullet, getting-started (both eras, Claude Code's 7-day legacy memory), CLAUDE.md; fragment 1025-protocol-2026-stdio.added.md. Co-Authored-By: Claude Fable 5.1 --- CLAUDE.md | 2 +- README.md | 2 +- changelog.d/1025-protocol-2026-stdio.added.md | 1 + docs/getting-started.md | 7 + src/cli/stdio-command.ts | 80 +++++----- .../unit/server/protocol-facade-stdio.test.ts | 125 +++++++++++++++ .../cli/stdio-era-negotiation.test.ts | 144 ++++++++++++++++++ tests/unit/cli/stdio-command.test.ts | 52 ++++++- 8 files changed, 372 insertions(+), 41 deletions(-) create mode 100644 changelog.d/1025-protocol-2026-stdio.added.md create mode 100644 tests/core/unit/server/protocol-facade-stdio.test.ts create mode 100644 tests/integration/cli/stdio-era-negotiation.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index c9f3adda..6d510b29 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -222,7 +222,7 @@ The codebase follows a **layered architecture with dependency injection** and ** - Entry point for MCP protocol communication - `src/server.ts` is the composition root and the shared core (`DebugMcpServer`: dependencies, lifecycle, session validation, breakpoint gating, the public facade methods, the `NotificationHub`); it implements `ToolContext` and passes itself to the handlers. It owns no SDK `Server`: `src/server/protocol-facade.ts` (`createProtocolServer(core, { era, deliver })`) builds one cheap SDK `Server` per connection or request over the core, and `src/server/notification-hub.ts` fans the coalesced `resources/updated` pings and `resources/list_changed` out to every facade's sink (issue #1025) - `src/server/` holds the tool schemas (`tool-schemas.ts`), argument coercion (`tool-arguments.ts`), the tools/call dispatch wrapper (`tool-dispatch.ts` over `handlers/index.ts`), argument validation (`tool-validation.ts`), result envelopes and the session-error catch helpers (`tool-result.ts`), one handler module per tool family (`handlers/*.ts`) over their shared helpers (`handlers/shared.ts`), and the resource (`output-resources.ts`) and prompt (`prompts.ts`) handlers - - Supports STDIO and Streamable HTTP transport modes. One core per process in both: on `http` every MCP session is a facade over the same debug sessions (the id is the capability; one process per agent is the isolation), a debug session outlives the MCP session that created it, and an abandoned session tree is closed by the per-debug-session idle policy (`MCP_DEBUG_SESSION_IDLE_MS`, 30 min on `http`, off on `stdio`, `0` disables; sweep `MCP_DEBUG_SESSION_IDLE_SWEEP_MS`) unless a client is attending the process (an open SSE stream). The HTTP reapers (`MCP_HTTP_STALE_SESSION_MS`, `MCP_HTTP_STREAM_LOST_SESSION_MS`) reap abandoned MCP sessions only (issue #1025) + - Supports STDIO and Streamable HTTP transport modes. `stdio` is served through the SDK's `serveStdio(factory, { transport })` and speaks both protocol eras (2026-07-28 for a client that probes with `server/discover`, 2025-11-25 and earlier for one that opens with `initialize`); `http` speaks the 2025 era (the 2026 era follows in #1025 step 2, PR 4). One core per process in both: on `http` every MCP session is a facade over the same debug sessions (the id is the capability; one process per agent is the isolation), a debug session outlives the MCP session that created it, and an abandoned session tree is closed by the per-debug-session idle policy (`MCP_DEBUG_SESSION_IDLE_MS`, 30 min on `http`, off on `stdio`, `0` disables; sweep `MCP_DEBUG_SESSION_IDLE_SWEEP_MS`) unless a client is attending the process (an open SSE stream). The HTTP reapers (`MCP_HTTP_STALE_SESSION_MS`, `MCP_HTTP_STREAM_LOST_SESSION_MS`) reap abandoned MCP sessions only (issue #1025) - Dynamically discovers available language adapters 2. **Adapter System** (NEW) diff --git a/README.md b/README.md index c9165414..366a6367 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,7 @@ If your agent runs in a terminal, a pipeline, or a cloud sandbox — or needs to - 💥 **Crash-state debugging by default** – Launch sessions pause on uncaught exceptions with stack and locals live (`breakOnExceptions`; exception class/message surfaced via `lastStop`) - 🪞 **Read-only IDE mirror** – `expose_session` opens a loopback, token-gated DAP endpoint so a human's IDE can inspect the agent's live session without taking control - ☸️ **Kubernetes debugging** – port-forward attach for interpreted runtimes, `kubectl debug --target` + attach-by-PID for native processes ([recipe](docs/kubernetes.md), [turnkey manifests](examples/kubernetes/)) -- 🔌 **STDIO and Streamable HTTP transports** – Works with any MCP client +- 🔌 **STDIO and Streamable HTTP transports** – Works with any MCP client. `stdio` speaks both protocol eras — the current 2026-07-28 revision (a probing client such as Claude Code's v2 runtime lands on it) and 2025-11-25 and earlier (a client that opens with `initialize`); `http` speaks 2025-11-25 and earlier, with 2026-07-28 next - 📦 **Zero-runtime dependencies** – Self-contained bundles via esbuild + tsup - ⚡ **npx ready** – Run directly with `npx @debugmcp/mcp-debugger` - no installation needed - 🐳 **Docker and npm packages** – Deploy anywhere diff --git a/changelog.d/1025-protocol-2026-stdio.added.md b/changelog.d/1025-protocol-2026-stdio.added.md new file mode 100644 index 00000000..d7086aaa --- /dev/null +++ b/changelog.d/1025-protocol-2026-stdio.added.md @@ -0,0 +1 @@ +**`stdio` speaks the current MCP protocol revision, 2026-07-28, beside 2025-11-25 and earlier.** The stdio transport is served through the SDK's own serving entry, which answers a client's `server/discover` probe and pins the connection to whichever era its opening message chose: a probing client — Claude Code's v2 runtime probes every stdio server on every connect, and until now recorded mcp-debugger as a legacy peer — lands on 2026-07-28, with `subscriptions/listen` for resource notifications and no `initialize` handshake; a client that opens with `initialize` gets the 2025 era exactly as before, `resources/subscribe` included. The 29 tools, the resources and the prompt are the same in both. Claude Code may keep a stdio server it once saw answer as legacy pinned that way for up to seven days; restart it after upgrading, or wait it out. Step 2 of #1025; `http` follows. diff --git a/docs/getting-started.md b/docs/getting-started.md index 702fcbf0..21c0ef96 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -99,6 +99,13 @@ Because `stdio` is the default subcommand, bare `mcp-debugger` starts in stdio m explicitly in a config file is still worth doing: it documents the intent. Console output is silenced in every mode so nothing can corrupt the JSON-RPC stream. +Both eras of the MCP protocol are served on `stdio`: a client that probes with `server/discover` +(Claude Code's v2 runtime does, on every connect) gets the current 2026-07-28 revision, and a +client that opens with `initialize` gets 2025-11-25 or earlier, as before. Claude Code may keep a +stdio server it once saw answer as legacy pinned that way for up to seven days; restarting Claude +Code after upgrading the server, or waiting it out, lets it probe again. `http` speaks 2025-11-25 +and earlier today; the 2026-07-28 era follows there. + ### Claude Desktop, or any client with a JSON config ```json diff --git a/src/cli/stdio-command.ts b/src/cli/stdio-command.ts index 88fe2d04..a5cf9972 100644 --- a/src/cli/stdio-command.ts +++ b/src/cli/stdio-command.ts @@ -1,5 +1,5 @@ import type { Logger as WinstonLoggerType } from 'winston'; -import { StdioServerTransport } from '@modelcontextprotocol/server/stdio'; +import { serveStdio, StdioServerTransport } from '@modelcontextprotocol/server/stdio'; import { DebugMcpServer, type DebugMcpServerOptions } from '../server.js'; import { createProtocolServer } from '../server/protocol-facade.js'; import type { StdioOptions } from './setup.js'; @@ -12,6 +12,8 @@ export interface StdioCommandDependencies { serverFactory: (options: ServerFactoryOptions) => DebugMcpServer; /** Builds the SDK Server facade over the core (issue #1025, step 2); defaults to createProtocolServer. */ protocolServerFactory?: typeof createProtocolServer; + /** The SDK's stdio serving entry (issue #1025, step 2); defaults to the real one. */ + serveStdio?: typeof serveStdio; exitProcess?: (code: number) => void; /** Injectable stdin for tests; defaults to proc.stdin. */ stdin?: NodeJS.ReadStream; @@ -25,13 +27,13 @@ export async function handleStdioCommand( ): Promise { const proc = dependencies.proc ?? process; const { logger, serverFactory, exitProcess = (code: number) => proc.exit(code) } = dependencies; - + if (options.logLevel) { logger.level = options.logLevel; } - + logger.info('Starting Debug MCP Server in stdio mode'); - + try { const debugMcpServer = serverFactory({ @@ -41,28 +43,42 @@ export async function handleStdioCommand( // defaults to off here (MCP_DEBUG_SESSION_IDLE_MS still turns it on; issue #1025). debugSessionIdle: { defaultMs: 0 } }); - + // Create stdio transport logger.info('[MCP] Creating StdioServerTransport...'); const transport = new StdioServerTransport(); // Keep the event loop alive even if stdin closes (e.g., detached containers). - // Cleared on transport close or signals. + // Cleared on shutdown. const keepAlive = setInterval(() => {}, 60000); - - // Connect a legacy-era protocol facade over the core to the transport - // (issue #1025, step 2): the core owns no SDK Server of its own. - logger.info('[MCP] Connecting server to stdio transport...'); + + // The SDK's stdio serving entry owns the wire and speaks both protocol + // eras (issue #1025, step 2): the opening message pins one facade for the + // connection — a 2026-07-28 client's server/discover lands on a modern + // instance, a 2025 client's initialize on a legacy one — and the entry + // itself answers server/discover and serves subscriptions/listen. The + // factory may run twice: once for a discover probe that a client then + // abandons for initialize, and once for the pinned instance. + logger.info('[MCP] Serving stdio in both protocol eras...'); const protocolServerFactory = dependencies.protocolServerFactory ?? createProtocolServer; - const facade = protocolServerFactory(debugMcpServer, { era: 'legacy', deliver: 'instance' }); - await facade.connect(transport); + const serve = dependencies.serveStdio ?? serveStdio; + const handle = serve( + ({ era }) => protocolServerFactory(debugMcpServer, { era, deliver: 'instance' }), + { + transport, + onerror: (error: Error) => { + logger.error('[MCP] Transport error:', { error }); + } + } + ); logger.info('[MCP] Server connected to stdio transport successfully'); // Every exit path must tear down the debug sessions first (issue #337): // exiting without DebugMcpServer.stop() → closeAllSessions() leaves the // proxy chains — and, for attach sessions, lldb-server's ptrace claim on - // the target — running with no owner. Bounded so a wedged proxy cannot - // block exit (ProxyManager.stop() is internally bounded but runs - // serially per session). + // the target — running with no owner. The serving handle goes first so + // an open subscriptions/listen stream ends gracefully. Bounded so a + // wedged proxy cannot block exit (ProxyManager.stop() is internally + // bounded but runs serially per session). let shutdownStarted = false; const shutdownAndExit = (code: number, why: string) => { if (shutdownStarted) return; @@ -73,29 +89,25 @@ export async function handleStdioCommand( const timer = setTimeout(resolve, 5000); timer.unref?.(); }); - void Promise.race([ - debugMcpServer.stop().catch((err) => { + const teardown = (async () => { + try { + await handle.close(); + } catch (err) { + logger.debug('[MCP] Error closing the stdio serving handle during exit:', { err }); + } + try { + await debugMcpServer.stop(); + } catch (err) { logger.error('[MCP] Error stopping debug server during exit:', { err }); - }), - guard - ]).finally(() => exitProcess(code)); + } + })(); + void Promise.race([teardown, guard]).finally(() => exitProcess(code)); }; - // Ensure deterministic shutdown on transport close (the Transport - // contract's own hook; the v1 SDK left it undocumented) - transport.onclose = () => { - shutdownAndExit(0, 'Transport closed'); - }; - // Start the debug server await debugMcpServer.start(); logger.info('Server started successfully in stdio mode'); - - // Add transport error handling - transport.onerror = (error) => { - logger.error('[MCP] Transport error:', { error }); - }; - + // Keep the process alive const stdin: NodeJS.ReadableStream = dependencies.stdin ?? proc.stdin; stdin.resume(); @@ -103,8 +115,8 @@ export async function handleStdioCommand( // Stdin is the MCP transport here, so any byte on it proves a client was // speaking to us — which makes a later EOF mean that client is gone. // Nothing else will reap us: on Windows a dying parent delivers no signal, - // the SDK transport never notices EOF, and `keepAlive` above holds the - // event loop open, so without this the process runs forever (issue #122). + // and `keepAlive` above holds the event loop open, so without this the + // process runs forever (issue #122). let sawClientTraffic = false; // Additive: Node broadcasts each chunk to every 'data' listener and // stdin.resume() above already put the stream in flowing mode, so this diff --git a/tests/core/unit/server/protocol-facade-stdio.test.ts b/tests/core/unit/server/protocol-facade-stdio.test.ts new file mode 100644 index 00000000..d9d19bb9 --- /dev/null +++ b/tests/core/unit/server/protocol-facade-stdio.test.ts @@ -0,0 +1,125 @@ +/** + * The facade served through the SDK's stdio entry (issue #1025, step 2), in + * process over the in-memory transport: `serveStdio(factory, { transport })` + * pins one facade per connection in whichever era the client opens. A + * 2026-07-28 client lands on a modern pin and hears list changes on its + * subscriptions/listen stream through the facade's own `sendResourceListChanged` + * (the SDK routes it); a 2025 client lands on a legacy pin and keeps + * resources/subscribe with its coalesced resources/updated pings. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { InMemoryTransport } from '@modelcontextprotocol/server'; +import { serveStdio, type StdioServerHandle } from '@modelcontextprotocol/server/stdio'; +import { Client } from '@modelcontextprotocol/client'; +import { DebugMcpServer } from '../../../../src/server.js'; +import { createProtocolServer } from '../../../../src/server/protocol-facade.js'; +import { OUTPUT_UPDATE_DEBOUNCE_MS } from '../../../../src/server/notification-hub.js'; +import { TOOL_NAMES } from '../../../../src/server/tool-schemas.js'; +import { SessionManager } from '../../../../src/session/session-manager.js'; +import { createProductionDependencies, type Dependencies } from '../../../../src/container/dependencies.js'; +import { + createMockDependencies, + createMockSessionManager, + type MockSessionManager +} from './server-test-helpers.js'; + +vi.mock('../../../../src/session/session-manager.js'); +vi.mock('../../../../src/container/dependencies.js'); + +const URI = 'debug://sessions/sess-1/output'; + +describe('createProtocolServer behind serveStdio (issue #1025, step 2)', () => { + let core: DebugMcpServer; + let mockSessionManager: MockSessionManager; + let mockDependencies: Dependencies; + let handle: StdioServerHandle | undefined; + let client: Client | undefined; + let outputCapturedListener: ((sessionId: string) => void) | undefined; + + beforeEach(() => { + mockDependencies = createMockDependencies(); + vi.mocked(createProductionDependencies).mockReturnValue(mockDependencies); + mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); + mockSessionManager.getSession.mockReturnValue({ id: 'sess-1', name: 'one', language: 'mock' } as any); + mockSessionManager.getAllSessions.mockReturnValue([]); + mockSessionManager.on.mockImplementation((event: string, listener: (...args: any[]) => void) => { + if (event === 'output-captured') { + outputCapturedListener = listener; + } + }); + vi.mocked(SessionManager).mockImplementation(function () { return mockSessionManager as any; }); + core = new DebugMcpServer({ logLevel: 'debug' }); + }); + + afterEach(async () => { + await client?.close().catch(() => undefined); + await handle?.close().catch(() => undefined); + mockSessionManager.closeAllSessions.mockResolvedValue(undefined); + await core.stop(); + vi.clearAllMocks(); + handle = undefined; + client = undefined; + outputCapturedListener = undefined; + }); + + function serve(): InMemoryTransport { + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + handle = serveStdio( + ({ era }) => createProtocolServer(core, { era, deliver: 'instance' }), + { transport: serverTransport } + ); + return clientTransport; + } + + it('a 2026-07-28 client lands on a modern pin and hears list changes on its listen stream', async () => { + const clientTransport = serve(); + client = new Client({ name: 'modern-test', version: '0.0.0' }, { versionNegotiation: { mode: { pin: '2026-07-28' } } }); + await client.connect(clientTransport); + + expect(client.getProtocolEra()).toBe('modern'); + expect(client.getNegotiatedProtocolVersion()).toBe('2026-07-28'); + expect(core.notifications.sinkCount).toBe(1); + const { tools } = await client.listTools(); + expect(tools.map((tool) => tool.name).sort()).toEqual([...TOOL_NAMES].sort()); + + const changes: string[] = []; + client.setNotificationHandler('notifications/resources/list_changed', () => { changes.push('list_changed'); }); + const subscription = await client.listen({ resourcesListChanged: true }); + expect(subscription.honoredFilter.resourcesListChanged).toBe(true); + + core.notifications.resourcesChanged(); + await vi.waitFor(() => expect(changes).toEqual(['list_changed'])); + + await subscription.close(); + }); + + it('a 2025 client lands on a legacy pin and keeps resources/subscribe with coalesced pings', async () => { + const clientTransport = serve(); + client = new Client({ name: 'legacy-test', version: '0.0.0' }); + await client.connect(clientTransport); + + expect(client.getProtocolEra()).toBe('legacy'); + const pings: string[] = []; + client.setNotificationHandler('notifications/resources/updated', (notification) => { + pings.push((notification.params as { uri: string }).uri); + }); + await client.subscribeResource({ uri: URI }); + + outputCapturedListener!('sess-1'); + outputCapturedListener!('sess-1'); + await new Promise((resolve) => setTimeout(resolve, OUTPUT_UPDATE_DEBOUNCE_MS + 50)); + await vi.waitFor(() => expect(pings).toEqual([URI])); + }); + + it('the handle closing detaches the pinned facade from the hub', async () => { + const clientTransport = serve(); + client = new Client({ name: 'modern-test', version: '0.0.0' }, { versionNegotiation: { mode: { pin: '2026-07-28' } } }); + await client.connect(clientTransport); + expect(core.notifications.sinkCount).toBe(1); + + await handle!.close(); + handle = undefined; + + await vi.waitFor(() => expect(core.notifications.sinkCount).toBe(0)); + }); +}); diff --git a/tests/integration/cli/stdio-era-negotiation.test.ts b/tests/integration/cli/stdio-era-negotiation.test.ts new file mode 100644 index 00000000..930d865d --- /dev/null +++ b/tests/integration/cli/stdio-era-negotiation.test.ts @@ -0,0 +1,144 @@ +/** + * The stdio transport speaks both protocol eras (issue #1025, step 2): a + * 2026-07-28 client's server/discover probe is answered, so Claude Code's v2 + * runtime (which probes every stdio server) lands on the modern era; a client + * that opens with initialize gets the 2025-11-25 era as before. Real server + * (`dist/index.js stdio`), the v2 SDK client in each negotiation mode, the + * mock adapter — no toolchain. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import { spawn } from 'node:child_process'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { Client } from '@modelcontextprotocol/client'; +import { StdioClientTransport } from '@modelcontextprotocol/client/stdio'; +import { serverEnvironment } from '../../test-utils/helpers/mcp-stdio-client.js'; + +const root = fileURLToPath(new URL('../../../', import.meta.url)); +const entry = path.join(root, 'dist/index.js'); +const MOCK_SCRIPT = path.join(root, 'examples/python/simple_test.py'); + +type Mode = 'auto' | 'legacy' | { pin: string }; + +interface Answer { + success: boolean; + sessionId?: string; + state?: string; + message?: string; + error?: string; +} + +const clients: Client[] = []; + +async function connect(mode: Mode, name: string): Promise { + const client = new Client({ name, version: '1.0.0' }, { versionNegotiation: { mode } }); + const transport = new StdioClientTransport({ + command: process.execPath, + args: [entry, 'stdio'], + cwd: root, + env: serverEnvironment(), + stderr: 'pipe' + }); + transport.stderr?.on('data', () => {}); + await client.connect(transport); + clients.push(client); + return client; +} + +async function call(client: Client, name: string, args: Record = {}): Promise { + const raw = await client.callTool({ name, arguments: args }, { timeout: 30_000 }); + const text = (raw.content as Array<{ type: string; text?: string }>).find((item) => item.type === 'text')?.text; + expect(text, JSON.stringify(raw)).toBeDefined(); + return JSON.parse(text!) as Answer; +} + +afterEach(async () => { + for (const client of clients.splice(0)) { + await client.close().catch(() => {}); + } +}); + +describe('stdio serves both protocol eras (issue #1025, step 2)', () => { + it('a probing client (Claude Code\'s posture) lands on 2026-07-28', async () => { + const client = await connect('auto', 'era-auto'); + + expect(client.getProtocolEra()).toBe('modern'); + expect(client.getNegotiatedProtocolVersion()).toBe('2026-07-28'); + expect(client.getServerVersion()?.name).toBe('debug-mcp-server'); + expect((await client.listTools()).tools.length).toBe(29); + }, 60_000); + + it('a client pinned to 2026-07-28 drives a full debug session and hears list changes on its listen stream', async () => { + const client = await connect({ pin: '2026-07-28' }, 'era-pin'); + expect(client.getProtocolEra()).toBe('modern'); + + const changes: string[] = []; + client.setNotificationHandler('notifications/resources/list_changed', () => { changes.push('list_changed'); }); + const subscription = await client.listen({ resourcesListChanged: true }); + expect(subscription.honoredFilter.resourcesListChanged).toBe(true); + + const created = await call(client, 'create_debug_session', { language: 'mock', name: 'era-pin' }); + expect(created.success, JSON.stringify(created)).toBe(true); + const sessionId = created.sessionId!; + await expect.poll(() => changes.length, { timeout: 10_000 }).toBeGreaterThanOrEqual(1); + + const bp = await call(client, 'set_breakpoint', { sessionId, file: MOCK_SCRIPT, line: 32 }); + expect(bp.success, JSON.stringify(bp)).toBe(true); + const started = await call(client, 'start_debugging', { sessionId, scriptPath: MOCK_SCRIPT, dapLaunchArgs: { stopOnEntry: true } }); + expect(started.success, JSON.stringify(started)).toBe(true); + const waited = await call(client, 'wait_for_stop', { sessionId, timeout: 15_000 }); + expect(waited.success, JSON.stringify(waited)).toBe(true); + expect(waited.state).toBe('paused'); + expect((await call(client, 'close_debug_session', { sessionId })).success).toBe(true); + + await subscription.close(); + }, 90_000); + + it('a client that opens with initialize gets the 2025-11-25 era, as before', async () => { + const client = await connect('legacy', 'era-legacy'); + + expect(client.getProtocolEra()).toBe('legacy'); + expect(client.getNegotiatedProtocolVersion()).toBe('2025-11-25'); + const created = await call(client, 'create_debug_session', { language: 'mock', name: 'era-legacy' }); + expect(created.success).toBe(true); + // The legacy subscription API still works on a legacy pin. + await client.subscribeResource({ uri: `debug://sessions/${created.sessionId}/output` }); + expect((await call(client, 'close_debug_session', { sessionId: created.sessionId! })).success).toBe(true); + }, 60_000); + + it('a raw server/discover line on stdin names 2026-07-28 among the supported versions', async () => { + const child = spawn(process.execPath, [entry, 'stdio'], { cwd: root, env: serverEnvironment(), stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true }); + try { + const frames: Array<{ id?: number; result?: { supportedVersions?: string[] } }> = []; + let buffer = ''; + child.stdout.setEncoding('utf8'); + child.stdout.on('data', (chunk: string) => { + buffer += chunk; + const lines = buffer.split('\n'); + buffer = lines.pop() ?? ''; + for (const line of lines) { + if (!line.trim()) continue; + try { frames.push(JSON.parse(line)); } catch { /* not a frame */ } + } + }); + child.stdin.write(`${JSON.stringify({ + jsonrpc: '2.0', + id: 7, + method: 'server/discover', + params: { + _meta: { + 'io.modelcontextprotocol/protocolVersion': '2026-07-28', + 'io.modelcontextprotocol/clientInfo': { name: 'raw-probe', version: '1.0.0' }, + 'io.modelcontextprotocol/clientCapabilities': {} + } + } + })}\n`); + + await expect.poll(() => frames.find((frame) => frame.id === 7), { timeout: 20_000 }).toBeDefined(); + const answer = frames.find((frame) => frame.id === 7)!; + expect(answer.result?.supportedVersions).toContain('2026-07-28'); + } finally { + child.kill(); + } + }, 30_000); +}); diff --git a/tests/unit/cli/stdio-command.test.ts b/tests/unit/cli/stdio-command.test.ts index 5fbe785b..aacc4e66 100644 --- a/tests/unit/cli/stdio-command.test.ts +++ b/tests/unit/cli/stdio-command.test.ts @@ -7,6 +7,7 @@ import { FakeCurrentProcess } from '../../test-utils/mocks/fake-current-process. import type { Logger as WinstonLoggerType } from 'winston'; import { DebugMcpServer } from '../../../src/server.js'; import type { createProtocolServer } from '../../../src/server/protocol-facade.js'; +import { StdioServerTransport, type serveStdio } from '@modelcontextprotocol/server/stdio'; vi.mock('../../../src/server.js'); @@ -17,6 +18,8 @@ describe('STDIO Command Handler', () => { let mockServer: DebugMcpServer; let mockFacade: { connect: Mock<() => Promise> }; let mockProtocolServerFactory: Mock; + let mockHandle: { close: Mock<() => Promise> }; + let mockServeStdio: Mock; let fakeProc: FakeCurrentProcess; function makeFakeStdin() { @@ -46,6 +49,10 @@ describe('STDIO Command Handler', () => { } as any; mockFacade = { connect: vi.fn().mockResolvedValue(undefined) }; mockProtocolServerFactory = vi.fn(() => mockFacade as any); + // The SDK's serving entry (issue #1025, step 2): it owns the wire and pins + // one facade per connection from the factory; its handle is what shutdown closes. + mockHandle = { close: vi.fn<() => Promise>().mockResolvedValue(undefined) }; + mockServeStdio = vi.fn(() => mockHandle as any); // Create mock server factory mockServerFactory = vi.fn(() => mockServer); @@ -74,6 +81,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, stdin: makeFakeStdin(), proc: fakeProc @@ -96,10 +104,19 @@ describe('STDIO Command Handler', () => { // Verify server start was called expect(mockServer.start).toHaveBeenCalled(); - // The transport is connected to a legacy-era protocol facade over the core, - // never to the core itself (issue #1025, step 2) - expect(mockProtocolServerFactory).toHaveBeenCalledWith(mockServer, { era: 'legacy', deliver: 'instance' }); - expect(mockFacade.connect).toHaveBeenCalledTimes(1); + // The SDK's serving entry owns the stdio wire and builds one facade per + // connection through the factory, in whichever era the client opens + // (issue #1025, step 2); nothing connects the core itself. + expect(mockServeStdio).toHaveBeenCalledTimes(1); + const [factory, serveOptions] = mockServeStdio.mock.calls[0]; + expect(serveOptions?.transport).toBeInstanceOf(StdioServerTransport); + expect(typeof serveOptions?.onerror).toBe('function'); + expect(serveOptions?.legacy).toBeUndefined(); + expect(await factory({ era: 'modern' })).toBe(mockFacade); + expect(mockProtocolServerFactory).toHaveBeenLastCalledWith(mockServer, { era: 'modern', deliver: 'instance' }); + expect(await factory({ era: 'legacy' })).toBe(mockFacade); + expect(mockProtocolServerFactory).toHaveBeenLastCalledWith(mockServer, { era: 'legacy', deliver: 'instance' }); + expect(mockFacade.connect).not.toHaveBeenCalled(); // Verify success log expect(mockLogger.info).toHaveBeenCalledWith('Server started successfully in stdio mode'); @@ -113,6 +130,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, stdin: makeFakeStdin(), proc: fakeProc @@ -142,6 +160,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, stdin: makeFakeStdin(), proc: fakeProc @@ -169,6 +188,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, proc: fakeProc }); @@ -190,6 +210,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, proc: fakeProc }); @@ -207,6 +228,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, proc: fakeProc }); @@ -226,6 +248,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -253,6 +276,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -276,6 +300,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -332,6 +357,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -353,6 +379,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -360,17 +387,32 @@ describe('STDIO Command Handler', () => { return stdin; } - it('stops the debug server before exiting when stdin ends', async () => { + it('closes the serving handle and stops the debug server before exiting when stdin ends', async () => { const stdin = await startWithStdin(); stdin.emit('end'); await vi.waitFor(() => expect(mockExitProcess).toHaveBeenCalledWith(0)); + // The SDK handle first (a graceful end for any open subscriptions/listen + // stream), then the core, then exit (issue #1025, step 2). + expect(mockHandle.close).toHaveBeenCalledTimes(1); expect(mockServer.stop).toHaveBeenCalled(); + expect(mockHandle.close.mock.invocationCallOrder[0]) + .toBeLessThan(vi.mocked(mockServer.stop).mock.invocationCallOrder[0]); expect(vi.mocked(mockServer.stop).mock.invocationCallOrder[0]) .toBeLessThan(mockExitProcess.mock.invocationCallOrder[0]); }); + it('still stops the debug server and exits when the serving handle refuses to close', async () => { + mockHandle.close = vi.fn<() => Promise>().mockRejectedValue(new Error('wire gone')); + const stdin = await startWithStdin(); + + stdin.emit('end'); + + await vi.waitFor(() => expect(mockExitProcess).toHaveBeenCalledWith(0)); + expect(mockServer.stop).toHaveBeenCalled(); + }); + it('stops the debug server before exiting on SIGTERM', async () => { await startWithStdin(); From 1333647c01f29270d8ad2176e3579fa352215de0 Mon Sep 17 00:00:00 2001 From: JF Date: Mon, 9 Nov 2026 21:56:51 -0500 Subject: [PATCH 2/3] fix(stdio): bound the serving handle's close and hear the wire closing by itself (#1025) Two findings from the review of PR #1116. The serving handle and core.stop() shared the one 5 s exit guard, so a handle.close() that never settled (a subscriptions/listen stream whose consumer stopped reading) starved closeAllSessions() and left the proxy chains orphaned. The handle close is now bounded on its own (HANDLE_CLOSE_TIMEOUT_MS, 1 s) inside the guard; a late rejection is swallowed, the timeout logged at debug, and the core always gets its turn. The SDK's StdioServerTransport closes itself not only on stdin's EOF but on a stdout error (the client's end of the pipe gone) or an oversized frame, with stdin still open, and tells only its own onclose, which serveStdio owns. Nothing exited: the process lived on with a dead instance. ObservedStdioServerTransport, a subclass whose close() reports once after super.close(), runs every wire close through the same gate as a stdin EOF: exit in host mode or after client traffic, stay alive in a detached container no client ever spoke to (#633 unchanged). The shutdown's own close of the wire re-enters the idempotent shutdown. Co-Authored-By: Claude Fable 5.1 --- src/cli/stdio-command.ts | 166 ++++++++++++++++++--------- tests/unit/cli/stdio-command.test.ts | 77 +++++++++++++ 2 files changed, 188 insertions(+), 55 deletions(-) diff --git a/src/cli/stdio-command.ts b/src/cli/stdio-command.ts index a5cf9972..f8ab701d 100644 --- a/src/cli/stdio-command.ts +++ b/src/cli/stdio-command.ts @@ -7,6 +7,48 @@ import type { ProcessLike } from '../interfaces/process-interfaces.js'; export type ServerFactoryOptions = DebugMcpServerOptions; +/** The whole teardown's bound on exit: a wedged proxy cannot block the process from leaving. */ +const SHUTDOWN_GUARD_MS = 5000; +/** + * The serving handle's own bound inside that guard. Closing the handle ends an + * open subscriptions/listen stream, which a consumer that stopped reading can + * hold open; bounded on its own, the core's closeAllSessions() always gets its + * turn (review of PR #1116). + */ +const HANDLE_CLOSE_TIMEOUT_MS = 1000; + +function unrefDelay(ms: number): Promise { + return new Promise((resolve) => { + const timer = setTimeout(resolve, ms); + timer.unref?.(); + }); +} + +/** + * The SDK's stdio transport closes itself — on stdin's EOF, but also on a + * stdout error (the client's end of the pipe is gone) or an oversized frame, + * with stdin still open — and tells only its own `onclose`, which `serveStdio` + * owns. This subclass lets the command hear every close of the wire, whatever + * ended it, so the process never lives on with a dead instance (issue #1025, + * step 2; review of PR #1116). + */ +class ObservedStdioServerTransport extends StdioServerTransport { + private notified = false; + private readonly onWireClosed: () => void; + + constructor(onWireClosed: () => void) { + super(); + this.onWireClosed = onWireClosed; + } + + override async close(): Promise { + await super.close(); + if (this.notified) return; + this.notified = true; + this.onWireClosed(); + } +} + export interface StdioCommandDependencies { logger: WinstonLoggerType; serverFactory: (options: ServerFactoryOptions) => DebugMcpServer; @@ -44,56 +86,43 @@ export async function handleStdioCommand( debugSessionIdle: { defaultMs: 0 } }); - // Create stdio transport - logger.info('[MCP] Creating StdioServerTransport...'); - const transport = new StdioServerTransport(); // Keep the event loop alive even if stdin closes (e.g., detached containers). // Cleared on shutdown. const keepAlive = setInterval(() => {}, 60000); - // The SDK's stdio serving entry owns the wire and speaks both protocol - // eras (issue #1025, step 2): the opening message pins one facade for the - // connection — a 2026-07-28 client's server/discover lands on a modern - // instance, a 2025 client's initialize on a legacy one — and the entry - // itself answers server/discover and serves subscriptions/listen. The - // factory may run twice: once for a discover probe that a client then - // abandons for initialize, and once for the pinned instance. - logger.info('[MCP] Serving stdio in both protocol eras...'); - const protocolServerFactory = dependencies.protocolServerFactory ?? createProtocolServer; - const serve = dependencies.serveStdio ?? serveStdio; - const handle = serve( - ({ era }) => protocolServerFactory(debugMcpServer, { era, deliver: 'instance' }), - { - transport, - onerror: (error: Error) => { - logger.error('[MCP] Transport error:', { error }); - } - } - ); - logger.info('[MCP] Server connected to stdio transport successfully'); - // Every exit path must tear down the debug sessions first (issue #337): // exiting without DebugMcpServer.stop() → closeAllSessions() leaves the // proxy chains — and, for attach sessions, lldb-server's ptrace claim on // the target — running with no owner. The serving handle goes first so - // an open subscriptions/listen stream ends gracefully. Bounded so a + // an open subscriptions/listen stream ends gracefully, bounded on its own + // so the core always gets its turn; the whole teardown is bounded so a // wedged proxy cannot block exit (ProxyManager.stop() is internally - // bounded but runs serially per session). + // bounded but runs serially per session). `serving.handle` is set once the + // serving entry below has started; shutdown reads it at call time. + const serving: { handle?: { close(): Promise } } = {}; let shutdownStarted = false; const shutdownAndExit = (code: number, why: string) => { if (shutdownStarted) return; shutdownStarted = true; try { clearInterval(keepAlive); } catch { /* already cleared */ } logger.warn(`[MCP] ${why} — stopping debug server and exiting.`); - const guard = new Promise((resolve) => { - const timer = setTimeout(resolve, 5000); - timer.unref?.(); - }); + const guard = unrefDelay(SHUTDOWN_GUARD_MS); const teardown = (async () => { - try { - await handle.close(); - } catch (err) { - logger.debug('[MCP] Error closing the stdio serving handle during exit:', { err }); + if (serving.handle) { + try { + const closing = serving.handle.close(); + // A rejection after the bound below has passed has nowhere to go. + closing.catch(() => {}); + const closed = await Promise.race([ + closing.then(() => true), + unrefDelay(HANDLE_CLOSE_TIMEOUT_MS).then(() => false) + ]); + if (!closed) { + logger.debug(`[MCP] The stdio serving handle did not close within ${HANDLE_CLOSE_TIMEOUT_MS} ms; stopping the debug server anyway.`, { timeoutMs: HANDLE_CLOSE_TIMEOUT_MS }); + } + } catch (err) { + logger.debug('[MCP] Error closing the stdio serving handle during exit:', { err }); + } } try { await debugMcpServer.stop(); @@ -104,47 +133,74 @@ export async function handleStdioCommand( void Promise.race([teardown, guard]).finally(() => exitProcess(code)); }; - // Start the debug server - await debugMcpServer.start(); - logger.info('Server started successfully in stdio mode'); - - // Keep the process alive - const stdin: NodeJS.ReadableStream = dependencies.stdin ?? proc.stdin; - stdin.resume(); - // Stdin is the MCP transport here, so any byte on it proves a client was // speaking to us — which makes a later EOF mean that client is gone. // Nothing else will reap us: on Windows a dying parent delivers no signal, // and `keepAlive` above holds the event loop open, so without this the // process runs forever (issue #122). let sawClientTraffic = false; - // Additive: Node broadcasts each chunk to every 'data' listener and - // stdin.resume() above already put the stream in flowing mode, so this - // cannot starve StdioServerTransport's own reader. - stdin.on('data', () => { sawClientTraffic = true; }); // Container mode keeps one exception, and only one: `docker run` WITHOUT // -i, where stdin is already closed before any client speaks and the // server must stay alive anyway (c251b3ff, named in fa827ec2/#130). // Once traffic has been seen that case is ruled out, so EOF is a real // disconnect and the container must exit — otherwise `docker run --rm` - // never fires and every session leaks a container (issue #633). - const onStdinGone = (reason: string): void => { + // never fires and every session leaks a container (issue #633). The + // transport closing the wire by itself goes through the same gate. + const onWireGone = (reason: string): void => { // Reads MCP_CONTAINER directly on purpose: `proc` is the injected // ProcessLike this command is tested against, not an IEnvironment. if (proc.env.MCP_CONTAINER === 'true' && !sawClientTraffic) { - logger.warn('[MCP] Stdin closed before any client traffic; detached container, staying alive.'); + logger.warn(`[MCP] ${reason} before any client traffic; detached container, staying alive.`); return; } - shutdownAndExit(0, reason); + shutdownAndExit(0, `${reason}; MCP client disconnected`); }; + // Create stdio transport + logger.info('[MCP] Creating StdioServerTransport...'); + const transport = new ObservedStdioServerTransport(() => onWireGone('Stdio transport closed')); + + // The SDK's stdio serving entry owns the wire and speaks both protocol + // eras (issue #1025, step 2): the opening message pins one facade for the + // connection — a 2026-07-28 client's server/discover lands on a modern + // instance, a 2025 client's initialize on a legacy one — and the entry + // itself answers server/discover and serves subscriptions/listen. The + // factory may run twice: once for a discover probe that a client then + // abandons for initialize, and once for the pinned instance. + logger.info('[MCP] Serving stdio in both protocol eras...'); + const protocolServerFactory = dependencies.protocolServerFactory ?? createProtocolServer; + const serve = dependencies.serveStdio ?? serveStdio; + serving.handle = serve( + ({ era }) => protocolServerFactory(debugMcpServer, { era, deliver: 'instance' }), + { + transport, + onerror: (error: Error) => { + logger.error('[MCP] Transport error:', { error }); + } + } + ); + logger.info('[MCP] Server connected to stdio transport successfully'); + + // Start the debug server + await debugMcpServer.start(); + logger.info('Server started successfully in stdio mode'); + + // Keep the process alive + const stdin: NodeJS.ReadableStream = dependencies.stdin ?? proc.stdin; + stdin.resume(); + + // Additive: Node broadcasts each chunk to every 'data' listener and + // stdin.resume() above already put the stream in flowing mode, so this + // cannot starve StdioServerTransport's own reader. + stdin.on('data', () => { sawClientTraffic = true; }); + // 'end' alone misses an abruptly dropped pipe. shutdownAndExit is - // idempotent, so the usual end-then-close pair costs nothing. - stdin.on('end', () => onStdinGone('Stdin ended; MCP client disconnected')); - stdin.on('close', () => onStdinGone('Stdin closed; MCP client disconnected')); - stdin.on('error', (error: Error) => - onStdinGone(`Stdin error (${error.message}); MCP client disconnected`)); + // idempotent, so the usual end-then-close pair — and the transport's own + // close on the same EOF — cost nothing. + stdin.on('end', () => onWireGone('Stdin ended')); + stdin.on('close', () => onWireGone('Stdin closed')); + stdin.on('error', (error: Error) => onWireGone(`Stdin error (${error.message})`)); // Add robust exit/signal diagnostics (logged to file; console output is silenced for protocol safety) proc.on('SIGTERM', () => { diff --git a/tests/unit/cli/stdio-command.test.ts b/tests/unit/cli/stdio-command.test.ts index aacc4e66..ee7cfa14 100644 --- a/tests/unit/cli/stdio-command.test.ts +++ b/tests/unit/cli/stdio-command.test.ts @@ -458,5 +458,82 @@ describe('STDIO Command Handler', () => { expect(mockServer.stop).toHaveBeenCalledTimes(1); expect(mockExitProcess).toHaveBeenCalledTimes(1); }); + + it('bounds the serving handle close on its own, so a hung close still leaves the debug server its teardown inside the guard', async () => { + // Review of PR #1116, Important #1: handle.close() and core.stop() used to share + // the one 5 s guard, so a handle that never settled (a listen stream whose + // consumer stopped reading) starved closeAllSessions() entirely. + mockHandle.close = vi.fn<() => Promise>().mockReturnValue(new Promise(() => {})); + const stdin = await startWithStdin(); + vi.useFakeTimers(); + try { + stdin.emit('end'); + expect(mockServer.stop).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(1000); + + expect(mockServer.stop).toHaveBeenCalledTimes(1); + expect(mockExitProcess).toHaveBeenCalledWith(0); + expect(mockLogger.debug).toHaveBeenCalledWith(expect.stringContaining('serving handle did not close'), expect.anything()); + } finally { + vi.useRealTimers(); + } + }); + }); + + describe('the stdio wire closing under the serving entry (issue #1025, step 2)', () => { + // The SDK transport closes itself — and tells only its own onclose, which + // serveStdio owns — on a stdout error (the client's end of the pipe is gone) + // or an oversized frame, with stdin still open: no EOF handler ever fires. + // Review of PR #1116, Important #2. + function servedTransport(): StdioServerTransport { + const [, serveOptions] = mockServeStdio.mock.calls[0]; + return serveOptions!.transport as StdioServerTransport; + } + + async function start(stdin = makeFakeStdin()) { + await handleStdioCommand({}, { + logger: mockLogger, + serverFactory: mockServerFactory, + protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, + exitProcess: mockExitProcess, + stdin, + proc: fakeProc + }); + return stdin; + } + + it('stops the debug server and exits when the transport closes the wire while stdin is still open', async () => { + await start(); + + await servedTransport().close(); + + await vi.waitFor(() => expect(mockExitProcess).toHaveBeenCalledWith(0)); + expect(mockServer.stop).toHaveBeenCalledTimes(1); + expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('Stdio transport closed')); + }); + + it('runs the wire close through the same container gate as a stdin EOF: no client ever spoke, so stay alive', async () => { + fakeProc.env.MCP_CONTAINER = 'true'; + await start(); + + await servedTransport().close(); + + expect(mockExitProcess).not.toHaveBeenCalled(); + expect(mockServer.stop).not.toHaveBeenCalled(); + expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('detached container, staying alive')); + }); + + it('tears down once when the shutdown itself closes the wire (the hook re-enters an idempotent shutdown)', async () => { + mockHandle.close = vi.fn<() => Promise>(async () => { await servedTransport().close(); }); + const stdin = await start(); + + stdin.emit('end'); + + await vi.waitFor(() => expect(mockExitProcess).toHaveBeenCalledWith(0)); + expect(mockServer.stop).toHaveBeenCalledTimes(1); + expect(mockExitProcess).toHaveBeenCalledTimes(1); + }); }); }); From 53c92ccc68745597b78968540863d1f140eec388 Mon Sep 17 00:00:00 2001 From: JF Date: Tue, 10 Nov 2026 11:15:30 -0500 Subject: [PATCH 3/3] fix(stdio): name a 2026-07-28 client in the log, and word the exit line from what happened (#1025) A second look at two rulings made while building step 2, on JF's review. The plan called for a log line naming each modern-era client; it was dropped because a facade per request would have logged every request, leaving nothing in the server's own log to say a 2026-07-28 client was ever there (the facade's comment claimed they "are identified per request"). The 2026 era has no handshake, but every request carries the client's name, version and protocol revision in its envelope. A modern facade now registers its handlers through a registrar that hands that envelope to the core first, and the core logs "[MCP Server] Client identified" once per distinct client per process: one line for a stdio connection, one for an http client however many per-request facades serve it. Client info is optional and self-reported, so it is a bounded label for the log and nothing else, and the set of identities is capped. The three handler modules now take Pick, which is all they ever used. The stdio exit line said "Stdio transport closed; MCP client disconnected" whenever the transport's close won the race, including after a stdout error or an oversized frame with the client still there. It is now worded from facts: a stdin that ended means the client left; a transport that closed itself with stdin open names the error it reported, and does not blame the client. Also: the stdio era test reads the tool count from TOOL_NAMES, and getting-started no longer says Claude Code probes on every connect two sentences before describing its seven-day legacy memory. Co-Authored-By: Claude Fable 5.2 --- changelog.d/1025-protocol-2026-stdio.added.md | 2 +- docs/getting-started.md | 2 +- src/cli/stdio-command.ts | 32 +++++-- src/server.ts | 26 +++++ src/server/output-resources.ts | 2 +- src/server/prompts.ts | 2 +- src/server/protocol-facade.ts | 48 +++++++++- src/server/tool-dispatch.ts | 2 +- .../server/protocol-facade-transport.test.ts | 94 +++++++++++++++++++ .../cli/stdio-era-negotiation.test.ts | 3 +- tests/unit/cli/stdio-command.test.ts | 38 ++++++++ 11 files changed, 233 insertions(+), 18 deletions(-) diff --git a/changelog.d/1025-protocol-2026-stdio.added.md b/changelog.d/1025-protocol-2026-stdio.added.md index d7086aaa..8336db6b 100644 --- a/changelog.d/1025-protocol-2026-stdio.added.md +++ b/changelog.d/1025-protocol-2026-stdio.added.md @@ -1 +1 @@ -**`stdio` speaks the current MCP protocol revision, 2026-07-28, beside 2025-11-25 and earlier.** The stdio transport is served through the SDK's own serving entry, which answers a client's `server/discover` probe and pins the connection to whichever era its opening message chose: a probing client — Claude Code's v2 runtime probes every stdio server on every connect, and until now recorded mcp-debugger as a legacy peer — lands on 2026-07-28, with `subscriptions/listen` for resource notifications and no `initialize` handshake; a client that opens with `initialize` gets the 2025 era exactly as before, `resources/subscribe` included. The 29 tools, the resources and the prompt are the same in both. Claude Code may keep a stdio server it once saw answer as legacy pinned that way for up to seven days; restart it after upgrading, or wait it out. Step 2 of #1025; `http` follows. +**`stdio` speaks the current MCP protocol revision, 2026-07-28, beside 2025-11-25 and earlier.** The stdio transport is served through the SDK's own serving entry, which answers a client's `server/discover` probe and pins the connection to whichever era its opening message chose: a probing client — Claude Code's v2 runtime probes its stdio servers, and until now recorded mcp-debugger as a legacy peer — lands on 2026-07-28, with `subscriptions/listen` for resource notifications and no `initialize` handshake; a client that opens with `initialize` gets the 2025 era exactly as before, `resources/subscribe` included. The 29 tools, the resources and the prompt are the same in both. The server log names each 2026-07-28 client once, on its first handled request (`[MCP Server] Client identified`, with its self-reported name and version and its protocol revision), as `Client initialized` does for a 2025 client; and the stdio exit line says what happened — the client left (stdin ended), or the transport closed after an error it names. Claude Code may keep a stdio server it once saw answer as legacy pinned that way for up to seven days; restart it after upgrading, or wait it out. Step 2 of #1025; `http` follows. diff --git a/docs/getting-started.md b/docs/getting-started.md index 21c0ef96..13e0c750 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -100,7 +100,7 @@ explicitly in a config file is still worth doing: it documents the intent. Conso silenced in every mode so nothing can corrupt the JSON-RPC stream. Both eras of the MCP protocol are served on `stdio`: a client that probes with `server/discover` -(Claude Code's v2 runtime does, on every connect) gets the current 2026-07-28 revision, and a +(Claude Code's v2 runtime does) gets the current 2026-07-28 revision, and a client that opens with `initialize` gets 2025-11-25 or earlier, as before. Claude Code may keep a stdio server it once saw answer as legacy pinned that way for up to seven days; restarting Claude Code after upgrading the server, or waiting it out, lets it probe again. `http` speaks 2025-11-25 diff --git a/src/cli/stdio-command.ts b/src/cli/stdio-command.ts index 899de3bc..de7f55db 100644 --- a/src/cli/stdio-command.ts +++ b/src/cli/stdio-command.ts @@ -144,19 +144,37 @@ export async function handleStdioCommand( // disconnect and the container must exit — otherwise `docker run --rm` // never fires and every session leaks a container (issue #633). The // transport closing the wire by itself goes through the same gate. - const onWireGone = (reason: string): void => { + const onWireGone = (reason: string, clientLeft: boolean): void => { // Reads MCP_CONTAINER directly on purpose: `proc` is the injected // ProcessLike this command is tested against, not an IEnvironment. if (proc.env.MCP_CONTAINER === 'true' && !sawClientTraffic) { logger.warn(`[MCP] ${reason} before any client traffic; detached container, staying alive.`); return; } - shutdownAndExit(0, `${reason}; MCP client disconnected`); + shutdownAndExit(0, clientLeft ? `${reason}; MCP client disconnected` : reason); + }; + + // The exit line is what an operator reads to learn why the server went + // away, so it says what is true rather than which listener ran first: only + // a stdin that ended means the client left. The transport also closes the + // wire by itself after a stdout error or an oversized frame, with the + // client possibly still there — then the line names that error instead. + const stdin: NodeJS.ReadableStream = dependencies.stdin ?? proc.stdin; + let lastTransportError: Error | undefined; + const onTransportClosed = (): void => { + const { readableEnded, destroyed } = stdin as { readableEnded?: boolean; destroyed?: boolean }; + if (readableEnded || destroyed) { + onWireGone('Stdin ended', true); + } else if (lastTransportError) { + onWireGone(`Stdio transport closed after an error (${lastTransportError.message})`, false); + } else { + onWireGone('Stdio transport closed', false); + } }; // Create stdio transport logger.info('[MCP] Creating StdioServerTransport...'); - const transport = new ObservedStdioServerTransport(() => onWireGone('Stdio transport closed')); + const transport = new ObservedStdioServerTransport(onTransportClosed); // The SDK's stdio serving entry owns the wire and speaks both protocol // eras (issue #1025, step 2): the opening message pins one facade for the @@ -173,6 +191,7 @@ export async function handleStdioCommand( { transport, onerror: (error: Error) => { + lastTransportError = error; logger.error('[MCP] Transport error:', { error }); } } @@ -184,7 +203,6 @@ export async function handleStdioCommand( logger.info('Server started successfully in stdio mode'); // Keep the process alive - const stdin: NodeJS.ReadableStream = dependencies.stdin ?? proc.stdin; stdin.resume(); // Additive: Node broadcasts each chunk to every 'data' listener and @@ -195,9 +213,9 @@ export async function handleStdioCommand( // 'end' alone misses an abruptly dropped pipe. shutdownAndExit is // idempotent, so the usual end-then-close pair — and the transport's own // close on the same EOF — cost nothing. - stdin.on('end', () => onWireGone('Stdin ended')); - stdin.on('close', () => onWireGone('Stdin closed')); - stdin.on('error', (error: Error) => onWireGone(`Stdin error (${error.message})`)); + stdin.on('end', () => onWireGone('Stdin ended', true)); + stdin.on('close', () => onWireGone('Stdin closed', true)); + stdin.on('error', (error: Error) => onWireGone(`Stdin error (${error.message})`, true)); // Add robust exit/signal diagnostics (logged to file; console output is silenced for protocol safety) proc.on('SIGTERM', () => { diff --git a/src/server.ts b/src/server.ts index 9ab07607..474296f5 100644 --- a/src/server.ts +++ b/src/server.ts @@ -126,6 +126,9 @@ export class DebugMcpServer implements ToolContext { * closes nothing. The transport layer supplies it; the default says nobody is. */ private attendedSupplier: () => boolean = () => false; + /** The 2026-07-28 clients already named in the log, by identity; bounded. */ + private readonly identifiedClients = new Set(); + private static readonly IDENTIFIED_CLIENT_LIMIT = 256; /** @internal Language discovery is a ToolContext service; see src/server/language-discovery.ts. */ public async getSupportedLanguagesAsync(): Promise { @@ -843,6 +846,29 @@ export class DebugMcpServer implements ToolContext { } } + /** + * A 2026-07-28 client reached a handler (issue #1025, step 2). The modern + * era has no handshake, so this is the counterpart of the legacy + * `Client initialized` line: each distinct client — its self-reported name + * and version, and its protocol revision — is logged once per process, + * whether it is one stdio connection or an `http` client served by a facade + * per request. Past the bound a new identity is logged at debug and not + * remembered, so a client that varies its name cannot grow the set. + */ + public noteModernClient(identity: { protocolVersion?: string; client?: string }): void { + const key = `${identity.protocolVersion ?? ''}|${identity.client ?? ''}`; + if (this.identifiedClients.has(key)) { + return; + } + const line = { era: 'modern', protocolVersion: identity.protocolVersion, client: identity.client }; + if (this.identifiedClients.size >= DebugMcpServer.IDENTIFIED_CLIENT_LIMIT) { + this.logger.debug('[MCP Server] Client identified', line); + return; + } + this.identifiedClients.add(key); + this.logger.info('[MCP Server] Client identified', line); + } + /** Who decides whether a client is attending the process; see attendedSupplier. */ public setAttendedSupplier(supplier: () => boolean): void { this.attendedSupplier = supplier; diff --git a/src/server/output-resources.ts b/src/server/output-resources.ts index 44a78938..656ef574 100644 --- a/src/server/output-resources.ts +++ b/src/server/output-resources.ts @@ -69,7 +69,7 @@ export class FacadeSubscriptions implements ResourceSubscriptions { * the bounded proxy-log tail is an on-demand diagnostic snapshot. */ export function registerResourceHandlers( - server: Server, + server: Pick, sessionManager: SessionManager, subscriptions: ResourceSubscriptions, fileSystem: Pick diff --git a/src/server/prompts.ts b/src/server/prompts.ts index 7ffa3a5b..b96f1b89 100644 --- a/src/server/prompts.ts +++ b/src/server/prompts.ts @@ -10,7 +10,7 @@ import { WireMcpError } from '../errors/debug-errors.js'; import { buildDebuggingWorkflowPrompt } from '../skill-content.js'; import { getBpAddressingMode } from '../utils/bp-addressing.js'; -export function registerPromptHandlers(server: Server, environment: IEnvironment): void { +export function registerPromptHandlers(server: Pick, environment: IEnvironment): void { const promptDescriptor = { name: 'debugging-workflow', description: diff --git a/src/server/protocol-facade.ts b/src/server/protocol-facade.ts index a3195d1b..421e6705 100644 --- a/src/server/protocol-facade.ts +++ b/src/server/protocol-facade.ts @@ -9,7 +9,7 @@ * that lives exactly as long as the facade does. No dependency container, no * SessionManager, no logger of its own. */ -import { Server } from '@modelcontextprotocol/server'; +import { CLIENT_INFO_META_KEY, PROTOCOL_VERSION_META_KEY, Server } from '@modelcontextprotocol/server'; import type { DebugMcpServer } from '../server.js'; import { facadeSink } from './notification-hub.js'; import { FacadeSubscriptions, registerResourceHandlers } from './output-resources.js'; @@ -31,6 +31,42 @@ export interface ProtocolFacadeOptions { deliver: 'instance' | 'external'; } +type Registrar = Pick; +type AnyHandler = (request: unknown, callCtx: unknown) => unknown; + +/** What a 2026-07-28 request says about its sender: the envelope the SDK lifts out of `_meta`. */ +function modernClientOf(server: Server, callCtx: unknown): { protocolVersion?: string; client?: string } { + // The SDK types the envelope as `{}`; the keys are its own exported constants. + const envelope = (callCtx as { mcpReq?: { envelope?: Record } } | undefined)?.mcpReq?.envelope; + const revision = envelope?.[PROTOCOL_VERSION_META_KEY]; + const info = envelope?.[CLIENT_INFO_META_KEY] as { name?: unknown; version?: unknown } | undefined; + // Client info is optional and self-reported: a label for the log, bounded, never a key for behaviour. + const client = typeof info?.name === 'string' + ? `${info.name}${typeof info.version === 'string' ? ` ${info.version}` : ''}`.slice(0, 200) + : undefined; + return { + protocolVersion: typeof revision === 'string' ? revision : server.getNegotiatedProtocolVersion(), + client + }; +} + +/** + * A registrar for a modern-era facade: every handler registered through it + * first tells the core who is calling. The 2026-07-28 era has no handshake to + * log a client from (`oninitialized` never fires), so the first request that + * reaches a handler is where a client becomes known; the core logs each + * distinct client once per process. + */ +function identifyingClients(server: Server, core: DebugMcpServer): Registrar { + const register = server.setRequestHandler.bind(server) as unknown as (method: string, handler: AnyHandler) => void; + const setRequestHandler = (method: string, handler: AnyHandler): void => + register(method, (request, callCtx) => { + core.noteModernClient(modernClientOf(server, callCtx)); + return handler(request, callCtx); + }); + return { setRequestHandler: setRequestHandler as unknown as Server['setRequestHandler'] }; +} + export function createProtocolServer(core: DebugMcpServer, options: ProtocolFacadeOptions): Server { // Everything the Server advertises is read off the core, computed once per // process: a facade is built per connection or per request. @@ -45,12 +81,14 @@ export function createProtocolServer(core: DebugMcpServer, options: ProtocolFaca // resources/subscribe bookkeeping is per facade: a subscription belongs to // the connection that made it. const subscriptions = new FacadeSubscriptions(); - registerToolHandlers(server, core); - registerResourceHandlers(server, core.sessionManager, subscriptions, core.fileSystem); - registerPromptHandlers(server, core.environment); + const registrar: Registrar = options.era === 'modern' ? identifyingClients(server, core) : server; + registerToolHandlers(registrar, core); + registerResourceHandlers(registrar, core.sessionManager, subscriptions, core.fileSystem); + registerPromptHandlers(registrar, core.environment); // The protocol version each legacy client negotiates (issue #1025). The - // modern era has no initialize; its clients are identified per request. + // modern era has no initialize; its clients are named on their first + // handled request, through the registrar above. server.oninitialized = () => { const client = server.getClientVersion(); core.logger.info('[MCP Server] Client initialized', { diff --git a/src/server/tool-dispatch.ts b/src/server/tool-dispatch.ts index b239008b..5529cf5a 100644 --- a/src/server/tool-dispatch.ts +++ b/src/server/tool-dispatch.ts @@ -73,7 +73,7 @@ function dispatchIndexFor(ctx: ToolContext): ReadonlyMap return index; } -export function registerToolHandlers(server: Server, ctx: ToolContext): void { +export function registerToolHandlers(server: Pick, ctx: ToolContext): void { const dispatchDefinitions = dispatchIndexFor(ctx); server.setRequestHandler('tools/list', async () => { diff --git a/tests/core/unit/server/protocol-facade-transport.test.ts b/tests/core/unit/server/protocol-facade-transport.test.ts index cddbef4d..30aea085 100644 --- a/tests/core/unit/server/protocol-facade-transport.test.ts +++ b/tests/core/unit/server/protocol-facade-transport.test.ts @@ -9,6 +9,7 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { InMemoryTransport } from '@modelcontextprotocol/server'; +import { serveStdio } from '@modelcontextprotocol/server/stdio'; import { Client } from '@modelcontextprotocol/client'; import { DebugMcpServer } from '../../../../src/server.js'; import { createProtocolServer } from '../../../../src/server/protocol-facade.js'; @@ -71,3 +72,96 @@ describe('createProtocolServer over a real transport', () => { await client.close().catch(() => undefined); }); }); + +/** + * Who is connected, in the server's own log (issue #1025, step 2). A 2025 + * client is named by the `Client initialized` line its handshake writes. A + * 2026-07-28 client has no handshake — its name, version and protocol + * revision ride on every request — so the facade names it on the first + * request that reaches a handler, once per distinct client per process: a + * stdio connection logs one line, and so does an `http` client however many + * per-request facades serve it. + */ +describe('a 2026-07-28 client is identified in the log', () => { + let core: DebugMcpServer; + let mockSessionManager: MockSessionManager; + let mockDependencies: Dependencies; + const closers: Array<() => Promise> = []; + + beforeEach(() => { + mockDependencies = createMockDependencies(); + vi.mocked(createProductionDependencies).mockReturnValue(mockDependencies); + mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); + vi.mocked(SessionManager).mockImplementation(function () { return mockSessionManager as any; }); + core = new DebugMcpServer(); + }); + + afterEach(async () => { + for (const close of closers.splice(0)) await close().catch(() => undefined); + mockSessionManager.closeAllSessions.mockResolvedValue(undefined); + await core.stop(); + vi.clearAllMocks(); + }); + + /** One client over the SDK's stdio entry, the way the stdio command serves it. */ + async function connect(name: string, mode: 'legacy' | { pin: string }): Promise { + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const handle = serveStdio(({ era }) => createProtocolServer(core, { era, deliver: 'instance' }), { transport: serverTransport }); + const client = new Client({ name, version: '1.2.3' }, { versionNegotiation: { mode } }); + await client.connect(clientTransport); + closers.push(() => client.close(), () => handle.close()); + return client; + } + + const identified = () => + vi.mocked(mockDependencies.logger.info).mock.calls.filter(([message]) => message === '[MCP Server] Client identified'); + + it('names the client and its protocol revision on its first handled request, and only once', async () => { + const client = await connect('era-client', { pin: '2026-07-28' }); + expect(identified()).toHaveLength(0); + + await client.listPrompts(); + await client.listPrompts(); + await client.listResources(); + + expect(identified()).toEqual([ + ['[MCP Server] Client identified', { era: 'modern', protocolVersion: '2026-07-28', client: 'era-client 1.2.3' }] + ]); + }); + + it('logs the same client once per process, and a different client once more', async () => { + await (await connect('era-client', { pin: '2026-07-28' })).listPrompts(); + await (await connect('era-client', { pin: '2026-07-28' })).listPrompts(); + await (await connect('other-client', { pin: '2026-07-28' })).listPrompts(); + + expect(identified().map(([, meta]) => (meta as { client: string }).client)).toEqual(['era-client 1.2.3', 'other-client 1.2.3']); + }); + + it('leaves a 2025 client to its Client initialized line', async () => { + await (await connect('legacy-client', 'legacy')).listPrompts(); + + expect(identified()).toHaveLength(0); + expect(mockDependencies.logger.info).toHaveBeenCalledWith( + '[MCP Server] Client initialized', + expect.objectContaining({ client: 'legacy-client 1.2.3' }) + ); + }); + + it('names a client that sent no client info by its protocol revision alone', () => { + core.noteModernClient({ protocolVersion: '2026-07-28' }); + + expect(identified()).toEqual([ + ['[MCP Server] Client identified', { era: 'modern', protocolVersion: '2026-07-28', client: undefined }] + ]); + }); + + it('keeps a bounded set of identities: past it, a new one is logged at debug and not remembered', () => { + for (let i = 0; i < 300; i++) core.noteModernClient({ protocolVersion: '2026-07-28', client: `client-${i} 1.0.0` }); + + expect(identified()).toHaveLength(256); + expect(mockDependencies.logger.debug).toHaveBeenCalledWith( + '[MCP Server] Client identified', + expect.objectContaining({ client: 'client-299 1.0.0' }) + ); + }); +}); diff --git a/tests/integration/cli/stdio-era-negotiation.test.ts b/tests/integration/cli/stdio-era-negotiation.test.ts index 930d865d..6f6f6836 100644 --- a/tests/integration/cli/stdio-era-negotiation.test.ts +++ b/tests/integration/cli/stdio-era-negotiation.test.ts @@ -13,6 +13,7 @@ import { fileURLToPath } from 'node:url'; import { Client } from '@modelcontextprotocol/client'; import { StdioClientTransport } from '@modelcontextprotocol/client/stdio'; import { serverEnvironment } from '../../test-utils/helpers/mcp-stdio-client.js'; +import { TOOL_NAMES } from '../../../src/server/tool-schemas.js'; const root = fileURLToPath(new URL('../../../', import.meta.url)); const entry = path.join(root, 'dist/index.js'); @@ -65,7 +66,7 @@ describe('stdio serves both protocol eras (issue #1025, step 2)', () => { expect(client.getProtocolEra()).toBe('modern'); expect(client.getNegotiatedProtocolVersion()).toBe('2026-07-28'); expect(client.getServerVersion()?.name).toBe('debug-mcp-server'); - expect((await client.listTools()).tools.length).toBe(29); + expect((await client.listTools()).tools.length).toBe(TOOL_NAMES.length); }, 60_000); it('a client pinned to 2026-07-28 drives a full debug session and hears list changes on its listen stream', async () => { diff --git a/tests/unit/cli/stdio-command.test.ts b/tests/unit/cli/stdio-command.test.ts index 5a6995c4..816f0e50 100644 --- a/tests/unit/cli/stdio-command.test.ts +++ b/tests/unit/cli/stdio-command.test.ts @@ -513,6 +513,44 @@ describe('STDIO Command Handler', () => { expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('Stdio transport closed')); }); + // The exit line is what an operator reads to learn why the server went + // away, so it is worded from what is true, not from which listener ran + // first: only a stdin that ended means the client left. + const exitLines = () => vi.mocked(mockLogger.warn).mock.calls.map(([line]) => String(line)).join(' | '); + + it('names the transport error and does not blame the client when the wire closes after a stdout error', async () => { + await start(); + const [, serveOptions] = mockServeStdio.mock.calls[0]; + + serveOptions!.onerror!(new Error('write EPIPE')); + await servedTransport().close(); + + await vi.waitFor(() => expect(mockExitProcess).toHaveBeenCalledWith(0)); + expect(exitLines()).toContain('Stdio transport closed after an error (write EPIPE)'); + expect(exitLines()).not.toContain('MCP client disconnected'); + }); + + it('says only that the transport closed when no error was reported and stdin is still open', async () => { + await start(); + + await servedTransport().close(); + + await vi.waitFor(() => expect(mockExitProcess).toHaveBeenCalledWith(0)); + expect(exitLines()).toContain('Stdio transport closed —'); + expect(exitLines()).not.toContain('MCP client disconnected'); + }); + + it('calls a wire close the client leaving when stdin has already ended', async () => { + const stdin = makeFakeStdin(); + (stdin as unknown as { readableEnded: boolean }).readableEnded = true; + await start(stdin); + + await servedTransport().close(); + + await vi.waitFor(() => expect(mockExitProcess).toHaveBeenCalledWith(0)); + expect(exitLines()).toContain('Stdin ended; MCP client disconnected'); + }); + it('runs the wire close through the same container gate as a stdin EOF: no client ever spoke, so stay alive', async () => { fakeProc.env.MCP_CONTAINER = 'true'; await start();