fix(stella-core,stella-pipeline): close the sub-agent bracket on cancellation (#1954) - #1978
Merged
Conversation
…ellation
A caller that drops a sub-agent future mid-flight — a latency ceiling, a
hard cancel — left `Started` open forever, so every ceiling-bearing caller
had to forge its own `Finished` and could only guess `steps: 0`: the
committed-call count lived inside the dropped turn.
`CancelBracket` moves that obligation into the primitive. It is armed
between `Started` and the normal `Finished`, and drop order does the
sequencing — the turn future drops first, so the engine's cancel guard has
already emitted the abandoned call's `UsageIncomplete { Cancelled }`
envelope and `SettleChildOnDrop` has folded the money back before the
bracket closes. It therefore reports only committed steps and cost, tallied
by `child_sender` as each `StepUsage` crosses the boundary.
The research stage drops its forged bracket accordingly; the next caller
with a ceiling inherits the fix instead of repeating the bug.
Closes #1954
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
Reviewer's GuideAdds a cancellation-aware sub-agent completion mechanism in stella-core so that dropped sub-agent futures still emit a truthful Finished bracket with committed steps and cost, replaces the pipeline’s locally-forged Finished on timeout with this primitive behavior, and adds focused tests plus helpers to witness the fix in both core and pipeline research stages. Sequence diagram for sub-agent cancellation with CancelBracketsequenceDiagram
actor Pipeline
participant TokioTimeout as tokio_time_timeout
participant Engine as Engine
participant CancelBracket as CancelBracket
participant CommittedTally as CommittedTally
participant Events as EventSender
Pipeline->>TokioTimeout: timeout(ceiling, engine.run_sub_agent_with_sender(...))
alt normal_completion
TokioTimeout->>Engine: run_sub_agent_with_sender(host, spec, carve, budget, events)
Engine->>CommittedTally: new CommittedTally::default()
Engine->>CancelBracket: create CancelBracket{ events, agent_id, tally, armed=true }
Engine->>Engine: run_child_turn(host, spec, carve, budget, events, &tally)
Engine->>Events: send(AgentEvent::SubAgent Finished{ steps: tally.steps(), cost_usd: tally.cost_usd() })
Engine->>CancelBracket: armed = false
TokioTimeout-->>Pipeline: Ok(SubAgentOutcome)
else timeout_or_cancellation
TokioTimeout-->>Pipeline: Err(_elapsed)
Note over Engine,CancelBracket: caller drops sub-agent future
CancelBracket-->>Events: Drop::drop() -> send(AgentEvent::SubAgent Finished{ status: Incomplete, steps: tally.steps(), cost_usd: tally.cost_usd() })
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
What
A caller that drops a sub-agent future mid-flight — a latency ceiling, a hard cancel — left
Startedopen forever. Every ceiling-bearing caller therefore had to forge its ownFinished, and could only guesssteps: 0, because the committed-call count lived inside the dropped turn. The pre-plan research stage (#1778, merged as 1bdf2da) was the first pipeline caller to hit this and forged exactly such a bracket.CancelBracketmoves the obligation into the primitive (crates/stella-core/src/subagent.rs) — design 2 of the two the issue proposed, so the next caller with a ceiling inherits the fix instead of repeating the bug. This is the same argument that moved the goal verifier ontoEngine::run_sub_agent.Drop order does the sequencing, which is what makes the numbers honest: the in-flight turn future is declared after the guard, so it drops first — the engine's own cancel guard has already emitted the abandoned call's
UsageIncomplete { Cancelled }envelope andSettleChildOnDrophas already folded the money back by the time the bracket closes. The bracket therefore reports only what was committed; the in-flight call's usage rides its own envelope and is never guessed at here.The two committed numbers travel as one
CommittedTallytype rather than two looseArcs — a bracket reporting a step count without the cost that produced it would be half an answer, and bundling them also keepsrun_child_turnunder the argument cap without an#[allow].research_stage.rsdrops its forged bracket accordingly.Witness
Both fail on the old code and pass with the change — verified by checking out
origin/main's copies of the two production files and re-running:subagent::tests::a_cancelled_child_closes_its_bracket_with_committed_steps_and_cost— on old code the event stream containsStartedwith noFinishedat all. (The dump also showsUsageIncomplete { Cancelled }already present on old code, which is the evidence that only the bracket needed owning.)pipeline::tests::research::a_child_past_the_ceiling_closes_its_bracket_with_committed_steps— the issue's verbatim scenario: balanced bracket,UsageIncompletewith reasoncancelled, andFinished.stepsequal to the committedStepUsagecount. Old code fails on"Finished.steps is the committed StepUsage count, not a forged zero".No protocol change was needed —
UsageIncompleteReason::Cancelledalready exists, so there is no wire-schema regeneration.Verification
mainis currently red (see #1971), so these were run on a local merge of this branch withunbreak-main-pipeline-tests:cargo test -p stella-core -p stella-pipeline— 0 failurescargo clippy -p stella-core -p stella-pipeline --all-targets -- -D warnings— cleancargo fmt --all -- --check— cleanscripts/check-file-size.shon this branch alone — OK, none grewThis PR's CI will stay red until #1971 lands, for reasons that have nothing to do with this diff.
Closes #1954
Summary by Sourcery
Ensure sub-agent cancellations close their Started/Finished bracket with accurate committed steps and cost, and rely on the core primitive rather than callers to emit synthetic finishes.
Bug Fixes:
Enhancements:
Tests: