[cherry-pick] sessions: navigate between chats in back/forward history

src/vs/sessions/services/sessions/browser/sessionNavigation.ts

@@ -15,10 +15,12 @@ import { ISessionsChangeEvent, ISessionsManagementService } from '../common/sess
 const MAX_HISTORY_SIZE = 50;
 
 /**
- * A navigation history entry. Stores the session resource URI.
+ * A navigation history entry. Stores the session and optionally the chat
+ * resource URI so that back/forward also restores the active chat.
  */
 interface INavigationEntry {
 	readonly sessionResource: URI;
+	readonly chatResource: URI | undefined;
 }
 
 /**
@@ -67,18 +69,21 @@ export class SessionsNavigation extends Disposable {
 		this._canGoBackCtx = CanGoBackContext.bindTo(contextKeyService);
 		this._canGoForwardCtx = CanGoForwardContext.bindTo(contextKeyService);
 
-		// Track active session changes to record history entries.
+		// Track active session/chat changes to record history entries.
 		// Skip undefined (new-session view) and Untitled sessions — only record
-		// sessions that have been saved/submitted.
-		// NOTE: activeSession.read(reader) must always be called first to keep the
-		// subscription alive even during navigation, otherwise the autorun loses its
-		// dependency and stops firing after goBack/goForward completes.
+		// sessions that have been saved/submitted. Also tracks active chat changes
+		// within a session so that switching chats is navigable.
+		// NOTE: all observables must always be read before the _navigating guard to
+		// keep subscriptions alive during navigation.
 		this._register(autorun(reader => {
 			const activeSession = this._sessionsManagementService.activeSession.read(reader);
+			const activeChat = activeSession?.activeChat.read(reader);
+			const sessionStatus = activeSession?.status.read(reader);
+			const chatStatus = activeChat?.status.read(reader);
 			if (this._navigating) {
 				return;
 			}
-			if (!activeSession || activeSession.status.read(reader) === SessionStatus.Untitled) {
+			if (!activeSession || sessionStatus === SessionStatus.Untitled) {
 				// User navigated to new-session view: if we have history, remember we're
 				// beyond the stack so Back can return to the last real session.
 				if (this._history.length > 0) {
@@ -87,8 +92,13 @@ export class SessionsNavigation extends Disposable {
 				return;
 			}
 
+			// Skip untitled chats (new-chat-in-session that hasn't been submitted)
+			const chatResource = activeChat && chatStatus !== SessionStatus.Untitled
+				? activeChat.resource
+				: undefined;
+
 			this._beyondHistory.set(false, undefined);
-			this._pushEntry({ sessionResource: activeSession.resource });
+			this._pushEntry({ sessionResource: activeSession.resource, chatResource });
 		}));
 
 		// Sync context keys with observables
@@ -159,7 +169,7 @@ export class SessionsNavigation extends Disposable {
 		this._currentIndex.set(this._history.length - 1, undefined);
 		this._historySize.set(this._history.length, undefined);
 
-		this._logService.trace(`[SessionNavigation] pushed entry idx=${this._history.length - 1} uri=${entry.sessionResource.toString()} historySize=${this._history.length}`);
+		this._logService.trace(`[SessionNavigation] pushed entry idx=${this._history.length - 1} session=${entry.sessionResource.toString()} chat=${entry.chatResource?.toString()} historySize=${this._history.length}`);
 	}
 
 	private async _navigateTo(targetIdx: number): Promise<void> {
@@ -168,15 +178,24 @@ export class SessionsNavigation extends Disposable {
 			return;
 		}
 
-		this._logService.trace(`[SessionNavigation] navigating to idx=${targetIdx} uri=${entry.sessionResource.toString()}`);
+		this._logService.trace(`[SessionNavigation] navigating to idx=${targetIdx} session=${entry.sessionResource.toString()} chat=${entry.chatResource?.toString()}`);
 
 		this._navigating = true;
 		try {
 			this._currentIndex.set(targetIdx, undefined);
 
 			const session = this._sessionsManagementService.getSession(entry.sessionResource);
 			if (session) {
-				await this._sessionsManagementService.openSession(entry.sessionResource);
+				if (entry.chatResource) {
+					const chatExists = session.chats.get().some(c => c.resource.toString() === entry.chatResource!.toString());
+					if (chatExists) {
+						await this._sessionsManagementService.openChat(session, entry.chatResource);
+					} else {
+						await this._sessionsManagementService.openSession(entry.sessionResource);
+					}
+				} else {
+					await this._sessionsManagementService.openSession(entry.sessionResource);
+				}
 			} else {
 				// Session no longer exists, remove from history
 				this._history.splice(targetIdx, 1);
@@ -191,7 +210,12 @@ export class SessionsNavigation extends Disposable {
 	}
 
 	private _isSameEntry(a: INavigationEntry, b: INavigationEntry): boolean {
-		return a.sessionResource.toString() === b.sessionResource.toString();
+		if (a.sessionResource.toString() !== b.sessionResource.toString()) {
+			return false;
+		}
+		const aChatStr = a.chatResource?.toString();
+		const bChatStr = b.chatResource?.toString();
+		return aChatStr === bChatStr;
 	}
 
 	private _removeEntriesMatching(predicate: (uri: URI) => boolean): void {

src/vs/sessions/services/sessions/test/browser/sessionNavigation.test.ts

@@ -22,7 +22,7 @@ const stubChat: IChat = {
 	createdAt: new Date(),
 	title: constObservable('Chat'),
 	updatedAt: constObservable(new Date()),
-	status: constObservable(0),
+	status: constObservable(SessionStatus.Completed),
 	changesets: constObservable([]),
 	changes: constObservable([]),
 	modelId: constObservable(undefined),
@@ -33,7 +33,26 @@ const stubChat: IChat = {
 	lastTurnEnd: constObservable(undefined),
 };
 
-function stubSession(id: string, status: SessionStatus = SessionStatus.Completed): ISession {
+function stubChatWithId(id: string, status: SessionStatus = SessionStatus.Completed): IChat {
+	return {
+		resource: URI.parse(`test:///chat-${id}`),
+		createdAt: new Date(),
+		title: constObservable(`Chat ${id}`),
+		updatedAt: constObservable(new Date()),
+		status: constObservable(status),
+		changesets: constObservable([]),
+		changes: constObservable([]),
+		modelId: constObservable(undefined),
+		mode: constObservable(undefined),
+		isArchived: constObservable(false),
+		isRead: constObservable(true),
+		description: constObservable(undefined),
+		lastTurnEnd: constObservable(undefined),
+	};
+}
+
+function stubSession(id: string, status: SessionStatus = SessionStatus.Completed, chats?: IChat[]): ISession {
+	const sessionChats = chats ?? [stubChat];
 	return {
 		sessionId: id,
 		resource: URI.parse(`test:///${id}`),
@@ -55,9 +74,9 @@ function stubSession(id: string, status: SessionStatus = SessionStatus.Completed
 		description: constObservable(undefined),
 		lastTurnEnd: constObservable(undefined),
 		gitHubInfo: constObservable(undefined),
-		chats: constObservable([stubChat]),
-		mainChat: stubChat,
-		capabilities: { supportsMultipleChats: false },
+		chats: constObservable(sessionChats),
+		mainChat: sessionChats[0],
+		capabilities: { supportsMultipleChats: chats !== undefined && chats.length > 1 },
 	};
 }
 
@@ -71,23 +90,33 @@ class MockSessionStore implements ISessionsManagementService {
 
 	private readonly _sessions = new Map<string, ISession>();
 	private _openedResource: URI | undefined;
+	private _openedChatResource: URI | undefined;
 	private _openedNewSession = false;
 
 	get lastOpenedResource(): URI | undefined { return this._openedResource; }
+	get lastOpenedChatResource(): URI | undefined { return this._openedChatResource; }
 	get lastOpenedNewSession(): boolean { return this._openedNewSession; }
 
-	setActiveSession(session: ISession | undefined): void {
+	setActiveSession(session: ISession | undefined, chat?: IChat): void {
 		if (session) {
+			const activeChat = chat ?? session.chats.get()[0] ?? stubChat;
 			const active: IActiveSession = {
 				...session,
-				activeChat: constObservable(stubChat),
+				activeChat: observableValue<IChat>(`test.activeChat-${session.sessionId}`, activeChat),
 			};
 			this.activeSession.set(active, undefined);
 		} else {
 			this.activeSession.set(undefined, undefined);
 		}
 	}
 
+	setActiveChat(chat: IChat): void {
+		const active = this.activeSession.get();
+		if (active) {
+			(active.activeChat as ReturnType<typeof observableValue<IChat>>).set(chat, undefined);
+		}
+	}
+
 	addSession(session: ISession): void {
 		this._sessions.set(session.resource.toString(), session);
 	}
@@ -102,6 +131,7 @@ class MockSessionStore implements ISessionsManagementService {
 
 	async openSession(sessionResource: URI): Promise<void> {
 		this._openedResource = sessionResource;
+		this._openedChatResource = undefined;
 		this._openedNewSession = false;
 		const session = this._sessions.get(sessionResource.toString());
 		if (session) {
@@ -112,10 +142,19 @@ class MockSessionStore implements ISessionsManagementService {
 	openNewSessionView(): void {
 		this._openedNewSession = true;
 		this._openedResource = undefined;
+		this._openedChatResource = undefined;
 		this.setActiveSession(undefined);
 	}
 
-	openChat(_session: ISession, _chatUri: URI): Promise<void> { throw new Error('not implemented'); }
+	async openChat(session: ISession, chatUri: URI): Promise<void> {
+		this._openedResource = session.resource;
+		this._openedChatResource = chatUri;
+		this._openedNewSession = false;
+		const chat = session.chats.get().find(c => c.resource.toString() === chatUri.toString());
+		if (chat) {
+			this.setActiveSession(session, chat);
+		}
+	}
 	restoreLastActiveSession(): Promise<void> { throw new Error('not implemented'); }
 	createNewSession(_providerId: string, _workspaceUri: URI, _sessionTypeId?: string): ISession { throw new Error('not implemented'); }
 	unsetNewSession(): void { throw new Error('not implemented'); }
@@ -357,4 +396,97 @@ suite('SessionsNavigation', () => {
 		store.setActiveSession(undefined); // go to new-session view again
 		assert.strictEqual(canGoBack(), true, 'back enabled after second new-session view');
 	});
+
+	test('switching chats within a session is recorded in history', () => {
+		const chatA = stubChatWithId('a');
+		const chatB = stubChatWithId('b');
+		const s1 = stubSession('s1', SessionStatus.Completed, [chatA, chatB]);
+		store.addSession(s1);
+
+		store.setActiveSession(s1, chatA);
+		assert.strictEqual(canGoBack(), false);
+
+		// Switch to chat B within the same session
+		store.setActiveChat(chatB);
+		assert.strictEqual(canGoBack(), true, 'back enabled after switching chat within session');
+	});
+
+	test('goBack restores previous chat within a session', async () => {
+		const chatA = stubChatWithId('a');
+		const chatB = stubChatWithId('b');
+		const s1 = stubSession('s1', SessionStatus.Completed, [chatA, chatB]);
+		store.addSession(s1);
+
+		store.setActiveSession(s1, chatA);
+		store.setActiveChat(chatB);
+
+		await nav.goBack();
+		assert.strictEqual(store.lastOpenedChatResource?.toString(), chatA.resource.toString());
+		assert.strictEqual(store.lastOpenedResource?.toString(), s1.resource.toString());
+	});
+
+	test('navigation across sessions and chats works together', async () => {
+		const chatA = stubChatWithId('a');
+		const chatB = stubChatWithId('b');
+		const s1 = stubSession('s1', SessionStatus.Completed, [chatA, chatB]);
+		const s2 = stubSession('s2');
+		store.addSession(s1);
+		store.addSession(s2);
+
+		// s1/chatA → s1/chatB → s2
+		store.setActiveSession(s1, chatA);
+		store.setActiveChat(chatB);
+		store.setActiveSession(s2);
+
+		// Go back to s1/chatB
+		await nav.goBack();
+		assert.strictEqual(store.lastOpenedChatResource?.toString(), chatB.resource.toString());
+
+		// Go back to s1/chatA
+		await nav.goBack();
+		assert.strictEqual(store.lastOpenedChatResource?.toString(), chatA.resource.toString());
+
+		// Go forward to s1/chatB
+		await nav.goForward();
+		assert.strictEqual(store.lastOpenedChatResource?.toString(), chatB.resource.toString());
+
+		// Go forward to s2
+		await nav.goForward();
+		assert.strictEqual(store.lastOpenedResource?.toString(), s2.resource.toString());
+	});
+
+	test('untitled chats are not recorded with a chat resource', () => {
+		const chatUntitled = stubChatWithId('untitled', SessionStatus.Untitled);
+		const s1 = stubSession('s1', SessionStatus.Completed, [chatUntitled]);
+		store.addSession(s1);
+
+		store.setActiveSession(s1, chatUntitled);
+		assert.strictEqual(canGoBack(), false, 'untitled chat produces a session-only entry, no second entry');
+	});
+
+	test('goBack falls back to openSession when chat was deleted', async () => {
+		const chatA = stubChatWithId('a');
+		const chatB = stubChatWithId('b');
+		const chatsObs = observableValue<readonly IChat[]>('test.chats', [chatA, chatB]);
+		const s1: ISession = {
+			...stubSession('s1', SessionStatus.Completed, [chatA, chatB]),
+			chats: chatsObs,
+		};
+		const s2 = stubSession('s2');
+		store.addSession(s1);
+		store.addSession(s2);
+
+		// Record history: s1/chatA → s1/chatB → s2
+		store.setActiveSession(s1, chatA);
+		store.setActiveChat(chatB);
+		store.setActiveSession(s2);
+
+		// Remove chatB from the session
+		chatsObs.set([chatA], undefined);
+
+		// Go back — chatB is stale, should fall back to openSession(s1)
+		await nav.goBack();
+		assert.strictEqual(store.lastOpenedResource?.toString(), s1.resource.toString());
+		assert.strictEqual(store.lastOpenedChatResource, undefined, 'should not open a stale chat');
+	});
 });