chore(ci): add composite setup-node action with npm cache#6177
Merged
macko911 merged 8 commits intoJun 3, 2026
Merged
Conversation
7cb53e5 to
4f87990
Compare
Adds .github/actions/setup-node wrapping actions/setup-node@v4 with cache: 'npm' and cache-dependency-path: 'package-lock.json' always set, and migrates every workflow that checks out the repo to use it. New workflows can no longer forget npm caching. In managed-release.yaml, reorder checkout before setup-node in both jobs so the local composite is on disk and package-lock.json is available for cache key computation. The verify job in cli-verification.yaml keeps the raw setup-node@v4 - it has no checkout step and only runs npm install -g, so the composite would neither resolve nor benefit from caching.
Most workflow runs on direct pushes to master originate from the merge-queue bot, which trips the existing should-run short-circuit and skips npm ci. As a result, no node-cache ever lands on refs/heads/master, and every feature branch pays a cold cache miss on its first run. This workflow runs unconditionally on push to master and does just enough work to populate the npm cache (~30-60s), so feature branches restore it on first run.
Add a check-lockfile input (default true) that runs npm install --package-lock-only --ignore-scripts followed by git diff --exit-code package-lock.json. Fails the job if package-lock.json is out of sync with package.json. Disabled for publish.yaml (pins npm to 11.5.1 after the composite runs, so the check would execute with the wrong npm version) and for managed-release.yaml (workspace-scoped install against master commits, already validated at merge time).
4f87990 to
c9b9af5
Compare
The check ran `npm install --package-lock-only` and diffed the result, but different npm versions (10.9.x on CI vs 11.x locally) produce slightly different lockfile output (e.g. `"peer": true` on optional platform packages), causing false positives on every CI job. The check is redundant anyway — all workflows run `npm ci` after setup-node, which already fails on a genuine lockfile/package.json mismatch.
Upgrade npm to 11.12.1 in the setup-node action so CI and local environments use the same version, eliminating lockfile format divergence caused by npm metadata differences across versions. Restore the lockfile consistency check and remove the now-redundant npm upgrade step from publish.yaml.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f4733713c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
rossmcewan
approved these changes
Jun 3, 2026
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
3 participants
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.
Problem
npm caching was inconsistent across CI workflows — some used
cache: 'npm', others didn't. New workflows could easily forget it. Also, master pushes from the merge-queue bot skipnpm civia theshould-runcheck, so no npm cache was ever saved onrefs/heads/master, meaning every feature branch paid a cold cache miss on first run.This also lays the ground for enabling
pnpmacross the monorepo.Solution
Add
.github/actions/setup-node, a composite action wrappingactions/setup-node@v4withcache: 'npm'always set. Migrate all workflows to use it.Add
cache-warmup.yamlthat runs on every master push, doing just enough (checkout+setup-node+npm ci) to land an npm cache onrefs/heads/masterso feature branches can restore it.Only
cli-verification.yamlandmanaged-release.yamlgenuinely gain new caching — the rest were already usingcache: 'npm'and are just migrated to the composite action.Fixes NAN-5527
Testing
Cache Warmupworkflow runs on the master push and saves a cache entry onrefs/heads/master