diff options
Diffstat (limited to 'internal')
| -rw-r--r-- | internal/scheduler/story_orchestrator.go | 36 | ||||
| -rw-r--r-- | internal/scheduler/story_orchestrator_test.go | 33 |
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()) |