fix(labels): unknown check outcomes and mixed rounds must not read green

Round 2 of #128. Two blockers from the bot panel, both real holes in the
invariant this PR exists to establish.

The check-rollup classifier enumerated the outcomes that block and
defaulted everything else to SUCCESS, so ERROR, CANCELLED and STALE fell
through to green. Inverted to an allow-list of the outcomes that do NOT
block — SUCCESS, NEUTRAL, SKIPPED and the pending set — with everything
else blocking. The rollup mixes two closed enums (CheckRun.conclusion and
StatusContext.state) and the costs are asymmetric: a false failure parks
the PR on the agent, who looks; a false success invites a human to merge a
tree that will not merge. Superseded runs are dropped first, each context
collapsing to its newest entry keyed on workflow + job name, so a re-run
does not strand its own PR in needs-rebase. The classifier also moved out
of main() into checks_state(), which is why no fixture caught this — it
was inline in the fetch loop and could only ever be injected pre-decided.

decide_state() returned from inside the bot loop on the first MISSING, so
a STALE belonging to a later bot in BOTS was never read, and a round that
was both unfinished and staled came out needs-human over a head nobody had
reviewed — the original bug wearing a different hat. The whole round is
now collected before any precedence is applied, STALE checked before
MISSING. The MISSING-yields-to-an-explicit-human-request rule is untouched.

Fixtures 29 -> 44, pinning the check-outcome enum, the supersede rule at
both orderings, and the mixed round at both ends of BOTS.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

.github/scripts/labels-reconcile.sh

@@ -52,6 +52,45 @@ run() { # every mutation goes through here — DRY_RUN=1 logs instead of doing
 
 requested() { grep -qxF "$1" <<<"$REQUESTED"; }
 
+checks_state() { # rollup JSON on stdin → SUCCESS | FAILURE | PENDING | NONE
+  # The rollup mixes two node types with two different closed enums: CheckRun
+  # carries `conclusion` (CheckConclusionState), StatusContext carries `state`
+  # (StatusState). Rather than list the outcomes that block — the version that
+  # shipped in this PR's first round listed four, and ERROR, CANCELLED and
+  # STALE fell through its `else` into SUCCESS — this lists the outcomes that
+  # DON'T, and treats everything else as blocking.
+  #
+  # That direction is the point. An outcome we do not recognise is one we
+  # cannot certify as mergeable, and certifying the unrecognised as green is
+  # the exact shape of #136. The cost of being wrong is symmetric in form and
+  # not in consequence: a false FAILURE parks the PR on the agent, who looks;
+  # a false SUCCESS invites a human to merge a tree that will not merge.
+  jq -r '
+    # NEUTRAL and SKIPPED satisfy branch protection — a skipped required check
+    # is not a failed one, and path-filtered jobs skip constantly here.
+    ["SUCCESS", "NEUTRAL", "SKIPPED"] as $passing
+    # "" covers a StatusContext still reported with no state at all.
+    | ["", "PENDING", "IN_PROGRESS", "QUEUED", "WAITING", "REQUESTED", "EXPECTED"] as $waiting
+
+    # A re-run does not evict the run it superseded — the rollup keeps both.
+    # This PR proved it: its own tip carried a CANCELLED `scope` (15:19:39)
+    # beside the SUCCESS `scope` (15:19:45) that replaced it, same workflow.
+    # Once CANCELLED blocks, judging every entry would strand this very PR in
+    # needs-rebase forever, so collapse each context to its newest entry first.
+    # Key on workflow + name because a bare job name is only unique within its
+    # workflow; ordering falls back through the timestamps a pending run has.
+    | [ (.statusCheckRollup // [])[]
+        | { ctx: [.workflowName // "", .name // .context // ""],
+            at:  (.completedAt // .startedAt // .createdAt // ""),
+            outcome: ((.conclusion // .state // "") | ascii_upcase) } ]
+    | group_by(.ctx) | map(sort_by(.at) | last | .outcome) as $latest
+
+    | if   ($latest | length) == 0                            then "NONE"
+      elif (($latest - $passing - $waiting) | length) > 0     then "FAILURE"
+      elif (($latest - $passing) | length) > 0                then "PENDING"
+      else "SUCCESS" end'
+}
+
 bot_verdict() { # $1 = login → MISSING | BLOCK | APPROVE | STALE | FEEDBACK
   local review state commit
   review="$(jq -c --arg u "$1" \
@@ -104,29 +143,37 @@ decide_state() { # → the one state:* label this PR should carry
   case "${MERGEABLE:-UNKNOWN}" in CONFLICTING) echo state:needs-rebase; return ;; esac
   case "${CHECKS:-NONE}" in FAILURE) echo state:needs-rebase; return ;; esac
 
-  local b v verdicts=""
+  local b verdicts=""
   for b in "${BOTS[@]}"; do
     if requested "$b"; then echo state:bots-reviewing; return; fi
   done
+  # Collect the WHOLE round before applying any precedence. Deciding inside
+  # the loop let BOTS order pick the winner: a MISSING returned immediately,
+  # so a STALE belonging to a later bot was never even read, and the mixed
+  # round (one approval staled by a push, another bot yet to review) came out
+  # needs-human — the #136 headline shape, with zero reviews bound to the head.
   for b in "${BOTS[@]}"; do
-    v="$(bot_verdict "$b")"
+    verdicts="$verdicts $(bot_verdict "$b")"
-    if [ "$v" = MISSING ]; then
-      # No verdict at all from this bot. An explicit human request still
-      # outranks an unfinished bot round — a maintainer pulling a PR to
-      # themselves early is a deliberate act, and the original precedence.
-      if requested "$HUMAN"; then echo state:needs-human; return; fi
-      echo state:bots-reviewing; return
-    fi
-    verdicts="$verdicts $v"
   done
   case "$verdicts" in
     # STALE = a verdict for an older head. Unlike MISSING, this outranks the
-    # human request: every approval was invalidated by a push, so NOBODY has
+    # human request: every approval it covers was invalidated by a push, so
-    # reviewed this tree. Handing that to the human is the #136 case where
+    # NOBODY has reviewed this tree. Handing that to the human is the #136 case
-    # everything reads green — mergeable, CI passing, "waiting on the human" —
+    # where everything reads green — mergeable, CI passing, "waiting on the
-    # over code no reviewer has seen. The agent owes a re-request.
+    # human" — over code no reviewer has seen. The agent owes a re-request.
+    # Checked before MISSING because "unfinished" must not swallow "and also
+    # stale": a round that is both is a push that outran the re-requests, not
+    # a maintainer deliberately claiming the PR early.
     *STALE*) echo state:addressing; return ;;
   esac
+  case "$verdicts" in
+    # No verdict at all from some bot, and nothing staled. An explicit human
+    # request still outranks an unfinished round — a maintainer pulling a PR
+    # to themselves early is a deliberate act, and the original precedence.
+    *MISSING*)
+      if requested "$HUMAN"; then echo state:needs-human; return; fi
+      echo state:bots-reviewing; return ;;
+  esac
   # an explicit human request outranks the remaining bot outcomes — it is the
   # final gate, and a maintainer pulling a PR to themselves early counts too
   if requested "$HUMAN"; then echo state:needs-human; return; fi
@@ -158,7 +205,7 @@ state:building|FBCA04|PR is a draft — the coding agent is still building
 state:bots-reviewing|1D76DB|Waiting on the bot reviewers to finish the round
 state:addressing|D93F0B|All bots reviewed — coding agent owes the single reply + fixes
 state:needs-rebase|B60205|Does not merge — conflicts or failing checks; the agent owes a fix
-state:needs-human|8250DF|All bots approve — waiting on the human reviewer
+state:needs-human|8250DF|Mergeable, green, all bots approve — waiting on the human reviewer
 merge-next|0E8A16|Head of the merge queue — merge this one next (set by hand/agent, cleared here)
 stale|B60205|No activity for 48h — needs a poke (sweep-managed)
 blocked|6A737D|Waiting on another PR or issue to land first
@@ -271,12 +318,7 @@ main() {
       # the "do not know" value that triggers nothing.
       GH_VIEW="$(gh pr view "$n" -R "$REPO" --json mergeable,statusCheckRollup 2>/dev/null || echo '{}')"
       MERGEABLE="$(jq -r '.mergeable // "UNKNOWN"' <<<"$GH_VIEW")"
-      CHECKS="$(jq -r '
+      CHECKS="$(checks_state <<<"$GH_VIEW")"
-        (.statusCheckRollup // []) as $c
-        | if ($c | length) == 0 then "NONE"
-          elif ($c | map(.conclusion // .state // "") | any(. == "FAILURE" or . == "TIMED_OUT" or . == "STARTUP_FAILURE" or . == "ACTION_REQUIRED")) then "FAILURE"
-          elif ($c | map(.conclusion // .state // "") | any(. == "" or . == "PENDING" or . == "IN_PROGRESS" or . == "QUEUED")) then "PENDING"
-          else "SUCCESS" end' <<<"$GH_VIEW")"
       reconcile_pr "$n"
     ) || log "#$n: reconcile failed — continuing with the remaining PRs"
   done

CHANGELOG.md

@@ -33,7 +33,27 @@ actually cutting it, and this file starts there.
   re-request. An *unfinished* round still yields to an explicit human request —
   a maintainer pulling a PR to themselves early is deliberate, and `MISSING`
   (nobody has reviewed yet) is a different fact from `STALE` (everyone reviewed
-  something else).
+  something else). Precedence is applied to the round as a whole, after every
+  verdict is collected: deciding inside the loop let the order of `BOTS` pick
+  the answer, so a round that was *both* unfinished and staled returned on the
+  `MISSING` before any later bot's `STALE` was read — and came out
+  `needs-human` over a head nobody had reviewed, the original bug wearing a
+  different hat.
+
+  Whether a check blocks is judged by listing the outcomes that *don't* —
+  `SUCCESS`, `NEUTRAL`, `SKIPPED`, and the pending set — rather than the
+  outcomes that do. The rollup mixes two closed enums (`CheckRun.conclusion`
+  and `StatusContext.state`), and an outcome the list forgets is one the label
+  cannot certify as mergeable: `ERROR`, `CANCELLED` and `STALE` all read as
+  green under an allow-list of failures. The costs are not symmetric — a false
+  failure parks the PR on the agent, who looks; a false success invites a human
+  to merge a tree that will not merge. Superseded runs are dropped first, each
+  context collapsing to its newest entry: a re-run does not evict the run it
+  replaced, and the rollup keeps both. That shape is live on this board — this
+  PR's own tip carried two `scope` and two `reconcile` entries — and on
+  heavy-duty/box#137's tip the superseded half was `CANCELLED`, so once
+  `CANCELLED` blocks, judging every entry rather than the newest would strand
+  every re-run PR in `needs-rebase`.
 
   `UNKNOWN` mergeability is deliberately not treated as unmergeable: GitHub
   reports it for about a minute after every merge while it recomputes, and
@@ -45,8 +65,9 @@ actually cutting it, and this file starts there.
   correct `needs-human` still does not say *which* of three ready PRs to merge
   first. Queue order is intent, so the reconciler never sets it; it only
   **clears** it once the PR stops being mergeable-by-a-human. Ported from
-  heavy-duty/box#137 so the three repos' reconcilers stay byte-identical;
+  heavy-duty/box#137 so the three repos' reconcilers stay byte-identical; both
-  fixtures 19 → 29.
+  live shapes, the mixed round, and the whole check-outcome enum are pinned in
+  `test/labels-reconcile.sh` (fixtures 19 → 44).
 
 ## 0.1.1 — 2026-07-19
 

LABELS.md

@@ -97,7 +97,7 @@ gh label create "state:building"       --color FBCA04 --description "PR is a dra
 gh label create "state:bots-reviewing" --color 1D76DB --description "Waiting on the bot reviewers to finish the round" --force
 gh label create "state:addressing"     --color D93F0B --description "All bots reviewed — coding agent owes the single reply + fixes" --force
 gh label create "state:needs-rebase"   --color B60205 --description "Does not merge — conflicts or failing checks; the agent owes a fix" --force
-gh label create "state:needs-human"    --color 8250DF --description "All bots approve — waiting on the human reviewer" --force
+gh label create "state:needs-human"    --color 8250DF --description "Mergeable, green, all bots approve — waiting on the human reviewer" --force
 gh label create "merge-next"           --color 0E8A16 --description "Head of the merge queue — merge this one next (set by hand/agent, cleared here)" --force
 gh label create "stale"                --color B60205 --description "No activity for 48h — needs a poke (sweep-managed)" --force
 gh label create "blocked"              --color 6A737D --description "Waiting on another PR or issue to land first" --force

test/labels-reconcile.sh

@@ -185,6 +185,21 @@ REVIEWS_JSON="$(reviews \
   "$(rev "$BOT3" APPROVED oldhead "" t3)")"
 expect "stale approvals outrank the human request (nobody reviewed this tree)" state:addressing "$(decide_state)"
 
+# -- ...and a round that is BOTH unfinished and staled is still the agent's.
+#    Deciding inside the bot loop made this depend on BOTS order: the MISSING
+#    returned before any later bot's STALE was read, so the mixed round came
+#    out needs-human with nothing bound to the head. Pinned at both ends of
+#    the array, because the whole failure was one of ordering.
+MERGEABLE=MERGEABLE CHECKS=SUCCESS REQUESTED="$HUMAN"
+REVIEWS_JSON="$(reviews \
+  "$(rev "$BOT1" APPROVED oldhead "" t1)" \
+  "$(rev "$BOT2" APPROVED oldhead "" t2)")"
+expect "stale approvals + a bot yet to review is addressing, not needs-human" \
+  state:addressing "$(decide_state)"
+REVIEWS_JSON="$(reviews "$(rev "$BOT3" APPROVED oldhead "" t3)")"
+expect "...and the same when the stale verdict is the LAST bot in BOTS" \
+  state:addressing "$(decide_state)"
+
 # -- but an UNFINISHED round still yields to an explicit human request: a
 #    maintainer pulling a PR to themselves early is deliberate, and was the
 #    original precedence. MISSING differs from STALE — nobody has reviewed
@@ -194,6 +209,61 @@ expect "an unfinished round still yields to an explicit human request" state:nee
 REQUESTED=""
 expect "...and without that request it is still bots-reviewing" state:bots-reviewing "$(decide_state)"
 
+# ---------------------------------------------------------------------------
+# checks_state: the rollup classifier. It lived inline in main() for the first
+# round of this PR, which is why nothing here caught it calling ERROR,
+# CANCELLED and STALE green. Extracted so the enum can be pinned down.
+# ---------------------------------------------------------------------------
+rollup() { jq -n --argjson c "$1" '{statusCheckRollup: $c}'; }
+run_() { jq -n --arg n "$1" --arg o "$2" --arg t "${3:-2026-07-20T15:00:00Z}" \
+  '{__typename:"CheckRun", workflowName:"ci", name:$n, conclusion:$o, completedAt:$t}'; }
+ctx_() { jq -n --arg n "$1" --arg s "$2" --arg t "${3:-2026-07-20T15:00:00Z}" \
+  '{__typename:"StatusContext", context:$n, state:$s, createdAt:$t}'; }
+
+expect "no checks at all is NONE" NONE "$(rollup '[]' | checks_state)"
+expect "all green is SUCCESS" SUCCESS \
+  "$(rollup "[$(run_ a SUCCESS),$(run_ b SUCCESS)]" | checks_state)"
+expect "a queued run is PENDING" PENDING \
+  "$(rollup "[$(run_ a SUCCESS),$(run_ b QUEUED)]" | checks_state)"
+expect "a plain failure is FAILURE" FAILURE \
+  "$(rollup "[$(run_ a SUCCESS),$(run_ b FAILURE)]" | checks_state)"
+
+# -- the round-1 gap: outcomes that are neither success nor pending, and that
+#    leave a required check unsatisfied. All three reached the old `else`.
+expect "a commit status ERROR blocks" FAILURE \
+  "$(rollup "[$(run_ a SUCCESS),$(ctx_ lint ERROR)]" | checks_state)"
+expect "a CANCELLED run blocks" FAILURE \
+  "$(rollup "[$(run_ a SUCCESS),$(run_ b CANCELLED)]" | checks_state)"
+expect "a STALE run blocks" FAILURE \
+  "$(rollup "[$(run_ a SUCCESS),$(run_ b STALE)]" | checks_state)"
+expect "an outcome the enum does not know blocks, it does not pass" FAILURE \
+  "$(rollup "[$(run_ a SUCCESS),$(run_ b SOME_FUTURE_STATE)]" | checks_state)"
+
+# -- NEUTRAL and SKIPPED satisfy branch protection; path-filtered jobs skip
+#    constantly, and calling that red would park every PR on the agent.
+expect "NEUTRAL and SKIPPED are not failures" SUCCESS \
+  "$(rollup "[$(run_ a SUCCESS),$(run_ b NEUTRAL),$(run_ c SKIPPED)]" | checks_state)"
+
+# -- latest-wins. The rollup keeps superseded runs, so this PR's own tip
+#    carried a CANCELLED `scope` beside the SUCCESS `scope` that replaced it.
+#    Without collapsing, making CANCELLED block would strand it forever.
+expect "a re-run supersedes the cancelled original" SUCCESS \
+  "$(rollup "[$(run_ scope CANCELLED 2026-07-20T15:19:39Z),\
+              $(run_ scope SUCCESS   2026-07-20T15:19:45Z)]" | checks_state)"
+expect "...and the reverse order is not a re-run passing, it is one failing" FAILURE \
+  "$(rollup "[$(run_ scope SUCCESS   2026-07-20T15:19:39Z),\
+              $(run_ scope CANCELLED 2026-07-20T15:19:45Z)]" | checks_state)"
+# same job name in a different workflow is a different context, not a re-run
+expect "same name in another workflow does not supersede" FAILURE \
+  "$(rollup "[$(jq -n '{__typename:"CheckRun",workflowName:"labels",name:"scope",conclusion:"FAILURE",completedAt:"2026-07-20T15:00:00Z"}'),\
+              $(run_ scope SUCCESS 2026-07-20T15:19:45Z)]" | checks_state)"
+
+# -- the classifier feeds the state machine: a cancelled required check must
+#    take the PR off the human's plate, which is the whole point of #136.
+DRAFT=false HEAD_SHA=head1 REQUESTED="$HUMAN" REVIEWS_JSON="$ALL_APPROVE" MERGEABLE=MERGEABLE
+CHECKS="$(rollup "[$(run_ a SUCCESS),$(run_ b CANCELLED)]" | checks_state)"
+expect "a cancelled check reaches decide_state as needs-rebase" state:needs-rebase "$(decide_state)"
+
 # -- the happy path survives all of the above.
 REVIEWS_JSON="$ALL_APPROVE" MERGEABLE=MERGEABLE CHECKS=SUCCESS REQUESTED=""
 expect "mergeable + green + three head-current approvals is needs-human" state:needs-human "$(decide_state)"