fix: reject replies from retired streamable http sessions by nickcoai · Pull Request #1206 · modelcontextprotocol/rust-sdk

nickcoai · 2026-08-23T14:59:26Z

Summary

Fix the late-handler reply issue from Dale's review.

This PR is based on main, independently of the worker refactor in #1205. The late-handler routing issue predates #1186. Whichever follow-up lands second will need a rebase to combine their transport changes.

Record the local transport and session generation when an inbound request is handed to the service.
Carry that origin through automatic responses and errors. Do not send an old handler's reply to a replacement session, including during shutdown draining.
Check handler identity when completing a request. If a new session reuses its request id, the old handler cannot remove or cancel the new one.
Keep the origin local. There are no wire-format changes or new public transport methods.

Scope

This protects automatic service replies, including request-association errors. Manually constructed raw transport replies have no captured origin.

Transport wrappers must preserve request extensions. Wrappers that defer the inner send within the same future retain the reply context; detached tasks must construct the inner send before spawning. Serializing requests or deferring send construction to a detached task loses the private context.

The tests cover late success and error replies, reused request ids, shutdown draining, and recovery during a pending reply send.

Validation

Rebased on main at 50fba5c2848d3ef9d0df0efacb8a7d001914f40e (October 17).
Exact head e575decaa047d9531fd918bb7afc61d074a0c9fa is green in CI, including all-features tests, non-local tests, clippy, MSRV, public API/semver checks, coverage, and examples. CodeQL also passed.

Local checks passed: 283 library tests with client,transport-streamable-http-client, all 24 streamable-http concurrency tests, nightly formatting, and git diff --check.

DaleSeo · 2026-09-09T01:08:57Z


+    /// Check both the captured generation and, for replies, the originating transport.
+    #[cfg(feature = "transport-streamable-http-client")]
+    pub(crate) fn is_current_control(&self, request: &WorkerSendRequest<W>) -> bool {


Is this supposed to be visible only within the crate?

DaleSeo · 2026-09-09T01:10:36Z

    pub responder: tokio::sync::oneshot::Sender<Result<(), W::Error>>,
    cancellation: Option<Arc<RequestCancellationRegistration>>,
    control_generation: u64,
+    #[cfg(feature = "transport-streamable-http-client")]


Should the origin only be stored when transport-streamable-http-client is enabled?

DaleSeo · 2026-09-09T01:11:37Z

Thanks for addressing my feedback, @nickcoai. I have a couple of questions before we move forward.

DaleSeo · 2026-09-09T01:28:01Z

+        }
+    }
+
+    #[cfg(all(feature = "transport-worker", not(feature = "local")))]


Do we not want to run these tests with --all-features?

DaleSeo · 2026-09-09T01:30:01Z

+            // Recovery may retire the origin between the initial check and the
+            // worker's dispatch check. Dropping that reply must not stop a drain
+            // from sending later, current-session replies.
+            if self.is_current() { result } else { Ok(()) }


so the drain can tell "dropped because stale" from "the transport just broke".

Suggested change

if self.is_current() { result } else { Ok(()) }

match result {

Err(error) if !self.is_current() => {

tracing::debug!(%error, "discarding retired reply's send failure");

Ok(())

}

result => result,

}

DaleSeo · 2026-10-05T18:47:45Z

This has merge conflicts with main now. Could you rebase onto the latest main and resolve them? Thanks!

Keep the inbound transport generation with automatic handler responses and errors. Reject replies after session recovery, including during the shutdown drain, without adding wire metadata. Match the completing handler's token before removing its request id so an old handler cannot cancel a replacement that reuses the id. Cover late replies, id reuse, scoped send context, and shutdown races.

DaleSeo · 2026-10-19T14:42:22Z

+            let result = send.await;
+            // Recovery may retire the origin between the initial check and the
+            // worker's dispatch check. Dropping that reply must not stop a drain
+            // from sending later, current-session replies.
+            if self.is_current() { result } else { Ok(()) }


Once the origin retires, this turns every send error into Ok(()), which also hides real transport failures that happen during recovery. Could the streamable HTTP worker return Ok(()) for stale replies, like the stale branch in streamable_http_client.rs already does for cancellations? Then we could remove this re-check and its comment.

Suggested change

let result = send.await;

// Recovery may retire the origin between the initial check and the

// worker's dispatch check. Dropping that reply must not stop a drain

// from sending later, current-session replies.

if self.is_current() { result } else { Ok(()) }

send.await

DaleSeo · 2026-10-19T14:42:22Z

+        #[tokio::test]
+        async fn shutdown_drain_preserves_current_handler_after_old_response_or_error() {
+            for error in [false, true] {
+                for replacement_id in [8, 7] {


The nested loops put four scenarios in one test, with a 5-second timeout for each step. If one case hangs, it hides the others. Could we use rstest values to make each combination its own test?

Suggested change

#[tokio::test]

async fn shutdown_drain_preserves_current_handler_after_old_response_or_error() {

for error in [false, true] {

for replacement_id in [8, 7] {

#[rstest::rstest]

#[tokio::test]

async fn shutdown_drain_preserves_current_handler_after_old_response_or_error(

#[values(false, true)] error: bool,

#[values(8, 7)] replacement_id: i64,

) {

{

{

DaleSeo · 2026-10-19T14:42:22Z

+    #[test]
+    fn old_handler_completion_preserves_reused_id_owner() {
+        for error in [false, true] {


We could use rstest here as well.

github-actions Bot added T-test Testing related changes T-core Core library changes T-transport Transport layer changes labels Aug 23, 2026

This was referenced Aug 23, 2026

fix: allow concurrent streamable http requests #1186

Merged

refactor!: keep streamable http state in its transport #1205

Closed

nickcoai marked this pull request as ready for review August 23, 2026 15:10

nickcoai requested a review from a team as a code owner August 23, 2026 15:10

nickcoai requested a review from DaleSeo August 23, 2026 15:10

Base automatically changed from agent/configurable-streamable-http-parallelism to main August 24, 2026 13:43

nickcoai force-pushed the agent/streamable-http-session-origin branch from 6e6416c to 8477d25 Compare September 2, 2026 22:08

DaleSeo reviewed Sep 6, 2026

View reviewed changes

Comment thread crates/rmcp/src/service.rs Outdated

Comment thread crates/rmcp/src/service.rs

nickcoai force-pushed the agent/streamable-http-session-origin branch from 8477d25 to ad88b6f Compare September 8, 2026 21:06

DaleSeo reviewed Sep 9, 2026

View reviewed changes

nickcoai force-pushed the agent/streamable-http-session-origin branch from ad88b6f to 205104e Compare October 9, 2026 23:20

nickcoai force-pushed the agent/streamable-http-session-origin branch from 205104e to e575dec Compare October 19, 2026 09:00

DaleSeo reviewed Oct 19, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix: reject replies from retired streamable http sessions - #1206

fix: reject replies from retired streamable http sessions#1206
nickcoai wants to merge 1 commit into
mainfrom
agent/streamable-http-session-origin

nickcoai commented Aug 23, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

DaleSeo Sep 9, 2026

Uh oh!

DaleSeo Sep 9, 2026

Uh oh!

DaleSeo commented Sep 9, 2026

Uh oh!

DaleSeo Sep 9, 2026

Uh oh!

DaleSeo Sep 9, 2026

Uh oh!

DaleSeo commented Oct 5, 2026

Uh oh!

DaleSeo Oct 19, 2026

Uh oh!

DaleSeo Oct 19, 2026

Uh oh!

DaleSeo Oct 19, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-            if self.is_current() { result } else { Ok(()) }
+            match result {
+                Err(error) if !self.is_current() => {
+                    tracing::debug!(%error, "discarding retired reply's send failure");
+                    Ok(())
+                }
+                result => result,
+            }

Conversation

nickcoai commented Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Scope

Validation

Uh oh!

Uh oh!

Uh oh!

DaleSeo Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

DaleSeo Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

DaleSeo commented Sep 9, 2026

Uh oh!

DaleSeo Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

DaleSeo Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

DaleSeo commented Oct 5, 2026

Uh oh!

DaleSeo Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

DaleSeo Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

DaleSeo Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nickcoai commented Aug 23, 2026 •

edited

Loading