fix(server): keep worktrees containing nested repositories #24061 · pingdotgg/t3code

MmacroscopeappBotapproved these changes6d ago

Approvability

Verdict: Approved at 1b2a549

Macroscope's review found this PR approvable — This focused server bug fix prevents existing cleanup from deleting nested repositories and preserves worktrees when filesystem inspection is incomplete. The accompanying integration tests cover nested repositories, pnpm stores, submodules, and failed inspection paths.

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

CcoderabbitaiBot6d ago

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: main
File: .coderabbit.config.ts
Commit: d7a8d6f
Layers: Repository TS config
Review profile: CHILL
Plan: Advanced
Run ID: fb60e674-7e5f-4894-ae5a-e2fa16bd218c

📥 Commits

Reviewing files that changed from the base of the PR and between d7a8d6f32001444e013aa07c8a468ce3e987f9cf and 1b2a549c1daa2d9b45314a1f15b7175530c9d309.

📒 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; 4 remain after this review.

📝 Walkthrough

Storage cleanup now inspects worktrees for nested Git repositories and incomplete scans. It retains a worktree when inspection finds a nested repository or cannot determine its contents.

Changes: Storage cleanup

Layer / File(s)	Summary
Repository-aware worktree measurement `apps/server/src/storageCleanup.ts`	`measureWorktree` can inspect nested repositories and registered submodules. It returns a structured result with byte measurements and an inspection reason. It reports incomplete inspection for scan limits, timeouts, truncated submodule output, and other scan failures.
Cleanup decisions and validation `apps/server/src/storageCleanup.ts`, `apps/server/src/storageCleanup.test.ts`	Cleanup retains worktrees when inspection reports a reason, and orphan-worktree size caching reads the result’s `bytes` field. Tests cover nested repositories in ignored directories, the pnpm virtual store, and submodules, plus unreadable worktrees.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: t3dotgg, juliusmarminge, maria-rcks

Merge Risk: ⚪ Minimal · up to 1b2a5

Cleanup now keeps worktrees that contain nested Git repositories or that cannot be fully inspected, which addresses the reported data-loss case. No concrete merge-blocking issue remains after review. Windows and Linux were not exercised by the author.

Pre-merge checks: ^Passed 4

Check name	Status	Explanation
Title check	Passed	The title clearly and concisely describes the main change: preventing cleanup from removing worktrees that contain nested repositories.
Description check	Passed	The description covers the problem, implementation, scope rationale, verification results, platform coverage, limitations, and agent attribution. It also references issue `#23866` and explains why this …
Linked Issues check	Passed	Issue `#23866` requires cleanup to keep an outer worktree when it contains an ignored nested Git repository or linked worktree. The diff adds repository inspection before removal and before destructive …
Out of Scope Changes check	Passed	The changes stay within the storage-cleanup data-loss guard for issue `#23866`. The structured measurement result, pnpm virtual-store traversal, submodule handling, inspection limits, timeout handling, …

Full details: Description check

Explanation

The description covers the problem, implementation, scope rationale, verification results, platform coverage, limitations, and agent attribution. It also references issue #23866 and explains why this focused bug fix does not require broader approval.

Full details: Linked Issues check

Explanation

Issue #23866 requires cleanup to keep an outer worktree when it contains an ignored nested Git repository or linked worktree. The diff adds repository inspection before removal and before destructive cleanup. It detects nested .git entries, keeps the worktree with Contains a nested Git repository, and keeps it when traversal fails, times out, or exceeds limits. The added tests cover nested clones and linked worktrees under both cleanup policies, preserve uncommitted files, and verify the clean outer status case. The existing removal path therefore covers idle and deletion-triggered cleanup calls that reach this pre-removal check.

Full details: Out of Scope Changes check

Explanation

The changes stay within the storage-cleanup data-loss guard for issue #23866. The structured measurement result, pnpm virtual-store traversal, submodule handling, inspection limits, timeout handling, and updated failure test support safe pre-removal inspection or preserve existing accounting behavior. The added tests directly verify these safety cases. No unrelated production feature or unrelated test change is evident.

✨ 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.}

1	Before: outer git status --porcelain => empty
2	cleanup runNow => removed (both keep policies, clone and linked worktree)
3	After: cleanup runNow => kept: Contains a nested Git repository
4	nested/uncommitted.txt => original contents preserved
5	nested git rev-parse --show-toplevel => original nested checkout
6	Read failure => kept: Worktree could not be fully inspected

fix(server): keep worktrees containing nested repositories#24061

Problem

Change

Verification

Checks

Approvability