Fix sign-in getting stuck with DNS error + dialog re-firing over auth screen#465
Conversation
…ation doesn't strand users on github-blocked networks
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughPrevent 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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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
📒 Files selected for processing (2)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/Main.ktfeature/auth/presentation/src/commonMain/kotlin/zed/rainxch/auth/presentation/AuthenticationViewModel.kt
…way Backend->Direct only
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:
Unable to resolve host "github.com": No address associated with hostname— even though the backend proxy at/v1/auth/device/pollis reachable. Polling never recovers.Root cause
(1)
AuthenticationViewModelpersistsauth_path(Backend/Direct) intoSavedStateHandleand restores it on process restart. The escalationBackend → Directis correctly one-way within a session, butSavedStateHandlesurvives process death too — so a user whose Backend hiccupped once gets stuck onDirectpermanently across reopens. On networks wheregithub.comis the unreachable side (the entire reason the backend proxy exists), this means polling keeps hittinghttps://github.com/login/oauth/access_tokenand dying with DNS errors.(2)
RateLimitDialogis rendered at the App composable root inMain.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-flipsshowRateLimitDialogtotrue— even while the user is mid-device-flow on the auth screen.Fix
Auth path (
AuthenticationViewModel):KEY_AUTH_PATHtoSavedStateHandleon initial save and on mid-session escalation.restoreFromSavedState, always resetauthPathtoBackendand remove any legacy bundle entry. The repository's existing escalation logic re-promotes toDirecton the next poll if Backend is genuinely unreachable on the new network.varsurvives config change because the ViewModel does).Rate-limit dialog (
Main.kt):currentScreen !is GithubStoreGraph.AuthenticationScreenso it can never overlay the auth screen.LaunchedEffectthat 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
github.comis 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.Summary by CodeRabbit