diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index c05d4aed4cfbf..79d79b81d51eb 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -482,6 +482,59 @@ const runCleanupTest = ( >, ) => 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,11 +1485,13 @@ 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) => @@ -1364,7 +1499,8 @@ describe("storage cleanup reports and local file policies", () => { ? { ...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); }), ), ); diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index 526aaffa2abb3..cd98f30d27f25 100644 --- a/apps/server/src/storageCleanup.ts +++ b/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,9 +326,10 @@ 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, { @@ -311,20 +337,38 @@ const make = Effect.gen(function* () { 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))