Skip to content

Commit 3c25740

Browse files
joelverhagenjoelverhagen
authored
Support server.json being returned by assisted MCP install (microsoft#259634)
* Support server.json being returned by assisted install * Fix NuGet command building
1 parent 6800196 commit 3c25740

File tree

2 files changed

+65
-16
lines changed

2 files changed

+65
-16
lines changed

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ export abstract class AbstractMcpResourceManagementService extends Disposable {
232232
}
233233
}
234234

235-
protected toScannedMcpServerAndInputs(manifest: IMcpServerManifest, packageType?: PackageType): { config: IMcpServerConfiguration; inputs?: IMcpServerVariable[] } {
235+
static toScannedMcpServerAndInputs(manifest: IMcpServerManifest, packageType?: PackageType): { config: IMcpServerConfiguration; inputs?: IMcpServerVariable[] } {
236236
if (packageType === undefined) {
237237
packageType = manifest.packages?.[0]?.registry_name ?? PackageType.REMOTE;
238238
}
@@ -246,7 +246,7 @@ export abstract class AbstractMcpResourceManagementService extends Disposable {
246246
const variables = input.variables ? this.getVariables(input.variables) : [];
247247
let value = input.value;
248248
for (const variable of variables) {
249-
value = value.replace(`{${variable.id}}`, `{input:${variable.id}}`);
249+
value = value.replace(`{${variable.id}}`, `\${input:${variable.id}}`);
250250
}
251251
headers[input.name] = value;
252252
if (variables.length) {
@@ -279,7 +279,7 @@ export abstract class AbstractMcpResourceManagementService extends Disposable {
279279
let value = arg.value;
280280
if (value) {
281281
for (const variable of variables) {
282-
value = value.replace(`{${variable.id}}`, `{input:${variable.id}}`);
282+
value = value.replace(`{${variable.id}}`, `\${input:${variable.id}}`);
283283
}
284284
}
285285
args.push(value ?? arg.value_hint);
@@ -288,7 +288,7 @@ export abstract class AbstractMcpResourceManagementService extends Disposable {
288288
if (arg.value) {
289289
let value = arg.value;
290290
for (const variable of variables) {
291-
value = value.replace(`{${variable.id}}`, `{input:${variable.id}}`);
291+
value = value.replace(`{${variable.id}}`, `\${input:${variable.id}}`);
292292
}
293293
args.push(value);
294294
}
@@ -302,7 +302,7 @@ export abstract class AbstractMcpResourceManagementService extends Disposable {
302302
const variables = input.variables ? this.getVariables(input.variables) : [];
303303
let value = input.value;
304304
for (const variable of variables) {
305-
value = value.replace(`{${variable.id}}`, `{input:${variable.id}}`);
305+
value = value.replace(`{${variable.id}}`, `\${input:${variable.id}}`);
306306
}
307307
env[input.name] = value;
308308
if (variables.length) {
@@ -325,10 +325,10 @@ export abstract class AbstractMcpResourceManagementService extends Disposable {
325325
}
326326
else if (serverPackage.registry_name === PackageType.NUGET) {
327327
args.push(serverPackage.version ? `${serverPackage.name}@${serverPackage.version}` : serverPackage.name);
328-
}
329-
330-
if (serverPackage.package_arguments && serverPackage.registry_name === PackageType.NUGET) {
331-
args.push('--');
328+
args.push('--yes'); // installation is confirmed by the UI, so --yes is appropriate here
329+
if (serverPackage.package_arguments?.length) {
330+
args.push('--');
331+
}
332332
}
333333

334334
for (const arg of serverPackage.package_arguments ?? []) {
@@ -337,7 +337,7 @@ export abstract class AbstractMcpResourceManagementService extends Disposable {
337337
let value = arg.value;
338338
if (value) {
339339
for (const variable of variables) {
340-
value = value.replace(`{${variable.id}}`, `{input:${variable.id}}`);
340+
value = value.replace(`{${variable.id}}`, `\${input:${variable.id}}`);
341341
}
342342
}
343343
args.push(value ?? arg.value_hint);
@@ -346,7 +346,7 @@ export abstract class AbstractMcpResourceManagementService extends Disposable {
346346
if (arg.value) {
347347
let value = arg.value;
348348
for (const variable of variables) {
349-
value = value.replace(`{${variable.id}}`, `{input:${variable.id}}`);
349+
value = value.replace(`{${variable.id}}`, `\${input:${variable.id}}`);
350350
}
351351
args.push(value);
352352
}
@@ -370,7 +370,7 @@ export abstract class AbstractMcpResourceManagementService extends Disposable {
370370
};
371371
}
372372

373-
private getCommandName(packageType: PackageType): string {
373+
private static getCommandName(packageType: PackageType): string {
374374
switch (packageType) {
375375
case PackageType.NODE: return 'npx';
376376
case PackageType.DOCKER: return 'docker';
@@ -380,7 +380,7 @@ export abstract class AbstractMcpResourceManagementService extends Disposable {
380380
return packageType;
381381
}
382382

383-
private getVariables(variableInputs: Record<string, IMcpServerInput>): IMcpServerVariable[] {
383+
private static getVariables(variableInputs: Record<string, IMcpServerInput>): IMcpServerVariable[] {
384384
const variables: IMcpServerVariable[] = [];
385385
for (const [key, value] of Object.entries(variableInputs)) {
386386
variables.push({
@@ -425,7 +425,7 @@ export class McpUserResourceManagementService extends AbstractMcpResourceManagem
425425

426426
try {
427427
const manifest = await this.updateMetadataFromGallery(server);
428-
const { config, inputs } = this.toScannedMcpServerAndInputs(manifest, options?.packageType);
428+
const { config, inputs } = AbstractMcpResourceManagementService.toScannedMcpServerAndInputs(manifest, options?.packageType);
429429
const installable: IInstallableMcpServer = {
430430
name: server.name,
431431
config: {

‎src/vs/workbench/contrib/mcp/browser/mcpCommandsAddConfiguration.ts‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import { ICommandService } from '../../../../platform/commands/common/commands.j
1818
import { ConfigurationTarget, IConfigurationService } from '../../../../platform/configuration/common/configuration.js';
1919
import { IFileService } from '../../../../platform/files/common/files.js';
2020
import { ILabelService } from '../../../../platform/label/common/label.js';
21+
import { IMcpServerManifest, PackageType } from '../../../../platform/mcp/common/mcpManagement.js';
22+
import { AbstractMcpResourceManagementService } from '../../../../platform/mcp/common/mcpManagementService.js';
2123
import { IMcpRemoteServerConfiguration, IMcpServerConfiguration, IMcpServerVariable, IMcpStdioServerConfiguration, McpServerType } from '../../../../platform/mcp/common/mcpPlatformTypes.js';
2224
import { INotificationService } from '../../../../platform/notification/common/notification.js';
2325
import { IOpenerService } from '../../../../platform/opener/common/opener.js';
@@ -111,6 +113,18 @@ type AddServerCompletedClassification = {
111113
target: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'The target of the MCP server configuration' };
112114
};
113115

116+
type AssistedServerConfiguration = {
117+
type?: 'vscode';
118+
name?: string;
119+
server: Omit<IMcpStdioServerConfiguration, 'type'>;
120+
inputs?: IMcpServerVariable[];
121+
inputValues?: Record<string, string>;
122+
} | {
123+
type: 'server.json';
124+
name?: string;
125+
server: IMcpServerManifest;
126+
};
127+
114128
export class McpAddConfigurationCommand {
115129
constructor(
116130
private readonly workspaceFolder: IWorkspaceFolder | undefined,
@@ -284,7 +298,7 @@ export class McpAddConfigurationCommand {
284298
return targetPick?.target;
285299
}
286300

287-
private async getAssistedConfig(type: AssistedConfigurationType): Promise<{ name: string; server: Omit<IMcpStdioServerConfiguration, 'type'>; inputs?: IMcpServerVariable[]; inputValues?: Record<string, string> } | undefined> {
301+
private async getAssistedConfig(type: AssistedConfigurationType): Promise<{ name?: string; server: Omit<IMcpStdioServerConfiguration, 'type'>; inputs?: IMcpServerVariable[]; inputValues?: Record<string, string> } | undefined> {
288302
const packageName = await this._quickInputService.input({
289303
ignoreFocusLost: true,
290304
title: AssistedTypes[type].title,
@@ -384,13 +398,33 @@ export class McpAddConfigurationCommand {
384398
return undefined;
385399
}
386400

387-
return await this._commandService.executeCommand<{ name: string; server: Omit<IMcpStdioServerConfiguration, 'type'>; inputs?: IMcpServerVariable[]; inputValues?: Record<string, string> }>(
401+
const config = await this._commandService.executeCommand<AssistedServerConfiguration>(
388402
AddConfigurationCopilotCommand.StartFlow,
389403
{
390404
name: packageName,
391405
type: packageType
392406
}
393407
);
408+
409+
if (config?.type === 'server.json') {
410+
const packageType = this.getPackageTypeEnum(type);
411+
if (!packageType) {
412+
throw new Error(`Unsupported assisted package type ${type}`);
413+
}
414+
const server = AbstractMcpResourceManagementService.toScannedMcpServerAndInputs(config.server, packageType);
415+
if (server.config.type !== McpServerType.LOCAL) {
416+
throw new Error(`Unexpected server type ${server.config.type} for assisted configuration from server.json.`);
417+
}
418+
return {
419+
name: config.name,
420+
server: server.config,
421+
inputs: server.inputs,
422+
};
423+
} else if (config?.type === 'vscode' || !config?.type) {
424+
return config;
425+
} else {
426+
assertNever(config?.type);
427+
}
394428
}
395429

396430
/** Shows the location of a server config once it's discovered. */
@@ -550,6 +584,21 @@ export class McpAddConfigurationCommand {
550584
}
551585
}
552586

587+
private getPackageTypeEnum(type: AddConfigurationType): PackageType | undefined {
588+
switch (type) {
589+
case AddConfigurationType.NpmPackage:
590+
return PackageType.NODE;
591+
case AddConfigurationType.PipPackage:
592+
return PackageType.PYTHON;
593+
case AddConfigurationType.NuGetPackage:
594+
return PackageType.NUGET;
595+
case AddConfigurationType.DockerImage:
596+
return PackageType.DOCKER;
597+
default:
598+
return undefined;
599+
}
600+
}
601+
553602
private getPackageType(serverType: AddConfigurationType): string | undefined {
554603
switch (serverType) {
555604
case AddConfigurationType.NpmPackage:

0 commit comments

Comments
 (0)