From e8185e0216f50fe36f97e482b3ff8d689e28dc62 Mon Sep 17 00:00:00 2001 From: Axit Date: Thu, 30 Jul 2026 20:07:35 +0530 Subject: [PATCH 1/2] fix(server): fire onsessionclosed once when DELETEs overlap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backport of the main-branch fix (#2583) to v1.x, where the same window exists in `WebStandardStreamableHTTPServerTransport`. `handleDeleteRequest` awaited `onsessionclosed` before `close()` ran, so `_closed` was still false while the callback was in flight. A second DELETE for the same session passed `validateSession`/`validateProtocolVersion` unchanged — neither consults `_closed` — and invoked the callback again for a session already being torn down. Claim the notification synchronously before the await. `_closed` cannot serve this purpose: it is set by `close()`, which only runs once the callback has settled. DELETE stays idempotent — a concurrent or repeat request still terminates the session and answers 200, it just does not re-run the callback. The regression test drives `handleRequest()` directly rather than a real socket: over HTTP the callback release wins the race, the two DELETEs serialize, and the bug does not reproduce. Refs #2562 (item 2) Co-Authored-By: Claude Opus 5 --- .changeset/v1x-delete-race-onsessionclosed.md | 5 ++ src/server/webStandardStreamableHttp.ts | 14 +++++- test/server/streamableHttp.test.ts | 50 +++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 .changeset/v1x-delete-race-onsessionclosed.md diff --git a/.changeset/v1x-delete-race-onsessionclosed.md b/.changeset/v1x-delete-race-onsessionclosed.md new file mode 100644 index 0000000000..42ccee3a6b --- /dev/null +++ b/.changeset/v1x-delete-race-onsessionclosed.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/sdk': patch +--- + +Fire `onsessionclosed` at most once when DELETE requests for the same session overlap. The first DELETE awaited the callback before `close()` ran, so `_closed` was still `false` and a second DELETE passed the same guards and invoked the callback again for a session already being torn down. The notification is now claimed synchronously before the await. DELETE remains idempotent — a concurrent or repeat request still terminates the session and answers 200. diff --git a/src/server/webStandardStreamableHttp.ts b/src/server/webStandardStreamableHttp.ts index db83c7cf22..dae552750f 100644 --- a/src/server/webStandardStreamableHttp.ts +++ b/src/server/webStandardStreamableHttp.ts @@ -246,6 +246,12 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { private _retryInterval?: number; private _keepAliveMs: number; private _closed = false; + /** + * Set synchronously before `onsessionclosed` is awaited, so a DELETE that arrives while the + * callback is in flight does not fire it again. `_closed` cannot serve this purpose: it is set + * by `close()`, which only runs once the callback has already settled. + */ + private _sessionClosedNotified = false; sessionId?: string; onclose?: () => void; @@ -979,7 +985,13 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { } try { - await Promise.resolve(this._onsessionclosed?.(this.sessionId!)); + // Claim the notification before awaiting it — see `_sessionClosedNotified`. DELETE stays + // idempotent: a concurrent or repeat request still terminates the session and answers 200, + // it just does not re-run the callback. + if (!this._sessionClosedNotified) { + this._sessionClosedNotified = true; + await Promise.resolve(this._onsessionclosed?.(this.sessionId!)); + } return new Response(null, { status: 200 }); } finally { await this.close(); diff --git a/test/server/streamableHttp.test.ts b/test/server/streamableHttp.test.ts index 99a6952088..a3c54dfe57 100644 --- a/test/server/streamableHttp.test.ts +++ b/test/server/streamableHttp.test.ts @@ -3114,6 +3114,56 @@ async function createTestServerWithDnsProtection(config: { }; } +describe('WebStandardStreamableHTTPServerTransport - concurrent DELETE', () => { + it('fires onsessionclosed once when two DELETEs overlap', async () => { + // The first DELETE parks on the callback await. `_closed` is only set by close(), which runs + // in the finally *after* that await, and neither validateSession nor validateProtocolVersion + // consults it — so a second DELETE passes the same guards and fires the callback again for a + // session already being torn down. + // + // Driven through handleRequest() rather than a real socket so the second DELETE is known to + // have reached the handler before the callback is released; over HTTP the release wins the + // race and the two requests simply serialize, which hides the bug. + let releaseCallback!: () => void; + const callbackGate = new Promise(resolve => { + releaseCallback = resolve; + }); + const onsessionclosed = vi.fn().mockReturnValue(callbackGate); + + const mcpServer = new McpServer({ name: 'test-server', version: '1.0.0' }); + const transport = new WebStandardStreamableHTTPServerTransport({ + sessionIdGenerator: () => randomUUID(), + onsessionclosed + }); + await mcpServer.connect(transport); + + const initResponse = await transport.handleRequest( + new Request('http://localhost/mcp', { + method: 'POST', + headers: { Accept: 'application/json, text/event-stream', 'Content-Type': 'application/json' }, + body: JSON.stringify(TEST_MESSAGES.initialize) + }) + ); + const sessionId = initResponse.headers.get('mcp-session-id') as string; + expect(sessionId).toBeDefined(); + + const sendDelete = (): Promise => + transport.handleRequest( + new Request('http://localhost/mcp', { + method: 'DELETE', + headers: { 'mcp-session-id': sessionId, 'mcp-protocol-version': '2025-11-25' } + }) + ); + + const first = sendDelete(); + const second = sendDelete(); + releaseCallback(); + await Promise.all([first, second]); + + expect(onsessionclosed).toHaveBeenCalledTimes(1); + }); +}); + describe('WebStandardStreamableHTTPServerTransport - onerror callback', () => { let transport: WebStandardStreamableHTTPServerTransport; let mcpServer: McpServer; From 131237043dd50aeffe6e88c273741e2cb7a84712 Mon Sep 17 00:00:00 2001 From: Axit Date: Thu, 30 Jul 2026 22:09:19 +0530 Subject: [PATCH 2/2] chore: format changeset with prettier `prettier --check .` covers .changeset/*.md on this branch; the file was written by hand and never formatted, which failed the build job. Co-Authored-By: Claude Opus 5 --- .changeset/v1x-delete-race-onsessionclosed.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.changeset/v1x-delete-race-onsessionclosed.md b/.changeset/v1x-delete-race-onsessionclosed.md index 42ccee3a6b..712b5d482d 100644 --- a/.changeset/v1x-delete-race-onsessionclosed.md +++ b/.changeset/v1x-delete-race-onsessionclosed.md @@ -2,4 +2,5 @@ '@modelcontextprotocol/sdk': patch --- -Fire `onsessionclosed` at most once when DELETE requests for the same session overlap. The first DELETE awaited the callback before `close()` ran, so `_closed` was still `false` and a second DELETE passed the same guards and invoked the callback again for a session already being torn down. The notification is now claimed synchronously before the await. DELETE remains idempotent — a concurrent or repeat request still terminates the session and answers 200. +Fire `onsessionclosed` at most once when DELETE requests for the same session overlap. The first DELETE awaited the callback before `close()` ran, so `_closed` was still `false` and a second DELETE passed the same guards and invoked the callback again for a session already being +torn down. The notification is now claimed synchronously before the await. DELETE remains idempotent — a concurrent or repeat request still terminates the session and answers 200.