diff --git a/build/lib/stylelint/vscode-known-variables.json b/build/lib/stylelint/vscode-known-variables.json index 79a40ba4087..efcdff888fc 100644 --- a/build/lib/stylelint/vscode-known-variables.json +++ b/build/lib/stylelint/vscode-known-variables.json @@ -983,6 +983,7 @@ "--activity-bar-icon-size", "--activity-bar-width", "--agent-sessions-editor-tab-padding", + "--session-chat-base-height", "--editor-font-size", "--background-dark", "--background-light", diff --git a/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css b/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css index 55fed2094c7..2aa85f7c31a 100644 --- a/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css +++ b/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css @@ -340,55 +340,6 @@ white-space: nowrap; } - .session-approval-row { - display: none; - gap: 8px; - margin-top: 4px; - margin-left: -6px; - padding: 4px 4px 4px 6px; - box-sizing: border-box; - border: 1px solid var(--vscode-contrastBorder, var(--vscode-widget-border, transparent)); - border-radius: var(--vscode-cornerRadius-large); - background-color: var(--vscode-editor-background); - color: var(--vscode-editor-foreground); - align-items: center; - - &.visible { - display: flex; - } - - .session-approval-label { - flex: 1; - overflow: hidden; - min-width: 0; - - & > .rendered-markdown, - & > .rendered-markdown > .code, - & > .rendered-markdown > .code > span { - display: block; - overflow: hidden; - } - - .monaco-tokenized-source { - display: block; - overflow: hidden; - white-space: nowrap; - text-overflow: ellipsis; - font-size: var(--vscode-fontSize-label1, 12px); - } - } - - .session-approval-button { - flex-shrink: 0; - - .monaco-button { - padding: 2px 10px; - font-size: var(--vscode-fontSize-label1, 12px); - white-space: nowrap; - } - } - } - /* Fix-CI row — a single line shown for blocked sessions whose PR is failing CI. Styled after the chat input's orange CI banner: an orange-tinted card with a summary on the left and a prominent orange "Fix CI" button. */ @@ -434,6 +385,58 @@ } } +/* Approval prompt shown when a chat is waiting for the user to allow a pending + tool action. Rendered on the session row (for the session's main chat) and on + each nested/side chat row (for that chat), so it is scoped to neither. */ +.session-approval-row { + display: none; + gap: 8px; + margin-top: 4px; + margin-left: -6px; + padding: 4px 4px 4px 6px; + box-sizing: border-box; + border: 1px solid var(--vscode-contrastBorder, var(--vscode-widget-border, transparent)); + border-radius: var(--vscode-cornerRadius-large); + background-color: var(--vscode-editor-background); + color: var(--vscode-editor-foreground); + align-items: center; + + &.visible { + display: flex; + } + + .session-approval-label { + flex: 1; + overflow: hidden; + min-width: 0; + + & > .rendered-markdown, + & > .rendered-markdown > .code, + & > .rendered-markdown > .code > span { + display: block; + overflow: hidden; + } + + .monaco-tokenized-source { + display: block; + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis; + font-size: var(--vscode-fontSize-label1, 12px); + } + } + + .session-approval-button { + flex-shrink: 0; + + .monaco-button { + padding: 2px 10px; + font-size: var(--vscode-fontSize-label1, 12px); + white-space: nowrap; + } + } +} + .monaco-list-row[aria-expanded="true"] .session-item::after { content: ''; position: absolute; @@ -444,17 +447,18 @@ } .session-chat-item { + /* Base (title-only) row height, matching SessionsTreeDelegate.CHAT_ITEM_HEIGHT. + The title row occupies this fixed height at the top; an optional approval + row is stacked beneath it and grows the overall row height. */ + --session-chat-base-height: 28px; display: flex; - align-items: center; - height: 100%; + flex-direction: column; box-sizing: border-box; position: relative; - gap: var(--vscode-spacing-size60); - padding: var(--vscode-spacing-size60) var(--vscode-spacing-size120) var(--vscode-spacing-size60) var(--vscode-spacing-size360); + padding: 0 var(--vscode-spacing-size120) 0 var(--vscode-spacing-size360); color: var(--vscode-foreground); font-size: var(--vscode-fontSize-body1); font-weight: var(--vscode-fontWeight-regular); - line-height: 17px; &::before { content: ''; @@ -468,7 +472,9 @@ &::after { content: ''; position: absolute; - top: 50%; + /* Align the horizontal connector with the title row's vertical center, + independent of any approval row stacked below it. */ + top: calc(var(--session-chat-base-height) / 2); left: var(--vscode-spacing-size200); width: var(--vscode-spacing-size240); border-top: var(--vscode-strokeThickness) solid var(--vscode-tree-inactiveIndentGuidesStroke); @@ -476,7 +482,9 @@ &.last-chat { &::before { - bottom: 50%; + /* Stop the vertical guide at the title row's center (its connector), + not at the middle of the taller approval-augmented row. */ + bottom: calc(100% - var(--session-chat-base-height) / 2); width: var(--vscode-spacing-size240); border-bottom: var(--vscode-strokeThickness) solid var(--vscode-tree-inactiveIndentGuidesStroke); border-bottom-left-radius: var(--vscode-cornerRadius-small); @@ -487,6 +495,15 @@ } } + .session-chat-title-row { + display: flex; + align-items: center; + flex: 0 0 auto; + height: var(--session-chat-base-height); + gap: var(--vscode-spacing-size60); + line-height: 17px; + } + .session-chat-icon { flex-shrink: 0; display: flex; @@ -509,6 +526,20 @@ text-overflow: ellipsis; white-space: nowrap; } + + /* Reuses the session row's approval-row styling; only the left inset is + reset since the chat row is already indented. When visible, a small bottom + margin adds slack that absorbs the rendered code-block's line-height + rounding (the chat row has no bottom padding of its own). As a flex-item + margin it reserves the same space without an ancestor `:has()` match. Kept + in sync with SessionsTreeDelegate.CHAT_APPROVAL_BOTTOM_SLACK. */ + .session-approval-row { + margin-left: 0; + + &.visible { + margin-bottom: 6px; + } + } } /* Show More */ @@ -894,7 +925,8 @@ } .session-chat-item { - padding: var(--vscode-spacing-size120) var(--vscode-spacing-size120) var(--vscode-spacing-size120) var(--vscode-spacing-size360); + --session-chat-base-height: 44px; + padding: 0 var(--vscode-spacing-size120) 0 var(--vscode-spacing-size360); font-size: var(--vscode-fontSize-body1); } diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts index d3d6a88fcb0..28862d0c57d 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts @@ -268,6 +268,14 @@ class SessionsTreeDelegate implements IListVirtualDelegate { private static readonly ITEM_HEIGHT_QUICK_CHAT = 28; private static readonly CHAT_ITEM_HEIGHT = 28; private static readonly CHAT_ITEM_HEIGHT_PHONE = 44; + /** + * Bottom slack reserved under a chat row's approval prompt. The session row + * absorbs the rendered code-block's line-height rounding in its own bottom + * padding; the chat row has none, so it reserves this small buffer instead. + * Keep in sync with the `.session-approval-row.visible` bottom margin in + * `sessionsList.css`. + */ + private static readonly CHAT_APPROVAL_BOTTOM_SLACK = 6; /** * Phone layout uses a taller row so the inline action toolbar can * meet the 44px minimum touch target without overflowing. Sized to @@ -286,11 +294,29 @@ class SessionsTreeDelegate implements IListVirtualDelegate { private readonly _approvalRowMaxLines: number = DEFAULT_APPROVAL_ROW_MAX_LINES, private readonly _ciFixModel: ISessionCIFixModel | undefined = undefined, private readonly _useCompactQuickChatRows = true, + /** + * Whether the session row surfaces an approval from any of its chats. Lists + * that render nested chats as their own rows (the main sessions tree) keep + * this `false` so the session row only shows its main chat's approval; + * flat, chat-less lists (blocked sessions, automations) set it `true`. + */ + private readonly _aggregateChatApprovals = false, ) { } getHeight(element: SessionListItem): number { if (isSessionChatItem(element)) { - return this._isPhone() ? SessionsTreeDelegate.CHAT_ITEM_HEIGHT_PHONE : SessionsTreeDelegate.CHAT_ITEM_HEIGHT; + let chatHeight = this._isPhone() ? SessionsTreeDelegate.CHAT_ITEM_HEIGHT_PHONE : SessionsTreeDelegate.CHAT_ITEM_HEIGHT; + if (this._approvalModel) { + const approval = this._approvalModel.getApproval(element.chat.resource).get(); + if (approval) { + // Reserve the approval row plus a small bottom slack (the chat row, + // unlike the session row, has no bottom padding to absorb the + // rendered code-block's line-height rounding). Kept in sync with the + // `.session-approval-row.visible` bottom margin in `sessionsList.css`. + chatHeight += SessionItemRenderer.getApprovalRowHeight(approval.label, this._approvalRowMaxLines) + SessionsTreeDelegate.CHAT_APPROVAL_BOTTOM_SLACK; + } + } + return chatHeight; } if (isSessionSection(element) || isSessionGroupItem(element)) { return SessionsTreeDelegate.SECTION_HEIGHT; @@ -311,7 +337,10 @@ class SessionsTreeDelegate implements IListVirtualDelegate { height = SessionsTreeDelegate.ITEM_HEIGHT; } if (this._approvalModel) { - const approval = getFirstApprovalAcrossChats(this._approvalModel, element as ISession, undefined); + // In the main tree only the main chat's approval renders on the session + // row (nested/side chats surface theirs on their own rows); flat lists + // with no chat rows aggregate an approval from any of the session's chats. + const approval = getSessionRowApproval(this._approvalModel, element as ISession, undefined, this._aggregateChatApprovals); if (approval) { height += SessionItemRenderer.getApprovalRowHeight(approval.label, this._approvalRowMaxLines); } @@ -323,6 +352,9 @@ class SessionsTreeDelegate implements IListVirtualDelegate { } hasDynamicHeight(element: SessionListItem): boolean { + if (isSessionChatItem(element)) { + return !!this._approvalModel; + } return (!!this._approvalModel || !!this._ciFixModel) && isSessionItem(element); } @@ -354,6 +386,9 @@ interface ISessionChatItemTemplate { readonly container: HTMLElement; readonly statusIcon: SessionStatusIcon; readonly title: HighlightedLabel; + readonly approvalRow: HTMLElement; + readonly approvalLabel: HTMLElement; + readonly approvalButtonContainer: HTMLElement; readonly disposables: DisposableStore; readonly elementDisposables: DisposableStore; } @@ -363,9 +398,19 @@ class SessionChatItemRenderer implements ITreeRenderer(); + readonly onDidChangeItemHeight: Event = this._onDidChangeItemHeight.event; + + private readonly _onDidApproveSession = new Emitter(); + /** Fires when the user approves a chat's pending action via its "Allow" button. */ + readonly onDidApproveSession: Event = this._onDidApproveSession.event; + constructor( private readonly hoverService: IHoverService, private readonly instantiationService: IInstantiationService, + private readonly markdownRendererService: IMarkdownRendererService | undefined, + private readonly approvalModel: AgentSessionApprovalModel | undefined, + private readonly approvalRowMaxLines: number, ) { } renderTemplate(container: HTMLElement): ISessionChatItemTemplate { @@ -373,12 +418,23 @@ class SessionChatItemRenderer implements ITreeRenderer e.stopPropagation())); + } + disposables.add(Gesture.ignoreTarget(approvalRow)); + + return { container, statusIcon, title, approvalRow, approvalLabel, approvalButtonContainer, disposables, elementDisposables }; } renderElement(node: ITreeNode, _index: number, template: ISessionChatItemTemplate): void { @@ -404,6 +460,51 @@ class SessionChatItemRenderer implements ITreeRenderer ({ content: getChatTitle(element.chat), }), { groupId: 'sessions-list' })); + + if (this.approvalModel) { + this.renderApprovalRow(element, template); + } + } + + private renderApprovalRow(element: ISessionChatItem, template: ISessionChatItemTemplate): void { + if (!this.approvalModel || !this.markdownRendererService) { + return; + } + + const approvalModel = this.approvalModel; + const markdownRendererService = this.markdownRendererService; + const chatResource = element.chat.resource; + let lastApprovalHeight = approvalRowHeightFor(approvalModel.getApproval(chatResource).get(), this.approvalRowMaxLines); + template.approvalRow.classList.toggle('visible', lastApprovalHeight > 0); + + const buttonStore = template.elementDisposables.add(new DisposableStore()); + + template.elementDisposables.add(autorun(reader => { + buttonStore.clear(); + + const info = approvalModel.getApproval(chatResource).read(reader); + const visible = !!info; + + template.approvalRow.classList.toggle('visible', visible); + + if (info) { + renderApprovalRowContent(info, { + label: template.approvalLabel, + buttonContainer: template.approvalButtonContainer, + }, buttonStore, markdownRendererService, this.hoverService, true, this.approvalRowMaxLines, approvalId => { + this._onDidApproveSession.fire({ session: element.session, approvalId }); + }); + } + + // Fire on any height change, not just visibility — the model can swap + // one pending approval for another whose label spans a different number + // of lines, which changes the reserved row height. + const height = approvalRowHeightFor(info, this.approvalRowMaxLines); + if (height !== lastApprovalHeight) { + lastApprovalHeight = height; + this._onDidChangeItemHeight.fire(element); + } + })); } disposeElement(_node: ITreeNode, _index: number, template: ISessionChatItemTemplate): void { @@ -417,6 +518,88 @@ class SessionChatItemRenderer implements ITreeRenderer void, +): void { + // Render up to `maxLines` lines as separate code blocks + const lines = info.label.split('\n'); + const visibleLines = lines.slice(0, maxLines); + if (lines.length > maxLines) { + visibleLines[maxLines - 1] = `${visibleLines[maxLines - 1]} \u2026`; + } + const langId = info.languageId ?? 'json'; + const labelContent = new MarkdownString(); + for (const line of visibleLines) { + labelContent.appendCodeblock(langId, line); + } + + elements.label.textContent = ''; + store.add(markdownRendererService.render(labelContent, {}, elements.label)); + + if (showHover) { + const fullContent = new MarkdownString().appendCodeblock(info.languageId ?? 'json', info.label); + store.add(hoverService.setupDelayedHover(elements.label, { + content: fullContent, + style: HoverStyle.Pointer, + position: { hoverPosition: HoverPosition.BELOW }, + })); + } + + elements.buttonContainer.textContent = ''; + const button = store.add(new Button(elements.buttonContainer, { + title: localize('allowActionOnce', "Allow once"), + // All simultaneously visible "Allow" buttons share the same visible label + // and tooltip, so give each an explicit accessible name that names the + // command/action it approves — otherwise screen-reader users can't tell + // which chat's action a button belongs to. + ariaLabel: localize('allowActionAria', "Allow: {0}", info.label), + secondary: true, + ...defaultButtonStyles + })); + button.label = localize('allowAction', "Allow"); + store.add(button.onDidClick(() => { + // Capture the approval's identity BEFORE confirming: `confirm()` may + // synchronously clear the pending approval, so we can't read it after. + const approvalId = agentSessionApprovalId(info); + info.confirm(); + onApprove(approvalId); + })); +} + +//#endregion + //#region Session Item Renderer /** @@ -530,7 +713,7 @@ class SessionItemRenderer implements ITreeRenderer = this._onDidApproveSession.event; constructor( - private readonly options: { grouping: () => SessionsGrouping; isPinned: (session: ISession) => boolean; isRenderedInCustomGroup?: (session: ISession) => boolean; visibleSessions: IObservable; getMultiSelectedSessions: (session: ISession) => ISession[]; showHover: boolean; useCompactQuickChatRows: boolean; approvalRowMaxLines: number; toolbarMenuId: MenuId | undefined; handleToolbarAction?: (action: IAction, session: ISession) => boolean | Promise; onDidRequestRename?: (session: ISession) => void }, + private readonly options: { grouping: () => SessionsGrouping; isPinned: (session: ISession) => boolean; isRenderedInCustomGroup?: (session: ISession) => boolean; visibleSessions: IObservable; getMultiSelectedSessions: (session: ISession) => ISession[]; showHover: boolean; useCompactQuickChatRows: boolean; approvalRowMaxLines: number; aggregateChatApprovals: boolean; toolbarMenuId: MenuId | undefined; handleToolbarAction?: (action: IAction, session: ISession) => boolean | Promise; onDidRequestRename?: (session: ISession) => void }, private readonly approvalModel: AgentSessionApprovalModel | undefined, private readonly ciFixModel: ISessionCIFixModel | undefined, private readonly instantiationService: IInstantiationService, @@ -906,64 +1089,36 @@ class SessionItemRenderer implements ITreeRenderer 0); const buttonStore = template.elementDisposables.add(new DisposableStore()); template.elementDisposables.add(autorun(reader => { buttonStore.clear(); - const info = getFirstApprovalAcrossChats(approvalModel, element, reader); + const info = getSessionRowApproval(approvalModel, element, reader, aggregate); const visible = !!info; template.approvalRow.classList.toggle('visible', visible); if (info) { - // Render up to `maxLines` lines as separate code blocks - const lines = info.label.split('\n'); - const maxLines = this.options.approvalRowMaxLines; - const visibleLines = lines.slice(0, maxLines); - if (lines.length > maxLines) { - visibleLines[maxLines - 1] = `${visibleLines[maxLines - 1]} \u2026`; - } - const langId = info.languageId ?? 'json'; - const labelContent = new MarkdownString(); - for (const line of visibleLines) { - labelContent.appendCodeblock(langId, line); - } - - template.approvalLabel.textContent = ''; - buttonStore.add(this.markdownRendererService.render(labelContent, {}, template.approvalLabel)); - - if (this.options.showHover) { - const fullContent = new MarkdownString().appendCodeblock(info.languageId ?? 'json', info.label); - buttonStore.add(this.hoverService.setupDelayedHover(template.approvalLabel, { - content: fullContent, - style: HoverStyle.Pointer, - position: { hoverPosition: HoverPosition.BELOW }, - })); - } - - template.approvalButtonContainer.textContent = ''; - const button = buttonStore.add(new Button(template.approvalButtonContainer, { - title: localize('allowActionOnce', "Allow once"), - secondary: true, - ...defaultButtonStyles - })); - button.label = localize('allowAction', "Allow"); - buttonStore.add(button.onDidClick(() => { - // Capture the approval's identity BEFORE confirming: `confirm()` may - // synchronously clear the pending approval, so we can't read it after. - const approvalId = agentSessionApprovalId(info); - info.confirm(); + renderApprovalRowContent(info, { + label: template.approvalLabel, + buttonContainer: template.approvalButtonContainer, + }, buttonStore, this.markdownRendererService, this.hoverService, this.options.showHover, this.options.approvalRowMaxLines, approvalId => { this._onDidApproveSession.fire({ session: element, approvalId }); - })); + }); } - if (wasVisible !== visible) { - wasVisible = visible; + // Fire on any height change, not just visibility — the model can swap + // one pending approval for another whose label spans a different number + // of lines, which changes the reserved row height. + const height = approvalRowHeightFor(info, this.options.approvalRowMaxLines); + if (height !== lastApprovalHeight) { + lastApprovalHeight = height; this._onDidChangeItemHeight.fire(element); } })); @@ -2018,6 +2173,13 @@ export interface ISessionsListControlOptions { */ canOpenSession?(session: ISession): Promise; onChatOpen?(session: ISession, chat: IChat, preserveFocus: boolean, sideBySide: boolean): void; + + /** + * Approval model tracking pending tool confirmations for the shown sessions + * and their chats. When omitted the list creates and owns its own; injectable + * so tests and fixtures can supply pending approvals without a live chat model. + */ + readonly approvalModel?: AgentSessionApprovalModel; } /** @@ -2174,7 +2336,7 @@ export class SessionsList extends Disposable implements ISessionsList { this.listContainer.classList.remove(SESSION_SECTION_FOCUS_FROM_POINTER_CLASS); }, true)); - const approvalModel = this._register(instantiationService.createInstance(AgentSessionApprovalModel)); + const approvalModel = this.options.approvalModel ?? this._register(instantiationService.createInstance(AgentSessionApprovalModel)); const markdownRendererService = instantiationService.invokeFunction(accessor => accessor.get(IMarkdownRendererService)); const hoverService = instantiationService.invokeFunction(accessor => accessor.get(IHoverService)); const sessionsProvidersService = instantiationService.invokeFunction(accessor => accessor.get(ISessionsProvidersService)); @@ -2215,6 +2377,7 @@ export class SessionsList extends Disposable implements ISessionsList { showHover: true, useCompactQuickChatRows: true, approvalRowMaxLines: DEFAULT_APPROVAL_ROW_MAX_LINES, + aggregateChatApprovals: false, toolbarMenuId: SessionItemToolbarMenuId, onDidRequestRename: session => { this.commandService.executeCommand(RENAME_SESSION_COMMAND_ID, session).catch(onUnexpectedError); @@ -2236,7 +2399,7 @@ export class SessionsList extends Disposable implements ISessionsList { const showMoreRenderer = new SessionShowMoreRenderer(); const placeholderRenderer = new SessionPlaceholderRenderer(hoverService); - const chatRenderer = new SessionChatItemRenderer(hoverService, instantiationService); + const chatRenderer = new SessionChatItemRenderer(hoverService, instantiationService, markdownRendererService, approvalModel, DEFAULT_APPROVAL_ROW_MAX_LINES); const selectHeader = (element: ISessionSection | ISessionGroupItem, event: MouseEvent) => { this.tree.setFocus([element], event); this.tree.setSelection([element], event); @@ -2441,6 +2604,12 @@ export class SessionsList extends Disposable implements ISessionsList { } })); + this._register(chatRenderer.onDidChangeItemHeight(chatItem => { + if (this.tree.hasElement(chatItem)) { + this.tree.updateElementHeight(chatItem, delegate.getHeight(chatItem)); + } + })); + // React to phone <-> desktop viewport transitions: refresh heights // for all known sessions so the virtual list reserves the correct // space for the new layout. Iterates `this.sessions` (all known @@ -3723,6 +3892,12 @@ export class SessionsList extends Disposable implements ISessionsList { //#region Approval Helpers +/** + * The oldest pending approval across every chat in the session, regardless of + * which chat it belongs to. Used where chats aren't rendered as separate rows + * (e.g. the flat blocked-sessions dropdown), so the session row is the only + * place an approval from any of its chats can surface. + */ export function getFirstApprovalAcrossChats(approvalModel: AgentSessionApprovalModel, session: ISession, reader: IReader | undefined,): IAgentSessionApprovalInfo | undefined { let oldest: IAgentSessionApprovalInfo | undefined; for (const chat of session.chats.read(reader)) { @@ -3734,6 +3909,29 @@ export function getFirstApprovalAcrossChats(approvalModel: AgentSessionApprovalM return oldest; } +/** + * The pending approval on a session's main chat only. Used by the main + * sessions tree, where nested/side chats are rendered as their own rows and + * surface their own approval there instead of being aggregated onto the + * parent session row. + */ +function getMainChatApproval(approvalModel: AgentSessionApprovalModel, session: ISession, reader: IReader | undefined): IAgentSessionApprovalInfo | undefined { + const mainChat = session.mainChat.read(reader); + if (!mainChat?.resource) { + return undefined; + } + return approvalModel.getApproval(mainChat.resource).read(reader); +} + +/** + * The approval to show on a session row. When `aggregate` is true (flat lists + * with no chat rows) it is the oldest approval across all chats; otherwise (the + * main tree, which renders chats as their own rows) it is only the main chat's. + */ +function getSessionRowApproval(approvalModel: AgentSessionApprovalModel, session: ISession, reader: IReader | undefined, aggregate: boolean): IAgentSessionApprovalInfo | undefined { + return aggregate ? getFirstApprovalAcrossChats(approvalModel, session, reader) : getMainChatApproval(approvalModel, session, reader); +} + //#endregion //#region Folder Matching @@ -4118,6 +4316,9 @@ export class SessionsFlatList extends Disposable { showHover: this.options.showSessionHover ?? true, useCompactQuickChatRows, approvalRowMaxLines: this.options.approvalRowMaxLines ?? DEFAULT_APPROVAL_ROW_MAX_LINES, + // This list renders no nested chat rows, so the session row is the + // only place an approval on any of its chats can surface. + aggregateChatApprovals: true, toolbarMenuId: this.options.toolbarMenuId ?? SessionItemToolbarMenuId, handleToolbarAction: this.options.onToolbarAction, }, @@ -4135,7 +4336,7 @@ export class SessionsFlatList extends Disposable { voicePlaybackService, ); - this._delegate = new SessionsTreeDelegate(approvalModel, () => false, this.options.approvalRowMaxLines ?? DEFAULT_APPROVAL_ROW_MAX_LINES, this.options.ciFixModel, useCompactQuickChatRows); + this._delegate = new SessionsTreeDelegate(approvalModel, () => false, this.options.approvalRowMaxLines ?? DEFAULT_APPROVAL_ROW_MAX_LINES, this.options.ciFixModel, useCompactQuickChatRows, true /* aggregateChatApprovals */); this.tree = this._register(instantiationService.createInstance( WorkbenchObjectTree, diff --git a/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts b/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts index 691fe5f823c..f8e8c56ed1a 100644 --- a/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts +++ b/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts @@ -7,7 +7,7 @@ import assert from 'assert'; import { mainWindow } from '../../../../../base/browser/window.js'; import { Codicon } from '../../../../../base/common/codicons.js'; import { ExtUri } from '../../../../../base/common/resources.js'; -import { constObservable, observableValue } from '../../../../../base/common/observable.js'; +import { constObservable, IObservable, observableValue } from '../../../../../base/common/observable.js'; import { URI } from '../../../../../base/common/uri.js'; import { mock, upcastPartial } from '../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; @@ -33,6 +33,7 @@ import { ChatInteractivity, ChatOriginKind, IChat, ISession, SessionStatus } fro import { IActiveSession, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; import { computeReorderSortChanges, groupByDate, groupByWorkspace, groupSessionsForList, ISessionSection, limitSessionsForList, SessionSectionRenderer, SessionsFlatList, SessionsList, sortSessions, SessionsGrouping, SessionsSorting } from '../../browser/views/sessionsList.js'; +import { AgentSessionApprovalKind, AgentSessionApprovalModel, IAgentSessionApprovalInfo } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentSessionApprovalModel.js'; import { getSessionSummaryHoverData } from '../../browser/sessionHoverContent.js'; import { createListHarness, createTestSession } from './sessionsListTestUtils.js'; import '../../browser/views/sessionsViewActions.js'; @@ -1288,6 +1289,163 @@ suite('Sessions - SessionsList', () => { visibleChats: [], }); }); + + function createApprovalModel(approvals: ReadonlyMap): AgentSessionApprovalModel { + return new class extends mock() { + override getApproval(resource: URI): IObservable { + return constObservable(approvals.get(resource.toString())); + } + }(); + } + + function terminalApproval(chat: IChat, command: string): IAgentSessionApprovalInfo { + return { approvalId: chat.resource.toString(), kind: AgentSessionApprovalKind.Terminal, label: command, languageId: 'shellscript', since: new Date(), confirm: () => { } }; + } + + function renderSessionChatsWithApprovals(session: ISession, approvalModel: AgentSessionApprovalModel): { container: HTMLElement; list: SessionsList } { + const harness = createListHarness(disposables, [session]); + const container = harness.createContainer(); + const list = harness.store.add(harness.instantiationService.createInstance(SessionsList, container, { + grouping: () => SessionsGrouping.Date, + sorting: () => SessionsSorting.Created, + onSessionOpen: () => { }, + approvalModel, + })); + list.layout(400, 400); + return { container, list }; + } + + function approvalRowFor(container: HTMLElement, title: string): HTMLElement | undefined { + return [...container.querySelectorAll('.session-chat-item')] + .find(item => item.querySelector('.session-chat-title')?.textContent === title) + ?.querySelector('.session-approval-row') ?? undefined; + } + + test('renders a pending approval on the owning chat row only, not on its siblings', () => { + const main = createChat('Main chat'); + const withApproval = createChat('Task A', ChatOriginKind.User); + const withoutApproval = createChat('Task B', ChatOriginKind.User); + const base = createTestSession('Session').session; + const session: ISession = { + ...base, + chats: constObservable([main, withApproval, withoutApproval]), + mainChat: constObservable(main), + capabilities: constObservable({ supportsMultipleChats: true }), + }; + const approvals = new Map([[withApproval.resource.toString(), terminalApproval(withApproval, 'npm run build')]]); + const { container } = renderSessionChatsWithApprovals(session, createApprovalModel(approvals)); + + const taskA = approvalRowFor(container, 'Task A'); + const taskB = approvalRowFor(container, 'Task B'); + assert.deepStrictEqual({ + taskAVisible: taskA?.classList.contains('visible'), + taskAHasAllow: taskA?.querySelector('.session-approval-button .monaco-button')?.textContent, + taskBVisible: taskB?.classList.contains('visible'), + sessionRowApprovalVisible: container.querySelector('.session-item .session-approval-row')?.classList.contains('visible'), + }, { + taskAVisible: true, + taskAHasAllow: 'Allow', + taskBVisible: false, + sessionRowApprovalVisible: false, + }); + }); + + test('renders the main chat approval on the session row, not on any chat row', () => { + const main = createChat('Main chat'); + const peer = createChat('Task A', ChatOriginKind.User); + const base = createTestSession('Session').session; + const session: ISession = { + ...base, + chats: constObservable([main, peer]), + mainChat: constObservable(main), + capabilities: constObservable({ supportsMultipleChats: true }), + }; + const approvals = new Map([[main.resource.toString(), terminalApproval(main, 'git push --force')]]); + const { container } = renderSessionChatsWithApprovals(session, createApprovalModel(approvals)); + + assert.deepStrictEqual({ + sessionRowApprovalVisible: container.querySelector('.session-item .session-approval-row')?.classList.contains('visible'), + chatRowApprovalVisible: approvalRowFor(container, 'Task A')?.classList.contains('visible'), + }, { + sessionRowApprovalVisible: true, + chatRowApprovalVisible: false, + }); + }); + + test('reserves extra row height for a chat with a pending approval', () => { + const main = createChat('Main chat'); + const withApproval = createChat('Task A', ChatOriginKind.User); + const withoutApproval = createChat('Task B', ChatOriginKind.User); + const base = createTestSession('Session').session; + const session: ISession = { + ...base, + chats: constObservable([main, withApproval, withoutApproval]), + mainChat: constObservable(main), + capabilities: constObservable({ supportsMultipleChats: true }), + }; + const approvals = new Map([[withApproval.resource.toString(), terminalApproval(withApproval, 'npm run build')]]); + const { container } = renderSessionChatsWithApprovals(session, createApprovalModel(approvals)); + + const rowHeight = (title: string) => [...container.querySelectorAll('.session-chat-item')] + .find(item => item.querySelector('.session-chat-title')?.textContent === title) + ?.closest('.monaco-list-row')?.style.height; + + const heights = { taskA: rowHeight('Task A'), taskB: rowHeight('Task B') }; + assert.ok(heights.taskA && heights.taskB && parseInt(heights.taskA) > parseInt(heights.taskB), `expected Task A (${heights.taskA}) taller than Task B (${heights.taskB})`); + }); + + test('confirms the chat approval when its Allow button is clicked', () => { + const main = createChat('Main chat'); + const peer = createChat('Task A', ChatOriginKind.User); + const base = createTestSession('Session').session; + const session: ISession = { + ...base, + chats: constObservable([main, peer]), + mainChat: constObservable(main), + capabilities: constObservable({ supportsMultipleChats: true }), + }; + let confirmed = 0; + const approval: IAgentSessionApprovalInfo = { ...terminalApproval(peer, 'npm run build'), confirm: () => { confirmed++; } }; + const { container } = renderSessionChatsWithApprovals(session, createApprovalModel(new Map([[peer.resource.toString(), approval]]))); + + const allow = approvalRowFor(container, 'Task A')?.querySelector('.session-approval-button .monaco-button'); + assert.ok(allow); + allow.dispatchEvent(new MouseEvent('click', { bubbles: true })); + + assert.strictEqual(confirmed, 1); + }); + + test('grows a chat row height when its approval is replaced with a taller one', () => { + const main = createChat('Main chat'); + const peer = createChat('Task A', ChatOriginKind.User); + const base = createTestSession('Session').session; + const session: ISession = { + ...base, + chats: constObservable([main, peer]), + mainChat: constObservable(main), + capabilities: constObservable({ supportsMultipleChats: true }), + }; + // A settable approval so we can swap one pending approval directly for + // another with a taller (multi-line) label — the row must re-reserve + // height on that change, not only when an approval appears/clears. + const pending = observableValue('pending', terminalApproval(peer, 'npm run build')); + const approvalModel = new class extends mock() { + override getApproval(resource: URI): IObservable { + return resource.toString() === peer.resource.toString() ? pending : constObservable(undefined); + } + }(); + const { container } = renderSessionChatsWithApprovals(session, approvalModel); + + const taskARowHeight = () => [...container.querySelectorAll('.session-chat-item')] + .find(item => item.querySelector('.session-chat-title')?.textContent === 'Task A') + ?.closest('.monaco-list-row')?.style.height; + + const singleLineHeight = taskARowHeight(); + pending.set(terminalApproval(peer, 'line one\nline two\nline three'), undefined); + const multiLineHeight = taskARowHeight(); + + assert.ok(singleLineHeight && multiLineHeight && parseInt(multiLineHeight) > parseInt(singleLineHeight), `expected taller row after multi-line approval (${singleLineHeight} -> ${multiLineHeight})`); + }); }); suite('SessionsFlatList quick-chat presentation', () => { @@ -1346,6 +1504,63 @@ suite('Sessions - SessionsList', () => { }, }); }); + + function createChat(id: string): IChat { + return upcastPartial({ + resource: URI.parse(`test-chat://${id}`), + title: constObservable(id), + updatedAt: constObservable(new Date()), + status: constObservable(SessionStatus.Completed), + interactivity: constObservable(ChatInteractivity.Full), + }); + } + + function flatApprovalModel(approvals: ReadonlyMap): AgentSessionApprovalModel { + return new class extends mock() { + override getApproval(resource: URI): IObservable { + return constObservable(approvals.get(resource.toString())); + } + }(); + } + + test('aggregates a non-main chat approval onto the flat session row and reserves height', () => { + // The blocked-sessions / automations flat list renders no nested chat + // rows, so an approval on any of a session's chats — including a + // non-main one — must surface on the session row itself. + const main = createChat('main'); + const worker = createChat('worker'); + const base = createTestSession('Session', { isQuickChat: false }).session; + const session: ISession = { + ...base, + chats: constObservable([main, worker]), + mainChat: constObservable(main), + capabilities: constObservable({ supportsMultipleChats: true }), + }; + const approval: IAgentSessionApprovalInfo = { approvalId: worker.resource.toString(), kind: AgentSessionApprovalKind.Terminal, label: 'npm run build', languageId: 'shellscript', since: new Date(), confirm: () => { } }; + const approvalModel = flatApprovalModel(new Map([[worker.resource.toString(), approval]])); + + const harness = createListHarness(disposables, [session]); + const container = harness.createContainer(); + const list = harness.store.add(harness.instantiationService.createInstance(SessionsFlatList, container, { + showSessionHover: false, + onSessionOpen: () => { }, + approvalModel, + })); + list.setSessions([session]); + const contentHeight = list.getContentHeight(); + list.layout(contentHeight, 400); + + const approvalRow = container.querySelector('.session-item .session-approval-row'); + assert.deepStrictEqual({ + approvalVisible: approvalRow?.classList.contains('visible'), + hasAllowButton: !!approvalRow?.querySelector('.session-approval-button .monaco-button'), + reservesHeight: contentHeight > list.getRowHeight(), + }, { + approvalVisible: true, + hasAllowButton: true, + reservesHeight: true, + }); + }); }); suite('computeReorderSortChanges', () => { diff --git a/src/vs/workbench/test/browser/componentFixtures/sessions/sessionsList.fixture.ts b/src/vs/workbench/test/browser/componentFixtures/sessions/sessionsList.fixture.ts index 1f340265b89..802cd8f694d 100644 --- a/src/vs/workbench/test/browser/componentFixtures/sessions/sessionsList.fixture.ts +++ b/src/vs/workbench/test/browser/componentFixtures/sessions/sessionsList.fixture.ts @@ -12,6 +12,10 @@ import { ThemeIcon, themeColorFromId } from '../../../../../base/common/themable import { URI } from '../../../../../base/common/uri.js'; import { mock } from '../../../../../base/test/common/mock.js'; import { IListService, ListService } from '../../../../../platform/list/browser/listService.js'; +import { IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js'; +import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; +import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { EditorMarkdownCodeBlockRenderer } from '../../../../../editor/browser/widget/markdownRenderer/browser/editorMarkdownCodeBlockRenderer.js'; import { IMarkdownRendererService, MarkdownRendererService } from '../../../../../platform/markdown/browser/markdownRenderer.js'; import { IAgentHostConnectionsService } from '../../../../../platform/agentHost/common/agentHostConnectionsService.js'; import { IUriIdentityService } from '../../../../../platform/uriIdentity/common/uriIdentity.js'; @@ -30,11 +34,14 @@ import { ISessionsService } from '../../../../../sessions/services/sessions/brow // eslint-disable-next-line local/code-import-patterns import { ICustomViewService } from '../../../../../sessions/services/customView/browser/customViewService.js'; // eslint-disable-next-line local/code-import-patterns -import { IChat, ISession, ISessionChangesSummary, ISessionFolder, ISessionWorkspace, SessionStatus } from '../../../../../sessions/services/sessions/common/session.js'; +import { IChat, ISession, ISessionChangesSummary, ISessionFolder, ISessionWorkspace, SessionStatus, ChatInteractivity } from '../../../../../sessions/services/sessions/common/session.js'; // eslint-disable-next-line local/code-import-patterns import { IActiveSession, ISessionsManagementService } from '../../../../../sessions/services/sessions/common/sessionsManagement.js'; // eslint-disable-next-line local/code-import-patterns import { SessionsGrouping, SessionsList, SessionsSorting } from '../../../../../sessions/contrib/sessions/browser/views/sessionsList.js'; +// eslint-disable-next-line local/code-import-patterns +import { IsPhoneLayoutContext } from '../../../../../sessions/common/contextkeys.js'; +import { AgentSessionApprovalKind, AgentSessionApprovalModel, IAgentSessionApprovalInfo } from '../../../../contrib/chat/browser/agentSessions/agentSessionApprovalModel.js'; import { IAgentSessionsService } from '../../../../contrib/chat/browser/agentSessions/agentSessionsService.js'; import { IAgentSession, IAgentSessionsModel } from '../../../../contrib/chat/browser/agentSessions/agentSessionsModel.js'; import { IAutomationService } from '../../../../contrib/chat/common/automations/automationService.js'; @@ -47,6 +54,14 @@ import { ComponentFixtureContext, createEditorServices, defineComponentFixture, // eslint-disable-next-line local/code-import-patterns import '../../../../../sessions/contrib/sessions/browser/media/sessionsList.css'; +interface IChatSpec { + readonly id: string; + readonly title: string; + readonly status?: SessionStatus; + /** Terminal command awaiting approval; renders an approval row with an Allow button on this chat's row. */ + readonly approvalCommand?: string; +} + interface ISessionSpec { readonly id: string; readonly title: string; @@ -56,6 +71,10 @@ interface ISessionSpec { readonly minutesAgo: number; readonly changesSummary?: ISessionChangesSummary; readonly group?: string; + /** Nested (non-main) chats shown as child rows under the session. */ + readonly chats?: readonly IChatSpec[]; + /** Terminal command awaiting approval on the session's main chat (renders on the session row). */ + readonly mainApprovalCommand?: string; } function createWorkspace(label: string): ISessionWorkspace { @@ -71,12 +90,47 @@ function createWorkspace(label: string): ISessionWorkspace { }; } -function createSession(spec: ISessionSpec): ISession { +function createChat(sessionId: string, spec: IChatSpec, updatedAt: Date, approvals: Map): IChat { + const resource = URI.parse(`vscode-session://session/${sessionId}/chat/${spec.id}`); + if (spec.approvalCommand !== undefined) { + approvals.set(resource.toString(), { + approvalId: resource.toString(), + kind: AgentSessionApprovalKind.Terminal, + label: spec.approvalCommand, + languageId: 'shellscript', + since: updatedAt, + confirm: () => { }, + }); + } + return new class extends mock() { + override readonly resource = resource; + override readonly title: IObservable = constObservable(spec.title); + override readonly updatedAt: IObservable = constObservable(updatedAt); + override readonly status: IObservable = constObservable(spec.status ?? SessionStatus.Completed); + override readonly interactivity: IObservable = constObservable(ChatInteractivity.Full); + }(); +} + +function createSession(spec: ISessionSpec, approvals: Map): ISession { const updatedAt = new Date(Date.now() - spec.minutesAgo * 60 * 1000); const description: IMarkdownString | undefined = spec.description ? new MarkdownString(spec.description) : undefined; + const mainChatResource = URI.parse(`vscode-session://session/${spec.id}/chat/main`); + if (spec.mainApprovalCommand !== undefined) { + approvals.set(mainChatResource.toString(), { + approvalId: mainChatResource.toString(), + kind: AgentSessionApprovalKind.Terminal, + label: spec.mainApprovalCommand, + languageId: 'shellscript', + since: updatedAt, + confirm: () => { }, + }); + } const mainChat = new class extends mock() { - override readonly resource = URI.parse(`vscode-session://session/${spec.id}/chat/main`); + override readonly resource = mainChatResource; + override readonly interactivity: IObservable = constObservable(ChatInteractivity.Full); }(); + const nestedChats = (spec.chats ?? []).map(chatSpec => createChat(spec.id, chatSpec, updatedAt, approvals)); + const chats: readonly IChat[] = [mainChat, ...nestedChats]; return new class extends mock() { override readonly sessionId = spec.id; override readonly resource = URI.parse(`vscode-session://session/${spec.id}`); @@ -94,9 +148,17 @@ function createSession(spec: ISessionSpec): ISession { override readonly changes: IObservable = constObservable([]); override readonly changesSummary: IObservable = constObservable(spec.changesSummary); override readonly description: IObservable = constObservable(description); - override readonly chats: IObservable = constObservable([]); + override readonly chats: IObservable = constObservable(chats); override readonly mainChat: IObservable = constObservable(mainChat); - override readonly capabilities = constObservable({ supportsMultipleChats: false }); + override readonly capabilities = constObservable({ supportsMultipleChats: nestedChats.length > 0 }); + }(); +} + +function createApprovalModel(approvals: Map): AgentSessionApprovalModel { + return new class extends mock() { + override getApproval(resource: URI): IObservable { + return constObservable(approvals.get(resource.toString())); + } }(); } @@ -110,7 +172,9 @@ interface IRenderOptions { function renderSessionsList(ctx: ComponentFixtureContext, options: IRenderOptions): void { const { container, disposableStore } = ctx; - const sessions = options.sessions.map(createSession); + const approvals = new Map(); + const sessions = options.sessions.map(spec => createSession(spec, approvals)); + const approvalModel = createApprovalModel(approvals); const groups = options.groups ?? []; const membership = new Map(); for (const spec of options.sessions) { @@ -203,6 +267,18 @@ function renderSessionsList(ctx: ComponentFixtureContext, options: IRenderOption }, }); + // Render terminal-approval labels as real (monospace) code blocks — otherwise + // the markdown renderer emits empty code-block spans and the command is blank. + (instantiationService.get(IConfigurationService) as TestConfigurationService).setUserConfiguration('editor', { fontFamily: 'monospace' }); + instantiationService.get(IMarkdownRendererService).setDefaultCodeBlockRenderer(instantiationService.createInstance(EditorMarkdownCodeBlockRenderer)); + + // Phone layout is driven by both a CSS class (visual) and a context key (row + // height reservation in the tree delegate). Set both so the reserved row + // height matches the rendered content. + if (options.phone) { + IsPhoneLayoutContext.bindTo(instantiationService.get(IContextKeyService)).set(true); + } + const width = options.width ?? 340; container.style.width = `${width}px`; container.style.height = options.phone ? '260px' : '220px'; @@ -217,6 +293,7 @@ function renderSessionsList(ctx: ComponentFixtureContext, options: IRenderOption grouping: () => options.grouping ?? SessionsGrouping.Workspace, sorting: () => SessionsSorting.Created, onSessionOpen: () => { }, + approvalModel, })); list.layout(options.phone ? 260 : 220, width); } @@ -260,4 +337,46 @@ export default defineThemedFixtureGroup({ path: 'sessions/' }, { SessionsList_CustomGroup_Phone: defineComponentFixture({ render: ctx => renderSessionsList(ctx, { sessions: GROUPED_SESSIONS, groups: [GROUP], phone: true, width: 340 }), }), + // A session whose nested chats each surface their own pending approval on + // their own row, plus an approval on the session's main chat (on the session + // row). Exercises the per-chat approval rendering and row-height reservation. + SessionsList_NestedChatApprovals: defineComponentFixture({ + render: ctx => renderSessionsList(ctx, { + sessions: [ + { + id: 'a', + title: 'HTTP Client Retry Plan', + workspace: 'vscode-tools', + minutesAgo: 2, + status: SessionStatus.NeedsInput, + mainApprovalCommand: 'yarn workspace @vscode-tools/server build --watch', + chats: [ + { id: 'task-a', title: 'Task A', status: SessionStatus.NeedsInput, approvalCommand: 'yarn workspace @vscode-tools/server build' }, + { id: 'task-b', title: 'Task B' }, + { id: 'task-c', title: 'Task C', status: SessionStatus.NeedsInput, approvalCommand: 'npm run test:integration -- --grep "retry"' }, + ], + }, + ], + width: 340, + }), + }), + SessionsList_NestedChatApprovals_Phone: defineComponentFixture({ + render: ctx => renderSessionsList(ctx, { + sessions: [ + { + id: 'a', + title: 'HTTP Client Retry Plan', + workspace: 'vscode-tools', + minutesAgo: 2, + status: SessionStatus.NeedsInput, + chats: [ + { id: 'task-a', title: 'Task A', status: SessionStatus.NeedsInput, approvalCommand: 'yarn workspace @vscode-tools/server build' }, + { id: 'task-b', title: 'Task B' }, + ], + }, + ], + phone: true, + width: 340, + }), + }), });