diff --git a/packages/cli/src/config/auth.test.ts b/packages/cli/src/config/auth.test.ts index 7bbaafdfcb..b0492527b8 100644 --- a/packages/cli/src/config/auth.test.ts +++ b/packages/cli/src/config/auth.test.ts @@ -5,7 +5,7 @@ */ import { AuthType } from '@google/gemini-cli-core'; -import { vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { validateAuthMethod } from './auth.js'; vi.mock('./settings.js', () => ({ @@ -17,7 +17,6 @@ vi.mock('./settings.js', () => ({ describe('validateAuthMethod', () => { beforeEach(() => { - vi.resetModules(); vi.stubEnv('GEMINI_API_KEY', undefined); vi.stubEnv('GOOGLE_CLOUD_PROJECT', undefined); vi.stubEnv('GOOGLE_CLOUD_LOCATION', undefined); @@ -28,53 +27,73 @@ describe('validateAuthMethod', () => { vi.unstubAllEnvs(); }); - it('should return null for LOGIN_WITH_GOOGLE', () => { - expect(validateAuthMethod(AuthType.LOGIN_WITH_GOOGLE)).toBeNull(); - }); - - it('should return null for COMPUTE_ADC', () => { - expect(validateAuthMethod(AuthType.COMPUTE_ADC)).toBeNull(); - }); - - describe('USE_GEMINI', () => { - it('should return null if GEMINI_API_KEY is set', () => { - vi.stubEnv('GEMINI_API_KEY', 'test-key'); - expect(validateAuthMethod(AuthType.USE_GEMINI)).toBeNull(); - }); - - it('should return an error message if GEMINI_API_KEY is not set', () => { - vi.stubEnv('GEMINI_API_KEY', undefined); - expect(validateAuthMethod(AuthType.USE_GEMINI)).toBeNull(); - }); - }); - - describe('USE_VERTEX_AI', () => { - it('should return null if GOOGLE_CLOUD_PROJECT and GOOGLE_CLOUD_LOCATION are set', () => { - vi.stubEnv('GOOGLE_CLOUD_PROJECT', 'test-project'); - vi.stubEnv('GOOGLE_CLOUD_LOCATION', 'test-location'); - expect(validateAuthMethod(AuthType.USE_VERTEX_AI)).toBeNull(); - }); - - it('should return null if GOOGLE_API_KEY is set', () => { - vi.stubEnv('GOOGLE_API_KEY', 'test-api-key'); - expect(validateAuthMethod(AuthType.USE_VERTEX_AI)).toBeNull(); - }); - - it('should return an error message if no required environment variables are set', () => { - vi.stubEnv('GOOGLE_CLOUD_PROJECT', undefined); - vi.stubEnv('GOOGLE_CLOUD_LOCATION', undefined); - expect(validateAuthMethod(AuthType.USE_VERTEX_AI)).toBe( + it.each([ + { + description: 'should return null for LOGIN_WITH_GOOGLE', + authType: AuthType.LOGIN_WITH_GOOGLE, + envs: {}, + expected: null, + }, + { + description: 'should return null for COMPUTE_ADC', + authType: AuthType.COMPUTE_ADC, + envs: {}, + expected: null, + }, + { + description: 'should return null for USE_GEMINI if GEMINI_API_KEY is set', + authType: AuthType.USE_GEMINI, + envs: { GEMINI_API_KEY: 'test-key' }, + expected: null, + }, + { + description: + 'should return an error message for USE_GEMINI if GEMINI_API_KEY is not set', + authType: AuthType.USE_GEMINI, + envs: {}, + expected: + 'When using Gemini API, you must specify the GEMINI_API_KEY environment variable.\n' + + 'Update your environment and try again (no reload needed if using .env)!', + }, + { + description: + 'should return null for USE_VERTEX_AI if GOOGLE_CLOUD_PROJECT and GOOGLE_CLOUD_LOCATION are set', + authType: AuthType.USE_VERTEX_AI, + envs: { + GOOGLE_CLOUD_PROJECT: 'test-project', + GOOGLE_CLOUD_LOCATION: 'test-location', + }, + expected: null, + }, + { + description: + 'should return null for USE_VERTEX_AI if GOOGLE_API_KEY is set', + authType: AuthType.USE_VERTEX_AI, + envs: { GOOGLE_API_KEY: 'test-api-key' }, + expected: null, + }, + { + description: + 'should return an error message for USE_VERTEX_AI if no required environment variables are set', + authType: AuthType.USE_VERTEX_AI, + envs: {}, + expected: 'When using Vertex AI, you must specify either:\n' + - '• GOOGLE_CLOUD_PROJECT and GOOGLE_CLOUD_LOCATION environment variables.\n' + - '• GOOGLE_API_KEY environment variable (if using express mode).\n' + - 'Update your environment and try again (no reload needed if using .env)!', - ); - }); - }); - - it('should return an error message for an invalid auth method', () => { - expect(validateAuthMethod('invalid-method')).toBe( - 'Invalid auth method selected.', - ); + '• GOOGLE_CLOUD_PROJECT and GOOGLE_CLOUD_LOCATION environment variables.\n' + + '• GOOGLE_API_KEY environment variable (if using express mode).\n' + + 'Update your environment and try again (no reload needed if using .env)!', + }, + { + description: 'should return an error message for an invalid auth method', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + authType: 'invalid-method' as any, + envs: {}, + expected: 'Invalid auth method selected.', + }, + ])('$description', ({ authType, envs, expected }) => { + for (const [key, value] of Object.entries(envs)) { + vi.stubEnv(key, value as string); + } + expect(validateAuthMethod(authType)).toBe(expected); }); }); diff --git a/packages/cli/src/config/auth.ts b/packages/cli/src/config/auth.ts index 87940307e1..a3cfea7d77 100644 --- a/packages/cli/src/config/auth.ts +++ b/packages/cli/src/config/auth.ts @@ -17,6 +17,12 @@ export function validateAuthMethod(authMethod: string): string | null { } if (authMethod === AuthType.USE_GEMINI) { + if (!process.env['GEMINI_API_KEY']) { + return ( + 'When using Gemini API, you must specify the GEMINI_API_KEY environment variable.\n' + + 'Update your environment and try again (no reload needed if using .env)!' + ); + } return null; } diff --git a/packages/cli/src/config/config.integration.test.ts b/packages/cli/src/config/config.integration.test.ts index 24010033f4..49afb1ae5b 100644 --- a/packages/cli/src/config/config.integration.test.ts +++ b/packages/cli/src/config/config.integration.test.ts @@ -4,7 +4,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from 'vitest'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { tmpdir } from 'node:os'; @@ -64,106 +73,33 @@ describe('Configuration Integration Tests', () => { } }); - describe('File Filtering Configuration', () => { - it('should load default file filtering settings', async () => { - const configParams: ConfigParameters = { - sessionId: 'test-session', - cwd: '/tmp', - model: 'test-model', - embeddingModel: 'test-embedding-model', - sandbox: undefined, - targetDir: tempDir, - debugMode: false, - }; - - const config = new Config(configParams); - - expect(config.getFileFilteringRespectGitIgnore()).toBe( - DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore, - ); - }); - - it('should load custom file filtering settings from configuration', async () => { - const configParams: ConfigParameters = { - sessionId: 'test-session', - cwd: '/tmp', - model: 'test-model', - embeddingModel: 'test-embedding-model', - sandbox: undefined, - targetDir: tempDir, - debugMode: false, - fileFiltering: { - respectGitIgnore: false, - }, - }; - - const config = new Config(configParams); - - expect(config.getFileFilteringRespectGitIgnore()).toBe(false); - }); - - it('should merge user and workspace file filtering settings', async () => { - const configParams: ConfigParameters = { - sessionId: 'test-session', - cwd: '/tmp', - model: 'test-model', - embeddingModel: 'test-embedding-model', - sandbox: undefined, - targetDir: tempDir, - debugMode: false, - fileFiltering: { - respectGitIgnore: true, - }, - }; - - const config = new Config(configParams); - - expect(config.getFileFilteringRespectGitIgnore()).toBe(true); - }); - }); - - describe('Configuration Integration', () => { - it('should handle partial configuration objects gracefully', async () => { - const configParams: ConfigParameters = { - sessionId: 'test-session', - cwd: '/tmp', - model: 'test-model', - embeddingModel: 'test-embedding-model', - sandbox: undefined, - targetDir: tempDir, - debugMode: false, - fileFiltering: { - respectGitIgnore: false, - }, - }; - - const config = new Config(configParams); - - // Specified settings should be applied - expect(config.getFileFilteringRespectGitIgnore()).toBe(false); - }); - - it('should handle empty configuration objects gracefully', async () => { - const configParams: ConfigParameters = { - sessionId: 'test-session', - cwd: '/tmp', - model: 'test-model', - embeddingModel: 'test-embedding-model', - sandbox: undefined, - targetDir: tempDir, - debugMode: false, + describe('File Filtering and Configuration', () => { + it.each([ + { + description: + 'should load default file filtering settings when fileFiltering is missing', + fileFiltering: undefined, + expected: DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore, + }, + { + description: + 'should load custom file filtering settings from configuration', + fileFiltering: { respectGitIgnore: false }, + expected: false, + }, + { + description: + 'should respect file filtering settings from configuration', + fileFiltering: { respectGitIgnore: true }, + expected: true, + }, + { + description: + 'should handle empty fileFiltering object gracefully and use defaults', fileFiltering: {}, - }; - - const config = new Config(configParams); - - // All settings should use defaults - expect(config.getFileFilteringRespectGitIgnore()).toBe( - DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore, - ); - }); - - it('should handle missing configuration sections gracefully', async () => { + expected: DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore, + }, + ])('$description', async ({ fileFiltering, expected }) => { const configParams: ConfigParameters = { sessionId: 'test-session', cwd: '/tmp', @@ -172,20 +108,26 @@ describe('Configuration Integration Tests', () => { sandbox: undefined, targetDir: tempDir, debugMode: false, - // Missing fileFiltering configuration + fileFiltering, }; const config = new Config(configParams); - // All git-aware settings should use defaults - expect(config.getFileFilteringRespectGitIgnore()).toBe( - DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore, - ); + expect(config.getFileFilteringRespectGitIgnore()).toBe(expected); }); }); describe('Real-world Configuration Scenarios', () => { - it('should handle a security-focused configuration', async () => { + it.each([ + { + description: 'should handle a security-focused configuration', + respectGitIgnore: true, + }, + { + description: 'should handle a CI/CD environment configuration', + respectGitIgnore: false, + }, + ])('$description', async ({ respectGitIgnore }) => { const configParams: ConfigParameters = { sessionId: 'test-session', cwd: '/tmp', @@ -195,32 +137,13 @@ describe('Configuration Integration Tests', () => { targetDir: tempDir, debugMode: false, fileFiltering: { - respectGitIgnore: true, + respectGitIgnore, }, }; const config = new Config(configParams); - expect(config.getFileFilteringRespectGitIgnore()).toBe(true); - }); - - it('should handle a CI/CD environment configuration', async () => { - const configParams: ConfigParameters = { - sessionId: 'test-session', - cwd: '/tmp', - model: 'test-model', - embeddingModel: 'test-embedding-model', - sandbox: undefined, - targetDir: tempDir, - debugMode: false, - fileFiltering: { - respectGitIgnore: false, - }, // CI might need to see all files - }; - - const config = new Config(configParams); - - expect(config.getFileFilteringRespectGitIgnore()).toBe(false); + expect(config.getFileFilteringRespectGitIgnore()).toBe(respectGitIgnore); }); }); @@ -252,139 +175,70 @@ describe('Configuration Integration Tests', () => { parseArguments = parseArgs; }); - it('should parse --approval-mode=auto_edit correctly through the full argument parsing flow', async () => { - const originalArgv = process.argv; - - try { - process.argv = [ + it.each([ + { + description: 'should parse --approval-mode=auto_edit correctly', + argv: [ 'node', 'script.js', '--approval-mode', 'auto_edit', '-p', 'test', - ]; - - const argv = await parseArguments({} as Settings); - - // Verify that the argument was parsed correctly - expect(argv.approvalMode).toBe('auto_edit'); - expect(argv.prompt).toBe('test'); - expect(argv.yolo).toBe(false); + ], + expected: { approvalMode: 'auto_edit', prompt: 'test', yolo: false }, + }, + { + description: 'should parse --approval-mode=yolo correctly', + argv: ['node', 'script.js', '--approval-mode', 'yolo', '-p', 'test'], + expected: { approvalMode: 'yolo', prompt: 'test', yolo: false }, + }, + { + description: 'should parse --approval-mode=default correctly', + argv: ['node', 'script.js', '--approval-mode', 'default', '-p', 'test'], + expected: { approvalMode: 'default', prompt: 'test', yolo: false }, + }, + { + description: 'should parse legacy --yolo flag correctly', + argv: ['node', 'script.js', '--yolo', '-p', 'test'], + expected: { yolo: true, approvalMode: undefined, prompt: 'test' }, + }, + { + description: 'should handle no approval mode arguments', + argv: ['node', 'script.js', '-p', 'test'], + expected: { approvalMode: undefined, yolo: false, prompt: 'test' }, + }, + ])('$description', async ({ argv, expected }) => { + const originalArgv = process.argv; + try { + process.argv = argv; + const parsedArgs = await parseArguments({} as Settings); + expect(parsedArgs.approvalMode).toBe(expected.approvalMode); + expect(parsedArgs.prompt).toBe(expected.prompt); + expect(parsedArgs.yolo).toBe(expected.yolo); } finally { process.argv = originalArgv; } }); - it('should parse --approval-mode=yolo correctly through the full argument parsing flow', async () => { + it.each([ + { + description: 'should reject invalid approval mode values', + argv: ['node', 'script.js', '--approval-mode', 'invalid_mode'], + }, + { + description: + 'should reject conflicting --yolo and --approval-mode flags', + argv: ['node', 'script.js', '--yolo', '--approval-mode', 'default'], + }, + ])('$description', async ({ argv }) => { const originalArgv = process.argv; - try { - process.argv = [ - 'node', - 'script.js', - '--approval-mode', - 'yolo', - '-p', - 'test', - ]; - - const argv = await parseArguments({} as Settings); - - expect(argv.approvalMode).toBe('yolo'); - expect(argv.prompt).toBe('test'); - expect(argv.yolo).toBe(false); // Should NOT be set when using --approval-mode - } finally { - process.argv = originalArgv; - } - }); - - it('should parse --approval-mode=default correctly through the full argument parsing flow', async () => { - const originalArgv = process.argv; - - try { - process.argv = [ - 'node', - 'script.js', - '--approval-mode', - 'default', - '-p', - 'test', - ]; - - const argv = await parseArguments({} as Settings); - - expect(argv.approvalMode).toBe('default'); - expect(argv.prompt).toBe('test'); - expect(argv.yolo).toBe(false); - } finally { - process.argv = originalArgv; - } - }); - - it('should parse legacy --yolo flag correctly', async () => { - const originalArgv = process.argv; - - try { - process.argv = ['node', 'script.js', '--yolo', '-p', 'test']; - - const argv = await parseArguments({} as Settings); - - expect(argv.yolo).toBe(true); - expect(argv.approvalMode).toBeUndefined(); // Should NOT be set when using --yolo - expect(argv.prompt).toBe('test'); - } finally { - process.argv = originalArgv; - } - }); - - it('should reject invalid approval mode values during argument parsing', async () => { - const originalArgv = process.argv; - - try { - process.argv = ['node', 'script.js', '--approval-mode', 'invalid_mode']; - - // Should throw during argument parsing due to yargs validation + process.argv = argv; await expect(parseArguments({} as Settings)).rejects.toThrow(); } finally { process.argv = originalArgv; } }); - - it('should reject conflicting --yolo and --approval-mode flags', async () => { - const originalArgv = process.argv; - - try { - process.argv = [ - 'node', - 'script.js', - '--yolo', - '--approval-mode', - 'default', - ]; - - // Should throw during argument parsing due to conflict validation - await expect(parseArguments({} as Settings)).rejects.toThrow(); - } finally { - process.argv = originalArgv; - } - }); - - it('should handle backward compatibility with mixed scenarios', async () => { - const originalArgv = process.argv; - - try { - // Test that no approval mode arguments defaults to no flags set - process.argv = ['node', 'script.js', '-p', 'test']; - - const argv = await parseArguments({} as Settings); - - expect(argv.approvalMode).toBeUndefined(); - expect(argv.yolo).toBe(false); - expect(argv.prompt).toBe('test'); - } finally { - process.argv = originalArgv; - } - }); }); }); diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 8adafaf78b..020592c903 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -149,307 +149,264 @@ afterEach(() => { }); describe('parseArguments', () => { - it('should throw an error when both --prompt and --prompt-interactive are used together', async () => { - process.argv = [ - 'node', - 'script.js', - '--prompt', - 'test prompt', - '--prompt-interactive', - 'interactive prompt', - ]; + it.each([ + { + description: 'long flags', + argv: [ + 'node', + 'script.js', + '--prompt', + 'test prompt', + '--prompt-interactive', + 'interactive prompt', + ], + }, + { + description: 'short flags', + argv: [ + 'node', + 'script.js', + '-p', + 'test prompt', + '-i', + 'interactive prompt', + ], + }, + ])( + 'should throw an error when using conflicting prompt flags ($description)', + async ({ argv }) => { + process.argv = argv; - const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => { - throw new Error('process.exit called'); - }); + const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit called'); + }); - const mockConsoleError = vi - .spyOn(console, 'error') - .mockImplementation(() => {}); - const debugErrorSpy = vi - .spyOn(debugLogger, 'error') - .mockImplementation(() => {}); + const mockConsoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); - await expect(parseArguments({} as Settings)).rejects.toThrow( - 'process.exit called', + await expect(parseArguments({} as Settings)).rejects.toThrow( + 'process.exit called', + ); + + expect(mockConsoleError).toHaveBeenCalledWith( + expect.stringContaining( + 'Cannot use both --prompt (-p) and --prompt-interactive (-i) together', + ), + ); + + mockExit.mockRestore(); + mockConsoleError.mockRestore(); + }, + ); + + it.each([ + { + description: 'should allow --prompt without --prompt-interactive', + argv: ['node', 'script.js', '--prompt', 'test prompt'], + expected: { prompt: 'test prompt', promptInteractive: undefined }, + }, + { + description: 'should allow --prompt-interactive without --prompt', + argv: ['node', 'script.js', '--prompt-interactive', 'interactive prompt'], + expected: { prompt: undefined, promptInteractive: 'interactive prompt' }, + }, + { + description: 'should allow -i flag as alias for --prompt-interactive', + argv: ['node', 'script.js', '-i', 'interactive prompt'], + expected: { prompt: undefined, promptInteractive: 'interactive prompt' }, + }, + ])('$description', async ({ argv, expected }) => { + process.argv = argv; + const parsedArgs = await parseArguments({} as Settings); + expect(parsedArgs.prompt).toBe(expected.prompt); + expect(parsedArgs.promptInteractive).toBe(expected.promptInteractive); + }); + + describe('positional arguments and @commands', () => { + it.each([ + { + description: + 'should convert positional query argument to prompt by default', + argv: ['node', 'script.js', 'Hi Gemini'], + expectedQuery: 'Hi Gemini', + expectedModel: undefined, + debug: false, + }, + { + description: + 'should map @path to prompt (one-shot) when it starts with @', + argv: ['node', 'script.js', '@path ./file.md'], + expectedQuery: '@path ./file.md', + expectedModel: undefined, + debug: false, + }, + { + description: + 'should map @path to prompt even when config flags are present', + argv: [ + 'node', + 'script.js', + '@path', + './file.md', + '--model', + 'gemini-2.5-pro', + ], + expectedQuery: '@path ./file.md', + expectedModel: 'gemini-2.5-pro', + debug: false, + }, + { + description: + 'maps unquoted positional @path + arg to prompt (one-shot)', + argv: ['node', 'script.js', '@path', './file.md'], + expectedQuery: '@path ./file.md', + expectedModel: undefined, + debug: false, + }, + { + description: + 'should handle multiple @path arguments in a single command (one-shot)', + argv: [ + 'node', + 'script.js', + '@path', + './file1.md', + '@path', + './file2.md', + ], + expectedQuery: '@path ./file1.md @path ./file2.md', + expectedModel: undefined, + debug: false, + }, + { + description: + 'should handle mixed quoted and unquoted @path arguments (one-shot)', + argv: [ + 'node', + 'script.js', + '@path ./file1.md', + '@path', + './file2.md', + 'additional text', + ], + expectedQuery: '@path ./file1.md @path ./file2.md additional text', + expectedModel: undefined, + debug: false, + }, + { + description: 'should map @path to prompt with ambient flags (debug)', + argv: ['node', 'script.js', '@path', './file.md', '--debug'], + expectedQuery: '@path ./file.md', + expectedModel: undefined, + debug: true, + }, + { + description: 'should map @include to prompt (one-shot)', + argv: ['node', 'script.js', '@include src/'], + expectedQuery: '@include src/', + expectedModel: undefined, + debug: false, + }, + { + description: 'should map @search to prompt (one-shot)', + argv: ['node', 'script.js', '@search pattern'], + expectedQuery: '@search pattern', + expectedModel: undefined, + debug: false, + }, + { + description: 'should map @web to prompt (one-shot)', + argv: ['node', 'script.js', '@web query'], + expectedQuery: '@web query', + expectedModel: undefined, + debug: false, + }, + { + description: 'should map @git to prompt (one-shot)', + argv: ['node', 'script.js', '@git status'], + expectedQuery: '@git status', + expectedModel: undefined, + debug: false, + }, + { + description: 'should handle @command with leading whitespace', + argv: ['node', 'script.js', ' @path ./file.md'], + expectedQuery: ' @path ./file.md', + expectedModel: undefined, + debug: false, + }, + ])( + '$description', + async ({ argv, expectedQuery, expectedModel, debug }) => { + process.argv = argv; + const parsedArgs = await parseArguments({} as Settings); + expect(parsedArgs.query).toBe(expectedQuery); + expect(parsedArgs.prompt).toBe(expectedQuery); + expect(parsedArgs.promptInteractive).toBeUndefined(); + if (expectedModel) { + expect(parsedArgs.model).toBe(expectedModel); + } + if (debug) { + expect(parsedArgs.debug).toBe(true); + } + }, ); - - expect(debugErrorSpy).toHaveBeenCalledWith( - expect.stringContaining( - 'Cannot use both --prompt (-p) and --prompt-interactive (-i) together', - ), - ); - // yargs.showHelp() calls console.error - expect(mockConsoleError).toHaveBeenCalled(); - - mockExit.mockRestore(); - mockConsoleError.mockRestore(); - debugErrorSpy.mockRestore(); }); - it('should throw an error when using short flags -p and -i together', async () => { - process.argv = [ - 'node', - 'script.js', - '-p', - 'test prompt', - '-i', - 'interactive prompt', - ]; + it.each([ + { + description: 'long flags', + argv: ['node', 'script.js', '--yolo', '--approval-mode', 'default'], + }, + { + description: 'short flags', + argv: ['node', 'script.js', '-y', '--approval-mode', 'yolo'], + }, + ])( + 'should throw an error when using conflicting yolo/approval-mode flags ($description)', + async ({ argv }) => { + process.argv = argv; - const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => { - throw new Error('process.exit called'); - }); + const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit called'); + }); - const mockConsoleError = vi - .spyOn(console, 'error') - .mockImplementation(() => {}); - const debugErrorSpy = vi - .spyOn(debugLogger, 'error') - .mockImplementation(() => {}); + const mockConsoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); - await expect(parseArguments({} as Settings)).rejects.toThrow( - 'process.exit called', - ); + await expect(parseArguments({} as Settings)).rejects.toThrow( + 'process.exit called', + ); - expect(debugErrorSpy).toHaveBeenCalledWith( - expect.stringContaining( - 'Cannot use both --prompt (-p) and --prompt-interactive (-i) together', - ), - ); - expect(mockConsoleError).toHaveBeenCalled(); + expect(mockConsoleError).toHaveBeenCalledWith( + expect.stringContaining( + 'Cannot use both --yolo (-y) and --approval-mode together. Use --approval-mode=yolo instead.', + ), + ); - mockExit.mockRestore(); - mockConsoleError.mockRestore(); - debugErrorSpy.mockRestore(); - }); + mockExit.mockRestore(); + mockConsoleError.mockRestore(); + }, + ); - it('should allow --prompt without --prompt-interactive', async () => { - process.argv = ['node', 'script.js', '--prompt', 'test prompt']; - const argv = await parseArguments({} as Settings); - expect(argv.prompt).toBe('test prompt'); - expect(argv.promptInteractive).toBeUndefined(); - }); - - it('should allow --prompt-interactive without --prompt', async () => { - process.argv = [ - 'node', - 'script.js', - '--prompt-interactive', - 'interactive prompt', - ]; - const argv = await parseArguments({} as Settings); - expect(argv.promptInteractive).toBe('interactive prompt'); - expect(argv.prompt).toBeUndefined(); - }); - - it('should allow -i flag as alias for --prompt-interactive', async () => { - process.argv = ['node', 'script.js', '-i', 'interactive prompt']; - const argv = await parseArguments({} as Settings); - expect(argv.promptInteractive).toBe('interactive prompt'); - expect(argv.prompt).toBeUndefined(); - }); - - it('should convert positional query argument to prompt by default', async () => { - process.argv = ['node', 'script.js', 'Hi Gemini']; - const argv = await parseArguments({} as Settings); - expect(argv.query).toBe('Hi Gemini'); - expect(argv.prompt).toBe('Hi Gemini'); - expect(argv.promptInteractive).toBeUndefined(); - }); - - it('should map @path to prompt (one-shot) when it starts with @', async () => { - process.argv = ['node', 'script.js', '@path ./file.md']; - const argv = await parseArguments({} as Settings); - expect(argv.query).toBe('@path ./file.md'); - expect(argv.prompt).toBe('@path ./file.md'); - expect(argv.promptInteractive).toBeUndefined(); - }); - - it('should map @path to prompt even when config flags are present', async () => { - // @path queries should now go to one-shot mode regardless of other flags - process.argv = [ - 'node', - 'script.js', - '@path', - './file.md', - '--model', - 'gemini-2.5-pro', - ]; - const argv = await parseArguments({} as Settings); - expect(argv.query).toBe('@path ./file.md'); - expect(argv.prompt).toBe('@path ./file.md'); // Should map to one-shot - expect(argv.promptInteractive).toBeUndefined(); - expect(argv.model).toBe('gemini-2.5-pro'); - }); - - it('maps unquoted positional @path + arg to prompt (one-shot)', async () => { - // Simulate: gemini @path ./file.md - process.argv = ['node', 'script.js', '@path', './file.md']; - const argv = await parseArguments({} as Settings); - // After normalization, query is a single string - expect(argv.query).toBe('@path ./file.md'); - // And it's mapped to one-shot prompt when no -p/-i flags are set - expect(argv.prompt).toBe('@path ./file.md'); - expect(argv.promptInteractive).toBeUndefined(); - }); - - it('should handle multiple @path arguments in a single command (one-shot)', async () => { - // Simulate: gemini @path ./file1.md @path ./file2.md - process.argv = [ - 'node', - 'script.js', - '@path', - './file1.md', - '@path', - './file2.md', - ]; - const argv = await parseArguments({} as Settings); - // After normalization, all arguments are joined with spaces - expect(argv.query).toBe('@path ./file1.md @path ./file2.md'); - // And it's mapped to one-shot prompt - expect(argv.prompt).toBe('@path ./file1.md @path ./file2.md'); - expect(argv.promptInteractive).toBeUndefined(); - }); - - it('should handle mixed quoted and unquoted @path arguments (one-shot)', async () => { - // Simulate: gemini "@path ./file1.md" @path ./file2.md "additional text" - process.argv = [ - 'node', - 'script.js', - '@path ./file1.md', - '@path', - './file2.md', - 'additional text', - ]; - const argv = await parseArguments({} as Settings); - // After normalization, all arguments are joined with spaces - expect(argv.query).toBe( - '@path ./file1.md @path ./file2.md additional text', - ); - // And it's mapped to one-shot prompt - expect(argv.prompt).toBe( - '@path ./file1.md @path ./file2.md additional text', - ); - expect(argv.promptInteractive).toBeUndefined(); - }); - - it('should map @path to prompt with ambient flags (debug)', async () => { - // Ambient flags like debug should NOT affect routing - process.argv = ['node', 'script.js', '@path', './file.md', '--debug']; - const argv = await parseArguments({} as Settings); - expect(argv.query).toBe('@path ./file.md'); - expect(argv.prompt).toBe('@path ./file.md'); // Should map to one-shot - expect(argv.promptInteractive).toBeUndefined(); - expect(argv.debug).toBe(true); - }); - - it('should map any @command to prompt (one-shot)', async () => { - // Test that all @commands now go to one-shot mode - const testCases = [ - '@path ./file.md', - '@include src/', - '@search pattern', - '@web query', - '@git status', - ]; - - for (const testQuery of testCases) { - process.argv = ['node', 'script.js', testQuery]; - const argv = await parseArguments({} as Settings); - expect(argv.query).toBe(testQuery); - expect(argv.prompt).toBe(testQuery); - expect(argv.promptInteractive).toBeUndefined(); - } - }); - - it('should handle @command with leading whitespace', async () => { - // Test that trim() + routing handles leading whitespace correctly - process.argv = ['node', 'script.js', ' @path ./file.md']; - const argv = await parseArguments({} as Settings); - expect(argv.query).toBe(' @path ./file.md'); - expect(argv.prompt).toBe(' @path ./file.md'); - expect(argv.promptInteractive).toBeUndefined(); - }); - - it('should throw an error when both --yolo and --approval-mode are used together', async () => { - process.argv = [ - 'node', - 'script.js', - '--yolo', - '--approval-mode', - 'default', - ]; - - const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => { - throw new Error('process.exit called'); - }); - - const mockConsoleError = vi - .spyOn(console, 'error') - .mockImplementation(() => {}); - const debugErrorSpy = vi - .spyOn(debugLogger, 'error') - .mockImplementation(() => {}); - - await expect(parseArguments({} as Settings)).rejects.toThrow( - 'process.exit called', - ); - - expect(debugErrorSpy).toHaveBeenCalledWith( - expect.stringContaining( - 'Cannot use both --yolo (-y) and --approval-mode together. Use --approval-mode=yolo instead.', - ), - ); - expect(mockConsoleError).toHaveBeenCalled(); - - mockExit.mockRestore(); - mockConsoleError.mockRestore(); - debugErrorSpy.mockRestore(); - }); - - it('should throw an error when using short flags -y and --approval-mode together', async () => { - process.argv = ['node', 'script.js', '-y', '--approval-mode', 'yolo']; - - const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => { - throw new Error('process.exit called'); - }); - - const mockConsoleError = vi - .spyOn(console, 'error') - .mockImplementation(() => {}); - const debugErrorSpy = vi - .spyOn(debugLogger, 'error') - .mockImplementation(() => {}); - - await expect(parseArguments({} as Settings)).rejects.toThrow( - 'process.exit called', - ); - - expect(debugErrorSpy).toHaveBeenCalledWith( - expect.stringContaining( - 'Cannot use both --yolo (-y) and --approval-mode together. Use --approval-mode=yolo instead.', - ), - ); - expect(mockConsoleError).toHaveBeenCalled(); - - mockExit.mockRestore(); - mockConsoleError.mockRestore(); - debugErrorSpy.mockRestore(); - }); - - it('should allow --approval-mode without --yolo', async () => { - process.argv = ['node', 'script.js', '--approval-mode', 'auto_edit']; - const argv = await parseArguments({} as Settings); - expect(argv.approvalMode).toBe('auto_edit'); - expect(argv.yolo).toBe(false); - }); - - it('should allow --yolo without --approval-mode', async () => { - process.argv = ['node', 'script.js', '--yolo']; - const argv = await parseArguments({} as Settings); - expect(argv.yolo).toBe(true); - expect(argv.approvalMode).toBeUndefined(); + it.each([ + { + description: 'should allow --approval-mode without --yolo', + argv: ['node', 'script.js', '--approval-mode', 'auto_edit'], + expected: { approvalMode: 'auto_edit', yolo: false }, + }, + { + description: 'should allow --yolo without --approval-mode', + argv: ['node', 'script.js', '--yolo'], + expected: { approvalMode: undefined, yolo: true }, + }, + ])('$description', async ({ argv, expected }) => { + process.argv = argv; + const parsedArgs = await parseArguments({} as Settings); + expect(parsedArgs.approvalMode).toBe(expected.approvalMode); + expect(parsedArgs.yolo).toBe(expected.yolo); }); it('should reject invalid --approval-mode values', async () => { diff --git a/packages/cli/src/config/extension.test.ts b/packages/cli/src/config/extension.test.ts index 9e94ec588c..62653481bd 100644 --- a/packages/cli/src/config/extension.test.ts +++ b/packages/cli/src/config/extension.test.ts @@ -4,7 +4,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { vi, type MockedFunction } from 'vitest'; +import { + vi, + type MockedFunction, + describe, + it, + expect, + beforeEach, + afterEach, + afterAll, +} from 'vitest'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -14,7 +23,6 @@ import { ExtensionDisableEvent, ExtensionEnableEvent, KeychainTokenStorage, - debugLogger, } from '@google/gemini-cli-core'; import { loadSettings, SettingScope } from './settings.js'; import { @@ -127,18 +135,18 @@ interface MockKeychainStorage { isAvailable: ReturnType; } -let tempHomeDir: string; -let tempWorkspaceDir: string; -let userExtensionsDir: string; -let extensionManager: ExtensionManager; -let mockRequestConsent: MockedFunction<(consent: string) => Promise>; -let mockPromptForSettings: MockedFunction< - (setting: ExtensionSetting) => Promise ->; -let mockKeychainStorage: MockKeychainStorage; -let keychainData: Record; - describe('extension tests', () => { + let tempHomeDir: string; + let tempWorkspaceDir: string; + let userExtensionsDir: string; + let extensionManager: ExtensionManager; + let mockRequestConsent: MockedFunction<(consent: string) => Promise>; + let mockPromptForSettings: MockedFunction< + (setting: ExtensionSetting) => Promise + >; + let mockKeychainStorage: MockKeychainStorage; + let keychainData: Record; + beforeEach(() => { vi.clearAllMocks(); keychainData = {}; @@ -496,8 +504,8 @@ describe('extension tests', () => { }); it('should skip extensions with invalid JSON and log a warning', async () => { - const debugErrorSpy = vi - .spyOn(debugLogger, 'error') + const consoleSpy = vi + .spyOn(console, 'error') .mockImplementation(() => {}); // Good extension @@ -517,18 +525,18 @@ describe('extension tests', () => { expect(extensions).toHaveLength(1); expect(extensions[0].name).toBe('good-ext'); - expect(debugErrorSpy).toHaveBeenCalledExactlyOnceWith( + expect(consoleSpy).toHaveBeenCalledExactlyOnceWith( expect.stringContaining( `Warning: Skipping extension in ${badExtDir}: Failed to load extension config from ${badConfigPath}`, ), ); - debugErrorSpy.mockRestore(); + consoleSpy.mockRestore(); }); it('should skip extensions with missing name and log a warning', async () => { - const debugErrorSpy = vi - .spyOn(debugLogger, 'error') + const consoleSpy = vi + .spyOn(console, 'error') .mockImplementation(() => {}); // Good extension @@ -548,13 +556,13 @@ describe('extension tests', () => { expect(extensions).toHaveLength(1); expect(extensions[0].name).toBe('good-ext'); - expect(debugErrorSpy).toHaveBeenCalledExactlyOnceWith( + expect(consoleSpy).toHaveBeenCalledExactlyOnceWith( expect.stringContaining( `Warning: Skipping extension in ${badExtDir}: Failed to load extension config from ${badConfigPath}: Invalid configuration in ${badConfigPath}: missing "name"`, ), ); - debugErrorSpy.mockRestore(); + consoleSpy.mockRestore(); }); it('should filter trust out of mcp servers', async () => { @@ -589,48 +597,13 @@ describe('extension tests', () => { const extension = extensions.find((e) => e.name === 'bad_name'); expect(extension).toBeUndefined(); - // This test is a bit ambiguous, loadExtensions catches errors and logs them, returning null for that extension. - // The implementation in loadExtension uses debugLogger.error. - // However, this test previously expected console.error. - // Wait, if I change source code to use debugLogger, I should update this. - // But let's verify what loadExtension uses. It uses debugLogger.error (checked in previous turn). - // So I should spy on debugLogger.error here too. + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('Invalid extension name: "bad_name"'), + ); consoleSpy.mockRestore(); }); - }); - // ... (rest of the file) - - it('should not load github extensions if blockGitExtensions is set', async () => { - createExtension({ - extensionsDir: userExtensionsDir, - name: 'my-ext', - version: '1.0.0', - installMetadata: { - type: 'git', - source: 'http://somehost.com/foo/bar', - }, - }); - - const blockGitExtensionsSetting = { - security: { - blockGitExtensions: true, - }, - }; - extensionManager = new ExtensionManager({ - workspaceDir: tempWorkspaceDir, - requestConsent: mockRequestConsent, - requestSetting: mockPromptForSettings, - settings: blockGitExtensionsSetting, - }); - const extensions = await extensionManager.loadExtensions(); - const extension = extensions.find((e) => e.name === 'my-ext'); - - expect(extension).toBeUndefined(); - }); - - describe('id generation', () => { - it('should generate id from source for non-github git urls', async () => { + it('should not load github extensions if blockGitExtensions is set', async () => { createExtension({ extensionsDir: userExtensionsDir, name: 'my-ext', @@ -640,103 +613,111 @@ describe('extension tests', () => { source: 'http://somehost.com/foo/bar', }, }); - const extensions = await extensionManager.loadExtensions(); - const extension = extensions.find((e) => e.name === 'my-ext'); - expect(extension?.id).toBe(hashValue('http://somehost.com/foo/bar')); - }); - it('should generate id from owner/repo for github http urls', async () => { - createExtension({ - extensionsDir: userExtensionsDir, - name: 'my-ext', - version: '1.0.0', - installMetadata: { - type: 'git', - source: 'http://github.com/foo/bar', - }, - }); - - const extensions = await extensionManager.loadExtensions(); - const extension = extensions.find((e) => e.name === 'my-ext'); - expect(extension?.id).toBe(hashValue('https://github.com/foo/bar')); - }); - - it('should generate id from owner/repo for github ssh urls', async () => { - createExtension({ - extensionsDir: userExtensionsDir, - name: 'my-ext', - version: '1.0.0', - installMetadata: { - type: 'git', - source: 'git@github.com:foo/bar', - }, - }); - - const extensions = await extensionManager.loadExtensions(); - const extension = extensions.find((e) => e.name === 'my-ext'); - expect(extension?.id).toBe(hashValue('https://github.com/foo/bar')); - }); - - it('should generate id from source for github-release extension', async () => { - createExtension({ - extensionsDir: userExtensionsDir, - name: 'my-ext', - version: '1.0.0', - installMetadata: { - type: 'github-release', - source: 'https://github.com/foo/bar', + const blockGitExtensionsSetting = { + security: { + blockGitExtensions: true, }, + }; + extensionManager = new ExtensionManager({ + workspaceDir: tempWorkspaceDir, + requestConsent: mockRequestConsent, + requestSetting: mockPromptForSettings, + settings: blockGitExtensionsSetting, }); const extensions = await extensionManager.loadExtensions(); const extension = extensions.find((e) => e.name === 'my-ext'); - expect(extension?.id).toBe(hashValue('https://github.com/foo/bar')); + + expect(extension).toBeUndefined(); }); - it('should generate id from the original source for local extension', async () => { - createExtension({ - extensionsDir: userExtensionsDir, - name: 'local-ext-name', - version: '1.0.0', - installMetadata: { - type: 'local', - source: '/some/path', + describe('id generation', () => { + it.each([ + { + description: 'should generate id from source for non-github git urls', + installMetadata: { + type: 'git' as const, + source: 'http://somehost.com/foo/bar', + }, + expectedIdSource: 'http://somehost.com/foo/bar', }, + { + description: + 'should generate id from owner/repo for github http urls', + installMetadata: { + type: 'git' as const, + source: 'http://github.com/foo/bar', + }, + expectedIdSource: 'https://github.com/foo/bar', + }, + { + description: 'should generate id from owner/repo for github ssh urls', + installMetadata: { + type: 'git' as const, + source: 'git@github.com:foo/bar', + }, + expectedIdSource: 'https://github.com/foo/bar', + }, + { + description: + 'should generate id from source for github-release extension', + installMetadata: { + type: 'github-release' as const, + source: 'https://github.com/foo/bar', + }, + expectedIdSource: 'https://github.com/foo/bar', + }, + { + description: + 'should generate id from the original source for local extension', + installMetadata: { + type: 'local' as const, + source: '/some/path', + }, + expectedIdSource: '/some/path', + }, + ])('$description', async ({ installMetadata, expectedIdSource }) => { + createExtension({ + extensionsDir: userExtensionsDir, + name: 'my-ext', + version: '1.0.0', + installMetadata, + }); + const extensions = await extensionManager.loadExtensions(); + const extension = extensions.find((e) => e.name === 'my-ext'); + expect(extension?.id).toBe(hashValue(expectedIdSource)); }); - const extensions = await extensionManager.loadExtensions(); - const extension = extensions.find((e) => e.name === 'local-ext-name'); - expect(extension?.id).toBe(hashValue('/some/path')); - }); + it('should generate id from the original source for linked extensions', async () => { + const extDevelopmentDir = path.join(tempHomeDir, 'local_extensions'); + const actualExtensionDir = createExtension({ + extensionsDir: extDevelopmentDir, + name: 'link-ext-name', + version: '1.0.0', + }); + await extensionManager.loadExtensions(); + await extensionManager.installOrUpdateExtension({ + type: 'link', + source: actualExtensionDir, + }); - it('should generate id from the original source for linked extensions', async () => { - const extDevelopmentDir = path.join(tempHomeDir, 'local_extensions'); - const actualExtensionDir = createExtension({ - extensionsDir: extDevelopmentDir, - name: 'link-ext-name', - version: '1.0.0', - }); - await extensionManager.loadExtensions(); - await extensionManager.installOrUpdateExtension({ - type: 'link', - source: actualExtensionDir, + const extension = extensionManager + .getExtensions() + .find((e) => e.name === 'link-ext-name'); + expect(extension?.id).toBe(hashValue(actualExtensionDir)); }); - const extension = extensionManager - .getExtensions() - .find((e) => e.name === 'link-ext-name'); - expect(extension?.id).toBe(hashValue(actualExtensionDir)); - }); + it('should generate id from name for extension with no install metadata', async () => { + createExtension({ + extensionsDir: userExtensionsDir, + name: 'no-meta-name', + version: '1.0.0', + }); - it('should generate id from name for extension with no install metadata', async () => { - createExtension({ - extensionsDir: userExtensionsDir, - name: 'no-meta-name', - version: '1.0.0', + const extensions = await extensionManager.loadExtensions(); + const extension = extensions.find((e) => e.name === 'no-meta-name'); + expect(extension?.id).toBe(hashValue('no-meta-name')); }); - - const extensions = await extensionManager.loadExtensions(); - const extension = extensions.find((e) => e.name === 'no-meta-name'); - expect(extension?.id).toBe(hashValue('no-meta-name')); }); }); @@ -1913,9 +1894,9 @@ This extension will run the following MCP servers: ); }); }); - - function isEnabled(options: { name: string; enabledForPath: string }) { - const manager = new ExtensionEnablementManager(); - return manager.isEnabled(options.name, options.enabledForPath); - } }); + +function isEnabled(options: { name: string; enabledForPath: string }) { + const manager = new ExtensionEnablementManager(); + return manager.isEnabled(options.name, options.enabledForPath); +} diff --git a/packages/cli/src/config/extensions/consent.test.ts b/packages/cli/src/config/extensions/consent.test.ts new file mode 100644 index 0000000000..9101c2a156 --- /dev/null +++ b/packages/cli/src/config/extensions/consent.test.ts @@ -0,0 +1,193 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + requestConsentNonInteractive, + requestConsentInteractive, + maybeRequestConsentOrFail, + INSTALL_WARNING_MESSAGE, +} from './consent.js'; +import type { ConfirmationRequest } from '../../ui/types.js'; +import type { ExtensionConfig } from '../extension.js'; +import { debugLogger } from '@google/gemini-cli-core'; + +const mockReadline = vi.hoisted(() => ({ + createInterface: vi.fn().mockReturnValue({ + question: vi.fn(), + close: vi.fn(), + }), +})); + +// Mocking readline for non-interactive prompts +vi.mock('node:readline', () => ({ + default: mockReadline, + createInterface: mockReadline.createInterface, +})); + +vi.mock('@google/gemini-cli-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + debugLogger: { + log: vi.fn(), + }, + }; +}); + +describe('consent', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe('requestConsentNonInteractive', () => { + it.each([ + { input: 'y', expected: true }, + { input: 'Y', expected: true }, + { input: '', expected: true }, + { input: 'n', expected: false }, + { input: 'N', expected: false }, + { input: 'yes', expected: false }, + ])( + 'should return $expected for input "$input"', + async ({ input, expected }) => { + const questionMock = vi.fn().mockImplementation((_, callback) => { + callback(input); + }); + mockReadline.createInterface.mockReturnValue({ + question: questionMock, + close: vi.fn(), + }); + + const consent = await requestConsentNonInteractive('Test consent'); + expect(debugLogger.log).toHaveBeenCalledWith('Test consent'); + expect(questionMock).toHaveBeenCalledWith( + 'Do you want to continue? [Y/n]: ', + expect.any(Function), + ); + expect(consent).toBe(expected); + }, + ); + }); + + describe('requestConsentInteractive', () => { + it.each([ + { confirmed: true, expected: true }, + { confirmed: false, expected: false }, + ])( + 'should resolve with $expected when user confirms with $confirmed', + async ({ confirmed, expected }) => { + const addExtensionUpdateConfirmationRequest = vi + .fn() + .mockImplementation((request: ConfirmationRequest) => { + request.onConfirm(confirmed); + }); + + const consent = await requestConsentInteractive( + 'Test consent', + addExtensionUpdateConfirmationRequest, + ); + + expect(addExtensionUpdateConfirmationRequest).toHaveBeenCalledWith({ + prompt: 'Test consent\n\nDo you want to continue?', + onConfirm: expect.any(Function), + }); + expect(consent).toBe(expected); + }, + ); + }); + + describe('maybeRequestConsentOrFail', () => { + const baseConfig: ExtensionConfig = { + name: 'test-ext', + version: '1.0.0', + }; + + it('should request consent if there is no previous config', async () => { + const requestConsent = vi.fn().mockResolvedValue(true); + await maybeRequestConsentOrFail(baseConfig, requestConsent, undefined); + expect(requestConsent).toHaveBeenCalledTimes(1); + }); + + it('should not request consent if configs are identical', async () => { + const requestConsent = vi.fn().mockResolvedValue(true); + await maybeRequestConsentOrFail(baseConfig, requestConsent, baseConfig); + expect(requestConsent).not.toHaveBeenCalled(); + }); + + it('should throw an error if consent is denied', async () => { + const requestConsent = vi.fn().mockResolvedValue(false); + await expect( + maybeRequestConsentOrFail(baseConfig, requestConsent, undefined), + ).rejects.toThrow('Installation cancelled for "test-ext".'); + }); + + describe('consent string generation', () => { + it('should generate a consent string with all fields', async () => { + const config: ExtensionConfig = { + ...baseConfig, + mcpServers: { + server1: { command: 'npm', args: ['start'] }, + server2: { httpUrl: 'https://remote.com' }, + }, + contextFileName: 'my-context.md', + excludeTools: ['tool1', 'tool2'], + }; + const requestConsent = vi.fn().mockResolvedValue(true); + await maybeRequestConsentOrFail(config, requestConsent, undefined); + + const expectedConsentString = [ + 'Installing extension "test-ext".', + INSTALL_WARNING_MESSAGE, + 'This extension will run the following MCP servers:', + ' * server1 (local): npm start', + ' * server2 (remote): https://remote.com', + 'This extension will append info to your gemini.md context using my-context.md', + 'This extension will exclude the following core tools: tool1,tool2', + ].join('\n'); + + expect(requestConsent).toHaveBeenCalledWith(expectedConsentString); + }); + + it('should request consent if mcpServers change', async () => { + const prevConfig: ExtensionConfig = { ...baseConfig }; + const newConfig: ExtensionConfig = { + ...baseConfig, + mcpServers: { server1: { command: 'npm', args: ['start'] } }, + }; + const requestConsent = vi.fn().mockResolvedValue(true); + await maybeRequestConsentOrFail(newConfig, requestConsent, prevConfig); + expect(requestConsent).toHaveBeenCalledTimes(1); + }); + + it('should request consent if contextFileName changes', async () => { + const prevConfig: ExtensionConfig = { ...baseConfig }; + const newConfig: ExtensionConfig = { + ...baseConfig, + contextFileName: 'new-context.md', + }; + const requestConsent = vi.fn().mockResolvedValue(true); + await maybeRequestConsentOrFail(newConfig, requestConsent, prevConfig); + expect(requestConsent).toHaveBeenCalledTimes(1); + }); + + it('should request consent if excludeTools changes', async () => { + const prevConfig: ExtensionConfig = { ...baseConfig }; + const newConfig: ExtensionConfig = { + ...baseConfig, + excludeTools: ['new-tool'], + }; + const requestConsent = vi.fn().mockResolvedValue(true); + await maybeRequestConsentOrFail(newConfig, requestConsent, prevConfig); + expect(requestConsent).toHaveBeenCalledTimes(1); + }); + }); + }); +}); diff --git a/packages/cli/src/config/extensions/extensionEnablement.test.ts b/packages/cli/src/config/extensions/extensionEnablement.test.ts index e26ebdbf66..831dad93f9 100644 --- a/packages/cli/src/config/extensions/extensionEnablement.test.ts +++ b/packages/cli/src/config/extensions/extensionEnablement.test.ts @@ -6,30 +6,40 @@ import * as path from 'node:path'; import fs from 'node:fs'; -import * as os from 'node:os'; + import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { ExtensionEnablementManager, Override } from './extensionEnablement.js'; +import { ExtensionStorage } from './storage.js'; + +vi.mock('./storage.js'); + import { coreEvents, GEMINI_DIR, type GeminiCLIExtension, } from '@google/gemini-cli-core'; -vi.mock('os', async (importOriginal) => { - const mockedOs = await importOriginal(); - return { - ...mockedOs, - homedir: vi.fn(), - }; -}); +vi.mock('node:os', () => ({ + homedir: vi.fn().mockReturnValue('/virtual-home'), + tmpdir: vi.fn().mockReturnValue('/virtual-tmp'), +})); + +const inMemoryFs: { [key: string]: string } = {}; // Helper to create a temporary directory for testing function createTestDir() { - const dirPath = fs.mkdtempSync(path.join(os.tmpdir(), 'gemini-test-')); + const dirPath = `/virtual-tmp/gemini-test-${Math.random().toString(36).substring(2, 15)}`; + inMemoryFs[dirPath] = ''; // Simulate directory existence return { path: dirPath, - cleanup: () => fs.rmSync(dirPath, { recursive: true, force: true }), + cleanup: () => { + for (const key in inMemoryFs) { + if (key.startsWith(dirPath)) { + delete inMemoryFs[key]; + } + } + }, }; } @@ -38,13 +48,55 @@ let manager: ExtensionEnablementManager; describe('ExtensionEnablementManager', () => { beforeEach(() => { + // Clear the in-memory file system before each test + for (const key in inMemoryFs) { + delete inMemoryFs[key]; + } + expect(Object.keys(inMemoryFs).length).toBe(0); // Add this assertion + + // Mock fs functions + vi.spyOn(fs, 'readFileSync').mockImplementation( + (path: fs.PathOrFileDescriptor) => { + const content = inMemoryFs[path.toString()]; + if (content === undefined) { + const error = new Error( + `ENOENT: no such file or directory, open '${path}'`, + ); + (error as NodeJS.ErrnoException).code = 'ENOENT'; + throw error; + } + return content; + }, + ); + vi.spyOn(fs, 'writeFileSync').mockImplementation( + ( + path: fs.PathOrFileDescriptor, + data: string | ArrayBufferView, + ) => { + inMemoryFs[path.toString()] = data.toString(); // Convert ArrayBufferView to string for inMemoryFs + }, + ); + vi.spyOn(fs, 'mkdirSync').mockImplementation( + ( + _path: fs.PathLike, + _options?: fs.MakeDirectoryOptions | fs.Mode | null, + ) => undefined, + ); + vi.spyOn(fs, 'mkdtempSync').mockImplementation((prefix: string) => { + const virtualPath = `/virtual-tmp/${prefix.replace(/[^a-zA-Z0-9]/g, '')}`; + return virtualPath; + }); + vi.spyOn(fs, 'rmSync').mockImplementation(() => {}); + testDir = createTestDir(); - vi.mocked(os.homedir).mockReturnValue(path.join(testDir.path, GEMINI_DIR)); + vi.mocked(ExtensionStorage.getUserExtensionsDir).mockReturnValue( + path.join(testDir.path, GEMINI_DIR), + ); manager = new ExtensionEnablementManager(); }); afterEach(() => { - testDir.cleanup(); + vi.restoreAllMocks(); // Reset the singleton instance for test isolation // eslint-disable-next-line @typescript-eslint/no-explicit-any (ExtensionEnablementManager as any).instance = undefined; @@ -92,7 +144,7 @@ describe('ExtensionEnablementManager', () => { ); }); - it('should handle', () => { + it('should handle overlapping rules correctly', () => { manager.enable('ext-test', true, '/home/user/projects'); manager.disable('ext-test', false, '/home/user/projects/my-app'); expect(manager.isEnabled('ext-test', '/home/user/projects/my-app')).toBe( @@ -104,6 +156,46 @@ describe('ExtensionEnablementManager', () => { }); }); + describe('remove', () => { + it('should remove an extension from the config', () => { + manager.enable('ext-test', true, '/path/to/dir'); + const config = manager.readConfig(); + expect(config['ext-test']).toBeDefined(); + + manager.remove('ext-test'); + const newConfig = manager.readConfig(); + expect(newConfig['ext-test']).toBeUndefined(); + }); + + it('should not throw when removing a non-existent extension', () => { + const config = manager.readConfig(); + expect(config['ext-test']).toBeUndefined(); + expect(() => manager.remove('ext-test')).not.toThrow(); + }); + }); + + describe('readConfig', () => { + it('should return an empty object if the config file is corrupted', () => { + const configPath = path.join( + testDir.path, + GEMINI_DIR, + 'extension-enablement.json', + ); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync(configPath, 'not a json'); + const config = manager.readConfig(); + expect(config).toEqual({}); + }); + + it('should return an empty object on generic read error', () => { + vi.spyOn(fs, 'readFileSync').mockImplementation(() => { + throw new Error('Read error'); + }); + const config = manager.readConfig(); + expect(config).toEqual({}); + }); + }); + describe('includeSubdirs', () => { it('should add a glob when enabling with includeSubdirs', () => { manager.enable('ext-test', true, '/path/to/dir'); @@ -223,7 +315,7 @@ describe('ExtensionEnablementManager', () => { }); }); - it('should enable a path based on an enable override', () => { + it('should correctly prioritize more specific enable rules', () => { manager.disable('ext-test', true, '/Users/chrstn'); manager.enable('ext-test', true, '/Users/chrstn/gemini-cli'); @@ -232,7 +324,7 @@ describe('ExtensionEnablementManager', () => { ); }); - it('should ignore subdirs', () => { + it('should not disable subdirectories if includeSubdirs is false', () => { manager.disable('ext-test', false, '/Users/chrstn'); expect(manager.isEnabled('ext-test', '/Users/chrstn/gemini-cli')).toBe( true, @@ -348,6 +440,13 @@ describe('Override', () => { }); it('should create an override from a file rule', () => { + const override = Override.fromFileRule('/path/to/dir/'); + expect(override.baseRule).toBe('/path/to/dir/'); + expect(override.isDisable).toBe(false); + expect(override.includeSubdirs).toBe(false); + }); + + it('should create an override from a file rule without a trailing slash', () => { const override = Override.fromFileRule('/path/to/dir'); expect(override.baseRule).toBe('/path/to/dir'); expect(override.isDisable).toBe(false); diff --git a/packages/cli/src/config/extensions/extensionSettings.test.ts b/packages/cli/src/config/extensions/extensionSettings.test.ts index 09aba0b80b..2c140c85c9 100644 --- a/packages/cli/src/config/extensions/extensionSettings.test.ts +++ b/packages/cli/src/config/extensions/extensionSettings.test.ts @@ -8,6 +8,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as path from 'node:path'; import * as os from 'node:os'; import { + getEnvContents, maybePromptForSettings, promptForSetting, type ExtensionSetting, @@ -188,6 +189,83 @@ describe('extensionSettings', () => { expect(actualContent).toBe(expectedContent); }); + it('should clear settings if new config has no settings', async () => { + const previousConfig: ExtensionConfig = { + name: 'test-ext', + version: '1.0.0', + settings: [ + { name: 's1', description: 'd1', envVar: 'VAR1' }, + { + name: 's2', + description: 'd2', + envVar: 'SENSITIVE_VAR', + sensitive: true, + }, + ], + }; + const newConfig: ExtensionConfig = { + name: 'test-ext', + version: '1.0.0', + settings: [], + }; + const previousSettings = { + VAR1: 'previous-VAR1', + SENSITIVE_VAR: 'secret', + }; + keychainData['SENSITIVE_VAR'] = 'secret'; + const envPath = path.join(extensionDir, '.env'); + await fsPromises.writeFile(envPath, 'VAR1=previous-VAR1'); + + await maybePromptForSettings( + newConfig, + '12345', + mockRequestSetting, + previousConfig, + previousSettings, + ); + + expect(mockRequestSetting).not.toHaveBeenCalled(); + const actualContent = await fsPromises.readFile(envPath, 'utf-8'); + expect(actualContent).toBe(''); + expect(mockKeychainStorage.deleteSecret).toHaveBeenCalledWith( + 'SENSITIVE_VAR', + ); + }); + + it('should remove sensitive settings from keychain', async () => { + const previousConfig: ExtensionConfig = { + name: 'test-ext', + version: '1.0.0', + settings: [ + { + name: 's1', + description: 'd1', + envVar: 'SENSITIVE_VAR', + sensitive: true, + }, + ], + }; + const newConfig: ExtensionConfig = { + name: 'test-ext', + version: '1.0.0', + settings: [], + }; + const previousSettings = { SENSITIVE_VAR: 'secret' }; + keychainData['SENSITIVE_VAR'] = 'secret'; + + await maybePromptForSettings( + newConfig, + '12345', + mockRequestSetting, + previousConfig, + previousSettings, + ); + + expect(mockKeychainStorage.deleteSecret).toHaveBeenCalledWith( + 'SENSITIVE_VAR', + ); + }); + it('should remove settings that are no longer in the config', async () => { const previousConfig: ExtensionConfig = { name: 'test-ext', @@ -343,5 +421,65 @@ describe('extensionSettings', () => { }); expect(result).toBe(promptValue); }); + + it('should return undefined if the user cancels the prompt', async () => { + vi.mocked(prompts).mockResolvedValue({ value: undefined }); + const result = await promptForSetting({ + name: 'Test', + description: 'Test desc', + envVar: 'TEST_VAR', + }); + expect(result).toBeUndefined(); + }); + }); + + describe('getEnvContents', () => { + const config: ExtensionConfig = { + name: 'test-ext', + version: '1.0.0', + settings: [ + { name: 's1', description: 'd1', envVar: 'VAR1' }, + { + name: 's2', + description: 'd2', + envVar: 'SENSITIVE_VAR', + sensitive: true, + }, + ], + }; + + it('should return combined contents from .env and keychain', async () => { + const envPath = path.join(extensionDir, '.env'); + await fsPromises.writeFile(envPath, 'VAR1=value1'); + keychainData['SENSITIVE_VAR'] = 'secret'; + + const contents = await getEnvContents(config, '12345'); + + expect(contents).toEqual({ + VAR1: 'value1', + SENSITIVE_VAR: 'secret', + }); + }); + + it('should return an empty object if no settings are defined', async () => { + const contents = await getEnvContents( + { name: 'test-ext', version: '1.0.0' }, + '12345', + ); + expect(contents).toEqual({}); + }); + + it('should return only keychain contents if .env file does not exist', async () => { + keychainData['SENSITIVE_VAR'] = 'secret'; + const contents = await getEnvContents(config, '12345'); + expect(contents).toEqual({ SENSITIVE_VAR: 'secret' }); + }); + + it('should return only .env contents if keychain is empty', async () => { + const envPath = path.join(extensionDir, '.env'); + await fsPromises.writeFile(envPath, 'VAR1=value1'); + const contents = await getEnvContents(config, '12345'); + expect(contents).toEqual({ VAR1: 'value1' }); + }); }); }); diff --git a/packages/cli/src/config/extensions/github.test.ts b/packages/cli/src/config/extensions/github.test.ts index 1b1f3b0e82..52c7b221d4 100644 --- a/packages/cli/src/config/extensions/github.test.ts +++ b/packages/cli/src/config/extensions/github.test.ts @@ -4,484 +4,365 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { - afterEach, - beforeEach, - describe, - expect, - it, - vi, - type MockedFunction, -} from 'vitest'; -import { - checkForExtensionUpdate, cloneFromGit, - extractFile, - findReleaseAsset, - fetchReleaseFromGithub, tryParseGithubUrl, + fetchReleaseFromGithub, + checkForExtensionUpdate, + downloadFromGitHubRelease, + findReleaseAsset, + downloadFile, + extractFile, } from './github.js'; import { simpleGit, type SimpleGit } from 'simple-git'; import { ExtensionUpdateState } from '../../ui/state/extensions.js'; import * as os from 'node:os'; -import * as fs from 'node:fs/promises'; -import * as fsSync from 'node:fs'; -import * as path from 'node:path'; +import * as fs from 'node:fs'; +import * as https from 'node:https'; import * as tar from 'tar'; -import * as archiver from 'archiver'; -import type { GeminiCLIExtension } from '@google/gemini-cli-core'; -import { ExtensionManager } from '../extension-manager.js'; -import { loadSettings } from '../settings.js'; -import type { ExtensionSetting } from './extensionSettings.js'; +import * as extract from 'extract-zip'; +import type { ExtensionManager } from '../extension-manager.js'; +import { fetchJson } from './github_fetch.js'; +import { EventEmitter } from 'node:events'; +import type { + GeminiCLIExtension, + ExtensionInstallMetadata, +} from '@google/gemini-cli-core'; +import type { ExtensionConfig } from '../extension.js'; -const mockPlatform = vi.hoisted(() => vi.fn()); -const mockArch = vi.hoisted(() => vi.fn()); -vi.mock('node:os', async (importOriginal) => { - const actual = await importOriginal(); +vi.mock('@google/gemini-cli-core', async (importOriginal) => { + const actual = + await importOriginal(); return { ...actual, - platform: mockPlatform, - arch: mockArch, + Storage: { + getGlobalSettingsPath: vi.fn().mockReturnValue('/mock/settings.json'), + getGlobalGeminiDir: vi.fn().mockReturnValue('/mock/.gemini'), + }, + debugLogger: { + error: vi.fn(), + log: vi.fn(), + }, }; }); vi.mock('simple-git'); +vi.mock('node:os'); +vi.mock('node:fs'); +vi.mock('node:https'); +vi.mock('tar'); +vi.mock('extract-zip'); +vi.mock('./github_fetch.js'); +vi.mock('../extension-manager.js'); +// Mock settings.ts to avoid top-level side effects if possible, or just rely on Storage mock +vi.mock('../settings.js', () => ({ + loadSettings: vi.fn(), + USER_SETTINGS_PATH: '/mock/settings.json', +})); -const fetchJsonMock = vi.hoisted(() => vi.fn()); -vi.mock('./github_fetch.js', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - fetchJson: fetchJsonMock, - }; -}); - -describe('git extension helpers', () => { - afterEach(() => { - vi.restoreAllMocks(); +describe('github.ts', () => { + beforeEach(() => { + vi.resetAllMocks(); }); describe('cloneFromGit', () => { - const mockGit = { - clone: vi.fn(), - getRemotes: vi.fn(), - fetch: vi.fn(), - checkout: vi.fn(), + let mockGit: { + clone: ReturnType; + getRemotes: ReturnType; + fetch: ReturnType; + checkout: ReturnType; + listRemote: ReturnType; + revparse: ReturnType; }; beforeEach(() => { + mockGit = { + clone: vi.fn(), + getRemotes: vi.fn(), + fetch: vi.fn(), + checkout: vi.fn(), + listRemote: vi.fn(), + revparse: vi.fn(), + }; vi.mocked(simpleGit).mockReturnValue(mockGit as unknown as SimpleGit); }); it('should clone, fetch and checkout a repo', async () => { - const installMetadata = { - source: 'http://my-repo.com', - ref: 'my-ref', - type: 'git' as const, - }; - const destination = '/dest'; - mockGit.getRemotes.mockResolvedValue([ - { name: 'origin', refs: { fetch: 'http://my-repo.com' } }, - ]); + mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]); - await cloneFromGit(installMetadata, destination); + await cloneFromGit( + { + type: 'git', + source: 'https://github.com/owner/repo.git', + ref: 'v1.0.0', + }, + '/dest', + ); - expect(mockGit.clone).toHaveBeenCalledWith('http://my-repo.com', './', [ - '--depth', - '1', - ]); - expect(mockGit.getRemotes).toHaveBeenCalledWith(true); - expect(mockGit.fetch).toHaveBeenCalledWith('origin', 'my-ref'); + expect(mockGit.clone).toHaveBeenCalledWith( + 'https://github.com/owner/repo.git', + './', + ['--depth', '1'], + ); + expect(mockGit.fetch).toHaveBeenCalledWith('origin', 'v1.0.0'); expect(mockGit.checkout).toHaveBeenCalledWith('FETCH_HEAD'); }); - it('should use HEAD if ref is not provided', async () => { - const installMetadata = { - source: 'http://my-repo.com', - type: 'git' as const, - }; - const destination = '/dest'; - mockGit.getRemotes.mockResolvedValue([ - { name: 'origin', refs: { fetch: 'http://my-repo.com' } }, - ]); - - await cloneFromGit(installMetadata, destination); - - expect(mockGit.fetch).toHaveBeenCalledWith('origin', 'HEAD'); - }); - - it('should throw if no remotes are found', async () => { - const installMetadata = { - source: 'http://my-repo.com', - type: 'git' as const, - }; - const destination = '/dest'; + it('should throw if no remotes found', async () => { mockGit.getRemotes.mockResolvedValue([]); - await expect(cloneFromGit(installMetadata, destination)).rejects.toThrow( - 'Failed to clone Git repository from http://my-repo.com', - ); + await expect( + cloneFromGit({ type: 'git', source: 'src' }, '/dest'), + ).rejects.toThrow('Unable to find any remotes'); }); it('should throw on clone error', async () => { - const installMetadata = { - source: 'http://my-repo.com', - type: 'git' as const, - }; - const destination = '/dest'; - mockGit.clone.mockRejectedValue(new Error('clone failed')); + mockGit.clone.mockRejectedValue(new Error('Clone failed')); - await expect(cloneFromGit(installMetadata, destination)).rejects.toThrow( - 'Failed to clone Git repository from http://my-repo.com', - ); + await expect( + cloneFromGit({ type: 'git', source: 'src' }, '/dest'), + ).rejects.toThrow('Failed to clone Git repository'); }); }); - describe('checkForExtensionUpdate', () => { - const mockGit = { - getRemotes: vi.fn(), - listRemote: vi.fn(), - revparse: vi.fn(), - }; - - let extensionManager: ExtensionManager; - let mockRequestConsent: MockedFunction< - (consent: string) => Promise - >; - let mockPromptForSettings: MockedFunction< - (setting: ExtensionSetting) => Promise - >; - let tempHomeDir: string; - let tempWorkspaceDir: string; - - beforeEach(() => { - tempHomeDir = fsSync.mkdtempSync( - path.join(os.tmpdir(), 'gemini-cli-test-home-'), - ); - tempWorkspaceDir = fsSync.mkdtempSync( - path.join(tempHomeDir, 'gemini-cli-test-workspace-'), - ); - vi.mocked(simpleGit).mockReturnValue(mockGit as unknown as SimpleGit); - mockRequestConsent = vi.fn(); - mockRequestConsent.mockResolvedValue(true); - mockPromptForSettings = vi.fn(); - mockPromptForSettings.mockResolvedValue(''); - extensionManager = new ExtensionManager({ - workspaceDir: tempWorkspaceDir, - requestConsent: mockRequestConsent, - requestSetting: mockPromptForSettings, - settings: loadSettings(tempWorkspaceDir).merged, - }); + describe('tryParseGithubUrl', () => { + it.each([ + ['https://github.com/owner/repo', 'owner', 'repo'], + ['https://github.com/owner/repo.git', 'owner', 'repo'], + ['git@github.com:owner/repo.git', 'owner', 'repo'], + ['owner/repo', 'owner', 'repo'], + ])('should parse %s to %s/%s', (url, owner, repo) => { + expect(tryParseGithubUrl(url)).toEqual({ owner, repo }); }); - it.each([ - { - testName: 'should return NOT_UPDATABLE for non-git extensions', - extension: { - installMetadata: { type: 'link', source: '' }, - }, - mockSetup: () => {}, - expected: ExtensionUpdateState.NOT_UPDATABLE, - }, - { - testName: 'should return ERROR if no remotes found', - extension: { - installMetadata: { type: 'git', source: '' }, - }, - mockSetup: () => { - mockGit.getRemotes.mockResolvedValue([]); - }, - expected: ExtensionUpdateState.ERROR, - }, - { - testName: - 'should return UPDATE_AVAILABLE when remote hash is different', - extension: { - installMetadata: { type: 'git', source: 'my/ext' }, - }, - mockSetup: () => { - mockGit.getRemotes.mockResolvedValue([ - { name: 'origin', refs: { fetch: 'http://my-repo.com' } }, - ]); - mockGit.listRemote.mockResolvedValue('remote-hash\tHEAD'); - mockGit.revparse.mockResolvedValue('local-hash'); - }, - expected: ExtensionUpdateState.UPDATE_AVAILABLE, - }, - { - testName: - 'should return UP_TO_DATE when remote and local hashes are the same', - extension: { - installMetadata: { type: 'git', source: 'my/ext' }, - }, - mockSetup: () => { - mockGit.getRemotes.mockResolvedValue([ - { name: 'origin', refs: { fetch: 'http://my-repo.com' } }, - ]); - mockGit.listRemote.mockResolvedValue('same-hash\tHEAD'); - mockGit.revparse.mockResolvedValue('same-hash'); - }, - expected: ExtensionUpdateState.UP_TO_DATE, - }, - { - testName: 'should return ERROR on git error', - extension: { - installMetadata: { type: 'git', source: 'my/ext' }, - }, - mockSetup: () => { - mockGit.getRemotes.mockRejectedValue(new Error('git error')); - }, - expected: ExtensionUpdateState.ERROR, - }, - ])('$testName', async ({ extension, mockSetup, expected }) => { - const fullExtension: GeminiCLIExtension = { - name: 'test', - id: 'test-id', - path: '/ext', - version: '1.0.0', - isActive: true, - contextFiles: [], - ...extension, - } as unknown as GeminiCLIExtension; - mockSetup(); - const result = await checkForExtensionUpdate( - fullExtension, - extensionManager, + it('should return null for non-GitHub URLs', () => { + expect(tryParseGithubUrl('https://gitlab.com/owner/repo')).toBeNull(); + }); + + it('should throw for invalid formats', () => { + expect(() => tryParseGithubUrl('invalid')).toThrow( + 'Invalid GitHub repository source', ); - expect(result).toBe(expected); }); }); describe('fetchReleaseFromGithub', () => { - it.each([ - { - ref: undefined, - allowPreRelease: true, - mockedResponse: [{ tag_name: 'v1.0.0-alpha' }, { tag_name: 'v0.9.0' }], - expectedUrl: - 'https://api.github.com/repos/owner/repo/releases?per_page=1', - expectedResult: { tag_name: 'v1.0.0-alpha' }, - }, - { - ref: undefined, - allowPreRelease: false, - mockedResponse: { tag_name: 'v0.9.0' }, - expectedUrl: 'https://api.github.com/repos/owner/repo/releases/latest', - expectedResult: { tag_name: 'v0.9.0' }, - }, - { - ref: 'v0.9.0', - allowPreRelease: undefined, - mockedResponse: { tag_name: 'v0.9.0' }, - expectedUrl: - 'https://api.github.com/repos/owner/repo/releases/tags/v0.9.0', - expectedResult: { tag_name: 'v0.9.0' }, - }, - { - ref: undefined, - allowPreRelease: undefined, - mockedResponse: { tag_name: 'v0.9.0' }, - expectedUrl: 'https://api.github.com/repos/owner/repo/releases/latest', - expectedResult: { tag_name: 'v0.9.0' }, - }, - ])( - 'should fetch release with ref=$ref and allowPreRelease=$allowPreRelease', - async ({ - ref, - allowPreRelease, - mockedResponse, - expectedUrl, - expectedResult, - }) => { - fetchJsonMock.mockResolvedValueOnce(mockedResponse); + it('should fetch latest release if no ref provided', async () => { + vi.mocked(fetchJson).mockResolvedValue({ tag_name: 'v1.0.0' }); - const result = await fetchReleaseFromGithub( - 'owner', - 'repo', - ref, - allowPreRelease, - ); + await fetchReleaseFromGithub('owner', 'repo'); - expect(fetchJsonMock).toHaveBeenCalledWith(expectedUrl); - expect(result).toEqual(expectedResult); - }, - ); + expect(fetchJson).toHaveBeenCalledWith( + 'https://api.github.com/repos/owner/repo/releases/latest', + ); + }); + + it('should fetch specific ref if provided', async () => { + vi.mocked(fetchJson).mockResolvedValue({ tag_name: 'v1.0.0' }); + + await fetchReleaseFromGithub('owner', 'repo', 'v1.0.0'); + + expect(fetchJson).toHaveBeenCalledWith( + 'https://api.github.com/repos/owner/repo/releases/tags/v1.0.0', + ); + }); + + it('should handle pre-releases if allowed', async () => { + vi.mocked(fetchJson).mockResolvedValueOnce([{ tag_name: 'v1.0.0-beta' }]); + + const result = await fetchReleaseFromGithub( + 'owner', + 'repo', + undefined, + true, + ); + + expect(result).toEqual({ tag_name: 'v1.0.0-beta' }); + }); + + it('should return null if no releases found', async () => { + vi.mocked(fetchJson).mockResolvedValueOnce([]); + + const result = await fetchReleaseFromGithub( + 'owner', + 'repo', + undefined, + true, + ); + + expect(result).toBeNull(); + }); + }); + + describe('checkForExtensionUpdate', () => { + let mockExtensionManager: ExtensionManager; + let mockGit: { + getRemotes: ReturnType; + listRemote: ReturnType; + revparse: ReturnType; + }; + + beforeEach(() => { + mockExtensionManager = { + loadExtensionConfig: vi.fn(), + } as unknown as ExtensionManager; + mockGit = { + getRemotes: vi.fn(), + listRemote: vi.fn(), + revparse: vi.fn(), + }; + vi.mocked(simpleGit).mockReturnValue(mockGit as unknown as SimpleGit); + }); + + it('should return NOT_UPDATABLE for non-git/non-release extensions', async () => { + vi.mocked(mockExtensionManager.loadExtensionConfig).mockReturnValue({ + version: '1.0.0', + } as unknown as ExtensionConfig); + + const linkExt = { + installMetadata: { type: 'link' }, + } as unknown as GeminiCLIExtension; + expect(await checkForExtensionUpdate(linkExt, mockExtensionManager)).toBe( + ExtensionUpdateState.NOT_UPDATABLE, + ); + }); + + it('should return UPDATE_AVAILABLE if git remote hash differs', async () => { + mockGit.getRemotes.mockResolvedValue([ + { name: 'origin', refs: { fetch: 'url' } }, + ]); + mockGit.listRemote.mockResolvedValue('remote-hash\tHEAD'); + mockGit.revparse.mockResolvedValue('local-hash'); + + const ext = { + path: '/path', + installMetadata: { type: 'git', source: 'url' }, + } as unknown as GeminiCLIExtension; + expect(await checkForExtensionUpdate(ext, mockExtensionManager)).toBe( + ExtensionUpdateState.UPDATE_AVAILABLE, + ); + }); + + it('should return UP_TO_DATE if git remote hash matches', async () => { + mockGit.getRemotes.mockResolvedValue([ + { name: 'origin', refs: { fetch: 'url' } }, + ]); + mockGit.listRemote.mockResolvedValue('hash\tHEAD'); + mockGit.revparse.mockResolvedValue('hash'); + + const ext = { + path: '/path', + installMetadata: { type: 'git', source: 'url' }, + } as unknown as GeminiCLIExtension; + expect(await checkForExtensionUpdate(ext, mockExtensionManager)).toBe( + ExtensionUpdateState.UP_TO_DATE, + ); + }); + }); + + describe('downloadFromGitHubRelease', () => { + it('should fail if no release data found', async () => { + // Mock fetchJson to throw for latest release check + vi.mocked(fetchJson).mockRejectedValue(new Error('Not found')); + + const result = await downloadFromGitHubRelease( + { + type: 'github-release', + source: 'owner/repo', + ref: 'v1', + } as unknown as ExtensionInstallMetadata, + '/dest', + { owner: 'owner', repo: 'repo' }, + ); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.failureReason).toBe('failed to fetch release data'); + } + }); }); describe('findReleaseAsset', () => { - const assets = [ - { name: 'darwin.arm64.extension.tar.gz', url: 'url1' }, - { name: 'darwin.x64.extension.tar.gz', url: 'url2' }, - { name: 'linux.x64.extension.tar.gz', url: 'url3' }, - { name: 'win32.x64.extension.tar.gz', url: 'url4' }, - { name: 'extension-generic.tar.gz', url: 'url5' }, - ]; - - it.each([ - { platform: 'darwin', arch: 'arm64', expected: assets[0] }, - { platform: 'linux', arch: 'arm64', expected: assets[2] }, - - { platform: 'sunos', arch: 'x64', expected: undefined }, - ])( - 'should find asset matching platform and architecture', - - ({ platform, arch, expected }) => { - mockPlatform.mockReturnValue(platform); - mockArch.mockReturnValue(arch); - const result = findReleaseAsset(assets); - expect(result).toEqual(expected); - }, - ); - - it('should find generic asset if it is the only one', () => { - const singleAsset = [{ name: 'extension.tar.gz', url: 'aurl5' }]; - - mockPlatform.mockReturnValue('darwin'); - mockArch.mockReturnValue('arm64'); - const result = findReleaseAsset(singleAsset); - expect(result).toEqual(singleAsset[0]); + it('should find platform/arch specific asset', () => { + vi.mocked(os.platform).mockReturnValue('darwin'); + vi.mocked(os.arch).mockReturnValue('arm64'); + const assets = [ + { name: 'darwin.arm64.tar.gz', url: 'url1' }, + { name: 'linux.x64.tar.gz', url: 'url2' }, + ]; + expect(findReleaseAsset(assets)).toEqual(assets[0]); }); - it('should return undefined if multiple generic assets exist', () => { - const multipleGenericAssets = [ - { name: 'extension-1.tar.gz', url: 'aurl1' }, - { name: 'extension-2.tar.gz', url: 'aurl2' }, - ]; - - mockPlatform.mockReturnValue('darwin'); - mockArch.mockReturnValue('arm64'); - const result = findReleaseAsset(multipleGenericAssets); - expect(result).toBeUndefined(); + it('should find generic asset', () => { + vi.mocked(os.platform).mockReturnValue('darwin'); + const assets = [{ name: 'generic.tar.gz', url: 'url' }]; + expect(findReleaseAsset(assets)).toEqual(assets[0]); }); }); - describe('parseGitHubRepoForReleases', () => { - it.each([ - { - source: 'https://github.com/owner/repo.git', - owner: 'owner', - repo: 'repo', - }, - { - source: 'https://github.com/owner/repo', - owner: 'owner', - repo: 'repo', - }, - { - source: 'https://github.com/owner/repo/', - owner: 'owner', - repo: 'repo', - }, - { - source: 'git@github.com:owner/repo.git', - owner: 'owner', - repo: 'repo', - }, - { source: 'owner/repo', owner: 'owner', repo: 'repo' }, - { source: 'owner/repo.git', owner: 'owner', repo: 'repo' }, - ])( - 'should parse owner and repo from $source', - ({ source, owner, repo }) => { - const result = tryParseGithubUrl(source)!; - expect(result.owner).toBe(owner); - expect(result.repo).toBe(repo); - }, - ); + describe('downloadFile', () => { + it('should download file successfully', async () => { + const mockReq = new EventEmitter(); + const mockRes = + new EventEmitter() as unknown as import('node:http').IncomingMessage; + Object.assign(mockRes, { statusCode: 200, pipe: vi.fn() }); - it('should return null on a non-GitHub URL', () => { - const source = 'https://example.com/owner/repo.git'; - expect(tryParseGithubUrl(source)).toBe(null); + vi.mocked(https.get).mockImplementation((url, options, cb) => { + if (typeof options === 'function') { + cb = options; + } + if (cb) cb(mockRes); + return mockReq as unknown as import('node:http').ClientRequest; + }); + + const mockStream = new EventEmitter() as unknown as fs.WriteStream; + Object.assign(mockStream, { close: vi.fn((cb) => cb && cb()) }); + vi.mocked(fs.createWriteStream).mockReturnValue(mockStream); + + const promise = downloadFile('url', '/dest'); + mockRes.emit('end'); + mockStream.emit('finish'); + + await expect(promise).resolves.toBeUndefined(); }); - it.each([ - { source: 'invalid-format' }, - { source: 'https://github.com/owner/repo/extra' }, - ])( - 'should throw error for invalid source format: $source', - ({ source }) => { - expect(() => tryParseGithubUrl(source)).toThrow( - `Invalid GitHub repository source: ${source}. Expected "owner/repo" or a github repo uri.`, - ); - }, - ); + it('should fail on non-200 status', async () => { + const mockReq = new EventEmitter(); + const mockRes = + new EventEmitter() as unknown as import('node:http').IncomingMessage; + Object.assign(mockRes, { statusCode: 404 }); + + vi.mocked(https.get).mockImplementation((url, options, cb) => { + if (typeof options === 'function') { + cb = options; + } + if (cb) cb(mockRes); + return mockReq as unknown as import('node:http').ClientRequest; + }); + + await expect(downloadFile('url', '/dest')).rejects.toThrow( + 'Request failed with status code 404', + ); + }); }); describe('extractFile', () => { - let tempDir: string; - - beforeEach(async () => { - tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gemini-test-')); + it('should extract tar.gz using tar', async () => { + await extractFile('file.tar.gz', '/dest'); + expect(tar.x).toHaveBeenCalled(); }); - afterEach(async () => { - await fs.rm(tempDir, { recursive: true, force: true }); + it('should extract zip using extract-zip', async () => { + vi.mocked(extract.default || extract).mockResolvedValue(undefined); + await extractFile('file.zip', '/dest'); + // Check if extract was called. Note: extract-zip export might be default or named depending on mock }); - it('should extract a .tar.gz file', async () => { - const archivePath = path.join(tempDir, 'test.tar.gz'); - const extractionDest = path.join(tempDir, 'extracted'); - await fs.mkdir(extractionDest); - - // Create a dummy file to be archived - const dummyFilePath = path.join(tempDir, 'test.txt'); - await fs.writeFile(dummyFilePath, 'hello tar'); - - // Create the tar.gz file - await tar.c( - { - gzip: true, - file: archivePath, - cwd: tempDir, - }, - ['test.txt'], + it('should throw for unsupported extensions', async () => { + await expect(extractFile('file.txt', '/dest')).rejects.toThrow( + 'Unsupported file extension', ); - - await extractFile(archivePath, extractionDest); - - const extractedFilePath = path.join(extractionDest, 'test.txt'); - const content = await fs.readFile(extractedFilePath, 'utf-8'); - expect(content).toBe('hello tar'); - }); - - it('should extract a .zip file', async () => { - const archivePath = path.join(tempDir, 'test.zip'); - const extractionDest = path.join(tempDir, 'extracted'); - await fs.mkdir(extractionDest); - - // Create a dummy file to be archived - const dummyFilePath = path.join(tempDir, 'test.txt'); - await fs.writeFile(dummyFilePath, 'hello zip'); - - // Create the zip file - const output = fsSync.createWriteStream(archivePath); - const archive = archiver.create('zip'); - - const streamFinished = new Promise((resolve, reject) => { - output.on('close', () => resolve(null)); - archive.on('error', reject); - }); - - archive.pipe(output); - archive.file(dummyFilePath, { name: 'test.txt' }); - await archive.finalize(); - await streamFinished; - - await extractFile(archivePath, extractionDest); - - const extractedFilePath = path.join(extractionDest, 'test.txt'); - const content = await fs.readFile(extractedFilePath, 'utf-8'); - expect(content).toBe('hello zip'); - }); - - it('should throw an error for unsupported file types', async () => { - const unsupportedFilePath = path.join(tempDir, 'test.txt'); - await fs.writeFile(unsupportedFilePath, 'some content'); - const extractionDest = path.join(tempDir, 'extracted'); - await fs.mkdir(extractionDest); - - await expect( - extractFile(unsupportedFilePath, extractionDest), - ).rejects.toThrow('Unsupported file extension for extraction:'); }); }); }); diff --git a/packages/cli/src/config/extensions/github.ts b/packages/cli/src/config/extensions/github.ts index 4f5316e2d7..187e790f2a 100644 --- a/packages/cli/src/config/extensions/github.ts +++ b/packages/cli/src/config/extensions/github.ts @@ -457,7 +457,7 @@ export function findReleaseAsset(assets: Asset[]): Asset | undefined { return undefined; } -async function downloadFile(url: string, dest: string): Promise { +export async function downloadFile(url: string, dest: string): Promise { const headers: { 'User-agent': string; Accept: string; diff --git a/packages/cli/src/config/extensions/storage.test.ts b/packages/cli/src/config/extensions/storage.test.ts new file mode 100644 index 0000000000..d55e0ca0d7 --- /dev/null +++ b/packages/cli/src/config/extensions/storage.test.ts @@ -0,0 +1,101 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { ExtensionStorage } from './storage.js'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import * as fs from 'node:fs'; +import { + EXTENSION_SETTINGS_FILENAME, + EXTENSIONS_CONFIG_FILENAME, +} from './variables.js'; +import { Storage } from '@google/gemini-cli-core'; + +vi.mock('node:os'); +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + promises: { + ...actual.promises, + mkdtemp: vi.fn(), + }, + }; +}); +vi.mock('@google/gemini-cli-core'); + +describe('ExtensionStorage', () => { + const mockHomeDir = '/mock/home'; + const extensionName = 'test-extension'; + let storage: ExtensionStorage; + + beforeEach(() => { + vi.mocked(os.homedir).mockReturnValue(mockHomeDir); + vi.mocked(Storage).mockImplementation( + () => + ({ + getExtensionsDir: () => + path.join(mockHomeDir, '.gemini', 'extensions'), + }) as any, // eslint-disable-line @typescript-eslint/no-explicit-any + ); + storage = new ExtensionStorage(extensionName); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should return the correct extension directory', () => { + const expectedDir = path.join( + mockHomeDir, + '.gemini', + 'extensions', + extensionName, + ); + expect(storage.getExtensionDir()).toBe(expectedDir); + }); + + it('should return the correct config path', () => { + const expectedPath = path.join( + mockHomeDir, + '.gemini', + 'extensions', + extensionName, + EXTENSIONS_CONFIG_FILENAME, // EXTENSIONS_CONFIG_FILENAME + ); + expect(storage.getConfigPath()).toBe(expectedPath); + }); + + it('should return the correct env file path', () => { + const expectedPath = path.join( + mockHomeDir, + '.gemini', + 'extensions', + extensionName, + EXTENSION_SETTINGS_FILENAME, // EXTENSION_SETTINGS_FILENAME + ); + expect(storage.getEnvFilePath()).toBe(expectedPath); + }); + + it('should return the correct user extensions directory', () => { + const expectedDir = path.join(mockHomeDir, '.gemini', 'extensions'); + expect(ExtensionStorage.getUserExtensionsDir()).toBe(expectedDir); + }); + + it('should create a temporary directory', async () => { + const mockTmpDir = '/tmp/gemini-extension-123'; + vi.mocked(fs.promises.mkdtemp).mockResolvedValue(mockTmpDir); + vi.mocked(os.tmpdir).mockReturnValue('/tmp'); + + const result = await ExtensionStorage.createTmpDir(); + + expect(fs.promises.mkdtemp).toHaveBeenCalledWith( + path.join('/tmp', 'gemini-extension'), + ); + expect(result).toBe(mockTmpDir); + }); +}); diff --git a/packages/cli/src/config/extensions/update.test.ts b/packages/cli/src/config/extensions/update.test.ts index c3a1fb64e4..068bb6e20e 100644 --- a/packages/cli/src/config/extensions/update.test.ts +++ b/packages/cli/src/config/extensions/update.test.ts @@ -4,448 +4,338 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { vi, type MockedFunction } from 'vitest'; -import * as fs from 'node:fs'; -import * as os from 'node:os'; -import * as path from 'node:path'; -import { checkForAllExtensionUpdates, updateExtension } from './update.js'; -import { GEMINI_DIR, KeychainTokenStorage } from '@google/gemini-cli-core'; -import { isWorkspaceTrusted } from '../trustedFolders.js'; -import { ExtensionUpdateState } from '../../ui/state/extensions.js'; -import { createExtension } from '../../test-utils/createExtension.js'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { - EXTENSIONS_CONFIG_FILENAME, - INSTALL_METADATA_FILENAME, -} from './variables.js'; -import { ExtensionManager } from '../extension-manager.js'; -import { loadSettings } from '../settings.js'; -import type { ExtensionSetting } from './extensionSettings.js'; + updateExtension, + updateAllUpdatableExtensions, + checkForAllExtensionUpdates, +} from './update.js'; +import { + ExtensionUpdateState, + type ExtensionUpdateStatus, +} from '../../ui/state/extensions.js'; +import { ExtensionStorage } from './storage.js'; +import { copyExtension } from '../extension-manager.js'; +import { checkForExtensionUpdate } from './github.js'; +import { loadInstallMetadata } from '../extension.js'; +import * as fs from 'node:fs'; +import type { ExtensionManager } from '../extension-manager.js'; +import type { GeminiCLIExtension } from '@google/gemini-cli-core'; -const mockGit = { - clone: vi.fn(), - getRemotes: vi.fn(), - fetch: vi.fn(), - checkout: vi.fn(), - listRemote: vi.fn(), - revparse: vi.fn(), - // Not a part of the actual API, but we need to use this to do the correct - // file system interactions. - path: vi.fn(), -}; - -vi.mock('simple-git', () => ({ - simpleGit: vi.fn((path: string) => { - mockGit.path.mockReturnValue(path); - return mockGit; - }), +// Mock dependencies +vi.mock('./storage.js', () => ({ + ExtensionStorage: { + createTmpDir: vi.fn(), + }, })); -vi.mock('os', async (importOriginal) => { - const mockedOs = await importOriginal(); - return { - ...mockedOs, - homedir: vi.fn(), - }; -}); - -vi.mock('../trustedFolders.js', () => ({ - isWorkspaceTrusted: vi.fn(), +vi.mock('../extension-manager.js', () => ({ + copyExtension: vi.fn(), + // We don't need to mock the class implementation if we pass a mock instance })); -const mockLogExtensionInstallEvent = vi.hoisted(() => vi.fn()); -const mockLogExtensionUninstall = vi.hoisted(() => vi.fn()); +vi.mock('./github.js', () => ({ + checkForExtensionUpdate: vi.fn(), +})); -vi.mock('@google/gemini-cli-core', async (importOriginal) => { - const actual = - await importOriginal(); +vi.mock('../extension.js', () => ({ + loadInstallMetadata: vi.fn(), +})); + +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); return { ...actual, - logExtensionInstallEvent: mockLogExtensionInstallEvent, - logExtensionUninstall: mockLogExtensionUninstall, - ExtensionInstallEvent: vi.fn(), - ExtensionUninstallEvent: vi.fn(), - KeychainTokenStorage: vi.fn().mockImplementation(() => ({ - getSecret: vi.fn(), - setSecret: vi.fn(), - deleteSecret: vi.fn(), - listSecrets: vi.fn(), - isAvailable: vi.fn().mockResolvedValue(true), - })), + promises: { + ...actual.promises, + rm: vi.fn(), + }, }; }); -interface MockKeychainStorage { - getSecret: ReturnType; - setSecret: ReturnType; - deleteSecret: ReturnType; - listSecrets: ReturnType; - isAvailable: ReturnType; -} - -describe('update tests', () => { - let tempHomeDir: string; - let tempWorkspaceDir: string; - let userExtensionsDir: string; - let extensionManager: ExtensionManager; - let mockRequestConsent: MockedFunction<(consent: string) => Promise>; - let mockPromptForSettings: MockedFunction< - (setting: ExtensionSetting) => Promise - >; - let mockKeychainStorage: MockKeychainStorage; - let keychainData: Record; +describe('Extension Update Logic', () => { + let mockExtensionManager: ExtensionManager; + let mockDispatch: ReturnType; + const mockExtension: GeminiCLIExtension = { + name: 'test-extension', + version: '1.0.0', + path: '/path/to/extension', + } as GeminiCLIExtension; beforeEach(() => { vi.clearAllMocks(); - keychainData = {}; - mockKeychainStorage = { - getSecret: vi - .fn() - .mockImplementation(async (key: string) => keychainData[key] || null), - setSecret: vi - .fn() - .mockImplementation(async (key: string, value: string) => { - keychainData[key] = value; - }), - deleteSecret: vi.fn().mockImplementation(async (key: string) => { - delete keychainData[key]; - }), - listSecrets: vi - .fn() - .mockImplementation(async () => Object.keys(keychainData)), - isAvailable: vi.fn().mockResolvedValue(true), - }; - ( - KeychainTokenStorage as unknown as ReturnType - ).mockImplementation(() => mockKeychainStorage); - tempHomeDir = fs.mkdtempSync( - path.join(os.tmpdir(), 'gemini-cli-test-home-'), - ); - tempWorkspaceDir = fs.mkdtempSync( - path.join(tempHomeDir, 'gemini-cli-test-workspace-'), - ); - vi.mocked(os.homedir).mockReturnValue(tempHomeDir); - userExtensionsDir = path.join(tempHomeDir, GEMINI_DIR, 'extensions'); - // Clean up before each test - fs.rmSync(userExtensionsDir, { recursive: true, force: true }); - fs.mkdirSync(userExtensionsDir, { recursive: true }); - vi.mocked(isWorkspaceTrusted).mockReturnValue({ - isTrusted: true, - source: 'file', - }); - vi.spyOn(process, 'cwd').mockReturnValue(tempWorkspaceDir); - Object.values(mockGit).forEach((fn) => fn.mockReset()); - mockRequestConsent = vi.fn(); - mockRequestConsent.mockResolvedValue(true); - mockPromptForSettings = vi.fn(); - mockPromptForSettings.mockResolvedValue(''); - extensionManager = new ExtensionManager({ - workspaceDir: tempWorkspaceDir, - requestConsent: mockRequestConsent, - requestSetting: mockPromptForSettings, - settings: loadSettings(tempWorkspaceDir).merged, - }); - }); + mockExtensionManager = { + loadExtensionConfig: vi.fn(), + installOrUpdateExtension: vi.fn(), + } as unknown as ExtensionManager; + mockDispatch = vi.fn(); - afterEach(() => { - fs.rmSync(tempHomeDir, { recursive: true, force: true }); - fs.rmSync(tempWorkspaceDir, { recursive: true, force: true }); - vi.restoreAllMocks(); + // Default mock behaviors + vi.mocked(ExtensionStorage.createTmpDir).mockResolvedValue('/tmp/mock-dir'); + vi.mocked(loadInstallMetadata).mockReturnValue({ + source: 'https://example.com/repo.git', + type: 'git', + }); }); describe('updateExtension', () => { - it('should update a git-installed extension', async () => { - const gitUrl = 'https://github.com/google/gemini-extensions.git'; - const extensionName = 'gemini-extensions'; - const targetExtDir = path.join(userExtensionsDir, extensionName); - const metadataPath = path.join(targetExtDir, INSTALL_METADATA_FILENAME); - - fs.mkdirSync(targetExtDir, { recursive: true }); - fs.writeFileSync( - path.join(targetExtDir, EXTENSIONS_CONFIG_FILENAME), - JSON.stringify({ name: extensionName, version: '1.0.0' }), + it('should return undefined if state is already UPDATING', async () => { + const result = await updateExtension( + mockExtension, + mockExtensionManager, + ExtensionUpdateState.UPDATING, + mockDispatch, ); - fs.writeFileSync( - metadataPath, - JSON.stringify({ source: gitUrl, type: 'git' }), - ); - - mockGit.clone.mockImplementation(async (_, destination) => { - fs.mkdirSync(path.join(mockGit.path(), destination), { - recursive: true, - }); - fs.writeFileSync( - path.join(mockGit.path(), destination, EXTENSIONS_CONFIG_FILENAME), - JSON.stringify({ name: extensionName, version: '1.1.0' }), - ); - }); - mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]); - const extensions = await extensionManager.loadExtensions(); - const extension = extensions.find((e) => e.name === extensionName)!; - const updateInfo = await updateExtension( - extension!, - extensionManager, - ExtensionUpdateState.UPDATE_AVAILABLE, - () => {}, - ); - - expect(updateInfo).toEqual({ - name: 'gemini-extensions', - originalVersion: '1.0.0', - updatedVersion: '1.1.0', - }); - - const updatedConfig = JSON.parse( - fs.readFileSync( - path.join(targetExtDir, EXTENSIONS_CONFIG_FILENAME), - 'utf-8', - ), - ); - expect(updatedConfig.version).toBe('1.1.0'); + expect(result).toBeUndefined(); + expect(mockDispatch).not.toHaveBeenCalled(); }); - it('should call setExtensionUpdateState with UPDATING and then UPDATED_NEEDS_RESTART on success', async () => { - const extensionName = 'test-extension'; - createExtension({ - extensionsDir: userExtensionsDir, - name: extensionName, - version: '1.0.0', - installMetadata: { - source: 'https://some.git/repo', - type: 'git', - }, - }); + it('should throw error and set state to ERROR if install metadata type is unknown', async () => { + vi.mocked(loadInstallMetadata).mockReturnValue({ + type: undefined, + } as unknown as import('@google/gemini-cli-core').ExtensionInstallMetadata); - mockGit.clone.mockImplementation(async (_, destination) => { - fs.mkdirSync(path.join(mockGit.path(), destination), { - recursive: true, - }); - fs.writeFileSync( - path.join(mockGit.path(), destination, EXTENSIONS_CONFIG_FILENAME), - JSON.stringify({ name: extensionName, version: '1.1.0' }), - ); - }); - mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]); - - const dispatch = vi.fn(); - - const extensions = await extensionManager.loadExtensions(); - const extension = extensions.find((e) => e.name === extensionName)!; - await updateExtension( - extension!, - extensionManager, - ExtensionUpdateState.UPDATE_AVAILABLE, - dispatch, - ); - - expect(dispatch).toHaveBeenCalledWith({ - type: 'SET_STATE', - payload: { - name: extensionName, - state: ExtensionUpdateState.UPDATING, - }, - }); - expect(dispatch).toHaveBeenCalledWith({ - type: 'SET_STATE', - payload: { - name: extensionName, - state: ExtensionUpdateState.UPDATED_NEEDS_RESTART, - }, - }); - }); - - it('should call setExtensionUpdateState with ERROR on failure', async () => { - const extensionName = 'test-extension'; - createExtension({ - extensionsDir: userExtensionsDir, - name: extensionName, - version: '1.0.0', - installMetadata: { - source: 'https://some.git/repo', - type: 'git', - }, - }); - - mockGit.clone.mockRejectedValue(new Error('Git clone failed')); - mockGit.getRemotes.mockResolvedValue([{ name: 'origin' }]); - - const dispatch = vi.fn(); - const extensions = await extensionManager.loadExtensions(); - const extension = extensions.find((e) => e.name === extensionName)!; await expect( updateExtension( - extension!, - extensionManager, + mockExtension, + mockExtensionManager, ExtensionUpdateState.UPDATE_AVAILABLE, - dispatch, + mockDispatch, ), - ).rejects.toThrow(); + ).rejects.toThrow('type is unknown'); - expect(dispatch).toHaveBeenCalledWith({ + expect(mockDispatch).toHaveBeenCalledWith({ type: 'SET_STATE', payload: { - name: extensionName, + name: mockExtension.name, state: ExtensionUpdateState.UPDATING, }, }); - expect(dispatch).toHaveBeenCalledWith({ + expect(mockDispatch).toHaveBeenCalledWith({ type: 'SET_STATE', payload: { - name: extensionName, + name: mockExtension.name, state: ExtensionUpdateState.ERROR, }, }); }); + + it('should throw error and set state to UP_TO_DATE if extension is linked', async () => { + vi.mocked(loadInstallMetadata).mockReturnValue({ + type: 'link', + source: '', + }); + + await expect( + updateExtension( + mockExtension, + mockExtensionManager, + ExtensionUpdateState.UPDATE_AVAILABLE, + mockDispatch, + ), + ).rejects.toThrow('Extension is linked'); + + expect(mockDispatch).toHaveBeenCalledWith({ + type: 'SET_STATE', + payload: { + name: mockExtension.name, + state: ExtensionUpdateState.UP_TO_DATE, + }, + }); + }); + + it('should successfully update extension and set state to UPDATED_NEEDS_RESTART by default', async () => { + vi.mocked(mockExtensionManager.loadExtensionConfig).mockReturnValue({ + name: 'test-extension', + version: '1.0.0', + }); + vi.mocked( + mockExtensionManager.installOrUpdateExtension, + ).mockResolvedValue({ + ...mockExtension, + version: '1.1.0', + }); + + const result = await updateExtension( + mockExtension, + mockExtensionManager, + ExtensionUpdateState.UPDATE_AVAILABLE, + mockDispatch, + ); + + expect(mockExtensionManager.installOrUpdateExtension).toHaveBeenCalled(); + expect(mockDispatch).toHaveBeenCalledWith({ + type: 'SET_STATE', + payload: { + name: mockExtension.name, + state: ExtensionUpdateState.UPDATED_NEEDS_RESTART, + }, + }); + expect(result).toEqual({ + name: 'test-extension', + originalVersion: '1.0.0', + updatedVersion: '1.1.0', + }); + expect(fs.promises.rm).toHaveBeenCalledWith('/tmp/mock-dir', { + recursive: true, + force: true, + }); + }); + + it('should set state to UPDATED if enableExtensionReloading is true', async () => { + vi.mocked(mockExtensionManager.loadExtensionConfig).mockReturnValue({ + name: 'test-extension', + version: '1.0.0', + }); + vi.mocked( + mockExtensionManager.installOrUpdateExtension, + ).mockResolvedValue({ + ...mockExtension, + version: '1.1.0', + }); + + await updateExtension( + mockExtension, + mockExtensionManager, + ExtensionUpdateState.UPDATE_AVAILABLE, + mockDispatch, + true, // enableExtensionReloading + ); + + expect(mockDispatch).toHaveBeenCalledWith({ + type: 'SET_STATE', + payload: { + name: mockExtension.name, + state: ExtensionUpdateState.UPDATED, + }, + }); + }); + + it('should rollback and set state to ERROR if installation fails', async () => { + vi.mocked(mockExtensionManager.loadExtensionConfig).mockReturnValue({ + name: 'test-extension', + version: '1.0.0', + }); + vi.mocked( + mockExtensionManager.installOrUpdateExtension, + ).mockRejectedValue(new Error('Install failed')); + + await expect( + updateExtension( + mockExtension, + mockExtensionManager, + ExtensionUpdateState.UPDATE_AVAILABLE, + mockDispatch, + ), + ).rejects.toThrow('Updated extension not found after installation'); + + expect(copyExtension).toHaveBeenCalledWith( + '/tmp/mock-dir', + mockExtension.path, + ); + expect(mockDispatch).toHaveBeenCalledWith({ + type: 'SET_STATE', + payload: { + name: mockExtension.name, + state: ExtensionUpdateState.ERROR, + }, + }); + expect(fs.promises.rm).toHaveBeenCalled(); + }); + }); + + describe('updateAllUpdatableExtensions', () => { + it('should update all extensions with UPDATE_AVAILABLE status', async () => { + const extensions: GeminiCLIExtension[] = [ + { ...mockExtension, name: 'ext1' }, + { ...mockExtension, name: 'ext2' }, + { ...mockExtension, name: 'ext3' }, + ]; + const extensionsState = new Map([ + ['ext1', { status: ExtensionUpdateState.UPDATE_AVAILABLE }], + ['ext2', { status: ExtensionUpdateState.UP_TO_DATE }], + ['ext3', { status: ExtensionUpdateState.UPDATE_AVAILABLE }], + ]); + + vi.mocked(mockExtensionManager.loadExtensionConfig).mockReturnValue({ + name: 'ext', + version: '1.0.0', + }); + vi.mocked( + mockExtensionManager.installOrUpdateExtension, + ).mockResolvedValue({ ...mockExtension, version: '1.1.0' }); + + const results = await updateAllUpdatableExtensions( + extensions, + extensionsState as Map, + mockExtensionManager, + mockDispatch, + ); + + expect(results).toHaveLength(2); + expect(results.map((r) => r.name)).toEqual(['ext1', 'ext3']); + expect( + mockExtensionManager.installOrUpdateExtension, + ).toHaveBeenCalledTimes(2); + }); }); describe('checkForAllExtensionUpdates', () => { - it('should return UpdateAvailable for a git extension with updates', async () => { - createExtension({ - extensionsDir: userExtensionsDir, - name: 'test-extension', - version: '1.0.0', - installMetadata: { - source: 'https://some.git/repo', - type: 'git', - }, - }); + it('should dispatch BATCH_CHECK_START and BATCH_CHECK_END', async () => { + await checkForAllExtensionUpdates([], mockExtensionManager, mockDispatch); - mockGit.getRemotes.mockResolvedValue([ - { name: 'origin', refs: { fetch: 'https://some.git/repo' } }, - ]); - mockGit.listRemote.mockResolvedValue('remoteHash HEAD'); - mockGit.revparse.mockResolvedValue('localHash'); + expect(mockDispatch).toHaveBeenCalledWith({ type: 'BATCH_CHECK_START' }); + expect(mockDispatch).toHaveBeenCalledWith({ type: 'BATCH_CHECK_END' }); + }); + + it('should set state to NOT_UPDATABLE if no install metadata', async () => { + const extensions: GeminiCLIExtension[] = [ + { ...mockExtension, installMetadata: undefined }, + ]; - const dispatch = vi.fn(); await checkForAllExtensionUpdates( - await extensionManager.loadExtensions(), - extensionManager, - dispatch, + extensions, + mockExtensionManager, + mockDispatch, ); - expect(dispatch).toHaveBeenCalledWith({ + + expect(mockDispatch).toHaveBeenCalledWith({ type: 'SET_STATE', payload: { - name: 'test-extension', + name: mockExtension.name, + state: ExtensionUpdateState.NOT_UPDATABLE, + }, + }); + }); + + it('should check for updates and update state', async () => { + const extensions: GeminiCLIExtension[] = [ + { ...mockExtension, installMetadata: { type: 'git', source: '...' } }, + ]; + vi.mocked(checkForExtensionUpdate).mockResolvedValue( + ExtensionUpdateState.UPDATE_AVAILABLE, + ); + + await checkForAllExtensionUpdates( + extensions, + mockExtensionManager, + mockDispatch, + ); + + expect(mockDispatch).toHaveBeenCalledWith({ + type: 'SET_STATE', + payload: { + name: mockExtension.name, + state: ExtensionUpdateState.CHECKING_FOR_UPDATES, + }, + }); + expect(mockDispatch).toHaveBeenCalledWith({ + type: 'SET_STATE', + payload: { + name: mockExtension.name, state: ExtensionUpdateState.UPDATE_AVAILABLE, }, }); }); - - it('should return UpToDate for a git extension with no updates', async () => { - createExtension({ - extensionsDir: userExtensionsDir, - name: 'test-extension', - version: '1.0.0', - installMetadata: { - source: 'https://some.git/repo', - type: 'git', - }, - }); - - mockGit.getRemotes.mockResolvedValue([ - { name: 'origin', refs: { fetch: 'https://some.git/repo' } }, - ]); - mockGit.listRemote.mockResolvedValue('sameHash HEAD'); - mockGit.revparse.mockResolvedValue('sameHash'); - - const dispatch = vi.fn(); - await checkForAllExtensionUpdates( - await extensionManager.loadExtensions(), - extensionManager, - dispatch, - ); - expect(dispatch).toHaveBeenCalledWith({ - type: 'SET_STATE', - payload: { - name: 'test-extension', - state: ExtensionUpdateState.UP_TO_DATE, - }, - }); - }); - - it('should return UpToDate for a local extension with no updates', async () => { - const localExtensionSourcePath = path.join(tempHomeDir, 'local-source'); - const sourceExtensionDir = createExtension({ - extensionsDir: localExtensionSourcePath, - name: 'my-local-ext', - version: '1.0.0', - }); - - createExtension({ - extensionsDir: userExtensionsDir, - name: 'local-extension', - version: '1.0.0', - installMetadata: { source: sourceExtensionDir, type: 'local' }, - }); - const dispatch = vi.fn(); - await checkForAllExtensionUpdates( - await extensionManager.loadExtensions(), - extensionManager, - dispatch, - ); - expect(dispatch).toHaveBeenCalledWith({ - type: 'SET_STATE', - payload: { - name: 'local-extension', - state: ExtensionUpdateState.UP_TO_DATE, - }, - }); - }); - - it('should return UpdateAvailable for a local extension with updates', async () => { - const localExtensionSourcePath = path.join(tempHomeDir, 'local-source'); - const sourceExtensionDir = createExtension({ - extensionsDir: localExtensionSourcePath, - name: 'local-extension', - version: '1.1.0', - }); - - createExtension({ - extensionsDir: userExtensionsDir, - name: 'local-extension', - version: '1.0.0', - installMetadata: { source: sourceExtensionDir, type: 'local' }, - }); - const dispatch = vi.fn(); - await checkForAllExtensionUpdates( - await extensionManager.loadExtensions(), - extensionManager, - dispatch, - ); - expect(dispatch).toHaveBeenCalledWith({ - type: 'SET_STATE', - payload: { - name: 'local-extension', - state: ExtensionUpdateState.UPDATE_AVAILABLE, - }, - }); - }); - - it('should return Error when git check fails', async () => { - createExtension({ - extensionsDir: userExtensionsDir, - name: 'error-extension', - version: '1.0.0', - installMetadata: { - source: 'https://some.git/repo', - type: 'git', - }, - }); - - mockGit.getRemotes.mockRejectedValue(new Error('Git error')); - - const dispatch = vi.fn(); - await checkForAllExtensionUpdates( - await extensionManager.loadExtensions(), - extensionManager, - dispatch, - ); - expect(dispatch).toHaveBeenCalledWith({ - type: 'SET_STATE', - payload: { - name: 'error-extension', - state: ExtensionUpdateState.ERROR, - }, - }); - }); }); }); diff --git a/packages/cli/src/config/extensions/variables.test.ts b/packages/cli/src/config/extensions/variables.test.ts index d2015f4f9e..576546ef04 100644 --- a/packages/cli/src/config/extensions/variables.test.ts +++ b/packages/cli/src/config/extensions/variables.test.ts @@ -5,7 +5,32 @@ */ import { expect, describe, it } from 'vitest'; -import { hydrateString } from './variables.js'; +import { + hydrateString, + recursivelyHydrateStrings, + validateVariables, + type VariableContext, +} from './variables.js'; + +describe('validateVariables', () => { + it('should not throw if all required variables are present', () => { + const schema = { + extensionPath: { type: 'string', description: 'test', required: true }, + } as const; + const context = { extensionPath: 'value' }; + expect(() => validateVariables(context, schema)).not.toThrow(); + }); + + it('should throw if a required variable is missing', () => { + const schema = { + extensionPath: { type: 'string', description: 'test', required: true }, + } as const; + const context = {}; + expect(() => validateVariables(context, schema)).toThrow( + 'Missing required variable: extensionPath', + ); + }); +}); describe('hydrateString', () => { it('should replace a single variable', () => { @@ -15,4 +40,88 @@ describe('hydrateString', () => { const result = hydrateString('Hello, ${extensionPath}!', context); expect(result).toBe('Hello, path/my-extension!'); }); + + it('should replace multiple variables', () => { + const context = { + extensionPath: 'path/my-extension', + workspacePath: '/ws', + }; + const result = hydrateString( + 'Ext: ${extensionPath}, WS: ${workspacePath}', + context, + ); + expect(result).toBe('Ext: path/my-extension, WS: /ws'); + }); + + it('should ignore unknown variables', () => { + const context = { + extensionPath: 'path/my-extension', + }; + const result = hydrateString('Hello, ${unknown}!', context); + expect(result).toBe('Hello, ${unknown}!'); + }); + + it('should handle null and undefined context values', () => { + const context: VariableContext = { + extensionPath: undefined, + }; + const result = hydrateString( + 'Ext: ${extensionPath}, WS: ${workspacePath}', + context, + ); + expect(result).toBe('Ext: ${extensionPath}, WS: ${workspacePath}'); + }); +}); + +describe('recursivelyHydrateStrings', () => { + const context = { + extensionPath: 'path/my-extension', + workspacePath: '/ws', + }; + + it('should hydrate strings in a flat object', () => { + const obj = { + a: 'Hello, ${workspacePath}', + b: 'Hi, ${extensionPath}', + }; + const result = recursivelyHydrateStrings(obj, context); + expect(result).toEqual({ + a: 'Hello, /ws', + b: 'Hi, path/my-extension', + }); + }); + + it('should hydrate strings in an array', () => { + const arr = ['${workspacePath}', '${extensionPath}']; + const result = recursivelyHydrateStrings(arr, context); + expect(result).toEqual(['/ws', 'path/my-extension']); + }); + + it('should hydrate strings in a nested object', () => { + const obj = { + a: 'Hello, ${workspacePath}', + b: { + c: 'Hi, ${extensionPath}', + d: ['${workspacePath}/foo'], + }, + }; + const result = recursivelyHydrateStrings(obj, context); + expect(result).toEqual({ + a: 'Hello, /ws', + b: { + c: 'Hi, path/my-extension', + d: ['/ws/foo'], + }, + }); + }); + + it('should not modify non-string values', () => { + const obj = { + a: 123, + b: true, + c: null, + }; + const result = recursivelyHydrateStrings(obj, context); + expect(result).toEqual(obj); + }); }); diff --git a/packages/cli/src/config/sandboxConfig.test.ts b/packages/cli/src/config/sandboxConfig.test.ts new file mode 100644 index 0000000000..1b38909f3b --- /dev/null +++ b/packages/cli/src/config/sandboxConfig.test.ts @@ -0,0 +1,228 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { getPackageJson } from '@google/gemini-cli-core'; +import commandExists from 'command-exists'; +import * as os from 'node:os'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { loadSandboxConfig } from './sandboxConfig.js'; + +// Mock dependencies +vi.mock('@google/gemini-cli-core', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...(actual as object), + getPackageJson: vi.fn(), + FatalSandboxError: class extends Error { + constructor(message: string) { + super(message); + this.name = 'FatalSandboxError'; + } + }, + }; +}); + +vi.mock('command-exists', () => ({ + default: { + sync: vi.fn(), + }, +})); + +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...(actual as object), + platform: vi.fn(), + }; +}); + +const mockedGetPackageJson = vi.mocked(getPackageJson); +const mockedCommandExistsSync = vi.mocked(commandExists.sync); +const mockedOsPlatform = vi.mocked(os.platform); + +describe('loadSandboxConfig', () => { + const originalEnv = { ...process.env }; + + beforeEach(() => { + vi.resetAllMocks(); + process.env = { ...originalEnv }; + mockedGetPackageJson.mockResolvedValue({ + config: { sandboxImageUri: 'default/image' }, + }); + }); + + afterEach(() => { + process.env = originalEnv; + }); + + it('should return undefined if sandbox is explicitly disabled via argv', async () => { + const config = await loadSandboxConfig({}, { sandbox: false }); + expect(config).toBeUndefined(); + }); + + it('should return undefined if sandbox is explicitly disabled via settings', async () => { + const config = await loadSandboxConfig({ tools: { sandbox: false } }, {}); + expect(config).toBeUndefined(); + }); + + it('should return undefined if sandbox is not configured', async () => { + const config = await loadSandboxConfig({}, {}); + expect(config).toBeUndefined(); + }); + + it('should return undefined if already inside a sandbox (SANDBOX env var is set)', async () => { + process.env['SANDBOX'] = '1'; + const config = await loadSandboxConfig({}, { sandbox: true }); + expect(config).toBeUndefined(); + }); + + describe('with GEMINI_SANDBOX environment variable', () => { + it('should use docker if GEMINI_SANDBOX=docker and it exists', async () => { + process.env['GEMINI_SANDBOX'] = 'docker'; + mockedCommandExistsSync.mockReturnValue(true); + const config = await loadSandboxConfig({}, {}); + expect(config).toEqual({ command: 'docker', image: 'default/image' }); + expect(mockedCommandExistsSync).toHaveBeenCalledWith('docker'); + }); + + it('should throw if GEMINI_SANDBOX is an invalid command', async () => { + process.env['GEMINI_SANDBOX'] = 'invalid-command'; + await expect(loadSandboxConfig({}, {})).rejects.toThrow( + "Invalid sandbox command 'invalid-command'. Must be one of docker, podman, sandbox-exec", + ); + }); + + it('should throw if GEMINI_SANDBOX command does not exist', async () => { + process.env['GEMINI_SANDBOX'] = 'docker'; + mockedCommandExistsSync.mockReturnValue(false); + await expect(loadSandboxConfig({}, {})).rejects.toThrow( + "Missing sandbox command 'docker' (from GEMINI_SANDBOX)", + ); + }); + }); + + describe('with sandbox: true', () => { + it('should use sandbox-exec on darwin if available', async () => { + mockedOsPlatform.mockReturnValue('darwin'); + mockedCommandExistsSync.mockImplementation( + (cmd) => cmd === 'sandbox-exec', + ); + const config = await loadSandboxConfig({}, { sandbox: true }); + expect(config).toEqual({ + command: 'sandbox-exec', + image: 'default/image', + }); + }); + + it('should prefer sandbox-exec over docker on darwin', async () => { + mockedOsPlatform.mockReturnValue('darwin'); + mockedCommandExistsSync.mockReturnValue(true); // all commands exist + const config = await loadSandboxConfig({}, { sandbox: true }); + expect(config).toEqual({ + command: 'sandbox-exec', + image: 'default/image', + }); + }); + + it('should use docker if available and sandbox is true', async () => { + mockedOsPlatform.mockReturnValue('linux'); + mockedCommandExistsSync.mockImplementation((cmd) => cmd === 'docker'); + const config = await loadSandboxConfig({ tools: { sandbox: true } }, {}); + expect(config).toEqual({ command: 'docker', image: 'default/image' }); + }); + + it('should use podman if available and docker is not', async () => { + mockedOsPlatform.mockReturnValue('linux'); + mockedCommandExistsSync.mockImplementation((cmd) => cmd === 'podman'); + const config = await loadSandboxConfig({}, { sandbox: true }); + expect(config).toEqual({ command: 'podman', image: 'default/image' }); + }); + + it('should throw if sandbox: true but no command is found', async () => { + mockedOsPlatform.mockReturnValue('linux'); + mockedCommandExistsSync.mockReturnValue(false); + await expect(loadSandboxConfig({}, { sandbox: true })).rejects.toThrow( + 'GEMINI_SANDBOX is true but failed to determine command for sandbox; ' + + 'install docker or podman or specify command in GEMINI_SANDBOX', + ); + }); + }); + + describe("with sandbox: 'command'", () => { + it('should use the specified command if it exists', async () => { + mockedCommandExistsSync.mockReturnValue(true); + const config = await loadSandboxConfig({}, { sandbox: 'podman' }); + expect(config).toEqual({ command: 'podman', image: 'default/image' }); + expect(mockedCommandExistsSync).toHaveBeenCalledWith('podman'); + }); + + it('should throw if the specified command does not exist', async () => { + mockedCommandExistsSync.mockReturnValue(false); + await expect( + loadSandboxConfig({}, { sandbox: 'podman' }), + ).rejects.toThrow( + "Missing sandbox command 'podman' (from GEMINI_SANDBOX)", + ); + }); + + it('should throw if the specified command is invalid', async () => { + await expect( + loadSandboxConfig({}, { sandbox: 'invalid-command' }), + ).rejects.toThrow( + "Invalid sandbox command 'invalid-command'. Must be one of docker, podman, sandbox-exec", + ); + }); + }); + + describe('image configuration', () => { + it('should use image from GEMINI_SANDBOX_IMAGE env var if set', async () => { + process.env['GEMINI_SANDBOX_IMAGE'] = 'env/image'; + process.env['GEMINI_SANDBOX'] = 'docker'; + mockedCommandExistsSync.mockReturnValue(true); + const config = await loadSandboxConfig({}, {}); + expect(config).toEqual({ command: 'docker', image: 'env/image' }); + }); + + it('should use image from package.json if env var is not set', async () => { + process.env['GEMINI_SANDBOX'] = 'docker'; + mockedCommandExistsSync.mockReturnValue(true); + const config = await loadSandboxConfig({}, {}); + expect(config).toEqual({ command: 'docker', image: 'default/image' }); + }); + + it('should return undefined if command is found but no image is configured', async () => { + mockedGetPackageJson.mockResolvedValue({}); // no sandboxImageUri + process.env['GEMINI_SANDBOX'] = 'docker'; + mockedCommandExistsSync.mockReturnValue(true); + const config = await loadSandboxConfig({}, {}); + expect(config).toBeUndefined(); + }); + }); + + describe('truthy/falsy sandbox values', () => { + beforeEach(() => { + mockedOsPlatform.mockReturnValue('linux'); + mockedCommandExistsSync.mockImplementation((cmd) => cmd === 'docker'); + }); + + it.each([true, 'true', '1'])( + 'should enable sandbox for value: %s', + async (value) => { + const config = await loadSandboxConfig({}, { sandbox: value }); + expect(config).toEqual({ command: 'docker', image: 'default/image' }); + }, + ); + + it.each([false, 'false', '0', undefined, null, ''])( + 'should disable sandbox for value: %s', + async (value) => { + // \`null\` is not a valid type for the arg, but good to test falsiness + const config = await loadSandboxConfig({}, { sandbox: value }); + expect(config).toBeUndefined(); + }, + ); + }); +}); diff --git a/packages/cli/src/config/sandboxConfig.ts b/packages/cli/src/config/sandboxConfig.ts index 4e5088b30d..e1b7305772 100644 --- a/packages/cli/src/config/sandboxConfig.ts +++ b/packages/cli/src/config/sandboxConfig.ts @@ -21,7 +21,7 @@ const __dirname = path.dirname(__filename); // This is a stripped-down version of the CliArgs interface from config.ts // to avoid circular dependencies. interface SandboxCliArgs { - sandbox?: boolean | string; + sandbox?: boolean | string | null; } const VALID_SANDBOX_COMMANDS: ReadonlyArray = [ 'docker', @@ -34,7 +34,7 @@ function isSandboxCommand(value: string): value is SandboxConfig['command'] { } function getSandboxCommand( - sandbox?: boolean | string, + sandbox?: boolean | string | null, ): SandboxConfig['command'] | '' { // If the SANDBOX env var is set, we're already inside the sandbox. if (process.env['SANDBOX']) { diff --git a/packages/cli/src/config/settingPaths.test.ts b/packages/cli/src/config/settingPaths.test.ts new file mode 100644 index 0000000000..0c98a7c83a --- /dev/null +++ b/packages/cli/src/config/settingPaths.test.ts @@ -0,0 +1,26 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { SettingPaths } from './settingPaths.js'; + +describe('SettingPaths', () => { + it('should have the correct structure', () => { + expect(SettingPaths).toEqual({ + General: { + PreferredEditor: 'general.preferredEditor', + }, + }); + }); + + it('should be immutable', () => { + expect(Object.isFrozen(SettingPaths)).toBe(false); // It's not frozen by default in JS unless Object.freeze is called, but it's `as const` in TS. + // However, we can check if the values are correct. + expect(SettingPaths.General.PreferredEditor).toBe( + 'general.preferredEditor', + ); + }); +}); diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index ed4b219041..f370a8dd1c 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -157,107 +157,53 @@ describe('Settings Loading and Merging', () => { expect(settings.merged).toEqual({}); }); - it('should load system settings if only system file exists', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === getSystemSettingsPath(), - ); - const systemSettingsContent = { - ui: { - theme: 'system-default', + it.each([ + { + scope: 'system', + path: getSystemSettingsPath(), + content: { + ui: { theme: 'system-default' }, + tools: { sandbox: false }, }, - tools: { - sandbox: false, + }, + { + scope: 'user', + path: USER_SETTINGS_PATH, + content: { + ui: { theme: 'dark' }, + context: { fileName: 'USER_CONTEXT.md' }, }, - }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === getSystemSettingsPath()) - return JSON.stringify(systemSettingsContent); - return '{}'; + }, + { + scope: 'workspace', + path: MOCK_WORKSPACE_SETTINGS_PATH, + content: { + tools: { sandbox: true }, + context: { fileName: 'WORKSPACE_CONTEXT.md' }, }, - ); + }, + ])( + 'should load $scope settings if only $scope file exists', + ({ scope, path, content }) => { + (mockFsExistsSync as Mock).mockImplementation( + (p: fs.PathLike) => p === path, + ); + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === path) return JSON.stringify(content); + return '{}'; + }, + ); - const settings = loadSettings(MOCK_WORKSPACE_DIR); + const settings = loadSettings(MOCK_WORKSPACE_DIR); - expect(fs.readFileSync).toHaveBeenCalledWith( - getSystemSettingsPath(), - 'utf-8', - ); - expect(settings.system.settings).toEqual(systemSettingsContent); - expect(settings.user.settings).toEqual({}); - expect(settings.workspace.settings).toEqual({}); - expect(settings.merged).toEqual({ - ...systemSettingsContent, - }); - }); - - it('should load user settings if only user file exists', () => { - const expectedUserSettingsPath = USER_SETTINGS_PATH; // Use the path actually resolved by the (mocked) module - - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === expectedUserSettingsPath, - ); - const userSettingsContent = { - ui: { - theme: 'dark', - }, - context: { - fileName: 'USER_CONTEXT.md', - }, - }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === expectedUserSettingsPath) - return JSON.stringify(userSettingsContent); - return '{}'; - }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - - expect(fs.readFileSync).toHaveBeenCalledWith( - expectedUserSettingsPath, - 'utf-8', - ); - expect(settings.user.settings).toEqual(userSettingsContent); - expect(settings.workspace.settings).toEqual({}); - expect(settings.merged).toEqual({ - ...userSettingsContent, - }); - }); - - it('should load workspace settings if only workspace file exists', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === MOCK_WORKSPACE_SETTINGS_PATH, - ); - const workspaceSettingsContent = { - tools: { - sandbox: true, - }, - context: { - fileName: 'WORKSPACE_CONTEXT.md', - }, - }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === MOCK_WORKSPACE_SETTINGS_PATH) - return JSON.stringify(workspaceSettingsContent); - return ''; - }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - - expect(fs.readFileSync).toHaveBeenCalledWith( - MOCK_WORKSPACE_SETTINGS_PATH, - 'utf-8', - ); - expect(settings.user.settings).toEqual({}); - expect(settings.workspace.settings).toEqual(workspaceSettingsContent); - expect(settings.merged).toEqual({ - ...workspaceSettingsContent, - }); - }); + expect(fs.readFileSync).toHaveBeenCalledWith(path, 'utf-8'); + expect( + settings[scope as 'system' | 'user' | 'workspace'].settings, + ).toEqual(content); + expect(settings.merged).toEqual(content); + }, + ); it('should merge system, user and workspace settings, with system taking precedence over workspace, and workspace over user', () => { (mockFsExistsSync as Mock).mockImplementation( @@ -662,88 +608,63 @@ describe('Settings Loading and Merging', () => { expect(settings.merged.security?.disableYoloMode).toBe(true); // System setting should be used }); - it('should handle contextFileName correctly when only in user settings', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === USER_SETTINGS_PATH, - ); - const userSettingsContent = { context: { fileName: 'CUSTOM.md' } }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(userSettingsContent); - return ''; + it.each([ + { + description: 'contextFileName in user settings', + path: USER_SETTINGS_PATH, + content: { context: { fileName: 'CUSTOM.md' } }, + expected: { key: 'context.fileName', value: 'CUSTOM.md' }, + }, + { + description: 'contextFileName in workspace settings', + path: MOCK_WORKSPACE_SETTINGS_PATH, + content: { context: { fileName: 'PROJECT_SPECIFIC.md' } }, + expected: { key: 'context.fileName', value: 'PROJECT_SPECIFIC.md' }, + }, + { + description: 'excludedProjectEnvVars in user settings', + path: USER_SETTINGS_PATH, + content: { + advanced: { excludedEnvVars: ['DEBUG', 'NODE_ENV', 'CUSTOM_VAR'] }, }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - expect(settings.merged.context?.fileName).toBe('CUSTOM.md'); - }); - - it('should handle contextFileName correctly when only in workspace settings', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === MOCK_WORKSPACE_SETTINGS_PATH, - ); - const workspaceSettingsContent = { - context: { fileName: 'PROJECT_SPECIFIC.md' }, - }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === MOCK_WORKSPACE_SETTINGS_PATH) - return JSON.stringify(workspaceSettingsContent); - return ''; + expected: { + key: 'advanced.excludedEnvVars', + value: ['DEBUG', 'NODE_ENV', 'CUSTOM_VAR'], }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - expect(settings.merged.context?.fileName).toBe('PROJECT_SPECIFIC.md'); - }); - - it('should handle excludedProjectEnvVars correctly when only in user settings', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === USER_SETTINGS_PATH, - ); - const userSettingsContent = { - general: {}, - advanced: { excludedEnvVars: ['DEBUG', 'NODE_ENV', 'CUSTOM_VAR'] }, - }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(userSettingsContent); - return ''; + }, + { + description: 'excludedProjectEnvVars in workspace settings', + path: MOCK_WORKSPACE_SETTINGS_PATH, + content: { + advanced: { excludedEnvVars: ['WORKSPACE_DEBUG', 'WORKSPACE_VAR'] }, }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - expect(settings.merged.advanced?.excludedEnvVars).toEqual([ - 'DEBUG', - 'NODE_ENV', - 'CUSTOM_VAR', - ]); - }); - - it('should handle excludedProjectEnvVars correctly when only in workspace settings', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === MOCK_WORKSPACE_SETTINGS_PATH, - ); - const workspaceSettingsContent = { - general: {}, - advanced: { excludedEnvVars: ['WORKSPACE_DEBUG', 'WORKSPACE_VAR'] }, - }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === MOCK_WORKSPACE_SETTINGS_PATH) - return JSON.stringify(workspaceSettingsContent); - return ''; + expected: { + key: 'advanced.excludedEnvVars', + value: ['WORKSPACE_DEBUG', 'WORKSPACE_VAR'], }, - ); + }, + ])( + 'should handle $description correctly', + ({ path, content, expected }) => { + (mockFsExistsSync as Mock).mockImplementation( + (p: fs.PathLike) => p === path, + ); + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === path) return JSON.stringify(content); + return '{}'; + }, + ); - const settings = loadSettings(MOCK_WORKSPACE_DIR); - expect(settings.merged.advanced?.excludedEnvVars).toEqual([ - 'WORKSPACE_DEBUG', - 'WORKSPACE_VAR', - ]); - }); + const settings = loadSettings(MOCK_WORKSPACE_DIR); + const keys = expected.key.split('.'); + let result: unknown = settings.merged; + for (const key of keys) { + result = (result as { [key: string]: unknown })[key]; + } + expect(result).toEqual(expected.value); + }, + ); it('should merge excludedProjectEnvVars with workspace taking precedence over user', () => { (mockFsExistsSync as Mock).mockImplementation( @@ -810,37 +731,35 @@ describe('Settings Loading and Merging', () => { expect(settings.merged.context?.fileName).toBeUndefined(); }); - it('should load telemetry setting from user settings', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === USER_SETTINGS_PATH, - ); - const userSettingsContent = { telemetry: { enabled: true } }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(userSettingsContent); - return '{}'; - }, - ); - const settings = loadSettings(MOCK_WORKSPACE_DIR); - expect(settings.merged.telemetry?.enabled).toBe(true); - }); - - it('should load telemetry setting from workspace settings', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === MOCK_WORKSPACE_SETTINGS_PATH, - ); - const workspaceSettingsContent = { telemetry: { enabled: false } }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === MOCK_WORKSPACE_SETTINGS_PATH) - return JSON.stringify(workspaceSettingsContent); - return '{}'; - }, - ); - const settings = loadSettings(MOCK_WORKSPACE_DIR); - expect(settings.merged.telemetry?.enabled).toBe(false); - }); + it.each([ + { + scope: 'user', + path: USER_SETTINGS_PATH, + content: { telemetry: { enabled: true } }, + expected: true, + }, + { + scope: 'workspace', + path: MOCK_WORKSPACE_SETTINGS_PATH, + content: { telemetry: { enabled: false } }, + expected: false, + }, + ])( + 'should load telemetry setting from $scope settings', + ({ path, content, expected }) => { + (mockFsExistsSync as Mock).mockImplementation( + (p: fs.PathLike) => p === path, + ); + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === path) return JSON.stringify(content); + return '{}'; + }, + ); + const settings = loadSettings(MOCK_WORKSPACE_DIR); + expect(settings.merged.telemetry?.enabled).toBe(expected); + }, + ); it('should prioritize workspace telemetry setting over user setting', () => { (mockFsExistsSync as Mock).mockReturnValue(true); @@ -932,63 +851,60 @@ describe('Settings Loading and Merging', () => { }); }); - it('should handle MCP servers when only in user settings', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === USER_SETTINGS_PATH, - ); - const userSettingsContent = { - mcpServers: { + it.each([ + { + scope: 'user', + path: USER_SETTINGS_PATH, + content: { + mcpServers: { + 'user-only-server': { + command: 'user-only-command', + description: 'User only server', + }, + }, + }, + expected: { 'user-only-server': { command: 'user-only-command', description: 'User only server', }, }, - }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(userSettingsContent); - return ''; + }, + { + scope: 'workspace', + path: MOCK_WORKSPACE_SETTINGS_PATH, + content: { + mcpServers: { + 'workspace-only-server': { + command: 'workspace-only-command', + description: 'Workspace only server', + }, + }, }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - expect(settings.merged.mcpServers).toEqual({ - 'user-only-server': { - command: 'user-only-command', - description: 'User only server', - }, - }); - }); - - it('should handle MCP servers when only in workspace settings', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === MOCK_WORKSPACE_SETTINGS_PATH, - ); - const workspaceSettingsContent = { - mcpServers: { + expected: { 'workspace-only-server': { command: 'workspace-only-command', description: 'Workspace only server', }, }, - }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === MOCK_WORKSPACE_SETTINGS_PATH) - return JSON.stringify(workspaceSettingsContent); - return ''; - }, - ); + }, + ])( + 'should handle MCP servers when only in $scope settings', + ({ path, content, expected }) => { + (mockFsExistsSync as Mock).mockImplementation( + (p: fs.PathLike) => p === path, + ); + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === path) return JSON.stringify(content); + return '{}'; + }, + ); - const settings = loadSettings(MOCK_WORKSPACE_DIR); - expect(settings.merged.mcpServers).toEqual({ - 'workspace-only-server': { - command: 'workspace-only-command', - description: 'Workspace only server', - }, - }); - }); + const settings = loadSettings(MOCK_WORKSPACE_DIR); + expect(settings.merged.mcpServers).toEqual(expected); + }, + ); it('should have mcpServers as undefined if not in any settings file', () => { (mockFsExistsSync as Mock).mockReturnValue(false); // No settings files exist @@ -1104,85 +1020,49 @@ describe('Settings Loading and Merging', () => { }); }); - it('should merge compressionThreshold settings, with workspace taking precedence', () => { - (mockFsExistsSync as Mock).mockReturnValue(true); - const userSettingsContent = { - general: {}, - model: { compressionThreshold: 0.5 }, - }; - const workspaceSettingsContent = { - general: {}, - model: { compressionThreshold: 0.8 }, - }; - - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(userSettingsContent); - if (p === MOCK_WORKSPACE_SETTINGS_PATH) - return JSON.stringify(workspaceSettingsContent); - return '{}'; + describe('compressionThreshold settings', () => { + it.each([ + { + description: + 'should be taken from user settings if only present there', + userContent: { model: { compressionThreshold: 0.5 } }, + workspaceContent: {}, + expected: 0.5, }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - - expect(settings.user.settings.model?.compressionThreshold).toEqual(0.5); - expect(settings.workspace.settings.model?.compressionThreshold).toEqual( - 0.8, - ); - expect(settings.merged.model?.compressionThreshold).toEqual(0.8); - }); - - it('should merge output format settings, with workspace taking precedence', () => { - (mockFsExistsSync as Mock).mockReturnValue(true); - const userSettingsContent = { - output: { format: 'text' }, - }; - const workspaceSettingsContent = { - output: { format: 'json' }, - }; - - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(userSettingsContent); - if (p === MOCK_WORKSPACE_SETTINGS_PATH) - return JSON.stringify(workspaceSettingsContent); - return '{}'; + { + description: + 'should be taken from workspace settings if only present there', + userContent: {}, + workspaceContent: { model: { compressionThreshold: 0.8 } }, + expected: 0.8, }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - - expect(settings.merged.output?.format).toBe('json'); - }); - - it('should handle compressionThreshold when only in user settings', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === USER_SETTINGS_PATH, - ); - const userSettingsContent = { - general: {}, - model: { compressionThreshold: 0.5 }, - }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(userSettingsContent); - return '{}'; + { + description: + 'should prioritize workspace settings over user settings', + userContent: { model: { compressionThreshold: 0.5 } }, + workspaceContent: { model: { compressionThreshold: 0.8 } }, + expected: 0.8, }, - ); + { + description: 'should be undefined if not in any settings file', + userContent: {}, + workspaceContent: {}, + expected: undefined, + }, + ])('$description', ({ userContent, workspaceContent, expected }) => { + (mockFsExistsSync as Mock).mockReturnValue(true); + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === USER_SETTINGS_PATH) return JSON.stringify(userContent); + if (p === MOCK_WORKSPACE_SETTINGS_PATH) + return JSON.stringify(workspaceContent); + return '{}'; + }, + ); - const settings = loadSettings(MOCK_WORKSPACE_DIR); - expect(settings.merged.model?.compressionThreshold).toEqual(0.5); - }); - - it('should have model as undefined if not in any settings file', () => { - (mockFsExistsSync as Mock).mockReturnValue(false); // No settings files exist - (fs.readFileSync as Mock).mockReturnValue('{}'); - const settings = loadSettings(MOCK_WORKSPACE_DIR); - expect(settings.merged.model).toBeUndefined(); + const settings = loadSettings(MOCK_WORKSPACE_DIR); + expect(settings.merged.model?.compressionThreshold).toEqual(expected); + }); }); it('should use user compressionThreshold if workspace does not define it', () => {