Repository navigation
Dispatcher mints a request id already used by a completed caller-supplied request (spec: ids MUST NOT be reused in a session) #3126
Description
Activity
- addedbugSomething isn't workingSomething isn't workingP2Moderate issues affecting some users, edge cases, potentially valuable featureModerate issues affecting some users, edge cases, potentially valuable featureneeds confirmationNeeds confirmation that the PR is actually required or needed.Needs confirmation that the PR is actually required or needed.v2Affects the v2 line (2.x on main)Affects the v2 line (2.x on main)
on Aug 14, 2026 Confirmed against current
main(57394b0): supplyingrequest_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 intest_minted_ids_skip_a_caller_supplied_id_still_in_flightfrom mints[1, 2, 4]to[4, 5, 6].Instead, accepted numeric keys are added to a small retired set; minting skips
pending ∪ retiredand prunes retired entries as the monotonic counter passes them. That fixes the reuse with zero change to any existing behavior:- supplied
1completes → 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
JSONRPCDispatcherandDirectDispatcher; regression tests for all three sequences run through the sharedpair_factoryso 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.)
- supplied
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 bothJSONRPCDispatcher.send_raw_requestandDirectDispatcher._dispatch_request. Minted ids are ints, so only numeric keys ("7"and7share the coerced key) can collide; the SDK-internallisten-Nids 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_flightwould 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.
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.
Initial Checks
Description
On current
main(v2, tested at 3a6f299), when a caller supplies its own request id viaCallOptions["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:(https://modelcontextprotocol.io/specification/2025-11-25/basic#requests, same wording since 2025-03-26. The SDK pins this itself as
protocol:request-id:uniqueintests/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):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, sincecoerce_request_idfolds"2"and2into 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 withself._in_flight_ids, which is discarded in thefinally.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/listendriver'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 publicCallOptions["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:
and the same in
direct_dispatcher.pywithin_flight_key. Happy to open a PR with regression tests for both dispatchers if this looks right.Example Code
Python & MCP Python SDK
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 :)