fix: handle missing corepack in setup.sh with npm fallback#9202
fix: handle missing corepack in setup.sh with npm fallback#9202valkrypton wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe setup.sh script now checks for the presence of corepack before enabling pnpm; if corepack is missing it prints Node.js/corepack installation guidance and exits. If corepack is present it runs ChangesSetup Script Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@setup.sh`:
- Around line 84-89: The npm-based fallback in the elif branch (the lines that
call `npm install -g corepack`, `npm install -g pnpm`, and the fallback block)
violates the package-manager policy; remove the npm-based installation logic and
instead attempt only `corepack enable pnpm` when corepack is present, and if
corepack is not available or enabling fails, print a clear remediation error
(e.g., instructing the user to install Corepack or pnpm manually) and exit
non‑zero (set success=false or call exit 1). Update the branch that currently
runs when `command -v npm` would match so it only reports the missing
Corepack/PNPM path and fails, referencing the existing conditional block that
echoes "${YELLOW}corepack not found — installing pnpm via npm...${NC}" to
replace that behavior with the policy-compliant failure message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Replaces npm fallback with an explicit error directing users to install Node.js 16.9+, keeping setup.sh pnpm/corepack-only per repo policy. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
daf2c19 to
dce8679
Compare
Falls back to npm install -g corepack, then npm install -g pnpm if corepack is unavailable. Adds guard before pnpm install to avoid silent exit 127.
Description
Type of Change
Summary by CodeRabbit