Repository navigation
[AI-1992] Report a provisioning timeout as pending, not as a failed sign-in - #608
Merged
Merged
Conversation
When WorkOS workspace provisioning outran its 4s x 150 poll, Core collapsed the InProgress outcome into an undistinguished AuthResult.Failed, so the wizard headlined "Sign-in failed." over a workspace that was on its way. Sign-in had already succeeded to get that far. The accurate line - "still provisioning, finish later by joining <slug> from the Connect step" - survived only as a detail. - AuthFailureReason gains ProvisioningInProgress. It rides Failed rather than becoming its own AuthResult case on purpose: every consumer discriminates on Committed and funnels the rest into a catch-all, so a new case would have landed in the very default arm that prints the wrong headline. Riding the reason forces the one switch that had to change to change. - ProvisionOffer carries the pending slug, so a caller can name the workspace it is telling the user to come back to. InProgress becomes a factory, matching the file's existing split between outcomes that carry data and ones that do not. - The headline is the fact and the sink's line is what to do about it. They reach the same reader in a GUI, so the headline deliberately does not repeat the guidance. - App.axaml.cs printed the same untruth to stderr when the window closed mid-poll. Declined is left generic for now, and deliberately NOT pinned by a test: it is arguably the same defect - the user chose it, and the step still says sign-in failed - but it is a different decision from this one and wants its own. The terminal setup path needed no change: its failure path prints nothing for this reason, so the only line a terminal user sees is the provisioner's own. The slug is passed there too for symmetry, though nothing reads it on that path yet. Closes #573
George-Payne
requested review from
alexeyzimarev,
nortonandreev,
realtonyyoung,
rsergeenko and
stktung
August 19, 2026 15:17
PR Summary by QodoReport provisioning timeouts as pending instead of failed sign-ins
AI Description
Diagram
High-Level Assessment
Files changed (12)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all |
From Tony's review of the previous commit. Round one of review said the headline and the progress line said the same thing twice in the wizard, so the headline became the bare fact and the guidance stayed with the sink. But the stderr handoff runs after the window has closed, and that view is where the guidance was - so a user who closes mid-poll learned the workspace was pending and nothing about how to resume. The two surfaces have different context available, so the handoff spells the instruction out rather than the message carrying it for everyone.
alexeyzimarev
approved these changes
Aug 19, 2026
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
3 participants
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.
When WorkOS workspace provisioning outran its 4s × 150 poll, Core collapsed the
ProvisionOffer.InProgressoutcome into an undistinguishedAuthResult.Failed, so the wizard's Sign-in step headlined "Sign-in failed." over a workspace that was on its way — sign-in had already succeeded to get that far. The accurate copy survived only as a detail line and a log entry.AuthFailureReasongainsProvisioningInProgress. It ridesFailedrather than becoming its ownAuthResultcase deliberately: every consumer discriminates onCommittedand funnels the rest into a catch-all (SignInStepViewModel:422,LoginCommand:66,SetupCommand:934,App.axaml.cs:429), so a new case would have landed silently in the verydefault:arm that prints the wrong headline. Riding the reason forces the one switch that had to change to change, andSetupCommand:50already sets the{ Reason: … }precedent.ProvisionOffercarries the pending slug, so a caller can name the workspace it is telling the user to come back to.InProgressbecomes a factory, matching the file's existing split between outcomes that carry data (Created,ExistingWorkspace) and ones that do not.WizardAuthBridgesreports that line as aNoticerather than anError, since nothing went wrong.App.axaml.csprinted the same untruth to stderr when the window was closed mid-poll — narrow, but it is literally the claim this ticket is about.kcap setupneeded no change, which is worth stating rather than implying otherwise. Its failure path prints nothing for this reason (SetupAuthProgress.ReportFailureonly speaks forUnreachable, andOnboardingFacade.Stopnever prints), so the only line a terminal user sees is the provisioner's own yellow "Still provisioning" — already correct. The slug is passed there too for symmetry, but nothing reads it on that path yet.Declinedis left generic, and deliberately not pinned by a test. It is arguably the same defect — the user chose to decline and the step still says sign-in failed — but it is a separate decision from this one, so asserting the current behaviour here would lock the wrong answer in with a passing test. Happy to file it.Tests: the discovery-level reason and slug, the reason surviving the flow→
AuthResultmapping, and the wizard headline itself (SignInStepViewModelTests) — which is the user-visible half and had no coverage. Verified by reverting each production change in turn and confirming the matching test reddens.WizardAuthBridgesTestsalso now pinsPendingSlug, which nothing did.Closes #573
#573