From 817665b5e82f6fa4f52c2ffda12e583c373c73ce Mon Sep 17 00:00:00 2001 From: Saibamen Date: Mon, 16 Feb 2026 17:27:35 +0100 Subject: [PATCH 1/9] Tests for #8726 --- test/lib/commands/ci.js | 144 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/test/lib/commands/ci.js b/test/lib/commands/ci.js index 8dc0f1d3cc149..670b27e2d3183 100644 --- a/test/lib/commands/ci.js +++ b/test/lib/commands/ci.js @@ -308,3 +308,147 @@ t.test('should use --workspace flag', async t => { assert.packageMissing('node_modules/abbrev@1.1.0') assert.packageInstalled('node_modules/lodash@1.1.1') }) + +// Issue #8726 - npm ci fails because npm install produces an out-of-sync lockfile +// https://github.com/npm/cli/issues/8726 +// +// Root cause: an optional peerDependency at an exact version causes +// buildIdealTree() to resolve a different version than what's in the lockfile. +// +// Pattern (mirrors real-world addons-linter / htmlhint / node-fetch scenario): +// - scanner@1.0.0 has optional peerDep: fetcher@1.0.0 (exact version) +// - hint@1.0.0 has regular dep: fetcher@^1.0.0 (semver range) +// - npm install resolves fetcher to 1.1.0 (latest matching ^1.0.0) +// - npm ci's buildIdealTree sees fetcher@1.1.0 doesn't satisfy the exact +// peerDep "1.0.0", treats it as a problem edge, resolves fetcher to 1.0.0 +// - validateLockfile: lockfile has 1.1.0, ideal tree has 1.0.0 → MISMATCH + +t.test('issue-8726: npm ci with optional peerDep causing lockfile version mismatch', async t => { + // Pre-built lockfile locks fetcher@1.1.0 (what npm install would pick). + // scanner has optional peerDep fetcher@1.0.0 (exact version). + // buildIdealTree should respect the lockfile version, but the bug causes + // it to resolve fetcher to 1.0.0, failing validateLockfile. + const { npm, registry } = await loadMockNpm(t, { + config: { audit: false, 'ignore-scripts': true }, + strictRegistryNock: false, + prefixDir: { + 'linter-tarball': { + 'package.json': JSON.stringify({ + name: 'linter', + version: '1.0.0', + dependencies: { scanner: '1.0.0' }, + }), + }, + 'scanner-tarball': { + 'package.json': JSON.stringify({ + name: 'scanner', + version: '1.0.0', + peerDependencies: { fetcher: '1.0.0' }, + peerDependenciesMeta: { fetcher: { optional: true } }, + }), + }, + 'hint-tarball': { + 'package.json': JSON.stringify({ + name: 'hint', + version: '1.0.0', + dependencies: { fetcher: '^1.0.0' }, + }), + }, + 'fetcher-1.0.0-tarball': { + 'package.json': JSON.stringify({ name: 'fetcher', version: '1.0.0' }), + }, + 'fetcher-1.1.0-tarball': { + 'package.json': JSON.stringify({ name: 'fetcher', version: '1.1.0' }), + }, + 'package.json': JSON.stringify({ + name: 'test-package', + version: '1.0.0', + devDependencies: { + linter: '1.0.0', + hint: '1.0.0', + }, + }), + 'package-lock.json': JSON.stringify({ + name: 'test-package', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { + '': { + name: 'test-package', + version: '1.0.0', + devDependencies: { linter: '1.0.0', hint: '1.0.0' }, + }, + 'node_modules/linter': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/linter/-/linter-1.0.0.tgz', + dev: true, + dependencies: { scanner: '1.0.0' }, + }, + 'node_modules/scanner': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/scanner/-/scanner-1.0.0.tgz', + dev: true, + peerDependencies: { fetcher: '1.0.0' }, + peerDependenciesMeta: { fetcher: { optional: true } }, + }, + 'node_modules/hint': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/hint/-/hint-1.0.0.tgz', + dev: true, + dependencies: { fetcher: '^1.0.0' }, + }, + 'node_modules/fetcher': { + version: '1.1.0', + resolved: 'https://registry.npmjs.org/fetcher/-/fetcher-1.1.0.tgz', + dev: true, + }, + }, + }), + }, + }) + + // With the fix, buildIdealTree no longer treats the invalid peerOptional + // edge as a problem, so npm ci proceeds to reification and needs tarballs. + const linterManifest = registry.manifest({ name: 'linter' }) + await registry.tarball({ + manifest: linterManifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'linter-tarball'), + }) + + const scannerManifest = registry.manifest({ name: 'scanner' }) + await registry.tarball({ + manifest: scannerManifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'scanner-tarball'), + }) + + const hintManifest = registry.manifest({ name: 'hint' }) + await registry.tarball({ + manifest: hintManifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'hint-tarball'), + }) + + const fetcherManifest = registry.manifest({ + name: 'fetcher', + versions: ['1.0.0', '1.1.0'], + }) + await registry.tarball({ + manifest: fetcherManifest.versions['1.1.0'], + tarball: path.join(npm.prefix, 'fetcher-1.1.0-tarball'), + }) + + // npm ci should succeed - the lockfile is valid and the fix ensures + // the peerOptional edge doesn't cause a version mismatch. + await npm.exec('ci', []) + + const installedFetcher = JSON.parse( + fs.readFileSync( + path.join(npm.prefix, 'node_modules', 'fetcher', 'package.json'), 'utf8' + ) + ) + t.equal( + installedFetcher.version, + '1.1.0', + 'installed the locked fetcher version, not the peer dep version' + ) +}) From 383aa4b9fe91c60eb713eef86ecc3b744af34480 Mon Sep 17 00:00:00 2001 From: Saibamen Date: Mon, 16 Feb 2026 17:29:12 +0100 Subject: [PATCH 2/9] Fix #8726 --- workspaces/arborist/lib/arborist/build-ideal-tree.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index b4319fab6bd37..cbd363212839c 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -339,7 +339,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { filter: node => node, visit: node => { for (const edge of node.edgesOut.values()) { - if ((!edge.to && edge.type !== 'peerOptional') || !edge.valid) { + if (edge.type !== 'peerOptional' && (!edge.to || !edge.valid)) { this.#depsQueue.push(node) break // no need to continue the loop after the first hit } @@ -1165,9 +1165,13 @@ This is a one-time fix-up, please be patient... continue } - // If the edge has an error, there's a problem. + // If the edge has an error, there's a problem, unless + // it's peerOptional and not explicitly requested. if (!edge.valid) { - problems.push(edge) + if (edge.type !== 'peerOptional' || + this.#explicitRequests.has(edge)) { + problems.push(edge) + } continue } From bae3f74607da02445bdc6b6eace5af0d97c90a16 Mon Sep 17 00:00:00 2001 From: Adam Stachowicz Date: Mon, 16 Feb 2026 20:30:41 +0100 Subject: [PATCH 3/9] fix(arborist): re-queue nodes when newly placed dep invalidates peerOptional When a dependency is placed (OK) and creates an invalid peerOptional edge with an already-processed node, re-queue that node for re-resolution. This handles the case where the peerOptional holder is processed before the dep exists in the tree (due to alphabetical processing order), and the dep is later placed by another path with a version that doesn't satisfy the peer spec. Also adds npm install tests for issue #8726 covering both the existing-lockfile and fresh-install (no lockfile) scenarios. Refs: https://github.com/npm/cli/issues/8726 Co-authored-by: Cursor --- test/lib/commands/install.js | 285 ++++++++++++++++++ .../arborist/lib/arborist/build-ideal-tree.js | 28 +- 2 files changed, 307 insertions(+), 6 deletions(-) diff --git a/test/lib/commands/install.js b/test/lib/commands/install.js index 7fd5eb7f16100..e23c16402b67c 100644 --- a/test/lib/commands/install.js +++ b/test/lib/commands/install.js @@ -1,3 +1,4 @@ +const fs = require('node:fs') const tspawk = require('../../fixtures/tspawk') const { cleanCwd, @@ -770,3 +771,287 @@ t.test('devEngines', async t => { t.ok(!output.includes('EBADDEVENGINES')) }) }) + +// Issue #8726 - npm install should re-resolve to satisfy peerOptional constraints +// https://github.com/npm/cli/issues/8726 +// +// When a lockfile has fetcher@1.1.0 but a peerOptional wants fetcher@1.0.0 (exact), +// npm install (save: true) should re-resolve fetcher to 1.0.0 to satisfy both +// the regular dep range (^1.0.0) and the exact peerOptional constraint. +t.test('issue-8726: npm install re-resolves to satisfy peerOptional constraint', async t => { + const { npm, registry } = await loadMockNpm(t, { + config: { audit: false, 'ignore-scripts': true }, + prefixDir: { + 'linter-tarball': { + 'package.json': JSON.stringify({ + name: 'linter', + version: '1.0.0', + dependencies: { scanner: '1.0.0' }, + }), + }, + 'scanner-tarball': { + 'package.json': JSON.stringify({ + name: 'scanner', + version: '1.0.0', + peerDependencies: { fetcher: '1.0.0' }, + peerDependenciesMeta: { fetcher: { optional: true } }, + }), + }, + 'hint-tarball': { + 'package.json': JSON.stringify({ + name: 'hint', + version: '1.0.0', + dependencies: { fetcher: '^1.0.0' }, + }), + }, + 'fetcher-1.0.0-tarball': { + 'package.json': JSON.stringify({ name: 'fetcher', version: '1.0.0' }), + }, + 'fetcher-1.1.0-tarball': { + 'package.json': JSON.stringify({ name: 'fetcher', version: '1.1.0' }), + }, + 'package.json': JSON.stringify({ + name: 'test-package', + version: '1.0.0', + devDependencies: { + linter: '1.0.0', + hint: '1.0.0', + }, + }), + 'package-lock.json': JSON.stringify({ + name: 'test-package', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { + '': { + name: 'test-package', + version: '1.0.0', + devDependencies: { linter: '1.0.0', hint: '1.0.0' }, + }, + 'node_modules/linter': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/linter/-/linter-1.0.0.tgz', + dev: true, + dependencies: { scanner: '1.0.0' }, + }, + 'node_modules/scanner': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/scanner/-/scanner-1.0.0.tgz', + dev: true, + peerDependencies: { fetcher: '1.0.0' }, + peerDependenciesMeta: { fetcher: { optional: true } }, + }, + 'node_modules/hint': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/hint/-/hint-1.0.0.tgz', + dev: true, + dependencies: { fetcher: '^1.0.0' }, + }, + 'node_modules/fetcher': { + version: '1.1.0', + resolved: 'https://registry.npmjs.org/fetcher/-/fetcher-1.1.0.tgz', + dev: true, + }, + }, + }), + }, + }) + + // Only set up mocks that npm install actually needs: + // - Tarballs for all installed packages (linter, scanner, hint, fetcher@1.0.0) + // - Fetcher packument (needed for re-resolution via #problemEdges) + // Packuments for linter/scanner/hint are NOT needed (already in lockfile). + // Fetcher@1.1.0 tarball is NOT needed (gets replaced by 1.0.0). + const linterManifest = registry.manifest({ name: 'linter' }) + await registry.tarball({ + manifest: linterManifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'linter-tarball'), + }) + + const scannerManifest = registry.manifest({ name: 'scanner' }) + await registry.tarball({ + manifest: scannerManifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'scanner-tarball'), + }) + + const hintManifest = registry.manifest({ name: 'hint' }) + await registry.tarball({ + manifest: hintManifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'hint-tarball'), + }) + + const fetcherManifest = registry.manifest({ + name: 'fetcher', + versions: ['1.0.0', '1.1.0'], + }) + await registry.package({ manifest: fetcherManifest }) + await registry.tarball({ + manifest: fetcherManifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'fetcher-1.0.0-tarball'), + }) + + await npm.exec('install', []) + + // Read the updated lockfile and verify fetcher was re-resolved to 1.0.0 + const lockfile = JSON.parse( + fs.readFileSync(path.join(npm.prefix, 'package-lock.json'), 'utf8') + ) + t.equal( + lockfile.packages['node_modules/fetcher'].version, + '1.0.0', + 'lockfile updated fetcher to satisfy peerOptional constraint' + ) + + // Also verify the installed package + const installedFetcher = JSON.parse( + fs.readFileSync( + path.join(npm.prefix, 'node_modules', 'fetcher', 'package.json'), 'utf8' + ) + ) + t.equal( + installedFetcher.version, + '1.0.0', + 'installed fetcher version satisfies peerOptional constraint' + ) +}) + +// Issue #8726 - fresh npm install (no lockfile) should pick a version that +// satisfies both the regular dep range AND the exact peerOptional constraint, +// even when the peerOptional holder is processed BEFORE the dep is placed. +// https://github.com/npm/cli/issues/8726 +// +// This test uses package names that reproduce the real-world alphabetical +// ordering from the original issue (addons-linter < htmlhint), which causes +// addons-scanner to be processed from the queue BEFORE htmlhint places +// node-fetcher. At that point the peerOptional edge has no destination +// (MISSING, valid for peerOptional). Later, htmlhint places node-fetcher@1.1.0 +// and the edge becomes INVALID. The fix re-queues addons-scanner so +// #problemEdges can trigger re-resolution of node-fetcher to 1.0.0. +// +// Dependency graph: +// root -> addons-linter@1.0.0 -> addons-scanner@1.0.0 +// -> PEER_OPTIONAL node-fetcher@1.0.0 +// root -> htmlhint@1.0.0 -> node-fetcher@^1.0.0 +// +// Processing order (alphabetical): +// addons-linter, then addons-scanner (dep of addons-linter), +// THEN htmlhint (which places node-fetcher@1.1.0) +t.test('issue-8726: fresh install re-queues scanner when dep placed later', async t => { + const { npm, registry } = await loadMockNpm(t, { + config: { audit: false, 'ignore-scripts': true }, + prefixDir: { + 'addons-linter-tarball': { + 'package.json': JSON.stringify({ + name: 'addons-linter', + version: '1.0.0', + dependencies: { 'addons-scanner': '1.0.0' }, + }), + }, + 'addons-scanner-tarball': { + 'package.json': JSON.stringify({ + name: 'addons-scanner', + version: '1.0.0', + peerDependencies: { 'node-fetcher': '1.0.0' }, + peerDependenciesMeta: { 'node-fetcher': { optional: true } }, + }), + }, + 'htmlhint-tarball': { + 'package.json': JSON.stringify({ + name: 'htmlhint', + version: '1.0.0', + dependencies: { 'node-fetcher': '^1.0.0' }, + }), + }, + 'node-fetcher-1.0.0-tarball': { + 'package.json': JSON.stringify({ name: 'node-fetcher', version: '1.0.0' }), + }, + 'node-fetcher-1.1.0-tarball': { + 'package.json': JSON.stringify({ name: 'node-fetcher', version: '1.1.0' }), + }, + 'package.json': JSON.stringify({ + name: 'test-package', + version: '1.0.0', + devDependencies: { + 'addons-linter': '1.0.0', + htmlhint: '1.0.0', + }, + }), + // NO package-lock.json — this is a fresh install + }, + }) + + // Fresh install needs packuments for all packages + const linterManifest = registry.manifest({ + name: 'addons-linter', + packuments: [{ version: '1.0.0', dependencies: { 'addons-scanner': '1.0.0' } }], + }) + await registry.package({ manifest: linterManifest }) + await registry.tarball({ + manifest: linterManifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'addons-linter-tarball'), + }) + + const scannerManifest = registry.manifest({ + name: 'addons-scanner', + packuments: [{ + version: '1.0.0', + peerDependencies: { 'node-fetcher': '1.0.0' }, + peerDependenciesMeta: { 'node-fetcher': { optional: true } }, + }], + }) + await registry.package({ manifest: scannerManifest }) + await registry.tarball({ + manifest: scannerManifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'addons-scanner-tarball'), + }) + + const hintManifest = registry.manifest({ + name: 'htmlhint', + packuments: [{ version: '1.0.0', dependencies: { 'node-fetcher': '^1.0.0' } }], + }) + await registry.package({ manifest: hintManifest }) + await registry.tarball({ + manifest: hintManifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'htmlhint-tarball'), + }) + + const fetcherManifest = registry.manifest({ + name: 'node-fetcher', + packuments: [{ version: '1.0.0' }, { version: '1.1.0' }], + }) + // Packument is fetched twice: once when htmlhint resolves node-fetcher@^1.0.0 + // (picking 1.1.0), and again when addons-scanner is re-queued and re-resolves + // node-fetcher (picking 1.0.0 to satisfy the exact peerOptional spec). + await registry.package({ manifest: fetcherManifest, times: 2 }) + await registry.tarball({ + manifest: fetcherManifest.versions['1.0.0'], + tarball: path.join(npm.prefix, 'node-fetcher-1.0.0-tarball'), + }) + // node-fetcher@1.1.0 tarball is NOT needed: it's replaced by 1.0.0 during + // tree building (before reification), so it's never downloaded. + + await npm.exec('install', []) + + // Verify the lockfile has node-fetcher@1.0.0 + const lockfile = JSON.parse( + fs.readFileSync(path.join(npm.prefix, 'package-lock.json'), 'utf8') + ) + t.equal( + lockfile.packages['node_modules/node-fetcher'].version, + '1.0.0', + 'fresh install picks node-fetcher@1.0.0 satisfying peerOptional constraint' + ) + + // Also verify the installed package + const installedFetcher = JSON.parse( + fs.readFileSync( + path.join(npm.prefix, 'node_modules', 'node-fetcher', 'package.json'), 'utf8' + ) + ) + t.equal( + installedFetcher.version, + '1.0.0', + 'installed node-fetcher version satisfies peerOptional constraint' + ) +}) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index cbd363212839c..bc480ffd406c6 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -339,7 +339,8 @@ module.exports = cls => class IdealTreeBuilder extends cls { filter: node => node, visit: node => { for (const edge of node.edgesOut.values()) { - if (edge.type !== 'peerOptional' && (!edge.to || !edge.valid)) { + const skipPeerOptional = edge.type === 'peerOptional' && this.options.save === false + if (!skipPeerOptional && (!edge.to || !edge.valid)) { this.#depsQueue.push(node) break // no need to continue the loop after the first hit } @@ -966,9 +967,21 @@ This is a one-time fix-up, please be patient... continue } const { from, valid, peerConflicted } = edgeIn - if (!peerConflicted && !valid && !this.#depsSeen.has(from)) { - this.addTracker('idealTree', from.name, from.location) - this.#depsQueue.push(edgeIn.from) + if (!peerConflicted && !valid) { + if (this.#depsSeen.has(from) && this.options.save) { + // Re-queue already-processed nodes when a newly placed + // dep creates an invalid edge during npm install + // (save=true). This handles the case where a peerOptional + // dep was valid (missing) when the node was first + // processed, but becomes invalid when the dep is later + // placed by another path with a version that doesn't + // satisfy the peer spec. See npm/cli#8726. + this.#depsSeen.delete(from) + this.#depsQueue.push(from) + } else if (!this.#depsSeen.has(from)) { + this.addTracker('idealTree', from.name, from.location) + this.#depsQueue.push(from) + } } } } else { @@ -1166,9 +1179,12 @@ This is a one-time fix-up, please be patient... } // If the edge has an error, there's a problem, unless - // it's peerOptional and not explicitly requested. + // it's peerOptional and we're not saving (e.g. npm ci), + // in which case we trust the lockfile and skip re-resolution. + // When saving (npm install), peerOptional invalid edges ARE + // treated as problems so the lockfile gets fixed. if (!edge.valid) { - if (edge.type !== 'peerOptional' || + if ((edge.type !== 'peerOptional' || this.options.save !== false) || this.#explicitRequests.has(edge)) { problems.push(edge) } From 95dc220d6fc1c8f4514b8b42e7e6bd503ec5c14a Mon Sep 17 00:00:00 2001 From: Adam Stachowicz Date: Wed, 18 Feb 2026 18:14:20 +0100 Subject: [PATCH 4/9] Comments: every sentence in new line --- test/lib/commands/install.js | 37 ++++++------------- .../arborist/lib/arborist/build-ideal-tree.js | 17 +++------ 2 files changed, 16 insertions(+), 38 deletions(-) diff --git a/test/lib/commands/install.js b/test/lib/commands/install.js index e23c16402b67c..584690b68b5c6 100644 --- a/test/lib/commands/install.js +++ b/test/lib/commands/install.js @@ -775,9 +775,7 @@ t.test('devEngines', async t => { // Issue #8726 - npm install should re-resolve to satisfy peerOptional constraints // https://github.com/npm/cli/issues/8726 // -// When a lockfile has fetcher@1.1.0 but a peerOptional wants fetcher@1.0.0 (exact), -// npm install (save: true) should re-resolve fetcher to 1.0.0 to satisfy both -// the regular dep range (^1.0.0) and the exact peerOptional constraint. +// When a lockfile has fetcher@1.1.0 but a peerOptional wants fetcher@1.0.0 (exact), npm install (save: true) should re-resolve fetcher to 1.0.0 to satisfy both the regular dep range (^1.0.0) and the exact peerOptional constraint. t.test('issue-8726: npm install re-resolves to satisfy peerOptional constraint', async t => { const { npm, registry } = await loadMockNpm(t, { config: { audit: false, 'ignore-scripts': true }, @@ -858,9 +856,7 @@ t.test('issue-8726: npm install re-resolves to satisfy peerOptional constraint', }, }) - // Only set up mocks that npm install actually needs: - // - Tarballs for all installed packages (linter, scanner, hint, fetcher@1.0.0) - // - Fetcher packument (needed for re-resolution via #problemEdges) + // Only set up mocks that npm install actually needs: tarballs for all installed packages (linter, scanner, hint, fetcher@1.0.0) and the fetcher packument (needed for re-resolution via #problemEdges). // Packuments for linter/scanner/hint are NOT needed (already in lockfile). // Fetcher@1.1.0 tarball is NOT needed (gets replaced by 1.0.0). const linterManifest = registry.manifest({ name: 'linter' }) @@ -916,27 +912,19 @@ t.test('issue-8726: npm install re-resolves to satisfy peerOptional constraint', ) }) -// Issue #8726 - fresh npm install (no lockfile) should pick a version that -// satisfies both the regular dep range AND the exact peerOptional constraint, -// even when the peerOptional holder is processed BEFORE the dep is placed. +// Issue #8726 - fresh npm install (no lockfile) should pick a version that satisfies both the regular dep range AND the exact peerOptional constraint, even when the peerOptional holder is processed BEFORE the dep is placed. // https://github.com/npm/cli/issues/8726 // -// This test uses package names that reproduce the real-world alphabetical -// ordering from the original issue (addons-linter < htmlhint), which causes -// addons-scanner to be processed from the queue BEFORE htmlhint places -// node-fetcher. At that point the peerOptional edge has no destination -// (MISSING, valid for peerOptional). Later, htmlhint places node-fetcher@1.1.0 -// and the edge becomes INVALID. The fix re-queues addons-scanner so -// #problemEdges can trigger re-resolution of node-fetcher to 1.0.0. +// This test uses package names that reproduce the real-world alphabetical ordering from the original issue (addons-linter < htmlhint), which causes addons-scanner to be processed from the queue BEFORE htmlhint places node-fetcher. +// At that point the peerOptional edge has no destination (MISSING, valid for peerOptional). +// Later, htmlhint places node-fetcher@1.1.0 and the edge becomes INVALID. +// The fix re-queues addons-scanner so #problemEdges can trigger re-resolution of node-fetcher to 1.0.0. // // Dependency graph: -// root -> addons-linter@1.0.0 -> addons-scanner@1.0.0 -// -> PEER_OPTIONAL node-fetcher@1.0.0 +// root -> addons-linter@1.0.0 -> addons-scanner@1.0.0 -> PEER_OPTIONAL node-fetcher@1.0.0 // root -> htmlhint@1.0.0 -> node-fetcher@^1.0.0 // -// Processing order (alphabetical): -// addons-linter, then addons-scanner (dep of addons-linter), -// THEN htmlhint (which places node-fetcher@1.1.0) +// Processing order (alphabetical): addons-linter, then addons-scanner (dep of addons-linter), THEN htmlhint (which places node-fetcher@1.1.0) t.test('issue-8726: fresh install re-queues scanner when dep placed later', async t => { const { npm, registry } = await loadMockNpm(t, { config: { audit: false, 'ignore-scripts': true }, @@ -1020,16 +1008,13 @@ t.test('issue-8726: fresh install re-queues scanner when dep placed later', asyn name: 'node-fetcher', packuments: [{ version: '1.0.0' }, { version: '1.1.0' }], }) - // Packument is fetched twice: once when htmlhint resolves node-fetcher@^1.0.0 - // (picking 1.1.0), and again when addons-scanner is re-queued and re-resolves - // node-fetcher (picking 1.0.0 to satisfy the exact peerOptional spec). + // Packument is fetched twice: once when htmlhint resolves node-fetcher@^1.0.0 (picking 1.1.0), and again when addons-scanner is re-queued and re-resolves node-fetcher (picking 1.0.0 to satisfy the exact peerOptional spec). await registry.package({ manifest: fetcherManifest, times: 2 }) await registry.tarball({ manifest: fetcherManifest.versions['1.0.0'], tarball: path.join(npm.prefix, 'node-fetcher-1.0.0-tarball'), }) - // node-fetcher@1.1.0 tarball is NOT needed: it's replaced by 1.0.0 during - // tree building (before reification), so it's never downloaded. + // node-fetcher@1.1.0 tarball is NOT needed: it's replaced by 1.0.0 during tree building (before reification), so it's never downloaded. await npm.exec('install', []) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index bc480ffd406c6..7cc201f4c993a 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -969,13 +969,9 @@ This is a one-time fix-up, please be patient... const { from, valid, peerConflicted } = edgeIn if (!peerConflicted && !valid) { if (this.#depsSeen.has(from) && this.options.save) { - // Re-queue already-processed nodes when a newly placed - // dep creates an invalid edge during npm install - // (save=true). This handles the case where a peerOptional - // dep was valid (missing) when the node was first - // processed, but becomes invalid when the dep is later - // placed by another path with a version that doesn't - // satisfy the peer spec. See npm/cli#8726. + // Re-queue already-processed nodes when a newly placed dep creates an invalid edge during npm install (save=true). + // This handles the case where a peerOptional dep was valid (missing) when the node was first processed, but becomes invalid when the dep is later placed by another path with a version that doesn't satisfy the peer spec. + // See npm/cli#8726. this.#depsSeen.delete(from) this.#depsQueue.push(from) } else if (!this.#depsSeen.has(from)) { @@ -1178,11 +1174,8 @@ This is a one-time fix-up, please be patient... continue } - // If the edge has an error, there's a problem, unless - // it's peerOptional and we're not saving (e.g. npm ci), - // in which case we trust the lockfile and skip re-resolution. - // When saving (npm install), peerOptional invalid edges ARE - // treated as problems so the lockfile gets fixed. + // If the edge has an error, there's a problem, unless it's peerOptional and we're not saving (e.g. npm ci), in which case we trust the lockfile and skip re-resolution. + // When saving (npm install), peerOptional invalid edges ARE treated as problems so the lockfile gets fixed. if (!edge.valid) { if ((edge.type !== 'peerOptional' || this.options.save !== false) || this.#explicitRequests.has(edge)) { From 9c703f8db25225920e41149181cd5a6cb3ab7758 Mon Sep 17 00:00:00 2001 From: Adam Stachowicz Date: Wed, 18 Feb 2026 18:17:21 +0100 Subject: [PATCH 5/9] Addendum to 95dc220d6fc1c8f4514b8b42e7e6bd503ec5c14a --- test/lib/commands/ci.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/test/lib/commands/ci.js b/test/lib/commands/ci.js index 670b27e2d3183..6db01d7b6ec7a 100644 --- a/test/lib/commands/ci.js +++ b/test/lib/commands/ci.js @@ -312,22 +312,19 @@ t.test('should use --workspace flag', async t => { // Issue #8726 - npm ci fails because npm install produces an out-of-sync lockfile // https://github.com/npm/cli/issues/8726 // -// Root cause: an optional peerDependency at an exact version causes -// buildIdealTree() to resolve a different version than what's in the lockfile. +// Root cause: an optional peerDependency at an exact version causes buildIdealTree() to resolve a different version than what's in the lockfile. // // Pattern (mirrors real-world addons-linter / htmlhint / node-fetch scenario): // - scanner@1.0.0 has optional peerDep: fetcher@1.0.0 (exact version) // - hint@1.0.0 has regular dep: fetcher@^1.0.0 (semver range) // - npm install resolves fetcher to 1.1.0 (latest matching ^1.0.0) -// - npm ci's buildIdealTree sees fetcher@1.1.0 doesn't satisfy the exact -// peerDep "1.0.0", treats it as a problem edge, resolves fetcher to 1.0.0 +// - npm ci's buildIdealTree sees fetcher@1.1.0 doesn't satisfy the exact peerDep "1.0.0", treats it as a problem edge, resolves fetcher to 1.0.0 // - validateLockfile: lockfile has 1.1.0, ideal tree has 1.0.0 → MISMATCH t.test('issue-8726: npm ci with optional peerDep causing lockfile version mismatch', async t => { // Pre-built lockfile locks fetcher@1.1.0 (what npm install would pick). // scanner has optional peerDep fetcher@1.0.0 (exact version). - // buildIdealTree should respect the lockfile version, but the bug causes - // it to resolve fetcher to 1.0.0, failing validateLockfile. + // buildIdealTree should respect the lockfile version, but the bug causes it to resolve fetcher to 1.0.0, failing validateLockfile. const { npm, registry } = await loadMockNpm(t, { config: { audit: false, 'ignore-scripts': true }, strictRegistryNock: false, @@ -408,8 +405,7 @@ t.test('issue-8726: npm ci with optional peerDep causing lockfile version mismat }, }) - // With the fix, buildIdealTree no longer treats the invalid peerOptional - // edge as a problem, so npm ci proceeds to reification and needs tarballs. + // With the fix, buildIdealTree no longer treats the invalid peerOptional edge as a problem, so npm ci proceeds to reification and needs tarballs. const linterManifest = registry.manifest({ name: 'linter' }) await registry.tarball({ manifest: linterManifest.versions['1.0.0'], @@ -437,8 +433,7 @@ t.test('issue-8726: npm ci with optional peerDep causing lockfile version mismat tarball: path.join(npm.prefix, 'fetcher-1.1.0-tarball'), }) - // npm ci should succeed - the lockfile is valid and the fix ensures - // the peerOptional edge doesn't cause a version mismatch. + // npm ci should succeed - the lockfile is valid and the fix ensures the peerOptional edge doesn't cause a version mismatch. await npm.exec('ci', []) const installedFetcher = JSON.parse( From 076b4fd44bc005e3fd47e9b6486fd595ea59f202 Mon Sep 17 00:00:00 2001 From: Saibamen Date: Wed, 18 Feb 2026 18:57:16 +0100 Subject: [PATCH 6/9] One more test to satisfy 100% Code Coverage --- .../test/arborist/build-ideal-tree.js | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index e56daa7ed76ad..10c3aaa059e2c 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -4460,3 +4460,51 @@ t.test('installLinks behavior with project-internal file dependencies', async t t.equal(edgeToA.to, packageA, 'the edge from b should point to package a') }) }) + +t.test('re-queue already-seen nodes when placed dep invalidates peerOptional (save=true, #8726)', async t => { + // Scenario: alpha has peerOptional on shared@1.0.0, beta has dep on shared@^1.0.0. + // With save=true, alpha is processed first (alphabetical order in DepsQueue), + // its peerOptional is not a problem (missing is OK for peerOptional). + // Beta is processed next, placing shared@1.1.0 (latest ^1.0.0). + // This invalidates alpha's peerOptional edge, triggering re-queue of alpha. + const registry = createRegistry(t, false) + + const alphaPack = registry.packument({ + name: 'alpha', + version: '1.0.0', + peerDependencies: { shared: '1.0.0' }, + peerDependenciesMeta: { shared: { optional: true } }, + }) + const alphaManifest = registry.manifest({ name: 'alpha', packuments: [alphaPack] }) + await registry.package({ manifest: alphaManifest }) + + const betaPack = registry.packument({ + name: 'beta', + version: '1.0.0', + dependencies: { shared: '^1.0.0' }, + }) + const betaManifest = registry.manifest({ name: 'beta', packuments: [betaPack] }) + await registry.package({ manifest: betaManifest }) + + const sharedPacks = registry.packuments(['1.0.0', '1.1.0'], 'shared') + const sharedManifest = registry.manifest({ name: 'shared', packuments: sharedPacks }) + await registry.package({ manifest: sharedManifest, times: 2 }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test-8726-install', + version: '1.0.0', + dependencies: { + alpha: '1.0.0', + beta: '1.0.0', + }, + }), + }) + + const arb = newArb(path, { save: true }) + const tree = await arb.buildIdealTree() + + t.ok(tree.children.get('alpha'), 'alpha is in the tree') + t.ok(tree.children.get('beta'), 'beta is in the tree') + t.ok(tree.children.get('shared'), 'shared is in the tree') +}) From 858e87aaab760a79843541d3676f383cb778cc97 Mon Sep 17 00:00:00 2001 From: Adam Stachowicz Date: Wed, 18 Feb 2026 19:36:34 +0100 Subject: [PATCH 7/9] fix(arborist): Removed the this.#explicitRequests.has(edge) fallback from the !edge.valid peerOptional check. This branch was unreachable because add: [...] changes peerOptional edges to regular dependency edges before they're added to #explicitRequests, so a peerOptional edge can never be both explicitly requested and still typed as peerOptional. The simplified condition edge.type !== 'peerOptional' || this.options.save !== false still correctly handles all reachable cases. --- workspaces/arborist/lib/arborist/build-ideal-tree.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 7cc201f4c993a..a256b79049eba 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -1176,9 +1176,9 @@ This is a one-time fix-up, please be patient... // If the edge has an error, there's a problem, unless it's peerOptional and we're not saving (e.g. npm ci), in which case we trust the lockfile and skip re-resolution. // When saving (npm install), peerOptional invalid edges ARE treated as problems so the lockfile gets fixed. + // See npm/cli#8726. if (!edge.valid) { - if ((edge.type !== 'peerOptional' || this.options.save !== false) || - this.#explicitRequests.has(edge)) { + if (edge.type !== 'peerOptional' || this.options.save !== false) { problems.push(edge) } continue From bda330f3806df4273ea9720b050892cb57c01984 Mon Sep 17 00:00:00 2001 From: Adam Stachowicz Date: Wed, 18 Feb 2026 21:15:14 +0100 Subject: [PATCH 8/9] test(arborist): add test to skip invalid peerOptional edges when save=false This test verifies that invalid peerOptional edges are not treated as problems when the save option is set to false, mimicking the behavior of `npm ci`. It ensures that the dependency tree is built correctly without raising issues for optional peer dependencies that do not meet the specified version requirements. --- .../test/arborist/build-ideal-tree.js | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 10c3aaa059e2c..8a827556b6240 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -4508,3 +4508,68 @@ t.test('re-queue already-seen nodes when placed dep invalidates peerOptional (sa t.ok(tree.children.get('beta'), 'beta is in the tree') t.ok(tree.children.get('shared'), 'shared is in the tree') }) + +t.test('skip invalid peerOptional edges in problemEdges when save=false (#8726)', async t => { + // With save=false (npm ci behavior), invalid peerOptional edges should NOT + // be treated as problems. We use update.names to force alpha into + // #problemEdges while shared@1.1.0 (invalid for alpha's peerOptional + // spec of 1.0.0) is already in the tree from the lockfile. + const registry = createRegistry(t, false) + + const utilPacks = registry.packuments(['1.0.0', '1.0.1'], 'util') + const utilManifest = registry.manifest({ name: 'util', packuments: utilPacks }) + await registry.package({ manifest: utilManifest }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test-8726-ci', + version: '1.0.0', + dependencies: { + alpha: '1.0.0', + beta: '1.0.0', + }, + }), + 'package-lock.json': JSON.stringify({ + name: 'test-8726-ci', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { + '': { + name: 'test-8726-ci', + version: '1.0.0', + dependencies: { alpha: '1.0.0', beta: '1.0.0' }, + }, + 'node_modules/alpha': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/alpha/-/alpha-1.0.0.tgz', + dependencies: { util: '^1.0.0' }, + peerDependencies: { shared: '1.0.0' }, + peerDependenciesMeta: { shared: { optional: true } }, + }, + 'node_modules/beta': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/beta/-/beta-1.0.0.tgz', + dependencies: { shared: '^1.0.0' }, + }, + 'node_modules/shared': { + version: '1.1.0', + resolved: 'https://registry.npmjs.org/shared/-/shared-1.1.0.tgz', + }, + 'node_modules/util': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/util/-/util-1.0.0.tgz', + }, + }, + }), + }) + + const arb = newArb(path, { save: false }) + const tree = await arb.buildIdealTree({ update: { names: ['util'] } }) + + t.ok(tree.children.get('alpha'), 'alpha is in the tree') + t.ok(tree.children.get('beta'), 'beta is in the tree') + t.equal(tree.children.get('shared').version, '1.1.0', + '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') +}) From 448db52ec6577afc5e0bd62b3910dc814b59fc60 Mon Sep 17 00:00:00 2001 From: Saibamen Date: Wed, 18 Feb 2026 21:18:28 +0100 Subject: [PATCH 9/9] Fix comments --- workspaces/arborist/test/arborist/build-ideal-tree.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 8a827556b6240..aa632426c03e1 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -4510,10 +4510,8 @@ t.test('re-queue already-seen nodes when placed dep invalidates peerOptional (sa }) t.test('skip invalid peerOptional edges in problemEdges when save=false (#8726)', async t => { - // With save=false (npm ci behavior), invalid peerOptional edges should NOT - // be treated as problems. We use update.names to force alpha into - // #problemEdges while shared@1.1.0 (invalid for alpha's peerOptional - // spec of 1.0.0) is already in the tree from the lockfile. + // With save=false (npm ci behavior), invalid peerOptional edges should NOT be treated as problems. + // We use update.names to force alpha into #problemEdges while shared@1.1.0 (invalid for alpha's peerOptional spec of 1.0.0) is already in the tree from the lockfile. const registry = createRegistry(t, false) const utilPacks = registry.packuments(['1.0.0', '1.0.1'], 'util')