Include Node version in CI setup cache key#120
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds the resolved Node version into the composite setup action's cache key so the node_modules cache is invalidated when the Node version changes, preventing potential native-binding ABI mismatches on cache restore.
Changes:
- Adds a
node-infostep that capturesnode -voutput. - Includes the Node version in both the primary cache key and the
restore-keysprefix.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
greghuels
approved these changes
May 26, 2026
Base automatically changed from
aarsilv/ffesupport-728/address-vulnerabilities
to
main
May 26, 2026 11:59
The composite setup action caches `**/node_modules` keyed on OS + `yarn.lock` hash only. If a `node_modules` cache built under a different Node version were restored, packages with native bindings could load mismatched ABI binaries — and because the cache-hit path skips the install step, deps wouldn't get rebuilt. Capture the resolved Node version (`node -v`) into a step output, then include it in both the primary cache key and the `restore-keys` prefix. That way, bumping Node automatically busts the cache instead of relying on lockfile churn to do it incidentally. Stays with `actions/setup-node@v3` and `actions/cache@v3` — this is the minimal-change variant. Caching Yarn's download cache instead of `node_modules` would be the more robust pattern, but that's a bigger restructure for a separate ticket if we want it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
aarsilv
force-pushed
the
aarsilv/ffesupport-731/ci-cache-key-node-version
branch
from
3b91940 to
960844e
Compare
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.
FFESUPPORT-731 · stacked on #118
Summary
The composite setup action caches
**/node_moduleskeyed on OS +yarn.lockhash only. If anode_modulescache built under a different Node version were restored, packages with native bindings could load mismatched ABI binaries — and because the cache-hit path skips the install step, deps wouldn't get rebuilt.Captures the resolved Node version (
node -v) into a step output, then includes it in both the primary cache key and therestore-keysprefix. Bumping Node now busts the cache automatically instead of relying on lockfile churn.Pre-existing best-practice gap, not a regression from #118 — flagged by Copilot as a low-confidence observation during the #118 review.
Notes
Minimal-change variant. Caching Yarn's download cache instead of
node_moduleswould be the more robust pattern, but that's a bigger restructure best handled as a separate ticket if we want it.Test plan
node -voutput is captured in the key).🤖 Generated with Claude Code