Route replies and key sessions by a per-message dispatch context - #264
Merged
Conversation
added 5 commits
July 6, 2026 18:37
A message's origin (connection, transport session id) and its reply path currently live in ambient server state stamped at accept time, which is wrong whenever connections interleave. MessageDispatchContext makes the origin travel with the message instead: the filter that parses a message constructs a stack-scoped context and dispatches through new context-carrying handler hooks, pairing decode and encode on the same connection. Defaults forward to the existing context-free hooks, so this commit changes no behavior; producers and the server migrate next. Distinct method names (onRequestWithContext) rather than overloads keep every existing implementation clear of -Woverloaded-virtual hiding.
Each adapter between the JSON-RPC filter and the application callbacks now forwards the per-message context instead of dropping it: the stdio, protocol-detection, enhanced and factory-header adapters, the JSON-RPC-to-protocol bridge, the application-base adapter, and the connection manager. The HTTP+SSE composite supplies its own richer context carrying the SSE stream id and a reply sink that writes to the origin POST connection, where its onWrite already decides the wire form (SSE-registry reroute or HTTP framing). No behavior change yet: the server still consumes the context-free hooks, which every context-carrying default forwards to.
…264) The server previously resolved sessions and wrote responses against an ambient current-connection pointer stamped at accept time, which is the most recently accepted connection — not the one the message arrived on. With concurrent connections that cross-wires replies (accept A, accept B, A's request answered on B), attributes a request to another connection's session on the connection-keyed fallback, and an unrelated close nulls the pointer so a live connection's response is silently skipped. onRequest/onNotification now consume the per-message context: the session fallback keys on the origin connection, the reply goes out through the context's return path, and a failed send is logged and counted instead of dropped. This also retires the response broadcast to the first connected stdio manager, which double-sent every HTTP response onto stdio when both transports were active. The context-free hooks remain as an explicitly degraded fallback for out-of-tree producers and warn when hit.
Both request-scoped ambient members are now dead: session keying and reply routing read the per-message dispatch context, so the accept-time current_connection_ stamp, its close-path null-out, and the announce-then-dispatch onTransportSessionBound protocol (interface method, server binding member, and the composite filter's three announce sites) all go away. The server-mode integration test now asserts the binding that travels with each message's context instead of the last announced value, and seeds a sentinel that would surface if dispatch ever regressed to the context-free path.
New integration suite drives a real server over real TCP sockets with two concurrent plain-HTTP clients and pins the fixed failure modes: a request is answered on the connection it arrived on while the other connection stays silent (the accept-time ambient pointer answered on the most recently accepted connection instead), an unrelated close no longer drops a live connection's response, and interleaved rounds keep every request/response pair on its own connection. Filter-level tests pin the context contract at the source: a parsed message dispatches through the context-carrying hook (never the context-free fallback), an unwired filter reports a null origin and empty transport session id rather than stale values, and a reply attempt with no connection surfaces an error instead of silently succeeding.
caleb2h
changed the title
caleb2h
approved these changes
Jul 7, 2026
gophergogo
pushed a commit
that referenced
this pull request
Jul 7, 2026
A message's origin (connection, transport session id) and its reply path currently live in ambient server state stamped at accept time, which is wrong whenever connections interleave. MessageDispatchContext makes the origin travel with the message instead: the filter that parses a message constructs a stack-scoped context and dispatches through new context-carrying handler hooks, pairing decode and encode on the same connection. Defaults forward to the existing context-free hooks, so this commit changes no behavior; producers and the server migrate next. Distinct method names (onRequestWithContext) rather than overloads keep every existing implementation clear of -Woverloaded-virtual hiding.
gophergogo
pushed a commit
that referenced
this pull request
Jul 7, 2026
Each adapter between the JSON-RPC filter and the application callbacks now forwards the per-message context instead of dropping it: the stdio, protocol-detection, enhanced and factory-header adapters, the JSON-RPC-to-protocol bridge, the application-base adapter, and the connection manager. The HTTP+SSE composite supplies its own richer context carrying the SSE stream id and a reply sink that writes to the origin POST connection, where its onWrite already decides the wire form (SSE-registry reroute or HTTP framing). No behavior change yet: the server still consumes the context-free hooks, which every context-carrying default forwards to.
gophergogo
pushed a commit
that referenced
this pull request
Jul 7, 2026
…264) The server previously resolved sessions and wrote responses against an ambient current-connection pointer stamped at accept time, which is the most recently accepted connection — not the one the message arrived on. With concurrent connections that cross-wires replies (accept A, accept B, A's request answered on B), attributes a request to another connection's session on the connection-keyed fallback, and an unrelated close nulls the pointer so a live connection's response is silently skipped. onRequest/onNotification now consume the per-message context: the session fallback keys on the origin connection, the reply goes out through the context's return path, and a failed send is logged and counted instead of dropped. This also retires the response broadcast to the first connected stdio manager, which double-sent every HTTP response onto stdio when both transports were active. The context-free hooks remain as an explicitly degraded fallback for out-of-tree producers and warn when hit.
gophergogo
pushed a commit
that referenced
this pull request
Jul 7, 2026
Both request-scoped ambient members are now dead: session keying and reply routing read the per-message dispatch context, so the accept-time current_connection_ stamp, its close-path null-out, and the announce-then-dispatch onTransportSessionBound protocol (interface method, server binding member, and the composite filter's three announce sites) all go away. The server-mode integration test now asserts the binding that travels with each message's context instead of the last announced value, and seeds a sentinel that would surface if dispatch ever regressed to the context-free path.
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
2 participants
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.
Summary
Retires the server's ambient
current_connection_/current_transport_session_id_members. A message's origin (connection + transport session id) and its reply path now travel with the message as aMessageDispatchContext, built stack-scoped by the filter that parsed it and passed through the callback chain to the server.Fixes, in order of the commit stack:
MessageDispatchContext+ context-carrying hooks (onRequestWithContext/onNotificationWithContext) onMcpProtocolCallbacksandJsonRpcProtocolFilter::MessageHandler, with default forwarding to the legacy hooks. Distinct names keep all existing implementers clear of-Woverloaded-virtual. No behavior change.onTransportSessionBoundannounce-then-dispatch protocol are deleted; a stale binding is now unrepresentable rather than cleared by discipline.Testing
Full suite: 134/134 pass. New suites:
McpServerRequestRoutingTest, context-contract cases inJsonRpcProtocolFilterTest, and the server-mode binding test now asserts the id that travels with each message's context.Known follow-ups (from post-merge review, will be addressed separately)
ProtocolCallbackBridge(config-drivenjson_rpc.dispatcherchains) and the four chainable QoS filters do not yet forward the context, so those chains fall to the degraded legacy path.sendNotificationToSessioninto the "no push channel" branch — stdio server-push needs a conn-manager-aware route.DirectJsonRpcCallbacksdefinitions (stdio factory / json_rpc_filter_factory.h) now diverge in declaration order — ODR cleanup needed.