prune force-deletes a branch whose content is not in the default branch #564

Open
opened 2026-09-21 21:01:56 +00:00 by hexajon · 1 comment
Owner

Part of #563.

ADR 0029 classifies prune as 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

LikelySquashMerged short-circuits on TreeIdentical before the per-commit check (internal/service/git/branch_merge_state.go:32-38):

if s.Ancestor { return false }
if s.TreeIdentical { return true }
return s.CherryEquivalent && s.AheadUnique == 0

TreeIdentical comes from git 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 feat adds X.txt then runs git rm X.txt:

git merge-base --is-ancestor feat main  -> exit 1   Ancestor      = false
git diff --quiet main feat              -> exit 0   TreeIdentical = true
=> LikelySquashMerged() == true

git cherry main feat  ->  + 8052f65
                          + dfb176e     both commits unique
git cat-file -p feat~1:X.txt  ->  "secret work"

prune --delete-branches force-deletes feat. The content survives only in the reflog until it is collected. The git cherry arm 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 with AheadUnique == 0 rather than returning early.

Acceptance criteria

  • A branch whose tip tree matches the default branch while holding content unique to itself is never force-deleted by prune.
  • The reproduction above is a regression test, and it fails against the current predicate.
  • docs/reference/destructive.md and ADR 0029's table still describe the predicate accurately after the change.
Part of #563. ADR 0029 classifies `prune` as 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 `LikelySquashMerged` short-circuits on `TreeIdentical` before the per-commit check (`internal/service/git/branch_merge_state.go:32-38`): ```go if s.Ancestor { return false } if s.TreeIdentical { return true } return s.CherryEquivalent && s.AheadUnique == 0 ``` `TreeIdentical` comes from `git 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 `feat` adds `X.txt` then runs `git rm X.txt`: ``` git merge-base --is-ancestor feat main -> exit 1 Ancestor = false git diff --quiet main feat -> exit 0 TreeIdentical = true => LikelySquashMerged() == true git cherry main feat -> + 8052f65 + dfb176e both commits unique git cat-file -p feat~1:X.txt -> "secret work" ``` `prune --delete-branches` force-deletes `feat`. The content survives only in the reflog until it is collected. The `git cherry` arm 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 with `AheadUnique == 0` rather than returning early. ## Acceptance criteria - A branch whose tip tree matches the default branch while holding content unique to itself is never force-deleted by `prune`. - The reproduction above is a regression test, and it fails against the current predicate. - `docs/reference/destructive.md` and ADR 0029's table still describe the predicate accurately after the change.
Author
Owner

Verification status, so nobody treats this as settled.

The predicate is confirmed: I read LikelySquashMerged at internal/service/git/branch_merge_state.go:32-38 and the TreeIdentical short-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.

Verification status, so nobody treats this as settled. The predicate is confirmed: I read `LikelySquashMerged` at `internal/service/git/branch_merge_state.go:32-38` and the `TreeIdentical` short-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.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
sixfold-space/madtea#564
No description provided.