diff --git a/CLAUDE.md b/CLAUDE.md index 7be61135..cd2f68bf 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 ends by its own close, its program ending, or the server exiting. The per-debug-session idle policy is the operator's opt-in (`MCP_DEBUG_SESSION_IDLE_MS`, off by default on every transport; sweep `MCP_DEBUG_SESSION_IDLE_SWEEP_MS`): when set, an abandoned session tree is closed unless a client is attending the process (an open SSE stream), `list_debug_sessions` rows show `idleMs`/`expiresInMs`, and a call naming a session the policy closed answers `closedBy: "idle-policy"`. A session outside the caller's owner scope is hidden from the call's lookups (`src/session/session-scope.ts`), so every tool answers it as an unknown id. 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 ends by its own close, its program ending, or the server exiting. The per-debug-session idle policy is the operator's opt-in (`MCP_DEBUG_SESSION_IDLE_MS`, off by default on every transport; sweep `MCP_DEBUG_SESSION_IDLE_SWEEP_MS`): when set, an abandoned session tree is closed unless a client is attending the process (an open SSE stream), `list_debug_sessions` rows show `idleMs`/`expiresInMs`, and a call naming a session the policy closed answers `closedBy: "idle-policy"`. A session outside the caller's owner scope is hidden from the call's lookups (`src/session/session-scope.ts`), so every tool answers it as an unknown id. 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 ec838374..e569dfb3 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..8336db6b --- /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 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 702fcbf0..13e0c750 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) 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 6f3731d5..de7f55db 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'; @@ -7,11 +7,55 @@ 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; /** 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,111 +69,153 @@ 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({ logLevel: options.logLevel, logFile: options.logFile }); - - // 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...'); - const protocolServerFactory = dependencies.protocolServerFactory ?? createProtocolServer; - const facade = protocolServerFactory(debugMcpServer, { era: 'legacy', deliver: 'instance' }); - await facade.connect(transport); - 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 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). `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?.(); - }); - void Promise.race([ - debugMcpServer.stop().catch((err) => { + const guard = unrefDelay(SHUTDOWN_GUARD_MS); + const teardown = (async () => { + 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(); + } 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(); - // 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 - // 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, 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] 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, 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(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 + // 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) => { + lastTransportError = 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 + 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', 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 841e01e7..65d483d0 100644 --- a/src/server/tool-dispatch.ts +++ b/src/server/tool-dispatch.ts @@ -77,7 +77,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-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/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 new file mode 100644 index 00000000..6f6f6836 --- /dev/null +++ b/tests/integration/cli/stdio-era-negotiation.test.ts @@ -0,0 +1,145 @@ +/** + * 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'; +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'); +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(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 () => { + 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 325ac198..816f0e50 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 @@ -168,6 +187,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, proc: fakeProc }); @@ -189,6 +209,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, proc: fakeProc }); @@ -206,6 +227,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, proc: fakeProc }); @@ -225,6 +247,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -252,6 +275,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -275,6 +299,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -331,6 +356,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -352,6 +378,7 @@ describe('STDIO Command Handler', () => { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, + serveStdio: mockServeStdio, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -359,17 +386,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(); @@ -415,5 +457,120 @@ 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')); + }); + + // 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(); + + 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); + }); }); });