Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-falsy-zero-request-id.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@modelcontextprotocol/core-internal": patch
---

Fix falsy-zero requestId bugs in debounce guard and cancellation handling
5 changes: 3 additions & 2 deletions packages/core-internal/src/shared/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ export abstract class Protocol<ContextT extends BaseContext> {
}

private async _oncancel(notification: CancelledNotification): Promise<void> {
if (!notification.params.requestId) {
if (notification.params.requestId === undefined || notification.params.requestId === null) {
return;
}
// Handle request cancellation
Expand Down Expand Up @@ -1611,7 +1611,8 @@ export abstract class Protocol<ContextT extends BaseContext> {
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.
Expand Down
59 changes: 59 additions & 0 deletions packages/core-internal/test/shared/protocol.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'] });
Expand Down Expand Up @@ -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
Expand Down
Loading