fix(review_receipt): only an independent SHIP naming the exact head is a receipt

scripts/ci/review_receipt.py

@@ -20,19 +20,17 @@
 on any head of the pull request (a receipt line, the marker or a change request) may be a newer BLOCK, so a later
 BLOCK beats an earlier SHIP wherever it sits. A first line that says it carries (CARRIED: the word in the reviewer
 label, "Review (codex; carried): SHIP at <new head>", or "carried from"/"carried over" on the line) is a lane's claim
-that an older head's SHIP carries across a base merge, not a verdict: it counts as the head's receipt only when no
-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. 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).
+that an older head's SHIP carries across a base merge, not a verdict, and it is never a receipt, at any risk tier.
+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; cmux#36322 (2026-11-05) landed on a SHIP at 71d93871 carried across a clean base merge to
+8e3e59a4, and the review of 8e3e59a4 itself then came back BLOCK. A base merge can change what a patch does even with
+no conflict, so only an independent SHIP naming the exact head counts. next-autoland (cmuxterm-hq#7261) and
+land_ready / next_review (leo-workspace#3542) apply the same rule.
 
 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 <sha>", "NOT SHIP" or "Do
 not merge", or a comment starting "Hold..." refuses even after a SHIP on the head; a carried SHIP is not a review
-there either, so it never overtakes one. When the 30-review or 40-comment window comes back full and reaches
+there either, so it never overtakes one or counts as a SHIP. When the 30-review or 40-comment window comes back full and reaches
 back no further than the SHIP, something newer may be cut off, so that refuses too. Comments starting `<!--`
 are tools' marked comments, not reviews. Nothing waives a refusal. A base merge (merge-main.sh, update-branch)
 makes a new head, which needs its own receipt.
@@ -267,30 +265,18 @@ def newest_independent(pr: dict) -> tuple[str, str, str] | None:
     return max(found, key=lambda f: f[0]) if found else 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
-    that stands behind the receipt was given, so window_cut_off checks nothing newer than it was cut off."""
-    found = head_verdicts(pr, head)
-    own = [f for f in found if f[1] != "carried"]
-    latest = newest_independent(pr)
-    if own:
-        at, kind, detail = max(own, key=lambda f: f[0])
-        blocks = [f for f in head_verdicts(pr, None) if f[1] == "block"]
-        if kind != "ship" or any(block[0] > at for block in blocks):
-            return None
-        return at, detail
-    carry = [f for f in found if f[1] == "carried"]
-    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]
+def ship_receipt(pr: dict, head: str) -> tuple[str, str] | None:
+    """(since, who) for the head's SHIP, else None. The newest independent verdict naming the exact head must be a
+    SHIP that no independent BLOCK on any head is newer than. A carried SHIP is never a receipt (cmux#36322). `since`
+    is when that SHIP was given, so window_cut_off checks nothing newer than it was cut off."""
+    own = [f for f in head_verdicts(pr, head) if f[1] != "carried"]
+    if not own:
+        return None
+    at, kind, detail = max(own, key=lambda f: f[0])
+    blocks = [f for f in head_verdicts(pr, None) if f[1] == "block"]
+    if kind != "ship" or any(block[0] > at for block in blocks):
+        return None
+    return at, detail
 
 
 def receipt(pr: dict, head: str) -> str:
@@ -302,7 +288,7 @@ def receipt(pr: dict, head: str) -> str:
 def window_cut_off(pr: dict, since: str | None) -> str:
     """The list ("reviews" or "comments") that came back full and reaches back no further than `since`, so items
     newer than `since` may have been cut off from it; "" when both lists reach past it. With None, any full
-    window is ambiguous: a carried receipt needs complete history to prove no verdict names its head."""
+    window is ambiguous."""
     for key, cap, stamp in (("reviews", REVIEWS_WINDOW, "submittedAt"), ("comments", COMMENTS_WINDOW, "createdAt")):
         nodes = [node for node in (pr.get(key) or {}).get("nodes") or [] if node]
         # A pending review has no timestamp; it says nothing about how far back the window reaches.
@@ -317,14 +303,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}"
-    high = high_risk(pr)
-    found = ship_receipt(pr, head, exact=high)
+    found = ship_receipt(pr, head)
     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"
+        newer = (f"; the newest independent verdict is a BLOCK ({latest[2]}), so no older SHIP stands"
                  if latest and latest[1] == "block" else "")
-        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}"
+        carry = ("; a carried SHIP is not a review of this head, so it never counts"
+                 if any(f[1] == "carried" for f in head_verdicts(pr, head)) else "")
+        return "", f"no SHIP review receipt on {head}{newer}{carry}"
     cut = window_cut_off(pr, found[0])
     if cut:
         size = REVIEWS_WINDOW if cut == "reviews" else COMMENTS_WINDOW
@@ -382,8 +368,7 @@ def main(argv: list[str] | None = None, run: Runner = _gh) -> int:
             raise Refused(f"{why}: after an independent review of this head, comment "
                           f"'Review (subagent, correctness first): SHIP at {args.sha}'. A newer BLOCK, FIX, hold "
                           "or change request on any head wins over an older SHIP, and a base merge (merge-main.sh or "
-                          "update-branch) makes a new head that needs its own receipt; a carried SHIP counts only "
-                          "while the newest independent verdict is a SHIP")
+                          "update-branch) makes a new head that needs its own receipt; a carried SHIP never counts")
     except RateLimited as error:
         print(f"rate-limited: {error}", file=sys.stderr)
         return 3

skills/cmux-review/SKILL.md

@@ -39,14 +39,13 @@ a bad default in the next nightly. `main` is nightly: stack fixes, do not revert
 Post the verdict as a PR comment whose first line is the receipt, and put nothing
 before it: `Review (<reviewer>): SHIP at <full 40-character head sha>` (or `BLOCK`,
 `FIX`). Findings go below that line. A SHIP counts only for the exact head it
-names. A receipt that carries an older head's SHIP to a new head across a base
-merge is not a review. Mark it in the reviewer label:
-`Review (<reviewer>; carried): SHIP at <new head sha>`. The checkers
-(`scripts/ci/review_receipt.py`, next-autoland) treat a first line as carried when
-the label says carried, or when the line says "carried from" or "carried over". A
-carried SHIP counts only while the newest independent verdict on the PR is a
-SHIP. Do not use "carry", "carries", "carried" or "carrying" in a real review's
-label, or "carried from" / "carried over" on its first line.
+names. A base merge makes a new head that needs its own review: a SHIP never
+carries to it, even when the merge was clean. The checkers
+(`scripts/ci/review_receipt.py`, next-autoland) treat a first line as a carry
+claim, and never as a receipt, when the reviewer label says carried, or when the
+line says "carried from" or "carried over". Do not use "carry", "carries",
+"carried" or "carrying" in a real review's label, or "carried from" / "carried
+over" on its first line.
 
 ## Adversarial review
 

tests/test_review_receipt.py

@@ -362,10 +362,31 @@ def test_a_carried_ship_does_not_clear_a_hold_or_a_bare_block(self):
             with self.subTest(line=line):
                 self.refused([self.ship(self.A, 1), comment(line, at=self.at(5)), self.carried(self.C, self.A, 9)])
 
-    def test_a_clean_carry_with_no_block_passes(self):
-        self.assertIn("carried", self.passes([self.ship(self.A, 1), self.carried(self.C, self.A, 9)]))
-        self.passes([self.box("SHIP", self.A, 1), self.carried(self.C, self.A, 9)])
-        self.passes([self.carried(self.C, self.A, 9)], [review("APPROVED", at=self.at(1), oid=self.A)])
+    def test_a_clean_carry_with_no_block_refuses(self):
+        """cmux#36322 (2026-11-05): SHIP at A (71d93871), a clean base merge made B (8e3e59a4), and the SHIP carried to
+        B landed it; the exact-head review of B then came back BLOCK. A carry is never a receipt, at any risk tier."""
+        for why in (self.refused([self.ship(self.A, 1)], head=self.B),
+                    self.refused([self.ship(self.A, 1), self.carried(self.B, self.A, 9)], head=self.B),
+                    self.refused([self.box("SHIP", self.A, 1), self.carried(self.B, self.A, 9)], head=self.B),
+                    self.refused([self.carried(self.B, self.A, 9)], [review("APPROVED", at=self.at(1), oid=self.A)],
+                                 head=self.B)):
+            self.assertIn(f"no SHIP review receipt on {self.B}", why)
+        why = self.refused([self.ship(self.A, 1), self.carried(self.B, self.A, 9)], head=self.B)
+        self.assertIn("a carried SHIP is not a review of this head", why)
+        # the merge commit's own SHIP lands it; its own BLOCK holds
+        self.passes([self.ship(self.A, 1), self.carried(self.B, self.A, 5), self.ship(self.B, 9)], head=self.B)
+        self.refused([self.ship(self.A, 1), self.box("BLOCK", self.B, 9)], head=self.B)
+
+    def test_the_command_refuses_a_carry_across_a_clean_base_merge(self):
+        payload = pr([self.ship(self.A, 1), self.carried(self.B, self.A, 9)], head=self.B)
+        with tempfile.NamedTemporaryFile("w", suffix=".json") as handle:
+            json.dump(payload, handle)
+            handle.flush()
+            err = io.StringIO()
+            with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(err):
+                status = receipt.main(["manaflow-ai/cmux", "36322", self.B, "--pr-file", handle.name])
+        self.assertEqual(status, 1)
+        self.assertIn("a carried SHIP never counts", err.getvalue())
 
     def test_a_ship_whose_text_says_carries_is_a_review(self):
         """cmux#27632 (2026-10-29): "Review (independent review subagent): SHIP at <head>. ... the warning carries the
@@ -401,9 +422,9 @@ def test_a_new_independent_review_after_the_block_decides(self):
         self.passes([*base, self.box("SHIP", self.C, 9)])
         # its BLOCK holds
         self.refused([*base, self.box("BLOCK", self.C, 9)])
-        # an independent SHIP newer than the BLOCK (the owner's fix, re-reviewed at A, then merged) carries again
-        self.passes([self.ship(self.A, 1), *block, self.ship(self.A, 7), self.carried(self.C, self.A, 9)])
-        # but a carried SHIP is never the newest independent verdict, so it never clears the BLOCK itself
+        # an independent SHIP newer than the BLOCK at A does not carry to C either
+        self.refused([self.ship(self.A, 1), *block, self.ship(self.A, 7), self.carried(self.C, self.A, 9)])
+        # and a carried SHIP is never the newest independent verdict, so it never clears the BLOCK itself
         self.refused([self.ship(self.A, 1), *block, self.carried(self.A, self.A, 7), self.carried(self.C, self.A, 9)])
 
     def test_a_ship_on_another_head_cannot_hide_a_newer_block_of_the_heads_ship(self):
@@ -437,7 +458,7 @@ def test_the_command_names_the_block_that_refused(self):
 
 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."""
+    (a lane's or next-review's carry across a base merge) counts for it. Since cmux#36322 that holds at every tier."""
     REVIEWED = "ab" * 20
 
     def carried(self, line):
@@ -447,14 +468,12 @@ def carried(self, line):
     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)):
+            for labels in (("risk:high",), ("RISK:HIGH", "ready-to-land"), ("risk:normal",), ("risk:low",), ()):
                 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)
+                    self.assertEqual(who, "", why)
+                    self.assertIn("a carried SHIP is not a review of this 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)