diff --git a/integration-tests/settings-migration.test.ts b/integration-tests/settings-migration.test.ts new file mode 100644 index 000000000..796fed02c --- /dev/null +++ b/integration-tests/settings-migration.test.ts @@ -0,0 +1,527 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { TestRig } from './test-helper.js'; +import { writeFileSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +/** + * Integration tests for settings migration chain (V1 -> V2 -> V3) + * + * These tests verify that: + * 1. V1 settings are automatically migrated to V3 on CLI startup + * 2. V2 settings are automatically migrated to V3 on CLI startup + * 3. V3 settings remain unchanged + * 4. Migration is idempotent (running multiple times produces same result) + */ +describe('settings-migration', () => { + let rig: TestRig; + + beforeEach(() => { + rig = new TestRig(); + }); + + afterEach(async () => { + await rig.cleanup(); + }); + + /** + * Sample V1 settings (flat structure, no $version field) + * This represents settings from early versions of the CLI + */ + const createV1Settings = () => ({ + theme: 'dark', + model: 'gemini', + autoAccept: true, + hideTips: false, + vimMode: true, + checkpointing: true, + disableAutoUpdate: true, + disableLoadingPhrases: true, + mcpServers: { + fetch: { + command: 'node', + args: ['fetch-server.js'], + }, + }, + customUserSetting: 'preserved-value', + }); + + /** + * Sample V2 settings (nested structure with $version: 2, disable* booleans) + */ + const createV2Settings = () => ({ + $version: 2, + ui: { + theme: 'light', + accessibility: { + disableLoadingPhrases: false, + }, + }, + general: { + disableAutoUpdate: false, + disableUpdateNag: false, + checkpointing: false, + }, + model: { + name: 'claude', + }, + context: { + fileFiltering: { + disableFuzzySearch: true, + }, + }, + mcpServers: {}, + }); + + /** + * Sample V3 settings (current format, should not be modified) + */ + const createV3Settings = () => ({ + $version: 3, + ui: { + theme: 'system', + accessibility: { + enableLoadingPhrases: true, + }, + }, + general: { + enableAutoUpdate: true, + }, + model: { + name: 'gemini-2.0', + }, + }); + + /** + * Helper to write settings file for an existing test rig. + * This overwrites the settings file created by rig.setup(). + */ + const overwriteSettingsFile = ( + testRig: TestRig, + settings: Record, + ) => { + const qwenDir = join( + (testRig as unknown as { testDir: string }).testDir, + '.qwen', + ); + writeFileSync( + join(qwenDir, 'settings.json'), + JSON.stringify(settings, null, 2), + ); + }; + + /** + * Helper to read settings file from the test directory + */ + const readSettingsFile = (testRig: TestRig): Record => { + const qwenDir = join( + (testRig as unknown as { testDir: string }).testDir, + '.qwen', + ); + const content = readFileSync(join(qwenDir, 'settings.json'), 'utf-8'); + return JSON.parse(content) as Record; + }; + + describe('V1 settings migration', () => { + it('should migrate V1 settings to V3 on CLI startup', async () => { + rig.setup('v1-to-v3-migration'); + + // Write V1 settings directly (overwrites the one created by setup) + overwriteSettingsFile(rig, createV1Settings()); + + // Run CLI with --help to trigger migration without API calls + // We expect this to fail due to missing API key, but migration should still occur + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail, we just need the settings file to be processed + } + + // Read migrated settings + const migratedSettings = readSettingsFile(rig); + + // Verify migration to V3 + expect(migratedSettings['$version']).toBe(3); + expect(migratedSettings['ui']).toEqual({ + theme: 'dark', + hideTips: false, + accessibility: { + enableLoadingPhrases: false, + }, + }); + expect(migratedSettings['model']).toEqual({ name: 'gemini' }); + expect(migratedSettings['tools']).toEqual({ autoAccept: true }); + expect(migratedSettings['general']).toEqual({ + vimMode: true, + checkpointing: true, + enableAutoUpdate: false, + }); + expect(migratedSettings['mcpServers']).toEqual({ + fetch: { + command: 'node', + args: ['fetch-server.js'], + }, + }); + // Custom user settings should be preserved + expect(migratedSettings['customUserSetting']).toBe('preserved-value'); + }); + + it('should handle V1 settings with partial V2 structure', async () => { + rig.setup('v1-partial-migration'); + + // V1 settings that might have been partially migrated + const partialV1Settings = { + theme: 'dark', + model: 'gemini', + // Some V2-like nested structure but no $version + ui: { + hideWindowTitle: true, + }, + }; + + overwriteSettingsFile(rig, partialV1Settings); + + // Run CLI with --help to trigger migration without API calls + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail + } + + // Read migrated settings + const migratedSettings = readSettingsFile(rig); + + // Should be migrated to V3 + expect(migratedSettings['$version']).toBe(3); + }); + }); + + describe('V2 settings migration', () => { + it('should migrate V2 settings to V3 on CLI startup', async () => { + rig.setup('v2-to-v3-migration'); + + // Write V2 settings directly (overwrites the one created by setup) + overwriteSettingsFile(rig, createV2Settings()); + + // Run CLI with --help to trigger migration without API calls + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail + } + + // Read migrated settings + const migratedSettings = readSettingsFile(rig); + + // Verify migration to V3 + expect(migratedSettings['$version']).toBe(3); + + // Verify disable* -> enable* conversion with inversion + expect( + ( + (migratedSettings['ui'] as Record)?.[ + 'accessibility' + ] as Record + )?.['enableLoadingPhrases'], + ).toBe(true); + expect( + (migratedSettings['general'] as Record)?.[ + 'enableAutoUpdate' + ], + ).toBe(true); + expect( + ( + (migratedSettings['context'] as Record)?.[ + 'fileFiltering' + ] as Record + )?.['enableFuzzySearch'], + ).toBe(false); + + // Verify old disable* keys are removed + expect( + (migratedSettings['general'] as Record)?.[ + 'disableAutoUpdate' + ], + ).toBeUndefined(); + expect( + (migratedSettings['general'] as Record)?.[ + 'disableUpdateNag' + ], + ).toBeUndefined(); + expect( + ( + (migratedSettings['ui'] as Record)?.[ + 'accessibility' + ] as Record + )?.['disableLoadingPhrases'], + ).toBeUndefined(); + expect( + ( + (migratedSettings['context'] as Record)?.[ + 'fileFiltering' + ] as Record + )?.['disableFuzzySearch'], + ).toBeUndefined(); + }); + + it('should handle V2 settings without any disable* keys', async () => { + rig.setup('v2-clean-migration'); + + const cleanV2Settings = { + $version: 2, + ui: { + theme: 'dark', + }, + model: { + name: 'gemini', + }, + }; + + overwriteSettingsFile(rig, cleanV2Settings); + + // Run CLI with --help to trigger migration without API calls + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail + } + + // Read migrated settings + const migratedSettings = readSettingsFile(rig); + + // Should be updated to V3 version + expect(migratedSettings['$version']).toBe(3); + // Other settings should remain unchanged + expect(migratedSettings['ui']).toEqual({ theme: 'dark' }); + expect(migratedSettings['model']).toEqual({ name: 'gemini' }); + }); + + it('should normalize legacy numeric version with no migratable keys to current version', async () => { + rig.setup('legacy-version-normalization'); + + const legacyVersionWithoutMigratableKeys = { + $version: 1, + customOnlyKey: 'value', + }; + + overwriteSettingsFile(rig, legacyVersionWithoutMigratableKeys); + + // Run CLI with --help to trigger settings load/write path + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail + } + + const migratedSettings = readSettingsFile(rig); + + // Version metadata should still be normalized to current version + expect(migratedSettings['$version']).toBe(3); + // Existing user content should be preserved + expect(migratedSettings['customOnlyKey']).toBe('value'); + }); + }); + + describe('V3 settings handling', () => { + it('should not modify existing V3 settings', async () => { + rig.setup('v3-no-migration'); + + const v3Settings = createV3Settings(); + overwriteSettingsFile(rig, v3Settings); + + // Run CLI with --help to trigger migration without API calls + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail + } + + // Read settings + const finalSettings = readSettingsFile(rig); + + // Should remain V3 and unchanged + expect(finalSettings['$version']).toBe(3); + expect(finalSettings).toEqual(v3Settings); + }); + }); + + describe('Migration idempotency', () => { + it('should produce consistent results when run multiple times on V1 settings', async () => { + rig.setup('v1-idempotency'); + + overwriteSettingsFile(rig, createV1Settings()); + + // Run CLI multiple times with --help + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail + } + const firstRunSettings = readSettingsFile(rig); + + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail + } + const secondRunSettings = readSettingsFile(rig); + + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail + } + const thirdRunSettings = readSettingsFile(rig); + + // All runs should produce identical results + expect(secondRunSettings).toEqual(firstRunSettings); + expect(thirdRunSettings).toEqual(firstRunSettings); + }); + + it('should produce consistent results when run multiple times on V2 settings', async () => { + rig.setup('v2-idempotency'); + + overwriteSettingsFile(rig, createV2Settings()); + + // Run CLI multiple times with --help + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail + } + const firstRunSettings = readSettingsFile(rig); + + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail + } + const secondRunSettings = readSettingsFile(rig); + + // Both runs should produce identical results + expect(secondRunSettings).toEqual(firstRunSettings); + }); + }); + + describe('Complex migration scenarios', () => { + it('should handle V2 settings with multiple disable* keys affecting the same enable* key', async () => { + rig.setup('v2-consolidated-booleans'); + + const v2SettingsWithMultipleDisables = { + $version: 2, + general: { + // Both disableAutoUpdate and disableUpdateNag should consolidate to enableAutoUpdate + disableAutoUpdate: true, // This should make enableAutoUpdate = false + disableUpdateNag: false, + checkpointing: true, + }, + }; + + overwriteSettingsFile(rig, v2SettingsWithMultipleDisables); + + // Run CLI with --help to trigger migration without API calls + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail + } + + // Read migrated settings + const migratedSettings = readSettingsFile(rig); + + // enableAutoUpdate should be false because disableAutoUpdate was true + expect( + (migratedSettings['general'] as Record)?.[ + 'enableAutoUpdate' + ], + ).toBe(false); + // Old keys should be removed + expect( + (migratedSettings['general'] as Record)?.[ + 'disableAutoUpdate' + ], + ).toBeUndefined(); + expect( + (migratedSettings['general'] as Record)?.[ + 'disableUpdateNag' + ], + ).toBeUndefined(); + }); + + it('should preserve custom user settings during full migration chain', async () => { + rig.setup('preserve-custom-settings'); + + const v1SettingsWithCustomKeys = { + theme: 'dark', + model: 'gemini', + myCustomKey: 'customValue', + anotherCustomSetting: { nested: true }, + }; + + overwriteSettingsFile(rig, v1SettingsWithCustomKeys); + + // Run CLI with --help to trigger migration without API calls + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail + } + + // Read migrated settings + const migratedSettings = readSettingsFile(rig); + + // Custom keys should be preserved + expect(migratedSettings['myCustomKey']).toBe('customValue'); + expect(migratedSettings['anotherCustomSetting']).toEqual({ + nested: true, + }); + }); + + it('should handle model.generationConfig.disableCacheControl migration', async () => { + rig.setup('v2-cache-control-migration'); + + const v2SettingsWithCacheControl = { + $version: 2, + model: { + name: 'gemini', + generationConfig: { + disableCacheControl: true, + }, + }, + }; + + overwriteSettingsFile(rig, v2SettingsWithCacheControl); + + // Run CLI with --help to trigger migration without API calls + try { + await rig.runCommand(['--help']); + } catch { + // Expected to potentially fail + } + + // Read migrated settings + const migratedSettings = readSettingsFile(rig); + + // disableCacheControl should be migrated to enableCacheControl with inverted value + expect( + ( + (migratedSettings['model'] as Record)?.[ + 'generationConfig' + ] as Record + )?.['enableCacheControl'], + ).toBe(false); + expect( + ( + (migratedSettings['model'] as Record)?.[ + 'generationConfig' + ] as Record + )?.['disableCacheControl'], + ).toBeUndefined(); + }); + }); +}); diff --git a/packages/cli/src/config/migration/index.test.ts b/packages/cli/src/config/migration/index.test.ts new file mode 100644 index 000000000..52bae237e --- /dev/null +++ b/packages/cli/src/config/migration/index.test.ts @@ -0,0 +1,383 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { + runMigrations, + needsMigration, + ALL_MIGRATIONS, + MigrationScheduler, +} from './index.js'; +import { SETTINGS_VERSION } from '../settings.js'; + +describe('Migration Framework Integration', () => { + describe('runMigrations', () => { + it('should migrate V1 settings to V3', () => { + const v1Settings = { + theme: 'dark', + model: 'gemini', + disableAutoUpdate: true, + disableLoadingPhrases: false, + }; + + const result = runMigrations(v1Settings, 'user'); + + expect(result.finalVersion).toBe(3); + expect(result.executedMigrations).toHaveLength(2); + expect(result.executedMigrations[0]).toEqual({ + fromVersion: 1, + toVersion: 2, + }); + expect(result.executedMigrations[1]).toEqual({ + fromVersion: 2, + toVersion: 3, + }); + + // Check V2 structure was created + const settings = result.settings as Record; + expect(settings['$version']).toBe(3); + expect(settings['ui']).toEqual({ + theme: 'dark', + accessibility: { enableLoadingPhrases: true }, + }); + expect(settings['model']).toEqual({ name: 'gemini' }); + + // Check disableAutoUpdate was inverted to enableAutoUpdate: false + expect( + (settings['general'] as Record)['enableAutoUpdate'], + ).toBe(false); + }); + + it('should migrate V2 settings to V3', () => { + const v2Settings = { + $version: 2, + ui: { theme: 'light' }, + general: { disableAutoUpdate: false }, + }; + + const result = runMigrations(v2Settings, 'user'); + + expect(result.finalVersion).toBe(3); + expect(result.executedMigrations).toHaveLength(1); + expect(result.executedMigrations[0]).toEqual({ + fromVersion: 2, + toVersion: 3, + }); + + const settings = result.settings as Record; + expect(settings['$version']).toBe(3); + expect( + (settings['general'] as Record)['enableAutoUpdate'], + ).toBe(true); + expect( + (settings['general'] as Record)['disableAutoUpdate'], + ).toBeUndefined(); + }); + + it('should not modify V3 settings', () => { + const v3Settings = { + $version: 3, + ui: { theme: 'dark' }, + general: { enableAutoUpdate: true }, + }; + + const result = runMigrations(v3Settings, 'user'); + + expect(result.finalVersion).toBe(3); + expect(result.executedMigrations).toHaveLength(0); + expect(result.settings).toEqual(v3Settings); + }); + + it('should be idempotent', () => { + const v1Settings = { + theme: 'dark', + disableAutoUpdate: true, + }; + + const result1 = runMigrations(v1Settings, 'user'); + const result2 = runMigrations(result1.settings, 'user'); + + expect(result1.executedMigrations).toHaveLength(2); + expect(result2.executedMigrations).toHaveLength(0); + expect(result1.finalVersion).toBe(result2.finalVersion); + }); + }); + + describe('needsMigration', () => { + it('should return true for V1 settings', () => { + const v1Settings = { + theme: 'dark', + model: 'gemini', + }; + + expect(needsMigration(v1Settings)).toBe(true); + }); + + it('should return true for V2 settings with deprecated keys', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: true }, + }; + + expect(needsMigration(v2Settings)).toBe(true); + }); + + it('should return true for V2 settings without deprecated keys', () => { + const cleanV2Settings = { + $version: 2, + ui: { theme: 'dark' }, + }; + + // V2 settings should be migrated to V3 to update the version number + expect(needsMigration(cleanV2Settings)).toBe(true); + }); + + it('should return false for V3 settings', () => { + const v3Settings = { + $version: 3, + general: { enableAutoUpdate: true }, + }; + + expect(needsMigration(v3Settings)).toBe(false); + }); + + it('should return false for legacy numeric version when no migration can execute', () => { + const legacyButUnknownSettings = { + $version: 1, + customOnlyKey: 'value', + }; + + expect(needsMigration(legacyButUnknownSettings)).toBe(false); + }); + }); + + describe('ALL_MIGRATIONS', () => { + it('should contain all migrations in order', () => { + expect(ALL_MIGRATIONS).toHaveLength(2); + + expect(ALL_MIGRATIONS[0].fromVersion).toBe(1); + expect(ALL_MIGRATIONS[0].toVersion).toBe(2); + + expect(ALL_MIGRATIONS[1].fromVersion).toBe(2); + expect(ALL_MIGRATIONS[1].toVersion).toBe(3); + }); + }); + + describe('MigrationScheduler with all migrations', () => { + it('should execute full migration chain', () => { + const scheduler = new MigrationScheduler([...ALL_MIGRATIONS], 'user'); + + const v1Settings = { + theme: 'dark', + disableAutoUpdate: true, + disableLoadingPhrases: true, + }; + + const result = scheduler.migrate(v1Settings); + + expect(result.executedMigrations).toHaveLength(2); + + const settings = result.settings as Record; + expect(settings['$version']).toBe(3); + expect((settings['ui'] as Record)['theme']).toBe('dark'); + expect( + (settings['general'] as Record)['enableAutoUpdate'], + ).toBe(false); + expect( + ( + (settings['ui'] as Record)[ + 'accessibility' + ] as Record + )['enableLoadingPhrases'], + ).toBe(false); + }); + }); + + describe('needsMigration and runMigrations consistency', () => { + it('needsMigration should return true when runMigrations would execute migrations', () => { + const v1Settings = { + theme: 'dark', + disableAutoUpdate: true, + }; + + // needsMigration should report that migration is needed + expect(needsMigration(v1Settings)).toBe(true); + + // runMigrations should actually execute migrations + const result = runMigrations(v1Settings, 'user'); + expect(result.executedMigrations.length).toBeGreaterThan(0); + }); + + it('needsMigration should return false when runMigrations would execute no migrations', () => { + const v3Settings = { + $version: 3, + general: { enableAutoUpdate: true }, + }; + + // needsMigration should report that no migration is needed + expect(needsMigration(v3Settings)).toBe(false); + + // runMigrations should execute no migrations + const result = runMigrations(v3Settings, 'user'); + expect(result.executedMigrations).toHaveLength(0); + }); + + it('should handle V2 settings without deprecated keys consistently', () => { + const cleanV2Settings = { + $version: 2, + ui: { theme: 'dark' }, + }; + + // needsMigration should report that migration is needed + expect(needsMigration(cleanV2Settings)).toBe(true); + + // runMigrations should execute the V2->V3 migration + const result = runMigrations(cleanV2Settings, 'user'); + expect(result.executedMigrations.length).toBeGreaterThan(0); + expect(result.finalVersion).toBe(3); + }); + }); + + describe('migration chain integrity', () => { + it('should have strictly increasing versions (toVersion > fromVersion)', () => { + for (const migration of ALL_MIGRATIONS) { + expect(migration.toVersion).toBeGreaterThan(migration.fromVersion); + } + }); + + it('should have no gaps in the chain (adjacent versions)', () => { + for (let i = 1; i < ALL_MIGRATIONS.length; i++) { + const prevMigration = ALL_MIGRATIONS[i - 1]; + const currMigration = ALL_MIGRATIONS[i]; + expect(currMigration.fromVersion).toBe(prevMigration.toVersion); + } + }); + + it('should have no duplicate fromVersions', () => { + const fromVersions = ALL_MIGRATIONS.map((m) => m.fromVersion); + const uniqueFromVersions = new Set(fromVersions); + expect(uniqueFromVersions.size).toBe(fromVersions.length); + }); + + it('should have no duplicate toVersions', () => { + const toVersions = ALL_MIGRATIONS.map((m) => m.toVersion); + const uniqueToVersions = new Set(toVersions); + expect(uniqueToVersions.size).toBe(toVersions.length); + }); + + it('should be acyclic (no version appears as fromVersion more than once)', () => { + const fromVersionCounts = new Map(); + for (const migration of ALL_MIGRATIONS) { + const count = fromVersionCounts.get(migration.fromVersion) || 0; + fromVersionCounts.set(migration.fromVersion, count + 1); + } + + for (const count of fromVersionCounts.values()) { + expect(count).toBe(1); + } + }); + + it('should chain from version 1 to SETTINGS_VERSION', () => { + if (ALL_MIGRATIONS.length > 0) { + expect(ALL_MIGRATIONS[0].fromVersion).toBe(1); + const lastMigration = ALL_MIGRATIONS[ALL_MIGRATIONS.length - 1]; + expect(lastMigration.toVersion).toBe(SETTINGS_VERSION); + } + }); + }); + + describe('single source of truth for version constant', () => { + it('should use SETTINGS_VERSION from settings module', () => { + // The last migration's toVersion should match SETTINGS_VERSION + const lastMigration = ALL_MIGRATIONS[ALL_MIGRATIONS.length - 1]; + expect(lastMigration.toVersion).toBe(SETTINGS_VERSION); + }); + + it('needsMigration should use SETTINGS_VERSION for version comparison', () => { + // Create settings with version equal to SETTINGS_VERSION + const currentVersionSettings = { + $version: SETTINGS_VERSION, + general: { enableAutoUpdate: true }, + }; + + // needsMigration should return false for current version + expect(needsMigration(currentVersionSettings)).toBe(false); + + // Create settings with version less than SETTINGS_VERSION + const oldVersionSettings = { + $version: SETTINGS_VERSION - 1, + general: { disableAutoUpdate: true }, + }; + + // needsMigration should return true for old version + expect(needsMigration(oldVersionSettings)).toBe(true); + }); + + it('should have SETTINGS_VERSION defined exactly once in codebase', () => { + // SETTINGS_VERSION is imported from settings.js + // This test verifies the wiring is correct + expect(SETTINGS_VERSION).toBeDefined(); + expect(typeof SETTINGS_VERSION).toBe('number'); + expect(SETTINGS_VERSION).toBeGreaterThan(0); + }); + }); + + describe('invalid version handling', () => { + it('should treat non-numeric version with V1 shape as needing migration', () => { + const settingsWithInvalidVersion = { + $version: 'invalid', + theme: 'dark', + disableAutoUpdate: true, + }; + + // Should detect migration needed based on V1 shape + expect(needsMigration(settingsWithInvalidVersion)).toBe(true); + + // Should run migrations + const result = runMigrations(settingsWithInvalidVersion, 'user'); + expect(result.executedMigrations.length).toBeGreaterThan(0); + expect(result.finalVersion).toBe(SETTINGS_VERSION); + }); + + it('should not migrate non-numeric version with already-migrated shape (normalized by loader)', () => { + const settingsWithInvalidVersionButMigratedShape = { + $version: 'invalid', + general: { enableAutoUpdate: true }, + }; + + // needsMigration returns false because no migration applies to this shape + // The settings loader will handle version normalization separately + expect(needsMigration(settingsWithInvalidVersionButMigratedShape)).toBe( + false, + ); + + // No migrations should execute + const result = runMigrations( + settingsWithInvalidVersionButMigratedShape, + 'user', + ); + expect(result.executedMigrations).toHaveLength(0); + }); + + it('should avoid repeated no-op migration loops', () => { + // Settings that might cause repeated migrations + const v3Settings = { + $version: 3, + general: { enableAutoUpdate: true }, + }; + + // First check + expect(needsMigration(v3Settings)).toBe(false); + const result1 = runMigrations(v3Settings, 'user'); + expect(result1.executedMigrations).toHaveLength(0); + + // Second check should be consistent + expect(needsMigration(result1.settings)).toBe(false); + const result2 = runMigrations(result1.settings, 'user'); + expect(result2.executedMigrations).toHaveLength(0); + }); + }); +}); diff --git a/packages/cli/src/config/migration/index.ts b/packages/cli/src/config/migration/index.ts new file mode 100644 index 000000000..40d176cbe --- /dev/null +++ b/packages/cli/src/config/migration/index.ts @@ -0,0 +1,106 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +// Export types +export type { SettingsMigration, MigrationResult } from './types.js'; + +// Export scheduler +export { MigrationScheduler } from './scheduler.js'; + +// Export migrations +export { v1ToV2Migration, V1ToV2Migration } from './versions/v1-to-v2.js'; +export { v2ToV3Migration, V2ToV3Migration } from './versions/v2-to-v3.js'; + +// Import settings version from single source of truth +import { SETTINGS_VERSION } from '../settings.js'; + +// Ordered array of all migrations for use with MigrationScheduler +// Each migration handles one version transition (N → N+1) +// Order matters: migrations must be sorted by ascending version +import { v1ToV2Migration } from './versions/v1-to-v2.js'; +import { v2ToV3Migration } from './versions/v2-to-v3.js'; +import { MigrationScheduler } from './scheduler.js'; +import type { MigrationResult } from './types.js'; + +/** + * Ordered array of all settings migrations. + * Use this with MigrationScheduler to run the full migration chain. + * + * @example + * ```typescript + * const scheduler = new MigrationScheduler(ALL_MIGRATIONS); + * const result = scheduler.migrate(settings); + * ``` + */ +export const ALL_MIGRATIONS = [v1ToV2Migration, v2ToV3Migration] as const; + +/** + * Convenience function that runs all migrations on the given settings. + * This is the primary entry point for settings migration. + * + * @param settings - The settings object to migrate + * @param scope - The scope of settings being migrated + * @returns MigrationResult containing the final settings, version, and execution log + * + * @example + * ```typescript + * const result = runMigrations(settings, 'User'); + * if (result.executedMigrations.length > 0) { + * console.log(`Migrated from version ${result.executedMigrations[0].fromVersion} to ${result.finalVersion}`); + * } + * ``` + */ +export function runMigrations( + settings: unknown, + scope: string, +): MigrationResult { + const scheduler = new MigrationScheduler([...ALL_MIGRATIONS], scope); + return scheduler.migrate(settings); +} + +/** + * Checks if the given settings need migration. + * Returns true only if at least one registered migration would be applied. + * + * This function checks: + * 1. If $version field exists and is a number: + * - Returns false if $version >= SETTINGS_VERSION + * - Returns true only when $version < SETTINGS_VERSION AND at least one + * migration can execute for the current settings shape + * 2. If $version field is missing or invalid: + * - Uses fallback logic by checking individual migrations + * + * Note: + * - Legacy numeric versions that have no executable migrations are handled by + * the settings loader via version normalization (bump metadata to current). + * + * @param settings - The settings object to check + * @returns true if migration is needed, false otherwise + */ +export function needsMigration(settings: unknown): boolean { + if (typeof settings !== 'object' || settings === null) { + return false; + } + + const s = settings as Record; + const version = s['$version']; + const hasApplicableMigration = ALL_MIGRATIONS.some((migration) => + migration.shouldMigrate(settings), + ); + + // If $version is a valid number, use version comparison + if (typeof version === 'number') { + if (version >= SETTINGS_VERSION) { + return false; + } + // Guardrail: only report migration-needed if at least one migration can execute. + return hasApplicableMigration; + } + + // If $version exists but is not a number (invalid), or is missing: + // Use fallback logic - check if any migration would be applied + return hasApplicableMigration; +} diff --git a/packages/cli/src/config/migration/scheduler.test.ts b/packages/cli/src/config/migration/scheduler.test.ts new file mode 100644 index 000000000..91e9eff98 --- /dev/null +++ b/packages/cli/src/config/migration/scheduler.test.ts @@ -0,0 +1,164 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi } from 'vitest'; +import { MigrationScheduler } from './scheduler.js'; + +import type { SettingsMigration } from './types.js'; + +describe('MigrationScheduler', () => { + // Mock migration for testing + const createMockMigration = ( + fromVersion: number, + toVersion: number, + shouldMigrateResult: boolean, + ): SettingsMigration => ({ + fromVersion, + toVersion, + shouldMigrate: vi.fn().mockReturnValue(shouldMigrateResult), + migrate: vi.fn((settings) => ({ + settings: { + ...(settings as Record), + $version: toVersion, + }, + warnings: [], + })), + }); + + it('should execute migrations in order when shouldMigrate returns true', () => { + const migration1 = createMockMigration(1, 2, true); + const migration2 = createMockMigration(2, 3, true); + + const scheduler = new MigrationScheduler([migration1, migration2], 'user'); + const result = scheduler.migrate({ $version: 1, someKey: 'value' }); + + expect(migration1.shouldMigrate).toHaveBeenCalledTimes(1); + expect(migration1.migrate).toHaveBeenCalledTimes(1); + expect(migration2.shouldMigrate).toHaveBeenCalledTimes(1); + expect(migration2.migrate).toHaveBeenCalledTimes(1); + + expect(result.executedMigrations).toHaveLength(2); + expect(result.executedMigrations[0]).toEqual({ + fromVersion: 1, + toVersion: 2, + }); + expect(result.executedMigrations[1]).toEqual({ + fromVersion: 2, + toVersion: 3, + }); + expect(result.finalVersion).toBe(3); + }); + + it('should skip migrations when shouldMigrate returns false', () => { + const migration1 = createMockMigration(1, 2, false); + const migration2 = createMockMigration(2, 3, true); + + const scheduler = new MigrationScheduler([migration1, migration2], 'user'); + const result = scheduler.migrate({ $version: 2, someKey: 'value' }); + + expect(migration1.shouldMigrate).toHaveBeenCalledTimes(1); + expect(migration1.migrate).not.toHaveBeenCalled(); + expect(migration2.shouldMigrate).toHaveBeenCalledTimes(1); + expect(migration2.migrate).toHaveBeenCalledTimes(1); + + expect(result.executedMigrations).toHaveLength(1); + expect(result.executedMigrations[0]).toEqual({ + fromVersion: 2, + toVersion: 3, + }); + }); + + it('should be idempotent - running migrations twice produces same result', () => { + // Create a migration that checks the version to determine if migration is needed + const migration1: SettingsMigration = { + fromVersion: 1, + toVersion: 2, + shouldMigrate: vi.fn((settings) => { + const s = settings as Record; + return s['$version'] !== 2; + }), + migrate: vi.fn((settings) => ({ + settings: { + ...(settings as Record), + $version: 2, + }, + warnings: [], + })), + }; + + const scheduler = new MigrationScheduler([migration1], 'user'); + const input = { theme: 'dark' }; + + const result1 = scheduler.migrate(input); + const result2 = scheduler.migrate(result1.settings); + + expect(result1.executedMigrations).toHaveLength(1); + expect(result2.executedMigrations).toHaveLength(0); + expect(result1.finalVersion).toBe(result2.finalVersion); + }); + + it('should pass updated settings to each migration', () => { + const migration1: SettingsMigration = { + fromVersion: 1, + toVersion: 2, + shouldMigrate: vi.fn().mockReturnValue(true), + migrate: vi.fn(() => ({ + settings: { $version: 2, transformed: true }, + warnings: [], + })), + }; + + const migration2: SettingsMigration = { + fromVersion: 2, + toVersion: 3, + shouldMigrate: vi.fn().mockReturnValue(true), + migrate: vi.fn((s) => ({ settings: s, warnings: [] })), + }; + + const scheduler = new MigrationScheduler([migration1, migration2], 'user'); + scheduler.migrate({ $version: 1 }); + + expect(migration2.shouldMigrate).toHaveBeenCalledWith( + expect.objectContaining({ $version: 2, transformed: true }), + ); + }); + + it('should handle empty migrations array', () => { + const scheduler = new MigrationScheduler([], 'user'); + const result = scheduler.migrate({ $version: 1, key: 'value' }); + + expect(result.executedMigrations).toHaveLength(0); + expect(result.finalVersion).toBe(1); + expect(result.settings).toEqual({ $version: 1, key: 'value' }); + }); + + it('should throw error when migration fails', () => { + const migration1: SettingsMigration = { + fromVersion: 1, + toVersion: 2, + shouldMigrate: vi.fn().mockReturnValue(true), + migrate: vi.fn().mockImplementation(() => { + throw new Error('Migration failed'); + }), + }; + + const scheduler = new MigrationScheduler([migration1], 'user'); + + expect(() => scheduler.migrate({ $version: 1 })).toThrow( + 'Migration failed', + ); + }); + + it('should handle settings without version field', () => { + const migration1 = createMockMigration(1, 2, true); + + const scheduler = new MigrationScheduler([migration1], 'user'); + const result = scheduler.migrate({ theme: 'dark' }); + + expect(result.finalVersion).toBe(2); + expect(result.executedMigrations).toHaveLength(1); + }); +}); diff --git a/packages/cli/src/config/migration/scheduler.ts b/packages/cli/src/config/migration/scheduler.ts new file mode 100644 index 000000000..7bbcc43d6 --- /dev/null +++ b/packages/cli/src/config/migration/scheduler.ts @@ -0,0 +1,115 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { createDebugLogger } from '@qwen-code/qwen-code-core'; +import type { SettingsMigration, MigrationResult } from './types.js'; + +const debugLogger = createDebugLogger('SETTINGS_MIGRATION'); + +/** + * Formats a SettingScope enum value to a human-readable string. + * - Converts to lowercase + * - Special case: 'SystemDefaults' -> 'system default' + */ +export function formatScope(scope: string): string { + if (scope === 'SystemDefaults') { + return 'system default'; + } + return scope.toLowerCase(); +} + +/** + * Chain scheduler for settings migrations. + * + * The MigrationScheduler orchestrates multiple migrations in sequence, + * delegating version detection to each individual migration via `shouldMigrate`. + * It has no centralized version logic - migrations self-determine applicability. + * + * Key characteristics: + * - Linear chain execution: migrations are applied in registration order + * - Idempotent: already-migrated versions return false from shouldMigrate + * - Adjacent versions only: each migration handles N → N+1 + * - Pure functions: migrations don't modify input objects + */ +export class MigrationScheduler { + /** + * Creates a new MigrationScheduler with the given migrations. + * + * @param migrations - Array of migrations in execution order (typically ascending version) + * @param scope - The scope of settings being migrated + */ + constructor( + private readonly migrations: SettingsMigration[], + private readonly scope: string, + ) {} + + /** + * Executes the migration chain on the given settings. + * + * Iterates through all registered migrations in order. For each migration: + * 1. Calls `shouldMigrate` with the current settings + * 2. If true, calls `migrate` to transform the settings + * 3. Records the execution + * + * The scheduler itself has no version awareness - all version detection + * is delegated to the individual migrations. + * + * @param settings - The settings object to migrate + * @returns MigrationResult containing the final settings, version, and execution log + */ + migrate(settings: unknown): MigrationResult { + debugLogger.debug('MigrationScheduler: Starting migration chain'); + + let current = settings; + const executed: Array<{ fromVersion: number; toVersion: number }> = []; + const allWarnings: string[] = []; + + for (const migration of this.migrations) { + try { + if (migration.shouldMigrate(current)) { + debugLogger.debug( + `MigrationScheduler: Executing migration ${migration.fromVersion} → ${migration.toVersion}`, + ); + + const formattedScope = formatScope(this.scope); + const result = migration.migrate(current, formattedScope); + current = result.settings; + allWarnings.push(...result.warnings); + + executed.push({ + fromVersion: migration.fromVersion, + toVersion: migration.toVersion, + }); + + debugLogger.debug( + `MigrationScheduler: Migration ${migration.fromVersion} → ${migration.toVersion} completed successfully`, + ); + } + } catch (error) { + debugLogger.error( + `MigrationScheduler: Migration ${migration.fromVersion} → ${migration.toVersion} failed:`, + error, + ); + throw error; + } + } + + // Determine final version from the settings object + const finalVersion = + ((current as Record)['$version'] as number) ?? 1; + + debugLogger.debug( + `MigrationScheduler: Migration chain complete. Final version: ${finalVersion}, Executed: ${executed.length} migrations`, + ); + + return { + settings: current, + finalVersion, + executedMigrations: executed, + warnings: allWarnings, + }; + } +} diff --git a/packages/cli/src/config/migration/types.ts b/packages/cli/src/config/migration/types.ts new file mode 100644 index 000000000..ca1e23aaf --- /dev/null +++ b/packages/cli/src/config/migration/types.ts @@ -0,0 +1,58 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Interface that all settings migrations must implement. + * Each migration handles a single version transition (N → N+1). + */ +export interface SettingsMigration { + /** Source version number */ + readonly fromVersion: number; + + /** Target version number */ + readonly toVersion: number; + + /** + * Determines whether this migration should be applied to the given settings. + * The migration inspects the settings object to detect its current version + * and returns true if this migration is applicable. + * + * @param settings - The current settings object + * @returns true if this migration should be applied, false otherwise + */ + shouldMigrate(settings: unknown): boolean; + + /** + * Executes the migration transformation. + * This should be a pure function that does not modify the input object. + * + * @param settings - The current settings object of version N + * @param scope - The scope of settings being migrated + * @returns The migrated settings object of version N+1 with optional warnings + * @throws Error if the migration fails + */ + migrate( + settings: unknown, + scope: string, + ): { settings: unknown; warnings: string[] }; +} + +/** + * Result of a migration execution by MigrationScheduler. + */ +export interface MigrationResult { + /** The final settings object after all applicable migrations */ + settings: unknown; + + /** The final version number after migrations */ + finalVersion: number; + + /** List of migrations that were executed */ + executedMigrations: Array<{ fromVersion: number; toVersion: number }>; + + /** List of warning messages generated during migration */ + warnings: string[]; +} diff --git a/packages/cli/src/config/migration/versions/v1-to-v2-shared.ts b/packages/cli/src/config/migration/versions/v1-to-v2-shared.ts new file mode 100644 index 000000000..c87fa4480 --- /dev/null +++ b/packages/cli/src/config/migration/versions/v1-to-v2-shared.ts @@ -0,0 +1,180 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Structural mapping table for V1 -> V2. + * + * Used by: + * - v1->v2 migration execution + * - warnings for residual legacy keys in latest-version settings files + */ +export const V1_TO_V2_MIGRATION_MAP: Record = { + accessibility: 'ui.accessibility', + allowedTools: 'tools.allowed', + allowMCPServers: 'mcp.allowed', + autoAccept: 'tools.autoAccept', + autoConfigureMaxOldSpaceSize: 'advanced.autoConfigureMemory', + bugCommand: 'advanced.bugCommand', + chatCompression: 'model.chatCompression', + checkpointing: 'general.checkpointing', + coreTools: 'tools.core', + contextFileName: 'context.fileName', + customThemes: 'ui.customThemes', + customWittyPhrases: 'ui.customWittyPhrases', + debugKeystrokeLogging: 'general.debugKeystrokeLogging', + dnsResolutionOrder: 'advanced.dnsResolutionOrder', + enforcedAuthType: 'security.auth.enforcedType', + excludeTools: 'tools.exclude', + excludeMCPServers: 'mcp.excluded', + excludedProjectEnvVars: 'advanced.excludedEnvVars', + 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', + 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', + model: 'model.name', + preferredEditor: 'general.preferredEditor', + sandbox: 'tools.sandbox', + selectedAuthType: 'security.auth.selectedType', + shouldUseNodePtyShell: 'tools.shell.enableInteractiveShell', + shellPager: 'tools.shell.pager', + shellShowColor: 'tools.shell.showColor', + 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', + enableWelcomeBack: 'ui.enableWelcomeBack', + approvalMode: 'tools.approvalMode', + sessionTokenLimit: 'model.sessionTokenLimit', + contentGenerator: 'model.generationConfig', + skipLoopDetection: 'model.skipLoopDetection', + skipStartupContext: 'model.skipStartupContext', + enableOpenAILogging: 'model.enableOpenAILogging', + tavilyApiKey: 'advanced.tavilyApiKey', +}; + +/** + * Top-level keys that are V2/V3 containers. + * If one of these keys already has object value, treat it as latest-format data. + */ +export const V2_CONTAINER_KEYS = new Set([ + 'ui', + 'tools', + 'mcp', + 'advanced', + 'model', + 'general', + 'context', + 'security', + 'ide', + 'privacy', + 'telemetry', + 'extensions', +]); + +/** + * Legacy disable* keys that remain in disable* form for V2. + */ +export const V1_TO_V2_PRESERVE_DISABLE_MAP: Record = { + disableAutoUpdate: 'general.disableAutoUpdate', + disableUpdateNag: 'general.disableUpdateNag', + disableLoadingPhrases: 'ui.accessibility.disableLoadingPhrases', + disableFuzzySearch: 'context.fileFiltering.disableFuzzySearch', + disableCacheControl: 'model.generationConfig.disableCacheControl', +}; + +export const CONSOLIDATED_DISABLE_KEYS = new Set([ + 'disableAutoUpdate', + 'disableUpdateNag', +]); + +/** + * Keys that indicate V1-like top-level structure when holding primitive values. + */ +export const V1_INDICATOR_KEYS = [ + // From V1_TO_V2_MIGRATION_MAP - keys that map to different paths in V2 + 'theme', + 'model', + 'autoAccept', + 'hideTips', + 'vimMode', + 'checkpointing', + 'accessibility', + 'allowedTools', + 'allowMCPServers', + 'autoConfigureMaxOldSpaceSize', + 'bugCommand', + 'chatCompression', + 'coreTools', + 'contextFileName', + 'customThemes', + 'customWittyPhrases', + 'debugKeystrokeLogging', + 'dnsResolutionOrder', + 'enforcedAuthType', + 'excludeTools', + 'excludeMCPServers', + 'excludedProjectEnvVars', + 'fileFiltering', + 'folderTrustFeature', + 'folderTrust', + 'hasSeenIdeIntegrationNudge', + 'hideWindowTitle', + 'showStatusInTitle', + 'showLineNumbers', + 'showCitations', + 'ideMode', + 'includeDirectories', + 'loadMemoryFromIncludeDirectories', + 'maxSessionTurns', + 'mcpServerCommand', + 'memoryImportFormat', + 'preferredEditor', + 'sandbox', + 'selectedAuthType', + 'shouldUseNodePtyShell', + 'shellPager', + 'shellShowColor', + 'skipNextSpeakerCheck', + 'summarizeToolOutput', + 'toolDiscoveryCommand', + 'toolCallCommand', + 'usageStatisticsEnabled', + 'useExternalAuth', + 'useRipgrep', + 'enableWelcomeBack', + 'approvalMode', + 'sessionTokenLimit', + 'contentGenerator', + 'skipLoopDetection', + 'skipStartupContext', + 'enableOpenAILogging', + 'tavilyApiKey', + // From V1_TO_V2_PRESERVE_DISABLE_MAP - disable* keys that get nested in V2 + 'disableAutoUpdate', + 'disableUpdateNag', + 'disableLoadingPhrases', + 'disableFuzzySearch', + 'disableCacheControl', +]; diff --git a/packages/cli/src/config/migration/versions/v1-to-v2.test.ts b/packages/cli/src/config/migration/versions/v1-to-v2.test.ts new file mode 100644 index 000000000..cbe655c54 --- /dev/null +++ b/packages/cli/src/config/migration/versions/v1-to-v2.test.ts @@ -0,0 +1,277 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { V1ToV2Migration } from './v1-to-v2.js'; + +describe('V1ToV2Migration', () => { + const migration = new V1ToV2Migration(); + + describe('shouldMigrate', () => { + it('should return true for V1 settings without version and with V1 keys', () => { + const v1Settings = { + theme: 'dark', + model: 'gemini', + }; + + expect(migration.shouldMigrate(v1Settings)).toBe(true); + }); + + it('should return true for V1 settings with disable* keys', () => { + const v1Settings = { + disableAutoUpdate: true, + disableLoadingPhrases: false, + }; + + expect(migration.shouldMigrate(v1Settings)).toBe(true); + }); + + it('should return false for settings with $version field', () => { + const v2Settings = { + $version: 2, + ui: { theme: 'dark' }, + }; + + expect(migration.shouldMigrate(v2Settings)).toBe(false); + }); + + it('should return false for V3 settings', () => { + const v3Settings = { + $version: 3, + general: { enableAutoUpdate: true }, + }; + + expect(migration.shouldMigrate(v3Settings)).toBe(false); + }); + + it('should return false for settings without V1 indicator keys', () => { + const unknownSettings = { + customKey: 'value', + anotherKey: 123, + }; + + expect(migration.shouldMigrate(unknownSettings)).toBe(false); + }); + + it('should return false for null input', () => { + expect(migration.shouldMigrate(null)).toBe(false); + }); + + it('should return false for non-object input', () => { + expect(migration.shouldMigrate('string')).toBe(false); + expect(migration.shouldMigrate(123)).toBe(false); + }); + }); + + describe('migrate', () => { + it('should migrate flat V1 keys to nested V2 structure', () => { + const v1Settings = { + theme: 'dark', + model: 'gemini', + autoAccept: true, + hideTips: false, + }; + + const { settings: result } = migration.migrate(v1Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(2); + expect(result['ui']).toEqual({ theme: 'dark', hideTips: false }); + expect(result['model']).toEqual({ name: 'gemini' }); + expect(result['tools']).toEqual({ autoAccept: true }); + }); + + it('should migrate disable* keys to nested V2 paths without inversion', () => { + const v1Settings = { + theme: 'light', + disableAutoUpdate: true, + disableLoadingPhrases: false, + }; + + const { settings: result } = migration.migrate(v1Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(2); + expect(result['general']).toEqual({ disableAutoUpdate: true }); + expect(result['ui']).toEqual({ + theme: 'light', + accessibility: { disableLoadingPhrases: false }, + }); + }); + + it('should normalize consolidated disable* non-boolean values to false', () => { + const v1Settings = { + theme: 'dark', + disableAutoUpdate: 'false', + disableUpdateNag: null, + }; + + const { settings: result } = migration.migrate(v1Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(2); + expect(result['general']).toEqual({ + disableAutoUpdate: false, + disableUpdateNag: false, + }); + }); + + it('should drop non-boolean non-consolidated disable* values', () => { + const v1Settings = { + theme: 'dark', + disableLoadingPhrases: 'TRUE', + disableFuzzySearch: 1, + }; + + const { settings: result } = migration.migrate(v1Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(2); + expect( + (result['ui'] as Record)?.['accessibility'], + ).toBeUndefined(); + expect( + ( + (result['context'] as Record)?.[ + 'fileFiltering' + ] as Record + )?.['disableFuzzySearch'], + ).toBeUndefined(); + }); + + it('should preserve mcpServers at top level', () => { + const v1Settings = { + theme: 'dark', + mcpServers: { + myServer: { command: 'node', args: ['server.js'] }, + }, + }; + + const { settings: result } = migration.migrate(v1Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(2); + expect(result['mcpServers']).toEqual({ + myServer: { command: 'node', args: ['server.js'] }, + }); + }); + + it('should preserve unrecognized keys', () => { + const v1Settings = { + theme: 'dark', + myCustomSetting: 'value', + anotherCustom: 123, + }; + + const { settings: result } = migration.migrate(v1Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(2); + expect(result['myCustomSetting']).toBe('value'); + expect(result['anotherCustom']).toBe(123); + }); + + it('should preserve non-object parent path values on collision', () => { + const v1Settings = { + theme: 'dark', + disableAutoUpdate: true, + ui: 'legacy-ui-string', + general: 'legacy-general-string', + }; + + const { settings: result } = migration.migrate(v1Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(2); + expect(result['ui']).toBe('legacy-ui-string'); + expect(result['general']).toBe('legacy-general-string'); + }); + + it('should not modify the input object', () => { + const v1Settings = { + theme: 'dark', + model: 'gemini', + }; + + const { settings: result } = migration.migrate(v1Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(v1Settings).toEqual({ theme: 'dark', model: 'gemini' }); + expect(result).not.toBe(v1Settings); + }); + + it('should throw error for non-object input', () => { + expect(() => migration.migrate(null, 'user')).toThrow( + 'Settings must be an object', + ); + expect(() => migration.migrate('string', 'user')).toThrow( + 'Settings must be an object', + ); + }); + + it('should handle empty V1 settings', () => { + const v1Settings = { + theme: 'dark', + }; + + const { settings: result } = migration.migrate(v1Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(2); + expect(result['ui']).toEqual({ theme: 'dark' }); + }); + + it('should correctly handle all V1 indicator keys', () => { + const v1Settings = { + theme: 'dark', + model: 'gemini', + autoAccept: true, + hideTips: false, + vimMode: true, + checkpointing: false, + telemetry: {}, + accessibility: {}, + extensions: [], + mcpServers: {}, + }; + + const { settings: result } = migration.migrate(v1Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(2); + }); + }); + + describe('version properties', () => { + it('should have correct fromVersion', () => { + expect(migration.fromVersion).toBe(1); + }); + + it('should have correct toVersion', () => { + expect(migration.toVersion).toBe(2); + }); + }); +}); diff --git a/packages/cli/src/config/migration/versions/v1-to-v2.ts b/packages/cli/src/config/migration/versions/v1-to-v2.ts new file mode 100644 index 000000000..d2e920edc --- /dev/null +++ b/packages/cli/src/config/migration/versions/v1-to-v2.ts @@ -0,0 +1,299 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { SettingsMigration } from '../types.js'; +import { + CONSOLIDATED_DISABLE_KEYS, + V1_INDICATOR_KEYS, + V1_TO_V2_MIGRATION_MAP, + V1_TO_V2_PRESERVE_DISABLE_MAP, + V2_CONTAINER_KEYS, +} from './v1-to-v2-shared.js'; + +/** + * Heuristic indicators for deciding whether an object is "V1-like". + * + * Detection strategy: + * - A file is considered migratable as V1 when: + * 1) It is not explicitly versioned as V2+ (`$version` is missing or invalid), and + * 2) At least one indicator key appears in a legacy-compatible top-level shape. + * - Indicator list intentionally excludes keys that are valid top-level entries in + * both old and new structures to reduce false positives. + * + * Shape rule: + * - Object values for indicator keys are treated as already-nested V2-like content + * and do not alone trigger migration. + * - Primitive/array/null values on indicator keys are treated as legacy V1 signals. + */ + +/** + * Best-effort nested setter with collision protection. + * + * Strategy: + * - Create missing intermediate objects while traversing the path. + * - If traversal hits a non-object parent, abort this write. + * + * Rationale: + * - Migration should never coerce scalars into objects implicitly because that can + * destroy user data shape. Skipping the write is safer; later carry-over logic + * preserves original values. + */ +function setNestedProperty( + obj: Record, + path: string, + value: unknown, +): void { + const keys = path.split('.'); + const lastKey = keys.pop(); + if (!lastKey) return; + + let current: Record = obj; + for (const key of keys) { + if (current[key] === undefined) { + current[key] = {}; + } + const next = current[key]; + if (typeof next === 'object' && next !== null) { + current = next as Record; + } else { + // Path collision with non-object, stop here + return; + } + } + current[lastKey] = value; +} + +/** + * V1 -> V2 migration (structural normalization stage). + * + * Migration contract: + * - Input: settings in legacy V1-like shape (mostly flat, may contain mixed partial V2). + * - Output: V2-compatible nested structure with `$version: 2`. + * - No semantic inversion of disable* naming in this stage. + * + * Data-preservation strategy: + * - Prefer transforming known keys into canonical V2 locations. + * - Preserve unrecognized keys verbatim. + * - Preserve parent-path scalar values when nested writes would collide with them. + * - Preserve/merge existing partial V2 objects where safe. + * + * This class intentionally optimizes for backward compatibility and non-destructive + * behavior over aggressive normalization. + */ +export class V1ToV2Migration implements SettingsMigration { + readonly fromVersion = 1; + readonly toVersion = 2; + + /** + * Determines whether this migration should execute. + * + * Decision strategy: + * - Hard-stop when `$version` is a number >= 2 (already V2+). + * - Otherwise, scan indicator keys and trigger only when at least one indicator is + * still in legacy top-level shape (primitive/array/null). + * + * Mixed-shape tolerance: + * - Files that are partially migrated are supported; V2-like object-valued indicators + * are ignored while legacy-shaped indicators can still trigger migration. + */ + shouldMigrate(settings: unknown): boolean { + if (typeof settings !== 'object' || settings === null) { + return false; + } + + const s = settings as Record; + + // If $version exists and is a number >= 2, it's not V1 + const version = s['$version']; + if (typeof version === 'number' && version >= 2) { + return false; + } + + // Check for V1 indicator keys with primitive values + // A setting is considered V1 if ANY indicator key has a primitive value + // (string, number, boolean, null, or array) at the top level. + // Keys with object values are skipped as they may already be in V2 format. + return V1_INDICATOR_KEYS.some((key) => { + if (!(key in s)) { + return false; + } + const value = s[key]; + // Skip keys with object values - they may already be in V2 nested format + // But don't let them block migration of other keys + if ( + typeof value === 'object' && + value !== null && + !Array.isArray(value) + ) { + // This key appears to be in V2 format, skip it but continue + // checking other keys + return false; + } + // Found a key with primitive value - this is V1 format + return true; + }); + } + + /** + * Performs non-destructive V1 -> V2 transformation. + * + * Detailed strategy: + * 1) Relocate known V1 keys using `V1_TO_V2_MIGRATION_MAP`. + * - If a source value is already an object and maps to a child path of itself + * (partial V2 shape), merge child properties into target path. + * 2) Relocate disable* keys into V2 disable* locations. + * - Consolidated keys (`disableAutoUpdate`, `disableUpdateNag`): normalize to + * boolean with stable-compatible presence semantics (`value === true`). + * - Other disable* keys: migrate only boolean values. + * 3) Preserve `mcpServers` top-level placement. + * 4) Carry over remaining keys: + * - If a key is parent of migrated nested paths, merge unprocessed object children. + * - If parent value is non-object, preserve that scalar/array/null as-is. + * - Otherwise copy untouched key/value. + * 5) Stamp `$version = 2`. + * + * The method is pure with respect to input mutation. + */ + migrate( + settings: unknown, + _scope: string, + ): { settings: unknown; warnings: string[] } { + if (typeof settings !== 'object' || settings === null) { + throw new Error('Settings must be an object'); + } + + const source = settings as Record; + const result: Record = {}; + const processedKeys = new Set(); + const warnings: string[] = []; + + // Step 1: Map known V1 keys to V2 nested paths + for (const [v1Key, v2Path] of Object.entries(V1_TO_V2_MIGRATION_MAP)) { + if (v1Key in source) { + const value = source[v1Key]; + + // Safety check: If this key is a V2 container (like 'model') and it's + // already an object, it's likely already in V2 format. Skip migration + // to prevent double-nesting (e.g., model.name.name). + if ( + V2_CONTAINER_KEYS.has(v1Key) && + typeof value === 'object' && + value !== null && + !Array.isArray(value) + ) { + // This is already a V2 container, carry it over as-is + result[v1Key] = value; + processedKeys.add(v1Key); + continue; + } + + // If value is already an object and the path matches the key, + // it might be a partial V2 structure. Merge its contents. + if ( + typeof value === 'object' && + value !== null && + !Array.isArray(value) && + v2Path.startsWith(v1Key + '.') + ) { + // Merge nested properties from this partial V2 structure + for (const [nestedKey, nestedValue] of Object.entries(value)) { + setNestedProperty(result, `${v2Path}.${nestedKey}`, nestedValue); + } + } else { + setNestedProperty(result, v2Path, value); + } + processedKeys.add(v1Key); + } + } + + // Step 2: Map V1 disable* keys to V2 nested disable* paths + for (const [v1Key, v2Path] of Object.entries( + V1_TO_V2_PRESERVE_DISABLE_MAP, + )) { + if (v1Key in source) { + const value = source[v1Key]; + if (CONSOLIDATED_DISABLE_KEYS.has(v1Key)) { + // Preserve stable behavior: consolidated keys use presence semantics. + // Only literal true remains true; all other present values become false. + setNestedProperty(result, v2Path, value === true); + } else if (typeof value === 'boolean') { + // Non-consolidated disable* keys only migrate when explicitly boolean. + setNestedProperty(result, v2Path, value); + } + processedKeys.add(v1Key); + } + } + + // Step 3: Preserve mcpServers at the top level + if ('mcpServers' in source) { + result['mcpServers'] = source['mcpServers']; + processedKeys.add('mcpServers'); + } + + // Step 4: Carry over any unrecognized keys (including unknown nested objects) + // Important: Skip keys that are parent paths of already-migrated properties + // to avoid overwriting merged structures (e.g., 'ui' should not overwrite 'ui.theme') + for (const key of Object.keys(source)) { + if (!processedKeys.has(key)) { + // Check if this key is a parent of any already-migrated path + const isParentOfMigratedPath = Array.from(processedKeys).some( + (processedKey) => { + // Get the v2 path for this processed key + const v2Path = + V1_TO_V2_MIGRATION_MAP[processedKey] || + V1_TO_V2_PRESERVE_DISABLE_MAP[processedKey]; + if (!v2Path) return false; + // Check if the v2 path starts with this key + '.' + return v2Path.startsWith(key + '.'); + }, + ); + + if (isParentOfMigratedPath) { + // This key is a parent of an already-migrated path + // Merge its unprocessed children instead of overwriting + const existingValue = source[key]; + if ( + typeof existingValue === 'object' && + existingValue !== null && + !Array.isArray(existingValue) + ) { + for (const [nestedKey, nestedValue] of Object.entries( + existingValue, + )) { + // Only merge if this nested key wasn't already processed + const fullNestedPath = `${key}.${nestedKey}`; + const wasProcessed = Array.from(processedKeys).some( + (processedKey) => { + const v2Path = + V1_TO_V2_MIGRATION_MAP[processedKey] || + V1_TO_V2_PRESERVE_DISABLE_MAP[processedKey]; + return v2Path === fullNestedPath; + }, + ); + if (!wasProcessed) { + setNestedProperty(result, fullNestedPath, nestedValue); + } + } + } else { + // Preserve non-object parent values to match legacy overwrite semantics. + result[key] = source[key]; + } + } else { + // Not a parent path, safe to copy as-is + result[key] = source[key]; + } + } + } + + // Step 5: Set version to 2 + result['$version'] = 2; + + return { settings: result, warnings }; + } +} + +/** Singleton instance of V1→V2 migration */ +export const v1ToV2Migration = new V1ToV2Migration(); diff --git a/packages/cli/src/config/migration/versions/v2-to-v3.test.ts b/packages/cli/src/config/migration/versions/v2-to-v3.test.ts new file mode 100644 index 000000000..dc4ed97c4 --- /dev/null +++ b/packages/cli/src/config/migration/versions/v2-to-v3.test.ts @@ -0,0 +1,549 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { V2ToV3Migration } from './v2-to-v3.js'; + +describe('V2ToV3Migration', () => { + const migration = new V2ToV3Migration(); + + describe('shouldMigrate', () => { + it('should return true for V2 settings with deprecated disable* keys', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: true }, + }; + + expect(migration.shouldMigrate(v2Settings)).toBe(true); + }); + + it('should return true for V2 settings with ui.accessibility.disableLoadingPhrases', () => { + const v2Settings = { + $version: 2, + ui: { accessibility: { disableLoadingPhrases: false } }, + }; + + expect(migration.shouldMigrate(v2Settings)).toBe(true); + }); + + it('should return false for V3 settings', () => { + const v3Settings = { + $version: 3, + general: { enableAutoUpdate: true }, + }; + + expect(migration.shouldMigrate(v3Settings)).toBe(false); + }); + + it('should return false for V1 settings without version', () => { + const v1Settings = { + theme: 'dark', + disableAutoUpdate: true, + }; + + expect(migration.shouldMigrate(v1Settings)).toBe(false); + }); + + it('should return true for V2 settings without deprecated keys', () => { + const cleanV2Settings = { + $version: 2, + ui: { theme: 'dark' }, + general: { enableAutoUpdate: true }, + }; + + // V2 settings should always be migrated to V3 to update the version number + expect(migration.shouldMigrate(cleanV2Settings)).toBe(true); + }); + + it('should return false for null input', () => { + expect(migration.shouldMigrate(null)).toBe(false); + }); + + it('should return false for non-object input', () => { + expect(migration.shouldMigrate('string')).toBe(false); + expect(migration.shouldMigrate(123)).toBe(false); + }); + }); + + describe('migrate', () => { + it('should migrate disableAutoUpdate to enableAutoUpdate with inverted value', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: true }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['general'] as Record)['enableAutoUpdate'], + ).toBe(false); + expect( + (result['general'] as Record)['disableAutoUpdate'], + ).toBeUndefined(); + }); + + it('should migrate disableLoadingPhrases to enableLoadingPhrases', () => { + const v2Settings = { + $version: 2, + ui: { accessibility: { disableLoadingPhrases: true } }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['ui'] as Record)['accessibility'], + ).toEqual({ + enableLoadingPhrases: false, + }); + }); + + it('should migrate disableFuzzySearch to enableFuzzySearch', () => { + const v2Settings = { + $version: 2, + context: { fileFiltering: { disableFuzzySearch: false } }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['context'] as Record)['fileFiltering'], + ).toEqual({ + enableFuzzySearch: true, + }); + }); + + it('should migrate disableCacheControl to enableCacheControl', () => { + const v2Settings = { + $version: 2, + model: { generationConfig: { disableCacheControl: true } }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['model'] as Record)['generationConfig'], + ).toEqual({ + enableCacheControl: false, + }); + }); + + it('should handle consolidated disableAutoUpdate and disableUpdateNag', () => { + const v2Settings = { + $version: 2, + general: { + disableAutoUpdate: true, + disableUpdateNag: false, + }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + // If ANY disable* is true, enable should be false + expect( + (result['general'] as Record)['enableAutoUpdate'], + ).toBe(false); + expect( + (result['general'] as Record)['disableAutoUpdate'], + ).toBeUndefined(); + expect( + (result['general'] as Record)['disableUpdateNag'], + ).toBeUndefined(); + }); + + it('should set enableAutoUpdate to true when both disable* are false', () => { + const v2Settings = { + $version: 2, + general: { + disableAutoUpdate: false, + disableUpdateNag: false, + }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['general'] as Record)['enableAutoUpdate'], + ).toBe(true); + }); + + it('should preserve other settings during migration', () => { + const v2Settings = { + $version: 2, + ui: { + theme: 'dark', + accessibility: { disableLoadingPhrases: true }, + }, + model: { + name: 'gemini', + }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect((result['ui'] as Record)['theme']).toBe('dark'); + expect((result['model'] as Record)['name']).toBe( + 'gemini', + ); + expect( + (result['ui'] as Record)['accessibility'], + ).toEqual({ + enableLoadingPhrases: false, + }); + }); + + it('should not modify the input object', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: true }, + }; + + const result = migration.migrate(v2Settings, 'user'); + + expect(v2Settings.general).toEqual({ disableAutoUpdate: true }); + expect(result).not.toBe(v2Settings); + }); + + it('should throw error for non-object input', () => { + expect(() => migration.migrate(null, 'user')).toThrow( + 'Settings must be an object', + ); + expect(() => migration.migrate('string', 'user')).toThrow( + 'Settings must be an object', + ); + }); + + it('should handle multiple deprecated keys in one migration', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: false }, + ui: { accessibility: { disableLoadingPhrases: false } }, + context: { fileFiltering: { disableFuzzySearch: false } }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['general'] as Record)['enableAutoUpdate'], + ).toBe(true); + expect( + (result['ui'] as Record)['accessibility'], + ).toEqual({ + enableLoadingPhrases: true, + }); + expect( + (result['context'] as Record)['fileFiltering'], + ).toEqual({ + enableFuzzySearch: true, + }); + }); + + it('should preserve string "true" without creating enable key', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: 'true' }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['general'] as Record)['disableAutoUpdate'], + ).toBe('true'); + expect( + (result['general'] as Record)['enableAutoUpdate'], + ).toBeUndefined(); + }); + + it('should preserve string "false" without creating enable key', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: 'false' }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['general'] as Record)['disableAutoUpdate'], + ).toBe('false'); + expect( + (result['general'] as Record)['enableAutoUpdate'], + ).toBeUndefined(); + }); + + it('should preserve string "TRUE" without creating enable key', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: 'TRUE' }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['general'] as Record)['disableAutoUpdate'], + ).toBe('TRUE'); + }); + + it('should preserve string "FALSE" without creating enable key', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: 'FALSE' }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['general'] as Record)['disableAutoUpdate'], + ).toBe('FALSE'); + }); + + it('should preserve number value and not create enable key', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: 123 }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['general'] as Record)['disableAutoUpdate'], + ).toBe(123); + expect( + (result['general'] as Record)['enableAutoUpdate'], + ).toBeUndefined(); + }); + + it('should preserve invalid string value and not create enable key', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: 'invalid-string' }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['general'] as Record)['disableAutoUpdate'], + ).toBe('invalid-string'); + expect( + (result['general'] as Record)['enableAutoUpdate'], + ).toBeUndefined(); + }); + + it('should preserve disableCacheControl string "true"', () => { + const v2Settings = { + $version: 2, + model: { generationConfig: { disableCacheControl: 'true' } }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['model'] as Record)['generationConfig'], + ).toEqual({ + disableCacheControl: 'true', + }); + }); + + it('should preserve disableCacheControl string "false"', () => { + const v2Settings = { + $version: 2, + model: { generationConfig: { disableCacheControl: 'false' } }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['model'] as Record)['generationConfig'], + ).toEqual({ + disableCacheControl: 'false', + }); + }); + + it('should preserve disableCacheControl number value and not create enable key', () => { + const v2Settings = { + $version: 2, + model: { generationConfig: { disableCacheControl: 456 } }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['model'] as Record)['generationConfig'], + ).toEqual({ disableCacheControl: 456 }); + expect( + ( + (result['model'] as Record)[ + 'generationConfig' + ] as Record + )['enableCacheControl'], + ).toBeUndefined(); + }); + + it('should handle mixed valid and invalid disableAutoUpdate and disableUpdateNag', () => { + const v2Settings = { + $version: 2, + general: { + disableAutoUpdate: true, + disableUpdateNag: 'invalid', + }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + // Only valid values should contribute to the consolidated result + // Since disableAutoUpdate is true, enableAutoUpdate should be false + expect( + (result['general'] as Record)['enableAutoUpdate'], + ).toBe(false); + expect( + (result['general'] as Record)['disableAutoUpdate'], + ).toBeUndefined(); + expect( + (result['general'] as Record)['disableUpdateNag'], + ).toBe('invalid'); + }); + + it('should preserve object value for disable key', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: { nested: 'value' } }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['general'] as Record)['disableAutoUpdate'], + ).toEqual({ nested: 'value' }); + expect( + (result['general'] as Record)['enableAutoUpdate'], + ).toBeUndefined(); + }); + + it('should preserve array value for disable key', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: [1, 2, 3] }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['general'] as Record)['disableAutoUpdate'], + ).toEqual([1, 2, 3]); + expect( + (result['general'] as Record)['enableAutoUpdate'], + ).toBeUndefined(); + }); + + it('should preserve null value for disable key', () => { + const v2Settings = { + $version: 2, + general: { disableAutoUpdate: null }, + }; + + const { settings: result } = migration.migrate(v2Settings, 'user') as { + settings: Record; + warnings: unknown[]; + }; + + expect(result['$version']).toBe(3); + expect( + (result['general'] as Record)['disableAutoUpdate'], + ).toBeNull(); + expect( + (result['general'] as Record)['enableAutoUpdate'], + ).toBeUndefined(); + }); + }); + + describe('version properties', () => { + it('should have correct fromVersion', () => { + expect(migration.fromVersion).toBe(2); + }); + + it('should have correct toVersion', () => { + expect(migration.toVersion).toBe(3); + }); + }); +}); diff --git a/packages/cli/src/config/migration/versions/v2-to-v3.ts b/packages/cli/src/config/migration/versions/v2-to-v3.ts new file mode 100644 index 000000000..0761af4c9 --- /dev/null +++ b/packages/cli/src/config/migration/versions/v2-to-v3.ts @@ -0,0 +1,241 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { SettingsMigration } from '../types.js'; + +/** + * Path mapping for boolean polarity migration (V2 disable* -> V3 enable*). + * + * Strategy: + * - For each mapped path, only boolean inputs are transformed. + * - Transformation is inversion-based: disable=true -> enable=false, disable=false -> enable=true. + * - Non-boolean values are intentionally ignored here to preserve stable compatibility. + */ +const V2_TO_V3_BOOLEAN_MAP: Record = { + 'general.disableAutoUpdate': 'general.enableAutoUpdate', + 'general.disableUpdateNag': 'general.enableAutoUpdate', + 'ui.accessibility.disableLoadingPhrases': + 'ui.accessibility.enableLoadingPhrases', + 'context.fileFiltering.disableFuzzySearch': + 'context.fileFiltering.enableFuzzySearch', + 'model.generationConfig.disableCacheControl': + 'model.generationConfig.enableCacheControl', +}; + +/** + * Consolidated old paths that collapse into one V3 field. + * + * Current policy: + * - `general.disableAutoUpdate` and `general.disableUpdateNag` both drive + * `general.enableAutoUpdate`. + * - If any observed boolean source is true, target becomes false. + * - If no boolean source exists, consolidation does not emit a target value. + */ +const CONSOLIDATED_V2_PATHS: Record = { + 'general.enableAutoUpdate': [ + 'general.disableAutoUpdate', + 'general.disableUpdateNag', + ], +}; + +/** + * Safe nested getter used during migration path inspection. + * + * Returns `undefined` when traversal cannot continue (missing key or non-object parent). + */ +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; +} + +/** + * Safe nested setter used for emitting migrated paths. + * + * Behavior: + * - Creates intermediate objects when absent. + * - Aborts write if a parent segment is a non-object (collision protection). + */ +function setNestedProperty( + obj: Record, + path: string, + value: unknown, +): void { + const keys = path.split('.'); + const lastKey = keys.pop(); + if (!lastKey) return; + + let current: Record = obj; + for (const key of keys) { + if (current[key] === undefined) { + current[key] = {}; + } + const next = current[key]; + if (typeof next === 'object' && next !== null) { + current = next as Record; + } else { + // Path collision with non-object, stop here + return; + } + } + current[lastKey] = value; +} + +/** + * Best-effort nested delete for removing deprecated disable* keys. + * + * If traversal hits a non-object parent, deletion is skipped. + */ +function deleteNestedProperty( + obj: Record, + path: string, +): void { + const keys = path.split('.'); + const lastKey = keys.pop(); + if (!lastKey) return; + + let current: Record = obj; + for (const key of keys) { + const next = current[key]; + if (typeof next !== 'object' || next === null) { + return; + } + current = next as Record; + } + delete current[lastKey]; +} + +/** + * JSON-based deep clone used to keep `migrate()` input immutable. + */ +function deepClone(obj: T): T { + return JSON.parse(JSON.stringify(obj)); +} + +/** + * V2 -> V3 migration (boolean polarity normalization stage). + * + * Migration contract: + * - Input: V2 settings object (`$version: 2`). + * - Output: `$version: 3` with boolean disable* fields migrated to enable* equivalents. + * + * Compatibility strategy: + * - Transform only boolean-valued deprecated fields. + * - Preserve non-boolean deprecated values untouched. + * - Always bump version to 3 so future loads are idempotent and skip repeated checks. + * + * This implementation mirrors stable behavior and prioritizes parity over aggressive cleanup. + */ +export class V2ToV3Migration implements SettingsMigration { + readonly fromVersion = 2; + readonly toVersion = 3; + + /** + * Migration trigger rule. + * + * Execute only when `$version === 2`. + * This includes V2 files with no migratable disable* booleans so that version + * metadata still advances to 3. + */ + shouldMigrate(settings: unknown): boolean { + if (typeof settings !== 'object' || settings === null) { + return false; + } + + const s = settings as Record; + + // Migrate if $version is 2 + return s['$version'] === 2; + } + + /** + * Applies V2 -> V3 transformation with stable-compatible rules. + * + * Detailed strategy: + * 1) Clone input. + * 2) Process consolidated paths first: + * - Inspect each source path. + * - If value is boolean, consume it (delete old key) and contribute to aggregate. + * - Emit consolidated target when at least one boolean source was consumed. + * 3) Process remaining one-to-one mappings: + * - For each unmapped source, if boolean -> delete old key and write inverted target. + * - If non-boolean -> keep old value unchanged. + * 4) Set `$version = 3`. + * + * Guarantees: + * - Input object is not mutated. + * - Boolean migration is deterministic. + * - Non-boolean legacy values are preserved for compatibility. + */ + migrate( + settings: unknown, + _scope: string, + ): { settings: unknown; warnings: string[] } { + if (typeof settings !== 'object' || settings === null) { + throw new Error('Settings must be an object'); + } + + // Deep clone to avoid mutating input + const result = deepClone(settings) as Record; + const processedPaths = new Set(); + const warnings: string[] = []; + + // Step 1: Handle consolidated paths (multiple old paths → single new path) + // Policy: if ANY of the old disable* settings is true, the new enable* should be false + for (const [newPath, oldPaths] of Object.entries(CONSOLIDATED_V2_PATHS)) { + let hasAnyDisable = false; + let hasAnyBooleanValue = false; + + for (const oldPath of oldPaths) { + const oldValue = getNestedProperty(result, oldPath); + if (typeof oldValue === 'boolean') { + hasAnyBooleanValue = true; + if (oldValue === true) { + hasAnyDisable = true; + } + deleteNestedProperty(result, oldPath); + processedPaths.add(oldPath); + } + } + + if (hasAnyBooleanValue) { + // enableAutoUpdate = !hasAnyDisable (if any disable* was true, enable should be false) + setNestedProperty(result, newPath, !hasAnyDisable); + } + } + + // Step 2: Handle remaining individual disable* → enable* mappings + for (const [oldPath, newPath] of Object.entries(V2_TO_V3_BOOLEAN_MAP)) { + if (processedPaths.has(oldPath)) { + continue; + } + + const oldValue = getNestedProperty(result, oldPath); + if (typeof oldValue === 'boolean') { + deleteNestedProperty(result, oldPath); + // Set new property with inverted value + setNestedProperty(result, newPath, !oldValue); + } + } + + // Step 3: Always update version to 3 + result['$version'] = 3; + + return { settings: result, warnings }; + } +} + +/** Singleton instance of V2→V3 migration */ +export const v2ToV3Migration = new V2ToV3Migration(); diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index bea89475f..d4241c7ba 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -18,16 +18,6 @@ vi.mock('os', async (importOriginal) => { }; }); -// Mock './settings.js' to ensure it uses the mocked 'os.homedir()' for its internal constants. -vi.mock('./settings.js', async (importActual) => { - const originalModule = await importActual(); - return { - __esModule: true, // Ensure correct module shape - ...originalModule, // Re-export all original members - // We are relying on originalModule's USER_SETTINGS_PATH being constructed with mocked os.homedir() - }; -}); - // Mock trustedFolders vi.mock('./trustedFolders.js', () => ({ isWorkspaceTrusted: vi @@ -46,7 +36,6 @@ import { afterEach, type Mocked, type Mock, - fail, } from 'vitest'; import * as fs from 'node:fs'; // fs will be mocked separately import stripJsonComments from 'strip-json-comments'; // Will be mocked separately @@ -60,13 +49,12 @@ import { getSystemSettingsPath, getSystemDefaultsPath, SETTINGS_DIRECTORY_NAME, // This is from the original module, but used by the mock. - migrateSettingsToV1, - needsMigration, type Settings, loadEnvironment, SETTINGS_VERSION, SETTINGS_VERSION_KEY, } from './settings.js'; +import { needsMigration } from './migration/index.js'; import { FatalConfigError, QWEN_DIR } from '@qwen-code/qwen-code-core'; const MOCK_WORKSPACE_DIR = '/mock/workspace'; @@ -84,6 +72,23 @@ type TestSettings = Settings & { nestedObj?: { [key: string]: unknown }; }; +vi.mock('node:fs', async (importOriginal) => { + // Get all the functions from the real 'fs' module + const actualFs = await importOriginal(); + + return { + ...actualFs, // Keep all the real functions + // Now, just override the ones we need for the test + existsSync: vi.fn(), + readFileSync: vi.fn(), + writeFileSync: vi.fn(), + renameSync: vi.fn(), + mkdirSync: vi.fn(), + realpathSync: (p: string) => p, + }; +}); + +// Also mock 'fs' for compatibility vi.mock('fs', async (importOriginal) => { // Get all the functions from the real 'fs' module const actualFs = await importOriginal(); @@ -594,19 +599,22 @@ describe('Settings Loading and Merging', () => { loadSettings(MOCK_WORKSPACE_DIR); - // Verify that fs.writeFileSync was called (to add version) - // but NOT fs.renameSync (no backup needed, just adding version) - expect(fs.renameSync).not.toHaveBeenCalled(); - expect(fs.writeFileSync).toHaveBeenCalledTimes(1); - - const writeCall = (fs.writeFileSync as Mock).mock.calls[0]; - const writtenPath = writeCall[0]; + // Version normalization now uses writeWithBackupSync (temp write + rename) + // Verify that writeFileSync was called with the temp file path + const writeCall = (fs.writeFileSync as Mock).mock.calls.find( + (call: unknown[]) => call[0] === `${USER_SETTINGS_PATH}.tmp`, + ); + expect(writeCall).toBeDefined(); + if (!writeCall) { + throw new Error('Expected temp write call for version normalization'); + } const writtenContent = JSON.parse(writeCall[1] as string); - expect(writtenPath).toBe(USER_SETTINGS_PATH); expect(writtenContent[SETTINGS_VERSION_KEY]).toBe(SETTINGS_VERSION); expect(writtenContent.ui?.theme).toBe('dark'); expect(writtenContent.model?.name).toBe('qwen-coder'); + // Verify writeWithBackupSync was called by checking temp file write + expect(fs.writeFileSync).toHaveBeenCalled(); }); it('should correctly handle partially migrated settings without version field', () => { @@ -734,14 +742,85 @@ describe('Settings Loading and Merging', () => { loadSettings(MOCK_WORKSPACE_DIR); // Version should be bumped to 3 even though no keys needed migration + // writeWithBackupSync writes to a temp file first, then renames const writeCall = (fs.writeFileSync as Mock).mock.calls.find( - (call: unknown[]) => call[0] === USER_SETTINGS_PATH, + (call: unknown[]) => call[0] === `${USER_SETTINGS_PATH}.tmp`, ); expect(writeCall).toBeDefined(); + if (!writeCall) { + throw new Error('Expected temp write call for V2->V3 version bump'); + } const writtenContent = JSON.parse(writeCall[1] as string); expect(writtenContent.$version).toBe(SETTINGS_VERSION); }); + it('should normalize invalid version metadata when no migration is applicable', () => { + (mockFsExistsSync as Mock).mockImplementation( + (p: fs.PathLike) => p === USER_SETTINGS_PATH, + ); + const invalidVersionSettings = { + $version: 'invalid-version', + general: { + enableAutoUpdate: true, + }, + }; + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === USER_SETTINGS_PATH) + return JSON.stringify(invalidVersionSettings); + return '{}'; + }, + ); + + loadSettings(MOCK_WORKSPACE_DIR); + + const writeCall = (fs.writeFileSync as Mock).mock.calls.find( + (call: unknown[]) => call[0] === `${USER_SETTINGS_PATH}.tmp`, + ); + expect(writeCall).toBeDefined(); + if (!writeCall) { + throw new Error( + 'Expected temp write call for invalid version normalization', + ); + } + const writtenContent = JSON.parse(writeCall[1] as string); + expect(writtenContent.$version).toBe(SETTINGS_VERSION); + expect(writtenContent.general?.enableAutoUpdate).toBe(true); + }); + + it('should normalize legacy numeric version when no migration can execute', () => { + (mockFsExistsSync as Mock).mockImplementation( + (p: fs.PathLike) => p === USER_SETTINGS_PATH, + ); + const staleVersionSettings = { + $version: 1, + // No V1/V2 indicators recognized by migrations + customOnlyKey: 'value', + }; + (fs.readFileSync as Mock).mockImplementation( + (p: fs.PathOrFileDescriptor) => { + if (p === USER_SETTINGS_PATH) + return JSON.stringify(staleVersionSettings); + return '{}'; + }, + ); + + loadSettings(MOCK_WORKSPACE_DIR); + + const writeCall = (fs.writeFileSync as Mock).mock.calls.find( + (call: unknown[]) => call[0] === `${USER_SETTINGS_PATH}.tmp`, + ); + expect(writeCall).toBeDefined(); + if (!writeCall) { + throw new Error( + 'Expected temp write call for stale version normalization', + ); + } + const writtenContent = JSON.parse(writeCall[1] as string); + expect(writtenContent.$version).toBe(SETTINGS_VERSION); + expect(writtenContent.customOnlyKey).toBe('value'); + }); + it('should correctly merge and migrate legacy array properties from multiple scopes', () => { (mockFsExistsSync as Mock).mockReturnValue(true); const legacyUserSettings = { @@ -1619,7 +1698,7 @@ describe('Settings Loading and Merging', () => { try { loadSettings(MOCK_WORKSPACE_DIR); - fail('loadSettings should have thrown a FatalConfigError'); + throw new Error('loadSettings should have thrown a FatalConfigError'); } catch (e) { expect(e).toBeInstanceOf(FatalConfigError); const error = e as FatalConfigError; @@ -2261,385 +2340,6 @@ describe('Settings Loading and Merging', () => { }); }); - 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: { - hideTips: true, - customThemes: { - myTheme: {}, - }, - }, - model: { - name: 'gemini-pro', - chatCompression: { - contextPercentageThreshold: 0.5, - }, - }, - mcpServers: { - 'server-1': { - command: 'node server.js', - }, - }, - unrecognized: { - should: 'be-preserved', - }, - }; - const v1Settings = migrateSettingsToV1(v2Settings); - expect(v1Settings).toEqual({ - disableAutoUpdate: true, - hideTips: true, - customThemes: { - myTheme: {}, - }, - model: 'gemini-pro', - chatCompression: { - contextPercentageThreshold: 0.5, - }, - 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-pro', - chatCompression: { - contextPercentageThreshold: 0.8, - }, - }, - 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-pro', - chatCompression: { - contextPercentageThreshold: 0.8, - }, - 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-1.5-pro', - }, - unrecognized: 'value', - }; - - const v1Settings = migrateSettingsToV1(v2Settings); - - expect(v1Settings).toEqual({ - vimMode: false, - model: 'gemini-1.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: false, - }, - }, - }; - - const v1Settings = migrateSettingsToV1(v2Settings); - - expect(v1Settings).toEqual({ - vimMode: false, - maxSessionTurns: -1, - includeDirectories: [], - folderTrust: false, - }); - }); - - 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'], - }); - }); - - it('should remove version field when migrating to V1', () => { - const v2Settings = { - [SETTINGS_VERSION_KEY]: SETTINGS_VERSION, - ui: { - theme: 'dark', - }, - model: { - name: 'qwen-coder', - }, - }; - const v1Settings = migrateSettingsToV1(v2Settings); - - // Version field should not be present in V1 settings - expect(v1Settings[SETTINGS_VERSION_KEY]).toBeUndefined(); - // Other fields should be properly migrated - expect(v1Settings).toEqual({ - theme: 'dark', - model: 'qwen-coder', - }); - }); - - it('should handle version field in unrecognized properties', () => { - const v2Settings = { - [SETTINGS_VERSION_KEY]: SETTINGS_VERSION, - general: { - vimMode: true, - }, - someUnrecognizedKey: 'value', - }; - const v1Settings = migrateSettingsToV1(v2Settings); - - // Version field should be filtered out - expect(v1Settings[SETTINGS_VERSION_KEY]).toBeUndefined(); - // Unrecognized keys should be preserved - expect(v1Settings['someUnrecognizedKey']).toBe('value'); - expect(v1Settings['vimMode']).toBe(true); - }); - }); - describe('loadEnvironment', () => { function setup({ isFolderTrustEnabled = true, diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index e261cc723..072ef7743 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -14,6 +14,9 @@ import { QWEN_DIR, getErrorMessage, Storage, + setDebugLogSession, + sanitizeCwd, + createDebugLogger, } from '@qwen-code/qwen-code-core'; import stripJsonComments from 'strip-json-comments'; import { DefaultLight } from '../ui/themes/default-light.js'; @@ -28,9 +31,15 @@ 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 { writeStderrLine } from '../utils/stdioHelpers.js'; +const debugLogger = createDebugLogger('SETTINGS'); +import { runMigrations, needsMigration } from './migration/index.js'; +import { + V1_TO_V2_MIGRATION_MAP, + V2_CONTAINER_KEYS, +} from './migration/versions/v1-to-v2-shared.js'; +import { writeWithBackupSync } from '../utils/writeWithBackup.js'; function getMergeStrategyForPath(path: string[]): MergeStrategy | undefined { let current: SettingDefinition | undefined = undefined; @@ -54,113 +63,10 @@ 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; - // Settings version to track migration state export const SETTINGS_VERSION = 3; export const SETTINGS_VERSION_KEY = '$version'; -const MIGRATION_MAP: Record = { - accessibility: 'ui.accessibility', - allowedTools: 'tools.allowed', - allowMCPServers: 'mcp.allowed', - autoAccept: 'tools.autoAccept', - autoConfigureMaxOldSpaceSize: 'advanced.autoConfigureMemory', - bugCommand: 'advanced.bugCommand', - chatCompression: 'model.chatCompression', - checkpointing: 'general.checkpointing', - coreTools: 'tools.core', - contextFileName: 'context.fileName', - customThemes: 'ui.customThemes', - customWittyPhrases: 'ui.customWittyPhrases', - debugKeystrokeLogging: 'general.debugKeystrokeLogging', - dnsResolutionOrder: 'advanced.dnsResolutionOrder', - enforcedAuthType: 'security.auth.enforcedType', - excludeTools: 'tools.exclude', - excludeMCPServers: 'mcp.excluded', - excludedProjectEnvVars: 'advanced.excludedEnvVars', - 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', - 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', - model: 'model.name', - preferredEditor: 'general.preferredEditor', - sandbox: 'tools.sandbox', - selectedAuthType: 'security.auth.selectedType', - shouldUseNodePtyShell: 'tools.shell.enableInteractiveShell', - shellPager: 'tools.shell.pager', - shellShowColor: 'tools.shell.showColor', - 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', - - enableWelcomeBack: 'ui.enableWelcomeBack', - approvalMode: 'tools.approvalMode', - sessionTokenLimit: 'model.sessionTokenLimit', - contentGenerator: 'model.generationConfig', - skipLoopDetection: 'model.skipLoopDetection', - skipStartupContext: 'model.skipStartupContext', - enableOpenAILogging: 'model.enableOpenAILogging', - tavilyApiKey: 'advanced.tavilyApiKey', -}; - -// Settings that need boolean inversion during migration (V1 -> V3) -// Old negative naming -> new positive naming with inverted value -const INVERTED_BOOLEAN_MIGRATIONS: Record = { - disableAutoUpdate: 'general.enableAutoUpdate', - disableUpdateNag: 'general.enableAutoUpdate', - disableLoadingPhrases: 'ui.accessibility.enableLoadingPhrases', - disableFuzzySearch: 'context.fileFiltering.enableFuzzySearch', - disableCacheControl: 'model.generationConfig.enableCacheControl', -}; - -// Consolidated settings: multiple old V1 keys that map to a single new key. -// Policy: if ANY of the old disable* settings is true, the new enable* should be false. -const CONSOLIDATED_SETTINGS: Record = { - 'general.enableAutoUpdate': ['disableAutoUpdate', 'disableUpdateNag'], -}; - -// V2 nested paths that need inversion when migrating to V3 -const INVERTED_V2_PATHS: Record = { - 'general.disableAutoUpdate': 'general.enableAutoUpdate', - 'general.disableUpdateNag': 'general.enableAutoUpdate', - 'ui.accessibility.disableLoadingPhrases': - 'ui.accessibility.enableLoadingPhrases', - 'context.fileFiltering.disableFuzzySearch': - 'context.fileFiltering.enableFuzzySearch', - 'model.generationConfig.disableCacheControl': - 'model.generationConfig.enableCacheControl', -}; - -// Consolidated V2 paths: multiple old paths that map to a single new path. -// Policy: if ANY of the old disable* settings is true, the new enable* should be false. -const CONSOLIDATED_V2_PATHS: Record = { - 'general.enableAutoUpdate': [ - 'general.disableAutoUpdate', - 'general.disableUpdateNag', - ], -}; - export function getSystemSettingsPath(): string { if (process.env['QWEN_CODE_SYSTEM_SETTINGS_PATH']) { return process.env['QWEN_CODE_SYSTEM_SETTINGS_PATH']; @@ -243,287 +149,6 @@ function setNestedProperty( current[lastKey] = value; } -// 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]), - ...Object.values(INVERTED_BOOLEAN_MIGRATIONS).map( - (path) => path.split('.')[0], - ), -]); - -export function needsMigration(settings: Record): boolean { - // Check version field first - if present and matches current version, no migration needed - if (SETTINGS_VERSION_KEY in settings) { - const version = settings[SETTINGS_VERSION_KEY]; - if (typeof version === 'number' && version >= SETTINGS_VERSION) { - return false; - } - } - - // Fallback to legacy detection: 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 both 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; - }); - - // Also check for old inverted boolean keys (disable* -> enable*) - const hasInvertedBooleanKeys = Object.keys(INVERTED_BOOLEAN_MIGRATIONS).some( - (v1Key) => v1Key in settings, - ); - - return hasV1Keys || hasInvertedBooleanKeys; -} - -/** - * Migrates V1 (flat) settings directly to V3. - * This includes both structural migration (flat -> nested) and boolean - * inversion (disable* -> enable*), so migrateV2ToV3 will be skipped. - */ -function migrateV1ToV3( - 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)) { - // Safety check: If this key is a V2 container (like 'model') and it's - // already an object, it's likely already in V2 format. Skip migration - // to prevent double-nesting (e.g., model.name.name). - if ( - KNOWN_V2_CONTAINERS.has(oldKey) && - typeof flatSettings[oldKey] === 'object' && - flatSettings[oldKey] !== null && - !Array.isArray(flatSettings[oldKey]) - ) { - // This is already a V2 container, carry it over as-is - v2Settings[oldKey] = flatSettings[oldKey]; - flatKeys.delete(oldKey); - continue; - } - - setNestedProperty(v2Settings, newPath, flatSettings[oldKey]); - flatKeys.delete(oldKey); - } - } - - // Handle consolidated settings first (multiple old keys -> single new key) - // Policy: if ANY of the old disable* settings is true, the new enable* should be false - for (const [newPath, oldKeys] of Object.entries(CONSOLIDATED_SETTINGS)) { - let hasAnyDisable = false; - let hasAnyValue = false; - for (const oldKey of oldKeys) { - if (flatKeys.has(oldKey)) { - hasAnyValue = true; - const oldValue = flatSettings[oldKey]; - if (typeof oldValue === 'boolean' && oldValue === true) { - hasAnyDisable = true; - } - flatKeys.delete(oldKey); - } - } - if (hasAnyValue) { - // enableAutoUpdate = !hasAnyDisable (if any disable* was true, enable should be false) - setNestedProperty(v2Settings, newPath, !hasAnyDisable); - } - } - - // Handle remaining V1 settings that need boolean inversion (disable* -> enable*) - // Skip keys that were already handled by consolidated settings - const consolidatedKeys = new Set(Object.values(CONSOLIDATED_SETTINGS).flat()); - for (const [oldKey, newPath] of Object.entries(INVERTED_BOOLEAN_MIGRATIONS)) { - if (consolidatedKeys.has(oldKey)) { - continue; - } - if (flatKeys.has(oldKey)) { - const oldValue = flatSettings[oldKey]; - if (typeof oldValue === 'boolean') { - setNestedProperty(v2Settings, newPath, !oldValue); - } - 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, - {}, - newValue as MergeableObject, - existingValue as MergeableObject, - ); - } else { - v2Settings[remainingKey] = newValue; - } - } - - // Set version field to indicate this is a V2 settings file - v2Settings[SETTINGS_VERSION_KEY] = SETTINGS_VERSION; - - return v2Settings; -} - -// Migrate V2 settings to V3 (invert disable* -> enable* booleans) -function migrateV2ToV3( - settings: Record, -): Record | null { - const version = settings[SETTINGS_VERSION_KEY]; - if (typeof version === 'number' && version >= 3) { - return null; - } - - let changed = false; - const result = structuredClone(settings); - const processedPaths = new Set(); - - // Handle consolidated V2 paths first (multiple old paths -> single new path) - // Policy: if ANY of the old disable* settings is true, the new enable* should be false - for (const [newPath, oldPaths] of Object.entries(CONSOLIDATED_V2_PATHS)) { - let hasAnyDisable = false; - let hasAnyValue = false; - for (const oldPath of oldPaths) { - const oldValue = getNestedProperty(result, oldPath); - if (typeof oldValue === 'boolean') { - hasAnyValue = true; - if (oldValue === true) { - hasAnyDisable = true; - } - deleteNestedProperty(result, oldPath); - processedPaths.add(oldPath); - changed = true; - } - } - if (hasAnyValue) { - // enableAutoUpdate = !hasAnyDisable (if any disable* was true, enable should be false) - setNestedProperty(result, newPath, !hasAnyDisable); - } - } - - // Handle remaining V2 paths that need inversion - for (const [oldPath, newPath] of Object.entries(INVERTED_V2_PATHS)) { - if (processedPaths.has(oldPath)) { - continue; - } - const oldValue = getNestedProperty(result, oldPath); - if (typeof oldValue === 'boolean') { - // Remove old property - deleteNestedProperty(result, oldPath); - // Set new property with inverted value - setNestedProperty(result, newPath, !oldValue); - changed = true; - } - } - - if (changed) { - result[SETTINGS_VERSION_KEY] = SETTINGS_VERSION; - return result; - } - - // Even if no changes, bump version to 3 to skip future migration checks - if (typeof version === 'number' && version < SETTINGS_VERSION) { - result[SETTINGS_VERSION_KEY] = SETTINGS_VERSION; - return result; - } - - return null; -} - -function deleteNestedProperty( - obj: Record, - path: string, -): void { - const keys = path.split('.'); - const lastKey = keys.pop(); - if (!lastKey) return; - - let current: Record = obj; - for (const key of keys) { - const next = current[key]; - if (typeof next !== 'object' || next === null) { - return; - } - current = next as Record; - } - delete current[lastKey]; -} - -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]), -); - -// Reverse map for old V2 paths (before rename) to V1 keys. -// Used when migrating settings that still have old V2 naming (e.g., general.disableAutoUpdate). -const OLD_V2_TO_V1_MAP: Record = {}; -for (const [oldV2Path, newV3Path] of Object.entries(INVERTED_V2_PATHS)) { - // Find the V1 key that maps to this V3 path - for (const [v1Key, v3Path] of Object.entries(INVERTED_BOOLEAN_MIGRATIONS)) { - if (v3Path === newV3Path) { - OLD_V2_TO_V1_MAP[oldV2Path] = v1Key; - break; - } - } -} - -// Reverse map for new V3 paths to V1 keys (with boolean inversion). -// Used when migrating settings that have new V3 naming (e.g., general.enableAutoUpdate). -const V3_TO_V1_INVERTED_MAP: Record = Object.fromEntries( - Object.entries(INVERTED_BOOLEAN_MIGRATIONS).map(([v1Key, v3Path]) => [ - v3Path, - v1Key, - ]), -); - function getSettingsFileKeyWarnings( settings: Record, settingsFilePath: string, @@ -537,7 +162,7 @@ function getSettingsFileKeyWarnings( const ignoredLegacyKeys = new Set(); // Ignored legacy keys (V1 top-level keys that moved to a nested V2 path). - for (const [oldKey, newPath] of Object.entries(MIGRATION_MAP)) { + for (const [oldKey, newPath] of Object.entries(V1_TO_V2_MIGRATION_MAP)) { if (oldKey === newPath) { continue; } @@ -550,7 +175,7 @@ function getSettingsFileKeyWarnings( // 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) && + V2_CONTAINER_KEYS.has(oldKey) && typeof oldValue === 'object' && oldValue !== null && !Array.isArray(oldValue) @@ -586,7 +211,8 @@ function getSettingsFileKeyWarnings( } /** - * Collects warnings for ignored legacy and unknown settings keys. + * Collects warnings for ignored legacy and unknown settings keys, + * as well as migration warnings. * * For `$version: 2` settings files, we do not apply implicit migrations. * Instead, we surface actionable, de-duplicated warnings in the terminal UI. @@ -594,6 +220,11 @@ function getSettingsFileKeyWarnings( export function getSettingsWarnings(loadedSettings: LoadedSettings): string[] { const warningSet = new Set(); + // Add migration warnings first + for (const warning of loadedSettings.migrationWarnings) { + warningSet.add(`Warning: ${warning}`); + } + for (const scope of [SettingScope.User, SettingScope.Workspace]) { const settingsFile = loadedSettings.forScope(scope); if (settingsFile.rawJson === undefined) { @@ -616,75 +247,6 @@ export function getSettingsWarnings(loadedSettings: LoadedSettings): string[] { return [...warningSet]; } -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]); - } - } - - // Handle old V2 inverted paths (no value inversion needed) - // e.g., general.disableAutoUpdate -> disableAutoUpdate - for (const [oldV2Path, v1Key] of Object.entries(OLD_V2_TO_V1_MAP)) { - const value = getNestedProperty(v2Settings, oldV2Path); - if (value !== undefined) { - v1Settings[v1Key] = value; - v2Keys.delete(oldV2Path.split('.')[0]); - } - } - - // Handle new V3 inverted paths (WITH value inversion) - // e.g., general.enableAutoUpdate -> disableAutoUpdate (inverted) - for (const [v3Path, v1Key] of Object.entries(V3_TO_V1_INVERTED_MAP)) { - const value = getNestedProperty(v2Settings, v3Path); - if (value !== undefined && typeof value === 'boolean') { - v1Settings[v1Key] = !value; - v2Keys.delete(v3Path.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) { - // Skip the version field - it's only for V2 format - if (remainingKey === SETTINGS_VERSION_KEY) { - continue; - } - - 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; -} - function mergeSettings( system: Settings, systemDefaults: Settings, @@ -718,6 +280,7 @@ export class LoadedSettings { workspace: SettingsFile, isTrusted: boolean, migratedInMemorScopes: Set, + migrationWarnings: string[] = [], ) { this.system = system; this.systemDefaults = systemDefaults; @@ -725,6 +288,7 @@ export class LoadedSettings { this.workspace = workspace; this.isTrusted = isTrusted; this.migratedInMemorScopes = migratedInMemorScopes; + this.migrationWarnings = migrationWarnings; this._merged = this.computeMergedSettings(); } @@ -734,6 +298,7 @@ export class LoadedSettings { readonly workspace: SettingsFile; readonly isTrusted: boolean; readonly migratedInMemorScopes: Set; + readonly migrationWarnings: string[]; private _merged: Settings; @@ -793,6 +358,7 @@ export function createMinimalSettings(): LoadedSettings { emptySettingsFile, false, new Set(), + [], ); } @@ -933,6 +499,16 @@ export function loadEnvironment(settings: Settings): void { export function loadSettings( workspaceDir: string = process.cwd(), ): LoadedSettings { + // Set up a temporary debug log session for the startup phase. + // This allows migration errors to be logged to file instead of being + // exposed to users via stderr. The Config class will override this + // with the actual session once initialized. + const resolvedWorkspaceDir = path.resolve(workspaceDir); + const sanitizedProjectId = sanitizeCwd(resolvedWorkspaceDir); + setDebugLogSession({ + getSessionId: () => `startup-${sanitizedProjectId}`, + }); + let systemSettings: Settings = {}; let systemDefaultSettings: Settings = {}; let userSettings: Settings = {}; @@ -943,7 +519,7 @@ export function loadSettings( const migratedInMemorScopes = new Set(); // Resolve paths to their canonical representation to handle symlinks - const resolvedWorkspaceDir = path.resolve(workspaceDir); + // Note: resolvedWorkspaceDir is already defined at the top of the function const resolvedHomeDir = path.resolve(homedir()); let realWorkspaceDir = resolvedWorkspaceDir; @@ -964,7 +540,7 @@ export function loadSettings( const loadAndMigrate = ( filePath: string, scope: SettingScope, - ): { settings: Settings; rawJson?: string } => { + ): { settings: Settings; rawJson?: string; migrationWarnings?: string[] } => { try { if (fs.existsSync(filePath)) { const content = fs.readFileSync(filePath, 'utf-8'); @@ -983,74 +559,59 @@ export function loadSettings( } let settingsObject = rawSettings as Record; + const hasVersionKey = SETTINGS_VERSION_KEY in settingsObject; + const versionValue = settingsObject[SETTINGS_VERSION_KEY]; + const hasInvalidVersion = + hasVersionKey && typeof versionValue !== 'number'; + const hasLegacyNumericVersion = + typeof versionValue === 'number' && versionValue < SETTINGS_VERSION; + let migrationWarnings: string[] | undefined; + + const persistSettingsObject = (warningPrefix: string) => { + try { + writeWithBackupSync( + filePath, + JSON.stringify(settingsObject, null, 2), + ); + } catch (e) { + debugLogger.error(`${warningPrefix}: ${getErrorMessage(e)}`); + } + }; + if (needsMigration(settingsObject)) { - const migratedSettings = migrateV1ToV3(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) { - writeStderrLine( - `Error migrating settings file on disk: ${getErrorMessage( - e, - )}`, - ); - } - } else { - migratedInMemorScopes.add(scope); - } - settingsObject = migratedSettings; + const migrationResult = runMigrations(settingsObject, scope); + if (migrationResult.executedMigrations.length > 0) { + settingsObject = migrationResult.settings as Record< + string, + unknown + >; + migrationWarnings = migrationResult.warnings; + persistSettingsObject('Error migrating settings file on disk'); + } else if (hasLegacyNumericVersion || hasInvalidVersion) { + // Migration was deemed needed but nothing executed. Normalize version metadata + // to avoid repeated no-op checks on startup. + settingsObject[SETTINGS_VERSION_KEY] = SETTINGS_VERSION; + debugLogger.warn( + `Settings version metadata in ${filePath} could not be migrated by any registered migration. Normalizing ${SETTINGS_VERSION_KEY} to ${SETTINGS_VERSION}.`, + ); + persistSettingsObject('Error normalizing settings version on disk'); } - } else if (!(SETTINGS_VERSION_KEY in settingsObject)) { - // No migration needed, but version field is missing - add it for future optimizations + } else if ( + !hasVersionKey || + hasInvalidVersion || + hasLegacyNumericVersion + ) { + // No migration needed/executable, but version metadata is missing or invalid. + // Normalize it to current version to avoid repeated startup work. settingsObject[SETTINGS_VERSION_KEY] = SETTINGS_VERSION; - if (MIGRATE_V2_OVERWRITE) { - try { - fs.writeFileSync( - filePath, - JSON.stringify(settingsObject, null, 2), - 'utf-8', - ); - } catch (e) { - writeStderrLine( - `Error adding version to settings file: ${getErrorMessage(e)}`, - ); - } - } + persistSettingsObject('Error normalizing settings version on disk'); } - // V2 to V3 migration (invert disable* -> enable* booleans) - const v3Migrated = migrateV2ToV3(settingsObject); - if (v3Migrated) { - if (MIGRATE_V2_OVERWRITE) { - try { - // Only backup if not already backed up by V1->V2 migration - const backupPath = `${filePath}.orig`; - if (!fs.existsSync(backupPath)) { - fs.renameSync(filePath, backupPath); - } - fs.writeFileSync( - filePath, - JSON.stringify(v3Migrated, null, 2), - 'utf-8', - ); - } catch (e) { - writeStderrLine( - `Error migrating settings file to V3: ${getErrorMessage(e)}`, - ); - } - } else { - migratedInMemorScopes.add(scope); - } - settingsObject = v3Migrated; - } - - return { settings: settingsObject as Settings, rawJson: content }; + return { + settings: settingsObject as Settings, + rawJson: content, + migrationWarnings, + }; } } catch (error: unknown) { settingsErrors.push({ @@ -1068,7 +629,11 @@ export function loadSettings( ); const userResult = loadAndMigrate(USER_SETTINGS_PATH, SettingScope.User); - let workspaceResult: { settings: Settings; rawJson?: string } = { + let workspaceResult: { + settings: Settings; + rawJson?: string; + migrationWarnings?: string[]; + } = { settings: {} as Settings, rawJson: undefined, }; @@ -1138,6 +703,14 @@ export function loadSettings( ); } + // Collect all migration warnings from all scopes + const allMigrationWarnings: string[] = [ + ...(systemResult.migrationWarnings ?? []), + ...(systemDefaultsResult.migrationWarnings ?? []), + ...(userResult.migrationWarnings ?? []), + ...(workspaceResult.migrationWarnings ?? []), + ]; + return new LoadedSettings( { path: systemSettingsPath, @@ -1165,6 +738,7 @@ export function loadSettings( }, isTrusted, migratedInMemorScopes, + allMigrationWarnings, ); } @@ -1176,21 +750,14 @@ 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; - } - // Use the format-preserving update function updateSettingsFilePreservingFormat( settingsFile.path, - settingsToSave as Record, + settingsFile.originalSettings as Record, ); } catch (error) { - writeStderrLine('Error saving user settings file.'); - writeStderrLine(error instanceof Error ? error.message : String(error)); + debugLogger.error('Error saving user settings file.'); + debugLogger.error(error instanceof Error ? error.message : String(error)); throw error; } } diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 6c48658ad..928d89945 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -190,6 +190,7 @@ describe('gemini.tsx main function', () => { }, setValue: vi.fn(), forScope: () => ({ settings: {}, originalSettings: {}, path: '' }), + migrationWarnings: [], } as never); try { await main(); @@ -322,6 +323,7 @@ describe('gemini.tsx main function', () => { }, setValue: vi.fn(), forScope: () => ({ settings: {}, originalSettings: {}, path: '' }), + migrationWarnings: [], } as never); vi.mocked(parseArguments).mockResolvedValue({ @@ -452,6 +454,7 @@ describe('gemini.tsx main function kitty protocol', () => { }, setValue: vi.fn(), forScope: () => ({ settings: {}, originalSettings: {}, path: '' }), + migrationWarnings: [], } as never); vi.mocked(parseArguments).mockResolvedValue({ model: undefined, diff --git a/packages/cli/src/utils/writeWithBackup.test.ts b/packages/cli/src/utils/writeWithBackup.test.ts new file mode 100644 index 000000000..219bda81b --- /dev/null +++ b/packages/cli/src/utils/writeWithBackup.test.ts @@ -0,0 +1,232 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { writeWithBackup, writeWithBackupSync } from './writeWithBackup.js'; + +describe('writeWithBackup', () => { + let tempDir: string; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'writeWithBackup-test-')); + }); + + afterEach(() => { + // Clean up temp directory + try { + fs.rmSync(tempDir, { recursive: true, force: true }); + } catch (_e) { + // Ignore cleanup errors + } + }); + + describe('writeWithBackupSync', () => { + it('should write content to a new file', () => { + const targetPath = path.join(tempDir, 'test-file.txt'); + const content = 'Hello, World!'; + + writeWithBackupSync(targetPath, content); + + expect(fs.existsSync(targetPath)).toBe(true); + expect(fs.readFileSync(targetPath, 'utf-8')).toBe(content); + }); + + it('should backup existing file before writing', () => { + const targetPath = path.join(tempDir, 'test-file.txt'); + const originalContent = 'Original content'; + const newContent = 'New content'; + + fs.writeFileSync(targetPath, originalContent); + writeWithBackupSync(targetPath, newContent); + + expect(fs.readFileSync(targetPath, 'utf-8')).toBe(newContent); + expect(fs.existsSync(`${targetPath}.orig`)).toBe(true); + expect(fs.readFileSync(`${targetPath}.orig`, 'utf-8')).toBe( + originalContent, + ); + }); + + it('should use custom backup suffix', () => { + const targetPath = path.join(tempDir, 'test-file.txt'); + const originalContent = 'Original'; + + fs.writeFileSync(targetPath, originalContent); + writeWithBackupSync(targetPath, 'New', { backupSuffix: '.bak' }); + + expect(fs.existsSync(`${targetPath}.bak`)).toBe(true); + expect(fs.existsSync(`${targetPath}.orig`)).toBe(false); + }); + + it('should clean up temp file on failure', () => { + const targetPath = path.join(tempDir, 'test-file.txt'); + const tempPath = `${targetPath}.tmp`; + + // Create a situation where rename will fail (e.g., by creating a directory at target) + fs.mkdirSync(targetPath); + + expect(() => writeWithBackupSync(targetPath, 'content')).toThrow(); + expect(fs.existsSync(tempPath)).toBe(false); + }); + + it('should preserve original file content when write fails after backup', () => { + const targetPath = path.join(tempDir, 'test-file.txt'); + const originalContent = 'Original content that must be preserved'; + + // Create original file + fs.writeFileSync(targetPath, originalContent); + + // Create a situation where rename will fail (by creating a directory at temp path) + const tempPath = `${targetPath}.tmp`; + fs.mkdirSync(tempPath); + + // The write should fail + expect(() => writeWithBackupSync(targetPath, 'New content')).toThrow(); + + // Original file should still exist with original content + expect(fs.existsSync(targetPath)).toBe(true); + expect(fs.statSync(targetPath).isFile()).toBe(true); + expect(fs.readFileSync(targetPath, 'utf-8')).toBe(originalContent); + + // Cleanup + fs.rmdirSync(tempPath); + }); + + it('should restore original file from backup when rename fails', () => { + const targetPath = path.join(tempDir, 'test-file.txt'); + const backupPath = `${targetPath}.orig`; + const originalContent = 'Original content'; + const newContent = 'New content'; + + // Create original file + fs.writeFileSync(targetPath, originalContent); + + // Write new content successfully first + writeWithBackupSync(targetPath, newContent); + + // Verify backup exists with original content + expect(fs.existsSync(backupPath)).toBe(true); + expect(fs.readFileSync(backupPath, 'utf-8')).toBe(originalContent); + + // Verify target has new content + expect(fs.readFileSync(targetPath, 'utf-8')).toBe(newContent); + + // Now simulate a failure scenario: delete target and try to restore from backup + fs.unlinkSync(targetPath); + + // Restore from backup manually to verify backup integrity + fs.copyFileSync(backupPath, targetPath); + expect(fs.readFileSync(targetPath, 'utf-8')).toBe(originalContent); + }); + + it('should include recovery information in error message', () => { + const targetPath = path.join(tempDir, 'test-file.txt'); + + // Create a situation where rename will fail (directory at target) + fs.mkdirSync(targetPath); + + let errorMessage = ''; + try { + writeWithBackupSync(targetPath, 'content'); + } catch (error) { + errorMessage = error instanceof Error ? error.message : String(error); + } + + // Error message should be descriptive + expect(errorMessage).toContain('directory'); + expect(errorMessage.length).toBeGreaterThan(10); + }); + + it('should handle backup failure with descriptive error', () => { + const targetPath = path.join(tempDir, 'test-file.txt'); + const backupPath = `${targetPath}.orig`; + const originalContent = 'Original content'; + + // Create original file + fs.writeFileSync(targetPath, originalContent); + + // Create a directory at backup path to cause backup to fail + fs.mkdirSync(backupPath); + + let errorMessage = ''; + try { + writeWithBackupSync(targetPath, 'New content'); + } catch (error) { + errorMessage = error instanceof Error ? error.message : String(error); + } + + // Error message should mention backup failure + expect(errorMessage).toContain('backup'); + + // Original file should still exist + expect(fs.existsSync(targetPath)).toBe(true); + expect(fs.readFileSync(targetPath, 'utf-8')).toBe(originalContent); + + // Cleanup + fs.rmdirSync(backupPath); + }); + + it('should clean up temp file when backup creation fails', () => { + const targetPath = path.join(tempDir, 'test-file.txt'); + const tempPath = `${targetPath}.tmp`; + const backupPath = `${targetPath}.orig`; + const originalContent = 'Original content'; + + // Create original file + fs.writeFileSync(targetPath, originalContent); + + // Create a directory at backup path to cause backup to fail + fs.mkdirSync(backupPath); + + // The write should fail + expect(() => writeWithBackupSync(targetPath, 'New content')).toThrow(); + + // Temp file should be cleaned up + expect(fs.existsSync(tempPath)).toBe(false); + + // Cleanup + fs.rmdirSync(backupPath); + }); + }); + + describe('writeWithBackup (async)', () => { + it('should write content to a new file', async () => { + const targetPath = path.join(tempDir, 'test-file.txt'); + const content = 'Hello, World!'; + + await writeWithBackup(targetPath, content); + + expect(fs.existsSync(targetPath)).toBe(true); + expect(fs.readFileSync(targetPath, 'utf-8')).toBe(content); + }); + + it('should backup existing file before writing', async () => { + const targetPath = path.join(tempDir, 'test-file.txt'); + const originalContent = 'Original content'; + const newContent = 'New content'; + + fs.writeFileSync(targetPath, originalContent); + await writeWithBackup(targetPath, newContent); + + expect(fs.readFileSync(targetPath, 'utf-8')).toBe(newContent); + expect(fs.existsSync(`${targetPath}.orig`)).toBe(true); + expect(fs.readFileSync(`${targetPath}.orig`, 'utf-8')).toBe( + originalContent, + ); + }); + + it('should use custom encoding', async () => { + const targetPath = path.join(tempDir, 'test-file.txt'); + const content = 'Hello, World!'; + + await writeWithBackup(targetPath, content, { encoding: 'utf8' }); + + expect(fs.readFileSync(targetPath, 'utf-8')).toBe(content); + }); + }); +}); diff --git a/packages/cli/src/utils/writeWithBackup.ts b/packages/cli/src/utils/writeWithBackup.ts new file mode 100644 index 000000000..2c341ae38 --- /dev/null +++ b/packages/cli/src/utils/writeWithBackup.ts @@ -0,0 +1,169 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'node:fs'; + +/** + * Options for writeWithBackup function. + */ +export interface WriteWithBackupOptions { + /** Suffix for backup file (default: '.orig') */ + backupSuffix?: string; + /** File encoding (default: 'utf-8') */ + encoding?: BufferEncoding; +} + +/** + * Safely writes content to a file with backup protection. + * + * This function ensures data safety by: + * 1. Writing content to a temporary file first + * 2. Backing up the existing target file (if any) + * 3. Renaming the temporary file to the target path + * + * If any step fails, an error is thrown and no partial changes are left on disk. + * The backup file (if created) can be used for manual recovery. + * + * Note: This is not 100% atomic but provides good protection. In the worst case, + * a .orig backup file remains that can be manually restored. + * + * @param targetPath - The path to write to + * @param content - The content to write + * @param options - Optional configuration + * @throws Error if any step of the write process fails + * + * @example + * ```typescript + * await writeWithBackup('/path/to/settings.json', JSON.stringify(settings, null, 2)); + * // If /path/to/settings.json existed, it's now backed up to /path/to/settings.json.orig + * ``` + */ +export async function writeWithBackup( + targetPath: string, + content: string, + options: WriteWithBackupOptions = {}, +): Promise { + // Async version delegates to sync version since file operations are synchronous + writeWithBackupSync(targetPath, content, options); +} + +/** + * Synchronous version of writeWithBackup. + * + * @param targetPath - The path to write to + * @param content - The content to write + * @param options - Optional configuration + * @throws Error if any step of the write process fails + */ +export function writeWithBackupSync( + targetPath: string, + content: string, + options: WriteWithBackupOptions = {}, +): void { + const { backupSuffix = '.orig', encoding = 'utf-8' } = options; + const tempPath = `${targetPath}.tmp`; + const backupPath = `${targetPath}${backupSuffix}`; + + // Clean up any existing temp file from previous failed attempts + try { + if (fs.existsSync(tempPath)) { + fs.unlinkSync(tempPath); + } + } catch (_e) { + // Ignore cleanup errors + } + + try { + // Step 1: Write to temporary file + fs.writeFileSync(tempPath, content, { encoding }); + + // Step 2: If target exists, back it up + if (fs.existsSync(targetPath)) { + // Check if target is a directory - we can't write to a directory + const targetStat = fs.statSync(targetPath); + if (targetStat.isDirectory()) { + // Clean up temp file before throwing + try { + fs.unlinkSync(tempPath); + } catch (_e) { + // Ignore cleanup error + } + throw new Error( + `Cannot write to '${targetPath}' because it is a directory`, + ); + } + + try { + fs.renameSync(targetPath, backupPath); + } catch (backupError) { + // Clean up temp file before throwing + try { + fs.unlinkSync(tempPath); + } catch (_e) { + // Ignore cleanup error + } + throw new Error( + `Failed to backup existing file: ${backupError instanceof Error ? backupError.message : String(backupError)}`, + ); + } + } + + // Step 3: Rename temp file to target + try { + fs.renameSync(tempPath, targetPath); + } catch (renameError) { + let restoreFailedMessage: string | undefined; + let backupExisted = false; + + // Attempt to restore backup if rename failed + if (fs.existsSync(backupPath)) { + backupExisted = true; + try { + fs.renameSync(backupPath, targetPath); + } catch (restoreError) { + restoreFailedMessage = + restoreError instanceof Error + ? restoreError.message + : String(restoreError); + } + } + + const writeFailureMessage = + renameError instanceof Error + ? renameError.message + : String(renameError); + + if (restoreFailedMessage) { + throw new Error( + `Failed to write file: ${writeFailureMessage}. ` + + `Automatic restore failed: ${restoreFailedMessage}. ` + + `Manual recovery may be required using backup file '${backupPath}'.`, + ); + } + + if (backupExisted) { + throw new Error( + `Failed to write file: ${writeFailureMessage}. ` + + `Target was automatically restored from backup '${backupPath}'.`, + ); + } + + throw new Error( + `Failed to write file: ${writeFailureMessage}. No backup file was available for restoration.`, + ); + } + } catch (error) { + // Ensure temp file is cleaned up on any error + try { + if (fs.existsSync(tempPath)) { + fs.unlinkSync(tempPath); + } + } catch (_e) { + // Ignore cleanup error + } + throw error; + } +} diff --git a/packages/core/src/config/storage.ts b/packages/core/src/config/storage.ts index f9d0107e5..3293280a8 100644 --- a/packages/core/src/config/storage.ts +++ b/packages/core/src/config/storage.ts @@ -7,7 +7,7 @@ import * as path from 'node:path'; import * as os from 'node:os'; import * as fs from 'node:fs'; -import { getProjectHash } from '../utils/paths.js'; +import { getProjectHash, sanitizeCwd } from '../utils/paths.js'; export const QWEN_DIR = '.qwen'; export const GOOGLE_ACCOUNTS_FILENAME = 'google_accounts.json'; @@ -82,7 +82,7 @@ export class Storage { } getProjectDir(): string { - const projectId = this.sanitizeCwd(this.getProjectRoot()); + const projectId = sanitizeCwd(this.getProjectRoot()); const projectsDir = path.join(Storage.getGlobalQwenDir(), PROJECT_DIR_NAME); return path.join(projectsDir, projectId); } @@ -140,10 +140,4 @@ export class Storage { getHistoryFilePath(): string { return path.join(this.getProjectTempDir(), 'shell_history'); } - - private sanitizeCwd(cwd: string): string { - // On Windows, normalize to lowercase for case-insensitive matching - const normalizedCwd = os.platform() === 'win32' ? cwd.toLowerCase() : cwd; - return normalizedCwd.replace(/[^a-zA-Z0-9]/g, '-'); - } } diff --git a/packages/core/src/utils/paths.ts b/packages/core/src/utils/paths.ts index 96856a5dc..dc4434ece 100644 --- a/packages/core/src/utils/paths.ts +++ b/packages/core/src/utils/paths.ts @@ -202,6 +202,25 @@ export function getProjectHash(projectRoot: string): string { return crypto.createHash('sha256').update(normalizedPath).digest('hex'); } +/** + * Sanitizes a directory path to create a safe project ID. + * + * - On Windows: normalizes to lowercase for case-insensitive matching + * - Replaces all non-alphanumeric characters with hyphens + * + * This is used for: + * - Creating project-specific directories + * - Generating session IDs for debug logging during startup + * + * @param cwd - The directory path to sanitize + * @returns A sanitized string safe for use as a project identifier + */ +export function sanitizeCwd(cwd: string): string { + // On Windows, normalize to lowercase for case-insensitive matching + const normalizedCwd = os.platform() === 'win32' ? cwd.toLowerCase() : cwd; + return normalizedCwd.replace(/[^a-zA-Z0-9]/g, '-'); +} + /** * Checks if a path is a subpath of another path. * @param parentPath The parent path.