Implement PWD recovery (#12779)

This PR has two parts. The first part is the addition of the
`Stack::set_pwd()` API. It strips trailing slashes from paths for
convenience, but will reject otherwise bad paths, leaving PWD in a good
state. This should reduce the impact of faulty code incorrectly trying
to set PWD.
(https://github.com/nushell/nushell/pull/12760#issuecomment-2095393012)

The second part is implementing a PWD recovery mechanism. PWD can become
bad even when we did nothing wrong. For example, Unix allows you to
remove any directory when another process might still be using it, which
means PWD can just "disappear" under our nose. This PR makes it possible
to use `cd` to reset PWD into a good state. Here's a demonstration:

```sh
mkdir /tmp/foo
cd /tmp/foo

# delete "/tmp/foo" in a subshell, because Nushell is smart and refuse to delete PWD
nu -c 'cd /; rm -r /tmp/foo'

ls          # Error:   × $env.PWD points to a non-existent directory
            # help: Use `cd` to reset $env.PWD into a good state

cd /
pwd         # prints /
```

Also, auto-cd should be working again.

Cargo.lock

@@ -2852,6 +2852,7 @@ dependencies = [
  "reedline",
  "rstest",
  "sysinfo",
+ "tempfile",
  "unicode-segmentation",
  "uuid",
  "which",

crates/nu-cli/Cargo.toml

@@ -15,6 +15,7 @@ nu-cmd-lang = { path = "../nu-cmd-lang", version = "0.93.1" }
 nu-command = { path = "../nu-command", version = "0.93.1" }
 nu-test-support = { path = "../nu-test-support", version = "0.93.1" }
 rstest = { workspace = true, default-features = false }
+tempfile = { workspace = true }
 
 [dependencies]
 nu-cmd-base = { path = "../nu-cmd-base", version = "0.93.1" }

crates/nu-cli/src/repl.rs

@@ -870,7 +870,7 @@ fn parse_operation(
     let tokens = lex(s.as_bytes(), 0, &[], &[], false);
     // Check if this is a single call to a directory, if so auto-cd
     #[allow(deprecated)]
-    let cwd = nu_engine::env::current_dir_str(engine_state, stack)?;
+    let cwd = nu_engine::env::current_dir_str(engine_state, stack).unwrap_or_default();
     let mut orig = s.clone();
     if orig.starts_with('`') {
         orig = trim_quotes_str(&orig).to_string()
@@ -927,7 +927,10 @@ fn do_auto_cd(
 
     //FIXME: this only changes the current scope, but instead this environment variable
     //should probably be a block that loads the information from the state in the overlay
-    stack.add_env_var("PWD".into(), Value::string(path.clone(), Span::unknown()));
+    if let Err(err) = stack.set_cwd(&path) {
+        report_error_new(engine_state, &err);
+        return;
+    };
     let cwd = Value::string(cwd, span);
 
     let shells = stack.get_env_var(engine_state, "NUSHELL_SHELLS");
@@ -1479,3 +1482,136 @@ fn are_session_ids_in_sync() {
         engine_state.history_session_id
     );
 }
+
+#[cfg(test)]
+mod test_auto_cd {
+    use super::{do_auto_cd, parse_operation, ReplOperation};
+    use nu_protocol::engine::{EngineState, Stack};
+    use std::path::Path;
+    use tempfile::tempdir;
+
+    /// Create a symlink. Works on both Unix and Windows.
+    #[cfg(any(unix, windows))]
+    fn symlink(original: impl AsRef<Path>, link: impl AsRef<Path>) -> std::io::Result<()> {
+        #[cfg(unix)]
+        {
+            std::os::unix::fs::symlink(original, link)
+        }
+        #[cfg(windows)]
+        {
+            if original.as_ref().is_dir() {
+                std::os::windows::fs::symlink_dir(original, link)
+            } else {
+                std::os::windows::fs::symlink_file(original, link)
+            }
+        }
+    }
+
+    /// Run one test case on the auto-cd feature. PWD is initially set to
+    /// `before`, and after `input` is parsed and evaluated, PWD should be
+    /// changed to `after`.
+    #[track_caller]
+    fn check(before: impl AsRef<Path>, input: &str, after: impl AsRef<Path>) {
+        // Setup EngineState and Stack.
+        let mut engine_state = EngineState::new();
+        let mut stack = Stack::new();
+        stack.set_cwd(before).unwrap();
+
+        // Parse the input. It must be an auto-cd operation.
+        let op = parse_operation(input.to_string(), &engine_state, &stack).unwrap();
+        let ReplOperation::AutoCd { cwd, target, span } = op else {
+            panic!("'{}' was not parsed into an auto-cd operation", input)
+        };
+
+        // Perform the auto-cd operation.
+        do_auto_cd(target, cwd, &mut stack, &mut engine_state, span);
+        let updated_cwd = engine_state.cwd(Some(&stack)).unwrap();
+
+        // Check that `updated_cwd` and `after` point to the same place. They
+        // don't have to be byte-wise equal (on Windows, the 8.3 filename
+        // conversion messes things up),
+        let updated_cwd = std::fs::canonicalize(updated_cwd).unwrap();
+        let after = std::fs::canonicalize(after).unwrap();
+        assert_eq!(updated_cwd, after);
+    }
+
+    #[test]
+    fn auto_cd_root() {
+        let tempdir = tempdir().unwrap();
+        let root = if cfg!(windows) { r"C:\" } else { "/" };
+        check(&tempdir, root, root);
+    }
+
+    #[test]
+    fn auto_cd_tilde() {
+        let tempdir = tempdir().unwrap();
+        let home = nu_path::home_dir().unwrap();
+        check(&tempdir, "~", home);
+    }
+
+    #[test]
+    fn auto_cd_dot() {
+        let tempdir = tempdir().unwrap();
+        check(&tempdir, ".", &tempdir);
+    }
+
+    #[test]
+    fn auto_cd_double_dot() {
+        let tempdir = tempdir().unwrap();
+        let dir = tempdir.path().join("foo");
+        std::fs::create_dir_all(&dir).unwrap();
+        check(dir, "..", &tempdir);
+    }
+
+    #[test]
+    fn auto_cd_triple_dot() {
+        let tempdir = tempdir().unwrap();
+        let dir = tempdir.path().join("foo").join("bar");
+        std::fs::create_dir_all(&dir).unwrap();
+        check(dir, "...", &tempdir);
+    }
+
+    #[test]
+    fn auto_cd_relative() {
+        let tempdir = tempdir().unwrap();
+        let foo = tempdir.path().join("foo");
+        let bar = tempdir.path().join("bar");
+        std::fs::create_dir_all(&foo).unwrap();
+        std::fs::create_dir_all(&bar).unwrap();
+
+        let input = if cfg!(windows) { r"..\bar" } else { "../bar" };
+        check(foo, input, bar);
+    }
+
+    #[test]
+    fn auto_cd_trailing_slash() {
+        let tempdir = tempdir().unwrap();
+        let dir = tempdir.path().join("foo");
+        std::fs::create_dir_all(&dir).unwrap();
+
+        let input = if cfg!(windows) { r"foo\" } else { "foo/" };
+        check(&tempdir, input, dir);
+    }
+
+    #[test]
+    fn auto_cd_symlink() {
+        let tempdir = tempdir().unwrap();
+        let dir = tempdir.path().join("foo");
+        std::fs::create_dir_all(&dir).unwrap();
+        let link = tempdir.path().join("link");
+        symlink(&dir, &link).unwrap();
+
+        let input = if cfg!(windows) { r".\link" } else { "./link" };
+        check(&tempdir, input, link);
+    }
+
+    #[test]
+    #[should_panic(expected = "was not parsed into an auto-cd operation")]
+    fn auto_cd_nonexistent_directory() {
+        let tempdir = tempdir().unwrap();
+        let dir = tempdir.path().join("foo");
+
+        let input = if cfg!(windows) { r"foo\" } else { "foo/" };
+        check(&tempdir, input, dir);
+    }
+}

crates/nu-cmd-base/src/util.rs

@@ -1,6 +1,6 @@
 use nu_protocol::{
     engine::{EngineState, Stack},
-    report_error_new, Range, ShellError, Span, Value,
+    Range, ShellError, Span, Value,
 };
 use std::{ops::Bound, path::PathBuf};
 
@@ -13,10 +13,9 @@ pub fn get_init_cwd() -> PathBuf {
 }
 
 pub fn get_guaranteed_cwd(engine_state: &EngineState, stack: &Stack) -> PathBuf {
-    engine_state.cwd(Some(stack)).unwrap_or_else(|e| {
-        report_error_new(engine_state, &e);
-        crate::util::get_init_cwd()
-    })
+    engine_state
+        .cwd(Some(stack))
+        .unwrap_or(crate::util::get_init_cwd())
 }
 
 type MakeRangeError = fn(&str, Span) -> ShellError;

crates/nu-command/src/filesystem/cd.rs

@@ -1,3 +1,4 @@
+use nu_cmd_base::util::get_init_cwd;
 use nu_engine::command_prelude::*;
 use nu_utils::filesystem::{have_permission, PermissionResult};
 
@@ -39,7 +40,10 @@ impl Command for Cd {
     ) -> Result<PipelineData, ShellError> {
         let physical = call.has_flag(engine_state, stack, "physical")?;
         let path_val: Option<Spanned<String>> = call.opt(engine_state, stack, 0)?;
-        let cwd = engine_state.cwd(Some(stack))?;
+
+        // If getting PWD failed, default to the initial directory. This way, the
+        // user can use `cd` to recover PWD to a good state.
+        let cwd = engine_state.cwd(Some(stack)).unwrap_or(get_init_cwd());
 
         let path_val = {
             if let Some(path) = path_val {
@@ -52,13 +56,13 @@ impl Command for Cd {
             }
         };
 
-        let (path, span) = match path_val {
+        let path = match path_val {
             Some(v) => {
                 if v.item == "-" {
                     if let Some(oldpwd) = stack.get_env_var(engine_state, "OLDPWD") {
-                        (oldpwd.to_path()?, v.span)
+                        oldpwd.to_path()?
                     } else {
-                        (cwd, v.span)
+                        cwd
                     }
                 } else {
                     // Trim whitespace from the end of path.
@@ -66,7 +70,7 @@ impl Command for Cd {
                         &v.item.trim_end_matches(|x| matches!(x, '\x09'..='\x0d'));
 
                     // If `--physical` is specified, canonicalize the path; otherwise expand the path.
-                    let path = if physical {
+                    if physical {
                         if let Ok(path) = nu_path::canonicalize_with(path_no_whitespace, &cwd) {
                             if !path.is_dir() {
                                 return Err(ShellError::NotADirectory { span: v.span });
@@ -90,19 +94,12 @@ impl Command for Cd {
                             return Err(ShellError::NotADirectory { span: v.span });
                         };
                         path
-                    };
-                    (path, v.span)
+                    }
                 }
             }
-            None => {
-                let path = nu_path::expand_tilde("~");
-                (path, call.head)
-            }
+            None => nu_path::expand_tilde("~"),
         };
 
-        // Strip the trailing slash from the new path. This is required for PWD.
-        let path = nu_path::strip_trailing_slash(&path);
-
         // Set OLDPWD.
         // We're using `Stack::get_env_var()` instead of `EngineState::cwd()` to avoid a conversion roundtrip.
         if let Some(oldpwd) = stack.get_env_var(engine_state, "PWD") {
@@ -113,7 +110,7 @@ impl Command for Cd {
             //FIXME: this only changes the current scope, but instead this environment variable
             //should probably be a block that loads the information from the state in the overlay
             PermissionResult::PermissionOk => {
-                stack.add_env_var("PWD".into(), Value::string(path.to_string_lossy(), span));
+                stack.set_cwd(path)?;
                 Ok(PipelineData::empty())
             }
             PermissionResult::PermissionDenied(reason) => Err(ShellError::IOError {

crates/nu-command/tests/commands/cd.rs

@@ -312,3 +312,17 @@ fn cd_permission_denied_folder() {
         assert!(actual.err.contains("Folder is not able to read"));
     });
 }
+
+#[test]
+#[cfg(unix)]
+fn pwd_recovery() {
+    let nu = nu_test_support::fs::executable_path().display().to_string();
+    let tmpdir = std::env::temp_dir().join("foobar").display().to_string();
+
+    // We `cd` into a temporary directory, then spawn another `nu` process to
+    // delete that directory. Then we attempt to recover by running `cd /`.
+    let cmd = format!("mkdir {tmpdir}; cd {tmpdir}; {nu} -c 'cd /; rm -r {tmpdir}'; cd /; pwd");
+    let actual = nu!(cmd);
+
+    assert_eq!(actual.out, "/");
+}

crates/nu-engine/src/eval.rs

@@ -654,7 +654,7 @@ impl Eval for EvalRuntime {
         } else if quoted {
             Ok(Value::string(path, span))
         } else {
-            let cwd = engine_state.cwd(Some(stack))?;
+            let cwd = engine_state.cwd(Some(stack)).unwrap_or_default();
             let path = expand_path_with(path, cwd, true);
 
             Ok(Value::string(path.to_string_lossy(), span))

crates/nu-protocol/src/engine/engine_state.rs

@@ -931,13 +931,12 @@ impl EngineState {
     /// directory on the stack that have yet to be merged into the engine state.
     pub fn cwd(&self, stack: Option<&Stack>) -> Result<PathBuf, ShellError> {
         // Helper function to create a simple generic error.
-        // Its messages are not especially helpful, but these errors don't occur often, so it's probably fine.
-        fn error(msg: &str) -> Result<PathBuf, ShellError> {
+        fn error(msg: &str, cwd: impl AsRef<Path>) -> Result<PathBuf, ShellError> {
             Err(ShellError::GenericError {
                 error: msg.into(),
-                msg: "".into(),
+                msg: format!("$env.PWD = {}", cwd.as_ref().display()),
                 span: None,
-                help: None,
+                help: Some("Use `cd` to reset $env.PWD into a good state".into()),
                 inner: vec![],
             })
         }
@@ -967,21 +966,21 @@ impl EngineState {
                 // Technically, a root path counts as "having trailing slashes", but
                 // for the purpose of PWD, a root path is acceptable.
                 if !is_root(&path) && has_trailing_slash(&path) {
-                    error("$env.PWD contains trailing slashes")
+                    error("$env.PWD contains trailing slashes", path)
                 } else if !path.is_absolute() {
-                    error("$env.PWD is not an absolute path")
+                    error("$env.PWD is not an absolute path", path)
                 } else if !path.exists() {
-                    error("$env.PWD points to a non-existent directory")
+                    error("$env.PWD points to a non-existent directory", path)
                 } else if !path.is_dir() {
-                    error("$env.PWD points to a non-directory")
+                    error("$env.PWD points to a non-directory", path)
                 } else {
                     Ok(path)
                 }
             } else {
-                error("$env.PWD is not a string")
+                error("$env.PWD is not a string", format!("{pwd:?}"))
             }
         } else {
-            error("$env.PWD not found")
+            error("$env.PWD not found", "")
         }
     }
 

crates/nu-protocol/src/engine/stack.rs

@@ -592,6 +592,40 @@ impl Stack {
         self.out_dest.pipe_stderr = None;
         self
     }
+
+    /// Set the PWD environment variable to `path`.
+    ///
+    /// This method accepts `path` with trailing slashes, but they're removed
+    /// before writing the value into PWD.
+    pub fn set_cwd(&mut self, path: impl AsRef<std::path::Path>) -> Result<(), ShellError> {
+        // Helper function to create a simple generic error.
+        // Its messages are not especially helpful, but these errors don't occur often, so it's probably fine.
+        fn error(msg: &str) -> Result<(), ShellError> {
+            Err(ShellError::GenericError {
+                error: msg.into(),
+                msg: "".into(),
+                span: None,
+                help: None,
+                inner: vec![],
+            })
+        }
+
+        let path = path.as_ref();
+
+        if !path.is_absolute() {
+            error("Cannot set $env.PWD to a non-absolute path")
+        } else if !path.exists() {
+            error("Cannot set $env.PWD to a non-existent directory")
+        } else if !path.is_dir() {
+            error("Cannot set $env.PWD to a non-directory")
+        } else {
+            // Strip trailing slashes, if any.
+            let path = nu_path::strip_trailing_slash(path);
+            let value = Value::string(path.to_string_lossy(), Span::unknown());
+            self.add_env_var("PWD".into(), value);
+            Ok(())
+        }
+    }
 }
 
 #[cfg(test)]