fix: close claude CLI window after successful /login #1926
fix: close claude CLI window after successful /login #1926OriginalUtkin wants to merge 2 commits intoAndyMik90:developfrom
Conversation
|
|
📝 WalkthroughWalkthroughThe pull request updates product branding from "Auto Claude" to "Aperant" in documentation and modifies the OAuth authentication flow to automatically close the terminal after successful credential verification instead of waiting for onboarding completion. Changes
Sequence DiagramsequenceDiagram
participant User
participant AuthTerminal
participant Handler as OAuth Handler
participant Electron
User->>Handler: Initiate OAuth Login
Handler->>Handler: Receive OAuth Token
Handler->>AuthTerminal: needsOnboarding = false
AuthTerminal->>AuthTerminal: Set status: success
AuthTerminal->>AuthTerminal: Invoke onAuthSuccess
AuthTerminal->>AuthTerminal: Start 1500ms timeout
AuthTerminal->>Electron: destroyTerminal()
Electron->>AuthTerminal: Terminal destroyed
AuthTerminal->>AuthTerminal: Invoke onClose
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Summary of ChangesHello, 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 significantly improves the user experience during the login process by ensuring the authentication window correctly closes after a successful login. Previously, the window would remain open due to an incorrect onboarding flag, leading to a confusing and repetitive login prompt. The changes involve adjusting the authentication handler to signal that no further onboarding is required and adding a mechanism to automatically dismiss the login terminal. Additionally, a branding update from 'Auto Claude' to 'Aperant' was applied to the documentation. Highlights
Changelog
Activity
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
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thanks for your first PR!
A maintainer will review it soon. Please make sure:
- Your branch is synced with
develop - CI checks pass
- You've followed our contribution guide
Welcome to the Auto Claude community!
![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 addresses a user experience issue where the login window would not close after a successful /login command. The changes correctly identify that the /login flow is non-interactive and remove the logic that was waiting for an onboarding screen. The fix involves setting needsOnboarding: false in the backend and adding logic in the frontend to auto-close the terminal on success. The implementation is sound. I've added one suggestion to improve maintainability by refactoring duplicated code in AuthTerminal.tsx.
| // Auto-close after a brief delay to show success UI | ||
| successTimeoutRef.current = setTimeout(() => { | ||
| if (isCreatedRef.current) { | ||
| window.electronAPI.destroyTerminal(terminalId).catch(console.error); | ||
| isCreatedRef.current = false; | ||
| } | ||
| onClose(); | ||
| }, 1500); |
There was a problem hiding this comment.
This auto-close logic is duplicated. A similar block exists in the onTerminalOnboardingComplete handler (lines 325-331). To avoid duplication and improve maintainability, you could extract this logic into a helper function. Also, the timeout duration 1500 is a magic number and could be defined as a constant.
For example, you could define these within the component:
const SUCCESS_CLOSE_DELAY_MS = 1500;
const scheduleCloseOnSuccess = useCallback(() => {
// Auto-close after a brief delay to show success UI
successTimeoutRef.current = setTimeout(() => {
if (isCreatedRef.current) {
window.electronAPI.destroyTerminal(terminalId).catch(console.error);
isCreatedRef.current = false;
}
onClose();
}, SUCCESS_CLOSE_DELAY_MS);
}, [terminalId, onClose]);Then, you can call scheduleCloseOnSuccess() here and in the other location.
| // Auto-close after a brief delay to show success UI | |
| successTimeoutRef.current = setTimeout(() => { | |
| if (isCreatedRef.current) { | |
| window.electronAPI.destroyTerminal(terminalId).catch(console.error); | |
| isCreatedRef.current = false; | |
| } | |
| onClose(); | |
| }, 1500); | |
| scheduleCloseOnSuccess(); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/main/terminal/claude-integration-handler.ts (1)
698-706: 🧹 Nitpick | 🔵 TrivialConsider documenting reduced scope of
handleOnboardingComplete.With
needsOnboarding: falsenow returned for all auth terminal OAuth flows, this handler will only activate for non-auth terminal scenarios (if any). The early return at line 704 (if (!terminal.awaitingOnboardingComplete)) means auth terminals will never reach the onboarding completion logic.If this handler is still needed for other flows, a brief comment clarifying its remaining use cases would improve maintainability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/frontend/src/main/terminal/claude-integration-handler.ts` around lines 698 - 706, Document that handleOnboardingComplete now only runs for non-auth terminal flows by updating the comment around the early return in handleOnboardingComplete: note that because OAuth flows now return needsOnboarding: false, auth terminals won't set terminal.awaitingOnboardingComplete and thus this handler only applies to other terminal types (mention any specific cases if applicable), reference terminal.awaitingOnboardingComplete and the function handleOnboardingComplete so future readers know why the early return exists and what non-auth scenarios should trigger the handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/frontend/src/main/terminal/claude-integration-handler.ts`:
- Around line 543-554: Add a clarifying comment inside handleOnboardingComplete
immediately above the guard that checks terminal.awaitingOnboardingComplete
explaining that this handler is only used for terminals that explicitly set
awaitingOnboardingComplete (i.e., non-auth flows) and will never run for
OAuth/login terminals because those close immediately after OAuth completes;
reference the guard (terminal.awaitingOnboardingComplete) and the OAuth flow
behavior described in the earlier safeSendToRenderer block to make the intent
clear for future maintainers.
---
Outside diff comments:
In `@apps/frontend/src/main/terminal/claude-integration-handler.ts`:
- Around line 698-706: Document that handleOnboardingComplete now only runs for
non-auth terminal flows by updating the comment around the early return in
handleOnboardingComplete: note that because OAuth flows now return
needsOnboarding: false, auth terminals won't set
terminal.awaitingOnboardingComplete and thus this handler only applies to other
terminal types (mention any specific cases if applicable), reference
terminal.awaitingOnboardingComplete and the function handleOnboardingComplete so
future readers know why the early return exists and what non-auth scenarios
should trigger the handler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8d2dcecb-dccf-47fd-88ef-7787615a7f5e
📒 Files selected for processing (3)
CLAUDE.mdapps/frontend/src/main/terminal/claude-integration-handler.tsapps/frontend/src/renderer/components/settings/AuthTerminal.tsx
| // Credentials verified - close the auth terminal immediately. | ||
| // claude /login exits after OAuth completes and does not continue as an interactive | ||
| // session, so the welcome screen never appears and waiting for it would leave the | ||
| // window open forever. | ||
| safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_OAUTH_TOKEN, { | ||
| terminalId: terminal.id, | ||
| profileId, | ||
| email: emailFromOutput || keychainCreds.email || profile?.email, | ||
| success: true, | ||
| needsOnboarding: true, | ||
| needsOnboarding: false, | ||
| detectedAt: new Date().toISOString() | ||
| } as OAuthTokenEvent); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for where awaitingOnboardingComplete is set to true
echo "=== Searching for awaitingOnboardingComplete assignments ==="
rg -n 'awaitingOnboardingComplete\s*=' --type=ts
echo ""
echo "=== Searching for references to handleOnboardingComplete ==="
rg -n 'handleOnboardingComplete|onOnboardingComplete' --type=ts -C2Repository: AndyMik90/Aperant
Length of output: 2515
🏁 Script executed:
#!/bin/bash
# Read the handleOnboardingComplete function to see what's at line 704
echo "=== Reading handleOnboardingComplete function (lines 698-750) ==="
sed -n '698,750p' apps/frontend/src/main/terminal/claude-integration-handler.ts
echo ""
echo "=== Searching for where onOnboardingComplete is triggered ==="
rg -n 'onOnboardingComplete|ONBOARDING_COMPLETE' --type=ts --type=tsx -C3
echo ""
echo "=== Checking if needsOnboarding affects event triggering ==="
rg -n 'needsOnboarding' --type=ts --type=tsx -C2Repository: AndyMik90/Aperant
Length of output: 2534
🏁 Script executed:
#!/bin/bash
# Search for awaitingOnboardingComplete initialization or being set to true
echo "=== Searching for awaitingOnboardingComplete initialization ==="
rg -n 'awaitingOnboardingComplete' apps/frontend/src/main/terminal/ -A1 -B1
echo ""
echo "=== Checking terminal type definition ==="
rg -n 'awaitingOnboardingComplete' apps/frontend/src/main/terminal/types.ts -A2 -B2
echo ""
echo "=== Searching for where it should be set to true ==="
rg -n 'needsOnboarding.*true' apps/frontend/src/main/terminal/ -B3 -A3Repository: AndyMik90/Aperant
Length of output: 1431
🏁 Script executed:
#!/bin/bash
# Search handleOAuthToken to see what it does
echo "=== Reading handleOAuthToken function ==="
rg -n 'export function handleOAuthToken' apps/frontend/src/main/terminal/claude-integration-handler.ts -A50 | head -70
echo ""
echo "=== Look for any assignment of awaitingOnboardingComplete to true ==="
rg -n 'awaitingOnboardingComplete\s*=\s*true' apps/frontend/src/
echo ""
echo "=== Check what happens with needsOnboarding in the original logic ==="
rg -n 'needsOnboarding' apps/frontend/src/main/terminal/claude-integration-handler.ts -B3 -A3Repository: AndyMik90/Aperant
Length of output: 3318
🏁 Script executed:
#!/bin/bash
# Search for where terminals are created and how they might set flags
echo "=== Search for terminal creation and initialization ==="
rg -n 'createTerminal|new TerminalProcess' apps/frontend/src/main/terminal/ -B2 -A5
echo ""
echo "=== Check if there are regular Claude terminals (non-auth) ==="
rg -n 'claude-session|createClaudeTerminal' apps/frontend/src/main/terminal/ -B3 -A3
echo ""
echo "=== Search for where handleOnboardingComplete might be triggered for non-auth terminals ==="
rg -n 'isOnboardingCompleteOutput|ONBOARDING_COMPLETE' apps/frontend/src/main/Repository: AndyMik90/Aperant
Length of output: 3685
🏁 Script executed:
#!/bin/bash
# Understand the broader context - what terminal types exist?
echo "=== Search for terminal type constants/enums ==="
rg -n 'TerminalType|AUTH|CLAUDE' apps/frontend/src/main/terminal/types.ts -B2 -A2
echo ""
echo "=== Check terminal-event-handler to see all terminal event paths ==="
sed -n '1,60p' apps/frontend/src/main/terminal/terminal-event-handler.ts
echo ""
echo "=== Look at the full flow - when is onOnboardingComplete callback invoked? ==="
sed -n '40,60p' apps/frontend/src/main/terminal/terminal-event-handler.tsRepository: AndyMik90/Aperant
Length of output: 2887
Add a comment to handleOnboardingComplete clarifying its scope for auth terminals.
The guard at line 704 (if (!terminal.awaitingOnboardingComplete) { return; }) means handleOnboardingComplete is effectively dead code for OAuth login terminals since awaitingOnboardingComplete is never set to true for them. This is intentional—as the comment at lines 543-546 explains, claude /login exits immediately after OAuth completes without showing an onboarding screen, so skipping the onboarding handler is correct.
For future maintainers, consider adding a comment in handleOnboardingComplete (around the guard) noting that it only activates when awaitingOnboardingComplete is true, which currently applies only to non-auth terminal flows, to clarify the scope and prevent confusion about why the function appears unreachable for auth terminals.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/frontend/src/main/terminal/claude-integration-handler.ts` around lines
543 - 554, Add a clarifying comment inside handleOnboardingComplete immediately
above the guard that checks terminal.awaitingOnboardingComplete explaining that
this handler is only used for terminals that explicitly set
awaitingOnboardingComplete (i.e., non-auth flows) and will never run for
OAuth/login terminals because those close immediately after OAuth completes;
reference the guard (terminal.awaitingOnboardingComplete) and the OAuth flow
behavior described in the earlier safeSendToRenderer block to make the intent
clear for future maintainers.
OriginalUtkin
changed the title
2 participants
Base Branch
developbranch (required for all feature/fix PRs)main(hotfix only - maintainers)Description
Just a minor UX improvement ensures login window actually closes after a successful login. That preventing a confusing continuous
/loginloop. Currently, it seems to be stuck in login window even after onboarding prompts are answered.Eventually,
CLAUDE_CONFIG_DIRis passed explicitly per-profile as an environment variable when spawning agents.Further questions from
Claude CLI'sworkspace/folder during onboarding set up ~/.claude/ defaults, but projectbypasses those entirely by injecting its own
configDirpath at runtime.So those post-login questions only affect Claude CLI's own default behavior when run standalone. Those default ain't used by Aperent as it always passes its own configuration. It seems to be safe to close the auth window straight after credentials are confirmed in Keychain without answering CLIs prompt after login is confirmed.
The Problem
The code was setting
needsOnboarding: trueand waiting indefinitely for a welcome screen that never appears. This left the auth window stuck open after a successful login, continuously prompting the user to log in again.Technical Changes
claude-integration-handler.ts: SendneedsOnboarding: falseafter a successful login (for both thekeychain-tokenandconfigDir-fallbackpaths). Removed theawaitingOnboardingCompleteflag that was waiting for a welcome screen that never arrives during theclaude /loginflow.Type of Change
Area
Testing level:
Platform Testing Checklist
CRITICAL: This project supports Windows, macOS, and Linux. Platform-specific bugs are a common source of breakage.
platform/module instead of directprocess.platformchecksfindExecutable()or platform abstractions)If you only have access to one OS: CI now tests on all platforms. Ensure all checks pass before submitting.
CI/Testing Requirements
Screenshots
Before:
TBD
After:
Behavior after stays the same except window is now closed right after successful authentication
Breaking Changes
Breaking: No
Summary by CodeRabbit
Documentation
Improvements