terminalChat: fix OutputMonitor dispose race that stalls run_in_terminal (#313106)

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/monitoring/outputMonitor.ts

@@ -102,6 +102,45 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
 	private _command = '';
 	private _invocationContext: IToolInvocationContext | undefined;
 	private _currentMonitoringCts: CancellationTokenSource | undefined;
+	/**
+	 * Tracks whether onDidFinishCommand has fired so the event is delivered at
+	 * most once. The event must fire synchronously during dispose so consumers
+	 * awaiting `Event.toPromise(onDidFinishCommand)` are unblocked before the
+	 * underlying emitter is torn down by super.dispose().
+	 */
+	private _didFinish = false;
+
+	private _fireFinishedOnce(): void {
+		if (this._didFinish) {
+			return;
+		}
+		this._didFinish = true;
+		this._onDidFinishCommand.fire();
+	}
+
+	override dispose(): void {
+		// Deliver onDidFinishCommand to consumers BEFORE super.dispose() tears
+		// down the emitter. Field-initialized disposables (including
+		// _onDidFinishCommand) are registered before any disposable added in
+		// the constructor body and are disposed first by DisposableStore in
+		// insertion order. Without this override, consumers awaiting
+		// `Event.toPromise(onDidFinishCommand)` would race with emitter
+		// teardown and hang when dispose lands while _startMonitoring is still
+		// in flight.
+		if (!this._didFinish) {
+			// Synthesize a Cancelled pollingResult so consumers that read
+			// `monitor.pollingResult` after awaiting onDidFinishCommand always
+			// see a defined value with the output collected so far.
+			this._pollingResult ??= {
+				state: OutputMonitorState.Cancelled,
+				output: this._execution.getOutput(),
+				pollDurationMs: 0,
+				resources: undefined,
+			};
+		}
+		this._fireFinishedOnce();
+		super.dispose();
+	}
 
 	constructor(
 		private readonly _execution: IExecution,
@@ -231,7 +270,10 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
 			};
 			// Clean up idle input listener if still active
 			this._userInputListener.clear();
-			this._onDidFinishCommand.fire();
+			// Fire at most once. If dispose() already fired the event synchronously
+			// (e.g. the monitor was torn down before this async loop reached its
+			// finally), skip firing on a potentially disposed emitter.
+			this._fireFinishedOnce();
 		}
 	}
 

src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/outputMonitor.test.ts

@@ -500,6 +500,28 @@ suite('OutputMonitor', () => {
 				monitor.dispose();
 			});
 		});
+
+		test('disposing while monitoring is in-flight still resolves onDidFinishCommand', async () => {
+			// Regression: if dispose() races the async _startMonitoring loop, the loop's
+			// finally block fires onDidFinishCommand AFTER super.dispose() has already
+			// torn down the emitter. Consumers awaiting Event.toPromise(onDidFinishCommand)
+			// would never resolve and the agent would hang on the run_in_terminal call.
+			//
+			// Fix: dispose() must fire onDidFinishCommand synchronously, before the
+			// emitter is disposed. It must also surface a Cancelled pollingResult so
+			// consumers that read monitor.pollingResult after awaiting the event see a
+			// defined value rather than undefined.
+			return runWithFakedTimers({}, async () => {
+				monitor = store.add(instantiationService.createInstance(OutputMonitor, execution, undefined, createTestContext('1'), cts.token, 'test command'));
+				const finished = Event.toPromise(monitor.onDidFinishCommand);
+				// Dispose immediately, before the deferred _startMonitoring even starts.
+				monitor.dispose();
+				// Must resolve — would hang prior to the synchronous-fire-on-dispose fix.
+				await finished;
+				assert.ok(monitor.pollingResult, 'pollingResult should be defined after dispose-induced finish');
+				assert.strictEqual(monitor.pollingResult!.state, OutputMonitorState.Cancelled);
+			});
+		});
 	});
 
 	suite('detectsGenericPressAnyKeyPattern', () => {