From 7c53267377934a2bdb48dab15ddd2d5f2c40a684 Mon Sep 17 00:00:00 2001 From: dan-claude-bot Date: Sun, 2 Aug 2026 13:25:59 +0000 Subject: [PATCH] fix: a standing non-approving verdict outranks draft in decide_state round_outranks_draft consults the round before draft short-circuits: a re-drafted PR carrying CHANGES_REQUESTED, an owed round-reply, or push-staled approvals reads state:addressing; a live panel request on a draft surfaces as state:bots-reviewing rather than being absorbed (the must-not-paper-over combination, decided as: visible). Approvals do not outrank draft, so a draft never reads needs-human, and a virgin draft is byte-identical to before. LABELS.md's state:building row makes draft evidence, not the definition. Refs #205 Co-Authored-By: Claude Fable 5 --- LABELS.md | 2 +- actions/labels-reconcile/labels-reconcile.sh | 31 +++++++++++- changelog.d/205.md | 6 +++ test/labels-reconcile.test.sh | 52 ++++++++++++++++++++ 4 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 changelog.d/205.md diff --git a/LABELS.md b/LABELS.md index 8eba639..7402bd4 100644 --- a/LABELS.md +++ b/LABELS.md @@ -17,7 +17,7 @@ and the reconciler recomputes it from GitHub's own facts. | Label | Color | Waiting on | |---|---|---| -| `state:building` | `#FBCA04` | the builder — PR is a draft | +| `state:building` | `#FBCA04` | the builder — pre-round: no verdict stands against the head. Draft is evidence for it, not the definition of it: a draft carrying a standing non-approving verdict is a fix round and reads `state:addressing` (#205) | | `state:bots-reviewing` | `#1D76DB` | the reviewer panel to finish the round (a request is live) | | `state:addressing` | `#D93F0B` | the builder — round complete without full approval, or nobody was asked, or a blocker is up, or a ruling is pending | | `state:needs-human` | `#8250DF` | the human — **this PR could be merged right now**: zero blockers, whole panel approved the current head | diff --git a/actions/labels-reconcile/labels-reconcile.sh b/actions/labels-reconcile/labels-reconcile.sh index 432c841..9df22b8 100755 --- a/actions/labels-reconcile/labels-reconcile.sh +++ b/actions/labels-reconcile/labels-reconcile.sh @@ -469,8 +469,37 @@ blockers() { # → the blocker:* labels this PR should carry, one per line fi } +round_outranks_draft() { # 0 when the round's standing word survives a re-draft (#205) + # A standing non-approving verdict outranks draft: a PR that took a round, + # carries CHANGES_REQUESTED (or a comment owed a reply, or approvals a push + # staled), and is then converted back to draft is a fix round in progress, + # not a build — and hiding it behind state:building is a dropped ball the + # staleness sweep reads as work in progress. Approvals do NOT outrank + # draft: a re-draft after a passed round is deliberately building again, + # and a draft must never read state:needs-human. + # + # A LIVE panel request on a draft also falls through — deliberately + # surfaced, not absorbed (#205's must-not-paper-over): the bots ignore + # drafts by design, so a draft wearing state:bots-reviewing on the board + # is the visible symptom of a real defect (a request nobody cleared at + # round close, or a hand-requested draft), and reading it as building + # would hide exactly that. + local b + for b in "${REQUIRED_BOTS[@]}"; do + requested "$b" && return 0 + case "$(bot_verdict "$b")" in BLOCK | FEEDBACK | STALE) return 0 ;; esac + done + [ "$(bot_verdict "$HUMAN")" = BLOCK ] +} + decide_state() { # → the one state:* label this PR should carry - if [ "$DRAFT" = true ]; then echo state:building; return; fi + # Draft decides the state only when the round implies nothing else (#205): + # a draft with no round history reads state:building exactly as it always + # has, and round_outranks_draft is what "nothing else" means. + if [ "$DRAFT" = true ] && ! round_outranks_draft; then + echo state:building + return + fi local s s="$(round_state)" diff --git a/changelog.d/205.md b/changelog.d/205.md new file mode 100644 index 0000000..d1c2371 --- /dev/null +++ b/changelog.d/205.md @@ -0,0 +1,6 @@ +### Fixed + +- A standing non-approving verdict now outranks draft in `decide_state`: a + re-drafted PR mid-round reads `state:addressing`, a live panel request on a + draft surfaces as `state:bots-reviewing`, and a draft with no round history + still reads `state:building` (#205). diff --git a/test/labels-reconcile.test.sh b/test/labels-reconcile.test.sh index 8752026..accba15 100755 --- a/test/labels-reconcile.test.sh +++ b/test/labels-reconcile.test.sh @@ -986,6 +986,58 @@ for ev in schedule pull_request_target; do expect "...and deletes nothing" \ no "$(grep -q '^delete ' "$EXEC/record" && echo yes || echo no)" done +# -- a re-drafted fix round is not a build (#205) ---------------------------- +# Draft used to short-circuit decide_state before the round was consulted, so +# a PR carrying a standing CHANGES_REQUESTED that its builder converted back +# to draft read state:building — and the staleness sweep read a dropped fix +# round as a build in progress. +load_config .github/labels.conf +set_required_bots codex-bot-andresmgsl +MERGEABLE=MERGEABLE CHECKS=SUCCESS LABELS="" HEAD_SHA=head1 +DRAFT=true REQUESTED="" REVIEWS_JSON="$(reviews \ + "$(rev "$BOT1" CHANGES_REQUESTED head1 no t1)" \ + "$(rev "$BOT2" APPROVED head1 ok t2)" \ + "$(rev "$BOT3" APPROVED head1 ok t3)")" +expect "a re-drafted PR with a standing block is addressing, not building" \ + state:addressing "$(decide_state)" +REVIEWS_JSON="$(reviews "$(rev "$BOT1" COMMENTED head1 thoughts t1)")" +expect "a re-drafted PR owing a round-reply is addressing" \ + state:addressing "$(decide_state)" +REVIEWS_JSON="$(reviews \ + "$(rev "$BOT1" APPROVED head0 ok t1)" \ + "$(rev "$BOT2" APPROVED head0 ok t2)" \ + "$(rev "$BOT3" APPROVED head0 ok t3)")" +expect "a re-drafted PR whose approvals a push staled is addressing" \ + state:addressing "$(decide_state)" +REVIEWS_JSON="$(reviews \ + "$(rev "$BOT1" APPROVED head1 ok t1)" \ + "$(rev "$BOT2" APPROVED head1 ok t2)" \ + "$(rev "$BOT3" APPROVED head1 ok t3)" \ + "$(rev "$HUMAN" CHANGES_REQUESTED head1 no t4)")" +expect "the human's standing changes-requested outranks draft too" \ + state:addressing "$(decide_state)" +# Approvals do NOT outrank draft: a re-draft after a passed round is +# deliberately building again — and a draft must never read needs-human. +REVIEWS_JSON="$(reviews \ + "$(rev "$BOT1" APPROVED head1 ok t1)" \ + "$(rev "$BOT2" APPROVED head1 ok t2)" \ + "$(rev "$BOT3" APPROVED head1 ok t3)")" +expect "a re-draft after a passed round is building again" \ + state:building "$(decide_state)" +REQUESTED="$HUMAN" +expect "...even with the human requested — a draft never reads needs-human" \ + state:building "$(decide_state)" +# The must-not-paper-over combination: a live panel request on a draft is a +# board defect (the bots ignore drafts by design) and stays VISIBLE as +# bots-reviewing rather than being absorbed into building. +REQUESTED="$BOT2" REVIEWS_JSON='[]' +expect "a live panel request on a draft surfaces as bots-reviewing" \ + state:bots-reviewing "$(decide_state)" +# The byte-identical baseline: a virgin draft still reads building. +REQUESTED="" REVIEWS_JSON='[]' +expect "a draft with no round history still reads building" \ + state:building "$(decide_state)" + # -- per-author panels (#224): the required set flows from the one ---------- # resolution point, and convergence counts the effective set — never the # base panel beside a reduced request set (the must-fail the issue names)