Repository navigation
Conversation
Use the same bin path derivation for regular and linked installs so both consume the defensive normalization from npm/bin-links#187. Add traversal and optional-cleanup regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a direct isolated-tree assertion so the regression fails when linked bin cleanup paths bypass bin-links normalization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3e53db77-0fe4-4320-8ada-f2962ddd303c
martinrrm
force-pushed
the
fix/arborist-bin-paths
branch
from
October 28, 2026 01:42
b6edc9a to
aa3bb20
Compare
Look up the isolated linked dependency using path.join so the regression runs on Windows as well as POSIX. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3e53db77-0fe4-4320-8ada-f2962ddd303c
martinrrm
marked this pull request as ready for review
October 28, 2026 01:53
jeffkl
previously approved these changes
Oct 28, 2026
nkolev92
previously approved these changes
Oct 28, 2026
Contributor
Wouldn't a simple backport via the label work for this one? |
There was a problem hiding this comment.
🟡 Changes recommended
The linked-mode test can pass without exercising optional-failure cleanup because its invalid digest does not force extraction to fail; it needs an independent tarball failure and an assertion that the optional dependency is absent.
1 open issue
What changed in this PR
This PR routes Arborist’s linked-install bin cleanup through bin-links#getPaths(), sharing the defensive normalization already used by regular installs.
Changes:
- Derive linked bin cleanup paths from consumer-facing installation paths.
- Add coverage for unsafe and invalid bin metadata without mutating caller data.
- Add optional-extraction-failure sentinel tests and a direct linked-tree normalization regression.
| File | Description |
|---|---|
workspaces/arborist/test/node.js |
Tests bin normalization and metadata preservation. |
workspaces/arborist/test/arborist/reify.js |
Adds cleanup-safety and linked-tree regression tests. |
workspaces/arborist/lib/arborist/isolated-reifier.js |
Replaces manual bin path construction with getPaths(). |
Review effort: Balanced
Mock a missing tarball independently of integrity so both hoisted and linked installs exercise optional-dependency cleanup. Assert that the failed dependency is absent before checking the external sentinel. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: eac8b072-e785-47d5-ba34-9a7b20684ae0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Route linked-install bin cleanup paths through
bin-links#getPaths()instead of manually joining lockfile-provided bin keys. RegularNode.binPathsalready uses this API, so both strategies can share the defensive bin normalization introduced by npm/bin-links#187.This is the Arborist integration for CLI-CROW-GEN-002. It does not introduce an Arborist-owned normalizer or modify the bundled dependency.
Changes
bin-links#getPaths()using the dependency's consumer-facing installation path, not its store path.Node.binPathsregression coverage for traversal-bearing, absolute, Windows-style and invalid bin metadata, including preservation of the caller's metadata.Dependency status
npm CLI now bundles released bin-links@7.0.1 through #10152. This branch is rebased onto latest and uses that released normalization. A corresponding release/v11 backport is still needed.
Validation