add request-scoped message dispatch for MCP servers - #528
Conversation
Part of dart-lang#162. Adds `handleRequestScopedMessage` and `MCPServerFactory`: each decoded JSON-RPC message is served on a fresh `MCPServer` instance (following dart-lang#524 (comment)), which gets `initialize(MCPServerInitialization)` with the per-request context and a completed `initialized` before the message is delivered. The shape follows the sketch in dart-lang#162 (comment): a server factory plus a handle function, no new transport abstraction. Each message round-trips through an in-memory string channel, so the exchange runs through the exact same `Peer` validation and dispatch path as a wire connection; a message-typed channel could drop the extra encode/decode later. Notifications emitted while handling are surfaced through a callback so a transport can decide how to deliver them, and server-to-client requests fail with an error inside their handler instead of deadlocking the exchange; real routing for those comes with the transport work. Also makes `MCPServerInitialization.clientInfo` (and `MCPServer.clientInfo`) nullable, since modelcontextprotocol/modelcontextprotocol#3002 made the per-request client info optional, and records the server implementation on responses under the reserved `io.modelcontextprotocol/serverInfo` result metadata key which the same spec change added. `RootsTrackingSupport` now tolerates the connection closing while `listRoots` is in flight, which the per-request teardown made easy to hit. Tests: - `dart_mcp`: analyze clean; format clean (dev SDK); 828/828 tests across chrome/vm and dart2wasm/dart2js/kernel/exe. 22 new request-scoped tests cover fresh-instance isolation, per-request capabilities, absent client info, server info stamping (including malformed metadata), notification passthrough, server-to-client request failure, malformed, legacy, and response-shaped message rejection, early shutdown, and initialization failure, plus a legacy handshake regression test for the nullable client info. - `dart_mcp_examples` (path-overrides the local `dart_mcp`): analyze clean. This PR does not add Streamable HTTP or any transport; wire formats, envelope validation, and `server/discover` land separately.
There was a problem hiding this comment.
Code Review
This pull request introduces support for request-scoped MCP servers by adding handleRequestScopedMessage and MCPServerFactory, allowing individual JSON-RPC messages to be served on fresh server instances. It also makes client information optional during initialization and updates roots tracking to handle connection closures gracefully. Feedback on the changes suggests safely casting the result of jsonDecode in request_scoped.dart to avoid potential runtime TypeErrors.
Package publishingIf you have publishing permissions, you can use the links below to publish the changes after merging this PR.
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
|
I am generally wondering how we are going to want to deal with roots in the new version, since we already know we can't rely on roots/list being sensible and need a fallback, but the fallback requires some kind of stateful session. I don't think that matters for this PR, just something I was thinking about while reviewing it. |
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with |
|
Addressed the review comments in 87d0746:
|
|
I think the RC splits this in two. The ask-the-client path is stateless now: server-to-client requests (roots/list included) became MRTR embeddings (SEP-2322), the server hands the roots/list back inside the result with an opaque |
|
The problem with no stateful roots is that we actually want them to be persistent because that allows us to keep a live LSP server open to those roots (faster analysis, less duplicate work per request). We can do this somewhat separately - keep that on the side and just deactivate inactive roots (we already do this anyways, so its really just moving code around). Maybe that would anyways be better - we already require roots to be given per request that needs them, and we can just drop the add/remove root functions entirely, and treat any root given by any request as an |
|
Agreed, and treating per-request roots as the input is what makes dropping add/remove safe rather than just tidier: the warm-LSP pool becomes a cache derived from those roots, not state you have to keep consistent. Timeout/LRU eviction can then only cost a cold re-analysis the next time a request names that root, never a wrong answer. It also makes the pool root-keyed rather than session-keyed, so two clients on the same workspace share one warm analyzer instead of each paying for their own. What you lose is the full root set up front, since a root only warms once a request touches it, but lazy warm-up seems fine for analysis and it turns the fallback into a cache miss instead of a consistency problem. |
jakemac53
left a comment
There was a problem hiding this comment.
just the one last comment
Follow-up to [this thread](#528 (comment)) in #528: `MCPBase`, `MCPClient.connectServer`, `MCPServerFactory`, and `stdioChannel` now operate on `StreamChannel<Map<String, Object?>>` instead of `StreamChannel<String>`, so JSON is encoded and decoded only at the transport edge rather than once more within the process. - `MCPBase` hands the channel to `Peer.withoutJson`. The channel type admits only JSON objects, so json_rpc_2 never receives a batch and never emits the `List` frames its batch support would answer one with. - `stdioChannel` still speaks newline-delimited JSON on the wire and now returns the decoded channel. The string-to-map adaptation is exposed separately as `jsonRpcChannel` for other `StreamChannel<String>` transports. - Behavior deltas worth calling out: - Frames which are not valid JSON, or which decode to something other than a JSON object, are answered at the transport edge by `jsonRpcChannel` instead of by json_rpc_2 inside the peer. The two existing invalid-JSON tests moved to `stdio_test.dart` accordingly, and neither those frames nor their error responses appear in `protocolLogSink` anymore (which now logs each message re-encoded as JSON, falling back to `toString` when a message cannot be encoded). If wire-level visibility of rejected frames turns out to matter for debugging, an optional log sink on `jsonRpcChannel` would be a small follow-up. - JSON-RPC batch frames now get a single invalid request error response with a null id and a message pointing at the 2025-06-18 removal. Batching was added in protocol version 2025-03-26 and removed in 2025-06-18; this package never sent batches itself. - `handleRequestScopedMessage` now stamps server info onto a copy of the response. With decoded channels the response embeds the very maps the handler returned — which may be retained by the handler or even unmodifiable, like `EmptyResult()`'s — where previously it saw private `jsonDecode` copies. - The URL elicitation retry path now restores a typed view of the error data itself. `RpcException.serialize` copies the data into an untyped map, and the JSON round trip previously re-laundered it to `Map<String, dynamic>` before user code saw it. Does not add any transport or change protocol behavior beyond the frame type: HTTP, `server/discover`, and roots are untouched. Since 0.6.0-wip has never been published, current `dart_mcp` users only see this when they opt into 0.6.0; `dart_mcp_server` pins `^0.5.0` and migrates with its own 0.6.0 dependency bump. ## Tests - New `stdio_test.dart` covers `jsonRpcChannel` (decode/encode round trip, parse error and non-object frames with null ids, batch rejection, and a server and a client each surviving invalid frames) plus `stdioChannel` at the byte level. - New request-scoped tests pin that server info is stamped onto unmodifiable results (the built-in `ping` path) without modifying the map the handler returned, and skipped for metadata with non-string keys. - A new test pins that a server survives a map which is not a valid message, now representable at the public channel seam. - `dart test -p chrome,vm -c dart2wasm,dart2js,kernel,exe` passes (218 tests in each of the four configurations). - `dart analyze --fatal-infos` and `dart format` (dev SDK) are clean here, and in `mcp_examples` resolved against this branch. Part of #162.
Part of #162.
Adds
handleRequestScopedMessageandMCPServerFactory: each decodedJSON-RPC message is served on a fresh
MCPServerinstance (following#524 (comment)), which gets
initialize(MCPServerInitialization)with the per-request context and acompleted
initializedbefore the message is delivered. The shape followsthe sketch in
#162 (comment): a server
factory plus a handle function, no new transport abstraction.
Each message round-trips through an in-memory string channel, so the exchange
runs through the exact same
Peervalidation and dispatch path as a wireconnection; a message-typed channel could drop the extra encode/decode later.
Notifications emitted while handling are surfaced through a callback so a
transport can decide how to deliver them, and server-to-client requests fail
with an error inside their handler instead of deadlocking the exchange; real
routing for those comes with the transport work.
Also makes
MCPServerInitialization.clientInfo(andMCPServer.clientInfo)nullable, since
modelcontextprotocol/modelcontextprotocol#3002 made
the per-request client info optional, and records the server implementation
on responses under the reserved
io.modelcontextprotocol/serverInforesultmetadata key which the same spec change added.
RootsTrackingSupportnowtolerates the connection closing while
listRootsis in flight, which theper-request teardown made easy to hit.
Tests:
dart_mcp: analyze clean; format clean (dev SDK); 828/828 tests acrosschrome/vm and dart2wasm/dart2js/kernel/exe. 22 new request-scoped tests
cover fresh-instance isolation, per-request capabilities, absent client
info, server info stamping (including malformed metadata), notification
passthrough, server-to-client request failure, malformed, legacy, and
response-shaped message rejection, early shutdown, and initialization
failure, plus a legacy handshake regression test for the nullable client
info.
dart_mcp_examples(path-overrides the localdart_mcp): analyze clean.This PR does not add Streamable HTTP or any transport; wire formats, envelope
validation, and
server/discoverland separately.