diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/basicExecuteStrategy.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/basicExecuteStrategy.ts index 25e6f69fc8f..de2f1ae7056 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/basicExecuteStrategy.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/basicExecuteStrategy.ts @@ -6,7 +6,7 @@ import type { CancellationToken } from '../../../../../../base/common/cancellation.js'; import { CancellationError } from '../../../../../../base/common/errors.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; -import { DisposableStore, MutableDisposable } from '../../../../../../base/common/lifecycle.js'; +import { Disposable, DisposableStore, MutableDisposable } from '../../../../../../base/common/lifecycle.js'; import { isNumber } from '../../../../../../base/common/types.js'; import type { ICommandDetectionCapability } from '../../../../../../platform/terminal/common/capabilities/capabilities.js'; import { ITerminalLogService } from '../../../../../../platform/terminal/common/terminal.js'; @@ -37,11 +37,11 @@ import { createAltBufferPromise, setupRecreatingStartMarker } from './strategyHe * output. We lean on the LLM to be able to differentiate the actual output from prompts and bad * output when it's not ideal. */ -export class BasicExecuteStrategy implements ITerminalExecuteStrategy { +export class BasicExecuteStrategy extends Disposable implements ITerminalExecuteStrategy { readonly type = 'basic'; - private readonly _startMarker = new MutableDisposable(); + private readonly _startMarker = this._register(new MutableDisposable()); - private readonly _onDidCreateStartMarker = new Emitter; + private readonly _onDidCreateStartMarker = this._register(new Emitter); public onDidCreateStartMarker: Event = this._onDidCreateStartMarker.event; @@ -51,6 +51,7 @@ export class BasicExecuteStrategy implements ITerminalExecuteStrategy { private readonly _commandDetection: ICommandDetectionCapability, @ITerminalLogService private readonly _logService: ITerminalLogService, ) { + super(); } async execute(commandLine: string, token: CancellationToken, commandId?: string): Promise { diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/executeStrategy.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/executeStrategy.ts index cae93dc83b7..f95f297a958 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/executeStrategy.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/executeStrategy.ts @@ -6,11 +6,11 @@ import { DeferredPromise, RunOnceScheduler } from '../../../../../../base/common/async.js'; import type { CancellationToken } from '../../../../../../base/common/cancellation.js'; import type { Event } from '../../../../../../base/common/event.js'; -import { DisposableStore } from '../../../../../../base/common/lifecycle.js'; +import { DisposableStore, type IDisposable } from '../../../../../../base/common/lifecycle.js'; import type { ITerminalInstance } from '../../../../terminal/browser/terminal.js'; import type { IMarker as IXtermMarker } from '@xterm/xterm'; -export interface ITerminalExecuteStrategy { +export interface ITerminalExecuteStrategy extends IDisposable { readonly type: 'rich' | 'basic' | 'none'; /** * Executes a command line and gets a result designed to be passed directly to an LLM. The diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/noneExecuteStrategy.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/noneExecuteStrategy.ts index 523906025b7..ddc2a7f8939 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/noneExecuteStrategy.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/noneExecuteStrategy.ts @@ -6,7 +6,7 @@ import type { CancellationToken } from '../../../../../../base/common/cancellation.js'; import { CancellationError } from '../../../../../../base/common/errors.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; -import { DisposableStore, MutableDisposable } from '../../../../../../base/common/lifecycle.js'; +import { Disposable, DisposableStore, MutableDisposable } from '../../../../../../base/common/lifecycle.js'; import { ITerminalLogService } from '../../../../../../platform/terminal/common/terminal.js'; import { waitForIdle, waitForIdleWithPromptHeuristics, type ITerminalExecuteStrategy, type ITerminalExecuteStrategyResult } from './executeStrategy.js'; import type { IMarker as IXtermMarker } from '@xterm/xterm'; @@ -19,12 +19,12 @@ import { createAltBufferPromise, setupRecreatingStartMarker } from './strategyHe * with `sendText` instead of `shellIntegration.executeCommand` and relying on idle events instead * of execution events. */ -export class NoneExecuteStrategy implements ITerminalExecuteStrategy { +export class NoneExecuteStrategy extends Disposable implements ITerminalExecuteStrategy { readonly type = 'none'; - private readonly _startMarker = new MutableDisposable(); + private readonly _startMarker = this._register(new MutableDisposable()); - private readonly _onDidCreateStartMarker = new Emitter; + private readonly _onDidCreateStartMarker = this._register(new Emitter); public onDidCreateStartMarker: Event = this._onDidCreateStartMarker.event; constructor( @@ -32,6 +32,7 @@ export class NoneExecuteStrategy implements ITerminalExecuteStrategy { private readonly _hasReceivedUserInput: () => boolean, @ITerminalLogService private readonly _logService: ITerminalLogService, ) { + super(); } async execute(commandLine: string, token: CancellationToken, commandId?: string): Promise { diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/richExecuteStrategy.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/richExecuteStrategy.ts index c65b53492d6..b68f7a499d4 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/richExecuteStrategy.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/richExecuteStrategy.ts @@ -6,7 +6,7 @@ import type { CancellationToken } from '../../../../../../base/common/cancellation.js'; import { CancellationError } from '../../../../../../base/common/errors.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; -import { DisposableStore, MutableDisposable } from '../../../../../../base/common/lifecycle.js'; +import { Disposable, DisposableStore, MutableDisposable } from '../../../../../../base/common/lifecycle.js'; import { isNumber } from '../../../../../../base/common/types.js'; import type { ICommandDetectionCapability } from '../../../../../../platform/terminal/common/capabilities/capabilities.js'; import { ITerminalLogService } from '../../../../../../platform/terminal/common/terminal.js'; @@ -22,11 +22,11 @@ import { createAltBufferPromise, setupRecreatingStartMarker } from './strategyHe * wrong in this state, minimal verification is done in this mode since rich command detection is a * strong signal that it's behaving correctly. */ -export class RichExecuteStrategy implements ITerminalExecuteStrategy { +export class RichExecuteStrategy extends Disposable implements ITerminalExecuteStrategy { readonly type = 'rich'; - private readonly _startMarker = new MutableDisposable(); + private readonly _startMarker = this._register(new MutableDisposable()); - private readonly _onDidCreateStartMarker = new Emitter; + private readonly _onDidCreateStartMarker = this._register(new Emitter); public onDidCreateStartMarker: Event = this._onDidCreateStartMarker.event; constructor( @@ -34,6 +34,7 @@ export class RichExecuteStrategy implements ITerminalExecuteStrategy { private readonly _commandDetection: ICommandDetectionCapability, @ITerminalLogService private readonly _logService: ITerminalLogService, ) { + super(); } async execute(commandLine: string, token: CancellationToken, commandId?: string): Promise { diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts index c5dc252fc85..f05e44f4010 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts @@ -724,25 +724,24 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { let executionPromise: Promise | undefined; try { - const strategy = this._getExecuteStrategy(toolTerminal.shellIntegrationQuality, toolTerminal, commandDetection!); + // Create unified ActiveTerminalExecution (creates and owns the strategy) + const execution = new ActiveTerminalExecution( + this._instantiationService, + chatSessionId, + termId, + toolTerminal, + commandDetection!, + args.isBackground + ); if (toolTerminal.shellIntegrationQuality === ShellIntegrationQuality.None) { toolResultMessage = '$(info) Enable [shell integration](https://code.visualstudio.com/docs/terminal/shell-integration) to improve command detection'; } - this._logService.debug(`RunInTerminalTool: Using \`${strategy.type}\` execute strategy for command \`${command}\``); - - // Create unified ActiveTerminalExecution - const execution = new ActiveTerminalExecution( - toolTerminal.instance, - chatSessionId, - termId, - strategy, - args.isBackground - ); - // Don't add execution to store - its lifecycle is managed separately via _activeExecutions + this._logService.debug(`RunInTerminalTool: Using \`${execution.strategy.type}\` execute strategy for command \`${command}\``); + store.add(execution); RunInTerminalTool._activeExecutions.set(termId, execution); // Set up OutputMonitor when start marker is created - store.add(strategy.onDidCreateStartMarker(startMarker => { + store.add(execution.strategy.onDidCreateStartMarker(startMarker => { if (!outputMonitor) { outputMonitor = store.add(this._instantiationService.createInstance( OutputMonitor, @@ -853,7 +852,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { toolSpecificData.terminalCommandState = state; } - this._logService.debug(`RunInTerminalTool: Finished \`${strategy.type}\` execute strategy with exitCode \`${executeResult.exitCode}\`, result.length \`${executeResult.output?.length}\`, error \`${executeResult.error}\``); + this._logService.debug(`RunInTerminalTool: Finished \`${execution.strategy.type}\` execute strategy with exitCode \`${executeResult.exitCode}\`, result.length \`${executeResult.output?.length}\`, error \`${executeResult.error}\``); outputLineCount = executeResult.output === undefined ? 0 : count(executeResult.output.trim(), '\n') + 1; exitCode = executeResult.exitCode; error = executeResult.error; @@ -1131,22 +1130,6 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { } } } - - private _getExecuteStrategy(shellIntegrationQuality: ShellIntegrationQuality, toolTerminal: IToolTerminal, commandDetection: ICommandDetectionCapability): ITerminalExecuteStrategy { - let strategy: ITerminalExecuteStrategy; - switch (shellIntegrationQuality) { - case ShellIntegrationQuality.None: - strategy = this._instantiationService.createInstance(NoneExecuteStrategy, toolTerminal.instance, () => toolTerminal.receivedUserInput ?? false); - break; - case ShellIntegrationQuality.Basic: - strategy = this._instantiationService.createInstance(BasicExecuteStrategy, toolTerminal.instance, () => toolTerminal.receivedUserInput ?? false, commandDetection!); - break; - case ShellIntegrationQuality.Rich: - strategy = this._instantiationService.createInstance(RichExecuteStrategy, toolTerminal.instance, commandDetection!); - break; - } - return strategy; - } // #endregion } @@ -1176,18 +1159,30 @@ class ActiveTerminalExecution extends Disposable { return this._startMarker; } + readonly strategy: ITerminalExecuteStrategy; + private readonly _toolTerminal: IToolTerminal; + + get instance(): ITerminalInstance { + return this._toolTerminal.instance; + } + constructor( - readonly instance: ITerminalInstance, + instantiationService: IInstantiationService, readonly sessionId: string, readonly termId: string, - private readonly _strategy: ITerminalExecuteStrategy, + toolTerminal: IToolTerminal, + commandDetection: ICommandDetectionCapability, isBackground: boolean, ) { super(); + this._toolTerminal = toolTerminal; this._isBackground = isBackground; this._completionDeferred = new DeferredPromise(); - this._register(this._strategy.onDidCreateStartMarker(marker => { + // Create and register the strategy for disposal to clean up its internal resources + this.strategy = this._register(this._createStrategy(instantiationService, commandDetection)); + + this._register(this.strategy.onDidCreateStartMarker(marker => { if (marker) { // Don't register marker - strategy already manages its lifecycle this._startMarker = marker; @@ -1195,6 +1190,17 @@ class ActiveTerminalExecution extends Disposable { })); } + private _createStrategy(instantiationService: IInstantiationService, commandDetection: ICommandDetectionCapability): ITerminalExecuteStrategy { + switch (this._toolTerminal.shellIntegrationQuality) { + case ShellIntegrationQuality.None: + return instantiationService.createInstance(NoneExecuteStrategy, this._toolTerminal.instance, () => this._toolTerminal.receivedUserInput ?? false); + case ShellIntegrationQuality.Basic: + return instantiationService.createInstance(BasicExecuteStrategy, this._toolTerminal.instance, () => this._toolTerminal.receivedUserInput ?? false, commandDetection); + case ShellIntegrationQuality.Rich: + return instantiationService.createInstance(RichExecuteStrategy, this._toolTerminal.instance, commandDetection); + } + } + /** * Starts the command execution using the execute strategy. * @param commandLine The command to execute @@ -1204,7 +1210,7 @@ class ActiveTerminalExecution extends Disposable { */ async start(commandLine: string, token: CancellationToken, commandId?: string): Promise { try { - const result = await this._strategy.execute(commandLine, token, commandId); + const result = await this.strategy.execute(commandLine, token, commandId); this._completionDeferred.complete(result); return result; } catch (e) {