Hold a Rollup of Only Unreadable Nodes as an Empty One

scripts/README.md

@@ -208,7 +208,7 @@ uv run --no-project --script scripts/pr_review.py cleanup 452 --repo ptr727/Proj
 | `41` | `wait` | The newest round is a refusal or a quota state, its kind printed in the status line, and in `refusal=` for a refusal on this pull request. |
 | `42` | `status`, `wait` | The round covering the head states partial coverage. Or a non-refusal head round exists while an earlier round states partial, and one reads `UNKNOWN` or none states coverage. |
 | `44` | `wait` | The review loop closed, the merge reads `BLOCKED`, and a required check is in a shape no wait clears. |
-| `45` | `wait` | An open attested held head that nothing holds, its merge other than `CLEAN`, `UNSTABLE`, or `HAS_HOOKS`. Its rollup was unreadable, a `DIRTY` merge has none posted, or a `DIRTY` or `BEHIND` merge lacks a required check or has it from another app, judged on rules read and a whole rollup. |
+| `45` | `wait` | An open attested held head that nothing holds, its merge other than `CLEAN`, `UNSTABLE`, or `HAS_HOOKS`. Its rollup was unreadable, or a `DIRTY` merge has no check posted that the reader can parse. Or a `DIRTY` or `BEHIND` merge lacks a required check or has it from another app, judged on rules read and a whole rollup. |
 | `49` | `wait` | No Copilot round covers the head and none was requested, on a pull request into a branch other than the default with no attestation. |
 | `50` | `status`, `wait` | The round covering the head opens on a verdict other than the clean one while it counts no finding. Or a head round carries a part the reader cannot place, and nothing acknowledges its fingerprint. |
 | `60` to `63` | `reply` | No thread matched, more than one did, the reply returned no comment url, or the resolve did not report the thread resolved. |
@@ -218,7 +218,7 @@ uv run --no-project --script scripts/pr_review.py cleanup 452 --repo ptr727/Proj
 
 `1` is a GraphQL read that failed and ended the run, and `2` is argparse's own usage error.
 
-`wait` exits `30` with `status=CHECKS_PENDING` on an attested held head while its checks can still move the merge at the timeout. It reads them in this order. A pull request no longer open holds nothing, and a merge reading `UNKNOWN` holds. A `BLOCKED` merge ends the poll on the first read where a required check is stuck in a shape waiting does not clear. A check running long or gated is not such a shape. A failed one is the exception while a newer run of its workflow on the head has not finished, which the digest names `CHECK SUPERSEDED`. Next, a workflow run on the head that has not completed holds, where the read reaches it. The read reaches the head's last 50 check suites and those of its first `CHECKS_WINDOW` rollup contexts. A run's later jobs enter the rollup only as their `needs:` finish, so a green rollup proves nothing while one is open. The status line names those runs. Next, a merge reading `CLEAN`, `UNSTABLE`, or `HAS_HOOKS` ends the poll. Otherwise a required check still settling holds, one running long included, and so does any settling check while no required one has posted. A `BLOCKED` merge also holds while a check the base branch's rules require has not registered. That hold needs the base branch's rules read and a rollup that is not `CHECKS TRUNCATED`. Where the rollup holds a type the reader cannot parse, its status line says that check may sit in the unread part. An empty rollup holds under `BLOCKED`, `BEHIND`, or `DRAFT`. An open head released short of those holds exits `45` with `status=CHECKS_MISSING` where a check it is owed is missing from the read. Each such case falls under a merge other than `CLEAN`, `UNSTABLE`, or `HAS_HOOKS`. One is a payload with no commit matching the head, which the digest names `CHECKS UNREADABLE` and which reads no run and no check. Another is a rollup with no check posted, which only a `DIRTY` merge releases, since a conflicted pull request runs no `pull_request` workflow. The last is a `DIRTY` or `BEHIND` merge whose rollup lacks a context the base branch's rules require, or carries it from another app. That one needs the rules read and a rollup that is not `CHECKS TRUNCATED`. Where the rollup holds a type the reader cannot parse, which the digest names `CHECKS PARTIALLY UNREAD`, the status says the check may sit there. Exit `0` can then leave a required check unregistered only under unread rules, a cut rollup, a `DRAFT` merge, or a pull request not open. `--min-rounds N` counts what `status` prints as `rounds=`, every round on every head, refusals included, so N is the `rounds=` a `status` read before a re-request.
+`wait` exits `30` with `status=CHECKS_PENDING` on an attested held head while its checks can still move the merge at the timeout. It reads them in this order. A pull request no longer open holds nothing, and a merge reading `UNKNOWN` holds. A `BLOCKED` merge ends the poll on the first read where a required check is stuck in a shape waiting does not clear. A check running long or gated is not such a shape. A failed one is the exception while a newer run of its workflow on the head has not finished, which the digest names `CHECK SUPERSEDED`. Next, a workflow run on the head that has not completed holds, where the read reaches it. The read reaches the head's last 50 check suites and those of its first `CHECKS_WINDOW` rollup contexts. A run's later jobs enter the rollup only as their `needs:` finish, so a green rollup proves nothing while one is open. The status line names those runs. Next, a merge reading `CLEAN`, `UNSTABLE`, or `HAS_HOOKS` ends the poll. Otherwise a required check still settling holds, one running long included, and so does any settling check while no required one has posted. A `BLOCKED` merge also holds while a check the base branch's rules require has not registered. That hold needs the base branch's rules read and a rollup that is not `CHECKS TRUNCATED`. Where the rollup holds a type the reader cannot parse, its status line says that check may sit in the unread part. An empty rollup holds under `BLOCKED`, `BEHIND`, or `DRAFT`, as does one holding only types the reader cannot parse. An open head released short of those holds exits `45` with `status=CHECKS_MISSING` where a check it is owed is missing from the read. Each such case falls under a merge other than `CLEAN`, `UNSTABLE`, or `HAS_HOOKS`. One is a payload with no commit matching the head, which the digest names `CHECKS UNREADABLE` and which reads no run and no check. Another is a rollup with no check posted that the reader can parse, which only a `DIRTY` merge releases. That is because a conflicted pull request runs no `pull_request` workflow. The last is a `DIRTY` or `BEHIND` merge whose rollup lacks a context the base branch's rules require, or carries it from another app. That one needs the rules read and a rollup that is not `CHECKS TRUNCATED`. Where the rollup holds a type the reader cannot parse, which the digest names `CHECKS PARTIALLY UNREAD`, the status says the check may sit there. Exit `0` can then leave a required check unregistered only under unread rules, a cut rollup, a `DRAFT` merge, or a pull request not open. `--min-rounds N` counts what `status` prints as `rounds=`, every round on every head, refusals included, so N is the `rounds=` a `status` read before a re-request.
 
 `wait` exits `40` when no Copilot review covers the head and the reviewer has said why in another shape. A plain comment newer than its newest review carries no commit, so it satisfies no coverage check whatever it says. Its body prints whole because its wording is the only thing separating a refusal from a remark. A comment older than the newest review is spent. Every connection reads the newest `WINDOW` nodes rather than the reviewer's own, so `window_blind` guards both sides. Blind on comments reads as `answered_outside_review=unknown`, and blind on reviews reports nothing and lets the wait keep polling. A request that returns success and adds neither a pending reviewer nor a review-request event also exits `40`, as `status=REQUEST_NOT_RECORDED`. So does a reviewer writing under a renamed login, as `status=REVIEWER_LOGIN_DRIFT`.
 

scripts/pr_review.py

@@ -138,11 +138,11 @@
            every workflow run the read reaches on that head has then completed. The read reaches the
            head's last 50 check suites and those of its first CHECKS_WINDOW rollup contexts. A
            CLEAN, UNSTABLE, or HAS_HOOKS merge owes no required check. Under any other merge, an
-           open attested held head nothing holds exits 45 where its rollup was unreadable or, under
-           DIRTY, none posted. A DIRTY or BEHIND one also exits 45 where a required check is
-           unregistered or from another app. Exit 0 can then leave a required check unregistered
-           only under unread rules, a `CHECKS TRUNCATED` rollup, a DRAFT merge, or a pull request
-           not open.
+           open attested held head nothing holds exits 45 where its rollup was unreadable. So does
+           a DIRTY one where no check this can read has posted. A DIRTY or BEHIND one also exits 45
+           where a required check is unregistered or from another app. Exit 0 can then leave a
+           required check unregistered only under unread rules, a `CHECKS TRUNCATED` rollup, a
+           DRAFT merge, or a pull request not open.
            30 = still pending at the timeout. On a held head it is any hold
            `status=CHECKS_PENDING` names, a required check still settling and a workflow run still
            open among them. Where the rollup holds a type this cannot read, a BLOCKED hold's note
@@ -166,16 +166,17 @@
            never posted, running far past the job's cost, held by its environment, or failed. On
            a held head a check running long or held by its environment polls to 30 instead.
            45 = an open attested held head that nothing holds, its merge other than CLEAN,
-           UNSTABLE, or HAS_HOOKS. Either the payload carries no commit matching the head, named
-           `CHECKS UNREADABLE` in the digest, or a DIRTY merge has no check posted. Or the merge
-           reads DIRTY or BEHIND and a check the base branch's rules require has not registered,
-           or came from another app. That last needs the rules read and a rollup not
-           `CHECKS TRUNCATED`. Where the rollup holds a type this cannot read, named
+           UNSTABLE, or HAS_HOOKS. One case is a payload with no commit matching the head, named
+           `CHECKS UNREADABLE` in the digest. Another is a DIRTY merge with no check posted that
+           this can read. The last is a DIRTY or BEHIND merge where a check the base branch's
+           rules require has not registered, or came from another app. That last needs the rules read and a
+           rollup not `CHECKS TRUNCATED`. Where the rollup holds a type this cannot read, named
            `CHECKS PARTIALLY UNREAD` in the digest, the status says the check may sit there.
            `status=CHECKS_MISSING` names which. A DIRTY merge runs no pull_request workflow until
            its conflict is resolved. A required check not yet registered holds a BLOCKED merge to
            30 instead, where the rules and the head's rollup were read, not `CHECKS TRUNCATED`. An
-           empty rollup read off the head holds a BLOCKED, BEHIND, or DRAFT one.
+           empty rollup read off the head holds a BLOCKED, BEHIND, or DRAFT one, as does a
+           rollup holding only types this cannot read.
            49 = no Copilot round covers this head and none was requested. The situation is a
            pull request merging into a branch other than the default with no attestation vouching
            for the head. 64 = the write scope could not
@@ -1716,7 +1717,7 @@ def read_live() -> tuple[bool, dict | None, list[str], bool]:
         ):
             print(
                 "status=CHECKS_PENDING an attested local pass covers this head, and by the "
-                "timeout a check had not concluded, none had posted, a workflow run on the head "
+                "timeout a check had not concluded, none this can read had posted, a workflow run on the head "
                 "had not completed, a check the base branch requires had not registered or came "
                 "from another app, or the merge was still UNKNOWN, so the merge is not ready yet: "
                 "wait again, or read the checks above"

scripts/pr_review_checks.py

@@ -262,7 +262,7 @@ def held_checks_open(
     only as their `needs:` finish, so a green rollup proves nothing while one is open. A CLEAN,
     UNSTABLE, or HAS_HOOKS merge has no required check outstanding, so an unrequired check does not
     hold it. Otherwise a required check still settling holds, as does any settling check while no
-    required check has posted. A rollup with no check holds while the merge
+    required check has posted. A rollup with no check it can read holds while the merge
     reads BLOCKED, BEHIND, or DRAFT. An UNKNOWN merge holds whatever the rollup carries, and a pull
     request no longer open holds nothing.
 
@@ -290,7 +290,8 @@ def held_checks_open(
         return True
     if merge == "BLOCKED" and required_unregistered(pr, nodes, required):
         return True
-    return not nodes and merge in ("BLOCKED", "BEHIND", "DRAFT") and not checks_unreadable(pr)
+    unposted = not checks_tally(nodes)[1]
+    return unposted and merge in ("BLOCKED", "BEHIND", "DRAFT") and not checks_unreadable(pr)
 
 
 def checks_never_ran(
@@ -300,16 +301,18 @@ def checks_never_ran(
 
     A CLEAN, UNSTABLE, or HAS_HOOKS merge is owed nothing. The caller passes `required` only for a
     merge whose release judges registration, and a context is judged only on the head's own
-    rollup, not cut.
+    rollup, not cut. A rollup holding only types this cannot read is judged as one with no check.
     """
     if pr.get("state", "OPEN") != "OPEN" or pr.get("mergeStateStatus") in MERGE_UNGATED:
         return ""
     if checks_unreadable(pr):
         return "the payload carries no commit matching the head, so read the check runs on the head commit"
-    if not nodes:
+    if not checks_tally(nodes)[1]:
+        only = ", ".join(checks_unread(nodes))
+        held = f" that this can read, the rollup holding only {only}" if only else ""
         return (
-            "no check has posted, so resolve the head's conflict with its base, since a DIRTY "
-            "merge runs no pull_request workflow until then"
+            f"no check has posted{held}, so resolve the head's conflict with its base, since a "
+            "DIRTY merge runs no pull_request workflow until then"
         )
     unmet = ", ".join(required_unregistered(pr, nodes, required))
     unread = checks_unread(nodes)
@@ -376,8 +379,8 @@ def checks_truncated(pr: dict) -> bool:
 
 
 def checks_unread(nodes: list[dict]) -> list[str]:
-    """The rollup typenames this could not read, in the order they appeared."""
-    return [n["unreadable"] for n in nodes if n.get("unreadable")]
+    """The rollup typenames this could not read, each named once and sorted."""
+    return sorted({n["unreadable"] for n in nodes if n.get("unreadable")})
 
 
 def checks_unreadable(pr: dict) -> bool:
@@ -419,7 +422,7 @@ def check_lines(
     unread = checks_unread(checks)
     if unread:
         lines.append(
-            f"  CHECKS PARTIALLY UNREAD ({', '.join(sorted(set(unread)))}): the rollup "
+            f"  CHECKS PARTIALLY UNREAD ({', '.join(unread)}): the rollup "
             "carries a context type this cannot normalize, so it is absent from the "
             "tally and the stuck reading alike and neither speaks for it"
         )

tests/test_pr_review.py

@@ -3716,6 +3716,26 @@ def test_an_empty_rollup_holds_any_merge_but_a_conflicted_one(self) -> None:
                         self.out.getvalue(),
                     )
 
+    def test_a_rollup_of_only_unreadable_nodes_holds_as_an_empty_one(self) -> None:
+        """A node this cannot parse is no check read, so it must not release what an empty rollup holds."""
+        for merge, code in (("BLOCKED", 30), ("BEHIND", 30), ("DRAFT", 30), ("DIRTY", 45)):
+            with self.subTest(merge=merge):
+                unread = [{"__typename": "UnknownContext"}]
+                pr = self.into(payload([review(oid=OLD)], merge=merge, checks=unread), attest=True)
+                self.answer(pr)
+                self.wire_history([hist_review(7, OVERVIEW + "\n" + COVERED)])
+                self.out.seek(0)
+                self.out.truncate()
+                with mock.patch.object(pr_review.time, "sleep"):
+                    self.assertEqual(code, self.cli(["wait", "7", "--timeout", "0"]))
+                if code == 45:
+                    self.assertIn("status=CHECKS_MISSING", self.out.getvalue())
+                    self.assertIn(
+                        "no check has posted that this can read, the rollup holding only "
+                        "UnknownContext, so resolve the head's conflict",
+                        self.out.getvalue(),
+                    )
+
     def test_an_optional_check_does_not_hold_once_the_required_gate_concluded(self) -> None:
         """A merge BLOCKED on a thread is not waiting on a check nothing requires."""
         optional = {
@@ -6643,6 +6663,23 @@ def test_checks_never_ran_judges_only_an_open_gated_merge(self) -> None:
         self.assertIn("no check has posted", pr_review_checks.checks_never_ran(pr, [], frozenset()))
         nodes = pr_review_checks.check_nodes(payload([], merge="DIRTY", checks=[check()]))
         self.assertEqual("", pr_review_checks.checks_never_ran(pr, nodes, frozenset()))
+        unknown = {"__typename": "UnknownContext"}
+        mixed = pr_review_checks.check_nodes(payload([], checks=[check(), unknown]))
+        self.assertEqual("", pr_review_checks.checks_never_ran(pr, mixed, frozenset()))
+        unread = pr_review_checks.check_nodes(payload([], checks=[unknown, unknown]))
+        self.assertIn(
+            "no check has posted that this can read, the rollup holding only UnknownContext, so",
+            pr_review_checks.checks_never_ran(pr, unread, frozenset()),
+        )
+        zeta, alpha = {"__typename": "ZetaContext"}, {"__typename": "AlphaContext"}
+        mixed_types = pr_review_checks.check_nodes(payload([], checks=[zeta, alpha, zeta]))
+        self.assertEqual(
+            ["AlphaContext", "ZetaContext"], pr_review_checks.checks_unread(mixed_types)
+        )
+        self.assertIn(
+            "the rollup holding only AlphaContext, ZetaContext, so",
+            pr_review_checks.checks_never_ran(pr, mixed_types, frozenset()),
+        )
         later = pr_review_checks.checks_never_ran(pr, nodes, frozenset({("Later", None)}))
         self.assertIn("has not registered or came from another app, Later", later)
         for ungated in ({**pr, "mergeStateStatus": "CLEAN"}, {**pr, "state": "MERGED"}):