Fix setting migration nosiness and merging (#7571)

packages/cli/src/config/settings.test.ts

@@ -142,6 +142,11 @@ describe('Settings Loading and Merging', () => {
           workspacesWithMigrationNudge: [],
         },
         security: {},
+        general: {},
+        privacy: {},
+        telemetry: {},
+        tools: {},
+        ide: {},
       });
       expect(settings.errors.length).toBe(0);
     });
@@ -197,6 +202,13 @@ describe('Settings Loading and Merging', () => {
           workspacesWithMigrationNudge: [],
         },
         security: {},
+        general: {},
+        privacy: {},
+        telemetry: {},
+        tools: {
+          sandbox: false,
+        },
+        ide: {},
       });
     });
 
@@ -253,6 +265,11 @@ describe('Settings Loading and Merging', () => {
           workspacesWithMigrationNudge: [],
         },
         security: {},
+        general: {},
+        privacy: {},
+        telemetry: {},
+        tools: {},
+        ide: {},
       });
     });
 
@@ -308,6 +325,10 @@ describe('Settings Loading and Merging', () => {
           workspacesWithMigrationNudge: [],
         },
         security: {},
+        general: {},
+        privacy: {},
+        telemetry: {},
+        ide: {},
       });
     });
 
@@ -374,6 +395,10 @@ describe('Settings Loading and Merging', () => {
           chatCompression: {},
         },
         security: {},
+        general: {},
+        privacy: {},
+        telemetry: {},
+        ide: {},
       });
     });
 
@@ -439,6 +464,7 @@ describe('Settings Loading and Merging', () => {
         },
         tools: {
           sandbox: false,
+          core: ['tool1'],
         },
         telemetry: { enabled: false },
         context: {
@@ -460,6 +486,9 @@ describe('Settings Loading and Merging', () => {
           chatCompression: {},
         },
         security: {},
+        general: {},
+        privacy: {},
+        ide: {},
       });
     });
 
@@ -538,6 +567,10 @@ describe('Settings Loading and Merging', () => {
           workspacesWithMigrationNudge: [],
         },
         security: {},
+        privacy: {},
+        telemetry: {},
+        tools: {},
+        ide: {},
       });
     });
 
@@ -668,7 +701,7 @@ describe('Settings Loading and Merging', () => {
           chatCompression: {},
         },
         security: {},
-        telemetry: false,
+        telemetry: {},
         tools: {
           sandbox: false,
         },
@@ -676,6 +709,9 @@ describe('Settings Loading and Merging', () => {
           customThemes: {},
           theme: 'system-theme',
         },
+        general: {},
+        privacy: {},
+        ide: {},
       });
     });
 
@@ -901,7 +937,7 @@ describe('Settings Loading and Merging', () => {
       (mockFsExistsSync as Mock).mockImplementation(
         (p: fs.PathLike) => p === USER_SETTINGS_PATH,
       );
-      const userSettingsContent = { telemetry: true };
+      const userSettingsContent = { telemetry: { enabled: true } };
       (fs.readFileSync as Mock).mockImplementation(
         (p: fs.PathOrFileDescriptor) => {
           if (p === USER_SETTINGS_PATH)
@@ -910,14 +946,14 @@ describe('Settings Loading and Merging', () => {
         },
       );
       const settings = loadSettings(MOCK_WORKSPACE_DIR);
-      expect(settings.merged.telemetry).toBe(true);
+      expect(settings.merged.telemetry?.enabled).toBe(true);
     });
 
     it('should load telemetry setting from workspace settings', () => {
       (mockFsExistsSync as Mock).mockImplementation(
         (p: fs.PathLike) => p === MOCK_WORKSPACE_SETTINGS_PATH,
       );
-      const workspaceSettingsContent = { telemetry: false };
+      const workspaceSettingsContent = { telemetry: { enabled: false } };
       (fs.readFileSync as Mock).mockImplementation(
         (p: fs.PathOrFileDescriptor) => {
           if (p === MOCK_WORKSPACE_SETTINGS_PATH)
@@ -926,13 +962,13 @@ describe('Settings Loading and Merging', () => {
         },
       );
       const settings = loadSettings(MOCK_WORKSPACE_DIR);
-      expect(settings.merged.telemetry).toBe(false);
+      expect(settings.merged.telemetry?.enabled).toBe(false);
     });
 
     it('should prioritize workspace telemetry setting over user setting', () => {
       (mockFsExistsSync as Mock).mockReturnValue(true);
-      const userSettingsContent = { telemetry: true };
+      const userSettingsContent = { telemetry: { enabled: true } };
-      const workspaceSettingsContent = { telemetry: false };
+      const workspaceSettingsContent = { telemetry: { enabled: false } };
       (fs.readFileSync as Mock).mockImplementation(
         (p: fs.PathOrFileDescriptor) => {
           if (p === USER_SETTINGS_PATH)
@@ -943,14 +979,14 @@ describe('Settings Loading and Merging', () => {
         },
       );
       const settings = loadSettings(MOCK_WORKSPACE_DIR);
-      expect(settings.merged.telemetry).toBe(false);
+      expect(settings.merged.telemetry?.enabled).toBe(false);
     });
 
     it('should have telemetry as undefined if not in any settings file', () => {
       (mockFsExistsSync as Mock).mockReturnValue(false); // No settings files exist
       (fs.readFileSync as Mock).mockReturnValue('{}');
       const settings = loadSettings(MOCK_WORKSPACE_DIR);
-      expect(settings.merged.telemetry).toBeUndefined();
+      expect(settings.merged.telemetry).toEqual({});
       expect(settings.merged.ui?.customThemes).toEqual({});
       expect(settings.merged.mcpServers).toEqual({});
     });
@@ -1400,6 +1436,11 @@ describe('Settings Loading and Merging', () => {
           workspacesWithMigrationNudge: [],
         },
         security: {},
+        general: {},
+        privacy: {},
+        tools: {},
+        telemetry: {},
+        ide: {},
       });
 
       // Check that error objects are populated in settings.errors
@@ -1828,6 +1869,10 @@ describe('Settings Loading and Merging', () => {
             workspacesWithMigrationNudge: [],
           },
           security: {},
+          general: {},
+          privacy: {},
+          telemetry: {},
+          ide: {},
         });
       });
     });

packages/cli/src/config/settings.ts

@@ -30,7 +30,6 @@ export const DEFAULT_EXCLUDED_ENV_VARS = ['DEBUG', 'DEBUG_MODE'];
 
 const MIGRATE_V2_OVERWRITE = false;
 
-// As defined in spec.md
 const MIGRATION_MAP: Record<string, string> = {
   preferredEditor: 'general.preferredEditor',
   vimMode: 'general.vimMode',
@@ -63,6 +62,7 @@ const MIGRATION_MAP: Record<string, string> = {
   fileFiltering: 'context.fileFiltering',
   sandbox: 'tools.sandbox',
   shouldUseNodePtyShell: 'tools.usePty',
+  allowedTools: 'tools.allowed',
   coreTools: 'tools.core',
   excludeTools: 'tools.exclude',
   toolDiscoveryCommand: 'tools.discoveryCommand',
@@ -299,6 +299,12 @@ function mergeSettings(
     ...user,
     ...safeWorkspaceWithoutFolderTrust,
     ...system,
+    general: {
+      ...(systemDefaults.general || {}),
+      ...(user.general || {}),
+      ...(safeWorkspaceWithoutFolderTrust.general || {}),
+      ...(system.general || {}),
+    },
     ui: {
       ...(systemDefaults.ui || {}),
       ...(user.ui || {}),
@@ -311,6 +317,24 @@ function mergeSettings(
         ...(system.ui?.customThemes || {}),
       },
     },
+    ide: {
+      ...(systemDefaults.ide || {}),
+      ...(user.ide || {}),
+      ...(safeWorkspaceWithoutFolderTrust.ide || {}),
+      ...(system.ide || {}),
+    },
+    privacy: {
+      ...(systemDefaults.privacy || {}),
+      ...(user.privacy || {}),
+      ...(safeWorkspaceWithoutFolderTrust.privacy || {}),
+      ...(system.privacy || {}),
+    },
+    telemetry: {
+      ...(systemDefaults.telemetry || {}),
+      ...(user.telemetry || {}),
+      ...(safeWorkspaceWithoutFolderTrust.telemetry || {}),
+      ...(system.telemetry || {}),
+    },
     security: {
       ...(systemDefaults.security || {}),
       ...(user.security || {}),
@@ -329,6 +353,12 @@ function mergeSettings(
       ...(safeWorkspaceWithoutFolderTrust.mcpServers || {}),
       ...(system.mcpServers || {}),
     },
+    tools: {
+      ...(systemDefaults.tools || {}),
+      ...(user.tools || {}),
+      ...(safeWorkspaceWithoutFolderTrust.tools || {}),
+      ...(system.tools || {}),
+    },
     context: {
       ...(systemDefaults.context || {}),
       ...(user.context || {}),
@@ -667,7 +697,6 @@ export function loadSettings(workspaceDir: string): LoadedSettings {
 
         let settingsObject = rawSettings as Record<string, unknown>;
         if (needsMigration(settingsObject)) {
-          console.error(`Legacy settings file detected at: ${filePath}`);
           const migratedSettings = migrateSettingsToV2(settingsObject);
           if (migratedSettings) {
             if (MIGRATE_V2_OVERWRITE) {
@@ -678,9 +707,6 @@ export function loadSettings(workspaceDir: string): LoadedSettings {
                   JSON.stringify(migratedSettings, null, 2),
                   'utf-8',
                 );
-                console.log(
-                  `Successfully migrated and saved settings file: ${filePath}`,
-                );
               } catch (e) {
                 console.error(
                   `Error migrating settings file on disk: ${getErrorMessage(
@@ -689,9 +715,6 @@ export function loadSettings(workspaceDir: string): LoadedSettings {
                 );
               }
             } else {
-              console.log(
-                `Successfully migrated settings for ${filePath} in-memory for the current session.`,
-              );
               migratedInMemorScopes.add(scope);
             }
             settingsObject = migratedSettings;