Skip to content

perf(server): storage cleanup no longer runs a full status or size walk for worktrees it keeps #17911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
t3dotgg merged 1 commit into from
Oct 11, 2026
+213 −51
Merged

perf(server): storage cleanup no longer runs a full status or size walk for worktrees it keeps #17911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 99 additions & 2 deletions apps/server/src/storageCleanup.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import * as ProjectionStore from "./orchestration-v2/ProjectionStore.ts";
import * as ProjectStore from "./orchestration-v2/ProjectStore.ts";
import * as Orchestrator from "./orchestration-v2/Orchestrator.ts";
import * as TerminalManager from "./terminal/Manager.ts";
import { CodexProviderCapabilitiesV2 } from "./orchestration-v2/Adapters/CodexAdapterV2.ts";
import { ServerActivation } from "./serverActivation.ts";
import { describe, expect, it } from "@effect/vitest";
import {
Expand Down Expand Up @@ -248,6 +249,8 @@ const cleanupFixture = Effect.gen(function* () {
};
let threads = [shell({ branch: "feature", worktreePath: worktree })];
let editBeforeRemoval = false;
let openSessionCwd: string | null = null;
let fullStatusCalls = 0;
const context = yield* Layer.build(StorageCleanup.layer).pipe(
Effect.provideService(GitVcsDriver.GitVcsDriver, {
...git,
Expand All @@ -257,6 +260,10 @@ const cleanupFixture = Effect.gen(function* () {
yield* fs.writeFileString(`${worktree}/tracked.txt`, "late edit\n");
return yield* git.execute(input);
}),
statusDetailsLocal: (...args) => {
fullStatusCalls++;
return git.statusDetailsLocal(...args);
},
}),
Effect.provideService(Settings.ServerSettingsService, {
getSettings: Effect.sync(() => settings),
Expand All @@ -272,8 +279,27 @@ const cleanupFixture = Effect.gen(function* () {
Effect.provideService(Orchestrator.OrchestratorV2, {
streamDomainEvents: Stream.empty,
} as unknown as Orchestrator.OrchestratorV2["Service"]),
Effect.provideService(SqlClient.SqlClient, (() =>
Effect.succeed([])) as unknown as SqlClient.SqlClient),
Effect.provideService(SqlClient.SqlClient, ((strings: TemplateStringsArray) =>
Effect.sync(() =>
openSessionCwd !== null && strings.join("").includes("projection_provider_sessions")
? [
{
payload_json: JSON.stringify({
id: "session-1",
driver: "codex",
providerInstanceId: "codex",
status: "ready",
cwd: openSessionCwd,
model: null,
capabilities: CodexProviderCapabilitiesV2,
createdAt: "2026-06-01T12:00:00.000Z",
updatedAt: "2026-06-01T12:00:00.000Z",
lastError: null,
}),
},
]
: [],
)) as unknown as SqlClient.SqlClient),
Effect.provideService(GitManager.GitManager, {
invalidateStatus: () => Effect.void,
} as unknown as GitManager.GitManager["Service"]),
Expand Down Expand Up @@ -313,6 +339,10 @@ const cleanupFixture = Effect.gen(function* () {
setThreads: (next: typeof threads) => {
threads = next;
},
setOpenSessionCwd: (cwd: string | null) => {
openSessionCwd = cwd;
},
fullStatusCalls: () => fullStatusCalls,
};
});
const cleanupTestLayer = GitVcsDriver.layer.pipe(
Expand Down Expand Up @@ -413,6 +443,73 @@ describe("storage cleanup reports and local file policies", () => {
}),
),
);
it.live("does not measure a worktree kept for an open provider session", () =>
runCleanupTest(
Effect.gen(function* () {
const fs = yield* FileSystem.FileSystem;
const walked: Array<string> = [];
const fixture = yield* cleanupFixture.pipe(
Effect.provideService(FileSystem.FileSystem, {
...fs,
readDirectory: (directory, options) => {
walked.push(directory);
return fs.readDirectory(directory, options);
},
}),
);
const root = yield* fs.realPath(fixture.worktree);
fixture.setOpenSessionCwd(fixture.worktree);
const kept = yield* fixture.service.runNow;
expect(kept.entries[0]).toMatchObject({
outcome: "kept",
reason: "Provider session is still open",
bytes: null,
});
expect(walked.filter((directory) => directory.startsWith(root))).toEqual([]);
fixture.setOpenSessionCwd(null);
const removed = yield* fixture.service.runNow;
expect(removed.entries[0]).toMatchObject({ outcome: "removed" });
expect(removed.entries[0]?.bytes).toBeGreaterThan(0);
expect(walked).toContain(root);
}),
),
);
it.live("compares the checked-out branch without a full status", () =>
runCleanupTest(
Effect.gen(function* () {
const { service, fs, worktree, command, setThreads, fullStatusCalls } =
yield* cleanupFixture;
const changed = { outcome: "kept", reason: "Repository or branch changed" };
yield* command(worktree, ["checkout", "--detach"]);
expect((yield* service.runNow).entries[0]).toMatchObject(changed);
// A branch that has no commit yet is still a different branch.
yield* command(worktree, ["switch", "--orphan", "other"]);
expect((yield* service.runNow).entries[0]).toMatchObject(changed);
// Status output reserves a leading "(" for a detached HEAD.
yield* command(worktree, ["switch", "-c", "(feature)", "feature"]);
setThreads([shell({ branch: "(feature)", worktreePath: worktree })]);
expect((yield* service.runNow).entries[0]).toMatchObject(changed);
setThreads([shell({ branch: "feature", worktreePath: worktree })]);
// Tags named after the branch or HEAD make Git's short names ambiguous.
yield* command(worktree, ["tag", "feature", "feature"]);
// Newer Git refuses to create a tag named HEAD, but still reads one.
yield* command(worktree, ["update-ref", "refs/tags/HEAD", "refs/heads/feature"]);
yield* command(worktree, ["switch", "feature"]);
const index = (yield* command(worktree, ["rev-parse", "--git-path", "index"])).stdout;
yield* fs.writeFileString(`${index.trim()}.lock`, "");
const locked = (yield* service.runNow).entries[0];
expect(locked).toMatchObject({ outcome: "failed" });
expect(locked?.reason).toContain("Git index is locked");
yield* fs.remove(`${index.trim()}.lock`);
const gitFile = yield* fs.readFileString(`${worktree}/.git`);
yield* fs.writeFileString(`${worktree}/.git`, "gitdir: /missing-git-directory\n");
expect((yield* service.runNow).entries[0]).toMatchObject(changed);
yield* fs.writeFileString(`${worktree}/.git`, gitFile);
expect((yield* service.runNow).entries[0]).toMatchObject({ outcome: "removed" });
expect(fullStatusCalls()).toBe(0);
}),
),
);
it("decodes reports from servers without size fields", () => {
const report = decodeCleanupReport({
trigger: "manual",
Expand Down
163 changes: 114 additions & 49 deletions apps/server/src/storageCleanup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,63 @@ const make = Effect.gen(function* () {
return false;
});

// Whether cwd stopped being a checkout of this branch. Two small Git reads,
// since every worktree in a sweep is asked; statusDetailsLocal also diffs and
// counts.
const branchChanged = Effect.fn("StorageCleanup.branchChanged")(function* (
cwd: string,
branch: string | null,
) {
const failure = (detail: string) =>
new GitCommandError({
operation: "StorageCleanup.branchChanged",
command: "git",
cwd,
detail,
});
// Null when cwd is not a repository, or is gone. Git's own messages are
// matched, so they must not be translated.
const read = Effect.fnUntraced(function* (args: string[], allowedExitCode?: number) {
const result = yield* git
.execute({
operation: "StorageCleanup.branchChanged",
cwd,
args,
env: { LC_ALL: "C" },
allowNonZeroExit: true,
})
.pipe(
Effect.catch((error) =>
Effect.flatMap(fs.exists(cwd), (exists) =>
exists ? Effect.fail(error) : Effect.succeed(null),
),
),
);
if (result === null) return null;
if (result.exitCode === 0 || result.exitCode === allowedExitCode) return result;
if (result.stderr.toLowerCase().includes("not a git repository")) return null;
return yield* failure(
result.stderr.trim().split(/\r?\n/)[0]?.slice(0, 1000) || "command failed",
);
});
const index = yield* read(["rev-parse", "--git-path", "index"]);
if (index === null) return true;
// Another Git process is writing to this checkout.
if (yield* fs.exists(`${path.resolve(cwd, index.stdout.trim())}.lock`))
return yield* failure(
"Git index is locked. Status will resume when the index lock is removed.",
);
// Exits 1 for a detached HEAD. Reads the full ref name, which is never
// shortened to "heads/<branch>" when a tag shares the branch name.
const head = yield* read(["symbolic-ref", "--quiet", "HEAD"], 1);
if (head === null) return true;
const ref = head.exitCode === 0 ? head.stdout.trim() : "";
const name = ref.startsWith("refs/heads/") ? ref.slice("refs/heads/".length).trim() : null;
// Status output writes a detached HEAD as "(detached)", so a branch named
// with a leading "(" has always been read as one.
return (name?.startsWith("(") ? null : name) !== branch;
});

const localChangesReason = Effect.fn("StorageCleanup.localChangesReason")(function* (
cwd: string,
rules: WorktreeCleanupRules,
Expand Down Expand Up @@ -415,8 +472,7 @@ const make = Effect.gen(function* () {
// A linked worktree has a .git file. Never remove a main checkout.
if ((yield* fs.stat(path.join(worktreePath, ".git"))).type !== "File")
return keep("not a linked worktree");
const status = yield* git.statusDetailsLocal(worktreePath);
if (!status.isRepo || status.branch !== thread.branch)
if (yield* branchChanged(worktreePath, thread.branch))
return keep("repository or branch changed");
const changes = yield* localChangesReason(worktreePath, settings);
if (changes !== null) return keep(changes);
Expand Down Expand Up @@ -480,55 +536,64 @@ const make = Effect.gen(function* () {
? "not merged"
: "has commits beyond the default branch",
);
const bytes = yield* measureWorktree(worktreePath);
// Re-read after Git/host calls and size measurement so a queued turn,
// resumed session or new thread sharing this path cancels the removal.
const latestSnapshot = yield* readThreads();
if (yield* containsProjectRoot(worktreePath, [project, ...latestSnapshot.projects]))
return keep("contains a project checkout");
const latest = latestSnapshot.threads.filter(
(entry) =>
entry.worktreePath !== null && path.resolve(entry.worktreePath) === worktreePath,
);
if (hasTerminal(worktreePath)) return keep("open terminal");
if (deleted) {
if (
latest.length > 0 ||
!resolveWorktreeCleanup(yield* settingsService.getSettings, thread.projectId)
.worktreeOnDelete
// Threads, queued deletion work and provider sessions as they are now, so a
// queued turn, resumed session or new thread sharing this path cancels
// the removal.
const blockedSinceCheck = Effect.fnUntraced(function* () {
const latestSnapshot = yield* readThreads();
if (yield* containsProjectRoot(worktreePath, [project, ...latestSnapshot.projects]))
return "contains a project checkout";
const latest = latestSnapshot.threads.filter(
(entry) =>
entry.worktreePath !== null && path.resolve(entry.worktreePath) === worktreePath,
);
if (hasTerminal(worktreePath)) return "open terminal";
if (deleted) {
if (
latest.length > 0 ||
!resolveWorktreeCleanup(yield* settingsService.getSettings, thread.projectId)
.worktreeOnDelete
)
return "thread or cleanup settings changed since check";
// V2 deletion queues durable cleanup. Do not remove its checkout until
// every effect has finished successfully or was explicitly cancelled.
const pendingCleanup = yield* sql`
SELECT 1 FROM orchestration_v2_effect_outbox
WHERE thread_id = ${thread.id} AND status NOT IN ('succeeded', 'cancelled') LIMIT 1
`;
if (pendingCleanup.length > 0) return "thread deletion is still pending";
} else if (
latest.length !== 1 ||
latest[0]!.id !== thread.id ||
!storageCleanupThreadIdle(latest[0]!, now) ||
storageCleanupActivityAt(latest[0]!) !== storageCleanupActivityAt(thread)
)
return keep("thread or cleanup settings changed since check");
// V2 deletion queues durable cleanup. Do not remove its checkout until
// every effect has finished successfully or was explicitly cancelled.
const pendingCleanup = yield* sql`
SELECT 1 FROM orchestration_v2_effect_outbox
WHERE thread_id = ${thread.id} AND status NOT IN ('succeeded', 'cancelled') LIMIT 1
return "thread activity or shared worktree changed since check";
// Sessions can outlive their run and can be shared across app threads.
const sessionRows = yield* sql<{ payload_json: string }>`
SELECT payload_json FROM orchestration_v2_projection_provider_sessions
WHERE status != 'stopped'
`;
if (pendingCleanup.length > 0) return keep("thread deletion is still pending");
} else if (
latest.length !== 1 ||
latest[0]!.id !== thread.id ||
!storageCleanupThreadIdle(latest[0]!, now) ||
storageCleanupActivityAt(latest[0]!) !== storageCleanupActivityAt(thread)
)
return keep("thread activity or shared worktree changed since check");
// Sessions can outlive their run and can be shared across app threads.
const sessionRows = yield* sql<{ payload_json: string }>`
SELECT payload_json FROM orchestration_v2_projection_provider_sessions
WHERE status != 'stopped'
`;
const sessions = yield* Effect.forEach(sessionRows, (row) =>
decodeCleanupSession(row.payload_json),
);
if (
sessions.some((session) => {
const cwd = path.resolve(session.cwd);
return cwd === worktreePath || inside(worktreePath, cwd);
})
)
return keep("provider session is still open");
const finalStatus = yield* git.statusDetailsLocal(worktreePath);
if (!finalStatus.isRepo || finalStatus.branch !== thread.branch)
const sessions = yield* Effect.forEach(sessionRows, (row) =>
decodeCleanupSession(row.payload_json),
);
if (
sessions.some((session) => {
const cwd = path.resolve(session.cwd);
return cwd === worktreePath || inside(worktreePath, cwd);
})
)
return "provider session is still open";
return null;
});
// Most cancelled removals are caught here, before the worktree is walked.
const blocked = yield* blockedSinceCheck();
if (blocked !== null) return keep(blocked);
const bytes = yield* measureWorktree(worktreePath);
// The walk can take a while; ask again for anything that started during it.
const blockedWhileMeasuring = yield* blockedSinceCheck();
if (blockedWhileMeasuring !== null) return keep(blockedWhileMeasuring);
if (yield* branchChanged(worktreePath, thread.branch))
return keep("repository or branch changed since check");
if (
(yield* git.resolveCommit({ cwd: worktreePath, revision: "HEAD" })).commitSha !==
Expand Down
Loading