From cdaeeca1fc0af4d34be4621d354a7c3420d297fd Mon Sep 17 00:00:00 2001 From: ScottN-PV Date: Fri, 16 Oct 2026 15:29:20 -0400 Subject: [PATCH 1/8] fix(server): clean up worktrees with initialized submodules --- apps/server/src/storageCleanup.test.ts | 181 +++++++++++++++++++++++++ apps/server/src/storageCleanup.ts | 97 ++++++++++++- 2 files changed, 275 insertions(+), 3 deletions(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index e3870f8af0c4..69848a6f11e7 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -480,6 +480,187 @@ const runCleanupTest = ( >, ) => effect.pipe(Effect.provide(cleanupTestLayer), Effect.scoped); +const submoduleCleanupFixture = () => + Effect.gen(function* () { + const fixture = yield* cleanupFixture(); + const { command, fs, worktree } = fixture; + const source = yield* fs.makeTempDirectoryScoped({ prefix: "cleanup-submodule-" }); + const remote = yield* fs.makeTempDirectoryScoped({ prefix: "cleanup-submodule-remote-" }); + yield* command(source, ["init", "-b", "main"]); + yield* command(source, ["config", "user.email", "cleanup@example.test"]); + yield* command(source, ["config", "user.name", "Cleanup test"]); + yield* fs.writeFileString(`${source}/tracked.txt`, "original\n"); + yield* fs.writeFileString(`${source}/.gitignore`, ".env\nnode_modules/\n"); + yield* command(source, ["add", "."]); + yield* command(source, ["commit", "-m", "initial"]); + yield* command(source, ["clone", "--bare", source, remote]); + yield* command(worktree, [ + "-c", + "protocol.file.allow=always", + "submodule", + "add", + remote, + "module", + ]); + yield* command(`${worktree}/module`, ["config", "user.name", "Cleanup test"]); + yield* command(`${worktree}/module`, ["config", "user.email", "cleanup@example.test"]); + yield* command(worktree, ["commit", "-am", "add submodule"]); + const submodule = `${worktree}/module`; + const addNested = () => + Effect.gen(function* () { + yield* command(submodule, [ + "-c", + "protocol.file.allow=always", + "submodule", + "add", + remote, + "child", + ]); + yield* command(submodule, ["commit", "-am", "add child"]); + yield* command(submodule, ["push", "origin", "HEAD:main"]); + yield* command(worktree, ["commit", "-am", "update module"]); + return `${submodule}/child`; + }); + return { ...fixture, submodule, addNested }; + }); + +it.live.each(["populated", "deinitialized"])( + "removes a clean worktree with %s nested submodules", + (state) => + runCleanupTest( + Effect.gen(function* () { + const { service, fs, worktree, submodule, command, addNested } = + yield* submoduleCleanupFixture(); + yield* addNested(); + const unpushed = yield* command(submodule, [ + "rev-list", + "--count", + "HEAD", + "--not", + "--remotes", + ]); + expect(unpushed.stdout.trim()).toBe("0"); + if (state === "deinitialized") yield* command(worktree, ["submodule", "deinit", "--all"]); + const report = yield* service.runNow; + expect(report.entries.find((entry) => entry.path === worktree)).toMatchObject({ + outcome: "removed", + }); + expect(yield* fs.exists(worktree)).toBe(false); + }), + ), +); + +it.live.each(["HEAD", "local branch"])("keeps an unpushed submodule %s", (unpushed) => + runCleanupTest( + Effect.gen(function* () { + const { service, fs, worktree, submodule, command } = yield* submoduleCleanupFixture(); + yield* command(submodule, ["checkout", "-b", "local-only"]); + yield* command(submodule, ["commit", "--allow-empty", "-m", "local only"]); + if (unpushed === "local branch") yield* command(submodule, ["checkout", "main"]); + yield* command(worktree, ["add", "module"]); + yield* command(worktree, ["commit", "--allow-empty", "-m", "record module"]); + const report = yield* service.runNow; + expect(report.entries[0]).toMatchObject({ outcome: "kept" }); + expect(report.entries[0]?.reason).toContain("not on a remote-tracking ref"); + expect(yield* fs.exists(submodule)).toBe(true); + }), + ), +); + +it.live.each([ + ["tracked.txt", "uncommitted-changes"], + [".env", "any-local-files"], +] as const)("keeps submodule %s under %s", ([file, policy]) => + runCleanupTest( + Effect.gen(function* () { + const { service, fs, submodule, setPolicy } = yield* submoduleCleanupFixture(); + setPolicy(policy); + yield* fs.writeFileString(`${submodule}/${file}`, "local edit\n"); + const report = yield* service.runNow; + expect(report.entries[0]).toMatchObject({ outcome: "kept" }); + if (file === ".env") expect(report.entries[0]?.reason).toContain("ignored files (.env)"); + expect(yield* fs.readFileString(`${submodule}/${file}`)).toBe("local edit\n"); + }), + ), +); + +it.live("keeps a deinitialized nested submodule's local branch in an embedded parent", () => + runCleanupTest( + Effect.gen(function* () { + const { service, fs, worktree, submodule, command, addNested } = + yield* submoduleCleanupFixture(); + const child = yield* addNested(); + for (const [key, value] of [ + ["user.name", "Cleanup test"], + ["user.email", "cleanup@example.test"], + ] as const) + yield* command(child, ["config", key, value]); + yield* command(child, ["checkout", "-b", "local-only"]); + yield* command(child, ["commit", "--allow-empty", "-m", "local only"]); + const local = (yield* command(child, ["rev-parse", "HEAD"])).stdout.trim(); + yield* command(child, ["checkout", "main"]); + yield* command(submodule, ["submodule", "deinit", "--all"]); + const parentGit = (yield* command(submodule, [ + "rev-parse", + "--absolute-git-dir", + ])).stdout.trim(); + yield* command(submodule, ["config", "core.worktree", submodule]); + yield* fs.remove(`${submodule}/.git`); + yield* fs.rename(parentGit, `${submodule}/.git`); + const childGit = `${submodule}/.git/modules/child`; + const report = yield* service.runNow; + expect(report.entries[0]).toMatchObject({ outcome: "kept" }); + expect(report.entries[0]?.reason).toContain("not on a remote-tracking ref"); + expect(yield* fs.exists(`${childGit}/HEAD`)).toBe(true); + expect( + (yield* command(worktree, [ + "--git-dir", + childGit, + "rev-parse", + "refs/heads/local-only", + ])).stdout.trim(), + ).toBe(local); + }), + ), +); + +it.live("keeps a worktree when a stored submodule cannot be inspected", () => + runCleanupTest( + Effect.gen(function* () { + const { service, fs, worktree, command } = yield* submoduleCleanupFixture(); + const admin = (yield* command(worktree, ["rev-parse", "--absolute-git-dir"])).stdout.trim(); + yield* fs.makeDirectory(`${admin}/modules/broken`); + yield* fs.writeFileString(`${admin}/modules/broken/HEAD`, "not a ref\n"); + const report = yield* service.runNow; + expect(report.entries[0]).toMatchObject({ outcome: "kept" }); + expect(report.entries[0]?.reason).toContain("could not be inspected"); + expect(yield* fs.exists(worktree)).toBe(true); + expect(yield* fs.readFileString(`${admin}/modules/broken/HEAD`)).toBe("not a ref\n"); + }), + ), +); + +it.live("preserves late tracked edits with never-initialized submodules", () => + 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", "add uninitialized module"]); + yield* fs.makeDirectory(`${worktree}/module`); + fixture.editBeforeRemoval(); + expect((yield* service.runNow).entries[0]).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 d68444867428..9857dc563c56 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -533,6 +533,83 @@ 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[]) => + git.execute({ + operation: "StorageCleanup.submodules", + cwd, + args, + maxOutputBytes: 64 * 1024, + }); + const admin = yield* inspect(["rev-parse", "--absolute-git-dir"]); + if (admin.stdoutTruncated) + return { force: false, reason: "submodule inspection exceeded the limit" }; + const modules = path.join(admin.stdout.trim(), "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) + 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 changes = yield* localChangesReason(submodule, rules); + if (changes !== null) return { force: false, reason: `submodule ${changes}` }; + const directory = yield* git.execute({ + operation: "StorageCleanup.submoduleGitDirectory", + cwd: submodule, + args: ["rev-parse", "--absolute-git-dir"], + maxOutputBytes: 64 * 1024, + }); + if (directory.stdoutTruncated) + return { force: false, reason: "submodule inspection exceeded the limit" }; + pending.push(directory.stdout.trim()); + } + 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", + "--not", + "--remotes", + ]); + if (unpushed.stdoutTruncated || 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)); + } + } + return { force: stored || paths.length > 0, reason: null }; + }, + Effect.orElseSucceed(() => ({ force: false, reason: "submodules could not be inspected" })), + ); + const cleanupGit = Effect.fn("StorageCleanup.cleanupGit")(function* ( cwd: string, args: string[], @@ -829,13 +906,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 }); From f84676ce6e348570e261f5195fcbf2de94929b4c Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 08:27:32 +0000 Subject: [PATCH 2/8] test(server): simplify submodule cleanup regression coverage --- apps/server/src/storageCleanup.test.ts | 205 +++++++++++-------------- 1 file changed, 88 insertions(+), 117 deletions(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index 69848a6f11e7..d24461849abb 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -483,135 +483,100 @@ const runCleanupTest = ( const submoduleCleanupFixture = () => Effect.gen(function* () { const fixture = yield* cleanupFixture(); - const { command, fs, worktree } = fixture; - const source = yield* fs.makeTempDirectoryScoped({ prefix: "cleanup-submodule-" }); - const remote = yield* fs.makeTempDirectoryScoped({ prefix: "cleanup-submodule-remote-" }); - yield* command(source, ["init", "-b", "main"]); - yield* command(source, ["config", "user.email", "cleanup@example.test"]); - yield* command(source, ["config", "user.name", "Cleanup test"]); - yield* fs.writeFileString(`${source}/tracked.txt`, "original\n"); - yield* fs.writeFileString(`${source}/.gitignore`, ".env\nnode_modules/\n"); - yield* command(source, ["add", "."]); - yield* command(source, ["commit", "-m", "initial"]); - yield* command(source, ["clone", "--bare", source, remote]); - yield* command(worktree, [ - "-c", - "protocol.file.allow=always", - "submodule", - "add", - remote, - "module", - ]); - yield* command(`${worktree}/module`, ["config", "user.name", "Cleanup test"]); - yield* command(`${worktree}/module`, ["config", "user.email", "cleanup@example.test"]); - yield* command(worktree, ["commit", "-am", "add submodule"]); + const { command, repo, worktree } = fixture; const submodule = `${worktree}/module`; - const addNested = () => + 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, [ - "-c", - "protocol.file.allow=always", - "submodule", - "add", - remote, - "child", - ]); - yield* command(submodule, ["commit", "-am", "add child"]); - yield* command(submodule, ["push", "origin", "HEAD:main"]); - yield* command(worktree, ["commit", "-am", "update module"]); - return `${submodule}/child`; + yield* command(submodule, ["commit", "-am", "record child"]); + yield* command(submodule, ["push", "origin", "HEAD:submodule"]); + yield* command(worktree, ["commit", "-am", "record module"]); }); - return { ...fixture, submodule, addNested }; + yield* recordSubmodules(); + return { ...fixture, submodule, child, recordSubmodules }; }); -it.live.each(["populated", "deinitialized"])( - "removes a clean worktree with %s nested submodules", - (state) => - runCleanupTest( - Effect.gen(function* () { - const { service, fs, worktree, submodule, command, addNested } = - yield* submoduleCleanupFixture(); - yield* addNested(); - const unpushed = yield* command(submodule, [ - "rev-list", - "--count", - "HEAD", - "--not", - "--remotes", - ]); - expect(unpushed.stdout.trim()).toBe("0"); - if (state === "deinitialized") yield* command(worktree, ["submodule", "deinit", "--all"]); - const report = yield* service.runNow; - expect(report.entries.find((entry) => entry.path === worktree)).toMatchObject({ - outcome: "removed", - }); - expect(yield* fs.exists(worktree)).toBe(false); - }), - ), -); - -it.live.each(["HEAD", "local branch"])("keeps an unpushed submodule %s", (unpushed) => +it.live("removes clean submodules recursively, including deinitialized repositories", () => runCleanupTest( Effect.gen(function* () { - const { service, fs, worktree, submodule, command } = yield* submoduleCleanupFixture(); - yield* command(submodule, ["checkout", "-b", "local-only"]); - yield* command(submodule, ["commit", "--allow-empty", "-m", "local only"]); - if (unpushed === "local branch") yield* command(submodule, ["checkout", "main"]); - yield* command(worktree, ["add", "module"]); - yield* command(worktree, ["commit", "--allow-empty", "-m", "record module"]); + 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[0]).toMatchObject({ outcome: "kept" }); - expect(report.entries[0]?.reason).toContain("not on a remote-tracking ref"); - expect(yield* fs.exists(submodule)).toBe(true); + 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.each([ - ["tracked.txt", "uncommitted-changes"], - [".env", "any-local-files"], -] as const)("keeps submodule %s under %s", ([file, policy]) => +it.live("keeps nested submodule files protected by the current keep rule", () => runCleanupTest( Effect.gen(function* () { - const { service, fs, submodule, setPolicy } = yield* submoduleCleanupFixture(); - setPolicy(policy); - yield* fs.writeFileString(`${submodule}/${file}`, "local edit\n"); - const report = yield* service.runNow; - expect(report.entries[0]).toMatchObject({ outcome: "kept" }); - if (file === ".env") expect(report.entries[0]?.reason).toContain("ignored files (.env)"); - expect(yield* fs.readFileString(`${submodule}/${file}`)).toBe("local edit\n"); + const { service, fs, worktree, child, 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"); }), ), ); -it.live("keeps a deinitialized nested submodule's local branch in an embedded parent", () => +it.live("keeps unpushed submodule history after checkout and deinitialization", () => runCleanupTest( Effect.gen(function* () { - const { service, fs, worktree, submodule, command, addNested } = + const { service, fs, worktree, submodule, child, command, recordSubmodules } = yield* submoduleCleanupFixture(); - const child = yield* addNested(); - for (const [key, value] of [ - ["user.name", "Cleanup test"], - ["user.email", "cleanup@example.test"], - ] as const) - yield* command(child, ["config", key, value]); yield* command(child, ["checkout", "-b", "local-only"]); - yield* command(child, ["commit", "--allow-empty", "-m", "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* command(submodule, ["submodule", "deinit", "--all"]); - const parentGit = (yield* command(submodule, [ - "rev-parse", - "--absolute-git-dir", - ])).stdout.trim(); - yield* command(submodule, ["config", "core.worktree", submodule]); - yield* fs.remove(`${submodule}/.git`); - yield* fs.rename(parentGit, `${submodule}/.git`); - const childGit = `${submodule}/.git/modules/child`; - const report = yield* service.runNow; - expect(report.entries[0]).toMatchObject({ outcome: "kept" }); - expect(report.entries[0]?.reason).toContain("not on a remote-tracking ref"); - expect(yield* fs.exists(`${childGit}/HEAD`)).toBe(true); + 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", @@ -620,27 +585,31 @@ it.live("keeps a deinitialized nested submodule's local branch in an embedded pa "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"); }), ), ); -it.live("keeps a worktree when a stored submodule cannot be inspected", () => +it.live("keeps submodules when stored repository metadata is malformed", () => runCleanupTest( Effect.gen(function* () { - const { service, fs, worktree, command } = yield* submoduleCleanupFixture(); - const admin = (yield* command(worktree, ["rev-parse", "--absolute-git-dir"])).stdout.trim(); - yield* fs.makeDirectory(`${admin}/modules/broken`); - yield* fs.writeFileString(`${admin}/modules/broken/HEAD`, "not a ref\n"); - const report = yield* service.runNow; - expect(report.entries[0]).toMatchObject({ outcome: "kept" }); - expect(report.entries[0]?.reason).toContain("could not be inspected"); + 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(`${admin}/modules/broken/HEAD`)).toBe("not a ref\n"); + expect(yield* fs.readFileString(`${childGit}/HEAD`)).toBe("not a ref\n"); }), ), ); -it.live("preserves late tracked edits with never-initialized submodules", () => +it.live("preserves late tracked edits when submodules were never initialized", () => runCleanupTest( Effect.gen(function* () { const fixture = yield* cleanupFixture(); @@ -652,10 +621,12 @@ it.live("preserves late tracked edits with never-initialized submodules", () => ); yield* command(worktree, ["add", ".gitmodules"]); yield* command(worktree, ["update-index", "--add", "--cacheinfo", `160000,${head},module`]); - yield* command(worktree, ["commit", "-m", "add uninitialized module"]); + yield* command(worktree, ["commit", "-m", "record uninitialized module"]); yield* fs.makeDirectory(`${worktree}/module`); fixture.editBeforeRemoval(); - expect((yield* service.runNow).entries[0]).toMatchObject({ outcome: "failed" }); + expect( + (yield* service.runNow).entries.find((entry) => entry.path === worktree), + ).toMatchObject({ outcome: "failed" }); expect(yield* fs.readFileString(`${worktree}/tracked.txt`)).toBe("late edit\n"); }), ), From d97c136b06997c31cf82cb528120b227692c9292 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 08:36:49 +0000 Subject: [PATCH 3/8] fix(server): tighten submodule cleanup inspection --- apps/server/src/storageCleanup.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index 9857dc563c56..fc2f427483ff 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -543,9 +543,9 @@ const make = Effect.gen(function* () { maxOutputBytes: 64 * 1024, }); const admin = yield* inspect(["rev-parse", "--absolute-git-dir"]); - if (admin.stdoutTruncated) + if (admin.stdoutTruncated || admin.stderrTruncated) return { force: false, reason: "submodule inspection exceeded the limit" }; - const modules = path.join(admin.stdout.trim(), "modules"); + 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 }; @@ -556,22 +556,15 @@ const make = Effect.gen(function* () { "--quiet", 'printf "%s\\0" "$toplevel/$sm_path"', ]); - if (populated.stdoutTruncated) + 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 changes = yield* localChangesReason(submodule, rules); - if (changes !== null) return { force: false, reason: `submodule ${changes}` }; - const directory = yield* git.execute({ - operation: "StorageCleanup.submoduleGitDirectory", - cwd: submodule, - args: ["rev-parse", "--absolute-git-dir"], - maxOutputBytes: 64 * 1024, - }); - if (directory.stdoutTruncated) + 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.trim()); + pending.push(directory.stdout.replace(/\n$/, "")); } const visited = new Set(); while (pending.length > 0) { @@ -597,7 +590,9 @@ const make = Effect.gen(function* () { "--not", "--remotes", ]); - if (unpushed.stdoutTruncated || unpushed.stdout.trim() !== "") + 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 { @@ -605,6 +600,11 @@ const make = Effect.gen(function* () { 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}` }; + } return { force: stored || paths.length > 0, reason: null }; }, Effect.orElseSucceed(() => ({ force: false, reason: "submodules could not be inspected" })), From 68b88e79e9127723a2bbf4e253fe8d20a433bc13 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 08:50:22 +0000 Subject: [PATCH 4/8] test(server): expose file loss after submodule deinitialization Extend the nested keep-policy regression to require surviving .env bytes after deinitialization. Expected red on the current runtime: 141 pass, one fails because cleanup removes the worktree and file. Leave the runtime fix to the separate implementation round. --- apps/server/src/storageCleanup.test.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index d24461849abb..569b0eec80c5 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -534,7 +534,8 @@ it.live("removes clean submodules recursively, including deinitialized repositor it.live("keeps nested submodule files protected by the current keep rule", () => runCleanupTest( Effect.gen(function* () { - const { service, fs, worktree, child, setPolicy } = yield* submoduleCleanupFixture(); + const { service, fs, worktree, submodule, child, command, setPolicy } = + yield* submoduleCleanupFixture(); setPolicy("tracked-changes"); yield* fs.writeFileString(`${child}/tracked.txt`, "local edit\n"); expect( @@ -548,6 +549,17 @@ it.live("keeps nested submodule files protected by the current keep rule", () => (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" }); }), ), ); From a5bb82ca94e053931c9501cc459d05ff107370f1 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 08:54:58 +0000 Subject: [PATCH 5/8] fix(server): protect files in deinitialized submodules --- apps/server/src/storageCleanup.ts | 36 ++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index fc2f427483ff..7df748a3c420 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -605,7 +605,41 @@ const make = Effect.gen(function* () { const changes = yield* localChangesReason(submodule, rules); if (changes !== null) return { force: false, reason: `submodule ${changes}` }; } - return { force: stored || paths.length > 0, reason: null }; + const force = stored || paths.length > 0; + if (force && rules.worktreeKeepWhen === "any-local-files") { + // Git status and foreach omit files inside deinitialized gitlink directories. + const populatedPaths = new Set(paths.map(pathKey)); + const unchecked: string[] = []; + for (const repository of [cwd, ...paths]) { + const index = yield* inspect(["-C", repository, "ls-files", "--stage", "-z"]); + 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(cwd, 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" })), ); From c5e41a9ed3e56e58042faba29f023bbbc7c0cdcd Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 09:07:09 +0000 Subject: [PATCH 6/8] test(server): protect deinitialized files under default cleanup policy --- apps/server/src/storageCleanup.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index 569b0eec80c5..b0d2a3d2e80c 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -560,6 +560,17 @@ it.live("keeps nested submodule files protected by the current keep rule", () => ? 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" }); }), ), ); From bffca7f18c967a9a2f79eb183a33dc09251ea21b Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 09:12:19 +0000 Subject: [PATCH 7/8] fix(server): preserve deinitialized files across cleanup policies --- apps/server/src/storageCleanup.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index 7df748a3c420..429f0a9b1214 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -535,12 +535,12 @@ const make = Effect.gen(function* () { const submoduleRemoval = Effect.fn("StorageCleanup.submoduleRemoval")( function* (cwd: string, rules: WorktreeCleanupRules) { - const inspect = (args: string[]) => + const inspect = (args: string[], maxOutputBytes = 64 * 1024) => git.execute({ operation: "StorageCleanup.submodules", cwd, args, - maxOutputBytes: 64 * 1024, + maxOutputBytes, }); const admin = yield* inspect(["rev-parse", "--absolute-git-dir"]); if (admin.stdoutTruncated || admin.stderrTruncated) @@ -606,18 +606,22 @@ const make = Effect.gen(function* () { if (changes !== null) return { force: false, reason: `submodule ${changes}` }; } const force = stored || paths.length > 0; - if (force && rules.worktreeKeepWhen === "any-local-files") { + 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 [cwd, ...paths]) { - const index = yield* inspect(["-C", repository, "ls-files", "--stage", "-z"]); + 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(cwd, submodule)) + 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); From f9caddc24e724196615713a37017b4e90fe6e8d8 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 20 Oct 2026 04:49:36 +0000 Subject: [PATCH 8/8] fix(server): retain reflog-only submodule history during cleanup --- apps/server/src/storageCleanup.test.ts | 7 +++++++ apps/server/src/storageCleanup.ts | 1 + 2 files changed, 8 insertions(+) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index cb297b3384c9..ba98b3cbb2a3 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -635,6 +635,13 @@ it.live("keeps unpushed submodule history after checkout and deinitialization", (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"); }), ), ); diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index 9dac97df555a..a4d1e831d900 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -592,6 +592,7 @@ const make = Effect.gen(function* () { "--max-count=1", "HEAD", "--all", + "--reflog", "--not", "--remotes", ]);