diff --git a/common/changes/@microsoft/rush/copilot-remove-external-filter_2026-03-16-21-07.json b/common/changes/@microsoft/rush/copilot-remove-external-filter_2026-03-16-21-07.json new file mode 100644 index 00000000000..e91bc81bdc6 --- /dev/null +++ b/common/changes/@microsoft/rush/copilot-remove-external-filter_2026-03-16-21-07.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "comment": "In PnpmShrinkwrapFile getIntegrityForImporter, remove the external filter and include workspace-local link: dependencies by recursing into their importer entries, so shrinkwrap-deps.json hashes cover the full dependency tree.", + "type": "patch", + "packageName": "@microsoft/rush" + } + ], + "packageName": "@microsoft/rush" +} diff --git a/libraries/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts b/libraries/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts index 97068b557ec..d07b86919dc 100644 --- a/libraries/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts +++ b/libraries/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts @@ -911,36 +911,57 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile { if (!integrityMap) { const importer: IPnpmShrinkwrapImporterYaml | undefined = this.getImporter(importerKey); if (importer) { - integrityMap = new Map(); - this._integrities.set(importerKey, integrityMap); + const resolvedIntegrityMap: Map = new Map(); + integrityMap = resolvedIntegrityMap; + this._integrities.set(importerKey, resolvedIntegrityMap); const sha256Digest: string = crypto .createHash('sha256') .update(JSON.stringify(importer)) .digest('base64'); const selfIntegrity: string = `${importerKey}:${sha256Digest}:`; - integrityMap.set(importerKey, selfIntegrity); + resolvedIntegrityMap.set(importerKey, selfIntegrity); const { dependencies, devDependencies, optionalDependencies } = importer; - const externalFilter: (name: string, version: IPnpmVersionSpecifier) => boolean = ( - name: string, - versionSpecifier: IPnpmVersionSpecifier - ): boolean => { - const version: string = normalizePnpmVersionSpecifier(versionSpecifier); - return !version.includes('link:'); + const processCollection = ( + collection: Record, + optional: boolean + ): void => { + const externalDeps: Record = {}; + for (const [name, versionSpecifier] of Object.entries(collection)) { + const version: string = normalizePnpmVersionSpecifier(versionSpecifier); + if (version.startsWith('link:')) { + // This is a workspace-local dependency; resolve it to an importer key and recurse. + // The link: path is relative to the project folder (which is the importer key itself), + // so we join the importer key with the link path (not dirname). + // Lockfile paths are always POSIX, so we use path.posix helpers. + const linkPath: string = version.slice('link:'.length); + const targetKey: string = path.posix.normalize(path.posix.join(importerKey, linkPath)); + const linkedIntegrities: Map | undefined = + this.getIntegrityForImporter(targetKey); + if (linkedIntegrities) { + for (const [dep, integrity] of linkedIntegrities) { + resolvedIntegrityMap.set(dep, integrity); + } + } + } else { + externalDeps[name] = versionSpecifier; + } + } + this._addIntegrities(resolvedIntegrityMap, externalDeps, optional); }; if (dependencies) { - this._addIntegrities(integrityMap, dependencies, false, externalFilter); + processCollection(dependencies, false); } if (devDependencies) { - this._addIntegrities(integrityMap, devDependencies, false, externalFilter); + processCollection(devDependencies, false); } if (optionalDependencies) { - this._addIntegrities(integrityMap, optionalDependencies, true, externalFilter); + processCollection(optionalDependencies, true); } } } @@ -1269,14 +1290,9 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile { private _addIntegrities( integrityMap: Map, collection: Record, - optional: boolean, - filter?: (name: string, version: IPnpmVersionSpecifier) => boolean + optional: boolean ): void { for (const [name, version] of Object.entries(collection)) { - if (filter && !filter(name, version)) { - continue; - } - const packageId: string = this._getPackageId(name, version); if (integrityMap.has(packageId)) { // The entry could already have been added as a nested dependency diff --git a/libraries/rush-lib/src/logic/pnpm/test/PnpmShrinkwrapFile.test.ts b/libraries/rush-lib/src/logic/pnpm/test/PnpmShrinkwrapFile.test.ts index b0a090ed9fc..a8271ef93e6 100644 --- a/libraries/rush-lib/src/logic/pnpm/test/PnpmShrinkwrapFile.test.ts +++ b/libraries/rush-lib/src/logic/pnpm/test/PnpmShrinkwrapFile.test.ts @@ -314,6 +314,234 @@ snapshots: // because the sub-dependency (bar) resolved to different versions expect(fooIntegrity1).not.toEqual(fooIntegrity2); }); + + it('includes workspace-local link: dependencies by recursing into their importer entries', () => { + // This test verifies that link: (workspace-local) dependencies are no longer filtered out. + // The shrinkwrap-deps.json for an importer should include hashes from its workspace + // dependencies' importer sections, all the way down the tree. + // + // In a real Rush repo (no subspaces), importer keys start with '../../' and + // link: paths start with '../'. + // + // Topology: project-1 -> (link:) project-2 -> lodash@4.17.21 + + const shrinkwrapContent: string = ` +lockfileVersion: '9.0' +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false +importers: + .: + {} + ../../project-1: + dependencies: + project-2: + specifier: workspace:* + version: link:../project-2 + ../../project-2: + dependencies: + lodash: + specifier: ^4.17.0 + version: 4.17.21 +packages: + lodash@4.17.21: + resolution: + integrity: sha512-lodash== +snapshots: + lodash@4.17.21: {} +`; + + const shrinkwrapFile = PnpmShrinkwrapFile.loadFromString(shrinkwrapContent, { + subspaceHasNoProjects: false + }); + + PnpmShrinkwrapFile.clearCache(); + + const proj1IntegrityMap = shrinkwrapFile.getIntegrityForImporter('../../project-1'); + + expect(proj1IntegrityMap).toBeDefined(); + + // project-1's integrity map should include project-2's importer entry + expect(proj1IntegrityMap!.has('../../project-2')).toBe(true); + + // It should also include the transitive external dependency of project-2 + expect(proj1IntegrityMap!.has('lodash@4.17.21')).toBe(true); + + // The integrity map for project-2 itself should also be populated + const proj2IntegrityMap = shrinkwrapFile.getIntegrityForImporter('../../project-2'); + expect(proj2IntegrityMap).toBeDefined(); + expect(proj2IntegrityMap!.has('../../project-2')).toBe(true); + expect(proj2IntegrityMap!.has('lodash@4.17.21')).toBe(true); + }); + + it('produces different hashes when a workspace-local dependency changes', () => { + // This test verifies that changing the dependencies of a workspace-local package + // causes the dependent importer's integrity to differ. + // + // Topology: project-1 -> (link:) project-2 -> lodash@4.17.x (version differs between cases) + + const buildContent = (lodashVersion: string): string => ` +lockfileVersion: '9.0' +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false +importers: + .: + {} + ../../project-1: + dependencies: + project-2: + specifier: workspace:* + version: link:../project-2 + ../../project-2: + dependencies: + lodash: + specifier: ^4.17.0 + version: ${lodashVersion} +packages: + lodash@${lodashVersion}: + resolution: + integrity: sha512-lodash-${lodashVersion}== +snapshots: + lodash@${lodashVersion}: {} +`; + + const shrinkwrapFile1 = PnpmShrinkwrapFile.loadFromString(buildContent('4.17.21'), { + subspaceHasNoProjects: false + }); + const shrinkwrapFile2 = PnpmShrinkwrapFile.loadFromString(buildContent('4.17.20'), { + subspaceHasNoProjects: false + }); + + PnpmShrinkwrapFile.clearCache(); + + const proj1IntegrityMap1 = shrinkwrapFile1.getIntegrityForImporter('../../project-1'); + const proj1IntegrityMap2 = shrinkwrapFile2.getIntegrityForImporter('../../project-1'); + + expect(proj1IntegrityMap1).toBeDefined(); + expect(proj1IntegrityMap2).toBeDefined(); + + // The self-hash of project-1 does NOT change because the root importer object itself is + // identical in both cases (it still references link:../project-2). However, project-2's + // integrity hash should differ because its lodash dependency resolved to a different version. + const proj2Integrity1 = proj1IntegrityMap1!.get('../../project-2'); + const proj2Integrity2 = proj1IntegrityMap2!.get('../../project-2'); + + expect(proj2Integrity1).toBeDefined(); + expect(proj2Integrity2).toBeDefined(); + expect(proj2Integrity1).not.toEqual(proj2Integrity2); + }); + + it('scenario 1: workspace project 1 -> workspace project 2 -> external dep 1 -> external dep 2', () => { + // Tests the full chain: project-1 links to project-2, project-2 depends on ext-a, + // and ext-a transitively depends on ext-b. All four should appear in project-1's integrity map. + + const shrinkwrapContent: string = ` +lockfileVersion: '9.0' +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false +importers: + .: + {} + ../../project-1: + dependencies: + project-2: + specifier: workspace:* + version: link:../project-2 + ../../project-2: + dependencies: + ext-a: + specifier: ^1.0.0 + version: 1.0.0 +packages: + ext-a@1.0.0: + resolution: + integrity: sha512-ext-a== + ext-b@1.0.0: + resolution: + integrity: sha512-ext-b== +snapshots: + ext-a@1.0.0: + dependencies: + ext-b: 1.0.0 + ext-b@1.0.0: {} +`; + + const shrinkwrapFile = PnpmShrinkwrapFile.loadFromString(shrinkwrapContent, { + subspaceHasNoProjects: false + }); + + PnpmShrinkwrapFile.clearCache(); + + const proj1IntegrityMap = shrinkwrapFile.getIntegrityForImporter('../../project-1'); + + expect(proj1IntegrityMap).toBeDefined(); + // project-2's importer entry + expect(proj1IntegrityMap!.has('../../project-2')).toBe(true); + // ext-a (direct dep of project-2) + expect(proj1IntegrityMap!.has('ext-a@1.0.0')).toBe(true); + // ext-b (transitive dep through ext-a) + expect(proj1IntegrityMap!.has('ext-b@1.0.0')).toBe(true); + }); + + it('scenario 2: workspace project 1 -> external dep 1 and workspace project 2 -> external dep 2', () => { + // Tests that when project-1 has both an external direct dependency AND a workspace link + // dependency, both sub-trees are fully captured in the integrity map. + // + // project-1 depends on ext-x (direct external) and project-2 (workspace link). + // project-2 depends on ext-y. + // All four should appear in project-1's integrity map. + + const shrinkwrapContent: string = ` +lockfileVersion: '9.0' +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false +importers: + .: + {} + ../../project-1: + dependencies: + ext-x: + specifier: ^2.0.0 + version: 2.0.0 + project-2: + specifier: workspace:* + version: link:../project-2 + ../../project-2: + dependencies: + ext-y: + specifier: ^3.0.0 + version: 3.0.0 +packages: + ext-x@2.0.0: + resolution: + integrity: sha512-ext-x== + ext-y@3.0.0: + resolution: + integrity: sha512-ext-y== +snapshots: + ext-x@2.0.0: {} + ext-y@3.0.0: {} +`; + + const shrinkwrapFile = PnpmShrinkwrapFile.loadFromString(shrinkwrapContent, { + subspaceHasNoProjects: false + }); + + PnpmShrinkwrapFile.clearCache(); + + const proj1IntegrityMap = shrinkwrapFile.getIntegrityForImporter('../../project-1'); + + expect(proj1IntegrityMap).toBeDefined(); + // ext-x (direct external dep of project-1) + expect(proj1IntegrityMap!.has('ext-x@2.0.0')).toBe(true); + // project-2 (workspace link dep of project-1) + expect(proj1IntegrityMap!.has('../../project-2')).toBe(true); + // ext-y (external dep of project-2, reached via link) + expect(proj1IntegrityMap!.has('ext-y@3.0.0')).toBe(true); + }); }); describe('Check is workspace project modified', () => {