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

2026-07-07T04:39:02-05:00

jcwalker3 commented

2026-07-07 04:39:02 -05:00

Summary

Implements #298: final report validator must reject eligible/reviewed wording for already-landed PRs.

Adds assess_already_landed_report_wording(report_text, *, already_landed, review_validated=False) to review_proofs.py:

Already-landed mode (already_landed=True) — the report must carry the reconciliation wording from the issue: Oldest open PR requiring action, Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED, Candidate head SHA, Reviewed head SHA: none, Review worktree used: false. Any legacy phrase (oldest eligible PR, next eligible PR, Pinned reviewed head, Scratch worktree used) anywhere in the report fails, as does a populated reviewed head SHA — an already-landed PR is reconciliation-only, never review/merge eligible.
Non-already-landed mode — a populated Pinned reviewed head or Reviewed head SHA fails unless review_validated=True (validation and diff review actually passed this session). This covers blocked-infra and validation-failed handoffs claiming a reviewed head prematurely.
Fails closed; returns {complete, downgraded, reasons} matching sibling verifiers.

Tests

New tests/test_already_landed_report_wording.py: 11 tests — correct reconciliation wording, each forbidden legacy field (oldest/next eligible, pinned head, scratch worktree), missing eligibility class, populated reviewed SHA, normal reviewed report, blocked-infra report, validation-failed report with and without reviewed-SHA claim (acceptance criteria of #298).
TDD: tests verified failing on import first; 11/11 pass after.
Full suite in the branch worktree: 1226 passed, 6 skipped, 20 subtests, 2 failed. Both failures reproduce on the clean branch base e1dabd1 with my changes stashed (verified) and on current master: test_agent_temp_artifacts.py::TestIssueLockArtifactWarning (tracked by #359, fix pending in PR #364) and test_final_report_validator.py::TestReviewPrRules::test_clean_reviewer_report_passes (appears to be an interaction between the #327 validator fixture and the #320 legacy-field rejection that both landed on master today). Neither is introduced by this branch.

Closes #298

🤖 Generated with Claude Code

## Summary Implements #298: final report validator must reject eligible/reviewed wording for already-landed PRs. Adds `assess_already_landed_report_wording(report_text, *, already_landed, review_validated=False)` to `review_proofs.py`: - **Already-landed mode** (`already_landed=True`) — the report must carry the reconciliation wording from the issue: `Oldest open PR requiring action`, `Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED`, `Candidate head SHA`, `Reviewed head SHA: none`, `Review worktree used: false`. Any legacy phrase (`oldest eligible PR`, `next eligible PR`, `Pinned reviewed head`, `Scratch worktree used`) anywhere in the report fails, as does a populated reviewed head SHA — an already-landed PR is reconciliation-only, never review/merge eligible. - **Non-already-landed mode** — a populated `Pinned reviewed head` or `Reviewed head SHA` fails unless `review_validated=True` (validation and diff review actually passed this session). This covers blocked-infra and validation-failed handoffs claiming a reviewed head prematurely. - Fails closed; returns `{complete, downgraded, reasons}` matching sibling verifiers. ## Tests - New `tests/test_already_landed_report_wording.py`: 11 tests — correct reconciliation wording, each forbidden legacy field (oldest/next eligible, pinned head, scratch worktree), missing eligibility class, populated reviewed SHA, normal reviewed report, blocked-infra report, validation-failed report with and without reviewed-SHA claim (acceptance criteria of #298). - TDD: tests verified failing on import first; 11/11 pass after. - Full suite in the branch worktree: `1226 passed, 6 skipped, 20 subtests, 2 failed`. Both failures reproduce on the clean branch base e1dabd1 with my changes stashed (verified) and on current master: `test_agent_temp_artifacts.py::TestIssueLockArtifactWarning` (tracked by #359, fix pending in PR #364) and `test_final_report_validator.py::TestReviewPrRules::test_clean_reviewer_report_passes` (appears to be an interaction between the #327 validator fixture and the #320 legacy-field rejection that both landed on master today). Neither is introduced by this branch. Closes #298 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jcwalker3 added 1 commit 2026-07-07 04:39:02 -05:00

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

Add assess_already_landed_report_wording to review_proofs: when the
already-landed gate fires, final reports and controller handoffs must
use the reconciliation wording (Oldest open PR requiring action,
Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED, Candidate head
SHA, Reviewed head SHA: none, Review worktree used: false) and must not
use the legacy fields (oldest/next eligible PR, Pinned reviewed head,
Scratch worktree used) or claim a reviewed head SHA. In
non-already-landed reports, a populated Pinned reviewed head or
Reviewed head SHA fails unless validation and diff review actually
passed this session. Fails closed. Tests cover the correct
reconciliation wording, each forbidden legacy field, missing required
fields, populated reviewed SHA, normal reviewed reports, blocked-infra
reports, and validation-failed reports.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

jcwalker3 referenced this pull request

2026-07-07 04:46:29 -05:00

feat: add dedicated reconciliation controller-handoff schema (Closes #303) #373

jcwalker3 referenced this pull request

2026-07-07 08:20:45 -05:00

feat: verify canonical workflow citation and version in review reports (Closes #296) #380

sysadmin approved these changes 2026-07-07 08:25:30 -05:00

sysadmin left a comment

All tests pass successfully after merging master (1300 passed, 1 failed [pre-existing]). Reviewed the implementation of assess_already_landed_report_wording in review_proofs.py and the test coverage in tests/test_already_landed_report_wording.py. It correctly rejects eligible/reviewed wording for already-landed PRs. Verified that the remaining failure is pre-existing on master.

All tests pass successfully after merging master (1300 passed, 1 failed [pre-existing]). Reviewed the implementation of `assess_already_landed_report_wording` in `review_proofs.py` and the test coverage in `tests/test_already_landed_report_wording.py`. It correctly rejects eligible/reviewed wording for already-landed PRs. Verified that the remaining failure is pre-existing on master.

sysadmin merged commit b41d9d6006 into master

2026-07-07 08:25:45 -05:00

jcwalker3 referenced this pull request

2026-07-07 08:33:00 -05:00

feat: enforce canonical reconciliation handoff schema (Closes #307) #387

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#367