fix(scripts): a merged pull request is not a mergeability still being computed

changelog.d/3232-merge-blockers-already-merged-is-terminal.md

@@ -0,0 +1,14 @@
+### Fixed: `check_merge_blockers.py` names an already-merged pull request instead of asking for a re-read that cannot settle
+
+A pull request that had already merged reported as `merge-state-unknown`, owed by
+nobody, with the remedy "Re-read the pull request: mergeability is computed on
+demand and settles on a later read". It never settles - a merged pull request is
+closed, so `mergeable` stays null for good, and the report described a wait with
+no terminating condition while the merge had already landed. Measured on #3219
+and #3230 minutes after they squashed: both `closed` / `merged: true` /
+`mergeable: null`, byte-identical in every field the check read to open #3205's
+genuine recompute at the same moment. The `merged` key was already in the payload
+`resolve_state` fetches and was simply not read. There is now a terminal
+`already-merged` outcome, reported ahead of every ruleset rule and alone, because
+"0 of 1 approvals" is not a rule the merge left unsatisfied. An open pull
+request's null still reads as an uncomputed mergeability, unchanged.

scripts/check_merge_blockers.py

@@ -86,7 +86,22 @@
     run just after a merge is precisely when it is null. Reported as *gating*,
     because reading the null as clean is how a conflicted branch is reported as
     owed by a reviewer -- measured on #1035, which read
-    ``pusher-only-approval`` while it was ``DIRTY``. Re-read to resolve it.
+    ``pusher-only-approval`` while it was ``DIRTY``. Re-read to resolve it,
+    which works only because the pull request is open: the entry below reads the
+    same null for a reason no read settles.
+
+``already-merged``
+    Nobody, and unlike every entry above there is no state left to reach. A
+    merged pull request is closed, so ``mergeable`` stays null for good -- the
+    same null the entry above reads as "still computing", on the one pull
+    request whose own merge invalidated it. Reported ahead of every ruleset rule
+    and short-circuiting them: once the change is on the base, "0 of 1
+    approvals" is not a rule the merge left unsatisfied, it is a question about
+    a closed pull request. Measured on #3219 and #3230, which read ``closed`` /
+    ``merged: true`` / ``mergeable: null`` while open #3205 read ``mergeable:
+    null`` for the honest reason at the same moment -- byte-identical in every
+    field this check had been reading, so the ``merged`` key it already fetches
+    is what separates them (#3231).
 
 ``required-check-cancelled``
     A **maintainer**, by re-running the run. A cancelled run is not a verdict
@@ -222,6 +237,7 @@ def _load_sibling() -> Any:
 # Outcome names. Ordered here the way they bind in practice, which is also the
 # order they are reported in: a conflict makes the approval question moot, and
 # an unresolved thread makes it moot for a different reason.
+ALREADY_MERGED = "already-merged"
 MERGE_CONFLICT = "merge-conflict"
 MERGE_STATE_UNKNOWN = "merge-state-unknown"
 DRAFT = "draft"
@@ -244,6 +260,7 @@ def _load_sibling() -> Any:
 ANYONE = "anyone, by attempting the merge"
 
 _OWED_BY: dict[str, str] = {
+    ALREADY_MERGED: NOBODY,
     MERGE_CONFLICT: AUTHOR,
     MERGE_STATE_UNKNOWN: NOBODY,
     DRAFT: AUTHOR,
@@ -270,7 +287,12 @@ def _load_sibling() -> Any:
 # because the cost is asymmetric -- reporting the approval rule as the next
 # action on a branch that turns out to be conflicted burns an approval, and
 # reporting it as necessary-but-not-sufficient costs one re-read (#2585).
-_GATING: frozenset[str] = frozenset({MERGE_CONFLICT, MERGE_STATE_UNKNOWN, DRAFT})
+# ``already-merged`` gates for the strongest reason of the three: there is no
+# rule left to assess rather than one that cannot be assessed yet. It is in this
+# set so the report names it as the next action's subject and prints "owed by
+# nobody, on `already-merged`" rather than the ungated wording "the answer is not
+# in yet", which is the sentence #3231 measured as the misleading one.
+_GATING: frozenset[str] = frozenset({ALREADY_MERGED, MERGE_CONFLICT, MERGE_STATE_UNKNOWN, DRAFT})
 
 
 # The outcomes a scheduled author-side pass can act on without anyone else.
@@ -353,6 +375,12 @@ class PullRequestState:
     mergeable: bool | None
     merge_state: str
     unresolved_threads: int
+    # Whether the pull request has already merged. Lives among the defaulted
+    # fields for the mechanical reason that the seven above it carry no default,
+    # not because it is peripheral: it is read before any of them. ``False`` is
+    # the safe default because it is the only value that leaves every rule below
+    # evaluated.
+    merged: bool = False
     check_conclusions: dict[str, str | None] = field(default_factory=dict)
     # Each check suite's conclusion on the head, or ``None`` for a census that
     # was not read. ``()`` is a positive observation -- zero suites exist -- and
@@ -472,6 +500,29 @@ def evaluate(state: PullRequestState, rules: Ruleset) -> tuple[Blocker, ...]:
     """
     found: list[Blocker] = []
 
+    # Ahead of every rule below, the draft check included: a merged pull request
+    # has no unsatisfied rule to name. Separated from the uncomputed
+    # mergeability further down because the two are indistinguishable in every
+    # other field this check reads -- a merged pull request is closed, so
+    # ``mergeable`` stays null for good, and #2586's "re-read, it settles on a
+    # later read" then prescribes a wait with no terminating condition (#3231).
+    #
+    # Returned alone rather than added to ``found`` and gated: gating says the
+    # rules below cannot be assessed *yet*, and here there is nothing left to
+    # assess in any future read.
+    if state.merged:
+        return (
+            Blocker(
+                ALREADY_MERGED,
+                "(not a ruleset rule)",
+                f"The pull request is already merged into {state.base_ref}, so no "
+                f"rule is outstanding and no later read will change that. Cached "
+                f"merge state is {state.merge_state or 'unknown'}, and mergeability "
+                f"is null here because the pull request is closed rather than "
+                f"because GitHub is still computing it.",
+            ),
+        )
+
     if state.draft:
         found.append(
             Blocker(
@@ -810,6 +861,9 @@ def resolve_state(repo: str, pr: int, token: str) -> PullRequestState:
         head_sha=head_sha,
         base_ref=base_ref,
         draft=bool(payload.get("draft")),
+        # Already in this payload and previously unread, which is what let a
+        # merged pull request report as an uncomputed mergeability (#3231).
+        merged=bool(payload.get("merged")),
         mergeable=payload.get("mergeable"),
         merge_state=str(payload.get("mergeable_state") or ""),
         unresolved_threads=resolve_unresolved_threads(repo, pr, token),

tests/test_merge_blockers.py

@@ -548,6 +548,131 @@ def test_an_unknown_mergeability_does_not_read_as_the_stale_state_case() -> None
     assert "Attempt the merge" not in rendered
 
 
+# --------------------------------------------------------------------------
+# The third reading of a null mergeability: the pull request already merged.
+#
+# #3219 and #3230 were squashed at 12:26:39Z and 12:37:58Z on 2026-09-05, and
+# `GET /repos/strands-labs/robots/pulls/{n}` then reported, for both:
+#
+#     state=closed  merged=true  mergeable=None  mergeable_state=unknown
+#
+# which is what the cluster above reads as "GitHub is still computing". Open
+# #3205, read in the same minute, was `merged=false` with an identically null
+# `mergeable` -- so the honest transient #2586 was written for is live at the
+# same time, and every field the check read before this was the same on all
+# three. `merged` is the one that separates them, and it was already in the
+# payload `resolve_state` fetches. See issue #3231.
+# --------------------------------------------------------------------------
+
+
+@pytest.mark.parametrize(
+    ("merged", "expected"),
+    [
+        pytest.param(True, "already-merged", id="merged-3219-and-3230"),
+        pytest.param(False, "merge-state-unknown", id="open-3205"),
+    ],
+)
+def test_the_same_null_mergeability_gets_two_verdicts_by_whether_it_merged(merged: bool, expected: str) -> None:
+    """The measured pair, differing in the one field that was not being read.
+
+    Pre-fix both rows report ``merge-state-unknown``, so the two are one verdict
+    and the merged row is told to re-read a value that is already final.
+    """
+    st = state(merged=merged, mergeable=None, merge_state="unknown")
+    assert outcomes(mod.evaluate(st, MAIN)) == [expected]
+
+
+def test_a_merged_pull_request_names_no_rule_underneath_it() -> None:
+    """Every rule is moot, not merely unanswerable, so none is listed.
+
+    Deliberately hostile inputs: no approval, two unresolved threads and no
+    check conclusion at all. On an open pull request each is a blocker the
+    report must name. On a merged one, naming them invents obligations against a
+    closed pull request -- the change is already on the base, so there is no
+    approval that could admit it and no thread whose resolution could.
+    """
+    st = state(
+        merged=True,
+        mergeable=None,
+        merge_state="unknown",
+        approvers=(),
+        unresolved_threads=2,
+        check_conclusions={},
+    )
+    assert outcomes(mod.evaluate(st, MAIN)) == [mod.ALREADY_MERGED]
+
+
+def test_a_merged_pull_request_is_owed_by_nobody_and_is_not_a_finding() -> None:
+    """Owed by nobody for the opposite reason to ``required-check-pending``.
+
+    That one is waiting; this one is finished. Not a finding either way, because
+    the exit status is for what a scheduled author-side pass can act on alone,
+    and there is nothing left to act on.
+    """
+    blocker = mod.primary(mod.evaluate(state(merged=True, mergeable=None), MAIN))
+    assert blocker.outcome == mod.ALREADY_MERGED
+    assert blocker.owed_by == mod.NOBODY
+    assert blocker.is_finding is False
+    assert blocker.is_gating is True
+
+
+def test_a_merged_pull_request_is_not_prescribed_a_read_that_cannot_settle() -> None:
+    """The measured cost: the remedy that describes a wait with no end.
+
+    A merged pull request is closed, so ``mergeable`` never becomes non-null and
+    "settles on a later read" is false however many times it is re-read. The
+    ungated wording is refused for the same reason -- "the answer is not in yet"
+    is the sentence that read as reassuring while the answer had been in for
+    eleven minutes -- and so is #2574's merge attempt, which would be a second
+    merge of a merged branch.
+    """
+    st = state(merged=True, mergeable=None, merge_state="unknown")
+    rendered = mod.render(st, MAIN, mod.evaluate(st, MAIN), "o/r")
+    assert "Re-read" not in rendered
+    assert "the answer is not in yet" not in rendered
+    assert "Attempt the merge" not in rendered
+    assert f"Next action is owed by {mod.NOBODY}, on `{mod.ALREADY_MERGED}`" in rendered
+    assert "already merged" in rendered
+
+
+def test_a_merged_pull_request_is_reported_alone_rather_than_as_a_gate() -> None:
+    """Gating says "not yet"; this says "never again", so nothing trails it.
+
+    ``_next_action`` appends "an approval is necessary but not sufficient while
+    it stands" whenever a gating blocker has siblings. On a merged pull request
+    no approval is necessary at all, so that sentence would be false -- it is
+    absent because the blocker is returned alone, which this pins rather than
+    assuming.
+    """
+    st = state(merged=True, mergeable=None, approvers=(), unresolved_threads=1)
+    rendered = mod.render(st, MAIN, mod.evaluate(st, MAIN), "o/r")
+    assert "necessary but not sufficient" not in rendered
+
+
+def test_the_merged_verdict_does_not_reach_an_open_pull_request() -> None:
+    """A guard against fixing this by reporting every null as merged.
+
+    #2586 exists because reading the null as clean handed a ``DIRTY`` branch to
+    a reviewer (#1035). Reading it as merged would be the same error wearing the
+    opposite sign, so the whole open cluster above must be untouched.
+    """
+    for mergeable in (True, False, None):
+        st = state(merged=False, mergeable=mergeable)
+        assert mod.ALREADY_MERGED not in outcomes(mod.evaluate(st, MAIN))
+
+
+def test_the_merged_key_is_read_from_the_payload_that_already_carried_it() -> None:
+    """``resolve_state`` must populate the field, or the report never sees it.
+
+    The seam #3231 measured was not the classifier but the read: ``evaluate``
+    cannot separate the two nulls if every state reaching it says ``merged=
+    False``. Graded on the constructed state rather than the transport, so the
+    fixture is the REST payload's own key names.
+    """
+    source = _SCRIPT.read_text(encoding="utf-8")
+    assert 'merged=bool(payload.get("merged"))' in source
+
+
 def test_the_next_action_line_names_one_owner_when_a_gating_blocker_is_present() -> None:
     blockers = mod.evaluate(state(mergeable=False, approvers=()), MAIN)
     rendered = mod.render(state(mergeable=False, approvers=()), MAIN, blockers, "o/r")