Skip to content

fix: Remove external filter in PnpmShrinkwrapFile.getIntegrityForImporter - #5702

Merged
David Michon (dmichon-msft) merged 6 commits into
mainfrom
copilot/remove-external-filter
Aug 11, 2026
Merged

David Michon (dmichon-msft) merged 6 commits into
mainfrom
copilot/remove-external-filter

Conversation

Copilot AI commented Mar 16, 2026

Copy link
Copy Markdown
Contributor

Summary

In PnpmShrinkwrapFile.getIntegrityForImporter, completely remove the externalFilter that previously excluded workspace-local (link:) dependencies from integrity hashing. Instead, these dependencies are now recursively resolved to their importer keys and their integrity hashes are included in shrinkwrap-deps.json all the way down the dependency tree.

Changes

libraries/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts

  • Removed the externalFilter constant that filtered out link: dependencies
  • Added a processCollection helper inside getIntegrityForImporter that:
    • For link: deps in importer sections: resolves the target importer key using path.posix.join(importerKey, linkPath) + path.posix.normalize() (since link: paths are relative to the project folder itself, and lockfile paths are always POSIX), then calls getIntegrityForImporter recursively, merging the results
    • For non-link: deps: passes them to _addIntegrities as before
  • Updated _addIntegrities to also handle link: deps encountered in package snapshot dependency sections: pnpm resolves these relative to the workspace root, so the path after link: is used directly as an importer key with no further path manipulation, and getIntegrityForImporter is called recursively
  • Aliased integrityMap to a const resolvedIntegrityMap immediately after creation so the scope analyzer can confirm definite assignment throughout the closure
  • Removed the optional filter parameter from _addIntegrities (no longer needed)

libraries/rush-lib/src/logic/pnpm/test/PnpmShrinkwrapFile.test.ts

Added four new tests in the getIntegrityForImporter describe block, all using realistic Rush importer paths (../../project-name, link:../project-name):

  1. Includes workspace-local link: dependencies by recursing — verifies ../../project-2 and its transitive dep lodash@4.17.21 appear in ../../project-1's integrity map
  2. Produces different hashes when a workspace-local dependency changes — verifies that changing a linked workspace package's transitive dep causes a different integrity hash
  3. Scenario 1workspace project 1 → workspace project 2 → external dep 1 → external dep 2: verifies the full linear chain of workspace + external transitive deps all appear in the integrity map
  4. Scenario 2workspace project 1 → external dep 1 → (link:) workspace project 2 → external dep 2: verifies that a link: reference inside an external package's snapshot correctly traverses back into the workspace and captures the linked project's full integrity sub-tree

common/changes/@microsoft/rush/copilot-remove-external-filter_2026-03-16-21-07.json

Rush change file (patch) for @microsoft/rush.

Security Summary

CodeQL analysis found no alerts.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits March 16, 2026 21:06
… workspace deps in integrity hash tree

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>
Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>
Comment thread libraries/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts Outdated
Comment thread libraries/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts Outdated
Comment thread libraries/rush-lib/src/logic/pnpm/test/PnpmShrinkwrapFile.test.ts Outdated
Comment thread libraries/rush-lib/src/logic/pnpm/test/PnpmShrinkwrapFile.test.ts
…move .has check, realistic test paths, add scenario tests

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>
…cenario 2 test topology

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/8abf80c0-662c-484f-a61b-87c6ce4c9a17
@dmichon-msft

Copy link
Copy Markdown
Contributor

Copilot Can you push another commit (just rebase onto latest main) to reset this PR's statuses?

Copilot AI commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Copilot Can you push another commit (just rebase onto latest main) to reset this PR's statuses?

The local branch has been rebased onto the latest main (merge-base is now 4fb44509fb — "Align subspace registries with pipeline proxy (#5919)"). However, the push is failing due to a GPG signing error in the report_progress tool's rebase environment. The branch itself is clean and ready; the CI statuses should reset once the push goes through. I'll retry, but if the infrastructure issue persists you may need to force-push the branch manually.

Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>
@dmichon-msft
David Michon (dmichon-msft) merged commit 0fb78ed into main Aug 11, 2026
9 checks passed
@dmichon-msft
David Michon (dmichon-msft) deleted the copilot/remove-external-filter branch August 11, 2026 22:35
@github-project-automation github-project-automation Bot moved this from Needs triage to Closed in Bug Triage Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

3 participants