Skip to content

fix(setup-node-with-cache): compute the dependency cache hash once, before restore - #273

Closed
sh-waqar wants to merge 1 commit into
mainfrom
fix/setup-node-cache-key-once
Closed

sh-waqar wants to merge 1 commit into
mainfrom
fix/setup-node-cache-key-once

Conversation

@sh-waqar

Copy link
Copy Markdown
Contributor

Problem

Debug cache contents and Log cache status and decision print the cache key by recomputing it:

echo "Cache key: …-pnpm-${{ hashFiles('**/pnpm-lock.yaml', '**/package.json') }}"

They run after the cache restore, so **/package.json also matches every package.json inside the restored node_modules. GitHub evaluates both the yarn and pnpm lines in each run: block, so each step hashes that tree twice. That causes two problems:

  • It's slow: about 8 s per step, and every caller pays it on every job.
  • The logged key is wrong: it doesn't match the key the cache was actually restored with.

Fix

  • A new Compute dependency cache hash step (id: dep-hash) runs before the restore. It hashes the same files as today.
  • The two cache keys and the four logging lines reuse steps.dep-hash.outputs.{yarn,pnpm}.
  • No new inputs.
  • The cache keys don't change: the hash is calculated from the same files at the same point as before, so existing caches keep hitting.

Verified

Tested on frontend-packages in throwaway PR #37: the same inputs as frontend-pr-workflow, run side by side on ci-universal-scale-set, with both runs hitting the same cache.

@v1 (current) this PR
Debug cache contents 9.7 s under 1 s
Log cache status and decision 7.8 s under 1 s
Post Setup Node with Cache 5 s 1 s
Logged key …4c45dc… ❌ …15aca3… ✅
Key restored …15aca3…-full …15aca3…-full
Dependencies usable after restore ✅ ✅

That's about 20 s saved per job. frontend-pr-workflow runs this action in both Build and Unit Tests, so each PR saves about 40 s of runner time.

The #258 idea, keying on the lockfile alone, is left for a separate change.

🤖 Generated with Claude Code

…efore restore

The debug and log steps recomputed hashFiles('**/package.json', ...) after the
cache restore, when the glob also matches every package.json inside the restored
node_modules. On frontend-packages each call took ~4s (two per step, since both
the yarn and pnpm lines are evaluated), ~16s per job, and printed a different key
than the one the cache was restored with. Hash once in a dep-hash step before the
restore and reuse it for the cache keys and both log lines. Keys are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sh-waqar
sh-waqar requested a review from a team as a code owner September 28, 2026 17:25
@pr-auditor

pr-auditor Bot commented Sep 28, 2026

Copy link
Copy Markdown

✅ Security Analysis Results

No security issues found. 1 files reviewed.


@pr-auditor rescan to re-run · Powered by Claude Sonnet 5 · Docs · #security-engineering-team

@sh-waqar

Copy link
Copy Markdown
Contributor Author

Closing for now. This is deferred, not rejected: nothing depends on it, and it isn't worth a v1 release on its own. The branch fix/setup-node-cache-key-once is kept, so this can be reopened as is. It's verified in frontend-packages #37 (same cache keys, about 17–20 s saved per job) and could ship with the next v1 release.

@sh-waqar sh-waqar closed this Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant