Fix endless loop in wizard od failed auth - #1090
Open
TheOneRing wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed control-flow issues that can emit success after failure (and still run WebFinger lookup on insecure-url errors), plus null-safety gaps for the new “OAuth pointer may be null” behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adjusts the OAuth credentials step in the new setup wizard to avoid re-triggering authentication indefinitely after a failed OAuth attempt, by introducing an explicit “failed/error” state and a retry path to restart authentication from the UI.
Changes:
- Add a retry signal (
evaluationRetry) and wire it through the controller to re-evaluate the current wizard state without treating it as a failure. - Update the OAuth credentials wizard state to support a “failed” mode (no OAuth object) and allow restarting auth from the page/UI.
- Make
QmlOAuthCredentialstolerateOAuth* == nullptrfor error-state rendering.
File summaries
| File | Description |
|---|---|
| src/gui/newwizard/states/oauthcredentialssetupwizardstate.h | Extend credentials state constructor to accept a “failed” flag. |
| src/gui/newwizard/states/oauthcredentialssetupwizardstate.cpp | Add failed-mode behavior, restart wiring, and updated OAuth result handling. |
| src/gui/newwizard/states/abstractsetupwizardstate.h | Introduce evaluationRetry signal for retrying a state evaluation. |
| src/gui/newwizard/setupwizardcontroller.cpp | Pass failure reason into credentials state and add retry handling in controller. |
| src/gui/newwizard/setupwizardcontroller_p.h | Extend ChangeReason with EvaluationRetry. |
| src/gui/newwizard/pages/oauthcredentialssetupwizardpage.h | Add requestAuthRestart signal from the credentials page. |
| src/gui/newwizard/pages/oauthcredentialssetupwizardpage.cpp | Forward QML restart requests to the page signal. |
| src/gui/creds/qmlcredentials.h | Document that OAuth pointer may be null in error state; add restart signal. |
| src/gui/creds/qmlcredentials.cpp | Guard OAuth signal hookups when OAuth is null (error state). |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
TheOneRing
marked this pull request as draft
September 8, 2026 09:43
TheOneRing
force-pushed
the
work/loop
branch
from
September 8, 2026 11:06
4fb909a to
ea36716
Compare
TheOneRing
marked this pull request as ready for review
September 8, 2026 11:07
TheOneRing
force-pushed
the
work/loop
branch
from
September 8, 2026 11:08
ea36716 to
3833c9f
Compare
TheOneRing
force-pushed
the
work/loop
branch
from
September 8, 2026 11:21
0a34c15 to
4364e42
Compare
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.
No description provided.