feat(ui): improve startup warnings UX with dismissal and show-count limits (#19584)

packages/cli/src/ui/components/Notifications.test.tsx

@@ -4,7 +4,12 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { render, persistentStateMock } from '../../test-utils/render.js';
+import {
+  persistentStateMock,
+  renderWithProviders,
+} from '../../test-utils/render.js';
+import { createMockSettings } from '../../test-utils/settings.js';
+import type { LoadedSettings } from '../../config/settings.js';
 import { waitFor } from '../../test-utils/async.js';
 import { Notifications } from './Notifications.js';
 import { describe, it, expect, vi, beforeEach } from 'vitest';
@@ -13,6 +18,7 @@ import { useUIState, type UIState } from '../contexts/UIStateContext.js';
 import { useIsScreenReaderEnabled } from 'ink';
 import * as fs from 'node:fs/promises';
 import { act } from 'react';
+import { WarningPriority } from '@google/gemini-cli-core';
 
 // Mock dependencies
 vi.mock('../contexts/AppContext.js');
@@ -61,22 +67,18 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
     ...actual,
     GEMINI_DIR: '.gemini',
     homedir: () => '/mock/home',
+    WarningPriority: {
+      Low: 'low',
+      High: 'high',
+    },
     Storage: {
       ...actual.Storage,
       getGlobalTempDir: () => '/mock/temp',
+      getGlobalSettingsPath: () => '/mock/home/.gemini/settings.json',
     },
   };
 });
 
-vi.mock('../../config/settings.js', () => ({
-  DEFAULT_MODEL_CONFIGS: {},
-  LoadedSettings: class {
-    constructor() {
-      // this.merged = {};
-    }
-  },
-}));
-
 describe('Notifications', () => {
   const mockUseAppContext = vi.mocked(useAppContext);
   const mockUseUIState = vi.mocked(useUIState);
@@ -84,9 +86,14 @@ describe('Notifications', () => {
   const mockFsAccess = vi.mocked(fs.access);
   const mockFsUnlink = vi.mocked(fs.unlink);
 
+  let settings: LoadedSettings;
+
   beforeEach(() => {
     vi.clearAllMocks();
     persistentStateMock.reset();
+    settings = createMockSettings({
+      ui: { useAlternateBuffer: true },
+    });
     mockUseAppContext.mockReturnValue({
       startupWarnings: [],
       version: '1.0.0',
@@ -100,60 +107,195 @@ describe('Notifications', () => {
   });
 
   it('renders nothing when no notifications', async () => {
-    const { lastFrame, waitUntilReady, unmount } = render(<Notifications />);
+    const { lastFrame, waitUntilReady, unmount } = renderWithProviders(
+      <Notifications />,
+      {
+        settings,
+        width: 100,
+      },
+    );
     await waitUntilReady();
     expect(lastFrame({ allowEmpty: true })).toBe('');
     unmount();
   });
 
-  it.each([[['Warning 1']], [['Warning 1', 'Warning 2']]])(
-    'renders startup warnings: %s',
-    async (warnings) => {
-      mockUseAppContext.mockReturnValue({
-        startupWarnings: warnings,
-        version: '1.0.0',
-      } as AppState);
-      const { lastFrame, waitUntilReady, unmount } = render(<Notifications />);
-      await waitUntilReady();
-      const output = lastFrame();
-      warnings.forEach((warning) => {
-        expect(output).toContain(warning);
-      });
-      unmount();
-    },
-  );
+  it.each([
+    [[{ id: 'w1', message: 'Warning 1', priority: WarningPriority.High }]],
+    [
+      [
+        { id: 'w1', message: 'Warning 1', priority: WarningPriority.High },
+        { id: 'w2', message: 'Warning 2', priority: WarningPriority.High },
+      ],
+    ],
+  ])('renders startup warnings: %s', async (warnings) => {
+    const appState = {
+      startupWarnings: warnings,
+      version: '1.0.0',
+    } as AppState;
+    mockUseAppContext.mockReturnValue(appState);
+    const { lastFrame, waitUntilReady, unmount } = renderWithProviders(
+      <Notifications />,
+      {
+        appState,
+        settings,
+        width: 100,
+      },
+    );
+    await waitUntilReady();
+    const output = lastFrame();
+    warnings.forEach((warning) => {
+      expect(output).toContain(warning.message);
+    });
+    unmount();
+  });
+
+  it('increments show count for low priority warnings', async () => {
+    const warnings = [
+      { id: 'low-1', message: 'Low priority 1', priority: WarningPriority.Low },
+    ];
+    const appState = {
+      startupWarnings: warnings,
+      version: '1.0.0',
+    } as AppState;
+    mockUseAppContext.mockReturnValue(appState);
+
+    const { waitUntilReady, unmount } = renderWithProviders(<Notifications />, {
+      appState,
+      settings,
+      width: 100,
+    });
+    await waitUntilReady();
+
+    expect(persistentStateMock.set).toHaveBeenCalledWith(
+      'startupWarningCounts',
+      { 'low-1': 1 },
+    );
+    unmount();
+  });
+
+  it('filters out low priority warnings that exceeded max show count', async () => {
+    const warnings = [
+      { id: 'low-1', message: 'Low priority 1', priority: WarningPriority.Low },
+      {
+        id: 'high-1',
+        message: 'High priority 1',
+        priority: WarningPriority.High,
+      },
+    ];
+    const appState = {
+      startupWarnings: warnings,
+      version: '1.0.0',
+    } as AppState;
+    mockUseAppContext.mockReturnValue(appState);
+
+    persistentStateMock.setData({
+      startupWarningCounts: { 'low-1': 3 },
+    });
+
+    const { lastFrame, waitUntilReady, unmount } = renderWithProviders(
+      <Notifications />,
+      {
+        appState,
+        settings,
+        width: 100,
+      },
+    );
+    await waitUntilReady();
+    const output = lastFrame();
+    expect(output).not.toContain('Low priority 1');
+    expect(output).toContain('High priority 1');
+    unmount();
+  });
+
+  it('dismisses warnings on keypress', async () => {
+    const warnings = [
+      {
+        id: 'high-1',
+        message: 'High priority 1',
+        priority: WarningPriority.High,
+      },
+    ];
+    const appState = {
+      startupWarnings: warnings,
+      version: '1.0.0',
+    } as AppState;
+    mockUseAppContext.mockReturnValue(appState);
+
+    const { lastFrame, stdin, waitUntilReady, unmount } = renderWithProviders(
+      <Notifications />,
+      {
+        appState,
+        settings,
+        width: 100,
+      },
+    );
+    await waitUntilReady();
+    expect(lastFrame()).toContain('High priority 1');
+
+    await act(async () => {
+      stdin.write('a');
+    });
+    await waitUntilReady();
+
+    expect(lastFrame({ allowEmpty: true })).not.toContain('High priority 1');
+    unmount();
+  });
 
   it('renders init error', async () => {
-    mockUseUIState.mockReturnValue({
+    const uiState = {
       initError: 'Something went wrong',
       streamingState: 'idle',
       updateInfo: null,
-    } as unknown as UIState);
-    const { lastFrame, waitUntilReady, unmount } = render(<Notifications />);
+    } as unknown as UIState;
+    mockUseUIState.mockReturnValue(uiState);
+    const { lastFrame, waitUntilReady, unmount } = renderWithProviders(
+      <Notifications />,
+      {
+        uiState,
+        settings,
+        width: 100,
+      },
+    );
     await waitUntilReady();
     expect(lastFrame()).toMatchSnapshot();
     unmount();
   });
 
   it('does not render init error when streaming', async () => {
-    mockUseUIState.mockReturnValue({
+    const uiState = {
       initError: 'Something went wrong',
       streamingState: 'responding',
       updateInfo: null,
-    } as unknown as UIState);
-    const { lastFrame, waitUntilReady, unmount } = render(<Notifications />);
+    } as unknown as UIState;
+    mockUseUIState.mockReturnValue(uiState);
+    const { lastFrame, waitUntilReady, unmount } = renderWithProviders(
+      <Notifications />,
+      {
+        uiState,
+        settings,
+        width: 100,
+      },
+    );
     await waitUntilReady();
     expect(lastFrame({ allowEmpty: true })).toBe('');
     unmount();
   });
 
   it('renders update notification', async () => {
-    mockUseUIState.mockReturnValue({
+    const uiState = {
       initError: null,
       streamingState: 'idle',
       updateInfo: { message: 'Update available' },
-    } as unknown as UIState);
-    const { lastFrame, waitUntilReady, unmount } = render(<Notifications />);
+    } as unknown as UIState;
+    mockUseUIState.mockReturnValue(uiState);
+    const { lastFrame, waitUntilReady, unmount } = renderWithProviders(
+      <Notifications />,
+      {
+        uiState,
+        settings,
+        width: 100,
+      },
+    );
     await waitUntilReady();
     expect(lastFrame()).toMatchSnapshot();
     unmount();
@@ -164,7 +306,13 @@ describe('Notifications', () => {
     persistentStateMock.setData({ hasSeenScreenReaderNudge: false });
     mockFsAccess.mockRejectedValue(new Error('No legacy file'));
 
-    const { lastFrame, waitUntilReady, unmount } = render(<Notifications />);
+    const { lastFrame, waitUntilReady, unmount } = renderWithProviders(
+      <Notifications />,
+      {
+        settings,
+        width: 100,
+      },
+    );
     await waitUntilReady();
 
     expect(lastFrame()).toContain('screen reader-friendly view');
@@ -182,7 +330,10 @@ describe('Notifications', () => {
     persistentStateMock.setData({ hasSeenScreenReaderNudge: undefined });
     mockFsAccess.mockResolvedValue(undefined);
 
-    const { waitUntilReady, unmount } = render(<Notifications />);
+    const { waitUntilReady, unmount } = renderWithProviders(<Notifications />, {
+      settings,
+      width: 100,
+    });
 
     await act(async () => {
       await waitUntilReady();
@@ -201,7 +352,13 @@ describe('Notifications', () => {
     mockUseIsScreenReaderEnabled.mockReturnValue(true);
     persistentStateMock.setData({ hasSeenScreenReaderNudge: true });
 
-    const { lastFrame, waitUntilReady, unmount } = render(<Notifications />);
+    const { lastFrame, waitUntilReady, unmount } = renderWithProviders(
+      <Notifications />,
+      {
+        settings,
+        width: 100,
+      },
+    );
     await waitUntilReady();
 
     expect(lastFrame({ allowEmpty: true })).toBe('');

packages/cli/src/ui/components/Notifications.tsx

@@ -5,15 +5,22 @@
  */
 
 import { Box, Text, useIsScreenReaderEnabled } from 'ink';
-import { useEffect, useState } from 'react';
+import { useEffect, useState, useMemo, useRef, useCallback } from 'react';
 import { useAppContext } from '../contexts/AppContext.js';
 import { useUIState } from '../contexts/UIStateContext.js';
 import { theme } from '../semantic-colors.js';
 import { StreamingState } from '../types.js';
 import { UpdateNotification } from './UpdateNotification.js';
 import { persistentState } from '../../utils/persistentState.js';
+import { useKeypress } from '../hooks/useKeypress.js';
+import { KeypressPriority } from '../contexts/KeypressContext.js';
 
-import { GEMINI_DIR, Storage, homedir } from '@google/gemini-cli-core';
+import {
+  GEMINI_DIR,
+  Storage,
+  homedir,
+  WarningPriority,
+} from '@google/gemini-cli-core';
 
 import * as fs from 'node:fs/promises';
 import path from 'node:path';
@@ -25,12 +32,13 @@ const screenReaderNudgeFilePath = path.join(
   'seen_screen_reader_nudge.json',
 );
 
+const MAX_STARTUP_WARNING_SHOW_COUNT = 3;
+
 export const Notifications = () => {
   const { startupWarnings } = useAppContext();
   const { initError, streamingState, updateInfo } = useUIState();
 
   const isScreenReaderEnabled = useIsScreenReaderEnabled();
-  const showStartupWarnings = startupWarnings.length > 0;
   const showInitError =
     initError && streamingState !== StreamingState.Responding;
 
@@ -38,6 +46,57 @@ export const Notifications = () => {
     persistentState.get('hasSeenScreenReaderNudge'),
   );
 
+  const [dismissed, setDismissed] = useState(false);
+
+  // Track if we have already incremented the show count in this session
+  const hasIncrementedRef = useRef(false);
+
+  // Filter warnings based on persistent state count if low priority
+  const visibleWarnings = useMemo(() => {
+    if (dismissed) return [];
+
+    const counts = persistentState.get('startupWarningCounts') || {};
+    return startupWarnings.filter((w) => {
+      if (w.priority === WarningPriority.Low) {
+        const count = counts[w.id] || 0;
+        return count < MAX_STARTUP_WARNING_SHOW_COUNT;
+      }
+      return true;
+    });
+  }, [startupWarnings, dismissed]);
+
+  const showStartupWarnings = visibleWarnings.length > 0;
+
+  // Increment counts for low priority warnings when shown
+  useEffect(() => {
+    if (visibleWarnings.length > 0 && !hasIncrementedRef.current) {
+      const counts = { ...(persistentState.get('startupWarningCounts') || {}) };
+      let changed = false;
+      visibleWarnings.forEach((w) => {
+        if (w.priority === WarningPriority.Low) {
+          counts[w.id] = (counts[w.id] || 0) + 1;
+          changed = true;
+        }
+      });
+      if (changed) {
+        persistentState.set('startupWarningCounts', counts);
+      }
+      hasIncrementedRef.current = true;
+    }
+  }, [visibleWarnings]);
+
+  const handleKeyPress = useCallback(() => {
+    if (showStartupWarnings) {
+      setDismissed(true);
+    }
+    return false;
+  }, [showStartupWarnings]);
+
+  useKeypress(handleKeyPress, {
+    isActive: showStartupWarnings,
+    priority: KeypressPriority.Critical,
+  });
+
   useEffect(() => {
     const checkLegacyScreenReaderNudge = async () => {
       if (hasSeenScreenReaderNudge !== undefined) return;
@@ -89,13 +148,13 @@ export const Notifications = () => {
       {updateInfo && <UpdateNotification message={updateInfo.message} />}
       {showStartupWarnings && (
         <Box marginY={1} flexDirection="column">
-          {startupWarnings.map((warning, index) => (
+          {visibleWarnings.map((warning, index) => (
             <Box key={index} flexDirection="row">
               <Box width={3}>
                 <Text color={theme.status.warning}>⚠ </Text>
               </Box>
               <Box flexGrow={1}>
-                <Text color={theme.status.warning}>{warning}</Text>
+                <Text color={theme.status.warning}>{warning.message}</Text>
               </Box>
             </Box>
           ))}