fix(server): keep worktrees containing nested repositories

apps/server/src/storageCleanup.test.ts

@@ -482,6 +482,59 @@ const runCleanupTest = <A, E>(
   >,
 ) => effect.pipe(Effect.provide(cleanupTestLayer), Effect.scoped);
 
+it.live.each([
+  { policy: "tracked-changes", kind: "worktree" },
+  { policy: "tracked-changes", kind: "repository" },
+  { policy: "tracked-changes", kind: "uppercase" },
+  { policy: "tracked-changes", kind: "bare" },
+  { policy: "uncommitted-changes", kind: "worktree" },
+  { policy: "uncommitted-changes", kind: "repository" },
+  { policy: "uncommitted-changes", kind: "uppercase" },
+  { policy: "uncommitted-changes", kind: "bare" },
+] as const)("keeps an ignored nested $policy / $kind", ({ policy, kind }) =>
+  runCleanupTest(
+    Effect.gen(function* () {
+      const { service, fs, repo, worktree, command, setPolicy } = yield* cleanupFixture();
+      setPolicy(policy);
+      const nested = `${worktree}/dist/nested`;
+      if (kind === "worktree" || kind === "uppercase") {
+        yield* command(repo, ["worktree", "add", "-b", "nested", nested]);
+      } else {
+        yield* command(repo, [
+          "clone",
+          "--no-hardlinks",
+          ...(kind === "bare" ? ["--bare"] : []),
+          repo,
+          nested,
+        ]);
+      }
+      if (kind === "uppercase") yield* fs.rename(`${nested}/.git`, `${nested}/.GIT`);
+      yield* fs.writeFileString(`${nested}/uncommitted.txt`, "irreplaceable work\n");
+      expect((yield* command(worktree, ["status", "--porcelain"])).stdout).toBe("");
+      const report = yield* service.runNow;
+      expect(report.entries.find((entry) => entry.path === worktree)).toMatchObject({
+        outcome: "kept",
+        reason: "Contains a nested Git repository",
+      });
+      expect(yield* fs.readFileString(`${nested}/uncommitted.txt`)).toBe("irreplaceable work\n");
+      expect(
+        (yield* command(nested, [
+          ...(kind === "uppercase"
+            ? [
+                `--git-dir=${(yield* fs.readFileString(`${nested}/.GIT`)).trim().slice(8)}`,
+                "--work-tree=.",
+              ]
+            : []),
+          "-c",
+          "safe.bareRepository=all",
+          "rev-parse",
+          "--is-bare-repository",
+        ])).stdout.trim(),
+      ).toBe(String(kind === "bare"));
+    }),
+  ),
+);
+
 const submoduleCleanupFixture = () =>
   Effect.gen(function* () {
     const fixture = yield* cleanupFixture();
@@ -506,6 +559,55 @@ const submoduleCleanupFixture = () =>
     return { ...fixture, submodule, child, recordSubmodules };
   });
 
+it.live.each(["repository", "worktree"] as const)(
+  "inspects a %s in a pnpm store excluded from reclaimed bytes",
+  (kind) =>
+    runCleanupTest(
+      Effect.gen(function* () {
+        const { service, fs, repo, worktree, command, setPolicy } = yield* cleanupFixture();
+        setPolicy("tracked-changes");
+        const storeDir = yield* fs.makeTempDirectoryScoped({ prefix: "cleanup-store-" });
+        const nested = `${worktree}/node_modules/.pnpm/nested`;
+        yield* command(
+          repo,
+          kind === "worktree"
+            ? ["worktree", "add", "-b", "nested", nested]
+            : ["clone", "--no-hardlinks", repo, nested],
+        );
+        yield* fs.writeFileString(`${nested}/uncommitted.txt`, "keep me");
+        yield* fs.writeFileString(
+          `${worktree}/node_modules/.modules.yaml`,
+          JSON.stringify({ storeDir, virtualStoreDir: ".pnpm" }),
+        );
+        const report = yield* service.runNow;
+        expect(report.entries.find((entry) => entry.path === worktree)).toMatchObject({
+          outcome: "kept",
+          reason: "Contains a nested Git repository",
+        });
+        expect(yield* fs.readFileString(`${nested}/uncommitted.txt`)).toBe("keep me");
+      }),
+    ),
+);
+
+it.live("keeps an ignored repository inside a registered submodule", () =>
+  runCleanupTest(
+    Effect.gen(function* () {
+      const { service, fs, repo, worktree, submodule, command, setPolicy } =
+        yield* submoduleCleanupFixture();
+      setPolicy("tracked-changes");
+      const nested = `${submodule}/dist/nested`;
+      yield* command(repo, ["clone", "--no-hardlinks", repo, nested]);
+      yield* fs.writeFileString(`${nested}/uncommitted.txt`, "keep me");
+      const report = yield* service.runNow;
+      expect(report.entries.find((entry) => entry.path === worktree)).toMatchObject({
+        outcome: "kept",
+        reason: "Contains a nested Git repository",
+      });
+      expect(yield* fs.readFileString(`${nested}/uncommitted.txt`)).toBe("keep me");
+    }),
+  ),
+);
+
 it.live("removes clean submodules recursively, including deinitialized repositories", () =>
   runCleanupTest(
     Effect.gen(function* () {
@@ -519,6 +621,37 @@ it.live("removes clean submodules recursively, including deinitialized repositor
         repo,
         "stored",
       ]);
+      const legacy = `${worktree}/legacy`;
+      yield* command(repo, ["clone", "--no-hardlinks", repo, legacy]);
+      yield* command(worktree, ["submodule", "add", repo, "legacy"]);
+      expect((yield* fs.stat(`${legacy}/.git`)).type).toBe("Directory");
+      yield* command(legacy, [
+        "-c",
+        "protocol.file.allow=always",
+        "submodule",
+        "add",
+        repo,
+        "child",
+      ]);
+      yield* command(legacy, [
+        "-c",
+        "protocol.file.allow=always",
+        "submodule",
+        "add",
+        repo,
+        "stored",
+      ]);
+      yield* command(legacy, [
+        "-c",
+        "user.name=Cleanup test",
+        "-c",
+        "user.email=cleanup@example.test",
+        "commit",
+        "-am",
+        "record legacy children",
+      ]);
+      yield* command(legacy, ["push", "origin", "HEAD:legacy"]);
+      yield* command(legacy, ["submodule", "deinit", "stored"]);
       yield* recordSubmodules();
       yield* command(submodule, ["submodule", "deinit", "stored"]);
       const admin = (yield* command(worktree, ["rev-parse", "--absolute-git-dir"])).stdout.trim();
@@ -1352,19 +1485,22 @@ describe("storage cleanup reports and local file policies", () => {
         Effect.gen(function* () {
           const fs = yield* FileSystem.FileSystem;
           const storeDir = yield* fs.makeTempDirectoryScoped({ prefix: "pnpm-store-" });
+          const statted: string[] = [];
           const { service, worktree, setPolicy } = yield* cleanupFixture().pipe(
             Effect.provideService(FileSystem.FileSystem, {
               ...fs,
-              stat: (target) =>
-                fs
+              stat: (target) => {
+                statted.push(target);
+                return fs
                   .stat(target)
                   .pipe(
                     Effect.map((info) =>
                       store === "other-device" && target === storeDir
                         ? { ...info, dev: info.dev + 1n }
                         : info,
                     ),
-                  ),
+                  );
+              },
             }),
           );
           setPolicy("tracked-changes");
@@ -1395,6 +1531,9 @@ describe("storage cleanup reports and local file policies", () => {
           );
           const report = yield* service.runNow;
           expect(report.entries[0]).toMatchObject({ outcome: "removed", bytes });
+          expect(statted.includes(`${nodeModules}/.pnpm/pkg@1.0.0/index.js`)).toBe(
+            store !== "same-device",
+          );
         }),
       ),
   );
@@ -1428,7 +1567,7 @@ describe("storage cleanup reports and local file policies", () => {
       }),
     ),
   );
-  it.live("still removes a worktree when measuring its size fails", () =>
+  it.live("keeps a worktree when the inspection cannot read its directories", () =>
     runCleanupTest(
       Effect.gen(function* () {
         const fs = yield* FileSystem.FileSystem;
@@ -1439,9 +1578,14 @@ describe("storage cleanup reports and local file policies", () => {
           }),
         );
         const report = yield* service.runNow;
-        expect(report.entries[0]).toMatchObject({ outcome: "removed", bytes: null, files: null });
+        expect(report.entries[0]).toMatchObject({
+          outcome: "kept",
+          reason: "Worktree could not be fully inspected",
+          bytes: null,
+          files: null,
+        });
         expect(report.bytesFreed).toBe(0);
-        expect(yield* fs.exists(worktree)).toBe(false);
+        expect(yield* fs.exists(worktree)).toBe(true);
       }),
     ),
   );

apps/server/src/storageCleanup.ts

@@ -1,3 +1,5 @@
+// @effect-diagnostics nodeBuiltinImport:off
+import * as NodeFSP from "node:fs/promises";
 import {
   GitCommandError,
   OrchestrationV2AppThreadJson,
@@ -279,15 +281,38 @@ const make = Effect.gen(function* () {
   );
 
   const measureWorktree = Effect.fn("storageCleanup.measureWorktree")(
-    function* (worktreePath: string) {
+    function* (worktreePath: string, inspectRepositories = false) {
       const root = yield* fs.realPath(worktreePath);
       const skipped = yield* sharedPnpmVirtualStore(root);
+      const incomplete = { bytes: null, reason: "worktree could not be fully inspected" };
+      const repositories = new Set([pathKey(root)]);
+      const moduleStores: string[] = [];
+      if (inspectRepositories && (yield* fs.exists(path.join(root, ".gitmodules")))) {
+        const submodules = yield* git.execute({
+          operation: "StorageCleanup.nestedRepositories",
+          cwd: root,
+          args: [
+            "submodule",
+            "foreach",
+            "--recursive",
+            "--quiet",
+            'printf "%s\\0" "$toplevel/$sm_path"',
+          ],
+          maxOutputBytes: 64 * 1024,
+        });
+        if (submodules.stdoutTruncated || submodules.stderrTruncated) return incomplete;
+        for (const submodule of submodules.stdout.split("\0").filter(Boolean)) {
+          repositories.add(pathKey(path.resolve(submodule)));
+          repositories.add(pathKey(path.resolve(submodule, ".git")));
+          moduleStores.push(pathKey(path.resolve(submodule, ".git", "modules")));
+        }
+      }
       const pending = [root];
       let entries = 1;
       let bytes = 0;
       while (pending.length > 0) {
         const target = pending.pop()!;
-        if (target === skipped) continue;
+        if (target === skipped && !inspectRepositories) continue;
         // Effect's stat follows links. Probe with readLink first to get lstat
         // semantics, including skipping dangling links and directory links.
         const isLink = yield* fs.readLink(target).pipe(
@@ -301,30 +326,49 @@ const make = Effect.gen(function* () {
           ),
         );
         if (isLink) continue;
-        if ((yield* fs.realPath(target)) !== target) return null;
+        if ((yield* fs.realPath(target)) !== target) return incomplete;
         const stat = yield* fs.stat(target);
         if (stat.type === "File") {
+          if (skipped !== null && inside(skipped, target)) continue;
           // Removing a shared hardlink leaves its blocks allocated.
           if (Option.getOrElse(stat.nlink, () => 1n) !== 1n) continue;
           bytes += Option.match(stat.blocks, {
             onNone: () => Number(stat.size),
             onSome: (blocks) => Number(blocks * 512n),
           });
         } else if (stat.type === "Directory") {
-          const names = yield* fs.readDirectory(target);
+          const dirents =
+            skipped !== null && (target === skipped || inside(skipped, target))
+              ? yield* Effect.tryPromise(() => NodeFSP.readdir(target, { withFileTypes: true }))
+              : null;
+          const names = dirents?.map((entry) => entry.name) ?? (yield* fs.readDirectory(target));
+          const markers = inspectRepositories ? names.map((name) => name.toLowerCase()) : [];
+          // Git status omits ignored repositories, and worktree remove deletes them.
+          // Registered submodules have their own recursive safety checks below.
+          if (
+            inspectRepositories &&
+            !repositories.has(pathKey(target)) &&
+            !moduleStores.some((store) => pathKey(target) === store || inside(store, target)) &&
+            (markers.includes(".git") ||
+              (markers.includes("head") && markers.includes("objects") && markers.includes("refs")))
+          )
+            return { bytes: null, reason: "contains a nested Git repository" };
           entries += names.length;
-          if (entries > 2_000_000) return null;
-          for (const name of names) {
+          if (entries > 2_000_000) return incomplete;
+          const directories = dirents
+            ?.filter((entry) => entry.isDirectory())
+            .map((entry) => entry.name);
+          for (const name of directories ?? names) {
             const child = path.join(target, name);
-            if (!inside(root, child)) return null;
+            if (!inside(root, child)) return incomplete;
             pending.push(child);
           }
         }
       }
-      return bytes;
+      return { bytes, reason: null };
     },
     Effect.timeout("30 seconds"),
-    Effect.orElseSucceed(() => null),
+    Effect.orElseSucceed(() => ({ bytes: null, reason: "worktree could not be fully inspected" })),
   );
 
   // Large snapshots take seconds; use the reader worker instead of blocking the event loop.
@@ -433,7 +477,7 @@ const make = Effect.gen(function* () {
             firstMeasurement ? Effect.asSome : Effect.timeoutOption(remaining),
           );
           firstMeasurement = false;
-          if (Option.isSome(measured)) orphanSizes.set(key, measured.value);
+          if (Option.isSome(measured)) orphanSizes.set(key, measured.value.bytes);
         }
         const bytes = orphanSizes.get(key) ?? null;
         entries.push({
@@ -982,7 +1026,9 @@ const make = Effect.gen(function* () {
         });
         const blocked = yield* blockedSinceCheck();
         if (blocked !== null) return keep(blocked);
-        const bytes = yield* measureWorktree(worktreePath);
+        const inspection = yield* measureWorktree(worktreePath, true);
+        if (inspection.reason !== null) return keep(inspection.reason);
+        const bytes = inspection.bytes;
         const blockedWhileMeasuring = yield* blockedSinceCheck();
         if (blockedWhileMeasuring !== null) return keep(blockedWhileMeasuring);
         if (yield* branchChanged(worktreePath, thread.branch))