feat: hard-stop review mutations on already-landed PR heads (Closes #292)

A reviewer workflow approved and attempted to merge PR #278 although its
head commit was already an ancestor of master, ending in a Gitea 405 and
manual reconciliation. Add the missing pre-mutation gate:

- assess_already_landed_review_gate classifies the pinned candidate
  head against the fetched target branch: NORMAL_REVIEW_CANDIDATE,
  ALREADY_LANDED_RECONCILE_REQUIRED, or GATE_NOT_PROVEN (fail closed).
  Already-landed PRs block approval and the merge API, allow
  request-changes only for a real blocker, and require a reconciliation
  handoff (PR number/title, candidate head SHA, target branch + SHA,
  ancestor proof, linked issue status, recommended action, capability
  proof for close/comment mutations). Unchecked ancestry, invalid or
  missing SHAs, and a head changed since pinning all fail closed.
  Mergeability stays a separate gate.

- assess_already_landed_report_state rejects APPROVED / MERGED /
  READY_TO_MERGE wording and missing ALREADY_LANDED_RECONCILE_REQUIRED
  state when the session's gate fired, complementing the text-marker
  rules from #327 which need the report to admit the landed state.

Tests cover already-landed, normal, non-mergeable, changed-head,
unchecked-ancestry, and invalid-SHA gate paths plus all report-state
verdicts.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

review_proofs.py

@@ -3363,6 +3363,172 @@ def assess_reviewer_baseline_validation_proof(
     }
 
 
+# ---------------------------------------------------------------------------
+# Already-landed review gate (#292)
+# ---------------------------------------------------------------------------
+
+ALREADY_LANDED_STATE = "ALREADY_LANDED_RECONCILE_REQUIRED"
+
+ALREADY_LANDED_HANDOFF_FIELDS = (
+    "PR number/title",
+    "candidate head SHA",
+    "target branch",
+    "target branch SHA",
+    "ancestor proof",
+    "linked issue status",
+    "recommended reconciliation action",
+    "capability proof for close/comment mutations",
+)
+
+_FORBIDDEN_LANDED_STATES_RE = re.compile(
+    r"review decision\s*:\s*approved?\b|"
+    r"merge result\s*:\s*merged\b|"
+    r"\bready[_ ]to[_ ]merge\b|"
+    r"\bmerge[d]?\s*:\s*success\b",
+    re.IGNORECASE,
+)
+
+
+def assess_already_landed_review_gate(
+    *,
+    pr_number: int | None,
+    candidate_head_sha: str | None,
+    target_branch: str | None,
+    target_branch_sha: str | None,
+    head_is_ancestor_of_target: bool | None,
+    live_head_sha: str | None = None,
+    real_blocker: bool = False,
+    mergeable: bool | None = None,
+) -> dict:
+    """#292: hard gate before any review mutation on an already-landed PR.
+
+    Runs after PR selection and head pinning, before approval, request-
+    changes, or the merge API. *head_is_ancestor_of_target* is the result
+    of an ancestry check of *candidate_head_sha* against the freshly
+    fetched target branch at *target_branch_sha* (e.g. ``git merge-base
+    --is-ancestor``). ``None`` means the check was not run — the gate then
+    fails closed.
+
+    *live_head_sha*, when provided, must equal *candidate_head_sha*; a
+    changed head invalidates the pinned ancestry result until re-checked.
+
+    *mergeable* is accepted for caller convenience but never consulted:
+    conflict/mergeability handling is a separate gate.
+    """
+    del mergeable  # ancestry gate only; mergeability is a separate gate
+    reasons: list[str] = []
+
+    if not isinstance(pr_number, int) or pr_number <= 0:
+        reasons.append("PR number missing or invalid (#292)")
+    if not _FULL_SHA.match((candidate_head_sha or "").strip()):
+        reasons.append(
+            "candidate head SHA is not a full 40-hex commit SHA (#292)"
+        )
+    if not (target_branch or "").strip():
+        reasons.append("target branch missing (#292)")
+    if not _FULL_SHA.match((target_branch_sha or "").strip()):
+        reasons.append(
+            "target branch SHA is not a full 40-hex commit SHA; fetch the "
+            "target branch and record its SHA before the ancestry check "
+            "(#292)"
+        )
+    if live_head_sha is not None and candidate_head_sha and (
+        live_head_sha.strip() != candidate_head_sha.strip()
+    ):
+        reasons.append(
+            "PR head changed since the ancestry check was pinned; re-fetch "
+            "and re-run the already-landed gate (#292)"
+        )
+    if head_is_ancestor_of_target is None:
+        reasons.append(
+            "ancestry of the PR head against the target branch was not "
+            "checked; the already-landed gate must run before any review "
+            "mutation (#292)"
+        )
+
+    if reasons:
+        return {
+            "state": "GATE_NOT_PROVEN",
+            "approve_allowed": False,
+            "request_changes_allowed": False,
+            "merge_api_allowed": False,
+            "reconciliation_handoff_required": False,
+            "required_handoff_fields": (),
+            "reasons": reasons,
+            "safe_next_action": (
+                "fetch the target branch, pin the live PR head, run the "
+                "ancestry check, then re-run this gate"
+            ),
+        }
+
+    if head_is_ancestor_of_target:
+        return {
+            "state": ALREADY_LANDED_STATE,
+            "approve_allowed": False,
+            "request_changes_allowed": bool(real_blocker),
+            "merge_api_allowed": False,
+            "reconciliation_handoff_required": True,
+            "required_handoff_fields": ALREADY_LANDED_HANDOFF_FIELDS,
+            "reasons": [
+                f"PR #{pr_number} head {candidate_head_sha} is already an "
+                f"ancestor of {target_branch} @ {target_branch_sha}; the PR "
+                "is reconciliation-only (#292)"
+            ],
+            "safe_next_action": (
+                "stop before review mutation and emit an "
+                f"{ALREADY_LANDED_STATE} reconciliation handoff; any PR/issue "
+                "close or comment requires exact capability proof"
+            ),
+        }
+
+    return {
+        "state": "NORMAL_REVIEW_CANDIDATE",
+        "approve_allowed": True,
+        "request_changes_allowed": True,
+        "merge_api_allowed": True,
+        "reconciliation_handoff_required": False,
+        "required_handoff_fields": (),
+        "reasons": [],
+        "safe_next_action": "proceed with the normal review workflow gates",
+    }
+
+
+def assess_already_landed_report_state(
+    report_text: str | None,
+    *,
+    gate_fired: bool,
+) -> dict:
+    """#292: reject APPROVED/MERGED/READY_TO_MERGE after the gate fired.
+
+    *gate_fired* comes from the session's own gate result, so a report
+    that omits the already-landed markers entirely still fails closed —
+    unlike the text-only #327 wording rules this does not depend on the
+    report admitting the PR was already landed.
+    """
+    if not gate_fired:
+        return {"complete": True, "block": False, "reasons": []}
+
+    text = report_text or ""
+    reasons: list[str] = []
+
+    if _FORBIDDEN_LANDED_STATES_RE.search(text):
+        reasons.append(
+            "report claims APPROVED/MERGED/READY_TO_MERGE although the "
+            "already-landed gate fired (#292)"
+        )
+    if ALREADY_LANDED_STATE.lower() not in text.lower():
+        reasons.append(
+            f"report must state {ALREADY_LANDED_STATE} when the "
+            "already-landed gate fired (#292)"
+        )
+
+    return {
+        "complete": not reasons,
+        "block": bool(reasons),
+        "reasons": reasons,
+    }
+
+
 # ---------------------------------------------------------------------------
 # Identity disclosure (#305)
 # ---------------------------------------------------------------------------

tests/test_review_proofs.py

@@ -23,6 +23,8 @@ sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.par
 from review_proofs import (  # noqa: E402
     ISSUE_SELECTION_CONTINUATION_EXPLICIT,
     ISSUE_SELECTION_REPRESENTED_BY_OPEN_PR,
+    assess_already_landed_report_state,
+    assess_already_landed_review_gate,
     assess_author_pr_report,
     assess_reviewer_baseline_validation_proof,
     assess_capability_evidence,
@@ -2281,5 +2283,139 @@ class TestReviewerBaselineValidationProof(unittest.TestCase):
         self.assertFalse(report["baseline_validation_proven"])
 
 
+class TestAlreadyLandedReviewGate(unittest.TestCase):
+    """Issue #292: PR head already on target blocks approval and merge."""
+
+    def _gate(self, **overrides):
+        kwargs = {
+            "pr_number": 278,
+            "candidate_head_sha": PINNED,
+            "target_branch": "master",
+            "target_branch_sha": OTHER,
+            "head_is_ancestor_of_target": True,
+            "live_head_sha": PINNED,
+        }
+        kwargs.update(overrides)
+        return assess_already_landed_review_gate(**kwargs)
+
+    def test_already_landed_blocks_approval_and_merge(self):
+        result = self._gate()
+        self.assertEqual(result["state"], "ALREADY_LANDED_RECONCILE_REQUIRED")
+        self.assertFalse(result["approve_allowed"])
+        self.assertFalse(result["merge_api_allowed"])
+        self.assertFalse(result["request_changes_allowed"])
+        self.assertTrue(result["reconciliation_handoff_required"])
+
+    def test_already_landed_with_real_blocker_allows_request_changes(self):
+        result = self._gate(real_blocker=True)
+        self.assertEqual(result["state"], "ALREADY_LANDED_RECONCILE_REQUIRED")
+        self.assertFalse(result["approve_allowed"])
+        self.assertFalse(result["merge_api_allowed"])
+        self.assertTrue(result["request_changes_allowed"])
+
+    def test_normal_candidate_passes_gate(self):
+        result = self._gate(head_is_ancestor_of_target=False)
+        self.assertEqual(result["state"], "NORMAL_REVIEW_CANDIDATE")
+        self.assertTrue(result["approve_allowed"])
+        self.assertTrue(result["merge_api_allowed"])
+        self.assertFalse(result["reconciliation_handoff_required"])
+
+    def test_non_mergeable_normal_pr_still_passes_this_gate(self):
+        # Mergeability/conflicts are separate gates; ancestry gate only
+        # decides already-landed vs normal candidate.
+        result = self._gate(
+            head_is_ancestor_of_target=False, mergeable=False)
+        self.assertEqual(result["state"], "NORMAL_REVIEW_CANDIDATE")
+        self.assertTrue(result["request_changes_allowed"])
+
+    def test_unchecked_ancestry_blocks_review_mutations(self):
+        result = self._gate(head_is_ancestor_of_target=None)
+        self.assertEqual(result["state"], "GATE_NOT_PROVEN")
+        self.assertFalse(result["approve_allowed"])
+        self.assertFalse(result["merge_api_allowed"])
+
+    def test_changed_head_since_pin_blocks_until_recheck(self):
+        result = self._gate(
+            head_is_ancestor_of_target=False, live_head_sha=OTHER)
+        self.assertEqual(result["state"], "GATE_NOT_PROVEN")
+        self.assertFalse(result["approve_allowed"])
+        self.assertTrue(any(
+            "head" in reason.lower() for reason in result["reasons"]
+        ))
+
+    def test_invalid_shas_block_gate(self):
+        result = self._gate(candidate_head_sha="deadbeef")
+        self.assertEqual(result["state"], "GATE_NOT_PROVEN")
+        self.assertFalse(result["approve_allowed"])
+
+        result = self._gate(target_branch_sha=None)
+        self.assertEqual(result["state"], "GATE_NOT_PROVEN")
+
+    def test_already_landed_handoff_lists_required_fields(self):
+        result = self._gate()
+        for field in (
+            "PR number/title",
+            "candidate head SHA",
+            "target branch",
+            "target branch SHA",
+            "ancestor proof",
+            "linked issue status",
+            "recommended reconciliation action",
+            "capability proof for close/comment mutations",
+        ):
+            self.assertIn(field, result["required_handoff_fields"])
+
+
+class TestAlreadyLandedReportState(unittest.TestCase):
+    """Issue #292: reports cannot say APPROVED after the gate fired."""
+
+    GOOD_REPORT = (
+        "Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED\n"
+        "Candidate head SHA: " + PINNED + "\n"
+        "Review decision: none\n"
+        "Merge result: none\n"
+    )
+
+    def test_gate_fired_with_reconcile_state_passes(self):
+        result = assess_already_landed_report_state(
+            self.GOOD_REPORT, gate_fired=True)
+        self.assertTrue(result["complete"])
+        self.assertFalse(result["block"])
+
+    def test_gate_fired_with_approved_state_blocks(self):
+        report = self.GOOD_REPORT.replace(
+            "Review decision: none", "Review decision: APPROVED")
+        result = assess_already_landed_report_state(report, gate_fired=True)
+        self.assertTrue(result["block"])
+
+    def test_gate_fired_with_merged_state_blocks(self):
+        report = self.GOOD_REPORT.replace(
+            "Merge result: none", "Merge result: MERGED")
+        result = assess_already_landed_report_state(report, gate_fired=True)
+        self.assertTrue(result["block"])
+
+    def test_gate_fired_with_ready_to_merge_blocks(self):
+        result = assess_already_landed_report_state(
+            self.GOOD_REPORT + "Current status: READY_TO_MERGE\n",
+            gate_fired=True)
+        self.assertTrue(result["block"])
+
+    def test_gate_fired_without_reconcile_state_blocks(self):
+        result = assess_already_landed_report_state(
+            "Current status: review complete.\n", gate_fired=True)
+        self.assertTrue(result["block"])
+        self.assertTrue(any(
+            "already_landed_reconcile_required" in reason.lower()
+            for reason in result["reasons"]
+        ))
+
+    def test_gate_not_fired_reports_pass_untouched(self):
+        result = assess_already_landed_report_state(
+            "Review decision: APPROVED\nMerge result: MERGED\n",
+            gate_fired=False)
+        self.assertTrue(result["complete"])
+        self.assertFalse(result["block"])
+
+
 if __name__ == "__main__":
     unittest.main()