Skip to content

Commit d668f83

Browse files
roblourensroblourens
authored
Fixes for tool invocations/confirmations to work better with external agents (#264180)
For #262756
1 parent f4bea1d commit d668f83

File tree

11 files changed

+57
-28
lines changed

11 files changed

+57
-28
lines changed

‎src/vs/workbench/api/common/extHostLanguageModelTools.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { IDisposable, toDisposable } from '../../../base/common/lifecycle.js';
1111
import { revive } from '../../../base/common/marshalling.js';
1212
import { generateUuid } from '../../../base/common/uuid.js';
1313
import { IExtensionDescription } from '../../../platform/extensions/common/extensions.js';
14-
import { IPreparedToolInvocation, isToolInvocationContext, IToolInvocation, IToolInvocationContext, IToolInvocationPreparationContext, IToolResult } from '../../contrib/chat/common/languageModelToolsService.js';
14+
import { IPreparedToolInvocation, isToolInvocationContext, IToolInvocation, IToolInvocationContext, IToolInvocationPreparationContext, IToolResult, ToolInvocationPresentation } from '../../contrib/chat/common/languageModelToolsService.js';
1515
import { ExtensionEditToolId, InternalEditToolId } from '../../contrib/chat/common/tools/editFileTool.js';
1616
import { InternalFetchWebPageToolId } from '../../contrib/chat/common/tools/tools.js';
1717
import { checkProposedApiEnabled, isProposedApiEnabled } from '../../services/extensions/common/extensions.js';
@@ -264,7 +264,7 @@ export class ExtHostLanguageModelTools implements ExtHostLanguageModelToolsShape
264264
} : undefined,
265265
invocationMessage: typeConvert.MarkdownString.fromStrict(result.invocationMessage),
266266
pastTenseMessage: typeConvert.MarkdownString.fromStrict(result.pastTenseMessage),
267-
presentation: result.presentation
267+
presentation: result.presentation as ToolInvocationPresentation | undefined
268268
};
269269
}
270270

‎src/vs/workbench/api/common/extHostTypeConverters.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,24 +34,28 @@ import * as languages from '../../../editor/common/languages.js';
3434
import { EndOfLineSequence, TrackedRangeStickiness } from '../../../editor/common/model.js';
3535
import { ITextEditorOptions } from '../../../platform/editor/common/editor.js';
3636
import { IExtensionDescription, IRelaxedExtensionDescription } from '../../../platform/extensions/common/extensions.js';
37+
import { ILogService } from '../../../platform/log/common/log.js';
3738
import { IMarkerData, IRelatedInformation, MarkerSeverity, MarkerTag } from '../../../platform/markers/common/markers.js';
3839
import { ProgressLocation as MainProgressLocation } from '../../../platform/progress/common/progress.js';
3940
import { DEFAULT_EDITOR_ASSOCIATION, SaveReason } from '../../common/editor.js';
4041
import { IViewBadge } from '../../common/views.js';
4142
import { IChatAgentRequest, IChatAgentResult } from '../../contrib/chat/common/chatAgents.js';
4243
import { IChatRequestDraft } from '../../contrib/chat/common/chatEditingService.js';
44+
import { IChatAgentMarkdownContentWithVulnerability, IChatCodeCitation, IChatCommandButton, IChatConfirmation, IChatContentInlineReference, IChatContentReference, IChatExtensionsContent, IChatFollowup, IChatMarkdownContent, IChatMoveMessage, IChatMultiDiffData, IChatPrepareToolInvocationPart, IChatProgressMessage, IChatPullRequestContent, IChatResponseCodeblockUriPart, IChatTaskDto, IChatTaskResult, IChatTextEdit, IChatThinkingPart, IChatToolInvocationSerialized, IChatTreeData, IChatUserActionEvent, IChatWarningMessage } from '../../contrib/chat/common/chatService.js';
4345
import { IChatRequestVariableEntry, isImageVariableEntry } from '../../contrib/chat/common/chatVariableEntries.js';
44-
import { IChatAgentMarkdownContentWithVulnerability, IChatCodeCitation, IChatCommandButton, IChatConfirmation, IChatContentInlineReference, IChatContentReference, IChatExtensionsContent, IChatFollowup, IChatMarkdownContent, IChatMoveMessage, IChatMultiDiffData, IChatPrepareToolInvocationPart, IChatProgressMessage, IChatPullRequestContent, IChatResponseCodeblockUriPart, IChatTaskDto, IChatTaskResult, IChatTextEdit, IChatThinkingPart, IChatTreeData, IChatUserActionEvent, IChatWarningMessage } from '../../contrib/chat/common/chatService.js';
46+
import { ChatAgentLocation } from '../../contrib/chat/common/constants.js';
4547
import { IToolResult, IToolResultInputOutputDetails, IToolResultOutputDetails, ToolDataSource } from '../../contrib/chat/common/languageModelToolsService.js';
4648
import * as chatProvider from '../../contrib/chat/common/languageModels.js';
4749
import { IChatMessageDataPart, IChatResponseDataPart, IChatResponsePromptTsxPart, IChatResponseTextPart } from '../../contrib/chat/common/languageModels.js';
4850
import { DebugTreeItemCollapsibleState, IDebugVisualizationTreeItem } from '../../contrib/debug/common/debug.js';
51+
import { McpServerLaunch, McpServerTransportType } from '../../contrib/mcp/common/mcpTypes.js';
4952
import * as notebooks from '../../contrib/notebook/common/notebookCommon.js';
5053
import { CellEditType } from '../../contrib/notebook/common/notebookCommon.js';
5154
import { ICellRange } from '../../contrib/notebook/common/notebookRange.js';
5255
import * as search from '../../contrib/search/common/search.js';
5356
import { TestId } from '../../contrib/testing/common/testId.js';
5457
import { CoverageDetails, DetailType, ICoverageCount, IFileCoverage, ISerializedTestResults, ITestErrorMessage, ITestItem, ITestRunProfileReference, ITestTag, TestMessageType, TestResultItem, TestRunProfileBitset, denamespaceTestTag, namespaceTestTag } from '../../contrib/testing/common/testTypes.js';
58+
import { AiSettingsSearchResult, AiSettingsSearchResultKind } from '../../services/aiSettingsSearch/common/aiSettingsSearch.js';
5559
import { EditorGroupColumn } from '../../services/editor/common/editorGroupColumn.js';
5660
import { ACTIVE_GROUP, SIDE_GROUP } from '../../services/editor/common/editorService.js';
5761
import { checkProposedApiEnabled, isProposedApiEnabled } from '../../services/extensions/common/extensions.js';
@@ -61,10 +65,6 @@ import { CommandsConverter } from './extHostCommands.js';
6165
import { getPrivateApiFor } from './extHostTestingPrivateApi.js';
6266
import * as types from './extHostTypes.js';
6367
import { LanguageModelDataPart, LanguageModelPromptTsxPart, LanguageModelTextPart } from './extHostTypes.js';
64-
import { ChatAgentLocation } from '../../contrib/chat/common/constants.js';
65-
import { AiSettingsSearchResult, AiSettingsSearchResultKind } from '../../services/aiSettingsSearch/common/aiSettingsSearch.js';
66-
import { McpServerLaunch, McpServerTransportType } from '../../contrib/mcp/common/mcpTypes.js';
67-
import { ILogService } from '../../../platform/log/common/log.js';
6868

6969
export namespace Command {
7070

@@ -2784,7 +2784,7 @@ export namespace ChatPrepareToolInvocationPart {
27842784
}
27852785

27862786
export namespace ChatToolInvocationPart {
2787-
export function from(part: vscode.ChatToolInvocationPart): any {
2787+
export function from(part: vscode.ChatToolInvocationPart): IChatToolInvocationSerialized {
27882788
// Convert extension API ChatToolInvocationPart to internal serialized format
27892789
return {
27902790
kind: 'toolInvocationSerialized',
@@ -2795,8 +2795,8 @@ export namespace ChatToolInvocationPart {
27952795
pastTenseMessage: part.pastTenseMessage ? MarkdownString.from(part.pastTenseMessage) : undefined,
27962796
isConfirmed: part.isConfirmed,
27972797
isComplete: part.isComplete ?? true,
2798-
isError: part.isError ?? false,
2799-
resultDetails: undefined,
2798+
source: ToolDataSource.External,
2799+
// isError: part.isError ?? false,
28002800
toolSpecificData: part.toolSpecificData ? convertToolSpecificData(part.toolSpecificData) : undefined,
28012801
presentation: undefined
28022802
};

‎src/vs/workbench/contrib/chat/browser/actions/chatToolPicker.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,8 @@ export async function showToolsPicker(
234234
return BucketOrdinal.BuiltIn.toString();
235235
case 'user':
236236
return BucketOrdinal.User.toString();
237+
case 'external':
238+
throw new Error('should not be reachable');
237239
default:
238240
assertNever(source);
239241
}

‎src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatToolInvocationPart.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,26 +8,26 @@ import { Emitter } from '../../../../../../base/common/event.js';
88
import { markdownCommandLink, MarkdownString } from '../../../../../../base/common/htmlContent.js';
99
import { Disposable, DisposableStore, IDisposable } from '../../../../../../base/common/lifecycle.js';
1010
import { MarkdownRenderer } from '../../../../../../editor/browser/widget/markdownRenderer/browser/markdownRenderer.js';
11+
import { localize } from '../../../../../../nls.js';
1112
import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js';
1213
import { IChatToolInvocation, IChatToolInvocationSerialized, ToolConfirmKind } from '../../../common/chatService.js';
1314
import { IChatRendererContent } from '../../../common/chatViewModel.js';
1415
import { CodeBlockModelCollection } from '../../../common/codeBlockModelCollection.js';
15-
import { isToolResultInputOutputDetails, isToolResultOutputDetails } from '../../../common/languageModelToolsService.js';
16+
import { isToolResultInputOutputDetails, isToolResultOutputDetails, ToolInvocationPresentation } from '../../../common/languageModelToolsService.js';
1617
import { ChatTreeItem, IChatCodeBlockInfo } from '../../chat.js';
1718
import { IChatContentPart, IChatContentPartRenderContext } from '../chatContentParts.js';
1819
import { EditorPool } from '../chatMarkdownContentPart.js';
1920
import { CollapsibleListPool } from '../chatReferencesContentPart.js';
2021
import { ExtensionsInstallConfirmationWidgetSubPart } from './chatExtensionsInstallToolSubPart.js';
2122
import { ChatInputOutputMarkdownProgressPart } from './chatInputOutputMarkdownProgressPart.js';
2223
import { ChatResultListSubPart } from './chatResultListSubPart.js';
23-
import { ChatTerminalToolProgressPart } from './chatTerminalToolProgressPart.js';
2424
import { ChatTerminalToolConfirmationSubPart } from './chatTerminalToolConfirmationSubPart.js';
25+
import { ChatTerminalToolProgressPart } from './chatTerminalToolProgressPart.js';
26+
import { ChatTodoListSubPart } from './chatTodoListSubPart.js';
2527
import { ToolConfirmationSubPart } from './chatToolConfirmationSubPart.js';
2628
import { BaseChatToolInvocationSubPart } from './chatToolInvocationSubPart.js';
2729
import { ChatToolOutputSubPart } from './chatToolOutputPart.js';
2830
import { ChatToolProgressSubPart } from './chatToolProgressPart.js';
29-
import { localize } from '../../../../../../nls.js';
30-
import { ChatTodoListSubPart } from './chatTodoListSubPart.js';
3131

3232
export class ChatToolInvocationPart extends Disposable implements IChatContentPart {
3333
public readonly domNode: HTMLElement;
@@ -71,6 +71,10 @@ export class ChatToolInvocationPart extends Disposable implements IChatContentPa
7171
dom.clearNode(this.domNode);
7272
partStore.clear();
7373

74+
if (toolInvocation.presentation === ToolInvocationPresentation.HiddenAfterComplete && toolInvocation.isComplete) {
75+
return;
76+
}
77+
7478
this.subPart = partStore.add(this.createToolInvocationSubPart());
7579
this.domNode.appendChild(this.subPart.domNode);
7680
partStore.add(this.subPart.onDidChangeHeight(() => this._onDidChangeHeight.fire()));

‎src/vs/workbench/contrib/chat/browser/chatListRenderer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
759759
if (
760760
!lastPart ||
761761
lastPart.kind === 'references' ||
762-
(lastPart.kind === 'toolInvocation' && (lastPart.isComplete || lastPart.presentation === 'hidden')) ||
762+
((lastPart.kind === 'toolInvocation' || lastPart.kind === 'toolInvocationSerialized') && (lastPart.isComplete || lastPart.presentation === 'hidden')) ||
763763
((lastPart.kind === 'textEditGroup' || lastPart.kind === 'notebookEditGroup') && lastPart.done && !partsToRender.some(part => part.kind === 'toolInvocation' && !part.isComplete)) ||
764764
(lastPart.kind === 'progressTask' && lastPart.deferred.isSettled) ||
765765
lastPart.kind === 'prepareToolInvocation'

‎src/vs/workbench/contrib/chat/browser/contrib/chatInputCompletions.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,7 @@ class BuiltinDynamicCompletions extends Disposable {
758758
// If locked to an agent that doesn't support file attachments, skip
759759
if (widget.lockedAgentId) {
760760
const agent = this.chatAgentService.getAgent(widget.lockedAgentId);
761-
if (agent && agent.capabilities && agent.capabilities.supportsFileAttachments === false) {
761+
if (agent && !agent.capabilities?.supportsFileAttachments) {
762762
return result;
763763
}
764764
}
@@ -1125,7 +1125,7 @@ class ToolCompletions extends Disposable {
11251125
// If locked to an agent that doesn't support tool attachments, skip
11261126
if (widget.lockedAgentId) {
11271127
const agent = this.chatAgentService.getAgent(widget.lockedAgentId);
1128-
if (agent && agent.capabilities?.supportsToolAttachments === false) {
1128+
if (agent && !agent.capabilities?.supportsToolAttachments) {
11291129
return null;
11301130
}
11311131
}

‎src/vs/workbench/contrib/chat/common/chatServiceImpl.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -967,7 +967,17 @@ export class ChatService extends Disposable implements IChatService {
967967
} satisfies IChatAgentRequest;
968968
};
969969

970-
if (this.configurationService.getValue('chat.detectParticipant.enabled') !== false && this.chatAgentService.hasChatParticipantDetectionProviders() && !agentPart && !commandPart && !agentSlashCommandPart && enableCommandDetection && options?.modeInfo?.kind !== ChatModeKind.Agent && options?.modeInfo?.kind !== ChatModeKind.Edit) {
970+
if (
971+
this.configurationService.getValue('chat.detectParticipant.enabled') !== false &&
972+
this.chatAgentService.hasChatParticipantDetectionProviders() &&
973+
!agentPart &&
974+
!commandPart &&
975+
!agentSlashCommandPart &&
976+
enableCommandDetection &&
977+
options?.modeInfo?.kind !== ChatModeKind.Agent &&
978+
options?.modeInfo?.kind !== ChatModeKind.Edit &&
979+
!options?.agentIdSilent
980+
) {
971981
// We have no agent or command to scope history with, pass the full history to the participant detection provider
972982
const defaultAgentHistory = this.getHistoryEntriesFromModel(requests, model.sessionId, location, defaultAgent.id);
973983

‎src/vs/workbench/contrib/chat/common/languageModelToolsService.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,18 +75,25 @@ export type ToolDataSource =
7575
| {
7676
type: 'internal';
7777
label: string;
78+
} | {
79+
type: 'external';
80+
label: string;
7881
};
7982

8083
export namespace ToolDataSource {
8184

8285
export const Internal: ToolDataSource = { type: 'internal', label: 'Built-In' };
8386

87+
/** External tools may not be contributed or invoked, but may be invoked externally and described in an IChatToolInvocationSerialized */
88+
export const External: ToolDataSource = { type: 'external', label: 'External' };
89+
8490
export function toKey(source: ToolDataSource): string {
8591
switch (source.type) {
8692
case 'extension': return `extension:${source.extensionId.value}`;
8793
case 'mcp': return `mcp:${source.collectionId}:${source.definitionId}`;
8894
case 'user': return `user:${source.file.toString()}`;
8995
case 'internal': return 'internal';
96+
case 'external': return 'external';
9097
}
9198
}
9299

@@ -222,12 +229,17 @@ export interface IToolConfirmationAction {
222229

223230
export type ToolConfirmationAction = IToolConfirmationAction | Separator;
224231

232+
export enum ToolInvocationPresentation {
233+
Hidden = 'hidden',
234+
HiddenAfterComplete = 'hiddenAfterComplete'
235+
}
236+
225237
export interface IPreparedToolInvocation {
226238
invocationMessage?: string | IMarkdownString;
227239
pastTenseMessage?: string | IMarkdownString;
228240
originMessage?: string | IMarkdownString;
229241
confirmationMessages?: IToolConfirmationMessages;
230-
presentation?: 'hidden' | undefined;
242+
presentation?: ToolInvocationPresentation;
231243
toolSpecificData?: IChatTerminalToolInvocationData | IChatToolInputInvocationData | IChatExtensionsContent | IChatTodoListContent;
232244
}
233245

‎src/vs/workbench/contrib/chat/common/tools/confirmationTool.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { CancellationToken } from '../../../../../base/common/cancellation.js';
77
import { MarkdownString } from '../../../../../base/common/htmlContent.js';
88
import { IChatTerminalToolInvocationData } from '../chatService.js';
9-
import { CountTokensCallback, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, ToolDataSource, ToolProgress } from '../languageModelToolsService.js';
9+
import { CountTokensCallback, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, ToolDataSource, ToolInvocationPresentation, ToolProgress } from '../languageModelToolsService.js';
1010

1111
export const ConfirmationToolId = 'vscode_get_confirmation';
1212

@@ -41,7 +41,7 @@ export const ConfirmationToolData: IToolData = {
4141
}
4242
};
4343

44-
export interface ConfirmationToolParams {
44+
export interface IConfirmationToolParams {
4545
title: string;
4646
message: string;
4747
confirmationType?: 'basic' | 'terminal';
@@ -50,7 +50,7 @@ export interface ConfirmationToolParams {
5050

5151
export class ConfirmationTool implements IToolImpl {
5252
async prepareToolInvocation(context: IToolInvocationPreparationContext, token: CancellationToken): Promise<IPreparedToolInvocation | undefined> {
53-
const parameters = context.parameters as ConfirmationToolParams;
53+
const parameters = context.parameters as IConfirmationToolParams;
5454
if (!parameters.title || !parameters.message) {
5555
throw new Error('Missing required parameters for ConfirmationTool');
5656
}
@@ -80,7 +80,8 @@ export class ConfirmationTool implements IToolImpl {
8080
message: new MarkdownString(parameters.message),
8181
allowAutoConfirm: false
8282
},
83-
toolSpecificData
83+
toolSpecificData,
84+
presentation: ToolInvocationPresentation.HiddenAfterComplete
8485
};
8586
}
8687

@@ -89,7 +90,7 @@ export class ConfirmationTool implements IToolImpl {
8990
return {
9091
content: [{
9192
kind: 'text',
92-
value: `Confirmed`
93+
value: 'yes' // Consumers should check for this label to know whether the tool was confirmed or skipped
9394
}]
9495
};
9596
}

‎src/vs/workbench/contrib/chat/common/tools/editFileTool.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { INotebookService } from '../../../notebook/common/notebookService.js';
1313
import { ICodeMapperService } from '../../common/chatCodeMapperService.js';
1414
import { ChatModel } from '../../common/chatModel.js';
1515
import { IChatService } from '../../common/chatService.js';
16-
import { CountTokensCallback, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, ToolDataSource, ToolProgress } from '../../common/languageModelToolsService.js';
16+
import { CountTokensCallback, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, ToolDataSource, ToolInvocationPresentation, ToolProgress } from '../../common/languageModelToolsService.js';
1717

1818
export const ExtensionEditToolId = 'vscode_editFile';
1919
export const InternalEditToolId = 'vscode_editFile_internal';
@@ -138,7 +138,7 @@ export class EditTool implements IToolImpl {
138138

139139
async prepareToolInvocation(context: IToolInvocationPreparationContext, token: CancellationToken): Promise<IPreparedToolInvocation | undefined> {
140140
return {
141-
presentation: 'hidden'
141+
presentation: ToolInvocationPresentation.Hidden
142142
};
143143
}
144144
}

0 commit comments

Comments
 (0)