diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts index 7d49548bc5c0b..25310ee3d4cc9 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts @@ -330,6 +330,10 @@ export class ChatTerminalToolProgressPart extends BaseChatToolInvocationSubPart return this._contentIndex; } + public get terminalToolSessionId(): string | undefined { + return this._terminalData.terminalToolSessionId; + } + constructor( toolInvocation: IChatToolInvocation | IChatToolInvocationSerialized, terminalData: IChatTerminalToolInvocationData | ILegacyChatTerminalToolInvocationData, @@ -438,7 +442,6 @@ export class ChatTerminalToolProgressPart extends BaseChatToolInvocationSubPart initializeTerminalActionsOnce(); }); - // Listen for continue in background — updates toolbar to auto-hide the action const terminalToolSessionId = this._terminalData.terminalToolSessionId; if (terminalToolSessionId) { if (this._terminalData.isPty === false) { @@ -449,13 +452,6 @@ export class ChatTerminalToolProgressPart extends BaseChatToolInvocationSubPart } })); } - this._register(this._terminalChatService.onDidContinueInBackground(sessionId => { - if (sessionId === terminalToolSessionId) { - this._terminalData.didContinueInBackground = true; - this._toolbarCanContinueInBackground = false; - this._updateToolbarActions(); - } - })); } let pastTenseMessage: string | undefined; if (toolInvocation.pastTenseMessage) { @@ -1217,6 +1213,12 @@ export class ChatTerminalToolProgressPart extends BaseChatToolInvocationSubPart } } + public markContinuedInBackground(): void { + this._terminalData.didContinueInBackground = true; + this._toolbarCanContinueInBackground = false; + this._updateToolbarActions(); + } + public async toggleOutputFromAction(): Promise { this._userToggledOutput = true; diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatTerminalToolProgressPart.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatTerminalToolProgressPart.test.ts index 03fedbf67d120..bee11808a7e7f 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatTerminalToolProgressPart.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatTerminalToolProgressPart.test.ts @@ -6,26 +6,220 @@ import assert from 'assert'; import type { Terminal } from '@xterm/xterm'; import { importAMDNodeModule } from '../../../../../../../amdX.js'; +import { renderAsPlaintext } from '../../../../../../../base/browser/markdownRenderer.js'; import { mainWindow } from '../../../../../../../base/browser/window.js'; import { Emitter, Event } from '../../../../../../../base/common/event.js'; import { observableValue } from '../../../../../../../base/common/observable.js'; import { URI } from '../../../../../../../base/common/uri.js'; import { toDisposable } from '../../../../../../../base/common/lifecycle.js'; +import { mock } from '../../../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../../base/test/common/utils.js'; import { runWithFakedTimers } from '../../../../../../../base/test/common/timeTravelScheduler.js'; import { timeout } from '../../../../../../../base/common/async.js'; import { TestInstantiationService } from '../../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { IAccessibleViewService } from '../../../../../../../platform/accessibility/browser/accessibleView.js'; +import { IMarkdownRenderer } from '../../../../../../../platform/markdown/browser/markdownRenderer.js'; +import { TerminalCapabilityStore } from '../../../../../../../platform/terminal/common/capabilities/terminalCapabilityStore.js'; import { workbenchInstantiationService } from '../../../../../../test/browser/workbenchTestServices.js'; +import { IAiEditTelemetryService } from '../../../../../editTelemetry/browser/telemetry/aiEditTelemetry/aiEditTelemetryService.js'; +import { IChatOutputRendererService } from '../../../../browser/chatOutputItemRenderer.js'; +import { IChatMarkdownAnchorService } from '../../../../browser/widget/chatContentParts/chatMarkdownAnchorService.js'; import { IChatContentPartRenderContext, InlineTextModelCollection } from '../../../../browser/widget/chatContentParts/chatContentParts.js'; import { DiffEditorPool, EditorPool } from '../../../../browser/widget/chatContentParts/chatContentCodePools.js'; -import { ChatTerminalThinkingCollapsibleWrapper, ChatTerminalToolOutputSection } from '../../../../browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.js'; +import { ChatTerminalThinkingCollapsibleWrapper, ChatTerminalToolOutputSection, ChatTerminalToolProgressPart } from '../../../../browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.js'; +import { IChatSessionsService } from '../../../../common/chatSessionsService.js'; +import { IChatTerminalToolInvocationData, IChatToolInvocationSerialized, ToolConfirmKind } from '../../../../common/chatService/chatService.js'; import { IChatResponseViewModel } from '../../../../common/model/chatViewModel.js'; import { TerminalToolAutoExpand, TerminalToolAutoExpandTimeout } from '../../../../browser/widget/chatContentParts/toolInvocationParts/terminalToolAutoExpand.js'; -import { ITerminalConfigurationService, ITerminalService, type IDetachedXTermOptions } from '../../../../../terminal/browser/terminal.js'; +import { IChatTerminalToolProgressPart, ITerminalChatService, ITerminalConfigurationService, ITerminalInstance, ITerminalService, type IDetachedXTermOptions } from '../../../../../terminal/browser/terminal.js'; import type { ITerminalFont } from '../../../../../terminal/common/terminal.js'; import { createFakeDetachedTerminal } from '../../../../../terminal/test/browser/chatTerminalMirrorTestUtils.js'; +function listenerCount(emitter: Emitter): number { + return (emitter as unknown as { _size: number })._size ?? 0; +} + +class TestTerminalChatService extends mock() { + override readonly onDidRegisterTerminalInstanceWithToolSession = Event.None; + override readonly onDidRegisterOutputSource = Event.None; + override readonly onDidContinueInBackground: Event; + + private readonly progressParts = new Set(); + + constructor( + private readonly continueInBackgroundEmitter: Emitter, + private readonly terminalInstance: ITerminalInstance, + ) { + super(); + this.onDidContinueInBackground = continueInBackgroundEmitter.event; + } + + override async getTerminalInstanceByToolSessionId(_terminalToolSessionId: string): Promise { + return this.terminalInstance; + } + + override registerProgressPart(part: IChatTerminalToolProgressPart) { + this.progressParts.add(part); + return toDisposable(() => this.progressParts.delete(part)); + } + + override continueInBackground(terminalToolSessionId: string): void { + this.continueInBackgroundEmitter.fire(terminalToolSessionId); + for (const part of this.progressParts) { + if (part.terminalToolSessionId === terminalToolSessionId) { + part.markContinuedInBackground(); + } + } + } + + override isBackgroundTerminal(): boolean { + return false; + } + + override getOutputSource() { + return undefined; + } + + override getAhpCommandSource() { + return undefined; + } + + override setFocusedProgressPart(): void { } + override clearFocusedProgressPart(): void { } +} + +suite('ChatTerminalToolProgressPart listener ownership', () => { + const store = ensureNoDisposablesAreLeakedInTestSuite(); + + test('rendered parts do not accumulate continue listeners and duplicate rows update', async () => { + const instantiationService = workbenchInstantiationService(undefined, store); + const continueInBackgroundEmitter = store.add(new Emitter()); + const capabilities = store.add(new TerminalCapabilityStore()); + const terminalInstance = new class extends mock() { + override readonly isDisposed = false; + override readonly onDisposed = Event.None; + override readonly onWillData = Event.None; + override readonly capabilities = capabilities; + }(); + const terminalChatService = new TestTerminalChatService(continueInBackgroundEmitter, terminalInstance); + instantiationService.stub(ITerminalChatService, terminalChatService); + instantiationService.stub(ITerminalService, new class extends mock() { + override readonly whenConnected = Promise.resolve(); + }()); + instantiationService.stub(IAccessibleViewService, new class extends mock() { }()); + instantiationService.stub(IChatMarkdownAnchorService, { + _serviceBrand: undefined, + register: () => toDisposable(() => { }), + lastFocusedAnchor: undefined, + }); + instantiationService.stub(IAiEditTelemetryService, new class extends mock() { }()); + instantiationService.stub(IChatOutputRendererService, new class extends mock() { + override hasCodeBlockRenderer(): boolean { + return false; + } + }()); + instantiationService.stub(IChatSessionsService, new class extends mock() { }()); + + const markdownRenderer: IMarkdownRenderer = { + render: (markdown, _options, outElement) => { + const element = outElement ?? mainWindow.document.createElement('div'); + element.textContent = renderAsPlaintext(markdown); + return { element, dispose() { } }; + } + }; + const editorPool = Object.create(EditorPool.prototype) as EditorPool; + const host = mainWindow.document.createElement('div'); + mainWindow.document.body.appendChild(host); + store.add(toDisposable(() => host.remove())); + const eventSessionIds: string[] = []; + store.add(continueInBackgroundEmitter.event(sessionId => eventSessionIds.push(sessionId))); + const listenerCountBeforeRender = listenerCount(continueInBackgroundEmitter); + + const targetSessionId = 'terminal-session-target'; + const terminalData: IChatTerminalToolInvocationData[] = []; + const parts: ChatTerminalToolProgressPart[] = []; + for (let index = 0; index < 50; index++) { + const data: IChatTerminalToolInvocationData = { + kind: 'terminal', + commandLine: { original: `echo ${index}` }, + language: 'shellscript', + terminalToolSessionId: index === 24 || index === 25 ? targetSessionId : `terminal-session-${index}`, + }; + const invocation: IChatToolInvocationSerialized = { + presentation: undefined, + toolSpecificData: data, + invocationMessage: 'Running command', + originMessage: undefined, + pastTenseMessage: 'Ran command', + isConfirmed: { type: ToolConfirmKind.ConfirmationNotNeeded }, + isComplete: true, + toolCallId: `tool-call-${index}`, + toolId: 'run_in_terminal', + source: undefined, + kind: 'toolInvocationSerialized', + }; + const element = Object.assign(Object.create(null) as IChatResponseViewModel, { + id: `response-${index}`, + isComplete: true, + sessionResource: URI.parse('chat-session://test/session'), + setVote() { }, + get model() { return {} as IChatResponseViewModel['model']; }, + }); + const context: IChatContentPartRenderContext = { + element, + elementIndex: index, + container: host, + content: [invocation], + contentIndex: 0, + inlineTextModels: Object.create(InlineTextModelCollection.prototype) as InlineTextModelCollection, + editorPool, + codeBlockStartIndex: 0, + treeStartIndex: 0, + diffEditorPool: Object.create(DiffEditorPool.prototype) as DiffEditorPool, + currentWidth: observableValue('testWidth', 500), + onDidChangeVisibility: Event.None, + }; + const part = store.add(instantiationService.createInstance( + ChatTerminalToolProgressPart, + invocation, + data, + context, + markdownRenderer, + editorPool, + () => 500, + 0, + )); + host.appendChild(part.domNode); + terminalData.push(data); + parts.push(part); + } + await timeout(0); + + const listenerCountAfterRender = listenerCount(continueInBackgroundEmitter); + const actionCountsBefore = parts.map(part => part.domNode.querySelectorAll('.action-item').length); + parts[24].continueInBackground(); + const actionCountsAfter = parts.map(part => part.domNode.querySelectorAll('.action-item').length); + + assert.deepStrictEqual({ + renderedRows: parts.filter(part => part.domNode.isConnected).length, + listenerCounts: [listenerCountBeforeRender, listenerCountAfterRender], + actionCountsBefore: [...new Set(actionCountsBefore)], + continuedRows: terminalData.flatMap((data, index) => data.didContinueInBackground ? [index] : []), + matchingActionCountsAfter: [actionCountsAfter[24], actionCountsAfter[25]], + unmatchedActionCountAfter: actionCountsAfter[0], + eventSessionIds, + }, { + renderedRows: 50, + listenerCounts: [1, 1], + actionCountsBefore: [2], + continuedRows: [24, 25], + matchingActionCountsAfter: [1, 1], + unmatchedActionCountAfter: 2, + eventSessionIds: [targetSessionId], + }); + }); +}); + suite('ChatTerminalToolProgressPart Auto-Expand Logic', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); diff --git a/src/vs/workbench/contrib/terminal/browser/terminal.ts b/src/vs/workbench/contrib/terminal/browser/terminal.ts index 593e875a60dab..0dcfa551ee8b9 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminal.ts @@ -123,10 +123,12 @@ export interface IAhpTerminalCommandSource extends IDisposable { export interface IChatTerminalToolProgressPart { readonly elementIndex: number; readonly contentIndex: number; + readonly terminalToolSessionId: string | undefined; focusTerminal(): Promise; toggleOutputFromKeyboard(): Promise; toggleOutputFromAction(): Promise; continueInBackground(): void; + markContinuedInBackground(): void; focusOutput(): void; getCommandAndOutputAsText(): string | undefined; } diff --git a/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts b/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts index 584d47815e780..33c5d43295d6e 100644 --- a/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts +++ b/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts @@ -467,6 +467,11 @@ export class TerminalChatService extends Disposable implements ITerminalChatServ continueInBackground(terminalToolSessionId: string): void { this._onDidContinueInBackground.fire(terminalToolSessionId); + for (const part of this._activeProgressParts) { + if (part.terminalToolSessionId === terminalToolSessionId) { + part.markContinuedInBackground(); + } + } } registerAhpCommandSource(terminalToolSessionId: string, source: IAhpTerminalCommandSource, promisedTerminal: Promise): IDisposable { diff --git a/src/vs/workbench/contrib/terminalContrib/chat/test/browser/terminalChatService.test.ts b/src/vs/workbench/contrib/terminalContrib/chat/test/browser/terminalChatService.test.ts index 7068955a9e67c..5ed938b4586ad 100644 --- a/src/vs/workbench/contrib/terminalContrib/chat/test/browser/terminalChatService.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chat/test/browser/terminalChatService.test.ts @@ -17,7 +17,7 @@ import { ILogService, NullLogService } from '../../../../../../platform/log/comm import { ITreeSitterLibraryService } from '../../../../../../editor/common/services/treeSitter/treeSitterLibraryService.js'; import { InMemoryStorageService, IStorageService } from '../../../../../../platform/storage/common/storage.js'; import { IChatService } from '../../../../chat/common/chatService/chatService.js'; -import { IAhpTerminalCommandSource, ITerminalInstance, ITerminalService } from '../../../../terminal/browser/terminal.js'; +import { IAhpTerminalCommandSource, IChatTerminalToolProgressPart, ITerminalInstance, ITerminalService } from '../../../../terminal/browser/terminal.js'; import { TerminalChatService } from '../../browser/terminalChatService.js'; /** @@ -106,6 +106,35 @@ suite('TerminalChatService', () => { assert.strictEqual(service.getToolSessionIdForInstance(instance), 'tool-session-a'); }); + test('continueInBackground notifies every matching progress part', () => { + const markedPartIndices: number[] = []; + const targetSessionId = 'tool-session-target'; + for (let index = 0; index < 50; index++) { + const sessionId = index === 25 || index === 26 ? targetSessionId : `tool-session-${index}`; + store.add(service.registerProgressPart(new class extends mock() { + override readonly elementIndex = index; + override readonly contentIndex = 0; + override readonly terminalToolSessionId = sessionId; + + override markContinuedInBackground(): void { + markedPartIndices.push(index); + } + }())); + } + const eventSessionIds: string[] = []; + store.add(service.onDidContinueInBackground(sessionId => eventSessionIds.push(sessionId))); + + service.continueInBackground(targetSessionId); + + assert.deepStrictEqual({ + markedPartIndices, + eventSessionIds, + }, { + markedPartIndices: [25, 26], + eventSessionIds: [targetSessionId], + }); + }); + test('getTerminalInstanceByToolSessionId waits for pending AHP terminal creation', async () => { const pendingTerminal = new DeferredPromise(); const instance = { instanceId: 3 } as ITerminalInstance;