fix(plans): honor abort during parallel wave slot-wait#85
Merged
Conversation
In executeStepsInWaves, when all maxConcurrent slots were full of slow steps, a cancelled plan kept spinning the slot-wait poll until a step finished — the wave-top abort check can't fire while the inner scheduling loop holds control. Cancellation was still eventually honored (next wave top), but delayed, and the remaining ready steps were scheduled anyway. The slot-wait and the step-scheduling loop now re-check signal.aborted and break. Crucially they break rather than throw mid-loop: throwing would orphan the already-pushed stepPromises (Promise.all is what attaches their rejection handlers), reintroducing the unhandled-rejection class just fixed in JobQueueService. Scheduled steps are still awaited via Promise.all, then the abort is surfaced as 'cancelled'. Regression test: with maxConcurrent=1 and stepA gated, aborting while stepB waits for the slot leaves stepB unscheduled and resolves the plan 'cancelled' (not failed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merged
HoneyAdam
pushed a commit
to HoneyAdam/OwnPilot
that referenced
this pull request
Jun 6, 2026
Patch release shipping the post-0.7.1 fix: Fixed - Plan cancellation is now prompt during parallel wave execution: the maxConcurrent slot-wait and step-scheduling loop re-check signal.aborted and stop scheduling immediately; scheduled steps are still awaited via Promise.all so no in-flight promise is orphaned. (ownpilot#85) Version bumped across all workspace packages + docs/ARCHITECTURE.md + start scripts. release:verify green (17240 gateway tests, 8/8 turbo). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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
Found during a deep-read audit of
plans/executor.ts(a minor observation I'd flagged earlier, now fixed).In
executeStepsInWaves, when everymaxConcurrentslot was full of slow steps, a cancelled plan kept spinning the slot-wait poll (while (executingSteps.size >= maxConcurrent) await sleep(10)) until a step happened to finish — the wave-topsignal.abortedcheck can't fire while the inner scheduling loop holds control. Cancellation was still eventually honored at the next wave top, but delayed, and the rest of the wave's ready steps were scheduled anyway.Fix
The slot-wait and the step-scheduling loop now re-check
signal.abortedand stop.The subtle part: they break, not throw. Throwing mid-loop would orphan the already-pushed
stepPromises—Promise.all(below) is what attaches their rejection handlers — so a later-failing in-flight step would surface as an unhandled rejection (the exact class just fixed in #83). Instead we break out of scheduling, stillawait Promise.all(stepPromises)on what was scheduled, then throw'Plan execution aborted'→ the catch surfaces'cancelled'.Test plan
New regression test (
wave execution abort):maxConcurrent=1,stepA's tool gated to hold the slot; aborting whilestepBwaits leavesstepBunscheduled (calledTools === ['tool_a']) and resolves the plan'cancelled'(notfailed, noplan:failed). Without the fix, the freed slot after release would scheduletool_b— failing the assertion.🤖 Generated with Claude Code