Repository navigation
Stop caching pnpm in npm publishers - #4217
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesNPM publisher setup
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/tests/test_release_pipeline_contract.py (1)
934-942: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winScope the contract assertions to the publisher job and parsed inputs.
These whole-file string checks can pass if another job contains
actions/setup-node@v6, andcache: "pnpm"would evade the literalcache: pnpmcheck. Parse each workflow’spublish-npmjob and assert the actual setup-node step’sregistry-urlandcachevalues.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/tests/test_release_pipeline_contract.py` around lines 934 - 942, The test test_npm_publishers_do_not_enable_an_unused_pnpm_cache must parse each workflow’s publish-npm job instead of scanning the whole file. Locate the actual setup-node step and assert its registry-url equals the npm registry, its cache value is not pnpm regardless of YAML quoting, and the job does not use the local setup-node action.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/tests/test_release_pipeline_contract.py`:
- Around line 934-942: The test
test_npm_publishers_do_not_enable_an_unused_pnpm_cache must parse each
workflow’s publish-npm job instead of scanning the whole file. Locate the actual
setup-node step and assert its registry-url equals the npm registry, its cache
value is not pnpm regardless of YAML quoting, and the job does not use the local
setup-node action.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b6fc5d4-7eb6-4679-a6f5-cbaac8c61943
📒 Files selected for processing (3)
.github/workflows/publish2-nodejs-sdk.yaml.github/workflows/publish2-web-sdk.yamlscripts/tests/test_release_pipeline_contract.py
|
Addressed CodeRabbit’s valid contract-test nit in |
Binary size checks passed✅ 7 passed
Generated by |
Summary
actions/setup-node@v6directly in the Node.js and Web npm-only publishersWhy
The
0.15.1-nightly.20260727.aNode and Web packages were successfully uploaded with OIDC provenance, but both release jobs failed afterward in the shared setup action's pnpm cache post-step. These publisher jobs intentionally install no pnpm dependency tree, so the cache path does not exist and should not be configured.Validation
python3 -m unittest scripts.tests.test_release_pipeline_contract(20 tests)actionlintfor both changed workflows (excluding pre-existing SC2055 in untouched Web publish code)git diff --checkSummary by CodeRabbit
Bug Fixes
Tests