Merge pull request #2455 from Sakuranda/fix/2454-model-settings-preservation

fix(cli): preserve runtime-added models when saving settings

Author: tanzhenxin

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

@@ -48,6 +48,7 @@ import {
   USER_SETTINGS_PATH, // This IS the mocked path.
   getSystemSettingsPath,
   getSystemDefaultsPath,
+  SettingScope,
   SETTINGS_DIRECTORY_NAME, // This is from the original module, but used by the mock.
   type Settings,
   loadEnvironment,
@@ -2334,6 +2335,85 @@ describe('Settings Loading and Merging', () => {
     });
   });
 
+  describe('setValue persistence', () => {
+    it('preserves models added to settings.json after startup when updating model.name', () => {
+      (mockFsExistsSync as Mock).mockReturnValue(true);
+
+      const initialUserSettingsContent = {
+        [SETTINGS_VERSION_KEY]: SETTINGS_VERSION,
+        modelProviders: {
+          openai: [
+            {
+              id: 'existing-model',
+              name: 'Existing Model',
+              baseUrl: 'https://example.com/v1',
+              envKey: 'OPENAI_API_KEY',
+            },
+          ],
+        },
+        model: {
+          name: 'existing-model',
+        },
+      };
+
+      const externallyModifiedUserSettingsContent = {
+        [SETTINGS_VERSION_KEY]: SETTINGS_VERSION,
+        modelProviders: {
+          openai: [
+            {
+              id: 'existing-model',
+              name: 'Existing Model',
+              baseUrl: 'https://example.com/v1',
+              envKey: 'OPENAI_API_KEY',
+            },
+            {
+              id: 'manually-added-model',
+              name: 'Manually Added Model',
+              baseUrl: 'https://example.com/v1',
+              envKey: 'OPENAI_API_KEY',
+            },
+          ],
+        },
+        model: {
+          name: 'existing-model',
+        },
+      };
+
+      let currentUserSettingsContent = JSON.stringify(
+        initialUserSettingsContent,
+      );
+
+      (fs.readFileSync as Mock).mockImplementation(
+        (p: fs.PathOrFileDescriptor) => {
+          if (p === USER_SETTINGS_PATH) {
+            return currentUserSettingsContent;
+          }
+          return '{}';
+        },
+      );
+
+      const settings = loadSettings(MOCK_WORKSPACE_DIR);
+      currentUserSettingsContent = JSON.stringify(
+        externallyModifiedUserSettingsContent,
+      );
+
+      settings.setValue(
+        SettingScope.User,
+        'model.name',
+        'manually-added-model',
+      );
+
+      const writeCall = (fs.writeFileSync as Mock).mock.calls.at(-1);
+      expect(writeCall).toBeDefined();
+
+      const writtenContent = JSON.parse(String(writeCall?.[1]));
+      expect(writtenContent.model.name).toBe('manually-added-model');
+      expect(writtenContent.modelProviders.openai).toEqual(
+        externallyModifiedUserSettingsContent.modelProviders.openai,
+      );
+    });
+  });
+
   describe('loadEnvironment', () => {
     function setup({
       isFolderTrustEnabled = true,

packages/cli/src/config/settings.ts

@@ -375,7 +375,7 @@ export class LoadedSettings {
     setNestedPropertySafe(settingsFile.settings, key, value);
     setNestedPropertySafe(settingsFile.originalSettings, key, value);
     this._merged = this.computeMergedSettings();
-    saveSettings(settingsFile);
+    saveSettings(settingsFile, createSettingsUpdate(key, value));
   }
 }
 
@@ -771,7 +771,22 @@ export function loadSettings(
   );
 }
 
-export function saveSettings(settingsFile: SettingsFile): void {
+function createSettingsUpdate(
+  key: string,
+  value: unknown,
+): Record<string, unknown> {
+  const root: Record<string, unknown> = {};
+  setNestedPropertySafe(root, key, value);
+  return root;
+}
+
+export function saveSettings(
+  settingsFile: SettingsFile,
+  updates: Record<string, unknown> = settingsFile.originalSettings as Record<
+    string,
+    unknown
+  >,
+): void {
   try {
     // Ensure the directory exists
     const dirPath = path.dirname(settingsFile.path);
@@ -780,10 +795,7 @@ export function saveSettings(settingsFile: SettingsFile): void {
     }
 
     // Use the format-preserving update function
-    updateSettingsFilePreservingFormat(
-      settingsFile.path,
-      settingsFile.originalSettings as Record<string, unknown>,
-    );
+    updateSettingsFilePreservingFormat(settingsFile.path, updates);
   } catch (error) {
     debugLogger.error('Error saving user settings file.');
     debugLogger.error(error instanceof Error ? error.message : String(error));