From 5723554d6781cd75fce882108601a0ed7d295f78 Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Thu, 25 Jun 2026 00:03:55 +0530 Subject: [PATCH] fix(arborist): repair wrong-but-existing symlink target in linked strategy (#9628) In continuation of our exploration of using `install-strategy=linked` in the [Gutenberg monorepo](https://github.com/WordPress/gutenberg/pull/75814), which powers the WordPress Block Editor. Under `install-strategy=linked`, if a top-level `node_modules/` symlink points to a store key that exists on disk but is the **wrong version**, re-running `npm install` does not repair it. npm reports success and leaves the dependency resolving to the wrong version. This is the state an interrupted update leaves behind: the new store key is extracted but the symlink has not yet been repointed. A symlink pointing at a **non-existent** target is already repaired on reinstall; only a wrong-but-existing target slips through, because cleanup validates the link name, not its target. ## Why For linked installs, `#buildLinkedActualForDiff` synthesizes the "actual" tree the diff compares against from the **ideal** children, never reading the real on-disk symlink target. So a link whose on-disk target is a valid-but-wrong store key looks identical to the ideal node, the diff reports no change, and the symlink is left untouched. The hoisted strategy is unaffected because it self-heals the analogous corruption. ## How In `#buildLinkedActualForDiff`, when an existing link's resolved on-disk target differs from its ideal target, skip creating a synthetic actual entry for it. With no actual entry to match, the diff treats the link as an `ADD`, and `#reifyNode` removes the old symlink and recreates it pointing at the correct store key. A new `#linkTargetMismatch` helper compares the two resolved targets; it runs only after the existing `existsSync` guards, so both paths are known to exist. This repairs both the top-level symlink and wrong transitive/sibling links inside the store, and leaves already-correct trees untouched (no spurious relinking on an idempotent reinstall). ## References Fixes #9611 (cherry picked from commit 0ffce98cb6f2e189fa3a866c1ba1209293fb48f9) --- workspaces/arborist/lib/arborist/reify.js | 12 +++++++- workspaces/arborist/test/isolated-mode.js | 36 +++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index da882db46c180..6f1e2f402e55a 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -11,7 +11,7 @@ const { callLimit: promiseCallLimit } = require('promise-call-limit') const { depth: dfwalk } = require('treeverse') const { dirname, resolve, relative, join, sep } = require('node:path') const { log, time } = require('proc-log') -const { existsSync } = require('node:fs') +const { existsSync, realpathSync } = require('node:fs') const { lstat, mkdir, readdir, readlink, rm, symlink } = require('node:fs/promises') const { moveFile } = require('@npmcli/fs') const { subset, intersects } = require('semver') @@ -904,6 +904,10 @@ module.exports = cls => class Reifier extends cls { if (child.isLink && child.resolved?.startsWith('file:.store/') && !existsSync(child.realpath)) { continue } + // Skip a link whose on-disk target is a valid-but-wrong store key (e.g. an interrupted update), so the diff repoints it via ADD. + if (child.isLink && this.#linkTargetMismatch(child)) { + continue + } let entry if (child.isLink) { entry = new IsolatedLink(child) @@ -943,6 +947,12 @@ module.exports = cls => class Reifier extends cls { return wrapper } + // True when the link's on-disk target resolves to a different path than its ideal target. + // The caller only invokes this once both paths exist, so realpathSync won't throw. + #linkTargetMismatch (child) { + return realpathSync(child.path) !== realpathSync(child.realpath) + } + // When extracting a registry-resolved package, the spec we hand to pacote is name@URL. // pacote re-parses that with npa and gets spec.type === 'remote', so without an override the allow-remote gate would fire on every registry tarball (both =none and =root mis-fire). // Returns true only when we are confident this is a registry-mediated install. diff --git a/workspaces/arborist/test/isolated-mode.js b/workspaces/arborist/test/isolated-mode.js index a82958c12a1c3..4a8047a898daa 100644 --- a/workspaces/arborist/test/isolated-mode.js +++ b/workspaces/arborist/test/isolated-mode.js @@ -414,6 +414,42 @@ tap.test('idempotent install with legacyPeerDeps and workspace peer deps', async } }) +tap.test('reinstall repairs a wrong-but-existing top-level symlink target', async t => { + // Regression for https://github.com/npm/cli/issues/9611: an interrupted update can leave a top-level symlink pointing at a valid-but-wrong store key. Reinstall must repoint it, not treat it as already-correct. + // root depends on a@1.0.0 directly and on b@1.0.0, which pulls a@2.0.0 — so both versions of `a` live in the store. + const graph = { + registry: [ + { name: 'a', version: '1.0.0' }, + { name: 'a', version: '2.0.0' }, + { name: 'b', version: '1.0.0', dependencies: { a: '2.0.0' } }, + ], + root: { + name: 'myroot', version: '1.0.0', dependencies: { a: '1.0.0', b: '1.0.0' }, + }, + } + + const { dir, registry } = await getRepo(graph) + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + const opts = { path: dir, registry, packumentCache: new Map(), cache } + + await new Arborist(opts).reify({ installStrategy: 'linked' }) + + const topLink = path.join(dir, 'node_modules', 'a') + const readVersion = p => JSON.parse(fs.readFileSync(path.join(p, 'package.json'), 'utf8')).version + t.equal(readVersion(fs.realpathSync(topLink)), '1.0.0', 'top-level a resolves to 1.0.0 after install') + + // Simulate an interrupted update: repoint node_modules/a at the wrong (but real) a@2.0.0 store key. + const storeDir = path.join(dir, 'node_modules', '.store') + const wrongKey = fs.readdirSync(storeDir).find(k => k.startsWith('a@2.0.0')) + fs.unlinkSync(topLink) + fs.symlinkSync(path.join('.store', wrongKey, 'node_modules', 'a'), topLink, 'junction') + t.equal(readVersion(fs.realpathSync(topLink)), '2.0.0', 'symlink corrupted to 2.0.0 before reinstall') + + await new Arborist({ ...opts, packumentCache: new Map() }).reify({ installStrategy: 'linked' }) + + t.equal(readVersion(fs.realpathSync(topLink)), '1.0.0', 'reinstall repaired the symlink back to 1.0.0') +}) + tap.test('Lock file is same in hoisted and in isolated mode', async t => { const graph = { registry: [