From bddcb3b1089d2fc29f9c995833ad1758b3611829 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Thu, 22 Oct 2026 00:11:00 -0700 Subject: [PATCH 1/2] fix: reject nameless added packages before arborist reads edge.spec After the manifest is loaded, a spec that still has no name and whose manifest name is missing or empty throws EMISSINAME, carrying the raw spec, before that dependency is recorded. Explicit-request collection adds an edge only when edgesOut returns one. In reify's updateNodes, a missing outgoing edge is skipped, the same way an edge with no target already is. New tests cover a versionless named file: add, a nameless manifest, and a mixed add that leaves no "undefined" dependency key. Fixes #4819 --- .../arborist/lib/arborist/build-ideal-tree.js | 19 ++++- workspaces/arborist/lib/arborist/reify.js | 5 ++ workspaces/arborist/test/arborist/reify.js | 85 +++++++++++++++++++ 3 files changed, 107 insertions(+), 2 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 536b0f8a48f91..98471ee7409ed 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -666,11 +666,17 @@ module.exports = cls => class IdealTreeBuilder extends cls { for (const spec of this[_resolvedAdd]) { if (spec.tree === tree) { - this.#explicitRequests.add(tree.edgesOut.get(spec.name)) + const edge = tree.edgesOut.get(spec.name) + if (edge) { + this.#explicitRequests.add(edge) + } } } for (const name of globalExplicitUpdateNames) { - this.#explicitRequests.add(tree.edgesOut.get(name)) + const edge = tree.edgesOut.get(name) + if (edge) { + this.#explicitRequests.add(edge) + } } this.#depsQueue.push(tree) @@ -699,6 +705,15 @@ module.exports = cls => class IdealTreeBuilder extends cls { _isRoot, before: this.#releaseAgeBefore(spec), }) + // Specs that arrived without a name take it from the manifest. + // A missing name would be stored as the key "undefined", and + // edgesOut.get(undefined) would then throw on edge.spec. + if (!spec.name && (typeof mani.name !== 'string' || mani.name === '')) { + throw Object.assign( + new Error('Invalid package ' + rawSpec + ': package.json is missing a name'), + { code: 'EMISSINAME', spec: rawSpec } + ) + } if (isTag) { // translate tag to a version spec = npa(`${mani.name}@${mani.version}`) diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index b099d4d72c6a4..15ebced5e420e 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -1801,6 +1801,11 @@ module.exports = cls => class Reifier extends cls { for (const { name, tree: addTree } of nodes) { // addTree either the root, or a workspace const edge = addTree.edgesOut.get(name) + // A missing edge is the same skip as a missing edge.to below. + // Nameless adds never create this edge; do not read edge.spec. + if (!edge) { + continue + } const pkg = addTree.package const req = npa.resolve(name, edge.spec, addTree.realpath) const { rawSpec, subSpec } = req diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js index 943e340b22c1f..33cdec1b66400 100644 --- a/workspaces/arborist/test/arborist/reify.js +++ b/workspaces/arborist/test/arborist/reify.js @@ -1868,6 +1868,91 @@ t.test('saving should not replace file: dep with version', async t => { 'still a file: spec after a bare name install') }) +t.test('file dependency may omit version when the manifest has a name', async t => { + const cwd = process.cwd() + t.teardown(() => process.chdir(cwd)) + const path = t.testdir({ + 'named-only': { + 'package.json': JSON.stringify({ + name: 'named-only', + }), + }, + 'package.json': JSON.stringify({}), + }) + createRegistry(t, false) + process.chdir(path) + + await newArb({ path, save: true }).reify({ add: ['file:named-only'] }) + const pj = JSON.parse(fs.readFileSync(resolve(path, 'package.json'), 'utf8')) + t.equal(pj.dependencies['named-only'], 'file:named-only', + 'saved as a file: spec under the manifest name') +}) + +t.test('reject added package whose manifest has no name', async t => { + const cwd = process.cwd() + t.teardown(() => process.chdir(cwd)) + const path = t.testdir({ + noname: { + 'package.json': JSON.stringify({}), + }, + 'package.json': JSON.stringify({}), + }) + createRegistry(t, false) + process.chdir(path) + + const pj = resolve(path, 'package.json') + let err + try { + await newArb({ path, save: true }).reify({ add: ['file:noname'] }) + } catch (er) { + err = er + } + t.match(err, { + code: 'EMISSINAME', + spec: 'file:noname', + message: /missing a name/, + }) + t.notMatch(err && err.message, /reading 'spec'/) + const saved = JSON.parse(fs.readFileSync(pj, 'utf8')) + t.equal(saved.dependencies, undefined, 'parent package.json gains no dependency') +}) + +t.test('nameless file dep fails the whole add', async t => { + const cwd = process.cwd() + t.teardown(() => process.chdir(cwd)) + const path = t.testdir({ + named: { + 'package.json': JSON.stringify({ + name: 'named', + version: '1.0.0', + }), + }, + noname: { + 'package.json': JSON.stringify({ version: '1.2.3' }), + }, + 'package.json': JSON.stringify({}), + }) + createRegistry(t, false) + process.chdir(path) + + const pj = resolve(path, 'package.json') + let err + try { + await newArb({ path, save: true }).reify({ add: ['file:named', 'file:noname'] }) + } catch (er) { + err = er + } + t.match(err, { + code: 'EMISSINAME', + spec: 'file:noname', + message: /missing a name/, + }) + t.notMatch(err && err.message, /reading 'spec'/) + const saved = JSON.parse(fs.readFileSync(pj, 'utf8')) + t.equal(Object.prototype.hasOwnProperty.call(saved.dependencies || {}, 'undefined'), false, + 'no undefined dependency key') +}) + t.test('filtered reification in workspaces', async t => { const path = t.testdir({ 'package.json': JSON.stringify({ From b48f135f437e93656066c6e611010f617eb5483c Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Thu, 22 Oct 2026 08:12:11 -0700 Subject: [PATCH 2/2] fix: drop unreachable edge guards from nameless add path The EMISSINAME check rejects nameless specs before these lookups run, so the guards were never reached and broke the coverage gate. --- workspaces/arborist/lib/arborist/build-ideal-tree.js | 10 ++-------- workspaces/arborist/lib/arborist/reify.js | 5 ----- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 98471ee7409ed..6e3f0c34a0ec3 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -666,17 +666,11 @@ module.exports = cls => class IdealTreeBuilder extends cls { for (const spec of this[_resolvedAdd]) { if (spec.tree === tree) { - const edge = tree.edgesOut.get(spec.name) - if (edge) { - this.#explicitRequests.add(edge) - } + this.#explicitRequests.add(tree.edgesOut.get(spec.name)) } } for (const name of globalExplicitUpdateNames) { - const edge = tree.edgesOut.get(name) - if (edge) { - this.#explicitRequests.add(edge) - } + this.#explicitRequests.add(tree.edgesOut.get(name)) } this.#depsQueue.push(tree) diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index 15ebced5e420e..b099d4d72c6a4 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -1801,11 +1801,6 @@ module.exports = cls => class Reifier extends cls { for (const { name, tree: addTree } of nodes) { // addTree either the root, or a workspace const edge = addTree.edgesOut.get(name) - // A missing edge is the same skip as a missing edge.to below. - // Nameless adds never create this edge; do not read edge.spec. - if (!edge) { - continue - } const pkg = addTree.package const req = npa.resolve(name, edge.spec, addTree.realpath) const { rawSpec, subSpec } = req