settings(pr-disposition): isCommentMergeStateHeld ignores unstableExplainedByIgnoredChecks, so the comment surface still holds a PR the planner does not

[JSONbored]

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

src/settings/pr-disposition.ts exists specifically to remove the "four surfaces each re-derive their own
meaning for GitHub's raw mergeable_state" class its own header describes
(src/settings/pr-disposition.ts:1–src/settings/pr-disposition.ts:8). It exports two things that are
supposed to agree:

// src/settings/pr-disposition.ts:128
const unstableHolds = mergeable === "unstable" && input.unstableExplainedByIgnoredChecks !== true;
...
// src/settings/pr-disposition.ts:148
const commentMergeStateHeld = mergeable === "conflict" || mergeable === "behind" || unstableHolds;

// src/settings/pr-disposition.ts:155
 *  uses. Equal by construction to derivePrDisposition(...).commentMergeStateHeld. */
export function isCommentMergeStateHeld(state: string | null | undefined): boolean {
  const mergeable = assessMergeableState(state);
  return mergeable === "conflict" || mergeable === "behind" || mergeable === "unstable";
}

The "equal by construction" claim was true before unstableExplainedByIgnoredChecks was introduced
(src/settings/pr-disposition.ts:93). It is false now: for mergeableState === "unstable" with
unstableExplainedByIgnoredChecks: true, derivePrDisposition(...).commentMergeStateHeld is false while
isCommentMergeStateHeld("unstable") is true. Two exported functions in the single-interpretation module
disagree, and one of them documents that they cannot.

The divergence is live, not theoretical. The planner resolves the flag from real data:

// src/settings/agent-actions.ts:1167
unstableExplainedByIgnoredChecks:
  input.ignoredCheckNonPassing !== undefined && input.ignoredCheckNonPassing.length > 0 && (input.nonRequiredCheckFailures ?? []).length === 0 && input.ciState !== "failed",

The comment surface does not — it is the only production caller of the state-only helper, and it passes the
raw string:

// src/queue/processors.ts:2188
...(mergeStateLabel ? { mergeStateHeld: isCommentMergeStateHeld(mergeStateLabel) } : {}),

That boolean is authoritative downstream: src/review/unified-comment.ts:394–src/review/unified-comment.ts:396
turns an otherwise-"ready" status into "held" whenever readiness.mergeStateHeld is true.

So for a PR whose mergeable_state is unstable explained solely by a check the maintainer listed in
gate.ignoredCheckRuns, the planner correctly does not hold it (that is what
unstableExplainedByIgnoredChecks was added for — see the field's own doc at
src/settings/pr-disposition.ts:87–src/settings/pr-disposition.ts:92, which cites the observed live case of
a PR held with reason "non-required check(s) not passing: Contributor trust"), while the public unified
comment still renders it held and refuses to say the PR is safe to merge. The ignoredCheckRuns setting is
half-effective in exactly the way that field was added to stop.

The invariant test that is supposed to pin this cannot catch it, because it only ever exercises the default
arm:

// test/unit/pr-disposition-invariants.test.ts:82
it("isCommentMergeStateHeld equals derivePrDisposition(...).commentMergeStateHeld for every raw state (equal by construction, pinned)", () => {
  for (const raw of RAW_STATES) {
    expect(isCommentMergeStateHeld(raw), `state=${String(raw)}`).toBe(
      derivePrDisposition(dispositionInput({ mergeableState: raw })).commentMergeStateHeld,
    );
  }
});

dispositionInput never sets unstableExplainedByIgnoredChecks, so the loop compares only the arm where the
two happen to still agree.

Requirements

• isCommentMergeStateHeld must accept the ignored-checks resolution as an explicit parameter and honour it,
  so it is genuinely equal by construction to derivePrDisposition(...).commentMergeStateHeld for every
  combination of raw state and that flag. Concretely: it must take a second argument (e.g.
  unstableExplainedByIgnoredChecks?: boolean | undefined) and return false for "unstable" when that
  argument is true, while every other raw state keeps its current result for both values of the argument.
  Omitting the argument must keep today's behaviour exactly (undefined ⇒ unstable holds), so no caller that
  cannot resolve it changes.
• src/queue/processors.ts:2188 must pass the same resolution the planner computes at
  src/settings/agent-actions.ts:1167. buildPublicCommentMergeFacts already holds the inputs that formula
  needs — args.liveCi.nonRequiredFailingDetails and the derived ciState
  (src/queue/processors.ts:2175–src/queue/processors.ts:2180) — plus whatever names the repo's ignored
  checks; the ignored-check list must be threaded to this call site rather than re-derived from a different
  source, so the two surfaces read one resolution and not two.
• The "equal by construction" JSDoc at src/settings/pr-disposition.ts:155 must remain TRUE after the change,
  and the invariant test must actually prove it across both values of the flag.
• Must NOT change: assessMergeableState's mapping (src/settings/pr-disposition.ts:39), the
  heldForManualReview / wouldApprove / wouldMerge formulas, or the deliberate asymmetry documented at
  src/settings/pr-disposition.ts:143–src/settings/pr-disposition.ts:147 whereby "behind" downgrades the
  comment but never holds the planner.

⚠️ Required pattern: the corrected isCommentMergeStateHeld must compute its unstable term with the
identical expression derivePrDisposition uses at src/settings/pr-disposition.ts:128
(mergeable === "unstable" && flag !== true), so the two cannot drift again. What does NOT satisfy this
issue: (a) deleting isCommentMergeStateHeld and having src/queue/processors.ts:2188 call
derivePrDisposition with a fabricated PrDispositionInput — that surface has no access to the hold
inputs and would have to invent them; (b) weakening the JSDoc to say the two are "approximately" equal or
deleting the "equal by construction" sentence instead of restoring the property; (c) a test-only PR that
adds the missing flag case and asserts the CURRENT divergent values; (d) changing
src/review/unified-comment.ts's zero-import contract by importing pr-disposition.ts there — the bridge
must keep passing a resolved boolean.

Deliverables

• isCommentMergeStateHeld (src/settings/pr-disposition.ts) takes the ignored-checks resolution and
  returns false for ("unstable", true), true for ("unstable", false) and ("unstable", undefined),
  and its existing result for "dirty", "behind", "clean", "blocked", "unknown", undefined
  under both values.
• src/queue/processors.ts:2188 passes the resolution computed by the same formula as
  src/settings/agent-actions.ts:1167, from buildPublicCommentMergeFacts's own inputs.
• test/unit/pr-disposition-invariants.test.ts:82's "equal by construction" loop is extended to iterate
  unstableExplainedByIgnoredChecks over [undefined, false, true] for every raw state, and passes.
• A regression test named for this bug at test/unit/pr-disposition-invariants.test.ts asserting that for
  mergeableState: "unstable" with unstableExplainedByIgnoredChecks: true,
  isCommentMergeStateHeld("unstable", true) and
  derivePrDisposition({...base, mergeableState: "unstable", unstableExplainedByIgnoredChecks: true}).commentMergeStateHeld
  are both false — the exact pair that disagrees today.
• A test at test/unit/pr-disposition-invariants.test.ts asserting that
  deriveUnifiedStatus-equivalent behaviour is preserved for a "ready" status with
  mergeStateHeld: false (renders ready) and mergeStateHeld: true (renders held), so the
  src/review/unified-comment.ts:394 branch stays covered on both arms.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that fixes isCommentMergeStateHeld's signature but leaves src/queue/processors.ts:2188 passing only the
raw string, so the comment surface still holds the PR — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts and packages/loopover-engine/src/**/*.ts; both src/settings/pr-disposition.ts and
src/queue/processors.ts are measured. The change introduces a new && flag !== true conjunct in
isCommentMergeStateHeld — both arms need a test ("unstable" with the flag true, and "unstable" with
it false/undefined) — and a new conditional at the src/queue/processors.ts:2188 call site whose true and
false results must each be exercised through buildPublicCommentMergeFacts.

Expected Outcome

A repo that lists a check in gate.ignoredCheckRuns gets the full effect of that setting: the planner does
not hold the PR and the public review comment stops claiming it is held. The two exported functions in
pr-disposition.ts are once again equal by construction across every input, and the invariant test proves it
over the flag rather than only over the raw state — so the next field added to PrDispositionInput cannot
silently reintroduce the split.

Links & Resources

• src/settings/pr-disposition.ts:128 — unstableHolds, the term the helper omits
• src/settings/pr-disposition.ts:155 — the false "equal by construction" claim
• src/settings/agent-actions.ts:1167 — the planner's resolution of unstableExplainedByIgnoredChecks
• src/queue/processors.ts:2188 — the only production caller of the state-only helper
• src/review/unified-comment.ts:394 — where mergeStateHeld turns "ready" into "held"
• test/unit/pr-disposition-invariants.test.ts:82 — the invariant test that cannot currently fail

[linear-code]

JSO-7030