From 33c4bb90e06155b3bb145b26dd19e7efcea339a4 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Thu, 16 Jul 2026 00:24:57 -0400 Subject: [PATCH] =?UTF-8?q?Revert=20"feat:=20keep=20the=20sandbox=20provid?= =?UTF-8?q?er=20step=20visible=20when=20runtime=20env=20satisfi=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit eebaedd073ee4bb33973595ad2c2ca6cd7a94781. --- .../setup/StepComputeConfig.client.test.tsx | 92 ------- .../(onboarding)/setup/StepComputeConfig.tsx | 29 +-- .../(onboarding)/setup/hooks.client.test.tsx | 244 +----------------- apps/web/src/app/(onboarding)/setup/hooks.ts | 39 +-- .../src/trpc/commands/setup-new/index.test.ts | 28 -- apps/web/src/trpc/commands/setup-new/index.ts | 30 +-- packages/types/src/setup-compute-config.ts | 1 - 7 files changed, 25 insertions(+), 438 deletions(-) diff --git a/apps/web/src/app/(onboarding)/setup/StepComputeConfig.client.test.tsx b/apps/web/src/app/(onboarding)/setup/StepComputeConfig.client.test.tsx index fb0f841df..5e5d96abf 100644 --- a/apps/web/src/app/(onboarding)/setup/StepComputeConfig.client.test.tsx +++ b/apps/web/src/app/(onboarding)/setup/StepComputeConfig.client.test.tsx @@ -193,18 +193,6 @@ describe('StepComputeConfig', () => { hostedImageRef: null, hostedReady: false, }, - // The base image ref derives from the worker image, so it cannot - // be default-satisfied when no hosted-ready image exists. - providers: [ - { - ...buildHostedProvider(), - fields: buildHostedProvider().fields.map((field) => - field.envVarName === 'MODAL_BASE_IMAGE_REF' - ? { ...field, defaultSatisfied: false } - : field, - ), - }, - ], })} selectedProviderId="modal" onContinue={vi.fn()} @@ -247,15 +235,6 @@ describe('StepComputeConfig', () => { defaultSatisfied: false, setupProvisionable: false, }, - { - envVarName: 'E2B_TEMPLATE_ID', - label: 'Worker Template', - category: 'infrastructure', - runtimeSatisfied: false, - savedSatisfied: false, - defaultSatisfied: false, - setupProvisionable: true, - }, { envVarName: 'E2B_DOMAIN', label: 'E2B Domain', @@ -284,77 +263,6 @@ describe('StepComputeConfig', () => { ).toBeDisabled(); }); - it('lets an already-provisioned provider continue despite a local worker image', () => { - // Recovery path: the config step re-confirms an already-configured - // provider to commit it as the dispatch default. Saving touches no - // managed artifacts (the template already exists), so a locked - // local-tag worker image must not dead-end the flow. - render( - , - ); - - expect( - screen.getByRole('button', { name: /continue|save and continue/i }), - ).toBeEnabled(); - // The missing-image warning must not contradict the enabled Continue. - expect( - screen.queryByText(/registry-qualified worker image/i), - ).not.toBeInTheDocument(); - }); - it('continues onboarding while a Blaxel image build runs in the background', async () => { mockSetupStatus.current = { setupNewState: { diff --git a/apps/web/src/app/(onboarding)/setup/StepComputeConfig.tsx b/apps/web/src/app/(onboarding)/setup/StepComputeConfig.tsx index 1772da546..217fd7e21 100644 --- a/apps/web/src/app/(onboarding)/setup/StepComputeConfig.tsx +++ b/apps/web/src/app/(onboarding)/setup/StepComputeConfig.tsx @@ -170,29 +170,7 @@ export function StepComputeConfig({ deriveModalBaseImageRefDefault(workerImageValue) !== null; const hostedWorkerImageReady = workerImage.hostedReady || submittedHostedWorkerImageReady; - // Hosted providers need a pullable (registry-qualified) worker image, but - // only when saving would actually derive or provision from it — Modal/E2B/ - // Daytona build their base image, template, or snapshot from that image - // server-side. When every required managed artifact is already satisfied, - // saving touches none of them: this is the recovery path that re-confirms - // an already-configured provider to commit it as the dispatch default, and - // it must not dead-end on (or warn about) a locked local-tag worker image - // with no Back route. - const pendingManagedInfrastructure = - selectedProvider?.fields.some( - (field) => - isComputeInfrastructureField(field) && - !isComputeOperatorEditableField(field) && - field.required !== false && - !field.runtimeSatisfied && - !field.savedSatisfied && - !field.defaultSatisfied, - ) ?? true; - // Drives the missing-image warning, the auto-opened advanced section, and - // the Continue guard together so the form never blocks or warns on a - // worker image that saving would not use. - const missingHostedWorkerImage = - isHostedProvider && pendingManagedInfrastructure && !hostedWorkerImageReady; + const missingHostedWorkerImage = isHostedProvider && !hostedWorkerImageReady; const canEditAdvancedWorkerImage = isHostedProvider && !workerImage.runtimeSatisfied; const shouldRenderAdvancedWorkerImage = @@ -205,7 +183,10 @@ export function StepComputeConfig({ ? getComputeCredentialsHint(selectedProvider.provider) : null; - const hostedRequirementMet = !missingHostedWorkerImage; + // Hosted providers need a pullable (registry-qualified) worker image. A bare + // process-env local tag must not enable Save — Modal/E2B/Daytona derive or + // provision from that image server-side, not from form base-image fields. + const hostedRequirementMet = !isHostedProvider || hostedWorkerImageReady; const credentialsMet = credentialFields.every( (field) => diff --git a/apps/web/src/app/(onboarding)/setup/hooks.client.test.tsx b/apps/web/src/app/(onboarding)/setup/hooks.client.test.tsx index 9bf8796b4..20160ea8c 100644 --- a/apps/web/src/app/(onboarding)/setup/hooks.client.test.tsx +++ b/apps/web/src/app/(onboarding)/setup/hooks.client.test.tsx @@ -369,246 +369,6 @@ describe('useSetupFlow', () => { expect(result.current.step).toBe('source-control-provider'); }); - it('shows compute-provider even when runtime env satisfies a hosted provider', async () => { - // A deployment that preconfigures a sandbox provider (e.g. Roomote - // Sandbox seeded by the hosting operator): the picker still renders - // until the user actually chooses. - mockStatus({ - hasSlack: true, - authSetup: { - setupSatisfiedByRuntimeEnv: false, - selectedProvider: 'slack', - preselectedProvider: 'slack', - runtimeConfiguredProvider: null, - runtimeConfiguredProviders: [], - lockReason: null, - providers: [ - { - id: 'slack', - label: 'Slack', - fields: [], - runtimeSatisfied: true, - savedSatisfied: false, - setupSatisfied: true, - }, - ], - }, - modelSetup: { - setupSatisfied: true, - setupSatisfiedByRuntimeEnv: true, - preselectedProvider: 'openrouter', - }, - sourceControlSetup: { - setupSatisfied: true, - setupSatisfiedByRuntimeEnv: false, - selectedProvider: 'github', - preselectedProvider: 'github', - runtimeConfiguredProvider: null, - runtimeConfiguredProviders: [], - lockReason: null, - connectedProvider: 'github', - providers: [], - }, - computeSetup: { - setupSatisfied: true, - setupSatisfiedByRuntimeEnv: true, - selectedProvider: null, - preselectedProvider: 'roomote', - runtimeDefaultProvider: 'roomote', - persistedDefaultProvider: null, - providers: [ - { - provider: 'roomote', - label: 'Roomote Sandbox', - description: '', - supportsSnapshots: true, - fields: [], - runtimeConfigSatisfied: true, - savedConfigSatisfied: false, - configSatisfied: true, - }, - ], - }, - setupNewState: { - authProvider: 'slack', - modelProvider: 'openrouter', - computeProvider: null, - sourceControlProvider: 'github', - selectedRepositoryIds: [], - onboardingTaskId: null, - onboardingTaskStartedAt: null, - slackChannel: null, - slackThreadTs: null, - }, - }); - - const { result } = renderHook(() => useSetupFlow()); - - await waitFor(() => { - expect(result.current.step).toBe('compute-provider'); - }); - }); - - it('does not let a runtime-satisfied provider skip config for a different chosen provider', async () => { - // Seeded deployment (roomote satisfied via env) where the user chose - // bring-your-own E2B: its config step must still render. - mockStatus({ - hasSlack: true, - authSetup: { - setupSatisfiedByRuntimeEnv: false, - selectedProvider: 'slack', - preselectedProvider: 'slack', - runtimeConfiguredProvider: null, - runtimeConfiguredProviders: [], - lockReason: null, - providers: [ - { - id: 'slack', - label: 'Slack', - fields: [], - runtimeSatisfied: true, - savedSatisfied: false, - setupSatisfied: true, - }, - ], - }, - modelSetup: { - setupSatisfied: true, - setupSatisfiedByRuntimeEnv: true, - preselectedProvider: 'openrouter', - }, - sourceControlSetup: { - setupSatisfied: true, - setupSatisfiedByRuntimeEnv: false, - selectedProvider: 'github', - preselectedProvider: 'github', - runtimeConfiguredProvider: null, - runtimeConfiguredProviders: [], - lockReason: null, - connectedProvider: 'github', - providers: [], - }, - computeSetup: { - setupSatisfied: true, - setupSatisfiedByRuntimeEnv: true, - selectedProvider: 'e2b', - preselectedProvider: 'e2b', - runtimeDefaultProvider: 'roomote', - persistedDefaultProvider: null, - providers: [ - { - provider: 'e2b', - label: 'E2B', - description: '', - supportsSnapshots: true, - comment: 'Recommended', - fields: [], - runtimeConfigSatisfied: false, - savedConfigSatisfied: false, - configSatisfied: false, - }, - ], - }, - setupNewState: { - authProvider: 'slack', - modelProvider: 'openrouter', - computeProvider: 'e2b', - sourceControlProvider: 'github', - selectedRepositoryIds: [], - onboardingTaskId: null, - onboardingTaskStartedAt: null, - slackChannel: null, - slackThreadTs: null, - }, - }); - - const { result } = renderHook(() => useSetupFlow()); - - await waitFor(() => { - expect(result.current.step).toBe('compute-config'); - }); - }); - - it('keeps compute-config for a configured provider that is not yet the dispatch default', async () => { - // configSatisfied alone must not skip config: until the runtime default - // commits, dispatch still targets the previous provider. - mockStatus({ - hasSlack: true, - authSetup: { - setupSatisfiedByRuntimeEnv: false, - selectedProvider: 'slack', - preselectedProvider: 'slack', - runtimeConfiguredProvider: null, - runtimeConfiguredProviders: [], - lockReason: null, - providers: [ - { - id: 'slack', - label: 'Slack', - fields: [], - runtimeSatisfied: true, - savedSatisfied: false, - setupSatisfied: true, - }, - ], - }, - modelSetup: { - setupSatisfied: true, - setupSatisfiedByRuntimeEnv: true, - preselectedProvider: 'openrouter', - }, - sourceControlSetup: { - setupSatisfied: true, - setupSatisfiedByRuntimeEnv: false, - selectedProvider: 'github', - preselectedProvider: 'github', - runtimeConfiguredProvider: null, - runtimeConfiguredProviders: [], - lockReason: null, - connectedProvider: 'github', - providers: [], - }, - computeSetup: { - setupSatisfied: true, - setupSatisfiedByRuntimeEnv: false, - selectedProvider: 'modal', - preselectedProvider: 'modal', - runtimeDefaultProvider: 'docker', - persistedDefaultProvider: null, - providers: [ - { - provider: 'modal', - label: 'Modal', - description: '', - supportsSnapshots: true, - comment: 'Recommended', - fields: [], - runtimeConfigSatisfied: false, - savedConfigSatisfied: true, - configSatisfied: true, - }, - ], - }, - setupNewState: { - authProvider: 'slack', - modelProvider: 'openrouter', - computeProvider: 'modal', - sourceControlProvider: 'github', - selectedRepositoryIds: [], - onboardingTaskId: null, - onboardingTaskStartedAt: null, - slackChannel: null, - slackThreadTs: null, - }, - }); - - const { result } = renderHook(() => useSetupFlow()); - - await waitFor(() => { - expect(result.current.step).toBe('compute-config'); - }); - }); - it('shows compute-provider after source control when compute setup is pending', async () => { mockStatus({ hasSlack: true, @@ -858,9 +618,7 @@ describe('useSetupFlow', () => { selectedProvider: 'docker', preselectedProvider: 'docker', runtimeDefaultProvider: null, - // Choosing a credentialless provider commits the runtime default in - // the same mutation, so the refetched status already reflects it. - persistedDefaultProvider: 'docker', + persistedDefaultProvider: null, providers: [ { provider: 'docker', diff --git a/apps/web/src/app/(onboarding)/setup/hooks.ts b/apps/web/src/app/(onboarding)/setup/hooks.ts index 1126ee1e0..0a48f8664 100644 --- a/apps/web/src/app/(onboarding)/setup/hooks.ts +++ b/apps/web/src/app/(onboarding)/setup/hooks.ts @@ -412,18 +412,13 @@ export function useSetupFlow( case 'qualification-blocked': return activeQualificationBlock === null; case 'compute-provider': - // Runtime env vars alone must not count as a choice: a deployment - // that preconfigures a sandbox provider (e.g. Roomote Sandbox) - // still renders the picker so the user sees what they are getting - // and that alternatives exist. selectedComputeProvider only - // reflects genuine choices — the wizard pick or a persisted - // default — mirroring the communication/source-control steps. - return !hasStaleComputeProvider && selectedComputeProvider !== null; + return ( + !hasStaleComputeProvider && + (status.computeSetup.setupSatisfied || + selectedComputeProvider !== null) + ); case 'compute-config': { - // Scoped to the chosen provider: a deployment-wide setupSatisfied - // (another provider fully configured by env vars) must not skip - // configuration for an unconfigured provider the user picked. - if (hasStaleComputeProvider) { + if (hasStaleComputeProvider || status.computeSetup.setupSatisfied) { return true; } @@ -435,13 +430,6 @@ export function useSetupFlow( (provider) => provider.provider === selectedComputeProvider, ); - // A chosen provider missing from the offered list has no config - // step to render; the stale-choice guard above already covers - // exclusions, so a lookup miss means nothing to configure. - if (!computeProviderStatus) { - return true; - } - if ( isSetupProvisionableComputeProvider(selectedComputeProvider) && getSetupNewComputeProvisioningState( @@ -452,20 +440,7 @@ export function useSetupFlow( return true; } - // configSatisfied proves the provider can run, not that dispatch - // targets it: the runtime default commits with the config - // confirmation (or with the choice itself when it is already - // configured). Skip only once the chosen provider is the - // effective default, so a canonical /setup re-entry cannot - // advance past config while dispatch still uses the old default. - const effectiveDefaultComputeProvider = - status.computeSetup.persistedDefaultProvider ?? - status.computeSetup.runtimeDefaultProvider; - - return ( - computeProviderStatus.configSatisfied && - selectedComputeProvider === effectiveDefaultComputeProvider - ); + return computeProviderStatus?.configSatisfied ?? false; } case 'slack': if (communicationStepResolved) { diff --git a/apps/web/src/trpc/commands/setup-new/index.test.ts b/apps/web/src/trpc/commands/setup-new/index.test.ts index fac9b976c..ba727b3f8 100644 --- a/apps/web/src/trpc/commands/setup-new/index.test.ts +++ b/apps/web/src/trpc/commands/setup-new/index.test.ts @@ -678,34 +678,6 @@ describe('setup-new compute config commands', () => { expect(result.runtimeComputeConfig.defaultProvider).toBe('docker'); }); - it('commits an already-configured hosted provider as the runtime default when chosen', async () => { - vi.stubEnv('MODAL_TOKEN_ID', 'token-id'); - vi.stubEnv('MODAL_TOKEN_SECRET', 'token-secret'); - vi.stubEnv('MODAL_BASE_IMAGE_REF', 'ghcr.io/roomote/modal-worker:test'); - - const result = await saveSetupNewComputeProviderChoiceCommand( - buildMockAuth(), - { - provider: 'modal', - }, - ); - - expect(result.setupNewState.computeProvider).toBe('modal'); - expect(result.runtimeComputeConfig.defaultProvider).toBe('modal'); - }); - - it('leaves the runtime default unchanged for an unconfigured hosted provider choice', async () => { - const result = await saveSetupNewComputeProviderChoiceCommand( - buildMockAuth(), - { - provider: 'modal', - }, - ); - - expect(result.setupNewState.computeProvider).toBe('modal'); - expect(result.runtimeComputeConfig?.defaultProvider ?? null).toBeNull(); - }); - it('rejects an excluded provider choice', async () => { vi.stubEnv('EXCLUDED_COMPUTE_PROVIDERS', 'docker'); diff --git a/apps/web/src/trpc/commands/setup-new/index.ts b/apps/web/src/trpc/commands/setup-new/index.ts index 0f80a3707..001d8a7bc 100644 --- a/apps/web/src/trpc/commands/setup-new/index.ts +++ b/apps/web/src/trpc/commands/setup-new/index.ts @@ -1626,24 +1626,18 @@ export async function saveSetupNewComputeProviderChoiceCommand( const hasCredentialFields = providerStatus.fields.some( isComputeCredentialField, ); - // Credential-backed providers normally commit the runtime default when - // their config step is confirmed, so merely browsing a hosted provider - // must not switch the deployment onto it. When the chosen provider is - // already fully configured there is nothing left for the config step to - // collect (the wizard skips it), so the choice itself is the - // confirmation and commits the default — otherwise the wizard would - // advance while dispatch still targets the previous default. - // Credentialless providers such as Local Docker have no config step to - // confirm and commit immediately for the same reason. - const commitsRuntimeDefault = - !hasCredentialFields || providerStatus.configSatisfied; - const runtimeComputeConfig = commitsRuntimeDefault - ? normalizeDeploymentComputeConfig({ + const runtimeComputeConfig = hasCredentialFields + ? persistedRuntimeComputeConfig + : normalizeDeploymentComputeConfig({ ...persistedRuntimeComputeConfig, defaultProvider: input.provider, - }) - : persistedRuntimeComputeConfig; + }); + // Providers with credentials are only recorded as the wizard choice here. + // The runtime default commits when their config step is confirmed, so + // merely browsing a hosted provider must not switch the deployment onto it. + // Credentialless providers such as Local Docker have no config step to + // confirm, so choosing them commits the runtime default immediately. const setupNewState = normalizeSetupNewState({ ...currentState, computeProvider: input.provider, @@ -1652,9 +1646,9 @@ export async function saveSetupNewComputeProviderChoiceCommand( await Promise.all([ savePersistedSetupNewState(setupNewState, tx), - ...(commitsRuntimeDefault - ? [savePersistedRuntimeComputeConfig(runtimeComputeConfig, tx)] - : []), + ...(hasCredentialFields + ? [] + : [savePersistedRuntimeComputeConfig(runtimeComputeConfig, tx)]), ]); return { diff --git a/packages/types/src/setup-compute-config.ts b/packages/types/src/setup-compute-config.ts index fb588b49d..ef33b2818 100644 --- a/packages/types/src/setup-compute-config.ts +++ b/packages/types/src/setup-compute-config.ts @@ -238,7 +238,6 @@ export const SETUP_COMPUTE_PROVIDER_CATALOG = [ { provider: 'roomote', label: 'Roomote Sandbox', - comment: 'Works out of the box', description: 'Managed sandboxes preconfigured by your deployment, with snapshot support. Nothing to set up.', supportsSnapshots: true,