feat(auth): add auth failure detection modal for Claude CLI 401 errors #1361
Conversation
Detect authentication failures (401 errors) from Claude CLI and display a modal prompting the user to re-authenticate. This improves UX by providing clear feedback when tokens expire, are invalid, or are missing. Changes: - Add AuthFailureInfo interface for auth failure events - Add CLAUDE_AUTH_FAILURE IPC channel for main→renderer communication - Add auth-failure event handler in agent-events-handlers.ts - Add AuthFailureModal component with i18n translation support - Add useAuthFailureStore Zustand store for modal state management - Wire up IPC listeners in useIpc.ts Co-Authored-By: Claude Opus 4.5 <[email protected]>
github-actions
bot
added
area/fullstack
|
Caution Review failedThe pull request is closed. 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. 📝 WalkthroughWalkthroughAdds end-to-end handling for Claude authentication failures: new Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent
participant Main as Main Process
participant Preload as Preload
participant Renderer as Renderer
participant Store as Auth Store
participant Modal as AuthFailureModal
Agent->>Main: emit("auth-failure", taskId, authFailure)
Main->>Main: (optional) resolve profileName
Main->>Preload: safeSendToRenderer(CLAUDE_AUTH_FAILURE, AuthFailureInfo)
Preload->>Renderer: IPC CLAUDE_AUTH_FAILURE
Renderer->>Store: showAuthFailureModal(AuthFailureInfo)
Store-->>Modal: authFailureInfo & isModalOpen
Modal->>Modal: render UI (message, details)
User->>Modal: Click "Dismiss" / "Go to Settings"
Modal->>Store: clearAuthFailure() or hideAuthFailureModal()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Summary of ChangesHello @AndyMik90, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a crucial user experience improvement by introducing an authentication failure detection mechanism for the Claude CLI. It ensures that users are promptly notified via a dedicated modal when their Claude authentication tokens become invalid, expired, or are missing, typically indicated by a 401 HTTP error. The system is designed to provide actionable feedback, guiding users to re-authenticate through settings, while maintaining the existing architecture where Claude CLI handles the underlying authentication lifecycle. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 pull request introduces a new feature to detect authentication failures (401 errors) from the Claude CLI and display a user-friendly modal for re-authentication. The changes include defining a new AuthFailureInfo interface, adding an IPC channel for communication between the main and renderer processes, implementing an event handler in the main process, creating a dedicated AuthFailureModal React component, and setting up a Zustand store for managing the modal's state. The IPC listeners are correctly wired up in the useIpc.ts hook, ensuring proper data flow and type conversion for detectedAt timestamps. The new modal provides clear user feedback and options to navigate to settings for re-authentication or dismiss the message. Overall, the implementation is well-structured and addresses the stated goal of improving UX feedback for authentication issues.
| console.warn(`[AgentEvents] Auth failure detected for task ${taskId}:`, authFailure); | ||
|
|
||
| // Get profile name for display | ||
| const { getClaudeProfileManager } = require("../claude-profile-manager"); |
There was a problem hiding this comment.
Using require inside a TypeScript file is generally discouraged in favor of import statements. While it might work due to transpilation, using import provides better static analysis, type checking, and aligns with modern TypeScript practices.
import { getClaudeProfileManager } from "../claude-profile-manager";| return t('auth.failure.tokenExpired', 'Your authentication token has expired.'); | ||
| case 'invalid': | ||
| return t('auth.failure.tokenInvalid', 'Your authentication token is invalid.'); | ||
| case 'missing': | ||
| return t('auth.failure.tokenMissing', 'No authentication token found.'); | ||
| default: | ||
| return t('auth.failure.authFailed', 'Authentication failed.'); |
There was a problem hiding this comment.
The AuthFailureModal component uses several i18n keys (e.g., auth.failure.title, auth.failure.tokenExpired, auth.failure.description, auth.failure.goToSettings) that are not present in the provided common.json file. This will result in untranslated strings or fallback to the default English values, which might not be ideal for localization. Please ensure these keys are added to the common.json file for proper internationalization.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts`:
- Around line 146-151: Replace the dynamic require of getClaudeProfileManager
with a static import at the top of the module and remove the runtime require()
call: add a top-level import for getClaudeProfileManager, then call
getClaudeProfileManager() where profileManager is created (replace the require()
lines that create profileManager and profile), preserving the existing logic
that uses authFailure.profileId to select profile via
profileManager.getProfile(...) or profileManager.getActiveProfile(). Ensure you
reference the existing symbols getClaudeProfileManager, profileManager, profile,
and authFailure when making the change.
In `@apps/frontend/src/renderer/components/AuthFailureModal.tsx`:
- Line 28: The fallback string for profileName is hardcoded; replace the literal
'Unknown Profile' with a localized string using the app's i18n helper (e.g.,
useTranslation hook / t function) in AuthFailureModal.tsx so profileName becomes
something like authFailureInfo.profileName || t('auth.unknownProfile'); update
the appropriate translation resource (e.g., auth.unknownProfile) in your locale
files and ensure the useTranslation import is added to the component if not
already present.
- Around line 31-42: getFailureMessage is recreated on every render even though
it only depends on authFailureInfo.failureType and the i18n function t; either
inline the switch where it's used or wrap the function's return value in useMemo
(dependent on [authFailureInfo.failureType, t]) to avoid unnecessary
re-creation. Locate the getFailureMessage helper in AuthFailureModal.tsx and
replace it with a useMemo hook that returns the switch result or move the switch
logic directly into the JSX where the message is consumed, keeping references to
authFailureInfo.failureType and t intact.
| const getFailureMessage = () => { | ||
| switch (authFailureInfo.failureType) { | ||
| case 'expired': | ||
| return t('auth.failure.tokenExpired', 'Your authentication token has expired.'); | ||
| case 'invalid': | ||
| return t('auth.failure.tokenInvalid', 'Your authentication token is invalid.'); | ||
| case 'missing': | ||
| return t('auth.failure.tokenMissing', 'No authentication token found.'); | ||
| default: | ||
| return t('auth.failure.authFailed', 'Authentication failed.'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider memoizing getFailureMessage or inlining the switch.
The getFailureMessage function is recreated on every render. Since it depends on authFailureInfo.failureType and t, you could use useMemo or simply inline the logic. This is a minor optimization.
♻️ Optional: Memoize with useMemo
+import { useMemo } from 'react';
...
- const getFailureMessage = () => {
- switch (authFailureInfo.failureType) {
+ const failureMessage = useMemo(() => {
+ switch (authFailureInfo?.failureType) {
case 'expired':
return t('auth.failure.tokenExpired', 'Your authentication token has expired.');
case 'invalid':
return t('auth.failure.tokenInvalid', 'Your authentication token is invalid.');
case 'missing':
return t('auth.failure.tokenMissing', 'No authentication token found.');
default:
return t('auth.failure.authFailed', 'Authentication failed.');
}
- };
-
- const failureMessage = getFailureMessage();
+ }, [authFailureInfo?.failureType, t]);🤖 Prompt for AI Agents
In `@apps/frontend/src/renderer/components/AuthFailureModal.tsx` around lines 31 -
42, getFailureMessage is recreated on every render even though it only depends
on authFailureInfo.failureType and the i18n function t; either inline the switch
where it's used or wrap the function's return value in useMemo (dependent on
[authFailureInfo.failureType, t]) to avoid unnecessary re-creation. Locate the
getFailureMessage helper in AuthFailureModal.tsx and replace it with a useMemo
hook that returns the switch result or move the switch logic directly into the
JSX where the message is consumed, keeping references to
authFailureInfo.failureType and t intact.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| <SDKRateLimitModal /> | ||
|
|
||
| {/* Auth Failure Modal - shows when Claude CLI encounters 401/auth errors */} | ||
| <AuthFailureModal onOpenSettings={() => setIsSettingsDialogOpen(true)} /> |
There was a problem hiding this comment.
Settings button doesn't navigate to Claude profiles section
Medium Severity
The AuthFailureModal's "Go to Settings" button only calls setIsSettingsDialogOpen(true) without setting setSettingsInitialSection('integrations'). According to the PR test plan, this button should navigate directly to Claude profiles settings (which are under the 'integrations' section), but currently it opens settings at the default view, leaving users to find Claude profiles manually.
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🟠 NEEDS REVISION
🟠 Needs revision - 2 issue(s) require attention.
2 issue(s) must be addressed (0 required, 2 recommended), 3 suggestions
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | Medium | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
Findings Summary
- Medium: 2 issue(s)
- Low: 3 issue(s)
Generated by Auto Claude PR Review
Findings (5 selected of 5 total)
🟡 [228209ac3536] [MEDIUM] Missing i18n translation keys for auth.failure namespace
📁 apps/frontend/src/renderer/components/AuthFailureModal.tsx:34
The component uses translation keys under 'auth.failure.*' namespace (auth.failure.title, auth.failure.tokenExpired, auth.failure.tokenInvalid, etc.) but these keys are not defined in either the English (en/common.json) or French (fr/common.json) translation files. Per CLAUDE.md requirements: 'Always use i18n translation keys for all user-facing text' and 'add the translation key to ALL language files'. While fallback strings are provided via t('key', 'fallback'), this violates project i18n standards.
Suggested fix:
Add the auth.failure.* translation keys to both apps/frontend/src/shared/i18n/locales/en/common.json and apps/frontend/src/shared/i18n/locales/fr/common.json. Example structure: { "auth": { "failure": { "title": "Authentication Required", "tokenExpired": "Your authentication token has expired.", ... } } }
🟡 [738d584c46f6] [MEDIUM] Incorrect i18n translation key path for Dismiss button
📁 apps/frontend/src/renderer/components/AuthFailureModal.tsx:104
The Dismiss button uses t('common.dismiss', 'Dismiss') but since the component uses useTranslation('common'), the correct key path should be 'labels.dismiss' (which exists at line 94 of common.json), not 'common.dismiss'.
Suggested fix:
Change line 104 from t('common.dismiss', 'Dismiss') to t('labels.dismiss', 'Dismiss')
🔵 [fffa8e1bbba1] [LOW] Dynamic require() inside event handler callback
📁 apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts:147
Using require() dynamically inside the event handler callback is inconsistent with the file's import style. While other files import claude-profile-manager via static ES imports without circular dependency issues, this file uses require() at runtime. This creates unnecessary runtime overhead and makes dependencies harder to track.
Suggested fix:
Move to top-level import: import { getClaudeProfileManager } from '../claude-profile-manager'; Then use directly in the handler.
🔵 [57e108fedd11] [LOW] Unused state property hasPendingAuthFailure
📁 apps/frontend/src/renderer/stores/auth-failure-store.ts:10
The store defines and updates hasPendingAuthFailure but this property is never consumed anywhere in the codebase. It's set to true in showAuthFailureModal, preserved in hideAuthFailureModal, and cleared in clearAuthFailure, but no component reads it. This appears to be dead code.
Suggested fix:
Either remove hasPendingAuthFailure from the store if not needed, or implement its intended usage (e.g., showing a badge/indicator). If keeping for future use, add a TODO comment.
🔵 [a82a37ea88d7] [LOW] Hardcoded 'Unknown Profile' string should use i18n
📁 apps/frontend/src/renderer/components/AuthFailureModal.tsx:28
The fallback string 'Unknown Profile' on line 28 is hardcoded and not wrapped with the t() translation function, unlike other user-facing text in the component.
Suggested fix:
Change line 28 from: const profileName = authFailureInfo.profileName || 'Unknown Profile'; to: const profileName = authFailureInfo.profileName || t('auth.failure.unknownProfile', 'Unknown Profile');
This review was generated by Auto Claude.
…ssues Address PR review findings: - Add auth.failure.* translation keys to en/common.json and fr/common.json - Fix 'common.dismiss' → 'labels.dismiss' for correct i18n key path - Fix hardcoded 'Unknown Profile' to use translation key - Replace dynamic require() with static import for claude-profile-manager - Add TODO comment for hasPendingAuthFailure explaining intended use Co-Authored-By: Claude Opus 4.5 <[email protected]>
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
🟡 Follow-up Review: Merge With Changes
🟡 Can merge - Minor suggestions noted, no blockers.
Resolution Status
- ✅ Resolved: 5 previous findings addressed
- ❌ Unresolved: 0 previous findings remain
- 🆕 New Issues: 3 new findings in recent changes
Finding Validation
- 🔍 Dismissed as False Positives: 1 findings were re-investigated and found to be incorrect
- ✓ Confirmed Valid: 0 findings verified as genuine issues
- 👤 Needs Human Review: 0 findings require manual verification
Verdict
✅ All 21 CI checks passing - no blocking CI issues.
Previous findings resolved: All 5 previous findings have been addressed:
- 2 MEDIUM findings (missing i18n keys, incorrect translation path) - FIXED
- 3 LOW findings (dynamic require, unused property, hardcoded string) - FIXED or DISMISSED
The unused hasPendingAuthFailure property was validated and dismissed as a false positive - it's documented intentional scaffolding for a planned sidebar indicator feature, following a common development pattern.
New findings: 3 LOW severity suggestions only:
- Consider sanitizing originalError before display (security hygiene)
- Track sidebar indicator feature as follow-up task
- Consider documenting Date/string IPC serialization behavior
Recommendation: Merge is safe. The author has effectively addressed all review feedback from the previous review. The 3 new suggestions are optional polish items that can be addressed post-merge if desired. The core auth failure handling feature is well-implemented with proper i18n support, event cleanup, and type definitions.
Review Process
Agents invoked: resolution-verifier, new-code-reviewer, finding-validator
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (3 selected of 3 total)
🔵 [NEWCODE-001] [LOW] Potential sensitive information exposure in error details
📁 apps/frontend/src/renderer/components/AuthFailureModal.tsx:90
The modal displays authFailureInfo.originalError in an expandable details section. If the backend passes raw error messages from the CLI, these could contain sensitive information like partial tokens or internal paths. The error is rendered directly without sanitization.
Suggested fix:
Consider sanitizing or filtering the originalError content before display, or truncate excessively long error messages.
🔵 [NEWCODE-002] [LOW] TODO comment indicating incomplete visual indicator feature
📁 apps/frontend/src/renderer/stores/auth-failure-store.ts:9
The store contains a TODO comment indicating hasPendingAuthFailure should show a visual indicator (badge/red dot) in the sidebar, but this is not yet implemented. Users may miss pending auth failures if they dismiss the modal.
Suggested fix:
Track as follow-up task to add visual indicator for pending auth failures.
🔵 [NEWCODE-003] [LOW] AuthFailureInfo detectedAt type discrepancy over IPC
📁 apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts:153
The detectedAt field is set as new Date() in main process but becomes a string over IPC serialization. The renderer handles this correctly with runtime conversion, but the type definition declares it as Date. Consider adding JSDoc comment noting the IPC behavior.
Suggested fix:
Add JSDoc comment to AuthFailureInfo type indicating detectedAt may be a string when received over IPC, or create separate IPC payload type.
This review was generated by Auto Claude.
Clarifies that Date objects become ISO strings when sent over IPC. Co-Authored-By: Claude Opus 4.5 <[email protected]>
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
✅ Auto Claude Review - APPROVED
Status: Ready to Merge
Summary: ## ✅ Follow-up Review: Ready To Merge
✅ Ready to merge - All checks passing and findings addressed.
Resolution Status
- ✅ Resolved: 2 previous findings addressed
- ❌ Unresolved: 1 previous findings remain
- 🆕 New Issues: 0 new findings in recent changes
Finding Validation
- 🔍 Dismissed as False Positives: 1 findings were re-investigated and found to be incorrect
- ✓ Confirmed Valid: 1 findings verified as genuine issues
- 👤 Needs Human Review: 0 findings require manual verification
Verdict
✅ All 21 CI checks passing. Of the 3 previous LOW-severity findings: (1) NEWCODE-003 was RESOLVED through added IPC serialization documentation, (2) NEWCODE-002 was DISMISSED as false positive - TODO comments documenting future enhancements are acceptable technical debt and not a code quality issue, (3) NEWCODE-001 remains valid but is appropriately LOW severity - the error details are hidden behind a collapsed section and CLI errors rarely contain actual tokens. No new issues were introduced. The single commit was a targeted documentation fix. All blocking concerns addressed.
Review Process
Agents invoked: resolution-verifier, finding-validator
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
💡 Suggestions (1)
These are non-blocking suggestions for consideration:
🔵 [NEWCODE-001] [LOW] [UNRESOLVED] Potential sensitive information exposure in error details
📁 apps/frontend/src/renderer/components/AuthFailureModal.tsx:90
The modal displays authFailureInfo.originalError in an expandable details section. If the backend passes raw error messages from the CLI, these could contain sensitive information like partial tokens or internal paths. The error is rendered directly without sanitization.
Resolution note: {authFailureInfo.originalError && (
{t('auth.failure.technicalDetails', 'Technical details')}
{authFailureInfo.originalError}
Suggested fix:
Consider sanitizing or filtering the originalError content before display, or truncate excessively long error messages.
This automated review found no blocking issues. The PR can be safely merged.
Generated by Auto Claude
Summary
Changes
AuthFailureInfointerface for auth failure eventsCLAUDE_AUTH_FAILUREIPC channel for main→renderer communicationauth-failureevent handler inagent-events-handlers.tsAuthFailureModalcomponent with i18n translation supportuseAuthFailureStoreZustand store for modal state managementuseIpc.tsTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Localization
✏️ Tip: You can customize this high-level summary in your review settings.