From 25cf4c9d4d6f3c18ee7565bf8e6172896fff00c3 Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Sat, 4 Apr 2026 21:59:58 +0000 Subject: fix: prevent SHIPPABLE stories and wrong READY state on failed tasks Three related bugs fixed: 1. maybeUnblockParent: guard against promoting QUEUED leaf tasks (no subtasks) to READY. The vacuously-true 'all subtasks done' check was advancing tasks that stalled in QUEUED (due to a prior SQLite lock error) to READY on server restart via RecoverStaleBlocked, despite having only failed executions and no commits. 2. checkStoryCompletion: require COMPLETED (not just READY) for all top-level tasks before advancing a story to SHIPPABLE. READY means the checker agent is still pending or the task awaits human review; a story with READY tasks is not ready to ship. 3. handleAcceptTask: call CheckStoryCompletion after a task is accepted so stories with parent tasks (whose subtasks are all done and then the parent is manually accepted) can auto-advance to SHIPPABLE. Co-Authored-By: Claude Sonnet 4.6 --- internal/executor/executor.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'internal/executor/executor.go') diff --git a/internal/executor/executor.go b/internal/executor/executor.go index 8257f31..384a323 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -543,6 +543,12 @@ func (p *Pool) handleRunResult(ctx context.Context, t *task.Task, exec *storage. // 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. +// CheckStoryCompletion is the exported entry point for story completion checks +// called from outside the package (e.g. the API accept handler). +func (p *Pool) CheckStoryCompletion(ctx context.Context, storyID string) { + p.checkStoryCompletion(ctx, storyID) +} + func (p *Pool) checkStoryCompletion(ctx context.Context, storyID string) { story, err := p.store.GetStory(storyID) if err != nil { @@ -566,8 +572,8 @@ func (p *Pool) checkStoryCompletion(ctx context.Context, storyID string) { continue // subtasks are covered by their parent } topLevelCount++ - if t.State != task.StateCompleted && t.State != task.StateReady { - return // not all top-level tasks done + if t.State != task.StateCompleted { + return // not all top-level tasks done; READY alone is not sufficient (checker may be pending) } } if topLevelCount == 0 { @@ -1251,6 +1257,11 @@ func (p *Pool) maybeUnblockParent(parentID string) { p.logger.Error("maybeUnblockParent: list subtasks", "parentID", parentID, "error", err) return } + // A task with no subtasks was never blocked by subtask delegation — don't promote it. + // This prevents incorrectly promoting leaf tasks that are stuck in QUEUED to READY. + if len(subtasks) == 0 { + return + } for _, sub := range subtasks { if sub.State != task.StateCompleted { return -- cgit v1.2.3