diff options
Diffstat (limited to 'internal/scheduler/story_orchestrator.go')
| -rw-r--r-- | internal/scheduler/story_orchestrator.go | 146 |
1 files changed, 138 insertions, 8 deletions
diff --git a/internal/scheduler/story_orchestrator.go b/internal/scheduler/story_orchestrator.go index b70b487..c3d5215 100644 --- a/internal/scheduler/story_orchestrator.go +++ b/internal/scheduler/story_orchestrator.go @@ -216,6 +216,11 @@ func (o *StoryOrchestrator) logf(msg string, args ...any) { // returns as soon as it finds a stage that isn't ready to progress yet — the // next tick picks up where this one left off). func (o *StoryOrchestrator) processStory(ctx context.Context, st *story.Story) { + if st.Status == "NEEDS_FIX" { + o.ensureFixAttempt(ctx, st) + return + } + root, err := o.Store.GetTask(st.RootTaskID) if err != nil { o.logf("story orchestrator: get root task", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err) @@ -401,7 +406,7 @@ func (o *StoryOrchestrator) finalizeArbitration(st *story.Story, arbitration *ta return } - approved, hasVerdict := o.arbitrationVerdict(st, arbitration) + approved, _, hasVerdict := o.arbitrationVerdict(st, arbitration) payload, _ := json.Marshal(struct { TaskID string `json:"task_id"` @@ -437,28 +442,32 @@ func (o *StoryOrchestrator) finalizeArbitration(st *story.Story, arbitration *ta // arbitrationVerdict reads the arbitration task's own event stream for a // KindVerdictReported event (AgentChannel.ReportVerdict) and returns its -// approved value. hasVerdict is false if no such event was ever recorded — -// callers must treat that as "no structured verdict was reported", not as -// an implicit rejection. -func (o *StoryOrchestrator) arbitrationVerdict(st *story.Story, arbitration *task.Task) (approved bool, hasVerdict bool) { +// approved value and reasoning text. hasVerdict is false if no such event was +// ever recorded — callers must treat that as "no structured verdict was +// reported", not as an implicit rejection. reasoning feeds ensureFixAttempt's +// automatic fix-attempt instructions; finalizeArbitration itself only needs +// approved/hasVerdict and discards reasoning. +func (o *StoryOrchestrator) arbitrationVerdict(st *story.Story, arbitration *task.Task) (approved bool, reasoning string, hasVerdict bool) { events, err := o.Store.ListEvents(arbitration.ID, 0) if err != nil { o.logf("story orchestrator: list arbitration task events", "storyID", st.ID, "taskID", arbitration.ID, "error", err) - return false, false + return false, "", false } for _, e := range events { if e.Kind != event.KindVerdictReported { continue } var payload struct { - Approved bool `json:"approved"` + Approved bool `json:"approved"` + Reasoning string `json:"reasoning"` } if json.Unmarshal(e.Payload, &payload) == nil { approved = payload.Approved + reasoning = payload.Reasoning hasVerdict = true } } - return approved, hasVerdict + return approved, reasoning, hasVerdict } // optionalBool returns a pointer to v if present is true, nil otherwise — @@ -472,6 +481,127 @@ func optionalBool(v bool, present bool) *bool { return &v } +// maxFixAttempts caps automatic fix-attempt spawning (see ensureFixAttempt) +// to prevent an unbounded loop if arbitration keeps rejecting a story's +// work. This is a simple, hardcoded safety net, not real cost/escalation +// tiering — the design spec's Non-Goals explicitly deferred "no depth or +// cost bound in this design... left as a distinct future piece of work". +// Once hit, the story stays at NEEDS_FIX exactly like it does today, for a +// human to intervene manually via PUT /api/stories/{id}. +const maxFixAttempts = 3 + +// ensureFixAttempt handles a story sitting at NEEDS_FIX: it spawns one new +// top-level builder-role task depending on the rejected root (purely for +// structural discoverability/audit trail — the rejected root is already +// COMPLETED, so this dependency is immediately satisfied), whose +// instructions carry the original story spec/acceptance criteria plus the +// arbitration's rejection reasoning, then re-points st.RootTaskID at the new +// task and resets st.Status to IN_PROGRESS. The very next tick re-enters +// processStory's normal Builder->Evaluators->Arbitration flow against the +// new root, completely unchanged — no new pipeline, the existing one +// re-entered. +// +// Idempotency is structural, mirroring every other stage in this file: it +// looks for an existing builder-role dependent of the rejected root before +// spawning a new one, so calling this repeatedly (or after a restart between +// "task spawned" and "story updated") never spawns duplicates. The +// maxFixAttempts cap is only checked in the "spawn a new one" branch — a +// dependent that was already committed to being spawned still gets +// re-pointed to, regardless of the cap, since refusing to do so would leave +// a task dangling with nothing tracking it. +func (o *StoryOrchestrator) ensureFixAttempt(ctx context.Context, st *story.Story) { + oldRoot, err := o.Store.GetTask(st.RootTaskID) + if err != nil { + o.logf("story orchestrator: fix attempt: get rejected root task", "storyID", st.ID, "rootTaskID", st.RootTaskID, "error", err) + return + } + + dependents, err := o.Store.ListDependents(oldRoot.ID) + if err != nil { + o.logf("story orchestrator: fix attempt: list root dependents", "storyID", st.ID, "error", err) + return + } + var fix *task.Task + for _, d := range dependents { + if d.Agent.Role == "builder" { + fix = d + break + } + } + + if fix == nil { + if depth := o.fixAttemptDepth(oldRoot); depth >= maxFixAttempts { + o.logf("story orchestrator: fix attempt: max fix attempts reached, leaving story at NEEDS_FIX for a human", "storyID", st.ID, "depth", depth) + return + } + reasoning := o.rejectionReasoning(st, oldRoot) + instructions := fmt.Sprintf( + "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) + if err != nil { + o.logf("story orchestrator: fix attempt: spawn", "storyID", st.ID, "error", err) + return + } + fix = nt + } + + st.RootTaskID = fix.ID + st.Status = "IN_PROGRESS" + if err := o.Store.UpdateStory(st); err != nil { + o.logf("story orchestrator: fix attempt: repoint story root", "storyID", st.ID, "error", err) + } +} + +// rejectionReasoning finds the rejected root's evaluator/arbitration chain +// (read-only, via findEvaluators/findArbitration — the same lookups +// processRetro uses) and returns the arbitration task's structured +// KindVerdictReported reasoning, falling back to its plain-text Summary if no +// structured verdict was reported, or a placeholder if neither is available. +func (o *StoryOrchestrator) rejectionReasoning(st *story.Story, oldRoot *task.Task) string { + evaluators, ok := o.findEvaluators(oldRoot.ID) + if !ok { + return "(no arbitration reasoning found)" + } + arbitration, ok := o.findArbitration(evaluators) + if !ok { + return "(no arbitration reasoning found)" + } + _, reasoning, hasVerdict := o.arbitrationVerdict(st, arbitration) + if hasVerdict && reasoning != "" { + return reasoning + } + if arbitration.Summary != "" { + return arbitration.Summary + } + return "(no arbitration reasoning found)" +} + +// fixAttemptDepth walks backward through builder-role tasks connected by a +// single DependsOn edge — the exact shape ensureFixAttempt creates — counting +// how many fix attempts precede root. Stops as soon as it finds a task with +// anything other than exactly one DependsOn entry (the original, non-fix- +// attempt root, which was created by whatever process first submitted the +// story). Bounded by maxFixAttempts+1 iterations since callers only care +// whether the cap is hit, not the exact depth beyond that. +func (o *StoryOrchestrator) fixAttemptDepth(root *task.Task) int { + depth := 0 + current := root + for depth <= maxFixAttempts { + if len(current.DependsOn) != 1 { + break + } + prev, err := o.Store.GetTask(current.DependsOn[0]) + if err != nil { + break + } + depth++ + current = prev + } + return depth +} + // processRetro is the Phase 8 retro stage: it runs for every story the Tick // loop has already observed at status DONE (see Tick above). Unlike // processStory's stages, it never spawns or advances a Builder/Evaluator/ |