fix(dispatcher): advance mint counter past caller-supplied IDs (#3126)

src/mcp/shared/direct_dispatcher.py

@@ -250,6 +250,10 @@ async def _dispatch_request(
                     in_flight_key = coerce_request_id(request_id)
                     if in_flight_key in self._in_flight_ids:
                         raise ValueError(f"request id {request_id!r} is already in flight")
+                    # Advance the mint counter past any supplied integer id so
+                    # the monotonic sequence never revisits it after completion.
+                    if isinstance(in_flight_key, int):
+                        self._next_id = max(self._next_id, in_flight_key)
                 else:
                     # Synthesize an id (the DispatchContext contract reserves None
                     # for notifications), minting past any key a supplied id

src/mcp/shared/jsonrpc_dispatcher.py

@@ -346,6 +346,12 @@ async def send_raw_request(
             pending_key = coerce_request_id(request_id)
             if pending_key in self._pending:
                 raise ValueError(f"request id {request_id!r} is already in flight")
+            # Advance the mint counter past any supplied integer id so the
+            # monotonic sequence can never revisit it after the request completes.
+            # This satisfies the spec: "The request ID MUST NOT have been
+            # previously used by the requestor within the same session."
+            if isinstance(pending_key, int):
+                self._next_id = max(self._next_id, pending_key)
         else:
             # Mint past any key a supplied id occupies: the collision error is
             # reserved for the caller who actually chose the id.

tests/shared/test_dispatcher.py

@@ -474,11 +474,45 @@ async def parked() -> None:
 
                 tg.start_soon(parked)
                 await entered.wait()
-                # The counter mints 1 and 2, then skips the occupied 3 to 4.
+                # The counter is advanced to 3 when "3" is supplied, so
+                # subsequent mints produce 4, 5, 6 — never revisiting 3.
                 for _ in range(3):
                     await client.send_raw_request("plain", None)
                 release.set()
-            assert [request_id for request_id in seen_ids if request_id != "3"] == [1, 2, 4]
+            assert [request_id for request_id in seen_ids if request_id != "3"] == [4, 5, 6]
+
+
+@pytest.mark.anyio
+async def test_minted_id_skips_injected_consecutive_in_flight_ids():
+    """Defensive guard: if _in_flight_ids contains the next sequential
+    candidates, the while-loop advances past all of them."""
+
+    async def noop(
+        ctx: DispatchContext[TransportContext], method: str, params: Mapping[str, Any] | None
+    ) -> dict[str, Any]:
+        return {}
+
+    client, server, close = direct_pair()
+    assert isinstance(server, DirectDispatcher)
+    try:
+        async with anyio.create_task_group() as tg:
+            await tg.start(client.run, noop, noop)
+            await tg.start(server.run, noop, noop)
+            # send_raw_request on client dispatches on the server's peer
+            # (_dispatch_request runs on server). Inject synthetic in-flight
+            # keys into the SERVER so the mint loop must skip them.
+            # _next_id is 0, so mint increments to 1, finds it occupied,
+            # increments to 2, finds it occupied, increments to 3, finds it
+            # occupied, and finally lands on 4.
+            server._in_flight_ids.update({1, 2, 3})
+            result = await client.send_raw_request("ping", None)
+            assert result == {}
+            # After completion the id is discarded from in_flight, but the
+            # counter must have advanced to 4 (skipping 1, 2, 3).
+            assert server._next_id == 4
+            tg.cancel_scope.cancel()
+    finally:
+        close()
 
 
 @pytest.mark.anyio
@@ -512,6 +546,64 @@ async def first() -> None:
             assert await client.send_raw_request("again", None, {"request_id": "7"}) == {}
 
 
+@pytest.mark.anyio
+async def test_minted_ids_never_reuse_a_completed_caller_supplied_id(pair_factory: PairFactory):
+    """Regression: after a caller-supplied integer id completes, the mint counter
+    must have advanced past it so no future minted id collides. This is the bug
+    from GH-3126: the counter could land on a previously-used supplied id because
+    the guard only checked `_pending`/`_in_flight_ids` (cleared on completion)."""
+    seen_ids: list[RequestId | None] = []
+
+    async def track(
+        ctx: DispatchContext[TransportContext], method: str, params: Mapping[str, Any] | None
+    ) -> dict[str, Any]:
+        seen_ids.append(ctx.request_id)
+        return {}
+
+    async with running_pair(pair_factory, server_on_request=track) as (client, *_):
+        with anyio.fail_after(5):
+            # Send a request with a caller-supplied integer id.
+            await client.send_raw_request("supplied", None, {"request_id": 5})
+            # Now send several auto-minted requests. None should reuse id 5.
+            for _ in range(6):
+                await client.send_raw_request("minted", None)
+
+    # The first id is the supplied 5; the rest are minted sequentially starting
+    # above 5 (i.e. 6, 7, 8, 9, 10, 11).
+    assert seen_ids[0] == 5
+    minted_ids = seen_ids[1:]
+    assert 5 not in minted_ids
+    # Verify they are unique and monotonically increasing integers > 5.
+    assert all(isinstance(i, int) and i > 5 for i in minted_ids)
+    assert len(minted_ids) == len(set(minted_ids))
+
+
+@pytest.mark.anyio
+async def test_minted_ids_never_reuse_a_completed_numeric_string_id(pair_factory: PairFactory):
+    """Same as above but with a numeric-string supplied id ("3"), which coerces to
+    int 3 in the collision domain. Minted ids must skip past 3."""
+    seen_ids: list[RequestId | None] = []
+
+    async def track(
+        ctx: DispatchContext[TransportContext], method: str, params: Mapping[str, Any] | None
+    ) -> dict[str, Any]:
+        seen_ids.append(ctx.request_id)
+        return {}
+
+    async with running_pair(pair_factory, server_on_request=track) as (client, *_):
+        with anyio.fail_after(5):
+            await client.send_raw_request("supplied", None, {"request_id": "3"})
+            for _ in range(4):
+                await client.send_raw_request("minted", None)
+
+    assert seen_ids[0] == "3"
+    minted_ids = seen_ids[1:]
+    # 3 should never appear (even though "3" completed and left _pending/_in_flight).
+    assert 3 not in minted_ids
+    assert all(isinstance(i, int) and i > 3 for i in minted_ids)
+    assert len(minted_ids) == len(set(minted_ids))
+
+
 @pytest.mark.anyio
 async def test_notify_intercept_sees_every_notification_and_consumes_on_true(pair_factory: PairFactory):
     """The intercept sees every inbound notification; a frame it consumes never reaches `on_notify`, the rest do."""