From b8ebdb9e4f0f0807b309ce1bd0f228d04363a34d Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Thu, 25 Jun 2026 23:54:34 +0530 Subject: [PATCH 1/2] fix(reify): report added count for fresh linked installs (#9661) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under `install-strategy=linked`, a fresh `npm install` that actually creates `.store` entries and symlinks printed `up to date, audited N packages` instead of `added N packages`, misleading users and CI into thinking nothing changed. `reify-output` counts adds with `actualTree.inventory.has(d.ideal)`. Under the linked strategy the diff's `ideal` nodes belong to the isolated store tree (locations under `node_modules/.store//node_modules/`), while the `actualTree` returned after reify is the logical hoisted-layout tree. The two never share node identity, so the lookup always failed and `added` stayed `0`. The ADD branch now also counts a node when it is a real store package node (`isInStore && !isLink`), which maps 1:1 to a hoisted package add. This excludes the internal store symlinks and the top-level consumer symlink, so the count matches hoisted for registry dependencies (verified: `minimatch` 4/4, `rimraf` 12/12, `express` 69/69). Omitted/incompatible optional deps have no store entry and are still not counted, and hoisted behavior is unchanged. Known limitations (tracked separately): workspace and `file:`/`npm link` additions are represented as links rather than store nodes under linked, so they are not yet counted here — counting them is entangled with the self-link bug (#9398) and the undeclared-workspaces bug (#9618). The `--json` `add[].path` for a counted store package points to its `.store` realpath rather than a logical `node_modules/` path. ## References Fixes #9623 (cherry picked from commit 0c339479113d8f2c2e3455afc1344037e33e532a) --- lib/utils/reify-output.js | 3 ++- .../test/lib/utils/reify-output.js.test.cjs | 5 +++++ test/lib/utils/reify-output.js | 19 +++++++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/utils/reify-output.js b/lib/utils/reify-output.js index dcef8d0aac845..6e25f94340632 100644 --- a/lib/utils/reify-output.js +++ b/lib/utils/reify-output.js @@ -66,7 +66,8 @@ const reifyOutput = (npm, arb, extras = {}) => { if (showDiff) { output.standard(`${chalk.green('add')} ${d.ideal.name} ${d.ideal.package.version}`) } - if (actualTree.inventory.has(d.ideal)) { + // Linked store packages live under .store, absent from the logical actualTree, so identity lookup misses them; count each store package node (non-link). + if (actualTree.inventory.has(d.ideal) || (d.ideal.isInStore && !d.ideal.isLink)) { summary.added++ summary.add.push({ name: d.ideal.name, diff --git a/tap-snapshots/test/lib/utils/reify-output.js.test.cjs b/tap-snapshots/test/lib/utils/reify-output.js.test.cjs index 40d4cd221ca02..828a99679eab6 100644 --- a/tap-snapshots/test/lib/utils/reify-output.js.test.cjs +++ b/tap-snapshots/test/lib/utils/reify-output.js.test.cjs @@ -10,6 +10,11 @@ exports[`test/lib/utils/reify-output.js TAP added packages should be looked up w added 1 package in {TIME} ` +exports[`test/lib/utils/reify-output.js TAP added packages should be looked up within returned tree linked store package counted though absent from actualTree > must match snapshot 1`] = ` + +added 1 package in {TIME} +` + exports[`test/lib/utils/reify-output.js TAP added packages should be looked up within returned tree missing added pkg in inventory > must match snapshot 1`] = ` up to date in {TIME} diff --git a/test/lib/utils/reify-output.js b/test/lib/utils/reify-output.js index 596272f21fa97..0f2be24b0246e 100644 --- a/test/lib/utils/reify-output.js +++ b/test/lib/utils/reify-output.js @@ -391,6 +391,25 @@ t.test('added packages should be looked up within returned tree', async t => { t.matchSnapshot(out) }) + + t.test('linked store package counted though absent from actualTree', async t => { + const out = await mockReify(t, { + actualTree: { + name: 'foo', + inventory: { + has: () => false, + }, + }, + diff: { + children: [ + { action: 'ADD', ideal: { path: 'test/baz', name: 'baz', isInStore: true, isLink: false, package: { version: '1.0.0' } } }, + { action: 'ADD', ideal: { path: 'test/baz-link', name: 'baz', isInStore: false, isLink: true, package: { version: '1.0.0' } } }, + ], + }, + }) + + t.matchSnapshot(out) + }) }) t.test('prints dedupe difference on dry-run', async t => { From 6074b25db637b61414969b3533413d55ff7a86e0 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 25 Jun 2026 11:51:46 -0700 Subject: [PATCH 2/2] chore(ls): exercise linked workspace edge filter to restore coverage The `ls --install-strategy=linked` tests for undeclared and declared-but-missing workspaces passed without ever entering the undeclared-workspace branch of `filterLinkedStrategyEdges` in lib/commands/ls.js, because their mock filesystems omitted the hidden `node_modules/.package-lock.json` that a real linked install writes. Without that hidden lockfile, loadActual resolves the workspace root edges via the workspace globs instead of marking them missing, so the `edge.missing` guard is never satisfied and the branch stays uncovered. This surfaced as a global coverage gate failure (ls.js branch below 100%) on the first PR after the actual-tree workspace changes in #9666, because the Test matrix only runs on PRs and not on direct pushes to release/v11. Add the hidden lockfile to both tests so they reproduce a real linked install: the undeclared workspace now resolves as a missing root edge and is correctly skipped, and the declared-but-missing workspace resolves as missing and is still reported as UNMET DEPENDENCY. This exercises both branches and restores 100% coverage. No production code changes. --- test/lib/commands/ls.js | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/test/lib/commands/ls.js b/test/lib/commands/ls.js index ab98773bc68e5..17733bc03ef3f 100644 --- a/test/lib/commands/ls.js +++ b/test/lib/commands/ls.js @@ -5332,6 +5332,25 @@ t.test('ls --install-strategy=linked', async t => { node_modules: { 'workspace-a': t.fixture('symlink', '../packages/workspace-a'), // workspace-b intentionally NOT linked (undeclared in dependencies) + // The hidden lockfile a real linked install writes records only the + // declared workspace as linked into root node_modules, so loadActual + // resolves workspace-b's root edge as missing (the undeclared-workspace case). + '.package-lock.json': JSON.stringify({ + lockfileVersion: 3, + requires: true, + packages: { + 'node_modules/workspace-a': { + resolved: 'packages/workspace-a', + link: true, + }, + 'packages/workspace-a': { + version: '1.0.0', + }, + 'packages/workspace-b': { + version: '1.0.0', + }, + }, + }), }, }, }) @@ -5397,7 +5416,20 @@ t.test('ls --install-strategy=linked', async t => { }, }, node_modules: { - // workspace-a is declared but its symlink is missing + // workspace-a is declared but its symlink is missing. + // The hidden lockfile records the workspace target without a root + // node_modules link, so loadActual resolves the declared workspace's + // root edge as missing (the declared-but-missing case), which must + // still be reported as UNMET DEPENDENCY. + '.package-lock.json': JSON.stringify({ + lockfileVersion: 3, + requires: true, + packages: { + 'packages/workspace-a': { + version: '1.0.0', + }, + }, + }), }, }, })