feat: enforce branches-only baseline validation in reviewer reports (Closes #325)
Add assess_reviewer_baseline_validation_proof to block test-suite runs in the main checkout and require complete baseline worktree proof when reports claim pre-existing master failures. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
+199
-1
@@ -1147,7 +1147,8 @@ def build_final_report(checkout_proof, inventory, validation, contamination,
|
||||
role_boundary=None, review_mutation=None,
|
||||
report_text=None, review_decision_lock=None,
|
||||
controller_handoff=None, capability_proof=None,
|
||||
sweep_proof=None, worktree_proof=None):
|
||||
sweep_proof=None, worktree_proof=None,
|
||||
baseline_validation=None, project_root=None):
|
||||
"""Required behavior 6 + acceptance criteria: one report, distinct proofs.
|
||||
|
||||
Combines the individual proof verdicts into the final-report fields the
|
||||
@@ -1174,6 +1175,18 @@ def build_final_report(checkout_proof, inventory, validation, contamination,
|
||||
)
|
||||
|
||||
empty_queue_report = assess_empty_queue_report(report_text)
|
||||
if baseline_validation is None and report_text:
|
||||
baseline_validation = assess_reviewer_baseline_validation_proof(
|
||||
report_text=report_text,
|
||||
project_root=project_root,
|
||||
)
|
||||
elif baseline_validation is None:
|
||||
baseline_validation = {
|
||||
"proven": True,
|
||||
"block": False,
|
||||
"reasons": [],
|
||||
"violations": [],
|
||||
}
|
||||
|
||||
contamination_status = contamination.get("status", "unknown")
|
||||
checkout_proven = bool(checkout_proof.get("proven"))
|
||||
@@ -1303,6 +1316,11 @@ def build_final_report(checkout_proof, inventory, validation, contamination,
|
||||
"empty-queue report missing or failed trust-gate proof (#198)"
|
||||
)
|
||||
downgrade_reasons.extend(empty_queue_report.get("reasons", []))
|
||||
if not baseline_validation.get("proven"):
|
||||
downgrade_reasons.append(
|
||||
"reviewer baseline validation proof missing or failed (#325)"
|
||||
)
|
||||
downgrade_reasons.extend(baseline_validation.get("reasons", []))
|
||||
|
||||
merge_allowed = (
|
||||
identity_eligible
|
||||
@@ -1314,9 +1332,11 @@ def build_final_report(checkout_proof, inventory, validation, contamination,
|
||||
# #179: no merge without a proven final live-state recheck.
|
||||
and live_state_proven
|
||||
and worktree_proven
|
||||
and baseline_validation.get("proven")
|
||||
)
|
||||
|
||||
violations = []
|
||||
violations.extend(baseline_validation.get("violations", []))
|
||||
if merge_performed and not merge_allowed:
|
||||
violations.append(
|
||||
"merge was performed/claimed although the proofs did not allow "
|
||||
@@ -1368,6 +1388,10 @@ def build_final_report(checkout_proof, inventory, validation, contamination,
|
||||
else True
|
||||
),
|
||||
"empty_queue_trust_gate_status": empty_queue_report.get("status"),
|
||||
"baseline_validation_proven": bool(baseline_validation.get("proven")),
|
||||
"baseline_validation_violations": list(
|
||||
baseline_validation.get("violations") or []
|
||||
),
|
||||
}
|
||||
|
||||
|
||||
@@ -2924,6 +2948,180 @@ def build_review_mutation_proof(run_log: list[dict]) -> dict:
|
||||
}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Reviewer baseline validation (#325)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_TEST_VALIDATION_COMMAND_RE = re.compile(
|
||||
r"(?:^|\s)(?:venv/)?(?:bin/)?(?:python\s+-m\s+)?"
|
||||
r"(?:pytest|make\s+test|npm\s+test|cargo\s+test|go\s+test\b)",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
_PREEXISTING_FAILURE_CLAIM_RE = re.compile(
|
||||
r"(?:\bsame\s+as\s+master\b|"
|
||||
r"\bpre-?existing(?:\s+(?:on\s+)?master)?\b|"
|
||||
r"\bfailures?\s+(?:are|is)\s+pre-?existing\b|"
|
||||
r"\bmaster\s+also\s+fails?\b|"
|
||||
r"\bfull-?suite\s+failures?\s+(?:are|is)\s+pre-?existing\b)",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
_REPORT_VALIDATION_CWD_RE = re.compile(
|
||||
r"(?:working\s+directory|cwd|directory)\s*:\s*(\S+)",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
|
||||
def _normalize_path(path: str) -> str:
|
||||
return (path or "").replace("\\", "/").rstrip("/")
|
||||
|
||||
|
||||
def _path_under_branches(path: str, project_root: str | None = None) -> bool:
|
||||
"""True when *path* is inside the project's ``branches/`` directory."""
|
||||
normalized = _normalize_path(path)
|
||||
if not normalized:
|
||||
return False
|
||||
if "/branches/" in f"{normalized}/":
|
||||
return True
|
||||
if normalized.endswith("/branches"):
|
||||
return True
|
||||
if project_root:
|
||||
root = _normalize_path(project_root)
|
||||
if normalized.startswith(f"{root}/"):
|
||||
rel = normalized[len(root) + 1 :]
|
||||
return rel == "branches" or rel.startswith("branches/")
|
||||
return False
|
||||
|
||||
|
||||
def _is_test_validation_command(command: str) -> bool:
|
||||
return bool(_TEST_VALIDATION_COMMAND_RE.search((command or "").strip()))
|
||||
|
||||
|
||||
def _is_full_sha(value: str | None) -> bool:
|
||||
return bool(value and _FULL_SHA.match((value or "").strip()))
|
||||
|
||||
|
||||
def assess_reviewer_baseline_validation_proof(
|
||||
*,
|
||||
validation_runs: list[dict] | None = None,
|
||||
baseline_proof: dict | None = None,
|
||||
report_text: str | None = None,
|
||||
project_root: str | None = None,
|
||||
) -> dict:
|
||||
"""#325: reviewer validation and baseline comparison must use ``branches/``.
|
||||
|
||||
*validation_runs* entries: ``command``, ``working_directory`` (or ``cwd``),
|
||||
optional ``project_root``.
|
||||
|
||||
*baseline_proof* keys when claiming pre-existing master failures:
|
||||
``worktree_path``, ``baseline_target_sha``, ``pr_head_sha``,
|
||||
``baseline_failures``, ``pr_failures``, ``failure_signatures_match``,
|
||||
``clean_before``, ``clean_after``.
|
||||
"""
|
||||
reasons: list[str] = []
|
||||
violations: list[str] = []
|
||||
text = report_text or ""
|
||||
|
||||
runs = list(validation_runs or [])
|
||||
pending_command = None
|
||||
for raw_line in text.splitlines():
|
||||
line = raw_line.strip().lstrip("-*").strip()
|
||||
lower = line.lower()
|
||||
if lower.startswith("validation command:"):
|
||||
pending_command = line.split(":", 1)[1].strip()
|
||||
continue
|
||||
cwd_match = _REPORT_VALIDATION_CWD_RE.search(line)
|
||||
if cwd_match:
|
||||
cwd = cwd_match.group(1).strip().rstrip(",.;")
|
||||
command = pending_command or line
|
||||
if _is_test_validation_command(command):
|
||||
runs.append(
|
||||
{
|
||||
"command": command,
|
||||
"working_directory": cwd,
|
||||
"project_root": project_root,
|
||||
}
|
||||
)
|
||||
pending_command = None
|
||||
|
||||
for run in runs:
|
||||
command = (run.get("command") or "").strip()
|
||||
if not command or not _is_test_validation_command(command):
|
||||
continue
|
||||
cwd = (run.get("working_directory") or run.get("cwd") or "").strip()
|
||||
root = run.get("project_root") or project_root
|
||||
if not cwd:
|
||||
reasons.append(
|
||||
"test validation command stated without working directory; "
|
||||
"cannot prove branches-only execution (#325)"
|
||||
)
|
||||
continue
|
||||
if not _path_under_branches(cwd, root):
|
||||
violations.append(
|
||||
f"test validation ran outside branches/ worktree: cwd={cwd!r}"
|
||||
)
|
||||
reasons.append(
|
||||
"reviewer workflow must not run test suites in the main "
|
||||
f"checkout; cwd {cwd!r} is not under branches/ (#325)"
|
||||
)
|
||||
|
||||
claims_preexisting = bool(_PREEXISTING_FAILURE_CLAIM_RE.search(text))
|
||||
if claims_preexisting:
|
||||
proof = baseline_proof or {}
|
||||
worktree = (proof.get("worktree_path") or "").strip()
|
||||
if not worktree or not _path_under_branches(worktree, project_root):
|
||||
reasons.append(
|
||||
"pre-existing master failure claimed without a baseline "
|
||||
"worktree path under branches/ (#325)"
|
||||
)
|
||||
if not _is_full_sha(proof.get("baseline_target_sha")):
|
||||
reasons.append(
|
||||
"pre-existing master failure claimed without "
|
||||
"baseline_target_sha proof (#325)"
|
||||
)
|
||||
if not _is_full_sha(proof.get("pr_head_sha")):
|
||||
reasons.append(
|
||||
"pre-existing master failure claimed without pr_head_sha "
|
||||
"proof (#325)"
|
||||
)
|
||||
baseline_failures = proof.get("baseline_failures")
|
||||
pr_failures = proof.get("pr_failures")
|
||||
if baseline_failures is None or pr_failures is None:
|
||||
reasons.append(
|
||||
"pre-existing master failure claimed without baseline and "
|
||||
"PR failure listings (#325)"
|
||||
)
|
||||
if proof.get("failure_signatures_match") is not True:
|
||||
reasons.append(
|
||||
"pre-existing master failure claimed without proven matching "
|
||||
"failure signatures (#325)"
|
||||
)
|
||||
if proof.get("clean_before") is not True:
|
||||
reasons.append(
|
||||
"baseline worktree clean-before validation not proven (#325)"
|
||||
)
|
||||
if proof.get("clean_after") is not True:
|
||||
reasons.append(
|
||||
"baseline worktree clean-after validation not proven (#325)"
|
||||
)
|
||||
|
||||
proven = not reasons and not violations
|
||||
return {
|
||||
"proven": proven,
|
||||
"block": bool(violations),
|
||||
"claims_preexisting": claims_preexisting,
|
||||
"reasons": reasons,
|
||||
"violations": violations,
|
||||
"safe_next_action": (
|
||||
"create a clean baseline worktree under branches/, e.g. "
|
||||
"branches/baseline-master-pr<N>, and rerun validation there"
|
||||
if not proven
|
||||
else "proceed"
|
||||
),
|
||||
}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Identity disclosure (#305)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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_reviewer_baseline_validation_proof,
|
||||
assess_capability_evidence,
|
||||
assess_capability_proof,
|
||||
assess_contradictory_no_pr_claim,
|
||||
@@ -2160,5 +2161,125 @@ class TestCreateIssueFinalReport(unittest.TestCase):
|
||||
self.assertTrue(result["downgraded"])
|
||||
|
||||
|
||||
class TestReviewerBaselineValidationProof(unittest.TestCase):
|
||||
"""Issue #325: baseline validation must use branches/ worktrees."""
|
||||
|
||||
ROOT = "/repo/Gitea-Tools"
|
||||
|
||||
def _good_baseline_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,
|
||||
}
|
||||
proof.update(overrides)
|
||||
return proof
|
||||
|
||||
def test_main_checkout_test_run_blocks(self):
|
||||
result = assess_reviewer_baseline_validation_proof(
|
||||
validation_runs=[
|
||||
{
|
||||
"command": "venv/bin/pytest tests/",
|
||||
"working_directory": self.ROOT,
|
||||
"project_root": self.ROOT,
|
||||
}
|
||||
],
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(result["block"])
|
||||
self.assertTrue(result["violations"])
|
||||
|
||||
def test_branches_worktree_test_run_passes(self):
|
||||
result = assess_reviewer_baseline_validation_proof(
|
||||
validation_runs=[
|
||||
{
|
||||
"command": "venv/bin/pytest tests/",
|
||||
"working_directory": f"{self.ROOT}/branches/review-pr-280",
|
||||
"project_root": self.ROOT,
|
||||
}
|
||||
],
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_preexisting_claim_without_baseline_proof_fails(self):
|
||||
result = assess_reviewer_baseline_validation_proof(
|
||||
report_text=(
|
||||
"Validation: full-suite failures are pre-existing on master."
|
||||
),
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(result["claims_preexisting"])
|
||||
|
||||
def test_preexisting_claim_with_complete_baseline_proof_passes(self):
|
||||
result = assess_reviewer_baseline_validation_proof(
|
||||
report_text="Failures are pre-existing on master.",
|
||||
baseline_proof=self._good_baseline_proof(),
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_mismatched_failures_block_preexisting_claim(self):
|
||||
result = assess_reviewer_baseline_validation_proof(
|
||||
report_text="Same as master.",
|
||||
baseline_proof=self._good_baseline_proof(
|
||||
failure_signatures_match=False,
|
||||
pr_failures=["tests/test_other.py::test_other"],
|
||||
),
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
def test_report_parsed_main_checkout_cwd_blocks(self):
|
||||
result = assess_reviewer_baseline_validation_proof(
|
||||
report_text=(
|
||||
"- Validation command: venv/bin/pytest tests/\n"
|
||||
f"- Working directory: {self.ROOT}\n"
|
||||
),
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(result["block"])
|
||||
|
||||
def test_build_final_report_downgrades_main_checkout_validation(self):
|
||||
report = build_final_report(
|
||||
checkout_proof=_good_checkout(),
|
||||
inventory=_good_inventory(),
|
||||
validation=_good_validation(),
|
||||
contamination=_good_contamination(),
|
||||
identity_eligible=True,
|
||||
merge_performed=False,
|
||||
issue_status_verified=True,
|
||||
capability_evidence=_good_capability_evidence(),
|
||||
sweep=_good_sweep(),
|
||||
live_state=_good_live_state(),
|
||||
role_boundary=_good_role_boundary(),
|
||||
review_mutation=_good_review_mutation(),
|
||||
controller_handoff=_good_handoff(),
|
||||
capability_proof=_good_capability_proof(),
|
||||
sweep_proof=_good_secret_sweep(),
|
||||
worktree_proof={
|
||||
"worktree_path": "/repo/branches/review-feat-issue-224",
|
||||
"porcelain_status": "",
|
||||
"pr_scope_files": ["docs/wiki/Repositories.md"],
|
||||
"scratch_used": True,
|
||||
},
|
||||
report_text=(
|
||||
"- Validation command: venv/bin/pytest tests/\n"
|
||||
f"- Working directory: {self.ROOT}\n"
|
||||
),
|
||||
project_root=self.ROOT,
|
||||
)
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
self.assertFalse(report["baseline_validation_proven"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user