feat(auth): handle auth flow errors gracefully (#1256)

3 files changed, 55 insertions, 19 deletions

diff --git a/packages/cli/src/ui/App.tsx b/packages/cli/src/ui/App.tsx
index c481ebd3..b89a20ed 100644
--- a/packages/cli/src/ui/App.tsx
+++ b/packages/cli/src/ui/App.tsx
@@ -587,11 +587,6 @@ const App = ({ config, settings, startupWarnings = [] }: AppProps) => {
             </Box>
           ) : isAuthDialogOpen ? (
             <Box flexDirection="column">
-              {authError && (
-                <Box marginBottom={1}>
-                  <Text color={Colors.AccentRed}>{authError}</Text>
-                </Box>
-              )}
               <AuthDialog
                 onSelect={handleAuthSelect}
                 onHighlight={handleAuthHighlight}
diff --git a/packages/cli/src/ui/hooks/useAuthCommand.ts b/packages/cli/src/ui/hooks/useAuthCommand.ts
index a9b1cb1e..2c2b5f93 100644
--- a/packages/cli/src/ui/hooks/useAuthCommand.ts
+++ b/packages/cli/src/ui/hooks/useAuthCommand.ts
@@ -6,7 +6,12 @@
 
 import { useState, useCallback, useEffect } from 'react';
 import { LoadedSettings, SettingScope } from '../../config/settings.js';
-import { AuthType, Config, clearCachedCredentialFile } from '@gemini-cli/core';
+import {
+  AuthType,
+  Config,
+  clearCachedCredentialFile,
+  getErrorMessage,
+} from '@gemini-cli/core';
 
 async function performAuthFlow(authMethod: AuthType, config: Config) {
   await config.refreshAuth(authMethod);
@@ -22,16 +27,36 @@ export const useAuthCommand = (
     settings.merged.selectedAuthType === undefined,
   );
 
-  useEffect(() => {
-    if (!isAuthDialogOpen) {
-      performAuthFlow(settings.merged.selectedAuthType as AuthType, config);
-    }
-  }, [isAuthDialogOpen, settings, config]);
-
   const openAuthDialog = useCallback(() => {
     setIsAuthDialogOpen(true);
   }, []);
 
+  useEffect(() => {
+    const authFlow = async () => {
+      if (isAuthDialogOpen || !settings.merged.selectedAuthType) {
+        return;
+      }
+
+      try {
+        await performAuthFlow(
+          settings.merged.selectedAuthType as AuthType,
+          config,
+        );
+      } catch (e) {
+        const errorMessage =
+          settings.merged.selectedAuthType ===
+          AuthType.LOGIN_WITH_GOOGLE_PERSONAL
+            ? `Failed to login. Ensure your Google account is not an enterprise account.
+Message: ${getErrorMessage(e)}`
+            : `Failed to login. Message: ${getErrorMessage(e)}`;
+        setAuthError(errorMessage);
+        openAuthDialog();
+      }
+    };
+
+    void authFlow();
+  }, [isAuthDialogOpen, settings, config, setAuthError, openAuthDialog]);
+
   const handleAuthSelect = useCallback(
     async (authMethod: string | undefined, scope: SettingScope) => {
       if (authMethod) {
diff --git a/packages/cli/src/ui/hooks/usePhraseCycler.test.ts b/packages/cli/src/ui/hooks/usePhraseCycler.test.ts
index acabf2c0..ec04aca6 100644
--- a/packages/cli/src/ui/hooks/usePhraseCycler.test.ts
+++ b/packages/cli/src/ui/hooks/usePhraseCycler.test.ts
@@ -63,6 +63,20 @@ describe('usePhraseCycler', () => {
   });
 
   it('should reset to a witty phrase when isActive becomes true after being false (and not waiting)', () => {
+    // Ensure there are at least two phrases for this test to be meaningful.
+    if (WITTY_LOADING_PHRASES.length < 2) {
+      return;
+    }
+
+    // Mock Math.random to make the test deterministic.
+    let callCount = 0;
+    vi.spyOn(Math, 'random').mockImplementation(() => {
+      // Cycle through 0, 1, 0, 1, ...
+      const val = callCount % 2;
+      callCount++;
+      return val / WITTY_LOADING_PHRASES.length;
+    });
+
     const { result, rerender } = renderHook(
       ({ isActive, isWaiting }) => usePhraseCycler(isActive, isWaiting),
       { initialProps: { isActive: false, isWaiting: false } },
@@ -72,25 +86,27 @@ describe('usePhraseCycler', () => {
     rerender({ isActive: true, isWaiting: false });
     const firstActivePhrase = result.current;
     expect(WITTY_LOADING_PHRASES).toContain(firstActivePhrase);
+    // With our mock, this should be the first phrase.
+    expect(firstActivePhrase).toBe(WITTY_LOADING_PHRASES[0]);
 
     act(() => {
       vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS);
     });
-    // Phrase should change if enough phrases and interval passed
-    if (WITTY_LOADING_PHRASES.length > 1) {
-      expect(result.current).not.toBe(firstActivePhrase);
-    }
-    expect(WITTY_LOADING_PHRASES).toContain(result.current);
+
+    // Phrase should change to the second phrase.
+    expect(result.current).not.toBe(firstActivePhrase);
+    expect(result.current).toBe(WITTY_LOADING_PHRASES[1]);
 
     // Set to inactive - should reset to the default initial phrase
     rerender({ isActive: false, isWaiting: false });
     expect(result.current).toBe(WITTY_LOADING_PHRASES[0]);
 
-    // Set back to active - should pick a random witty phrase
+    // Set back to active - should pick a random witty phrase (which our mock controls)
     act(() => {
       rerender({ isActive: true, isWaiting: false });
     });
-    expect(WITTY_LOADING_PHRASES).toContain(result.current);
+    // The random mock will now return 0, so it should be the first phrase again.
+    expect(result.current).toBe(WITTY_LOADING_PHRASES[0]);
   });
 
   it('should clear phrase interval on unmount when active', () => {