From 9e5fb43b37730fb5b15d69dde2f4a43d6c4b888e Mon Sep 17 00:00:00 2001 From: Rohan Malpani Date: Thu, 18 Jun 2026 09:42:23 -0700 Subject: [PATCH 1/7] Update new chat in session tip text Reword the tip shown above the prompt box when creating a new chat within a session to clearly convey it is a parallel conversation that builds on the session's changes. --- .../sessions/contrib/chat/browser/newChatInSessionWidget.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/newChatInSessionWidget.ts b/src/vs/sessions/contrib/chat/browser/newChatInSessionWidget.ts index a528c0da3931c..a149295ab6adb 100644 --- a/src/vs/sessions/contrib/chat/browser/newChatInSessionWidget.ts +++ b/src/vs/sessions/contrib/chat/browser/newChatInSessionWidget.ts @@ -107,7 +107,7 @@ export class NewChatInSessionWidget extends Disposable { const tipContainer = dom.append(container, dom.$('.sub-session-tip-container')); const tipWidget = dom.append(tipContainer, dom.$('.sub-session-tip-widget')); tipWidget.setAttribute('role', 'status'); - tipWidget.setAttribute('aria-label', localize('subSessionTip.ariaLabel', "Sub-session tip")); + tipWidget.setAttribute('aria-label', localize('subSessionTip.ariaLabel', "New chat tip")); // Tip icon const iconEl = dom.append(tipWidget, renderIcon(Codicon.lightbulb)); @@ -117,7 +117,7 @@ export class NewChatInSessionWidget extends Disposable { const textEl = dom.append(tipWidget, dom.$('span.sub-session-tip-text')); textEl.textContent = localize( 'subSessionTip.message', - "This is a sub-session, a new chat in the same workspace. Use it to ask questions, run tasks, or explore ideas with fresh context." + "Start a parallel conversation to build on all the changes made in this session." ); // Dismiss button From 4c2511ba0c1627a4126e971dc4e02464c29e2d28 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Fri, 19 Jun 2026 00:26:32 +0200 Subject: [PATCH 2/7] agent host: persist and restore peer chats across restart (#321989) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * agentHost: persist and restore peer chats across restart Peer (additional) chats in a Copilot CLI session were in-memory only and disappeared on restart. Persist each peer chat's backing SDK conversation id in the session DB, resume it on session restore, and re-register it (with history) in the state-manager chat catalog. - copilotAgent: persist {chatId -> {sdkSessionId, model}} under copilot.chats; add getChats, _ensureChatSession (resume from persisted id), and make disposeChat delete the persisted entry + SDK conversation. - IAgent: add optional getChats. - agentHostStateManager: add restoreChat to seed a peer chat's ChatState and add it to the catalog. - agentService: restoreSession now restores persisted peer chats with history. - Tests for getChats/disposeChat persistence and restore re-registration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: address CCR feedback on peer-chat persistence - Validate persisted peer-chat entries on read; drop ones without a usable string sdkSessionId instead of letting invalid ids reach deleteSession. - Build the serialized catalog with a null-prototype object to avoid prototype pollution from a client-chosen chatId (e.g. __proto__). - Clarify the disposeChat comment (it resolves the SDK id, it does not resume). - Add a regression test covering corrupted/invalid persisted entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: fix restored peer chat stuck loading after reload Switching to a restored peer chat after a window reload left the chat on a perpetual loading spinner. Disposing the outgoing default chat session ran _releaseSessionSubscription(session), which tore down ALL chat subscriptions of the backend session — including the incoming peer chat's subscription that provideChatSessionContent was concurrently awaiting. _whenSubscriptionHydrated then waited forever on a disposed subscription, so the chat content promise never resolved. A chat session's dispose now releases only its own conversation subscription via _releaseChatSessionSubscriptions, never a sibling peer chat's. The shared session subscription (and its lockstep default-chat subscription) is kept alive while any sibling chat session is still active or mid-hydration (tracked by _hydratingChatSessions). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: confirm before deleting a chat Deleting an individual chat in a Local/Remote Agent Host session (Copilot CLI) removed the chat immediately with no confirmation, unlike the Copilot Chat CLI session provider. Show the same confirmation dialog before disposing the chat, and bail out when the user cancels. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../platform/agentHost/common/agentService.ts | 8 + .../agentHost/node/agentHostStateManager.ts | 28 +++ .../platform/agentHost/node/agentService.ts | 53 +++++ .../agentHost/node/copilot/copilotAgent.ts | 196 ++++++++++++++++- .../agentHost/test/node/agentService.test.ts | 40 +++- .../agentHost/test/node/copilotAgent.test.ts | 75 +++++++ .../browser/baseAgentHostSessionsProvider.ts | 11 + .../browser/localAgentHostSessionsProvider.ts | 4 +- .../localAgentHostSessionsProvider.test.ts | 48 ++++- .../remoteAgentHostSessionsProvider.ts | 5 +- .../remoteAgentHostSessionsProvider.test.ts | 3 +- .../agentHost/agentHostSessionHandler.ts | 202 ++++++++++++------ .../agentHostChatContribution.test.ts | 27 ++- 13 files changed, 628 insertions(+), 72 deletions(-) diff --git a/src/vs/platform/agentHost/common/agentService.ts b/src/vs/platform/agentHost/common/agentService.ts index ba11d6668d3fe..055e77bf7badd 100644 --- a/src/vs/platform/agentHost/common/agentService.ts +++ b/src/vs/platform/agentHost/common/agentService.ts @@ -886,6 +886,14 @@ export interface IAgent { */ disposeChat?(session: URI, chat: URI): Promise; + /** + * Returns the persisted catalog of additional (non-default) peer chats for a + * session as their channel URIs. Used to re-register peer chats (and seed + * their history) when a session is restored after a process restart. + * Optional: harnesses without multi-chat persistence omit it. + */ + getChats?(session: URI): Promise; + /** * Called when the session's pending (steering) message changes. * The agent harness decides how to react — e.g. inject steering diff --git a/src/vs/platform/agentHost/node/agentHostStateManager.ts b/src/vs/platform/agentHost/node/agentHostStateManager.ts index 3309b486a8be3..1ad4e57113745 100644 --- a/src/vs/platform/agentHost/node/agentHostStateManager.ts +++ b/src/vs/platform/agentHost/node/agentHostStateManager.ts @@ -476,6 +476,34 @@ export class AgentHostStateManager extends Disposable { return chatSummary; } + /** + * Re-registers an additional (non-default) peer chat when a session is + * restored from persistent storage, seeding its {@link ChatState} with the + * supplied turns. Unlike {@link addChat} this does not snapshot the session + * title onto the default chat (the default chat's persisted title is + * restored independently) and it seeds history. The catalog entry is added + * in place so the object identity returned by {@link restoreSession} stays + * live; no {@link ActionType.SessionChatAdded} is dispatched because restore + * runs before clients subscribe. + */ + restoreChat(session: URI, chatUri: URI, options: { readonly title?: string; readonly turns: Turn[] }): void { + const sessionState = this._sessionStates.get(session); + if (!sessionState) { + this._logService.warn(`[AgentHostStateManager] restoreChat for unknown session: ${session}`); + return; + } + if (sessionState.chats.some(c => c.resource === chatUri)) { + return; + } + const chatSummary: ChatSummary = { + ...createDefaultChatSummary(sessionState.summary, chatUri), + title: options.title ?? '', + status: SessionStatus.Idle, + }; + this._chatStates.set(chatUri, { ...createChatState(chatSummary), turns: options.turns }); + sessionState.chats = [...sessionState.chats, chatSummary]; + } + /** * Removes an additional chat from a session. Deletes its * {@link ChatState}, dispatches {@link ActionType.SessionChatRemoved}, and diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index 61fb2e1009d93..8617a2e5ca9d7 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -1499,6 +1499,11 @@ export class AgentService extends Disposable implements IAgentService { this._stateManager.restoreSession(summary, [...turns]); + // Restore any additional (non-default) peer chats the provider has + // persisted for this session, seeding each with its own history and + // persisted title so they reappear after a process restart. + await this._restorePeerChats(agent, session); + const changesets = buildDefaultChangesetCatalogue(sessionStr); this._stateManager.setSessionChangesets(sessionStr, changesets); @@ -1540,6 +1545,54 @@ export class AgentService extends Disposable implements IAgentService { this._attachGitState(session, meta.workingDirectory); } + /** + * Restores the additional (non-default) peer chats persisted for a session. + * For each chat returned by the provider, loads its history and persisted + * title and re-registers it in the state manager so it reappears in the + * session's chat catalog after a process restart. Best-effort: a chat whose + * history fails to load is restored with no turns rather than dropped. + */ + private async _restorePeerChats(agent: IAgent, session: URI): Promise { + if (!agent.getChats) { + return; + } + let chats: readonly URI[]; + try { + chats = await agent.getChats(session); + } catch (err) { + this._logService.warn(`[AgentService] Failed to enumerate peer chats for ${session.toString()}: ${toErrorMessage(err)}`); + return; + } + if (chats.length === 0) { + return; + } + for (const chatUri of chats) { + let turns: readonly Turn[] = []; + try { + turns = await agent.getSessionMessages(chatUri); + } catch (err) { + this._logService.warn(`[AgentService] Failed to load history for peer chat ${chatUri.toString()}: ${toErrorMessage(err)}`); + } + const title = await this._readPersistedChatTitle(session, chatUri); + this._stateManager.restoreChat(session.toString(), chatUri.toString(), { title, turns: [...turns] }); + } + } + + /** Reads a peer chat's persisted custom title, if any. */ + private async _readPersistedChatTitle(session: URI, chatUri: URI): Promise { + const ref = await this._sessionDataService.tryOpenDatabase?.(session); + if (!ref) { + return undefined; + } + try { + return (await ref.object.getMetadata(`customChatTitle:${chatUri.toString()}`)) ?? undefined; + } catch { + return undefined; + } finally { + ref.dispose(); + } + } + private async _getSessionMetadataForRestore(agent: IAgent, session: URI): Promise { const sessionStr = session.toString(); if (agent.getSessionMetadata) { diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts index aa37993b82811..d33f640f4bcb8 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts @@ -43,7 +43,7 @@ import type { ResolveSessionConfigResult, SessionConfigCompletionsResult } from import { ProtectedResourceMetadata, type AgentSelection, type ChildCustomizationType, type ConfigPropertySchema, type ConfigSchema, type ModelSelection, type ToolDefinition } from '../../common/state/protocol/state.js'; import { ActionType, type SessionAction } from '../../common/state/sessionActions.js'; import { AHP_AUTH_REQUIRED, ProtocolError } from '../../common/state/sessionProtocol.js'; -import { AgentCustomization, CustomizationLoadStatus, CustomizationType, ResponsePartKind, RuleCustomization, ChatInputResponseKind, SkillCustomization, customizationId, isDefaultChatUri, parseChatUri, parseSubagentSessionUri, type ChildCustomization, type ClientPluginCustomization, type Customization, type DirectoryCustomization, type MessageAttachment, type PendingMessage, type PluginCustomization, type PolicyState, type ResponsePart, type ChatInputAnswer, type ToolCallResult, type Turn } from '../../common/state/sessionState.js'; +import { AgentCustomization, CustomizationLoadStatus, CustomizationType, ResponsePartKind, RuleCustomization, ChatInputResponseKind, SkillCustomization, customizationId, buildChatUri, isDefaultChatUri, parseChatUri, parseSubagentSessionUri, type ChildCustomization, type ClientPluginCustomization, type Customization, type DirectoryCustomization, type MessageAttachment, type PendingMessage, type PluginCustomization, type PolicyState, type ResponsePart, type ChatInputAnswer, type ToolCallResult, type Turn } from '../../common/state/sessionState.js'; import { ActiveClientState } from '../activeClientState.js'; import { IAgentConfigurationService } from '../agentConfigurationService.js'; import { IAgentHostCompletions } from '../agentHostCompletions.js'; @@ -128,6 +128,16 @@ interface ISerializedModelSelection { config?: unknown; } +/** + * A persisted additional (non-default) peer chat. Records the SDK conversation + * id that backs the chat so it can be resumed after a process restart, along + * with any model override chosen at creation time. + */ +interface IPersistedChat { + readonly sdkSessionId: string; + readonly model?: ModelSelection; +} + /** * Augments the published `@vscode/copilot-api` `ModelBilling` with the `tokenPrices` field the runtime CAPI `/models` * payload already carries but the SDK type doesn't yet declare. Mirror of `IClaudeModelSupports` in `claudeAgent.ts`. @@ -1298,7 +1308,7 @@ export class CopilotAgent extends Disposable implements IAgent { // Additional (non-default) chats are backed by their own SDK // conversation tracked in `_chatSessions`, keyed by the chat URI. if (chat && !isDefaultChatUri(chat)) { - const entry = this._chatSessions.get(chat.toString()); + const entry = await this._ensureChatSession(session, chat); if (!entry) { throw new Error(`[Copilot] sendMessage for unknown chat: ${chat.toString()}`); } @@ -1417,6 +1427,15 @@ export class CopilotAgent extends Disposable implements IAgent { } async getSessionMessages(session: URI): Promise { + // An additional (non-default) peer chat is addressed by its `ahp-chat` + // channel URI. Resume its backing SDK conversation and return its turns. + const chatInfo = parseChatUri(session); + if (chatInfo && !isDefaultChatUri(session)) { + const parentSession = URI.parse(chatInfo.session); + const entry = await this._ensureChatSession(parentSession, session); + return entry ? entry.getMessages() : []; + } + // If the URI describes a subagent child session (`/subagent/`), // load the parent's events once and extract the child's filtered turns. const subagentInfo = parseSubagentSessionUri(session); @@ -1635,6 +1654,12 @@ export class CopilotAgent extends Disposable implements IAgent { agentSession = this._createAgentSession(launchPlan, workingDirectory, activeClient, chat); await agentSession.initializeSession(); this._chatSessions.set(chatKey, agentSession); + const parsed = parseChatUri(chat); + if (parsed) { + const persisted = await this._readPersistedChats(session); + persisted.set(parsed.chatId, { sdkSessionId: chatSdkId, ...(options?.model ? { model: options.model } : {}) }); + await this._writePersistedChats(session, persisted); + } this._logService.info(`[Copilot] Created additional chat ${chatKey} in session ${session.toString()}`); } catch (error) { agentSession?.dispose(); @@ -1643,11 +1668,114 @@ export class CopilotAgent extends Disposable implements IAgent { }); } - async disposeChat(_session: URI, chat: URI): Promise { + async disposeChat(session: URI, chat: URI): Promise { if (isDefaultChatUri(chat)) { return; } - this._chatSessions.deleteAndDispose(chat.toString()); + const chatKey = chat.toString(); + // Resolve the chat's backing SDK conversation id — from the in-memory + // session if present, otherwise from the persisted catalog — so we can + // delete it from the SDK's on-disk store. Without this a fresh process + // could re-resume an orphaned conversation that no longer has a catalog + // entry. Best-effort: a missing id still drops the catalog entry below. + const parsed = parseChatUri(chat); + let sdkSessionId = this._chatSessions.get(chatKey)?.sessionId; + if (parsed) { + const persisted = await this._readPersistedChats(session); + sdkSessionId ??= persisted.get(parsed.chatId)?.sdkSessionId; + if (persisted.delete(parsed.chatId)) { + await this._writePersistedChats(session, persisted); + } + } + this._chatSessions.deleteAndDispose(chatKey); + if (sdkSessionId) { + try { + const client = await this._ensureClient(); + await client.deleteSession(sdkSessionId); + } catch (err) { + this._logService.warn(`[Copilot] Failed to delete SDK session for chat ${chatKey}: ${err instanceof Error ? err.message : String(err)}`); + } + } + } + + /** + * Returns the catalog of additional (non-default) peer chats persisted for a + * session, as `ahp-chat` channel URIs. Used by the agent service to + * re-register peer chats (and seed their history) when a session is restored + * after a process restart. + */ + async getChats(session: URI): Promise { + const persisted = await this._readPersistedChats(session); + const result: URI[] = []; + for (const chatId of persisted.keys()) { + result.push(URI.parse(buildChatUri(session.toString(), chatId))); + } + return result; + } + + /** + * Returns the SDK-backed {@link CopilotAgentSession} for an additional peer + * chat, resuming its persisted SDK conversation if it is not already in + * memory (e.g. after a process restart). Returns `undefined` when the chat + * has no persisted backing conversation. + */ + private async _ensureChatSession(session: URI, chat: URI): Promise { + const chatKey = chat.toString(); + const existing = this._chatSessions.get(chatKey); + if (existing) { + return existing; + } + const parsed = parseChatUri(chat); + if (!parsed) { + return undefined; + } + const sessionId = AgentSession.id(session); + return this._sessionSequencer.queue(sessionId, async () => { + const again = this._chatSessions.get(chatKey); + if (again) { + return again; + } + const persisted = await this._readPersistedChats(session); + const info = persisted.get(parsed.chatId); + if (!info) { + return undefined; + } + const parentEntry = this._sessions.get(sessionId) ?? await this._resumeSession(sessionId).catch(() => undefined); + const workingDirectory = parentEntry?.workingDirectory + ?? this._provisionalSessions.get(sessionId)?.workingDirectory; + if (!workingDirectory) { + this._logService.warn(`[Copilot] Cannot resume chat ${chatKey}: missing working directory`); + return undefined; + } + const client = await this._ensureClient(); + const activeClient = this._getOrCreateActiveClient(session, workingDirectory); + const snapshot = await activeClient.snapshot(); + const shellManager = this._instantiationService.createInstance(ShellManager, chat, workingDirectory); + const launchPlan: CopilotSessionLaunchPlan = { + kind: 'resume', + client, + sessionId: info.sdkSessionId, + workingDirectory, + resolvedAgentName: undefined, + snapshot, + activeClientState: activeClient.state, + shellManager, + githubToken: this._githubToken, + fallback: { model: info.model }, + }; + let agentSession: CopilotAgentSession | undefined; + try { + agentSession = this._createAgentSession(launchPlan, workingDirectory, activeClient, chat); + await agentSession.initializeSession(); + this._chatSessions.set(chatKey, agentSession); + this._logService.info(`[Copilot] Resumed additional chat ${chatKey} in session ${session.toString()}`); + return agentSession; + } catch (error) { + agentSession?.dispose(); + this._logService.warn(`[Copilot] Failed to resume additional chat ${chatKey}: ${error instanceof Error ? error.message : String(error)}`); + return undefined; + } + }); } async truncateSession(session: URI, turnId?: string): Promise { @@ -2015,6 +2143,66 @@ export class CopilotAgent extends Disposable implements IAgent { private static readonly _META_WORKTREE_BRANCH = 'copilot.worktree.branchName'; private static readonly _META_WORKTREE_PATH = 'copilot.worktree.path'; private static readonly _META_WORKTREE_REPOSITORY_ROOT = 'copilot.worktree.repositoryRoot'; + /** Persisted catalog of additional (non-default) peer chats, keyed by chatId. */ + private static readonly _META_CHATS = 'copilot.chats'; + + /** + * Reads the persisted peer-chat catalog for a session. Each entry maps a + * chatId (the `ahp-chat` authority) to the SDK conversation that backs it + * (and its optional model override), so the chat can be resumed after a + * restart even though {@link _chatSessions} is empty in a fresh process. + */ + private async _readPersistedChats(session: URI): Promise> { + const ref = await this._sessionDataService.tryOpenDatabase(session); + if (!ref) { + return new Map(); + } + try { + const raw = await ref.object.getMetadata(CopilotAgent._META_CHATS); + if (!raw) { + return new Map(); + } + const parsed = JSON.parse(raw) as Record; + const result = new Map(); + for (const [chatId, value] of Object.entries(parsed)) { + // The metadata blob is client-influenced and may be corrupted or + // tampered: drop entries that don't carry a usable SDK session id + // rather than letting an invalid id reach `client.deleteSession`. + if (!value || typeof value !== 'object') { + continue; + } + const { sdkSessionId, model } = value as { sdkSessionId?: unknown; model?: unknown }; + if (typeof sdkSessionId !== 'string' || !sdkSessionId) { + continue; + } + result.set(chatId, { sdkSessionId, ...(model ? { model: model as ModelSelection } : {}) }); + } + return result; + } catch (err) { + this._logService.warn(`[Copilot] Failed to read persisted chats for ${session.toString()}: ${err instanceof Error ? err.message : String(err)}`); + return new Map(); + } finally { + ref.dispose(); + } + } + + /** Writes the persisted peer-chat catalog for a session. */ + private async _writePersistedChats(session: URI, chats: Map): Promise { + const dbRef = this._sessionDataService.openDatabase(session); + try { + // Use a null-prototype object: chatIds derive from a client-chosen + // chat URI authority, so a value like `__proto__` would otherwise + // pollute the prototype / corrupt the serialized payload. + const obj: Record = Object.create(null); + for (const [chatId, info] of chats) { + obj[chatId] = info; + } + await dbRef.object.setMetadata(CopilotAgent._META_CHATS, JSON.stringify(obj)); + } finally { + dbRef.dispose(); + } + } + private async _writeWorktreeMetadata(session: URI, metadata: { branchName: string; baseBranch: string | undefined; worktreePath: URI; repositoryRoot: URI }): Promise { const dbRef = this._sessionDataService.openDatabase(session); diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index 4f6f4e169fba9..4f037b018857c 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -25,7 +25,7 @@ import { AgentSession, GITHUB_COPILOT_PROTECTED_RESOURCE } from '../../common/ag import { ISessionDatabase, ISessionDataService } from '../../common/sessionDataService.js'; import { SessionDatabase } from '../../node/sessionDatabase.js'; import { ActionType, ActionEnvelope } from '../../common/state/sessionActions.js'; -import { ChangesetStatus, CustomizationType, MessageAttachmentKind, MessageKind, SessionActiveClient, ResponsePartKind, ROOT_STATE_URI, SessionLifecycle, SessionStatus, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildChatUri, buildSubagentSessionUri, customizationId, isSubagentSession, type ChangesetState, type MarkdownResponsePart, type ToolCallCompletedState, type ToolCallResponsePart } from '../../common/state/sessionState.js'; +import { ChangesetStatus, CustomizationType, MessageAttachmentKind, MessageKind, SessionActiveClient, ResponsePartKind, ROOT_STATE_URI, SessionLifecycle, SessionStatus, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildChatUri, buildSubagentSessionUri, customizationId, isSubagentSession, type ChangesetState, type MarkdownResponsePart, type ToolCallCompletedState, type ToolCallResponsePart, type Turn } from '../../common/state/sessionState.js'; import { type MessageResourceAttachment } from '../../common/state/protocol/state.js'; import { IProductService } from '../../../product/common/productService.js'; import { AgentService } from '../../node/agentService.js'; @@ -2089,6 +2089,44 @@ suite('AgentService (node dispatcher)', () => { inCatalog: false, }); }); + + test('restoreSession re-registers persisted peer chats with their history', async () => { + class MultiChatAgent extends MockAgent { + async createChat(_session: URI, _chat: URI): Promise { } + async getChats(session: URI): Promise { + return [URI.parse(buildChatUri(session, 'peer-1'))]; + } + override async getSessionMessages(session: URI): Promise { + if (session.scheme === 'ahp-chat') { + return [{ + id: 'peer-turn-1', + state: TurnState.Complete, + message: { text: 'hi peer', origin: { kind: MessageKind.User } }, + responseParts: [], + usage: undefined, + }]; + } + return []; + } + } + const agent = disposables.add(new MultiChatAgent('copilot')); + service.registerProvider(agent); + const { session } = await agent.createSession(); + service.stateManager.deleteSession(session.toString()); + + await service.restoreSession(session); + + const state = service.stateManager.getSessionState(session.toString()); + const peerUri = buildChatUri(session, 'peer-1'); + const peerChatState = service.stateManager.getChatState(URI.parse(peerUri).toString()); + assert.deepStrictEqual({ + inCatalog: !!state?.chats.some(c => c.resource.toString() === peerUri), + peerTurnIds: peerChatState?.turns.map(t => t.id) ?? [], + }, { + inCatalog: true, + peerTurnIds: ['peer-turn-1'], + }); + }); }); suite('subscriber refcount eviction', () => { diff --git a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts index 05939c6e2b45c..2ee65166854dd 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts @@ -1784,6 +1784,81 @@ suite('CopilotAgent', () => { await disposeAgent(agent); } }); + + test('getChats returns the persisted peer chat catalog', async () => { + const sessionDataService = disposables.add(new TestSessionDataService()); + const agent = createTestAgent(disposables, { sessionDataService, copilotClient: new TestCopilotClient([]) }); + try { + const session = AgentSession.uri('copilotcli', 'session-getchats'); + const db = sessionDataService.openDatabase(session); + await db.object.setMetadata('copilot.chats', JSON.stringify({ + 'peer-a': { sdkSessionId: 'sdk-a' }, + 'peer-b': { sdkSessionId: 'sdk-b' }, + })); + + const chats = await agent.getChats(session); + + assert.deepStrictEqual( + chats.map(c => c.toString()).sort(), + [buildChatUri(session, 'peer-a'), buildChatUri(session, 'peer-b')].sort(), + ); + } finally { + await disposeAgent(agent); + } + }); + + test('getChats drops corrupted or invalid persisted entries', async () => { + const sessionDataService = disposables.add(new TestSessionDataService()); + const agent = createTestAgent(disposables, { sessionDataService, copilotClient: new TestCopilotClient([]) }); + try { + const session = AgentSession.uri('copilotcli', 'session-getchats-invalid'); + const db = sessionDataService.openDatabase(session); + await db.object.setMetadata('copilot.chats', JSON.stringify({ + 'peer-ok': { sdkSessionId: 'sdk-ok' }, + 'peer-null': null, + 'peer-missing-id': { model: { id: 'm' } }, + 'peer-nonstring-id': { sdkSessionId: 42 }, + 'peer-empty-id': { sdkSessionId: '' }, + })); + + const chats = await agent.getChats(session); + + assert.deepStrictEqual( + chats.map(c => c.toString()), + [buildChatUri(session, 'peer-ok')], + ); + } finally { + await disposeAgent(agent); + } + }); + + test('disposeChat removes the persisted entry and deletes its SDK conversation', async () => { + const sessionDataService = disposables.add(new TestSessionDataService()); + const client = new TestCopilotClient([]); + const agent = createTestAgent(disposables, { sessionDataService, copilotClient: client }); + try { + await agent.authenticate('https://api.github.com', 'token'); + const session = AgentSession.uri('copilotcli', 'session-dispose-chat'); + const db = sessionDataService.openDatabase(session); + await db.object.setMetadata('copilot.chats', JSON.stringify({ + 'peer-a': { sdkSessionId: 'sdk-a' }, + })); + const chatUri = URI.parse(buildChatUri(session, 'peer-a')); + + await agent.disposeChat(session, chatUri); + + const remaining = await db.object.getMetadata('copilot.chats'); + assert.deepStrictEqual({ + remaining: remaining ? JSON.parse(remaining) : {}, + deleted: client.deletedSessionIds, + }, { + remaining: {}, + deleted: ['sdk-a'], + }); + } finally { + await disposeAgent(agent); + } + }); }); // Regression for the #319516 incident: a window reload reconnects with a diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 12f69c3203366..2374ef47432a9 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -31,6 +31,7 @@ import { AgentInfo, buildChatUri, buildDefaultChatUri, isDefaultChatUri, parseCh import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; +import { IDialogService } from '../../../../../platform/dialogs/common/dialogs.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; import { IAgentHostActiveClientService } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostActiveClientService.js'; import { IChatWidgetService } from '../../../../../workbench/contrib/chat/browser/chat.js'; @@ -1409,6 +1410,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement @ISessionsService protected readonly _sessionsService: ISessionsService, @IAgentHostActiveClientService protected readonly _activeClientService: IAgentHostActiveClientService, @IStorageService protected readonly _storageService: IStorageService, + @IDialogService protected readonly _dialogService: IDialogService, ) { super(); this._register(toDisposable(() => { @@ -2290,6 +2292,15 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement const sessionUri = AgentSession.uri(cached.agentProvider, rawId); const ahpChatUri = URI.parse(buildChatUri(sessionUri, chatId)); + const confirmed = await this._dialogService.confirm({ + message: localize('deleteChat.confirm', "Are you sure you want to delete this chat?"), + detail: localize('deleteChat.detail', "This action cannot be undone."), + primaryButton: localize('deleteChat.delete', "Delete") + }); + if (!confirmed.confirmed) { + return; + } + // Keep the session-state subscription alive so the `chatRemoved` the // host emits flows into `applyChatCatalog` and drops the chat from // `cached.chats`. diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts index 5e30ececa0d57..8a9654ac64492 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts @@ -18,6 +18,7 @@ import { IInstantiationService } from '../../../../../platform/instantiation/com import { ILabelService } from '../../../../../platform/label/common/label.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; import { IStorageService } from '../../../../../platform/storage/common/storage.js'; +import { IDialogService } from '../../../../../platform/dialogs/common/dialogs.js'; import { IAgentHostActiveClientService } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostActiveClientService.js'; import { IChatWidgetService } from '../../../../../workbench/contrib/chat/browser/chat.js'; import { IChatService } from '../../../../../workbench/contrib/chat/common/chatService/chatService.js'; @@ -82,9 +83,10 @@ export class LocalAgentHostSessionsProvider extends BaseAgentHostSessionsProvide @ISessionsService sessionsService: ISessionsService, @IAgentHostActiveClientService activeClientService: IAgentHostActiveClientService, @IStorageService storageService: IStorageService, + @IDialogService dialogService: IDialogService, @IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService, ) { - super(chatSessionsService, chatService, chatWidgetService, languageModelsService, _configurationService, logService, gitHubService, instantiationService, sessionsService, activeClientService, storageService); + super(chatSessionsService, chatService, chatWidgetService, languageModelsService, _configurationService, logService, gitHubService, instantiationService, sessionsService, activeClientService, storageService, dialogService); this._isSessionsWindow = environmentService.isSessionsWindow; 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 3fff3586f736c..95f858110499c 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 @@ -21,7 +21,7 @@ import { ActionType, NotificationType, type ActionEnvelope, type IRootConfigChan import { SessionConfigKey } from '../../../../../../platform/agentHost/common/sessionConfigKeys.js'; import { ConfigurationTarget, IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; -import { IFileDialogService } from '../../../../../../platform/dialogs/common/dialogs.js'; +import { IDialogService, IFileDialogService } from '../../../../../../platform/dialogs/common/dialogs.js'; import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { InMemoryStorageService, IStorageService, StorageScope, StorageTarget } from '../../../../../../platform/storage/common/storage.js'; import { IChatWidget, IChatWidgetService } from '../../../../../../workbench/contrib/chat/browser/chat.js'; @@ -111,6 +111,11 @@ class MockAgentHostService extends mock() { this._sessions.delete(rawId); } + public disposedChats: URI[] = []; + override async disposeChat(chat: URI): Promise { + this.disposedChats.push(chat); + } + public createdSessionUris: URI[] = []; public createSessionConfigs: { config?: Record }[] = []; /** @@ -278,13 +283,14 @@ function createPolicyRestrictedConfigurationService(): TestConfigurationService function createProvider(disposables: DisposableStore, agentHostService: MockAgentHostService, contributions = [ { type: 'agent-host-copilotcli', name: 'copilot', displayName: 'Copilot', description: 'test', icon: undefined }, -], options?: { sendRequest?: (resource: URI, message: string, options?: IChatSendRequestOptions) => Promise; openSession?: boolean; configurationService?: IConfigurationService; activeSession?: IObservable; storageService?: IStorageService; isSessionsWindow?: boolean }): LocalAgentHostSessionsProvider { +], options?: { sendRequest?: (resource: URI, message: string, options?: IChatSendRequestOptions) => Promise; openSession?: boolean; configurationService?: IConfigurationService; activeSession?: IObservable; storageService?: IStorageService; isSessionsWindow?: boolean; confirmDelete?: boolean }): LocalAgentHostSessionsProvider { const instantiationService = disposables.add(new TestInstantiationService()); instantiationService.stub(IAgentHostService, agentHostService); instantiationService.stub(IConfigurationService, options?.configurationService ?? new TestConfigurationService()); instantiationService.stub(IWorkbenchEnvironmentService, { isSessionsWindow: options?.isSessionsWindow ?? true } as IWorkbenchEnvironmentService); instantiationService.stub(IFileDialogService, {}); + instantiationService.stub(IDialogService, { confirm: async () => ({ confirmed: options?.confirmDelete ?? true }) }); instantiationService.stub(IChatSessionsService, { getChatSessionContribution: (chatSessionType: string) => contributions.find(c => c.type === chatSessionType), getAllChatSessionContributions: () => contributions, @@ -1735,6 +1741,44 @@ suite('LocalAgentHostSessionsProvider', () => { }); }); + test('deleteChat prompts for confirmation and disposes the peer chat when confirmed', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + const provider = createProvider(disposables, agentHost, undefined, { confirmDelete: true }); + const session = setupMultiChatSession(provider, 'multi-del'); + const sessionUri = AgentSession.uri('copilotcli', 'multi-del').toString(); + const defaultChat = buildDefaultChatUri(sessionUri); + const peerChat = buildChatUri(sessionUri, 'peer-1'); + + agentHost.setSessionState('multi-del', 'copilotcli', makeState('multi-del', [ + makeChatSummary(defaultChat, ''), + makeChatSummary(peerChat, 'Peer'), + ], { defaultChat })); + + const peer = session.chats.get().find(c => c.resource.fragment === 'peer-1'); + assert.ok(peer); + await provider.deleteChat(session.sessionId, peer!.resource); + + assert.deepStrictEqual(agentHost.disposedChats.map(u => u.toString()), [peerChat]); + })); + + test('deleteChat does not dispose the peer chat when the confirmation is cancelled', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + const provider = createProvider(disposables, agentHost, undefined, { confirmDelete: false }); + const session = setupMultiChatSession(provider, 'multi-del-cancel'); + const sessionUri = AgentSession.uri('copilotcli', 'multi-del-cancel').toString(); + const defaultChat = buildDefaultChatUri(sessionUri); + const peerChat = buildChatUri(sessionUri, 'peer-1'); + + agentHost.setSessionState('multi-del-cancel', 'copilotcli', makeState('multi-del-cancel', [ + makeChatSummary(defaultChat, ''), + makeChatSummary(peerChat, 'Peer'), + ], { defaultChat })); + + const peer = session.chats.get().find(c => c.resource.fragment === 'peer-1'); + assert.ok(peer); + await provider.deleteChat(session.sessionId, peer!.resource); + + assert.deepStrictEqual(agentHost.disposedChats, []); + })); + test('single-chat catalog degrades to the default chat only', () => { const provider = createProvider(disposables, agentHost); const session = setupMultiChatSession(provider, 'multi-single'); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts index 9d9fde8d569e6..cde3c5041aa24 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts @@ -20,7 +20,7 @@ import { AgentSession, type IAgentConnection, type IAgentSessionMetadata } from import { IRemoteAgentHostService, RemoteAgentHostConnectionStatus, remoteAgentHostLogOutputChannelId } from '../../../../../platform/agentHost/common/remoteAgentHostService.js'; import type { ISessionGitState } from '../../../../../platform/agentHost/common/state/sessionState.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; -import { IFileDialogService } from '../../../../../platform/dialogs/common/dialogs.js'; +import { IDialogService, IFileDialogService } from '../../../../../platform/dialogs/common/dialogs.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { ILabelService } from '../../../../../platform/label/common/label.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; @@ -216,8 +216,9 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid @IInstantiationService instantiationService: IInstantiationService, @ISessionsService sessionsService: ISessionsService, @IAgentHostActiveClientService activeClientService: IAgentHostActiveClientService, + @IDialogService dialogService: IDialogService, ) { - super(chatSessionsService, chatService, chatWidgetService, languageModelsService, _configurationService, logService, gitHubService, instantiationService, sessionsService, activeClientService, storageService); + super(chatSessionsService, chatService, chatWidgetService, languageModelsService, _configurationService, logService, gitHubService, instantiationService, sessionsService, activeClientService, storageService, dialogService); this._connectionAuthority = agentHostAuthority(config.address); this._connectOnDemand = config.connectOnDemand; diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts index 47f5633d4c870..3b6dc03eb23e2 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts @@ -19,7 +19,7 @@ import { buildDefaultChatUri, SessionStatus as ProtocolSessionStatus, StateCompo import type { IAgentSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js'; import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; -import { IFileDialogService } from '../../../../../../platform/dialogs/common/dialogs.js'; +import { IDialogService, IFileDialogService } from '../../../../../../platform/dialogs/common/dialogs.js'; import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { INotificationService } from '../../../../../../platform/notification/common/notification.js'; import { InMemoryStorageService, IStorageService } from '../../../../../../platform/storage/common/storage.js'; @@ -193,6 +193,7 @@ function createProvider(disposables: DisposableStore, connection: MockAgentConne const instantiationService = disposables.add(new TestInstantiationService()); instantiationService.stub(IFileDialogService, {}); + instantiationService.stub(IDialogService, { confirm: async () => ({ confirmed: true }) }); instantiationService.stub(IConfigurationService, new TestConfigurationService()); instantiationService.stub(INotificationService, { error: () => { } }); instantiationService.stub(IChatSessionsService, { diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts index 692fab3914a44..1a77d971553d9 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts @@ -436,6 +436,15 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC */ private readonly _additionalChatSubscriptions = new Map>>(); + /** + * Backend session URIs with an in-flight {@link provideChatSessionContent} + * call, keyed by session URI string with a refcount value. While a chat is + * still hydrating its subscriptions, a sibling chat of the same session + * closing must not tear down the shared session subscription out from under + * it (see {@link _releaseChatSessionSubscriptions} / {@link _hasOtherSessionHold}). + */ + private readonly _hydratingChatSessions = new Map(); + constructor( config: IAgentHostSessionHandlerConfig, @IChatAgentService private readonly _chatAgentService: IChatAgentService, @@ -637,68 +646,82 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC let initialProgress: IChatProgress[] | undefined; let activeTurnId: string | undefined; let sessionTitle: string | undefined; - if (!isNewSession) { - try { - const sub = this._ensureSessionSubscription(resolvedSession.toString()); - const chatSub = this._ensureChatSubscription(resolvedSession.toString(), chatKey); - // Wait for both the session summary and its default-chat - // conversation state to hydrate from the server. After the - // multi-chat protocol adoption, turns/activeTurn live on the - // separate chat channel, so reading them before the chat - // subscription lands would yield an empty history. - await Promise.all([ - this._whenSubscriptionHydrated(sub, token), - this._whenSubscriptionHydrated(chatSub, token), - ]); - const sessionState = this._getSessionState(resolvedSession.toString(), chatKey); - if (sessionState) { - sessionTitle = sessionState.summary.title; - const fallbackRawModelId = sessionState.summary.model?.id; - const lookup = this._createTurnModelLookup(sessionResource, fallbackRawModelId); - history.push(...turnsToHistory(resolvedSession, sessionState.turns, this._config.agentId, this._config.connectionAuthority, lookup, this._chatErrorContext())); - - // Enrich history with inner tool calls from subagent - // child sessions. Subscribes to each child session so - // its tool calls appear grouped under the parent widget. - await this._enrichHistoryWithSubagentCalls(history, resolvedSession); - - // Store historical turns so the editing session can seed a - // request-level checkpoint for each turn (with file edits - // folded in) when the controller is created lazily. We seed - // for every turn — not just those with edits — so "Restore - // Checkpoint" on any historical request can find a boundary - // to navigate to. - if (sessionState.turns.length > 0) { - this._pendingHistoryTurns.set(sessionResource, sessionState.turns); - } + // Mark this session as hydrating so that a sibling chat of the same + // session closing while we await our subscriptions does not tear down + // the shared session subscription (which would strand us forever). + const hydrationKey = resolvedSession.toString(); + this._hydratingChatSessions.set(hydrationKey, (this._hydratingChatSessions.get(hydrationKey) ?? 0) + 1); + try { + if (!isNewSession) { + try { + const sub = this._ensureSessionSubscription(resolvedSession.toString()); + const chatSub = this._ensureChatSubscription(resolvedSession.toString(), chatKey); + // Wait for both the session summary and its default-chat + // conversation state to hydrate from the server. After the + // multi-chat protocol adoption, turns/activeTurn live on the + // separate chat channel, so reading them before the chat + // subscription lands would yield an empty history. + await Promise.all([ + this._whenSubscriptionHydrated(sub, token), + this._whenSubscriptionHydrated(chatSub, token), + ]); + const sessionState = this._getSessionState(resolvedSession.toString(), chatKey); + if (sessionState) { + sessionTitle = sessionState.summary.title; + const fallbackRawModelId = sessionState.summary.model?.id; + const lookup = this._createTurnModelLookup(sessionResource, fallbackRawModelId); + history.push(...turnsToHistory(resolvedSession, sessionState.turns, this._config.agentId, this._config.connectionAuthority, lookup, this._chatErrorContext())); + + // Enrich history with inner tool calls from subagent + // child sessions. Subscribes to each child session so + // its tool calls appear grouped under the parent widget. + await this._enrichHistoryWithSubagentCalls(history, resolvedSession); + + // Store historical turns so the editing session can seed a + // request-level checkpoint for each turn (with file edits + // folded in) when the controller is created lazily. We seed + // for every turn — not just those with edits — so "Restore + // Checkpoint" on any historical request can find a boundary + // to navigate to. + if (sessionState.turns.length > 0) { + this._pendingHistoryTurns.set(sessionResource, sessionState.turns); + } - // If there's an active turn, include its request in history - // with an empty response so the chat service creates a - // pending request, then provide accumulated progress via - // progressObs for live streaming. - if (sessionState.activeTurn) { - activeTurnId = sessionState.activeTurn.id; - const activeRawModelId = sessionState.activeTurn.usage?.model ?? fallbackRawModelId; - history.push({ - type: 'request', - prompt: sessionState.activeTurn.message.text, - participant: this._config.agentId, - modelId: lookup.toLanguageModelId(activeRawModelId), - variableData: messageToVariableData(sessionState.activeTurn.message, this._config.connectionAuthority), - isSystemInitiated: sessionState.activeTurn.message.origin.kind === MessageKind.SystemNotification, - }); - history.push({ - type: 'response', - parts: [], - participant: this._config.agentId, - details: lookup.toResponseDetails(activeRawModelId, sessionState.activeTurn.usage), - }); - initialProgress = activeTurnToProgress(resolvedSession, sessionState.activeTurn, this._config.connectionAuthority); - this._logService.info(`[AgentHost] Reconnecting to active turn ${activeTurnId} for session ${resolvedSession.toString()}`); + // If there's an active turn, include its request in history + // with an empty response so the chat service creates a + // pending request, then provide accumulated progress via + // progressObs for live streaming. + if (sessionState.activeTurn) { + activeTurnId = sessionState.activeTurn.id; + const activeRawModelId = sessionState.activeTurn.usage?.model ?? fallbackRawModelId; + history.push({ + type: 'request', + prompt: sessionState.activeTurn.message.text, + participant: this._config.agentId, + modelId: lookup.toLanguageModelId(activeRawModelId), + variableData: messageToVariableData(sessionState.activeTurn.message, this._config.connectionAuthority), + isSystemInitiated: sessionState.activeTurn.message.origin.kind === MessageKind.SystemNotification, + }); + history.push({ + type: 'response', + parts: [], + participant: this._config.agentId, + details: lookup.toResponseDetails(activeRawModelId, sessionState.activeTurn.usage), + }); + initialProgress = activeTurnToProgress(resolvedSession, sessionState.activeTurn, this._config.connectionAuthority); + this._logService.info(`[AgentHost] Reconnecting to active turn ${activeTurnId} for session ${resolvedSession.toString()}`); + } } + } catch (err) { + this._logService.warn(`[AgentHost] Failed to subscribe to existing session: ${resolvedSession.toString()}`, err); } - } catch (err) { - this._logService.warn(`[AgentHost] Failed to subscribe to existing session: ${resolvedSession.toString()}`, err); + } + } finally { + const remaining = (this._hydratingChatSessions.get(hydrationKey) ?? 1) - 1; + if (remaining > 0) { + this._hydratingChatSessions.set(hydrationKey, remaining); + } else { + this._hydratingChatSessions.delete(hydrationKey); } } const session = this._instantiationService.createInstance( @@ -726,7 +749,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC this._pendingMessageSubscriptions.deleteAndDispose(sessionResource); this._serverTurnWatchers.deleteAndDispose(sessionResource); this._pendingHistoryTurns.delete(sessionResource); - this._releaseSessionSubscription(resolvedSession.toString()); + this._releaseChatSessionSubscriptions(resolvedSession.toString(), chatKey); }, () => { const sessionKey = resolvedSession.toString(); @@ -3308,6 +3331,65 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC } } + /** + * Release the subscriptions held by a single chat session on dispose. + * + * Unlike {@link _releaseSessionSubscription} (which tears down every chat + * of a session at once), this only releases the disposed chat's own + * conversation subscription and never touches sibling peer chats: closing + * one chat of a multi-chat session must not strand another chat — including + * one that is concurrently hydrating in {@link provideChatSessionContent} — + * on a disposed subscription. The session summary subscription (and its + * lockstep default-chat subscription) is shared by every chat of the + * session, so it is only torn down once no sibling chat session is still + * active or mid-hydration for the same backend session. + */ + private _releaseChatSessionSubscriptions(sessionUri: string, chatUri: string): void { + // Release this chat's own conversation subscription. The default chat's + // subscription is keyed by session URI and torn down together with the + // shared session subscription below; peer chats own a dedicated entry. + if (chatUri !== this._resolveDefaultChatUri(sessionUri)) { + const chatRef = this._additionalChatSubscriptions.get(chatUri); + if (chatRef) { + this._additionalChatSubscriptions.delete(chatUri); + chatRef.dispose(); + } + } + // Keep the shared session subscription alive while any sibling chat of + // the same backend session is still active or hydrating. + if (this._hasOtherSessionHold(sessionUri)) { + return; + } + const ref = this._sessionSubscriptions.get(sessionUri); + if (ref) { + this._sessionSubscriptions.delete(sessionUri); + ref.dispose(); + } + const chatRef = this._defaultChatSubscriptions.get(sessionUri); + if (chatRef) { + this._defaultChatSubscriptions.delete(sessionUri); + chatRef.dispose(); + } + } + + /** + * Returns whether another chat session for the given backend session URI is + * still active or in the middle of hydrating its subscriptions, so the + * shared session subscription must be kept alive. Callers invoke this after + * removing their own entry from {@link _activeSessions}. + */ + private _hasOtherSessionHold(sessionUri: string): boolean { + if ((this._hydratingChatSessions.get(sessionUri) ?? 0) > 0) { + return true; + } + for (const resource of this._activeSessions.keys()) { + if (this._resolveSessionUri(resource).toString() === sessionUri) { + return true; + } + } + return false; + } + /** * Read the current optimistic session state for a backend session URI, * merged with its default chat so conversation contents (turns, active diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts index a55c243897896..f2953642d38b1 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts @@ -22,7 +22,7 @@ import { AgentFeedbackAttachmentDisplayKind, AgentFeedbackAttachmentMetadataKey import { ActionType, isSessionAction, isChatAction, type ActionEnvelope, type IRootConfigChangedAction, type SessionAction, type ChatAction, type TerminalAction, type INotification, type IToolCallConfirmedAction, type ITurnStartedAction, type ClientAnnotationsAction } from '../../../../../../platform/agentHost/common/state/sessionActions.js'; import type { IStateSnapshot } from '../../../../../../platform/agentHost/common/state/sessionProtocol.js'; import { CustomizationType, type ClientPluginCustomization, type ToolDefinition } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; -import { ChatInputAnswerState, ChatInputAnswerValueKind, ChatInputQuestionKind, ChatInputResponseKind, SessionLifecycle, SessionStatus, TurnState, ToolCallStatus, ToolCallConfirmationReason, createSessionState, createChatState, createDefaultChatSummary, buildDefaultChatUri, parseDefaultChatUri, isAhpChatChannel, createActiveTurn, isAhpRootChannel, PolicyState, ResponsePartKind, StateComponents, buildSubagentSessionUri, ToolResultContentType, MessageAttachmentKind, MessageKind, type SessionState, type SessionSummary, type ChatState, type ISessionWithDefaultChat, RootState, type ToolCallState, type AgentInfo } from '../../../../../../platform/agentHost/common/state/sessionState.js'; +import { ChatInputAnswerState, ChatInputAnswerValueKind, ChatInputQuestionKind, ChatInputResponseKind, SessionLifecycle, SessionStatus, TurnState, ToolCallStatus, ToolCallConfirmationReason, createSessionState, createChatState, createDefaultChatSummary, buildChatUri, buildDefaultChatUri, parseDefaultChatUri, isAhpChatChannel, createActiveTurn, isAhpRootChannel, PolicyState, ResponsePartKind, StateComponents, buildSubagentSessionUri, ToolResultContentType, MessageAttachmentKind, MessageKind, type SessionState, type SessionSummary, type ChatState, type ISessionWithDefaultChat, RootState, type ToolCallState, type AgentInfo } from '../../../../../../platform/agentHost/common/state/sessionState.js'; import { CompletionItemKind as AhpCompletionItemKind, type CompletionsParams, type CompletionsResult } from '../../../../../../platform/agentHost/common/state/protocol/commands.js'; import { sessionReducer, chatReducer } from '../../../../../../platform/agentHost/common/state/sessionReducers.js'; import { IDefaultAccountService } from '../../../../../../platform/defaultAccount/common/defaultAccount.js'; @@ -1126,6 +1126,31 @@ suite('AgentHostChatContribution', () => { assert.strictEqual(fired, 1, 'onWillDispose should fire exactly once when the session is disposed'); }); + + test('disposing one chat does not tear down a sibling peer chat subscription (peer chat never loads after reload)', async () => { + const { sessionHandler, agentHostService } = createContribution(disposables); + + const sessionResource = URI.from({ scheme: 'agent-host-copilot', path: '/multi' }); + const backendSession = AgentSession.uri('copilot', 'multi'); + const peerResource = URI.from({ scheme: 'agent-host-copilot', path: '/multi', fragment: 'peer-1' }); + const peerChatUri = URI.parse(buildChatUri(backendSession.toString(), 'peer-1')); + + // Open the session's default chat and an additional peer chat. + const defaultSession = await sessionHandler.provideChatSessionContent(sessionResource, CancellationToken.None); + const peerSession = await sessionHandler.provideChatSessionContent(peerResource, CancellationToken.None); + disposables.add(toDisposable(() => peerSession.dispose())); + + assert.ok(agentHostService.getSubscriptionUnmanaged(StateComponents.Chat, peerChatUri), 'peer chat subscription should be live after it is opened'); + + // Closing the default chat must NOT dispose the still-open peer + // chat's subscription. The regression left the peer chat's + // `provideChatSessionContent` awaiting a dead subscription, so it + // never resolved and the chat stayed stuck on a loading spinner. + defaultSession.dispose(); + + assert.ok(agentHostService.getSubscriptionUnmanaged(StateComponents.Chat, peerChatUri), 'peer chat subscription must stay live after the sibling default chat is disposed'); + assert.ok(agentHostService.getSubscriptionUnmanaged(StateComponents.Session, backendSession), 'shared session subscription must stay live while the peer chat is still open'); + }); }); // ---- Session list (IChatSessionItemController) ---------------------- From 1c49cfa555731563f6069c8e55fba5ba5b71b142 Mon Sep 17 00:00:00 2001 From: BeniBenj Date: Fri, 19 Jun 2026 00:39:12 +0200 Subject: [PATCH 3/7] pr polling improvements --- .../github/browser/github.contribution.ts | 149 +++++++++++------- .../test/browser/githubContribution.test.ts | 47 +++++- 2 files changed, 140 insertions(+), 56 deletions(-) diff --git a/src/vs/sessions/contrib/github/browser/github.contribution.ts b/src/vs/sessions/contrib/github/browser/github.contribution.ts index 4556ff7ee7eca..4b4af4c50e48b 100644 --- a/src/vs/sessions/contrib/github/browser/github.contribution.ts +++ b/src/vs/sessions/contrib/github/browser/github.contribution.ts @@ -3,8 +3,9 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Disposable, DisposableMap, DisposableStore } from '../../../../base/common/lifecycle.js'; -import { autorun, derivedOpts } from '../../../../base/common/observable.js'; +import { Disposable, DisposableMap, IDisposable } from '../../../../base/common/lifecycle.js'; +import { autorun, derived, derivedOpts, IReader } from '../../../../base/common/observable.js'; +import { structuralEquals } from '../../../../base/common/equals.js'; import { isEqual } from '../../../../base/common/resources.js'; import { URI } from '../../../../base/common/uri.js'; import { InstantiationType, registerSingleton } from '../../../../platform/instantiation/common/extensions.js'; @@ -12,7 +13,6 @@ import { IWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase import { ISession } from '../../../services/sessions/common/session.js'; import { ISessionsChangeEvent, ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js'; import { ISessionsService } from '../../../services/sessions/browser/sessionsService.js'; -import { getPullRequestKey } from '../common/utils.js'; import { GitHubPullRequestState } from '../common/types.js'; import { GitHubService, IGitHubService } from './githubService.js'; @@ -20,7 +20,8 @@ export class GitHubPullRequestPollingContribution extends Disposable implements static readonly ID = 'sessions.contrib.githubPullRequestPolling'; - private readonly _pullRequests = new DisposableMap(); + /** Per-session pollers, keyed by `session.sessionId`. */ + private readonly _sessionTrackers = new DisposableMap(); constructor( @IGitHubService private readonly _gitHubService: IGitHubService, @@ -86,83 +87,123 @@ export class GitHubPullRequestPollingContribution extends Disposable implements } private _onDidChangeSessions(e: ISessionsChangeEvent): void { - // Added sessions + // Track added and changed sessions. Archived state and async PR-number + // resolution are handled reactively inside the per-session poller, so we + // can track unconditionally here (the tracker is a no-op until a PR + // number actually resolves). for (const session of e.added) { - // Archived - if (session.isArchived.get()) { - continue; - } - - this._startPolling(session); + this._trackSession(session); } - // Changes sessions for (const session of e.changed) { - // Archived - if (session.isArchived.get()) { - this._disposePolling(session); - continue; - } - - this._startPolling(session); + this._trackSession(session); } // Removed sessions for (const session of e.removed) { - this._disposePolling(session); + this._sessionTrackers.deleteAndDispose(session.sessionId); } } - private _startPolling(session: ISession): void { - const gitHubInfo = session.workspace.get()?.folders[0]?.gitRepository?.gitHubInfo.get(); - if (!gitHubInfo || !gitHubInfo.pullRequest) { - return; - } - - const key = getPullRequestKey(gitHubInfo.owner, gitHubInfo.repo, gitHubInfo.pullRequest.number); - if (this._pullRequests.has(key)) { + private _trackSession(session: ISession): void { + if (this._sessionTrackers.has(session.sessionId)) { return; } - const disposables = new DisposableStore(); - const modelRef = this._gitHubService.createPullRequestModelReference(gitHubInfo.owner, gitHubInfo.repo, gitHubInfo.pullRequest.number); - - disposables.add(modelRef); - disposables.add(modelRef.object.startPolling()); + this._sessionTrackers.set(session.sessionId, this._createSessionPoller(session)); + } - // Poll CI checks and review threads so the session's PR icon can reflect - // failing checks / unresolved comments even when the session is not active. - const prNumber = gitHubInfo.pullRequest.number; - disposables.add(autorun(reader => { - const prDetails = modelRef.object.pullRequest.read(reader); - if (!prDetails || prDetails.isDraft || prDetails.state !== GitHubPullRequestState.Open) { + /** + * Reactively poll the pull request for a single session. + * + * Unlike a one-shot snapshot, the returned autorun re-runs when the session's + * pull-request identity changes — so polling starts once a provider resolves + * the PR number asynchronously (e.g. the agent host), and stops when the + * session is archived or the PR goes away. A merged pull request can never + * change again, so it stops polling unless it is the active session. + */ + private _createSessionPoller(session: ISession): IDisposable { + // PR identity (owner/repo/number) only. Structural equality keeps this + // stable while the PR's live data — and therefore its computed icon — + // updates, so the poller doesn't churn (or feed back into itself) every + // time `gitHubInfo` re-derives. + const pullRequestIdentityObs = derivedOpts<{ readonly owner: string; readonly repo: string; readonly prNumber: number } | undefined>( + { owner: this, equalsFn: structuralEquals }, + reader => { + if (session.isArchived.read(reader)) { + return undefined; + } + + const gitHubInfo = session.workspace.read(reader)?.folders[0]?.gitRepository?.gitHubInfo.read(reader); + if (!gitHubInfo?.pullRequest) { + return undefined; + } + + return { owner: gitHubInfo.owner, repo: gitHubInfo.repo, prNumber: gitHubInfo.pullRequest.number }; + }); + + return autorun(reader => { + const identity = pullRequestIdentityObs.read(reader); + if (!identity) { + // No PR number yet (or archived); this autorun re-runs once it resolves. return; } - const ciModelRef = reader.store.add(this._gitHubService.createPullRequestCIModelReference(gitHubInfo.owner, gitHubInfo.repo, prNumber, prDetails.headSha)); - ciModelRef.object.refresh(); - reader.store.add(ciModelRef.object.startPolling()); + const { owner, repo, prNumber } = identity; - const reviewThreadsModelRef = reader.store.add(this._gitHubService.createPullRequestReviewThreadsModelReference(gitHubInfo.owner, gitHubInfo.repo, prNumber)); - reviewThreadsModelRef.object.refresh(); - reader.store.add(reviewThreadsModelRef.object.startPolling()); - })); + const modelRef = reader.store.add(this._gitHubService.createPullRequestModelReference(owner, repo, prNumber)); + const model = modelRef.object; - this._pullRequests.set(key, disposables); + // Fetch once so we learn the PR state and can render the icon — even for + // a merged PR that won't keep polling. + model.refresh(); + + // Gate the repeating poll loop on a stable boolean so poll results (which + // update `pullRequest`) don't toggle the loop on every refresh. + const shouldPollObs = derived(this, pollReader => { + const prDetails = model.pullRequest.read(pollReader); + const isMerged = prDetails?.state === GitHubPullRequestState.Merged; + return !isMerged || this._isActiveSession(session, pollReader); + }); + reader.store.add(autorun(pollReader => { + if (!shouldPollObs.read(pollReader)) { + return; + } + + pollReader.store.add(model.startPolling()); + })); + + // Poll CI checks and review threads so the session's PR icon can reflect + // failing checks / unresolved comments even when the session is not active. + // Only open, non-draft PRs need this (merged/closed/draft don't surface it). + reader.store.add(autorun(statusReader => { + const prDetails = model.pullRequest.read(statusReader); + if (!prDetails || prDetails.isDraft || prDetails.state !== GitHubPullRequestState.Open) { + return; + } + + const ciModelRef = statusReader.store.add(this._gitHubService.createPullRequestCIModelReference(owner, repo, prNumber, prDetails.headSha)); + ciModelRef.object.refresh(); + statusReader.store.add(ciModelRef.object.startPolling()); + + const reviewThreadsModelRef = statusReader.store.add(this._gitHubService.createPullRequestReviewThreadsModelReference(owner, repo, prNumber)); + reviewThreadsModelRef.object.refresh(); + statusReader.store.add(reviewThreadsModelRef.object.startPolling()); + })); + }); } - private _disposePolling(session: ISession): void { - const gitHubInfo = session.workspace.get()?.folders[0]?.gitRepository?.gitHubInfo.get(); - if (!gitHubInfo || !gitHubInfo.pullRequest) { - return; + private _isActiveSession(session: ISession, reader: IReader): boolean { + const activeSession = this._sessionsService.activeSession.read(reader); + if (!activeSession || activeSession.isArchived.read(reader)) { + return false; } - const key = getPullRequestKey(gitHubInfo.owner, gitHubInfo.repo, gitHubInfo.pullRequest.number); - this._pullRequests.deleteAndDispose(key); + return isEqual(activeSession.resource, session.resource); } override dispose(): void { - this._pullRequests.dispose(); + this._sessionTrackers.dispose(); super.dispose(); } diff --git a/src/vs/sessions/contrib/github/test/browser/githubContribution.test.ts b/src/vs/sessions/contrib/github/test/browser/githubContribution.test.ts index 0f8ad9bdb476e..46e210019da3f 100644 --- a/src/vs/sessions/contrib/github/test/browser/githubContribution.test.ts +++ b/src/vs/sessions/contrib/github/test/browser/githubContribution.test.ts @@ -8,7 +8,7 @@ import { Codicon } from '../../../../../base/common/codicons.js'; import { Emitter, Event } from '../../../../../base/common/event.js'; import { IMarkdownString } from '../../../../../base/common/htmlContent.js'; import { DisposableStore, IDisposable, ImmortalReference, IReference, toDisposable } from '../../../../../base/common/lifecycle.js'; -import { constObservable, IObservable, observableValue } from '../../../../../base/common/observable.js'; +import { constObservable, IObservable, ISettableObservable, observableValue } from '../../../../../base/common/observable.js'; import { GitHubPullRequestModel } from '../../browser/models/githubPullRequestModel.js'; import { GitHubPullRequestCIModel } from '../../browser/models/githubPullRequestCIModel.js'; import { GitHubPullRequestReviewThreadsModel } from '../../browser/models/githubPullRequestReviewThreadsModel.js'; @@ -28,11 +28,13 @@ suite('GitHubPullRequestPollingContribution', () => { let sessionsManagementService: TestSessionsManagementService; let sessionsService: ISessionsService; let gitHubService: TestGitHubService; + let activeSession: ISettableObservable; setup(() => { sessionsManagementService = new TestSessionsManagementService(store); + activeSession = observableValue('test.activeSession', undefined); sessionsService = new class extends mock() { - override readonly activeSession = constObservable(undefined); + override readonly activeSession = activeSession; }; gitHubService = new TestGitHubService(); }); @@ -124,6 +126,47 @@ suite('GitHubPullRequestPollingContribution', () => { assert.deepStrictEqual(gitHubService.statusModelSnapshot(), { ci: {}, reviewThreads: {} }); }); + + test('starts polling once an asynchronously resolved PR number appears', () => { + // Mirrors the agent-host provider, whose `gitHubInfo` initially has no PR + // number (it is resolved asynchronously via findPullRequestNumberByHeadBranch). + const session = sessionsManagementService.addSession('async', { owner: 'owner', repo: 'repo' }); + store.add(new GitHubPullRequestPollingContribution(gitHubService, sessionsManagementService, sessionsService)); + + // No PR number yet → nothing is polled. + assert.deepStrictEqual(gitHubService.snapshot(), {}); + + // The PR number resolves later. + sessionsManagementService.setGitHubInfo(session, makeGitHubInfo(1)); + + assert.deepStrictEqual(gitHubService.snapshot(), { + 'owner/repo/1': { startPollingCalls: 1, stopPollingCalls: 0, disposeCalls: 0 }, + }); + }); + + test('stops polling a merged pull request unless it is the active session', () => { + const session = sessionsManagementService.addSession('session', makeGitHubInfo(1)); + store.add(new GitHubPullRequestPollingContribution(gitHubService, sessionsManagementService, sessionsService)); + + // Open PR → polling. + gitHubService.setPullRequestDetails('owner', 'repo', 1, { state: GitHubPullRequestState.Open, isDraft: false, headSha: 'sha1' }); + assert.deepStrictEqual(gitHubService.snapshot(), { + 'owner/repo/1': { startPollingCalls: 1, stopPollingCalls: 0, disposeCalls: 0 }, + }); + + // Merges while not the active session → the repeating poll loop stops (the + // single initial fetch already produced the merged icon). + gitHubService.setPullRequestDetails('owner', 'repo', 1, { state: GitHubPullRequestState.Merged, isDraft: false, headSha: 'sha1' }); + assert.deepStrictEqual(gitHubService.snapshot(), { + 'owner/repo/1': { startPollingCalls: 1, stopPollingCalls: 1, disposeCalls: 0 }, + }); + + // Becomes the active session → polling resumes even though it is merged. + activeSession.set(session as unknown as IActiveSession, undefined); + assert.deepStrictEqual(gitHubService.snapshot(), { + 'owner/repo/1': { startPollingCalls: 2, stopPollingCalls: 1, disposeCalls: 0 }, + }); + }); }); class TestSessionsManagementService extends mock() { From 2fd23c841630c479c2f6489ab8a75e92efacd571 Mon Sep 17 00:00:00 2001 From: BeniBenj Date: Fri, 19 Jun 2026 00:55:03 +0200 Subject: [PATCH 4/7] nit --- .../contrib/github/browser/github.contribution.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/vs/sessions/contrib/github/browser/github.contribution.ts b/src/vs/sessions/contrib/github/browser/github.contribution.ts index 4b4af4c50e48b..73a95b79ec0d9 100644 --- a/src/vs/sessions/contrib/github/browser/github.contribution.ts +++ b/src/vs/sessions/contrib/github/browser/github.contribution.ts @@ -21,7 +21,7 @@ export class GitHubPullRequestPollingContribution extends Disposable implements static readonly ID = 'sessions.contrib.githubPullRequestPolling'; /** Per-session pollers, keyed by `session.sessionId`. */ - private readonly _sessionTrackers = new DisposableMap(); + private readonly _sessionTrackers = this._register(new DisposableMap()); constructor( @IGitHubService private readonly _gitHubService: IGitHubService, @@ -201,12 +201,6 @@ export class GitHubPullRequestPollingContribution extends Disposable implements return isEqual(activeSession.resource, session.resource); } - - override dispose(): void { - this._sessionTrackers.dispose(); - - super.dispose(); - } } registerWorkbenchContribution2(GitHubPullRequestPollingContribution.ID, GitHubPullRequestPollingContribution, WorkbenchPhase.AfterRestored); From 909d04597cbd13895a0170d0d12147f9273d8d45 Mon Sep 17 00:00:00 2001 From: Anthony Kim <62267334+anthonykim1@users.noreply.github.com> Date: Thu, 18 Jun 2026 16:16:56 -0700 Subject: [PATCH 5/7] Suggest Copilot CLI Agent Host when editor local chat is disabled (#321981) * Suggest Agent Host Copilot CLI for historical local harness session * address copilot feedback * continue in button without agent host tag * wording to mention agent host and not copilot cli --- .../localAgentDisabledInputTipContribution.ts | 115 +++++++ .../chat/browser/chat.shared.contribution.ts | 2 + .../browser/widget/input/chatInputPart.ts | 5 + ...lAgentDisabledInputTipContribution.test.ts | 284 ++++++++++++++++++ 4 files changed, 406 insertions(+) create mode 100644 src/vs/workbench/contrib/chat/browser/agentSessions/localAgentDisabledInputTipContribution.ts create mode 100644 src/vs/workbench/contrib/chat/test/browser/agentSessions/localAgentDisabledInputTipContribution.test.ts diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentDisabledInputTipContribution.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentDisabledInputTipContribution.ts new file mode 100644 index 0000000000000..3e2bf03e608af --- /dev/null +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentDisabledInputTipContribution.ts @@ -0,0 +1,115 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Disposable } from '../../../../../base/common/lifecycle.js'; +import { localize } from '../../../../../nls.js'; +import { CommandsRegistry } from '../../../../../platform/commands/common/commands.js'; +import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; +import { ServicesAccessor } from '../../../../../platform/instantiation/common/instantiation.js'; +import { IsSessionsWindowContext } from '../../../../common/contextkeys.js'; +import { IWorkbenchContribution } from '../../../../common/contributions.js'; +import { ChatConfiguration } from '../../common/constants.js'; +import { IChatSessionsService, localChatSessionType, SessionType } from '../../common/chatSessionsService.js'; +import { getChatSessionType } from '../../common/model/chatUri.js'; +import { IChatWidget, IChatWidgetService, isIChatResourceViewContext } from '../chat.js'; +import { ChatInputNotificationSeverity, IChatInputNotificationService } from '../widget/input/chatInputNotificationService.js'; + +const LOCAL_AGENT_DISABLED_NOTIFICATION_ID = 'chat.localAgentDisabled.continueInAgentHostCopilot'; +export const LOCAL_AGENT_DISABLED_CONTINUE_IN_AGENT_HOST_COPILOT_COMMAND_ID = '_chat.localAgentDisabled.continueInAgentHostCopilot'; + +CommandsRegistry.registerCommand(LOCAL_AGENT_DISABLED_CONTINUE_IN_AGENT_HOST_COPILOT_COMMAND_ID, async (accessor: ServicesAccessor) => { + const chatWidgetService = accessor.get(IChatWidgetService); + const chatSessionsService = accessor.get(IChatSessionsService); + const widget = chatWidgetService.lastFocusedWidget; + if (!widget || !chatSessionsService.getChatSessionContribution(SessionType.AgentHostCopilot)) { + return; + } + + widget.input.continueInSession(SessionType.AgentHostCopilot); +}); + +export class LocalAgentDisabledInputTipContribution extends Disposable implements IWorkbenchContribution { + + static readonly ID = 'workbench.contrib.localAgentDisabledInputTip'; + + private _lastPostedFor: string | undefined; + + constructor( + @IChatWidgetService private readonly chatWidgetService: IChatWidgetService, + @IChatInputNotificationService private readonly notificationService: IChatInputNotificationService, + @IConfigurationService private readonly configurationService: IConfigurationService, + @IChatSessionsService private readonly chatSessionsService: IChatSessionsService, + ) { + super(); + + this._register(this.chatWidgetService.onDidChangeFocusedSession(() => this.update())); + this._register(this.chatWidgetService.onDidAddWidget(() => this.update())); + this._register(this.chatSessionsService.onDidChangeAvailability(() => this.update())); + this._register(this.configurationService.onDidChangeConfiguration(e => { + if (e.affectsConfiguration(ChatConfiguration.EditorLocalAgentEnabled) || e.affectsConfiguration(ChatConfiguration.EditorDefaultProvider)) { + this.update(true); + } + })); + + this.update(); + } + + private update(resetDismissal = false): void { + const widget = this.chatWidgetService.lastFocusedWidget; + const sessionResource = widget?.viewModel?.sessionResource; + const key = sessionResource?.toString(); + + if (!widget || !sessionResource || !this.isEligible(widget)) { + this.clear(); + return; + } + + if (!resetDismissal && this._lastPostedFor === key) { + return; + } + + this._lastPostedFor = key; + this.notificationService.setNotification({ + id: LOCAL_AGENT_DISABLED_NOTIFICATION_ID, + severity: ChatInputNotificationSeverity.Info, + message: localize('chat.localAgentDisabled.continueInAgentHostCopilot.message', "Continue using the agent host."), + description: localize('chat.localAgentDisabled.continueInAgentHostCopilot.description', "You can bring your local harness history into a new chat with the agent host. To keep using the local harness instead and hide this notification, set \"chat.editor.localAgent.enabled\" to true."), + actions: [{ + label: localize('chat.localAgentDisabled.continueInAgentHostCopilot.action', "Continue In Agent Host"), + commandId: LOCAL_AGENT_DISABLED_CONTINUE_IN_AGENT_HOST_COPILOT_COMMAND_ID, + }], + dismissible: true, + autoDismissOnMessage: false, + sessionTypes: [localChatSessionType], + }); + } + + private isEligible(widget: IChatWidget): boolean { + const model = widget.viewModel?.model; + const sessionResource = widget.viewModel?.sessionResource; + return !!model + && !!sessionResource + && getChatSessionType(sessionResource) === localChatSessionType + && model.hasRequests + && this.configurationService.getValue(ChatConfiguration.EditorLocalAgentEnabled) === false + && this.configurationService.getValue(ChatConfiguration.EditorDefaultProvider) === 'copilotAh' + && !!this.chatSessionsService.getChatSessionContribution(SessionType.AgentHostCopilot) + && !this.isQuickOrInlineChat(widget) + && !IsSessionsWindowContext.getValue(widget.scopedContextKeyService); + } + + private isQuickOrInlineChat(widget: IChatWidget): boolean { + return isIChatResourceViewContext(widget.viewContext) + && (Boolean(widget.viewContext.isQuickChat) || Boolean(widget.viewContext.isInlineChat)); + } + + private clear(): void { + if (!this._lastPostedFor) { + return; + } + this._lastPostedFor = undefined; + this.notificationService.deleteNotification(LOCAL_AGENT_DISABLED_NOTIFICATION_ID); + } +} diff --git a/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts index f92ccac1cbc63..0b193aa517e01 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts @@ -57,6 +57,7 @@ import { ChatSlashCommandService, IChatSlashCommandService } from '../common/par import { ChatArtifactsService, IChatArtifactsService } from '../common/tools/chatArtifactsService.js'; import { ChatTodoListService, IChatTodoListService } from '../common/tools/chatTodoListService.js'; import { ChatTransferService, IChatTransferService } from '../common/model/chatTransferService.js'; +import { LocalAgentDisabledInputTipContribution } from './agentSessions/localAgentDisabledInputTipContribution.js'; import { IChatVariablesService } from '../common/attachments/chatVariables.js'; import { ChatWidgetHistoryService, IChatWidgetHistoryService } from '../common/widget/chatWidgetHistoryService.js'; import { ChatAgentLocation, ChatConfiguration, ChatNotificationMode, ChatPermissionLevel } from '../common/constants.js'; @@ -2389,6 +2390,7 @@ registerWorkbenchContribution2(ChatViewsWelcomeHandler.ID, ChatViewsWelcomeHandl registerWorkbenchContribution2(ChatGettingStartedContribution.ID, ChatGettingStartedContribution, WorkbenchPhase.Eventually); registerWorkbenchContribution2(ChatSetupContribution.ID, ChatSetupContribution, WorkbenchPhase.BlockRestore); registerWorkbenchContribution2(ChatQuotaNotificationContribution.ID, ChatQuotaNotificationContribution, WorkbenchPhase.AfterRestored); +registerWorkbenchContribution2(LocalAgentDisabledInputTipContribution.ID, LocalAgentDisabledInputTipContribution, WorkbenchPhase.AfterRestored); registerWorkbenchContribution2(HasByokModelsContribution.ID, HasByokModelsContribution, WorkbenchPhase.BlockRestore); registerWorkbenchContribution2(ChatTeardownContribution.ID, ChatTeardownContribution, WorkbenchPhase.AfterRestored); registerWorkbenchContribution2(ChatStatusBarEntry.ID, ChatStatusBarEntry, WorkbenchPhase.BlockRestore); diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts b/src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts index c98497e0e912a..4c589de5720ea 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts @@ -2428,6 +2428,11 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge * agent and model pickers. Re-selecting the active session clears the pending target and * restores the pickers. */ + public continueInSession(provider: AgentSessionTarget): void { + this.setPendingDelegationTarget(provider); + this.focus(); + } + private setPendingDelegationTarget(provider: AgentSessionTarget): void { const isActive = this.getActiveSessionTypeForDelegation() === provider; this._pendingDelegationTarget = isActive ? undefined : provider; diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/localAgentDisabledInputTipContribution.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/localAgentDisabledInputTipContribution.test.ts new file mode 100644 index 0000000000000..a4c235f80e851 --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/localAgentDisabledInputTipContribution.test.ts @@ -0,0 +1,284 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { Emitter, Event } from '../../../../../../base/common/event.js'; +import { DisposableStore } from '../../../../../../base/common/lifecycle.js'; +import { URI } from '../../../../../../base/common/uri.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; +import { CommandsRegistry } from '../../../../../../platform/commands/common/commands.js'; +import { ConfigurationTarget } from '../../../../../../platform/configuration/common/configuration.js'; +import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; +import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { MockContextKeyService } from '../../../../../../platform/keybinding/test/common/mockKeybindingService.js'; +import { IsSessionsWindowContext } from '../../../../../common/contextkeys.js'; +import { IChatWidget, IChatWidgetService, IChatWidgetViewContext } from '../../../browser/chat.js'; +import { LocalAgentDisabledInputTipContribution, LOCAL_AGENT_DISABLED_CONTINUE_IN_AGENT_HOST_COPILOT_COMMAND_ID } from '../../../browser/agentSessions/localAgentDisabledInputTipContribution.js'; +import { ChatInputNotificationSeverity, IChatInputNotification, IChatInputNotificationService } from '../../../browser/widget/input/chatInputNotificationService.js'; +import { IChatModel } from '../../../common/model/chatModel.js'; +import { LocalChatSessionUri } from '../../../common/model/chatUri.js'; +import { IChatViewModel } from '../../../common/model/chatViewModel.js'; +import { ChatConfiguration } from '../../../common/constants.js'; +import { IChatSessionsExtensionPoint, IChatSessionsService, localChatSessionType, SessionType } from '../../../common/chatSessionsService.js'; +import { MockChatSessionsService } from '../../common/mockChatSessionsService.js'; + +class TestChatWidgetService implements IChatWidgetService { + declare readonly _serviceBrand: undefined; + + private readonly _onDidAddWidget = new Emitter(); + readonly onDidAddWidget = this._onDidAddWidget.event; + + private readonly _onDidChangeFocusedSession = new Emitter(); + readonly onDidChangeFocusedSession = this._onDidChangeFocusedSession.event; + + readonly onDidBackgroundSession = Event.None; + readonly onDidChangeFocusedWidget = Event.None; + + lastFocusedWidget: IChatWidget | undefined; + + setFocusedWidget(widget: IChatWidget | undefined): void { + this.lastFocusedWidget = widget; + this._onDidChangeFocusedSession.fire(); + } + + fireDidAddWidget(widget: IChatWidget): void { + this._onDidAddWidget.fire(widget); + } + + revealWidget(): Promise { return Promise.resolve(undefined); } + reveal(): Promise { return Promise.resolve(true); } + getAllWidgets(): ReadonlyArray { return []; } + getWidgetByInputUri(): IChatWidget | undefined { return undefined; } + getWidgetBySessionResource(): IChatWidget | undefined { return undefined; } + getWidgetsByLocations(): ReadonlyArray { return []; } + openSession(): Promise { return Promise.resolve(undefined); } + register(): { dispose(): void } { return { dispose() { } }; } +} + +class TestChatInputNotificationService implements IChatInputNotificationService { + declare readonly _serviceBrand: undefined; + + readonly onDidChange = Event.None; + readonly onDidDismiss = Event.None; + readonly setCalls: IChatInputNotification[] = []; + readonly deleteCalls: string[] = []; + + setNotification(notification: IChatInputNotification): void { + this.setCalls.push(notification); + } + + deleteNotification(id: string): void { + this.deleteCalls.push(id); + } + + dismissNotification(): void { } + getActiveNotification(): IChatInputNotification | undefined { return this.setCalls.at(-1); } + handleMessageSent(): void { } +} + +suite('LocalAgentDisabledInputTipContribution', () => { + const store = ensureNoDisposablesAreLeakedInTestSuite(); + + let widgetService: TestChatWidgetService; + let notificationService: TestChatInputNotificationService; + let configurationService: TestConfigurationService; + let chatSessionsService: MockChatSessionsService; + let testDisposables: DisposableStore; + + setup(() => { + testDisposables = store.add(new DisposableStore()); + widgetService = new TestChatWidgetService(); + notificationService = new TestChatInputNotificationService(); + configurationService = new TestConfigurationService({ + [ChatConfiguration.EditorLocalAgentEnabled]: false, + [ChatConfiguration.EditorDefaultProvider]: 'copilotAh', + }); + chatSessionsService = new MockChatSessionsService(); + chatSessionsService.setContributions([createContribution(SessionType.AgentHostCopilot)]); + }); + + teardown(() => { + testDisposables.dispose(); + }); + + function createContribution(type: string): IChatSessionsExtensionPoint { + return { + type, + name: type, + displayName: type, + description: type, + }; + } + + function createTipContribution(): LocalAgentDisabledInputTipContribution { + const contribution = new LocalAgentDisabledInputTipContribution(widgetService, notificationService, configurationService, chatSessionsService); + testDisposables.add(contribution); + return contribution; + } + + function createWidget(options?: { + readonly sessionResource?: URI; + readonly hasRequests?: boolean; + readonly viewContext?: IChatWidgetViewContext; + readonly isSessionsWindow?: boolean; + }): IChatWidget { + const contextKeyService = new MockContextKeyService(); + IsSessionsWindowContext.bindTo(contextKeyService).set(options?.isSessionsWindow ?? false); + + const sessionResource = options?.sessionResource ?? LocalChatSessionUri.forSession('history'); + const model = { hasRequests: options?.hasRequests ?? true } as IChatModel; + const viewModel = { sessionResource, model } as IChatViewModel; + + return { + viewModel, + viewContext: options?.viewContext ?? { viewId: 'workbench.panel.chat' }, + scopedContextKeyService: contextKeyService, + } as unknown as IChatWidget; + } + + function fireConfigChange(...keys: string[]): void { + configurationService.onDidChangeConfigurationEmitter.fire({ + source: ConfigurationTarget.USER, + affectsConfiguration: (key: string) => keys.includes(key), + affectedKeys: new Set(keys), + change: { keys, overrides: [] }, + }); + } + + test('shows tip for non-empty local session when local is disabled and agent host Copilot is configured', () => { + createTipContribution(); + + widgetService.setFocusedWidget(createWidget()); + + assert.strictEqual(notificationService.setCalls.length, 1); + assert.strictEqual(notificationService.setCalls[0].id, 'chat.localAgentDisabled.continueInAgentHostCopilot'); + assert.strictEqual(notificationService.setCalls[0].severity, ChatInputNotificationSeverity.Info); + assert.strictEqual(notificationService.setCalls[0].actions.length, 1); + assert.strictEqual(notificationService.setCalls[0].actions[0].label, 'Continue In Agent Host'); + assert.strictEqual(notificationService.setCalls[0].actions[0].commandId, LOCAL_AGENT_DISABLED_CONTINUE_IN_AGENT_HOST_COPILOT_COMMAND_ID); + assert.deepStrictEqual(notificationService.setCalls[0].sessionTypes, [localChatSessionType]); + }); + + test('continue action selects agent host Copilot as pending delegation target', async () => { + const instantiationService = new TestInstantiationService(); + instantiationService.set(IChatWidgetService, widgetService); + instantiationService.set(IChatSessionsService, chatSessionsService); + + let continuedInSession: string | undefined; + widgetService.setFocusedWidget(Object.assign(createWidget(), { + input: { + continueInSession: (provider: string) => continuedInSession = provider, + }, + }) as unknown as IChatWidget); + + const command = CommandsRegistry.getCommand(LOCAL_AGENT_DISABLED_CONTINUE_IN_AGENT_HOST_COPILOT_COMMAND_ID); + assert.ok(command); + + await command.handler(instantiationService); + + assert.strictEqual(continuedInSession, SessionType.AgentHostCopilot); + }); + + test('shows tip for normal Chat view sessions', () => { + createTipContribution(); + + widgetService.setFocusedWidget(createWidget({ viewContext: { viewId: 'workbench.panel.chat' } })); + + assert.strictEqual(notificationService.setCalls.length, 1); + }); + + test('does not show tip for empty local session', () => { + createTipContribution(); + + widgetService.setFocusedWidget(createWidget({ hasRequests: false })); + + assert.strictEqual(notificationService.setCalls.length, 0); + }); + + test('does not show tip when local is enabled', () => { + configurationService = new TestConfigurationService({ + [ChatConfiguration.EditorLocalAgentEnabled]: true, + [ChatConfiguration.EditorDefaultProvider]: 'copilotAh', + }); + createTipContribution(); + + widgetService.setFocusedWidget(createWidget()); + + assert.strictEqual(notificationService.setCalls.length, 0); + }); + + test('does not show tip when default provider is not agent host Copilot', () => { + configurationService = new TestConfigurationService({ + [ChatConfiguration.EditorLocalAgentEnabled]: false, + [ChatConfiguration.EditorDefaultProvider]: 'copilotEh', + }); + createTipContribution(); + + widgetService.setFocusedWidget(createWidget()); + + assert.strictEqual(notificationService.setCalls.length, 0); + }); + + test('does not show tip before agent host Copilot contribution registers', () => { + chatSessionsService.setContributions([]); + createTipContribution(); + + widgetService.setFocusedWidget(createWidget()); + + assert.strictEqual(notificationService.setCalls.length, 0); + }); + + test('does not show tip for non-local sessions', () => { + createTipContribution(); + + widgetService.setFocusedWidget(createWidget({ sessionResource: URI.from({ scheme: SessionType.AgentHostCopilot, path: '/session' }) })); + + assert.strictEqual(notificationService.setCalls.length, 0); + }); + + test('does not show tip for quick, inline, or Agents Window sessions', () => { + createTipContribution(); + + widgetService.setFocusedWidget(createWidget({ viewContext: { isQuickChat: true } })); + widgetService.setFocusedWidget(createWidget({ viewContext: { isInlineChat: true } })); + widgetService.setFocusedWidget(createWidget({ isSessionsWindow: true })); + + assert.strictEqual(notificationService.setCalls.length, 0); + }); + + test('clears tip when focused session becomes ineligible', () => { + createTipContribution(); + widgetService.setFocusedWidget(createWidget()); + + widgetService.setFocusedWidget(createWidget({ hasRequests: false })); + + assert.deepStrictEqual(notificationService.deleteCalls, ['chat.localAgentDisabled.continueInAgentHostCopilot']); + }); + + test('does not repost for the same eligible session unless relevant configuration changes', async () => { + createTipContribution(); + const widget = createWidget(); + widgetService.setFocusedWidget(widget); + + widgetService.setFocusedWidget(widget); + assert.strictEqual(notificationService.setCalls.length, 1); + + await configurationService.setUserConfiguration(ChatConfiguration.EditorDefaultProvider, 'copilotAh'); + fireConfigChange(ChatConfiguration.EditorDefaultProvider); + + assert.strictEqual(notificationService.setCalls.length, 2); + }); + + test('shows tip when agent host Copilot contribution registers after focus', () => { + chatSessionsService.setContributions([]); + createTipContribution(); + widgetService.setFocusedWidget(createWidget()); + + chatSessionsService.setContributions([createContribution(SessionType.AgentHostCopilot)]); + chatSessionsService.fireDidChangeAvailability(); + + assert.strictEqual(notificationService.setCalls.length, 1); + }); +}); From 3d9770762219de347df3adb6144ae85693ee056b Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 19 Jun 2026 13:36:16 +1000 Subject: [PATCH 6/7] Add session restore coalescing and error handling in AgentService (#322022) --- .../platform/agentHost/node/agentService.ts | 44 ++++++ .../agentHost/test/node/agentService.test.ts | 140 +++++++++++++++++- 2 files changed, 183 insertions(+), 1 deletion(-) diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index 8617a2e5ca9d7..ff235e574dbba 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -154,6 +154,8 @@ export class AgentService extends Disposable implements IAgentService { * for it. */ private readonly _resourceSubscribers = new ResourceMap>(); + private readonly _restoreSessionInFlight = new Map>(); + private readonly _restoreSubagentInFlight = new Map>(); /** * Pending {@link _runSessionGc} timers, keyed by session URI. A timer is @@ -1395,6 +1397,27 @@ export class AgentService extends Disposable implements IAgentService { return; } + const inFlight = this._restoreSessionInFlight.get(sessionStr); + if (inFlight) { + return inFlight; + } + + const restore = this._doRestoreSession(session, sessionStr); + this._restoreSessionInFlight.set(sessionStr, restore); + try { + await restore; + } finally { + if (this._restoreSessionInFlight.get(sessionStr) === restore) { + this._restoreSessionInFlight.delete(sessionStr); + } + } + } + + private async _doRestoreSession(session: URI, sessionStr: string): Promise { + if (this._stateManager.getSessionState(sessionStr)) { + return; + } + const agent = this._findProviderForSession(session); if (!agent) { throw new ProtocolError(AHP_SESSION_NOT_FOUND, `No agent for session: ${sessionStr}`); @@ -2064,6 +2087,27 @@ export class AgentService extends Disposable implements IAgentService { * turns from those events. */ private async _restoreSubagentSession(subagentUri: string, parentSession: URI): Promise { + if (this._stateManager.getSessionState(subagentUri)) { + return; + } + + const inFlight = this._restoreSubagentInFlight.get(subagentUri); + if (inFlight) { + return inFlight; + } + + const restore = this._doRestoreSubagentSession(subagentUri, parentSession); + this._restoreSubagentInFlight.set(subagentUri, restore); + try { + await restore; + } finally { + if (this._restoreSubagentInFlight.get(subagentUri) === restore) { + this._restoreSubagentInFlight.delete(subagentUri); + } + } + } + + private async _doRestoreSubagentSession(subagentUri: string, parentSession: URI): Promise { // Ensure the parent session is loaded first const parentSessionKey = parentSession.toString(); if (!this._stateManager.getSessionState(parentSessionKey)) { diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index 4f037b018857c..64d6ed82bb92a 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -9,6 +9,7 @@ import type { CCAModel } from '@vscode/copilot-api'; import { mkdtempSync, readFileSync, rmSync } from 'fs'; import { tmpdir } from 'os'; import { fileURLToPath } from 'url'; +import { DeferredPromise } from '../../../../base/common/async.js'; import { encodeBase64, VSBuffer } from '../../../../base/common/buffer.js'; import { Emitter, Event } from '../../../../base/common/event.js'; import { DisposableStore, IReference, toDisposable } from '../../../../base/common/lifecycle.js'; @@ -25,7 +26,7 @@ import { AgentSession, GITHUB_COPILOT_PROTECTED_RESOURCE } from '../../common/ag import { ISessionDatabase, ISessionDataService } from '../../common/sessionDataService.js'; import { SessionDatabase } from '../../node/sessionDatabase.js'; import { ActionType, ActionEnvelope } from '../../common/state/sessionActions.js'; -import { ChangesetStatus, CustomizationType, MessageAttachmentKind, MessageKind, SessionActiveClient, ResponsePartKind, ROOT_STATE_URI, SessionLifecycle, SessionStatus, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildChatUri, buildSubagentSessionUri, customizationId, isSubagentSession, type ChangesetState, type MarkdownResponsePart, type ToolCallCompletedState, type ToolCallResponsePart, type Turn } from '../../common/state/sessionState.js'; +import { ChangesetStatus, CustomizationType, MessageAttachmentKind, MessageKind, SessionActiveClient, ResponsePartKind, ROOT_STATE_URI, SessionLifecycle, SessionStatus, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildChatUri, buildSubagentSessionUri, customizationId, isSubagentSession, parseSubagentSessionUri, type ChangesetState, type MarkdownResponsePart, type ToolCallCompletedState, type ToolCallResponsePart, type Turn } from '../../common/state/sessionState.js'; import { type MessageResourceAttachment } from '../../common/state/protocol/state.js'; import { IProductService } from '../../../product/common/productService.js'; import { AgentService } from '../../node/agentService.js'; @@ -1869,6 +1870,91 @@ suite('AgentService (node dispatcher)', () => { }); }); + test('coalesces concurrent restores for the same session', async () => { + class BlockingRestoreAgent extends MockAgent { + readonly metadataReached = new DeferredPromise(); + readonly metadataGate = new DeferredPromise(); + getSessionMetadataCalls = 0; + getSessionMessagesCalls = 0; + + override async getSessionMetadata(session: URI) { + this.getSessionMetadataCalls++; + this.metadataReached.complete(); + await this.metadataGate.p; + return super.getSessionMetadata(session); + } + + override async getSessionMessages(session: URI): Promise { + this.getSessionMessagesCalls++; + return super.getSessionMessages(session); + } + } + + const agent = disposables.add(new BlockingRestoreAgent('copilot')); + service.registerProvider(agent); + const { session } = await agent.createSession(); + service.stateManager.deleteSession(session.toString()); + agent.sessionMessages = [ + { type: 'message', session, role: 'user', messageId: 'msg-1', content: 'Hello', toolRequests: [] }, + { type: 'message', session, role: 'assistant', messageId: 'msg-2', content: 'Hi', toolRequests: [] }, + ]; + + const firstRestore = service.restoreSession(session); + await agent.metadataReached.p; + const secondRestore = service.restoreSession(session); + + assert.strictEqual(agent.getSessionMetadataCalls, 1); + agent.metadataGate.complete(); + await Promise.all([firstRestore, secondRestore]); + + assert.deepStrictEqual({ + metadataCalls: agent.getSessionMetadataCalls, + messageCalls: agent.getSessionMessagesCalls, + restored: !!service.stateManager.getSessionState(session.toString()), + }, { + metadataCalls: 1, + messageCalls: 1, + restored: true, + }); + }); + + test('clears failed restore attempts so sessions can be retried', async () => { + class FailingOnceRestoreAgent extends MockAgent { + shouldFailRestore = true; + getSessionMessagesCalls = 0; + + override async getSessionMessages(session: URI): Promise { + this.getSessionMessagesCalls++; + if (this.shouldFailRestore) { + throw new Error('restore failed'); + } + return super.getSessionMessages(session); + } + } + + const agent = disposables.add(new FailingOnceRestoreAgent('copilot')); + service.registerProvider(agent); + const { session } = await agent.createSession(); + service.stateManager.deleteSession(session.toString()); + agent.sessionMessages = [ + { type: 'message', session, role: 'user', messageId: 'msg-1', content: 'Hello', toolRequests: [] }, + { type: 'message', session, role: 'assistant', messageId: 'msg-2', content: 'Hi', toolRequests: [] }, + ]; + + await assert.rejects(() => service.restoreSession(session), /restore failed/); + + agent.shouldFailRestore = false; + await service.restoreSession(session); + + assert.deepStrictEqual({ + messageCalls: agent.getSessionMessagesCalls, + restored: !!service.stateManager.getSessionState(session.toString()), + }, { + messageCalls: 2, + restored: true, + }); + }); + test('restores a session with subagent tool calls', async () => { service.registerProvider(copilotAgent); const { session } = await copilotAgent.createSession(); @@ -1980,6 +2066,58 @@ suite('AgentService (node dispatcher)', () => { const mdParts = state!.turns[0].responseParts.filter((p): p is MarkdownResponsePart => p.kind === ResponsePartKind.Markdown); assert.ok(mdParts.length > 0, 'Should have markdown content'); }); + + test('coalesces concurrent restores for the same subagent session', async () => { + class BlockingSubagentAgent extends MockAgent { + readonly subagentReached = new DeferredPromise(); + readonly subagentGate = new DeferredPromise(); + subagentGetSessionMessagesCalls = 0; + + override async getSessionMessages(session: URI): Promise { + if (parseSubagentSessionUri(session)) { + this.subagentGetSessionMessagesCalls++; + this.subagentReached.complete(); + await this.subagentGate.p; + } + return super.getSessionMessages(session); + } + } + + const agent = disposables.add(new BlockingSubagentAgent('copilot')); + service.registerProvider(agent); + const { session } = await agent.createSession(); + const sessions = await agent.listSessions(); + const sessionResource = sessions[0].session; + + agent.sessionMessages = [ + { type: 'message', session, role: 'user', messageId: 'msg-1', content: 'Review', toolRequests: [] }, + { type: 'message', session, role: 'assistant', messageId: 'msg-2', content: '', toolRequests: [{ toolCallId: 'tc-sub', name: 'task' }] }, + { type: 'tool_start', session, toolCallId: 'tc-sub', toolName: 'task', displayName: 'Task', invocationMessage: 'Delegating...', toolKind: 'subagent' as const, subagentDescription: 'Find related files', subagentAgentName: 'explore' }, + { type: 'subagent_started', session, toolCallId: 'tc-sub', agentName: 'explore', agentDisplayName: 'Explore', agentDescription: 'Explores the codebase' }, + { type: 'tool_start', session, toolCallId: 'tc-inner', toolName: 'bash', displayName: 'Bash', invocationMessage: 'Running ls...', parentToolCallId: 'tc-sub' }, + { type: 'tool_complete', session, toolCallId: 'tc-inner', result: { success: true, pastTenseMessage: 'Ran ls', content: [{ type: ToolResultContentType.Text, text: 'file1.ts' }] }, parentToolCallId: 'tc-sub' }, + { type: 'tool_complete', session, toolCallId: 'tc-sub', result: { success: true, pastTenseMessage: 'Delegated task', content: [{ type: ToolResultContentType.Text, text: 'Found files' }] } }, + { type: 'message', session, role: 'assistant', messageId: 'msg-3', content: 'Done.', toolRequests: [] }, + ]; + await service.restoreSession(sessionResource); + + const childSessionUri = URI.parse(buildSubagentSessionUri(sessionResource.toString(), 'tc-sub')); + const firstSubscribe = service.subscribe(childSessionUri, 'client-1'); + await agent.subagentReached.p; + const secondSubscribe = service.subscribe(childSessionUri, 'client-2'); + + assert.strictEqual(agent.subagentGetSessionMessagesCalls, 1); + agent.subagentGate.complete(); + await Promise.all([firstSubscribe, secondSubscribe]); + + assert.deepStrictEqual({ + messageCalls: agent.subagentGetSessionMessagesCalls, + childTurns: service.stateManager.getSessionState(childSessionUri.toString())?.turns.length, + }, { + messageCalls: 1, + childTurns: 1, + }); + }); }); // ---- createChat (multi-chat) ---------------------------------------- From 17df838042619b0ba24d27e130d693f0806f80e9 Mon Sep 17 00:00:00 2001 From: Anthony Kim <62267334+anthonykim1@users.noreply.github.com> Date: Thu, 18 Jun 2026 20:36:20 -0700 Subject: [PATCH 7/7] Improve terminal file write path quote normalization (#322019) * Improve terminal file write path quote normalization * address feedback --- .../commandLineFileWriteAnalyzer.ts | 11 ++++++----- .../commandLineFileWriteAnalyzer.test.ts | 4 ++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineFileWriteAnalyzer.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineFileWriteAnalyzer.ts index 50316fd08bfe1..2f77821b26162 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineFileWriteAnalyzer.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineFileWriteAnalyzer.ts @@ -96,13 +96,14 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand } private _stripSurroundingQuotes(text: string): string { - if ( - (text.startsWith('"') && text.endsWith('"')) || - (text.startsWith('\'') && text.endsWith('\'')) + let result = text; + while ( + (result.startsWith('"') && result.endsWith('"')) || + (result.startsWith('\'') && result.endsWith('\'')) ) { - return text.slice(1, -1); + result = result.slice(1, -1); } - return text; + return result; } private _mapNullDevice(options: ICommandLineAnalyzerOptions, rawFileWrite: string): string | typeof nullDevice { diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineAnalyzer/commandLineFileWriteAnalyzer.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineAnalyzer/commandLineFileWriteAnalyzer.test.ts index 6074edd320433..ee1db744354d8 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineAnalyzer/commandLineFileWriteAnalyzer.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineAnalyzer/commandLineFileWriteAnalyzer.test.ts @@ -110,6 +110,9 @@ suite('CommandLineFileWriteAnalyzer', () => { test('absolute path - /home - block', () => t('echo hello > /home/user/file.txt', 'outsideWorkspace', false, 1)); test('absolute path - root - block', () => t('echo hello > /file.txt', 'outsideWorkspace', false, 1)); test('absolute path - /dev/null - allow (null device)', () => t('echo hello > /dev/null', 'outsideWorkspace', true, 1)); + test('triple-quoted absolute path outside workspace - block', () => t('echo hello > \'\'\'/tmp/file.txt\'\'\'', 'outsideWorkspace', false, 1)); + test('triple-quoted settings path outside workspace - block', () => t('echo "{}" > \'\'\'/home/user/.config/Code/User/settings.json\'\'\'', 'outsideWorkspace', false, 1)); + test('triple double-quoted absolute path outside workspace - block', () => t('echo hello > """/tmp/file.txt"""', 'outsideWorkspace', false, 1)); // Special cases test('no workspace folders - block', () => t('echo hello > file.txt', 'outsideWorkspace', false, 1, [])); @@ -161,6 +164,7 @@ suite('CommandLineFileWriteAnalyzer', () => { // /tmp writes are allowed only when session auto-approval is enabled test('/tmp - allow when auto-approval enabled', () => tWithAutoApproval('echo hello > /tmp/file.txt', 'outsideWorkspace', true, true)); test('/tmp subdirectory - allow when auto-approval enabled', () => tWithAutoApproval('echo hello > /tmp/sub/file.txt', 'outsideWorkspace', true, true)); + test('triple-quoted /tmp - allow when auto-approval enabled', () => tWithAutoApproval('echo hello > \'\'\'/tmp/file.txt\'\'\'', 'outsideWorkspace', true, true)); test('/tmp - block when auto-approval disabled', () => tWithAutoApproval('echo hello > /tmp/file.txt', 'outsideWorkspace', false, false)); // Other outside-workspace paths remain blocked even with auto-approval enabled