From 665dcef1296d22bb7f423702bce9113153fb0e47 Mon Sep 17 00:00:00 2001 From: Romil Bijarnia Date: Wed, 11 Nov 2026 17:18:56 +1100 Subject: [PATCH 1/3] fix(server): keep files when submodule checks retain a worktree --- apps/server/src/storageCleanup.test.ts | 8 +++++++- apps/server/src/storageCleanup.ts | 6 +++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index de1ccfefcf64f..232670f01be9f 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -751,15 +751,21 @@ it.live("keeps unpushed submodule history after checkout and deinitialization", it.live("keeps submodules when stored repository metadata is malformed", () => runCleanupTest( Effect.gen(function* () { - const { service, fs, worktree, submodule, child, command } = yield* submoduleCleanupFixture(); + const { service, fs, worktree, submodule, child, command, setPolicy } = + yield* submoduleCleanupFixture(); + setPolicy("tracked-changes"); 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"); + yield* fs.writeFileString(`${worktree}/notes.txt`, "keep while the worktree stays\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"); + expect(yield* fs.readFileString(`${worktree}/notes.txt`)).toBe( + "keep while the worktree stays\n", + ); }), ), ); diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index a5f9cd7eefa12..497de408c7fc4 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -1206,15 +1206,15 @@ const make = Effect.gen(function* () { ) ) return keep("settings changed since 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); } + // Submodule checks can retain the worktree, so do not discard its files before them. + if (settings.worktreeKeepWhen === "tracked-changes") + yield* cleanupGit(worktreePath, ["clean", "-ffdx"], "git clean"); yield* removeLinkedWorktree({ operation: "StorageCleanup.cleanupGit", cwd: project.workspaceRoot, From db9d40886b4b205ca143a15e20b4b88b0ace9b6b Mon Sep 17 00:00:00 2001 From: Romil Bijarnia Date: Wed, 11 Nov 2026 17:35:33 +1100 Subject: [PATCH 2/3] fix(server): avoid cleaning between checks and forced worktree removal --- apps/server/src/storageCleanup.test.ts | 27 ++++++++++++++++++++++++-- apps/server/src/storageCleanup.ts | 4 ++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index 232670f01be9f..1fa9f8f5179b0 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -632,8 +632,26 @@ const submoduleCleanupFixture = () => it.live("removes clean submodules recursively, including deinitialized repositories", () => runCleanupTest( Effect.gen(function* () { - const { service, fs, repo, worktree, submodule, command, recordSubmodules } = - yield* submoduleCleanupFixture(); + const fs = yield* FileSystem.FileSystem; + const git = yield* GitVcsDriver.GitVcsDriver; + let editedDuringClean = false; + const { service, repo, worktree, submodule, command, recordSubmodules, setPolicy } = + yield* submoduleCleanupFixture().pipe( + Effect.provideService(GitVcsDriver.GitVcsDriver, { + ...git, + execute: (input) => + git.execute(input).pipe( + Effect.tap(() => { + if (input.args[0] !== "clean") return Effect.void; + editedDuringClean = true; + return fs.writeFileString(`${input.cwd}/tracked.txt`, "late edit\n"); + }), + ), + }), + ); + setPolicy("tracked-changes"); + yield* fs.writeFileString(`${worktree}/notes.txt`, "untracked\n"); + yield* fs.writeFileString(`${worktree}/.env`, "ignored\n"); yield* command(submodule, [ "-c", "protocol.file.allow=always", @@ -646,6 +664,11 @@ it.live("removes clean submodules recursively, including deinitialized repositor yield* command(submodule, ["submodule", "deinit", "stored"]); const admin = (yield* command(worktree, ["rev-parse", "--absolute-git-dir"])).stdout.trim(); const report = yield* service.runNow; + if (editedDuringClean) { + expect(yield* fs.readFileString(`${worktree}/tracked.txt`)).toBe("late edit\n"); + expect(report.entries.find((entry) => entry.path === worktree)?.outcome).toBe("kept"); + return; + } expect(report.entries.find((entry) => entry.path === worktree)).toMatchObject({ outcome: "removed", }); diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index 497de408c7fc4..f062a04e7dfaf 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -1212,8 +1212,8 @@ const make = Effect.gen(function* () { const changes = yield* localChangesReason(worktreePath, settings); if (changes !== null) return keep(changes); } - // Submodule checks can retain the worktree, so do not discard its files before them. - if (settings.worktreeKeepWhen === "tracked-changes") + // Forced removal already discards local files; do not widen its final-check window. + if (settings.worktreeKeepWhen === "tracked-changes" && !submodules.force) yield* cleanupGit(worktreePath, ["clean", "-ffdx"], "git clean"); yield* removeLinkedWorktree({ operation: "StorageCleanup.cleanupGit", From 76087ead5c945038ff0d24f1b293baba434d6476 Mon Sep 17 00:00:00 2001 From: Romil Bijarnia Date: Wed, 11 Nov 2026 17:54:10 +1100 Subject: [PATCH 3/3] test(server): require forced worktree removal without preliminary clean --- apps/server/src/storageCleanup.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index 1fa9f8f5179b0..c3c7c27bc4b23 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -664,11 +664,7 @@ it.live("removes clean submodules recursively, including deinitialized repositor yield* command(submodule, ["submodule", "deinit", "stored"]); const admin = (yield* command(worktree, ["rev-parse", "--absolute-git-dir"])).stdout.trim(); const report = yield* service.runNow; - if (editedDuringClean) { - expect(yield* fs.readFileString(`${worktree}/tracked.txt`)).toBe("late edit\n"); - expect(report.entries.find((entry) => entry.path === worktree)?.outcome).toBe("kept"); - return; - } + expect(editedDuringClean).toBe(false); expect(report.entries.find((entry) => entry.path === worktree)).toMatchObject({ outcome: "removed", });