From 14d6eeb45e8c3400c7170f8554d4df31c4283607 Mon Sep 17 00:00:00 2001 From: dumbCodesOnly Date: Wed, 15 Jul 2026 21:35:21 +0330 Subject: [PATCH 1/2] Fix falsy-zero requestId bugs (debounce guard + cancellation) - _notificationViaCodec: canDebounce used `!options?.relatedRequestId`, which treats relatedRequestId === 0 the same as undefined. A notification tied to request 0 could be incorrectly coalesced with unrelated notifications of the same method. - _oncancel: `if (!notification.params.requestId)` dropped notifications/cancelled for requestId 0, so the abort controller for request 0 never fired. Request IDs start at 0 (_requestMessageId = 0), so both are reachable in practice, not just theoretical edge cases. Fixes #2117, #2283 --- packages/core-internal/src/shared/protocol.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/core-internal/src/shared/protocol.ts b/packages/core-internal/src/shared/protocol.ts index ffab6e8df8..92842f9afd 100644 --- a/packages/core-internal/src/shared/protocol.ts +++ b/packages/core-internal/src/shared/protocol.ts @@ -723,7 +723,7 @@ export abstract class Protocol { } private async _oncancel(notification: CancelledNotification): Promise { - if (!notification.params.requestId) { + if (notification.params.requestId === undefined || notification.params.requestId === null) { return; } // Handle request cancellation @@ -1610,7 +1610,8 @@ export abstract class Protocol { const debouncedMethods = this._options?.debouncedNotificationMethods ?? []; // A notification can only be debounced if it's in the list AND it's "simple" // (i.e., has no parameters and no related request ID that could be lost). - const canDebounce = debouncedMethods.includes(notification.method) && !notification.params && !options?.relatedRequestId; + const canDebounce = + debouncedMethods.includes(notification.method) && !notification.params && options?.relatedRequestId === undefined; if (canDebounce) { // If a notification of this type is already scheduled, do nothing. From bfa0fb2d8933797b6f47a78d583940080bfd09cb Mon Sep 17 00:00:00 2001 From: dumbCodesOnly Date: Wed, 15 Jul 2026 21:36:04 +0330 Subject: [PATCH 2/2] Add regression tests for falsy-zero requestId bugs - relatedRequestId: 0 must never be debounce-coalesced (issue #2117) - notifications/cancelled with requestId: 0 must abort the correct request's controller (issue #2283) --- .../test/shared/protocol.test.ts | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/packages/core-internal/test/shared/protocol.test.ts b/packages/core-internal/test/shared/protocol.test.ts index 2ecdc40adc..ea98427960 100644 --- a/packages/core-internal/test/shared/protocol.test.ts +++ b/packages/core-internal/test/shared/protocol.test.ts @@ -656,6 +656,24 @@ describe('protocol tests', () => { expect(sendSpy).toHaveBeenCalledWith(expect.any(Object), { relatedRequestId: 'req-2' }); }); + it('should NOT debounce a notification with relatedRequestId 0 (falsy but valid)', async () => { + // ARRANGE + protocol = new TestProtocolImpl({ debouncedNotificationMethods: ['test/debounced_with_options'] }); + await protocol.connect(transport); + + // ACT + // relatedRequestId: 0 is a valid, real request id (ids start at 0) — it + // must not be treated the same as "no relatedRequestId" just because + // 0 is falsy in JS. + await protocol.notification({ method: 'test/debounced_with_options' }, { relatedRequestId: 0 }); + await protocol.notification({ method: 'test/debounced_with_options' }, { relatedRequestId: 0 }); + + // ASSERT + // Both sends must go out immediately/individually, never coalesced. + expect(sendSpy).toHaveBeenCalledTimes(2); + expect(sendSpy).toHaveBeenCalledWith(expect.any(Object), { relatedRequestId: 0 }); + }); + it('should clear pending debounced notifications on connection close', async () => { // ARRANGE protocol = new TestProtocolImpl({ debouncedNotificationMethods: ['test/debounced'] }); @@ -819,6 +837,47 @@ describe('protocol tests', () => { // Verify the request was aborted expect(wasAborted).toBe(true); }); + + test('should abort request handler when notifications/cancelled is received for requestId 0', async () => { + await protocol.connect(transport); + + // Request id 0 is the first id ever issued (_requestMessageId starts + // at 0) — a falsy-but-valid id that must not be silently dropped by + // the cancellation handler. + let wasAborted = false; + protocol.setRequestHandler('ping', async (_request, ctx) => { + await new Promise(resolve => setTimeout(resolve, 100)); + wasAborted = ctx.mcpReq.signal.aborted; + return {}; + }); + + const requestId = 0; + if (transport.onmessage) { + transport.onmessage({ + jsonrpc: '2.0', + id: requestId, + method: 'ping', + params: {} + }); + } + + await new Promise(resolve => setTimeout(resolve, 10)); + + if (transport.onmessage) { + transport.onmessage({ + jsonrpc: '2.0', + method: 'notifications/cancelled', + params: { + requestId: requestId, + reason: 'User cancelled' + } + }); + } + + await new Promise(resolve => setTimeout(resolve, 150)); + + expect(wasAborted).toBe(true); + }); }); // Spec basic/patterns/cancellation §Transport-Specific (2026-07-28): on a