feat: JSONRPCDispatcher#2458
Draft
maxisbey wants to merge 7 commits intomaxisbey/v2-dispatcher-protocolfrom
Draft
feat: JSONRPCDispatcher#2458maxisbey wants to merge 7 commits intomaxisbey/v2-dispatcher-protocolfrom
maxisbey wants to merge 7 commits intomaxisbey/v2-dispatcher-protocolfrom
Conversation
Chunk (a) of JSONRPCDispatcher: constructor, _Pending/_InFlight/_JSONRPCDispatchContext, send_request/notify and helpers. run() is stubbed. The Dispatcher contract tests are now parametrized over a pair_factory fixture (direct + jsonrpc). The 9 jsonrpc cases are strict-xfail until run()/ _handle_request land in the next commits; once those pass, strict xfail flips to XPASS and forces removal of the marker. Factories return (client, server, close) so running_pair can shut down any implementation uniformly.
run() drives the receive loop in a per-request task group; task_status.started() fires once send_request is usable. _dispatch routes each inbound message synchronously (no awaits — send_nowait/_spawn only) to avoid head-of-line blocking. _spawn propagates the sender's contextvars via Context.run(tg.start_soon, ...) so auth/OTel set by ASGI middleware survive. _fan_out_closed wakes pending send_request waiters with CONNECTION_CLOSED on shutdown (called both post-EOF and in finally; idempotent). Wire-param extraction (progressToken, cancelled.requestId, progress fields) uses structural match patterns — runtime narrowing, no casts, no mcp.types model coupling; malformed input fails to match and the correlation is skipped. _handle_request is happy-path only here (run on_request, write response); the exception-to-wire boundary lands in the next commit. Dispatcher.run() Protocol gained a task_status kwarg (it's a contract-level guarantee). DirectDispatcher.run() updated to match. running_pair now uses tg.start so the test body runs only once the dispatcher is ready. 20 contract tests pass; the 2 needing the exception boundary are strict-xfail.
_handle_request is now the single exception-to-wire boundary: - MCPError -> JSONRPCError(e.error) - pydantic ValidationError -> INVALID_PARAMS - Exception -> INTERNAL_ERROR(str(e)), logged, optionally re-raised - outer-cancel (run() TG shutdown) -> shielded REQUEST_CANCELLED write, re-raise - peer-cancel (notifications/cancelled) -> scope swallows, no response written dctx.close() runs in an inner finally so the back-channel shuts the moment the handler exits. _write_result/_write_error swallow Broken/ClosedResourceError so a dropped connection during the response write doesn't crash the dispatcher. All 22 contract tests now pass against both DirectDispatcher and JSONRPCDispatcher; chunk-c xfail markers removed.
Covers behaviors with no DirectDispatcher analog: out-of-order response correlation, INTERNAL_ERROR over the wire, peer-cancel in interrupt and signal modes, CONNECTION_CLOSED on stream EOF mid-await, late-response drop, raise_handler_exceptions propagation, ServerMessageMetadata tagging on ctx.send_request, null-id JSONRPCError drop, ValidationError->INVALID_PARAMS, contextvar propagation via _spawn, and the defensive Broken/Closed/WouldBlock catches. Two small src tweaks for coverage: - _cancel_outbound: combine the two except arms into one tuple - _dispatch: pragma no-branch on the final case (match is exhaustive over JSONRPCMessage; the no-match arc is unreachable) 43 tests, 100% coverage on all PR2 modules, 0.15s wall-clock.
The pull_request branch filter meant the test/lint/coverage matrix only ran on PRs targeting main or v1.x. Stacked PRs (targeting feature branches) only got the conformance checks, which are continue-on-error and don't exercise unit tests. Removing the filter so the full matrix runs on every PR.
3.14: nested async-with arc misreporting on three create_task_group lines (the documented AGENTS.md case) — pragma: no branch. 3.11: lines after async-CM exit with pytest.raises mis-traced in one test — moved the asserts inside the context manager.
Follows the Outbound Protocol rename in the previous commit. Mechanical rename across JSONRPCDispatcher, _JSONRPCDispatchContext, and tests.
maxisbey
force-pushed
the
maxisbey/v2-jsonrpc-dispatcher
branch
from
86efb5c to
eb74b7c
Compare
9 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
1 participant
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.
Stacked on #2452. The production
Dispatcherimplementation over the existingSessionMessagestream contract.Motivation and Context
JSONRPCDispatcheris the JSON-RPC implementation of theDispatcherProtocol from #2452. It owns request-id correlation, the receive loop, per-request task isolation, cancellation/progress wiring, and the single exception-to-wire boundary. The MCP type layer (ServerRunner,Context,Client— later PRs) sits above it and sees only(ctx, method, params) -> dict; transports sit below and see onlySessionMessagereads/writes.The dispatcher is mostly MCP-agnostic — methods/params are opaque — but it intercepts
notifications/cancelled(must, since it owns the handler task scopes) andnotifications/progress(chosen, to avoid duplicating the correlation table in everyrun()caller). Those wire shapes are extracted with structuralmatchpatterns; a malformed payload simply fails to match and the correlation is skipped.How Has This Been Tested?
The 9 contract tests from #2452 are now parametrized over both
DirectDispatcherandJSONRPCDispatcher(via crossed in-memory streams) — same behavioral assertions, two implementations.tests/shared/test_jsonrpc_dispatcher.pyadds 21 JSON-RPC-specific tests for correlation, the exception boundary, peer-cancel in both modes,CONNECTION_CLOSEDfan-out, late-response handling,raise_handler_exceptions, contextvar propagation via_spawn, and the metadata routing. 43 tests total, 100% coverage on the new module, 0.15s wall-clock.Breaking Changes
None. New code only; nothing existing is wired in yet.
Dispatcher.run()gained atask_statuskwarg (Protocol-level guarantee thatsend_requestis usable oncerun()has started);DirectDispatcherupdated to match.Types of changes
Checklist
Additional context
Stack:
Notable design points (see commit messages for detail):
_Pendingbuffer is1not unbounded (aWouldBlockmeans the waiter already has an outcome and dropping is correct);_dispatchis fully synchronous (no awaits —send_nowait/_spawnonly) to avoid head-of-line blocking;_spawnpropagates the sender's contextvars viaContext.run(tg.start_soon, ...)so auth/OTel set by ASGI middleware reach handler tasks.AI Disclaimer