fix(validator): align final-report validator with canonical schema (Closes #698)
Fixes the final-report validator defects from Issue #698 (original lead plus the independent reproduction recorded during the PR #703 formal review, comment 11246): - Legacy fields: the review/merger required-field tables no longer demand 'Pinned reviewed head', 'Scratch worktree used', 'Worktree path', 'Worktree dirty', 'Mutations', 'Next', 'Issue/PR', 'Branch/SHA', or 'Files changed' — names the canonical review-merge-final-report schema forbids or replaces. The canonical names ('Reviewed head SHA', 'Review worktree path/dirty', 'Safe next action', mutation categories) are required instead, with backward-compatible aliases where the schema permits them. - Structured proof: workflow-load helper results are recognized in colon, key=value, and JSON renderings; validation pass proof is accepted anywhere in the Validation field value (for example 'focused 50 passed; full 2665 passed'). - Mutation inference: review mutations are inferred only from authoritative evidence (performed=true and not gated); read-only diagnostics and pre-API rejections no longer count as mutations. - Lease release vs cleanup: canonical reviewer lease release (release tool call or terminal phase=released marker) is lease lifecycle, not post-merge cleanup, and no longer triggers the branch/worktree cleanup checklist; genuine delete/remove claims still require full proof. - Blocked reports: a legitimately blocked run that states an explicit 'Reviewed/Candidate head SHA: none' with no verdict, merge, or started validation owes no head proofs; approval-time and merge-time live-head proofs are demanded only once the corresponding phase begins, and a report that states no head at all still fails closed. - action_log robustness: malformed (non-dict) entries and non-list logs are reported as clear sanitized findings (position and type only, no content echo) instead of crashing with AttributeError; a defective validator rule now fails closed with a sanitized block finding rather than raising a secondary exception. 27 new regression tests, including canonical fixtures modeled on the PR #703 formal-review handoff and the blocked preflight report from the prior #698 reproductions. Two legacy-field test fixtures updated to the canonical schema. Full suite: 2663 passed, 6 skipped, 161 subtests. Co-Authored-By: Claude Fable 5 <[email protected]>
This commit is contained in:
+67
-6
@@ -403,10 +403,37 @@ _REVIEWER_ACTIVE_RE = re.compile(
|
||||
r"whether any reviewer was active\s*:\s*(yes|no|true|false)",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
# #698 phase detection for phase-specific head proofs.
|
||||
_NO_REVIEWED_HEAD_RE = re.compile(
|
||||
r"(?:reviewed head sha|candidate head sha)\s*:\s*none\b",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
_VERDICT_RECORDED_RE = re.compile(
|
||||
r"review decision\s*:\s*(?:approve[d]?|request[_ ]changes)\b"
|
||||
r"|review_status\s*:\s*(?:approved|request_changes)\b"
|
||||
r"|terminal review mutation\s*:\s*(?!none\b)\S",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
_MERGE_ATTEMPTED_RE = re.compile(
|
||||
r"merge result\s*:\s*(?:merged|success|performed|failed|attempted)\b"
|
||||
r"|merge mutations\s*:\s*(?!none\b|not applicable\b)\S",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
_VALIDATION_STARTED_RE = re.compile(
|
||||
r"validation\s*:\s*(?!none\b|not run\b|not applicable\b|not started\b)"
|
||||
r"[^\n]*(?:pass|fail|ran|executed|\d+\s+passed)",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
|
||||
def assess_reviewer_stale_head_final_report(report_text: str) -> dict[str, Any]:
|
||||
"""Final-report proof for reviewed vs live head SHAs (#399 AC 6)."""
|
||||
"""Final-report proof for reviewed vs live head SHAs (#399 AC 6).
|
||||
|
||||
#698: head proofs are phase-specific. A legitimately blocked run that
|
||||
never began validation (no reviewed head, no formal verdict, no merge)
|
||||
owes none of them; approval-time and merge-time live-head proofs are
|
||||
owed only once the corresponding phase actually begins.
|
||||
"""
|
||||
text = report_text or ""
|
||||
reasons: list[str] = []
|
||||
reviewed = _normalize_sha(_REVIEWED_HEAD_RE.search(text).group(1) if _REVIEWED_HEAD_RE.search(text) else None)
|
||||
@@ -422,15 +449,49 @@ def assess_reviewer_stale_head_final_report(report_text: str) -> dict[str, Any]:
|
||||
)
|
||||
push_during = _PUSH_DURING_VALIDATION_RE.search(text)
|
||||
|
||||
if not reviewed:
|
||||
# Phase detection from the report's own claims.
|
||||
no_head_stated = bool(_NO_REVIEWED_HEAD_RE.search(text))
|
||||
verdict_recorded = bool(_VERDICT_RECORDED_RE.search(text))
|
||||
merge_attempted = bool(_MERGE_ATTEMPTED_RE.search(text))
|
||||
validation_started = bool(reviewed) or bool(_VALIDATION_STARTED_RE.search(text))
|
||||
blocked_before_validation = (
|
||||
no_head_stated
|
||||
and not reviewed
|
||||
and not verdict_recorded
|
||||
and not merge_attempted
|
||||
and not validation_started
|
||||
)
|
||||
|
||||
if blocked_before_validation:
|
||||
return {
|
||||
"proven": True,
|
||||
"block": False,
|
||||
"reasons": [],
|
||||
"reviewed_head_sha": None,
|
||||
"live_head_sha_before_approval": None,
|
||||
"live_head_sha_before_merge": None,
|
||||
"push_during_validation": (
|
||||
push_during.group(1).lower() if push_during else None
|
||||
),
|
||||
"phase": "blocked_before_validation",
|
||||
}
|
||||
|
||||
if not reviewed and not no_head_stated:
|
||||
# The head must always be STATED — either a SHA or an explicit
|
||||
# 'none'. Silence is not a phase claim and fails closed.
|
||||
reasons.append(
|
||||
"reviewed head SHA not stated in final report "
|
||||
"(state the SHA or an explicit 'none')"
|
||||
)
|
||||
elif not reviewed and (validation_started or verdict_recorded or merge_attempted):
|
||||
reasons.append("reviewed head SHA not stated in final report")
|
||||
if not live_approval:
|
||||
if verdict_recorded and not live_approval:
|
||||
reasons.append("final live head SHA before approval not stated")
|
||||
if not live_merge:
|
||||
if merge_attempted and not live_merge:
|
||||
reasons.append("final live head SHA before merge not stated")
|
||||
if not push_during:
|
||||
if validation_started and not push_during:
|
||||
reasons.append("whether push occurred during validation not stated")
|
||||
elif reviewed and live_approval and reviewed != live_approval:
|
||||
if reviewed and live_approval and reviewed != live_approval:
|
||||
reasons.append("live head before approval differs from reviewed head SHA")
|
||||
elif reviewed and live_merge and reviewed != live_merge:
|
||||
reasons.append("live head before merge differs from reviewed head SHA")
|
||||
|
||||
Reference in New Issue
Block a user