feat(scheduler): automatically spawn and re-point a fix attempt when a story is rejected

1 files changed, 222 insertions, 0 deletions

diff --git a/internal/scheduler/story_orchestrator_test.go b/internal/scheduler/story_orchestrator_test.go
index 0d96840..0ace7e7 100644
--- a/internal/scheduler/story_orchestrator_test.go
+++ b/internal/scheduler/story_orchestrator_test.go
@@ -599,6 +599,228 @@ func TestStoryOrchestrator_ArbitrationApproves_SetsReviewReady(t *testing.T) {
 	}
 }
 
+// seedNeedsFixStory drives a story through the full Builder -> Evaluators ->
+// Arbitration chain to a rejected verdict (mirrors
+// TestStoryOrchestrator_ArbitrationRejects_SetsStoryNeedsFix's own setup),
+// leaving it at status NEEDS_FIX with its original (rejected) root task
+// still in the store -- the starting point ensureFixAttempt operates on.
+// Returns the store, the story (as currently persisted), the rejected root
+// task, the evaluators, and the arbitration task.
+func seedNeedsFixStory(t *testing.T) (*fakeStoryStore, *story.Story, *task.Task, []*task.Task, *task.Task) {
+	t.Helper()
+	store, st, evaluators := seedStoryWithEvaluators(t, task.StateCompleted)
+	oldRoot, err := store.GetTask(st.RootTaskID)
+	if err != nil {
+		t.Fatalf("get root: %v", err)
+	}
+
+	pool := &fakePool{}
+	orch := &StoryOrchestrator{Store: store, Pool: pool}
+	orch.Tick(context.Background()) // spawns arbitration
+
+	arbitrations := store.dependentsWithRole(evaluators[0].ID, "planner")
+	if len(arbitrations) != 1 {
+		t.Fatalf("expected 1 arbitration task, got %d", len(arbitrations))
+	}
+	arb := arbitrations[0]
+
+	payload, _ := json.Marshal(struct {
+		Approved  bool   `json:"approved"`
+		Reasoning string `json:"reasoning"`
+	}{Approved: false, Reasoning: "breaks the running-log panel styling"})
+	if err := store.CreateEvent(&event.Event{
+		TaskID:  arb.ID,
+		Kind:    event.KindVerdictReported,
+		Actor:   event.ActorAgent,
+		Payload: payload,
+	}); err != nil {
+		t.Fatalf("seed verdict event: %v", err)
+	}
+	store.setTaskState(arb.ID, task.StateCompleted)
+	store.setTaskSummary(arb.ID, "found a real problem")
+
+	orch.Tick(context.Background()) // finalizeArbitration runs, sets NEEDS_FIX
+
+	stories, _ := store.ListStories(storage.StoryFilter{})
+	var got *story.Story
+	for _, s := range stories {
+		if s.ID == st.ID {
+			got = s
+		}
+	}
+	if got == nil || got.Status != "NEEDS_FIX" {
+		t.Fatalf("setup failed: story = %+v, want status NEEDS_FIX", got)
+	}
+	return store, got, oldRoot, evaluators, arb
+}
+
+// TestStoryOrchestrator_NeedsFix_SpawnsFixAttemptAndRepointsRoot 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 is re-pointed
+// 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) {
+	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)
+	}
+}
+
+// TestStoryOrchestrator_NeedsFix_FixAttemptInstructionsIncludeRejectionReasoning
+// proves the fix attempt's instructions carry the arbitration's structured
+// rejection reasoning (read via the KindVerdictReported event), not just a
+// generic "try again" -- the whole point of automating this loop is that the
+// next attempt has something concrete to act on.
+func TestStoryOrchestrator_NeedsFix_FixAttemptInstructionsIncludeRejectionReasoning(t *testing.T) {
+	store, _, 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, got %d", len(fixAttempts))
+	}
+	instructions := fixAttempts[0].Agent.Instructions
+	if !strings.Contains(instructions, "breaks the running-log panel styling") {
+		t.Errorf("fix-attempt instructions do not include the arbitration's rejection reasoning:\n%s", instructions)
+	}
+	if !strings.Contains(instructions, "Test Story") {
+		t.Errorf("fix-attempt instructions do not include the story name:\n%s", instructions)
+	}
+}
+
+// TestStoryOrchestrator_NeedsFix_IdempotentAcrossTicks proves repeated ticks
+// don't spawn more than one fix-attempt task -- after the first tick the
+// story's status flips to IN_PROGRESS, so the NEEDS_FIX branch simply isn't
+// re-entered on subsequent ticks (the normal Builder-not-complete-yet path
+// takes over instead, since the fix attempt is freshly QUEUED).
+func TestStoryOrchestrator_NeedsFix_IdempotentAcrossTicks(t *testing.T) {
+	store, _, oldRoot, _, _ := seedNeedsFixStory(t)
+
+	pool := &fakePool{}
+	orch := &StoryOrchestrator{Store: store, Pool: pool}
+	orch.Tick(context.Background())
+	orch.Tick(context.Background())
+	orch.Tick(context.Background())
+
+	fixAttempts := store.dependentsWithRole(oldRoot.ID, "builder")
+	if len(fixAttempts) != 1 {
+		t.Fatalf("expected exactly 1 fix-attempt task after repeated ticks, got %d", len(fixAttempts))
+	}
+}
+
+// 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)
+	}
+}
+
+// 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)
+	}
+}
+
 // TestStoryOrchestrator_DoesNothing_WhenBuilderNotComplete proves the
 // orchestrator is inert for a story whose builder task hasn't reached
 // COMPLETED yet and isn't auto-acceptable either (still RUNNING — not