agents: fix spinner stuck after session completes

src/vs/platform/agentHost/node/agentHostStateManager.ts

@@ -237,16 +237,30 @@ export class AgentHostStateManager extends Disposable {
 	}
 
 	/**
-	 * Removes a session from in-memory state without emitting a notification.
+	 * Removes a session from in-memory state without emitting a
+	 * {@link NotificationType.SessionRemoved} notification.
 	 * Use {@link deleteSession} when the session is being permanently deleted
-	 * and clients need to be notified.
+	 * and clients need to be notified of its removal.
+	 *
+	 * Any pending summary change is flushed synchronously before the session is
+	 * torn down, so clients receive the final status (e.g. Idle after a turn
+	 * completes) even when the session is evicted before the scheduler fires.
+	 * A {@link NotificationType.SessionSummaryChanged} notification may therefore
+	 * be emitted as a side-effect of this call.
 	 */
 	removeSession(session: URI): void {
 		const state = this._sessionStates.get(session);
 		if (!state) {
 			return;
 		}
 
+		// Flush any pending summary notification before tearing down state so
+		// that the final status (e.g. Idle) reaches clients even if the session
+		// is evicted within the scheduler's debounce window.
+		if (this._dirtySummaries.has(session)) {
+			this._flushSummaryNotificationFor(session);
+		}
+
 		// Clean up active turn tracking
 		if (state.activeTurn) {
 			this._activeTurnToSession.delete(state.activeTurn.id);
@@ -271,6 +285,10 @@ export class AgentHostStateManager extends Disposable {
 	 */
 	deleteSession(session: URI): void {
 		const wasAnnounced = this._lastNotifiedSummaries.has(session);
+		// Drop any pending summary diff: the forthcoming SessionRemoved notification
+		// supersedes it and we don't want to emit spurious SessionSummaryChanged
+		// events just before the session disappears from the client's view.
+		this._dirtySummaries.delete(session);
 		this.removeSession(session);
 		if (wasAnnounced) {
 			this._onDidEmitNotification.fire({
@@ -385,33 +403,45 @@ export class AgentHostStateManager extends Disposable {
 
 	private _flushSummaryNotifications(): void {
 		for (const session of this._dirtySummaries) {
-			const state = this._sessionStates.get(session);
-			const lastNotified = this._lastNotifiedSummaries.get(session);
-			if (!state || !lastNotified || state.summary === lastNotified) {
-				continue;
-			}
-
-			const current = state.summary;
-			const changes: Partial<SessionSummary> = {};
-			if (current.title !== lastNotified.title) { changes.title = current.title; }
-			if (current.status !== lastNotified.status) { changes.status = current.status; }
-			if (current.activity !== lastNotified.activity) { changes.activity = current.activity; }
-			if (current.modifiedAt !== lastNotified.modifiedAt) { changes.modifiedAt = current.modifiedAt; }
-			if (current.project !== lastNotified.project) { changes.project = current.project; }
-			if (current.model !== lastNotified.model) { changes.model = current.model; }
-			if (current.workingDirectory !== lastNotified.workingDirectory) { changes.workingDirectory = current.workingDirectory; }
-			if (current.diffs !== lastNotified.diffs) { changes.diffs = current.diffs; }
-
-			this._lastNotifiedSummaries.set(session, current);
-
-			if (Object.keys(changes).length > 0) {
-				this._onDidEmitNotification.fire({
-					type: NotificationType.SessionSummaryChanged,
-					session,
-					changes,
-				});
-			}
+			this._flushSummaryNotificationFor(session);
 		}
 		this._dirtySummaries.clear();
 	}
+
+	/**
+	 * Emits a {@link NotificationType.SessionSummaryChanged} notification for
+	 * `session` if its current summary differs from the last one sent to
+	 * clients, then advances `_lastNotifiedSummaries` to the current summary.
+	 *
+	 * Does NOT remove `session` from `_dirtySummaries` — callers are
+	 * responsible for that bookkeeping.
+	 */
+	private _flushSummaryNotificationFor(session: string): void {
+		const state = this._sessionStates.get(session);
+		const lastNotified = this._lastNotifiedSummaries.get(session);
+		if (!state || !lastNotified || state.summary === lastNotified) {
+			return;
+		}
+
+		const current = state.summary;
+		const changes: Partial<SessionSummary> = {};
+		if (current.title !== lastNotified.title) { changes.title = current.title; }
+		if (current.status !== lastNotified.status) { changes.status = current.status; }
+		if (current.activity !== lastNotified.activity) { changes.activity = current.activity; }
+		if (current.modifiedAt !== lastNotified.modifiedAt) { changes.modifiedAt = current.modifiedAt; }
+		if (current.project !== lastNotified.project) { changes.project = current.project; }
+		if (current.model !== lastNotified.model) { changes.model = current.model; }
+		if (current.workingDirectory !== lastNotified.workingDirectory) { changes.workingDirectory = current.workingDirectory; }
+		if (current.diffs !== lastNotified.diffs) { changes.diffs = current.diffs; }
+
+		this._lastNotifiedSummaries.set(session, current);
+
+		if (Object.keys(changes).length > 0) {
+			this._onDidEmitNotification.fire({
+				type: NotificationType.SessionSummaryChanged,
+				session,
+				changes,
+			});
+		}
+	}
 }

src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts

@@ -382,6 +382,51 @@ suite('AgentHostStateManager', () => {
 			assert.strictEqual(changed.length, 0, 'should not emit for deleted sessions');
 		});
 	});
+
+	test('removeSession flushes pending status=Idle notification before eviction', () => {
+		// Regression: when _maybeEvictIdleSession calls removeSession within the
+		// 100 ms scheduler window after a turn completes, the client must still
+		// receive a SessionSummaryChanged with status=Idle so the spinner clears.
+		//
+		// The key precondition is that _lastNotifiedSummaries already has
+		// status=InProgress (the scheduler must have fired after TurnStarted so
+		// the client knows the session is busy). Then TurnComplete flips the
+		// summary back to Idle and schedules another flush. If removeSession
+		// races with that 100 ms window the flush must happen synchronously.
+		return runWithFakedTimers({ useFakeTimers: true }, async () => {
+			manager.createSession(makeSessionSummary());
+			manager.dispatchServerAction({ type: ActionType.SessionReady, session: sessionUri });
+
+			// Start a turn → status becomes InProgress.
+			manager.dispatchServerAction({
+				type: ActionType.SessionTurnStarted,
+				session: sessionUri,
+				turnId: 'turn-1',
+				userMessage: { text: 'hello' },
+			});
+
+			// Let the scheduler fire so _lastNotifiedSummaries now has status=InProgress.
+			await new Promise(r => setTimeout(r, 150));
+
+			const notifications: INotification[] = [];
+			disposables.add(manager.onDidEmitNotification(n => notifications.push(n)));
+
+			// Turn completes — status flips back to Idle. This schedules a summary
+			// flush 100 ms later but we will call removeSession before it fires.
+			manager.dispatchServerAction({
+				type: ActionType.SessionTurnComplete,
+				session: sessionUri,
+				turnId: 'turn-1',
+			});
+
+			// Simulate eviction within the 100 ms debounce window.
+			manager.removeSession(sessionUri);
+
+			const changed = notifications.filter(n => n.type === NotificationType.SessionSummaryChanged) as SessionSummaryChangedNotification[];
+			assert.strictEqual(changed.length, 1, 'should emit SessionSummaryChanged synchronously in removeSession');
+			assert.strictEqual(changed[0].changes.status, SessionStatus.Idle, 'status should be Idle so the spinner clears');
+		});
+	});
 });
 
 suite('Subagent URI helpers', () => {