fix: verdict contract, head-bound approvals, serialized reconcile — and a testable state machine
Round-1 blockers, all three reviewers concurring:
- COMMENTED agreement now counts: agreement_signal recognizes the live bots'
durable markers (Verdict: Approve / I agree with everything / leading ✅) —
the gate to needs-human can actually close. Formal verdicts remain the
contract (CONTRIBUTING), this is the documented transitional workaround.
- Every counting verdict is bound to the head SHA; a stale approval parks the
PR in addressing (agent owes re-request) instead of promoting unreviewed
code. CHANGES_REQUESTED blocks at any head, per GitHub's own semantic.
- reconcile serializes under ONE job-level concurrency group; scope stays
per-PR. No more cron-vs-event race on the request-the-human-once guard.
- Sweep resilience: per-PR subshell (one failure logs and continues), label
edits warn instead of wedging; the self-heal claim now matches reality
(dispatch-only bootstrap).
- The state machine is extracted pure (globals in, state out) and sourceable:
test/labels-reconcile.sh proves 14 fixture transitions — comment-only
agreement, stale approval, comment-without-verdict, human precedence and
human-block — wired into CI.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-18 19:31:04 +00:00
|
|
|
#!/usr/bin/env bash
|
|
|
|
|
set -euo pipefail
|
|
|
|
|
|
2026-07-18 19:51:55 +00:00
|
|
|
# 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.
|
fix: verdict contract, head-bound approvals, serialized reconcile — and a testable state machine
Round-1 blockers, all three reviewers concurring:
- COMMENTED agreement now counts: agreement_signal recognizes the live bots'
durable markers (Verdict: Approve / I agree with everything / leading ✅) —
the gate to needs-human can actually close. Formal verdicts remain the
contract (CONTRIBUTING), this is the documented transitional workaround.
- Every counting verdict is bound to the head SHA; a stale approval parks the
PR in addressing (agent owes re-request) instead of promoting unreviewed
code. CHANGES_REQUESTED blocks at any head, per GitHub's own semantic.
- reconcile serializes under ONE job-level concurrency group; scope stays
per-PR. No more cron-vs-event race on the request-the-human-once guard.
- Sweep resilience: per-PR subshell (one failure logs and continues), label
edits warn instead of wedging; the self-heal claim now matches reality
(dispatch-only bootstrap).
- The state machine is extracted pure (globals in, state out) and sourceable:
test/labels-reconcile.sh proves 14 fixture transitions — comment-only
agreement, stale approval, comment-without-verdict, human precedence and
human-block — wired into CI.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-18 19:31:04 +00:00
|
|
|
# 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)"
|
|
|
|
|
|
2026-07-18 19:51:55 +00:00
|
|
|
# -- a comment is a non-verdict, agreement body or not: the author escalates --
|
fix: verdict contract, head-bound approvals, serialized reconcile — and a testable state machine
Round-1 blockers, all three reviewers concurring:
- COMMENTED agreement now counts: agreement_signal recognizes the live bots'
durable markers (Verdict: Approve / I agree with everything / leading ✅) —
the gate to needs-human can actually close. Formal verdicts remain the
contract (CONTRIBUTING), this is the documented transitional workaround.
- Every counting verdict is bound to the head SHA; a stale approval parks the
PR in addressing (agent owes re-request) instead of promoting unreviewed
code. CHANGES_REQUESTED blocks at any head, per GitHub's own semantic.
- reconcile serializes under ONE job-level concurrency group; scope stays
per-PR. No more cron-vs-event race on the request-the-human-once guard.
- Sweep resilience: per-PR subshell (one failure logs and continues), label
edits warn instead of wedging; the self-heal claim now matches reality
(dispatch-only bootstrap).
- The state machine is extracted pure (globals in, state out) and sourceable:
test/labels-reconcile.sh proves 14 fixture transitions — comment-only
agreement, stale approval, comment-without-verdict, human precedence and
human-block — wired into CI.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-18 19:31:04 +00:00
|
|
|
REVIEWS_JSON="$(reviews \
|
|
|
|
|
"$(rev "$BOT1" COMMENTED head1 "✅ **Reviewed — I agree with everything.**" t1)" \
|
2026-07-18 19:51:55 +00:00
|
|
|
"$(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)"
|
fix: verdict contract, head-bound approvals, serialized reconcile — and a testable state machine
Round-1 blockers, all three reviewers concurring:
- COMMENTED agreement now counts: agreement_signal recognizes the live bots'
durable markers (Verdict: Approve / I agree with everything / leading ✅) —
the gate to needs-human can actually close. Formal verdicts remain the
contract (CONTRIBUTING), this is the documented transitional workaround.
- Every counting verdict is bound to the head SHA; a stale approval parks the
PR in addressing (agent owes re-request) instead of promoting unreviewed
code. CHANGES_REQUESTED blocks at any head, per GitHub's own semantic.
- reconcile serializes under ONE job-level concurrency group; scope stays
per-PR. No more cron-vs-event race on the request-the-human-once guard.
- Sweep resilience: per-PR subshell (one failure logs and continues), label
edits warn instead of wedging; the self-heal claim now matches reality
(dispatch-only bootstrap).
- The state machine is extracted pure (globals in, state out) and sourceable:
test/labels-reconcile.sh proves 14 fixture transitions — comment-only
agreement, stale approval, comment-without-verdict, human precedence and
human-block — wired into CI.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-18 19:31:04 +00:00
|
|
|
|
|
|
|
|
# -- 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)" \
|
2026-07-18 19:51:55 +00:00
|
|
|
"$(rev "$BOT3" APPROVED head1 "" t4)")"
|
fix: verdict contract, head-bound approvals, serialized reconcile — and a testable state machine
Round-1 blockers, all three reviewers concurring:
- COMMENTED agreement now counts: agreement_signal recognizes the live bots'
durable markers (Verdict: Approve / I agree with everything / leading ✅) —
the gate to needs-human can actually close. Formal verdicts remain the
contract (CONTRIBUTING), this is the documented transitional workaround.
- Every counting verdict is bound to the head SHA; a stale approval parks the
PR in addressing (agent owes re-request) instead of promoting unreviewed
code. CHANGES_REQUESTED blocks at any head, per GitHub's own semantic.
- reconcile serializes under ONE job-level concurrency group; scope stays
per-PR. No more cron-vs-event race on the request-the-human-once guard.
- Sweep resilience: per-PR subshell (one failure logs and continues), label
edits warn instead of wedging; the self-heal claim now matches reality
(dispatch-only bootstrap).
- The state machine is extracted pure (globals in, state out) and sourceable:
test/labels-reconcile.sh proves 14 fixture transitions — comment-only
agreement, stale approval, comment-without-verdict, human precedence and
human-block — wired into CI.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-18 19:31:04 +00:00
|
|
|
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=""
|
|
|
|
|
|
2026-07-18 20:16:59 +00:00
|
|
|
# -- 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.
In this repo the SECOND half is the live one: three PRs sit at
state:needs-human simultaneously with nothing saying which to merge first, and
they will conflict through CHANGELOG.md the moment one lands. The stickiness
has not bitten here yet only because nothing has conflicted -- the code carried
it identically, so the first merge would have reproduced box's situation.
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 -- the label this repo needs most today, since a 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, only CLEARS it.
Fixtures 19 -> 29. DRY_RUN against this repo changes NOTHING, which is the
correct result: every open PR here is currently mergeable, so the new
precedence is a no-op on a healthy board and fires only when something is
actually wrong. npm test 623 passed.
Closes #127
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 15:28:50 +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): 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>
2026-07-20 16:05:17 +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.
In this repo the SECOND half is the live one: three PRs sit at
state:needs-human simultaneously with nothing saying which to merge first, and
they will conflict through CHANGELOG.md the moment one lands. The stickiness
has not bitten here yet only because nothing has conflicted -- the code carried
it identically, so the first merge would have reproduced box's situation.
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 -- the label this repo needs most today, since a 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, only CLEARS it.
Fixtures 19 -> 29. DRY_RUN against this repo changes NOTHING, which is the
correct result: every open PR here is currently mergeable, so the new
precedence is a no-op on a healthy board and fires only when something is
actually wrong. npm test 623 passed.
Closes #127
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 15:28:50 +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): 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>
2026-07-20 16:05:17 +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 when it finished
Round 3 of #128. @claude-bot-andresmgsl and @codex-bot-andresmgsl
independently found the same regression in the round-2 supersede rule.
The collapse-to-newest step 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 `//` only falls
through on null/false. So the sentinel was taken as the sort key and
sorted below every real timestamp: the live re-run went to the bottom of
its context and `last` discarded it, judging the very run it superseded.
That inverted the rule in both directions. A green context with a
replacement mid-flight read SUCCESS — #136 restored, needs-human pointing
a human at a disabled merge button — and a CANCELLED original whose
replacement was still running read FAILURE, the flap the collapse was
added to prevent.
A run is now dated by the newest timestamp it actually carries, with both
spellings of absent discarded (null, and the zero sentinel), rather than
by assuming which field is populated. An entry carrying no usable
timestamp sorts last rather than first: something undateable is most
likely the thing just created, so ambiguity resolves toward "not settled"
instead of toward a stale success.
Fixtures 44 -> 48. The gap was structural — the existing run_() helper
always sets a real completedAt, so every supersede fixture raced two
finished runs and none could express an in-flight one.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 16:19:57 +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 —
|
2026-07-20 16:23:09 +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 when it finished
Round 3 of #128. @claude-bot-andresmgsl and @codex-bot-andresmgsl
independently found the same regression in the round-2 supersede rule.
The collapse-to-newest step 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 `//` only falls
through on null/false. So the sentinel was taken as the sort key and
sorted below every real timestamp: the live re-run went to the bottom of
its context and `last` discarded it, judging the very run it superseded.
That inverted the rule in both directions. A green context with a
replacement mid-flight read SUCCESS — #136 restored, needs-human pointing
a human at a disabled merge button — and a CANCELLED original whose
replacement was still running read FAILURE, the flap the collapse was
added to prevent.
A run is now dated by the newest timestamp it actually carries, with both
spellings of absent discarded (null, and the zero sentinel), rather than
by assuming which field is populated. An entry carrying no usable
timestamp sorts last rather than first: something undateable is most
likely the thing just created, so ambiguity resolves toward "not settled"
instead of toward a stale success.
Fixtures 44 -> 48. The gap was structural — the existing run_() helper
always sets a real completedAt, so every supersede fixture raced two
finished runs and none could express an in-flight one.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 16:19:57 +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)"
|
2026-07-20 16:23:09 +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)"
|
fix(labels): date a check run by when it started, not when it finished
Round 3 of #128. @claude-bot-andresmgsl and @codex-bot-andresmgsl
independently found the same regression in the round-2 supersede rule.
The collapse-to-newest step 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 `//` only falls
through on null/false. So the sentinel was taken as the sort key and
sorted below every real timestamp: the live re-run went to the bottom of
its context and `last` discarded it, judging the very run it superseded.
That inverted the rule in both directions. A green context with a
replacement mid-flight read SUCCESS — #136 restored, needs-human pointing
a human at a disabled merge button — and a CANCELLED original whose
replacement was still running read FAILURE, the flap the collapse was
added to prevent.
A run is now dated by the newest timestamp it actually carries, with both
spellings of absent discarded (null, and the zero sentinel), rather than
by assuming which field is populated. An entry carrying no usable
timestamp sorts last rather than first: something undateable is most
likely the thing just created, so ambiguity resolves toward "not settled"
instead of toward a stale success.
Fixtures 44 -> 48. The gap was structural — the existing run_() helper
always sets a real completedAt, so every supersede fixture raced two
finished runs and none could express an in-flight one.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 16:19:57 +00:00
|
|
|
|
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>
2026-07-20 16:05:17 +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.
In this repo the SECOND half is the live one: three PRs sit at
state:needs-human simultaneously with nothing saying which to merge first, and
they will conflict through CHANGELOG.md the moment one lands. The stickiness
has not bitten here yet only because nothing has conflicted -- the code carried
it identically, so the first merge would have reproduced box's situation.
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 -- the label this repo needs most today, since a 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, only CLEARS it.
Fixtures 19 -> 29. DRY_RUN against this repo changes NOTHING, which is the
correct result: every open PR here is currently mergeable, so the new
precedence is a no-op on a healthy board and fires only when something is
actually wrong. npm test 623 passed.
Closes #127
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-20 15:28:50 +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='[]'
|
|
|
|
|
|
fix: verdict contract, head-bound approvals, serialized reconcile — and a testable state machine
Round-1 blockers, all three reviewers concurring:
- COMMENTED agreement now counts: agreement_signal recognizes the live bots'
durable markers (Verdict: Approve / I agree with everything / leading ✅) —
the gate to needs-human can actually close. Formal verdicts remain the
contract (CONTRIBUTING), this is the documented transitional workaround.
- Every counting verdict is bound to the head SHA; a stale approval parks the
PR in addressing (agent owes re-request) instead of promoting unreviewed
code. CHANGES_REQUESTED blocks at any head, per GitHub's own semantic.
- reconcile serializes under ONE job-level concurrency group; scope stays
per-PR. No more cron-vs-event race on the request-the-human-once guard.
- Sweep resilience: per-PR subshell (one failure logs and continues), label
edits warn instead of wedging; the self-heal claim now matches reality
(dispatch-only bootstrap).
- The state machine is extracted pure (globals in, state out) and sourceable:
test/labels-reconcile.sh proves 14 fixture transitions — comment-only
agreement, stale approval, comment-without-verdict, human precedence and
human-block — wired into CI.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-18 19:31:04 +00:00
|
|
|
printf 'labels-reconcile tests: %d passed, %d failed\n' "$pass" "$fail"
|
|
|
|
|
[ "$fail" -eq 0 ]
|