Skip to content

refactor: introduce restore keys for cache - #14

Merged
zkochan merged 2 commits into
pnpm:mainfrom
SukkaW:cache-restore-keys
Aug 2, 2026
Merged

refactor: introduce restore keys for cache#14
zkochan merged 2 commits into
pnpm:mainfrom
SukkaW:cache-restore-keys

Conversation

@SukkaW

@SukkaW SukkaW commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Add restoreKeys for restoring the store cache.

The idea is that we currently include the pnpm-lock.yaml content hash within the cache primary key. This means even if only one dependency has changed, the entire pnpm-lock.yaml content hash changes, and the entire pnpm store cache is now thrown away.

But we don't have to invalidate the entire store. Other dependencies' cache may still be utilized. We can still restore the previous store cache, and only the changed dependencies need to be and will be downloaded.

restoreKeys doesn't change the fact that the cache is still miss (primaryKey), so during the post action we still upload the latest pnpm store to a new cache entry.

The PR is almost identical to pnpm/action-setup#280.

Summary by CodeRabbit

  • Bug Fixes
    • Improved pnpm cache restoration by supporting partial matches across platforms.
    • Updated cache-hit reporting to accurately indicate whether the exact requested cache key was restored.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1d5dd190-6fff-46be-9efb-58b3182161f5

📥 Commits

Reviewing files that changed from the base of the PR and between 6523ce9 and 495b168.

⛔ Files ignored due to path filters (1)
  • dist/index.js is excluded by !**/dist/**
📒 Files selected for processing (1)
  • src/cache-restore/run.ts
📜 Recent review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-05-11T16:19:49.450Z
Learnt from: zkochan
Repo: pnpm/setup PR: 1
File: src/cache-restore/run.ts:35-35
Timestamp: 2026-05-11T16:19:49.450Z
Learning: When using `actions/exec` (`getExecOutput` / `exec`), it is valid for the `commandLine` option to include both the command and its arguments in a single string (e.g., `getExecOutput('pnpm store path --silent')`). The library tokenizes `commandLine` internally (via `argStringToArray()`), so this behaves like passing an equivalent command + args array (e.g., `getExecOutput('pnpm', ['store','path','--silent'])`). In code reviews, do not flag this as incorrect—this matches documented behavior and a production-tested pattern.

Applied to files:

  • src/cache-restore/run.ts
🔇 Additional comments (1)
src/cache-restore/run.ts (1)

21-29: LGTM!


📝 Walkthrough

Walkthrough

runRestoreCache now supports platform-specific partial cache restoration. The cache-hit output is true only when the restored key exactly matches the primary key.

Changes

Cache restore behavior

Layer / File(s) Summary
Restore keys and hit detection
src/cache-restore/run.ts
restoreCache now uses a platform-specific pnpm-cache fallback prefix. The cache-hit output now checks for an exact primary-key match.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: zkochan

Poem

A rabbit tucked the cache away,
With fallback keys for clouds of gray.
An exact match earns a happy cheer,
While partial keys restore what’s near.
Hop, hop—clean hits appear!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the introduction of restore keys for cache restoration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add restore keys for pnpm store cache fallback

✨ Enhancement 🕐 10-20 Minutes

Grey Divider

AI Description

• Add restore keys to reuse prior pnpm store caches across lockfile hash changes.
• Report cache-hit only when the primary key matches exactly.
• Regenerate the compiled dist bundle to ship the updated caching behavior.
Diagram

graph TD
  A["GitHub Action step"] --> B["Compute lockfile hash"] --> C["Primary cache key"] --> E[("GitHub Cache")]
  D["Restore keys prefix"] --> E
  E --> F["pnpm store dir"] --> G["Output: cache-hit (exact match)"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Remove lockfile hash from the primary key
  • ➕ Maximizes cache reuse across dependency changes
  • ➖ Higher risk of stale/incorrect caches; less deterministic behavior
  • ➖ Harder to reason about cache correctness across branches/changes
2. Use more granular restore keys (e.g., based on pnpm version + major lockfile changes)
  • ➕ Better balance between reuse and relevance than a broad OS/arch prefix
  • ➖ More complexity; requires defining a stable, meaningful granularity signal
  • ➖ May still not outperform the simple prefix approach in practice

Recommendation: Current approach (keep a strict primary key, add a broad OS/arch restoreKeys prefix) is the best tradeoff: it preserves correctness and deterministic cache hits while improving performance by reusing older pnpm store contents after small lockfile changes. Ensure reviewers agree with the updated cache-hit semantics (exact primary-key match only), which is typically what downstream workflows expect.

Files changed (2) +113 / -107

Enhancement (1) +8 / -2
run.tsAdd restoreKeys fallback and tighten cache-hit semantics +8/-2

Add restoreKeys fallback and tighten cache-hit semantics

• Adds an OS/arch-based restore key prefix to allow restoring a previous pnpm store cache even when the lockfile hash changes. Updates the 'cache-hit' output to be true only when the restored key equals the primary key.

src/cache-restore/run.ts

Other (1) +105 / -105
index.jsRebuild dist bundle with restoreKeys cache behavior +105/-105

Rebuild dist bundle with restoreKeys cache behavior

• Updates the generated distribution artifact to include the new restoreKeys-based cache restore logic and the refined 'cache-hit' output behavior.

dist/index.js

@zkochan
zkochan merged commit 4f13ee5 into pnpm:main Aug 2, 2026
43 of 44 checks passed
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.

3 participants