Skip to content

requestId 0 is silently dropped by _oncancel, making the first request from every Protocol uncancellable #2115

@feldmannn

Description

@feldmannn

Problem

packages/core/src/shared/protocol.ts:411-414:

private async _oncancel(notification: CancelledNotification): Promise<void> {
    if (!notification.params.requestId) {
        return;
    }
    const controller = this._requestHandlerAbortControllers.get(notification.params.requestId);
    controller?.abort(notification.params.reason);
}

The if (!notification.params.requestId) check is falsy on 0. Combined with:

  • _requestMessageId = 0 (line 313)
  • const messageId = this._requestMessageId++ (line 868, post-increment)

The first outbound request from every Client or Server instance gets wire id 0. A peer notifications/cancelled { requestId: 0 } is silently dropped. The handler runs to completion with ctx.mcpReq.signal.aborted === false.

Both Client and Server extend Protocol, so the bug applies in both directions (peer-cancelling the client's first request, or peer-cancelling the server's first peer-initiated request).

Reproducer

Requires a build from main or the next alpha (InMemoryTransport is not in the 2.0.0-alpha.2 publish; see #1834). The reproducer drives raw JSON-RPC messages so it can pin the wire id to 0:

import { Server, InMemoryTransport } from '@modelcontextprotocol/server';

const [a, b] = InMemoryTransport.createLinkedPair();
const server = new Server({ name: 's', version: '1' }, { capabilities: { tools: {} } });

let observed;
server.setRequestHandler('tools/call', async (req, ctx) => {
    await new Promise(resolve => {
        ctx.mcpReq.signal.addEventListener('abort', () => {
            observed = true;
            resolve();
        }, { once: true });
        setTimeout(() => {
            observed = ctx.mcpReq.signal.aborted;
            resolve();
        }, 500);
    });
    return { content: [{ type: 'text', text: 'done' }] };
});

await server.connect(a);
b.onmessage = () => {};

await b.send({ jsonrpc: '2.0', id: 0, method: 'tools/call', params: { name: 'foo', arguments: {} } });
await new Promise(r => setTimeout(r, 50));
await b.send({ jsonrpc: '2.0', method: 'notifications/cancelled', params: { requestId: 0 } });
await new Promise(r => setTimeout(r, 600));

console.log('observed:', observed); // false (BUG: should be true)

The same script with id: 1 (a non-zero id) cancels in under 50ms.

Proposed fix

if (notification.params.requestId === undefined) {
    return;
}

JSON-RPC permits string ids too, so a defense-in-depth version is === undefined || requestId === null.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions