diff --git a/.github/scripts/labels-reconcile.sh b/.github/scripts/labels-reconcile.sh index c95366e..3d40ba1 100644 --- a/.github/scripts/labels-reconcile.sh +++ b/.github/scripts/labels-reconcile.sh @@ -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 diff --git a/CHANGELOG.md b/CHANGELOG.md index e85a7a3..af11411 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,7 +45,7 @@ actually cutting it, and this file starts there. sight, so retiring a label heals the board instead of stranding one that nothing recomputes. A verdict is owed in two shapes and both raise `blocker:unrequested`: `MISSING` (nobody reviewed) and `STALE` (everybody - reviewed an older head). Fixtures 51 → 68. + reviewed an older head). Fixtures 51 → 72. ### Fixed @@ -59,7 +59,12 @@ actually cutting it, and this file starts there. line. The add side is 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), and an unreadable label set filters *nothing* rather - than everything — a failed read must not silently strip the board. + than everything — a failed read must not silently strip the board. A missing + *state* label skips only the label edit, not the rest of the PR: clearing a + stale `merge-next` and the staleness sweep depend on no part of the `state:*` + taxonomy, and a cold-start repo that skipped them would leave "merge this one + next" sitting on a PR the board had moved to the agent — the same false + invitation, one scope smaller. - **An unreadable check rollup is no longer read as "nothing is failing"** — when `gh pr view` failed, the fallback left the `statusCheckRollup` key diff --git a/test/labels-reconcile.sh b/test/labels-reconcile.sh index 5970c08..cce4691 100644 --- a/test/labels-reconcile.sh +++ b/test/labels-reconcile.sh @@ -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 ]