1 files changed, 433 insertions, 0 deletions
diff --git a/docs/superpowers/plans/2026-07-09-nested-subtask-completion.md b/docs/superpowers/plans/2026-07-09-nested-subtask-completion.md
new file mode 100644
index 0000000..8ca1acf
--- /dev/null
+++ b/docs/superpowers/plans/2026-07-09-nested-subtask-completion.md
@@ -0,0 +1,433 @@
+# Nested Subtask Completion Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Fix a real, pre-existing bug that blocks arbitrary-depth recursive decomposition: today, `internal/executor.Pool` only correctly handles subtask delegation *one level deep*. A task that is itself a subtask (`ParentTaskID != ""`) and *also* spawns its own subtasks (nested decomposition) is marked `COMPLETED` the instant its own agent turn ends — even if its freshly-spawned children haven't run yet. This is piece 2a of `docs/superpowers/specs/2026-07-09-recursive-arbitrated-review-design.md`'s implementation order, discovered while grounding piece 2 in the actual code — a prerequisite the design spec didn't anticipate, since the recursive-review generalization is moot if nested decomposition doesn't even structurally complete correctly.
+
+**Architecture:** Three functions in `internal/executor/executor.go` currently special-case "is this a top-level task" (`ParentTaskID == ""`) instead of asking the real question, "does this task have its own pending subtasks":
+1. `handleRunResult`'s completion branch only checks for pending subtasks when `t.ParentTaskID == ""` — a subtask that spawned its own children skips that check entirely and is marked `COMPLETED` unconditionally.
+2. `maybeUnblockParent` always promotes an unblocked parent to `READY` — correct for a true top-level task (needs a human accept), wrong for a subtask-parent (which should skip straight to `COMPLETED`, mirroring how any other subtask completes, and then recursively unblock *its own* parent).
+3. `RecoverStaleBlocked` explicitly skips any `BLOCKED`/`QUEUED` task with `ParentTaskID != ""` ("only promote actual parents") — so a restarted server can never recover a `BLOCKED` subtask-parent once its own children finish.
+
+All three get the same fix: ask "does this task have subtasks" uniformly, regardless of whether it's top-level or itself a subtask; decide `READY` vs `COMPLETED` based on whether *this* task has a `ParentTaskID`, not based on why the check ran.
+
+**Tech Stack:** Go. No schema/storage changes — this is pure control-flow correctness in `internal/executor`.
+
+## Global Constraints
+
+- Do not touch `currentAttempt()`/fix-loop generalization, `ensureEvaluators`/`ensureArbitration`, or anything in `internal/scheduler` — those are separate, later pieces of the same design spec's implementation order (piece 2b and beyond), deliberately sequenced after this prerequisite fix.
+- The `QUEUED`-parent path in `maybeUnblockParent` is left unchanged (still promotes to `READY` unconditionally) — a subtask can only ever reach a state where its own children exist and are checked *after* it has already run (i.e., while `BLOCKED`), since `spawn_subtask` is called from within the subtask's own execution. A subtask can never legitimately be `QUEUED` with pre-existing completed children the way `maybeUnblockParent`'s `QUEUED` branch was built to tolerate for hand-constructed task trees (e.g. YAML batch files declaring a parent+children together) — and `task.ValidTransition` doesn't even permit `QUEUED → COMPLETED`, so attempting the new behavior there would be an invalid transition, not just semantically wrong. Only the `BLOCKED` case gets the new completion-vs-ready branch.
+
+---
+
+## Task 1: Fix nested subtask completion detection
+
+**Files:**
+- Modify: `internal/executor/executor.go` (`handleRunResult`, `maybeUnblockParent`, `RecoverStaleBlocked`)
+- Test: `internal/executor/executor_test.go` (append)
+
+**Interfaces:**
+- Produces: nothing new for later work — this is a correctness fix to existing, already-consumed machinery. Piece 2b (`currentAttempt()` generalization) will build on this corrected foundation.
+
+- [ ] **Step 1: Write the failing tests**
+
+Append to `internal/executor/executor_test.go`, directly after `TestPool_Submit_NotLastSubtask_ParentStaysBlocked` (find it — it's the last of the `TestPool_Submit_*` cluster before the file moves on to other test groups):
+
+```go
+// TestPool_Submit_SubtaskWithOwnSubtasks_GoesBlocked proves the fix for a
+// real pre-existing bug: a subtask (ParentTaskID != "") that itself spawns
+// subtasks must go BLOCKED when its own agent turn ends, not COMPLETED --
+// mirroring exactly how a top-level task with subtasks already behaves
+// (TestPool_Submit_TopLevel_WithSubtasks_GoesBlocked). Before this fix,
+// handleRunResult only checked for pending subtasks when ParentTaskID == "",
+// so a decomposing subtask was marked COMPLETED the instant its own turn
+// ended, ignoring the children it just spawned.
+func TestPool_Submit_SubtaskWithOwnSubtasks_GoesBlocked(t *testing.T) {
+	store := testStore(t)
+	runner := &mockRunner{}
+	runners := map[string]Runner{"claude": runner}
+	logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError}))
+	pool := NewPool(2, runners, store, logger)
+
+	middle := makeTask("middle-with-own-subtasks")
+	middle.ParentTaskID = "grandparent-1" // middle is itself a subtask
+	store.CreateTask(middle)
+
+	// middle spawned its own child, but that child hasn't been submitted yet.
+	grandchild := makeTask("grandchild-of-middle")
+	grandchild.ParentTaskID = middle.ID
+	store.CreateTask(grandchild)
+
+	if err := pool.Submit(context.Background(), middle); err != nil {
+		t.Fatalf("submit: %v", err)
+	}
+
+	result := <-pool.Results()
+	if result.Err != nil {
+		t.Errorf("expected no error, got: %v", result.Err)
+	}
+	if result.Execution.Status != "BLOCKED" {
+		t.Errorf("status: want BLOCKED, got %q", result.Execution.Status)
+	}
+	got, _ := store.GetTask(middle.ID)
+	if got.State != task.StateBlocked {
+		t.Errorf("task state: want BLOCKED, got %v (this is the bug: a decomposing subtask must not skip straight to COMPLETED)", got.State)
+	}
+}
+
+// TestPool_Submit_GrandchildCompletion_CascadesThroughNestedParents proves
+// the full 3-level recursive propagation: when the deepest leaf completes,
+// its immediate parent (itself a subtask) goes straight to COMPLETED --
+// not READY, since subtasks never need a human accept -- which then
+// recursively unblocks the true top-level root to READY.
+func TestPool_Submit_GrandchildCompletion_CascadesThroughNestedParents(t *testing.T) {
+	store := testStore(t)
+	runner := &mockRunner{}
+	runners := map[string]Runner{"claude": runner}
+	logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError}))
+	pool := NewPool(2, runners, store, logger)
+
+	root := makeTask("nested-root")
+	root.State = task.StateBlocked // already ran, delegated to middle
+	store.CreateTask(root)
+
+	middle := makeTask("nested-middle")
+	middle.ParentTaskID = root.ID
+	middle.State = task.StateBlocked // already ran, delegated to grandchild
+	store.CreateTask(middle)
+
+	grandchild := makeTask("nested-grandchild")
+	grandchild.ParentTaskID = middle.ID
+	store.CreateTask(grandchild) // fresh, about to be submitted
+
+	if err := pool.Submit(context.Background(), grandchild); err != nil {
+		t.Fatalf("submit: %v", err)
+	}
+
+	result := <-pool.Results()
+	if result.Err != nil {
+		t.Errorf("expected no error, got: %v", result.Err)
+	}
+	if result.Execution.Status != "COMPLETED" {
+		t.Errorf("grandchild status: want COMPLETED, got %q", result.Execution.Status)
+	}
+
+	gotMiddle, err := store.GetTask(middle.ID)
+	if err != nil {
+		t.Fatalf("get middle: %v", err)
+	}
+	if gotMiddle.State != task.StateCompleted {
+		t.Errorf("middle state: want COMPLETED (it's a subtask, no accept needed), got %v", gotMiddle.State)
+	}
+
+	gotRoot, err := store.GetTask(root.ID)
+	if err != nil {
+		t.Fatalf("get root: %v", err)
+	}
+	if gotRoot.State != task.StateReady {
+		t.Errorf("root state: want READY (top-level, awaiting accept), got %v", gotRoot.State)
+	}
+}
+
+// TestPool_RecoverStaleBlocked_PromotesNestedSubtaskParent proves
+// RecoverStaleBlocked no longer skips BLOCKED subtask-parents -- before
+// this fix, its "only promote actual parents" filter (ParentTaskID == "")
+// meant a restarted server could never recover a BLOCKED subtask-parent
+// once its own children finished, leaving it stuck forever.
+func TestPool_RecoverStaleBlocked_PromotesNestedSubtaskParent(t *testing.T) {
+	store := testStore(t)
+	logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError}))
+	pool := NewPool(2, map[string]Runner{"claude": &mockRunner{}}, store, logger)
+
+	root := makeTask("recover-nested-root")
+	root.State = task.StateBlocked
+	store.CreateTask(root)
+
+	middle := makeTask("recover-nested-middle")
+	middle.ParentTaskID = root.ID
+	middle.State = task.StateBlocked
+	store.CreateTask(middle)
+
+	grandchild := makeTask("recover-nested-grandchild")
+	grandchild.ParentTaskID = middle.ID
+	grandchild.State = task.StateCompleted
+	store.CreateTask(grandchild)
+
+	pool.RecoverStaleBlocked()
+
+	gotMiddle, err := store.GetTask(middle.ID)
+	if err != nil {
+		t.Fatalf("get middle: %v", err)
+	}
+	if gotMiddle.State != task.StateCompleted {
+		t.Errorf("middle state: want COMPLETED, got %v", gotMiddle.State)
+	}
+
+	gotRoot, err := store.GetTask(root.ID)
+	if err != nil {
+		t.Fatalf("get root: %v", err)
+	}
+	if gotRoot.State != task.StateReady {
+		t.Errorf("root state: want READY, got %v", gotRoot.State)
+	}
+}
+```
+
+- [ ] **Step 2: Run tests to verify they fail**
+
+Run: `go test ./internal/executor/ -run 'TestPool_(Submit_SubtaskWithOwnSubtasks|Submit_GrandchildCompletion|RecoverStaleBlocked_PromotesNestedSubtaskParent)' -v`
+Expected: `TestPool_Submit_SubtaskWithOwnSubtasks_GoesBlocked` FAILS (middle goes to COMPLETED, not BLOCKED — the bug). `TestPool_Submit_GrandchildCompletion_CascadesThroughNestedParents` FAILS (middle goes to READY instead of COMPLETED — wait, actually: today's code puts middle straight to COMPLETED unconditionally when ITS OWN turn ends, which never happens correctly today because middle here is pre-seeded as already-BLOCKED, not submitted — so today's `maybeUnblockParent` bug fires here: it will promote middle to READY, not COMPLETED, since it always uses READY today). `TestPool_RecoverStaleBlocked_PromotesNestedSubtaskParent` FAILS (middle stays BLOCKED forever, root never reaches READY, since the recovery loop skips subtask-parents entirely today). Paste the actual output for all three failures.
+
+- [ ] **Step 3: Fix `handleRunResult`'s completion branch**
+
+In `internal/executor/executor.go`, find:
+
+```go
+	} else {
+		p.mu.Lock()
+		p.consecutiveFailures[agentType] = 0
+		p.mu.Unlock()
+		if t.ParentTaskID == "" {
+			subtasks, subErr := p.store.ListSubtasks(t.ID)
+			if subErr != nil {
+				p.logger.Error("failed to list subtasks", "taskID", t.ID, "error", subErr)
+			}
+			if subErr == nil && len(subtasks) > 0 {
+				exec.Status = "BLOCKED"
+				if err := p.store.UpdateTaskState(t.ID, task.StateBlocked); err != nil {
+					p.logger.Error("failed to update task state", "taskID", t.ID, "state", task.StateBlocked, "error", err)
+				}
+			} else {
+				exec.Status = "READY"
+				if err := p.store.UpdateTaskState(t.ID, task.StateReady); err != nil {
+					p.logger.Error("failed to update task state", "taskID", t.ID, "state", task.StateReady, "error", err)
+				}
+			}
+		} else {
+			exec.Status = "COMPLETED"
+			if err := p.store.UpdateTaskState(t.ID, task.StateCompleted); err != nil {
+				p.logger.Error("failed to update task state", "taskID", t.ID, "state", task.StateCompleted, "error", err)
+			}
+			p.maybeUnblockParent(t.ParentTaskID)
+		}
+	}
+```
+
+Replace it with:
+
+```go
+	} else {
+		p.mu.Lock()
+		p.consecutiveFailures[agentType] = 0
+		p.mu.Unlock()
+		// Check for pending subtasks uniformly, regardless of whether t is
+		// itself top-level or a subtask -- a subtask that decomposed further
+		// (nested spawn_subtask) must block on its own children exactly like
+		// a top-level task does; only the READY-vs-COMPLETED choice below
+		// depends on whether t has its own ParentTaskID.
+		subtasks, subErr := p.store.ListSubtasks(t.ID)
+		if subErr != nil {
+			p.logger.Error("failed to list subtasks", "taskID", t.ID, "error", subErr)
+		}
+		if subErr == nil && len(subtasks) > 0 {
+			exec.Status = "BLOCKED"
+			if err := p.store.UpdateTaskState(t.ID, task.StateBlocked); err != nil {
+				p.logger.Error("failed to update task state", "taskID", t.ID, "state", task.StateBlocked, "error", err)
+			}
+		} else if t.ParentTaskID == "" {
+			exec.Status = "READY"
+			if err := p.store.UpdateTaskState(t.ID, task.StateReady); err != nil {
+				p.logger.Error("failed to update task state", "taskID", t.ID, "state", task.StateReady, "error", err)
+			}
+		} else {
+			exec.Status = "COMPLETED"
+			if err := p.store.UpdateTaskState(t.ID, task.StateCompleted); err != nil {
+				p.logger.Error("failed to update task state", "taskID", t.ID, "state", task.StateCompleted, "error", err)
+			}
+			p.maybeUnblockParent(t.ParentTaskID)
+		}
+	}
+```
+
+- [ ] **Step 4: Fix `maybeUnblockParent` to recurse correctly for nested subtask-parents**
+
+In the same file, find:
+
+```go
+// 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 (subtasks created before parent ran).
+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 && parent.State != task.StateQueued {
+		return
+	}
+	subtasks, err := p.store.ListSubtasks(parentID)
+	if err != nil {
+		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
+		}
+	}
+	if err := p.store.UpdateTaskState(parentID, task.StateReady); err != nil {
+		p.logger.Error("maybeUnblockParent: update parent state", "parentID", parentID, "error", err)
+	}
+}
+```
+
+Replace it with:
+
+```go
+// maybeUnblockParent transitions the parent task once all of its subtasks
+// are in the COMPLETED state: to READY if parentID is itself top-level
+// (awaiting a human accept, same as ever), or straight to COMPLETED if
+// parentID is itself a subtask (ParentTaskID set) -- mirroring
+// handleRunResult's own subtask-completion branch, since a subtask never
+// needs a human accept regardless of whether it got there by finishing its
+// own direct work or by having all of its delegated children finish. That
+// COMPLETED transition recurses into maybeUnblockParent(parent.ParentTaskID)
+// so a chain of nested decomposition (subtask spawns subtask spawns
+// subtask...) fully cascades to completion when the deepest leaves finish,
+// not just one level.
+//
+// Handles both BLOCKED parents (ran, created subtasks, paused) and QUEUED
+// parents (subtasks created before parent ran) for the READY path only —
+// the COMPLETED-instead-of-READY branch only applies to BLOCKED parents,
+// since a subtask can only have already-completed children after it has
+// itself run (BLOCKED), never while still QUEUED, and task.ValidTransition
+// doesn't permit QUEUED → COMPLETED regardless.
+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 && parent.State != task.StateQueued {
+		return
+	}
+	subtasks, err := p.store.ListSubtasks(parentID)
+	if err != nil {
+		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
+		}
+	}
+	if parent.State == task.StateBlocked && parent.ParentTaskID != "" {
+		if err := p.store.UpdateTaskState(parentID, task.StateCompleted); err != nil {
+			p.logger.Error("maybeUnblockParent: update parent state", "parentID", parentID, "error", err)
+			return
+		}
+		p.maybeUnblockParent(parent.ParentTaskID)
+		return
+	}
+	if err := p.store.UpdateTaskState(parentID, task.StateReady); err != nil {
+		p.logger.Error("maybeUnblockParent: update parent state", "parentID", parentID, "error", err)
+	}
+}
+```
+
+- [ ] **Step 5: Fix `RecoverStaleBlocked` to stop skipping subtask-parents**
+
+In the same file, find:
+
+```go
+// 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.
+// Call this once on server startup, after RecoverStaleRunning and RecoverStaleQueued.
+func (p *Pool) RecoverStaleBlocked() {
+	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
+			}
+			p.maybeUnblockParent(t.ID)
+		}
+	}
+}
+```
+
+Replace it with:
+
+```go
+// RecoverStaleBlocked promotes any BLOCKED or QUEUED task to READY (or, for a
+// task that is itself a subtask, straight to COMPLETED — see
+// maybeUnblockParent) when all of its own subtasks are already COMPLETED.
+// This handles the case where the server was restarted after subtasks
+// finished but before maybeUnblockParent could fire. Iterates every
+// BLOCKED/QUEUED task, not just top-level ones — a subtask that itself
+// decomposed (nested spawn_subtask) needs the exact same recovery
+// maybeUnblockParent already provides; maybeUnblockParent's own
+// "len(subtasks) == 0" guard is what correctly no-ops for a task that isn't
+// actually a parent of anything, so no separate filter is needed here.
+// Call this once on server startup, after RecoverStaleRunning and RecoverStaleQueued.
+func (p *Pool) RecoverStaleBlocked() {
+	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 {
+			p.maybeUnblockParent(t.ID)
+		}
+	}
+}
+```
+
+- [ ] **Step 6: Run tests to verify they pass**
+
+Run: `go test ./internal/executor/ -run 'TestPool_(Submit_SubtaskWithOwnSubtasks|Submit_GrandchildCompletion|RecoverStaleBlocked)' -v`
+Expected: PASS, all of them — the three new tests, plus every pre-existing `TestPool_RecoverStaleBlocked_*` test (`UnblocksWhenAllSubtasksCompleted`, `KeepsBlockedWhenSubtaskIncomplete`, `DoesNotPromoteQueuedLeafTask`) unmodified, proving the fix didn't regress the top-level cases those already covered. Paste the actual output.
+
+- [ ] **Step 7: Run the full package suite, then the full repo suite**
+
+Run: `go test ./internal/executor/...`
+Expected: PASS for everything — this package has the widest blast radius of any change in this plan, since `handleRunResult`/`maybeUnblockParent`/`RecoverStaleBlocked` are exercised by many existing tests beyond the ones this plan added.
+
+Then run: `go test ./...` (the entire repo). This must also pass — paste the actual output. If `internal/api` flakes under the race detector on a one-off "sql: database is closed" teardown error unrelated to this task's actual changes, rerun once before treating it as real. If it fails a second time in a way connected to your actual changes, stop and call ask_user.
+
+- [ ] **Step 8: Commit and push to `main` directly, matching this repo's existing workflow**
+
+```bash
+git add internal/executor/executor.go internal/executor/executor_test.go
+git commit -m "fix(executor): support nested subtask decomposition (subtask-with-own-subtasks correctly blocks, cascades, and recovers)"
+```
+
+## Mandatory verification disclosure
+
+When you call report_summary, paste the actual terminal output of every test command above — literal pass/fail counts, not a claim of success, including the full-repo `go test ./...` run. If anything here is ambiguous or conflicts with what you find in the actual repo, call ask_user and describe the specific conflict — don't guess or silently decide.
+
+---
+
+## Final Verification
+
+- [ ] Run `go build ./...` — passes.
+- [ ] Run `go test ./...` — passes, full repo.
+- [ ] Run `git diff --stat` (or `git show --stat HEAD`) to confirm only `internal/executor/executor.go` and `internal/executor/executor_test.go` were touched.