diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatAccessibilityHelp.ts b/src/vs/workbench/contrib/chat/browser/actions/chatAccessibilityHelp.ts index 8c4745ba4ff500..50902d48232677 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatAccessibilityHelp.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatAccessibilityHelp.ts @@ -82,6 +82,7 @@ export function getAccessibilityHelpText(type: 'panelChat' | 'inlineChat' | 'qui content.push(localize('chat.modelPicker.pricingDetails', "Pricing Details expands in place without moving the model's controls. Expansion and collapse are immediate when reduced motion is enabled. If the details exceed the available space, use Page Up or Page Down while the model details have focus to scroll.")); content.push(localize('chat.modelPicker.search', "Type while the model list is focused to search across all providers. In the search field, use Up and Down Arrow to navigate results, Enter to select a model, and Escape to close the picker. Left and Right Arrow move the text cursor.")); content.push(localize('chat.fileChangesDisclosure', 'File change summaries show the total files, additions, and deletions. Focus the disclosure and press Enter or Space to show or hide the individual files. Focus an additions and deletions label and press Enter or Space to open the changes in a diff editor.')); + content.push(localize('chat.pendingRequestEditing', "Queued messages can be edited before they are sent. Pending steering messages cannot be edited because they may already have been submitted to the agent.")); } if (type === 'panelChat' || type === 'quickChat' || type === 'agentView') { if (type === 'quickChat') { diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatQueueActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatQueueActions.ts index 1f9f9e3ad06973..87c8946471e5f3 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatQueueActions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatQueueActions.ts @@ -17,7 +17,7 @@ import { ChatContextKeys } from '../../common/actions/chatContextKeys.js'; import { ChatRequestQueueKind, IChatService } from '../../common/chatService/chatService.js'; import { IChatSideChatService } from '../../common/chatSideChatService.js'; import { ChatConfiguration } from '../../common/constants.js'; -import { isRequestVM } from '../../common/model/chatViewModel.js'; +import { isEditableRequestVM, isRequestVM } from '../../common/model/chatViewModel.js'; import { IChatWidgetService } from '../chat.js'; import { captureSideChatSelection } from '../chatSideChat.js'; import { CHAT_CATEGORY } from './chatActions.js'; @@ -278,7 +278,7 @@ export class ChatEditPendingRequestAction extends Action2 { group: 'navigation', order: 2, when: ContextKeyExpr.and( - ChatContextKeys.isRequest, + ChatContextKeys.isEditableRequest, ChatContextKeys.isPendingRequest, ContextKeyExpr.notEquals(`config.${ChatConfiguration.EditRequests}`, 'hover'), ContextKeyExpr.notEquals(`config.${ChatConfiguration.EditRequests}`, 'input') @@ -291,7 +291,7 @@ export class ChatEditPendingRequestAction extends Action2 { const widgetService = accessor.get(IChatWidgetService); const [context] = args; - if (!isRequestVM(context) || !context.pendingKind) { + if (!isEditableRequestVM(context) || !context.pendingKind) { return; } diff --git a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingActions.ts b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingActions.ts index 0f404817b5b860..2a4bf4c5ba87c3 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingActions.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingActions.ts @@ -31,7 +31,7 @@ import { isChatViewTitleActionContext } from '../../common/actions/chatActions.j import { ChatContextKeyExprs, ChatContextKeys } from '../../common/actions/chatContextKeys.js'; import { applyingChatEditsFailedContextKey, CHAT_EDITING_MULTI_DIFF_SOURCE_RESOLVER_SCHEME, chatEditingResourceContextKey, chatEditingWidgetFileStateContextKey, decidedChatEditingResourceContextKey, hasAppliedChatEditsContextKey, hasUndecidedChatEditingResourceContextKey, IChatEditingService, IChatEditingSession, ModifiedFileEntryState } from '../../common/editing/chatEditingService.js'; import { IChatService } from '../../common/chatService/chatService.js'; -import { isChatTreeItem, isRequestVM, isResponseVM } from '../../common/model/chatViewModel.js'; +import { isChatTreeItem, isEditableRequestVM, isRequestVM, isResponseVM } from '../../common/model/chatViewModel.js'; import { ChatAgentLocation, ChatConfiguration, ChatModeKind } from '../../common/constants.js'; import { CHAT_CATEGORY } from '../actions/chatActions.js'; import { ChatTreeItem, IChatWidget, IChatWidgetService } from '../chat.js'; @@ -671,7 +671,7 @@ registerAction2(class EditAction extends Action2 { id: MenuId.ChatMessageTitle, group: 'navigation', order: 2, - when: ContextKeyExpr.and(ContextKeyExpr.or(ContextKeyExpr.equals(`config.${ChatConfiguration.EditRequests}`, 'hover'), ContextKeyExpr.equals(`config.${ChatConfiguration.EditRequests}`, 'input')), ChatContextKeys.readOnly.negate()) + when: ContextKeyExpr.and(ContextKeyExpr.or(ContextKeyExpr.equals(`config.${ChatConfiguration.EditRequests}`, 'hover'), ContextKeyExpr.equals(`config.${ChatConfiguration.EditRequests}`, 'input')), ChatContextKeys.readOnly.negate(), ChatContextKeys.isEditableRequest) } ] }); @@ -689,7 +689,7 @@ registerAction2(class EditAction extends Action2 { return; } - if (isRequestVM(item)) { + if (isEditableRequestVM(item)) { widget?.startEditing(item.id); } } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts index 321fa60fa5e785..cc115ac399200e 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts @@ -62,7 +62,7 @@ import { ChatQuestionCarouselData } from '../../common/model/chatProgressTypes/c import { localChatSessionType, SessionType } from '../../common/chatSessionsService.js'; import { getChatSessionType } from '../../common/model/chatUri.js'; import { getExplicitFileOrImageAttachmentSummary, IChatRequestVariableEntry, isExplicitFileOrImageVariableEntry, isPasteVariableEntry } from '../../common/attachments/chatVariableEntries.js'; -import { getStickyScrollTargetItem, IChatChangesSummaryPart, IChatCodeCitations, IChatErrorDetailsPart, IChatReferences, IChatRendererContent, IChatRequestViewModel, IChatResponseViewModel, IChatViewModel, IChatWorkingProgress, isRequestVM, isResponseVM, IChatPendingDividerViewModel, isPendingDividerVM, IChatTurnPillsPart } from '../../common/model/chatViewModel.js'; +import { getStickyScrollTargetItem, IChatChangesSummaryPart, IChatCodeCitations, IChatErrorDetailsPart, IChatReferences, IChatRendererContent, IChatRequestViewModel, IChatResponseViewModel, IChatViewModel, IChatWorkingProgress, isEditableRequestVM, isRequestVM, isResponseVM, IChatPendingDividerViewModel, isPendingDividerVM, IChatTurnPillsPart } from '../../common/model/chatViewModel.js'; import { getNWords } from '../../common/model/chatWordCounter.js'; import { CHAT_OPEN_AGENT_HOST_CHAT_COMMAND_ID, ChatAgentLocation, ChatConfiguration, ChatModeKind, ChatProgressAnimation, CollapsedToolsDisplayMode, ThinkingDisplayMode } from '../../common/constants.js'; import { getConfiguredProgressAnimation } from './chatWorkingLogo.js'; @@ -1473,6 +1473,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer('chat.editRequests') !== 'none' && this.rendererOptions.editable) { + if (this.configService.getValue('chat.editRequests') !== 'none' && this.rendererOptions.editable && isEditableRequestVM(element)) { templateData.elementDisposables.add(dom.addDisposableListener(templateData.rowContainer, dom.EventType.KEY_DOWN, e => { const ev = new StandardKeyboardEvent(e); if (ev.equals(KeyCode.Space) || ev.equals(KeyCode.Enter)) { @@ -2471,7 +2472,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer('chat.editRequests') === 'inline' && this.rendererOptions.editable) { + if (this.configService.getValue('chat.editRequests') === 'inline' && this.rendererOptions.editable && isEditableRequestVM(element)) { container.classList.add('clickable'); store.add(dom.addDisposableListener(container, dom.EventType.CLICK, (e: MouseEvent) => { if (this.viewModel?.editing?.id === element.id) { @@ -5151,7 +5152,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer this.fireItemHeightChange(templateData))); if (isRequestVM(element)) { markdownPart.domNode.tabIndex = 0; - if (this.configService.getValue('chat.editRequests') === 'inline' && this.rendererOptions.editable) { + if (this.configService.getValue('chat.editRequests') === 'inline' && this.rendererOptions.editable && isEditableRequestVM(element)) { markdownPart.domNode.classList.add('clickable'); markdownPart.addDisposable(dom.addDisposableListener(markdownPart.domNode, dom.EventType.CLICK, (e: MouseEvent) => { if (this.viewModel?.editing?.id === element.id) { diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts index 7ed6a9bbcc194d..06eda31ccff351 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts @@ -44,6 +44,7 @@ import { ITextResourceEditorInput } from '../../../../../platform/editor/common/ import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { ServiceCollection } from '../../../../../platform/instantiation/common/serviceCollection.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; +import { INotificationService } from '../../../../../platform/notification/common/notification.js'; import { bindContextKey } from '../../../../../platform/observable/common/platformObservableUtils.js'; import product from '../../../../../platform/product/common/product.js'; import { Progress } from '../../../../../platform/progress/common/progress.js'; @@ -71,7 +72,7 @@ import { IChatSessionsService, localChatSessionType } from '../../common/chatSes import { IChatSlashCommandService } from '../../common/participants/chatSlashCommands.js'; import { IChatTodoListService } from '../../common/tools/chatTodoListService.js'; import { ChatRequestVariableSet, IChatRequestTranscriptContextVariableEntry, IChatRequestVariableEntry, isPastedTextArtifact, isPromptFileVariableEntry, isPromptTextVariableEntry, isWorkspaceVariableEntry, PromptFileVariableKind, toPromptFileVariableEntry } from '../../common/attachments/chatVariableEntries.js'; -import { ChatViewModel, IChatResponseViewModel, isRequestVM, isResponseVM } from '../../common/model/chatViewModel.js'; +import { ChatViewModel, IChatResponseViewModel, isEditableRequestVM, isRequestVM, isResponseVM } from '../../common/model/chatViewModel.js'; import { ChatMessageRole, IChatMessage } from '../../common/languageModels.js'; import { ChatAgentLocation, ChatConfiguration, ChatModeKind, ChatPermissionLevel, IResolvedNewChatSessionType, ThinkingDisplayMode } from '../../common/constants.js'; import { IChatGoalSummaryService } from '../chatGoalSummaryService.js'; @@ -584,6 +585,7 @@ export class ChatWidget extends Disposable implements IChatWidget { @IChatPasteTargetService private readonly chatPasteTargetService: IChatPasteTargetService, @IChatAccessibilityService private readonly chatAccessibilityService: IChatAccessibilityService, @ILogService private readonly logService: ILogService, + @INotificationService private readonly notificationService: INotificationService, @IThemeService private readonly themeService: IThemeService, @IChatSlashCommandService private readonly chatSlashCommandService: IChatSlashCommandService, @IChatEditingService chatEditingService: IChatEditingService, @@ -2224,7 +2226,11 @@ export class ChatWidget extends Disposable implements IChatWidget { private clickedRequest(item: IChatListItemTemplate) { const currentElement = item.currentElement; - if (isRequestVM(currentElement) && !this.viewModel?.editing) { + if (!isEditableRequestVM(currentElement)) { + return; + } + + if (!this.viewModel?.editing) { const requests = this.viewModel?.model.getRequests(); if (!requests || !this.viewModel?.sessionResource) { @@ -3236,6 +3242,19 @@ export class ChatWidget extends Disposable implements IChatWidget { return true; } + private _validateRequestEdit(): boolean { + const editing = this.viewModel?.editing; + if (!editing || editing.pendingKind === undefined) { + return true; + } + if (this.viewModel?.model.getPendingRequests().some(pending => pending.request.id === editing.id && pending.kind === ChatRequestQueueKind.Queued)) { + return true; + } + + this.notificationService.warn(localize('chat.editRequest.noLongerQueued', "This message is no longer queued and cannot be edited. Your edits have been kept in the input.")); + return false; + } + private async _acceptInput(query: { query: string } | undefined, options: IChatAcceptInputOptions = {}): Promise { if (!query && this.input.generating) { // if the user submits the input and generation finishes quickly, just submit it for them @@ -3251,7 +3270,7 @@ export class ChatWidget extends Disposable implements IChatWidget { await Event.toPromise(this.onDidChangeViewModel, this._store); } - if (!this.viewModel) { + if (!this.viewModel || !this._validateRequestEdit()) { return; } @@ -3316,6 +3335,9 @@ export class ChatWidget extends Disposable implements IChatWidget { if (await this._executeSlashCommandDuringRequest(requestInputs.input, { attachedContext }, isUserQuery, options.preserveFocus)) { return; } + if (!this._validateRequestEdit()) { + return; + } const isEditing = this.viewModel?.editing; const submittedFromEditing = shouldUnlockChatPetRequestRevision(isEditing !== undefined, isUserQuery); // Captured before `finishedEditing` tears the inline editor down, while `this.input` still diff --git a/src/vs/workbench/contrib/chat/common/actions/chatContextKeys.ts b/src/vs/workbench/contrib/chat/common/actions/chatContextKeys.ts index 7241af0630240c..68b18eab30a463 100644 --- a/src/vs/workbench/contrib/chat/common/actions/chatContextKeys.ts +++ b/src/vs/workbench/contrib/chat/common/actions/chatContextKeys.ts @@ -37,6 +37,7 @@ export namespace ChatContextKeys { export const contextMenuIsBackground = new RawContextKey('chatContextMenuIsBackground', false, { type: 'boolean', description: localize('chatContextMenuIsBackground', "Whether the chat context menu was opened from the transcript background rather than chat item content.") }); export const isFirstRequest = new RawContextKey('chatFirstRequest', false, { type: 'boolean', description: localize('chatFirstRequest', "The chat item is the first request in the session.") }); export const isPendingRequest = new RawContextKey('chatRequestIsPending', false, { type: 'boolean', description: localize('chatRequestIsPending', "True when the chat request item is pending in the queue.") }); + export const isEditableRequest = new RawContextKey('chatRequestIsEditable', false, { type: 'boolean', description: localize('chatRequestIsEditable', "True when the chat request item can be edited.") }); export const itemId = new RawContextKey('chatItemId', '', { type: 'string', description: localize('chatItemId', "The id of the chat item.") }); export const lastItemId = new RawContextKey('chatLastItemId', [], { type: 'string', description: localize('chatLastItemId', "The id of the last chat item.") }); diff --git a/src/vs/workbench/contrib/chat/common/model/chatViewModel.ts b/src/vs/workbench/contrib/chat/common/model/chatViewModel.ts index 91df3055f3c507..f8e9ae29185035 100644 --- a/src/vs/workbench/contrib/chat/common/model/chatViewModel.ts +++ b/src/vs/workbench/contrib/chat/common/model/chatViewModel.ts @@ -24,6 +24,11 @@ export function isRequestVM(item: unknown): item is IChatRequestViewModel { return !!item && typeof item === 'object' && 'message' in item; } +/** Pending steering may already be in the agent's input queue and cannot be safely replaced. */ +export function isEditableRequestVM(item: unknown): item is IChatRequestViewModel { + return isRequestVM(item) && item.pendingKind !== ChatRequestQueueKind.Steering; +} + export function isResponseVM(item: unknown): item is IChatResponseViewModel { return !!item && typeof (item as IChatResponseViewModel).setVote !== 'undefined'; } @@ -425,7 +430,7 @@ class ChatRequestViewModel implements IChatRequestViewModel { * An ID that changes when the request should be re-rendered. */ get dataId() { - return `${this.id}_${this._model.version + (this._model.response?.isComplete ? 1 : 0)}`; + return `${this.id}_${this._model.version + (this._model.response?.isComplete ? 1 : 0)}_${this._pendingKind ?? ''}`; } get sessionResource() { diff --git a/src/vs/workbench/contrib/chat/test/browser/accessibility/chatAccessibilityHelp.test.ts b/src/vs/workbench/contrib/chat/test/browser/accessibility/chatAccessibilityHelp.test.ts index 6d6c007be47794..6ab7d136c75016 100644 --- a/src/vs/workbench/contrib/chat/test/browser/accessibility/chatAccessibilityHelp.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/accessibility/chatAccessibilityHelp.test.ts @@ -14,6 +14,14 @@ import { AGENT_SESSION_RENAME_ACTION_ID } from '../../../browser/agentSessions/a suite('Chat Accessibility Help', () => { ensureNoDisposablesAreLeakedInTestSuite(); + test('distinguishes editable queued messages from pending steering', () => { + const help = getAccessibilityHelpText('agentView', new MockKeybindingService(), true); + assert.deepStrictEqual({ + queued: help.includes('Queued messages can be edited before they are sent'), + steering: help.includes('Pending steering messages cannot be edited'), + }, { queued: true, steering: true }); + }); + for (const type of ['panelChat', 'editsView', 'agentView'] as const) { test(`documents draft copying, preservation, and invitation dismissal in ${type}`, () => { const help = getAccessibilityHelpText(type, new MockKeybindingService(), false); diff --git a/src/vs/workbench/contrib/chat/test/browser/actions/chatQueueActions.test.ts b/src/vs/workbench/contrib/chat/test/browser/actions/chatQueueActions.test.ts index f70612f72d1b52..9f42efd30e7e10 100644 --- a/src/vs/workbench/contrib/chat/test/browser/actions/chatQueueActions.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/actions/chatQueueActions.test.ts @@ -12,6 +12,8 @@ import { URI } from '../../../../../../base/common/uri.js'; import { upcastPartial } from '../../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { ICodeEditor } from '../../../../../../editor/browser/editorBrowser.js'; +import { isIMenuItem, MenuId, MenuRegistry } from '../../../../../../platform/actions/common/actions.js'; +import { CommandsRegistry } from '../../../../../../platform/commands/common/commands.js'; import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; import { ContextKeyService } from '../../../../../../platform/contextkey/browser/contextKeyService.js'; import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; @@ -23,17 +25,92 @@ import { ILogService, NullLogService } from '../../../../../../platform/log/comm import { INotificationService } from '../../../../../../platform/notification/common/notification.js'; import { TestNotificationService } from '../../../../../../platform/notification/test/common/testNotificationService.js'; import { IChatWidget, IChatWidgetService } from '../../../browser/chat.js'; -import { ChatAskInSideChatAction, ChatQueueMessageAction, ChatSteerWithMessageAction, registerChatQueueActions } from '../../../browser/actions/chatQueueActions.js'; +import { ChatAskInSideChatAction, ChatEditPendingRequestAction, ChatQueueMessageAction, ChatRemovePendingRequestAction, ChatSendPendingImmediatelyAction, ChatSteerWithMessageAction, registerChatQueueActions } from '../../../browser/actions/chatQueueActions.js'; +import '../../../browser/chatEditing/chatEditingActions.js'; import { ChatContextKeys } from '../../../common/actions/chatContextKeys.js'; import { IChatSideChatService } from '../../../common/chatSideChatService.js'; import { ChatConfiguration } from '../../../common/constants.js'; import { IChatModel, IChatRequestModel } from '../../../common/model/chatModel.js'; -import { IChatViewModel } from '../../../common/model/chatViewModel.js'; +import { IChatRequestViewModel, IChatViewModel } from '../../../common/model/chatViewModel.js'; import { ChatRequestQueueKind } from '../../../common/chatService/chatService.js'; // Register actions once so the keybindings appear in KeybindingsRegistry. registerChatQueueActions(); +suite('Pending request editing actions', () => { + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + const editRequestId = 'workbench.action.chat.editRequests'; + + for (const editMode of ['inline', 'hover', 'input']) { + test(`hides steering edit actions in ${editMode} mode without hiding other queue actions`, () => { + const config = new TestConfigurationService({ [ChatConfiguration.EditRequests]: editMode }); + const contextKeyService = disposables.add(new ContextKeyService(config)); + const menuItems = MenuRegistry.getMenuItems(MenuId.ChatMessageTitle).filter(isIMenuItem); + const visibleActions = (pendingKind: ChatRequestQueueKind | undefined) => { + const context = contextKeyService.createOverlay([ + [ChatContextKeys.isRequest.key, true], + [ChatContextKeys.isPendingRequest.key, pendingKind !== undefined], + [ChatContextKeys.isEditableRequest.key, pendingKind !== ChatRequestQueueKind.Steering], + ]); + const actions = menuItems.filter(item => context.contextMatchesRules(item.when)).map(item => item.command.id); + return { + edit: actions.filter(id => id === editRequestId || id === ChatEditPendingRequestAction.ID), + remove: actions.includes(ChatRemovePendingRequestAction.ID), + send: actions.includes(ChatSendPendingImmediatelyAction.ID), + }; + }; + + assert.deepStrictEqual({ + steering: visibleActions(ChatRequestQueueKind.Steering), + queued: visibleActions(ChatRequestQueueKind.Queued), + sent: visibleActions(undefined), + }, { + steering: { edit: [], remove: true, send: true }, + queued: { edit: [editMode === 'inline' ? ChatEditPendingRequestAction.ID : editRequestId], remove: true, send: true }, + sent: { edit: editMode === 'inline' ? [] : [editRequestId], remove: false, send: false }, + }); + }); + } + + test('guards command and keyboard editing while preserving queued and sent editing', async () => { + const instantiationService = disposables.add(new TestInstantiationService()); + const sessionResource = URI.parse('test:///session'); + const editedRequests: string[] = []; + let focusedRequest: IChatRequestViewModel | undefined; + const widget = upcastPartial({ + startEditing: id => editedRequests.push(id), + getFocus: () => focusedRequest, + }); + instantiationService.stub(IChatWidgetService, upcastPartial({ + getWidgetBySessionResource: () => widget, + lastFocusedWidget: widget, + })); + const pendingAction = new ChatEditPendingRequestAction(); + const editCommand = CommandsRegistry.getCommand(editRequestId); + assert.ok(editCommand); + + for (const pendingKind of [ChatRequestQueueKind.Steering, ChatRequestQueueKind.Queued, undefined]) { + focusedRequest = upcastPartial({ + id: pendingKind ?? 'sent', + sessionResource, + message: { text: 'request', parts: [] }, + pendingKind, + }); + instantiationService.invokeFunction(accessor => pendingAction.run(accessor, focusedRequest)); + await instantiationService.invokeFunction(accessor => editCommand.handler(accessor, focusedRequest)); + await instantiationService.invokeFunction(accessor => editCommand.handler(accessor)); + } + + assert.deepStrictEqual(editedRequests, [ + ChatRequestQueueKind.Queued, + ChatRequestQueueKind.Queued, + ChatRequestQueueKind.Queued, + 'sent', + 'sent', + ]); + }); +}); + suite('Queue/Steer keybinding resolution', () => { ensureNoDisposablesAreLeakedInTestSuite(); diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/chatListRenderer.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/chatListRenderer.test.ts index b6d7eaa7bab423..51c1b90d09c67d 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/chatListRenderer.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/chatListRenderer.test.ts @@ -39,6 +39,7 @@ import { ChatInputPart } from '../../../browser/widget/input/chatInputPart.js'; import { ChatToolConfirmationCarouselPart } from '../../../browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.js'; import { ChatSubagentContentPart } from '../../../browser/widget/chatContentParts/chatSubagentContentPart.js'; import { OpenSubagentChatActionViewItem } from '../../../browser/widget/chatContentParts/chatSubagentOpenChat.js'; +import { ChatContextKeys } from '../../../common/actions/chatContextKeys.js'; import { ChatThinkingContentPart } from '../../../browser/widget/chatContentParts/chatThinkingContentPart.js'; import { ChatMarkdownContentPart } from '../../../browser/widget/chatContentParts/chatMarkdownContentPart.js'; import { aggregateChatEditDiffs } from '../../../browser/widget/chatContentParts/chatEditStatsButton.js'; @@ -817,6 +818,83 @@ suite('ChatListRenderer', () => { }); }); + for (const sticky of [false, true]) { + test(`pending steering has no mouse or keyboard edit affordances${sticky ? ' in sticky scroll' : ''}`, async () => { + const disposables = store.add(new DisposableStore()); + const instantiationService = workbenchInstantiationService(undefined, disposables); + const configurationService = new TestConfigurationService(); + await configurationService.setUserConfiguration(ChatConfiguration.EditRequests, 'inline'); + await configurationService.setUserConfiguration(ChatConfiguration.CheckpointsEnabled, false); + instantiationService.stub(IConfigurationService, configurationService); + instantiationService.stub(IChatService, new MockChatService()); + instantiationService.stub(IChatModelFeedbackSurveyService, new MockChatModelFeedbackSurveyService()); + instantiationService.stub(IChatAgentService, disposables.add(instantiationService.createInstance(ChatAgentService))); + + const model = disposables.add(instantiationService.createInstance(ChatModel, undefined, { initialLocation: ChatAgentLocation.Chat, canUseTools: true })); + const viewModel = disposables.add(instantiationService.createInstance(ChatViewModel, model, undefined)); + const text = 'request'; + const request = model.addRequest({ + text, + parts: [new ChatRequestTextPart(new OffsetRange(0, text.length), new Range(1, 1, 1, text.length + 1), text)] + }, { variables: [] }, Date.now()); + const container = mainWindow.document.createElement('div'); + container.classList.toggle('monaco-tree-sticky-row', sticky); + mainWindow.document.body.appendChild(container); + disposables.add(toDisposable(() => container.remove())); + const renderer = disposables.add(instantiationService.createInstance( + ChatListItemRenderer, + {} as ChatEditorOptions, + { editable: true }, + { + getListLength: () => 1, + container, + currentChatMode: () => ChatModeKind.Agent, + isStickyScrollEnabled: () => sticky, + refreshStickyScroll: () => { }, + stickyScrollTopPadding: 0, + }, + undefined, + viewModel, + )); + const template = renderer.renderTemplate(container); + disposables.add(toDisposable(() => renderer.disposeTemplate(template))); + let editEvents = 0; + disposables.add(renderer.onDidClickRequest(() => editEvents++)); + const states = []; + + for (const pendingKind of [ChatRequestQueueKind.Queued, ChatRequestQueueKind.Steering, undefined]) { + model.removePendingRequest(request.id); + if (pendingKind !== undefined) { + model.addPendingRequest(request, pendingKind, {}); + } + const requestViewModel = viewModel.getItems().filter(isRequestVM).find(item => item.pendingKind === pendingKind); + assert.ok(requestViewModel); + const node = { element: requestViewModel, children: [], depth: 0, visibleChildrenCount: 0, visibleChildIndex: 0, collapsible: false, collapsed: false, visible: true, filterData: undefined }; + renderer.renderElement(node, 0, template); + const markdown = template.value.querySelector('.rendered-markdown'); + assert.ok(markdown); + editEvents = 0; + markdown.click(); + for (const keyCode of [13, 32]) { + markdown.dispatchEvent(new mainWindow.KeyboardEvent('keydown', { keyCode, bubbles: true, cancelable: true })); + } + states.push({ + pendingKind, + editable: template.contextKeyService.getContextKeyValue(ChatContextKeys.isEditableRequest.key), + clickable: markdown.classList.contains('clickable'), + editEvents, + }); + renderer.disposeElement(node, 0, template); + } + + assert.deepStrictEqual(states, [ + { pendingKind: ChatRequestQueueKind.Queued, editable: true, clickable: true, editEvents: 3 }, + { pendingKind: ChatRequestQueueKind.Steering, editable: false, clickable: false, editEvents: 0 }, + { pendingKind: undefined, editable: true, clickable: true, editEvents: 3 }, + ]); + }); + } + test('pending divider clears a timestamp from a recycled request template', () => { const disposables = store.add(new DisposableStore()); const instantiationService = workbenchInstantiationService(undefined, disposables); diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/chatListWidget.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/chatListWidget.test.ts index db85137745fc90..a1e630b5c8882d 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/chatListWidget.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/chatListWidget.test.ts @@ -24,10 +24,10 @@ import { IChatAccessibilityService } from '../../../browser/chat.js'; import { ChatAttachmentWidgetRegistry, IChatAttachmentWidgetRegistry } from '../../../browser/attachments/chatAttachmentWidgetRegistry.js'; import { computeScrollDownState, getAnchoredScrollTop, AutoScrollHolds, UserToggleResizeState, ChatListWidget, IChatListWidgetOptions, getChatContextMenuTargetContext, isChatBackgroundContextMenuTarget } from '../../../browser/widget/chatListWidget.js'; import { ChatEditorOptions } from '../../../browser/widget/chatOptions.js'; -import { IChatService } from '../../../common/chatService/chatService.js'; +import { ChatRequestQueueKind, IChatService } from '../../../common/chatService/chatService.js'; import { IChatSideChatService } from '../../../common/chatSideChatService.js'; import { ChatAgentLocation, ChatConfiguration, ChatModeKind } from '../../../common/constants.js'; -import { ChatModel } from '../../../common/model/chatModel.js'; +import { ChatModel, ChatRequestModel } from '../../../common/model/chatModel.js'; import { ChatToolInvocation } from '../../../common/model/chatProgressTypes/chatToolInvocation.js'; import { ChatViewModel, isRequestVM, isResponseVM } from '../../../common/model/chatViewModel.js'; import { ChatAgentService, IChatAgentService } from '../../../common/participants/chatAgents.js'; @@ -428,6 +428,58 @@ suite('ChatListWidget', () => { disposables.dispose(); }); + test('refreshes editing affordances when a queued request becomes steering', async () => { + const { disposables, model, widget } = createWidget({ + rendererOptions: { editable: true }, + }, configurationService => { + configurationService.setUserConfiguration(ChatConfiguration.EditRequests, 'inline'); + }); + const text = 'pending request'; + const request = new ChatRequestModel({ + session: model, + message: { + text, + parts: [new ChatRequestTextPart(new OffsetRange(0, text.length), new Range(1, 1, 1, text.length + 1), text)], + }, + variableData: { variables: [] }, + timestamp: 0, + }); + model.addPendingRequest(request, ChatRequestQueueKind.Queued, {}); + widget.refresh(); + widget.layout(300, 500); + await waitForStableLayout(widget); + + let editEvents = 0; + disposables.add(widget.onDidClickRequest(() => editEvents++)); + const states = []; + for (const kind of [ChatRequestQueueKind.Queued, ChatRequestQueueKind.Steering, ChatRequestQueueKind.Queued]) { + model.setPendingRequests([{ requestId: request.id, kind }]); + widget.refresh(); + const template = widget.getTemplateDataForRequestId(request.id); + assert.ok(template && isRequestVM(template.currentElement)); + const markdown = template.value.querySelector('.rendered-markdown'); + assert.ok(markdown); + editEvents = 0; + markdown.click(); + for (const keyCode of [13, 32]) { + markdown.dispatchEvent(new mainWindow.KeyboardEvent('keydown', { keyCode, bubbles: true, cancelable: true })); + } + states.push({ + pendingKind: template.currentElement.pendingKind, + clickable: markdown.classList.contains('clickable'), + editEvents, + }); + } + + assert.deepStrictEqual(states, [ + { pendingKind: ChatRequestQueueKind.Queued, clickable: true, editEvents: 3 }, + { pendingKind: ChatRequestQueueKind.Steering, clickable: false, editEvents: 0 }, + { pendingKind: ChatRequestQueueKind.Queued, clickable: true, editEvents: 3 }, + ]); + + disposables.dispose(); + }); + test('keeps tree sticky scroll disabled when the legacy prompt header is selected', () => { const { disposables, container } = createWidget({}, configurationService => { configurationService.setUserConfiguration(PROMPT_TIMELINE_STICKY_SCROLL_SETTING, true); diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/chatWidget.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/chatWidget.test.ts index f62979319f45fe..0341739058e5de 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/chatWidget.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/chatWidget.test.ts @@ -8,13 +8,14 @@ import { mainWindow } from '../../../../../../base/browser/window.js'; import { DeferredPromise, timeout } from '../../../../../../base/common/async.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; import { Disposable, MutableDisposable } from '../../../../../../base/common/lifecycle.js'; -import { observableValue } from '../../../../../../base/common/observable.js'; +import { constObservable, observableValue } from '../../../../../../base/common/observable.js'; import { URI } from '../../../../../../base/common/uri.js'; import { mockObject, upcastPartial } from '../../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { OffsetRange } from '../../../../../../editor/common/core/ranges/offsetRange.js'; import { Range } from '../../../../../../editor/common/core/range.js'; import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { INotificationService } from '../../../../../../platform/notification/common/notification.js'; import { NullTelemetryService } from '../../../../../../platform/telemetry/common/telemetryUtils.js'; import { SaveReason } from '../../../../../common/editor.js'; import { ISaveAllEditorsOptions, ISaveEditorsResult } from '../../../../../services/editor/common/editorService.js'; @@ -22,9 +23,12 @@ import { TestEditorService } from '../../../../../test/browser/workbenchTestServ import { acceptAndAwaitSentRequest, ChatWidget, computeChatSessionStateIndicatorState, getImmediateSilentSlashCommandPart, layoutChatWidgetForInputHeight, saveAllBeforeChatSend, shouldShowChatTip, shouldShowChatWelcome, shouldUnlockChatPetQueueOrSteeringMessage, shouldUnlockChatPetRequestRevision } from '../../../browser/widget/chatWidget.js'; import { IChatListItemTemplate } from '../../../browser/widget/chatListRenderer.js'; import { IChatListItemRendererOptions } from '../../../browser/chat.js'; +import { IChatSubmitRequestHandlerService } from '../../../browser/chatSubmitRequestHandlerService.js'; import { ChatInputPart } from '../../../browser/widget/input/chatInputPart.js'; -import { ChatRequestQueueKind, ChatSendResult, ChatSendResultSent, IChatSendRequestData } from '../../../common/chatService/chatService.js'; -import { ChatAgentLocation, ChatConfiguration } from '../../../common/constants.js'; +import { ChatRequestVariableSet } from '../../../common/attachments/chatVariableEntries.js'; +import { ChatRequestQueueKind, ChatSendResult, ChatSendResultSent, IChatSendRequestData, IChatService } from '../../../common/chatService/chatService.js'; +import { ChatAgentLocation, ChatConfiguration, ChatModeKind } from '../../../common/constants.js'; +import { IChatPendingRequest, IChatRequestModel } from '../../../common/model/chatModel.js'; import { computeChatModelIsIdle } from '../../../common/model/chatModelIdle.js'; import { IChatRequestViewModel } from '../../../common/model/chatViewModel.js'; import { ChatRequestSlashCommandPart, ChatRequestTextPart, IParsedChatRequest } from '../../../common/requestParser/chatParserTypes.js'; @@ -156,21 +160,30 @@ suite('ChatWidget', () => { }]); }); - test('editing a steering request passes its model and configuration to the input', async () => { + async function createQueuedRequestEditWidget() { const modelId = 'agent-host-copilot:claude-opus-4.8'; const modelConfiguration = { reasoningEffort: 'xhigh' }; const configurationService = new TestConfigurationService(); await configurationService.setUserConfiguration('chat.editRequests', 'input'); + await configurationService.setUserConfiguration(ChatConfiguration.SaveBeforeSend, false); + let inputValue = 'original request'; const input = mockObject()({ element: mainWindow.document.createElement('div'), + currentModeKind: ChatModeKind.Agent, + generating: undefined, + hasPendingProgrammaticModelSelection: false, inputEditor: upcastPartial({ - getValue: () => 'original request', getModel: () => null, focus: () => { }, + getValue: () => inputValue, getModel: () => null, focus: () => { }, }), attachmentModel: upcastPartial({ getAttachmentIDs: () => new Set() }), dnd: upcastPartial({ setDisabledOverlay: () => { } }), onDidClickOverlay: Event.None, }); input.requestModelByIdentifier.resolves(true); + input.setValue.callsFake(value => { inputValue = value; }); + const attachedContext = new ChatRequestVariableSet(); + input.getAttachedContext.returns(attachedContext); + input.getAttachedAndImplicitContext.returns(attachedContext); const request = upcastPartial({ id: 'request', message: { text: 'original request', parts: [] }, @@ -178,23 +191,42 @@ suite('ChatWidget', () => { variables: [], modelId, modelConfiguration, - pendingKind: ChatRequestQueueKind.Steering, + pendingKind: ChatRequestQueueKind.Queued, }); + const pendingRequest: IChatPendingRequest = { + request: upcastPartial({ id: request.id }), + kind: ChatRequestQueueKind.Queued, + sendOptions: {}, + }; + let pendingRequests: readonly IChatPendingRequest[] = [pendingRequest]; let editing: IChatRequestViewModel | undefined; + const viewModel = { + model: { + getRequests: () => [], + getPendingRequests: () => pendingRequests, + setCheckpoint: () => { }, + hasActiveRequest: constObservable(false), + }, + sessionResource: URI.parse('agent-host-copilot:/session'), + get editing() { return editing; }, + setEditing: (request: IChatRequestViewModel | undefined) => { editing = request; }, + }; + const chatService = mockObject()({}); + const notificationService = mockObject()({}); + const submitRequestHandlerService = mockObject()({}); + submitRequestHandlerService.tryHandle.resolves(false); const widget = Object.create(ChatWidget.prototype) as ChatWidget; Object.defineProperties(widget, { _store: { value: store }, _editingAutoScrollHold: { value: store.add(new MutableDisposable()) }, + _onDidAcceptInput: { value: store.add(new Emitter()) }, configurationService: { value: configurationService }, telemetryService: { value: NullTelemetryService }, - viewModel: { - value: { - model: { getRequests: () => [], setCheckpoint: () => { } }, - sessionResource: URI.parse('agent-host-copilot:/session'), - get editing() { return editing; }, - setEditing: (request: IChatRequestViewModel) => { editing = request; }, - }, - }, + chatService: { value: chatService }, + notificationService: { value: notificationService }, + chatSubmitRequestHandlerService: { value: submitRequestHandlerService }, + _viewModel: { value: viewModel }, + viewOptions: { value: {} }, input: { value: input }, inputPart: { value: input }, contribs: { value: [] }, @@ -203,13 +235,86 @@ suite('ChatWidget', () => { value: { getTemplateDataForRequestId: () => ({ currentElement: request }), acquireAutoScrollHold: () => Disposable.None, + setScrollLock: () => { }, }, }, }); + return { + widget, input, request, chatService, notificationService, submitRequestHandlerService, + setPendingKind: (kind: ChatRequestQueueKind | undefined) => { + pendingRequests = kind === undefined ? [] : [{ ...pendingRequest, kind }]; + }, + }; + } + + test('editing a queued request passes its model and configuration to the input', async () => { + const { widget, input, request } = await createQueuedRequestEditWidget(); + widget.startEditing(request.id); + + assert.deepStrictEqual(input.requestModelByIdentifier.firstCall.args, [request.modelId, request.modelConfiguration]); + }); + + for (const duringSubmission of [false, true]) { + for (const kind of [ChatRequestQueueKind.Steering, undefined]) { + test(`preserves edits when the queued request is ${kind === undefined ? 'removed' : 'changed to steering'} ${duringSubmission ? 'during' : 'before'} submission`, async () => { + const { widget, input, request, chatService, notificationService, submitRequestHandlerService, setPendingKind } = await createQueuedRequestEditWidget(); + widget.startEditing(request.id); + input.setValue('edited request', false); + chatService.removePendingRequest.callsFake(() => assert.fail('Must not remove a request that is no longer queued')); + if (duringSubmission) { + submitRequestHandlerService.tryHandle.callsFake(async () => { + setPendingKind(kind); + return false; + }); + } else { + setPendingKind(kind); + } + + await widget.acceptInput(); + + assert.deepStrictEqual({ + pendingKinds: widget.viewModel?.model.getPendingRequests().map(pending => pending.kind), + editingRequest: widget.viewModel?.editing?.id, + input: widget.getInput(), + removed: chatService.removePendingRequest.callCount, + sent: chatService.sendRequest.callCount, + prepared: submitRequestHandlerService.tryHandle.callCount, + warnings: notificationService.warn.args, + }, { + pendingKinds: kind === undefined ? [] : [kind], + editingRequest: request.id, + input: 'edited request', + removed: 0, + sent: 0, + prepared: duringSubmission ? 1 : 0, + warnings: [['This message is no longer queued and cannot be edited. Your edits have been kept in the input.']], + }); + }); + } + } + + test('does not start editing a pending steering request', () => { + const request = upcastPartial({ + id: 'steering-request', + message: { text: 'original steering', parts: [] }, + pendingKind: ChatRequestQueueKind.Steering, + }); + const widget = Object.create(ChatWidget.prototype) as ChatWidget; + Object.defineProperties(widget, { + viewModel: { + value: { + model: { getRequests: () => assert.fail('Editing pending steering must not touch the model') }, + }, + }, + listWidget: { + value: { getTemplateDataForRequestId: () => ({ currentElement: request }) }, + }, + }); + widget.startEditing(request.id); - assert.deepStrictEqual(input.requestModelByIdentifier.firstCall.args, [modelId, modelConfiguration]); + assert.strictEqual(widget.viewModel?.editing, undefined); }); test('confirms before cancelling changed request edits', async () => { diff --git a/src/vs/workbench/contrib/chat/test/common/model/chatViewModel.test.ts b/src/vs/workbench/contrib/chat/test/common/model/chatViewModel.test.ts index 8b77609e8d3435..6e199e009569ce 100644 --- a/src/vs/workbench/contrib/chat/test/common/model/chatViewModel.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/model/chatViewModel.test.ts @@ -6,7 +6,7 @@ import assert from 'assert'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { ChatRequestQueueKind } from '../../../common/chatService/chatService.js'; -import { getStickyScrollTargetItem } from '../../../common/model/chatViewModel.js'; +import { getStickyScrollTargetItem, isEditableRequestVM } from '../../../common/model/chatViewModel.js'; interface ITestChatViewModelItem { readonly id: string; @@ -17,6 +17,18 @@ interface ITestChatViewModelItem { suite('ChatViewModel', () => { ensureNoDisposablesAreLeakedInTestSuite(); + test('pending steering requests are not editable', () => { + const message = { text: 'request', parts: [] }; + + assert.deepStrictEqual([ + isEditableRequestVM({ message }), + isEditableRequestVM({ message, pendingKind: ChatRequestQueueKind.Queued }), + isEditableRequestVM({ message, pendingKind: ChatRequestQueueKind.Steering }), + isEditableRequestVM({ kind: 'pendingDivider' }), + isEditableRequestVM(undefined), + ], [true, true, false, false, false]); + }); + test('sticky scroll target ignores trailing pending items', () => { const response: ITestChatViewModelItem = { id: 'response' }; const pendingOnly: ITestChatViewModelItem = { id: 'pending-only', pendingKind: ChatRequestQueueKind.Queued };