Skip to content

feat(workbench): setup#907

Draft
gu-stav wants to merge 44 commits into
mainfrom
feat/workbench
Draft

feat(workbench): setup#907
gu-stav wants to merge 44 commits into
mainfrom
feat/workbench

Conversation

@gu-stav
Copy link
Copy Markdown
Member

@gu-stav gu-stav commented Apr 9, 2026

Description

Note

Please ignore this PR. It's just open to have visibility into the state of the feature branch.

What to review

Testing

Notes for release

@claude
Copy link
Copy Markdown

claude Bot commented Apr 9, 2026

Claude finished @gu-stav's task in 3m 58s —— View job


Claude PR Review

  • Analyze PR diff scope and structure
  • Review new dev server registry & federation logic
  • Review workbench dev server + runtime
  • Review build/vite config changes
  • Review schema/config & manifest changes
  • Check test coverage adequacy
  • Verify changesets
  • Post final review

Reviewing only the high-signal issues — this is a 100-file feature branch made up of many sub-PRs that were merged in over time, so most code has already been reviewed in those individual PRs. The findings below are the things I'd want addressed before this lands on main.

Bugs / logic issues

1. Workbench lock leaks if createWorkbenchViteServer throwspackages/@sanity/cli/src/actions/dev/startWorkbenchDevServer.ts:93-104

The lock is acquired at line 77, but the work between then and the if (!result) cleanup happens inside createWorkbenchViteServer, which can throw from three places that are not wrapped in try/finally:

  • parseRemoteUrl (line 138) throws on invalid SANITY_INTERNAL_WORKBENCH_REMOTE_URL
  • resolveOrganizationId (line 140) throws if neither app.organizationId nor api.projectId is set
  • writeWorkbenchRuntime can reject on filesystem errors

When any of these throws, the lock file stays on disk. The error then propagates out of startWorkbenchDevServer before devAction.ts's catch block can call closeWorkbenchServer (which is never assigned because destructuring of the throwing call doesn't happen).

It self-heals on the next sanity dev invocation (stale-lock pruning), but during the current process lifetime the user gets a confusing "workbench already running" the next time they retry. Wrap the body of startWorkbenchDevServer after acquireWorkbenchLock in try/catch that releases the lock and rethrows.

2. Org-ID failure crashes sanity dev entirelypackages/@sanity/cli/src/actions/dev/startWorkbenchDevServer.ts:226-242

resolveOrganizationId throws when neither app.organizationId nor an api.projectId (resolvable to an org) is configured. Because the throw escapes createWorkbenchViteServer, sanity dev fails outright instead of falling back to "no workbench" mode — which is the documented degradation path for every other workbench failure in this file (missing package, port collision, etc.). The graceful behavior would be to output.warn(...) and return {workbenchAvailable: false, ...}.

3. extractCoreAppManifest icon path check misses Windows cross-drive pathspackages/@sanity/cli/src/actions/manifest/extractCoreAppManifest.ts:19-26

const resolvedPath = resolve(workDir, iconPath)
const pathRelativeToWorkDir = relative(workDir, resolvedPath)
if (pathRelativeToWorkDir.startsWith('..')) { throw  }

On Windows, relative('C:\\work', 'D:\\evil') returns 'D:\\evil' (no leading ..), so cross-drive paths bypass the containment check. Low impact because the user controls their own sanity.cli.ts, but the guard reads as a security check — either tighten it (path.isAbsolute(iconPath) || pathRelativeToWorkDir.startsWith('..')) or drop the check and document the behavior. Also: the comment "Brett sanitizes SVGs so it's skipped here" references an internal team member — replace with the package/process that does the sanitizing.

4. federation block always written into generated configpackages/@sanity/cli/src/actions/init/createCliConfig.ts:18-20, createAppCliConfig.ts:11-13

The template always emits federation: { enabled: false } even when the user opted out. For a feature that's currently alpha, this surfaces an opaque setting to every new project. Conditionally include the block only when federation: true — matches how deployment is handled and avoids confusing users who don't know what federation is.

5. flagOrDefault(options.federation, true) defaults to enabled in non-interactive flowspackages/@sanity/cli/src/actions/init/initAction.ts:254

In --unattended mode (CI, scripts), federation is silently turned on. Given federation is alpha and the workbench rollout is gated, the unattended default should be false. Otherwise CI-bootstrapped projects opt in to alpha behavior without anyone making that call.

Test coverage

extractDevServerManifest.ts reports 20.0% statement coverage in the coverage delta. Add a test that mocks extractManifest + readFile and verifies the JSON parse path / error propagation.

Changeset issues

The product-copy skill applies here — multiple changesets read like commit messages or internal jargon rather than user-facing descriptions:

  • pr-988.md, pr-989.md — typo: "promt" → "prompt"
  • pr-1028.md fix(workbench): add __mf__temp directory to .gitignore — implementation detail; describe the user-facing effect ("Generated projects now ignore the workbench temporary build directory.")
  • pr-1057.md fix(workbench): prune stale lock files — internal mechanism; user-facing description: "Recover automatically when a previous workbench process crashed without releasing its lock."
  • pr-1066.md fix(workbench): throw error on invalid … — phrase from the user's perspective
  • pr-1042.md perf(workbench): preload workbench and warmup dev-server files — drop the conventional-commit prefix; describe the effect ("Workbench cold-start is faster…")
  • pr-992.md use 'workbench' dist-tag for 'sanity' package — implementation detail
  • pr-770.md "Enforce single workbench instance with dev-server registry, fix PID-reuse detection and signal cleanup" — multiple changes in one line; split or summarize as the user-facing outcome

Many other entries start with fix(...)/feat(...) prefixes which are typically stripped from user-facing changelogs. Worth a sweep to bring them in line with Sanity product copy conventions (no marketing voice, no implementation details, one concise sentence on the user-facing effect).

Minor

  • packages/@sanity/cli/src/actions/dev/writeWorkbenchRuntime.ts:77-85buildPrefetchHints does new URL(remoteUrl) again and swallows errors silently. Since parseRemoteUrl already validates upstream, the catch is unreachable; either drop the try/catch or assert with a clear error.
  • packages/@sanity/cli/src/actions/dev/devServerRegistry.ts:209,228 — registry writes aren't atomic. A crash mid-write produces invalid JSON. safeParse handles it in getRegisteredServers, but a write-then-rename to a .tmp file would avoid the noise on the next startup.
  • packages/@sanity/cli/src/prompts/init/federation.ts:6 — the prompt "Would you like to enable federation for this project?" gives users zero context about what federation is or that it's alpha. Add a brief description and an (alpha) marker, or link to docs.
    · branch: feat/workbench

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2026

📦 Bundle Stats — @sanity/cli

Compared against main (cd110e7c)

@sanity/cli

Metric Value vs main (cd110e7)
Internal (raw) 2.1 KB +8 B, +0.4%
Internal (gzip) 801 B +2 B, +0.3%
Bundled (raw) 10.97 MB +78 B, +0.0%
Bundled (gzip) 2.06 MB +19 B, +0.0%
Import time 836ms +15ms, +1.8%

bin:sanity

Metric Value vs main (cd110e7)
Internal (raw) 1023 B -
Internal (gzip) 486 B -
Bundled (raw) 9.84 MB -
Bundled (gzip) 1.77 MB -
Import time 2.27s +339ms, +17.5% ⚠️

🗺️ 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 main (cd110e7c)

Metric Value vs main (cd110e7)
Internal (raw) 99.5 KB +2.5 KB, +2.6%
Internal (gzip) 23.4 KB +657 B, +2.8%
Bundled (raw) 21.64 MB +2.2 KB, +0.0%
Bundled (gzip) 3.43 MB +676 B, +0.0%
Import time 801ms +9ms, +1.2%

🗺️ 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 main (cd110e7c)

Metric Value vs main (cd110e7)
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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2026

Coverage Delta

File Statements
packages/@sanity/cli-core/src/config/cli/getCliConfig.ts 100.0% (±0%)
packages/@sanity/cli-core/src/config/cli/schemas.ts 100.0% (±0%)
packages/@sanity/cli-core/src/services/cliUserConfig.ts 100.0% (±0%)
packages/@sanity/cli-core/src/util/getSanityConfigDir.ts 66.7% (new)
packages/@sanity/cli/src/actions/build/buildApp.ts 95.3% (±0%)
packages/@sanity/cli/src/actions/build/buildStaticFiles.ts 97.6% (+ 1.0%)
packages/@sanity/cli/src/actions/build/buildStudio.ts 96.7% (±0%)
packages/@sanity/cli/src/actions/build/getViteConfig.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/build/writeSanityRuntime.ts 96.2% (+ 0.7%)
packages/@sanity/cli/src/actions/deploy/deployApp.ts 80.5% (±0%)
packages/@sanity/cli/src/actions/deploy/deployStudioSchemasAndManifests.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/dev/devAction.ts 96.8% (- 3.2%)
packages/@sanity/cli/src/actions/dev/devServerRegistry.ts 93.8% (new)
packages/@sanity/cli/src/actions/dev/extractDevServerManifest.ts 20.0% (new)
packages/@sanity/cli/src/actions/dev/getDevServerConfig.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/dev/startAppDevServer.ts 100.0% (+ 13.0%)
packages/@sanity/cli/src/actions/dev/startDevManifestWatcher.ts 90.9% (new)
packages/@sanity/cli/src/actions/dev/startFederationRegistration.ts 100.0% (new)
packages/@sanity/cli/src/actions/dev/startStudioDevServer.ts 100.0% (+ 5.0%)
packages/@sanity/cli/src/actions/dev/startWorkbenchDevServer.ts 98.7% (new)
packages/@sanity/cli/src/actions/dev/writeWorkbenchRuntime.ts 100.0% (new)
packages/@sanity/cli/src/actions/init/bootstrapLocalTemplate.ts 89.7% (±0%)
packages/@sanity/cli/src/actions/init/bootstrapTemplate.ts 0.0% (±0%)
packages/@sanity/cli/src/actions/init/createAppCliConfig.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/init/createCliConfig.ts 100.0% (+ 50.0%)
packages/@sanity/cli/src/actions/init/createStudioConfig.ts 83.3% (±0%)
packages/@sanity/cli/src/actions/init/initAction.ts 97.2% (+ 0.1%)
packages/@sanity/cli/src/actions/init/initApp.ts 96.0% (±0%)
packages/@sanity/cli/src/actions/init/initStudio.ts 87.0% (±0%)
packages/@sanity/cli/src/actions/init/scaffoldTemplate.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/init/types.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/manifest/extractCoreAppManifest.ts 93.1% (new)
packages/@sanity/cli/src/actions/manifest/extractManifest.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/manifest/types.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/manifest/writeManifestFile.ts 100.0% (±0%)
packages/@sanity/cli/src/commands/dev.ts 100.0% (±0%)
packages/@sanity/cli/src/commands/init.ts 100.0% (±0%)
packages/@sanity/cli/src/commands/manifest/extract.ts 100.0% (±0%)
packages/@sanity/cli/src/commands/schemas/deploy.ts 95.0% (±0%)
packages/@sanity/cli/src/prompts/init/federation.ts 100.0% (new)
packages/@sanity/cli/src/server/devServer.ts 94.1% (±0%)
packages/@sanity/cli/src/util/resolveReactStrictMode.ts 100.0% (new)

Comparing 42 changed files against main @ cd110e7c2b0cdad7438f53d958a0831fea2a3773

Overall Coverage

Metric Coverage
Statements 84.6% (+ 0.3%)
Branches 74.7% (+ 0.4%)
Functions 84.5% (+ 0.3%)
Lines 85.1% (+ 0.4%)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2026

Preview this PR with pkg.pr.new

Run the Sanity CLI

npx https://pkg.pr.new/sanity-io/cli/@sanity/cli@d73d551 <command>

...Or upgrade project dependencies

📦 @sanity/cli
pnpm install https://pkg.pr.new/@sanity/cli@d73d551
📦 @sanity/cli-build
pnpm install https://pkg.pr.new/@sanity/cli-build@d73d551
📦 @sanity/cli-core
pnpm install https://pkg.pr.new/@sanity/cli-core@d73d551
📦 @sanity/cli-test
pnpm install https://pkg.pr.new/@sanity/cli-test@d73d551
📦 @sanity/eslint-config-cli
pnpm install https://pkg.pr.new/@sanity/eslint-config-cli@d73d551

View Commit (d73d551)

@socket-security
Copy link
Copy Markdown

socket-security Bot commented Apr 9, 2026

All alerts resolved. Learn more about Socket for GitHub.

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report

@gu-stav
Copy link
Copy Markdown
Member Author

gu-stav commented Apr 10, 2026

The axios security warning will be resolved by module-federation/core#4644 I think

@gu-stav
Copy link
Copy Markdown
Member Author

gu-stav commented Apr 13, 2026

Rebased onto main

@joshuaellis joshuaellis force-pushed the feat/workbench branch 3 times, most recently from ffc8344 to f2b1294 Compare April 17, 2026 12:39
@gu-stav gu-stav force-pushed the feat/workbench branch 2 times, most recently from 5e50a1d to 1e221bd Compare April 23, 2026 12:19
@gu-stav gu-stav force-pushed the feat/workbench branch 2 times, most recently from 16e7dfe to 319ef83 Compare May 5, 2026 09:17
@joshuaellis joshuaellis force-pushed the feat/workbench branch 2 times, most recently from 905b492 to cf3839a Compare May 7, 2026 11:47
gu-stav and others added 9 commits May 21, 2026 15:44
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>
gu-stav and others added 29 commits May 21, 2026 15:50
* 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>
Adds `--federation` / `--no-federation` flag to `sanity init` and wires
it through `initAction` so the value is passed to `initApp` / `initStudio`
alongside the federation prompt added in #988.

The flag plumbing was previously part of the rebase merge that resolved
feat/workbench against main, and was dropped in the latest rebase.
* fix(workbench): prune stale lock files

* fix: add back

* chore: update auto-generated changeset for PR #1057

---------

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>
…OTE_URL` (#1066)

* fix(workbench): throw error in invalid `SANITY_INTERNAL_WORKBENCH_REMOTE_URL`

* chore: update auto-generated changeset for PR #1066

---------

Co-authored-by: squiggler-app[bot] <265501495+squiggler-app[bot]@users.noreply.github.com>
* fix(workbench): detect PID reuse on Windows via PowerShell

* chore: update auto-generated changeset for PR #1067

* chore(changeset): rewrite changeset summary as user-facing description

Co-authored-by: Rune Botten <runeb@users.noreply.github.com>

* chore: update auto-generated changeset for PR #1067

---------

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: Rune Botten <runeb@users.noreply.github.com>
After rebasing onto main:
- buildStudio.ts: plumb `federation` through `InternalBuildOptions` so the
  `!options.federation?.enabled` guard and the `federation` prop passed to
  `buildStaticFiles` resolve. Mirrors the same plumbing already done in
  buildApp.ts during the rebase.
- buildStaticFiles.test.ts: `copyDir`/`writeFavicons` moved to `@sanity/cli-build/_internal`
  in main's #1062 refactor — point the mock at the new path.
- startStudioDevServer.test.ts: `checkStudioDependencyVersions` moved to
  `@sanity/cli-build/_internal`; `getLocalPackageVersion` moved to
  `@sanity/cli-core` (main's #1062 and #1053). Update mocks accordingly.
Fix stale plugin mock paths and align typegen test assertions with the
app dev server output.
# Conflicts:
#	packages/@sanity/cli/src/actions/manifest/extractManifest.ts
#	packages/@sanity/cli/src/commands/manifest/extract.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants