| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| Expand Up | @@ -863,7 +863,7 @@ | |
|
|
||
| ### Streamable HTTP: session manager, `EventStore`, and stateless mode unchanged | ||
|
|
||
| Beyond the constructor parameters that moved to `run()`/`streamable_http_app()` and the lifespan change above, the server-side Streamable HTTP machinery is as in v1: | ||
| Beyond the constructor parameters that moved to `run()`/`streamable_http_app()`, the lifespan change above, and the transport rework in the next section (which does not touch the public surface), the server-side Streamable HTTP API is as in v1: | ||
|
Check warning on line 866 in docs/migration.md
|
||
|
|
||
| - `mcp.server.streamable_http` still exports the `EventStore` ABC (`store_event()`, `replay_events_after()`), `EventMessage`, `EventCallback`, `EventId`, and `StreamId` with unchanged signatures; a custom `EventStore` keeps importing `JSONRPCMessage` from `mcp.types`, unchanged. | ||
| - `StreamableHTTPSessionManager` keeps its constructor and its `run()` / `handle_request()` methods (see [Lowlevel `Server`: what did not change](#lowlevel-server-what-did-not-change)); its `stateless=` parameter is unrelated to the removed [`Server.run(stateless=)` flag](#serverrun-no-longer-takes-a-stateless-flag). | ||
| Expand All | @@ -872,6 +872,59 @@ | |
|
|
||
| Only private attributes moved: `mcp._mcp_server` is now `mcp._lowlevel_server` (see [Registering lowlevel handlers from `MCPServer`](#registering-lowlevel-handlers-from-mcpserver)), and `_session_manager` now lives on that lowlevel `Server`. Prefer the public `mcp.session_manager` property to either. | ||
|
|
||
| ### Streamable HTTP: `StreamableHTTPServerTransport` is driven per request, not per stream | ||
|
|
||
| `StreamableHTTPServerTransport` no longer exposes a `connect()` context manager yielding a | ||
| `(read_stream, write_stream)` pair for you to run a server loop over. Each HTTP request is now | ||
| dispatched to the server's handlers directly: a request's outbound messages ride that request's | ||
| own response stream (backed by the optional `EventStore` for `Last-Event-ID` resumability), the | ||
| client's POSTed answers to server-initiated requests are correlated back by request id, and the | ||
| standalone GET stream is a further per-connection channel. The transport is the per-session | ||
| core; `StreamableHTTPSessionManager` binds one to the `Server` for each session (via the new | ||
| keyword-only `app` / `lifespan_state` constructor arguments) and routes requests to it. | ||
|
|
||
| Nothing changes if you serve through `streamable_http_app()` / `run(transport="streamable-http")` | ||
| or mount `StreamableHTTPSessionManager` — the wire behaviour (session ids, GET stream, event | ||
| store, `ctx.close_sse_stream()`, `related_request_id` routing) is unchanged. Only code that | ||
| constructed a transport and consumed `transport.connect()` by hand needs to move to the session | ||
| manager: | ||
|
|
||
| ```python | ||
| # Before (v1) | ||
| transport = StreamableHTTPServerTransport(mcp_session_id=session_id, ...) | ||
| async with transport.connect() as (read_stream, write_stream): | ||
| await server.run(read_stream, write_stream, server.create_initialization_options()) | ||
|
|
||
| # After (v2): serve the app the SDK builds ... | ||
| app = server.streamable_http_app(event_store=..., json_response=...) | ||
| ``` | ||
|
|
||
| ... or, when composing your own Starlette/FastAPI app, mount a `StreamableHTTPSessionManager` and | ||
| enter `session_manager.run()` in the lifespan — see [Mounting the ASGI app](run/asgi.md) for the | ||
| full wiring. | ||
|
|
||
| Behaviour clarified in the same change: | ||
|
|
||
| - In JSON-response mode a *request-scoped* server-to-client request (`ctx.elicit()`, or any | ||
| `ctx.session` call carrying `related_request_id`) now raises `NoBackChannelError` — the POST's | ||
| single JSON body has no stream to carry the nested request, and previously the call would hang | ||
| waiting for an answer that could never be delivered. Connection-scoped sends (calls without | ||
| `related_request_id`) are unchanged and still ride the standalone GET stream. | ||
| - A GET carrying `Last-Event-ID` on a server without an `EventStore` opens the standalone stream | ||
| as a plain GET would, since there is nothing to replay. | ||
|
Comment thread
claude[bot] marked this conversation as resolved.
|
||
| - Two concurrent POSTs that share a JSON-RPC request id each keep their own response stream; the | ||
| second no longer silently takes over the first's queue. | ||
| - Stream ids handed to your `EventStore` are minted by the transport in its own session-scoped | ||
| namespace (previously the raw `str(request_id)` and a single global GET-stream key), so two | ||
| sessions sharing one store no longer collide, and a `Last-Event-ID` replay only releases frames | ||
| of the requesting session's own streams. Treat the ids as opaque. | ||
| - A failing `EventStore.store_event` degrades resumability for that message rather than taking | ||
| the stream down: the message is still delivered live (with no event id to resume from) and the | ||
| store's exception is logged, never sent to the client. | ||
| - A server-to-client request that can reach no client at all (no attached stream and nothing | ||
| storing it, or a request-scoped one in JSON-response mode) fails the calling handler with | ||
| `CONNECTION_CLOSED` instead of parking it for an answer that cannot arrive. | ||
|
Check warning on line 926 in docs/migration.md
|
||
|
Comment thread
Comment on lines
+923
to
+926
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality🟡 The last bullet of the "Behaviour clarified in the same change" list attributes CONNECTION_CLOSED to "a request-scoped [server-to-client request] in JSON-response mode", but that case never reaches the CONNECTION_CLOSED path — it raises NoBackChannelError (serialized as INVALID_REQUEST), exactly as the first bullet of the same list already documents. Deleting the ", or a request-scoped one in JSON-response mode" parenthetical resolves the self-contradiction; CONNECTION_CLOSED is accurate only for the no-attached-stream-and-nothing-storing-it case. Extended reasoning...What the bug is. The new "Behaviour clarified in the same change" list in docs/migration.md gives two contradictory error types for the same scenario. Bullet 1 (lines 908–912) correctly says that in JSON-response mode a request-scoped server-to-client request (ctx.elicit(), or any ctx.session call carrying related_request_id) raises NoBackChannelError. The last bullet (lines 923–925) then says a server-to-client request that can reach no client — "(no attached stream and nothing storing it, or a request-scoped one in JSON-response mode)" — fails the handler with CONNECTION_CLOSED. Both cannot be true, and the code says bullet 1 is right.\n\nThe code path. In JSON-response mode, StreamableHTTPServerTransport._can_send_request is False (self.mcp_session_id is not None and not self.is_json_response_enabled — src/mcp/server/streamable_http.py). The request's dispatch context inherits that via TransportContext(can_send_request=...), so _HTTPRequestDispatchContext.send_raw_request hits if not self.can_send_request: raise NoBackChannelError(method) before _call_over_channel is ever invoked. _call_over_channel is the only site where the CONNECTION_CLOSED conversion can fire (channel.write returning False → anyio.ClosedResourceError → the correlator surfaces MCPError(CONNECTION_CLOSED)). The gate makes that path unreachable for the JSON-mode request-scoped case.\n\nWhy nothing else corrects it. NoBackChannelError (src/mcp/shared/exceptions.py:55–70) is constructed with code=INVALID_REQUEST (-32600) and its docstring says it serializes to an INVALID_REQUEST error response — not CONNECTION_CLOSED (-32000). The PR's own test pins this: test_json_mode_refuses_a_request_scoped_server_to_client_request (tests/server/test_streamable_http_transport.py) asserts exc_info.value.error.code == INVALID_REQUEST. The PR description's version of this very sentence omits the parenthetical ("no attached stream and nothing storing it" only), which supports this being an editing slip introduced in the doc, not an intended claim.\n\nStep-by-step proof. (1) A client opens a stateful session with json_response=True and POSTs tools/call. (2) The handler calls ctx.session.send_request(ElicitRequest(...), metadata=ServerMessageMetadata(related_request_id=ctx.request_id)) — a request-scoped server-to-client request. (3) The transport built the dispatch context with can_send_request = (mcp_session_id is not None and not is_json_response_enabled) = False. (4) send_raw_request raises NoBackChannelError at the gate; _call_over_channel/RequestCorrelator.call never runs, so no CONNECTION_CLOSED can be produced. (5) The handler's failure is serialized as a JSON-RPC error with code INVALID_REQUEST (-32600), matching the PR's test.\n\nImpact. A migrating user who follows the last bullet and keys error handling on CONNECTION_CLOSED for JSON-mode elicitation/sampling failures will never match the actual INVALID_REQUEST error — and the two bullets in the same list telling them different things makes the guide confusing even before they write code. CONNECTION_CLOSED remains accurate for the case the rest of the bullet describes (connection-scoped request over the standalone stream with no attached GET stream and no event store), which is pinned by test_a_server_request_no_client_can_receive_fails_the_call_instead_of_hanging.\n\nHow to fix. Delete ", or a request-scoped one in JSON-response mode" from the last bullet so it reads "...can reach no client at all (no attached stream and nothing storing it) fails the calling handler with CONNECTION_CLOSED...". Bullet 1 already documents the JSON-mode request-scoped case correctly, so no replacement text is needed. Docs-only: nothing breaks at runtime, hence nit severity.
Sorry, something went wrong.
claude[bot] reacted with thumbs up emoji
claude[bot] reacted with thumbs down emoji
All reactions
|
||
|
|
||
| ### `MCPServer.get_context()` removed | ||
|
|
||
| `MCPServer.get_context()` has been removed. Context is now injected by the framework and passed explicitly — there is no ambient ContextVar to read from. | ||
| Expand Down | ||
| Back | FazBrowse Home | New Git URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality🟡 The parenthetical at docs/migration.md:866 says the transport rework "does not touch the public surface", but the very next section (added by this same PR) documents that the rework removes the public StreamableHTTPServerTransport.connect() context manager and adds keyword-only app/lifespan_state constructor arguments — the PR description itself calls connect() removal "the one API removal". Consider rewording to something like "(whose only public-surface change is the removal of transport.connect(), covered there)" so a reader skimming the "unchanged" section doesn't conclude no migration is needed.
Extended reasoning...What the issue is. This PR edits the sentence at docs/migration.md:866 (in the "Streamable HTTP: session manager, EventStore, and stateless mode unchanged" section) to read: "Beyond the constructor parameters that moved to run()/streamable_http_app(), the lifespan change above, and the transport rework in the next section (which does not touch the public surface), the server-side Streamable HTTP API is as in v1". The parenthetical claims the transport rework has no public-surface impact.
Why it's contradicted three paragraphs later. The very next section this PR adds — "Streamable HTTP: StreamableHTTPServerTransport is driven per request, not per stream" (starting at line 875) — opens with: "StreamableHTTPServerTransport no longer exposes a connect() context manager…" and documents the new keyword-only app / lifespan_state constructor arguments, complete with a Before/After migration snippet for users who called transport.connect() by hand. The PR's own description calls the connect() removal "the one API removal" and checks the Breaking-change box, and per AGENTS.md the change is (correctly) documented in docs/migration.md — so by the PR's own accounting the rework does touch the public surface.
Step-by-step walk-through of the contradiction. (1) A v1 user who hand-constructed a transport and drove it via async with transport.connect() reads the migration guide top to bottom. (2) They reach the "unchanged" section, which tells them the transport rework in the next section "does not touch the public surface" — signalling the next section is internal detail they can skip. (3) They skip it and miss the one migration they actually need: moving from transport.connect() to StreamableHTTPSessionManager (or streamable_http_app()). (4) On upgrade, their code fails with AttributeError: 'StreamableHTTPServerTransport' object has no attribute 'connect', and the guide's own framing told them nothing applied to them.
The defensible readings don't hold up. One could argue "public surface" means the module-level exports the bulleted list enumerates (EventStore, EventMessage, EventCallback, etc.), which are indeed unchanged — and StreamableHTTPServerTransport is not in mcp.__init__'s __all__. But the sentence structure has the parenthetical directly modifying "the transport rework", not the export list; and the doc's own next section treats connect() as an exposed public API being removed (it provides a migration snippet, which only makes sense for public API). If the rework genuinely didn't touch the public surface, it wouldn't need to be listed as an exception to "as in v1" at all.
Impact. No code behaviour is affected — this is purely a documentation-accuracy issue. The breaking change is fully documented in the adjacent section, so a careful reader still finds it; the risk is limited to a skimming reader taking the "unchanged" section's parenthetical at face value and skipping the section that applies to them.
How to fix. Drop or reword the parenthetical, e.g.: "…and the transport rework in the next section (whose only public-surface change is the removal of transport.connect(), covered there), the server-side Streamable HTTP API is as in v1". A one-line edit; no other text needs to change.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.