From 6fd575b14b003f778da1f5cd62963da6620f23ae Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Sat, 30 May 2026 15:07:25 +0530 Subject: [PATCH] fix(arborist): clean up orphaned scoped store entries in linked strategy --- workspaces/arborist/lib/arborist/reify.js | 42 +++++++++++- workspaces/arborist/test/isolated-mode.js | 79 +++++++++++++++++++++++ 2 files changed, 119 insertions(+), 2 deletions(-) diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index 40e6c1853287d..202c75373e199 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -1322,9 +1322,27 @@ module.exports = cls => class Reifier extends cls { const nmDir = resolve(this.path, 'node_modules') const storeDir = resolve(nmDir, '.store') + // Enumerate on-disk store entries as full keys, descending one level into each @scope directory because scoped keys nest as .store/@scope/pkg@version-hash. let entries try { - entries = await readdir(storeDir) + const topLevel = await readdir(storeDir, { withFileTypes: true }) + entries = [] + for (const ent of topLevel) { + if (ent.name.startsWith('@')) { + let scoped + try { + scoped = await readdir(resolve(storeDir, ent.name)) + } catch { + /* istanbul ignore next -- readdir of an entry we just listed should not fail */ + continue + } + for (const name of scoped) { + entries.push(`${ent.name}/${name}`) + } + } else { + entries.push(ent.name) + } + } } catch { entries = null } @@ -1340,7 +1358,10 @@ module.exports = cls => class Reifier extends cls { for (const child of this.idealTree.children.values()) { const loc = child.location.replace(/\\/g, '/') if (child.isInStore) { - const key = loc.split('/')[2] + // Store location is node_modules/.store/{key}/node_modules/{pkg}. + // For a scoped package the key is @scope/pkg@version-hash, which spans two path segments, so reconstruct both instead of taking only the scope. + const parts = loc.split('/') + const key = parts[2].startsWith('@') ? `${parts[2]}/${parts[3]}` : parts[2] validKeys.add(key) continue } @@ -1422,6 +1443,23 @@ module.exports = cls => class Reifier extends cls { er => log.warn('cleanup', `Failed to remove orphaned store entry ${e}`, er)) ) ) + // Removing the last scoped orphan under a scope leaves an empty @scope directory behind, so prune any scope directory that is now empty. + const scopes = new Set( + orphaned.filter(e => e.startsWith('@')).map(e => e.split('/')[0]) + ) + await promiseAllRejectLate( + [...scopes].map(async scope => { + const scopeDir = resolve(storeDir, scope) + try { + const remaining = await readdir(scopeDir) + if (!remaining.length) { + await rm(scopeDir, { recursive: true, force: true }) + } + } catch { + /* istanbul ignore next -- readdir of a scope dir we just listed should not fail */ + } + }) + ) } } diff --git a/workspaces/arborist/test/isolated-mode.js b/workspaces/arborist/test/isolated-mode.js index 2be8561806b3f..fb83428738fe2 100644 --- a/workspaces/arborist/test/isolated-mode.js +++ b/workspaces/arborist/test/isolated-mode.js @@ -1760,6 +1760,85 @@ tap.test('orphaned store entries are cleaned up on dependency update', async t = 'old which@1.0.0 store entry is removed after update') }) +tap.test('orphaned scoped store entries are cleaned up on dependency update', async t => { + // https://github.com/npm/cli/issues/9440 — a scoped store key spans two path segments (.store/@scope/pkg@version-hash), so the single-segment orphan cleanup never swept the stale entry. + const graph = { + registry: [ + { name: '@scope/which', version: '1.0.0', dependencies: { isexe: '^1.0.0' } }, + { name: '@scope/which', version: '2.0.0', dependencies: { isexe: '^1.0.0' } }, + { name: 'isexe', version: '1.0.0' }, + ], + root: { + name: 'myproject', + version: '1.0.0', + dependencies: { '@scope/which': '1.0.0' }, + }, + } + const { dir, registry } = await getRepo(graph) + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + const storeDir = path.join(dir, 'node_modules', '.store') + const scopeDir = path.join(storeDir, '@scope') + + // First install — @scope/which@1.0.0 + const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb1.reify({ installStrategy: 'linked' }) + + const entriesAfterV1 = fs.readdirSync(scopeDir) + t.ok(entriesAfterV1.some(e => e.startsWith('which@1.0.0-')), + 'store has @scope/which@1.0.0 entry after first install') + + // Update package.json to depend on @scope/which@2.0.0 + const pkgPath = path.join(dir, 'package.json') + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')) + pkg.dependencies['@scope/which'] = '2.0.0' + fs.writeFileSync(pkgPath, JSON.stringify(pkg)) + + // Second install — @scope/which@2.0.0 + const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb2.reify({ installStrategy: 'linked' }) + + const entriesAfterV2 = fs.readdirSync(scopeDir) + t.ok(entriesAfterV2.some(e => e.startsWith('which@2.0.0-')), + 'store has @scope/which@2.0.0 entry after update') + t.notOk(entriesAfterV2.some(e => e.startsWith('which@1.0.0-')), + 'old @scope/which@1.0.0 store entry is removed after update') +}) + +tap.test('orphaned scoped store entries leave no empty scope directory when last dep is removed', async t => { + // https://github.com/npm/cli/issues/9440 — when the last package under a scope is orphaned, the now-empty @scope directory should also be pruned. + const graph = { + registry: [ + { name: '@scope/which', version: '1.0.0', dependencies: { isexe: '^1.0.0' } }, + { name: 'isexe', version: '1.0.0' }, + ], + root: { + name: 'myproject', + version: '1.0.0', + dependencies: { '@scope/which': '1.0.0' }, + }, + } + const { dir, registry } = await getRepo(graph) + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + const storeDir = path.join(dir, 'node_modules', '.store') + + const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb1.reify({ installStrategy: 'linked' }) + + t.ok(fs.existsSync(path.join(storeDir, '@scope')), 'store has @scope directory after install') + + // Remove the dependency + 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(storeDir, '@scope')), + 'empty @scope directory is pruned after the last scoped dep is removed') +}) + tap.test('orphaned store entries are cleaned up on dependency removal', async t => { const graph = { registry: [