diff --git a/scripts/README.md b/scripts/README.md index 732392dbb..eeae0d7a2 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -205,7 +205,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 or 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, 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. | | `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. | diff --git a/scripts/pr_review.py b/scripts/pr_review.py index adad84f4f..5a47703ac 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -118,10 +118,11 @@ 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 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 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. 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 @@ -143,11 +144,11 @@ 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 no check has 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 `CHECKS PARTIALLY UNREAD` in the - digest, the status says the check may sit there. + `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 + `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 diff --git a/scripts/pr_review_checks.py b/scripts/pr_review_checks.py index 5c8d8643f..01f19f154 100644 --- a/scripts/pr_review_checks.py +++ b/scripts/pr_review_checks.py @@ -311,7 +311,10 @@ def checks_never_ran( 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: - return "no check has posted, and a DIRTY merge runs no pull_request workflow until resolved" + 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" + ) unmet = ", ".join(required_unregistered(pr, nodes, required)) unread = checks_unread(nodes) where = ", sits in the rollup's unread part," if unread else "" diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index aec58f356..e33f9b84d 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -3592,7 +3592,10 @@ def test_an_empty_rollup_holds_any_merge_but_a_conflicted_one(self) -> None: self.assertEqual(code, self.cli(["wait", "7", "--timeout", "0"])) if code == 45: self.assertIn("status=CHECKS_MISSING", self.out.getvalue()) - self.assertIn("but no check has posted", self.out.getvalue()) + self.assertIn( + "but no check has posted, 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.""" @@ -3742,7 +3745,8 @@ def test_a_released_head_exits_by_what_its_read_shows(self) -> None: self.wire_history([hist_review(7, OVERVIEW + "\n" + COVERED)]) with mock.patch.object(pr_review.time, "sleep"): self.assertEqual(code, self.cli(["wait", "7", "--timeout", "1"])) - self.assertIn(reason, self.out.getvalue()) + if reason: + self.assertIn(reason, self.out.getvalue()) if label == "another app": self.assertNotIn("checks and statuses first", self.out.getvalue()) if code == 0: