fix(install): link a local workspace member's bin into node_modules/.bin

cli/task_runner.rs

@@ -1,6 +1,7 @@
 // Copyright 2018-2026 the Deno authors. MIT license.
 
 use std::collections::HashMap;
+use std::collections::HashSet;
 use std::ffi::OsStr;
 use std::ffi::OsString;
 use std::io::Write;
@@ -18,6 +19,7 @@ use deno_task_shell::ShellCommand;
 use deno_task_shell::ShellCommandContext;
 use deno_task_shell::ShellPipeReader;
 use deno_task_shell::ShellPipeWriter;
+use node_resolver::BinValue;
 use tokio::task::JoinHandle;
 use tokio::task::LocalSet;
 use tokio_util::sync::CancellationToken;
@@ -533,6 +535,20 @@ pub fn resolve_custom_commands(
   let mut commands = match npm_resolver {
     CliNpmResolver::Byonm(_) => {
       // Walk the bin dirs in order (closest first) and merge; closest wins.
+      //
+      // NOTE: unlike the managed branch below this deliberately routes *every*
+      // entry through `deno run`, including ones `read_bin_value` classified as
+      // `Executable`. A JS bin without a shebang classifies as `Executable`,
+      // and in BYONM mode running it through deno is the only thing that makes
+      // it work, so narrowing this would be a regression. The managed branch
+      // can be stricter because its snapshot packages are already resolved.
+      //
+      // The asymmetry is intentional and observable: a workspace member whose
+      // `bin` points at a JS file with NO shebang works here but, under the
+      // managed resolver, falls through to `PATH` and fails to exec on unix
+      // (`Exec format error`) — same as `npm`/`node`, which also produce a
+      // non-executable shim target in that case. Pinned by the
+      // `root-noshebang` step of `tests/specs/workspaces/workspace_member_bin`.
       let mut commands: HashMap<String, Rc<dyn ShellCommand>> = HashMap::new();
       for bin_dir in bin_dirs {
         for (name, cmd) in
@@ -544,19 +560,91 @@ pub fn resolve_custom_commands(
       commands
     }
     CliNpmResolver::Managed(npm_resolver) => {
-      resolve_managed_npm_commands(node_resolver, npm_resolver)?
+      let mut commands =
+        resolve_managed_npm_commands(node_resolver, npm_resolver)?;
+      // Local workspace members are not part of the npm resolution snapshot,
+      // so a `bin` they declare is only ever visible as a `node_modules/.bin`
+      // entry. Merge those in so they're runnable from a task (#36313).
+      // Prepending the bin dirs to `PATH` isn't enough on its own: on Windows
+      // the entries are plain `<name>`, `<name>.cmd` and `<name>.ps1` files
+      // rather than executables.
+      //
+      // Only `JsFile` entries are merged. An `Executable` entry is a shell
+      // script or a native binary; those already resolve through `PATH` (the
+      // bin dirs are prepended in `prepare_env_vars`) and `deno_task_shell`
+      // honours their shebang, whereas running one with `deno run --ext=js`
+      // would fail with a `SyntaxError`. A workspace member's JS bin still
+      // classifies as `JsFile` on Windows because `windows_shim::generate_sh`
+      // emits `exec node  "$basedir/..." "$@"`, which
+      // `resolve_execution_path_from_npx_shim` matches.
+      //
+      // `seen` holds every name classified in *any* bin dir, whether or not it
+      // ended up merged. A closer `Executable` entry must still shadow a
+      // farther `JsFile` of the same name: the closer one is what `PATH`
+      // resolves to, and a custom command beats `PATH` in `deno_task_shell`,
+      // so inserting the farther entry would invert closest-first precedence.
+      //
+      // KNOWN EXCEPTION — closest-first only holds *among the bin dirs*.
+      // Entries are merged with `or_insert` semantics (the `contains_key`
+      // filter), so the first bin dir to provide a name wins over later ones.
+      // But `commands` was pre-seeded above from the resolution snapshot's
+      // top-level packages, so a root dependency's bin beats a nearer
+      // workspace member's — or a nearer dependency's — bin of the same name,
+      // and it does so outright because a custom command also beats `PATH`.
+      // npm and pnpm run the *nearer* one.
+      //
+      // We accept this rather than fix it here: the snapshot pre-seeding is
+      // pre-existing behaviour that applies to every managed task, so
+      // reordering it would reach well beyond workspaces. The fix belongs
+      // alongside the `TODO(nathanwhit)` on `resolve_managed_npm_commands`
+      // below, which has to stop flattening top-level package bins into
+      // unconditional commands before this can be ordered correctly.
+      let mut seen: HashSet<String> = HashSet::new();
+      for bin_dir in bin_dirs {
+        // Only classify names that aren't already resolved — classifying reads
+        // the entire file.
+        let bin_values = node_resolver
+          .resolve_npm_commands_from_bin_dir_filtered(bin_dir, |name| {
+            !commands.contains_key(name) && !seen.contains(name)
+          });
+        for (name, bin_value) in bin_values {
+          seen.insert(name.clone());
+          let BinValue::JsFile(path) = bin_value else {
+            continue;
+          };
+          commands.insert(
+            name.clone(),
+            Rc::new(NodeModulesFileRunCommand {
+              command_name: name,
+              path,
+            }) as Rc<dyn ShellCommand>,
+          );
+        }
+      }
+      commands
     }
   };
   commands.insert("npm".to_string(), Rc::new(NpmCommand));
   Ok(commands)
 }
 
-/// Builds the list of `node_modules/.bin` directories to consult for a task.
+/// Builds the list of `node_modules/.bin` directories to consult for a task,
+/// ordered closest-first.
 ///
 /// 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`.
+/// `<ancestor>/node_modules/.bin` directory, mirroring the directory *lookup
+/// order* Node, npm, and pnpm use.
+///
+/// For the managed npm resolver the walk is bounded by the workspace root
+/// (the directory holding the root `node_modules`), so a task run with
+/// `--cwd <member>` also sees that member's own `node_modules/.bin` without
+/// picking up unrelated directories above the workspace.
+///
+/// The returned paths are candidates only: they aren't checked for existence,
+/// and the ordering is a property of this list rather than a guarantee about
+/// which executable a task ends up running. See `resolve_custom_commands` for
+/// how the order is applied, and for the case where the resolution snapshot's
+/// top-level packages take precedence over it.
 pub fn resolve_task_node_modules_bin_dirs(
   npm_resolver: &CliNpmResolver,
   cwd: &Path,
@@ -566,10 +654,27 @@ pub fn resolve_task_node_modules_bin_dirs(
       .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(),
+    CliNpmResolver::Managed(npm_resolver) => {
+      let Some(root_node_modules_path) = npm_resolver.root_node_modules_path()
+      else {
+        return Vec::new();
+      };
+      let mut bin_dirs = Vec::new();
+      // When the cwd is outside the workspace only the root `.bin` applies —
+      // don't reach into unrelated `node_modules` directories.
+      if let Some(root_dir) = root_node_modules_path.parent()
+        && cwd.starts_with(root_dir)
+      {
+        bin_dirs.extend(
+          cwd
+            .ancestors()
+            .take_while(|dir| *dir != root_dir)
+            .map(|dir| dir.join("node_modules").join(".bin")),
+        );
+      }
+      bin_dirs.push(root_node_modules_path.join(".bin"));
+      bin_dirs
+    }
   }
 }
 
@@ -599,6 +704,11 @@ fn resolve_managed_npm_commands(
   let mut result = HashMap::new();
   for id in npm_resolver.resolution().top_level_packages() {
     let package_folder = npm_resolver.resolve_pkg_folder_from_pkg_id(&id)?;
+    // TODO(nathanwhit): this discards the `BinValue` that
+    // `resolve_npm_binary_commands_for_package` already computed, so a registry
+    // package whose bin is a native binary or a shell script is still handed to
+    // `deno run --ext=js`. See the `.bin` handling in `resolve_custom_commands`
+    // above for how that distinction should be honoured.
     let bins =
       node_resolver.resolve_npm_binary_commands_for_package(&package_folder)?;
     result.extend(bins.into_iter().map(|(command_name, path)| {

libs/node_resolver/resolution.rs

@@ -1131,6 +1131,22 @@ impl<
   pub fn resolve_npm_commands_from_bin_dir(
     &self,
     bin_dir: &Path,
+  ) -> BTreeMap<String, BinValue> {
+    self.resolve_npm_commands_from_bin_dir_filtered(bin_dir, |_| true)
+  }
+
+  /// Same as [`Self::resolve_npm_commands_from_bin_dir`], but only classifies
+  /// entries whose command name passes `filter`.
+  ///
+  /// Classifying an entry means reading the whole file off disk
+  /// ([`read_bin_value`]), and a `node_modules/.bin` can hold hundreds of
+  /// entries, some of them multi-megabyte bundled CLIs. Callers that already
+  /// know they will discard some of the names should filter here so that I/O
+  /// never happens.
+  pub fn resolve_npm_commands_from_bin_dir_filtered(
+    &self,
+    bin_dir: &Path,
+    filter: impl Fn(&str) -> bool,
   ) -> BTreeMap<String, BinValue> {
     log::debug!("Resolving npm commands in '{}'.", bin_dir.display());
     let mut result = BTreeMap::new();
@@ -1141,7 +1157,7 @@ impl<
             continue;
           };
           if let Some((command, bin_value)) =
-            self.resolve_bin_dir_entry_command(entry)
+            self.resolve_bin_dir_entry_command(entry, &filter)
           {
             result.insert(command, bin_value);
           }
@@ -1157,10 +1173,16 @@ impl<
   fn resolve_bin_dir_entry_command(
     &self,
     entry: TSys::ReadDirEntry,
+    filter: &impl Fn(&str) -> bool,
   ) -> Option<(String, BinValue)> {
     if entry.path().extension().is_some() {
       return None; // only look at files without extensions (even on Windows)
     }
+    let command_name = entry.file_name().to_string_lossy().into_owned();
+    // check the name before touching the file system any further
+    if !filter(&command_name) {
+      return None;
+    }
     let file_type = entry.file_type().ok()?;
     let path = if file_type.is_file() {
       entry.path()
@@ -1169,7 +1191,6 @@ impl<
     } else {
       return None;
     };
-    let command_name = entry.file_name().to_string_lossy().into_owned();
     let bin_value = read_bin_value(&path, &self.sys)?;
     Some((command_name, bin_value))
   }

libs/npm_installer/bin_entries.rs

@@ -39,6 +39,26 @@ fn default_bin_name(package: &NpmResolutionPackage) -> &str {
     .unwrap_or(package.id.nv.name.as_str())
 }
 
+/// The `node_modules/.bin` entry names a package would contribute, after
+/// normalization. Mirrors what [`BinEntries::add`] registers.
+pub fn bin_names<'a>(
+  package: &'a NpmResolutionPackage,
+  extra: &'a NpmPackageExtraInfo,
+) -> Vec<&'a str> {
+  match extra.bin.as_ref() {
+    Some(deno_npm::registry::NpmPackageVersionBinEntry::String(_)) => {
+      vec![default_bin_name(package)]
+    }
+    Some(deno_npm::registry::NpmPackageVersionBinEntry::Map(entries)) => {
+      entries
+        .keys()
+        .map(|name| normalize_bin_name(name))
+        .collect()
+    }
+    None => Vec::new(),
+  }
+}
+
 fn normalize_bin_name(bin_name: &str) -> &str {
   let trimmed = bin_name.trim_end_matches(['/', '\\']);
   // Leave validation of empty or special path component names to package
@@ -86,6 +106,12 @@ impl<'a, TSys: SetupBinEntrySys> BinEntries<'a, TSys> {
     }
   }
 
+  /// Whether some already-added package contributes a `.bin` entry with this
+  /// (normalized) name.
+  pub fn has_bin_name(&self, name: &str) -> bool {
+    self.seen_names.contains_key(name)
+  }
+
   /// Add a new bin entry (package with a bin field)
   pub fn add<'b>(
     &mut self,
@@ -548,6 +574,36 @@ mod test {
     )
   }
 
+  /// A snapshot with `package` as the only top-level package.
+  fn snapshot_with(package: &NpmResolutionPackage) -> NpmResolutionSnapshot {
+    let req = deno_semver::package::PackageReq::from_str(&format!(
+      "{}@{}",
+      package.id.nv.name, package.id.nv.version
+    ))
+    .unwrap();
+    NpmResolutionSnapshot::new(
+      SerializedNpmResolutionSnapshot {
+        root_packages: [(req, package.id.clone())].into_iter().collect(),
+        packages: vec![
+          deno_npm::resolution::SerializedNpmResolutionSnapshotPackage {
+            id: package.id.clone(),
+            system: Default::default(),
+            dist: None,
+            dependencies: Default::default(),
+            optional_dependencies: Default::default(),
+            optional_peer_dependencies: Default::default(),
+            extra: None,
+            is_deprecated: false,
+            has_bin: true,
+            has_scripts: false,
+          },
+        ],
+      }
+      .into_valid()
+      .unwrap(),
+    )
+  }
+
   fn test_dir(name: &str) -> (PathBuf, impl Drop) {
     static COUNTER: std::sync::atomic::AtomicU64 =
       std::sync::atomic::AtomicU64::new(0);
@@ -606,6 +662,52 @@ mod test {
     );
   }
 
+  /// A workspace member's bins are added after every snapshot package so that
+  /// a real dependency wins a name collision (see `add_workspace_bin_entries`
+  /// in local.rs). A collision triggers the depth sort, and because the
+  /// synthetic workspace package is not in the snapshot it gets the maximum
+  /// depth and sorts last. The names here are chosen so the depth ordering is
+  /// what decides: the name tiebreak alone would pick `z-member`.
+  #[test]
+  fn snapshot_package_wins_collision_with_workspace_member() {
+    let sys = sys_traits::impls::RealSys;
+    let dependency = test_package("a-dep");
+    let dep_extra = NpmPackageExtraInfo {
+      bin: Some(deno_npm::registry::NpmPackageVersionBinEntry::Map(
+        [("shared-cli".to_string(), "dep.js".to_string())]
+          .into_iter()
+          .collect(),
+      )),
+      ..Default::default()
+    };
+    // stands in for a workspace member declaring the same bin name
+    let member = test_package("z-member");
+    let member_extra = NpmPackageExtraInfo {
+      bin: Some(deno_npm::registry::NpmPackageVersionBinEntry::Map(
+        [("shared-cli".to_string(), "member.js".to_string())]
+          .into_iter()
+          .collect(),
+      )),
+      ..Default::default()
+    };
+
+    let mut entries = BinEntries::new(sys.with_paths_in_errors());
+    entries.add(
+      &dependency,
+      &dep_extra,
+      PathBuf::from("/node_modules/a-dep"),
+    );
+    entries.add(&member, &member_extra, PathBuf::from("/workspace/z-member"));
+
+    assert_eq!(
+      entries.collect_bin_files(&snapshot_with(&dependency)),
+      vec![(
+        "shared-cli".to_string(),
+        PathBuf::from("/node_modules/a-dep/dep.js")
+      )]
+    );
+  }
+
   #[cfg(unix)]
   #[test]
   fn set_up_bin_entry_uses_normalized_bin_name() {