Add fail-closed proofs for blind PR queue review workflow (Issue #173) #176

2026-07-05T13:31:51-05:00

jcwalker3 commented

2026-07-05 13:31:51 -05:00

This PR addresses Gitea-Tools Issue #173 by adding the reviewer-side workflow proofs, tests, and harness assertions for blind PR queue reviews.

Changes:

New review_proofs.py — pure, fail-closed proof helpers:
- verify_pinned_head_checkout: proves local HEAD equals the pinned PR head SHA (full 40-hex, prefix/abbreviated matches rejected) and that the diff base is the PR base branch; any mismatch blocks review/merge.
- assess_inventory_completeness: an exhaustive "only PRs found" claim requires every configured repository listed with a stated state filter, proven pagination, and a reported total open-PR count.
- assess_validation_report: a validation result is unclaimable unless the exact command was stated and its output read; missing passed/failed/skipped counts, unjustified ignored paths, or unexplained deviation from the canonical command downgrade the report to "weak".
- assess_self_review_contamination: contamination must be evidence-backed; a same-session authorship claim without an evidence source (in either direction) yields "unknown" with a choose-another-PR-or-stop action — never an assumed verdict.
- build_final_report: distinguishes identity eligibility, author/reviewer distinctness, session contamination, validation-on-pinned-head, merge performed, and issue verification; grades "A" only when every proof is present, downgrades otherwise, and marks a merge performed without allowing proofs as a blocked violation.
New tests/test_review_proofs.py (35 tests) covering all seven Required-behavior-7 harness assertions from the issue, including: fetched-but-unchecked-out pinned head blocks merge; complete multi-repo inventory; unfinished pagination refuses the exhaustive claim; missing test counts flagged weak; contamination requires evidence; unread output cannot claim validation passed; ignored paths need explicit justification.
skills/llm-project-workflow/SKILL.md §F and skills/llm-project-workflow/templates/review-pr.md updated to require the checkout proof, inventory proof, exact validation reporting, evidence-backed contamination assessment, and the distinguished final report.

No review/merge safety gate is weakened; the MCP-level gates (mcp-control-plane #68/#76) remain authoritative for mutations.

Validation:

venv/bin/pytest tests/test_review_proofs.py -q: 35 passed
venv/bin/pytest tests/ --ignore=branches -q (canonical; branches/ holds review worktrees that duplicate test module names and break collection): 681 tests, 0 failures, 0 errors, 6 skipped (exit 0; counts read from junitxml because a pre-existing suite quirk closes stdout mid-run — noted below)
py_compile on both new files: passed
git diff --check (unstaged and staged): passed
Secret/provenance sweep over the staged diff: clean; no URLs or credentials

Note for reviewers: running the full suite prints no terminal output (stdout appears to be closed by a test mid-run) even though the run passes; junitxml confirms the counts. This quirk pre-exists this branch and may deserve its own issue.

Closes #173

This PR addresses Gitea-Tools Issue #173 by adding the reviewer-side workflow proofs, tests, and harness assertions for blind PR queue reviews. Changes: 1. New `review_proofs.py` — pure, fail-closed proof helpers: - `verify_pinned_head_checkout`: proves local `HEAD` equals the pinned PR head SHA (full 40-hex, prefix/abbreviated matches rejected) and that the diff base is the PR base branch; any mismatch blocks review/merge. - `assess_inventory_completeness`: an exhaustive "only PRs found" claim requires every configured repository listed with a stated state filter, proven pagination, and a reported total open-PR count. - `assess_validation_report`: a validation result is unclaimable unless the exact command was stated and its output read; missing passed/failed/skipped counts, unjustified ignored paths, or unexplained deviation from the canonical command downgrade the report to "weak". - `assess_self_review_contamination`: contamination must be evidence-backed; a same-session authorship claim without an evidence source (in either direction) yields "unknown" with a choose-another-PR-or-stop action — never an assumed verdict. - `build_final_report`: distinguishes identity eligibility, author/reviewer distinctness, session contamination, validation-on-pinned-head, merge performed, and issue verification; grades "A" only when every proof is present, downgrades otherwise, and marks a merge performed without allowing proofs as a blocked violation. 2. New `tests/test_review_proofs.py` (35 tests) covering all seven Required-behavior-7 harness assertions from the issue, including: fetched-but-unchecked-out pinned head blocks merge; complete multi-repo inventory; unfinished pagination refuses the exhaustive claim; missing test counts flagged weak; contamination requires evidence; unread output cannot claim validation passed; ignored paths need explicit justification. 3. `skills/llm-project-workflow/SKILL.md` §F and `skills/llm-project-workflow/templates/review-pr.md` updated to require the checkout proof, inventory proof, exact validation reporting, evidence-backed contamination assessment, and the distinguished final report. No review/merge safety gate is weakened; the MCP-level gates (mcp-control-plane #68/#76) remain authoritative for mutations. Validation: - `venv/bin/pytest tests/test_review_proofs.py -q`: 35 passed - `venv/bin/pytest tests/ --ignore=branches -q` (canonical; `branches/` holds review worktrees that duplicate test module names and break collection): 681 tests, 0 failures, 0 errors, 6 skipped (exit 0; counts read from junitxml because a pre-existing suite quirk closes stdout mid-run — noted below) - `py_compile` on both new files: passed - `git diff --check` (unstaged and staged): passed - Secret/provenance sweep over the staged diff: clean; no URLs or credentials Note for reviewers: running the full suite prints no terminal output (stdout appears to be closed by a test mid-run) even though the run passes; junitxml confirms the counts. This quirk pre-exists this branch and may deserve its own issue. Closes #173

jcwalker3 added 1 commit 2026-07-05 13:31:51 -05:00

feat(review-workflow): add fail-closed proofs for blind PR queue review (#173 ) ccc5ef79df

Adds review_proofs.py with pure, fail-closed helpers for the reviewer-side
blind queue workflow:

- verify_pinned_head_checkout: proves HEAD == pinned PR head (full 40-hex,
  no prefix matches) and diff base == PR base; any mismatch blocks
  review/merge.
- assess_inventory_completeness: exhaustive "only PRs found" claims require
  every configured repo listed with stated filters, proven pagination, and
  reported open-PR totals.
- assess_validation_report: validation results are unclaimable unless the
  exact command was stated and its output read; missing pass/fail/skip
  counts, unjustified ignored paths, or unexplained deviation from the
  canonical command downgrade the report to weak.
- assess_self_review_contamination: contamination must be evidence-backed;
  bare same-session claims (either way) yield "unknown" with a
  choose-another-PR-or-stop action, never an assumed verdict.
- build_final_report: distinguishes identity eligibility, author/reviewer
  distinctness, session contamination, validation-on-pinned-head, merge,
  and issue verification; grades "A" only when every proof is present,
  downgrades otherwise, and marks a merge without allowing proofs as a
  blocked violation.

tests/test_review_proofs.py covers all seven harness assertions from the
issue (unchecked-out pinned head blocks merge; multi-repo inventory;
pagination; missing counts flagged weak; evidence-required contamination;
unread output cannot claim validation passed; ignored paths need
justification).

SKILL.md section F and templates/review-pr.md now require the checkout,
inventory, validation-evidence, and contamination proofs and the
distinguished final report. No review/merge safety gate is weakened.

Closes #173

Co-Authored-By: Claude Fable 5 <[email protected]>

jcwalker3 referenced this pull request

2026-07-05 13:46:20 -05:00

Harden author workflow against branch drift and accidental commits to master #177

jcwalker3 referenced this pull request

2026-07-05 13:58:51 -05:00

Fix test suite stdout capture issue that forces junitxml workaround #178

jcwalker3 referenced this pull request

2026-07-05 14:01:06 -05:00

Harden blind PR queue review workflow: require checked-out pinned head, complete inventory, and evidence-backed eligibility #173

sysadmin referenced this pull request

2026-07-05 14:03:56 -05:00

Harden blind PR queue review workflow: require checked-out pinned head, complete inventory, and evidence-backed eligibility #173

sysadmin reviewed 2026-07-05 14:08:50 -05:00

sysadmin left a comment

I have checked out head SHA ccc5ef79df, verified it matches local HEAD, and reviewed the diff against master. The changes successfully add fail-closed proof helpers to review_proofs.py and comprehensive tests to test_review_proofs.py, addressing all requirements of Issue #173. All targeted and full suite tests pass successfully.

I have checked out head SHA ccc5ef79dfe629853e144763238593bd808d57e0, verified it matches local HEAD, and reviewed the diff against master. The changes successfully add fail-closed proof helpers to review_proofs.py and comprehensive tests to test_review_proofs.py, addressing all requirements of Issue #173. All targeted and full suite tests pass successfully.

sysadmin merged commit 64dc334a92 into master

2026-07-05 14:08:58 -05:00

jcwalker3 referenced this pull request

2026-07-05 14:15:29 -05:00

Tighten A-level reviewer proof requirements after #173 merge #179

jcwalker3 referenced this issue from a commit

2026-07-05 14:16:39 -05:00

feat(author-workflow): add fail-closed branch-identity proofs for commit/push (#177)

jcwalker3 referenced this pull request

2026-07-05 14:17:03 -05:00

test: isolate CLI output capture with monkeypatch/redirect to fix stdout corruption in full suite (Issue #178) #180

jcwalker3 referenced this pull request