summaryrefslogtreecommitdiff
path: root/internal/scheduler
diff options
context:
space:
mode:
Diffstat (limited to 'internal/scheduler')
-rw-r--r--internal/scheduler/story_orchestrator.go198
-rw-r--r--internal/scheduler/story_orchestrator_test.go277
2 files changed, 420 insertions, 55 deletions
diff --git a/internal/scheduler/story_orchestrator.go b/internal/scheduler/story_orchestrator.go
index 19383c6..d190ba7 100644
--- a/internal/scheduler/story_orchestrator.go
+++ b/internal/scheduler/story_orchestrator.go
@@ -260,7 +260,7 @@ func (o *StoryOrchestrator) processStory(ctx context.Context, st *story.Story) {
}
// Stage 2: Evaluators -> Arbitration.
- arbitration, ok := o.ensureArbitration(ctx, st, evaluators)
+ arbitration, ok := o.ensureArbitration(ctx, st, root, evaluators)
if !ok {
return
}
@@ -272,7 +272,7 @@ func (o *StoryOrchestrator) processStory(ctx context.Context, st *story.Story) {
// task.CurrentAttempt resolves future lookups of this position forward
// to it).
if arbitration.State == task.StateCompleted {
- o.finalizeArbitration(st, root, arbitration)
+ o.finalizeArbitration(ctx, st, root, arbitration)
}
}
@@ -332,13 +332,18 @@ func (o *StoryOrchestrator) ensureEvaluators(ctx context.Context, st *story.Stor
}
}
+ criteria := root.AcceptanceCriteria
+ if len(criteria) == 0 {
+ criteria = st.AcceptanceCriteria
+ }
+
spawnedAny := false
for _, r := range evaluatorRoles {
if _, ok := found[r]; ok {
continue
}
- nt, err := o.spawnRoleTask(ctx, fmt.Sprintf("%s: %s", r, st.Name), r, []string{root.ID}, root,
- fmt.Sprintf("Evaluate the changes made by task %s against the %q dimension for story %q.\n\nStory spec:\n%s", root.ID, r, st.Name, st.Spec))
+ nt, err := o.spawnRoleTask(ctx, fmt.Sprintf("%s: %s", r, st.Name), r, []string{root.ID}, "", root,
+ fmt.Sprintf("Evaluate the changes made by task %s against the %q dimension for story %q.\n\nStory spec:\n%s\n\nAcceptance criteria:\n%s", root.ID, r, st.Name, st.Spec, formatAcceptanceCriteria(criteria)))
if err != nil {
o.logf("story orchestrator: spawn evaluator", "storyID", st.ID, "role", r, "error", err)
continue
@@ -369,7 +374,9 @@ func (o *StoryOrchestrator) ensureEvaluators(ctx context.Context, st *story.Stor
// structural: it looks for an existing "planner"-role dependent of the first
// evaluator task that depends on every evaluator ID, rather than relying on
// story.Status (which a human can freely rewrite via PUT /api/stories/{id}).
-func (o *StoryOrchestrator) ensureArbitration(ctx context.Context, st *story.Story, evaluators []*task.Task) (*task.Task, bool) {
+// node is the task being evaluated (root or nested) -- used only to read its
+// own AcceptanceCriteria, falling back to the story's if node has none set.
+func (o *StoryOrchestrator) ensureArbitration(ctx context.Context, st *story.Story, node *task.Task, evaluators []*task.Task) (*task.Task, bool) {
ids := make([]string, len(evaluators))
for i, ev := range evaluators {
ids[i] = ev.ID
@@ -386,11 +393,15 @@ func (o *StoryOrchestrator) ensureArbitration(ctx context.Context, st *story.Sto
}
}
+ criteria := node.AcceptanceCriteria
+ if len(criteria) == 0 {
+ criteria = st.AcceptanceCriteria
+ }
instructions := fmt.Sprintf(
- "Arbitrate the 4 evaluator verdicts for story %q (task %s). Read each evaluator task's summary/events "+
- "and decide whether the story is ready to ship. Acceptance criteria:\n%s",
- st.Name, st.ID, formatAcceptanceCriteria(st.AcceptanceCriteria))
- nt, err := o.spawnRoleTask(ctx, "Arbitration: "+st.Name, arbitrationRole, ids, evaluators[0], instructions)
+ "Arbitrate the 4 evaluator verdicts for task %q (%s) within story %q. Read each evaluator task's summary/events "+
+ "and decide whether this task is ready to ship. Acceptance criteria:\n%s",
+ node.Name, node.ID, st.Name, formatAcceptanceCriteria(criteria))
+ nt, err := o.spawnRoleTask(ctx, "Arbitration: "+st.Name, arbitrationRole, ids, "", evaluators[0], instructions)
if err != nil {
o.logf("story orchestrator: spawn arbitration", "storyID", st.ID, "error", err)
return nil, false
@@ -398,38 +409,63 @@ func (o *StoryOrchestrator) ensureArbitration(ctx context.Context, st *story.Sto
return nt, true
}
-// finalizeArbitration handles the Arbitration task reaching COMPLETED: it
-// emits KindArbitrationDecided, then either approves or rejects root based on
-// whether the arbitration task recorded a structured KindVerdictReported
-// event (AgentChannel.ReportVerdict). An arbitration task that never calls
+// finalizeArbitration handles an Arbitration task reaching COMPLETED for any
+// builder-role node in a story's tree -- root or nested -- and either
+// approves or rejects that node based on whether the arbitration task
+// recorded a structured KindVerdictReported event
+// (AgentChannel.ReportVerdict). An arbitration task that never calls
// report_verdict defaults to approval, preserving the prior behavior for
// agents that don't report a structured verdict.
//
-// On approval, root is promoted READY -> COMPLETED *here* — not eagerly the
-// moment it first reached READY — so COMPLETED means "verified by
-// arbitration", uniformly, the same rule this design applies at every depth
-// of a builder-role task tree (see
+// Idempotency is structural and keyed to this specific arbitration task, not
+// story.Status (a story-level field only the root position has any claim
+// to): it checks the story's own event stream for a KindArbitrationDecided
+// event already referencing arbitration.ID, and does nothing further if
+// found. This is what lets this same function run for every builder node in
+// a story's tree without a node-specific analog of story.Status to gate on.
+//
+// On approval, node is promoted READY -> COMPLETED *here* -- not eagerly the
+// moment it first reached READY -- so COMPLETED means "verified by
+// arbitration", uniformly, at every depth (see
// docs/superpowers/specs/2026-07-09-recursive-arbitrated-review-design.md).
-// On rejection, root is left untouched at READY: it has been superseded, and
+// On rejection, node is left untouched at READY: it has been superseded, and
// task.CurrentAttempt will resolve future lookups of this position forward
-// to whatever fix-attempt task ensureFixAttempt spawns once the story lands
-// at NEEDS_FIX below — READY already satisfies a dependent's DependsOn (see
-// executor.depDoneStates), so the fix attempt is immediately dispatchable
-// without root ever becoming COMPLETED.
+// to whatever fix-attempt gets spawned.
//
-// Gated on st.Status == "VALIDATING" so repeated ticks (or a story a human
-// already advanced past REVIEW_READY) don't re-emit the event, re-promote
-// root, or re-write the status — this is the one place in the orchestrator
-// where the story's own status field, not a structural dependents check, is
-// the idempotency guard, because by this stage there's nothing further to
-// check structurally: the Arbitration task is the last task in the chain, so
-// "does a subsequent task exist" isn't an available signal.
-func (o *StoryOrchestrator) finalizeArbitration(st *story.Story, root, arbitration *task.Task) {
- if st.Status != "VALIDATING" {
+// st.Status is only ever touched here when node IS the story's root
+// (node.ParentTaskID == "", true only for the position anchored at
+// st.RootTaskID within this story's own tree): REVIEW_READY on approval,
+// NEEDS_FIX on rejection -- exactly as before this function was
+// generalized. A rejected ROOT's fix-attempt is still spawned by
+// ensureFixAttempt on the next tick, polling st.Status == "NEEDS_FIX" --
+// unchanged, since a Story row is a natural, already-existing anchor for
+// that trigger. A rejected NESTED node has no story-level field to poll, so
+// its fix-attempt is spawned directly, right here (spawnNestedFixAttempt,
+// below), respecting the same maxFixAttempts cap via fixAttemptDepth. Both
+// paths produce the identical semantic outcome (a same-role, same-position
+// fix-attempt, capped) through different wiring suited to where a
+// story-level anchor exists (root) or doesn't (nested) -- not a special case
+// in the review mechanism itself, which stays 100% identical at every depth.
+func (o *StoryOrchestrator) finalizeArbitration(ctx context.Context, st *story.Story, node, arbitration *task.Task) {
+ events, err := o.Store.ListEvents(st.ID, 0)
+ if err != nil {
+ o.logf("story orchestrator: finalize arbitration: list story events", "storyID", st.ID, "error", err)
return
}
+ for _, e := range events {
+ if e.Kind != event.KindArbitrationDecided {
+ continue
+ }
+ var payload struct {
+ TaskID string `json:"task_id"`
+ }
+ if json.Unmarshal(e.Payload, &payload) == nil && payload.TaskID == arbitration.ID {
+ return // already finalized
+ }
+ }
- approved, _, hasVerdict := o.arbitrationVerdict(st, arbitration)
+ approved, reasoning, hasVerdict := o.arbitrationVerdict(st, arbitration)
+ isRoot := node.ParentTaskID == ""
payload, _ := json.Marshal(struct {
TaskID string `json:"task_id"`
@@ -446,19 +482,68 @@ func (o *StoryOrchestrator) finalizeArbitration(st *story.Story, root, arbitrati
}
if hasVerdict && !approved {
- st.Status = "NEEDS_FIX"
- } else {
- // Approved (or no structured verdict reported -- preserves the
- // prior unconditional-approve default). root only becomes COMPLETED
- // here, after arbitration.
- if err := o.Store.UpdateTaskState(root.ID, task.StateCompleted); err != nil {
- o.logf("story orchestrator: finalize arbitration: promote root to completed", "storyID", st.ID, "taskID", root.ID, "error", err)
+ if isRoot {
+ st.Status = "NEEDS_FIX"
+ if err := o.Store.UpdateStory(st); err != nil {
+ o.logf("story orchestrator: update story status after arbitration", "storyID", st.ID, "status", st.Status, "error", err)
+ }
return
}
+ o.spawnNestedFixAttempt(ctx, st, node, reasoning)
+ return
+ }
+
+ // Approved (or no structured verdict reported -- preserves the prior
+ // unconditional-approve default). node only becomes COMPLETED here,
+ // after arbitration.
+ if err := o.Store.UpdateTaskState(node.ID, task.StateCompleted); err != nil {
+ o.logf("story orchestrator: finalize arbitration: promote node to completed", "storyID", st.ID, "taskID", node.ID, "error", err)
+ return
+ }
+ if isRoot {
st.Status = "REVIEW_READY"
+ if err := o.Store.UpdateStory(st); err != nil {
+ o.logf("story orchestrator: update story status after arbitration", "storyID", st.ID, "status", st.Status, "error", err)
+ }
}
- if err := o.Store.UpdateStory(st); err != nil {
- o.logf("story orchestrator: update story status after arbitration", "storyID", st.ID, "status", st.Status, "error", err)
+}
+
+// spawnNestedFixAttempt handles a rejected NESTED builder node
+// (node.ParentTaskID != "") -- the counterpart to ensureFixAttempt, which
+// handles the root position via st.Status == "NEEDS_FIX" polling. A nested
+// position has no story-level field to poll, so its fix-attempt is spawned
+// directly here, the moment rejection is discovered. Idempotency and the
+// maxFixAttempts cap mirror ensureFixAttempt exactly: look for an existing
+// role-matched dependent before spawning, and refuse to spawn a new one past
+// the cap (leaving node's position permanently unresolved, node's own parent
+// stuck BLOCKED forever -- the same "leave it for a human" resolution the
+// root's own cap already accepts, per this design's Non-Goals).
+func (o *StoryOrchestrator) spawnNestedFixAttempt(ctx context.Context, st *story.Story, node *task.Task, reasoning string) {
+ dependents, err := o.Store.ListDependents(node.ID)
+ if err != nil {
+ o.logf("story orchestrator: nested fix attempt: list node dependents", "storyID", st.ID, "nodeID", node.ID, "error", err)
+ return
+ }
+ for _, d := range dependents {
+ if d.Agent.Role == node.Agent.Role {
+ return // fix attempt already spawned
+ }
+ }
+
+ if depth := o.fixAttemptDepth(node); depth >= maxFixAttempts {
+ o.logf("story orchestrator: nested fix attempt: max fix attempts reached, leaving node rejected for a human", "storyID", st.ID, "nodeID", node.ID, "depth", depth)
+ return
+ }
+
+ if reasoning == "" {
+ reasoning = "(no arbitration reasoning found)"
+ }
+ instructions := fmt.Sprintf(
+ "A previous attempt at task %q (%s) within story %q was rejected by arbitration. Fix the issues and resubmit.\n\n"+
+ "Acceptance criteria:\n%s\n\nArbitration's rejection reasoning:\n%s",
+ node.Name, node.ID, st.Name, formatAcceptanceCriteria(node.AcceptanceCriteria), reasoning)
+ if _, err := o.spawnRoleTask(ctx, "Fix attempt: "+node.Name, node.Agent.Role, []string{node.ID}, node.ParentTaskID, node, instructions); err != nil {
+ o.logf("story orchestrator: nested fix attempt: spawn", "storyID", st.ID, "nodeID", node.ID, "error", err)
}
}
@@ -564,7 +649,7 @@ func (o *StoryOrchestrator) ensureFixAttempt(ctx context.Context, st *story.Stor
"A previous attempt at story %q (task %s) was rejected by arbitration. Fix the issues and resubmit.\n\n"+
"Story spec:\n%s\n\nAcceptance criteria:\n%s\n\nArbitration's rejection reasoning:\n%s",
st.Name, oldRoot.ID, st.Spec, formatAcceptanceCriteria(st.AcceptanceCriteria), reasoning)
- nt, err := o.spawnRoleTask(ctx, "Fix attempt: "+st.Name, "builder", []string{oldRoot.ID}, oldRoot, instructions)
+ nt, err := o.spawnRoleTask(ctx, "Fix attempt: "+st.Name, "builder", []string{oldRoot.ID}, "", oldRoot, instructions)
if err != nil {
o.logf("story orchestrator: fix attempt: spawn", "storyID", st.ID, "error", err)
return
@@ -679,7 +764,7 @@ func (o *StoryOrchestrator) processRetro(ctx context.Context, st *story.Story) {
}
if retro == nil {
instructions := o.buildRetroInstructions(st, root, arbitration)
- nt, err := o.spawnRoleTask(ctx, "Retro: "+st.Name, retroRole, []string{arbitration.ID}, root, instructions)
+ nt, err := o.spawnRoleTask(ctx, "Retro: "+st.Name, retroRole, []string{arbitration.ID}, "", root, instructions)
if err != nil {
o.logf("story orchestrator: retro: spawn", "storyID", st.ID, "error", err)
return
@@ -957,19 +1042,26 @@ func (o *StoryOrchestrator) maybeEmitVerdict(st *story.Story, ev *task.Task) {
}
}
-// spawnRoleTask creates a new role-typed, top-level task (no ParentTaskID —
-// these are DAG siblings via DependsOn, not delegated subtasks; see
-// internal/executor.Pool.cascadeFail's doc comment for why that distinction
-// matters) and submits it to the pool. Agent.Type/Model are left empty so
-// Phase 5's role-based dispatch resolves them from the role's escalation
-// ladder on first dispatch (internal/executor.Pool.execute()); if no active
-// role_configs row exists for the role, that same code path logs a warning
-// and dispatches without role resolution — an accepted degraded mode we
-// don't special-case here either.
-func (o *StoryOrchestrator) spawnRoleTask(ctx context.Context, name, roleName string, dependsOn []string, template *task.Task, instructions string) (*task.Task, error) {
+// spawnRoleTask creates a new role-typed task and submits it to the pool.
+// parentTaskID is "" for the DAG-sibling tasks every caller except
+// spawnNestedFixAttempt creates (Evaluators, Arbitration, Retro, and the
+// root's own fix-attempts spawned by ensureFixAttempt are always top-level —
+// see internal/executor.Pool.cascadeFail's doc comment for why that
+// distinction matters); spawnNestedFixAttempt is the one caller that passes
+// a non-empty parentTaskID, so a rejected nested node's fix-attempt lands at
+// the same tree position as the node it supersedes (required for
+// task.CurrentAttempt's ParentTaskID-equality matching to resolve it).
+// Agent.Type/Model are left empty so Phase 5's role-based dispatch resolves
+// them from the role's escalation ladder on first dispatch
+// (internal/executor.Pool.execute()); if no active role_configs row exists
+// for the role, that same code path logs a warning and dispatches without
+// role resolution — an accepted degraded mode we don't special-case here
+// either.
+func (o *StoryOrchestrator) spawnRoleTask(ctx context.Context, name, roleName string, dependsOn []string, parentTaskID string, template *task.Task, instructions string) (*task.Task, error) {
nt := &task.Task{
ID: uuid.NewString(),
Name: name,
+ ParentTaskID: parentTaskID,
Project: template.Project,
RepositoryURL: template.RepositoryURL,
Agent: task.AgentConfig{
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)
}
generated by cgit v1.2.3 (git 2.39.1) at 2026-07-16 08:13:24 +0000