chore: relax devEngines.packageManager requirements - #6459
Conversation
There was a problem hiding this comment.
🤖 AI Review
Both independent reviews completed. After deduplication, the repository-wide weakening of the pnpm guard is confirmed as a minor concern, and the removal of the only explanation for its CI coupling is confirmed as a nit. The alleged risk that the preview smoke test can execute the same-version local workspace package is refuted: the root does not expose that package, and npx resolves a remote tarball by source identity rather than name/version alone.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | package.json:48 |
package-management |
claude+codex | Changing devEngines.packageManager.onFail from error to warn weakens the pnpm-only package-manager guard for every repository-root npm operation instead of scoping the exception to the preview smoke test. |
| ⚪ NIT | package.json:48 |
maintainability |
claude | The dependency between the warning policy and in-checkout npx CI commands is no longer documented. |
Refuted findings (kept for transparency, not posted as review comments)
.github/workflows/publish-preview-cli-packages.yml:134(ci): Running the preview smoke test in the checkout may execute the locally version-synchronized supabase workspace package instead of the published preview tarball.
Refuted: package.json:31-43 has no supabase dependency, so the pnpm-installed root tree does not expose the apps/cli package to npx. More importantly, PREVIEW_URL is a non-registry remote tarball spec; npm exec matches such packages by their resolved source, not merely by package name and version. A same-version local workspace link therefore would not satisfy this request.
Stats
Claude findings: 3 · Codex findings: 1 · Confirmed: 2 · Refuted: 1 · Uncertain: 0
Models: claude-opus-5 + gpt-5.6-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@75dd8992be24644bba6b4888635ae041fe359795Preview package for commit |
Summary
In #6424, I added a strict
onFail: 'error'property so any time someone usesnpm,yarn,or an incorrectpnpmversion, those CLIs error out.This was a bit aggressive — we use the
npmCLI for a variety of things related to our release process. Rather than backport all of thosenpmusages topnpm, I opted to updateonFailto bewarn.That should unblock one error in our release process, hopefully that's all of them? I'm not sure what to make of this error 🤔