From ea50db404c0ae905186fce70b3e549637abd82fb Mon Sep 17 00:00:00 2001 From: ump45nose Date: Fri, 16 Oct 2026 19:35:47 +0800 Subject: [PATCH] fix(arborist): keep an override written in two scopes on a shared node --- workspaces/arborist/lib/override-set.js | 18 ++++++++++- workspaces/arborist/test/node.js | 30 +++++++++++++++++ workspaces/arborist/test/override-set.js | 41 ++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js index ab597a7f33bd4..b5b5842b62fe6 100644 --- a/workspaces/arborist/lib/override-set.js +++ b/workspaces/arborist/lib/override-set.js @@ -202,7 +202,23 @@ class OverrideSet { } // The override sets are structurally incomparable but have compatible rules. Fall back to their nearest common ancestor so the node still has a valid override set. - return this.findCommonAncestor(first, second) + const ancestor = this.findCommonAncestor(first, second) + + // When both sets are the same rule written in two scopes (e.g. `minimatch: { brace-expansion }` at the top level and again under `tshy > ... > glob`), the ancestor alone would drop that rule's nested overrides. + // Keep the one hung directly under the ancestor: it adds only the rules both paths agree on. + if (ancestor && first.isSameRule(second)) { + for (const candidate of [first, second]) { + if (ancestor.isEqual(candidate.parent)) { + return candidate + } + } + } + + return ancestor + } + + isSameRule (other) { + return this.key === other.key && this.value === other.value && this.childrenAreEqual(other) } static findCommonAncestor (first, second) { diff --git a/workspaces/arborist/test/node.js b/workspaces/arborist/test/node.js index ec474e0b7d75e..32f4dd0c4a2ac 100644 --- a/workspaces/arborist/test/node.js +++ b/workspaces/arborist/test/node.js @@ -3240,6 +3240,36 @@ t.test('should propagate the new override set to the target node', t => { t.end() }) +t.test('a hoisted dep reached through two scopes of the same override keeps it', t => { + // Regression test for https://github.com/npm/cli/issues/10137 + const tree = new Node({ + loadOverrides: true, + path: '/root', + pkg: { + name: 'root', + version: '1.0.0', + dependencies: { tshy: '4.1.3' }, + overrides: { + minimatch: { 'brace-expansion': '^5.0.12' }, + tshy: { glob: { minimatch: { 'brace-expansion': '^5.0.12' } } }, + }, + }, + children: [ + { name: 'tshy', version: '4.1.3', pkg: { dependencies: { glob: '^13.0.0', minimatch: '^10.0.3' } } }, + { name: 'glob', version: '13.0.0', pkg: { dependencies: { minimatch: '^10.2.2' } } }, + { name: 'minimatch', version: '10.2.6', pkg: { dependencies: { 'brace-expansion': '^5.0.8' } } }, + { name: 'brace-expansion', version: '5.0.9', pkg: {} }, + ], + }) + + const minimatch = tree.children.get('minimatch') + t.equal(minimatch.edgesIn.size, 2, 'minimatch is reached from tshy and from glob') + const edge = minimatch.edgesOut.get('brace-expansion') + t.equal(edge.spec, '^5.0.12', 'the override still applies to brace-expansion') + t.notOk(edge.valid, 'brace-expansion@5.0.9 does not satisfy the override') + t.end() +}) + t.test('override conflict detection with semantic comparison', t => { t.test('non-conflicting different override sets should be valid', t => { // Regression test for issue #8688 diff --git a/workspaces/arborist/test/override-set.js b/workspaces/arborist/test/override-set.js index b892c02c7cc85..b35f59245af70 100644 --- a/workspaces/arborist/test/override-set.js +++ b/workspaces/arborist/test/override-set.js @@ -537,6 +537,47 @@ t.test('findSpecificOverrideSet returns common ancestor for compatible siblings' t.ok(reverse.isEqual(root), 'reverse order should also return the root') }) +t.test('findSpecificOverrideSet keeps the same rule written in two scopes', async (t) => { + // Regression test for https://github.com/npm/cli/issues/10137 + // A hoisted minimatch is reached through the top-level `minimatch` rule and through `tshy > glob > minimatch`. + // Both say the same thing about brace-expansion, so the node must keep that rule instead of falling back to the root, which has none. + const root = new OverrideSet({ + overrides: { + minimatch: { 'brace-expansion': '^5.0.12' }, + tshy: { glob: { minimatch: { 'brace-expansion': '^5.0.12' } } }, + }, + }) + + const topLevel = root.children.get('minimatch') + const scoped = root.children.get('tshy').children.get('glob').children.get('minimatch') + + t.equal(OverrideSet.findSpecificOverrideSet(topLevel, scoped), topLevel, 'keeps the top-level rule') + t.equal(OverrideSet.findSpecificOverrideSet(scoped, topLevel), topLevel, 'keeps the top-level rule in either order') + + const edge = { name: 'brace-expansion', rawSpec: '^5.0.0', spec: '^5.0.0' } + t.equal(OverrideSet.findSpecificOverrideSet(scoped, topLevel).getEdgeRule(edge).value, '^5.0.12', 'brace-expansion is still overridden') +}) + +t.test('findSpecificOverrideSet falls back to the ancestor when two scopes differ', async (t) => { + const root = new OverrideSet({ + overrides: { + minimatch: { 'brace-expansion': '^5.0.12' }, + tshy: { glob: { minimatch: { 'brace-expansion': '^5.0.12', other: '1.0.0' } } }, + a: { minimatch: { 'brace-expansion': '^5.0.12' } }, + b: { minimatch: { 'brace-expansion': '^5.0.12' } }, + }, + }) + + const topLevel = root.children.get('minimatch') + const scoped = root.children.get('tshy').children.get('glob').children.get('minimatch') + t.equal(OverrideSet.findSpecificOverrideSet(topLevel, scoped), root, 'different nested rules use the common ancestor') + + // the same rule in two scopes, neither of them directly under the common ancestor + const underA = root.children.get('a').children.get('minimatch') + const underB = root.children.get('b').children.get('minimatch') + t.equal(OverrideSet.findSpecificOverrideSet(underA, underB), root, 'scoped copies of a rule do not leak their scope') +}) + t.test('findCommonAncestor', async (t) => { const root = new OverrideSet({ overrides: {