perf: cache pnpm's lockfile verification results - #30
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe action now caches pnpm lockfile verification results with the pnpm store. It restores and saves verification data by lockfile hash, resolves platform-specific cache paths, normalizes Windows paths, and validates the workflow on Linux, macOS, and Windows. ChangesLockfile verification cache
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to The change adds best-effort caching for pnpm lockfile verification results without introducing a supplied merge-blocking correctness or production risk; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Workflow
participant SetupAction
participant VerificationCache
participant PnpmStore
Workflow->>SetupAction: Run with pnpm 12.0.0
SetupAction->>VerificationCache: Restore by lockfile hash
SetupAction->>PnpmStore: Restore store cache when requested
SetupAction->>VerificationCache: Snapshot and save verification log
SetupAction->>PnpmStore: Prune and save store cache
Workflow->>VerificationCache: Check lockfile-verified.jsonl
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
PR Summary by Qodoperf: cache pnpm's lockfile verification results across CI runs
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains. Reviews (9): Last reviewed commit: "fix: keep the pnpm pin the Windows store..." | Re-trigger Greptile |
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
pnpm v11+ verifies every lockfile entry against the configured supply-chain policies and memoizes the verdict in `<cacheDir>/lockfile-verified.jsonl`. `cache: true` caches only the store, so every job re-checks the whole lockfile against the registry — 16.6s on a 2058-entry lockfile with a warm store, 40.1s of a 42.4s install on Windows. Restore the log before the install and save it under its own key, keyed on the exact lockfile hash with no prefix fallback: a verdict is only valid for the lockfile it was recorded for. It is cached regardless of `cache`, being a fraction of a kilobyte. Every failure in this path is a warning — the worst case is that the next job re-verifies. Saving runs before `pnpm store prune`, which deletes the log along with the store's other derived state, and immediately after the install that wrote it, so nothing the job runs afterwards ends up in what other jobs restore. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R7B41egL5GwZk1gw2DU7sY
7f46262 to
9dce9c1
Compare
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
`pnpm cache path` reports it directly, in v11 as well as v12 — v11 being the oldest release this action installs — so the per-platform default no longer has to be mirrored here. It also removes the `pnpm config get` detour, which reports settings rather than defaults and prints `undefined` for an unset `cacheDir`. The warning v11 emits about global config goes to stderr, so stdout is just the path. The regression job now compares against `pnpm cache path` too, rather than deriving the location a second way in shell. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R7B41egL5GwZk1gw2DU7sY
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
`pnpm cache path` is not in every release the action installs: 11.22.0 has it, 12.0.0-rc.4 does not, and the regression job pinned rc.4 — which is how CI caught this. Ask pnpm first and fall back to `pnpm config get cacheDir`, then to pnpm's per-platform default, for releases that cannot answer. The job derives the location in shell again rather than reading it back from `pnpm cache path`, so it still guards that fallback against pnpm's own behaviour on all three platforms. Bump every pinned 12.x prerelease across the workflow, the README and action.yml to the released 12.0.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R7B41egL5GwZk1gw2DU7sY
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
The blanket bump to 12.0.0 flattened this one too, but pnpm 12.0.0 reports a plain `D:\.pnpm-store\v11` for that store-dir — only an older release still emits the `\\?\` prefix the job exists to cover, so the assertion failed. Restore 12.0.0-rc.0 and say in the comment that the pin is load-bearing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R7B41egL5GwZk1gw2DU7sY
Why
pnpm v11 and newer verify every lockfile entry against the configured supply-chain policies (
minimumReleaseAge,trustPolicy, …) and memoize the verdict in<cacheDir>/lockfile-verified.jsonl.cache: truecaches onlypnpm store path, so every job starts with that verdict missing and re-checks the whole lockfile against the registry.Measured on typescript-eslint's repository (2058 lockfile entries), where the store cache was warm:
On Windows it was
40.1sof a42.4sinstall. Locally, the same install with the verdict already recorded takes 1.5s instead of 13.5s — the log itself is under a kilobyte.What
pnpm installand saved in the post step, under its own key,pnpm-lockfile-verified-<OS>-<arch>-<lockfile hash>.pnpm store prune, which deletes the log along with the store's other derived state.pnpm cache path, which pnpm reports in v11 as well as v12.warning, never a failed build: the worst case is that the next job re-verifies.Tests
A new job installs with a supply-chain policy configured on ubuntu, macOS and Windows, then asserts that pnpm wrote
lockfile-verified.jsonlwhere the action looks for it — that the log is named and placed as expected on every platform.Follow-ups (not in this PR)
pnpm store prunedeleteslockfile-verified.jsonl; the Rust CLI's does not. Worth reconciling — the log is derived from the lockfile and the policies, not from the store.Written by an agent (Claude Code, claude-opus-5).
Summary by CodeRabbit
New Features
Documentation