-
Notifications
You must be signed in to change notification settings - Fork 0
Review 5482
rubyycheung · DESIGNOWNERS · #5482 · MERGED 2026-08-25 18:13
This PR fixes a defect this loop shipped and reported as done.
#5477 (ours, merged 03:57) made build-storybook upload the built theme artifacts so pr-visual could stop dying on "Theme butter is not built". It fixed half the problem. The theme artifacts import Core's consumer-facing dist files, and pr-visual starts from a fresh checkout that installs without building — so the job still failed, now with ERR_MODULE_NOT_FOUND: packages/core/dist/theme/index.js.
Why our verification missed it: the check was run on a Mac that already had a built packages/core/dist from earlier work. A contaminated environment produced a false pass. The failure was live on every component PR for ~14 hours and was reported to Cindy as fixed.
Reproduced before approving: removed packages/core/dist from a full build, ran loadThemeOverrides → the exact CI error. Ran pnpm -F @astryxdesign/core build → all seven themes load.
The fix adds one step to pr-visual after install, before the gate. It preserves #5477's point — no repeat of the full workspace build — while making the downloaded artifacts resolvable.
Lesson, and it generalises past CI: a local check for a missing artifact must remove the artifact first. Verifying on a warm tree can only ever prove the warm case.