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>
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>
Its own Fixed entry rather than a line in the refactor above it: the two-axis
split is a design change, this is a correctness bug in a different function,
and a reader bisecting a false "merge me" wants the second one named.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gh returning nothing left the rollup KEY absent, which collapsed into the same
NONE as a PR that genuinely has no checks -- so an API hiccup presented as
mergeable-by-a-human. That is the unknown-certified-as-green shape this machine
exists to stop, surviving in the one place the #136 fix did not look.
checks_state now reports UNREADABLE for an absent key, and the sweep leaves
that PR exactly as it is rather than recomputing on facts it did not read.
Blocking on it instead would flap the whole board on one bad call.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
