Merge pull request microsoft#257147 from microsoft/tyriar/257129

Handle marker disposable when fetching tool output

Author: Tyriar

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/basicExecuteStrategy.ts

@@ -7,6 +7,7 @@ import type { CancellationToken } from '../../../../../../base/common/cancellati
 import { CancellationError } from '../../../../../../base/common/errors.js';
 import { Event } from '../../../../../../base/common/event.js';
 import { DisposableStore } 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';
 import type { ITerminalInstance } from '../../../../terminal/browser/terminal.js';
@@ -80,14 +81,22 @@ export class BasicExecuteStrategy implements ITerminalExecuteStrategy {
 			this._logService.debug('RunInTerminalTool#Basic: Waiting for idle');
 			await waitForIdle(this._instance.onData, 1000);
 
+			// Record where the command started. If the marker gets disposed, re-created it where
+			// the cursor is. This can happen in prompts where they clear the line and rerender it
+			// like powerlevel10k's transient prompt
+			let startMarker = store.add(xterm.raw.registerMarker());
+			store.add(startMarker.onDispose(() => {
+				this._logService.debug(`RunInTerminalTool#Basic: Start marker was disposed, recreating`);
+				startMarker = xterm.raw.registerMarker();
+			}));
+
 			// Execute the command
-			const startMarker = store.add(xterm.raw.registerMarker());
 			this._logService.debug(`RunInTerminalTool#Basic: Executing command line \`${commandLine}\``);
 			this._instance.runCommand(commandLine, true);
 
 			// Wait for the next end execution event - note that this may not correspond to the actual
 			// execution requested
-			const doneData = await onDone;
+			const finishedCommand = await onDone;
 
 			// Wait for the terminal to idle
 			this._logService.debug('RunInTerminalTool#Basic: Waiting for idle');
@@ -98,13 +107,36 @@ export class BasicExecuteStrategy implements ITerminalExecuteStrategy {
 			const endMarker = store.add(xterm.raw.registerMarker());
 
 			// Assemble final result
-			let result = xterm.getContentsAsText(startMarker, endMarker);
-			if (doneData && typeof doneData.exitCode === 'number' && doneData.exitCode > 0) {
-				result += `\n\nCommand exited with code ${doneData.exitCode}`;
+			let result: string | undefined;
+			if (finishedCommand) {
+				const commandOutput = finishedCommand?.getOutput();
+				if (commandOutput !== undefined) {
+					this._logService.debug('RunInTerminalTool#Basic: Fetched output via finished command');
+					result = commandOutput;
+				}
+			}
+			if (result === undefined) {
+				try {
+					result = xterm.getContentsAsText(startMarker, endMarker);
+					this._logService.debug('RunInTerminalTool#Basic: Fetched output via markers');
+				} catch {
+					this._logService.debug('RunInTerminalTool#Basic: Failed to fetch output via markers');
+					result = 'Failed to retrieve command output';
+				}
+			}
+
+			if (result.trim().length === 0) {
+				result = 'Command produced no output';
 			}
+
+			const exitCode = finishedCommand?.exitCode;
+			if (isNumber(exitCode) && exitCode > 0) {
+				result += `\n\nCommand exited with code ${exitCode}`;
+			}
+
 			return {
 				result,
-				exitCode: doneData?.exitCode,
+				exitCode,
 			};
 		} finally {
 			store.dispose();

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/noneExecuteStrategy.ts

@@ -46,8 +46,16 @@ export class NoneExecuteStrategy implements ITerminalExecuteStrategy {
 				throw new CancellationError();
 			}
 
+			// Record where the command started. If the marker gets disposed, re-created it where
+			// the cursor is. This can happen in prompts where they clear the line and rerender it
+			// like powerlevel10k's transient prompt
+			let startMarker = store.add(xterm.raw.registerMarker());
+			store.add(startMarker.onDispose(() => {
+				this._logService.debug(`RunInTerminalTool#Rich: Start marker was disposed, recreating`);
+				startMarker = xterm.raw.registerMarker();
+			}));
+
 			// Execute the command
-			const startMarker = store.add(xterm.raw.registerMarker());
 			this._logService.debug(`RunInTerminalTool#None: Executing command line \`${commandLine}\``);
 			this._instance.runCommand(commandLine, true);
 
@@ -60,8 +68,16 @@ export class NoneExecuteStrategy implements ITerminalExecuteStrategy {
 			const endMarker = store.add(xterm.raw.registerMarker());
 
 			// Assemble final result - exit code is not available without shell integration
+			let result: string;
+			try {
+				result = xterm.getContentsAsText(startMarker, endMarker);
+				this._logService.debug('RunInTerminalTool#None: Fetched output via markers');
+			} catch {
+				this._logService.debug('RunInTerminalTool#None: Failed to fetch output via markers');
+				result = 'Failed to retrieve command output';
+			}
 			return {
-				result: xterm.getContentsAsText(startMarker, endMarker),
+				result,
 				exitCode: undefined,
 			};
 		} finally {

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/richExecuteStrategy.ts

@@ -53,9 +53,17 @@ export class RichExecuteStrategy implements ITerminalExecuteStrategy {
 				}),
 			]);
 
+			// Record where the command started. If the marker gets disposed, re-created it where
+			// the cursor is. This can happen in prompts where they clear the line and rerender it
+			// like powerlevel10k's transient prompt
+			let startMarker = store.add(xterm.raw.registerMarker());
+			store.add(startMarker.onDispose(() => {
+				this._logService.debug(`RunInTerminalTool#Rich: Start marker was disposed, recreating`);
+				startMarker = xterm.raw.registerMarker();
+			}));
+
 			// Execute the command
 			this._logService.debug(`RunInTerminalTool#Rich: Executing command line \`${commandLine}\``);
-			const startMarker = store.add(xterm.raw.registerMarker());
 			this._instance.runCommand(commandLine, true);
 
 			this._logService.debug(`RunInTerminalTool#Rich: Waiting for done event`);
@@ -74,8 +82,13 @@ export class RichExecuteStrategy implements ITerminalExecuteStrategy {
 				}
 			}
 			if (result === undefined) {
-				this._logService.debug('RunInTerminalTool#Rich: Fetched output via markers');
-				result = xterm.getContentsAsText(startMarker, endMarker);
+				try {
+					result = xterm.getContentsAsText(startMarker, endMarker);
+					this._logService.debug('RunInTerminalTool#Rich: Fetched output via markers');
+				} catch {
+					this._logService.debug('RunInTerminalTool#Basic: Failed to fetch output via markers');
+					result = 'Failed to retrieve command output';
+				}
 			}
 
 			if (result.trim().length === 0) {