Skip to content

gate: an unstable merge state summons a human and then latches the manual-review label #10116

Description

@JSONbored

An unstable merge state summons a human, and then latches

GitHub reports mergeable_state: "unstable" for two different situations: a non-required check is failing, and checks are still running. The gate treats both as a manual-review hold. On a repo meant to run autonomously that is the wrong answer in both cases — a pending PR is simply not ready yet, and a failing one has already been answered by CI, which will answer again on the next push. Neither needs a maintainer.

Observed on #10098 and JSONbored/metagraphed#8838: both carried manual-review, neither was a guardrail hit, and #10098 sat open with finished (red) CI until a human stripped the label by hand.

Why it holds, and why guardrail escalation does not save it

derivePrDisposition computes:

const heldForManualReview = heldBy.length > 0 || unstableHolds;

heldBy is the list of declared hold inputs (MERGE_HOLD_INPUTS). unstableHolds is not one of them — it is OR-ed in separately. Two consequences:

  1. The escalation config cannot release it. releasedHolds in derivePrDisposition contains exactly one key, guardrailHit. Routing guardrail-path PRs to a stronger model with onCleanReview: proceed releases the guardrail term and nothing else, so a PR held by unstableHolds stays held no matter how the escalated review comes back. Worse, guardrailEscalationCleared requires reviewGood, which requires green CI — so on a red-CI PR it can never fire at all.
  2. The ledger cannot say why. heldBy stays empty, so the hold reaches decision_records with no cause and reason_code falls through to the gate conclusion — this is a large share of the 518 holds filed under reason_code: "success" that orb(ledger): a hold records reason_code 'success', so the manual-review surface cannot be split by cause #9991 went looking for.

Why the PR then gets stuck

heldForManualReview is not advisory. Three separate mechanisms read it or the label it produces:

  • the executor denies merge and approve while the label is present;
  • merge-train.ts:152 filters !sibling.heldForManualReview, so a held PR is evicted from the train entirely — which is why these PRs get skipped with no merge-train comment while other PRs merge past them;
  • section 1b's release condition (noManualReviewHoldWanted) also tested !mergeableStateUnstable.

That last one is the latch: the label is applied while CI is red, and then cannot be lifted because CI is red. Nothing short of a human removing it clears the state.

The fix

An unstable merge state stops being a manual-review hold. It does not become mergeable.

The contributor still learns the state: the unified comment refuses "safe to merge" via mergeStateHeld, and the PR's own Checks tab names the unhappy checks — always current, unlike a bot comment posted once per pass on every in-flight PR.

Not in scope

deriveUnifiedStatus renders mergeStateHeld as "Manual Review" in the comment's verdict box. That wording is now inconsistent with the disposition, but it is text on a comment — it blocks nothing and the same code path serves dirty/behind. Filed separately rather than widened into this change.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions