fix(arborist): record the linked .store layout in the hidden lockfile (#9630)

In continuation of our exploration of using `install-strategy=linked` in
the [Gutenberg
monorepo](WordPress/gutenberg#75814), which
powers the WordPress Block Editor.

Under `install-strategy=linked`, the hidden lockfile
`node_modules/.package-lock.json` recorded the hoisted logical layout
(`node_modules/<pkg>`) instead of the actual on-disk `.store`/symlink
layout. The hidden lockfile is meant to cache what `loadActual()` finds
on disk so the actual tree can be validated cheaply, but because it
recorded the wrong layout it was rejected on every reload, so it never
served as a cache and misrepresented the installed layout.

## Why

A linked reify swaps `idealTree` for the isolated tree, materializes the
`.store`/symlink layout, then swaps the logical tree back before saving.
The hidden lockfile was serialized from that logical tree, so it listed
packages at their hoisted paths. On the next load, `assertNoNewer()`
walked the real `node_modules` (the root symlink plus `.store/`) and
could not reconcile it with the hoisted entries, throwing `missing from
lockfile`, so `loadActual()` always fell back to a full filesystem scan.

## How

`reify.js` serializes the hidden lockfile from the isolated tree, which
mirrors the on-disk layout, while `package-lock.json` still comes from
the logical tree. It records every store package directory and symlink,
adds an entry for each `.store/<key>` container directory (these are the
fsParents `loadVirtual()` needs so a store package can resolve its
sibling deps), includes the workspace directories, and skips tree-only
undeclared-workspace self-links that are never materialized on disk.

`assertNoNewer()` additionally validates the directories the plain
`node_modules` walk cannot reach under the linked strategy: a store
package's deps live as symlinked siblings under
`.store/<key>/node_modules` (and `.store` is skipped as a dot-dir), and
an undeclared workspace is not symlinked into the root `node_modules` at
all. These directories are derived from the lockfile entries. A
workspace directory is only walked when it is not the target of a link
entry, so the hoisted strategy keeps its existing, stricter validation
unchanged — a stale workspace symlink that points at the wrong target
still surfaces as a missing entry and rejects the cache.

## References

Fixes #9612
Part of #9608

Author: manzoorwanijk

workspaces/arborist/lib/arborist/reify.js

@@ -28,7 +28,8 @@ const { applyPatchToDir, patchIntegrity } = require('../patch.js')
 const { readFile } = require('node:fs/promises')
 const retirePath = require('../retire-path.js')
 const treeCheck = require('../tree-check.js')
-const { defaultLockfileVersion } = require('../shrinkwrap.js')
+const Shrinkwrap = require('../shrinkwrap.js')
+const { defaultLockfileVersion } = Shrinkwrap
 const { saveTypeMap, hasSubKey } = require('../add-rm-pkg-deps.js')
 const { IsolatedNode, IsolatedLink } = require('../isolated-classes.js')
 
@@ -115,12 +116,15 @@ module.exports = cls => class Reifier extends cls {
     await this[_loadTrees](options)
 
     const oldTree = this.idealTree
+    // Kept to serialize the hidden lockfile from the on-disk .store/symlink layout.
+    let isolatedTree = null
     if (linked) {
       // swap out the tree with the isolated tree
       // this is currently technical debt which will be resolved in a refactor
       // of Node/Link trees
       log.warn('reify', 'The "linked" install strategy is EXPERIMENTAL and may contain bugs.')
       this.idealTree = await this.createIsolatedTree()
+      isolatedTree = this.idealTree
       if (this.actualTree) {
         this.#linkedActualForDiff = this.#buildLinkedActualForDiff(
           this.idealTree, this.actualTree
@@ -241,17 +245,24 @@ module.exports = cls => class Reifier extends cls {
       calcDepFlags(this.idealTree)
     }
 
-    // save the ideal's meta as a hidden lockfile after we actualize it
-    this.idealTree.meta.filename =
-      this.idealTree.realpath + '/node_modules/.package-lock.json'
-    this.idealTree.meta.hiddenLockfile = true
-    this.idealTree.meta.lockfileVersion = defaultLockfileVersion
+    // save the ideal's meta as a hidden lockfile after we actualize it.
+    // Under linked the logical tree is the hoisted layout, so the hidden lockfile is serialized from the isolated tree instead.
+    if (!linked) {
+      this.idealTree.meta.filename =
+        this.idealTree.realpath + '/node_modules/.package-lock.json'
+      this.idealTree.meta.hiddenLockfile = true
+      this.idealTree.meta.lockfileVersion = defaultLockfileVersion
+    }
 
     this.actualTree = this.idealTree
     this.idealTree = null
 
     if (!this.options.global && !this.options.dryRun) {
-      await this.actualTree.meta.save()
+      if (linked) {
+        await this.#saveLinkedHiddenLockfile(isolatedTree)
+      } else {
+        await this.actualTree.meta.save()
+      }
       const ignoreScripts = !!this.options.ignoreScripts
       // if we aren't doing a dry run or ignoring scripts and we actually made changes to the dep
       // tree, then run the dependencies scripts
@@ -851,6 +862,47 @@ module.exports = cls => class Reifier extends cls {
     return join(filePath)
   }
 
+  // Serialize the hidden lockfile from the isolated tree, which mirrors the on-disk .store/symlink layout.
+  // Its children are every materialized node_modules entry: store package dirs and all symlinks.
+  async #saveLinkedHiddenLockfile (isolatedTree) {
+    const path = isolatedTree.realpath
+    const meta = new Shrinkwrap({
+      path,
+      hiddenLockfile: true,
+      lockfileVersion: defaultLockfileVersion,
+      resolveOptions: this.options,
+    })
+    meta.reset()
+    meta.filename = resolve(path, 'node_modules/.package-lock.json')
+    const storeRe = /^(.*\/\.store\/.+?)\/node_modules\//
+    const containers = new Set()
+    const nodes = new Set()
+    for (const node of isolatedTree.children.values()) {
+      // Tree-only undeclared workspace self-links aren't on disk.
+      if (node.isUndeclaredWorkspaceLink) {
+        continue
+      }
+      nodes.add(node)
+      // Record the enclosing .store/<key> dir so loadVirtual can resolve a store package's sibling deps.
+      // node.location uses the platform separator; lockfile keys are posix.
+      const m = node.location.replace(/\\/g, '/').match(storeRe)
+      if (m) {
+        containers.add(m[1])
+      }
+    }
+    // Workspace dirs hold their own dep symlinks; record them so the cache can validate those subtrees.
+    for (const ws of isolatedTree.fsChildren) {
+      nodes.add(ws)
+    }
+    for (const node of nodes) {
+      meta.add(node)
+    }
+    for (const loc of containers) {
+      meta.data.packages[loc] = {}
+    }
+    await meta.save()
+  }
+
   // 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.

workspaces/arborist/lib/shrinkwrap.js

@@ -178,6 +178,35 @@ const assertNoNewer = async (path, data, lockTime, dir, seen) => {
     return
   }
 
+  // The walk above can't reach two linked-strategy layouts: a store package's sibling deps under .store/<key>/node_modules (.store is a skipped dot-dir), and an undeclared workspace not symlinked into root node_modules. Walk those dirs, derived from the lockfile entries.
+  // A dir reachable through a link entry is skipped: it was already reached (or, if the symlink is stale, correctly stays unseen so the lockfile is rejected). This keeps the hoisted strategy's stale-symlink detection intact, since there every workspace is a link target.
+  const linkTargets = new Set()
+  for (const loc in data.packages) {
+    const { link, resolved } = data.packages[loc]
+    if (link && resolved) {
+      linkTargets.add(resolved.replace(/\\/g, '/'))
+    }
+  }
+  const extraDirs = new Set()
+  for (const loc in data.packages) {
+    const store = loc.match(/^(.*\/\.store\/.+?)\/node_modules\//)
+    if (store) {
+      // .store/<key> is never walked but has no entry, so mark it seen.
+      seen.add(store[1])
+      extraDirs.add(`${store[1]}/node_modules`)
+      continue
+    }
+    // A workspace/fsChild dir outside node_modules, e.g. packages/a.
+    const i = loc.indexOf('/node_modules/')
+    const root = i === -1 ? loc : loc.slice(0, i)
+    if (root && !/(^|\/)node_modules(\/|$)/.test(root) && !linkTargets.has(root)) {
+      extraDirs.add(root)
+    }
+  }
+  for (const rel of extraDirs) {
+    await assertNoNewer(path, data, lockTime, resolve(path, rel), seen)
+  }
+
   // assert that all the entries in the lockfile were seen
   for (const loc in data.packages) {
     if (!seen.has(loc)) {

workspaces/arborist/test/arborist/reify-npm-extension.js

@@ -215,10 +215,14 @@ t.test('a project with no .npm-extension installs normally and records no state'
 t.test('provenance round-trips under install-strategy=linked', async t => {
   const dir = await setup(t)
   await newArb(dir, { installStrategy: 'linked' }).reify()
-  // a second linked reify rescans the store and links, re-deriving provenance on both
+  // the hidden lockfile carries the provenance forward on a second linked reify
   const tree = await newArb(dir, { installStrategy: 'linked' }).reify()
   const foo = [...tree.inventory.values()].find(n => n.name === 'foo')
   t.ok(foo.npmExtensionApplied || foo.target?.npmExtensionApplied, 'provenance present on the linked node or its target')
+  // a full rescan (no cache) re-derives provenance on the store node and mirrors it onto the link
+  const actual = await newArb(dir, { installStrategy: 'linked' }).loadActual({ forceActual: true })
+  const fooLink = [...actual.inventory.values()].find(n => n.name === 'foo' && n.isLink)
+  t.ok(fooLink?.npmExtensionApplied, 'a rescan mirrors provenance onto the linked location')
 })
 
 t.test('loadActual re-derives provenance only for transformed installed deps', async t => {

workspaces/arborist/test/arborist/reify.js

@@ -4220,6 +4220,119 @@ t.test('install strategy linked', async (t) => {
     t.ok(abbrev.isSymbolicLink(), 'abbrev got installed')
   })
 
+  t.test('hidden lockfile records the linked .store layout and round-trips', async t => {
+    // Regression for #9612: the hidden lockfile must record the on-disk .store/symlink layout so it round-trips as a valid cache.
+    const Shrinkwrap = require('../../lib/shrinkwrap.js')
+    const path = t.testdir({
+      'package.json': JSON.stringify({
+        name: 'root',
+        version: '1.0.0',
+        // once depends on wrappy, so the store has a transitive symlink to validate
+        dependencies: { once: '1.4.0' },
+      }),
+    })
+
+    createRegistry(t, true)
+    await reify(path, { installStrategy: 'linked' })
+
+    const hidden = require(resolve(path, 'node_modules/.package-lock.json'))
+    const locs = Object.keys(hidden.packages)
+    // the layout is recorded at .store paths, not hoisted node_modules/<name>
+    t.ok(locs.some(l => /^node_modules\/\.store\/once@/.test(l)),
+      'once is recorded under .store')
+    t.ok(locs.some(l => /^node_modules\/\.store\/.*\/node_modules\/wrappy$/.test(l)),
+      'the transitive wrappy symlink is recorded inside the store')
+    t.notOk(locs.includes('node_modules/wrappy'),
+      'wrappy is not recorded at a hoisted path')
+
+    // the cache is accepted on reload: assertNoNewer matches it against the real disk layout
+    const meta = await Shrinkwrap.load({ path, hiddenLockfile: true })
+    t.equal(meta.loadedFromDisk, true, 'hidden lockfile is a valid cache of the disk layout')
+
+    // loadActual must reconstruct the tree from the cache with once->wrappy resolved through the store.
+    const actual = await newArb({ path, installStrategy: 'linked' }).loadActual()
+    const onceNode = [...actual.inventory.values()].find(n => n.name === 'once' && !n.isLink)
+    t.ok(onceNode, 'once is in the cached actual tree')
+    const wrappyEdge = onceNode.edgesOut.get('wrappy')
+    t.ok(wrappyEdge && !wrappyEdge.missing, 'once resolves its wrappy dep through the cached store layout')
+  })
+
+  t.test('hidden lockfile round-trips with an undeclared workspace', async t => {
+    // Regression for #9612: an undeclared workspace materializes deps in its own node_modules but isn't linked into root, and the cache must still validate that subtree.
+    const Shrinkwrap = require('../../lib/shrinkwrap.js')
+    const path = t.testdir({
+      'package.json': JSON.stringify({
+        name: 'host',
+        version: '1.0.0',
+        workspaces: ['packages/a'],
+        // root does not depend on the workspace, so it stays undeclared
+      }),
+      packages: {
+        a: {
+          'package.json': JSON.stringify({
+            name: 'a',
+            version: '1.0.0',
+            dependencies: { once: '1.4.0' },
+          }),
+        },
+      },
+    })
+
+    createRegistry(t, true)
+    await reify(path, { installStrategy: 'linked' })
+
+    // the workspace's dep is materialized under its own node_modules, not the root's
+    t.ok(fs.lstatSync(resolve(path, 'packages/a/node_modules/once')).isSymbolicLink(),
+      'once is symlinked into the workspace node_modules')
+    t.notOk(fs.existsSync(resolve(path, 'node_modules/a')),
+      'the undeclared workspace is not symlinked into the root node_modules')
+
+    const meta = await Shrinkwrap.load({ path, hiddenLockfile: true })
+    t.equal(meta.loadedFromDisk, true, 'hidden lockfile validates the undeclared workspace subtree')
+  })
+
+  t.test('hidden lockfile round-trips with an undeclared workspace and no store entries', async t => {
+    // Regression for #9612: with only local deps there is no .store, so the cache must still walk the undeclared workspace subtree to validate it.
+    const Shrinkwrap = require('../../lib/shrinkwrap.js')
+    const path = t.testdir({
+      'package.json': JSON.stringify({
+        name: 'host',
+        version: '1.0.0',
+        workspaces: ['packages/w', 'packages/a', 'packages/b'],
+        // only w is declared; a and b stay undeclared, and a depends on b locally
+        dependencies: { w: '1.0.0' },
+      }),
+      packages: {
+        w: { 'package.json': JSON.stringify({ name: 'w', version: '1.0.0' }) },
+        a: {
+          'package.json': JSON.stringify({
+            name: 'a',
+            version: '1.0.0',
+            dependencies: { b: '1.0.0' },
+          }),
+        },
+        b: { 'package.json': JSON.stringify({ name: 'b', version: '1.0.0' }) },
+      },
+    })
+
+    createRegistry(t, false)
+    await reify(path, { installStrategy: 'linked' })
+
+    t.notOk(fs.existsSync(resolve(path, 'node_modules/.store')),
+      'no store is created for an all-local graph')
+    t.ok(fs.lstatSync(resolve(path, 'packages/a/node_modules/b')).isSymbolicLink(),
+      'the undeclared workspace links its local dep')
+
+    const meta = await Shrinkwrap.load({ path, hiddenLockfile: true })
+    t.equal(meta.loadedFromDisk, true, 'hidden lockfile validates the subtree without any store entry')
+
+    // loadActual must reconstruct the undeclared workspace from the cache with its local dep resolved.
+    const actual = await newArb({ path, installStrategy: 'linked' }).loadActual()
+    const aNode = [...actual.inventory.values()].find(n => n.name === 'a' && !n.isLink)
+    const bEdge = aNode && aNode.edgesOut.get('b')
+    t.ok(bEdge && !bEdge.missing, 'the undeclared workspace resolves its local dep through the cache')
+  })
+
   t.test('does not re-create a workspace dir removed from manifest', async t => {
     // Regression test for https://github.com/npm/cli/issues/9331
     const path = t.testdir({