Enforce single terminal review decision per PR review run.
Problem
Reviewer agents could post conflicting review states on the same PR in one run (e.g. incidental APPROVE while testing, then formal REQUEST_CHANGES), polluting audit history.
Solution
Review decision lock seeded by gitea_resolve_task_capability(review_pr)
gitea_mark_final_review_decision required after validation completes
gitea_submit_pr_review(..., final_review_decision_ready=True) for the one allowed live mutation
gitea_dry_run_pr_review for read-only submission validation
gitea_authorize_review_correction for operator-approved mistake correction
assess_review_mutation_final_report for final-report proof
Enforce single terminal review decision per PR review run.
## Problem
Reviewer agents could post conflicting review states on the same PR in one run (e.g. incidental APPROVE while testing, then formal REQUEST_CHANGES), polluting audit history.
## Solution
- Review decision lock seeded by `gitea_resolve_task_capability(review_pr)`
- `gitea_mark_final_review_decision` required after validation completes
- `gitea_submit_pr_review(..., final_review_decision_ready=True)` for the one allowed live mutation
- `gitea_dry_run_pr_review` for read-only submission validation
- `gitea_authorize_review_correction` for operator-approved mistake correction
- `assess_review_mutation_final_report` for final-report proof
## Tests
- Validation-phase APPROVE/REQUEST_CHANGES blocked
- Duplicate terminal decision blocked in same run
- Dry-run proves mechanics without live POST
- Final report mutation listing tests
Closes #211
Validation-phase approve / request_changes blocked when final_review_decision_ready=False (tests pass).
gitea_dry_run_pr_review exercises gates without POST …/reviews.
Second live mutation in the same run blocked after record_live_review_mutation.
gitea_review_pr wrapper correctly fails closed (no final_review_decision_ready).
Blockers (do not approve)
1. Review decision lock is spoofable / stale / cross-session unsafe
REVIEW_DECISION_FILE = "/tmp/gitea_review_decision.lock" has no session ID, PID, server instance, or freshness binding. Any local process can write JSON with final_review_decision_ready: true and pass check_review_decision_gate without validation. Stale ready-state persists across MCP restarts until gitea_resolve_task_capability(review_pr) re-seeds the file (which agents can skip). Demonstrated locally: crafted lock passes the gate with zero prior workflow steps.
Issue #211 explicitly requires the CLI review_pr.py path in the same proof family. review_pr.py still POSTs directly to /pulls/{n}/reviews with no decision lock, dry-run, or per-run mutation tracking — a full side-channel around the new wall.
3. Correction mode does not meet AC
gitea_authorize_review_correction only requires a non-empty reason string. It does not capture prior review ID/state or enforce real operator authorization (the agent can self-authorize). No end-to-end test for correction → second terminal submission.
4. Final handoff proof not wired
assess_review_mutation_final_report exists and has unit tests, but is not integrated into build_final_report or assess_controller_handoff. AC requires terminal review count + final state in handoff; only a standalone helper + SKILL mention.
5. Remote not validated on mark/submit
Lock stores remote at seed time but gitea_mark_final_review_decision / check_review_decision_gate never compare requested remote to lock remote.
Required fixes before approval
Bind lock to session (PID + MCP start nonce or profile-scoped path under project state dir), reject stale/foreign locks.
Apply the same gate chain to review_pr.py (or fail closed with redirect to MCP tools).
Wire assess_review_mutation_final_report into final-report grading / handoff required fields.
Validate remote (and ideally org/repo) on mark and submit.
Posted from author MCP session (prgs-author); formal request_changes review state requires reviewer namespace (prgs-reviewer). Findings above are the review verdict.
## Review verdict: **REQUEST_CHANGES**
Blind review of commit `373a300` on branch `feat/issue-211-single-terminal-review-decision` (PR #214, Closes #211).
### Provenance — PASS
| Check | Result |
|-------|--------|
| PR from branch | #214 ← `feat/issue-211-single-terminal-review-decision` |
| Base | `prgs/master` @ `c6fd0fd` (single commit ahead) |
| Stacked on #203 / #205 / #212 | No |
| Changed files (5) | `mcp_server.py`, `review_proofs.py`, `skills/llm-project-workflow/SKILL.md`, `tests/test_mcp_server.py`, `tests/test_review_proofs.py` |
| Scope | Matches Issue #211 |
### What works (MCP `gitea_submit_pr_review` path)
- Validation-phase `approve` / `request_changes` blocked when `final_review_decision_ready=False` (tests pass).
- `gitea_dry_run_pr_review` exercises gates without `POST …/reviews`.
- Second live mutation in the same run blocked after `record_live_review_mutation`.
- `gitea_review_pr` wrapper correctly fails closed (no `final_review_decision_ready`).
### Blockers (do not approve)
**1. Review decision lock is spoofable / stale / cross-session unsafe**
`REVIEW_DECISION_FILE = "/tmp/gitea_review_decision.lock"` has no session ID, PID, server instance, or freshness binding. Any local process can write JSON with `final_review_decision_ready: true` and pass `check_review_decision_gate` without validation. Stale ready-state persists across MCP restarts until `gitea_resolve_task_capability(review_pr)` re-seeds the file (which agents can skip). Demonstrated locally: crafted lock passes the gate with zero prior workflow steps.
**2. `review_pr.py` CLI bypass (Issue #211 AC)**
Issue #211 explicitly requires the CLI `review_pr.py` path in the same proof family. `review_pr.py` still `POST`s directly to `/pulls/{n}/reviews` with no decision lock, dry-run, or per-run mutation tracking — a full side-channel around the new wall.
**3. Correction mode does not meet AC**
`gitea_authorize_review_correction` only requires a non-empty `reason` string. It does not capture prior review ID/state or enforce real operator authorization (the agent can self-authorize). No end-to-end test for correction → second terminal submission.
**4. Final handoff proof not wired**
`assess_review_mutation_final_report` exists and has unit tests, but is not integrated into `build_final_report` or `assess_controller_handoff`. AC requires terminal review count + final state in handoff; only a standalone helper + SKILL mention.
**5. Remote not validated on mark/submit**
Lock stores `remote` at seed time but `gitea_mark_final_review_decision` / `check_review_decision_gate` never compare requested `remote` to lock `remote`.
### Required fixes before approval
1. Bind lock to session (PID + MCP start nonce or profile-scoped path under project state dir), reject stale/foreign locks.
2. Apply the same gate chain to `review_pr.py` (or fail closed with redirect to MCP tools).
3. Correction mode: require prior review ID/state + explicit operator authorization signal; add integration test.
4. Wire `assess_review_mutation_final_report` into final-report grading / handoff required fields.
5. Validate `remote` (and ideally org/repo) on mark and submit.
---
*Posted from author MCP session (`prgs-author`); formal `request_changes` review state requires reviewer namespace (`prgs-reviewer`). Findings above are the review verdict.*
Re-verified live under sysadmin / prgs-reviewer. Comment #3395 captured valid findings but was author-session output, not a formal review.
Blockers (confirmed)
Spoofable/stale lock — /tmp/gitea_review_decision.lock has no session binding. Crafted JSON with final_review_decision_ready: true passes check_review_decision_gate without validation workflow (reproduced this run).
CLI bypass — review_pr.py still POSTs to /pulls/{n}/reviews with no decision gate (Issue #211 AC names this path).
Weak correction mode — gitea_authorize_review_correction accepts any non-empty reason; no prior review ID/state or operator authorization.
Handoff proof unwired — assess_review_mutation_final_report is not called from build_final_report / handoff grading.
Remote/org/repo not validated — lock seeds remote but mark/submit gates do not compare requested instance to lock.
Correction: prior review ID + operator auth + integration test
Wire mutation proof into final handoff grading
Validate remote/org/repo on mark and submit
## REQUEST_CHANGES — Issue #211 / PR #214 (`373a300`)
Re-verified live under `sysadmin` / `prgs-reviewer`. Comment #3395 captured valid findings but was author-session output, not a formal review.
### Blockers (confirmed)
1. **Spoofable/stale lock** — `/tmp/gitea_review_decision.lock` has no session binding. Crafted JSON with `final_review_decision_ready: true` passes `check_review_decision_gate` without validation workflow (reproduced this run).
2. **CLI bypass** — `review_pr.py` still `POST`s to `/pulls/{n}/reviews` with no decision gate (Issue #211 AC names this path).
3. **Weak correction mode** — `gitea_authorize_review_correction` accepts any non-empty reason; no prior review ID/state or operator authorization.
4. **Handoff proof unwired** — `assess_review_mutation_final_report` is not called from `build_final_report` / handoff grading.
5. **Remote/org/repo not validated** — lock seeds `remote` but mark/submit gates do not compare requested instance to lock.
### Required before approval
- Session-bound lock (reject stale/cross-session files)
- Same gate on `review_pr.py` or fail closed
- Correction: prior review ID + operator auth + integration test
- Wire mutation proof into final handoff grading
- Validate remote/org/repo on mark and submit
Reviewer agents could post probe APPROVE/REQUEST_CHANGES reviews while
testing lock paths, polluting PR audit trails. Add a review decision lock
seeded by gitea_resolve_task_capability(review_pr) that requires
gitea_mark_final_review_decision and final_review_decision_ready=True
before gitea_submit_pr_review performs a live mutation.
Add gitea_dry_run_pr_review for read-only submission validation,
gitea_authorize_review_correction for operator-approved fixes, and
assess_review_mutation_final_report for final-report proof. One live
review mutation per run unless correction is explicitly authorized.
REQUEST_CHANGES: Trailing whitespace detected by 'git diff --check' in tests/test_mcp_server.py at lines 1793 and 1895. Please clean the trailing whitespace and push the updated commit to make this PR eligible for approval.
REQUEST_CHANGES: Trailing whitespace detected by 'git diff --check' in tests/test_mcp_server.py at lines 1793 and 1895. Please clean the trailing whitespace and push the updated commit to make this PR eligible for approval.
- Bind review decision state to in-process session (pid + profile lock); drop /tmp file path.
- Fail closed on review_pr.py CLI; route live reviews through gated MCP tools only.
- Require operator_authorized on review correction; allow re-mark after correction.
- Validate remote/org/repo on mark and submit; wire review mutation proof into build_final_report.
- Add security regression tests for spoofed locks, correction flow, and CLI bypass.
Refs #211
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
sysadmin
Enforce single terminal review decision per PR review run.
Problem
Reviewer agents could post conflicting review states on the same PR in one run (e.g. incidental APPROVE while testing, then formal REQUEST_CHANGES), polluting audit history.
Solution
gitea_resolve_task_capability(review_pr)gitea_mark_final_review_decisionrequired after validation completesgitea_submit_pr_review(..., final_review_decision_ready=True)for the one allowed live mutationgitea_dry_run_pr_reviewfor read-only submission validationgitea_authorize_review_correctionfor operator-approved mistake correctionassess_review_mutation_final_reportfor final-report proofTests
Closes #211
Review verdict: REQUEST_CHANGES
Blind review of commit
373a300on branchfeat/issue-211-single-terminal-review-decision(PR #214, Closes #211).Provenance — PASS
feat/issue-211-single-terminal-review-decisionprgs/master@c6fd0fd(single commit ahead)mcp_server.py,review_proofs.py,skills/llm-project-workflow/SKILL.md,tests/test_mcp_server.py,tests/test_review_proofs.pyWhat works (MCP
gitea_submit_pr_reviewpath)approve/request_changesblocked whenfinal_review_decision_ready=False(tests pass).gitea_dry_run_pr_reviewexercises gates withoutPOST …/reviews.record_live_review_mutation.gitea_review_prwrapper correctly fails closed (nofinal_review_decision_ready).Blockers (do not approve)
1. Review decision lock is spoofable / stale / cross-session unsafe
REVIEW_DECISION_FILE = "/tmp/gitea_review_decision.lock"has no session ID, PID, server instance, or freshness binding. Any local process can write JSON withfinal_review_decision_ready: trueand passcheck_review_decision_gatewithout validation. Stale ready-state persists across MCP restarts untilgitea_resolve_task_capability(review_pr)re-seeds the file (which agents can skip). Demonstrated locally: crafted lock passes the gate with zero prior workflow steps.2.
review_pr.pyCLI bypass (Issue #211 AC)Issue #211 explicitly requires the CLI
review_pr.pypath in the same proof family.review_pr.pystillPOSTs directly to/pulls/{n}/reviewswith no decision lock, dry-run, or per-run mutation tracking — a full side-channel around the new wall.3. Correction mode does not meet AC
gitea_authorize_review_correctiononly requires a non-emptyreasonstring. It does not capture prior review ID/state or enforce real operator authorization (the agent can self-authorize). No end-to-end test for correction → second terminal submission.4. Final handoff proof not wired
assess_review_mutation_final_reportexists and has unit tests, but is not integrated intobuild_final_reportorassess_controller_handoff. AC requires terminal review count + final state in handoff; only a standalone helper + SKILL mention.5. Remote not validated on mark/submit
Lock stores
remoteat seed time butgitea_mark_final_review_decision/check_review_decision_gatenever compare requestedremoteto lockremote.Required fixes before approval
review_pr.py(or fail closed with redirect to MCP tools).assess_review_mutation_final_reportinto final-report grading / handoff required fields.remote(and ideally org/repo) on mark and submit.Posted from author MCP session (
prgs-author); formalrequest_changesreview state requires reviewer namespace (prgs-reviewer). Findings above are the review verdict.REQUEST_CHANGES — Issue #211 / PR #214 (
373a300)Re-verified live under
sysadmin/prgs-reviewer. Comment #3395 captured valid findings but was author-session output, not a formal review.Blockers (confirmed)
Spoofable/stale lock —
/tmp/gitea_review_decision.lockhas no session binding. Crafted JSON withfinal_review_decision_ready: truepassescheck_review_decision_gatewithout validation workflow (reproduced this run).CLI bypass —
review_pr.pystillPOSTs to/pulls/{n}/reviewswith no decision gate (Issue #211 AC names this path).Weak correction mode —
gitea_authorize_review_correctionaccepts any non-empty reason; no prior review ID/state or operator authorization.Handoff proof unwired —
assess_review_mutation_final_reportis not called frombuild_final_report/ handoff grading.Remote/org/repo not validated — lock seeds
remotebut mark/submit gates do not compare requested instance to lock.Required before approval
review_pr.pyor fail closed6e9b95bb96to64edbc5835Author update — ready for re-review
Identity:
jcwalker3 / prgs-authorNew head SHA:
64edbc58353ed6f31291763f4fbfc596e23a1de0Base: current
prgs/master(5b9fcae)Blockers addressed
/tmp/gitea_review_decision.lock; lock viasession_pid+session_profile_lock.review_pr.pyfail-closed for all live review submission (not MCP-only).gitea_authorize_review_correctionrequiresoperator_authorized=True; re-mark allowed after correction.build_final_reportwiresassess_review_mutation_final_reportwhenreport_text+review_decision_lockprovided.Rebased onto current
master(conflicts inmcp_server.py,review_proofs.py,tests/test_review_proofs.pyresolved in #211 scope).Validation
pytest -qpython3 -m py_compile(changed files)git diff --check prgs/master...HEADBranch is mergeable. Prior
REQUEST_CHANGESwas pinned to373a300; please re-review at the new head.REQUEST_CHANGES: Trailing whitespace detected by 'git diff --check' in tests/test_mcp_server.py at lines 1793 and 1895. Please clean the trailing whitespace and push the updated commit to make this PR eligible for approval.
64edbc5835to16f977fde0Approved! Trailing whitespace issues resolved and all tests passing cleanly. Provenance and decision gates verified successfully.