feat: add per-PR reviewer leases for parallel review (Closes #407) #424

2026-07-07T12:07:54-05:00

jcwalker3 commented

2026-07-07 12:07:54 -05:00

Summary

Closes #407.

Adds structured per-PR reviewer leases stored as PR-thread comments so parallel reviewer sessions can work different PRs without racing the same queue entry.

Changes

reviewer_pr_lease.py — lease format, acquire assessment, freshness classification, mutation gate
gitea_acquire_reviewer_pr_lease — post lease comment and record in-session ownership
gitea_heartbeat_reviewer_pr_lease — phase/activity updates
gitea_assess_reviewer_pr_lease — read-only lease inspection
Review/merge mutation gates require owned active lease
review-merge-pr.md §26B documents lease workflow
Tests: tests/test_reviewer_pr_lease.py (8 cases)

Validation

venv/bin/python -m pytest tests/test_reviewer_pr_lease.py -q

8 passed

Worktree

branches/feat-issue-407-reviewer-pr-leases @ a9d6a9c

## Summary Closes #407. Adds structured per-PR reviewer leases stored as PR-thread comments so parallel reviewer sessions can work different PRs without racing the same queue entry. ## Changes - `reviewer_pr_lease.py` — lease format, acquire assessment, freshness classification, mutation gate - `gitea_acquire_reviewer_pr_lease` — post lease comment and record in-session ownership - `gitea_heartbeat_reviewer_pr_lease` — phase/activity updates - `gitea_assess_reviewer_pr_lease` — read-only lease inspection - Review/merge mutation gates require owned active lease - `review-merge-pr.md` §26B documents lease workflow - Tests: `tests/test_reviewer_pr_lease.py` (8 cases) ## Validation ``` venv/bin/python -m pytest tests/test_reviewer_pr_lease.py -q ``` **8 passed** ## Worktree `branches/feat-issue-407-reviewer-pr-leases` @ `a9d6a9c`

sysadmin requested changes 2026-07-07 15:24:20 -05:00

Dismissed

sysadmin left a comment

Independent audit — REQUEST CHANGES (test regressions detected)

Auditor: sysadmin / prgs-reviewer (not PR author jcwalker3).
Pinned HEAD: a9d6a9c863497ac27ce0b8f339f2c32c041c4995

Worktree + Cleanliness

Fresh isolated review worktree created successfully.
git status clean before and after running tests.

Regression Test Failures

Running the target tests passed:

pytest tests/test_reviewer_pr_lease.py -q -> 8 passed.

However, running the full test suite (pytest) revealed 25 regression failures across existing review, merge, audit, and inventory tests.

Root Cause

The newly introduced _reviewer_pr_lease_gate unconditionally intercepts all live reviewer and merger mutations (gitea_submit_pr_review, gitea_merge_pr) to check for an active reviewer lease comment on Gitea.
This extra GET request (_fetch_pr_comments) consumes mock side effects in existing tests and fails closed because no reviewer lease is established or mocked in those pre-existing tests.

Required Action

Please update or mock the lease gate in the pre-existing unit tests (e.g., by adding a test helper to mock/stub lease checking, or updating mock side-effects) so that the entire test suite passes cleanly.

## Independent audit — REQUEST CHANGES (test regressions detected) Auditor: `sysadmin` / `prgs-reviewer` (not PR author `jcwalker3`). Pinned HEAD: `a9d6a9c863497ac27ce0b8f339f2c32c041c4995` ### Worktree + Cleanliness - Fresh isolated review worktree created successfully. - `git status` clean before and after running tests. ### Regression Test Failures Running the target tests passed: - `pytest tests/test_reviewer_pr_lease.py -q` -> 8 passed. However, running the full test suite (`pytest`) revealed **25 regression failures** across existing review, merge, audit, and inventory tests. ### Root Cause The newly introduced `_reviewer_pr_lease_gate` unconditionally intercepts all live reviewer and merger mutations (`gitea_submit_pr_review`, `gitea_merge_pr`) to check for an active reviewer lease comment on Gitea. This extra GET request (`_fetch_pr_comments`) consumes mock side effects in existing tests and fails closed because no reviewer lease is established or mocked in those pre-existing tests. ### Required Action Please update or mock the lease gate in the pre-existing unit tests (e.g., by adding a test helper to mock/stub lease checking, or updating mock side-effects) so that the entire test suite passes cleanly.

sysadmin approved these changes 2026-07-07 15:37:53 -05:00

Dismissed

sysadmin left a comment

APPROVED — reviewer: sysadmin/prgs-reviewer (not author). Reviewed updated PR #424 head 8b61c4b in isolated worktree branches/review-feat-issue-407-reviewer-pr-leases (clean before + after; no local edits).

Changes verified:

Scoped strictly to test harness fixes in tests/test_audit.py, tests/test_mcp_server.py, and tests/test_pr_queue_inventory.py.
Installs mock owned reviewer lease during test setup, avoiding side-effect drift without weakening production lease gates or mutation checks.
Enforced lease checks (_reviewer_pr_lease_gate) remain fully intact in production code.

Test verification:

Focused lease tests passed (pytest tests/test_reviewer_pr_lease.py -q -> 8 passed).
Integration test suite passed cleanly (pytest tests/ -q -> 16088 passed, 6 skipped).
Verified clean full-suite behavior from a clean review worktree with no lock pollution.

Approved!

APPROVED — reviewer: sysadmin/prgs-reviewer (not author). Reviewed updated PR #424 head `8b61c4b` in isolated worktree `branches/review-feat-issue-407-reviewer-pr-leases` (clean before + after; no local edits). Changes verified: - Scoped strictly to test harness fixes in `tests/test_audit.py`, `tests/test_mcp_server.py`, and `tests/test_pr_queue_inventory.py`. - Installs mock owned reviewer lease during test setup, avoiding side-effect drift without weakening production lease gates or mutation checks. - Enforced lease checks (`_reviewer_pr_lease_gate`) remain fully intact in production code. Test verification: - Focused lease tests passed (`pytest tests/test_reviewer_pr_lease.py -q` -> 8 passed). - Integration test suite passed cleanly (`pytest tests/ -q` -> 16088 passed, 6 skipped). - Verified clean full-suite behavior from a clean review worktree with no lock pollution. Approved!

jcwalker3 added 3 commits 2026-07-07 16:18:10 -05:00

feat: add per-PR reviewer leases for parallel review (Closes #407 ) 06c2069234

Structured PR-thread leases with acquire/heartbeat MCP tools and
mutation-time enforcement so reviewers cannot race the same PR queue.

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

test: fix reviewer lease gate regressions in MCP integration tests (#407 ) 1830360850

Align merge/review/audit mocks with the per-PR reviewer lease gate introduced
in #407: install owned session leases after init_review_decision_lock, stub
_fetch_pr_comments and _authenticated_username to preserve mock ordering, and
update head-SHA mismatch expectations for lease-first fail-closed paths.

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

test: align duplicate gate lock fixtures with #447 provenance (#407 ) 3e4b721d60

Rebase onto master (#413/#463) requires sanctioned lock_provenance in
create_pr duplicate-recheck tests.

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

jcwalker3 force-pushed feat/issue-407-reviewer-pr-leases from 8b61c4b41f to 3e4b721d60

2026-07-07 16:18:10 -05:00

Compare

sysadmin approved these changes 2026-07-07 16:27:00 -05:00

sysadmin left a comment

Re-review after rebase onto master (`cc4741a`)

Pinned head: 3e4b721d60e97147ba0704773cf57cd0d42cbe31
Reviewer: sysadmin / prgs-reviewer
Prior approval at 8b61c4b is stale — fresh validation at rebased head.

Scope (#407 only)

reviewer_pr_lease.py — lease format, acquire/freshness/mutation gates
gitea_mcp_server.py — acquire/heartbeat/assess tools + _reviewer_pr_lease_gate on review/merge paths
review-merge-pr.md §26B documentation
Tests isolated via _install_owned_reviewer_lease helper
Rebase conflict fix: duplicate-gate lock fixtures aligned with #447 provenance (from #413/#463)

Lease gate preservation

Production review/merge mutations require owned in-session lease + active PR-thread lease
Head SHA drift blocks mutation (fail-closed)
Parallel reviewers blocked on same PR; different PRs allowed
No weakening of eligibility, terminal-review, or self-review gates

Tests (detached worktree @ `3e4b721`, branches/pr424-review-detached)

pytest tests/test_reviewer_pr_lease.py -q                          → 8 passed
pytest tests/test_mcp_server.py::{MergePR,SubmitPrReview,...} -q   → 65 passed
test_submit_review_success_audited                                 → 1 passed
test_reviewer_profile_can_proceed_from_inventory_to_review_gates   → 1 passed
pytest tests/ -q                                                   → 1672 passed, 6 skipped
git diff --check                                                   → clean

Verdict

APPROVE — rebased head preserves reviewer PR lease behavior; scope limited to #407; no #440 umbrella leakage.

## Re-review after rebase onto master (cc4741a) **Pinned head:** `3e4b721d60e97147ba0704773cf57cd0d42cbe31` **Reviewer:** sysadmin / prgs-reviewer **Prior approval at 8b61c4b is stale** — fresh validation at rebased head. ### Scope (#407 only) - `reviewer_pr_lease.py` — lease format, acquire/freshness/mutation gates - `gitea_mcp_server.py` — acquire/heartbeat/assess tools + `_reviewer_pr_lease_gate` on review/merge paths - `review-merge-pr.md` §26B documentation - Tests isolated via `_install_owned_reviewer_lease` helper - Rebase conflict fix: duplicate-gate lock fixtures aligned with #447 provenance (from #413/#463) ### Lease gate preservation - Production review/merge mutations require owned in-session lease + active PR-thread lease - Head SHA drift blocks mutation (fail-closed) - Parallel reviewers blocked on same PR; different PRs allowed - No weakening of eligibility, terminal-review, or self-review gates ### Tests (detached worktree @ 3e4b721, branches/pr424-review-detached) ``` pytest tests/test_reviewer_pr_lease.py -q → 8 passed pytest tests/test_mcp_server.py::{MergePR,SubmitPrReview,...} -q → 65 passed test_submit_review_success_audited → 1 passed test_reviewer_profile_can_proceed_from_inventory_to_review_gates → 1 passed pytest tests/ -q → 1672 passed, 6 skipped git diff --check → clean ``` ### Verdict **APPROVE** — rebased head preserves reviewer PR lease behavior; scope limited to #407; no #440 umbrella leakage.

sysadmin merged commit f061fee18d into master

2026-07-07 16:27:20 -05:00

jcwalker3 referenced this pull request

2026-07-07 16:30:08 -05:00

Merge gate must require approval for the current PR head SHA #471

sysadmin referenced this pull request

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

feat: add conflict-fix leases and stale-head protection (Closes #399) #416

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