feat(auth): implement GitHub Device Flow authentication#72
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR introduces a comprehensive brand-consistent procedural icon system (PaintDaubeIcon) replacing emoji/text indicators throughout the application, implements real GitHub Device Flow OAuth authentication with stateful flow management and secure token storage, and refactors multiple UI components to consume the new type-safe icon system with configurable colors and sizes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as GitHub Auth Screen
participant GitHubAuthService
participant GitHub as GitHub OAuth API
participant CredentialService as Credential Service
User->>App: Mount/Initialize
App->>GitHubAuthService: checkForToken()
GitHubAuthService->>CredentialService: retrieve('github')
CredentialService-->>GitHubAuthService: token or null
alt Token Exists
GitHubAuthService->>GitHub: GET /user (with token)
GitHub-->>GitHubAuthService: User data
GitHubAuthService-->>App: Success state with user data
App->>User: Show "Connected" success view
else Token Missing
App->>User: Show landing screen
User->>App: Start Device Flow
App->>GitHubAuthService: startDeviceFlow(clientId, scopes)
GitHubAuthService->>GitHub: POST /login/device/code
GitHub-->>GitHubAuthService: device_code, user_code, verification_uri
GitHubAuthService->>App: Display user_code + verification_uri
App->>User: Show code & GitHub link
User->>GitHub: Open verification_uri
User->>GitHub: Enter device code
loop Polling (every interval)
App->>GitHubAuthService: pollForToken(clientId, device_code)
GitHubAuthService->>GitHub: POST /login/oauth/access_token
alt Pending Authorization
GitHub-->>GitHubAuthService: error: "authorization_pending"
GitHubAuthService-->>App: Polling state update
App->>App: Show poll progress
else Token Received
GitHub-->>GitHubAuthService: access_token, scope
GitHubAuthService->>CredentialService: store('github', access_token)
CredentialService-->>GitHubAuthService: Stored
GitHubAuthService->>GitHub: GET /user
GitHub-->>GitHubAuthService: User data
GitHubAuthService-->>App: Success state
App->>User: Show "Connected" success with username
User->>App: Continue to next screen
else Error (expired, denied, etc.)
GitHub-->>GitHubAuthService: error: "expired_token"
GitHubAuthService-->>App: Error state
App->>User: Show error + retry button
end
end
end
User->>App: Unmount
App->>GitHubAuthService: cancel()
GitHubAuthService-->>GitHubAuthService: Clean up polling state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
🚀 Expo preview is ready!
|
Contributor
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements GitHub Device Flow authentication for mobile and introduces a comprehensive procedural icon system to replace emoji throughout the app, establishing consistent brand identity.
Changes:
- Implemented real GitHub Device Flow OAuth authentication with token polling and secure storage
- Created a procedural "paint daube" SVG icon system with 50+ brand-consistent icons
- Replaced all emoji usage with organic paint daube icons across the entire application
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/core/src/auth/GitHubAuthService.ts |
Core service implementing GitHub Device Flow with polling, error handling, and token storage |
packages/core/src/auth/types.ts |
TypeScript type definitions for Device Flow states, responses, and errors |
packages/core/src/__tests__/GitHubAuthService.test.ts |
Comprehensive test suite for authentication service |
src/components/icons/PaintDaubeIcon.tsx |
Procedural SVG icon component with 50+ variants using organic shapes and feTurbulence filters |
src/components/icons/index.ts |
Module exports for icon system |
app/(onboarding)/github-auth.tsx |
Updated onboarding screen to use real Device Flow API with polling UI and error states |
| Multiple component files | Replaced emoji with paint daube icons across Badge, Toast, Progress, Modal, EmptyState, etc. |
| Multiple screen files | Updated all app screens to use new icon system instead of emoji |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 3
🤖 Fix all issues with AI agents
In `@packages/core/src/auth/GitHubAuthService.ts`:
- Around line 35-41: The pollForToken flow can hang if cancel() is called
between polls and stale in‑flight responses can overwrite state; update
GitHubAuthService to track cancellation and resolve or reject any pending poll
promise immediately when cancel() runs and to ignore stale responses by
comparing a poll token/version (e.g., increment pollAttempt or use a unique
pollId) before applying results. Specifically, ensure cancel() clears
pollTimeoutId, calls abortController?.abort(), sets state to 'idle' (or
appropriate), and signals any awaiting promise from pollForToken so it returns
promptly; modify pollForToken to check the current pollAttempt/pollId and
this.state at each continuation and when receiving network responses to skip
handling if cancelled or outdated, and always clean up timeouts/abortController
on completion.
In `@src/components/display/Badge.tsx`:
- Around line 94-99: The text-only glyph (rendered when textIcon && !Icon in
Badge.tsx) currently inherits default text color; update the Text element to
include a color class derived from the same source used for the SVG Icon (use
the iconColor prop or fallback to 'warmGray') so the glyph matches the icon
color and renders correctly in dark theme; modify the className on the Text
(referencing sizing.text and textIcon render branch) to append a color
token/class based on iconColor (or default) and ensure it mirrors the styling
logic used for the <Icon ... color={iconColor || 'warmGray'} /> case.
In `@src/components/feedback/Toast.tsx`:
- Around line 178-180: The dismiss Pressable in Toast.tsx is icon-only and lacks
an accessibility label; update the Pressable that uses onDismiss and CloseIcon
to include an accessibilityLabel (e.g., "Dismiss toast" or a localized string)
and set accessibilityRole="button" (and optionally accessible={true}) so screen
readers announce it; ensure the label text is sourced from the same
i18n/localization helper used elsewhere if available.
🧹 Nitpick comments (5)
src/components/chat/ApprovalCard.tsx (1)
30-48: Consider removing the unusedbgColorproperty.The
bgColorproperty is defined inActionInfoand populated in theactionMap, but it's never used anywhere in the component rendering. If this was intended for future use, consider adding a TODO comment; otherwise, remove it to avoid confusion.♻️ Suggested fix
interface ActionInfo { Icon: ActionIconComponent; iconColor: IconColor; label: string; - bgColor: string; } /** * Get action type icon and label */ function getActionInfo(actionType: ApprovalMessage['metadata']['actionType']): ActionInfo { const actionMap: Record<ApprovalMessage['metadata']['actionType'], ActionInfo> = { - commit: { Icon: EditIcon, iconColor: 'teal', label: 'Commit Changes', bgColor: 'bg-teal-500' }, - push: { Icon: GitIcon, iconColor: 'coral', label: 'Push to Remote', bgColor: 'bg-coral-500' }, - merge: { Icon: BranchIcon, iconColor: 'gold', label: 'Merge Branch', bgColor: 'bg-gold-500' }, - deploy: { Icon: LightningIcon, iconColor: 'coral', label: 'Deploy', bgColor: 'bg-coral-600' }, - file_change: { Icon: FileIcon, iconColor: 'teal', label: 'File Changes', bgColor: 'bg-teal-600' }, + commit: { Icon: EditIcon, iconColor: 'teal', label: 'Commit Changes' }, + push: { Icon: GitIcon, iconColor: 'coral', label: 'Push to Remote' }, + merge: { Icon: BranchIcon, iconColor: 'gold', label: 'Merge Branch' }, + deploy: { Icon: LightningIcon, iconColor: 'coral', label: 'Deploy' }, + file_change: { Icon: FileIcon, iconColor: 'teal', label: 'File Changes' }, }; return actionMap[actionType] || actionMap.commit; }app/(onboarding)/github-auth.tsx (4)
39-50: Add error handling for the async auth check.If
isAuthenticated()orgetCurrentUser()throws (e.g., due to a corrupted credential store or network issue), this will result in an unhandled promise rejection. Consider wrapping in try/catch to prevent the app from silently failing on mount.Suggested fix
useEffect(() => { const checkExistingAuth = async () => { + try { const isAuth = await GitHubAuthService.isAuthenticated(); if (isAuth) { const user = await GitHubAuthService.getCurrentUser(); if (user) { setGithubUser(user); setIsConnected(true); } } + } catch (err) { + // Silently fail - user can still proceed with fresh auth + console.warn('Failed to check existing auth:', err); + } }; checkExistingAuth();
89-91: Add error handling forLinking.openURL.
Linking.openURLcan reject if the URL cannot be opened (e.g., no browser installed, URL scheme not supported). Consider wrapping in try/catch and showing user feedback on failure.Suggested fix
const openGitHub = useCallback(async () => { - await Linking.openURL(verificationUri); + try { + await Linking.openURL(verificationUri); + } catch (err) { + Alert.alert( + 'Unable to Open Browser', + `Please manually visit ${verificationUri} and enter the code.`, + [{ text: 'OK' }] + ); + } }, [verificationUri]);
113-117: Add error handling forgetCurrentUserafter successful authorization.If
getCurrentUser()fails after the token is successfully obtained, the promise rejection is unhandled andisConnectedwill remain false despite successful auth. This could leave the user in a confusing state.Suggested fix
if (result.authorized) { - const user = await GitHubAuthService.getCurrentUser(); - setGithubUser(user); + try { + const user = await GitHubAuthService.getCurrentUser(); + setGithubUser(user); + } catch (err) { + // Token was stored successfully, user fetch failed + // Still mark as connected so user can proceed + console.warn('Failed to fetch user details:', err); + } setIsConnected(true); }
135-135: Remove unusedisLoadingvariable.
isLoadingis defined but never used in the component. The JSX directly checksflowStatevalues instead.Suggested fix
- const isLoading = flowState === 'requesting_code' || flowState === 'polling';
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an excellent pull request that accomplishes two major goals: implementing a robust GitHub Device Flow authentication and overhauling the entire application's iconography with a new, creative procedural icon system. The new GitHubAuthService is well-structured, and the inclusion of a comprehensive test suite is fantastic. The UI changes are consistent and the new 'paint daube' icons give the app a unique and polished brand identity. I've found a couple of areas for improvement in the cancellation logic for the authentication flow to make it even more robust. Great work on this significant feature!
jbdevprimary
force-pushed
the
feat/github-device-flow-auth
branch
from
0acd95c to
98de3b7
Compare
Implement real GitHub Device Flow OAuth for mobile authentication. - Add GitHubAuthService with device code request and token polling - Create auth types for Device Flow responses and states - Update github-auth.tsx to use real API instead of mocks - Add proper error handling with user-friendly messages - Support cancel/retry flow with proper cleanup - Display authenticated user info on success The Device Flow is ideal for mobile apps where users authenticate in their phone's browser and return to the app. Closes #69 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive unit tests for the GitHub Device Flow service: - Test service initialization and state management - Test device code request flow - Test error handling for missing clientId - Test API error responses - Test cancel flow and cleanup - Test token polling states Note: Jest configuration may need updates to transform TypeScript files in the packages/ directory. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…tion or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Add cancellation tracking with isCancelled flag and pollResolve - Handle AbortError specifically in catch block instead of generic error - Add AbortSignal to checkForToken fetch request - Resolve pending poll promise when cancel() is called - Add accessibility attributes to Toast dismiss button - Add environment variable validation warning in development Also includes lint fixes: - Remove unused React imports (new JSX transform) - Fix import type annotations - Organize imports alphabetically per Biome rules Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The test was failing in CI because expo-constants native module was not properly mocked. Added jest.mock for expo-constants before any imports to resolve the test suite failure. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
jbdevprimary
force-pushed
the
feat/github-device-flow-auth
branch
from
f48134e to
13f074e
Compare
|
Closed
22 tasks
3 tasks
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.
Summary
GitHubAuthServicewith device code request and token pollinggithub-auth.tsxonboarding screen to use real API instead of mocksImplementation Details
New Files
packages/core/src/auth/GitHubAuthService.ts- Main service implementing Device Flowpackages/core/src/auth/types.ts- TypeScript types for Device Flowpackages/core/src/auth/index.ts- Module exportsModified Files
packages/core/src/index.ts- Export auth moduleapp/(onboarding)/github-auth.tsx- Use real service, add error states, polling UIDevice Flow Steps
github.com/login/device/codegithub.com/login/deviceCredentialServiceTest plan
Closes #69
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.