Repository navigation
Offer sign-in when the app is disconnected instead of a raw 401 - #740
Conversation
The re-auth dialog stages the wizard's own Paste intent for the profile's configured server, so it reuses the wizard commit boundary and cannot repoint server_url; a Paste intent never answers Retarget, which is what lets the dialog exist without a Connect step. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 286dc19adf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| window.Closed += (_, _) => { | ||
| graph.SignIn.PropertyChanged -= OnSignInChanged; | ||
| _signInWindow = null; | ||
| _ = FinishSignInAsync(graph); |
There was a problem hiding this comment.
When the user quits while this dialog has a live sign-in attempt, the graph exists only in this closure: DisposeAndShutdownAsync quiesces _wizardAuth but neither closes nor awaits this steady-state graph. The confirmed TryShutdown eventually closes the window and starts FinishSignInAsync, but that task is fire-and-forget, so the process can terminate while cancellation or a post-boundary credential commit is still running. Retain the re-auth graph/task and settle it before confirming shutdown, as is already done for wizard authentication.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 398190e. The dialog's settle task (FinishSignInAsync) is now retained in a field; DisposeAndShutdownAsync closes the dialog first — still on the UI thread, before the ConfigureAwait(false) quiesce hop — and awaits the settle, so a live attempt is cancelled pre-boundary or a commit already past the boundary finishes before the process exits. This also covers a quit arriving just after the user closed the dialog mid-attempt, where the settle was previously still in flight.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Closes #739 — AI-2407
What & why
Starting a session while the app could not authenticate rendered the raw transport text ("Response status code does not indicate success: 401 (Unauthorized).") under the composer, with no way back to a login. Start is now disabled unless both the daemon attach and its upstream server connection are up, the launcher says why (connecting / daemon not running / signed out), and both a lost server connection and a typed 401 launch outcome offer a Sign in button. That button opens the wizard's sign-in step in a standalone dialog pinned to the configured server; a committed sign-in closes the dialog and clears the prompt.
Where to look
ReauthComposition stages the wizard's own Paste intent for the profile's server — sign-in reuses the wizard's commit boundary unchanged, cannot repoint
server_url, and a Paste intent never produces the WorkOS Retarget answer, which is why the dialog needs no Connect step. The daemon reports no reason for "disconnected", so the sign-in prompt also shows when the server itself is down; the dialog then fails visibly with a network error rather than fixing anything.Verification
dotnet run --project test/Capacitor.App.Tests.Unit/...).dotnet test --solution: 10898 tests, 2 failures — both on the documented environmental list (codex schema pin drift;Wedged_initialize_*load flake, passes 1/1 in isolation) in a suite that does not reference Capacitor.App.AvailabilityForor the 401 flag fails exactly the new tests.🤖 Generated with Claude Code