fix(task): walk ancestor node_modules/.bin in BYONM mode (#34364)

In pnpm-style workspace layouts, hoisted dependencies live only in the
workspace root's `node_modules/.bin` — the member packages have no
`node_modules` of their own. Running `deno task` from a member failed
with `command not found` for those bins because BYONM resolution
consulted only the single closest `node_modules/.bin`.

Match Node, npm, and pnpm by walking up from the task cwd through every
`<ancestor>/node_modules/.bin`, with the closest entry winning for both
shell command resolution and the `PATH` env var passed to spawned
processes. Managed npm is unaffected since it always uses a single flat
`node_modules` layout.

Closes #25845

Author: bartlomieju

cli/npm.rs

@@ -563,7 +563,9 @@ impl DenoTaskLifeCycleScriptsExecutor {
           custom_commands: custom_commands.clone(),
           init_cwd: options.init_cwd,
           argv: &[],
-          root_node_modules_dir: Some(options.root_node_modules_dir_path),
+          node_modules_bin_dirs: &[options
+            .root_node_modules_dir_path
+            .join(".bin")],
           stdio: Some(crate::task_runner::TaskIo {
             stderr: TaskStdio::piped(),
             stdout: TaskStdio::piped(),

cli/task_runner.rs

@@ -165,7 +165,9 @@ pub struct RunTaskOptions<'a> {
   pub env_vars: HashMap<OsString, OsString>,
   pub argv: &'a [String],
   pub custom_commands: HashMap<String, Rc<dyn ShellCommand>>,
-  pub root_node_modules_dir: Option<&'a Path>,
+  /// Directories to prepend to `PATH` for the duration of the task,
+  /// ordered closest-first (closest entry ends up first in `PATH`).
+  pub node_modules_bin_dirs: &'a [PathBuf],
   pub stdio: Option<TaskIo>,
   pub kill_signal: KillSignal,
 }
@@ -185,7 +187,7 @@ pub async fn run_task(
   let seq_list = deno_task_shell::parser::parse(&script)
     .with_context(|| format!("Error parsing script '{}'.", opts.task_name))?;
   let env_vars =
-    prepare_env_vars(opts.env_vars, opts.init_cwd, opts.root_node_modules_dir);
+    prepare_env_vars(opts.env_vars, opts.init_cwd, opts.node_modules_bin_dirs);
   if !opts.custom_commands.contains_key("deno") {
     opts
       .custom_commands
@@ -244,7 +246,7 @@ pub async fn run_task(
 fn prepare_env_vars(
   mut env_vars: HashMap<OsString, OsString>,
   initial_cwd: &Path,
-  node_modules_dir: Option<&Path>,
+  node_modules_bin_dirs: &[PathBuf],
 ) -> HashMap<OsString, OsString> {
   const INIT_CWD_NAME: &str = "INIT_CWD";
   if !env_vars.contains_key(OsStr::new(INIT_CWD_NAME)) {
@@ -262,11 +264,9 @@ fn prepare_env_vars(
       crate::npm::get_npm_config_user_agent().into(),
     );
   }
-  if let Some(node_modules_dir) = node_modules_dir {
-    prepend_to_path(
-      &mut env_vars,
-      node_modules_dir.join(".bin").into_os_string(),
-    );
+  // Prepend in reverse so the closest dir ends up first in `PATH`.
+  for bin_dir in node_modules_bin_dirs.iter().rev() {
+    prepend_to_path(&mut env_vars, bin_dir.as_os_str().to_os_string());
   }
   env_vars
 }
@@ -533,12 +533,20 @@ impl ShellCommand for NodeModulesFileRunCommand {
 pub fn resolve_custom_commands(
   node_resolver: &CliNodeResolver,
   npm_resolver: &CliNpmResolver,
+  bin_dirs: &[PathBuf],
 ) -> Result<HashMap<String, Rc<dyn ShellCommand>>, AnyError> {
   let mut commands = match npm_resolver {
-    CliNpmResolver::Byonm(npm_resolver) => {
-      let node_modules_dir = npm_resolver.root_node_modules_path().unwrap();
-      let bin_dir = node_modules_dir.join(".bin");
-      resolve_npm_commands_from_bin_dir(&bin_dir, node_resolver)
+    CliNpmResolver::Byonm(_) => {
+      // Walk the bin dirs in order (closest first) and merge; closest wins.
+      let mut commands: HashMap<String, Rc<dyn ShellCommand>> = HashMap::new();
+      for bin_dir in bin_dirs {
+        for (name, cmd) in
+          resolve_npm_commands_from_bin_dir(bin_dir, node_resolver)
+        {
+          commands.entry(name).or_insert(cmd);
+        }
+      }
+      commands
     }
     CliNpmResolver::Managed(npm_resolver) => {
       resolve_managed_npm_commands(node_resolver, npm_resolver)?
@@ -548,6 +556,28 @@ pub fn resolve_custom_commands(
   Ok(commands)
 }
 
+/// Builds the list of `node_modules/.bin` directories to consult for a task.
+///
+/// For BYONM this walks up the filesystem from `cwd` collecting every
+/// `<ancestor>/node_modules/.bin` directory (matching how Node, npm, and pnpm
+/// resolve bin commands). For the managed npm resolver there is only ever a
+/// single `node_modules/.bin`.
+pub fn resolve_task_node_modules_bin_dirs(
+  npm_resolver: &CliNpmResolver,
+  cwd: &Path,
+) -> Vec<PathBuf> {
+  match npm_resolver {
+    CliNpmResolver::Byonm(_) => cwd
+      .ancestors()
+      .map(|dir| dir.join("node_modules").join(".bin"))
+      .collect(),
+    CliNpmResolver::Managed(npm_resolver) => npm_resolver
+      .root_node_modules_path()
+      .map(|p| vec![p.join(".bin")])
+      .unwrap_or_default(),
+  }
+}
+
 pub fn resolve_npm_commands_from_bin_dir(
   bin_dir: &Path,
   node_resolver: &CliNodeResolver,

cli/tools/task.rs

@@ -318,6 +318,7 @@ struct RunSingleOptions<'a> {
   script: &'a str,
   cwd: PathBuf,
   custom_commands: HashMap<String, Rc<dyn ShellCommand>>,
+  node_modules_bin_dirs: Vec<PathBuf>,
   kill_signal: KillSignal,
   argv: &'a [String],
   parallel_info: Option<ParallelInfo>,
@@ -566,9 +567,12 @@ impl<'a> TaskRunner<'a> {
       }
     };
 
+    let node_modules_bin_dirs =
+      task_runner::resolve_task_node_modules_bin_dirs(self.npm_resolver, &cwd);
     let custom_commands = task_runner::resolve_custom_commands(
       self.node_resolver,
       self.npm_resolver,
+      &node_modules_bin_dirs,
     )?;
 
     self
@@ -578,6 +582,7 @@ impl<'a> TaskRunner<'a> {
         script: command,
         cwd,
         custom_commands,
+        node_modules_bin_dirs,
         kill_signal,
         argv,
         parallel_info,
@@ -615,9 +620,12 @@ impl<'a> TaskRunner<'a> {
       task_name.to_string(),
       format!("post{}", task_name),
     ];
+    let node_modules_bin_dirs =
+      task_runner::resolve_task_node_modules_bin_dirs(self.npm_resolver, &cwd);
     let custom_commands = task_runner::resolve_custom_commands(
       self.node_resolver,
       self.npm_resolver,
+      &node_modules_bin_dirs,
     )?;
 
     for task_name in &task_names {
@@ -629,6 +637,7 @@ impl<'a> TaskRunner<'a> {
             script,
             cwd: cwd.to_path_buf(),
             custom_commands: custom_commands.clone(),
+            node_modules_bin_dirs: node_modules_bin_dirs.clone(),
             kill_signal: kill_signal.clone(),
             argv,
             parallel_info,
@@ -653,6 +662,7 @@ impl<'a> TaskRunner<'a> {
       script,
       cwd,
       custom_commands,
+      node_modules_bin_dirs,
       kill_signal,
       argv,
       parallel_info,
@@ -686,7 +696,7 @@ impl<'a> TaskRunner<'a> {
       custom_commands,
       init_cwd: self.cli_options.initial_cwd(),
       argv,
-      root_node_modules_dir: self.npm_resolver.root_node_modules_path(),
+      node_modules_bin_dirs: &node_modules_bin_dirs,
       stdio,
       kill_signal,
     })

tests/specs/task/byonm_workspace_ancestor_bin/__test__.jsonc

@@ -0,0 +1,18 @@
+{
+  "tempDir": true,
+  "tests": {
+    // pnpm-style workspace layout: root has node_modules/.bin/cowsay, but
+    // packages/child has no node_modules of its own. Running a task from
+    // the child must still resolve the bin by walking ancestors.
+    "child_finds_root_bin": {
+      "steps": [{
+        "commandName": "npm",
+        "args": "install",
+        "output": "[WILDCARD]"
+      }, {
+        "args": "task -f child say",
+        "output": "child_say.out"
+      }]
+    }
+  }
+}

tests/specs/task/byonm_workspace_ancestor_bin/child_say.out

@@ -0,0 +1,9 @@
+Task say (child) cowsay hi
+ ____
+< hi >
+ ----
+        \   ^__^
+         \  (oo)\_______
+            (__)\       )\/\
+                ||----w |
+                ||     ||

tests/specs/task/byonm_workspace_ancestor_bin/deno.json

@@ -0,0 +1,4 @@
+{
+  "workspace": ["./packages/child"],
+  "unstable": ["byonm"]
+}

tests/specs/task/byonm_workspace_ancestor_bin/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "root",
+  "private": true,
+  "dependencies": {
+    "cowsay": "*"
+  }
+}

tests/specs/task/byonm_workspace_ancestor_bin/packages/child/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "child",
+  "version": "1.0.0",
+  "scripts": {
+    "say": "cowsay hi"
+  }
+}