diff --git a/review_proofs.py b/review_proofs.py index fdebea9..e0e948a 100644 --- a/review_proofs.py +++ b/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) # --------------------------------------------------------------------------- diff --git a/tests/test_review_proofs.py b/tests/test_review_proofs.py index 3a3ea7a..8721545 100644 --- a/tests/test_review_proofs.py +++ b/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()