diff --git a/src/vs/workbench/contrib/browserView/common/browserView.ts b/src/vs/workbench/contrib/browserView/common/browserView.ts index 50412147a2a..e010f27d84c 100644 --- a/src/vs/workbench/contrib/browserView/common/browserView.ts +++ b/src/vs/workbench/contrib/browserView/common/browserView.ts @@ -234,7 +234,7 @@ export interface IBrowserViewModel extends IDisposable { stopFindInPage(keepSelection?: boolean): Promise; getSelectedText(): Promise; clearStorage(): Promise; - setSharedWithAgent(shared: boolean): Promise; + setSharedWithAgent(shared: boolean): Promise; trustCertificate(host: string, fingerprint: string): Promise; untrustCertificate(host: string, fingerprint: string): Promise; zoomIn(): Promise; @@ -576,7 +576,7 @@ export class BrowserViewModel extends Disposable implements IBrowserViewModel { private static readonly SHARE_DONT_ASK_KEY = 'browserView.shareWithAgent.dontAskAgain'; - async setSharedWithAgent(shared: boolean): Promise { + async setSharedWithAgent(shared: boolean): Promise { if (shared) { // Block sharing when the current page URL is denied by network policy. if (this._url) { @@ -587,7 +587,7 @@ export class BrowserViewModel extends Disposable implements IBrowserViewModel { localize('browserView.shareBlocked.title', "Cannot Share with Agent"), this.agentNetworkFilterService.formatError(uri), ); - return; + return false; } } catch { } } @@ -623,7 +623,7 @@ export class BrowserViewModel extends Disposable implements IBrowserViewModel { ); if (!result.confirmed) { - return; + return false; } } else { this.telemetryService.publicLog2( @@ -641,6 +641,8 @@ export class BrowserViewModel extends Disposable implements IBrowserViewModel { await this.playwrightService.stopTrackingPage(this.id); this._setSharedWithAgent(false); } + + return true; } private _setSharedWithAgent(isShared: boolean): void { diff --git a/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts b/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts index 641cd8bb0dc..a3f45467277 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts @@ -12,7 +12,7 @@ import { IAgentNetworkFilterService } from '../../../../../platform/networkFilte import { IEditorService } from '../../../../services/editor/common/editorService.js'; import { IToolResult } from '../../../chat/common/tools/languageModelToolsService.js'; import { BrowserEditorInput } from '../../common/browserEditorInput.js'; -import { IBrowserViewWorkbenchService } from '../../common/browserView.js'; +import { BrowserViewSharingState, IBrowserViewWorkbenchService } from '../../common/browserView.js'; // eslint-disable-next-line local/code-import-patterns import type { Page } from 'playwright-core'; @@ -51,7 +51,7 @@ export function formatBrowserEditorList(editorService: IEditorService, editors: const title = blocked ? localize('browser.blockedByPolicy', "Blocked by network domain policy") : (editor.title || 'Untitled'); const displayUrl = blocked ? '' : ` (${url})`; - const hint = editor === activeEditor ? ' (active)' : visibleEditors.has(editor) ? ' (visible)' : ''; + const hint = editor === activeEditor ? ' (active)' : visibleEditors.has(editor) ? ' (visible)' : ' (not visible)'; const id = options?.excludeIds ? '' : `[${editor.id}] `; // By default, use numbers only if we're excluding IDs, so models don't get confused about which ID to use. @@ -142,49 +142,40 @@ export function errorResult(message: string): IToolResult { } /** - * Checks whether a browser editor with the same host (hostname + port) already - * exists. When {@link playwrightService} is provided, only pages tracked by Playwright - * (i.e. shared with the agent) are considered. + * Checks whether a browser editor with the same host (hostname + port) already exists. * * @returns All matching {@link BrowserEditorInput}s. */ -async function findExistingPagesByHost( +export function findExistingPagesByHost( browserViewService: IBrowserViewWorkbenchService, - playwrightService: IPlaywrightService | undefined, url: string, -): Promise { + options?: { + includeBlank?: boolean; + sharingState?: BrowserViewSharingState; + } +): BrowserEditorInput[] { const parsed = URL.parse(url); if (!parsed || (parsed.protocol !== 'file:' && !parsed.host)) { return []; } - const trackedIds = playwrightService - ? new Set(await playwrightService.getTrackedPages()) - : undefined; - const results: BrowserEditorInput[] = []; for (const editor of browserViewService.getKnownBrowserViews().values()) { if (!(editor instanceof BrowserEditorInput)) { continue; } - if (trackedIds && !trackedIds.has(editor.id)) { + if (options?.sharingState && editor.model?.sharingState !== options.sharingState) { continue; } const editorUrl = URL.parse(editor.url || ''); if ( - !editor.url || + options?.includeBlank && (!editor.url || editor.url === 'about:blank') || editorUrl?.host === parsed.host || - (parsed.protocol === 'file:' && editorUrl?.protocol === 'file:') - ) { - results.push(editor); - } - // Check for subdomain matches - if ( - editorUrl?.host && parsed.host && - ( + (parsed.protocol === 'file:' && editorUrl?.protocol === 'file:') || + (editorUrl?.host && parsed.host && ( editorUrl.host.endsWith('.' + parsed.host) || parsed.host.endsWith('.' + editorUrl.host) - ) + )) ) { results.push(editor); } @@ -198,12 +189,9 @@ async function findExistingPagesByHost( */ export async function getExistingPagesResult( editorService: IEditorService, - browserViewService: IBrowserViewWorkbenchService, - playwrightService: IPlaywrightService | undefined, - url: string, + existing: BrowserEditorInput[], formatOptions?: FormatBrowserEditorLinesOptions ): Promise { - const existing = await findExistingPagesByHost(browserViewService, playwrightService, url); if (existing.length === 0) { return undefined; } diff --git a/src/vs/workbench/contrib/browserView/electron-browser/tools/browserTools.contribution.ts b/src/vs/workbench/contrib/browserView/electron-browser/tools/browserTools.contribution.ts index b1830ed66f1..3e53460e4b0 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/tools/browserTools.contribution.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/tools/browserTools.contribution.ts @@ -104,6 +104,8 @@ class BrowserChatAgentToolsContribution extends Disposable implements IWorkbench this._syncModelListeners(); this._updateBrowserContext(); })); + this._toolsStore.add(this.editorService.onDidActiveEditorChange(() => this._updateBrowserContext())); + this._toolsStore.add(this.editorService.onDidVisibleEditorsChange(() => this._updateBrowserContext())); this._toolsStore.add(this.agentNetworkFilterService.onDidChange(() => this._updateBrowserContext())); this._updateBrowserContext(); @@ -154,6 +156,7 @@ class BrowserChatAgentToolsContribution extends Disposable implements IWorkbench value += '\n\n'; } value += `${unsharedCount} ${unsharedCount === 1 ? 'page is' : 'pages are'} open but not shared.`; + value += `\nUse the 'open_browser_page' tool to open a new page or to help the user share an existing page.`; } this.chatContextService.updateWorkspaceContextItems(BrowserChatAgentToolsContribution.CONTEXT_ID, [{ diff --git a/src/vs/workbench/contrib/browserView/electron-browser/tools/openBrowserTool.ts b/src/vs/workbench/contrib/browserView/electron-browser/tools/openBrowserTool.ts index 89b5d2159b0..747dbe4a85d 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/tools/openBrowserTool.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/tools/openBrowserTool.ts @@ -3,17 +3,27 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { raceCancellation } from '../../../../../base/common/async.js'; import type { CancellationToken } from '../../../../../base/common/cancellation.js'; import { Codicon } from '../../../../../base/common/codicons.js'; +import { CancellationError } from '../../../../../base/common/errors.js'; import { MarkdownString } from '../../../../../base/common/htmlContent.js'; +import { hasKey } from '../../../../../base/common/types.js'; import { URI } from '../../../../../base/common/uri.js'; import { localize } from '../../../../../nls.js'; import { IPlaywrightService } from '../../../../../platform/browserView/common/playwrightService.js'; -import { IEditorService } from '../../../../services/editor/common/editorService.js'; -import { ToolDataSource, type CountTokensCallback, type IPreparedToolInvocation, type IToolData, type IToolImpl, type IToolInvocation, type IToolInvocationPreparationContext, type IToolResult, type ToolProgress } from '../../../chat/common/tools/languageModelToolsService.js'; +import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; +import { ILogService } from '../../../../../platform/log/common/log.js'; import { IAgentNetworkFilterService } from '../../../../../platform/networkFilter/common/networkFilterService.js'; -import { createBrowserPageLink, getExistingPagesResult } from './browserToolHelpers.js'; -import { IBrowserViewWorkbenchService } from '../../common/browserView.js'; +import { IEditorService } from '../../../../services/editor/common/editorService.js'; +import { IChatQuestion, IChatQuestionAnswers, IChatService, IChatSingleSelectAnswer } from '../../../chat/common/chatService/chatService.js'; +import { ChatConfiguration, ChatPermissionLevel } from '../../../chat/common/constants.js'; +import { ChatQuestionCarouselData } from '../../../chat/common/model/chatProgressTypes/chatQuestionCarouselData.js'; +import { IChatRequestModel } from '../../../chat/common/model/chatModel.js'; +import { ToolDataSource, type CountTokensCallback, type IPreparedToolInvocation, type IToolData, type IToolImpl, type IToolInvocation, type IToolInvocationPreparationContext, type IToolResult, type ToolProgress } from '../../../chat/common/tools/languageModelToolsService.js'; +import { BrowserViewSharingState, IBrowserViewWorkbenchService } from '../../common/browserView.js'; +import { BrowserEditorInput } from '../../common/browserEditorInput.js'; +import { createBrowserPageLink, findExistingPagesByHost, getExistingPagesResult } from './browserToolHelpers.js'; export const OpenPageToolId = 'open_browser_page'; @@ -22,7 +32,11 @@ export const OpenBrowserToolData: IToolData = { toolReferenceName: 'openBrowserPage', displayName: localize('openBrowserTool.displayName', 'Open Browser Page'), userDescription: localize('openBrowserTool.userDescription', 'Open a URL in the integrated browser'), - modelDescription: 'Open a new browser page in the integrated browser at the given URL. Returns a page ID that must be used with other browser tools to interact with the page. Prefer to reuse existing pages whenever possible and only call this tool if a new page is necessary.', + modelDescription: `Open a new browser page in the integrated browser at the given URL. +May prompt the user to share a page if there is a similar one already open, unless "forceNew" is true. +Returns a page ID that must be used with other browser tools to interact with the page, as well as an accessibility snapshot of the page. + +Important: Prefer to reuse existing pages whenever possible and only call this tool if you do not already have access to a tab you can reuse.`, icon: Codicon.openInProduct, source: ToolDataSource.Internal, inputSchema: { @@ -37,28 +51,36 @@ export const OpenBrowserToolData: IToolData = { description: 'Whether to force opening a new page even if a page with the same host already exists. Default is false.' } }, - required: ['url'], + $comment: 'If you omit "url", the user will be prompted to share an existing page instead. Use this if there are unshared pages that the user may be interested in sharing with you.' }, }; export interface IOpenBrowserToolParams { - url: string; + url?: string; forceNew?: boolean; } +const DECLINE_OPTION_ID = '__decline__'; + export class OpenBrowserTool implements IToolImpl { constructor( @IPlaywrightService private readonly playwrightService: IPlaywrightService, @IEditorService private readonly editorService: IEditorService, @IBrowserViewWorkbenchService private readonly browserViewService: IBrowserViewWorkbenchService, @IAgentNetworkFilterService private readonly agentNetworkFilterService: IAgentNetworkFilterService, + @IChatService private readonly chatService: IChatService, + @IConfigurationService private readonly configService: IConfigurationService, + @ILogService private readonly logService: ILogService, ) { } async prepareToolInvocation(context: IToolInvocationPreparationContext, _token: CancellationToken): Promise { const params = context.parameters as IOpenBrowserToolParams; if (!params.url) { - throw new Error('The "url" parameter is required.'); + return { + invocationMessage: localize('browser.open.prompt.invocation', "Prompting user to share a browser tab"), + pastTenseMessage: localize('browser.open.prompt.past', "Prompted user to share a browser tab"), + }; } const parsed = URL.parse(params.url); @@ -66,6 +88,8 @@ export class OpenBrowserTool implements IToolImpl { throw new Error('You must provide a complete, valid URL.'); } + params.url = parsed.href; // Ensure URL is in a normalized format + const uri = URI.parse(params.url); if (!this.agentNetworkFilterService.isUriAllowed(uri)) { throw new Error(this.agentNetworkFilterService.formatError(uri)); @@ -82,27 +106,212 @@ export class OpenBrowserTool implements IToolImpl { }; } - async invoke(invocation: IToolInvocation, _countTokens: CountTokensCallback, _progress: ToolProgress, _token: CancellationToken): Promise { + async invoke(invocation: IToolInvocation, _countTokens: CountTokensCallback, _progress: ToolProgress, token: CancellationToken): Promise { const params = invocation.parameters as IOpenBrowserToolParams; - if (!params.forceNew) { - const existingResult = await getExistingPagesResult(this.editorService, this.browserViewService, this.playwrightService, params.url, { agentNetworkFilterService: this.agentNetworkFilterService }); - if (existingResult) { - return existingResult; + // If no URL is specified, prompt the user for a page to share. + if (!params.url) { + const allPages = [...this.browserViewService.getKnownBrowserViews().values()]; + if (allPages.length === 0) { + return { content: [{ kind: 'text', value: 'No browser pages are currently open.' }] }; + } + + const shareResult = await this._promptForUnsharedPages(invocation, allPages, params, token); + if (shareResult) { + return shareResult; + } else { + return { content: [{ kind: 'text', value: 'The user opted not to share an existing page.' }] }; } } - const { pageId, summary } = await this.playwrightService.openPage(params.url); + if (!params.forceNew) { + // If there are already-shared pages, tell the model to reuse them + const shared = findExistingPagesByHost(this.browserViewService, params.url, { includeBlank: true, sharingState: BrowserViewSharingState.Shared }); + const alreadyShared = await getExistingPagesResult(this.editorService, shared, { agentNetworkFilterService: this.agentNetworkFilterService }); + if (alreadyShared) { + return alreadyShared; + } + // If there are unshared (but shareable) pages on the same host, prompt user to share one + const unshared = findExistingPagesByHost(this.browserViewService, params.url, { includeBlank: false, sharingState: BrowserViewSharingState.NotShared }); + if (unshared.length > 0) { + const shareResult = await this._promptForUnsharedPages(invocation, unshared, params, token); + if (shareResult) { + return shareResult; + } + } + } + + return this._openNewPage(params.url); + } + + /** + * Shows a carousel prompting the user to share one of the given unshared + * browser pages instead of opening a new page. Returns `undefined` if the + * prompt should be skipped or the user chose to open a new page. + */ + private async _promptForUnsharedPages(invocation: IToolInvocation, candidateEditors: BrowserEditorInput[], params: IOpenBrowserToolParams, token: CancellationToken): Promise { + + const chatSessionResource = invocation.context?.sessionResource; + const chatRequestId = invocation.chatRequestId; + const request = this._getRequest(chatSessionResource, chatRequestId); + + if (!request) { + return undefined; // No chat context — skip prompt, proceed to open + } + + // In autopilot/auto-reply, don't block — just open the new page + if (request.modeInfo?.permissionLevel === ChatPermissionLevel.Autopilot || this.configService.getValue(ChatConfiguration.AutoReply)) { + return undefined; + } + + const carousel = this._buildShareCarousel(candidateEditors, params.url, invocation.chatStreamToolCallId ?? invocation.callId); + this.chatService.appendProgress(request, carousel); + + const externalAnswerListener = this.chatService.onDidReceiveQuestionCarouselAnswer(event => { + if (event.resolveId !== carousel.resolveId || carousel.isUsed) { + return; + } + carousel.dismiss(event.answers); + }); + + let answerResult: { answers: IChatQuestionAnswers | undefined } | undefined; + try { + answerResult = await raceCancellation(carousel.completion.p, token); + } catch (error) { + if (error instanceof CancellationError) { + carousel.dismiss(undefined); + } + throw error; + } finally { + externalAnswerListener.dispose(); + } + + if (!answerResult || token.isCancellationRequested) { + carousel.dismiss(undefined); + throw new CancellationError(); + } + + // Extract the selected option + const selectedOptionId = this._extractSelectedOption(answerResult.answers); + + // User skipped/cancelled or chose "Open new page" — fall through to open + if (!selectedOptionId || selectedOptionId === DECLINE_OPTION_ID) { + return undefined; + } + + // User selected an existing tab + const editor = this.browserViewService.getKnownBrowserViews().get(selectedOptionId); + if (!editor) { + this.logService.warn(`[OpenBrowserTool] Selected option '${selectedOptionId}' not found.`); + return undefined; + } + + return this._shareExistingPage(editor); + } + + private _buildShareCarousel(editors: BrowserEditorInput[], url: string | undefined, resolveId: string): ChatQuestionCarouselData { + const options: IChatQuestion['options'] = []; + + for (const editor of editors) { + const editorTitle = (editor.title || editor.getName()).replaceAll(' - ', '\u00A0-\u00A0'); // nbsp around hyphens to prevent formatting in the carousel + const editorUrl = editor.url || 'about:blank'; + const truncatedUrl = editorUrl.length > 40 ? editorUrl.substring(0, 40) + '\u2026' : editorUrl; + options.push({ + id: editor.id, + label: localize( + { key: 'browser.open.shareExistingOption', comment: ['{Locked=" - "}', '{0} is the editor title', '{1} is the truncated URL'] }, + 'Yes, share "{0}" - {1}', + editorTitle, + truncatedUrl, + ), + value: editor.id, + }); + } + + // Default option: decline sharing + options.push({ + id: DECLINE_OPTION_ID, + label: url + ? localize('browser.open.newPageOption', "No, open a new page at {0}", url) + : localize({ key: 'browser.open.noPagesOption', comment: ['{Locked=" - "}'] }, "No - Do not share any tabs with the agent"), + value: DECLINE_OPTION_ID, + }); + + const question: IChatQuestion = { + id: `${resolveId}:0`, + type: 'singleSelect', + title: localize('browser.open.shareQuestion.title', "Share Browser Tab"), + message: localize('browser.open.shareQuestion.message', "Share an existing browser tab?"), + options, + defaultValue: DECLINE_OPTION_ID, + allowFreeformInput: false, + }; + + return new ChatQuestionCarouselData([question], true, resolveId); + } + + private _extractSelectedOption(answers: IChatQuestionAnswers | undefined): string | undefined { + if (!answers) { + return undefined; + } + + for (const answer of Object.values(answers)) { + if (typeof answer === 'string') { + return answer; + } + if (typeof answer === 'object' && answer !== null && hasKey(answer, { selectedValue: true })) { + return (answer as IChatSingleSelectAnswer).selectedValue; + } + } + + return undefined; + } + + private async _openNewPage(url: string): Promise { + const { pageId, summary } = await this.playwrightService.openPage(url); + return this._pageResult(pageId, summary, localize('browser.open.result', "Opened {0}", createBrowserPageLink(pageId))); + } + + private async _shareExistingPage(editor: BrowserEditorInput): Promise { + const model = await editor.resolve(); + if (model.sharingState !== BrowserViewSharingState.Shared) { + if (!(await model.setSharedWithAgent(true))) { + return { content: [{ kind: 'text', value: 'The user declined to share the page.' }] }; + } + } + + const summary = await this.playwrightService.getSummary(editor.id); + return this._pageResult(editor.id, summary, localize('browser.open.sharedResult', "User shared {0}", createBrowserPageLink(editor.id))); + } + + private _pageResult(pageId: string, summary: string, resultMessage: string): IToolResult { return { - content: [{ - kind: 'text', - value: `Page ID: ${pageId}\n\nSummary:\n`, - }, { - kind: 'text', - value: summary, - }], - toolResultMessage: new MarkdownString(localize('browser.open.result', "Opened {0}", createBrowserPageLink(pageId))) + content: [ + { kind: 'text', value: `Page ID: ${pageId}\n\nSummary:\n` }, + { kind: 'text', value: summary }, + ], + toolResultMessage: new MarkdownString(resultMessage), }; } + + private _getRequest(chatSessionResource: URI | undefined, chatRequestId: string | undefined): IChatRequestModel | undefined { + if (!chatSessionResource) { + return undefined; + } + + const model = this.chatService.getSession(chatSessionResource); + if (!model) { + return undefined; + } + + if (chatRequestId) { + const request = model.getRequests().find(r => r.id === chatRequestId); + if (request) { + return request; + } + } + + return model.getRequests().at(-1); + } } diff --git a/src/vs/workbench/contrib/browserView/electron-browser/tools/openBrowserToolNonAgentic.ts b/src/vs/workbench/contrib/browserView/electron-browser/tools/openBrowserToolNonAgentic.ts index c280c295548..bb33f871c55 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/tools/openBrowserToolNonAgentic.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/tools/openBrowserToolNonAgentic.ts @@ -13,12 +13,17 @@ import { IEditorService } from '../../../../services/editor/common/editorService import { type CountTokensCallback, type IPreparedToolInvocation, type IToolData, type IToolImpl, type IToolInvocation, type IToolInvocationPreparationContext, type IToolResult, type ToolProgress } from '../../../chat/common/tools/languageModelToolsService.js'; import { IOpenBrowserToolParams, OpenBrowserToolData } from './openBrowserTool.js'; import { MarkdownString } from '../../../../../base/common/htmlContent.js'; -import { createBrowserPageLink, getExistingPagesResult } from './browserToolHelpers.js'; +import { createBrowserPageLink, findExistingPagesByHost, getExistingPagesResult } from './browserToolHelpers.js'; import { IBrowserViewWorkbenchService } from '../../common/browserView.js'; export const OpenBrowserToolNonAgenticData: IToolData = { ...OpenBrowserToolData, modelDescription: 'Open a new browser page in the integrated browser at the given URL.', + inputSchema: { + ...OpenBrowserToolData.inputSchema, + required: ['url'], + $comment: undefined + } }; export class OpenBrowserToolNonAgentic implements IToolImpl { @@ -54,7 +59,8 @@ export class OpenBrowserToolNonAgentic implements IToolImpl { const params = invocation.parameters as IOpenBrowserToolParams; if (!params.forceNew) { - const existingResult = await getExistingPagesResult(this.editorService, this.browserViewService, undefined, params.url, { excludeIds: true }); + const existingPages = findExistingPagesByHost(this.browserViewService, params.url!); + const existingResult = await getExistingPagesResult(this.editorService, existingPages, { excludeIds: true }); if (existingResult) { return existingResult; } diff --git a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatQuestionCarouselData.ts b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatQuestionCarouselData.ts index 656784ee3bd..d41cb97237d 100644 --- a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatQuestionCarouselData.ts +++ b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatQuestionCarouselData.ts @@ -25,6 +25,22 @@ export class ChatQuestionCarouselData implements IChatQuestionCarousel { */ public dismissedByTerminalInput?: boolean; + /** + * Marks the carousel as dismissed with the given answers and clears draft + * state. Safe to call multiple times — subsequent calls are no-ops. + */ + dismiss(answers: IChatQuestionAnswers | undefined): void { + if (this.isUsed) { + return; + } + this.data = answers ?? {}; + this.isUsed = true; + this.draftAnswers = undefined; + this.draftCurrentIndex = undefined; + this.draftCollapsed = undefined; + void this.completion.complete({ answers }); + } + constructor( public questions: IChatQuestion[], public allowSkip: boolean, diff --git a/src/vs/workbench/contrib/chat/common/tools/builtinTools/askQuestionsTool.ts b/src/vs/workbench/contrib/chat/common/tools/builtinTools/askQuestionsTool.ts index b6efa0a3081..8838e19c65b 100644 --- a/src/vs/workbench/contrib/chat/common/tools/builtinTools/askQuestionsTool.ts +++ b/src/vs/workbench/contrib/chat/common/tools/builtinTools/askQuestionsTool.ts @@ -226,40 +226,22 @@ export class AskQuestionsTool extends Disposable implements IToolImpl { if (event.resolveId !== carousel.resolveId || carousel.isUsed) { return; } - - carousel.data = event.answers ?? {}; - carousel.isUsed = true; - carousel.draftAnswers = undefined; - carousel.draftCurrentIndex = undefined; - carousel.draftCollapsed = undefined; - void carousel.completion.complete({ answers: event.answers }); + carousel.dismiss(event.answers); }); let answerResult: { answers: IChatQuestionAnswers | undefined } | undefined; try { answerResult = await raceCancellation(carousel.completion.p, token); } catch (error) { - if (error instanceof CancellationError && !carousel.isUsed) { - carousel.data = {}; - carousel.isUsed = true; - carousel.draftAnswers = undefined; - carousel.draftCurrentIndex = undefined; - carousel.draftCollapsed = undefined; - await carousel.completion.complete({ answers: undefined }); + if (error instanceof CancellationError) { + carousel.dismiss(undefined); } throw error; } finally { externalAnswerListener.dispose(); } if (!answerResult) { - if (!carousel.isUsed) { - carousel.data = {}; - carousel.isUsed = true; - carousel.draftAnswers = undefined; - carousel.draftCurrentIndex = undefined; - carousel.draftCollapsed = undefined; - await carousel.completion.complete({ answers: undefined }); - } + carousel.dismiss(undefined); throw new CancellationError(); } if (token.isCancellationRequested) { diff --git a/src/vs/workbench/contrib/chat/test/common/model/chatQuestionCarouselData.test.ts b/src/vs/workbench/contrib/chat/test/common/model/chatQuestionCarouselData.test.ts index 5707ebbe4ab..aa2a42de9c4 100644 --- a/src/vs/workbench/contrib/chat/test/common/model/chatQuestionCarouselData.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/model/chatQuestionCarouselData.test.ts @@ -134,4 +134,47 @@ suite('ChatQuestionCarouselData', () => { }); }); }); + + suite('dismiss', () => { + test('sets data, marks used, clears drafts, and resolves completion', async () => { + const carousel = new ChatQuestionCarouselData(createQuestions(), true, 'resolve-1'); + carousel.draftAnswers = { q1: 'draft' }; + carousel.draftCurrentIndex = 1; + carousel.draftCollapsed = true; + + const answers = { q1: 'answer1' }; + carousel.dismiss(answers); + + assert.deepStrictEqual(carousel.data, answers); + assert.strictEqual(carousel.isUsed, true); + assert.strictEqual(carousel.draftAnswers, undefined); + assert.strictEqual(carousel.draftCurrentIndex, undefined); + assert.strictEqual(carousel.draftCollapsed, undefined); + + const result = await carousel.completion.p; + assert.deepStrictEqual(result, { answers }); + }); + + test('with undefined answers stores empty object as data', async () => { + const carousel = new ChatQuestionCarouselData(createQuestions(), true, 'resolve-1'); + carousel.dismiss(undefined); + + assert.deepStrictEqual(carousel.data, {}); + assert.strictEqual(carousel.isUsed, true); + + const result = await carousel.completion.p; + assert.strictEqual(result.answers, undefined); + }); + + test('is a no-op when already dismissed', async () => { + const carousel = new ChatQuestionCarouselData(createQuestions(), true, 'resolve-1'); + + carousel.dismiss({ q1: 'first' }); + carousel.dismiss({ q1: 'second' }); + + assert.deepStrictEqual(carousel.data, { q1: 'first' }, 'Second dismiss should not overwrite data'); + const result = await carousel.completion.p; + assert.deepStrictEqual(result.answers, { q1: 'first' }); + }); + }); });