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,50 @@ use vt_path::{AbsolutePath, AbsolutePathBuf, RelativePathBuf};
 
 mod ps1_shim;
 
+/// 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()
+    }
+}
+
+fn resolve_bin_from_path_entry(
+    bin_name: &str,
+    path_entry: &Path,
+    cwd: &AbsolutePath,
+) -> Option<std::path::PathBuf> {
+    if path_entry.starts_with("~") || !is_plain_relative_path(path_entry) {
+        // Preserve tilde expansion and Windows-special path semantics by passing the original
+        // entry through `which`. Since this entry came from `split_paths`, serializing it alone
+        // cannot introduce the command cwd's PATH separator.
+        let path_env = std::env::join_paths([path_entry]).ok()?;
+        which::which_in(bin_name, Some(path_env), cwd).ok()
+    } else {
+        // Search the absolute candidate directly. Re-serializing `cwd.join(path_entry)` into PATH
+        // would fail on Unix when cwd contains `:`, even though the relative PATH entry is valid.
+        let candidate = cwd.as_path().join(path_entry).join(bin_name);
+        which::which_in(candidate, None::<&OsStr>, cwd).ok()
+    }
+}
+
 /// Result of running a command with fspy tracking.
 #[derive(Debug)]
 pub struct FspyCommandResult {
@@ -39,15 +84,29 @@ 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())
-        .map_err(|_| Error::CannotFindBinaryPath(bin_name.into()))?;
+    let bin_path = Path::new(bin_name);
+    let path = if bin_path.is_absolute() || bin_path.components().count() > 1 {
+        // Preserve `which` semantics for an explicit program path: it is resolved directly against
+        // `cwd` and does not search PATH.
+        which::which_in(bin_name, Some(path_env), cwd)
+            .map_err(|_| Error::CannotFindBinaryPath(bin_name.into()))?
+    } else {
+        // `which` resolves relative PATH entries against the process cwd instead of the supplied
+        // command cwd. Search each entry in order so ordinary relative entries can be resolved
+        // against `cwd` without serializing that absolute path back into PATH.
+        std::env::split_paths(path_env)
+            .find_map(|entry| resolve_bin_from_path_entry(bin_name, &entry, cwd))
+            .ok_or_else(|| 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 +458,153 @@ 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_resolve_bin_with_relative_path_entry_when_cwd_contains_path_separator() {
+        use std::path::PathBuf;
+
+        let temp_dir = create_temp_dir();
+        let cwd_path = temp_dir.path().join("project:fixture");
+        std::fs::create_dir_all(&cwd_path).unwrap();
+        let cwd_path = cwd_path.canonicalize().unwrap();
+        let cwd = AbsolutePathBuf::new(cwd_path.clone()).unwrap();
+        let bin_path = cwd_path.join("tools/fake-node");
+        create_executable(&bin_path);
+        let path_env = std::env::join_paths([PathBuf::from("./tools")]).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_relative_entry_when_cwd_contains_path_separator() {
+        use std::path::PathBuf;
+
+        let temp_dir = create_temp_dir();
+        let cwd_path = temp_dir.path().join("project:fixture");
+        std::fs::create_dir_all(&cwd_path).unwrap();
+        let cwd_path = cwd_path.canonicalize().unwrap();
+        let cwd = AbsolutePathBuf::new(cwd_path).unwrap();
+        let fallback_bin_dir = temp_dir.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_explicit_relative_program_path() {
+        use std::path::PathBuf;
+
+        let temp_dir = create_temp_dir();
+        let cwd_path = temp_dir.path().join("project:fixture");
+        std::fs::create_dir_all(&cwd_path).unwrap();
+        let cwd_path = cwd_path.canonicalize().unwrap();
+        let cwd = AbsolutePathBuf::new(cwd_path.clone()).unwrap();
+        let bin_path = cwd_path.join("scripts/fake-node");
+        create_executable(&bin_path);
+        let path_env = std::env::join_paths([PathBuf::from("./tools")]).unwrap();
+
+        let resolved = resolve_bin("./scripts/fake-node", Some(&path_env), &cwd).unwrap();
+
+        assert_eq!(resolved.into_path_buf(), bin_path);
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn test_windows_absolute_entries_are_not_plain_relative_paths() {
+        for entry in [r"C:\tools\bin", r"\\server\share\bin"] {
+            assert!(!is_plain_relative_path(Path::new(entry)));
+        }
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn test_windows_special_relative_entries_are_not_plain_relative_paths() {
+        for entry in [r"C:tools\bin", r"\tools\bin"] {
+            assert!(!is_plain_relative_path(Path::new(entry)));
+        }
+    }
+
     mod run_command_tests {
 
         use super::*;