fix(arborist): prevent workspace bin hoisting collisions

tap-snapshots/workspaces/arborist/test/arborist/rebuild.js.test.cjs

@@ -0,0 +1,87 @@
+/* IMPORTANT
+ * This snapshot file is auto-generated, but designed for humans.
+ * It should be checked into source control and tracked carefully.
+ * Re-generate by setting TAP_SNAPSHOT=1 and running tests.
+ * Make sure to inspect the output below.  Do not ignore changes!
+ */
+'use strict'
+exports[`workspaces/arborist/test/arborist/rebuild.js TAP verify dep flags in script environments > saved script results 1`] = `
+Array [
+  Object {
+    "cmd": "node ../../env.js",
+    "code": 0,
+    "event": "postinstall",
+    "pkg": Object {
+      "_id": "devdep@1.0.0",
+      "dependencies": Object {
+        "devopt": "",
+      },
+      "name": "devdep",
+      "optionalDependencies": Object {
+        "opt-and-dev": "",
+      },
+      "scripts": Object {
+        "postinstall": "node ../../env.js",
+      },
+      "version": "1.0.0",
+    },
+    "signal": null,
+    "stderr": "stderr",
+    "stdout": "npm_package_dev",
+  },
+  Object {
+    "cmd": "node ../../env.js",
+    "code": 0,
+    "event": "postinstall",
+    "pkg": Object {
+      "_id": "devopt@1.0.0",
+      "name": "devopt",
+      "scripts": Object {
+        "postinstall": "node ../../env.js",
+      },
+      "version": "1.0.0",
+    },
+    "signal": null,
+    "stderr": "stderr",
+    "stdout": "npm_package_dev_optional",
+  },
+  Object {
+    "cmd": "node ../../env.js",
+    "code": 0,
+    "event": "postinstall",
+    "pkg": Object {
+      "_id": "opt-and-dev@1.0.0",
+      "name": "opt-and-dev",
+      "scripts": Object {
+        "postinstall": "node ../../env.js",
+      },
+      "version": "1.0.0",
+    },
+    "signal": null,
+    "stderr": "stderr",
+    "stdout": String(
+      npm_package_dev
+      npm_package_optional
+    ),
+  },
+  Object {
+    "cmd": "node ../../env.js",
+    "code": 0,
+    "event": "postinstall",
+    "pkg": Object {
+      "_id": "optdep@1.0.0",
+      "dependencies": Object {
+        "devopt": "",
+      },
+      "name": "optdep",
+      "scripts": Object {
+        "postinstall": "node ../../env.js",
+      },
+      "version": "1.0.0",
+    },
+    "signal": null,
+    "stderr": "stderr",
+    "stdout": "npm_package_optional",
+  },
+]
+`

workspaces/arborist/lib/arborist/rebuild.js

@@ -3,6 +3,9 @@
 
 const PackageJson = require('@npmcli/package-json')
 const binLinks = require('bin-links')
+const isWindows = require('bin-links/lib/is-windows.js')
+/* istanbul ignore next */
+const linkBin = isWindows ? require('bin-links/lib/shim-bin.js') : require('bin-links/lib/link-bin.js')
 const localeCompare = require('@isaacs/string-locale-compare')('en')
 const promiseAllRejectLate = require('promise-all-reject-late')
 const runScript = require('@npmcli/run-script')
@@ -11,7 +14,7 @@ const { depth: dfwalk } = require('treeverse')
 const { isNodeGypPackage, defaultGypInstallScript } = require('@npmcli/node-gyp')
 const { promiseRetry } = require('@gar/promise-retry')
 const { log, time } = require('proc-log')
-const { resolve, delimiter } = require('node:path')
+const { resolve, delimiter, dirname, relative, sep } = require('node:path')
 const { isScriptAllowed } = require('../script-allowed.js')
 
 const boolEnv = b => b ? '1' : ''
@@ -73,9 +76,68 @@ module.exports = cls => class Builder extends cls {
       await this.#build(linkNodes, { type: 'links' })
     }
 
+    if (this.options.binLinks) {
+      await this.#reconcileWorkspaceBins(linkNodes)
+    }
+
     timeEnd()
   }
 
+  async #reconcileWorkspaceBins (rebuiltNodes) {
+    const queue = []
+
+    binLinks.resetSeen()
+
+    for (const node of rebuiltNodes) {
+      if (!node.isWorkspace || !node.isLink) {
+        continue
+      }
+
+      const wsTarget = node.target
+
+      for (const edge of wsTarget.edgesOut.values()) {
+        if (edge.type === 'workspace' || !edge.to) {
+          continue
+        }
+
+        const depNode = edge.to
+
+        // Skip deps physically nested inside the workspace —
+        // #linkAllBins already created correct shims for these.
+        if (depNode.path.startsWith(wsTarget.path + sep)) {
+          continue
+        }
+
+        const pkg = depNode.package
+        /* istanbul ignore next - PackageJson.normalize converts strings to objects, this is defensive */
+        const bins = typeof pkg.bin === 'string'
+          ? { [pkg.name]: pkg.bin }
+          : pkg.bin
+
+        if (!bins || typeof bins !== 'object' || Array.isArray(bins)) {
+          continue
+        }
+
+        for (const [binName, binScript] of Object.entries(bins)) {
+          queue.push(async () => {
+            const to = resolve(wsTarget.path, 'node_modules', '.bin', binName)
+            const absFrom = resolve(depNode.path, binScript)
+            const from = relative(dirname(to), absFrom)
+            await linkBin({ path: depNode.path, from, to, absFrom, force: true })
+          })
+        }
+      }
+    }
+
+    if (queue.length) {
+      const timeEnd = time.start('build:reconcileWorkspaceBins')
+      await promiseCallLimit(queue, {
+        limit: this.options.foregroundScripts ? 1 : undefined,
+      })
+      timeEnd()
+    }
+  }
+
   // if we don't have a set of nodes, then just rebuild
   // the actual tree on disk.
   async #loadDefaultNodes () {

workspaces/arborist/test/arborist/rebuild.js

@@ -904,6 +904,96 @@ t.test('only rebuild for workspace', async t => {
   t.throws(() => fs.readFileSync(bdepTxt, 'utf8'), { code: 'ENOENT' }, 'bdep not rebuilt')
 })
 
+t.test('workspace bin hoisting collision', async t => {
+  const path = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'my-workspaces-powered-project',
+      workspaces: ['packages/*'],
+    }),
+    node_modules: {
+      'dep-1': {
+        'package.json': JSON.stringify({
+          name: 'dep-1',
+          version: '1.0.0',
+          bin: './bin.js',
+        }),
+        'bin.js': '#!/usr/bin/env node\\nconsole.log("dep-1")',
+      },
+      'dep-2': {
+        'package.json': JSON.stringify({
+          name: 'dep-2',
+          version: '1.0.0',
+          bin: { 'my-bin': './bin.js' },
+        }),
+        'bin.js': '#!/usr/bin/env node\\nconsole.log("dep-2")',
+      },
+      'dep-3': {
+        'package.json': JSON.stringify({
+          name: 'dep-3',
+          version: '1.0.0',
+          bin: { 'multi-bin-1': './bin1.js', 'multi-bin-2': './bin2.js' },
+        }),
+        'bin1.js': '#!/usr/bin/env node\\n',
+        'bin2.js': '#!/usr/bin/env node\\n',
+      },
+      'dep-5': {
+        'package.json': JSON.stringify({
+          name: 'dep-5',
+          version: '1.0.0',
+          bin: ['invalid'],
+        }),
+      },
+      workspace_a: t.fixture('symlink', '../packages/workspace_a'),
+      workspace_b: t.fixture('symlink', '../packages/workspace_b'),
+      workspace_c: t.fixture('symlink', '../packages/workspace_c'),
+    },
+    packages: {
+      workspace_a: {
+        'package.json': JSON.stringify({
+          name: 'workspace_a',
+          version: '1.0.0',
+          dependencies: { 'dep-1': '1.0.0', 'dep-5': '1.0.0' },
+        }),
+      },
+      workspace_b: {
+        'package.json': JSON.stringify({
+          name: 'workspace_b',
+          version: '1.0.0',
+          dependencies: { 'dep-2': '1.0.0', 'dep-3': '1.0.0' },
+        }),
+      },
+      workspace_c: {
+        'package.json': JSON.stringify({
+          name: 'workspace_c',
+          version: '1.0.0',
+          dependencies: { 'dep-4': '1.0.0' },
+        }),
+        node_modules: {
+          'dep-4': {
+            'package.json': JSON.stringify({
+              name: 'dep-4',
+              version: '1.0.0',
+              bin: './bin.js',
+            }),
+            'bin.js': '#!/usr/bin/env node\\n',
+          },
+        },
+      },
+    },
+  })
+
+  const arb = newArb({ path, foregroundScripts: true })
+  await arb.rebuild()
+
+  const checkFile = p => fs.statSync(resolve(path, p)).isFile()
+
+  t.ok(checkFile('packages/workspace_a/node_modules/.bin/dep-1'), 'workspace_a local bin created')
+  t.ok(checkFile('packages/workspace_b/node_modules/.bin/my-bin'), 'workspace_b local bin created')
+  t.ok(checkFile('packages/workspace_b/node_modules/.bin/multi-bin-1'), 'workspace_b multi-bin 1 created')
+  t.ok(checkFile('packages/workspace_b/node_modules/.bin/multi-bin-2'), 'workspace_b multi-bin 2 created')
+  t.ok(checkFile('packages/workspace_c/node_modules/.bin/dep-4'), 'workspace_c nested bin created via linkAllBins')
+})
+
 t.test('no workspaces', async t => {
   const path = t.testdir({
     'package.json': JSON.stringify({