Skip to content

feat: keep the sandbox provider step visible when runtime env satisfies it#403

Merged
mrubens merged 5 commits into
developfrom
feat/setup-compute-explicit-choice
Jul 16, 2026
Merged

feat: keep the sandbox provider step visible when runtime env satisfies it#403
mrubens merged 5 commits into
developfrom
feat/setup-compute-explicit-choice

Conversation

@mrubens

@mrubens mrubens commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Stacked on #402 (base branch is feat/roomote-cloud-compute-provider; retarget to develop after it merges).

What

Mirrors #388's communication/source-control treatment for the compute step, which was the remaining step that silently vanished when env vars preconfigured a provider:

  • compute-provider completes only on a genuine choice (the wizard pick or a persisted Settings default) — runtime satisfaction alone no longer skips it. A deployment that preconfigures a sandbox provider still shows the picker with that provider preselected: one click, nothing to fill in, but the user sees what they're getting and that alternatives exist.
  • compute-config skip is scoped to the chosen provider. Previously the deployment-wide setupSatisfied short-circuit meant that on a deployment with a preconfigured provider, choosing an unconfigured bring-your-own provider (e.g. E2B) skipped its credential step entirely — the same scoping bug [Improve] Let users choose a setup provider even when its env vars are already configured #388 fixed for source control.
  • A chosen provider missing from the offered providers list is treated as having nothing to configure (the stale-choice guard already covers exclusions), preserving behavior for under-specified statuses.

No changes to the picker UI or persistence — the existing saveComputeProviderChoice flow already persists the choice and short-paths credentialless providers past the config step.

Tests

Two new useSetupFlow cases: the seeded-deployment picker-still-renders scenario, and the chosen-unconfigured-provider-still-configures scenario. Full web client project passes (201 files / 1395 tests); lint and check-types clean.

@roomote-roomote

roomote-roomote Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

No new code issues found. See task

  • apps/web/src/app/(onboarding)/setup/hooks.ts:465 — Re-entry can strand an already-configured hosted provider before its dispatch default is committed. For example, with saved E2B credentials/template, an old Docker default, and a runtime DOCKER_WORKER_IMAGE that is only a local tag, this new default-match check opens compute-config; the existing form disables Continue because no hosted-ready worker image exists and hides the worker-image input because the runtime value is locked. All earlier steps are skipped, so there is no Back route, even though the server would accept the already-provisioned artifact and use this confirmation to commit E2B as the default. Require a hosted worker image only when provisioning is actually pending so this recovery path can complete.
  • apps/web/src/app/(onboarding)/setup/StepComputeConfig.tsx:204 — The new predicate scopes only the Continue-button guard. In the recovered already-provisioned state, missingHostedWorkerImage remains true, so the same screen enables Continue while still telling the operator to set the locked DOCKER_WORKER_IMAGE “before continuing.” Scope the missing-image warning/advanced section to pending managed infrastructure too, and assert the recovery test no longer renders that warning.

Reviewed 0782db0

Base automatically changed from feat/roomote-cloud-compute-provider to develop July 15, 2026 23:32
…es it

Mirrors the communication/source-control treatment from #388 for the
compute step: runtime env vars alone no longer count as a choice, so a
deployment that preconfigures a sandbox provider (e.g. Roomote Sandbox)
still renders the picker — preselected, one click, nothing to fill in —
instead of silently skipping it. The step completes on a genuine choice:
the wizard pick or a persisted Settings default.

Also scopes the compute-config skip to the chosen provider: a
deployment-wide setupSatisfied (another provider fully configured by env
vars) previously skipped configuration for an unconfigured provider the
user picked, e.g. bring-your-own E2B on a deployment with a seeded
managed provider. A chosen provider missing from the offered list has no
config step to render (the stale-choice guard covers exclusions), which
preserves behavior for statuses without provider entries.
@mrubens
mrubens force-pushed the feat/setup-compute-explicit-choice branch from aa200b6 to 179843e Compare July 15, 2026 23:48
configSatisfied only proves a provider can run, not that dispatch
targets it: saveComputeProviderChoice deliberately defers the runtime
default to the config confirmation for credential-backed providers, so
choosing an already-configured provider skipped config and advanced the
wizard while dispatch kept the previous default.

Both halves per review: the choice mutation now commits the default when
the chosen provider is already fully configured (the wizard skips its
config step, so the choice is the confirmation — same reasoning as the
existing credentialless path), and the compute-config skip additionally
requires the chosen provider to be the effective persisted/runtime
default, so a canonical /setup re-entry with pre-fix persisted state
renders config once and commits through its confirmation.
@mrubens

mrubens commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in fcd8be0c3 — took both halves of the suggestion since they protect different paths:

  • Choice mutation commits when already configured: saveComputeProviderChoice now treats a fully-configured chosen provider like the credentialless path and commits the runtime default in the same transaction — the wizard skips its config step, so the choice is the confirmation.
  • Skip requires the effective default to match: compute-config now skips only when the chosen provider is the effective persisted/runtime default, so a canonical /setup re-entry with pre-fix persisted state (choice recorded, default not committed) renders config once and commits through its confirmation instead of advancing while dispatch targets the old default.

New coverage: command tests for commit-on-configured-choice and no-commit-on-unconfigured-choice; a flow test pinning configured-but-not-default → compute-config. The Local Docker fixture now reflects the default the server persists with the choice. Full web client project green (201 files / 1399 tests).

The compute-config recovery path (re-confirming an already-configured
provider to commit it as the dispatch default) could dead-end: with
saved credentials and an already-provisioned artifact but a locked
local-tag DOCKER_WORKER_IMAGE, Continue stayed disabled on the hosted
worker-image requirement while every earlier step was skipped, leaving
no route out.

Saving only derives or provisions from the worker image when a required
managed artifact (base image ref, template id, snapshot name, image) is
still unsatisfied, so the requirement now applies exactly then. Fresh
hosted setups keep the guard; the E2B disable-test fixture gains the
managed template field it was missing (real catalogs always carry one),
and a new test pins the recovery path continuing on a local-tag image.
@mrubens

mrubens commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in 91b47868e: the hosted worker-image requirement in StepComputeConfig is now scoped to when saving would actually derive or provision from it — i.e. a required managed artifact (base image ref / template id / snapshot name / image) is still unsatisfied. The recovery path that re-confirms an already-provisioned provider (saved E2B creds + existing template, locked local-tag DOCKER_WORKER_IMAGE) can now complete and commit the dispatch default; fresh hosted setups keep the guard since their pending artifact genuinely needs a registry-qualified image.

Test updates: the fresh-E2B disable test's fixture gains the E2B_TEMPLATE_ID managed field it was missing (every real hosted catalog entry carries one — without it the fixture would read as nothing-to-provision), plus a new test pinning the recovery path continuing on a local-tag image. Full web client project green (201 files / 1400 tests).

The previous commit scoped only the Continue guard, so the recovered
already-provisioned state enabled Continue while still warning that the
locked DOCKER_WORKER_IMAGE must be set before continuing, with the
advanced section forced open. missingHostedWorkerImage now carries the
pending-managed-infrastructure predicate, driving the warning, the
auto-opened advanced section, and the Continue guard together.

The misleading-advanced-toggle test fixture now clears defaultSatisfied
on the base image ref when its worker image is not hosted-ready — those
cannot diverge in production since the default derives from that image —
and the recovery test asserts the warning no longer renders.
@mrubens

mrubens commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in d14076c83: missingHostedWorkerImage now carries the pending-managed-infrastructure predicate, so the warning, the auto-opened advanced section, and the Continue guard move together — the recovered already-provisioned state neither blocks nor warns, and hostedRequirementMet collapses to its negation. The recovery test asserts the warning no longer renders, and the misleading-advanced-toggle fixture now keeps MODAL_BASE_IMAGE_REF.defaultSatisfied consistent with its non-hosted-ready worker image (they cannot diverge in production — the default derives from that image).

Same catalog comment mechanism as Local Docker's 'Run on this host'.
@mrubens
mrubens merged commit eebaedd into develop Jul 16, 2026
17 checks passed
@mrubens
mrubens deleted the feat/setup-compute-explicit-choice branch July 16, 2026 00:31
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.

1 participant