Repository navigation
fix(agents): refuse a stream under another run's id or a second time - #797
Open
justintime4tea wants to merge 6 commits into
Open
justintime4tea wants to merge 6 commits into
justintime4tea wants to merge 6 commits into
Conversation
|
justintime4tea
added this pull request to stack #798
October 9, 2026 00:32
justintime4tea
marked this pull request as ready for review
October 9, 2026 00:45
justintime4tea
force-pushed
the
justingross/GH-790-refuse-stream-misuse
branch
from
October 9, 2026 00:46
262ab78 to
b063bad
Compare
justintime4tea
force-pushed
the
justingross/GH-790-refuse-stream-misuse
branch
from
October 9, 2026 05:55
b063bad to
c339c83
Compare
justintime4tea
force-pushed
the
justingross/GH-790-refuse-stream-misuse
branch
from
October 9, 2026 14:05
c339c83 to
a42a20e
Compare
justintime4tea
force-pushed
the
justingross/GH-790-refuse-stream-misuse
branch
from
October 9, 2026 14:26
a42a20e to
c020de9
Compare
The envelope names a run by RunId, a UUID, while the task-local run context named it by the HTTP request id string, so the two could never be the same value. RunContext::id is now a RunId. The chat, A2A, and Slack handlers mint one before building the agent and use its string form as the request id, so every request-keyed registry (HITL approvals and their sweep, MCP cancellation, the A2A cancel map) keeps one value per run. begin_run, AgentRuntimeConfig, and RigBuilder's build_agent, build_streaming_agent_with_headers and build_streaming_agent_with_tools take a RunId, and an agent built without one mints its own. The orchestration factory streams under the run id it was built for, as Agent already did. RunContext::has_id compares a request id string against the run without allocating. Request ids become hyphenated UUIDs rather than req_<hex>, a2a_<task_id>, and slack_<channel>_<ts>; the A2A executor and the Slack runner log each run id beside the task or message it serves, at debug, so the two stay joinable. The orchestration RunId becomes a re-export of aura_events::RunId, so there is one run id type. Orchestration persistence still mints its own value for checkpoints and the park owner key; adopting the run's id there waits for the runtime, which owns resume. Fixes: GH-778 Ref: GH-578 Ref: GH-780
`orchestration_run()` fell back to a run under the config's id, or a fresh one when the config carried none, and it minted that fresh id on every call. An orchestrator built from a config without a run id and driven outside a run's scope therefore began its coordinator and each of its workers within a different run, and the doc's "the run the orchestration serves" named several. `Orchestrator::new` now mints the id once, as `OrchestratorFactory::new` does, writes it back to the config and keeps it on the orchestrator; `orchestration_run()` reads that. The note beside the persistence run id states the two ids as they are rather than as a change to come. Ref: GH-778
`RunContext::has_id` parsed the string it was given and compared UUIDs, so the same id in upper case, in simple form or as a URN named the run; `StreamClaim::claim` demands the run's own `Display` form, because that string is the key every request-keyed registry holds. The two answers to "does this string name this run" disagreed, and the lenient one is what the MCP registries use to route cancellation and tool events. `has_id` now compares the spelling, encoded into a stack buffer rather than allocated, so a run answers to one string everywhere. Ref: GH-778
A run's id is a bare UUID, so a log line that names it no longer says which A2A task or Slack message the run serves, and the line that tied them together was debug-only. Each ingress now opens its run's agent.stream root span with the run's id and its origin: the A2A task and context ids, or the Slack channel and message ts. Chat's span carries the run's id, which its HTTP span already records. A2A had no agent.stream span; its execution stream is now polled inside one, as chat's and Slack's runs already are. The Slack run id is minted when the message is queued, where its span is opened. Ref: GH-778
The comment in OrchestratorFactory::stream restated how an Agent picks its run, which StreamingAgent::stream already states as the contract both implementors keep. It now says only what is particular to the factory: the run it was built for, or a fresh one. Ref: GH-778
A built Agent streamed under the id it began with, whatever request_id it was handed, logging a mismatch at debug. A library caller streaming under its own id therefore had approvals, MCP tracking and the hook keyed under another one, and cancel_and_close_mcp with its own id matched nothing. A second stream reused the run: the first AgentRun's drop guard had cancelled its token, so the second stopped at its first hook, with no observer and the first's leftover tool-call ids. The orchestration factory had the same mismatch, streaming under the run id it was built with. Both now claim the run's one stream through StreamClaim. A request_id that is not the run's id, spelled as the run spells it, or a second call, returns a run whose stream yields one StreamRefused and does nothing. The id is checked before the claim is taken, so a misnamed call leaves the stream to the caller that names the run. Only the trait method claims: the orchestrator's transient retry re-streams a coordinator through the inherent stream_chat_with_depth, which is untouched. An OrchestratorFactory fixes its run id when built, minting one when the config names none, and the orchestration it spawns carries the same id. StreamingAgent::run_id reads the run an agent streams, so a caller holding only the trait object, as every builder returns, can stream an agent built without an id. It replaces the inherent run_id on Agent and OrchestratorFactory, and a refusal for another run points at it. StreamClaim is public, so a StreamingAgent outside aura keeps the same contract. The test mock does: a wrong id or a second stream gets the refusal a real agent gives, and its callers stream under its run_id(). The StreamingAgent::stream doc states the contract, and the doc examples build with the id they stream under. The server paths already do: chat completions, A2A and Slack each build for the run id they stream under, and stream once. Fixes: GH-790 Ref: GH-778
justintime4tea
force-pushed
the
justingross/GH-790-refuse-stream-misuse
branch
from
October 9, 2026 20:52
c020de9 to
a417f91
Compare
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
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.
StreamingAgent::streamon a builtAgentstreamed under the id it began with whateverrequest_idit was given, and a second call reused the already-cancelled run. The run-id layer (#796) had given the orchestration factory the same mismatch. Both now claim the run's one stream throughStreamClaim: arequest_idthat isn't the run's id (in the run's own spelling), or a second call, returns a run whose stream yields oneStreamRefusedand does nothing. The id is checked before the claim is taken. Only the trait method claims, so the coordinator's transient retry (stream_chat_with_depth) still re-streams. AnOrchestratorFactoryfixes its run id when built, minting one if the config names none.StreamingAgent::run_id()returns the run an agent streams, so a caller holding only the trait object a builder returns can stream an agent built withrun_id: None; it replaces the inherentrun_idonAgentand the factory.StreamClaimis public so aStreamingAgentoutside aura keeps the same contract; the test mock does, and its callers stream under itsrun_id(). The trait doc states the contract and the doc examples build with the id they stream under; the server paths (chat, A2A, Slack) already did.Verification
cargo +nightly fmt --check,cargo clippy --workspace --all-targets --all-features -- -D warnings, andcargo test --workspace(2,588 passed) on this layer.Fixes: GH-790
Ref: GH-778