modelcontextprotocol / modelcontextprotocol/python-sdk

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

Abierto
#3,126 2 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

bug needs confirmation P2 v2
Lenguaje dominante
Python
Estrellas
24.3k
Forks
4k
Merge medio
1 d 1 h
PR fusionados (30 d)
31

Descripción

Initial Checks
Description

On current main (v2, tested at 3a6f2996), 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 :)

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Comienza con JSONRPCDispatcher.send_raw_request en src/mcp/shared/jsonrpc_dispatcher.py y DirectDispatcher._dispatch_request en src/mcp/shared/direct_dispatcher.py; después, inspecciona los requisitos de request-id en tests/interaction/_requirements.py. Reproduce la secuencia de IDs proporcionada y añade cobertura de regresión para ambos dispatchers; se considera terminado cuando los IDs de wire generados nunca reutilizan, dentro de una sesión, un ID entero proporcionado por el llamador que ya se haya completado.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
python
Área
api, testing
Tipo de issue
Error
Dificultad
3/5
Tiempo estimado
1-2 días
Estado de actividad
Activo
Claridad
Bien especificado
Aptitud para principiantes
86/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.