fix(ci): bump the cache-key salt so #9596's widened path list actually takes effect - #9597
Merged
Conversation
…y takes effect #9596 widened the node_modules cache to every workspace but left the key untouched, so nothing changed for a branch with an unchanged lockfile: it keeps HITTING the entry archived under the narrow list, which restores no apps/loopover-miner-ui/node_modules, and a hit skips npm ci -- leaving the tree exactly as broken and UI lint still failing on 'Cannot find module @eslint/js'. Add an explicit npm-v2- generation salt so one clean miss re-archives a complete tree under the current path list, and document that the salt must be bumped whenever that list changes.
Contributor
|
Superagent did not find any vulnerabilities or security issues in this PR. |
Contributor
|
Important 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏳ LoopOver is waiting…LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting |
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
JSONbored
added a commit
that referenced
this pull request
Jul 28, 2026
…ath list (#9603) #9596/#9597 changed the node_modules cache to glob every workspace and added a generation salt to the key, but the composite-action test still asserted the old literal `apps/loopover-ui/node_modules`, so it has been failing on main and on every branch since. Assert the contract that now holds, and tighten it in two places the old test left open: - both workspace globs, matching what package.json declares; - the `npm-v2-` salt, asserted beside the paths, because widening the paths without bumping the salt is a no-op and the two must move together; - the SAVE step's path list equals the RESTORE step's -- the save list decides what a future run can restore, so a narrowing there would stay invisible until some later job failed on a missing workspace.
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
1 participant
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.
Why this is needed on top of #9596
#9596 widened the node_modules cache to every workspace, but the salt commit that made it work raced the gate's auto-merge and did not land — the merged action has the widened
pathlist and the original key.That combination is a no-op. The key is computed from the manifests only, so a branch with an unchanged lockfile keeps hitting the entry archived under the narrow list. That entry contains no
apps/loopover-miner-ui/node_modules, and because it's a hit,npm ciis skipped — so the tree stays exactly as incomplete and UI lint keeps failing onCannot find module '@eslint/js'.The fix
Add an explicit
npm-v2-generation salt to the key, forcing one clean miss so a complete tree is archived under the current path list. The comment states the rule for next time: bumping the salt is required whenever thepathlist changes, since widening a path list never invalidates entries archived under the old one.Verified
lint:composite-actionsclean; the real proof is the first run here installing fresh and every later PR restoring a complete tree.