From ba49b54a1986d071fd7d874727f547971525bc93 Mon Sep 17 00:00:00 2001 From: SK Akram Date: Tue, 23 Dec 2025 06:25:30 +0000 Subject: [PATCH] fix: prevent data loss during V2 to V1 settings migration --- packages/cli/src/config/settings.test.ts | 113 +++++++++++++++++++++-- packages/cli/src/config/settings.ts | 91 ++++++++++++------ 2 files changed, 166 insertions(+), 38 deletions(-) diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index 5335df0..dd07fae 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -148,7 +148,13 @@ describe('Settings Loading and Merging', () => { disabled: [], workspacesWithMigrationNudge: [], }, - security: {}, + security: { + auth: { + blackbox: {}, + openai: {}, + openrouter: {}, + }, + }, general: {}, privacy: {}, telemetry: {}, @@ -211,7 +217,13 @@ describe('Settings Loading and Merging', () => { disabled: [], workspacesWithMigrationNudge: [], }, - security: {}, + security: { + auth: { + blackbox: {}, + openai: {}, + openrouter: {}, + }, + }, general: {}, privacy: {}, telemetry: {}, @@ -277,7 +289,13 @@ describe('Settings Loading and Merging', () => { disabled: [], workspacesWithMigrationNudge: [], }, - security: {}, + security: { + auth: { + blackbox: {}, + openai: {}, + openrouter: {}, + }, + }, general: {}, privacy: {}, telemetry: {}, @@ -340,7 +358,13 @@ describe('Settings Loading and Merging', () => { disabled: [], workspacesWithMigrationNudge: [], }, - security: {}, + security: { + auth: { + blackbox: {}, + openai: {}, + openrouter: {}, + }, + }, general: {}, privacy: {}, telemetry: {}, @@ -413,7 +437,13 @@ describe('Settings Loading and Merging', () => { model: { chatCompression: {}, }, - security: {}, + security: { + auth: { + blackbox: {}, + openai: {}, + openrouter: {}, + }, + }, general: {}, privacy: {}, telemetry: {}, @@ -507,7 +537,13 @@ describe('Settings Loading and Merging', () => { model: { chatCompression: {}, }, - security: {}, + security: { + auth: { + blackbox: {}, + openai: {}, + openrouter: {}, + }, + }, general: {}, privacy: {}, ide: {}, @@ -591,7 +627,13 @@ describe('Settings Loading and Merging', () => { disabled: [], workspacesWithMigrationNudge: [], }, - security: {}, + security: { + auth: { + blackbox: {}, + openai: {}, + openrouter: {}, + }, + }, privacy: {}, telemetry: {}, tools: {}, @@ -728,7 +770,13 @@ describe('Settings Loading and Merging', () => { model: { chatCompression: {}, }, - security: {}, + security: { + auth: { + blackbox: {}, + openai: {}, + openrouter: {}, + }, + }, telemetry: {}, tools: { sandbox: false, @@ -1466,7 +1514,13 @@ describe('Settings Loading and Merging', () => { disabled: [], workspacesWithMigrationNudge: [], }, - security: {}, + security: { + auth: { + blackbox: {}, + openai: {}, + openrouter: {}, + }, + }, general: {}, privacy: {}, tools: {}, @@ -1926,7 +1980,13 @@ describe('Settings Loading and Merging', () => { disabled: [], workspacesWithMigrationNudge: [], }, - security: {}, + security: { + auth: { + blackbox: {}, + openai: {}, + openrouter: {}, + }, + }, general: {}, privacy: {}, telemetry: {}, @@ -2465,5 +2525,38 @@ describe('Settings Loading and Merging', () => { allowMCPServers: ['serverA'], }); }); + + it('should preserve unmapped nested properties during partial migration', () => { + const v2Settings: Record = { + general: { + preferredEditor: 'vscode', + newFeatureFlag: true, + }, + security: { + folderTrust: { enabled: true }, + auth: { + selectedType: 'api-key', + blackbox: { apiKey: 'secret' }, + }, + }, + }; + + const v1Settings = migrateSettingsToV1(v2Settings); + + expect(v1Settings['preferredEditor']).toBe('vscode'); + expect(v1Settings['folderTrust']).toBe(true); + expect(v1Settings['selectedAuthType']).toBe('api-key'); + + expect( + (v1Settings['general'] as Record)?.['newFeatureFlag'], + ).toBe(true); + expect( + ( + (v1Settings['security'] as Record)?.[ + 'auth' + ] as Record + )?.['blackbox'], + ).toEqual({ apiKey: 'secret' }); + }); }); }); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index d1231f2..2a41f8c 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -18,7 +18,7 @@ import { DefaultLight } from '../ui/themes/default-light.js'; import { DefaultDark } from '../ui/themes/default.js'; import { isWorkspaceTrusted } from './trustedFolders.js'; import type { Settings, MemoryImportFormat } from './settingsSchema.js'; -import { mergeWith } from 'lodash-es'; +import { mergeWith, unset } from 'lodash-es'; export type { Settings, MemoryImportFormat }; @@ -222,60 +222,95 @@ export function migrateSettingsToV1( v2Settings: Record, ): Record { const v1Settings: Record = {}; - const v2Keys = new Set(Object.keys(v2Settings)); + const consumedPaths = new Set(); 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]); + consumedPaths.add(newPath); } } // Preserve mcpServers at the top level if (v2Settings['mcpServers']) { v1Settings['mcpServers'] = v2Settings['mcpServers']; - v2Keys.delete('mcpServers'); - } - - // Preserve provider credentials in security.auth (V2 format) - // These are new fields that don't have V1 equivalents, so we keep them in V2 format - const security = v2Settings['security'] as Record | undefined; - if (security && typeof security === 'object') { - const auth = security['auth'] as Record | undefined; - if (auth && typeof auth === 'object') { - // Preserve the entire security.auth structure with provider credentials - if (!v1Settings['security']) { - v1Settings['security'] = {}; - } - (v1Settings['security'] as Record)['auth'] = auth; - } + consumedPaths.add('mcpServers'); } - // Carry over any unrecognized keys - for (const remainingKey of v2Keys) { - const value = v2Settings[remainingKey]; + for (const key of Object.keys(v2Settings)) { + const value = v2Settings[key]; if (value === undefined) { continue; } - // Don't carry over empty objects that were just containers for migrated settings. + if (consumedPaths.has(key)) { + continue; + } + + const consumedChildren = Array.from(consumedPaths).filter((p) => + p.startsWith(`${key}.`), + ); + if ( - KNOWN_V2_CONTAINERS.has(remainingKey) && + consumedChildren.length > 0 && typeof value === 'object' && value !== null && - !Array.isArray(value) && - Object.keys(value).length === 0 + !Array.isArray(value) ) { - continue; - } + const remaining = structuredClone(value); + for (const childPath of consumedChildren) { + const relativePath = childPath.slice(key.length + 1); + unset(remaining, relativePath); + } + + const cleaned = removeEmptyObjects(remaining); - v1Settings[remainingKey] = value; + if ( + typeof cleaned === 'object' && + cleaned !== null && + Object.keys(cleaned).length > 0 + ) { + v1Settings[key] = cleaned as Record; + } + } else { + if ( + KNOWN_V2_CONTAINERS.has(key) && + typeof value === 'object' && + value !== null && + !Array.isArray(value) && + Object.keys(value).length === 0 + ) { + continue; + } + v1Settings[key] = value; + } } return v1Settings; } +function removeEmptyObjects(obj: unknown): unknown { + if (typeof obj === 'object' && obj !== null && !Array.isArray(obj)) { + const newObj = { ...obj } as Record; + for (const key of Object.keys(newObj)) { + const cleanVal = removeEmptyObjects(newObj[key]); + if ( + typeof cleanVal === 'object' && + cleanVal !== null && + !Array.isArray(cleanVal) && + Object.keys(cleanVal).length === 0 + ) { + delete newObj[key]; + } else { + newObj[key] = cleanVal; + } + } + return newObj; + } + return obj; +} + function mergeSettings( system: Settings, systemDefaults: Settings,