From 8f34f135f2a686cd4d2e2fa7166445149709441b Mon Sep 17 00:00:00 2001 From: ecanturk Date: Tue, 12 May 2026 10:43:26 +0300 Subject: [PATCH 1/2] fix(arborist): skip lockfile entries for optional deps with incomplete manifests When using a proxy/upstream registry, fetching manifests for platform-specific optional dependencies that the proxy has not cached can return incomplete metadata (missing version field). This caused npm to write lockfile entries like: "node_modules/@esbuild/aix-ppc64": { "optional": true } without a version, resolved, or integrity field. Subsequent `npm ci` runs would fail with "Invalid Version:" errors. Fix (both layers are independently needed): 1. build-ideal-tree: When a registry manifest lacks a version field, treat it as a load failure (EINCOMPLETEMANIFEST) so that #pruneFailedOptional() marks it inert. Only applies to registry specs, not file: dependencies which may legitimately omit version. 2. shrinkwrap: In commit(), inventory iteration does not filter inert nodes, so a separate guard skips writing entries where the node is optional, has no version, is not the root, and has a registry-resolved URL (https://). Local deps (file: or resolved=null) are preserved. Closes: https://github.com/npm/cli/issues/9342 --- .../arborist/lib/arborist/build-ideal-tree.js | 17 ++- workspaces/arborist/lib/shrinkwrap.js | 12 +- .../test/arborist/build-ideal-tree.js | 67 +++++++++ workspaces/arborist/test/shrinkwrap.js | 135 ++++++++++++++++++ 4 files changed, 229 insertions(+), 2 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index c9d9c308cbc9c..7fecd6759c041 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -1351,7 +1351,22 @@ This is a one-time fix-up, please be patient... // doesn't satisfy the edge. try to fetch a manifest and build a node from that. return this.#fetchManifest(spec, parent, edge) .then( - pkg => new Node({ name, pkg, parent, installLinks, legacyPeerDeps }), + pkg => { + // When a proxy/upstream registry returns an incomplete manifest + // (e.g. missing version field for platform-specific packages it + // hasn't cached), treat it as a load failure so that optional deps + // are properly pruned instead of written to the lockfile without + // version metadata. Only apply to registry specs — file: deps + // legitimately omit version. + if (!pkg.version && spec.registry) { + const error = Object.assign( + new Error(`incomplete manifest for ${name}, missing version`), + { code: 'EINCOMPLETEMANIFEST' } + ) + return this.#failureNode(name, parent, error, edge) + } + return new Node({ name, pkg, parent, installLinks, legacyPeerDeps }) + }, error => this.#failureNode(name, parent, error, edge) ) } diff --git a/workspaces/arborist/lib/shrinkwrap.js b/workspaces/arborist/lib/shrinkwrap.js index 8cb1814661c6d..8b5119b7739f3 100644 --- a/workspaces/arborist/lib/shrinkwrap.js +++ b/workspaces/arborist/lib/shrinkwrap.js @@ -909,10 +909,20 @@ class Shrinkwrap { if (node.extraneous && !/(^|\/)node_modules\//.test(loc) && loc !== 'node_modules') { continue } - this.data.packages[loc] = Shrinkwrap.metaFromNode( + const meta = Shrinkwrap.metaFromNode( node, this.path, this.resolveOptions) + // Skip inert nodes — these are optional deps that failed to load + // (e.g. 404 from a proxy registry that hasn't cached the package, + // or incomplete manifest missing version field). + // #pruneFailedOptional marks them inert so they won't be reified; + // writing them to the lockfile produces invalid entries like + // {"optional": true} that cause "Invalid Version:" errors. + if (node.inert) { + continue + } + this.data.packages[loc] = meta } } else if (this.#awaitingUpdate.size > 0) { for (const loc of this.#awaitingUpdate.keys()) { diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 143255a0ec9cd..d6705222ef73e 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -4816,3 +4816,70 @@ t.test('allow-directory=root soft-skips a transitive optional directory dependen t.ok(optChild, 'blocked optional transitive is recorded in the tree') t.equal(optChild.inert, true, 'blocked optional transitive is marked inert (will not be reified)') }) + +t.test('incomplete manifest from proxy registry prunes optional dep (#9342)', async t => { + // When a proxy/upstream registry returns an + // incomplete manifest for a platform-specific optional dep it hasn't + // cached, the version field is missing. Our fix in #nodeFromSpec + // treats this as EINCOMPLETEMANIFEST load failure so that + // #pruneFailedOptional() marks it inert instead of writing a broken + // lockfile entry like {"optional": true}. + const registry = createRegistry(t, false) + + // parent package with an optional dep + const esbuildPack = registry.packument({ + name: 'esbuild', + version: '0.27.7', + optionalDependencies: { + '@esbuild/aix-ppc64': '0.27.7', + }, + }) + const esbuildManifest = registry.manifest({ name: 'esbuild', packuments: [esbuildPack] }) + await registry.package({ manifest: esbuildManifest }) + + // simulate proxy registry returning incomplete manifest (no version field) + await registry.package({ + manifest: { + _id: '@esbuild/aix-ppc64', + _rev: '00-incomplete', + name: '@esbuild/aix-ppc64', + description: 'incomplete proxy manifest', + 'dist-tags': { latest: '0.27.7' }, + versions: { + '0.27.7': { + _id: '@esbuild/aix-ppc64@0.27.7', + name: '@esbuild/aix-ppc64', + // NO version field — this is the proxy registry bug + dependencies: {}, + dist: { + tarball: 'https://registry.npmjs.org/@esbuild/aix-ppc64/-/aix-ppc64-0.27.7.tgz', + }, + }, + }, + }, + }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test-incomplete-manifest', + version: '1.0.0', + devDependencies: { esbuild: '^0.27.0' }, + }), + }) + + const arb = newArb(path) + const tree = await arb.buildIdealTree() + + // esbuild itself should be in the tree + t.ok(tree.children.get('esbuild'), 'esbuild is installed') + t.equal(tree.children.get('esbuild').version, '0.27.7', 'esbuild has correct version') + + // @esbuild/aix-ppc64 should be marked inert (EINCOMPLETEMANIFEST → loadFailure) + // pruneFailedOptional marks it inert so it won't be written to lockfile + const aixNodes = [...tree.inventory.query('name', '@esbuild/aix-ppc64')] + const aixNode = aixNodes.find(n => n.root === tree) + t.ok(aixNode, 'incomplete optional dep node exists in tree') + t.equal(aixNode.inert, true, 'incomplete optional dep is marked inert') + t.equal(aixNode.errors[0].code, 'EINCOMPLETEMANIFEST', + 'node has EINCOMPLETEMANIFEST error') +}) diff --git a/workspaces/arborist/test/shrinkwrap.js b/workspaces/arborist/test/shrinkwrap.js index bcdf50b0f87b0..79389a862caa0 100644 --- a/workspaces/arborist/test/shrinkwrap.js +++ b/workspaces/arborist/test/shrinkwrap.js @@ -863,6 +863,141 @@ t.test('load a hidden lockfile', async t => { t.equal(data.dependencies, undefined, 'deleted legacy metadata') }) +t.test('skip inert nodes in commit', async t => { + // When a proxy registry returns 404 or incomplete manifests for + // platform-specific optional deps, #pruneFailedOptional marks them + // inert. commit() must skip inert nodes — otherwise the lockfile + // gets entries like {"optional": true} without version/resolved/integrity + // that cause "Invalid Version:" errors on subsequent npm ci. + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'proxy-registry-repro', + version: '1.0.0', + devDependencies: { esbuild: '^0.27.0' }, + }), + }) + const meta = new Shrinkwrap({ path }) + meta.data = { + lockfileVersion: 3, + packages: {}, + } + + const root = new Node({ + pkg: { + name: 'proxy-registry-repro', + version: '1.0.0', + devDependencies: { esbuild: '^0.27.0' }, + }, + path, + realpath: path, + }) + + // esbuild with full metadata (valid) + const esbuild = new Node({ + pkg: { + name: 'esbuild', + version: '0.27.7', + optionalDependencies: { + '@esbuild/linux-x64': '0.27.7', + '@esbuild/aix-ppc64': '0.27.7', + }, + }, + name: 'esbuild', + parent: root, + }) + esbuild.dev = true + + // platform dep with full metadata (current platform — valid, NOT inert) + const validDep = new Node({ + pkg: { + name: '@esbuild/linux-x64', + version: '0.27.7', + os: ['linux'], + cpu: ['x64'], + }, + name: '@esbuild/linux-x64', + parent: root, + }) + validDep.optional = true + validDep.dev = true + + // platform dep marked inert (proxy registry 404'd or returned incomplete manifest) + // #pruneFailedOptional sets inert=true on these nodes + const brokenDep = new Node({ + pkg: { + name: '@esbuild/aix-ppc64', + // no version — proxy registry returned 404 or incomplete manifest + }, + name: '@esbuild/aix-ppc64', + parent: esbuild, + }) + brokenDep.optional = true + brokenDep.dev = true + brokenDep.extraneous = false + brokenDep.inert = true + + // file: optional dep WITHOUT version (legitimate — NOT inert, should be kept) + const fileDep = new Node({ + pkg: { + name: 'my-local-optional', + // no version — but this is a file: dep, so it's legitimate + }, + name: 'my-local-optional', + parent: root, + resolved: 'file:../my-local-optional', + }) + fileDep.optional = true + fileDep.extraneous = false + + // local optional dep WITHOUT version or resolved (NOT inert, should be kept) + const localDep = new Node({ + pkg: { + name: 'my-disk-optional', + // no version, no resolved — loaded from local node_modules + }, + name: 'my-disk-optional', + parent: root, + }) + localDep.optional = true + localDep.extraneous = false + + meta.tree = root + const committed = meta.commit() + + // The valid platform dep should be in the lockfile + const validLoc = 'node_modules/@esbuild/linux-x64' + t.ok( + committed.packages[validLoc], + 'valid optional dep is included' + ) + t.equal( + committed.packages[validLoc].version, + '0.27.7', + 'valid optional dep has version' + ) + + // The inert dep should NOT be in the lockfile + const brokenLoc = 'node_modules/esbuild/node_modules/@esbuild/aix-ppc64' + t.notOk( + committed.packages[brokenLoc], + 'inert optional dep is excluded from lockfile' + ) + + // The file: optional dep WITHOUT version SHOULD be kept (not inert) + const fileLoc = 'node_modules/my-local-optional' + t.ok( + committed.packages[fileLoc], + 'file: optional dep without version is preserved in lockfile' + ) + + // The local (resolved=null) optional dep WITHOUT version SHOULD be kept (not inert) + const localLoc = 'node_modules/my-disk-optional' + t.ok( + committed.packages[localLoc], + 'local optional dep without version is preserved in lockfile' + ) +}) + t.test('load a fresh hidden lockfile', async t => { const sw = await Shrinkwrap.reset({ path: hiddenLockfileFixture, From 868e20442343c8529d77ea68b20806b81fecf7ba Mon Sep 17 00:00:00 2001 From: ecanturk <46566566+ecanturk@users.noreply.github.com> Date: Fri, 15 May 2026 18:58:30 +0300 Subject: [PATCH 2/2] Update workspaces/arborist/lib/shrinkwrap.js Co-authored-by: Michael Smith --- workspaces/arborist/lib/shrinkwrap.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workspaces/arborist/lib/shrinkwrap.js b/workspaces/arborist/lib/shrinkwrap.js index 8b5119b7739f3..ce2c58457098d 100644 --- a/workspaces/arborist/lib/shrinkwrap.js +++ b/workspaces/arborist/lib/shrinkwrap.js @@ -919,7 +919,7 @@ class Shrinkwrap { // #pruneFailedOptional marks them inert so they won't be reified; // writing them to the lockfile produces invalid entries like // {"optional": true} that cause "Invalid Version:" errors. - if (node.inert) { + if (node.inert && !node.package.version) { continue } this.data.packages[loc] = meta