Skip to content

Prevent removal of submodule changes hidden by config - #4252

Merged
max-sixty merged 1 commit into
max-sixty:mainfrom
Duang777:fix/remove-hidden-submodule-dirt
Sep 24, 2026
Merged

max-sixty merged 1 commit into
max-sixty:mainfrom
Duang777:fix/remove-hidden-submodule-dirt

Conversation

@Duang777

Copy link
Copy Markdown
Contributor

Summary

  • make worktree clean checks override submodule.<name>.ignore
  • preserve initialized submodule changes during ordinary wt remove
  • add an end-to-end regression test for ignore=all

Root cause

WorkingTree::is_dirty and dirty_files inherited the user's submodule ignore setting. With submodule.<name>.ignore=all, Git reported a modified initialized submodule as clean. Worktrunk then used the internally required git worktree remove --force, deleting the worktree and its uncommitted submodule change.

The clean checks now pass --ignore-submodules=none, matching the existing plumbing diff behavior and keeping display preferences out of destructive safety decisions.

Verification

  • Red: the new test reproduced successful removal and loss of the modified worktree on the old code
  • Green: targeted regression and related tests passed
  • cargo run -- hook pre-merge --yes
    • 4,963 tests passed
    • 1 skipped
    • fmt, Clippy, doctests, rustdoc, lockfile, and snapshot checks passed

Worktree cleanliness checks inherited submodule.<name>.ignore, so remove could treat a modified initialized submodule as clean and delete it. Force --ignore-submodules=none in both clean-check paths so display preferences cannot weaken destructive guards.

Add a regression test that proves ordinary status hides the change while wt remove refuses and preserves the modified file.
@max-sixty
max-sixty merged commit c2feef0 into max-sixty:main Sep 24, 2026
38 checks passed
@max-sixty max-sixty mentioned this pull request Sep 27, 2026
max-sixty added a commit that referenced this pull request Sep 27, 2026
Bumps 0.79.0 → 0.80.0 and adds the `## 0.80.0` changelog section, cut
from `12efb5159`. The bump is minor because the library API breaks (see
Verification).

## Verification

- `cargo run -- hook pre-merge --yes` on the cut tip: 4985 passed.
- `nightly.yaml` on `12efb5159`, all jobs green including
feature-powerset:
https://github.com/max-sixty/worktrunk/actions/runs/36338233859
- `cargo semver-checks check-release -p worktrunk` reports five
major-level lints, the removals and signature changes listed in the
Internal entry.
- Changelog: repeated verification passes against source until one came
back clean. Fixed headlines name the condition a bug needed rather than
its worst case, and near-zero-reach fixes are folded into two roll-up
bullets, per #4295.
- Data-loss surface review over `v0.79.0..HEAD` with independent
finders. The one real finding, a nested stale detached worktree read as
the enclosing repo's rebase so `wt step prune` could unregister it, was
fixed in #4302. The review also found a squash regression in the backup
path, fixed in #4303. The remaining candidates were adjudicated
acceptable and signed off: #4228 unregistering stale worktrees whose
directory remains (files stay on disk; the branch is deleted only if
integrated), #4206 leaving a detached `HEAD` on Ctrl-C during a slow
commit hook, #4251 allowing `--no-squash` merges with untracked files,
#4219 dropping `--no-hooks` from the skill example, and #4279's `docker
stop` recipe. #4252 closes a submodule data-loss path.

> _This was written by Claude Code on behalf of max-sixty_

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants