Repository navigation
fix(worktree): offer Force Delete with a plain warning when a checkout has submodules - #28878
Conversation
Git refuses to remove any worktree with an initialized submodule unless --force is set, even when nothing is uncommitted. After that specific refusal, re-check the checkout including files inside the submodule and pass --force once. A dirty submodule still blocks the delete. A second --force is not added, because that would also clear a Git lock.
…modules Drop the automatic --force retry. A forced removal deletes the submodule's repository under .git/worktrees/<id>/modules, including commits, local branches and stashes that were never pushed, and the parent's dirty check cannot see them (the retry #24602 removed for the same reason). Instead classify Git's "working trees containing submodules cannot be moved or removed" refusal as a new Force Delete reason, 'submodules', after the lock check and the force guard. The renderer classifies the error string, so older SSH and paired hosts get it too with no wire change. The toast, the delete dialog and the Space manager row show one plain warning naming what Force Delete would lose instead of Git's text. Keep --ignore-submodules=none in the pre-teardown dirty check so a user's ignore setting cannot hide submodule edits from it. Replace the retry test with one real-Git test: a clean checkout whose submodule holds a local-only commit is refused without --force, stays registered, keeps the commit, and its refusal classifies as 'submodules'.
…ew for it The warning now says Git deletes the workspace only with Force Delete, the submodule refusal no longer offers View (there are no changes to show), the real-Git test pins that Force Delete then succeeds, and a comment no longer describes the removed retry.
…lete, and clear a stale delete toast With a commit inside a submodule that the parent has not recorded, the parent sees `M vendor`, so Orca's dirty check refused first with the generic "It has changed files". Force Delete then destroyed the never-pushed submodule commit without warning. On a dirty refusal the check now asks `git status --porcelain=v2` whether a changed entry is a submodule, and if so reports a refusal the app shows with the submodule Force Delete warning. The detail keeps the dirty sentence, so older apps still offer Force Delete. After the Space manager deleted a workspace (its row Force, or a later delete), the failure toast for that workspace stayed up and still offered Force Delete. A Space delete now dismisses it.
…out that has them Review found the warning still vanished when a submodule commit was already recorded and some other file was dirty: the parent's status shows no submodule change, yet Force Delete destroys the commit. The changed-files check now asks whether the checkout has a populated submodule (a .gitmodules path holding a .git, as Git's own refusal checks) instead of whether one changed. The refusal keeps the changed paths after the detail, and View stays available since there are changes to view.
…1-submodule-delete
Review loop status: doneWhat this PR changes for users: deleting a workspace whose checkout has set-up Git submodules, through a delete that doesn't force (confirmation turned off, the Space manager), used to fail with Git's raw "working trees containing submodules cannot be moved or removed" and no way forward. Now the failure says in plain words that Force Delete is needed and that it also permanently removes anything in those submodules that was never pushed (commits, local branches, stashes), and offers Force Delete. Nothing is deleted without that click. Review: four rounds, plus an architecture challenge and a design ruling.
Deferred, listed in the PR body Notes (unchanged from main or rare):
QA: real Electron on the M4Air (macOS), built app, isolated home, every delete a real UI click.
CI: the full PR workflow is green on this head after the final main merge. QA ran on d626edd; later commits are main merges (latest a64df01) plus a reviewed conflict resolution, and two reviewed fixes for a CodeRabbit finding: the failure toast is now keyed by host and workspace, so a Space delete on one host can't close another host's toast for a workspace with the same id (66d6300, 2407678). |
brennanb2025
changed the title
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the
Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 47.06% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 13 files. (1 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the primary change: offering Force Delete with a warning for worktrees that contain submodules. Description check The description is complete and follows the repository template. It explains the user impact, implementation, rationale, linked issue, visual proof, testing, known limitations, cross-platform consider… 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 follows the repository template. It explains the user impact, implementation, rationale, linked issue, visual proof, testing, known limitations, cross-platform considerations, and checklist status.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 47.06% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 13 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
- 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.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
7993bd66-2f3d-4c52-8162-5526d6fecddf
📒 Files selected for processing: 11
src/main/git/remove-worktree-clean-preflight.test.tssrc/main/git/worktree-removal-preflight.tssrc/main/git/worktree-removal-real-git.test.tssrc/renderer/src/components/sidebar/delete-worktree-failure-toast.tsxsrc/renderer/src/components/sidebar/delete-worktree-toast.tssrc/renderer/src/components/sidebar/worktree-delete-error-display.tssrc/renderer/src/components/status-bar/use-workspace-space-manager-panel.tssrc/renderer/src/components/status-bar/workspace-space-worktree-row.tsxsrc/renderer/src/i18n/locales/en.jsonsrc/shared/git-command-failure-text.tssrc/shared/worktree/removal.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.
…1-submodule-delete # Conflicts: # src/renderer/src/components/status-bar/use-workspace-space-manager-panel.ts
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
8a2d3e4a-70da-4be0-8f71-536746f24475
📒 Files selected for processing: 3
src/renderer/src/components/status-bar/use-workspace-space-manager-panel.tssrc/renderer/src/components/status-bar/workspace-space-worktree-row.tsxsrc/renderer/src/i18n/locales/en.json
🚧 Files skipped from review as they are similar to previous changes: 1
- src/renderer/src/i18n/locales/en.json
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
…1-submodule-delete
A Space delete on one host must not dismiss another host's failure toast for a worktree with the same id.
…r a Space row Force
ELI5
Some workspace deletes failed with a raw Git error and no way forward when the workspace had git submodules set up. Git won't delete a checkout with submodules unless it's forced. Orca now explains that in plain words and offers the existing Force Delete button, with a warning that forcing also throws away anything in the submodules that was never pushed.
What Changed
Before: deleting a workspace whose checkout had an initialized submodule, through a delete that doesn't force, failed with Git's own text: "fatal: working trees containing submodules cannot be moved or removed". No Force Delete button appeared, so a user who had turned off the delete confirmation couldn't delete the workspace from the app. The deletes that don't force are: deleting with the confirmation turned off ("don't ask again"), the Space manager's delete, and
orca worktree rmwithout--force. The ordinary delete confirmation dialog already forces, so it was not affected and is unchanged.After: in the app, the failure toast and the Space manager row say: "This workspace contains submodules, so Git deletes it only with Force Delete. Force Delete also permanently removes anything in those submodules that was never pushed: commits, local branches and stashes." The Force Delete button is offered (the View button is hidden, since there are no changes to view), and clicking it deletes the workspace. Nothing is deleted without that click. The
orca worktree rmoutput is unchanged.The same warning also shows when the delete stops because the workspace has changed files and it has set-up submodules. That includes a commit made inside a submodule, which the parent sees as a changed
vendor. Before, that case said only "It has changed files. Use Force Delete to delete it anyway.", and Force Delete then removed the never-pushed submodule commit with no warning. The View button stays available there, since there are changes to view. QA found this on the first build of this pull request.After the Space manager deletes a workspace (its row's Force button, or a later delete), the failure toast for that workspace now closes. Before, it stayed on screen and still offered Force Delete for a workspace that no longer existed.
Also: Orca's dirty-file check before a delete now passes
--ignore-submodules=none, so a repository or user setting that hides submodule changes can't make a workspace with edited submodule files look clean. Those deletes now stop before any terminal is closed, with Force Delete and the existing "It has changed files" message (or the submodule warning above when a submodule is what changed).Why
Git refuses a non-forced
git worktree removefor any checkout with an initialized submodule, even a clean one. The only way through is--force, and for a linked checkout--forcealso deletes the submodule's own repository (it lives inside that workspace's Git admin folder). Commits, local branches and stashes made inside the submodule and never pushed exist nowhere else, andgit statusin the parent can't see them. A delete that doesn't ask can't know whether it is about to destroy that work.The first version of this pull request retried automatically with
--forcewhengit statuswas clean. Review reproduced, with real Git 2.50.1 and 2.25.1, that the retry permanently destroyed a submodule commit the parent branch still pointed at. #24602 had already removed the same automatic retry for that reason, and its real-Git guard test was later lost in #27215. Orca's rule is not to clear user data automatically to recover from an error, but to offer a button. So this version keeps Git's refusal and makes it actionable through the existing Force Delete path. It adds one Force Delete reason (submodules) beside the others insrc/shared/worktree/removal.ts. It adds no new force mechanism and no extra Git command on every delete. Trying a delete first and offering Force Delete when Git refuses is the common pattern.The refusal is recognized in the app's window from the error text (Git output is pinned to English on every host), so this also works when the workspace lives on an SSH host or paired server running an older version. Nothing on the wire changed.
For a workspace with changed files, Orca's own changed-files check refuses before Git does, so Git's submodule refusal never appears, and the parent's
git statuscan't show commits that exist only inside a submodule. So only when that check is already refusing, it checks whether the workspace has a set-up submodule, the way Git's own refusal does: a path listed in.gitmodules(git config --file .gitmodules, works on Git 2.25) that holds a.git. If so, it reports a submodule refusal, with the changed paths still listed after it. Clean deletes run no extra command. Mixed versions (intended): that refusal still contains the "uncommitted or untracked changes" sentence, so an older app reads it as changed files and keeps offering Force Delete; an older host paired with this app shows the generic changed-files copy, as before.#25979 is an open pull request with the same approach.
Linked Issue
Tracked in #28164 (Linear STA-10391).
Visual Proof
Remote Electron QA on the M4Air (macOS), built app, isolated home, every delete a real UI click, on head 00f2ba1 against the base on main.
Delete with the confirmation turned off, clean workspace with an initialized submodule. Before (main): red toast with Git's raw text, no Force Delete. After: plain warning with Force Delete.
After Force Delete, the workspace is gone:
Space manager delete. Before (main): row shows Git's raw text with no Force button. After: the plain warning and a Force button, which removed the workspace.
A real signed-in Claude CLI running in the workspace: same warning, and Force Delete removed it.
Unchanged and still working: delete through the confirmation dialog, an untracked file inside the submodule (refused with Force Delete; the file survived), and a plain workspace with a terminal open.
The first QA run found two things, both fixed and re-checked on the M4Air at d626edd (every flow passed):
A commit inside the submodule (the parent shows a changed
vendor). Before the fix (00f2ba1) it said only "It has changed files"; now it shows the submodule warning, keeps View, and the commit survives the refusal.A submodule commit already recorded in the workspace plus an unrelated new file: same warning. A repo without submodules and an edited file: the unchanged "It has changed files" copy.
Space manager: after the row's Force removes the workspace, its failure toast closes; only "Workspace deleted" remains.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Real Git: a clean checkout whose submodule holds a commit that exists only there is refused without force; the checkout and the commit survive, the refusal is classified as the submodule Force Delete reason, and a forced delete then succeeds. This guards the Git output the app depends on.
Real Git: a workspace with a never-pushed submodule commit and an unrelated new file is refused by Orca's changed-files check as the submodule Force Delete reason, with the changed path still listed and the sentence older apps rely on still present.
The earlier unit test "preserves Git refusal even when parent status cannot reveal unpublished submodule commits" is back as it is on main.
Not run: a live WSL distro or a live SSH host. Git 2.25.1 was checked by hand during review: same refusal text, and the status flag works.
Agent skill upstream boundary
Notes
Author: @BrennanKB5
Known, unchanged from main, and out of scope here: the ordinary delete confirmation already forces, so it deletes never-pushed submodule work without this warning; on the deletes that don't force, Orca closes the workspace's terminals and agents before Git refuses; the workspace cleanup list still shows Git's raw text for any refused delete;
orca worktree rmcan report success for a delete that is refused later in the background; a delete failure toast still stays up when the workspace is deleted some other way than the Space manager (from the terminal, the cleanup list or another window); in the Space manager the failure toast can cover a failed row's Force button until it is closed (toasts share one screen position); and a workspace whose submodule history remains without a.gitmodulesentry pointing at it (a deinitialized submodule, or.gitmodulesremoved) gets the generic changed-files message instead of the submodule warning, still with Force Delete.Cross-platform: the Git arguments are the same on macOS, Linux and Windows. Folder workspaces are not Git checkouts and are unaffected.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)