Skip to content

Fix sign-in getting stuck with DNS error + dialog re-firing over auth screen#465

Merged
rainxchzed merged 3 commits intomainfrom
fix/auth-stuck-on-direct-and-dialog-loop
Apr 28, 2026
Merged

Fix sign-in getting stuck with DNS error + dialog re-firing over auth screen#465
rainxchzed merged 3 commits intomainfrom
fix/auth-stuck-on-direct-and-dialog-loop

Conversation

@rainxchzed
Copy link
Copy Markdown
Member

@rainxchzed rainxchzed commented Apr 28, 2026

Symptom

User hits the rate-limit prompt → taps "Sign in with GitHub" → gets a device code → enters it on github.com → reopens the app and:

  1. DNS error: Unable to resolve host "github.com": No address associated with hostname — even though the backend proxy at /v1/auth/device/poll is reachable. Polling never recovers.
  2. Rate-limit dialog re-emits over the auth screen — the same prompt the user already accepted keeps interrupting the flow.

Root cause

(1) AuthenticationViewModel persists auth_path (Backend / Direct) into SavedStateHandle and restores it on process restart. The escalation Backend → Direct is correctly one-way within a session, but SavedStateHandle survives process death too — so a user whose Backend hiccupped once gets stuck on Direct permanently across reopens. On networks where github.com is the unreachable side (the entire reason the backend proxy exists), this means polling keeps hitting https://github.com/login/oauth/access_token and dying with DNS errors.

(2) RateLimitDialog is rendered at the App composable root in Main.kt, with no awareness of the current screen. Any background API call (e.g., installed-apps sync) that hits 403 + exhausted rate-limit headers re-flips showRateLimitDialog to true — even while the user is mid-device-flow on the auth screen.

Fix

Auth path (AuthenticationViewModel):

  • Stop persisting KEY_AUTH_PATH to SavedStateHandle on initial save and on mid-session escalation.
  • On restoreFromSavedState, always reset authPath to Backend and remove any legacy bundle entry. The repository's existing escalation logic re-promotes to Direct on the next poll if Backend is genuinely unreachable on the new network.
  • Net effect: process restart re-evaluates the network rather than carrying forward a stale escalation. Within a single session, behavior is unchanged (the in-memory var survives config change because the ViewModel does).

Rate-limit dialog (Main.kt):

  • Gate the dialog render with currentScreen !is GithubStoreGraph.AuthenticationScreen so it can never overlay the auth screen.
  • Add a LaunchedEffect that auto-dismisses the flag when the user is on the auth screen and the flag flips back on, so the dialog doesn't ghost back when the user returns to home after signing in.

Test plan

  • On a network where github.com is filtered/throttled but the backend is reachable: trigger the rate-limit prompt, sign in, enter the code, kill the app, reopen. Expect: polling resumes via Backend, login completes. Before the fix: DNS error.
  • Trigger the rate-limit dialog from home → tap "Sign in" → on the auth screen, force any background API call to 403 (or just wait — the symptom reproduces against the live backend rate limits). Expect: dialog stays hidden. Before the fix: it re-overlays the auth screen.
  • Complete a successful sign-in and navigate back to home: dialog does not flash on entry. Before the fix: the latent flag from a background 403 made it ghost-appear.

Summary by CodeRabbit

  • Bug Fixes
    • Prevented the rate-limit dialog from appearing while on the authentication screen, reducing UI interruptions during sign-in.
    • Improved authentication stability by preventing unexpected mid-session route changes—transitions are now restricted to a safe, one-directional upgrade path to avoid inconsistent auth states.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b1a1cb2c-eb94-42c3-a8af-042d2e41d8b4

📥 Commits

Reviewing files that changed from the base of the PR and between 18655c2 and d61bb2d.

📒 Files selected for processing (1)
  • feature/auth/presentation/src/commonMain/kotlin/zed/rainxch/auth/presentation/AuthenticationViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • feature/auth/presentation/src/commonMain/kotlin/zed/rainxch/auth/presentation/AuthenticationViewModel.kt

Walkthrough

Prevent the rate-limit dialog from appearing while the navigation destination is the authentication screen by dismissing that dialog state via a LaunchedEffect; tighten polling auth-path updates in the AuthenticationViewModel to allow only a one-way Backend→Direct transition and emit a warning for refused transitions.

Changes

Cohort / File(s) Summary
Dialog State Management
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/Main.kt
When onAuthScreen is true and state.showRateLimitDialog is set, a LaunchedEffect dispatches MainAction.DismissRateLimitDialog; the rate-limit dialog is rendered only when state.showRateLimitDialog && state.rateLimitInfo != null and the current destination is not the Authentication screen.
Auth Polling / Path Transition
feature/auth/presentation/src/commonMain/kotlin/zed/rainxch/auth/presentation/AuthenticationViewModel.kt
Changed poll loop behavior: poll outcomes no longer unconditionally overwrite authPath. The ViewModel only updates and persists the path when transitioning from Backend to Direct; other attempted transitions are ignored and trigger a warning log.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐇 I hop through code with whiskers bright,
Dialogs dismissed where auth takes flight,
Paths now one-way — Backend to Direct,
A tidy hop, a careful inspect. 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: fixing sign-in being stuck with DNS errors and preventing rate-limit dialog from appearing over the authentication screen.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/auth-stuck-on-direct-and-dialog-loop

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@feature/auth/presentation/src/commonMain/kotlin/zed/rainxch/auth/presentation/AuthenticationViewModel.kt`:
- Line 348: The ViewModel stops persisting authPath which breaks the
restart/resume contract: update the save path so authPath is stored in the
SavedStateHandle when saving UI state (call or extend saveToSavedState used at
saveToSavedState(start.deviceCode, startUi) to also persist authPath or write it
alongside using the AuthenticationViewModel's SavedStateHandle), and remove the
forced reset/removal of authPath during restore (the code around the
restore/reset block at lines ~535-543) so in-progress auth_path is reloaded on
recreation; keep the same SavedStateHandle key name “authPath” and ensure
restore logic reads that key back into the AuthenticationViewModel state.
- Around line 424-433: The current check in AuthenticationViewModel that flips
authPath whenever outcome.path != authPath allows Direct→Backend switches
mid-session; change it so authPath is only updated once at session start or
escalated one-way Backend→Direct on infrastructure errors: in the place that
handles pollDeviceTokenOnce outcomes, only set authPath = outcome.path when the
session is initializing (sessionStart flag/logic) or when current authPath ==
Backend and outcome.path == Direct AND outcome.error indicates an infrastructure
failure (timeout, socket/connect error, or 5xx response); do NOT change authPath
when current authPath == Direct and outcome.path == Backend or for
non-infrastructure errors. Ensure this logic is implemented around the existing
authPath/outcome.path handling in AuthenticationViewModel and that any comment
about non-persistence remains accurate.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d66d1aad-aeab-47ab-94c8-4fd1bc027029

📥 Commits

Reviewing files that changed from the base of the PR and between 376d4e7 and 18655c2.

📒 Files selected for processing (2)
  • composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/Main.kt
  • feature/auth/presentation/src/commonMain/kotlin/zed/rainxch/auth/presentation/AuthenticationViewModel.kt

@rainxchzed rainxchzed merged commit 376ae3e into main Apr 28, 2026
1 check passed
@rainxchzed rainxchzed deleted the fix/auth-stuck-on-direct-and-dialog-loop branch April 28, 2026 16:13
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.

1 participant