Improve pr builder - #4319
Conversation
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughChangesPlaywright browser caching
PR Builder workflow controls
Server deployment configuration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant PR as Pull request event
participant Workflow as pr-builder workflow
participant API as GitHub Actions API
PR->>Workflow: Start workflow run
Workflow->>API: List active superseded runs
API-->>Workflow: Return matching runs
Workflow->>API: Cancel older runs
Workflow-->>PR: Continue label-gated jobs
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
9ff1851 to
278012b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pr-builder.yml (1)
1202-1206: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGuard
merge_groupbefore readingpull_requestfields.
cleanup-turbo-cacheis triggered bymerge_group/workflow_dispatch, but itsifhas nogithub.event_nameguard. Sincegithub.event.pull_requestis only populated for pull-request events, this condition evaluates as false and skips the dedicated cleanup pass for merge-queue Turbo caches.🗑️ Proposed fix
- if: always() && github.event.pull_request.head.repo.full_name == github.repository && contains(github.event.pull_request.labels.*.name, 'trigger-pr-builder') && (github.event.action != 'labeled' || github.event.label.name == 'trigger-pr-builder') + if: always() && (github.event_name == 'merge_group' || (github.event.pull_request.head.repo.full_name == github.repository && contains(github.event.pull_request.labels.*.name, 'trigger-pr-builder') && (github.event.action != 'labeled' || github.event.label.name == 'trigger-pr-builder')))🤖 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 1202 - 1206, Update the cleanup-turbo-cache job condition to explicitly allow merge_group events before accessing pull_request fields, while preserving the existing pull-request label, repository, and action checks for pull_request-triggered runs. Ensure merge_group cleanup is not skipped because github.event.pull_request is unavailable.
🤖 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/actions/setup-playwright/action.yml:
- Line 39: Update the Playwright cache key near the existing version and scope
components to include a normalized identity for inputs.browsers and the relevant
browser configuration, plus runner.arch. Ensure cache hits cannot reuse browser
binaries across different browser selections or architectures while preserving
the existing cache behavior.
In @.github/workflows/pr-builder.yml:
- Line 93: Update the workflow dispatch conditions for detect-code-changes,
detect-powershell-changes, and detect-docs-changes to include github.event_name
== 'workflow_dispatch', ensuring manual runs execute these detection jobs and
produce outputs for build, frontend tests, build_samples, and
test-integration-status.
---
Outside diff comments:
In @.github/workflows/pr-builder.yml:
- Around line 1202-1206: Update the cleanup-turbo-cache job condition to
explicitly allow merge_group events before accessing pull_request fields, while
preserving the existing pull-request label, repository, and action checks for
pull_request-triggered runs. Ensure merge_group cleanup is not skipped because
github.event.pull_request is unavailable.
🪄 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: 65ef3b54-db2a-4551-835e-ff15fea1e287
📒 Files selected for processing (2)
.github/actions/setup-playwright/action.yml.github/workflows/pr-builder.yml
| security-audit: | ||
| name: 🔒 Security Audit | ||
| if: ${{ (github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'trigger-pr-builder')) || github.event_name == 'merge_group' }} | ||
| if: ${{ (github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'trigger-pr-builder') && (github.event.action != 'labeled' || github.event.label.name == 'trigger-pr-builder')) || github.event_name == 'merge_group' }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow files =="
git ls-files .github/workflows | sed -n '1,120p'
echo "== target file size =="
wc -l .github/workflows/pr-builder.yml
echo "== search relevant conditions and workflow_name =="
rg -n "workflow_dispatch|pull_request|merge_group|detection|security-audit|dependency-review|verify-mocks|lint|detect-code-changes|detect-powershell-changes|detect-docs-changes|build_samples|test-integration-status" .github/workflows/pr-builder.yml
echo "== sections around referenced lines =="
for line in 88 108 153 189 330 366 404 453 581 722 1123 1148 233 370 408 457 585 726; do
echo "--- $((line-3)) - $((line+3)) ---"
sed -n "$((line-3)),$((line+3))p" .github/workflows/pr-builder.yml | nl -ba -v "$((line-3))"
doneRepository: thunder-id/thunderid
Length of output: 9687
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('.github/workflows/pr-builder.yml')
text = p.read_text()
lines = text.splitlines()
targets = {
"security-audit": 93,
"dependency-review": 114,
"verify-mocks": 158,
"lint": 194,
"detect-code-changes": 335,
"build": 238,
"test-frontend-packages": 371,
"test-frontend-gate-app": 409,
"test-frontend-console-app": 458,
"build_samples": 586,
"detect-powershell-changes": 1128,
"detect-docs-changes": 1153,
}
print("line\tjob\tcontains workflow_dispatch")
for job, line in targets.items():
cond = lines[line-1]
print(f"{line}\t{job}\t{'workflow_dispatch' in cond}\n{cond}")
PYRepository: thunder-id/thunderid
Length of output: 4393
Make workflow_dispatch jobs depend on the workflow-dispatched detection jobs.
workflow_dispatch is excluded from the gated detection jobs (detect-code-changes, detect-powershell-changes, detect-docs-changes), so build, frontend tests, build_samples, and test-integration-status skip on manual dispatch because their detect-* needs evaluate to skipped/fail rather than producing the downstream test jobs. Add github.event_name == 'workflow_dispatch' to the changed-file detection jobs, or set their outputs unconditionally for direct/manual dispatches before the downstream jobs run.
🤖 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 at line 93, Update the workflow dispatch
conditions for detect-code-changes, detect-powershell-changes, and
detect-docs-changes to include github.event_name == 'workflow_dispatch',
ensuring manual runs execute these detection jobs and produce outputs for build,
frontend tests, build_samples, and test-integration-status.
7bd7c5c to
1899a08
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1899a08 to
da25e34
Compare
Purpose
This pull request introduces several improvements to the GitHub Actions workflows, primarily focusing on more robust and precise handling of workflow concurrency, caching, and job triggering conditions. The main highlights are the addition of a job to actively cancel superseded workflow runs, enhanced Playwright browser caching keyed by version, and stricter job execution conditions to avoid unnecessary or duplicate runs.
Approach
Workflow Concurrency Management:
cancel-supersededjob in.github/workflows/pr-builder.ymlto actively cancel older, still-running workflow runs for the same PR using the GitHub REST API, ensuring that only the latest workflow run is active and preventing resource contention.concurrencygroup key in.github/workflows/pr-builder.ymlto include the event name, making it more robust against different event types and reducing accidental concurrency collisions.Playwright Browser Caching:
.github/actions/setup-playwright/action.ymlto resolve and cache Playwright browsers based on the exact Playwright version, improving cache hit rates and reliability. The workflow now restores the cache before installing browsers and system dependencies. [1] [2]Job Triggering and Filtering:
ifconditions for most jobs in.github/workflows/pr-builder.ymlto ensure jobs only run when appropriate, especially when thetrigger-pr-builderlabel is added or removed, preventing unnecessary runs on label-only events. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13]Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
Chores
Security
allowed_originsforhttps://localhost:8090configuration.