diff --git a/CLAUDE.md b/CLAUDE.md index 973dc732..478a6fe8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -220,7 +220,7 @@ The codebase follows a **layered architecture with dependency injection** and ** 1. **MCP Server Layer** (`src/server.ts`, `src/server/`, `src/index.ts`) - Entry point for MCP protocol communication - - `src/server.ts` is the composition root (`DebugMcpServer`: dependencies, lifecycle, session validation, breakpoint gating, the public facade methods); it implements `ToolContext` and passes itself to the handlers + - `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 - Dynamically discovers available language adapters diff --git a/docs/architecture/system-overview.md b/docs/architecture/system-overview.md index eedcedb9..61fda6c6 100644 --- a/docs/architecture/system-overview.md +++ b/docs/architecture/system-overview.md @@ -54,7 +54,8 @@ graph TB ### 1. MCP Server Layer (`src/server.ts`, `src/server/`, `src/index.ts`) - **Purpose**: Entry point for MCP protocol communication - **Key Files**: - - `src/server.ts` - `DebugMcpServer`, the composition root: dependencies, lifecycle, session validation, breakpoint gating and the public facade methods; implements `ToolContext` + - `src/server.ts` - `DebugMcpServer`, the composition root and the shared core: dependencies, lifecycle, session validation, breakpoint gating, the public facade methods and the `NotificationHub`; implements `ToolContext`. It owns no SDK `Server` + - `src/server/protocol-facade.ts` - `createProtocolServer(core, { era, deliver })`: one cheap SDK `Server` per connection or request over the core (the eight MCP handlers, the error and initialized hooks, one notification sink that leaves with the facade); `src/server/notification-hub.ts` - the process-wide fan-out of coalesced `resources/updated` pings and `resources/list_changed` to every facade's sink (issue #1025) - `src/server/` - 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 - `src/index.ts` - the `dist/index.js` entry: the prologue (`src/cli/entry-prologue.ts`, console silencing and argv cleanup, evaluated before anything else), then `main()` from `src/main.ts` — the side-effect-free module the npm bundle's `cli-entry.ts` imports directly (#732); the four subcommands are wired in `src/cli/setup.ts`: `stdio` (default), `http` (Streamable HTTP, recommended, `-p/--port`, default 3001), `doctor` (toolchain self-check), `check-rust-binary` - `src/server/output-resources.ts` - the MCP resource handlers: `debug://sessions/{id}/output` (subscribable plain-text transcript of captured debuggee output, coalesced `resources/updated` notifications) and `debug://sessions/{id}/proxy-log` (a sanitized, bounded tail of the session's proxy log — 64 KiB / 80 lines, on-demand snapshot, not subscribable, and listed only once the session has a log directory) diff --git a/src/cli/http-command.ts b/src/cli/http-command.ts index 7e92df96..194a14c0 100644 --- a/src/cli/http-command.ts +++ b/src/cli/http-command.ts @@ -3,9 +3,10 @@ import type { Express, Request, Response, NextFunction } from 'express'; import express from 'express'; import { randomUUID } from 'crypto'; import { IncomingMessage, ServerResponse } from 'http'; -import { isInitializeRequest, ProtocolErrorCode } from '@modelcontextprotocol/server'; +import { isInitializeRequest, ProtocolErrorCode, type Server } from '@modelcontextprotocol/server'; import { NodeStreamableHTTPServerTransport } from '@modelcontextprotocol/node'; import { DebugMcpServer, type DebugMcpServerOptions } from '../server.js'; +import { createProtocolServer } from '../server/protocol-facade.js'; import { attachSharedFileTransport } from '../utils/logger.js'; import type { HttpOptions } from './setup.js'; import { @@ -68,6 +69,8 @@ export function describeForeignDebugSession( export interface HttpCommandDependencies { logger: WinstonLoggerType; serverFactory: (options: ServerFactoryOptions) => DebugMcpServer; + /** Builds the SDK Server facade over a core (issue #1025, step 2); defaults to createProtocolServer. */ + protocolServerFactory?: typeof createProtocolServer; exitProcess?: (code: number) => void; /** Injectable stdin for tests; defaults to proc.stdin. */ stdin?: NodeJS.ReadStream; @@ -78,6 +81,8 @@ export interface HttpCommandDependencies { interface SessionData { transport: NodeStreamableHTTPServerTransport; server: DebugMcpServer; + /** The SDK Server facade connected to this session's transport (issue #1025, step 2). */ + facade: Server; /** Timestamp of the last routed request (or stream close) for this session. */ lastActivity: number; /** Open SSE (GET) streams; a session with a live stream is never idle. */ @@ -181,6 +186,7 @@ export function createHttpApp( dependencies: HttpCommandDependencies ): Express { const { logger, serverFactory } = dependencies; + const protocolServerFactory = dependencies.protocolServerFactory ?? createProtocolServer; const proc = dependencies.proc ?? process; // The app is built here rather than with the SDK's createMcpExpressApp() @@ -346,6 +352,9 @@ export function createHttpApp( describeForeignSession, }); await newDebugServer.start(); + // The SDK Server this session speaks through: a legacy-era facade over + // the core (issue #1025, step 2). + const facade = protocolServerFactory(newDebugServer, { era: 'legacy', deliver: 'instance' }); // Forward declarations so the closures below can refer to the transport. // The SDK assigns its own internal sessionId before invoking onsessioninitialized. @@ -358,6 +367,7 @@ export function createHttpApp( httpSessions.set(sid, { transport: createdTransport, server: newDebugServer, + facade, lastActivity: Date.now(), openStreams: 0, hadStream: false, @@ -368,6 +378,10 @@ export function createHttpApp( }); createdTransport = newTransport; + // Set BEFORE facade.connect(): the SDK wraps a transport.onclose that + // is already in place and then runs its own teardown, which is what + // reaches server.onclose and detaches the facade's notification sink. + // Assigning after connect() would replace that wrapper (issue #1025). newTransport.onclose = () => { const sid = newTransport.sessionId; if (!sid) return; @@ -385,7 +399,7 @@ export function createHttpApp( logger.error(`HTTP transport error for session ${sid}`, error); }; - await newDebugServer.server.connect(newTransport); + await facade.connect(newTransport); transport = newTransport; } else if (sessionId) { logger.warn('Rejecting MCP request: unknown session ID', { method: req.method }); diff --git a/src/cli/stdio-command.ts b/src/cli/stdio-command.ts index 8f29dc34..9b34e732 100644 --- a/src/cli/stdio-command.ts +++ b/src/cli/stdio-command.ts @@ -1,6 +1,7 @@ import type { Logger as WinstonLoggerType } from 'winston'; import { StdioServerTransport } from '@modelcontextprotocol/server/stdio'; import { DebugMcpServer } from '../server.js'; +import { createProtocolServer } from '../server/protocol-facade.js'; import type { StdioOptions } from './setup.js'; import type { ProcessLike } from '../interfaces/process-interfaces.js'; @@ -12,6 +13,8 @@ export interface ServerFactoryOptions { 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; exitProcess?: (code: number) => void; /** Injectable stdin for tests; defaults to proc.stdin. */ stdin?: NodeJS.ReadStream; @@ -46,9 +49,12 @@ export async function handleStdioCommand( // Cleared on transport close or signals. const keepAlive = setInterval(() => {}, 60000); - // Connect MCP server to transport + // 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...'); - await debugMcpServer.server.connect(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): diff --git a/src/server.ts b/src/server.ts index 21697d7c..1b24fbf5 100644 --- a/src/server.ts +++ b/src/server.ts @@ -5,7 +5,7 @@ * the sibling directory src/server/ (NodeNext resolves ./server.js and * ./server/x.js without conflict). */ -import { Server, ProtocolError, ProtocolErrorCode } from '@modelcontextprotocol/server'; +import { ProtocolError, ProtocolErrorCode } from '@modelcontextprotocol/server'; import { buildServerInstructions } from './skill-content.js'; import { getVersion } from './cli/version.js'; import { @@ -29,6 +29,7 @@ import { isSessionTerminated, SessionState, IEnvironment, + IFileSystem, ILogger, ExceptionBreakMode } from '@debugmcp/shared'; @@ -49,15 +50,14 @@ import { import { isRedactionEnabled } from './utils/redaction-mode.js'; import { getVariableAccessMode } from './utils/variable-access.js'; import { assertLineContent, resolveStatement, stripTrailingComment } from './utils/breakpoint-resolver.js'; -import { OutputResourceNotifier, registerResourceHandlers } from './server/output-resources.js'; -import { registerPromptHandlers } from './server/prompts.js'; +import { outputResourceUri } from './server/output-resources.js'; +import { NotificationHub } from './server/notification-hub.js'; import { discoverSupportedLanguages, buildLanguageMetadata, LanguageMetadata } from './server/language-discovery.js'; import type { ToolContext, SetBreakpointRequest } from './server/tool-context.js'; -import { registerToolHandlers } from './server/tool-dispatch.js'; export { coerceToolArguments } from './server/tool-arguments.js'; export type { SetBreakpointRequest }; @@ -79,10 +79,13 @@ export interface DebugMcpServerOptions { } /** - * Main Debug MCP Server class + * The debug core (issue #1025, step 2): the dependency container, the + * SessionManager, the per-process services and the notification hub, shared by + * every protocol facade in the process. It owns no SDK Server: the CLI + * commands build one per connection or request with + * `createProtocolServer(core, …)` (src/server/protocol-facade.ts). */ export class DebugMcpServer implements ToolContext { - public server: Server; /** @internal ToolContext dependency; read live by the tool handlers. */ public readonly sessionManager: SessionManager; /** @internal ToolContext dependency; read live by the tool handlers. */ @@ -97,10 +100,26 @@ export class DebugMcpServer implements ToolContext { public readonly buildStaleness: BuildStalenessChecker; /** @internal ToolContext dependency; read live by the tool handlers. */ public readonly environment: IEnvironment; + /** The file system the resource handlers read proxy-log tails through. */ + public readonly fileSystem: IFileSystem; + /** + * The release version every facade advertises (clients and the MCP Registry + * show it; the Docker bundle reads it baked in, issue #1054). Read once here: + * the helper walks the file system, and a facade is built per request. + */ + public readonly version: string; + /** + * The MCP `instructions` every facade advertises. Mode-gated (issue #271): + * the handshake must not teach restricted addressing features. Env is + * process-stable, so construction time is fine. Redaction state rides along + * (issue #237) so agents aren't surprised by placeholders. + */ + public readonly instructions: string; - // Debuggee-output resource subscriptions (issue #218). + // Debuggee-output resource notifications (issue #218), fanned out to every + // facade's sink by the one hub (issue #1025, step 2). /** @internal ToolContext dependency; read live by the tool handlers. */ - public readonly outputResources: OutputResourceNotifier; + public readonly notifications: NotificationHub; /** @internal ToolContext dependency; read live by the tool handlers. */ public readonly validationCache = new ValidationResultCache(); @@ -731,53 +750,21 @@ export class DebugMcpServer implements ToolContext { this.logger ); - // The release version, not a literal: clients and the MCP Registry show this - // string, and the Docker bundle reads it baked in (issue #1054). - this.server = new Server( - { name: 'debug-mcp-server', version: getVersion() }, - { - capabilities: { tools: {}, resources: { subscribe: true, listChanged: true }, prompts: {} }, - // Mode-gated (issue #271): the handshake must not teach restricted - // addressing features. Env is process-stable, so constructor-time is fine. - // Redaction state rides along (issue #237) so agents aren't surprised - // by placeholders. - instructions: buildServerInstructions(getBpAddressingMode(this.environment), { - redactionEnabled: isRedactionEnabled(this.environment), - variableAccessMode: getVariableAccessMode(this.environment) - }) - } - ); - // The protocol version each client negotiates, so the move to the - // 2026-07-28 revision can be timed from real traffic (issue #1025). - this.server.oninitialized = () => { - const client = this.server.getClientVersion(); - this.logger.info('[MCP Server] Client initialized', { - protocolVersion: this.server.getNegotiatedProtocolVersion(), - client: client ? `${client.name} ${client.version}` : undefined - }); - }; + this.fileSystem = dependencies.fileSystem; + this.version = getVersion(); + this.instructions = buildServerInstructions(getBpAddressingMode(this.environment), { + redactionEnabled: isRedactionEnabled(this.environment), + variableAccessMode: getVariableAccessMode(this.environment) + }); const sessionManagerConfig: SessionManagerConfig = { logDirBase: containerConfig.sessionLogDirBase }; - + this.sessionManager = new SessionManager(sessionManagerConfig, dependencies); - this.outputResources = new OutputResourceNotifier(this.server, this.logger); - - registerToolHandlers(this.server, this); - // Tool handlers read every dependency live through `ctx` (=== this) at call - // time; the resource and prompt layers capture `sessionManager` and - // `environment` here instead. Both fields are readonly, so production never - // observes the difference — but a test that swaps `sessionManager` after - // construction sees the swap in tool handlers only, not in resource handlers. - registerResourceHandlers( - this.server, - this.sessionManager, - this.outputResources, - dependencies.fileSystem - ); - registerPromptHandlers(this.server, this.environment); - this.sessionManager.on('output-captured', this.outputResources.handleOutputCaptured); + this.notifications = new NotificationHub(this.logger); + + this.sessionManager.on('output-captured', this.handleOutputCaptured); // A child session adopted by the server (the session tree, issue #1081) // is a new output resource: subscribed clients learn of it as they do of // a session created or closed through the tools. @@ -785,19 +772,20 @@ export class DebugMcpServer implements ToolContext { // Removals also happen during relaunch and terminating detach, outside // close_debug_session. Clean up each resource at the removal itself. this.sessionManager.on('session-removed', this.handleSessionRemoved); - this.server.onerror = (error) => { - this.logger.error('Server error', { error }); - }; } - /** One listener instance, so stop() can remove exactly what start wired (issue #1081). */ + /** One listener instance each, so stop() can remove exactly what the constructor wired (issue #1081). */ + private readonly handleOutputCaptured = (sessionId: string): void => { + this.notifications.resourceUpdated(outputResourceUri(sessionId)); + }; + private readonly handleChildSessionAdopted = (): void => { - this.outputResources.notifyListChanged(); + this.notifications.resourcesChanged(); }; private readonly handleSessionRemoved = (sessionId: string): void => { - this.outputResources.forgetSession(sessionId); - this.outputResources.notifyListChanged(); + this.notifications.forgetUri(outputResourceUri(sessionId)); + this.notifications.resourcesChanged(); }; /** @@ -828,16 +816,16 @@ export class DebugMcpServer implements ToolContext { public async start(): Promise { // For MCP servers, start is handled by transport const buildTime = new Date().toISOString(); - this.logger.info(`[MCP Server] Started at ${buildTime}, version: ${getVersion()}`); + this.logger.info(`[MCP Server] Started at ${buildTime}, version: ${this.version}`); } public async stop(): Promise { await this.sessionManager.closeAllSessions(); - // Tear down output-resource bookkeeping (issue #218): pending debounce - // timers and the SessionManager listener must not outlive the server - // (the test suite runs with a strict leak guard). - this.outputResources.dispose(); - this.sessionManager.removeListener('output-captured', this.outputResources.handleOutputCaptured); + // Tear down notification bookkeeping (issue #218): pending debounce + // timers, the sinks and the SessionManager listeners must not outlive the + // core (the test suite runs with a strict leak guard). + this.notifications.dispose(); + this.sessionManager.removeListener('output-captured', this.handleOutputCaptured); this.sessionManager.removeListener('child-session-adopted', this.handleChildSessionAdopted); this.sessionManager.removeListener('session-removed', this.handleSessionRemoved); this.logger.info('Debug MCP Server stopped'); diff --git a/src/server/handlers/session-tools.ts b/src/server/handlers/session-tools.ts index c39dcd27..eabf25e6 100644 --- a/src/server/handlers/session-tools.ts +++ b/src/server/handlers/session-tools.ts @@ -91,8 +91,8 @@ export const createDebugSessionTool: ToolHandler = async (ctx, args) => { timestamp: Date.now() }); - // A new output resource is now listable (issue #218) - ctx.outputResources.notifyListChanged(); + // A new output resource is now listable (issue #218), for every connected client + ctx.notifications.resourcesChanged(); if (attachRequest) { // Attach mode: immediately attach to the running process diff --git a/src/server/notification-hub.ts b/src/server/notification-hub.ts new file mode 100644 index 00000000..7bd30e9e --- /dev/null +++ b/src/server/notification-hub.ts @@ -0,0 +1,144 @@ +/** + * Process-wide fan-out for the MCP notifications a debug session produces + * (issue #1025, step 2). + * + * The 2026-07-28 protocol serves one SDK `Server` per request, so nothing + * that outlives a request can hang off a Server: the per-URI coalescer for + * resources/updated pings and the list of parties to tell live here, on the + * one shared core, and every protocol facade (or an HTTP handler's notifier) + * attaches a sink for as long as it is open. Debounced so notification volume + * is independent of debuggee output volume (issue #218). + */ +import type { Server } from '@modelcontextprotocol/server'; +import type { ILogger } from '@debugmcp/shared'; + +/** Debounce window for resources/updated pings. */ +export const OUTPUT_UPDATE_DEBOUNCE_MS = 150; + +/** One party that receives the hub's notifications. */ +export interface NotificationSink { + resourceUpdated(uri: string): void | Promise; + resourcesChanged(): void | Promise; + /** The resource is gone (its session was removed): drop any per-sink state for it. */ + forget?(uri: string): void; +} + +/** The subscription set a legacy facade filters its pings through. */ +export interface ResourceSubscriptionFilter { + has(uri: string): boolean; + forget(uri: string): void; +} + +export class NotificationHub { + private readonly sinks = new Set(); + private readonly outputUpdateTimers = new Map(); + + constructor(private readonly logger: ILogger) {} + + /** How many sinks are attached (diagnostics and tests). */ + get sinkCount(): number { + return this.sinks.size; + } + + /** Attach a sink; the returned function detaches it. */ + addSink(sink: NotificationSink): () => void { + this.sinks.add(sink); + return () => { + this.sinks.delete(sink); + }; + } + + /** + * Throttled resources/updated ping: the first event after a quiet period + * arms a timer for that URI; everything that arrives inside the window + * rides the same ping. Each sink decides for itself whether to send it. + */ + resourceUpdated(uri: string): void { + if (this.outputUpdateTimers.has(uri)) { + return; + } + const timer = setTimeout(() => { + this.outputUpdateTimers.delete(uri); + for (const sink of this.sinks) { + this.deliver(() => sink.resourceUpdated(uri), `[Server] Failed to send resources/updated for ${uri}`); + } + }, OUTPUT_UPDATE_DEBOUNCE_MS); + timer.unref?.(); + this.outputUpdateTimers.set(uri, timer); + } + + /** Fire-and-forget resources/list_changed to every sink (sessions appeared/disappeared). */ + resourcesChanged(): void { + for (const sink of this.sinks) { + this.deliver(() => sink.resourcesChanged(), '[Server] Failed to send resources/list_changed'); + } + } + + /** The resource is gone: cancel its pending ping and let every sink drop it. */ + forgetUri(uri: string): void { + this.clearTimer(uri); + for (const sink of this.sinks) { + // Runs inside the SessionManager's 'session-removed' emit (relaunch, + // terminating detach): a sink's failure must not escape into those paths. + this.deliver(() => sink.forget?.(uri), `[Server] Failed to forget ${uri}`); + } + } + + /** + * Tear down: pending timers must not outlive the core (the test suite runs + * with a strict leak guard), and no sink hears anything afterwards. + */ + dispose(): void { + for (const uri of [...this.outputUpdateTimers.keys()]) { + this.clearTimer(uri); + } + this.sinks.clear(); + } + + /** A sink that cannot send (not connected yet, transport gone) is not an error. */ + private deliver(send: () => void | Promise, what: string): void { + try { + const result = send(); + if (result && typeof (result as Promise).then === 'function') { + (result as Promise).catch((error: unknown) => { + this.logger.debug(what, { error }); + }); + } + } catch (error) { + this.logger.debug(what, { error }); + } + } + + private clearTimer(uri: string): void { + const timer = this.outputUpdateTimers.get(uri); + if (timer) { + clearTimeout(timer); + this.outputUpdateTimers.delete(uri); + } + } +} + +/** + * The sink a protocol facade attaches for its own SDK Server. With a filter + * (a legacy-era facade) only the URIs that facade subscribed to through + * resources/subscribe are pinged; without one (a modern-era pin, where the SDK + * routes `sendResourceUpdated` onto the open subscriptions/listen stream and + * filters by the listen set itself) every URI is. + */ +export function facadeSink( + server: Pick, + filter?: ResourceSubscriptionFilter +): NotificationSink { + return { + resourceUpdated: (uri: string) => { + if (filter && !filter.has(uri)) { + return; + } + return server.sendResourceUpdated({ uri }); + }, + resourcesChanged: () => server.sendResourceListChanged(), + forget: (uri: string) => { + filter?.forget(uri); + } + }; +} diff --git a/src/server/output-resources.ts b/src/server/output-resources.ts index 6e23f1fd..ef7c84e8 100644 --- a/src/server/output-resources.ts +++ b/src/server/output-resources.ts @@ -5,10 +5,12 @@ * debug://sessions/{id}/output — a verbatim console transcript (all categories * interleaved, in arrival order). Clients may subscribe to receive coalesced * resources/updated pings as output arrives; structured/cursor access is - * available via the get_output tool. + * available via the get_output tool. The pings themselves are coalesced and + * fanned out by the core's NotificationHub (issue #1025, step 2); this module + * holds the handlers and the per-facade subscription set. */ import { Server, ProtocolErrorCode } from '@modelcontextprotocol/server'; -import { ILogger, type IFileSystem } from '@debugmcp/shared'; +import type { IFileSystem } from '@debugmcp/shared'; import { WireMcpError } from '../errors/debug-errors.js'; import type { SessionManager } from '../session/session-manager.js'; import { proxyLogPathFor } from '../proxy/session-log-layout.js'; @@ -28,91 +30,36 @@ export { proxyLogResourceUri } from '../session/session-resource-uris.js'; -/** Debounce window for resources/updated pings. */ -export const OUTPUT_UPDATE_DEBOUNCE_MS = 150; +/** Which output URIs one protocol facade subscribed to through resources/subscribe. */ +export interface ResourceSubscriptions { + subscribe(uri: string): void; + unsubscribe(uri: string): void; +} /** - * Subscription bookkeeping and debounced resources/updated pings for the - * output resources. URIs subscribed via resources/subscribe; updated-pings are - * debounced so notification volume is independent of debuggee output volume. + * The per-facade subscription set (issue #1025, step 2). A subscription + * belongs to the connection that made it; the coalesced resources/updated + * pings themselves come from the core's NotificationHub, which asks this set + * whether the facade wants a given URI. */ -export class OutputResourceNotifier { - private readonly subscribedUris = new Set(); - private readonly outputUpdateTimers = new Map(); - - constructor( - private readonly server: Server, - private readonly logger: ILogger - ) {} - - /** - * Stable listener for SessionManager's 'output-captured' event: the owner - * registers this exact reference with on() and removes it with - * removeListener() on stop(). - */ - readonly handleOutputCaptured = (sessionId: string): void => { - this.scheduleUpdated(sessionId); - }; +export class FacadeSubscriptions implements ResourceSubscriptions { + private readonly uris = new Set(); subscribe(uri: string): void { - this.subscribedUris.add(uri); + this.uris.add(uri); } unsubscribe(uri: string): void { - this.subscribedUris.delete(uri); - this.clearTimer(uri); - } - - /** The session's output resource is gone: drop its subscription and timer. */ - forgetSession(sessionId: string): void { - this.unsubscribe(outputResourceUri(sessionId)); - } - - /** - * Throttled resources/updated ping for a session's output resource: the - * first captured event after a quiet period arms a timer; everything that - * arrives inside the window rides the same ping. - */ - scheduleUpdated(sessionId: string): void { - const uri = outputResourceUri(sessionId); - if (!this.subscribedUris.has(uri) || this.outputUpdateTimers.has(uri)) { - return; - } - const timer = setTimeout(() => { - this.outputUpdateTimers.delete(uri); - this.server.sendResourceUpdated({ uri }).catch((error: unknown) => { - // Not connected yet / transport gone — nothing to notify, not an error. - this.logger.debug(`[Server] Failed to send resources/updated for ${uri}`, { error }); - }); - }, OUTPUT_UPDATE_DEBOUNCE_MS); - timer.unref?.(); - this.outputUpdateTimers.set(uri, timer); - } - - /** Fire-and-forget resources/list_changed (sessions appeared/disappeared). */ - notifyListChanged(): void { - this.server.sendResourceListChanged().catch((error: unknown) => { - this.logger.debug('[Server] Failed to send resources/list_changed', { error }); - }); + this.uris.delete(uri); } - /** - * Tear down output-resource bookkeeping: pending debounce timers must not - * outlive the server (the test suite runs with a strict leak guard). - */ - dispose(): void { - for (const uri of this.outputUpdateTimers.keys()) { - this.clearTimer(uri); - } - this.subscribedUris.clear(); + has(uri: string): boolean { + return this.uris.has(uri); } - private clearTimer(uri: string): void { - const timer = this.outputUpdateTimers.get(uri); - if (timer) { - clearTimeout(timer); - this.outputUpdateTimers.delete(uri); - } + /** The resource is gone (its session was removed): drop the subscription. */ + forget(uri: string): void { + this.uris.delete(uri); } } @@ -123,7 +70,7 @@ export class OutputResourceNotifier { export function registerResourceHandlers( server: Server, sessionManager: SessionManager, - notifier: OutputResourceNotifier, + subscriptions: ResourceSubscriptions, fileSystem: Pick ): void { server.setRequestHandler('resources/list', async () => { @@ -192,13 +139,13 @@ export function registerResourceHandlers( if (!sessionId || !sessionManager.getSession(sessionId)) { throw new WireMcpError(ProtocolErrorCode.InvalidParams, `Unknown resource: ${uri}`); } - notifier.subscribe(uri); + subscriptions.subscribe(uri); return {}; }); server.setRequestHandler('resources/unsubscribe', async (request) => { const uri = request.params.uri; - notifier.unsubscribe(uri); + subscriptions.unsubscribe(uri); return {}; }); } diff --git a/src/server/protocol-facade.ts b/src/server/protocol-facade.ts new file mode 100644 index 00000000..a3195d1b --- /dev/null +++ b/src/server/protocol-facade.ts @@ -0,0 +1,81 @@ +/** + * The protocol facade (issue #1025, step 2): one SDK `Server` per connection + * or per request over the one shared DebugMcpServer core. + * + * The 2026-07-28 serving entries build a Server per HTTP request + * (`createMcpHandler`) or per stdio connection plus a discarded probe instance + * (`serveStdio`), so a facade has to be cheap: the eight MCP handlers closing + * over the core, the error and initialized hooks, and one notification sink + * 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 type { DebugMcpServer } from '../server.js'; +import { facadeSink } from './notification-hub.js'; +import { FacadeSubscriptions, registerResourceHandlers } from './output-resources.js'; +import { registerPromptHandlers } from './prompts.js'; +import { registerToolHandlers } from './tool-dispatch.js'; + +/** The wire era a facade serves: the 2025 handshake family, or 2026-07-28. */ +export type ProtocolEra = 'legacy' | 'modern'; + +export interface ProtocolFacadeOptions { + era: ProtocolEra; + /** + * How this facade's Server delivers notifications. `instance`: the Server + * itself is long-lived (a stdio pin, a legacy HTTP session) and sends them, + * so the facade attaches a sink to the core's hub. `external`: the Server + * serves one HTTP request and cannot send anything afterwards; the HTTP + * layer's handler notifier is the sink instead, so the facade attaches none. + */ + deliver: 'instance' | 'external'; +} + +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. + const server = new Server( + { name: 'debug-mcp-server', version: core.version }, + { + capabilities: { tools: {}, resources: { subscribe: true, listChanged: true }, prompts: {} }, + instructions: core.instructions + } + ); + + // 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); + + // The protocol version each legacy client negotiates (issue #1025). The + // modern era has no initialize; its clients are identified per request. + server.oninitialized = () => { + const client = server.getClientVersion(); + core.logger.info('[MCP Server] Client initialized', { + protocolVersion: server.getNegotiatedProtocolVersion(), + client: client ? `${client.name} ${client.version}` : undefined + }); + }; + server.onerror = (error) => { + core.logger.error('Server error', { error }); + }; + + if (options.deliver === 'instance') { + // A legacy facade pings what it subscribed to; a modern pin pings every + // URI and the SDK's listen router filters. Either way the sink leaves + // with the facade: the serving entries chain onclose rather than + // replacing it, so this hook survives them. + const detach = core.notifications.addSink( + facadeSink(server, options.era === 'legacy' ? subscriptions : undefined) + ); + const previous = server.onclose; + server.onclose = () => { + detach(); + previous?.(); + }; + } + + return server; +} diff --git a/src/server/tool-context.ts b/src/server/tool-context.ts index 4e0084af..9d5e89ed 100644 --- a/src/server/tool-context.ts +++ b/src/server/tool-context.ts @@ -31,7 +31,7 @@ import type { SimpleFileChecker, FileExistenceResult } from '../utils/simple-fil import type { LineReader } from '../utils/line-reader.js'; import type { BuildStalenessChecker } from '../session/breakpoints/build-staleness.js'; import type { ValidationResultCache } from '../utils/language-availability.js'; -import type { OutputResourceNotifier } from './output-resources.js'; +import type { NotificationHub } from './notification-hub.js'; import type { LanguageMetadata } from './language-discovery.js'; import type { ToolArguments } from './tool-arguments.js'; import type { ToolResult } from './tool-result.js'; @@ -65,7 +65,8 @@ export interface ToolContext { readonly buildStaleness: BuildStalenessChecker; readonly environment: IEnvironment; readonly validationCache: ValidationResultCache; - readonly outputResources: OutputResourceNotifier; + /** The process-wide notification hub: resource list changes and coalesced output pings (issue #1025, step 2). */ + readonly notifications: NotificationHub; // ---- context services ---- getAdapterRegistry(): IAdapterRegistry; diff --git a/src/server/tool-dispatch.ts b/src/server/tool-dispatch.ts index 2585da80..591207db 100644 --- a/src/server/tool-dispatch.ts +++ b/src/server/tool-dispatch.ts @@ -41,14 +41,29 @@ function foreignSessionResult(ctx: ToolContext, definition: ToolDefinition, sess return explanation ? failureResult(explanation) : undefined; } +/** + * Required fields do not depend on language discovery, so the dispatch index + * is immutable per core and built once per core, not per facade: the modern + * HTTP leg constructs a facade per request (issue #1025, step 2). tools/list + * still rebuilds descriptions with the dynamically discovered language enum + * for each request. + */ +const DISPATCH_INDEX = new WeakMap>(); + +function dispatchIndexFor(ctx: ToolContext): ReadonlyMap { + let index = DISPATCH_INDEX.get(ctx); + if (!index) { + index = new Map( + buildToolDefinitions({ supportedLanguages: [], environment: ctx.environment }) + .map((definition) => [definition.name, definition] as const) + ); + DISPATCH_INDEX.set(ctx, index); + } + return index; +} + export function registerToolHandlers(server: Server, ctx: ToolContext): void { - // Required fields do not depend on language discovery. Build this immutable - // dispatch index once; tools/list can still rebuild descriptions with the - // dynamically discovered language enum for each request. - const dispatchDefinitions = new Map( - buildToolDefinitions({ supportedLanguages: [], environment: ctx.environment }) - .map((definition) => [definition.name, definition] as const) - ); + const dispatchDefinitions = dispatchIndexFor(ctx); server.setRequestHandler('tools/list', async () => { ctx.logger.debug('Handling ListToolsRequest'); diff --git a/tests/core/unit/server/dynamic-tool-documentation.test.ts b/tests/core/unit/server/dynamic-tool-documentation.test.ts index 938a6354..ca48e649 100644 --- a/tests/core/unit/server/dynamic-tool-documentation.test.ts +++ b/tests/core/unit/server/dynamic-tool-documentation.test.ts @@ -68,29 +68,19 @@ async function getToolsFromServer(server: DebugMcpServer): Promise> { - // Tool registration lives in src/server/tool-dispatch.ts; re-run it against - // the live server to capture the tools/list handler it registers + // Tool registration lives in src/server/tool-dispatch.ts. The core owns no + // SDK Server (issue #1025, step 2), so register against a bare recorder and + // capture the tools/list handler it registers. let capturedHandler: unknown = null; - - // Spy on setRequestHandler. The SDK keys handlers by method string; the - // cast narrows its overloads to the two-argument form this spy forwards. - type Registration = [method: string, handler: unknown]; - const originalSetRequestHandler = server.server.setRequestHandler.bind(server.server) as unknown as - (...registration: Registration) => void; - server.server.setRequestHandler = vi.fn().mockImplementation( - (...registration: Registration) => { - const [schema, handler] = registration; - // Capture the tools/list registration - if (schema === 'tools/list') { + const recorder = { + setRequestHandler: vi.fn((method: string, handler: unknown) => { + if (method === 'tools/list') { capturedHandler = handler; } - return originalSetRequestHandler(...registration); - } - ); - - // Re-register tools to capture them - registerToolHandlers(server.server, server); - + }) + }; + registerToolHandlers(recorder as unknown as Parameters[0], server); + // Check if we captured the handler if (!capturedHandler) { throw new Error('tools/list handler not found'); @@ -100,9 +90,6 @@ async function getToolsFromServer(server: DebugMcpServer): Promise Promise; const result = await listToolsHandler({ jsonrpc: '2.0', method: 'tools/list' }) as { tools: Array }; - // Restore original - server.server.setRequestHandler = originalSetRequestHandler; - return result.tools as Array<{ name: string; description: string; diff --git a/tests/core/unit/server/mcp-wire-errors.test.ts b/tests/core/unit/server/mcp-wire-errors.test.ts index e9b8db43..ca219dfa 100644 --- a/tests/core/unit/server/mcp-wire-errors.test.ts +++ b/tests/core/unit/server/mcp-wire-errors.test.ts @@ -17,7 +17,7 @@ import { registerPromptHandlers } from '../../../../src/server/prompts.js'; import { registerResourceHandlers } from '../../../../src/server/output-resources.js'; import type { ToolContext } from '../../../../src/server/tool-context.js'; import type { SessionManager } from '../../../../src/session/session-manager.js'; -import type { OutputResourceNotifier } from '../../../../src/server/output-resources.js'; +import type { ResourceSubscriptions } from '../../../../src/server/output-resources.js'; import { SessionNotFoundError } from '../../../../src/errors/debug-errors.js'; const logger = { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }; @@ -121,7 +121,7 @@ describe('protocol errors reach SDK clients as the bare reason, with the code be getAllSessions: () => [], getSession: () => undefined } as unknown as SessionManager; - const notifier = { subscribe: vi.fn(), unsubscribe: vi.fn() } as unknown as OutputResourceNotifier; + const notifier: ResourceSubscriptions = { subscribe: vi.fn(), unsubscribe: vi.fn() }; registerResourceHandlers(server, sessionManager, notifier, { readTail: vi.fn() }); client = await connect(server); diff --git a/tests/core/unit/server/notification-hub.test.ts b/tests/core/unit/server/notification-hub.test.ts new file mode 100644 index 00000000..62e36772 --- /dev/null +++ b/tests/core/unit/server/notification-hub.test.ts @@ -0,0 +1,233 @@ +/** + * The notification hub (issue #1025, step 2): one process-wide coalescer for + * resources/updated pings and one fan-out point for resources/list_changed, + * feeding however many protocol facades (or an HTTP handler's notifier) are + * alive. Replaces the per-server OutputResourceNotifier, whose subscription + * set and SDK Server were one object — a shape a per-request facade cannot + * hold. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + NotificationHub, + OUTPUT_UPDATE_DEBOUNCE_MS, + facadeSink, + type NotificationSink +} from '../../../../src/server/notification-hub.js'; +import { FacadeSubscriptions } from '../../../../src/server/output-resources.js'; + +const logger = { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }; +const URI = 'debug://sessions/sess-1/output'; +const OTHER = 'debug://sessions/sess-2/output'; + +function fakeSink() { + return { + resourceUpdated: vi.fn<(uri: string) => void>(), + resourcesChanged: vi.fn<() => void>(), + forget: vi.fn<(uri: string) => void>() + } satisfies NotificationSink; +} + +describe('NotificationHub', () => { + let hub: NotificationHub; + + beforeEach(() => { + vi.useFakeTimers(); + hub = new NotificationHub(logger); + }); + + afterEach(() => { + hub.dispose(); + vi.clearAllTimers(); + vi.useRealTimers(); + vi.clearAllMocks(); + }); + + it('coalesces a burst of updates for one URI into a single delivery per sink', async () => { + const sink = fakeSink(); + hub.addSink(sink); + + for (let i = 0; i < 50; i++) { + hub.resourceUpdated(URI); + } + expect(sink.resourceUpdated).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_DEBOUNCE_MS + 10); + + expect(sink.resourceUpdated).toHaveBeenCalledTimes(1); + expect(sink.resourceUpdated).toHaveBeenCalledWith(URI); + + // A later burst gets its own delivery + hub.resourceUpdated(URI); + await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_DEBOUNCE_MS + 10); + expect(sink.resourceUpdated).toHaveBeenCalledTimes(2); + }); + + it('keeps one timer per URI, so two sessions ping independently', async () => { + const sink = fakeSink(); + hub.addSink(sink); + + hub.resourceUpdated(URI); + hub.resourceUpdated(OTHER); + await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_DEBOUNCE_MS + 10); + + expect(sink.resourceUpdated.mock.calls.map(([uri]) => uri).sort()).toEqual([URI, OTHER].sort()); + }); + + it('fans a delivery out to every sink and reports how many are attached', async () => { + const a = fakeSink(); + const b = fakeSink(); + hub.addSink(a); + hub.addSink(b); + expect(hub.sinkCount).toBe(2); + + hub.resourceUpdated(URI); + await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_DEBOUNCE_MS + 10); + hub.resourcesChanged(); + + expect(a.resourceUpdated).toHaveBeenCalledWith(URI); + expect(b.resourceUpdated).toHaveBeenCalledWith(URI); + expect(a.resourcesChanged).toHaveBeenCalledTimes(1); + expect(b.resourcesChanged).toHaveBeenCalledTimes(1); + }); + + it('stops delivering to a sink once its remover has run', async () => { + const sink = fakeSink(); + const remove = hub.addSink(sink); + remove(); + expect(hub.sinkCount).toBe(0); + + hub.resourceUpdated(URI); + await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_DEBOUNCE_MS + 10); + hub.resourcesChanged(); + + expect(sink.resourceUpdated).not.toHaveBeenCalled(); + expect(sink.resourcesChanged).not.toHaveBeenCalled(); + }); + + it('forgetUri cancels the pending ping and tells every sink to drop the URI', async () => { + const sink = fakeSink(); + hub.addSink(sink); + hub.resourceUpdated(URI); + + hub.forgetUri(URI); + await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_DEBOUNCE_MS + 10); + + expect(sink.resourceUpdated).not.toHaveBeenCalled(); + expect(sink.forget).toHaveBeenCalledWith(URI); + }); + + it('dispose cancels every pending ping and detaches every sink', async () => { + const sink = fakeSink(); + hub.addSink(sink); + hub.resourceUpdated(URI); + hub.resourceUpdated(OTHER); + + hub.dispose(); + await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_DEBOUNCE_MS + 10); + + expect(sink.resourceUpdated).not.toHaveBeenCalled(); + expect(hub.sinkCount).toBe(0); + }); + + it('a sink that throws or rejects is logged at debug and never blocks its siblings', async () => { + const broken: NotificationSink = { + resourceUpdated: vi.fn(async () => { throw new Error('transport gone'); }), + resourcesChanged: vi.fn(() => { throw new Error('closed'); }) + }; + const healthy = fakeSink(); + hub.addSink(broken); + hub.addSink(healthy); + + hub.resourceUpdated(URI); + await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_DEBOUNCE_MS + 10); + expect(() => hub.resourcesChanged()).not.toThrow(); + await vi.advanceTimersByTimeAsync(0); + + expect(healthy.resourceUpdated).toHaveBeenCalledWith(URI); + expect(healthy.resourcesChanged).toHaveBeenCalledTimes(1); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringContaining('Failed to send resources/updated'), + expect.objectContaining({ error: expect.any(Error) }) + ); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringContaining('Failed to send resources/list_changed'), + expect.objectContaining({ error: expect.any(Error) }) + ); + expect(logger.error).not.toHaveBeenCalled(); + }); +}); + +describe('NotificationHub forget guard', () => { + it('a sink whose forget throws is logged at debug and does not block its siblings', () => { + const hub = new NotificationHub(logger); + const broken: NotificationSink = { + resourceUpdated: vi.fn(), + resourcesChanged: vi.fn(), + forget: vi.fn(() => { throw new Error('gone'); }) + }; + const healthy = fakeSink(); + hub.addSink(broken); + hub.addSink(healthy); + + expect(() => hub.forgetUri(URI)).not.toThrow(); + + expect(healthy.forget).toHaveBeenCalledWith(URI); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringContaining('Failed to forget'), + expect.objectContaining({ error: expect.any(Error) }) + ); + hub.dispose(); + }); +}); + +describe('facadeSink', () => { + const server = () => ({ + sendResourceUpdated: vi.fn().mockResolvedValue(undefined), + sendResourceListChanged: vi.fn().mockResolvedValue(undefined) + }); + + it('with a subscription filter, pings only the URIs that facade subscribed to', async () => { + const s = server(); + const subscriptions = new FacadeSubscriptions(); + subscriptions.subscribe(URI); + const sink = facadeSink(s, subscriptions); + + await sink.resourceUpdated(URI); + await sink.resourceUpdated(OTHER); + + expect(s.sendResourceUpdated).toHaveBeenCalledTimes(1); + expect(s.sendResourceUpdated).toHaveBeenCalledWith({ uri: URI }); + }); + + it('without a filter (a modern-era pin, where the SDK filters), pings every URI', async () => { + const s = server(); + const sink = facadeSink(s); + + await sink.resourceUpdated(URI); + await sink.resourceUpdated(OTHER); + + expect(s.sendResourceUpdated).toHaveBeenCalledTimes(2); + }); + + it('list changes are sent regardless of subscriptions, and forget drops the URI from the filter', async () => { + const s = server(); + const subscriptions = new FacadeSubscriptions(); + subscriptions.subscribe(URI); + const sink = facadeSink(s, subscriptions); + + await sink.resourcesChanged(); + sink.forget?.(URI); + await sink.resourceUpdated(URI); + + expect(s.sendResourceListChanged).toHaveBeenCalledTimes(1); + expect(s.sendResourceUpdated).not.toHaveBeenCalled(); + expect(subscriptions.has(URI)).toBe(false); + }); + + it('returns the send promise so the hub can observe a rejection', async () => { + const s = server(); + s.sendResourceUpdated.mockRejectedValue(new Error('not connected')); + const sink = facadeSink(s); + + await expect(sink.resourceUpdated(URI)).rejects.toThrow('not connected'); + }); +}); diff --git a/tests/core/unit/server/protocol-facade-transport.test.ts b/tests/core/unit/server/protocol-facade-transport.test.ts new file mode 100644 index 00000000..cddbef4d --- /dev/null +++ b/tests/core/unit/server/protocol-facade-transport.test.ts @@ -0,0 +1,73 @@ +/** + * The facade's sink leaves with the facade through the REAL SDK close chain + * (issue #1025, step 2): a transport close runs `Protocol._onclose` → + * `server.onclose` → the hub detach the facade installed. The sibling suite + * mocks the SDK Server and can only invoke `onclose` by hand; this one drives + * a real Server/Client pair over the in-memory transport, because from PR 2 on + * (one core per process) this chain is the only thing that lets a closed + * connection's Server be collected. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { InMemoryTransport } from '@modelcontextprotocol/server'; +import { Client } from '@modelcontextprotocol/client'; +import { DebugMcpServer } from '../../../../src/server.js'; +import { createProtocolServer } from '../../../../src/server/protocol-facade.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 over a real transport', () => { + let core: DebugMcpServer; + let mockSessionManager: MockSessionManager; + let mockDependencies: Dependencies; + + beforeEach(() => { + mockDependencies = createMockDependencies(); + vi.mocked(createProductionDependencies).mockReturnValue(mockDependencies); + mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); + mockSessionManager.getSession.mockReturnValue({ id: 'sess-1', name: 'one', language: 'mock' } as any); + vi.mocked(SessionManager).mockImplementation(function () { return mockSessionManager as any; }); + core = new DebugMcpServer({ logLevel: 'debug' }); + }); + + afterEach(async () => { + mockSessionManager.closeAllSessions.mockResolvedValue(undefined); + await core.stop(); + vi.clearAllMocks(); + }); + + it('detaches its sink when the transport closes, with a live subscription in place', async () => { + const facade = createProtocolServer(core, { era: 'legacy', deliver: 'instance' }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: 'facade-close-test', version: '0.0.0' }); + await Promise.all([facade.connect(serverTransport), client.connect(clientTransport)]); + await client.subscribeResource({ uri: URI }); + expect(core.notifications.sinkCount).toBe(1); + + await client.close(); + + expect(core.notifications.sinkCount).toBe(0); + }); + + it('detaches its sink when the server side closes', async () => { + const facade = createProtocolServer(core, { era: 'legacy', deliver: 'instance' }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: 'facade-close-test', version: '0.0.0' }); + await Promise.all([facade.connect(serverTransport), client.connect(clientTransport)]); + expect(core.notifications.sinkCount).toBe(1); + + await facade.close(); + + expect(core.notifications.sinkCount).toBe(0); + await client.close().catch(() => undefined); + }); +}); diff --git a/tests/core/unit/server/protocol-facade.test.ts b/tests/core/unit/server/protocol-facade.test.ts new file mode 100644 index 00000000..c610924f --- /dev/null +++ b/tests/core/unit/server/protocol-facade.test.ts @@ -0,0 +1,235 @@ +/** + * The protocol facade (issue #1025, step 2): one cheap SDK `Server` per + * connection or request over the one shared DebugMcpServer core. It carries + * the eight MCP handlers, the error and initialized hooks, and one + * notification sink that lives exactly as long as the facade does. The core + * never constructs a Server itself any more. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { Server } from '@modelcontextprotocol/server'; +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 { SessionManager } from '../../../../src/session/session-manager.js'; +import { createProductionDependencies, type Dependencies } from '../../../../src/container/dependencies.js'; +import { getVersion } from '../../../../src/cli/version.js'; +import { + createMockDependencies, + createMockServer, + createMockSessionManager, + getResourceHandlers, + type MockServer, + type MockSessionManager +} from './server-test-helpers.js'; + +vi.mock('@modelcontextprotocol/server', async () => + (await import('../../../test-utils/mocks/sdk-server-module.js')).sdkServerModuleMock()); +vi.mock('../../../../src/session/session-manager.js'); +vi.mock('../../../../src/container/dependencies.js'); +vi.mock('../../../../src/cli/version.js', () => ({ getVersion: vi.fn(() => '1.2.3-test') })); + +const URI = 'debug://sessions/sess-1/output'; + +describe('createProtocolServer', () => { + let core: DebugMcpServer; + let mockServers: MockServer[]; + let mockSessionManager: MockSessionManager; + let mockDependencies: Dependencies; + let outputCapturedListener: ((sessionId: string) => void) | undefined; + + beforeEach(() => { + vi.useFakeTimers(); + mockDependencies = createMockDependencies(); + vi.mocked(createProductionDependencies).mockReturnValue(mockDependencies); + + mockServers = []; + vi.mocked(Server).mockImplementation(function () { + const s = createMockServer(); + mockServers.push(s); + return s as any; + }); + + mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); + 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 () => { + mockSessionManager.closeAllSessions.mockResolvedValue(undefined); + await core.stop(); + vi.clearAllTimers(); + vi.useRealTimers(); + vi.clearAllMocks(); + outputCapturedListener = undefined; + }); + + const legacy = () => createProtocolServer(core, { era: 'legacy', deliver: 'instance' }); + + it('the core constructs no SDK Server of its own', () => { + expect(Server).not.toHaveBeenCalled(); + expect(mockServers).toHaveLength(0); + }); + + it('builds the Server with the release identity, the capabilities and the mode-gated instructions', () => { + legacy(); + + expect(Server).toHaveBeenCalledTimes(1); + expect(Server).toHaveBeenCalledWith( + { name: 'debug-mcp-server', version: '1.2.3-test' }, + { + capabilities: { tools: {}, resources: { subscribe: true, listChanged: true }, prompts: {} }, + instructions: expect.stringContaining('mcp-debugger drives real step-through debuggers') + } + ); + expect(vi.mocked(Server).mock.calls[0][1]?.instructions).toBe(core.instructions); + }); + + it('reads the release version once, on the core, not per facade (the version helper walks the file system)', () => { + expect(core.version).toBe('1.2.3-test'); + const callsAfterCore = vi.mocked(getVersion).mock.calls.length; + + legacy(); + legacy(); + + expect(vi.mocked(getVersion).mock.calls.length).toBe(callsAfterCore); + expect(vi.mocked(Server).mock.calls[0][0]).toEqual({ name: 'debug-mcp-server', version: core.version }); + }); + + it('two legacy facades over one core are pinged only for their own subscriptions', async () => { + legacy(); + legacy(); + const [a, b] = mockServers; + mockSessionManager.getSession.mockImplementation((id: string) => ({ id, name: id, language: 'mock' }) as any); + await getResourceHandlers(a).subscribeHandler({ method: 'resources/subscribe', params: { uri: URI } }); + await getResourceHandlers(b).subscribeHandler({ method: 'resources/subscribe', params: { uri: 'debug://sessions/sess-2/output' } }); + + outputCapturedListener!('sess-1'); + outputCapturedListener!('sess-2'); + await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_DEBOUNCE_MS + 10); + + expect(a.sendResourceUpdated).toHaveBeenCalledTimes(1); + expect(a.sendResourceUpdated).toHaveBeenCalledWith({ uri: URI }); + expect(b.sendResourceUpdated).toHaveBeenCalledTimes(1); + expect(b.sendResourceUpdated).toHaveBeenCalledWith({ uri: 'debug://sessions/sess-2/output' }); + }); + + it('registers exactly the eight MCP handlers', () => { + legacy(); + + const [server] = mockServers; + const methods = server.setRequestHandler.mock.calls.map(([method]: unknown[]) => method).sort(); + expect(methods).toEqual([ + 'prompts/get', + 'prompts/list', + 'resources/list', + 'resources/read', + 'resources/subscribe', + 'resources/unsubscribe', + 'tools/call', + 'tools/list' + ]); + }); + + it('routes SDK errors to the core logger', () => { + legacy(); + const [server] = mockServers; + expect(server.onerror).toBeDefined(); + + const testError = new Error('Test error'); + server.onerror!(testError); + + expect(mockDependencies.logger.error).toHaveBeenCalledWith('Server error', { error: testError }); + }); + + it('logs the negotiated protocol version and client on a legacy initialize (issue #1025)', () => { + legacy(); + const [server] = mockServers; + server.getNegotiatedProtocolVersion.mockReturnValue('2025-11-25'); + server.getClientVersion.mockReturnValue({ name: 'claude-code', version: '2.1.300' }); + + expect(server.oninitialized).toBeDefined(); + server.oninitialized!(); + + expect(mockDependencies.logger.info).toHaveBeenCalledWith('[MCP Server] Client initialized', { + protocolVersion: '2025-11-25', + client: 'claude-code 2.1.300' + }); + }); + + it('a legacy instance facade pings only the output URIs it subscribed to, and only while it is open', async () => { + const facade = legacy(); + const [server] = mockServers; + expect(core.notifications.sinkCount).toBe(1); + mockSessionManager.getSession.mockReturnValue({ id: 'sess-1', name: 'one', language: 'mock' } as any); + const { subscribeHandler } = getResourceHandlers(server); + await subscribeHandler({ method: 'resources/subscribe', params: { uri: URI } }); + + outputCapturedListener!('sess-1'); + outputCapturedListener!('sess-2'); + await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_DEBOUNCE_MS + 10); + + expect(server.sendResourceUpdated).toHaveBeenCalledTimes(1); + expect(server.sendResourceUpdated).toHaveBeenCalledWith({ uri: URI }); + + // The facade's own close detaches its sink + expect(facade.onclose).toBeDefined(); + facade.onclose!(); + expect(core.notifications.sinkCount).toBe(0); + outputCapturedListener!('sess-1'); + await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_DEBOUNCE_MS + 10); + expect(server.sendResourceUpdated).toHaveBeenCalledTimes(1); + }); + + it('a modern instance facade (a stdio pin) pings every output URI; the SDK filters by the listen set', async () => { + createProtocolServer(core, { era: 'modern', deliver: 'instance' }); + const [server] = mockServers; + + outputCapturedListener!('sess-1'); + await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_DEBOUNCE_MS + 10); + + expect(server.sendResourceUpdated).toHaveBeenCalledWith({ uri: URI }); + }); + + it('an external-delivery facade (a per-request HTTP instance) registers no sink', async () => { + createProtocolServer(core, { era: 'modern', deliver: 'external' }); + const [server] = mockServers; + expect(core.notifications.sinkCount).toBe(0); + + outputCapturedListener!('sess-1'); + core.notifications.resourcesChanged(); + await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_DEBOUNCE_MS + 10); + + expect(server.sendResourceUpdated).not.toHaveBeenCalled(); + expect(server.sendResourceListChanged).not.toHaveBeenCalled(); + }); + + it('two facades on one core both hear a list change', () => { + legacy(); + legacy(); + const [a, b] = mockServers; + + core.notifications.resourcesChanged(); + + expect(a.sendResourceListChanged).toHaveBeenCalledTimes(1); + expect(b.sendResourceListChanged).toHaveBeenCalledTimes(1); + }); + + it('is cheap: a second facade over a built core takes well under the per-request budget', () => { + legacy(); + vi.useRealTimers(); + const started = performance.now(); + legacy(); + const elapsed = performance.now() - started; + + // Generous bound (the per-request HTTP leg builds one per call; a loaded + // CI runner pauses for GC): the dispatch index and the version string are + // built once per core, not per facade. + expect(elapsed).toBeLessThan(50); + }); +}); diff --git a/tests/core/unit/server/server-bp-addressing-gating.test.ts b/tests/core/unit/server/server-bp-addressing-gating.test.ts index fc9dc847..ad37c19a 100644 --- a/tests/core/unit/server/server-bp-addressing-gating.test.ts +++ b/tests/core/unit/server/server-bp-addressing-gating.test.ts @@ -21,7 +21,8 @@ import { getToolHandlers, type ListToolsHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -54,7 +55,7 @@ describe('DEBUG_MCP_BP_ADDRESSING gating (#271)', () => { }); function buildServer() { - new DebugMcpServer(); + createDebugServerWithFacade(); return getToolHandlers(mockServer); } @@ -138,7 +139,7 @@ describe('DEBUG_MCP_BP_ADDRESSING gating (#271)', () => { it('mentions expectedContent in server instructions only outside line mode', () => { vi.stubEnv('DEBUG_MCP_BP_ADDRESSING', 'line'); - new DebugMcpServer(); + createDebugServerWithFacade(); const lineInstructions = (vi.mocked(Server).mock.calls.at(-1)![1] as { instructions?: string; }).instructions; @@ -146,7 +147,7 @@ describe('DEBUG_MCP_BP_ADDRESSING gating (#271)', () => { expect(lineInstructions).not.toContain('expectedContent'); vi.stubEnv('DEBUG_MCP_BP_ADDRESSING', 'content'); - new DebugMcpServer(); + createDebugServerWithFacade(); const contentInstructions = (vi.mocked(Server).mock.calls.at(-1)![1] as { instructions?: string; }).instructions; diff --git a/tests/core/unit/server/server-breakpoint-management-tools.test.ts b/tests/core/unit/server/server-breakpoint-management-tools.test.ts index f0a93181..8497bb5e 100644 --- a/tests/core/unit/server/server-breakpoint-management-tools.test.ts +++ b/tests/core/unit/server/server-breakpoint-management-tools.test.ts @@ -18,7 +18,8 @@ import { type CallToolHandler, type ListToolsHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; // Mock dependencies @@ -48,7 +49,7 @@ describe('Server Breakpoint Management Tools', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); const handlers = getToolHandlers(mockServer); callToolHandler = handlers.callToolHandler; listToolsHandler = handlers.listToolsHandler; diff --git a/tests/core/unit/server/server-child-session-refusals.test.ts b/tests/core/unit/server/server-child-session-refusals.test.ts index cd8ae279..b320760c 100644 --- a/tests/core/unit/server/server-child-session-refusals.test.ts +++ b/tests/core/unit/server/server-child-session-refusals.test.ts @@ -24,7 +24,8 @@ import { getToolHandlers, type CallToolHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -110,7 +111,7 @@ describe('child session refusals (session tree, issue #1081)', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function () { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; }); diff --git a/tests/core/unit/server/server-control-tools.test.ts b/tests/core/unit/server/server-control-tools.test.ts index c5e128e7..dc3af25c 100644 --- a/tests/core/unit/server/server-control-tools.test.ts +++ b/tests/core/unit/server/server-control-tools.test.ts @@ -17,7 +17,8 @@ import { getToolHandlers, type CallToolHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; // Mock dependencies @@ -46,7 +47,7 @@ describe('Server Control Tools Tests', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; }); diff --git a/tests/core/unit/server/server-doctor-parity.test.ts b/tests/core/unit/server/server-doctor-parity.test.ts index 4fe7cc99..9a0c88a6 100644 --- a/tests/core/unit/server/server-doctor-parity.test.ts +++ b/tests/core/unit/server/server-doctor-parity.test.ts @@ -21,7 +21,8 @@ import { createMockDependencies, createMockServer, createMockSessionManager, - getToolHandlers + getToolHandlers, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -126,7 +127,7 @@ describe('doctor / list_supported_languages availability parity (issue #435)', ( it('reports identical per-language modes from both paths against the same registry', async () => { // Server path - new DebugMcpServer(); + createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); const serverResult = await callToolHandler({ method: 'tools/call', diff --git a/tests/core/unit/server/server-expected-content.test.ts b/tests/core/unit/server/server-expected-content.test.ts index f490b940..531ad9d5 100644 --- a/tests/core/unit/server/server-expected-content.test.ts +++ b/tests/core/unit/server/server-expected-content.test.ts @@ -21,7 +21,8 @@ import { getToolHandlers, type CallToolHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -64,7 +65,7 @@ describe('set_breakpoint expectedContent (#271)', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; mockSessionManager.getSession.mockReturnValue({ diff --git a/tests/core/unit/server/server-expose-tools.test.ts b/tests/core/unit/server/server-expose-tools.test.ts index e874b3ac..a229602e 100644 --- a/tests/core/unit/server/server-expose-tools.test.ts +++ b/tests/core/unit/server/server-expose-tools.test.ts @@ -17,7 +17,8 @@ import { type CallToolHandler, type ListToolsHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -51,7 +52,7 @@ describe('expose_session and unexpose_session tools', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function () { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); const handlers = getToolHandlers(mockServer); callToolHandler = handlers.callToolHandler; listToolsHandler = handlers.listToolsHandler; diff --git a/tests/core/unit/server/server-function-breakpoint-gating.test.ts b/tests/core/unit/server/server-function-breakpoint-gating.test.ts index aa976183..acfa3033 100644 --- a/tests/core/unit/server/server-function-breakpoint-gating.test.ts +++ b/tests/core/unit/server/server-function-breakpoint-gating.test.ts @@ -23,7 +23,8 @@ import { type CallToolHandler, type ListToolsHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -51,7 +52,7 @@ describe('set_breakpoint function gating (#271 phase 3)', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); const handlers = getToolHandlers(mockServer); callToolHandler = handlers.callToolHandler; listToolsHandler = handlers.listToolsHandler; @@ -244,7 +245,7 @@ describe('set_breakpoint function-name normalization (issue #467)', () => { vi.mocked(StdioServerTransport).mockImplementation(function() { return mockStdioTransport as any; }); mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; mockSessionManager.getSession.mockReturnValue({ id: 'test-session', sessionLifecycle: 'active' }); }); diff --git a/tests/core/unit/server/server-initialization.test.ts b/tests/core/unit/server/server-initialization.test.ts index c798cb1b..0487fc17 100644 --- a/tests/core/unit/server/server-initialization.test.ts +++ b/tests/core/unit/server/server-initialization.test.ts @@ -14,6 +14,7 @@ import { createMockServer, createMockSessionManager, createMockStdioTransport, + createDebugServerWithFacade, getToolHandlers, type MockServer, type MockSessionManager @@ -54,17 +55,13 @@ describe('Server Initialization Tests', () => { }); describe('Constructor and Initialization', () => { - it('should initialize server with correct configuration', () => { + it('builds the core without an SDK Server of its own (issue #1025, step 2)', () => { debugServer = new DebugMcpServer({ logLevel: 'debug' }); - - expect(Server).toHaveBeenCalledWith( - { name: 'debug-mcp-server', version: '1.2.3-test' }, - { - capabilities: { tools: {}, resources: { subscribe: true, listChanged: true }, prompts: {} }, - instructions: expect.stringContaining('mcp-debugger drives real step-through debuggers') - } - ); - + + // The facade (createProtocolServer) constructs the Server per connection + // or request; the core carries what every facade advertises. + expect(Server).not.toHaveBeenCalled(); + expect(debugServer.instructions).toContain('mcp-debugger drives real step-through debuggers'); expect(createProductionDependencies).toHaveBeenCalledWith({ logLevel: 'debug', logFile: undefined, @@ -97,16 +94,16 @@ describe('Server Initialization Tests', () => { expect(() => new DebugMcpServer()).toThrow('Failed to create dependencies'); }); - it('should register tool and resource handlers', () => { - debugServer = new DebugMcpServer(); + it('a facade over the core registers the tool, resource and prompt handlers', () => { + debugServer = createDebugServerWithFacade(); // ListTools + CallTool, ListResources/ReadResource/Subscribe/Unsubscribe (issue #218), // plus ListPrompts/GetPrompt for the debugging-workflow prompt expect(mockServer.setRequestHandler).toHaveBeenCalledTimes(8); }); - it('should set error handler', () => { - debugServer = new DebugMcpServer(); + it('a facade routes SDK errors to the core logger', () => { + debugServer = createDebugServerWithFacade(); expect(mockServer.onerror).toBeDefined(); @@ -122,7 +119,7 @@ describe('Server Initialization Tests', () => { describe('Tool Handler Registration', () => { it('should handle tools/list request', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { listToolsHandler } = getToolHandlers(mockServer); const result = await listToolsHandler({ method: 'tools/list', params: {} }); @@ -139,7 +136,7 @@ describe('Server Initialization Tests', () => { }); it('should handle unknown tool error', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); await expect(callToolHandler({ @@ -152,7 +149,7 @@ describe('Server Initialization Tests', () => { }); it('should handle tool execution errors', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockSessionManager.createSession.mockRejectedValue(new Error('Session creation failed')); diff --git a/tests/core/unit/server/server-inspection-tools.test.ts b/tests/core/unit/server/server-inspection-tools.test.ts index b92eaacb..fc306e91 100644 --- a/tests/core/unit/server/server-inspection-tools.test.ts +++ b/tests/core/unit/server/server-inspection-tools.test.ts @@ -15,7 +15,8 @@ import { getToolHandlers, type CallToolHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; import { OutputRingBuffer } from '../../../../src/session/output-buffer.js'; import { CppAdapterPolicy, JavaAdapterPolicy, JsDebugAdapterPolicy, SessionState, type AdapterPolicy } from '@debugmcp/shared'; @@ -49,7 +50,7 @@ describe('Server Inspection Tools Tests', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; }); diff --git a/tests/core/unit/server/server-language-discovery.test.ts b/tests/core/unit/server/server-language-discovery.test.ts index 10dd9e6b..0198267e 100644 --- a/tests/core/unit/server/server-language-discovery.test.ts +++ b/tests/core/unit/server/server-language-discovery.test.ts @@ -15,7 +15,8 @@ import { createMockSessionManager, getToolHandlers, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; // Mock dependencies @@ -66,7 +67,7 @@ describe('Server Language Discovery Tests', () => { describe('JavaScript availability and metadata', () => { it('should report javascript installed:true in available and include rich JS metadata when resolvable', async () => { - const debugServer = new DebugMcpServer(); + const debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); // Simulate dynamic registry reporting JS resolvable (installed) @@ -105,7 +106,7 @@ describe('Server Language Discovery Tests', () => { describe('Ruby availability and metadata', () => { it('should report ruby metadata when dynamically discovered', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.listLanguages = vi.fn().mockResolvedValue(['python', 'mock', 'ruby']); @@ -149,7 +150,7 @@ describe('Server Language Discovery Tests', () => { describe('C/C++ availability and metadata', () => { it('should report cpp metadata when dynamically discovered', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.listLanguages = vi.fn().mockResolvedValue(['python', 'mock', 'cpp']); @@ -193,7 +194,7 @@ describe('Server Language Discovery Tests', () => { describe('per-mode availability (issue #331)', () => { it('reports attach-only availability when the launch toolchain is missing on a direct-connect adapter', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.listAvailableAdapters = vi.fn().mockResolvedValue([ @@ -219,7 +220,7 @@ describe('Server Language Discovery Tests', () => { }); it('reports attach unsupported for adapters that declare no attach implementation', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.listAvailableAdapters = vi.fn().mockResolvedValue([ @@ -246,7 +247,7 @@ describe('Server Language Discovery Tests', () => { it('keeps disabled languages in available[] with a disabled reason for both modes', async () => { vi.stubEnv('DEBUG_MCP_DISABLE_LANGUAGES', 'go'); - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.listAvailableAdapters = vi.fn().mockResolvedValue([ @@ -273,7 +274,7 @@ describe('Server Language Discovery Tests', () => { }); it('reports uninstalled adapters as unavailable in both modes with a not-installed reason', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.listAvailableAdapters = vi.fn().mockResolvedValue([ @@ -297,7 +298,7 @@ describe('Server Language Discovery Tests', () => { }); it('fails open (reports available) when the toolchain probe throws', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.listAvailableAdapters = vi.fn().mockResolvedValue([ @@ -322,7 +323,7 @@ describe('Server Language Discovery Tests', () => { describe('getSupportedLanguagesAsync', () => { it('should return languages from dynamic discovery when available', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); // Mock the registry to support dynamic discovery @@ -350,7 +351,7 @@ describe('Server Language Discovery Tests', () => { }); it('should fallback to getSupportedLanguages when dynamic discovery fails', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); // Mock dynamic discovery to fail @@ -379,7 +380,7 @@ describe('Server Language Discovery Tests', () => { // the session manager. Delete the key rather than widen the doubles' types. Reflect.deleteProperty(mockSessionManager, 'adapterRegistry'); - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); const result = await callToolHandler({ @@ -397,7 +398,7 @@ describe('Server Language Discovery Tests', () => { }); it('should handle empty language lists from registry', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); // Mock empty responses @@ -427,7 +428,7 @@ describe('Server Language Discovery Tests', () => { mockAdapterRegistry.listLanguages = vi.fn().mockResolvedValue(['mock']); mockAdapterRegistry.getSupportedLanguages = vi.fn().mockReturnValue(['mock']); - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); const result = await callToolHandler({ @@ -448,7 +449,7 @@ describe('Server Language Discovery Tests', () => { describe('getLanguageMetadata', () => { it('should generate metadata for discovered languages', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.listLanguages = vi.fn().mockResolvedValue(['python', 'mock']); @@ -469,7 +470,7 @@ describe('Server Language Discovery Tests', () => { }); it('should handle unknown languages in metadata generation', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.listLanguages = vi.fn().mockResolvedValue(['python', 'unknown-language']); @@ -505,7 +506,7 @@ describe('Server Language Discovery Tests', () => { describe('create_debug_session with language validation', () => { it('should validate language support before creating session', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.listLanguages = vi.fn().mockResolvedValue(['python', 'mock']); @@ -537,7 +538,7 @@ describe('Server Language Discovery Tests', () => { }); it('should reject unsupported languages', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.listLanguages = vi.fn().mockResolvedValue(['python', 'mock']); @@ -556,7 +557,7 @@ describe('Server Language Discovery Tests', () => { }); it('should handle language validation errors gracefully', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); // Mock language discovery to fail @@ -577,7 +578,7 @@ describe('Server Language Discovery Tests', () => { }); it('fails fast with the availability reason when the toolchain probe reports unavailable (issue #360)', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.getFactory = vi.fn().mockResolvedValue({ @@ -606,7 +607,7 @@ describe('Server Language Discovery Tests', () => { }); it('fails open (creates the session) when the toolchain probe throws (issue #360)', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.getFactory = vi.fn().mockResolvedValue({ @@ -631,7 +632,7 @@ describe('Server Language Discovery Tests', () => { }); it('allows session creation despite a failed launch probe when direct-connect attach is usable (issue #360)', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); const validate = vi.fn().mockResolvedValue({ valid: false, errors: ['no toolchain'], warnings: [] }); @@ -659,7 +660,7 @@ describe('Server Language Discovery Tests', () => { }); it('still fails creation when launch is unavailable and attach is spawn-based (issue #360)', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.getFactory = vi.fn().mockResolvedValue({ @@ -693,7 +694,7 @@ describe('Server Language Discovery Tests', () => { }); it('starts debugging for a READY session', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); const result = await callToolHandler({ @@ -722,7 +723,7 @@ describe('Server Language Discovery Tests', () => { }); it('starts debugging without re-validating the session language', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); // A session language outside the default list: start_debugging does not @@ -774,7 +775,7 @@ describe('Server Language Discovery Tests', () => { Reflect.deleteProperty(registryWithoutListLanguages, 'listLanguages'); mockSessionManager.adapterRegistry = registryWithoutListLanguages; - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); const result = await callToolHandler({ @@ -799,7 +800,7 @@ describe('Server Language Discovery Tests', () => { }); it('should handle registry method exceptions', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); const { callToolHandler } = getToolHandlers(mockServer); mockAdapterRegistry.listLanguages = vi.fn().mockImplementation(() => { diff --git a/tests/core/unit/server/server-lifecycle.test.ts b/tests/core/unit/server/server-lifecycle.test.ts index 06223929..ea429f1b 100644 --- a/tests/core/unit/server/server-lifecycle.test.ts +++ b/tests/core/unit/server/server-lifecycle.test.ts @@ -13,7 +13,8 @@ import { createMockSessionManager, createMockStdioTransport, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; // Mock dependencies @@ -49,7 +50,7 @@ describe('Server Lifecycle Tests', () => { describe('Server Start', () => { it('should start server with stdio transport', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); await debugServer.start(); @@ -59,7 +60,7 @@ describe('Server Lifecycle Tests', () => { describe('Server Stop', () => { it('should stop server and close all sessions', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); mockSessionManager.closeAllSessions.mockResolvedValue(undefined); await debugServer.stop(); @@ -70,7 +71,7 @@ describe('Server Lifecycle Tests', () => { it('stop() disposes the container logger so churning HTTP sessions do not leak (issue #404)', async () => { mockDependencies.disposeLogger = vi.fn(); - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); mockSessionManager.closeAllSessions.mockResolvedValue(undefined); await debugServer.stop(); @@ -79,7 +80,7 @@ describe('Server Lifecycle Tests', () => { }); it('should propagate closeAllSessions errors from stop', async () => { - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); mockSessionManager.closeAllSessions.mockRejectedValue(new Error('Close sessions failed')); // stop() awaits closeAllSessions() without catching, so the rejection propagates diff --git a/tests/core/unit/server/server-logpoint-gating.test.ts b/tests/core/unit/server/server-logpoint-gating.test.ts index 4740228b..af6de58b 100644 --- a/tests/core/unit/server/server-logpoint-gating.test.ts +++ b/tests/core/unit/server/server-logpoint-gating.test.ts @@ -21,7 +21,8 @@ import { getToolHandlers, type CallToolHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -49,7 +50,7 @@ describe('set_breakpoint logMessage gating', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; mockSessionManager.getSession.mockReturnValue({ diff --git a/tests/core/unit/server/server-loud-snapping.test.ts b/tests/core/unit/server/server-loud-snapping.test.ts index eb1cc5b5..1be1fa3d 100644 --- a/tests/core/unit/server/server-loud-snapping.test.ts +++ b/tests/core/unit/server/server-loud-snapping.test.ts @@ -21,7 +21,8 @@ import { getToolHandlers, type CallToolHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -62,7 +63,7 @@ describe('set_breakpoint loud snapping (#271)', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; mockSessionManager.getSession.mockReturnValue({ diff --git a/tests/core/unit/server/server-prompts.test.ts b/tests/core/unit/server/server-prompts.test.ts index 52fade61..7d40c7e9 100644 --- a/tests/core/unit/server/server-prompts.test.ts +++ b/tests/core/unit/server/server-prompts.test.ts @@ -16,7 +16,8 @@ import { createMockSessionManager, createMockStdioTransport, getPromptHandlers, - type MockServer + type MockServer, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -50,7 +51,7 @@ describe('Server Prompts Tests', () => { const mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); }); afterEach(() => { diff --git a/tests/core/unit/server/server-redaction.test.ts b/tests/core/unit/server/server-redaction.test.ts index fe2847f2..ca43f8da 100644 --- a/tests/core/unit/server/server-redaction.test.ts +++ b/tests/core/unit/server/server-redaction.test.ts @@ -21,7 +21,8 @@ import { getToolHandlers, type CallToolHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; import { OutputRingBuffer } from '../../../../src/session/output-buffer.js'; @@ -52,7 +53,7 @@ describe('Server redaction notices (issue #237)', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; mockSessionManager.getSession.mockReturnValue({ diff --git a/tests/core/unit/server/server-redefine-and-attach.test.ts b/tests/core/unit/server/server-redefine-and-attach.test.ts index a017792d..e8b25e59 100644 --- a/tests/core/unit/server/server-redefine-and-attach.test.ts +++ b/tests/core/unit/server/server-redefine-and-attach.test.ts @@ -20,7 +20,8 @@ import { type CallToolHandler, type ListToolsHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -49,7 +50,7 @@ describe('hot_reload and attach stopOnEntry tests', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function () { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); const handlers = getToolHandlers(mockServer); callToolHandler = handlers.callToolHandler; listToolsHandler = handlers.listToolsHandler; diff --git a/tests/core/unit/server/server-required-arguments.test.ts b/tests/core/unit/server/server-required-arguments.test.ts index 6e22cc0c..748d80c4 100644 --- a/tests/core/unit/server/server-required-arguments.test.ts +++ b/tests/core/unit/server/server-required-arguments.test.ts @@ -14,7 +14,8 @@ import { createMockSessionManager, createMockStdioTransport, getToolHandlers, - type CallToolHandler + type CallToolHandler, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -53,7 +54,7 @@ describe('schema-driven required arguments', () => { const manager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return manager as never; }); - new DebugMcpServer(); + createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; }); diff --git a/tests/core/unit/server/server-resources.test.ts b/tests/core/unit/server/server-resources.test.ts index f5afb6e7..263ef4d9 100644 --- a/tests/core/unit/server/server-resources.test.ts +++ b/tests/core/unit/server/server-resources.test.ts @@ -19,7 +19,8 @@ import { createMockStdioTransport, getResourceHandlers, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -72,7 +73,7 @@ describe('Server Output Resources Tests', () => { }); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - debugServer = new DebugMcpServer(); + debugServer = createDebugServerWithFacade(); }); afterEach(() => { @@ -342,7 +343,7 @@ describe('Server Output Resources Tests', () => { await expect(subscribe('debug://sessions/sess-1/proxy-log')).rejects.toBeInstanceOf(ProtocolError); }); - it('stops pinging after unsubscribe, cancelling any pending timer', async () => { + it('stops pinging after unsubscribe: a pending ping fires into the facade filter and is dropped', async () => { mockSessionManager.getSession.mockReturnValue(mockSession()); await subscribe('debug://sessions/sess-1/output'); diff --git a/tests/core/unit/server/server-restart-tool.test.ts b/tests/core/unit/server/server-restart-tool.test.ts index e691d654..3ac236d7 100644 --- a/tests/core/unit/server/server-restart-tool.test.ts +++ b/tests/core/unit/server/server-restart-tool.test.ts @@ -19,7 +19,8 @@ import { type CallToolHandler, type ListToolsHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -48,7 +49,7 @@ describe('restart_debugging tool', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); const handlers = getToolHandlers(mockServer); callToolHandler = handlers.callToolHandler; listToolsHandler = handlers.listToolsHandler; diff --git a/tests/core/unit/server/server-session-tools.test.ts b/tests/core/unit/server/server-session-tools.test.ts index 0b57bb26..4c3f10e6 100644 --- a/tests/core/unit/server/server-session-tools.test.ts +++ b/tests/core/unit/server/server-session-tools.test.ts @@ -16,7 +16,8 @@ import { getToolHandlers, type CallToolHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; // Mock dependencies @@ -52,7 +53,7 @@ describe('Server Session Tools Tests', () => { }); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - server = new DebugMcpServer(); + server = createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; }); @@ -248,8 +249,6 @@ describe('Server Session Tools Tests', () => { }); describe('close_debug_session', () => { - const outputResources = () => - (server as unknown as { outputResources: { forgetSession: (id: string) => void; notifyListChanged: () => void } }).outputResources; it('should close session successfully', async () => { mockSessionManager.closeSessionReporting.mockResolvedValue({ closed: true, closedSessionIds: ['test-session'] }); @@ -272,8 +271,8 @@ describe('Server Session Tools Tests', () => { closedSessionIds.forEach(sessionRemovedListener); return { closed: true, closedSessionIds, childrenClosed: 2, released: true }; }); - const forget = vi.spyOn(outputResources(), 'forgetSession'); - const announce = vi.spyOn(outputResources(), 'notifyListChanged'); + const forget = vi.spyOn(server.notifications, 'forgetUri'); + const announce = vi.spyOn(server.notifications, 'resourcesChanged'); const result = await callToolHandler({ method: 'tools/call', @@ -282,7 +281,11 @@ describe('Server Session Tools Tests', () => { const content = JSON.parse(result.content[0].text); expect(content).toEqual({ success: true, message: 'Closed debug session: test-session', childrenClosed: 2, released: true }); - expect(forget.mock.calls.map(([id]) => id)).toEqual(['grandchild', 'child', 'test-session']); + expect(forget.mock.calls.map(([uri]) => uri)).toEqual([ + 'debug://sessions/grandchild/output', + 'debug://sessions/child/output', + 'debug://sessions/test-session/output' + ]); // Once per removal, with no duplicate notification from the handler. expect(announce).toHaveBeenCalledTimes(3); }); diff --git a/tests/core/unit/server/server-stale-build.test.ts b/tests/core/unit/server/server-stale-build.test.ts index 7c79b3b0..52f275fa 100644 --- a/tests/core/unit/server/server-stale-build.test.ts +++ b/tests/core/unit/server/server-stale-build.test.ts @@ -25,7 +25,8 @@ import { getToolHandlers, type CallToolHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -88,7 +89,7 @@ describe('set_breakpoint stale-build warning (#938)', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; // A pre-launch session: no lastLaunch, so nothing says the program runs from .ts. diff --git a/tests/core/unit/server/server-statement-anchor.test.ts b/tests/core/unit/server/server-statement-anchor.test.ts index dbd6ceea..2e2f253d 100644 --- a/tests/core/unit/server/server-statement-anchor.test.ts +++ b/tests/core/unit/server/server-statement-anchor.test.ts @@ -22,7 +22,8 @@ import { getToolHandlers, type CallToolHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -65,7 +66,7 @@ describe('set_breakpoint statement anchors (#271)', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; mockSessionManager.getSession.mockReturnValue({ diff --git a/tests/core/unit/server/server-test-helpers.ts b/tests/core/unit/server/server-test-helpers.ts index fbeb1bd5..cd913997 100644 --- a/tests/core/unit/server/server-test-helpers.ts +++ b/tests/core/unit/server/server-test-helpers.ts @@ -4,7 +4,8 @@ import { vi } from 'vitest'; import type { EmptyResult, GetPromptResult, ListPromptsResult, ListResourcesResult, ReadResourceResult, Tool } from '@modelcontextprotocol/server'; import type { IAdapterRegistry } from '@debugmcp/shared'; -import { DebugMcpServer } from '../../../../src/server.js'; +import { DebugMcpServer, type DebugMcpServerOptions } from '../../../../src/server.js'; +import { createProtocolServer } from '../../../../src/server/protocol-facade.js'; import type { SessionManager } from '../../../../src/session/session-manager.js'; import type { ToolResult } from '../../../../src/server/tool-result.js'; import { @@ -56,10 +57,26 @@ export function createMockServer() { close: vi.fn(), sendResourceUpdated: vi.fn().mockResolvedValue(undefined), sendResourceListChanged: vi.fn().mockResolvedValue(undefined), - onerror: undefined as ((error: Error) => void) | undefined + getClientVersion: vi.fn(), + getNegotiatedProtocolVersion: vi.fn(), + onerror: undefined as ((error: Error) => void) | undefined, + onclose: undefined as (() => void) | undefined, + oninitialized: undefined as (() => void) | undefined }; } +/** + * The core plus one legacy-era facade over it, which is what every suite that + * reads handlers off the mocked SDK Server needs (issue #1025, step 2): the + * core itself constructs no Server, so `new DebugMcpServer()` alone registers + * nothing on `createMockServer()`. + */ +export function createDebugServerWithFacade(options?: DebugMcpServerOptions): DebugMcpServer { + const core = new DebugMcpServer(options); + createProtocolServer(core, { era: 'legacy', deliver: 'instance' }); + return core; +} + export type MockServer = ReturnType; export type MockSessionManager = ReturnType; diff --git a/tests/core/unit/server/server-tool-list-snapshot.test.ts b/tests/core/unit/server/server-tool-list-snapshot.test.ts index 74de3560..3e52e8e4 100644 --- a/tests/core/unit/server/server-tool-list-snapshot.test.ts +++ b/tests/core/unit/server/server-tool-list-snapshot.test.ts @@ -30,7 +30,8 @@ import { createMockStdioTransport, getToolHandlers, getResourceHandlers, - getPromptHandlers + getPromptHandlers, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -91,7 +92,7 @@ describe('Server tool/resource/prompt list snapshot fence', () => { vi.stubEnv('DEBUG_MCP_VARIABLE_ACCESS', vaMode); vi.stubEnv('MCP_CONTAINER', containerMode); - new DebugMcpServer(); + createDebugServerWithFacade(); const { listToolsHandler } = getToolHandlers(mockServer); const { listResourcesHandler } = getResourceHandlers(mockServer); const { listPromptsHandler } = getPromptHandlers(mockServer); diff --git a/tests/core/unit/server/server-tool-response-logging.test.ts b/tests/core/unit/server/server-tool-response-logging.test.ts index 50e03172..0b25d839 100644 --- a/tests/core/unit/server/server-tool-response-logging.test.ts +++ b/tests/core/unit/server/server-tool-response-logging.test.ts @@ -20,7 +20,8 @@ import { getToolHandlers, type CallToolHandler, type MockServer, - type MockSessionManager + type MockSessionManager, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -48,7 +49,7 @@ describe('tool:response logging (issue #397)', () => { mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function () { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; }); diff --git a/tests/core/unit/server/server-variable-access-gating.test.ts b/tests/core/unit/server/server-variable-access-gating.test.ts index 6ea9b805..46969679 100644 --- a/tests/core/unit/server/server-variable-access-gating.test.ts +++ b/tests/core/unit/server/server-variable-access-gating.test.ts @@ -19,7 +19,8 @@ import { createMockSessionManager, createMockStdioTransport, findTool, - getToolHandlers + getToolHandlers, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -45,7 +46,7 @@ function createServer(envOverrides: Record) { const mockSessionManager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return mockSessionManager as any; }); - new DebugMcpServer(); + createDebugServerWithFacade(); mockSessionManager.getSession.mockReturnValue({ id: 'test-session', sessionLifecycle: 'ACTIVE' }); return { ...getToolHandlers(mockServer), mockSessionManager }; diff --git a/tests/core/unit/server/tool-alias.test.ts b/tests/core/unit/server/tool-alias.test.ts index ee8d4b2e..e5be2ac1 100644 --- a/tests/core/unit/server/tool-alias.test.ts +++ b/tests/core/unit/server/tool-alias.test.ts @@ -16,7 +16,8 @@ import { createMockSessionManager, createMockStdioTransport, getToolHandlers, - type CallToolHandler + type CallToolHandler, + createDebugServerWithFacade } from './server-test-helpers.js'; vi.mock('@modelcontextprotocol/server', async () => @@ -39,7 +40,7 @@ describe('tool aliases (issue #876)', () => { vi.mocked(StdioServerTransport).mockImplementation(function() { return createMockStdioTransport() as never; }); manager = createMockSessionManager(mockDependencies.adapterRegistry); vi.mocked(SessionManager).mockImplementation(function() { return manager as never; }); - new DebugMcpServer(); + createDebugServerWithFacade(); callToolHandler = getToolHandlers(mockServer).callToolHandler; }); diff --git a/tests/core/unit/server/tool-dispatch-foreign-session.test.ts b/tests/core/unit/server/tool-dispatch-foreign-session.test.ts index 66bc6407..153024dd 100644 --- a/tests/core/unit/server/tool-dispatch-foreign-session.test.ts +++ b/tests/core/unit/server/tool-dispatch-foreign-session.test.ts @@ -38,7 +38,7 @@ function fakeContext(overrides: Partial = {}): ToolContext { throw new SessionNotFoundError(sessionId); }, closeDebugSession: vi.fn(async () => ({ closed: false, closedSessionIds: [] })), - outputResources: { forgetSession: vi.fn(), notifyListChanged: vi.fn() }, + notifications: { forgetUri: vi.fn(), resourcesChanged: vi.fn() }, ...overrides } as unknown as ToolContext; } diff --git a/tests/unit/cli/http-command.test.ts b/tests/unit/cli/http-command.test.ts index e94d8c62..cfe31ecb 100644 --- a/tests/unit/cli/http-command.test.ts +++ b/tests/unit/cli/http-command.test.ts @@ -7,6 +7,7 @@ import { FakeCurrentProcess } from '../../test-utils/mocks/fake-current-process. import { invokeMiddleware } from '../../test-utils/mocks/express-stubs.js'; import type { Logger as WinstonLoggerType } from 'winston'; import { DebugMcpServer } from '../../../src/server.js'; +import type { createProtocolServer } from '../../../src/server/protocol-facade.js'; vi.mock('../../../src/server.js'); vi.mock('@modelcontextprotocol/node'); @@ -38,6 +39,8 @@ describe('HTTP Command Handler', () => { let mockExitProcess: Mock<(code: number) => void>; let mockServer: DebugMcpServer; let mockTransport: any; + let mockFacade: { connect: Mock<() => Promise> }; + let mockProtocolServerFactory: Mock; let fakeProc: FakeCurrentProcess; let mockApp: any; @@ -54,17 +57,18 @@ describe('HTTP Command Handler', () => { level: 'info', } as any; + // The core (issue #1025, step 2): it owns no SDK Server; the app builds a + // protocol facade per MCP session and connects THAT to the transport. mockServer = { start: vi.fn().mockResolvedValue(undefined), stop: vi.fn().mockResolvedValue(undefined), - server: { - connect: vi.fn().mockResolvedValue(undefined), - }, sessionManager: { getAllSessions: vi.fn().mockReturnValue([]), getSession: vi.fn().mockReturnValue(undefined), }, } as any; + mockFacade = { connect: vi.fn<() => Promise>().mockResolvedValue(undefined) }; + mockProtocolServerFactory = vi.fn(() => mockFacade as any); mockServerFactory = vi.fn<(options: ServerFactoryOptions) => DebugMcpServer>().mockReturnValue(mockServer); mockExitProcess = vi.fn<(code: number) => void>(); @@ -124,7 +128,7 @@ describe('HTTP Command Handler', () => { describe('createHttpApp', () => { it('builds the app itself: Host allowlist first, then CORS, then a single 10 MB JSON parser (issues #667, #670)', () => { - createHttpApp({ port: '3001' }, { logger: mockLogger, serverFactory: mockServerFactory }); + createHttpApp({ port: '3001' }, { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory }); expect(expressMock.express).toHaveBeenCalledTimes(1); const uses = mockApp.use.mock.calls.map((c: any[]) => c[0]); @@ -143,7 +147,7 @@ describe('HTTP Command Handler', () => { }); it('answers body-parser failures in the JSON-RPC shape and passes every other error on (issue #670)', () => { - createHttpApp({ port: '3001' }, { logger: mockLogger, serverFactory: mockServerFactory }); + createHttpApp({ port: '3001' }, { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory }); const errorHandler = mockApp.use.mock.calls[3][0] as (err: unknown, req: any, res: any, next: any) => void; const run = (err: unknown, headersSent = false) => { const json = vi.fn(); @@ -192,7 +196,7 @@ describe('HTTP Command Handler', () => { }); it('accepts only the loopback trio by default', () => { - createHttpApp({ port: '3001' }, { logger: mockLogger, serverFactory: mockServerFactory }); + createHttpApp({ port: '3001' }, { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory }); const hostMiddleware = mockApp.use.mock.calls[0][0]; for (const host of ['localhost', '127.0.0.1:3001', '[::1]:3001']) { expect(runHost(hostMiddleware, host).next, host).toHaveBeenCalledTimes(1); @@ -204,7 +208,7 @@ describe('HTTP Command Handler', () => { fakeProc.env.MCP_HTTP_ALLOWED_HOSTS = 'from-env.example'; createHttpApp( { port: '3001', allowedHost: ['from-flag.example'] }, - { logger: mockLogger, serverFactory: mockServerFactory, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, proc: fakeProc } ); const hostMiddleware = mockApp.use.mock.calls[0][0]; expect(runHost(hostMiddleware, 'from-flag.example:3000').next).toHaveBeenCalledTimes(1); @@ -218,25 +222,25 @@ describe('HTTP Command Handler', () => { it('exposes the per-session transport map for graceful shutdown', () => { const app = createHttpApp( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory } ); expect((app as any).httpSessions).toBeInstanceOf(Map); }); it('registers /mcp on POST, GET, and DELETE', () => { - createHttpApp({ port: '3001' }, { logger: mockLogger, serverFactory: mockServerFactory }); + createHttpApp({ port: '3001' }, { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory }); expect(mockApp.post).toHaveBeenCalledWith('/mcp', expect.any(Function)); expect(mockApp.get).toHaveBeenCalledWith('/mcp', expect.any(Function)); expect(mockApp.delete).toHaveBeenCalledWith('/mcp', expect.any(Function)); }); it('registers a /health endpoint', () => { - createHttpApp({ port: '3001' }, { logger: mockLogger, serverFactory: mockServerFactory }); + createHttpApp({ port: '3001' }, { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory }); expect(mockApp.get).toHaveBeenCalledWith('/health', expect.any(Function)); }); it('installs a CORS middleware that exposes Mcp-Session-Id and related headers', () => { - createHttpApp({ port: '3001' }, { logger: mockLogger, serverFactory: mockServerFactory }); + createHttpApp({ port: '3001' }, { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory }); // CORS is the second use() call, after the Host allowlist const corsMiddleware = mockApp.use.mock.calls[1][0]; @@ -275,7 +279,7 @@ describe('HTTP Command Handler', () => { function getHandler() { const app = createHttpApp( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory } ); const postCall = mockApp.post.mock.calls.find((c: any) => c[0] === '/mcp'); return { app, handler: postCall![1] as (req: any, res: any) => Promise }; @@ -314,7 +318,8 @@ describe('HTTP Command Handler', () => { expect(mockServerFactory).toHaveBeenCalledTimes(1); expect(MockedStreamableHTTPServerTransport).toHaveBeenCalledTimes(1); - expect(mockServer.server.connect).toHaveBeenCalledWith(mockTransport); + expect(mockProtocolServerFactory).toHaveBeenCalledWith(mockServer, { era: 'legacy', deliver: 'instance' }); + expect(mockFacade.connect).toHaveBeenCalledWith(mockTransport); expect(typeof lastTransportOptions.sessionIdGenerator).toBe('function'); expect(typeof lastTransportOptions.onsessioninitialized).toBe('function'); expect(mockTransport.handleRequest).toHaveBeenCalledWith(req, res, req.body); @@ -487,7 +492,7 @@ describe('HTTP Command Handler', () => { it('reports the address and port the server listens on (issue #680)', () => { createHttpApp( { port: '3001', bind: '0.0.0.0' }, - { logger: mockLogger, serverFactory: mockServerFactory } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory } ); const healthCall = mockApp.get.mock.calls.find((c: any) => c[0] === '/health'); const res = { json: vi.fn() }; @@ -498,7 +503,7 @@ describe('HTTP Command Handler', () => { it('reports mode http and the active session count', async () => { const app = createHttpApp( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory } ); const healthCall = mockApp.get.mock.calls.find((c: any) => c[0] === '/health'); const healthHandler = healthCall![1]; @@ -536,7 +541,7 @@ describe('HTTP Command Handler', () => { vi.setSystemTime(100_000); const app = createHttpApp( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory } ); const healthHandler = mockApp.get.mock.calls.find((c: any) => c[0] === '/health')![1]; (mockServer as any).sessionManager.getAllSessions.mockReturnValue([ @@ -592,7 +597,7 @@ describe('HTTP Command Handler', () => { } const app = createHttpApp( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, proc: fakeProc } ); const postCall = mockApp.post.mock.calls.find((c: any) => c[0] === '/mcp'); return { app, handler: postCall![1] as (req: any, res: any) => Promise }; @@ -862,7 +867,7 @@ describe('HTTP Command Handler', () => { Object.assign(fakeProc.env, env); const app = createHttpApp( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, proc: fakeProc } ); const handler = mockApp.post.mock.calls.find((c: any) => c[0] === '/mcp')![1]; await handler( @@ -913,7 +918,7 @@ describe('HTTP Command Handler', () => { it('refuses to start on an unusable --allowed-host value, naming it (issue #667)', async () => { await handleHttpCommand( { port: '3001', allowedHost: ['http://bad.example'] }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); expect(mockExitProcess).toHaveBeenCalledWith(1); @@ -946,7 +951,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '4000', bind: '0.0.0.0' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); expect(listen).toHaveBeenCalledWith(4000, '0.0.0.0'); @@ -968,7 +973,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '4000' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); expect(listen).toHaveBeenCalledWith(4000, '10.0.0.5'); expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('(from MCP_HTTP_BIND)')); @@ -977,7 +982,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '4001', bind: '127.0.0.1' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); expect(listen).toHaveBeenCalledWith(4001, '127.0.0.1'); }); @@ -988,7 +993,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '4000', bind: 'localhost' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); expect(listen).toHaveBeenCalledWith(4000, '127.0.0.1'); @@ -1004,7 +1009,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); expect(listen).toHaveBeenCalledWith(3001, '0.0.0.0'); @@ -1024,7 +1029,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '3001', bind: '10.255.255.9' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); handlers.error(Object.assign(new Error('listen EADDRNOTAVAIL'), { code: 'EADDRNOTAVAIL' })); @@ -1049,7 +1054,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '0' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); const healthCall = mockApp.get.mock.calls.find((c: any) => c[0] === '/health'); const before = { json: vi.fn() }; @@ -1065,7 +1070,7 @@ describe('HTTP Command Handler', () => { it.each(['abc', '3001x'])("refuses to start on an unusable --port value '%s', naming it (issue #689)", async (port) => { await handleHttpCommand( { port }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); expect(mockExitProcess).toHaveBeenCalledWith(1); @@ -1096,7 +1101,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '0' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); // Not announced until the socket is bound: the requested port is not the bound one. expect(mockLogger.info).not.toHaveBeenCalledWith(expect.stringContaining('listening on')); @@ -1115,7 +1120,7 @@ describe('HTTP Command Handler', () => { it('refuses to start on an unusable --bind value, naming it (issue #680)', async () => { await handleHttpCommand( { port: '3001', bind: 'myhost' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); expect(mockExitProcess).toHaveBeenCalledWith(1); @@ -1135,7 +1140,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '4000', logLevel: 'debug' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); expect(mockLogger.level).toBe('debug'); @@ -1159,7 +1164,7 @@ describe('HTTP Command Handler', () => { // must swallow that (best-effort) and startup must proceed normally. await handleHttpCommand( { port: '4001', logLevel: 'debug', logFile: path.join(os.tmpdir(), 'http-cmd-502-test.log') }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); expect(listen).toHaveBeenCalledWith(4001, '127.0.0.1'); @@ -1174,7 +1179,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); expect(mockLogger.error).toHaveBeenCalledWith('Failed to start server in HTTP mode', { error }); @@ -1194,7 +1199,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); handlers.error(Object.assign(new Error('listen EADDRINUSE'), { code: 'EADDRINUSE' })); @@ -1209,7 +1214,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); // gracefulShutdown is async — grab the registered listener so it can be awaited @@ -1240,7 +1245,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); const sigintHandler = fakeProc.lastListener('SIGINT'); @@ -1269,7 +1274,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); const sigintHandler = fakeProc.lastListener('SIGINT'); @@ -1302,7 +1307,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, stdin, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, stdin, proc: fakeProc } ); // The watchdog must resume stdin so EOF is actually observed @@ -1327,7 +1332,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, stdin, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, stdin, proc: fakeProc } ); stdin.emit('end'); @@ -1343,7 +1348,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, stdin, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, stdin, proc: fakeProc } ); expect(stdin.resume).not.toHaveBeenCalled(); @@ -1365,7 +1370,7 @@ describe('HTTP Command Handler', () => { await handleHttpCommand( { port: '3001' }, - { logger: mockLogger, serverFactory: mockServerFactory, exitProcess: mockExitProcess, proc: fakeProc } + { logger: mockLogger, serverFactory: mockServerFactory, protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc } ); const err = Object.assign(new Error('addr in use'), { code: 'EADDRINUSE' }); diff --git a/tests/unit/cli/stdio-command.test.ts b/tests/unit/cli/stdio-command.test.ts index cc2e26d5..d9c789ca 100644 --- a/tests/unit/cli/stdio-command.test.ts +++ b/tests/unit/cli/stdio-command.test.ts @@ -6,6 +6,7 @@ import type { ServerFactoryOptions } from '../../../src/cli/stdio-command.js'; import { FakeCurrentProcess } from '../../test-utils/mocks/fake-current-process.js'; import type { Logger as WinstonLoggerType } from 'winston'; import { DebugMcpServer } from '../../../src/server.js'; +import type { createProtocolServer } from '../../../src/server/protocol-facade.js'; vi.mock('../../../src/server.js'); @@ -14,6 +15,8 @@ describe('STDIO Command Handler', () => { let mockServerFactory: Mock<(options: ServerFactoryOptions) => DebugMcpServer>; let mockExitProcess: Mock<(code: number) => void>; let mockServer: DebugMcpServer; + let mockFacade: { connect: Mock<() => Promise> }; + let mockProtocolServerFactory: Mock; let fakeProc: FakeCurrentProcess; function makeFakeStdin() { @@ -35,14 +38,14 @@ describe('STDIO Command Handler', () => { level: 'info' } as any; - // Create mock server with proper structure + // The core (issue #1025, step 2): it owns no SDK Server; the command + // builds a protocol facade over it and connects THAT to the transport. mockServer = { - server: { - connect: vi.fn().mockResolvedValue(undefined) - }, start: vi.fn().mockResolvedValue(undefined), stop: vi.fn().mockResolvedValue(undefined) } as any; + mockFacade = { connect: vi.fn().mockResolvedValue(undefined) }; + mockProtocolServerFactory = vi.fn(() => mockFacade as any); // Create mock server factory mockServerFactory = vi.fn(() => mockServer); @@ -70,6 +73,7 @@ describe('STDIO Command Handler', () => { await handleStdioCommand(options, { logger: mockLogger, serverFactory: mockServerFactory, + protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, stdin: makeFakeStdin(), proc: fakeProc @@ -90,6 +94,11 @@ 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); + // Verify success log expect(mockLogger.info).toHaveBeenCalledWith('Server started successfully in stdio mode'); @@ -101,6 +110,7 @@ describe('STDIO Command Handler', () => { await handleStdioCommand({}, { logger: mockLogger, serverFactory: mockServerFactory, + protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, stdin: makeFakeStdin(), proc: fakeProc @@ -129,6 +139,7 @@ describe('STDIO Command Handler', () => { await handleStdioCommand(options, { logger: mockLogger, serverFactory: mockServerFactory, + protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, stdin: makeFakeStdin(), proc: fakeProc @@ -154,6 +165,7 @@ describe('STDIO Command Handler', () => { await handleStdioCommand(options, { logger: mockLogger, serverFactory: mockServerFactory, + protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc }); @@ -174,6 +186,7 @@ describe('STDIO Command Handler', () => { await handleStdioCommand({}, { logger: mockLogger, serverFactory: mockServerFactory, + protocolServerFactory: mockProtocolServerFactory, proc: fakeProc }); @@ -190,6 +203,7 @@ describe('STDIO Command Handler', () => { await handleStdioCommand({}, { logger: mockLogger, serverFactory: mockServerFactory, + protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, proc: fakeProc }); @@ -208,6 +222,7 @@ describe('STDIO Command Handler', () => { await handleStdioCommand({}, { logger: mockLogger, serverFactory: mockServerFactory, + protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -234,6 +249,7 @@ describe('STDIO Command Handler', () => { await handleStdioCommand({}, { logger: mockLogger, serverFactory: mockServerFactory, + protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -256,6 +272,7 @@ describe('STDIO Command Handler', () => { await handleStdioCommand({}, { logger: mockLogger, serverFactory: mockServerFactory, + protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -311,6 +328,7 @@ describe('STDIO Command Handler', () => { await handleStdioCommand({}, { logger: mockLogger, serverFactory: mockServerFactory, + protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, stdin, proc: fakeProc @@ -331,6 +349,7 @@ describe('STDIO Command Handler', () => { await handleStdioCommand({}, { logger: mockLogger, serverFactory: mockServerFactory, + protocolServerFactory: mockProtocolServerFactory, exitProcess: mockExitProcess, stdin, proc: fakeProc diff --git a/tests/unit/server-coverage.test.ts b/tests/unit/server-coverage.test.ts index e3d9133f..267f11fc 100644 --- a/tests/unit/server-coverage.test.ts +++ b/tests/unit/server-coverage.test.ts @@ -12,7 +12,8 @@ import { SessionManager } from '../../src/session/session-manager.js'; import { createMockDependencies, createMockServer, - getResourceHandlers + getResourceHandlers, + createDebugServerWithFacade } from '../core/unit/server/server-test-helpers.js'; // The same preamble the sibling suites under tests/core/unit/server/ use. @@ -93,7 +94,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { // No logFile: with the container mocked the option is inert, and naming a // path here is what made this suite write one. - server = new DebugMcpServer({ logLevel: 'info' }); + server = createDebugServerWithFacade({ logLevel: 'info' }); (server as any).logger = mockLogger; });