fix: reject replies from retired streamable http sessions - #1206
Open
nickcoai wants to merge 1 commit into
Open
Conversation
github-actions
Bot
added
T-test
This was referenced Aug 23, 2026
nickcoai
marked this pull request as ready for review
August 23, 2026 15:10
Base automatically changed from
agent/configurable-streamable-http-parallelism
to
main
August 24, 2026 13:43
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.
nickcoai
force-pushed
the
agent/streamable-http-session-origin
branch
from
September 2, 2026 22:08
6e6416c to
8477d25
Compare
DaleSeo
reviewed
Sep 6, 2026
Comment on lines
+1339
to
+1341
| .extensions() | ||
| .get::<crate::transport::worker::ResponseOrigin>() | ||
| .cloned(), |
Member
There was a problem hiding this comment.
I think we should take the origin out at capture instead. ReplyContext::from_request only clones it, so the token rides mem::swap into the public RequestContext::extensions.
Suggested change
| .extensions() | |
| .get::<crate::transport::worker::ResponseOrigin>() | |
| .cloned(), | |
| .extensions_mut() | |
| .remove::<crate::transport::worker::ResponseOrigin>(), |
| handlers.remove(id); | ||
| true | ||
| } else { | ||
| false |
Member
There was a problem hiding this comment.
The old Event::ToSink arm logged before discarding a response.
Suggested change
| false | |
| tracing::debug!(%id, "dropping response: request was cancelled or its id was reused"); | |
| false |
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
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.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
Validated at
8477d25, rebased ontomainat3b5ca4d:clientandtransport-streamable-http-client.mainat3b5ca4dand pass with this fix.Full all-features tests, clippy, and downstream adapter tests were not rerun after this rebase.