fix(arborist): fix non-idempotent linked install with workspace projects (#9094)

Author: manzoorwanijk

workspaces/arborist/lib/arborist/reify.js

@@ -808,28 +808,23 @@ module.exports = cls => class Reifier extends cls {
     // Combined Map keyed by path (how allChildren() in diff.js keys)
     const combined = new Map()
 
-    // Add actual tree's children (the top-level symlinks)
-    for (const child of actualTree.children.values()) {
-      combined.set(child.path, child)
-    }
-
-    // Add synthetic entries for store nodes and store links that exist on disk.
-    // The proxy tree is flat: all store entries (isInStore) and store links (isStoreLink) are direct children of root.
-    // The actual tree only has top-level links as root children, so store entries need synthetic actual entries for the diff to match them.
+    // Create synthetic actual entries for ALL ideal children that exist on disk.
+    // The isolated ideal tree is flat (all entries as root children), but loadActual() produces a nested tree where workspace deps are under fsChildren and store entries are deep link targets.
+    // Synthetic entries ensure the diff compares matching resolved/integrity values (e.g. workspace links have resolved=undefined in the ideal tree but resolved="file:../packages/..." in the actual tree).
     for (const child of idealTree.children.values()) {
-      if (!combined.has(child.path) && (child.isInStore || child.isStoreLink) &&
-          existsSync(child.path)) {
-        let entry
-        if (child.isLink) {
-          entry = new IsolatedLink(child)
-        } else {
-          entry = new IsolatedNode(child)
-        }
-        if (child.isLink && combined.has(child.realpath)) {
-          entry.target = combined.get(child.realpath)
-        }
-        combined.set(child.path, entry)
+      if (combined.has(child.path) || !existsSync(child.path)) {
+        continue
+      }
+      let entry
+      if (child.isLink) {
+        entry = new IsolatedLink(child)
+      } else {
+        entry = new IsolatedNode(child)
+      }
+      if (child.isLink && combined.has(child.realpath)) {
+        entry.target = combined.get(child.realpath)
       }
+      combined.set(child.path, entry)
     }
 
     // Proxy .get(name) to original actual tree for filterNodes compatibility
@@ -850,7 +845,9 @@ module.exports = cls => class Reifier extends cls {
     wrapper.binPaths = actualTree.binPaths
     wrapper.children = combined
     wrapper.edgesOut = actualTree.edgesOut
-    wrapper.fsChildren = actualTree.fsChildren
+    // Use empty fsChildren so that allChildren() only picks up entries from the combined map.
+    // The actual fsChildren have real children with different resolved values (e.g. file:../../../node_modules/.store/... vs file:.store/...) that would overwrite our synthetic entries in allChildren().
+    wrapper.fsChildren = new Set()
     wrapper.integrity = actualTree.integrity
     wrapper.inventory = actualTree.inventory
 

workspaces/arborist/test/isolated-mode.js

@@ -1863,6 +1863,43 @@ tap.test('workspace links are not affected by store resolved fix', async t => {
   t.ok(arb2.diff.unchanged.length > 0, 'second install should have unchanged nodes')
 })
 
+tap.test('idempotent install with cross-workspace deps (diamond pattern)', async t => {
+  // Regression: when workspace-x depends on workspace-y (and both share a registry dep), the second install would report "changed N packages" because (1) workspace link resolved values didn't match between ideal and actual trees, and (2) actual fsChildren overwrote synthetic store entries in the diff proxy.
+  const graph = {
+    registry: [
+      { name: 'abbrev', version: '2.0.0' },
+    ],
+    root: {
+      name: 'myproject',
+      version: '1.0.0',
+      dependencies: { 'workspace-x': '*', 'workspace-y': '*' },
+    },
+    workspaces: [
+      { name: 'workspace-x', version: '1.0.0', dependencies: { abbrev: '2.0.0', 'workspace-y': '*' } },
+      { name: 'workspace-y', version: '1.0.0', dependencies: { abbrev: '2.0.0' } },
+    ],
+  }
+  const { dir, registry } = await getRepo(graph)
+  const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
+
+  // First install
+  const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
+  await arb1.reify({ installStrategy: 'linked' })
+
+  // Second install — should detect everything is up-to-date
+  const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
+  await arb2.reify({ installStrategy: 'linked' })
+
+  const leaves = arb2.diff?.leaves || []
+  const actions = leaves.filter(l => l.action)
+  t.equal(actions.length, 0, 'second install should have no diff actions')
+  t.ok(arb2.diff.unchanged.length > 0, 'second install should have unchanged nodes')
+
+  // Verify packages are still correctly installed (abbrev is a workspace dep, not root)
+  t.ok(setupRequire(path.join(dir, 'packages', 'workspace-y'))('abbrev'),
+    'abbrev is requireable from workspace after second install')
+})
+
 tap.test('orphaned store entries are cleaned up on dependency update', async t => {
   const graph = {
     registry: [