Skip to content

Commit 46c04ba

Browse files
committed
Address review round two: bounded lead, exact context check, prose
- Frames arriving ahead of the client's first request are kept only up to a small bound and never decide the era: past the bound they are dropped while the loop keeps waiting for the opening request, so a bare notifications/initialized still opens the gate and a flood cannot grow the buffer or pick an era. - Check the captured sender context with `is not None`, since an empty contextvars.Context is falsy. - Word the legacy refusal to the actual rule (first request carried no 2026 envelope) and update the Server.run docstring and one test to the decide-from-the-opening-request model.
1 parent c3b0e01 commit 46c04ba

3 files changed

Lines changed: 39 additions & 38 deletions

File tree

src/mcp/server/lowlevel/server.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -703,9 +703,9 @@ async def run(
703703
704704
Thin wrapper over `serve_dual_era_loop`: enters the server lifespan,
705705
then drives the loop, serving the legacy handshake era and the modern
706-
per-request-envelope era (the first era-distinctive message to succeed
707-
locks the connection). Transports with their own lifespan owner (the
708-
streamable-HTTP manager) call `serve_loop` directly instead.
706+
per-request-envelope era (the client's first request decides which).
707+
Transports with their own lifespan owner (the streamable-HTTP manager)
708+
call `serve_loop` directly instead.
709709
"""
710710
async with self.lifespan(self) as lifespan_context:
711711
await serve_dual_era_loop(

src/mcp/server/runner.py

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -643,39 +643,39 @@ async def serve_dual_era_loop(
643643
await write_stream.aclose()
644644

645645

646-
_OPENING_PEEK_LIMIT: int = 32
647-
"""How many frames may precede the client's first request before the loop
648-
stops looking for one. Every legitimate client opens with a request, so this
649-
only bounds a peer that streams other frames without ever sending one."""
646+
_PRE_REQUEST_REPLAY_LIMIT: int = 8
647+
"""How many frames arriving ahead of the client's first request are kept
648+
for the chosen era's loop (a bare `notifications/initialized` is the one that
649+
matters); further ones are dropped and never decide the era."""
650650

651651

652652
def _sender_context(stream: ReadStream[Any]) -> contextvars.Context:
653653
"""The per-message sender context a context-aware stream carries, else the current one."""
654-
return getattr(stream, "last_context", None) or contextvars.copy_context()
654+
ctx = getattr(stream, "last_context", None)
655+
return ctx if ctx is not None else contextvars.copy_context()
655656

656657

657658
@asynccontextmanager
658659
async def _replay_from_opening_request(
659660
read_stream: ReadStream[SessionMessage | Exception],
660661
) -> AsyncIterator[tuple[JSONRPCRequest | None, ReadStream[SessionMessage | Exception]]]:
661-
"""Peek at the client's first request without consuming anything.
662+
"""Peek at the client's first request without consuming it.
662663
663-
Reads frames until the first JSON-RPC request arrives, then yields that
664-
request together with a stream that replays every frame read so far and
665-
relays the rest of `read_stream` behind it, sender contexts included. The
666-
request is `None` if the channel closes - or `_OPENING_PEEK_LIMIT` frames
667-
pass - before any request appears.
664+
Yields that request together with a stream that replays it - preceded by
665+
up to `_PRE_REQUEST_REPLAY_LIMIT` earlier frames - and relays the rest of
666+
`read_stream` behind it, sender contexts included. The request is `None`
667+
if the channel closes before one arrives.
668668
"""
669-
peeked: list[tuple[contextvars.Context, SessionMessage | Exception]] = []
670-
opening: JSONRPCRequest | None = None
669+
lead: list[tuple[contextvars.Context, SessionMessage | Exception]] = []
670+
opening_request: JSONRPCRequest | None = None
671671
replay_send, replay_receive = anyio.create_memory_object_stream[
672672
tuple[contextvars.Context, SessionMessage | Exception]
673673
]()
674674
replayed = ContextReceiveStream(replay_receive)
675675

676676
async def replay_then_relay() -> None:
677677
async with replay_send:
678-
for envelope in peeked:
678+
for envelope in lead:
679679
await replay_send.send(envelope)
680680
async for item in read_stream:
681681
await replay_send.send((_sender_context(read_stream), item))
@@ -685,15 +685,17 @@ async def replay_then_relay() -> None:
685685
# closes it and the replay channel.
686686
try:
687687
async for item in read_stream:
688-
peeked.append((_sender_context(read_stream), item))
689688
if isinstance(item, SessionMessage) and isinstance(item.message, JSONRPCRequest):
690-
opening = item.message
691-
break
692-
if len(peeked) >= _OPENING_PEEK_LIMIT:
689+
opening_request = item.message
690+
elif len(lead) >= _PRE_REQUEST_REPLAY_LIMIT:
691+
logger.debug("dropped a frame received before the first request: %r", item)
692+
continue
693+
lead.append((_sender_context(read_stream), item))
694+
if opening_request is not None:
693695
break
694696
async with anyio.create_task_group() as tg:
695697
tg.start_soon(replay_then_relay)
696-
yield opening, replayed
698+
yield opening_request, replayed
697699
tg.cancel_scope.cancel()
698700
finally:
699701
await read_stream.aclose()
@@ -728,8 +730,8 @@ async def on_request(
728730
if method != "initialize" and _has_modern_envelope(params):
729731
raise MCPError(
730732
code=INVALID_REQUEST,
731-
message="connection was opened with the initialize handshake; "
732-
"2026-07-28 envelope requests are not accepted on it",
733+
message="this connection's first request carried no 2026-07-28 envelope, so it "
734+
"serves the handshake era; enveloped requests are not accepted on it",
733735
)
734736
return await runner.on_request(dctx, method, params)
735737

tests/server/test_runner.py

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@
5858
from mcp.server.lowlevel.server import NotificationOptions, Server
5959
from mcp.server.models import InitializationOptions
6060
from mcp.server.runner import (
61-
_OPENING_PEEK_LIMIT,
6261
ServerRunner,
6362
_extract_meta,
6463
_has_modern_envelope,
@@ -1545,7 +1544,7 @@ async def test_dual_era_loop_initialize_locks_legacy_and_rejects_modern_traffic(
15451544
assert result["tools"][0]["name"] == "t"
15461545
assert discover_exc.value.error.code == INVALID_REQUEST
15471546
assert envelope_exc.value.error.code == INVALID_REQUEST
1548-
assert "opened with the initialize handshake" in envelope_exc.value.error.message
1547+
assert "carried no 2026-07-28 envelope" in envelope_exc.value.error.message
15491548

15501549

15511550
@pytest.mark.anyio
@@ -1725,9 +1724,9 @@ async def test_dual_era_loop_initialize_with_envelope_takes_the_handshake_path(s
17251724

17261725

17271726
@pytest.mark.anyio
1728-
async def test_dual_era_loop_modern_notification_dispatches_at_locked_version(server: SrvT):
1729-
"""Notifications carry no envelope, so on a modern-locked connection they
1730-
dispatch with the locked protocol version."""
1727+
async def test_dual_era_loop_modern_notification_dispatches_at_the_served_version(server: SrvT):
1728+
"""Notifications carry no envelope, so on a 2026 connection they dispatch
1729+
at the modern version the server serves."""
17311730
seen_versions: list[str] = []
17321731
handled = anyio.Event()
17331732

@@ -2008,16 +2007,16 @@ async def probe(ctx: Ctx, params: RequestParams | None) -> dict[str, Any]:
20082007

20092008

20102009
@pytest.mark.anyio
2011-
async def test_dual_era_loop_stops_peeking_after_a_flood_of_pre_request_frames(server: SrvT):
2012-
"""A peer that streams notifications and never opens with a request only
2013-
gets the first `_OPENING_PEEK_LIMIT` frames buffered: past that the loop
2014-
stops looking for an opening request and serves the rest as an ordinary
2015-
handshake connection, so the handshake that follows still lands."""
2010+
async def test_dual_era_loop_leading_notifications_never_decide_the_era(server: SrvT):
2011+
"""Frames a peer sends ahead of its first request never decide the era: a
2012+
stream of leading notifications well past the retained lead is followed by
2013+
an enveloped request, which still opens a 2026 connection and serves. The
2014+
lead is bounded, so the flood cannot grow the loop's buffer either."""
20162015
async with dual_era_client(server) as (client, _):
2017-
for _ in range(_OPENING_PEEK_LIMIT + 5):
2018-
await client.notify("notifications/flood", None)
2019-
init = await client.send_raw_request("initialize", _initialize_params())
2020-
assert init["protocolVersion"] == LATEST_HANDSHAKE_VERSION
2016+
for _ in range(50):
2017+
await client.notify("notifications/lead", None)
2018+
result = await client.send_raw_request("tools/list", _modern_params())
2019+
assert result["tools"][0]["name"] == "t"
20212020

20222021

20232022
@pytest.mark.anyio

0 commit comments

Comments
 (0)