[Draft] Improve PR builder - #4340
Conversation
ThaminduDilshan
added
skip-changelog
ThaminduDilshan
added
trigger-pr-builder
📝 WalkthroughWalkthroughCI coverage now runs backend unit tests through ChangesCI and test tooling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant PlaywrightContainer
participant BackendServer
participant SampleApp
participant Playwright
GitHubActions->>PlaywrightContainer: Start containerized E2E job
PlaywrightContainer->>BackendServer: Launch start.sh with setsid
PlaywrightContainer->>SampleApp: Launch npm run dev with setsid
PlaywrightContainer->>Playwright: Run browser tests with configured workers
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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: 2
🧹 Nitpick comments (1)
.github/workflows/pr-builder.yml (1)
382-388: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider caching the pnpm store now that
setup-pnpmis bypassed.Both
test-frontend-packagesandtest-e2enow enable pnpm via rawcorepackinstead of./.github/actions/setup-pnpm, which likely provided pnpm-store caching. Without an equivalentactions/cachestep for the pnpm store, every run reinstalls the full frontend dependency tree from the registry, adding CI time across two jobs.♻️ Example caching step (add after enabling pnpm, before install)
+ - name: 🗄️ Get pnpm Store Path + id: pnpm-cache + run: echo "STORE_PATH=$(pnpm store path --silent)" >> "$GITHUB_OUTPUT" + + - name: 🗄️ Cache pnpm Store + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: ${{ steps.pnpm-cache.outputs.STORE_PATH }} + key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }}Also applies to: 767-773
🤖 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/pr-builder.yml around lines 382 - 388, Add an actions/cache step after the “📦 Enable pnpm via corepack” step in both the test-frontend-packages and test-e2e job flows, caching the pnpm store used by pnpm install and keying it by the runner OS plus the lockfile hash; ensure the cache is restored before dependency installation.
🤖 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/pr-builder.yml:
- Around line 263-264: Update all three new actions/checkout steps in the
test-backend-unit, test-frontend-packages, and test-e2e jobs to set
persist-credentials to false, matching the existing patch-coverage-gate checkout
configuration.
- Around line 351-353: Ensure the workflow’s installed `@playwright/test`
dependency resolves to version 1.60.0, matching the Playwright Docker image
v1.60.0-noble. Update any latest or mismatched package or lockfile version
references used by the workflow while preserving the existing container
configuration.
---
Nitpick comments:
In @.github/workflows/pr-builder.yml:
- Around line 382-388: Add an actions/cache step after the “📦 Enable pnpm via
corepack” step in both the test-frontend-packages and test-e2e job flows,
caching the pnpm store used by pnpm install and keying it by the runner OS plus
the lockfile hash; ensure the cache is restored before dependency installation.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f7dbfe9-b34e-4e9e-9b38-aaeb5327a863
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.github/actions/setup-playwright/action.yml.github/workflows/pr-builder.yml.github/workflows/warm-playwright-cache.ymlfrontend/apps/console/package.jsonfrontend/apps/console/vite.config.tsfrontend/apps/gate/package.jsonfrontend/apps/gate/vite.config.tspnpm-workspace.yamltests/e2e/playwright.config.tsturbo.json
💤 Files with no reviewable changes (2)
- .github/actions/setup-playwright/action.yml
- .github/workflows/warm-playwright-cache.yml
| - name: 📥 Checkout Code | ||
| uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Set persist-credentials: false on the new checkout steps.
zizmor flags credential persistence risk on all three new actions/checkout steps (test-backend-unit, test-frontend-packages, test-e2e). Each of these jobs subsequently runs untrusted package installs (go mod download, pnpm install, npm install), so a compromised dependency's postinstall script could read the persisted token from .git/config. The existing patch-coverage-gate job (lines 570-574) already follows the safer pattern.
🔒 Proposed fix (repeat for all three checkout steps)
- name: 📥 Checkout Code
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
+ with:
+ persist-credentials: falseAlso applies to: 379-380, 764-765
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 263-264: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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/pr-builder.yml around lines 263 - 264, Update all three
new actions/checkout steps in the test-backend-unit, test-frontend-packages, and
test-e2e jobs to set persist-credentials to false, matching the existing
patch-coverage-gate checkout configuration.
Source: Linters/SAST tools
| container: | ||
| image: mcr.microsoft.com/playwright:v1.60.0-noble | ||
| options: --user root |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the pinned `@playwright/test` version against the container image tag (v1.60.0).
rg -n '"`@playwright/test`"' -g '*package.json'
rg -n '`@playwright/test`' pnpm-lock.yaml | head -5Repository: thunder-id/thunderid
Length of output: 2058
🌐 Web query:
Playwright Docker image version should match @playwright/test version unable to locate browser executables
💡 Result:
The error "unable to locate browser executables" when running Playwright in Docker is almost always caused by a version mismatch between the Playwright library installed in your project and the version of the Playwright Docker image you are using [1][2][3]. The official Playwright Docker images come pre-installed with the specific browser versions required for that exact release of Playwright [4][5][6]. If your project installs a different version (e.g., via npm or yarn during the build process), Playwright will look for browsers in a directory path that corresponds to the version in your project, which does not exist in the Docker container [7][5]. To resolve and prevent this issue, you must keep the versions synchronized [7][3]. Recommended Solutions: 1. Exact Version Pinning (Best Practice) Pin both your project dependencies and your Docker image to the exact same version [7][3]. In your Dockerfile, use the specific tag matching your project's Playwright version: FROM mcr.microsoft.com/playwright:v1.61.0-noble In your package.json, ensure your dependency is pinned without carets (^) or tildes (~): "@playwright/test": "1.61.0" This prevents your CI environment from accidentally installing a newer version of Playwright while the Docker image remains on an older one [7][3]. 2. Use 'npx playwright install' (Alternative) If you do not want to manage Docker image tags manually, you can use a generic base image (e.g., node:20-bookworm) and install the browsers during your build or CI process [1][7]. This ensures the browsers always match the installed library version: RUN npx -y playwright@1.61.0 install --with-deps Note that this method is typically slower than using the official pre-built image because it must download browser binaries on every build [3]. Important Troubleshooting Tips: - Check for Drift: If you have a package-lock.json or yarn.lock, ensure you are running npm ci or yarn install to guarantee the exact versions are used [6]. - Check User Permissions: If you run your container as a non-root user, ensure that user has proper access to the browser installation directory, or set the PLAYWRIGHT_BROWSERS_PATH environment variable to a directory where the user has read/write permissions [8][9]. - Avoid 'latest': The 'latest' tag for Docker images is not recommended because the Playwright library and Docker image releases are not always synchronized, which leads to this exact error [7].
Citations:
- 1: https://playwright.dev/docs/docker
- 2: https://hub.docker.com/r/microsoft/playwright
- 3: https://scrolltest.com/playwright-docker-github-actions-ci-cd-pipeline/
- 4: https://github.com/microsoft/playwright/blob/main/docs/src/docker.md
- 5: [Question] Executable doesn't exist at /ms-playwright/chromium-1005/chrome-linux/chrome microsoft/playwright#15079
- 6: [Question] Troubleshoot matching Playwright version and Docker image complaining about browser mismatch. microsoft/playwright#17535
- 7: [Feature]: Hardcoded image version in yaml file microsoft/playwright#36449
- 8: [Bug]: Running Playwright in NextJS Docker microsoft/playwright#31746
- 9: [Docs]: Clarify the use of a custom install path for browsers in Docker image microsoft/playwright#31316
🌐 Web query:
playwright mcr.microsoft.com/playwright v1.60.0-noble @playwright/test 1.60.0 package
💡 Result:
Playwright version 1.60.0 was released on May 11, 2026 [1][2]. The artifact mcr.microsoft.com/playwright:v1.60.0-noble refers to a Docker image containing the Playwright environment based on Ubuntu 24.04 LTS (Noble Numbat) for version 1.60.0 [3][4]. Playwright publishes Docker images tagged with specific versions and operating systems (e.g., -noble for Ubuntu 24.04, -jammy for Ubuntu 22.04) to provide pre-configured environments with the necessary browser system dependencies [4][5]. It is recommended to pin your Docker image to a specific version to ensure consistency between your project dependencies and the environment [4][5]. The package @playwright/test version 1.60.0 is the corresponding test runner release published to npm on the same date [6]. This version introduced several new features, including: - HAR recording as a first-class tracing API (using tracing.startHar and tracing.stopHar) [1][7]. - A new Drop API (locator.drop) for simulating drag-and-drop actions [1][7]. - Enhancements to Aria snapshots, including support on Page objects and a new boxes option for AI-friendly bounding box output [1][7]. - The test.abort method for immediate test termination [1][7]. For project setup, it is recommended to manage the @playwright/test dependency via npm or other package managers and use npx playwright install to download the required browser binaries [8][9].
Citations:
- 1: https://github.com/microsoft/playwright/releases/tag/v1.60.0
- 2: https://npmx.dev/package-changelog/@playwright/test/v/1.60.0
- 3: https://mcr.microsoft.com/en-us/artifact/mar/playwright/tag/v1.60.0-noble
- 4: https://playwright.dev/docs/docker
- 5: https://github.com/microsoft/playwright/blob/main/docs/src/docker.md
- 6: https://newreleases.io/project/npm/@playwright/test/release/1.60.0
- 7: https://playwright.dev/docs/next/release-notes
- 8: https://www.npmjs.com/package/@playwright/test
- 9: https://playwright.dev/docs/release-notes
Confirm the installed @playwright/test version matches v1.60.0-noble.
The pinned Docker image is the Playwright 1.60.0 Noble image, and the corresponding @playwright/test release is 1.60.0. If the workflow installs latest or a lockfile/version specifier resolves to a different version, keep the Docker image and package version in sync to avoid browser-executable lookup failures.
🤖 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/pr-builder.yml around lines 351 - 353, Ensure the
workflow’s installed `@playwright/test` dependency resolves to version 1.60.0,
matching the Playwright Docker image v1.60.0-noble. Update any latest or
mismatched package or lockfile version references used by the workflow while
preserving the existing container configuration.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Purpose
Approach
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
Testing Improvements
Coverage