chat: fix 'server options' button showing for non-mcp servers (microsoft#261128)

connor4312 · web-flow · commit d6b3b495be08 · 2025-08-11T16:44:39.000-07:00
Closes microsoft#260818
diff --git a/src/vs/workbench/contrib/chat/browser/chatContentParts/chatConfirmationWidget.ts b/src/vs/workbench/contrib/chat/browser/chatContentParts/chatConfirmationWidget.ts
@@ -41,7 +41,7 @@ export interface IChatConfirmationWidgetOptions {
 	title: string | IMarkdownString;
 	subtitle?: string | IMarkdownString;
 	buttons: IChatConfirmationButton[];
-	toolbarData?: { arg: any; partType: string };
+	toolbarData?: { arg: any; partType: string; partSource?: string };
 }
 
 export class ChatQueryTitlePart extends Disposable {
@@ -209,7 +209,10 @@ abstract class BaseChatConfirmationWidget extends Disposable {
 
 		// Create toolbar if actions are provided
 		if (options?.toolbarData) {
-			const overlay = contextKeyService.createOverlay([['chatConfirmationPartType', options.toolbarData.partType]]);
+			const overlay = contextKeyService.createOverlay([
+				['chatConfirmationPartType', options.toolbarData.partType],
+				['chatConfirmationPartSource', options.toolbarData.partSource],
+			]);
 			const nestedInsta = this._register(instantiationService.createChild(new ServiceCollection([IContextKeyService, overlay])));
 			this._register(nestedInsta.createInstance(
 				MenuWorkbenchToolBar,
diff --git a/src/vs/workbench/contrib/chat/browser/chatContentParts/chatElicitationContentPart.ts b/src/vs/workbench/contrib/chat/browser/chatContentParts/chatElicitationContentPart.ts
@@ -31,7 +31,7 @@ export class ChatElicitationContentPart extends Disposable implements IChatConte
 			{ label: elicitation.acceptButtonLabel, data: true },
 			{ label: elicitation.rejectButtonLabel, data: false, isSecondary: true },
 		];
-		const confirmationWidget = this._register(this.instantiationService.createInstance(ChatConfirmationWidget, context.container, { title: elicitation.title, subtitle: elicitation.subtitle, buttons, message: this.getMessageToRender(elicitation), toolbarData: { partType: elicitation.source ? `${elicitation.source.type}Elicitation` : 'elicitation', arg: elicitation } }));
+		const confirmationWidget = this._register(this.instantiationService.createInstance(ChatConfirmationWidget, context.container, { title: elicitation.title, subtitle: elicitation.subtitle, buttons, message: this.getMessageToRender(elicitation), toolbarData: { partType: 'elicitation', partSource: elicitation.source?.type, arg: elicitation } }));
 		confirmationWidget.setShowButtons(elicitation.state === 'pending');
 
 		this._register(elicitation.onDidRequestHide(() => this.domNode.remove()));
diff --git a/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatToolConfirmationSubPart.ts b/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatToolConfirmationSubPart.ts
@@ -109,7 +109,7 @@ export class ToolConfirmationSubPart extends BaseChatToolInvocationSubPart {
 			confirmWidget = this._register(this.instantiationService.createInstance(
 				ChatConfirmationWidget,
 				this.context.container,
-				{ title, subtitle: toolInvocation.originMessage, buttons, message, toolbarData: { arg: toolInvocation, partType: 'chatToolConfirmation' } }
+				{ title, subtitle: toolInvocation.originMessage, buttons, message, toolbarData: { arg: toolInvocation, partType: 'chatToolConfirmation', partSource: toolInvocation.source.type } }
 			));
 		} else {
 			const codeBlockRenderOptions: ICodeBlockRenderOptions = {
@@ -275,7 +275,7 @@ export class ToolConfirmationSubPart extends BaseChatToolInvocationSubPart {
 			confirmWidget = this._register(this.instantiationService.createInstance(
 				ChatCustomConfirmationWidget,
 				this.context.container,
-				{ title, subtitle: toolInvocation.originMessage, buttons, message: elements.root, toolbarData: { arg: toolInvocation, partType: 'chatToolConfirmation' } },
+				{ title, subtitle: toolInvocation.originMessage, buttons, message: elements.root, toolbarData: { arg: toolInvocation, partType: 'chatToolConfirmation', partSource: toolInvocation.source?.type } },
 			));
 		}
 
diff --git a/src/vs/workbench/contrib/chat/browser/chatElicitationRequestPart.ts b/src/vs/workbench/contrib/chat/browser/chatElicitationRequestPart.ts
@@ -7,6 +7,7 @@ import { Emitter } from '../../../../base/common/event.js';
 import { IMarkdownString } from '../../../../base/common/htmlContent.js';
 import { Disposable } from '../../../../base/common/lifecycle.js';
 import { IChatElicitationRequest } from '../common/chatService.js';
+import { ToolDataSource } from '../common/languageModelToolsService.js';
 
 export class ChatElicitationRequestPart extends Disposable implements IChatElicitationRequest {
 	public readonly kind = 'elicitation';
@@ -24,7 +25,7 @@ export class ChatElicitationRequestPart extends Disposable implements IChatElici
 		public readonly rejectButtonLabel: string,
 		public readonly accept: () => Promise<void>,
 		public readonly reject: () => Promise<void>,
-		public readonly source?: { type: 'mcp'; definitionId: string },
+		public readonly source?: ToolDataSource,
 	) {
 		super();
 	}
diff --git a/src/vs/workbench/contrib/chat/common/chatProgressTypes/chatToolInvocation.ts b/src/vs/workbench/contrib/chat/common/chatProgressTypes/chatToolInvocation.ts
@@ -9,7 +9,7 @@ import { IMarkdownString } from '../../../../../base/common/htmlContent.js';
 import { observableValue } from '../../../../../base/common/observable.js';
 import { localize } from '../../../../../nls.js';
 import { IChatExtensionsContent, IChatToolInputInvocationData, IChatTodoListContent, IChatToolInvocation, IChatToolInvocationSerialized, type IChatTerminalToolInvocationData } from '../chatService.js';
-import { IPreparedToolInvocation, isToolResultOutputDetails, IToolConfirmationMessages, IToolData, IToolProgressStep, IToolResult } from '../languageModelToolsService.js';
+import { IPreparedToolInvocation, isToolResultOutputDetails, IToolConfirmationMessages, IToolData, IToolProgressStep, IToolResult, ToolDataSource } from '../languageModelToolsService.js';
 
 export class ChatToolInvocation implements IChatToolInvocation {
 	public readonly kind: 'toolInvocation' = 'toolInvocation';
@@ -45,6 +45,7 @@ export class ChatToolInvocation implements IChatToolInvocation {
 	private _confirmationMessages: IToolConfirmationMessages | undefined;
 	public readonly presentation: IPreparedToolInvocation['presentation'];
 	public readonly toolId: string;
+	public readonly source: ToolDataSource;
 
 	public readonly toolSpecificData?: IChatTerminalToolInvocationData | IChatToolInputInvocationData | IChatExtensionsContent | IChatTodoListContent;
 
@@ -60,6 +61,7 @@ export class ChatToolInvocation implements IChatToolInvocation {
 		this.presentation = preparedInvocation?.presentation;
 		this.toolSpecificData = preparedInvocation?.toolSpecificData;
 		this.toolId = toolData.id;
+		this.source = toolData.source;
 
 		if (!this._confirmationMessages) {
 			// No confirmation needed
@@ -107,6 +109,7 @@ export class ChatToolInvocation implements IChatToolInvocation {
 			originMessage: this.originMessage,
 			isConfirmed: this._isConfirmed,
 			isComplete: this._isComplete,
+			source: this.source,
 			resultDetails: isToolResultOutputDetails(this._resultDetails)
 				? { output: { type: 'data', mimeType: this._resultDetails.output.mimeType, base64Data: encodeBase64(this._resultDetails.output.value) } }
 				: this._resultDetails,
diff --git a/src/vs/workbench/contrib/chat/common/chatService.ts b/src/vs/workbench/contrib/chat/common/chatService.ts
@@ -24,7 +24,7 @@ import { IChatParserContext } from './chatRequestParser.js';
 import { IChatRequestVariableEntry } from './chatVariableEntries.js';
 import { IChatRequestVariableValue } from './chatVariables.js';
 import { ChatAgentLocation, ChatModeKind } from './constants.js';
-import { IPreparedToolInvocation, IToolConfirmationMessages, IToolResult, IToolResultInputOutputDetails } from './languageModelToolsService.js';
+import { IPreparedToolInvocation, IToolConfirmationMessages, IToolResult, IToolResultInputOutputDetails, ToolDataSource } from './languageModelToolsService.js';
 
 export interface IChatRequest {
 	message: string;
@@ -263,7 +263,7 @@ export interface IChatElicitationRequest {
 	acceptButtonLabel: string;
 	rejectButtonLabel: string;
 	subtitle?: string | IMarkdownString;
-	source?: { type: 'mcp'; definitionId: string };
+	source?: ToolDataSource;
 	state: 'pending' | 'accepted' | 'rejected';
 	acceptedResult?: Record<string, unknown>;
 	accept(): Promise<void>;
@@ -319,6 +319,7 @@ export interface IChatToolInvocation {
 	invocationMessage: string | IMarkdownString;
 	pastTenseMessage: string | IMarkdownString | undefined;
 	resultDetails: IToolResult['toolResultDetails'];
+	source: ToolDataSource;
 	progress: IObservable<{ message?: string | IMarkdownString; progress: number }>;
 	readonly toolId: string;
 	readonly toolCallId: string;
@@ -351,6 +352,7 @@ export interface IChatToolInvocationSerialized {
 	isComplete: boolean;
 	toolCallId: string;
 	toolId: string;
+	source: ToolDataSource;
 	kind: 'toolInvocationSerialized';
 }
 
diff --git a/src/vs/workbench/contrib/mcp/browser/mcpCommands.ts b/src/vs/workbench/contrib/mcp/browser/mcpCommands.ts
@@ -174,9 +174,12 @@ export class McpConfirmationServerOptionsCommand extends Action2 {
 			f1: false,
 			menu: [{
 				id: MenuId.ChatConfirmationMenu,
-				when: ContextKeyExpr.or(
-					ContextKeyExpr.equals('chatConfirmationPartType', 'chatToolConfirmation'),
-					ContextKeyExpr.equals('chatConfirmationPartType', 'mcpElicitation'),
+				when: ContextKeyExpr.and(
+					ContextKeyExpr.equals('chatConfirmationPartSource', 'mcp'),
+					ContextKeyExpr.or(
+						ContextKeyExpr.equals('chatConfirmationPartType', 'chatToolConfirmation'),
+						ContextKeyExpr.equals('chatConfirmationPartType', 'elicitation'),
+					),
 				),
 				group: 'navigation'
 			}],
@@ -191,7 +194,7 @@ export class McpConfirmationServerOptionsCommand extends Action2 {
 				accessor.get(ICommandService).executeCommand(McpCommandIds.ServerOptions, tool.source.definitionId);
 			}
 		} else if (arg.kind === 'elicitation') {
-			if (arg.source) {
+			if (arg.source?.type === 'mcp') {
 				accessor.get(ICommandService).executeCommand(McpCommandIds.ServerOptions, arg.source.definitionId);
 			}
 		} else {
diff --git a/src/vs/workbench/contrib/mcp/browser/mcpElicitationService.ts b/src/vs/workbench/contrib/mcp/browser/mcpElicitationService.ts
@@ -14,6 +14,7 @@ import { ChatElicitationRequestPart } from '../../chat/browser/chatElicitationRe
 import { ChatModel } from '../../chat/common/chatModel.js';
 import { IChatService } from '../../chat/common/chatService.js';
 import { IMcpElicitationService, IMcpServer, IMcpToolCallContext } from '../common/mcpTypes.js';
+import { mcpServerToSourceData } from '../common/mcpTypesUtils.js';
 import { MCP } from '../common/modelContextProtocol.js';
 
 const noneItem: IQuickPickItem = { id: undefined, label: localize('mcp.elicit.enum.none', 'None'), description: localize('mcp.elicit.enum.none.description', 'No selection'), alwaysShow: true };
@@ -52,7 +53,7 @@ export class McpElicitationService implements IMcpElicitationService {
 							part.state = 'rejected';
 							return Promise.resolve();
 						},
-						{ type: 'mcp', definitionId: server.definition.id },
+						mcpServerToSourceData(server),
 					);
 					chatModel.acceptResponseProgress(request, part);
 				}
diff --git a/src/vs/workbench/contrib/mcp/common/mcpLanguageModelToolContribution.ts b/src/vs/workbench/contrib/mcp/common/mcpLanguageModelToolContribution.ts
@@ -25,6 +25,7 @@ import { ChatResponseResource, getAttachableImageExtension } from '../../chat/co
 import { CountTokensCallback, ILanguageModelToolsService, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, IToolResultInputOutputDetails, ToolDataSource, ToolProgress, ToolSet } from '../../chat/common/languageModelToolsService.js';
 import { IMcpRegistry } from './mcpRegistryTypes.js';
 import { IMcpServer, IMcpService, IMcpTool, LazyCollectionState, McpResourceURI, McpServerCacheState } from './mcpTypes.js';
+import { mcpServerToSourceData } from './mcpTypesUtils.js';
 
 interface ISyncedToolData {
 	toolData: IToolData;
@@ -65,15 +66,7 @@ export class McpLanguageModelToolContribution extends Disposable implements IWor
 
 				const store = new DisposableStore();
 				const toolSet = new Lazy(() => {
-					const metadata = server.serverMetadata.get();
-					const source: ToolDataSource = {
-						type: 'mcp',
-						serverLabel: metadata?.serverName,
-						instructions: metadata?.serverInstructions,
-						label: server.definition.label,
-						collectionId: server.collection.id,
-						definitionId: server.definition.id
-					};
+					const source = mcpServerToSourceData(server);
 					const toolSet = store.add(this._toolsService.createToolSet(
 						source,
 						server.definition.id, server.definition.label,
diff --git a/src/vs/workbench/contrib/mcp/common/mcpTypesUtils.ts b/src/vs/workbench/contrib/mcp/common/mcpTypesUtils.ts
@@ -8,6 +8,7 @@ import { CancellationToken } from '../../../../base/common/cancellation.js';
 import { CancellationError } from '../../../../base/common/errors.js';
 import { DisposableStore } from '../../../../base/common/lifecycle.js';
 import { autorun } from '../../../../base/common/observable.js';
+import { ToolDataSource } from '../../chat/common/languageModelToolsService.js';
 import { IMcpServer, IMcpServerStartOpts, IMcpService, McpConnectionState, McpServerCacheState } from './mcpTypes.js';
 
 /**
@@ -72,3 +73,15 @@ export function startServerAndWaitForLiveTools(server: IMcpServer, opts?: IMcpSe
 		});
 	}).finally(() => store.dispose());
 }
+
+export function mcpServerToSourceData(server: IMcpServer): ToolDataSource {
+	const metadata = server.serverMetadata.get();
+	return {
+		type: 'mcp',
+		serverLabel: metadata?.serverName,
+		instructions: metadata?.serverInstructions,
+		label: server.definition.label,
+		collectionId: server.collection.id,
+		definitionId: server.definition.id
+	};
+}
