Add per-PR reviewer leases for safe parallel PR cleanup #407

New Issue

2026-07-07T10:15:53-05:00

jcwalker3 commented

2026-07-07 10:15:53 -05:00

Multiple reviewer LLM sessions may need to review different PRs at the same time. Today this is unsafe because sessions can race the same PR queue under the same reviewer identity, causing stale inventories, duplicate review attempts, target-branch drift, and conflicting worktrees.

Recent observed failure:
One reviewer run listed PR #380 as open, then another sysadmin / prgs-reviewer session approved and merged PR #380 mid-run. The first run correctly stopped because its inventory and target branch were stale. This shows we need reviewer queue/PR lease tracking before allowing parallel reviewer sessions.

Required behavior:
Parallel reviewers are allowed only when each reviewer holds a live lease for a distinct PR. A reviewer must not validate, approve, request changes, merge, or cleanup a PR unless it holds the lease for that PR.

Lease location:
Prefer storing the lease as a structured comment on the linked issue and/or PR. If labels are used, they should be supplemental, not the only proof.

Suggested labels:

review:in-progress
review:stale
review:blocked

Suggested lease comment format:

<!-- mcp-review-lease:v1 -->
repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #382
issue: #295
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: <generated-session-id>
worktree: branches/review-pr382-...
phase: claimed | validating | approved | request-changes | merging | blocked | done
candidate_head: <sha>
target_branch: master
target_branch_sha: <sha>
last_activity: <iso timestamp>
expires_at: <iso timestamp>
blocker: none

Acceptance criteria:

Add a reviewer lease model for PR review/merge work.
Before reviewing a PR, the reviewer must acquire a lease for that PR.
A reviewer cannot acquire a lease if another unexpired lease exists for the same PR.
A reviewer may work in parallel only if each reviewer holds a different PR lease.
Lease acquisition records:
- repo
- PR number
- linked issue
- reviewer identity
- profile
- session id
- worktree path
- candidate head SHA
- target branch SHA
- phase
- last activity
- expiry time
Lease heartbeat updates are required:
- after lease acquisition
- before validation
- after validation
- before review mutation
- before merge
- after merge or request-changes
- at least every 30 minutes during long work
Suggested default lease TTL:
- stale warning after 30 minutes with no heartbeat
- reclaimable after 60 minutes if no heartbeat and no active worktree/process proof
- blocked/manual cleanup after 120 minutes
If target branch changes during a lease, the reviewer must stop or refresh inventory and revalidate according to canonical workflow.
If PR head changes during a lease, the reviewer must stop and restart validation.
If another reviewer merges any PR that advances target branch, all active reviewer leases must be marked target-stale unless they explicitly refresh and revalidate.
Queue inventory must show:

PRs with active review leases
stale review leases
reclaimable review leases
PRs awaiting review
PRs blocked by prior request-changes

The canonical review workflow must refuse review/merge mutation unless the current session owns the selected PR lease.
Cleanup mode must release the lease after terminal mutation or blocked final report.
Tests cover:

two reviewers cannot lease the same PR
two reviewers can lease different PRs
stale lease becomes warning
expired lease becomes reclaimable
PR head change invalidates lease
target branch change invalidates lease
merge releases lease
request-changes releases or marks blocked
reviewer without lease cannot approve or merge
stale lease cleanup requires exact capability

Final reports must include:

lease id/session id
lease acquisition proof
lease heartbeat status
lease release/blocked status
whether any other reviewer changed the queue during the run

Non-goals:

Do not rely only on labels.
Do not allow silent takeover of another reviewer’s active PR.
Do not allow one reviewer run to mutate multiple PRs after a terminal review mutation.
Do not allow branch/worktree cleanup without lease or cleanup-policy proof.

Multiple reviewer LLM sessions may need to review different PRs at the same time. Today this is unsafe because sessions can race the same PR queue under the same reviewer identity, causing stale inventories, duplicate review attempts, target-branch drift, and conflicting worktrees. Recent observed failure: One reviewer run listed PR #380 as open, then another `sysadmin / prgs-reviewer` session approved and merged PR #380 mid-run. The first run correctly stopped because its inventory and target branch were stale. This shows we need reviewer queue/PR lease tracking before allowing parallel reviewer sessions. Required behavior: Parallel reviewers are allowed only when each reviewer holds a live lease for a distinct PR. A reviewer must not validate, approve, request changes, merge, or cleanup a PR unless it holds the lease for that PR. Lease location: Prefer storing the lease as a structured comment on the linked issue and/or PR. If labels are used, they should be supplemental, not the only proof. Suggested labels: - `review:in-progress` - `review:stale` - `review:blocked` Suggested lease comment format: ```text  repo: Scaled-Tech-Consulting/Gitea-Tools pr: #382 issue: #295 reviewer_identity: sysadmin profile: prgs-reviewer session_id: <generated-session-id> worktree: branches/review-pr382-... phase: claimed | validating | approved | request-changes | merging | blocked | done candidate_head: <sha> target_branch: master target_branch_sha: <sha> last_activity: <iso timestamp> expires_at: <iso timestamp> blocker: none ``` Acceptance criteria: 1. Add a reviewer lease model for PR review/merge work. 2. Before reviewing a PR, the reviewer must acquire a lease for that PR. 3. A reviewer cannot acquire a lease if another unexpired lease exists for the same PR. 4. A reviewer may work in parallel only if each reviewer holds a different PR lease. 5. Lease acquisition records: - repo - PR number - linked issue - reviewer identity - profile - session id - worktree path - candidate head SHA - target branch SHA - phase - last activity - expiry time 6. Lease heartbeat updates are required: - after lease acquisition - before validation - after validation - before review mutation - before merge - after merge or request-changes - at least every 30 minutes during long work 7. Suggested default lease TTL: - stale warning after 30 minutes with no heartbeat - reclaimable after 60 minutes if no heartbeat and no active worktree/process proof - blocked/manual cleanup after 120 minutes 8. If target branch changes during a lease, the reviewer must stop or refresh inventory and revalidate according to canonical workflow. 9. If PR head changes during a lease, the reviewer must stop and restart validation. 10. If another reviewer merges any PR that advances target branch, all active reviewer leases must be marked target-stale unless they explicitly refresh and revalidate. 11. Queue inventory must show: - PRs with active review leases - stale review leases - reclaimable review leases - PRs awaiting review - PRs blocked by prior request-changes 12. The canonical review workflow must refuse review/merge mutation unless the current session owns the selected PR lease. 13. Cleanup mode must release the lease after terminal mutation or blocked final report. 14. Tests cover: - two reviewers cannot lease the same PR - two reviewers can lease different PRs - stale lease becomes warning - expired lease becomes reclaimable - PR head change invalidates lease - target branch change invalidates lease - merge releases lease - request-changes releases or marks blocked - reviewer without lease cannot approve or merge - stale lease cleanup requires exact capability 15. Final reports must include: - lease id/session id - lease acquisition proof - lease heartbeat status - lease release/blocked status - whether any other reviewer changed the queue during the run Non-goals: * Do not rely only on labels. * Do not allow silent takeover of another reviewer’s active PR. * Do not allow one reviewer run to mutate multiple PRs after a terminal review mutation. * Do not allow branch/worktree cleanup without lease or cleanup-policy proof.

jcwalker3 added the status:in-progress label 2026-07-07 12:04:35 -05:00

jcwalker3 commented

2026-07-07 12:04:35 -05:00

Issue claim heartbeat

kind: claim
issue: Add per-PR reviewer leases for safe parallel PR cleanup (#407)
branch: feat/issue-407-reviewer-pr-leases
phase: claimed
profile: prgs-author
pr: none
blocker: none
next_action: create worktree and begin implementation

**Issue claim heartbeat** - kind: claim - issue: #407 - branch: feat/issue-407-reviewer-pr-leases - phase: claimed - profile: prgs-author - pr: none - blocker: none - next_action: create worktree and begin implementation

jcwalker3 referenced this issue from a commit

2026-07-07 12:07:29 -05:00

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

jcwalker3 referenced a pull request that will close this issue

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

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

jcwalker3 referenced this issue

2026-07-07 12:20:59 -05:00

Tracker: MCP Control Plane web UI #425

jcwalker3 referenced this issue

2026-07-07 12:21:02 -05:00

Web UI: Lease and collision visibility #433