From 48e51e38373625e9921d018571c3be0be9e90674 Mon Sep 17 00:00:00 2001 From: Satoshi Ito Date: Thu, 21 May 2026 10:37:08 +0000 Subject: [PATCH 1/2] fix: send HTTP DELETE to terminate server sessions after each scenario The conformance test harness opened sessions against servers under test but never issued an HTTP DELETE to terminate them, leaving a dangling session on the server after each scenario. This violates the "hermetic" expectation called out in the Streamable HTTP transport spec. - `connection/sdk-client.connectToServer` now calls `transport.terminateSession()` before `client.close()`. A 405 response (DELETE not supported) is tolerated because the spec allows the server to refuse. - A new `terminateSessionRaw` helper handles scenarios that bypass the SDK client and open sessions via raw `fetch` (`lifecycle.ts`, `http-standard-headers.ts`). - `sse-polling.ts` and `sse-multiple-streams.ts`, which manage their own transport, now terminate the session in their existing `finally` block before closing the client. - `lifecycle.test.ts` gains two cases asserting that DELETE is sent with the issued session ID and that no DELETE is sent when the server did not assign one. Verified manually against `examples/servers/typescript/everything-server.ts` that running a server scenario now causes the server to log `Received session termination request for session `. Fixes #79 Signed-off-by: Satoshi Ito --- src/connection/sdk-client.ts | 39 ++++++++++++++++- src/scenarios/server/lifecycle.test.ts | 44 ++++++++++++++++++-- src/scenarios/server/lifecycle.ts | 11 ++++- src/scenarios/server/sse-multiple-streams.ts | 10 ++++- src/scenarios/server/sse-polling.ts | 10 ++++- 5 files changed, 107 insertions(+), 7 deletions(-) diff --git a/src/connection/sdk-client.ts b/src/connection/sdk-client.ts index eebbd9b5..7c124b2b 100644 --- a/src/connection/sdk-client.ts +++ b/src/connection/sdk-client.ts @@ -15,7 +15,12 @@ export interface MCPClientConnection { } /** - * Create and connect an MCP client to a server + * Create and connect an MCP client to a server. + * + * 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 @@ -41,11 +46,43 @@ export async function connectToServer( return { client, close: async () => { + if (transport.sessionId) { + try { + await transport.terminateSession(); + } catch { + // The spec allows the server to respond 405 to a DELETE; best-effort. + } + } await client.close(); } }; } +/** + * 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 + } + }); + } 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 d787fe82..5cc63609 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'; @@ -98,4 +103,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 61fac0b4..6ca094bf 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,6 +94,7 @@ 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', @@ -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, '2025-11-25'); + } } return checks; diff --git a/src/scenarios/server/sse-multiple-streams.ts b/src/scenarios/server/sse-multiple-streams.ts index 23f7e797..7a76a33c 100644 --- a/src/scenarios/server/sse-multiple-streams.ts +++ b/src/scenarios/server/sse-multiple-streams.ts @@ -291,7 +291,15 @@ 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 && sessionId) { + try { + await transport.terminateSession(); + } catch { + // Server MAY respond 405 to DELETE; best-effort. + } + } if (client) { try { await client.close(); diff --git a/src/scenarios/server/sse-polling.ts b/src/scenarios/server/sse-polling.ts index 0f52f406..bb8dc77b 100644 --- a/src/scenarios/server/sse-polling.ts +++ b/src/scenarios/server/sse-polling.ts @@ -596,7 +596,15 @@ 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 && sessionId) { + try { + await transport.terminateSession(); + } catch { + // Server MAY respond 405 to DELETE; best-effort. + } + } if (client) { try { await client.close(); From 363088f4eafb7575edff1926c5aa840fe8039453 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Fri, 31 Jul 2026 16:47:29 +0100 Subject: [PATCH 2/2] Use negotiated protocol version for teardown DELETE and bound it Review follow-ups on the session-termination change: - terminateSessionRaw: abort the DELETE after 5s so a wedged server cannot stall suite teardown. - terminateSessionBestEffort: shared SDK-path teardown helper that delegates to terminateSessionRaw with the transport's negotiated protocol version, so the socket is truly released at the bound and the version-mismatch 400 on strict servers (the failure mode of issue #79) goes away. Used by connectToServer close() and both SSE scenarios, replacing their inline try/catch blocks. - lifecycle.ts: the raw session-id probe and its teardown DELETE now use ctx.specVersion instead of a hardcoded 2025-11-25, matching the negotiated-version convention from #415/#416. - Add sdk-client.test.ts regression tests asserting close() sends a DELETE with the issued session ID (and none without one), so a bad future merge cannot silently drop the fix. --- src/connection/sdk-client.ts | 38 +++++++++++++++----- src/scenarios/server/lifecycle.ts | 6 ++-- src/scenarios/server/sse-multiple-streams.ts | 9 ++--- src/scenarios/server/sse-polling.ts | 9 ++--- 4 files changed, 39 insertions(+), 23 deletions(-) diff --git a/src/connection/sdk-client.ts b/src/connection/sdk-client.ts index ada402e2..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,18 +144,34 @@ export async function connectToServer( return { client, close: async () => { - if (transport.sessionId) { - try { - await transport.terminateSession(); - } catch { - // The spec allows the server to respond 405 to a DELETE; best-effort. - } - } + 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. * @@ -169,7 +190,8 @@ export async function terminateSessionRaw( headers: { 'mcp-session-id': sessionId, 'MCP-Protocol-Version': protocolVersion - } + }, + signal: AbortSignal.timeout(SESSION_TERMINATE_TIMEOUT_MS) }); } catch { // best-effort cleanup diff --git a/src/scenarios/server/lifecycle.ts b/src/scenarios/server/lifecycle.ts index b1e6bad5..7be721b1 100644 --- a/src/scenarios/server/lifecycle.ts +++ b/src/scenarios/server/lifecycle.ts @@ -101,14 +101,14 @@ and validates session ID format if one is assigned.`; 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', @@ -177,7 +177,7 @@ and validates session ID format if one is assigned.`; }); } finally { if (rawSessionId) { - await terminateSessionRaw(serverUrl, rawSessionId, '2025-11-25'); + await terminateSessionRaw(serverUrl, rawSessionId, ctx.specVersion); } } diff --git a/src/scenarios/server/sse-multiple-streams.ts b/src/scenarios/server/sse-multiple-streams.ts index 50c778cb..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'; @@ -295,12 +296,8 @@ export class ServerSSEMultipleStreamsScenario implements ClientScenario { } finally { // Clean up: terminate the session before closing the client so the // server is left hermetic. - if (transport && sessionId) { - try { - await transport.terminateSession(); - } catch { - // Server MAY respond 405 to DELETE; best-effort. - } + if (transport) { + await terminateSessionBestEffort(transport, serverUrl); } if (client) { try { diff --git a/src/scenarios/server/sse-polling.ts b/src/scenarios/server/sse-polling.ts index 46bd7495..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 => { @@ -600,12 +601,8 @@ export class ServerSSEPollingScenario implements ClientScenario { } finally { // Clean up: terminate the session before closing the client so the // server is left hermetic. - if (transport && sessionId) { - try { - await transport.terminateSession(); - } catch { - // Server MAY respond 405 to DELETE; best-effort. - } + if (transport) { + await terminateSessionBestEffort(transport, serverUrl); } if (client) { try {