diff --git a/.changeset/related-request-id-zero-debounce.md b/.changeset/related-request-id-zero-debounce.md new file mode 100644 index 0000000000..e061430af1 --- /dev/null +++ b/.changeset/related-request-id-zero-debounce.md @@ -0,0 +1,6 @@ +--- +'@modelcontextprotocol/client': patch +'@modelcontextprotocol/server': patch +--- + +Fixed a bug where a notification's `relatedRequestId: 0` was treated as absent by the debounce guard in `Protocol.notification()`, because the check used truthiness instead of testing for presence. `0` is a valid JSON-RPC request id — and the first id a `Protocol` instance issues — so a notification tied to request `0` could be silently coalesced by the debounce path even though related notifications are supposed to bypass debouncing. diff --git a/packages/core-internal/src/shared/protocol.ts b/packages/core-internal/src/shared/protocol.ts index 0a19770082..892e3ccd52 100644 --- a/packages/core-internal/src/shared/protocol.ts +++ b/packages/core-internal/src/shared/protocol.ts @@ -1611,7 +1611,9 @@ 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; + // relatedRequestId is checked for presence, not truthiness, since 0 is a valid request id. + const hasRelatedRequestId = options?.relatedRequestId !== undefined; + const canDebounce = debouncedMethods.includes(notification.method) && !notification.params && !hasRelatedRequestId; if (canDebounce) { // If a notification of this type is already scheduled, do nothing. diff --git a/packages/core-internal/test/shared/protocol.test.ts b/packages/core-internal/test/shared/protocol.test.ts index 2ecdc40adc..dc41303531 100644 --- a/packages/core-internal/test/shared/protocol.test.ts +++ b/packages/core-internal/test/shared/protocol.test.ts @@ -656,6 +656,23 @@ describe('protocol tests', () => { expect(sendSpy).toHaveBeenCalledWith(expect.any(Object), { relatedRequestId: 'req-2' }); }); + it('should NOT debounce a notification with relatedRequestId 0', async () => { + // ARRANGE + // 0 is a valid request id and is also the first id a Protocol instance + // issues, so a truthiness check on relatedRequestId would wrongly treat + // it as absent and debounce the notification. + protocol = new TestProtocolImpl({ debouncedNotificationMethods: ['test/debounced_with_options'] }); + await protocol.connect(transport); + + // ACT + await protocol.notification({ method: 'test/debounced_with_options' }, { relatedRequestId: 0 }); + await protocol.notification({ method: 'test/debounced_with_options' }, { relatedRequestId: 0 }); + + // ASSERT + 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'] });