emrg: two gates answer a PR they never looked at with a pass

scripts/check-merge-pairs.py

@@ -92,7 +92,8 @@
        land badly - that is why this is 0 here and 3 in `check-merge-sequence.py`, where
        a conflict leaves later steps of a chain unmeasured)
     1  at least one ordered pair merges cleanly and lands a tree that fails the guards
-    2  the question could not be answered (git/gh/guard failure) - fail loud; never
+    2  the question could not be answered (git/gh/guard failure, or a requested PR
+       number whose head cannot be fetched) - fail loud; never
        report "no dangerous pair" about pairs that were not measured
 """
 
@@ -205,6 +206,16 @@ def main(argv: list[str] | None = None) -> int:
         # itself's twin and buy the same answer twice, and the ordering makes the output
         # diffable between runs.
         numbers = sorted(set(args.prs or seq._open_pr_numbers(args.repo)))
+        # A number that is not an open PR (closed, merged, or mistyped) fetches no
+        # head. It used to be taken verbatim: a lone number produces no pairs at all,
+        # so the loop below never ran, the summary line claimed "1 PR(s) -> 0 ordered
+        # pair(s)" about a PR that does not exist, and the run exited 0 - "no
+        # dangerous pair" about a PR that was never measured, which is the reading the
+        # exit-code contract above rules out. Resolving each head once, here, makes
+        # such a number fail loud as rc 2. The resolved heads are reused below, so
+        # this costs no extra fetches - and every pair is now measured against one
+        # snapshot of each head rather than a fresh fetch per pair.
+        heads = {n: seq._fetch_head(n) for n in numbers}
     except MeasurementError as exc:
         print(f"could not measure: {exc}", file=sys.stderr)
         return 2
@@ -229,12 +240,12 @@ def main(argv: list[str] | None = None) -> int:
         for a, b in pairs:
             try:
                 if a not in first_step:
-                    first_step[a] = seq._merge_commit(base, seq._fetch_head(a))
+                    first_step[a] = seq._merge_commit(base, heads[a])
                 landed_a = first_step[a]
                 if landed_a is None:
                     blocked += 1
                     continue
-                landed_b = seq._merge_commit(landed_a, seq._fetch_head(b))
+                landed_b = seq._merge_commit(landed_a, heads[b])
                 if landed_b is None:
                     blocked += 1
                     continue

scripts/check-pr-base.py

@@ -59,7 +59,8 @@
 0  no open PR's base is a dead end (bases either reach master, or are an open
    PR's head - the latter reported as LIVE)
 1  at least one open PR is based on a dead end - retarget it before merging
-2  the question could not be answered (gh failed, response unparseable)
+2  the question could not be answered (gh failed, response unparseable, or a
+   requested PR number is not among the open PRs)
 
 Never reports a count it could not obtain: a check that guesses "OK" when it
 could not read the state is worse than no check, because the failure it hides is
@@ -238,6 +239,25 @@ def main(argv: list[str] | None = None) -> int:
 
     if args.prs:
         wanted = set(args.prs)
+        # A requested number that is not among the open PRs used to be dropped in
+        # silence: the loop below then had nothing to classify, so the tool printed
+        # nothing and returned 0 - a clean bill for a PR it never looked at. That is
+        # the one thing its own contract forbids (never guess "OK" when the state
+        # could not be read), and it is indistinguishable from a pass at the exit
+        # code, which is what every caller reads. The number is answered as
+        # unmeasurable instead: this tool lists open PRs, so a closed, merged or
+        # mistyped number has no base it could classify.
+        missing = sorted(wanted - {p["number"] for p in prs})
+        if missing:
+            print(
+                "cannot determine PR bases: "
+                + ", ".join(f"#{n}" for n in missing)
+                + " is not among the open PRs (closed, merged, or nonexistent) - "
+                "this tool classifies open PRs only, and will not report a clean bill "
+                "for one it did not look at",
+                file=sys.stderr,
+            )
+            return 2
         prs = [p for p in prs if p["number"] in wanted]
 
     dead = 0

tests/test_check_merge_pairs.py

@@ -549,3 +549,69 @@ def fake_rev_parse(ref):
 
     assert rc == 2, err
     assert refreshed == [], "a base refused as a stray local branch must not be refreshed"
+
+
+def test_a_requested_pr_whose_head_cannot_be_fetched_is_not_a_pass(mod, monkeypatch, capsys):
+    """A lone unmeasurable number must not become a reassurance about zero pairs.
+
+    Measured before the fix (`cyc20260919-173431`): `check-merge-pairs.py 99999` printed
+    `pairs: 1 PR(s) -> 0 ordered pair(s)` and `no ordered pair merges cleanly into a
+    failing tree`, exit 0. The number was taken verbatim, a single PR forms no pair, the
+    loop never ran, and the summary line *became* the verdict - "no dangerous pair" about
+    a PR that does not exist. The exit-code contract above rules that reading out, and the
+    empty-pair case reaches it without a single `_fetch_head` call, which is why the head
+    is resolved for every requested number before any pair is formed.
+
+    The existing tests are the other direction: explicit numbers that *do* resolve are
+    still measured, so this cannot be satisfied by refusing every explicit selection.
+    """
+
+    def missing(n):
+        raise mod.seq.MeasurementError(f"could not fetch PR #{n}")
+
+    monkeypatch.setattr(mod.seq, "_rev_parse", lambda ref: BASE)
+    monkeypatch.setattr(mod.seq, "_refresh_base", lambda ref: None)
+    monkeypatch.setattr(mod.seq, "_fetch_head", missing)
+    monkeypatch.setattr(mod.seq, "_merge_commit", lambda a, b: pytest.fail("no pair to merge"))
+    rc = mod.main(["99999"])
+    captured = capsys.readouterr()
+
+    assert rc == 2, captured.out
+    assert "no ordered pair" not in captured.out, (
+        "a verdict about unformed pairs is the fail-open shape this tool forbids"
+    )
+    assert "99999" in captured.err, captured.err
+
+
+def test_every_requested_head_is_resolved_once_before_any_pair(mod, monkeypatch, capsys):
+    """The heads are fetched once per run, not once per pair - and before the pairs.
+
+    Two properties in one measurement, because they are the same edit: the up-front
+    resolution is what turns an unfetchable number into rc 2, and reusing its result is
+    what keeps that from costing a fetch per pair. Asserted by count, with three PRs so
+    each head is an element of more than one ordered pair.
+    """
+    fetched: list[int] = []
+    chain = {
+        (BASE, C1): C1, (BASE, C2): C2, (BASE, C3): C3,
+        (C1, C2): C2, (C1, C3): C3, (C2, C1): C1, (C2, C3): C3, (C3, C1): C1, (C3, C2): C2,
+    }
+    heads = {1: C1, 2: C2, 3: C3}
+
+    def fetch(n):
+        fetched.append(n)
+        return heads[n]
+
+    monkeypatch.setattr(mod.seq, "_rev_parse", lambda ref: BASE)
+    monkeypatch.setattr(mod.seq, "_refresh_base", lambda ref: None)
+    monkeypatch.setattr(mod.seq, "_fetch_head", fetch)
+    monkeypatch.setattr(mod.seq, "_merge_commit", lambda a, b: chain.get((a, b)))
+    monkeypatch.setattr(mod.seq, "_guard_verdict", lambda tree, workdir: (True, "documents 1564"))
+    monkeypatch.setattr(
+        mod.seq, "_run", lambda argv, cwd=None: _FakeProc(argv[-1].removesuffix("^{tree}"))
+    )
+    rc = mod.main(["1", "2", "3"])
+    capsys.readouterr()
+
+    assert rc == 0, "three clean, healthy pairs"
+    assert sorted(fetched) == [1, 2, 3], f"each head once, not once per pair: {fetched}"

tests/test_check_pr_base.py

@@ -219,6 +219,40 @@ def test_selecting_one_pr_filters_the_rest(self, mod, monkeypatch, capsys) -> No
         assert "#2" in out and "#1" not in out
 
 
+    def test_a_requested_pr_that_is_not_open_exits_2(self, mod, monkeypatch, capsys) -> None:
+        """A number this tool never looked at must not come back as a clean bill.
+
+        Measured before the fix (`cyc20260919-173431`): `check-pr-base.py 99999` printed
+        **nothing** and exited **0**. The requested number was filtered out of the
+        open-PR list, so the loop had nothing to classify and `dead` stayed 0 - at the
+        exit code, byte-identical to "every base reaches master", which is exactly what
+        a Committer reads before merging. The sibling gates all answer rc 2 for the same
+        input (`check-vote-count.py`, `check-merge-freshness.py`, `check-merge-landing-diff.py`,
+        `check-merge-order.py`, `check-merge-sequence.py`); this one was the outlier.
+        """
+        prs = [{"number": 1151, "baseRefName": "master", "headRefName": "h1"}]
+        self._wire(mod, monkeypatch, prs, {}, lambda sha: True)
+        rc = mod.main(["--repo", "owner/repo", "99999"])
+        err = capsys.readouterr().err
+
+        assert rc == 2, "a PR that was never looked at is not a pass"
+        assert "#99999" in err and "not among the open PRs" in err, err
+
+    def test_asking_about_an_open_pr_is_still_answered(self, mod, monkeypatch, capsys) -> None:
+        """The other direction: the refusal is about *membership*, not about selecting.
+
+        Without this, a mutant that answered rc 2 for every explicit selection would pass
+        the test above while making `check-pr-base.py <N>` useless.
+        """
+        prs = [{"number": 1151, "baseRefName": "master", "headRefName": "h1"}]
+        self._wire(mod, monkeypatch, prs, {}, lambda sha: True)
+        rc = mod.main(["--repo", "owner/repo", "1151"])
+        out = capsys.readouterr().out
+
+        assert rc == 0, out
+        assert "#1151" in out and "base is master" in out, out
+
+
 class TestRealInvocationSurface:
     """The script must be runnable as a tool, and must not silently pass on error."""