fix(labels): never name a label the repo lacks; a stale round owes an ask too

Round 1 review (claude-bot, codex-bot on #138 and the sibling PRs).

gh issue edit rejects the WHOLE call on one unknown label name, and this
change made state and blockers share a single edit for anti-flicker. On a repo
whose taxonomy predates the blocker:* labels -- every repo, at merge -- the
first sweep would therefore converge NOTHING on exactly the PRs the change
exists to fix, surfacing only as a warning in a cron log. Adds are now
filtered against the repo's real label set, read once per sweep. Removals need
no filter: they are built from has_label, so they provably exist. An
unreadable label set does not filter, because a failed read must not silently
strip the board.

blocker:unrequested also missed the stale round. MISSING and STALE are both
verdicts this head does not have -- nobody reviewed it, or everybody reviewed
something else -- and the agent owes an ask either way. The stale one is the
worse of the two: it has approvals on the page that no longer describe the
tree, which is the case LABELS.md already called the most dangerous.

Fixtures 66 -> 68.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
dan-claude-bot 2026-07-20 17:50:00 +00:00
parent 074aea31d8
commit 27cfc5b8c1
4 changed files with 56 additions and 6 deletions

View file

@ -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, # explicit human request — a maintainer claiming a PR early is deliberate,
# not a dropped ball. # not a dropped ball.
if [ "$DRAFT" != true ] && ! requested "$HUMAN"; then 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 for b in "${BOTS[@]}"; do
requested "$b" && any_requested=true 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 done
if [ "$any_missing" = true ] && [ "$any_requested" = false ]; then if [ "$owed" = true ] && [ "$any_requested" = false ]; then
echo blocker:unrequested echo blocker:unrequested
fi fi
fi fi
@ -350,6 +355,30 @@ reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch
done done
add="${add#,}" add="${add#,}"
remove="${remove#,}" 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 if ! has_label "$desired" || [ -n "$remove" ] || [ -n "$add" ]; then
args=(--add-label "$desired${add:+,$add}") args=(--add-label "$desired${add:+,$add}")
[ -n "$remove" ] && args+=(--remove-label "$remove") [ -n "$remove" ] && args+=(--remove-label "$remove")
@ -404,6 +433,11 @@ main() {
bootstrap_labels bootstrap_labels
fi 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 local n
for n in $(gh pr list -R "$REPO" --state open --limit 100 --json number --jq '.[].number'); do for n in $(gh pr list -R "$REPO" --state open --limit 100 --json number --jq '.[].number'); do
( (

View file

@ -33,7 +33,13 @@ which records not just what changed but what each drill run proved.
human request — a maintainer claiming a PR early is deliberate. human request — a maintainer claiming a PR early is deliberate.
The reconciler strips `state:needs-rebase` on sight, so the retirement heals 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 board rather than stranding a label nothing recomputes. It also never
*names* a label the repo does not have: `gh issue edit` rejects the whole
call on one unknown name, so on a repo whose taxonomy predates this change
an unbootstrapped `blocker:*` would otherwise take the state convergence
down with it, on exactly the PRs the change exists to fix. Adds are filtered
against the repo's real label set and the shortfall is logged.
Fixtures 51 → 66.
- **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`**

View file

@ -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: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, 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` | | `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 `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: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: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 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. blocker means the work is the agent's, whatever the review round says.

View file

@ -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 # ...the partial case is equally stalled: one verdict in, nobody asked for the rest
REVIEWS_JSON="$(reviews "$(rev "$BOT1" APPROVED head1 "" t1)")" REVIEWS_JSON="$(reviews "$(rev "$BOT1" APPROVED head1 "" t1)")"
expect "one bot in, none requested is still unrequested" blocker:unrequested "$(blockers)" 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 # ...but a live request means an answer IS coming
REVIEWS_JSON="$(reviews "$(rev "$BOT1" APPROVED head1 "" t1)")"
REQUESTED="$BOT2" REQUESTED="$BOT2"
expect "a live bot request is not a stalled round" "" "$(blockers)" expect "a live bot request is not a stalled round" "" "$(blockers)"
# ...and a draft is exempt: the bots ignore drafts by design # ...and a draft is exempt: the bots ignore drafts by design