diff --git a/scripts/ci/review_receipt.py b/scripts/ci/review_receipt.py index d53dd0553969..8c6794ca11e5 100644 --- a/scripts/ci/review_receipt.py +++ b/scripts/ci/review_receipt.py @@ -24,7 +24,10 @@ independent verdict names the head and the newest independent verdict on the pull request is a SHIP (next-autoland's ship_receipt, cmuxterm-hq#4975). cmux#27189 landed 2026-10-29 on "Review (Astra; carried): SHIP at 127a023e" while the box review of its pre-merge head had returned BLOCK. Carried receipts also require both history windows to be below their limits: a full window could hide a verdict naming -the target head, which a carry cannot replace. +the target head, which a carry cannot replace. A risk:high pull request never merges on a carried receipt: it needs an +independent verdict of its own on the exact head (cmux-ad, 2026-10-30, decision 788, as docs/ci/risk-tiers.md says; +a base merge can change what a risk:high patch does even with no conflict). next-autoland applies the same rule +(cmuxterm-hq#5420, exact_receipt). Before that, as next-autoland's review_state() judges it, the newest review-like word on any head must not say wait: a change request, "Review (codex): BLOCK", "Verdict: BLOCK", a bare first line "BLOCK at ", "NOT SHIP" or "Do @@ -75,7 +78,7 @@ CARRIED = re.compile(r"^\W*(?:independent\s+)?review\s*\([^)\n]*\bcarr(?:y|ies|ied|ying)\b[^)\n]*\)\s*:" r"|\bcarried\s+(?:from|over)\b", re.I) QUERY = ("query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){" - "headRefOid " + "headRefOid labels(first:100){nodes{name}} " f"reviews(last:{REVIEWS_WINDOW})" "{nodes{state submittedAt body authorAssociation author{__typename login} commit{oid}}} " f"comments(last:{COMMENTS_WINDOW})" @@ -207,7 +210,13 @@ def newest_independent(pr: dict) -> tuple[str, str, str] | None: return max(found, key=lambda f: f[0]) if found else None -def ship_receipt(pr: dict, head: str) -> tuple[str, str] | None: +def high_risk(pr: dict) -> bool: + """The pull request carries the risk:high tier label (any case).""" + return any(((node or {}).get("name") or "").lower() == "risk:high" + for node in (pr.get("labels") or {}).get("nodes") or []) + + +def ship_receipt(pr: dict, head: str, exact: bool = False) -> tuple[str, str] | None: """(since, who) for the head's SHIP, else None. The newest independent verdict naming the head must be a SHIP that no independent BLOCK on any head is newer than; with none naming the head, a carried SHIP naming it stands only while the newest independent verdict on the pull request is a SHIP. `since` is when the independent SHIP @@ -222,8 +231,8 @@ def ship_receipt(pr: dict, head: str) -> tuple[str, str] | None: return None return at, detail carry = [f for f in found if f[1] == "carried"] - if not carry or not latest or latest[1] != "ship" or window_cut_off(pr, None): - return None + if exact or not carry or not latest or latest[1] != "ship" or window_cut_off(pr, None): + return None # `exact`: a carried SHIP is no receipt at all (risk:high, decision 788) return latest[0], max(carry, key=lambda f: f[0])[2] @@ -251,12 +260,14 @@ def decide(pr: dict, head: str) -> tuple[str, str]: state, detail = review_state(pr) if state in ("blocked", "held"): return "", f"no SHIP review receipt on {head} can merge while the newest review says wait: {detail}" - found = ship_receipt(pr, head) + high = high_risk(pr) + found = ship_receipt(pr, head, exact=high) if not found: latest = newest_independent(pr) newer = (f"; the newest independent verdict is a BLOCK ({latest[2]}), so no older or carried SHIP stands" if latest and latest[1] == "block" else "") - return "", f"no SHIP review receipt on {head}{newer}" + exact = "; risk:high needs an independent review of this exact head, and no carried SHIP counts" if high else "" + return "", f"no SHIP review receipt on {head}{newer}{exact}" cut = window_cut_off(pr, found[0]) if cut: size = REVIEWS_WINDOW if cut == "reviews" else COMMENTS_WINDOW diff --git a/tests/test_review_receipt.py b/tests/test_review_receipt.py index e59b5673c1d7..e2ed09a153d4 100644 --- a/tests/test_review_receipt.py +++ b/tests/test_review_receipt.py @@ -30,8 +30,9 @@ def review(state="COMMENTED", body="", at="2026-10-21T05:00:00Z", oid=HEAD, auth "commit": {"oid": oid}} -def pr(comments=(), reviews=(), head=HEAD): - return {"headRefOid": head, "comments": {"nodes": list(comments)}, "reviews": {"nodes": list(reviews)}} +def pr(comments=(), reviews=(), head=HEAD, labels=()): + return {"headRefOid": head, "comments": {"nodes": list(comments)}, "reviews": {"nodes": list(reviews)}, + "labels": {"nodes": [{"name": name} for name in labels]}} SHIP = f"Review (subagent, correctness first): SHIP at {HEAD}" @@ -362,6 +363,36 @@ def test_the_command_names_the_block_that_refused(self): self.assertIn("the newest independent verdict is a BLOCK (autoreview BLOCK by teamleaderleo)", err.getvalue()) +class RiskHighExactHead(unittest.TestCase): + """cmux-ad, 2026-10-30, decision 788: a risk:high pull request needs a receipt on the exact head; no carried SHIP + (a lane's or next-review's carry across a base merge) counts for it. risk:normal and risk:low keep the carry.""" + REVIEWED = "ab" * 20 + + def carried(self, line): + return pr([comment(f"Review (subagent, correctness first): SHIP at {self.REVIEWED}", at="2026-10-21T05:00:00Z"), + comment(line, at="2026-10-21T06:00:00Z")], labels=self.labels) + + def test_a_carried_ship_never_clears_risk_high(self): + for line in (f"Review (next-review patch-id; carried): SHIP at {HEAD}", + f"Review (subagent): SHIP at {HEAD} (carried from {self.REVIEWED[:12]})"): + for labels, passes in ((("risk:high",), False), (("RISK:HIGH", "ready-to-land"), False), + (("risk:normal",), True), (("risk:low",), True), ((), True)): + with self.subTest(line=line, labels=labels): + self.labels = labels + who, why = receipt.decide(self.carried(line), HEAD) + self.assertEqual(bool(who), passes, why) + if not passes: + self.assertIn("risk:high needs an independent review of this exact head", why) + + def test_an_own_ship_on_the_exact_head_clears_risk_high(self): + who, why = receipt.decide(pr([comment(SHIP)], labels=("risk:high",)), HEAD) + self.assertIn("SHIP", who, why) + self.assertIn("approved", receipt.decide(pr(reviews=[review("APPROVED")], labels=("risk:high",)), HEAD)[0]) + # A newer BLOCK on another head still takes it back. + block = comment(f"Review (subagent): BLOCK at {OTHER}", at="2026-10-21T07:00:00Z") + self.assertEqual(receipt.decide(pr([comment(SHIP), block], labels=("risk:high",)), HEAD)[0], "") + + class Command(unittest.TestCase): def run_main(self, payload, sha=HEAD): with tempfile.NamedTemporaryFile("w", suffix=".json") as handle: @@ -406,7 +437,8 @@ def run(args): self.assertEqual(len(calls), 1) query = " ".join(calls[0]) # next-autoland's windows, so the two tools judge the same reviews and comments. - for field in ("__typename", "authorAssociation", "commit{oid}", "reviews(last:30)", "comments(last:40)"): + for field in ("__typename", "authorAssociation", "commit{oid}", "reviews(last:30)", "comments(last:40)", + "labels(first:100){nodes{name}}"): self.assertIn(field, query) def test_a_rate_limited_read_exits_3(self):