fix(cli): preserve manually added configurations in settings.json

packages/cli/src/commands/mcp/remove.ts

@@ -29,7 +29,7 @@ async function removeMcpServer(
     return;
   }
 
-  delete mcpServers[name];
+  (mcpServers as Record<string, unknown>)[name] = undefined;
 
   settings.setValue(settingsScope, 'mcpServers', mcpServers);
 

packages/cli/src/config/settings.ts

@@ -868,14 +868,14 @@ export function migrateDeprecatedSettings(
       if (typeof newValue === 'boolean') {
         // Both exist, trust the new one
         if (removeDeprecated) {
-          delete settings[oldKey];
+          settings[oldKey] = undefined;
           modified = true;
         }
       } else {
         // Only old exists, migrate to new (inverted)
         settings[newKey] = !oldValue;
         if (removeDeprecated) {
-          delete settings[oldKey];
+          settings[oldKey] = undefined;
         }
         modified = true;
       }
@@ -1017,8 +1017,7 @@ export function migrateDeprecatedSettings(
         }
 
         if (removeDeprecated) {
-          const newTools = { ...toolsSettings };
-          delete newTools['approvalMode'];
+          const newTools = { ...toolsSettings, approvalMode: undefined };
           loadedSettings.setValue(scope, 'tools', newTools);
           if (!settingsFile.readOnly) {
             anyModified = true;
@@ -1229,10 +1228,12 @@ function migrateExperimentalSettings(
       loadedSettings.setValue(scope, 'agents', agentsSettings);
 
       if (removeDeprecated) {
-        const newExperimental = { ...experimentalSettings };
-        delete newExperimental['codebaseInvestigatorSettings'];
-        delete newExperimental['cliHelpAgentSettings'];
-        delete newExperimental['plan'];
+        const newExperimental = {
+          ...experimentalSettings,
+          codebaseInvestigatorSettings: undefined,
+          cliHelpAgentSettings: undefined,
+          plan: undefined,
+        };
         loadedSettings.setValue(scope, 'experimental', newExperimental);
       }
       return true;

packages/cli/src/utils/commentJson.test.ts

@@ -204,7 +204,7 @@ describe('commentJson', () => {
       expect(updatedContent).not.toContain('"server2"');
     });
 
-    it('should sync nested objects, removing omitted fields', () => {
+    it('should preserve nested objects when omitted (Bug #23138)', () => {
       const originalContent = `{
         // Configuration
         "model": "gemini-2.5-pro",
@@ -222,18 +222,41 @@ describe('commentJson', () => {
         ui: {
           theme: 'light',
         },
-        preservedField: 'keep me',
       });
 
       const updatedContent = fs.readFileSync(testFilePath, 'utf-8');
       expect(updatedContent).toContain('// Configuration');
       expect(updatedContent).toContain('"model": "gemini-2.5-flash"');
       expect(updatedContent).toContain('"theme": "light"');
-      expect(updatedContent).not.toContain('"existingSetting": "value"');
+      // These should now be preserved
+      expect(updatedContent).toContain('"existingSetting": "value"');
       expect(updatedContent).toContain('"preservedField": "keep me"');
     });
 
-    it('should handle mcpServers field deletion properly', () => {
+    it('should delete keys when explicitly set to undefined', () => {
+      const originalContent = `{
+        "model": "gemini-2.5-pro",
+        "ui": {
+          "theme": "dark",
+          "deleteMe": "remove"
+        }
+      }`;
+
+      fs.writeFileSync(testFilePath, originalContent, 'utf-8');
+
+      updateSettingsFilePreservingFormat(testFilePath, {
+        ui: {
+          theme: 'light',
+          deleteMe: undefined,
+        },
+      });
+
+      const updatedContent = fs.readFileSync(testFilePath, 'utf-8');
+      expect(updatedContent).toContain('"theme": "light"');
+      expect(updatedContent).not.toContain('"deleteMe"');
+    });
+
+    it('should handle mcpServers field deletion using explicit undefined', () => {
       const originalContent = `{
         "model": "gemini-2.5-pro",
         "mcpServers": {
@@ -259,6 +282,7 @@ describe('commentJson', () => {
             command: 'node',
             args: ['server.js'],
           },
+          oldServer: undefined,
         },
       });
 
@@ -270,7 +294,7 @@ describe('commentJson', () => {
       expect(updatedContent).toContain('// Server to remove');
     });
 
-    it('preserves sibling-level commented-out blocks when removing another key', () => {
+    it('preserves sibling-level commented-out blocks when removing another key with undefined', () => {
       const originalContent = `{
         "mcpServers": {
           // "sleep": {
@@ -294,7 +318,9 @@ describe('commentJson', () => {
       fs.writeFileSync(testFilePath, originalContent, 'utf-8');
 
       updateSettingsFilePreservingFormat(testFilePath, {
-        mcpServers: {},
+        mcpServers: {
+          playwright: undefined,
+        },
       });
 
       const updatedContent = fs.readFileSync(testFilePath, 'utf-8');
@@ -322,7 +348,7 @@ describe('commentJson', () => {
       expect(updatedContent).toContain('"item2"');
     });
 
-    it('should remove both nested and non-nested objects when omitted', () => {
+    it('should preserve both nested and non-nested objects when omitted', () => {
       const originalContent = `{
         // Top-level config
         "topLevelObject": {
@@ -356,10 +382,8 @@ describe('commentJson', () => {
 
       const updatedContent = fs.readFileSync(testFilePath, 'utf-8');
 
-      expect(updatedContent).not.toContain('"topLevelObject"');
-
-      expect(updatedContent).not.toContain('"nestedObject"');
-
+      expect(updatedContent).toContain('"topLevelObject"');
+      expect(updatedContent).toContain('"nestedObject"');
       expect(updatedContent).toContain('"keepThis": "value"');
       expect(updatedContent).toContain('"preservedObject"');
       expect(updatedContent).toContain('"data": "keep"');

packages/cli/src/utils/commentJson.ts

@@ -116,15 +116,17 @@ function applyKeyDiff(
   base: Record<string, unknown>,
   desired: Record<string, unknown>,
 ): void {
-  for (const existingKey of Object.getOwnPropertyNames(base)) {
-    if (!Object.prototype.hasOwnProperty.call(desired, existingKey)) {
-      preserveCommentsOnPropertyDeletion(base, existingKey);
-      delete base[existingKey];
-    }
-  }
-
   for (const nextKey of Object.getOwnPropertyNames(desired)) {
     const nextVal = desired[nextKey];
+
+    if (nextVal === undefined) {
+      if (Object.prototype.hasOwnProperty.call(base, nextKey)) {
+        preserveCommentsOnPropertyDeletion(base, nextKey);
+        delete base[nextKey];
+      }
+      continue;
+    }
+
     const baseVal = base[nextKey];
 
     const isObj =