From 063a6c1661a91370e2f70d7fab670acd09561619 Mon Sep 17 00:00:00 2001 From: Claudomator Agent Date: Fri, 10 Jul 2026 05:49:36 +0000 Subject: refactor(scheduler): generalize finalizeArbitration to any builder-role node, add nested fix-attempt spawning and acceptance-criteria fallback --- internal/scheduler/story_orchestrator_test.go | 277 +++++++++++++++++++++++++- 1 file changed, 275 insertions(+), 2 deletions(-) (limited to 'internal/scheduler/story_orchestrator_test.go') diff --git a/internal/scheduler/story_orchestrator_test.go b/internal/scheduler/story_orchestrator_test.go index a13659d..03d4437 100644 --- a/internal/scheduler/story_orchestrator_test.go +++ b/internal/scheduler/story_orchestrator_test.go @@ -444,6 +444,279 @@ func TestStoryOrchestrator_EmitsEvalVerdict_OncePerEvaluator(t *testing.T) { } } +// TestStoryOrchestrator_FinalizeArbitration_NestedNodeApproved_DoesNotTouchStoryStatus +// proves finalizeArbitration, generalized to operate on any builder-role +// node (not just the story root), promotes a NESTED node to COMPLETED on +// approval without touching story.Status at all -- REVIEW_READY is reserved +// for the actual root position (node.ParentTaskID == ""). +func TestStoryOrchestrator_FinalizeArbitration_NestedNodeApproved_DoesNotTouchStoryStatus(t *testing.T) { + store := newFakeStoryStore() + root := builderTask("nested-approve-root", task.StateBlocked) + store.tasks[root.ID] = root + st := newStoryWithRoot("nested-approve-story", root.ID, "IN_PROGRESS") + store.stories[st.ID] = st + + nested := builderTask("nested-approve-node", task.StateReady) + nested.ParentTaskID = root.ID + store.tasks[nested.ID] = nested + + arb := &task.Task{ + ID: "nested-approve-arb", + Name: "Arbitration", + Agent: task.AgentConfig{Role: arbitrationRole}, + State: task.StateCompleted, + Summary: "ship it", + } + store.tasks[arb.ID] = arb + payload, _ := json.Marshal(struct { + Approved bool `json:"approved"` + Reasoning string `json:"reasoning"` + }{Approved: true, Reasoning: "meets criteria"}) + 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) + } + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.finalizeArbitration(context.Background(), st, nested, arb) + + got, err := store.GetTask(nested.ID) + if err != nil { + t.Fatal(err) + } + if got.State != task.StateCompleted { + t.Errorf("nested node State = %v, want COMPLETED", got.State) + } + + stories, _ := store.ListStories(storage.StoryFilter{}) + var gotStory *story.Story + for _, s := range stories { + if s.ID == st.ID { + gotStory = s + } + } + if gotStory.Status != "IN_PROGRESS" { + t.Errorf("story Status = %q, want unchanged IN_PROGRESS (only root's own arbitration should touch story.Status)", gotStory.Status) + } +} + +// TestStoryOrchestrator_FinalizeArbitration_NestedNodeRejected_SpawnsFixAttemptDirectly +// proves the nested-rejection path: since a nested position has no +// story-level field to poll (unlike the root's NEEDS_FIX/ensureFixAttempt +// flow), its fix-attempt is spawned immediately, right here -- same role, +// same ParentTaskID as the rejected node (so task.CurrentAttempt resolves +// it correctly), and story.Status is left untouched. +func TestStoryOrchestrator_FinalizeArbitration_NestedNodeRejected_SpawnsFixAttemptDirectly(t *testing.T) { + store := newFakeStoryStore() + root := builderTask("nested-reject-root", task.StateBlocked) + store.tasks[root.ID] = root + st := newStoryWithRoot("nested-reject-story", root.ID, "IN_PROGRESS") + store.stories[st.ID] = st + + nested := builderTask("nested-reject-node", task.StateReady) + nested.ParentTaskID = root.ID + store.tasks[nested.ID] = nested + + arb := &task.Task{ + ID: "nested-reject-arb", + Name: "Arbitration", + Agent: task.AgentConfig{Role: arbitrationRole}, + State: task.StateCompleted, + Summary: "found a real problem", + } + store.tasks[arb.ID] = arb + payload, _ := json.Marshal(struct { + Approved bool `json:"approved"` + Reasoning string `json:"reasoning"` + }{Approved: false, Reasoning: "breaks the widget"}) + 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) + } + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.finalizeArbitration(context.Background(), st, nested, arb) + + got, err := store.GetTask(nested.ID) + if err != nil { + t.Fatal(err) + } + if got.State != task.StateReady { + t.Errorf("rejected nested node State = %v, want unchanged READY", got.State) + } + + fixAttempts := store.dependentsWithRole(nested.ID, "builder") + if len(fixAttempts) != 1 { + t.Fatalf("expected exactly 1 fix-attempt task depending on the rejected nested node, got %d", len(fixAttempts)) + } + fix := fixAttempts[0] + if fix.ParentTaskID != nested.ParentTaskID { + t.Errorf("fix attempt ParentTaskID = %q, want %q (same position as the rejected node)", fix.ParentTaskID, nested.ParentTaskID) + } + if fix.State != task.StateQueued { + t.Errorf("fix attempt State = %v, want QUEUED", fix.State) + } + + stories, _ := store.ListStories(storage.StoryFilter{}) + var gotStory *story.Story + for _, s := range stories { + if s.ID == st.ID { + gotStory = s + } + } + if gotStory.Status != "IN_PROGRESS" { + t.Errorf("story Status = %q, want unchanged IN_PROGRESS", gotStory.Status) + } +} + +// TestStoryOrchestrator_FinalizeArbitration_NestedNodeRejected_CapsAtMaxFixAttempts +// mirrors the root's own safety net (TestStoryOrchestrator_NeedsFix_CapsAtMaxFixAttempts) +// for a nested position: once a chain of maxFixAttempts consecutive fix +// attempts already exists at this position, no further fix attempt is +// spawned. +func TestStoryOrchestrator_FinalizeArbitration_NestedNodeRejected_CapsAtMaxFixAttempts(t *testing.T) { + store := newFakeStoryStore() + root := builderTask("nested-cap-root", task.StateBlocked) + store.tasks[root.ID] = root + st := newStoryWithRoot("nested-cap-story", root.ID, "IN_PROGRESS") + store.stories[st.ID] = st + + nested := builderTask("nested-cap-node", task.StateReady) + nested.ParentTaskID = root.ID + store.tasks[nested.ID] = nested + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + + // Build a chain of maxFixAttempts prior fix-attempt tasks at this same + // nested position, ending at prev -- simulating a position that has + // already exhausted its automatic fix attempts. + prev := nested + for i := 0; i < maxFixAttempts; i++ { + nt, err := orch.spawnRoleTask(context.Background(), fmt.Sprintf("Fix attempt %d", i), "builder", []string{prev.ID}, root.ID, root, "fix it") + if err != nil { + t.Fatalf("seed fix-attempt chain: %v", err) + } + prev = nt + } + + arb := &task.Task{ + ID: "nested-cap-arb", + Name: "Arbitration", + Agent: task.AgentConfig{Role: arbitrationRole}, + State: task.StateCompleted, + Summary: "still broken", + } + store.tasks[arb.ID] = arb + payload, _ := json.Marshal(struct { + Approved bool `json:"approved"` + Reasoning string `json:"reasoning"` + }{Approved: false, Reasoning: "still broken"}) + 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) + } + + orch.finalizeArbitration(context.Background(), st, prev, arb) + + 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)) + } +} + +// TestStoryOrchestrator_FinalizeArbitration_RootStillSetsStoryStatus proves +// the generalized finalizeArbitration still drives story.Status exactly as +// before when node IS the root position (node.ParentTaskID == "") -- the +// one case where touching story.Status is still correct, now decided by +// node.ParentTaskID rather than being the function's only mode. +func TestStoryOrchestrator_FinalizeArbitration_RootStillSetsStoryStatus(t *testing.T) { + store := newFakeStoryStore() + root := builderTask("root-status-root", task.StateReady) + store.tasks[root.ID] = root + st := newStoryWithRoot("root-status-story", root.ID, "VALIDATING") + store.stories[st.ID] = st + + arb := &task.Task{ + ID: "root-status-arb", + Name: "Arbitration", + Agent: task.AgentConfig{Role: arbitrationRole}, + State: task.StateCompleted, + Summary: "ship it", + } + store.tasks[arb.ID] = arb + payload, _ := json.Marshal(struct { + Approved bool `json:"approved"` + Reasoning string `json:"reasoning"` + }{Approved: true, Reasoning: "meets criteria"}) + 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) + } + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.finalizeArbitration(context.Background(), st, root, arb) + + gotRoot, err := store.GetTask(root.ID) + if err != nil { + t.Fatal(err) + } + if gotRoot.State != task.StateCompleted { + t.Errorf("root State = %v, want COMPLETED", gotRoot.State) + } + + stories, _ := store.ListStories(storage.StoryFilter{}) + var gotStory *story.Story + for _, s := range stories { + if s.ID == st.ID { + gotStory = s + } + } + if gotStory.Status != "REVIEW_READY" { + t.Errorf("story Status = %q, want REVIEW_READY (root's own approval must still drive it)", gotStory.Status) + } +} + +// TestStoryOrchestrator_FinalizeArbitration_IdempotentViaArbitrationDecidedEvent +// proves the generalized idempotency check (a KindArbitrationDecided event +// already referencing this arbitration.ID) correctly no-ops a repeated call +// for the same already-decided arbitration -- the mechanism every node in a +// story's tree now shares, replacing the root-only story.Status == +// "VALIDATING" gate. +func TestStoryOrchestrator_FinalizeArbitration_IdempotentViaArbitrationDecidedEvent(t *testing.T) { + store := newFakeStoryStore() + root := builderTask("idempotent-root", task.StateReady) + store.tasks[root.ID] = root + st := newStoryWithRoot("idempotent-story", root.ID, "VALIDATING") + store.stories[st.ID] = st + + arb := &task.Task{ + ID: "idempotent-arb", + Name: "Arbitration", + Agent: task.AgentConfig{Role: arbitrationRole}, + State: task.StateCompleted, + Summary: "ship it", + } + store.tasks[arb.ID] = arb + payload, _ := json.Marshal(struct { + Approved bool `json:"approved"` + Reasoning string `json:"reasoning"` + }{Approved: true, Reasoning: "meets criteria"}) + 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) + } + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.finalizeArbitration(context.Background(), st, root, arb) + orch.finalizeArbitration(context.Background(), st, root, arb) + orch.finalizeArbitration(context.Background(), st, root, arb) + + decided := store.eventsOfKind(event.KindArbitrationDecided) + if len(decided) != 1 { + t.Fatalf("expected exactly 1 arbitration_decided event after 3 calls, got %d", len(decided)) + } +} + // TestStoryOrchestrator_ArbitrationCompletes_EmitsDecisionAndReviewReady is // verification item (d): arbitration reaching COMPLETED emits // KindArbitrationDecided and moves the story to REVIEW_READY. @@ -788,7 +1061,7 @@ func TestStoryOrchestrator_EnsureFixAttempt_FindsExistingSpawnedAttempt(t *testi 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") + 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) } @@ -827,7 +1100,7 @@ func TestStoryOrchestrator_NeedsFix_CapsAtMaxFixAttempts(t *testing.T) { // 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") + 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) } -- cgit v1.2.3