fix(arborist): don't crash when a peer dep is evicted from its virtual root (#9911)

workspaces/arborist/lib/place-dep.js

@@ -298,41 +298,49 @@ class PlaceDep {
     // because we're copying rather than moving them out of the virtual root,
     // otherwise they'd be gone and the peer set would change throughout
     // this loop.
-    for (const peerEdge of this.placed.edgesOut.values()) {
-      if (peerEdge.valid || !peerEdge.peer || peerEdge.peerConflicted) {
-        continue
-      }
+    // The dep we're placing may have been evicted from its virtual root while
+    // a later peer edge was being resolved (a recursive #loadPeerSet can
+    // detach the original node when a newer copy takes its place in the
+    // virtual root's inventory).  In that case there is no sibling peer set
+    // left to place here; the placed node's unmet peers are re-resolved when
+    // it is processed from the deps queue.
+    if (virtualRoot) {
+      for (const peerEdge of this.placed.edgesOut.values()) {
+        if (peerEdge.valid || !peerEdge.peer || peerEdge.peerConflicted) {
+          continue
+        }
 
-      const peer = virtualRoot.children.get(peerEdge.name)
+        const peer = virtualRoot.children.get(peerEdge.name)
 
-      // Note: if the virtualRoot *doesn't* have the peer, then that means
-      // it's an optional peer dep.  If it's not being properly met (ie,
-      // peerEdge.valid is false), then this is likely heading for an
-      // ERESOLVE error, unless it can walk further up the tree.
-      if (!peer) {
-        continue
-      }
+        // Note: if the virtualRoot *doesn't* have the peer, then that means
+        // it's an optional peer dep.  If it's not being properly met (ie,
+        // peerEdge.valid is false), then this is likely heading for an
+        // ERESOLVE error, unless it can walk further up the tree.
+        if (!peer) {
+          continue
+        }
 
-      // peerConflicted peerEdge, just accept what's there already
-      if (!peer.satisfies(peerEdge)) {
-        continue
-      }
+        // peerConflicted peerEdge, just accept what's there already
+        if (!peer.satisfies(peerEdge)) {
+          continue
+        }
 
-      this.children.push(new PlaceDep({
-        auditReport: this.auditReport,
-        explicitRequest: this.explicitRequest,
-        force: this.force,
-        installLinks: this.installLinks,
-        installStrategy: this.installStrategy,
-        legacyPeerDeps: this.legacyPeerDeps,
-        preferDedupe: this.preferDedupe,
-        strictPeerDeps: this.strictPeerDeps,
-        updateNames: this.updateName,
-        parent: this,
-        dep: peer,
-        node: this.placed,
-        edge: peerEdge,
-      }))
+        this.children.push(new PlaceDep({
+          auditReport: this.auditReport,
+          explicitRequest: this.explicitRequest,
+          force: this.force,
+          installLinks: this.installLinks,
+          installStrategy: this.installStrategy,
+          legacyPeerDeps: this.legacyPeerDeps,
+          preferDedupe: this.preferDedupe,
+          strictPeerDeps: this.strictPeerDeps,
+          updateNames: this.updateName,
+          parent: this,
+          dep: peer,
+          node: this.placed,
+          edge: peerEdge,
+        }))
+      }
     }
   }
 

workspaces/arborist/test/arborist/build-ideal-tree.js

@@ -4959,6 +4959,62 @@ t.test('circular peer back-off does not crash when node is detached mid-resoluti
     'backs off to plugin@1.0.0 to satisfy the optional peer instead of crashing')
 })
 
+t.test('peer task dep evicted from its virtual root by a later peer edge does not crash (#9911)', async t => {
+  // A global install queues tui's peer edges as problem edges resolved one at
+  // a time.  Resolving the @t/agent edge places agent@2.0.0 into the virtual
+  // root, then resolving @t/y's peer set replaces it with agent@1.0.0 in the
+  // same virtual root (the canReplace path in #loadPeerSet), detaching the
+  // node the agent task still holds.  Building that task's PlaceDep used to
+  // dereference the now-null virtual root while trying to place agent's
+  // invalid peer edge llm@^2.0.0, crashing with "Cannot read properties of
+  // null (reading 'children')".  The unmet peers are instead re-resolved from
+  // the deps queue, so the tree backs off to the versions the peer set wants.
+  const registry = createRegistry(t, false)
+
+  const tuiPack = registry.packument({
+    name: '@t/tui',
+    version: '1.0.0',
+    peerDependencies: { '@t/agent': '*', '@t/y': '1.0.0' },
+  })
+  const tuiManifest = registry.manifest({ name: '@t/tui', packuments: [tuiPack] })
+  await registry.package({ manifest: tuiManifest })
+
+  const agentPacks = [
+    registry.packument({ name: '@t/agent', version: '1.0.0', peerDependencies: { '@t/llm': '^1.0.0' } }),
+    registry.packument({ name: '@t/agent', version: '2.0.0', peerDependencies: { '@t/llm': '^2.0.0' } }),
+  ]
+  const agentManifest = registry.manifest({ name: '@t/agent', packuments: agentPacks })
+  await registry.package({ manifest: agentManifest, times: 2 })
+
+  const yPack = registry.packument({
+    name: '@t/y',
+    version: '1.0.0',
+    peerDependencies: { '@t/agent': '^1.0.0', '@t/llm': '^1.0.0' },
+  })
+  const yManifest = registry.manifest({ name: '@t/y', packuments: [yPack] })
+  await registry.package({ manifest: yManifest })
+
+  const llmPacks = [
+    registry.packument({ name: '@t/llm', version: '1.0.0' }),
+    registry.packument({ name: '@t/llm', version: '2.0.0' }),
+  ]
+  const llmManifest = registry.manifest({ name: '@t/llm', packuments: llmPacks })
+  await registry.package({ manifest: llmManifest, times: 2 })
+
+  const path = t.testdir({
+    'package.json': JSON.stringify({ name: 'test-9911' }),
+  })
+  const arb = newArb(path, { global: true })
+  const tree = await arb.buildIdealTree({ add: ['@t/tui@1.0.0'] })
+
+  const tui = tree.children.get('@t/tui')
+  t.ok(tui, 'tui is installed at top level')
+  t.equal(tui.children.get('@t/agent').version, '1.0.0',
+    'backs off to agent@1.0.0 to satisfy the peer set instead of crashing')
+  t.equal(tui.children.get('@t/llm').version, '1.0.0',
+    'llm backs off to the version the peer set can satisfy')
+})
+
 t.test('does not fetch packuments for peerOptional deps that will not be installed', async t => {
   const registry = createRegistry(t, false)