Enhance node resolution by adding real node path priority handling and updating environment variables in command execution. Introduce tests for package manager shebang preservation and real node path management.

spa5k · spa5k · commit 82d1e7309481 · 2026-03-12T22:38:56.000+05:30
diff --git a/src/app/version.rs b/src/app/version.rs
@@ -5,7 +5,7 @@ use crate::{
         detect::detect,
         resolve::{ResolveContext, version_command_for_pm},
     },
-    platform::node::resolve_real_node_path,
+    platform::node::{REAL_NODE_ENV, path_with_real_node_priority, resolve_real_node_path},
 };
 
 pub fn print_versions(ctx: &ResolveContext) {
@@ -45,11 +45,17 @@ pub fn print_versions(ctx: &ResolveContext) {
 }
 
 fn run_version(program: String, args: Vec<String>, ctx: &ResolveContext) -> Option<String> {
-    let output = Command::new(program)
-        .args(args)
-        .current_dir(&ctx.cwd)
-        .output()
-        .ok()?;
+    let mut command = Command::new(program);
+    command.args(args).current_dir(&ctx.cwd);
+
+    if let Ok(real_node) = resolve_real_node_path() {
+        command.env(REAL_NODE_ENV, &real_node);
+        if let Some(path) = path_with_real_node_priority(&real_node, std::env::var_os("PATH")) {
+            command.env("PATH", path);
+        }
+    }
+
+    let output = command.output().ok()?;
 
     if !output.status.success() {
         return None;
diff --git a/src/core/runner.rs b/src/core/runner.rs
@@ -7,7 +7,9 @@ use super::{
     shell::shell_escape,
     types::ResolvedExecution,
 };
-use crate::platform::node::{SHIM_ACTIVE_ENV, resolve_real_node_path};
+use crate::platform::node::{
+    REAL_NODE_ENV, SHIM_ACTIVE_ENV, path_with_real_node_priority, resolve_real_node_path,
+};
 
 pub fn run(exec: &ResolvedExecution, use_sfw: bool) -> Result<ExitCode> {
     if let Some(mode) = BatchMode::from_internal_program(&exec.program) {
@@ -24,6 +26,13 @@ pub fn run(exec: &ResolvedExecution, use_sfw: bool) -> Result<ExitCode> {
         .stdout(Stdio::inherit())
         .stderr(Stdio::inherit());
 
+    if let Ok(real_node) = resolve_real_node_path() {
+        command.env(REAL_NODE_ENV, &real_node);
+        if let Some(path) = path_with_real_node_priority(&real_node, std::env::var_os("PATH")) {
+            command.env("PATH", path);
+        }
+    }
+
     if passthrough {
         command.env(SHIM_ACTIVE_ENV, "1");
     }
diff --git a/src/platform/node.rs b/src/platform/node.rs
@@ -1,5 +1,7 @@
 use std::{
-    env, fs,
+    env,
+    ffi::OsString,
+    fs,
     path::{Path, PathBuf},
 };
 
@@ -59,16 +61,7 @@ fn scan_path_for_real_node() -> Option<PathBuf> {
         .and_then(|path| path.parent().map(Path::to_path_buf));
     let candidates = which::which_all("node").ok()?;
     for candidate in candidates {
-        if let Some(current_dir) = &current_dir
-            && let Some(parent) = candidate.parent()
-            && paths_equal(parent, current_dir)
-        {
-            continue;
-        }
-
-        if let Some(current_exe) = &current_exe
-            && paths_equal(&candidate, current_exe)
-        {
+        if should_skip_node_candidate(&candidate, current_exe.as_deref(), current_dir.as_deref()) {
             continue;
         }
         return Some(candidate);
@@ -77,9 +70,108 @@ fn scan_path_for_real_node() -> Option<PathBuf> {
     None
 }
 
+pub fn path_with_real_node_priority(
+    real_node: &Path,
+    current_path: Option<OsString>,
+) -> Option<OsString> {
+    let real_node_dir = real_node.parent()?;
+    let mut ordered = Vec::new();
+    ordered.push(real_node_dir.to_path_buf());
+
+    if let Some(current_path) = current_path {
+        ordered.extend(
+            env::split_paths(&current_path).filter(|entry| !paths_equal(entry, real_node_dir)),
+        );
+    }
+
+    env::join_paths(ordered).ok()
+}
+
+fn should_skip_node_candidate(
+    candidate: &Path,
+    current_exe: Option<&Path>,
+    current_dir: Option<&Path>,
+) -> bool {
+    if let Some(current_dir) = current_dir
+        && let Some(parent) = candidate.parent()
+        && paths_equal(parent, current_dir)
+    {
+        return true;
+    }
+
+    if let Some(current_exe) = current_exe
+        && paths_equal(candidate, current_exe)
+    {
+        return true;
+    }
+
+    matches!(
+        candidate
+            .canonicalize()
+            .ok()
+            .as_deref()
+            .and_then(Path::file_name)
+            .and_then(|name| name.to_str()),
+        Some("hni") | Some("hni.exe")
+    )
+}
+
 fn paths_equal(a: &Path, b: &Path) -> bool {
     match (a.canonicalize(), b.canonicalize()) {
         (Ok(a), Ok(b)) => a == b,
         _ => a == b,
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::fs;
+    use tempfile::tempdir;
+
+    #[test]
+    fn path_with_real_node_priority_prepends_real_node_dir_once() {
+        let path = path_with_real_node_priority(
+            Path::new("/real/node"),
+            Some(OsString::from("/shim:/real:/other")),
+        )
+        .unwrap();
+        let entries = env::split_paths(&path).collect::<Vec<_>>();
+
+        assert_eq!(
+            entries,
+            vec![
+                PathBuf::from("/real"),
+                PathBuf::from("/shim"),
+                PathBuf::from("/other"),
+            ]
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn skips_node_candidates_that_resolve_to_hni() {
+        use std::os::unix::fs::symlink;
+
+        let dir = tempdir().unwrap();
+        let release_dir = dir.path().join("release");
+        let debug_dir = dir.path().join("debug");
+        let shim_dir = dir.path().join("shim");
+
+        fs::create_dir_all(&release_dir).unwrap();
+        fs::create_dir_all(&debug_dir).unwrap();
+        fs::create_dir_all(&shim_dir).unwrap();
+
+        let release_hni = release_dir.join("hni");
+        let debug_hni = debug_dir.join("hni");
+        fs::write(&release_hni, b"release").unwrap();
+        fs::write(&debug_hni, b"debug").unwrap();
+        symlink(&release_hni, shim_dir.join("node")).unwrap();
+
+        assert!(should_skip_node_candidate(
+            &shim_dir.join("node"),
+            Some(&debug_hni),
+            Some(&debug_dir),
+        ));
+    }
+}
diff --git a/tests/init_contract.rs b/tests/init_contract.rs
@@ -146,6 +146,98 @@ fn bash_init_gives_node_shim_precedence_and_preserves_real_node() {
     });
 }
 
+#[cfg(unix)]
+#[test]
+fn bash_init_keeps_package_manager_shebangs_on_real_node() {
+    support::with_env_lock(|| {
+        use std::os::unix::fs::PermissionsExt;
+
+        let Some(bash) = which::which("bash").ok() else {
+            return;
+        };
+
+        let dir = tempfile::tempdir().unwrap();
+        let hni_bin = dir.path().join("hni-bin");
+        let real_node_bin = dir.path().join("real-node-bin");
+        let pm_bin = dir.path().join("pm-bin");
+        let fake_home = dir.path().join("home");
+        let fake_config = dir.path().join("config");
+        let shim_log = dir.path().join("shim.log");
+
+        fs::create_dir_all(&hni_bin).unwrap();
+        fs::create_dir_all(&real_node_bin).unwrap();
+        fs::create_dir_all(&pm_bin).unwrap();
+        fs::create_dir_all(&fake_home).unwrap();
+        fs::create_dir_all(&fake_config).unwrap();
+
+        let source_exe = support::hni_executable_path();
+        let copied_hni = hni_bin.join("hni");
+        fs::copy(&source_exe, &copied_hni).unwrap();
+        set_executable_if_needed(&copied_hni);
+
+        let fake_node = real_node_bin.join("node");
+        fs::write(
+            &fake_node,
+            "#!/bin/sh\nif [ \"$1\" = \"--version\" ]; then\n  printf 'v99.0.0\\n'\n  exit 0\nfi\nprintf '99.0.0\\n'\n",
+        )
+        .unwrap();
+        set_executable_if_needed(&fake_node);
+
+        let bad_shim_node = hni_bin.join("node");
+        fs::write(
+            &bad_shim_node,
+            format!(
+                "#!/bin/sh\nprintf 'shim-used\\n' >> '{}'\nexit 97\n",
+                shim_log.display()
+            ),
+        )
+        .unwrap();
+        set_executable_if_needed(&bad_shim_node);
+
+        let fake_npm = pm_bin.join("npm");
+        fs::write(&fake_npm, "#!/usr/bin/env node\nconsole.log('npm');\n").unwrap();
+        set_executable_if_needed(&fake_npm);
+
+        let base_path = format!(
+            "{}:{}:{}",
+            pm_bin.display(),
+            real_node_bin.display(),
+            std::env::var("PATH").unwrap_or_default()
+        );
+        let script = format!(
+            "eval \"$({} init bash)\"\n{} --version\n",
+            copied_hni.display(),
+            copied_hni.display()
+        );
+
+        let output = Command::new(bash)
+            .arg("-c")
+            .arg(script)
+            .env("PATH", base_path)
+            .env("HOME", &fake_home)
+            .env("XDG_CONFIG_HOME", &fake_config)
+            .env("APPDATA", &fake_config)
+            .output()
+            .expect("failed to run bash version flow");
+
+        assert!(output.status.success());
+        let stdout = String::from_utf8_lossy(&output.stdout);
+        assert!(stdout.contains("node       v99.0.0"));
+        assert!(stdout.contains("agent      npm (99.0.0)"));
+        assert!(stdout.contains("global     npm (99.0.0)"));
+        assert!(
+            !shim_log.exists()
+                || fs::read_to_string(&shim_log)
+                    .unwrap_or_default()
+                    .trim()
+                    .is_empty()
+        );
+
+        let perms = fs::metadata(&bad_shim_node).unwrap().permissions();
+        assert_eq!(perms.mode() & 0o111, 0o111);
+    });
+}
+
 fn set_executable_if_needed(path: &Path) {
     #[cfg(unix)]
     {
