diff options
Diffstat (limited to 'internal/scheduler/story_orchestrator.go')
| -rw-r--r-- | internal/scheduler/story_orchestrator.go | 72 |
1 files changed, 47 insertions, 25 deletions
diff --git a/internal/scheduler/story_orchestrator.go b/internal/scheduler/story_orchestrator.go index 8061b35..19383c6 100644 --- a/internal/scheduler/story_orchestrator.go +++ b/internal/scheduler/story_orchestrator.go @@ -226,9 +226,16 @@ func (o *StoryOrchestrator) processStory(ctx context.Context, st *story.Story) { o.logf("story orchestrator: resolve current root attempt", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err) return } - root = o.autoAccept(st, root) - if root.State != task.StateCompleted { - return // Builder hasn't reached COMPLETED yet (still running, or not yet READY to auto-accept) + // root is NOT auto-accepted here. Unlike evaluator/arbitration tasks, + // root is builder-role: its own completion must mean "verified by + // arbitration", not merely "the agent finished" -- so it stays READY + // through the whole Evaluators/Arbitration cycle below, and only + // finalizeArbitration's approval branch ever promotes it to COMPLETED. + // READY already satisfies a dependent's DependsOn (see + // executor.depDoneStates), so evaluators/arbitration can depend on a + // still-READY root without issue. + if root.State != task.StateReady { + return // not yet finished (still running/blocked); or already COMPLETED, meaning a prior tick's finalizeArbitration already approved it -- nothing further to do } // Stage 1: Builder -> Evaluators (+ story -> VALIDATING). @@ -259,9 +266,13 @@ func (o *StoryOrchestrator) processStory(ctx context.Context, st *story.Story) { } arbitration = o.autoAccept(st, arbitration) - // Stage 3: Arbitration -> REVIEW_READY. + // Stage 3: Arbitration -> approve (root: READY -> COMPLETED, story -> + // REVIEW_READY) or reject (story -> NEEDS_FIX; a fix attempt is spawned + // next tick by ensureFixAttempt, depending on the still-READY root -- + // task.CurrentAttempt resolves future lookups of this position forward + // to it). if arbitration.State == task.StateCompleted { - o.finalizeArbitration(st, arbitration) + o.finalizeArbitration(st, root, arbitration) } } @@ -388,20 +399,32 @@ func (o *StoryOrchestrator) ensureArbitration(ctx context.Context, st *story.Sto } // finalizeArbitration handles the Arbitration task reaching COMPLETED: it -// emits KindArbitrationDecided and moves the story to REVIEW_READY or -// NEEDS_FIX, depending on whether the arbitration task recorded a structured -// KindVerdictReported event (AgentChannel.ReportVerdict). An arbitration -// task that never calls report_verdict defaults to REVIEW_READY, preserving -// the prior behavior for agents that don't report a structured verdict. +// 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 +// 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 +// docs/superpowers/specs/2026-07-09-recursive-arbitrated-review-design.md). +// On rejection, root 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. // // Gated on st.Status == "VALIDATING" so repeated ticks (or a story a human -// already advanced past REVIEW_READY) don't re-emit the event 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, arbitration *task.Task) { +// 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" { return } @@ -422,17 +445,16 @@ func (o *StoryOrchestrator) finalizeArbitration(st *story.Story, arbitration *ta o.logf("story orchestrator: emit arbitration_decided", "storyID", st.ID, "error", err) } - // Documented simplification (Phase 7b) closed: a structured - // KindVerdictReported event (AgentChannel.ReportVerdict) on the - // arbitration task, if present, now decides REVIEW_READY vs NEEDS_FIX. - // An arbitration task that never calls report_verdict (hasVerdict == - // false) preserves the prior unconditional REVIEW_READY behavior — a - // human or chatbot can still manually set NEEDS_FIX via the existing - // PUT /api/stories/{id} for a story arbitrated by an agent that doesn't - // report a structured verdict. 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) + return + } st.Status = "REVIEW_READY" } if err := o.Store.UpdateStory(st); err != nil { |