Skip to content

Dispatcher mints a request id already used by a completed caller-supplied request (spec: ids MUST NOT be reused in a session) #3126

Description

@ayaangazali

Initial Checks

Description

On current main (v2, tested at 3a6f299), when a caller supplies its own request id via CallOptions["request_id"] (added in #3046), the dispatcher's minted-id sequence can later land on that same id after the supplied request completes. The session then sends two different requests with the same id, which the spec forbids:

The request ID MUST NOT have been previously used by the requestor within the same session.

(https://modelcontextprotocol.io/specification/2025-11-25/basic#requests, same wording since 2025-03-26. The SDK pins this itself as protocol:request-id:unique in tests/interaction/_requirements.py: "ids are never reused within the session".)

Root cause

In JSONRPCDispatcher.send_raw_request (src/mcp/shared/jsonrpc_dispatcher.py, around line 338):

else:
    # Mint past any key a supplied id occupies: the collision error is
    # reserved for the caller who actually chose the id.
    request_id = self._allocate_id()
    while request_id in self._pending:
        request_id = self._allocate_id()

The comment says minting should skip past any supplied id, but the guard only checks self._pending, and pending entries are popped when a request completes (line ~428). So for a supplied integer id (or numeric string, since coerce_request_id folds "2" and 2 into one key): once that request finishes, the monotonic counter eventually reaches the same value and reuses it on the wire.

DirectDispatcher._dispatch_request (src/mcp/shared/direct_dispatcher.py, around line 253) has the same pattern with self._in_flight_ids, which is discarded in the finally.

Why it matters

The receiving side is allowed to assume per-session id uniqueness. The SDK's own stateful Streamable HTTP server already mishandles duplicate ids (#3060, response cross-wiring), so an innocent client that supplies small integer ids for a few calls and then keeps using the same session can hit that server-side behavior through no fault of its own. The SDK-internal user of this feature (the subscriptions/listen driver's "listen-N" ids) is immune because non-numeric string ids never collide with the minted integer sequence, so this only bites users of the new public CallOptions["request_id"] with integer or numeric-string ids.

Note this is only about the dispatcher's own minting crossing a completed supplied id. The reverse direction (a caller re-supplying an id it already used before) is currently allowed on purpose and asserted by tests, so I left that alone.

Proposed fix

When accepting a supplied id, advance the mint counter past its coerced key so minted ids can never revisit it:

# jsonrpc_dispatcher.py, after computing pending_key for a supplied id:
if isinstance(pending_key, int):
    self._next_id = max(self._next_id, pending_key)

and the same in direct_dispatcher.py with in_flight_key. Happy to open a PR with regression tests for both dispatchers if this looks right.

Example Code

import anyio
from mcp_types import JSONRPCRequest, JSONRPCResponse

from mcp.shared.jsonrpc_dispatcher import JSONRPCDispatcher
from mcp.shared.message import SessionMessage


async def main() -> None:
    c2s_send, c2s_recv = anyio.create_memory_object_stream[SessionMessage | Exception](4)
    s2c_send, s2c_recv = anyio.create_memory_object_stream[SessionMessage | Exception](4)
    client = JSONRPCDispatcher(s2c_recv, c2s_send)

    async def on_request(ctx, method, params):
        return {}

    async def on_notify(ctx, method, params):
        pass

    wire_ids = []

    async def respond() -> None:
        async for wire in c2s_recv:
            if isinstance(wire, SessionMessage) and isinstance(wire.message, JSONRPCRequest):
                wire_ids.append(wire.message.id)
                await s2c_send.send(
                    SessionMessage(JSONRPCResponse(jsonrpc="2.0", id=wire.message.id, result={}))
                )

    async with anyio.create_task_group() as tg:
        await tg.start(client.run, on_request, on_notify)
        tg.start_soon(respond)

        await client.send_raw_request("ping", None, {"request_id": 2})
        await client.send_raw_request("ping", None)
        await client.send_raw_request("ping", None)

        tg.cancel_scope.cancel()

    print("wire request ids in order:", wire_ids)
    # prints: wire request ids in order: [2, 1, 2]
    # the third request (dispatcher-minted) reuses the completed supplied id 2


anyio.run(main)

Python & MCP Python SDK

python: 3.14.5
mcp: main @ 3a6f2996 (v2 development line)

AI disclosure, since it's policy here: I leaned on Claude Code to help navigate the codebase and pressure-test this analysis, but I ran and verified the repro myself and I understand the fix I'm proposing. Freshman in college trying to contribute something genuinely useful, so if I've misjudged the intent behind the mint loop I'd honestly love the correction :)

Activity

  1. added
    bugSomething isn't working
    P2Moderate issues affecting some users, edge cases, potentially valuable feature
    needs confirmationNeeds confirmation that the PR is actually required or needed.
    v2Affects the v2 line (2.x on main)
    on Aug 14, 2026
  2. Parker-Fawcett commented on Aug 23, 2026

    @Parker-Fawcett

    Confirmed against current main (57394b0): supplying request_id=1, letting it complete, then minting gives wire ids [1, 1, 2, 3] on both dispatchers.

    I have a working fix on a branch, with one deliberate deviation from the proposed max(self._next_id, pending_key) advancement worth flagging before I open a PR: advancing at accept time also burns every id below a supplied one whenever it is accepted while still in flight, which flips the existing documented contract in test_minted_ids_skip_a_caller_supplied_id_still_in_flight from mints [1, 2, 4] to [4, 5, 6].

    Instead, accepted numeric keys are added to a small retired set; minting skips pending ∪ retired and prunes retired entries as the monotonic counter passes them. That fixes the reuse with zero change to any existing behavior:

    • supplied 1 completes → mints [2, 3, 4] (was [1, 1, 2])
    • supplied "7" completes → mints walk [1..6, 8, 9, 10] (7 skipped when reached)
    • supplied "3" still in flight → mints still [1, 2, 4] (unchanged)

    Applied symmetrically to JSONRPCDispatcher and DirectDispatcher; regression tests for all three sequences run through the shared pair_factory so both dispatchers are covered. Full suite, 100% coverage, ruff, pyright green locally.

    Happy to open the PR if the issue gets assigned to me — or if you'd rather keep the simpler accept-time advancement and adjust the in-flight expectation instead, that tradeoff is yours to call.

    (AI-assisted analysis and drafting, prepared with a coding agent and reviewed by me.)

  3. w1977-0 commented on Sep 3, 2026

    @w1977-0

    I independently confirmed the reproduction and root cause on current main (both dispatchers): the mint loop only consults the in-flight table, so once a supplied numeric id completes, the counter revisits it on the wire — supplied 2, then three minted pings → [2, 1, 2].

    Why it matters for my use case: I drive long-lived MCP client sessions where a supervisor layer supplies its own request ids for the initial setup calls (they double as correlation keys in our logs), then the session keeps issuing dispatcher-minted calls for hours. A minted id colliding with a completed supplied id is exactly the cross-wiring window #3060 describes on the receiving side, and it appeared in our logs as a response delivered to the wrong waiter.

    Approach I would take (happy to be told a better one): when a supplied id is accepted, advance the mint counter past its coerced key — if isinstance(key, int): self._next_id = max(self._next_id, key) — in both JSONRPCDispatcher.send_raw_request and DirectDispatcher._dispatch_request. Minted ids are ints, so only numeric keys ("7" and 7 share the coerced key) can collide; the SDK-internal listen-N ids are unaffected, and caller re-supply of its own used id stays allowed per the existing tests. One existing test expectation would change: test_minted_ids_skip_a_caller_supplied_id_still_in_flight would see [4, 5, 6] instead of [1, 2, 4], because the counter clears the supplied id at acceptance rather than skipping only on collision — the never-collide contract itself still holds.

    I have a working fix with regression tests for both dispatchers parameterized through pair_factory (confirming the completed-id case, the numeric-string twin case, and a guard that re-supply of one's own used id remains allowed). I opened #3433 with it — I see it was auto-closed pending assignment; keeping the branch alive either way. If the approach sounds right and capacity allows, I'd be glad to be assigned. If it's faster for a maintainer to write the fix directly, the comment above should be enough to reproduce and verify it.

    AI disclosure: I used an AI coding agent to help navigate the codebase, and I verified the repro, root cause, fix, and full local test runs myself.

  4. maxisbey commented on Oct 2, 2026

    @maxisbey
    Contributor

    I closed this with #3619, which is smaller than the fix you proposed. Your repro and root cause were both right, thank you.

    The numbering stays as it is. The CallOptions["request_id"] docstring now says a numeric id can be minted again once its request has finished, so a non-numeric string is the way to keep an id unique for the whole session.

    I did look at fixing it in code, but remembering every supplied id grows without bound, and moving the counter changes the minted ids for callers who already use this option.

    If that leaves a real case uncovered, a new issue is very welcome.

    AI Disclaimer

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

    P2Moderate issues affecting some users, edge cases, potentially valuable featurebugSomething isn't workingneeds confirmationNeeds confirmation that the PR is actually required or needed.v2Affects the v2 line (2.x on main)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions