Merge pull request 'feat: block approval on full-suite failure without baseline proof (Closes #323)' (#361) from feat/issue-323-full-suite-approval-gate into master

review_proofs.py

@@ -3652,6 +3652,113 @@ def assess_reviewer_baseline_validation_proof(
     }
 
 
+# ---------------------------------------------------------------------------
+# Full-suite failure approval gate (#323)
+# ---------------------------------------------------------------------------
+
+
+def assess_full_suite_failure_approval_gate(
+    *,
+    review_decision: str,
+    full_suite_passed: bool | None,
+    baseline_validation: dict | None = None,
+    baseline_proof: dict | None = None,
+    report_text: str | None = None,
+    project_root: str | None = None,
+) -> dict:
+    """#323: approving a PR whose full suite fails requires baseline proof.
+
+    Default action on a full-suite failure is REQUEST_CHANGES. Approval is
+    allowed only when the #325 baseline proof passes in full (branches/
+    worktree, pinned SHAs, matching failure signatures) and the extra #323
+    fields prove the failures are pre-existing: ``pr_suite_command``,
+    ``baseline_suite_command``, ``new_tests_passed``, and a non-empty
+    ``unrelated_explanation``.
+
+    *review_decision* is the decision the workflow intends to submit; only
+    ``approve`` can be blocked. ``block`` is True when an approval was
+    requested that the proofs do not allow.
+    """
+    decision = (review_decision or "").strip().lower().replace("-", "_")
+    wants_approval = decision == "approve"
+    reasons: list[str] = []
+
+    if full_suite_passed is True:
+        return {
+            "approve_allowed": True,
+            "block": False,
+            "default_action": "proceed",
+            "reasons": [],
+        }
+
+    default_action = "request_changes"
+
+    if full_suite_passed is None:
+        reasons.append(
+            "full-suite result not proven; approval requires a recorded "
+            "full-suite command and result (#323)"
+        )
+    else:
+        if baseline_validation is None:
+            baseline_validation = assess_reviewer_baseline_validation_proof(
+                baseline_proof=baseline_proof,
+                report_text=report_text,
+                project_root=project_root,
+            )
+        proof = baseline_proof or {}
+
+        if not proof:
+            reasons.append(
+                "full suite failed but no baseline proof was provided; "
+                "default action is REQUEST_CHANGES (#323)"
+            )
+        if not baseline_validation.get("proven"):
+            reasons.append(
+                "baseline validation proof missing or failed (#323/#325)"
+            )
+            reasons.extend(baseline_validation.get("reasons", []))
+
+        if proof:
+            worktree = (proof.get("worktree_path") or "").strip()
+            if not _path_under_branches(worktree, project_root):
+                reasons.append(
+                    "baseline comparison must run in a clean baseline "
+                    "worktree under branches/, not the main checkout (#323)"
+                )
+            if not (proof.get("pr_suite_command") or "").strip():
+                reasons.append(
+                    "PR full-suite command/result missing from baseline "
+                    "proof (#323)"
+                )
+            if not (proof.get("baseline_suite_command") or "").strip():
+                reasons.append(
+                    "baseline full-suite command/result missing from "
+                    "baseline proof (#323)"
+                )
+            if proof.get("new_tests_passed") is not True:
+                reasons.append(
+                    "proof that new/changed tests passed is missing (#323)"
+                )
+            if not (proof.get("unrelated_explanation") or "").strip():
+                reasons.append(
+                    "explanation why failures are unrelated to the PR is "
+                    "missing (#323)"
+                )
+            if proof.get("failure_signatures_match") is not True:
+                reasons.append(
+                    "PR and baseline failure signatures do not match; "
+                    "request changes (#323)"
+                )
+
+    approve_allowed = not reasons
+    return {
+        "approve_allowed": approve_allowed,
+        "block": wants_approval and not approve_allowed,
+        "default_action": "proceed" if approve_allowed else default_action,
+        "reasons": reasons,
+    }
+
+
 # ---------------------------------------------------------------------------
 # Identity disclosure (#305)
 # ---------------------------------------------------------------------------

tests/test_review_proofs.py

@@ -24,6 +24,7 @@ from review_proofs import (  # noqa: E402
     ISSUE_SELECTION_CONTINUATION_EXPLICIT,
     ISSUE_SELECTION_REPRESENTED_BY_OPEN_PR,
     assess_author_pr_report,
+    assess_full_suite_failure_approval_gate,
     assess_queue_ordering_proof,
     assess_reviewer_baseline_validation_proof,
     assess_capability_evidence,
@@ -2539,5 +2540,170 @@ class TestReviewerBaselineValidationProof(unittest.TestCase):
         self.assertFalse(report["baseline_validation_proven"])
 
 
+class TestFullSuiteFailureApprovalGate(unittest.TestCase):
+    """Issue #323: full-suite failure blocks approval without baseline proof."""
+
+    ROOT = "/repo/Gitea-Tools"
+
+    def _good_proof(self, **overrides):
+        proof = {
+            "worktree_path": f"{self.ROOT}/branches/baseline-master-pr280",
+            "baseline_target_sha": PINNED,
+            "pr_head_sha": OTHER,
+            "baseline_failures": ["tests/test_foo.py::test_bar"],
+            "pr_failures": ["tests/test_foo.py::test_bar"],
+            "failure_signatures_match": True,
+            "clean_before": True,
+            "clean_after": True,
+            "pr_suite_command": "venv/bin/pytest tests/",
+            "baseline_suite_command": "venv/bin/pytest tests/",
+            "new_tests_passed": True,
+            "unrelated_explanation": (
+                "failures touch agent_temp_artifacts only; PR changes "
+                "review_proofs handoff fields"
+            ),
+        }
+        proof.update(overrides)
+        return proof
+
+    def _good_report(self):
+        return (
+            "Full suite failed on PR worktree and on baseline.\n"
+            "- Validation command: venv/bin/pytest tests/\n"
+            f"- Working directory: {self.ROOT}/branches/review-pr-280\n"
+            "Failures are pre-existing on master.\n"
+        )
+
+    def test_full_suite_pass_allows_approval_without_baseline(self):
+        result = assess_full_suite_failure_approval_gate(
+            review_decision="approve",
+            full_suite_passed=True,
+            project_root=self.ROOT,
+        )
+        self.assertTrue(result["approve_allowed"])
+        self.assertFalse(result["block"])
+        self.assertEqual(result["default_action"], "proceed")
+
+    def test_full_suite_failure_defaults_to_request_changes(self):
+        result = assess_full_suite_failure_approval_gate(
+            review_decision="request_changes",
+            full_suite_passed=False,
+            project_root=self.ROOT,
+        )
+        self.assertFalse(result["block"])
+        self.assertEqual(result["default_action"], "request_changes")
+
+    def test_approval_with_failure_and_no_proof_blocks(self):
+        result = assess_full_suite_failure_approval_gate(
+            review_decision="approve",
+            full_suite_passed=False,
+            project_root=self.ROOT,
+        )
+        self.assertFalse(result["approve_allowed"])
+        self.assertTrue(result["block"])
+        self.assertEqual(result["default_action"], "request_changes")
+
+    def test_approval_with_matching_baseline_proof_allowed(self):
+        result = assess_full_suite_failure_approval_gate(
+            review_decision="approve",
+            full_suite_passed=False,
+            baseline_proof=self._good_proof(),
+            report_text=self._good_report(),
+            project_root=self.ROOT,
+        )
+        self.assertTrue(result["approve_allowed"])
+        self.assertFalse(result["block"])
+
+    def test_mismatched_failure_signatures_block_approval(self):
+        result = assess_full_suite_failure_approval_gate(
+            review_decision="approve",
+            full_suite_passed=False,
+            baseline_proof=self._good_proof(
+                failure_signatures_match=False,
+                pr_failures=["tests/test_other.py::test_other"],
+            ),
+            report_text=self._good_report(),
+            project_root=self.ROOT,
+        )
+        self.assertFalse(result["approve_allowed"])
+        self.assertTrue(result["block"])
+
+    def test_main_checkout_baseline_blocks_approval(self):
+        result = assess_full_suite_failure_approval_gate(
+            review_decision="approve",
+            full_suite_passed=False,
+            baseline_proof=self._good_proof(worktree_path=self.ROOT),
+            report_text=self._good_report(),
+            project_root=self.ROOT,
+        )
+        self.assertFalse(result["approve_allowed"])
+        self.assertTrue(result["block"])
+
+    def test_missing_new_tests_proof_blocks_approval(self):
+        result = assess_full_suite_failure_approval_gate(
+            review_decision="approve",
+            full_suite_passed=False,
+            baseline_proof=self._good_proof(new_tests_passed=None),
+            report_text=self._good_report(),
+            project_root=self.ROOT,
+        )
+        self.assertFalse(result["approve_allowed"])
+        self.assertTrue(any(
+            "new" in reason.lower() and "test" in reason.lower()
+            for reason in result["reasons"]
+        ))
+
+    def test_missing_unrelated_explanation_blocks_approval(self):
+        result = assess_full_suite_failure_approval_gate(
+            review_decision="approve",
+            full_suite_passed=False,
+            baseline_proof=self._good_proof(unrelated_explanation=""),
+            report_text=self._good_report(),
+            project_root=self.ROOT,
+        )
+        self.assertFalse(result["approve_allowed"])
+
+    def test_missing_suite_commands_block_approval(self):
+        result = assess_full_suite_failure_approval_gate(
+            review_decision="approve",
+            full_suite_passed=False,
+            baseline_proof=self._good_proof(
+                pr_suite_command="", baseline_suite_command=""),
+            report_text=self._good_report(),
+            project_root=self.ROOT,
+        )
+        self.assertFalse(result["approve_allowed"])
+        self.assertTrue(any(
+            "command" in reason.lower() for reason in result["reasons"]
+        ))
+
+    def test_unknown_suite_result_blocks_approval(self):
+        result = assess_full_suite_failure_approval_gate(
+            review_decision="approve",
+            full_suite_passed=None,
+            project_root=self.ROOT,
+        )
+        self.assertFalse(result["approve_allowed"])
+        self.assertTrue(result["block"])
+
+    def test_vague_same_as_master_claim_without_proof_blocks(self):
+        result = assess_full_suite_failure_approval_gate(
+            review_decision="approve",
+            full_suite_passed=False,
+            report_text="Validation: failures same as master; approving.",
+            project_root=self.ROOT,
+        )
+        self.assertFalse(result["approve_allowed"])
+        self.assertTrue(result["block"])
+
+    def test_non_approve_decision_never_blocks(self):
+        result = assess_full_suite_failure_approval_gate(
+            review_decision="comment",
+            full_suite_passed=None,
+            project_root=self.ROOT,
+        )
+        self.assertFalse(result["block"])
+
+
 if __name__ == "__main__":
     unittest.main()