fix(git): scrub inherited GIT_DIR on worktree-local plumbing (#4082)

CHANGELOG.md

@@ -20,6 +20,10 @@
 
 - **`wt switch` recovers from a removed worktree in a bare repository**: recovery walks up from `$PWD` looking for a `.git` directory, and a bare repo has none — not at the bare directory and not at any ancestor — so the walk ran to the filesystem root and found nothing. Bare worktrees typically sit *inside* the bare directory, leaving it nowhere else to land, so `wt switch` and the picker surfaced git's raw "cannot resolve CWD" error instead of recovering and the removed-directory message dropped its `wt switch ^` suggestion. A directory that is itself a bare repository now counts as one to recover from; the deleted path still has to appear in that repository's worktree list before recovery accepts it. ([#4067](https://github.com/max-sixty/worktrunk/pull/4067))
 
+- **`wt merge` no longer treats the feature's own files as uncommitted changes on the target**: when `wt` runs with an inherited `GIT_DIR` pinned to the invoking worktree — a `!wt` git alias from a linked worktree is one source; git also exports discovery vars to the hooks it spawns — worktree-local `status` and `read-tree` forwarded it, so the target conflict check compared the invoking index to the target worktree and refused a merge that should have been clean. Those calls now discover the worktree from the directory `wt` chose; repo-level plumbing keeps the inherited context so the alias still finds the repository.
+
+- **`wt remove` no longer deletes a dirty worktree when an inherited `GIT_DIR` masks its changes**: the same forwarded `GIT_DIR` made `ensure_clean` compare the target's working tree against the invoking worktree's index, so when the two agreed on a path a genuinely dirty target read as clean and removal proceeded with no `--force` and no warning. The dirty gate now reads the worktree being removed.
+
 - **`wt config update` migrates `[ci] platform` into a `[forge]` section that only sets `hostname`**: the migration stood down whenever a `[forge]` section existed at all, so the config a GitHub Enterprise or self-hosted GitLab user ends up with — `[ci] platform` from before the rename, `[forge] hostname` added later for an SSH host alias — kept the deprecated key with no warning and nothing `wt config update` would do about it. The platform still resolved, so nothing broke; the deprecation notice that precedes `[ci]`'s eventual removal simply never arrived. A `[forge]` that already sets `platform`, or a `forge` that isn't a section, still stands down.
 
 - **`wt config shell install` migrates a fish wrapper at the deprecated `conf.d` path even when `~/.config/fish/functions` doesn't exist yet**: fish was skipped for want of a config location, so the bare command left the deprecated wrapper running and only `wt config shell install fish` migrated it. A worktrunk wrapper at the old path now counts as fish being configured, just at the old path. `wt config show` reports that wrapper too, where before it showed nothing at all for fish unless fish was on `PATH`.

CLAUDE.md

@@ -86,7 +86,7 @@ Cmd::new("gh").args(["pr", "list"]).run()?;  // no context for standalone tools
 
 ### Git-Discovery Env Vars Follow Who Chose the Cwd
 
-Git resolves `GIT_DIR`/`GIT_WORK_TREE` (and the rest of `INHERITED_GIT_PATH_VARS`) before walking up from the cwd, so an inherited value silently overrides a child's working directory. **Any spawn site that relocates a user command into a `wt`-chosen worktree — hooks, `wt step for-each`, the `--execute` program — must scrub these vars** (`Cmd::scrub_git_discovery_env` or `scrub_git_discovery_env_vars`); children running in the user's own context (aliases, `commit.generation`) and `wt`'s internal git plumbing keep the inherited context (absolutized). Full site classification and rationale: `scrub_git_discovery_env_vars` in `src/shell_exec.rs`.
+Git resolves `GIT_DIR`/`GIT_WORK_TREE` (and the rest of `INHERITED_GIT_PATH_VARS`) before walking up from the cwd, so an inherited value silently overrides a child's working directory. **Any spawn site whose cwd names a `wt`-chosen worktree must scrub these vars** (`Cmd::scrub_git_discovery_env` or `scrub_git_discovery_env_vars`) — both relocated user commands (hooks, `wt step for-each`, the `--execute` program) and `wt`'s own worktree-local plumbing (`WorkingTree::run_command`, `TempIndex::command`). A site that supplies its own value for one of the scrubbed vars sets it after the scrub; `Cmd` applies env mutations in call order, so the set wins. Children running in the user's own context (aliases, `commit.generation`) and repo-level plumbing (`Repository::run_command`) keep the inherited context (absolutized) — that exemption is about scope, not cwd: `Repository::at` is handed a `wt`-chosen worktree at several sites, but repo-level questions are worktree-agnostic within one repository, and every worktree-scoped answer routes through `WorkingTree`. Full site classification and rationale: `scrub_git_discovery_env_vars` in `src/shell_exec.rs`.
 
 ### Real-time Output Streaming
 

src/commands/step/shared.rs

@@ -217,6 +217,7 @@ fn list_ignored_entries(
         .args(args)
         .current_dir(worktree_path)
         .context(context)
+        .scrub_git_discovery_env()
         .run()
         .context("Failed to run git ls-files")?;
 

src/git/repository/working_tree.rs

@@ -297,13 +297,26 @@ impl<'a> WorkingTree<'a> {
     ///
     /// Use this when you need to check exit codes directly (e.g., for commands
     /// where non-zero exit is not an error condition).
+    ///
+    /// Scrubs the inherited git-discovery vars
+    /// ([`INHERITED_GIT_PATH_VARS`](crate::shell_exec::INHERITED_GIT_PATH_VARS)).
+    /// This call relocates git into `self.path`; those vars are pinned to
+    /// the *invoking* worktree when `wt` runs with an inherited `GIT_DIR`
+    /// (a `!wt` git alias from a linked worktree is one source, and git
+    /// exports discovery vars to the hooks it spawns), so forwarding them
+    /// makes `status`, `rev-parse --git-dir`, and `read-tree` operate on the
+    /// wrong tree. A redirected repository's own `GIT_OBJECT_DIRECTORY` is
+    /// unaffected: `with_object_store_env` sets it after the scrub, and `Cmd`
+    /// applies env mutations in call order. Repo-level
+    /// [`Repository::run_command`] keeps the inherited context on purpose.
     pub fn run_command_output(&self, args: &[&str]) -> anyhow::Result<std::process::Output> {
         self.repo
             .with_object_store_env(
                 Cmd::new("git")
                     .args(args.iter().copied())
                     .current_dir(&self.path)
-                    .context(path_to_logging_context(&self.path)),
+                    .context(path_to_logging_context(&self.path))
+                    .scrub_git_discovery_env(),
             )
             .run()
             .with_context(|| format!("Failed to execute: git {}", args.join(" ")))
@@ -1237,6 +1250,12 @@ impl TempIndex {
     /// Wires `current_dir` to the worktree root, the worktree's logging
     /// context, and `GIT_INDEX_FILE`. The caller adds the subcommand and
     /// chooses `.run()` / `.stream()`.
+    ///
+    /// Scrubs the inherited git-discovery vars for the same reason
+    /// [`WorkingTree::run_command_output`] does, then sets its own
+    /// `GIT_INDEX_FILE` (and, for a redirected repository, its own object-store
+    /// vars) after the scrub — `Cmd` applies env mutations in call order, so
+    /// those sets survive it.
     pub(super) fn command<I, S>(&self, args: I) -> Cmd
     where
         I: IntoIterator<Item = S>,
@@ -1246,6 +1265,7 @@ impl TempIndex {
             .args(args)
             .current_dir(&self.worktree_root)
             .context(self.log_ctx.clone())
+            .scrub_git_discovery_env()
             .env("GIT_INDEX_FILE", self.path());
         match &self.object_store_environment {
             Some((directory, alternates)) => command

src/shell_exec.rs

@@ -644,14 +644,35 @@ pub fn apply_cd_directive_env(cmd: &mut std::process::Command, cd_file: &std::pa
 ///   commands (spawned with no `current_dir`) — the inherited context *is* the
 ///   user's context, so it is forwarded untouched.
 ///
-/// - **`wt`'s own git plumbing** ([`Cmd`] via `Repository::run_command`) keeps
-///   the inherited context on purpose (relative values absolutized, see issue
-///   #1914): `wt` honoring the context it was handed is the point of running
-///   `wt` under `git`.
+/// - **`wt`'s own git plumbing** splits on the same question. Repo-level
+///   ([`Cmd`] via `Repository::run_command`) keeps the inherited context on
+///   purpose (relative values absolutized, see issue #1914): `wt` honoring
+///   the context it was handed is the point of running `wt` under `git`. Its
+///   cwd is `discovery_path`, which is often but not always where the user
+///   invoked `wt` — `Repository::at` is handed a `wt`-chosen worktree at
+///   several sites (the post-switch hook repo, the pipeline repo, `finish`'s
+///   destination repo, the `pre-remove` render repo). The exemption rests on
+///   scope rather than cwd: repo-level questions are worktree-agnostic within
+///   one repository, and every worktree-scoped answer routes through
+///   [`crate::git::WorkingTree`], which scrubs.
+///   **Worktree-local** plumbing — [`crate::git::WorkingTree::run_command`],
+///   `TempIndex::command`, `list_ignored_entries` — relocates git into a
+///   worktree `wt` resolved, so it scrubs, the same way hooks and `for-each`
+///   do. Otherwise a `!wt` alias from a linked worktree (`GIT_DIR` pinned to
+///   that worktree's private gitdir) makes `status` / `read-tree` on a
+///   *different* worktree use the invoking tree's index.
 ///
-/// Any new spawn site that relocates a user command into a `wt`-chosen
-/// worktree must apply this scrub, via this helper or
-/// [`Cmd::scrub_git_discovery_env`].
+/// Every site uses this one list; a site that needs its own value for a
+/// scrubbed var sets it *after* the scrub rather than subsetting the list
+/// ([`Cmd::scrub_git_discovery_env`]). That includes `GIT_OBJECT_DIRECTORY`:
+/// a redirected repository re-sets its own immediately after, so the only
+/// value a worktree-local scrub drops is an **inherited** one — which git
+/// supplies to push-quarantine hooks, and which is pinned to the invoking
+/// context exactly as `GIT_DIR` is. Dropping it is the deliberate call, not
+/// an artifact of reusing the list.
+///
+/// Any new spawn site whose cwd names a `wt`-chosen worktree must apply this
+/// scrub, via this helper or [`Cmd::scrub_git_discovery_env`].
 pub fn scrub_git_discovery_env_vars(cmd: &mut std::process::Command) {
     for var in INHERITED_GIT_PATH_VARS {
         cmd.env_remove(var);
@@ -1061,8 +1082,14 @@ pub struct Cmd {
     context: Option<String>,
     stdin_data: Option<Vec<u8>>,
     timeout: Option<std::time::Duration>,
-    envs: Vec<(OsString, OsString)>,
-    env_removes: Vec<OsString>,
+    /// Environment mutations in call order: `Some(value)` sets, `None`
+    /// removes. One ordered list rather than a set list plus a remove list, so
+    /// the last builder call naming a variable wins — the property
+    /// [`Cmd::scrub_git_discovery_env`] relies on when a caller drops the whole
+    /// inherited git context and then supplies its own value for one of those
+    /// vars (`TempIndex`'s `GIT_INDEX_FILE`, a redirected repository's
+    /// `GIT_OBJECT_DIRECTORY`).
+    env_ops: Vec<(OsString, Option<OsString>)>,
     /// If true, wrap command through ShellConfig (for stream())
     shell_wrap: bool,
     /// Stdout configuration for stream() (defaults to inherit)
@@ -1244,8 +1271,7 @@ impl Cmd {
             context: None,
             stdin_data: None,
             timeout: None,
-            envs: Vec::new(),
-            env_removes: Vec::new(),
+            env_ops: Vec::new(),
             shell_wrap,
             stdout_cfg: None,
             stdin_cfg: None,
@@ -1321,14 +1347,15 @@ impl Cmd {
             cmd.env(key, val);
         }
 
-        // Before `self.envs`, so a per-command env can override the floor.
+        // Before `self.env_ops`, so a per-command env can override the floor.
         apply_hermetic_test_env(cmd);
 
-        for (key, val) in &self.envs {
-            cmd.env(key, val);
-        }
-        for key in &self.env_removes {
-            cmd.env_remove(key);
+        // In builder-call order, so the last mutation naming a variable wins.
+        for (key, val) in &self.env_ops {
+            match val {
+                Some(val) => cmd.env(key, val),
+                None => cmd.env_remove(key),
+            };
         }
 
         // Prevent subprocesses from writing shell directives (security).
@@ -1413,30 +1440,39 @@ impl Cmd {
     /// Accepts the same types as [`Command::env`]: string literals, `String`,
     /// `&Path`, `PathBuf`, `OsString`, etc.
     pub fn env(mut self, key: impl AsRef<OsStr>, val: impl AsRef<OsStr>) -> Self {
-        self.envs
-            .push((key.as_ref().to_os_string(), val.as_ref().to_os_string()));
+        self.env_ops.push((
+            key.as_ref().to_os_string(),
+            Some(val.as_ref().to_os_string()),
+        ));
         self
     }
 
     /// Remove an environment variable.
+    ///
+    /// A later [`Cmd::env`] for the same variable overrides this.
     pub fn env_remove(mut self, key: impl AsRef<OsStr>) -> Self {
-        self.env_removes.push(key.as_ref().to_os_string());
+        self.env_ops.push((key.as_ref().to_os_string(), None));
         self
     }
 
     /// Scrub inherited git-discovery vars ([`INHERITED_GIT_PATH_VARS`]) from the
-    /// child environment. Applied by spawn sites that relocate a user command
-    /// into a `wt`-chosen worktree (hooks, `wt step for-each`) so the command's
-    /// `git` calls discover the repository from the working directory `wt` sets,
-    /// not a `GIT_DIR`/`GIT_WORK_TREE` `wt` inherited. See
+    /// child environment. Applied by every spawn site whose `current_dir` names
+    /// a worktree `wt` chose — relocated user commands (hooks, `wt step
+    /// for-each`, the `--execute` program) and `wt`'s own worktree-local
+    /// plumbing ([`crate::git::WorkingTree::run_command`], `TempIndex`) alike —
+    /// so git discovers the repository from that working directory, not a
+    /// `GIT_DIR`/`GIT_WORK_TREE` `wt` inherited. See
     /// [`scrub_git_discovery_env_vars`] for the site classification (issue #3373).
     ///
     /// Applied after the inherited-`GIT_*` absolutization in
-    /// `apply_common_settings` (env-removes run last), so it also overrides the
-    /// relative-path absolutization that would otherwise re-add these vars.
+    /// `apply_common_settings`, so it also drops the relative-path
+    /// absolutization that would otherwise re-add these vars.
+    ///
+    /// A site that supplies its own value for one of the scrubbed vars calls
+    /// [`Cmd::env`] *after* this: `env_ops` is call-ordered, so the set wins.
     pub fn scrub_git_discovery_env(mut self) -> Self {
         for var in INHERITED_GIT_PATH_VARS {
-            self.env_removes.push(OsString::from(*var));
+            self.env_ops.push((OsString::from(*var), None));
         }
         self
     }
@@ -2333,6 +2369,57 @@ pub fn forward_signal_with_escalation(pgid: i32, sig: i32) {
 mod tests {
     use super::*;
 
+    /// The property the worktree-local scrub sites rest on: `TempIndex` and a
+    /// redirected repository's object store scrub the whole
+    /// [`INHERITED_GIT_PATH_VARS`] list and then set their own value for one of
+    /// those vars. If `Cmd` ever applies removes after sets again, that set is
+    /// silently dropped and those sites fall back to the ambient index / object
+    /// store — so pin call order rather than the split-vector shape.
+    #[test]
+    fn test_env_mutations_apply_in_call_order() {
+        let scrubbed = Cmd::new("child")
+            .scrub_git_discovery_env()
+            .env("GIT_INDEX_FILE", "chosen-index");
+        let mut cmd = std::process::Command::new("child");
+        scrubbed.apply_common_settings(&mut cmd);
+
+        let env = |var: &str| {
+            cmd.get_envs()
+                .find(|(key, _)| *key == std::ffi::OsStr::new(var))
+                .map(|(_, value)| value)
+        };
+
+        assert_eq!(
+            env("GIT_INDEX_FILE"),
+            Some(Some(std::ffi::OsStr::new("chosen-index"))),
+            "a set after the scrub must win"
+        );
+        for var in INHERITED_GIT_PATH_VARS
+            .iter()
+            .filter(|var| **var != "GIT_INDEX_FILE")
+        {
+            assert_eq!(
+                env(var),
+                Some(None),
+                "{var} should be removed from the child environment"
+            );
+        }
+
+        // And the reverse order still removes, so `env_remove` isn't inert.
+        let set_then_removed = Cmd::new("child")
+            .env("GIT_INDEX_FILE", "chosen-index")
+            .scrub_git_discovery_env();
+        let mut cmd = std::process::Command::new("child");
+        set_then_removed.apply_common_settings(&mut cmd);
+        assert_eq!(
+            cmd.get_envs()
+                .find(|(key, _)| *key == std::ffi::OsStr::new("GIT_INDEX_FILE"))
+                .map(|(_, value)| value),
+            Some(None),
+            "a scrub after the set must win"
+        );
+    }
+
     #[test]
     fn test_scrub_directive_env_vars_covers_every_directive_variable() {
         assert_eq!(RETIRED_DIRECTIVE_FILE_ENV_VAR, "WORKTRUNK_DIRECTIVE_FILE");

tests/integration_tests/merge.rs

@@ -84,6 +84,49 @@ fn test_merge_as_git_subcommand(merge_scenario: (TestRepo, PathBuf)) {
     });
 }
 
+/// A `!wt` alias from the feature worktree exports `GIT_DIR` as that
+/// worktree's private gitdir. `advance_target` then runs `read-tree -m -u`
+/// with `current_dir` on main; if the child still sees the inherited
+/// `GIT_DIR`, it writes feature's index and can leave main's worktree
+/// unsynced (or dirty) while still reporting success.
+#[rstest]
+fn test_merge_syncs_target_when_git_dir_names_the_source(merge_scenario: (TestRepo, PathBuf)) {
+    let (repo, feature_wt) = merge_scenario;
+    let git_dir = fs::read_to_string(feature_wt.join(".git")).unwrap();
+    let git_dir = PathBuf::from(git_dir.trim().strip_prefix("gitdir: ").unwrap());
+
+    let output = repo
+        .wt_command()
+        .current_dir(&feature_wt)
+        .args(["merge", "main", "--no-remove", "--yes"])
+        .env("GIT_DIR", &git_dir)
+        .output()
+        .unwrap();
+
+    assert!(
+        output.status.success(),
+        "wt merge must succeed when GIT_DIR names the source worktree.\nstdout: {}\nstderr: {}",
+        String::from_utf8_lossy(&output.stdout),
+        String::from_utf8_lossy(&output.stderr),
+    );
+    assert_eq!(
+        fs::read_to_string(repo.root_path().join("feature.txt")).unwrap_or_default(),
+        "feature content",
+        "the target worktree must receive the merged file",
+    );
+    let status = repo
+        .git_command()
+        .args(["status", "--porcelain"])
+        .current_dir(repo.root_path())
+        .run()
+        .unwrap();
+    assert!(
+        String::from_utf8_lossy(&status.stdout).trim().is_empty(),
+        "the target worktree must be clean after the sync; got: {}",
+        String::from_utf8_lossy(&status.stdout),
+    );
+}
+
 #[rstest]
 fn test_merge_primary_not_on_default_with_default_worktree(
     mut repo_with_alternate_primary: TestRepo,

tests/integration_tests/remove.rs

@@ -8,6 +8,7 @@ use insta::assert_snapshot;
 use insta_cmd::assert_cmd_snapshot;
 use path_slash::PathExt as _;
 use rstest::rstest;
+use std::fs;
 use std::path::{Path, PathBuf};
 
 #[rstest]
@@ -776,6 +777,66 @@ fn test_remove_by_name_dirty_target(mut repo: TestRepo) {
     assert_cmd_snapshot!(make_snapshot_cmd(&repo, "remove", &["feature-dirty"], None));
 }
 
+/// An inherited `GIT_DIR` pinned to the invoking worktree makes
+/// `ensure_clean` compare the target's working tree against the invoking
+/// index. When those agree on a path, a genuinely dirty target reads as
+/// clean and removal proceeds — a silent data-loss path (#4081).
+#[rstest]
+fn test_remove_refuses_dirty_target_when_git_dir_names_invoking_worktree(mut repo: TestRepo) {
+    fs::write(repo.root_path().join("base.txt"), "base").unwrap();
+    repo.run_git_in(repo.root_path(), &["add", "base.txt"]);
+    repo.run_git_in(repo.root_path(), &["commit", "-m", "add base"]);
+
+    let other_wt = repo.add_worktree("other");
+    let feature_wt = repo.add_worktree("feature");
+
+    fs::write(feature_wt.join("base.txt"), "modified").unwrap();
+    repo.run_git_in(&feature_wt, &["add", "base.txt"]);
+    repo.run_git_in(&feature_wt, &["commit", "-m", "feature edits base.txt"]);
+
+    // Dirty against `other`'s HEAD, but matches `feature`'s index.
+    fs::write(other_wt.join("base.txt"), "modified").unwrap();
+    let status = repo
+        .git_command()
+        .args(["status", "--porcelain"])
+        .current_dir(&other_wt)
+        .run()
+        .unwrap();
+    assert!(
+        String::from_utf8_lossy(&status.stdout).contains("base.txt"),
+        "other must be genuinely dirty: {}",
+        String::from_utf8_lossy(&status.stdout),
+    );
+
+    let git_dir = fs::read_to_string(feature_wt.join(".git")).unwrap();
+    let git_dir = PathBuf::from(git_dir.trim().strip_prefix("gitdir: ").unwrap());
+
+    let output = repo
+        .wt_command()
+        .current_dir(&feature_wt)
+        .args(["remove", "other", "--yes"])
+        .env("GIT_DIR", &git_dir)
+        .output()
+        .unwrap();
+
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        !output.status.success(),
+        "wt remove must refuse when GIT_DIR names the invoking worktree.\nstdout: {}\nstderr: {stderr}",
+        String::from_utf8_lossy(&output.stdout),
+    );
+    assert!(
+        stderr.contains("has uncommitted changes"),
+        "removal must be refused by the dirty gate, not another error: {stderr}"
+    );
+    assert!(other_wt.exists(), "the dirty target worktree must survive");
+    assert_eq!(
+        fs::read_to_string(other_wt.join("base.txt")).unwrap(),
+        "modified",
+        "uncommitted changes must survive",
+    );
+}
+
 /// --force allows removal of dirty worktrees (issue #658)
 /// This test: untracked files, branch at same commit as main
 #[rstest]