Skip to content

Commit 4f8fe73

Browse files
Retire caller-supplied numeric request ids against future mints
A caller-supplied id left the dispatcher's in-flight set once its request completed, so the minted-id sequence eventually landed on the same id again - the spec forbids reusing an id within a session. Accepted numeric supplied ids are now retired until the monotonic counter passes them, on both JSONRPCDispatcher and DirectDispatcher. Minting skips retired keys alongside in-flight ones and prunes them as the counter advances, preserving the existing behavior where mints keep using low ids around a still-in-flight supplied id. Fixes #3126
1 parent 57394b0 commit 4f8fe73

3 files changed

Lines changed: 46 additions & 2 deletions

File tree

‎src/mcp/shared/direct_dispatcher.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ def __init__(self, transport_ctx: TransportContext, *, raise_handler_exceptions:
117117
self._on_notify_intercept: OnNotifyIntercept | None = None
118118
self._next_id = 0
119119
self._in_flight_ids: set[RequestId] = set()
120+
self._retired_ids: set[int] = set()
120121
self._ready = anyio.Event()
121122
self._close_event = anyio.Event()
122123
self._running = False
@@ -250,16 +251,21 @@ async def _dispatch_request(
250251
in_flight_key = coerce_request_id(request_id)
251252
if in_flight_key in self._in_flight_ids:
252253
raise ValueError(f"request id {request_id!r} is already in flight")
254+
# Same no-reuse rule as JSONRPCDispatcher: retire the coerced
255+
# key so a later minted request can't land on it.
256+
if isinstance(in_flight_key, int):
257+
self._retired_ids.add(in_flight_key)
253258
else:
254259
# Synthesize an id (the DispatchContext contract reserves None
255260
# for notifications), minting past any key a supplied id
256261
# occupies: the collision error is reserved for the caller
257262
# who actually chose the id.
258263
self._next_id += 1
259-
while self._next_id in self._in_flight_ids:
264+
while self._next_id in self._in_flight_ids or self._next_id in self._retired_ids:
260265
self._next_id += 1
261266
request_id = self._next_id
262267
in_flight_key = request_id
268+
self._retired_ids = {key for key in self._retired_ids if key > request_id}
263269
self._in_flight_ids.add(in_flight_key)
264270
dctx = self._make_context(on_progress=opts.get("on_progress"), request_id=request_id)
265271
try:

‎src/mcp/shared/jsonrpc_dispatcher.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,7 @@ def __init__(
307307
self._next_id = 0
308308
self._pending: dict[RequestId, _Pending] = {}
309309
self._in_flight: dict[RequestId, _InFlight[TransportT]] = {}
310+
self._retired_ids: set[int] = set()
310311
self._on_notify_intercept: OnNotifyIntercept | None = None
311312
self._tg: anyio.abc.TaskGroup | None = None
312313
self._running = False
@@ -346,12 +347,18 @@ async def send_raw_request(
346347
pending_key = coerce_request_id(request_id)
347348
if pending_key in self._pending:
348349
raise ValueError(f"request id {request_id!r} is already in flight")
350+
# Spec: an id is never reused in a session, even after completion —
351+
# retire the coerced key so a later minted request can't land on it.
352+
if isinstance(pending_key, int):
353+
self._retired_ids.add(pending_key)
349354
else:
350355
# Mint past any key a supplied id occupies: the collision error is
351356
# reserved for the caller who actually chose the id.
352357
request_id = self._allocate_id()
353-
while request_id in self._pending:
358+
while request_id in self._pending or request_id in self._retired_ids:
354359
request_id = self._allocate_id()
360+
# The counter never goes back, so retired keys below it are spent.
361+
self._retired_ids = {key for key in self._retired_ids if key > request_id}
355362
pending_key = request_id
356363
out_params = dict(params) if params is not None else {}
357364
out_meta = dict(out_params.get("_meta") or {})

‎tests/shared/test_dispatcher.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,37 @@ async def parked() -> None:
481481
assert [request_id for request_id in seen_ids if request_id != "3"] == [1, 2, 4]
482482

483483

484+
@pytest.mark.anyio
485+
async def test_minted_ids_advance_past_a_completed_caller_supplied_numeric_id(pair_factory: PairFactory):
486+
"""Spec: an id MUST NOT be reused by the requestor within a session — not even
487+
after its request completed and left the in-flight set. Accepting a numeric
488+
supplied id advances the mint counter past it, so minted ids never revisit it."""
489+
async with running_pair(pair_factory) as (client, _server, _crec, srec):
490+
with anyio.fail_after(5):
491+
await client.send_raw_request("first", None, {"request_id": 1})
492+
for _ in range(3):
493+
await client.send_raw_request("plain", None)
494+
supplied, *minted = (ctx.request_id for ctx in srec.contexts)
495+
assert supplied == 1
496+
assert minted == [2, 3, 4]
497+
498+
499+
@pytest.mark.anyio
500+
async def test_minted_ids_advance_past_a_completed_supplied_numeric_string_id(pair_factory: PairFactory):
501+
"""The collision domain folds "7" and 7 into one key, so accepting the string form
502+
retires 7 against future mints even though it left the in-flight set."""
503+
async with running_pair(pair_factory) as (client, _server, _crec, srec):
504+
with anyio.fail_after(5):
505+
await client.send_raw_request("first", None, {"request_id": "7"})
506+
# Mints walk 1..9 but must skip the retired 7.
507+
for _ in range(9):
508+
await client.send_raw_request("plain", None)
509+
supplied, *minted = (ctx.request_id for ctx in srec.contexts)
510+
assert supplied == "7"
511+
assert type(supplied) is str
512+
assert minted == [1, 2, 3, 4, 5, 6, 8, 9, 10]
513+
514+
484515
@pytest.mark.anyio
485516
async def test_supplied_numeric_string_id_collides_with_its_int_twin(pair_factory: PairFactory):
486517
""" "7" and 7 are one id in the collision domain on BOTH dispatchers, so the

0 commit comments

Comments
 (0)