fix(pipeline): parked pipelines stamped completed_at + spawned steps out of order#660
Merged
Conversation
UpdateTaskStatus stamped completed_at for every 'blocked' transition, including DAG-waiting pipeline steps that never ran. A freshly-created pipeline flips its non-root steps to 'blocked' (pipeline.go), so all of them were stamped completed_at = created_at — looking finished on the board and feeding false "done" signals to the workflow sweeps that key off completed_at. 'blocked' covers two cases: a step waiting in a DAG (never started) and a task that ran and is now parked awaiting review. Only stamp completed_at for the latter (started_at != nil), preserving the board's order-by-completed_at for genuinely-closed blocked tasks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Defense-in-depth for the workflow DAG invariant: a step must never run before its blockers complete. GetQueuedTasks should only return ready tasks, but a race or stray status flip can leave a step 'queued' while a blocker is still incomplete — we observed a parked (never-executed) pipeline spawn a downstream review/build step whose [Plan] blocker never ran, editing the main checkout out of order. processNextTask now gates every queued task through admitQueuedTask: if the task still has open blockers it is reverted to 'blocked' and logged loudly (rather than run), so the safety-net sweep re-queues it in order once the blocker really finishes — and the anomaly is captured if the underlying race recurs. 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
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 & why
A pipeline created and left in backlog (never executed) misbehaved two ways: every step showed up marked as if completed, and downstream steps spawned real Claude executors — in the main checkout, out of DAG order — despite their
[Plan]blocker never running. Root-caused both defects.Defect A — zombie
completed_at(confirmed root cause)UpdateTaskStatusstampedcompleted_at = CURRENT_TIMESTAMPfor everyblockedtransition, not just done/archived. Pipeline creation flips each non-root step toblocked(pipeline.go), so all of them were stampedcompleted_at = created_at— looking finished on the board and feeding false "done" signals to the workflow sweeps that key offcompleted_at.blockedcovers two cases: a step waiting in a DAG (never started) and a task that ran and is now parked awaiting review. Only the latter has completed a turn. Fix: only stampcompleted_aton ablockedtransition whenstarted_at != nil. Preserves the board's order-by-completed_atfor genuinely-parked blocked tasks.Defect B — premature spawn (defense-in-depth)
A step ran while its blocker was provably incomplete — a DAG-invariant violation. Every static path (
GetQueuedTasks,GetOpenBlockerCount, the sweeps) is individually correct, so this is a race, not a static logic bug; the daemon's stderr wasn't persisted so the exact trigger couldn't be pinned post-hoc.Rather than guess, this adds a spawn-gate invariant as the last line of defense:
processNextTaskruns every queued task throughadmitQueuedTask, which refuses any task with open blockers, reverts it toblocked, and logs loudly. The safety-net sweep re-queues it in order once the blocker really finishes — and the log line captures the anomaly if the underlying race recurs.Tests
TestUpdateTaskStatusBlockedTimestamps— DAG-waiting step leavescompleted_atnil; review-parked (started) step sets it.TestUpdateTaskStatus— a never-started blocked task no longer expectscompleted_at.TestAdmitQueuedTaskRefusesOpenBlocker/TestAdmitQueuedTaskAllowsReadyTask— spawn gate refuses open-blocker tasks, admits ready ones.Full suite green (18 packages), clean build.
Caveats
"Refused to start: N blocker(s) not yet complete"ever appears in a task log, that's the breadcrumb to the real trigger.processNextTaskseam, not a live end-to-end daemon run.🤖 Generated with Claude Code