Skip to content

Improve pr builder - #4319

Merged
rajithacharith merged 1 commit into
thunder-id:mainfrom
ThaminduDilshan:thamindu-fix-2
Jul 24, 2026
Merged

Improve pr builder#4319
rajithacharith merged 1 commit into
thunder-id:mainfrom
ThaminduDilshan:thamindu-fix-2

Conversation

@ThaminduDilshan

@ThaminduDilshan ThaminduDilshan commented Jul 24, 2026

Copy link
Copy Markdown
Member

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:

  • Added a cancel-superseded job in .github/workflows/pr-builder.yml to 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.
  • Improved the concurrency group key in .github/workflows/pr-builder.yml to include the event name, making it more robust against different event types and reducing accidental concurrency collisions.

Playwright Browser Caching:

  • Updated .github/actions/setup-playwright/action.yml to 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:

  • Refined the if conditions for most jobs in .github/workflows/pr-builder.yml to ensure jobs only run when appropriate, especially when the trigger-pr-builder label 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

  • N/A

Related PRs

  • N/A

Checklist

  • Followed the contribution guidelines.
  • Manual test round performed and verified.
  • Documentation provided. (Add links if there are any)
    • Ran Vale and fixed all errors and warnings
  • Tests provided. (Add links if there are any)
    • Unit Tests
    • Integration Tests
  • Breaking changes. (Fill if applicable)
    • Breaking changes section filled.
    • breaking change label added.

Security checks

  • Followed secure coding standards in WSO2 Secure Coding Guidelines
  • Confirmed that this PR doesn't commit any keys, passwords, tokens, usernames, or other secrets.

Summary by CodeRabbit

  • Chores

    • Improved Playwright setup by caching and installing browsers based on the resolved Playwright version, reducing repeat build time while keeping browser dependencies available.
    • Tightened pull request validation so checks run only when explicitly triggered by the expected label.
    • Added automatic cancellation of superseded in-progress validation runs to avoid redundant CI work.
    • Applied consistent trigger handling across multiple validation and cleanup jobs.
  • Security

    • Removed local passkey allowed_origins for https://localhost:8090 configuration.

@ThaminduDilshan ThaminduDilshan added skip-changelog Skip generating changelog for a particular PR trigger-pr-builder Add when the PR is ready for CI; starts the PR Builder for this and all later pushes and removed trigger-pr-builder Add when the PR is ready for CI; starts the PR Builder for this and all later pushes labels Jul 24, 2026
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@ThaminduDilshan, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 868fa4ef-9094-4754-9735-fc602de4829f

📥 Commits

Reviewing files that changed from the base of the PR and between 278012b and da25e34.

📒 Files selected for processing (3)
  • .github/actions/setup-playwright/action.yml
  • .github/workflows/pr-builder.yml
  • .github/workflows/pr-label-check.yml
📝 Walkthrough

Walkthrough

Changes

Playwright browser caching

Layer / File(s) Summary
Versioned browser cache and installation
.github/actions/setup-playwright/action.yml
Resolves the Playwright version, keys browser caching by operating system, version, and scope, and separates cache-miss browser installation from cache-hit dependency installation.

PR Builder workflow controls

Layer / File(s) Summary
Run grouping and superseded-run cancellation
.github/workflows/pr-builder.yml
Documents concurrency behavior and adds cancellation of older active workflow runs through the GitHub API.
Trigger-label job gating
.github/workflows/pr-builder.yml
Tightens trigger-label and labeled-event conditions across security, dependency, build, test, detection, integration, and cleanup jobs.

Server deployment configuration

Layer / File(s) Summary
Passkey origin configuration removal
backend/cmd/server/deployment.yaml
Removes the localhost passkey allowed_origins block.

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
Loading

Possibly related PRs

Suggested reviewers: brionmario

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is related to the PR, but it is too generic to tell what part of the pr-builder workflow changed. Rename it to mention the main change, such as workflow concurrency, Playwright caching, or trigger conditions.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description follows the template well, covering purpose, approach, related links, checklist, and security checks.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Guard merge_group before reading pull_request fields.

cleanup-turbo-cache is triggered by merge_group/workflow_dispatch, but its if has no github.event_name guard. Since github.event.pull_request is 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

📥 Commits

Reviewing files that changed from the base of the PR and between 189e635 and 9ff1851.

📒 Files selected for processing (2)
  • .github/actions/setup-playwright/action.yml
  • .github/workflows/pr-builder.yml

Comment thread .github/actions/setup-playwright/action.yml Outdated
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' }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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))"
done

Repository: 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}")
PY

Repository: 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.

@ThaminduDilshan
ThaminduDilshan force-pushed the thamindu-fix-2 branch 2 times, most recently from 7bd7c5c to 1899a08 Compare July 24, 2026 06:44
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@ThaminduDilshan ThaminduDilshan removed the trigger-pr-builder Add when the PR is ready for CI; starts the PR Builder for this and all later pushes label Jul 24, 2026
@ThaminduDilshan ThaminduDilshan added the trigger-pr-builder Add when the PR is ready for CI; starts the PR Builder for this and all later pushes label Jul 24, 2026
@rajithacharith
rajithacharith enabled auto-merge July 24, 2026 07:24
@rajithacharith
rajithacharith added this pull request to the merge queue Jul 24, 2026
Merged via the queue into thunder-id:main with commit 9f130ae Jul 24, 2026
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Skip generating changelog for a particular PR trigger-pr-builder Add when the PR is ready for CI; starts the PR Builder for this and all later pushes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants