Canceled background steps should not impact job result - #4482
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts how background step results are folded into the overall job result so that background steps explicitly canceled via a cancel control step don’t cause the job to become Canceled, while genuine Failed outcomes still propagate. It also refactors background-step draining vs. result aggregation into two separate coordinator methods.
Changes:
- Track explicitly-canceled background step IDs and ignore their
Canceledoutcome during background-result aggregation (while still propagatingFailed). - Split background-step “drain” (
WaitForUnwaitedStepsAsync) from “aggregate” (GetAggregatedResult) and updateStepsRunnerto call both. - Add L0 coverage for “canceled background step doesn’t affect job result” and “failed background step targeted by cancel still fails the job”.
Show a summary per file
| File | Description |
|---|---|
| src/Test/L0/Worker/BackgroundStepsL0.cs | Adds new L0 tests validating aggregation semantics for explicitly-canceled vs. failed background steps. |
| src/Runner.Worker/StepsRunner.cs | Updates the post-job safety-net flow to drain background steps first, then aggregate results separately. |
| src/Runner.Worker/BackgroundStepCoordinator.cs | Introduces explicit-cancel tracking and separates draining from aggregation logic. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 2
lokesh755
force-pushed
the
lokesh755-fix-cancel-steps-result
branch
from
June 8, 2026 20:50
558b317 to
ba1670a
Compare
TingluoHuang
reviewed
Jun 8, 2026
TingluoHuang
approved these changes
Jun 8, 2026
|
Commit now |
|
Successful to Quic Request |
|
Change like datalog info 04/152026 |
|
Exit to all bad error. Request now done true |
6 tasks
This was referenced Aug 1, 2026
3 tasks
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.
Summary
Canceled background steps should not impact the job result. A background step that is explicitly canceled by a
cancelcontrol step is expected to be canceled, so itsCanceledresult no longer influences the overall job result. A genuinely failed step still counts, even when it was targeted by a cancel (e.g. it failed before the cancellation took effect).This also splits
BackgroundStepCoordinator.WaitForUnwaitedStepsAsyncinto two members so each has a single responsibility:WaitForUnwaitedStepsAsync— drains any unwaited background steps at the post-job boundary.GetAggregatedResult()— folds the background step results into a singleTaskResultfor the caller.Changes
_explicitlyCanceledStepIds) so aCanceledoutcome from acancelstep is skipped during result aggregation, whileFailedoutcomes still propagate.WaitForUnwaitedStepsAsync(drain) fromGetAggregatedResult()(aggregation); updated theStepsRunnercall site to call both in sequence.