fix(libnpmexec): correctly resolve workspace dependency binaries from graph

workspaces/libnpmexec/lib/index.js

@@ -19,8 +19,6 @@ const runScript = require('./run-script.js')
 const isWindows = require('./is-windows.js')
 const withLock = require('./with-lock.js')
 
-const binPaths = []
-
 // when checking the local tree we look up manifests, cache those results by
 // spec.raw so we don't have to fetch again when we check npxCache
 const manifests = new Map()
@@ -113,6 +111,8 @@ const exec = async (opts) => {
     ...flatOptions
   } = opts
 
+  const binPaths = []
+
   let pkgPaths = opts.pkgPath
   if (typeof pkgPaths === 'string') {
     pkgPaths = [pkgPaths]
@@ -137,6 +137,13 @@ const exec = async (opts) => {
     return run()
   }
 
+  // Detect workspace context before appending the root path.
+  // In workspace mode, the CLI explicitly sets pkgPath to the workspace dir.
+  // When pkgPath is not provided (undefined), we're not in workspace context.
+  const explicitPkgPath = opts.pkgPath
+  const inWorkspaceContext = explicitPkgPath != null
+    && resolve(String(explicitPkgPath)) !== resolve(path)
+
   // Look in the local tree too
   pkgPaths.push(path)
 
@@ -146,6 +153,7 @@ const exec = async (opts) => {
   // - in any local packages (pkgPaths can have workspaces in them or just the root)
   // - in the local tree (path)
   // - globally
+  let localTree
   if (needPackageCommandSwap) {
     // Local packages and local tree
     for (const p of pkgPaths) {
@@ -163,18 +171,58 @@ const exec = async (opts) => {
       }
     }
     if (needPackageCommandSwap) {
-      // no bin entry in local packages or in tree, now we look for binPaths
-      const dir = dirname(dirname(localBin))
-      const localBinPath = await localFileExists(dir, args[0], '/')
-      if (localBinPath) {
-        binPaths.push(localBinPath)
-        return await run()
-      } else if (globalPath && await fileExists(`${globalBin}/${args[0]}`)) {
-        binPaths.push(globalBin)
+      const localArb = new Arborist({ ...flatOptions, path })
+      localTree = await localArb.loadActual()
+
+      let targetNode = null
+      for (const p of pkgPaths) {
+        let wsNode = localTree
+        for (const node of localTree.inventory.values()) {
+          if (node.path === p) {
+            wsNode = node; break
+          }
+        }
+
+        // If we didn't find the workspace node in the inventory, and it's not the root path, skip it.
+        if (wsNode === localTree && p !== path) {
+          continue
+        }
+
+        for (const edge of wsNode.edgesOut.values()) {
+          if (edge.to?.package?.bin?.[args[0]]) {
+            targetNode = edge.to
+            break
+          }
+        }
+        if (targetNode) {
+          break
+        }
+      }
+
+      if (targetNode) {
+        const binFile = targetNode.package.bin[args[0]]
+        const execNode = isWindows ? `"${process.execPath}"` : process.execPath
+        args.splice(0, 1, execNode, resolve(targetNode.path, binFile))
         return await run()
+      } else {
+        // In workspace context, the dep-graph lookup is authoritative —
+        // don't fall back to the hoisted .bin which may contain bins from
+        // other workspaces' dependencies that this workspace doesn't declare.
+        if (!inWorkspaceContext) {
+          const dir = dirname(dirname(localBin))
+          const localBinPath = await localFileExists(dir, args[0], '/')
+          if (localBinPath) {
+            binPaths.push(localBinPath)
+            return await run()
+          }
+        }
+        if (globalPath && await fileExists(`${globalBin}/${args[0]}`)) {
+          binPaths.push(globalBin)
+          return await run()
+        }
+        // We swap out args[0] with the bin from the manifest later
+        packages.push(args[0])
       }
-      // We swap out args[0] with the bin from the manifest later
-      packages.push(args[0])
     }
   }
 
@@ -188,15 +236,21 @@ const exec = async (opts) => {
     }
   }
 
-  const localArb = new Arborist({ ...flatOptions, path })
-  const localTree = await localArb.loadActual()
+  if (!localTree) {
+    const localArb = new Arborist({ ...flatOptions, path })
+    localTree = await localArb.loadActual()
+  }
 
   // Find anything that isn't installed locally
   const needInstall = []
   let commandManifest
   await Promise.all(packages.map(async (pkg, i) => {
     const spec = npa(pkg, path)
-    const { manifest, node } = await missingFromTree({ spec, tree: localTree, flatOptions })
+    const { manifest, node } = await missingFromTree({
+      spec,
+      tree: localTree,
+      flatOptions,
+    })
     if (manifest) {
       // Package does not exist in the local tree
       needInstall.push({ spec, manifest })

workspaces/libnpmexec/test/index.js

@@ -14,3 +14,178 @@ t.test('no args', async t => {
 
   await exec()
 })
+
+t.test('resolves binary from workspace dependencies', async t => {
+  t.plan(2)
+
+  const { exec, path } = setup(t, {
+    testdir: {
+      'tool-v1': {
+        'package.json': { name: 'tool-v1', version: '1.0.0', bin: { 'shared-bin': 'cli.js' } },
+        'cli.js': { key: 'tool-v1', value: 'hello' },
+      },
+      packages: {
+        a: {},
+      },
+    },
+    mocks: {
+      '../../lib/is-windows.js': false,
+      '@npmcli/arborist': class MockArborist {
+        constructor (options) {
+          this.path = options.path
+        }
+
+        async loadActual () {
+          const resolve = require('path').resolve
+          const wsPath = resolve(this.path, 'packages/a')
+          const wsNode = {
+            path: wsPath,
+            name: 'workspace-a',
+            edgesOut: new Map([
+              ['tool-v1', {
+                to: {
+                  path: `${this.path}/tool-v1`,
+                  package: { name: 'tool-v1', bin: { 'shared-bin': 'cli.js' } },
+                },
+              }],
+            ]),
+          }
+          const inventory = new Map([
+            [wsPath, wsNode],
+          ])
+          inventory.query = () => []
+          return {
+            inventory,
+            edgesOut: new Map(),
+          }
+        }
+
+        async reify () {}
+      },
+      '../../lib/run-script': ({ args }) => {
+        t.equal(args[0], process.execPath, 'args[0] should be process.execPath')
+        t.match(args[1], /tool-v1[/\\]cli\.js$/, 'args[1] should point to correct cli.js')
+      },
+    },
+  })
+
+  const resolve = require('path').resolve
+  await exec({ pkgPath: resolve(path, 'packages/a'), args: ['shared-bin'] })
+})
+
+t.test('resolves binary from workspace dependencies (windows)', async t => {
+  t.plan(2)
+
+  const { exec, path } = setup(t, {
+    testdir: {
+      'tool-v1': {
+        'package.json': { name: 'tool-v1', version: '1.0.0', bin: { 'shared-bin': 'cli.js' } },
+        'cli.js': { key: 'tool-v1', value: 'hello' },
+      },
+      packages: {
+        a: {},
+      },
+    },
+    mocks: {
+      '../../lib/is-windows.js': true,
+      '@npmcli/arborist': class MockArborist {
+        constructor (options) {
+          this.path = options.path
+        }
+
+        async loadActual () {
+          const resolve = require('path').resolve
+          const wsPath = resolve(this.path, 'packages/a')
+          const wsNode = {
+            path: wsPath,
+            name: 'workspace-a',
+            edgesOut: new Map([
+              ['tool-v1', {
+                to: {
+                  path: `${this.path}/tool-v1`,
+                  package: { name: 'tool-v1', bin: { 'shared-bin': 'cli.js' } },
+                },
+              }],
+            ]),
+          }
+          const inventory = new Map([
+            [wsPath, wsNode],
+          ])
+          inventory.query = () => []
+          return {
+            inventory,
+            edgesOut: new Map(),
+          }
+        }
+
+        async reify () {}
+      },
+      '../../lib/run-script': ({ args }) => {
+        t.equal(args[0], `"${process.execPath}"`, 'args[0] should be securely quoted on Windows')
+        t.match(args[1], /tool-v1[/\\]cli\.js$/, 'args[1] should point to correct cli.js')
+      },
+    },
+  })
+
+  const resolve = require('path').resolve
+  await exec({ pkgPath: resolve(path, 'packages/a'), args: ['shared-bin'] })
+})
+
+t.test('does not fallback to hoisted .bin for missing workspace dep', async t => {
+  t.plan(1)
+
+  const { exec, path } = setup(t, {
+    testdir: {
+      packages: {
+        a: {},
+      },
+    },
+    mocks: {
+      '@npmcli/arborist': class MockArborist {
+        constructor (options) {
+          this.path = options.path
+        }
+
+        async loadActual () {
+          const resolve = require('path').resolve
+          const wsPath = resolve(this.path, 'packages/a')
+          const wsNode = {
+            path: wsPath,
+            name: 'workspace-a',
+            edgesOut: new Map(),
+          }
+          const inventory = new Map([
+            [wsPath, wsNode],
+          ])
+          inventory.query = () => []
+          return {
+            inventory,
+            edgesOut: new Map(),
+          }
+        }
+
+        async reify () {}
+      },
+      '../../lib/file-exists.js': {
+        localFileExists: async () => {
+          t.fail('localFileExists should not be called in workspace context for undeclared bins')
+          return null
+        },
+        fileExists: async () => false,
+      },
+      pacote: {
+        manifest: async () => {
+          throw new Error('ABORT_TEST')
+        },
+      },
+    },
+  })
+
+  const resolve = require('path').resolve
+  try {
+    await exec({ pkgPath: resolve(path, 'packages/a'), args: ['missing-bin'] })
+    t.fail('should have aborted')
+  } catch (err) {
+    t.equal(err.message, 'ABORT_TEST', 'skipped localFileExists and proceeded to missingFromTree')
+  }
+})