# Migrate Story Fix Loop to CurrentAttempt 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:** Migrate `internal/scheduler.StoryOrchestrator`'s fix-and-re-evaluate loop off `story.RootTaskID` re-pointing, onto `task.CurrentAttempt` resolution (built in the previous plan). `story.RootTaskID` becomes a true immutable anchor, set once at story creation and never written again — every place that needs "the task currently representing the story's root" resolves it through `task.CurrentAttempt` instead. This is piece 3 of `docs/superpowers/specs/2026-07-09-recursive-arbitrated-review-design.md`'s implementation order. **Architecture:** Three functions in `internal/scheduler/story_orchestrator.go` currently read `st.RootTaskID` and treat it as the live root: `processStory` (the main dispatch loop), `processRetro` (the Phase 8 retro stage), and `ensureFixAttempt` (which also *writes* `st.RootTaskID = fix.ID`, the mutation this plan removes). All three switch to `task.CurrentAttempt(o.Store, st.RootTaskID)`, which walks forward through however many fix-attempt links exist and returns the tip — behaving identically to a plain `GetTask` when no fix-attempt exists (the overwhelming majority case), and correctly finding the latest attempt when one or more rejections have occurred. `fixAttemptDepth`'s own backward walk is unaffected — it already operates on whatever resolved task it's handed, regardless of how that resolution happened. **Tech Stack:** Go. No schema/storage changes — `story.RootTaskID` keeps its existing field and column; only *when* it's written changes (never, after initial creation). ## Global Constraints - `story.RootTaskID` must not be written anywhere in `internal/scheduler/story_orchestrator.go` after this plan — verify with `grep -n "RootTaskID = " internal/scheduler/story_orchestrator.go` returning nothing. - Do not touch `internal/task/currentattempt.go`, `internal/executor/executor.go`, `internal/api/stories.go`, or anything about generalizing arbitrated review to subtask trees (piece 4) — this plan is scoped purely to migrating the story-level fix loop's *read* pattern onto the primitive the previous plan already built and proved. - `internal/api/stories.go`'s `GET /api/stories/{id}/task-tree` does **not** need any change: it already walks the whole graph bidirectionally (both `ParentTaskID` children and `DependsOn` edges in either direction) starting from `st.RootTaskID`, so it discovers every fix-attempt in the chain regardless of which one is "current" — it was never relying on `RootTaskID` being kept up to date. --- ## Task 1: Migrate `ensureFixAttempt`/`processStory`/`processRetro` to `CurrentAttempt` **Files:** - Modify: `internal/scheduler/story_orchestrator.go` (`processStory`, `processRetro`, `ensureFixAttempt`) - Modify: `internal/scheduler/story_orchestrator_test.go` (update 3 existing tests whose assertions describe the old re-pointing behavior) **Interfaces:** - Consumes: `task.CurrentAttempt` (previous plan). `internal/scheduler` already imports `github.com/thepeterstone/claudomator/internal/task` — no new import needed. - Produces: nothing new for later work — piece 4 (generalizing arbitrated review to subtask trees) is what will actually spawn subtask-level fix-attempts that exercise `CurrentAttempt`'s forward-chain-walking beyond the single-hop case this plan's own tests cover. - [ ] **Step 1: Update `ensureFixAttempt` to resolve via `CurrentAttempt` and stop writing `RootTaskID`** In `internal/scheduler/story_orchestrator.go`, find `ensureFixAttempt` (including its doc comment): ```go // ensureFixAttempt handles a story sitting at NEEDS_FIX: it spawns one new // top-level builder-role task depending on the rejected root (purely for // structural discoverability/audit trail — the rejected root is already // COMPLETED, so this dependency is immediately satisfied), whose // instructions carry the original story spec/acceptance criteria plus the // arbitration's rejection reasoning, then re-points st.RootTaskID at the new // task and resets st.Status to IN_PROGRESS. The very next tick re-enters // processStory's normal Builder->Evaluators->Arbitration flow against the // new root, completely unchanged — no new pipeline, the existing one // re-entered. // // Idempotency is structural, mirroring every other stage in this file: it // looks for an existing builder-role dependent of the rejected root before // spawning a new one, so calling this repeatedly (or after a restart between // "task spawned" and "story updated") never spawns duplicates. The // maxFixAttempts cap is only checked in the "spawn a new one" branch — a // dependent that was already committed to being spawned still gets // re-pointed to, regardless of the cap, since refusing to do so would leave // a task dangling with nothing tracking it. func (o *StoryOrchestrator) ensureFixAttempt(ctx context.Context, st *story.Story) { oldRoot, err := o.Store.GetTask(st.RootTaskID) if err != nil { o.logf("story orchestrator: fix attempt: get rejected root task", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err) return } dependents, err := o.Store.ListDependents(oldRoot.ID) if err != nil { o.logf("story orchestrator: fix attempt: list root dependents", "storyID", st.ID, "error", err) return } var fix *task.Task for _, d := range dependents { if d.Agent.Role == "builder" { fix = d break } } if fix == nil { if depth := o.fixAttemptDepth(oldRoot); depth >= maxFixAttempts { o.logf("story orchestrator: fix attempt: max fix attempts reached, leaving story at NEEDS_FIX for a human", "storyID", st.ID, "depth", depth) return } reasoning := o.rejectionReasoning(st, oldRoot) instructions := fmt.Sprintf( "A previous attempt at story %q (task %s) was rejected by arbitration. Fix the issues and resubmit.\n\n"+ "Story spec:\n%s\n\nAcceptance criteria:\n%s\n\nArbitration's rejection reasoning:\n%s", st.Name, oldRoot.ID, st.Spec, formatAcceptanceCriteria(st.AcceptanceCriteria), reasoning) nt, err := o.spawnRoleTask(ctx, "Fix attempt: "+st.Name, "builder", []string{oldRoot.ID}, oldRoot, instructions) if err != nil { o.logf("story orchestrator: fix attempt: spawn", "storyID", st.ID, "error", err) return } fix = nt } st.RootTaskID = fix.ID st.Status = "IN_PROGRESS" if err := o.Store.UpdateStory(st); err != nil { o.logf("story orchestrator: fix attempt: repoint story root", "storyID", st.ID, "error", err) } } ``` Replace it with: ```go // ensureFixAttempt handles a story sitting at NEEDS_FIX: it spawns one new // top-level builder-role task depending on the rejected root (purely for // structural discoverability/audit trail — the rejected root is already // COMPLETED, so this dependency is immediately satisfied), whose // instructions carry the original story spec/acceptance criteria plus the // arbitration's rejection reasoning, then resets st.Status to IN_PROGRESS. // st.RootTaskID is never written here — it is an immutable anchor set once // at story creation; every caller that needs "the task currently // representing the story's root" resolves it via task.CurrentAttempt, which // walks forward through however many fix-attempt links exist. The very next // tick re-enters processStory's normal Builder->Evaluators->Arbitration flow // against whatever CurrentAttempt now resolves to — completely unchanged, no // new pipeline, the existing one re-entered. // // Idempotency is structural, mirroring every other stage in this file: it // looks for an existing builder-role dependent of the rejected root before // spawning a new one, so calling this repeatedly (or after a restart between // "task spawned" and "story status reset") never spawns duplicates. The // maxFixAttempts cap is only checked in the "spawn a new one" branch — a // dependent that was already committed to being spawned still gets used, // regardless of the cap, since refusing to do so would leave a task // dangling with nothing tracking it. func (o *StoryOrchestrator) ensureFixAttempt(ctx context.Context, st *story.Story) { oldRoot, err := task.CurrentAttempt(o.Store, st.RootTaskID) if err != nil { o.logf("story orchestrator: fix attempt: resolve current root attempt", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err) return } dependents, err := o.Store.ListDependents(oldRoot.ID) if err != nil { o.logf("story orchestrator: fix attempt: list root dependents", "storyID", st.ID, "error", err) return } var fix *task.Task for _, d := range dependents { if d.Agent.Role == "builder" { fix = d break } } if fix == nil { if depth := o.fixAttemptDepth(oldRoot); depth >= maxFixAttempts { o.logf("story orchestrator: fix attempt: max fix attempts reached, leaving story at NEEDS_FIX for a human", "storyID", st.ID, "depth", depth) return } reasoning := o.rejectionReasoning(st, oldRoot) instructions := fmt.Sprintf( "A previous attempt at story %q (task %s) was rejected by arbitration. Fix the issues and resubmit.\n\n"+ "Story spec:\n%s\n\nAcceptance criteria:\n%s\n\nArbitration's rejection reasoning:\n%s", st.Name, oldRoot.ID, st.Spec, formatAcceptanceCriteria(st.AcceptanceCriteria), reasoning) nt, err := o.spawnRoleTask(ctx, "Fix attempt: "+st.Name, "builder", []string{oldRoot.ID}, oldRoot, instructions) if err != nil { o.logf("story orchestrator: fix attempt: spawn", "storyID", st.ID, "error", err) return } fix = nt } st.Status = "IN_PROGRESS" if err := o.Store.UpdateStory(st); err != nil { o.logf("story orchestrator: fix attempt: reset story status", "storyID", st.ID, "error", err) } } ``` - [ ] **Step 2: Update `processStory`'s root lookup** In the same file, find the start of `processStory`: ```go func (o *StoryOrchestrator) processStory(ctx context.Context, st *story.Story) { if st.Status == "NEEDS_FIX" { o.ensureFixAttempt(ctx, st) return } root, err := o.Store.GetTask(st.RootTaskID) if err != nil { o.logf("story orchestrator: get root task", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err) return } ``` Replace it with: ```go func (o *StoryOrchestrator) processStory(ctx context.Context, st *story.Story) { if st.Status == "NEEDS_FIX" { o.ensureFixAttempt(ctx, st) return } root, err := task.CurrentAttempt(o.Store, st.RootTaskID) if err != nil { o.logf("story orchestrator: resolve current root attempt", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err) return } ``` - [ ] **Step 3: Update `processRetro`'s root lookup** In the same file, find the start of `processRetro`: ```go func (o *StoryOrchestrator) processRetro(ctx context.Context, st *story.Story) { root, err := o.Store.GetTask(st.RootTaskID) if err != nil { o.logf("story orchestrator: retro: get root task", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err) return } ``` Replace it with: ```go func (o *StoryOrchestrator) processRetro(ctx context.Context, st *story.Story) { root, err := task.CurrentAttempt(o.Store, st.RootTaskID) if err != nil { o.logf("story orchestrator: retro: resolve current root attempt", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err) return } ``` - [ ] **Step 4: Update the three tests whose assertions describe the old re-pointing behavior** In `internal/scheduler/story_orchestrator_test.go`, find `TestStoryOrchestrator_NeedsFix_SpawnsFixAttemptAndRepointsRoot` in full: ```go func TestStoryOrchestrator_NeedsFix_SpawnsFixAttemptAndRepointsRoot(t *testing.T) { store, st, oldRoot, _, _ := seedNeedsFixStory(t) pool := &fakePool{} orch := &StoryOrchestrator{Store: store, Pool: pool} orch.Tick(context.Background()) fixAttempts := store.dependentsWithRole(oldRoot.ID, "builder") if len(fixAttempts) != 1 { t.Fatalf("expected exactly 1 fix-attempt task depending on the rejected root, got %d", len(fixAttempts)) } fix := fixAttempts[0] if fix.State != task.StateQueued { t.Errorf("fix attempt State = %v, want QUEUED", fix.State) } if fix.ParentTaskID != "" { t.Errorf("fix attempt ParentTaskID = %q, want empty (top-level, DAG sibling not delegated subtask)", fix.ParentTaskID) } stories, _ := store.ListStories(storage.StoryFilter{}) var got *story.Story for _, s := range stories { if s.ID == st.ID { got = s } } if got == nil { t.Fatal("story not found") } if got.RootTaskID != fix.ID { t.Errorf("story RootTaskID = %q, want the new fix-attempt task %q", got.RootTaskID, fix.ID) } if got.Status != "IN_PROGRESS" { t.Errorf("story Status = %q, want IN_PROGRESS", got.Status) } } ``` Replace it with: ```go // TestStoryOrchestrator_NeedsFix_SpawnsFixAttempt proves the core mechanism: // a story at NEEDS_FIX gets a new builder-role fix-attempt task spawned // (depending on the rejected root), and the story's Status resets to // IN_PROGRESS so the very next tick re-enters the normal // Builder->Evaluators->Arbitration flow against the new attempt -- // discovered via task.CurrentAttempt resolving forward from st.RootTaskID, // which is never mutated (it's an immutable anchor set once at story // creation). func TestStoryOrchestrator_NeedsFix_SpawnsFixAttempt(t *testing.T) { store, st, oldRoot, _, _ := seedNeedsFixStory(t) pool := &fakePool{} orch := &StoryOrchestrator{Store: store, Pool: pool} orch.Tick(context.Background()) fixAttempts := store.dependentsWithRole(oldRoot.ID, "builder") if len(fixAttempts) != 1 { t.Fatalf("expected exactly 1 fix-attempt task depending on the rejected root, got %d", len(fixAttempts)) } fix := fixAttempts[0] if fix.State != task.StateQueued { t.Errorf("fix attempt State = %v, want QUEUED", fix.State) } if fix.ParentTaskID != "" { t.Errorf("fix attempt ParentTaskID = %q, want empty (top-level, DAG sibling not delegated subtask)", fix.ParentTaskID) } stories, _ := store.ListStories(storage.StoryFilter{}) var got *story.Story for _, s := range stories { if s.ID == st.ID { got = s } } if got == nil { t.Fatal("story not found") } if got.RootTaskID != oldRoot.ID { t.Errorf("story RootTaskID = %q, want unchanged (still %q -- it's an immutable anchor now, resolved via task.CurrentAttempt)", got.RootTaskID, oldRoot.ID) } if got.Status != "IN_PROGRESS" { t.Errorf("story Status = %q, want IN_PROGRESS", got.Status) } } ``` Find `TestStoryOrchestrator_EnsureFixAttempt_FindsExistingSpawnedAttempt` in full: ```go // TestStoryOrchestrator_EnsureFixAttempt_FindsExistingSpawnedAttempt proves // ensureFixAttempt's own structural idempotency: simulating a restart // between "fix task spawned" and "story RootTaskID/Status updated to point // at it" -- calling ensureFixAttempt again must find the already-spawned // builder-role dependent rather than spawning a second one, and still // complete the re-pointing. func TestStoryOrchestrator_EnsureFixAttempt_FindsExistingSpawnedAttempt(t *testing.T) { store, st, oldRoot, _, _ := seedNeedsFixStory(t) pool := &fakePool{} orch := &StoryOrchestrator{Store: store, Pool: pool} preexisting, err := orch.spawnRoleTask(context.Background(), "Fix attempt: pre-existing", "builder", []string{oldRoot.ID}, oldRoot, "fix it") if err != nil { t.Fatalf("seed pre-existing fix attempt: %v", err) } orch.ensureFixAttempt(context.Background(), st) fixAttempts := store.dependentsWithRole(oldRoot.ID, "builder") if len(fixAttempts) != 1 { t.Fatalf("expected exactly 1 fix-attempt task (the pre-existing one, not a new one), got %d", len(fixAttempts)) } if fixAttempts[0].ID != preexisting.ID { t.Errorf("expected ensureFixAttempt to reuse the pre-existing task %s, got a different one %s", preexisting.ID, fixAttempts[0].ID) } if st.RootTaskID != preexisting.ID { t.Errorf("story RootTaskID = %q, want the pre-existing fix attempt %q", st.RootTaskID, preexisting.ID) } if st.Status != "IN_PROGRESS" { t.Errorf("story Status = %q, want IN_PROGRESS", st.Status) } } ``` Replace it with: ```go // TestStoryOrchestrator_EnsureFixAttempt_FindsExistingSpawnedAttempt proves // ensureFixAttempt's own structural idempotency: simulating a restart // between "fix task spawned" and "story Status reset" -- calling // ensureFixAttempt again must find the already-spawned builder-role // dependent rather than spawning a second one, and still reset Status. func TestStoryOrchestrator_EnsureFixAttempt_FindsExistingSpawnedAttempt(t *testing.T) { store, st, oldRoot, _, _ := seedNeedsFixStory(t) pool := &fakePool{} orch := &StoryOrchestrator{Store: store, Pool: pool} preexisting, err := orch.spawnRoleTask(context.Background(), "Fix attempt: pre-existing", "builder", []string{oldRoot.ID}, oldRoot, "fix it") if err != nil { t.Fatalf("seed pre-existing fix attempt: %v", err) } orch.ensureFixAttempt(context.Background(), st) fixAttempts := store.dependentsWithRole(oldRoot.ID, "builder") if len(fixAttempts) != 1 { t.Fatalf("expected exactly 1 fix-attempt task (the pre-existing one, not a new one), got %d", len(fixAttempts)) } if fixAttempts[0].ID != preexisting.ID { t.Errorf("expected ensureFixAttempt to reuse the pre-existing task %s, got a different one %s", preexisting.ID, fixAttempts[0].ID) } if st.RootTaskID != oldRoot.ID { t.Errorf("story RootTaskID = %q, want unchanged (still %q -- it's an immutable anchor now)", st.RootTaskID, oldRoot.ID) } if st.Status != "IN_PROGRESS" { t.Errorf("story Status = %q, want IN_PROGRESS", st.Status) } } ``` Find `TestStoryOrchestrator_NeedsFix_CapsAtMaxFixAttempts` in full: ```go // TestStoryOrchestrator_NeedsFix_CapsAtMaxFixAttempts proves the safety net: // once a chain of maxFixAttempts consecutive fix attempts already exists, // ensureFixAttempt stops spawning new ones and leaves the story at // NEEDS_FIX, exactly like today's fully-manual behavior. func TestStoryOrchestrator_NeedsFix_CapsAtMaxFixAttempts(t *testing.T) { store, st, oldRoot, _, _ := seedNeedsFixStory(t) pool := &fakePool{} orch := &StoryOrchestrator{Store: store, Pool: pool} // Build a chain of maxFixAttempts prior fix-attempt tasks, each // depending on the one before it, ending at oldRoot -- simulating a // story that has already exhausted its automatic fix attempts. prev := oldRoot for i := 0; i < maxFixAttempts; i++ { nt, err := orch.spawnRoleTask(context.Background(), fmt.Sprintf("Fix attempt %d", i), "builder", []string{prev.ID}, oldRoot, "fix it") if err != nil { t.Fatalf("seed fix-attempt chain: %v", err) } prev = nt } st.RootTaskID = prev.ID if err := store.UpdateStory(st); err != nil { t.Fatalf("persist chained root: %v", err) } orch.Tick(context.Background()) fixAttempts := store.dependentsWithRole(prev.ID, "builder") if len(fixAttempts) != 0 { t.Fatalf("expected no new fix-attempt task once maxFixAttempts is reached, got %d", len(fixAttempts)) } stories, _ := store.ListStories(storage.StoryFilter{}) var got *story.Story for _, s := range stories { if s.ID == st.ID { got = s } } if got.Status != "NEEDS_FIX" { t.Errorf("story Status = %q, want NEEDS_FIX (cap reached, no further automation)", got.Status) } } ``` Replace it with: ```go // TestStoryOrchestrator_NeedsFix_CapsAtMaxFixAttempts proves the safety net: // once a chain of maxFixAttempts consecutive fix attempts already exists, // ensureFixAttempt stops spawning new ones and leaves the story at // NEEDS_FIX, exactly like today's fully-manual behavior. st.RootTaskID stays // at oldRoot.ID throughout (it's an immutable anchor); ensureFixAttempt // resolves the chain's tip itself via task.CurrentAttempt. func TestStoryOrchestrator_NeedsFix_CapsAtMaxFixAttempts(t *testing.T) { store, st, oldRoot, _, _ := seedNeedsFixStory(t) pool := &fakePool{} orch := &StoryOrchestrator{Store: store, Pool: pool} // Build a chain of maxFixAttempts prior fix-attempt tasks, each // depending on the one before it, ending at oldRoot -- simulating a // story that has already exhausted its automatic fix attempts. prev := oldRoot for i := 0; i < maxFixAttempts; i++ { nt, err := orch.spawnRoleTask(context.Background(), fmt.Sprintf("Fix attempt %d", i), "builder", []string{prev.ID}, oldRoot, "fix it") if err != nil { t.Fatalf("seed fix-attempt chain: %v", err) } prev = nt } orch.Tick(context.Background()) fixAttempts := store.dependentsWithRole(prev.ID, "builder") if len(fixAttempts) != 0 { t.Fatalf("expected no new fix-attempt task once maxFixAttempts is reached, got %d", len(fixAttempts)) } stories, _ := store.ListStories(storage.StoryFilter{}) var got *story.Story for _, s := range stories { if s.ID == st.ID { got = s } } if got.Status != "NEEDS_FIX" { t.Errorf("story Status = %q, want NEEDS_FIX (cap reached, no further automation)", got.Status) } } ``` - [ ] **Step 5: Run tests to verify they pass** Run: `go test ./internal/scheduler/ -run 'TestStoryOrchestrator_(NeedsFix|EnsureFixAttempt|Arbitration|Retro)' -v` Expected: PASS, all of them — the 3 updated tests, plus `TestStoryOrchestrator_NeedsFix_FixAttemptInstructionsIncludeRejectionReasoning` and `TestStoryOrchestrator_NeedsFix_IdempotentAcrossTicks` (unmodified — they don't assert on `RootTaskID` at all, so they're unaffected by this migration), plus the `TestStoryOrchestrator_Arbitration*` and `TestStoryOrchestrator_Retro_*` clusters (unmodified — the retro tests' `seedDoneStory` fixture has no fix-attempt chain, so `task.CurrentAttempt` resolves to the exact same task a plain `GetTask` would have). Paste the actual output. - [ ] **Step 6: Run the full package suite, then the full repo suite** Run: `go test ./internal/scheduler/...` Expected: PASS for everything — this file's ~1000+ lines of tests exercise far more than just the fix loop; a `RootTaskID`/`CurrentAttempt` resolution mistake would likely show up as a failure somewhere in this package even if not directly targeted. 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/scheduler/story_orchestrator.go internal/scheduler/story_orchestrator_test.go` before committing, and `gofmt -w` any file it flags that wasn't already gofmt-dirty before your changes (check with `git show HEAD: | gofmt -l -` if unsure). - [ ] **Step 7: Commit and push to `main` directly, matching this repo's existing workflow** ```bash git add internal/scheduler/story_orchestrator.go internal/scheduler/story_orchestrator_test.go git commit -m "refactor(scheduler): migrate story fix loop off RootTaskID re-pointing onto CurrentAttempt resolution" ``` ## 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 -n "RootTaskID = " internal/scheduler/story_orchestrator.go` — must return nothing (confirms `RootTaskID` is never written by this file anymore). - [ ] Run `grep -n "task.CurrentAttempt" internal/scheduler/story_orchestrator.go` — should show exactly 2 call sites (`processStory`, `processRetro`) plus 1 more inside `ensureFixAttempt` (3 total).