From ff550a838cda9ee76656dfd5df2ef63fbda57907 Mon Sep 17 00:00:00 2001 From: Steeve Amonles Date: Wed, 14 Oct 2026 10:16:22 -0400 Subject: [PATCH 1/2] fix: deliver progress notifications that share a read with their response _onnotification deferred handlers via Promise.resolve(), while _onresponse runs synchronously and deletes the progress handler. When both messages arrive in one turn, the response won the race and the progress was dropped. Dispatch notification handlers synchronously (async rejections still go to _onerror) and add a regression test for the same-turn case. Fixes #3018 --- packages/core-internal/src/shared/protocol.ts | 16 +++++-- .../test/shared/protocol.test.ts | 42 +++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/packages/core-internal/src/shared/protocol.ts b/packages/core-internal/src/shared/protocol.ts index 9d60be8efe..c0edd73222 100644 --- a/packages/core-internal/src/shared/protocol.ts +++ b/packages/core-internal/src/shared/protocol.ts @@ -929,10 +929,18 @@ export abstract class Protocol { return; } - // Starting with Promise.resolve() puts any synchronous errors into the monad as well. - Promise.resolve() - .then(() => (handler === undefined ? fallback!(notification) : handler(notification, codec))) - .catch(error => this._onerror(new Error(`Uncaught error in notification handler: ${error}`))); + // Dispatch synchronously so a notification that shares a read with + // its request's response is not deferred past _onresponse, which + // deletes the matching progress handler (see #3018). Async handler + // rejections and synchronous throws still route to _onerror. + try { + const result = handler === undefined ? fallback!(notification) : handler(notification, codec); + if (result !== undefined && typeof (result as PromiseLike).then === 'function') { + Promise.resolve(result).catch(error => this._onerror(new Error(`Uncaught error in notification handler: ${error}`))); + } + } catch (error) { + this._onerror(new Error(`Uncaught error in notification handler: ${error}`)); + } } private _onrequest(rawRequest: JSONRPCRequest, extra?: MessageExtraInfo): void { diff --git a/packages/core-internal/test/shared/protocol.test.ts b/packages/core-internal/test/shared/protocol.test.ts index 2fb0f64813..fcb1923ffc 100644 --- a/packages/core-internal/test/shared/protocol.test.ts +++ b/packages/core-internal/test/shared/protocol.test.ts @@ -617,6 +617,48 @@ describe('protocol tests', () => { await Promise.resolve(); await expect(requestPromise).resolves.toEqual({ result: 'success' }); }); + + test('should deliver progress when notification and response arrive in the same turn', async () => { + // Repro for #3018: progress was deferred via Promise.resolve(), so + // a same-turn response deleted the progress handler first. + await protocol.connect(transport); + const request = { method: 'example', params: {} }; + const mockSchema: ZodType<{ result: string }> = z.object({ + result: z.string() + }); + const onProgressMock = vi.fn(); + const onErrorMock = vi.fn(); + protocol.onerror = onErrorMock; + + const requestPromise = testRequest(protocol, request, mockSchema, { + timeout: 1000, + onprogress: onProgressMock + }); + + if (transport.onmessage) { + transport.onmessage({ + jsonrpc: '2.0', + method: 'notifications/progress', + params: { + progressToken: 0, + progress: 50, + total: 100 + } + }); + transport.onmessage({ + jsonrpc: '2.0', + id: 0, + result: { result: 'success' } + }); + } + + await expect(requestPromise).resolves.toEqual({ result: 'success' }); + expect(onProgressMock).toHaveBeenCalledWith({ + progress: 50, + total: 100 + }); + expect(onErrorMock).not.toHaveBeenCalled(); + }); }); describe('Debounced Notifications', () => { From 9af4566b1010d1496cd8c3633e622bc27d0cc7d1 Mon Sep 17 00:00:00 2001 From: Steeve-Crypto Date: Wed, 14 Oct 2026 10:43:59 -0400 Subject: [PATCH 2/2] test(e2e): drop SSE knownFailures for fixed progress delivery The sync notification dispatch in #3023 delivers progress that shares a read with its response, so the SSE progress scenarios should pass. --- test/e2e/requirements.ts | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/test/e2e/requirements.ts b/test/e2e/requirements.ts index 36da4fc523..13b4e47fb4 100644 --- a/test/e2e/requirements.ts +++ b/test/e2e/requirements.ts @@ -214,23 +214,11 @@ export const REQUIREMENTS: Record = { 'protocol:progress:callback': { source: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/utilities/progress#progress-flow', behavior: - "Progress notifications emitted by a handler during a request are delivered to the caller's progress callback, in order, with their progress, total, and message.", - knownFailures: [ - { - transport: 'sse', - note: "Real-socket SSE delivers a handler's progress notifications and its response in one batch; the response is processed first, so the progress notifications never reach the caller's progress callback." - } - ] + "Progress notifications emitted by a handler during a request are delivered to the caller's progress callback, in order, with their progress, total, and message." }, 'typescript:protocol:progress:token-injected': { source: 'sdk', - behavior: 'Passing onprogress causes a progressToken to be injected into request _meta, preserving existing _meta fields.', - knownFailures: [ - { - transport: 'sse', - note: "Real-socket SSE delivers a handler's progress notifications and its response in one batch; the response is processed first, so the progress notifications never reach the caller's progress callback." - } - ] + behavior: 'Passing onprogress causes a progressToken to be injected into request _meta, preserving existing _meta fields.' }, 'protocol:progress:token-unique': { source: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/utilities/progress#progress-flow', @@ -246,13 +234,7 @@ export const REQUIREMENTS: Record = { }, 'protocol:timeout:reset-on-progress': { source: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/lifecycle#timeouts', - behavior: "When configured to do so, each progress notification resets the request's read timeout.", - knownFailures: [ - { - transport: 'sse', - note: 'Same real-socket SSE batching race as protocol:progress:callback: the progress notifications are dropped before they can reset the timeout, so the request times out.' - } - ] + behavior: "When configured to do so, each progress notification resets the request's read timeout." }, 'protocol:timeout:sends-cancellation': { source: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/lifecycle#timeouts', @@ -391,13 +373,7 @@ export const REQUIREMENTS: Record = { }, 'tools:call:progress': { source: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/utilities/progress#progress-flow', - behavior: "Progress notifications emitted by a tool handler reach the caller's progress callback before the tool result returns.", - knownFailures: [ - { - transport: 'sse', - note: "Real-socket SSE delivers a handler's progress notifications and its response in one batch; the response is processed first, so the progress notifications never reach the caller's progress callback." - } - ] + behavior: "Progress notifications emitted by a tool handler reach the caller's progress callback before the tool result returns." }, 'tools:call:sampling-roundtrip': { transports: STATEFUL_TRANSPORTS,