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

[tris203]

Problem

A storage cleanup sweep that removes nothing still costs about a minute on an install with a few dozen worktrees (#17814). Two things in cleanWorktrees account for roughly two thirds of it:

• git.statusDetailsLocal is called per worktree only to read isRepo and branch, but it runs git status, a numstat diff and an ahead count. localChangesReason runs the real dirty check straight after, and the heavy call is repeated before removal. In one measured 54.5 s sweep this was 21.4 s across 78 calls.
• measureWorktree walks the whole worktree before the checks that most often cancel a removal. A worktree kept as "Provider session is still open" is fully walked on every sweep and then kept. About 16 s of the same sweep, for three worktrees.

Expected: a sweep that removes nothing costs little more than a cheap branch read and one dirty check per worktree, and a worktree is not measured when it is about to be kept.

Change

One file, apps/server/src/storageCleanup.ts, plus its test.

• The two statusDetailsLocal calls are replaced by a local branchChanged helper that makes two small Git reads: rev-parse --git-path index and symbolic-ref --quiet HEAD. It keeps what the old call decided for cleanup: a non-repository, a missing directory or a detached HEAD is "Repository or branch changed", and a locked index is still a failure with the same message. No existing GitVcsDriver method fits; the only cheap branch read in the driver is internal and cached.
• The thread, pending-deletion and provider-session recheck now runs before measureWorktree as well as after it. Running it first is what stops the wasted walk. It still runs afterwards because turn start does not take the workspace lease, so that recheck is the only thing that cancels a removal when a thread starts during the walk; the existing test "keeps a worktree when its thread starts during size measurement" covers this and is unchanged. The cost is one extra thread read and one or two queries, only for a worktree that reaches measurement.

Not included, deliberately:

• Loading non-stopped provider sessions once per sweep and skipping those worktrees before any Git call. It would change the reported reason for a worktree that has both an open session and uncommitted changes, so open-session worktrees still pay their Git calls, just not the walk.
• The speed of the walk itself, hardlink accounting ([Bug]: Storage cleanup overstates recovered space for worktrees with hardlinked dependencies (pnpm) #17815), the default-branch fallback ([Bug]: Storage cleanup keeps every worktree in a project when origin/HEAD is unset ("Default branch is unavailable") #17807), and sweep frequency (fix(server): worktree cleanup no longer runs sweeps back to back #17480).

The kept or removed outcome and its reason are meant to be unchanged. One known difference: when Git itself fails for another reason (for example invalid config), the entry is still failed, but the detail is now Git's own first error line rather than "Git status failed.", since no status runs any more.

Scope and approval

Fixes #17814, which I reported with trace measurements from my own install. It has not been triaged by a maintainer yet, so this PR does not have that prerequisite and I understand it may wait on it. It is a performance fix for one problem (the cost of a sweep that removes nothing), with no intended behavior change.

Verification

Established from server.trace.ndjson spans on a Linux desktop host (about 160 worktrees, 7 projects); the table and span order are in #17814.

Focused checks on Linux, on this branch:

Command	Result
vp test run apps/server/src/storageCleanup.test.ts	56 passed on Git 2.43 and on Git 2.55
same file with storageCleanup.ts restored from main	the two new tests fail, the other 54 pass
vp lint on the two files	clean
vp run --filter t3 typecheck	exit 0

New tests:

• "does not measure a worktree kept for an open provider session": no directory under the worktree is read while a session is open; once it is gone the worktree is removed with a size. On main this fails because the walk happens.
• "compares the checked-out branch without a full status": detached HEAD, an unborn orphan branch, a branch named (feature), and a broken .git file are all kept as "Repository or branch changed"; a locked index fails with "Git index is locked"; tags named feature and HEAD do not stop the removal. It then asserts statusDetailsLocal was never called. On main every outcome above is the same and only that last assertion fails (7 calls), which is the evidence that the cheap read decides the same way.

Git processes spawned by one sweep of the single-worktree test fixture, counted with a logging git shim on PATH, fixture setup subtracted:

Sweep	main	This branch
Worktree kept for uncommitted changes	14	3
Worktree removed	26	9

Not checked:

• Windows, and Git versions other than 2.43 and 2.55.
• A real sweep after the change: I have not re-measured wall-clock time on the install from the issue, so the time saved there is inferred from the spans, not observed.
• Two paths have no test of their own: a checkout deleted between the checks, and a branch whose name starts with a non-breaking space (the old parser trimmed it; the new read does too).

Claude Opus 5.5 in Claude Code, running in T3 Code. Reviewed in four rounds by GPT-6.1-Sol in Codex.

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR materially changes the existing worktree cleanup path by introducing custom Git state detection and reordering safety checks before destructive removal. Although the change is localized and includes focused tests, its production impact and deletion-related control flow merit human review.

Notes:

• All code in this push has already been reviewed. Approvability was decided on eligibility alone.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: fc35a59b-fb36-46ab-8c68-5158cf8148fa

---

📥 Commits

Reviewing files that changed from the base of the PR and between 4f63275 and 74ad9ee.

---

📒 Files selected for processing (2)

• apps/server/src/storageCleanup.test.ts
• apps/server/src/storageCleanup.ts

---

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

---

📝 Walkthrough

Walkthrough

Storage cleanup now checks Git branch state with targeted reads instead of full status calls. It checks removal blockers before and after measuring worktree size, including active provider sessions. Tests cover these checks and several repository states.

Changes

Storage cleanup

Layer / File(s)	Summary
Branch validation apps/server/src/storageCleanup.ts, apps/server/src/storageCleanup.test.ts	Cleanup uses targeted Git reads to detect branch changes. Tests cover detached, unborn, ambiguous, and unavailable repository states, as well as index locks. They also verify that branch checks do not call full Git status.
Removal eligibility checks apps/server/src/storageCleanup.ts, apps/server/src/storageCleanup.test.ts	Cleanup checks thread, project, terminal, deletion, and provider-session state before and after measuring the worktree. Tests verify that an open provider session prevents measurement and removal.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge

---

Merge Risk | ⚪ Minimal · up to 74ad9

Merge Risk: ⚪ Minimal · up to 74ad9

Cleanup can avoid measuring worktrees that are already blocked while retaining the inspected removal safeguards. No issue identified here prevents merging after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 74ad9

The optimization preserves existing deletion safeguards and adds an earlier activity check. No expanded access or deletion authority was identified. Work starting concurrently with final removal remains an existing lifecycle uncertainty, rather than a demonstrated regression.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed destructive outcome remains removal of application-managed linked worktrees under the existing cleanup authority. The PR does not establish new caller reachability, cross-service authority, or broader filesystem ownership.

Trust Boundaries and Controls

• observed — Deletion still requires canonical non-symlink path identity, managed-root containment, exclusion of project checkouts, and a linked-worktree Git file. After measurement, cleanup repeats blocker and branch validation and retains final commit, dirty-state, and policy checks before the existing Git removal operation.

Resilience and Maintainability Implications

• observed — Session attachment events update projections inside the event-write transaction before successful publication. Nevertheless, session runtime creation precedes attachment persistence, and provider turn startup does not share cleanup's workspace lease. The observational check-to-removal window already existed in the base; this is an unchanged lifecycle limitation, not an active PR concern.

---

Pre-merge checks | 3 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check		Issue [#17814] requires checks that need no Git and no filesystem walk, including open provider-session checks, to run before Git work. The change moves the recheck before measureWorktree, and it re…	Load or cache non-stopped provider sessions once per sweep. Check the cached sessions, thread state, and age before branchChanged and localChangesReason. Retain the final recheck before removal.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely summarizes the main performance change: avoiding full Git status and size walks for worktrees that cleanup keeps.
Description check		The description includes all required sections. It clearly explains the problem, implementation, scope, verification results, limitations, and agent details. It also explicitly notes that maintainer t…
Out of Scope Changes check		The changes are limited to apps/server/src/storageCleanup.ts and its focused test. The new Git reads, the pre-measurement recheck, and the test instrumentation directly support issue [#17814]. No un…

---

Full details: Linked Issues check

Explanation

Issue [#17814] requires checks that need no Git and no filesystem walk, including open provider-session checks, to run before Git work. The change moves the recheck before measureWorktree, and it replaces both statusDetailsLocal calls with branchChanged, as covered by the new tests. However, the provider-session query remains inside blockedSinceCheck, after branchChanged and localChangesReason. A worktree kept only because its provider session is open still performs Git reads and a dirty check. The PR description explicitly excludes the sweep-wide session preload, but that work is part of the linked issue's stated expected behavior.

---

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}