[bug][reconciler] No PR-scoped post-merge cleanup executor: merged-PR worktree/branch reclaim is batch-only and stalls behind expired reviewer leases #855

New Issue

2026-07-23T17:36:50-05:00

jcwalker3 commented

2026-07-23 17:36:50 -05:00

Problem

There is no sanctioned way for a reconciler to complete post-merge cleanup for exactly one merged PR.

gitea_reconcile_merged_cleanups(dry_run=true) correctly prescribes the safe sequence — remove safe registered worktrees, reassess branch ownership, then delete the remote branch. But its execute path is batch-wide: it has no pr_number (or equivalent single-target) filter, so applying the prescription for one PR would also act on every other merged PR and its worktrees in the same sweep. gitea_cleanup_merged_pr_branch is PR-scoped but only deletes the remote branch; it cannot remove the registered worktrees, and therefore cannot clear the ownership binding that blocks its own deletion step.

Net effect: a single merged PR whose cleanup requires the worktree-removal step first has no safe execution path at all. Operators must either leave the residue in place indefinitely, or run a batch reconciliation that touches unrelated PRs — which is not an acceptable blast radius.

A second, compounding defect appears in the same scenario: an expired reviewer lease continues to protect the merged branch because expiry alone is not treated as proof of reclaim. With no live claimant and the PR already merged, that lease should not be able to protect the branch forever.

Reproduction case (PR #818 / issue #638)

Observed state at master 188e83c4. This residue is deliberately left untouched as the reproduction fixture — do not clean it up while triaging this issue.

PR #818 is merged; issue #638 is closed.
The merger lease for #818 is already consumed; no active lease remains.
Remote branch feat/issue-638-webui-app-shell-phase1 still exists.
Registered worktree branches/issue-638-webui-app-shell-phase1 still binds that branch.
Detached review worktree branches/review-pr-818 also remains.
An expired reviewer lease still protects the branch, because reclaim is not proven.
reconcile_merged_cleanups(dry_run=true) prescribes: worktree removal -> ownership reassessment -> branch deletion.
That executor is batch-wide and would clean unrelated PRs and worktrees.
No PR-scoped worktree-cleanup executor and no pr_number filter exist.
cleanup_merged_pr_branch therefore cannot complete the prescribed sequence safely.

Relationship to issue #851

Issue #851 (closed, remediated) fixed the ordering defect inside the batch executor: a worktree that is independently safe to remove must no longer be skipped just because remote deletion was blocked by that same worktree_binding. That fix is necessary but not sufficient. It corrected the order of operations within a batch sweep; it did not introduce any way to scope that sweep to one merged PR, and it did not address expired reviewer leases protecting an already-merged branch. This issue tracks the remaining gap.

Required behavior

A reconciler can target exactly one merged PR for post-merge cleanup.
For that PR only, it can remove the PR's safe registered author and review worktrees, leaving unrelated worktrees untouched.
After worktree removal it reassesses branch ownership rather than reusing the pre-removal assessment.
An expired reviewer lease on an already-merged PR does not indefinitely protect the merged branch, provided no live claimant exists.
It can then delete exactly that merged PR's remote branch when doing so is independently safe.
Dry-run and apply modes both report every intended target and every mutation.
All operations stay fail-closed.

Acceptance criteria

A reconciler operation accepts a single merged PR as its target and acts on that PR alone.
That operation removes the target PR's safe registered author worktree and review worktree, and touches no unrelated worktree.
Branch ownership is reassessed after worktree removal, not carried over from the earlier assessment.
An expired reviewer lease on an already-merged PR does not indefinitely protect the merged branch when no live claimant exists; reclaim of an expired lease is decided explicitly rather than defaulting to permanent protection.
The target merged PR's remote branch can then be deleted when it is independently safe to delete.
Dry-run mode reports every intended target and every intended mutation.
Apply mode reports every target acted on and every mutation performed.
Operations remain fail-closed for dirty worktrees, live ownership, ambiguous PR-to-worktree/branch mappings, and parity failures.
Tests prove unrelated worktrees, branches, leases, PRs, and issues remain untouched by a PR-scoped run.

Non-goals

Cleaning up PR #818 residue as part of triage; it is retained as the reproduction fixture.
Weakening ownership guards for remote branch deletion.
Re-opening or otherwise altering PR #818 or issue #638.
Replacing the batch reconciliation path; PR-scoped execution is additive.

References

Reproduction: PR #818 / issue #638 post-merge cleanup, observed at master 188e83c4.
Ordering-only predecessor: issue #851 (closed).
Code paths: gitea_reconcile_merged_cleanups execute loop; gitea_cleanup_merged_pr_branch.

## Problem There is no sanctioned way for a reconciler to complete post-merge cleanup for **exactly one** merged PR. `gitea_reconcile_merged_cleanups(dry_run=true)` correctly *prescribes* the safe sequence — remove safe registered worktrees, reassess branch ownership, then delete the remote branch. But its execute path is **batch-wide**: it has no `pr_number` (or equivalent single-target) filter, so applying the prescription for one PR would also act on every other merged PR and its worktrees in the same sweep. `gitea_cleanup_merged_pr_branch` is PR-scoped but only deletes the remote branch; it cannot remove the registered worktrees, and therefore cannot clear the ownership binding that blocks its own deletion step. Net effect: a single merged PR whose cleanup requires the worktree-removal step first has **no safe execution path at all**. Operators must either leave the residue in place indefinitely, or run a batch reconciliation that touches unrelated PRs — which is not an acceptable blast radius. A second, compounding defect appears in the same scenario: an **expired** reviewer lease continues to protect the merged branch because expiry alone is not treated as proof of reclaim. With no live claimant and the PR already merged, that lease should not be able to protect the branch forever. ## Reproduction case (PR #818 / issue #638) Observed state at master `188e83c4`. **This residue is deliberately left untouched as the reproduction fixture — do not clean it up while triaging this issue.** - PR #818 is merged; issue #638 is closed. - The merger lease for #818 is already consumed; no active lease remains. - Remote branch `feat/issue-638-webui-app-shell-phase1` still exists. - Registered worktree `branches/issue-638-webui-app-shell-phase1` still binds that branch. - Detached review worktree `branches/review-pr-818` also remains. - An expired reviewer lease still protects the branch, because reclaim is not proven. - `reconcile_merged_cleanups(dry_run=true)` prescribes: worktree removal -> ownership reassessment -> branch deletion. - That executor is batch-wide and would clean unrelated PRs and worktrees. - No PR-scoped worktree-cleanup executor and no `pr_number` filter exist. - `cleanup_merged_pr_branch` therefore cannot complete the prescribed sequence safely. ## Relationship to issue #851 Issue #851 (closed, remediated) fixed the **ordering** defect inside the batch executor: a worktree that is independently safe to remove must no longer be skipped just because remote deletion was blocked by that same `worktree_binding`. That fix is necessary but not sufficient. It corrected the order of operations within a batch sweep; it did not introduce any way to **scope** that sweep to one merged PR, and it did not address expired reviewer leases protecting an already-merged branch. This issue tracks the remaining gap. ## Required behavior 1. A reconciler can target exactly one merged PR for post-merge cleanup. 2. For that PR only, it can remove the PR's safe registered author and review worktrees, leaving unrelated worktrees untouched. 3. After worktree removal it reassesses branch ownership rather than reusing the pre-removal assessment. 4. An expired reviewer lease on an already-merged PR does not indefinitely protect the merged branch, provided no live claimant exists. 5. It can then delete exactly that merged PR's remote branch when doing so is independently safe. 6. Dry-run and apply modes both report every intended target and every mutation. 7. All operations stay fail-closed. ## Acceptance criteria - [ ] A reconciler operation accepts a single merged PR as its target and acts on that PR alone. - [ ] That operation removes the target PR's safe registered author worktree and review worktree, and touches no unrelated worktree. - [ ] Branch ownership is reassessed after worktree removal, not carried over from the earlier assessment. - [ ] An expired reviewer lease on an already-merged PR does not indefinitely protect the merged branch when no live claimant exists; reclaim of an expired lease is decided explicitly rather than defaulting to permanent protection. - [ ] The target merged PR's remote branch can then be deleted when it is independently safe to delete. - [ ] Dry-run mode reports every intended target and every intended mutation. - [ ] Apply mode reports every target acted on and every mutation performed. - [ ] Operations remain fail-closed for dirty worktrees, live ownership, ambiguous PR-to-worktree/branch mappings, and parity failures. - [ ] Tests prove unrelated worktrees, branches, leases, PRs, and issues remain untouched by a PR-scoped run. ## Non-goals - Cleaning up PR #818 residue as part of triage; it is retained as the reproduction fixture. - Weakening ownership guards for remote branch deletion. - Re-opening or otherwise altering PR #818 or issue #638. - Replacing the batch reconciliation path; PR-scoped execution is additive. ## References - Reproduction: PR #818 / issue #638 post-merge cleanup, observed at master `188e83c4`. - Ordering-only predecessor: issue #851 (closed). - Code paths: `gitea_reconcile_merged_cleanups` execute loop; `gitea_cleanup_merged_pr_branch`.

jcwalker3 added the status:ready workflow-hardening type:bug labels 2026-07-23 17:36:51 -05:00

jcwalker3 added status:in-progress and removed status:ready labels 2026-07-23 19:17:35 -05:00

jcwalker3 commented

2026-07-23 19:17:35 -05:00

Issue claim heartbeat

kind: claim
issue: [bug][reconciler] No PR-scoped post-merge cleanup executor: merged-PR worktree/branch reclaim is batch-only and stalls behind expired reviewer leases (#855)
branch: fix/issue-855-pr-scoped-merged-cleanup
phase: claimed
profile: prgs-author
pr: none
blocker: none
next_action: create worktree and begin implementation

**Issue claim heartbeat** - kind: claim - issue: #855 - branch: fix/issue-855-pr-scoped-merged-cleanup - phase: claimed - profile: prgs-author - pr: none - blocker: none - next_action: create worktree and begin implementation

sysadmin referenced this issue

2026-07-23 19:43:56 -05:00

Implement native author issue worktree bootstrap (#850) #853

jcwalker3 referenced this issue

2026-07-23 20:01:40 -05:00

Make worktree cleanup audit merged-PR aware for issue worktrees #858

jcwalker3 referenced this issue from a commit

2026-07-23 20:20:01 -05:00

fix(worktree-audit): make cleanup audit merged-PR aware for issue worktrees (Closes #858)

jcwalker3 referenced this issue

2026-07-23 20:21:52 -05:00

fix(worktree-audit): make cleanup audit merged-PR aware for issue worktrees (Closes #858) #859