fix(auth): prefill device code URL, strip hyphen on copy#604
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
rainxchzed has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Contributor
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds localized whatsnew bullets in 13 languages and updates auth: ViewModel sanitizes/copies device codes, prefers verificationUriComplete or builds a prefilled verification URL, and repository startDeviceFlow refines fallback eligibility and logging. ChangesDevice code authentication improvements
Sequence DiagramsequenceDiagram
participant User
participant AuthenticationViewModel
participant BackendService
participant GitHub
User->>AuthenticationViewModel: startLogin / Start device flow
AuthenticationViewModel->>AuthenticationViewModel: sanitize userCode (alphanumeric)
AuthenticationViewModel->>AuthenticationViewModel: copy sanitized code to clipboard
AuthenticationViewModel->>BackendService: request backend-start (optional)
BackendService->>AuthenticationViewModel: backend device/start response (verificationUri, verificationUriComplete)
alt verificationUriComplete available
AuthenticationViewModel->>GitHub: open verificationUriComplete URL
else fallback
AuthenticationViewModel->>AuthenticationViewModel: buildPrefilledUrl(verificationUri, userCode)
AuthenticationViewModel->>GitHub: open prefilled URL with user_code param
end
GitHub->>User: show device code page
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/auth/presentation/src/commonMain/kotlin/zed/rainxch/auth/presentation/AuthenticationViewModel.kt`:
- Around line 626-629: In buildPrefilledUrl(verificationUri: String, userCode:
String) the userCode is concatenated raw into the query string; URL-encode it
using io.ktor.http.encodeURLParameter before appending so the query param is
correctly escaped (follow the same pattern used in FeedbackComposer.kt), and add
the import for encodeURLParameter if missing.
🪄 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: 3fccf9fd-5238-4bdf-8b94-e8c327ff27ec
📒 Files selected for processing (14)
core/presentation/src/commonMain/composeResources/files/whatsnew/18.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ar/18.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/bn/18.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/es/18.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/fr/18.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/hi/18.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/it/18.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ja/18.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ko/18.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/pl/18.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ru/18.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/tr/18.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/zh-CN/18.jsonfeature/auth/presentation/src/commonMain/kotlin/zed/rainxch/auth/presentation/AuthenticationViewModel.kt
There was a problem hiding this comment.
rainxchzed has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/auth/data/src/commonMain/kotlin/zed/rainxch/auth/data/repository/AuthenticationRepositoryImpl.kt`:
- Line 51: The start-flow log in AuthenticationRepositoryImpl.kt currently
prints the live device authorization secret (dto.userCode) which must be
removed; update the logger.info calls (the one that logs "Device flow started
via Backend..." and the other occurrence around the same start-flow logic) to
omit dto.userCode and only include non-secret fields (e.g., dto.intervalSec,
dto.expiresInSec, and whether dto.verificationUriComplete is present) so the
active sign-in code is not written to logs.
- Around line 59-63: The current fallback to Direct when
isBackendStartFallbackEligible() is true incorrectly treats backend 4xx
responses (e.g., BackendHttpException with status 429) as infrastructure
failures; update the logic in AuthenticationRepositoryImpl (where backend start
errors are handled and in the similar block at the other occurrence) to only
escalate to Direct for true infra errors (timeouts and 5xx), and explicitly
treat HTTP 4xx (including 429) and GitHub negative responses as definitive
backend answers—either by refining isBackendStartFallbackEligible() to return
false for 4xx statuses or by checking the exception/status (e.g.,
BackendHttpException.status) before calling the fallback path so AuthPath
remains Backend for client errors.
🪄 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: 858dd274-3a34-4a94-967d-34b56e1e69aa
📒 Files selected for processing (2)
feature/auth/data/src/commonMain/kotlin/zed/rainxch/auth/data/repository/AuthenticationRepositoryImpl.ktfeature/auth/presentation/src/commonMain/kotlin/zed/rainxch/auth/presentation/AuthenticationViewModel.kt
There was a problem hiding this comment.
rainxchzed has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
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
1 participant
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.
Sprint 3 Task 8 + diagnostics. Backend
/v1/auth/device/startreturns 429 rate_limited (10/hr/IP, ~1hr cooldown) when user retries — current code treated 429 as non-infra and gave no fallback path.openGitHub: build prefilled?user_code=URL client-side when backend omitsverification_uri_complete.copyCode: strip hyphen so paste lands cleanly on GitHub's split-digit boxes.startDeviceFlow: fall back to Direct GitHub on backend 429 (start path only — poll path still respects backend rate limits).logger.errorwith full cause chain + status code + origin on every failure path.Test plan
Source: Siddharth Patil + 陈sir + Tao Feng + on-device repro (HTTP 429 confirmed via curl).
Summary by CodeRabbit
Bug Fixes
Documentation