fix(labels): a missing state label must skip the EDIT, not the whole PR

Round 2 review (claude-bot, codex-bot, grok-bot -- all three, independently).

The label pre-flight added in round 1 returned out of reconcile_pr entirely.
Everything below that point is independent of the state:* taxonomy: clearing a
stale merge-next, and the stale sweep. Stranding them meant a merge-next claim
reading "merge this one next" survived on a PR the board had moved to the
agent -- the same false invitation as #136, one scope smaller -- and the
staleness detector went silent. On a cold-start repo, where no state:* label
exists yet, that was EVERY PR.

It was also a regression against main rather than a missed improvement: the old
code failed the edit, logged, and fell through to both blocks. The round-1 fix
turned a per-edit failure into a per-PR abort.

Now skip_edit=true, and control reaches the rest of the function.

Also taken, both from claude-bot and grok-bot: the dead "$desired" term in the
filter loop, and `[ -n "$missing" ] && log` becoming a proper elif rather than
an &&-as-statement under set -e.

Four fixtures now drive reconcile_pr itself, stubbing run/gh -- the first in
this suite to reach past the pure functions, which is exactly why a per-PR
return was invisible to the fixtures that existed. Fixtures 68 -> 72.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
dan-claude-bot 2026-07-20 18:09:07 +00:00
parent 27cfc5b8c1
commit f956a85a1c
3 changed files with 51 additions and 8 deletions

View file

@ -365,21 +365,29 @@ reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch
# 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.
local skip_edit=false
if [ -n "${REPO_LABELS:-}" ]; then
local kept="" missing="" want
for want in ${add//,/ } "$desired"; do
[ "$want" = "$desired" ] && continue
for want in ${add//,/ }; do
if grep -qxF "$want" <<<"$REPO_LABELS"; then kept="$kept,$want"
else missing="$missing $want"; fi
done
add="${kept#,}"
# A missing STATE label skips only the EDIT — never the rest of this
# function. Everything below is independent of the state:* taxonomy, and
# returning here stranded it: `merge-next` kept claiming "merge this one
# next" on a PR the board had moved to the agent, and the stale sweep
# stopped running. That is the original false-invitation bug, reintroduced
# in the very fix meant to survive a cold-start repo — and a regression
# against the old behaviour, which failed the edit and fell through.
if ! grep -qxF "$desired" <<<"$REPO_LABELS"; then
log "#$n: WARNING: state label '$desired' does not exist — run the workflow manually to bootstrap"
return
log "#$n: WARNING: state label '$desired' does not exist — skipping the label edit; dispatch the workflow to bootstrap"
skip_edit=true
elif [ -n "$missing" ]; then
log "#$n: WARNING: missing label(s)$missing — state still converged; dispatch the workflow to bootstrap"
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 [ "$skip_edit" = false ] && { ! has_label "$desired" || [ -n "$remove" ] || [ -n "$add" ]; }; then
args=(--add-label "$desired${add:+,$add}")
[ -n "$remove" ] && args+=(--remove-label "$remove")
if run gh issue edit "$n" -R "$REPO" "${args[@]}" >/dev/null; then

View file

@ -38,8 +38,10 @@ which records not just what changed but what each drill run proved.
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.
against the repo's real label set and the shortfall is logged — and a
taxonomy gap skips only the label edit, never the `merge-next` clearing or
the stale sweep, which do not depend on the `state:*` set.
Fixtures 51 → 72.
- **The tenant templates carry rig's family suffix: `claude``claude-box`,
`codex``codex-box`, `grok``grok-box`, `staging``staging-box`**

View file

@ -379,5 +379,38 @@ 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='[]'
# ---------------------------------------------------------------------------
# reconcile_pr's cold-start path. Everything above tests pure functions, which
# is exactly why a per-PR `return` in the label pre-flight got through review:
# the fixtures could not reach it. A missing state:* label must skip the label
# EDIT only — merge-next clearing and the stale sweep are independent of the
# taxonomy, and stranding them reintroduced the false-invitation bug (a
# `merge-next` claim surviving on a PR the board had moved to the agent).
# ---------------------------------------------------------------------------
reconcile_probe() { # $1 = REPO_LABELS content → the log lines reconcile_pr emits
(
REPO_LABELS="$1" REPO=owner/repo NOW="$(date +%s)"
LABELS="merge-next" # the PR carries a queue claim
DRAFT=false HEAD_SHA=head1 REQUESTED="" REVIEWS_JSON='[]'
MERGEABLE=MERGEABLE CHECKS=SUCCESS
PR_JSON='{"created_at":"2020-01-01T00:00:00Z"}'
run() { :; } # swallow mutations
gh() { :; } # no network
reconcile_pr 777 2>&1
)
}
cold="$(reconcile_probe "merge-next")" # state:* labels absent entirely
expect "a cold-start repo still clears merge-next" \
yes "$(grep -q 'cleared merge-next' <<<"$cold" && echo yes || echo no)"
expect "...and still runs the stale sweep" \
yes "$(grep -q 'stale (' <<<"$cold" && echo yes || echo no)"
expect "...while warning that the state label is missing" \
yes "$(grep -q "state label 'state:addressing' does not exist" <<<"$cold" && echo yes || echo no)"
warm="$(reconcile_probe "$(printf 'state:addressing\nmerge-next\nstale\nblocker:unrequested')")"
expect "a bootstrapped repo converges the state as well" \
yes "$(grep -q 'state -> state:addressing' <<<"$warm" && echo yes || echo no)"
printf 'labels-reconcile tests: %d passed, %d failed\n' "$pass" "$fail"
[ "$fail" -eq 0 ]