fix(workbench): keep the registry in sync when an interface-set rebuild fails or moves ports#1289
Draft
gu-stav wants to merge 79 commits into
Draft
fix(workbench): keep the registry in sync when an interface-set rebuild fails or moves ports#1289gu-stav wants to merge 79 commits into
gu-stav wants to merge 79 commits into
Conversation
Co-authored-by: Josh <joshua.ellis18@gmail.com> fix(workbench): allow for a dynamic port (#830)
) * feat(dev): forward CLI config organization id to workbench runtime * chore: update auto-generated changeset for PR #905 --------- Co-authored-by: ecospark[bot] <ecospark[bot]@users.noreply.github.com>
Co-authored-by: Gustav Hansen <gustav.hansen@sanity.io>
…913) Pass `reactRefreshHost` to `@vitejs/plugin-react` so federated Studio modules connect their react-refresh preamble to the workbench host, enabling component-level HMR across the module federation boundary. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(dev): disable strict ports for applications * chore: update auto-generated changeset for PR #930 * fix: format * fix: format * chore: update tests --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(workbench): propagate staging env to workbench dev server The workbench dev server was missing the `__SANITY_STAGING__` Vite define that the app/studio dev servers receive via `getViteConfig`. This meant `SANITY_INTERNAL_ENV=staging` had no effect on the workbench client bundle. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: update auto-generated changeset for PR #964 --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
* fix(workbench): externalize sanity and @sanity/workbench * chore: update auto-generated changeset for PR #971 * chore: exclude .github from oxfmt format check Co-authored-by: Gustav Hansen <gu-stav@users.noreply.github.com> * fix: revert update changeset --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Gustav Hansen <gu-stav@users.noreply.github.com>
Co-authored-by: Gustav Hansen <gu-stav@users.noreply.github.com> Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
* feat(init): add promt for federation * chore: update auto-generated changeset for PR #988 --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
* fix: types * chore: update auto-generated changeset for PR #989 --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(workbench): extend local application payloads * fix: types * fix: pr feedback * chore: improve tests * fix: concise * fix: pr feedback
* feat(init): use `workbench` dist-tag for `sanity` package * chore: update auto-generated changeset for PR #992 * test: add unit tests --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
* fix(init): do not resolve dist tags * chore: update auto-generated changeset for PR #1000 --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
* feat(dev): extract studio manifest and pass it for local applications * chore: update auto-generated changeset for PR #997 * fix: rework to use manifests for both * chore: update auto-generated changeset for PR #997 * fix: cleanup * chore: share cache dir constant * feat: extract manifest in background * fix: path resolution on windows * fix: pr feedback * fix: pr feedback --------- Co-Authored-By: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
* fix(workbench): remove warmup for dependencies * chore: update auto-generated changeset for PR #1047 --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
…lifecycle explicit (#1256)
…ion build path Rebasing onto main brought in the single-vite vendor build (#1223): buildVendorDependencies → resolveVendorBuildConfig, importMap → AutoUpdatesBuildConfig, a 2-arg finalizeViteConfig, and getViteConfig moving into @sanity/cli-build. Fold those into the workbench/federation build path: - getViteConfig: adopt the autoUpdates vendor-chunk model on the client path, keep the isWorkbenchApp federation plugin branch. - buildApp/buildStudio/buildStaticFiles: switch to resolveVendorBuildConfig + autoUpdates, keep the services/views/isWorkbenchApp passthrough. - tests: getViteConfig.test.ts moved to the autoUpdates API (federation cases kept); restore main's buildStaticFiles vendor integration tests and move the federation unit tests to buildStaticFiles.federation.test.ts.
…orkbench projects (#1257) * fix(dev): restore dashboard URL output for non-workbench apps The workbench dev server work replaced the dashboard URL announcement with a bare port log for every app. Non-federated apps still load through the dashboard, so dropping the URL left users with nothing to open. Bring back main's output for them and keep the port-only line for workbench apps, where the workbench URL is announced instead. * chore: update auto-generated changeset for PR #1257 * fix(dev): re-add --load-in-dashboard with main's semantics The flag was dropped wholesale on this branch, changing studio and app output relative to main. Restore main's behavior for non-workbench projects (defaults, warnings, dashboard URL flows) and ignore the flag for workbench apps — warning when it's passed explicitly, since they never load through the dashboard. Also consolidates the dev suites' duplicated test setup (output mock, flags, dev-server config, vite-result factory, fetch stub) into the shared testHelpers. * chore: update auto-generated changeset for PR #1257 * fix(dev): satisfy mockResolvedValue arity in shared dev test helper The pre-commit eslint --fix strips the explicit undefined argument, which tsc then rejects. Use a plain resolved-promise mock instead so the formatter has nothing to rewrite. --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
* feat(workbench): deploy federated builds and log what ships to Brett `sanity deploy` required `index.html` in the build output, but federated (`unstable_defineApp`) builds emit only a module federation remote — every deploy died on a generic "Error checking directory" with the real cause hidden behind a debug flag. checkDir now validates the federation shape instead: the manifest must parse, declare a remote entry and at least one exposed module, and every referenced asset must exist on disk — catching partial or stale builds before upload. checkDir failures surface verbatim, and both deploy paths (studio and coreApp) log the upload target, metadata, and tarball contents before posting to the user-applications service. * chore: update auto-generated changeset for PR #1243 * chore(deploy): remove upload summary logging Keeps the PR scoped to federated build validation. The endpoint and tarball listing printed on every deploy was debugging aid, not product output. * chore: update auto-generated changeset for PR #1243 * chore(deploy): cut federation validation back to manifest presence and exposes Checking the remote entry and every referenced asset against disk re-verified what the bundler already guarantees and would break on any manifest format change. An empty exposes list still fails: a federated app must export at least one interface. * refactor(deploy): split checkWorkbenchAppDir out of checkDir A boolean option made checkDir's contract depend on the app shape. Callers already know whether they deploy a workbench app, so each build shape gets its own self-contained check and checkDir keeps its original index.html contract. * chore(deploy): check declared interfaces instead of built exposes The interfaces a federated app ships are exactly what unstable_defineApp declares (entry, views, services), so the at-least-one rule reads the config and fails before any prompts or API calls instead of parsing mf-manifest.json after the build. The manifest check shrinks to an existence marker that a federation build ran. Workbench studios always expose the studio config, so the rule only applies to the app path. * chore(deploy): say views or services instead of interfaces in the no-interface error "Interfaces" is application-service vocabulary; views and services are the words the user typed into unstable_defineApp. * refactor(deploy): gather workbench deploy checks in workbenchChecks The config-level interface check and the build-dir check guard the same deploy, so they live in one module: checkWorkbenchApp (throws like its sibling) and checkWorkbenchAppDir. * chore(deploy): mention entry in the no-interface deploy error --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
…ench projects (#1259) * fix(build): restore strict-port for plain studio dev servers The workbench refactor hardcoded strictPort: false, so a plain studio on a busy port silently drifted to the next free one instead of failing like on main. Only apps and workbench runs need the fallback. * fix(dev): keep default signal handling for plain projects devAction installed SIGINT/SIGTERM handlers for every project, changing Ctrl-C semantics (and exit codes) for plain runs that have no workbench lock or registry entry to clean up. Only register them when workbench state exists. * fix(dev): only override the port flag when the workbench runs devAction pre-resolved the port and injected it into flags for every project, taking resolution away from getDevServerConfig where main does it. The override only exists so the app server can move off the port the workbench claimed — without a workbench the flags now pass through untouched. * chore: update auto-generated changeset for PR #1259 * chore: update auto-generated changeset for PR #1259 --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
…ver (#1262) Everything the workbench vite server serves is either plain JS (the generated workbench.js), compiled dist (sanity/workbench), or TypeScript source that vite's own esbuild transforms (@sanity/workbench). plugin-react excludes node_modules from fast refresh, so it never transformed a single module here — its only output was a preamble script nothing consumed. Removing it also drops @vitejs/plugin-react from @sanity/cli's dependencies, keeping vite plugins contained to @sanity/cli-build.
…/cli re-exports (#1269)
…#1267) * fix(dev): display localhost for non-routable workbench bind addresses sanity dev --host 0.0.0.0 printed http://0.0.0.0:3333 as the workbench URL — both for the binding process and for any process reusing the lock, which stores the bind host verbatim. 0.0.0.0/:: don't open in every browser (notably on Windows). Only the displayed URL is normalized; the bind address and lock contents are untouched. * chore: update auto-generated changeset for PR #1267 --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
* feat(cli-build)!: require organizationId in unstable_defineApp unstable_defineApp is the workbench opt-in and the organization ID is what the workbench runs and deploys against, so it's part of the contract — required in the schema and the DefineAppInput/DefineAppResult types, with a pointed validation message. Ports sanity-io/workbench#250 to the vendored extension API. * chore: update auto-generated changeset for PR #1279 --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
* fix(dev): release the workbench lock when server creation throws createWorkbenchViteServer can throw after the lock is acquired (runtime- file write failure, invalid remote URL); only the undefined-result path released it, so a throw leaked the lock file until the next acquire pruned the stale PID. * chore: update auto-generated changeset for PR #1266 --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
Residue of the main sync after the rebase: the regenerated lockfile entries (sanity v6 fixture refs, @types/node 22 peer suffixes) and the organizationId brand on deploy.studio.test.ts's unstable_defineApp call required by #1279.
…rs (#1264) * fix(build): apply caller-provided vite plugins to workbench dev servers additionalPlugins (typegen in dev) was only spread into the non-workbench plugin branch, so typegen.enabled was silently ignored for workbench apps. The plugins aren't client-specific — hoist them out of the branch. * chore: update auto-generated changeset for PR #1264 --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
…pp (#1265) unstable_defineApp is the workbench opt-in and carries the organization ID, so the fallback that resolved it from the configured project is gone — it needed an authenticated user (a logged-out sanity dev died on a raw auth error) and an API round-trip on every startup for something the opt-in already declares. A missing ID is now a readable error pointing at unstable_defineApp in sanity.cli.ts.
* ci: revert snapshot-release.yml to main The branch had gutted the forceBump full-release half of the workflow and hardcoded the workbench dist-tag; merging that to main would have silently removed the manual release capability. Restoring main's file verbatim removes the merge delta entirely. Branch snapshots are published by dispatching the workflow manually with 'workbench' as the tag input — pushes to feat/workbench no longer auto-publish. * chore: curate the workbench changesets for the main changelog 40 accumulated changesets — auto-generated per-PR entries, superseded alpha bumps, and snapshot-publish-ordering notes — would have landed in the public changelog verbatim at merge. Squashed into three human-written entries; the resulting bump plan is unchanged (majors across the board, driven by main's Node 20 drop), so branch snapshot releases version identically.
… entry The dts change ships as part of the workbench feature; the curated workbench.md entry already covers it for the public changelog.
…1287) * fix(build): fail fast when a workbench studio has no sanity config An explicit applicationType: 'studio' wins over detection, so the federation studio path is reachable without a sanity.config file. The non-null assertion let null flow into the generated remote entry, which then imports its config from "null" — a broken build with no hint at the cause. * chore: update auto-generated changeset for PR #1287 --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
…1284) * fix(workbench): stop wiping studio interfaces from the dev registry The studio extract hardcoded interfaces: undefined while the registry patch is a shallow merge — the panels/workers forwarded by the initial registration were wiped on the first regeneration, which fires right at watcher startup. Re-derive them from a fresh config read, the way the core-app extract already does. * chore: update auto-generated changeset for PR #1284 --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
debug is imported in getViteConfig.ts for type checking only; without the ignore knip flags @types/debug as unused once dependencies shift.
* revert(workbench): keep federated remote type generation off Reverts the behavior change from #1282. Real TS apps fail with TYPE-001: the exposes are generated .js/.jsx shims, which tsc refuses without allowJs (TS6504) — no app template sets it. With allowJs worked around, declaration emit pulls in the user's own noEmit app code, which was never written to be declaration-emittable (TS2742 non-portable inferred types under pnpm, TS4082 private names in default exports). The #1282 verification only passed because the federated-studio fixture set allowJs — removed here so the fixture matches real templates. The recursive plugin scoping from #1282 stays: it's correct regardless. * chore: update auto-generated changeset for PR #1288 --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
…anges (#1286) * fix(workbench): rebuild federated studios when their interface set changes Studios declare views/services in unstable_defineApp like apps do, but the dev flow assumed they couldn't: no rebuild hook, and the manifest watcher only reacted to sanity.config.* (the resolved project root) while the declarations live in sanity.cli.*. Adding or removing a studio view/service required a manual dev-server restart to get an expose for it. * chore: update auto-generated changeset for PR #1286 --------- Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
…ld fails or moves ports
Contributor
📦 Bundle Stats —
|
| Metric | Value | vs feat/workbench (5a42492) |
|---|---|---|
| Internal (raw) | 2.7 KB | - |
| Internal (gzip) | 1.1 KB | - |
| Bundled (raw) | 11.15 MB | - |
| Bundled (gzip) | 2.10 MB | - |
| Import time | 906ms | +28ms, +3.1% |
bin:sanity
| Metric | Value | vs feat/workbench (5a42492) |
|---|---|---|
| Internal (raw) | 782 B | - |
| Internal (gzip) | 423 B | - |
| Bundled (raw) | 9.87 MB | - |
| Bundled (gzip) | 1.77 MB | - |
| Import time | 2.12s | +75ms, +3.6% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @sanity/cli-core
Compared against feat/workbench (5a424922)
| Metric | Value | vs feat/workbench (5a42492) |
|---|---|---|
| Internal (raw) | 105.5 KB | - |
| Internal (gzip) | 25.8 KB | - |
| Bundled (raw) | 21.71 MB | - |
| Bundled (gzip) | 3.45 MB | - |
| Import time | 798ms | +13ms, +1.7% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — create-sanity
Compared against feat/workbench (5a424922)
| Metric | Value | vs feat/workbench (5a42492) |
|---|---|---|
| Internal (raw) | 908 B | - |
| Internal (gzip) | 483 B | - |
| Bundled (raw) | 931 B | - |
| Bundled (gzip) | 491 B | - |
| Import time | ❌ ChildProcess denied: node | - |
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
Contributor
Coverage Delta
Comparing 69 changed files against main @ Overall Coverage
|
gu-stav
force-pushed
the
feat/workbench
branch
2 times, most recently
from
80854d6 to
f5283f9
Compare
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
2 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
Bugbot flagged 3 issues with the interface-set rebuild on #907 (review of 5a42492): dead server after failed restart, set id committed before the rebuild, and stale port in the registry. They share a root cause: the rebuild isn't transactional with the registry state.
Three failure paths, one shape:
started: false(e.g.organizationIdremoved mid-edit) left the Vite server down while the watcher still patched the registry with the new interfaces — the workbench reloaded into a port with nothing listening.lastInterfaceSetIdwas committed before the rebuild ran, so a thrown rebuild was never retried: the next pass over the same declarations saw "set unchanged" and skipped it. Fixing the config while keeping the new view left the server down until a manual restart.Now
onInterfaceSetChangeresolves with the recreated server (and throws when the restart doesn't produce one). The registration commits the set id only after the rebuild succeeds — a failed pass retries on the next config save — and patches the registry with the rebuilt server's actual address.When the restart fails, the registry keeps the old entry (old interfaces, old port). There's no live address to advertise at that point; the entry self-heals on the next successful rebuild.
What to review
startDevServerRegistration'supdatecallback is the core change: early-return for an unchanged set, rebuild → commit id → patch address for a changed one.devAction's rebuild hook now returns the server instead of swallowing the outcome.Testing
Covered in
devAction.test.ts(hook resolves with the recreated server / rejects on early exit) andstartDevServerRegistration.test.ts(failed rebuild retries on the next pass without touching the registry; successful rebuild patches the new address; manifest-only passes don't rewrite it).