Skip to content

Commit 5d11cba

Browse files
committed
fix(core): preserve related request id zero for debounce
1 parent 16d13ab commit 5d11cba

3 files changed

Lines changed: 24 additions & 1 deletion

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@modelcontextprotocol/core': patch
3+
---
4+
5+
Preserve `relatedRequestId: 0` when deciding whether notifications can be debounced. Request id `0` is valid, so request-associated notifications with that id now bypass debounce like other related notifications.

packages/core/src/shared/protocol.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1010,7 +1010,10 @@ export abstract class Protocol<ContextT extends BaseContext> {
10101010
// A notification can only be debounced if it's in the list AND it's "simple"
10111011
// (i.e., has no parameters and no related request ID or related task that could be lost).
10121012
const canDebounce =
1013-
debouncedMethods.includes(notification.method) && !notification.params && !options?.relatedRequestId && !options?.relatedTask;
1013+
debouncedMethods.includes(notification.method) &&
1014+
!notification.params &&
1015+
options?.relatedRequestId === undefined &&
1016+
!options?.relatedTask;
10141017

10151018
if (canDebounce) {
10161019
// If a notification of this type is already scheduled, do nothing.

packages/core/test/shared/protocol.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,21 @@ describe('protocol tests', () => {
768768
expect(sendSpy).toHaveBeenCalledWith(expect.any(Object), { relatedRequestId: 'req-2' });
769769
});
770770

771+
it('should NOT debounce a notification that has relatedRequestId 0', async () => {
772+
// ARRANGE
773+
protocol = new TestProtocolImpl({ debouncedNotificationMethods: ['test/debounced_with_options'] });
774+
await protocol.connect(transport);
775+
776+
// ACT
777+
const firstNotification = protocol.notification({ method: 'test/debounced_with_options' }, { relatedRequestId: 0 });
778+
const secondNotification = protocol.notification({ method: 'test/debounced_with_options' }, { relatedRequestId: 0 });
779+
await Promise.all([firstNotification, secondNotification]);
780+
781+
// ASSERT
782+
expect(sendSpy).toHaveBeenCalledTimes(2);
783+
expect(sendSpy).toHaveBeenCalledWith(expect.any(Object), { relatedRequestId: 0 });
784+
});
785+
771786
it('should clear pending debounced notifications on connection close', async () => {
772787
// ARRANGE
773788
protocol = new TestProtocolImpl({ debouncedNotificationMethods: ['test/debounced'] });

0 commit comments

Comments
 (0)