Repository navigation
ci: drop build-sandbox and the Pages preview leftovers; Storybook is built once per run - #8887
Conversation
…built once per run Sandbox previews come from the exact-head Vercel deployment and the PR comment links them there, so build-sandbox's ~4-minute arm build and its sandbox-<hash> artifact served nothing (no workflow downloads it). The GitHub Pages PR preview is retired; build-storybook's sandbox_url output, a github.io/<repo>/pr/<n>/sandbox URL nothing reads, goes with it. - The build join is build-storybook + typecheck, each required to succeed. - ci-duration's job list drops build-sandbox. - New ci-shared-storybook.test.mjs pins it: no Sandbox build or Pages URL, one Storybook build per pull-request run (build-storybook), and every Storybook consumer downloads that artifact. It fails on main. - ci-tooling-routing and spec-owner-workflow follow the removed job.
|
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.
|
… net-negative The new contract file becomes one case in ci-tooling-routing.test.mjs: no build-sandbox job, no sandbox_url output, and one Storybook build per pull-request run (build-storybook). It still fails on main.
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested: .github/scripts/visual-gate/workflow-concurrency.test.mjs (line 163) still expects ci.yml to upload sandbox-${{ steps.urls.outputs.short_hash }}, so that suite fails on this head and will turn the required test check red. Please drop or invert that expectation, and remove the old build-sandbox header comment left above the build join (ci.yml lines 1300–1303) along with the "Sandbox artifact remains on Pages" note at lines 1135–1137. Worth a line in the description: with build-sandbox gone, a change that breaks only the Sandbox build no longer fails any required check, and only the Vercel deployment would show it.
[Automated review]
| - name: Upload Sandbox artifact | ||
| uses: actions/upload-artifact@v7 | ||
| with: | ||
| name: sandbox-${{ steps.urls.outputs.short_hash }} |
There was a problem hiding this comment.
This upload is still asserted by .github/scripts/visual-gate/workflow-concurrency.test.mjs:163-165 (expect(value).toContain('name: sandbox-${{ steps.urls.outputs.short_hash }}')), so that suite fails once this step is gone. Drop or invert that expectation in the same change.
| # its own job so it overlaps build-storybook instead of serializing after it. | ||
| # PR-only: merge_group runs never deploy previews, and docsite-only PRs skip | ||
| # preview builds entirely. |
There was a problem hiding this comment.
These lines, with line 1300 above them, are the deleted build-sandbox job's header; they now sit directly above the build join and read as describing it. Delete lines 1300–1303.
The workflow-concurrency test asserted the deleted sandbox-<hash> upload; it now asserts CI uploads no Sandbox. The removed job's header comment no longer sits above the build join.
There was a problem hiding this comment.
This looks ready: build still reports on every pull request and fails when build-storybook or typecheck fails, and the visual-gate test now asserts that CI uploads no sandbox- artifact. One small cleanup, before or after merging: the comment in build-storybook's "Write PR comment metadata" step (ci.yml lines 1135–1137) still says the Sandbox artifact remains on Pages, and two comments in scripts/score-ledger.mjs (lines 1241–1242 and 1315–1317) still name build-sandbox. Consider a line in the description noting that a change which breaks only the Sandbox build will now show up on the Vercel deployment rather than on a required check.
[Automated review]
…es Sandbox The PR comment metadata note says both preview links come from Vercel, and score-ledger's two shape-guard comments name the Sandbox build rather than the removed CI job.
There was a problem hiding this comment.
This looks ready to merge: build still reports on every pull request and fails unless build-storybook and typecheck succeed, the contract tests pin the new shape, and the comment cleanup from the previous round is done. Optionally, the description could note that a change which breaks only the Sandbox build will now show up only on the Vercel deployment rather than failing build.
[Automated review]
# Conflicts: # .github/workflows/ci.yml
There was a problem hiding this comment.
This looks ready to merge: build still reports on every pull request and fails unless build-storybook and typecheck succeed, and the contract tests pin the new shape. Two optional touch-ups: the rewritten build join comment still says a merge-queue run or a failed check-scope skips build-storybook, though neither does (see the inline note), and the description could mention that a change breaking only the Sandbox build will now show up on the Vercel deployment rather than failing build.
[Automated review]
| # it fails when build-storybook or typecheck fails. A skipped | ||
| # build-storybook (merge_group, or a failed check-scope) skips this gate | ||
| # too, as the old single-job build did. |
There was a problem hiding this comment.
Optional, and carried over from the old comment: neither case in the parenthetical skips build-storybook. check-scope writes spec_only=false for every non-pull-request event (line 210), so build-storybook and build both run in the merge queue, as the file header says; and when check-scope fails, build-storybook still runs under always() and fails at its classification step (lines 1050-1054), so build runs and fails. The skip cases are spec-only and release-bump pull requests (line 216), for example: A skipped build-storybook (a spec-only or release-bump pull request) skips this gate too.
check-scope reports spec_only=false outside pull requests, so the merge queue runs build-storybook, and a failed check-scope fails it rather than skipping it. The skips are spec-only and release-bump pull requests.
There was a problem hiding this comment.
This looks ready to merge: build still reports on every pull request and fails unless build-storybook and typecheck succeed, the contract tests pin the new shape, and the build join comment now names the cases that really skip it. Worth a line in the description: with build-sandbox gone, a change that breaks only the Sandbox build fails only the Vercel status, which is not a required check, so it no longer blocks merging. The description also still calls the removed job a 2-core arm job, though it has run on a standard ubuntu-24.04-arm runner since #8885.
[Automated review]
Maintainer impact
Every code pull request ran
build-sandbox: a 2-core arm job of about 4 minutes (plus a per-PR Next.js cache) that built the Sandbox and uploaded asandbox-<hash>artifact. No workflow downloads that artifact. Sandbox previews come from the exact-head Vercel deployment, which the PR comment already links, and the GitHub Pages PR preview is retired. Removing the job takes about 4 arm-minutes off every in-scope PR run (about 5,000 a day at recent volume) and one job off thebuildcontext's critical path. Reviewers see no change: the Sandbox preview link stays.Change
build-sandbox. Thebuildjoin now needsbuild-storybookandtypecheck, and fails unless both succeed.build-storybook'ssandbox_urloutput and the step that composed thegithub.io/<repo>/pr/<n>/sandbox/URL. Nothing reads them.build-storybookand shared asstorybook-<hash>.ci-duration's job list dropsbuild-sandbox, and comments are updated.Tests
ci-tooling-routing.test.mjs:build-sandboxjob, nosandbox_urloutput, and exactly one Storybook build that can run on a pull request (build-storybook). It fails on main and passes here.build=build-storybook+typecheck, both required to succeed.spec-owner-workflow.test.mjsnow ends thebuild-storybookslice at the next top-level job instead of atbuild-sandbox:.visual-gate/workflow-concurrency.test.mjsnow asserts CI uploads nosandbox-artifact (it used to require the deleted upload)..github/scriptssuite passes locally: 77 files, 1,424 tests.Lines
+45 / −122, net −77:
ci.yml+11/−105, tests +29/−12,scripts/score-ledger.mjscomments +4/−4 (they named the removed job).Order
This is part of the CI-spend set. It is based on main and rebases onto whatever lands before it. It touches only
build-sandbox,build, andbuild-storybook's outputs and URL step, so it doesn't overlap the heavy-lane conditions orruns-onlabels changed elsewhere.