Repository navigation
Conversation
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 `@src/install-pnpm/run.ts`:
- Around line 70-74: The unconditional exportVariable('pnpm_config_pm_on_fail',
'download') is overriding project policy even when an explicit target version is
being installed; change it so the export is applied only during the bootstrap
case (no explicit target version). Locate the code that computes the
requested/self-update target (e.g., the variable or function that yields the
explicit version or "targetVersion"/"requestedVersion" used later for
self-update) and wrap the exportVariable('pnpm_config_pm_on_fail', 'download')
call in a conditional that only runs when that target is not present (or when in
bootstrap/install-with-action-version mode); ensure the export is not set once a
concrete version is determined before performing the self-update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0f6af364-d98d-4ef1-8d74-8c8d09e27f6a
⛔ Files ignored due to path filters (1)
dist/index.jsis excluded by!**/dist/**
📒 Files selected for processing (4)
.github/workflows/test.yamlsrc/install-pnpm/bootstrap/exe-lock.jsonsrc/install-pnpm/bootstrap/pnpm-lock.jsonsrc/install-pnpm/run.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Test with run_install (global, windows-latest)
- GitHub Check: Test bin_dest output points to requested version (10.33.2, ubuntu-latest)
- GitHub Check: Test devEngines.packageManager with onFail=error (>=9.15.0, ubuntu-latest)
- GitHub Check: Test devEngines.packageManager with onFail=error (>=9.15.0, macos-latest)
- GitHub Check: Test bin_dest output points to requested version (9.15.5, windows-latest)
- GitHub Check: Test with devEngines.packageManager (windows-latest, 9.15.5)
- GitHub Check: Test with standalone (windows-latest)
- GitHub Check: Test with default inputs (9.15.5, windows-latest)
- GitHub Check: Test packageManager field is respected (10.33.0, macos-latest)
- GitHub Check: Test with run_install (null, windows-latest)
- GitHub Check: Test with run_install (global, windows-latest)
- GitHub Check: Test devEngines.packageManager with onFail=error (>=9.15.0, ubuntu-latest)
- GitHub Check: Test devEngines.packageManager with onFail=error (>=9.15.0, windows-latest)
- GitHub Check: Test with devEngines.packageManager (macos-latest, >=9.15.0)
- GitHub Check: Test devEngines.packageManager with onFail=error (>=9.15.0, macos-latest)
- GitHub Check: Test packageManager field is respected (9.15.5, ubuntu-latest)
- GitHub Check: Test packageManager field is respected (10.33.0, windows-latest)
- GitHub Check: Test version input is actually installed (9.15.5, windows-latest)
- GitHub Check: Test with devEngines.packageManager (ubuntu-latest, >=9.15.0)
- GitHub Check: Test with devEngines.packageManager (windows-latest, >=9.15.0)
🔇 Additional comments (3)
src/install-pnpm/bootstrap/pnpm-lock.json (1)
8-14: Lockfile bump is consistent and looks correct.The root spec, resolved tarball, and integrity hash are aligned for
pnpm@11.0.9.src/install-pnpm/bootstrap/exe-lock.json (1)
8-145: Bootstrap exe lockfile refresh looks internally consistent.
@pnpm/exeand its platform artifacts are coherently pinned to11.0.9with matching resolved URLs and integrity fields..github/workflows/test.yaml (1)
301-350: Good regression coverage fordevEngines.packageManager.onFail=error.This job reproduces the failing path and validates both exact and ranged constraints across all three OS runners.
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 @.github/workflows/test.yaml:
- Around line 94-143: Add a new matrix case that provides an explicit
packageManager version together with devEngines.packageManager.onFail="error"
and an intentionally mismatched REQUIRED so the action should fail; specifically
add a matrix entry (label like 'devEngines onFail=error, explicit mismatch')
whose manifest contains
'{"devEngines":{"packageManager":{"name":"pnpm","version":"9.15.5","onFail":"error"}}}'
but set version: '10.33.0'. Then update the test step "Test: pnpm reports the
pinned version" (the run block that reads REQUIRED from matrix.version and
compares actual="$(pnpm --version)") to handle this new case by asserting the
opposite: for the explicit onFail=error mismatch case ensure the test treats a
mismatch as the expected outcome (i.e., succeed when actual != REQUIRED and fail
if they match) so the workflow verifies that onFail="error" causes a failing
result when an explicit version is provided but doesn't match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e00719cf-42ed-4ec5-89f8-4a52232d7e85
📒 Files selected for processing (1)
.github/workflows/test.yaml
📜 Review details
🔇 Additional comments (1)
.github/workflows/test.yaml (1)
51-73: Nice regression guard for PATH vsbin_destconsistency.The check around Line 51–Line 71 is strong: validating both
pnpmon PATH and"$BIN_DEST/pnpm"directly closes the Windows path-shadow/bin destination regressions cleanly.
Export pnpm_config_pm_on_fail=download so the bootstrap pnpm switches to the version pinned in devEngines.packageManager instead of throwing BAD_PM_VERSION on the user's first invocation.
Only export `pnpm_config_pm_on_fail=download` when the action is relying on bootstrap pnpm's runtime switch. When the user passes an explicit `version:` input, self-update aligns the binary and we should not silently override their `devEngines.packageManager.onFail = "error"` policy.
- Limit push trigger to master so PRs run jobs once instead of twice. - Fold test_default_inputs / test_dest / test_version_respects_request / test_bin_dest_output into one cross-OS `smoke` job (5 matrix entries: ubuntu × 2 versions + ubuntu custom-dest + macos + windows). - Fold test_package_manager_field / test_dev_engines / test_dev_engines_on_fail_error into one ubuntu-only `manifest_pin` job (6 matrix entries) — manifest handling is OS-independent. - Keep standalone and run_install but ubuntu-only. Total runs per PR push: ~88 → 14.
Adds a manifest_pin matrix entry combining `version: 10.33.0` with `devEngines.packageManager` pinned to 9.15.5/onFail=error. The action self-updates to 10.33.0, then `pnpm --version` must fail with BAD_PM_VERSION — confirming pnpm_config_pm_on_fail=download is scoped to the no-target-version branch and does not silently override the user's strict policy.
…vior The previous version of this test self-updated to pnpm 10.33.0 and expected pnpm --version to fail under devEngines.onFail=error. But pnpm v10 only reads the packageManager field, not devEngines, so the check never fired and the test failed. Switch the assertion to the contract the af8e203 scope fix actually enforces: when an explicit version: is supplied, pnpm_config_pm_on_fail must not be exported by the action. That's deterministic and pnpm-version-agnostic.
…11571) `pnpm` could hang for the lifetime of the worker pool after a command logically finished whenever the code path returned through `main` without `process.exit(...)`. `main.ts` only ran `finishWorkers()` inside the command-handler `finally` block. Short-circuit returns that came earlier — `--version`, `--help`, fatal config errors that surface before a command runs — bypassed it and left the integrity-resolution worker pool active. The CLI entry (`pnpm/src/pnpm.ts`) now runs `finishWorkers()` in its own `finally`, so every exit path tears down the pool. Cleanup rejections are swallowed so the `finally` never masks the real command result. ## Reproducer ```bash echo '{"devEngines":{"packageManager":{"name":"pnpm","version":"11.0.9","onFail":"download"}}}' > package.json time pnpm --version # before (Linux): prints 11.0.9, then hangs for the worker-pool lifetime # after: prints 11.0.9, exits in ~1-3 s ``` `switchCliVersion` resolves the integrity (spawning workers), finds nothing to swap (range/version already matches running pnpm), and returns through main's `--version` short-circuit. The unterminated workers used to keep the event loop alive. Surfaced in pnpm/action-setup#254 — bumping the action's bootstrap to 11.0.9 made every CI job using `devEngines.packageManager` with `onFail: "download"` hang.
Bumps [pnpm/action-setup](https://github.com/pnpm/action-setup) from 5.0.0 to 6.1.0. ## Release notes Sourced from pnpm/action-setup's releases. ### v6.1.0 ### What's Changed - feat: support pnpm v12 by `@zkochan` in pnpm/action-setup#288 Full Changelog: pnpm/action-setup@v6.0.10...v6.1.0 ### v6.0.10 ### What's Changed - docs(README): point users to the successor pnpm/setup action by `@BlackHole1` in pnpm/action-setup#282 - refactor: introduce restore keys for cache by `@SukkaW` in pnpm/action-setup#280 - ci: use pnpm 11 for `pr-check` by `@jamietanna` in pnpm/action-setup#284 - fix: update pnpm to v11.19.0 by `@jamietanna` in pnpm/action-setup#283 - docs: update README to include devEngines.packageManager by `@nemchik` in pnpm/action-setup#273 ### New Contributors - `@jamietanna` made their first contribution in pnpm/action-setup#284 - `@nemchik` made their first contribution in pnpm/action-setup#273 Full Changelog: pnpm/action-setup@v6...v6.0.10 ### v6.0.9 ### What's Changed - fix: update pnpm to v11.7.0 by `@zkochan` in pnpm/action-setup#267 Full Changelog: pnpm/action-setup@v6...v6.0.9 ### v6.0.8 ### What's Changed - docs(README): fix `cache_dependency_path` type by `@haines` in pnpm/action-setup#257 - fix: drop patchPnpmEnv so standalone+self-update works on Windows by `@zkochan` in pnpm/action-setup#258 - fix: update pnpm to 11.1.1 by `@mungodewar` in pnpm/action-setup#248 ### New Contributors - `@mungodewar` made their first contribution in pnpm/action-setup#248 Full Changelog: pnpm/action-setup@v6.0.7...v6.0.8 ### v6.0.7 ### What's Changed - fix: honor devEngines.packageManager.onFail=error (#252) by `@zkochan` in pnpm/action-setup#254 - fix: restore inputs from state in post by `@haines` in pnpm/action-setup#255 - fix: self-update bootstrap to packageManager-pinned version (#233) by `@zkochan` in pnpm/action-setup#256 ### New Contributors - `@haines` made their first contribution in pnpm/action-setup#255 Full Changelog: pnpm/action-setup@v6.0.6...v6.0.7 ... (truncated) ## Commits - `ea17c68` feat: support pnpm v12 (#288) - `0977fd9` docs: Update README to include devEngines.packageManager (#273) - `48261ac` fix: update pnpm to v11.19.0 (#283) - `75677f7` ci: use pnpm 11 for `pr-check` (#284) - `769ae71` refactor: introduce restore keys for cache (#280) - `6fed91f` docs(README): point users to the successor pnpm/setup action (#282) - `0ebf471` fix: update pnpm to v11.7.0 (#267) - `0e279bb` fix: update pnpm to 11.1.1 (#248) - `3e83581` fix: drop patchPnpmEnv so standalone+self-update works on Windows (#258) - `551b42e` docs(README): fix `cache_dependency_path` type (#257) - Additional commits viewable in compare view

Summary
pnpm_config_pm_on_fail=downloadfrom the action so the bootstrap pnpm switches versions viamanage-package-manager-versionsinstead of throwingBAD_PM_VERSIONwhen a project pinsdevEngines.packageManager.onFail = "error"without supplying aversion:input.test_dev_engines_on_fail_errorworkflow job (Linux/macOS/Windows, exact + range) that reproduces Action fails with devEngines.packageManager.onFail="error" when version input is omitted #252.Fixes #252.
Test plan
test_dev_engines_on_fail_errorpasses on all three OSestest_dev_engines(onFail=download) still passestest_package_manager_fieldstill passesSummary by CodeRabbit
Chores
Bug Fixes
Tests