refactor(scheduler): migrate story fix loop off RootTaskID re-pointing onto CurrentAttempt resolution

2 files changed, 38 insertions, 31 deletions

diff --git a/internal/scheduler/story_orchestrator.go b/internal/scheduler/story_orchestrator.go
index c3d5215..8061b35 100644
--- a/internal/scheduler/story_orchestrator.go
+++ b/internal/scheduler/story_orchestrator.go
@@ -221,9 +221,9 @@ func (o *StoryOrchestrator) processStory(ctx context.Context, st *story.Story) {
 		return
 	}
 
-	root, err := o.Store.GetTask(st.RootTaskID)
+	root, err := task.CurrentAttempt(o.Store, st.RootTaskID)
 	if err != nil {
-		o.logf("story orchestrator: get root task", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err)
+		o.logf("story orchestrator: resolve current root attempt", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err)
 		return
 	}
 	root = o.autoAccept(st, root)
@@ -495,24 +495,27 @@ const maxFixAttempts = 3
 // 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.
+// 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 updated") never spawns duplicates. The
+// "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
-// re-pointed to, regardless of the cap, since refusing to do so would leave
-// a task dangling with nothing tracking it.
+// 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 := o.Store.GetTask(st.RootTaskID)
+	oldRoot, err := task.CurrentAttempt(o.Store, st.RootTaskID)
 	if err != nil {
-		o.logf("story orchestrator: fix attempt: get rejected root task", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err)
+		o.logf("story orchestrator: fix attempt: resolve current root attempt", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err)
 		return
 	}
 
@@ -547,10 +550,9 @@ func (o *StoryOrchestrator) ensureFixAttempt(ctx context.Context, st *story.Stor
 		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)
+		o.logf("story orchestrator: fix attempt: reset story status", "storyID", st.ID, "error", err)
 	}
 }
 
@@ -626,9 +628,9 @@ func (o *StoryOrchestrator) fixAttemptDepth(root *task.Task) int {
 // (auto-accepted from READY exactly like every other task in this pipeline),
 // hands off to finalizeRetro to emit KindRetroCaptured.
 func (o *StoryOrchestrator) processRetro(ctx context.Context, st *story.Story) {
-	root, err := o.Store.GetTask(st.RootTaskID)
+	root, err := task.CurrentAttempt(o.Store, st.RootTaskID)
 	if err != nil {
-		o.logf("story orchestrator: retro: get root task", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err)
+		o.logf("story orchestrator: retro: resolve current root attempt", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err)
 		return
 	}
 
diff --git a/internal/scheduler/story_orchestrator_test.go b/internal/scheduler/story_orchestrator_test.go
index 0ace7e7..200e38e 100644
--- a/internal/scheduler/story_orchestrator_test.go
+++ b/internal/scheduler/story_orchestrator_test.go
@@ -660,7 +660,15 @@ func seedNeedsFixStory(t *testing.T) (*fakeStoryStore, *story.Story, *task.Task,
 // at it (RootTaskID updated, Status reset to IN_PROGRESS) so the very next
 // tick re-enters the normal Builder->Evaluators->Arbitration flow against
 // the new attempt.
-func TestStoryOrchestrator_NeedsFix_SpawnsFixAttemptAndRepointsRoot(t *testing.T) {
+// 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{}
@@ -689,8 +697,8 @@ func TestStoryOrchestrator_NeedsFix_SpawnsFixAttemptAndRepointsRoot(t *testing.T
 	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.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)
@@ -744,10 +752,9 @@ func TestStoryOrchestrator_NeedsFix_IdempotentAcrossTicks(t *testing.T) {
 
 // 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.
+// 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)
 
@@ -768,8 +775,8 @@ func TestStoryOrchestrator_EnsureFixAttempt_FindsExistingSpawnedAttempt(t *testi
 	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.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)
@@ -779,7 +786,9 @@ func TestStoryOrchestrator_EnsureFixAttempt_FindsExistingSpawnedAttempt(t *testi
 // 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.
+// 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)
 
@@ -797,10 +806,6 @@ func TestStoryOrchestrator_NeedsFix_CapsAtMaxFixAttempts(t *testing.T) {
 		}
 		prev = nt
 	}
-	st.RootTaskID = prev.ID
-	if err := store.UpdateStory(st); err != nil {
-		t.Fatalf("persist chained root: %v", err)
-	}
 
 	orch.Tick(context.Background())