modelcontextprotocol/typescript-sdk
Watch
1
0
Fork
You've already forked typescript-sdk
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix: handle cancellation for request ID 0 (#2654)

Browse source 
Co-authored-by: Konstantin Konstantinov <konstantin@mach5technology.com>
Co-authored-by: Konstantin Konstantinov <KKonstantinov@users.noreply.github.com>
This commit is contained in:
pshah19 2026-08-16 02:17:04 -07:00 • committed by GitHub
commit 03842cd9ca
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 85 additions and 41 deletions
Download patch file Download diff file Expand all files Collapse all files

12
.changeset/cancelled-request-id-zero.md Normal file
View file

@ -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".

10
packages/core-internal/src/shared/protocol.ts
View file

@ -724,7 +724,9 @@ export abstract class Protocol<ContextT extends BaseContext> {
}
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
@ -1611,7 +1613,11 @@ 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;
// 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.

108
packages/core-internal/test/shared/protocol.test.ts
View file

@ -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);
// 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 {};
});
// Simulate an incoming request
const requestId = 123;
if (transport.onmessage) {
transport.onmessage({
jsonrpc: '2.0',
id: requestId,
method: 'ping',
params: {}
// 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
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'
}
});
}
// Wait for the handler to complete
await new Promise(resolve => setTimeout(resolve, 150));
// Verify the request was aborted
expect(wasAborted).toBe(true);
}
// 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'
}
});
}
// Wait for the handler to complete
await new Promise(resolve => setTimeout(resolve, 150));
// Verify the request was aborted
expect(wasAborted).toBe(true);
});
);
});
// Spec basic/patterns/cancellation §Transport-Specific (2026-07-28): on a