Merge pull request 'feat: hard-stop review mutations on already-landed PR heads (Closes #292)' (#368) from feat/issue-292-already-landed-gate into master

This commit was merged in pull request #368.
This commit is contained in:
2026-07-07 08:28:04 -05:00
2 changed files with 301 additions and 0 deletions
+166
View File
@@ -3964,6 +3964,172 @@ def assess_reviewer_baseline_validation_proof(
}
# ---------------------------------------------------------------------------
# Already-landed review gate (#292)
# ---------------------------------------------------------------------------
ALREADY_LANDED_STATE = "ALREADY_LANDED_RECONCILE_REQUIRED"
ALREADY_LANDED_HANDOFF_FIELDS = (
"PR number/title",
"candidate head SHA",
"target branch",
"target branch SHA",
"ancestor proof",
"linked issue status",
"recommended reconciliation action",
"capability proof for close/comment mutations",
)
_FORBIDDEN_LANDED_STATES_RE = re.compile(
r"review decision\s*:\s*approved?\b|"
r"merge result\s*:\s*merged\b|"
r"\bready[_ ]to[_ ]merge\b|"
r"\bmerge[d]?\s*:\s*success\b",
re.IGNORECASE,
)
def assess_already_landed_review_gate(
*,
pr_number: int | None,
candidate_head_sha: str | None,
target_branch: str | None,
target_branch_sha: str | None,
head_is_ancestor_of_target: bool | None,
live_head_sha: str | None = None,
real_blocker: bool = False,
mergeable: bool | None = None,
) -> dict:
"""#292: hard gate before any review mutation on an already-landed PR.
Runs after PR selection and head pinning, before approval, request-
changes, or the merge API. *head_is_ancestor_of_target* is the result
of an ancestry check of *candidate_head_sha* against the freshly
fetched target branch at *target_branch_sha* (e.g. ``git merge-base
--is-ancestor``). ``None`` means the check was not run — the gate then
fails closed.
*live_head_sha*, when provided, must equal *candidate_head_sha*; a
changed head invalidates the pinned ancestry result until re-checked.
*mergeable* is accepted for caller convenience but never consulted:
conflict/mergeability handling is a separate gate.
"""
del mergeable # ancestry gate only; mergeability is a separate gate
reasons: list[str] = []
if not isinstance(pr_number, int) or pr_number <= 0:
reasons.append("PR number missing or invalid (#292)")
if not _FULL_SHA.match((candidate_head_sha or "").strip()):
reasons.append(
"candidate head SHA is not a full 40-hex commit SHA (#292)"
)
if not (target_branch or "").strip():
reasons.append("target branch missing (#292)")
if not _FULL_SHA.match((target_branch_sha or "").strip()):
reasons.append(
"target branch SHA is not a full 40-hex commit SHA; fetch the "
"target branch and record its SHA before the ancestry check "
"(#292)"
)
if live_head_sha is not None and candidate_head_sha and (
live_head_sha.strip() != candidate_head_sha.strip()
):
reasons.append(
"PR head changed since the ancestry check was pinned; re-fetch "
"and re-run the already-landed gate (#292)"
)
if head_is_ancestor_of_target is None:
reasons.append(
"ancestry of the PR head against the target branch was not "
"checked; the already-landed gate must run before any review "
"mutation (#292)"
)
if reasons:
return {
"state": "GATE_NOT_PROVEN",
"approve_allowed": False,
"request_changes_allowed": False,
"merge_api_allowed": False,
"reconciliation_handoff_required": False,
"required_handoff_fields": (),
"reasons": reasons,
"safe_next_action": (
"fetch the target branch, pin the live PR head, run the "
"ancestry check, then re-run this gate"
),
}
if head_is_ancestor_of_target:
return {
"state": ALREADY_LANDED_STATE,
"approve_allowed": False,
"request_changes_allowed": bool(real_blocker),
"merge_api_allowed": False,
"reconciliation_handoff_required": True,
"required_handoff_fields": ALREADY_LANDED_HANDOFF_FIELDS,
"reasons": [
f"PR #{pr_number} head {candidate_head_sha} is already an "
f"ancestor of {target_branch} @ {target_branch_sha}; the PR "
"is reconciliation-only (#292)"
],
"safe_next_action": (
"stop before review mutation and emit an "
f"{ALREADY_LANDED_STATE} reconciliation handoff; any PR/issue "
"close or comment requires exact capability proof"
),
}
return {
"state": "NORMAL_REVIEW_CANDIDATE",
"approve_allowed": True,
"request_changes_allowed": True,
"merge_api_allowed": True,
"reconciliation_handoff_required": False,
"required_handoff_fields": (),
"reasons": [],
"safe_next_action": "proceed with the normal review workflow gates",
}
def assess_already_landed_report_state(
report_text: str | None,
*,
gate_fired: bool,
) -> dict:
"""#292: reject APPROVED/MERGED/READY_TO_MERGE after the gate fired.
*gate_fired* comes from the session's own gate result, so a report
that omits the already-landed markers entirely still fails closed —
unlike the text-only #327 wording rules this does not depend on the
report admitting the PR was already landed.
"""
if not gate_fired:
return {"complete": True, "block": False, "reasons": []}
text = report_text or ""
reasons: list[str] = []
if _FORBIDDEN_LANDED_STATES_RE.search(text):
reasons.append(
"report claims APPROVED/MERGED/READY_TO_MERGE although the "
"already-landed gate fired (#292)"
)
if ALREADY_LANDED_STATE.lower() not in text.lower():
reasons.append(
f"report must state {ALREADY_LANDED_STATE} when the "
"already-landed gate fired (#292)"
)
return {
"complete": not reasons,
"block": bool(reasons),
"reasons": reasons,
}
# ---------------------------------------------------------------------------
# Full-suite failure approval gate (#323)
# ---------------------------------------------------------------------------