Skip to content

[Improve] Let users choose a setup provider even when its env vars are already configured#388

Merged
brunobergher merged 4 commits into
developfrom
improve/onboarding-provider-choice-when-env-configured-1x91oj4d61fjh
Jul 15, 2026
Merged

[Improve] Let users choose a setup provider even when its env vars are already configured#388
brunobergher merged 4 commits into
developfrom
improve/onboarding-provider-choice-when-env-configured-1x91oj4d61fjh

Conversation

@roomote-roomote

@roomote-roomote roomote-roomote Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Opened on behalf of Bruno Bergher. Follow up by mentioning @roomote-roomote, in the web UI, or in Slack.

Related issue

N/A — internal Roomote work.

Why this PR exists

  • I am a maintainer / this is internal Roomote work

What changed

In the signed-in setup wizard, choosing a communication provider (e.g. Slack) or a source-control provider (e.g. GitHub) was skipped entirely whenever that provider was already configured by runtime env vars — the user was taken straight into that provider's sign-in flow with no choice.

Now the wizard always shows the provider chooser:

  • Runtime env vars alone no longer count as a made choice, so the auth-provider and source-control-provider chooser steps render even when a provider is fully configured by env vars.
  • If the user picks a provider that has no env vars, they configure it inline as before.
  • If the user picks a provider whose env vars are already set, they skip config and go straight to the connect/sign-up step.
  • The runtime-configured connect step copy changed from "Roomote needs access to your repositories to work on your codebase. Since <Provider> is already configured, let's go with it." to just "Connect to continue" (applies to both the GitHub App copy and the token-backed provider copy).
  • The connect step's "already configured" copy is now scoped to the provider the user actually landed on (configSatisfiedByRuntimeEnv) rather than a global lock flag, so choosing an unconfigured provider while a different provider has env vars set no longer shows the "already configured" wording.
  • The chosen communication provider is now persisted (saveAuthProviderChoice) and only marked pending/navigated on a successful save, so a reload keeps the user's choice even when the config step is auto-skipped for a runtime-configured provider, and a failed save never prematurely skips the chooser with an unsaved selection. Previously the comms choice lived only in local component state and could be lost on reload, which could then preselect a different provider. This mirrors how the source-control and compute choosers already persist their choice. Telegram stays on its existing UI-only path.
  • While the comms choice is saving, the provider buttons, Back, and the Do this later skip link are all disabled, so the user cannot navigate away mid-save and have the later save callback override that navigation.

Scope: signed-in setup wizard only. The signed-out first-admin bootstrap flow is intentionally left unchanged.

How it was tested

  • Updated and extended the wizard routing unit tests (hooks.client.test.tsx): the comms/source-control choosers are now shown when a provider is only runtime-configured with no saved choice, and the connect step is reached once a runtime-configured provider is chosen.
  • Updated the connect-step unit tests (StepSourceControlConnect.client.test.tsx) for the new "Connect to continue" copy, plus a regression test asserting that a selected unconfigured provider keeps its own copy when a different provider is runtime-configured.
  • Added StepAuthProvider tests covering the disabled-while-saving state for the provider buttons, Back, and the skip link.
  • Full setup-directory Vitest suite passes (167 tests); pnpm --filter @roomote/web check-types and pnpm lint pass.

Visual proof was not captured: reaching the runtime-configured connect/reload state in the seeded dev sandbox is not possible without fabricating contradictory setup state (no communication or source-control provider env vars are configured for this environment), and there is no matching Storybook story for these setup steps.

Checklist

  • The PR title follows the repo convention
  • This PR is small and scoped to one change
  • pnpm lint and pnpm check-types pass locally
  • I added tests or included a clear manual validation note above
  • I removed secrets, tokens, private keys, and customer data from code, logs, and screenshots
  • If this change should appear in the changelog, I ran pnpm changeset

@roomote-roomote

roomote-roomote Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

No new code issues found. See task

  • Wait for the communication-provider choice to save before exposing it as pending (apps/web/src/app/(onboarding)/setup/page.tsx:545). setPendingAuthProvider(provider) runs before saveAuthProviderChoice resolves, and useSetupFlow immediately treats that pending value as a completed chooser selection, so its auto-skip watchdog can leave the chooser (and skip runtime-satisfied config) while the request is still in flight. If the mutation fails, onError only shows a toast; the flow continues with the unsaved local choice, and a reload still loses it.
  • Disable Back and "Do this later" while the communication-provider choice is saving (apps/web/src/app/(onboarding)/setup/StepAuthProvider.tsx:74). The follow-up now keeps the chooser mounted until the save succeeds, but disabled only reaches the provider buttons; Back and skip remain active. Either can navigate away while the request is in flight, then onSuccess persists the provider and calls goToStep('auth-env-vars'), unexpectedly overriding the user's navigation (and, for skip, their explicit decision to defer setup).

Reviewed 8291afa

@brunobergher brunobergher marked this pull request as ready for review July 15, 2026 19:27
@brunobergher brunobergher merged commit 4eba626 into develop Jul 15, 2026
18 checks passed
@brunobergher brunobergher deleted the improve/onboarding-provider-choice-when-env-configured-1x91oj4d61fjh branch July 15, 2026 19:27
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.

2 participants