feat: reject eligible/reviewed wording for already-landed PRs (Closes #298) #367

Merged
sysadmin merged 1 commits from feat/issue-298-already-landed-wording into master 2026-07-07 08:25:45 -05:00
2 changed files with 247 additions and 0 deletions
+96
View File
@@ -1171,6 +1171,102 @@ def assess_workspace_mutation_consistency(report_text, observed_commands=None):
}
# ── Already-landed report wording (Issue #298) ───────────────────────────────
#
# An already-landed PR is reconciliation-only: it must never be called
# eligible, its head SHA must never be called reviewed, and the legacy
# eligible/reviewed/scratch-worktree handoff fields must not appear.
# Reviewed-head claims in any report require validation + diff review to
# have actually passed.
ALREADY_LANDED_FORBIDDEN_PHRASES = (
"oldest eligible pr",
"next eligible pr",
"pinned reviewed head",
"scratch worktree used",
)
ALREADY_LANDED_ELIGIBILITY_CLASS = "ALREADY_LANDED_RECONCILE_REQUIRED"
def assess_already_landed_report_wording(report_text, *, already_landed,
review_validated=False):
"""#298: reject eligible/reviewed wording for already-landed PRs.
*already_landed* states whether the already-landed gate fired for the
selected PR. *review_validated* states whether validation and diff
review actually passed this session; only then may a reviewed head
SHA be populated in a non-already-landed report.
Returns {'complete', 'downgraded', 'reasons'}; fails closed.
"""
text_lower = (report_text or "").lower()
fields = _report_labeled_fields(report_text)
reasons = []
reviewed_head = fields.get("reviewed head sha")
reviewed_head_populated = bool(
reviewed_head and not reviewed_head.strip().lower().startswith("none")
)
pinned_head_present = "pinned reviewed head" in fields
if already_landed:
for phrase in ALREADY_LANDED_FORBIDDEN_PHRASES:
if phrase in text_lower:
reasons.append(
f"already-landed report must not use '{phrase}'; the PR "
"is reconciliation-only, not review/merge eligible"
)
eligibility = fields.get("eligibility class", "")
if ALREADY_LANDED_ELIGIBILITY_CLASS not in eligibility.upper():
reasons.append(
"already-landed report missing 'Eligibility class: "
f"{ALREADY_LANDED_ELIGIBILITY_CLASS}'"
)
for required in ("oldest open pr requiring action",
"candidate head sha"):
if required not in fields:
reasons.append(
f"already-landed report missing '{required}' field"
)
if "reviewed head sha" not in fields:
reasons.append(
"already-landed report must state 'Reviewed head SHA: none'"
)
elif reviewed_head_populated:
reasons.append(
"already-landed report must not claim a reviewed head SHA; "
"no validation or diff review passed for this PR"
)
worktree_used = fields.get("review worktree used", "")
if worktree_used.strip().lower() not in ("false", "no"):
reasons.append(
"already-landed report must state 'Review worktree used: "
"false'"
)
elif not review_validated:
if pinned_head_present:
reasons.append(
"'Pinned reviewed head' populated before validation and "
"diff review passed"
)
if reviewed_head_populated:
reasons.append(
"reviewed head SHA populated before validation and diff "
"review passed"
)
return {
"complete": not reasons,
"downgraded": bool(reasons),
"reasons": reasons,
}
def assess_role_boundary(proof=None, *, task_role=None, namespaces_used=None,
justification=None):
"""Assess reviewer/author role separation for blind queue workflows.
+151
View File
@@ -0,0 +1,151 @@
"""Tests for already-landed final-report wording validator (#298).
An already-landed PR is reconciliation-only, never review/merge
eligible, and a head SHA may not be called reviewed unless validation
and diff review actually passed. Final reports and controller handoffs
must use the reconciliation wording, not the legacy eligible/reviewed
fields.
"""
import sys
import unittest
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
from review_proofs import assess_already_landed_report_wording # noqa: E402
GOOD_ALREADY_LANDED = (
"Controller Handoff\n"
"- Oldest open PR requiring action: PR #278\n"
"- Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED\n"
"- Candidate head SHA: 2a544b7\n"
"- Reviewed head SHA: none\n"
"- Review worktree used: false\n"
)
class TestAlreadyLandedWording(unittest.TestCase):
def test_correct_reconciliation_wording_passes(self):
result = assess_already_landed_report_wording(
GOOD_ALREADY_LANDED, already_landed=True
)
self.assertTrue(result["complete"])
self.assertFalse(result["downgraded"])
self.assertEqual(result["reasons"], [])
def test_oldest_eligible_wording_fails(self):
report = GOOD_ALREADY_LANDED + "- oldest eligible PR: PR #278\n"
result = assess_already_landed_report_wording(
report, already_landed=True
)
self.assertFalse(result["complete"])
self.assertTrue(
any("oldest eligible pr" in r.lower() for r in result["reasons"])
)
def test_next_eligible_wording_fails(self):
report = GOOD_ALREADY_LANDED + "- next eligible PR: PR #278\n"
result = assess_already_landed_report_wording(
report, already_landed=True
)
self.assertFalse(result["complete"])
def test_pinned_reviewed_head_fails(self):
report = GOOD_ALREADY_LANDED + "- Pinned reviewed head: 2a544b7\n"
result = assess_already_landed_report_wording(
report, already_landed=True
)
self.assertFalse(result["complete"])
self.assertTrue(
any("pinned reviewed head" in r.lower() for r in result["reasons"])
)
def test_scratch_worktree_field_fails(self):
report = GOOD_ALREADY_LANDED + "- Scratch worktree used: False\n"
result = assess_already_landed_report_wording(
report, already_landed=True
)
self.assertFalse(result["complete"])
def test_missing_eligibility_class_fails(self):
report = (
"Controller Handoff\n"
"- Oldest open PR requiring action: PR #278\n"
"- Candidate head SHA: 2a544b7\n"
"- Reviewed head SHA: none\n"
"- Review worktree used: false\n"
)
result = assess_already_landed_report_wording(
report, already_landed=True
)
self.assertFalse(result["complete"])
self.assertTrue(
any("eligibility class" in r.lower() for r in result["reasons"])
)
def test_populated_reviewed_head_sha_fails(self):
report = GOOD_ALREADY_LANDED.replace(
"- Reviewed head SHA: none\n",
"- Reviewed head SHA: 2a544b7\n",
)
result = assess_already_landed_report_wording(
report, already_landed=True
)
self.assertFalse(result["complete"])
self.assertTrue(
any("reviewed head" in r.lower() for r in result["reasons"])
)
class TestNonAlreadyLandedReports(unittest.TestCase):
def test_normal_reviewed_report_passes(self):
report = (
"- Selected PR: 281\n"
"- Pinned reviewed head: abc1234\n"
"- Review decision: approve\n"
)
result = assess_already_landed_report_wording(
report, already_landed=False, review_validated=True
)
self.assertTrue(result["complete"])
def test_blocked_infra_report_with_pinned_head_fails(self):
report = (
"- Selected PR: 281\n"
"- Pinned reviewed head: abc1234\n"
"- Current status: blocked on infra_stop\n"
)
result = assess_already_landed_report_wording(
report, already_landed=False, review_validated=False
)
self.assertFalse(result["complete"])
self.assertTrue(
any("before validation" in r.lower() for r in result["reasons"])
)
def test_validation_failed_report_with_reviewed_sha_fails(self):
report = (
"- Selected PR: 281\n"
"- Reviewed head SHA: abc1234\n"
"- Validation: full suite failed\n"
)
result = assess_already_landed_report_wording(
report, already_landed=False, review_validated=False
)
self.assertFalse(result["complete"])
def test_validation_failed_report_with_reviewed_none_passes(self):
report = (
"- Selected PR: 281\n"
"- Reviewed head SHA: none\n"
"- Validation: full suite failed\n"
)
result = assess_already_landed_report_wording(
report, already_landed=False, review_validated=False
)
self.assertTrue(result["complete"])
if __name__ == "__main__":
unittest.main()