From 92127f0a252c13463f4e9409065a045a21e55b08 Mon Sep 17 00:00:00 2001 From: "duanjialing.777" Date: Thu, 24 Sep 2026 13:52:39 +0800 Subject: [PATCH] fix: detect submodule dirt hidden by config Worktree cleanliness checks inherited submodule..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. --- src/git/repository/working_tree.rs | 19 ++++-- tests/integration_tests/remove.rs | 101 +++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 4 deletions(-) diff --git a/src/git/repository/working_tree.rs b/src/git/repository/working_tree.rs index 8615f678f..992bc87c1 100644 --- a/src/git/repository/working_tree.rs +++ b/src/git/repository/working_tree.rs @@ -584,10 +584,16 @@ impl<'a> WorkingTree<'a> { /// 3. Users who use skip-worktree are power users who understand the implications /// 4. A warning wouldn't prevent data loss anyway — it's informational only /// - /// Untracked files are always included, regardless of the user's - /// `status.showUntrackedFiles` display preference. + /// Untracked files and dirty submodules are always included, regardless of + /// the user's `status.showUntrackedFiles` or `submodule..ignore` + /// display preferences. pub fn is_dirty(&self) -> anyhow::Result { - let stdout = self.run_command(&["status", "--porcelain", "--untracked-files=normal"])?; + let stdout = self.run_command(&[ + "status", + "--porcelain", + "--untracked-files=normal", + "--ignore-submodules=none", + ])?; Ok(!stdout.trim().is_empty()) } @@ -598,7 +604,12 @@ impl<'a> WorkingTree<'a> { /// [`GitError::UncommittedChanges`] in [`Self::ensure_clean`]. The same /// caveats as [`Self::is_dirty`] apply (skip-worktree files are invisible). pub fn dirty_files(&self) -> anyhow::Result> { - let stdout = self.run_command(&["status", "--porcelain", "--untracked-files=normal"])?; + let stdout = self.run_command(&[ + "status", + "--porcelain", + "--untracked-files=normal", + "--ignore-submodules=none", + ])?; Ok(stdout.lines().map(str::to_owned).collect()) } diff --git a/tests/integration_tests/remove.rs b/tests/integration_tests/remove.rs index e142520cf..51950c449 100644 --- a/tests/integration_tests/remove.rs +++ b/tests/integration_tests/remove.rs @@ -4259,6 +4259,107 @@ fn test_remove_worktree_submodule_dirty_fails_closed(mut repo: TestRepo) { ); } +/// `submodule..ignore=all` is a display preference, not permission to +/// delete modifications inside an initialized submodule. This is especially +/// important because Worktrunk must internally force Git's worktree removal +/// when initialized submodules are present. +#[rstest] +fn test_remove_refuses_dirty_submodule_hidden_by_user_config(mut repo: TestRepo) { + let sub_source = repo.root_path().parent().unwrap().join("sub-source-hidden"); + fs::create_dir_all(&sub_source).unwrap(); + repo.run_git_in(&sub_source, &["init", "-b", "main"]); + fs::write(sub_source.join("sub.txt"), "submodule content").unwrap(); + repo.run_git_in(&sub_source, &["add", "sub.txt"]); + repo.run_git_in(&sub_source, &["commit", "-m", "sub init"]); + + let output = repo + .git_command() + .args([ + "-c", + "protocol.file.allow=always", + "submodule", + "add", + sub_source.to_str().unwrap(), + "submod", + ]) + .run() + .unwrap(); + assert!( + output.status.success(), + "Failed to add submodule: {}", + String::from_utf8_lossy(&output.stderr) + ); + repo.run_git(&["commit", "-m", "add submodule"]); + + let worktree_path = repo.add_worktree("feature-submod-hidden-dirty"); + let output = repo + .git_command() + .current_dir(&worktree_path) + .args([ + "-c", + "protocol.file.allow=always", + "submodule", + "update", + "--init", + ]) + .run() + .unwrap(); + assert!( + output.status.success(), + "Failed to init submodule: {}", + String::from_utf8_lossy(&output.stderr) + ); + + repo.run_git(&["config", "submodule.submod.ignore", "all"]); + let changed_file = worktree_path.join("submod/sub.txt"); + fs::write(&changed_file, "DIRTIED\n").unwrap(); + + let hidden = repo + .git_command() + .current_dir(&worktree_path) + .args(["status", "--porcelain"]) + .run() + .unwrap(); + assert!( + hidden.stdout.is_empty(), + "the fixture must demonstrate that the user setting hides the dirty submodule" + ); + let forced = repo + .git_command() + .current_dir(&worktree_path) + .args(["status", "--porcelain", "--ignore-submodules=none"]) + .run() + .unwrap(); + assert!( + String::from_utf8_lossy(&forced.stdout).contains("submod"), + "the explicit safety query must reveal the dirty submodule" + ); + + let output = repo + .wt_command() + .args(["remove", "--foreground", "feature-submod-hidden-dirty"]) + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !output.status.success(), + "remove must refuse a dirty submodule hidden by config; stderr:\n{stderr}" + ); + assert!( + stderr.contains("submod"), + "the refusal must name the dirty submodule; stderr:\n{stderr}" + ); + assert!( + worktree_path.exists(), + "the hidden dirty submodule's worktree must be preserved" + ); + assert_eq!( + fs::read_to_string(changed_file).unwrap(), + "DIRTIED\n", + "the hidden submodule change must remain recoverable" + ); +} + /// Restore write permissions recursively so TempDir cleanup succeeds. #[cfg(unix)] fn restore_dir_permissions(dir: &std::path::Path) {