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', () => { 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,