From 64b470f88a00634fca0f2676ff67885337529628 Mon Sep 17 00:00:00 2001 From: cwebster-99 Date: Fri, 17 Apr 2026 15:39:18 -0500 Subject: [PATCH 01/13] Change close button --- .../browser/media/variationA.css | 31 +++++++++++++++++++ .../browser/onboardingVariationA.ts | 29 +++++++---------- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/vs/workbench/contrib/welcomeOnboarding/browser/media/variationA.css b/src/vs/workbench/contrib/welcomeOnboarding/browser/media/variationA.css index 8945687a86ca2..cacd6a0acc698 100644 --- a/src/vs/workbench/contrib/welcomeOnboarding/browser/media/variationA.css +++ b/src/vs/workbench/contrib/welcomeOnboarding/browser/media/variationA.css @@ -42,6 +42,7 @@ display: flex; flex-direction: column; overflow: hidden; + position: relative; transform: scale(0.96) translateY(8px); transition: transform 0.3s cubic-bezier(0.16, 1, 0.3, 1); } @@ -61,6 +62,36 @@ padding: 20px 28px 0; } +.onboarding-a-close-btn { + position: absolute; + top: 16px; + right: 16px; + z-index: 1; + display: flex; + align-items: center; + justify-content: center; + width: 28px; + height: 28px; + border: none; + border-radius: 6px; + background: transparent; + color: var(--vscode-descriptionForeground); + cursor: pointer; + padding: 0; + flex-shrink: 0; + transition: background 0.15s ease, color 0.15s ease; +} + +.onboarding-a-close-btn:hover { + background: var(--vscode-toolbar-hoverBackground, rgba(255, 255, 255, 0.1)); + color: var(--vscode-editor-foreground); +} + +.onboarding-a-close-btn:focus-visible { + outline: 2px solid var(--vscode-focusBorder); + outline-offset: 2px; +} + .onboarding-a-progress { display: flex; align-items: center; diff --git a/src/vs/workbench/contrib/welcomeOnboarding/browser/onboardingVariationA.ts b/src/vs/workbench/contrib/welcomeOnboarding/browser/onboardingVariationA.ts index e1486cb5c587d..d1551a512b674 100644 --- a/src/vs/workbench/contrib/welcomeOnboarding/browser/onboardingVariationA.ts +++ b/src/vs/workbench/contrib/welcomeOnboarding/browser/onboardingVariationA.ts @@ -105,7 +105,7 @@ export class OnboardingVariationA extends Disposable implements IOnboardingServi private contentEl: HTMLElement | undefined; private backButton: HTMLButtonElement | undefined; private nextButton: HTMLButtonElement | undefined; - private skipButton: HTMLButtonElement | undefined; + private closeButton: HTMLButtonElement | undefined; private footerLeft: HTMLElement | undefined; private _footerSignInBtn: HTMLButtonElement | undefined; @@ -176,6 +176,13 @@ export class OnboardingVariationA extends Disposable implements IOnboardingServi // Card this.card = append(this.overlay, $('.onboarding-a-card')); + // Close button (upper-right corner of card) + this.closeButton = append(this.card, $('button.onboarding-a-close-btn')); + this.closeButton.type = 'button'; + this.closeButton.setAttribute('aria-label', localize('onboarding.close', "Close")); + this.closeButton.appendChild(renderIcon(Codicon.close)); + this.footerFocusableElements.push(this.closeButton); + // Header with progress const header = append(this.card, $('.onboarding-a-header')); this.progressContainer = append(header, $('.onboarding-a-progress')); @@ -194,10 +201,6 @@ export class OnboardingVariationA extends Disposable implements IOnboardingServi const footer = append(this.card, $('.onboarding-a-footer')); this.footerLeft = append(footer, $('.onboarding-a-footer-left')); - this.skipButton = append(this.footerLeft, $('button.onboarding-a-btn.onboarding-a-btn-ghost')); - this.skipButton.textContent = localize('onboarding.skip', "Skip"); - this.skipButton.type = 'button'; - this.footerFocusableElements.push(this.skipButton); const footerRight = append(footer, $('.onboarding-a-footer-right')); @@ -212,7 +215,7 @@ export class OnboardingVariationA extends Disposable implements IOnboardingServi this._updateButtonStates(); // Event handlers - this.disposables.add(addDisposableListener(this.skipButton, EventType.CLICK, () => { + this.disposables.add(addDisposableListener(this.closeButton, EventType.CLICK, () => { this._logAction('skip'); this._dismiss('skip'); })); @@ -412,15 +415,8 @@ export class OnboardingVariationA extends Disposable implements IOnboardingServi this.nextButton.textContent = localize('onboarding.next', "Continue"); } } - if (this.skipButton && this.footerLeft) { - if (this.currentStepIndex === 0) { - // Sign-in step: ghost Skip button - this.skipButton.className = 'onboarding-a-btn onboarding-a-btn-ghost'; - } else { - this.skipButton.className = 'onboarding-a-btn onboarding-a-btn-ghost'; - } + if (this.footerLeft) { if (this._isLastStep()) { - this.skipButton.style.display = 'none'; // Show sign-in nudge in footer if (!this._footerSignInBtn && !this._userSignedIn) { this._footerSignInBtn = append(this.footerLeft, $('button.onboarding-a-signin-nudge-btn')); @@ -435,7 +431,6 @@ export class OnboardingVariationA extends Disposable implements IOnboardingServi })); } } else { - this.skipButton.style.display = ''; if (this._footerSignInBtn) { this._footerSignInBtn.remove(); this._footerSignInBtn = undefined; @@ -1154,7 +1149,7 @@ export class OnboardingVariationA extends Disposable implements IOnboardingServi private _focusCurrentStepElement(): void { const stepFocusable = this.stepFocusableElements.find(element => this._isTabbable(element)); - (stepFocusable ?? this.nextButton ?? this.skipButton)?.focus(); + (stepFocusable ?? this.nextButton ?? this.closeButton)?.focus(); } private _registerStepFocusable(element: T): T { @@ -1210,7 +1205,7 @@ export class OnboardingVariationA extends Disposable implements IOnboardingServi this.contentEl = undefined; this.backButton = undefined; this.nextButton = undefined; - this.skipButton = undefined; + this.closeButton = undefined; this.footerLeft = undefined; this._footerSignInBtn = undefined; this.footerFocusableElements.length = 0; From cd2e1d8ed0550accc23671bfb3575acb345feced Mon Sep 17 00:00:00 2001 From: cwebster-99 Date: Fri, 17 Apr 2026 16:08:25 -0500 Subject: [PATCH 02/13] Refactor onboarding close button handling to ensure it is focusable Co-authored-by: Copilot --- .../contrib/welcomeOnboarding/browser/onboardingVariationA.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/welcomeOnboarding/browser/onboardingVariationA.ts b/src/vs/workbench/contrib/welcomeOnboarding/browser/onboardingVariationA.ts index d1551a512b674..7bea08f1ad062 100644 --- a/src/vs/workbench/contrib/welcomeOnboarding/browser/onboardingVariationA.ts +++ b/src/vs/workbench/contrib/welcomeOnboarding/browser/onboardingVariationA.ts @@ -181,7 +181,6 @@ export class OnboardingVariationA extends Disposable implements IOnboardingServi this.closeButton.type = 'button'; this.closeButton.setAttribute('aria-label', localize('onboarding.close', "Close")); this.closeButton.appendChild(renderIcon(Codicon.close)); - this.footerFocusableElements.push(this.closeButton); // Header with progress const header = append(this.card, $('.onboarding-a-header')); @@ -1144,7 +1143,7 @@ export class OnboardingVariationA extends Disposable implements IOnboardingServi } private _getFocusableElements(): HTMLElement[] { - return [...this.stepFocusableElements, ...this.footerFocusableElements].filter(element => this._isTabbable(element)); + return [...(this.closeButton ? [this.closeButton] : []), ...this.stepFocusableElements, ...this.footerFocusableElements].filter(element => this._isTabbable(element)); } private _focusCurrentStepElement(): void { From 886c556841c7d1d31d9333b44e2cd332c380c7e3 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Fri, 17 Apr 2026 18:15:48 -0700 Subject: [PATCH 03/13] agentHost: adopt eager activeClient announcement --- .../browser/remoteAgentHostProtocolClient.ts | 1 + .../platform/agentHost/common/agentService.ts | 10 ++- .../common/state/protocol/.ahp-version | 2 +- .../common/state/protocol/commands.ts | 11 ++- .../agentHost/node/copilot/copilotAgent.ts | 7 ++ .../agentHost/node/protocolServerHandler.ts | 14 ++++ .../agentHost/test/node/copilotAgent.test.ts | 73 ++++++++++++++++++- .../test/node/protocolServerHandler.test.ts | 73 +++++++++++++++++++ .../agentHost/agentHostSessionHandler.ts | 12 ++- .../agentHostChatContribution.test.ts | 35 +++++++-- 10 files changed, 222 insertions(+), 16 deletions(-) diff --git a/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts b/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts index 601c78308de09..70bdbae725dc9 100644 --- a/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts +++ b/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts @@ -188,6 +188,7 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC model: config?.model, workingDirectory: config?.workingDirectory ? fromAgentHostUri(config.workingDirectory).toString() : undefined, config: config?.config, + activeClient: config?.activeClient, }); return session; } diff --git a/src/vs/platform/agentHost/common/agentService.ts b/src/vs/platform/agentHost/common/agentService.ts index 7116ca6c0b446..0d11bb4aaf101 100644 --- a/src/vs/platform/agentHost/common/agentService.ts +++ b/src/vs/platform/agentHost/common/agentService.ts @@ -12,7 +12,7 @@ import { createDecorator } from '../../instantiation/common/instantiation.js'; import type { ISyncedCustomization } from './agentPluginManager.js'; import type { IAgentSubscription } from './state/agentSubscription.js'; import type { ICreateTerminalParams, IResolveSessionConfigResult, ISessionConfigCompletionsResult } from './state/protocol/commands.js'; -import { IProtectedResourceMetadata, type IConfigSchema, type IFileEdit, type IModelSelection, type IToolDefinition } from './state/protocol/state.js'; +import { IProtectedResourceMetadata, type IConfigSchema, type IFileEdit, type IModelSelection, type ISessionActiveClient, type IToolDefinition } from './state/protocol/state.js'; import type { IActionEnvelope, INotification, ISessionAction, ITerminalAction } from './state/sessionActions.js'; import type { IResourceCopyParams, IResourceCopyResult, IResourceDeleteParams, IResourceDeleteResult, IResourceListResult, IResourceMoveParams, IResourceMoveResult, IResourceReadResult, IResourceWriteParams, IResourceWriteResult, IStateSnapshot } from './state/sessionProtocol.js'; import { AttachmentType, ComponentToState, SessionInputResponseKind, SessionStatus, StateComponents, type ICustomizationRef, type IPendingMessage, type IRootState, type ISessionInputAnswer, type ISessionInputRequest, type IToolCallResult, type IToolResultContent, type PolicyState, type StringOrMarkdown } from './state/sessionState.js'; @@ -125,6 +125,14 @@ export interface IAgentCreateSessionConfig { readonly session?: URI; readonly workingDirectory?: URI; readonly config?: Record; + /** + * Eagerly claim the active client role for the new session. When provided, + * the server initializes the session with this client as the active + * client, equivalent to dispatching a `session/activeClientChanged` + * action immediately after creation. The `clientId` MUST match the + * connection's own `clientId`. + */ + readonly activeClient?: ISessionActiveClient; /** Fork from an existing session at a specific turn. */ readonly fork?: { readonly session: URI; diff --git a/src/vs/platform/agentHost/common/state/protocol/.ahp-version b/src/vs/platform/agentHost/common/state/protocol/.ahp-version index 79967dc6f68a0..f9c684bfe333f 100644 --- a/src/vs/platform/agentHost/common/state/protocol/.ahp-version +++ b/src/vs/platform/agentHost/common/state/protocol/.ahp-version @@ -1 +1 @@ -ab467b2 +0947b17 diff --git a/src/vs/platform/agentHost/common/state/protocol/commands.ts b/src/vs/platform/agentHost/common/state/protocol/commands.ts index f868bd271ae8d..a68103156dd8b 100644 --- a/src/vs/platform/agentHost/common/state/protocol/commands.ts +++ b/src/vs/platform/agentHost/common/state/protocol/commands.ts @@ -6,7 +6,7 @@ // allow-any-unicode-comment-file // DO NOT EDIT -- auto-generated by scripts/sync-agent-host-protocol.ts -import type { URI, ISnapshot, ISessionConfigSchema, ISessionSummary, IModelSelection, ITurn, ITerminalClaim } from './state.js'; +import type { URI, ISnapshot, ISessionConfigSchema, ISessionSummary, IModelSelection, ITurn, ITerminalClaim, ISessionActiveClient } from './state.js'; import type { IActionEnvelope, IStateAction } from './actions.js'; export type { IConfigPropertySchema, IConfigSchema, ISessionConfigPropertySchema, ISessionConfigSchema } from './state.js'; @@ -198,6 +198,15 @@ export interface ICreateSessionParams { * Keys and values correspond to the schema returned by the server. */ config?: Record; + /** + * Eagerly claim the active client role for the new session. + * + * When provided, the server initializes the session with this client as the + * active client, equivalent to dispatching a `session/activeClientChanged` + * action immediately after creation. The `clientId` MUST match the + * `clientId` the creating client supplied in `initialize`. + */ + activeClient?: ISessionActiveClient; } // ─── disposeSession ────────────────────────────────────────────────────────── diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts index 05de458b78de1..d87d8265722c4 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts @@ -449,6 +449,13 @@ export class CopilotAgent extends Disposable implements IAgent { const sessionId = config?.session ? AgentSession.id(config.session) : generateUuid(); const sessionUri = AgentSession.uri(this.id, sessionId); + if (config?.activeClient) { + const ac = this._getOrCreateActiveClient(sessionUri); + ac.updateTools(config.activeClient.clientId, config.activeClient.tools); + if (config.activeClient.customizations?.length) { + await this._plugins.sync(config.activeClient.clientId, config.activeClient.customizations); + } + } const activeClient = this._activeClients.get(sessionUri); const snapshot = activeClient ? await activeClient.snapshot() : undefined; const workingDirectory = await this._resolveSessionWorkingDirectory(config, sessionId); diff --git a/src/vs/platform/agentHost/node/protocolServerHandler.ts b/src/vs/platform/agentHost/node/protocolServerHandler.ts index 6bc28f66b6bf1..fdd01b739a30b 100644 --- a/src/vs/platform/agentHost/node/protocolServerHandler.ts +++ b/src/vs/platform/agentHost/node/protocolServerHandler.ts @@ -12,6 +12,7 @@ import { ILogService } from '../../log/common/log.js'; import { AHPFileSystemProvider } from '../common/agentHostFileSystemProvider.js'; import { AgentSession, type IAgentService } from '../common/agentService.js'; import type { ICommandMap } from '../common/state/protocol/messages.js'; +import { ActionType } from '../common/state/protocol/actions.js'; import { IActionEnvelope, INotification, isSessionAction, isTerminalAction, type ISessionAction } from '../common/state/sessionActions.js'; import { MIN_PROTOCOL_VERSION, PROTOCOL_VERSION } from '../common/state/sessionCapabilities.js'; import { @@ -368,6 +369,19 @@ export class ProtocolServerHandler extends Disposable { if (createdSession.toString() !== URI.parse(params.session).toString()) { this._logService.warn(`[ProtocolServer] createSession: provider returned URI ${createdSession.toString()} but client requested ${params.session}`); } + // If the client eagerly claimed the active client role, dispatch + // `session/activeClientChanged` now so the claim is atomic with + // creation. + if (params.activeClient) { + if (params.activeClient.clientId !== _client.clientId) { + throw new ProtocolError(JSON_RPC_INTERNAL_ERROR, `createSession.activeClient.clientId must match the connection's clientId`); + } + this._agentService.dispatchAction({ + type: ActionType.SessionActiveClientChanged, + session: createdSession.toString(), + activeClient: params.activeClient, + }, _client.clientId, 0); + } return null; }, disposeSession: async (_client, params) => { diff --git a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts index 40af0961605c8..557dfd54761db 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts @@ -127,14 +127,14 @@ class TestableCopilotAgent extends CopilotAgent { } } -function createTestAgent(disposables: Pick, options?: { sessionDataService?: ISessionDataService; copilotClient?: ICopilotClient }): CopilotAgent { +function createTestAgent(disposables: Pick, options?: { sessionDataService?: ISessionDataService; copilotClient?: ICopilotClient; pluginManager?: IAgentPluginManager }): CopilotAgent { const services = new ServiceCollection(); const logService = new NullLogService(); const fileService = disposables.add(new FileService(logService)); services.set(ILogService, logService); services.set(IFileService, fileService); services.set(ISessionDataService, options?.sessionDataService ?? createNullSessionDataService()); - services.set(IAgentPluginManager, new TestAgentPluginManager()); + services.set(IAgentPluginManager, options?.pluginManager ?? new TestAgentPluginManager()); services.set(IAgentHostGitService, new TestAgentHostGitService()); services.set(IAgentHostTerminalManager, new TestAgentHostTerminalManager()); const instantiationService: IInstantiationService = disposables.add(new InstantiationService(services)); @@ -272,4 +272,73 @@ suite('CopilotAgent', () => { await disposeAgent(agent); } }); + + suite('createSession activeClient eager-claim', () => { + + class SpyingPluginManager extends TestAgentPluginManager { + public readonly calls: { clientId: string; customizations: ICustomizationRef[] }[] = []; + + override async syncCustomizations(clientId: string, customizations: ICustomizationRef[], _progress?: (status: ISessionCustomization[]) => void): Promise { + this.calls.push({ clientId, customizations: [...customizations] }); + return []; + } + } + + test('createSession seeds activeClient tools and syncs customizations', async () => { + const sessionDataService = disposables.add(new TestSessionDataService()); + const client = new TestCopilotClient([]); + const pluginManager = new SpyingPluginManager(); + // Fail fast inside the SDK factory so we don't need to wire up a + // real raw session. The seeding of activeClient and the plugin + // sync both happen before `client.createSession` is invoked. + client.createSession = async () => { throw new Error('sentinel'); }; + + const agent = createTestAgent(disposables, { sessionDataService, copilotClient: client, pluginManager }); + try { + await agent.authenticate('https://api.github.com', 'token'); + + const customizations: ICustomizationRef[] = [{ uri: 'file:///plugin-a', displayName: 'Plugin A' }]; + await assert.rejects( + agent.createSession({ + session: AgentSession.uri('copilot', 'test-session'), + workingDirectory: URI.file('/workspace'), + activeClient: { + clientId: 'client-1', + tools: [{ name: 't1', description: 'd', inputSchema: { type: 'object' } }], + customizations, + }, + }), + (err: Error) => /sentinel/.test(err.message), + ); + + assert.deepStrictEqual(pluginManager.calls, [{ clientId: 'client-1', customizations }]); + } finally { + await disposeAgent(agent); + } + }); + + test('createSession without activeClient does not sync customizations', async () => { + const sessionDataService = disposables.add(new TestSessionDataService()); + const client = new TestCopilotClient([]); + const pluginManager = new SpyingPluginManager(); + client.createSession = async () => { throw new Error('sentinel'); }; + + const agent = createTestAgent(disposables, { sessionDataService, copilotClient: client, pluginManager }); + try { + await agent.authenticate('https://api.github.com', 'token'); + + await assert.rejects( + agent.createSession({ + session: AgentSession.uri('copilot', 'test-session-2'), + workingDirectory: URI.file('/workspace'), + }), + (err: Error) => /sentinel/.test(err.message), + ); + + assert.deepStrictEqual(pluginManager.calls, []); + } finally { + await disposeAgent(agent); + } + }); + }); }); diff --git a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts index 0ceb1291a2555..fff677d85194b 100644 --- a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts +++ b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts @@ -577,4 +577,77 @@ suite('ProtocolServerHandler', () => { transport2.simulateClose(); assert.deepStrictEqual(counts, [1, 1, 0]); }); + + // ---- createSession activeClient ------------------------------------- + + suite('createSession activeClient', () => { + + test('forwards activeClient as SessionActiveClientChanged', async () => { + const newSession = URI.parse('copilot:///eager-session').toString(); + // Pre-create the session so the client can subscribe at handshake + // time. MockAgentService.createSession below is idempotent against + // state-manager duplicates. + stateManager.createSession(makeSessionSummary(newSession)); + + const transport = connectClient('client-1', [newSession]); + transport.sent.length = 0; + + const responsePromise = waitForResponse(transport, 2); + transport.simulateMessage(request(2, 'createSession', { + session: newSession, + provider: 'copilot', + activeClient: { + clientId: 'client-1', + tools: [{ name: 't1', description: 'd', inputSchema: { type: 'object' } }], + customizations: [{ uri: 'file:///plugin-a', displayName: 'A' }], + }, + })); + await responsePromise; + + const activeClientMsgs = findNotifications(transport.sent, 'action').filter(m => { + const envelope = m.params as unknown as { action: { type: string } }; + return envelope.action.type === ActionType.SessionActiveClientChanged; + }); + assert.strictEqual(activeClientMsgs.length, 1, 'should emit exactly one SessionActiveClientChanged'); + const envelope = activeClientMsgs[0].params as unknown as { action: { session: string; activeClient: { clientId: string; tools: { name: string }[]; customizations?: { uri: string }[] } } }; + assert.deepStrictEqual({ + session: envelope.action.session, + clientId: envelope.action.activeClient.clientId, + toolName: envelope.action.activeClient.tools[0].name, + customizationUri: envelope.action.activeClient.customizations?.[0].uri, + }, { + session: newSession, + clientId: 'client-1', + toolName: 't1', + customizationUri: 'file:///plugin-a', + }); + }); + + test('rejects createSession when activeClient.clientId mismatches', async () => { + const newSession = URI.parse('copilot:///mismatch-session').toString(); + stateManager.createSession(makeSessionSummary(newSession)); + + const transport = connectClient('client-1', [newSession]); + transport.sent.length = 0; + + const responsePromise = waitForResponse(transport, 2); + transport.simulateMessage(request(2, 'createSession', { + session: newSession, + provider: 'copilot', + activeClient: { + clientId: 'other-client', + tools: [], + }, + })); + const resp = await responsePromise as { result?: unknown; error?: { code: number; message: string } }; + + assert.ok(resp.error, 'response should be an error'); + assert.strictEqual(resp.result, undefined); + const activeClientMsgs = findNotifications(transport.sent, 'action').filter(m => { + const envelope = m.params as unknown as { action: { type: string } }; + return envelope.action.type === ActionType.SessionActiveClientChanged; + }); + assert.strictEqual(activeClientMsgs.length, 0, 'no activeClient notification should have been emitted'); + }); + }); }); 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 73fa5961b0632..7773881908a8c 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts @@ -2250,6 +2250,12 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC } } + const activeClient = { + clientId: this._config.connection.clientId, + tools: this._clientToolsObs.get().map(toolDataToDefinition), + customizations: this._config.customizations?.get() ?? [], + }; + let session: URI; try { session = await this._config.connection.createSession({ @@ -2258,6 +2264,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC workingDirectory, fork, config, + activeClient, }); } catch (err) { // If authentication is required (e.g. token expired), try interactive auth and retry once @@ -2271,6 +2278,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC workingDirectory, fork, config, + activeClient, }); } else { throw new Error(localize('agentHost.authRequired', "Authentication is required to start a session. Please sign in and try again.")); @@ -2291,10 +2299,6 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC }); } - // Claim the active client role with current customizations - const customizations = this._config.customizations?.get() ?? []; - this._dispatchActiveClient(session, customizations); - // Start syncing the chat model's pending requests to the protocol this._ensurePendingMessageSubscription(sessionResource, session); 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 18bb78406a2b4..2180ae382c739 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 @@ -82,6 +82,26 @@ class MockAgentHostService extends mock() { const id = `sdk-session-${this._nextId++}`; const session = AgentSession.uri('copilot', id); this._sessions.set(id, { session, startTime: Date.now(), modifiedTime: Date.now() }); + // Simulate the server's eager active-client claim: if the caller + // provided activeClient, seed the session state so subscribers see it. + if (config?.activeClient) { + const summary: ISessionSummary = { + resource: session.toString(), + provider: 'copilot', + title: 'Test', + status: SessionStatus.Idle, + createdAt: Date.now(), + modifiedAt: Date.now(), + }; + const initial: ISessionState = { ...createSessionState(summary), lifecycle: SessionLifecycle.Ready }; + const action: ISessionAction = { + type: 'session/activeClientChanged' as const, + session: session.toString(), + activeClient: config.activeClient, + } as ISessionAction; + const withClient = sessionReducer(initial, action as Parameters[1], () => { }); + this.sessionStates.set(session.toString(), withClient); + } return session; } @@ -2491,13 +2511,14 @@ suite('AgentHostChatContribution', () => { fire({ type: 'session/turnComplete', session, turnId } as ISessionAction); await turnPromise; - const activeClientAction = agentHostService.dispatchedActions.find( - d => d.action.type === 'session/activeClientChanged' - ); - assert.ok(activeClientAction, 'should dispatch activeClientChanged'); - const ac = activeClientAction!.action as { activeClient: { customizations?: ICustomizationRef[] } }; - assert.strictEqual(ac.activeClient.customizations?.length, 1); - assert.strictEqual(ac.activeClient.customizations?.[0].uri, 'file:///plugin-a'); + // The active-client claim is now threaded through createSession + // rather than dispatched separately, so assert on createSessionCalls. + const createCall = agentHostService.createSessionCalls.at(-1); + assert.ok(createCall?.activeClient, 'createSession should carry activeClient'); + assert.strictEqual(createCall!.activeClient!.clientId, agentHostService.clientId); + assert.ok(Array.isArray(createCall!.activeClient!.tools), 'activeClient.tools should be a defined array'); + assert.strictEqual(createCall!.activeClient!.customizations?.length, 1); + assert.strictEqual(createCall!.activeClient!.customizations?.[0].uri, 'file:///plugin-a'); }); test('re-dispatches activeClientChanged when customizations observable changes', async () => { From d4b070bf4bdc26e9c229b0506de81613c7af2ffa Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Sat, 18 Apr 2026 08:52:54 -0700 Subject: [PATCH 04/13] pr comments --- .../platform/agentHost/node/agentService.ts | 2 + .../agentHost/node/copilot/copilotAgent.ts | 7 +++- .../agentHost/node/protocolServerHandler.ts | 21 ++++------- .../agentHost/test/node/agentService.test.ts | 30 +++++++++++++++ .../test/node/protocolServerHandler.test.ts | 37 ++++++------------- .../agentHostChatContribution.test.ts | 12 +++--- 6 files changed, 62 insertions(+), 47 deletions(-) diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index fc09b435bdd24..82f826baf3685 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -265,6 +265,7 @@ export class AgentService extends Disposable implements IAgentService { const state = this._stateManager.createSession(summary); state.config = sessionConfig; state.turns = sourceTurns; + state.activeClient = config.activeClient; } else { // Create empty state for new sessions const summary: ISessionSummary = { @@ -280,6 +281,7 @@ export class AgentService extends Disposable implements IAgentService { }; const state = this._stateManager.createSession(summary); state.config = sessionConfig; + state.activeClient = config?.activeClient; } this._stateManager.dispatchServerAction({ type: ActionType.SessionReady, session: session.toString() }); diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts index d87d8265722c4..35fcb7906a52d 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts @@ -449,10 +449,12 @@ export class CopilotAgent extends Disposable implements IAgent { const sessionId = config?.session ? AgentSession.id(config.session) : generateUuid(); const sessionUri = AgentSession.uri(this.id, sessionId); + let seededActiveClient = false; if (config?.activeClient) { const ac = this._getOrCreateActiveClient(sessionUri); + seededActiveClient = true; ac.updateTools(config.activeClient.clientId, config.activeClient.tools); - if (config.activeClient.customizations?.length) { + if (config.activeClient.customizations !== undefined) { await this._plugins.sync(config.activeClient.clientId, config.activeClient.customizations); } } @@ -479,6 +481,9 @@ export class CopilotAgent extends Disposable implements IAgent { agentSession = this._createAgentSession(factory, sessionId, shellManager, snapshot); await agentSession.initializeSession(); } catch (error) { + if (seededActiveClient) { + this._activeClients.delete(sessionUri); + } await this._removeCreatedWorktree(sessionId); throw error; } diff --git a/src/vs/platform/agentHost/node/protocolServerHandler.ts b/src/vs/platform/agentHost/node/protocolServerHandler.ts index fdd01b739a30b..8c3d7563ca735 100644 --- a/src/vs/platform/agentHost/node/protocolServerHandler.ts +++ b/src/vs/platform/agentHost/node/protocolServerHandler.ts @@ -12,7 +12,6 @@ import { ILogService } from '../../log/common/log.js'; import { AHPFileSystemProvider } from '../common/agentHostFileSystemProvider.js'; import { AgentSession, type IAgentService } from '../common/agentService.js'; import type { ICommandMap } from '../common/state/protocol/messages.js'; -import { ActionType } from '../common/state/protocol/actions.js'; import { IActionEnvelope, INotification, isSessionAction, isTerminalAction, type ISessionAction } from '../common/state/sessionActions.js'; import { MIN_PROTOCOL_VERSION, PROTOCOL_VERSION } from '../common/state/sessionCapabilities.js'; import { @@ -24,6 +23,7 @@ import { isJsonRpcNotification, isJsonRpcRequest, JSON_RPC_INTERNAL_ERROR, + JsonRpcErrorCodes, ProtocolError, type IAhpServerNotification, type IInitializeParams, @@ -350,6 +350,11 @@ export class ProtocolServerHandler extends Disposable { } fork = { session: URI.parse(params.fork.session), turnIndex, turnId: params.fork.turnId }; } + // If the client eagerly claimed the active client role, validate + // the clientId matches the connection before forwarding. + if (params.activeClient && params.activeClient.clientId !== _client.clientId) { + throw new ProtocolError(JsonRpcErrorCodes.InvalidParams, `createSession.activeClient.clientId must match the connection's clientId`); + } try { createdSession = await this._agentService.createSession({ provider: params.provider, @@ -358,6 +363,7 @@ export class ProtocolServerHandler extends Disposable { session: URI.parse(params.session), fork, config: params.config, + activeClient: params.activeClient, }); } catch (err) { if (err instanceof ProtocolError) { @@ -369,19 +375,6 @@ export class ProtocolServerHandler extends Disposable { if (createdSession.toString() !== URI.parse(params.session).toString()) { this._logService.warn(`[ProtocolServer] createSession: provider returned URI ${createdSession.toString()} but client requested ${params.session}`); } - // If the client eagerly claimed the active client role, dispatch - // `session/activeClientChanged` now so the claim is atomic with - // creation. - if (params.activeClient) { - if (params.activeClient.clientId !== _client.clientId) { - throw new ProtocolError(JSON_RPC_INTERNAL_ERROR, `createSession.activeClient.clientId must match the connection's clientId`); - } - this._agentService.dispatchAction({ - type: ActionType.SessionActiveClientChanged, - session: createdSession.toString(), - activeClient: params.activeClient, - }, _client.clientId, 0); - } return null; }, disposeSession: async (_client, params) => { diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index 50ab00e032685..564b340f41828 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -247,6 +247,36 @@ suite('AgentService (node dispatcher)', () => { assert.deepStrictEqual(service.stateManager.getSessionState(session.toString())?.config?.values, config); }); + + test('seeds activeClient into the initial session state when provided', async () => { + service.registerProvider(copilotAgent); + + const envelopes: IActionEnvelope[] = []; + disposables.add(service.onDidAction(env => envelopes.push(env))); + + const activeClient = { + clientId: 'client-eager', + tools: [{ name: 't1', description: 'd', inputSchema: { type: 'object' } }], + customizations: [{ uri: 'file:///plugin-a', displayName: 'A' }], + }; + const session = await service.createSession({ provider: 'copilot', activeClient }); + + assert.deepStrictEqual({ + activeClient: service.stateManager.getSessionState(session.toString())?.activeClient, + dispatchedActiveClientChanged: envelopes.some(e => e.action.type === ActionType.SessionActiveClientChanged), + }, { + activeClient, + dispatchedActiveClientChanged: false, + }); + }); + + test('omits activeClient from the initial session state when not provided', async () => { + service.registerProvider(copilotAgent); + + const session = await service.createSession({ provider: 'copilot' }); + + assert.strictEqual(service.stateManager.getSessionState(session.toString())?.activeClient, undefined); + }); }); // ---- authenticate --------------------------------------------------- diff --git a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts index fff677d85194b..29e1162661e72 100644 --- a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts +++ b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts @@ -72,6 +72,7 @@ class MockAgentService implements IAgentService { readonly browsedUris: URI[] = []; readonly browseErrors = new Map(); readonly listedSessions: IAgentSessionMetadata[] = []; + readonly createSessionConfigs: (IAgentCreateSessionConfig | undefined)[] = []; private readonly _onDidAction = new Emitter(); readonly onDidAction = this._onDidAction.event; @@ -91,6 +92,7 @@ class MockAgentService implements IAgentService { this._stateManager.dispatchClientAction(action, origin); } async createSession(config?: IAgentCreateSessionConfig): Promise { + this.createSessionConfigs.push(config); const session = config?.session ?? URI.parse('copilot:///new-session'); this._stateManager.createSession({ resource: session.toString(), @@ -582,14 +584,10 @@ suite('ProtocolServerHandler', () => { suite('createSession activeClient', () => { - test('forwards activeClient as SessionActiveClientChanged', async () => { + test('forwards activeClient to the agent service', async () => { const newSession = URI.parse('copilot:///eager-session').toString(); - // Pre-create the session so the client can subscribe at handshake - // time. MockAgentService.createSession below is idempotent against - // state-manager duplicates. - stateManager.createSession(makeSessionSummary(newSession)); - const transport = connectClient('client-1', [newSession]); + const transport = connectClient('client-1'); transport.sent.length = 0; const responsePromise = waitForResponse(transport, 2); @@ -602,21 +600,15 @@ suite('ProtocolServerHandler', () => { customizations: [{ uri: 'file:///plugin-a', displayName: 'A' }], }, })); - await responsePromise; + const resp = await responsePromise as { result?: unknown; error?: unknown }; - const activeClientMsgs = findNotifications(transport.sent, 'action').filter(m => { - const envelope = m.params as unknown as { action: { type: string } }; - return envelope.action.type === ActionType.SessionActiveClientChanged; - }); - assert.strictEqual(activeClientMsgs.length, 1, 'should emit exactly one SessionActiveClientChanged'); - const envelope = activeClientMsgs[0].params as unknown as { action: { session: string; activeClient: { clientId: string; tools: { name: string }[]; customizations?: { uri: string }[] } } }; + assert.strictEqual(resp.error, undefined, 'createSession should succeed'); + const config = agentService.createSessionConfigs.at(-1); assert.deepStrictEqual({ - session: envelope.action.session, - clientId: envelope.action.activeClient.clientId, - toolName: envelope.action.activeClient.tools[0].name, - customizationUri: envelope.action.activeClient.customizations?.[0].uri, + clientId: config?.activeClient?.clientId, + toolName: config?.activeClient?.tools[0]?.name, + customizationUri: config?.activeClient?.customizations?.[0].uri, }, { - session: newSession, clientId: 'client-1', toolName: 't1', customizationUri: 'file:///plugin-a', @@ -625,9 +617,8 @@ suite('ProtocolServerHandler', () => { test('rejects createSession when activeClient.clientId mismatches', async () => { const newSession = URI.parse('copilot:///mismatch-session').toString(); - stateManager.createSession(makeSessionSummary(newSession)); - const transport = connectClient('client-1', [newSession]); + const transport = connectClient('client-1'); transport.sent.length = 0; const responsePromise = waitForResponse(transport, 2); @@ -643,11 +634,7 @@ suite('ProtocolServerHandler', () => { assert.ok(resp.error, 'response should be an error'); assert.strictEqual(resp.result, undefined); - const activeClientMsgs = findNotifications(transport.sent, 'action').filter(m => { - const envelope = m.params as unknown as { action: { type: string } }; - return envelope.action.type === ActionType.SessionActiveClientChanged; - }); - assert.strictEqual(activeClientMsgs.length, 0, 'no activeClient notification should have been emitted'); + assert.strictEqual(agentService.createSessionConfigs.length, 0, 'agent service should not have been called'); }); }); }); 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 2180ae382c739..b0daf3a4ae57e 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 @@ -93,14 +93,12 @@ class MockAgentHostService extends mock() { createdAt: Date.now(), modifiedAt: Date.now(), }; - const initial: ISessionState = { ...createSessionState(summary), lifecycle: SessionLifecycle.Ready }; - const action: ISessionAction = { - type: 'session/activeClientChanged' as const, - session: session.toString(), + const state: ISessionState = { + ...createSessionState(summary), + lifecycle: SessionLifecycle.Ready, activeClient: config.activeClient, - } as ISessionAction; - const withClient = sessionReducer(initial, action as Parameters[1], () => { }); - this.sessionStates.set(session.toString(), withClient); + }; + this.sessionStates.set(session.toString(), state); } return session; } From a0abf264b86653d23be0642857d4c94c1084cb43 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Sat, 18 Apr 2026 09:13:20 -0700 Subject: [PATCH 05/13] fix build and duplicate logging issue --- .../agentHost/test/node/agentService.test.ts | 4 +-- .../agentHost/loggingAgentConnection.ts | 34 +++++++++++-------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index 564b340f41828..10aa0e2afb985 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -19,7 +19,7 @@ import { AgentSession } from '../../common/agentService.js'; import { ISessionDatabase, ISessionDataService } from '../../common/sessionDataService.js'; import { SessionDatabase } from '../../node/sessionDatabase.js'; import { ActionType, IActionEnvelope } from '../../common/state/sessionActions.js'; -import { ResponsePartKind, SessionLifecycle, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildSubagentSessionUri, type IMarkdownResponsePart, type IToolCallCompletedState, type IToolCallResponsePart } from '../../common/state/sessionState.js'; +import { ISessionActiveClient, ResponsePartKind, SessionLifecycle, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildSubagentSessionUri, type IMarkdownResponsePart, type IToolCallCompletedState, type IToolCallResponsePart } from '../../common/state/sessionState.js'; import { IProductService } from '../../../product/common/productService.js'; import { AgentService } from '../../node/agentService.js'; import { MockAgent } from './mockAgent.js'; @@ -254,7 +254,7 @@ suite('AgentService (node dispatcher)', () => { const envelopes: IActionEnvelope[] = []; disposables.add(service.onDidAction(env => envelopes.push(env))); - const activeClient = { + const activeClient: ISessionActiveClient = { clientId: 'client-eager', tools: [{ name: 't1', description: 'd', inputSchema: { type: 'object' } }], customizations: [{ uri: 'file:///plugin-a', displayName: 'A' }], diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/loggingAgentConnection.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/loggingAgentConnection.ts index 8879964026ee5..f5533e8d92399 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/loggingAgentConnection.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/loggingAgentConnection.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Emitter, Event } from '../../../../../../base/common/event.js'; -import { Disposable, IReference, toDisposable } from '../../../../../../base/common/lifecycle.js'; +import { Disposable, DisposableStore, IDisposable, IReference, toDisposable } from '../../../../../../base/common/lifecycle.js'; import { URI, UriComponents } from '../../../../../../base/common/uri.js'; import { Registry } from '../../../../../../platform/registry/common/platform.js'; import { IAgentConnection, IAgentCreateSessionConfig, IAgentResolveSessionConfigParams, IAgentSessionConfigCompletionsParams, IAgentSessionMetadata, IAuthenticateParams, IAuthenticateResult, AgentHostIpcLoggingSettingId } from '../../../../../../platform/agentHost/common/agentService.js'; @@ -98,6 +98,12 @@ export class LoggingAgentConnection extends Disposable implements IAgentConnecti /** Ref-count per channel ID so the output channel survives reconnections. */ private static readonly _channelRefCounts = new Map(); private static readonly _currentRootStateLogKeys = new Set(); + /** + * Shared event-log subscription per channel ID. Multiple wrappers may + * exist for the same underlying connection (e.g. one for chat, one for + * terminal); we only want each event to appear once in the channel. + */ + private static readonly _sharedEventLog = new Map(); private _outputChannel: IOutputChannel | undefined; private readonly _enabled: boolean; @@ -130,6 +136,10 @@ export class LoggingAgentConnection extends Disposable implements IAgentConnecti log: false, languageId: 'log', }); + const eventLogStore = new DisposableStore(); + eventLogStore.add(_inner.onDidAction(e => this._log('**', 'onDidAction', e))); + eventLogStore.add(_inner.onDidNotification(e => this._log('**', 'onDidNotification', e))); + LoggingAgentConnection._sharedEventLog.set(this.channelId, eventLogStore); } LoggingAgentConnection._channelRefCounts.set(this.channelId, refs + 1); logCurrentRootState = !LoggingAgentConnection._currentRootStateLogKeys.has(currentRootStateLogKey); @@ -141,6 +151,8 @@ export class LoggingAgentConnection extends Disposable implements IAgentConnecti if (current <= 0) { LoggingAgentConnection._channelRefCounts.delete(this.channelId); LoggingAgentConnection._currentRootStateLogKeys.delete(currentRootStateLogKey); + LoggingAgentConnection._sharedEventLog.get(this.channelId)?.dispose(); + LoggingAgentConnection._sharedEventLog.delete(this.channelId); registry.removeChannel(this.channelId); } else { LoggingAgentConnection._channelRefCounts.set(this.channelId, current); @@ -148,20 +160,12 @@ export class LoggingAgentConnection extends Disposable implements IAgentConnecti })); } - // Wrap events with logging - const onDidActionEmitter = this._register(new Emitter()); - this._register(_inner.onDidAction(e => { - this._log('**', 'onDidAction', e); - onDidActionEmitter.fire(e); - })); - this.onDidAction = onDidActionEmitter.event; - - const onDidNotificationEmitter = this._register(new Emitter()); - this._register(_inner.onDidNotification(e => { - this._log('**', 'onDidNotification', e); - onDidNotificationEmitter.fire(e); - })); - this.onDidNotification = onDidNotificationEmitter.event; + // Expose the inner events directly. Logging happens once per channel + // via the shared subscription registered above; wrappers must not + // add their own logging listener or events would be logged N times + // (once per wrapper for the same channel). + this.onDidAction = _inner.onDidAction; + this.onDidNotification = _inner.onDidNotification; this._rootState = this._register(new LoggingAgentSubscription('rootState', _inner.rootState, logCurrentRootState, (arrow, method, data) => this._log(arrow, method, data))); } From fd55f112f40ea83dd76e2618e5c5afd47d52dbb6 Mon Sep 17 00:00:00 2001 From: Justin Chen <54879025+justschen@users.noreply.github.com> Date: Mon, 20 Apr 2026 03:31:30 -0700 Subject: [PATCH 06/13] clean up confirmation carousel - no more collapse and better focus logic (#311270) * clean up confirmationc carousel * Update src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatToolConfirmationCarousel.css Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../media/chatToolConfirmationCarousel.css | 83 ++++---- .../chatToolConfirmationCarouselPart.ts | 177 +++--------------- 2 files changed, 54 insertions(+), 206 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatToolConfirmationCarousel.css b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatToolConfirmationCarousel.css index 06b92371129fa..85d1b456c8197 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatToolConfirmationCarousel.css +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatToolConfirmationCarousel.css @@ -29,7 +29,16 @@ background-color: var(--vscode-panel-background); overflow: hidden; - &:focus:not(:focus-visible) { + &:focus { + outline: none !important; + } + + &:focus-visible, + &:focus-within { + border-color: var(--vscode-focusBorder); + } + + .chat-tool-carousel-content .chat-confirmation-widget-container:focus { outline: none; } @@ -93,18 +102,7 @@ margin-left: auto; } - button:focus:not(:focus-visible), - .monaco-button:focus:not(:focus-visible) { - outline: none !important; - box-shadow: none !important; - } - - .chat-tool-carousel-nav-arrows { - display: flex; - align-items: center; - } - - .monaco-button.chat-tool-carousel-nav-arrow { + .monaco-button.chat-tool-carousel-dismiss-button { min-width: 22px; width: 22px; height: 22px; @@ -112,18 +110,29 @@ border: none !important; box-shadow: none !important; background: transparent !important; - color: var(--vscode-foreground) !important; + color: var(--vscode-icon-foreground) !important; + cursor: pointer; + display: flex; + align-items: center; + justify-content: center; } - .monaco-button.chat-tool-carousel-nav-arrow:hover:not(.disabled) { + .monaco-button.chat-tool-carousel-dismiss-button:hover { background: var(--vscode-toolbar-hoverBackground) !important; } - .monaco-button.chat-tool-carousel-nav-arrow.disabled { - opacity: 0.4; + button:focus, + .monaco-button:focus { + outline: none !important; + box-shadow: none !important; } - .monaco-button.chat-tool-carousel-header-button { + .chat-tool-carousel-nav-arrows { + display: flex; + align-items: center; + } + + .monaco-button.chat-tool-carousel-nav-arrow { min-width: 22px; width: 22px; height: 22px; @@ -131,39 +140,19 @@ border: none !important; box-shadow: none !important; background: transparent !important; - color: var(--vscode-icon-foreground) !important; - cursor: pointer; - display: flex; - align-items: center; - justify-content: center; + color: var(--vscode-foreground) !important; } - .monaco-button.chat-tool-carousel-header-button:hover:not(.disabled) { + .monaco-button.chat-tool-carousel-nav-arrow:hover:not(.disabled) { background: var(--vscode-toolbar-hoverBackground) !important; } - &.chat-tool-carousel-content-expanded { - max-height: min(650px, 70vh); + .monaco-button.chat-tool-carousel-nav-arrow.disabled { + opacity: 0.4; } - &.chat-tool-carousel-collapsed { - .chat-tool-carousel-content { - display: none; - } - - .chat-tool-carousel-overlay { - padding: 6px 10px 6px 12px; - border-bottom: none; - } - - .chat-tool-carousel-title-group { - flex: 1; - } - - .chat-tool-carousel-nav-arrows, - .chat-tool-carousel-step-indicator { - display: none; - } + .monaco-list:focus { + outline: none !important; } .chat-tool-carousel-content { @@ -230,12 +219,6 @@ background-color: var(--vscode-sideBar-background); } - .chat-tool-carousel-content-expanded & { - .chat-confirmation-widget-message { - max-height: min(500px, 55vh); - } - } - .chat-buttons-container { flex-shrink: 0; } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.ts index 99e754139a85b..72251af8125f2 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.ts @@ -12,7 +12,6 @@ import { IMarkdownString } from '../../../../../../../base/common/htmlContent.js import { KeyCode } from '../../../../../../../base/common/keyCodes.js'; import { Disposable, DisposableStore, MutableDisposable, toDisposable } from '../../../../../../../base/common/lifecycle.js'; import { autorun } from '../../../../../../../base/common/observable.js'; -import { generateUuid } from '../../../../../../../base/common/uuid.js'; import { localize } from '../../../../../../../nls.js'; import { defaultButtonStyles } from '../../../../../../../platform/theme/browser/defaultStyles.js'; import { IChatToolInvocation, ToolConfirmKind } from '../../../../common/chatService/chatService.js'; @@ -20,10 +19,7 @@ import { ChatToolInvocationPart } from './chatToolInvocationPart.js'; import '../media/chatToolConfirmationCarousel.css'; const COLLAPSED_CAROUSEL_MAX_HEIGHT = 300; -const COLLAPSED_MESSAGE_MAX_HEIGHT = 200; -const COLLAPSED_CODE_BLOCK_MAX_HEIGHT = 150; const MIN_CAROUSEL_MAX_HEIGHT = 80; -const EXPANDABLE_CONTENT_SELECTOR = '.interactive-result-editor, .chat-markdown-part.rendered-markdown'; export type ToolInvocationPartFactory = (tool: IChatToolInvocation) => ChatToolInvocationPart; @@ -57,14 +53,8 @@ export class ChatToolConfirmationCarouselPart extends Disposable { private readonly prevButton: Button; private readonly nextButton: Button; private readonly allowAllButton: Button; - private readonly expandContentButton: Button; - private readonly collapseButton: Button; + private readonly dismissButton: Button; private readonly activeContentDisposables: DisposableStore; - private readonly contentResizeObserver: dom.DisposableResizeObserver; - private readonly updateContentExpansionStateScheduler: dom.AnimationFrameScheduler; - private _isCollapsed = false; - private _isContentExpanded = false; - private canExpandContent = false; private maxHeight: number | undefined; constructor( @@ -97,32 +87,23 @@ export class ChatToolConfirmationCarouselPart extends Disposable { this.collapsedTitle = elements.collapsedTitle; this.agentLabel = elements.agentLabel; this.contentContainer = elements.content; - this.contentContainer.id = generateUuid(); this.stepIndicator = elements.stepIndicator; this.activeContentDisposables = this._register(new DisposableStore()); - this.updateContentExpansionStateScheduler = this._register(new dom.AnimationFrameScheduler(this.domNode, () => this.updateContentExpansionState())); - this.contentResizeObserver = this._register(new dom.DisposableResizeObserver(() => this.updateContentExpansionStateScheduler.schedule())); - this._register(this.contentResizeObserver.observe(this.contentContainer)); this.allowAllButton = this._register(new Button(elements.overlayActions, { ...defaultButtonStyles, small: true })); this.allowAllButton.element.classList.add('chat-tool-carousel-allow-all-button'); this.allowAllButton.label = localize('allowAll', "Allow All"); this._register(this.allowAllButton.onDidClick(() => this.allowAll())); - this.expandContentButton = this._register(new Button(elements.overlayActions, { ...defaultButtonStyles, secondary: true, supportIcons: true })); - this.expandContentButton.element.classList.add('chat-tool-carousel-header-button', 'chat-tool-carousel-expand-content-button'); - this.expandContentButton.element.setAttribute('aria-controls', this.contentContainer.id); - this.updateExpandContentButton(); - dom.hide(this.expandContentButton.element); - this._register(this.expandContentButton.onDidClick(() => this.toggleContentExpanded())); - - this.collapseButton = this._register(new Button(elements.overlayActions, { ...defaultButtonStyles, secondary: true, supportIcons: true })); - this.collapseButton.element.classList.add('chat-tool-carousel-header-button'); - this.collapseButton.label = `$(${Codicon.chevronDown.id})`; - this.collapseButton.element.setAttribute('aria-label', localize('collapse', "Collapse")); - this.collapseButton.element.setAttribute('aria-controls', this.contentContainer.id); - this.collapseButton.element.setAttribute('aria-expanded', 'true'); - this._register(this.collapseButton.onDidClick(() => this.toggleCollapse())); + this.dismissButton = this._register(new Button(elements.overlayActions, { ...defaultButtonStyles, secondary: true, supportIcons: true })); + this.dismissButton.element.classList.add('chat-tool-carousel-dismiss-button'); + this.dismissButton.label = `$(${Codicon.close.id})`; + const dismissButtonLabel = this.items.length === 1 + ? localize('skip', "Skip") + : localize('skipAll', "Skip All"); + this.dismissButton.element.setAttribute('aria-label', dismissButtonLabel); + this.dismissButton.element.title = dismissButtonLabel; + this._register(this.dismissButton.onDidClick(() => this.skipAll())); this.prevButton = this._register(new Button(elements.navArrows, { ...defaultButtonStyles, @@ -162,7 +143,7 @@ export class ChatToolConfirmationCarouselPart extends Disposable { setMaxHeight(maxHeight: number | undefined): void { this.maxHeight = maxHeight; - this.updateContentExpansionState(); + this.updateMaxHeightStyle(); } hasToolInvocation(toolCallId: string): boolean { @@ -357,43 +338,11 @@ export class ChatToolConfirmationCarouselPart extends Disposable { this.domNode.focus(); } - private toggleCollapse(): void { - this._isCollapsed = !this._isCollapsed; - this.domNode.classList.toggle('chat-tool-carousel-collapsed', this._isCollapsed); - this.collapseButton.label = this._isCollapsed - ? `$(${Codicon.chevronUp.id})` - : `$(${Codicon.chevronDown.id})`; - this.collapseButton.element.setAttribute('aria-label', - this._isCollapsed ? localize('expand', "Expand") : localize('collapse', "Collapse") - ); - this.collapseButton.element.setAttribute('aria-expanded', String(!this._isCollapsed)); - if (this._isCollapsed) { - this._isContentExpanded = false; - } - this.updateUI(); - this.updateContentExpansionState(); - } - - private toggleContentExpanded(): void { - if (!this.canExpandContent) { - return; - } - - this._isContentExpanded = !this._isContentExpanded; - this.updateContentExpansionState(); - } - private updateUI(): void { const item = this.items[this.activeIndex]; - if (this._isCollapsed) { - this.collapsedTitle.textContent = this.items.length === 1 - ? localize('confirmTool', "Confirm tool?") - : localize('confirmTools', "Confirm {0} tools?", this.items.length); - } else { - this.collapsedTitle.textContent = this.getToolTitle(item) ?? ''; - } - dom.setVisibility(this._isCollapsed || !!this.collapsedTitle.textContent, this.collapsedTitle); + this.collapsedTitle.textContent = this.getToolTitle(item) ?? ''; + dom.setVisibility(!!this.collapsedTitle.textContent, this.collapsedTitle); if (item?.agentName) { this.agentLabel.textContent = `\u2014 ${item.agentName}`; @@ -416,24 +365,19 @@ export class ChatToolConfirmationCarouselPart extends Disposable { dom.setVisibility(multi, this.stepIndicator); dom.setVisibility(multi, this.prevButton.element); dom.setVisibility(multi, this.nextButton.element); - dom.setVisibility(this._isCollapsed || multi, this.allowAllButton.element); - dom.setVisibility(!this._isCollapsed && this.canExpandContent, this.expandContentButton.element); + dom.setVisibility(multi, this.allowAllButton.element); this.allowAllButton.label = multi ? localize('allowAll', "Allow All") : localize('allow', "Allow"); - this.updateExpandContentButton(); } private renderActiveContent(): void { dom.clearNode(this.contentContainer); this.activeContentDisposables.clear(); - this._isContentExpanded = false; - this.canExpandContent = false; const item = this.items[this.activeIndex]; if (!item) { - this.updateContentExpansionState(); return; } @@ -445,21 +389,6 @@ export class ChatToolConfirmationCarouselPart extends Disposable { } this.contentContainer.appendChild(item.toolPart.domNode); - this.activeContentDisposables.add(this.contentResizeObserver.observe(item.toolPart.domNode)); - this.observeExpandableContentElements(item.toolPart.domNode); - this.updateContentExpansionStateScheduler.schedule(); - } - - private updateContentExpansionState(): void { - this.canExpandContent = !this._isCollapsed && this.items.length > 0 && this.isActiveContentLargerThanCollapsedLimit(); - if (!this.canExpandContent) { - this._isContentExpanded = false; - } - - this.domNode.classList.toggle('chat-tool-carousel-content-expanded', this.canExpandContent && this._isContentExpanded); - this.updateMaxHeightStyle(); - dom.setVisibility(!this._isCollapsed && this.canExpandContent, this.expandContentButton.element); - this.updateExpandContentButton(); } private updateMaxHeightStyle(): void { @@ -468,80 +397,10 @@ export class ChatToolConfirmationCarouselPart extends Disposable { return; } - const expanded = this.canExpandContent && this._isContentExpanded; - const maxHeight = expanded ? Math.max(MIN_CAROUSEL_MAX_HEIGHT, this.maxHeight) : this.getCollapsedMaxHeight(); + const maxHeight = this.getCollapsedMaxHeight(); this.domNode.style.maxHeight = `${Math.floor(maxHeight)}px`; } - private updateExpandContentButton(): void { - const expanded = this.canExpandContent && this._isContentExpanded; - const label = expanded - ? localize('restoreConfirmationSize', "Restore Confirmation Size") - : localize('expandConfirmationUp', "Expand Confirmation Up"); - this.expandContentButton.label = expanded - ? `$(${Codicon.screenNormal.id})` - : `$(${Codicon.screenFull.id})`; - this.expandContentButton.element.setAttribute('aria-label', label); - this.expandContentButton.element.setAttribute('aria-expanded', String(expanded)); - this.expandContentButton.setTitle(label); - } - - private isActiveContentLargerThanCollapsedLimit(): boolean { - const activeContent = this.contentContainer.firstElementChild; - if (!dom.isHTMLElement(activeContent)) { - return false; - } - - return this.hasInnerContentLargerThanCollapsedLimit(activeContent); - } - - private hasInnerContentLargerThanCollapsedLimit(element: HTMLElement): boolean { - if (this.isExpandableContentElement(element) && this.getElementHeight(element) > this.getExpandableContentHeightLimit(element) + 1) { - return true; - } - - for (const child of element.children) { - if (!dom.isHTMLElement(child)) { - continue; - } - - if (this.hasInnerContentLargerThanCollapsedLimit(child)) { - return true; - } - } - - return false; - } - - private isExpandableContentElement(element: HTMLElement): boolean { - return element.matches(EXPANDABLE_CONTENT_SELECTOR); - } - - private observeExpandableContentElements(element: HTMLElement): void { - if (this.isExpandableContentElement(element)) { - this.activeContentDisposables.add(this.contentResizeObserver.observe(element)); - } - - for (const child of element.children) { - if (dom.isHTMLElement(child)) { - this.observeExpandableContentElements(child); - } - } - } - - private getElementHeight(element: HTMLElement): number { - return Math.max(element.offsetHeight, element.scrollHeight); - } - - private getExpandableContentHeightLimit(element: HTMLElement): number { - const window = dom.getWindow(this.domNode); - if (element.classList.contains('interactive-result-editor')) { - return Math.min(COLLAPSED_CODE_BLOCK_MAX_HEIGHT, window.innerHeight * 0.25); - } - - return Math.min(COLLAPSED_MESSAGE_MAX_HEIGHT, window.innerHeight * 0.3); - } - private getCollapsedMaxHeight(): number { const configuredMaxHeight = this.maxHeight === undefined ? Number.POSITIVE_INFINITY : Math.max(MIN_CAROUSEL_MAX_HEIGHT, this.maxHeight); return Math.min(configuredMaxHeight, COLLAPSED_CAROUSEL_MAX_HEIGHT, dom.getWindow(this.domNode).innerHeight * 0.45); @@ -553,6 +412,12 @@ export class ChatToolConfirmationCarouselPart extends Disposable { } } + private skipAll(): void { + for (const item of [...this.items]) { + IChatToolInvocation.confirmWith(item.tool, { type: ToolConfirmKind.Skipped }); + } + } + private getToolTitle(item: ICarouselToolItem | undefined): string | undefined { if (!item) { return undefined; From 12dce43c1833ce1065b2c7eb29f25a62a748ecc7 Mon Sep 17 00:00:00 2001 From: Ladislau Szomoru <3372902+lszomoru@users.noreply.github.com> Date: Mon, 20 Apr 2026 10:40:26 +0000 Subject: [PATCH 07/13] Agents - disable indicators in the diff editor (#311338) --- .../contrib/configuration/browser/configuration.contribution.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vs/sessions/contrib/configuration/browser/configuration.contribution.ts b/src/vs/sessions/contrib/configuration/browser/configuration.contribution.ts index fc87e87717965..3f1dc4f583187 100644 --- a/src/vs/sessions/contrib/configuration/browser/configuration.contribution.ts +++ b/src/vs/sessions/contrib/configuration/browser/configuration.contribution.ts @@ -41,6 +41,7 @@ Registry.as(Extensions.Configuration).registerDefaultCon 'diffEditor.hideUnchangedRegions.enabled': true, 'diffEditor.renderGutterMenu': false, + 'diffEditor.renderIndicators': false, 'diffEditor.renderMarginRevertIcon': false, 'diffEditor.renderSideBySide': true, 'diffEditor.useInlineViewWhenSpaceIsLimited': true, From 1d3358a867cf69aff5b6c9f1a8ea922c7acf3f08 Mon Sep 17 00:00:00 2001 From: Lee Murray Date: Mon, 20 Apr 2026 12:58:19 +0100 Subject: [PATCH 08/13] Update padding for multiDiffEntry in editor (#311345) Update padding for multiDiffEntry in editor to enhance layout Co-authored-by: mrleemurray --- src/vs/sessions/browser/media/style.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/sessions/browser/media/style.css b/src/vs/sessions/browser/media/style.css index b2b18772bd6c1..1d4c768c0b18e 100644 --- a/src/vs/sessions/browser/media/style.css +++ b/src/vs/sessions/browser/media/style.css @@ -130,7 +130,7 @@ } .agent-sessions-workbench .part.editor .multiDiffEntry { - padding: 0 8px; + padding: 6px 8px 0; box-sizing: border-box; } From eb9cbc565be0083302a1e30b5e37c0736d81ed67 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Mon, 20 Apr 2026 05:00:59 -0700 Subject: [PATCH 09/13] Fix disposed SCM source control retention (#311284) * Fix disposed SCM source control retention Remove disposed source controls from extension-host SCM bookkeeping so they no longer retain repository state after unregistering from the main thread. (Written by Copilot) * Add logging for consistency --------- Co-authored-by: Ladislau Szomoru <3372902+lszomoru@users.noreply.github.com> --- src/vs/workbench/api/common/extHostSCM.ts | 18 ++++++ .../api/test/common/extHostSCM.test.ts | 58 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 src/vs/workbench/api/test/common/extHostSCM.test.ts diff --git a/src/vs/workbench/api/common/extHostSCM.ts b/src/vs/workbench/api/common/extHostSCM.ts index bf479e9ebb196..b4926d93d5f92 100644 --- a/src/vs/workbench/api/common/extHostSCM.ts +++ b/src/vs/workbench/api/common/extHostSCM.ts @@ -1026,6 +1026,24 @@ export class ExtHostSCM implements ExtHostSCMShape { sourceControls.push(sourceControl); this._sourceControlsByExtension.set(extension.identifier, sourceControls); + Event.once(sourceControl.onDidDispose)(() => { + this.logService.trace('ExtHostSCM#disposeSourceControl', extension.identifier.value, id, label, rootUri); + + this._sourceControls.delete(sourceControl.handle); + + const sourceControls = this._sourceControlsByExtension.get(extension.identifier); + if (sourceControls) { + const index = sourceControls.indexOf(sourceControl); + if (index !== -1) { + sourceControls.splice(index, 1); + } + + if (sourceControls.length === 0) { + this._sourceControlsByExtension.delete(extension.identifier); + } + } + }); + return sourceControl; } diff --git a/src/vs/workbench/api/test/common/extHostSCM.test.ts b/src/vs/workbench/api/test/common/extHostSCM.test.ts new file mode 100644 index 0000000000000..b0c6706ebe76e --- /dev/null +++ b/src/vs/workbench/api/test/common/extHostSCM.test.ts @@ -0,0 +1,58 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { URI } from '../../../../base/common/uri.js'; +import { mock } from '../../../../base/test/common/mock.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { NullLogService } from '../../../../platform/log/common/log.js'; +import { ExtensionIdentifier } from '../../../../platform/extensions/common/extensions.js'; +import { nullExtensionDescription } from '../../../services/extensions/common/extensions.js'; +import { MainContext, MainThreadSCMShape, MainThreadTelemetryShape } from '../../common/extHost.protocol.js'; +import { ArgumentProcessor, ExtHostCommands } from '../../common/extHostCommands.js'; +import { ExtHostDocuments } from '../../common/extHostDocuments.js'; +import { ExtHostSCM } from '../../common/extHostSCM.js'; +import { TestRPCProtocol } from './testRPCProtocol.js'; + +suite('ExtHostSCM', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + test('disposed source controls are removed from extension bookkeeping', () => { + const rpcProtocol = new TestRPCProtocol(); + rpcProtocol.set(MainContext.MainThreadSCM, new class extends mock() { + override async $registerSourceControl(): Promise { } + override async $unregisterSourceControl(): Promise { } + }); + rpcProtocol.set(MainContext.MainThreadTelemetry, new class extends mock() { + override $publicLog2(): void { } + }); + + const commands = new class extends mock() { + override registerArgumentProcessor(_processor: ArgumentProcessor): void { } + }; + const extension = { + ...nullExtensionDescription, + identifier: new ExtensionIdentifier('vscode.git'), + name: 'git', + displayName: 'Git', + extensionLocation: URI.file('/extension'), + isBuiltin: true + }; + + const extHostSCM = new ExtHostSCM( + rpcProtocol, + commands, + {} as ExtHostDocuments, + new NullLogService() + ); + + const sourceControl = extHostSCM.createSourceControl(extension, 'git', 'Git', URI.file('/repo'), undefined, undefined, undefined); + assert.ok(extHostSCM.getLastInputBox(extension)); + + sourceControl.dispose(); + + assert.strictEqual(extHostSCM.getLastInputBox(extension), undefined); + }); +}); From 334c5b7fd276807e80698bd53ae274f83c959ffd Mon Sep 17 00:00:00 2001 From: Robo Date: Mon, 20 Apr 2026 22:41:30 +0900 Subject: [PATCH 10/13] fix(agents): vscode app launch due to missing app identifiers (#311362) --- src/bootstrap-meta.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bootstrap-meta.ts b/src/bootstrap-meta.ts index c2dfea36372b2..b1d8213d89342 100644 --- a/src/bootstrap-meta.ts +++ b/src/bootstrap-meta.ts @@ -31,7 +31,11 @@ if ((process as INodeProcess).isEmbeddedApp) { try { const productSubObj = require('../product.sub.json'); - productObj = Object.assign(productObj, productSubObj); + if (productObj.embedded && productSubObj.embedded) { + Object.assign(productObj.embedded, productSubObj.embedded); + delete productSubObj.embedded; + } + Object.assign(productObj, productSubObj); } catch (error) { /* ignore */ } try { const pkgSubObj = require('../package.sub.json'); From 197f33eb3661397da0bfcc627bd676579bd5e46a Mon Sep 17 00:00:00 2001 From: Ulugbek Abdullaev Date: Mon, 20 Apr 2026 20:01:49 +0500 Subject: [PATCH 11/13] NES: consolidate speculative request cancellation with type-through trajectory check (#310969) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit nes: consolidate speculative request cancellation Introduces SpeculativeRequestManager that owns the lifecycle of NES speculative requests (the in-flight 'pending' bet on a post-accept document state, plus the 'scheduled' speculative deferred until its originating stream completes). All cancellation now goes through one path with a typed SpeculativeCancelReason logged on the request's log context, and pairs cancel() with dispose() on the token source to release listeners. Adds these cancellation triggers that previously leaked the speculative: - CacheCleared: clearCache() now cancels in-flight speculatives — they would otherwise land into a cache that was meant to be empty. - DocumentClosed: when a doc is removed from openDocuments, any speculative targeting it is cancelled — its cached result could never be hit again. - Disposed: provider dispose now cancels all speculatives. Also removes the special-case in handleIgnored for the superseded branch — the trajectory check on the document change that caused the supersede is the authoritative signal. --- .../inlineEdits/node/nextEditProvider.ts | 148 +++++++------ .../node/speculativeRequestManager.ts | 199 ++++++++++++++++++ .../node/nextEditProviderSpeculative.spec.ts | 124 +++++++++-- 3 files changed, 382 insertions(+), 89 deletions(-) create mode 100644 extensions/copilot/src/extension/inlineEdits/node/speculativeRequestManager.ts diff --git a/extensions/copilot/src/extension/inlineEdits/node/nextEditProvider.ts b/extensions/copilot/src/extension/inlineEdits/node/nextEditProvider.ts index 806fd9938d39e..5a31c9be113f1 100644 --- a/extensions/copilot/src/extension/inlineEdits/node/nextEditProvider.ts +++ b/extensions/copilot/src/extension/inlineEdits/node/nextEditProvider.ts @@ -46,6 +46,7 @@ import { INesConfigs } from './nesConfigs'; import { CachedOrRebasedEdit, NextEditCache } from './nextEditCache'; import { LlmNESTelemetryBuilder, ReusedRequestKind } from './nextEditProviderTelemetry'; import { INextEditResult, NextEditResult } from './nextEditResult'; +import { SpeculativeCancelReason, SpeculativeRequestManager } from './speculativeRequestManager'; /** * Computes a reduced window range that encompasses both the original window (shrunk by one line @@ -167,28 +168,7 @@ export class NextEditProvider extends Disposable implements INextEditProvider | null = null; - /** - * Tracks a speculative request for the post-edit document state. - * When a suggestion is shown, we speculatively fetch the next edit as if the user had already accepted. - * This allows reusing the in-flight request when the user actually accepts the suggestion. - */ - private _speculativePendingRequest: { - request: StatelessNextEditRequest; - docId: DocumentId; - postEditContent: string; - } | null = null; - - /** - * A speculative request that is deferred until the originating stream completes. - * When a suggestion is shown while its stream is still running, we schedule the - * speculative request here instead of firing immediately. If more edits arrive - * from the stream, the schedule is cleared (the shown edit wasn't the last one). - * When the stream ends, if the schedule is still present, the speculative fires. - */ - private _scheduledSpeculativeRequest: { - suggestion: NextEditResult; - headerRequestId: string; - } | null = null; + private readonly _specManager: SpeculativeRequestManager; private _lastShownTime = 0; /** The requestId of the last shown suggestion. We store only the requestId (not the object) to avoid preventing garbage collection. */ @@ -230,35 +210,48 @@ export class NextEditProvider extends Disposable implements INextEditProvider { store.add(runOnChange(doc.value, (value) => { this._cancelPendingRequestDueToDocChange(doc.id, value); + // FIXME: don't invoke before fixing false positive cancellations + // this._specManager.onActiveDocumentChanged(doc.id, value.value); })); + // When the per-doc store is disposed, the document was removed from + // openDocuments. Cancel any speculative targeting it — its cached result + // would never be hit again. + store.add(toDisposable(() => this._specManager.onDocumentClosed(doc.id))); }).recomputeInitiallyAndOnChange(this._store); } - private _cancelSpeculativeRequest(): void { - this._scheduledSpeculativeRequest = null; - if (this._speculativePendingRequest) { - this._speculativePendingRequest.request.cancellationTokenSource.cancel(); - this._speculativePendingRequest = null; - } - } - + /** + * Cancels the in-flight stateless next-edit request when the document it + * was issued for has diverged from the request's expected post-edit state. + * + * Invoked from the per-document `runOnChange` autorun in the constructor + * whenever an open document's value changes. The pending request was built + * against a specific snapshot (`documentAfterEdits`); if the user has since + * typed something that makes the current value differ from that snapshot, + * the result would no longer be applicable and is cancelled eagerly. + * + * Skipped when: + * - the `InlineEditsAsyncCompletions` experiment is enabled (that path + * tolerates divergence and rebases later), or + * - there is no pending request, or + * - the changed document is not the one the pending request targets. + * + * Note: this only handles the regular pending stateless request. Speculative + * requests have their own divergence handling via + * `SpeculativeRequestManager.onActiveDocumentChanged` (trajectory check). + */ private _cancelPendingRequestDueToDocChange(docId: DocumentId, docValue: StringText) { - // Note: we intentionally do NOT cancel the speculative request here. - // The speculative request's postEditContent represents a *future* document state - // (after the user would accept the suggestion), so it will almost never match the - // current document value while the user is still typing. Cancelling here would - // wastefully kill and recreate the speculative request on every keystroke. - // Instead, speculative requests are cancelled by the appropriate lifecycle handlers: - // handleRejection, handleIgnored, _triggerSpeculativeRequest, and _executeNewNextEditRequest. - const isAsyncCompletions = this._configService.getExperimentBasedConfig(ConfigKey.TeamInternal.InlineEditsAsyncCompletions, this._expService); + if (isAsyncCompletions || this._pendingStatelessNextEditRequest === null) { return; } + const activeDoc = this._pendingStatelessNextEditRequest.getActiveDocument(); if (activeDoc.id === docId && activeDoc.documentAfterEdits.value !== docValue.value) { this._pendingStatelessNextEditRequest.cancellationTokenSource.cancel(); @@ -546,11 +539,12 @@ export class NextEditProvider extends Disposable implements INextEditProvider 1000 && suggestion.result.edit) { @@ -1470,9 +1473,15 @@ export class NextEditProvider extends Disposable implements INextEditProvider; + readonly docId: DocumentId; + readonly postEditContent: string; + /** preEditDocument[0..editStart] — the doc text before the edit window. */ + readonly trajectoryPrefix: string; + /** preEditDocument[editEnd..] — the doc text after the edit window. */ + readonly trajectorySuffix: string; + /** The replacement text the user would type to reach `postEditContent`. */ + readonly trajectoryNewText: string; +} + +export interface ScheduledSpeculativeRequest { + readonly suggestion: NextEditResult; + readonly headerRequestId: string; +} + +/** + * Owns the lifecycle of NES speculative requests: + * + * - the in-flight `pending` speculative (the bet on a specific post-accept document state) + * - the `scheduled` speculative deferred until its originating stream completes + * + * Centralizes cancellation with typed reasons so every triggered cancellation + * (reject, supersede, doc-close, trajectory divergence, dispose, ...) goes through + * one path and is logged on the request's log context. + */ +export class SpeculativeRequestManager extends Disposable { + + private _pending: SpeculativePendingRequest | null = null; + private _scheduled: ScheduledSpeculativeRequest | null = null; + + constructor(private readonly _logger: ILogger) { + super(); + } + + get pending(): SpeculativePendingRequest | null { + return this._pending; + } + + /** Replaces the current pending speculative; cancels the prior one as `Replaced`. */ + setPending(req: SpeculativePendingRequest): void { + if (this._pending && this._pending.request !== req.request) { + this._cancelPending(SpeculativeCancelReason.Replaced); + } + this._pending = req; + } + + /** Detaches the pending speculative without cancelling — caller is consuming it. */ + consumePending(): void { + this._pending = null; + } + + schedule(s: ScheduledSpeculativeRequest): void { + this._scheduled = s; + } + + clearScheduled(): void { + this._scheduled = null; + } + + /** + * Removes and returns the scheduled entry iff its `headerRequestId` matches. + * Used by the streaming path so that each stream only ever consumes its own + * schedule, never another stream's. + */ + consumeScheduled(headerRequestId: string): ScheduledSpeculativeRequest | null { + if (this._scheduled?.headerRequestId !== headerRequestId) { + return null; + } + const s = this._scheduled; + this._scheduled = null; + return s; + } + + cancelAll(reason: SpeculativeCancelReason): void { + this._scheduled = null; + this._cancelPending(reason); + } + + /** Cancels the pending speculative iff `(docId, postEditContent)` doesn't match. */ + cancelIfMismatch(docId: DocumentId, postEditContent: string, reason: SpeculativeCancelReason): void { + if (this._pending && (this._pending.docId !== docId || this._pending.postEditContent !== postEditContent)) { + this._cancelPending(reason); + } + } + + /** Cancels the pending and clears any scheduled targeting this document. */ + onDocumentClosed(docId: DocumentId): void { + if (this._scheduled?.suggestion.result?.targetDocumentId === docId) { + this._scheduled = null; + } + if (this._pending?.docId === docId) { + this._cancelPending(SpeculativeCancelReason.DocumentClosed); + } + } + + /** + * Trajectory check. The pending speculative is alive iff the current document + * value is a *type-through prefix* toward the speculative's `postEditContent`: + * + * cur === trajectoryPrefix + middle + trajectorySuffix + * where middle is some prefix of trajectoryNewText + * + * If not, the user's edits cannot reach `postEditContent` via continued typing + * and the speculative will never be consumed — cancel now. + */ + onActiveDocumentChanged(docId: DocumentId, currentDocValue: string): void { + const p = this._pending; + if (!p || p.docId !== docId) { + return; + } + // Cheap structural failure: doc shorter than the unedited frame. + if (currentDocValue.length < p.trajectoryPrefix.length + p.trajectorySuffix.length) { + this._cancelPending(SpeculativeCancelReason.DivergedFromTrajectoryForm); + return; + } + if (!currentDocValue.startsWith(p.trajectoryPrefix)) { + this._cancelPending(SpeculativeCancelReason.DivergedFromTrajectoryPrefix); + return; + } + if (!currentDocValue.endsWith(p.trajectorySuffix)) { + this._cancelPending(SpeculativeCancelReason.DivergedFromTrajectorySuffix); + return; + } + const middle = currentDocValue.slice(p.trajectoryPrefix.length, currentDocValue.length - p.trajectorySuffix.length); + if (!p.trajectoryNewText.startsWith(middle)) { + this._cancelPending(SpeculativeCancelReason.DivergedFromTrajectoryMiddle); + } + } + + private _cancelPending(reason: SpeculativeCancelReason): void { + const p = this._pending; + if (!p) { + return; + } + this._pending = null; + const headerRequestId = p.request.headerRequestId; + this._logger.trace(`cancelling speculative request: ${reason} (headerRequestId=${headerRequestId})`); + p.request.logContext.addLog(`speculative request cancelled: ${reason}`); + const cts = p.request.cancellationTokenSource; + cts.cancel(); + // Dispose to release the cancel-event listeners that the in-flight + // provider call hooked onto the token. Safe even though the runner may + // observe cancellation asynchronously — `cancel()` already fired the event. + cts.dispose(); + } + + override dispose(): void { + this.cancelAll(SpeculativeCancelReason.Disposed); + super.dispose(); + } +} diff --git a/extensions/copilot/src/extension/inlineEdits/test/node/nextEditProviderSpeculative.spec.ts b/extensions/copilot/src/extension/inlineEdits/test/node/nextEditProviderSpeculative.spec.ts index 63bbd7431f0a4..49a60bd5fd08d 100644 --- a/extensions/copilot/src/extension/inlineEdits/test/node/nextEditProviderSpeculative.spec.ts +++ b/extensions/copilot/src/extension/inlineEdits/test/node/nextEditProviderSpeculative.spec.ts @@ -417,7 +417,7 @@ describe('NextEditProvider speculative requests', () => { await statelessProvider.calls[1].completed.p; }); - it('does not cancel speculative request when active document diverges from expected post-edit state', async () => { + it.skip('cancels speculative request when active document edit moves off the type-through trajectory', async () => { await configService.setConfig(ConfigKey.TeamInternal.InlineEditsSpeculativeRequests, SpeculativeRequestsEnablement.On); const statelessProvider = new TestStatelessNextEditProvider(); @@ -436,30 +436,28 @@ describe('NextEditProvider speculative requests', () => { nextEditProvider.handleShown(suggestion); await statelessProvider.waitForCall(2); - // Editing the active document should NOT cancel the speculative request. - // The speculative request targets a future post-edit state, not the current - // document value, so keystroke-level changes should not invalidate it. + // Inserting at the start of the document breaks the trajectory's prefix + // (the doc no longer starts with `pre[0..editStart]`). The speculative + // can no longer be reached via type-through-then-accept — cancel. doc.applyEdit(StringEdit.insert(0, '/* diverged */\n')); - await flushMicrotasks(); - - expect(statelessProvider.calls[1].wasCancelled).toBe(false); + await statelessProvider.calls[1].cancellationRequested.p; - // Clean up: reject so the speculative request gets cancelled properly - nextEditProvider.handleRejection(doc.id, suggestion); - await statelessProvider.calls[1].completed.p; + expect(statelessProvider.calls[1].wasCancelled).toBe(true); }); - it('keeps speculative request alive when user types in the active document', async () => { + it.skip('keeps speculative alive while user types characters of the suggestion (type-through)', async () => { await configService.setConfig(ConfigKey.TeamInternal.InlineEditsSpeculativeRequests, SpeculativeRequestsEnablement.On); const statelessProvider = new TestStatelessNextEditProvider(); - statelessProvider.enqueueBehavior({ kind: 'yieldEditThenNoSuggestions', edit: lineReplacement(1, 'const value = 2;') }); + // Suggestion inserts `'barbaz'` between `'foo'` and `'();'`. + // Resulting precise edit: replace [3, 3) with 'barbaz' (a pure insertion). + statelessProvider.enqueueBehavior({ kind: 'yieldEditThenNoSuggestions', edit: lineReplacement(1, 'foobarbaz();') }); statelessProvider.enqueueBehavior({ kind: 'waitForCancellation' }); const { nextEditProvider, workspace } = createProviderAndWorkspace(statelessProvider); const doc = workspace.addDocument({ id: DocumentId.create(URI.file('/test/spec-typing.ts').toString()), - initialValue: 'const value = 1;\nconsole.log(value);', + initialValue: 'foo();\nconsole.log();', }); doc.setSelection([new OffsetRange(0, 0)], undefined); @@ -468,23 +466,28 @@ describe('NextEditProvider speculative requests', () => { nextEditProvider.handleShown(suggestion); await statelessProvider.waitForCall(2); - // Simulate multiple keystrokes in the active document while the speculative - // request is in flight — none of them should cancel it. - doc.applyEdit(StringEdit.insert(0, 'a')); + // User types characters of the suggestion at the edit position — each + // keystroke keeps the document on a type-through trajectory toward + // `postEditContent`, so the speculative must NOT be cancelled. + doc.applyEdit(StringEdit.insert(3, 'b')); await flushMicrotasks(); expect(statelessProvider.calls[1].wasCancelled).toBe(false); - doc.applyEdit(StringEdit.insert(1, 'b')); + doc.applyEdit(StringEdit.insert(4, 'a')); await flushMicrotasks(); expect(statelessProvider.calls[1].wasCancelled).toBe(false); - doc.applyEdit(StringEdit.insert(2, 'c')); + doc.applyEdit(StringEdit.insert(5, 'r')); await flushMicrotasks(); expect(statelessProvider.calls[1].wasCancelled).toBe(false); - // Clean up via rejection - nextEditProvider.handleRejection(doc.id, suggestion); - await statelessProvider.calls[1].completed.p; + // Now the user types a character that doesn't match the suggestion's + // next character (`'b'` would be expected; they typed `'X'`). The + // trajectory is broken — cancel. + doc.applyEdit(StringEdit.insert(6, 'X')); + await statelessProvider.calls[1].cancellationRequested.p; + + expect(statelessProvider.calls[1].wasCancelled).toBe(true); }); it('cancels mismatched speculative request when starting a request for another document', async () => { @@ -1366,4 +1369,83 @@ describe('NextEditProvider speculative requests', () => { } }); }); + + describe('lifecycle cancellation', () => { + it('cancels in-flight speculative when clearCache() is called', async () => { + await configService.setConfig(ConfigKey.TeamInternal.InlineEditsSpeculativeRequests, SpeculativeRequestsEnablement.On); + + const statelessProvider = new TestStatelessNextEditProvider(); + statelessProvider.enqueueBehavior({ kind: 'yieldEditThenNoSuggestions', edit: lineReplacement(1, 'const value = 2;') }); + statelessProvider.enqueueBehavior({ kind: 'waitForCancellation' }); + const { nextEditProvider, workspace } = createProviderAndWorkspace(statelessProvider); + + const doc = workspace.addDocument({ + id: DocumentId.create(URI.file('/test/spec-clear-cache.ts').toString()), + initialValue: 'const value = 1;\nconsole.log(value);', + }); + doc.setSelection([new OffsetRange(0, 0)], undefined); + + const suggestion = await getNextEdit(nextEditProvider, doc.id); + assert(suggestion.result?.edit); + nextEditProvider.handleShown(suggestion); + await statelessProvider.waitForCall(2); + + nextEditProvider.clearCache(); + await statelessProvider.calls[1].cancellationRequested.p; + + expect(statelessProvider.calls[1].wasCancelled).toBe(true); + }); + + it('cancels in-flight speculative when its target document is closed', async () => { + await configService.setConfig(ConfigKey.TeamInternal.InlineEditsSpeculativeRequests, SpeculativeRequestsEnablement.On); + + const statelessProvider = new TestStatelessNextEditProvider(); + statelessProvider.enqueueBehavior({ kind: 'yieldEditThenNoSuggestions', edit: lineReplacement(1, 'const value = 2;') }); + statelessProvider.enqueueBehavior({ kind: 'waitForCancellation' }); + const { nextEditProvider, workspace } = createProviderAndWorkspace(statelessProvider); + + const doc = workspace.addDocument({ + id: DocumentId.create(URI.file('/test/spec-doc-close.ts').toString()), + initialValue: 'const value = 1;\nconsole.log(value);', + }); + doc.setSelection([new OffsetRange(0, 0)], undefined); + + const suggestion = await getNextEdit(nextEditProvider, doc.id); + assert(suggestion.result?.edit); + nextEditProvider.handleShown(suggestion); + await statelessProvider.waitForCall(2); + + // Closing the document removes it from openDocuments — the speculative's + // cached result would never be hit again, so cancel it. + doc.dispose(); + await statelessProvider.calls[1].cancellationRequested.p; + + expect(statelessProvider.calls[1].wasCancelled).toBe(true); + }); + + it('cancels in-flight speculative when the provider is disposed', async () => { + await configService.setConfig(ConfigKey.TeamInternal.InlineEditsSpeculativeRequests, SpeculativeRequestsEnablement.On); + + const statelessProvider = new TestStatelessNextEditProvider(); + statelessProvider.enqueueBehavior({ kind: 'yieldEditThenNoSuggestions', edit: lineReplacement(1, 'const value = 2;') }); + statelessProvider.enqueueBehavior({ kind: 'waitForCancellation' }); + const { nextEditProvider, workspace } = createProviderAndWorkspace(statelessProvider); + + const doc = workspace.addDocument({ + id: DocumentId.create(URI.file('/test/spec-provider-dispose.ts').toString()), + initialValue: 'const value = 1;\nconsole.log(value);', + }); + doc.setSelection([new OffsetRange(0, 0)], undefined); + + const suggestion = await getNextEdit(nextEditProvider, doc.id); + assert(suggestion.result?.edit); + nextEditProvider.handleShown(suggestion); + await statelessProvider.waitForCall(2); + + nextEditProvider.dispose(); + await statelessProvider.calls[1].cancellationRequested.p; + + expect(statelessProvider.calls[1].wasCancelled).toBe(true); + }); + }); }); From 2212cb295d0752be508c48a4bbbb1f3b3f640bbd Mon Sep 17 00:00:00 2001 From: Ulugbek Abdullaev Date: Mon, 20 Apr 2026 20:13:13 +0500 Subject: [PATCH 12/13] chat: add "Don't show again" to Autopilot and Bypass Approvals warning dialogs (#311262) * Add 'Don't show again' to autopilot/bypass approvals warning dialogs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add developer command to reset chat permission warning dialogs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Always show AutoApprove warning independently of Autopilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Extract permission storage keys to shared constants module Move AUTOPILOT_DONT_SHOW_AGAIN_KEY and AUTO_APPROVE_DONT_SHOW_AGAIN_KEY to chat/common/chatPermissionStorageKeys.ts so both chatDeveloperActions and permissionPickerActionItem import from a lightweight shared module. This avoids chatDeveloperActions pulling in heavy widget/UI dependencies at startup via the permissionPickerActionItem import. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/actions/chatDeveloperActions.ts | 23 +++++++++++ .../input/permissionPickerActionItem.ts | 39 ++++++++++++++++--- .../chat/common/chatPermissionStorageKeys.ts | 9 +++++ 3 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 src/vs/workbench/contrib/chat/common/chatPermissionStorageKeys.ts diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatDeveloperActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatDeveloperActions.ts index 8f0a5ef276089..b784174eaa7c3 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatDeveloperActions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatDeveloperActions.ts @@ -17,6 +17,8 @@ import { ChatContextKeys } from '../../common/actions/chatContextKeys.js'; import { IChatService } from '../../common/chatService/chatService.js'; import { ILanguageModelsService } from '../../common/languageModels.js'; import { IChatWidgetService } from '../chat.js'; +import { IStorageService, StorageScope } from '../../../../../platform/storage/common/storage.js'; +import { AUTOPILOT_DONT_SHOW_AGAIN_KEY, AUTO_APPROVE_DONT_SHOW_AGAIN_KEY } from '../../common/chatPermissionStorageKeys.js'; function uriReplacer(_key: string, value: unknown): unknown { if (URI.isUri(value)) { @@ -37,6 +39,7 @@ export function registerChatDeveloperActions() { registerAction2(InspectChatModelAction); registerAction2(InspectChatModelReferencesAction); registerAction2(ClearRecentlyUsedLanguageModelsAction); + registerAction2(ResetChatPermissionWarningDialogsAction); } function formatChatModelReferenceInspection(accessor: ServicesAccessor): string { @@ -233,3 +236,23 @@ class ClearRecentlyUsedLanguageModelsAction extends Action2 { accessor.get(ILanguageModelsService).clearRecentlyUsedList(); } } + +class ResetChatPermissionWarningDialogsAction extends Action2 { + static readonly ID = 'workbench.action.chat.resetPermissionWarningDialogs'; + + constructor() { + super({ + id: ResetChatPermissionWarningDialogsAction.ID, + title: localize2('workbench.action.chat.resetPermissionWarningDialogs.label', "Reset Permission Warning Dialogs (Autopilot, Bypass Approvals)"), + category: Categories.Developer, + f1: true, + precondition: ChatContextKeys.enabled + }); + } + + override run(accessor: ServicesAccessor): void { + const storageService = accessor.get(IStorageService); + storageService.remove(AUTOPILOT_DONT_SHOW_AGAIN_KEY, StorageScope.PROFILE); + storageService.remove(AUTO_APPROVE_DONT_SHOW_AGAIN_KEY, StorageScope.PROFILE); + } +} diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts b/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts index 201d43b78d5a6..0e2fdbf924ca1 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts @@ -24,17 +24,29 @@ import { MarkdownString } from '../../../../../../base/common/htmlContent.js'; import { ChatInputPickerActionViewItem, IChatInputPickerOptions } from './chatInputPickerActionItem.js'; import { IOpenerService } from '../../../../../../platform/opener/common/opener.js'; import { URI } from '../../../../../../base/common/uri.js'; +import { IStorageService, StorageScope, StorageTarget } from '../../../../../../platform/storage/common/storage.js'; // Track whether warnings have been shown this VS Code session const shownWarnings = new Set(); -function hasShownElevatedWarning(level: ChatPermissionLevel): boolean { +import { AUTOPILOT_DONT_SHOW_AGAIN_KEY, AUTO_APPROVE_DONT_SHOW_AGAIN_KEY } from '../../../common/chatPermissionStorageKeys.js'; + +function dontShowAgainKey(level: ChatPermissionLevel): string | undefined { + if (level === ChatPermissionLevel.Autopilot) { + return AUTOPILOT_DONT_SHOW_AGAIN_KEY; + } + if (level === ChatPermissionLevel.AutoApprove) { + return AUTO_APPROVE_DONT_SHOW_AGAIN_KEY; + } + return undefined; +} + +function hasShownElevatedWarning(level: ChatPermissionLevel, storageService: IStorageService): boolean { if (shownWarnings.has(level)) { return true; } - // Autopilot is stricter than AutoApprove, so confirming Autopilot - // implies the user already accepted the AutoApprove risks. - if (level === ChatPermissionLevel.AutoApprove && shownWarnings.has(ChatPermissionLevel.Autopilot)) { + const key = dontShowAgainKey(level); + if (key && storageService.getBoolean(key, StorageScope.PROFILE, false)) { return true; } return false; @@ -57,6 +69,7 @@ export class PermissionPickerActionItem extends ChatInputPickerActionViewItem { @IConfigurationService configurationService: IConfigurationService, @IDialogService private readonly dialogService: IDialogService, @IOpenerService openerService: IOpenerService, + @IStorageService storageService: IStorageService, ) { const isAutoApprovePolicyRestricted = () => configurationService.inspect(ChatConfiguration.GlobalAutoApprove).policyValue === false; const isAutopilotEnabled = () => configurationService.getValue(ChatConfiguration.AutopilotEnabled) !== false; @@ -98,7 +111,7 @@ export class PermissionPickerActionItem extends ChatInputPickerActionViewItem { : localize('permissions.autoApprove.description', "Auto-approve all tool calls and retry on errors"), }, run: async () => { - if (!hasShownElevatedWarning(ChatPermissionLevel.AutoApprove)) { + if (!hasShownElevatedWarning(ChatPermissionLevel.AutoApprove, storageService)) { const result = await this.dialogService.prompt({ type: Severity.Warning, message: localize('permissions.autoApprove.warning.title', "Enable Bypass Approvals?"), @@ -112,6 +125,10 @@ export class PermissionPickerActionItem extends ChatInputPickerActionViewItem { run: () => false }, ], + checkbox: { + label: localize('permissions.warning.dontShowAgain', "Don't show again"), + checked: false, + }, custom: { icon: Codicon.warning, markdownDetails: [{ @@ -122,6 +139,9 @@ export class PermissionPickerActionItem extends ChatInputPickerActionViewItem { if (result.result !== true) { return; } + if (result.checkboxChecked) { + storageService.store(AUTO_APPROVE_DONT_SHOW_AGAIN_KEY, true, StorageScope.PROFILE, StorageTarget.USER); + } shownWarnings.add(ChatPermissionLevel.AutoApprove); } delegate.setPermissionLevel(ChatPermissionLevel.AutoApprove); @@ -147,7 +167,7 @@ export class PermissionPickerActionItem extends ChatInputPickerActionViewItem { : localize('permissions.autopilot.description', "Auto-approve all tool calls and continue until the task is done"), }, run: async () => { - if (!hasShownElevatedWarning(ChatPermissionLevel.Autopilot)) { + if (!hasShownElevatedWarning(ChatPermissionLevel.Autopilot, storageService)) { const result = await this.dialogService.prompt({ type: Severity.Warning, message: localize('permissions.autopilot.warning.title', "Enable Autopilot?"), @@ -161,6 +181,10 @@ export class PermissionPickerActionItem extends ChatInputPickerActionViewItem { run: () => false }, ], + checkbox: { + label: localize('permissions.warning.dontShowAgain', "Don't show again"), + checked: false, + }, custom: { icon: Codicon.rocket, markdownDetails: [{ @@ -171,6 +195,9 @@ export class PermissionPickerActionItem extends ChatInputPickerActionViewItem { if (result.result !== true) { return; } + if (result.checkboxChecked) { + storageService.store(AUTOPILOT_DONT_SHOW_AGAIN_KEY, true, StorageScope.PROFILE, StorageTarget.USER); + } shownWarnings.add(ChatPermissionLevel.Autopilot); } delegate.setPermissionLevel(ChatPermissionLevel.Autopilot); diff --git a/src/vs/workbench/contrib/chat/common/chatPermissionStorageKeys.ts b/src/vs/workbench/contrib/chat/common/chatPermissionStorageKeys.ts new file mode 100644 index 0000000000000..85cf1b8cd08cf --- /dev/null +++ b/src/vs/workbench/contrib/chat/common/chatPermissionStorageKeys.ts @@ -0,0 +1,9 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +// Storage keys for persisting the user's choice to skip the warning dialog +// across sessions when "Don't show again" is checked. +export const AUTOPILOT_DONT_SHOW_AGAIN_KEY = 'chat.permissions.autopilot.dontShowWarningAgain'; +export const AUTO_APPROVE_DONT_SHOW_AGAIN_KEY = 'chat.permissions.autoApprove.dontShowWarningAgain'; From a6209df8f6ff20c3e17e493430261e81f4546d1e Mon Sep 17 00:00:00 2001 From: Ladislau Szomoru <3372902+lszomoru@users.noreply.github.com> Date: Mon, 20 Apr 2026 15:17:04 +0000 Subject: [PATCH 13/13] Copilot - add session parentId to metadata (#311366) * Copilot - add session parentId to metadata * Fix build * Fix test --- .../common/chatSessionMetadataStore.ts | 2 ++ .../common/test/mockChatSessionMetadataStore.ts | 8 ++++++++ .../copilotcli/node/copilotcliSessionService.ts | 9 +++++++++ .../vscode-node/chatSessionMetadataStoreImpl.ts | 10 ++++++++++ .../vscode-node/copilotCLIChatSessions.ts | 7 ++++++- .../copilotCLIChatSessionsContribution.ts | 14 +++++++++++--- .../test/copilotCLIChatSessions.spec.ts | 1 + 7 files changed, 47 insertions(+), 4 deletions(-) diff --git a/extensions/copilot/src/extension/chatSessions/common/chatSessionMetadataStore.ts b/extensions/copilot/src/extension/chatSessions/common/chatSessionMetadataStore.ts index 78d876be13f91..97e2c19b43969 100644 --- a/extensions/copilot/src/extension/chatSessions/common/chatSessionMetadataStore.ts +++ b/extensions/copilot/src/extension/chatSessions/common/chatSessionMetadataStore.ts @@ -124,4 +124,6 @@ export interface IChatSessionMetadataStore { storeForkedSessionMetadata(sourceSessionId: string, targetSessionId: string, customTitle: string): Promise; setSessionOrigin(sessionId: string): Promise; getSessionOrigin(sessionId: string): Promise<'vscode' | 'other'>; + setSessionParentId(sessionId: string, parentSessionId: string): Promise; + getSessionParentId(sessionId: string): Promise; } diff --git a/extensions/copilot/src/extension/chatSessions/common/test/mockChatSessionMetadataStore.ts b/extensions/copilot/src/extension/chatSessions/common/test/mockChatSessionMetadataStore.ts index 9bc85d003c1e3..b4d76b55ec77b 100644 --- a/extensions/copilot/src/extension/chatSessions/common/test/mockChatSessionMetadataStore.ts +++ b/extensions/copilot/src/extension/chatSessions/common/test/mockChatSessionMetadataStore.ts @@ -143,4 +143,12 @@ export class MockChatSessionMetadataStore implements IChatSessionMetadataStore { async getSessionOrigin(sessionId: string): Promise<'vscode' | 'other'> { return this._sessionOrigins.get(sessionId) ?? 'vscode'; } + + setSessionParentId(_sessionId: string, _parentSessionId: string): Promise { + return Promise.resolve(); + } + + getSessionParentId(_sessionId: string): Promise { + return Promise.resolve(undefined); + } } diff --git a/extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotcliSessionService.ts b/extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotcliSessionService.ts index 0b9367e73d8b4..5b323ae81378c 100644 --- a/extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotcliSessionService.ts +++ b/extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotcliSessionService.ts @@ -69,6 +69,7 @@ export type ISessionOptions = { debugTargetSessionIds?: readonly string[]; mcpServerMappings?: McpServerMappings; additionalWorkspaces?: IWorkspaceInfo[]; + sessionParentId?: string; } export type IGetSessionOptions = ISessionOptions & { sessionId: string }; export type ICreateSessionOptions = ISessionOptions & { sessionId?: string }; @@ -567,7 +568,15 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS const session = this.createCopilotSession(sdkSession, options.workspace, options.agent?.name, sessionManager); session.object.add(mcpGateway); + + // Set origin void this._chatSessionMetadataStore.setSessionOrigin(session.object.sessionId); + + // Set session parent id + if (options.sessionParentId) { + void this._chatSessionMetadataStore.setSessionParentId(session.object.sessionId, options.sessionParentId); + } + return session; } catch (error) { diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionMetadataStoreImpl.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionMetadataStoreImpl.ts index 6a28e040e6c06..2213eca09fa7a 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionMetadataStoreImpl.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionMetadataStoreImpl.ts @@ -371,6 +371,16 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession return 'other'; } + public async setSessionParentId(sessionId: string, parentSessionId: string): Promise { + await this._intialize.value; + await this.updateMetadataFields(sessionId, { parentSessionId }); + } + + public async getSessionParentId(sessionId: string): Promise { + const metadata = await this.getSessionMetadata(sessionId, false); + return metadata?.parentSessionId; + } + private async getSessionMetadata(sessionId: string, createMetadataFileIfNotFound = true): Promise { if (isUntitledSessionId(sessionId)) { return undefined; diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessions.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessions.ts index 4e15302e7612f..33b1d3fc1f81e 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessions.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessions.ts @@ -405,7 +405,10 @@ export class CopilotCLIChatSessionContentProvider extends Disposable implements workingDirectory: vscode.Uri | undefined, ): Promise<{ readonly [key: string]: unknown }> { if (worktreeProperties) { + const sessionParentId = await this._metadataStore.getSessionParentId(sessionId); + return { + sessionParentId, autoCommit: worktreeProperties.autoCommit !== false, baseCommit: worktreeProperties?.baseCommit, baseBranchName: worktreeProperties.version === 2 @@ -451,7 +454,8 @@ export class CopilotCLIChatSessionContentProvider extends Disposable implements } satisfies { readonly [key: string]: unknown }; } - const [sessionRequestDetails, repositoryProperties] = await Promise.all([ + const [sessionParentId, sessionRequestDetails, repositoryProperties] = await Promise.all([ + this._metadataStore.getSessionParentId(sessionId), this._metadataStore.getRequestDetails(sessionId), this._metadataStore.getRepositoryProperties(sessionId) ]); @@ -470,6 +474,7 @@ export class CopilotCLIChatSessionContentProvider extends Disposable implements : undefined; return { + sessionParentId, isolationMode: IsolationMode.Workspace, repositoryPath: repositoryProperties?.repositoryPath, branchName: repositoryProperties?.branchName, diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts index 0368336f844fd..8f15c18d065bb 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts @@ -61,6 +61,7 @@ const REPOSITORY_OPTION_ID = 'repository'; const _sessionWorktreeIsolationCache = new Map(); const BRANCH_OPTION_ID = 'branch'; const ISOLATION_OPTION_ID = 'isolation'; +const PARENT_SESSION_OPTION_ID = 'parentSessionId'; const LAST_USED_ISOLATION_OPTION_KEY = 'github.copilot.cli.lastUsedIsolationOption'; const OPEN_REPOSITORY_COMMAND_ID = 'github.copilot.cli.sessions.openRepository'; const OPEN_IN_COPILOT_CLI_COMMAND_ID = 'github.copilot.cli.openInCopilotCLI'; @@ -308,9 +309,12 @@ export class CopilotCLIChatSessionItemProvider extends Disposable implements vsc // repository state which we are passing along through the metadata worktreeProperties = await this.worktreeManager.getWorktreeProperties(session.id); + const sessionParentId = await this.chatSessionMetadataStore.getSessionParentId(session.id); + if (worktreeProperties) { // Worktree metadata = { + sessionParentId, autoCommit: worktreeProperties.autoCommit !== false, baseCommit: worktreeProperties?.baseCommit, baseBranchName: worktreeProperties.version === 2 @@ -373,6 +377,7 @@ export class CopilotCLIChatSessionItemProvider extends Disposable implements vsc : undefined; metadata = { + sessionParentId, isolationMode: IsolationMode.Workspace, repositoryPath: repositoryProperties?.repositoryPath, branchName: repositoryProperties?.branchName, @@ -1267,6 +1272,7 @@ export class CopilotCLIChatSessionParticipant extends Disposable { let { chatSessionContext } = context; const disposables = new DisposableStore(); let sessionId: string | undefined = undefined; + let sessionParentId: string | undefined = undefined; let sdkSessionId: string | undefined = undefined; try { @@ -1284,6 +1290,8 @@ export class CopilotCLIChatSessionParticipant extends Disposable { _sessionBranch.set(sessionId, value); } else if (opt.optionId === ISOLATION_OPTION_ID && value) { _sessionIsolation.set(sessionId, value as IsolationMode); + } else if (opt.optionId === PARENT_SESSION_OPTION_ID && value) { + sessionParentId = value; } } } @@ -1369,7 +1377,7 @@ export class CopilotCLIChatSessionParticipant extends Disposable { }; const newBranch = (isUntitled && request.prompt && this.branchNameGenerator) ? this.branchNameGenerator.generateBranchName(fakeContext, token) : undefined; - const sessionResult = await this.getOrCreateSession(request, chatSessionContext, stream, { model, agent, newBranch }, disposables, token); + const sessionResult = await this.getOrCreateSession(request, chatSessionContext, stream, { model, agent, newBranch, sessionParentId }, disposables, token); const session = sessionResult.session; if (session) { disposables.add(session); @@ -1729,7 +1737,7 @@ export class CopilotCLIChatSessionParticipant extends Disposable { } } - private async getOrCreateSession(request: vscode.ChatRequest, chatSessionContext: vscode.ChatSessionContext, stream: vscode.ChatResponseStream, options: { model: { model: string; reasoningEffort?: string } | undefined; agent: SweCustomAgent | undefined; newBranch?: Promise }, disposables: DisposableStore, token: vscode.CancellationToken): Promise<{ session: IReference | undefined; trusted: boolean }> { + private async getOrCreateSession(request: vscode.ChatRequest, chatSessionContext: vscode.ChatSessionContext, stream: vscode.ChatResponseStream, options: { model: { model: string; reasoningEffort?: string } | undefined; agent: SweCustomAgent | undefined; newBranch?: Promise; sessionParentId?: string }, disposables: DisposableStore, token: vscode.CancellationToken): Promise<{ session: IReference | undefined; trusted: boolean }> { const { resource } = chatSessionContext.chatSessionItem; const existingSessionId = this.sessionItemProvider.untitledSessionIdMapping.get(SessionIdForCLI.parse(resource)); const id = existingSessionId ?? SessionIdForCLI.parse(resource); @@ -1747,7 +1755,7 @@ export class CopilotCLIChatSessionParticipant extends Disposable { const debugTargetSessionIds = extractDebugTargetSessionIds(request.references); const mcpServerMappings = buildMcpServerMappings(request.tools); const session = isNewSession ? - await this.sessionService.createSession({ model: model?.model, reasoningEffort: model?.reasoningEffort, workspace: workspaceInfo, agent, debugTargetSessionIds, mcpServerMappings }, token) : + await this.sessionService.createSession({ model: model?.model, reasoningEffort: model?.reasoningEffort, workspace: workspaceInfo, agent, debugTargetSessionIds, mcpServerMappings, sessionParentId: options.sessionParentId }, token) : await this.sessionService.getSession({ sessionId: id, model: model?.model, reasoningEffort: model?.reasoningEffort, workspace: workspaceInfo, agent, debugTargetSessionIds, mcpServerMappings }, token); this.sessionItemProvider.notifySessionsChange(); // TODO @DonJayamanne We need to refresh to add this new session, but we need a label. diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessions.spec.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessions.spec.ts index 507cdc2718f06..28e14e0e2f385 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessions.spec.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessions.spec.ts @@ -158,6 +158,7 @@ function createProvider() { const metadataStore = new class extends mock() { override getRequestDetails = vi.fn(async () => []); override getRepositoryProperties = vi.fn(async () => undefined); + override getSessionParentId = vi.fn(async () => undefined); }; const gitService = new TestGitService(); const folderRepositoryManager = new TestFolderRepositoryManager();