diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index 47d936f67a2d4..cc08f667e6a22 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -1423,6 +1423,8 @@ module.exports = cls => class Reifier extends cls { // Locations are normalized to forward slashes here because IsolatedNode/IsolatedLink locations are built with path.join, which uses backslashes on Windows. const validKeys = new Set() const nmDirs = new Map() + // Valid bin shim names per node_modules dir, collected from each top-level link's package.bin so the .bin sweep keeps only shims a still-linked package provides. + const binsByDir = new Map() const NM_PREFIX = 'node_modules/' const STORE_MARKER = '/.store/' for (const child of this.idealTree.children.values()) { @@ -1462,6 +1464,19 @@ module.exports = cls => class Reifier extends cls { nmDirs.set(dir, set) } set.add(entry) + + // package.bin is normalized to an object keyed by bin name; shim names are unscoped even for scoped packages. + const bin = child.package?.bin + if (bin && typeof bin === 'object') { + let binSet = binsByDir.get(dir) + if (!binSet) { + binSet = new Set() + binsByDir.set(dir, binSet) + } + for (const bn of Object.keys(bin)) { + binSet.add(bn) + } + } } // Determine which node_modules directories to sweep. @@ -1535,7 +1550,38 @@ module.exports = cls => class Reifier extends cls { for (const [dir, valid] of nmDirs) { await this.#cleanOrphanedTopLevelLinks(dir, valid) + await this.#cleanStaleBinLinks(dir, binsByDir.get(dir)) + } + } + + // Remove stale bin shims left in node_modules/.bin after an uninstall under linked, where the diff never emits an action to drop them. + // A shim is stale when no still-linked package provides its name, or when it is a dangling symlink; matching by name handles both POSIX symlinks and Windows .cmd/.ps1 shims. + async #cleanStaleBinLinks (nmDir, validBins = new Set()) { + const binDir = resolve(nmDir, '.bin') + let names + try { + names = await readdir(binDir) + } catch { + return } + + const stale = names.filter(name => { + const base = name.replace(/\.(cmd|ps1)$/, '') + return !validBins.has(base) || !existsSync(resolve(binDir, name)) + }) + + if (!stale.length) { + return + } + + log.silly('reify', 'cleaning stale bin links', stale) + await promiseAllRejectLate( + stale.map(name => + rm(resolve(binDir, name), { force: true }) + .catch(/* istanbul ignore next -- rm with force rarely fails */ + er => log.warn('cleanup', `Failed to remove stale bin link ${name}`, er)) + ) + ) } // Remove node_modules/ entries that aren't represented in the ideal tree. diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js index 81d91f08f1d95..adb8bab5ae135 100644 --- a/workspaces/arborist/test/arborist/reify.js +++ b/workspaces/arborist/test/arborist/reify.js @@ -4252,6 +4252,40 @@ t.test('install strategy linked', async (t) => { 'packages/b should remain absent after reinstall' ) }) + + t.test('removes stale .bin shims after uninstall, keeps surviving ones', async t => { + // Regression test for https://github.com/npm/cli/issues/9613 + const path = t.testdir({ + 'package.json': JSON.stringify({ name: 'un', version: '1.0.0' }), + }) + createRegistry(t, true) + const binDir = resolve(path, 'node_modules/.bin') + const rbin = resolve(binDir, 'rimraf') + const sbin = resolve(binDir, 'semver') + + await reify(path, { add: ['rimraf@2.7.1', 'semver@7.3.2'], installStrategy: 'linked' }) + // lstatSync throws if missing; don't assert symlink since Windows shims are regular files. + t.ok(fs.lstatSync(rbin), 'rimraf shim created') + t.ok(fs.lstatSync(sbin), 'semver shim created') + + // Plant Windows shim files alongside the POSIX symlinks to cover both layouts: stale (rimraf) and surviving (semver). + const rcmd = resolve(binDir, 'rimraf.cmd') + const rps1 = resolve(binDir, 'rimraf.ps1') + const scmd = resolve(binDir, 'semver.cmd') + const sps1 = resolve(binDir, 'semver.ps1') + fs.writeFileSync(rcmd, '@echo off\r\n"%~dp0\\..\\rimraf\\bin.js" %*\r\n') + fs.writeFileSync(rps1, '& "$basedir/../rimraf/bin.js" $args\r\n') + fs.writeFileSync(scmd, '@echo off\r\n"%~dp0\\..\\semver\\bin\\semver.js" %*\r\n') + fs.writeFileSync(sps1, '& "$basedir/../semver/bin/semver.js" $args\r\n') + + await reify(path, { rm: ['rimraf'], installStrategy: 'linked' }) + t.throws(() => fs.lstatSync(rbin), 'stale rimraf symlink removed') + t.throws(() => fs.lstatSync(rcmd), 'stale rimraf.cmd removed') + t.throws(() => fs.lstatSync(rps1), 'stale rimraf.ps1 removed') + t.ok(fs.lstatSync(sbin), 'surviving semver shim kept') + t.ok(fs.lstatSync(scmd), 'surviving semver.cmd kept') + t.ok(fs.lstatSync(sps1), 'surviving semver.ps1 kept') + }) }) t.test('linked strategy exposes store node_modules via NODE_PATH for lifecycle scripts', async t => {