prune force-deletes a branch whose content is not in the default branch #564
Labels
No labels
breaking
bug
documentation
enhancement
epic
good first issue
help wanted
refactoring
resolution/duplicate
resolution/invalid
resolution/wontfix
security
severity/critical
severity/high
severity/low
severity/medium
status/abandoned
status/blocked
status/needs-decision
status/needs-info
status/needs-verification
testing
upstream
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Blocks
#563 Epic: the destructive surface has outgrown ADR 0029's classification table
sixfold-space/madtea
Reference
sixfold-space/madtea#564
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Part of #563.
ADR 0029 classifies
pruneas machine-proof: it "deletes a ref only after content-verifying it is contained in the default branch". The predicate behind that claim does not verify containment.The defect
LikelySquashMergedshort-circuits onTreeIdenticalbefore the per-commit check (internal/service/git/branch_merge_state.go:32-38):TreeIdenticalcomes fromgit diff --quiet <base> <branch>. It compares two tip trees. It says nothing about the branch's intermediate commits. A branch that adds content and later removes it has a tip tree identical to the default branch while holding content that exists in no other branch.Reproduction
In a scratch repo, branch
feataddsX.txtthen runsgit rm X.txt:prune --delete-branchesforce-deletesfeat. The content survives only in the reflog until it is collected. Thegit cherryarm of the same function would have refused.The delete-time re-verify does not help. It is intact and correct (
deleteSquashMerged,internal/service/prune/run.go:593-635), but it re-runs the same predicate and re-confirms the same wrong answer. A re-verify guards against state changing between the survey and the delete. It cannot guard against a weak predicate.Fix
Drop the short-circuit, so containment requires
CherryEquivalent && AheadUnique == 0. If a tip-tree match is still wanted as a fast path, it has to be conjoined withAheadUnique == 0rather than returning early.Acceptance criteria
prune.docs/reference/destructive.mdand ADR 0029's table still describe the predicate accurately after the change.Verification status, so nobody treats this as settled.
The predicate is confirmed: I read
LikelySquashMergedatinternal/service/git/branch_merge_state.go:32-38and theTreeIdenticalshort-circuit is there, ahead of the per-commit check, exactly as described.The scratch-repo reproduction in the body is not independently confirmed. The reasoning follows from the predicate, but the commands were not re-run before filing. Re-run them before writing the regression test, and correct this issue if the observed behaviour differs.