Skip to content

fix: skip npm self-upgrade when version unspecified#90

Open
fibble wants to merge 1 commit intomainfrom
fix/install-deps-skip-npm-upgrade
Open

fix: skip npm self-upgrade when version unspecified#90
fibble wants to merge 1 commit intomainfrom
fix/install-deps-skip-npm-upgrade

Conversation

@fibble
Copy link
Copy Markdown
Contributor

@fibble fibble commented Apr 13, 2026

Summary

The shared install-deps composite action runs npm i -g npm@latest unconditionally on every job. This currently fails on Node 22 with:

npm error Cannot find module 'promise-retry'
npm error Require stack:
npm error - .../node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/rebuild.js

Tracked upstream as npm/cli#9151. Open with no fix or ETA.

Impact: every DVSA Node CI pipeline that uses nodejs-lint.yaml / nodejs-test.yaml / nodejs-build.yaml without setting an explicit npm-version has been red since ~2026-03-04. v5.0.10 is byte-identical to v5.0.4 for install-deps/action.yaml, so consumers cannot escape by bumping their caller tag.

Approach

  • Change the default of npm-version from 'latest' to ''
  • Gate the npm i -g step with if: inputs.npm-version != ''
  • Empty string means "use the npm bundled with the selected node version" — strictly safer than self-upgrading to whatever is on the registry
  • Explicit version pins (e.g. '10.9.7') are unchanged
  • Anyone explicitly passing 'latest' still gets the upgrade attempt; that's an explicit ask we honor

The three reusable workflows (nodejs-{lint,test,build}.yaml) also get their internal install-deps@vX.Y.Z pin bumped to v5.0.11 so that consumers who bump their caller tag actually pick up the fix. Forward-referencing v5.0.11 is fine because action refs resolve at run time, not commit time — once the tag is cut on the merge commit the references resolve consistently.

Behavior matrix

Consumer setting Before After
No npm-version set 💥 latest is broken ✅ skip upgrade, use node-bundled npm
npm-version: '10.9.7' ✅ works ✅ unchanged
npm-version: 'latest' (explicit) 💥 broken 💥 still broken (explicit ask)
npm-version: '' would fail (npm@) ✅ skipped

The only behavior change is for consumers relying on the old latest default — and they're currently red anyway, so "change" = "unbroken".

Release / rollout

After merge, this commit needs to be tagged v5.0.11 for the forward refs in the workflows to resolve. Consumers can then bump their caller (e.g. nodejs-lint.yaml@v5.0.10@v5.0.11) and stop hitting the regression.

Test plan

  • CI on this PR passes
  • After merge: tag v5.0.11 on the merge commit
  • Re-run CI on dvsa/inr-erru-middleware PR #293 after bumping its caller tag to @v5.0.11 — confirm lint and test jobs go green

The shared install-deps action runs `npm i -g npm@latest` unconditionally,
which currently fails on Node 22 with `Cannot find module 'promise-retry'`
(npm/cli#9151). This breaks every consumer that uses the default
`npm-version` value, and v5.0.10 is byte-identical to v5.0.4 so consumers
can't bump out of it.

Switch the default to empty string and gate the upgrade step on
`inputs.npm-version != ''`. Empty means "use the npm bundled with the
selected node version". Explicit pins (e.g. '10.9.7') continue to work
unchanged. Consumers that explicitly set 'latest' still get the broken
behavior — they asked for it.

Also bumps the internal install-deps ref in nodejs-{lint,test,build}.yaml
from v5.0.4/v4.1.1 to v5.0.11 so the new default actually reaches
consumers when they bump their caller tag.
@sdh100shaun sdh100shaun requested review from Copilot and sdh100shaun and removed request for Copilot April 14, 2026 07:04
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.

1 participant