heavy-duty/box

Fork 1

labels: `state:needs-human` is sticky and blind to mergeability, and nothing says which PR to merge next #136

New issue

Closed

opened 2026-07-20 14:28:59 +00:00 by dan-claude-bot · 2 comments

dan-claude-bot commented

2026-07-20 14:28:59 +00:00

(Migrated from github.com)

state:needs-human is sticky and blind to mergeability, so the board says "your turn" on PRs that cannot be merged — and there is no label that answers "which one do I merge next?"

The bug

decide_state() in .github/scripts/labels-reconcile.sh computes state from exactly three inputs: the draft flag, requested reviewers, and submitted review verdicts. It reads nothing about mergeable, mergeStateStatus, or statusCheckRollup.

And this line short-circuits everything:

if requested "$HUMAN"; then echo state:needs-human; return; fi

Once the maintainer is requested, the PR reads state:needs-human permanently. It stays there after the branch develops a conflict, after CI goes red, after a force-push staled every approval. Nothing demotes it.

Live right now in this repo, all four labelled state:needs-human:

PR	actual state
#119	`CONFLICTING/DIRTY`
#120	`CONFLICTING/DIRTY`
#127	`CONFLICTING/DIRTY`
#121	`MERGEABLE/CLEAN`, but only 2 of 3 bots have approved at the current head

Not one of them is mergeable. All four are telling the maintainer to come and merge.

Why this matters more than a wrong colour: the label is the only thing that makes the board scannable. A maintainer working from GitHub mobile — which is the realistic case, and the whole reason the label exists — has to open every PR to discover none of them are ready. The label is doing the opposite of its job: it converts "nothing to do" into four false invitations.

It is the same failure shape as a stale approval: the state looks settled because nothing contradicted it.

Fix 1 — mergeability outranks the human request

A review request is an opinion about who should look next. mergeable is a fact about the tree. Facts should win. Proposed precedence:

draft                    -> state:building
not mergeable / red CI   -> (new state, see below)   <-- NEW, and above the human short-circuit
human requested          -> state:needs-human
bot round incomplete     -> state:bots-reviewing
verdicts owed            -> state:addressing

The human short-circuit stays — it is right that an explicit request outranks the bot rounds — it just must not outrank "this branch does not merge."

Fix 2 — a state for "the agent owes a rebase"

state:addressing currently means the coding agent owes a reply and fixes to a review round. A conflicted branch also needs agent work, but it is a different job, triggered by a different event, and needs a different action. Folding them together loses that.

Proposal: state:needs-rebase — the branch does not merge (conflict, or a failing required check) and the agent owes a rebase or a fix. Distinct from addressing, which stays "owes a reply to reviewers".

If you would rather not add a state, reusing state:addressing is still strictly better than the status quo, because it at least stops claiming the maintainer is the blocker.

Fix 3 — `merge-next`, so the board answers the actual question

Even with the above correct, state:needs-human on three PRs does not say which one to merge first. Ordering matters here: these PRs conflict with each other through CHANGELOG.md and test/cli.sh, so merging out of order costs a rebase every time.

Proposal: a merge-next label on exactly one PR — the head of the merge queue.

Unlike state:*, this cannot be derived from GitHub's facts: queue order is intent. So it follows the blocked / release convention in LABELS.md — set by a human or by the agent maintaining the queue, and never guessed by the reconciler. The reconciler's only job would be to clear it when the PR merges or stops being mergeable, so it cannot go stale the way needs-human did.

With all three, the mobile view becomes answerable at a glance:

merge-next + state:needs-human -> merge this one
state:needs-rebase -> agent is on it, ignore
state:bots-reviewing / state:addressing -> not your turn

Acceptance

decide_state() reads mergeable/mergeStateStatus and the check rollup
a non-mergeable PR never carries state:needs-human, even with the human requested
state:needs-rebase (or a documented decision to reuse state:addressing)
merge-next exists, is agent/human-set, and is cleared automatically when the PR merges or stops being mergeable
LABELS.md documents who owns each of the new labels
a fixture pins the exact live case: human requested + CONFLICTING must NOT resolve to state:needs-human

Note

rig and cast share this reconciler and this failure. Filing there as thin cross-references so the three do not drift; whichever lands first is the reference implementation.

`state:needs-human` is sticky and blind to mergeability, so the board says "your turn" on PRs that cannot be merged — and there is no label that answers "which one do I merge next?" ## The bug `decide_state()` in `.github/scripts/labels-reconcile.sh` computes state from exactly three inputs: the draft flag, requested reviewers, and submitted review verdicts. It reads **nothing** about `mergeable`, `mergeStateStatus`, or `statusCheckRollup`. And this line short-circuits everything: ```bash if requested "$HUMAN"; then echo state:needs-human; return; fi ``` Once the maintainer is requested, the PR reads `state:needs-human` **permanently**. It stays there after the branch develops a conflict, after CI goes red, after a force-push staled every approval. Nothing demotes it. Live right now in this repo, all four labelled `state:needs-human`: | PR | actual state | |---|---| | #119 | `CONFLICTING/DIRTY` | | #120 | `CONFLICTING/DIRTY` | | #127 | `CONFLICTING/DIRTY` | | #121 | `MERGEABLE/CLEAN`, but only 2 of 3 bots have approved *at the current head* | Not one of them is mergeable. All four are telling the maintainer to come and merge. **Why this matters more than a wrong colour:** the label is the only thing that makes the board scannable. A maintainer working from GitHub mobile — which is the realistic case, and the whole reason the label exists — has to open every PR to discover none of them are ready. The label is doing the opposite of its job: it converts "nothing to do" into four false invitations. It is the same failure shape as a stale approval: the state looks settled because nothing contradicted it. ## Fix 1 — mergeability outranks the human request A review request is an opinion about *who should look next*. `mergeable` is a fact about *the tree*. Facts should win. Proposed precedence: ``` draft -> state:building not mergeable / red CI -> (new state, see below) <-- NEW, and above the human short-circuit human requested -> state:needs-human bot round incomplete -> state:bots-reviewing verdicts owed -> state:addressing ``` The human short-circuit stays — it is right that an explicit request outranks the *bot* rounds — it just must not outrank "this branch does not merge." ## Fix 2 — a state for "the agent owes a rebase" `state:addressing` currently means *the coding agent owes a reply and fixes to a review round*. A conflicted branch also needs agent work, but it is a different job, triggered by a different event, and needs a different action. Folding them together loses that. Proposal: **`state:needs-rebase`** — the branch does not merge (conflict, or a failing required check) and the agent owes a rebase or a fix. Distinct from `addressing`, which stays "owes a reply to reviewers". If you would rather not add a state, reusing `state:addressing` is still strictly better than the status quo, because it at least stops claiming the maintainer is the blocker. ## Fix 3 — `merge-next`, so the board answers the actual question Even with the above correct, `state:needs-human` on three PRs does not say **which one to merge first**. Ordering matters here: these PRs conflict with each other through `CHANGELOG.md` and `test/cli.sh`, so merging out of order costs a rebase every time. Proposal: a **`merge-next`** label on exactly one PR — the head of the merge queue. Unlike `state:*`, this cannot be derived from GitHub's facts: queue order is *intent*. So it follows the `blocked` / `release` convention in [LABELS.md](LABELS.md) — set by a human or by the agent maintaining the queue, and never guessed by the reconciler. The reconciler's only job would be to **clear it when the PR merges or stops being mergeable**, so it cannot go stale the way `needs-human` did. With all three, the mobile view becomes answerable at a glance: - `merge-next` + `state:needs-human` -> **merge this one** - `state:needs-rebase` -> agent is on it, ignore - `state:bots-reviewing` / `state:addressing` -> not your turn ## Acceptance - [ ] `decide_state()` reads `mergeable`/`mergeStateStatus` and the check rollup - [ ] a non-mergeable PR never carries `state:needs-human`, even with the human requested - [ ] `state:needs-rebase` (or a documented decision to reuse `state:addressing`) - [ ] `merge-next` exists, is agent/human-set, and is **cleared automatically** when the PR merges or stops being mergeable - [ ] LABELS.md documents who owns each of the new labels - [ ] a fixture pins the exact live case: human requested + `CONFLICTING` must NOT resolve to `state:needs-human` ## Note rig and cast share this reconciler and this failure. Filing there as thin cross-references so the three do not drift; whichever lands first is the reference implementation.

dan-claude-bot commented

2026-07-20 14:32:46 +00:00

(Migrated from github.com)

Siblings, same defect byte for byte, each with its own evidence: heavy-duty/rig#87, heavy-duty/cast#127.

dan-claude-bot commented

2026-07-20 14:53:08 +00:00

(Migrated from github.com)

A second live case, and it is a different flavour from the conflicts in the issue body — worth pinning because a fix aimed only at mergeability would miss it.

Right now, #119:

mergeable:  MERGEABLE / CLEAN
CI:         all four checks SUCCESS
label:      state:needs-human
head:       ecce72f0
reviews:    grok   APPROVED  807be331   <- previous head
            codex  APPROVED  807be331   <- previous head
            claude APPROVED  807be331   <- previous head
head-current verdicts: NONE

Everything the maintainer can see says merge me: green, mergeable, "waiting on the human". But no bot has reviewed this tree. A force-push (a rebase, in this case) moved head and staled all three approvals; the reconciler kept state:needs-human because if requested "$HUMAN" short-circuits at decide_state() line 90, before the *STALE* arm at line 103 is ever consulted.

So the sticky label has two failure modes, not one:

not mergeable — conflicts or red CI, label still says needs-human (the issue body: #119/#120/#127 an hour ago)
mergeable but unreviewed at head — approvals stale after a push, label still says needs-human (this case)

Both end at the same place: the board invites a merge that should not happen. The second is the more dangerous one, because nothing else on the page contradicts it — with conflicts, at least the merge button is disabled.

Implication for the fix: precedence must put the *STALE* check above the human short-circuit too, not just mergeability. Suggested order:

draft                          -> state:building
not mergeable / red CI         -> state:needs-rebase
no head-current verdicts / stale -> state:bots-reviewing (or addressing)
human requested                -> state:needs-human
bot round incomplete           -> state:bots-reviewing

The rule underneath: state:needs-human should mean "a human could merge this right now". Anything that makes that false — unmergeable, red, or unreviewed at head — must outrank the request that put it there.

Worth a fixture for this exact shape: human requested + MERGEABLE/CLEAN + zero head-current reviews must NOT resolve to state:needs-human.

A second live case, and it is a **different flavour** from the conflicts in the issue body — worth pinning because a fix aimed only at mergeability would miss it. Right now, #119: ``` mergeable: MERGEABLE / CLEAN CI: all four checks SUCCESS label: state:needs-human head: ecce72f0 reviews: grok APPROVED 807be331 <- previous head codex APPROVED 807be331 <- previous head claude APPROVED 807be331 <- previous head head-current verdicts: NONE ``` Everything the maintainer can see says *merge me*: green, mergeable, "waiting on the human". But **no bot has reviewed this tree**. A force-push (a rebase, in this case) moved head and staled all three approvals; the reconciler kept `state:needs-human` because `if requested "$HUMAN"` short-circuits at `decide_state()` line 90, before the `*STALE*` arm at line 103 is ever consulted. So the sticky label has two failure modes, not one: 1. **not mergeable** — conflicts or red CI, label still says needs-human (the issue body: #119/#120/#127 an hour ago) 2. **mergeable but unreviewed at head** — approvals stale after a push, label still says needs-human (this case) Both end at the same place: the board invites a merge that should not happen. The second is the more dangerous one, because nothing else on the page contradicts it — with conflicts, at least the merge button is disabled. **Implication for the fix:** precedence must put the `*STALE*` check above the human short-circuit too, not just mergeability. Suggested order: ``` draft -> state:building not mergeable / red CI -> state:needs-rebase no head-current verdicts / stale -> state:bots-reviewing (or addressing) human requested -> state:needs-human bot round incomplete -> state:bots-reviewing ``` The rule underneath: `state:needs-human` should mean *"a human could merge this right now"*. Anything that makes that false — unmergeable, red, or unreviewed at head — must outrank the request that put it there. Worth a fixture for this exact shape: human requested + `MERGEABLE/CLEAN` + zero head-current reviews must NOT resolve to `state:needs-human`.

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/box#136

No description provided.

labels: state:needs-human is sticky and blind to mergeability, and nothing says which PR to merge next #136

The bug

Fix 1 — mergeability outranks the human request

Fix 2 — a state for "the agent owes a rebase"

Fix 3 — merge-next, so the board answers the actual question

Acceptance

Note

labels: `state:needs-human` is sticky and blind to mergeability, and nothing says which PR to merge next #136

Fix 3 — `merge-next`, so the board answers the actual question