From a2ebac10a33cae8a38d9e07656fce30352014fb7 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Mon, 2 Jun 2025 17:13:38 -0700 Subject: [PATCH] tools: implement tool confirmation polish (#250351) Closes https://github.com/microsoft/vscode/issues/248567 --- .../chatToolConfirmationSubPart.ts | 61 +++++++++++++++---- .../contrib/chat/browser/media/chat.css | 34 +++++++++-- .../chat/common/languageModelToolsService.ts | 1 + .../contrib/mcp/common/mcpService.ts | 6 +- 4 files changed, 81 insertions(+), 21 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatToolConfirmationSubPart.ts b/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatToolConfirmationSubPart.ts index f903a543643..85170e72008 100644 --- a/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatToolConfirmationSubPart.ts +++ b/src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatToolConfirmationSubPart.ts @@ -5,10 +5,12 @@ import * as dom from '../../../../../../base/browser/dom.js'; import { RunOnceScheduler } from '../../../../../../base/common/async.js'; +import { IMarkdownString, MarkdownString } from '../../../../../../base/common/htmlContent.js'; import { toDisposable } from '../../../../../../base/common/lifecycle.js'; import { count } from '../../../../../../base/common/strings.js'; import { isEmptyObject } from '../../../../../../base/common/types.js'; import { generateUuid } from '../../../../../../base/common/uuid.js'; +import { ElementSizeObserver } from '../../../../../../editor/browser/config/elementSizeObserver.js'; import { MarkdownRenderer } from '../../../../../../editor/browser/widget/markdownRenderer/browser/markdownRenderer.js'; import { ILanguageService } from '../../../../../../editor/common/languages/language.js'; import { IModelService } from '../../../../../../editor/common/services/model.js'; @@ -19,7 +21,7 @@ import { IInstantiationService } from '../../../../../../platform/instantiation/ import { IKeybindingService } from '../../../../../../platform/keybinding/common/keybinding.js'; import { IMarkerData, IMarkerService, MarkerSeverity } from '../../../../../../platform/markers/common/markers.js'; import { ChatContextKeys } from '../../../common/chatContextKeys.js'; -import { IChatMarkdownContent, IChatToolInvocation } from '../../../common/chatService.js'; +import { IChatToolInvocation } from '../../../common/chatService.js'; import { CodeBlockModelCollection } from '../../../common/codeBlockModelCollection.js'; import { createToolInputUri, createToolSchemaUri, ILanguageModelToolsService } from '../../../common/languageModelToolsService.js'; import { CancelChatActionId } from '../../actions/chatExecuteActions.js'; @@ -33,12 +35,14 @@ import { IChatMarkdownAnchorService } from '../chatMarkdownAnchorService.js'; import { ChatMarkdownContentPart, EditorPool } from '../chatMarkdownContentPart.js'; import { BaseChatToolInvocationSubPart } from './chatToolInvocationSubPart.js'; +const SHOW_MORE_MESSAGE_HEIGHT_TRIGGER = 30; + export class ToolConfirmationSubPart extends BaseChatToolInvocationSubPart { public readonly domNode: HTMLElement; - private markdownPart: ChatMarkdownContentPart | undefined; + private markdownParts: ChatMarkdownContentPart[] = []; public get codeblocks(): IChatCodeBlockInfo[] { - return this.markdownPart?.codeblocks ?? []; + return this.markdownParts.flatMap(part => part.codeblocks); } constructor( @@ -65,7 +69,7 @@ export class ToolConfirmationSubPart extends BaseChatToolInvocationSubPart { if (!toolInvocation.confirmationMessages) { throw new Error('Confirmation messages are missing'); } - const { title, message, allowAutoConfirm } = toolInvocation.confirmationMessages; + const { title, message, allowAutoConfirm, disclaimer } = toolInvocation.confirmationMessages; const continueLabel = localize('continue', "Continue"); const continueKeybinding = keybindingService.lookupKeybinding(AcceptToolConfirmationActionId)?.getLabel(); const continueTooltip = continueKeybinding ? `${continueLabel} (${continueKeybinding})` : continueLabel; @@ -109,10 +113,6 @@ export class ToolConfirmationSubPart extends BaseChatToolInvocationSubPart { this.context.container, )); } else { - const chatMarkdownContent: IChatMarkdownContent = { - kind: 'markdownContent', - content: message, - }; const codeBlockRenderOptions: ICodeBlockRenderOptions = { hideToolbar: true, reserveWidth: 19, @@ -125,12 +125,20 @@ export class ToolConfirmationSubPart extends BaseChatToolInvocationSubPart { }; const elements = dom.h('div', [ - dom.h('.message@message'), + dom.h('.message@messageContainer', [ + dom.h('.message-wrapper@message'), + dom.h('a.see-more@showMore'), + ]), dom.h('.editor@editor'), + dom.h('.disclaimer@disclaimer'), ]); if (toolInvocation.toolSpecificData?.kind === 'input' && toolInvocation.toolSpecificData.rawInput && !isEmptyObject(toolInvocation.toolSpecificData.rawInput)) { + const title = document.createElement('h3'); + title.textContent = localize('chat.input', "Input"); + elements.editor.appendChild(title); + const inputData = toolInvocation.toolSpecificData; const codeBlockRenderOptions: ICodeBlockRenderOptions = { @@ -238,11 +246,30 @@ export class ToolConfirmationSubPart extends BaseChatToolInvocationSubPart { } } - this.markdownPart = this._register(this.instantiationService.createInstance(ChatMarkdownContentPart, chatMarkdownContent, this.context, this.editorPool, false, this.codeBlockStartIndex, this.renderer, this.currentWidthDelegate(), this.codeBlockModelCollection, { codeBlockRenderOptions })); - renderFileWidgets(this.markdownPart.domNode, this.instantiationService, this.chatMarkdownAnchorService, this._store); - elements.message.append(this.markdownPart.domNode); + this._makeMarkdownPart(elements.message, message, codeBlockRenderOptions); + elements.showMore.textContent = localize('seeMore', "See more"); + + const messageSeeMoreObserver = this._register(new ElementSizeObserver(elements.message, undefined)); + const updateSeeMoreDisplayed = () => { + const show = messageSeeMoreObserver.getHeight() > SHOW_MORE_MESSAGE_HEIGHT_TRIGGER; + elements.messageContainer.classList.toggle('can-see-more', show); + }; + + this._register(dom.addDisposableListener(elements.showMore, 'click', () => { + elements.messageContainer.classList.toggle('can-see-more', false); + messageSeeMoreObserver.dispose(); + })); + + + this._register(messageSeeMoreObserver.onDidChange(updateSeeMoreDisplayed)); + messageSeeMoreObserver.startObserving(); + + if (disclaimer) { + this._makeMarkdownPart(elements.disclaimer, disclaimer, codeBlockRenderOptions); + } else { + elements.disclaimer.remove(); + } - this._register(this.markdownPart.onDidChangeHeight(() => this._onDidChangeHeight.fire())); confirmWidget = this._register(this.instantiationService.createInstance( ChatCustomConfirmationWidget, title, @@ -288,4 +315,12 @@ export class ToolConfirmationSubPart extends BaseChatToolInvocationSubPart { }); this.domNode = confirmWidget.domNode; } + + private _makeMarkdownPart(container: HTMLElement, message: string | IMarkdownString, codeBlockRenderOptions: ICodeBlockRenderOptions) { + const part = this._register(this.instantiationService.createInstance(ChatMarkdownContentPart, { kind: 'markdownContent', content: typeof message === 'string' ? new MarkdownString().appendText(message) : message }, this.context, this.editorPool, false, this.codeBlockStartIndex, this.renderer, this.currentWidthDelegate(), this.codeBlockModelCollection, { codeBlockRenderOptions })); + renderFileWidgets(part.domNode, this.instantiationService, this.chatMarkdownAnchorService, this._store); + container.append(part.domNode); + + this._register(part.onDidChangeHeight(() => this._onDidChangeHeight.fire())); + } } diff --git a/src/vs/workbench/contrib/chat/browser/media/chat.css b/src/vs/workbench/contrib/chat/browser/media/chat.css index 422481d7a7d..ce66ce94e48 100644 --- a/src/vs/workbench/contrib/chat/browser/media/chat.css +++ b/src/vs/workbench/contrib/chat/browser/media/chat.css @@ -389,6 +389,35 @@ .interactive-item-container .value > .chat-tool-invocation-part { .rendered-markdown p { margin: 0 0 6px 0; + opacity: 0.85; + } + + .disclaimer { + margin-top: 6px; + margin-bottom: -6px; + + .rendered-markdown p:last-child { + margin-bottom: 0; + } + } + + .message .see-more { + display: none; + color: var(--vscode-textLink-foreground); + text-decoration: underline; + cursor: pointer; + } + + .message.can-see-more { + .message-wrapper { + mask-image: linear-gradient(#000 0%, transparent 100%); + pointer-events: none; + max-height: 2.5em; + } + + .see-more { + display: block; + } } .progress-container .rendered-markdown [data-code] { @@ -1121,11 +1150,6 @@ have to be updated for changes to the rules above, or to support more deeply nes .chat-attached-context .chat-prompt-attachment .monaco-icon-label-container { margin-top: -0.1em; } -/* - * This overly-specific CSS selector is needed to beat priority of some - * styles applied on the `.chat-attached-context-attachment` element. - */ -.chat-attached-context .chat-prompt-attachment.error.implicit, .chat-attached-context .chat-prompt-attachment.warning.implicit { border: 1px solid currentColor; } diff --git a/src/vs/workbench/contrib/chat/common/languageModelToolsService.ts b/src/vs/workbench/contrib/chat/common/languageModelToolsService.ts index 6d04b0b1168..ef9dd06cd17 100644 --- a/src/vs/workbench/contrib/chat/common/languageModelToolsService.ts +++ b/src/vs/workbench/contrib/chat/common/languageModelToolsService.ts @@ -179,6 +179,7 @@ export interface IToolResultDataPart { export interface IToolConfirmationMessages { title: string | IMarkdownString; message: string | IMarkdownString; + disclaimer?: string | IMarkdownString; allowAutoConfirm?: boolean; } diff --git a/src/vs/workbench/contrib/mcp/common/mcpService.ts b/src/vs/workbench/contrib/mcp/common/mcpService.ts index ed3397f59b8..e26872293f1 100644 --- a/src/vs/workbench/contrib/mcp/common/mcpService.ts +++ b/src/vs/workbench/contrib/mcp/common/mcpService.ts @@ -251,8 +251,7 @@ class McpToolImplementation implements IToolImpl { const mcpToolWarning = localize( 'mcp.tool.warning', - "{0} Note that MCP servers or malicious conversation content may attempt to misuse '{1}' through tools.", - '$(info)', + "Note that MCP servers or malicious conversation content may attempt to misuse '{0}' through tools.", this._productService.nameShort ); @@ -263,7 +262,8 @@ class McpToolImplementation implements IToolImpl { return { confirmationMessages: needsConfirmation ? { title: new MarkdownString(localize('msg.title', "Run {0}", title)), - message: new MarkdownString(localize('msg.msg', "{0}\n\n {1}", tool.definition.description, mcpToolWarning), { supportThemeIcons: true }), + message: new MarkdownString(tool.definition.description, { supportThemeIcons: true }), + disclaimer: mcpToolWarning, allowAutoConfirm: true, } : undefined, invocationMessage: new MarkdownString(localize('msg.run', "Running {0}", title)),