fix(release): honour engines.npm — setup-node ships the Node-bundled npm - #525
Merged
Conversation
rubenvdlinde
added a commit
that referenced
this pull request
Aug 21, 2026
…esolved lock (#534) The first real run of this workflow (docudesk PR #710) produced an 84-add / 16-delete package-lock diff for what is a 3-line version bump. MEASURED, both directions, on that exact pair of lockfiles: npm 10 (what setup-node bundles with Node 22): packages added 0, removed 0 -- but 81 `@esbuild/*` platform binaries newly marked `"dev": true`, while `esbuild` itself stayed unflagged. npm 11 (what every sampled app declares, `engines.npm: ^11.0.0`): packages added 0, removed 0, dev-flag changes 0, diff exactly +3/-3. So the tree never changed; npm 10 simply classifies it differently. This is the same trap release.yml already documented and fixed in #525 -- setup-node installs the npm BUNDLED WITH NODE, not the one the repo asks for. release.yml reads ./package.json once because it runs inside one repo; this workflow walks 21, so the range is re-read and re-honoured inside the loop, with the global install cached per distinct range (one install for the fleet as it stands). Adds a second, independent check so a wrong toolchain cannot quietly ship again: a lock-only bump of ONE package must not add packages, remove packages, or flip an unrelated package's `dev` flag. If it does, the tree was re-resolved rather than nudged -- the npm side is reverted and annotated rather than opened as a PR nobody can review. Verified to FAIL on the known-bad lock (81 flips) and to PASS on the correct one (0 flips), so it is a check that demonstrably can fail. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
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.
The failure
launchpad and portaliq releases fail on
npm ci:The lock is not out of sync. The identical commit installs cleanly under
npm 11 —
npm ci --dry-run, exit 0, "added 1416 packages".The cause
setup-nodeinstalls the npm bundled with the Node it selects. Node 22bundles npm 10. Both apps declare
engines.npm: ^11.0.0and ship an.npmrcusing
min-release-age— a supply-chain cooldown (“do not install anythingpublished less than N days ago”) that exists only on npm 11+.
On npm 10 that setting is silently ignored, resolution differs, and
npm cirejects a perfectly good lock.
The dangerous part is the error text: it names the lock, so the obvious fix is
to regenerate it. That would have re-broken the cooldown policy — the lock
is deliberately "behind" because packages inside the cooldown window are
excluded — while papering over a toolchain mismatch that would resurface on
the next dependency bump.
The fix
After
setup-node, readengines.npmfrompackage.jsonand install thatnpm. A repo declaring no
engines.npmkeeps the bundled npm and isunaffected, so this is a no-op for every app that is currently green.
Blast radius
Fleet-wide workflow, but the step is conditional on a field the repo itself
declares. Of the 18 core apps, only launchpad and portaliq declare
engines.npmtoday — the other 16 hit theexit 0path with their bundlednpm unchanged.
Verified the edited file still parses as YAML and the
releasejob is intact.