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

Closed
opened 2026-07-20 14:29:13 +00:00 by dan-claude-bot · 1 comment
dan-claude-bot commented 2026-07-20 14:29:13 +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: no open PRs — the batch just drained. So the evidence here is the code plus what happened while it was draining.

grep -c 'mergeable\|mergeStateStatus\|statusCheckRollup' .github/scripts/labels-reconcile.sh returns 0, and the short-circuit above sits at decide_state() line 90 — byte-identical to box's.

It bit during the 10-PR batch merged earlier today. Every time one PR merged, the rest went CONFLICTING through CHANGELOG.md — and every one of them kept its state:needs-human label the whole time. The board invited the maintainer to merge PRs that could not be merged, repeatedly, for hours. It was diagnosed only because someone opened them one by one.

The second half bit too: with six PRs simultaneously reading state:needs-human, nothing on the board said which to merge first — and order mattered, because merging out of sequence cost a rebase of everything below it.

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 — the realistic case, and the whole reason the label exists — has to open every PR to discover which are ready. The label does the opposite of its job.

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

box, rig and cast share this reconciler and this failure, byte for byte. Full write-ups filed in all three (heavy-duty/box#136) so none is a stub; whichever lands first is the reference implementation and the other two should follow it rather than diverge.

`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: **no open PRs** — the batch just drained. So the evidence here is the code plus what happened while it was draining. `grep -c 'mergeable\|mergeStateStatus\|statusCheckRollup' .github/scripts/labels-reconcile.sh` returns **0**, and the short-circuit above sits at `decide_state()` line 90 — byte-identical to box's. It bit during the 10-PR batch merged earlier today. Every time one PR merged, the rest went `CONFLICTING` through `CHANGELOG.md` — and every one of them kept its `state:needs-human` label the whole time. The board invited the maintainer to merge PRs that could not be merged, repeatedly, for hours. It was diagnosed only because someone opened them one by one. The second half bit too: with six PRs simultaneously reading `state:needs-human`, nothing on the board said which to merge first — and order mattered, because merging out of sequence cost a rebase of everything below it. **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 — the realistic case, and the whole reason the label exists — has to open every PR to discover which are ready. The label does the opposite of its job. 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 box, rig and cast share this reconciler and this failure, byte for byte. Full write-ups filed in all three (heavy-duty/box#136) so none is a stub; whichever lands first is the reference implementation and the other two should follow it rather than diverge.
dan-claude-bot commented 2026-07-20 14:32:43 +00:00 (Migrated from github.com)

Siblings, same defect byte for byte: heavy-duty/box#136, heavy-duty/cast#127.

Siblings, same defect byte for byte: heavy-duty/box#136, heavy-duty/cast#127.
Sign in to join this conversation.
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/rig#87
No description provided.