fix(box): worktrees with submodules can be removed when nothing would be lost

app/src/lib/kept-reason.test.ts

@@ -0,0 +1,29 @@
+// node --experimental-strip-types --test src/lib/kept-reason.test.ts (pnpm test)
+import assert from "node:assert/strict";
+import { test } from "node:test";
+
+import { keptReason } from "./kept-reason.ts";
+
+const box = (why: string) => `git worktree remove: '/home/acme/code/shop-fix' ${why}, use --force to delete it`;
+
+test("work git or the box would lose keeps the worktree, and says what it is", () => {
+  assert.match(keptReason(box("contains modified or untracked files"))!, /uncommitted changes/);
+  assert.match(keptReason(box("contains modified or untracked files in a submodule"))!, /uncommitted changes/);
+  assert.match(keptReason(box("contains submodule commits or stashes that no remote has"))!, /commits or stashes no remote has/);
+});
+
+test("a check that couldn't run still offers Remove anyway, without calling it uncommitted changes", () => {
+  for (const why of ["could not be checked for changes", "has submodules that could not be checked for unpushed work"]) {
+    const said = keptReason(box(why));
+    assert.ok(said, why);
+    assert.doesNotMatch(said!, /uncommitted/);
+  }
+});
+
+test("other failures aren't a reason to force", () => {
+  assert.equal(keptReason("git worktree remove: fatal: 'shop-fix' is not a working tree"), null);
+  assert.equal(keptReason("not connected"), null);
+  // Words that only sound like unsaved work.
+  assert.equal(keptReason("git worktree remove: fatal: '/home/acme/code/shop-fix' contains a .git file that is not a gitlink"), null);
+  assert.equal(keptReason("git worktree remove: error: unable to unlink old 'src/modified.ts': Permission denied"), null);
+});

app/src/lib/kept-reason.ts

@@ -0,0 +1,10 @@
+// keptReason reads a box's refusal to remove a worktree unforced: why it
+// was kept, in words for the "Kept" toast that offers Remove anyway, or
+// null when the error is something else that forcing wouldn't fix.
+export function keptReason(msg: string): string | null {
+  if (/submodule commits/i.test(msg)) return "A submodule in it has commits or stashes no remote has, so it was left as it is.";
+  if (/could not be checked/i.test(msg)) return "Shipyard couldn't check it for work that removing it would lose, so it was left as it is.";
+  // git's own words for work it would lose, which the box's refusals use too.
+  if (/modified or untracked files/i.test(msg)) return "It has uncommitted changes, so git left it as it is.";
+  return null;
+}

app/src/lib/remove-worktree.ts

@@ -2,6 +2,7 @@ import { toastError } from "@/components/error-note";
 import { toastManager } from "@/components/ui/toast";
 import type { Client } from "@/lib/api";
 import { errorMessage } from "@/lib/format";
+import { keptReason } from "@/lib/kept-reason";
 import { unpinWorktree } from "@/lib/pinned-worktrees";
 import { clearRemoving, markRemoving, markScript, pruneRemovals, type RemovalKind, removalOf, removeResult, useRemovals } from "@/lib/removing";
 import { scheduleRefresh, useStore } from "@/lib/store";
@@ -71,12 +72,12 @@ async function drain(box: string) {
       await removeWorktreeOnBox(client, box, location, wt, "remove", { force });
     } catch (err) {
       clearRemoving(box, wt.path);
-      // git's own words for work it would lose.
-      if (!force && /modified|untracked|uncommitted|contains/i.test(errorMessage(err))) {
+      const why = force ? null : keptReason(errorMessage(err));
+      if (why) {
         toastManager.add({
           type: "warning",
           title: `Kept ${wt.name}`,
-          description: "It has uncommitted changes, so git left it as it is.",
+          description: why,
           actionProps: { children: "Remove anyway", onClick: () => queueRemoval(box, location, wt, { force: true }) },
         });
       } else toastError(err, { title: `Couldn't remove ${wt.name}`, box });

docs/changelog.mdx

@@ -11,6 +11,14 @@ again.
 
 ## Unreleased
 
+- **Worktrees with submodules can be removed.** git refuses to remove any
+  worktree with a submodule checked out, even a clean one, so removing or
+  archiving one failed. Shipyard now checks the worktree and its submodules
+  itself and removes it when nothing would be lost. If a submodule has
+  changes, stashes, or commits that aren't pushed anywhere (a commit only a
+  local tag points at counts), the worktree is kept and you can still choose
+  **Remove anyway**.
+
 - **Pin worktrees to the top of the sidebar.** Right-click a worktree, or
   open its ⋯ menu, and choose **Pin to top**. Pinned worktrees sit in their
   own list above your projects, in the order you pinned them, whether or not

internal/box/locations.go

@@ -417,7 +417,17 @@ func (l *Locations) RemoveWorktree(ctx context.Context, location, name string, f
 		if force {
 			args = append(args, "--force")
 		}
-		if out, err := git(ctx, args...); err != nil {
+		out, err := git(ctx, args...)
+		// git refuses any worktree with a submodule checked out, however
+		// clean, and only --force gets past it. Look for the work git won't,
+		// and force only when there is none to lose.
+		if err != nil && !force && strings.Contains(string(out), "containing submodules") {
+			if why := unsavedWork(ctx, w.Path); why != "" {
+				return httpError{status: http.StatusConflict, msg: fmt.Sprintf("git worktree remove: '%s' %s, use --force to delete it", w.Path, why)}
+			}
+			out, err = git(ctx, append(args, "--force")...)
+		}
+		if err != nil {
 			return fmt.Errorf("git worktree remove: %s", strings.TrimSpace(string(out)))
 		}
 		// A worktree made again under the same name starts without a title.
@@ -549,6 +559,106 @@ func dirtyWorktree(dir string) bool {
 	return err == nil && strings.TrimSpace(string(out)) != ""
 }
 
+// unsavedWork says what removing a worktree with submodules would lose, or
+// "" for nothing: changes in it or its submodules, or submodule commits or
+// stashes no remote has (any local ref's: a branch's, a tag's, the stash),
+// since a worktree keeps its submodules' repositories inside its own git
+// folder. A ref's name on history a remote has is not kept: whether a
+// remote has the tag itself would take asking it, on every removal.
+// Untracked files count whatever status.showUntrackedFiles says, and when
+// git can't tell, that counts as something to lose.
+func unsavedWork(ctx context.Context, dir string) string {
+	out, err := git(ctx, "-C", dir, "status", "--porcelain", "--untracked-files=normal", "--ignore-submodules=none")
+	if err != nil {
+		return "could not be checked for changes"
+	}
+	if strings.TrimSpace(string(out)) != "" {
+		return "contains modified or untracked files"
+	}
+	// Each checked-out submodule is asked itself too, so its own config
+	// (status.showUntrackedFiles, submodule.<name>.ignore for the ones
+	// inside it) can't hide a change from the status above.
+	out, err = git(ctx, "-C", dir, "submodule", "foreach", "--quiet", "--recursive",
+		"git status --porcelain --untracked-files=normal --ignore-submodules=none")
+	if err != nil {
+		return "has submodules that could not be checked for changes"
+	}
+	if strings.TrimSpace(string(out)) != "" {
+		return "contains modified or untracked files in a submodule"
+	}
+	// Every submodule repository the removal deletes, against every local
+	// ref it has: those in the worktree's git folder, checked out or not (a
+	// deinit leaves one behind), and those a checked-out submodule keeps in
+	// its own .git (one made with git clone), with the ones under that .git
+	// in turn.
+	gitDir, err := git(ctx, "-C", dir, "rev-parse", "--absolute-git-dir")
+	if err != nil {
+		return "has submodules that could not be checked for unpushed work"
+	}
+	repos, err := moduleRepos(filepath.Join(strings.TrimSpace(string(gitDir)), "modules"))
+	if err != nil {
+		return "has submodules that could not be checked for unpushed work"
+	}
+	out, err = git(ctx, "-C", dir, "submodule", "foreach", "--quiet", "--recursive", "git rev-parse --absolute-git-dir")
+	if err != nil {
+		return "has submodules that could not be checked for unpushed work"
+	}
+	for _, repo := range strings.Split(strings.TrimSpace(string(out)), "\n") {
+		if repo == "" {
+			continue
+		}
+		nested, err := moduleRepos(filepath.Join(repo, "modules"))
+		if err != nil {
+			return "has submodules that could not be checked for unpushed work"
+		}
+		for _, r := range append([]string{repo}, nested...) {
+			if !slices.Contains(repos, r) {
+				repos = append(repos, r)
+			}
+		}
+	}
+	for _, repo := range repos {
+		out, err := git(ctx, "--git-dir="+repo, "rev-list", "-1", "--all", "--not", "--remotes")
+		if err != nil {
+			return "has submodules that could not be checked for unpushed work"
+		}
+		if strings.TrimSpace(string(out)) != "" {
+			return "contains submodule commits or stashes that no remote has"
+		}
+	}
+	return ""
+}
+
+// moduleRepos lists the submodule repositories git keeps under a modules
+// folder, nested submodules' included. A name may hold slashes, so any
+// folder that isn't a repository is looked into.
+func moduleRepos(dir string) ([]string, error) {
+	entries, err := os.ReadDir(dir)
+	if os.IsNotExist(err) {
+		return nil, nil
+	}
+	if err != nil {
+		return nil, err
+	}
+	var repos []string
+	for _, e := range entries {
+		if !e.IsDir() {
+			continue
+		}
+		p := filepath.Join(dir, e.Name())
+		if _, err := os.Stat(filepath.Join(p, "HEAD")); err == nil {
+			repos = append(repos, p)
+			p = filepath.Join(p, "modules")
+		}
+		nested, err := moduleRepos(p)
+		if err != nil {
+			return nil, err
+		}
+		repos = append(repos, nested...)
+	}
+	return repos, nil
+}
+
 func expandHome(path string) string {
 	if path == "~" || strings.HasPrefix(path, "~/") {
 		if home, err := os.UserHomeDir(); err == nil {