@npmcli/arborist - npm Package Compare versions

Comparing version 9.4.0 to 9.4.1

lib/isolated-classes.js

+// Alternate versions of different classes that we use for isolated mode
+const CaseInsensitiveMap = require('./case-insensitive-map.js')
+const { resolve } = require('node:path')
+
+// fake lib/inventory.js
+class IsolatedInventory extends Map {
+  query () {
+    return []
+  }
+}
+
+// fake lib/node.js
+class IsolatedNode {
+  binPaths = []
+  children = new CaseInsensitiveMap()
+  edgesIn = new Set()
+  edgesOut = new CaseInsensitiveMap()
+  fsChildren = new Set()
+  hasShrinkwrap = false
+  integrity = null
+  inventory = new IsolatedInventory()
+  isInStore = false
+  linksIn = new Set()
+  meta = { loadedFromDisk: false }
+  optional = false
+  parent = null
+  root = null
+  tops = new Set()
+  workspaces = new Map()
+
+  constructor (options) {
+    this.location = options.location
+    this.name = options.name
+    this.package = options.package
+    this.path = options.path
+    this.realpath = !this.isLink ? this.path : resolve(options.realpath)
+
+    if (options.parent) {
+      this.parent = options.parent
+    }
+    if (options.resolved) {
+      this.resolved = options.resolved
+    }
+    if (options.root) {
+      this.root = options.root
+    }
+    if (options.isInStore) {
+      this.isInStore = true
+    }
+    if (options.optional) {
+      this.optional = true
+    }
+  }
+
+  get isRoot () {
+    return this === this.root
+  }
+
+  // The idealGraph is where this is set to true
+  get isProjectRoot () {
+    return false
+  }
+
+  get inDepBundle () {
+    return false
+  }
+
+  get isLink () {
+    return false
+  }
+
+  get isTop () {
+    return !this.parent
+  }
+
+  /* istanbul ignore next -- emulate lib/node.js */
+  get global () {
+    return false
+  }
+
+  get globalTop () {
+    return false
+  }
+
+  /* istanbul ignore next -- emulate lib/node.js */
+  set target (t) {
+    // nop
+    // In the real lib/node.js this throws in debug mode
+  }
+
+  get target () {
+    return this
+  }
+
+  /* istanbul ignore next -- emulate lib/node.js */
+  getBundler () {
+    return null
+  }
+
+  /* istanbul ignore next -- emulate lib/node.js */
+  get hasInstallScript () {
+    const { hasInstallScript, scripts } = this.package
+    const { install, preinstall, postinstall } = scripts || {}
+    return !!(hasInstallScript || install || preinstall || postinstall)
+  }
+
+  get version () {
+    return this.package.version
+  }
+}
+
+// fake lib/link.js
+class IsolatedLink extends IsolatedNode {
+  #target
+  isStoreLink = false
+
+  constructor (options) {
+    super(options)
+    this.#target = options.target
+    if (options.isStoreLink) {
+      this.isStoreLink = true
+    }
+  }
+
+  get isLink () {
+    return true
+  }
+
+  set target (t) {
+    this.#target = t
+  }
+
+  get target () {
+    return this.#target
+  }
+}
+
+module.exports = { IsolatedNode, IsolatedLink }

lib/arborist/isolated-reifier.js

@@ -1,3 +0,1 @@
-const _makeIdealGraph = Symbol('makeIdealGraph')
-const _createIsolatedTree = Symbol.for('createIsolatedTree')
 const { mkdirSync } = require('node:fs')
@@ -8,19 +6,56 @@ const pacote = require('pacote')
 const crypto = require('node:crypto')
+const { IsolatedNode, IsolatedLink } = require('../isolated-classes.js')
 
-// cache complicated function results
-const memoize = (fn) => {
-  const memo = new Map()
-  return async function (arg) {
-    const key = arg
-    if (memo.has(key)) {
-      return memo.get(key)
-    }
-    const result = {}
-    memo.set(key, result)
-    await fn(result, arg)
-    return result
-  }
+// generate short hash key based on the dependency tree starting at this node
+const getKey = (startNode) => {
+  const deps = []
+  const branch = []
+  depth({
+    tree: startNode,
+    getChildren: node => node.dependencies,
+    visit: node => {
+      branch.push(`${node.packageName}@${node.version}`)
+      deps.push(`${branch.join('->')}::${node.resolved}`)
+    },
+    leave: () => {
+      branch.pop()
+    },
+  })
+  deps.sort()
+  // TODO these replaces were originally to deal with node 14 not supporting base64url and likely don't need to happen anymore
+  // Changing this is a pretty significant breaking change, but removing parts of the hash increases collision possibilities (even if slight).
+  const hash = crypto.createHash('shake256', { outputLength: 16 })
+    .update(deps.join(','))
+    .digest('base64')
+    .replace(/\+/g, '-')
+    .replace(/\//g, '_')
+    .replace(/=+$/m, '')
+  return `${startNode.packageName}@${startNode.version}-${hash}`
 }
 
 module.exports = cls => class IsolatedReifier extends cls {
+  #externalProxies = new Map()
+  #omit = new Set()
+  #rootDeclaredDeps = new Set()
+  #processedEdges = new Set()
+  #workspaceProxies = new Map()
+
+  #generateChild (node, location, pkg, isInStore, root) {
+    const newChild = new IsolatedNode({
+      isInStore,
+      location,
+      name: node.packageName || node.name,
+      optional: node.optional,
+      package: pkg,
+      parent: root,
+      path: join(this.idealGraph.localPath, location),
+      resolved: node.resolved,
+      root,
+    })
+    // XXX top is from place-dep not lib/node.js
+    newChild.top = { path: this.idealGraph.localPath }
+    root.children.set(newChild.location, newChild)
+    root.inventory.set(newChild.location, newChild)
+  }
+
   /**
@@ -32,40 +67,37 @@ * Create an ideal graph.
    *
-   * This entire file should be considered technical debt that will be resolved
-   * with an Arborist refactor or rewrite. Embedded logic in Nodes and Links,
-   * and the incremental state of building trees and reifying contains too many
-   * assumptions to do a linked mode properly.
+   * This entire file should be considered technical debt that will be resolved with an Arborist refactor or rewrite.
+   * Embedded logic in Nodes and Links, and the incremental state of building trees and reifying contains too many assumptions to do a linked mode properly.
    *
-   * Instead, this approach takes a tree built from build-ideal-tree, and
-   * returns a new tree-like structure without the embedded logic of Node and
-   * Link classes.
+   * Instead, this approach takes a tree built from build-ideal-tree, and returns a new tree-like structure without the embedded logic of Node and Link classes.
    *
-   * Since the RFC requires leaving the package-lock in place, this approach
-   * temporarily replaces the tree state for a couple of steps of reifying.
+   * Since the RFC requires leaving the package-lock in place, this approach temporarily replaces the tree state for a couple of steps of reifying.
    *
    **/
-  async [_makeIdealGraph] (options) {
-    /* Make sure that the ideal tree is build as the rest of
-     * the algorithm depends on it.
-     */
-    const bitOpt = {
-      ...options,
-      complete: false,
-    }
-    await this.buildIdealTree(bitOpt)
+  async makeIdealGraph () {
     const idealTree = this.idealTree
+    this.#omit = new Set(this.options.omit)
+    const omit = this.#omit
 
-    this.rootNode = {}
-    const root = this.rootNode
+    // npm auto-creates 'workspace' edges from root to all workspaces.
+    // For isolated/linked mode, only include workspaces that root explicitly declares as dependencies.
+    // When omitting dep types, exclude those from the declared set so their workspaces aren't hoisted.
+    const rootPkg = idealTree.package
+    this.#rootDeclaredDeps = new Set(Object.keys(Object.assign({},
+      rootPkg.dependencies,
+      (!omit.has('dev') && rootPkg.devDependencies),
+      (!omit.has('optional') && rootPkg.optionalDependencies),
+      (!omit.has('peer') && rootPkg.peerDependencies)
+    )))
+
+    // XXX this sometimes acts like a node too
+    this.idealGraph = {
+      external: [],
+      isProjectRoot: true,
+      localLocation: idealTree.location,
+      localPath: idealTree.path,
+      path: idealTree.path,
+    }
     this.counter = 0
 
-    // memoize to cache generating proxy Nodes
-    this.externalProxyMemo = memoize(this.externalProxy.bind(this))
-    this.workspaceProxyMemo = memoize(this.workspaceProxy.bind(this))
-
-    root.external = []
-    root.isProjectRoot = true
-    root.localLocation = idealTree.location
-    root.localPath = idealTree.path
-    root.workspaces = await Promise.all(
-      Array.from(idealTree.fsChildren.values(), this.workspaceProxyMemo))
+    this.idealGraph.workspaces = await Promise.all(Array.from(idealTree.fsChildren.values(), w => this.#workspaceProxy(w)))
     const processed = new Set()
@@ -79,19 +111,24 @@ const queue = [idealTree, ...idealTree.fsChildren]
       processed.add(next.location)
-      next.edgesOut.forEach(e => {
-        if (!e.to || (next.package.bundleDependencies || next.package.bundledDependencies || []).includes(e.to.name)) {
-          return
+      next.edgesOut.forEach(edge => {
+        if (edge.to && !(next.package.bundleDependencies || next.package.bundledDependencies || []).includes(edge.to.name) && !edge.to.shouldOmit?.(omit)) {
+          queue.push(edge.to)
         }
-        queue.push(e.to)
       })
-      if (!next.isProjectRoot && !next.isWorkspace && !next.inert) {
-        root.external.push(await this.externalProxyMemo(next))
+      // local `file:` deps are in fsChildren but are not workspaces.
+      // they are already handled as workspace-like proxies above and should not go through the external/store extraction path.
+      if (!next.isProjectRoot && !next.isWorkspace && !next.inert && !idealTree.fsChildren.has(next) && !idealTree.fsChildren.has(next.target)) {
+        this.idealGraph.external.push(await this.#externalProxy(next))
       }
     }
 
-    await this.assignCommonProperties(idealTree, root)
-
-    this.idealGraph = root
+    await this.#assignCommonProperties(idealTree, this.idealGraph)
   }
 
-  async workspaceProxy (result, node) {
+  async #workspaceProxy (node) {
+    if (this.#workspaceProxies.has(node)) {
+      return this.#workspaceProxies.get(node)
+    }
+    const result = {}
+    // XXX this goes recursive if we don't set here because assignCommonProperties also calls this.#workspaceProxy
+    this.#workspaceProxies.set(node, result)
     result.localLocation = node.location
@@ -101,7 +138,14 @@ result.localPath = node.path
     result.resolved = node.resolved
-    await this.assignCommonProperties(node, result)
+    await this.#assignCommonProperties(node, result)
+    return result
   }
 
-  async externalProxy (result, node) {
-    await this.assignCommonProperties(node, result)
+  async #externalProxy (node) {
+    if (this.#externalProxies.has(node)) {
+      return this.#externalProxies.get(node)
+    }
+    const result = {}
+    // XXX this goes recursive if we don't set here because assignCommonProperties also calls this.#externalProxy
+    this.#externalProxies.set(node, result)
+    await this.#assignCommonProperties(node, result, !node.hasShrinkwrap)
     if (node.hasShrinkwrap) {
@@ -115,4 +159,3 @@ const dir = join(
       mkdirSync(dir, { recursive: true })
-      // TODO this approach feels wrong
-      // and shouldn't be necessary for shrinkwraps
+      // TODO this approach feels wrong and shouldn't be necessary for shrinkwraps
       await pacote.extract(node.resolved, dir, {
@@ -125,8 +168,13 @@ ...this.options,
       const arb = new Arborist({ ...this.options, path: dir })
-      await arb[_makeIdealGraph]({ dev: false })
-      this.rootNode.external.push(...arb.idealGraph.external)
-      arb.idealGraph.external.forEach(e => {
-        e.root = this.rootNode
-        e.id = `${node.id}=>${e.id}`
+      // Make sure that the ideal tree is build as the rest of the algorithm depends on it.
+      await arb.buildIdealTree({
+        complete: false,
+        dev: false,
       })
+      await arb.makeIdealGraph()
+      this.idealGraph.external.push(...arb.idealGraph.external)
+      for (const edge of arb.idealGraph.external) {
+        edge.root = this.idealGraph
+        edge.id = `${node.id}=>${edge.id}`
+      }
       result.localDependencies = []
@@ -137,3 +185,2 @@ result.externalDependencies = arb.idealGraph.externalDependencies
         ...result.externalDependencies,
-        ...result.localDependencies,
         ...result.externalOptionalDependencies,
@@ -145,22 +192,58 @@ ]
     result.version = node.version
+    return result
   }
 
-  async assignCommonProperties (node, result) {
-    function validEdgesOut (node) {
-      return [...node.edgesOut.values()].filter(e => e.to && e.to.target && !(node.package.bundledDependencies || node.package.bundleDependencies || []).includes(e.to.name))
+  async #assignCommonProperties (node, result, populateDeps = true) {
+    result.root = this.idealGraph
+    // XXX does anything need this?
+    result.id = this.counter++
+    /* istanbul ignore next - packageName is always set for real packages */
+    result.name = result.isWorkspace ? (node.packageName || node.name) : node.name
+    result.packageName = node.packageName || node.name
+    result.package = { ...node.package }
+    result.package.bundleDependencies = undefined
+
+    if (!populateDeps) {
+      return
     }
-    const edges = validEdgesOut(node)
-    const optionalDeps = edges.filter(e => e.optional).map(e => e.to.target)
-    const nonOptionalDeps = edges.filter(e => !e.optional).map(e => e.to.target)
 
-    // When legacyPeerDeps is enabled, peer dep edges are not created on the
-    // node. Resolve them from the tree so they get symlinked in the store.
+    let edges = [...node.edgesOut.values()].filter(edge =>
+      edge.to?.target &&
+      !(node.package.bundledDependencies || node.package.bundleDependencies)?.includes(edge.to.name)
+    )
+
+    // Only omit edge types for root and workspace nodes (matching shouldOmit scope)
+    if ((node.isProjectRoot || node.isWorkspace) && this.#omit.size) {
+      edges = edges.filter(edge => {
+        if (edge.dev && this.#omit.has('dev')) {
+          return false
+        }
+        if (edge.optional && this.#omit.has('optional')) {
+          return false
+        }
+        if (edge.peer && this.#omit.has('peer')) {
+          return false
+        }
+        return true
+      })
+    }
+
+    let nonOptionalDeps = edges.filter(edge => !edge.optional).map(edge => edge.to.target)
+
+    // npm auto-creates 'workspace' edges from root to all workspaces.
+    // For isolated/linked mode, only include workspaces that root explicitly declares as dependencies.
+    if (node.isProjectRoot) {
+      nonOptionalDeps = nonOptionalDeps.filter(n => !n.isWorkspace || this.#rootDeclaredDeps.has(n.packageName))
+    }
+
+    // When legacyPeerDeps is enabled, peer dep edges are not created on the node.
+    // Resolve them from the tree so they get symlinked in the store.
     const peerDeps = node.package.peerDependencies
     if (peerDeps && node.legacyPeerDeps) {
-      const edgeNames = new Set(edges.map(e => e.name))
-      for (const peerName of Object.keys(peerDeps)) {
+      const edgeNames = new Set(edges.map(edge => edge.name))
+      for (const peerName in peerDeps) {
         if (!edgeNames.has(peerName)) {
           const resolved = node.resolve(peerName)
           if (resolved && resolved !== node && !resolved.inert) {
-            nonOptionalDeps.push(resolved)
+            nonOptionalDeps.push(resolved.target)
           }
@@ -171,5 +254,8 @@ }
 
-    result.localDependencies = await Promise.all(nonOptionalDeps.filter(n => n.isWorkspace).map(this.workspaceProxyMemo))
-    result.externalDependencies = await Promise.all(nonOptionalDeps.filter(n => !n.isWorkspace && !n.inert).map(this.externalProxyMemo))
-    result.externalOptionalDependencies = await Promise.all(optionalDeps.filter(n => !n.inert).map(this.externalProxyMemo))
+    // local `file:` deps (non-workspace fsChildren) should be treated as local dependencies, not external, so they get symlinked directly instead of being extracted into the store.
+    const isLocal = (n) => n.isWorkspace || node.fsChildren?.has(n)
+    const optionalDeps = edges.filter(edge => edge.optional).map(edge => edge.to.target)
+    result.localDependencies = await Promise.all(nonOptionalDeps.filter(isLocal).map(n => this.#workspaceProxy(n)))
+    result.externalDependencies = await Promise.all(nonOptionalDeps.filter(n => !isLocal(n) && !n.inert).map(n => this.#externalProxy(n)))
+    result.externalOptionalDependencies = await Promise.all(optionalDeps.filter(n => !n.inert).map(n => this.#externalProxy(n)))
     result.dependencies = [
@@ -180,10 +266,2 @@ ...result.externalDependencies,
     ]
-    result.root = this.rootNode
-    result.id = this.counter++
-    /* istanbul ignore next - packageName is always set for real packages */
-    result.name = result.isWorkspace ? (node.packageName || node.name) : node.name
-    result.packageName = node.packageName || node.name
-    result.package = { ...node.package }
-    result.package.bundleDependencies = undefined
-    result.hasInstallScript = node.hasInstallScript
   }
@@ -230,7 +308,7 @@
 
-      to.edgesOut.forEach(e => {
+      to.edgesOut.forEach(edge => {
         // an edge out should always have a to
         /* istanbul ignore else */
-        if (e.to) {
-          queue.push({ from: e.from, to: e.to })
+        if (edge.to) {
+          queue.push({ from: edge.from, to: edge.to })
         }
@@ -242,121 +320,45 @@ })
 
-  async [_createIsolatedTree] () {
-    await this[_makeIdealGraph](this.options)
-
-    const proxiedIdealTree = this.idealGraph
-
+  async createIsolatedTree () {
+    await this.makeIdealGraph()
     const bundledTree = await this.#createBundledTree()
 
-    const treeHash = (startNode) => {
-      // generate short hash based on the dependency tree
-      // starting at this node
-      const deps = []
-      const branch = []
-      depth({
-        tree: startNode,
-        getChildren: node => node.dependencies,
-        filter: node => node,
-        visit: node => {
-          branch.push(`${node.packageName}@${node.version}`)
-          deps.push(`${branch.join('->')}::${node.resolved}`)
-        },
-        leave: () => {
-          branch.pop()
-        },
-      })
-      deps.sort()
-      return crypto.createHash('shake256', { outputLength: 16 })
-        .update(deps.join(','))
-        .digest('base64')
-        // Node v14 doesn't support base64url
-        .replace(/\+/g, '-')
-        .replace(/\//g, '_')
-        .replace(/=+$/m, '')
-    }
-
-    const getKey = (idealTreeNode) => {
-      return `${idealTreeNode.packageName}@${idealTreeNode.version}-${treeHash(idealTreeNode)}`
-    }
-
-    const root = {
-      fsChildren: [],
-      integrity: null,
-      inventory: new Map(),
-      isLink: false,
-      isRoot: true,
-      binPaths: [],
-      edgesIn: new Set(),
-      edgesOut: new Map(),
-      hasShrinkwrap: false,
-      parent: null,
-      // TODO: we should probably not reference this.idealTree
-      resolved: this.idealTree.resolved,
-      isTop: true,
-      path: proxiedIdealTree.root.localPath,
-      realpath: proxiedIdealTree.root.localPath,
-      package: proxiedIdealTree.root.package,
-      meta: { loadedFromDisk: false },
-      global: false,
-      isProjectRoot: true,
-      children: [],
-    }
-    // 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.query = () => {
-      return []
-    }
+    const root = new IsolatedNode(this.idealGraph)
+    root.root = root
+    root.inventory.set('', root)
     const processed = new Set()
-    proxiedIdealTree.workspaces.forEach(c => {
-      const workspace = {
-        edgesIn: new Set(),
-        edgesOut: new Map(),
-        children: [],
-        hasInstallScript: c.hasInstallScript,
-        binPaths: [],
+    for (const c of this.idealGraph.workspaces) {
+      const wsName = c.packageName
+      // XXX parent? root?
+      const workspace = new IsolatedNode({
+        location: c.localLocation,
+        name: wsName,
         package: c.package,
-        location: c.localLocation,
         path: c.localPath,
-        realpath: c.localPath,
         resolved: c.resolved,
-      }
-      root.fsChildren.push(workspace)
+      })
+      root.fsChildren.add(workspace)
       root.inventory.set(workspace.location, workspace)
-    })
-    const generateChild = (node, location, pkg, inStore) => {
-      const newChild = {
-        global: false,
-        globalTop: false,
-        isProjectRoot: false,
-        isTop: false,
-        location,
-        name: node.packageName || node.name,
-        optional: node.optional,
-        top: { path: proxiedIdealTree.root.localPath },
-        children: [],
-        edgesIn: new Set(),
-        edgesOut: new Map(),
-        binPaths: [],
-        fsChildren: [],
-        /* istanbul ignore next -- emulate Node */
-        getBundler () {
-          return null
-        },
-        hasShrinkwrap: false,
-        inDepBundle: false,
-        integrity: null,
-        isLink: false,
-        isRoot: false,
-        isInStore: inStore,
-        path: join(proxiedIdealTree.root.localPath, location),
-        realpath: join(proxiedIdealTree.root.localPath, location),
-        resolved: node.resolved,
-        version: pkg.version,
-        package: pkg,
+      root.workspaces.set(wsName, workspace.path)
+
+      // Create workspace Link. For root declared deps, link at root node_modules/. For undeclared deps, link at the workspace's own node_modules/ (self-link).
+      const isDeclared = this.#rootDeclaredDeps.has(wsName)
+      const wsLink = new IsolatedLink({
+        location: isDeclared ? join('node_modules', wsName) : join(c.localLocation, 'node_modules', wsName),
+        name: wsName,
+        package: workspace.package,
+        parent: root,
+        path: isDeclared ? join(root.path, 'node_modules', wsName) : join(root.path, c.localLocation, 'node_modules', wsName),
+        realpath: workspace.path,
+        root,
+        target: workspace,
+      })
+      if (!isDeclared) {
+        workspace.children.set(wsName, wsLink)
       }
-      newChild.target = newChild
-      root.children.push(newChild)
-      root.inventory.set(newChild.location, newChild)
+      root.children.set(wsName, wsLink)
+      root.inventory.set(wsLink.location, wsLink)
+      workspace.linksIn.add(wsLink)
     }
-    proxiedIdealTree.external.forEach(c => {
+
+    this.idealGraph.external.forEach(c => {
       const key = getKey(c)
@@ -368,123 +370,116 @@ if (processed.has(key)) {
       const location = join('node_modules', '.store', key, 'node_modules', c.packageName)
-      generateChild(c, location, c.package, true)
+      this.#generateChild(c, location, c.package, true, root)
     })
+
     bundledTree.nodes.forEach(node => {
-      generateChild(node, node.location, node.pkg, false)
+      this.#generateChild(node, node.location, node.pkg, false, root)
     })
-    bundledTree.edges.forEach(e => {
-      const from = e.from === 'root' ? root : root.inventory.get(e.from)
-      const to = root.inventory.get(e.to)
+
+    bundledTree.edges.forEach(edge => {
+      const from = edge.from === 'root' ? root : root.inventory.get(edge.from)
+      const to = root.inventory.get(edge.to)
       // Maybe optional should be propagated from the original edge
-      const edge = { optional: false, from, to }
-      from.edgesOut.set(to.name, edge)
-      to.edgesIn.add(edge)
+      const newEdge = { optional: false, from, to }
+      from.edgesOut.set(to.name, newEdge)
+      to.edgesIn.add(newEdge)
     })
-    const memo = new Set()
 
-    function processEdges (node, externalEdge) {
-      externalEdge = !!externalEdge
-      const key = getKey(node)
-      if (memo.has(key)) {
-        return
-      }
-      memo.add(key)
+    this.#processEdges(this.idealGraph, false, root)
+    for (const node of this.idealGraph.workspaces) {
+      this.#processEdges(node, false, root)
+    }
+    return root
+  }
 
-      let from, nmFolder
-      if (externalEdge) {
-        const fromLocation = join('node_modules', '.store', key, 'node_modules', node.packageName)
-        from = root.children.find(c => c.location === fromLocation)
-        nmFolder = join('node_modules', '.store', key, 'node_modules')
-      } else {
-        from = node.isProjectRoot ? root : root.fsChildren.find(c => c.location === node.localLocation)
-        nmFolder = join(node.localLocation, 'node_modules')
-      }
-      /* istanbul ignore next - strict-peer-deps can exclude nodes from the tree */
-      if (!from) {
-        return
-      }
+  #processEdges (node, externalEdge, root) {
+    const key = getKey(node)
+    if (this.#processedEdges.has(key)) {
+      return
+    }
+    this.#processedEdges.add(key)
 
-      const processDeps = (dep, optional, external) => {
-        optional = !!optional
-        external = !!external
+    let from, nmFolder
+    if (externalEdge) {
+      const fromLocation = join('node_modules', '.store', key, 'node_modules', node.packageName)
+      from = root.children.get(fromLocation)
+      nmFolder = join('node_modules', '.store', key, 'node_modules')
+    } else {
+      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 */
+    if (!from) {
+      return
+    }
 
-        const location = join(nmFolder, dep.name)
-        const binNames = dep.package.bin && Object.keys(dep.package.bin) || []
-        const toKey = getKey(dep)
+    for (const dep of node.localDependencies) {
+      this.#processEdges(dep, false, root)
+      // nonOptional, local
+      this.#processDeps(dep, false, false, root, from, nmFolder)
+    }
+    for (const dep of node.externalDependencies) {
+      this.#processEdges(dep, true, root)
+      // nonOptional, external
+      this.#processDeps(dep, false, true, root, from, nmFolder)
+    }
+    for (const dep of node.externalOptionalDependencies) {
+      this.#processEdges(dep, true, root)
+      // optional, external
+      this.#processDeps(dep, true, true, root, from, nmFolder)
+    }
+  }
 
-        let target
-        if (external) {
-          const toLocation = join('node_modules', '.store', toKey, 'node_modules', dep.packageName)
-          target = root.children.find(c => c.location === toLocation)
-        } else {
-          target = root.fsChildren.find(c => c.location === 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 */
-        if (!target) {
-          return
-        }
+  #processDeps (dep, optional, external, root, from, nmFolder) {
+    const toKey = getKey(dep)
 
-        binNames.forEach(bn => {
-          target.binPaths.push(join(from.realpath, 'node_modules', '.bin', bn))
-        })
+    let target
+    if (external) {
+      const toLocation = join('node_modules', '.store', toKey, 'node_modules', dep.packageName)
+      target = root.children.get(toLocation)
+    } else {
+      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 */
+    if (!target) {
+      return
+    }
 
-        const link = {
-          global: false,
-          globalTop: false,
-          isProjectRoot: false,
-          edgesIn: new Set(),
-          edgesOut: new Map(),
-          binPaths: [],
-          isTop: false,
-          optional,
-          location: location,
-          path: join(dep.root.localPath, nmFolder, dep.name),
-          realpath: target.path,
-          name: toKey,
-          resolved: dep.resolved,
-          top: { path: dep.root.localPath },
-          children: [],
-          fsChildren: [],
-          isLink: true,
-          isStoreLink: true,
-          isRoot: false,
-          package: { _id: 'abc', bundleDependencies: undefined, deprecated: undefined, bin: target.package.bin, scripts: dep.package.scripts },
-          target,
-        }
-        const newEdge1 = { optional, from, to: link }
-        from.edgesOut.set(dep.name, newEdge1)
-        link.edgesIn.add(newEdge1)
-        const newEdge2 = { optional: false, from: link, to: target }
-        link.edgesOut.set(dep.name, newEdge2)
-        target.edgesIn.add(newEdge2)
-        root.children.push(link)
+    if (dep.package.bin) {
+      for (const bn in dep.package.bin) {
+        target.binPaths.push(join(dep.root.localPath, nmFolder, '.bin', bn))
       }
-
-      for (const dep of node.localDependencies) {
-        processEdges(dep, false)
-        // nonOptional, local
-        processDeps(dep, false, false)
-      }
-      for (const dep of node.externalDependencies) {
-        processEdges(dep, true)
-        // nonOptional, external
-        processDeps(dep, false, true)
-      }
-      for (const dep of node.externalOptionalDependencies) {
-        processEdges(dep, true)
-        // optional, external
-        processDeps(dep, true, true)
-      }
     }
 
-    processEdges(proxiedIdealTree, false)
-    for (const node of proxiedIdealTree.workspaces) {
-      processEdges(node, false)
+    const pkg = {
+      _id: dep.package._id,
+      bin: target.package.bin,
+      bundleDependencies: undefined,
+      deprecated: undefined,
+      scripts: dep.package.scripts,
+      version: dep.package.version,
     }
-    root.children.forEach(c => c.parent = root)
-    root.children.forEach(c => c.root = root)
-    root.root = root
-    root.target = root
-    return root
+    const link = new IsolatedLink({
+      isStoreLink: true,
+      location: join(nmFolder, dep.name),
+      name: toKey,
+      optional,
+      parent: root,
+      package: pkg,
+      path: join(dep.root.localPath, nmFolder, dep.name),
+      realpath: target.path,
+      resolved: external ? `file:.store/${toKey}/node_modules/${dep.packageName}` : dep.resolved,
+      root,
+      target,
+    })
+    // XXX top is from place-dep not lib/link.js
+    link.top = { path: dep.root.localPath }
+    const newEdge1 = { optional, from, to: link }
+    from.edgesOut.set(dep.name, newEdge1)
+    link.edgesIn.add(newEdge1)
+    const newEdge2 = { optional: false, from: link, to: target }
+    link.edgesOut.set(dep.name, newEdge2)
+    target.edgesIn.add(newEdge2)
+    root.children.set(link.location, link)
   }
 }

lib/arborist/rebuild.js

@@ -12,2 +12,3 @@ // Arborist.rebuild({path = this.path}) will do all the binlinks and
 const { isNodeGypPackage, defaultGypInstallScript } = require('@npmcli/node-gyp')
+const { promiseRetry } = require('@gar/promise-retry')
 const { log, time } = require('proc-log')
@@ -385,3 +386,4 @@ const { resolve } = require('node:path')
 
-    const p = binLinks({
+    // On Windows, antivirus/indexer can transiently lock files, causing EPERM/EACCES/EBUSY on the rename inside write-file-atomic (used by bin-links/fix-bin.js), so, retry with backoff.
+    const p = promiseRetry((retry) => binLinks({
       pkg: node.package,
@@ -392,3 +394,9 @@ path: node.path,
       global: !!node.globalTop,
-    })
+    }).catch(/* istanbul ignore next - Windows-only transient antivirus locks */ err => {
+      if (process.platform === 'win32' &&
+          (err.code === 'EPERM' || err.code === 'EACCES' || err.code === 'EBUSY')) {
+        return retry(err)
+      }
+      throw err
+    }), { retries: 5, minTimeout: 500 })
 
@@ -395,0 +403,0 @@ await (this.#doHandleOptionalFailure

lib/arborist/reify.js

@@ -11,5 +11,6 @@ // mixin implementing the reify method
 const { depth: dfwalk } = require('treeverse')
-const { dirname, resolve, relative, join } = require('node:path')
+const { dirname, resolve, relative, join, sep } = require('node:path')
 const { log, time } = require('proc-log')
-const { lstat, mkdir, rm, symlink } = require('node:fs/promises')
+const { existsSync } = require('node:fs')
+const { lstat, mkdir, readdir, rm, symlink } = require('node:fs/promises')
 const { moveFile } = require('@npmcli/fs')
@@ -30,2 +31,3 @@ const { subset, intersects } = require('semver')
 const { saveTypeMap, hasSubKey } = require('../add-rm-pkg-deps.js')
+const { IsolatedNode, IsolatedLink } = require('../isolated-classes.js')
 
@@ -68,4 +70,2 @@ // Part of steps (steps need refactoring before we can do anything about these)
 
-const _createIsolatedTree = Symbol.for('createIsolatedTree')
-
 module.exports = cls => class Reifier extends cls {
@@ -81,2 +81,3 @@ #bundleMissing = new Set() // child nodes we'd EXPECT to be included in a bundle, but aren't
   #sparseTreeRoots = new Set()
+  #linkedActualForDiff = null
 
@@ -122,3 +123,6 @@ constructor (options) {
       log.warn('reify', 'The "linked" install strategy is EXPERIMENTAL and may contain bugs.')
-      this.idealTree = await this[_createIsolatedTree]()
+      this.idealTree = await this.createIsolatedTree()
+      this.#linkedActualForDiff = this.#buildLinkedActualForDiff(
+        this.idealTree, this.actualTree
+      )
     }
@@ -128,2 +132,3 @@ await this[_diffTrees]()
     if (linked) {
+      await this.#cleanOrphanedStoreEntries()
       // swap back in the idealTree
@@ -134,2 +139,3 @@ // so that the lockfile is preserved
     await this[_saveIdealTree](options)
+    this.#linkedActualForDiff = null
     // clean inert
@@ -156,3 +162,3 @@ for (const node of this.idealTree.inventory.values()) {
     // save the hidden lockfile.
-    if (this.diff && this.diff.filterSet.size) {
+    if (this.diff && this.diff.filterSet.size && !linked) {
       const reroot = new Set()
@@ -433,9 +439,14 @@
         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)
           }
-          const actual = this.actualTree.children.get(ws)
-          if (actual) {
-            filterNodes.push(actual)
+          // Skip actual-side filterNodes when using the linked diff wrapper.
+          // Those nodes have root===actualTree, not root===linkedActualForDiff, and Diff.calculate requires filterNode.root to match actual.
+          // The ideal filterNode alone is sufficient to scope the workspace diff.
+          if (!this.#linkedActualForDiff) {
+            const actual = this.actualTree.children.get(ws)
+            if (actual) {
+              filterNodes.push(actual)
+            }
           }
@@ -462,3 +473,3 @@ }
       filterNodes,
-      actual: this.actualTree,
+      actual: this.#linkedActualForDiff || this.actualTree,
       ideal: this.idealTree,
@@ -586,2 +597,3 @@ })
       // omit it from the #sparseTreeRoots
+      /* istanbul ignore next -- pre-existing: mkdir returns undefined when dir exists, covered in reify tests but lost in aggregate coverage merge */
       if (made) {
@@ -803,2 +815,55 @@ this.#sparseTreeRoots.add(made)
 
+  // Build a flat actual tree wrapper for linked installs so the diff can correctly match store entries that already exist on disk.
+  // The proxy tree from createIsolatedTree() is flat (all children on root), but loadActual() produces a nested tree where store entries are deep link targets.
+  // This wrapper surfaces them at the root level for comparison.
+  #buildLinkedActualForDiff (idealTree, actualTree) {
+    // Combined Map keyed by path (how allChildren() in diff.js keys)
+    const combined = new Map()
+
+    // 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) || !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
+    // (scoped workspace installs use .get(name), allChildren uses .values())
+    const origGet = actualTree.children.get.bind(actualTree.children)
+    const combinedGet = combined.get.bind(combined)
+    /* istanbul ignore next -- only reached during scoped workspace installs */
+    combined.get = (key) => combinedGet(key) || origGet(key)
+
+    let wrapper
+    /* istanbul ignore next - untested! */
+    if (actualTree.isLink) {
+      wrapper = new IsolatedLink(actualTree)
+    } else {
+      wrapper = new IsolatedNode(actualTree)
+    }
+    wrapper.root = wrapper
+    wrapper.binPaths = actualTree.binPaths
+    wrapper.children = combined
+    wrapper.edgesOut = actualTree.edgesOut
+    // 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
+
+    return wrapper
+  }
+
   #registryResolved (resolved) {
@@ -1262,2 +1327,37 @@ // the default registry url is a magic value meaning "the currently
 
+  // After a linked install, scan node_modules/.store/ and remove any directories that are not referenced by the current ideal tree.
+  // Store entries become orphaned when dependencies are updated or removed, because the diff never sees the old store keys.
+  async #cleanOrphanedStoreEntries () {
+    const storeDir = resolve(this.path, 'node_modules', '.store')
+    let entries
+    try {
+      entries = await readdir(storeDir)
+    } catch {
+      return
+    }
+
+    // Collect valid store keys from the isolated ideal tree (location: node_modules/.store/{key}/node_modules/{pkg})
+    const validKeys = new Set()
+    for (const child of this.idealTree.children.values()) {
+      if (child.isInStore) {
+        const key = child.location.split(sep)[2]
+        validKeys.add(key)
+      }
+    }
+
+    const orphaned = entries.filter(e => !validKeys.has(e))
+    if (!orphaned.length) {
+      return
+    }
+
+    log.silly('reify', 'cleaning orphaned store entries', orphaned)
+    await promiseAllRejectLate(
+      orphaned.map(e =>
+        rm(resolve(storeDir, e), { recursive: true, force: true })
+          .catch(/* istanbul ignore next -- rm with force rarely fails */
+            er => log.warn('cleanup', `Failed to remove orphaned store entry ${e}`, er))
+      )
+    )
+  }
+
   // last but not least, we save the ideal tree metadata to the package-lock
@@ -1264,0 +1364,0 @@ // or shrinkwrap file, and any additions or removals to package.json

lib/audit-report.js

@@ -299,2 +299,4 @@ // an object representing the set of vulnerabilities in a tree
       node.isRoot ||
+      node.isLink ||
+      node.linksIn?.size > 0 ||
       (this.filterSet && this.filterSet?.size !== 0 && !this.filterSet?.has(node))
@@ -301,0 +303,0 @@ ) {

lib/diff.js

@@ -74,2 +74,3 @@ // a tree representing the difference between two trees
         getChildren: node => {
+          const orig = node
           node = node.target
@@ -91,3 +92,8 @@ const loc = node.location
 
-          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
         },
@@ -94,0 +100,0 @@ })

lib/inventory.js

@@ -1,3 +0,2 @@
-// a class to manage an inventory and set of indexes of a set of objects based
-// on specific fields.
+// a class to manage an inventory and set of indexes of a set of objects based on specific fields.
 const { hasOwnProperty } = Object.prototype
@@ -4,0 +3,0 @@ const debug = require('./debug.js')

lib/node.js

@@ -76,31 +76,30 @@ // inventory, path, realpath, root, and parent
     const {
-      root,
-      path,
-      realpath,
-      parent,
+      children,
+      dev = true,
+      devOptional = true,
+      dummy = false,
       error,
-      meta,
+      extraneous = true,
+      fsChildren,
       fsParent,
-      resolved,
+      global = false,
+      hasShrinkwrap,
+      inert = false,
+      installLinks = false,
       integrity,
-      // allow setting name explicitly when we haven't set a path yet
-      name,
-      children,
-      fsChildren,
-      installLinks = false,
+      isInStore = false,
       legacyPeerDeps = false,
       linksIn,
-      isInStore = false,
-      hasShrinkwrap,
-      overrides,
       loadOverrides = false,
-      extraneous = true,
-      dev = true,
+      meta,
+      name, // allow setting name explicitly when we haven't set a path yet
       optional = true,
-      devOptional = true,
+      overrides,
+      parent,
+      path,
       peer = true,
-      global = false,
-      dummy = false,
+      realpath,
+      resolved,
+      root,
       sourceReference = null,
-      inert = false,
     } = options
@@ -107,0 +106,0 @@ // this object gives querySelectorAll somewhere to stash context about a node

lib/query-selector-all.js

@@ -796,5 +796,10 @@ 'use strict'
 
-    // follows logical parent for link ancestors
+    // Follows logical parent for link ancestors (e.g. workspaces whose target lives outside node_modules).
+    // Only match if the node has a link whose parent is the compareNode. Without this check, nodes deep in the store (linked strategy) would incorrectly match as children of root via their fsParent chain.
     if (node.isTop && (node.resolveParent === compareNode)) {
-      return true
+      for (const link of node.linksIn) {
+        if (link.parent === compareNode) {
+          return true
+        }
+      }
     }
@@ -801,0 +806,0 @@ // follows edges-in to check if they match a possible parent

package.json

 {
   "name": "@npmcli/arborist",
-  "version": "9.4.0",
+  "version": "9.4.1",
   "description": "Manage node_modules trees",
   "dependencies": {
+    "@gar/promise-retry": "^1.0.0",
     "@isaacs/string-locale-compare": "^1.1.0",
@@ -7,0 +8,0 @@ "@npmcli/fs": "^5.0.0",

New alerts

Network access

Supply chain risk

This module accesses the network.

Found 1 instance in 1 package

Environment variable access

Supply chain risk

Package accesses environment variables, which may be a sign of credential stuffing or data theft.

Found 6 instances in 1 package

AI-detected potential code anomaly

Supply chain risk

AI has identified unusual behaviors that may pose a security risk.

Found 1 instance in 1 package

Long strings

Supply chain risk

Contains long string literals, which may be a sign of obfuscated or packed code.

Found 1 instance in 1 package

URL strings

Supply chain risk

Package contains fragments of external URLs or IP addresses, which the package may be accessing at runtime.

Found 1 instance in 1 package

Fixed alerts

Network access

Supply chain risk

This module accesses the network.

Found 1 instance in 1 package

Environment variable access

Supply chain risk

Package accesses environment variables, which may be a sign of credential stuffing or data theft.

Found 6 instances in 1 package

AI-detected potential code anomaly

Supply chain risk

AI has identified unusual behaviors that may pose a security risk.

Found 1 instance in 1 package

Long strings

Supply chain risk

Contains long string literals, which may be a sign of obfuscated or packed code.

Found 1 instance in 1 package

URL strings

Supply chain risk

Package contains fragments of external URLs or IP addresses, which the package may be accessing at runtime.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

486289

2.1%

Number of package files

63

1.61%

Lines of code

12814

1.72%

Worsened metrics

Dependency count

34

3.03%

Number of low supply chain risk alerts

40

8.11%

Dependency changes

• + Added

  @gar/promise-retry@^1.0.0