path: root/docs/superpowers/plans/2026-07-09-current-attempt-resolution.md

# Current Attempt Resolution 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:** Build the `CurrentAttempt` resolution primitive `docs/superpowers/specs/2026-07-09-recursive-arbitrated-review-design.md` calls for, and wire `internal/executor.Pool.maybeUnblockParent` to resolve each of a roll-up's subtasks through it before checking completion. This is piece 2b of that design's implementation order.

**Architecture:** Every position in a task tree is identified by an anchor task ID, set once. "The task currently representing that position" is *resolved*, not stored — `CurrentAttempt` walks forward through a chain of fix-attempt `DependsOn` edges (a fix-attempt is structurally: a task sharing the same `ParentTaskID` as the task it supersedes, whose sole `DependsOn` entry is that task's own ID). This is the exact shape `internal/scheduler.StoryOrchestrator.ensureFixAttempt` already creates for a story's root today (`ParentTaskID == ""`), and the exact shape any future subtask-level fix-attempt spawning will create too (`ParentTaskID` set to the rejected subtask's own parent). One function, one rule, works identically at any depth.

`CurrentAttempt` lives in `internal/task` — the lowest-level package both `internal/executor` and `internal/scheduler` already import — behind a minimal `TaskLookup` interface (`GetTask`, `ListDependents`), which both `executor.Store` and `scheduler.StoryStore` already satisfy structurally. This avoids duplicating the walk logic once for each caller; `internal/executor.Pool.maybeUnblockParent` becomes the first real caller in this plan, and piece 3 (migrating the story-level fix loop off `RootTaskID` re-pointing) will become the second, later.

**Tech Stack:** Go. No schema/storage changes.

## Global Constraints

- **This plan does not change *when* arbitrated review runs, or make `maybeUnblockParent` aware of pending/rejected verdicts.** Today, nothing spawns a subtask-level fix-attempt yet (that's piece 4, generalizing `ensureEvaluators`/`ensureArbitration` to recurse into subtask trees — deliberately not this plan). This plan only makes the *counting/completion-gate* logic correct for whenever such a fix-attempt chain *does* exist: it resolves each subtask through `CurrentAttempt` so a superseded original doesn't get counted as "the" position's final state, and so the gate doesn't wait on a stale task forever. Whether a roll-up should be allowed to unblock *before* arbitration has even had a chance to reject one of its children is a separate, open question that belongs to piece 4, which is what will actually introduce that race — this plan does not attempt to resolve it.
- **Do not touch `ensureFixAttempt`, `story.RootTaskID`, or anything in `internal/scheduler`.** Migrating the story-level fix loop onto `CurrentAttempt` is piece 3, sequenced after this one so `CurrentAttempt` has one proven, isolated caller (`maybeUnblockParent`) before a second one is layered on.
- **Do not touch `internal/api` or `handleRunResult`/`RecoverStaleBlocked`.** This plan only touches `maybeUnblockParent`'s subtask-counting loop, plus the new `internal/task/currentattempt.go`.

---

## Task 1: `CurrentAttempt` resolution + `maybeUnblockParent` generalization

**Files:**
- Create: `internal/task/currentattempt.go`
- Test: `internal/task/currentattempt_test.go`
- Modify: `internal/executor/executor.go` (`maybeUnblockParent`'s subtask-completion loop)
- Test: `internal/executor/executor_test.go` (append)

**Interfaces:**
- Produces: `task.TaskLookup` interface, `task.CurrentAttempt(store TaskLookup, anchorID string) (*Task, error)`. Piece 3 (story-level fix-loop migration) will call this directly with `StoryOrchestrator.Store`, exactly as this plan calls it with `executor.Pool`'s own `store` field.

- [ ] **Step 1: Write the failing `CurrentAttempt` tests**

Create `internal/task/currentattempt_test.go`:

```go
package task

import (
	"errors"
	"testing"
)

type fakeLookup struct {
	tasks map[string]*Task
}

func newFakeLookup() *fakeLookup {
	return &fakeLookup{tasks: make(map[string]*Task)}
}

func (f *fakeLookup) GetTask(id string) (*Task, error) {
	t, ok := f.tasks[id]
	if !ok {
		return nil, errors.New("not found")
	}
	return t, nil
}

func (f *fakeLookup) ListDependents(taskID string) ([]*Task, error) {
	var out []*Task
	for _, t := range f.tasks {
		for _, d := range t.DependsOn {
			if d == taskID {
				out = append(out, t)
				break
			}
		}
	}
	return out, nil
}

func TestCurrentAttempt_NoFixAttempt_ReturnsAnchorItself(t *testing.T) {
	lookup := newFakeLookup()
	lookup.tasks["a"] = &Task{ID: "a"}

	got, err := CurrentAttempt(lookup, "a")
	if err != nil {
		t.Fatalf("CurrentAttempt: %v", err)
	}
	if got.ID != "a" {
		t.Errorf("got %q, want a", got.ID)
	}
}

func TestCurrentAttempt_OneFixAttempt_ResolvesToIt(t *testing.T) {
	lookup := newFakeLookup()
	lookup.tasks["a"] = &Task{ID: "a", ParentTaskID: "parent-1"}
	lookup.tasks["a-fix1"] = &Task{ID: "a-fix1", ParentTaskID: "parent-1", DependsOn: []string{"a"}}

	got, err := CurrentAttempt(lookup, "a")
	if err != nil {
		t.Fatalf("CurrentAttempt: %v", err)
	}
	if got.ID != "a-fix1" {
		t.Errorf("got %q, want a-fix1", got.ID)
	}
}

func TestCurrentAttempt_ChainOfFixAttempts_ResolvesToTip(t *testing.T) {
	lookup := newFakeLookup()
	lookup.tasks["a"] = &Task{ID: "a", ParentTaskID: "parent-1"}
	lookup.tasks["a-fix1"] = &Task{ID: "a-fix1", ParentTaskID: "parent-1", DependsOn: []string{"a"}}
	lookup.tasks["a-fix2"] = &Task{ID: "a-fix2", ParentTaskID: "parent-1", DependsOn: []string{"a-fix1"}}

	got, err := CurrentAttempt(lookup, "a")
	if err != nil {
		t.Fatalf("CurrentAttempt: %v", err)
	}
	if got.ID != "a-fix2" {
		t.Errorf("got %q, want a-fix2 (tip of the chain)", got.ID)
	}

	// Resolving from an intermediate link in the chain also lands on the tip.
	got, err = CurrentAttempt(lookup, "a-fix1")
	if err != nil {
		t.Fatalf("CurrentAttempt: %v", err)
	}
	if got.ID != "a-fix2" {
		t.Errorf("resolving from a-fix1: got %q, want a-fix2", got.ID)
	}
}

// TestCurrentAttempt_DependentWithDifferentParent_NotTreatedAsFixAttempt
// proves a task that depends on "a" but has a DIFFERENT ParentTaskID (e.g.
// an unrelated Evaluator task depending on a Builder for scheduling reasons)
// must not be mistaken for a's fix attempt.
func TestCurrentAttempt_DependentWithDifferentParent_NotTreatedAsFixAttempt(t *testing.T) {
	lookup := newFakeLookup()
	lookup.tasks["a"] = &Task{ID: "a", ParentTaskID: "parent-1"}
	lookup.tasks["unrelated"] = &Task{ID: "unrelated", ParentTaskID: "", DependsOn: []string{"a"}}

	got, err := CurrentAttempt(lookup, "a")
	if err != nil {
		t.Fatalf("CurrentAttempt: %v", err)
	}
	if got.ID != "a" {
		t.Errorf("got %q, want a (unrelated dependent must not be treated as a fix attempt)", got.ID)
	}
}

// TestCurrentAttempt_DependentWithMultipleDependsOn_NotTreatedAsFixAttempt
// proves a fix attempt has exactly one DependsOn entry (the task it
// supersedes); a task depending on "a" among several dependencies (e.g. an
// Arbitration task depending on all 4 Evaluators) must not be mistaken for
// a's fix attempt.
func TestCurrentAttempt_DependentWithMultipleDependsOn_NotTreatedAsFixAttempt(t *testing.T) {
	lookup := newFakeLookup()
	lookup.tasks["a"] = &Task{ID: "a", ParentTaskID: "parent-1"}
	lookup.tasks["multi-dep"] = &Task{ID: "multi-dep", ParentTaskID: "parent-1", DependsOn: []string{"a", "b"}}

	got, err := CurrentAttempt(lookup, "a")
	if err != nil {
		t.Fatalf("CurrentAttempt: %v", err)
	}
	if got.ID != "a" {
		t.Errorf("got %q, want a", got.ID)
	}
}

func TestCurrentAttempt_UnknownAnchor_ReturnsError(t *testing.T) {
	lookup := newFakeLookup()
	if _, err := CurrentAttempt(lookup, "does-not-exist"); err == nil {
		t.Error("expected an error for an unknown anchor ID")
	}
}
```

- [ ] **Step 2: Run tests to verify they fail**

Run: `go test ./internal/task/ -run TestCurrentAttempt -v`
Expected: compile failure — `CurrentAttempt`/`TaskLookup` don't exist yet. Paste the actual output.

- [ ] **Step 3: Implement `CurrentAttempt`**

Create `internal/task/currentattempt.go`:

```go
package task

// TaskLookup is the minimal interface CurrentAttempt needs: resolving a
// task by ID and finding tasks that depend on it. Both
// internal/executor.Store and internal/scheduler.StoryStore already satisfy
// this structurally, so CurrentAttempt lives here (in the lowest-level
// package both already import) rather than being duplicated in each.
type TaskLookup interface {
	GetTask(id string) (*Task, error)
	ListDependents(taskID string) ([]*Task, error)
}

// maxAttemptChainWalk bounds CurrentAttempt's forward walk as a defense
// against a pathological/cyclic DependsOn chain (task creation doesn't
// reject cycles) -- in practice, no caller creates chains anywhere near
// this long; a much smaller, intentional policy limit (e.g.
// internal/scheduler's own maxFixAttempts) is what actually bounds real
// fix-attempt chains in normal operation.
const maxAttemptChainWalk = 100

// CurrentAttempt resolves the task that currently represents the logical
// position anchored at anchorID: anchorID itself, or -- if it was rejected
// and a fix attempt exists -- the fix attempt, walking forward through
// however many rejections have occurred. A fix-attempt task is identified
// structurally: a task with the same ParentTaskID as the task it supersedes,
// whose sole DependsOn entry is that task's own ID -- the exact shape a
// fix-attempt is spawned with, whether at a story's root (ParentTaskID ==
// "") or at an arbitrary subtask position (ParentTaskID set). This is the
// one place "which task is current" is ever decided; callers resolve
// through this before checking a position's own completion/verdict state,
// never operating on a raw anchor ID directly once a fix-attempt might
// exist. See docs/superpowers/specs/2026-07-09-recursive-arbitrated-review-design.md.
func CurrentAttempt(store TaskLookup, anchorID string) (*Task, error) {
	current, err := store.GetTask(anchorID)
	if err != nil {
		return nil, err
	}
	for i := 0; i < maxAttemptChainWalk; i++ {
		dependents, err := store.ListDependents(current.ID)
		if err != nil {
			return nil, err
		}
		var next *Task
		for _, d := range dependents {
			if d.ParentTaskID == current.ParentTaskID && len(d.DependsOn) == 1 && d.DependsOn[0] == current.ID {
				next = d
				break
			}
		}
		if next == nil {
			return current, nil
		}
		current = next
	}
	return current, nil
}
```

- [ ] **Step 4: Run tests to verify they pass**

Run: `go test ./internal/task/ -run TestCurrentAttempt -v`
Expected: PASS, all 6 tests. Paste the actual output.

- [ ] **Step 5: Write the failing `maybeUnblockParent` generalization test**

Append to `internal/executor/executor_test.go`, directly after `TestPool_RecoverStaleBlocked_PromotesNestedSubtaskParent` (the last test added by the previous plan):

```go
// TestPool_MaybeUnblockParent_ResolvesSubtaskThroughCurrentAttempt proves
// the parent's "are all subtasks done" gate resolves each subtask through
// task.CurrentAttempt first -- so a subtask that was superseded by a
// fix-attempt sibling (same ParentTaskID, DependsOn=[original]) correctly
// waits for the FIX ATTEMPT to complete, not just the stale original that
// already reached COMPLETED before being superseded.
func TestPool_MaybeUnblockParent_ResolvesSubtaskThroughCurrentAttempt(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)

	parent := makeTask("resolve-parent")
	parent.State = task.StateBlocked
	store.CreateTask(parent)

	original := makeTask("resolve-original")
	original.ParentTaskID = parent.ID
	original.State = task.StateCompleted // rejected, but its own execution finished
	store.CreateTask(original)

	fixAttempt := makeTask("resolve-fix-attempt")
	fixAttempt.ParentTaskID = parent.ID
	fixAttempt.DependsOn = []string{original.ID}
	fixAttempt.State = task.StateRunning // still in progress
	store.CreateTask(fixAttempt)

	pool.maybeUnblockParent(parent.ID)

	got, err := store.GetTask(parent.ID)
	if err != nil {
		t.Fatalf("get parent: %v", err)
	}
	if got.State != task.StateBlocked {
		t.Errorf("parent state: want still BLOCKED (fix attempt not done yet), got %v", got.State)
	}

	// Now the fix attempt finishes.
	if err := store.UpdateTaskState(fixAttempt.ID, task.StateCompleted); err != nil {
		t.Fatalf("complete fix attempt: %v", err)
	}
	pool.maybeUnblockParent(parent.ID)

	got, err = store.GetTask(parent.ID)
	if err != nil {
		t.Fatalf("get parent: %v", err)
	}
	if got.State != task.StateReady {
		t.Errorf("parent state: want READY (fix attempt now done), got %v", got.State)
	}
}
```

- [ ] **Step 6: Run the test to verify it fails**

Run: `go test ./internal/executor/ -run TestPool_MaybeUnblockParent_ResolvesSubtaskThroughCurrentAttempt -v`
Expected: FAILS at the first assertion — today's `maybeUnblockParent` checks `original.State` directly (already `COMPLETED`) and ignores that it's been superseded by the still-running `fixAttempt`, so it incorrectly promotes `parent` to `READY` immediately instead of waiting. Paste the actual output.

- [ ] **Step 7: Generalize `maybeUnblockParent`'s subtask-completion loop**

In `internal/executor/executor.go`, find the loop inside `maybeUnblockParent`:

```go
	for _, sub := range subtasks {
		if sub.State != task.StateCompleted {
			return
		}
	}
```

Replace it with:

```go
	for _, sub := range subtasks {
		current, err := task.CurrentAttempt(p.store, sub.ID)
		if err != nil {
			p.logger.Error("maybeUnblockParent: resolve current attempt", "parentID", parentID, "subtaskID", sub.ID, "error", err)
			return
		}
		if current.State != task.StateCompleted {
			return
		}
	}
```

- [ ] **Step 8: Run tests to verify they pass**

Run: `go test ./internal/executor/ -run 'TestPool_(MaybeUnblockParent|Submit_|RecoverStaleBlocked)' -v`
Expected: PASS, all of them — the new test, plus every pre-existing `TestPool_Submit_*`/`TestPool_RecoverStaleBlocked_*` test from this and the previous plan, unmodified. A subtask with no fix-attempt resolves to itself via `CurrentAttempt` (Step 3's `TestCurrentAttempt_NoFixAttempt_ReturnsAnchorItself` already proves this in isolation), so every existing test's behavior is unchanged. Paste the actual output.

- [ ] **Step 9: Run the full package suite, then the full repo suite**

Run: `go test ./internal/task/... ./internal/executor/...`
Expected: PASS for everything.

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.

Also run `gofmt -l internal/task/currentattempt.go internal/task/currentattempt_test.go internal/executor/executor.go internal/executor/executor_test.go` before committing — this repo has had gofmt regressions slip through in prior tasks this session by copying plan markdown's own formatting verbatim; run `gofmt -w` on any file it flags that wasn't already gofmt-dirty before your changes (check with `git show HEAD:<path> | gofmt -l -` if unsure whether dirtiness is pre-existing).

- [ ] **Step 10: Commit and push to `main` directly, matching this repo's existing workflow**

```bash
git add internal/task/currentattempt.go internal/task/currentattempt_test.go internal/executor/executor.go internal/executor/executor_test.go
git commit -m "feat(task,executor): add CurrentAttempt resolution, wire maybeUnblockParent to resolve through it"
```

## 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 `grep -rn "CurrentAttempt" internal/` to visually confirm the File Map was actually touched (should be `internal/task/currentattempt.go`, `internal/task/currentattempt_test.go`, and `internal/executor/executor.go` only — nothing in `internal/scheduler` yet, since that's piece 3).