From ea4ccd5f449b87c6aee31ccca7fa207e09bc6a95 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Tue, 18 Nov 2025 11:05:28 -0800 Subject: [PATCH] chat: add requestNeedsInput on the chat model (#278171) With a side quest that makes elicitations correctly trigger this, which involved refactoring their state into an observable. Also swap requestInProgress/Obs to a single requestInProgress observable. Synchronous callers can just `.get()` it as needed. --- .../browser/actions/chatElicitationActions.ts | 3 +- .../chat/browser/chatAccessibilityProvider.ts | 2 +- .../chat/browser/chatAccessibilityService.ts | 4 +- .../chatElicitationContentPart.ts | 61 ++++++++++--------- .../chatEditingEditorContextKeys.ts | 2 +- .../chatEditing/chatEditingEditorOverlay.ts | 2 +- .../browser/chatElicitationRequestPart.ts | 27 +++++--- .../contrib/chat/browser/chatListRenderer.ts | 10 ++- .../browser/chatResponseAccessibleView.ts | 2 +- .../chatSessions/chatSessionTracker.ts | 2 +- .../chatSessions/localChatSessionsProvider.ts | 4 +- .../contrib/chat/browser/chatWidget.ts | 4 +- .../contrib/chat/common/chatModel.ts | 31 ++++++---- .../contrib/chat/common/chatService.ts | 22 ++++++- .../contrib/chat/common/chatServiceImpl.ts | 2 +- .../contrib/chat/common/chatViewModel.ts | 5 -- .../browser/inlineChatController.ts | 6 +- .../browser/inlineChatSessionServiceImpl.ts | 2 +- .../inlineChat/browser/inlineChatWidget.ts | 2 +- .../test/browser/inlineChatController.test.ts | 2 +- .../contrib/mcp/browser/mcpCommands.ts | 2 +- .../mcp/browser/mcpElicitationService.ts | 23 +++---- .../browser/tools/monitoring/outputMonitor.ts | 8 ++- 23 files changed, 136 insertions(+), 92 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatElicitationActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatElicitationActions.ts index 04dcede5bc8..932179d1d0a 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatElicitationActions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatElicitationActions.ts @@ -10,6 +10,7 @@ import { Action2, registerAction2 } from '../../../../../platform/actions/common import { ContextKeyExpr } from '../../../../../platform/contextkey/common/contextkey.js'; import { KeybindingWeight } from '../../../../../platform/keybinding/common/keybindingsRegistry.js'; import { ChatContextKeys } from '../../common/chatContextKeys.js'; +import { ElicitationState } from '../../common/chatService.js'; import { isResponseVM } from '../../common/chatViewModel.js'; import { IChatWidgetService } from '../chat.js'; import { CHAT_CATEGORY } from './chatActions.js'; @@ -50,7 +51,7 @@ class AcceptElicitationRequestAction extends Action2 { } for (const content of item.response.value) { - if (content.kind === 'elicitation' && content.state === 'pending') { + if (content.kind === 'elicitation2' && content.state.get() === ElicitationState.Pending) { await content.accept(true); widget.focusInput(); return; diff --git a/src/vs/workbench/contrib/chat/browser/chatAccessibilityProvider.ts b/src/vs/workbench/contrib/chat/browser/chatAccessibilityProvider.ts index da5e8e0cc3d..6a970d45271 100644 --- a/src/vs/workbench/contrib/chat/browser/chatAccessibilityProvider.ts +++ b/src/vs/workbench/contrib/chat/browser/chatAccessibilityProvider.ts @@ -142,7 +142,7 @@ export class ChatAccessibilityProvider implements IListAccessibilityProvider v.kind === 'elicitation'); + const elicitationCount = element.response.value.filter(v => v.kind === 'elicitation2' || v.kind === 'elicitationSerialized'); let elicitationHint = ''; for (const elicitation of elicitationCount) { const title = typeof elicitation.title === 'string' ? elicitation.title : elicitation.title.value; diff --git a/src/vs/workbench/contrib/chat/browser/chatAccessibilityService.ts b/src/vs/workbench/contrib/chat/browser/chatAccessibilityService.ts index 3919426e934..7a473fc7986 100644 --- a/src/vs/workbench/contrib/chat/browser/chatAccessibilityService.ts +++ b/src/vs/workbench/contrib/chat/browser/chatAccessibilityService.ts @@ -17,7 +17,7 @@ import { IInstantiationService } from '../../../../platform/instantiation/common import { FocusMode } from '../../../../platform/native/common/native.js'; import { IHostService } from '../../../services/host/browser/host.js'; import { AccessibilityVoiceSettingId } from '../../accessibility/browser/accessibilityConfiguration.js'; -import { IChatElicitationRequest } from '../common/chatService.js'; +import { ElicitationState, IChatElicitationRequest } from '../common/chatService.js'; import { IChatResponseViewModel } from '../common/chatViewModel.js'; import { ChatConfiguration } from '../common/constants.js'; import { IChatAccessibilityService, IChatWidgetService } from './chat.js'; @@ -73,7 +73,7 @@ export class ChatAccessibilityService extends Disposable implements IChatAccessi } } acceptElicitation(elicitation: IChatElicitationRequest): void { - if (elicitation.state !== 'pending') { + if (elicitation.state.get() !== ElicitationState.Pending) { return; } const title = typeof elicitation.title === 'string' ? elicitation.title : elicitation.title.value; diff --git a/src/vs/workbench/contrib/chat/browser/chatContentParts/chatElicitationContentPart.ts b/src/vs/workbench/contrib/chat/browser/chatContentParts/chatElicitationContentPart.ts index e2beffdabb4..8da7eb3c247 100644 --- a/src/vs/workbench/contrib/chat/browser/chatContentParts/chatElicitationContentPart.ts +++ b/src/vs/workbench/contrib/chat/browser/chatContentParts/chatElicitationContentPart.ts @@ -12,7 +12,7 @@ import { IContextKeyService } from '../../../../../platform/contextkey/common/co import { IKeybindingService } from '../../../../../platform/keybinding/common/keybinding.js'; import { IChatProgressRenderableResponseContent } from '../../common/chatModel.js'; import { ChatContextKeys } from '../../common/chatContextKeys.js'; -import { IChatElicitationRequest } from '../../common/chatService.js'; +import { ElicitationState, IChatElicitationRequest, IChatElicitationRequestSerialized } from '../../common/chatService.js'; import { IChatAccessibilityService } from '../chat.js'; import { AcceptElicitationRequestActionId } from '../actions/chatElicitationActions.js'; import { ChatConfirmationWidget, IChatConfirmationButton } from './chatConfirmationWidget.js'; @@ -36,7 +36,7 @@ export class ChatElicitationContentPart extends Disposable implements IChatConte } constructor( - elicitation: IChatElicitationRequest, + private readonly elicitation: IChatElicitationRequest | IChatElicitationRequestSerialized, context: IChatContentPartRenderContext, @IInstantiationService private readonly instantiationService: IInstantiationService, @IChatAccessibilityService private readonly chatAccessibilityService: IChatAccessibilityService, @@ -45,17 +45,12 @@ export class ChatElicitationContentPart extends Disposable implements IChatConte ) { super(); - const hasElicitationKey = ChatContextKeys.Editing.hasElicitationRequest.bindTo(this.contextKeyService); - if (elicitation.state === 'pending') { - hasElicitationKey.set(true); - } - this._register(toDisposable(() => hasElicitationKey.reset())); + const buttons: IChatConfirmationButton[] = []; + if (elicitation.kind === 'elicitation2') { + const acceptKeybinding = this.keybindingService.lookupKeybinding(AcceptElicitationRequestActionId); + const acceptTooltip = acceptKeybinding ? `${elicitation.acceptButtonLabel} (${acceptKeybinding.getLabel()})` : elicitation.acceptButtonLabel; - const acceptKeybinding = this.keybindingService.lookupKeybinding(AcceptElicitationRequestActionId); - const acceptTooltip = acceptKeybinding ? `${elicitation.acceptButtonLabel} (${acceptKeybinding.getLabel()})` : elicitation.acceptButtonLabel; - - const buttons: IChatConfirmationButton[] = [ - { + buttons.push({ label: elicitation.acceptButtonLabel, tooltip: acceptTooltip, data: true, @@ -64,11 +59,26 @@ export class ChatElicitationContentPart extends Disposable implements IChatConte data: action, run: action.run })) - }, - ]; - if (elicitation.rejectButtonLabel && elicitation.reject) { - buttons.push({ label: elicitation.rejectButtonLabel, data: false, isSecondary: true }); + }); + if (elicitation.rejectButtonLabel && elicitation.reject) { + buttons.push({ label: elicitation.rejectButtonLabel, data: false, isSecondary: true }); + } + + this._register(autorun(reader => { + if (elicitation.isHidden?.read(reader)) { + this.domNode.remove(); + } + })); + + const hasElicitationKey = ChatContextKeys.Editing.hasElicitationRequest.bindTo(this.contextKeyService); + this._register(autorun(reader => { + hasElicitationKey.set(elicitation.state.read(reader) === ElicitationState.Pending); + })); + this._register(toDisposable(() => hasElicitationKey.reset())); + + this.chatAccessibilityService.acceptElicitation(elicitation); } + const confirmationWidget = this._register(this.instantiationService.createInstance(ChatConfirmationWidget, context, { title: elicitation.title, subtitle: elicitation.subtitle, @@ -77,19 +87,15 @@ export class ChatElicitationContentPart extends Disposable implements IChatConte toolbarData: { partType: 'elicitation', partSource: elicitation.source?.type, arg: elicitation }, })); this._confirmWidget = confirmationWidget; - confirmationWidget.setShowButtons(elicitation.state === 'pending'); - - if (elicitation.isHidden) { - this._register(autorun(reader => { - if (elicitation.isHidden?.read(reader)) { - this.domNode.remove(); - } - })); - } + confirmationWidget.setShowButtons(elicitation.kind === 'elicitation2' && elicitation.state.get() === ElicitationState.Pending); this._register(confirmationWidget.onDidChangeHeight(() => this._onDidChangeHeight.fire())); this._register(confirmationWidget.onDidClick(async e => { + if (elicitation.kind !== 'elicitation2') { + return; + } + let result: boolean | IAction | undefined; if (typeof e.data === 'boolean' && e.data === true) { result = e.data; @@ -110,14 +116,13 @@ export class ChatElicitationContentPart extends Disposable implements IChatConte this._onDidChangeHeight.fire(); })); - this.chatAccessibilityService.acceptElicitation(elicitation); this.domNode = confirmationWidget.domNode; this.domNode.tabIndex = 0; const messageToRender = this.getMessageToRender(elicitation); this.domNode.ariaLabel = elicitation.title + ' ' + (typeof messageToRender === 'string' ? messageToRender : messageToRender.value || ''); } - private getMessageToRender(elicitation: IChatElicitationRequest): IMarkdownString | string { + private getMessageToRender(elicitation: IChatElicitationRequest | IChatElicitationRequestSerialized): IMarkdownString | string { if (!elicitation.acceptedResult) { return elicitation.message; } @@ -129,7 +134,7 @@ export class ChatElicitationContentPart extends Disposable implements IChatConte hasSameContent(other: IChatProgressRenderableResponseContent): boolean { // No other change allowed for this content type - return other.kind === 'elicitation'; + return other === this.elicitation; } addDisposable(disposable: IDisposable): void { diff --git a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingEditorContextKeys.ts b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingEditorContextKeys.ts index 7e39ffbe7e3..58b33e3443a 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingEditorContextKeys.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingEditorContextKeys.ts @@ -121,7 +121,7 @@ class ContextKeyGroup { this._ctxHasEditorModification.set(entry?.state.read(r) === ModifiedFileEntryState.Modified); this._ctxIsGlobalEditingSession.set(session.isGlobalEditingSession); this._ctxReviewModeEnabled.set(entry ? entry.reviewMode.read(r) : false); - this._ctxHasRequestInProgress.set(chatModel?.requestInProgressObs.read(r) ?? false); + this._ctxHasRequestInProgress.set(chatModel?.requestInProgress.read(r) ?? false); this._ctxIsCurrentlyBeingModified.set(!!entry?.isCurrentlyBeingModifiedBy.read(r)); // number of requests diff --git a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingEditorOverlay.ts b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingEditorOverlay.ts index 58f63cf2d10..a97a8f123f2 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingEditorOverlay.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingEditorOverlay.ts @@ -376,7 +376,7 @@ class ChatEditingOverlayController { } const chatModel = chatService.getSession(session.chatSessionResource)!; - return chatModel.requestInProgressObs.read(r); + return chatModel.requestInProgress.read(r); }); this._store.add(autorun(r => { diff --git a/src/vs/workbench/contrib/chat/browser/chatElicitationRequestPart.ts b/src/vs/workbench/contrib/chat/browser/chatElicitationRequestPart.ts index 5df3c17deb1..2302742a684 100644 --- a/src/vs/workbench/contrib/chat/browser/chatElicitationRequestPart.ts +++ b/src/vs/workbench/contrib/chat/browser/chatElicitationRequestPart.ts @@ -7,12 +7,12 @@ import { IAction } from '../../../../base/common/actions.js'; import { IMarkdownString } from '../../../../base/common/htmlContent.js'; import { Disposable } from '../../../../base/common/lifecycle.js'; import { IObservable, observableValue } from '../../../../base/common/observable.js'; -import { IChatElicitationRequest } from '../common/chatService.js'; +import { ElicitationState, IChatElicitationRequest, IChatElicitationRequestSerialized } from '../common/chatService.js'; import { ToolDataSource } from '../common/languageModelToolsService.js'; export class ChatElicitationRequestPart extends Disposable implements IChatElicitationRequest { - public readonly kind = 'elicitation'; - public state: 'pending' | 'accepted' | 'rejected' = 'pending'; + public readonly kind = 'elicitation2'; + public state = observableValue('state', ElicitationState.Pending); public acceptedResult?: Record; private readonly _isHiddenValue = observableValue('isHidden', false); @@ -25,8 +25,8 @@ export class ChatElicitationRequestPart extends Disposable implements IChatElici public readonly acceptButtonLabel: string, public readonly rejectButtonLabel: string | undefined, // True when the primary action is accepted, otherwise the action that was selected - public readonly accept: (value: IAction | true) => Promise, - public readonly reject?: () => Promise, + public readonly _accept: (value: IAction | true) => Promise, + public readonly _reject?: () => Promise, public readonly source?: ToolDataSource, public readonly moreActions?: IAction[], public readonly onHide?: () => void, @@ -34,6 +34,12 @@ export class ChatElicitationRequestPart extends Disposable implements IChatElici super(); } + accept(value: IAction | true): Promise { + return this._accept(value).then(state => { + this.state.set(state, undefined); + }); + } + hide(): void { if (this._isHiddenValue.get()) { return; @@ -44,12 +50,17 @@ export class ChatElicitationRequestPart extends Disposable implements IChatElici } public toJSON() { + const state = this.state.get(); + return { - kind: 'elicitation', + kind: 'elicitationSerialized', title: this.title, message: this.message, - state: this.state === 'pending' ? 'rejected' : this.state, + state: state === ElicitationState.Pending ? ElicitationState.Rejected : state, acceptedResult: this.acceptedResult, - } satisfies Partial; + subtitle: this.subtitle, + source: this.source, + isHidden: this._isHiddenValue.get(), + } satisfies IChatElicitationRequestSerialized; } } diff --git a/src/vs/workbench/contrib/chat/browser/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/chatListRenderer.ts index 0d1f6239d00..1a4847776dc 100644 --- a/src/vs/workbench/contrib/chat/browser/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/chatListRenderer.ts @@ -53,7 +53,7 @@ import { IChatAgentMetadata } from '../common/chatAgents.js'; import { ChatContextKeys } from '../common/chatContextKeys.js'; import { IChatTextEditGroup } from '../common/chatModel.js'; import { chatSubcommandLeader } from '../common/chatParserTypes.js'; -import { ChatAgentVoteDirection, ChatAgentVoteDownReason, ChatErrorLevel, IChatChangesSummary, IChatConfirmation, IChatContentReference, IChatElicitationRequest, IChatExtensionsContent, IChatFollowup, IChatMarkdownContent, IChatMcpServersStarting, IChatMultiDiffData, IChatPullRequestContent, IChatTask, IChatTaskSerialized, IChatThinkingPart, IChatToolInvocation, IChatToolInvocationSerialized, IChatTreeData, IChatUndoStop, isChatFollowup } from '../common/chatService.js'; +import { ChatAgentVoteDirection, ChatAgentVoteDownReason, ChatErrorLevel, IChatChangesSummary, IChatConfirmation, IChatContentReference, IChatElicitationRequest, IChatElicitationRequestSerialized, IChatExtensionsContent, IChatFollowup, IChatMarkdownContent, IChatMcpServersStarting, IChatMultiDiffData, IChatPullRequestContent, IChatTask, IChatTaskSerialized, IChatThinkingPart, IChatToolInvocation, IChatToolInvocationSerialized, IChatTreeData, IChatUndoStop, isChatFollowup } from '../common/chatService.js'; import { IChatRequestVariableEntry } from '../common/chatVariableEntries.js'; import { IChatChangesSummaryPart, IChatCodeCitations, IChatErrorDetailsPart, IChatReferences, IChatRendererContent, IChatRequestViewModel, IChatResponseViewModel, IChatViewModel, isRequestVM, isResponseVM } from '../common/chatViewModel.js'; import { getNWords } from '../common/chatWordCounter.js'; @@ -1346,7 +1346,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer elicitation.kind === other.kind); + } + const part = this.instantiationService.createInstance(ChatElicitationContentPart, elicitation, context); part.addDisposable(part.onDidChangeHeight(() => this.updateItemHeight(templateData))); return part; diff --git a/src/vs/workbench/contrib/chat/browser/chatResponseAccessibleView.ts b/src/vs/workbench/contrib/chat/browser/chatResponseAccessibleView.ts index 1275aab41f1..aed06778344 100644 --- a/src/vs/workbench/contrib/chat/browser/chatResponseAccessibleView.ts +++ b/src/vs/workbench/contrib/chat/browser/chatResponseAccessibleView.ts @@ -70,7 +70,7 @@ class ChatResponseAccessibleProvider extends Disposable implements IAccessibleVi responseContent = item.errorDetails.message; } if (isResponseVM(item)) { - item.response.value.filter(item => item.kind === 'elicitation').forEach(elicitation => { + item.response.value.filter(item => item.kind === 'elicitation2' || item.kind === 'elicitationSerialized').forEach(elicitation => { const title = elicitation.title; if (typeof title === 'string') { responseContent += `${title}\n`; diff --git a/src/vs/workbench/contrib/chat/browser/chatSessions/chatSessionTracker.ts b/src/vs/workbench/contrib/chat/browser/chatSessions/chatSessionTracker.ts index e5a0da5a830..9e07bb0b363 100644 --- a/src/vs/workbench/contrib/chat/browser/chatSessions/chatSessionTracker.ts +++ b/src/vs/workbench/contrib/chat/browser/chatSessions/chatSessionTracker.ts @@ -131,7 +131,7 @@ export class ChatSessionTracker extends Disposable { } private modelToStatus(model: IChatModel): ChatSessionStatus | undefined { - if (model.requestInProgress) { + if (model.requestInProgress.get()) { return ChatSessionStatus.InProgress; } const requests = model.getRequests(); diff --git a/src/vs/workbench/contrib/chat/browser/chatSessions/localChatSessionsProvider.ts b/src/vs/workbench/contrib/chat/browser/chatSessions/localChatSessionsProvider.ts index e20ad3b81c6..1e5af2b3202 100644 --- a/src/vs/workbench/contrib/chat/browser/chatSessions/localChatSessionsProvider.ts +++ b/src/vs/workbench/contrib/chat/browser/chatSessions/localChatSessionsProvider.ts @@ -92,7 +92,7 @@ export class LocalChatSessionsProvider extends Disposable implements IChatSessio const register = () => { this.registerModelTitleListener(widget); if (widget.viewModel) { - this.registerProgressListener(widget.viewModel.model.requestInProgressObs); + this.registerProgressListener(widget.viewModel.model.requestInProgress); } }; // Listen for view model changes on this widget @@ -156,7 +156,7 @@ export class LocalChatSessionsProvider extends Disposable implements IChatSessio } private modelToStatus(model: IChatModel): ChatSessionStatus | undefined { - if (model.requestInProgress) { + if (model.requestInProgress.get()) { return ChatSessionStatus.InProgress; } else { const requests = model.getRequests(); diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index 78f405eac3a..947444a93b3 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -2206,7 +2206,7 @@ export class ChatWidget extends Disposable implements IChatWidget { return; } - this.requestInProgress.set(this.viewModel.requestInProgress); + this.requestInProgress.set(this.viewModel.model.requestInProgress.get()); // Update the editor's placeholder text when it changes in the view model if (events?.some(e => e?.kind === 'changePlaceholder')) { @@ -2432,7 +2432,7 @@ export class ChatWidget extends Disposable implements IChatWidget { } private async _acceptInput(query: { query: string } | undefined, options?: IChatAcceptInputOptions): Promise { - if (this.viewModel?.requestInProgress) { + if (this.viewModel?.model.requestInProgress.get()) { return; } diff --git a/src/vs/workbench/contrib/chat/common/chatModel.ts b/src/vs/workbench/contrib/chat/common/chatModel.ts index b6f6ccf3849..d8e0ca5870b 100644 --- a/src/vs/workbench/contrib/chat/common/chatModel.ts +++ b/src/vs/workbench/contrib/chat/common/chatModel.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { asArray } from '../../../../base/common/arrays.js'; +import { softAssertNever } from '../../../../base/common/assert.js'; import { BugIndicatingError } from '../../../../base/common/errors.js'; import { Emitter, Event } from '../../../../base/common/event.js'; import { IMarkdownString, MarkdownString, isMarkdownString } from '../../../../base/common/htmlContent.js'; @@ -28,7 +29,7 @@ import { migrateLegacyTerminalToolSpecificData } from './chat.js'; import { IChatAgentCommand, IChatAgentData, IChatAgentResult, IChatAgentService, UserSelectedTools, reviveSerializedAgent } from './chatAgents.js'; import { IChatEditingService, IChatEditingSession } from './chatEditingService.js'; import { ChatRequestTextPart, IParsedChatRequest, reviveParsedChatRequest } from './chatParserTypes.js'; -import { ChatAgentVoteDirection, ChatAgentVoteDownReason, ChatResponseClearToPreviousToolInvocationReason, IChatAgentMarkdownContentWithVulnerability, IChatClearToPreviousToolInvocation, IChatCodeCitation, IChatCommandButton, IChatConfirmation, IChatContentInlineReference, IChatContentReference, IChatEditingSessionAction, IChatElicitationRequest, IChatExtensionsContent, IChatFollowup, IChatLocationData, IChatMarkdownContent, IChatMcpServersStarting, IChatMultiDiffData, IChatNotebookEdit, IChatPrepareToolInvocationPart, IChatProgress, IChatProgressMessage, IChatPullRequestContent, IChatResponseCodeblockUriPart, IChatResponseProgressFileTreeData, IChatSessionContext, IChatTask, IChatTaskSerialized, IChatTextEdit, IChatThinkingPart, IChatToolInvocation, IChatToolInvocationSerialized, IChatTreeData, IChatUndoStop, IChatUsedContext, IChatWarningMessage, isIUsedContext } from './chatService.js'; +import { ChatAgentVoteDirection, ChatAgentVoteDownReason, ChatResponseClearToPreviousToolInvocationReason, ElicitationState, IChatAgentMarkdownContentWithVulnerability, IChatClearToPreviousToolInvocation, IChatCodeCitation, IChatCommandButton, IChatConfirmation, IChatContentInlineReference, IChatContentReference, IChatEditingSessionAction, IChatElicitationRequest, IChatElicitationRequestSerialized, IChatExtensionsContent, IChatFollowup, IChatLocationData, IChatMarkdownContent, IChatMcpServersStarting, IChatMultiDiffData, IChatNotebookEdit, IChatPrepareToolInvocationPart, IChatProgress, IChatProgressMessage, IChatPullRequestContent, IChatResponseCodeblockUriPart, IChatResponseProgressFileTreeData, IChatSessionContext, IChatTask, IChatTaskSerialized, IChatTextEdit, IChatThinkingPart, IChatToolInvocation, IChatToolInvocationSerialized, IChatTreeData, IChatUndoStop, IChatUsedContext, IChatWarningMessage, isIUsedContext } from './chatService.js'; import { LocalChatSessionUri } from './chatUri.js'; import { ChatRequestToolReferenceEntry, IChatRequestVariableEntry } from './chatVariableEntries.js'; import { ChatAgentLocation, ChatModeKind } from './constants.js'; @@ -144,6 +145,7 @@ export type IChatProgressResponseContent = | IChatUndoStop | IChatPrepareToolInvocationPart | IChatElicitationRequest + | IChatElicitationRequestSerialized | IChatClearToPreviousToolInvocation | IChatMcpServersStarting; @@ -394,7 +396,8 @@ class AbstractResponse implements IResponse { case 'pullRequest': case 'undoStop': case 'prepareToolInvocation': - case 'elicitation': + case 'elicitation2': + case 'elicitationSerialized': case 'thinking': case 'multiDiffData': case 'mcpServersStarting': @@ -432,7 +435,8 @@ class AbstractResponse implements IResponse { segment = { text: part.content.value }; break; default: - // Ignore any unknown/obsolete parts + // Ignore any unknown/obsolete parts, but assert that all are handled: + softAssertNever(part); continue; } @@ -921,6 +925,7 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel return this._response.value.some(part => part.kind === 'toolInvocation' && part.state.read(r).type === IChatToolInvocation.StateKind.WaitingForConfirmation || part.kind === 'confirmation' && part.isUsed === false + || part.kind === 'elicitation2' && part.state.read(r) === ElicitationState.Pending ); }); @@ -1071,8 +1076,10 @@ export interface IChatModel extends IDisposable { readonly initialLocation: ChatAgentLocation; readonly title: string; readonly hasCustomTitle: boolean; - readonly requestInProgress: boolean; - readonly requestInProgressObs: IObservable; + /** True whenever a request is currently running */ + readonly requestInProgress: IObservable; + /** True whenever a request needs user interaction to continue */ + readonly requestNeedsInput: IObservable; readonly inputPlaceholder?: string; readonly editingSession?: IChatEditingSession | undefined; /** @@ -1375,12 +1382,8 @@ export class ChatModel extends Disposable implements IChatModel { return this._sessionResource; } - get requestInProgress(): boolean { - return this.requestInProgressObs.get(); - } - - readonly requestInProgressObs: IObservable; - + readonly requestInProgress: IObservable; + readonly requestNeedsInput: IObservable; get hasRequests(): boolean { return this._requests.length > 0; @@ -1481,9 +1484,13 @@ export class ChatModel extends Disposable implements IChatModel { const lastResponse = observableFromEvent(this, this.onDidChange, () => this._requests.at(-1)?.response); - this.requestInProgressObs = lastResponse.map((response, r) => { + this.requestInProgress = lastResponse.map((response, r) => { return response?.isInProgress.read(r) ?? false; }); + + this.requestNeedsInput = lastResponse.map((response, r) => { + return response?.isPendingConfirmation.read(r) ?? false; + }); } startEditingSession(isGlobalEditingSession?: boolean, transferFromSession?: IChatEditingSession): void { diff --git a/src/vs/workbench/contrib/chat/common/chatService.ts b/src/vs/workbench/contrib/chat/common/chatService.ts index a35330f25cb..422713a89e7 100644 --- a/src/vs/workbench/contrib/chat/common/chatService.ts +++ b/src/vs/workbench/contrib/chat/common/chatService.ts @@ -279,15 +279,21 @@ export interface IChatConfirmation { kind: 'confirmation'; } +export const enum ElicitationState { + Pending = 'pending', + Accepted = 'accepted', + Rejected = 'rejected', +} + export interface IChatElicitationRequest { - kind: 'elicitation'; + kind: 'elicitation2'; // '2' because initially serialized data used the same kind title: string | IMarkdownString; message: string | IMarkdownString; acceptButtonLabel: string; rejectButtonLabel: string | undefined; subtitle?: string | IMarkdownString; source?: ToolDataSource; - state: 'pending' | 'accepted' | 'rejected'; + state: IObservable; acceptedResult?: Record; moreActions?: IAction[]; accept(value: IAction | true): Promise; @@ -296,6 +302,17 @@ export interface IChatElicitationRequest { hide?(): void; } +export interface IChatElicitationRequestSerialized { + kind: 'elicitationSerialized'; + title: string | IMarkdownString; + message: string | IMarkdownString; + subtitle: string | IMarkdownString | undefined; + source: ToolDataSource | undefined; + state: ElicitationState.Accepted | ElicitationState.Rejected; + isHidden: boolean; + acceptedResult?: Record; +} + export interface IChatThinkingPart { kind: 'thinking'; value?: string | string[]; @@ -679,6 +696,7 @@ export type IChatProgress = | IChatThinkingPart | IChatTaskSerialized | IChatElicitationRequest + | IChatElicitationRequestSerialized | IChatMcpServersStarting; export interface IChatFollowup { diff --git a/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts b/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts index e1c6c99f84e..664752e3c70 100644 --- a/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts @@ -221,7 +221,7 @@ export class ChatService extends Disposable implements IChatService { this.requestInProgressObs = derived(reader => { const models = this._sessionModels.observable.read(reader).values(); - return Array.from(models).some(model => model.requestInProgressObs.read(reader)); + return Iterable.some(models, model => model.requestInProgress.read(reader)); }); } diff --git a/src/vs/workbench/contrib/chat/common/chatViewModel.ts b/src/vs/workbench/contrib/chat/common/chatViewModel.ts index cf985b3dc48..80ac737207f 100644 --- a/src/vs/workbench/contrib/chat/common/chatViewModel.ts +++ b/src/vs/workbench/contrib/chat/common/chatViewModel.ts @@ -67,7 +67,6 @@ export interface IChatViewModel { readonly sessionResource: URI; readonly onDidDisposeModel: Event; readonly onDidChange: Event; - readonly requestInProgress: boolean; readonly inputPlaceholder?: string; getItems(): (IChatRequestViewModel | IChatResponseViewModel)[]; setInputPlaceholder(text: string): void; @@ -267,10 +266,6 @@ export class ChatViewModel extends Disposable implements IChatViewModel { return this._model.sessionResource; } - get requestInProgress(): boolean { - return this._model.requestInProgress; - } - constructor( private readonly _model: IChatModel, public readonly codeBlockModelCollection: CodeBlockModelCollection, diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts index 8c3b369982d..51efc31b32f 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts @@ -565,7 +565,7 @@ export class InlineChatController1 implements IEditorContribution { } options.position = await this._strategy.renderChanges(); - if (this._session.chatModel.requestInProgress) { + if (this._session.chatModel.requestInProgress.get()) { return State.SHOW_REQUEST; } else { return State.WAIT_FOR_INPUT; @@ -647,7 +647,7 @@ export class InlineChatController1 implements IEditorContribution { private async [State.SHOW_REQUEST](options: InlineChatRunOptions): Promise { assertType(this._session); assertType(this._strategy); - assertType(this._session.chatModel.requestInProgress); + assertType(this._session.chatModel.requestInProgress.get()); this._ctxRequestInProgress.set(true); @@ -1429,7 +1429,7 @@ export class InlineChatController2 implements IEditorContribution { entry?.enableReviewModeUntilSettled(); } - const inProgress = session.chatModel.requestInProgressObs.read(r); + const inProgress = session.chatModel.requestInProgress.read(r); this._zone.value.widget.domNode.classList.toggle('request-in-progress', inProgress); if (!inProgress) { this._zone.value.widget.chatWidget.setInputPlaceholder(localize('placeholder', "Edit, refactor, and generate code")); diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatSessionServiceImpl.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatSessionServiceImpl.ts index d553be901bc..3d69c2043b7 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatSessionServiceImpl.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatSessionServiceImpl.ts @@ -392,7 +392,7 @@ export class InlineChatSessionServiceImpl implements IInlineChatSessionService { && !entry.isCurrentlyBeingModifiedBy.read(r); }); - if (allSettled && !chatModel.requestInProgress) { + if (allSettled && !chatModel.requestInProgress.read(undefined)) { // self terminate store.dispose(); } diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts index ea38c5cfcec..3abeca40167 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts @@ -209,7 +209,7 @@ export class InlineChatWidget { viewModelStore.add(viewModel.onDidChange(() => { - this._requestInProgress.set(viewModel.requestInProgress, undefined); + this._requestInProgress.set(viewModel.model.requestInProgress.get(), undefined); const last = viewModel.getItems().at(-1); toolbar2.context = last; diff --git a/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts b/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts index 3b9577dd613..85dc306faf5 100644 --- a/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts +++ b/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts @@ -919,7 +919,7 @@ suite('InlineChatController', function () { await (await chatService.sendRequest(newSession.chatModel.sessionResource, 'Existing', { location: ChatAgentLocation.EditorInline }))?.responseCreatedPromise; - assert.strictEqual(newSession.chatModel.requestInProgress, true); + assert.strictEqual(newSession.chatModel.requestInProgress.get(), true); const response = newSession.chatModel.lastRequest?.response; assertType(response); diff --git a/src/vs/workbench/contrib/mcp/browser/mcpCommands.ts b/src/vs/workbench/contrib/mcp/browser/mcpCommands.ts index 649f5851402..b9a8dd4bde1 100644 --- a/src/vs/workbench/contrib/mcp/browser/mcpCommands.ts +++ b/src/vs/workbench/contrib/mcp/browser/mcpCommands.ts @@ -195,7 +195,7 @@ export class McpConfirmationServerOptionsCommand extends Action2 { if (tool?.source.type === 'mcp') { accessor.get(ICommandService).executeCommand(McpCommandIds.ServerOptions, tool.source.definitionId); } - } else if (arg.kind === 'elicitation') { + } else if (arg.kind === 'elicitation2') { if (arg.source?.type === 'mcp') { accessor.get(ICommandService).executeCommand(McpCommandIds.ServerOptions, arg.source.definitionId); } diff --git a/src/vs/workbench/contrib/mcp/browser/mcpElicitationService.ts b/src/vs/workbench/contrib/mcp/browser/mcpElicitationService.ts index 2f8774232e7..b11f9b251b2 100644 --- a/src/vs/workbench/contrib/mcp/browser/mcpElicitationService.ts +++ b/src/vs/workbench/contrib/mcp/browser/mcpElicitationService.ts @@ -18,7 +18,7 @@ import { IOpenerService } from '../../../../platform/opener/common/opener.js'; import { IQuickInputService, IQuickPick, IQuickPickItem } from '../../../../platform/quickinput/common/quickInput.js'; import { ChatElicitationRequestPart } from '../../chat/browser/chatElicitationRequestPart.js'; import { ChatModel } from '../../chat/common/chatModel.js'; -import { IChatService } from '../../chat/common/chatService.js'; +import { ElicitationState, IChatService } from '../../chat/common/chatService.js'; import { LocalChatSessionUri } from '../../chat/common/chatUri.js'; import { ElicitationKind, ElicitResult, IFormModeElicitResult, IMcpElicitationService, IMcpServer, IMcpToolCallContext, IUrlModeElicitResult, McpConnectionState, MpcResponseError } from '../common/mcpTypes.js'; import { mcpServerToSourceData } from '../common/mcpTypesUtils.js'; @@ -26,8 +26,10 @@ 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 }; -function isFormElicitation(params: MCP.ElicitRequest['params']): params is MCP.ElicitRequestFormParams { - return params.mode === 'form'; +type Pre20251125ElicitationParams = Omit & { mode?: undefined }; + +function isFormElicitation(params: MCP.ElicitRequest['params'] | Pre20251125ElicitationParams): params is (MCP.ElicitRequestFormParams | Pre20251125ElicitationParams) { + return params.mode === 'form' || (params.mode === undefined && !!(params as Pre20251125ElicitationParams).requestedSchema); } function isUrlElicitation(params: MCP.ElicitRequest['params']): params is MCP.ElicitRequestURLParams { @@ -80,7 +82,7 @@ export class McpElicitationService implements IMcpElicitationService { } } - private async _elicitForm(server: IMcpServer, context: IMcpToolCallContext | undefined, elicitation: MCP.ElicitRequestFormParams, token: CancellationToken): Promise { + private async _elicitForm(server: IMcpServer, context: IMcpToolCallContext | undefined, elicitation: MCP.ElicitRequestFormParams | Pre20251125ElicitationParams, token: CancellationToken): Promise { const store = new DisposableStore(); const value = await new Promise(resolve => { const chatModel = context?.chatSessionId && this._chatService.getSession(LocalChatSessionUri.forSession(context.chatSessionId)); @@ -97,13 +99,12 @@ export class McpElicitationService implements IMcpElicitationService { const p = this._doElicitForm(elicitation, token); resolve(p); const result = await p; - part.state = result.action === 'accept' ? 'accepted' : 'rejected'; part.acceptedResult = result.content; + return result.action === 'accept' ? ElicitationState.Accepted : ElicitationState.Rejected; }, () => { resolve({ action: 'decline' }); - part.state = 'rejected'; - return Promise.resolve(); + return Promise.resolve(ElicitationState.Rejected); }, mcpServerToSourceData(server), ); @@ -166,12 +167,12 @@ export class McpElicitationService implements IMcpElicitationService { async () => { const result = await this._doElicitUrl(elicitation, token); resolve(result); - part.state = result.action === 'accept' ? 'accepted' : 'rejected'; + completePromise.then(() => part.hide()); + return result.action === 'accept' ? ElicitationState.Accepted : ElicitationState.Rejected; }, () => { resolve({ action: 'decline' }); - part.state = 'rejected'; - return Promise.resolve(); + return Promise.resolve(ElicitationState.Rejected); }, mcpServerToSourceData(server), ); @@ -216,7 +217,7 @@ export class McpElicitationService implements IMcpElicitationService { return { action: 'decline' }; } - private async _doElicitForm(elicitation: MCP.ElicitRequestFormParams, token: CancellationToken): Promise { + private async _doElicitForm(elicitation: MCP.ElicitRequestFormParams | Pre20251125ElicitationParams, token: CancellationToken): Promise { const quickPick = this._quickInputService.createQuickPick(); const store = new DisposableStore(); diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/monitoring/outputMonitor.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/monitoring/outputMonitor.ts index 4fc81ed3d9a..f26d26a1c7e 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/monitoring/outputMonitor.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/monitoring/outputMonitor.ts @@ -16,7 +16,7 @@ import { ExtensionIdentifier } from '../../../../../../../platform/extensions/co import { IChatWidgetService } from '../../../../../chat/browser/chat.js'; import { ChatElicitationRequestPart } from '../../../../../chat/browser/chatElicitationRequestPart.js'; import { ChatModel } from '../../../../../chat/common/chatModel.js'; -import { IChatService } from '../../../../../chat/common/chatService.js'; +import { ElicitationState, IChatService } from '../../../../../chat/common/chatService.js'; import { ChatAgentLocation } from '../../../../../chat/common/constants.js'; import { ChatMessageRole, ILanguageModelsService } from '../../../../../chat/common/languageModels.js'; import { IToolInvocationContext } from '../../../../../chat/common/languageModelToolsService.js'; @@ -650,7 +650,6 @@ export class OutputMonitor extends Disposable implements IOutputMonitor { acceptLabel, rejectLabel, async (value: IAction | true) => { - thePart.state = 'accepted'; thePart.hide(); this._promptPart = undefined; try { @@ -659,9 +658,10 @@ export class OutputMonitor extends Disposable implements IOutputMonitor { } catch { resolve(undefined); } + + return ElicitationState.Accepted; }, async () => { - thePart.state = 'rejected'; thePart.hide(); this._promptPart = undefined; try { @@ -670,6 +670,8 @@ export class OutputMonitor extends Disposable implements IOutputMonitor { } catch { resolve(undefined); } + + return ElicitationState.Rejected; }, undefined, // source moreActions,