[nwparker]

	Files	Added	Deleted	Net
Test	2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​24	$\color{#cf222e}{\Huge{\mathbf{−}}}$​2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​22
Prod	2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​48	$\color{#cf222e}{\Huge{\mathbf{−}}}$​46	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​2

ELI5

Git won't delete a workspace that has submodules unless you force it, even when nothing in it has changed. Orca used to find that out too late: it had already stopped the workspace's terminals and told the caller the delete was underway, and Git's refusal then happened in the background, where the CLI never saw it. Orca now runs Git's own check first and refuses up front, before anything is stopped.

What Changed

• Before: deleting a clean workspace whose checkout has submodules, without Force, passed Orca's pre-delete check. Orca stopped its terminals and accepted the delete, then git worktree remove failed in the background with "working trees containing submodules cannot be moved or removed". The CLI (orca worktree rm) and automation never saw that error, and every retry stopped the terminals again.
• After: the pre-delete check (assertWorktreeCleanForRemoval) applies Git's own submodule test: submodule repositories under the worktree's admin directory, or an index gitlink checked out with its .git. The native Windows path already used this test. A clean checkout that fails it is refused before teardown. The same test now decides when a dirty refusal is labelled as containing submodules, replacing a probe based on .gitmodules that missed submodules which were deinitialized but still have a repository under the admin directory.
• The refusal text keeps Git's wording, so every app version (including ones that predate this change) classifies it as the existing "submodules" Force Delete case and shows the warning about losing unpushed submodule work. The CLI shows the text itself, so it says what force would delete.
• The check reuses resolveGitDir and resolveWorktreeHostPath, so WSL guest paths resolve the same way as elsewhere. If the probe itself fails anywhere except native Windows, Git decides, as before.

Why

The refusal belongs with the other refusals that run before teardown. That is what the acceptance contract promises: everything that can refuse happens before terminals stop and before the host answers. Forcing automatically was rejected on purpose: #28878 removed exactly that, because --force also deletes submodule commits, local branches and stashes that were never pushed.

The CLI reply contract (removed: true before deletion, failures only in the host log) is fixed separately in the companion PR for #35135.

Linked Issue

Refs #35135

Thanks to MrClean73 for the precise report and the plain-git reproduction.

Visual Proof

N/A. No new UI: the existing submodule Force Delete toast now appears before teardown instead of after the background delete fails.

Testing

• Reproduced with plain git (Git on macOS): a clean linked worktree with an initialized submodule passes git status --porcelain --ignore-submodules=none with no output, then git worktree remove fails with rc=128. A deinitialized submodule whose modules directory remains is refused the same way.

• New real-Git cases in src/main/git/worktree-removal-real-git.test.ts (checked-out submodule, and deinitialized submodule with its repository still in the admin directory) fail on origin/main and pass here. Each also asserts that Git itself refuses the checkout.

• Removal preflight, IPC removal and Windows removal suites pass.

• pnpm check:changed --maxWorkers=2: everything passes except tests that also fail on origin/main on this machine (worktree-create-preparation-real-git partial-checkout abort, and electron-serve-browser-process, which trips the real-home write guard). Neither touches this change.

• Not run: native Windows or a WSL distro. The Windows path keeps its existing check, and the probe path is the same on every OS.

• I manually tested these changes locally

• Automated tests added/updated, or explained why not below

Review

Agent skill upstream boundary

• Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

• Security: no new access. Only a refusal moves earlier.
• Cross-platform: the native Windows path is unchanged. WSL paths are translated through the shared resolvers.
• Remote SSH: orcad runs the same main-process code. The message is classified by its text, so mixed client and host versions behave correctly.
• Mobile: mobile gets the refusal as a request error, the same as other refusals.
• Performance: one git ls-files --stage after git status per non-forced delete, plus one stat call.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[coderabbitai]

Walkthrough

The removal preflight now detects checked-out submodules through the Git admin directory and staged gitlinks instead of .gitmodules paths. It refuses removal with submodule-specific messages for clean and dirty worktrees. Tests cover probe failure, checked-out submodules, and force-deinitialized submodules.

Priority: Normal

Merge Risk: Moderate · up to d150c

A large worktree index can cause terminals to close even though Git refuses to remove the worktree. Resolve that preflight failure path before merging unless this risk is explicitly accepted.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: refusing submodule checkouts before terminal teardown.
Description check		The description is complete and focused. It covers the user impact, implementation, rationale, linked issue, visual proof, testing, platform considerations, and checklist status. It also documents the…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description is complete and focused. It covers the user impact, implementation, rationale, linked issue, visual proof, testing, platform considerations, and checklist status. It also documents the known unrelated test failures and the untested Windows and WSL environments.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 4 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 2

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable failures were found.

Summary

Moves Git's submodule refusal into the check that runs before terminals stop.

• Worktree removal refuses submodule checkouts before terminal teardown.

No actionable issues were found. Tests were inspected, not run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Remove workspace] --> B{Force requested?}
  B -->|Yes| F[Continue existing removal flow]
  B -->|No| C[Check checkout state]
  C --> D{Contains submodule repositories?}
  D -->|Yes| E[Refuse before terminals stop]
  D -->|No| F

Loading

_{Reviews (2) · Last reviewed commit: "fix(worktrees): refuse a submodule check..." · Reviewed by Greptile}

[coderabbitai]

Actionable comments posted: 1

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 86243f48-fb54-42a1-b4f1-af492b08b34c

Commits

Reviewing files that changed from the base of the PR and between 4c31ba5 and d150cfd.

Files selected for processing: 4

• src/main/git/remove-worktree-clean-preflight.test.ts
• src/main/git/worktree-removal-preflight.ts
• src/main/git/worktree-removal-real-git.test.ts
• src/shared/worktree/removal.ts

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

[coderabbitai]

Major · Stability & Availability · Heavy lift

Keep large-index probe failures from reaching teardown.

If a populated gitlink has no <adminDirectory>/modules directory and ls-files --stage -z emits more than 10 MiB, gitExecFileAsync rejects the output. The caller converts that failure to false. A clean worktree can then lose its terminals before git worktree remove refuses the populated submodule. Use a bounded probe that can find a gitlink without capturing the full index, or refuse deletion when this probe exceeds its output limit. (raw.githubusercontent.com)