Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,26 @@ func TestRunHookWorktreeRemoveUsesPrimaryHead(t *testing.T) {
assertKept(t, repo, wtB, "agent-b")
}

func TestRunHookWorktreeRemoveKeepsDuplicateBranch(t *testing.T) {
repo := newTestRepo(t)
ctx, root := loadRepoWithRoot(t, repo)
wt := mustResolve(t, ctx, repo, "agent-abc")
dup := filepath.Join(root, "dup")
gitT(t, repo, "worktree", "add", "-q", "--force", dup, "agent-abc")

// The hook removes by branch, and the branch names two worktrees: it
// must not delete the other one in place of the path it was given.
code, _, stderr := runEda(t, repo, removeHookInput(repo, dup), "hook", "worktree-remove")
if code != 0 {
t.Fatalf("keeping a worktree is a success for the hook: exit=%d stderr=%q", code, stderr)
}
if !strings.Contains(stderr, "worktree kept") {
t.Errorf("the kept worktree must be reported on stderr, got %q", stderr)
}
assertKept(t, repo, wt, "agent-abc")
assertKept(t, repo, dup, "agent-abc")
}

func TestRunHookWorktreeRemoveFromOtherDirectory(t *testing.T) {
// The session cwd follows `cd`, so by the time the session ends it may
// be in another repository or outside any; worktree_path alone must
Expand Down
18 changes: 13 additions & 5 deletions remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ func (b removeBase) headFor(path string) (oid, desc string) {
// is set, it refuses to touch anything when the worktree is dirty or the
// branch tip is not reachable from its base, the rule of `git branch -d`:
// the base is the branch's upstream when that ref resolves, otherwise the
// HEAD that base names.
// HEAD that base names. A branch attached to more than one worktree (which
// `git worktree add --force` allows) is refused: the pair is not unique.
//
// All conditions are checked before any deletion. A failure between the
// worktree removal and the branch deletion leaves the branch behind without
Expand All @@ -91,16 +92,23 @@ func removeWorktree(ctx *repoContext, base removeBase, branch string, force bool
if len(ctx.Entries) > 0 && ctx.Entries[0].Branch == branch {
return refusalf("branch %q is checked out in the primary checkout; eda does not remove it", branch)
}
var entry *worktreeEntry
var matches []*worktreeEntry
for i, e := range ctx.Entries[1:] {
if !e.Bare && !e.Detached && e.Branch == branch {
entry = &ctx.Entries[i+1]
break
matches = append(matches, &ctx.Entries[i+1])
}
}
if entry == nil {
if len(matches) == 0 {
return refusalf("no worktree found for branch %q", branch)
}
if len(matches) > 1 {
paths := make([]string, len(matches))
for i, e := range matches {
paths[i] = e.Path
}
return refusalf("branch %q is attached to more than one worktree (%s); detach the extra ones with `git worktree remove`", branch, strings.Join(paths, ", "))
}
entry := matches[0]
if entry.Locked {
return refusalf("worktree %s is locked; unlock it with `git worktree unlock` first", entry.Path)
}
Expand Down
55 changes: 55 additions & 0 deletions remove_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,61 @@ func TestRemoveWorktreeRefusesPrunable(t *testing.T) {
}
}

func TestRemoveWorktreeRefusesDuplicateBranch(t *testing.T) {
repo := newTestRepo(t)
ctx, root := loadRepoWithRoot(t, repo)
wt := mustResolve(t, ctx, repo, "topic")
// `git worktree add --force` attaches a branch that is already checked
// out elsewhere; the branch then names two worktrees and eda cannot
// tell which one to remove.
dup := filepath.Join(root, "dup")
gitT(t, repo, "worktree", "add", "-q", "--force", dup, "topic")

err := removeFrom(t, repo, repo, "topic", true)
var refusal refusalError
if !errors.As(err, &refusal) {
t.Fatalf("branch attached to two worktrees must be refused, got %v", err)
}
for _, path := range []string{wt, dup} {
if !strings.Contains(err.Error(), path) {
t.Errorf("refusal must list the worktree %q, got %q", path, err)
}
}
list := gitT(t, repo, "worktree", "list", "--porcelain")
for _, path := range []string{wt, dup} {
if !strings.Contains(list, "worktree "+path+"\n") {
t.Errorf("worktree %q must stay registered, got %q", path, list)
}
}
assertKept(t, repo, wt, "topic")
assertKept(t, repo, dup, "topic")
}

func TestRemoveWorktreeRefusesDuplicateBranchOutsideRoot(t *testing.T) {
repo := newTestRepo(t)
ctx, _ := loadRepoWithRoot(t, repo)
wt := mustResolve(t, ctx, repo, "topic")
// The duplicate outside the root is not eda's, but neither is the
// decision which attach is the real one: nothing is touched.
dup := filepath.Join(t.TempDir(), "manual-wt")
gitT(t, repo, "worktree", "add", "-q", "--force", dup, "topic")

err := removeFrom(t, repo, repo, "topic", true)
var refusal refusalError
if !errors.As(err, &refusal) {
t.Fatalf("branch attached to two worktrees must be refused, got %v", err)
}
// The duplicate refusal names both worktrees; the unmanaged refusal
// names only one and must not take precedence.
for _, path := range []string{wt, dup} {
if !strings.Contains(err.Error(), path) {
t.Errorf("refusal must list the worktree %q, got %q", path, err)
}
}
assertKept(t, repo, wt, "topic")
assertKept(t, repo, dup, "topic")
}

func TestRemoveWorktreeCleanCheckIgnoresStatusConfig(t *testing.T) {
repo := newTestRepo(t)
ctx, _ := loadRepoWithRoot(t, repo)
Expand Down