refactor(labels): split PR labels into state (whose ball) and blocker (what is in the way)
Retires state:needs-rebase. Mergeability, check status and the review round are independent facts; forcing them through one totally-ordered label meant one always won and the rest vanished, which is where every precedence bug in this machine has lived. needs-rebase was the clearest casualty -- it fired on both a conflict and a failing check, which need opposite work. Blockers are a set, so there is no ordering between them to get wrong. The ordered axis is now purely about reviews. bots-reviewing also tightens to mean a request is actually live; a PR nobody was asked to review is the agent's. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
e408b7a0ed
commit
2d49a5ad42
4 changed files with 214 additions and 50 deletions
112
.github/scripts/labels-reconcile.sh
vendored
112
.github/scripts/labels-reconcile.sh
vendored
|
|
@ -31,7 +31,11 @@ set -euo pipefail
|
||||||
|
|
||||||
HUMAN="${HUMAN_REVIEWER:-danmt}"
|
HUMAN="${HUMAN_REVIEWER:-danmt}"
|
||||||
BOTS=(claude-bot-andresmgsl codex-bot-andresmgsl grok-bot-andresmgsl)
|
BOTS=(claude-bot-andresmgsl codex-bot-andresmgsl grok-bot-andresmgsl)
|
||||||
STATES=(state:building state:needs-rebase state:bots-reviewing state:addressing state:needs-human)
|
STATES=(state:building state:bots-reviewing state:addressing state:needs-human)
|
||||||
|
BLOCKERS=(blocker:conflict blocker:ci-red blocker:unrequested)
|
||||||
|
# Labels this machine used to own and no longer does. Cleared on sight so a
|
||||||
|
# retirement heals the board instead of stranding a label nothing recomputes.
|
||||||
|
RETIRED=(state:needs-rebase)
|
||||||
STALE_AFTER=$((48 * 3600))
|
STALE_AFTER=$((48 * 3600))
|
||||||
|
|
||||||
log() { printf 'labels: %s\n' "$*"; }
|
log() { printf 'labels: %s\n' "$*"; }
|
||||||
|
|
@ -153,25 +157,61 @@ human_request_needed() { # 0 when needs-human requires a FRESH human request
|
||||||
return 0
|
return 0
|
||||||
}
|
}
|
||||||
|
|
||||||
|
blockers() { # → the blocker:* labels this PR should carry, one per line
|
||||||
|
# The second axis. These are FACTS ABOUT THE BRANCH, and they are mutually
|
||||||
|
# independent — a PR can be conflicted and red and unasked at once — so they
|
||||||
|
# are a set, not an ordering. That is the whole point of splitting them out
|
||||||
|
# of state:*: every precedence bug this machine has had (needs-human
|
||||||
|
# surviving a conflict, MISSING swallowing STALE) came from projecting
|
||||||
|
# independent facts onto one totally-ordered label. A set has no precedence
|
||||||
|
# to get wrong.
|
||||||
|
#
|
||||||
|
# UNKNOWN mergeability is deliberately NOT a conflict: GitHub reports it for
|
||||||
|
# about a minute after every merge while it recomputes, and flapping every
|
||||||
|
# open PR on each merge would be worse than the bug. Same for a failed read
|
||||||
|
# of either fact — both default to the "do not know" value, which blocks
|
||||||
|
# nothing. An unset global (an older fixture, a failed fetch) must never
|
||||||
|
# invent a verdict it did not read.
|
||||||
|
case "${MERGEABLE:-UNKNOWN}" in CONFLICTING) echo blocker:conflict ;; esac
|
||||||
|
case "${CHECKS:-NONE}" in FAILURE) echo blocker:ci-red ;; esac
|
||||||
|
|
||||||
|
# Nobody is on the hook for a verdict somebody still owes. Distinct from
|
||||||
|
# bots-reviewing, which says a request is live and an answer is coming:
|
||||||
|
# here the round is stalled because no one was ever asked, and the board
|
||||||
|
# said "waiting on the bots" for the 48h it took `stale` to notice.
|
||||||
|
# A draft is exempt (the bots ignore drafts by design), and so is an
|
||||||
|
# explicit human request — a maintainer claiming a PR early is deliberate,
|
||||||
|
# not a dropped ball.
|
||||||
|
if [ "$DRAFT" != true ] && ! requested "$HUMAN"; then
|
||||||
|
local b any_missing=false any_requested=false
|
||||||
|
for b in "${BOTS[@]}"; do
|
||||||
|
requested "$b" && any_requested=true
|
||||||
|
[ "$(bot_verdict "$b")" = MISSING ] && any_missing=true
|
||||||
|
done
|
||||||
|
if [ "$any_missing" = true ] && [ "$any_requested" = false ]; then
|
||||||
|
echo blocker:unrequested
|
||||||
|
fi
|
||||||
|
fi
|
||||||
|
}
|
||||||
|
|
||||||
decide_state() { # → the one state:* label this PR should carry
|
decide_state() { # → the one state:* label this PR should carry
|
||||||
if [ "$DRAFT" = true ]; then echo state:building; return; fi
|
if [ "$DRAFT" = true ]; then echo state:building; return; fi
|
||||||
|
|
||||||
# state:needs-human means ONE thing: a human could merge this right now.
|
local s
|
||||||
# Anything that makes that false outranks the request that put it there —
|
s="$(round_state)"
|
||||||
# otherwise the board invites a merge that cannot or must not happen, and
|
|
||||||
# nothing else on the page contradicts it (#136).
|
|
||||||
#
|
|
||||||
# A conflicted or red branch is the agent's to fix, not the human's to
|
|
||||||
# merge. UNKNOWN is deliberately NOT treated as unmergeable: GitHub reports
|
|
||||||
# it for a minute after every merge while it recomputes, and flapping every
|
|
||||||
# open PR through needs-rebase on each merge would be worse than the bug.
|
|
||||||
# An unknown mergeability simply does not trigger this arm; the next sweep
|
|
||||||
# sees the settled value.
|
|
||||||
# Both default to the "do not know" value: an unset global (older fixture,
|
|
||||||
# a failed fetch) must never invent a verdict it did not read.
|
|
||||||
case "${MERGEABLE:-UNKNOWN}" in CONFLICTING) echo state:needs-rebase; return ;; esac
|
|
||||||
case "${CHECKS:-NONE}" in FAILURE) echo state:needs-rebase; return ;; esac
|
|
||||||
|
|
||||||
|
# The one rule joining the two axes: state:needs-human means a human could
|
||||||
|
# merge this RIGHT NOW, so it requires a clear branch. Any blocker at all
|
||||||
|
# means the work is the agent's — whatever the review round says — and the
|
||||||
|
# blocker label says which work it is. Nothing else in this function reads
|
||||||
|
# the branch, which is what keeps the ordering below purely about reviews.
|
||||||
|
if [ "$s" = state:needs-human ] && [ -n "$(blockers)" ]; then
|
||||||
|
echo state:addressing; return
|
||||||
|
fi
|
||||||
|
echo "$s"
|
||||||
|
}
|
||||||
|
|
||||||
|
round_state() { # → the state the REVIEW ROUND alone implies; knows no branch facts
|
||||||
local b verdicts=""
|
local b verdicts=""
|
||||||
for b in "${BOTS[@]}"; do
|
for b in "${BOTS[@]}"; do
|
||||||
if requested "$b"; then echo state:bots-reviewing; return; fi
|
if requested "$b"; then echo state:bots-reviewing; return; fi
|
||||||
|
|
@ -199,9 +239,16 @@ decide_state() { # → the one state:* label this PR should carry
|
||||||
# No verdict at all from some bot, and nothing staled. An explicit human
|
# No verdict at all from some bot, and nothing staled. An explicit human
|
||||||
# request still outranks an unfinished round — a maintainer pulling a PR
|
# request still outranks an unfinished round — a maintainer pulling a PR
|
||||||
# to themselves early is a deliberate act, and the original precedence.
|
# to themselves early is a deliberate act, and the original precedence.
|
||||||
|
#
|
||||||
|
# Otherwise it is the AGENT's ball, not the bots'. The loop above already
|
||||||
|
# returned for every live bot request, so reaching here with a MISSING
|
||||||
|
# means somebody owes a verdict and nobody was asked for one — the round
|
||||||
|
# is not running. Calling that bots-reviewing was the lie that let a
|
||||||
|
# forgotten PR read "waiting on the reviewers" for the 48h it took the
|
||||||
|
# stale sweep to notice. blocker:unrequested says why.
|
||||||
*MISSING*)
|
*MISSING*)
|
||||||
if requested "$HUMAN"; then echo state:needs-human; return; fi
|
if requested "$HUMAN"; then echo state:needs-human; return; fi
|
||||||
echo state:bots-reviewing; return ;;
|
echo state:addressing; return ;;
|
||||||
esac
|
esac
|
||||||
# an explicit human request outranks the remaining bot outcomes — it is the
|
# 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
|
# final gate, and a maintainer pulling a PR to themselves early counts too
|
||||||
|
|
@ -233,8 +280,10 @@ bootstrap_labels() { # dispatch-only: ~20 upserts is too chatty for every cron t
|
||||||
state:building|FBCA04|PR is a draft — the coding agent is still building
|
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: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: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|No blockers, all bots approve — waiting on the human reviewer
|
||||||
state:needs-human|8250DF|Mergeable, green, all bots approve — waiting on the human reviewer
|
blocker:conflict|B60205|Does not merge — the branch conflicts and the agent owes a rebase
|
||||||
|
blocker:ci-red|B60205|A check is failing — the agent owes a fix (not a rebase)
|
||||||
|
blocker:unrequested|E99695|Somebody still owes a verdict and nobody was asked for one
|
||||||
merge-next|0E8A16|Head of the merge queue — merge this one next (set by hand/agent, cleared here)
|
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)
|
stale|B60205|No activity for 48h — needs a poke (sweep-managed)
|
||||||
blocked|6A737D|Waiting on another PR or issue to land first
|
blocked|6A737D|Waiting on another PR or issue to land first
|
||||||
|
|
@ -267,17 +316,34 @@ reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch
|
||||||
log "#$n: requested $HUMAN (round passed)"
|
log "#$n: requested $HUMAN (round passed)"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
# ---- converge the state:* labels ----
|
# ---- converge both axes ----
|
||||||
|
# state:* is exclusive (everything but $desired comes off); blocker:* is a
|
||||||
|
# set (each one on or off on its own); RETIRED always comes off. One edit
|
||||||
|
# call for all of it, so a PR never flickers through a half-applied board.
|
||||||
|
local want_blockers add=""
|
||||||
|
want_blockers="$(blockers)"
|
||||||
|
|
||||||
remove=""
|
remove=""
|
||||||
for s in "${STATES[@]}"; do
|
for s in "${STATES[@]}"; do
|
||||||
if [ "$s" != "$desired" ] && has_label "$s"; then remove="$remove,$s"; fi
|
if [ "$s" != "$desired" ] && has_label "$s"; then remove="$remove,$s"; fi
|
||||||
done
|
done
|
||||||
|
for s in "${RETIRED[@]}"; do
|
||||||
|
if has_label "$s"; then remove="$remove,$s"; fi
|
||||||
|
done
|
||||||
|
for s in "${BLOCKERS[@]}"; do
|
||||||
|
if grep -qxF "$s" <<<"$want_blockers"; then
|
||||||
|
has_label "$s" || add="$add,$s"
|
||||||
|
else
|
||||||
|
has_label "$s" && remove="$remove,$s"
|
||||||
|
fi
|
||||||
|
done
|
||||||
|
add="${add#,}"
|
||||||
remove="${remove#,}"
|
remove="${remove#,}"
|
||||||
if ! has_label "$desired" || [ -n "$remove" ]; then
|
if ! has_label "$desired" || [ -n "$remove" ] || [ -n "$add" ]; then
|
||||||
args=(--add-label "$desired")
|
args=(--add-label "$desired${add:+,$add}")
|
||||||
[ -n "$remove" ] && args+=(--remove-label "$remove")
|
[ -n "$remove" ] && args+=(--remove-label "$remove")
|
||||||
if run gh issue edit "$n" -R "$REPO" "${args[@]}" >/dev/null; then
|
if run gh issue edit "$n" -R "$REPO" "${args[@]}" >/dev/null; then
|
||||||
log "#$n: state -> $desired${remove:+ (cleared $remove)}"
|
log "#$n: state -> $desired${add:+ +$add}${remove:+ (cleared $remove)}"
|
||||||
else
|
else
|
||||||
# a deleted label must not wedge the sweep — dispatch heals the taxonomy
|
# a deleted label must not wedge the sweep — dispatch heals the taxonomy
|
||||||
log "#$n: WARNING: label edit failed (missing label? run the workflow manually to bootstrap)"
|
log "#$n: WARNING: label edit failed (missing label? run the workflow manually to bootstrap)"
|
||||||
|
|
|
||||||
28
CHANGELOG.md
28
CHANGELOG.md
|
|
@ -7,6 +7,34 @@ which records not just what changed but what each drill run proved.
|
||||||
|
|
||||||
### Changed
|
### Changed
|
||||||
|
|
||||||
|
- **PR labels split into two axes: `state:*` (whose ball) and `blocker:*`
|
||||||
|
(what is in the way)** — `state:needs-rebase` is retired, replaced by
|
||||||
|
`blocker:conflict`, `blocker:ci-red` and `blocker:unrequested`. One rule
|
||||||
|
joins them: `state:needs-human` requires zero blockers.
|
||||||
|
|
||||||
|
The single-label design forced independent facts through one totally-ordered
|
||||||
|
value, and the ordering was where every bug lived. Mergeability, check
|
||||||
|
status and the review round move independently — a PR can be conflicted
|
||||||
|
*and* red *and* stalled at once — so a total order has to pick a winner and
|
||||||
|
silently drop the rest. `state:needs-rebase` was the clearest casualty: it
|
||||||
|
fired on both a conflict and a failing check, which need opposite work, and
|
||||||
|
told an agent to rebase when what it owed was a bug fix. On this repo's own
|
||||||
|
board, #120 was conflicted **and** red and could only say one of them.
|
||||||
|
|
||||||
|
Blockers are a set, so there is no precedence between them to get wrong.
|
||||||
|
What is left on the ordered axis is purely about reviews, which is the one
|
||||||
|
place an ordering is genuinely meaningful.
|
||||||
|
|
||||||
|
`state:bots-reviewing` also tightens to mean strictly *a request is live and
|
||||||
|
an answer is coming*. A ready PR nobody was asked to review read "waiting on
|
||||||
|
the reviewers" for the 48 hours it took the stale sweep to notice; it is now
|
||||||
|
`state:addressing` + `blocker:unrequested`, because the agent owes the ask.
|
||||||
|
Drafts are exempt (the bots ignore drafts by design), as is an explicit
|
||||||
|
human request — a maintainer claiming a PR early is deliberate.
|
||||||
|
|
||||||
|
The reconciler strips `state:needs-rebase` on sight, so the retirement heals
|
||||||
|
the board rather than stranding a label nothing recomputes. Fixtures 51 → 64.
|
||||||
|
|
||||||
- **The tenant templates carry rig's family suffix: `claude` → `claude-box`,
|
- **The tenant templates carry rig's family suffix: `claude` → `claude-box`,
|
||||||
`codex` → `codex-box`, `grok` → `grok-box`, `staging` → `staging-box`**
|
`codex` → `codex-box`, `grok` → `grok-box`, `staging` → `staging-box`**
|
||||||
(#123, following heavy-duty/rig#76) — rig is growing a second family of
|
(#123, following heavy-duty/rig#76) — rig is growing a second family of
|
||||||
|
|
|
||||||
62
LABELS.md
62
LABELS.md
|
|
@ -16,33 +16,61 @@ single reply, and a human takes the final review.
|
||||||
|---|---|---|---|---|
|
|---|---|---|---|---|
|
||||||
| `state:building` | `#FBCA04` | the coding agent, still building | PR opened as draft | marked ready + bot reviews requested |
|
| `state:building` | `#FBCA04` | the coding agent, still building | PR opened as draft | marked ready + bot reviews requested |
|
||||||
| `state:bots-reviewing` | `#1D76DB` | the reviewer bots to finish the round | ready with reviews requested, or fixes pushed and reviews re-requested | all three bots have reviewed the round |
|
| `state:bots-reviewing` | `#1D76DB` | the reviewer bots to finish the round | ready with reviews requested, or fixes pushed and reviews re-requested | all three bots have reviewed the round |
|
||||||
| `state:addressing` | `#D93F0B` | the coding agent to reply and push fixes | all bots reviewed the round, not all approved | the single round-reply is posted and fixes pushed |
|
| `state:addressing` | `#D93F0B` | the coding agent to reply, fix, or ask | all bots reviewed and not all approved; or nobody was asked; or a blocker is up | the thing the blocker names is done |
|
||||||
| `state:needs-rebase` | `#B60205` | the coding agent to rebase or fix | the branch does not merge — GitHub says `CONFLICTING`, or a check has failed | it merges cleanly and checks are green again |
|
| `state:needs-human` | `#8250DF` | the human reviewer | the PR **could be merged right now**: no blockers, three formal head-current approvals — and the human review is requested | merged — or changes requested, which cycles back to `state:addressing` |
|
||||||
| `state:needs-human` | `#8250DF` | the human reviewer | the PR **could be merged right now**: mergeable, checks not failing, three formal head-current approvals — and the human review is requested | merged — or changes requested, which cycles back to `state:addressing` |
|
|
||||||
|
|
||||||
`bots-reviewing` and `addressing` are deliberately distinct: staleness in the
|
`bots-reviewing` and `addressing` are deliberately distinct: staleness in the
|
||||||
first means *poke the bots*, staleness in the second means *the agent dropped
|
first means *poke the bots*, staleness in the second means *the agent dropped
|
||||||
the ball*. Collapsing them loses exactly the information a sweep needs.
|
the ball*. Collapsing them loses exactly the information a sweep needs.
|
||||||
|
`bots-reviewing` therefore means strictly *a request is live and an answer is
|
||||||
|
coming* — a PR nobody was asked to review is the agent's ball, not the bots'.
|
||||||
|
|
||||||
|
## The second axis: `blocker:*`
|
||||||
|
|
||||||
|
State answers *whose ball is it*. Blockers answer *what is in the way*, and
|
||||||
|
unlike states they are *facts about the branch* — mutually independent, so a
|
||||||
|
PR carries as many as apply.
|
||||||
|
|
||||||
|
| Label | Color | Means | Clears when |
|
||||||
|
|---|---|---|---|
|
||||||
|
| `blocker:conflict` | `#B60205` | GitHub says `CONFLICTING` — the agent owes a **rebase** | it merges cleanly |
|
||||||
|
| `blocker:ci-red` | `#B60205` | a check failed — the agent owes a **fix**, which a rebase will not provide | checks are green |
|
||||||
|
| `blocker:unrequested` | `#E99695` | somebody still owes a verdict and **nobody was asked** for one | reviews are requested |
|
||||||
|
|
||||||
|
One rule joins the axes: **`state:needs-human` requires zero blockers.** Any
|
||||||
|
blocker means the work is the agent's, whatever the review round says.
|
||||||
|
|
||||||
|
This split exists because the single-label version kept lying. Independent
|
||||||
|
facts were projected onto one totally-ordered label, so one always had to win
|
||||||
|
and the losers vanished off the board: a PR that was *both* conflicted and red
|
||||||
|
could only say one of them, and `needs-rebase` told an agent to rebase when
|
||||||
|
what it actually owed was a bug fix. Precedence between two blockers is not a
|
||||||
|
question a set has to answer, which is why every ordering bug this machine has
|
||||||
|
had — `needs-human` surviving a conflict, `MISSING` swallowing `STALE` — lived
|
||||||
|
on the axis that had to be totally ordered.
|
||||||
|
|
||||||
|
`state:needs-rebase` was the first attempt at this and is **retired**; the
|
||||||
|
reconciler strips it on sight so no PR is left carrying a label nothing
|
||||||
|
recomputes.
|
||||||
|
|
||||||
**`state:needs-human` means one thing: a human could merge this right now.**
|
**`state:needs-human` means one thing: a human could merge this right now.**
|
||||||
Anything that makes that false outranks the review request that put it there,
|
The label is the only signal a maintainer scanning the board (or a phone)
|
||||||
because the label is the only signal a maintainer scanning the board (or a
|
actually reads, and one that says "your turn" on an unmergeable PR is worse
|
||||||
phone) actually reads — and a label that says "your turn" on an unmergeable PR
|
than no label at all. So beyond the blockers, one review fact also outranks an
|
||||||
is worse than no label at all. Two things therefore take precedence over an
|
|
||||||
explicit human request:
|
explicit human request:
|
||||||
|
|
||||||
- **it does not merge** — `CONFLICTING`, or a failing check → `state:needs-rebase`
|
|
||||||
- **nobody reviewed *this* head** — every approval staled by a push → `state:addressing`,
|
- **nobody reviewed *this* head** — every approval staled by a push → `state:addressing`,
|
||||||
because the agent owes a re-request
|
because the agent owes a re-request
|
||||||
|
|
||||||
The second is the more dangerous of the two: with a conflict, GitHub at least
|
That case is more dangerous than any blocker: a blocked PR at least shows an X
|
||||||
disables the merge button, while a staled-approval PR reads green, mergeable
|
or a disabled merge button, while a staled-approval PR reads green, mergeable
|
||||||
and "waiting on the human" over code no reviewer has seen.
|
and "waiting on the human" over code no reviewer has seen.
|
||||||
|
|
||||||
`UNKNOWN` mergeability is deliberately **not** treated as unmergeable. GitHub
|
`UNKNOWN` mergeability is deliberately **not** treated as a conflict. GitHub
|
||||||
reports it for about a minute after every merge while it recomputes, and
|
reports it for about a minute after every merge while it recomputes, and
|
||||||
flapping every open PR through `needs-rebase` on each merge would be worse than
|
flapping every open PR through `blocker:conflict` on each merge would be worse
|
||||||
the bug this precedence fixes.
|
than the bug this fixes. A failed read of either branch fact degrades to the
|
||||||
|
same "do not know" value, for the same reason.
|
||||||
|
|
||||||
An *unfinished* round still yields to an explicit human request — a maintainer
|
An *unfinished* round still yields to an explicit human request — a maintainer
|
||||||
pulling a PR to themselves early is a deliberate act. `MISSING` (nobody has
|
pulling a PR to themselves early is a deliberate act. `MISSING` (nobody has
|
||||||
|
|
@ -96,8 +124,12 @@ missing label idempotently. To create them by hand (needs push access):
|
||||||
gh label create "state:building" --color FBCA04 --description "PR is a draft — the coding agent is still building" --force
|
gh label create "state:building" --color FBCA04 --description "PR is a draft — the coding agent is still building" --force
|
||||||
gh label create "state:bots-reviewing" --color 1D76DB --description "Waiting on the bot reviewers to finish the round" --force
|
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: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 "blocker:conflict" --color B60205 --description "Does not merge — the branch conflicts and the agent owes a rebase" --force
|
||||||
gh label create "state:needs-human" --color 8250DF --description "Mergeable, green, all bots approve — waiting on the human reviewer" --force
|
gh label create "blocker:ci-red" --color B60205 --description "A check is failing — the agent owes a fix (not a rebase)" --force
|
||||||
|
gh label create "blocker:unrequested" --color E99695 --description "Somebody still owes a verdict and nobody was asked for one" --force
|
||||||
|
# retired — the reconciler strips it; delete it once no PR carries it
|
||||||
|
# gh label delete "state:needs-rebase"
|
||||||
|
gh label create "state:needs-human" --color 8250DF --description "No blockers, 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 "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 "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
|
gh label create "blocked" --color 6A737D --description "Waiting on another PR or issue to land first" --force
|
||||||
|
|
|
||||||
|
|
@ -44,10 +44,15 @@ $BOT3" REVIEWS_JSON='[]'
|
||||||
expect "requested bots mean bots-reviewing" state:bots-reviewing "$(decide_state)"
|
expect "requested bots mean bots-reviewing" state:bots-reviewing "$(decide_state)"
|
||||||
|
|
||||||
# -- a bot that never reviewed keeps the round open ---------------------------
|
# -- a bot that never reviewed keeps the round open ---------------------------
|
||||||
REQUESTED="" REVIEWS_JSON="$(reviews \
|
# With a live request that is the bots' ball; with NO request outstanding it
|
||||||
|
# is the agent's, because nothing is coming until somebody asks.
|
||||||
|
REQUESTED="$BOT3" REVIEWS_JSON="$(reviews \
|
||||||
"$(rev "$BOT1" APPROVED head1 "" t1)" \
|
"$(rev "$BOT1" APPROVED head1 "" t1)" \
|
||||||
"$(rev "$BOT2" APPROVED head1 "" t2)")"
|
"$(rev "$BOT2" APPROVED head1 "" t2)")"
|
||||||
expect "missing bot review means bots-reviewing" state:bots-reviewing "$(decide_state)"
|
expect "a missing bot WITH a live request is bots-reviewing" state:bots-reviewing "$(decide_state)"
|
||||||
|
REQUESTED=""
|
||||||
|
expect "...but with nobody asked it is the agent's ball" state:addressing "$(decide_state)"
|
||||||
|
expect "...and the blocker names the stall" blocker:unrequested "$(blockers)"
|
||||||
|
|
||||||
# -- a comment is a non-verdict, agreement body or not: the author escalates --
|
# -- a comment is a non-verdict, agreement body or not: the author escalates --
|
||||||
REVIEWS_JSON="$(reviews \
|
REVIEWS_JSON="$(reviews \
|
||||||
|
|
@ -157,23 +162,55 @@ ALL_APPROVE="$(reviews \
|
||||||
"$(rev "$BOT3" APPROVED head1 "" t3)")"
|
"$(rev "$BOT3" APPROVED head1 "" t3)")"
|
||||||
|
|
||||||
# -- flavour 1: not mergeable. The merge button is disabled, yet the board
|
# -- flavour 1: not mergeable. The merge button is disabled, yet the board
|
||||||
# said "your turn" on #119/#120/#127 for hours.
|
# said "your turn" on #119/#120/#127 for hours. The branch fact now rides
|
||||||
|
# the blocker axis; the state says whose ball it is, which is the agent's.
|
||||||
DRAFT=false HEAD_SHA=head1 REQUESTED="" REVIEWS_JSON="$ALL_APPROVE" MERGEABLE=CONFLICTING CHECKS=SUCCESS
|
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)"
|
expect "a CONFLICTING PR is the agent's, not the human's" state:addressing "$(decide_state)"
|
||||||
|
expect "...and says WHY on the blocker axis" blocker:conflict "$(blockers)"
|
||||||
REQUESTED="$HUMAN"
|
REQUESTED="$HUMAN"
|
||||||
expect "...even with the human explicitly requested" state:needs-rebase "$(decide_state)"
|
expect "...even with the human explicitly requested" state:addressing "$(decide_state)"
|
||||||
|
|
||||||
# -- red CI is the same claim: not something a human should merge.
|
# -- red CI is the same claim, but NOT the same work: a rebase does not fix a
|
||||||
|
# failing test. Collapsing both into one needs-rebase label told the agent
|
||||||
|
# to do the wrong thing, which is why the axis split exists.
|
||||||
REQUESTED="" MERGEABLE=MERGEABLE CHECKS=FAILURE
|
REQUESTED="" MERGEABLE=MERGEABLE CHECKS=FAILURE
|
||||||
expect "a red PR is needs-rebase" state:needs-rebase "$(decide_state)"
|
expect "a red PR is the agent's" state:addressing "$(decide_state)"
|
||||||
|
expect "...and is distinguishable from a conflict" blocker:ci-red "$(blockers)"
|
||||||
REQUESTED="$HUMAN"
|
REQUESTED="$HUMAN"
|
||||||
expect "...and a human request does not override red CI" state:needs-rebase "$(decide_state)"
|
expect "...and a human request does not override red CI" state:addressing "$(decide_state)"
|
||||||
|
|
||||||
|
# -- both at once. The single-axis design could not say this at all: one label
|
||||||
|
# had to win, and the loser silently vanished off the board.
|
||||||
|
REQUESTED="" MERGEABLE=CONFLICTING CHECKS=FAILURE
|
||||||
|
expect "a conflicted AND red PR reports both blockers" "blocker:conflict
|
||||||
|
blocker:ci-red" "$(blockers)"
|
||||||
|
expect "...and is still just the agent's ball" state:addressing "$(decide_state)"
|
||||||
|
|
||||||
# -- UNKNOWN is NOT unmergeable. GitHub reports it for ~a minute after every
|
# -- 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
|
# 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.
|
# on each merge — worse than the bug being fixed.
|
||||||
REQUESTED="" MERGEABLE=UNKNOWN CHECKS=PENDING
|
REQUESTED="" MERGEABLE=UNKNOWN CHECKS=PENDING
|
||||||
expect "UNKNOWN mergeability does not trigger needs-rebase" state:needs-human "$(decide_state)"
|
expect "UNKNOWN mergeability blocks nothing" state:needs-human "$(decide_state)"
|
||||||
|
expect "...and raises no blocker" "" "$(blockers)"
|
||||||
|
|
||||||
|
# -- blocker:unrequested — the stalled round. Nobody owes an answer because
|
||||||
|
# nobody was ever asked, yet the board read "waiting on the bots" until
|
||||||
|
# `stale` noticed 48h later.
|
||||||
|
MERGEABLE=MERGEABLE CHECKS=SUCCESS REQUESTED="" REVIEWS_JSON='[]'
|
||||||
|
expect "ready, nobody asked, nothing reviewed raises unrequested" blocker:unrequested "$(blockers)"
|
||||||
|
# ...the partial case is equally stalled: one verdict in, nobody asked for the rest
|
||||||
|
REVIEWS_JSON="$(reviews "$(rev "$BOT1" APPROVED head1 "" t1)")"
|
||||||
|
expect "one bot in, none requested is still unrequested" blocker:unrequested "$(blockers)"
|
||||||
|
# ...but a live request means an answer IS coming
|
||||||
|
REQUESTED="$BOT2"
|
||||||
|
expect "a live bot request is not a stalled round" "" "$(blockers)"
|
||||||
|
# ...and a draft is exempt: the bots ignore drafts by design
|
||||||
|
DRAFT=true REQUESTED="" REVIEWS_JSON='[]'
|
||||||
|
expect "a draft with nobody asked is not stalled" "" "$(blockers)"
|
||||||
|
# ...as is an explicit human request — claiming a PR early is deliberate
|
||||||
|
DRAFT=false REQUESTED="$HUMAN"
|
||||||
|
expect "an early human claim is not a stalled round" "" "$(blockers)"
|
||||||
|
REQUESTED="" REVIEWS_JSON="$ALL_APPROVE" MERGEABLE=MERGEABLE CHECKS=SUCCESS
|
||||||
|
|
||||||
# -- flavour 2 (the dangerous one): mergeable, green, human requested, and
|
# -- flavour 2 (the dangerous one): mergeable, green, human requested, and
|
||||||
# NOBODY has reviewed this head. Observed on #119 after a rebase: every
|
# NOBODY has reviewed this head. Observed on #119 after a rebase: every
|
||||||
|
|
@ -207,7 +244,7 @@ expect "...and the same when the stale verdict is the LAST bot in BOTS" \
|
||||||
REVIEWS_JSON="$(reviews "$(rev "$BOT1" APPROVED head1 "" t1)")"
|
REVIEWS_JSON="$(reviews "$(rev "$BOT1" APPROVED head1 "" t1)")"
|
||||||
expect "an unfinished round still yields to an explicit human request" state:needs-human "$(decide_state)"
|
expect "an unfinished round still yields to an explicit human request" state:needs-human "$(decide_state)"
|
||||||
REQUESTED=""
|
REQUESTED=""
|
||||||
expect "...and without that request it is still bots-reviewing" state:bots-reviewing "$(decide_state)"
|
expect "...and without that request the agent owes the ask" state:addressing "$(decide_state)"
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# checks_state: the rollup classifier. It lived inline in main() for the first
|
# checks_state: the rollup classifier. It lived inline in main() for the first
|
||||||
|
|
@ -314,7 +351,8 @@ expect "...and the same when it finished green — mid-flight is not mergeable"
|
||||||
# take the PR off the human's plate, which is the whole point of #136.
|
# 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
|
DRAFT=false HEAD_SHA=head1 REQUESTED="$HUMAN" REVIEWS_JSON="$ALL_APPROVE" MERGEABLE=MERGEABLE
|
||||||
CHECKS="$(rollup "[$(run_ a SUCCESS),$(run_ b CANCELLED)]" | checks_state)"
|
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)"
|
expect "a cancelled check reaches decide_state as the agent's ball" state:addressing "$(decide_state)"
|
||||||
|
expect "...via blocker:ci-red, not a conflict" blocker:ci-red "$(blockers)"
|
||||||
|
|
||||||
# -- the happy path survives all of the above.
|
# -- the happy path survives all of the above.
|
||||||
REVIEWS_JSON="$ALL_APPROVE" MERGEABLE=MERGEABLE CHECKS=SUCCESS REQUESTED=""
|
REVIEWS_JSON="$ALL_APPROVE" MERGEABLE=MERGEABLE CHECKS=SUCCESS REQUESTED=""
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue