Skip to content

feat: Implement SEP-2260 require server requests to associate with client requests#1027

Closed
alexhancock wants to merge 1 commit into
mainfrom
alexhancock/sep-2260
Closed

feat: Implement SEP-2260 require server requests to associate with client requests#1027
alexhancock wants to merge 1 commit into
mainfrom
alexhancock/sep-2260

Conversation

@alexhancock

Copy link
Copy Markdown
Contributor

Implements SEP-2260.

Tracking issue: #873

What

Servers MUST send sampling/createMessage, elicitation/create, and roots/list requests only in association with an originating client request (e.g. during tools/call, resources/read, or prompts/get processing). Standalone server-initiated requests of these types are not supported. ping is excepted.

How

  • Added a tokio::task_local! (ORIGINATING_REQUEST) that is scoped around each inbound client-request handler in the serve loop, so any server→client request issued while handling a client request is recognized as associated.
  • Added Peer<RoleServer>::ensure_request_association, called from the server-side outbound request helpers (create_message, create_elicitation, create_elicitation_with_timeout, list_roots). When the negotiated protocol version is 2026-07-28 or newer and the call is made outside a request-handler scope, it returns an error instead of sending an unassociated request.
  • ping uses a separate path and is not affected.
  • Gated behind ProtocolVersion::V_2026_07_28 so older peers keep legacy behavior.

Notes

  • Client-side behavior is unchanged (the SEP requires no client changes).
  • The canonical nested pattern (sampling/elicitation/roots inside a tool/resource/prompt handler) remains fully supported.

Test

Added test_sep_2260_request_association verifying nested sampling inside call_tool succeeds while a standalone (spawned) sampling request outside the handler scope is rejected under 2026-07-28.

@alexhancock
alexhancock requested a review from a team as a code owner July 22, 2026 18:55
@github-actions github-actions Bot added T-test Testing related changes T-core Core library changes T-service Service layer changes labels Jul 22, 2026
@alexhancock
alexhancock force-pushed the alexhancock/sep-2260 branch 3 times, most recently from ccdbd48 to 070220b Compare July 22, 2026 20:11
@alexhancock
alexhancock force-pushed the alexhancock/sep-2260 branch from 070220b to d772e4c Compare July 22, 2026 20:44
}
let strict =
peer_info.is_some_and(|info| info.protocol_version >= ProtocolVersion::V_2026_07_28);
if strict && !in_request_handler_scope {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handshake-free services can enter serve_inner with peer_info == None and take the protocol version from each request's metadata

let (peer, peer_rx) = Peer::new(id_provider, None);
peer.require_request_metadata();
let context = RequestContext {
ct: ct.child_token(),
id: id.clone(),
meta: std::mem::take(request.get_meta_mut()),
extensions: std::mem::take(request.extensions_mut()),
peer: peer.clone(),
};
let response = match service.handle_request(request, context).await {
Ok(result) => ServerJsonRpcMessage::response(result, id),
Err(error) => ServerJsonRpcMessage::error(error, Some(id)),
};
transport.send(response).await.map_err(|error| {
ServerInitializeError::transport::<T>(error, "sending negotiated request response")
})?;
return Ok(serve_inner(service, transport, peer, peer_rx, ct));

Here that makes strict false even after the stateless lifecycle has been selected. How should an unassociated send be rejected on that supported path when no handshake-derived PeerInfo exists?

Comment on lines +352 to +356
let originating_request = crate::service::ORIGINATING_REQUEST
.try_with(|id| id.clone())
.ok();
let handle = tokio::spawn(async move {
let result = future.await;
let result = run_task_operation(originating_request, future).await;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TaskManager::spawn returns the task to the handler immediately, so this child future can con
tinue after the originating JSON-RPC response has closed. Re-establishing the task-local makes
a later direct sampling or elicitation pass the guard anyway.

Comment on lines +1423 to +1426
let handler_id = id.clone();
spawn_service_task(async move {
let result = service
.handle_request(request, context)
let result = ORIGINATING_REQUEST
.scope(handler_id, service.handle_request(request, context))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Tokio task-local values are not inherited by tokio::spawn, a handler that spawns sampling work and awaits its JoinHandle before returning is rejected even though the parent client request is still in flight?

@alexhancock

Copy link
Copy Markdown
Contributor Author

These changes are now included in #1029 with more of what we need! Closing this one

gocamille added a commit to gocamille/rust-sdk that referenced this pull request Jul 23, 2026
…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.
alexhancock added a commit that referenced this pull request Jul 23, 2026
…E stream (#1029)

* feat: implement SEP-2260 require server requests to associate with client requests

* feat(service): attach originating request id to in-handler outbound requests (SEP-2260)

Reuses the ORIGINATING_REQUEST task-local from #1027; the marker rides
the request's non-serialized Extensions so the streamable HTTP server
can route associated requests to the originating SSE stream.

* feat(transport): route associated server requests to originating SSE stream (SEP-2260)

* test: end-to-end SEP-2260 stream routing over streamable HTTP

* test: register SEP-2260 routing test with required-features; drop dead list_tools

* docs: document SEP-2260 association requirement and stream routing

* docs: fix redundant explicit link on OriginatingRequestId

* docs: note marker is role-agnostic; add reason to deprecated expect

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.

* docs: condense inline comments to repo convention

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).

* docs: condense SEP-2260 rustdoc sections

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.

* docs: single-source SEP-2260 caller requirements on OriginatingRequestId

* feat(service): reject unassociated server-to-client requests on the client (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.

---------

Co-authored-by: Alex Hancock <alexhancock@block.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2026-07-28 T-core Core library changes T-service Service layer changes T-test Testing related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants