forked from heavy-duty/rig
fix(labels): survive a repo that lacks the new blocker labels
Round 1 review (claude-bot, codex-bot — both raised 1 and 2). 1. `gh issue edit --add-label` rejects the WHOLE call on one unknown label name, and the blocker:* labels are created only by the dispatch-only bootstrap. So the first sweep after this lands would have converged NOTHING on exactly the PRs this change exists to heal, surfacing only as a WARNING in a cron log. Batching state and blockers into one edit for anti-flicker is what widened that blast radius. Every label about to be ADDED is now filtered against the repo's real label set, read once per sweep; removals need no filter because they are built from has_label. An unreadable label set does not filter, so a failed read cannot silently strip the board. 2. blocker:unrequested fired only on MISSING, so a round whose approvals all staled behind a push — with nothing re-requested — carried no blocker at all, though the agent owes exactly the same ask. Now MISSING or STALE: both mean this head has no verdict from that reviewer. 3. LABELS.md: restore the substantive "Leaves when" text for state:addressing, and widen the blocker:unrequested row to name both shapes now that (2) changes what the label means. Fixtures 66 -> 68. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
4f40cfabf2
commit
b07e734fc0
4 changed files with 63 additions and 9 deletions
40
.github/scripts/labels-reconcile.sh
vendored
40
.github/scripts/labels-reconcile.sh
vendored
|
|
@ -194,12 +194,17 @@ blockers() { # → the blocker:* labels this PR should carry, one per line
|
|||
# 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
|
||||
local b v owed=false any_requested=false
|
||||
for b in "${BOTS[@]}"; do
|
||||
requested "$b" && any_requested=true
|
||||
[ "$(bot_verdict "$b")" = MISSING ] && any_missing=true
|
||||
# MISSING and STALE are both verdicts this head does not have: nobody
|
||||
# reviewed it, or everybody reviewed something else. The agent owes an
|
||||
# ask either way — the stale round is if anything the worse of the two,
|
||||
# since it has approvals on the page that no longer describe the tree.
|
||||
v="$(bot_verdict "$b")"
|
||||
case "$v" in MISSING | STALE) owed=true ;; esac
|
||||
done
|
||||
if [ "$any_missing" = true ] && [ "$any_requested" = false ]; then
|
||||
if [ "$owed" = true ] && [ "$any_requested" = false ]; then
|
||||
echo blocker:unrequested
|
||||
fi
|
||||
fi
|
||||
|
|
@ -350,6 +355,30 @@ reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch
|
|||
done
|
||||
add="${add#,}"
|
||||
remove="${remove#,}"
|
||||
|
||||
# Never NAME a label the repo does not have. `gh issue edit --add-label`
|
||||
# rejects the WHOLE call on one unknown name — nothing is applied — so a
|
||||
# single missing blocker would take the state convergence down with it, on
|
||||
# exactly the PRs this change exists to fix, surfacing only as a log line.
|
||||
# Batching state and blockers into one edit for anti-flicker is what widened
|
||||
# that blast radius; filtering the add side is what closes it again.
|
||||
# Removals need no filter: they are built from has_label, so the label
|
||||
# provably exists. REPO_LABELS unreadable means no filtering rather than
|
||||
# filtering everything out — a failed read must not silently strip the board.
|
||||
if [ -n "${REPO_LABELS:-}" ]; then
|
||||
local kept="" missing="" want
|
||||
for want in ${add//,/ } "$desired"; do
|
||||
[ "$want" = "$desired" ] && continue
|
||||
if grep -qxF "$want" <<<"$REPO_LABELS"; then kept="$kept,$want"
|
||||
else missing="$missing $want"; fi
|
||||
done
|
||||
add="${kept#,}"
|
||||
if ! grep -qxF "$desired" <<<"$REPO_LABELS"; then
|
||||
log "#$n: WARNING: state label '$desired' does not exist — run the workflow manually to bootstrap"
|
||||
return
|
||||
fi
|
||||
[ -n "$missing" ] && log "#$n: WARNING: missing label(s)$missing — state still converged; dispatch the workflow to bootstrap"
|
||||
fi
|
||||
if ! has_label "$desired" || [ -n "$remove" ] || [ -n "$add" ]; then
|
||||
args=(--add-label "$desired${add:+,$add}")
|
||||
[ -n "$remove" ] && args+=(--remove-label "$remove")
|
||||
|
|
@ -404,6 +433,11 @@ main() {
|
|||
bootstrap_labels
|
||||
fi
|
||||
|
||||
# The repo's label set, read ONCE per sweep — reconcile_pr filters every
|
||||
# add against it, because one unknown name fails the whole edit call.
|
||||
REPO_LABELS="$(gh label list -R "$REPO" --limit 200 --json name --jq '.[].name' 2>/dev/null || echo "")"
|
||||
[ -z "$REPO_LABELS" ] && log "WARNING: could not read the label set — applying labels unfiltered"
|
||||
|
||||
local n
|
||||
for n in $(gh pr list -R "$REPO" --state open --limit 100 --json number --jq '.[].number'); do
|
||||
(
|
||||
|
|
|
|||
18
CHANGELOG.md
18
CHANGELOG.md
|
|
@ -219,13 +219,23 @@ on the way to cutting its first release, and this file starts there.
|
|||
and an answer is coming*. A ready PR nobody was asked to review used to read
|
||||
as "waiting on the reviewers" until the stale sweep caught up; it now reads
|
||||
`state:addressing` + `blocker:unrequested`, because the agent owes the ask
|
||||
and the board should say so. Drafts stay exempt — the bots ignore drafts by
|
||||
design — as does an explicit human request, since a maintainer claiming a PR
|
||||
early is deliberate.
|
||||
and the board should say so. `blocker:unrequested` covers both shapes of
|
||||
"this head has no verdict from somebody": nobody reviewed it, or everybody
|
||||
reviewed an older tree and the approvals staled behind a push. The second is
|
||||
the worse of the two, since it leaves approvals on the page that no longer
|
||||
describe the code. Drafts stay exempt — the bots ignore drafts by design —
|
||||
as does an explicit human request, since a maintainer claiming a PR early is
|
||||
deliberate.
|
||||
|
||||
The reconciler strips `state:needs-rebase` on sight via a `RETIRED` list, so
|
||||
the retirement heals the existing board instead of stranding a label that
|
||||
nothing recomputes. Fixtures 51 → 66.
|
||||
nothing recomputes. It also filters every label it is about to *add* against
|
||||
the repo's actual label set, read once per sweep: `gh issue edit` rejects the
|
||||
whole call on one unknown name, so on a repo that has not yet bootstrapped
|
||||
the new `blocker:*` labels a single missing one would have taken the state
|
||||
convergence down with it — on exactly the PRs this change exists to heal.
|
||||
Now the state still converges and the missing labels are named in the log.
|
||||
Fixtures 51 → 68.
|
||||
|
||||
- **BREAKING: `--class human|server` is now `--root-door closed|open`** (#77) —
|
||||
the trait was named for who *lives on* a box; what it decides is one thing,
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ 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: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, 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: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 round-reply is posted and fixes pushed — and any blocker named alongside is cleared |
|
||||
| `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` |
|
||||
|
||||
`bots-reviewing` and `addressing` are deliberately distinct: staleness in the
|
||||
|
|
@ -35,7 +35,7 @@ PR carries as many as apply.
|
|||
|---|---|---|---|
|
||||
| `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 |
|
||||
| `blocker:unrequested` | `#E99695` | this head has no verdict from somebody — never reviewed, or staled by a push — 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.
|
||||
|
|
|
|||
|
|
@ -201,7 +201,17 @@ expect "ready, nobody asked, nothing reviewed raises unrequested" blocker:unrequ
|
|||
# ...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)"
|
||||
# ...a STALE round with nobody asked is the same debt, and arguably worse: the
|
||||
# page carries approvals that no longer describe the tree. Guarding on
|
||||
# MISSING alone let this one through with no blocker at all.
|
||||
REVIEWS_JSON="$(reviews \
|
||||
"$(rev "$BOT1" APPROVED oldhead "" t1)" \
|
||||
"$(rev "$BOT2" APPROVED oldhead "" t2)" \
|
||||
"$(rev "$BOT3" APPROVED oldhead "" t3)")"
|
||||
expect "a stale round with nobody asked is unrequested too" blocker:unrequested "$(blockers)"
|
||||
expect "...and is still the agent's ball" state:addressing "$(decide_state)"
|
||||
# ...but a live request means an answer IS coming
|
||||
REVIEWS_JSON="$(reviews "$(rev "$BOT1" APPROVED head1 "" t1)")"
|
||||
REQUESTED="$BOT2"
|
||||
expect "a live bot request is not a stalled round" "" "$(blockers)"
|
||||
# ...and a draft is exempt: the bots ignore drafts by design
|
||||
|
|
|
|||
Loading…
Reference in a new issue