Repository navigation
fix(box): worktrees with submodules can be removed when nothing would be lost - #197
Merged
Merged
Conversation
… be lost git refuses to remove any worktree with a submodule checked out, however clean, so removing or archiving one always failed. On that refusal, RemoveWorktree now checks for the work git won't (changes in the worktree or its submodules, submodule commits no remote has) and forces the removal only when there is none. Otherwise it answers 409 in git's words, so the app keeps the worktree and offers Remove anyway. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nd stashes Address Codex review (round 1): - the safety check counts untracked files even when status.showUntrackedFiles hides them, in the worktree and in each submodule (each submodule is now asked itself, so its own config can't hide a change) - a submodule stash counts as work to keep, since it lives only in the worktree's copy of the submodule's history Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d deinitialized submodules' history Address Codex review (round 2): - every submodule repository under the worktree's git folder is checked, not only checked-out ones: a deinit leaves the repository (and its branches) behind, and the removal would delete it - each one is checked against all its local refs (branches, tags, stashes, HEAD), so a commit only a local tag holds counts as work to keep Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…odule's own .git Address Codex review (round 3): - a checked-out submodule's own git folder is checked too, so one cloned by hand (its repository embedded in the worktree's folder, not under the worktree's modules) can't lose commits no remote has Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Address Codex review (round 4): - the refusal, the app's toast and the changelog promised to keep "tags"; what is kept is commits no remote has (one only a tag holds included) and stashes. A local tag's name on history a remote has isn't checked: that would take asking the remote on every removal Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…fers remove anyway Address cubic review on #197: - a refusal because the box couldn't check a worktree with submodules got a plain error with no way forward; it now gets the Kept toast with Remove anyway, in its own words rather than "uncommitted changes" - which refusal says what is now keptReason, with a unit test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…hand-cloned ones Address Codex review (round 6): - a hand-cloned submodule keeps its own submodules' repositories under its own .git/modules; a deinitialized one there is no longer skipped, so its unpushed branches and stashes keep the worktree Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… ignore setting Address Codex review (round 7): - each submodule's own status now overrides submodule.<name>.ignore, so a nested submodule checked out at another commit (an uncommitted change) keeps the worktree even when the submodule above it ignores it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Address cubic review on #197: - keptReason matched bare words (contains, modified, untracked), so an unrelated removal failure could be called uncommitted changes and offer a force; it now matches the refusal's own words, "modified or untracked files", which git and the box both use Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merged
This was referenced Oct 28, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
git refuses to remove any worktree with a submodule checked out, however clean ("working trees containing submodules cannot be moved or removed"). Only
--forcegets past it, so in repos with submodules every removal or archive failed, and the app showed a dead-end "Couldn't remove" (its "Remove anyway" toast only matches git's dirty-files wording).RemoveWorktree(internal/box/locations.go) now catches that refusal on an unforced removal, runs the check git skips, and forces only when nothing would be lost:git status --porcelain --untracked-files=normal --ignore-submodules=noneon the worktree, plusstatusinside each checked-out submodule, so neitherstatus.showUntrackedFilesnor a submodule's own config can hide anything.modules/, deinitialized ones included, nested ones too, and a checked-out submodule's own embedded.git) is checked withrev-list --all --not --remotes. That covers commits that only a branch, a tag or the stash holds.When something would be lost, the box answers 409 in git's usual "…, use --force to delete it" form. The app keeps the worktree and offers Remove anyway, and its toast says "commits or stashes no remote has" rather than "uncommitted changes" when that's the reason.
Testing
TestAWorktreeWithSubmodulesIsRemovedWhenNothingWouldBeLostcovers each case that must keep the worktree, then checks that a clean one with a deinitialized submodule is removed. The cases: a file in a submodule, the same with untracked files hidden by config, a file in the worktree hidden by config, a submodule stash, an uncommitted and then a committed-but-unpushed submodule commit, a commit only a local tag holds, a branch in a deinitialized submodule, and a commit in a hand-cloned (embedded) submodule. With the embedded lookup turned off, the test fails at that case.go vet ./...,go test -race ./...,cargo check, all the app checks,pnpm test,pnpm build,npx tsc -p e2eand the docs build pass.TestReviewRoutesOverTheWireflakes on TempDir cleanup ("directory not empty"), 3 runs in 10 on both this branch andorigin/main. It's unrelated to this change and left alone.Review
Five Codex review rounds, each by a fresh reviewer:
status.showUntrackedFiles=nohid untracked files from the check. Fixed with--untracked-files=normalandstatusin each submodule. Fixed (P1): submodule stashes weren't checked.modules/, whereforeachnever looks. Every repository there is now walked. Fixed (P1): commits that only a local tag holds were missed. The check now uses--allinstead ofHEAD --branches..git(made withgit clone) wasn't checked. Each checked-out submodule's own git dir is now included.git ls-remote(network, maybe auth) on every removal, and offline it would refuse every submodule worktree again. Such a tag holds no commits that aren't already on a remote, the same as a local branch on pushed history, whose name is also not kept. The fixed part: the message, toast and changelog had promised to keep "tags", and now say exactly what is kept.After the PR opened:
git worktree removedeletes such a worktree (reproduced), and Shipyard's ordinary path does too. Counting ignored files would refuse nearly every real worktree (node_modules, build output).keptReason, unit-tested)..git/modules, which nothing scanned. Those are now walked too. A new test fails without the fix.submodule.<name>.ignore=allin the submodule above it. Each submodule's own status now uses--ignore-submodules=none. A test covers it and fails without the fix.keptReasonmatched bare words (contains,modified), so an unrelated failure could have offered Remove anyway. It now matches the refusal's own words.origin/main. Codex round 9: clean.🤖 Generated with Claude Code