Feature/vercel preview performance check - #179
Merged
Merged
Conversation
Adds an opt-in Lighthouse check that measures Core Web Vitals against the preview deployment and comments the median results on the pull request. The check is off by default and runs only when `performance-check` is true and `measured-paths` is non-empty, so existing callers are unaffected. The change is purely additive: no existing line is modified, and the deploy job gains only an `outputs.url` so the new job can consume the preview URL. The caller supplies the measured paths and a Lighthouse CI config holding run count and budgets. Documented with a worked example, including why the config should set `aggregationMethod: median` (LHCI defaults to `optimistic`, which takes the best run) and why the budgets should be advisory on a cold preview. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The secret was named in the inputs table and the example, but nothing said where to obtain it, how to tell whether a project needs it, or what happens without it. Someone hitting a 401 during warm-up had no path from the error to the fix. Adds a Deployment Protection section covering how to generate the secret, the permissions required, the redeploy caveat when rotating it, and how to check whether a project has protection enabled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The check was an input on the deploy workflow, which meant it could only ever measure a deployment that workflow made, and its cadence was fixed by whatever triggered the deploy. As a separate workflow taking a deployment URL, the caller decides what to measure and when: pair it with a deploy via `needs` on pull requests, call it again on push to the default branch to record a baseline, or point it at a second project. None of that is expressible as an input. vercel-preview.yml goes back to deploying only, losing six inputs and one secret. The two baseline booleans collapse into a `baseline-mode` enum, so the invalid "record and compare" combination is no longer representable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Baseline caches were keyed only by form factor, so a repository measuring two deployments would have them overwrite each other and pull requests would compare against whichever recorded last. A `baseline-key` input now namespaces them; it defaults to "default", so existing single-deployment callers are unaffected. Adds vercel-performance-keepalive.yml. GitHub evicts a cache that has not been read for 7 days, and the baseline is only rewritten on a merge, so a quiet fortnight would drop it silently. Reading a cache resets that clock, so the workflow restores the baselines and does nothing else, costing seconds rather than the minutes a re-measurement would. It expands the caller's baseline keys across the form factors this repository measures, so callers do not duplicate that list. A full restore is used rather than `lookup-only`, because whether a metadata-only lookup resets the eviction clock is not documented, and the payload is a few hundred bytes either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deploying several projects from one workflow had them share a group, so cancel-in-progress made each call cancel the previous one.
The group used deployment-url, but that is only known once the deploy the call depends on has finished, while the group is evaluated when the call is queued. Every caller therefore shared an empty group and cancelled each other.
The deploy job set an output, but a reusable workflow only exposes what it declares in on.workflow_call.outputs, so callers always read an empty string.
crispy101
requested review from
AdamJHall,
aaronmedina-dev and
tmthrgd-aligent
August 26, 2026 04:59
Two deployments measured in one pull request shared a comment marker, so whichever finished last overwrote the other's results.
…w-performance-check # Conflicts: # .github/workflows/vercel-preview.yml # docs/vercel-preview.md
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
actions/cache was tag-pinned, which the blanket policy rejects. setup-node was hash-pinned to an unrelated Dependabot commit rather than to v6.0.0, and the lighthouse-ci-action comment named a v12.6.1 tag that does not exist. Both now use the versions already referenced elsewhere in this repo.
…w-performance-check
The two form factors run as independent matrix jobs, so each posted its own comment and the two had to be read side by side to compare them. Join them into one table instead, with a Median/vs baseline/status column group per form factor. The measurement job now writes its medians, baseline and warnings to summary.json and uploads it, rather than rendering markdown itself. A new `comment` job downloads both summaries and renders them together. Keeping rendering out of the matrix is what makes the join possible; the measurement jobs stay parallel and independent, and `always()` on the join means a partial result is still reported when one form factor fails. GitHub-flavoured Markdown has no colspan, so each column is labelled with its form factor icon rather than grouped under a merged Desktop/Mobile heading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The job read node-version-file without checking the calling repository out, so setup-node failed with ".nvmrc does not exist" and no comment was posted. It only runs a dependency-free script, so use the Node already on the runner rather than adding a checkout for one file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three faults let a protected deployment be measured and reported as healthy. Deployment Protection answers with a 200 from Vercel's SSO login page, so warm-up accepted it and Lighthouse measured the login page for every path, at around 40/100. Probe the deployment once without the bypass header before warming up: if it redirects off its own host, protection is on and the secret is required, so fail with that reason rather than measuring the wrong page. Projects without protection are unaffected. Warm-up now also checks the final URL per request, which catches a secret that is present but rejected. The warning lookup never matched, so every metric showed a tick however far over budget it was. A category assertion arrives as auditId "categories" with the category in auditProperty, not as "categories:performance"; normalise both shapes, and ignore assertions that passed. Add a Target column per form factor, and the score target to the score line. Budgets come from the Lighthouse config rather than the assertion results: an audit that passes on every run produces no assertion entry, so its budget would otherwise be missing from the table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Warm-up sends the bypass as a request header, which authenticates curl but not Lighthouse: that drives a real browser, so a protected deployment redirected it to the login page and it measured that instead, scoring around 40. Both the probe and warm-up passed throughout, because both only prove curl can reach the site. Vercel accepts the same bypass as query parameters, which do survive a browser navigation, so append them to each measured URL along with x-vercel-set-bypass-cookie so subsequent navigations and subresources stay authenticated. Paths that already carry a query string get "&" rather than "?". toLabel keeps dropping the query, so the secret reaches neither the comment nor a baseline key; say so there, since that is now load-bearing rather than incidental. Also stop the comment job failing when both form factors failed before uploading anything. They have already reported why and turned the run red, so report that on the pull request instead of failing a second time with nothing to show. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Passing the bypass as a query parameter put the secret in every measured URL, and those URLs are written into the Lighthouse reports uploaded as artifacts, so the secret was stored in plaintext for anyone with read access to the calling repository. Give Chrome the bypass through collect.settings.extraHeaders instead. The caller's config is committed, so the header cannot be written there; copy the config to RUNNER_TEMP at resolve time and add the header to the copy. Writing it outside the workspace keeps a caller's dirty-tree check clean, and the copy preserves the assertions the summariser reads budgets from. Projects without Deployment Protection pass no secret and keep using their own config untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
extraHeaders kept the secret out of the measured URLs, and so out of the manifest, the comment and the baseline keys, but Lighthouse records the settings it ran with into every report, so the header still arrived in the results verbatim and was uploaded as an artifact. The action uploads from inside its own step, leaving no point at which to redact, so turn uploadArtifacts off and upload from the workflow instead, with a redaction step in between. That step searches every file rather than a list of extensions, replaces the secret literally, and fails rather than upload if the secret survives. Uploading here also gives the artifact a name scoped by baseline-key, so a repository measuring two deployments no longer has both upload "lighthouse-reports-desktop" in the same run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The upload step reported success but produced no artifact, so the reports are missing with nothing in the log explaining why. List the directory after redaction, and mark the upload if-no-files-found: warn with an explicit trailing slash on the path, so an empty result is visible rather than silent. The reports are a debugging aid rather than a result, so a missing one should not fail a measurement that otherwise succeeded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Redact with find -exec sed rather than a per-file Python loop. The loop re-read what grep had already found, and a Vercel bypass secret is alphanumeric so it needs no escaping to be used as a pattern. The check that follows still fails the step if anything survives, which covers the case where that stops being true. - Build the Lighthouse config with jq instead of Node. jq creates the missing parents, so one expression replaces thirteen lines. - Use case() for BASELINE_PATH, matching aws-cdk.yml and avoiding the && / || form's behaviour on falsy values. - tee the rendered summary rather than redirecting and then cat-ing it. pipefail is already set, so a failing render still fails the step. - Build the keepalive JSON with jq --argjson rather than piping the accumulated value back through printf. The loop itself stays: it reads as "for each key, for each form factor", which a single cartesian-product expression would not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@tmthrgd-aligent @aaronmedina-dev Thanks for the review. I was just being lazy. I've updated as advised. |
tmthrgd-aligent
approved these changes
Sep 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of the proposed changes
add workflows for:
Screenshots (if applicable)
Other solutions considered (if any)
Notes to PR author
Notes to reviewers
ℹ️ When you've finished leaving feedback, please add a final comment to the PR tagging the author, letting them know that you have finished leaving feedback
Time Tracking
ABC-xxx: code review, select project XXXX