diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index 1f5645c87f78..ba98b3cbb2a3 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -503,6 +503,188 @@ const runCleanupTest = ( >, ) => effect.pipe(Effect.provide(cleanupTestLayer), Effect.scoped); +const submoduleCleanupFixture = () => + Effect.gen(function* () { + const fixture = yield* cleanupFixture(); + const { command, repo, worktree } = fixture; + const submodule = `${worktree}/module`; + const child = `${submodule}/child`; + for (const [cwd, name] of [ + [worktree, "module"], + [submodule, "child"], + ] as const) { + yield* command(cwd, ["-c", "protocol.file.allow=always", "submodule", "add", repo, name]); + yield* command(`${cwd}/${name}`, ["config", "user.name", "Cleanup test"]); + yield* command(`${cwd}/${name}`, ["config", "user.email", "cleanup@example.test"]); + } + const recordSubmodules = () => + Effect.gen(function* () { + yield* command(submodule, ["commit", "-am", "record child"]); + yield* command(submodule, ["push", "origin", "HEAD:submodule"]); + yield* command(worktree, ["commit", "-am", "record module"]); + }); + yield* recordSubmodules(); + return { ...fixture, submodule, child, recordSubmodules }; + }); + +it.live("removes clean submodules recursively, including deinitialized repositories", () => + runCleanupTest( + Effect.gen(function* () { + const { service, fs, repo, worktree, submodule, command, recordSubmodules } = + yield* submoduleCleanupFixture(); + yield* command(submodule, [ + "-c", + "protocol.file.allow=always", + "submodule", + "add", + repo, + "stored", + ]); + yield* recordSubmodules(); + yield* command(submodule, ["submodule", "deinit", "stored"]); + const admin = (yield* command(worktree, ["rev-parse", "--absolute-git-dir"])).stdout.trim(); + const report = yield* service.runNow; + expect(report.entries.find((entry) => entry.path === worktree)).toMatchObject({ + outcome: "removed", + }); + expect(report.counts).toEqual({ removed: 1, kept: 0, failed: 0 }); + expect(yield* fs.exists(worktree)).toBe(false); + expect(yield* fs.exists(admin)).toBe(false); + }), + ), +); + +it.live("keeps nested submodule files protected by the current keep rule", () => + runCleanupTest( + Effect.gen(function* () { + const { service, fs, worktree, submodule, child, command, setPolicy } = + yield* submoduleCleanupFixture(); + setPolicy("tracked-changes"); + yield* fs.writeFileString(`${child}/tracked.txt`, "local edit\n"); + expect( + (yield* service.runNow).entries.find((entry) => entry.path === worktree), + ).toMatchObject({ outcome: "kept" }); + expect(yield* fs.readFileString(`${child}/tracked.txt`)).toBe("local edit\n"); + yield* fs.writeFileString(`${child}/tracked.txt`, "original\n"); + setPolicy("any-local-files"); + yield* fs.writeFileString(`${child}/.env`, "local secret\n"); + expect( + (yield* service.runNow).entries.find((entry) => entry.path === worktree), + ).toMatchObject({ outcome: "kept" }); + expect(yield* fs.readFileString(`${child}/.env`)).toBe("local secret\n"); + + yield* fs.remove(`${child}/.env`); + yield* command(submodule, ["submodule", "deinit", "child"]); + yield* fs.writeFileString(`${child}/.env`, "secret after deinit\n"); + const report = yield* service.runNow; + expect({ + outcome: report.entries.find((entry) => entry.path === worktree)?.outcome, + contents: (yield* fs.exists(`${child}/.env`)) + ? yield* fs.readFileString(`${child}/.env`) + : null, + }).toEqual({ outcome: "kept", contents: "secret after deinit\n" }); + + yield* fs.remove(`${child}/.env`); + setPolicy("uncommitted-changes"); + yield* fs.writeFileString(`${child}/notes.txt`, "notes after deinit\n"); + const untrackedReport = yield* service.runNow; + expect({ + outcome: untrackedReport.entries.find((entry) => entry.path === worktree)?.outcome, + contents: (yield* fs.exists(`${child}/notes.txt`)) + ? yield* fs.readFileString(`${child}/notes.txt`) + : null, + }).toEqual({ outcome: "kept", contents: "notes after deinit\n" }); + }), + ), +); + +it.live("keeps unpushed submodule history after checkout and deinitialization", () => + runCleanupTest( + Effect.gen(function* () { + const { service, fs, worktree, submodule, child, command, recordSubmodules } = + yield* submoduleCleanupFixture(); + yield* command(child, ["checkout", "-b", "local-only"]); + yield* fs.writeFileString(`${child}/tracked.txt`, "unpublished work\n"); + yield* command(child, ["commit", "-am", "local work"]); + const local = (yield* command(child, ["rev-parse", "HEAD"])).stdout.trim(); + yield* recordSubmodules(); + expect( + (yield* service.runNow).entries.find((entry) => entry.path === worktree), + ).toMatchObject({ outcome: "kept", reason: expect.stringContaining("remote-tracking") }); + expect(yield* fs.readFileString(`${child}/tracked.txt`)).toBe("unpublished work\n"); + + yield* command(child, ["checkout", "main"]); + yield* recordSubmodules(); + expect( + (yield* service.runNow).entries.find((entry) => entry.path === worktree), + ).toMatchObject({ outcome: "kept", reason: expect.stringContaining("remote-tracking") }); + const childGit = (yield* command(child, ["rev-parse", "--absolute-git-dir"])).stdout.trim(); + yield* command(submodule, ["submodule", "deinit", "child"]); + expect( + (yield* service.runNow).entries.find((entry) => entry.path === worktree), + ).toMatchObject({ outcome: "kept", reason: expect.stringContaining("remote-tracking") }); + expect( + (yield* command(worktree, [ + "--git-dir", + childGit, + "rev-parse", + "refs/heads/local-only", + ])).stdout.trim(), + ).toBe(local); + expect( + (yield* command(worktree, ["--git-dir", childGit, "show", "local-only:tracked.txt"])) + .stdout, + ).toBe("unpublished work\n"); + yield* command(worktree, ["--git-dir", childGit, "branch", "-D", "local-only"]); + expect( + (yield* service.runNow).entries.find((entry) => entry.path === worktree), + ).toMatchObject({ outcome: "kept", reason: expect.stringContaining("remote-tracking") }); + expect( + (yield* command(worktree, ["--git-dir", childGit, "show", `${local}:tracked.txt`])).stdout, + ).toBe("unpublished work\n"); + }), + ), +); + +it.live("keeps submodules when stored repository metadata is malformed", () => + runCleanupTest( + Effect.gen(function* () { + const { service, fs, worktree, submodule, child, command } = yield* submoduleCleanupFixture(); + const childGit = (yield* command(child, ["rev-parse", "--absolute-git-dir"])).stdout.trim(); + yield* command(submodule, ["submodule", "deinit", "child"]); + yield* fs.writeFileString(`${childGit}/HEAD`, "not a ref\n"); + expect( + (yield* service.runNow).entries.find((entry) => entry.path === worktree), + ).toMatchObject({ outcome: "kept", reason: expect.stringContaining("inspected") }); + expect(yield* fs.exists(worktree)).toBe(true); + expect(yield* fs.readFileString(`${childGit}/HEAD`)).toBe("not a ref\n"); + }), + ), +); + +it.live("preserves late tracked edits when submodules were never initialized", () => + runCleanupTest( + Effect.gen(function* () { + const fixture = yield* cleanupFixture(); + const { service, fs, worktree, command } = fixture; + const head = (yield* command(worktree, ["rev-parse", "HEAD"])).stdout.trim(); + yield* fs.writeFileString( + `${worktree}/.gitmodules`, + '[submodule "module"]\n\tpath = module\n\turl = /unused\n', + ); + yield* command(worktree, ["add", ".gitmodules"]); + yield* command(worktree, ["update-index", "--add", "--cacheinfo", `160000,${head},module`]); + yield* command(worktree, ["commit", "-m", "record uninitialized module"]); + yield* fs.makeDirectory(`${worktree}/module`); + fixture.editBeforeRemoval(); + expect( + (yield* service.runNow).entries.find((entry) => entry.path === worktree), + ).toMatchObject({ outcome: "failed" }); + expect(yield* fs.readFileString(`${worktree}/tracked.txt`)).toBe("late edit\n"); + }), + ), +); + describe("subagent worktree ownership", () => { it.effect.each([ "idle", diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index 55275e5976b0..a4d1e831d900 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -538,6 +538,122 @@ const make = Effect.gen(function* () { return file === undefined ? null : `has ignored files (${file})`; }); + const submoduleRemoval = Effect.fn("StorageCleanup.submoduleRemoval")( + function* (cwd: string, rules: WorktreeCleanupRules) { + const inspect = (args: string[], maxOutputBytes = 64 * 1024) => + git.execute({ + operation: "StorageCleanup.submodules", + cwd, + args, + maxOutputBytes, + }); + const admin = yield* inspect(["rev-parse", "--absolute-git-dir"]); + if (admin.stdoutTruncated || admin.stderrTruncated) + return { force: false, reason: "submodule inspection exceeded the limit" }; + const modules = path.join(admin.stdout.replace(/\n$/, ""), "modules"); + const stored = yield* fs.exists(modules); + if (!stored && !(yield* fs.exists(path.join(cwd, ".gitmodules")))) + return { force: false, reason: null }; + const populated = yield* inspect([ + "submodule", + "foreach", + "--recursive", + "--quiet", + 'printf "%s\\0" "$toplevel/$sm_path"', + ]); + if (populated.stdoutTruncated || populated.stderrTruncated) + return { force: false, reason: "submodule inspection exceeded the limit" }; + const paths = populated.stdout.split("\0").filter(Boolean); + const pending = [modules]; + for (const submodule of paths) { + const directory = yield* inspect(["-C", submodule, "rev-parse", "--absolute-git-dir"]); + if (directory.stdoutTruncated || directory.stderrTruncated) + return { force: false, reason: "submodule inspection exceeded the limit" }; + pending.push(directory.stdout.replace(/\n$/, "")); + } + const visited = new Set(); + while (pending.length > 0) { + const directory = pending.pop()!; + if (!(yield* fs.exists(directory))) continue; + const canonical = yield* fs.realPath(directory); + if (visited.has(canonical)) continue; + visited.add(canonical); + if (visited.size > 10_000) + return { force: false, reason: "submodule inspection exceeded the limit" }; + if ((yield* fs.stat(directory)).type !== "Directory") + return { force: false, reason: "submodule repository could not be inspected" }; + if (yield* fs.exists(path.join(directory, "HEAD"))) { + const unpushed = yield* inspect([ + "--git-dir", + directory, + "--work-tree", + cwd, + "rev-list", + "--max-count=1", + "HEAD", + "--all", + "--reflog", + "--not", + "--remotes", + ]); + if (unpushed.stdoutTruncated || unpushed.stderrTruncated) + return { force: false, reason: "submodule inspection exceeded the limit" }; + if (unpushed.stdout.trim() !== "") + return { force: false, reason: "submodule has commits not on a remote-tracking ref" }; + pending.push(path.join(directory, "modules")); + } else { + for (const name of yield* fs.readDirectory(directory)) + pending.push(path.join(directory, name)); + } + } + // Check files after walking stored history to shorten the gap before forced removal. + for (const submodule of paths) { + const changes = yield* localChangesReason(submodule, rules); + if (changes !== null) return { force: false, reason: `submodule ${changes}` }; + } + const force = stored || paths.length > 0; + if (force && rules.worktreeKeepWhen !== "tracked-changes") { + const root = yield* fs.realPath(cwd); + // Git status and foreach omit files inside deinitialized gitlink directories. + const populatedPaths = new Set(paths.map(pathKey)); + const unchecked: string[] = []; + for (const repository of [root, ...paths]) { + const index = yield* inspect( + ["-C", repository, "ls-files", "--stage", "-z"], + 16 * 1024 * 1024, + ); + if (index.stdoutTruncated || index.stderrTruncated) + return { force: false, reason: "submodule inspection exceeded the limit" }; + for (const entry of index.stdout.split("\0")) { + if (!entry.startsWith("160000 ")) continue; + const submodule = path.resolve(repository, entry.slice(entry.indexOf("\t") + 1)); + if (!inside(root, submodule)) + return { force: false, reason: "submodules could not be inspected" }; + if (!populatedPaths.has(pathKey(submodule)) && (yield* fs.exists(submodule))) + unchecked.push(submodule); + } + } + let inspected = unchecked.length; + while (unchecked.length > 0) { + if (inspected > 10_000) + return { force: false, reason: "submodule inspection exceeded the limit" }; + const target = unchecked.pop()!; + if ( + (yield* fs.realPath(target)) !== path.resolve(target) || + (yield* fs.stat(target)).type !== "Directory" + ) + return { force: false, reason: "submodule has local files" }; + if (path.basename(target) === "node_modules") continue; + const names = yield* fs.readDirectory(target); + inspected += names.length; + for (const name of names) unchecked.push(path.join(target, name)); + } + } + return { force, reason: null }; + }, + Effect.orElseSucceed(() => ({ force: false, reason: "submodules could not be inspected" })), + ); + const cleanupGit = Effect.fn("StorageCleanup.cleanupGit")(function* ( cwd: string, args: string[], @@ -859,13 +975,27 @@ const make = Effect.gen(function* () { ) ) return keep("settings changed since check"); - // Clean only untracked files; Git must still refuse removal if a tracked - // edit arrives after our last status check. + // Clean only untracked files. Ordinary worktrees retain Git's final tracked-edit check. if (settings.worktreeKeepWhen === "tracked-changes") yield* cleanupGit(worktreePath, ["clean", "-ffdx"], "git clean"); + const submodules = yield* submoduleRemoval(worktreePath, settings); + if (submodules.reason !== null) return keep(submodules.reason); + if (submodules.force) { + const changes = yield* localChangesReason(worktreePath, settings); + if (changes !== null) return keep(changes); + } yield* cleanupGit( project.workspaceRoot, - ["-c", "status.showUntrackedFiles=normal", "worktree", "remove", "--", worktreePath], + [ + "-c", + "status.showUntrackedFiles=normal", + "worktree", + "remove", + // Git requires force for submodules; recursive checks above cannot make removal atomic. + ...(submodules.force ? ["--force"] : []), + "--", + worktreePath, + ], "git worktree remove", ); entries.push({ ...entry, outcome: "removed", reason: sentence(removalReason), bytes });