From 759396855a967a3d509498cc55faa3b4d8cadfba Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Thu, 26 Mar 2026 09:36:30 +0000 Subject: fix: story stays PENDING when all subtasks complete but parent task stays QUEUED MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a story is approved with pre-created subtasks, parent tasks are QUEUED but never run. Their subtasks complete, but: - maybeUnblockParent only handled BLOCKED parents, not QUEUED ones - checkStoryCompletion required ALL tasks (incl. subtasks) to be done Fixes: - maybeUnblockParent now also promotes QUEUED parents to READY when all subtasks are COMPLETED - checkStoryCompletion only checks top-level tasks (parent_task_id="") - RecoverStaleBlocked now also scans QUEUED parents on startup and triggers checkStoryCompletion if it promotes them - Add QUEUED→READY to valid state transitions (subtask delegation path) Co-Authored-By: Claude Sonnet 4.6 --- internal/executor/executor.go | 57 ++++++++++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 17 deletions(-) (limited to 'internal/executor/executor.go') diff --git a/internal/executor/executor.go b/internal/executor/executor.go index ae040c2..48626ec 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -511,8 +511,10 @@ func (p *Pool) handleRunResult(ctx context.Context, t *task.Task, exec *storage. p.resultCh <- &Result{TaskID: t.ID, Execution: exec, Err: err} } -// checkStoryCompletion checks whether all tasks in a story have reached a terminal -// success state and transitions the story to SHIPPABLE if so. +// checkStoryCompletion checks whether all top-level tasks in a story have reached +// a terminal success state and transitions the story to SHIPPABLE if so. +// Subtasks are intentionally excluded — a parent task reaching READY/COMPLETED +// already accounts for its subtasks. func (p *Pool) checkStoryCompletion(ctx context.Context, storyID string) { tasks, err := p.store.ListTasksByStory(storyID) if err != nil { @@ -522,11 +524,19 @@ func (p *Pool) checkStoryCompletion(ctx context.Context, storyID string) { if len(tasks) == 0 { return } + topLevelCount := 0 for _, t := range tasks { + if t.ParentTaskID != "" { + continue // subtasks are covered by their parent + } + topLevelCount++ if t.State != task.StateCompleted && t.State != task.StateReady { - return // not all tasks done + return // not all top-level tasks done } } + if topLevelCount == 0 { + return // no top-level tasks — don't auto-complete + } if err := p.store.UpdateStoryStatus(storyID, task.StoryShippable); err != nil { p.logger.Error("checkStoryCompletion: failed to update story status", "storyID", storyID, "error", err) return @@ -1016,18 +1026,31 @@ func (p *Pool) RecoverStaleQueued(ctx context.Context) { } } -// RecoverStaleBlocked promotes any BLOCKED parent task to READY when all of its -// subtasks are already COMPLETED. This handles the case where the server was -// restarted after subtasks finished but before maybeUnblockParent could fire. +// RecoverStaleBlocked promotes any BLOCKED or QUEUED parent task to READY when +// all of its subtasks are already COMPLETED. This handles the case where the +// server was restarted after subtasks finished but before maybeUnblockParent +// could fire, and also the case where story approval pre-created subtasks +// without ever running the parent task. // Call this once on server startup, after RecoverStaleRunning and RecoverStaleQueued. func (p *Pool) RecoverStaleBlocked() { - tasks, err := p.store.ListTasks(storage.TaskFilter{State: task.StateBlocked}) - if err != nil { - p.logger.Error("RecoverStaleBlocked: list tasks", "error", err) - return - } - for _, t := range tasks { - p.maybeUnblockParent(t.ID) + ctx := context.Background() + for _, state := range []task.State{task.StateBlocked, task.StateQueued} { + tasks, err := p.store.ListTasks(storage.TaskFilter{State: state}) + if err != nil { + p.logger.Error("RecoverStaleBlocked: list tasks", "error", err, "state", state) + continue + } + for _, t := range tasks { + if t.ParentTaskID != "" { + continue // only promote actual parents + } + before := t.State + p.maybeUnblockParent(t.ID) + // If the parent was promoted, check story completion. + if after, err := p.store.GetTask(t.ID); err == nil && after.State != before && t.StoryID != "" { + p.checkStoryCompletion(ctx, t.StoryID) + } + } } } @@ -1102,16 +1125,16 @@ func withFailureHistory(t *task.Task, execs []*storage.Execution, err error) *ta return © } -// maybeUnblockParent transitions the parent task from BLOCKED to READY if all -// of its subtasks are in the COMPLETED state. If any subtask is not COMPLETED -// (including FAILED, CANCELLED, RUNNING, etc.) the parent stays BLOCKED. +// maybeUnblockParent transitions the parent task to READY if all of its subtasks +// are in the COMPLETED state. Handles both BLOCKED parents (ran, created subtasks, +// paused) and QUEUED parents (story approval created subtasks without running parent). func (p *Pool) maybeUnblockParent(parentID string) { parent, err := p.store.GetTask(parentID) if err != nil { p.logger.Error("maybeUnblockParent: get parent", "parentID", parentID, "error", err) return } - if parent.State != task.StateBlocked { + if parent.State != task.StateBlocked && parent.State != task.StateQueued { return } subtasks, err := p.store.ListSubtasks(parentID) -- cgit v1.2.3