Handle Claude chat logout recovery#679
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@copilot review but do not make fixes |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR adds a fast-fail path for Claude authentication failures in the backend chat service, extends Claude auth-error text detection patterns, and introduces renderer UI for unauthenticated errors with retry-turn and auth-recovery events, cross-pane resend logic, and a sticky login prompt bar. ChangesClaude auth fast-fail and recovery
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/desktop/src/renderer/components/chat/AgentChatMessageList.tsx (1)
3177-3183: 🚀 Performance & Scalability | 🔵 TrivialDuplicated
AgentCliAuthCardinvocation.The unauthenticated branch and the generic fallback branch both construct an
AgentCliAuthCardwith identical props (agentCli,laneId,chatSessionId,runtimeName,onRevealTerminal). Consider extracting a small local variable/helper for the shared props object to avoid drift between the two call sites.Also applies to: 3224-3230
🤖 Prompt for 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. In `@apps/desktop/src/renderer/components/chat/AgentChatMessageList.tsx` around lines 3177 - 3183, The duplicated AgentCliAuthCard call in AgentChatMessageList should be deduplicated so the unauthenticated and fallback branches stay in sync. Extract the shared props into a local variable or small helper near AgentChatMessageList, and use that shared object at both AgentCliAuthCard call sites (including the other matching branch mentioned in the review) so agentCli, laneId, chatSessionId, runtimeName, and onRevealTerminal cannot drift.apps/desktop/src/renderer/components/chat/AgentChatPane.tsx (1)
6348-6384: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test coverage for the CHAT_AUTH_RECOVERED_EVENT dispatch path.
This is new, non-trivial logic (locate last unauthenticated error, scan forward for done/text/tool_call recovery signals, dedupe by
sessionId:index), but only the retry/resend path is covered by the added test. Worth a test asserting the event fires (with the rightsessionId) once a recovery signal follows the error.Want me to draft this test alongside the existing "resends the latest user message" test in
AgentChatPane.test.tsx?🤖 Prompt for 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. In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx` around lines 6348 - 6384, Add test coverage for the new CHAT_AUTH_RECOVERED_EVENT dispatch logic in AgentChatPane’s useEffect. In AgentChatPane.test.tsx, verify that when selectedEventsForDisplay contains a last unauthenticated error followed by a recovery signal (done/completed, non-empty text, or tool_call), the effect dispatches CHAT_AUTH_RECOVERED_EVENT exactly once with the correct sessionId, and does not dispatch again for the same sessionId:index key. Use the existing AgentChatPane and CHAT_AUTH_RECOVERED_EVENT symbols to locate the behavior.
🤖 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 `@apps/desktop/src/renderer/components/chat/AgentChatMessageList.tsx`:
- Around line 3173-3198: The unauthenticated card wrapper in
AgentChatMessageList uses a hardcoded Claude border color, which can conflict
with the agent-specific accent already chosen by AgentCliAuthCard. Update the
outer card styling to derive its border accent from agentCli.agent the same way
the inner auth card does, so the wrapper and AgentCliAuthCard stay visually
consistent for non-Claude agents.
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 6304-6333: The resend flow in resendLastUserMessageForAuthRetry
should match sendMessageOrSteerIfBusy by handling a turn-already-active race
instead of surfacing the raw send error. Update the retry path in AgentChatPane
so that when window.ade.agentChat.send fails because the session already has an
active turn, it falls back to the existing steer behavior for that session
rather than setting the backend error directly. Keep the current auth-retry
behavior and reuse the same session/turn detection logic used by
sendMessageOrSteerIfBusy.
---
Nitpick comments:
In `@apps/desktop/src/renderer/components/chat/AgentChatMessageList.tsx`:
- Around line 3177-3183: The duplicated AgentCliAuthCard call in
AgentChatMessageList should be deduplicated so the unauthenticated and fallback
branches stay in sync. Extract the shared props into a local variable or small
helper near AgentChatMessageList, and use that shared object at both
AgentCliAuthCard call sites (including the other matching branch mentioned in
the review) so agentCli, laneId, chatSessionId, runtimeName, and
onRevealTerminal cannot drift.
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 6348-6384: Add test coverage for the new CHAT_AUTH_RECOVERED_EVENT
dispatch logic in AgentChatPane’s useEffect. In AgentChatPane.test.tsx, verify
that when selectedEventsForDisplay contains a last unauthenticated error
followed by a recovery signal (done/completed, non-empty text, or tool_call),
the effect dispatches CHAT_AUTH_RECOVERED_EVENT exactly once with the correct
sessionId, and does not dispatch again for the same sessionId:index key. Use the
existing AgentChatPane and CHAT_AUTH_RECOVERED_EVENT symbols to locate the
behavior.
🪄 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: 4e7f8641-d75c-4581-919c-977e72a3dbd0
⛔ Files ignored due to path filters (1)
docs/features/chat/README.mdis excluded by!docs/**
📒 Files selected for processing (10)
apps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/renderer/components/chat/AgentChatMessageList.test.tsxapps/desktop/src/renderer/components/chat/AgentChatMessageList.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/chat/AgentCliAuthCard.test.tsxapps/desktop/src/renderer/components/chat/AgentCliAuthCard.tsxapps/desktop/src/renderer/lib/claudeAuthPrompt.test.tsapps/desktop/src/renderer/lib/claudeAuthPrompt.ts
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@codex review |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d70081c35d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
arul28
force-pushed
the
ade/titled-move-unmapped-lane-banner-29486227
branch
from
d7440d3 to
6ad035e
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ad035ea60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b2cc41750
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
arul28
force-pushed
the
ade/titled-move-unmapped-lane-banner-29486227
branch
from
b0b0ec9 to
a961e1c
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a961e1cc8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Validation
git diff --checkenv PATH=/Users/arul/.asdf/installs/nodejs/22.13.1/bin:$PATH npx vitest run src/main/services/chat/agentChatService.test.ts src/renderer/components/chat/AgentCliAuthCard.test.tsx src/renderer/components/chat/AgentChatPane.test.tsx src/renderer/components/chat/AgentChatMessageList.test.tsx src/renderer/lib/claudeAuthPrompt.test.tsenv PATH=/Users/arul/.asdf/installs/nodejs/22.13.1/bin:$PATH npm --prefix apps/desktop run typecheckenv PATH=/Users/arul/.asdf/installs/nodejs/22.13.1/bin:$PATH node scripts/validate-docs.mjsnpm --prefix apps/desktop run lint(passes with existing warnings)env PATH=/Users/arul/.asdf/installs/nodejs/22.13.1/bin:$PATH npm --prefix apps/ade-cli run typecheckenv PATH=/Users/arul/.asdf/installs/nodejs/22.13.1/bin:$PATH npm --prefix apps/ade-cli testenv PATH=/Users/arul/.asdf/installs/nodejs/22.13.1/bin:$PATH npx vitest run src/tuiClientADE PR: https://ade-app.dev/open?type=pr&repo=arul28%2FADE&number=679
Summary by CodeRabbit
New Features
Bug Fixes
Greptile Summary
This PR stops the Claude SDK from exhausting its full retry budget on 401/logout errors by throwing
CLAUDE_RUNTIME_AUTH_ERRORon the first definitive auth signal, then renders the resulting error event as an inline re-login card (AgentCliAuthCard) with a "Retry turn" button that resends the last user message after a forced provider-health refresh.agentChatService.ts:failClaudeTurnUnauthenticated()guards four auth-signal paths (auth_status, api_retry 401, assistantauthentication_failed, result errors matchingisClaudeRuntimeAuthError), deduplicates the notice emission, and always re-throws so the existing catch path closes the query and emits the decorated error event.AgentChatPane.tsx: Listens forCHAT_RETRY_AUTH_TURN_EVENTto resend the last user message (with full attachment/context replay), dispatchesCHAT_AUTH_RETRY_REJECTED_EVENTimmediately if a submit is already in flight, and detects post-auth-error recovery to dispatchCHAT_AUTH_RECOVERED_EVENT; recovery keys are content-stable (turnId/itemId/timestamp) avoiding the previously noted index-sensitivity.AgentCliAuthCard.tsx: Handles the three window events for retry/rejection/recovery state, uses per-agent accent tokens (terracotta for Claude, amber for others), and collapses to a "Reconnected" chip on recovery.Confidence Score: 5/5
Safe to merge — the fast-fail logic is well-guarded, the retry/recovery event wiring is tested end-to-end, and no existing error paths are removed.
All three fast-fail trigger sites throw via a single idempotent helper; the once-guard prevents duplicate notices while the throw is unconditional, so every caller gets the expected catch-path behaviour. The recovery dedup key is now content-stable. Two findings are cosmetic: a regex that could match 'claude' appearing contextually in an otherwise unrelated error message, and copy-button hover colours that stay amber on the terracotta Claude card.
claudeAuthPrompt.ts — the new regex pattern; AgentCliAuthCard.tsx — CommandCopyButton accent colours
Important Files Changed
failClaudeTurnUnauthenticated()helper that fast-fails on 401/auth signals (auth_status, api_retry, assistant error, result errors) instead of exhausting SDK retries; once guard flag prevents duplicate notices while always rethrowing the auth error for the catch path.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant SDK as Claude SDK participant SVC as agentChatService participant PANE as AgentChatPane participant CARD as AgentCliAuthCard participant UI as Transcript / Composer SDK->>SVC: "assistant{error:"authentication_failed"} / api_retry{error_status:401} / auth_status{error}" SVC->>SVC: "failClaudeTurnUnauthenticated()<br/>emit system_notice "logged out"<br/>throw CLAUDE_RUNTIME_AUTH_ERROR" SVC->>PANE: "emit error event<br/>errorInfo.agentCli.category="unauthenticated"" PANE->>UI: "render AgentCliAuthCard (terracotta)<br/>+ sticky ClaudeLoginPromptButton above composer" Note over CARD,PANE: User runs claude auth login, then clicks Retry turn CARD->>PANE: "window event CHAT_RETRY_AUTH_TURN_EVENT {sessionId}" PANE->>PANE: "resendLastUserMessageForAuthRetry()<br/>force-refresh provider health" PANE->>SDK: agentChat.send(last user message) alt Submit already in flight or no user message PANE->>CARD: "CHAT_AUTH_RETRY_REJECTED_EVENT {sessionId}" CARD->>CARD: clearRetrying() — spinner reset immediately else Turn succeeds SDK->>SVC: "done{status:"completed"}" SVC->>PANE: done event PANE->>PANE: detect recovery (auth error followed by success) PANE->>CARD: "CHAT_AUTH_RECOVERED_EVENT {sessionId}" CARD->>UI: collapse to Reconnected to Claude Code end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant SDK as Claude SDK participant SVC as agentChatService participant PANE as AgentChatPane participant CARD as AgentCliAuthCard participant UI as Transcript / Composer SDK->>SVC: "assistant{error:"authentication_failed"} / api_retry{error_status:401} / auth_status{error}" SVC->>SVC: "failClaudeTurnUnauthenticated()<br/>emit system_notice "logged out"<br/>throw CLAUDE_RUNTIME_AUTH_ERROR" SVC->>PANE: "emit error event<br/>errorInfo.agentCli.category="unauthenticated"" PANE->>UI: "render AgentCliAuthCard (terracotta)<br/>+ sticky ClaudeLoginPromptButton above composer" Note over CARD,PANE: User runs claude auth login, then clicks Retry turn CARD->>PANE: "window event CHAT_RETRY_AUTH_TURN_EVENT {sessionId}" PANE->>PANE: "resendLastUserMessageForAuthRetry()<br/>force-refresh provider health" PANE->>SDK: agentChat.send(last user message) alt Submit already in flight or no user message PANE->>CARD: "CHAT_AUTH_RETRY_REJECTED_EVENT {sessionId}" CARD->>CARD: clearRetrying() — spinner reset immediately else Turn succeeds SDK->>SVC: "done{status:"completed"}" SVC->>PANE: done event PANE->>PANE: detect recovery (auth error followed by success) PANE->>CARD: "CHAT_AUTH_RECOVERED_EVENT {sessionId}" CARD->>UI: collapse to Reconnected to Claude Code endComments Outside Diff (1)
General comment
textHasClaudeAuthError('GitHub request failed: Invalid authentication credentials'), even though the PR contract says generic/non-Claude authentication strings should be rejected so only Claude auth failures trigger the Claude login recovery presentation. The runtime harness shows this exact row failing on head with expected=false and actual=true.CLAUDE_INVALID_CREDENTIALS_PATTERNSincludes the broad pattern/\binvalid\s+authentication\s+credentials\b/iinapps/desktop/src/renderer/lib/claudeAuthPrompt.ts, andtextHasClaudeAuthErrorapplies it without requiring Claude,/login,claude auth login, or API 401 Claude-specific context.Reviews (6): Last reviewed commit: "Show auth retry CTA in compact chats" | Re-trigger Greptile