test(prune): Guard the property that makes a force-delete safe - #77
Merged
Conversation
`pr-merged` is the only reason that force-deletes a branch, and nothing in `branchDeletionFor` gates that force. What makes it safe is one line in `selectPRForBranch`: a merged PR only counts when its head SHA equals the local tip, so a branch carrying commits made after the merge stops matching and falls through to a reason that uses `-d`. Three review lanes independently proposed adding a `rev-list --not --remotes` gate on the force, and that proposal was measured wrong (the count is > 0 for every correctly squash-merged branch, so it would refuse the feature's main path). A comment now records why it is absent; this pins the property that comment depends on, so it cannot regress quietly. Guarded at both levels: at `selectPRForBranch`, and end to end through `scanRepo` driving the real `ghPRStateLookup` against a stub `gh`, because `scanRepo` does no identity filtering of its own and a `lookupPR` stub would bypass the code under test. The end-to-end pair asserts the outcome that actually matters: after a real prune run the branch survives and the unpublished commit is still reachable. Verified to fail: removing the tip check makes all three go red, including the one that proves the commit would be destroyed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #76.
Why
pr-mergedis the only prune reason that force-deletes a branch withgit branch -D. What makes that safe is a single property: a PR only counts as this branch's PR when itsheadRefOidequals the local branch tip. If that check ever weakens, this is the failure it produces:git worktree prune(dropping the per-worktree reflog), and force-deletes the branch, taking all three recovery handles at once.During review of #76 that scenario was proposed as unfixed and a
git rev-list --count <branch> --not --remotes == 0gate was suggested three separate times. The gate is measurably wrong — it reports 0 while the remote branch still exists and 1 after auto-delete-on-merge, so withdelete_branch_on_mergeenabled it would refuse to clean up exactly the squash-merged branches the feature exists for. The real protection is the identity check, one function away, which is easy to miss from the deletion site. #76 documents that in a comment; this pins it as tests.What is asserted
Three tests, at both levels:
selectPRForBranch: a merged PR whoseheadRefOidis not the local tip returns nothing. Carries an explicit "do not delete as a duplicate" note, since it resembles the branch-name-reuse case but exists for a different reason.scanRepoagainst a real git fixture, driving the realghPRStateLookupthrough a stubghon PATH. Injecting alookupPRstub instead would bypass the exact code under test, becausescanRepodoes no identity filtering itself.runPrune, asserting the outcome that actually matters:branchDeleted === false, the branch still listed, and the unpublished commit still resolvable by SHA.Verification
The guards were confirmed capable of failing, not merely passing: removing the tip check turns all three red, including the one proving the commit is destroyed. Source restored afterwards.
bun run typecheckclean;bun test3402 pass / 0 fail (main's 3399 plus these three). Test-only change, +144 lines in one file.