From 65893345164f64339d0a31f8af7bb94a8ae75673 Mon Sep 17 00:00:00 2001 From: Dominic Black Date: Tue, 13 Oct 2026 12:03:39 +0100 Subject: [PATCH] Forget worktrees whose directory was deleted before syncing If you delete a worktree's directory by hand rather than with git worktree remove, git still thinks its branch is checked out there. Sync goes by what git says, so a merged branch in a dead worktree was kept forever with "sync from there or remove the worktree", pointing at a directory that no longer exists. The only way out was knowing to run git worktree prune yourself. Sync now starts by looking for worktrees git marks prunable (i.e. their directory is gone), runs git worktree prune and says which ones it forgot, then re-reads the branches so cleanup sees them as ordinary branches and deletes them like any other merged one. Locked worktrees are left alone, the same as git does (that's what the lock is for, a checkout on a drive that isn't mounted for example), but if the directory has gone you get told to run git worktree unlock. --- README.md | 4 ++- docs/architecture.md | 6 ++++- pkg/app/sync.go | 25 +++++++++++++------ pkg/app/sync_cleanup_test.go | 47 ++++++++++++++++++++++++++++++++++++ pkg/app/sync_worktrees.go | 43 +++++++++++++++++++++++++++++++++ pkg/git/branch.go | 22 +++++++++++++++++ pkg/git/worktree_test.go | 42 ++++++++++++++++++++++++++++++++ 7 files changed, 180 insertions(+), 9 deletions(-) create mode 100644 pkg/app/sync_worktrees.go diff --git a/README.md b/README.md index cd90435..1ab3616 100644 --- a/README.md +++ b/README.md @@ -341,7 +341,9 @@ git may already have updated the checkout. It never deletes a lock itself. If the branch being deleted is the one you're on, sync switches you to its trunk first, and like `git switch` your uncommitted changes come along. It only keeps the branch when git says the -switch would overwrite them, and then names the files. +switch would overwrite them, and then names the files. A worktree whose directory you deleted +by hand (rather than with `git worktree remove`) is forgotten at the start of the sync, so its +branches can be cleaned up like any other; a locked one is left alone with a notice. Flags: - `-d` / `--delete-all` delete merged or closed branches without asking diff --git a/docs/architecture.md b/docs/architecture.md index c18fe53..e6e9293 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -272,7 +272,11 @@ dropped then, so it heals over two syncs. A candidate checked out in the current trunk is checked out in another worktree leaves HEAD detached at the trunk tip rather than failing. Moving the current checkout off a branch being deleted is a plain `git switch`, so uncommitted changes git can carry come along; only git's "would be overwritten" refusal keeps -the branch (as `dirty`, files named); any other failure stops the sync as before. Consent for deletion is `stack.sync.prune` (`always` by default), `-d`, or `-f`; +the branch (as `dirty`, files named); any other failure stops the sync as before. Before any of +this, sync runs `git worktree prune` when `git worktree list` marks an entry prunable (its +directory was deleted by hand): otherwise git still counts its branch as checked out there and +the branch could never be deleted. Locked worktrees are never pruned, matching git; one whose +directory is gone gets a notice to unlock it. Consent for deletion is `stack.sync.prune` (`always` by default), `-d`, or `-f`; with no terminal and `ask`, branches are kept with a notice. Nothing is pushed; `submit` does that. diff --git a/pkg/app/sync.go b/pkg/app/sync.go index 044216a..e7432d7 100644 --- a/pkg/app/sync.go +++ b/pkg/app/sync.go @@ -193,6 +193,9 @@ func (a *App) Sync(ctx context.Context, repo git.Repo, o SyncOptions) (SyncResul return res, err } res.Remote = st.remote + if err := a.syncPruneWorktrees(ctx, st, &res); err != nil { + return res, err + } // Listing pull requests is a forge round trip that can take longer than // the fetch itself on a busy repository, so it runs alongside the fetch @@ -290,7 +293,7 @@ func (a *App) gatherSync(ctx context.Context, repo git.Repo) (*syncState, error) if err != nil { return nil, err } - st := &syncState{repo: repo, graph: graph, dirty: newDirtyCache(), local: map[string]git.Branch{}} + st := &syncState{repo: repo, graph: graph, dirty: newDirtyCache()} st.trunks = graph.Trunks if len(st.trunks) == 0 { def, ok, err := a.d.Git.DefaultBranch(ctx, repo) @@ -304,17 +307,25 @@ func (a *App) gatherSync(ctx context.Context, repo git.Repo) (*syncState, error) st.trunks = []string{def} } st.remote = a.d.Git.RemoteFor(ctx, repo, st.trunks[0]) - branches, err := a.d.Git.Branches(ctx, repo) - if err != nil { + if err := a.loadCheckouts(ctx, st); err != nil { return nil, err } + return st, nil +} + +// loadCheckouts reads every local branch, with its tip and where it is +// checked out, and the worktrees. +func (a *App) loadCheckouts(ctx context.Context, st *syncState) error { + branches, err := a.d.Git.Branches(ctx, st.repo) + if err != nil { + return err + } + st.local = make(map[string]git.Branch, len(branches)) for _, b := range branches { st.local[b.Name] = b } - if st.worktrees, err = a.d.Git.Worktrees(ctx, repo); err != nil { - return nil, err - } - return st, nil + st.worktrees, err = a.d.Git.Worktrees(ctx, st.repo) + return err } // moveBranch moves a local branch from one tip to another. A branch nobody diff --git a/pkg/app/sync_cleanup_test.go b/pkg/app/sync_cleanup_test.go index 33c44f7..1ad7d7e 100644 --- a/pkg/app/sync_cleanup_test.go +++ b/pkg/app/sync_cleanup_test.go @@ -2,6 +2,7 @@ package app_test import ( "context" + "os" "path/filepath" "slices" "strings" @@ -416,6 +417,52 @@ func TestSyncKeepsDeletedBranchWhenSwitchWouldOverwriteChanges(t *testing.T) { } } +func TestSyncPrunesWorktreeWhoseDirectoryIsGone(t *testing.T) { + f := newSyncFixture(t) + gittest.Run(t, f.dir, "switch", "-q", "main") + wt := filepath.Join(t.TempDir(), "wt") + gittest.Run(t, f.dir, "worktree", "add", "-q", wt, "a") + if err := os.RemoveAll(wt); err != nil { + t.Fatal(err) + } + f.mergeOnRemote(t, "a", 0) + res, err := f.sync(t, app.SyncOptions{NoRestack: true}) + if err != nil { + t.Fatal(err) + } + if _, ok := deleted(res, "a"); !ok || branchExists(t, f.dir, "a") { + t.Errorf("a should be deleted once its dead worktree is pruned: deleted = %+v kept = %+v", res.Deleted, res.Kept) + } + if n := len(strings.Split(gittest.Run(t, f.dir, "worktree", "list"), "\n")); n != 1 { + t.Errorf("%d worktrees left, want just the main one", n) + } + if !strings.Contains(strings.Join(res.Notices, "\n"), "its directory is gone") { + t.Errorf("notices = %v", res.Notices) + } +} + +func TestSyncLeavesLockedWorktreeWhoseDirectoryIsGone(t *testing.T) { + f := newSyncFixture(t) + gittest.Run(t, f.dir, "switch", "-q", "main") + wt := filepath.Join(t.TempDir(), "wt") + gittest.Run(t, f.dir, "worktree", "add", "-q", wt, "a") + gittest.Run(t, f.dir, "worktree", "lock", wt) + if err := os.RemoveAll(wt); err != nil { + t.Fatal(err) + } + f.mergeOnRemote(t, "a", 0) + res, err := f.sync(t, app.SyncOptions{NoRestack: true}) + if err != nil { + t.Fatal(err) + } + if k, ok := kept(res, "a"); !ok || k.Reason != app.KeptOtherWorktree || !branchExists(t, f.dir, "a") { + t.Errorf("kept = %+v", res.Kept) + } + if !strings.Contains(strings.Join(res.Notices, "\n"), "git worktree unlock") { + t.Errorf("notices = %v", res.Notices) + } +} + func TestSyncDropsGoneBranchesFromMetadata(t *testing.T) { f := newSyncFixture(t) f.deps.Config.SyncPrune = config.SyncPruneNever diff --git a/pkg/app/sync_worktrees.go b/pkg/app/sync_worktrees.go new file mode 100644 index 0000000..b4f6dcb --- /dev/null +++ b/pkg/app/sync_worktrees.go @@ -0,0 +1,43 @@ +package app + +import ( + "context" + "errors" + "io/fs" + "os" +) + +// syncPruneWorktrees forgets worktrees whose directory was deleted without +// git worktree remove. git still counts their branches as checked out +// there, so until they are pruned sync would keep those branches and say to +// sync from a directory that no longer exists. A locked worktree is never +// pruned (that's what the lock is for, e.g. a checkout on a drive that isn't +// mounted), so it only gets a notice. +func (a *App) syncPruneWorktrees(ctx context.Context, st *syncState, res *SyncResult) error { + var gone []string + for _, wt := range st.worktrees { + switch { + case wt.Prunable && !wt.Locked: + gone = append(gone, wt.Path) + case wt.Locked && missing(wt.Path): + res.notice("the worktree at %s is locked, so git keeps it although its directory is gone; run git worktree unlock %s and sync again to clean it up", + shortPath(wt.Path), wt.Path) + } + } + if len(gone) == 0 { + return nil + } + if err := a.d.Git.PruneWorktrees(ctx, st.repo); err != nil { + return err + } + for _, p := range gone { + res.notice("forgot the worktree at %s because its directory is gone", shortPath(p)) + } + return a.loadCheckouts(ctx, st) +} + +// missing reports whether nothing exists at path. +func missing(path string) bool { + _, err := os.Stat(path) + return errors.Is(err, fs.ErrNotExist) +} diff --git a/pkg/git/branch.go b/pkg/git/branch.go index 60ed213..3640c1c 100644 --- a/pkg/git/branch.go +++ b/pkg/git/branch.go @@ -137,6 +137,19 @@ func (c *Client) SwitchDetached(ctx context.Context, repo Repo, rev string) erro return nil } +// PruneWorktrees forgets every worktree whose directory is gone, the way +// git worktree prune does. Locked ones are left alone. +func (c *Client) PruneWorktrees(ctx context.Context, repo Repo) error { + _, err := c.gitIn(ctx, repo, "worktree", "prune") + if err != nil { + if ee, ok := errors.AsType[*exec.ExitError](err); ok { + return fmt.Errorf("worktree prune: %s", ee.Result.Err()) + } + return err + } + return nil +} + // Worktree is one entry of `git worktree list`. type Worktree struct { // Path is the working tree root. @@ -146,6 +159,11 @@ type Worktree struct { Branch string Detached bool Bare bool + // Prunable is set when git would prune the entry, usually because its + // directory has been deleted. + Prunable bool + // Locked is set when the entry is locked against pruning. + Locked bool } // Worktrees lists the main checkout and every linked worktree. @@ -180,6 +198,10 @@ func (c *Client) Worktrees(ctx context.Context, repo Repo) ([]Worktree, error) { cur.Detached = true case line == "bare": cur.Bare = true + case line == "prunable" || strings.HasPrefix(line, "prunable "): + cur.Prunable = true + case line == "locked" || strings.HasPrefix(line, "locked "): + cur.Locked = true } } flush() diff --git a/pkg/git/worktree_test.go b/pkg/git/worktree_test.go index d5fbbc4..1cd443d 100644 --- a/pkg/git/worktree_test.go +++ b/pkg/git/worktree_test.go @@ -3,6 +3,7 @@ package git_test import ( "context" "errors" + "os" "path/filepath" "testing" @@ -42,6 +43,47 @@ func TestWorktrees(t *testing.T) { } } +func TestWorktreesMarksMissingDirectoriesPrunable(t *testing.T) { + gittest.Isolate(t) + dir := gittest.InitRepo(t) + c := newClient() + ctx := context.Background() + repo, err := c.Discover(ctx, dir) + if err != nil { + t.Fatal(err) + } + gone := filepath.Join(t.TempDir(), "gone") + gittest.Run(t, dir, "worktree", "add", "-q", "-b", "gone", gone) + locked := filepath.Join(t.TempDir(), "locked") + gittest.Run(t, dir, "worktree", "add", "-q", "-b", "locked", locked) + gittest.Run(t, dir, "worktree", "lock", "--reason", "on a usb stick", locked) + for _, p := range []string{gone, locked} { + if err := os.RemoveAll(p); err != nil { + t.Fatal(err) + } + } + + wts, err := c.Worktrees(ctx, repo) + if err != nil { + t.Fatal(err) + } + // git never offers a locked worktree for pruning, gone or not. + if len(wts) != 3 || wts[0].Prunable || !wts[1].Prunable || wts[1].Locked || wts[2].Prunable || !wts[2].Locked { + t.Fatalf("worktrees = %+v", wts) + } + + if err := c.PruneWorktrees(ctx, repo); err != nil { + t.Fatal(err) + } + wts, err = c.Worktrees(ctx, repo) + if err != nil { + t.Fatal(err) + } + if len(wts) != 2 || wts[1].Branch != "locked" { + t.Errorf("after prune = %+v; want the locked one kept", wts) + } +} + func TestSwitchReportsFilesInTheWay(t *testing.T) { gittest.Isolate(t) dir := gittest.InitRepo(t)