From 4ef5b6e0439297048dee92729b5c93529ad39488 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 27 Feb 2026 08:42:07 -0800 Subject: [PATCH 1/5] fix!: stop resolving node path via whichnode BREAKING CHANGE: npm will no longer attempt to resolve the path to node via whichnode. process.execPath is already set by Node to the resolved real path of the node binary, so the lookup was redundant. Scripts that expected npm to override process.execPath with a PATH-resolved (potentially symlinked) node path may be affected. --- lib/npm.js | 11 --- .../test/lib/cli/exit-handler.js.test.cjs | 2 - .../test/lib/commands/install.js.test.cjs | 12 ++-- test/lib/npm.js | 69 +------------------ 4 files changed, 7 insertions(+), 87 deletions(-) diff --git a/lib/npm.js b/lib/npm.js index b2ab377e95d71..2b4b2843e9218 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -1,6 +1,5 @@ const { resolve, dirname, join } = require('node:path') const Config = require('@npmcli/config') -const which = require('which') const fs = require('node:fs/promises') const { definitions, flatten, nerfDarts, shorthands } = require('@npmcli/config/lib/definitions') const usage = require('./utils/npm-usage.js') @@ -82,16 +81,6 @@ class Npm { } async #load () { - await time.start('npm:load:whichnode', async () => { - // TODO should we throw here? - const node = await which(process.argv[0]).catch(() => {}) - if (node && node.toUpperCase() !== process.execPath.toUpperCase()) { - log.verbose('node symlink', node) - process.execPath = node - this.config.execPath = node - } - }) - await time.start('npm:load:configload', () => this.config.load()) // npm --versions diff --git a/tap-snapshots/test/lib/cli/exit-handler.js.test.cjs b/tap-snapshots/test/lib/cli/exit-handler.js.test.cjs index fd68eea57795b..29865aadf56ed 100644 --- a/tap-snapshots/test/lib/cli/exit-handler.js.test.cjs +++ b/tap-snapshots/test/lib/cli/exit-handler.js.test.cjs @@ -6,7 +6,6 @@ */ 'use strict' exports[`test/lib/cli/exit-handler.js TAP handles unknown error with logs and debug file > debug file contents 1`] = ` -XX timing npm:load:whichnode Completed in {TIME}ms XX silly config load:file:{CWD}/npmrc XX silly config load:file:{CWD}/prefix/.npmrc XX silly config load:file:{CWD}/home/.npmrc @@ -36,7 +35,6 @@ XX error A complete log of this run can be found in: {CWD}/cache/_logs/{DATE}-de ` exports[`test/lib/cli/exit-handler.js TAP handles unknown error with logs and debug file > logs 1`] = ` -timing npm:load:whichnode Completed in {TIME}ms silly config load:file:{CWD}/npmrc silly config load:file:{CWD}/prefix/.npmrc silly config load:file:{CWD}/home/.npmrc diff --git a/tap-snapshots/test/lib/commands/install.js.test.cjs b/tap-snapshots/test/lib/commands/install.js.test.cjs index dd618ee3688f1..1d0a2733a9d2b 100644 --- a/tap-snapshots/test/lib/commands/install.js.test.cjs +++ b/tap-snapshots/test/lib/commands/install.js.test.cjs @@ -135,8 +135,8 @@ verbose stack Error: The developer of this package has specified the following t verbose stack Invalid devEngines.runtime verbose stack Invalid name "nondescript" does not match "node" for "runtime" verbose stack at Install.checkDevEngines ({CWD}/lib/base-cmd.js:247:27) -verbose stack at MockNpm.execCommandClass ({CWD}/lib/npm.js:292:7) -verbose stack at MockNpm.exec ({CWD}/lib/npm.js:193:9) +verbose stack at MockNpm.execCommandClass ({CWD}/lib/npm.js:281:7) +verbose stack at MockNpm.exec ({CWD}/lib/npm.js:182:9) error code EBADDEVENGINES error EBADDEVENGINES The developer of this package has specified the following through devEngines error EBADDEVENGINES Invalid devEngines.runtime @@ -200,8 +200,8 @@ verbose stack Error: The developer of this package has specified the following t verbose stack Invalid devEngines.runtime verbose stack Invalid name "nondescript" does not match "node" for "runtime" verbose stack at Install.checkDevEngines ({CWD}/lib/base-cmd.js:247:27) -verbose stack at MockNpm.execCommandClass ({CWD}/lib/npm.js:292:7) -verbose stack at MockNpm.exec ({CWD}/lib/npm.js:193:9) +verbose stack at MockNpm.execCommandClass ({CWD}/lib/npm.js:281:7) +verbose stack at MockNpm.exec ({CWD}/lib/npm.js:182:9) error code EBADDEVENGINES error EBADDEVENGINES The developer of this package has specified the following through devEngines error EBADDEVENGINES Invalid devEngines.runtime @@ -226,8 +226,8 @@ verbose stack Error: The developer of this package has specified the following t verbose stack Invalid devEngines.runtime verbose stack Invalid name "nondescript" does not match "node" for "runtime" verbose stack at Install.checkDevEngines ({CWD}/lib/base-cmd.js:247:27) -verbose stack at MockNpm.execCommandClass ({CWD}/lib/npm.js:292:7) -verbose stack at MockNpm.exec ({CWD}/lib/npm.js:193:9) +verbose stack at MockNpm.execCommandClass ({CWD}/lib/npm.js:281:7) +verbose stack at MockNpm.exec ({CWD}/lib/npm.js:182:9) error code EBADDEVENGINES error EBADDEVENGINES The developer of this package has specified the following through devEngines error EBADDEVENGINES Invalid devEngines.runtime diff --git a/test/lib/npm.js b/test/lib/npm.js index c4c0d720e86e8..f744d0376e9ea 100644 --- a/test/lib/npm.js +++ b/test/lib/npm.js @@ -1,5 +1,5 @@ const t = require('tap') -const { resolve, dirname, join } = require('node:path') +const { resolve, join } = require('node:path') const fs = require('node:fs/promises') const { time } = require('proc-log') const { load: loadMockNpm } = require('../fixtures/mock-npm.js') @@ -88,73 +88,6 @@ t.test('npm.load', async t => { ]) }) - await t.test('node is a symlink', async t => { - const node = process.platform === 'win32' ? 'node.exe' : 'node' - const { Npm, npm, logs, outputs, prefix } = await loadMockNpm(t, { - prefixDir: { - bin: t.fixture('symlink', dirname(process.execPath)), - }, - config: { - timing: true, - usage: '', - scope: 'foo', - }, - argv: [ - 'token', - 'revoke', - 'blergggg', - ], - globals: (dirs) => ({ - 'process.env.PATH': resolve(dirs.prefix, 'bin'), - 'process.argv': [ - node, - process.argv[1], - ], - }), - }) - - t.equal(npm.config.get('scope'), '@foo', 'added the @ sign to scope') - - t.match([ - ...logs.timing.filter((p) => p.startsWith('npm:load:whichnode')), - ...logs.verbose, - ...logs.timing.filter((p) => p.startsWith('npm:load')), - ], [ - /npm:load:whichnode Completed in [0-9.]+ms/, - `node symlink ${resolve(prefix, 'bin', node)}`, - /title npm token revoke blergggg/, - /argv "token" "revoke" "blergggg".*"--usage" "--scope" "foo"/, - /logfile logs-max:\d+ dir:.*/, - /logfile .*-debug-0.log/, - /npm:load:.* Completed in [0-9.]+ms/, - ]) - t.equal(process.execPath, resolve(prefix, 'bin', node)) - - outputs.length = 0 - logs.length = 0 - await npm.exec('ll', []) - - t.equal(npm.command, 'll', 'command set to first npm command') - t.equal(npm.flatOptions.npmCommand, 'll', 'npmCommand flatOption set') - - const ll = Npm.cmd('ll') - t.same(outputs, [ll.describeUsage], 'print usage') - npm.config.set('usage', false) - - outputs.length = 0 - logs.length = 0 - await npm.exec('get', ['scope', 'usage']) - - t.strictSame([npm.command, npm.flatOptions.npmCommand], ['ll', 'll'], - 'does not change npm.command when another command is called') - - t.match(logs, [ - /timing config:load:flatten Completed in [0-9.]+ms/, - /timing command:config Completed in [0-9.]+ms/, - ]) - t.same(outputs, ['scope=@foo\nusage=false']) - }) - await t.test('--no-workspaces with --workspace', async t => { const { npm } = await loadMockNpm(t, { prefixDir: { From 0dc5585fa284f5c8cac36579983119304775c1c8 Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Mon, 30 Mar 2026 16:09:29 -0400 Subject: [PATCH 2/5] fix(arborist): handle `npm link` with install-strategy=linked --- .../arborist/lib/arborist/isolated-reifier.js | 6 ++- workspaces/arborist/test/isolated-mode.js | 51 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/workspaces/arborist/lib/arborist/isolated-reifier.js b/workspaces/arborist/lib/arborist/isolated-reifier.js index c0f4eccc5f43e..1f086e97b3cd5 100644 --- a/workspaces/arborist/lib/arborist/isolated-reifier.js +++ b/workspaces/arborist/lib/arborist/isolated-reifier.js @@ -113,7 +113,11 @@ module.exports = cls => class IsolatedReifier extends cls { }) // 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)) { + // Links with file: resolved paths (from `npm link`) should also be treated as local dependencies and symlinked directly instead of being extracted into the store. + const isLocalFileDep = next.isLink && next.resolved?.startsWith('file:') + if (isLocalFileDep && !idealTree.fsChildren.has(next) && !idealTree.fsChildren.has(next.target)) { + this.idealGraph.workspaces.push(await this.#workspaceProxy(next.target)) + } else if (!next.isProjectRoot && !next.isWorkspace && !next.inert && !idealTree.fsChildren.has(next) && !idealTree.fsChildren.has(next.target)) { this.idealGraph.external.push(await this.#externalProxy(next)) } } diff --git a/workspaces/arborist/test/isolated-mode.js b/workspaces/arborist/test/isolated-mode.js index 2f0c60257ee3c..3fb74f2ffc29a 100644 --- a/workspaces/arborist/test/isolated-mode.js +++ b/workspaces/arborist/test/isolated-mode.js @@ -1789,6 +1789,57 @@ tap.test('file: dependency with linked strategy', async t => { t.ok(setupRequire(dir)('project2'), 'project2 can be required from root') }) +tap.test('npm link (external file: dep) with linked strategy', async t => { + // Regression test: `npm link` creates a file: dependency pointing outside the project root. + // The linked strategy should symlink it directly instead of trying to extract it into .store/. + const graph = { + registry: [ + { name: 'abbrev', version: '2.0.0' }, + ], + root: { + name: 'my-app', + version: '1.0.0', + dependencies: { abbrev: '2.0.0' }, + }, + } + + const { dir, registry } = await getRepo(graph) + + // Create an external package OUTSIDE the project root (simulates npm link target) + const externalPkgDir = path.join(path.dirname(dir), 'external-pkg') + fs.mkdirSync(externalPkgDir, { recursive: true }) + fs.writeFileSync(path.join(externalPkgDir, 'package.json'), JSON.stringify({ + name: 'external-pkg', + version: '1.0.0', + })) + fs.writeFileSync(path.join(externalPkgDir, 'index.js'), "module.exports = 'external'") + + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + + // First install without the linked package + const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb1.reify({ installStrategy: 'linked' }) + + // Now simulate `npm link external-pkg` by adding a file: dep and reifying + const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb2.reify({ installStrategy: 'linked', add: [`file:${externalPkgDir}`] }) + + // The external package should be symlinked in node_modules + const linkPath = path.join(dir, 'node_modules', 'external-pkg') + const stat = fs.lstatSync(linkPath) + t.ok(stat.isSymbolicLink(), 'external-pkg is a symlink in node_modules') + + // The symlink should resolve to the actual external directory + const realpath = fs.realpathSync(linkPath) + t.equal(realpath, externalPkgDir, 'symlink points to the correct external directory') + + // The existing store packages should still be intact + const storePath = path.join(dir, 'node_modules', '.store') + const storeEntries = fs.readdirSync(storePath) + t.ok(storeEntries.some(e => e.startsWith('abbrev@')), 'abbrev is still in the store') + t.notOk(storeEntries.some(e => e.startsWith('external-pkg@')), 'external-pkg is NOT in the store') +}) + tap.test('subsequent linked install is a no-op', async t => { const graph = { registry: [ From 1ab20c8abd58cadc976429bb3b7b35cd6b627db4 Mon Sep 17 00:00:00 2001 From: Caleb Everett Date: Fri, 17 Apr 2026 09:16:19 -0700 Subject: [PATCH 3/5] fix(arborist): fix infinite loop with bundledDependencies and overrides (#9235) Fixes https://github.com/npm/cli/issues/9227 `npm install` hangs when a project uses `bundledDependencies` and `overrides` targeting a transitive dep shared by multiple bundled deps. In `edge.js` `satisfiedBy()`, the `inBundle` check (added in #4963) uses `rawSpec` for bundled nodes to prevent overrides from applying to pre-resolved deps inside a dependency's tarball. However, `inBundle` is also true for deps the root itself will bundle - these are freshly resolved from the registry and overrides should apply. The override was always applied at placement time (correct version installed), but the edge stayed invalid because `satisfiedBy` checked `rawSpec`. Two bundled deps sharing the overridden transitive dep would endlessly re-queue each other via REPLACE. The fix changes `inBundle` to `inDepBundle`, which is only true when the bundler is a non-root package. This preserves the #4963 behavior for deps pre-resolved inside a dependency's bundle/shrinkwrap while allowing the root's overrides to work. Note: it is unclear whether overrides _should_ be applied to deps that will be bundled or shrinkwrapped. The comment says that we explicitly don't, but I can't find supporting docs, and the existing behavior is that overrides are applied to dependencies that will be bundled/shrinkwrapped. I added tests asserting that behavior. These new tests passed without the change: - overrides do not apply inside a dependency that bundles - node bundled inside a dependency uses rawSpec - node inside a shrinkwrap uses rawSpec These new tests failed, they produced the same tree, but the edges were marked invalid: - node bundled by root uses overridden spec - overrides apply to deps the root will bundle and edges are valid This test hung forever: - does not infinite loop In both cases overrides that are 'baked into' dependnecies appear as 'invalid'. This happens because the root package doesn't read the bundler's overrides, and doesn't know why the shrinkwrap/bundle included the out-of-spec version. This commit doesn't affect that behavior. --- workspaces/arborist/lib/edge.js | 2 +- .../test/arborist/build-ideal-tree.js | 79 +++++++++++++++++++ workspaces/arborist/test/edge.js | 78 ++++++++++++++++++ 3 files changed, 158 insertions(+), 1 deletion(-) diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js index 83a9c18cb6aac..8a5e060ec1427 100644 --- a/workspaces/arborist/lib/edge.js +++ b/workspaces/arborist/lib/edge.js @@ -110,7 +110,7 @@ class Edge { // NOTE: this condition means we explicitly do not support overriding // bundled or shrinkwrapped dependencies - if (node.hasShrinkwrap || node.inShrinkwrap || node.inBundle) { + if (node.hasShrinkwrap || node.inShrinkwrap || node.inDepBundle) { return depValid(node, this.rawSpec, this.#accept, this.#from) } diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index aa632426c03e1..7a87f3ea50826 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -4571,3 +4571,82 @@ t.test('skip invalid peerOptional edges in problemEdges when save=false (#8726)' 'shared stays at 1.1.0 - peerOptional mismatch is not treated as a problem') t.ok(tree.children.get('util'), 'util is in the tree') }) + +t.test('overrides with bundledDependencies', async t => { + t.test('does not infinite loop with bundledDependencies and overrides', async t => { + // https://github.com/npm/cli/issues/9227 + const registry = createRegistry(t, false) + + const bPacks = registry.packuments([ + { version: '1.0.0', dependencies: { bar: '1.0.0' } }, + ], 'b') + const cPacks = registry.packuments([ + { version: '1.0.0', dependencies: { bar: '1.0.0' } }, + ], 'c') + const barPacks = registry.packuments(['1.0.0', '2.0.0'], 'bar') + await registry.package({ manifest: registry.manifest({ name: 'b', packuments: bPacks }) }) + await registry.package({ manifest: registry.manifest({ name: 'c', packuments: cPacks }) }) + await registry.package({ manifest: registry.manifest({ name: 'bar', packuments: barPacks }), times: 2 }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + dependencies: { b: '1.0.0', c: '1.0.0' }, + bundledDependencies: true, + overrides: { bar: '2.0.0' }, + }), + }) + + const tree = await buildIdeal(path) + t.equal(tree.children.get('bar').version, '2.0.0', 'override applied') + }) + + t.test('overrides apply to deps the root will bundle and edges are valid', async t => { + const registry = createRegistry(t, false) + + const fooPacks = registry.packuments([ + { version: '1.0.0', dependencies: { bar: '1.0.0' } }, + ], 'foo') + const barPacks = registry.packuments(['1.0.0', '2.0.0'], 'bar') + await registry.package({ manifest: registry.manifest({ name: 'foo', packuments: fooPacks }) }) + await registry.package({ manifest: registry.manifest({ name: 'bar', packuments: barPacks }) }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + dependencies: { foo: '1.0.0' }, + bundledDependencies: ['foo'], + overrides: { bar: '2.0.0' }, + }), + }) + + const tree = await buildIdeal(path) + t.equal(tree.children.get('bar').version, '2.0.0', 'override installed correct version') + + const fooBarEdge = tree.edgesOut.get('foo').to.edgesOut.get('bar') + t.equal(fooBarEdge.valid, true, 'overridden edge is valid') + }) + + t.test('overrides do not apply inside a dependency that bundles', async t => { + const registry = createRegistry(t, false) + + const depPacks = registry.packuments([{ + version: '1.0.0', + dependencies: { bar: '1.0.0' }, + bundleDependencies: ['bar'], + }], 'dep') + await registry.package({ manifest: registry.manifest({ name: 'dep', packuments: depPacks }) }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + dependencies: { dep: '1.0.0' }, + overrides: { bar: '2.0.0' }, + }), + }) + + const tree = await buildIdeal(path) + t.equal(tree.edgesOut.get('dep').valid, true, 'dep edge is valid') + t.notOk(tree.children.get('bar'), 'bar stays inside dep bundle') + }) +}) diff --git a/workspaces/arborist/test/edge.js b/workspaces/arborist/test/edge.js index e3f6204da9556..739adbbffe258 100644 --- a/workspaces/arborist/test/edge.js +++ b/workspaces/arborist/test/edge.js @@ -1332,3 +1332,81 @@ t.test('edge with overrides should not crash when target has no overrides', t => t.ok(edge.valid, 'edge should be valid') t.end() }) + +t.test('overrides and bundled/shrinkwrapped deps in satisfiedBy', t => { + const makeNode = (name, version, extras = {}) => ({ + name, + packageName: name, + version, + package: { name, version }, + edgesOut: new Map(), + edgesIn: new Set(), + explain: () => `${name}@${version}`, + isTop: false, + parent: top, + resolve: () => undefined, + addEdgeOut (edge) { + this.edgesOut.set(edge.name, edge) + }, + addEdgeIn (edge) { + this.edgesIn.add(edge) + }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, + hasShrinkwrap: false, + inShrinkwrap: false, + inBundle: false, + inDepBundle: false, + ...extras, + }) + + const overrides = new OverrideSet({ overrides: { bar: '2.x' } }) + a.overrides = overrides + + const makeOverriddenEdge = () => { + const edge = new Edge({ + from: a, + type: 'prod', + name: 'bar', + spec: '1.x', + overrides: overrides.getEdgeRule({ name: 'bar', spec: '1.x' }), + }) + reset(a) + a.overrides = overrides + return edge + } + + t.test('node bundled by root uses overridden spec', t => { + const edge = makeOverriddenEdge() + const node = makeNode('bar', '2.0.0', { inBundle: true, inDepBundle: false }) + t.ok(edge.satisfiedBy(node), 'bar@2.0.0 bundled by root satisfies override 2.x') + + const nodeOld = makeNode('bar', '1.0.0', { inBundle: true, inDepBundle: false }) + t.notOk(edge.satisfiedBy(nodeOld), 'bar@1.0.0 bundled by root does not satisfy override 2.x') + t.end() + }) + + t.test('node bundled inside a dependency uses rawSpec', t => { + const edge = makeOverriddenEdge() + const node = makeNode('bar', '1.0.0', { inBundle: true, inDepBundle: true }) + t.ok(edge.satisfiedBy(node), 'bar@1.0.0 in dep bundle satisfies rawSpec 1.x') + + const nodeNew = makeNode('bar', '2.0.0', { inBundle: true, inDepBundle: true }) + t.notOk(edge.satisfiedBy(nodeNew), 'bar@2.0.0 in dep bundle does not satisfy rawSpec 1.x') + t.end() + }) + + t.test('node inside a shrinkwrap uses rawSpec', t => { + const edge = makeOverriddenEdge() + const node = makeNode('bar', '1.0.0', { inShrinkwrap: true }) + t.ok(edge.satisfiedBy(node), 'bar@1.0.0 in shrinkwrap satisfies rawSpec 1.x') + + const nodeNew = makeNode('bar', '2.0.0', { inShrinkwrap: true }) + t.notOk(edge.satisfiedBy(nodeNew), 'bar@2.0.0 in shrinkwrap does not satisfy rawSpec 1.x') + t.end() + }) + + delete a.overrides + t.end() +}) From bc32d94d3845078784603022e9c3504e1c5cde4a Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Fri, 17 Apr 2026 22:05:13 +0530 Subject: [PATCH 4/5] fix(arborist): propagate overrides through Link nodes to targets (#9198) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. When using `install-strategy=linked`, npm overrides for transitive dependencies were ignored. The overridden version was installed but reported as `invalid` instead of `overridden`, and with `strict-peer-deps` the install failed entirely with `ERESOLVE`. The root cause is that override propagation stops at Link nodes and never reaches their targets. Overrides propagate through the tree via `addEdgeIn` -> `updateOverridesEdgeInAdded` -> `recalculateOutEdgesOverrides`. When a Link node receives overrides, `recalculateOutEdgesOverrides` iterates over `this.edgesOut` — but Links have no `edgesOut` (their targets do). So overrides never reach the target node's dependency edges, and those edges use `rawSpec` instead of the overridden spec. In the linked strategy, all packages in `node_modules/` are Links pointing to targets in `.store/`. This meant no overrides propagated past the first level of the dependency tree. The fix overrides `recalculateOutEdgesOverrides` in the `Link` class to forward overrides to the target node. When `buildIdealTree` creates a root Link (e.g. on macOS where `/tmp` -> `/private/tmp`), the target Node is now created with `loadOverrides: true` so it loads override rules from `package.json`. The `#applyRootOverridesToWorkspaces` workaround method is removed — it was compensating for this exact bug by detaching workspace edges whose specs didn't match. With proper propagation, workspace edges already have the correct overridden spec, making the workaround dead code. ## References Fixes #9197 --- .../arborist/lib/arborist/build-ideal-tree.js | 32 +----------- workspaces/arborist/lib/link.js | 8 +++ .../test/arborist/load-virtual.js.test.cjs | 10 +++- .../test/arborist/build-ideal-tree.js | 8 +-- workspaces/arborist/test/link.js | 49 +++++++++++++++++++ 5 files changed, 70 insertions(+), 37 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index fdbbd4679bd80..802e10ddda204 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -13,7 +13,6 @@ const { lstat, readlink } = require('node:fs/promises') const { depth } = require('treeverse') const { log, time } = require('proc-log') const { redact } = require('@npmcli/redact') -const semver = require('semver') const { OK, @@ -294,10 +293,6 @@ module.exports = cls => class IdealTreeBuilder extends cls { }).then(meta => Object.assign(root, { meta })) } else { return this.loadVirtual({ root }) - .then(tree => { - this.#applyRootOverridesToWorkspaces(tree) - return tree - }) } }) @@ -406,6 +401,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { global: this.options.global, installLinks: this.installLinks, legacyPeerDeps: this.legacyPeerDeps, + loadOverrides: true, root, }) } @@ -1507,32 +1503,6 @@ This is a one-time fix-up, please be patient... timeEnd() } - #applyRootOverridesToWorkspaces (tree) { - const rootOverrides = tree.root.package.overrides || {} - - for (const node of tree.root.inventory.values()) { - if (!node.isWorkspace) { - continue - } - - for (const depName of Object.keys(rootOverrides)) { - const edge = node.edgesOut.get(depName) - const rootNode = tree.root.children.get(depName) - - // safely skip if either edge or rootNode doesn't exist yet - if (!edge || !rootNode) { - continue - } - - const resolvedRootVersion = rootNode.package.version - if (!semver.satisfies(resolvedRootVersion, edge.spec)) { - edge.detach() - node.children.delete(depName) - } - } - } - } - #idealTreePrune () { for (const node of this.idealTree.inventory.values()) { if (node.extraneous) { diff --git a/workspaces/arborist/lib/link.js b/workspaces/arborist/lib/link.js index 42bc1faf48860..d200f3d8f8d78 100644 --- a/workspaces/arborist/lib/link.js +++ b/workspaces/arborist/lib/link.js @@ -109,6 +109,14 @@ class Link extends Node { // so this is a no-op [_loadDeps] () {} + // When a Link receives overrides (via edgesIn), forward them to the target node which holds the actual edgesOut. + // Without this, overrides stop at the Link and never reach the target's dependency edges. + recalculateOutEdgesOverrides () { + if (this.target) { + this.target.updateOverridesEdgeInAdded(this.overrides) + } + } + // links can't have children, only their targets can // fix it to an empty list so that we can still call // things that iterate over them, just as a no-op diff --git a/workspaces/arborist/tap-snapshots/test/arborist/load-virtual.js.test.cjs b/workspaces/arborist/tap-snapshots/test/arborist/load-virtual.js.test.cjs index 25ac9457c8c86..7fea252f5126b 100644 --- a/workspaces/arborist/tap-snapshots/test/arborist/load-virtual.js.test.cjs +++ b/workspaces/arborist/tap-snapshots/test/arborist/load-virtual.js.test.cjs @@ -16382,15 +16382,18 @@ ArboristNode { "arg" => ArboristNode { "edgesIn": Set { EdgeIn { - "error": "INVALID", "from": "ws", "name": "arg", + "override": "4.1.3", "spec": "4.1.2", "type": "prod", }, }, "location": "node_modules/arg", "name": "arg", + "overrides": Map { + "arg" => "4.1.3", + }, "path": "{CWD}/test/fixtures/workspaces-with-overrides/node_modules/arg", "resolved": "https://registry.npmjs.org/arg/-/arg-4.1.3.tgz", "version": "4.1.3", @@ -16431,8 +16434,8 @@ ArboristNode { ArboristNode { "edgesOut": Map { "arg" => EdgeOut { - "error": "INVALID", "name": "arg", + "override": "4.1.3", "spec": "4.1.2", "to": "node_modules/arg", "type": "prod", @@ -16441,6 +16444,9 @@ ArboristNode { "isWorkspace": true, "location": "ws", "name": "ws", + "overrides": Map { + "arg" => "4.1.3", + }, "path": "{CWD}/test/fixtures/workspaces-with-overrides/ws", "version": "1.0.0", }, diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 7a87f3ea50826..6400d62b82d9d 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -4064,10 +4064,10 @@ t.test('overrides', async t => { '1.1.1', 'second install: root "abbrev" is still forced to version 1.1.1') - // Overrides should NOT persist unnecessarily - t.notOk( - onepackageNode2.overrides && onepackageNode2.overrides.has('abbrev'), - 'workspace node should not unnecessarily retain overrides after subsequent install' + // Workspace targets inherit the root override set via their parent Link node, which is correct behavior needed for proper override propagation through the dependency tree. + t.ok( + onepackageNode2.overrides, + 'workspace target inherits root overrides via link propagation' ) }) }) diff --git a/workspaces/arborist/test/link.js b/workspaces/arborist/test/link.js index 82c4bda014705..f9b7d8eb1d96d 100644 --- a/workspaces/arborist/test/link.js +++ b/workspaces/arborist/test/link.js @@ -207,6 +207,55 @@ t.test('link gets version from target', t => { t.end() }) +t.test('recalculateOutEdgesOverrides forwards overrides to target', t => { + const root = new Node({ + path: '/path/to/root', + pkg: { + name: 'root', + dependencies: { foo: '1.0.0' }, + overrides: { bar: '2.0.0' }, + }, + loadOverrides: true, + }) + + const target = new Node({ + path: '/path/to/store/foo', + pkg: { + name: 'foo', + version: '1.0.0', + dependencies: { bar: '1.0.0' }, + }, + root, + }) + + const link = new Link({ + pkg: { name: 'foo', version: '1.0.0' }, + path: '/path/to/root/node_modules/foo', + realpath: '/path/to/store/foo', + target, + parent: root, + }) + + // The root has overrides, and the edge from root -> link should propagate them + t.ok(root.overrides, 'root has overrides') + t.ok(link.overrides, 'link received overrides from root edge') + t.ok(link.target.overrides, 'target received overrides forwarded from link') + + // The target's edge to "bar" should have the override applied + const barEdge = link.target.edgesOut.get('bar') + t.ok(barEdge, 'target has edge to bar') + t.ok(barEdge.overrides, 'bar edge has overrides') + t.equal(barEdge.spec, '2.0.0', 'bar edge spec is overridden to 2.0.0') + t.equal(barEdge.rawSpec, '1.0.0', 'bar edge rawSpec is original 1.0.0') + + // recalculateOutEdgesOverrides is a no-op when target is null + link.target = null + t.doesNotThrow(() => link.recalculateOutEdgesOverrides(), + 'no-op when target is null') + + t.end() +}) + t.test('link to root path gets root as target', t => { const root = new Node({ path: '/project/root', From e9b0157b367aef184e7c4e99b90d9fcb8f0bff54 Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Fri, 17 Apr 2026 23:00:23 +0530 Subject: [PATCH 5/5] fix(libnpmexec): skip redundant reify for cached directory specs (#9255) `npx` unconditionally re-reifies `file:`/directory specs on every invocation, even when the package is already installed in the npx cache. This happens because `missingFromTree()` has an early return for directory specs that bypasses the cache lookup entirely. Registry packages correctly skip reify on cache hit by checking `node.package.resolved === manifest._resolved`, but directory specs never reach that check. The fix makes two changes to `missingFromTree()` in `libnpmexec/lib/index.js`: 1. The early return for directory specs is now scoped to non-npx trees (`!isNpxTree`), so the npx cache tree is actually consulted on subsequent runs. 2. Added `node.realpath === manifest._resolved` as an alternative match condition, since `file:` spec nodes in the npx cache have `undefined` for `package.resolved` but their `realpath` contains the matching absolute path. A regression test verifies that running `exec` twice with the same `file:` spec only triggers `reify` once (on the cold cache run). ## References Fixes #9251 --- workspaces/libnpmexec/lib/index.js | 4 +-- workspaces/libnpmexec/test/local.js | 53 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/workspaces/libnpmexec/lib/index.js b/workspaces/libnpmexec/lib/index.js index 502082c6b0dcc..3681653d8217d 100644 --- a/workspaces/libnpmexec/lib/index.js +++ b/workspaces/libnpmexec/lib/index.js @@ -72,12 +72,12 @@ const missingFromTree = async ({ spec, tree, flatOptions, isNpxTree, shallow }) // non-registry spec, or a specific tag, or name only in npx tree. Look up // manifest and check resolved to see if it's in the tree. const manifest = await getManifest(spec, flatOptions) - if (spec.type === 'directory') { + if (spec.type === 'directory' && !isNpxTree) { return { manifest } } const nodesByManifest = tree.inventory.query('packageName', manifest.name) for (const node of nodesByManifest) { - if (node.package.resolved === manifest._resolved) { + if (node.package.resolved === manifest._resolved || node.realpath === manifest._resolved) { // we have a package by the same name and the same resolved destination, nothing to add. return { node } } diff --git a/workspaces/libnpmexec/test/local.js b/workspaces/libnpmexec/test/local.js index 5b0133105393d..23035b01769e3 100644 --- a/workspaces/libnpmexec/test/local.js +++ b/workspaces/libnpmexec/test/local.js @@ -278,6 +278,59 @@ t.test('no npxCache', async t => { }), /Must provide a valid npxCache path/) }) +t.test('local file system path - skips reify on subsequent runs', async t => { + let reifyCount = 0 + const Arborist = require('@npmcli/arborist') + const { exec, chmod, readOutput, rmOutput, path } = setup(t, { + mocks: { + 'ci-info': { isCI: true }, + '@npmcli/arborist': class extends Arborist { + async reify (...args) { + reifyCount++ + return super.reify(...args) + } + }, + }, + testdir: { + a: { + 'package.json': { + name: 'a', + bin: { + a: './index.js', + }, + }, + 'index.js': { key: 'a', value: 'LOCAL PKG' }, + }, + }, + }) + + await chmod('a/index.js') + + // First run should reify (cold cache) + await exec({ + args: [`file:${resolve(path, 'a')}`, 'resfile'], + }) + + t.match(await readOutput('a'), { + value: 'LOCAL PKG', + args: ['resfile'], + }) + t.equal(reifyCount, 1, 'first run should reify') + + await rmOutput('a') + + // Second run should skip reify (cached) + await exec({ + args: [`file:${resolve(path, 'a')}`, 'resfile'], + }) + + t.match(await readOutput('a'), { + value: 'LOCAL PKG', + args: ['resfile'], + }) + t.equal(reifyCount, 1, 'second run should not reify') +}) + t.test('local file system path', async t => { const { exec, chmod, readOutput, path } = setup(t, { mocks: {