rig/test/labels-reconcile.sh

329 lines
18 KiB
Bash
Raw Normal View History

#!/usr/bin/env bash
set -euo pipefail
# Fixture tests for the labels-reconcile state machine: a comment is a
# non-verdict whatever its body says (the AUTHOR escalates by requesting the
# human), a stale approval does not promote unreviewed code, and an explicit
# human request outranks everything.
# Dependency-free beyond jq; no network, no daemon — pure decide_state.
cd "$(dirname "$0")/.."
# shellcheck source=.github/scripts/labels-reconcile.sh
. .github/scripts/labels-reconcile.sh
# The DRAFT/HEAD_SHA/REQUESTED/REVIEWS_JSON assignments below are the state
# machine's inputs, consumed inside the sourced decide_state — not unused.
# shellcheck disable=SC2034
BOT1="${BOTS[0]}" BOT2="${BOTS[1]}" BOT3="${BOTS[2]}"
pass=0 fail=0
expect() { # $1 = description, $2 = want, $3 = got
if [ "$2" = "$3" ]; then
pass=$((pass + 1))
else
fail=$((fail + 1))
printf 'FAIL: %s — want %s, got %s\n' "$1" "$2" "$3"
fi
}
rev() { # $1=login $2=state $3=commit $4=body $5=submitted_at → one review object
jq -n --arg u "$1" --arg s "$2" --arg c "$3" --arg b "$4" --arg t "$5" \
'{user: {login: $u}, state: $s, commit_id: $c, body: $b, submitted_at: $t}'
}
reviews() { jq -s '.' <<<"$*"; } # collect review objects into an array
# -- drafts are building, whoever is requested --------------------------------
DRAFT=true HEAD_SHA=head1 REQUESTED="" REVIEWS_JSON='[]'
expect "draft PR is building" state:building "$(decide_state)"
# -- fresh ready PR with bots requested ---------------------------------------
DRAFT=false REQUESTED="$BOT1
$BOT2
$BOT3" REVIEWS_JSON='[]'
expect "requested bots mean bots-reviewing" state:bots-reviewing "$(decide_state)"
# -- a bot that never reviewed keeps the round open ---------------------------
REQUESTED="" REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" APPROVED head1 "" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)")"
expect "missing bot review means bots-reviewing" state:bots-reviewing "$(decide_state)"
# -- a comment is a non-verdict, agreement body or not: the author escalates --
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" COMMENTED head1 "✅ **Reviewed — I agree with everything.**" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)")"
expect "comment-only agreement still parks on the author" state:addressing "$(decide_state)"
# ...and the author's escalation — requesting the human — flips it
REQUESTED="$HUMAN"
expect "author escalation flips to needs-human" state:needs-human "$(decide_state)"
REQUESTED=""
# -- three formal approvals need no author judgment ---------------------------
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" APPROVED head1 "" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)")"
expect "three formal approvals reach needs-human" state:needs-human "$(decide_state)"
# -- a comment WITHOUT a verdict parks the PR on the agent --------------------
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" COMMENTED head1 "🔧 Reviewed — I agree with most; feedback below." t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)")"
expect "comment without verdict is addressing" state:addressing "$(decide_state)"
# -- changes requested blocks, at any head ------------------------------------
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" CHANGES_REQUESTED old1 "blockers below" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)")"
expect "changes-requested blocks even from an old head" state:addressing "$(decide_state)"
# -- a stale approval must not promote unreviewed code ------------------------
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" APPROVED old1 "" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)")"
expect "stale approval is addressing (agent owes re-request)" state:addressing "$(decide_state)"
# -- a re-requested bot reopens the round even with an old approval on file ---
REQUESTED="$BOT1"
expect "re-requested bot means bots-reviewing" state:bots-reviewing "$(decide_state)"
REQUESTED=""
# -- only the LATEST review per bot counts ------------------------------------
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" CHANGES_REQUESTED head1 "blockers" t1)" \
"$(rev "$BOT1" APPROVED head1 "" t2)" \
"$(rev "$BOT2" APPROVED head1 "" t3)" \
"$(rev "$BOT3" APPROVED head1 "" t4)")"
expect "later approval supersedes earlier block" state:needs-human "$(decide_state)"
# -- an explicit human request outranks the bot rounds ------------------------
REQUESTED="$HUMAN" REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" COMMENTED head1 "feedback, no verdict" t1)")"
expect "human requested outranks bots" state:needs-human "$(decide_state)"
REQUESTED=""
# -- human CHANGES_REQUESTED puts the ball back on the agent ------------------
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" APPROVED head1 "" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)" \
"$(rev "$HUMAN" CHANGES_REQUESTED head1 "not yet" t4)")"
expect "human block with bots approving is addressing" state:addressing "$(decide_state)"
# ...and re-requesting the human hands it back to them
REQUESTED="$HUMAN"
expect "re-requested human is needs-human again" state:needs-human "$(decide_state)"
REQUESTED=""
# -- an old human comment must not wedge the handoff (codex, #85 round 3) -----
REVIEWS_JSON="$(reviews \
"$(rev "$HUMAN" COMMENTED old1 "early thoughts" t0)" \
"$(rev "$BOT1" APPROVED head1 "" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)")"
expect "old human comment + three approvals is needs-human" state:needs-human "$(decide_state)"
expect "old human comment still needs a fresh request" needed "$(human_request_needed && echo needed || echo not-needed)"
# ...a stale human APPROVAL likewise needs a re-request for the new head
REVIEWS_JSON="$(reviews \
"$(rev "$HUMAN" APPROVED old1 "" t0)" \
"$(rev "$BOT1" APPROVED head1 "" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)")"
expect "stale human approval needs a fresh request" needed "$(human_request_needed && echo needed || echo not-needed)"
# ...a HEAD-CURRENT human approval needs nothing more
REVIEWS_JSON="$(reviews \
"$(rev "$HUMAN" APPROVED head1 "" t0)" \
"$(rev "$BOT1" APPROVED head1 "" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)")"
expect "head-current human approval needs no request" not-needed "$(human_request_needed && echo needed || echo not-needed)"
# ...and a live request suppresses re-requesting
REQUESTED="$HUMAN"
expect "live human request suppresses re-request" not-needed "$(human_request_needed && echo needed || echo not-needed)"
REQUESTED=""
fix(labels): state:needs-human means a human could merge it right now Ported from heavy-duty/box#137 (heavy-duty/box#136) so the three repos' reconcilers stay byte-identical. The state machine here was byte-identical to box's before this change, and remains so after -- only the scope:* taxonomy differs, correctly. decide_state() derived state from three inputs -- draft flag, requested reviewers, submitted reviews -- and read NOTHING about mergeability or checks. With the `if requested "$HUMAN"` short-circuit at the top of its precedence, the label was sticky: once the maintainer was requested, a PR read state:needs-human through conflicts, through red CI, through a force-push that staled every approval. This repo paid for it directly. During the ten-PR batch merged today, every merge re-conflicted the PRs below it through CHANGELOG.md, and each kept its state:needs-human label throughout -- inviting merges that could not happen. It was caught only by opening them one at a time, which is the work the label exists to save. The rule the label now keeps: state:needs-human means a human could merge this RIGHT NOW, so anything making that false outranks the request that put it there. CONFLICTING or failing checks -> state:needs-rebase (new; the agent's to fix) approvals staled by a push -> state:addressing (nobody reviewed this tree) An UNFINISHED round still yields to an explicit human request -- MISSING (nobody has reviewed yet) is a different fact from STALE (everyone reviewed something else). UNKNOWN mergeability is NOT treated as unmergeable: GitHub reports it for about a minute after every merge, and flapping every open PR through needs-rebase on each merge would be worse than the bug. A failed read degrades to the same "do not know" value. Also adds merge-next: queue order is intent, so the reconciler never sets it, only CLEARS it once the PR stops being mergeable-by-a-human. Fixtures 19 -> 29, including that UNKNOWN does not trigger needs-rebase and a draft outranks a conflict. No live dry-run evidence here -- this repo has no open PRs right now -- so the fixtures and box's live dry-run are the proof. Closes #87 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 15:26:54 +00:00
# ---------------------------------------------------------------------------
# #136: state:needs-human must mean "a human could merge this RIGHT NOW".
# Both cases below were observed live in this repo on 2026-07-20, and both
# showed state:needs-human while being unmergeable in different ways.
# ---------------------------------------------------------------------------
ALL_APPROVE="$(reviews \
"$(rev "$BOT1" APPROVED head1 "" t1)" \
"$(rev "$BOT2" APPROVED head1 "" t2)" \
"$(rev "$BOT3" APPROVED head1 "" t3)")"
# -- flavour 1: not mergeable. The merge button is disabled, yet the board
# said "your turn" on #119/#120/#127 for hours.
DRAFT=false HEAD_SHA=head1 REQUESTED="" REVIEWS_JSON="$ALL_APPROVE" MERGEABLE=CONFLICTING CHECKS=SUCCESS
expect "a CONFLICTING PR is needs-rebase, not needs-human" state:needs-rebase "$(decide_state)"
REQUESTED="$HUMAN"
expect "...even with the human explicitly requested" state:needs-rebase "$(decide_state)"
# -- red CI is the same claim: not something a human should merge.
REQUESTED="" MERGEABLE=MERGEABLE CHECKS=FAILURE
expect "a red PR is needs-rebase" state:needs-rebase "$(decide_state)"
REQUESTED="$HUMAN"
expect "...and a human request does not override red CI" state:needs-rebase "$(decide_state)"
# -- UNKNOWN is NOT unmergeable. GitHub reports it for ~a minute after every
# merge while it recomputes; treating it as broken would flap every open PR
# into needs-rebase on each merge — worse than the bug being fixed.
REQUESTED="" MERGEABLE=UNKNOWN CHECKS=PENDING
expect "UNKNOWN mergeability does not trigger needs-rebase" state:needs-human "$(decide_state)"
# -- flavour 2 (the dangerous one): mergeable, green, human requested, and
# NOBODY has reviewed this head. Observed on #119 after a rebase: every
# signal read "merge me" and nothing on the page contradicted it.
MERGEABLE=MERGEABLE CHECKS=SUCCESS REQUESTED="$HUMAN"
REVIEWS_JSON="$(reviews \
"$(rev "$BOT1" APPROVED oldhead "" t1)" \
"$(rev "$BOT2" APPROVED oldhead "" t2)" \
"$(rev "$BOT3" APPROVED oldhead "" t3)")"
expect "stale approvals outrank the human request (nobody reviewed this tree)" state:addressing "$(decide_state)"
fix(labels): unrecognised check outcomes block, and STALE outranks MISSING Round 2 review found two ways the "a human could merge this right now" invariant still leaked, both of which let state:needs-human land on a PR the button would refuse. The check-rollup classifier enumerated the outcomes that block and defaulted the rest to SUCCESS, so ERROR, CANCELLED and STALE fell through into green. Inverted to an allow-list of the outcomes that DON'T block (SUCCESS, NEUTRAL, SKIPPED, plus the pending set); everything else, including an outcome neither enum has today, blocks. The rollup mixes CheckRun.conclusion with StatusContext.state and an outcome the list forgets is one we cannot certify as mergeable — a false FAILURE parks the PR on the agent, a false SUCCESS invites a bad merge. 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 the jq itself was untestable. Once CANCELLED blocks, superseded runs must be dropped first — a re-run does not evict the run it replaced, and judging every entry would strand every re-run PR in needs-rebase. Each context now collapses to its newest entry, keyed on workflow + job name because a bare job name is only unique within its workflow. 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: a round that was both unfinished and staled came out needs-human with nothing bound to the head. The whole round is now collected before any precedence is applied, STALE ahead of MISSING. The MISSING-yields-to-an-explicit-human-request rule is untouched. Fixtures 29 -> 44, pinning the whole check-outcome enum, the supersede rule in both orders, and the mixed round at both ends of BOTS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 16:05:39 +00:00
# -- ...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)"
fix(labels): state:needs-human means a human could merge it right now Ported from heavy-duty/box#137 (heavy-duty/box#136) so the three repos' reconcilers stay byte-identical. The state machine here was byte-identical to box's before this change, and remains so after -- only the scope:* taxonomy differs, correctly. decide_state() derived state from three inputs -- draft flag, requested reviewers, submitted reviews -- and read NOTHING about mergeability or checks. With the `if requested "$HUMAN"` short-circuit at the top of its precedence, the label was sticky: once the maintainer was requested, a PR read state:needs-human through conflicts, through red CI, through a force-push that staled every approval. This repo paid for it directly. During the ten-PR batch merged today, every merge re-conflicted the PRs below it through CHANGELOG.md, and each kept its state:needs-human label throughout -- inviting merges that could not happen. It was caught only by opening them one at a time, which is the work the label exists to save. The rule the label now keeps: state:needs-human means a human could merge this RIGHT NOW, so anything making that false outranks the request that put it there. CONFLICTING or failing checks -> state:needs-rebase (new; the agent's to fix) approvals staled by a push -> state:addressing (nobody reviewed this tree) An UNFINISHED round still yields to an explicit human request -- MISSING (nobody has reviewed yet) is a different fact from STALE (everyone reviewed something else). UNKNOWN mergeability is NOT treated as unmergeable: GitHub reports it for about a minute after every merge, and flapping every open PR through needs-rebase on each merge would be worse than the bug. A failed read degrades to the same "do not know" value. Also adds merge-next: queue order is intent, so the reconciler never sets it, only CLEARS it once the PR stops being mergeable-by-a-human. Fixtures 19 -> 29, including that UNKNOWN does not trigger needs-rebase and a draft outranks a conflict. No live dry-run evidence here -- this repo has no open PRs right now -- so the fixtures and box's live dry-run are the proof. Closes #87 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 15:26:54 +00:00
# -- 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
# YET, versus everyone reviewed something else.
REVIEWS_JSON="$(reviews "$(rev "$BOT1" APPROVED head1 "" t1)")"
expect "an unfinished round still yields to an explicit human request" state:needs-human "$(decide_state)"
REQUESTED=""
expect "...and without that request it is still bots-reviewing" state:bots-reviewing "$(decide_state)"
fix(labels): unrecognised check outcomes block, and STALE outranks MISSING Round 2 review found two ways the "a human could merge this right now" invariant still leaked, both of which let state:needs-human land on a PR the button would refuse. The check-rollup classifier enumerated the outcomes that block and defaulted the rest to SUCCESS, so ERROR, CANCELLED and STALE fell through into green. Inverted to an allow-list of the outcomes that DON'T block (SUCCESS, NEUTRAL, SKIPPED, plus the pending set); everything else, including an outcome neither enum has today, blocks. The rollup mixes CheckRun.conclusion with StatusContext.state and an outcome the list forgets is one we cannot certify as mergeable — a false FAILURE parks the PR on the agent, a false SUCCESS invites a bad merge. 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 the jq itself was untestable. Once CANCELLED blocks, superseded runs must be dropped first — a re-run does not evict the run it replaced, and judging every entry would strand every re-run PR in needs-rebase. Each context now collapses to its newest entry, keyed on workflow + job name because a bare job name is only unique within its workflow. 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: a round that was both unfinished and staled came out needs-human with nothing bound to the head. The whole round is now collected before any precedence is applied, STALE ahead of MISSING. The MISSING-yields-to-an-explicit-human-request rule is untouched. Fixtures 29 -> 44, pinning the whole check-outcome enum, the supersede rule in both orders, and the mixed round at both ends of BOTS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 16:05:39 +00:00
# ---------------------------------------------------------------------------
# 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)"
fix(labels): date a check run by when it started, not by a zero completion The supersede collapse added in the previous commit dated each run by `.completedAt // .startedAt // .createdAt`. A run still in flight has no completion, but `gh` does not omit the field: its Go struct marshals the zero time as the string "0001-01-01T00:00:00Z", and jq's `//` falls through null and false only. The sentinel was therefore taken as the sort key, and it sorts before every real timestamp — so the LIVE re-run became the oldest entry in its context, `last` discarded it, and the run it superseded was judged instead. That restored #136 through the fix for it: a green context with a replacement mid-flight reported SUCCESS, so a PR read mergeable, green, all bots approve — state:needs-human — while branch protection had the merge button disabled. It also narrowed rather than removed the flap the supersede rule exists to prevent: between "run A cancelled by the concurrency group" and "run B finishes", the PR reported FAILURE and the agent was sent to fix something that was not broken. Runs are now dated by the newest timestamp they actually carry, with both spellings of absent discarded (null, and the zero sentinel). Entries that carry no usable timestamp sort LAST rather than first: something we cannot date is most likely the thing just created, and treating it as newest keeps an undateable in-flight run from being discarded in favour of a stale success. Every ambiguity resolves toward "not settled". Found independently by claude-bot-andresmgsl and codex-bot-andresmgsl. The fixtures could not have caught it: `run_()` always emits a real completedAt, so every supersede fixture was a race between two finished runs, and the bug lived in the one shape the helper could not express. New `inflight_()` helper covers it; fixtures 44 -> 48. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 16:18:50 +00:00
# -- a run still IN FLIGHT. `run_()` cannot express this: it always carries a
# real completedAt, which is exactly why the supersede rule shipped dating
# runs by completion and nothing caught it. Both spellings of "no
# completion" are pinned, because `gh` emits the zero sentinel (a string,
# which `//` does not fall through) while the API emits null.
inflight_() { jq -n --arg n "$1" --arg t "$2" --arg c "${3:-0001-01-01T00:00:00Z}" \
'{__typename:"CheckRun", workflowName:"ci", name:$n, status:"IN_PROGRESS",
conclusion:"", startedAt:$t, completedAt:(if $c == "null" then null else $c end)}'; }
expect "a re-run in flight beats the success it superseded (zero sentinel)" PENDING \
"$(rollup "[$(run_ build SUCCESS 2026-07-20T15:00:00Z),\
$(inflight_ build 2026-07-20T15:10:00Z)]" | checks_state)"
expect "...and the same when the absent completion is null" PENDING \
"$(rollup "[$(run_ build SUCCESS 2026-07-20T15:00:00Z),\
$(inflight_ build 2026-07-20T15:10:00Z null)]" | checks_state)"
expect "a replacement in flight for a CANCELLED run is pending, not failed" PENDING \
"$(rollup "[$(run_ build CANCELLED 2026-07-20T15:00:00Z),\
$(inflight_ build 2026-07-20T15:10:00Z)]" | checks_state)"
# an entry carrying no usable timestamp is treated as newest, not oldest —
fix(labels): order check runs by one consistent quantity — when they began The dating expression took the newest stamp each run carries. That reads `completedAt` for a finished run and `startedAt` for a live one, so the comparison comes down to "when this one ended" against "when that one began" — which is not an ordering on runs at all. A run cancelled by the concurrency group does not stop the instant its replacement starts: the runner has to receive the signal and wind down. So `predecessor.completedAt > successor.startedAt` is the ordinary case, not a corner. On the box#137 tip that motivated the supersede rule the window was 13s wide — the superseding run started 15:19:38, the run it cancelled did not finish until 15:19:51 — and for that whole window the dying predecessor out-dated its own live replacement, so `last` discarded the replacement and judged the corpse. Both round-3 failure modes came back inside that window, narrowed rather than closed: a draining CANCELLED predecessor reported FAILURE and sent the agent to fix nothing, and a draining SUCCESS predecessor reported SUCCESS — mergeable, all bots approve, state:needs-human — over a tree whose merge button branch protection had already disabled. #136 again, one field over. Dated by `first` of the preference-ordered stamps rather than `max` of them: start time if the run recorded one, falling back only if it did not. The sentinel filtering is unchanged, and finished runs still date by completion when that is all they carry, so the supersede rule keeps the case it exists for. Found independently by claude-bot-andresmgsl and codex-bot-andresmgsl. No existing fixture could express it — `run_()` carries no startedAt, so every supersede fixture spaced the predecessor's completion safely before the successor's start, the same blind spot as round 3 one field over. New `drained_()` helper pins both directions; fixtures 48 -> 51 (with the reverse-direction in-flight fixture ported from cast#128). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 16:32:53 +00:00
# ambiguity resolves toward "not settled" rather than toward a stale success.
# Guarded by the sort tiebreak rather than the dating expression: reverting
# only `at:` leaves this passing, so the two changes are separately pinned.
fix(labels): date a check run by when it started, not by a zero completion The supersede collapse added in the previous commit dated each run by `.completedAt // .startedAt // .createdAt`. A run still in flight has no completion, but `gh` does not omit the field: its Go struct marshals the zero time as the string "0001-01-01T00:00:00Z", and jq's `//` falls through null and false only. The sentinel was therefore taken as the sort key, and it sorts before every real timestamp — so the LIVE re-run became the oldest entry in its context, `last` discarded it, and the run it superseded was judged instead. That restored #136 through the fix for it: a green context with a replacement mid-flight reported SUCCESS, so a PR read mergeable, green, all bots approve — state:needs-human — while branch protection had the merge button disabled. It also narrowed rather than removed the flap the supersede rule exists to prevent: between "run A cancelled by the concurrency group" and "run B finishes", the PR reported FAILURE and the agent was sent to fix something that was not broken. Runs are now dated by the newest timestamp they actually carry, with both spellings of absent discarded (null, and the zero sentinel). Entries that carry no usable timestamp sort LAST rather than first: something we cannot date is most likely the thing just created, and treating it as newest keeps an undateable in-flight run from being discarded in favour of a stale success. Every ambiguity resolves toward "not settled". Found independently by claude-bot-andresmgsl and codex-bot-andresmgsl. The fixtures could not have caught it: `run_()` always emits a real completedAt, so every supersede fixture was a race between two finished runs, and the bug lived in the one shape the helper could not express. New `inflight_()` helper covers it; fixtures 44 -> 48. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 16:18:50 +00:00
expect "an undateable in-flight run is not discarded for a stale success" PENDING \
"$(rollup "[$(run_ build SUCCESS 2026-07-20T15:00:00Z),\
$(jq -n '{__typename:"CheckRun",workflowName:"ci",name:"build",conclusion:"",startedAt:null,completedAt:null}')]" \
| checks_state)"
fix(labels): order check runs by one consistent quantity — when they began The dating expression took the newest stamp each run carries. That reads `completedAt` for a finished run and `startedAt` for a live one, so the comparison comes down to "when this one ended" against "when that one began" — which is not an ordering on runs at all. A run cancelled by the concurrency group does not stop the instant its replacement starts: the runner has to receive the signal and wind down. So `predecessor.completedAt > successor.startedAt` is the ordinary case, not a corner. On the box#137 tip that motivated the supersede rule the window was 13s wide — the superseding run started 15:19:38, the run it cancelled did not finish until 15:19:51 — and for that whole window the dying predecessor out-dated its own live replacement, so `last` discarded the replacement and judged the corpse. Both round-3 failure modes came back inside that window, narrowed rather than closed: a draining CANCELLED predecessor reported FAILURE and sent the agent to fix nothing, and a draining SUCCESS predecessor reported SUCCESS — mergeable, all bots approve, state:needs-human — over a tree whose merge button branch protection had already disabled. #136 again, one field over. Dated by `first` of the preference-ordered stamps rather than `max` of them: start time if the run recorded one, falling back only if it did not. The sentinel filtering is unchanged, and finished runs still date by completion when that is all they carry, so the supersede rule keeps the case it exists for. Found independently by claude-bot-andresmgsl and codex-bot-andresmgsl. No existing fixture could express it — `run_()` carries no startedAt, so every supersede fixture spaced the predecessor's completion safely before the successor's start, the same blind spot as round 3 one field over. New `drained_()` helper pins both directions; fixtures 48 -> 51 (with the reverse-direction in-flight fixture ported from cast#128). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 16:32:53 +00:00
# ...and the reverse direction, which stops "in flight sorts last" being
# widened into "in flight always wins": a run that FINISHED after an earlier
# in-flight entry is the newer word, and the context is settled.
expect "a finished re-run supersedes an earlier in-flight run" SUCCESS \
"$(rollup "[$(inflight_ build 2026-07-20T15:19:00Z),\
$(run_ build SUCCESS 2026-07-20T15:19:45Z)]" | checks_state)"
# -- the wind-down window. A predecessor cancelled by the concurrency group
# does not stop the instant its replacement starts, so its completion
# routinely lands AFTER the successor's start — on box's aa5a6ba the
# replacement started 15:19:38 and the run it cancelled finished 15:19:51.
# Dating by "newest stamp of any kind" compares the dead run's completion
# against the live run's start, which is not an ordering on runs, and the
# predecessor wins. Every fixture above spaces completion before start, so
# none of them can see it. run_() cannot express the overlap either — it
# carries no startedAt — hence the explicit payloads.
overlap_() { jq -n --arg n "$1" --arg o "$2" --arg s "$3" --arg c "$4" \
'{__typename:"CheckRun", workflowName:"ci", name:$n, conclusion:$o,
startedAt:$s, completedAt:$c}'; }
expect "a predecessor finishing after its replacement started is still older (CANCELLED)" PENDING \
"$(rollup "[$(overlap_ scope CANCELLED 2026-07-20T15:19:00Z 2026-07-20T15:19:51Z),\
$(inflight_ scope 2026-07-20T15:19:38Z)]" | checks_state)"
expect "...and the same when it finished green — mid-flight is not mergeable" PENDING \
"$(rollup "[$(overlap_ build SUCCESS 2026-07-20T15:19:00Z 2026-07-20T15:19:51Z),\
fix(labels): order check runs by one consistent quantity — when they began The dating expression took the newest stamp each run carries. That reads `completedAt` for a finished run and `startedAt` for a live one, so the comparison comes down to "when this one ended" against "when that one began" — which is not an ordering on runs at all. A run cancelled by the concurrency group does not stop the instant its replacement starts: the runner has to receive the signal and wind down. So `predecessor.completedAt > successor.startedAt` is the ordinary case, not a corner. On the box#137 tip that motivated the supersede rule the window was 13s wide — the superseding run started 15:19:38, the run it cancelled did not finish until 15:19:51 — and for that whole window the dying predecessor out-dated its own live replacement, so `last` discarded the replacement and judged the corpse. Both round-3 failure modes came back inside that window, narrowed rather than closed: a draining CANCELLED predecessor reported FAILURE and sent the agent to fix nothing, and a draining SUCCESS predecessor reported SUCCESS — mergeable, all bots approve, state:needs-human — over a tree whose merge button branch protection had already disabled. #136 again, one field over. Dated by `first` of the preference-ordered stamps rather than `max` of them: start time if the run recorded one, falling back only if it did not. The sentinel filtering is unchanged, and finished runs still date by completion when that is all they carry, so the supersede rule keeps the case it exists for. Found independently by claude-bot-andresmgsl and codex-bot-andresmgsl. No existing fixture could express it — `run_()` carries no startedAt, so every supersede fixture spaced the predecessor's completion safely before the successor's start, the same blind spot as round 3 one field over. New `drained_()` helper pins both directions; fixtures 48 -> 51 (with the reverse-direction in-flight fixture ported from cast#128). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 16:32:53 +00:00
$(inflight_ build 2026-07-20T15:19:38Z)]" | checks_state)"
fix(labels): date a check run by when it started, not by a zero completion The supersede collapse added in the previous commit dated each run by `.completedAt // .startedAt // .createdAt`. A run still in flight has no completion, but `gh` does not omit the field: its Go struct marshals the zero time as the string "0001-01-01T00:00:00Z", and jq's `//` falls through null and false only. The sentinel was therefore taken as the sort key, and it sorts before every real timestamp — so the LIVE re-run became the oldest entry in its context, `last` discarded it, and the run it superseded was judged instead. That restored #136 through the fix for it: a green context with a replacement mid-flight reported SUCCESS, so a PR read mergeable, green, all bots approve — state:needs-human — while branch protection had the merge button disabled. It also narrowed rather than removed the flap the supersede rule exists to prevent: between "run A cancelled by the concurrency group" and "run B finishes", the PR reported FAILURE and the agent was sent to fix something that was not broken. Runs are now dated by the newest timestamp they actually carry, with both spellings of absent discarded (null, and the zero sentinel). Entries that carry no usable timestamp sort LAST rather than first: something we cannot date is most likely the thing just created, and treating it as newest keeps an undateable in-flight run from being discarded in favour of a stale success. Every ambiguity resolves toward "not settled". Found independently by claude-bot-andresmgsl and codex-bot-andresmgsl. The fixtures could not have caught it: `run_()` always emits a real completedAt, so every supersede fixture was a race between two finished runs, and the bug lived in the one shape the helper could not express. New `inflight_()` helper covers it; fixtures 44 -> 48. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 16:18:50 +00:00
fix(labels): unrecognised check outcomes block, and STALE outranks MISSING Round 2 review found two ways the "a human could merge this right now" invariant still leaked, both of which let state:needs-human land on a PR the button would refuse. The check-rollup classifier enumerated the outcomes that block and defaulted the rest to SUCCESS, so ERROR, CANCELLED and STALE fell through into green. Inverted to an allow-list of the outcomes that DON'T block (SUCCESS, NEUTRAL, SKIPPED, plus the pending set); everything else, including an outcome neither enum has today, blocks. The rollup mixes CheckRun.conclusion with StatusContext.state and an outcome the list forgets is one we cannot certify as mergeable — a false FAILURE parks the PR on the agent, a false SUCCESS invites a bad merge. 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 the jq itself was untestable. Once CANCELLED blocks, superseded runs must be dropped first — a re-run does not evict the run it replaced, and judging every entry would strand every re-run PR in needs-rebase. Each context now collapses to its newest entry, keyed on workflow + job name because a bare job name is only unique within its workflow. 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: a round that was both unfinished and staled came out needs-human with nothing bound to the head. The whole round is now collected before any precedence is applied, STALE ahead of MISSING. The MISSING-yields-to-an-explicit-human-request rule is untouched. Fixtures 29 -> 44, pinning the whole check-outcome enum, the supersede rule in both orders, and the mixed round at both ends of BOTS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 16:05:39 +00:00
# -- 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)"
fix(labels): state:needs-human means a human could merge it right now Ported from heavy-duty/box#137 (heavy-duty/box#136) so the three repos' reconcilers stay byte-identical. The state machine here was byte-identical to box's before this change, and remains so after -- only the scope:* taxonomy differs, correctly. decide_state() derived state from three inputs -- draft flag, requested reviewers, submitted reviews -- and read NOTHING about mergeability or checks. With the `if requested "$HUMAN"` short-circuit at the top of its precedence, the label was sticky: once the maintainer was requested, a PR read state:needs-human through conflicts, through red CI, through a force-push that staled every approval. This repo paid for it directly. During the ten-PR batch merged today, every merge re-conflicted the PRs below it through CHANGELOG.md, and each kept its state:needs-human label throughout -- inviting merges that could not happen. It was caught only by opening them one at a time, which is the work the label exists to save. The rule the label now keeps: state:needs-human means a human could merge this RIGHT NOW, so anything making that false outranks the request that put it there. CONFLICTING or failing checks -> state:needs-rebase (new; the agent's to fix) approvals staled by a push -> state:addressing (nobody reviewed this tree) An UNFINISHED round still yields to an explicit human request -- MISSING (nobody has reviewed yet) is a different fact from STALE (everyone reviewed something else). UNKNOWN mergeability is NOT treated as unmergeable: GitHub reports it for about a minute after every merge, and flapping every open PR through needs-rebase on each merge would be worse than the bug. A failed read degrades to the same "do not know" value. Also adds merge-next: queue order is intent, so the reconciler never sets it, only CLEARS it once the PR stops being mergeable-by-a-human. Fixtures 19 -> 29, including that UNKNOWN does not trigger needs-rebase and a draft outranks a conflict. No live dry-run evidence here -- this repo has no open PRs right now -- so the fixtures and box's live dry-run are the proof. Closes #87 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 15:26:54 +00:00
# -- 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)"
# -- and a draft outranks everything, including a conflict.
DRAFT=true MERGEABLE=CONFLICTING
expect "a draft is building even when conflicted" state:building "$(decide_state)"
DRAFT=false MERGEABLE=MERGEABLE CHECKS=SUCCESS REQUESTED="" REVIEWS_JSON='[]'
printf 'labels-reconcile tests: %d passed, %d failed\n' "$pass" "$fail"
[ "$fail" -eq 0 ]