Skip to content

fix(arborist): keep an override written in two scopes on a shared node - #10138

Open
ump45nose wants to merge 1 commit into
npm:latestfrom
ump45nose:fix/10137-override-same-rule-two-scopes
Open

ump45nose wants to merge 1 commit into
npm:latestfrom
ump45nose:fix/10137-override-same-rule-two-scopes

Conversation

@ump45nose

Copy link
Copy Markdown

When a hoisted package is reached through two override scopes that say the same thing, findSpecificOverrideSet falls back to the scopes' common ancestor (added in #9110). The ancestor doesn't carry the nested rule, so the package's own dependencies are no longer overridden.

In the reproduction from #10137, node_modules/minimatch is reached from tshy (edge override set root > minimatch) and from glob (root > tshy > sync-content > glob > minimatch). Both carry brace-expansion: ^5.0.12, but the node ends up with the root set, so its brace-expansion edge keeps the plain ^5.0.8 spec, brace-expansion@5.0.9 counts as valid, npm ci accepts the stale lockfile and npm install doesn't fix it.

This change keeps the common-ancestor fallback, with one exception: when both sets are the same rule (same key, value and nested rules) and one of them sits directly under the common ancestor, it uses that one. That set adds only the rules both paths agree on to the ancestor's ruleset, so nothing from one scope leaks into the other. Scoped copies that are both deeper (for example a > minimatch and b > minimatch) still fall back to the ancestor, as before.

Testing

  • New test/override-set.js cases: the same rule in two scopes keeps the top-level one in either order, and its brace-expansion rule still applies. A rule whose nested overrides differ, and two scoped copies, still return the common ancestor. Without the fix, the first case fails (it returns the root and getEdgeRule gives no ^5.0.12).
  • New test/node.js case that builds the tree from the issue: minimatch is reached from tshy and from glob, and its brace-expansion edge spec is ^5.0.12 and invalid against 5.0.9. Without the fix this fails with edge.spec ^5.0.8.
  • npx tap in workspaces/arborist (full suite, Node 24.22.0): all passing, 100% coverage. npx eslint on the changed files is clean.
  • Ran the reproduction script from the issue against this branch (node bin/npm-cli.js): npm ci now fails with Invalid: lock file's brace-expansion@5.0.9 does not satisfy brace-expansion@5.0.12, and npm install updates the tree to brace-expansion@5.0.12. On latest (12.2.1) without the change, the same script let npm ci install 5.0.9.
  • I did not run the root npm test for the whole CLI.

AI assistance: I worked through this change with an AI coding assistant. I reviewed the code, the tests and this description, and ran the commands listed above.

References

Fixes #10137

@ump45nose
ump45nose requested a review from a team as a code owner October 16, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] npm ci accepts a lockfile that violates overrides when a hoisted package is reached through multiple override scopes

1 participant