ci: harden Lighthouse budget execution - #1746
Conversation
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:44 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. 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 Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis change hardens Lighthouse execution with shared deadlines, process cleanup, and bounded retries. It adds live Web Vitals input validation, enforces a single Chrome baseline, exports both Chromium path variables, and expands workflow tests and documentation. ChangesLighthouse hardening
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow as live-web-vitals.yml
participant Validator as live-web-vitals-inputs.mjs
participant Runner as run-lighthouse-budget.mjs
participant Lighthouse as Lighthouse process
Workflow->>Validator: Validate origin, routes, samples, and matrix size
Validator-->>Workflow: Export normalized inputs and timeout settings
Workflow->>Runner: Run bounded Lighthouse measurement cells
Runner->>Lighthouse: Start each cell with remaining suite timeout
Lighthouse-->>Runner: Return result or failure
Runner-->>Workflow: Preserve results and warnings until suite deadline
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:cb2e7622e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- run-lighthouse-budget: spawn Lighthouse in an owned process group and stopOwnedProcessTree on per-cell timeout instead of spawnSync timeout (which only stopped the npx wrapper). - live-web-vitals: export normalized ROUTES/SAMPLES to GITHUB_ENV; cap the measurement loop with a 32-minute suite deadline inside the 45-minute job. - Update contract tests for spawn-based measurement and whitespace samples.
The workflow now uses a dynamic run_timeout under the suite deadline; update the focused CI contract test to match.
Conflict on lighthouse-budget.json: took main's fresher baseline numbers and updatedAt; preserved this PR's runner hardening (time budget, process-tree kill, live-web-vitals inputs, chrome-version uniqueness check, perf scope).
…resume) Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Resolve docs/scripts-index.md count-line conflict via docs:update after absorbing main. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/run-lighthouse-budget.mjs (1)
143-155: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winEnforce a wall-clock timeout for each readiness request.
http.get(..., { timeout })only limits socket inactivity. Periodic response data can keepget()pending for 242 ms with a 100 ms timeout, sowaitForServercan exceed its deadline. Add a timer that destroys the request, resolves once, and clears on every terminal path.🤖 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 `@scripts/run-lighthouse-budget.mjs` around lines 143 - 155, Update get(url, timeoutMs) to enforce a wall-clock timeout using a timer that destroys the request and resolves with null when the deadline expires. Ensure resolution occurs only once, and clear the timer on response completion, request timeout, and request error while preserving the existing successful-response behavior.Source: MCP tools
🤖 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/live-web-vitals.yml:
- Around line 137-155: Update the deadline calculation around run_timeout to
reserve the timeout command’s 10-second kill grace: define or reuse a kill-grace
value, stop the suite when remaining time is less than or equal to that grace,
and cap run_timeout at remaining minus the grace period before invoking
Lighthouse. Preserve the existing suite_expired handling and warning behavior.
In `@docs/testing.md`:
- Around line 200-204: Update the provider-access statement in the live workflow
documentation to clarify that neither workflow requires application secrets or
authenticated application-provider access. Remove the broader claim that neither
uses providers while preserving the surrounding descriptions of GitHub Actions,
npm, and deployed-origin access.
In `@scripts/run-lighthouse-budget.mjs`:
- Around line 401-403: Update the timeout handling around
stopOwnedProcessTree(child) to schedule a short-grace-period fallback that sends
SIGKILL to the same process group if the child remains alive after SIGTERM.
Track and clear both the existing termination timer and the force-kill timer
when the child closes, preserving the current Windows behavior and process-group
ownership logic.
---
Outside diff comments:
In `@scripts/run-lighthouse-budget.mjs`:
- Around line 143-155: Update get(url, timeoutMs) to enforce a wall-clock
timeout using a timer that destroys the request and resolves with null when the
deadline expires. Ensure resolution occurs only once, and clear the timer on
response completion, request timeout, and request error while preserving the
existing successful-response behavior.
🪄 Autofix
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
Run ID: 89a9e8cd-cf05-45cf-a5fe-6e772a992b90
📒 Files selected for processing (14)
.github/actions/setup-lighthouse-chromium/action.yml.github/workflows/ci.yml.github/workflows/live-web-vitals.ymldocs/branch-review-ledger.mddocs/scripts-index.mddocs/testing.mdpackage.jsonscripts/ci-change-scope.mjsscripts/lighthouse-time-budget.mjsscripts/live-web-vitals-inputs.mjsscripts/run-lighthouse-budget.mjstests/check-lighthouse-budget.test.tstests/ci-cache-safety.test.tstests/live-web-vitals-inputs.test.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Summary
CHROME_PATHand Playwright's executable path, expand perf-scope coverage for runner dependencies, and guard a mixed or empty browser baseline refresh.Verification
npm run test:ci-workflows— 13 files passed; 248 passed, 11 skipped.npm run check:ci-scope,npm run check:github-actions,npm run check:branch-review-ledger,npm run verify:lighthouse -- --dry-run, and formatting checks.npm run verify:pr-localreached its full unit suite but exited 1 only on 12 pre-existing Windows host assumptions: 11pr-handoff-stopfailures require Unixln, and onemode-nav-addon-slotfailure expects POSIX relative paths. The exact two suites reproduce the same 12 failures from cleanorigin/mainateda8fe872de040e304621bce49535e1dfebb091e.Risk and rollout
cb2e7622e57e712ea155f1ad93adcf957384f483to restore the previous runner and workflow behavior.Notes
eda8fe872de040e304621bce49535e1dfebb091e; no retrieval, ranking, clinical output, or provider-facing application behavior changed.Summary by CodeRabbit
Bug Fixes
Documentation
Tests