[ScottN-PV]

Git refuses to remove eligible worktrees after their submodules have been initialized. This fixes #20355 by inspecting populated submodules and stored nested repositories, then using one --force only when Git requires it. Inspection failures, unpublished live refs, and policy-protected files retain the worktree. Indexed gitlinks also expose files added after deinitialization, which Git status omits.

Rebuilt from @ScottN-PV’s original contribution, preserving contributor history and merging main. Nine added cases were reduced to five in the existing suite: 146 → 142 total, with all 137 baseline cases unchanged. Four rebuilt cases fail against main. Review found and fixed file loss after deinitialization under both file-protecting policies, plus symlink-parent and index-limit false retention. Both reviewers found no confirmed code findings in round three.

Verification at bffca7f: 142/142 real-Git service tests, server typecheck, scoped lint and formatting passed on Linux/Git 2.43.0. Temporary service probes also removed clean nested worktrees through symlinked parents under both policies and with a 77,132-byte index. Nonvisual server behavior; screenshots do not apply.

Evidence: proof and limits, before-fix ignored-file loss and default-policy loss (141 passed, one failed each), final independent 142-pass run, three service probes.

Human safety approval required before merging. Inspection and forced removal are not atomic against external writers. History checks local remote-tracking refs; tag/SHA-only commits can conservatively retain clean worktrees. The default policy also conservatively retains ignored files inside deinitialized directories, apart from the existing node_modules directory exception. Bounded or unreadable metadata retains the worktree. Git 2.34.1, macOS, Windows, iOS, Android, Electron desktop and browser flows were not executed. Published-head build/typecheck, lint and all eight server shards passed. A duplicate native-fingerprint run was cancelled and superseded by a successful run; skipped jobs remain skipped. No new review findings or submitted human reviews. Human safety approval is still missing; no merge performed.

Tests: GPT-6-Astra; implementation: GPT-6.1-Sol; adversarial reviews: Claude Opus 5.5 and GPT-6-Sol. Codex and Claude Code harnesses, high reasoning effort, orchestrated through T3 Code.

[macroscopeapp]

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — The server cleanup path now recursively inspects submodule repositories and can force-remove eligible worktrees, materially changing destructive runtime behavior. Although the change includes broad safety tests, the new recursive logic and force-removal semantics merit human review.

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

[coderabbitai]

Warning

Review limit reached

Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.

Next included review available in 12 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5382f80d-f562-4d9a-b2a6-1681fefaa895

📥 Commits

Reviewing files that changed from the base of the PR and between cdaeeca and f9caddc.

📒 Files selected for processing: 2

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

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: 919eba7c-45b1-40a4-a396-651c5dc9ecfd

📥 Commits

Reviewing files that changed from the base of the PR and between 1b9d4ca and cdaeeca.

📒 Files selected for processing: 1

• apps/server/src/storageCleanup.ts

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

📝 Walkthrough

Storage cleanup now inspects submodules before removing eligible worktrees. It keeps worktrees with submodule changes, unpushed commits, or inspection problems. When checks pass and force is required, cleanup rechecks local changes before removal.

Changes: Submodule-aware cleanup

Layer / File(s)	Summary
Inspect submodule state apps/server/src/storageCleanup.ts, apps/server/src/storageCleanup.test.ts	Cleanup checks populated submodules and stored repositories for local changes and unpushed commits. It bounds inspection and keeps the worktree when inspection fails. Tests cover local edits, unpushed commits, hidden local branches, and malformed metadata.
Apply checks during worktree removal apps/server/src/storageCleanup.ts, apps/server/src/storageCleanup.test.ts	Cleanup runs the submodule check after untracked-file cleanup. If force is required, it rechecks local changes before removal. Tests cover clean populated or deinitialized nested submodules and a late edit in a never-initialized submodule.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant StorageCleanup
  participant submoduleRemoval
  participant Git
  StorageCleanup->>submoduleRemoval: inspect submodule state
  submoduleRemoval->>Git: check metadata, status, and refs
  Git-->>submoduleRemoval: return submodule state
  submoduleRemoval-->>StorageCleanup: return keep reason or force requirement
  StorageCleanup->>Git: recheck local changes and remove worktree

Loading

Suggested reviewers: maria-rcks

Merge Risk: ⚪ Minimal · up to cdaee

Storage cleanup now removes worktrees with clean submodules and keeps those with local changes or unpushed commits. No merge-blocking risk was found.

### Architecture Summary

Architecture risk: 🔵 Low · up to cdaee

The change affects 1 system.

Changed systems: apps/server

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — apps/server (service) was modified; 2 changed files map to changed impact.

Before / after behavior

• observed — Modified behavior in apps/server/src/storageCleanup.test.ts: Added a fixture that creates a worktree containing a submodule and provides a helper for adding a nested submodule, committing it, and pushing its commit to the remote.
• observed — Modified behavior in apps/server/src/storageCleanup.test.ts: Added cases expecting cleanup to remove a clean worktree with populated or deinitialized nested submodules after the nested commit is pushed.
• observed — Modified behavior in apps/server/src/storageCleanup.test.ts: Added cases expecting cleanup to keep a worktree when a submodule has an unpushed commit on HEAD or a local branch.
• observed — Modified behavior in apps/server/src/storageCleanup.test.ts: Added cases expecting cleanup to retain a submodule with a tracked edit under uncommitted-changes or an ignored .env file under any-local-files.

Pre-merge checks: 3 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Description check		The description thoroughly documents the problem, implementation, verification results, limitations, and testing environment. However, it does not provide the required explicit maintainer approval for…	Add the required Scope and approval information, including a link to the approval comment or an explanation of the applicable exemption. Record the required human safety approval before merging.

Passed checks: 3 passed

Check name	Status	Explanation
Linked Issues check		Issue #20355 requires server cleanup to remove eligible worktrees with initialized submodules while retaining local-change protection and the late tracked-edit guard. storageCleanup.ts recursively c…
Out of Scope Changes check		The reviewed changes are limited to apps/server/src/storageCleanup.ts and its focused integration tests. The changes support issue #20355. No desktop toast change or unrelated feature change is pres…
Title check		The title clearly and concisely describes the main server-side change: cleaning up worktrees that contain initialized submodules.

Full details: Linked Issues check

Explanation

Issue #20355 requires server cleanup to remove eligible worktrees with initialized submodules while retaining local-change protection and the late tracked-edit guard. storageCleanup.ts recursively checks populated submodules, inspects stored submodule repositories, keeps the worktree when inspection finds changes or fails, and uses --force only after the checks pass. The forced path rechecks the worktree immediately before removal. storageCleanup.test.ts covers populated and deinitialized nested submodules, unpushed refs, local changes, malformed stored metadata, and a late tracked edit.

Full details: Out of Scope Changes check

Explanation

The reviewed changes are limited to apps/server/src/storageCleanup.ts and its focused integration tests. The changes support issue #20355. No desktop toast change or unrelated feature change is present. The separate #20211 toast issue remains out of scope.

Full details: Description check

Explanation

The description thoroughly documents the problem, implementation, verification results, limitations, and testing environment. However, it does not provide the required explicit maintainer approval for scope and direction, and it states that human safety approval is still missing.

Resolution

Add the required Scope and approval information, including a link to the approval comment or an explanation of the applicable exemption. Record the required human safety approval before merging.

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

[juliusmarminge]

Taking over this one for batch worktree-git-11. I will reduce the real-Git regression tests, rebuild the smallest safe submodule cleanup change, and run independent adversarial reviews. No merge.