diff --git a/packages/cli/index.ts b/packages/cli/index.ts index 7df29cd0..5478f034 100644 --- a/packages/cli/index.ts +++ b/packages/cli/index.ts @@ -7,3 +7,15 @@ */ import './src/gemini.js'; +import { main } from './src/gemini.js'; + +// --- Global Entry Point --- +main().catch((error) => { + console.error('An unexpected critical error occurred:'); + if (error instanceof Error) { + console.error(error.message); + } else { + console.error(String(error)); + } + process.exit(1); +}); diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index 6c075b38..cfa5e2b5 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -96,6 +96,7 @@ describe('Settings Loading and Merging', () => { expect(settings.user.settings).toEqual({}); expect(settings.workspace.settings).toEqual({}); expect(settings.merged).toEqual({}); + expect(settings.errors.length).toBe(0); }); it('should load user settings if only user file exists', () => { @@ -300,27 +301,61 @@ describe('Settings Loading and Merging', () => { }); it('should handle JSON parsing errors gracefully', () => { - (mockFsExistsSync as Mock).mockReturnValue(true); + (mockFsExistsSync as Mock).mockReturnValue(true); // Both files "exist" + const invalidJsonContent = 'invalid json'; + const userReadError = new SyntaxError( + "Expected ',' or '}' after property value in JSON at position 10", + ); + const workspaceReadError = new SyntaxError( + 'Unexpected token i in JSON at position 0', + ); + (fs.readFileSync as Mock).mockImplementation( (p: fs.PathOrFileDescriptor) => { - // Make it return invalid json for the paths it will try to read - if (p === USER_SETTINGS_PATH || p === MOCK_WORKSPACE_SETTINGS_PATH) - return 'invalid json'; - return ''; + if (p === USER_SETTINGS_PATH) { + // Simulate JSON.parse throwing for user settings + vi.spyOn(JSON, 'parse').mockImplementationOnce(() => { + throw userReadError; + }); + return invalidJsonContent; // Content that would cause JSON.parse to throw + } + if (p === MOCK_WORKSPACE_SETTINGS_PATH) { + // Simulate JSON.parse throwing for workspace settings + vi.spyOn(JSON, 'parse').mockImplementationOnce(() => { + throw workspaceReadError; + }); + return invalidJsonContent; + } + return '{}'; // Default for other reads }, ); - const consoleErrorSpy = vi - .spyOn(console, 'error') - .mockImplementation(() => {}); const settings = loadSettings(MOCK_WORKSPACE_DIR); + // Check that settings are empty due to parsing errors expect(settings.user.settings).toEqual({}); expect(settings.workspace.settings).toEqual({}); expect(settings.merged).toEqual({}); - expect(consoleErrorSpy).toHaveBeenCalledTimes(2); - consoleErrorSpy.mockRestore(); + // Check that error objects are populated in settings.errors + expect(settings.errors).toBeDefined(); + // Assuming both user and workspace files cause errors and are added in order + expect(settings.errors.length).toEqual(2); + + const userError = settings.errors.find( + (e) => e.path === USER_SETTINGS_PATH, + ); + expect(userError).toBeDefined(); + expect(userError?.message).toBe(userReadError.message); + + const workspaceError = settings.errors.find( + (e) => e.path === MOCK_WORKSPACE_SETTINGS_PATH, + ); + expect(workspaceError).toBeDefined(); + expect(workspaceError?.message).toBe(workspaceReadError.message); + + // Restore JSON.parse mock if it was spied on specifically for this test + vi.restoreAllMocks(); // Or more targeted restore if needed }); it('should resolve environment variables in user settings', () => { @@ -585,13 +620,14 @@ describe('Settings Loading and Merging', () => { 'MY_AGENTS.md', ); expect(loadedSettings.merged.contextFileName).toBe('MY_AGENTS.md'); - expect(loadedSettings.merged.theme).toBe('matrix'); + expect(loadedSettings.merged.theme).toBe('matrix'); // User setting should still be there expect(fs.writeFileSync).toHaveBeenCalledWith( MOCK_WORKSPACE_SETTINGS_PATH, JSON.stringify({ contextFileName: 'MY_AGENTS.md' }, null, 2), 'utf-8', ); + // Workspace theme overrides user theme loadedSettings.setValue(SettingScope.Workspace, 'theme', 'ocean'); expect(loadedSettings.workspace.settings.theme).toBe('ocean'); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 427fb898..d3a4dacb 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -7,7 +7,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { homedir } from 'os'; -import { MCPServerConfig } from '@gemini-code/core'; +import { MCPServerConfig, getErrorMessage } from '@gemini-code/core'; import stripJsonComments from 'strip-json-comments'; import { DefaultLight } from '../ui/themes/default-light.js'; import { DefaultDark } from '../ui/themes/default.js'; @@ -47,19 +47,30 @@ export interface Settings { // Add other settings here. } +export interface SettingsError { + message: string; + path: string; +} + export interface SettingsFile { settings: Settings; path: string; } export class LoadedSettings { - constructor(user: SettingsFile, workspace: SettingsFile) { + constructor( + user: SettingsFile, + workspace: SettingsFile, + errors: SettingsError[], + ) { this.user = user; this.workspace = workspace; + this.errors = errors; this._merged = this.computeMergedSettings(); } readonly user: SettingsFile; readonly workspace: SettingsFile; + readonly errors: SettingsError[]; private _merged: Settings; @@ -147,6 +158,7 @@ function resolveEnvVarsInObject(obj: T): T { export function loadSettings(workspaceDir: string): LoadedSettings { let userSettings: Settings = {}; let workspaceSettings: Settings = {}; + const settingsErrors: SettingsError[] = []; // Load user settings try { @@ -163,8 +175,11 @@ export function loadSettings(workspaceDir: string): LoadedSettings { userSettings.theme = DefaultDark.name; } } - } catch (error) { - console.error('Error reading user settings file:', error); + } catch (error: unknown) { + settingsErrors.push({ + message: getErrorMessage(error), + path: USER_SETTINGS_PATH, + }); } const workspaceSettingsPath = path.join( @@ -190,13 +205,23 @@ export function loadSettings(workspaceDir: string): LoadedSettings { workspaceSettings.theme = DefaultDark.name; } } - } catch (error) { - console.error('Error reading workspace settings file:', error); + } catch (error: unknown) { + settingsErrors.push({ + message: getErrorMessage(error), + path: workspaceSettingsPath, + }); } return new LoadedSettings( - { path: USER_SETTINGS_PATH, settings: userSettings }, - { path: workspaceSettingsPath, settings: workspaceSettings }, + { + path: USER_SETTINGS_PATH, + settings: userSettings, + }, + { + path: workspaceSettingsPath, + settings: workspaceSettings, + }, + settingsErrors, ); } diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx new file mode 100644 index 00000000..f77f9577 --- /dev/null +++ b/packages/cli/src/gemini.test.tsx @@ -0,0 +1,140 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import stripAnsi from 'strip-ansi'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { main } from './gemini.js'; +import { + LoadedSettings, + SettingsFile, + loadSettings, +} from './config/settings.js'; + +// Custom error to identify mock process.exit calls +class MockProcessExitError extends Error { + constructor(readonly code?: string | number | null | undefined) { + super('PROCESS_EXIT_MOCKED'); + this.name = 'MockProcessExitError'; + } +} + +// Mock dependencies +vi.mock('./config/settings.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + loadSettings: vi.fn(), + }; +}); + +vi.mock('./config/config.js', () => ({ + loadCliConfig: vi.fn().mockResolvedValue({ + config: { + getSandbox: vi.fn(() => false), + getQuestion: vi.fn(() => ''), + }, + modelWasSwitched: false, + originalModelBeforeSwitch: null, + finalModel: 'test-model', + }), +})); + +vi.mock('read-package-up', () => ({ + readPackageUp: vi.fn().mockResolvedValue({ + packageJson: { version: 'test-version' }, + path: '/fake/path/package.json', + }), +})); + +vi.mock('./utils/sandbox.js', () => ({ + sandbox_command: vi.fn(() => ''), // Default to no sandbox command + start_sandbox: vi.fn(() => Promise.resolve()), // Mock as an async function that resolves +})); + +describe('gemini.tsx main function', () => { + let consoleErrorSpy: ReturnType; + let loadSettingsMock: ReturnType>; + let originalEnvGeminiSandbox: string | undefined; + let originalEnvSandbox: string | undefined; + + const processExitSpy = vi + .spyOn(process, 'exit') + .mockImplementation((code) => { + throw new MockProcessExitError(code); + }); + + beforeEach(() => { + loadSettingsMock = vi.mocked(loadSettings); + + // Store and clear sandbox-related env variables to ensure a consistent test environment + originalEnvGeminiSandbox = process.env.GEMINI_SANDBOX; + originalEnvSandbox = process.env.SANDBOX; + delete process.env.GEMINI_SANDBOX; + delete process.env.SANDBOX; + + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + // Restore original env variables + if (originalEnvGeminiSandbox !== undefined) { + process.env.GEMINI_SANDBOX = originalEnvGeminiSandbox; + } else { + delete process.env.GEMINI_SANDBOX; + } + if (originalEnvSandbox !== undefined) { + process.env.SANDBOX = originalEnvSandbox; + } else { + delete process.env.SANDBOX; + } + vi.restoreAllMocks(); + }); + + it('should call process.exit(1) if settings have errors', async () => { + const settingsError = { + message: 'Test settings error', + path: '/test/settings.json', + }; + const userSettingsFile: SettingsFile = { + path: '/user/settings.json', + settings: {}, + }; + const workspaceSettingsFile: SettingsFile = { + path: '/workspace/.gemini/settings.json', + settings: {}, + }; + const mockLoadedSettings = new LoadedSettings( + userSettingsFile, + workspaceSettingsFile, + [settingsError], + ); + + loadSettingsMock.mockReturnValue(mockLoadedSettings); + + try { + await main(); + // If main completes without throwing, the test should fail because process.exit was expected + expect.fail('main function did not exit as expected'); + } catch (error) { + expect(error).toBeInstanceOf(MockProcessExitError); + if (error instanceof MockProcessExitError) { + expect(error.code).toBe(1); + } + } + + // Verify console.error was called with the error message + expect(consoleErrorSpy).toHaveBeenCalledTimes(2); + expect(stripAnsi(String(consoleErrorSpy.mock.calls[0][0]))).toBe( + 'Error in /test/settings.json: Test settings error', + ); + expect(stripAnsi(String(consoleErrorSpy.mock.calls[1][0]))).toBe( + 'Please fix /test/settings.json and try again.', + ); + + // Verify process.exit was called (indirectly, via the thrown error) + expect(processExitSpy).toHaveBeenCalledWith(1); + }); +}); diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index f9b73074..e2169980 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -37,7 +37,7 @@ import { const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); -async function main() { +export async function main() { // warn about deprecated environment variables if (process.env.GEMINI_CODE_MODEL) { console.warn('GEMINI_CODE_MODEL is deprecated. Use GEMINI_MODEL instead.'); @@ -61,6 +61,18 @@ async function main() { const settings = loadSettings(workspaceRoot); const geminiIgnorePatterns = loadGeminiIgnorePatterns(workspaceRoot); + if (settings.errors.length > 0) { + for (const error of settings.errors) { + let errorMessage = `Error in ${error.path}: ${error.message}`; + if (!process.env.NO_COLOR) { + errorMessage = `\x1b[31m${errorMessage}\x1b[0m`; + } + console.error(errorMessage); + console.error(`Please fix ${error.path} and try again.`); + } + process.exit(1); + } + const { config, modelWasSwitched, originalModelBeforeSwitch, finalModel } = await loadCliConfig(settings.merged, geminiIgnorePatterns); @@ -144,17 +156,6 @@ process.on('unhandledRejection', (reason, _promise) => { process.exit(1); }); -// --- Global Entry Point --- -main().catch((error) => { - console.error('An unexpected critical error occurred:'); - if (error instanceof Error) { - console.error(error.message); - } else { - console.error(String(error)); - } - process.exit(1); -}); - async function loadNonInteractiveConfig( config: Config, settings: LoadedSettings, diff --git a/packages/cli/src/ui/App.test.tsx b/packages/cli/src/ui/App.test.tsx index c1430bb5..90adb24f 100644 --- a/packages/cli/src/ui/App.test.tsx +++ b/packages/cli/src/ui/App.test.tsx @@ -171,7 +171,7 @@ describe('App UI', () => { path: '/workspace/.gemini/settings.json', settings, }; - return new LoadedSettings(userSettingsFile, workspaceSettingsFile); + return new LoadedSettings(userSettingsFile, workspaceSettingsFile, []); }; beforeEach(() => {