hook: clear stale exit-dir state

CHANGELOG.md

@@ -10,6 +10,7 @@
 - Fixed shell hook spawning a nested `devenv shell` when `devenv shell` was entered manually. Follow-up to [#2815](https://github.com/cachix/devenv/pull/2815).
 - Fixed "zoxide: infinite loop detected" when using `zoxide init --cmd=cd fish` and `cd`-ing into a devenv project. The fish hook now defers spawning `devenv shell` to the next prompt instead of spawning inline inside the PWD event handler, so in-progress shell state never leaks into the devenv shell ([#2841](https://github.com/cachix/devenv/issues/2841)).
 - Fixed stale task and option results that lingered when `devenv.nix` was edited while a command was already evaluating. The eval cache no longer stores results whose tracked input files were modified mid-evaluation, so the next run no longer needs `.devenv/nix-eval-cache.db*` to be wiped to see the new definitions ([#2745](https://github.com/cachix/devenv/issues/2745)).
+- Fixed shell hook auto-activation reusing a stale `.devenv/exit-dir` when a shell override blocked cleanup (for example a custom `rm` function), which could send the next shell exit to an old directory.
 
 ### Improvements
 

devenv/hooks/hook.fish

@@ -48,12 +48,14 @@ end
 # Spawn devenv shell in $project_dir and follow the user if they cd'd out.
 function _devenv_hook_activate
     set -l project_dir $argv[1]
+    set -l exit_dir_file "$project_dir/.devenv/exit-dir"
+    # Drop stale state from an earlier failed cleanup before launching a new shell.
+    command rm -f "$exit_dir_file"
     env -C $project_dir _DEVENV_HOOK_DIR=$project_dir devenv shell
     # If the devenv shell exited due to cd outside the project, follow the user there
-    set -l exit_dir_file "$project_dir/.devenv/exit-dir"
     if test -f "$exit_dir_file"
         set -l target_dir (cat "$exit_dir_file")
-        rm -f "$exit_dir_file"
+        command rm -f "$exit_dir_file"
         if test -d "$target_dir"
             builtin cd "$target_dir"
         end

devenv/hooks/hook.nu

@@ -27,13 +27,15 @@ $env.config = ($env.config | upsert hooks.env_change.PWD (
         if $result.exit_code == 0 {
             let dir = ($result.stdout | str trim)
             if $dir != "" {
+                let exit_dir_file = ($dir + "/.devenv/exit-dir")
+                # Drop stale state from an earlier failed cleanup before launching a new shell.
+                ^rm -f $exit_dir_file
                 do { cd $dir; ^devenv shell }
                 $env._DEVENV_HOOK_UNTRUSTED = ""
                 # If the devenv shell exited due to cd outside the project, follow the user there
-                let exit_dir_file = ($dir + "/.devenv/exit-dir")
                 if ($exit_dir_file | path exists) {
                     let target_dir = (open $exit_dir_file | str trim)
-                    rm -f $exit_dir_file
+                    ^rm -f $exit_dir_file
                     if ($target_dir | path exists) {
                         cd $target_dir
                     }
@@ -63,13 +65,15 @@ $env.config = ($env.config | upsert hooks.pre_prompt (
         if $result.exit_code == 0 {
             let dir = ($result.stdout | str trim)
             if $dir != "" {
+                let exit_dir_file = ($dir + "/.devenv/exit-dir")
+                # Drop stale state from an earlier failed cleanup before launching a new shell.
+                ^rm -f $exit_dir_file
                 do { cd $dir; ^devenv shell }
                 $env._DEVENV_HOOK_UNTRUSTED = ""
                 # If the devenv shell exited due to cd outside the project, follow the user there
-                let exit_dir_file = ($dir + "/.devenv/exit-dir")
                 if ($exit_dir_file | path exists) {
                     let target_dir = (open $exit_dir_file | str trim)
-                    rm -f $exit_dir_file
+                    ^rm -f $exit_dir_file
                     if ($target_dir | path exists) {
                         cd $target_dir
                     }

devenv/hooks/hook.posix.sh

@@ -38,12 +38,14 @@ _devenv_hook() {
         # Cache PWD before launching so a SIGINT/failure inside devenv shell
         # doesn't leave us re-launching on every prompt redraw.
         _DEVENV_HOOK_PWD="$PWD"
-        (cd "$project_dir" && _DEVENV_HOOK_DIR="$project_dir" devenv shell)
         local exit_dir_file="$project_dir/.devenv/exit-dir"
+        # Drop stale state from an earlier failed cleanup before launching a new shell.
+        command rm -f "$exit_dir_file"
+        (cd "$project_dir" && _DEVENV_HOOK_DIR="$project_dir" devenv shell)
         if [[ -f "$exit_dir_file" ]]; then
             local target_dir
             target_dir=$(cat "$exit_dir_file")
-            rm -f "$exit_dir_file"
+            command rm -f "$exit_dir_file"
             if [[ -d "$target_dir" ]]; then
                 cd "$target_dir"
                 _DEVENV_HOOK_PWD="$PWD"

devenv/tests/hook_outer_shell_survives.rs

@@ -19,17 +19,30 @@ fn devenv_bin() -> &'static str {
     env!("CARGO_BIN_EXE_devenv")
 }
 
+type PathOverrideFn = fn(&Path) -> String;
+type ShellSnippetFn = fn() -> &'static str;
+
 /// Available shells with the syntax needed to (a) source the hook from the
 /// real `devenv` binary and (b) override `PATH`.
-fn shells() -> Vec<(&'static str, String, fn(&Path) -> String)> {
+fn shells() -> Vec<(&'static str, String, PathOverrideFn, ShellSnippetFn)> {
     let bash_src = format!(r#"eval "$({} hook bash)""#, devenv_bin());
     let fish_src = format!("{} hook fish | source", devenv_bin());
     [
-        ("bash", bash_src, bash_path_override as fn(&Path) -> String),
-        ("fish", fish_src, fish_path_override as fn(&Path) -> String),
+        (
+            "bash",
+            bash_src,
+            bash_path_override as PathOverrideFn,
+            bash_block_rm as ShellSnippetFn,
+        ),
+        (
+            "fish",
+            fish_src,
+            fish_path_override as PathOverrideFn,
+            fish_block_rm as ShellSnippetFn,
+        ),
     ]
     .into_iter()
-    .filter(|(s, _, _)| have(s))
+    .filter(|(s, _, _, _)| have(s))
     .collect()
 }
 
@@ -41,6 +54,14 @@ fn fish_path_override(dir: &Path) -> String {
     format!("set -gx PATH {:?} $PATH", dir)
 }
 
+fn bash_block_rm() -> &'static str {
+    "rm() { echo BLOCKED >&2; }"
+}
+
+fn fish_block_rm() -> &'static str {
+    "function rm; echo BLOCKED >&2; end"
+}
+
 fn have(shell: &str) -> bool {
     Command::new("sh")
         .args(["-c", &format!("command -v {shell}")])
@@ -56,12 +77,20 @@ fn fake_project() -> tempfile::TempDir {
 }
 
 fn run(shell: &str, script: &str) -> std::process::Output {
-    Command::new(shell).arg("-c").arg(script).output().unwrap()
+    Command::new(shell)
+        .env_remove("DEVENV_ROOT")
+        .env_remove("_DEVENV_HOOK_DIR")
+        .env_remove("_DEVENV_HOOK_UNTRUSTED")
+        .env_remove("_DEVENV_ACTIVATE_DIR")
+        .arg("-c")
+        .arg(script)
+        .output()
+        .unwrap()
 }
 
 #[test]
 fn outer_shell_survives_cd_out() {
-    for (shell, src, _) in shells() {
+    for (shell, src, _, _) in shells() {
         let tmp = fake_project();
         let script = format!(
             "export DEVENV_ROOT={root:?}\n{src}\ncd /\n_devenv_hook\necho SURVIVED\n",
@@ -79,7 +108,7 @@ fn outer_shell_survives_cd_out() {
 
 #[test]
 fn inner_shell_exits_on_cd_out() {
-    for (shell, src, _) in shells() {
+    for (shell, src, _, _) in shells() {
         let tmp = fake_project();
         let script = format!(
             "export DEVENV_ROOT={root:?}\nexport _DEVENV_HOOK_DIR={root:?}\n\
@@ -99,7 +128,7 @@ fn inner_shell_exits_on_cd_out() {
 
 #[test]
 fn no_respawn_inside_devenv_shell() {
-    for (shell, src, path_override) in shells() {
+    for (shell, src, path_override, _) in shells() {
         let tmp = fake_project();
         // Shim `devenv` on PATH so any `hook-should-activate` / `shell`
         // invocation from inside the hook is recorded.
@@ -133,3 +162,53 @@ fn no_respawn_inside_devenv_shell() {
         );
     }
 }
+
+#[test]
+fn stale_exit_dir_cleanup_bypasses_shell_rm_overrides() {
+    for (shell, src, path_override, block_rm) in shells() {
+        let tmp = fake_project();
+        let exit_dir = tmp.path().join(".devenv/exit-dir");
+
+        // Shim `devenv` on PATH so the hook activates the temp project and
+        // exits the child shell immediately without writing a new exit-dir.
+        let bin_dir = tempfile::tempdir().unwrap();
+        let fake = bin_dir.path().join("devenv");
+        fs::write(
+            &fake,
+            format!(
+                "#!/usr/bin/env bash\ncase \"$1\" in\n  hook-should-activate) printf '%s\\n' {:?} ;;\n  shell) exit 0 ;;\n  *) exit 1 ;;\nesac\n",
+                tmp.path(),
+            ),
+        )
+        .unwrap();
+        fs::set_permissions(&fake, fs::Permissions::from_mode(0o755)).unwrap();
+
+        let activate = if shell == "fish" {
+            format!("_devenv_hook_activate {:?}", tmp.path())
+        } else {
+            "_devenv_hook".to_string()
+        };
+
+        let script = format!(
+            "cd {root:?}\nprintf '%s' / > {exit_dir:?}\n{src}\n{po}\n{block_rm}\n{activate}\npwd\n",
+            root = tmp.path(),
+            exit_dir = exit_dir,
+            po = path_override(bin_dir.path()),
+            block_rm = block_rm(),
+            activate = activate,
+        );
+        let out = run(shell, &script);
+        let stdout = String::from_utf8_lossy(&out.stdout);
+        assert!(
+            stdout
+                .lines()
+                .any(|line| line == tmp.path().to_string_lossy()),
+            "[{shell}] stale exit-dir changed directories even though rm was overridden.\nstdout: {stdout}\nstderr: {}",
+            String::from_utf8_lossy(&out.stderr),
+        );
+        assert!(
+            !exit_dir.exists(),
+            "[{shell}] stale exit-dir was not removed when rm was overridden",
+        );
+    }
+}