Ci: cache npm downloads with a lockfile-prefix fallback - #669
Open
AmaadMartin wants to merge 2 commits into
Open
Ci: cache npm downloads with a lockfile-prefix fallback#669AmaadMartin wants to merge 2 commits into
AmaadMartin wants to merge 2 commits into
Conversation
Both CI workflows re-download the whole dependency closure on every run. setup-node's built-in npm cache is inert here (the root package.json declares neither packageManager nor devEngines.packageManager) and restores on an exact lockfile hash only. Add an actions/cache step keyed on the root lockfile hash with a restore-keys prefix fallback, so a lockfile bump restores the previous cache and npm install fetches only the delta. package-manager-cache: false keeps npm caching single-owned if a packageManager field is added later.
AmaadMartin
force-pushed
the
feat/ci-npm-cache-restore-keys
branch
from
August 5, 2026 09:01
1bfff8f to
d2cae04
Compare
package-manager-cache: false is a no-op: setup-node only auto-caches when the cache input is set or when package.json declares packageManager, and neither holds here. The cross-language job runs on macOS only, so its cache directory is ~/.npm and the carriage-return guard could never fire. Use the path directly and delete the resolver step. validation.yaml keeps the resolver because its matrix includes windows-latest.
This was referenced Aug 8, 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
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.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
Link to an existing issue (if applicable):
N/A
Or, if no issue exists, describe the change:
Problem: CI does no npm caching today, so every job re-downloads the whole dependency closure on all three OS legs.
setup-node's built-in cache is inert here, because it only auto-enables when the rootpackage.jsondeclarespackageManagerordevEngines.packageManager, and this manifest declares neither. That cache is also exact-match only for npm: inactions/setup-node@v6src/cache-restore.ts, only the yarn-berry branch passes a fallback prefix tocache.restoreCache.Solution: Add an
actions/cache@v6step to both npm-installing workflows, keyed on the root lockfile hash with arestore-keysprefix fallback. A lockfile bump now restores the previous cache, sonpm installdownloads only the delta instead of all 1,174 lockfile entries. Each workflow gets its own key lane, because both run a macOS job against the same lockfile with different cache contents and a shared key would make them race.validation.yamlresolves the cache directory withnpm config get cache, because its matrix includeswindows-latest, where the directory is not~/.npm; the cross-language job is macOS-only and uses~/.npmdirectly.Collision check: five open PRs on this fork touch the same two files (#428, #416, #406, #306, #296). All five use
cache: 'npm'onsetup-node, which is the exact-match form. None usesrestore-keys, so none supersedes this change. I branched frommainrather than stacking, because stacking on any of them would give the npm cache two owners, and three of them also pin the Node version, which is a separate concern.Testing Plan
Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
This change adds zero lines of TypeScript, so there is no new function or branch to cover. I did not add a test that parses the YAML and asserts on the string literals I just wrote, because it would re-assert the diff and carry no regression signal.
Unit Tests:
[ ] I have added or updated unit tests for my change.
[x] All unit tests pass locally.
Static validation:
Both pass.
actionlintv1.7.12 reports no findings.Every fact in this description was checked against a source of truth:
actions/cache@v6exists (tagsv6,v6.0.0,v6.1.0) and declarespath,keyandrestore-keys;src/cache-restore.tsatsetup-node@v6passesrestoreKeyson the yarn-berry branch only;src/cache-utils.tsresolves the npm cache path withnpm config get cache.Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.
The workflows are the test, so I ran them on this branch and read the logs with
gh run view --log.Run 1 (cold). Every leg logged
Cache not found for input keys: npm-<lane>-<OS>-<hash>, npm-<lane>-<OS>-and every leg saved in the post step. All four jobs were green. The Windows leg resolvedpath: C:\npm\cachewith no trailing character, which is thetr -d '\r'assertion.Run 2 (temporary
concurrently9.2.1 -> 9.2.3 lockfile bump, since dropped from the branch). The lockfile hash changed, the primary key missed, and the prefix key hit on all four jobs:That is the acceptance criterion, and it passes. The Windows leg hashes the lockfile to a different digest than Linux and macOS because of checkout line-ending conversion; the keys are
runner.os-scoped, so the two digests never cross-miss.Run 3, after I dropped the bump commit. All four jobs green, every leg an exact-key hit:
Cache restored from key: npm-<lane>-<OS>-<hash>.Run 4 is the current head. A complexity review asked me to delete two pieces of dead config, and I did:
package-manager-cache: falsein both files, and the cache-directory resolver in the macOS-only cross-language job. That took the diff from 36 added lines to 22. All four jobs are green. The threevalidationlegs restored on the exact key, and the Windows leg still resolvedpath: C:\npm\cache.Switching the cross-language
pathfrom the resolved/Users/runner/.npmto~/.npmchanges the literal thatactions/cachehashes into the entry version, so that job took one more cold run and re-saved. A re-run then loggedCache restored from key: npm-cross-language-macOS-413b02d6...againstpath: ~/.npm, which confirms the shorter form restores. This is a one-time cost that only affects this branch.Install dependencieswall clock:I am not claiming a speed-up from these numbers. The root install is dominated by linking 1,174 packages, not by downloading them, so the difference on a GitHub-hosted runner sits inside run-to-run noise. The download cost the cache removes shows up where registry latency is higher, measured below.
Run 2's macOS validation leg failed on
app_loader_test.tsandbuild_setup_test.ts, and the Windows leg was cancelled by matrix fail-fast. Those are the known macOS timeout flakes, not this change: runs 1 and 3 are green on the same workflow files, and the only difference in run 2 was the lockfile bump.Local proof that a warm npm cache is worth the step, on the
tests/integration/build_setup/ts_esmfixture (606 packages, 297 MB of cache entries). Two installs into the same empty-then-populated cache directory, withnode_modulesand the generated lockfile removed in between:The
integrationvitest project runsnpm installin five such fixtures, so this is the cost the cache removes.Windows carriage-return guard, checked locally:
Without
tr -d '\r'the Windows leg would handactions/cachea path that does not exist and cache nothing.Checklist
[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[ ] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.