Keep worktrees git could not read, never delete them

Any git error on a directory under the worktree root was classified orphan,
and orphan deletes the directory outright, so a transient failure reading the
source root became data loss on a plain `agentws prune --yes`.

- prove a backing repo gone by stat before calling a directory an orphan
- classify an unexplained git failure as keep, naming the error
- refuse to remove a worktree whose git state is unknown, even with --include-keep
- spell out that --include-keep discards uncommitted and in-progress work

README.md

@@ -181,11 +181,18 @@ spelled out, or `--json` for scripting. Neither form needs a terminal.
 | `--no-fetch` | judge against the refs already on disk, for offline use |
 | `--json` | emit the report as JSON on stdout, notes on stderr |
 | `--include-unmanaged` | also remove worktrees outside the worktree root |
-| `--include-keep` | dangerous: also remove worktrees classified `keep`, never their branch |
+| `--include-keep` | dangerous: also remove worktrees classified `keep`, destroying uncommitted and in-progress work |
 
 Without `--include-unmanaged` a hand-made worktree is reported and then skipped,
 naming the flag that would remove it. `--include-keep` is the only way past a
-`keep`, and it still leaves the branch, so the commits outlive the worktree.
+`keep`. It leaves the branch, so committed work outlives the worktree, but
+`git worktree remove --force` discards a dirty working tree and a paused
+rebase's sequencer state without a word, and no branch was carrying those.
+
+A directory under the worktree root is deleted outright only when its git dir
+and the repo's shared `.git` are both proven absent by `stat`. When git merely
+fails to answer for a checkout, the verdict is `keep` with the error as its
+reason: an unread state is never a dead one.
 
 ### Auth / credential-helper design
 

cmd/agentws/main.go

@@ -231,8 +231,11 @@ func newListCmd() *cobra.Command {
 			}
 			for _, w := range managed {
 				branch := w.branch
-				if w.orphan {
+				switch {
+				case w.orphan:
 					branch = "(orphan)"
+				case w.inspectErr != nil:
+					branch = "(unreadable)"
 				}
 				_, _ = fmt.Fprintf(out, "%s\t%s\t%s\n", w.repo, branch, w.path)
 			}
@@ -258,15 +261,20 @@ type managedWt struct {
 	// missing is a registration whose working tree is gone: nothing to inspect,
 	// nothing to lose.
 	missing bool
-	// orphan is a directory under the worktree root whose backing repo no longer
-	// resolves, so no git state can be read from it at all.
+	// orphan is a directory under the worktree root whose backing git dir is
+	// proven gone, so no git state can be read from it ever again.
 	orphan bool
+	// inspectErr is set when git refused to answer for a checkout and the reason
+	// was not a proven-absent backing repo. The state is unknown, never removable.
+	inspectErr error
 }
 
 // managedWorktrees scans the worktree root and resolves each entry's repo and
 // branch from git so branch names are accurate (not the sanitized dir name).
-// Directories whose backing repo no longer resolves are returned as orphans
-// rather than dropped, so callers can see (and clean up) the leftovers.
+// Directories whose backing repo is proven gone are returned as orphans rather
+// than dropped, so callers can see (and clean up) the leftovers; a directory git
+// merely failed to answer for is returned with its error instead, because an
+// unread state must never be mistaken for a dead one.
 func managedWorktrees() ([]managedWt, error) {
 	wr, err := worktreeRoot()
 	if err != nil {
@@ -291,7 +299,15 @@ func managedWorktrees() ([]managedWt, error) {
 		branch, branchErr := agent.GitCurrentBranch(path)
 		srcDir, srcErr := agent.SourceRepoDir(path)
 		if branchErr != nil || srcErr != nil {
-			out = append(out, managedWt{repo: repoFromDirName(e.Name()), path: path, managed: true, orphan: true})
+			entry := managedWt{repo: repoFromDirName(e.Name()), path: path, managed: true}
+			if gone, err := backingRepoGone(path); err == nil && gone {
+				entry.orphan = true
+			} else if branchErr != nil {
+				entry.inspectErr = branchErr
+			} else {
+				entry.inspectErr = srcErr
+			}
+			out = append(out, entry)
 			continue
 		}
 		out = append(out, managedWt{
@@ -306,6 +322,49 @@ func managedWorktrees() ([]managedWt, error) {
 	return out, nil
 }
 
+// backingRepoGone proves, by stat alone, that a worktree directory's backing
+// repo no longer exists: its .git file names a git dir that is absent, and the
+// repo's shared .git the git dir lived in is absent too. Only that pair licenses
+// deleting the directory. Every other outcome — an unreadable .git file, a git
+// dir still on disk, a stat that failed for any reason other than "not there",
+// or a mere lost registration in a repo that is still present — reports false,
+// so a transient or unexplained failure can never be read as "safe to delete".
+func backingRepoGone(path string) (bool, error) {
+	dot := filepath.Join(path, ".git")
+	info, err := os.Lstat(dot)
+	if err != nil {
+		return false, err
+	}
+	if info.IsDir() {
+		return false, nil // a standalone checkout, not a linked worktree
+	}
+	data, err := os.ReadFile(dot)
+	if err != nil {
+		return false, err
+	}
+	rest, ok := strings.CutPrefix(strings.TrimSpace(string(data)), "gitdir:")
+	if !ok {
+		return false, fmt.Errorf("%s: not a worktree gitdir pointer", dot)
+	}
+	gitDir := strings.TrimSpace(rest)
+	if gitDir == "" {
+		return false, fmt.Errorf("%s: empty gitdir", dot)
+	}
+	if !filepath.IsAbs(gitDir) {
+		gitDir = filepath.Join(path, gitDir)
+	}
+	// The git dir is "<repo>/.git/worktrees/<name>"; both it and the shared .git
+	// it sits in must be absent before the repo counts as gone.
+	for _, dir := range []string{gitDir, filepath.Dir(filepath.Dir(gitDir))} {
+		if _, err := os.Stat(dir); err == nil {
+			return false, nil
+		} else if !os.IsNotExist(err) {
+			return false, err
+		}
+	}
+	return true, nil
+}
+
 // repoFromDirName recovers the repo name from the "<repo>__<branch>" layout used
 // under the worktree root, for entries git can no longer answer for.
 func repoFromDirName(name string) string {
@@ -509,6 +568,10 @@ func resolveWorktree(target string) (managedWt, error) {
 // proved the commits survive elsewhere may set it.
 func removeWorktree(out io.Writer, wt managedWt, deleteBranch, forceBranch bool) error {
 	switch {
+	case wt.inspectErr != nil:
+		// No srcDir to act through and no idea what is in there; --include-keep
+		// must not turn that into a delete.
+		return fmt.Errorf("refusing to remove %s: git state unreadable: %w", wt.path, wt.inspectErr)
 	case wt.orphan:
 		return removeOrphanDir(out, wt)
 	case wt.missing:

cmd/agentws/prune.go

@@ -88,7 +88,7 @@ func newPruneCmd() *cobra.Command {
 	f.BoolVar(&opts.keepBranches, "keep-branches", false, "Never delete a local branch, whatever the classification")
 	f.BoolVar(&opts.noFetch, "no-fetch", false, "Do not fetch; judge against the refs already on disk")
 	f.BoolVar(&opts.jsonOut, "json", false, "Emit JSON instead of a table")
-	f.BoolVar(&opts.includeKeep, "include-keep", false, "Dangerous: also remove worktrees classified keep (needs --yes; never deletes their branch)")
+	f.BoolVar(&opts.includeKeep, "include-keep", false, "Dangerous: also remove worktrees classified keep (needs --yes). Destroys uncommitted changes and paused rebase/merge state, which no branch is carrying; only the branch itself survives")
 	f.BoolVar(&opts.includeUnmanaged, "include-unmanaged", false, "Also remove worktrees that live outside the worktree root")
 	return cmd
 }
@@ -142,11 +142,15 @@ func classifyAll(notes io.Writer, prs prLister, worktrees []managedWt, opts prun
 	byRepo := map[string][]managedWt{}
 	var results []pruneResult
 	for _, w := range worktrees {
-		if w.orphan {
+		switch {
+		case w.inspectErr != nil:
+			// Unknown is not gone: a checkout git refused to answer for keeps.
+			results = append(results, pruneResult{wt: w, verdict: verdictKeep, reason: "inspection failed: " + oneLine(w.inspectErr.Error())})
+		case w.orphan:
 			results = append(results, pruneResult{wt: w, verdict: verdictRemove, reason: "backing repo gone, no git state to read"})
-			continue
+		default:
+			byRepo[w.srcDir] = append(byRepo[w.srcDir], w)
 		}
-		byRepo[w.srcDir] = append(byRepo[w.srcDir], w)
 	}
 
 	srcDirs := make([]string, 0, len(byRepo))
@@ -159,14 +163,14 @@ func classifyAll(notes io.Writer, prs prLister, worktrees []managedWt, opts prun
 		ctx, err := newRepoCtx(notes, prs, srcDir, opts.noFetch)
 		if err != nil {
 			for _, w := range byRepo[srcDir] {
-				results = append(results, pruneResult{wt: w, verdict: verdictKeep, reason: "repo state unknown: " + err.Error()})
+				results = append(results, pruneResult{wt: w, verdict: verdictKeep, reason: "repo state unknown: " + oneLine(err.Error())})
 			}
 			continue
 		}
 		for _, w := range byRepo[srcDir] {
 			res, err := classify(w, ctx)
 			if err != nil {
-				res = pruneResult{wt: w, verdict: verdictKeep, reason: "inspection failed: " + err.Error()}
+				res = pruneResult{wt: w, verdict: verdictKeep, reason: "inspection failed: " + oneLine(err.Error())}
 			}
 			results = append(results, res)
 		}
@@ -210,6 +214,11 @@ func report(out io.Writer, results []pruneResult, opts pruneOpts) error {
 	return tw.Flush()
 }
 
+// oneLine flattens a git error onto a single line so one row stays one row.
+func oneLine(s string) string {
+	return strings.Join(strings.Fields(s), " ")
+}
+
 func dash(s string) string {
 	if s == "" {
 		return "-"

cmd/agentws/prune_test.go

@@ -795,6 +795,98 @@ func TestPruneOrphanedDirectoryWhenRepoIsGone(t *testing.T) {
 	}
 }
 
+// addForeignWorktree clones a second repo under the source root and gives it a
+// worktree under the worktree root, so it can be broken without touching the
+// fixture's own repo. It returns the clone and the worktree path.
+func (f *fixture) addForeignWorktree(t *testing.T, repo, branch string) (string, string) {
+	t.Helper()
+	src := filepath.Join(f.root, "src", repo)
+	git(t, filepath.Join(f.root, "src"), "clone", f.bare, src)
+	identity(t, src)
+	wt := filepath.Join(f.wtRoot, agent.WorktreeDirName(repo, branch))
+	git(t, src, "worktree", "add", wt, "-b", branch, "origin/main")
+	return src, wt
+}
+
+// The regression this guards: git failing for a reason that is not "the backing
+// repo is gone" used to be read as orphan, and orphan deletes the directory
+// outright. Here the git dir is still on disk — only its commondir pointer is
+// broken, as a half-written or momentarily unreachable repo would be — so the
+// verdict must be keep and the directory must survive a --yes run.
+func TestPruneKeepsWorktreeWhenGitFailsButRepoExists(t *testing.T) {
+	f := newFixture(t)
+	src, wt := f.addForeignWorktree(t, "other", "benvin/unreadable")
+	writeCommit(t, wt, "u.txt", "u\n", "work")
+	gitDir, err := agent.GitDir(wt)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Remove(filepath.Join(gitDir, "commondir")); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := agent.GitCurrentBranch(wt); err == nil {
+		t.Fatal("fixture did not break git in the worktree")
+	}
+	if !exists(gitDir) || !exists(filepath.Join(src, ".git")) {
+		t.Fatal("fixture removed the backing repo; it must still be present")
+	}
+
+	srv := fakeGitea(t)
+	out := runOpts(t, client(srv), pruneOpts{apply: true})
+	assertVerdict(t, out, "-", verdictKeep, "inspection failed")
+	if !exists(wt) {
+		t.Error("a worktree git could not be read must not be deleted")
+	}
+}
+
+// The same rule one step further in: the registration is gone but the repo is
+// not, so the directory is still recoverable and must not be deleted.
+func TestPruneKeepsWorktreeWhenOnlyRegistrationIsGone(t *testing.T) {
+	f := newFixture(t)
+	src, wt := f.addForeignWorktree(t, "other", "benvin/deregistered")
+	gitDir, err := agent.GitDir(wt)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.RemoveAll(gitDir); err != nil {
+		t.Fatal(err)
+	}
+	if !exists(filepath.Join(src, ".git")) {
+		t.Fatal("fixture removed the backing repo; it must still be present")
+	}
+
+	srv := fakeGitea(t)
+	out := runOpts(t, client(srv), pruneOpts{apply: true})
+	assertVerdict(t, out, "-", verdictKeep, "inspection failed")
+	if !exists(wt) {
+		t.Error("a worktree whose repo still exists must not be deleted")
+	}
+}
+
+// --include-keep overrides a keep, but it must not become a second route to the
+// deletion finding #1 closed: with no readable git state there is nothing to
+// remove through, so the removal fails loudly and the directory stays.
+func TestPruneIncludeKeepDoesNotDeleteUnreadableWorktree(t *testing.T) {
+	f := newFixture(t)
+	_, wt := f.addForeignWorktree(t, "other", "benvin/forced")
+	gitDir, err := agent.GitDir(wt)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Remove(filepath.Join(gitDir, "commondir")); err != nil {
+		t.Fatal(err)
+	}
+
+	var out, errOut bytes.Buffer
+	err = runPrune(&out, &errOut, client(fakeGitea(t)), pruneOpts{apply: true, includeKeep: true})
+	if err == nil {
+		t.Error("forcing removal of an unreadable worktree should fail, not succeed silently")
+	}
+	if !exists(wt) {
+		t.Error("--include-keep must not delete a worktree whose git state is unknown")
+	}
+}
+
 // --- safety signals -------------------------------------------------------
 
 // An interrupted rebase holds sequencer state that exists nowhere else, and it