feat: add frozen-lockfile and ci install modes - #23
Conversation
The `install` input now selects which install runs: `true`/`install` (`pnpm install`, unchanged default), `frozen-lockfile` (`pnpm install --frozen-lockfile`), `ci` (`pnpm ci`), or `false` to skip. Closes pnpm#8 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FQ4xoewnc8tAGuQ8cyawRC
📝 WalkthroughWalkthroughThe action now supports ChangesInstall mode support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Action
participant Cache
participant InstallLogic
participant pnpm
Action->>Cache: restore dependency cache
Cache-->>Action: cache restored
Action->>InstallLogic: pass parsed install mode
InstallLogic->>pnpm: run selected install command
pnpm-->>InstallLogic: return status
InstallLogic-->>Action: report result
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 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 |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds support for multiple install modes in the action so workflows can choose between a normal install, a frozen-lockfile install, a CI-style clean install, or skipping installs entirely.
Changes:
- Extend the
installinput to accepttrue/install,frozen-lockfile,ci, orfalse. - Update install execution to run the selected pnpm command (
pnpm install,pnpm install --frozen-lockfile, orpnpm ci) and improve related messaging. - Document the new modes and add workflow coverage for the new
installbehaviors and validation.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pnpm-install/index.ts | Builds the pnpm install command based on the selected install mode and executes it. |
| src/inputs/index.ts | Parses and validates the expanded install input values (`InstallMode |
| src/index.ts | Always calls the install step; install skipping is now handled inside pnpmInstall. |
| README.md | Documents the new install modes with examples and caveats. |
| action.yml | Updates the install input documentation/contract to match the new modes. |
| .github/workflows/test.yaml | Adds CI coverage to verify frozen-lockfile, ci, and invalid/empty install behaviors. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review feedback on pnpm#23: - `command` was built before `--no-runtime` was appended, so a failing install reported `pnpm ci` while the log showed `pnpm ci --no-runtime`. Build it from the final args so every message matches. - `if (status)` treated a signal-terminated install as success, since spawnSync reports `status: null` with no `error` in that case. Fail on `signal`, and use `status !== 0`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FQ4xoewnc8tAGuQ8cyawRC
| runtime isn't shadowed by a different version declared in | ||
| `devEngines.runtime`. | ||
| - `true` (the default) or `install`: `pnpm install` | ||
| - `frozen-lockfile`: `pnpm install --frozen-lockfile` — fails instead of |
There was a problem hiding this comment.
Isn't frozen-lockfile the default, so this is unnecessary?
Per the docs:
This setting is true by default in CI environments.
There was a problem hiding this comment.
On pnpm 12 the lockfile is created (if missing) and silently updated (if outdated). On pnpm 11 the lockfile is properly failing when the lockfile is outdated but still created if missing. Tested with pnpm 11.21.0, pnpm 12.0.0-rc.3 with CI=true and GITHUB_ACTIONS=true.
There was a problem hiding this comment.
pnpm v12 should have the same behaviour as v11 (v12 has no intentional breaking changes), so that sounds like a bug that should be reported there.
The problem for me is that having a frozen-lockfile setting here implies that the default is not frozen lockfile, which isn't the case; the default is "frozen lockfile if lockfile exists". Perhaps the setting should be require-lockfile or something to make it clearer 🤷♂️
There was a problem hiding this comment.
Yeah, good idea. I'll change the name of the flag. I’m not a fan of how frozen-lockfile seems to differ when invoked via CI: true vs explicitly using the flag..
Raised an issue here: pnpm/pnpm#13760
…ile` Review feedback on pnpm#23. The value now describes what it guarantees — the install must be fully described by pnpm-lock.yaml — rather than naming the pnpm flag it happens to pass. The flag itself is unchanged. Docs now spell out that this is not the same as pnpm's own CI default: pnpm 11 only blocks updates to an existing lockfile, and pnpm 12 does not apply the CI default at all as of 12.0.0-rc.3. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FQ4xoewnc8tAGuQ8cyawRC
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/test.yaml (2)
667-679: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the renamed install mode explicitly.
The test rejects
install: frozen, but the previous public spelling wasfrozen-lockfile. If the rename intentionally removes the old spelling, addinstall: frozen-lockfileas another invalid case. The current test does not verify that contract change.Suggested coverage
+ - id: renamed + continue-on-error: true + uses: ./ + with: + version: '12.0.0-beta.4' + install: frozen-lockfile + - id: control ... OUTCOME: ${{ steps.invalid.outcome }} + RENAMED: ${{ steps.renamed.outcome }} EMPTY: ${{ steps.empty.outcome }} CONTROL: ${{ steps.control.outcome }} ... + if [ "${RENAMED}" != "failure" ]; then + echo "Expected the old frozen-lockfile mode to be rejected"; exit 1 + fiAlso applies to: 689-704
🤖 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 @.github/workflows/test.yaml around lines 667 - 679, Add a separate invalid workflow test alongside the existing invalid and empty cases, using install: frozen-lockfile with the same version and continue-on-error settings, to explicitly verify the old spelling is rejected after the rename.
524-543: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winVerify that the successful
require-lockfilerun preserves the lockfile.Line 548 creates the checksum after the successful action run at Lines 533-536. If that run modifies
pnpm-lock.yaml, the test records the modified file as the baseline and still passes. Create the checksum before the action and verify it after the install.Suggested assertion
- run: pnpm install --lockfile-only shell: bash + - name: Save matching lockfile checksum + run: sha256sum pnpm-lock.yaml > matching-lockfile.sha256 + shell: bash - uses: ./ with: version: '12.0.0-beta.4' install: require-lockfile - name: 'Test: dependencies installed from the lockfile' run: | set -e if [ ! -d node_modules/is-odd ]; then echo "Expected install: require-lockfile to populate node_modules/is-odd"; exit 1 fi shell: bash + - name: 'Test: matching lockfile was not changed' + run: sha256sum --check --status matching-lockfile.sha256 + shell: bash🤖 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 @.github/workflows/test.yaml around lines 524 - 543, Update the workflow test around the successful require-lockfile action invocation to checksum pnpm-lock.yaml before the action runs, then verify the checksum afterward. Keep the existing node_modules/is-odd installation assertion, but ensure any lockfile modification by the action causes the test to fail.
🤖 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 @.github/workflows/test.yaml:
- Around line 667-679: Add a separate invalid workflow test alongside the
existing invalid and empty cases, using install: frozen-lockfile with the same
version and continue-on-error settings, to explicitly verify the old spelling is
rejected after the rename.
- Around line 524-543: Update the workflow test around the successful
require-lockfile action invocation to checksum pnpm-lock.yaml before the action
runs, then verify the checksum afterward. Keep the existing node_modules/is-odd
installation assertion, but ensure any lockfile modification by the action
causes the test to fail.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 79d059c7-610f-420b-ae03-b15267647b0a
⛔ Files ignored due to path filters (1)
dist/index.jsis excluded by!**/dist/**
📒 Files selected for processing (5)
.github/workflows/test.yamlREADME.mdaction.ymlsrc/inputs/index.tssrc/pnpm-install/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- action.yml
- README.md
- src/pnpm-install/index.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-05-11T16:19:49.450Z
Learnt from: zkochan
Repo: pnpm/setup PR: 1
File: src/cache-restore/run.ts:35-35
Timestamp: 2026-05-11T16:19:49.450Z
Learning: When using `actions/exec` (`getExecOutput` / `exec`), it is valid for the `commandLine` option to include both the command and its arguments in a single string (e.g., `getExecOutput('pnpm store path --silent')`). The library tokenizes `commandLine` internally (via `argStringToArray()`), so this behaves like passing an equivalent command + args array (e.g., `getExecOutput('pnpm', ['store','path','--silent'])`). In code reviews, do not flag this as incorrect—this matches documented behavior and a production-tested pattern.
Applied to files:
src/inputs/index.ts
🔇 Additional comments (2)
src/inputs/index.ts (1)
14-16: LGTM!.github/workflows/test.yaml (1)
582-655: 🗄️ Data Integrity & IntegrationNo change needed. The
install: citest invokespnpm ci --no-runtime, and that command is accepted by the pinned pnpm major track used here.
The
installinput now selects which install runs:true/install(pnpm install, unchanged default),frozen-lockfile(pnpm install --frozen-lockfile),ci(pnpm ci), orfalseto skip.Closes #8
Closes #10
Summary by CodeRabbit
Summary by CodeRabbit