Fix inconsistent UI state and concurrent setup flow in Toolbox wizard#302
Merged
Conversation
Toolbox currently relies on getOverrideUiPage to decide whether to render a custom page or fall back to environments, but this logic is stateless and leads to inconsistent behavior when the window is closed and reopened. If the user exits Toolbox mid-setup (e.g., during CLI download in ConnectStep) and reopens it, a new wizard instance is created even though the previous setup process is still running in the background. This results in multiple overlapping setup flows, race conditions, and UI inconsistencies such as: - Returning to the initial URL step while setup is still in progress - Conflicting login attempts when restarting the wizard - Background processes (CLI download, HTTP setup) continuing without visible UI - Security dialogs triggered due to shared/overwritten resources - Undefined behavior when switching deployments mid-setup The root cause is lack of persistent, centralized state management for the setup flow. Current reliance on static state (e.g., CoderSetupWizardState) and recomputation via getOverrideUiPage does not properly track in-progress operations or restore the correct UI state. - resolves https://linear.app/codercom/issue/DEVEX-223/
URI handler now schedules pages to be displayed via pager router and at the end trigger the window to show up which instead it will call getOverrideUiPage.
in order to abstract even further some of the steps necessary for creating a step and navigating to it.
ConnectStep should not care if it was it is the first screen in the login process (auto login) or not. Instead, the wizard page should orchestrate the behavior of the back button depending on whether it is auto login or not
jeremyruppel
approved these changes
May 13, 2026
jeremyruppel
left a comment
jeremyruppel
left a comment
There was a problem hiding this comment.
one small comment about the delay but the rest of this looks great! 👍
with a different URL from the one that is already opened.
but the user disabled the "Prefer OAuth2 option..."
Route OAuth callbacks through a pending OAuth connection instead of reading wizard model state directly, then create and schedule a fresh connect-step wizard after token exchange.
There was a problem hiding this comment.
Pull request overview
This PR introduces per-provider, persistent routing/state for the Coder setup wizard to prevent overlapping setup flows and inconsistent UI when the Toolbox window is closed/reopened or when deep links/OAuth callbacks arrive mid-setup.
Changes:
- Added a
PageRouterto persist and reuse a single wizard instance across visibility cycles, with explicit disposal when replaced. - Replaced global singleton wizard state (
CoderSetupWizardState/CoderSetupWizardContext) with per-wizardWizardModel. - Updated deep-link and OAuth callback handling to drive routing via the router and rebuild the wizard in the correct step with the correct credentials.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt | Uses PageRouter to persist/replace wizard pages and to safely handle deep links + OAuth callbacks without spawning overlapping flows. |
| src/main/kotlin/com/coder/toolbox/views/CoderSetupWizardPage.kt | Converts wizard to use WizardModel, adds disposal, and provides factory constructors for URL vs connect step startup. |
| src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt | Migrates to WizardModel and adds dispose(), updates navigation behavior. |
| src/main/kotlin/com/coder/toolbox/views/DeploymentUrlStep.kt | Migrates URL/OAuth session state to WizardModel. |
| src/main/kotlin/com/coder/toolbox/views/TokenStep.kt | Migrates token handling to WizardModel. |
| src/main/kotlin/com/coder/toolbox/views/state/PageRouter.kt | New centralized router to preserve the active wizard page across Toolbox lifecycle changes. |
| src/main/kotlin/com/coder/toolbox/views/state/WizardModel.kt | New per-wizard model for step + credential state, plus supporting types. |
| src/main/kotlin/com/coder/toolbox/views/state/OAuthSessionContext.kt | Extracted OAuth session types and added StoredOAuthSession.toSessionContext(). |
| src/main/kotlin/com/coder/toolbox/views/state/CoderSetupWizardState.kt | Removed singleton wizard step state. |
| src/main/kotlin/com/coder/toolbox/views/state/CoderSetupWizardContext.kt | Removed singleton wizard context (URL/token/OAuth session). |
| src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt | Adds coverage for router overrides and credential precedence in auto-setup. |
| src/test/kotlin/com/coder/toolbox/views/state/PageRouterTest.kt | Adds unit tests for PageRouter.pendingOAuthConnection behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The error never reached the notifier because of the lambda return statement.
connect() catches Exception and always calls navigateBack(). This includes CancellationException triggered by onBack() and dispose(), which means navigation can happen twice (once in onBack()'s finally, and again in the catch), and dispose() no longer actually cancels “without navigating” as the docstring states. Handle CancellationException separately (and avoid calling navigateBack() for dispose / already-handled back navigation), or rethrow cancellation to stop the coroutine without side effects.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Toolbox currently relies on getOverrideUiPage to decide whether to render a custom page or fall back to environments, but this logic is stateless and leads to inconsistent behavior when the window is closed and reopened.
If the user exits Toolbox mid-setup (e.g., during CLI download in ConnectStep) and reopens it, a new wizard instance is created even though the previous setup process is still running in the background. This results in multiple overlapping setup flows, race conditions, and UI inconsistencies such as:
The root cause is lack of persistent, centralized state management for the setup flow. Current reliance on static state (e.g., CoderSetupWizardState) and recomputation via getOverrideUiPage does not properly track in-progress operations or restore the correct UI state.