diff options
| author | Claude Sonnet 5 <noreply@anthropic.com> | 2026-07-04 04:08:41 +0000 |
|---|---|---|
| committer | Claude Sonnet 5 <noreply@anthropic.com> | 2026-07-04 04:08:41 +0000 |
| commit | e4087a7dc133fe8c8523ca585b1841ff2b0be2d9 (patch) | |
| tree | ae3de24d5d1b6abd5634eeab6fd99424baf847dc /internal/scheduler/story_orchestrator_test.go | |
| parent | 392c7c1ada310b2f928dca89b75ba628478f7694 (diff) | |
feat(story): add StoryOrchestrator -- Builder->Evaluators->Arbitration->accept (Phase 7b)
A deterministic, poll-based watcher (internal/scheduler.StoryOrchestrator,
sibling to the Phase 5 Scheduler) that drives a story.Story through its
execution pipeline, rather than relying on an LLM agent to correctly
orchestrate its own fan-out via tool calls.
Mechanism: polling, not a handleRunResult hook. Every task the orchestrator
watches (a story's root/Builder task, 4 Evaluators, Arbitration) is
top-level (no ParentTaskID), and executor.Pool.handleRunResult only ever
lands a top-level task at READY or BLOCKED -- never COMPLETED directly, since
that transition normally requires a human/chatbot POST /api/tasks/{id}/accept
in a different package. A handleRunResult hook would never observe it;
polling doesn't care how/whether a task reached a given state.
Stages: Builder COMPLETED -> spawn 4 role-typed Evaluator tasks
(evaluator_quality/security/correctness/performance, DependsOn: [builder],
no ParentTaskID -- true DAG siblings, not delegated subtasks) + story ->
VALIDATING. Each Evaluator COMPLETED -> emit KindEvalVerdict (attached to
the story's ID, so one GET /api/stories/{id}/events call surfaces every
verdict). All 4 Evaluators COMPLETED -> spawn 1 Arbitration task
(role: planner, DependsOn: all 4 evaluator IDs). Arbitration COMPLETED ->
emit KindArbitrationDecided, story -> REVIEW_READY. POST
/api/stories/{id}/accept (mirrors handleAcceptTask) -> DONE, emits
KindHumanAccepted.
Fixes a gap caught before merging: since none of Builder/Evaluators/
Arbitration have a ParentTaskID, none of them auto-complete -- each would
otherwise need a separate manual /api/tasks/{id}/accept, meaning 6 human
clicks per story before ever reaching the intended single story-level gate.
StoryOrchestrator.autoAccept now transitions each of these specific tasks
READY->COMPLETED itself (via the same validated Store.UpdateTaskState path
acceptTask uses), scoped only to tasks already established as part of a
story's pipeline (root task, or role-matched dependents from
ensureEvaluators/ensureArbitration) -- never a blanket sweep of unrelated
READY tasks. This makes POST /api/stories/{id}/accept the system's only
required human touchpoint for the whole chain, matching the design goal
that story (not task/subtask) is the human-interaction atom.
Idempotency: structural for task-creation stages (ensureEvaluators/
ensureArbitration check ListDependents for already-existing role-matched
tasks before creating -- crash/restart-safe); story.Status=="VALIDATING"
gates the Arbitration->REVIEW_READY write (nothing further downstream to
check structurally there); an in-memory handledVerdicts set (mirrors
Scheduler.handled) dedupes per-evaluator KindEvalVerdict emission across
poll ticks, resetting harmlessly on restart.
Documented simplification: finalizeArbitration never parses the Arbitration
summary for approve/reject -- always routes to REVIEW_READY; NEEDS_FIX is
manually settable via PUT /api/stories/{id}. A later phase could close this
with a dedicated verdict-reporting AgentChannel method instead of parsing
free text.
go build/vet/test -race -count=1 all pass, full suite (20 packages).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V1moSNCJRcP6kykA4tyUSs
Diffstat (limited to 'internal/scheduler/story_orchestrator_test.go')
| -rw-r--r-- | internal/scheduler/story_orchestrator_test.go | 769 |
1 files changed, 769 insertions, 0 deletions
diff --git a/internal/scheduler/story_orchestrator_test.go b/internal/scheduler/story_orchestrator_test.go new file mode 100644 index 0000000..bee326d --- /dev/null +++ b/internal/scheduler/story_orchestrator_test.go @@ -0,0 +1,769 @@ +package scheduler + +import ( + "context" + "encoding/json" + "fmt" + "sync" + "testing" + + "github.com/google/uuid" + "github.com/thepeterstone/claudomator/internal/event" + "github.com/thepeterstone/claudomator/internal/storage" + "github.com/thepeterstone/claudomator/internal/story" + "github.com/thepeterstone/claudomator/internal/task" +) + +// fakeStoryStore is a minimal, in-memory implementation of StoryStore for +// unit-testing StoryOrchestrator without a real SQLite database. Mirrors the +// fakeStore pattern already used by scheduler_test.go for the Phase 5 +// Scheduler. +type fakeStoryStore struct { + mu sync.Mutex + stories map[string]*story.Story + tasks map[string]*task.Task + events []*event.Event +} + +func newFakeStoryStore() *fakeStoryStore { + return &fakeStoryStore{ + stories: make(map[string]*story.Story), + tasks: make(map[string]*task.Task), + } +} + +func (f *fakeStoryStore) ListStories(_ storage.StoryFilter) ([]*story.Story, error) { + f.mu.Lock() + defer f.mu.Unlock() + var out []*story.Story + for _, s := range f.stories { + cp := *s + out = append(out, &cp) + } + return out, nil +} + +func (f *fakeStoryStore) UpdateStory(st *story.Story) error { + f.mu.Lock() + defer f.mu.Unlock() + if _, ok := f.stories[st.ID]; !ok { + return fmt.Errorf("story %q not found", st.ID) + } + cp := *st + f.stories[st.ID] = &cp + return nil +} + +func (f *fakeStoryStore) GetTask(id string) (*task.Task, error) { + f.mu.Lock() + defer f.mu.Unlock() + t, ok := f.tasks[id] + if !ok { + return nil, fmt.Errorf("task %q not found", id) + } + cp := *t + return &cp, nil +} + +func (f *fakeStoryStore) ListDependents(taskID string) ([]*task.Task, error) { + f.mu.Lock() + defer f.mu.Unlock() + var out []*task.Task + for _, t := range f.tasks { + for _, d := range t.DependsOn { + if d == taskID { + cp := *t + out = append(out, &cp) + break + } + } + } + return out, nil +} + +func (f *fakeStoryStore) CreateTask(t *task.Task) error { + f.mu.Lock() + defer f.mu.Unlock() + if _, ok := f.tasks[t.ID]; ok { + return fmt.Errorf("task %q already exists", t.ID) + } + cp := *t + f.tasks[t.ID] = &cp + return nil +} + +func (f *fakeStoryStore) UpdateTaskState(id string, newState task.State) error { + f.mu.Lock() + defer f.mu.Unlock() + t, ok := f.tasks[id] + if !ok { + return fmt.Errorf("task %q not found", id) + } + t.State = newState + return nil +} + +func (f *fakeStoryStore) CreateEvent(e *event.Event) error { + f.mu.Lock() + defer f.mu.Unlock() + e.ID = uuid.NewString() + f.events = append(f.events, e) + return nil +} + +func (f *fakeStoryStore) setTaskState(id string, s task.State) { + f.mu.Lock() + defer f.mu.Unlock() + if t, ok := f.tasks[id]; ok { + t.State = s + } +} + +func (f *fakeStoryStore) setTaskSummary(id, summary string) { + f.mu.Lock() + defer f.mu.Unlock() + if t, ok := f.tasks[id]; ok { + t.Summary = summary + } +} + +func (f *fakeStoryStore) eventsOfKind(k event.Kind) []*event.Event { + f.mu.Lock() + defer f.mu.Unlock() + var out []*event.Event + for _, e := range f.events { + if e.Kind == k { + out = append(out, e) + } + } + return out +} + +func (f *fakeStoryStore) dependentsWithRole(taskID, role string) []*task.Task { + deps, _ := f.ListDependents(taskID) + var out []*task.Task + for _, d := range deps { + if d.Agent.Role == role { + out = append(out, d) + } + } + return out +} + +func builderTask(id string, state task.State) *task.Task { + return &task.Task{ + ID: id, + Name: "Builder", + Agent: task.AgentConfig{Type: "claude", Role: "builder", Instructions: "build it"}, + RepositoryURL: "git@example.com:org/repo.git", + State: state, + } +} + +func newStoryWithRoot(id, rootTaskID, status string) *story.Story { + return &story.Story{ID: id, Name: "Test Story", Status: status, RootTaskID: rootTaskID} +} + +// TestStoryOrchestrator_SpawnsEvaluators_WhenBuilderCompletes is verification +// item (a): a builder task reaching COMPLETED for a story spawns exactly 4 +// evaluator tasks with correct roles/depends_on, moves the story to +// VALIDATING. +func TestStoryOrchestrator_SpawnsEvaluators_WhenBuilderCompletes(t *testing.T) { + store := newFakeStoryStore() + root := builderTask("builder-1", task.StateCompleted) + store.tasks[root.ID] = root + st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS") + store.stories[st.ID] = st + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) + + deps, err := store.ListDependents(root.ID) + if err != nil { + t.Fatal(err) + } + if len(deps) != 4 { + t.Fatalf("expected 4 evaluator tasks, got %d: %+v", len(deps), deps) + } + gotRoles := map[string]bool{} + for _, d := range deps { + gotRoles[d.Agent.Role] = true + if len(d.DependsOn) != 1 || d.DependsOn[0] != root.ID { + t.Errorf("evaluator %s: DependsOn = %+v, want [%s]", d.ID, d.DependsOn, root.ID) + } + if d.ParentTaskID != "" { + t.Errorf("evaluator %s: ParentTaskID = %q, want empty (DAG sibling, not subtask)", d.ID, d.ParentTaskID) + } + if d.State != task.StateQueued { + t.Errorf("evaluator %s: State = %v, want QUEUED", d.ID, d.State) + } + } + for _, r := range evaluatorRoles { + if !gotRoles[r] { + t.Errorf("missing evaluator with role %q", r) + } + } + if pool.submitCount() != 4 { + t.Fatalf("expected 4 pool submissions, got %d", pool.submitCount()) + } + + got, err := func() (*story.Story, error) { + stories, err := store.ListStories(storage.StoryFilter{}) + if err != nil { + return nil, err + } + for _, s := range stories { + if s.ID == st.ID { + return s, nil + } + } + return nil, fmt.Errorf("not found") + }() + if err != nil { + t.Fatal(err) + } + if got.Status != "VALIDATING" { + t.Errorf("story status: want VALIDATING, got %q", got.Status) + } +} + +// TestStoryOrchestrator_DoesNotDuplicateEvaluators is verification item (b): +// re-checking the same story after evaluators already exist does not spawn +// duplicates. +func TestStoryOrchestrator_DoesNotDuplicateEvaluators(t *testing.T) { + store := newFakeStoryStore() + root := builderTask("builder-1", task.StateCompleted) + store.tasks[root.ID] = root + st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS") + store.stories[st.ID] = st + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) + orch.Tick(context.Background()) + orch.Tick(context.Background()) + + deps, _ := store.ListDependents(root.ID) + if len(deps) != 4 { + t.Fatalf("expected exactly 4 evaluator tasks after 3 ticks, got %d", len(deps)) + } + if pool.submitCount() != 4 { + t.Fatalf("expected exactly 4 submissions after 3 ticks, got %d", pool.submitCount()) + } +} + +// evaluatorTask builds a completed (or not) evaluator task depending on +// rootID with the given role. +func evaluatorTask(id, rootID, role string, state task.State) *task.Task { + return &task.Task{ + ID: id, + Name: role, + Agent: task.AgentConfig{Role: role}, + DependsOn: []string{rootID}, + State: state, + Summary: "looks good", + } +} + +// seedStoryWithEvaluators wires up a story whose builder is COMPLETED and +// whose 4 evaluators already exist (in the given state), returning the +// fakeStoryStore, story, and evaluator tasks (in evaluatorRoles order). +func seedStoryWithEvaluators(t *testing.T, evalState task.State) (*fakeStoryStore, *story.Story, []*task.Task) { + t.Helper() + store := newFakeStoryStore() + root := builderTask("builder-1", task.StateCompleted) + store.tasks[root.ID] = root + st := newStoryWithRoot("story-1", root.ID, "VALIDATING") + store.stories[st.ID] = st + + evaluators := make([]*task.Task, len(evaluatorRoles)) + for i, r := range evaluatorRoles { + ev := evaluatorTask(fmt.Sprintf("eval-%d", i), root.ID, r, evalState) + store.tasks[ev.ID] = ev + evaluators[i] = ev + } + return store, st, evaluators +} + +// TestStoryOrchestrator_SpawnsArbitration_WhenAllEvaluatorsComplete is +// verification item (c): all 4 evaluators reaching COMPLETED spawns exactly +// 1 arbitration task depending on all 4. +func TestStoryOrchestrator_SpawnsArbitration_WhenAllEvaluatorsComplete(t *testing.T) { + store, _, evaluators := seedStoryWithEvaluators(t, task.StateCompleted) + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) + + arbitrations := store.dependentsWithRole(evaluators[0].ID, "planner") + if len(arbitrations) != 1 { + t.Fatalf("expected exactly 1 arbitration task, got %d: %+v", len(arbitrations), arbitrations) + } + arb := arbitrations[0] + if len(arb.DependsOn) != len(evaluators) { + t.Fatalf("arbitration DependsOn = %+v, want all %d evaluator IDs", arb.DependsOn, len(evaluators)) + } + for _, ev := range evaluators { + if !dependsOnAll(arb, []string{ev.ID}) { + t.Errorf("arbitration does not depend on evaluator %s", ev.ID) + } + } + if arb.ParentTaskID != "" { + t.Errorf("arbitration ParentTaskID = %q, want empty", arb.ParentTaskID) + } + if arb.State != task.StateQueued { + t.Errorf("arbitration State = %v, want QUEUED", arb.State) + } + + // Re-ticking must not spawn a second arbitration task. + orch.Tick(context.Background()) + orch.Tick(context.Background()) + arbitrations = store.dependentsWithRole(evaluators[0].ID, "planner") + if len(arbitrations) != 1 { + t.Fatalf("expected exactly 1 arbitration task after repeated ticks, got %d", len(arbitrations)) + } +} + +// TestStoryOrchestrator_DoesNotSpawnArbitration_UntilAllEvaluatorsComplete +// proves the fan-in gate: even with 3 of 4 evaluators COMPLETED, no +// arbitration task is created yet. +func TestStoryOrchestrator_DoesNotSpawnArbitration_UntilAllEvaluatorsComplete(t *testing.T) { + store, _, evaluators := seedStoryWithEvaluators(t, task.StateCompleted) + // Knock one evaluator back to RUNNING. + store.setTaskState(evaluators[0].ID, task.StateRunning) + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) + + arbitrations := store.dependentsWithRole(evaluators[0].ID, "planner") + if len(arbitrations) != 0 { + t.Fatalf("expected no arbitration task while an evaluator is incomplete, got %d", len(arbitrations)) + } +} + +// TestStoryOrchestrator_EmitsEvalVerdict_OncePerEvaluator proves +// maybeEmitVerdict fires exactly once per completed evaluator, attached to +// the story's ID, even across repeated ticks. +func TestStoryOrchestrator_EmitsEvalVerdict_OncePerEvaluator(t *testing.T) { + store, st, evaluators := seedStoryWithEvaluators(t, task.StateCompleted) + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) + orch.Tick(context.Background()) + orch.Tick(context.Background()) + + verdicts := store.eventsOfKind(event.KindEvalVerdict) + if len(verdicts) != len(evaluators) { + t.Fatalf("expected exactly %d eval_verdict events, got %d", len(evaluators), len(verdicts)) + } + seenTaskIDs := map[string]bool{} + for _, e := range verdicts { + if e.TaskID != st.ID { + t.Errorf("eval_verdict event attached to %q, want story ID %q", e.TaskID, st.ID) + } + var payload struct { + TaskID string `json:"task_id"` + Role string `json:"role"` + Summary string `json:"summary"` + } + if err := json.Unmarshal(e.Payload, &payload); err != nil { + t.Fatalf("unmarshal payload: %v", err) + } + seenTaskIDs[payload.TaskID] = true + if payload.Role == "" { + t.Errorf("payload missing role: %+v", payload) + } + if payload.Summary != "looks good" { + t.Errorf("payload summary = %q, want %q", payload.Summary, "looks good") + } + } + for _, ev := range evaluators { + if !seenTaskIDs[ev.ID] { + t.Errorf("no eval_verdict event found for evaluator %s", ev.ID) + } + } +} + +// TestStoryOrchestrator_ArbitrationCompletes_EmitsDecisionAndReviewReady is +// verification item (d): arbitration reaching COMPLETED emits +// KindArbitrationDecided and moves the story to REVIEW_READY. +func TestStoryOrchestrator_ArbitrationCompletes_EmitsDecisionAndReviewReady(t *testing.T) { + store, st, evaluators := seedStoryWithEvaluators(t, task.StateCompleted) + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + // Tick 1: spawns the arbitration task. + orch.Tick(context.Background()) + + arbitrations := store.dependentsWithRole(evaluators[0].ID, "planner") + if len(arbitrations) != 1 { + t.Fatalf("expected 1 arbitration task, got %d", len(arbitrations)) + } + arb := arbitrations[0] + store.setTaskState(arb.ID, task.StateCompleted) + store.setTaskSummary(arb.ID, "ship it") + + // Tick 2: arbitration is now COMPLETED. + orch.Tick(context.Background()) + + decided := store.eventsOfKind(event.KindArbitrationDecided) + if len(decided) != 1 { + t.Fatalf("expected exactly 1 arbitration_decided event, got %d", len(decided)) + } + if decided[0].TaskID != st.ID { + t.Errorf("arbitration_decided attached to %q, want story ID %q", decided[0].TaskID, st.ID) + } + var payload struct { + TaskID string `json:"task_id"` + Summary string `json:"summary"` + } + if err := json.Unmarshal(decided[0].Payload, &payload); err != nil { + t.Fatalf("unmarshal payload: %v", err) + } + if payload.TaskID != arb.ID || payload.Summary != "ship it" { + t.Errorf("unexpected payload: %+v", payload) + } + + 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.Status != "REVIEW_READY" { + t.Errorf("story status: want REVIEW_READY, got %q", got.Status) + } + + // Tick 3+: must not re-emit or re-decide. + orch.Tick(context.Background()) + orch.Tick(context.Background()) + decided = store.eventsOfKind(event.KindArbitrationDecided) + if len(decided) != 1 { + t.Fatalf("expected still exactly 1 arbitration_decided event after repeated ticks, got %d", len(decided)) + } +} + +// 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 +// READY, so autoAccept has nothing to do). +func TestStoryOrchestrator_DoesNothing_WhenBuilderNotComplete(t *testing.T) { + store := newFakeStoryStore() + root := builderTask("builder-1", task.StateRunning) + store.tasks[root.ID] = root + st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS") + store.stories[st.ID] = st + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) + + deps, _ := store.ListDependents(root.ID) + if len(deps) != 0 { + t.Fatalf("expected no evaluator tasks while builder is RUNNING, got %d", len(deps)) + } + if pool.submitCount() != 0 { + t.Fatalf("expected no submissions, got %d", pool.submitCount()) + } + got, err := store.GetTask(root.ID) + if err != nil { + t.Fatal(err) + } + if got.State != task.StateRunning { + t.Errorf("builder state must be untouched: want RUNNING, got %v", got.State) + } +} + +// TestStoryOrchestrator_AutoAcceptsReadyBuilder is the core regression test +// for the auto-accept fix: a builder task sitting at READY (execution +// succeeded, awaiting what would otherwise be a manual +// POST /api/tasks/{id}/accept) is transitioned to COMPLETED by the +// orchestrator itself — with no external accept call — and, because that +// unblocks Stage 1 in the same tick, the 4 evaluators are spawned +// immediately too. +func TestStoryOrchestrator_AutoAcceptsReadyBuilder(t *testing.T) { + store := newFakeStoryStore() + root := builderTask("builder-1", task.StateReady) + store.tasks[root.ID] = root + st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS") + store.stories[st.ID] = st + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) + + got, err := store.GetTask(root.ID) + if err != nil { + t.Fatal(err) + } + if got.State != task.StateCompleted { + t.Fatalf("builder should be auto-accepted to COMPLETED, got %v", got.State) + } + + deps, _ := store.ListDependents(root.ID) + if len(deps) != 4 { + t.Fatalf("expected 4 evaluator tasks spawned in the same tick the builder auto-accepts, got %d", len(deps)) + } +} + +// TestStoryOrchestrator_AutoAcceptsReadyEvaluators proves READY evaluator +// tasks are auto-accepted to COMPLETED by the orchestrator, with no external +// accept call, and that doing so unblocks arbitration spawning. +func TestStoryOrchestrator_AutoAcceptsReadyEvaluators(t *testing.T) { + store, _, evaluators := seedStoryWithEvaluators(t, task.StateReady) + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) + + for _, ev := range evaluators { + got, err := store.GetTask(ev.ID) + if err != nil { + t.Fatal(err) + } + if got.State != task.StateCompleted { + t.Errorf("evaluator %s should be auto-accepted to COMPLETED, got %v", ev.ID, got.State) + } + } + + arbitrations := store.dependentsWithRole(evaluators[0].ID, "planner") + if len(arbitrations) != 1 { + t.Fatalf("expected arbitration spawned once all evaluators auto-accept, got %d", len(arbitrations)) + } +} + +// TestStoryOrchestrator_AutoAcceptsReadyArbitration proves a READY +// arbitration task is auto-accepted to COMPLETED by the orchestrator, with +// no external accept call, and that this in turn triggers +// finalizeArbitration (KindArbitrationDecided + REVIEW_READY) in the same +// tick. +func TestStoryOrchestrator_AutoAcceptsReadyArbitration(t *testing.T) { + store, st, evaluators := seedStoryWithEvaluators(t, task.StateCompleted) + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + // Tick 1: spawns the arbitration task (starts at QUEUED). + orch.Tick(context.Background()) + + arbitrations := store.dependentsWithRole(evaluators[0].ID, "planner") + if len(arbitrations) != 1 { + t.Fatalf("expected 1 arbitration task, got %d", len(arbitrations)) + } + arb := arbitrations[0] + // Simulate the arbitration's execution succeeding (RUNNING -> READY), + // exactly as executor.Pool.handleRunResult would do for any top-level + // task — without ever calling POST /api/tasks/{id}/accept. + store.setTaskState(arb.ID, task.StateReady) + store.setTaskSummary(arb.ID, "approved") + + // Tick 2: orchestrator must auto-accept READY -> COMPLETED itself. + orch.Tick(context.Background()) + + got, err := store.GetTask(arb.ID) + if err != nil { + t.Fatal(err) + } + if got.State != task.StateCompleted { + t.Fatalf("arbitration should be auto-accepted to COMPLETED, got %v", got.State) + } + + decided := store.eventsOfKind(event.KindArbitrationDecided) + if len(decided) != 1 { + t.Fatalf("expected exactly 1 arbitration_decided event after auto-accept, got %d", len(decided)) + } + + stories, _ := store.ListStories(storage.StoryFilter{}) + for _, s := range stories { + if s.ID == st.ID && s.Status != "REVIEW_READY" { + t.Errorf("story status: want REVIEW_READY after arbitration auto-accepts, got %q", s.Status) + } + } +} + +// TestStoryOrchestrator_AutoAccept_DoesNotTouchUnrelatedReadyTask proves the +// auto-accept behavior is scoped to a story's own pipeline tasks (root task +// + its role-matched evaluator/arbitration dependents) and does not sweep up +// an unrelated READY task that merely happens to exist in the store. +func TestStoryOrchestrator_AutoAccept_DoesNotTouchUnrelatedReadyTask(t *testing.T) { + store := newFakeStoryStore() + root := builderTask("builder-1", task.StateCompleted) + store.tasks[root.ID] = root + st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS") + store.stories[st.ID] = st + + unrelated := &task.Task{ID: "unrelated-1", Name: "unrelated", Agent: task.AgentConfig{Type: "claude"}, State: task.StateReady} + store.tasks[unrelated.ID] = unrelated + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) + + got, err := store.GetTask(unrelated.ID) + if err != nil { + t.Fatal(err) + } + if got.State != task.StateReady { + t.Errorf("unrelated task must not be auto-accepted: want READY, got %v", got.State) + } +} + +// TestStoryOrchestrator_SkipsStoriesWithNoRootTask proves a story with no +// root_task_id set is left completely untouched. +func TestStoryOrchestrator_SkipsStoriesWithNoRootTask(t *testing.T) { + store := newFakeStoryStore() + st := &story.Story{ID: "story-1", Name: "no root yet", Status: "DISCOVERY"} + store.stories[st.ID] = st + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) // must not panic or error despite no root task existing + + if pool.submitCount() != 0 { + t.Fatalf("expected no submissions, got %d", pool.submitCount()) + } +} + +// TestStoryOrchestrator_SkipsTerminalStories proves DONE/CANCELLED stories +// are never revisited, even if (hypothetically) their root task is +// COMPLETED and evaluators don't yet exist. +func TestStoryOrchestrator_SkipsTerminalStories(t *testing.T) { + for _, status := range []string{"DONE", "CANCELLED"} { + t.Run(status, func(t *testing.T) { + store := newFakeStoryStore() + root := builderTask("builder-1", task.StateCompleted) + store.tasks[root.ID] = root + st := newStoryWithRoot("story-1", root.ID, status) + store.stories[st.ID] = st + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + orch.Tick(context.Background()) + + deps, _ := store.ListDependents(root.ID) + if len(deps) != 0 { + t.Fatalf("status %s: expected no evaluator tasks spawned, got %d", status, len(deps)) + } + }) + } +} + +// TestStoryOrchestrator_EndToEnd drives a story through the full chain — +// builder complete -> evaluators -> arbitration -> REVIEW_READY — using only +// the fake store/pool, proving the whole ceremony holds together end to end +// at the orchestrator level (verification item 5, the higher-level test). +// +// Every task in the chain is driven to READY (never directly to COMPLETED), +// mirroring exactly what executor.Pool.handleRunResult does for a real +// top-level task whose execution succeeds — proving the orchestrator's own +// auto-accept (not an external POST /api/tasks/{id}/accept call, which this +// test never makes) is what carries each task the rest of the way to +// COMPLETED and advances the story. The only accept call anywhere in this +// flow is the story-level one, which isn't part of this test — it's covered +// separately in internal/api's story accept-gate tests. +func TestStoryOrchestrator_EndToEnd(t *testing.T) { + store := newFakeStoryStore() + root := builderTask("builder-1", task.StatePending) + store.tasks[root.ID] = root + st := newStoryWithRoot("story-1", root.ID, "IN_PROGRESS") + store.stories[st.ID] = st + + pool := &fakePool{} + orch := &StoryOrchestrator{Store: store, Pool: pool} + + // Before the builder completes, nothing happens. + orch.Tick(context.Background()) + if deps, _ := store.ListDependents(root.ID); len(deps) != 0 { + t.Fatalf("expected no evaluators before builder completes, got %d", len(deps)) + } + + // Builder's execution succeeds (RUNNING -> READY, exactly like + // handleRunResult) — no POST /api/tasks/{id}/accept call here. + store.setTaskState(root.ID, task.StateReady) + orch.Tick(context.Background()) + + rootAfter, err := store.GetTask(root.ID) + if err != nil { + t.Fatal(err) + } + if rootAfter.State != task.StateCompleted { + t.Fatalf("builder should be auto-accepted to COMPLETED without an accept call, got %v", rootAfter.State) + } + + deps, _ := store.ListDependents(root.ID) + if len(deps) != 4 { + t.Fatalf("expected 4 evaluators, got %d", len(deps)) + } + statusOf := func() string { + stories, _ := store.ListStories(storage.StoryFilter{}) + for _, s := range stories { + if s.ID == st.ID { + return s.Status + } + } + return "" + } + if statusOf() != "VALIDATING" { + t.Fatalf("expected VALIDATING after evaluators spawn, got %q", statusOf()) + } + + // Evaluators' executions succeed one by one (READY, not COMPLETED); + // arbitration must not spawn early. + for i, d := range deps { + store.setTaskState(d.ID, task.StateReady) + store.setTaskSummary(d.ID, fmt.Sprintf("verdict %d", i)) + orch.Tick(context.Background()) + arbs := store.dependentsWithRole(deps[0].ID, "planner") + if i < len(deps)-1 && len(arbs) != 0 { + t.Fatalf("arbitration spawned too early, after %d/%d evaluators complete", i+1, len(deps)) + } + } + + for _, d := range deps { + got, err := store.GetTask(d.ID) + if err != nil { + t.Fatal(err) + } + if got.State != task.StateCompleted { + t.Errorf("evaluator %s should be auto-accepted to COMPLETED, got %v", d.ID, got.State) + } + } + + arbs := store.dependentsWithRole(deps[0].ID, "planner") + if len(arbs) != 1 { + t.Fatalf("expected exactly 1 arbitration task, got %d", len(arbs)) + } + arb := arbs[0] + + verdicts := store.eventsOfKind(event.KindEvalVerdict) + if len(verdicts) != 4 { + t.Fatalf("expected 4 eval_verdict events, got %d", len(verdicts)) + } + + // Arbitration's execution succeeds (READY, not COMPLETED). + store.setTaskState(arb.ID, task.StateReady) + store.setTaskSummary(arb.ID, "approved") + orch.Tick(context.Background()) + + arbAfter, err := store.GetTask(arb.ID) + if err != nil { + t.Fatal(err) + } + if arbAfter.State != task.StateCompleted { + t.Fatalf("arbitration should be auto-accepted to COMPLETED, got %v", arbAfter.State) + } + + if statusOf() != "REVIEW_READY" { + t.Fatalf("expected REVIEW_READY after arbitration completes, got %q", statusOf()) + } + decided := store.eventsOfKind(event.KindArbitrationDecided) + if len(decided) != 1 { + t.Fatalf("expected exactly 1 arbitration_decided event, got %d", len(decided)) + } +} |
