diff --git a/src/connection/sdk-client.test.ts b/src/connection/sdk-client.test.ts new file mode 100644 index 00000000..57cf7c4d --- /dev/null +++ b/src/connection/sdk-client.test.ts @@ -0,0 +1,77 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import express from 'express'; +import type { Server as HttpServer } from 'http'; +import { connectToServer } from './sdk-client'; + +// Regression guard for the session-termination behavior of close(): a bad +// merge of connectToServer must not silently drop the DELETE (issue #79). +describe('connectToServer close()', () => { + let httpServer: HttpServer | undefined; + + afterEach( + () => + new Promise((resolve) => { + if (!httpServer) return resolve(); + httpServer.closeAllConnections?.(); + httpServer.close(() => resolve()); + httpServer = undefined; + }) + ); + + function startServer(sessionId?: string): Promise<{ + url: string; + deletes: Array; + }> { + const deletes: Array = []; + const app = express(); + app.use(express.json()); + app.post('/mcp', (req, res) => { + if (req.body?.method === 'initialize') { + if (sessionId) res.set('mcp-session-id', sessionId); + res.json({ + jsonrpc: '2.0', + id: req.body.id, + result: { + protocolVersion: '2025-11-25', + capabilities: {}, + serverInfo: { name: 'delete-probe', version: '1.0.0' } + } + }); + } else { + res.status(202).end(); + } + }); + app.delete('/mcp', (req, res) => { + deletes.push(req.headers['mcp-session-id'] as string | undefined); + res.status(200).end(); + }); + return new Promise((resolve, reject) => { + const server = app.listen(0); + httpServer = server; + server.on('error', reject); + server.on('listening', () => { + const addr = server.address(); + const port = typeof addr === 'object' && addr ? addr.port : 0; + resolve({ url: `http://localhost:${port}/mcp`, deletes }); + }); + }); + } + + it('sends an HTTP DELETE with the session ID to terminate the session', async () => { + const { url, deletes } = await startServer('session-abc-123'); + + const connection = await connectToServer(url, {}, '2025-11-25'); + await connection.close(); + + expect(deletes).toEqual(['session-abc-123']); + }); + + it('does not send DELETE when the server assigned no session ID', async () => { + const { url, deletes } = await startServer(); + + const connection = await connectToServer(url, {}, '2025-11-25'); + await connection.close(); + + expect(deletes).toEqual([]); + }); +}); diff --git a/src/connection/sdk-client.ts b/src/connection/sdk-client.ts index d0080504..0d7575db 100644 --- a/src/connection/sdk-client.ts +++ b/src/connection/sdk-client.ts @@ -121,6 +121,11 @@ function instrumentTransport( * conformant `initialize`. The transport is instrumented so every wire * message in both directions is validated against `specVersion`'s spec * JSON schema; scenarios that call this directly pass `ctx.specVersion`. + * + * The returned `close()` sends an HTTP DELETE to terminate the server-side + * session (per the Streamable HTTP transport spec) before closing the client, + * so each scenario leaves the server hermetic. A server that responds 405 + * (DELETE not supported) is tolerated, since the spec allows that. */ export async function connectToServer( serverUrl: string, @@ -139,11 +144,60 @@ export async function connectToServer( return { client, close: async () => { + await terminateSessionBestEffort(transport, serverUrl); await client.close(); } }; } +// Shared teardown bound so a wedged server-under-test cannot stall the suite. +const SESSION_TERMINATE_TIMEOUT_MS = 5000; + +/** + * Best-effort session termination for an SDK transport. Delegates to + * terminateSessionRaw (with the transport's negotiated protocol version) so + * the DELETE is truly bounded — the socket is aborted at the timeout rather + * than left pending against a wedged server. No-op when the server never + * issued a session. + */ +export async function terminateSessionBestEffort( + transport: StreamableHTTPClientTransport, + serverUrl: string +): Promise { + if (!transport.sessionId || !transport.protocolVersion) return; + await terminateSessionRaw( + serverUrl, + transport.sessionId, + transport.protocolVersion + ); +} + +/** + * Best-effort HTTP DELETE to terminate a raw (non-SDK) session. + * + * Used by scenarios that open sessions via raw `fetch` instead of going through + * the SDK's StreamableHTTPClientTransport. Failures are swallowed because the + * spec allows servers to respond 405, and cleanup must not derail the scenario. + */ +export async function terminateSessionRaw( + serverUrl: string, + sessionId: string, + protocolVersion: string +): Promise { + try { + await fetch(serverUrl, { + method: 'DELETE', + headers: { + 'mcp-session-id': sessionId, + 'MCP-Protocol-Version': protocolVersion + }, + signal: AbortSignal.timeout(SESSION_TERMINATE_TIMEOUT_MS) + }); + } catch { + // best-effort cleanup + } +} + /** * Helper to collect notifications (logging and progress) */ diff --git a/src/scenarios/server/lifecycle.test.ts b/src/scenarios/server/lifecycle.test.ts index 899d63b8..ab3628a9 100644 --- a/src/scenarios/server/lifecycle.test.ts +++ b/src/scenarios/server/lifecycle.test.ts @@ -2,9 +2,14 @@ import { testContext } from '../../connection/testing'; import { ServerInitializeScenario } from './lifecycle'; import { connectToServer } from '../../connection/sdk-client'; -vi.mock('../../connection/sdk-client', () => ({ - connectToServer: vi.fn() -})); +vi.mock('../../connection/sdk-client', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + connectToServer: vi.fn() + }; +}); describe('ServerInitializeScenario', () => { const serverUrl = 'http://localhost:3000/mcp'; @@ -102,4 +107,37 @@ describe('ServerInitializeScenario', () => { } }); }); + + it('sends an HTTP DELETE with the issued session ID to terminate the raw session', async () => { + fetchMock.mockResolvedValue( + new Response(null, { + headers: { + 'mcp-session-id': 'session-123_ABC' + } + }) + ); + + await new ServerInitializeScenario().run(testContext(serverUrl)); + + expect(fetchMock).toHaveBeenCalledWith( + serverUrl, + expect.objectContaining({ + method: 'DELETE', + headers: expect.objectContaining({ + 'mcp-session-id': 'session-123_ABC' + }) + }) + ); + }); + + it('does not send DELETE when the server did not assign a session ID', async () => { + fetchMock.mockResolvedValue(new Response(null)); + + await new ServerInitializeScenario().run(testContext(serverUrl)); + + const deleteCalls = fetchMock.mock.calls.filter( + ([, init]) => (init as RequestInit | undefined)?.method === 'DELETE' + ); + expect(deleteCalls).toHaveLength(0); + }); }); diff --git a/src/scenarios/server/lifecycle.ts b/src/scenarios/server/lifecycle.ts index 2e2e0c9f..7be721b1 100644 --- a/src/scenarios/server/lifecycle.ts +++ b/src/scenarios/server/lifecycle.ts @@ -8,7 +8,10 @@ import { DRAFT_PROTOCOL_VERSION } from '../../types'; import type { RunContext } from '../../connection'; -import { connectToServer } from '../../connection/sdk-client'; +import { + connectToServer, + terminateSessionRaw +} from '../../connection/sdk-client'; const VISIBLE_ASCII_REGEX = /^[\x21-\x7E]+$/; @@ -91,20 +94,21 @@ and validates session ID format if one is assigned.`; // Check: Session ID visible ASCII validation // Use a raw fetch to inspect the MCP-Session-Id response header, // since the SDK client transport does not expose it. + let rawSessionId: string | null = null; try { const response = await fetch(serverUrl, { method: 'POST', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/event-stream', - 'mcp-protocol-version': '2025-11-25' + 'mcp-protocol-version': ctx.specVersion }, body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'initialize', params: { - protocolVersion: '2025-11-25', + protocolVersion: ctx.specVersion, capabilities: {}, clientInfo: { name: 'conformance-session-id-test', @@ -115,6 +119,7 @@ and validates session ID format if one is assigned.`; }); const sessionId = response.headers.get('mcp-session-id'); + rawSessionId = sessionId; if (!sessionId) { checks.push({ @@ -170,6 +175,10 @@ and validates session ID format if one is assigned.`; errorMessage: `Failed to send initialize request for session ID check: ${error instanceof Error ? error.message : String(error)}`, specReferences: SESSION_SPEC_REFERENCES }); + } finally { + if (rawSessionId) { + await terminateSessionRaw(serverUrl, rawSessionId, ctx.specVersion); + } } return checks; diff --git a/src/scenarios/server/sse-multiple-streams.ts b/src/scenarios/server/sse-multiple-streams.ts index d2eb4931..91bd24ca 100644 --- a/src/scenarios/server/sse-multiple-streams.ts +++ b/src/scenarios/server/sse-multiple-streams.ts @@ -18,6 +18,7 @@ import { buildStandardHeaders, type RunContext } from '../../connection'; import { EventSourceParserStream } from 'eventsource-parser/stream'; import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; +import { terminateSessionBestEffort } from '../../connection/sdk-client'; export class ServerSSEMultipleStreamsScenario implements ClientScenario { name = 'server-sse-multiple-streams'; @@ -293,7 +294,11 @@ export class ServerSSEMultipleStreamsScenario implements ClientScenario { ] }); } finally { - // Clean up + // Clean up: terminate the session before closing the client so the + // server is left hermetic. + if (transport) { + await terminateSessionBestEffort(transport, serverUrl); + } if (client) { try { await client.close(); diff --git a/src/scenarios/server/sse-polling.ts b/src/scenarios/server/sse-polling.ts index a15d5e98..280fe791 100644 --- a/src/scenarios/server/sse-polling.ts +++ b/src/scenarios/server/sse-polling.ts @@ -17,6 +17,7 @@ import type { RunContext } from '../../connection'; import { EventSourceParserStream } from 'eventsource-parser/stream'; import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; +import { terminateSessionBestEffort } from '../../connection/sdk-client'; function createLoggingFetch(checks: ConformanceCheck[]) { return async (url: string, options: RequestInit): Promise => { @@ -598,7 +599,11 @@ export class ServerSSEPollingScenario implements ClientScenario { ] }); } finally { - // Clean up + // Clean up: terminate the session before closing the client so the + // server is left hermetic. + if (transport) { + await terminateSessionBestEffort(transport, serverUrl); + } if (client) { try { await client.close();