From 320da59c5bb5333add8bc236ce03133258aa2213 Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Wed, 6 May 2026 20:36:42 +0530 Subject: [PATCH] fix(arborist): clean up orphan top-level symlinks in linked strategy (#9309) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In continuation of our exploration of using `install-strategy=linked` in the [Gutenberg monorepo](https://github.com/WordPress/gutenberg/pull/75814), which powers the WordPress Block Editor. When using `install-strategy=linked`, removing a dependency leaves a dangling symlink at `node_modules/` (and `/node_modules/` for workspace deps). The store entry under `node_modules/.store/@…` is correctly cleaned up by `#cleanOrphanedStoreEntries`, but the top-level link pointing into it is left behind, so `require('')` fails with `Cannot find module` even though the entry still appears in `node_modules/`. The root cause is the same family of issue as #9106. `#buildLinkedActualForDiff` builds the synthetic actual tree from the ideal tree, so any dependency that exists on disk but is no longer in the ideal tree is never compared, and the diff produces no REMOVE action for its top-level symlink. Fixed by extending `#cleanOrphanedStoreEntries` to also collect, per `node_modules` directory (root and each workspace), the set of valid top-level link names from the ideal tree, then sweeping each directory and removing any symlink whose name is not in that set. The root `node_modules` and every workspace's `node_modules` (via `idealTree.fsChildren`) are always seeded into the sweep, so the case of removing the last dependency from the project root or from a workspace still triggers cleanup, including when the workspace itself is declared as a root dependency and therefore has its self-link at the root rather than under its own `node_modules`. The sweep is restricted to symlinks whose target resolves inside the project root, so it covers both store links (e.g. `node_modules/eslint -> .store/...`) and workspace self-links that no longer belong (e.g. `node_modules/a -> ../packages/a` after `a` is undeclared) without touching symlinks that point outside the project, such as those created by `npm link ` without `--save`. Real directories and npm-managed entries (`.bin`, `.store`, `.package-lock.json`) are left alone. The workspace self-link inside its own `node_modules` (e.g. `packages/a/node_modules/a -> ..`) is in the ideal tree as a non-store link, so it's preserved. The sweep also respects the install mode: - It is skipped entirely for `dryRun` and `packageLockOnly` installs, both of which short-circuit `#reifyPackages` and must not mutate `node_modules`. - For workspace-filtered installs (`npm install -w --install-strategy=linked`), the set of `node_modules` directories to sweep is restricted to the workspaces named in `--workspace`, so dropped dependencies from the in-scope workspace get cleaned up while out-of-scope workspaces and the project root are left untouched. `IsolatedNode`/`IsolatedLink` locations are built with `path.join`, which uses backslashes on Windows; locations are normalized to forward slashes inside the sweep so the parser works on both POSIX and Windows. ## Trade-off This aligns the linked strategy with npm's normal `node_modules`-is-managed model: any in-project symlink that isn't in the ideal tree is treated as orphaned, matching what happens today under the default install strategy. A consequence is that hand-made or unsaved `npm link` symlinks pointing to other paths inside the project root (e.g. `node_modules/foo -> ../examples/foo`) are also swept, since npm doesn't currently record which links it owns and they are indistinguishable from workspace self-links by target alone. A more discriminating ownership check (recording managed link names in the hidden lockfile and only sweeping those) is a worthwhile follow-up but materially larger than this fix. ## References Fixes #9308 Related to #9106 (cherry picked from commit 076551b8724741b0c615b26d44595348c3111dc2) --- workspaces/arborist/lib/arborist/reify.js | 166 ++++++++- workspaces/arborist/test/isolated-mode.js | 391 ++++++++++++++++++++++ 2 files changed, 546 insertions(+), 11 deletions(-) diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index 26ad0016be3a9..98335d6f2e8f1 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -11,7 +11,7 @@ const { depth: dfwalk } = require('treeverse') const { dirname, resolve, relative, join, sep } = require('node:path') const { log, time } = require('proc-log') const { existsSync } = require('node:fs') -const { lstat, mkdir, readdir, rm, symlink } = require('node:fs/promises') +const { lstat, mkdir, readdir, readlink, rm, symlink } = require('node:fs/promises') const { moveFile } = require('@npmcli/fs') const { subset, intersects } = require('semver') const { walkUp } = require('walk-up-path') @@ -126,7 +126,11 @@ module.exports = cls => class Reifier extends cls { await this[_diffTrees]() await this.#reifyPackages() if (linked) { - await this.#cleanOrphanedStoreEntries() + // The sweep mutates node_modules on disk, so skip it for dry runs and lockfile-only installs (those modes also short-circuit #reifyPackages). + // The sweep itself scopes to in-filter workspaces when a filter is active, so it's safe to run for filtered installs too. + if (!this.options.dryRun && !this.options.packageLockOnly) { + await this.#cleanOrphanedStoreEntries() + } // swap back in the idealTree // so that the lockfile is preserved this.idealTree = oldTree @@ -1321,35 +1325,175 @@ module.exports = cls => class Reifier extends cls { // After a linked install, scan node_modules/.store/ and remove any directories that are not referenced by the current ideal tree. // Store entries become orphaned when dependencies are updated or removed, because the diff never sees the old store keys. + // Then sweep the top-level node_modules/ for orphaned symlinks (e.g. an uninstalled dep whose store entry was just removed) so we don't leave dangling links. async #cleanOrphanedStoreEntries () { - const storeDir = resolve(this.path, 'node_modules', '.store') + const nmDir = resolve(this.path, 'node_modules') + const storeDir = resolve(nmDir, '.store') + let entries try { entries = await readdir(storeDir) } catch { - return + entries = null } - // Collect valid store keys from the isolated ideal tree (location: node_modules/.store/{key}/node_modules/{pkg}) + // Collect valid store keys and valid top-level links per node_modules directory. + // Store entries have location node_modules/.store/{key}/node_modules/{pkg}. + // Top-level links have location {prefix}/node_modules/{pkg} or {prefix}/node_modules/@scope/{pkg}, where {prefix} is empty for the root project and the workspace's localLocation for workspace deps. + // 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() + const NM_PREFIX = 'node_modules/' + const STORE_MARKER = '/.store/' for (const child of this.idealTree.children.values()) { + const loc = child.location.replace(/\\/g, '/') if (child.isInStore) { - const key = child.location.split(sep)[2] + const key = loc.split('/')[2] validKeys.add(key) + continue + } + if (!child.isLink) { + continue + } + const nmIdx = loc.lastIndexOf(NM_PREFIX) + if (nmIdx === -1 || loc.includes(STORE_MARKER)) { + continue + } + const prefix = loc.slice(0, nmIdx) + const dir = resolve(this.path, prefix, 'node_modules') + const rest = loc.slice(nmIdx + NM_PREFIX.length) + let entry + if (rest.startsWith('@')) { + const [scope, name] = rest.split('/') + entry = `${scope}${sep}${name}` + } else { + entry = rest.split('/')[0] + } + let set = nmDirs.get(dir) + if (!set) { + set = new Set() + nmDirs.set(dir, set) + } + set.add(entry) + } + + // Determine which node_modules directories to sweep. + // For an unfiltered install, sweep the project root and every workspace's node_modules even if no top-level links remain (e.g. last dep was just uninstalled). + // For a filtered install (npm install -w ), restrict the sweep to the in-scope workspaces so out-of-scope workspaces are left untouched, mirroring what the diff would do. + // When --include-workspace-root is set, the filter scope pulls in root deps too, so the root node_modules is included in the sweep. + const filteredNames = this.options.workspaces + const isFiltered = Array.isArray(filteredNames) && filteredNames.length > 0 + if (isFiltered) { + const allowedDirs = new Set() + for (const ws of this.idealTree.fsChildren) { + if (filteredNames.includes(ws.packageName) || filteredNames.includes(ws.name)) { + allowedDirs.add(resolve(ws.path, 'node_modules')) + } + } + if (this.options.includeWorkspaceRoot) { + allowedDirs.add(nmDir) + } + for (const dir of [...nmDirs.keys()]) { + if (!allowedDirs.has(dir)) { + nmDirs.delete(dir) + } + } + for (const dir of allowedDirs) { + if (!nmDirs.has(dir)) { + nmDirs.set(dir, new Set()) + } + } + } else { + if (!nmDirs.has(nmDir)) { + nmDirs.set(nmDir, new Set()) + } + for (const ws of this.idealTree.fsChildren) { + const wsNmDir = resolve(ws.path, 'node_modules') + if (!nmDirs.has(wsNmDir)) { + nmDirs.set(wsNmDir, new Set()) + } + } + } + + if (entries) { + const orphaned = entries.filter(e => !validKeys.has(e)) + if (orphaned.length) { + log.silly('reify', 'cleaning orphaned store entries', orphaned) + await promiseAllRejectLate( + orphaned.map(e => + rm(resolve(storeDir, e), { recursive: true, force: true }) + .catch(/* istanbul ignore next -- rm with force rarely fails */ + er => log.warn('cleanup', `Failed to remove orphaned store entry ${e}`, er)) + ) + ) + } + } + + for (const [dir, valid] of nmDirs) { + await this.#cleanOrphanedTopLevelLinks(dir, valid) + } + } + + // Remove node_modules/ entries that aren't represented in the ideal tree. + // Run for the project root and each workspace's node_modules. + // The linked diff path can't see these because #buildLinkedActualForDiff derives the actual tree from the ideal, so removed deps are never compared. + // Only symlinks whose target resolves inside the project root are removed — that covers store links (node_modules/.store/...) and workspace self-links (e.g. node_modules/ -> ../packages/) that npm itself created. + // Symlinks pointing outside the project (e.g. `npm link foo` without --save targeting the global prefix, or hand-made `ln -s` to an external path) and real directories are preserved. + async #cleanOrphanedTopLevelLinks (nmDir, validTopLevel) { + const projectPrefix = resolve(this.path) + sep + let dirents + try { + dirents = await readdir(nmDir, { withFileTypes: true }) + } catch { + return + } + + const isOurOrphan = async (linkPath) => { + let target + try { + target = await readlink(linkPath) + } catch { + /* istanbul ignore next -- readlink of an entry we just listed as a symlink should not fail */ + return false + } + return resolve(dirname(linkPath), target).startsWith(projectPrefix) + } + + const orphaned = [] + for (const ent of dirents) { + // skip npm-managed entries (.bin, .store, .package-lock.json, etc) + if (ent.name.startsWith('.')) { + continue + } + if (ent.name.startsWith('@')) { + let scoped + try { + scoped = await readdir(resolve(nmDir, ent.name), { withFileTypes: true }) + } catch { + /* istanbul ignore next -- readdir of an entry we just listed should not fail */ + continue + } + for (const pkgEnt of scoped) { + const key = `${ent.name}${sep}${pkgEnt.name}` + if (!validTopLevel.has(key) && pkgEnt.isSymbolicLink() && await isOurOrphan(resolve(nmDir, key))) { + orphaned.push(key) + } + } + } else if (!validTopLevel.has(ent.name) && ent.isSymbolicLink() && await isOurOrphan(resolve(nmDir, ent.name))) { + orphaned.push(ent.name) } } - const orphaned = entries.filter(e => !validKeys.has(e)) if (!orphaned.length) { return } - log.silly('reify', 'cleaning orphaned store entries', orphaned) + log.silly('reify', 'cleaning orphaned top-level links', orphaned) await promiseAllRejectLate( - orphaned.map(e => - rm(resolve(storeDir, e), { recursive: true, force: true }) + orphaned.map(name => + rm(resolve(nmDir, name), { recursive: true, force: true }) .catch(/* istanbul ignore next -- rm with force rarely fails */ - er => log.warn('cleanup', `Failed to remove orphaned store entry ${e}`, er)) + er => log.warn('cleanup', `Failed to remove orphaned link ${name}`, er)) ) ) } diff --git a/workspaces/arborist/test/isolated-mode.js b/workspaces/arborist/test/isolated-mode.js index 3fb74f2ffc29a..57aff712af4c9 100644 --- a/workspaces/arborist/test/isolated-mode.js +++ b/workspaces/arborist/test/isolated-mode.js @@ -2028,6 +2028,397 @@ tap.test('orphaned store entries are cleaned up on dependency removal', async t const entriesAfterRemoval = fs.readdirSync(storeDir) t.equal(entriesAfterRemoval.length, 0, 'all store entries are removed when dependencies are removed') + + // https://github.com/npm/cli/issues/9308 — the top-level node_modules symlink for the removed dep was left behind, dangling into the just-cleaned store. + t.notOk(fs.existsSync(path.join(dir, 'node_modules', 'which')), + 'top-level symlink for removed dependency is also cleaned up') +}) + +tap.test('orphaned link inside workspace node_modules is cleaned up on dependency removal', async t => { + const graph = { + registry: [ + { name: 'abbrev', version: '4.0.0' }, + ], + root: { + name: 'root', + version: '1.0.0', + }, + workspaces: [ + { name: 'a', version: '1.0.0', dependencies: { abbrev: '^4.0.0' } }, + ], + } + const { dir, registry } = await getRepo(graph) + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + + const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb1.reify({ installStrategy: 'linked' }) + + const wsLink = path.join(dir, 'packages', 'a', 'node_modules', 'abbrev') + t.ok(fs.existsSync(wsLink), 'abbrev is linked into workspace node_modules') + + // Drop abbrev from the workspace package.json + const wsPkgPath = path.join(dir, 'packages', 'a', 'package.json') + const wsPkg = JSON.parse(fs.readFileSync(wsPkgPath, 'utf8')) + delete wsPkg.dependencies + fs.writeFileSync(wsPkgPath, JSON.stringify(wsPkg)) + + const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb2.reify({ installStrategy: 'linked' }) + + t.notOk(fs.existsSync(wsLink), 'abbrev symlink target no longer resolves') + t.notOk(fs.lstatSync(wsLink, { throwIfNoEntry: false }), + 'abbrev symlink itself is removed from workspace node_modules') +}) + +tap.test('orphaned link in declared-workspace node_modules is cleaned up when last dep is removed', async t => { + // Reproduces the case where the workspace is also a root dependency, so its self-link sits at the ROOT node_modules and the workspace's own node_modules has no surviving links after removing its only dep. + // Without explicitly seeding each workspace's node_modules into the sweep map, that directory would never be visited and the orphan symlink would remain. + const graph = { + registry: [ + { name: 'abbrev', version: '4.0.0' }, + ], + root: { + name: 'root', + version: '1.0.0', + dependencies: { a: '*' }, + }, + workspaces: [ + { name: 'a', version: '1.0.0', dependencies: { abbrev: '^4.0.0' } }, + ], + } + const { dir, registry } = await getRepo(graph) + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + + const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb1.reify({ installStrategy: 'linked' }) + + const wsLink = path.join(dir, 'packages', 'a', 'node_modules', 'abbrev') + t.ok(fs.lstatSync(wsLink, { throwIfNoEntry: false }), 'abbrev is linked into workspace node_modules') + + const wsPkgPath = path.join(dir, 'packages', 'a', 'package.json') + const wsPkg = JSON.parse(fs.readFileSync(wsPkgPath, 'utf8')) + delete wsPkg.dependencies + fs.writeFileSync(wsPkgPath, JSON.stringify(wsPkg)) + + const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb2.reify({ installStrategy: 'linked' }) + + t.notOk(fs.lstatSync(wsLink, { throwIfNoEntry: false }), + 'abbrev symlink is removed even though the workspace itself is the only declared root dep') +}) + +tap.test('orphan sweep is skipped on dryRun and packageLockOnly linked installs', async t => { + // The sweep mutates node_modules; dry-run and package-lock-only installs must not touch the filesystem. + const graph = { + registry: [ + { name: 'abbrev', version: '4.0.0' }, + ], + root: { + name: 'myproject', + version: '1.0.0', + dependencies: { abbrev: '^4.0.0' }, + }, + } + const { dir, registry } = await getRepo(graph) + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + + const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb1.reify({ installStrategy: 'linked' }) + const linkPath = path.join(dir, 'node_modules', 'abbrev') + t.ok(fs.lstatSync(linkPath, { throwIfNoEntry: false }), 'abbrev link present after first install') + + // Drop the dep, then run dryRun and packageLockOnly — neither should remove the orphan + const pkgPath = path.join(dir, 'package.json') + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')) + delete pkg.dependencies + fs.writeFileSync(pkgPath, JSON.stringify(pkg)) + + const arbDry = new Arborist({ path: dir, registry, packumentCache: new Map(), cache, dryRun: true }) + await arbDry.reify({ installStrategy: 'linked', dryRun: true }) + t.ok(fs.lstatSync(linkPath, { throwIfNoEntry: false }), + 'dryRun does not remove orphan symlink') + + const arbLockOnly = new Arborist({ path: dir, registry, packumentCache: new Map(), cache, packageLockOnly: true }) + await arbLockOnly.reify({ installStrategy: 'linked', packageLockOnly: true }) + t.ok(fs.lstatSync(linkPath, { throwIfNoEntry: false }), + 'packageLockOnly does not remove orphan symlink') + + // A real install does perform the cleanup + const arbReal = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arbReal.reify({ installStrategy: 'linked' }) + t.notOk(fs.lstatSync(linkPath, { throwIfNoEntry: false }), + 'real install cleans up the orphan symlink') +}) + +tap.test('orphan sweep is scoped to in-filter workspaces during workspace-filtered linked install', async t => { + // Filtered installs should clean up dependencies removed from the targeted workspace, but leave out-of-scope workspaces alone. + // ws-a is the in-filter workspace: it keeps one dep (which) and drops one (abbrev) so both the surviving-link and orphan-sweep paths are exercised inside an in-scope workspace. + // ws-c is also in the filter but starts with one dep (abbrev) and drops it entirely, exercising the case where the in-filter workspace's node_modules dir is not populated by any surviving links. + // ws-b is out of filter and gets a stale link planted to verify it is preserved. + const graph = { + registry: [ + { name: 'abbrev', version: '4.0.0' }, + { name: 'which', version: '4.0.0' }, + ], + root: { + name: 'myroot', + version: '1.0.0', + // ws-c is declared as a root dep so its self-link lives at root node_modules — that means ws-c's own node_modules has no self-link, and dropping its only dep leaves the dir empty. + dependencies: { 'ws-c': '*' }, + }, + workspaces: [ + { name: 'ws-a', version: '1.0.0', dependencies: { abbrev: '4.0.0', which: '4.0.0' } }, + { name: 'ws-b', version: '1.0.0', dependencies: { abbrev: '4.0.0' } }, + { name: 'ws-c', version: '1.0.0', dependencies: { abbrev: '4.0.0' } }, + ], + } + + const { dir, registry } = await getRepo(graph) + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + + const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb1.reify({ installStrategy: 'linked' }) + + // Plant a stale orphan link inside ws-b (out-of-filter): the sweep must NOT touch it. + const stalePath = path.join(dir, 'packages', 'ws-b', 'node_modules', 'stale-pkg') + fs.symlinkSync('../../../node_modules/.store/nonexistent/node_modules/stale-pkg', stalePath) + + // Drop abbrev from ws-a's package.json so that ws-a/node_modules/abbrev becomes orphan; which stays as a surviving link. + const wsAPkgPath = path.join(dir, 'packages', 'ws-a', 'package.json') + const wsAPkg = JSON.parse(fs.readFileSync(wsAPkgPath, 'utf8')) + wsAPkg.dependencies = { which: '4.0.0' } + fs.writeFileSync(wsAPkgPath, JSON.stringify(wsAPkg)) + + // Drop all deps from ws-c so its node_modules has no surviving links — exercises the in-filter empty-dir seeding path. + const wsCPkgPath = path.join(dir, 'packages', 'ws-c', 'package.json') + const wsCPkg = JSON.parse(fs.readFileSync(wsCPkgPath, 'utf8')) + delete wsCPkg.dependencies + fs.writeFileSync(wsCPkgPath, JSON.stringify(wsCPkg)) + + const arb2 = new Arborist({ + path: dir, + registry, + packumentCache: new Map(), + cache, + workspaces: ['ws-a', 'ws-c'], + }) + await arb2.reify({ + installStrategy: 'linked', + workspaces: ['ws-a', 'ws-c'], + }) + + t.ok(arb2.diff.filterSet.size > 0, 'filterSet is populated for filtered install') + t.notOk(fs.lstatSync(path.join(dir, 'packages', 'ws-a', 'node_modules', 'abbrev'), { throwIfNoEntry: false }), + 'orphan link in in-filter workspace with surviving deps is removed') + t.ok(fs.lstatSync(path.join(dir, 'packages', 'ws-a', 'node_modules', 'which'), { throwIfNoEntry: false }), + 'surviving link in in-filter workspace is preserved') + t.notOk(fs.lstatSync(path.join(dir, 'packages', 'ws-c', 'node_modules', 'abbrev'), { throwIfNoEntry: false }), + 'orphan link in in-filter workspace with no surviving deps is removed') + t.ok(fs.lstatSync(stalePath, { throwIfNoEntry: false }), + 'stale link in out-of-filter workspace is preserved') +}) + +tap.test('orphan sweep includes root node_modules when --include-workspace-root is set', async t => { + // With --include-workspace-root, the filter scope pulls root deps in too, so dropped root deps must be cleaned up alongside the in-filter workspaces. + const graph = { + registry: [ + { name: 'abbrev', version: '4.0.0' }, + { name: 'which', version: '4.0.0' }, + ], + root: { + name: 'myroot', + version: '1.0.0', + dependencies: { which: '4.0.0' }, + }, + workspaces: [ + { name: 'ws-a', version: '1.0.0', dependencies: { abbrev: '4.0.0' } }, + ], + } + + const { dir, registry } = await getRepo(graph) + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + + const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb1.reify({ installStrategy: 'linked' }) + t.ok(fs.lstatSync(path.join(dir, 'node_modules', 'which'), { throwIfNoEntry: false }), 'which is installed at root') + + // Drop the root dep so node_modules/which becomes orphan. + const pkgPath = path.join(dir, 'package.json') + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')) + delete pkg.dependencies + fs.writeFileSync(pkgPath, JSON.stringify(pkg)) + + const arb2 = new Arborist({ + path: dir, + registry, + packumentCache: new Map(), + cache, + workspaces: ['ws-a'], + includeWorkspaceRoot: true, + }) + await arb2.reify({ + installStrategy: 'linked', + workspaces: ['ws-a'], + includeWorkspaceRoot: true, + }) + + t.notOk(fs.lstatSync(path.join(dir, 'node_modules', 'which'), { throwIfNoEntry: false }), + 'orphan root dep is removed when --include-workspace-root scope covers it') +}) + +tap.test('hand-made symlink inside the project root is intentionally swept by linked install', async t => { + // Documents an explicit trade-off: a hand-made symlink whose target lives inside the project (e.g. node_modules/local-tool -> ../tools/local-tool) is indistinguishable from a workspace self-link or store link by target alone. + // The linked sweep treats it as orphaned and removes it on the next reify, matching how the default install strategy already behaves with intra-project symlinks. + // External targets (outside the project root) remain preserved — see the sibling test 'unmanaged symlinks (e.g. npm link) in node_modules are preserved across reify'. + const graph = { + registry: [ + { name: 'abbrev', version: '4.0.0' }, + ], + root: { + name: 'myproject', + version: '1.0.0', + dependencies: { abbrev: '^4.0.0' }, + }, + } + const { dir, registry } = await getRepo(graph) + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + + const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb1.reify({ installStrategy: 'linked' }) + + // Create a target folder inside the project root and link to it from node_modules. + const localToolDir = path.join(dir, 'tools', 'local-tool') + fs.mkdirSync(localToolDir, { recursive: true }) + fs.writeFileSync(path.join(localToolDir, 'package.json'), + JSON.stringify({ name: 'local-tool', version: '0.0.0' })) + const intraLink = path.join(dir, 'node_modules', 'local-tool') + fs.symlinkSync(path.join('..', 'tools', 'local-tool'), intraLink) + + const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb2.reify({ installStrategy: 'linked' }) + + t.notOk(fs.lstatSync(intraLink, { throwIfNoEntry: false }), + 'intra-project hand-made symlink is removed by the sweep (intentional trade-off)') + t.ok(fs.existsSync(localToolDir), + 'the target directory itself is left intact — only the symlink is removed') +}) + +tap.test('orphaned workspace self-link in root node_modules is cleaned up when workspace is undeclared', async t => { + // When root declares a workspace as a dependency, the workspace gets a self-link at root node_modules (e.g. node_modules/a -> ../packages/a). + // If the workspace is later removed from root's dependencies, that self-link must be cleaned up. + // It is a symlink npm itself created, but its target lives outside .store/, so the sweep must accept any orphan whose target resolves inside the project root. + const graph = { + registry: [], + root: { + name: 'root', + version: '1.0.0', + dependencies: { a: '*', b: '*' }, + }, + workspaces: [ + { name: 'a', version: '1.0.0' }, + { name: 'b', version: '1.0.0' }, + ], + } + const { dir, registry } = await getRepo(graph) + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + + const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb1.reify({ installStrategy: 'linked' }) + + t.ok(fs.lstatSync(path.join(dir, 'node_modules', 'a'), { throwIfNoEntry: false }), 'a self-link present') + t.ok(fs.lstatSync(path.join(dir, 'node_modules', 'b'), { throwIfNoEntry: false }), 'b self-link present') + + // Drop workspace a from both root deps and the workspaces glob + const pkgPath = path.join(dir, 'package.json') + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')) + pkg.workspaces = ['packages/b'] + pkg.dependencies = { b: '*' } + fs.writeFileSync(pkgPath, JSON.stringify(pkg)) + + const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb2.reify({ installStrategy: 'linked' }) + + t.notOk(fs.lstatSync(path.join(dir, 'node_modules', 'a'), { throwIfNoEntry: false }), + 'orphan workspace self-link is removed') + t.ok(fs.lstatSync(path.join(dir, 'node_modules', 'b'), { throwIfNoEntry: false }), + 'still-declared workspace self-link is preserved') +}) + +tap.test('unmanaged symlinks (e.g. npm link) in node_modules are preserved across reify', async t => { + // The orphan sweep should only touch links the linked strategy itself created (those resolving into the project's node_modules/.store/). + // A symlink pointing outside .store/ — e.g. one created by `npm link foo` without --save or by hand — must be left alone. + const graph = { + registry: [ + { name: 'abbrev', version: '4.0.0' }, + ], + root: { + name: 'myproject', + version: '1.0.0', + dependencies: { abbrev: '^4.0.0' }, + }, + } + const { dir, registry } = await getRepo(graph) + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + + const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb1.reify({ installStrategy: 'linked' }) + + // Drop a hand-made symlink to a target outside the project's .store/ + const externalDir = fs.mkdtempSync(`${getTempDir()}/external-`) + fs.writeFileSync(path.join(externalDir, 'package.json'), + JSON.stringify({ name: 'external-pkg', version: '1.0.0' })) + const externalLink = path.join(dir, 'node_modules', 'external-pkg') + fs.symlinkSync(externalDir, externalLink) + + // Remove abbrev so the sweep runs and would otherwise consider external-pkg orphaned + const pkgPath = path.join(dir, 'package.json') + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')) + delete pkg.dependencies + fs.writeFileSync(pkgPath, JSON.stringify(pkg)) + + const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb2.reify({ installStrategy: 'linked' }) + + t.notOk(fs.existsSync(path.join(dir, 'node_modules', 'abbrev')), + 'orphan link into our .store/ is removed') + t.ok(fs.lstatSync(externalLink, { throwIfNoEntry: false }), + 'unmanaged symlink pointing outside .store/ is preserved') +}) + +tap.test('orphaned scoped top-level link is cleaned up when only one of two scoped deps is removed', async t => { + const graph = { + registry: [ + { name: '@scope/a', version: '1.0.0' }, + { name: '@scope/b', version: '1.0.0' }, + ], + root: { + name: 'myproject', + version: '1.0.0', + dependencies: { '@scope/a': '1.0.0', '@scope/b': '1.0.0' }, + }, + } + const { dir, registry } = await getRepo(graph) + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + + const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb1.reify({ installStrategy: 'linked' }) + + t.ok(fs.existsSync(path.join(dir, 'node_modules', '@scope', 'a')), '@scope/a installed') + t.ok(fs.existsSync(path.join(dir, 'node_modules', '@scope', 'b')), '@scope/b installed') + + // Drop @scope/a from package.json + const pkgPath = path.join(dir, 'package.json') + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')) + delete pkg.dependencies['@scope/a'] + fs.writeFileSync(pkgPath, JSON.stringify(pkg)) + + const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb2.reify({ installStrategy: 'linked' }) + + t.notOk(fs.existsSync(path.join(dir, 'node_modules', '@scope', 'a')), + '@scope/a top-level symlink is removed') + t.ok(fs.existsSync(path.join(dir, 'node_modules', '@scope', 'b')), + '@scope/b top-level symlink is preserved') }) tap.test('store symlinks are updated when hash changes after adding a dep', async t => {