Skip to content

refactor(deploy): restructure core app deploy and extract shared deploy checks#1406

Open
gu-stav wants to merge 8 commits into
mainfrom
refactor/deploy-coreapp
Open

refactor(deploy): restructure core app deploy and extract shared deploy checks#1406
gu-stav wants to merge 8 commits into
mainfrom
refactor/deploy-coreapp

Conversation

@gu-stav

@gu-stav gu-stav commented Jun 30, 2026

Copy link
Copy Markdown
Member

Description

Note

Lays the foundation for the --dry-run flag for the deploy command. I've created another stacked PR that applies the same refactoring for studios, to keep this one smaller and easier to review.

The deploy command had grown into one long function mixing validation, prompting, building, schema upload, packaging and reporting — hard to follow and impossible to reuse for a read-only check.

This is the first of two PRs that restructure it. It covers the core app deploy and lands the shared scaffolding the studio refactor (follow-up) builds on.

deployApp now runs through a single createAppDeployment that validates, prepares and ships the deployment, reporting through a small checks seam. The reusable pieces live in their own modules so the studio path can adopt them next: the checks seam and step helpers, read-only deploy-target resolution, and one home for the auto-update rules. Workbench-specific view validation moves out of the CLI into @sanity/workbench-cli.

The checks seam mirrors the sanity doctor command: each step reports a pass/warn/fail outcome to a reporter, and the mode decides what a failure means — a real deploy stops at the first one, a dry run collects them all.

Behavior-preserving, with one deliberate exception: assertDeployable runs before the other checks now, so its "nothing to deploy" error keeps its USAGE_ERROR exit code. For an app that's both non-deployable and missing config (sdk-react/org id), that error now surfaces first.

What to review

  • createAppDeployment against the old deployApp: prompts, spinners, error messages and exit codes should be unchanged.
  • The shared modules (deployChecks, resolveDeployTarget), and that the studio deploy still works on its existing code.

Testing

The integration deploy tests pass unchanged. New unit tests cover the checks seam, app target resolution, and the auto-update check mapping.


Note

Medium Risk
Touches the core app deploy path and user-application resolution/API flow; behavior is intended to be preserved but ordering changes (e.g. assertDeployable first) and new unattended errors need careful review.

Overview
Refactors core app sanity deploy into a linear createAppDeployment flow where validation, build, output checks, manifest/title sync, and upload each report through a shared checks seam (CheckReporter: fail-fast for real deploys, collecting for future dry-run).

Adds reusable deploy plumbing: deployChecks (package version, auto-updates, build, output dir, app target), read-only resolveAppDeployTarget, and pure auto-update resolution in shouldAutoUpdate (resolveAutoUpdates + shared messages). findUserApplicationForApp now maps those verdicts to prompts and supports unattended deploy via flags.yes (errors instead of select/title prompts). The deploy command propagates unattended mode from --yes or a non-interactive terminal and skips the non-empty output-dir confirm when unattended.

Workbench: checkBuiltOutput is a standalone export from @sanity/workbench-cli/deploy; getWorkbench keeps assertDeployable and gains buildViewDeploymentPayload while the CLI removes inline view-payload logging from app deploy. Studio deploy only switches to the exported checkBuiltOutput. Manifest title sync uses new resolveTitleUpdate.

Reviewed by Cursor Bugbot for commit cb42bf2. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

📦 Bundle Stats — @sanity/cli

Compared against main (e56340ae)

@sanity/cli

Metric Value vs main (e56340a)
Internal (raw) 2.7 KB -
Internal (gzip) 1.0 KB -
Bundled (raw) 11.16 MB -
Bundled (gzip) 2.10 MB -
Import time 882ms -8ms, -0.9%

bin:sanity

Metric Value vs main (e56340a)
Internal (raw) 782 B -
Internal (gzip) 423 B -
Bundled (raw) 9.87 MB -
Bundled (gzip) 1.78 MB -
Import time 2.31s -7ms, -0.3%

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

Metric Value vs main (e56340a)
Internal (raw) 106.7 KB -
Internal (gzip) 26.7 KB -
Bundled (raw) 21.72 MB -
Bundled (gzip) 3.46 MB -
Import time 790ms +8ms, +1.0%

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

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

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Coverage Delta

File Statements
packages/@sanity/cli/src/actions/build/shouldAutoUpdate.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/deploy/deployChecks.ts 61.5% (new)
packages/@sanity/cli/src/actions/deploy/resolveDeployTarget.ts 33.3% (new)
packages/@sanity/cli/src/actions/manifest/extractCoreAppManifest.ts 94.4% (+ 0.5%)
packages/@sanity/cli/src/util/errorMessages.ts 100.0% (±0%)
packages/@sanity/workbench-cli/src/actions/deploy/checkBuiltOutput.ts 100.0% (new)
packages/@sanity/workbench-cli/src/actions/deploy/getWorkbench.ts 87.5% (- 12.5%)
packages/@sanity/workbench-cli/src/actions/deploy/viewDeployment.ts 66.7% (new)

Comparing 8 changed files against main @ e56340aea13282b039ed01625801c5268fee9964

Overall Coverage

Metric Coverage
Statements 74.4% (- 0.1%)
Branches 64.3% (- 0.0%)
Functions 68.9% (- 0.0%)
Lines 75.0% (- 0.1%)

@gu-stav gu-stav force-pushed the refactor/deploy-coreapp branch 4 times, most recently from 2ba1b26 to 9e824bc Compare June 30, 2026 15:15
@gu-stav gu-stav marked this pull request as ready for review June 30, 2026 15:29
@gu-stav gu-stav requested a review from a team as a code owner June 30, 2026 15:29
@gu-stav gu-stav requested review from filmaj and snocorp June 30, 2026 15:30
@gu-stav

This comment was marked as outdated.

@squiggler-app

This comment was marked as outdated.

Comment thread packages/@sanity/cli/src/actions/deploy/deployChecks.ts
gu-stav added 2 commits June 30, 2026 17:48
…oy checks

The app deploy command had grown into one long interleaved function.
Restructure it into a single createAppDeployment that validates, prepares and
ships, reporting through a shared checks seam, so the command reads as one
clear call.

This also lands the shared scaffolding the studio deploy will reuse: the checks
seam and step helpers (deployChecks), read-only target resolution
(resolveDeployTarget), and one home for the auto-update rules
(resolveAutoUpdates/getAutoUpdateIssueMessage). Workbench-specific validation
moves out of the CLI: viewDeployment goes into @sanity/workbench-cli.

Tighten the deploy seams while here: verifyOutputDir takes an isWorkbenchApp
boolean instead of a workbench object, checkBuiltOutput becomes a standalone
@sanity/workbench-cli/deploy export rather than a method on getWorkbench, and
DeployTarget models isExternal as its own field instead of folding it into the
type. Adds unit coverage for resolveDeployTarget and resolveTitleUpdate.

No behavior change — the integration deploy tests pass unchanged.
@gu-stav gu-stav force-pushed the refactor/deploy-coreapp branch from 9e824bc to 26139a7 Compare June 30, 2026 15:48
Comment thread packages/@sanity/cli/src/actions/deploy/deployApp.ts Outdated
@gu-stav gu-stav requested a review from joshuaellis July 1, 2026 07:37
The DeployChecks {add, all, run} interface was hard to follow: each method
behaved differently per mode, and two were no-ops or pass-throughs in the
fail-fast mode that actually runs today (all() returned [], run was identity).

The modes differ in one thing — what a reported check does — so collapse the
seam to a single CheckReporter {report}. A real deploy fails fast
(createFailFastReporter); a dry run collects (createCollectingReporter); the
error-to-fail conversion lives in one shared runStep. verifyOutputDir now
reports through the reporter like every other check, and createAppDeployment
documents the one-sequence-two-modes control flow.

Mirrors the shape of sanity doctor's runDoctorChecks (checks produce results, a
runner decides) as its fail-fast sibling, without sharing code — doctor collects
every check, deploy aborts on the first failure of a dependent sequence.

A build failure now surfaces as "Build failed: <err>" (the formatError that was
dead on the real-deploy path) rather than the generic deploy error.
The view/service deployment payload is validated and logged but never
persisted (the storing service doesn't exist yet). Remove it for now; proper
view/service handling will be added later.
Comment thread packages/@sanity/cli/src/actions/deploy/deployApp.ts
joshuaellis
joshuaellis previously approved these changes Jul 1, 2026

@joshuaellis joshuaellis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks fine, would be good to get @snocorp's eyes on it as well

Comment thread packages/@sanity/workbench-cli/src/_exports/deploy.ts Outdated
Drop the unused DeployCheck.name field (nothing read it) and the checkPackageVersion
`name` param it required. Make checkAppTarget report-only (Promise<void>) and delete
the DeployTarget descriptor — production never consumed the returned target, only the
reported check.
gu-stav added 2 commits July 1, 2026 13:52
Adds an optional `solution` to DeployCheck and populates it for the blockers
with a clear fix (missing package version, build failure, output dir, unresolved
app target). The dry-run report renders these under each problem.
snocorp
snocorp previously approved these changes Jul 1, 2026

@snocorp snocorp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, I left a couple of suggestions

Comment thread .changeset/refactor-core-app-deploy.md
Comment thread packages/@sanity/cli/test/integration/commands/deploy.app.test.ts Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1a67719. Configure here.

Comment thread packages/@sanity/cli/src/actions/deploy/deployApp.ts
`sanity deploy` for a core app could block on a prompt in CI or with --yes: the
app-target resolver prompted to pick or name an application, and the custom
output-dir overwrite confirm ignored unattended mode. Detect unattended via
`isUnattended()` (--yes or a non-TTY terminal), like the rest of the CLI, and
error with guidance instead of prompting.

Also addresses review feedback: patch @sanity/cli in the changeset, and adopt the
vi.mock(import(...)) style for the mocks that spread the original module.
@gu-stav gu-stav force-pushed the refactor/deploy-coreapp branch from 1a67719 to cb42bf2 Compare July 2, 2026 08:05
@gu-stav

gu-stav commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

Addressed all review comments ✅ One of the other PRs also surfaced I shouldn't look at the --yes flag directly and instead use isUnattended(), which I've changed too.

This makes it easier, to add --json as new unattended indicator, like Rune asked and is more aligned with the rest of the codebase.

@gu-stav gu-stav requested review from joshuaellis and snocorp July 2, 2026 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants