From 204fe6c536d5d3bca2cb712f66b1d63054807dac Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Sat, 11 Jul 2026 05:37:45 +0000 Subject: fix(scheduler): make arbitration fail-closed on a missing verdict, seed planner's system prompt A live production run (2026-07-11) showed finalizeArbitration's 'no verdict reported = approve' default silently shipping work an evaluator had already flagged as factually wrong -- the arbitration agent never called report_verdict because the planner role had no system prompt telling it to. finalizeArbitration now treats a missing verdict the same as an explicit rejection (fail-closed), and SeedRoleConfigs now also seeds planner with a prompt that explicitly mandates calling report_verdict before finishing. --- internal/scheduler/story_orchestrator.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) (limited to 'internal/scheduler/story_orchestrator.go') diff --git a/internal/scheduler/story_orchestrator.go b/internal/scheduler/story_orchestrator.go index a526552..31efa39 100644 --- a/internal/scheduler/story_orchestrator.go +++ b/internal/scheduler/story_orchestrator.go @@ -486,6 +486,16 @@ func (o *StoryOrchestrator) ensureArbitration(ctx context.Context, st *story.Sto // 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. +// Approval requires an EXPLICIT verdict with approved=true; an explicit +// rejection OR no structured verdict reported at all both reject the node. +// This is fail-closed by design: an earlier version defaulted to approval +// when no verdict was reported ("preserving prior behavior for agents that +// don't report a structured verdict"). A live production run (2026-07-11) +// demonstrated why that default is dangerous for a review gate: an +// arbitration agent that never calls report_verdict (because its role had +// no system prompt telling it to -- see SeedRoleConfigs) shipped work an +// evaluator had already found factually wrong, entirely undetected, because +// "no verdict" was silently treated the same as "approved." 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 { @@ -521,7 +531,10 @@ func (o *StoryOrchestrator) finalizeArbitration(ctx context.Context, st *story.S o.logf("story orchestrator: emit arbitration_decided", "storyID", st.ID, "error", err) } - if hasVerdict && !approved { + if !hasVerdict || !approved { + if !hasVerdict { + o.logf("story orchestrator: finalize arbitration: no verdict reported, treating as rejection (fail-closed)", "storyID", st.ID, "taskID", node.ID, "arbitrationID", arbitration.ID) + } if isRoot { st.Status = "NEEDS_FIX" if err := o.Store.UpdateStory(st); err != nil { @@ -533,9 +546,8 @@ func (o *StoryOrchestrator) finalizeArbitration(ctx context.Context, st *story.S return } - // Approved (or no structured verdict reported -- preserves the prior - // unconditional-approve default). node only becomes COMPLETED here, - // after arbitration. + // Approved -- an explicit, structured verdict with approved=true. 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 -- cgit v1.2.3