Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion workspaces/arborist/lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
}
Expand Down
12 changes: 11 additions & 1 deletion workspaces/arborist/lib/shrinkwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 && !node.package.version) {
continue
}
this.data.packages[loc] = meta
}
} else if (this.#awaitingUpdate.size > 0) {
for (const loc of this.#awaitingUpdate.keys()) {
Expand Down
67 changes: 67 additions & 0 deletions workspaces/arborist/test/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
135 changes: 135 additions & 0 deletions workspaces/arborist/test/shrinkwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading