Skip to content

Commit bb404ec

Browse files
authored
microsoft#257420 - handle duplicate servers (microsoft#257450)
1 parent f767c1b commit bb404ec

File tree

7 files changed

+56
-23
lines changed

7 files changed

+56
-23
lines changed

src/vs/platform/mcp/common/mcpManagement.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ export interface ILocalMcpServer {
1919
readonly version?: string;
2020
readonly mcpResource: URI;
2121
readonly location?: URI;
22-
readonly id?: string;
2322
readonly displayName?: string;
2423
readonly url?: string;
2524
readonly description?: string;

src/vs/platform/mcp/common/mcpManagementService.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,6 @@ export abstract class AbstractMcpResourceManagementService extends Disposable {
181181
mcpResource: this.mcpResource,
182182
version: mcpServerInfo.version,
183183
location: mcpServerInfo.location,
184-
id: mcpServerInfo.id,
185184
displayName: mcpServerInfo.displayName,
186185
description: mcpServerInfo.description,
187186
publisher: mcpServerInfo.publisher,

src/vs/workbench/contrib/mcp/browser/mcpServerActions.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ export class StartServerAction extends McpServerAction {
314314
if (!this.mcpServer.local) {
315315
return;
316316
}
317-
return this.mcpService.servers.get().find(s => s.definition.label === this.mcpServer?.name);
317+
return this.mcpService.servers.get().find(s => s.definition.id === this.mcpServer?.id);
318318
}
319319
}
320320

@@ -361,7 +361,7 @@ export class StopServerAction extends McpServerAction {
361361
if (!this.mcpServer.local) {
362362
return;
363363
}
364-
return this.mcpService.servers.get().find(s => s.definition.label === this.mcpServer?.name);
364+
return this.mcpService.servers.get().find(s => s.definition.id === this.mcpServer?.id);
365365
}
366366
}
367367

@@ -410,7 +410,7 @@ export class RestartServerAction extends McpServerAction {
410410
if (!this.mcpServer.local) {
411411
return;
412412
}
413-
return this.mcpService.servers.get().find(s => s.definition.label === this.mcpServer?.name);
413+
return this.mcpService.servers.get().find(s => s.definition.id === this.mcpServer?.id);
414414
}
415415
}
416416

@@ -483,7 +483,7 @@ export class AuthServerAction extends McpServerAction {
483483
if (!this.mcpServer.local) {
484484
return;
485485
}
486-
return this.mcpService.servers.get().find(s => s.definition.label === this.mcpServer?.name);
486+
return this.mcpService.servers.get().find(s => s.definition.id === this.mcpServer?.id);
487487
}
488488

489489
private getAccountQuery(): IAccountQuery | undefined {
@@ -550,7 +550,7 @@ export class ShowServerOutputAction extends McpServerAction {
550550
if (!this.mcpServer.local) {
551551
return;
552552
}
553-
return this.mcpService.servers.get().find(s => s.definition.label === this.mcpServer?.name);
553+
return this.mcpService.servers.get().find(s => s.definition.id === this.mcpServer?.id);
554554
}
555555
}
556556

@@ -626,7 +626,7 @@ export class ConfigureModelAccessAction extends McpServerAction {
626626
if (!this.mcpServer.local) {
627627
return;
628628
}
629-
return this.mcpService.servers.get().find(s => s.definition.label === this.mcpServer?.name);
629+
return this.mcpService.servers.get().find(s => s.definition.id === this.mcpServer?.id);
630630
}
631631
}
632632

@@ -680,7 +680,7 @@ export class ShowSamplingRequestsAction extends McpServerAction {
680680
if (!this.mcpServer.local) {
681681
return;
682682
}
683-
return this.mcpService.servers.get().find(s => s.definition.label === this.mcpServer?.name);
683+
return this.mcpService.servers.get().find(s => s.definition.id === this.mcpServer?.id);
684684
}
685685
}
686686

@@ -731,7 +731,7 @@ export class BrowseResourcesAction extends McpServerAction {
731731
if (!this.mcpServer.local) {
732732
return;
733733
}
734-
return this.mcpService.servers.get().find(s => s.definition.label === this.mcpServer?.name);
734+
return this.mcpService.servers.get().find(s => s.definition.id === this.mcpServer?.id);
735735
}
736736
}
737737

@@ -769,7 +769,7 @@ export class McpServerStatusAction extends McpServerAction {
769769
return;
770770
}
771771

772-
if (this.mcpServer.installState === McpServerInstallState.Uninstalled) {
772+
if ((this.mcpServer.gallery || this.mcpServer.installable) && this.mcpServer.installState === McpServerInstallState.Uninstalled) {
773773
const result = this.mcpWorkbenchService.canInstall(this.mcpServer);
774774
if (result !== true) {
775775
this.updateStatus({ icon: warningIcon, message: result }, true);

src/vs/workbench/contrib/mcp/browser/mcpServersView.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ export class McpServersListView extends AbstractExtensionsListView<IWorkbenchMcp
146146
if (e.element) {
147147
const disposables = new DisposableStore();
148148
const manageExtensionAction = disposables.add(this.instantiationService.createInstance(ManageMcpServerAction, false));
149-
const extension = e.element ? this.mcpWorkbenchService.local.find(local => local.name === e.element!.name) || e.element
149+
const extension = e.element ? this.mcpWorkbenchService.local.find(local => local.id === e.element!.id) || e.element
150150
: e.element;
151151
manageExtensionAction.mcpServer = extension;
152152
let groups: IAction[][] = [];
@@ -278,7 +278,7 @@ export class McpServersListView extends AbstractExtensionsListView<IWorkbenchMcp
278278
let index = -1;
279279
const previousMcpServerInNew = newMcpServers[from];
280280
if (previousMcpServerInNew) {
281-
index = oldMcpServers.findIndex(e => e.name === previousMcpServerInNew.name);
281+
index = oldMcpServers.findIndex(e => e.id === previousMcpServerInNew.id);
282282
if (index === -1) {
283283
return findPreviousMcpServerIndex(from - 1);
284284
}
@@ -289,7 +289,7 @@ export class McpServersListView extends AbstractExtensionsListView<IWorkbenchMcp
289289
let hasChanged: boolean = false;
290290
for (let index = 0; index < newMcpServers.length; index++) {
291291
const mcpServer = newMcpServers[index];
292-
if (mcpServers.every(r => r.name !== mcpServer.name)) {
292+
if (mcpServers.every(r => r.id !== mcpServer.id)) {
293293
hasChanged = true;
294294
mcpServers.splice(findPreviousMcpServerIndex(index - 1) + 1, 0, mcpServer);
295295
}
@@ -393,7 +393,10 @@ class McpServerRenderer implements IListRenderer<IWorkbenchMcpServer, IMcpServer
393393
data.mcpServer = mcpServer;
394394

395395
const updateEnablement = () => {
396-
const disabled = mcpServer.installState === McpServerInstallState.Installed && !!mcpServer.local && this.allowedMcpServersService.isAllowed(mcpServer.local) !== true;
396+
const disabled = !!mcpServer.local &&
397+
(mcpServer.installState === McpServerInstallState.Installed
398+
? this.allowedMcpServersService.isAllowed(mcpServer.local) !== true
399+
: mcpServer.installState === McpServerInstallState.Uninstalled);
397400
data.root.classList.toggle('disabled', disabled);
398401
};
399402
updateEnablement();

src/vs/workbench/contrib/mcp/browser/mcpWorkbenchService.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import { IWorkbenchContribution } from '../../../common/contributions.js';
3131
import { MCP_CONFIGURATION_KEY, WORKSPACE_STANDALONE_CONFIGURATIONS } from '../../../services/configuration/common/configuration.js';
3232
import { ACTIVE_GROUP, IEditorService } from '../../../services/editor/common/editorService.js';
3333
import { IWorkbenchEnvironmentService } from '../../../services/environment/common/environmentService.js';
34-
import { DidUninstallWorkbenchMcpServerEvent, IWorkbenchLocalMcpServer, IWorkbenchMcpManagementService, IWorkbenchMcpServerInstallResult, LocalMcpServerScope } from '../../../services/mcp/common/mcpWorkbenchManagementService.js';
34+
import { DidUninstallWorkbenchMcpServerEvent, IWorkbenchLocalMcpServer, IWorkbenchMcpManagementService, IWorkbenchMcpServerInstallResult, LocalMcpServerScope, REMOTE_USER_CONFIG_ID, USER_CONFIG_ID, WORKSPACE_CONFIG_ID, WORKSPACE_FOLDER_CONFIG_ID_PREFIX } from '../../../services/mcp/common/mcpWorkbenchManagementService.js';
3535
import { IRemoteAgentService } from '../../../services/remote/common/remoteAgentService.js';
3636
import { mcpConfigurationSection } from '../common/mcpConfiguration.js';
3737
import { HasInstalledMcpServersContext, IMcpConfigPath, IMcpWorkbenchService, IWorkbenchMcpServer, McpCollectionSortOrder, McpServerInstallState, McpServersGalleryEnabledContext } from '../common/mcpTypes.js';
@@ -55,7 +55,7 @@ class McpWorkbenchServer implements IWorkbenchMcpServer {
5555
}
5656

5757
get id(): string {
58-
return this.gallery?.id ?? this.local?.id ?? this.installable?.name ?? '';
58+
return this.local?.id ?? this.gallery?.id ?? this.installable?.name ?? this.name;
5959
}
6060

6161
get name(): string {
@@ -462,7 +462,7 @@ export class McpWorkbenchService extends Disposable implements IMcpWorkbenchServ
462462

463463
private getUserMcpConfigPath(mcpResource: URI): IMcpConfigPath {
464464
return {
465-
id: 'usrlocal',
465+
id: USER_CONFIG_ID,
466466
key: 'userLocalValue',
467467
target: ConfigurationTarget.USER_LOCAL,
468468
label: localize('mcp.configuration.userLocalValue', 'Global in {0}', this.productService.nameShort),
@@ -475,7 +475,7 @@ export class McpWorkbenchService extends Disposable implements IMcpWorkbenchServ
475475

476476
private getRemoteMcpConfigPath(mcpResource: URI): IMcpConfigPath {
477477
return {
478-
id: 'usrremote',
478+
id: REMOTE_USER_CONFIG_ID,
479479
key: 'userRemoteValue',
480480
target: ConfigurationTarget.USER_REMOTE,
481481
label: this.environmentService.remoteAuthority ? this.labelService.getHostLabel(Schemas.vscodeRemote, this.environmentService.remoteAuthority) : 'Remote',
@@ -491,7 +491,7 @@ export class McpWorkbenchService extends Disposable implements IMcpWorkbenchServ
491491
const workspace = this.workspaceService.getWorkspace();
492492
if (workspace.configuration && this.uriIdentityService.extUri.isEqual(workspace.configuration, mcpResource)) {
493493
return {
494-
id: 'workspace',
494+
id: WORKSPACE_CONFIG_ID,
495495
key: 'workspaceValue',
496496
target: ConfigurationTarget.WORKSPACE,
497497
label: basename(mcpResource),
@@ -508,7 +508,7 @@ export class McpWorkbenchService extends Disposable implements IMcpWorkbenchServ
508508
const workspaceFolder = workspaceFolders[index];
509509
if (this.uriIdentityService.extUri.isEqual(this.uriIdentityService.extUri.joinPath(workspaceFolder.uri, WORKSPACE_STANDALONE_CONFIGURATIONS[MCP_CONFIGURATION_KEY]), mcpResource)) {
510510
return {
511-
id: `wf${index}`,
511+
id: `${WORKSPACE_FOLDER_CONFIG_ID_PREFIX}${index}`,
512512
key: 'workspaceFolderValue',
513513
target: ConfigurationTarget.WORKSPACE_FOLDER,
514514
label: `${workspaceFolder.name}/.vscode/mcp.json`,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ export class McpServerContainers extends Disposable {
648648
update(server: IWorkbenchMcpServer | undefined): void {
649649
for (const container of this.containers) {
650650
if (server && container.mcpServer) {
651-
if (server.name === container.mcpServer.name) {
651+
if (server.id === container.mcpServer.id) {
652652
container.mcpServer = server;
653653
}
654654
} else {

src/vs/workbench/services/mcp/common/mcpWorkbenchManagementService.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ import { AbstractMcpManagementService, AbstractMcpResourceManagementService, ILo
2424
import { IFileService } from '../../../../platform/files/common/files.js';
2525
import { ResourceMap } from '../../../../base/common/map.js';
2626

27+
export const USER_CONFIG_ID = 'usrlocal';
28+
export const REMOTE_USER_CONFIG_ID = 'usrremote';
29+
export const WORKSPACE_CONFIG_ID = 'workspace';
30+
export const WORKSPACE_FOLDER_CONFIG_ID_PREFIX = 'ws';
31+
2732
export interface IWorkbencMcpServerInstallOptions extends InstallOptions {
2833
target?: ConfigurationTarget | IWorkspaceFolder;
2934
}
@@ -35,6 +40,7 @@ export const enum LocalMcpServerScope {
3540
}
3641

3742
export interface IWorkbenchLocalMcpServer extends ILocalMcpServer {
43+
readonly id: string;
3844
readonly scope: LocalMcpServerScope;
3945
}
4046

@@ -288,7 +294,33 @@ export class WorkbenchMcpManagementService extends AbstractMcpManagementService
288294
}
289295

290296
private toWorkspaceMcpServer(server: ILocalMcpServer, scope: LocalMcpServerScope): IWorkbenchLocalMcpServer {
291-
return { ...server, scope };
297+
return { ...server, id: `mcp.config.${this.getConfigId(server, scope)}.${server.name}`, scope };
298+
}
299+
300+
private getConfigId(server: ILocalMcpServer, scope: LocalMcpServerScope): string {
301+
if (scope === LocalMcpServerScope.User) {
302+
return USER_CONFIG_ID;
303+
}
304+
305+
if (scope === LocalMcpServerScope.RemoteUser) {
306+
return REMOTE_USER_CONFIG_ID;
307+
}
308+
309+
if (scope === LocalMcpServerScope.Workspace) {
310+
const workspace = this.workspaceContextService.getWorkspace();
311+
if (workspace.configuration && this.uriIdentityService.extUri.isEqual(workspace.configuration, server.mcpResource)) {
312+
return WORKSPACE_CONFIG_ID;
313+
}
314+
315+
const workspaceFolders = workspace.folders;
316+
for (let index = 0; index < workspaceFolders.length; index++) {
317+
const workspaceFolder = workspaceFolders[index];
318+
if (this.uriIdentityService.extUri.isEqual(this.uriIdentityService.extUri.joinPath(workspaceFolder.uri, WORKSPACE_STANDALONE_CONFIGURATIONS[MCP_CONFIGURATION_KEY]), server.mcpResource)) {
319+
return `${WORKSPACE_FOLDER_CONFIG_ID_PREFIX}${index}`;
320+
}
321+
}
322+
}
323+
return 'unknown';
292324
}
293325

294326
async install(server: IInstallableMcpServer, options?: IWorkbencMcpServerInstallOptions): Promise<IWorkbenchLocalMcpServer> {

0 commit comments

Comments
 (0)