Skip to content

fix(arborist): recover prepare scripts for git deps from disk - #10186

Open
ump45nose wants to merge 2 commits into
npm:latestfrom
ump45nose:fix/10182-install-scripts-git-prepare
Open

ump45nose wants to merge 2 commits into
npm:latestfrom
ump45nose:fix/10182-install-scripts-git-prepare

Conversation

@ump45nose

Copy link
Copy Markdown
Contributor

Fixes #10182

What

npm install-scripts ls / prune missed prepare on git dependencies when the tree was loaded from node_modules/.package-lock.json. That hidden lockfile omits script bodies and often omits hasInstallScript for prepare-only git deps, so prune deleted a still-needed allowScripts entry and ls hid the package.

Why this approach

loadActual({ forceActual: true }) does recover scripts, but it also drops resolved for those nodes, which breaks git policy matching (github:owner/repo#sha). Instead, extend the existing read-only package.json disk hydration in getInstallScripts so non-registry nodes are checked even when hasInstallScript is absent. Registry behavior is unchanged.

Tests

  • arborist unit: prepare-only git dep hydrates prepare from disk without hasInstallScript
  • install-scripts prune keeps a git prepare allowScripts entry under a hidden lockfile
  • install-scripts ls lists that git prepare dep under a hidden lockfile
tap workspaces/arborist/test/install-scripts.js --grep='prepare-only git'
tap test/lib/commands/install-scripts.js --grep='git prepare'

AI disclosure

This change was written with AI assistance. I reviewed every line, ran the tests above, and chose disk hydration over forceActual after verifying that forceActual clears resolved and breaks git allowScripts matching.

Hidden lockfiles omit script bodies and often omit hasInstallScript for
prepare-only git dependencies, so install-scripts ls/prune treated them
as having no scripts (npm#10182). Extend the existing package.json
disk read to cover non-registry nodes even when hasInstallScript is
absent, without using forceActual (which drops resolved and breaks git
policy matching).
@ump45nose
ump45nose requested a review from a team as a code owner October 28, 2026 23:30
Gate disk script recovery on the resolved URL instead of
hasNonRegistryShape/isRegistryDependency, and emit the
hasInstallScript sentinel only when the lockfile asserted scripts.
Linked-store nodes expose isRegistryDependency as boolean false by
default, which previously made empty store roots look like pending
install-script packages.
@ump45nose

Copy link
Copy Markdown
Contributor Author

Pushed a follow-up for the CI failures on this branch:

  • Lint: removed the extra blank line in test/lib/commands/install-scripts.js.
  • approve-scripts linked-store case: disk script recovery was gated on hasNonRegistryShape / isRegistryDependency. Linked-store Node subclasses expose isRegistryDependency as a boolean false by default, so empty store-root inventory entries entered recovery and hit the hasInstallScript sentinel ((install scripts present)), showing up as unreviewed under a registry pin. Recovery now keys off the resolved URL only, and the sentinel is emitted only when hasInstallScript === true.

Local: arborist install-scripts unit tests (incl. a new store-root regression) and CLI install-scripts / approve-scripts suites are green.

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] install-scripts prune and ls miss the prepare script of a git dependency

1 participant