From 8c56b612fb3564df79fa116bbd6bd08f0cb28a59 Mon Sep 17 00:00:00 2001 From: liqoingyu Date: Wed, 7 Jan 2026 19:49:40 +0800 Subject: [PATCH 1/2] fix(cli): warn on deprecated/unknown settings keys --- packages/cli/src/config/settings.test.ts | 79 ++++++++++++++++++++ packages/cli/src/config/settings.ts | 91 ++++++++++++++++++++++++ packages/cli/src/gemini.tsx | 21 ++++-- 3 files changed, 184 insertions(+), 7 deletions(-) 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. From 5f8e1ebc94b109de1dbd3c775157ae6be57109bc Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Mon, 12 Jan 2026 14:29:40 +0800 Subject: [PATCH 2/2] chore(settings): update legacy settings alias implementation and tests Co-authored-by: Qwen-Coder --- packages/cli/src/config/settings.test.ts | 8 +++++--- packages/cli/src/config/settings.ts | 14 +++++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index a8ebcffe5..6549f6f71 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -419,7 +419,7 @@ describe('Settings Loading and Merging', () => { }); }); - it('should warn about deprecated legacy keys in a v2 settings file', () => { + it('should warn about ignored legacy keys in a v2 settings file', () => { (mockFsExistsSync as Mock).mockImplementation( (p: fs.PathLike) => p === USER_SETTINGS_PATH, ); @@ -440,7 +440,7 @@ describe('Settings Loading and Merging', () => { expect(getSettingsWarnings(settings)).toEqual( expect.arrayContaining([ expect.stringContaining( - "Deprecated setting 'usageStatisticsEnabled'", + "Legacy setting 'usageStatisticsEnabled' will be ignored", ), ]), ); @@ -471,7 +471,9 @@ describe('Settings Loading and Merging', () => { expect(getSettingsWarnings(settings)).toEqual( expect.arrayContaining([ - expect.stringContaining("Unknown setting 'someUnknownKey'"), + expect.stringContaining( + "Unknown setting 'someUnknownKey' will be ignored", + ), ]), ); }); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 5c8fec3cb..c9b845cd8 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -354,9 +354,9 @@ function getSettingsFileKeyWarnings( } const warnings: string[] = []; - const deprecatedKeys = new Set(); + const ignoredLegacyKeys = new Set(); - // Deprecated keys (V1 top-level keys that moved to a nested V2 path). + // Ignored legacy 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; @@ -378,9 +378,9 @@ function getSettingsFileKeyWarnings( continue; } - deprecatedKeys.add(oldKey); + ignoredLegacyKeys.add(oldKey); warnings.push( - `⚠️ Warning: Deprecated setting '${oldKey}' in ${settingsFilePath}. Please use '${newPath}' instead.`, + `⚠️ Legacy setting '${oldKey}' will be ignored in ${settingsFilePath}. Please use '${newPath}' instead.`, ); } @@ -390,7 +390,7 @@ function getSettingsFileKeyWarnings( if (key === SETTINGS_VERSION_KEY) { continue; } - if (deprecatedKeys.has(key)) { + if (ignoredLegacyKeys.has(key)) { continue; } if (schemaKeys.has(key)) { @@ -398,7 +398,7 @@ function getSettingsFileKeyWarnings( } warnings.push( - `⚠️ Warning: Unknown setting '${key}' in ${settingsFilePath}. This setting will be ignored.`, + `⚠️ Unknown setting '${key}' will be ignored in ${settingsFilePath}.`, ); } @@ -406,7 +406,7 @@ function getSettingsFileKeyWarnings( } /** - * Collects warnings for deprecated and unknown settings keys. + * Collects warnings for ignored legacy 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.