Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces an experimental “tool confirmation carousel” UI that batches multiple parallel tool confirmations and displays them above the chat input, instead of rendering each confirmation inline in the response.
Changes:
- Added a new experimental setting (
chat.tools.confirmationCarousel.enabled) to gate the feature. - Implemented a
ChatToolConfirmationCarouselPartUI component (with CSS) and integrated it intoChatInputPart. - Updated
ChatListItemRendererto divert eligible tool invocations into the carousel when multiple confirmations are pending.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/common/constants.ts | Adds a new configuration key for enabling the confirmation carousel. |
| src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts | Hosts and switches per-session tool confirmation carousels above the input. |
| src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts | Routes multiple waiting tool confirmations into the carousel and suppresses inline rendering. |
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolInvocationPart.ts | Exposes toolCallId for identifying rendered tool parts. |
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.ts | New carousel UI component for batching confirmations, with bulk allow/skip actions. |
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatToolConfirmationCarousel.css | Styling for the new carousel UI. |
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatConfirmationWidget.css | Small layout/formatting tweaks to confirmation widget styles. |
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatSubagentContentPart.ts | Ensures subagent tool sections auto-expand when confirmation-required tools are appended. |
| src/vs/workbench/contrib/chat/browser/chat.contribution.ts | Registers the new experimental configuration setting. |
Comments suppressed due to low confidence (2)
src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts:2891
clearToolConfirmationCarousel()always clears/hideschatToolConfirmationCarouselContainer, even when deleting a carousel for a non-current session (e.g. from an asynconDidEmpty). Consider adding an optionalsessionKeyparameter and only mutating the DOM when the cleared key matches_currentSessionKey; otherwise just delete+dispose the stored carousel.
dom.hide(this.chatToolConfirmationCarouselContainer);
}
}
setWorkingSetCollapsed(collapsed: boolean): void {
this._workingSetCollapsed.set(collapsed, undefined);
}
src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts:2029
- The inline comment says this is "disabled on stable", but the logic is only gated by the setting value. Either remove/update the comment, or ensure the setting is actually not available on stable builds (e.g. by using
included: product.quality !== 'stable'in the configuration contribution).
} else {
this.finalizeCurrentThinkingPart(context, templateData);
}
}
// Check for subagent grouping before creating tool part - subagent part handles lazy creation
...chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.ts
Outdated
Show resolved
Hide resolved
...chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.ts
Outdated
Show resolved
Hide resolved
...chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.ts
Outdated
Show resolved
Hide resolved
...chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Adds an experimental “tool confirmation carousel” that batches multiple simultaneous tool confirmations into a queued, tabbed UI rendered above the chat input, reducing inline clutter when parallel tool calls require confirmation.
Changes:
- Introduces
chat.tools.confirmationCarousel.enabled(default on non-stable) and wires it into chat configuration. - Adds a new
ChatToolConfirmationCarouselPart(+ CSS) and integrates it withChatInputPartandChatListItemRendererto move eligible confirmations above the input when multiple are pending. - Extends
IChatWidgetwithinputPartto reliably target the main (non-inline-edit) input part.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/common/constants.ts | Adds configuration key for the carousel feature. |
| src/vs/workbench/contrib/chat/browser/chat.contribution.ts | Registers the new experimental setting and defaults. |
| src/vs/workbench/contrib/chat/browser/chat.ts | Adds inputPart to IChatWidget for main-input access. |
| src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts | Hosts per-session carousel instances and swaps them on session change. |
| src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts | Routes multiple pending confirmations into the carousel and suppresses redundant inline rendering/progress UI. |
| src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts | Clears the carousel when replacing an editing request. |
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolInvocationPart.ts | Exposes toolCallId for renderer bookkeeping. |
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.ts | New carousel UI component with bulk allow/skip and tab switching. |
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatToolConfirmationCarousel.css | New styling for the above-input carousel. |
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatConfirmationWidget.css | Layout tweaks for confirmation widget presentation in the new context. |
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatSubagentContentPart.ts | Auto-expands subagent section when a confirmation-requiring tool is appended. |
Copilot's findings
- Files reviewed: 11/11 changed files
- Comments generated: 4
| this._register(part.onDidEmpty(() => { | ||
| this._chatToolConfirmationCarousels.deleteAndDispose(capturedKey); | ||
| if (this._currentSessionKey === capturedKey) { | ||
| dom.clearNode(this.chatToolConfirmationCarouselContainer); | ||
| dom.hide(this.chatToolConfirmationCarouselContainer); | ||
| } | ||
| })); |
There was a problem hiding this comment.
The onDidEmpty listener is registered on the ChatInputPart (this._register(...)) for each carousel instance. When carousels are created/emptied repeatedly, these listener disposables accumulate until the entire input part is disposed. Consider scoping the listener lifecycle to the carousel instance (e.g., register it on the carousel/entry disposables) so it is disposed when the carousel is removed, not when the widget is torn down.
| this._register(part.onDidEmpty(() => { | |
| this._chatToolConfirmationCarousels.deleteAndDispose(capturedKey); | |
| if (this._currentSessionKey === capturedKey) { | |
| dom.clearNode(this.chatToolConfirmationCarouselContainer); | |
| dom.hide(this.chatToolConfirmationCarouselContainer); | |
| } | |
| })); | |
| part.onDidEmpty(() => { | |
| this._chatToolConfirmationCarousels.deleteAndDispose(capturedKey); | |
| if (this._currentSessionKey === capturedKey) { | |
| dom.clearNode(this.chatToolConfirmationCarouselContainer); | |
| dom.hide(this.chatToolConfirmationCarouselContainer); | |
| } | |
| }); |
| if (!isEditing && state.type === IChatToolInvocation.StateKind.WaitingForConfirmation && state.confirmationMessages?.title) { | ||
| const widget = this.chatWidgetService.getWidgetBySessionResource(context.element.sessionResource); | ||
| if (widget) { | ||
| const otherWaitingTools = context.element.response.value.filter((part): part is IChatToolInvocation => | ||
| part.kind === 'toolInvocation' && | ||
| part.toolCallId !== toolInvocation.toolCallId && | ||
| part.state.get().type === IChatToolInvocation.StateKind.WaitingForConfirmation |
There was a problem hiding this comment.
otherWaitingTools is filtered only by WaitingForConfirmation state. This can pull in tool invocations that are presentation === 'hidden' or that don’t have the confirmation title/messages required by the carousel, causing hidden/unsupported tools to be surfaced above the input. Filter otherWaitingTools with the same eligibility conditions used for the primary toolInvocation before adding them to the carousel.
| if (!isEditing && state.type === IChatToolInvocation.StateKind.WaitingForConfirmation && state.confirmationMessages?.title) { | |
| const widget = this.chatWidgetService.getWidgetBySessionResource(context.element.sessionResource); | |
| if (widget) { | |
| const otherWaitingTools = context.element.response.value.filter((part): part is IChatToolInvocation => | |
| part.kind === 'toolInvocation' && | |
| part.toolCallId !== toolInvocation.toolCallId && | |
| part.state.get().type === IChatToolInvocation.StateKind.WaitingForConfirmation | |
| const isEligibleForToolConfirmationCarousel = (tool: IChatToolInvocation): boolean => { | |
| const toolState = tool.state.get(); | |
| return toolState.type === IChatToolInvocation.StateKind.WaitingForConfirmation && !!toolState.confirmationMessages?.title; | |
| }; | |
| if (!isEditing && isEligibleForToolConfirmationCarousel(toolInvocation)) { | |
| const widget = this.chatWidgetService.getWidgetBySessionResource(context.element.sessionResource); | |
| if (widget) { | |
| const otherWaitingTools = context.element.response.value.filter((part): part is IChatToolInvocation => | |
| part.kind === 'toolInvocation' && | |
| part.toolCallId !== toolInvocation.toolCallId && | |
| part.presentation !== 'hidden' && | |
| isEligibleForToolConfirmationCarousel(part) |
| // For terminal tools, use the command as the label | ||
| if (tool.toolSpecificData?.kind === 'terminal') { | ||
| const terminalData = migrateLegacyTerminalToolSpecificData(tool.toolSpecificData); | ||
| const cmd = terminalData.presentationOverrides?.commandLine ?? terminalData.confirmation?.commandLine ?? (terminalData.commandLine.toolEdited ?? terminalData.commandLine.original).trimStart(); |
There was a problem hiding this comment.
For terminal tool invocations, the tab label currently ignores commandLine.forDisplay and commandLine.userEdited, even though other UI locations prefer those for display. This can show a stale/less-readable command in the carousel tabs. Consider using the same precedence used elsewhere (presentationOverrides/forDisplay/userEdited/toolEdited/original) when deriving the label.
| const cmd = terminalData.presentationOverrides?.commandLine ?? terminalData.confirmation?.commandLine ?? (terminalData.commandLine.toolEdited ?? terminalData.commandLine.original).trimStart(); | |
| const cmd = ( | |
| terminalData.presentationOverrides?.commandLine ?? | |
| terminalData.commandLine.forDisplay ?? | |
| terminalData.commandLine.userEdited ?? | |
| terminalData.commandLine.toolEdited ?? | |
| terminalData.commandLine.original ?? | |
| terminalData.confirmation?.commandLine | |
| ).trimStart(); |
|
|
||
| disposables.add(dom.addDisposableListener(tabElement, 'click', selectTab)); | ||
| disposables.add(dom.addDisposableListener(tabElement, 'keydown', (e: KeyboardEvent) => { | ||
| const event = new StandardKeyboardEvent(e); | ||
| if (event.keyCode === KeyCode.Enter || event.keyCode === KeyCode.Space) { | ||
| e.preventDefault(); | ||
| selectTab(); | ||
| } |
There was a problem hiding this comment.
Keyboard handling for the tabs only supports Enter/Space. For a role="tab" UI, arrow key navigation (Left/Right, Home/End) is generally expected for accessibility and parity with other tab UIs in the workbench. Consider adding this navigation so keyboard users can switch tabs efficiently.
| disposables.add(dom.addDisposableListener(tabElement, 'click', selectTab)); | |
| disposables.add(dom.addDisposableListener(tabElement, 'keydown', (e: KeyboardEvent) => { | |
| const event = new StandardKeyboardEvent(e); | |
| if (event.keyCode === KeyCode.Enter || event.keyCode === KeyCode.Space) { | |
| e.preventDefault(); | |
| selectTab(); | |
| } | |
| const navigateToTab = (targetIndex: number) => { | |
| if (targetIndex < 0 || targetIndex >= this.items.length) { | |
| return; | |
| } | |
| this.setActiveIndex(targetIndex); | |
| this.items[targetIndex].tabElement.focus(); | |
| }; | |
| disposables.add(dom.addDisposableListener(tabElement, 'click', selectTab)); | |
| disposables.add(dom.addDisposableListener(tabElement, 'keydown', (e: KeyboardEvent) => { | |
| const event = new StandardKeyboardEvent(e); | |
| if (event.keyCode === KeyCode.Enter || event.keyCode === KeyCode.Space) { | |
| e.preventDefault(); | |
| selectTab(); | |
| return; | |
| } | |
| const currentIndex = this.items.findIndex(i => i.toolCallId === tool.toolCallId); | |
| if (currentIndex < 0) { | |
| return; | |
| } | |
| switch (event.keyCode) { | |
| case KeyCode.LeftArrow: | |
| e.preventDefault(); | |
| navigateToTab((currentIndex + this.items.length - 1) % this.items.length); | |
| break; | |
| case KeyCode.RightArrow: | |
| e.preventDefault(); | |
| navigateToTab((currentIndex + 1) % this.items.length); | |
| break; | |
| case KeyCode.Home: | |
| e.preventDefault(); | |
| navigateToTab(0); | |
| break; | |
| case KeyCode.End: | |
| e.preventDefault(); | |
| navigateToTab(this.items.length - 1); | |
| break; | |
| } |
allows confirmations to be batched and queued if they come in at the same time