path: root/docs/superpowers/plans/2026-07-09-nested-subtask-completion.md

# 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.