fix: prevent data loss during V2 to V1 settings migration

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<string, unknown> = {
+        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<string, unknown>)?.['newFeatureFlag'],
+      ).toBe(true);
+      expect(
+        (
+          (v1Settings['security'] as Record<string, unknown>)?.[
+            'auth'
+          ] as Record<string, unknown>
+        )?.['blackbox'],
+      ).toEqual({ apiKey: 'secret' });
+    });
   });
 });

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<string, unknown>,
 ): Record<string, unknown> {
   const v1Settings: Record<string, unknown> = {};
-  const v2Keys = new Set(Object.keys(v2Settings));
+  const consumedPaths = new Set<string>();
 
   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<string, unknown> | undefined;
-  if (security && typeof security === 'object') {
-    const auth = security['auth'] as Record<string, unknown> | undefined;
-    if (auth && typeof auth === 'object') {
-      // Preserve the entire security.auth structure with provider credentials
-      if (!v1Settings['security']) {
-        v1Settings['security'] = {};
-      }
-      (v1Settings['security'] as Record<string, unknown>)['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<string, unknown>;
+      }
+    } 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<string, unknown>;
+    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,