Merge pull request 'feat: reject eligible/reviewed wording for already-landed PRs (Closes #298)' (#367) from feat/issue-298-already-landed-wording into master

review_proofs.py

@@ -1242,6 +1242,102 @@ def assess_workspace_mutation_consistency(report_text, observed_commands=None):
     }
 
 
+# ── Already-landed report wording (Issue #298) ───────────────────────────────
+#
+# An already-landed PR is reconciliation-only: it must never be called
+# eligible, its head SHA must never be called reviewed, and the legacy
+# eligible/reviewed/scratch-worktree handoff fields must not appear.
+# Reviewed-head claims in any report require validation + diff review to
+# have actually passed.
+
+ALREADY_LANDED_FORBIDDEN_PHRASES = (
+    "oldest eligible pr",
+    "next eligible pr",
+    "pinned reviewed head",
+    "scratch worktree used",
+)
+
+ALREADY_LANDED_ELIGIBILITY_CLASS = "ALREADY_LANDED_RECONCILE_REQUIRED"
+
+
+def assess_already_landed_report_wording(report_text, *, already_landed,
+                                         review_validated=False):
+    """#298: reject eligible/reviewed wording for already-landed PRs.
+
+    *already_landed* states whether the already-landed gate fired for the
+    selected PR. *review_validated* states whether validation and diff
+    review actually passed this session; only then may a reviewed head
+    SHA be populated in a non-already-landed report.
+
+    Returns {'complete', 'downgraded', 'reasons'}; fails closed.
+    """
+    text_lower = (report_text or "").lower()
+    fields = _report_labeled_fields(report_text)
+    reasons = []
+
+    reviewed_head = fields.get("reviewed head sha")
+    reviewed_head_populated = bool(
+        reviewed_head and not reviewed_head.strip().lower().startswith("none")
+    )
+    pinned_head_present = "pinned reviewed head" in fields
+
+    if already_landed:
+        for phrase in ALREADY_LANDED_FORBIDDEN_PHRASES:
+            if phrase in text_lower:
+                reasons.append(
+                    f"already-landed report must not use '{phrase}'; the PR "
+                    "is reconciliation-only, not review/merge eligible"
+                )
+
+        eligibility = fields.get("eligibility class", "")
+        if ALREADY_LANDED_ELIGIBILITY_CLASS not in eligibility.upper():
+            reasons.append(
+                "already-landed report missing 'Eligibility class: "
+                f"{ALREADY_LANDED_ELIGIBILITY_CLASS}'"
+            )
+
+        for required in ("oldest open pr requiring action",
+                         "candidate head sha"):
+            if required not in fields:
+                reasons.append(
+                    f"already-landed report missing '{required}' field"
+                )
+
+        if "reviewed head sha" not in fields:
+            reasons.append(
+                "already-landed report must state 'Reviewed head SHA: none'"
+            )
+        elif reviewed_head_populated:
+            reasons.append(
+                "already-landed report must not claim a reviewed head SHA; "
+                "no validation or diff review passed for this PR"
+            )
+
+        worktree_used = fields.get("review worktree used", "")
+        if worktree_used.strip().lower() not in ("false", "no"):
+            reasons.append(
+                "already-landed report must state 'Review worktree used: "
+                "false'"
+            )
+    elif not review_validated:
+        if pinned_head_present:
+            reasons.append(
+                "'Pinned reviewed head' populated before validation and "
+                "diff review passed"
+            )
+        if reviewed_head_populated:
+            reasons.append(
+                "reviewed head SHA populated before validation and diff "
+                "review passed"
+            )
+
+    return {
+        "complete": not reasons,
+        "downgraded": bool(reasons),
+        "reasons": reasons,
+    }
+
+
 def assess_role_boundary(proof=None, *, task_role=None, namespaces_used=None,
                          justification=None):
     """Assess reviewer/author role separation for blind queue workflows.

tests/test_already_landed_report_wording.py

@@ -0,0 +1,151 @@
+"""Tests for already-landed final-report wording validator (#298).
+
+An already-landed PR is reconciliation-only, never review/merge
+eligible, and a head SHA may not be called reviewed unless validation
+and diff review actually passed. Final reports and controller handoffs
+must use the reconciliation wording, not the legacy eligible/reviewed
+fields.
+"""
+import sys
+import unittest
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+
+from review_proofs import assess_already_landed_report_wording  # noqa: E402
+
+
+GOOD_ALREADY_LANDED = (
+    "Controller Handoff\n"
+    "- Oldest open PR requiring action: PR #278\n"
+    "- Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED\n"
+    "- Candidate head SHA: 2a544b7\n"
+    "- Reviewed head SHA: none\n"
+    "- Review worktree used: false\n"
+)
+
+
+class TestAlreadyLandedWording(unittest.TestCase):
+    def test_correct_reconciliation_wording_passes(self):
+        result = assess_already_landed_report_wording(
+            GOOD_ALREADY_LANDED, already_landed=True
+        )
+        self.assertTrue(result["complete"])
+        self.assertFalse(result["downgraded"])
+        self.assertEqual(result["reasons"], [])
+
+    def test_oldest_eligible_wording_fails(self):
+        report = GOOD_ALREADY_LANDED + "- oldest eligible PR: PR #278\n"
+        result = assess_already_landed_report_wording(
+            report, already_landed=True
+        )
+        self.assertFalse(result["complete"])
+        self.assertTrue(
+            any("oldest eligible pr" in r.lower() for r in result["reasons"])
+        )
+
+    def test_next_eligible_wording_fails(self):
+        report = GOOD_ALREADY_LANDED + "- next eligible PR: PR #278\n"
+        result = assess_already_landed_report_wording(
+            report, already_landed=True
+        )
+        self.assertFalse(result["complete"])
+
+    def test_pinned_reviewed_head_fails(self):
+        report = GOOD_ALREADY_LANDED + "- Pinned reviewed head: 2a544b7\n"
+        result = assess_already_landed_report_wording(
+            report, already_landed=True
+        )
+        self.assertFalse(result["complete"])
+        self.assertTrue(
+            any("pinned reviewed head" in r.lower() for r in result["reasons"])
+        )
+
+    def test_scratch_worktree_field_fails(self):
+        report = GOOD_ALREADY_LANDED + "- Scratch worktree used: False\n"
+        result = assess_already_landed_report_wording(
+            report, already_landed=True
+        )
+        self.assertFalse(result["complete"])
+
+    def test_missing_eligibility_class_fails(self):
+        report = (
+            "Controller Handoff\n"
+            "- Oldest open PR requiring action: PR #278\n"
+            "- Candidate head SHA: 2a544b7\n"
+            "- Reviewed head SHA: none\n"
+            "- Review worktree used: false\n"
+        )
+        result = assess_already_landed_report_wording(
+            report, already_landed=True
+        )
+        self.assertFalse(result["complete"])
+        self.assertTrue(
+            any("eligibility class" in r.lower() for r in result["reasons"])
+        )
+
+    def test_populated_reviewed_head_sha_fails(self):
+        report = GOOD_ALREADY_LANDED.replace(
+            "- Reviewed head SHA: none\n",
+            "- Reviewed head SHA: 2a544b7\n",
+        )
+        result = assess_already_landed_report_wording(
+            report, already_landed=True
+        )
+        self.assertFalse(result["complete"])
+        self.assertTrue(
+            any("reviewed head" in r.lower() for r in result["reasons"])
+        )
+
+
+class TestNonAlreadyLandedReports(unittest.TestCase):
+    def test_normal_reviewed_report_passes(self):
+        report = (
+            "- Selected PR: 281\n"
+            "- Pinned reviewed head: abc1234\n"
+            "- Review decision: approve\n"
+        )
+        result = assess_already_landed_report_wording(
+            report, already_landed=False, review_validated=True
+        )
+        self.assertTrue(result["complete"])
+
+    def test_blocked_infra_report_with_pinned_head_fails(self):
+        report = (
+            "- Selected PR: 281\n"
+            "- Pinned reviewed head: abc1234\n"
+            "- Current status: blocked on infra_stop\n"
+        )
+        result = assess_already_landed_report_wording(
+            report, already_landed=False, review_validated=False
+        )
+        self.assertFalse(result["complete"])
+        self.assertTrue(
+            any("before validation" in r.lower() for r in result["reasons"])
+        )
+
+    def test_validation_failed_report_with_reviewed_sha_fails(self):
+        report = (
+            "- Selected PR: 281\n"
+            "- Reviewed head SHA: abc1234\n"
+            "- Validation: full suite failed\n"
+        )
+        result = assess_already_landed_report_wording(
+            report, already_landed=False, review_validated=False
+        )
+        self.assertFalse(result["complete"])
+
+    def test_validation_failed_report_with_reviewed_none_passes(self):
+        report = (
+            "- Selected PR: 281\n"
+            "- Reviewed head SHA: none\n"
+            "- Validation: full suite failed\n"
+        )
+        result = assess_already_landed_report_wording(
+            report, already_landed=False, review_validated=False
+        )
+        self.assertTrue(result["complete"])
+
+
+if __name__ == "__main__":
+    unittest.main()