Fix issue with merging configs (#6084)

## Test plan Unit tests added. Also before this fix problem was visible e.g. in duplicated shortcut tooltips next to the lens actions (due to property `cody.advanced.agent.running` being incorrectly overwritten). After this fix problem should disappear.
sourcegraph · Nov 7, 2024 · 45d2d57 · 45d2d57
1 parent bc8b14e
commit 45d2d57
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 11 deletions.
diff --git a/agent/src/AgentWorkspaceConfiguration.test.ts b/agent/src/AgentWorkspaceConfiguration.test.ts
@@ -23,6 +23,7 @@ describe('AgentWorkspaceConfiguration', () => {
           "cody.debug": {
             "verbose": true
           },
+          "cody.advanced.hasNativeWebview": false,
           "cody.autocomplete.advanced.provider": "anthropic",
           "cody.experimental": {
             "tracing": true
@@ -54,6 +55,9 @@ describe('AgentWorkspaceConfiguration', () => {
         verboseDebug: true,
         codebase: 'test-repo',
         customConfigurationJson: customConfigJson,
+        customConfiguration: {
+            'cody.debug.additional': true,
+        },
     }
 
     beforeEach(() => {
@@ -70,7 +74,7 @@ describe('AgentWorkspaceConfiguration', () => {
             expect(config.get('cody.customHeaders')).toEqual({ 'X-Test': 'test' })
             expect(config.get('cody.telemetry.level')).toBe('agent')
             // clientName undefined because custom JSON specified telemetry with level alone.
-            expect(config.get('cody.telemetry.clientName')).toBeUndefined()
+            expect(config.get('cody.telemetry.clientName')).toBe('test-client')
             expect(config.get('cody.autocomplete.enabled')).toBe(true)
             expect(config.get('cody.autocomplete.advanced.provider')).toBe('anthropic')
             expect(config.get('cody.autocomplete.advanced.model')).toBe('claude-2')
@@ -108,7 +112,7 @@ describe('AgentWorkspaceConfiguration', () => {
                         },
                         running: true,
                     },
-                    hasNativeWebview: true,
+                    hasNativeWebview: false,
                 },
                 autocomplete: {
                     advanced: {
@@ -123,12 +127,14 @@ describe('AgentWorkspaceConfiguration', () => {
                 },
                 debug: {
                     verbose: true,
+                    additional: true,
                 },
                 experimental: {
                     tracing: true,
                 },
                 serverEndpoint: 'https://sourcegraph.test',
                 telemetry: {
+                    clientName: 'test-client',
                     level: 'agent',
                 },
             })
@@ -167,7 +173,7 @@ describe('AgentWorkspaceConfiguration', () => {
 
         it('handles agent capabilities correctly', () => {
             expect(config.get('cody.advanced.agent.capabilities.storage')).toBe(true)
-            expect(config.get('cody.advanced.hasNativeWebview')).toBe(true)
+            expect(config.get('cody.advanced.hasNativeWebview')).toBe(false)
         })
 
         it('returns default value for unknown sections', () => {
@@ -236,7 +242,11 @@ describe('AgentWorkspaceConfiguration', () => {
 
         it('updates nested configuration object', async () => {
             await config.update('cody.debug', { verbose: false, newSetting: true })
-            expect(config.get('cody.debug')).toEqual({ verbose: false, newSetting: true })
+            expect(config.get('cody.debug')).toEqual({
+                verbose: false,
+                newSetting: true,
+                additional: true,
+            })
             expect(config.get('cody.debug.newSetting')).toEqual(true)
         })
 

diff --git a/agent/src/AgentWorkspaceConfiguration.ts b/agent/src/AgentWorkspaceConfiguration.ts
@@ -111,19 +111,26 @@ export class AgentWorkspaceConfiguration implements vscode.WorkspaceConfiguratio
             },
         }
 
+        function mergeWithBaseConfig(config: any) {
+            for (const [key, value] of Object.entries(config)) {
+                if (typeof value === 'object') {
+                    const existing = _.get(baseConfig, key) ?? {}
+                    const merged = _.merge(existing, value)
+                    _.set(baseConfig, key, merged)
+                } else {
+                    _.set(baseConfig, key, value)
+                }
+            }
+        }
+
         const customConfiguration = config?.customConfiguration
         if (customConfiguration) {
-            for (const [key, value] of Object.entries(customConfiguration)) {
-                _.set(baseConfig, key, value)
-            }
+            mergeWithBaseConfig(customConfiguration)
         }
 
         const fromCustomConfigurationJson = config?.customConfigurationJson
         if (fromCustomConfigurationJson) {
-            const configJson = JSON.parse(fromCustomConfigurationJson)
-            for (const [key, value] of Object.entries(configJson)) {
-                _.set(baseConfig, key, value)
-            }
+            mergeWithBaseConfig(JSON.parse(fromCustomConfigurationJson))
         }
 
         const fromBaseConfig = _.get(baseConfig, section)