summaryrefslogtreecommitdiff
path: root/project_claudomator_arbitration_fail_open_incident.md
blob: 475c4eeef19aabe0515862e142d25c81dfb40a19 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
---
name: claudomator-arbitration-fail-open-incident
description: FIXED 2026-07-11 (commits cc1aa8d/204fe6c) — a live story shipped factually-wrong work because arbitration never called report_verdict and the system defaulted to silent approval
metadata: 
  node_type: memory
  type: project
  originSessionId: 560b9055-7a3f-483e-b1a0-1a4c70419390
---

**Status: fixed**, commits `cc1aa8d`/`204fe6c` on `main`, deployed 2026-07-11. `StoryOrchestrator.finalizeArbitration` is now fail-closed: approval requires an explicit `report_verdict(approved: true, ...)` call; an explicit rejection OR no verdict reported at all both reject the node (previously, no verdict silently defaulted to approval). `SeedRoleConfigs` now also seeds `planner` (the arbitration role) with a system prompt that explicitly mandates calling `report_verdict` before finishing — the role previously had no system prompt at all, only `builder` did (piece 5's original scoping).

**What happened (real incident, first live run of the finished recursive-arbitrated-review design):** the very first story run through the newly-deployed design (a "diagram claudomator's workflow" story, self-referential dogfooding) had `evaluator_correctness` correctly flag 3 factually-wrong data-flow claims in the produced diagram — but the arbitration task that read all 4 evaluators' findings never called `report_verdict` at all (no system prompt told it the tool existed or was mandatory). The old fail-open default treated "no verdict" the same as "approved," so the story moved to `REVIEW_READY` with broken work, and nothing in the system detected this — the user caught it by reading the evaluator's own verdict text and asking "shouldn't this eval have failed?"

**Why this matters beyond the one bug:** this is the second gap traced to piece 5's deliberate choice to seed only `builder`'s system prompt, not the other 5 roles (`evaluator_*`, `planner`, `retro`) — see [[claudomator-task-board-accept-workflow]] session context. The first gap (no `role_configs` = no `Scheduler` retry safety net on transient failure) was found earlier the same session; this one (an unguided arbitration agent silently no-ops the entire review gate) is more serious, since it defeats the whole point of "arbitrated review" as quality control, not just availability.

**How to apply:** If seeding role prompts for the other 4 roles (`evaluator_quality/security/correctness/performance`, `retro`) comes up, check whether any of them have a similarly load-bearing "must call this specific tool" requirement with no explicit instruction to do so — `retro`'s `propose_role_config`/`report_summary` calls are a plausible candidate to check.

**Also surfaced during cleanup of this incident:** a rejected root task that had *already* been incorrectly approved (`READY → COMPLETED`) before the fix landed cannot be un-approved — `COMPLETED` is a terminal state in `task.ValidTransition`. Manually resetting the *story* back to `NEEDS_FIX` after that point is not enough by itself: `ensureFixAttempt`'s idempotency check looks for an existing `builder`-role dependent of the (now-permanently-`COMPLETED`) root and, once that fix-attempt itself later dies (e.g. an infra failure exhausting its retry ladder), deleting the dead fix-attempt task alone does *not* re-trigger `ensureFixAttempt` — the story's `status` has already moved to `IN_PROGRESS` and stays there, `task.CurrentAttempt` resolves back to the original `COMPLETED` root, and `processStory`'s `root.State == StateCompleted` short-circuit means nothing further ever happens for that story. The fix is to re-set `story.status = "NEEDS_FIX"` again after deleting the dead fix-attempt (not just delete the task) — `ensureFixAttempt` doesn't care that the anchor root is `COMPLETED`, it just needs the story status to actually be `NEEDS_FIX` to fire at all.