Skip to content

Commit 4932a44

Browse files
committed
Widen 2026 cancel silence to every request on a modern-locked connection
The silence fact attached only after classification succeeded, so a request rejected before classification (envelope-less, malformed envelope) on a modern-locked connection could still answer after a peer cancel: the reject runs on a task spawned after the read loop may have already consumed an adjacent cancel. Unreachable under asyncio's FIFO scheduling - the reject's answer write always starts before the cancel is consumed, which is the legitimate already-answering case - but reachable under trio's randomized scheduling and under any future checkpoint on the reject path, where the frame written would have been the legacy code-0 answer or a post-cancel rejection error. Attach the fact at the top of the modern-locked branch instead, rejections included: a rejection error is a legitimate answer, only the post-cancel frame must be silent. On a still-unlocked connection the rule stays classification-scoped (an unclassifiable request has not proven modern semantics and keeps the legacy answer), initialize has no window at all (inline dispatch parks the read loop), and a legacy straggler cancelled after the lock keeps its legacy answer - scope documented at both attachment sites and on serve_dual_era_loop. With the locked branch owning the connection-scoped fact, serve_modern now attaches its two per-request facts only while the era is unlocked: once locked, the silence fact is already set and the era-lock observer is a permanent no-op, so post-lock requests skip the partial allocation and the per-frame observer invocation.
1 parent 34d4f74 commit 4932a44

2 files changed

Lines changed: 76 additions & 7 deletions

File tree

src/mcp/server/runner.py

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -626,8 +626,19 @@ async def serve_dual_era_loop(
626626
Classified-modern requests are also governed by the 2026 cancellation
627627
rule for their whole lifetime: after an inbound `notifications/cancelled`
628628
the server sends nothing further for that request - no result, no error
629-
(the transport spec's MUST NOT). Legacy-era requests keep the
630-
byte-identical "Request cancelled" answer released 2025 clients expect.
629+
(the transport spec's MUST NOT). Once the connection is LOCKED modern the
630+
rule widens from classification-scoped to connection-scoped: every
631+
request - including one rejected before classification succeeds
632+
(envelope-less, malformed envelope) - carries the silence commitment, so
633+
a rejection error computed after the peer's cancel never reaches the wire
634+
(the rejection itself, absent a cancel, is of course still answered).
635+
Legacy-era requests keep the byte-identical "Request cancelled" answer
636+
released 2025 clients expect - including a legacy-classified request
637+
still in flight when the modern lock commits (the straggler carve-out
638+
covers its cancel answer, not just its response) - as does a request that
639+
fails classification on a still-unlocked connection: an unclassifiable
640+
request has not proven modern semantics, so the legacy answer stands
641+
there.
631642
"""
632643
dispatcher: JSONRPCDispatcher[TransportContext] = JSONRPCDispatcher(
633644
read_stream,
@@ -672,11 +683,18 @@ async def serve_modern(
672683
if isinstance(route, InboundLadderRejection):
673684
raise MCPError(code=route.code, message=route.message, data=route.data)
674685
# Classification succeeded: 2026 wire semantics govern this request's
675-
# lifecycle from here. Both facts attach synchronously - no checkpoint
676-
# since the dispatcher entered the handler scope - so a peer cancel
677-
# can never interleave and observe the legacy defaults.
678-
suppress_cancel_answer(dctx)
679-
observe_success_frames(dctx, partial(commit_modern, dctx, route.protocol_version))
686+
# lifecycle from here. On a still-unlocked connection this is the
687+
# silence rule's exact scope - a request that fails classification
688+
# above has not proven modern semantics and keeps the legacy cancel
689+
# answer - and the era lock rides the request's first success frame.
690+
# Both facts attach synchronously - no checkpoint since the
691+
# dispatcher entered the handler scope - so a peer cancel can never
692+
# interleave and observe the legacy defaults. Once the connection is
693+
# locked, neither attaches here: `on_request` owns the
694+
# connection-scoped silence fact, and the lock can never move again.
695+
if era == "unlocked":
696+
suppress_cancel_answer(dctx)
697+
observe_success_frames(dctx, partial(commit_modern, dctx, route.protocol_version))
680698
connection = Connection.from_envelope(
681699
route.protocol_version,
682700
route.client_info,
@@ -718,6 +736,16 @@ async def on_request(
718736
# METHOD_NOT_FOUND a handshake-only server produced, byte for byte.
719737
return await loop_runner.on_request(dctx, method, params)
720738
if era == "modern":
739+
# Connection-scoped silence: once locked modern, EVERY request is
740+
# governed by the 2026 cancellation rule, rejections included - a
741+
# rejection error is a legitimate answer, but nothing may follow
742+
# the peer's `notifications/cancelled`. Pre-classification rejects
743+
# run on a spawned task, so an adjacent cancel can precede them
744+
# under trio's randomized scheduling (asyncio's FIFO always starts
745+
# the reject's answer write first - the legitimate
746+
# already-answering case; inline `initialize` has no window at
747+
# all). See the cancellation paragraph on `serve_dual_era_loop`.
748+
suppress_cancel_answer(dctx)
721749
if method == "initialize":
722750
raise MCPError(
723751
code=UNSUPPORTED_PROTOCOL_VERSION,

tests/server/test_runner.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2101,6 +2101,47 @@ async def list_tools(ctx: Ctx, params: PaginatedRequestParams | None) -> ListToo
21012101
assert answer.error.code == UNSUPPORTED_PROTOCOL_VERSION
21022102

21032103

2104+
@pytest.mark.anyio
2105+
async def test_dual_era_loop_modern_locked_envelope_less_reject_with_adjacent_cancel_never_answers_legacy():
2106+
"""On a modern-locked connection the silence commitment is
2107+
connection-scoped: it attaches to every request, including one rejected
2108+
before classification (envelope-less). With the cancel adjacent behind
2109+
the request, asyncio's FIFO scheduling always starts the reject's answer
2110+
write before the read loop consumes the cancel - the legitimate
2111+
already-answering case - so exactly one frame appears for the id: the
2112+
INVALID_PARAMS classification error, never a code-0 legacy cancel answer,
2113+
and nothing after it. (The silenced arm - the cancel consumed before the
2114+
reject computes, reachable under trio's randomized scheduling - is pinned
2115+
at the dispatcher level by
2116+
test_suppress_cancel_answer_silences_even_an_error_raised_after_the_cancel_landed.)"""
2117+
2118+
async def list_tools(ctx: Ctx, params: PaginatedRequestParams | None) -> ListToolsResult:
2119+
return ListToolsResult(tools=[Tool(name="t", input_schema={"type": "object"})])
2120+
2121+
srv: SrvT = Server(name="fast-server", version="0.0.1", on_list_tools=list_tools)
2122+
async with raw_dual_era_loop(srv) as (c2s, s2c):
2123+
# Lock modern with a classified request.
2124+
c2s.send_nowait(_raw_req(1, "tools/list", _modern_params()))
2125+
locked = (await s2c.receive()).message
2126+
assert isinstance(locked, JSONRPCResponse)
2127+
# Envelope-less request with the cancel adjacent behind it.
2128+
c2s.send_nowait(_raw_req(9, "tools/list", {"plain": True}))
2129+
c2s.send_nowait(_raw_note("notifications/cancelled", {"requestId": 9}))
2130+
answer = (await s2c.receive()).message
2131+
assert isinstance(answer, JSONRPCError)
2132+
assert answer.id == 9
2133+
assert answer.error.code == INVALID_PARAMS
2134+
assert answer.error != ErrorData(code=0, message="Request cancelled")
2135+
# Nothing further for the cancelled id, and the connection stays healthy.
2136+
await anyio.wait_all_tasks_blocked()
2137+
with pytest.raises(anyio.WouldBlock):
2138+
s2c.receive_nowait()
2139+
c2s.send_nowait(_raw_req(10, "tools/list", _modern_params()))
2140+
result = (await s2c.receive()).message
2141+
assert isinstance(result, JSONRPCResponse)
2142+
assert result.id == 10
2143+
2144+
21042145
@pytest.mark.anyio
21052146
async def test_dual_era_loop_maps_unmapped_handler_exceptions_like_the_modern_http_entry():
21062147
"""An unmapped handler exception on a modern request surfaces as the

0 commit comments

Comments
 (0)