| author |  Tristan Knight <admin@snappeh.com> 2026-10-11 02:45:03+01:00 |
| committer |  GitHub <noreply@github.com> 2026-10-10 18:45:03-07:00 |
| log | 4258c8f14ddf6affe67c4e1d5d85ec834e6c4346 |
| tree | 8002e8b2f425e712f4d9bae86a36c67a779dbd03 |
| parent | 1c7b267424a18d257c81d0e9a91f403fb20d2458 |
| signature | Signed by PGP key B5690EEEBB952194 |
perf(server): storage cleanup no longer runs a full status or size walk for worktrees it keeps (#17911)
2 files changed, 213 insertions(+), 51 deletions(-)
apps
apps/server/src/storageCleanup.test.ts+99-2
| ... | @@ -24,6 +24,7 @@ import * as ProjectionStore from "./orchestration-v2/ProjectionStore.ts"; | ... | @@ -24,6 +24,7 @@ import * as ProjectionStore from "./orchestration-v2/ProjectionStore.ts"; |
| 24 | import * as ProjectStore from "./orchestration-v2/ProjectStore.ts"; | 24 | import * as ProjectStore from "./orchestration-v2/ProjectStore.ts"; |
| 25 | import * as Orchestrator from "./orchestration-v2/Orchestrator.ts"; | 25 | import * as Orchestrator from "./orchestration-v2/Orchestrator.ts"; |
| 26 | import * as TerminalManager from "./terminal/Manager.ts"; | 26 | import * as TerminalManager from "./terminal/Manager.ts"; |
| 27 | import { CodexProviderCapabilitiesV2 } from "./orchestration-v2/Adapters/CodexAdapterV2.ts"; | ||
| 27 | import { ServerActivation } from "./serverActivation.ts"; | 28 | import { ServerActivation } from "./serverActivation.ts"; |
| 28 | import { describe, expect, it } from "@effect/vitest"; | 29 | import { describe, expect, it } from "@effect/vitest"; |
| 29 | import { | 30 | import { |
| ... | @@ -248,6 +249,8 @@ const cleanupFixture = Effect.gen(function* () { | ... | @@ -248,6 +249,8 @@ const cleanupFixture = Effect.gen(function* () { |
| 248 | }; | 249 | }; |
| 249 | let threads = [shell({ branch: "feature", worktreePath: worktree })]; | 250 | let threads = [shell({ branch: "feature", worktreePath: worktree })]; |
| 250 | let editBeforeRemoval = false; | 251 | let editBeforeRemoval = false; |
| 252 | let openSessionCwd: string | null = null; | ||
| 253 | let fullStatusCalls = 0; | ||
| 251 | const context = yield* Layer.build(StorageCleanup.layer).pipe( | 254 | const context = yield* Layer.build(StorageCleanup.layer).pipe( |
| 252 | Effect.provideService(GitVcsDriver.GitVcsDriver, { | 255 | Effect.provideService(GitVcsDriver.GitVcsDriver, { |
| 253 | ...git, | 256 | ...git, |
| ... | @@ -257,6 +260,10 @@ const cleanupFixture = Effect.gen(function* () { | ... | @@ -257,6 +260,10 @@ const cleanupFixture = Effect.gen(function* () { |
| 257 | yield* fs.writeFileString(`${worktree}/tracked.txt`, "late edit\n"); | 260 | yield* fs.writeFileString(`${worktree}/tracked.txt`, "late edit\n"); |
| 258 | return yield* git.execute(input); | 261 | return yield* git.execute(input); |
| 259 | }), | 262 | }), |
| 263 | statusDetailsLocal: (...args) => { | ||
| 264 | fullStatusCalls++; | ||
| 265 | return git.statusDetailsLocal(...args); | ||
| 266 | }, | ||
| 260 | }), | 267 | }), |
| 261 | Effect.provideService(Settings.ServerSettingsService, { | 268 | Effect.provideService(Settings.ServerSettingsService, { |
| 262 | getSettings: Effect.sync(() => settings), | 269 | getSettings: Effect.sync(() => settings), |
| ... | @@ -272,8 +279,27 @@ const cleanupFixture = Effect.gen(function* () { | ... | @@ -272,8 +279,27 @@ const cleanupFixture = Effect.gen(function* () { |
| 272 | Effect.provideService(Orchestrator.OrchestratorV2, { | 279 | Effect.provideService(Orchestrator.OrchestratorV2, { |
| 273 | streamDomainEvents: Stream.empty, | 280 | streamDomainEvents: Stream.empty, |
| 274 | } as unknown as Orchestrator.OrchestratorV2["Service"]), | 281 | } as unknown as Orchestrator.OrchestratorV2["Service"]), |
| 275 | Effect.provideService(SqlClient.SqlClient, (() => | 282 | Effect.provideService(SqlClient.SqlClient, ((strings: TemplateStringsArray) => |
| 276 | Effect.succeed([])) as unknown as SqlClient.SqlClient), | 283 | Effect.sync(() => |
| 284 | openSessionCwd !== null && strings.join("").includes("projection_provider_sessions") | ||
| 285 | ? [ | ||
| 286 | { | ||
| 287 | payload_json: JSON.stringify({ | ||
| 288 | id: "session-1", | ||
| 289 | driver: "codex", | ||
| 290 | providerInstanceId: "codex", | ||
| 291 | status: "ready", | ||
| 292 | cwd: openSessionCwd, | ||
| 293 | model: null, | ||
| 294 | capabilities: CodexProviderCapabilitiesV2, | ||
| 295 | createdAt: "2026-06-01T12:00:00.000Z", | ||
| 296 | updatedAt: "2026-06-01T12:00:00.000Z", | ||
| 297 | lastError: null, | ||
| 298 | }), | ||
| 299 | }, | ||
| 300 | ] | ||
| 301 | : [], | ||
| 302 | )) as unknown as SqlClient.SqlClient), | ||
| 277 | Effect.provideService(GitManager.GitManager, { | 303 | Effect.provideService(GitManager.GitManager, { |
| 278 | invalidateStatus: () => Effect.void, | 304 | invalidateStatus: () => Effect.void, |
| 279 | } as unknown as GitManager.GitManager["Service"]), | 305 | } as unknown as GitManager.GitManager["Service"]), |
| ... | @@ -313,6 +339,10 @@ const cleanupFixture = Effect.gen(function* () { | ... | @@ -313,6 +339,10 @@ const cleanupFixture = Effect.gen(function* () { |
| 313 | setThreads: (next: typeof threads) => { | 339 | setThreads: (next: typeof threads) => { |
| 314 | threads = next; | 340 | threads = next; |
| 315 | }, | 341 | }, |
| 342 | setOpenSessionCwd: (cwd: string | null) => { | ||
| 343 | openSessionCwd = cwd; | ||
| 344 | }, | ||
| 345 | fullStatusCalls: () => fullStatusCalls, | ||
| 316 | }; | 346 | }; |
| 317 | }); | 347 | }); |
| 318 | const cleanupTestLayer = GitVcsDriver.layer.pipe( | 348 | const cleanupTestLayer = GitVcsDriver.layer.pipe( |
| ... | @@ -413,6 +443,73 @@ describe("storage cleanup reports and local file policies", () => { | ... | @@ -413,6 +443,73 @@ describe("storage cleanup reports and local file policies", () => { |
| 413 | }), | 443 | }), |
| 414 | ), | 444 | ), |
| 415 | ); | 445 | ); |
| 446 | it.live("does not measure a worktree kept for an open provider session", () => | ||
| 447 | runCleanupTest( | ||
| 448 | Effect.gen(function* () { | ||
| 449 | const fs = yield* FileSystem.FileSystem; | ||
| 450 | const walked: Array<string> = []; | ||
| 451 | const fixture = yield* cleanupFixture.pipe( | ||
| 452 | Effect.provideService(FileSystem.FileSystem, { | ||
| 453 | ...fs, | ||
| 454 | readDirectory: (directory, options) => { | ||
| 455 | walked.push(directory); | ||
| 456 | return fs.readDirectory(directory, options); | ||
| 457 | }, | ||
| 458 | }), | ||
| 459 | ); | ||
| 460 | const root = yield* fs.realPath(fixture.worktree); | ||
| 461 | fixture.setOpenSessionCwd(fixture.worktree); | ||
| 462 | const kept = yield* fixture.service.runNow; | ||
| 463 | expect(kept.entries[0]).toMatchObject({ | ||
| 464 | outcome: "kept", | ||
| 465 | reason: "Provider session is still open", | ||
| 466 | bytes: null, | ||
| 467 | }); | ||
| 468 | expect(walked.filter((directory) => directory.startsWith(root))).toEqual([]); | ||
| 469 | fixture.setOpenSessionCwd(null); | ||
| 470 | const removed = yield* fixture.service.runNow; | ||
| 471 | expect(removed.entries[0]).toMatchObject({ outcome: "removed" }); | ||
| 472 | expect(removed.entries[0]?.bytes).toBeGreaterThan(0); | ||
| 473 | expect(walked).toContain(root); | ||
| 474 | }), | ||
| 475 | ), | ||
| 476 | ); | ||
| 477 | it.live("compares the checked-out branch without a full status", () => | ||
| 478 | runCleanupTest( | ||
| 479 | Effect.gen(function* () { | ||
| 480 | const { service, fs, worktree, command, setThreads, fullStatusCalls } = | ||
| 481 | yield* cleanupFixture; | ||
| 482 | const changed = { outcome: "kept", reason: "Repository or branch changed" }; | ||
| 483 | yield* command(worktree, ["checkout", "--detach"]); | ||
| 484 | expect((yield* service.runNow).entries[0]).toMatchObject(changed); | ||
| 485 | // A branch that has no commit yet is still a different branch. | ||
| 486 | yield* command(worktree, ["switch", "--orphan", "other"]); | ||
| 487 | expect((yield* service.runNow).entries[0]).toMatchObject(changed); | ||
| 488 | // Status output reserves a leading "(" for a detached HEAD. | ||
| 489 | yield* command(worktree, ["switch", "-c", "(feature)", "feature"]); | ||
| 490 | setThreads([shell({ branch: "(feature)", worktreePath: worktree })]); | ||
| 491 | expect((yield* service.runNow).entries[0]).toMatchObject(changed); | ||
| 492 | setThreads([shell({ branch: "feature", worktreePath: worktree })]); | ||
| 493 | // Tags named after the branch or HEAD make Git's short names ambiguous. | ||
| 494 | yield* command(worktree, ["tag", "feature", "feature"]); | ||
| 495 | // Newer Git refuses to create a tag named HEAD, but still reads one. | ||
| 496 | yield* command(worktree, ["update-ref", "refs/tags/HEAD", "refs/heads/feature"]); | ||
| 497 | yield* command(worktree, ["switch", "feature"]); | ||
| 498 | const index = (yield* command(worktree, ["rev-parse", "--git-path", "index"])).stdout; | ||
| 499 | yield* fs.writeFileString(`${index.trim()}.lock`, ""); | ||
| 500 | const locked = (yield* service.runNow).entries[0]; | ||
| 501 | expect(locked).toMatchObject({ outcome: "failed" }); | ||
| 502 | expect(locked?.reason).toContain("Git index is locked"); | ||
| 503 | yield* fs.remove(`${index.trim()}.lock`); | ||
| 504 | const gitFile = yield* fs.readFileString(`${worktree}/.git`); | ||
| 505 | yield* fs.writeFileString(`${worktree}/.git`, "gitdir: /missing-git-directory\n"); | ||
| 506 | expect((yield* service.runNow).entries[0]).toMatchObject(changed); | ||
| 507 | yield* fs.writeFileString(`${worktree}/.git`, gitFile); | ||
| 508 | expect((yield* service.runNow).entries[0]).toMatchObject({ outcome: "removed" }); | ||
| 509 | expect(fullStatusCalls()).toBe(0); | ||
| 510 | }), | ||
| 511 | ), | ||
| 512 | ); | ||
| 416 | it("decodes reports from servers without size fields", () => { | 513 | it("decodes reports from servers without size fields", () => { |
| 417 | const report = decodeCleanupReport({ | 514 | const report = decodeCleanupReport({ |
| 418 | trigger: "manual", | 515 | trigger: "manual", |
apps/server/src/storageCleanup.ts+114-49
| ... | @@ -271,6 +271,63 @@ const make = Effect.gen(function* () { | ... | @@ -271,6 +271,63 @@ const make = Effect.gen(function* () { |
| 271 | return false; | 271 | return false; |
| 272 | }); | 272 | }); |
| 273 | 273 | ||
| 274 | // Whether cwd stopped being a checkout of this branch. Two small Git reads, | ||
| 275 | // since every worktree in a sweep is asked; statusDetailsLocal also diffs and | ||
| 276 | // counts. | ||
| 277 | const branchChanged = Effect.fn("StorageCleanup.branchChanged")(function* ( | ||
| 278 | cwd: string, | ||
| 279 | branch: string | null, | ||
| 280 | ) { | ||
| 281 | const failure = (detail: string) => | ||
| 282 | new GitCommandError({ | ||
| 283 | operation: "StorageCleanup.branchChanged", | ||
| 284 | command: "git", | ||
| 285 | cwd, | ||
| 286 | detail, | ||
| 287 | }); | ||
| 288 | // Null when cwd is not a repository, or is gone. Git's own messages are | ||
| 289 | // matched, so they must not be translated. | ||
| 290 | const read = Effect.fnUntraced(function* (args: string[], allowedExitCode?: number) { | ||
| 291 | const result = yield* git | ||
| 292 | .execute({ | ||
| 293 | operation: "StorageCleanup.branchChanged", | ||
| 294 | cwd, | ||
| 295 | args, | ||
| 296 | env: { LC_ALL: "C" }, | ||
| 297 | allowNonZeroExit: true, | ||
| 298 | }) | ||
| 299 | .pipe( | ||
| 300 | Effect.catch((error) => | ||
| 301 | Effect.flatMap(fs.exists(cwd), (exists) => | ||
| 302 | exists ? Effect.fail(error) : Effect.succeed(null), | ||
| 303 | ), | ||
| 304 | ), | ||
| 305 | ); | ||
| 306 | if (result === null) return null; | ||
| 307 | if (result.exitCode === 0 || result.exitCode === allowedExitCode) return result; | ||
| 308 | if (result.stderr.toLowerCase().includes("not a git repository")) return null; | ||
| 309 | return yield* failure( | ||
| 310 | result.stderr.trim().split(/\r?\n/)[0]?.slice(0, 1000) || "command failed", | ||
| 311 | ); | ||
| 312 | }); | ||
| 313 | const index = yield* read(["rev-parse", "--git-path", "index"]); | ||
| 314 | if (index === null) return true; | ||
| 315 | // Another Git process is writing to this checkout. | ||
| 316 | if (yield* fs.exists(`${path.resolve(cwd, index.stdout.trim())}.lock`)) | ||
| 317 | return yield* failure( | ||
| 318 | "Git index is locked. Status will resume when the index lock is removed.", | ||
| 319 | ); | ||
| 320 | // Exits 1 for a detached HEAD. Reads the full ref name, which is never | ||
| 321 | // shortened to "heads/<branch>" when a tag shares the branch name. | ||
| 322 | const head = yield* read(["symbolic-ref", "--quiet", "HEAD"], 1); | ||
| 323 | if (head === null) return true; | ||
| 324 | const ref = head.exitCode === 0 ? head.stdout.trim() : ""; | ||
| 325 | const name = ref.startsWith("refs/heads/") ? ref.slice("refs/heads/".length).trim() : null; | ||
| 326 | // Status output writes a detached HEAD as "(detached)", so a branch named | ||
| 327 | // with a leading "(" has always been read as one. | ||
| 328 | return (name?.startsWith("(") ? null : name) !== branch; | ||
| 329 | }); | ||
| 330 | |||
| 274 | const localChangesReason = Effect.fn("StorageCleanup.localChangesReason")(function* ( | 331 | const localChangesReason = Effect.fn("StorageCleanup.localChangesReason")(function* ( |
| 275 | cwd: string, | 332 | cwd: string, |
| 276 | rules: WorktreeCleanupRules, | 333 | rules: WorktreeCleanupRules, |
| ... | @@ -415,8 +472,7 @@ const make = Effect.gen(function* () { | ... | @@ -415,8 +472,7 @@ const make = Effect.gen(function* () { |
| 415 | // A linked worktree has a .git file. Never remove a main checkout. | 472 | // A linked worktree has a .git file. Never remove a main checkout. |
| 416 | if ((yield* fs.stat(path.join(worktreePath, ".git"))).type !== "File") | 473 | if ((yield* fs.stat(path.join(worktreePath, ".git"))).type !== "File") |
| 417 | return keep("not a linked worktree"); | 474 | return keep("not a linked worktree"); |
| 418 | const status = yield* git.statusDetailsLocal(worktreePath); | 475 | if (yield* branchChanged(worktreePath, thread.branch)) |
| 419 | if (!status.isRepo || status.branch !== thread.branch) | ||
| 420 | return keep("repository or branch changed"); | 476 | return keep("repository or branch changed"); |
| 421 | const changes = yield* localChangesReason(worktreePath, settings); | 477 | const changes = yield* localChangesReason(worktreePath, settings); |
| 422 | if (changes !== null) return keep(changes); | 478 | if (changes !== null) return keep(changes); |
| ... | @@ -480,55 +536,64 @@ const make = Effect.gen(function* () { | ... | @@ -480,55 +536,64 @@ const make = Effect.gen(function* () { |
| 480 | ? "not merged" | 536 | ? "not merged" |
| 481 | : "has commits beyond the default branch", | 537 | : "has commits beyond the default branch", |
| 482 | ); | 538 | ); |
| 483 | const bytes = yield* measureWorktree(worktreePath); | 539 | // Threads, queued deletion work and provider sessions as they are now, so a |
| 484 | // Re-read after Git/host calls and size measurement so a queued turn, | 540 | // queued turn, resumed session or new thread sharing this path cancels |
| 485 | // resumed session or new thread sharing this path cancels the removal. | 541 | // the removal. |
| 486 | const latestSnapshot = yield* readThreads(); | 542 | const blockedSinceCheck = Effect.fnUntraced(function* () { |
| 487 | if (yield* containsProjectRoot(worktreePath, [project, ...latestSnapshot.projects])) | 543 | const latestSnapshot = yield* readThreads(); |
| 488 | return keep("contains a project checkout"); | 544 | if (yield* containsProjectRoot(worktreePath, [project, ...latestSnapshot.projects])) |
| 489 | const latest = latestSnapshot.threads.filter( | 545 | return "contains a project checkout"; |
| 490 | (entry) => | 546 | const latest = latestSnapshot.threads.filter( |
| 491 | entry.worktreePath !== null && path.resolve(entry.worktreePath) === worktreePath, | 547 | (entry) => |
| 492 | ); | 548 | entry.worktreePath !== null && path.resolve(entry.worktreePath) === worktreePath, |
| 493 | if (hasTerminal(worktreePath)) return keep("open terminal"); | 549 | ); |
| 494 | if (deleted) { | 550 | if (hasTerminal(worktreePath)) return "open terminal"; |
| 495 | if ( | 551 | if (deleted) { |
| 496 | latest.length > 0 || | 552 | if ( |
| 497 | !resolveWorktreeCleanup(yield* settingsService.getSettings, thread.projectId) | 553 | latest.length > 0 || |
| 498 | .worktreeOnDelete | 554 | !resolveWorktreeCleanup(yield* settingsService.getSettings, thread.projectId) |
| 555 | .worktreeOnDelete | ||
| 556 | ) | ||
| 557 | return "thread or cleanup settings changed since check"; | ||
| 558 | // V2 deletion queues durable cleanup. Do not remove its checkout until | ||
| 559 | // every effect has finished successfully or was explicitly cancelled. | ||
| 560 | const pendingCleanup = yield* sql` | ||
| 561 | SELECT 1 FROM orchestration_v2_effect_outbox | ||
| 562 | WHERE thread_id = ${thread.id} AND status NOT IN ('succeeded', 'cancelled') LIMIT 1 | ||
| 563 | `; | ||
| 564 | if (pendingCleanup.length > 0) return "thread deletion is still pending"; | ||
| 565 | } else if ( | ||
| 566 | latest.length !== 1 || | ||
| 567 | latest[0]!.id !== thread.id || | ||
| 568 | !storageCleanupThreadIdle(latest[0]!, now) || | ||
| 569 | storageCleanupActivityAt(latest[0]!) !== storageCleanupActivityAt(thread) | ||
| 499 | ) | 570 | ) |
| 500 | return keep("thread or cleanup settings changed since check"); | 571 | return "thread activity or shared worktree changed since check"; |
| 501 | // V2 deletion queues durable cleanup. Do not remove its checkout until | 572 | // Sessions can outlive their run and can be shared across app threads. |
| 502 | // every effect has finished successfully or was explicitly cancelled. | 573 | const sessionRows = yield* sql<{ payload_json: string }>` |
| 503 | const pendingCleanup = yield* sql` | 574 | SELECT payload_json FROM orchestration_v2_projection_provider_sessions |
| 504 | SELECT 1 FROM orchestration_v2_effect_outbox | 575 | WHERE status != 'stopped' |
| 505 | WHERE thread_id = ${thread.id} AND status NOT IN ('succeeded', 'cancelled') LIMIT 1 | ||
| 506 | `; | 576 | `; |
| 507 | if (pendingCleanup.length > 0) return keep("thread deletion is still pending"); | 577 | const sessions = yield* Effect.forEach(sessionRows, (row) => |
| 508 | } else if ( | 578 | decodeCleanupSession(row.payload_json), |
| 509 | latest.length !== 1 || | 579 | ); |
| 510 | latest[0]!.id !== thread.id || | 580 | if ( |
| 511 | !storageCleanupThreadIdle(latest[0]!, now) || | 581 | sessions.some((session) => { |
| 512 | storageCleanupActivityAt(latest[0]!) !== storageCleanupActivityAt(thread) | 582 | const cwd = path.resolve(session.cwd); |
| 513 | ) | 583 | return cwd === worktreePath || inside(worktreePath, cwd); |
| 514 | return keep("thread activity or shared worktree changed since check"); | 584 | }) |
| 515 | // Sessions can outlive their run and can be shared across app threads. | 585 | ) |
| 516 | const sessionRows = yield* sql<{ payload_json: string }>` | 586 | return "provider session is still open"; |
| 517 | SELECT payload_json FROM orchestration_v2_projection_provider_sessions | 587 | return null; |
| 518 | WHERE status != 'stopped' | 588 | }); |
| 519 | `; | 589 | // Most cancelled removals are caught here, before the worktree is walked. |
| 520 | const sessions = yield* Effect.forEach(sessionRows, (row) => | 590 | const blocked = yield* blockedSinceCheck(); |
| 521 | decodeCleanupSession(row.payload_json), | 591 | if (blocked !== null) return keep(blocked); |
| 522 | ); | 592 | const bytes = yield* measureWorktree(worktreePath); |
| 523 | if ( | 593 | // The walk can take a while; ask again for anything that started during it. |
| 524 | sessions.some((session) => { | 594 | const blockedWhileMeasuring = yield* blockedSinceCheck(); |
| 525 | const cwd = path.resolve(session.cwd); | 595 | if (blockedWhileMeasuring !== null) return keep(blockedWhileMeasuring); |
| 526 | return cwd === worktreePath || inside(worktreePath, cwd); | 596 | if (yield* branchChanged(worktreePath, thread.branch)) |
| 527 | }) | ||
| 528 | ) | ||
| 529 | return keep("provider session is still open"); | ||
| 530 | const finalStatus = yield* git.statusDetailsLocal(worktreePath); | ||
| 531 | if (!finalStatus.isRepo || finalStatus.branch !== thread.branch) | ||
| 532 | return keep("repository or branch changed since check"); | 597 | return keep("repository or branch changed since check"); |
| 533 | if ( | 598 | if ( |
| 534 | (yield* git.resolveCommit({ cwd: worktreePath, revision: "HEAD" })).commitSha !== | 599 | (yield* git.resolveCommit({ cwd: worktreePath, revision: "HEAD" })).commitSha !== |