From fe428936d5cb3bb4de09a3d3e61cfadcff6d4415 Mon Sep 17 00:00:00 2001 From: Spencer Date: Fri, 20 Feb 2026 13:22:45 -0500 Subject: [PATCH] feat(ui): improve startup warnings UX with dismissal and show-count limits (#19584) --- packages/cli/src/gemini.test.tsx | 8 +- packages/cli/src/gemini.tsx | 14 +- packages/cli/src/ui/AppContainer.test.tsx | 17 +- packages/cli/src/ui/AppContainer.tsx | 3 +- .../src/ui/components/Notifications.test.tsx | 235 +++++++++++++++--- .../cli/src/ui/components/Notifications.tsx | 69 ++++- packages/cli/src/ui/contexts/AppContext.tsx | 3 +- packages/cli/src/utils/persistentState.ts | 1 + .../cli/src/utils/userStartupWarnings.test.ts | 72 +++--- packages/cli/src/utils/userStartupWarnings.ts | 26 +- packages/core/src/utils/compatibility.test.ts | 88 ++++++- packages/core/src/utils/compatibility.ts | 76 +++++- 12 files changed, 503 insertions(+), 109 deletions(-) diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 976d832abd..538fb8ee4e 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -38,6 +38,8 @@ import { appEvents, AppEvent } from './utils/events.js'; import { type Config, type ResumedSessionData, + type StartupWarning, + WarningPriority, debugLogger, coreEvents, AuthType, @@ -1193,7 +1195,9 @@ describe('startInteractiveUI', () => { }, }, } as LoadedSettings; - const mockStartupWarnings = ['warning1']; + const mockStartupWarnings: StartupWarning[] = [ + { id: 'w1', message: 'warning1', priority: WarningPriority.High }, + ]; const mockWorkspaceRoot = '/root'; const mockInitializationResult = { authError: null, @@ -1226,7 +1230,7 @@ describe('startInteractiveUI', () => { async function startTestInteractiveUI( config: Config, settings: LoadedSettings, - startupWarnings: string[], + startupWarnings: StartupWarning[], workspaceRoot: string, resumedSessionData: ResumedSessionData | undefined, initializationResult: InitializationResult, diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 7b385453bf..dc3d64046b 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -11,6 +11,7 @@ import { loadCliConfig, parseArguments } from './config/config.js'; import * as cliConfig from './config/config.js'; import { readStdin } from './utils/readStdin.js'; import { basename } from 'node:path'; +import { createHash } from 'node:crypto'; import v8 from 'node:v8'; import os from 'node:os'; import dns from 'node:dns'; @@ -37,6 +38,8 @@ import { cleanupExpiredSessions, } from './utils/sessionCleanup.js'; import { + type StartupWarning, + WarningPriority, type Config, type ResumedSessionData, type OutputPayload, @@ -180,7 +183,7 @@ ${reason.stack}` export async function startInteractiveUI( config: Config, settings: LoadedSettings, - startupWarnings: string[], + startupWarnings: StartupWarning[], workspaceRoot: string = process.cwd(), resumedSessionData: ResumedSessionData | undefined, initializationResult: InitializationResult, @@ -668,8 +671,13 @@ export async function main() { } let input = config.getQuestion(); - const startupWarnings = [ - ...(await getStartupWarnings()), + const rawStartupWarnings = await getStartupWarnings(); + const startupWarnings: StartupWarning[] = [ + ...rawStartupWarnings.map((message) => ({ + id: `startup-${createHash('sha256').update(message).digest('hex').substring(0, 16)}`, + message, + priority: WarningPriority.High, + })), ...(await getUserStartupWarnings(settings.merged)), ]; diff --git a/packages/cli/src/ui/AppContainer.test.tsx b/packages/cli/src/ui/AppContainer.test.tsx index 5554ecb58a..3aeef34292 100644 --- a/packages/cli/src/ui/AppContainer.test.tsx +++ b/packages/cli/src/ui/AppContainer.test.tsx @@ -26,6 +26,8 @@ import { CoreEvent, type UserFeedbackPayload, type ResumedSessionData, + type StartupWarning, + WarningPriority, AuthType, type AgentDefinition, CoreToolCallStatus, @@ -248,7 +250,7 @@ describe('AppContainer State Management', () => { config?: Config; version?: string; initResult?: InitializationResult; - startupWarnings?: string[]; + startupWarnings?: StartupWarning[]; resumedSessionData?: ResumedSessionData; } = {}) => ( @@ -501,7 +503,18 @@ describe('AppContainer State Management', () => { }); it('renders with startup warnings', async () => { - const startupWarnings = ['Warning 1', 'Warning 2']; + const startupWarnings: StartupWarning[] = [ + { + id: 'w1', + message: 'Warning 1', + priority: WarningPriority.High, + }, + { + id: 'w2', + message: 'Warning 2', + priority: WarningPriority.High, + }, + ]; let unmount: () => void; await act(async () => { diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index b0a2ade4b3..661a96d305 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -41,6 +41,7 @@ import { checkPermissions } from './hooks/atCommandProcessor.js'; import { MessageType, StreamingState } from './types.js'; import { ToolActionsProvider } from './contexts/ToolActionsContext.js'; import { + type StartupWarning, type EditorType, type Config, type IdeInfo, @@ -186,7 +187,7 @@ function isToolAwaitingConfirmation( interface AppContainerProps { config: Config; - startupWarnings?: string[]; + startupWarnings?: StartupWarning[]; version: string; initializationResult: InitializationResult; resumedSessionData?: ResumedSessionData; diff --git a/packages/cli/src/ui/components/Notifications.test.tsx b/packages/cli/src/ui/components/Notifications.test.tsx index 4929d61fd5..f732ff2f23 100644 --- a/packages/cli/src/ui/components/Notifications.test.tsx +++ b/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(); + const { lastFrame, waitUntilReady, unmount } = renderWithProviders( + , + { + 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(); - 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( + , + { + 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(, { + 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( + , + { + 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( + , + { + 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(); + } as unknown as UIState; + mockUseUIState.mockReturnValue(uiState); + const { lastFrame, waitUntilReady, unmount } = renderWithProviders( + , + { + 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(); + } as unknown as UIState; + mockUseUIState.mockReturnValue(uiState); + const { lastFrame, waitUntilReady, unmount } = renderWithProviders( + , + { + 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(); + } as unknown as UIState; + mockUseUIState.mockReturnValue(uiState); + const { lastFrame, waitUntilReady, unmount } = renderWithProviders( + , + { + 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(); + const { lastFrame, waitUntilReady, unmount } = renderWithProviders( + , + { + 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(); + const { waitUntilReady, unmount } = renderWithProviders(, { + 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(); + const { lastFrame, waitUntilReady, unmount } = renderWithProviders( + , + { + settings, + width: 100, + }, + ); await waitUntilReady(); expect(lastFrame({ allowEmpty: true })).toBe(''); diff --git a/packages/cli/src/ui/components/Notifications.tsx b/packages/cli/src/ui/components/Notifications.tsx index 1573ef682c..4753f8f6cb 100644 --- a/packages/cli/src/ui/components/Notifications.tsx +++ b/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 && } {showStartupWarnings && ( - {startupWarnings.map((warning, index) => ( + {visibleWarnings.map((warning, index) => ( ⚠ - {warning} + {warning.message} ))} diff --git a/packages/cli/src/ui/contexts/AppContext.tsx b/packages/cli/src/ui/contexts/AppContext.tsx index 791c8a73ac..ea5de0c105 100644 --- a/packages/cli/src/ui/contexts/AppContext.tsx +++ b/packages/cli/src/ui/contexts/AppContext.tsx @@ -5,10 +5,11 @@ */ import { createContext, useContext } from 'react'; +import type { StartupWarning } from '@google/gemini-cli-core'; export interface AppState { version: string; - startupWarnings: string[]; + startupWarnings: StartupWarning[]; } export const AppContext = createContext(null); diff --git a/packages/cli/src/utils/persistentState.ts b/packages/cli/src/utils/persistentState.ts index cbdf1fc6cb..4fc51d0e14 100644 --- a/packages/cli/src/utils/persistentState.ts +++ b/packages/cli/src/utils/persistentState.ts @@ -15,6 +15,7 @@ interface PersistentStateData { tipsShown?: number; hasSeenScreenReaderNudge?: boolean; focusUiEnabled?: boolean; + startupWarningCounts?: Record; // Add other persistent state keys here as needed } diff --git a/packages/cli/src/utils/userStartupWarnings.test.ts b/packages/cli/src/utils/userStartupWarnings.test.ts index 131d77f580..41ed061166 100644 --- a/packages/cli/src/utils/userStartupWarnings.test.ts +++ b/packages/cli/src/utils/userStartupWarnings.test.ts @@ -13,7 +13,10 @@ import { isFolderTrustEnabled, isWorkspaceTrusted, } from '../config/trustedFolders.js'; -import { getCompatibilityWarnings } from '@google/gemini-cli-core'; +import { + getCompatibilityWarnings, + WarningPriority, +} from '@google/gemini-cli-core'; // Mock os.homedir to control the home directory in tests vi.mock('os', async (importOriginal) => { @@ -31,6 +34,10 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { ...actual, homedir: () => os.homedir(), getCompatibilityWarnings: vi.fn().mockReturnValue([]), + WarningPriority: { + Low: 'low', + High: 'high', + }, }; }); @@ -65,12 +72,13 @@ describe('getUserStartupWarnings', () => { it('should return a warning when running in home directory', async () => { const warnings = await getUserStartupWarnings({}, homeDir); expect(warnings).toContainEqual( - expect.stringContaining( - 'Warning you are running Gemini CLI in your home directory', - ), - ); - expect(warnings).toContainEqual( - expect.stringContaining('warning can be disabled in /settings'), + expect.objectContaining({ + id: 'home-directory', + message: expect.stringContaining( + 'Warning you are running Gemini CLI in your home directory', + ), + priority: WarningPriority.Low, + }), ); }); @@ -78,9 +86,7 @@ describe('getUserStartupWarnings', () => { const projectDir = path.join(testRootDir, 'project'); await fs.mkdir(projectDir); const warnings = await getUserStartupWarnings({}, projectDir); - expect(warnings).not.toContainEqual( - expect.stringContaining('home directory'), - ); + expect(warnings.find((w) => w.id === 'home-directory')).toBeUndefined(); }); it('should not return a warning when showHomeDirectoryWarning is false', async () => { @@ -88,9 +94,7 @@ describe('getUserStartupWarnings', () => { { ui: { showHomeDirectoryWarning: false } }, homeDir, ); - expect(warnings).not.toContainEqual( - expect.stringContaining('home directory'), - ); + expect(warnings.find((w) => w.id === 'home-directory')).toBeUndefined(); }); it('should not return a warning when folder trust is enabled and workspace is trusted', async () => { @@ -101,9 +105,7 @@ describe('getUserStartupWarnings', () => { }); const warnings = await getUserStartupWarnings({}, homeDir); - expect(warnings).not.toContainEqual( - expect.stringContaining('home directory'), - ); + expect(warnings.find((w) => w.id === 'home-directory')).toBeUndefined(); }); }); @@ -112,10 +114,11 @@ describe('getUserStartupWarnings', () => { const rootDir = path.parse(testRootDir).root; const warnings = await getUserStartupWarnings({}, rootDir); expect(warnings).toContainEqual( - expect.stringContaining('root directory'), - ); - expect(warnings).toContainEqual( - expect.stringContaining('folder structure will be used'), + expect.objectContaining({ + id: 'root-directory', + message: expect.stringContaining('root directory'), + priority: WarningPriority.High, + }), ); }); @@ -123,9 +126,7 @@ describe('getUserStartupWarnings', () => { const projectDir = path.join(testRootDir, 'project'); await fs.mkdir(projectDir); const warnings = await getUserStartupWarnings({}, projectDir); - expect(warnings).not.toContainEqual( - expect.stringContaining('root directory'), - ); + expect(warnings.find((w) => w.id === 'root-directory')).toBeUndefined(); }); }); @@ -133,24 +134,37 @@ describe('getUserStartupWarnings', () => { it('should handle errors when checking directory', async () => { const nonExistentPath = path.join(testRootDir, 'non-existent'); const warnings = await getUserStartupWarnings({}, nonExistentPath); - const expectedWarning = + const expectedMessage = 'Could not verify the current directory due to a file system error.'; - expect(warnings).toEqual([expectedWarning, expectedWarning]); + expect(warnings).toEqual([ + expect.objectContaining({ message: expectedMessage }), + expect.objectContaining({ message: expectedMessage }), + ]); }); }); describe('compatibility warnings', () => { it('should include compatibility warnings by default', async () => { - vi.mocked(getCompatibilityWarnings).mockReturnValue(['Comp warning 1']); + const compWarning = { + id: 'comp-1', + message: 'Comp warning 1', + priority: WarningPriority.High, + }; + vi.mocked(getCompatibilityWarnings).mockReturnValue([compWarning]); const projectDir = path.join(testRootDir, 'project'); await fs.mkdir(projectDir); const warnings = await getUserStartupWarnings({}, projectDir); - expect(warnings).toContain('Comp warning 1'); + expect(warnings).toContainEqual(compWarning); }); it('should not include compatibility warnings when showCompatibilityWarnings is false', async () => { - vi.mocked(getCompatibilityWarnings).mockReturnValue(['Comp warning 1']); + const compWarning = { + id: 'comp-1', + message: 'Comp warning 1', + priority: WarningPriority.High, + }; + vi.mocked(getCompatibilityWarnings).mockReturnValue([compWarning]); const projectDir = path.join(testRootDir, 'project'); await fs.mkdir(projectDir); @@ -158,7 +172,7 @@ describe('getUserStartupWarnings', () => { { ui: { showCompatibilityWarnings: false } }, projectDir, ); - expect(warnings).not.toContain('Comp warning 1'); + expect(warnings).not.toContainEqual(compWarning); }); }); }); diff --git a/packages/cli/src/utils/userStartupWarnings.ts b/packages/cli/src/utils/userStartupWarnings.ts index cc2d2638d6..da9623acb6 100644 --- a/packages/cli/src/utils/userStartupWarnings.ts +++ b/packages/cli/src/utils/userStartupWarnings.ts @@ -7,7 +7,12 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import process from 'node:process'; -import { homedir, getCompatibilityWarnings } from '@google/gemini-cli-core'; +import { + homedir, + getCompatibilityWarnings, + WarningPriority, + type StartupWarning, +} from '@google/gemini-cli-core'; import type { Settings } from '../config/settingsSchema.js'; import { isFolderTrustEnabled, @@ -17,11 +22,13 @@ import { type WarningCheck = { id: string; check: (workspaceRoot: string, settings: Settings) => Promise; + priority: WarningPriority; }; // Individual warning checks const homeDirectoryCheck: WarningCheck = { id: 'home-directory', + priority: WarningPriority.Low, check: async (workspaceRoot: string, settings: Settings) => { if (settings.ui?.showHomeDirectoryWarning === false) { return null; @@ -53,6 +60,7 @@ const homeDirectoryCheck: WarningCheck = { const rootDirectoryCheck: WarningCheck = { id: 'root-directory', + priority: WarningPriority.High, check: async (workspaceRoot: string, _settings: Settings) => { try { const workspaceRealPath = await fs.realpath(workspaceRoot); @@ -80,11 +88,21 @@ const WARNING_CHECKS: readonly WarningCheck[] = [ export async function getUserStartupWarnings( settings: Settings, workspaceRoot: string = process.cwd(), -): Promise { +): Promise { const results = await Promise.all( - WARNING_CHECKS.map((check) => check.check(workspaceRoot, settings)), + WARNING_CHECKS.map(async (check) => { + const message = await check.check(workspaceRoot, settings); + if (message) { + return { + id: check.id, + message, + priority: check.priority, + }; + } + return null; + }), ); - const warnings = results.filter((msg) => msg !== null); + const warnings = results.filter((w): w is StartupWarning => w !== null); if (settings.ui?.showCompatibilityWarnings !== false) { warnings.push(...getCompatibilityWarnings()); diff --git a/packages/core/src/utils/compatibility.test.ts b/packages/core/src/utils/compatibility.test.ts index 8db512292a..c7819578f1 100644 --- a/packages/core/src/utils/compatibility.test.ts +++ b/packages/core/src/utils/compatibility.test.ts @@ -9,6 +9,7 @@ import os from 'node:os'; import { isWindows10, isJetBrainsTerminal, + supports256Colors, supportsTrueColor, getCompatibilityWarnings, } from './compatibility.js'; @@ -66,6 +67,25 @@ describe('compatibility', () => { }); }); + describe('supports256Colors', () => { + it('should return true when getColorDepth returns >= 8', () => { + process.stdout.getColorDepth = vi.fn().mockReturnValue(8); + expect(supports256Colors()).toBe(true); + }); + + it('should return true when TERM contains 256color', () => { + process.stdout.getColorDepth = vi.fn().mockReturnValue(4); + vi.stubEnv('TERM', 'xterm-256color'); + expect(supports256Colors()).toBe(true); + }); + + it('should return false when 256 colors are not supported', () => { + process.stdout.getColorDepth = vi.fn().mockReturnValue(4); + vi.stubEnv('TERM', 'xterm'); + expect(supports256Colors()).toBe(false); + }); + }); + describe('supportsTrueColor', () => { it('should return true when COLORTERM is truecolor', () => { vi.stubEnv('COLORTERM', 'truecolor'); @@ -103,8 +123,11 @@ describe('compatibility', () => { vi.stubEnv('TERMINAL_EMULATOR', ''); const warnings = getCompatibilityWarnings(); - expect(warnings).toContain( - 'Warning: Windows 10 detected. Some UI features like smooth scrolling may be degraded. Windows 11 is recommended for the best experience.', + expect(warnings).toContainEqual( + expect.objectContaining({ + id: 'windows-10', + message: expect.stringContaining('Windows 10 detected'), + }), ); }); @@ -113,35 +136,78 @@ describe('compatibility', () => { vi.stubEnv('TERMINAL_EMULATOR', 'JetBrains-JediTerm'); const warnings = getCompatibilityWarnings(); - expect(warnings).toContain( - 'Warning: JetBrains terminal detected. You may experience rendering or scrolling issues. Using an external terminal (e.g., Windows Terminal, iTerm2) is recommended.', + expect(warnings).toContainEqual( + expect.objectContaining({ + id: 'jetbrains-terminal', + message: expect.stringContaining('JetBrains terminal detected'), + }), ); }); - it('should return true color warning when not supported', () => { - vi.mocked(os.platform).mockReturnValue('darwin'); + it('should return 256-color warning when 256 colors are not supported', () => { + vi.mocked(os.platform).mockReturnValue('linux'); vi.stubEnv('TERMINAL_EMULATOR', ''); vi.stubEnv('COLORTERM', ''); + vi.stubEnv('TERM', 'xterm'); + process.stdout.getColorDepth = vi.fn().mockReturnValue(4); + + const warnings = getCompatibilityWarnings(); + expect(warnings).toContainEqual( + expect.objectContaining({ + id: '256-color', + message: expect.stringContaining('256-color support not detected'), + priority: 'high', + }), + ); + // Should NOT show true-color warning if 256-color warning is shown + expect(warnings.find((w) => w.id === 'true-color')).toBeUndefined(); + }); + + it('should return true color warning when 256 colors are supported but true color is not, and not Apple Terminal', () => { + vi.mocked(os.platform).mockReturnValue('linux'); + vi.stubEnv('TERMINAL_EMULATOR', ''); + vi.stubEnv('COLORTERM', ''); + vi.stubEnv('TERM_PROGRAM', 'xterm'); process.stdout.getColorDepth = vi.fn().mockReturnValue(8); const warnings = getCompatibilityWarnings(); - expect(warnings).toContain( - 'Warning: True color (24-bit) support not detected. Using a terminal with true color enabled will result in a better visual experience.', + expect(warnings).toContainEqual( + expect.objectContaining({ + id: 'true-color', + message: expect.stringContaining( + 'True color (24-bit) support not detected', + ), + priority: 'low', + }), ); }); + it('should NOT return true color warning for Apple Terminal', () => { + vi.mocked(os.platform).mockReturnValue('darwin'); + vi.stubEnv('TERMINAL_EMULATOR', ''); + vi.stubEnv('COLORTERM', ''); + vi.stubEnv('TERM_PROGRAM', 'Apple_Terminal'); + process.stdout.getColorDepth = vi.fn().mockReturnValue(8); + + const warnings = getCompatibilityWarnings(); + expect(warnings.find((w) => w.id === 'true-color')).toBeUndefined(); + }); + it('should return all warnings when all are detected', () => { vi.mocked(os.platform).mockReturnValue('win32'); vi.mocked(os.release).mockReturnValue('10.0.19041'); vi.stubEnv('TERMINAL_EMULATOR', 'JetBrains-JediTerm'); vi.stubEnv('COLORTERM', ''); + vi.stubEnv('TERM_PROGRAM', 'xterm'); process.stdout.getColorDepth = vi.fn().mockReturnValue(8); const warnings = getCompatibilityWarnings(); expect(warnings).toHaveLength(3); - expect(warnings[0]).toContain('Windows 10 detected'); - expect(warnings[1]).toContain('JetBrains terminal detected'); - expect(warnings[2]).toContain('True color (24-bit) support not detected'); + expect(warnings[0].message).toContain('Windows 10 detected'); + expect(warnings[1].message).toContain('JetBrains terminal detected'); + expect(warnings[2].message).toContain( + 'True color (24-bit) support not detected', + ); }); it('should return no warnings in a standard environment with true color', () => { diff --git a/packages/core/src/utils/compatibility.ts b/packages/core/src/utils/compatibility.ts index b1b6240a65..8099351ad0 100644 --- a/packages/core/src/utils/compatibility.ts +++ b/packages/core/src/utils/compatibility.ts @@ -30,6 +30,31 @@ export function isJetBrainsTerminal(): boolean { return process.env['TERMINAL_EMULATOR'] === 'JetBrains-JediTerm'; } +/** + * Detects if the current terminal is the default Apple Terminal.app. + */ +export function isAppleTerminal(): boolean { + return process.env['TERM_PROGRAM'] === 'Apple_Terminal'; +} + +/** + * Detects if the current terminal supports 256 colors (8-bit). + */ +export function supports256Colors(): boolean { + // Check if stdout supports at least 8-bit color depth + if (process.stdout.getColorDepth && process.stdout.getColorDepth() >= 8) { + return true; + } + + // Check TERM environment variable + const term = process.env['TERM'] || ''; + if (term.includes('256color')) { + return true; + } + + return false; +} + /** * Detects if the current terminal supports true color (24-bit). */ @@ -53,25 +78,52 @@ export function supportsTrueColor(): boolean { /** * Returns a list of compatibility warnings based on the current environment. */ -export function getCompatibilityWarnings(): string[] { - const warnings: string[] = []; +export enum WarningPriority { + Low = 'low', + High = 'high', +} + +export interface StartupWarning { + id: string; + message: string; + priority: WarningPriority; +} + +export function getCompatibilityWarnings(): StartupWarning[] { + const warnings: StartupWarning[] = []; if (isWindows10()) { - warnings.push( - 'Warning: Windows 10 detected. Some UI features like smooth scrolling may be degraded. Windows 11 is recommended for the best experience.', - ); + warnings.push({ + id: 'windows-10', + message: + 'Warning: Windows 10 detected. Some UI features like smooth scrolling may be degraded. Windows 11 is recommended for the best experience.', + priority: WarningPriority.High, + }); } if (isJetBrainsTerminal()) { - warnings.push( - 'Warning: JetBrains terminal detected. You may experience rendering or scrolling issues. Using an external terminal (e.g., Windows Terminal, iTerm2) is recommended.', - ); + warnings.push({ + id: 'jetbrains-terminal', + message: + 'Warning: JetBrains terminal detected. You may experience rendering or scrolling issues. Using an external terminal (e.g., Windows Terminal, iTerm2) is recommended.', + priority: WarningPriority.High, + }); } - if (!supportsTrueColor()) { - warnings.push( - 'Warning: True color (24-bit) support not detected. Using a terminal with true color enabled will result in a better visual experience.', - ); + if (!supports256Colors()) { + warnings.push({ + id: '256-color', + message: + 'Warning: 256-color support not detected. Using a terminal with at least 256-color support is recommended for a better visual experience.', + priority: WarningPriority.High, + }); + } else if (!supportsTrueColor() && !isAppleTerminal()) { + warnings.push({ + id: 'true-color', + message: + 'Warning: True color (24-bit) support not detected. Using a terminal with true color enabled will result in a better visual experience.', + priority: WarningPriority.Low, + }); } return warnings;