fix: fix workspace-filtered install with linked strategy

owlstronaut · owlstronaut · commit 26fa40eeafdb · 2026-03-02T09:47:31.000-08:00
diff --git a/workspaces/arborist/lib/arborist/isolated-reifier.js b/workspaces/arborist/lib/arborist/isolated-reifier.js
@@ -267,7 +267,7 @@ module.exports = cls => class IsolatedReifier extends cls {
     }
 
     const root = {
-      fsChildren: [],
+      fsChildren: new Set(),
       integrity: null,
       inventory: new Map(),
       isLink: false,
@@ -286,30 +286,67 @@ module.exports = cls => class IsolatedReifier extends cls {
       meta: { loadedFromDisk: false },
       global: false,
       isProjectRoot: true,
-      children: [],
+      children: new Map(),
+      workspaces: new Map(),
+      tops: new Set(),
+      linksIn: new Set(),
     }
-    // root.inventory.set('', t)
-    // root.meta = this.idealTree.meta
-    // TODO We should mock better the inventory object because it is used by audit-report.js ... maybe
+    root.inventory.set('', root)
+    // TODO inventory.query is a stub; audit-report needs 'packageName' support
     root.inventory.query = () => {
       return []
     }
     const processed = new Set()
     proxiedIdealTree.workspaces.forEach(c => {
+      const wsName = c.packageName
       const workspace = {
         edgesIn: new Set(),
         edgesOut: new Map(),
-        children: [],
+        children: new Map(),
+        fsChildren: new Set(),
         hasInstallScript: c.hasInstallScript,
         binPaths: [],
         package: c.package,
         location: c.localLocation,
         path: c.localPath,
         realpath: c.localPath,
         resolved: c.resolved,
+        isLink: false,
+        isRoot: false,
+        name: wsName,
+        linksIn: new Set(),
       }
-      root.fsChildren.push(workspace)
+      workspace.target = workspace
+      root.fsChildren.add(workspace)
       root.inventory.set(workspace.location, workspace)
+
+      // Create workspace Link entry in children for _diffTrees lookup
+      const wsLink = {
+        name: wsName,
+        location: join('node_modules', wsName),
+        path: join(root.path, 'node_modules', wsName),
+        realpath: workspace.path,
+        isLink: true,
+        target: workspace,
+        children: new Map(),
+        fsChildren: new Set(),
+        edgesIn: new Set(),
+        edgesOut: new Map(),
+        binPaths: [],
+        root,
+        parent: root,
+        isRoot: false,
+        isProjectRoot: false,
+        isTop: false,
+        global: false,
+        globalTop: false,
+        package: workspace.package,
+        linksIn: new Set(),
+      }
+      root.children.set(wsLink.name, wsLink)
+      root.inventory.set(wsLink.location, wsLink)
+      root.workspaces.set(wsName, workspace.path)
+      workspace.linksIn.add(wsLink)
     })
     const generateChild = (node, location, pkg, inStore) => {
       const newChild = {
@@ -321,11 +358,11 @@ module.exports = cls => class IsolatedReifier extends cls {
         name: node.packageName || node.name,
         optional: node.optional,
         top: { path: proxiedIdealTree.root.localPath },
-        children: [],
+        children: new Map(),
         edgesIn: new Set(),
         edgesOut: new Map(),
         binPaths: [],
-        fsChildren: [],
+        fsChildren: new Set(),
         /* istanbul ignore next -- emulate Node */
         getBundler () {
           return null
@@ -343,7 +380,7 @@ module.exports = cls => class IsolatedReifier extends cls {
         package: pkg,
       }
       newChild.target = newChild
-      root.children.push(newChild)
+      root.children.set(newChild.location, newChild)
       root.inventory.set(newChild.location, newChild)
     }
     proxiedIdealTree.external.forEach(c => {
@@ -379,10 +416,10 @@ module.exports = cls => class IsolatedReifier extends cls {
       let from, nmFolder
       if (externalEdge) {
         const fromLocation = join('node_modules', '.store', key, 'node_modules', node.packageName)
-        from = root.children.find(c => c.location === fromLocation)
+        from = root.children.get(fromLocation)
         nmFolder = join('node_modules', '.store', key, 'node_modules')
       } else {
-        from = node.isProjectRoot ? root : root.fsChildren.find(c => c.location === node.localLocation)
+        from = node.isProjectRoot ? root : root.inventory.get(node.localLocation)
         nmFolder = join(node.localLocation, 'node_modules')
       }
       /* istanbul ignore next - strict-peer-deps can exclude nodes from the tree */
@@ -401,9 +438,9 @@ module.exports = cls => class IsolatedReifier extends cls {
         let target
         if (external) {
           const toLocation = join('node_modules', '.store', toKey, 'node_modules', dep.packageName)
-          target = root.children.find(c => c.location === toLocation)
+          target = root.children.get(toLocation)
         } else {
-          target = root.fsChildren.find(c => c.location === dep.localLocation)
+          target = root.inventory.get(dep.localLocation)
         }
         // TODO: we should no-op is an edge has already been created with the same fromKey and toKey
         /* istanbul ignore next - strict-peer-deps can exclude nodes from the tree */
@@ -430,8 +467,8 @@ module.exports = cls => class IsolatedReifier extends cls {
           name: toKey,
           resolved: dep.resolved,
           top: { path: dep.root.localPath },
-          children: [],
-          fsChildren: [],
+          children: new Map(),
+          fsChildren: new Set(),
           isLink: true,
           isStoreLink: true,
           isRoot: false,
@@ -444,7 +481,7 @@ module.exports = cls => class IsolatedReifier extends cls {
         const newEdge2 = { optional: false, from: link, to: target }
         link.edgesOut.set(dep.name, newEdge2)
         target.edgesIn.add(newEdge2)
-        root.children.push(link)
+        root.children.set(link.location, link)
       }
 
       for (const dep of node.localDependencies) {
diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js
@@ -146,7 +146,7 @@ module.exports = cls => class Reifier extends cls {
     // was not changed, delete anything in the ideal and not actual.
     // Then we move the entire idealTree over to this.actualTree, and
     // save the hidden lockfile.
-    if (this.diff && this.diff.filterSet.size) {
+    if (this.diff && this.diff.filterSet.size && !linked) {
       const reroot = new Set()
 
       const { filterSet } = this.diff
@@ -422,7 +422,7 @@ module.exports = cls => class Reifier extends cls {
       if (includeWorkspaces) {
         // add all ws nodes to filterNodes
         for (const ws of this.options.workspaces) {
-          const ideal = this.idealTree.children.get && this.idealTree.children.get(ws)
+          const ideal = this.idealTree.children.get(ws)
           if (ideal) {
             filterNodes.push(ideal)
           }
diff --git a/workspaces/arborist/lib/diff.js b/workspaces/arborist/lib/diff.js
@@ -71,6 +71,7 @@ class Diff {
         tree: filterNode,
         visit: node => filterSet.add(node),
         getChildren: node => {
+          const orig = node
           node = node.target
           const loc = node.location
           const idealNode = ideal.inventory.get(loc)
@@ -87,7 +88,12 @@ class Diff {
             }
           }
 
-          return ideals.concat(actuals)
+          const result = ideals.concat(actuals)
+          // Include link targets so store entries end up in filterSet
+          if (orig.isLink) {
+            result.push(node)
+          }
+          return result
         },
       })
     }
diff --git a/workspaces/arborist/test/isolated-mode.js b/workspaces/arborist/test/isolated-mode.js
@@ -1601,6 +1601,62 @@ tap.test('postinstall scripts run once for store packages', async t => {
   t.equal(count, 1, 'postinstall ran exactly once')
 })
 
+tap.test('workspace-filtered install with linked strategy', async t => {
+  // Two workspaces sharing the same dependency must not crash when installing with --workspace + --install-strategy=linked.
+  const graph = {
+    registry: [
+      { name: 'abbrev', version: '2.0.0' },
+    ],
+    root: {
+      name: 'myroot', version: '1.0.0',
+    },
+    workspaces: [
+      { name: 'ws-a', version: '1.0.0', dependencies: { abbrev: '2.0.0' } },
+      { name: 'ws-b', version: '1.0.0', dependencies: { abbrev: '2.0.0' } },
+    ],
+  }
+
+  const { dir, registry } = await getRepo(graph)
+
+  const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
+  const arborist = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
+
+  // Full install first
+  await arborist.reify({ installStrategy: 'linked' })
+
+  // Verify store entry exists
+  const storeDir = path.join(dir, 'node_modules', '.store')
+  const storeEntries = fs.readdirSync(storeDir)
+  t.ok(storeEntries.some(e => e.startsWith('abbrev@')), 'store has abbrev entry after full install')
+
+  // Workspace-filtered install must not crash
+  const arborist2 = new Arborist({
+    path: dir,
+    registry,
+    packumentCache: new Map(),
+    cache,
+    workspaces: ['ws-a'],
+  })
+  await arborist2.reify({
+    installStrategy: 'linked',
+    workspaces: ['ws-a'],
+  })
+
+  // Verify workspace filtering was actually applied (not silently skipped)
+  t.ok(arborist2.diff.filterSet.size > 0, 'workspace filter was applied to diff')
+
+  // Store entries still intact
+  const storeEntries2 = fs.readdirSync(storeDir)
+  t.ok(storeEntries2.some(e => e.startsWith('abbrev@')), 'store entries preserved after ws install')
+
+  // Workspace symlinks preserved
+  const wsALink = fs.readlinkSync(path.join(dir, 'packages', 'ws-a', 'node_modules', 'abbrev'))
+  t.ok(wsALink.includes('.store'), 'workspace a abbrev symlink points to store')
+
+  const wsBLink = fs.readlinkSync(path.join(dir, 'packages', 'ws-b', 'node_modules', 'abbrev'))
+  t.ok(wsBLink.includes('.store'), 'workspace b abbrev symlink preserved')
+})
+
 tap.test('bins are installed', async t => {
   // Input of arborist
   const graph = {
