Skip to content

Progress notifications are dropped when they arrive in the same read as their request's response #3018

Open

Description

@ot-of-fincra

Version

@modelcontextprotocol/sdk 1.32.1

Summary

Protocol._onnotification defers its handler by a microtask, while Protocol._onresponse runs synchronously inside the same onmessage call. When a transport delivers a progress notification and the response that ends that request in a single read, the response is handled first, deletes the request's progress handler, and the deferred notification then finds nothing and is discarded as belonging to an unknown token.

shared/protocol.js:

_onnotification(notification) {
    ...
    // Starting with Promise.resolve() puts any synchronous errors into the monad as well.
    Promise.resolve()
        .then(() => handler(notification))
        .catch(error => this._onerror(...));
}

versus _onresponse, which runs synchronously and does:

if (!isTaskResponse) {
    this._progressHandlers.delete(messageId);
}

The messages are in the correct order on the wire. The reordering happens inside the dispatch.

Why it is intermittent

It only shows up when the reader is slow enough to take both messages out of the pipe in one read — so it appears under load and not on an idle machine. On a quiet machine the notification's microtask drains between reads and everything works.

Reproduction

A stdio server that sends a progress notification and then the result for the same request, with a client that passes onprogress to callTool. Under CPU contention (six test processes in parallel on an 8-core machine) the notification is lost in roughly 60% of runs. Two concurrent calls make it easier to hit, because the second call's notification is more likely to share a read with its response.

Instrumenting the handler map shows it directly:

register id=1, register id=2
onprogress token=1 known=true  keys=[1,2]
delete id=1
delete id=2                               <- response for request 2
onprogress token=2 known=false keys=[]    <- its notification, now orphaned

The caller sees no progress at all, and onerror reports "Received a progress notification for an unknown token".

Suggested fix

Dispatch the notification handler synchronously, as _onresponse already does, routing both synchronous and asynchronous errors to _onerror:

try {
    const result = handler(notification);
    if (result && typeof result.then === 'function') {
        result.catch(error => this._onerror(new Error(`Uncaught error in notification handler: ${error}`)));
    }
} catch (error) {
    this._onerror(new Error(`Uncaught error in notification handler: ${error}`));
}

Measured with that change applied locally, same harness and same contention: 11 lost notifications in 18 runs before, 0 in 18 after.

An alternative framing, if synchronous dispatch is unwanted: defer the response path as well, so notifications and responses cannot be reordered relative to each other.

Activity

  1. Ben-Home commented on Oct 14, 2026

    @Ben-Home
  2. Steeve-Crypto commented on Oct 14, 2026

    @Steeve-Crypto

    Working on a fix: dispatch progress/notification handlers synchronously in _onnotification (same as _onresponse) so a progress notification that shares a read with its response is not dropped after the progress handler map is cleared. Will include a regression test.

  3. added
    v1Issues / PRs related to v1.x
    v2Ideas, requests and plans for v2 of the SDK which will incorporate major changes and fixes
    on Oct 15, 2026
  4. ahmedibrahim085 commented on Oct 30, 2026

    @ahmedibrahim085

    Confirming this on @modelcontextprotocol/client 2.4.0 (Node 26.12.1).

    We first saw it with concurrent callTool requests over one shared stdio child: every caller except one lost its last progress step, and onerror reported "Received a progress notification for an unknown token". The mechanism matches the description here. _onnotification defers the handler through Promise.resolve().then(...), while _onresponse deletes the progress handler synchronously, so a last notification that arrives in the same read as its response finds no handler.

    Deterministic repro (no concurrency needed): a raw stdio server that sends its last progress notification and its response in one stdout write.

    // server.mjs: raw stdio, no SDK
    import readline from 'node:readline';
    const send = (...msgs) => process.stdout.write(msgs.map((m) => JSON.stringify(m) + '\n').join(''));
    readline.createInterface({ input: process.stdin }).on('line', (line) => {
      const m = JSON.parse(line);
      if (m.method === 'initialize')
        send({ jsonrpc: '2.0', id: m.id, result: { protocolVersion: m.params.protocolVersion, capabilities: { tools: {} }, serverInfo: { name: 'repro', version: '1' } } });
      else if (m.method === 'tools/list')
        send({ jsonrpc: '2.0', id: m.id, result: { tools: [{ name: 'work', inputSchema: { type: 'object' } }] } });
      else if (m.method === 'tools/call')
        send( // ONE write: last progress, then the response
          { jsonrpc: '2.0', method: 'notifications/progress', params: { progressToken: m.params._meta.progressToken, progress: 1, total: 1 } },
          { jsonrpc: '2.0', id: m.id, result: { content: [{ type: 'text', text: 'done' }] } });
    });
    // client.mjs (sketch of the measured loop)
    import { Client } from '@modelcontextprotocol/client';
    import { StdioClientTransport } from '@modelcontextprotocol/client/stdio';
    const client = new Client({ name: 'repro-client', version: '1' });
    await client.connect(new StdioClientTransport({ command: process.execPath, args: ['server.mjs'] }));
    const seen = [];
    await client.callTool({ name: 'work', arguments: {} }, { onprogress: (p) => seen.push(p.progress) });
    // seen never contains 1; onerror receives the unknown-token error

    Measured, 20 fresh client/server pairs each:

    Client Last step lost "unknown token" errors
    SDK onprogress 20 / 20 20
    Workaround below 0 / 20 0

    Client-side workaround that works on 2.4.0:

    • mint your own _meta.progressToken per call, and do not pass onprogress;
    • register one setNotificationHandler('notifications/progress', …) per client that routes by token to the call;
    • remove the route only after the callTool promise settles (in a finally). The deferred notification still finds its route; anything later is dropped quietly.

    This repro exercises the same-read ordering that #3023 and #2967 both target, so it can be used to validate either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    v1Issues / PRs related to v1.xv2Ideas, requests and plans for v2 of the SDK which will incorporate major changes and fixes

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions