fix(command): resolve relative PATH entries against cwd

crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "exec-relative-path-cwd",
+  "workspaces": [
+    "packages/*"
+  ]
+}

crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/packages/app/package.json

@@ -0,0 +1,3 @@
+{
+  "name": "app"
+}

crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/setup.js

@@ -0,0 +1,14 @@
+const fs = require('fs');
+
+function writeFakeNode(directory, message) {
+  fs.mkdirSync(directory, { recursive: true });
+  fs.writeFileSync(
+    `${directory}/fake-node`,
+    `#!/usr/bin/env node\nconsole.log(${JSON.stringify(message)});\n`,
+    { mode: 0o755 },
+  );
+  fs.writeFileSync(`${directory}/fake-node.cmd`, '@node "%~dp0\\fake-node" %*\n');
+}
+
+writeFakeNode('packages/app/tools', 'resolved from package cwd');
+writeFakeNode('packages/shared-tools', 'resolved from parent relative PATH');

crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/snapshots.toml

@@ -0,0 +1,11 @@
+[[case]]
+name = "command_exec_relative_path_cwd"
+vp = "local"
+skip-platforms = ["windows"]
+comment = "A relative PATH entry must resolve against the selected package cwd, not the vp process cwd."
+steps = [
+  { argv = ["node", "setup.js"], snapshot = false, continue-on-failure = true },
+  { argv = ["vp", "exec", "--filter", "app", "--", "fake-node"], envs = [["PATH", "./tools:${PATH}"]], comment = "relative PATH entry resolves from the selected package", continue-on-failure = true },
+  { argv = ["vp", "exec", "--filter", "app", "--", "fake-node"], envs = [["PATH", "tools:${PATH}"]], comment = "plain relative PATH entry resolves from the selected package", continue-on-failure = true },
+  { argv = ["vp", "exec", "--filter", "app", "--", "fake-node"], envs = [["PATH", "../shared-tools:${PATH}"]], comment = "parent relative PATH entry resolves from the selected package", continue-on-failure = true },
+]

crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_exec_relative_path_cwd/snapshots/command_exec_relative_path_cwd.md

@@ -0,0 +1,30 @@
+# command_exec_relative_path_cwd
+
+A relative PATH entry must resolve against the selected package cwd, not the vp process cwd.
+
+## `node setup.js`
+
+
+## `PATH=./tools:${PATH} vp exec --filter app -- fake-node`
+
+relative PATH entry resolves from the selected package
+
+```
+resolved from package cwd
+```
+
+## `PATH=tools:${PATH} vp exec --filter app -- fake-node`
+
+plain relative PATH entry resolves from the selected package
+
+```
+resolved from package cwd
+```
+
+## `PATH=../shared-tools:${PATH} vp exec --filter app -- fake-node`
+
+parent relative PATH entry resolves from the selected package
+
+```
+resolved from parent relative PATH
+```

crates/vp_command/src/lib.rs

@@ -10,6 +10,7 @@ use std::os::fd::{BorrowedFd, RawFd};
 use std::{
     collections::HashMap,
     ffi::{OsStr, OsString},
+    path::Path,
     process::{ExitStatus, Stdio},
 };
 
@@ -21,6 +22,44 @@ use vt_path::{AbsolutePath, AbsolutePathBuf, RelativePathBuf};
 
 mod ps1_shim;
 
+fn normalize_path_env(
+    path_env: &OsStr,
+    cwd: &AbsolutePath,
+) -> Result<OsString, std::env::JoinPathsError> {
+    std::env::join_paths(std::env::split_paths(path_env).map(|path| {
+        if path.starts_with("~") || !is_plain_relative_path(&path) {
+            path
+        } else {
+            cwd.as_path().join(path)
+        }
+    }))
+}
+
+/// Return whether a PATH entry is an ordinary relative path that should be resolved against the
+/// command cwd. This includes `tools`, `./tools`, and `../tools`.
+///
+/// Windows drive-relative (`C:tools`) and root-relative (`\tools`) paths have distinct native
+/// semantics. They are intentionally left unchanged, as are absolute drive and UNC paths.
+fn is_plain_relative_path(path: &Path) -> bool {
+    #[cfg(windows)]
+    {
+        !path.has_root()
+            && path.components().next().is_some_and(|component| {
+                matches!(
+                    component,
+                    std::path::Component::CurDir
+                        | std::path::Component::ParentDir
+                        | std::path::Component::Normal(_)
+                )
+            })
+    }
+
+    #[cfg(not(windows))]
+    {
+        !path.is_absolute()
+    }
+}
+
 /// Result of running a command with fspy tracking.
 #[derive(Debug)]
 pub struct FspyCommandResult {
@@ -39,15 +78,22 @@ pub fn resolve_bin(
     path_env: Option<&OsStr>,
     cwd: impl AsRef<AbsolutePath>,
 ) -> Result<AbsolutePathBuf, Error> {
+    let cwd = cwd.as_ref();
     let current_path;
     let path_env = if let Some(p) = path_env {
         p
     } else {
         current_path = std::env::var_os("PATH").unwrap_or_default();
         &current_path
     };
-    let path = which::which_in(bin_name, Some(path_env), cwd.as_ref())
+    // `which` resolves relative PATH entries against the process cwd instead of the supplied
+    // command cwd. Commands are spawned with `cwd`, so resolve the entries the same way first;
+    // leave `~` entries for `which` to expand against the user's home directory.
+    let path_env = normalize_path_env(path_env, cwd)
         .map_err(|_| Error::CannotFindBinaryPath(bin_name.into()))?;
+    let path = which::which_in(bin_name, Some(&path_env), cwd)
+        .map_err(|_| Error::CannotFindBinaryPath(bin_name.into()))?;
+    let path = if is_plain_relative_path(&path) { cwd.as_path().join(path) } else { path };
     AbsolutePathBuf::new(path).ok_or_else(|| Error::CannotFindBinaryPath(bin_name.into()))
 }
 
@@ -399,6 +445,182 @@ mod tests {
         tempdir().expect("Failed to create temp directory")
     }
 
+    #[cfg(unix)]
+    fn create_executable(path: &std::path::Path) {
+        use std::{fs, os::unix::fs::PermissionsExt};
+
+        fs::create_dir_all(path.parent().unwrap()).unwrap();
+        fs::write(path, "#!/bin/sh\nexit 0\n").unwrap();
+        let mut permissions = fs::metadata(path).unwrap().permissions();
+        permissions.set_mode(0o755);
+        fs::set_permissions(path, permissions).unwrap();
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_resolve_bin_with_relative_path_entry() {
+        use std::path::PathBuf;
+
+        let temp_dir = create_temp_dir();
+        let cwd_path = temp_dir.path().canonicalize().unwrap();
+        let cwd = AbsolutePathBuf::new(cwd_path.clone()).unwrap();
+        let bin_dir = cwd_path.join("node_modules/.bin");
+        let bin_path = bin_dir.join("fake-node");
+        let fallback_bin_dir = cwd_path.join("fallback-bin");
+        let fallback_bin_path = fallback_bin_dir.join("fake-node");
+
+        create_executable(&bin_path);
+        create_executable(&fallback_bin_path);
+
+        let path_env =
+            std::env::join_paths([PathBuf::from("./node_modules/.bin"), fallback_bin_dir]).unwrap();
+        let resolved = resolve_bin("fake-node", Some(&path_env), &cwd).unwrap();
+
+        assert_eq!(resolved.into_path_buf(), bin_path);
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_resolve_bin_continues_after_missing_relative_path_entry() {
+        use std::path::PathBuf;
+
+        let temp_dir = create_temp_dir();
+        let cwd_path = temp_dir.path().canonicalize().unwrap();
+        let cwd = AbsolutePathBuf::new(cwd_path.clone()).unwrap();
+        let fallback_bin_dir = cwd_path.join("fallback-bin");
+        let fallback_bin_path = fallback_bin_dir.join("fake-node");
+
+        create_executable(&fallback_bin_path);
+
+        let path_env =
+            std::env::join_paths([PathBuf::from("./missing-bin"), fallback_bin_dir]).unwrap();
+        let resolved = resolve_bin("fake-node", Some(&path_env), &cwd).unwrap();
+
+        assert_eq!(resolved.into_path_buf(), fallback_bin_path);
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_resolve_bin_with_empty_path_entry() {
+        use std::path::PathBuf;
+
+        let temp_dir = create_temp_dir();
+        let cwd_path = temp_dir.path().canonicalize().unwrap();
+        let cwd = AbsolutePathBuf::new(cwd_path.clone()).unwrap();
+        let bin_path = cwd_path.join("fake-node");
+
+        create_executable(&bin_path);
+
+        let path_env = std::env::join_paths([PathBuf::new()]).unwrap();
+        let resolved = resolve_bin("fake-node", Some(&path_env), &cwd).unwrap();
+
+        assert_eq!(resolved.into_path_buf(), bin_path);
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_normalize_path_env_preserves_tilde_entry() {
+        use std::path::PathBuf;
+
+        let temp_dir = create_temp_dir();
+        let cwd_path = temp_dir.path().canonicalize().unwrap();
+        let cwd = AbsolutePathBuf::new(cwd_path).unwrap();
+        let path_env = std::env::join_paths([PathBuf::from("~/bin")]).unwrap();
+
+        let normalized = normalize_path_env(&path_env, &cwd).unwrap();
+
+        assert_eq!(
+            std::env::split_paths(&normalized).collect::<Vec<_>>(),
+            [PathBuf::from("~/bin")]
+        );
+    }
+
+    #[test]
+    fn test_normalize_path_env_resolves_plain_relative_entry_against_cwd() {
+        use std::path::PathBuf;
+
+        let temp_dir = create_temp_dir();
+        let cwd_path = temp_dir.path().canonicalize().unwrap();
+        let cwd = AbsolutePathBuf::new(cwd_path.clone()).unwrap();
+        let path_env = std::env::join_paths([PathBuf::from("node_modules/.bin")]).unwrap();
+
+        let normalized = normalize_path_env(&path_env, &cwd).unwrap();
+
+        assert_eq!(
+            std::env::split_paths(&normalized).collect::<Vec<_>>(),
+            [cwd_path.join("node_modules/.bin")]
+        );
+    }
+
+    #[test]
+    fn test_normalize_path_env_resolves_common_relative_entries_and_keeps_absolute_entries() {
+        use std::path::PathBuf;
+
+        let temp_dir = create_temp_dir();
+        let cwd_path = temp_dir.path().canonicalize().unwrap();
+        let cwd = AbsolutePathBuf::new(cwd_path.clone()).unwrap();
+        let absolute_entry = cwd_path.join("absolute-bin");
+        let path_env = std::env::join_paths([
+            PathBuf::from("node_modules/.bin"),
+            PathBuf::from("./tools"),
+            PathBuf::from("../shared-bin"),
+            absolute_entry.clone(),
+        ])
+        .unwrap();
+
+        let normalized = normalize_path_env(&path_env, &cwd).unwrap();
+
+        assert_eq!(
+            std::env::split_paths(&normalized).collect::<Vec<_>>(),
+            [
+                cwd_path.join("node_modules/.bin"),
+                cwd_path.join("./tools"),
+                cwd_path.join("../shared-bin"),
+                absolute_entry,
+            ]
+        );
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn test_normalize_path_env_preserves_windows_absolute_entries() {
+        use std::path::PathBuf;
+
+        let temp_dir = create_temp_dir();
+        let cwd_path = temp_dir.path().canonicalize().unwrap();
+        let cwd = AbsolutePathBuf::new(cwd_path).unwrap();
+
+        for entry in [r"C:\tools\bin", r"\\server\share\bin"] {
+            let path_env = std::env::join_paths([PathBuf::from(entry)]).unwrap();
+            let normalized = normalize_path_env(&path_env, &cwd).unwrap();
+
+            assert_eq!(
+                std::env::split_paths(&normalized).collect::<Vec<_>>(),
+                [PathBuf::from(entry)]
+            );
+        }
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn test_normalize_path_env_does_not_rewrite_windows_special_relative_entries() {
+        use std::path::PathBuf;
+
+        let temp_dir = create_temp_dir();
+        let cwd_path = temp_dir.path().canonicalize().unwrap();
+        let cwd = AbsolutePathBuf::new(cwd_path).unwrap();
+
+        for entry in [r"C:tools\bin", r"\tools\bin"] {
+            let path_env = std::env::join_paths([PathBuf::from(entry)]).unwrap();
+            let normalized = normalize_path_env(&path_env, &cwd).unwrap();
+
+            assert_eq!(
+                std::env::split_paths(&normalized).collect::<Vec<_>>(),
+                [PathBuf::from(entry)]
+            );
+        }
+    }
+
     mod run_command_tests {
 
         use super::*;