Repository navigation
ci: PR report links the branch preview at once and measures the base from main's build - #8892
Conversation
…thout deepening history Checking out the PR head (#8812) made pr-scope-touches.sh deepen a shallow history back to the PR's recorded base. On long-lived branches that merge main often, the interaction-scope decide step went from a few seconds to up to three minutes, and one run hit the job's five-minute timeout, which fails the required test join. Both scope jobs now check out the merge commit with its parents and pass its first parent as the base, so the helper diffs the PR's own change against the base it merged into with no deepening. The merge queue keeps its base. A contract test pins the checkout and asserts no deepening on a branch that merged main four times.
…dates Pull request updates keep the fast checks and every required context. The visual, accessibility and RTL owners run for release-check, which Main now dispatches on each push to main (one at a time, skipping a commit main has moved past, failing Main when it fails), and which the release gate already runs before a cut. A pull request opts in with the ci:heavy-lanes label. A main failure is fixed forward. AST-030 FR12 and the new DEC-7 record the change.
One run per workflow and ref at a time, never cancelling a running check, so a burst of pushes is checked once at its newest commit, and Main's fast checks are no longer held behind the heavy wait.
lint.yml classifies a pull request's files with the trusted base classifier and skips full ESLint for a spec-only, docsite-only or tooling-only change. Its depth-1 checkout never let the merge commit reach its merge base, so the classifier was never fed and every pull request ran full lint (about four minutes of ESLint on a spec-only change). The checkout now takes the same 50-commit history ci.yml's check-scope does. A contract test runs the real step and classifier with real git.
The job checked out the whole history on a 4-core runner for every pull request into main, then learned from the branch name that only a release merge-back has anything to validate: about six minutes of larger-runner time per run for nothing. Its condition now names the merge-back branch pattern, so other pull requests skip it outright; the exact branch contract is still checked after the checkout, and required lint still validates merge-backs.
Internal Registry's dependency check runs only when a dependency declaration or the lockfile changes, the CLI smoke test only when a package, the workspace configuration or its own scripts change (its paths-ignore becomes an allowlist), and the hardening-issue opener only when Core component sources change. A registry records every workflow's kind and reason, and a contract test holds each workflow to it: no paths-ignore, no match-all glob, and no path filter on a workflow that reports a required context.
… classifier answered
…rds why (review) Internal Registry's header no longer says to make its check required: behind its path filter it never starts on a pull request that touches no dependency file. Re-enabling gating means dropping the filter and registering it as change-scope. A review-event workflow on pull_request_target now records in the registry why every pull-request update matters to review state, and the contract test requires it.
Removes the scope classifier jobs (check-scope, interaction-scope, native-swift-scope, check-components). Each job that owns tests now runs .github/actions/test-scope right after a sparse checkout: it reads the pull request's file list and the base commit's trusted classifier, and skips the job's remaining steps with success when nothing in its scope changed. Anything it cannot decide puts everything in scope (spec:AST-030 FR3, FR4). native-swift-runner is the one pre-job left, because only a job's runner has to be chosen before it starts. Also: - fixture-contrast's steps run in test-build (same Playwright runner); - registry-contract becomes test-registry-e2e and runs for release-check only; its network-free production gate runs with the unit tests; - ci-duration and the weekly a11y and RTL workflows are removed (the full audits run on main); - the Internal Registry job becomes test-deps, scoped by its trigger paths; - the test join requires release-bump on a release-branch pull request.
The main-tier release-check dispatcher runs on every landed commit, like the Main workflow, so it is a landed-commit workflow.
…from main's build PR Comment no longer waits for Vercel. The Storybook and Sandbox links are the branch's Vercel alias (astryx-git-<branch>-fbopensource.vercel.app, or git-fork-<owner>-<branch> for a fork), which is known before any deployment finishes. A label past 63 characters, which Vercel shortens with a hash, gets no links and points to Vercel's own comment. The resolve-preview and preview-comment jobs, the deployment_status trigger and the Vercel lookup helper are removed. The Main workflow keeps each commit's package builds for 14 days (dists-main-<sha>). size-base measures a PR's merge base from that build when one exists and builds the base only when none does.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested: the branch-alias preview links look sound, but the new kept-build path in size-base breaks the size report it feeds.
- On that path
.size-basegets nopnpm install, so esbuild cannot resolve@stylexjs/stylexand every base import and CSS size, and with them the touch-only budget flag, comes out N/A; install.size-basebefore the download and skip only the builds. - The
dists-main-<sha>lookup accepts any artifact with that name in the repository, including one a fork pull request's own CI run uploads; accept only an artifact from Main's push run onmainfor the merge-base commit. - Draft reports now say the preview address “cannot be known in advance”; pass
--preview-stateonly when an alias was attempted, and reword the two lines that still describe the removed deployment lane.
[Automated review]
| # the other component packages depend on them; another package builds | ||
| # only when the PR changes its source. | ||
| - name: Build the merge base's component packages | ||
| if: (steps.merge-base.outputs.found == 'true') && steps.download.outcome != 'success' && steps.scope.outputs.docsite_only != 'true' && steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.tooling_only != 'true' |
There was a problem hiding this comment.
This step is also the only place .size-base gets pnpm install. When the download succeeds it is skipped, and analyze-pr.js --base-root .size-base bundles each base component with esbuild, which then resolves bare imports from the head checkout's root node_modules, where @stylexjs/stylex is absent. Every base import and CSS size comes out null (Button: 45,543 B with an install, null without), so the report shows N/A and never raises the touch-only flag. Keep pnpm install --frozen-lockfile in .size-base on the kept-build path and skip only the builds.
| const name = `dists-main-${process.env.BASE_SHA}`; | ||
| const {data} = await github.rest.actions.listArtifactsForRepo({...context.repo, name, per_page: 1}); | ||
| const artifact = data.artifacts.find(candidate => !candidate.expired); | ||
| core.setOutput('run_id', artifact ? String(artifact.workflow_run.id) : ''); |
There was a problem hiding this comment.
This takes the first artifact the repository lists as dists-main-<sha>, whatever run uploaded it. A fork pull request's pull_request run executes the pull request's own workflow files and can upload an artifact under that name, which other pull requests' size reports would then measure as main's build. Accept only an artifact whose workflow_run.head_repository_id equals its repository_id, with head_branch main and head_sha equal to the merge base (dropping per_page: 1), or take it from Main's own push run for that commit.
| if (previewState) { | ||
| if (!storybookUrl || !sandboxUrl) | ||
| previewAvailabilitySection = `> **Preview availability:** Storybook and Sandbox are not both ready on the exact-head Vercel preview.${sourceConclusion && sourceConclusion !== 'success' ? ` CI concluded ${sourceConclusion}.` : ''}\n\n`; | ||
| previewAvailabilitySection = `> **Preview availability:** this branch's preview address cannot be known in advance; open the preview from Vercel's comment on this pull request.${sourceConclusion && sourceConclusion !== 'success' ? ` CI concluded ${sourceConclusion}.` : ''}\n\n`; |
There was a problem hiding this comment.
reconcilePrComment passes --preview-state none whenever there is no alias, drafts included, so every draft report now says its preview address "cannot be known in advance". Pass --preview-state only when an alias was attempted (createIfMissing and not a draft), or give drafts their own wording.
…per job Review follow-ups: - The registry contract, ci-duration, the weekly sweeps and the Internal Registry rename move to their own pull request, so this one only removes the scope classifier jobs and folds fixture-contrast into test-build. registry-contract stays a pull-request job here, deciding its own scope. - A ci:heavy-lanes pull request keeps its visual and RTL audits on the affected components (spec:AST-030 FR13): the scope action can also run the base's component-audit-scope.cjs and list the changed files, and the RTL and visual jobs read those instead of a classification job. - The interaction and native Swift scopes include the scope action, so a change to it runs the jobs it gates.
…ocsite-test validates Review follow-ups: - On a ci:heavy-lanes pull request, the accessibility, visual and RTL jobs decide their own component scope and skip their work, downloads included, when it does not apply, so a spec-, docsite- or tooling-only change no longer fails on a missing analysis artifact. The RTL join accepts shards that decided their scope and did nothing. - The scope action reports whether it decided. docsite-test validates spec records when its own scope is spec-only or undecided, so a failed lookup can never pass the required check unvalidated (spec:AST-030 FR4).
…y Main's artifact Review follow-ups: - The merge base's dependencies install on both paths, since measuring bundles each base component with esbuild; only the builds are skipped when Main's kept build is used. - The kept build is accepted only from Main's own run on main for the merge-base commit, never an artifact a pull request's run uploaded under that name. - A draft report says nothing about previews, and the fallback report points to Vercel's own comment instead of an exact-head preview.
There was a problem hiding this comment.
Changes requested for one wording fix: the branch-alias links, the shared preview wording and the kept-build size-base path conform to the CI spec, and the two contract tests an earlier revision removed are back unchanged. The preview note in generate-pr-comment.js and pr-preview.mjs says the links “can trail a new push by a minute or two”, but on recent pull requests Vercel took 7 to 10 minutes from a push to a Ready deployment, which is when the branch alias moves, so please drop or correct that duration in both copies. The description's line count also predates the restored tests (now +275 / −1,278).
[Automated review]
The branch alias moves when a push's deployment is ready, which has taken several minutes; the note and the fallback report now say so without a duration.
…ift run The rename-into-specs case now targets docs/specs/AST-030/spec.md, so it fails if a rename reaches the classifier without its old name, and the native Swift runner job is pinned to run its build outside a pull request (spec:AST-030 FR9).
There was a problem hiding this comment.
No further changes are needed: both copies of the preview note now say the branch-alias links can trail a new push until that push's deployment is ready, which is when Vercel moves the alias, and the rest of the change conforms to the CI spec. One trade-off to keep in mind: links now arrive only with the CI-completion report, so a pull request whose Vercel deployment is ready first gets them later than today's early comment does. The description still says the alias can trail a push “for a minute”; updating it to match the note is optional.
[Automated review]
The a11y guards and specs, the RTL jobs and the visual jobs read the scope step's component-audit outputs; dropping `component-audit: 'true'` from any of them now fails the suite instead of silently skipping its audits (spec:AST-030 FR9).
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewNo new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: The accessibility audit did not run on this pull request. It runs after merges to Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
There was a problem hiding this comment.
No changes needed: the branch-alias preview links, the shared preview wording and the kept-build size-base path conform to the CI spec, and the alias matches the preview URL Vercel itself reports on open pull requests. One trade-off to keep in mind: when a push's Vercel build fails, the alias keeps serving the last ready deployment, so the report's links open an older head until a later push deploys. The description's line count (+275 / −1,278) predates the latest merges of main; it is now +274 / −1,286 if you want to update it.
[Automated review]
Who this helps: reviewers and contributors reading a PR's report. Preview links appear as soon as the report is posted, instead of after Vercel finishes and every route probe passes. Each PR stops running 2 PR Comment jobs on every Vercel deployment event. A PR whose merge base is on main skips the base package builds.
Stacked: on #8919, the last of the four PRs #8888 was split into (#8916 → #8917 → #8918 → #8919). The base moves to
mainonce they land.What changes
Links without waiting. The Storybook and Sandbox links are the branch's Vercel alias:
astryx-git-<branch>-fbopensource.vercel.app, orastryx-git-fork-<owner>-<branch>-…for a fork.Removed:
resolve-previewandpreview-commentjobs and itsdeployment_statustrigger;pr-preview.mjs;.github/scripts/lib/vercel-preview.mjswith its tests. No other workflow uses it.Cached base build. Main's typecheck job already runs
pnpm build; it now keeps the package builds asdists-main-<sha>for 14 days.size-basestill installs the base's dependencies, because measuring bundles each base component with esbuild.mainfor that exact commit, never one a pull request's run uploaded under the same name.A draft's report links no preview and says nothing about one. Skipping the report on drafts entirely is planned in a separate PR.
With no trusted analysis, the fallback report uses the same preview wording: the note above the links, Vercel's comment only when the alias can't be computed, and nothing on a draft.
Line count
+275 / −1,278, net −1,003.
Testing
vitest run --project node .github/scripts: 1,504 pass.