sessions: fix: preserve side-question ownership and promotion focus

Reserve source-scoped presentation ownership before side-chat creation. Cancel superseded fallback navigation and promotion, keep older sends recoverable without taking focus, and focus the exact promoted full-chat destination. Release pending ownership and cover creation, close, promotion, selection-input, and multi-group focus races.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f48399f6-8524-4cb1-b187-d7f3eb899620

Author: ulugbekna

src/vs/sessions/contrib/chat/browser/btwSlashCommand.contribution.ts

@@ -81,17 +81,16 @@ export class BtwSlashCommandContribution extends Disposable implements IWorkbenc
 			}
 			const selection = captureSideChatSelection(chatWidgetService.getWidgetBySessionResource(chat.resource));
 
-			let sideChat;
+			let prepared;
 			try {
-				sideChat = await sessionsManagementService.createSideChatInSession(session, chat.resource, sourceTurn.id, selection);
+				prepared = await sideChatOrchestrationService.createAndPresent(session, chat, sourceTurn.id, remainder, selection);
 			} catch (err) {
 				logService.error('[btw] Failed to create side chat', err);
 				notificationService.error(localize('btw.createFailed', "The side chat could not be created."));
 				return;
 			}
 
 			try {
-				const prepared = await sideChatOrchestrationService.prepare(session, chat, sideChat, remainder);
 				await prepared.send({ query: remainder, attachedContext: options?.attachedContext });
 			} catch (err) {
 				logService.error('[btw] Failed to send side chat request', err);

src/vs/sessions/contrib/chat/browser/responseSelectionSideChatController.ts

@@ -344,7 +344,7 @@ export class ResponseSelectionSideChatController extends Disposable {
 	 * it — outside interactions like Escape or selection invalidation must not
 	 * race the in-flight create/open/send.
 	 */
-	private _dismiss(force = false): void {
+	private _dismiss(force = false, preserveFocus = false): void {
 		if (!force && this._input.isBusy) {
 			return;
 		}
@@ -359,7 +359,7 @@ export class ResponseSelectionSideChatController extends Disposable {
 		this._input.setBusy(false);
 		this._input.hide();
 		this._input.clearInput();
-		if (hadFocus) {
+		if (hadFocus && !preserveFocus) {
 			// Hiding the focused input would otherwise leave focus stranded on
 			// the body; return it to the transcript it was invoked from.
 			this._widget.focusResponseItem(true);
@@ -391,16 +391,16 @@ export class ResponseSelectionSideChatController extends Disposable {
 	}
 
 	private async _createAndSendSideChat(session: ISession, sourceChat: IChat, turnId: string, selectedText: string, query: string, generation: number): Promise<void> {
-		let presentedTransiently = false;
+		let presentation: SideChatPresentation | undefined;
 		try {
 			const prepared = await this._sideChatOrchestrationService.createAndPresent(session, sourceChat, turnId, query, { text: selectedText });
-			presentedTransiently = prepared.presentation === SideChatPresentation.Transient;
-			if (presentedTransiently && this._generation === generation) {
-				this._dismissAfterTransientPresentation();
+			presentation = prepared.presentation;
+			if (presentation !== SideChatPresentation.Full && this._generation === generation) {
+				this._dismissAfterBackgroundPresentation(presentation === SideChatPresentation.Superseded);
 			}
 
 			await prepared.send({ query });
-			if (!presentedTransiently && this._generation === generation) {
+			if (presentation === SideChatPresentation.Full && this._generation === generation) {
 				this._input.setBusy(false);
 				this._dismiss();
 			}
@@ -409,7 +409,7 @@ export class ResponseSelectionSideChatController extends Disposable {
 			if (this._generation !== generation) {
 				return;
 			}
-			if (presentedTransiently) {
+			if (presentation === SideChatPresentation.Transient || presentation === SideChatPresentation.Superseded) {
 				this._notificationService.error(localize('sessions.selectionSideChat.sendFailed', "The side question could not be answered."));
 				return;
 			}
@@ -422,10 +422,10 @@ export class ResponseSelectionSideChatController extends Disposable {
 		}
 	}
 
-	private _dismissAfterTransientPresentation(): void {
+	private _dismissAfterBackgroundPresentation(preserveFocus: boolean): void {
 		dom.getWindow(this._widget.domNode).getSelection()?.removeAllRanges();
 		this._input.setBusy(false);
-		this._dismiss();
+		this._dismiss(false, preserveFocus);
 		this._autoScrollHold.clear();
 	}
 }

src/vs/sessions/contrib/chat/browser/sideChatOrchestration.ts

@@ -10,13 +10,14 @@ import { ISessionsPartService } from '../../../services/sessions/browser/session
 import { ISessionsService } from '../../../services/sessions/browser/sessionsService.js';
 import { IChat, ISession, ISideChatSelection } from '../../../services/sessions/common/session.js';
 import { ISendRequestOptions, ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js';
-import { ITransientSideChatService } from './transientSideChatService.js';
+import { ITransientSideChatService, TransientSideChatPresentationResult } from './transientSideChatService.js';
 
 export const ISideChatOrchestrationService = createDecorator<ISideChatOrchestrationService>('sideChatOrchestrationService');
 
 export const enum SideChatPresentation {
 	Full = 'full',
 	Transient = 'transient',
+	Superseded = 'superseded',
 }
 
 export interface IPreparedSideChat {
@@ -27,7 +28,6 @@ export interface IPreparedSideChat {
 
 export interface ISideChatOrchestrationService {
 	readonly _serviceBrand: undefined;
-	prepare(session: ISession, sourceChat: IChat, sideChat: IChat, question: string): Promise<IPreparedSideChat>;
 	createAndPresent(session: ISession, sourceChat: IChat, turnId: string, question: string, selection?: ISideChatSelection): Promise<IPreparedSideChat>;
 }
 
@@ -42,34 +42,41 @@ export class SideChatOrchestrationService implements ISideChatOrchestrationServi
 	) { }
 
 	async createAndPresent(session: ISession, sourceChat: IChat, turnId: string, question: string, selection?: ISideChatSelection): Promise<IPreparedSideChat> {
-		const sideChat = await this.sessionsManagementService.createSideChatInSession(session, sourceChat.resource, turnId, selection);
-		return this.prepare(session, sourceChat, sideChat, question);
-	}
-
-	async prepare(session: ISession, sourceChat: IChat, sideChat: IChat, question: string): Promise<IPreparedSideChat> {
-		const presentation = await this.transientSideChatService.show(session, sourceChat, sideChat, question)
-			? SideChatPresentation.Transient
-			: SideChatPresentation.Full;
-		if (presentation === SideChatPresentation.Full) {
-			await this.sessionsService.openChat(session, sideChat.resource);
-			const activeSession = this.sessionsService.activeSession.get();
-			if (activeSession?.sessionId !== session.sessionId || !isEqual(activeSession.activeChat.get().resource, sideChat.resource)) {
-				throw new Error(`Side chat '${sideChat.resource.toString()}' did not open`);
+		const pendingPresentation = this.transientSideChatService.beginPresentation(sourceChat);
+		try {
+			const sideChat = await this.sessionsManagementService.createSideChatInSession(session, sourceChat.resource, turnId, selection);
+			const result = await pendingPresentation.show(session, sideChat, question);
+			let presentation = pendingPresentation.token.isCancellationRequested || result === TransientSideChatPresentationResult.Superseded
+				? SideChatPresentation.Superseded
+				: result === TransientSideChatPresentationResult.Shown ? SideChatPresentation.Transient : SideChatPresentation.Full;
+			if (presentation === SideChatPresentation.Full) {
+				await this.sessionsService.openChat(session, sideChat.resource, { token: pendingPresentation.token });
+				if (pendingPresentation.token.isCancellationRequested) {
+					await this.sessionsService.closeChat(session, sideChat, { skipHistory: true });
+					presentation = SideChatPresentation.Superseded;
+				} else {
+					const activeSession = this.sessionsService.activeSession.get();
+					if (activeSession?.sessionId !== session.sessionId || !isEqual(activeSession.activeChat.get().resource, sideChat.resource)) {
+						throw new Error(`Side chat '${sideChat.resource.toString()}' did not open`);
+					}
+					this.sessionsPartService.getSessionView(session.sessionId)?.splitChatToSide(sideChat.resource);
+				}
 			}
-			this.sessionsPartService.getSessionView(session.sessionId)?.splitChatToSide(sideChat.resource);
+			return {
+				sideChat,
+				presentation,
+				send: requestOptions => this._send(session, sideChat, presentation, requestOptions),
+			};
+		} finally {
+			pendingPresentation.dispose();
 		}
-		return {
-			sideChat,
-			presentation,
-			send: requestOptions => this._send(session, sideChat, presentation, requestOptions),
-		};
 	}
 
 	private async _send(session: ISession, sideChat: IChat, presentation: SideChatPresentation, requestOptions: ISendRequestOptions): Promise<ChatSideChatSendResult> {
 		try {
 			await this.sessionsManagementService.sendRequest(session, sideChat, {
 				...requestOptions,
-				preserveActiveChat: presentation === SideChatPresentation.Transient,
+				preserveActiveChat: presentation !== SideChatPresentation.Full,
 			});
 			return { kind: ChatSideChatSendResultKind.Sent };
 		} catch (error) {

src/vs/sessions/contrib/chat/browser/transientSideChatService.ts

@@ -3,7 +3,8 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { Disposable, IDisposable, toDisposable } from '../../../../base/common/lifecycle.js';
+import { CancellationToken, CancellationTokenSource } from '../../../../base/common/cancellation.js';
+import { Disposable, DisposableResourceMap, IDisposable, toDisposable } from '../../../../base/common/lifecycle.js';
 import { autorun, IObservable, IReader, observableSignalFromEvent, observableValue } from '../../../../base/common/observable.js';
 import { isEqual } from '../../../../base/common/resources.js';
 import { ResourceMap } from '../../../../base/common/map.js';
@@ -12,6 +13,7 @@ import { IConfigurationService } from '../../../../platform/configuration/common
 import { createDecorator } from '../../../../platform/instantiation/common/instantiation.js';
 import { IActiveSession, ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js';
 import { ChatInteractivity, IChat, ISession } from '../../../services/sessions/common/session.js';
+import { ISessionsPartService } from '../../../services/sessions/browser/sessionsPartService.js';
 import { ISessionsService } from '../../../services/sessions/browser/sessionsService.js';
 
 export const AGENT_SESSIONS_TRANSIENT_SIDE_CHAT_SETTING = 'chat.agentSessions.transientSideChat';
@@ -34,11 +36,22 @@ export interface IResolvedTransientSideChatState extends ITransientSideChatState
 
 export const ITransientSideChatService = createDecorator<ITransientSideChatService>('transientSideChatService');
 
+export const enum TransientSideChatPresentationResult {
+	Shown = 'shown',
+	Unavailable = 'unavailable',
+	Superseded = 'superseded',
+}
+
+export interface ITransientSideChatPresentation extends IDisposable {
+	readonly token: CancellationToken;
+	show(session: ISession, sideChat: IChat, question: string): Promise<TransientSideChatPresentationResult>;
+}
+
 export interface ITransientSideChatService {
 	readonly _serviceBrand: undefined;
 	readonly states: IObservable<readonly ITransientSideChatState[]>;
 	registerHost(sourceChat: URI): IDisposable;
-	show(session: ISession, sourceChat: IChat, sideChat: IChat, question: string): Promise<boolean>;
+	beginPresentation(sourceChat: IChat): ITransientSideChatPresentation;
 	resolveState(state: ITransientSideChatState, reader?: IReader): IResolvedTransientSideChatState | undefined;
 	promote(sourceChat: URI): Promise<boolean>;
 	dismiss(sourceChat: URI): void;
@@ -53,14 +66,14 @@ export class TransientSideChatService extends Disposable implements ITransientSi
 	readonly states: IObservable<readonly ITransientSideChatState[]> = this._states;
 
 	private readonly _hosts = new ResourceMap<Set<object>>();
-	private readonly _presentationIds = new ResourceMap<number>();
+	private readonly _presentations = this._register(new DisposableResourceMap<CancellationTokenSource>());
 	private readonly _catalogChanged: IObservable<void>;
-	private _presentationIdPool = 0;
 
 	constructor(
 		@ISessionsService private readonly sessionsService: ISessionsService,
 		@ISessionsManagementService private readonly sessionsManagementService: ISessionsManagementService,
 		@IConfigurationService private readonly configurationService: IConfigurationService,
+		@ISessionsPartService private readonly sessionsPartService: ISessionsPartService,
 	) {
 		super();
 		this._catalogChanged = observableSignalFromEvent(this, sessionsManagementService.onDidChangeSessions);
@@ -86,7 +99,7 @@ export class TransientSideChatService extends Disposable implements ITransientSi
 		}));
 		this._register(configurationService.onDidChangeConfiguration(event => {
 			if (event.affectsConfiguration(AGENT_SESSIONS_TRANSIENT_SIDE_CHAT_SETTING) && !this._isEnabled()) {
-				this._presentationIds.clear();
+				this._cancelPresentations();
 				this._states.set([], undefined);
 			}
 		}));
@@ -117,27 +130,36 @@ export class TransientSideChatService extends Disposable implements ITransientSi
 		});
 	}
 
-	async show(session: ISession, sourceChat: IChat, sideChat: IChat, question: string): Promise<boolean> {
-		if (!this._isEnabled()) {
-			return false;
-		}
-		if (!this._hasHost(sourceChat.resource)) {
-			return false;
-		}
-		if (sourceChat.interactivity.get() !== ChatInteractivity.Full) {
-			return false;
+	beginPresentation(sourceChat: IChat): ITransientSideChatPresentation {
+		this._presentations.get(sourceChat.resource)?.cancel();
+		const source = new CancellationTokenSource();
+		this._presentations.set(sourceChat.resource, source);
+		const token = source.token;
+		return Object.assign(toDisposable(() => {
+			source.cancel();
+			if (this._presentations.get(sourceChat.resource) === source) {
+				this._presentations.deleteAndDispose(sourceChat.resource);
+			}
+		}), {
+			token,
+			show: (session: ISession, sideChat: IChat, question: string) => this._show(session, sourceChat, sideChat, question, token),
+		});
+	}
+
+	private async _show(session: ISession, sourceChat: IChat, sideChat: IChat, question: string, token: CancellationToken): Promise<TransientSideChatPresentationResult> {
+		if (!token.isCancellationRequested && (!this._isEnabled()
+			|| !this._hasHost(sourceChat.resource)
+			|| sourceChat.interactivity.get() !== ChatInteractivity.Full)) {
+			return TransientSideChatPresentationResult.Unavailable;
 		}
 
-		const presentationId = ++this._presentationIdPool;
-		this._presentationIds.set(sourceChat.resource, presentationId);
+		// Superseded creations remain recoverable without taking over visible navigation.
 		await this.sessionsService.closeChat(session, sideChat, { skipHistory: true });
-		if (this._presentationIds.get(sourceChat.resource) !== presentationId
-			|| !this._isEnabled()
-			|| !this._hasHost(sourceChat.resource)) {
-			if (this._presentationIds.get(sourceChat.resource) === presentationId) {
-				this._presentationIds.delete(sourceChat.resource);
-			}
-			return false;
+		if (token.isCancellationRequested) {
+			return TransientSideChatPresentationResult.Superseded;
+		}
+		if (!this._isEnabled() || !this._hasHost(sourceChat.resource)) {
+			return TransientSideChatPresentationResult.Unavailable;
 		}
 		const liveSession = this.sessionsManagementService.getSession(session.resource);
 		const liveChats = liveSession?.chats.get();
@@ -147,8 +169,7 @@ export class TransientSideChatService extends Disposable implements ITransientSi
 			|| !liveSourceChat
 			|| liveSourceChat.interactivity.get() !== ChatInteractivity.Full
 			|| !liveChats.some(chat => isEqual(chat.resource, sideChat.resource))) {
-			this._presentationIds.delete(sourceChat.resource);
-			return false;
+			return TransientSideChatPresentationResult.Unavailable;
 		}
 		this._setState({
 			sessionResource: liveSession.resource,
@@ -159,7 +180,7 @@ export class TransientSideChatService extends Disposable implements ITransientSi
 			failed: false,
 			replacedExisting: this._getState(sourceChat.resource) !== undefined,
 		});
-		return true;
+		return TransientSideChatPresentationResult.Shown;
 	}
 
 	private _isEnabled(): boolean {
@@ -193,27 +214,34 @@ export class TransientSideChatService extends Disposable implements ITransientSi
 			throw new Error('The transient side chat is no longer available');
 		}
 
-		this._setState({ ...state, promoting: true });
+		const presentation = this.beginPresentation(resolved.sourceChat);
 		try {
-			await this.sessionsService.openChat(resolved.session, state.sideChatResource);
+			this._setState({ ...state, promoting: true });
+			await this.sessionsService.openChat(resolved.session, state.sideChatResource, { token: presentation.token });
+			if (presentation.token.isCancellationRequested) {
+				return false;
+			}
 			if (!this._isActiveChat(resolved.session, state.sideChatResource)) {
 				throw new Error('The transient side chat did not open');
 			}
+			this.sessionsPartService.focusSession(this.sessionsService.activeSession.get());
 			const current = this._getState(sourceChat);
 			if (current && isEqual(current.sideChatResource, state.sideChatResource) && current.promoting) {
 				this._remove(sourceChat);
 			}
 			return true;
-		} catch (error) {
+		} finally {
 			const current = this._getState(sourceChat);
 			if (current && isEqual(current.sideChatResource, state.sideChatResource) && current.promoting) {
 				this._setState({ ...current, promoting: false });
 			}
-			throw error;
+			presentation.dispose();
 		}
 	}
 
 	dismiss(sourceChat: URI): void {
+		this._presentations.get(sourceChat)?.cancel();
+		this._presentations.deleteAndDispose(sourceChat);
 		this._remove(sourceChat);
 	}
 
@@ -259,9 +287,7 @@ export class TransientSideChatService extends Disposable implements ITransientSi
 		const states = this._states.get();
 		const next: ITransientSideChatState[] = [];
 		for (const state of states) {
-			if (predicate(state)) {
-				this._presentationIds.delete(state.sourceChatResource);
-			} else {
+			if (!predicate(state)) {
 				next.push(state);
 			}
 		}
@@ -279,4 +305,16 @@ export class TransientSideChatService extends Disposable implements ITransientSi
 		return activeSession?.sessionId === session.sessionId && isEqual(activeSession.activeChat.get().resource, chatResource);
 	}
 
+	private _cancelPresentations(): void {
+		for (const source of this._presentations.values()) {
+			source.cancel();
+		}
+		this._presentations.clearAndDisposeAll();
+	}
+
+	override dispose(): void {
+		this._cancelPresentations();
+		super.dispose();
+	}
+
 }

src/vs/sessions/contrib/chat/browser/transientSideChatWidget.ts

@@ -701,7 +701,6 @@ export class TransientSideChatWidget extends Disposable {
 		}
 		try {
 			if (await this._transientSideChatService.promote(source.chat.resource)) {
-				this._mainWidget.focusInput();
 				announceStatus(localize('transientSideChat.promotedStatus', "Opened side question as a full chat"));
 			}
 		} catch (error) {