Skip to content

Commit 2dc20eb

Browse files
committed
rationalize configuration
1 parent eb85b51 commit 2dc20eb

File tree

12 files changed

+65
-13
lines changed

12 files changed

+65
-13
lines changed

src/vs/workbench/api/common/extHostMcp.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import { AUTH_SERVER_METADATA_DISCOVERY_PATH, getDefaultMetadataForUrl, getMetad
2020
import { URI } from '../../../base/common/uri.js';
2121
import { MCP } from '../../contrib/mcp/common/modelContextProtocol.js';
2222
import { CancellationError } from '../../../base/common/errors.js';
23+
import { ConfigurationTarget } from '../../../platform/configuration/common/configuration.js';
24+
import { IExtHostInitDataService } from './extHostInitDataService.js';
2325

2426
export const IExtHostMpcService = createDecorator<IExtHostMpcService>('IExtHostMpcService');
2527

@@ -38,6 +40,7 @@ export class ExtHostMcpService extends Disposable implements IExtHostMpcService
3840

3941
constructor(
4042
@IExtHostRpcService extHostRpc: IExtHostRpcService,
43+
@IExtHostInitDataService private readonly _extHostInitData: IExtHostInitDataService
4144
) {
4245
super();
4346
this._proxy = extHostRpc.getProxy(MainContext.MainThreadMcp);
@@ -105,6 +108,7 @@ export class ExtHostMcpService extends Disposable implements IExtHostMpcService
105108
scope: StorageScope.WORKSPACE,
106109
canResolveLaunch: typeof provider.resolveMcpServerDefinition === 'function',
107110
extensionId: extension.identifier.value,
111+
configTarget: this._extHostInitData.remote.isRemote ? ConfigurationTarget.USER_REMOTE : ConfigurationTarget.USER,
108112
};
109113

110114
const update = async () => {
@@ -124,7 +128,7 @@ export class ExtHostMcpService extends Disposable implements IExtHostMpcService
124128
id,
125129
label: item.label,
126130
cacheNonce: item.version,
127-
launch: Convert.McpServerDefinition.from(item)
131+
launch: Convert.McpServerDefinition.from(item),
128132
});
129133
}
130134

src/vs/workbench/api/node/extHostMcpNode.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@ import { McpConnectionState, McpServerLaunch, McpServerTransportStdio, McpServer
1515
import { ExtHostMcpService } from '../common/extHostMcp.js';
1616
import { IExtHostRpcService } from '../common/extHostRpcService.js';
1717
import * as path from '../../../base/common/path.js';
18+
import { IExtHostInitDataService } from '../common/extHostInitDataService.js';
1819

1920
export class NodeExtHostMpcService extends ExtHostMcpService {
2021
constructor(
2122
@IExtHostRpcService extHostRpc: IExtHostRpcService,
23+
@IExtHostInitDataService initDataService: IExtHostInitDataService,
2224
) {
23-
super(extHostRpc);
25+
super(extHostRpc, initDataService);
2426
}
2527

2628
private nodeServers = new Map<number, {

src/vs/workbench/contrib/chat/browser/chat.contribution.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ configurationRegistry.registerConfiguration({
253253
[mcpServerSamplingSection]: {
254254
type: 'object',
255255
description: nls.localize('chat.mcp.serverSampling', "Configures which models are exposed to MCP servers for sampling (making model requests in the background). This setting can be edited in a graphical way under the `{0}` command.", 'MCP: ' + nls.localize('mcp.list', 'List Servers')),
256+
scope: ConfigurationScope.RESOURCE,
256257
additionalProperties: {
257258
type: 'object',
258259
properties: {

src/vs/workbench/contrib/mcp/common/discovery/configMcpDiscovery.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ export class ConfigMcpDiscovery extends Disposable implements IMcpDiscovery {
164164
remoteAuthority: src.path.remoteAuthority || null,
165165
serverDefinitions: src.serverDefinitions,
166166
isTrustedByDefault: true,
167+
configTarget: src.path.target,
167168
scope: src.path.scope,
168169
});
169170
}

src/vs/workbench/contrib/mcp/common/discovery/extensionMcpDiscovery.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { Disposable, DisposableMap } from '../../../../../base/common/lifecycle.
77
import { observableValue } from '../../../../../base/common/observable.js';
88
import { isFalsyOrWhitespace } from '../../../../../base/common/strings.js';
99
import { localize } from '../../../../../nls.js';
10+
import { ConfigurationTarget } from '../../../../../platform/configuration/common/configuration.js';
1011
import { IMcpCollectionContribution } from '../../../../../platform/extensions/common/extensions.js';
1112
import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js';
1213
import { IExtensionService } from '../../../../services/extensions/common/extensions.js';
@@ -85,6 +86,7 @@ export class ExtensionMcpDiscovery extends Disposable implements IMcpDiscovery {
8586
remoteAuthority: null,
8687
isTrustedByDefault: true,
8788
scope: StorageScope.WORKSPACE,
89+
configTarget: ConfigurationTarget.USER,
8890
serverDefinitions: observableValue<McpServerDefinition[]>(this, serverDefs?.map(McpServerDefinition.fromSerialized) || []),
8991
lazy: {
9092
isCached: !!serverDefs,

src/vs/workbench/contrib/mcp/common/discovery/nativeMcpDiscoveryAbstract.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { Schemas } from '../../../../../base/common/network.js';
1010
import { autorunWithStore, IObservable, IReader, ISettableObservable, observableValue } from '../../../../../base/common/observable.js';
1111
import { URI } from '../../../../../base/common/uri.js';
1212
import { localize } from '../../../../../nls.js';
13-
import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
13+
import { ConfigurationTarget, IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
1414
import { IFileService } from '../../../../../platform/files/common/files.js';
1515
import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js';
1616
import { ILabelService } from '../../../../../platform/label/common/label.js';
@@ -144,6 +144,7 @@ export abstract class NativeFilesystemMcpDiscovery extends FilesystemMcpDiscover
144144
id: adapter.id,
145145
label: discoverySourceLabel[adapter.discoverySource] + this.suffix,
146146
remoteAuthority: adapter.remoteAuthority,
147+
configTarget: ConfigurationTarget.USER,
147148
scope: StorageScope.PROFILE,
148149
isTrustedByDefault: false,
149150
serverDefinitions: observableValue<readonly McpServerDefinition[]>(this, []),

src/vs/workbench/contrib/mcp/common/discovery/workspaceMcpDiscoveryAdapter.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { DisposableMap, IDisposable } from '../../../../../base/common/lifecycle
77
import { observableValue } from '../../../../../base/common/observable.js';
88
import { joinPath } from '../../../../../base/common/resources.js';
99
import { URI } from '../../../../../base/common/uri.js';
10-
import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
10+
import { ConfigurationTarget, IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
1111
import { IFileService } from '../../../../../platform/files/common/files.js';
1212
import { StorageScope } from '../../../../../platform/storage/common/storage.js';
1313
import { IWorkspaceContextService, IWorkspaceFolder } from '../../../../../platform/workspace/common/workspace.js';
@@ -56,6 +56,7 @@ export class CursorWorkspaceMcpDiscoveryAdapter extends FilesystemMcpDiscovery i
5656
scope: StorageScope.WORKSPACE,
5757
isTrustedByDefault: false,
5858
serverDefinitions: observableValue(this, []),
59+
configTarget: ConfigurationTarget.WORKSPACE_FOLDER,
5960
presentation: {
6061
origin: configFile,
6162
order: McpCollectionSortOrder.WorkspaceFolder + 1,

src/vs/workbench/contrib/mcp/common/mcpSamplingService.ts

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { Event } from '../../../../base/common/event.js';
1010
import { isDefined } from '../../../../base/common/types.js';
1111
import { localize } from '../../../../nls.js';
1212
import { ICommandService } from '../../../../platform/commands/common/commands.js';
13-
import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js';
13+
import { ConfigurationTarget, getConfigValueInTarget, IConfigurationService } from '../../../../platform/configuration/common/configuration.js';
1414
import { IDialogService } from '../../../../platform/dialogs/common/dialogs.js';
1515
import { ExtensionIdentifier } from '../../../../platform/extensions/common/extensions.js';
1616
import { INotificationService, Severity } from '../../../../platform/notification/common/notification.js';
@@ -226,21 +226,54 @@ export class McpSamplingService implements IMcpSamplingService {
226226
return foundModelIds[0]; // Return the first matching model
227227
}
228228

229+
private _configKey(server: IMcpServer) {
230+
return `${server.collection.label}: ${server.definition.label}`;
231+
}
232+
229233
public getConfig(server: IMcpServer): IMcpServerSamplingConfiguration {
230-
const mapping = this._configurationService.getValue<Record<string, IMcpServerSamplingConfiguration>>(mcpServerSamplingSection);
231-
return mapping[server.definition.id] || {};
234+
return this._getConfig(server).value || {};
232235
}
233236

234-
public async updateConfig(server: IMcpServer, mutate: (r: IMcpServerSamplingConfiguration) => unknown) {
237+
/**
238+
* _getConfig reads the sampling config reads the `{ server: data }` mapping
239+
* from the appropriate config. We read from the most specific possible
240+
* config up to the default configuration location that the MCP server itself
241+
* is defined in. We don't go further because then workspace-specific servers
242+
* would get in the user settings which is not meaningful and could lead
243+
* to confusion.
244+
*
245+
* todo@connor4312: generalize this for other esttings when we have them
246+
*/
247+
private _getConfig(server: IMcpServer) {
235248
const def = server.readDefinitions().get();
236-
const mapping = this._configurationService.getValue<Record<string, IMcpServerSamplingConfiguration>>(mcpServerSamplingSection, { resource: def.collection?.presentation?.origin });
249+
const mostSpecificConfig = ConfigurationTarget.MEMORY;
250+
const leastSpecificConfig = def.collection?.configTarget || ConfigurationTarget.USER;
251+
const key = this._configKey(server);
252+
const resource = def.collection?.presentation?.origin;
253+
254+
const configValue = this._configurationService.inspect<Record<string, IMcpServerSamplingConfiguration>>(mcpServerSamplingSection, { resource });
255+
for (let target = mostSpecificConfig; target >= leastSpecificConfig; target--) {
256+
const mapping = getConfigValueInTarget(configValue, target);
257+
const config = mapping?.[key];
258+
if (config) {
259+
return { value: config, key, mapping, target, resource };
260+
}
261+
}
262+
263+
return { value: undefined, mapping: undefined, key, target: leastSpecificConfig, resource };
264+
}
265+
266+
public async updateConfig(server: IMcpServer, mutate: (r: IMcpServerSamplingConfiguration) => unknown) {
267+
const { value, mapping, key, target, resource } = this._getConfig(server);
237268

238-
const newConfig = { ...mapping[server.definition.id] };
269+
const newConfig = { ...value };
239270
mutate(newConfig);
240271

241272
await this._configurationService.updateValue(
242273
mcpServerSamplingSection,
243-
{ ...mapping, [server.definition.id]: newConfig },
274+
{ ...mapping, [key]: newConfig },
275+
{ resource },
276+
target,
244277
);
245278
return newConfig;
246279
}

src/vs/workbench/contrib/mcp/common/mcpTypes.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ export interface McpCollectionDefinition {
5454
readonly isTrustedByDefault: boolean;
5555
/** Scope where associated collection info should be stored. */
5656
readonly scope: StorageScope;
57+
/** Configuration target where configuration related to this server should be stored. */
58+
readonly configTarget: ConfigurationTarget;
5759

5860
/** Resolves a server definition. If present, always called before a server starts. */
5961
resolveServerLanch?(definition: McpServerDefinition): Promise<McpServerLaunch | undefined>;
@@ -94,6 +96,7 @@ export namespace McpCollectionDefinition {
9496
readonly scope: StorageScope;
9597
readonly canResolveLaunch: boolean;
9698
readonly extensionId: string;
99+
readonly configTarget: ConfigurationTarget;
97100
}
98101

99102
export function equals(a: McpCollectionDefinition, b: McpCollectionDefinition): boolean {

src/vs/workbench/contrib/mcp/test/common/mcpRegistry.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,8 @@ suite('Workbench - MCP - Registry', () => {
155155
remoteAuthority: null,
156156
serverDefinitions: observableValue('serverDefs', []),
157157
isTrustedByDefault: true,
158-
scope: StorageScope.APPLICATION
158+
scope: StorageScope.APPLICATION,
159+
configTarget: ConfigurationTarget.USER,
159160
};
160161

161162
// Create base definition that can be reused

src/vs/workbench/contrib/mcp/test/common/mcpRegistryTypes.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ export class TestMcpRegistry implements IMcpRegistry {
167167
id: 'test-collection',
168168
remoteAuthority: null,
169169
label: 'Test Collection',
170+
configTarget: ConfigurationTarget.USER,
170171
serverDefinitions: observableValue(this, [{
171172
id: 'test-server',
172173
label: 'Test Server',

src/vs/workbench/contrib/mcp/test/common/mcpServerConnection.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { IMcpHostDelegate, IMcpMessageTransport } from '../../common/mcpRegistry
2020
import { McpServerConnection } from '../../common/mcpServerConnection.js';
2121
import { McpCollectionDefinition, McpConnectionState, McpServerDefinition, McpServerTransportType } from '../../common/mcpTypes.js';
2222
import { TestMcpMessageTransport } from './mcpRegistryTypes.js';
23+
import { ConfigurationTarget } from '../../../../../platform/configuration/common/configuration.js';
2324

2425
class TestMcpHostDelegate extends Disposable implements IMcpHostDelegate {
2526
private readonly _transport: TestMcpMessageTransport;
@@ -86,7 +87,8 @@ suite('Workbench - MCP - ServerConnection', () => {
8687
remoteAuthority: null,
8788
serverDefinitions: observableValue('serverDefs', []),
8889
isTrustedByDefault: true,
89-
scope: StorageScope.APPLICATION
90+
scope: StorageScope.APPLICATION,
91+
configTarget: ConfigurationTarget.USER,
9092
};
9193

9294
// Create server definition

0 commit comments

Comments
 (0)