| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
… opening request The stdio driver derived a connection's protocol era from whichever request finished first, so it could never host a request that does not return: subscriptions/listen was hard-refused with -32601 even though the same connection advertised list-changed capabilities, and a legacy handshake arriving during a slow 2026 request was accepted and locked the connection out from under it. Decide the era from the client's first request instead, once, before any handler runs: a request carrying the 2026-07-28 per-request envelope opens a 2026 connection, anything else (the initialize handshake) opens a 2025 one, and the deciding frame is replayed into the chosen serving loop. A conflicting later claim is refused in one place (initialize on a 2026 connection gets -32022 naming the served versions; an enveloped request on a handshake connection gets -32600). The listen refusal is deleted; the handler was always transport-agnostic.
📚 Documentation preview
|
Sorry, something went wrong.
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Sorry, something went wrong.
…ening peek - The opening-request replay now carries each frame's captured sender context, so the streams the SSE transport hands the loop keep their contextvars propagation; pinned by a test on the dual-era loop. - Bound how many frames may precede the client's first request; past the limit the loop stops looking for an opening request and serves a handshake connection, so a peer streaming pre-request frames cannot grow the buffer without bound. - Drop the stale docs admonition that stdio still rejects listen, and trim the era-decision docstring to the rule it now implements.
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Sorry, something went wrong.
- 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.
- Treat a read stream whose receive end the transport closed as end-of-input in the opening peek and the relay, mirroring the tolerance the dispatcher's read loop already had; the read loop moved here, so its EOF handling moves with it. Two tests close the stream before and after the first request and expect a clean return. - ListenHandler's docstring no longer claims it needs SSE mode. - The legacy refusal states the era rule instead of narrating what the first request was, so it holds for every opener.
There was a problem hiding this comment.
No new issues this round — all three prior review rounds are verified addressed in the latest commits, but this rewrites the era-decision core of the stdio serving path (new peek/replay/relay concurrency machinery), so it warrants a human pass before merge.
What was reviewed this run:
The PR replaces serve_dual_era_loop's lock-on-first-success era model with decide-from-the-opening-request: a new _replay_from_opening_request context manager peeks the client's first request, a bounded lead of pre-request frames is retained, and a relay task replays the stream (sender contexts included) into either _serve_legacy_stream or _serve_modern_stream. The subscriptions/listen stdio refusal is deleted, docs and the ListenHandler docstring are updated, and the dual-era test suite is rewritten to the new rule. Six files touched; the substantive change is concentrated in src/mcp/server/runner.py.
No auth, crypto, or permission surfaces are touched. The era-decision signal is the spec-reserved io.modelcontextprotocol/protocolVersion meta key, which legacy traffic cannot mint, and cross-era claims are refused with the spec's own error codes. The pre-request frame flood is bounded (_PRE_REQUEST_REPLAY_LIMIT), so a client cannot grow the server's buffer with leading notifications. The one context-propagation subtlety (an empty-but-valid sender context being discarded by a truthiness check, which could have leaked serve-task contextvars into handlers) was caught in round two and fixed with an explicit None check.
High. This is production-critical protocol routing for every stdio and in-memory connection, and the new implementation is concurrency-heavy: a peek loop, a zero-buffer replay channel, a relay child task in a task group, and ownership handoffs of both streams. Three review rounds surfaced real issues (sender-context loss through the replay, receive-end-close tolerance, several diagnostics describing the removed model), all fixed and test-pinned — which is evidence the review earned its depth, and also exactly the profile of change that should get a human maintainer's judgment on the design (e.g. the deliberate choice that a failed 2026 probe still fixes the connection modern, and the author's declining of the debug-mode BrokenResourceError containment).
Test coverage is strong: the dual-era suite was systematically rewritten, the straddle case (initialize during an in-flight modern request) is pinned, listen is driven end-to-end over the stream pair, and both teardown-as-EOF paths have tests. The author states 100% branch coverage, pyright and ruff clean. Two open process items argue for human sign-off regardless: the PR description flags that the upstream conformance suite may lack a listen-over-stdio test (AGENTS.md requires one for new 2026 features), and the behavior changes are user-visible protocol semantics that a maintainer should own.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Supersedes #3151, which grew far beyond the actual fix. This one does one thing.
Why
The stdio driver derived a connection's protocol era from whichever request completed first, so it could never host a request that doesn't return. subscriptions/listen was therefore hard-refused over stdio with -32601, even though the same connection advertised tools.listChanged: true. The same rule meant a legacy initialize arriving while a slow 2026 request was in flight got accepted and locked the connection out from under that request.
What changed
serve_dual_era_loop now decides the era from the client's first request, once, before any handler runs, and replays that frame into the chosen serving loop:
With the era no longer waiting on completions, the subscriptions/listen refusal is simply deleted; the handler was always transport-agnostic and now serves over the stream pair like any other 2026 request. No public API changes: same driver signature, Server, MCPServer, the dispatcher, and the transports are untouched.
Observable corners that move
Not in scope
The trailing {"code": 0, "message": "Request cancelled"} frame the dispatcher writes after a client cancel is a separate stdio conformance item and stays as it is here.
Testing
The dual-era tests in tests/server/test_runner.py are updated to the new rule (two that encoded "the era locks on the first request to succeed" are gone; one now drives an actual subscriptions/listen over the stream pair through ack and graceful close; the straddle case is pinned). Full suite green with 100% branch coverage; pyright and ruff clean.
One process note: AGENTS.md asks that new 2026 features carry a matching conformance-suite test. I don't believe the upstream conformance suite covers listen over stdio yet; flagging that here rather than skipping it.
AI Disclaimer