fix(ci): pin pnpm via pnpm/action-setup@v4 and harden playwright version detection#4091
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughSix GitHub Actions workflows migrate pnpm installation from shell-based global ChangesWorkflow pnpm Installation Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
.github/workflows/functional-tests.yml (2)
422-429: ⚡ Quick win
pnpm/action-setupplaced aftermake pre-build— move it before for consistency and resilience.In
build_enterprise_frontendpnpm is installed beforemake, but heremake pre-buildruns at line 422-424 before pnpm is available.get_playwright_test_filesalso runsmake pre-buildwithout pnpm, suggesting it works today, but the ordering diverges from the established pattern and will silently break if the Makefile ever calls pnpm duringpre-build.♻️ Proposed fix: hoist pnpm install before make pre-build
+ - name: Install pnpm + uses: pnpm/action-setup@v6 + with: + version: 10 + - name: Pre-build Enterprise Frontend working-directory: ${{ env.enterprise-frontend-directory }} run: make pre-build - - name: Install pnpm - uses: pnpm/action-setup@v6 - with: - version: 10 - - name: Install enterprise frontend dependencies🤖 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/functional-tests.yml around lines 422 - 429, Move the "Install pnpm" step to run before the "Pre-build Enterprise Frontend" step: ensure the pnpm/action-setup@v4 step (with version: 10) is hoisted above the step that runs make pre-build (refer to the "Pre-build Enterprise Frontend" step and the make pre-build invocation) so pnpm is available if the Makefile starts using pnpm in pre-build.
422-429: ⚡ Quick win
pnpm/action-setupplaced aftermake pre-build— inconsistent withbuild_enterprise_frontend.In
build_enterprise_frontend(line 99-108), pnpm is installed beforemake. Here inenterprise-functional-tests,make pre-buildruns at line 422-424 before pnpm is available at lines 426-429. Whileget_playwright_test_filesalso runsmake pre-buildwithout pnpm (suggesting it doesn't require pnpm directly today), this ordering is fragile and diverges from the pattern established inbuild_enterprise_frontend. Moving thepnpm/action-setupstep beforemake pre-buildcosts nothing and prevents a silent break if the Makefile changes.♻️ Proposed fix: move pnpm install before `make pre-build`
+ - name: Install pnpm + uses: pnpm/action-setup@v4 + with: + version: 10 + - name: Pre-build Enterprise Frontend working-directory: ${{ env.enterprise-frontend-directory }} run: make pre-build - - name: Install pnpm - uses: pnpm/action-setup@v4 - with: - version: 10 - - name: Install enterprise frontend dependencies🤖 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/functional-tests.yml around lines 422 - 429, The "Pre-build Enterprise Frontend" step runs `make pre-build` before the "Install pnpm" step, creating an ordering inconsistency with the earlier `build_enterprise_frontend` pattern; move the `pnpm/action-setup@v4` step (the "Install pnpm" step) so it executes before the "Pre-build Enterprise Frontend" step (i.e., install pnpm prior to running `make pre-build`), mirroring the ordering used in `build_enterprise_frontend` and avoiding potential fragile failures similar to `get_playwright_test_files`..github/workflows/startup-tests.yml (2)
244-246: ⚡ Quick win
enterprise-startup-*jobs rely on pnpm availability frommake/make pre-build— not updated in this PR.Lines 244-246, 259-261, and 289-291 call
pnpm execwithout apnpm/action-setupstep. These jobs were intentionally left unchanged, but if the Makefile stops installing pnpm globally, they will silently break. Consider addingpnpm/action-setup@v4to these jobs for the same consistency guarantees this PR achieves elsewhere.Also applies to: 259-261, 286-291
🤖 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/startup-tests.yml around lines 244 - 246, The workflow uses `pnpm exec playwright install` in the "Install Playwright Browsers" step (and the other similar steps in the enterprise-startup-* jobs) without ensuring pnpm is available; add a `uses: pnpm/action-setup@v4` step (with the desired node version if needed) immediately before any `pnpm exec` invocation so `pnpm` is guaranteed to be installed even if the Makefile stops installing it globally, and apply this to all occurrences of `pnpm exec playwright install` across the enterprise-startup-* jobs.
244-246: ⚡ Quick win
enterprise-startup-*jobs callpnpm execwith nopnpm/action-setupstep — add explicit pnpm install for consistency.These jobs use
pnpm execwithout apnpm/action-setupstep, relying on pnpm being available throughmake. While themaketargets likely install pnpm themselves (otherwise these jobs would already be broken), this is an implicit dependency that could silently break if the Makefile changes. Addingpnpm/action-setup@v6(once upgraded) here is a low-cost change for explicit, durable behavior consistent with the other jobs updated in this PR.Also applies to: 259-261, 289-291
🤖 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/startup-tests.yml around lines 244 - 246, The "Install Playwright Browsers" step (and the other pnpm exec steps in the enterprise-startup-* jobs) relies implicitly on pnpm being present; add an explicit pnpm/action-setup step before each pnpm exec to ensure pnpm is installed and pinned (use uses: pnpm/action-setup@v6), so insert that step immediately prior to the "Install Playwright Browsers" run step (and the analogous steps at the other two locations) to make pnpm availability explicit and consistent with the rest of the workflow.
🤖 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/frontend-coverage.yaml:
- Around line 39-42: The workflow step named "Install pnpm" is using the
deprecated action reference "pnpm/action-setup@v4"; update the step to use
"pnpm/action-setup@v6" (replace the uses value) so the workflow uses the
supported major version, keeping the existing with: version: 10 configuration
intact and then run/check the workflow to ensure the action resolves correctly.
In @.github/workflows/frontend-linters.yaml:
- Around line 38-41: The workflow uses the deprecated pnpm/action-setup@v4 in
the "Install pnpm" steps; update both occurrences (the steps labeled "Install
pnpm" that currently reference pnpm/action-setup@v4) to pnpm/action-setup@v6 so
the lint jobs use the supported action version.
In @.github/workflows/frontend-unit-tests.yml:
- Around line 38-41: Update every workflow that currently uses the pnpm setup
action to pin the v6 release instead of v4: replace occurrences of "uses:
pnpm/action-setup@v4" with "uses: pnpm/action-setup@v6" in all six workflow
files (the install step that names "Install pnpm" using pnpm/action-setup).
Ensure each workflow's "Install pnpm" step is changed so the action references
`@v6` to get Node.js 24 support and avoid the upcoming deprecation.
In @.github/workflows/functional-tests.yml:
- Around line 53-56: The workflow uses the deprecated action reference
pnpm/action-setup@v4 in multiple "Install pnpm" steps; update each occurrence to
pnpm/action-setup@v6 so the action is the supported version (keep existing
inputs like version: 10 unchanged). Locate the steps named "Install pnpm" that
currently have uses: pnpm/action-setup@v4 and change the uses value to
pnpm/action-setup@v6 for all four occurrences.
In @.github/workflows/rpm-build.yml:
- Around line 62-65: Update the "Install pnpm" workflow step to use
pnpm/action-setup@v6 instead of `@v4`; the action only installs pnpm (it does not
provision Node.js) and will work in this container because Node.js is installed
via the NodeSource install step, but you must upgrade the action to v6 to avoid
the Node.js 20 deprecation timeline.
- Around line 57-65: Replace the non-standard Node installation block that runs
"dnf install -y nodejs" and the subsequent "pnpm/action-setup@v4" usage with the
supported setup pairing: call "actions/setup-node" (e.g., actions/setup-node@v4)
to install Node on PATH before invoking "pnpm/action-setup@v4"; update the
workflow steps so the Node setup step runs prior to the pnpm action and remove
the dnf-based installation to ensure compatibility with pnpm/action-setup@v4.
In @.github/workflows/startup-tests.yml:
- Around line 83-86: Update the GitHub Actions step(s) that use
pnpm/action-setup@v4 (the steps named "Install pnpm") to use
pnpm/action-setup@v6 so the action supports Node.js 20; change the uses
references from pnpm/action-setup@v4 to pnpm/action-setup@v6 for the occurrences
around the "Install pnpm" step (also update the second occurrence noted at lines
136-139).
---
Nitpick comments:
In @.github/workflows/functional-tests.yml:
- Around line 422-429: Move the "Install pnpm" step to run before the "Pre-build
Enterprise Frontend" step: ensure the pnpm/action-setup@v4 step (with version:
10) is hoisted above the step that runs make pre-build (refer to the "Pre-build
Enterprise Frontend" step and the make pre-build invocation) so pnpm is
available if the Makefile starts using pnpm in pre-build.
- Around line 422-429: The "Pre-build Enterprise Frontend" step runs `make
pre-build` before the "Install pnpm" step, creating an ordering inconsistency
with the earlier `build_enterprise_frontend` pattern; move the
`pnpm/action-setup@v4` step (the "Install pnpm" step) so it executes before the
"Pre-build Enterprise Frontend" step (i.e., install pnpm prior to running `make
pre-build`), mirroring the ordering used in `build_enterprise_frontend` and
avoiding potential fragile failures similar to `get_playwright_test_files`.
In @.github/workflows/startup-tests.yml:
- Around line 244-246: The workflow uses `pnpm exec playwright install` in the
"Install Playwright Browsers" step (and the other similar steps in the
enterprise-startup-* jobs) without ensuring pnpm is available; add a `uses:
pnpm/action-setup@v4` step (with the desired node version if needed) immediately
before any `pnpm exec` invocation so `pnpm` is guaranteed to be installed even
if the Makefile stops installing it globally, and apply this to all occurrences
of `pnpm exec playwright install` across the enterprise-startup-* jobs.
- Around line 244-246: The "Install Playwright Browsers" step (and the other
pnpm exec steps in the enterprise-startup-* jobs) relies implicitly on pnpm
being present; add an explicit pnpm/action-setup step before each pnpm exec to
ensure pnpm is installed and pinned (use uses: pnpm/action-setup@v6), so insert
that step immediately prior to the "Install Playwright Browsers" run step (and
the analogous steps at the other two locations) to make pnpm availability
explicit and consistent with the rest of the workflow.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6cacc92d-8e36-48d0-8a55-9afdc8e95385
📒 Files selected for processing (6)
.github/workflows/frontend-coverage.yaml.github/workflows/frontend-linters.yaml.github/workflows/frontend-unit-tests.yml.github/workflows/functional-tests.yml.github/workflows/rpm-build.yml.github/workflows/startup-tests.yml
| - name: Install Node.js | ||
| run: | | ||
| curl -fsSL https://rpm.nodesource.com/setup_22.x | bash - | ||
| dnf install -y nodejs | ||
| npm install -g pnpm | ||
|
|
||
| - name: Install pnpm | ||
| uses: pnpm/action-setup@v4 | ||
| with: | ||
| version: 10 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does pnpm/action-setup@v4 work in GitHub Actions container jobs where Node.js is not installed via actions/setup-node?
💡 Result:
No, pnpm/action-setup@v4 does not work in GitHub Actions container jobs where Node.js is not installed via actions/setup-node (or otherwise pre-installed in the container image). The action runs as a Node.js script and installs pnpm by spawning the Node.js process (process.execPath) to execute its bundled pnpm.cjs self-installer script, which performs node pnpm.cjs install pnpm@${version} --no-lockfile in a temporary directory to bootstrap pnpm[1][2]. Without Node.js available on the PATH or in the environment, this spawn will fail with an error like ENOENT (no such file or directory). Official documentation explicitly states: "This action does not setup Node.js for you, use actions/setup-node yourself."[3] All examples pair it with setup-node or assume hosted runners with Node.js[4]. In container jobs using minimal images (e.g., node:slim without full Node tools or no Node), it fails unless Node.js is present[5]. However, with standalone: true, it installs @pnpm/exe (a self-contained executable bundling its own Node.js), enabling pnpm use without system Node.js afterward, but the action's initial execution still requires Node.js[6][7].
Citations:
- 1: https://github.com/pnpm/action-setup/blob/1e1c8eaf/src/install-pnpm/run.ts
- 2: https://git.homeinfra.org/actions/pnpm-action-setup/src/commit/3360b50db7f9e90357ec9065d23478c3899ee826/src/install-pnpm/run.ts
- 3: https://github.com/pnpm/action-setup/tree/v4
- 4: https://pnpm.io/continuous-integration
- 5: pnpm command does not exist actions/setup-node#1165
- 6: https://github.com/pnpm/action-setup/tree/refs/heads/master
- 7: https://github.com/pnpm/action-setup/
Use actions/setup-node instead of dnf install nodejs for a standard, tested setup with pnpm/action-setup@v4.
pnpm/action-setup@v4 requires Node.js on the PATH to execute its installer. While dnf install nodejs should place Node.js on the PATH, this approach is non-standard and undocumented for this action. Official pnpm/action-setup documentation and examples consistently pair it with actions/setup-node for container jobs. The dnf-based setup risks CI failure and is not verified by the pnpm team. Replace lines 57-60 with actions/setup-node@v4 (or later) to follow the tested, standard pattern.
🤖 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/rpm-build.yml around lines 57 - 65, Replace the
non-standard Node installation block that runs "dnf install -y nodejs" and the
subsequent "pnpm/action-setup@v4" usage with the supported setup pairing: call
"actions/setup-node" (e.g., actions/setup-node@v4) to install Node on PATH
before invoking "pnpm/action-setup@v4"; update the workflow steps so the Node
setup step runs prior to the pnpm action and remove the dnf-based installation
to ensure compatibility with pnpm/action-setup@v4.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/functional-tests.yml (1)
437-440:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame
require('@playwright/test/package.json')risk applies to the enterprise job.Identical concern as the community
functional-testsjob (Lines 256–259). Apply the samefs.readFileSyncfix here.🤖 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/functional-tests.yml around lines 437 - 440, The PLAYWRIGHT_VERSION assignment currently uses require('@playwright/test/package.json') which can fail in GitHub Actions; change the shell step that sets PLAYWRIGHT_VERSION to read the package.json file with fs.readFileSync and JSON.parse instead of require — i.e. replace the use of require('@playwright/test/package.json') with code that uses fs.readFileSync to load the module path and JSON.parse to extract the version, then echo "PLAYWRIGHT_VERSION=$PLAYWRIGHT_VERSION" >> "$GITHUB_ENV" as before so the GITHUB_ENV export remains unchanged.
🧹 Nitpick comments (1)
frontend/package.json (1)
86-86: ⚡ Quick win
pnpm/action-setup@v6can read the version directly from thispackageManagerfield.The action supports omitting
versionentirely and will read it from thepackageManagerfield inpackage.json. The field accepts an exact version (e.g.,10.9.8) or a range (e.g.,10,10.x.x,^10.9.8). The workflow currently specifiesversion: 11, which resolves to the latest 11.x.x patch and may diverge from the11.0.8pinned here. Dropping the explicitversioninput from the action would guarantee CI always uses the exact same pnpm version as declared here.🤖 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 `@frontend/package.json` at line 86, The CI workflow is specifying `version: 11` for pnpm/action-setup@v6 which can diverge from the pinned "packageManager": "pnpm@11.0.8" in package.json; remove the explicit `version` input from the GitHub Actions step using pnpm/action-setup@v6 so the action reads the exact pnpm version from the package.json `packageManager` field (leave the action invocation and step name intact, only delete the `version:` key).
🤖 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.
Duplicate comments:
In @.github/workflows/functional-tests.yml:
- Around line 437-440: The PLAYWRIGHT_VERSION assignment currently uses
require('@playwright/test/package.json') which can fail in GitHub Actions;
change the shell step that sets PLAYWRIGHT_VERSION to read the package.json file
with fs.readFileSync and JSON.parse instead of require — i.e. replace the use of
require('@playwright/test/package.json') with code that uses fs.readFileSync to
load the module path and JSON.parse to extract the version, then echo
"PLAYWRIGHT_VERSION=$PLAYWRIGHT_VERSION" >> "$GITHUB_ENV" as before so the
GITHUB_ENV export remains unchanged.
---
Nitpick comments:
In `@frontend/package.json`:
- Line 86: The CI workflow is specifying `version: 11` for pnpm/action-setup@v6
which can diverge from the pinned "packageManager": "pnpm@11.0.8" in
package.json; remove the explicit `version` input from the GitHub Actions step
using pnpm/action-setup@v6 so the action reads the exact pnpm version from the
package.json `packageManager` field (leave the action invocation and step name
intact, only delete the `version:` key).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aed4cb21-26e6-4c85-b136-555401633fa1
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/workflows/frontend-coverage.yaml.github/workflows/frontend-linters.yaml.github/workflows/frontend-unit-tests.yml.github/workflows/functional-tests.yml.github/workflows/rpm-build.yml.github/workflows/startup-tests.ymlfrontend/.npmrcfrontend/package.jsonfrontend/pnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- frontend/.npmrc
✅ Files skipped from review due to trivial changes (2)
- .github/workflows/frontend-coverage.yaml
- frontend/pnpm-workspace.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/startup-tests.yml
- .github/workflows/frontend-unit-tests.yml
- .github/workflows/frontend-linters.yaml
- .github/workflows/rpm-build.yml
1 participant
Summary by CodeRabbit