Commit Graph
2 Commits
Author SHA1 Message Date
Oleg Isonen 87968f40ff fix: Improve MCP authoring and preview reliability (#6105)
## Summary

Fix MCP reliability failures reported in ten issues against CLI 0.288.0
and 0.289.0:

- keep every advertised array input schema client-valid, including
compact and tuple schemas
- report a text asset update as successful when its persisted revision
and bytes confirm that the server committed despite a failed response
- preserve standard package import resolution in generated React Router
previews
- upload variable WOFF2 fonts when callers provide the documented static
font metadata shape
- explain unmatched JSX as a stateless fragment syntax error
- distinguish the `animation` component namespace from value helpers and
reject unsupported CSS at-rules with an actionable validation error
- forward optional session tools such as `download-asset` through
top-level CLI shortcuts
- isolate published generated-preview dependencies from parent
workspaces
- return structured screenshot failures when React Router production
builds fail

Sensitive visual regression checks also run automatically for
organization-authored fork PRs. External fork PRs remain gated by
`safe-to-deploy`.

## Root causes and implementation

The MCP handshake compactor retained an array's type but discarded its
`items` schema. Unconstrained and tuple arrays could also be advertised
without `items`. Schema normalization now adds `items: {}` whenever an
array omits it, while focused discovery retains complete constraints.
Catalog coverage rejects every missing array item, including tuple
schemas, and the handshake remains below its 200 KB budget.

Asset content updates previously treated any failed response as a failed
mutation, even when the server had already swapped the revision.
Recovery now reads the current revision, compares the persisted bytes,
and confirms the revision remains stable across the read. It returns the
updated asset only when all evidence agrees. Otherwise it returns
`ASSET_UPDATE_COMMIT_UNCERTAIN` with guidance not to retry blindly.
Definitive 4xx responses retain their original errors.

The generated React Router Vite config replaced the client and SSR
resolver condition lists without explicitly retaining the standard
`import` condition. Both lists now preserve `import`. The regression
test generates a dynamic route, stages the built React Router SDK as a
published dependency without the local `webstudio` source condition, and
runs the generated build.

Variable fonts are detected with `variationAxes`. Merging the documented
static `style` and `weight` upload metadata into that detected variable
shape made validation fail after the binary was read. Font override
validation and shape-aware merging now live in `@webstudio-is/fonts`.
The SDK delegates to that domain function, which validates the complete
override before applying compatible fields, retains the detected
static-or-variable shape, and keeps the detected format.

Fragment parsing is stateless. Unmatched opening and closing tags now
explain that every opening element must use the same closing tag or be
self-closing, and that refreshing the session cannot repair malformed
JSX.

The `animation` binding is a component namespace, not a callable helper.
Tool schema text, evaluation errors, and the MCP manual now separate
component namespaces from value helpers. The MCP JSX `css` binding
parses the final interpolated CSS with CSS-tree and recursively rejects
selectors and unsupported at-rules such as `@keyframes`, while
preserving supported declarations and case-insensitive `@media` rules.
Keeping this validation at the external binding avoids changing internal
component templates.

CLI shortcut discovery previously omitted optional session tools even
though the runtime registered their handlers. Shortcut discovery now
includes the same optional tool categories as the CLI MCP host, while
real top-level CLI commands keep precedence.

Published generated previews could reuse a stale `node_modules` symlink
into a parent source workspace. Only development manifests containing
the local placeholder version may now reuse the CLI dependency tree.
Published manifests replace symlinks with a local install and disable
npm workspace discovery.

React Router could report a bundler failure while its build command
exited successfully outside CI, allowing screenshot startup to terminate
without a structured result. Managed production builds now set `CI=1`,
so the existing MCP error boundary receives the nonzero build failure
and prints structured JSON.

Visual regression authorization now distinguishes repository members
from external contributors. Organization members' fork PRs run on open,
synchronize, and reopen events; external fork code runs only from a
`safe-to-deploy` label event, and a later commit does not reuse that
approval.

## Implementation checklist

- [x] Preserve valid array item schemas in compact MCP discovery
- [x] Normalize tuple input schemas with an `items` fallback
- [x] Keep the MCP handshake below its 200 KB size budget
- [x] Cover the real `update-props` schema and oversized array
compaction
- [x] Reconcile failed asset-update responses against stable persisted
content
- [x] Reject recovery when a concurrent asset revision is detected
- [x] Preserve `import` resolution in generated React Router client and
SSR builds
- [x] Build a generated dynamic route against a staged published SDK
package
- [x] Regenerate and verify affected React Router fixtures
- [x] Preserve detected variable-font axes with documented upload
metadata
- [x] Add actionable unmatched-tag and unsupported-CSS validation
- [x] Correct animation namespace guidance in tool discovery and errors
- [x] Forward optional session tools through top-level CLI shortcuts
- [x] Replace parent-workspace dependency links for published previews
- [x] Disable npm workspace discovery for isolated preview installs
- [x] Make React Router build failures return nonzero in managed
previews
- [x] Run visual regression automatically for organization-authored PRs
- [x] Verify each regression test fails without its corresponding fix
- [x] Review documentation impact and update the authoritative MCP
manual plus generated outputs
- [x] Confirm no database migration or compatibility migration is
required
- [ ] Run `pnpm evaluations` after explicit approval; not run because
repository policy requires approval before agent evaluations

## Verification

- `packages/cli/src/prebuild.test.ts`: 57 tests passed
- `packages/cli/src/commands/schema.test.ts` and `mcp.test.ts`: 51 tests
passed
- `packages/project-build/src/runtime/components.test.ts` and
`mcp.test.ts`: 194 tests passed
- `packages/asset-uploader`: 196 tests passed
- focused font schema, SDK asset schema, CSS parser, and template CSS
tests passed
- `packages/http-client/src/index.test.ts`: 38 tests passed
- relevant CLI, project-build, asset-uploader, fonts, SDK, CSS data,
template, HTTP client, and React Router SDK typechecks passed
- MCP documentation tests, typecheck, generation, and Markdown source
tests passed
- React Router SDK package build passed
- full fixture link, sync, and generation pipeline passed
- affected Docker and Vercel fixture typechecks and locally bootstrapped
production builds passed
- complete CLI suite: 61 test files and 966 tests passed
- CLI typecheck passed
- preserved nested-workspace dynamic preview installed local `0.289.0`
dependencies and completed its production build
- visual-regression workflow YAML parsing and authorization truth table
passed
- repository lint passed with warnings denied
- formatting and `git diff --check` passed

## Tradeoffs and limitations

Deferred array schemas advertise unconstrained `items` in the compact
handshake to remain client-valid without duplicating detailed
constraints. Agents can retrieve the complete item schema through
focused `meta.get-more-tools` discovery.

Asset reconciliation performs three read requests only after an
uncertain 5xx, malformed response, or transport failure. This keeps the
normal successful path unchanged and avoids claiming success if the
revision changes during recovery.

For variable fonts, detected axes define the metadata shape. Static
`style` and `weight` hints are ignored because they cannot be
represented alongside `variationAxes`; compatible fields such as
`family` still override detection.

Native CSS keyframe authoring remains unsupported in fragment `css`
templates. The tool now rejects it before rendering and directs callers
to editable Webstudio Animation components instead of advertising a
nonexistent callable helper.

Published previews perform one isolated npm install when no valid local
dependency tree is cached. Local source development keeps the fast
workspace symlink path for placeholder-version manifests.

Closes #6104
Closes #6099
Closes #6098
Closes #6106
Closes #6107
Closes #6108
Closes #6114
Closes #6115
Closes #6116
Closes #6117
2026-08-12 21:23:35 +02:00
Oleg Isonen db2d092268 ci: replace Lost Pixel with self-hosted visual regression (#6086)
## Summary

Replace the defunct Lost Pixel service and the interim Chromatic
workflow with a fast, self-hosted visual comparison pipeline.

The workflow indexes the existing CSF stories without building
Storybook, builds and serves a lightweight browser app with Vite,
renders the merge base and current revision through Webstudio's shared
browser tooling, and compares screenshots with Webstudio's pixel-diff
and OCR engine. It uses GitHub Actions cache and artifacts only—no R2
bucket or external visual-testing service.

Closes #6064

## Implementation

- [x] Remove the Lost Pixel workflow, configuration, acknowledgement,
and global two-second wait.
- [x] Remove the interim Chromatic workflow and settings.
- [x] Extract source-agnostic capture, diff, OCR, audit, cache, and
report tooling into `@webstudio-is/vision`; rename the
Webstudio-specific preview boundary to
`@webstudio-is/project-build/preview`.
- [x] Expose the reusable browser capture session through the shared
vision boundary.
- [x] Make OCR and diff-artifact generation conditional on detected
pixel changes.
- [x] Render existing CSF stories through a typed, lightweight browser
harness built with Vite.
- [x] Replace the embedded HTML and handwritten HTTP/MIME server with a
real HTML entry, Vite production build, and Vite preview server; isolate
declarative build configuration in `story-build.ts`.
- [x] Apply Storybook preview annotations, decorators, global CSS,
backgrounds, and story metadata.
- [x] Store story scopes as shared data and load each revision's own
configuration.
- [x] Discover changed, added, and removed stories from both revisions.
- [x] Populate private story sources symmetrically in the baseline
worktree.
- [x] Isolate every story with a fresh React root and deterministic
time, randomness, intervals, focus, animation, and layout readiness.
- [x] Preserve story-owned visual options for masking, settling, and
interval control.
- [x] Reuse Vite preview servers, one Chromium process, grouped browser
pages, production bundles, and a bounded CPU worker pool for image
comparison.
- [x] Recheck initial pixel differences with serial captures, then
recapture persistent baseline/current pairs on the same browser page so
renderer variance cannot create false failures.
- [x] Report individual story render failures without discarding
successful captures.
- [x] Enforce a 0.1 per-pixel threshold and 0.001% total mismatch
tolerance.
- [x] Run OCR only for changed comparisons and include text-change
evidence.
- [x] Move the generic report model, writer, static viewer, and opener
into the shared Vision package for reuse by CI and future CLI
comparisons.
- [x] Bundle report HTML, styling, and browser behavior as static Vision
assets without runtime source-file lookups.
- [x] Copy external screenshot inputs into self-contained report
artifacts for CLI reuse.
- [x] Handle cached removed-only comparisons without rendering the
removed story on the current server.
- [x] Support focused local reproduction and opening the report with one
command.
- [x] Pin CI to the lockfile's Playwright Chromium, cache complete
revision screenshots with matching restore/save paths and automatically
derived runtime invalidation, and render both revisions on a miss.
- [x] Keep cache archives bounded to one revision instead of
accumulating the cache history.
- [x] Run the complete visual suite in one CI job and upload one report
as a 14-day artifact.
- [x] Fail PR checks on visual differences, empty runs, missing
screenshots, missing or empty report artifacts, and infrastructure
failures.
- [x] Rerun fork checks when `safe-to-deploy` is added.
- [x] Allow reviewed visual-only differences through
`visual-change-approved` and invalidate approval after new commits.
- [x] Keep capture, pixel diff, OCR, and report generation
source-agnostic for future draft-versus-published project comparisons.
- [x] Use Vite for the complete story HTML, module, asset,
production-build, and preview-server lifecycle; inline the selected
stories and skip ineffective tree-shaking to keep the cached run under
one minute.
- [x] Review Builder product documentation impact: no user-facing
Builder documentation is affected; contributor usage is documented in
`scripts/visual-regression/README.md`.
- [x] Review fixture impact: no fixture inputs or generated fixture
outputs are affected.

## Verification

- [x] Project-build Vision tests, including the reusable report test,
and entrypoint contract tests pass.
- [x] CLI screenshot, preview, and browser-session tests pass.
- [x] Project-build, CLI, Builder, design-system, and visual-runner
typechecks pass.
- [x] Visual runner tests pass (10), and the complete Vision package
passes (79), including demonstrated fail-before/pass-after coverage.
- [x] Full Storybook production build passes.
- [x] Full local Vite run completes all 238 stories in 69.9s cold and
50.3s with a cached baseline plus two same-page serial verifications,
with zero differences; identical-image comparison remains sub-second.
- [x] The report writer passes a bundled-output smoke test and
materializes external screenshot assets.
- [x] The generated report validates all 238 entries and referenced
images.
- [x] Hosted cached-baseline comparison completes all 238 stories in
52.9s with zero differences; the cache-miss fallback completes in
101.3s.
- [x] Cache-miss fallback renders both Git revisions on demand.
- [x] Visual differences produce baseline, current, diff, OCR, JSON, and
HTML evidence and fail without approval.
- [x] Hosted single-job visual workflow restores both the pinned browser
and baseline cache, validates all 238 public and private stories with
zero differences, and uploads the report.
- [x] The final Vite production build renders all 238 stories without
dependency invalidation or duplicate React runtimes, and restores
`NODE_ENV` so cached serial fallback installs the baseline's dev
dependencies.
- [x] Root lint, package-boundary validation, and relevant formatting
checks pass.
- [x] Workflow YAML parses successfully.
- [x] `git diff --check`, staged-diff review, and final worktree review
pass.

## Usage

Run the complete local comparison and open the visual report:

```sh
pnpm visual-regression:ui
```

Focus one component while iterating:

```sh
pnpm visual-regression:ui -- --grep "Design system/Button"
```

Reopen the latest report without rerunning:

```sh
pnpm visual-regression:report
```

The CI report is available as the `visual-regression-report` artifact.

## Tradeoffs

- Baseline screenshots are a disposable acceleration cache, not source
of truth. A missing or invalid cache renders the merge base directly
from Git.
- GitHub artifact storage provides downloadable one-click reports;
permanently hosted browser links would require another hosting surface.
- OCR runs after pixel triage because invoking Tesseract for every
unchanged story would make the common path unnecessarily slow.
- Vite production builds add Rollup's module graph and HTML/asset
pipeline. The runner emits one bundle because every selected story is
rendered, and skips tree-shaking because it removed little while
increasing build time.
- The verified local path is 50.3s cached and 69.9s cold. The hosted
path is 52.9s cached and 101.3s cold. A new browser/runtime cache must
render its baseline once; the pinned browser keeps later runners on the
reusable path.
- The complete suite stays in one Actions job; browser-page concurrency
is internal and bounded.
2026-08-12 10:55:52 +02:00