fix(arborist): clean up orphaned scoped store entries in linked strategy

(cherry picked from commit 275bc69)

Author: manzoorwanijk

workspaces/arborist/lib/arborist/reify.js

@@ -1362,9 +1362,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
     }
@@ -1380,7 +1398,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
       }
@@ -1462,6 +1483,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 */
+            }
+          })
+        )
       }
     }
 

workspaces/arborist/test/isolated-mode.js

@@ -1994,6 +1994,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: [