diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts index 9c148231abe1f..af0e34d5dab4a 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts @@ -59,7 +59,6 @@ function createMockProvider(id: string, opts?: { unarchiveSession: async () => { }, deleteSession: async () => { }, deleteChat: async () => { }, - setRead: () => { }, sendAndCreateChat: async () => { throw new Error('Not implemented'); }, capabilities: { multipleChatsPerSession: false }, }; diff --git a/src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsProvider.ts b/src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsProvider.ts index a28805ea9e2fd..2038af7ca1bd2 100644 --- a/src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsProvider.ts +++ b/src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsProvider.ts @@ -1317,13 +1317,6 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions } } - setRead(sessionId: string, read: boolean): void { - const agentSession = this._findAgentSession(sessionId); - if (agentSession) { - agentSession.setRead(read); - } - } - // -- Send -- async sendAndCreateChat(sessionId: string, options: ISendRequestOptions): Promise { diff --git a/src/vs/sessions/contrib/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts b/src/vs/sessions/contrib/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts index fc3c3d486f133..95e6309ef2ef9 100644 --- a/src/vs/sessions/contrib/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts @@ -227,8 +227,6 @@ function createProviderForSendTests( return disposables.add(instantiationService.createInstance(CopilotChatSessionsProvider)); } - - suite('CopilotChatSessionsProvider', () => { const disposables = new DisposableStore(); let model: MockAgentSessionsModel; @@ -356,20 +354,6 @@ suite('CopilotChatSessionsProvider', () => { assert.strictEqual(agentSession.isArchived(), false); }); - test('setRead marks session as read', () => { - const resource = URI.from({ scheme: AgentSessionProviders.Background, path: '/session-1' }); - const agentSession = createMockAgentSession(resource, { read: false }); - model.addSession(agentSession); - - const provider = createProvider(disposables, model); - provider.getSessions(); - - const session = provider.getSessions()[0]; - provider.setRead(session.sessionId, true); - - assert.strictEqual(agentSession.isRead(), true); - }); - // ---- Single-chat mode (multi-chat disabled) ------- test('single-chat mode: each session has exactly one chat', () => { diff --git a/src/vs/sessions/contrib/localAgentHost/browser/localAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/localAgentHost/browser/localAgentHostSessionsProvider.ts index be5f50b7f4fdb..60dacf6d9599f 100644 --- a/src/vs/sessions/contrib/localAgentHost/browser/localAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/localAgentHost/browser/localAgentHostSessionsProvider.ts @@ -510,16 +510,6 @@ export class LocalAgentHostSessionsProvider extends Disposable implements ISessi // Agent host sessions don't support deleting individual chats } - setRead(sessionId: string, read: boolean): void { - const rawId = this._rawIdFromChatId(sessionId); - const cached = rawId ? this._sessionCache.get(rawId) : undefined; - if (cached && rawId) { - cached.isRead.set(read, undefined); - const action = { type: ActionType.SessionIsReadChanged as const, session: AgentSession.uri(cached.agentProvider, rawId).toString(), isRead: read }; - this._agentHostService.dispatch(action); - } - } - async sendAndCreateChat(chatId: string, options: ISendRequestOptions): Promise { const session = this._currentNewSession; if (!session || session.sessionId !== chatId) { diff --git a/src/vs/sessions/contrib/localAgentHost/test/browser/localAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/localAgentHost/test/browser/localAgentHostSessionsProvider.test.ts index e6ba16d407e38..c8da9ffc3352e 100644 --- a/src/vs/sessions/contrib/localAgentHost/test/browser/localAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/localAgentHost/test/browser/localAgentHostSessionsProvider.test.ts @@ -517,19 +517,6 @@ suite('LocalAgentHostSessionsProvider', () => { assert.strictEqual(provider.getSessions().find(s => s.title.get() === 'To Delete'), undefined); }); - test('setRead toggles read state locally', () => { - const provider = createProvider(disposables, agentHost); - fireSessionAdded(agentHost, 'read-sess', { title: 'Read Test' }); - - const sessions = provider.getSessions(); - const target = sessions.find(s => s.title.get() === 'Read Test'); - assert.ok(target); - - assert.strictEqual(target!.isRead.get(), true); - provider.setRead(target!.sessionId, false); - assert.strictEqual(target!.isRead.get(), false); - }); - // ---- Rename ------- test('renameChat dispatches SessionTitleChanged action', async () => { diff --git a/src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts index c17dc5f521289..2b6e49ed91f85 100644 --- a/src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts @@ -641,18 +641,6 @@ export class RemoteAgentHostSessionsProvider extends Disposable implements ISess // Agent host sessions don't support deleting individual chats } - setRead(sessionId: string, read: boolean): void { - const rawId = this._rawIdFromChatId(sessionId); - const cached = rawId ? this._sessionCache.get(rawId) : undefined; - if (cached) { - cached.isRead.set(read, undefined); - if (this._connection && rawId) { - const action = { type: ActionType.SessionIsReadChanged as const, session: AgentSession.uri(cached.agentProvider, rawId).toString(), isRead: read }; - this._connection.dispatch(action); - } - } - } - async sendAndCreateChat(chatId: string, options: ISendRequestOptions): Promise { if (!this._connection) { throw new Error(localize('notConnectedSend', "Cannot send request: not connected to remote agent host '{0}'.", this.label)); diff --git a/src/vs/sessions/contrib/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts index 900ae768ac21d..4e788367f70cf 100644 --- a/src/vs/sessions/contrib/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts @@ -500,19 +500,6 @@ suite('RemoteAgentHostSessionsProvider', () => { assert.strictEqual(remaining.find((s) => s.title.get() === 'To Delete'), undefined); }); - test('setRead toggles read state locally', () => { - const provider = createProvider(disposables, connection); - fireSessionAdded(connection, 'read-sess', { title: 'Read Test' }); - - const sessions = provider.getSessions(); - const target = sessions.find((s) => s.title.get() === 'Read Test'); - assert.ok(target, 'Session should exist'); - - assert.strictEqual(target!.isRead.get(), true); - provider.setRead(target!.sessionId, false); - assert.strictEqual(target!.isRead.get(), false); - }); - // ---- Rename ------- test('renameSession dispatches SessionTitleChanged action with correct session URI', async () => { diff --git a/src/vs/sessions/contrib/sessions/browser/sessionsTitleBarWidget.ts b/src/vs/sessions/contrib/sessions/browser/sessionsTitleBarWidget.ts index 43ebe1009ad2b..bd6b7b25543ec 100644 --- a/src/vs/sessions/contrib/sessions/browser/sessionsTitleBarWidget.ts +++ b/src/vs/sessions/contrib/sessions/browser/sessionsTitleBarWidget.ts @@ -28,10 +28,9 @@ import { IsAuxiliaryWindowContext } from '../../../../workbench/common/contextke import { ChatSessionProviderIdContext, IsNewChatSessionContext, SessionsWelcomeVisibleContext } from '../../../common/contextkeys.js'; import { ISessionsProvidersService } from '../../../services/sessions/browser/sessionsProvidersService.js'; import { SessionStatus } from '../../../services/sessions/common/session.js'; +import { ISessionsListModelService } from './views/sessionsListModelService.js'; import { SHOW_SESSIONS_PICKER_COMMAND_ID } from './sessionsActions.js'; import { IsSessionArchivedContext, IsSessionPinnedContext, IsSessionReadContext, SessionItemContextMenuId } from './views/sessionsList.js'; -import { SessionsView, SessionsViewId } from './views/sessionsView.js'; -import { IViewsService } from '../../../../workbench/services/views/common/viewsService.js'; import { basename } from '../../../../base/common/resources.js'; import { ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js'; @@ -65,12 +64,12 @@ export class SessionsTitleBarWidget extends BaseActionViewItem { options: IBaseActionViewItemOptions | undefined, @IHoverService private readonly hoverService: IHoverService, @ISessionsManagementService private readonly sessionsManagementService: ISessionsManagementService, + @ISessionsListModelService private readonly sessionsListModelService: ISessionsListModelService, @IContextMenuService private readonly contextMenuService: IContextMenuService, @IMenuService private readonly menuService: IMenuService, @IContextKeyService private readonly contextKeyService: IContextKeyService, @ISessionsProvidersService private readonly sessionsProvidersService: ISessionsProvidersService, @ICommandService private readonly commandService: ICommandService, - @IViewsService private readonly viewsService: IViewsService, ) { super(undefined, action, options); @@ -302,11 +301,13 @@ export class SessionsTitleBarWidget extends BaseActionViewItem { return; } - const isPinned = this.viewsService.getViewWithId(SessionsViewId)?.sessionsControl?.isSessionPinned(sessionData) ?? false; + const isPinned = this.sessionsListModelService.isSessionPinned(sessionData); + const isArchived = sessionData.isArchived.get(); + const isRead = this.sessionsListModelService.isSessionRead(sessionData); const contextOverlay: [string, boolean | string][] = [ [IsSessionPinnedContext.key, isPinned], - [IsSessionArchivedContext.key, sessionData.isArchived.get()], - [IsSessionReadContext.key, sessionData.isRead.get()], + [IsSessionArchivedContext.key, isArchived], + [IsSessionReadContext.key, isRead], ['chatSessionType', sessionData.sessionType], [ChatSessionProviderIdContext.key, sessionData.providerId], ]; @@ -339,6 +340,7 @@ class SidebarToggleActionViewItem extends ActionViewItem { action: IAction, options: IBaseActionViewItemOptions | undefined, @ISessionsManagementService private readonly sessionsManagementService: ISessionsManagementService, + @ISessionsListModelService private readonly sessionsListModelService: ISessionsListModelService, @IWorkbenchLayoutService private readonly layoutService: IWorkbenchLayoutService, ) { super(context, action, { ...options, icon: true, label: false }); @@ -354,11 +356,9 @@ class SidebarToggleActionViewItem extends ActionViewItem { this._countBadge.setAttribute('aria-hidden', 'true'); this._updateBadge(); - // Single autorun that tracks all badge-relevant state: - // - session list changes (add/remove) via observableSignalFromEvent - // - individual session observable state (status, isRead, isArchived) - // - sidebar visibility changes + // Track session list changes, status changes, and sidebar visibility const sessionsChanged = observableSignalFromEvent(this, this.sessionsManagementService.onDidChangeSessions); + const listModelChanged = observableSignalFromEvent(this, this.sessionsListModelService.onDidChange); const sidebarVisibilityChanged = observableSignalFromEvent(this, handler => this.layoutService.onDidChangePartVisibility(e => { if (e.partId === Parts.SIDEBAR_PART) { handler(e); @@ -366,11 +366,11 @@ class SidebarToggleActionViewItem extends ActionViewItem { })); this._register(autorun(reader => { sessionsChanged.read(reader); + listModelChanged.read(reader); sidebarVisibilityChanged.read(reader); for (const session of this.sessionsManagementService.getSessions()) { session.isArchived.read(reader); session.status.read(reader); - session.isRead.read(reader); } this.updateClass(); this._updateBadge(); @@ -412,7 +412,7 @@ class SidebarToggleActionViewItem extends ActionViewItem { private _countUnreadSessions(): number { let unread = 0; for (const session of this.sessionsManagementService.getSessions()) { - if (!session.isArchived.get() && session.status.get() === SessionStatus.Completed && !session.isRead.get()) { + if (!session.isArchived.get() && session.status.get() === SessionStatus.Completed && !this.sessionsListModelService.isSessionRead(session)) { unread++; } } diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts index 5116affe82e68..3c02e43fdef59 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts @@ -41,6 +41,7 @@ import { IHoverService } from '../../../../../platform/hover/browser/hover.js'; import { HoverStyle } from '../../../../../base/browser/ui/hover/hover.js'; import { HoverPosition } from '../../../../../base/browser/ui/hover/hoverWidget.js'; import { ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; +import { ISessionsListModelService } from './sessionsListModelService.js'; const $ = DOM.$; @@ -165,7 +166,7 @@ class SessionItemRenderer implements ITreeRenderer = this._onDidChangeItemHeight.event; constructor( - private readonly options: { grouping: () => SessionsGrouping; sorting: () => SessionsSorting; isPinned: (session: ISession) => boolean }, + private readonly options: { grouping: () => SessionsGrouping; sorting: () => SessionsSorting; isPinned: (session: ISession) => boolean; isRead: (session: ISession) => boolean }, private readonly approvalModel: AgentSessionApprovalModel | undefined, private readonly instantiationService: IInstantiationService, private readonly contextKeyService: IContextKeyService, @@ -218,7 +219,7 @@ class SessionItemRenderer implements ITreeRenderer { @@ -228,11 +229,10 @@ class SessionItemRenderer implements ITreeRenderer { const sessionStatus = element.status.read(reader); - const isRead = element.isRead.read(reader); + const isRead = this.options.isRead(element); const isArchived = element.isArchived.read(reader); const gitHubInfo = element.gitHubInfo.read(reader); DOM.clearNode(template.iconContainer); @@ -655,7 +655,6 @@ export interface ISessionsList { export class SessionsList extends Disposable implements ISessionsList { private static readonly SECTION_COLLAPSE_STATE_KEY = 'sessionsListControl.sectionCollapseState'; - private static readonly PINNED_SESSIONS_KEY = 'sessionsListControl.pinnedSessions'; private static readonly EXCLUDED_TYPES_KEY = 'sessionsListControl.excludedSessionTypes'; private static readonly EXCLUDED_STATUSES_KEY = 'sessionsListControl.excludedStatuses'; private static readonly EXCLUDE_ARCHIVED_KEY = 'sessionsListControl.excludeArchived'; @@ -667,7 +666,6 @@ export class SessionsList extends Disposable implements ISessionsList { private readonly tree: WorkbenchObjectTree; private sessions: ISession[] = []; private visible = true; - private readonly _pinnedSessionIds: Set; private readonly excludedSessionTypes: Set; private readonly excludedStatuses: Set; private _excludeArchived: boolean; @@ -688,6 +686,7 @@ export class SessionsList extends Disposable implements ISessionsList { container: HTMLElement, private readonly options: ISessionsListControlOptions, @ISessionsManagementService private readonly _sessionsManagementService: ISessionsManagementService, + @ISessionsListModelService private readonly _sessionsListModelService: ISessionsListModelService, @IInstantiationService instantiationService: IInstantiationService, @IContextKeyService private readonly contextKeyService: IContextKeyService, @IStorageService private readonly storageService: IStorageService, @@ -697,9 +696,6 @@ export class SessionsList extends Disposable implements ISessionsList { ) { super(); - // Load pinned sessions from storage - this._pinnedSessionIds = this.loadPinnedSessions(); - // Load excluded session types from storage this.excludedSessionTypes = this.loadExcludedSessionTypes(); @@ -717,7 +713,7 @@ export class SessionsList extends Disposable implements ISessionsList { const markdownRendererService = instantiationService.invokeFunction(accessor => accessor.get(IMarkdownRendererService)); const hoverService = instantiationService.invokeFunction(accessor => accessor.get(IHoverService)); const sessionRenderer = new SessionItemRenderer( - { grouping: this.options.grouping, sorting: this.options.sorting, isPinned: s => this.isSessionPinned(s) }, + { grouping: this.options.grouping, sorting: this.options.sorting, isPinned: s => this.isSessionPinned(s), isRead: s => this.isSessionRead(s) }, approvalModel, instantiationService, contextKeyService, @@ -793,6 +789,7 @@ export class SessionsList extends Disposable implements ISessionsList { return; } if (!isSessionSection(element)) { + this.markRead(element); this.options.onSessionOpen(element.resource, e.editorOptions.preserveFocus ?? false); } })); @@ -824,10 +821,20 @@ export class SessionsList extends Disposable implements ISessionsList { } })); + this._register(this._sessionsListModelService.onDidChange(() => { + if (this.visible) { + this.update(); + } + })); + // Re-update when the active session changes so that a filtered-out - // session becomes visible while active and hides again when unselected + // session becomes visible while active and hides again when unselected. + // Also mark the newly active session as read. this._register(autorun(reader => { - this._sessionsManagementService.activeSession.read(reader); + const activeSession = this._sessionsManagementService.activeSession.read(reader); + if (activeSession) { + this._sessionsListModelService.markRead(activeSession); + } if (this.visible) { this.update(); } @@ -856,7 +863,7 @@ export class SessionsList extends Disposable implements ISessionsList { filtered = filtered.filter(s => !s.isArchived.get()); } if (this._excludeRead) { - filtered = filtered.filter(s => !s.isRead.get()); + filtered = filtered.filter(s => !this.isSessionRead(s)); } // Always include the active session even if it was filtered out, @@ -1015,7 +1022,7 @@ export class SessionsList extends Disposable implements ISessionsList { const contextOverlay: [string, boolean | string][] = [ [IsSessionPinnedContext.key, this.isSessionPinned(element)], [IsSessionArchivedContext.key, element.isArchived.get()], - [IsSessionReadContext.key, element.isRead.get()], + [IsSessionReadContext.key, this.isSessionRead(element)], ['chatSessionType', element.sessionType], [ChatSessionProviderIdContext.key, element.providerId], ]; @@ -1038,42 +1045,29 @@ export class SessionsList extends Disposable implements ISessionsList { // -- Pinning -- pinSession(session: ISession): void { - this._pinnedSessionIds.add(session.sessionId); - this.savePinnedSessions(); - this.update(); + this._sessionsListModelService.pinSession(session); } unpinSession(session: ISession): void { - this._pinnedSessionIds.delete(session.sessionId); - this.savePinnedSessions(); - this.update(); + this._sessionsListModelService.unpinSession(session); } isSessionPinned(session: ISession): boolean { - return this._pinnedSessionIds.has(session.sessionId); + return this._sessionsListModelService.isSessionPinned(session); } - private loadPinnedSessions(): Set { - const raw = this.storageService.get(SessionsList.PINNED_SESSIONS_KEY, StorageScope.PROFILE); - if (raw) { - try { - const arr = JSON.parse(raw); - if (Array.isArray(arr)) { - return new Set(arr); - } - } catch { - // ignore corrupt data - } - } - return new Set(); + // -- Read/Unread -- + + markRead(session: ISession): void { + this._sessionsListModelService.markRead(session); } - private savePinnedSessions(): void { - if (this._pinnedSessionIds.size === 0) { - this.storageService.remove(SessionsList.PINNED_SESSIONS_KEY, StorageScope.PROFILE); - } else { - this.storageService.store(SessionsList.PINNED_SESSIONS_KEY, JSON.stringify([...this._pinnedSessionIds]), StorageScope.PROFILE, StorageTarget.USER); - } + markUnread(session: ISession): void { + this._sessionsListModelService.markUnread(session); + } + + isSessionRead(session: ISession): boolean { + return this._sessionsListModelService.isSessionRead(session); } // -- Session type filtering -- diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsListModelService.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsListModelService.ts new file mode 100644 index 0000000000000..e8693eb5df9f0 --- /dev/null +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsListModelService.ts @@ -0,0 +1,186 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Emitter, Event } from '../../../../../base/common/event.js'; +import { Disposable } from '../../../../../base/common/lifecycle.js'; +import { createDecorator } from '../../../../../platform/instantiation/common/instantiation.js'; +import { InstantiationType, registerSingleton } from '../../../../../platform/instantiation/common/extensions.js'; +import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; +import { ISession } from '../../../../services/sessions/common/session.js'; +import { ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; + +export const enum SessionListModelChangeKind { + Pinned = 'pinned', + Read = 'read', +} + +export interface ISessionListModelChangeEvent { + readonly changes: ReadonlyArray<{ readonly sessionId: string; readonly kind: SessionListModelChangeKind }>; +} + +/** + * Service that manages UI-only state for sessions: pinned and read. + * + * This state is purely local (persisted in storage) and not synced to providers. + * Extracted from SessionsList so it can be consumed by any component (title bar, + * views, actions) without going through the view. + */ +export interface ISessionsListModelService { + readonly _serviceBrand: undefined; + + /** Fires when a session's pinned or read state changes. */ + readonly onDidChange: Event; + + // -- Pinning -- + + pinSession(session: ISession): void; + unpinSession(session: ISession): void; + isSessionPinned(session: ISession): boolean; + + // -- Read/Unread -- + + markRead(session: ISession): void; + markUnread(session: ISession): void; + isSessionRead(session: ISession): boolean; + markAllRead(sessions: ISession[]): void; +} + +export const ISessionsListModelService = createDecorator('sessionsListModelService'); + +export class SessionsListModelService extends Disposable implements ISessionsListModelService { + + declare readonly _serviceBrand: undefined; + + private static readonly PINNED_SESSIONS_KEY = 'sessionsListControl.pinnedSessions'; + private static readonly READ_SESSIONS_KEY = 'sessionsListControl.readSessions'; + + private readonly _onDidChange = this._register(new Emitter()); + readonly onDidChange: Event = this._onDidChange.event; + + private readonly _pinnedSessionIds: Set; + private readonly _readSessionIds: Set; + + constructor( + @IStorageService private readonly storageService: IStorageService, + @ISessionsManagementService private readonly sessionsManagementService: ISessionsManagementService, + ) { + super(); + + this._pinnedSessionIds = this.loadSet(SessionsListModelService.PINNED_SESSIONS_KEY); + this._readSessionIds = this.loadSet(SessionsListModelService.READ_SESSIONS_KEY); + + this._register(this.sessionsManagementService.onDidChangeSessions(e => { + for (const session of e.removed) { + this.deleteSession(session); + } + })); + } + + // -- Pinning -- + + pinSession(session: ISession): void { + if (this._pinnedSessionIds.has(session.sessionId)) { + return; + } + this._pinnedSessionIds.add(session.sessionId); + this.saveSet(SessionsListModelService.PINNED_SESSIONS_KEY, this._pinnedSessionIds); + this._onDidChange.fire({ changes: [{ sessionId: session.sessionId, kind: SessionListModelChangeKind.Pinned }] }); + } + + unpinSession(session: ISession): void { + if (!this._pinnedSessionIds.has(session.sessionId)) { + return; + } + this._pinnedSessionIds.delete(session.sessionId); + this.saveSet(SessionsListModelService.PINNED_SESSIONS_KEY, this._pinnedSessionIds); + this._onDidChange.fire({ changes: [{ sessionId: session.sessionId, kind: SessionListModelChangeKind.Pinned }] }); + } + + isSessionPinned(session: ISession): boolean { + return this._pinnedSessionIds.has(session.sessionId); + } + + // -- Read/Unread -- + + markRead(session: ISession): void { + if (this._readSessionIds.has(session.sessionId)) { + return; + } + this._readSessionIds.add(session.sessionId); + this.saveSet(SessionsListModelService.READ_SESSIONS_KEY, this._readSessionIds); + this._onDidChange.fire({ changes: [{ sessionId: session.sessionId, kind: SessionListModelChangeKind.Read }] }); + } + + markUnread(session: ISession): void { + if (!this._readSessionIds.has(session.sessionId)) { + return; + } + this._readSessionIds.delete(session.sessionId); + this.saveSet(SessionsListModelService.READ_SESSIONS_KEY, this._readSessionIds); + this._onDidChange.fire({ changes: [{ sessionId: session.sessionId, kind: SessionListModelChangeKind.Read }] }); + } + + isSessionRead(session: ISession): boolean { + return this._readSessionIds.has(session.sessionId); + } + + markAllRead(sessions: ISession[]): void { + const changed: { sessionId: string; kind: SessionListModelChangeKind }[] = []; + for (const session of sessions) { + if (!this._readSessionIds.has(session.sessionId)) { + this._readSessionIds.add(session.sessionId); + changed.push({ sessionId: session.sessionId, kind: SessionListModelChangeKind.Read }); + } + } + if (changed.length > 0) { + this.saveSet(SessionsListModelService.READ_SESSIONS_KEY, this._readSessionIds); + this._onDidChange.fire({ changes: changed }); + } + } + + // -- Cleanup -- + + private deleteSession(session: ISession): void { + const changes: { sessionId: string; kind: SessionListModelChangeKind }[] = []; + if (this._pinnedSessionIds.delete(session.sessionId)) { + this.saveSet(SessionsListModelService.PINNED_SESSIONS_KEY, this._pinnedSessionIds); + changes.push({ sessionId: session.sessionId, kind: SessionListModelChangeKind.Pinned }); + } + if (this._readSessionIds.delete(session.sessionId)) { + this.saveSet(SessionsListModelService.READ_SESSIONS_KEY, this._readSessionIds); + changes.push({ sessionId: session.sessionId, kind: SessionListModelChangeKind.Read }); + } + if (changes.length > 0) { + this._onDidChange.fire({ changes }); + } + } + + // -- Storage helpers -- + + private loadSet(key: string): Set { + const raw = this.storageService.get(key, StorageScope.PROFILE); + if (raw) { + try { + const arr = JSON.parse(raw); + if (Array.isArray(arr)) { + return new Set(arr); + } + } catch { + // ignore corrupt data + } + } + return new Set(); + } + + private saveSet(key: string, set: Set): void { + if (set.size === 0) { + this.storageService.remove(key, StorageScope.PROFILE); + } else { + this.storageService.store(key, JSON.stringify([...set]), StorageScope.PROFILE, StorageTarget.USER); + } + } +} + +registerSingleton(ISessionsListModelService, SessionsListModelService, InstantiationType.Delayed); diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts index 0d3e5c8cb6b6a..9059f96fe44cd 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts @@ -25,6 +25,7 @@ import { IsWorkspaceGroupCappedContext, SessionsViewFilterOptionsSubMenu, Sessio import { SessionsViewId as NewChatViewId, NewChatViewPane } from '../../../chat/browser/newChatViewPane.js'; import { Menus } from '../../../../browser/menus.js'; import { ActiveSessionSupportsMultiChatContext, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; +import { ISessionsListModelService } from './sessionsListModelService.js'; import { ChatContextKeys } from '../../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; // Constants @@ -590,9 +591,9 @@ registerAction2(class MarkSessionReadAction extends Action2 { return; } const sessions = Array.isArray(context) ? context : [context]; - const sessionsManagementService = accessor.get(ISessionsManagementService); + const sessionsListModelService = accessor.get(ISessionsListModelService); for (const session of sessions) { - sessionsManagementService.setRead(session, true); + sessionsListModelService.markRead(session); } } }); @@ -618,9 +619,9 @@ registerAction2(class MarkSessionUnreadAction extends Action2 { return; } const sessions = Array.isArray(context) ? context : [context]; - const sessionsManagementService = accessor.get(ISessionsManagementService); + const sessionsListModelService = accessor.get(ISessionsListModelService); for (const session of sessions) { - sessionsManagementService.setRead(session, false); + sessionsListModelService.markUnread(session); } } }); @@ -670,12 +671,10 @@ registerAction2(class MarkAllSessionsReadAction extends Action2 { } run(accessor: ServicesAccessor): void { const sessionsManagementService = accessor.get(ISessionsManagementService); - const sessions = sessionsManagementService.getSessions(); - for (const session of sessions) { - if (!session.isArchived.get() && !session.isRead.get()) { - sessionsManagementService.setRead(session, true); - } - } + const sessionsListModelService = accessor.get(ISessionsListModelService); + const sessions = sessionsManagementService.getSessions() + .filter(s => !s.isArchived.get() && !sessionsListModelService.isSessionRead(s)); + sessionsListModelService.markAllRead(sessions); } }); @@ -723,7 +722,6 @@ registerAction2(class MarkSessionAsDoneAction extends Action2 { async run(accessor: ServicesAccessor): Promise { const sessionsManagementService = accessor.get(ISessionsManagementService); - const activeSession = sessionsManagementService.activeSession.get(); if (!activeSession || activeSession.status.get() === SessionStatus.Untitled) { return; diff --git a/src/vs/sessions/contrib/sessions/test/browser/sessionsListModelService.test.ts b/src/vs/sessions/contrib/sessions/test/browser/sessionsListModelService.test.ts new file mode 100644 index 0000000000000..46b5cfd01577e --- /dev/null +++ b/src/vs/sessions/contrib/sessions/test/browser/sessionsListModelService.test.ts @@ -0,0 +1,324 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { Codicon } from '../../../../../base/common/codicons.js'; +import { Emitter } from '../../../../../base/common/event.js'; +import { observableValue } from '../../../../../base/common/observable.js'; +import { URI } from '../../../../../base/common/uri.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { IStorageService, InMemoryStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; +import { IChat, ISession, SessionStatus } from '../../../../services/sessions/common/session.js'; +import { ISessionsChangeEvent, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; +import { ISessionListModelChangeEvent, SessionListModelChangeKind, SessionsListModelService } from '../../browser/views/sessionsListModelService.js'; +import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; +import { mock } from '../../../../../base/test/common/mock.js'; + +function createSession(id: string): ISession { + return { + sessionId: id, + resource: URI.parse(`session://${id}`), + providerId: 'test', + sessionType: 'test', + icon: Codicon.account, + createdAt: new Date(), + workspace: observableValue(`workspace-${id}`, undefined), + title: observableValue(`title-${id}`, id), + updatedAt: observableValue(`updatedAt-${id}`, new Date()), + status: observableValue(`status-${id}`, SessionStatus.Completed), + changes: observableValue(`changes-${id}`, []), + modelId: observableValue(`modelId-${id}`, undefined), + mode: observableValue(`mode-${id}`, undefined), + loading: observableValue(`loading-${id}`, false), + isArchived: observableValue(`isArchived-${id}`, false), + isRead: observableValue(`isRead-${id}`, true), + description: observableValue(`description-${id}`, undefined), + lastTurnEnd: observableValue(`lastTurnEnd-${id}`, undefined), + gitHubInfo: observableValue(`gitHubInfo-${id}`, undefined), + chats: observableValue(`chats-${id}`, []), + mainChat: undefined!, + }; +} + +suite('SessionsListModelService', () => { + + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + let service: SessionsListModelService; + let sessionsChangedEmitter: Emitter; + + setup(() => { + const instantiationService = disposables.add(new TestInstantiationService()); + instantiationService.stub(IStorageService, disposables.add(new InMemoryStorageService())); + sessionsChangedEmitter = disposables.add(new Emitter()); + instantiationService.stub(ISessionsManagementService, { + ...mock(), + onDidChangeSessions: sessionsChangedEmitter.event, + }); + service = disposables.add(instantiationService.createInstance(SessionsListModelService)); + }); + + // -- Pinning -- + + test('pinSession marks session as pinned', () => { + const session = createSession('s1'); + assert.strictEqual(service.isSessionPinned(session), false); + + service.pinSession(session); + + assert.strictEqual(service.isSessionPinned(session), true); + }); + + test('unpinSession marks session as not pinned', () => { + const session = createSession('s1'); + service.pinSession(session); + + service.unpinSession(session); + + assert.strictEqual(service.isSessionPinned(session), false); + }); + + test('pinSession is idempotent and fires onDidChange only once', () => { + const session = createSession('s1'); + let changeCount = 0; + disposables.add(service.onDidChange(() => changeCount++)); + + service.pinSession(session); + service.pinSession(session); + + assert.strictEqual(changeCount, 1); + }); + + test('unpinSession does not fire when not pinned', () => { + const session = createSession('s1'); + let changeCount = 0; + disposables.add(service.onDidChange(() => changeCount++)); + + service.unpinSession(session); + + assert.strictEqual(changeCount, 0); + }); + + test('pinning one session does not affect another', () => { + const s1 = createSession('s1'); + const s2 = createSession('s2'); + + service.pinSession(s1); + + assert.strictEqual(service.isSessionPinned(s1), true); + assert.strictEqual(service.isSessionPinned(s2), false); + }); + + // -- Read/Unread -- + + test('markRead marks session as read', () => { + const session = createSession('s1'); + assert.strictEqual(service.isSessionRead(session), false); + + service.markRead(session); + + assert.strictEqual(service.isSessionRead(session), true); + }); + + test('markUnread marks session as unread', () => { + const session = createSession('s1'); + service.markRead(session); + + service.markUnread(session); + + assert.strictEqual(service.isSessionRead(session), false); + }); + + test('markRead is idempotent', () => { + const session = createSession('s1'); + let changeCount = 0; + disposables.add(service.onDidChange(() => changeCount++)); + + service.markRead(session); + service.markRead(session); + + assert.strictEqual(changeCount, 1); + }); + + test('markUnread does not fire when already unread', () => { + const session = createSession('s1'); + let changeCount = 0; + disposables.add(service.onDidChange(() => changeCount++)); + + service.markUnread(session); + + assert.strictEqual(changeCount, 0); + }); + + test('markAllRead marks multiple sessions as read', () => { + const s1 = createSession('s1'); + const s2 = createSession('s2'); + const s3 = createSession('s3'); + + service.markAllRead([s1, s2, s3]); + + assert.strictEqual(service.isSessionRead(s1), true); + assert.strictEqual(service.isSessionRead(s2), true); + assert.strictEqual(service.isSessionRead(s3), true); + }); + + test('markAllRead does not fire when all already read', () => { + const s1 = createSession('s1'); + service.markRead(s1); + + let changeCount = 0; + disposables.add(service.onDidChange(() => changeCount++)); + + service.markAllRead([s1]); + + assert.strictEqual(changeCount, 0); + }); + + test('markAllRead fires once for multiple new reads', () => { + const s1 = createSession('s1'); + const s2 = createSession('s2'); + + let changeCount = 0; + disposables.add(service.onDidChange(() => changeCount++)); + + service.markAllRead([s1, s2]); + + assert.strictEqual(changeCount, 1); + }); + + // -- Independence -- + + test('read and pinned states are independent', () => { + const session = createSession('s1'); + + service.pinSession(session); + assert.strictEqual(service.isSessionPinned(session), true); + assert.strictEqual(service.isSessionRead(session), false); + + service.markRead(session); + assert.strictEqual(service.isSessionPinned(session), true); + assert.strictEqual(service.isSessionRead(session), true); + + service.unpinSession(session); + assert.strictEqual(service.isSessionPinned(session), false); + assert.strictEqual(service.isSessionRead(session), true); + }); + + // -- onDidChange -- + + test('onDidChange includes changes array with sessionId and kind', () => { + const session = createSession('s1'); + const events: ISessionListModelChangeEvent[] = []; + disposables.add(service.onDidChange(e => events.push(e))); + + service.pinSession(session); + service.unpinSession(session); + service.markRead(session); + service.markUnread(session); + + assert.deepStrictEqual(events, [ + { changes: [{ sessionId: 's1', kind: SessionListModelChangeKind.Pinned }] }, + { changes: [{ sessionId: 's1', kind: SessionListModelChangeKind.Pinned }] }, + { changes: [{ sessionId: 's1', kind: SessionListModelChangeKind.Read }] }, + { changes: [{ sessionId: 's1', kind: SessionListModelChangeKind.Read }] }, + ]); + }); + + test('markAllRead fires single event with all sessions', () => { + const s1 = createSession('s1'); + const s2 = createSession('s2'); + const events: ISessionListModelChangeEvent[] = []; + disposables.add(service.onDidChange(e => events.push(e))); + + service.markAllRead([s1, s2]); + + assert.deepStrictEqual(events, [ + { + changes: [ + { sessionId: 's1', kind: SessionListModelChangeKind.Read }, + { sessionId: 's2', kind: SessionListModelChangeKind.Read }, + ] + }, + ]); + }); + + // -- Cleanup -- + + test('cleans up state when session is removed', () => { + const session = createSession('s1'); + service.pinSession(session); + service.markRead(session); + + const events: ISessionListModelChangeEvent[] = []; + disposables.add(service.onDidChange(e => events.push(e))); + + sessionsChangedEmitter.fire({ added: [], removed: [session], changed: [] }); + + assert.strictEqual(service.isSessionPinned(session), false); + assert.strictEqual(service.isSessionRead(session), false); + assert.deepStrictEqual(events, [ + { + changes: [ + { sessionId: 's1', kind: SessionListModelChangeKind.Pinned }, + { sessionId: 's1', kind: SessionListModelChangeKind.Read }, + ] + }, + ]); + }); + + test('removal does not fire when session has no state', () => { + const session = createSession('s1'); + let changeCount = 0; + disposables.add(service.onDidChange(() => changeCount++)); + + sessionsChangedEmitter.fire({ added: [], removed: [session], changed: [] }); + + assert.strictEqual(changeCount, 0); + }); + + test('removal does not affect other sessions', () => { + const s1 = createSession('s1'); + const s2 = createSession('s2'); + service.pinSession(s1); + service.pinSession(s2); + + sessionsChangedEmitter.fire({ added: [], removed: [s1], changed: [] }); + + assert.strictEqual(service.isSessionPinned(s1), false); + assert.strictEqual(service.isSessionPinned(s2), true); + }); + + // -- Storage persistence -- + + test('state is loaded from storage on construction', () => { + const storageService = disposables.add(new InMemoryStorageService()); + + // Pre-populate storage + storageService.store('sessionsListControl.pinnedSessions', JSON.stringify(['s1']), StorageScope.PROFILE, StorageTarget.USER); + storageService.store('sessionsListControl.readSessions', JSON.stringify(['s2']), StorageScope.PROFILE, StorageTarget.USER); + + const instantiationService = disposables.add(new TestInstantiationService()); + instantiationService.stub(IStorageService, storageService); + instantiationService.stub(ISessionsManagementService, { ...mock(), onDidChangeSessions: disposables.add(new Emitter()).event }); + const loadedService = disposables.add(instantiationService.createInstance(SessionsListModelService)); + + assert.strictEqual(loadedService.isSessionPinned(createSession('s1')), true); + assert.strictEqual(loadedService.isSessionPinned(createSession('s2')), false); + assert.strictEqual(loadedService.isSessionRead(createSession('s2')), true); + assert.strictEqual(loadedService.isSessionRead(createSession('s1')), false); + }); + + test('corrupt storage data is handled gracefully', () => { + const storageService = disposables.add(new InMemoryStorageService()); + storageService.store('sessionsListControl.pinnedSessions', 'not-valid-json{', StorageScope.PROFILE, StorageTarget.USER); + + const instantiationService = disposables.add(new TestInstantiationService()); + instantiationService.stub(IStorageService, storageService); + instantiationService.stub(ISessionsManagementService, { ...mock(), onDidChangeSessions: disposables.add(new Emitter()).event }); + const loadedService = disposables.add(instantiationService.createInstance(SessionsListModelService)); + + // Should not throw and should return empty state + assert.strictEqual(loadedService.isSessionPinned(createSession('s1')), false); + }); +}); diff --git a/src/vs/sessions/services/sessions/browser/sessionsManagementService.ts b/src/vs/sessions/services/sessions/browser/sessionsManagementService.ts index 66a2fc0aff998..a86bdf4dffc24 100644 --- a/src/vs/sessions/services/sessions/browser/sessionsManagementService.ts +++ b/src/vs/sessions/services/sessions/browser/sessionsManagementService.ts @@ -240,7 +240,6 @@ class SessionsManagementService extends Disposable implements ISessionsManagemen this.logService.info(`[SessionsManagement] openSession: ${sessionResource.toString()} provider=${sessionData.providerId}`); this.isNewChatSessionContext.set(false); this.setActiveSession(sessionData); - this.setRead(sessionData, true); // mark as read when opened await this.chatWidgetService.openSession(sessionData.resource, ChatViewPaneTarget, { preserveFocus: options?.preserveFocus }); } @@ -419,10 +418,6 @@ class SessionsManagementService extends Disposable implements ISessionsManagemen async renameChat(session: ISession, chatUri: URI, title: string): Promise { await this._getProvider(session)?.renameChat(session.sessionId, chatUri, title); } - - setRead(session: ISession, read: boolean): void { - this._getProvider(session)?.setRead(session.sessionId, read); - } } registerSingleton(ISessionsManagementService, SessionsManagementService, InstantiationType.Delayed); diff --git a/src/vs/sessions/services/sessions/common/sessionsManagement.ts b/src/vs/sessions/services/sessions/common/sessionsManagement.ts index f3a92c1c6f1d2..6bb499cc57e31 100644 --- a/src/vs/sessions/services/sessions/common/sessionsManagement.ts +++ b/src/vs/sessions/services/sessions/common/sessionsManagement.ts @@ -140,8 +140,6 @@ export interface ISessionsManagementService { deleteChat(session: ISession, chatUri: URI): Promise; /** Rename a chat within a session. */ renameChat(session: ISession, chatUri: URI, title: string): Promise; - /** Mark a session as read or unread. */ - setRead(session: ISession, read: boolean): void; } export const ISessionsManagementService = createDecorator('sessionsManagementService'); diff --git a/src/vs/sessions/services/sessions/common/sessionsProvider.ts b/src/vs/sessions/services/sessions/common/sessionsProvider.ts index a7f4a9b18b499..9f0507ceed1e3 100644 --- a/src/vs/sessions/services/sessions/common/sessionsProvider.ts +++ b/src/vs/sessions/services/sessions/common/sessionsProvider.ts @@ -115,8 +115,6 @@ export interface ISessionsProvider { deleteSession(sessionId: string): Promise; /** Delete a single chat from a session. */ deleteChat(sessionId: string, chatUri: URI): Promise; - /** Mark a session as read or unread. */ - setRead(sessionId: string, read: boolean): void; // -- Send -- /** Send a request, creating a new chat in the session. */ diff --git a/src/vs/sessions/sessions.common.main.ts b/src/vs/sessions/sessions.common.main.ts index 9f5be21ebec73..4dda7ed4067fa 100644 --- a/src/vs/sessions/sessions.common.main.ts +++ b/src/vs/sessions/sessions.common.main.ts @@ -462,6 +462,7 @@ import './contrib/chat/browser/chat.contribution.js'; import './contrib/chat/browser/customizationsDebugLog.contribution.js'; import './contrib/copilotChatSessions/browser/copilotChatSessions.contribution.js'; import './contrib/sessions/browser/sessions.contribution.js'; +import './contrib/sessions/browser/views/sessionsListModelService.js'; import './contrib/sessions/browser/customizationsToolbar.contribution.js'; import './contrib/changes/browser/changesView.contribution.js'; import './contrib/layout/browser/layout.contribution.js';