diff --git a/src/vs/sessions/services/sessions/browser/sessionNavigation.ts b/src/vs/sessions/services/sessions/browser/sessionNavigation.ts index c09be10e1be3f..2acb51322e797 100644 --- a/src/vs/sessions/services/sessions/browser/sessionNavigation.ts +++ b/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 { @@ -168,7 +178,7 @@ 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 { @@ -176,7 +186,16 @@ export class SessionsNavigation extends Disposable { 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 { diff --git a/src/vs/sessions/services/sessions/test/browser/sessionNavigation.test.ts b/src/vs/sessions/services/sessions/test/browser/sessionNavigation.test.ts index ee026993989d5..b2f257e784dd9 100644 --- a/src/vs/sessions/services/sessions/test/browser/sessionNavigation.test.ts +++ b/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,16 +90,19 @@ class MockSessionStore implements ISessionsManagementService { private readonly _sessions = new Map(); 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(`test.activeChat-${session.sessionId}`, activeChat), }; this.activeSession.set(active, undefined); } else { @@ -88,6 +110,13 @@ class MockSessionStore implements ISessionsManagementService { } } + setActiveChat(chat: IChat): void { + const active = this.activeSession.get(); + if (active) { + (active.activeChat as ReturnType>).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 { 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 { throw new Error('not implemented'); } + async openChat(session: ISession, chatUri: URI): Promise { + 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 { 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('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'); + }); });