fix(server): pnpm-global provider updates no longer leave a broken CLI - #8363
fix(server): pnpm-global provider updates no longer leave a broken CLI#8363angelovdev wants to merge 3 commits into
Conversation
pnpm blocks install scripts by default, so a one-click provider update through pnpm can replace a working global CLI with a broken one and still report success. Scope an --allow-build allowlist to the package being updated, matching the npm-global fix in pingdotgg#5646.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a narrowly scoped two-file fix that updates only pnpm-global provider commands to allow build steps for the exact package being updated, with matching tests and no changes to other update paths. Older pnpm versions may fail explicitly on the new option, but the impact is bounded and avoids silently leaving updated CLIs unusable. You can add or adjust custom eligibility rules. Learn more. |
Dismissing prior approval to re-evaluate f56c567
Problem
pnpm 10 and later block install scripts by default. The one-click provider update runs
pnpm add -g <package>@latest, so for any provider whose postinstall is what finishes the install (fetching or unpacking a platform-native binary over a stub), pnpm skips that step and still exits 0. The user gets a "successful" update and a global CLI that no longer runs.This is the pnpm counterpart of the npm path fixed in #5646, which noted the pnpm and bun paths carried the same class of exposure and deliberately left them alone.
Fix
makePnpmGlobalProviderMaintenanceCapabilitiesnow passes--allow-build=<packageName>, scoped to exactly the package being updated. Two existing test expectations pick up the flag. No other update path changes.Compatibility
Worth a reviewer's attention, because pnpm differs from npm here: pnpm rejects unknown options outright rather than warning and continuing. I confirmed on 11.10.0 that
pnpm add -g --totally-bogus-flag-xyz cowsayexits with[ERROR] Unknown option, while--allow-build=<pkg>is accepted.--allow-buildlanded in pnpm 10.4 (February 2025). On 10.3 and older, this turns a silent partial install into a hard failure on the update. I think a loud failure is the better outcome, and it is still recoverable by hand, but it is a real behavior change that the npm fix did not carry. Happy to version-gate instead if you would rather not raise the floor.Not covered
bun i -ggates lifecycle scripts behind a trusted-dependencies allowlist and has the same exposure. Left out to keep this to one concern.Verification
apps/server/src/provider/providerMaintenance.test.tspasses, 18/18. Lint and format clean on both touched files.Note
Medium Risk
Changes the shell command executed for in-app provider updates on pnpm installs; older pnpm versions without
--allow-buildwill fail the update instead of silently skipping scripts.Overview
Fixes one-click pnpm global provider updates so they no longer report success while leaving a broken CLI when the package needs a postinstall/build step (pnpm 10+ blocks those by default).
makePnpmGlobalProviderMaintenanceCapabilitiesnow adds a package-scoped--allow-build=<npmPackageName>to the generatedpnpm add -g …@latestcommand and args, mirroring the existing npm--allow-scriptsbehavior. Two provider maintenance tests were updated to expect the new flag.Reviewed by Cursor Bugbot for commit 71d8048. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add
--allow-build=<package>flag to pnpm-global provider update commandsUpdates
makePnpmGlobalProviderMaintenanceCapabilitiesin providerMaintenance.ts to insert the package-scoped--allow-build=<npmPackageName>flag intoupdateArgs, placed between-gand<pkg>@latest. This prevents pnpm global updates from leaving a broken CLI after updating packages that require a build step. Test assertions in providerMaintenance.test.ts are updated to expect the new flag and args.Macroscope summarized 71d8048.