[v2] ClientSession runs on JSONRPCDispatcher; BaseSession removed#2838
Draft
maxisbey wants to merge 9 commits into
Draft
[v2] ClientSession runs on JSONRPCDispatcher; BaseSession removed#2838maxisbey wants to merge 9 commits into
maxisbey wants to merge 9 commits into
Conversation
- New CallOptions key cancel_on_abandon (default true): abandoning a request
(timeout or caller cancellation) sends notifications/cancelled unless the
caller opted out or the request carries resumption hints
- Bound the two shielded cancellation-path writes with a 5s deadline so a
wedged transport write cannot hang shutdown or a cancelled caller
- Capitalize the connection-closed fan-out message ("Connection closed")
- Pin the server-seat timeout contract in the interaction suite: a timed-out
server-initiated request is followed by notifications/cancelled
A raising notification handler ran as a bare task in the dispatcher's task group, so its exception cancelled the read loop and every in-flight request. Wrap spawned handlers in the same containment boundary progress callbacks already have: log the failure and keep the connection serving.
Transports yield Exception items on the read stream for connection faults and parse errors; the dispatcher debug-logged and dropped them. An optional observer now receives them (awaited in the read loop, contained so a raising observer costs the item, not the connection). Unset keeps the old behavior.
TransportT now defaults to TransportContext (PEP 696, same pattern as shared/context.py), so omitting transport_builder no longer needs a dedicated overload to pin the type parameter.
ClientSession keeps its public surface (constructor, typed methods, manual initialize, context-manager lifecycle) but now owns a JSONRPCDispatcher instead of inheriting the v1 BaseSession receive loop. Server-initiated requests are answered through the existing callbacks via the closed-union parse; notifications validate-or-drop and tee to message_handler; transport exceptions reach message_handler through the dispatcher's stream-exception observer. A from_dispatcher constructor accepts a pre-built dispatcher for in-process embedding. mcp.shared.session shrinks to the surviving names: the ProgressFnT re-export and a typing-only RequestResponder stub for MessageHandlerFnT annotations. Behavior changes (deliberate, to be covered in the migration guide): - request ids count from 1; the progress token follows - timeouts use the dispatcher error text and send notifications/cancelled, so a timed-out server handler is interrupted instead of running on - responses with unknown ids are ignored per spec instead of surfacing a RuntimeError to message_handler - a raising request callback is answered with code 0 and the exception text - notification callbacks run concurrently (no completion-before-response) Three interaction-suite divergence entries are resolved and deleted, and the server-to-client cancellation requirement is now pinned by a passing test.
Adds tests for ServerMessageMetadata routing, related-request-id notifications, and params-absent inbound requests over direct dispatch, plus the migration-guide entry for the ClientSession dispatcher swap.
Transport teardown closes the standalone stream's send side first, so a writer parked in receive() ends on a clean end-of-stream; but when teardown lands while the writer is between dequeues, the next receive() raises ClosedResourceError, which fell into the catch-all and logged a traceback at ERROR level for a routine disconnect. Catch it and end quietly. A new test pins the close ordering that keeps the parked path clean.
Replaces the from_dispatcher classmethod: read_stream/write_stream become optional and dispatcher is a keyword-only alternative, with mutual exclusion validated at construction. Drops the __new__-based alternate constructor and its shared state-init helper.
Covers MCPError.from_jsonrpc_error and the context-stream sync close() methods, whose only exercisers died with BaseSession and its tests, and restructures three test handler arms that could never take their false branch.
aec8155 to
a51357c
Compare
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
V2 client receive path:
Transport -> JSONRPCDispatcher -> ClientSession callbacks, replacing theBaseSessionreceive loop. Companion to #2710, which did the same for the server seat;BaseSessionis now deleted.Motivation and Context
ClientSessionwas the last consumer of the v1BaseSessionengine. Moving it onto the dispatcher unifies both seats on one receive path and fixes the v1 client's structural problems: server-initiated requests were handled inline in the receive loop (a slow sampling callback blocked everything, a callback that sent a request deadlocked, and peer cancellation was unactionable), and a raising notification handler could take down the connection.ClientSession's public surface is unchanged: same constructor, same typed methods, manualinitialize(), same context-manager lifecycle. A new keyword-onlydispatcher=constructor argument accepts a pre-built dispatcher instead of the stream pair (e.g.DirectDispatcherfor in-process embedding).What changed
CallOptions.cancel_on_abandon+ suppression: abandoning a request (timeout or caller cancellation) sends a courtesynotifications/cancelled, unless the caller opted out (initialize, which the spec forbids cancelling) or the request carries resumption hints (the peer's work must survive for the resume)on_stream_exceptionobserver are contained at the dispatcher: a raising handler costs itself, never the connection (matching the TypeScript/C#/Go engines)ClientSessioninternals rewritten over the dispatcher;mcp.shared.sessionshrinks to a compatibility module (ProgressFnTre-export, typing-onlyRequestResponderstub forMessageHandlerFnTannotations)Parity bar
The transport-parametrized interaction suite passes. Three recorded divergences are now resolved and deleted from the requirements manifest (
protocol:timeout:sends-cancellation,protocol:cancel:late-response-ignored,protocol:cancel:server-to-client), and the server-to-client cancellation requirement is pinned by a new test passing over all three transports.Breaking Changes
Documented in
docs/migration.md(one grouped entry):Request 'tools/call' timed out, and a timed-out or abandoned request sendsnotifications/cancelled, interrupting the server handlerRuntimeErrortomessage_handlerMETHOD_NOT_FOUND(carrying the method indata), completing Fix unknown-method error code and add a protocol version registry #2836's contract on the client seatcode=0and the exception text (previously flattened toINVALID_PARAMS)send_requestbefore entering the context manager raisesRuntimeErrorimmediatelyFixes #2489, #2507, #2610, #2673. Supersedes #2490.
How Has This Been Tested?
Full suite (1741 tests) including the interaction suite over in-memory, SSE, and streamable HTTP; 100% branch coverage held on all changed files. New tests: courtesy-cancel wire pins and suppression, wedged-transport shutdown bound (trio virtual clock), notification-handler containment, stream-exception observer, the
dispatcher=constructor over direct dispatch, server-seat timeout cancellation, abandoned-server-request cancellation, standalone-stream teardown ordering.Types of changes
Checklist
AI Disclaimer