feat: enforce session hard-stop after terminal review mutation (Closes #332) #345

2026-07-07T03:33:47-05:00

jcwalker3 commented

2026-07-07 03:33:47 -05:00

Closes #332

Summary

Extends the single-terminal-decision machinery (#211) to session-level hard-stop semantics:

terminal_review_hard_stop_reasons (new helper beside the #211 lock): after a live terminal verdict, the only permitted continuation is the merge sequence for the same approved PR. REQUEST_CHANGES stops the run. Operator-approved corrections (#211) re-open the review path only — never a cross-PR merge. Stop guidance names the consumed terminal mutation and the continuation rule applied.
gitea_merge_pr Gate 2b: consults the hard-stop between confirmation and eligibility — local, zero API calls — so a run can never submit a verdict on one PR and merge another.
gitea_mark_final_review_decision: hard-stop check with the terminal-specific stop message; plus duplicate REQUEST_CHANGES suppression — an unresolved request-changes at the current head SHA refuses a second request-changes, and an unverifiable feedback state fails closed.

Validation

pytest tests/test_terminal_review_hard_stop.py -q — 13 passed (acceptance scenarios: REQUEST_CHANGES stops everything; approve permits same-PR merge only; cross-PR merge/review/mark-ready blocked; correction re-opens review path only; duplicate RC blocked at same head, allowed when stale/no blocker, fail-closed when unverifiable; approve path never fetches feedback)
pytest tests/test_mcp_server.py tests/test_terminal_review_hard_stop.py -q — 197 passed
Full suite (pytest tests/ --ignore=tests/integration) in the issue worktree — 1111 tests, 1 failed: TestIssueLockArtifactWarning::test_lock_success_includes_artifact_warning — pre-existing on clean master, re-proven this session in baseline worktree branches/baseline-master-pr291 at current master fd396df (identical failure); not caused by this branch
git diff --check — clean; secret-pattern scan — no hits (one grep match is the pre-existing _SECRET_PREFIXES constant in diff context)
Validated at head 456e46d

Risk

Low-moderate. Additive gate before the merge API and in mark-ready; fail-closed direction only (never widens permissions). Existing #211 correction flow preserved and covered by updated tests. Three existing TestSubmitPrReview setups now stub the new feedback fetch — behavior change is intentional per AC6.

Worktree

/Users/jasonwalker/Development/Gitea-Tools/branches/feat-issue-332-terminal-review-hard-stop (branch feat/issue-332-terminal-review-hard-stop; main checkout stayed on master, not used for task work)

Closes #332 ## Summary Extends the single-terminal-decision machinery (#211) to session-level hard-stop semantics: - **`terminal_review_hard_stop_reasons`** (new helper beside the #211 lock): after a live terminal verdict, the only permitted continuation is the merge sequence for the same approved PR. REQUEST_CHANGES stops the run. Operator-approved corrections (#211) re-open the review path only — never a cross-PR merge. Stop guidance names the consumed terminal mutation and the continuation rule applied. - **`gitea_merge_pr` Gate 2b**: consults the hard-stop between confirmation and eligibility — local, zero API calls — so a run can never submit a verdict on one PR and merge another. - **`gitea_mark_final_review_decision`**: hard-stop check with the terminal-specific stop message; plus **duplicate REQUEST_CHANGES suppression** — an unresolved request-changes at the current head SHA refuses a second request-changes, and an unverifiable feedback state fails closed. ## Validation - `pytest tests/test_terminal_review_hard_stop.py -q` — 13 passed (acceptance scenarios: REQUEST_CHANGES stops everything; approve permits same-PR merge only; cross-PR merge/review/mark-ready blocked; correction re-opens review path only; duplicate RC blocked at same head, allowed when stale/no blocker, fail-closed when unverifiable; approve path never fetches feedback) - `pytest tests/test_mcp_server.py tests/test_terminal_review_hard_stop.py -q` — 197 passed - Full suite (`pytest tests/ --ignore=tests/integration`) in the issue worktree — 1111 tests, **1 failed**: `TestIssueLockArtifactWarning::test_lock_success_includes_artifact_warning` — **pre-existing on clean master**, re-proven this session in baseline worktree `branches/baseline-master-pr291` at current master `fd396df` (identical failure); not caused by this branch - `git diff --check` — clean; secret-pattern scan — no hits (one grep match is the pre-existing `_SECRET_PREFIXES` constant in diff context) - Validated at head `456e46d` ## Risk Low-moderate. Additive gate before the merge API and in mark-ready; fail-closed direction only (never widens permissions). Existing #211 correction flow preserved and covered by updated tests. Three existing `TestSubmitPrReview` setups now stub the new feedback fetch — behavior change is intentional per AC6. ## Worktree `/Users/jasonwalker/Development/Gitea-Tools/branches/feat-issue-332-terminal-review-hard-stop` (branch `feat/issue-332-terminal-review-hard-stop`; main checkout stayed on `master`, not used for task work)

jcwalker3 added 1 commit 2026-07-07 03:33:48 -05:00

feat: enforce session hard-stop after terminal review mutation (#332 ) 456e46d3fc

Extends the single-terminal-decision machinery (#211) to session-level
hard-stop semantics:

- terminal_review_hard_stop_reasons: after a live terminal verdict, only
  the merge sequence for the same approved PR may continue; REQUEST_CHANGES
  stops the run; operator-approved corrections re-open the review path
  only, never a cross-PR merge.
- gitea_merge_pr: new Gate 2b consults the hard-stop before any API call,
  so a run can never verdict one PR and merge another.
- gitea_mark_final_review_decision: hard-stop check with explicit stop
  guidance naming the consumed terminal mutation; plus duplicate
  REQUEST_CHANGES suppression — an unresolved request-changes at the
  current head SHA refuses a second request-changes (fail closed when
  feedback cannot be verified).
- tests/test_terminal_review_hard_stop.py: 13 tests covering the
  acceptance criteria; existing TestSubmitPrReview setups stub the new
  feedback fetch.

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

sysadmin approved these changes 2026-07-07 04:09:07 -05:00

sysadmin left a comment

All tests pass successfully. Reviewed the implementation in gitea_mcp_server.py and test coverage in tests/test_terminal_review_hard_stop.py. It correctly enforces session hard-stop after a terminal live review mutation, allowing only the merge sequence for the same approved PR, and suppresses duplicate REQUEST_CHANGES reviews on the same head SHA. Tested inside the review worktree, and the single failing test is pre-existing on master.

sysadmin merged commit 407483f6e0 into master

2026-07-07 04:09:19 -05:00

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#345