From 5bcfb040f4e28b4612fde0a77ec4dbc4f0dbd481 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Mon, 22 Jun 2026 03:12:34 +0200 Subject: [PATCH] sessions: support multi-session delete via session capability (#322310) * sessions: support multi-session delete via session capability Enable deleting multiple sessions at once and consolidate the delete action into the sessions workbench core instead of each provider. - Add required `deleteSessions(sessionIds)` batch API to `ISessionsProvider` and `deleteSessions(sessions)` to the sessions management service, which groups sessions by provider before delegating. - Implement a real batch delete in the agent host base provider that resolves all targets synchronously up front before disposing, avoiding a reconcile dropping not-yet-processed cached sessions. - Introduce a `supportsDelete` session capability and a `chatSessionSupportsDelete` context key, mirroring the `supportsRename` pattern. A single core `DeleteSessionAction` (gated on the capability) replaces the per-provider delete actions and the delete-session helper. - Move the delete confirmation dialog into the action and remove it from the provider implementations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sessions: address CCR feedback on multi-session delete - Filter the action's incoming context to sessions that advertise `capabilities.supportsDelete`, so a mixed multi-selection never deletes sessions that don't support it. - Use a singular vs plural delete error message and distinct localization keys instead of a plural-only message. - Make the management service batch delete best-effort: continue deleting other providers' sessions if one provider rejects, then surface the first error. Emit `onDidDeleteSession` only for providers that succeeded. - Clarify the `deleteSessions` JSDoc to say implementations may batch or delegate to `deleteSession`. - Revert the unrelated "Copilot CLI" -> "Copilot" session type label change. - Add a best-effort batch delete test to the management service suite. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/vs/sessions/common/contextkeys.ts | 1 + .../browser/sessionWorkspacePicker.test.ts | 1 + .../agentHost/AGENT_HOST_SESSIONS_PROVIDER.md | 3 +- .../browser/agentHostSessionDeleteAction.ts | 33 ----------- .../browser/baseAgentHostSessionsProvider.ts | 31 ++++++++-- .../localAgentHostSessionsProvider.test.ts | 18 ++++++ .../browser/copilotChatSessionsActions.ts | 39 +------------ .../browser/copilotChatSessionsProvider.ts | 24 ++++---- .../LOCAL_CHAT_SESSIONS_PROVIDER.md | 2 +- .../browser/localChatSessions.contribution.ts | 32 ++-------- .../browser/localChatSessionsProvider.ts | 7 +++ .../sessions/browser/deleteSessionHelper.ts | 50 ---------------- .../sessions/browser/views/sessionsList.ts | 3 +- .../browser/views/sessionsViewActions.ts | 49 +++++++++++++++- .../browser/sessionsManagementService.ts | 32 ++++++++++ .../services/sessions/common/session.ts | 7 +++ .../sessions/common/sessionsManagement.ts | 9 +++ .../sessions/common/sessionsProvider.ts | 8 +++ .../test/browser/sessionNavigation.test.ts | 1 + .../browser/sessionsManagementService.test.ts | 58 +++++++++++++++++++ src/vs/sessions/sessions.desktop.main.ts | 1 - src/vs/sessions/sessions.web.main.ts | 1 - 22 files changed, 238 insertions(+), 172 deletions(-) delete mode 100644 src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionDeleteAction.ts delete mode 100644 src/vs/sessions/contrib/sessions/browser/deleteSessionHelper.ts diff --git a/src/vs/sessions/common/contextkeys.ts b/src/vs/sessions/common/contextkeys.ts index 34f8a7f197f..4df02733486 100644 --- a/src/vs/sessions/common/contextkeys.ts +++ b/src/vs/sessions/common/contextkeys.ts @@ -19,6 +19,7 @@ export const ActiveSessionUsesCombinedConfigPickerContext = new RawContextKey('chatSessionProviderId', '', localize('chatSessionProviderId', "The provider ID of a session in context menu overlays")); export const ChatSessionTypeContext = new RawContextKey('chatSessionType', '', localize('chatSessionType', "The session type of a session in context menu overlays")); export const ChatSessionSupportsRenameContext = new RawContextKey('chatSessionSupportsRename', false, localize('chatSessionSupportsRename', "Whether a session in context menu overlays can be renamed")); +export const ChatSessionSupportsDeleteContext = new RawContextKey('chatSessionSupportsDelete', false, localize('chatSessionSupportsDelete', "Whether a session in context menu overlays can be deleted")); //#endregion 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 dd65334e69c..31214cf98ba 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts @@ -109,6 +109,7 @@ function createMockProvider(id: string, opts?: { archiveSession: async () => { }, unarchiveSession: async () => { }, deleteSession: async () => { }, + deleteSessions: async () => { }, deleteChat: async () => { }, createNewChat: async () => { throw new Error('Not implemented'); }, sendRequest: async (_sessionId: string, _chatResource: URI, _options: ISendRequestOptions) => { throw new Error('Not implemented'); }, diff --git a/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md b/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md index 5d4cc783353..fa494b176ca 100644 --- a/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md +++ b/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md @@ -134,7 +134,7 @@ sidebar list. ## CRUD & Stubbed Operations -- `archiveSession` / `unarchiveSession` / `deleteSession` — round-trip to the backend. +- `archiveSession` / `unarchiveSession` / `deleteSession` — round-trip to the backend. `deleteSessions` is the batch variant (used when multiple sessions are selected): it disposes each backend session and emits a single removal change event. Sessions advertise `capabilities.supportsDelete`, so the shared sessions-list "Delete..." action (contributed by the sessions workbench, gated on `ChatSessionSupportsDeleteContext`) confirms and invokes deletion — there is no provider-specific delete action. - `renameChat` — updates the session title. - `deleteChat` — no-op (agent host sessions don't model individually deletable chats). @@ -153,7 +153,6 @@ The provider ships a rich set of session-scoped UI in `browser/`: | `agentHostSkillButtons.ts` | Built-in skill toolbar buttons; defines the `sessions.isAgentHostSession` (`IsAgentHostSession`) context key bound to the active session's provider. | | `agentHostSessionChangesets.ts` / `agentHostDiffs.ts` | Changeset model and diff conversion (`mapProtocolStatus` maps the protocol status bitset → `SessionStatus`). | | `agentHostSessionBranchActions.ts` | Branch-related session actions. | -| `agentHostSessionDeleteAction.ts` | "Delete..." session context-menu action (gated on `ANY_AGENT_HOST_PROVIDER_RE`); delegates to the shared `confirmAndDeleteSessions` helper which confirms and calls `ISessionsManagementService.deleteSession`. | | `exportDebugLogsAction.ts` | "Export debug logs" developer action. | | `openSessionEventsFileActions.ts` | "Open Copilot CLI State File" — Sessions-app variant resolving the session via `ISessionsManagementService.activeSession`. | | `mobile/` | Phone-layout variants: `mobileAgentHostModePicker.ts`, `mobileChatInputConfigPicker.ts`, `mobileChatPhoneInputPresenter.ts`. | diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionDeleteAction.ts b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionDeleteAction.ts deleted file mode 100644 index 7beb56f3b3a..00000000000 --- a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionDeleteAction.ts +++ /dev/null @@ -1,33 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { localize2 } from '../../../../../nls.js'; -import { Action2, registerAction2 } from '../../../../../platform/actions/common/actions.js'; -import { ContextKeyExpr } from '../../../../../platform/contextkey/common/contextkey.js'; -import { ServicesAccessor } from '../../../../../platform/instantiation/common/instantiation.js'; -import { ANY_AGENT_HOST_PROVIDER_RE } from '../../../../common/agentHostSessionsProvider.js'; -import { ChatSessionProviderIdContext } from '../../../../common/contextkeys.js'; -import { ISession } from '../../../../services/sessions/common/session.js'; -import { confirmAndDeleteSessions } from '../../../sessions/browser/deleteSessionHelper.js'; -import { SessionItemContextMenuId } from '../../../sessions/browser/views/sessionsList.js'; - -registerAction2(class DeleteAgentHostSessionAction extends Action2 { - constructor() { - super({ - id: 'sessionsViewPane.agentHost.deleteSession', - title: localize2('deleteAgentHostSession', "Delete..."), - menu: [{ - id: SessionItemContextMenuId, - group: '1_edit', - order: 4, - when: ContextKeyExpr.regex(ChatSessionProviderIdContext.key, ANY_AGENT_HOST_PROVIDER_RE), - }] - }); - } - - run(accessor: ServicesAccessor, context?: ISession | ISession[]): Promise { - return confirmAndDeleteSessions(accessor, context); - } -}); diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 63cb6ec94c9..6be8e87dde0 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -319,7 +319,7 @@ export class AgentHostSessionAdapter extends Disposable implements ISession { this.sessionId = toSessionId(providerId, this.resource); this.providerId = providerId; this.sessionType = logicalSessionType; - this.capabilities = { supportsMultipleChats: logicalSessionType === CopilotCLISessionType.id, supportsRename: true }; + this.capabilities = { supportsMultipleChats: logicalSessionType === CopilotCLISessionType.id, supportsRename: true, supportsDelete: true }; this.icon = _options.icon; this.createdAt = new Date(metadata.startTime); this.title = observableValue('title', metadata.summary || `Session ${rawId.substring(0, 8)}`); @@ -1005,7 +1005,7 @@ class NewSession extends Disposable { lastTurnEnd, mainChat: this._mainChat, chats, - capabilities: { supportsMultipleChats: false, supportsRename: true }, + capabilities: { supportsMultipleChats: false, supportsRename: true, supportsDelete: true }, }; this.sessionId = this.session.sessionId; @@ -2294,15 +2294,34 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement } async deleteSession(sessionId: string): Promise { - const rawId = this._rawIdFromChatId(sessionId); - const cached = rawId ? this._sessionCache.get(rawId) : undefined; + await this.deleteSessions([sessionId]); + } + + async deleteSessions(sessionIds: readonly string[]): Promise { const connection = this.connection; - if (cached && rawId && connection) { + if (!connection) { + return; + } + const targets: { rawId: string; sessionId: string; cached: AgentHostSessionAdapter }[] = []; + for (const sessionId of sessionIds) { + const rawId = this._rawIdFromChatId(sessionId); + const cached = rawId ? this._sessionCache.get(rawId) : undefined; + if (cached && rawId) { + targets.push({ rawId, sessionId, cached }); + } + } + if (targets.length === 0) { + return; + } + for (const { rawId, sessionId, cached } of targets) { await connection.disposeSession(AgentSession.uri(cached.agentProvider, rawId)); this._sessionCache.delete(rawId); this._runningSessionConfigs.delete(sessionId); this._runningSessionConfigResolveSeq.delete(sessionId); - this._onDidChangeSessions.fire({ added: [], removed: [cached], changed: [] }); + } + const removed = targets.map(target => target.cached); + this._onDidChangeSessions.fire({ added: [], removed, changed: [] }); + for (const cached of removed) { cached.dispose(); } } diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts index 5ac8fa6107f..d5c1b327db0 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts @@ -1671,6 +1671,24 @@ suite('LocalAgentHostSessionsProvider', () => { assert.strictEqual(provider.getSessions().find(s => s.title.get() === 'To Delete'), undefined); }); + test('deleteSessions disposes all sessions and removes them from cache', async () => { + const provider = createProvider(disposables, agentHost); + fireSessionAdded(agentHost, 'del-1', { title: 'First' }); + fireSessionAdded(agentHost, 'del-2', { title: 'Second' }); + + const first = provider.getSessions().find(s => s.title.get() === 'First'); + const second = provider.getSessions().find(s => s.title.get() === 'Second'); + assert.ok(first); + assert.ok(second); + + await provider.deleteSessions([first!.sessionId, second!.sessionId]); + + assert.strictEqual(agentHost.disposedSessions.length, 2); + assert.deepStrictEqual(agentHost.disposedSessions.map(uri => AgentSession.id(uri)).sort(), ['del-1', 'del-2']); + assert.strictEqual(provider.getSessions().find(s => s.title.get() === 'First'), undefined); + assert.strictEqual(provider.getSessions().find(s => s.title.get() === 'Second'), undefined); + }); + // ---- Rename ------- test('renameSession dispatches SessionTitleChanged on the session channel', async () => { diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsActions.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsActions.ts index 1265cf0efab..c72931b8f3c 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsActions.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsActions.ts @@ -11,18 +11,15 @@ import { localize2 } from '../../../../../nls.js'; import { IActionViewItemService } from '../../../../../platform/actions/browser/actionViewItemService.js'; import { Action2, registerAction2 } from '../../../../../platform/actions/common/actions.js'; import { ContextKeyExpr, IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js'; -import { IInstantiationService, ServicesAccessor } from '../../../../../platform/instantiation/common/instantiation.js'; +import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { IWorkbenchContribution, WorkbenchPhase, registerWorkbenchContribution2 } from '../../../../../workbench/common/contributions.js'; import { Menus } from '../../../../browser/menus.js'; -import { ActiveSessionHasGitRepositoryContext, ActiveSessionProviderIdContext, ActiveSessionTypeContext, ChatSessionProviderIdContext, IsNewChatSessionContext } from '../../../../common/contextkeys.js'; +import { ActiveSessionHasGitRepositoryContext, ActiveSessionProviderIdContext, ActiveSessionTypeContext, IsNewChatSessionContext } from '../../../../common/contextkeys.js'; import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; -import { ISession } from '../../../../services/sessions/common/session.js'; -import { ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; -import { SessionItemContextMenuId } from '../../../sessions/browser/views/sessionsList.js'; import { BranchPicker } from './branchPicker.js'; import { ClaudePermissionModePicker } from './claudePermissionModePicker.js'; -import { ClaudeCodeSessionType, COPILOT_PROVIDER_ID, CopilotChatSessionsProvider, CopilotCloudSessionType } from './copilotChatSessionsProvider.js'; +import { ClaudeCodeSessionType, COPILOT_PROVIDER_ID, CopilotChatSessionsProvider } from './copilotChatSessionsProvider.js'; import { LocalSessionType } from '../../localChatSessions/browser/localChatSessionsProvider.js'; import { IsolationPicker } from './isolationPicker.js'; import { ModePicker, ModePickerModel } from './modePicker.js'; @@ -281,33 +278,3 @@ class CopilotActiveSessionContribution extends Disposable implements IWorkbenchC registerWorkbenchContribution2(CopilotPickerActionViewItemContribution.ID, CopilotPickerActionViewItemContribution, WorkbenchPhase.AfterRestored); registerWorkbenchContribution2(CopilotActiveSessionContribution.ID, CopilotActiveSessionContribution, WorkbenchPhase.AfterRestored); - -registerAction2(class DeleteSessionAction extends Action2 { - constructor() { - super({ - id: 'sessionsViewPane.copilot.deleteSession', - title: localize2('deleteSession', "Delete..."), - menu: [{ - id: SessionItemContextMenuId, - group: '1_edit', - order: 4, - when: ContextKeyExpr.and( - ContextKeyExpr.equals(ChatSessionProviderIdContext.key, COPILOT_PROVIDER_ID), - ContextKeyExpr.notEquals('chatSessionType', ClaudeCodeSessionType.id), - ContextKeyExpr.notEquals('chatSessionType', LocalSessionType.id), - ContextKeyExpr.notEquals('chatSessionType', CopilotCloudSessionType.id), - ), - }] - }); - } - async run(accessor: ServicesAccessor, context?: ISession | ISession[]): Promise { - if (!context) { - return; - } - const sessions = Array.isArray(context) ? context : [context]; - const sessionsManagementService = accessor.get(ISessionsManagementService); - for (const session of sessions) { - await sessionsManagementService.deleteSession(session); - } - } -}); diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts index 1885ddf84f0..9f94da7f5b5 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts @@ -1816,24 +1816,18 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions return; } - // Confirm deletion - const confirmed = await this.dialogService.confirm({ - message: localize('deleteSession.confirm', "Are you sure you want to delete this session?"), - detail: agentSessions.length > 1 - ? localize('deleteSession.detailMultiple', "This will delete all {0} chats in this session. This action cannot be undone.", agentSessions.length) - : localize('deleteSession.detail', "This action cannot be undone."), - primaryButton: localize('deleteSession.delete', "Delete") - }); - if (!confirmed.confirmed) { - return; - } - await this._deleteAgentSessions(agentSessions); this._sessionGroupCache.delete(sessionId); this._refreshSessionCache(); } + async deleteSessions(sessionIds: readonly string[]): Promise { + for (const sessionId of sessionIds) { + await this.deleteSession(sessionId); + } + } + async renameChat(sessionId: string, chatUri: URI, title: string): Promise { const agentSession = this.agentSessionsService.getSession(chatUri); if (agentSession?.providerType === CopilotCLISessionType.id) { @@ -2938,6 +2932,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions capabilities: { supportsMultipleChats: primaryChat.sessionType === CopilotCLISessionType.id && this._isMultiChatEnabled(), supportsRename: this._sessionTypeSupportsRename(primaryChat.sessionType), + supportsDelete: this._sessionTypeSupportsDelete(primaryChat.sessionType), // Cloud-agent sessions run worktreeCreated tasks server-side during // environment provisioning, so the agents-window dispatcher must // not re-run them. CLI / local sessions don't. @@ -2978,6 +2973,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions capabilities: { supportsMultipleChats: false, supportsRename: this._sessionTypeSupportsRename(chat.sessionType), + supportsDelete: this._sessionTypeSupportsDelete(chat.sessionType), runsWorktreeCreatedTasks: chat.sessionType === CopilotCloudSessionType.id, }, }; @@ -2991,6 +2987,10 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions return sessionType === CopilotCLISessionType.id || sessionType === AgentSessionProviders.Claude; } + private _sessionTypeSupportsDelete(sessionType: string): boolean { + return sessionType === CopilotCLISessionType.id; + } + private _toChat(chat: ICopilotChatSession, resource?: URI): IChat { return { resource: resource ?? chat.resource, diff --git a/src/vs/sessions/contrib/providers/localChatSessions/LOCAL_CHAT_SESSIONS_PROVIDER.md b/src/vs/sessions/contrib/providers/localChatSessions/LOCAL_CHAT_SESSIONS_PROVIDER.md index 9888cf530cb..a18a3164f7c 100644 --- a/src/vs/sessions/contrib/providers/localChatSessions/LOCAL_CHAT_SESSIONS_PROVIDER.md +++ b/src/vs/sessions/contrib/providers/localChatSessions/LOCAL_CHAT_SESSIONS_PROVIDER.md @@ -123,7 +123,7 @@ A `MutableDisposable` on `LocalSession` ensures repeated `trackModel` calls don' - **`createNewChat`** — for the current new session, returns the already-prepared `IChat` and updates `mainChat`. For an existing committed session, creates a subsequent (child) chat linked to the primary via `parentResource`. - **`deleteChat`** — removes a single child chat from a multi-chat session after a confirmation dialog; deleting the primary (or the last remaining chat) removes the whole session. An unknown/stale chat URI is a no-op. -A **"Delete..."** session context-menu action (registered in `localChatSessions.contribution.ts`, gated on `LOCAL_PROVIDER_ID`) delegates to the shared `confirmAndDeleteSessions` helper, which confirms and then calls `ISessionsManagementService.deleteSession` (routing to `deleteSession` above). +Local sessions advertise `capabilities.supportsDelete`, so the shared sessions-list **"Delete..."** action (contributed by the sessions workbench, gated on `ChatSessionSupportsDeleteContext`) confirms and then calls `ISessionsManagementService.deleteSessions` (routing to `deleteSession`/`deleteSessions` above). There is no provider-specific delete action. ## Multi-Chat Support diff --git a/src/vs/sessions/contrib/providers/localChatSessions/browser/localChatSessions.contribution.ts b/src/vs/sessions/contrib/providers/localChatSessions/browser/localChatSessions.contribution.ts index 368a1f19999..54145042bf3 100644 --- a/src/vs/sessions/contrib/providers/localChatSessions/browser/localChatSessions.contribution.ts +++ b/src/vs/sessions/contrib/providers/localChatSessions/browser/localChatSessions.contribution.ts @@ -4,26 +4,21 @@ *--------------------------------------------------------------------------------------------*/ import { IWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase } from '../../../../../workbench/common/contributions.js'; -import { IInstantiationService, ServicesAccessor } from '../../../../../platform/instantiation/common/instantiation.js'; +import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { Disposable, IDisposable } from '../../../../../base/common/lifecycle.js'; -import { LocalChatSessionsProvider, LOCAL_PROVIDER_ID, LOCAL_SESSION_ENABLED_SETTING } from './localChatSessionsProvider.js'; +import { LocalChatSessionsProvider, LOCAL_SESSION_ENABLED_SETTING } from './localChatSessionsProvider.js'; import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; import { Registry } from '../../../../../platform/registry/common/platform.js'; import { IConfigurationRegistry, Extensions as ConfigurationExtensions } from '../../../../../platform/configuration/common/configurationRegistry.js'; -import { localize, localize2 } from '../../../../../nls.js'; +import { localize } from '../../../../../nls.js'; import { ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; import { ForkConversationAction } from '../../../../../workbench/contrib/chat/browser/actions/chatForkActions.js'; -import { Action2, registerAction2 } from '../../../../../platform/actions/common/actions.js'; -import { ContextKeyExpr } from '../../../../../platform/contextkey/common/contextkey.js'; +import { registerAction2 } from '../../../../../platform/actions/common/actions.js'; import { URI } from '../../../../../base/common/uri.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; import { raceTimeout } from '../../../../../base/common/async.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; -import { ChatSessionProviderIdContext } from '../../../../common/contextkeys.js'; -import { ISession } from '../../../../services/sessions/common/session.js'; -import { confirmAndDeleteSessions } from '../../../sessions/browser/deleteSessionHelper.js'; -import { SessionItemContextMenuId } from '../../../sessions/browser/views/sessionsList.js'; Registry.as(ConfigurationExtensions.Configuration).registerConfiguration({ id: 'sessions', @@ -100,22 +95,3 @@ registerAction2(class extends ForkConversationAction { } }); -registerAction2(class DeleteLocalSessionAction extends Action2 { - constructor() { - super({ - id: 'sessionsViewPane.local.deleteSession', - title: localize2('deleteLocalSession', "Delete..."), - menu: [{ - id: SessionItemContextMenuId, - group: '1_edit', - order: 4, - when: ContextKeyExpr.equals(ChatSessionProviderIdContext.key, LOCAL_PROVIDER_ID), - }] - }); - } - - run(accessor: ServicesAccessor, context?: ISession | ISession[]): Promise { - return confirmAndDeleteSessions(accessor, context); - } -}); - diff --git a/src/vs/sessions/contrib/providers/localChatSessions/browser/localChatSessionsProvider.ts b/src/vs/sessions/contrib/providers/localChatSessions/browser/localChatSessionsProvider.ts index ba106504a47..3e4c0472184 100644 --- a/src/vs/sessions/contrib/providers/localChatSessions/browser/localChatSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/localChatSessions/browser/localChatSessionsProvider.ts @@ -805,6 +805,12 @@ export class LocalChatSessionsProvider extends Disposable implements ISessionsPr this._onDidChangeSessions.fire({ added: [], removed: [groupISession], changed: [] }); } + async deleteSessions(sessionIds: readonly string[]): Promise { + for (const sessionId of sessionIds) { + await this.deleteSession(sessionId); + } + } + async deleteChat(sessionId: string, chatUri: URI): Promise { const primary = this._findSession(sessionId); if (!primary || primary.parentResource) { @@ -1203,6 +1209,7 @@ export class LocalChatSessionsProvider extends Disposable implements ISessionsPr capabilities: { supportsMultipleChats: true, supportsRename: true, + supportsDelete: true, }, }; } diff --git a/src/vs/sessions/contrib/sessions/browser/deleteSessionHelper.ts b/src/vs/sessions/contrib/sessions/browser/deleteSessionHelper.ts deleted file mode 100644 index 9e6d9be6509..00000000000 --- a/src/vs/sessions/contrib/sessions/browser/deleteSessionHelper.ts +++ /dev/null @@ -1,50 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { toErrorMessage } from '../../../../base/common/errorMessage.js'; -import { localize } from '../../../../nls.js'; -import { IDialogService } from '../../../../platform/dialogs/common/dialogs.js'; -import { ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js'; -import { ISession } from '../../../services/sessions/common/session.js'; -import { ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js'; - -/** - * Confirms with the user, then permanently deletes the given sessions via - * {@link ISessionsManagementService.deleteSession}. Used by provider-specific - * "Delete..." context-menu actions whose backends do not surface their own - * confirmation dialog (agent host and local providers). Failures are surfaced - * via an error dialog and do not abort the remaining deletions. - */ -export async function confirmAndDeleteSessions(accessor: ServicesAccessor, context: ISession | ISession[] | undefined): Promise { - if (!context) { - return; - } - const sessions = Array.isArray(context) ? context : [context]; - if (sessions.length === 0) { - return; - } - - const dialogService = accessor.get(IDialogService); - const sessionsManagementService = accessor.get(ISessionsManagementService); - - const confirmed = await dialogService.confirm({ - message: sessions.length === 1 - ? localize('deleteSession.confirm', "Are you sure you want to delete this session?") - : localize('deleteSessions.confirm', "Are you sure you want to delete {0} sessions?", sessions.length), - detail: localize('deleteSession.detail', "This action cannot be undone."), - primaryButton: localize('deleteSession.delete', "Delete") - }); - if (!confirmed.confirmed) { - return; - } - - for (const session of sessions) { - try { - await sessionsManagementService.deleteSession(session); - } catch (err) { - dialogService.error(localize('deleteSession.error', "Failed to delete session: {0}", toErrorMessage(err))); - } - } -} diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts index dc22f78837c..bbb4542bd7c 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts @@ -26,7 +26,7 @@ import { MenuWorkbenchToolBar } from '../../../../../platform/actions/browser/to import { ICommandService } from '../../../../../platform/commands/common/commands.js'; import { IContextKeyService, RawContextKey } from '../../../../../platform/contextkey/common/contextkey.js'; import { MarshalledId } from '../../../../../base/common/marshallingIds.js'; -import { ChatSessionProviderIdContext, ChatSessionSupportsRenameContext, ChatSessionTypeContext, IsPhoneLayoutContext, SessionIsArchivedContext, SessionIsReadContext } from '../../../../common/contextkeys.js'; +import { ChatSessionProviderIdContext, ChatSessionSupportsDeleteContext, ChatSessionSupportsRenameContext, ChatSessionTypeContext, IsPhoneLayoutContext, SessionIsArchivedContext, SessionIsReadContext } from '../../../../common/contextkeys.js'; import { IContextMenuService } from '../../../../../platform/contextview/browser/contextView.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { IKeybindingService } from '../../../../../platform/keybinding/common/keybinding.js'; @@ -1476,6 +1476,7 @@ export class SessionsList extends Disposable implements ISessionsList { [ChatSessionTypeContext.key, element.sessionType], [ChatSessionProviderIdContext.key, element.providerId], [ChatSessionSupportsRenameContext.key, element.capabilities.supportsRename ?? false], + [ChatSessionSupportsDeleteContext.key, element.capabilities.supportsDelete ?? false], ]; const menu = this.menuService.createMenu(SessionItemContextMenuId, this.contextKeyService.createOverlay(contextOverlay)); diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts index b960833777b..f44a58f95dc 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Codicon } from '../../../../../base/common/codicons.js'; +import { toErrorMessage } from '../../../../../base/common/errorMessage.js'; import { KeyCode, KeyMod } from '../../../../../base/common/keyCodes.js'; import { isMobile, isWeb } from '../../../../../base/common/platform.js'; import { localize, localize2 } from '../../../../../nls.js'; @@ -19,7 +20,7 @@ import { IViewsService } from '../../../../../workbench/services/views/common/vi import { CLOSE_MOBILE_SIDEBAR_DRAWER_COMMAND_ID } from '../../../../browser/workbench.js'; import { EditorsVisibleContext, EditorAreaFocusContext, IsSessionsWindowContext } from '../../../../../workbench/common/contextkeys.js'; import { SessionsCategories } from '../../../../common/categories.js'; -import { ChatSessionSupportsRenameContext, IsActiveSessionArchivedContext, IsNewChatSessionContext, SessionIsArchivedContext, SessionIsCreatedContext, SessionIsReadContext } from '../../../../common/contextkeys.js'; +import { ChatSessionSupportsDeleteContext, ChatSessionSupportsRenameContext, IsActiveSessionArchivedContext, IsNewChatSessionContext, SessionIsArchivedContext, SessionIsCreatedContext, SessionIsReadContext } from '../../../../common/contextkeys.js'; import { SessionItemToolbarMenuId, SessionItemContextMenuId, SessionSectionToolbarMenuId, SessionSectionTypeContext, IsSessionPinnedContext, SessionsGrouping, SessionsSorting, ISessionSection } from './sessionsList.js'; import { ISession, SessionStatus } from '../../../../services/sessions/common/session.js'; import { IsWorkspaceGroupCappedContext, SessionsViewFilterOptionsSubMenu, SessionsViewFilterSubMenu, SessionsViewGroupingContext, SessionsViewId, SessionsView, SessionsViewSortingContext, openSessionToTheSide } from './sessionsView.js'; @@ -733,6 +734,52 @@ registerAction2(class RenameSessionAction extends Action2 { } }); +registerAction2(class DeleteSessionAction extends Action2 { + constructor() { + super({ + id: 'sessionsViewPane.deleteSession', + title: localize2('deleteSession', "Delete..."), + menu: [{ + id: SessionItemContextMenuId, + group: '1_edit', + order: 4, + when: ChatSessionSupportsDeleteContext, + }] + }); + } + async run(accessor: ServicesAccessor, context?: ISession | ISession[]): Promise { + if (!context) { + return; + } + const sessions = (Array.isArray(context) ? context : [context]).filter(session => session.capabilities.supportsDelete); + if (sessions.length === 0) { + return; + } + + const dialogService = accessor.get(IDialogService); + const sessionsManagementService = accessor.get(ISessionsManagementService); + + const confirmed = await dialogService.confirm({ + message: sessions.length === 1 + ? localize('deleteSession.confirm', "Are you sure you want to delete this session?") + : localize('deleteSessions.confirm', "Are you sure you want to delete {0} sessions?", sessions.length), + detail: localize('deleteSession.detail', "This action cannot be undone."), + primaryButton: localize('deleteSession.delete', "Delete") + }); + if (!confirmed.confirmed) { + return; + } + + try { + await sessionsManagementService.deleteSessions(sessions); + } catch (err) { + dialogService.error(sessions.length === 1 + ? localize('deleteSession.error', "Failed to delete the session: {0}", toErrorMessage(err)) + : localize('deleteSessions.error', "Failed to delete the sessions: {0}", toErrorMessage(err))); + } + } +}); + registerAction2(class MarkSessionReadAction extends Action2 { constructor() { super({ diff --git a/src/vs/sessions/services/sessions/browser/sessionsManagementService.ts b/src/vs/sessions/services/sessions/browser/sessionsManagementService.ts index 0302d92a69f..4043d7a0317 100644 --- a/src/vs/sessions/services/sessions/browser/sessionsManagementService.ts +++ b/src/vs/sessions/services/sessions/browser/sessionsManagementService.ts @@ -501,6 +501,38 @@ export class SessionsManagementService extends Disposable implements ISessionsMa this._onDidDeleteSession.fire(session); } + async deleteSessions(sessions: readonly ISession[]): Promise { + const byProvider = new Map(); + for (const session of sessions) { + const provider = this._getProvider(session); + if (!provider) { + continue; + } + const group = byProvider.get(provider); + if (group) { + group.push(session); + } else { + byProvider.set(provider, [session]); + } + } + + let firstError: unknown; + for (const [provider, providerSessions] of byProvider) { + try { + await provider.deleteSessions(providerSessions.map(session => session.sessionId)); + for (const session of providerSessions) { + this._onDidDeleteSession.fire(session); + } + } catch (error) { + firstError ??= error; + } + } + + if (firstError !== undefined) { + throw firstError; + } + } + async deleteChat(session: ISession, chatUri: URI): Promise { await this._getProvider(session)?.deleteChat(session.sessionId, chatUri); this._onDidDeleteChat.fire(session); diff --git a/src/vs/sessions/services/sessions/common/session.ts b/src/vs/sessions/services/sessions/common/session.ts index d4e3e34e114..a1ac76c48d6 100644 --- a/src/vs/sessions/services/sessions/common/session.ts +++ b/src/vs/sessions/services/sessions/common/session.ts @@ -381,6 +381,13 @@ export interface ISessionCapabilities { * Defaults to falsy (not renameable) when omitted. */ readonly supportsRename?: boolean; + /** + * Whether this session can be deleted. The agents-window sessions-list + * `Delete...` action gates on this flag rather than on the provider id, + * so delete is offered exactly where the backing provider supports it. + * Defaults to falsy (not deletable) when omitted. + */ + readonly supportsDelete?: boolean; /** * Whether the session's underlying runtime (e.g. a cloud agent host) * already runs `runOptions.runOn === 'worktreeCreated'` tasks during diff --git a/src/vs/sessions/services/sessions/common/sessionsManagement.ts b/src/vs/sessions/services/sessions/common/sessionsManagement.ts index 3b0cecbe23c..fb889e5b333 100644 --- a/src/vs/sessions/services/sessions/common/sessionsManagement.ts +++ b/src/vs/sessions/services/sessions/common/sessionsManagement.ts @@ -280,6 +280,15 @@ export interface ISessionsManagementService { /** Delete a session. */ deleteSession(session: ISession): Promise; + /** + * Delete multiple sessions at once. + * + * Groups the sessions by provider and deletes each group through its + * provider's batch {@link ISessionsProvider.deleteSessions}. Fires + * {@link onDidDeleteSession} once per deleted session. + */ + deleteSessions(sessions: readonly ISession[]): Promise; + /** Delete a single chat from a session by its URI. */ deleteChat(session: ISession, chatUri: URI): Promise; diff --git a/src/vs/sessions/services/sessions/common/sessionsProvider.ts b/src/vs/sessions/services/sessions/common/sessionsProvider.ts index 70cc3ad7039..83c3fadbe53 100644 --- a/src/vs/sessions/services/sessions/common/sessionsProvider.ts +++ b/src/vs/sessions/services/sessions/common/sessionsProvider.ts @@ -231,6 +231,14 @@ export interface ISessionsProvider { */ deleteSession(sessionId: string): Promise; + /** + * Delete multiple sessions at once. Implementations may delete the + * sessions more efficiently in a batch, or simply delegate to + * {@link deleteSession} for each id. + * @param sessionIds The IDs of the sessions to delete. + */ + deleteSessions(sessionIds: readonly string[]): Promise; + /** * Delete a single chat from a session. * @param sessionId The ID of the session containing the chat to delete. 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 89979a4aa7f..4c60198fc36 100644 --- a/src/vs/sessions/services/sessions/test/browser/sessionNavigation.test.ts +++ b/src/vs/sessions/services/sessions/test/browser/sessionNavigation.test.ts @@ -210,6 +210,7 @@ class MockSessionStore implements ISessionsManagementService { archiveSession(_session: ISession): Promise { throw new Error('not implemented'); } unarchiveSession(_session: ISession): Promise { throw new Error('not implemented'); } deleteSession(_session: ISession): Promise { throw new Error('not implemented'); } + deleteSessions(_sessions: readonly ISession[]): Promise { throw new Error('not implemented'); } deleteChat(_session: ISession, _chatUri: URI): Promise { throw new Error('not implemented'); } renameChat(_session: ISession, _chatUri: URI, _title: string): Promise { throw new Error('not implemented'); } renameSession(_session: ISession, _title: string): Promise { throw new Error('not implemented'); } diff --git a/src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts b/src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts index bef86968b8b..27703941243 100644 --- a/src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts +++ b/src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts @@ -153,6 +153,7 @@ class TestSessionsProvider extends mock() { override async archiveSession(): Promise { } override async unarchiveSession(): Promise { } override async deleteSession(): Promise { } + override async deleteSessions(_sessionIds: readonly string[]): Promise { } override async deleteChat(): Promise { } override deleteNewSession(): void { } override async sendRequest(_sessionId: string, _chatResource: URI, _options: ISendRequestOptions): Promise { return this._session; } @@ -902,6 +903,63 @@ suite('SessionsManagementService', () => { assert.strictEqual(view.activeSession.get()?.sessionId, 'b'); }); + suite('deleteSessions', () => { + + class RecordingProvider extends TestSessionsProvider { + readonly deleted: string[][] = []; + constructor(public override readonly id: string, private readonly _fail: boolean, session: ISession) { + super(session); + } + override async deleteSessions(sessionIds: readonly string[]): Promise { + this.deleted.push([...sessionIds]); + if (this._fail) { + throw new Error(`${this.id} failed`); + } + } + } + + function createService(providers: ISessionsProvider[]): ISessionsManagementService { + const instantiationService = disposables.add(new TestInstantiationService()); + instantiationService.stub(IStorageService, disposables.add(new InMemoryStorageService())); + instantiationService.stub(ILogService, new NullLogService()); + instantiationService.stub(IContextKeyService, disposables.add(new MockContextKeyService())); + instantiationService.stub(ISessionsProvidersService, new TestSessionsProvidersService(providers)); + instantiationService.stub(IUriIdentityService, { extUri: extUriBiasedIgnorePathCase }); + instantiationService.stub(IChatWidgetService, new TestChatWidgetService()); + instantiationService.stub(IProgressService, new TestProgressService()); + instantiationService.stub(IChatService, new class extends mock() { + override readonly onDidSubmitRequest = Event.None; + }); + instantiationService.stub(IChatWidgetHistoryService, new class extends mock() { + override moveHistory(): void { } + }); + return disposables.add(instantiationService.createInstance(SessionsManagementService)); + } + + test('groups sessions by provider and continues when one provider fails (best-effort)', async () => { + const s1 = stubSession({ sessionId: 's1', providerId: 'p1' }); + const s2 = stubSession({ sessionId: 's2', providerId: 'p2' }); + const failing = new RecordingProvider('p1', true, s1); + const succeeding = new RecordingProvider('p2', false, s2); + const service = createService([failing, succeeding]); + + const deleted: string[] = []; + disposables.add(service.onDidDeleteSession(session => deleted.push(session.sessionId))); + + await assert.rejects(service.deleteSessions([s1, s2]), /p1 failed/); + + assert.deepStrictEqual({ + failingDeleted: failing.deleted, + succeedingDeleted: succeeding.deleted, + eventsFired: deleted, + }, { + failingDeleted: [['s1']], + succeedingDeleted: [['s2']], + eventsFired: ['s2'], + }); + }); + }); + suite('createNewChatInSession', () => { test('reuses an existing untitled chat instead of creating a new one', async () => { diff --git a/src/vs/sessions/sessions.desktop.main.ts b/src/vs/sessions/sessions.desktop.main.ts index dc7a37043fd..92ad886d74f 100644 --- a/src/vs/sessions/sessions.desktop.main.ts +++ b/src/vs/sessions/sessions.desktop.main.ts @@ -223,7 +223,6 @@ import './contrib/providers/agentHost/browser/localAgentHost.contribution.js'; import './contrib/providers/agentHost/browser/agentSessionSettings.contribution.js'; import './contrib/providers/agentHost/browser/agentHostSettings.contribution.js'; import './contrib/providers/agentHost/browser/agentHostSessionBranchActions.js'; -import './contrib/providers/agentHost/browser/agentHostSessionDeleteAction.js'; import './contrib/providers/agentHost/browser/agentHostSkillButtons.js'; import './contrib/providers/agentHost/electron-browser/agentHost.contribution.js'; diff --git a/src/vs/sessions/sessions.web.main.ts b/src/vs/sessions/sessions.web.main.ts index 4c540b143be..7f9d9f60685 100644 --- a/src/vs/sessions/sessions.web.main.ts +++ b/src/vs/sessions/sessions.web.main.ts @@ -163,7 +163,6 @@ import './contrib/providers/remoteAgentHost/browser/remoteAgentHostActions.js'; import './contrib/providers/agentHost/browser/agentSessionSettings.contribution.js'; import './contrib/providers/agentHost/browser/agentHostSettings.contribution.js'; import './contrib/providers/agentHost/browser/agentHostSessionBranchActions.js'; -import './contrib/providers/agentHost/browser/agentHostSessionDeleteAction.js'; import './contrib/providers/agentHost/browser/agentHostSkillButtons.js'; // Host filter dropdown in the titlebar (scopes the sessions list to a host)