From c82def744f23d09562f0bd980ee8333eeb1b90e4 Mon Sep 17 00:00:00 2001 From: Claudomator Agent Date: Wed, 8 Jul 2026 09:43:25 +0000 Subject: feat(scheduler): finalizeArbitration consults a structured verdict instead of always routing to REVIEW_READY --- internal/scheduler/story_orchestrator.go | 84 ++++++++++++++++++++++++-------- 1 file changed, 65 insertions(+), 19 deletions(-) (limited to 'internal/scheduler/story_orchestrator.go') diff --git a/internal/scheduler/story_orchestrator.go b/internal/scheduler/story_orchestrator.go index b8e1b4c..b70b487 100644 --- a/internal/scheduler/story_orchestrator.go +++ b/internal/scheduler/story_orchestrator.go @@ -70,10 +70,9 @@ var evaluatorRoles = []string{ } // arbitrationRole is the role assigned to the single task spawned once all -// Evaluators for a story complete. It depends on all 4 Evaluator tasks and, -// per this phase's documented simplification (see finalizeArbitration), -// always routes the story to REVIEW_READY on completion rather than parsing -// its summary for an approve/reject verdict. +// Evaluators for a story complete. It depends on all 4 Evaluator tasks and +// routes the story to REVIEW_READY or NEEDS_FIX based on a structured +// KindVerdictReported event (see finalizeArbitration/arbitrationVerdict). const arbitrationRole = "planner" // retroRole is the role assigned to the single task spawned once a story @@ -384,16 +383,11 @@ 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. -// -// Documented simplification (Phase 7b, see CLAUDE.md Design Debt): this does -// NOT parse the arbitration task's summary for an approve/reject verdict — -// it always routes to REVIEW_READY. A human or chatbot who reads the -// arbitration summary and disagrees can manually set the story to NEEDS_FIX -// via the existing PUT /api/stories/{id}. A later phase could close this gap -// by giving the arbitration task a dedicated verdict-reporting tool (e.g. a -// new AgentChannel method) whose structured output this orchestrator could -// trust instead of free-text parsing. +// 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. // // 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 @@ -407,10 +401,13 @@ func (o *StoryOrchestrator) finalizeArbitration(st *story.Story, arbitration *ta return } + approved, hasVerdict := o.arbitrationVerdict(st, arbitration) + payload, _ := json.Marshal(struct { - TaskID string `json:"task_id"` - Summary string `json:"summary"` - }{TaskID: arbitration.ID, Summary: arbitration.Summary}) + TaskID string `json:"task_id"` + Summary string `json:"summary"` + Approved *bool `json:"approved,omitempty"` + }{TaskID: arbitration.ID, Summary: arbitration.Summary, Approved: optionalBool(approved, hasVerdict)}) if err := o.Store.CreateEvent(&event.Event{ TaskID: st.ID, Kind: event.KindArbitrationDecided, @@ -420,10 +417,59 @@ func (o *StoryOrchestrator) finalizeArbitration(st *story.Story, arbitration *ta o.logf("story orchestrator: emit arbitration_decided", "storyID", st.ID, "error", err) } - st.Status = "REVIEW_READY" + // 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 { + st.Status = "REVIEW_READY" + } if err := o.Store.UpdateStory(st); err != nil { - o.logf("story orchestrator: update story to REVIEW_READY", "storyID", st.ID, "error", err) + o.logf("story orchestrator: update story status after arbitration", "storyID", st.ID, "status", st.Status, "error", err) + } +} + +// 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) { + 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 + } + for _, e := range events { + if e.Kind != event.KindVerdictReported { + continue + } + var payload struct { + Approved bool `json:"approved"` + } + if json.Unmarshal(e.Payload, &payload) == nil { + approved = payload.Approved + hasVerdict = true + } + } + return approved, hasVerdict +} + +// optionalBool returns a pointer to v if present is true, nil otherwise — +// used so KindArbitrationDecided's payload omits "approved" entirely +// (rather than encoding a misleading false) when no structured verdict was +// ever reported. +func optionalBool(v bool, present bool) *bool { + if !present { + return nil } + return &v } // processRetro is the Phase 8 retro stage: it runs for every story the Tick -- cgit v1.2.3