From 15412e3a25990ff40740e51d0a7abee68564a1dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kukie=C5=82ka?= Date: Mon, 4 Nov 2024 16:02:36 +0100 Subject: [PATCH] Fix bugs in workspace::getConfiguration vscode shim (#6058) ## Changes `getConfiguration` was working incorrectly when scope was provided. Currently agent ignores scopes, but should respect configuration sections. In the buggy case if scope was give, `getConfiguration` was always returning top level scope ignoring section. ## Test plan Unittests are added. Test scenario with openctx providers: 1. Add the following configuration in the cody jetbrains settings: ``` "cody.experimental.noodle": true, "openctx": { "providers": { "https://gist.githubusercontent.com/thenamankumar/3708f084fb2abd57adafe7f14620bdf7/raw/e7c7059cb5b04f6feead002e7b3a90c5b1a4bb96/provider.js": true }, "enable": true } ``` 2. Restart IDE 3. You should see new @ provider ![image](https://github.com/user-attachments/assets/608881fb-90d6-4328-a427-2b13388d3db8) --- agent/src/vscode-shim.test.ts | 82 +++++++++++++++++++++++++++++++++++ agent/src/vscode-shim.ts | 8 ++-- 2 files changed, 85 insertions(+), 5 deletions(-) diff --git a/agent/src/vscode-shim.test.ts b/agent/src/vscode-shim.test.ts index bb010482932e..cf738df0bb8b 100644 --- a/agent/src/vscode-shim.test.ts +++ b/agent/src/vscode-shim.test.ts @@ -7,6 +7,7 @@ import { rimraf } from 'rimraf' import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { URI } from 'vscode-uri' +import { AgentWorkspaceConfiguration } from './AgentWorkspaceConfiguration' import { AgentWorkspaceDocuments } from './AgentWorkspaceDocuments' import * as vscode from './vscode-shim' import { setWorkspaceFolders, workspaceFolders } from './vscode-shim' @@ -351,3 +352,84 @@ describe('vscode_shim.onDidChangeWorkspaceFolders', () => { expect(result).toHaveLength(0) }) }) + +describe('vscode.workspace.getConfiguration', () => { + let configuration: AgentWorkspaceConfiguration + + const clientInfo = { + name: 'vscode', + version: '1.0.0', + ideVersion: '1.80.0', + workspaceRootUri: '/', + } + + const customConfigJson = ` + { + "cody.experimental.noodle": true, + "openctx": { + "providers": { + "https://gist.githubusercontent.com/someuser/provider.js": true + }, + "enable": true + } + } + ` + + const extensionConfig = { + serverEndpoint: 'https://sourcegraph.test', + customHeaders: { 'X-Test': 'test' }, + telemetryClientName: 'test-client', + autocompleteAdvancedProvider: 'anthropic', + autocompleteAdvancedModel: 'claude-2', + verboseDebug: true, + codebase: 'test-repo', + customConfigurationJson: customConfigJson, + } + + beforeEach(() => { + configuration = new AgentWorkspaceConfiguration( + [], + () => clientInfo, + () => extensionConfig + ) + vscode.setClientInfo(clientInfo) + vscode.setExtensionConfiguration(extensionConfig) + }) + + it('returns full configuration when section is undefined', () => { + const newConfig = vscode.workspace.getConfiguration() + expect(newConfig.get('openctx')).toMatchObject(configuration.get('openctx')) + }) + + it('returns scoped configuration for valid section', () => { + const newConfig = vscode.workspace.getConfiguration('openctx') + expect(newConfig).toBeDefined() + expect(newConfig.get('providers')).toMatchObject(configuration.get('openctx.providers')) + }) + + it('ignores scope parameter when section is undefined', () => { + const newConfig = vscode.workspace.getConfiguration(undefined, vscode.Uri.file('/test')) + expect(newConfig.get('openctx')).toMatchObject(configuration.get('openctx')) + }) + + it('falls back to global scope for language-scoped configuration', () => { + const newConfig = vscode.workspace.getConfiguration('[jsonc]') + expect(newConfig.get('openctx')).toMatchObject(configuration.get('openctx')) + }) + + it('handles nested section paths', () => { + const config = vscode.workspace.getConfiguration('openctx.providers') + expect(config).toBeDefined() + expect(config.get('https://gist.githubusercontent.com/someuser/provider.js')).toMatchObject( + configuration.get( + 'openctx.providers.https://gist.githubusercontent.com/someuser/provider.js' + ) + ) + }) + + it('returns same configuration regardless of scope when section is defined', () => { + const configNoScope = vscode.workspace.getConfiguration('openctx') + const configWithScope = vscode.workspace.getConfiguration('openctx', vscode.Uri.file('/test')) + expect(configNoScope.get('providers')).toEqual(configWithScope.get('providers')) + }) +}) diff --git a/agent/src/vscode-shim.ts b/agent/src/vscode-shim.ts index a520ef192c62..f775dd30e76c 100644 --- a/agent/src/vscode-shim.ts +++ b/agent/src/vscode-shim.ts @@ -394,16 +394,14 @@ const _workspace: typeof vscode.workspace = { // https://github.com/sourcegraph/cody/issues/4136 createFileSystemWatcher: () => emptyFileWatcher, getConfiguration: (section, scope): vscode.WorkspaceConfiguration => { - if (section !== undefined) { - if (scope === undefined) { - return configuration.withPrefix(section) - } - + if (section) { // Ignore language-scoped configuration sections like // '[jsonc].editor.insertSpaces', fallback to global scope instead. if (section.startsWith('[')) { return configuration } + + return configuration.withPrefix(section) } return configuration },