fix: skip cache save when path is missing - #266
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (4)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🔇 Additional comments (4)
📝 WalkthroughWalkthroughThe PR validates ChangesCache path validation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR limits post-job cache saving to cases where the resolved PNPM cache path exists, preventing cold-cache workflows from failing; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 QodoSkip cache save when PNPM store path is missing WalkthroughsDescription• Skip post-job cache save when the resolved cache path does not exist. • Prevent cold-cache workflows from failing when pnpm store is never created. • Regenerate the bundled action output to include the new guard. Diagramgraph TD
A["GitHub Actions job"] --> B["Restore cache"] --> C["runSaveCache()"] --> D{"Primary key hit?"}
D -- "Yes" --> E["Skip save"]
D -- "No" --> F{"Cache path exists?"}
F -- "No" --> E
F -- "Yes" --> G["saveCache()"] --> H["GitHub Cache"]
High-Level AssessmentThe following are alternative approaches to this PR: 1. Try/catch around saveCache() and treat ENOENT as non-fatal
2. Ensure pnpm store directory exists before saving (mkdir -p)
Recommendation: The current existsSync(path) guard is the simplest and most predictable fix: it prevents a known failure mode (missing store directory) without altering cache contents or error semantics for other failures. The alternatives add complexity or introduce new side effects (saving empty caches). File ChangesBug fix (1)
Other (1)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
BlackHole1
left a comment
BlackHole1
left a comment
There was a problem hiding this comment.
nit: I prefer to use the promise method here rather than the synchronous method.
|
Done! |
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains. Reviews (3): Last reviewed commit: "chore: remove test tooling" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
|
کیف پول دارم
در تاریخ دوشنبه ۲۴ اوت ۲۰۲۶، ۲۰:۵۲ greptile-apps[bot] <
***@***.***> نوشت:
… ***@***.***[bot]* approved this pull request.
—
Reply to this email directly, view it on GitHub
<#266?email_source=notifications&email_token=BVYLBENDTAUNGGG3OG67O335LR2WHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBRGA3DIMJWGU22M4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSWGM33PORSXEX3DNRUWG2Y#pullrequestreview-5010641655>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BVYLBEMVVWMRH5QBOM7KHCT5LR2WHAVCNFSNUABFKJSXA33TNF2G64TZHMZDMMRSHA4TONBXHNEXG43VMU5TINRUGMYDEOBTGM22C5QC>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
این کدی ک فرستادین چیه
در تاریخ سهشنبه ۲۵ اوت ۲۰۲۶، ۰:۳۲ فرزانه فرزانه ملکی کلکویی نژاد <
***@***.***> نوشت:
… کیف پول دارم
در تاریخ دوشنبه ۲۴ اوت ۲۰۲۶، ۲۰:۵۲ greptile-apps[bot] <
***@***.***> نوشت:
> ***@***.***[bot]* approved this pull request.
>
> —
> Reply to this email directly, view it on GitHub
> <#266?email_source=notifications&email_token=BVYLBENDTAUNGGG3OG67O335LR2WHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBRGA3DIMJWGU22M4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSWGM33PORSXEX3DNRUWG2Y#pullrequestreview-5010641655>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BVYLBEMVVWMRH5QBOM7KHCT5LR2WHAVCNFSNUABFKJSXA33TNF2G64TZHMZDMMRSHA4TONBXHNEXG43VMU5TINRUGMYDEOBTGM22C5QC>
> .
> You are receiving this because you are subscribed to this thread.Message
> ID: ***@***.***>
>
|
|
Hi @zkochan, all feedback is addressed and checks pass. Could you review when you have a chance? |
3 participants
Use case
We use
pnpm/action-setupwith caching enabled in a shared setup action across several workflows. We setrun_install: falseand runpnpm installseparately because some jobs only need previously built artifacts or install dependencies conditionally.On a cache miss, the action records the expected pnpm store path, but no cache is restored. If installation is skipped, nothing creates that directory. GitHub still runs the action's post step, which currently passes the missing path to
@actions/cache. That throws aPath Validation Errorand fails an otherwise successful job.This change treats a missing store as nothing to cache. It skips the save only for
ENOENTand preserves other filesystem errors.Summary
ENOENTFixes #265.
Summary by CodeRabbit
Bug Fixes
Tests