feat: route SEP-2260 associated server requests to the originating SSE stream#1029
Conversation
alexhancock
left a comment
There was a problem hiding this comment.
Left a comment about a SHOULD in the spec, but looking good to me.
| } | ||
| } | ||
|
|
||
| fn enforce_request_association( |
There was a problem hiding this comment.
Is there a reason we don't also do this in service/client.rs? Trying to figure out where this part of the spec is implemented:
Clients recieving server-to-client requests with no associated outbound request SHOULD respond with a -32602 (Invalid Params) error.
I think it may be missing?
There was a problem hiding this comment.
Yes @alexhancock it wasn't implemented anywhere, good catch. SEP-2260 defines no wire field, so the exact association is only seen at the transport layer and our streamable HTTP client merges the standalone GET stream and POST-response SSE streams into one flat stream before the service layer sees anything. On stdio there's no channel separation at all.
In f0b7fd3 if the client receives one of these requests with no outbound request in flight, it now responds with -32602. This only applies when the negotiated protocol is 2026-07-28+, and ping is exempt. The check lives in a new receive-side ServiceRole::enforce_peer_request_association hook, mirroring the send-side one from #1027.
For now the check is intentionally loose: with a request in flight we can't tell which outbound request the server request belongs to, so we accept it. Doing this properly (rejecting these requests when they arrive on the standalone GET stream, even while unrelated requests are in flight) needs the client transport to keep track of which stream each message came in on. If it's helpful I can file a follow-up so this PR stays scoped to routing plus the coarse check.
There was a problem hiding this comment.
That sounds good to me for now, yes. Thanks for the updated commit.
…equests (SEP-2260) Reuses the ORIGINATING_REQUEST task-local from modelcontextprotocol#1027; the marker rides the request's non-serialized Extensions so the streamable HTTP server can route associated requests to the originating SSE stream.
…stream (SEP-2260)
Review follow-ups: the OriginatingRequestId rustdoc now states the marker is attached for both roles with only the server transport reading it, and the test's deprecation suppression carries a reason per house style.
Trims multi-line inline comments added in this branch down to the terse one-to-two-line style used elsewhere in the codebase, keeping only the non-obvious rationale (spec references, fallback invariant, version choice).
Tightens the duplicated per-method association section from eleven lines to five, and trims the marker and SessionManager docs to the same information in fewer words.
…lient (SEP-2260) Implements the client receive-side of SEP-2260: restricted server requests (sampling/createMessage, roots/list, elicitation/create) received while the client has no outbound request in flight are answered with -32602 invalid params instead of being dispatched to the handler. Gated on negotiated protocol >= 2026-07-28; ping is exempt. With a request in flight we cannot tell which one the server request belongs to (SEP-2260 defines no wire field), so this is a deliberate under-approximation of the spec's SHOULD; exact stream-based enforcement for streamable HTTP needs receive-side provenance plumbing and is left as a follow-up.
f0b7fd3 to
4039fd9
Compare
|
Very good. Thanks @gocamille ! |
Builds on #1027 to implement SEP-2260's transport-layer requirement: server -> client requests associated with a client request are delivered on the originating request's SSE stream, not the standalone GET stream.
Tracking issue: #873
Motivation and Context
#1027 enforces that
sampling/createMessage,elicitation/create, androots/listare only sent while handling a client request. On streamable HTTP, the transport still delivered all server-initiated requests on the standalone GET stream, so the association was not observable by clients. SEP-2260 requires associated requests to ride the originating request's stream.How
OriginatingRequestId, a marker in the outbound request's non-serializedExtensions, attached at the send choke point (Peer::send_request_with_option) wheneverORIGINATING_REQUEST(from feat: Implement SEP-2260 require server requests to associate with client requests #1027) is in scope. SEP-2260 defines no wire field, so nothing changes on the wire.LocalSessionWorker::resolve_outbound_channelreads the marker and routes the request to the originating request's SSE channel via the existingresource_routerwithclose: falseso the stream stays open for the response.tracing::warn!rather than dropping the request.SessionManagerthat implementations serializing messages between processes lose the marker and need their own association mechanism; documented on the restricted request helpers that the association does not crosstokio::spawn.-32602(Invalid Params), gated on negotiated protocol2026-07-28+,pingexempt — the "Clients receiving server-to-client requests with no associated outbound request SHOULD respond with a -32602" line from SEP-2260. With a request in flight we cannot tell which one the server request belongs to (no wire field), so this is a deliberate under-approximation; exact stream-based enforcement is a follow-up.How Has This Been Tested?
service.rs) and for the three routing outcomes (local.rs).test_sep_2260_stream_routing: real streamable HTTP server; asserts an in-handlerelicitation/createappears on the originatingtools/callPOST SSE stream and never on the standalone GET stream. Pins protocol2025-11-25since SEP-2567 serves2026-07-28+statelessly with no standalone GET stream to test against.2026-07-28+): unassociatedsampling/createMessagerejected with-32602, permissive behavior preserved at2025-11-25, unassociatedpingstill answered.Breaking Changes
None. New public type
rmcp::service::OriginatingRequestId; routing behavior changes only in the bundledLocalSessionManager. CustomSessionManagerimplementations are unaffected (documented limitation).Types of changes
Checklist
Additional context
The marker is role-agnostic. The completed-originating-request fallback is deliberate as enforcement already happened at the service layer and dropping a committed in-flight request would hang the handler awaiting a reply, so we deliver on the standalone stream and
warninstead.One design note: the TypeScript SDK threads this association explicitly as
relatedRequestIdon transport send options. The equivalent here means a breaking change toTransport/PeerSinkMessageto carry data only this transport consumes, so this PR uses the existingExtensionsside-channel instead. But if those traits ever take a breaking revision, promoting the association to an explicit field would be a follow-up (and it would also let serializing session managers comply with SEP-2260).