diff --git a/integration-tests/test-helper.ts b/integration-tests/test-helper.ts index 53b409b733..9a2a6cefca 100644 --- a/integration-tests/test-helper.ts +++ b/integration-tests/test-helper.ts @@ -341,7 +341,9 @@ export class TestRig { ui: { useAlternateBuffer: true, }, - model: DEFAULT_GEMINI_MODEL, + model: { + name: DEFAULT_GEMINI_MODEL, + }, sandbox: env['GEMINI_SANDBOX'] !== 'false' ? env['GEMINI_SANDBOX'] : false, // Don't show the IDE connection dialog when running from VsCode diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index 4c07d08b38..004b60ea28 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -44,7 +44,7 @@ vi.mock('./settingsSchema.js', async (importOriginal) => { }); // NOW import everything else, including the (now effectively re-exported) settings.js -import path, * as pathActual from 'node:path'; // Restored for MOCK_WORKSPACE_SETTINGS_PATH +import * as pathActual from 'node:path'; // Restored for MOCK_WORKSPACE_SETTINGS_PATH import { describe, it, @@ -65,18 +65,12 @@ import { USER_SETTINGS_PATH, // This IS the mocked path. getSystemSettingsPath, getSystemDefaultsPath, - migrateSettingsToV1, - needsMigration, type Settings, - loadEnvironment, - migrateDeprecatedSettings, - SettingScope, saveSettings, type SettingsFile, getDefaultsFromSchema, } from './settings.js'; import { FatalConfigError, GEMINI_DIR } from '@google/gemini-cli-core'; -import { ExtensionManager } from './extension-manager.js'; import { updateSettingsFilePreservingFormat } from '../utils/commentJson.js'; import { getSettingsSchema, @@ -290,169 +284,6 @@ describe('Settings Loading and Merging', () => { }); }); - it('should correctly migrate a complex legacy (v1) settings file', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === USER_SETTINGS_PATH, - ); - const legacySettingsContent = { - theme: 'legacy-dark', - vimMode: true, - contextFileName: 'LEGACY_CONTEXT.md', - model: 'gemini-2.5-pro', - mcpServers: { - 'legacy-server-1': { - command: 'npm', - args: ['run', 'start:server1'], - description: 'Legacy Server 1', - }, - 'legacy-server-2': { - command: 'node', - args: ['server2.js'], - description: 'Legacy Server 2', - }, - }, - allowMCPServers: ['legacy-server-1'], - someUnrecognizedSetting: 'should-be-preserved', - }; - - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(legacySettingsContent); - return '{}'; - }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - - expect(settings.merged).toMatchObject({ - ui: { - theme: 'legacy-dark', - }, - general: { - vimMode: true, - }, - context: { - fileName: 'LEGACY_CONTEXT.md', - }, - model: { - name: 'gemini-2.5-pro', - }, - mcpServers: { - 'legacy-server-1': { - command: 'npm', - args: ['run', 'start:server1'], - description: 'Legacy Server 1', - }, - 'legacy-server-2': { - command: 'node', - args: ['server2.js'], - description: 'Legacy Server 2', - }, - }, - mcp: { - allowed: ['legacy-server-1'], - }, - someUnrecognizedSetting: 'should-be-preserved', - }); - }); - - it('should rewrite allowedTools to tools.allowed during migration', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === USER_SETTINGS_PATH, - ); - const legacySettingsContent = { - allowedTools: ['fs', 'shell'], - }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(legacySettingsContent); - return '{}'; - }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - - expect(settings.merged.tools?.allowed).toEqual(['fs', 'shell']); - expect((settings.merged as TestSettings)['allowedTools']).toBeUndefined(); - }); - - it('should allow V2 settings to override V1 settings when both are present (zombie setting fix)', () => { - (mockFsExistsSync as Mock).mockImplementation( - (p: fs.PathLike) => p === USER_SETTINGS_PATH, - ); - const mixedSettingsContent = { - // V1 setting (migrates to ui.accessibility.screenReader = true) - accessibility: { - screenReader: true, - }, - // V2 setting (explicitly set to false) - ui: { - accessibility: { - screenReader: false, - }, - }, - }; - - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(mixedSettingsContent); - return '{}'; - }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - - // We expect the V2 setting (false) to win, NOT the migrated V1 setting (true) - expect(settings.merged.ui?.accessibility?.screenReader).toBe(false); - }); - - it('should correctly merge and migrate legacy array properties from multiple scopes', () => { - (mockFsExistsSync as Mock).mockReturnValue(true); - const legacyUserSettings = { - includeDirectories: ['/user/dir'], - excludeTools: ['user-tool'], - excludedProjectEnvVars: ['USER_VAR'], - }; - const legacyWorkspaceSettings = { - includeDirectories: ['/workspace/dir'], - excludeTools: ['workspace-tool'], - excludedProjectEnvVars: ['WORKSPACE_VAR', 'USER_VAR'], - }; - - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(legacyUserSettings); - if (p === MOCK_WORKSPACE_SETTINGS_PATH) - return JSON.stringify(legacyWorkspaceSettings); - return '{}'; - }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - - // Verify includeDirectories are concatenated - expect(settings.merged.context?.includeDirectories).toEqual([ - '/user/dir', - '/workspace/dir', - ]); - - // Verify excludeTools are concatenated and de-duped - expect(settings.merged.tools?.exclude).toEqual([ - 'user-tool', - 'workspace-tool', - ]); - - // Verify excludedProjectEnvVars are concatenated and de-duped - expect(settings.merged.advanced?.excludedEnvVars).toEqual( - expect.arrayContaining(['USER_VAR', 'WORKSPACE_VAR']), - ); - expect(settings.merged.advanced?.excludedEnvVars).toHaveLength(4); - }); - it('should merge all settings files with the correct precedence', () => { // Mock schema to test defaults application const mockSchema = { @@ -1771,653 +1602,6 @@ describe('Settings Loading and Merging', () => { }); }); - describe('with workspace trust', () => { - it('should merge workspace settings when workspace is trusted', () => { - (mockFsExistsSync as Mock).mockReturnValue(true); - const userSettingsContent = { - ui: { theme: 'dark' }, - tools: { sandbox: false }, - }; - const workspaceSettingsContent = { - tools: { sandbox: true }, - context: { fileName: 'WORKSPACE.md' }, - }; - - (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 '{}'; - }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - expect(settings.merged.tools?.sandbox).toBe(true); - expect(settings.merged.context?.fileName).toBe('WORKSPACE.md'); - expect(settings.merged.ui?.theme).toBe('dark'); - }); - - it('should NOT merge workspace settings when workspace is not trusted', () => { - vi.mocked(isWorkspaceTrusted).mockReturnValue({ - isTrusted: false, - source: 'file', - }); - (mockFsExistsSync as Mock).mockReturnValue(true); - const userSettingsContent = { - ui: { theme: 'dark' }, - tools: { sandbox: false }, - context: { fileName: 'USER.md' }, - }; - const workspaceSettingsContent = { - tools: { sandbox: true }, - context: { fileName: 'WORKSPACE.md' }, - }; - - (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 '{}'; - }, - ); - - const settings = loadSettings(MOCK_WORKSPACE_DIR); - - expect(settings.merged.tools?.sandbox).toBe(false); // User setting - expect(settings.merged.context?.fileName).toBe('USER.md'); // User setting - expect(settings.merged.ui?.theme).toBe('dark'); // User setting - }); - }); - - describe('migrateSettingsToV1', () => { - it('should handle an empty object', () => { - const v2Settings = {}; - const v1Settings = migrateSettingsToV1(v2Settings); - expect(v1Settings).toEqual({}); - }); - - it('should migrate a simple v2 settings object to v1', () => { - const v2Settings = { - general: { - preferredEditor: 'vscode', - vimMode: true, - }, - ui: { - theme: 'dark', - }, - }; - const v1Settings = migrateSettingsToV1(v2Settings); - expect(v1Settings).toEqual({ - preferredEditor: 'vscode', - vimMode: true, - theme: 'dark', - }); - }); - - it('should handle nested properties correctly', () => { - const v2Settings = { - security: { - folderTrust: { - enabled: true, - }, - auth: { - selectedType: 'oauth', - }, - }, - advanced: { - autoConfigureMemory: true, - }, - }; - const v1Settings = migrateSettingsToV1(v2Settings); - expect(v1Settings).toEqual({ - folderTrust: true, - selectedAuthType: 'oauth', - autoConfigureMaxOldSpaceSize: true, - }); - }); - - it('should preserve mcpServers at the top level', () => { - const v2Settings = { - general: { - preferredEditor: 'vscode', - }, - mcpServers: { - 'my-server': { - command: 'npm start', - }, - }, - }; - const v1Settings = migrateSettingsToV1(v2Settings); - expect(v1Settings).toEqual({ - preferredEditor: 'vscode', - mcpServers: { - 'my-server': { - command: 'npm start', - }, - }, - }); - }); - - it('should carry over unrecognized top-level properties', () => { - const v2Settings = { - general: { - vimMode: false, - }, - unrecognized: 'value', - another: { - nested: true, - }, - }; - const v1Settings = migrateSettingsToV1(v2Settings); - expect(v1Settings).toEqual({ - vimMode: false, - unrecognized: 'value', - another: { - nested: true, - }, - }); - }); - - it('should handle a complex object with mixed properties', () => { - const v2Settings = { - general: { - disableAutoUpdate: true, - }, - ui: { - hideBanner: true, - customThemes: { - myTheme: {}, - }, - }, - model: { - name: 'gemini-pro', - }, - mcpServers: { - 'server-1': { - command: 'node server.js', - }, - }, - unrecognized: { - should: 'be-preserved', - }, - }; - const v1Settings = migrateSettingsToV1(v2Settings); - expect(v1Settings).toEqual({ - disableAutoUpdate: true, - hideBanner: true, - customThemes: { - myTheme: {}, - }, - model: 'gemini-pro', - mcpServers: { - 'server-1': { - command: 'node server.js', - }, - }, - unrecognized: { - should: 'be-preserved', - }, - }); - }); - - it('should not migrate a v1 settings object', () => { - const v1Settings = { - preferredEditor: 'vscode', - vimMode: true, - theme: 'dark', - }; - const migratedSettings = migrateSettingsToV1(v1Settings); - expect(migratedSettings).toEqual({ - preferredEditor: 'vscode', - vimMode: true, - theme: 'dark', - }); - }); - - it('should migrate a full v2 settings object to v1', () => { - const v2Settings: TestSettings = { - general: { - preferredEditor: 'code', - vimMode: true, - }, - ui: { - theme: 'dark', - }, - privacy: { - usageStatisticsEnabled: false, - }, - model: { - name: 'gemini-2.5-pro', - }, - context: { - fileName: 'CONTEXT.md', - includeDirectories: ['/src'], - }, - tools: { - sandbox: true, - exclude: ['toolA'], - }, - mcp: { - allowed: ['server1'], - }, - security: { - folderTrust: { - enabled: true, - }, - }, - advanced: { - dnsResolutionOrder: 'ipv4first', - excludedEnvVars: ['SECRET'], - }, - mcpServers: { - 'my-server': { - command: 'npm start', - }, - }, - unrecognizedTopLevel: { - value: 'should be preserved', - }, - }; - - const v1Settings = migrateSettingsToV1(v2Settings); - - expect(v1Settings).toEqual({ - preferredEditor: 'code', - vimMode: true, - theme: 'dark', - usageStatisticsEnabled: false, - model: 'gemini-2.5-pro', - contextFileName: 'CONTEXT.md', - includeDirectories: ['/src'], - sandbox: true, - excludeTools: ['toolA'], - allowMCPServers: ['server1'], - folderTrust: true, - dnsResolutionOrder: 'ipv4first', - excludedProjectEnvVars: ['SECRET'], - mcpServers: { - 'my-server': { - command: 'npm start', - }, - }, - unrecognizedTopLevel: { - value: 'should be preserved', - }, - }); - }); - - it('should handle partial v2 settings', () => { - const v2Settings: TestSettings = { - general: { - vimMode: false, - }, - ui: {}, - model: { - name: 'gemini-2.5-pro', - }, - unrecognized: 'value', - }; - - const v1Settings = migrateSettingsToV1(v2Settings); - - expect(v1Settings).toEqual({ - vimMode: false, - model: 'gemini-2.5-pro', - unrecognized: 'value', - }); - }); - - it('should handle settings with different data types', () => { - const v2Settings: TestSettings = { - general: { - vimMode: false, - }, - model: { - maxSessionTurns: -1, - }, - context: { - includeDirectories: [], - }, - security: { - folderTrust: { - enabled: undefined, - }, - }, - }; - - const v1Settings = migrateSettingsToV1(v2Settings); - - expect(v1Settings).toEqual({ - vimMode: false, - maxSessionTurns: -1, - includeDirectories: [], - security: { - folderTrust: { - enabled: undefined, - }, - }, - }); - }); - - it('should preserve unrecognized top-level keys', () => { - const v2Settings: TestSettings = { - general: { - vimMode: true, - }, - customTopLevel: { - a: 1, - b: [2], - }, - anotherOne: 'hello', - }; - - const v1Settings = migrateSettingsToV1(v2Settings); - - expect(v1Settings).toEqual({ - vimMode: true, - customTopLevel: { - a: 1, - b: [2], - }, - anotherOne: 'hello', - }); - }); - - it('should handle an empty v2 settings object', () => { - const v2Settings = {}; - const v1Settings = migrateSettingsToV1(v2Settings); - expect(v1Settings).toEqual({}); - }); - - it('should correctly handle mcpServers at the top level', () => { - const v2Settings: TestSettings = { - mcpServers: { - serverA: { command: 'a' }, - }, - mcp: { - allowed: ['serverA'], - }, - }; - - const v1Settings = migrateSettingsToV1(v2Settings); - - expect(v1Settings).toEqual({ - mcpServers: { - serverA: { command: 'a' }, - }, - allowMCPServers: ['serverA'], - }); - }); - - it('should correctly migrate customWittyPhrases', () => { - const v2Settings: Partial = { - ui: { - customWittyPhrases: ['test phrase'], - }, - }; - const v1Settings = migrateSettingsToV1(v2Settings as Settings); - expect(v1Settings).toEqual({ - customWittyPhrases: ['test phrase'], - }); - }); - }); - - describe('loadEnvironment', () => { - function setup({ - isFolderTrustEnabled = true, - isWorkspaceTrustedValue = true, - }) { - delete process.env['TESTTEST']; // reset - const geminiEnvPath = path.resolve(path.join(GEMINI_DIR, '.env')); - - vi.mocked(isWorkspaceTrusted).mockReturnValue({ - isTrusted: isWorkspaceTrustedValue, - source: 'file', - }); - (mockFsExistsSync as Mock).mockImplementation((p: fs.PathLike) => - [USER_SETTINGS_PATH, geminiEnvPath].includes(p.toString()), - ); - const userSettingsContent: Settings = { - ui: { - theme: 'dark', - }, - security: { - folderTrust: { - enabled: isFolderTrustEnabled, - }, - }, - context: { - fileName: 'USER_CONTEXT.md', - }, - }; - (fs.readFileSync as Mock).mockImplementation( - (p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(userSettingsContent); - if (p === geminiEnvPath) return 'TESTTEST=1234'; - return '{}'; - }, - ); - } - - it('sets environment variables from .env files', () => { - setup({ isFolderTrustEnabled: false, isWorkspaceTrustedValue: true }); - loadEnvironment(loadSettings(MOCK_WORKSPACE_DIR).merged); - - expect(process.env['TESTTEST']).toEqual('1234'); - }); - - it('does not load env files from untrusted spaces', () => { - setup({ isFolderTrustEnabled: true, isWorkspaceTrustedValue: false }); - loadEnvironment(loadSettings(MOCK_WORKSPACE_DIR).merged); - - expect(process.env['TESTTEST']).not.toEqual('1234'); - }); - }); - - describe('needsMigration', () => { - it('should return false for an empty object', () => { - expect(needsMigration({})).toBe(false); - }); - - it('should return false for settings that are already in V2 format', () => { - const v2Settings: Partial = { - ui: { - theme: 'dark', - }, - tools: { - sandbox: true, - }, - }; - expect(needsMigration(v2Settings)).toBe(false); - }); - - it('should return true for settings with a V1 key that needs to be moved', () => { - const v1Settings = { - theme: 'dark', // v1 key - }; - expect(needsMigration(v1Settings)).toBe(true); - }); - - it('should return true for settings with a mix of V1 and V2 keys', () => { - const mixedSettings = { - theme: 'dark', // v1 key - tools: { - sandbox: true, // v2 key - }, - }; - expect(needsMigration(mixedSettings)).toBe(true); - }); - - it('should return false for settings with only V1 keys that are the same in V2', () => { - const v1Settings = { - mcpServers: {}, - telemetry: {}, - extensions: [], - }; - expect(needsMigration(v1Settings)).toBe(false); - }); - - it('should return true for settings with a mix of V1 keys that are the same in V2 and V1 keys that need moving', () => { - const v1Settings = { - mcpServers: {}, // same in v2 - theme: 'dark', // needs moving - }; - expect(needsMigration(v1Settings)).toBe(true); - }); - - it('should return false for settings with unrecognized keys', () => { - const settings = { - someUnrecognizedKey: 'value', - }; - expect(needsMigration(settings)).toBe(false); - }); - - it('should return false for settings with v2 keys and unrecognized keys', () => { - const settings = { - ui: { theme: 'dark' }, - someUnrecognizedKey: 'value', - }; - expect(needsMigration(settings)).toBe(false); - }); - }); - - describe('migrateDeprecatedSettings', () => { - let mockFsExistsSync: Mock; - let mockFsReadFileSync: Mock; - - beforeEach(() => { - vi.resetAllMocks(); - mockFsExistsSync = vi.mocked(fs.existsSync); - mockFsExistsSync.mockReturnValue(true); - mockFsReadFileSync = vi.mocked(fs.readFileSync); - mockFsReadFileSync.mockReturnValue('{}'); - vi.mocked(isWorkspaceTrusted).mockReturnValue({ - isTrusted: true, - source: undefined, - }); - }); - - afterEach(() => { - vi.restoreAllMocks(); - }); - - it('should migrate disabled extensions from user and workspace settings', () => { - const userSettingsContent = { - extensions: { - disabled: ['user-ext-1', 'shared-ext'], - }, - }; - const workspaceSettingsContent = { - extensions: { - disabled: ['workspace-ext-1', 'shared-ext'], - }, - }; - - mockFsReadFileSync.mockImplementation((p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(userSettingsContent); - if (p === MOCK_WORKSPACE_SETTINGS_PATH) - return JSON.stringify(workspaceSettingsContent); - return '{}'; - }); - - const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR); - const setValueSpy = vi.spyOn(loadedSettings, 'setValue'); - const extensionManager = new ExtensionManager({ - settings: loadedSettings.merged, - workspaceDir: MOCK_WORKSPACE_DIR, - requestConsent: vi.fn(), - requestSetting: vi.fn(), - }); - const mockDisableExtension = vi.spyOn( - extensionManager, - 'disableExtension', - ); - mockDisableExtension.mockImplementation(async () => {}); - - migrateDeprecatedSettings(loadedSettings, extensionManager); - - // Check user settings migration - expect(mockDisableExtension).toHaveBeenCalledWith( - 'user-ext-1', - SettingScope.User, - ); - expect(mockDisableExtension).toHaveBeenCalledWith( - 'shared-ext', - SettingScope.User, - ); - - // Check workspace settings migration - expect(mockDisableExtension).toHaveBeenCalledWith( - 'workspace-ext-1', - SettingScope.Workspace, - ); - expect(mockDisableExtension).toHaveBeenCalledWith( - 'shared-ext', - SettingScope.Workspace, - ); - - // Check that setValue was called to remove the deprecated setting - expect(setValueSpy).toHaveBeenCalledWith( - SettingScope.User, - 'extensions', - { - disabled: undefined, - }, - ); - expect(setValueSpy).toHaveBeenCalledWith( - SettingScope.Workspace, - 'extensions', - { - disabled: undefined, - }, - ); - }); - - it('should not do anything if there are no deprecated settings', () => { - const userSettingsContent = { - extensions: { - enabled: ['user-ext-1'], - }, - }; - const workspaceSettingsContent = { - someOtherSetting: 'value', - }; - - mockFsReadFileSync.mockImplementation((p: fs.PathOrFileDescriptor) => { - if (p === USER_SETTINGS_PATH) - return JSON.stringify(userSettingsContent); - if (p === MOCK_WORKSPACE_SETTINGS_PATH) - return JSON.stringify(workspaceSettingsContent); - return '{}'; - }); - - const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR); - const setValueSpy = vi.spyOn(loadedSettings, 'setValue'); - const extensionManager = new ExtensionManager({ - settings: loadedSettings.merged, - workspaceDir: MOCK_WORKSPACE_DIR, - requestConsent: vi.fn(), - requestSetting: vi.fn(), - }); - const mockDisableExtension = vi.spyOn( - extensionManager, - 'disableExtension', - ); - mockDisableExtension.mockImplementation(async () => {}); - - migrateDeprecatedSettings(loadedSettings, extensionManager); - - expect(mockDisableExtension).not.toHaveBeenCalled(); - expect(setValueSpy).not.toHaveBeenCalled(); - }); - }); - describe('saveSettings', () => { it('should save settings using updateSettingsFilePreservingFormat', () => { const mockUpdateSettings = vi.mocked(updateSettingsFilePreservingFormat); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 43bdc1a627..07cc686524 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -10,7 +10,6 @@ import { platform } from 'node:os'; import * as dotenv from 'dotenv'; import process from 'node:process'; import { - debugLogger, FatalConfigError, GEMINI_DIR, getErrorMessage, @@ -32,14 +31,12 @@ import { getSettingsSchema, } from './settingsSchema.js'; import { resolveEnvVarsInObject } from '../utils/envVarResolver.js'; -import { customDeepMerge, type MergeableObject } from '../utils/deepMerge.js'; +import { customDeepMerge } from '../utils/deepMerge.js'; import { updateSettingsFilePreservingFormat } from '../utils/commentJson.js'; -import type { ExtensionManager } from './extension-manager.js'; import { validateSettings, formatValidationError, } from './settings-validation.js'; -import { SettingPaths } from './settingPaths.js'; function getMergeStrategyForPath(path: string[]): MergeStrategy | undefined { let current: SettingDefinition | undefined = undefined; @@ -68,79 +65,6 @@ export const USER_SETTINGS_PATH = Storage.getGlobalSettingsPath(); export const USER_SETTINGS_DIR = path.dirname(USER_SETTINGS_PATH); export const DEFAULT_EXCLUDED_ENV_VARS = ['DEBUG', 'DEBUG_MODE']; -const MIGRATE_V2_OVERWRITE = true; - -const MIGRATION_MAP: Record = { - accessibility: 'ui.accessibility', - allowedTools: 'tools.allowed', - allowMCPServers: 'mcp.allowed', - autoAccept: 'tools.autoAccept', - autoConfigureMaxOldSpaceSize: 'advanced.autoConfigureMemory', - bugCommand: 'advanced.bugCommand', - chatCompression: 'model.compressionThreshold', - checkpointing: 'general.checkpointing', - coreTools: 'tools.core', - contextFileName: 'context.fileName', - customThemes: 'ui.customThemes', - customWittyPhrases: 'ui.customWittyPhrases', - debugKeystrokeLogging: 'general.debugKeystrokeLogging', - disableAutoUpdate: 'general.disableAutoUpdate', - disableUpdateNag: 'general.disableUpdateNag', - dnsResolutionOrder: 'advanced.dnsResolutionOrder', - enableHooks: 'tools.enableHooks', - enablePromptCompletion: 'general.enablePromptCompletion', - enforcedAuthType: 'security.auth.enforcedType', - excludeTools: 'tools.exclude', - excludeMCPServers: 'mcp.excluded', - excludedProjectEnvVars: 'advanced.excludedEnvVars', - experimentalSkills: 'experimental.skills', - extensionManagement: 'experimental.extensionManagement', - extensions: 'extensions', - fileFiltering: 'context.fileFiltering', - folderTrustFeature: 'security.folderTrust.featureEnabled', - folderTrust: 'security.folderTrust.enabled', - hasSeenIdeIntegrationNudge: 'ide.hasSeenNudge', - hideWindowTitle: 'ui.hideWindowTitle', - showStatusInTitle: 'ui.showStatusInTitle', - hideTips: 'ui.hideTips', - hideBanner: 'ui.hideBanner', - hideFooter: 'ui.hideFooter', - hideCWD: 'ui.footer.hideCWD', - hideSandboxStatus: 'ui.footer.hideSandboxStatus', - hideModelInfo: 'ui.footer.hideModelInfo', - hideContextSummary: 'ui.hideContextSummary', - showMemoryUsage: 'ui.showMemoryUsage', - showLineNumbers: 'ui.showLineNumbers', - showCitations: 'ui.showCitations', - ideMode: 'ide.enabled', - includeDirectories: 'context.includeDirectories', - loadMemoryFromIncludeDirectories: 'context.loadFromIncludeDirectories', - maxSessionTurns: 'model.maxSessionTurns', - mcpServers: 'mcpServers', - mcpServerCommand: 'mcp.serverCommand', - memoryImportFormat: 'context.importFormat', - memoryDiscoveryMaxDirs: 'context.discoveryMaxDirs', - model: 'model.name', - preferredEditor: SettingPaths.General.PreferredEditor, - retryFetchErrors: 'general.retryFetchErrors', - sandbox: 'tools.sandbox', - selectedAuthType: 'security.auth.selectedType', - enableInteractiveShell: 'tools.shell.enableInteractiveShell', - shellPager: 'tools.shell.pager', - shellShowColor: 'tools.shell.showColor', - shellInactivityTimeout: 'tools.shell.inactivityTimeout', - skipNextSpeakerCheck: 'model.skipNextSpeakerCheck', - summarizeToolOutput: 'model.summarizeToolOutput', - telemetry: 'telemetry', - theme: 'ui.theme', - toolDiscoveryCommand: 'tools.discoveryCommand', - toolCallCommand: 'tools.callCommand', - usageStatisticsEnabled: 'privacy.usageStatisticsEnabled', - useExternalAuth: 'security.auth.useExternal', - useRipgrep: 'tools.useRipgrep', - vimMode: 'general.vimMode', -}; - export function getSystemSettingsPath(): string { if (process.env['GEMINI_CLI_SYSTEM_SETTINGS_PATH']) { return process.env['GEMINI_CLI_SYSTEM_SETTINGS_PATH']; @@ -270,162 +194,6 @@ function setNestedProperty( current[lastKey] = value; } -export function needsMigration(settings: Record): boolean { - // A file needs migration if it contains any top-level key that is moved to a - // nested location in V2. - const hasV1Keys = Object.entries(MIGRATION_MAP).some(([v1Key, v2Path]) => { - if (v1Key === v2Path || !(v1Key in settings)) { - return false; - } - // If a key exists that is a V1 key and a V2 container (like 'model'), - // we need to check the type. If it's an object, it's a V2 container and not - // a V1 key that needs migration. - if ( - KNOWN_V2_CONTAINERS.has(v1Key) && - typeof settings[v1Key] === 'object' && - settings[v1Key] !== null - ) { - return false; - } - return true; - }); - - return hasV1Keys; -} - -function migrateSettingsToV2( - flatSettings: Record, -): Record | null { - if (!needsMigration(flatSettings)) { - return null; - } - - const v2Settings: Record = {}; - const flatKeys = new Set(Object.keys(flatSettings)); - - for (const [oldKey, newPath] of Object.entries(MIGRATION_MAP)) { - if (flatKeys.has(oldKey)) { - // If the key exists and is a V2 container (like 'model'), and the value is an object, - // it is likely already migrated or partially migrated. We should not move it - // to the mapped V2 path (e.g. 'model' -> 'model.name'). - // Instead, let it fall through to the "Carry over" section to be merged. - if ( - KNOWN_V2_CONTAINERS.has(oldKey) && - typeof flatSettings[oldKey] === 'object' && - flatSettings[oldKey] !== null && - !Array.isArray(flatSettings[oldKey]) - ) { - continue; - } - - setNestedProperty(v2Settings, newPath, flatSettings[oldKey]); - flatKeys.delete(oldKey); - } - } - - // Preserve mcpServers at the top level - if (flatSettings['mcpServers']) { - v2Settings['mcpServers'] = flatSettings['mcpServers']; - flatKeys.delete('mcpServers'); - } - - // Carry over any unrecognized keys - for (const remainingKey of flatKeys) { - const existingValue = v2Settings[remainingKey]; - const newValue = flatSettings[remainingKey]; - - if ( - typeof existingValue === 'object' && - existingValue !== null && - !Array.isArray(existingValue) && - typeof newValue === 'object' && - newValue !== null && - !Array.isArray(newValue) - ) { - const pathAwareGetStrategy = (path: string[]) => - getMergeStrategyForPath([remainingKey, ...path]); - v2Settings[remainingKey] = customDeepMerge( - pathAwareGetStrategy, - {}, - existingValue as MergeableObject, - newValue as MergeableObject, - ); - } else { - v2Settings[remainingKey] = newValue; - } - } - - return v2Settings; -} - -function getNestedProperty( - obj: Record, - path: string, -): unknown { - const keys = path.split('.'); - let current: unknown = obj; - for (const key of keys) { - if (typeof current !== 'object' || current === null || !(key in current)) { - return undefined; - } - current = (current as Record)[key]; - } - return current; -} - -const REVERSE_MIGRATION_MAP: Record = Object.fromEntries( - Object.entries(MIGRATION_MAP).map(([key, value]) => [value, key]), -); - -// Dynamically determine the top-level keys from the V2 settings structure. -const KNOWN_V2_CONTAINERS = new Set( - Object.values(MIGRATION_MAP).map((path) => path.split('.')[0]), -); - -export function migrateSettingsToV1( - v2Settings: Record, -): Record { - const v1Settings: Record = {}; - const v2Keys = new Set(Object.keys(v2Settings)); - - for (const [newPath, oldKey] of Object.entries(REVERSE_MIGRATION_MAP)) { - const value = getNestedProperty(v2Settings, newPath); - if (value !== undefined) { - v1Settings[oldKey] = value; - v2Keys.delete(newPath.split('.')[0]); - } - } - - // Preserve mcpServers at the top level - if (v2Settings['mcpServers']) { - v1Settings['mcpServers'] = v2Settings['mcpServers']; - v2Keys.delete('mcpServers'); - } - - // Carry over any unrecognized keys - for (const remainingKey of v2Keys) { - const value = v2Settings[remainingKey]; - if (value === undefined) { - continue; - } - - // Don't carry over empty objects that were just containers for migrated settings. - if ( - KNOWN_V2_CONTAINERS.has(remainingKey) && - typeof value === 'object' && - value !== null && - !Array.isArray(value) && - Object.keys(value).length === 0 - ) { - continue; - } - - v1Settings[remainingKey] = value; - } - - return v1Settings; -} - export function getDefaultsFromSchema( schema: SettingsSchema = getSettingsSchema(), ): Settings { @@ -478,7 +246,6 @@ export class LoadedSettings { user: SettingsFile, workspace: SettingsFile, isTrusted: boolean, - migratedInMemoryScopes: Set, errors: SettingsError[] = [], ) { this.system = system; @@ -486,7 +253,6 @@ export class LoadedSettings { this.user = user; this.workspace = workspace; this.isTrusted = isTrusted; - this.migratedInMemoryScopes = migratedInMemoryScopes; this.errors = errors; this._merged = this.computeMergedSettings(); } @@ -496,7 +262,6 @@ export class LoadedSettings { readonly user: SettingsFile; readonly workspace: SettingsFile; readonly isTrusted: boolean; - readonly migratedInMemoryScopes: Set; readonly errors: SettingsError[]; private _merged: Settings; @@ -690,7 +455,6 @@ export function loadSettings( const settingsErrors: SettingsError[] = []; const systemSettingsPath = getSystemSettingsPath(); const systemDefaultsPath = getSystemDefaultsPath(); - const migratedInMemoryScopes = new Set(); // Resolve paths to their canonical representation to handle symlinks const resolvedWorkspaceDir = path.resolve(workspaceDir); @@ -711,10 +475,7 @@ export function loadSettings( workspaceDir, ).getWorkspaceSettingsPath(); - const loadAndMigrate = ( - filePath: string, - scope: SettingScope, - ): { settings: Settings; rawJson?: string } => { + const load = (filePath: string): { settings: Settings; rawJson?: string } => { try { if (fs.existsSync(filePath)) { const content = fs.readFileSync(filePath, 'utf-8'); @@ -733,33 +494,9 @@ export function loadSettings( return { settings: {} }; } - let settingsObject = rawSettings as Record; - if (needsMigration(settingsObject)) { - const migratedSettings = migrateSettingsToV2(settingsObject); - if (migratedSettings) { - if (MIGRATE_V2_OVERWRITE) { - try { - fs.renameSync(filePath, `${filePath}.orig`); - fs.writeFileSync( - filePath, - JSON.stringify(migratedSettings, null, 2), - 'utf-8', - ); - } catch (e) { - coreEvents.emitFeedback( - 'error', - 'Failed to migrate settings file.', - e, - ); - } - } else { - migratedInMemoryScopes.add(scope); - } - settingsObject = migratedSettings; - } - } + const settingsObject = rawSettings as Record; - // Validate settings structure with Zod after migration + // Validate settings structure with Zod const validationResult = validateSettings(settingsObject); if (!validationResult.success && validationResult.error) { const errorMessage = formatValidationError( @@ -785,22 +522,16 @@ export function loadSettings( return { settings: {} }; }; - const systemResult = loadAndMigrate(systemSettingsPath, SettingScope.System); - const systemDefaultsResult = loadAndMigrate( - systemDefaultsPath, - SettingScope.SystemDefaults, - ); - const userResult = loadAndMigrate(USER_SETTINGS_PATH, SettingScope.User); + const systemResult = load(systemSettingsPath); + const systemDefaultsResult = load(systemDefaultsPath); + const userResult = load(USER_SETTINGS_PATH); let workspaceResult: { settings: Settings; rawJson?: string } = { settings: {} as Settings, rawJson: undefined, }; if (realWorkspaceDir !== realHomeDir) { - workspaceResult = loadAndMigrate( - workspaceSettingsPath, - SettingScope.Workspace, - ); + workspaceResult = load(workspaceSettingsPath); } const systemOriginalSettings = structuredClone(systemResult.settings); @@ -888,37 +619,10 @@ export function loadSettings( rawJson: workspaceResult.rawJson, }, isTrusted, - migratedInMemoryScopes, settingsErrors, ); } -export function migrateDeprecatedSettings( - loadedSettings: LoadedSettings, - extensionManager: ExtensionManager, -): void { - const processScope = (scope: LoadableSettingScope) => { - const settings = loadedSettings.forScope(scope).settings; - if (settings.extensions?.disabled) { - debugLogger.log( - `Migrating deprecated extensions.disabled settings from ${scope} settings...`, - ); - for (const extension of settings.extensions.disabled ?? []) { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - extensionManager.disableExtension(extension, scope); - } - - const newExtensionsValue = { ...settings.extensions }; - newExtensionsValue.disabled = undefined; - - loadedSettings.setValue(scope, 'extensions', newExtensionsValue); - } - }; - - processScope(SettingScope.User); - processScope(SettingScope.Workspace); -} - export function saveSettings(settingsFile: SettingsFile): void { try { // Ensure the directory exists @@ -927,12 +631,7 @@ export function saveSettings(settingsFile: SettingsFile): void { fs.mkdirSync(dirPath, { recursive: true }); } - let settingsToSave = settingsFile.originalSettings; - if (!MIGRATE_V2_OVERWRITE) { - settingsToSave = migrateSettingsToV1( - settingsToSave as Record, - ) as Settings; - } + const settingsToSave = settingsFile.originalSettings; // Use the format-preserving update function updateSettingsFilePreservingFormat( diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 5dac29630b..32284b132d 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -20,11 +20,7 @@ import { loadTrustedFolders, type TrustedFoldersError, } from './config/trustedFolders.js'; -import { - loadSettings, - migrateDeprecatedSettings, - SettingScope, -} from './config/settings.js'; +import { loadSettings, SettingScope } from './config/settings.js'; import { getStartupWarnings } from './utils/startupWarnings.js'; import { getUserStartupWarnings } from './utils/userStartupWarnings.js'; import { ConsolePatcher } from './ui/utils/ConsolePatcher.js'; @@ -93,9 +89,7 @@ import { } from './utils/relaunch.js'; import { loadSandboxConfig } from './config/sandboxConfig.js'; import { deleteSession, listSessions } from './utils/sessions.js'; -import { ExtensionManager } from './config/extension-manager.js'; import { createPolicyUpdater } from './config/policy.js'; -import { requestConsentNonInteractive } from './config/extensions/consent.js'; import { ScrollProvider } from './ui/contexts/ScrollProvider.js'; import { isAlternateBufferEnabled } from './ui/hooks/useAlternateBuffer.js'; @@ -315,19 +309,6 @@ export async function main() { ); }); - const migrateHandle = startupProfiler.start('migrate_settings'); - migrateDeprecatedSettings( - settings, - // Temporary extension manager only used during this non-interactive UI phase. - new ExtensionManager({ - workspaceDir: process.cwd(), - settings: settings.merged, - enabledExtensionOverrides: [], - requestConsent: requestConsentNonInteractive, - requestSetting: null, - }), - ); - migrateHandle?.end(); await cleanupCheckpoints(); const parseArgsHandle = startupProfiler.start('parse_arguments'); diff --git a/packages/cli/src/test-utils/render.tsx b/packages/cli/src/test-utils/render.tsx index d02b5af8ae..e083918683 100644 --- a/packages/cli/src/test-utils/render.tsx +++ b/packages/cli/src/test-utils/render.tsx @@ -109,7 +109,7 @@ export const mockSettings = new LoadedSettings( { path: '', settings: {}, originalSettings: {} }, { path: '', settings: {}, originalSettings: {} }, true, - new Set(), + [], ); export const createMockSettings = ( @@ -122,7 +122,7 @@ export const createMockSettings = ( { path: '', settings, originalSettings: settings }, { path: '', settings: {}, originalSettings: {} }, true, - new Set(), + [], ); }; diff --git a/packages/cli/src/ui/App.test.tsx b/packages/cli/src/ui/App.test.tsx index 64f42fae11..1a8851685a 100644 --- a/packages/cli/src/ui/App.test.tsx +++ b/packages/cli/src/ui/App.test.tsx @@ -14,11 +14,7 @@ import { StreamingState } from './types.js'; import { ConfigContext } from './contexts/ConfigContext.js'; import { AppContext, type AppState } from './contexts/AppContext.js'; import { SettingsContext } from './contexts/SettingsContext.js'; -import { - type SettingScope, - LoadedSettings, - type SettingsFile, -} from '../config/settings.js'; +import { LoadedSettings, type SettingsFile } from '../config/settings.js'; vi.mock('ink', async (importOriginal) => { const original = await importOriginal(); @@ -92,7 +88,7 @@ describe('App', () => { mockSettingsFile, mockSettingsFile, true, - new Set(), + [], ); const mockAppState: AppState = { diff --git a/packages/cli/src/ui/components/SettingsDialog.test.tsx b/packages/cli/src/ui/components/SettingsDialog.test.tsx index d33130744a..78bd5a3917 100644 --- a/packages/cli/src/ui/components/SettingsDialog.test.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.test.tsx @@ -104,7 +104,7 @@ const createMockSettings = ( path: '/workspace/settings.json', }, true, - new Set(), + [], ); vi.mock('../../config/settingsSchema.js', async (importOriginal) => { diff --git a/packages/cli/src/ui/components/ThemeDialog.test.tsx b/packages/cli/src/ui/components/ThemeDialog.test.tsx index ef2306c122..bcfeb5a9c9 100644 --- a/packages/cli/src/ui/components/ThemeDialog.test.tsx +++ b/packages/cli/src/ui/components/ThemeDialog.test.tsx @@ -51,7 +51,7 @@ const createMockSettings = ( path: '/workspace/settings.json', }, true, - new Set(), + [], ); describe('ThemeDialog Snapshots', () => { diff --git a/packages/cli/src/ui/utils/CodeColorizer.test.tsx b/packages/cli/src/ui/utils/CodeColorizer.test.tsx index 641567c00b..94913c88bf 100644 --- a/packages/cli/src/ui/utils/CodeColorizer.test.tsx +++ b/packages/cli/src/ui/utils/CodeColorizer.test.tsx @@ -24,7 +24,7 @@ describe('colorizeCode', () => { }, { path: '', settings: {}, originalSettings: {} }, true, - new Set(), + [], ); const result = colorizeCode({ diff --git a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx index 927a69e154..bfb81e6519 100644 --- a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx +++ b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx @@ -198,7 +198,7 @@ Another paragraph. }, { path: '', settings: {}, originalSettings: {} }, true, - new Set(), + [], ); const { lastFrame } = renderWithProviders(