From 382aebbc2b48fdc394dfc4d0be63d265a0ed6574 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 7 Nov 2026 02:43:33 +0000 Subject: [PATCH 1/4] test(server): reproduce #32259 storage cleanup keeping submodules with remote tags Storage cleanup treats a submodule tag that origin also has, on a commit no remote branch contains, as unpushed history and keeps the worktree forever. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/storageCleanup.test.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index b551b0260147..0c0169ea7a42 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -652,6 +652,31 @@ it.live("removes clean submodules recursively, including deinitialized repositor ), ); +it.live("removes submodules whose tags match the remote's tags off every remote branch", () => + runCleanupTest( + Effect.gen(function* () { + const { service, fs, repo, worktree, submodule, command } = yield* submoduleCleanupFixture(); + const tree = (yield* command(repo, ["rev-parse", "HEAD^{tree}"])).stdout.trim(); + const release = (yield* command(repo, [ + "commit-tree", + tree, + "-p", + "HEAD", + "-m", + "release", + ])).stdout.trim(); + yield* command(repo, ["tag", "v0.9.1", release]); + yield* command(submodule, ["fetch", "origin", "tag", "v0.9.1"]); + const report = yield* service.runNow; + expect(report.entries.find((entry) => entry.path === worktree)).toMatchObject({ + outcome: "removed", + reason: expect.not.stringContaining("remote-tracking"), + }); + expect(yield* fs.exists(worktree)).toBe(false); + }), + ), +); + it.live("keeps nested submodule files protected by the current keep rule", () => runCleanupTest( Effect.gen(function* () { From 859f43ccdb9dc7ec67822622db5669805da640fe Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 7 Nov 2026 02:51:52 +0000 Subject: [PATCH 2/4] fix(server): storage cleanup treats submodule tags the remote has as pushed Storage cleanup's submodule check used `rev-list --all --not --remotes`, which counts every local tag as unpushed work. Submodule clones fetch all upstream tags, so a release or backup tag on a commit no remote branch contains kept the worktree forever. When the existing check finds history only tags reach, compare local tags with each remote's `ls-remote --tags --refs` output and only protect the commits reached by tags no remote has at the same object. The extra checks stay offline unless that case occurs. A remote that cannot be listed vouches for no tags, so the worktree is kept as before. Remote tag lists are cached per sweep by URL, ls-remote runs non-interactively with a 20s timeout, and unpushed tags reach rev-list over stdin. Fixes pingdotgg/t3code#32259 Co-Authored-By: Claude Opus 5.5 --- apps/server/src/storageCleanup.test.ts | 30 +++++++ apps/server/src/storageCleanup.ts | 102 +++++++++++++++++++----- apps/server/src/vcs/GitVcsDriverCore.ts | 6 +- 3 files changed, 116 insertions(+), 22 deletions(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index 0c0169ea7a42..104b282f49ae 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -677,6 +677,36 @@ it.live("removes submodules whose tags match the remote's tags off every remote ), ); +it.live("keeps submodules whose tags the remote lacks or has at another commit", () => + runCleanupTest( + Effect.gen(function* () { + const { service, fs, repo, worktree, submodule, command } = yield* submoduleCleanupFixture(); + const commitOff = (cwd: string, message: string) => + Effect.gen(function* () { + const tree = (yield* command(cwd, ["rev-parse", "HEAD^{tree}"])).stdout.trim(); + return (yield* command(cwd, [ + "commit-tree", + tree, + "-p", + "HEAD", + "-m", + message, + ])).stdout.trim(); + }); + yield* command(repo, ["tag", "v0.9.1", yield* commitOff(repo, "release")]); + const local = yield* commitOff(submodule, "local release"); + for (const tag of ["local-only", "v0.9.1"]) { + yield* command(submodule, ["tag", tag, local]); + expect( + (yield* service.runNow).entries.find((entry) => entry.path === worktree), + ).toMatchObject({ outcome: "kept", reason: expect.stringContaining("remote-tracking") }); + yield* command(submodule, ["tag", "-d", tag]); + } + expect(yield* fs.exists(worktree)).toBe(true); + }), + ), +); + it.live("keeps nested submodule files protected by the current keep rule", () => runCleanupTest( Effect.gen(function* () { diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index 5328f4cfaea9..eb5ea9f5af99 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -47,6 +47,7 @@ import * as Settings from "./serverSettings.ts"; import * as TerminalManager from "./terminal/Manager.ts"; import { isFilesystemRoot, managedWorktreesDirectories } from "./worktreesDirectory.ts"; import * as GitVcsDriver from "./vcs/GitVcsDriver.ts"; +import { NON_INTERACTIVE_GIT_ENV } from "./vcs/GitVcsDriverCore.ts"; import { removeLinkedWorktree } from "./vcs/worktreeRemoval.ts"; import { withWorkspaceLease } from "./workspace/workspaceLease.ts"; import * as HostProcess from "@t3tools/shared/HostProcess"; @@ -656,8 +657,85 @@ const make = Effect.gen(function* () { return file === undefined ? null : `has ignored files (${file})`; }); + const unpushedHistoryReason = Effect.fn("StorageCleanup.unpushedHistoryReason")(function* ( + cwd: string, + directory: string, + remoteTags: Map | null>, + ) { + const inspect = (args: string[], options?: Partial) => + git.execute({ + operation: "StorageCleanup.submodules", + cwd, + args: ["--git-dir", directory, "--work-tree", cwd, ...args], + maxOutputBytes: 64 * 1024, + ...options, + }); + const exceeded = "submodule inspection exceeded the limit"; + const unpushed = "submodule has commits not on a remote-tracking ref"; + const reachable = Effect.fnUntraced(function* (revisions: string[], stdin?: string) { + const result = yield* inspect( + ["rev-list", "--max-count=1", ...revisions, "--not", "--remotes"], + stdin === undefined ? {} : { stdin }, + ); + if (result.stdoutTruncated || result.stderrTruncated) return exceeded; + return result.stdout.trim() === "" ? null : unpushed; + }); + const all = yield* reachable(["HEAD", "--all", "--reflog"]); + if (all !== unpushed) return all; + const untagged = yield* reachable(["HEAD", "--exclude=refs/tags/*", "--all", "--reflog"]); + if (untagged !== null) return untagged; + // Clones fetch every upstream tag, so a tag a remote has at the same object is pushed. + const tags = yield* inspect( + ["for-each-ref", "--format=%(objectname)%09%(refname)", "refs/tags"], + { + maxOutputBytes: 16 * 1024 * 1024, + }, + ); + const remotes = yield* inspect(["remote"]); + if (tags.stdoutTruncated || tags.stderrTruncated || remotes.stdoutTruncated) return exceeded; + let local = tags.stdout.split("\n").filter(Boolean); + for (const remote of remotes.stdout.split("\n").filter(Boolean)) { + if (local.length === 0) break; + const url = yield* inspect(["ls-remote", "--get-url", "--", remote]); + if (url.stdoutTruncated) return exceeded; + const location = url.stdout.trim(); + // Relative remote paths resolve from the worktree, so key them by absolute path. + const key = + path.isAbsolute(location) || /^[^/]*:/.test(location) + ? location + : path.resolve(cwd, location); + let published = remoteTags.get(key); + if (published === undefined) { + published = yield* inspect(["ls-remote", "--tags", "--refs", "--", remote], { + operation: "StorageCleanup.remoteTags", + env: NON_INTERACTIVE_GIT_ENV, + timeoutMs: 20_000, + maxOutputBytes: 16 * 1024 * 1024, + }).pipe( + Effect.map((result) => + result.stdoutTruncated ? null : new Set(result.stdout.split("\n")), + ), + // An unreachable remote vouches for no tags. + Effect.orElseSucceed(() => new Set()), + ); + remoteTags.set(key, published); + } + if (published === null) return exceeded; + local = local.filter((line) => !published.has(line)); + } + if (local.length === 0) return null; + return yield* reachable( + ["--stdin"], + local.map((line) => `${line.slice(line.indexOf("\t") + 1)}\n`).join(""), + ); + }); + const submoduleRemoval = Effect.fn("StorageCleanup.submoduleRemoval")( - function* (cwd: string, rules: WorktreeCleanupRules) { + function* ( + cwd: string, + rules: WorktreeCleanupRules, + remoteTags: Map | null>, + ) { const inspect = (args: string[], maxOutputBytes = 64 * 1024) => git.execute({ operation: "StorageCleanup.submodules", @@ -701,23 +779,8 @@ const make = Effect.gen(function* () { 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" }; + const unpushed = yield* unpushedHistoryReason(cwd, directory, remoteTags); + if (unpushed !== null) return { force: false, reason: unpushed }; pending.push(path.join(directory, "modules")); } else { for (const name of yield* fs.readDirectory(directory)) @@ -830,6 +893,7 @@ const make = Effect.gen(function* () { const threadsById = new Map(snapshot.threads.map((thread) => [thread.id, thread])); const refreshedDefaultRefs = new Map>(); const defaultBranches = new Map(); + const remoteTags = new Map | null>(); const groups = Map.groupBy( snapshot.threads.filter((thread) => thread.worktreePath !== null), (thread) => path.resolve(thread.worktreePath!), @@ -1170,7 +1234,7 @@ const make = Effect.gen(function* () { // 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); + const submodules = yield* submoduleRemoval(worktreePath, settings, remoteTags); if (submodules.reason !== null) return keep(submodules.reason); if (submodules.force) { const changes = yield* localChangesReason(worktreePath, settings); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 85a7e524b1a6..b1d0f4844fdf 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -110,7 +110,7 @@ const LIST_REFS_REFRESH_COALESCE_TTL = Duration.seconds(5); const LIST_REFS_REFRESH_FAILURE_COOLDOWN = Duration.seconds(30); const STATUS_DEFAULT_BRANCH_CACHE_TTL = Duration.minutes(5); const STATUS_ORIGIN_EXISTS_CACHE_TTL = Duration.minutes(5); -const STATUS_UPSTREAM_REFRESH_ENV = Object.freeze({ +export const NON_INTERACTIVE_GIT_ENV = Object.freeze({ GCM_INTERACTIVE: "never", GIT_ASKPASS: "", GIT_TERMINAL_PROMPT: "0", @@ -1371,7 +1371,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* // healthy writer. A separate safety limit prevents a hung transport from // occupying its worker slot indefinitely. const started = yield* executeGit("GitVcsDriver.fetchRemoteForStatus", fetchCwd, args, { - env: STATUS_UPSTREAM_REFRESH_ENV, + env: NON_INTERACTIVE_GIT_ENV, fallbackErrorDetail: "Background Git fetch exited with a non-zero status.", timeoutMs: Duration.toMillis(STATUS_UPSTREAM_FETCH_TIMEOUT), forceKillAfterMs: 1_000, @@ -4529,7 +4529,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const fetchRemote = Effect.fn("fetchRemote")(function* (input: GitVcsDriver.GitFetchRemoteInput) { const args = ["fetch", "--quiet", "--", input.remoteName]; const options = { - env: STATUS_UPSTREAM_REFRESH_ENV, + env: NON_INTERACTIVE_GIT_ENV, fallbackErrorDetail: `git fetch ${input.remoteName} failed`, }; // Another fetch in the same repository, such as the background status From c388a10b2fc91e7309d94bc16f31043d6d015b17 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 7 Nov 2026 03:11:16 +0000 Subject: [PATCH 3/4] fix(server): storage cleanup trusts published submodule tags HEAD points at The tag-aware submodule check still flagged a commit that only a remote tag reaches whenever HEAD, a branch or the reflog pointed at it, such as a submodule pinned at an upstream release tag. It also listed remote tags from the parent worktree, so a relative remote URL resolved against the wrong directory and the worktree was kept. Run the offline check first. Only when it finds history, list each remote's tags and rerun it with a `^refs/tags/` negative on stdin for every local tag a remote has at the same object. Remote tags are listed from the submodule's own work tree (core.worktree), so relative remote paths resolve as git resolves them there; deinitialized submodules have no work tree and stay kept when their remote URL is relative. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/storageCleanup.test.ts | 12 +++-- apps/server/src/storageCleanup.ts | 63 ++++++++++++++++---------- 2 files changed, 47 insertions(+), 28 deletions(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index 104b282f49ae..49e1d14a3f61 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -8,7 +8,7 @@ import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Queue from "effect/Queue"; -import type * as Path from "effect/Path"; +import * as Path from "effect/Path"; import type * as Scope from "effect/Scope"; import * as Stream from "effect/Stream"; import { SCAN_CACHE_FILE_NAME } from "./usage/usageScanCache.ts"; @@ -656,17 +656,23 @@ it.live("removes submodules whose tags match the remote's tags off every remote runCleanupTest( Effect.gen(function* () { const { service, fs, repo, worktree, submodule, command } = yield* submoduleCleanupFixture(); - const tree = (yield* command(repo, ["rev-parse", "HEAD^{tree}"])).stdout.trim(); + const head = (yield* command(submodule, ["rev-parse", "HEAD"])).stdout.trim(); + const tree = (yield* command(submodule, ["rev-parse", "HEAD^{tree}"])).stdout.trim(); const release = (yield* command(repo, [ "commit-tree", tree, "-p", - "HEAD", + head, "-m", "release", ])).stdout.trim(); yield* command(repo, ["tag", "v0.9.1", release]); yield* command(submodule, ["fetch", "origin", "tag", "v0.9.1"]); + // Pinning the submodule at the release puts the tag's commit in HEAD and the reflog. + yield* command(submodule, ["checkout", "--detach", "v0.9.1"]); + yield* command(worktree, ["commit", "-am", "pin module release"]); + const { relative } = yield* Path.Path; + yield* command(submodule, ["config", "remote.origin.url", relative(submodule, repo)]); const report = yield* service.runNow; 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 eb5ea9f5af99..ee469233eac1 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -672,18 +672,25 @@ const make = Effect.gen(function* () { }); const exceeded = "submodule inspection exceeded the limit"; const unpushed = "submodule has commits not on a remote-tracking ref"; - const reachable = Effect.fnUntraced(function* (revisions: string[], stdin?: string) { + const reachable = Effect.fnUntraced(function* (stdin?: string) { const result = yield* inspect( - ["rev-list", "--max-count=1", ...revisions, "--not", "--remotes"], + [ + "rev-list", + "--max-count=1", + ...(stdin === undefined ? [] : ["--stdin"]), + "HEAD", + "--all", + "--reflog", + "--not", + "--remotes", + ], stdin === undefined ? {} : { stdin }, ); if (result.stdoutTruncated || result.stderrTruncated) return exceeded; return result.stdout.trim() === "" ? null : unpushed; }); - const all = yield* reachable(["HEAD", "--all", "--reflog"]); + const all = yield* reachable(); if (all !== unpushed) return all; - const untagged = yield* reachable(["HEAD", "--exclude=refs/tags/*", "--all", "--reflog"]); - if (untagged !== null) return untagged; // Clones fetch every upstream tag, so a tag a remote has at the same object is pushed. const tags = yield* inspect( ["for-each-ref", "--format=%(objectname)%09%(refname)", "refs/tags"], @@ -692,41 +699,47 @@ const make = Effect.gen(function* () { }, ); const remotes = yield* inspect(["remote"]); + const worktree = yield* inspect(["config", "core.worktree"], { allowNonZeroExit: true }); if (tags.stdoutTruncated || tags.stderrTruncated || remotes.stdoutTruncated) return exceeded; - let local = tags.stdout.split("\n").filter(Boolean); + // Relative remote paths resolve from the submodule's work tree. Deinit unsets it. + const base = path.resolve(directory, worktree.stdout.trim() || "."); + const local = tags.stdout.split("\n").filter(Boolean); + const pushed = new Set(); for (const remote of remotes.stdout.split("\n").filter(Boolean)) { - if (local.length === 0) break; + if (pushed.size === local.length) break; const url = yield* inspect(["ls-remote", "--get-url", "--", remote]); if (url.stdoutTruncated) return exceeded; const location = url.stdout.trim(); - // Relative remote paths resolve from the worktree, so key them by absolute path. const key = path.isAbsolute(location) || /^[^/]*:/.test(location) ? location - : path.resolve(cwd, location); + : path.resolve(base, location); let published = remoteTags.get(key); if (published === undefined) { - published = yield* inspect(["ls-remote", "--tags", "--refs", "--", remote], { - operation: "StorageCleanup.remoteTags", - env: NON_INTERACTIVE_GIT_ENV, - timeoutMs: 20_000, - maxOutputBytes: 16 * 1024 * 1024, - }).pipe( - Effect.map((result) => - result.stdoutTruncated ? null : new Set(result.stdout.split("\n")), - ), - // An unreachable remote vouches for no tags. - Effect.orElseSucceed(() => new Set()), - ); + published = yield* git + .execute({ + operation: "StorageCleanup.remoteTags", + cwd: base, + args: ["--git-dir", directory, "ls-remote", "--tags", "--refs", "--", remote], + env: NON_INTERACTIVE_GIT_ENV, + timeoutMs: 20_000, + maxOutputBytes: 16 * 1024 * 1024, + }) + .pipe( + Effect.map((result) => + result.stdoutTruncated ? null : new Set(result.stdout.split("\n")), + ), + // An unreachable remote vouches for no tags. + Effect.orElseSucceed(() => new Set()), + ); remoteTags.set(key, published); } if (published === null) return exceeded; - local = local.filter((line) => !published.has(line)); + for (const line of local) if (published.has(line)) pushed.add(line); } - if (local.length === 0) return null; + if (pushed.size === 0) return unpushed; return yield* reachable( - ["--stdin"], - local.map((line) => `${line.slice(line.indexOf("\t") + 1)}\n`).join(""), + [...pushed].map((line) => `^${line.slice(line.indexOf("\t") + 1)}\n`).join(""), ); }); From 80821ca23a69c2d232751864c74f463e7f1e4681 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 7 Nov 2026 03:27:39 +0000 Subject: [PATCH 4/4] fix(server): storage cleanup excludes verified submodule tag objects Exclude the object ids ls-remote vouched for instead of the tag names, so a tag moved after the listing cannot hide unpushed history. Cover a published tag with local work on top. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/storageCleanup.test.ts | 9 +++++++-- apps/server/src/storageCleanup.ts | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index 49e1d14a3f61..b6ff58dd3d96 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -687,14 +687,14 @@ it.live("keeps submodules whose tags the remote lacks or has at another commit", runCleanupTest( Effect.gen(function* () { const { service, fs, repo, worktree, submodule, command } = yield* submoduleCleanupFixture(); - const commitOff = (cwd: string, message: string) => + const commitOff = (cwd: string, message: string, parent = "HEAD") => Effect.gen(function* () { const tree = (yield* command(cwd, ["rev-parse", "HEAD^{tree}"])).stdout.trim(); return (yield* command(cwd, [ "commit-tree", tree, "-p", - "HEAD", + parent, "-m", message, ])).stdout.trim(); @@ -708,6 +708,11 @@ it.live("keeps submodules whose tags the remote lacks or has at another commit", ).toMatchObject({ outcome: "kept", reason: expect.stringContaining("remote-tracking") }); yield* command(submodule, ["tag", "-d", tag]); } + yield* command(submodule, ["fetch", "origin", "tag", "v0.9.1"]); + yield* command(submodule, ["branch", "work", yield* commitOff(submodule, "work", "v0.9.1")]); + expect( + (yield* service.runNow).entries.find((entry) => entry.path === worktree), + ).toMatchObject({ outcome: "kept", reason: expect.stringContaining("remote-tracking") }); expect(yield* fs.exists(worktree)).toBe(true); }), ), diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index ee469233eac1..986b1159bc13 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -739,7 +739,8 @@ const make = Effect.gen(function* () { } if (pushed.size === 0) return unpushed; return yield* reachable( - [...pushed].map((line) => `^${line.slice(line.indexOf("\t") + 1)}\n`).join(""), + // Exclude the verified objects, not the names: a tag can move after the listing. + [...pushed].map((line) => `^${line.slice(0, line.indexOf("\t"))}\n`).join(""), ); });