diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index 9db17b7d3..a8ebcffe5 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -55,6 +55,7 @@ import { disableExtension } from './extension.js'; // These imports will get the versions from the vi.mock('./settings.js', ...) factory. import { + getSettingsWarnings, loadSettings, USER_SETTINGS_PATH, // This IS the mocked path. getSystemSettingsPath, @@ -418,6 +419,84 @@ describe('Settings Loading and Merging', () => { }); }); + it('should warn about deprecated legacy keys in a v2 settings file', () => { + (mockFsExistsSync as Mock).mockImplementation( + (p: fs.PathLike) => p === USER_SETTINGS_PATH, + ); + const userSettingsContent = { + [SETTINGS_VERSION_KEY]: SETTINGS_VERSION, + usageStatisticsEnabled: false, + }; + (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(getSettingsWarnings(settings)).toEqual( + expect.arrayContaining([ + expect.stringContaining( + "Deprecated setting 'usageStatisticsEnabled'", + ), + ]), + ); + expect(getSettingsWarnings(settings)).toEqual( + expect.arrayContaining([ + expect.stringContaining("'privacy.usageStatisticsEnabled'"), + ]), + ); + }); + + it('should warn about unknown top-level keys in a v2 settings file', () => { + (mockFsExistsSync as Mock).mockImplementation( + (p: fs.PathLike) => p === USER_SETTINGS_PATH, + ); + const userSettingsContent = { + [SETTINGS_VERSION_KEY]: SETTINGS_VERSION, + someUnknownKey: 'value', + }; + (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(getSettingsWarnings(settings)).toEqual( + expect.arrayContaining([ + expect.stringContaining("Unknown setting 'someUnknownKey'"), + ]), + ); + }); + + it('should not warn for valid v2 container keys', () => { + (mockFsExistsSync as Mock).mockImplementation( + (p: fs.PathLike) => p === USER_SETTINGS_PATH, + ); + const userSettingsContent = { + [SETTINGS_VERSION_KEY]: SETTINGS_VERSION, + model: { name: 'qwen-coder' }, + }; + (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(getSettingsWarnings(settings)).toEqual([]); + }); + it('should rewrite allowedTools to tools.allowed during migration', () => { (mockFsExistsSync as Mock).mockImplementation( (p: fs.PathLike) => p === USER_SETTINGS_PATH, diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index ae29074b2..5c8fec3cb 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -344,6 +344,97 @@ const KNOWN_V2_CONTAINERS = new Set( Object.values(MIGRATION_MAP).map((path) => path.split('.')[0]), ); +function getSettingsFileKeyWarnings( + settings: Record, + settingsFilePath: string, +): string[] { + const version = settings[SETTINGS_VERSION_KEY]; + if (typeof version !== 'number' || version < SETTINGS_VERSION) { + return []; + } + + const warnings: string[] = []; + const deprecatedKeys = new Set(); + + // Deprecated keys (V1 top-level keys that moved to a nested V2 path). + for (const [oldKey, newPath] of Object.entries(MIGRATION_MAP)) { + if (oldKey === newPath) { + continue; + } + if (!(oldKey in settings)) { + continue; + } + + const oldValue = settings[oldKey]; + + // If this key is a V2 container (like 'model') and it's already an object, + // it's likely already in V2 format. Don't warn. + if ( + KNOWN_V2_CONTAINERS.has(oldKey) && + typeof oldValue === 'object' && + oldValue !== null && + !Array.isArray(oldValue) + ) { + continue; + } + + deprecatedKeys.add(oldKey); + warnings.push( + `⚠️ Warning: Deprecated setting '${oldKey}' in ${settingsFilePath}. Please use '${newPath}' instead.`, + ); + } + + // Unknown top-level keys. + const schemaKeys = new Set(Object.keys(getSettingsSchema())); + for (const key of Object.keys(settings)) { + if (key === SETTINGS_VERSION_KEY) { + continue; + } + if (deprecatedKeys.has(key)) { + continue; + } + if (schemaKeys.has(key)) { + continue; + } + + warnings.push( + `⚠️ Warning: Unknown setting '${key}' in ${settingsFilePath}. This setting will be ignored.`, + ); + } + + return warnings; +} + +/** + * Collects warnings for deprecated and unknown settings keys. + * + * For `$version: 2` settings files, we do not apply implicit migrations. + * Instead, we surface actionable, de-duplicated warnings in the terminal UI. + */ +export function getSettingsWarnings(loadedSettings: LoadedSettings): string[] { + const warningSet = new Set(); + + for (const scope of [SettingScope.User, SettingScope.Workspace]) { + const settingsFile = loadedSettings.forScope(scope); + if (settingsFile.rawJson === undefined) { + continue; // File not present / not loaded. + } + const settingsObject = settingsFile.originalSettings as unknown as Record< + string, + unknown + >; + + for (const warning of getSettingsFileKeyWarnings( + settingsObject, + settingsFile.path, + )) { + warningSet.add(warning); + } + } + + return [...warningSet]; +} + export function migrateSettingsToV1( v2Settings: Record, ): Record { diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index b05f12453..4591cf1d1 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -17,7 +17,11 @@ import * as cliConfig from './config/config.js'; import { loadCliConfig, parseArguments } from './config/config.js'; import { ExtensionStorage, loadExtensions } from './config/extension.js'; import type { DnsResolutionOrder, LoadedSettings } from './config/settings.js'; -import { loadSettings, migrateDeprecatedSettings } from './config/settings.js'; +import { + getSettingsWarnings, + loadSettings, + migrateDeprecatedSettings, +} from './config/settings.js'; import { initializeApp, type InitializationResult, @@ -400,12 +404,15 @@ export async function main() { let input = config.getQuestion(); const startupWarnings = [ - ...(await getStartupWarnings()), - ...(await getUserStartupWarnings({ - workspaceRoot: process.cwd(), - useRipgrep: settings.merged.tools?.useRipgrep ?? true, - useBuiltinRipgrep: settings.merged.tools?.useBuiltinRipgrep ?? true, - })), + ...new Set([ + ...(await getStartupWarnings()), + ...(await getUserStartupWarnings({ + workspaceRoot: process.cwd(), + useRipgrep: settings.merged.tools?.useRipgrep ?? true, + useBuiltinRipgrep: settings.merged.tools?.useBuiltinRipgrep ?? true, + })), + ...getSettingsWarnings(settings), + ]), ]; // Render UI, passing necessary config values. Check that there is no command line question.