From 45d2d57d79da45f90aa562c1bfb1bb46bdacb417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kukie=C5=82ka?= Date: Thu, 7 Nov 2024 13:34:13 +0100 Subject: [PATCH] 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. --- agent/src/AgentWorkspaceConfiguration.test.ts | 18 ++++++++++++---- agent/src/AgentWorkspaceConfiguration.ts | 21 ++++++++++++------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/agent/src/AgentWorkspaceConfiguration.test.ts b/agent/src/AgentWorkspaceConfiguration.test.ts index 698302fceb29..be5d8d54090e 100644 --- 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 index 7e86332946e3..55f0bcf32fcb 100644 --- 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)