fix: handle cancellation for request ID 0

.changeset/cancelled-request-id-zero.md

@@ -0,0 +1,12 @@
+---
+'@modelcontextprotocol/core-internal': patch
+'@modelcontextprotocol/client': patch
+'@modelcontextprotocol/server': patch
+---
+
+Treat request id `0` as a real id. Two guards tested a `RequestId` for truthiness, so the legal JSON-RPC ids `0` and `''` were read as absent. Id `0` is not a corner case: the outbound request counter is zero-based, so it is the first id every peer assigns, which on the server→client leg is the first `sampling/createMessage`, `elicitation/create`, or `roots/list` a server sends.
+
+- `notifications/cancelled` carrying id `0` was ignored, and the in-flight handler ran to completion with its `AbortSignal` never fired.
+- A notification sent with `relatedRequestId: 0` wrongly passed the debounce gate (for methods opted into `debouncedNotificationMethods`). Because the pending set is keyed by method alone, a second such notification in the same tick was silently dropped rather than sent.
+
+Absent is now the only value that means "no id".

packages/core-internal/src/shared/protocol.ts

@@ -721,13 +721,15 @@
      */
     protected _getRequestHandler(method: string): ((request: JSONRPCRequest, ctx: ContextT) => Promise<Result>) | undefined {
         return this._requestHandlers.get(method);
     }
 
     private async _oncancel(notification: CancelledNotification): Promise<void> {
-        if (!notification.params.requestId) {
+        // `requestId` is optional on the 2025-era wire schema. Absent is the
+        // only thing that means "no id": `0` and `''` are legal request ids.
+        if (notification.params.requestId === undefined) {
             return;
         }
         // Handle request cancellation
         const controller = this._requestHandlerAbortControllers.get(notification.params.requestId);
         controller?.abort(notification.params.reason);
     }
@@ -1611,7 +1613,11 @@
         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;
+        // Absent is the only thing that means "no id" here too: `0` and `''` are
+        // legal request ids, and the pending set is keyed by method alone, so
+        // treating them as absent lets a related notification be coalesced away.
+        const canDebounce =
+            debouncedMethods.includes(notification.method) && !notification.params && options?.relatedRequestId === undefined;
 
         if (canDebounce) {
             // If a notification of this type is already scheduled, do nothing.

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

@@ -656,6 +656,27 @@ describe('protocol tests', () => {
             expect(sendSpy).toHaveBeenCalledWith(expect.any(Object), { relatedRequestId: 'req-2' });
         });
 
+        // Same-tick coverage for every legal request id: the pending set is keyed
+        // by method alone, so a related notification that wrongly passes the
+        // debounce gate is coalesced away entirely. Awaiting between the two
+        // sends would flush the microtask and hide that, so they are fired in one
+        // tick. `0` and `''` are the ids a truthiness guard swallows.
+        test.each([0, 123, '', 'req-1'])('should NOT coalesce same-tick notifications related to requestId %j', async relatedRequestId => {
+            // ARRANGE
+            protocol = new TestProtocolImpl({ debouncedNotificationMethods: ['test/debounced_with_options'] });
+            await protocol.connect(transport);
+
+            // ACT — two related notifications in the same tick, no await between
+            void protocol.notification({ method: 'test/debounced_with_options' }, { relatedRequestId });
+            void protocol.notification({ method: 'test/debounced_with_options' }, { relatedRequestId });
+            await flushMicrotasks();
+
+            // ASSERT — both go out, each still carrying its related request id
+            expect(sendSpy).toHaveBeenCalledTimes(2);
+            expect(sendSpy).toHaveBeenNthCalledWith(1, expect.any(Object), { relatedRequestId });
+            expect(sendSpy).toHaveBeenNthCalledWith(2, expect.any(Object), { relatedRequestId });
+        });
+
         it('should clear pending debounced notifications on connection close', async () => {
             // ARRANGE
             protocol = new TestProtocolImpl({ debouncedNotificationMethods: ['test/debounced'] });
@@ -775,50 +796,55 @@ describe('protocol tests', () => {
     });
 
     describe('notifications/cancelled behavior', () => {
-        test('should abort request handler when notifications/cancelled is received', async () => {
-            await protocol.connect(transport);
-
-            // Set up a request handler that checks if it was aborted
-            let wasAborted = false;
-            protocol.setRequestHandler('ping', async (_request, ctx) => {
-                // Simulate a long-running operation
-                await new Promise(resolve => setTimeout(resolve, 100));
-                wasAborted = ctx.mcpReq.signal.aborted;
-                return {};
-            });
-
-            // Simulate an incoming request
-            const requestId = 123;
-            if (transport.onmessage) {
-                transport.onmessage({
-                    jsonrpc: '2.0',
-                    id: requestId,
-                    method: 'ping',
-                    params: {}
+        // Every legal JSON-RPC request id must cancel, including the ones a
+        // truthiness guard swallows: `0` (the first id every peer assigns,
+        // since the counter is zero-based) and the empty string.
+        test.each([0, 123, '', 'req-1'])(
+            'should abort request handler when notifications/cancelled carries requestId %j',
+            async requestId => {
+                await protocol.connect(transport);
+
+                // Set up a request handler that checks if it was aborted
+                let wasAborted = false;
+                protocol.setRequestHandler('ping', async (_request, ctx) => {
+                    // Simulate a long-running operation
+                    await new Promise(resolve => setTimeout(resolve, 100));
+                    wasAborted = ctx.mcpReq.signal.aborted;
+                    return {};
                 });
-            }
 
-            // Wait a bit for the handler to start
-            await new Promise(resolve => setTimeout(resolve, 10));
+                // Simulate an incoming request
+                if (transport.onmessage) {
+                    transport.onmessage({
+                        jsonrpc: '2.0',
+                        id: requestId,
+                        method: 'ping',
+                        params: {}
+                    });
+                }
+
+                // Wait a bit for the handler to start
+                await new Promise(resolve => setTimeout(resolve, 10));
 
-            // Send cancellation notification
-            if (transport.onmessage) {
-                transport.onmessage({
-                    jsonrpc: '2.0',
-                    method: 'notifications/cancelled',
-                    params: {
-                        requestId: requestId,
-                        reason: 'User cancelled'
-                    }
-                });
-            }
+                // Send cancellation notification
+                if (transport.onmessage) {
+                    transport.onmessage({
+                        jsonrpc: '2.0',
+                        method: 'notifications/cancelled',
+                        params: {
+                            requestId: requestId,
+                            reason: 'User cancelled'
+                        }
+                    });
+                }
 
-            // Wait for the handler to complete
-            await new Promise(resolve => setTimeout(resolve, 150));
+                // Wait for the handler to complete
+                await new Promise(resolve => setTimeout(resolve, 150));
 
-            // Verify the request was aborted
-            expect(wasAborted).toBe(true);
-        });
+                // Verify the request was aborted
+                expect(wasAborted).toBe(true);
+            }
+        );
     });
 
     // Spec basic/patterns/cancellation §Transport-Specific (2026-07-28): on a