From 8a4b506671e927679b5402f7fab93da250490670 Mon Sep 17 00:00:00 2001 From: Bogdan Dragomir Date: Tue, 6 Jan 2026 14:09:01 +0200 Subject: [PATCH] fix: show OAuth terminal during profile authentication (#671) * fix: show OAuth terminal during profile authentication (#670) The authentication flow was creating a terminal to run `claude setup-token` but never displaying it to the user. This caused the "browser window will open" message to appear while the terminal remained hidden. Changes: - Add CLAUDE_PROFILE_LOGIN_TERMINAL IPC event to notify renderer when login terminal is created - Add onClaudeProfileLoginTerminal listener to preload API - Add addExternalTerminal method to terminal store for terminals created in main process - Listen for login terminal events in OAuthStep and IntegrationSettings to show the terminal in the UI - Remove misleading alert messages since terminal is now visible Fixes #670 * refactor: extract useClaudeLoginTerminal hook and remove process.env usage - Created custom hook at apps/frontend/src/renderer/hooks/useClaudeLoginTerminal.ts - Handles onClaudeProfileLoginTerminal event listener setup - Calls addExternalTerminal without cwd parameter (uses internal default) - Removes process.env usage from React components - Updated OAuthStep.tsx to use the new hook - Removed useTerminalStore import and addExternalTerminal usage - Replaced inline useEffect with useClaudeLoginTerminal hook call - Removed process.env.HOME and process.env.USERPROFILE references - Updated IntegrationSettings.tsx to use the new hook - Removed useTerminalStore import and addExternalTerminal usage - Replaced inline useEffect with useClaudeLoginTerminal hook call - Removed process.env.HOME and process.env.USERPROFILE references This fixes PR review comments for issue #670 by: 1. Extracting duplicate code into a reusable custom hook 2. Removing process.env usage from React components (addExternalTerminal has its own fallback) 3. Improving code maintainability and consistency Verified with npm run typecheck and npm run lint - no errors. * fix: address PR review feedback for OAuth terminal visibility - HIGH: Handle silent failure when max terminals reached by showing toast notification - MEDIUM: Check terminal creation result before sending IPC event - MEDIUM: Fix inconsistent max terminals check to exclude exited terminals - MEDIUM: Rename IPC channel from claude:profileLoginTerminal to terminal:authCreated - LOW: Add i18n translation for auth terminal title - LOW: Export useClaudeLoginTerminal hook from barrel file --------- Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com> --- .../main/ipc-handlers/terminal-handlers.ts | 15 ++++++-- apps/frontend/src/preload/api/terminal-api.ts | 18 +++++++++ .../components/onboarding/OAuthStep.tsx | 21 +++++----- .../settings/IntegrationSettings.tsx | 22 +++++------ apps/frontend/src/renderer/hooks/index.ts | 1 + .../renderer/hooks/useClaudeLoginTerminal.ts | 37 ++++++++++++++++++ .../src/renderer/lib/mocks/terminal-mock.ts | 1 + .../src/renderer/stores/terminal-store.ts | 38 +++++++++++++++++++ apps/frontend/src/shared/constants/ipc.ts | 1 + .../src/shared/i18n/locales/en/terminal.json | 4 ++ .../src/shared/i18n/locales/fr/terminal.json | 4 ++ apps/frontend/src/shared/types/ipc.ts | 6 +++ 12 files changed, 140 insertions(+), 28 deletions(-) create mode 100644 apps/frontend/src/renderer/hooks/useClaudeLoginTerminal.ts diff --git a/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts b/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts index b72a62ef..c5e98984 100644 --- a/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts @@ -338,7 +338,15 @@ export function registerTerminalHandlers( }); // Create a new terminal for the login process - await terminalManager.create({ id: terminalId, cwd: homeDir }); + const createResult = await terminalManager.create({ id: terminalId, cwd: homeDir }); + + // If terminal creation failed, return the error + if (!createResult.success) { + return { + success: false, + error: createResult.error || 'Failed to create terminal for authentication' + }; + } // Wait a moment for the terminal to initialize await new Promise(resolve => setTimeout(resolve, 500)); @@ -377,10 +385,11 @@ export function registerTerminalHandlers( // Write the login command to the terminal terminalManager.write(terminalId, `${loginCommand}\r`); - // Notify the renderer that a login terminal was created + // Notify the renderer that an auth terminal was created + // This allows the UI to display the terminal so users can see the OAuth flow const mainWindow = getMainWindow(); if (mainWindow) { - mainWindow.webContents.send('claude-profile-login-terminal', { + mainWindow.webContents.send(IPC_CHANNELS.TERMINAL_AUTH_CREATED, { terminalId, profileId, profileName: profile.name diff --git a/apps/frontend/src/preload/api/terminal-api.ts b/apps/frontend/src/preload/api/terminal-api.ts index d7421373..e7b509f8 100644 --- a/apps/frontend/src/preload/api/terminal-api.ts +++ b/apps/frontend/src/preload/api/terminal-api.ts @@ -73,6 +73,9 @@ export interface TerminalAPI { onTerminalOAuthToken: ( callback: (info: { terminalId: string; profileId?: string; email?: string; success: boolean; message?: string; detectedAt: string }) => void ) => () => void; + onTerminalAuthCreated: ( + callback: (info: { terminalId: string; profileId: string; profileName: string }) => void + ) => () => void; onTerminalClaudeBusy: (callback: (id: string, isBusy: boolean) => void) => () => void; // Claude Profile Management @@ -265,6 +268,21 @@ export const createTerminalAPI = (): TerminalAPI => ({ }; }, + onTerminalAuthCreated: ( + callback: (info: { terminalId: string; profileId: string; profileName: string }) => void + ): (() => void) => { + const handler = ( + _event: Electron.IpcRendererEvent, + info: { terminalId: string; profileId: string; profileName: string } + ): void => { + callback(info); + }; + ipcRenderer.on(IPC_CHANNELS.TERMINAL_AUTH_CREATED, handler); + return () => { + ipcRenderer.removeListener(IPC_CHANNELS.TERMINAL_AUTH_CREATED, handler); + }; + }, + onTerminalClaudeBusy: ( callback: (id: string, isBusy: boolean) => void ): (() => void) => { diff --git a/apps/frontend/src/renderer/components/onboarding/OAuthStep.tsx b/apps/frontend/src/renderer/components/onboarding/OAuthStep.tsx index 7584f864..4fad5f33 100644 --- a/apps/frontend/src/renderer/components/onboarding/OAuthStep.tsx +++ b/apps/frontend/src/renderer/components/onboarding/OAuthStep.tsx @@ -26,6 +26,7 @@ import { Label } from '../ui/label'; import { Card, CardContent } from '../ui/card'; import { cn } from '../../lib/utils'; import { loadClaudeProfiles as loadGlobalClaudeProfiles } from '../../stores/claude-profile-store'; +import { useClaudeLoginTerminal } from '../../hooks/useClaudeLoginTerminal'; import type { ClaudeProfile } from '../../../shared/types'; interface OAuthStepProps { @@ -92,6 +93,9 @@ export function OAuthStep({ onNext, onBack, onSkip }: OAuthStepProps) { loadClaudeProfiles(); }, []); + // Listen for login terminal creation - makes the terminal visible so user can see OAuth flow + useClaudeLoginTerminal(); + // Listen for OAuth authentication completion useEffect(() => { const unsubscribe = window.electronAPI.onTerminalOAuthToken(async (info) => { @@ -144,11 +148,8 @@ export function OAuthStep({ onNext, onBack, onSkip }: OAuthStepProps) { await loadClaudeProfiles(); setNewProfileName(''); - alert( - `Authenticating "${profileName}"...\n\n` + - `A browser window will open for you to log in with your Claude account.\n\n` + - `The authentication will be saved automatically once complete.` - ); + // Note: The terminal is now visible in the UI via the onTerminalAuthCreated event + // Users can see the 'claude setup-token' output directly } else { await loadClaudeProfiles(); alert(`Failed to start authentication: ${initResult.error || 'Please try again.'}`); @@ -222,15 +223,11 @@ export function OAuthStep({ onNext, onBack, onSkip }: OAuthStepProps) { setError(null); try { const initResult = await window.electronAPI.initializeClaudeProfile(profileId); - if (initResult.success) { - alert( - `Authenticating profile...\n\n` + - `A browser window will open for you to log in with your Claude account.\n\n` + - `The authentication will be saved automatically once complete.` - ); - } else { + if (!initResult.success) { alert(`Failed to start authentication: ${initResult.error || 'Please try again.'}`); } + // Note: If successful, the terminal is now visible in the UI via the onTerminalAuthCreated event + // Users can see the 'claude setup-token' output and complete OAuth flow directly } catch (err) { setError(err instanceof Error ? err.message : 'Failed to authenticate profile'); alert('Failed to start authentication. Please try again.'); diff --git a/apps/frontend/src/renderer/components/settings/IntegrationSettings.tsx b/apps/frontend/src/renderer/components/settings/IntegrationSettings.tsx index 785add21..6cfbc706 100644 --- a/apps/frontend/src/renderer/components/settings/IntegrationSettings.tsx +++ b/apps/frontend/src/renderer/components/settings/IntegrationSettings.tsx @@ -27,6 +27,7 @@ import { Switch } from '../ui/switch'; import { cn } from '../../lib/utils'; import { SettingsSection } from './SettingsSection'; import { loadClaudeProfiles as loadGlobalClaudeProfiles } from '../../stores/claude-profile-store'; +import { useClaudeLoginTerminal } from '../../hooks/useClaudeLoginTerminal'; import type { AppSettings, ClaudeProfile, ClaudeAutoSwitchSettings } from '../../../shared/types'; interface IntegrationSettingsProps { @@ -72,6 +73,9 @@ export function IntegrationSettings({ settings, onSettingsChange, isOpen }: Inte } }, [isOpen]); + // Listen for login terminal creation - makes the terminal visible so user can see OAuth flow + useClaudeLoginTerminal(); + // Listen for OAuth authentication completion useEffect(() => { const unsubscribe = window.electronAPI.onTerminalOAuthToken(async (info) => { @@ -126,12 +130,8 @@ export function IntegrationSettings({ settings, onSettingsChange, isOpen }: Inte if (initResult.success) { await loadClaudeProfiles(); setNewProfileName(''); - - alert( - `Authenticating "${profileName}"...\n\n` + - `A browser window will open for you to log in with your Claude account.\n\n` + - `The authentication will be saved automatically once complete.` - ); + // Note: The terminal is now visible in the UI via the onTerminalAuthCreated event + // Users can see the 'claude setup-token' output directly } else { await loadClaudeProfiles(); alert(`Failed to start authentication: ${initResult.error || 'Please try again.'}`); @@ -201,15 +201,11 @@ export function IntegrationSettings({ settings, onSettingsChange, isOpen }: Inte setAuthenticatingProfileId(profileId); try { const initResult = await window.electronAPI.initializeClaudeProfile(profileId); - if (initResult.success) { - alert( - `Authenticating profile...\n\n` + - `A browser window will open for you to log in with your Claude account.\n\n` + - `The authentication will be saved automatically once complete.` - ); - } else { + if (!initResult.success) { alert(`Failed to start authentication: ${initResult.error || 'Please try again.'}`); } + // Note: If successful, the terminal is now visible in the UI via the onTerminalAuthCreated event + // Users can see the 'claude setup-token' output and complete OAuth flow directly } catch (err) { console.error('Failed to authenticate profile:', err); alert('Failed to start authentication. Please try again.'); diff --git a/apps/frontend/src/renderer/hooks/index.ts b/apps/frontend/src/renderer/hooks/index.ts index 0793d045..21f70a6a 100644 --- a/apps/frontend/src/renderer/hooks/index.ts +++ b/apps/frontend/src/renderer/hooks/index.ts @@ -1,3 +1,4 @@ // Export all custom hooks export { useIpcListeners } from './useIpc'; export { useVirtualizedTree } from './useVirtualizedTree'; +export { useClaudeLoginTerminal } from './useClaudeLoginTerminal'; diff --git a/apps/frontend/src/renderer/hooks/useClaudeLoginTerminal.ts b/apps/frontend/src/renderer/hooks/useClaudeLoginTerminal.ts new file mode 100644 index 00000000..616dfb26 --- /dev/null +++ b/apps/frontend/src/renderer/hooks/useClaudeLoginTerminal.ts @@ -0,0 +1,37 @@ +import { useEffect } from 'react'; +import { useTranslation } from 'react-i18next'; +import { useTerminalStore } from '../stores/terminal-store'; +import { toast } from './use-toast'; + +/** + * Custom hook to handle Claude profile login terminal visibility. + * Listens for onTerminalAuthCreated events and adds the terminal + * to the store so users can see the OAuth flow output. + */ +export function useClaudeLoginTerminal() { + const { t } = useTranslation('terminal'); + const addExternalTerminal = useTerminalStore((state) => state.addExternalTerminal); + + useEffect(() => { + const unsubscribe = window.electronAPI.onTerminalAuthCreated((info) => { + // Add the terminal to the store so it becomes visible in the UI + // This allows users to see the 'claude setup-token' output and complete the OAuth flow + // cwd is optional and defaults to HOME or '~' in addExternalTerminal + const terminal = addExternalTerminal( + info.terminalId, + t('auth.terminalTitle', { profileName: info.profileName }) + ); + + // If terminal creation failed (max terminals reached), show a notification + // The terminal was created in main process but we can't show it in UI + if (!terminal) { + toast({ + title: t('auth.maxTerminalsReached'), + variant: 'destructive', + }); + } + }); + + return unsubscribe; + }, [addExternalTerminal, t]); +} diff --git a/apps/frontend/src/renderer/lib/mocks/terminal-mock.ts b/apps/frontend/src/renderer/lib/mocks/terminal-mock.ts index 6faf96d7..a8a52b19 100644 --- a/apps/frontend/src/renderer/lib/mocks/terminal-mock.ts +++ b/apps/frontend/src/renderer/lib/mocks/terminal-mock.ts @@ -91,5 +91,6 @@ export const terminalMock = { onTerminalClaudeSession: () => () => {}, onTerminalRateLimit: () => () => {}, onTerminalOAuthToken: () => () => {}, + onTerminalAuthCreated: () => () => {}, onTerminalClaudeBusy: () => () => {} }; diff --git a/apps/frontend/src/renderer/stores/terminal-store.ts b/apps/frontend/src/renderer/stores/terminal-store.ts index 02adc498..57296387 100644 --- a/apps/frontend/src/renderer/stores/terminal-store.ts +++ b/apps/frontend/src/renderer/stores/terminal-store.ts @@ -41,6 +41,8 @@ interface TerminalState { // Actions addTerminal: (cwd?: string, projectPath?: string) => Terminal | null; addRestoredTerminal: (session: TerminalSession) => Terminal; + // Add a terminal with a specific ID (for terminals created in main process, like OAuth login terminals) + addExternalTerminal: (id: string, title: string, cwd?: string, projectPath?: string) => Terminal | null; removeTerminal: (id: string) => void; updateTerminal: (id: string, updates: Partial) => void; setActiveTerminal: (id: string | null) => void; @@ -133,6 +135,42 @@ export const useTerminalStore = create((set, get) => ({ return restoredTerminal; }, + addExternalTerminal: (id: string, title: string, cwd?: string, projectPath?: string) => { + const state = get(); + + // Check if terminal with this ID already exists + const existingTerminal = state.terminals.find(t => t.id === id); + if (existingTerminal) { + // Just activate it and return it + set({ activeTerminalId: id }); + return existingTerminal; + } + + // Use the same logic as canAddTerminal - count only non-exited terminals + // This ensures consistency and doesn't block new terminals when only exited ones exist + const activeTerminalCount = state.terminals.filter(t => t.status !== 'exited').length; + if (activeTerminalCount >= state.maxTerminals) { + return null; + } + + const newTerminal: Terminal = { + id, + title, + status: 'running', // External terminals are already running + cwd: cwd || process.env.HOME || '~', + createdAt: new Date(), + isClaudeMode: false, + projectPath, + }; + + set((state) => ({ + terminals: [...state.terminals, newTerminal], + activeTerminalId: newTerminal.id, + })); + + return newTerminal; + }, + removeTerminal: (id: string) => { // Clean up buffer manager terminalBufferManager.dispose(id); diff --git a/apps/frontend/src/shared/constants/ipc.ts b/apps/frontend/src/shared/constants/ipc.ts index 6e59bef4..c23d9f4c 100644 --- a/apps/frontend/src/shared/constants/ipc.ts +++ b/apps/frontend/src/shared/constants/ipc.ts @@ -88,6 +88,7 @@ export const IPC_CHANNELS = { TERMINAL_CLAUDE_SESSION: 'terminal:claudeSession', // Claude session ID captured TERMINAL_RATE_LIMIT: 'terminal:rateLimit', // Claude Code rate limit detected TERMINAL_OAUTH_TOKEN: 'terminal:oauthToken', // OAuth token captured from setup-token output + TERMINAL_AUTH_CREATED: 'terminal:authCreated', // Auth terminal created for OAuth flow TERMINAL_CLAUDE_BUSY: 'terminal:claudeBusy', // Claude Code busy state (for visual indicator) // Claude profile management (multi-account support) diff --git a/apps/frontend/src/shared/i18n/locales/en/terminal.json b/apps/frontend/src/shared/i18n/locales/en/terminal.json index 8db749f1..b29808a2 100644 --- a/apps/frontend/src/shared/i18n/locales/en/terminal.json +++ b/apps/frontend/src/shared/i18n/locales/en/terminal.json @@ -3,6 +3,10 @@ "expand": "Expand terminal", "collapse": "Collapse terminal" }, + "auth": { + "terminalTitle": "Auth: {{profileName}}", + "maxTerminalsReached": "Cannot open auth terminal: maximum terminals reached. Close a terminal first." + }, "worktree": { "create": "Worktree", "createNew": "New Worktree", diff --git a/apps/frontend/src/shared/i18n/locales/fr/terminal.json b/apps/frontend/src/shared/i18n/locales/fr/terminal.json index 525307de..80867cde 100644 --- a/apps/frontend/src/shared/i18n/locales/fr/terminal.json +++ b/apps/frontend/src/shared/i18n/locales/fr/terminal.json @@ -3,6 +3,10 @@ "expand": "Agrandir le terminal", "collapse": "Reduire le terminal" }, + "auth": { + "terminalTitle": "Auth: {{profileName}}", + "maxTerminalsReached": "Impossible d'ouvrir le terminal d'auth: nombre maximum de terminaux atteint. Fermez un terminal d'abord." + }, "worktree": { "create": "Worktree", "createNew": "Nouveau Worktree", diff --git a/apps/frontend/src/shared/types/ipc.ts b/apps/frontend/src/shared/types/ipc.ts index fbdc1fa0..d8da007e 100644 --- a/apps/frontend/src/shared/types/ipc.ts +++ b/apps/frontend/src/shared/types/ipc.ts @@ -225,6 +225,12 @@ export interface ElectronAPI { message?: string; detectedAt: string }) => void) => () => void; + /** Listen for auth terminal creation - allows UI to display the OAuth terminal */ + onTerminalAuthCreated: (callback: (info: { + terminalId: string; + profileId: string; + profileName: string + }) => void) => () => void; /** Listen for Claude busy state changes (for visual indicator: red=busy, green=idle) */ onTerminalClaudeBusy: (callback: (id: string, isBusy: boolean) => void) => () => void;