From 8d48b77e9fc7df97b659e8a04bc999bb6fb8f031 Mon Sep 17 00:00:00 2001 From: roblourens Date: Wed, 2 Sep 2026 21:51:27 -0700 Subject: [PATCH 01/13] agentHost: drain providers during standalone shutdown (#334151) * agentHost: drain providers during standalone shutdown Ensure the standalone WebSocket host stops protocol ingress, awaits provider teardown, and flushes persistence before disposal. This lets Claude finish its transcript write before an E2E restart launches the replacement host. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: dispose pending protocol connections Track accepted transports before initialization so standalone shutdown closes every connection before waiting for in-flight requests. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/node/agentHostServerMain.ts | 25 ++++++------ .../agentHost/node/agentHostShutdown.ts | 27 +++++++++++++ .../agentHost/node/protocolServerHandler.ts | 4 +- .../test/node/agentHostShutdown.test.ts | 40 ++++++++++++++++++- .../test/node/protocolServerHandler.test.ts | 11 +++++ 5 files changed, 92 insertions(+), 15 deletions(-) diff --git a/src/vs/platform/agentHost/node/agentHostServerMain.ts b/src/vs/platform/agentHost/node/agentHostServerMain.ts index e6bb4493437b7..b6e53d93e1ce1 100644 --- a/src/vs/platform/agentHost/node/agentHostServerMain.ts +++ b/src/vs/platform/agentHost/node/agentHostServerMain.ts @@ -29,7 +29,7 @@ import { LoggerService } from '../../log/node/loggerService.js'; import { OtlpEmitterLogger, OtlpLogEmitter } from '../common/otlp/otlpLogEmitter.js'; import product from '../../product/common/product.js'; import { IProductService } from '../../product/common/productService.js'; -import { flushAgentHostPersistenceBeforeShutdown } from './agentHostShutdown.js'; +import { shutdownAgentHostBeforeDispose } from './agentHostShutdown.js'; import { ITelemetryService } from '../../telemetry/common/telemetry.js'; import { createAgentHostRuntime } from './agentHostBootstrap.js'; import { IAgentConfigurationService } from './agentConfigurationService.js'; @@ -317,7 +317,7 @@ async function main(): Promise { const clientFileSystemProvider = disposables.add(new AgentHostClientFileSystemProvider()); disposables.add(fileService.registerProvider(AGENT_CLIENT_SCHEME, clientFileSystemProvider)); // Wire up protocol handler - disposables.add(instantiationService.createInstance( + const protocolHandler = disposables.add(instantiationService.createInstance( ProtocolServerHandler, agentService, stateManager, @@ -379,18 +379,17 @@ async function main(): Promise { } shuttingDown = true; logService.info('[AgentHostServer] Shutting down...'); - // Close the WebSocket server first so no further actions can be - // dispatched while we wait for in-flight writes to flush — otherwise - // a late-arriving action could keep queuing DB writes and either - // undermine the flush or push us past the timeout. + // Stop protocol ingress before draining requests so no late action can + // race provider shutdown or queue persistence after the idle check. + protocolHandler.dispose(); wsServer.dispose(); - // Wait for in-flight persistence writes to flush. Without this, a - // SIGTERM arriving during a session or agent-host storage write can - // drop the latest decision. - // Capped so a stuck write cannot hang shutdown indefinitely. - await flushAgentHostPersistenceBeforeShutdown( - [sessionDataService.whenIdle(), customizationEnablementService.whenIdle()], - 3000, + // Providers such as Claude finish writing their transcripts during + // shutdown, so drain them before waiting for persistence to go idle. + await shutdownAgentHostBeforeDispose( + () => protocolHandler.whenIdle(), + () => agentService.shutdown(), + () => [sessionDataService.whenIdle(), customizationEnablementService.whenIdle()], + 4500, logService, ); disposables.dispose(); diff --git a/src/vs/platform/agentHost/node/agentHostShutdown.ts b/src/vs/platform/agentHost/node/agentHostShutdown.ts index 3c37c379f59c7..61077de3c42cb 100644 --- a/src/vs/platform/agentHost/node/agentHostShutdown.ts +++ b/src/vs/platform/agentHost/node/agentHostShutdown.ts @@ -6,6 +6,33 @@ import { raceTimeout } from '../../../base/common/async.js'; import type { ILogService } from '../../log/common/log.js'; +/** + * Drains protocol requests and providers before flushing persistence, without letting shutdown block process exit indefinitely. + */ +export async function shutdownAgentHostBeforeDispose( + drainProtocol: () => Promise, + shutdownProviders: () => Promise, + flushPersistence: () => readonly Promise[], + timeoutMs: number, + logService: Pick, +): Promise { + await raceTimeout((async () => { + try { + await drainProtocol(); + } catch (error) { + logService.error('[AgentHostServer] Failed to drain protocol requests; continuing shutdown.', error); + } + try { + await shutdownProviders(); + } catch (error) { + logService.error('[AgentHostServer] Failed to shut down providers; continuing shutdown.', error); + } + await flushAgentHostPersistenceBeforeShutdown(flushPersistence(), timeoutMs, logService); + })(), timeoutMs, () => { + logService.warn('[AgentHostServer] Timed out waiting for graceful shutdown; exiting anyway.'); + }); +} + /** * Flushes Agent Host persistence without allowing a failed or stalled write to * prevent process cleanup and exit. diff --git a/src/vs/platform/agentHost/node/protocolServerHandler.ts b/src/vs/platform/agentHost/node/protocolServerHandler.ts index bb8aed912d53f..e9a8d30625206 100644 --- a/src/vs/platform/agentHost/node/protocolServerHandler.ts +++ b/src/vs/platform/agentHost/node/protocolServerHandler.ts @@ -359,6 +359,7 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien private readonly _replayBuffer: ActionEnvelope[] = []; private readonly _telemetryReporter: AgentHostTelemetryReporter; private readonly _managedSettingsOwnerId = generateUuid(); + private readonly _connectionDisposables = this._register(new DisposableMap()); private readonly _onDidChangeConnectionCount = this._register(new Emitter()); @@ -423,6 +424,7 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien private _handleNewConnection(transport: IProtocolTransport): void { const disposables = new DisposableStore(); + this._connectionDisposables.set(transport, disposables); let client: IConnectedClient | undefined; disposables.add(transport.onMessage(msg => { @@ -562,7 +564,7 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien this._reportClientDisconnected(client, subscriptionCount); } } - disposables.dispose(); + this._connectionDisposables.deleteAndDispose(transport); })); disposables.add(transport); diff --git a/src/vs/platform/agentHost/test/node/agentHostShutdown.test.ts b/src/vs/platform/agentHost/test/node/agentHostShutdown.test.ts index 8662c617868c2..772a52675f5f7 100644 --- a/src/vs/platform/agentHost/test/node/agentHostShutdown.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostShutdown.test.ts @@ -6,7 +6,7 @@ import assert from 'assert'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; import { NullLogService } from '../../../log/common/log.js'; -import { flushAgentHostPersistenceBeforeShutdown } from '../../node/agentHostShutdown.js'; +import { flushAgentHostPersistenceBeforeShutdown, shutdownAgentHostBeforeDispose } from '../../node/agentHostShutdown.js'; suite('AgentHostShutdown', () => { ensureNoDisposablesAreLeakedInTestSuite(); @@ -18,4 +18,42 @@ suite('AgentHostShutdown', () => { new NullLogService(), )); }); + + test('providers shut down before persistence is flushed', async () => { + const steps: string[] = []; + + await shutdownAgentHostBeforeDispose( + async () => { + steps.push('protocol drain'); + }, + async () => { + steps.push('provider shutdown'); + }, + () => { + steps.push('persistence flush'); + return [Promise.resolve()]; + }, + 3000, + new NullLogService(), + ); + + assert.deepStrictEqual(steps, ['protocol drain', 'provider shutdown', 'persistence flush']); + }); + + test('a failed provider shutdown still flushes persistence', async () => { + let persistenceFlushed = false; + + await shutdownAgentHostBeforeDispose( + () => Promise.resolve(), + () => Promise.reject(new Error('provider unavailable')), + () => { + persistenceFlushed = true; + return [Promise.resolve()]; + }, + 3000, + new NullLogService(), + ); + + assert.strictEqual(persistenceFlushed, true); + }); }); diff --git a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts index c9ac211b8829a..0b41c43be7c9a 100644 --- a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts +++ b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts @@ -43,6 +43,7 @@ import { AgentHostTelemetryService } from '../../node/agentHostTelemetryService. class MockProtocolTransport implements IProtocolTransport { constructor(readonly transportKind = AgentHostTransportKind.Unknown) { } + isDisposed = false; private readonly _onMessage = new Emitter(); readonly onMessage = this._onMessage.event; private readonly _onDidSend = new Emitter(); @@ -66,6 +67,7 @@ class MockProtocolTransport implements IProtocolTransport { } dispose(): void { + this.isDisposed = true; this._onMessage.dispose(); this._onDidSend.dispose(); this._onClose.dispose(); @@ -723,6 +725,15 @@ suite('ProtocolServerHandler', () => { transport.simulateClose(); }); + test('dispose closes a connection before initialize', () => { + const transport = new MockProtocolTransport(); + server.simulateConnection(transport); + + handler.dispose(); + + assert.strictEqual(transport.isDisposed, true); + }); + test('unknown requests return MethodNotFound before and after initialize', () => { const transport = new MockProtocolTransport(); disposables.add(transport); From af195aabe861b9d99fcb322904793322ae7f0952 Mon Sep 17 00:00:00 2001 From: "vs-code-engineering[bot]" <122617954+vs-code-engineering[bot]@users.noreply.github.com> Date: Thu, 3 Sep 2026 08:01:42 +0200 Subject: [PATCH 02/13] [cherry-pick] Refactor model capabilities (#334179) Co-authored-by: vs-code-engineering[bot] --- .../prompts/node/agent/defaultAgentInstructions.tsx | 8 ++++---- .../src/extension/tools/node/manageTodoListTool.tsx | 4 ++-- .../src/platform/endpoint/common/chatModelCapabilities.ts | 4 ++-- .../endpoint/test/node/chatModelCapabilities.spec.ts | 6 +++++- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/extensions/copilot/src/extension/prompts/node/agent/defaultAgentInstructions.tsx b/extensions/copilot/src/extension/prompts/node/agent/defaultAgentInstructions.tsx index 782f35c152f37..90918df3b64d7 100644 --- a/extensions/copilot/src/extension/prompts/node/agent/defaultAgentInstructions.tsx +++ b/extensions/copilot/src/extension/prompts/node/agent/defaultAgentInstructions.tsx @@ -6,7 +6,7 @@ import { BasePromptElementProps, PromptElement, PromptSizing } from '@vscode/prompt-tsx'; import type { LanguageModelToolInformation } from 'vscode'; import { ConfigKey, IConfigurationService } from '../../../../platform/configuration/common/configurationService'; -import { isGpt5PlusFamily } from '../../../../platform/endpoint/common/chatModelCapabilities'; +import { isGpt5PlusFamily, isHiddenModelN } from '../../../../platform/endpoint/common/chatModelCapabilities'; import { IChatEndpoint } from '../../../../platform/networking/common/networking'; import { IPromptPathRepresentationService } from '../../../../platform/prompts/common/promptPathRepresentationService'; import { IExperimentationService } from '../../../../platform/telemetry/common/nullExperimentationService'; @@ -480,12 +480,12 @@ export class ApplyPatchInstructions extends PromptElement To edit files in the workspace, use the {ToolName.ApplyPatch} tool. If you have issues with it, you should first try to fix your patch and continue using {ToolName.ApplyPatch}. {this.props.tools[ToolName.EditFile] && <>If you are stuck, you can fall back on the {ToolName.EditFile} tool, but {ToolName.ApplyPatch} is much faster and is the preferred tool.}
- {isGpt5 && <>Prefer the smallest set of changes needed to satisfy the task. Avoid reformatting unrelated code; preserve existing style and public APIs unless the task requires changes. When practical, complete all edits for a file within a single message.
} + {isGpt5OrHiddenModelN && <>Prefer the smallest set of changes needed to satisfy the task. Avoid reformatting unrelated code; preserve existing style and public APIs unless the task requires changes. When practical, complete all edits for a file within a single message.
} {!useSimpleInstructions && <> The tool call requires both `input`, a string representing the patch to apply, and `explanation`, a short description of what the patch aims to achieve. The input follows a special format. For each snippet of code that needs to be changed, repeat the following:

diff --git a/extensions/copilot/src/extension/tools/node/manageTodoListTool.tsx b/extensions/copilot/src/extension/tools/node/manageTodoListTool.tsx index 046396d20d058..7eabf3113737b 100644 --- a/extensions/copilot/src/extension/tools/node/manageTodoListTool.tsx +++ b/extensions/copilot/src/extension/tools/node/manageTodoListTool.tsx @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import type * as vscode from 'vscode'; -import { isGpt5PlusFamily } from '../../../platform/endpoint/common/chatModelCapabilities'; +import { isGpt5PlusFamily, isHiddenModelN } from '../../../platform/endpoint/common/chatModelCapabilities'; import { IChatEndpoint } from '../../../platform/networking/common/networking'; import { ToolName } from '../common/toolNames'; import { ICopilotTool, ToolRegistry } from '../common/toolsRegistry'; @@ -17,7 +17,7 @@ class ManageTodoListTool implements ICopilotTool { public static readonly nonDeferred = true; alternativeDefinition(tool: vscode.LanguageModelToolInformation, endpoint?: IChatEndpoint): vscode.LanguageModelToolInformation { - if (!isGpt5PlusFamily(endpoint)) { + if (!isGpt5PlusFamily(endpoint) && (!endpoint || !isHiddenModelN(endpoint))) { return tool; } diff --git a/extensions/copilot/src/platform/endpoint/common/chatModelCapabilities.ts b/extensions/copilot/src/platform/endpoint/common/chatModelCapabilities.ts index d6e6f215015d2..78201355048ec 100644 --- a/extensions/copilot/src/platform/endpoint/common/chatModelCapabilities.ts +++ b/extensions/copilot/src/platform/endpoint/common/chatModelCapabilities.ts @@ -362,7 +362,7 @@ export function modelCanUseApplyPatchExclusively(model: LanguageModelChat | ICha if (isVSCModelReplaceStringSet(model)) { return false; } - return isGpt5PlusFamily(model) || isVSCModelA(model) || isVSCModelB(model); + return isGpt5PlusFamily(model) || isHiddenModelN(model) || isVSCModelA(model) || isVSCModelB(model); } /** @@ -378,7 +378,7 @@ export function modelNeedsStrongReplaceStringHint(model: LanguageModelChat | ICh * Model can take the simple, modern apply_patch instructions. */ export function modelSupportsSimplifiedApplyPatchInstructions(model: LanguageModelChat | IChatEndpoint): boolean { - return isGpt5PlusFamily(model) || isVSCModelA(model) || isVSCModelB(model); + return isGpt5PlusFamily(model) || isHiddenModelN(model) || isVSCModelA(model) || isVSCModelB(model); } export function isAnthropicFamily(model: LanguageModelChat | IChatEndpoint): boolean { diff --git a/extensions/copilot/src/platform/endpoint/test/node/chatModelCapabilities.spec.ts b/extensions/copilot/src/platform/endpoint/test/node/chatModelCapabilities.spec.ts index 4141482a8cfb3..d82a8cf6c21d5 100644 --- a/extensions/copilot/src/platform/endpoint/test/node/chatModelCapabilities.spec.ts +++ b/extensions/copilot/src/platform/endpoint/test/node/chatModelCapabilities.spec.ts @@ -9,7 +9,7 @@ import { ConfigKey, IConfigurationService } from '../../../configuration/common/ import { DefaultsOnlyConfigurationService } from '../../../configuration/common/defaultsOnlyConfigurationService'; import { InMemoryConfigurationService } from '../../../configuration/test/common/inMemoryConfigurationService'; import type { IChatEndpoint } from '../../../networking/common/networking'; -import { getModelCapabilityOverride, getVerbosityForModelSync, isGpt51Family, isGpt53Codex, isGpt54, isGpt55, isGpt56, isHiddenModelN, isKimiFamily, isOpenAIModel, modelCanUseApplyPatchExclusively, modelCanUseReplaceStringExclusively, modelPrefersJsonNotebookRepresentation, modelSupportCacheBreakPoints, modelSupportsApplyPatch, modelSupportsContextEditing, modelSupportsMultiReplaceString, modelSupportsPDFDocuments, modelSupportsReplaceString, modelSupportsToolSearch } from '../../common/chatModelCapabilities'; +import { getModelCapabilityOverride, getVerbosityForModelSync, isGpt51Family, isGpt53Codex, isGpt54, isGpt55, isGpt56, isHiddenModelN, isKimiFamily, isOpenAIModel, modelCanUseApplyPatchExclusively, modelCanUseReplaceStringExclusively, modelPrefersJsonNotebookRepresentation, modelSupportCacheBreakPoints, modelSupportsApplyPatch, modelSupportsContextEditing, modelSupportsMultiReplaceString, modelSupportsPDFDocuments, modelSupportsReplaceString, modelSupportsSimplifiedApplyPatchInstructions, modelSupportsToolSearch } from '../../common/chatModelCapabilities'; function fakeModel(family: string, model: string = family) { return { family, model } as unknown as IChatEndpoint; @@ -65,6 +65,8 @@ describe('Hidden model N capabilities', () => { isHidden: isHiddenModelN(model), isGpt56: isGpt56(model), applyPatch: modelSupportsApplyPatch(model), + applyPatchExclusively: modelCanUseApplyPatchExclusively(model), + simplifiedApplyPatchInstructions: modelSupportsSimplifiedApplyPatchInstructions(model), jsonNotebook: modelPrefersJsonNotebookRepresentation(model), pdf: modelSupportsPDFDocuments(model), cacheBreakpoints: modelSupportCacheBreakPoints(model), @@ -75,6 +77,8 @@ describe('Hidden model N capabilities', () => { isHidden: true, isGpt56: false, applyPatch: true, + applyPatchExclusively: true, + simplifiedApplyPatchInstructions: true, jsonNotebook: true, pdf: true, cacheBreakpoints: true, From 34488796bc6a96cb1dc71fb6f8148b271645482d Mon Sep 17 00:00:00 2001 From: Dileep Yavanmandha <52841896+dileepyavan@users.noreply.github.com> Date: Wed, 2 Sep 2026 23:39:43 -0700 Subject: [PATCH 03/13] chat: replace sandboxed label with icon (#334154) sandbox icon update --- .../contrib/chat/browser/media/chatWidget.css | 9 ++++ .../agentHostPermissionPickerDelegate.ts | 2 + .../agentHostPermissionPickerDelegate.test.ts | 10 ++-- .../browser/permissionPicker.ts | 23 ++++++++-- .../test/browser/permissionPicker.test.ts | 46 ++++++++++++++++++- .../agentHost/agentHostChatInputPicker.ts | 42 +++++++++++------ .../media/agentHostChatInputPicker.css | 9 ++++ .../agentHostChatInputPicker.test.ts | 37 ++++++++------- 8 files changed, 136 insertions(+), 42 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css index cb02d27df0d2f..26008a8a05829 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css @@ -156,6 +156,10 @@ display: none; } +.new-chat-widget-container .compact-picker .sessions-chat-sandbox-icon { + display: none; +} + .agent-sessions-workbench:not(.phone-layout) .new-chat-widget-container .new-chat-bottom-container .compact-picker.action-item, .agent-sessions-workbench:not(.phone-layout) .new-chat-widget-container .new-chat-bottom-container .compact-picker.sessions-chat-picker-slot { box-sizing: border-box; @@ -306,6 +310,11 @@ min-width: 0; } +.sessions-chat-sandbox-icon { + flex-shrink: 0; + margin-left: var(--vscode-spacing-size60); +} + .sessions-chat-picker-slot { display: flex; align-items: center; diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostPermissionPickerDelegate.ts b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostPermissionPickerDelegate.ts index 39829d7e81b78..8b9b96360e615 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostPermissionPickerDelegate.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostPermissionPickerDelegate.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { Codicon } from '../../../../../base/common/codicons.js'; import { Disposable, DisposableMap, DisposableStore } from '../../../../../base/common/lifecycle.js'; import { derived, IObservable, IReader, observableSignal } from '../../../../../base/common/observable.js'; import { localize } from '../../../../../nls.js'; @@ -120,6 +121,7 @@ export class AgentHostPermissionPickerDelegate extends Disposable implements IPe ...meta, label: localize('agentHostPermissionPicker.manual.label', "Manual permissions"), detail: localize('agentHostPermissionPicker.askWhenNeeded.detail', "Asks when approval settings don't apply"), + icon: Codicon.key, }; case ChatPermissionLevel.Assisted: return { ...meta, detail: localize('agentHostPermissionPicker.approveWhenSafe.detail', "Evaluates risk before running tools") }; diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostPermissionPickerDelegate.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostPermissionPickerDelegate.test.ts index 6fb98067d5ef9..f0731be1bf72f 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostPermissionPickerDelegate.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostPermissionPickerDelegate.test.ts @@ -256,16 +256,16 @@ suite('AgentHostPermissionPickerDelegate', () => { current: delegate.currentPermissionLevel.get(), metadata: delegate.availableLevels.map(level => { const baseMeta = getPermissionLevelMeta(level); - const { label, detail, hover } = delegate.getPermissionLevelMeta(level, baseMeta); - return { label, detail, hover }; + const { label, detail, hover, icon } = delegate.getPermissionLevelMeta(level, baseMeta); + return { label, detail, hover, icon: icon.id }; }), available: delegate.availableLevels, }, { current: ChatPermissionLevel.Assisted, metadata: [ - { label: 'Manual permissions', detail: 'Asks when approval settings don\'t apply', hover: undefined }, - { label: 'Assisted permissions', detail: 'Evaluates risk before running tools', hover: 'An LLM judge evaluates each tool call. Tools it doesn\'t approve require your approval.' }, - { label: 'Allow all', detail: 'Runs tool calls without asking', hover: undefined }, + { label: 'Manual permissions', detail: 'Asks when approval settings don\'t apply', hover: undefined, icon: 'key' }, + { label: 'Assisted permissions', detail: 'Evaluates risk before running tools', hover: 'An LLM judge evaluates each tool call. Tools it doesn\'t approve require your approval.', icon: 'sparkle' }, + { label: 'Allow all', detail: 'Runs tool calls without asking', hover: undefined, icon: 'warning' }, ], available: [ ChatPermissionLevel.Default, diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/permissionPicker.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/permissionPicker.ts index f401ff7813d7a..e0ab11b8eaded 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/permissionPicker.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/permissionPicker.ts @@ -187,7 +187,7 @@ export class PermissionPicker extends Disposable { if (this._delegate.getPermissionLevelHover) { this._renderDisposables.add(this.hoverService.setupDelayedHover(trigger, () => { const meta = this._getPermissionLevelMeta(this._currentLevel); - return { content: this._getPermissionLevelHover(this._currentLevel, meta) ?? '' }; + return { content: this._getTriggerHover(this._currentLevel, meta) }; })); } @@ -410,18 +410,24 @@ export class PermissionPicker extends Disposable { dom.clearNode(trigger); const meta = this._getPermissionLevelMeta(this._currentLevel); - const label = this._isSandboxToggleAvailable() && this._isSandboxingEnabled() + const sandboxed = this._isSandboxToggleAvailable() && this._isSandboxingEnabled(); + const accessibleLabel = sandboxed ? localize('permissionPicker.sandboxedLabel', "{0} (sandboxed)", meta.label) : meta.label; dom.append(trigger, renderIcon(meta.icon)); const labelSpan = dom.append(trigger, dom.$('span.sessions-chat-dropdown-label')); - labelSpan.textContent = label; + labelSpan.textContent = meta.label; + if (sandboxed) { + const sandboxIcon = dom.append(trigger, renderIcon(Codicon.shield)); + sandboxIcon.classList.add('sessions-chat-sandbox-icon'); + sandboxIcon.ariaHidden = 'true'; + } const hover = this._getPermissionLevelHover(this._currentLevel, meta); trigger.ariaLabel = hover - ? localize('permissionPicker.triggerAriaLabelWithDescription', "Pick Permission Level, {0}, {1}", label, hover) - : localize('permissionPicker.triggerAriaLabel', "Pick Permission Level, {0}", label); + ? localize('permissionPicker.triggerAriaLabelWithDescription', "Pick Permission Level, {0}, {1}", accessibleLabel, hover) + : localize('permissionPicker.triggerAriaLabel', "Pick Permission Level, {0}", accessibleLabel); trigger.classList.toggle('warning', this._currentLevel === ChatPermissionLevel.Autopilot || this._currentLevel === ChatPermissionLevel.Assisted); trigger.classList.toggle('info', this._currentLevel === ChatPermissionLevel.AutoApprove); @@ -483,6 +489,13 @@ export class PermissionPicker extends Disposable { return this._delegate.getPermissionLevelHover?.(level, meta) ?? meta.hover; } + private _getTriggerHover(level: ChatPermissionLevel, meta: IPermissionLevelMeta): string { + const hover = this._getPermissionLevelHover(level, meta) ?? ''; + return this._isSandboxToggleAvailable() && this._isSandboxingEnabled() + ? localize('permissionPicker.sandboxedHover', "{0} Terminal commands are sandboxed.", hover) + : hover; + } + protected _getPermissionLevelMeta(level: ChatPermissionLevel): IPermissionLevelMeta { const meta = getPermissionLevelMeta(level); return this._delegate.getPermissionLevelMeta(level, meta); diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/permissionPicker.test.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/permissionPicker.test.ts index fe67cb3b06736..ee9ea1507d423 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/permissionPicker.test.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/permissionPicker.test.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; +import { Codicon } from '../../../../../../base/common/codicons.js'; import { mock } from '../../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { IActionListDelegate, IActionListItem } from '../../../../../../platform/actionWidget/browser/actionList.js'; @@ -12,8 +13,9 @@ import { TestConfigurationService } from '../../../../../../platform/configurati import { IDialogService } from '../../../../../../platform/dialogs/common/dialogs.js'; import { IHoverService } from '../../../../../../platform/hover/browser/hover.js'; import { IOpenerService } from '../../../../../../platform/opener/common/opener.js'; +import { AgentSandboxEnabledValue } from '../../../../../../platform/sandbox/common/settings.js'; import { NullTelemetryService } from '../../../../../../platform/telemetry/common/telemetryUtils.js'; -import { ChatPermissionLevel } from '../../../../../../workbench/contrib/chat/common/constants.js'; +import { ChatConfiguration, ChatPermissionLevel } from '../../../../../../workbench/contrib/chat/common/constants.js'; import { TestStorageService } from '../../../../../../workbench/test/common/workbenchTestServices.js'; import { DEFAULT_PERMISSION_LEVELS, getPermissionLevelMeta, IPermissionPickerDelegate, PermissionPicker } from '../../browser/permissionPicker.js'; @@ -93,4 +95,46 @@ suite('Copilot PermissionPicker', () => { }, ]); }); + + test('uses a shield icon for the visible sandboxed state', () => { + const sandboxSettingId = 'test.sandbox.enabled'; + const configurationService = new TestConfigurationService(); + configurationService.setUserConfiguration(ChatConfiguration.PermissionsSandboxToggleEnabled, true); + configurationService.setUserConfiguration(sandboxSettingId, AgentSandboxEnabledValue.On); + const delegate: IPermissionPickerDelegate = { + getPermissionLevelMeta: (_level, meta) => ({ ...meta, label: 'Manual permissions', icon: Codicon.key }), + setPermissionLevel: () => { }, + sandboxTogglePresentation: 'standalone', + isSandboxToggleApplicable: () => true, + getSandboxToggleSettingId: () => sandboxSettingId, + }; + const picker = store.add(new PermissionPicker( + delegate, + new class extends mock() { + override readonly isVisible = false; + }(), + configurationService, + new class extends mock() { }(), + new class extends mock() { }(), + store.add(new TestStorageService()), + NullTelemetryService, + new class extends mock() { }(), + )); + const container = document.createElement('div'); + picker.render(container); + const trigger = container.querySelector('a.action-label'); + assert.ok(trigger); + + assert.deepStrictEqual({ + visibleLabel: trigger.querySelector('.sessions-chat-dropdown-label')?.textContent, + permissionIcon: trigger.querySelector('.codicon-key')?.className, + sandboxIcon: trigger.querySelector('.sessions-chat-sandbox-icon')?.className, + triggerAriaLabel: trigger.ariaLabel, + }, { + visibleLabel: 'Manual permissions', + permissionIcon: 'codicon codicon-key', + sandboxIcon: 'codicon codicon-shield sessions-chat-sandbox-icon', + triggerAriaLabel: 'Pick Permission Level, Manual permissions (sandboxed)', + }); + }); }); diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.ts index 1ca811c4114c6..5028d80a5bbd7 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatInputPicker.ts @@ -82,7 +82,7 @@ function getConfigIcon(property: string, value: unknown | undefined): ThemeIcon if (value === 'assisted') { return Codicon.sparkle; } - return Codicon.shield; + return Codicon.key; } if (property === ClaudeSessionConfigKey.PermissionMode && typeof value === 'string') { switch (value) { @@ -140,7 +140,7 @@ export function getAgentHostSandboxSettingId(sessionType: string | undefined, cu return getAgentHostCopilotSandboxSettingId(customTerminalToolEnabled, windows); } -export function getConfigPickerTriggerLabel(schema: SessionConfigPropertySchema, value: unknown | undefined, sandboxed: boolean): string { +export function getConfigPickerTriggerLabel(schema: SessionConfigPropertySchema, value: unknown | undefined): string { let label: string; if (schema.type === 'boolean') { label = value === true @@ -152,6 +152,10 @@ export function getConfigPickerTriggerLabel(schema: SessionConfigPropertySchema, } else { label = schema.title; } + return label; +} + +export function getConfigPickerAccessibleTriggerLabel(label: string, sandboxed: boolean): string { return sandboxed ? localize('agentHostChatInputPicker.sandboxedLabel', "{0} (sandboxed)", label) : label; @@ -186,7 +190,7 @@ function getEnumValueDescription(schema: SessionConfigPropertySchema, value: unk return index >= 0 ? schema.enumDescriptions?.[index] : undefined; } -export function getConfigPickerTriggerHover(property: string, schema: SessionConfigPropertySchema, value: unknown | undefined, isReadOnly: boolean): string { +export function getConfigPickerTriggerHover(property: string, schema: SessionConfigPropertySchema, value: unknown | undefined, isReadOnly: boolean, sandboxed = false): string { if (property === CodexSessionConfigKey.PermissionsPreset) { return getEnumValueDescription(schema, value) ?? schema.description ?? schema.title; } @@ -194,9 +198,12 @@ export function getConfigPickerTriggerHover(property: string, schema: SessionCon return schema.description ?? schema.title; } - const hover = getAutoApproveHover(value, getEnumValueDescription(schema, value)); + let hover = getAutoApproveHover(value, getEnumValueDescription(schema, value)); if (isReadOnly) { - return localize('agentHostChatInputPicker.approvalsLevelHoverReadOnly', "{0} Read-only.", hover); + hover = localize('agentHostChatInputPicker.approvalsLevelHoverReadOnly', "{0} Read-only.", hover); + } + if (sandboxed) { + hover = localize('agentHostChatInputPicker.approvalsLevelHoverSandboxed', "{0} Terminal commands are sandboxed.", hover); } return hover; } @@ -546,10 +553,9 @@ export class AgentHostChatInputPicker extends Disposable { const isReadOnly = !!ctx.schema.readOnly || (isStartedSession && ctx.schema.sessionMutable === false); const trigger = renderPickerTrigger(slot, isReadOnly, this._renderDisposables, () => this._showPicker(trigger)); this._trigger = trigger; - const tooltip = getConfigPickerTriggerHover(this._property, ctx.schema, ctx.value, isReadOnly); - if (tooltip) { - this._renderDisposables.add(this._hoverService.setupDelayedHover(trigger, { content: tooltip })); - } + this._renderDisposables.add(this._hoverService.setupDelayedHover(trigger, () => ({ + content: getConfigPickerTriggerHover(this._property, ctx.schema, ctx.value, isReadOnly, this._isSandboxed()) + }))); this._renderTrigger(trigger, ctx.schema, ctx.value, isReadOnly); } @@ -566,12 +572,19 @@ export class AgentHostChatInputPicker extends Disposable { trigger.classList.toggle('warning', value === 'autopilot' || value === 'assisted'); trigger.classList.toggle('info', value === 'autoApprove'); } - const label = this._labelFor(schema, value); + const label = getConfigPickerTriggerLabel(schema, value); const labelSpan = dom.append(trigger, dom.$('span.agent-host-chat-input-picker-label')); labelSpan.textContent = label; + const sandboxed = this._isSandboxed(); + if (sandboxed) { + const sandboxIcon = dom.append(trigger, renderIcon(Codicon.shield)); + sandboxIcon.classList.add('agent-host-chat-input-picker-sandbox-icon'); + sandboxIcon.ariaHidden = 'true'; + } + const accessibleLabel = getConfigPickerAccessibleTriggerLabel(label, sandboxed); trigger.setAttribute('aria-label', isReadOnly - ? localize('agentHostChatInputPicker.triggerAriaReadOnly', "{0}: {1}, Read-Only", schema.title, label) - : localize('agentHostChatInputPicker.triggerAria', "{0}: {1}", schema.title, label)); + ? localize('agentHostChatInputPicker.triggerAriaReadOnly', "{0}: {1}, Read-Only", schema.title, accessibleLabel) + : localize('agentHostChatInputPicker.triggerAria', "{0}: {1}", schema.title, accessibleLabel)); } private _refreshTrigger(): void { @@ -586,11 +599,10 @@ export class AgentHostChatInputPicker extends Disposable { this._renderTrigger(trigger, ctx.schema, ctx.value, isReadOnly); } - private _labelFor(schema: SessionConfigPropertySchema, value: unknown | undefined): string { - const sandboxed = this._property === SessionConfigKey.AutoApprove + private _isSandboxed(): boolean { + return this._property === SessionConfigKey.AutoApprove && this._isSandboxToggleSettingEnabled() && this._isSandboxingEnabled(); - return getConfigPickerTriggerLabel(schema, value, sandboxed); } private _readContext(): { backendSession: URI; schema: SessionConfigPropertySchema; value: unknown | undefined } | undefined { diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/media/agentHostChatInputPicker.css b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/media/agentHostChatInputPicker.css index 58e754bf22651..5689d7f3fb9c5 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/media/agentHostChatInputPicker.css +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/media/agentHostChatInputPicker.css @@ -95,6 +95,10 @@ display: none; } +.interactive-session .compact-picker .agent-host-chat-input-picker-sandbox-icon { + display: none; +} + .interactive-session .compact-picker .agent-host-chat-input-picker-slot .action-label { box-sizing: border-box; width: 22px; @@ -148,3 +152,8 @@ .agent-host-chat-input-picker-slot .action-label .codicon + .agent-host-chat-input-picker-label { margin-left: 6px; } + +.agent-host-chat-input-picker-sandbox-icon { + flex-shrink: 0; + margin-left: var(--vscode-spacing-size60); +} diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts index 91f614e7692f6..f770220f3c89c 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatInputPicker.test.ts @@ -9,7 +9,7 @@ import { ClaudeSessionConfigKey } from '../../../../../../platform/agentHost/com import { SessionConfigKey } from '../../../../../../platform/agentHost/common/sessionConfigKeys.js'; import { CodexSessionConfigKey } from '../../../../../../platform/agentHost/common/codexSessionConfigKeys.js'; import type { SessionConfigPropertySchema } from '../../../../../../platform/agentHost/common/state/protocol/commands.js'; -import { getAgentHostSandboxSettingId, getConfigPickerItemHover, getConfigPickerListOptions, getConfigPickerTriggerHover, getConfigPickerTriggerLabel, resolveConfigChipValue } from '../../../browser/agentSessions/agentHost/agentHostChatInputPicker.js'; +import { getAgentHostSandboxSettingId, getConfigPickerAccessibleTriggerLabel, getConfigPickerItemHover, getConfigPickerListOptions, getConfigPickerTriggerHover, getConfigPickerTriggerLabel, resolveConfigChipValue } from '../../../browser/agentSessions/agentHost/agentHostChatInputPicker.js'; import { AgentHostSdkSandboxEnabledSettingId, AgentHostSdkSandboxWindowsEnabledSettingId } from '../../../../../../platform/agentHost/common/agentService.js'; import { AgentSandboxSettingId } from '../../../../../../platform/sandbox/common/settings.js'; import { SessionType } from '../../../common/chatSessionsService.js'; @@ -87,23 +87,25 @@ suite('AgentHostChatInputPicker - trigger labels', () => { enumLabels: ['Default permissions', 'Assisted permissions', 'Allow all', 'Autopilot'], } as SessionConfigPropertySchema; - test('appends the sandbox state to every selected permission mode', () => { + test('uses an icon-ready label while preserving the sandbox state for accessibility', () => { assert.deepStrictEqual({ - default: getConfigPickerTriggerLabel(permissionsSchema, ChatPermissionLevel.Default, true), - assisted: getConfigPickerTriggerLabel(permissionsSchema, ChatPermissionLevel.Assisted, true), - allowAll: getConfigPickerTriggerLabel(permissionsSchema, ChatPermissionLevel.AutoApprove, true), - autopilot: getConfigPickerTriggerLabel(permissionsSchema, ChatPermissionLevel.Autopilot, true), + default: getConfigPickerTriggerLabel(permissionsSchema, ChatPermissionLevel.Default), + assisted: getConfigPickerTriggerLabel(permissionsSchema, ChatPermissionLevel.Assisted), + allowAll: getConfigPickerTriggerLabel(permissionsSchema, ChatPermissionLevel.AutoApprove), + autopilot: getConfigPickerTriggerLabel(permissionsSchema, ChatPermissionLevel.Autopilot), + accessible: getConfigPickerAccessibleTriggerLabel('Default permissions', true), }, { - default: 'Default permissions (sandboxed)', - assisted: 'Assisted permissions (sandboxed)', - allowAll: 'Allow all (sandboxed)', - autopilot: 'Autopilot (sandboxed)', + default: 'Default permissions', + assisted: 'Assisted permissions', + allowAll: 'Allow all', + autopilot: 'Autopilot', + accessible: 'Default permissions (sandboxed)', }); }); - test('leaves the selected permission label unchanged when sandboxing is disabled', () => { + test('leaves the accessible permission label unchanged when sandboxing is disabled', () => { assert.strictEqual( - getConfigPickerTriggerLabel(permissionsSchema, ChatPermissionLevel.Assisted, false), + getConfigPickerAccessibleTriggerLabel('Assisted permissions', false), 'Assisted permissions' ); }); @@ -172,10 +174,13 @@ suite('AgentHostChatInputPicker - resolveConfigChipValue', () => { } as SessionConfigPropertySchema; test('explains the selected approval level on the trigger hover', () => { - assert.strictEqual( - getConfigPickerTriggerHover(SessionConfigKey.AutoApprove, approvalsSchema, 'autoApprove', false), - 'Copilot runs all tools without asking for approval.' - ); + assert.deepStrictEqual({ + unsandboxed: getConfigPickerTriggerHover(SessionConfigKey.AutoApprove, approvalsSchema, 'autoApprove', false), + sandboxed: getConfigPickerTriggerHover(SessionConfigKey.AutoApprove, approvalsSchema, 'autoApprove', false, true), + }, { + unsandboxed: 'Copilot runs all tools without asking for approval.', + sandboxed: 'Copilot runs all tools without asking for approval. Terminal commands are sandboxed.', + }); }); test('explains approval choices on item hover', () => { From ca9977749d9b7eedfee8231eef088e2b0216c7ae Mon Sep 17 00:00:00 2001 From: Anthony Kim <62267334+anthonykim1@users.noreply.github.com> Date: Thu, 3 Sep 2026 01:00:04 -0700 Subject: [PATCH 04/13] Add telemetry for terminal profile settings (#334180) * Add terminal profile setting change telemetry Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f4bbe757-733f-4851-a490-95b1ba98e743 * Reuse terminal telemetry contribution Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f4bbe757-733f-4851-a490-95b1ba98e743 * Filter terminal profile telemetry by effective changes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f4bbe757-733f-4851-a490-95b1ba98e743 * Report configured terminal profile settings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f4bbe757-733f-4851-a490-95b1ba98e743 --------- Copilot-Session: f4bbe757-733f-4851-a490-95b1ba98e743 --- .../terminalProfileConfigurationTelemetry.ts | 118 ++++++++++++++++ .../telemetry/browser/terminalTelemetry.ts | 5 + ...minalProfileConfigurationTelemetry.test.ts | 132 ++++++++++++++++++ 3 files changed, 255 insertions(+) create mode 100644 src/vs/workbench/contrib/terminalContrib/telemetry/browser/terminalProfileConfigurationTelemetry.ts create mode 100644 src/vs/workbench/contrib/terminalContrib/telemetry/test/browser/terminalProfileConfigurationTelemetry.test.ts diff --git a/src/vs/workbench/contrib/terminalContrib/telemetry/browser/terminalProfileConfigurationTelemetry.ts b/src/vs/workbench/contrib/terminalContrib/telemetry/browser/terminalProfileConfigurationTelemetry.ts new file mode 100644 index 0000000000000..6da1ed39ca427 --- /dev/null +++ b/src/vs/workbench/contrib/terminalContrib/telemetry/browser/terminalProfileConfigurationTelemetry.ts @@ -0,0 +1,118 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Disposable } from '../../../../../base/common/lifecycle.js'; +import { IConfigurationService, ConfigurationTargetToString } from '../../../../../platform/configuration/common/configuration.js'; +import { ITelemetryService } from '../../../../../platform/telemetry/common/telemetry.js'; +import { TerminalSettingId } from '../../../../../platform/terminal/common/terminal.js'; +import { TerminalChatAgentToolsSettingId } from '../../chatAgentTools/common/terminalChatAgentToolsConfiguration.js'; + +const terminalProfileSettings = [ + { settingId: TerminalChatAgentToolsSettingId.TerminalProfileLinux, profileType: 'chat', os: 'linux' }, + { settingId: TerminalChatAgentToolsSettingId.TerminalProfileMacOs, profileType: 'chat', os: 'osx' }, + { settingId: TerminalChatAgentToolsSettingId.TerminalProfileWindows, profileType: 'chat', os: 'windows' }, + { settingId: TerminalSettingId.AutomationProfileLinux, profileType: 'automation', os: 'linux' }, + { settingId: TerminalSettingId.AutomationProfileMacOs, profileType: 'automation', os: 'osx' }, + { settingId: TerminalSettingId.AutomationProfileWindows, profileType: 'automation', os: 'windows' }, + { settingId: TerminalSettingId.DefaultProfileLinux, profileType: 'default', os: 'linux' }, + { settingId: TerminalSettingId.DefaultProfileMacOs, profileType: 'default', os: 'osx' }, + { settingId: TerminalSettingId.DefaultProfileWindows, profileType: 'default', os: 'windows' }, +] as const; + +type TerminalProfileSetting = typeof terminalProfileSettings[number]; + +export class TerminalProfileConfigurationTelemetry extends Disposable { + private readonly _configuredSettings = new Map(); + + constructor( + @IConfigurationService configurationService: IConfigurationService, + @ITelemetryService telemetryService: ITelemetryService, + ) { + super(); + + for (const setting of terminalProfileSettings) { + const configured = this._isConfigured(configurationService, setting.settingId); + this._configuredSettings.set(setting.settingId, configured); + if (configured) { + this._reportSettingState(telemetryService, setting); + } + } + + this._register(configurationService.onDidChangeConfiguration(event => { + for (const setting of terminalProfileSettings) { + if (!event.affectsConfiguration(setting.settingId, {})) { + continue; + } + + this._reportSettingChanged(configurationService, telemetryService, setting, ConfigurationTargetToString(event.source) ?? 'UNKNOWN'); + } + })); + } + + private _reportSettingState(telemetryService: ITelemetryService, setting: TerminalProfileSetting): void { + type TerminalProfileSettingStateEvent = { + settingId: TerminalProfileSetting['settingId']; + profileType: TerminalProfileSetting['profileType']; + os: TerminalProfileSetting['os']; + configured: true; + }; + type TerminalProfileSettingStateClassification = { + owner: 'anthonykim1'; + comment: 'Tracks terminal profile settings that already have a configured value without collecting profile names, paths, arguments, environment variables, or other profile contents.'; + settingId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'The terminal profile setting that has a configured value.' }; + profileType: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Whether the configured setting controls chat, automation, or default terminals.' }; + os: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'The operating system targeted by the configured setting.' }; + configured: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Whether the setting has a configured value.' }; + }; + + telemetryService.publicLog2('terminal/profileSettingState', { + ...setting, + configured: true, + }); + } + + private _reportSettingChanged( + configurationService: IConfigurationService, + telemetryService: ITelemetryService, + setting: TerminalProfileSetting, + source: string, + ): void { + type TerminalProfileSettingChangedEvent = { + settingId: TerminalProfileSetting['settingId']; + profileType: TerminalProfileSetting['profileType']; + os: TerminalProfileSetting['os']; + configured: boolean; + changeType: 'added' | 'changed' | 'removed'; + source: string; + }; + type TerminalProfileSettingChangedClassification = { + owner: 'anthonykim1'; + comment: 'Tracks changes to terminal profile settings without collecting profile names, paths, arguments, environment variables, or other profile contents.'; + settingId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'The terminal profile setting that changed.' }; + profileType: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Whether the changed setting controls chat, automation, or default terminals.' }; + os: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'The operating system targeted by the changed setting.' }; + configured: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Whether the setting resolves to a configured profile after the change.' }; + changeType: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Whether the effective profile setting was added, changed, or removed.' }; + source: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'The broad configuration change source reported by the configuration service.' }; + }; + + const configured = this._isConfigured(configurationService, setting.settingId); + const wasConfigured = this._configuredSettings.get(setting.settingId) ?? false; + const changeType = configured ? (wasConfigured ? 'changed' : 'added') : (wasConfigured ? 'removed' : 'changed'); + this._configuredSettings.set(setting.settingId, configured); + + telemetryService.publicLog2('terminal/profileSettingChanged', { + ...setting, + configured, + changeType, + source, + }); + } + + private _isConfigured(configurationService: IConfigurationService, settingId: TerminalProfileSetting['settingId']): boolean { + const value = configurationService.getValue(settingId); + return value !== null && value !== undefined; + } +} diff --git a/src/vs/workbench/contrib/terminalContrib/telemetry/browser/terminalTelemetry.ts b/src/vs/workbench/contrib/terminalContrib/telemetry/browser/terminalTelemetry.ts index f6570068e6336..e7df46ada48a1 100644 --- a/src/vs/workbench/contrib/terminalContrib/telemetry/browser/terminalTelemetry.ts +++ b/src/vs/workbench/contrib/terminalContrib/telemetry/browser/terminalTelemetry.ts @@ -14,9 +14,11 @@ import { ITelemetryService } from '../../../../../platform/telemetry/common/tele import { TelemetryTrustedValue } from '../../../../../platform/telemetry/common/telemetryUtils.js'; import { TerminalCapability } from '../../../../../platform/terminal/common/capabilities/capabilities.js'; import { TerminalLocation, type IShellLaunchConfig, type ShellIntegrationInjectionFailureReason } from '../../../../../platform/terminal/common/terminal.js'; +import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import type { IWorkbenchContribution } from '../../../../common/contributions.js'; import { ILifecycleService } from '../../../../services/lifecycle/common/lifecycle.js'; import { ITerminalEditorService, ITerminalService, type ITerminalInstance } from '../../../terminal/browser/terminal.js'; +import { TerminalProfileConfigurationTelemetry } from './terminalProfileConfigurationTelemetry.js'; export class TerminalTelemetryContribution extends Disposable implements IWorkbenchContribution { static ID = 'terminalTelemetry'; @@ -25,10 +27,13 @@ export class TerminalTelemetryContribution extends Disposable implements IWorkbe @ILifecycleService lifecycleService: ILifecycleService, @ITerminalService terminalService: ITerminalService, @ITerminalEditorService terminalEditorService: ITerminalEditorService, + @IConfigurationService configurationService: IConfigurationService, @ITelemetryService private readonly _telemetryService: ITelemetryService, ) { super(); + this._register(new TerminalProfileConfigurationTelemetry(configurationService, this._telemetryService)); + this._register(terminalService.onDidCreateInstance(async instance => { const store = new DisposableStore(); this._store.add(store); diff --git a/src/vs/workbench/contrib/terminalContrib/telemetry/test/browser/terminalProfileConfigurationTelemetry.test.ts b/src/vs/workbench/contrib/terminalContrib/telemetry/test/browser/terminalProfileConfigurationTelemetry.test.ts new file mode 100644 index 0000000000000..6afb8f633d1fa --- /dev/null +++ b/src/vs/workbench/contrib/terminalContrib/telemetry/test/browser/terminalProfileConfigurationTelemetry.test.ts @@ -0,0 +1,132 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; +import { ConfigurationTarget } from '../../../../../../platform/configuration/common/configuration.js'; +import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { NullTelemetryServiceShape } from '../../../../../../platform/telemetry/common/telemetryUtils.js'; +import { TerminalSettingId } from '../../../../../../platform/terminal/common/terminal.js'; +import { TerminalChatAgentToolsSettingId } from '../../../chatAgentTools/common/terminalChatAgentToolsConfiguration.js'; +import { TerminalProfileConfigurationTelemetry } from '../../browser/terminalProfileConfigurationTelemetry.js'; + +class TestTelemetryService extends NullTelemetryServiceShape { + readonly events: { readonly name: string; readonly data: unknown }[] = []; + + override publicLog2(eventName?: string, data?: unknown): void { + if (eventName) { + this.events.push({ name: eventName, data }); + } + } +} + +const profileSettings = [ + { settingId: TerminalChatAgentToolsSettingId.TerminalProfileLinux, profileType: 'chat', os: 'linux', value: { path: '/bin/bash' } }, + { settingId: TerminalChatAgentToolsSettingId.TerminalProfileMacOs, profileType: 'chat', os: 'osx', value: { path: '/bin/zsh' } }, + { settingId: TerminalChatAgentToolsSettingId.TerminalProfileWindows, profileType: 'chat', os: 'windows', value: { path: 'pwsh.exe' } }, + { settingId: TerminalSettingId.AutomationProfileLinux, profileType: 'automation', os: 'linux', value: { path: '/bin/bash' } }, + { settingId: TerminalSettingId.AutomationProfileMacOs, profileType: 'automation', os: 'osx', value: { path: '/bin/zsh' } }, + { settingId: TerminalSettingId.AutomationProfileWindows, profileType: 'automation', os: 'windows', value: { path: 'pwsh.exe' } }, + { settingId: TerminalSettingId.DefaultProfileLinux, profileType: 'default', os: 'linux', value: 'bash' }, + { settingId: TerminalSettingId.DefaultProfileMacOs, profileType: 'default', os: 'osx', value: 'zsh' }, + { settingId: TerminalSettingId.DefaultProfileWindows, profileType: 'default', os: 'windows', value: 'PowerShell' }, +] as const; + +suite('TerminalProfileConfigurationTelemetry', () => { + const store = ensureNoDisposablesAreLeakedInTestSuite(); + + let configurationService: TestConfigurationService; + let telemetryService: TestTelemetryService; + + setup(() => { + configurationService = new TestConfigurationService(); + telemetryService = new TestTelemetryService(); + store.add(new TerminalProfileConfigurationTelemetry(configurationService, telemetryService)); + }); + + async function changeSetting(settingId: string, value: unknown, source = ConfigurationTarget.USER): Promise { + await configurationService.setUserConfiguration(settingId, value); + configurationService.onDidChangeConfigurationEmitter.fire({ + affectsConfiguration: (key, overrides) => key === settingId && overrides !== undefined, + affectedKeys: new Set([settingId]), + change: { keys: [settingId], overrides: [] }, + source, + }); + } + + test('reports changes for each terminal profile setting without profile contents', async () => { + for (const setting of profileSettings) { + await changeSetting(setting.settingId, setting.value); + } + + assert.deepStrictEqual(telemetryService.events, profileSettings.map(({ value, ...setting }) => ({ + name: 'terminal/profileSettingChanged', + data: { + ...setting, + configured: true, + changeType: 'added', + source: 'USER', + }, + }))); + }); + + test('reports settings that already have configured values', () => { + const configuredSettings = [profileSettings[0], profileSettings[7]]; + const configurationService = new TestConfigurationService(Object.fromEntries(configuredSettings.map(setting => [setting.settingId, setting.value]))); + const telemetryService = new TestTelemetryService(); + store.add(new TerminalProfileConfigurationTelemetry(configurationService, telemetryService)); + + assert.deepStrictEqual(telemetryService.events, configuredSettings.map(({ value, ...setting }) => ({ + name: 'terminal/profileSettingState', + data: { + ...setting, + configured: true, + }, + }))); + }); + + test('reports added, changed, and removed settings and ignores unrelated settings', async () => { + await changeSetting(TerminalSettingId.DefaultProfileLinux, 'bash'); + await changeSetting(TerminalSettingId.DefaultProfileLinux, 'zsh', ConfigurationTarget.WORKSPACE); + await changeSetting(TerminalSettingId.DefaultProfileLinux, null); + await changeSetting(TerminalSettingId.FontSize, 16); + + assert.deepStrictEqual(telemetryService.events, [ + { + name: 'terminal/profileSettingChanged', + data: { + settingId: TerminalSettingId.DefaultProfileLinux, + profileType: 'default', + os: 'linux', + configured: true, + changeType: 'added', + source: 'USER', + }, + }, + { + name: 'terminal/profileSettingChanged', + data: { + settingId: TerminalSettingId.DefaultProfileLinux, + profileType: 'default', + os: 'linux', + configured: true, + changeType: 'changed', + source: 'WORKSPACE', + }, + }, + { + name: 'terminal/profileSettingChanged', + data: { + settingId: TerminalSettingId.DefaultProfileLinux, + profileType: 'default', + os: 'linux', + configured: false, + changeType: 'removed', + source: 'USER', + }, + }, + ]); + }); +}); From cc293c6ea513f0c68132c4007028d5b03b69beb2 Mon Sep 17 00:00:00 2001 From: Martin Aeschlimann Date: Thu, 3 Sep 2026 10:00:45 +0200 Subject: [PATCH 05/13] test: ensure cancellation does not cache partially completed agent discovery (#334039) * test: ensure cancellation does not cache partially completed agent discovery * update --- .../service/promptsServiceImpl.ts | 10 ++-- .../service/promptsService.test.ts | 46 +++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts index 16fe3468b8d9d..2399b9c330eba 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts @@ -793,10 +793,13 @@ export class PromptsService extends Disposable implements IPromptsService { const skipReason = isEnabled ? undefined : 'disabled'; return { status, skipReason, promptPath: this.withPromptPathMetadata(promptPath, agent.name, agent.description), agent }; } catch (e) { + if (isCancellationError(e)) { + throw e; + } const error = e instanceof Error ? e : new Error(String(e)); if (error instanceof FileOperationError && error.fileOperationResult === FileOperationResult.FILE_NOT_FOUND) { this.logger.warn(`[computeAgentDiscoveryInfo] Skipping agent file that does not exist: ${uri}`, error.message); - } else if (!isCancellationError(e)) { + } else { this.logger.error(`[computeAgentDiscoveryInfo] Failed to parse agent file: ${uri}`, error); } return { @@ -1631,12 +1634,13 @@ class CachedPromise extends Disposable { throw err; }); // The pool is only meaningful while the computation is in flight. - promise.finally(() => { + const disposePool = () => { if (this.cachedPool === pool) { this.cachedPool = undefined; } pool!.dispose(); - }); + }; + promise.then(disposePool, disposePool); this.cachedPromise = promise; this.cachedPool = pool; } diff --git a/src/vs/workbench/contrib/chat/test/common/promptSyntax/service/promptsService.test.ts b/src/vs/workbench/contrib/chat/test/common/promptSyntax/service/promptsService.test.ts index bf9c544c58861..bb8a065010bd0 100644 --- a/src/vs/workbench/contrib/chat/test/common/promptSyntax/service/promptsService.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/promptSyntax/service/promptsService.test.ts @@ -999,6 +999,52 @@ suite('PromptsService', () => { ); }); + test('does not cache partially completed canceled agent discovery', async () => { + const rootFolder = '/custom-agents-cancellation'; + const rootFolderUri = URI.file(rootFolder); + const firstAgent = URI.joinPath(rootFolderUri, '.github/agents/agent1.agent.md'); + const secondAgent = URI.joinPath(rootFolderUri, '.github/agents/agent2.agent.md'); + + workspaceContextService.setWorkspace(testWorkspace(rootFolderUri)); + await mockFiles(fileService, [ + { path: firstAgent.path, contents: ['---', 'description: First agent.', '---'] }, + { path: secondAgent.path, contents: ['---', 'description: Second agent.', '---'] }, + ]); + + const firstReadCompleted = new DeferredPromise(); + const secondReadStarted = new DeferredPromise(); + const releaseSecondRead = new DeferredPromise(); + const readFile = fileService.readFile.bind(fileService); + const readFileStub = sinon.stub(fileService, 'readFile').callsFake(async (resource: URI, options?: IReadFileOptions, token?: CancellationToken): Promise => { + if (resource.toString() === firstAgent.toString()) { + const result = await readFile(resource, options, token); + firstReadCompleted.complete(); + return result; + } + if (resource.toString() === secondAgent.toString()) { + secondReadStarted.complete(); + await releaseSecondRead.p; + } + return readFile(resource, options, token); + }); + + const cancellationTokenSource = disposables.add(new CancellationTokenSource()); + const canceledDiscovery = service.getCustomAgents(cancellationTokenSource.token); + await Promise.all([firstReadCompleted.p, secondReadStarted.p]); + cancellationTokenSource.cancel(); + await assert.rejects(canceledDiscovery, CancellationError); + releaseSecondRead.complete(); + await timeout(0); + readFileStub.restore(); + + const agents = await service.getCustomAgents(CancellationToken.None); + + assert.deepStrictEqual( + agents.map(agent => agent.name).sort(), + ['agent1', 'agent2'], + ); + }); + test('header with handOffs', async () => { const rootFolderName = 'custom-agents-with-handoffs'; From 9e75f50f3708e19fe8aa381ecd9079bd0b2ee396 Mon Sep 17 00:00:00 2001 From: Christof Marti Date: Thu, 3 Sep 2026 10:01:41 +0200 Subject: [PATCH 06/13] Add certificate loading duration telemetry (#334188) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/vs/workbench/api/node/proxyResolver.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/api/node/proxyResolver.ts b/src/vs/workbench/api/node/proxyResolver.ts index 307d542efbf3b..8e5e42072a701 100644 --- a/src/vs/workbench/api/node/proxyResolver.ts +++ b/src/vs/workbench/api/node/proxyResolver.ts @@ -102,6 +102,7 @@ export function connectProxyResolver( return intervalSeconds * 1000; }, loadAdditionalCertificates: async () => { + const start = Date.now(); const useNodeSystemCerts = getExtHostConfigValue(configProvider, isRemote, 'http.systemCertificatesNode', systemCertificatesNodeDefault); const promises: Promise[] = []; if (isRemote) { @@ -126,6 +127,7 @@ export function connectProxyResolver( const result = (await Promise.all(promises)).flat(); mainThreadTelemetry.$publicLog2('additionalCertificates', { count: result.length, + duration: Date.now() - start, isRemote, loadLocalCertificates, useNodeSystemCerts, @@ -299,8 +301,9 @@ function recordFetchFeatureUse(mainThreadTelemetry: MainThreadTelemetryShape, fe type AdditionalCertificatesClassification = { owner: 'chrmarti'; - comment: 'Tracks the number of additional certificates loaded for TLS connections'; + comment: 'Tracks loading additional certificates for TLS connections'; count: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; isMeasurement: true; comment: 'Number of additional certificates loaded' }; + duration: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'Time in milliseconds spent loading additional certificates' }; isRemote: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; isMeasurement: true; comment: 'Whether this is a remote extension host' }; loadLocalCertificates: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; isMeasurement: true; comment: 'Whether local certificates are loaded' }; useNodeSystemCerts: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; isMeasurement: true; comment: 'Whether Node.js system certificates are used' }; @@ -308,6 +311,7 @@ type AdditionalCertificatesClassification = { type AdditionalCertificatesEvent = { count: number; + duration: number; isRemote: boolean; loadLocalCertificates: boolean; useNodeSystemCerts: boolean; From fd97f530dbf6d5c554391d384052ad301138b8f1 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Thu, 3 Sep 2026 10:29:32 +0200 Subject: [PATCH 07/13] sessions: refine grouped resource label homes (#334197) * sessions: address resource label review Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * label: simplify session home templates Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * label: restore formatter documentation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * label: simplify template parameter matching Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/baseAgentHostSessionsProvider.ts | 36 ++++++++++--- .../browser/localAgentHostSessionsProvider.ts | 2 +- .../localAgentHostSessionsProvider.test.ts | 51 +++++++++++++++++++ .../remoteAgentHostSessionsProvider.ts | 2 +- .../services/label/common/labelService.ts | 36 ++++--------- .../services/label/test/browser/label.test.ts | 39 ++++++++++++++ .../label/test/common/mockLabelService.ts | 43 ++++++++-------- 7 files changed, 152 insertions(+), 57 deletions(-) diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 4142bbb7402b1..a66d2f5f901d4 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -10,6 +10,7 @@ import { arrayEquals, structuralEquals } from '../../../../../base/common/equals import { Emitter, Event } from '../../../../../base/common/event.js'; import { IMarkdownString, MarkdownString, markdownStringEqual } from '../../../../../base/common/htmlContent.js'; import { Disposable, DisposableMap, DisposableStore, IDisposable, IReference, MutableDisposable, toDisposable } from '../../../../../base/common/lifecycle.js'; +import { mapsStrictEqualIgnoreOrder } from '../../../../../base/common/map.js'; import { equals } from '../../../../../base/common/objects.js'; import { constObservable, derived, derivedOpts, IObservable, IReader, ISettableObservable, ITransaction, observableFromEvent, observableValueOpts, subtransaction, transaction, waitForState, autorun, observableValue } from '../../../../../base/common/observable.js'; import { basename, dirname, getComparisonKey, isEqual, isEqualOrParent, joinPath, relativePath } from '../../../../../base/common/resources.js'; @@ -2605,6 +2606,10 @@ class NewSession extends Disposable { * URI-scheme mapping for session metadata, the agent-provider lookup, and * the browse UI. */ +function escapeResourceLabelPathSeparators(label: string): string { + return label.replaceAll('/', '\u2215').replaceAll('\\', '\u29F5'); +} + export abstract class BaseAgentHostSessionsProvider extends Disposable implements IAgentHostSessionsProvider { abstract readonly id: string; @@ -3287,8 +3292,9 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement private readonly _resourceLabelHomeRegistrations = this._register(new DisposableMap()); private readonly _resourceLabelHomeLabels = new Map>(); + private readonly _resourceLabelHomeFormattingEvents = this._register(new DisposableMap>()); - protected updateResourceLabelHomeFormatters(homes: readonly { readonly uri: URI; readonly label: string }[], labelService: ILabelService, onDidChange: Event): void { + protected updateResourceLabelHomeFormatters(homes: readonly { readonly uri: URI; readonly label: string }[], labelService: ILabelService): void { const groups = new Map }>(); for (const home of homes) { const parent = dirname(home.uri); @@ -3301,10 +3307,20 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement group.labels.set(basename(home.uri), home.label); } - this._resourceLabelHomeLabels.clear(); const registrationKeys = new Set(); + const removedGroupKeys = new Set(this._resourceLabelHomeLabels.keys()); + const changedFormattingEvents: Emitter[] = []; for (const [key, group] of groups) { + removedGroupKeys.delete(key); + const previousLabels = this._resourceLabelHomeLabels.get(key); this._resourceLabelHomeLabels.set(key, group.labels); + let formattingEvent = this._resourceLabelHomeFormattingEvents.get(key); + if (!formattingEvent) { + formattingEvent = new Emitter(); + this._resourceLabelHomeFormattingEvents.set(key, formattingEvent); + } else if (previousLabels && !mapsStrictEqualIgnoreOrder(previousLabels, group.labels)) { + changedFormattingEvents.push(formattingEvent); + } const separator = labelService.getSeparator(group.parent.scheme, group.parent.authority); const templateKey = `template:${key}`; if (group.labels.size > (group.labels.has('') ? 1 : 0)) { @@ -3312,7 +3328,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement if (!this._resourceLabelHomeRegistrations.has(templateKey)) { this._resourceLabelHomeRegistrations.set(templateKey, labelService.registerFormatter({ home: joinPath(group.parent, '${sessionId}'), - onDidChangeFormatting: onDidChange, + onDidChangeFormatting: formattingEvent.event, formatting: context => { const label = this._resourceLabelHomeLabels.get(key)?.get(context.parameters.get('sessionId') ?? ''); return label === undefined ? undefined : { label, separator }; @@ -3326,7 +3342,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement if (!this._resourceLabelHomeRegistrations.has(rootKey)) { this._resourceLabelHomeRegistrations.set(rootKey, labelService.registerFormatter({ home: group.parent, - onDidChangeFormatting: onDidChange, + onDidChangeFormatting: formattingEvent.event, formatting: () => { const label = this._resourceLabelHomeLabels.get(key)?.get(''); return label === undefined ? undefined : { label, separator }; @@ -3340,11 +3356,19 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement this._resourceLabelHomeRegistrations.deleteAndDispose(key); } } + for (const key of removedGroupKeys) { + this._resourceLabelHomeLabels.delete(key); + this._resourceLabelHomeFormattingEvents.deleteAndDispose(key); + } + for (const formattingEvent of changedFormattingEvents) { + formattingEvent.fire(); + } } protected getResourceLabelHomeLabel(session: ISession): string { - const providerLabel = this.sessionTypes.find(type => type.id === session.sessionType)?.label ?? session.sessionType; - return `${providerLabel}/${session.title.get() || localize('sessionHome', "Session")}`; + const providerLabel = escapeResourceLabelPathSeparators(this.sessionTypes.find(type => type.id === session.sessionType)?.label ?? session.sessionType); + const sessionLabel = escapeResourceLabelPathSeparators(session.title.get() || localize('sessionHome', "Session")); + return `${providerLabel}/${sessionLabel}`; } protected getKnownSessions(): ISession[] { diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts index 7cb582aa33e0c..8631d5c598acb 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts @@ -236,7 +236,7 @@ export class LocalAgentHostSessionsProvider extends BaseAgentHostSessionsProvide } } - this.updateResourceLabelHomeFormatters(homes, this._labelService, onDidChangeResourceLabelHomes); + this.updateResourceLabelHomeFormatters(homes, this._labelService); }; this._register(onDidChangeResourceLabelHomes(updateResourceLabelHomes)); updateResourceLabelHomes(); diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts index a14d279a436da..56ec50ca917ca 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts @@ -5960,15 +5960,41 @@ suite('LocalAgentHostSessionsProvider', () => { }; assert.deepStrictEqual({ + formatterCount: labelService.formatterCount, quickChat: getHomeLabel(URI.joinPath(claudeHome, 'artifact.md')), root: getHomeLabel(URI.file('/artifact.md')), copilotState: getHomeLabel(URI.file('/home/test/.copilot/session-state/copilot-session/artifact.md')), }, { + formatterCount: 4, quickChat: 'claude/Claude Quick Chat', root: 'claude/Root Quick Chat', copilotState: 'Copilot/Copilot Session', }); + let formatterChanges = 0; + disposables.add(labelService.onDidChangeFormatters(() => formatterChanges++)); + const copilotSession = AgentSession.uri('copilotcli', 'copilot-session').toString(); + agentHost.fireAction({ + channel: copilotSession, + action: { type: ActionType.SessionIsReadChanged, isRead: true }, + serverSeq: 1, + origin: undefined, + } as ActionEnvelope); + agentHost.fireAction({ + channel: copilotSession, + action: { type: ActionType.SessionTitleChanged, title: 'Renamed/Session\\Title' }, + serverSeq: 2, + origin: undefined, + } as ActionEnvelope); + + assert.deepStrictEqual({ + formatterChanges, + copilotState: getHomeLabel(URI.file('/home/test/.copilot/session-state/copilot-session/artifact.md')), + }, { + formatterChanges: 1, + copilotState: 'Copilot/Renamed\u2215Session\u29F5Title', + }); + provider.dispose(); assert.deepStrictEqual({ quickChat: labelService.getUriHome(URI.joinPath(claudeHome, 'artifact.md')), @@ -5979,6 +6005,31 @@ suite('LocalAgentHostSessionsProvider', () => { }); })); + test('shares one resource label formatter across session state homes', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + for (let index = 0; index < 100; index++) { + agentHost.addSession(createSession(`session-${index}`, { summary: `Session ${index}` })); + } + const labelService = new MockLabelService(); + const provider = createProvider(disposables, agentHost, undefined, { + pathService: new TestPathService(URI.file('/home/test')), + labelService, + }); + provider.getSessions(); + await timeout(0); + const resource = URI.file('/home/test/.copilot/session-state/session-42/artifact.md'); + const home = labelService.getUriHome(resource); + + assert.deepStrictEqual({ + formatterCount: labelService.formatterCount, + home: home?.toString(), + label: home ? labelService.getUriLabel(home) : undefined, + }, { + formatterCount: 1, + home: URI.file('/home/test/.copilot/session-state/session-42').toString(), + label: 'Copilot/Session 42', + }); + })); + test('registers the session state home before a quick chat is materialized', () => { const pathService = new TestPathService(URI.file('/home/test')); const labelService = new MockLabelService(); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts index fea5926f42262..5cb5124e96923 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts @@ -671,7 +671,7 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid } } } - this.updateResourceLabelHomeFormatters(homes, this._labelService, this._onDidChangeResourceLabelHomes); + this.updateResourceLabelHomeFormatters(homes, this._labelService); } /** diff --git a/src/vs/workbench/services/label/common/labelService.ts b/src/vs/workbench/services/label/common/labelService.ts index e04f685ffc537..ed523ee15c987 100644 --- a/src/vs/workbench/services/label/common/labelService.ts +++ b/src/vs/workbench/services/label/common/labelService.ts @@ -133,7 +133,6 @@ interface IStoredFormatters { interface IHomeFormatterRegistration { readonly formatter: ResourceLabelTemplateFormatter; readonly templateMatcher: RegExp; - readonly templateParameterNames: readonly string[]; } interface IResolvedHomeFormatter { @@ -218,10 +217,7 @@ export class LabelService extends Disposable implements ILabelService { if (!templateMatch) { continue; } - const parameters = new Map(); - for (const parameterName of registration.templateParameterNames) { - parameters.set(parameterName, templateMatch.groups?.[parameterName] ?? ''); - } + const parameters = new Map(Object.entries(templateMatch.groups ?? {})); const home = resource.with({ path: templateMatch[0], query: null, fragment: null }); const formatting = formatter.formatting({ resource, home, parameters }); if (!formatting) { @@ -562,29 +558,17 @@ export class LabelService extends Disposable implements ILabelService { private createTemplateFormatterRegistration(formatter: ResourceLabelTemplateFormatter): IHomeFormatterRegistration { const { home } = formatter; - const parameterNames: string[] = []; - const seenParameterNames = new Set(); - const patternSegments = home.path.split('/'); - const matcherSegments = patternSegments.map(segment => { - const parameterMatch = homeTemplateParameterRegex.exec(segment); - if (parameterMatch?.groups?.name) { - const parameterName = parameterMatch.groups.name; - if (seenParameterNames.has(parameterName)) { - throw new Error(`Duplicate resource label home template parameter: ${parameterName}`); - } - seenParameterNames.add(parameterName); - parameterNames.push(parameterName); - return `(?<${parameterName}>(?!\\.{1,2}(?:/|$))[^/]+)`; - } - if (segment.includes('${')) { - throw new Error(`Resource label home template parameters must occupy an entire path segment: ${segment}`); - } - return escapeRegExpCharacters(segment); - }); + const homePath = home.path.length > 1 ? home.path.replace(/\/+$/, '') : home.path; + const lastSeparator = homePath.lastIndexOf('/'); + const parameterMatch = homeTemplateParameterRegex.exec(homePath.slice(lastSeparator + 1)); + const pathSegmentParameter = parameterMatch?.groups?.name; + const matcherPattern = pathSegmentParameter + ? `${escapeRegExpCharacters(homePath.slice(0, lastSeparator + 1))}(?<${pathSegmentParameter}>(?!\\.{1,2}(?:/|$))[^/]+)` + : escapeRegExpCharacters(homePath); + const isRootHome = pathSegmentParameter === undefined && (homePath === '' || homePath === '/'); return { formatter, - templateMatcher: new RegExp(`^${matcherSegments.join('/')}${home.path === '' || home.path === '/' ? '' : '(?=/|$)'}`), - templateParameterNames: parameterNames, + templateMatcher: new RegExp(`^${matcherPattern}${isRootHome ? '' : '(?=/|$)'}`), }; } diff --git a/src/vs/workbench/services/label/test/browser/label.test.ts b/src/vs/workbench/services/label/test/browser/label.test.ts index 615a64ccf31c3..a3c192fa8cff9 100644 --- a/src/vs/workbench/services/label/test/browser/label.test.ts +++ b/src/vs/workbench/services/label/test/browser/label.test.ts @@ -277,6 +277,45 @@ suite('URI Label', () => { registration.dispose(); }); + test('URI home template parents are matched literally', () => { + const registration = labelService.registerFormatter({ + home: URI.parse('test://current/sessions/${literal}/${sessionId}'), + onDidChangeFormatting: Event.None, + formatting: () => ({ label: 'Session', separator: '/' }), + }); + const resource = URI.parse('test://current/sessions/${literal}/session-id/file.md'); + + assert.deepStrictEqual({ + home: labelService.getUriHome(resource)?.path, + label: labelService.getUriLabel(resource), + unrelatedHome: labelService.getUriHome(URI.parse('test://current/sessions/other/session-id/file.md')), + }, { + home: '/sessions/${literal}/session-id', + label: 'Session/file.md', + unrelatedHome: undefined, + }); + + registration.dispose(); + }); + + test('URI home templates support trailing separators', () => { + const registration = labelService.registerFormatter({ + home: URI.parse('test://current/sessions/${sessionId}/'), + onDidChangeFormatting: Event.None, + formatting: () => ({ label: 'Session', separator: '/' }), + }); + + assert.deepStrictEqual({ + exact: labelService.getUriLabel(URI.parse('test://current/sessions/session-id')), + descendant: labelService.getUriLabel(URI.parse('test://current/sessions/session-id/file.md')), + }, { + exact: 'Session', + descendant: 'Session/file.md', + }); + + registration.dispose(); + }); + test('equally specific URI home templates use registration order', () => { const formatter = { home: URI.parse('test://current/sessions/${sessionId}'), diff --git a/src/vs/workbench/services/label/test/common/mockLabelService.ts b/src/vs/workbench/services/label/test/common/mockLabelService.ts index 33dc35750c8e9..32845dfd73290 100644 --- a/src/vs/workbench/services/label/test/common/mockLabelService.ts +++ b/src/vs/workbench/services/label/test/common/mockLabelService.ts @@ -53,14 +53,20 @@ export class MockLabelService implements ILabelService { this.formatters.push(formatter); const scheme = isTemplateFormatter(formatter) ? formatter.home.scheme : formatter.scheme; this._onDidChangeFormatters.fire({ scheme }); + const changeListener = isTemplateFormatter(formatter) ? formatter.onDidChangeFormatting(() => this._onDidChangeFormatters.fire({ scheme })) : undefined; return { dispose: () => { + changeListener?.dispose(); this.formatters = this.formatters.filter(candidate => candidate !== formatter); this._onDidChangeFormatters.fire({ scheme }); } }; } + get formatterCount(): number { + return this.formatters.length; + } + getUriHome(resource: URI): URI | undefined { const formatter = this.findHomeFormatter(resource); return formatter?.home; @@ -78,40 +84,31 @@ export class MockLabelService implements ILabelService { (formatter.home.authority && formatter.home.authority.toLowerCase() !== resource.authority.toLowerCase())) { continue; } - const templateSegments = formatter.home.path.split('/'); - const resourceSegments = resource.path.split('/'); - if (!templateSegments.some(segment => homeTemplateParameterRegex.test(segment))) { - if (!isEqualOrParent(resource, resource.with({ path: formatter.home.path }))) { + const homePath = formatter.home.path.length > 1 ? formatter.home.path.replace(/\/+$/, '') : formatter.home.path; + const lastSeparator = homePath.lastIndexOf('/'); + const parameter = homeTemplateParameterRegex.exec(homePath.slice(lastSeparator + 1)); + const pathSegmentParameter = parameter?.groups?.name; + if (!pathSegmentParameter) { + if (!isEqualOrParent(resource, resource.with({ path: homePath }))) { continue; } - const home = resource.with({ path: formatter.home.path, query: null, fragment: null }); + const home = resource.with({ path: homePath, query: null, fragment: null }); const formatting = formatter.formatting({ resource, home, parameters: new Map() }); if (formatting) { candidate = { home, formatting }; } } else { - if (resourceSegments.length < templateSegments.length) { + const prefix = homePath.slice(0, lastSeparator + 1); + if (!resource.path.startsWith(prefix)) { continue; } - const parameters = new Map(); - let matches = true; - for (let index = 0; index < templateSegments.length; index++) { - const parameter = homeTemplateParameterRegex.exec(templateSegments[index]); - if (parameter?.groups?.name) { - if (resourceSegments[index] === '.' || resourceSegments[index] === '..') { - matches = false; - break; - } - parameters.set(parameter.groups.name, resourceSegments[index]); - } else if (templateSegments[index] !== resourceSegments[index]) { - matches = false; - break; - } - } - if (!matches) { + const pathSegment = resource.path.slice(prefix.length).split('/')[0]; + if (!pathSegment || pathSegment === '.' || pathSegment === '..') { continue; } - const home = formatter.home.with({ path: resourceSegments.slice(0, templateSegments.length).join('/'), query: null, fragment: null }); + const parameters = new Map(); + parameters.set(pathSegmentParameter, pathSegment); + const home = formatter.home.with({ path: `${prefix}${pathSegment}`, query: null, fragment: null }); const formatting = formatter.formatting({ resource, home, parameters }); if (formatting) { candidate = { home, formatting }; From 8e27ac162f8452989ad7841987c9eb867cccdfa1 Mon Sep 17 00:00:00 2001 From: Giuseppe Cianci <39117631+Giuspepe@users.noreply.github.com> Date: Thu, 3 Sep 2026 10:56:27 +0200 Subject: [PATCH 08/13] agent host: restore channel state after a host restart (#334003) An agent host crash (V8 OOM) left two Codex sessions rendering a running turn forever. The turns had already aborted with the process; only the client's view was stale. Recovery had two independent holes, both in the shared AHP layer: - The client re-pushed every cached authentication through a single `Promise.all`. One rejected per-resource credential (an MCP server the restarted host no longer recognised) rejected the whole thing and aborted the rest of restart recovery, so subscription restoration never ran. - `initialize` registers a state channel even when its snapshot has not materialized (`_addInitialSubscription` uses the synchronous `getSnapshot`, unlike `subscribe`, which awaits `AgentService.subscribe` and restores evicted state). `canReplay` then compares only a global cursor, so the next reconnect answered `replay` to channels holding no baseline from the current process. Deltas applied onto nothing leave the channel stranded on pre-restart state. Provider-agnostic: the same paths serve Codex, Claude and Copilot. The Copilot sessions in the same window were equally stranded and only went unnoticed because they were idle rather than mid-turn. Client: - Only the freshly resolved initial credential is fatal; a rejected cached per-resource token is logged and stepped over. The protocol requires hosts to reject resources they do not currently advertise, so this is an expected response, not a failure. - `_subscriptionsAwaitingRestore` tracks channels a fresh initialize did not snapshot and survives transport drops, so a later reconnect finishes the restore even when it resolves to a snapshot-less replay. - `_authenticationRestorePending` carries an interrupted authentication pass to the next reconnect. Authentication is process-global and survives a transport drop, so only a frame that never arrived needs redelivery. Subscriptions: - `beginSnapshotRefresh`/`cancelSnapshotRefresh` buffer envelopes while a re-subscribe is in flight. These channels are already live, so an action newer than the snapshot can arrive first and would otherwise be clobbered by the older snapshot -- losing, for instance, the action that ends a turn. Server: - `_baselineDebt` records state channels handed out without a snapshot and denies `canReplay` while any requested channel still owes one. Debt is released when a baseline is delivered, when a channel is reported `missing`, and on unsubscribe, so an unrestorable channel cannot force the snapshot path for a client's healthy channels. Each fix has a test verified to fail without it, including a deterministic reproduction of the reconnect against the real `ProtocolServerHandler`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/agentHostProtocolClient.ts | 189 +++++++--- .../common/state/agentSubscription.ts | 53 ++- .../agentHost/node/protocolServerHandler.ts | 83 ++++- .../test/common/agentSubscription.test.ts | 37 ++ .../agentHostProtocolClient.test.ts | 334 +++++++++++++++++- .../test/node/protocolServerHandler.test.ts | 107 ++++++ 6 files changed, 756 insertions(+), 47 deletions(-) diff --git a/src/vs/platform/agentHost/browser/agentHostProtocolClient.ts b/src/vs/platform/agentHost/browser/agentHostProtocolClient.ts index 9b552b37009c1..70634e21c2b48 100644 --- a/src/vs/platform/agentHost/browser/agentHostProtocolClient.ts +++ b/src/vs/platform/agentHost/browser/agentHostProtocolClient.ts @@ -275,6 +275,7 @@ export class AgentHostProtocolClient extends Disposable implements IAgentConnect private readonly _authentication = new Map(); private _nextRequestId = 1; + /** /** * Reverse requests awaiting a response, scoped to their incoming transport. * An active count proves the host is waiting for client work rather than @@ -289,6 +290,30 @@ export class AgentHostProtocolClient extends Disposable implements IAgentConnect private _livenessDeferred = false; private _livenessDeferredSince: number | undefined; + /** + * Channels that owe a fresh snapshot because the host forgot this client + * (a host restart) and the fresh initialize did not carry their state. + * Entries persist across reconnect attempts until the channel is + * re-subscribed: a later `reconnect` resolves against the *new* host's + * sequence and can legitimately answer with a `replay`, which carries + * deltas only — so a channel dropped from this set before it was + * re-snapshotted would keep serving pre-restart state forever. + */ + private readonly _subscriptionsAwaitingRestore = new Set(); + + /** + * Set while an authentication-restore pass is in flight, cleared only once + * the whole pass completes. + * + * Authentication state on the host is process-global and survives a + * transport drop, so an ordinary reconnect needs no re-authentication. But + * a drop *during* the restore pass leaves the host holding whichever + * credentials happened to arrive first, and the next reconnect usually + * resolves to a `replay` — which never re-runs authentication. This flag + * carries that debt forward so the next successful attempt finishes it. + */ + private _authenticationRestorePending = false; + /** * Timestamp of the most recent message of any kind received from the * server. Used only for diagnostic logging when the close timer fires. @@ -782,15 +807,27 @@ export class AgentHostProtocolClient extends Disposable implements IAgentConnect this._applyReconnectResult(result, freshInitialize); this._updateManagedSettingsPermissions(true); - if (freshInitialize && result.type === ReconnectResultType.Snapshot) { + // Re-authenticate on a fresh initialize (the new process holds no + // credentials), or when an earlier pass was cut short by a transport + // drop — that attempt may have delivered only some of them, and an + // ordinary `replay` reconnect never revisits authentication. + if ((freshInitialize && result.type === ReconnectResultType.Snapshot) || this._authenticationRestorePending) { + if (freshInitialize && result.type === ReconnectResultType.Snapshot) { + this._markSubscriptionsAwaitingRestore(result.snapshots); + } await this._restoreAuthenticationAfterFreshInitialize(AgentHostClientState.Reconnecting); if (this._state.kind !== AgentHostClientState.Reconnecting) { return; } - await this._restoreSubscriptionsAfterFreshInitialize(result.snapshots); } - if (this._state.kind !== AgentHostClientState.Reconnecting) { - return; + // Not limited to a fresh initialize: this attempt may be finishing a + // restore an earlier one started but never completed, in which case + // the reconnect above resolved to a snapshot-less `replay`. + if (this._subscriptionsAwaitingRestore.size > 0) { + await this._restoreSubscriptionsAwaitingRestore(); + if (this._state.kind !== AgentHostClientState.Reconnecting) { + return; + } } // Re-push renderer-owned config on reconnect too: a reconnected host may @@ -871,51 +908,102 @@ export class AgentHostProtocolClient extends Disposable implements IAgentConnect }; } - private async _restoreSubscriptionsAfterFreshInitialize(initialSnapshots: readonly IStateSnapshot[]): Promise { + /** + * Record every active subscription the fresh initialize did not carry a + * snapshot for. The host forgot this client, so those channels still hold + * state produced by the previous host process and cannot be trusted until + * they have been re-subscribed. Rebuilt from scratch so a channel the + * host has since snapshotted stops being re-requested. + */ + private _markSubscriptionsAwaitingRestore(initialSnapshots: readonly IStateSnapshot[]): void { const restored = new Set(initialSnapshots.map(snapshot => snapshot.resource)); - const active = this._subscriptionManager.getActiveSubscriptions() - .filter(subscription => !restored.has(subscription.resource.toString())); - const restoreGroup = async (subscriptions: typeof active) => { + this._subscriptionsAwaitingRestore.clear(); + for (const subscription of this._subscriptionManager.getActiveSubscriptions()) { + const resource = subscription.resource.toString(); + if (!restored.has(resource)) { + this._subscriptionsAwaitingRestore.add(resource); + } + } + } + + /** + * Re-subscribe every channel recorded by + * {@link _markSubscriptionsAwaitingRestore} so it is reseated with state + * from the current host process. + * + * A channel leaves the set once it has a fresh snapshot, the server + * reported it missing, or it is no longer subscribed. A dropped transport + * deliberately leaves the remaining entries in place so the next reconnect + * finishes the job — that reconnect may well be answered with a `replay`, + * which would otherwise never deliver the snapshot these channels need. + */ + private async _restoreSubscriptionsAwaitingRestore(): Promise { + if (this._subscriptionsAwaitingRestore.size === 0) { + return; + } + const pending = this._subscriptionManager.getActiveSubscriptions() + .filter(subscription => this._subscriptionsAwaitingRestore.has(subscription.resource.toString())); + // Entries that are no longer subscribed can never be reseated, and a + // later subscribe issues its own snapshot request anyway. + this._subscriptionsAwaitingRestore.clear(); + for (const subscription of pending) { + this._subscriptionsAwaitingRestore.add(subscription.resource.toString()); + // These channels are already live server-side, so an action newer + // than the snapshot we are about to request can arrive first. Buffer + // until the snapshot lands rather than letting it be overwritten. + this._subscriptionManager.beginSnapshotRefresh(subscription.resource); + } + const restoreGroup = async (subscriptions: typeof pending) => { await Promise.all(subscriptions.map(async subscription => { + const resource = subscription.resource.toString(); try { const result = await this._dispatchRequest('subscribe', { - channel: subscription.resource.toString(), + channel: resource, }, { bypassReconnectGate: true }); - if (result.snapshot) { - this._subscriptionManager.applyReconnectSnapshot( - result.snapshot.resource, - result.snapshot.state, - result.snapshot.fromSeq, - true, - ); - this._serverSeq = Math.max(this._serverSeq, result.snapshot.fromSeq); + if (!result.snapshot) { + // Nothing to reseat this channel with; treat it like a + // failed restore rather than a completed one. + throw new Error(`subscribe returned no snapshot for ${resource}`); } + this._subscriptionManager.applyReconnectSnapshot( + result.snapshot.resource, + result.snapshot.state, + result.snapshot.fromSeq, + true, + ); + this._serverSeq = Math.max(this._serverSeq, result.snapshot.fromSeq); + this._subscriptionsAwaitingRestore.delete(resource); } catch (error) { if (error instanceof ProtocolError && error.code === AHP_CLIENT_CONNECTION_CLOSED) { + // Keep the entry: the next reconnect retries this channel. + this._subscriptionManager.cancelSnapshotRefresh(subscription.resource); throw error; } - this._logService.warn(`[AgentHostProtocolClient] Failed to restore subscription ${subscription.resource.toString()} after host restart: ${error instanceof Error ? error.message : String(error)}`); + this._logService.warn(`[AgentHostProtocolClient] Failed to restore subscription ${resource} after host restart: ${error instanceof Error ? error.message : String(error)}`); + this._subscriptionManager.cancelSnapshotRefresh(subscription.resource); this._subscriptionManager.markSubscriptionsMissing([subscription.resource]); + this._subscriptionsAwaitingRestore.delete(resource); } })); }; - await restoreGroup(active.filter(subscription => subscription.kind === StateComponents.Session)); + await restoreGroup(pending.filter(subscription => subscription.kind === StateComponents.Session)); await Promise.all([ - restoreGroup(active.filter(subscription => subscription.kind === StateComponents.Chat)), - restoreGroup(active.filter(subscription => subscription.kind !== StateComponents.Session && subscription.kind !== StateComponents.Chat)), + restoreGroup(pending.filter(subscription => subscription.kind === StateComponents.Chat)), + restoreGroup(pending.filter(subscription => subscription.kind !== StateComponents.Session && subscription.kind !== StateComponents.Chat)), ]); } private async _restoreAuthenticationAfterFreshInitialize(expectedState: AgentHostClientState.Connecting | AgentHostClientState.Reconnecting): Promise { - let resolvedInitialAuthentication = false; + this._authenticationRestorePending = true; + let initialAuthenticationKey: string | undefined; if (this._resolveInitialAuthentication) { try { const initialAuthentication = await this._resolveInitialAuthentication(); if (initialAuthentication) { const normalizedParams = this._normalizeAuthenticationParams(initialAuthentication); - this._authentication.set(this._authenticationKey(normalizedParams), normalizedParams); - resolvedInitialAuthentication = true; + initialAuthenticationKey = this._authenticationKey(normalizedParams); + this._authentication.set(initialAuthenticationKey, normalizedParams); } } catch (error) { throw new InitialAuthenticationError(error); @@ -924,23 +1012,37 @@ export class AgentHostProtocolClient extends Disposable implements IAgentConnect return; } } - try { - await Promise.all([...this._authentication.values()].map(params => this._dispatchRequest('authenticate', { - channel: ROOT_STATE_URI, - ...params, - scopes: params.scopes ? [...params.scopes] : undefined, - }, this._state.kind === AgentHostClientState.Connecting - ? { bypassInitializeQueue: true, bypassReconnectGate: true } - : { bypassReconnectGate: true }))); - } catch (error) { - // A dropped transport is not an authentication failure. Wrapping it - // would classify a momentary blip as terminally incompatible and - // permanently stop recovery, so let it stay a reconnectable error. - if (resolvedInitialAuthentication && !isConnectionClosedError(error)) { - throw new InitialAuthenticationError(error); + await Promise.all([...this._authentication.entries()].map(async ([key, params]) => { + try { + await this._dispatchRequest('authenticate', { + channel: ROOT_STATE_URI, + ...params, + scopes: params.scopes ? [...params.scopes] : undefined, + }, this._state.kind === AgentHostClientState.Connecting + ? { bypassInitializeQueue: true, bypassReconnectGate: true } + : { bypassReconnectGate: true }); + } catch (error) { + // A dropped transport is not an authentication failure. Wrapping it + // would classify a momentary blip as terminally incompatible and + // permanently stop recovery, so let it stay a reconnectable error. + // The pending flag survives so the next attempt redelivers this. + if (isConnectionClosedError(error)) { + throw error; + } + // Only the freshly resolved initial authentication is essential: + // without it the host cannot serve this client at all. Every other + // entry is a cached per-resource token (an MCP server, say) that the + // host may legitimately reject — the protocol requires rejecting a + // resource the host does not currently advertise — and aborting here + // would skip the rest of the restart recovery, including the + // subscription re-snapshot that keeps restored sessions from freezing. + if (key === initialAuthenticationKey) { + throw new InitialAuthenticationError(error); + } + this._logService.warn(`[AgentHostProtocolClient] Failed to restore authentication for ${params.resource} after host restart: ${error instanceof Error ? error.message : String(error)}`); } - throw error; - } + })); + this._authenticationRestorePending = false; } private _clientMeta(): Record { @@ -1025,11 +1127,18 @@ export class AgentHostProtocolClient extends Disposable implements IAgentConnect if (result.missing.length > 0) { this._logService.info(`[RemoteAgentHostProtocol] Server cannot resume ${result.missing.length} subscription(s) after reconnect.`); this._subscriptionManager.markSubscriptionsMissing(result.missing.map(u => URI.parse(u))); + // A channel the server cannot resume can never be reseated. + for (const resource of result.missing) { + this._subscriptionsAwaitingRestore.delete(resource); + } } } else { let maxSeq = this._serverSeq; for (const snapshot of result.snapshots) { this._subscriptionManager.applyReconnectSnapshot(snapshot.resource, snapshot.state, snapshot.fromSeq, preservePending); + // This snapshot came from the current host, so it settles any + // restore still owed for the channel. + this._subscriptionsAwaitingRestore.delete(snapshot.resource); if (snapshot.fromSeq > maxSeq) { maxSeq = snapshot.fromSeq; } diff --git a/src/vs/platform/agentHost/common/state/agentSubscription.ts b/src/vs/platform/agentHost/common/state/agentSubscription.ts index bdd3576868c84..d08a9a67dc2a8 100644 --- a/src/vs/platform/agentHost/common/state/agentSubscription.ts +++ b/src/vs/platform/agentHost/common/state/agentSubscription.ts @@ -103,6 +103,7 @@ abstract class BaseAgentSubscription extends Disposable implements IAgentSubs protected _confirmedState: T | undefined; private _error: Error | undefined; private _bufferedEnvelopes: ActionEnvelope[] | undefined; + private _awaitingSnapshotRefresh = false; protected readonly _onDidChange = this._register(new Emitter()); readonly onDidChange: Event = this._onDidChange.event; @@ -140,12 +141,48 @@ abstract class BaseAgentSubscription extends Disposable implements IAgentSubs * Apply an initial snapshot from the server. */ handleSnapshot(state: T, fromSeq: number): void { + this._awaitingSnapshotRefresh = false; this._confirmedState = state; this._error = undefined; this._onSnapshotApplied(fromSeq); this._onDidChange.fire(this.value as T); } + /** + * Buffer incoming envelopes until the next {@link handleSnapshot}. + * + * Needed when a subscription that already holds confirmed state is + * re-subscribed to be reseated from a restarted host: the snapshot is + * computed at some `fromSeq`, but the channel is already live, so newer + * actions can reach the client before the subscribe response does. + * Applying them first and then installing the older snapshot would drop + * them silently — losing, say, the action that ends a turn. + */ + beginSnapshotRefresh(): void { + this._awaitingSnapshotRefresh = true; + } + + /** + * Abandon a refresh started by {@link beginSnapshotRefresh} when the + * snapshot never arrives, applying whatever was buffered meanwhile so the + * subscription does not silently lose those actions too. + */ + cancelSnapshotRefresh(): void { + if (!this._awaitingSnapshotRefresh) { + return; + } + this._awaitingSnapshotRefresh = false; + const buffered = this._bufferedEnvelopes; + if (!buffered || this._confirmedState === undefined) { + return; + } + this._bufferedEnvelopes = undefined; + for (const envelope of buffered) { + this._reconcile(envelope, envelope.origin?.clientId === this._clientId); + } + this._onDidChange.fire(this.value as T); + } + /** * Mark this subscription as failed. */ @@ -165,7 +202,7 @@ abstract class BaseAgentSubscription extends Disposable implements IAgentSubs // Buffer actions that arrive before the snapshot has been applied. // They're replayed in _onSnapshotApplied(). - if (this._confirmedState === undefined) { + if (this._confirmedState === undefined || this._awaitingSnapshotRefresh) { if (!this._bufferedEnvelopes) { this._bufferedEnvelopes = []; } @@ -1179,6 +1216,20 @@ export class AgentSubscriptionManager extends Disposable { entry.sub.handleSnapshot(state as never, fromSeq); } + /** + * Put a subscription into snapshot-refresh mode ahead of an explicit + * re-`subscribe` that reseats it from a restarted host. See + * {@link BaseAgentSubscription.beginSnapshotRefresh}. + */ + beginSnapshotRefresh(resource: URI): void { + this._subscriptions.get(resource)?.sub.beginSnapshotRefresh(); + } + + /** Abandon a refresh started by {@link beginSnapshotRefresh}. */ + cancelSnapshotRefresh(resource: URI): void { + this._subscriptions.get(resource)?.sub.cancelSnapshotRefresh(); + } + /** * Mark a set of subscriptions as no longer resumable on the server * (reported via `ReconnectReplayResult.missing`). The subscriptions diff --git a/src/vs/platform/agentHost/node/protocolServerHandler.ts b/src/vs/platform/agentHost/node/protocolServerHandler.ts index e9a8d30625206..dd8d5e53736ec 100644 --- a/src/vs/platform/agentHost/node/protocolServerHandler.ts +++ b/src/vs/platform/agentHost/node/protocolServerHandler.ts @@ -356,6 +356,19 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien * {@link IClientRecord}. */ private readonly _clients = new Map(); + /** + * State channels a client is subscribed to but has never been given a + * baseline snapshot for by THIS server process, keyed by clientId. + * + * `initialize` registers a state channel even when its snapshot has not + * materialized yet (see {@link _addInitialSubscription}), so a client can + * hold a live subscription with no state behind it. Replaying deltas onto + * that void silently strands the channel on whatever the client last saw — + * across a host restart that is pre-restart state, which is how an + * already-finished turn can render as perpetually running. Reconnect + * consults this to force a snapshot response for such channels. + */ + private readonly _baselineDebt = new Map>(); private readonly _replayBuffer: ActionEnvelope[] = []; private readonly _telemetryReporter: AgentHostTelemetryReporter; private readonly _managedSettingsOwnerId = generateUuid(); @@ -701,9 +714,47 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien client.subscriptions.set(sub.uri, sub); this._agentService.addSubscriber(URI.parse(sub.uri), client.clientId); this._clearClientToolCallDisconnectTimeout(client.clientId, sub.uri); + if (snapshot) { + this._clearBaselineDebt(client.clientId, sub.uri); + } else { + this._recordBaselineDebt(client.clientId, sub.uri); + } return snapshot; } + /** + * Note that `clientId` holds a subscription to `uri` with no baseline from + * this process. See {@link _baselineDebt}. + */ + private _recordBaselineDebt(clientId: string, uri: string): void { + let debt = this._baselineDebt.get(clientId); + if (!debt) { + debt = new Set(); + this._baselineDebt.set(clientId, debt); + } + debt.add(uri); + } + + /** Record that `clientId` has now been given a baseline for `uri`. */ + private _clearBaselineDebt(clientId: string, uri: string): void { + const debt = this._baselineDebt.get(clientId); + if (debt?.delete(uri) && debt.size === 0) { + this._baselineDebt.delete(clientId); + } + } + + /** Whether any of `subscriptions` still owes `clientId` a baseline. */ + private _hasBaselineDebt(clientId: string, subscriptions: readonly string[]): boolean { + const debt = this._baselineDebt.get(clientId); + if (!debt || debt.size === 0) { + return false; + } + return subscriptions.some(subscription => { + const classified = classifyChannel(subscription.toString()); + return classified !== undefined && debt.has(classified.uri); + }); + } + private async _subscribeStateChannel(channel: string, clientId: string, isActive?: () => boolean): Promise { if (!isAhpAutomationCatalogChannel(channel)) { return this._agentService.subscribe(URI.parse(channel), clientId, isActive); @@ -794,7 +845,12 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien this._registerClientFileSystemAuthority(params.clientId, initializationDisposables); const oldestBuffered = this._replayBuffer.length > 0 ? this._replayBuffer[0].serverSeq : this._stateManager.serverSeq; - const canReplay = params.lastSeenServerSeq >= oldestBuffered; + // A global cursor only proves the client saw every ACTION; it says + // nothing about whether a given channel ever received state from this + // process to apply them to. Force snapshots while any requested + // channel still owes a baseline. + const canReplay = params.lastSeenServerSeq >= oldestBuffered + && !this._hasBaselineDebt(params.clientId, params.subscriptions); const responsePromise = this._restoreReconnectSubscriptions(client, params, canReplay); client.telemetryConnectionActive = true; @@ -883,6 +939,7 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien if (!descriptor) { this._logService.info(`[ProtocolServer] Reconnect: resource watch ${key} no longer parses`); missing.push(sub); + this._clearBaselineDebt(client.clientId, classified.uri); return undefined; } if (canReplay) { @@ -918,6 +975,11 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien } this._logService.info(`[ProtocolServer] Reconnect: failed to restore subscription ${key}: ${err instanceof Error ? err.message : String(err)}`); missing.push(sub); + // Reported as missing, so it can never be baselined. Leaving the + // debt would deny `canReplay` for this client's healthy channels + // on every future reconnect, since `missing` is not repeated on + // the snapshot branch and nothing else would ever clear it. + this._clearBaselineDebt(client.clientId, classified.uri); return undefined; } })); @@ -947,9 +1009,16 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien return undefined; } const subscription = client.subscriptions.get(snapshot.resource.toString()); - return subscription?.kind === ChannelKind.State - ? this._stateManager.getSnapshot(subscription.uri) - : snapshot; + if (subscription?.kind !== ChannelKind.State) { + return snapshot; + } + const refreshed = this._stateManager.getSnapshot(subscription.uri); + if (refreshed) { + // Key off the subscription, not the snapshot's echoed resource, + // so the debt entry recorded under the same key is really cleared. + this._clearBaselineDebt(client.clientId, subscription.uri); + } + return refreshed; }); return { type: 'snapshot', snapshots: refreshedSnapshots.filter((s): s is IStateSnapshot => s !== undefined) }; } @@ -1164,6 +1233,7 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien this._clients.set(client.clientId, previousRecord); } else { this._clients.delete(client.clientId); + this._baselineDebt.delete(client.clientId); } } } @@ -1331,6 +1401,7 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien && record.lastSeenAt < cutoff) { record.disconnectTimeouts.dispose(); this._clients.delete(clientId); + this._baselineDebt.delete(clientId); } } } @@ -1429,6 +1500,7 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien } client.subscriptions.set(classified.uri, classified); this._clearClientToolCallDisconnectTimeout(client.clientId, classified.uri); + this._clearBaselineDebt(client.clientId, classified.uri); // `IStateSnapshot` is widened with `ChatState` (see sessionProtocol.ts); // the generated wire `Snapshot` union does not list it yet. The value // is JSON over the wire, so narrowing at this boundary is safe. @@ -2027,6 +2099,9 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien return; } client.subscriptions.delete(classified.uri); + // An unsubscribed channel owes this client nothing; a later subscribe + // records its own debt if it again lands without a baseline. + this._clearBaselineDebt(client.clientId, classified.uri); if (sub.kind === ChannelKind.State) { const record = this._clients.get(client.clientId); if (record && this._hasSubscriptionInOtherConnection(record, client, sub.uri)) { diff --git a/src/vs/platform/agentHost/test/common/agentSubscription.test.ts b/src/vs/platform/agentHost/test/common/agentSubscription.test.ts index 706885b378e4d..b105454c3987d 100644 --- a/src/vs/platform/agentHost/test/common/agentSubscription.test.ts +++ b/src/vs/platform/agentHost/test/common/agentSubscription.test.ts @@ -338,6 +338,43 @@ suite('RootStateSubscription', () => { assert.strictEqual((sub.value as RootState).activeSessions, 0); }); + test('snapshot refresh buffers newer envelopes so a stale snapshot cannot drop them', () => { + const sub = disposables.add(new RootStateSubscription('c1', noop)); + sub.handleSnapshot(makeRootState({ activeSessions: 1 }), 10); + + // Re-subscribing an already-seated channel: the snapshot is computed at + // seq 23 but a newer action reaches the client first. + sub.beginSnapshotRefresh(); + sub.receiveEnvelope(makeEnvelope( + { type: ActionType.RootActiveSessionsChanged, activeSessions: 7 }, + 24, + )); + assert.strictEqual((sub.value as RootState).activeSessions, 1, 'newer action is held back until the snapshot lands'); + + sub.handleSnapshot(makeRootState({ activeSessions: 3 }), 23); + assert.strictEqual((sub.value as RootState).activeSessions, 7, 'newer action wins over the older snapshot'); + }); + + test('cancelling a snapshot refresh applies what it buffered', () => { + const sub = disposables.add(new RootStateSubscription('c1', noop)); + sub.handleSnapshot(makeRootState({ activeSessions: 1 }), 10); + sub.beginSnapshotRefresh(); + sub.receiveEnvelope(makeEnvelope( + { type: ActionType.RootActiveSessionsChanged, activeSessions: 7 }, + 24, + )); + + sub.cancelSnapshotRefresh(); + assert.strictEqual((sub.value as RootState).activeSessions, 7); + + // Refresh mode is off again: later envelopes apply directly. + sub.receiveEnvelope(makeEnvelope( + { type: ActionType.RootActiveSessionsChanged, activeSessions: 9 }, + 25, + )); + assert.strictEqual((sub.value as RootState).activeSessions, 9); + }); + test('setError makes value return the error', () => { const sub = disposables.add(new RootStateSubscription('c1', noop)); sub.handleSnapshot(makeRootState(), 0); diff --git a/src/vs/platform/agentHost/test/electron-browser/agentHostProtocolClient.test.ts b/src/vs/platform/agentHost/test/electron-browser/agentHostProtocolClient.test.ts index aa03e63be162a..3906d696de5f0 100644 --- a/src/vs/platform/agentHost/test/electron-browser/agentHostProtocolClient.test.ts +++ b/src/vs/platform/agentHost/test/electron-browser/agentHostProtocolClient.test.ts @@ -26,11 +26,11 @@ import { ContentEncoding, ReconnectResultType } from '../../common/state/protoco import { ChatSourceKind } from '../../common/state/protocol/channels-chat/commands.js'; import { AhpErrorCodes, JsonRpcErrorCodes } from '../../common/state/protocol/errors.js'; import { PROTOCOL_VERSION, SUPPORTED_PROTOCOL_VERSIONS } from '../../common/state/protocol/version/registry.js'; -import { ActionType, type ChatTurnStartedAction, type SessionActiveClientSetAction, type SessionActiveClientRemovedAction, type SessionTitleChangedAction } from '../../common/state/sessionActions.js'; +import { ActionType, type ChatTurnCompleteAction, type ChatTurnStartedAction, type SessionActiveClientSetAction, type SessionActiveClientRemovedAction, type SessionTitleChangedAction } from '../../common/state/sessionActions.js'; import { ProtocolError, type AhpServerNotification, type JsonRpcNotification, type JsonRpcRequest, type JsonRpcResponse, type ProtocolMessage } from '../../common/state/sessionProtocol.js'; import { hasKey } from '../../../../base/common/types.js'; import { mainWindow } from '../../../../base/browser/window.js'; -import { AUTOMATION_CATALOG_URI, buildChatUri, CustomizationType, MessageAttachmentKind, MessageKind, PendingMessageKind, readSessionExternal, readSessionWorkspaceless, ROOT_STATE_URI, SessionStatus, StateComponents, customizationId, withSessionExternal, withSessionWorkspaceless } from '../../common/state/sessionState.js'; +import { AUTOMATION_CATALOG_URI, buildChatUri, CustomizationType, MessageAttachmentKind, MessageKind, PendingMessageKind, readSessionExternal, readSessionWorkspaceless, ROOT_STATE_URI, SessionStatus, StateComponents, TurnState, customizationId, withSessionExternal, withSessionWorkspaceless } from '../../common/state/sessionState.js'; import { AgentHostTransportFailureReason, NonReconnectableTransportError, type IClientTransport, type IProtocolTransport } from '../../common/state/sessionTransport.js'; import { TestConfigurationService } from '../../../configuration/test/common/testConfigurationService.js'; import { ITelemetryService, TelemetryConfiguration, TelemetryLevel, TELEMETRY_SETTING_ID } from '../../../telemetry/common/telemetry.js'; @@ -2262,6 +2262,40 @@ suite('AgentHostProtocolClient', () => { } } + /** + * Like {@link waitForRequestAt}, but gives up instead of spinning forever. + * Yields through the timer queue so a regression that never issues the + * request fails with a readable assertion rather than starving Mocha's + * own timeout. + */ + async function waitForRequestAtWithin(transport: TestProtocolTransport, method: string, index: number, timeoutMs = 8_000): Promise { + const deadline = Date.now() + timeoutMs; + while (true) { + const requests = transport.sentMessages.filter( + (message): message is JsonRpcRequest => hasKey(message, { method: true, id: true }) && message.method === method, + ); + if (requests[index]) { + return requests[index]; + } + if (Date.now() > deadline) { + const sent = transport.sentMessages.map(m => hasKey(m, { method: true }) ? m.method : 'response').join(', '); + throw new Error(`Timed out waiting for '${method}' request #${index}; saw ${requests.length}. Sent: [${sent}]`); + } + await new Promise(r => setTimeout(r, 5)); + } + } + + /** Wait for the client to reach {@link AgentHostClientState.Connected}, bounded. */ + async function waitForConnectedWithin(client: AgentHostProtocolClient, timeoutMs = 8_000): Promise { + const deadline = Date.now() + timeoutMs; + while (client.connectionState !== AgentHostClientState.Connected) { + if (Date.now() > deadline) { + throw new Error(`Timed out waiting for Connected; state is ${client.connectionState}`); + } + await new Promise(r => setTimeout(r, 5)); + } + } + /** Wait for the next time the new transport is created by the factory. */ async function waitForTransport(transports: TestClientProtocolTransport[], index: number): Promise { while (transports.length <= index) { @@ -2806,6 +2840,302 @@ suite('AgentHostProtocolClient', () => { client.dispose(); }); + test('restores subscriptions when a cached resource authentication is rejected after a host restart', async function () { + this.timeout(20_000); + const { client, transports } = createFactoryClient(); + const sessionUri = URI.parse('codex:/stuck-session'); + const connectPromise = client.connect(); + await completeHandshake(transports[0], connectPromise); + + const sessionRef = client.getSubscription(StateComponents.Session, sessionUri, 'test'); + const initialSubscribe = await waitForRequest(transports[0], 'subscribe'); + transports[0].fireMessage({ + jsonrpc: '2.0', id: initialSubscribe.id, + result: { snapshot: { resource: sessionUri.toString(), state: { lifecycle: 'ready' }, fromSeq: 5 } }, + }); + const authentication = client.authenticate({ resource: 'https://mcp.example.com', token: 'token' }); + const initialAuthenticate = await waitForRequest(transports[0], 'authenticate'); + transports[0].fireMessage({ jsonrpc: '2.0', id: initialAuthenticate.id, result: {} }); + await authentication; + await flushMicrotasks(); + + transports[0].fireClose(); + await waitForReconnecting(client); + const reconnectTransport = await waitForTransport(transports, 1); + reconnectTransport.connectDeferred.complete(); + const reconnect = await waitForRequest(reconnectTransport, 'reconnect'); + reconnectTransport.fireMessage({ + jsonrpc: '2.0', id: reconnect.id, + error: { code: AhpErrorCodes.NotFound, message: 'Reconnect client not found' }, + }); + const initialize = await waitForRequest(reconnectTransport, 'initialize'); + reconnectTransport.fireMessage({ + jsonrpc: '2.0', id: initialize.id, + result: { + protocolVersion: PROTOCOL_VERSION, + serverSeq: 0, + snapshots: [{ resource: ROOT_STATE_URI, state: { agents: [], activeSessions: 0 }, fromSeq: 0 }], + }, + }); + + // The restarted host no longer accepts this cached third-party token. + // That must not abort the restart recovery: the session channel still + // holds pre-restart state and would otherwise never be reseated. + const restoredAuthenticate = await waitForRequestAt(reconnectTransport, 'authenticate', 0); + reconnectTransport.fireMessage({ + jsonrpc: '2.0', id: restoredAuthenticate.id, + error: { code: AhpErrorCodes.AuthRequired, message: 'Authentication failed for resource: https://mcp.example.com' }, + }); + + const restoredSubscribe = await waitForRequestAtWithin(reconnectTransport, 'subscribe', 0); + reconnectTransport.fireMessage({ + jsonrpc: '2.0', id: restoredSubscribe.id, + result: { snapshot: { resource: sessionUri.toString(), state: { lifecycle: 'ready' }, fromSeq: 3 } }, + }); + await waitForConnectedWithin(client); + + assert.deepStrictEqual({ + channel: (restoredSubscribe.params as { channel: string }).channel, + state: client.connectionState, + }, { + channel: sessionUri.toString(), + state: AgentHostClientState.Connected, + }); + + sessionRef.dispose(); + client.dispose(); + }); + + test('finishes an interrupted post-restart subscription restore on the next reconnect', async function () { + this.timeout(20_000); + const { client, transports } = createFactoryClient(); + const sessionUri = URI.parse('codex:/stuck-session'); + const chatUri = URI.parse('ahp-chat://default/stuck-session'); + const connectPromise = client.connect(); + await completeHandshake(transports[0], connectPromise); + + const sessionRef = client.getSubscription(StateComponents.Session, sessionUri, 'test'); + const initialSessionSubscribe = await waitForRequestAt(transports[0], 'subscribe', 0); + transports[0].fireMessage({ + jsonrpc: '2.0', id: initialSessionSubscribe.id, + result: { snapshot: { resource: sessionUri.toString(), state: { lifecycle: 'ready' }, fromSeq: 5 } }, + }); + const chatRef = client.getSubscription(StateComponents.Chat, chatUri, 'test'); + const initialChatSubscribe = await waitForRequestAt(transports[0], 'subscribe', 1); + transports[0].fireMessage({ + jsonrpc: '2.0', id: initialChatSubscribe.id, + result: { snapshot: { resource: chatUri.toString(), state: { turns: [] }, fromSeq: 5 } }, + }); + await flushMicrotasks(); + + transports[0].fireClose(); + await waitForReconnecting(client); + const reconnectTransport = await waitForTransport(transports, 1); + reconnectTransport.connectDeferred.complete(); + const reconnect = await waitForRequest(reconnectTransport, 'reconnect'); + reconnectTransport.fireMessage({ + jsonrpc: '2.0', id: reconnect.id, + error: { code: AhpErrorCodes.NotFound, message: 'Reconnect client not found' }, + }); + const initialize = await waitForRequest(reconnectTransport, 'initialize'); + reconnectTransport.fireMessage({ + jsonrpc: '2.0', id: initialize.id, + result: { + protocolVersion: PROTOCOL_VERSION, + serverSeq: 22, + snapshots: [{ resource: ROOT_STATE_URI, state: { agents: [], activeSessions: 0 }, fromSeq: 22 }], + }, + }); + + // The session is reseated, then the transport drops before the chat + // channel gets its snapshot. + const restoredSessionSubscribe = await waitForRequestAtWithin(reconnectTransport, 'subscribe', 0); + reconnectTransport.fireMessage({ + jsonrpc: '2.0', id: restoredSessionSubscribe.id, + result: { snapshot: { resource: sessionUri.toString(), state: { lifecycle: 'ready' }, fromSeq: 23 } }, + }); + await flushMicrotasks(); + reconnectTransport.fireClose(); + await waitForReconnecting(client); + + // The host now remembers the client, so this reconnect resolves to a + // replay — which carries no snapshot for the chat channel. + const secondTransport = await waitForTransport(transports, 2); + secondTransport.connectDeferred.complete(); + const secondReconnect = await waitForRequest(secondTransport, 'reconnect'); + secondTransport.fireMessage({ + jsonrpc: '2.0', id: secondReconnect.id, + result: { type: ReconnectResultType.Replay, actions: [], missing: [] }, + }); + + const restoredChatSubscribe = await waitForRequestAtWithin(secondTransport, 'subscribe', 0); + secondTransport.fireMessage({ + jsonrpc: '2.0', id: restoredChatSubscribe.id, + result: { snapshot: { resource: chatUri.toString(), state: { turns: [] }, fromSeq: 40 } }, + }); + await waitForConnectedWithin(client); + + assert.deepStrictEqual({ + resubscribedAfterReplay: (restoredChatSubscribe.params as { channel: string }).channel, + sessionResubscribedAgain: secondTransport.sentMessages.filter( + message => hasKey(message, { method: true }) && message.method === 'subscribe' + && (message.params as { channel: string }).channel === sessionUri.toString(), + ).length, + state: client.connectionState, + }, { + resubscribedAfterReplay: chatUri.toString(), + sessionResubscribedAgain: 0, + state: AgentHostClientState.Connected, + }); + + chatRef.dispose(); + sessionRef.dispose(); + client.dispose(); + }); + + test('keeps an action that arrives before the post-restart restore snapshot', async function () { + this.timeout(20_000); + const { client, transports } = createFactoryClient(); + const chatUri = URI.parse('ahp-chat://default/racing-session'); + const connectPromise = client.connect(); + await completeHandshake(transports[0], connectPromise); + + const chatRef = client.getSubscription<{ turns: { id: string; state?: TurnState }[] }>(StateComponents.Chat, chatUri, 'test'); + const initialSubscribe = await waitForRequest(transports[0], 'subscribe'); + transports[0].fireMessage({ + jsonrpc: '2.0', id: initialSubscribe.id, + result: { snapshot: { resource: chatUri.toString(), state: { turns: [] }, fromSeq: 5 } }, + }); + await flushMicrotasks(); + + transports[0].fireClose(); + await waitForReconnecting(client); + const reconnectTransport = await waitForTransport(transports, 1); + reconnectTransport.connectDeferred.complete(); + const reconnect = await waitForRequest(reconnectTransport, 'reconnect'); + reconnectTransport.fireMessage({ + jsonrpc: '2.0', id: reconnect.id, + error: { code: AhpErrorCodes.NotFound, message: 'Reconnect client not found' }, + }); + const initialize = await waitForRequest(reconnectTransport, 'initialize'); + reconnectTransport.fireMessage({ + jsonrpc: '2.0', id: initialize.id, + result: { + protocolVersion: PROTOCOL_VERSION, + serverSeq: 22, + snapshots: [{ resource: ROOT_STATE_URI, state: { agents: [], activeSessions: 0 }, fromSeq: 22 }], + }, + }); + + // The channel is already live server-side, so an action newer than + // the snapshot being computed can reach the client first. It must + // survive the older snapshot that follows. + const restoreSubscribe = await waitForRequestAtWithin(reconnectTransport, 'subscribe', 0); + const completion: ChatTurnCompleteAction = { type: ActionType.ChatTurnComplete, turnId: 'turn-1', duration: 10 }; + reconnectTransport.fireMessage({ + jsonrpc: '2.0', + method: 'action', + params: { + channel: chatUri.toString(), + action: completion, + serverSeq: 24, + origin: undefined, + }, + }); + reconnectTransport.fireMessage({ + jsonrpc: '2.0', id: restoreSubscribe.id, + result: { + snapshot: { + resource: chatUri.toString(), + // An in-flight turn lives in `activeTurn`; this is exactly the + // stale shape that left the UI spinning forever. + state: { + turns: [], + activeTurn: { + id: 'turn-1', + startedAt: '2026-09-02T00:00:00.000Z', + message: { text: 'hi', origin: { kind: MessageKind.User } }, + responseParts: [], + usage: undefined, + }, + }, + fromSeq: 23, + }, + }, + }); + await waitForConnectedWithin(client); + + const value = chatRef.object.value as { turns: { id: string; state?: TurnState }[]; activeTurn?: { id: string } }; + assert.deepStrictEqual({ + turnStates: value.turns.map(turn => turn.state), + stillActive: value.activeTurn?.id, + }, { + turnStates: [TurnState.Complete], + stillActive: undefined, + }); + + chatRef.dispose(); + client.dispose(); + }); + + test('finishes an interrupted authentication restore on the next reconnect', async function () { + this.timeout(20_000); + const { client, transports } = createFactoryClient(); + const connectPromise = client.connect(); + await completeHandshake(transports[0], connectPromise); + + const authentication = client.authenticate({ resource: 'https://sandbox.example.com', token: 'sealed' }); + const initialAuthenticate = await waitForRequest(transports[0], 'authenticate'); + transports[0].fireMessage({ jsonrpc: '2.0', id: initialAuthenticate.id, result: {} }); + await authentication; + await flushMicrotasks(); + + transports[0].fireClose(); + await waitForReconnecting(client); + const reconnectTransport = await waitForTransport(transports, 1); + reconnectTransport.connectDeferred.complete(); + const reconnect = await waitForRequest(reconnectTransport, 'reconnect'); + reconnectTransport.fireMessage({ + jsonrpc: '2.0', id: reconnect.id, + error: { code: AhpErrorCodes.NotFound, message: 'Reconnect client not found' }, + }); + const initialize = await waitForRequest(reconnectTransport, 'initialize'); + reconnectTransport.fireMessage({ + jsonrpc: '2.0', id: initialize.id, + result: { protocolVersion: PROTOCOL_VERSION, serverSeq: 0, snapshots: [] }, + }); + + // The transport dies after the authenticate frame goes out but before + // the host answers, so the credential may never have landed. + await waitForRequestAtWithin(reconnectTransport, 'authenticate', 0); + reconnectTransport.fireClose(); + await waitForReconnecting(client); + + // The host remembers the client now, so this reconnect is a replay — + // which historically never revisited authentication. + const secondTransport = await waitForTransport(transports, 2); + secondTransport.connectDeferred.complete(); + const secondReconnect = await waitForRequest(secondTransport, 'reconnect'); + secondTransport.fireMessage({ + jsonrpc: '2.0', id: secondReconnect.id, + result: { type: ReconnectResultType.Replay, actions: [], missing: [] }, + }); + + const retriedAuthenticate = await waitForRequestAtWithin(secondTransport, 'authenticate', 0); + secondTransport.fireMessage({ jsonrpc: '2.0', id: retriedAuthenticate.id, result: {} }); + await waitForConnectedWithin(client); + + assert.deepStrictEqual({ + resource: (retriedAuthenticate.params as { resource: string }).resource, + state: client.connectionState, + }, { + resource: 'https://sandbox.example.com', + state: AgentHostClientState.Connected, + }); + + client.dispose(); + }); + test('marks an Automation catalogue subscription missing when restore fails', async function () { this.timeout(10_000); const { client, transports } = createFactoryClient(); diff --git a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts index 0b41c43be7c9a..def42ccb47300 100644 --- a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts +++ b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts @@ -2832,6 +2832,113 @@ suite('ProtocolServerHandler', () => { assert.ok(stateManager.getSnapshot(sessionUri), 'state should have been re-hydrated by reconnect'); }); + test('reconnect answers replay even when initialize gave a channel no baseline', async () => { + // Reproduces the server half of the eternal-spinner incident. `initialize` + // registers a state channel whose snapshot has not materialized yet + // (`_addInitialSubscription` uses the SYNCHRONOUS `getSnapshot`, unlike + // `subscribe`, which awaits `AgentService.subscribe` and restores evicted + // state). The client is subscribed with no baseline from THIS process, yet + // the next reconnect passes the purely-global `canReplay` check and is + // answered with deltas only — which a client applies onto nothing. + const transport1 = connectClient('client-no-baseline', [sessionUri]); + const initResp = findResponse(transport1.sent, 1) as { result: InitializeResult }; + const initSeq = initResp.result.serverSeq; + const sessionSnapshot = initResp.result.snapshots?.find(snapshot => snapshot.resource === sessionUri); + transport1.simulateClose(); + + // Mirror the incident's timing: the replay buffer is empty and the client + // is level with the server, so `canReplay` is true. The session then + // materializes DURING the async restore — exactly what happened as the + // restarted host restored its sessions while answering the reconnect. + agentService.subscribe = async (resource, _clientId) => { + if (!stateManager.getSnapshot(resource.toString())) { + stateManager.restoreSession(makeSessionSummary(), []); + stateManager.dispatchServerAction(sessionUri, { type: ActionType.SessionReady, }); + } + return stateManager.getSnapshot(resource.toString())!; + }; + + const transport2 = new MockProtocolTransport(); + server.simulateConnection(transport2); + const reconnectRespPromise = waitForResponse(transport2, 1); + transport2.simulateMessage(request(1, 'reconnect', { + clientId: 'client-no-baseline', + lastSeenServerSeq: initSeq, + subscriptions: [sessionUri], + })); + const reconnectResp = await reconnectRespPromise as { result: ReconnectResult }; + + assert.deepStrictEqual({ + initializeGaveSnapshot: sessionSnapshot !== undefined, + reconnectType: reconnectResp.result.type, + }, { + // The channel was subscribed without a baseline... + initializeGaveSnapshot: false, + // ...so the server must send a snapshot, not deltas onto nothing. + reconnectType: 'snapshot', + }); + }); + + test('a channel reported missing does not deny replay for the client\'s other channels', async () => { + // A terminal channel from a dead host process can never be baselined: + // `initialize` registers it with no snapshot, and the follow-up restore + // reports it `missing`. Its baseline debt must be released — otherwise a + // single unrestorable channel would force full snapshots for every other + // channel this client holds, forever. (The real incident's reconnect + // reported 15 such terminal channels.) + const deadTerminal = 'agenthost-terminal:/dead-from-previous-process'; + const transport1 = connectClient('client-missing-debt', [sessionUri, deadTerminal]); + const initResp = findResponse(transport1.sent, 1) as { result: InitializeResult }; + const initSeq = initResp.result.serverSeq; + transport1.simulateClose(); + + // The session restores; the terminal cannot. + agentService.subscribe = async (resource, _clientId) => { + if (resource.toString() === deadTerminal) { + throw new Error('No agent for session: ' + deadTerminal); + } + if (!stateManager.getSnapshot(resource.toString())) { + stateManager.restoreSession(makeSessionSummary(), []); + } + return stateManager.getSnapshot(resource.toString())!; + }; + + const transport2 = new MockProtocolTransport(); + server.simulateConnection(transport2); + const firstRespPromise = waitForResponse(transport2, 1); + transport2.simulateMessage(request(1, 'reconnect', { + clientId: 'client-missing-debt', + lastSeenServerSeq: initSeq, + subscriptions: [sessionUri, deadTerminal], + })); + const firstResp = await firstRespPromise as { result: ReconnectResult }; + + // A second reconnect, now that the session has a baseline and the dead + // terminal has been reported missing, must be free to replay again. + transport2.simulateClose(); + const transport3 = new MockProtocolTransport(); + server.simulateConnection(transport3); + const secondRespPromise = waitForResponse(transport3, 1); + transport3.simulateMessage(request(1, 'reconnect', { + clientId: 'client-missing-debt', + lastSeenServerSeq: stateManager.serverSeq, + subscriptions: [sessionUri, deadTerminal], + })); + const secondResp = await secondRespPromise as { result: ReconnectResult }; + + assert.deepStrictEqual({ + firstType: firstResp.result.type, + firstMissing: firstResp.result.type === 'replay' ? firstResp.result.missing : [], + secondType: secondResp.result.type, + }, { + // Debt forced a snapshot the first time, as intended... + firstType: 'snapshot', + firstMissing: [], + // ...and the unrestorable terminal did not poison the next reconnect. + secondType: 'replay', + }); + }); + test('reconnect re-registers the reverse-RPC filesystem authority', async () => { // The server-side filesystem provider talks back to the client via // reverse-RPC (e.g. `resourceList`). If the authority is not From 40ad4eed3e843bdece6d8f65a873f220dcb8dec8 Mon Sep 17 00:00:00 2001 From: Martin Aeschlimann Date: Thu, 3 Sep 2026 11:07:47 +0200 Subject: [PATCH 09/13] Github Copilot change from Agent to Plan Mode triggers "Start Implementation" (#334202) --- src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts index b8c50f6e8d122..10014ff6858af 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts @@ -1762,10 +1762,12 @@ export class ChatWidget extends Disposable implements IChatWidget { // This ensures handoffs reflect what the response agent offers, regardless of mode picker state. // Fall back to the current mode picker for old sessions where modeInfo was not persisted. const modeInfo = lastItem.model.request?.modeInfo; - let responseMode: IChatMode | undefined; const modes = this.input.currentChatModesObs.get(); + let responseMode: IChatMode | undefined; if (modeInfo?.modeInstructions?.name) { responseMode = modes.findModeByName(modeInfo.modeInstructions.name); + } else if (modeInfo?.kind) { + responseMode = modes.findModeById(modeInfo.kind); } else { responseMode = this.input.currentModeObs.get(); } From 80c7fc21e4a46fc7efdcd7502c00e133c9a6968f Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Thu, 3 Sep 2026 11:08:35 +0200 Subject: [PATCH 10/13] label: restore generic home template parameters (#334204) label: support multiple home template parameters Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/baseAgentHostSessionsProvider.ts | 8 +-- .../services/label/common/labelService.ts | 24 ++++++--- .../services/label/test/browser/label.test.ts | 17 ++++--- .../label/test/common/mockLabelService.ts | 50 +++++++++---------- 4 files changed, 54 insertions(+), 45 deletions(-) diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index a66d2f5f901d4..49f058d851e42 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -2592,6 +2592,10 @@ class NewSession extends Disposable { // BaseAgentHostSessionsProvider — shared base for local and remote providers // ============================================================================ +function escapeResourceLabelPathSeparators(label: string): string { + return label.replaceAll('/', '\u2215').replaceAll('\\', '\u29F5'); +} + /** * Shared base class for the local and remote agent host sessions providers. * @@ -2606,10 +2610,6 @@ class NewSession extends Disposable { * URI-scheme mapping for session metadata, the agent-provider lookup, and * the browse UI. */ -function escapeResourceLabelPathSeparators(label: string): string { - return label.replaceAll('/', '\u2215').replaceAll('\\', '\u29F5'); -} - export abstract class BaseAgentHostSessionsProvider extends Disposable implements IAgentHostSessionsProvider { abstract readonly id: string; diff --git a/src/vs/workbench/services/label/common/labelService.ts b/src/vs/workbench/services/label/common/labelService.ts index ed523ee15c987..5f16657826844 100644 --- a/src/vs/workbench/services/label/common/labelService.ts +++ b/src/vs/workbench/services/label/common/labelService.ts @@ -559,13 +559,23 @@ export class LabelService extends Disposable implements ILabelService { private createTemplateFormatterRegistration(formatter: ResourceLabelTemplateFormatter): IHomeFormatterRegistration { const { home } = formatter; const homePath = home.path.length > 1 ? home.path.replace(/\/+$/, '') : home.path; - const lastSeparator = homePath.lastIndexOf('/'); - const parameterMatch = homeTemplateParameterRegex.exec(homePath.slice(lastSeparator + 1)); - const pathSegmentParameter = parameterMatch?.groups?.name; - const matcherPattern = pathSegmentParameter - ? `${escapeRegExpCharacters(homePath.slice(0, lastSeparator + 1))}(?<${pathSegmentParameter}>(?!\\.{1,2}(?:/|$))[^/]+)` - : escapeRegExpCharacters(homePath); - const isRootHome = pathSegmentParameter === undefined && (homePath === '' || homePath === '/'); + const parameterNames = new Set(); + const matcherPattern = homePath.split('/').map(segment => { + const parameterMatch = homeTemplateParameterRegex.exec(segment); + if (parameterMatch?.groups?.name) { + const parameterName = parameterMatch.groups.name; + if (parameterNames.has(parameterName)) { + throw new Error(`Duplicate resource label home template parameter: ${parameterName}`); + } + parameterNames.add(parameterName); + return `(?<${parameterName}>(?!\\.{1,2}(?:/|$))[^/]+)`; + } + if (segment.includes('${')) { + throw new Error(`Resource label home template parameters must occupy an entire path segment: ${segment}`); + } + return escapeRegExpCharacters(segment); + }).join('/'); + const isRootHome = homePath === '' || homePath === '/'; return { formatter, templateMatcher: new RegExp(`^${matcherPattern}${isRootHome ? '' : '(?=/|$)'}`), diff --git a/src/vs/workbench/services/label/test/browser/label.test.ts b/src/vs/workbench/services/label/test/browser/label.test.ts index a3c192fa8cff9..ec6bd9e50c3e5 100644 --- a/src/vs/workbench/services/label/test/browser/label.test.ts +++ b/src/vs/workbench/services/label/test/browser/label.test.ts @@ -277,22 +277,23 @@ suite('URI Label', () => { registration.dispose(); }); - test('URI home template parents are matched literally', () => { + test('URI home templates resolve all parameters', () => { const registration = labelService.registerFormatter({ - home: URI.parse('test://current/sessions/${literal}/${sessionId}'), + home: URI.parse('test://current/orgs/${org}/sessions/${sessionId}'), onDidChangeFormatting: Event.None, - formatting: () => ({ label: 'Session', separator: '/' }), + formatting: context => ({ + label: `${context.parameters.get('org')}/${context.parameters.get('sessionId')}`, + separator: '/', + }), }); - const resource = URI.parse('test://current/sessions/${literal}/session-id/file.md'); + const resource = URI.parse('test://current/orgs/acme/sessions/session-id/file.md'); assert.deepStrictEqual({ home: labelService.getUriHome(resource)?.path, label: labelService.getUriLabel(resource), - unrelatedHome: labelService.getUriHome(URI.parse('test://current/sessions/other/session-id/file.md')), }, { - home: '/sessions/${literal}/session-id', - label: 'Session/file.md', - unrelatedHome: undefined, + home: '/orgs/acme/sessions/session-id', + label: 'acme/session-id/file.md', }); registration.dispose(); diff --git a/src/vs/workbench/services/label/test/common/mockLabelService.ts b/src/vs/workbench/services/label/test/common/mockLabelService.ts index 32845dfd73290..f9e83694294df 100644 --- a/src/vs/workbench/services/label/test/common/mockLabelService.ts +++ b/src/vs/workbench/services/label/test/common/mockLabelService.ts @@ -7,6 +7,7 @@ import { Emitter } from '../../../../../base/common/event.js'; import { IDisposable } from '../../../../../base/common/lifecycle.js'; import { basename, normalize } from '../../../../../base/common/path.js'; import { isEqualOrParent } from '../../../../../base/common/resources.js'; +import { escapeRegExpCharacters } from '../../../../../base/common/strings.js'; import { URI } from '../../../../../base/common/uri.js'; import { IFormatterChangeEvent, ILabelService, ResourceLabelFormatter, ResourceLabelFormatting, ResourceLabelTemplateFormatter, Verbosity } from '../../../../../platform/label/common/label.js'; import { IWorkspace, IWorkspaceIdentifier } from '../../../../../platform/workspace/common/workspace.js'; @@ -85,34 +86,31 @@ export class MockLabelService implements ILabelService { continue; } const homePath = formatter.home.path.length > 1 ? formatter.home.path.replace(/\/+$/, '') : formatter.home.path; - const lastSeparator = homePath.lastIndexOf('/'); - const parameter = homeTemplateParameterRegex.exec(homePath.slice(lastSeparator + 1)); - const pathSegmentParameter = parameter?.groups?.name; - if (!pathSegmentParameter) { - if (!isEqualOrParent(resource, resource.with({ path: homePath }))) { - continue; + const parameterNames = new Set(); + const matcherPattern = homePath.split('/').map(segment => { + const parameterMatch = homeTemplateParameterRegex.exec(segment); + if (parameterMatch?.groups?.name) { + const parameterName = parameterMatch.groups.name; + if (parameterNames.has(parameterName)) { + throw new Error(`Duplicate resource label home template parameter: ${parameterName}`); + } + parameterNames.add(parameterName); + return `(?<${parameterName}>(?!\\.{1,2}(?:/|$))[^/]+)`; } - const home = resource.with({ path: homePath, query: null, fragment: null }); - const formatting = formatter.formatting({ resource, home, parameters: new Map() }); - if (formatting) { - candidate = { home, formatting }; - } - } else { - const prefix = homePath.slice(0, lastSeparator + 1); - if (!resource.path.startsWith(prefix)) { - continue; - } - const pathSegment = resource.path.slice(prefix.length).split('/')[0]; - if (!pathSegment || pathSegment === '.' || pathSegment === '..') { - continue; - } - const parameters = new Map(); - parameters.set(pathSegmentParameter, pathSegment); - const home = formatter.home.with({ path: `${prefix}${pathSegment}`, query: null, fragment: null }); - const formatting = formatter.formatting({ resource, home, parameters }); - if (formatting) { - candidate = { home, formatting }; + if (segment.includes('${')) { + throw new Error(`Resource label home template parameters must occupy an entire path segment: ${segment}`); } + return escapeRegExpCharacters(segment); + }).join('/'); + const isRootHome = homePath === '' || homePath === '/'; + const templateMatch = new RegExp(`^${matcherPattern}${isRootHome ? '' : '(?=/|$)'}`).exec(resource.path); + if (!templateMatch) { + continue; + } + const home = resource.with({ path: templateMatch[0], query: null, fragment: null }); + const formatting = formatter.formatting({ resource, home, parameters: new Map(Object.entries(templateMatch.groups ?? {})) }); + if (formatting) { + candidate = { home, formatting }; } } else if (formatter.scheme === resource.scheme && (!formatter.authority || formatter.authority === resource.authority) && isEqualOrParent(resource, resource.with({ path: formatter.home }))) { From 71bb7d0ed771eecc85f4ceb27b6035b246fe4c8c Mon Sep 17 00:00:00 2001 From: Ulugbek Abdullaev Date: Thu, 3 Sep 2026 14:16:04 +0500 Subject: [PATCH 11/13] Implement new badge design for automations in sidebar (#334091) * sessions: feat: show New badge for Automations Show a theme-aware first-use badge until the Automations view is activated. Preserve the decision across local profiles and cover migration, accessibility, recycled templates, and visual states. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Agent Host changes for ulugbekna/agents/sidebar-automation-new-badge-designs * sessions: fix: address Automations badge review feedback Fall back when treatment resolution fails, preserve high-contrast badge colors, cover observable accessible labels, and render the production Sessions header in fixtures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 922f1dc5-a262-4914-a0b4-7fed0cb79b4b * sessions: test: accept Automations badge screenshots Add the Linux-rendered blocks-CI hashes for the new badge style, narrow, running, and high-contrast fixtures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 922f1dc5-a262-4914-a0b4-7fed0cb79b4b --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 922f1dc5-a262-4914-a0b4-7fed0cb79b4b --- .../sessions/browser/automationsNewBadge.ts | 137 ++++++++++ .../sessions/browser/media/sessionsList.css | 38 +++ .../sessions/browser/sessions.contribution.ts | 16 +- .../sessions/browser/views/sessionsList.ts | 75 ++++-- .../sessions/browser/views/sessionsView.ts | 59 +++-- .../browser/views/sessionsViewActions.ts | 20 ++ .../test/browser/automationsNewBadge.test.ts | 236 ++++++++++++++++++ .../test/browser/sessionsList.test.ts | 109 ++++++++ .../sessions/sessionsList.fixture.ts | 174 ++++++++++++- .../blocks-ci-screenshots.md | 45 ++++ 10 files changed, 862 insertions(+), 47 deletions(-) create mode 100644 src/vs/sessions/contrib/sessions/browser/automationsNewBadge.ts create mode 100644 src/vs/sessions/contrib/sessions/test/browser/automationsNewBadge.test.ts diff --git a/src/vs/sessions/contrib/sessions/browser/automationsNewBadge.ts b/src/vs/sessions/contrib/sessions/browser/automationsNewBadge.ts new file mode 100644 index 0000000000000..62445fc9f2a4b --- /dev/null +++ b/src/vs/sessions/contrib/sessions/browser/automationsNewBadge.ts @@ -0,0 +1,137 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Disposable } from '../../../../base/common/lifecycle.js'; +import { autorun, derived, observableValue } from '../../../../base/common/observable.js'; +import { onUnexpectedError } from '../../../../base/common/errors.js'; +import { IConfigurationService, isConfigured } from '../../../../platform/configuration/common/configuration.js'; +import { ILogService } from '../../../../platform/log/common/log.js'; +import { observableMemento, type ObservableMemento } from '../../../../platform/observable/common/observableMemento.js'; +import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js'; +import { IAutomationService } from '../../../../workbench/contrib/chat/common/automations/automationService.js'; +import { IWorkbenchAssignmentService } from '../../../../workbench/services/assignment/common/assignmentService.js'; +import { ICustomViewService } from '../../../services/customView/browser/customViewService.js'; +import { AUTOMATIONS_CUSTOM_VIEW_ID } from './automationsConstants.js'; + +export const AUTOMATIONS_NEW_BADGE_SEEN_STORAGE_KEY = 'sessions.automations.newBadgeSeen'; +export const AUTOMATIONS_NEW_BADGE_STYLE_SETTING = 'sessions.automations.newBadgeStyle'; +export const AUTOMATIONS_NEW_BADGE_STYLE_TREATMENT = 'agentSessionsAutomationsNewBadgeStyle'; + +export type AutomationsNewBadgeStyle = 'accent' | 'soft' | 'outline'; + +const DEFAULT_AUTOMATIONS_NEW_BADGE_STYLE: AutomationsNewBadgeStyle = 'outline'; + +const automationsNewBadgeSeenMemento = observableMemento({ + key: AUTOMATIONS_NEW_BADGE_SEEN_STORAGE_KEY, + defaultValue: false, + toStorage: value => String(value), + fromStorage: value => value === 'true', +}); + +/** Owns the first-use state for the Automations shortcut badge. */ +export class AutomationsNewBadgeState extends Disposable { + + private readonly seen: ObservableMemento; + private readonly resolvedStyle = observableValue(this, undefined); + private observingActiveView = false; + private initializationPromise: Promise | undefined; + private styleRequest = 0; + readonly presentation = derived(this, reader => this.seen.read(reader) ? undefined : this.resolvedStyle.read(reader)); + readonly showNewBadge = derived(this, reader => this.presentation.read(reader) !== undefined); + + constructor( + @IAutomationService private readonly automationService: IAutomationService, + @ICustomViewService private readonly customViewService: ICustomViewService, + @IStorageService private readonly storageService: IStorageService, + @IWorkbenchAssignmentService private readonly assignmentService: IWorkbenchAssignmentService, + @IConfigurationService private readonly configurationService: IConfigurationService, + @ILogService private readonly logService: ILogService, + ) { + super(); + this.seen = this._register(automationsNewBadgeSeenMemento(StorageScope.APPLICATION, StorageTarget.MACHINE, storageService)); + } + + initialize(): Promise { + if (!this.observingActiveView) { + this.observingActiveView = true; + this._register(autorun(reader => { + if (this.customViewService.activeCustomView.read(reader)?.id === AUTOMATIONS_CUSTOM_VIEW_ID) { + this.markSeen(); + } + })); + } + if (!this.initializationPromise) { + this.initializationPromise = this.doInitialize(); + this._register(this.configurationService.onDidChangeConfiguration(event => { + if (event.affectsConfiguration(AUTOMATIONS_NEW_BADGE_STYLE_SETTING)) { + void this.updateStyle().catch(onUnexpectedError); + } + })); + this._register(this.assignmentService.onDidRefetchAssignments(() => { + void this.updateStyle().catch(onUnexpectedError); + })); + } + return this.initializationPromise; + } + + async reset(): Promise { + this.seen.set(false, undefined); + this.storageService.remove(AUTOMATIONS_NEW_BADGE_SEEN_STORAGE_KEY, StorageScope.APPLICATION); + this.resolvedStyle.set(undefined, undefined); + await this.updateStyle(); + } + + private async doInitialize(): Promise { + const hasPriorUse = this.seen.get() + || this.automationService.automations.get().length > 0 + || this.automationService.runs.get().length > 0; + if (hasPriorUse) { + this.markSeen(); + return; + } + + await this.updateStyle(); + } + + private async updateStyle(): Promise { + if (this.seen.get()) { + return; + } + + const request = ++this.styleRequest; + const inspection = this.configurationService.inspect(AUTOMATIONS_NEW_BADGE_STYLE_SETTING); + let value: string | undefined; + if (isConfigured(inspection)) { + value = inspection.value; + } else { + try { + value = await this.assignmentService.getTreatment(AUTOMATIONS_NEW_BADGE_STYLE_TREATMENT); + } catch (error) { + this.logService.warn(`[AutomationsNewBadgeState] Failed to resolve badge style treatment; using '${DEFAULT_AUTOMATIONS_NEW_BADGE_STYLE}'.`, error); + } + } + if (request !== this.styleRequest || this.seen.get()) { + return; + } + this.resolvedStyle.set(this.normalizeStyle(value), undefined); + } + + private normalizeStyle(value: string | undefined): AutomationsNewBadgeStyle { + if (value === undefined || value === DEFAULT_AUTOMATIONS_NEW_BADGE_STYLE) { + return DEFAULT_AUTOMATIONS_NEW_BADGE_STYLE; + } + if (value === 'accent' || value === 'soft') { + return value; + } + this.logService.warn(`[AutomationsNewBadgeState] Unsupported badge style treatment '${value}'; using '${DEFAULT_AUTOMATIONS_NEW_BADGE_STYLE}'.`); + return DEFAULT_AUTOMATIONS_NEW_BADGE_STYLE; + } + + private markSeen(): void { + if (!this.seen.get()) { + this.seen.set(true, undefined); + } + } +} diff --git a/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css b/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css index 75e27e16a6b68..73c9cf83b78f1 100644 --- a/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css +++ b/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css @@ -724,6 +724,39 @@ margin-right: 4px; } + .session-section-new-badge { + display: none; + align-items: center; + flex-shrink: 0; + height: var(--vscode-spacing-size160); + margin-left: var(--vscode-spacing-size40); + padding: 0 var(--vscode-spacing-size40); + box-sizing: border-box; + border: var(--vscode-strokeThickness) solid transparent; + border-radius: var(--vscode-cornerRadius-circle); + color: var(--vscode-foreground); + font-size: var(--vscode-fontSize-label3); + font-weight: var(--vscode-fontWeight-semiBold); + letter-spacing: 0.04em; + text-transform: uppercase; + white-space: nowrap; + pointer-events: none; + } + + .session-section-new-badge-accent { + border-color: var(--vscode-activityBarBadge-background); + background-color: var(--vscode-activityBarBadge-background); + color: var(--vscode-activityBarBadge-foreground); + } + + .session-section-new-badge-soft { + background-color: color-mix(in srgb, var(--vscode-foreground) 12%, transparent); + } + + .session-section-new-badge-outline { + border-color: var(--vscode-descriptionForeground); + } + .session-section-toolbar { flex-shrink: 0; margin-left: 4px; @@ -750,6 +783,11 @@ } } +.hc-black .session-section-new-badge, +.hc-light .session-section-new-badge { + border-color: var(--vscode-contrastBorder); +} + .monaco-list-row:hover .session-section .session-section-toolbar, .monaco-list-row.focused .session-section .session-section-toolbar, .monaco-list-row .session-section.dropdown-active .session-section-toolbar { diff --git a/src/vs/sessions/contrib/sessions/browser/sessions.contribution.ts b/src/vs/sessions/contrib/sessions/browser/sessions.contribution.ts index 38a00abdd3d8a..4a2518f6be12f 100644 --- a/src/vs/sessions/contrib/sessions/browser/sessions.contribution.ts +++ b/src/vs/sessions/contrib/sessions/browser/sessions.contribution.ts @@ -18,8 +18,9 @@ import { SessionsView, SessionsViewId } from './views/sessionsView.js'; import { AutomationsCustomViewContribution } from './views/automationsView.js'; import './views/sessionsViewActions.js'; import { KeyCode, KeyMod } from '../../../../base/common/keyCodes.js'; -import { Extensions as ConfigurationExtensions, IConfigurationRegistry } from '../../../../platform/configuration/common/configurationRegistry.js'; +import { ConfigurationScope, Extensions as ConfigurationExtensions, IConfigurationRegistry } from '../../../../platform/configuration/common/configurationRegistry.js'; import { SESSIONS_LIST_SHOW_EMPTY_DEFAULT_GROUPS_SETTING } from './views/sessionsList.js'; +import { AUTOMATIONS_NEW_BADGE_STYLE_SETTING, AUTOMATIONS_NEW_BADGE_STYLE_TREATMENT } from './automationsNewBadge.js'; import { SessionsMouseNavigationContribution } from './sessionsMouseNavigation.js'; import './sessionDetailsAction.js'; import { SessionsWindowNotifier } from './sessionsWindowNotifier.js'; @@ -69,6 +70,19 @@ Registry.as(ConfigurationExtensions.Configuration).regis default: true, experiment: { mode: 'auto' } }, + [AUTOMATIONS_NEW_BADGE_STYLE_SETTING]: { + type: 'string', + enum: ['accent', 'soft', 'outline'], + default: 'outline', + scope: ConfigurationScope.APPLICATION, + included: false, + tags: ['experimental'], + experiment: { + mode: 'auto', + name: AUTOMATIONS_NEW_BADGE_STYLE_TREATMENT, + }, + description: localize('sessions.automations.newBadgeStyle', "Controls the visual style of the Automations first-use badge."), + }, }, }); diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts index 109218084ae84..6f4d91d9ec37d 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts @@ -96,6 +96,7 @@ import { ChatAutomationsEnabledContext } from '../../../../../workbench/contrib/ import { IAutomationService } from '../../../../../workbench/contrib/chat/common/automations/automationService.js'; import { ICustomViewService } from '../../../../services/customView/browser/customViewService.js'; import { AUTOMATIONS_CUSTOM_VIEW_ID } from '../automationsConstants.js'; +import { AutomationsNewBadgeState, type AutomationsNewBadgeStyle } from '../automationsNewBadge.js'; import { Menus } from '../../../../browser/menus.js'; import { getSessionConversationStatusAriaLabel } from '../../../../browser/sessionConversationGroups.js'; import { getAgentMergeAwarePullRequestIcon, getSessionAgentMergeConfigurationObservable, ISessionAgentMergeConfiguration, isAgentMergePullRequestIcon } from '../../../../browser/sessionAgentMerge.js'; @@ -1266,6 +1267,7 @@ interface ISessionSectionTemplate extends ISessionHeaderTemplate { readonly icon: HTMLElement; readonly label: HTMLElement; readonly count: HTMLElement; + readonly newBadge: HTMLElement; readonly chevron: HTMLElement; readonly contextKeyService: IContextKeyService; readonly disposables: DisposableStore; @@ -1318,6 +1320,7 @@ export class SessionSectionRenderer implements ITreeRenderer, + private readonly automationNewBadgePresentation: IObservable, private readonly uriIdentityService: IUriIdentityService, private readonly customViewService: ICustomViewService, private readonly menuService: IMenuService, @@ -1339,6 +1342,9 @@ export class SessionSectionRenderer implements ITreeRenderer, _index: number, template: ISessionSectionTemplate): void { @@ -1397,6 +1403,12 @@ export class SessionSectionRenderer implements ITreeRenderer { const activeCustomView = this.customViewService.activeCustomView.read(reader); template.container.classList.toggle('active', activeCustomView?.id === AUTOMATIONS_CUSTOM_VIEW_ID); + const badgeStyle = this.automationNewBadgePresentation.read(reader); + template.newBadge.style.display = badgeStyle ? 'inline-flex' : 'none'; + template.newBadge.classList.toggle('session-section-new-badge-accent', badgeStyle === 'accent'); + template.newBadge.classList.toggle('session-section-new-badge-soft', badgeStyle === 'soft'); + template.newBadge.classList.toggle('session-section-new-badge-outline', badgeStyle === 'outline'); })); const statusIcon = template.elementDisposables.add(this.instantiationService.createInstance(SessionStatusIcon, template.icon)); template.elementDisposables.add(autorun(reader => { @@ -1747,6 +1764,7 @@ interface ISessionsAccessibilityProviderOptions { readonly isPinned: (session: ISession) => boolean; readonly isRenderedInCustomGroup?: (session: ISession) => boolean; readonly includeQuickChatInAriaLabel?: boolean; + readonly automationNewBadgeVisible?: IObservable; /** Mirrors {@link SessionItemRenderer}'s option of the same name — see there for rationale. */ readonly deriveStatusFromMainChat?: boolean; } @@ -1776,20 +1794,23 @@ class SessionsAccessibilityProvider { } if (isSessionSection(element)) { if (element.id === AUTOMATIONS_SECTION_ID) { - return this.automationStatus - ? derived(this, reader => { - switch (this.automationStatus?.read(reader)) { - case SessionStatus.NeedsInput: - return localize('automationsNeedsInputAria', "{0}, run needs input", element.label); - case SessionStatus.InProgress: - return localize('automationsActiveAria', "{0}, run in progress", element.label); - case SessionStatus.Completed: - return localize('automationsUnreadRunAria', "{0}, unread run", element.label); - default: - return element.label; - } - }) - : element.label; + return derived(this, reader => { + let label = element.label; + switch (this.automationStatus?.read(reader)) { + case SessionStatus.NeedsInput: + label = localize('automationsNeedsInputAria', "{0}, run needs input", element.label); + break; + case SessionStatus.InProgress: + label = localize('automationsActiveAria', "{0}, run in progress", element.label); + break; + case SessionStatus.Completed: + label = localize('automationsUnreadRunAria', "{0}, unread run", element.label); + break; + } + return this.options?.automationNewBadgeVisible?.read(reader) + ? localize('automationsNewFeatureAria', "{0}, new feature", label) + : label; + }); } return `${element.label}, ${element.sessions.length}`; } @@ -2323,6 +2344,7 @@ export class SessionsList extends Disposable implements ISessionsList { */ private readonly chatApprovalHeightReconcile = this._register(new MutableDisposable()); private readonly automationSessions = observableValue(this, []); + private readonly automationsNewBadgeState: AutomationsNewBadgeState; /** * Session IDs whose hierarchy indent/connector guides should be visible: * the union of the currently-hovered session (if any) and every session @@ -2396,6 +2418,13 @@ export class SessionsList extends Disposable implements ISessionsList { get element(): HTMLElement { return this.listContainer; } + async resetAutomationsNewBadge(): Promise { + if (this.customViewService.activeCustomView.get()?.id === AUTOMATIONS_CUSTOM_VIEW_ID) { + this.customViewService.hideCustomView(); + } + await this.automationsNewBadgeState.reset(); + } + constructor( container: HTMLElement, private readonly options: ISessionsListControlOptions, @@ -2424,6 +2453,7 @@ export class SessionsList extends Disposable implements ISessionsList { @IPreferencesService private readonly preferencesService: IPreferencesService, ) { super(); + this.automationsNewBadgeState = this._register(instantiationService.createInstance(AutomationsNewBadgeState)); // Load excluded session types from storage this.excludedSessionTypes = this.loadExcludedSessionTypes(); @@ -2518,7 +2548,18 @@ export class SessionsList extends Disposable implements ISessionsList { this.tree.setFocus([element], event); this.tree.setSelection([element], event); }; - const sectionRenderer = new SessionSectionRenderer(true /* hideSectionCount */, selectHeader, instantiationService, contextKeyService, this.automationService, this.automationSessions, this.uriIdentityService, this.customViewService, this.menuService); + const sectionRenderer = new SessionSectionRenderer( + true /* hideSectionCount */, + selectHeader, + instantiationService, + contextKeyService, + this.automationService, + this.automationSessions, + this.automationsNewBadgeState.presentation, + this.uriIdentityService, + this.customViewService, + this.menuService, + ); this._sectionRenderer = sectionRenderer; const groupRenderer = new SessionGroupRenderer({ commitEdit: (group, name) => this.commitGroupEdit(group, name), @@ -2561,6 +2602,7 @@ export class SessionsList extends Disposable implements ISessionsList { isPinned: session => this.isSessionPinned(session), isRenderedInCustomGroup: session => this.isRenderedInCustomGroup(session), deriveStatusFromMainChat: true, + automationNewBadgeVisible: this.automationsNewBadgeState.showNewBadge, }), dnd: this._register(new SessionsListDragAndDrop({ isReorderable: session => this.isReorderable(session), @@ -3200,6 +3242,7 @@ export class SessionsList extends Disposable implements ISessionsList { }; if (this.contextKeyService.getContextKeyValue(ChatAutomationsEnabledContext.key)) { + void this.automationsNewBadgeState.initialize().catch(onUnexpectedError); children.push(renderSection({ id: AUTOMATIONS_SECTION_ID, label: localize('automations', "Automations"), sessions: [] })); } diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsView.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsView.ts index dc530fc8137b8..ce50eaef16dfa 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/sessionsView.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsView.ts @@ -60,6 +60,40 @@ export const SessionsViewGroupingContext = new RawContextKey('sessionsVi export const SessionsViewSortingContext = new RawContextKey('sessionsViewPane.sorting', SessionsSorting.Created); export const IsWorkspaceGroupCappedContext = new RawContextKey('sessionsViewPane.workspaceGroupCapped', true); +export interface ISessionsHeaderElements { + readonly row: HTMLElement; + readonly label: HTMLElement; + readonly actions: HTMLElement; + readonly toolbar: MenuWorkbenchToolBar | undefined; +} + +export function renderSessionsHeader( + parent: HTMLElement, + phoneLayout: boolean, + instantiationService: IInstantiationService, + contextKeyService: IContextKeyService, + disposables: DisposableStore, +): ISessionsHeaderElements { + const row = DOM.append(parent, $('.agent-sessions-header-row')); + const label = DOM.append(row, $('.agent-sessions-header-label')); + const actions = DOM.append(row, $('.agent-sessions-header-actions')); + let toolbar: MenuWorkbenchToolBar | undefined; + + if (!phoneLayout) { + label.textContent = localize('sessionsHeader', "Sessions"); + const scopedInstantiationService = disposables.add(instantiationService.createChild(new ServiceCollection([IContextKeyService, contextKeyService]))); + toolbar = disposables.add(scopedInstantiationService.createInstance(MenuWorkbenchToolBar, actions, Menus.SidebarSessionsHeader, { + hiddenItemStrategy: HiddenItemStrategy.NoHide, + telemetrySource: 'sessionsView.header', + toolbarOptions: { primaryGroup: () => true }, + })); + } else { + row.classList.add('phone-layout-empty'); + } + + return { row, label, actions, toolbar }; +} + export class SessionsView extends ViewPane { private viewPaneContainer: HTMLElement | undefined; @@ -156,32 +190,15 @@ export class SessionsView extends ViewPane { // Sessions content container const sessionsContent = DOM.append(sessionsSection, $('.agent-sessions-content')); - // Header row: "Sessions" label (left) + compact "New" button (right) - const headerRow = this.headerRow = DOM.append(sessionsContent, $('.agent-sessions-header-row')); - const headerLabel = this.headerLabel = DOM.append(headerRow, $('.agent-sessions-header-label')); - - const headerActions = this.headerActions = DOM.append(headerRow, $('.agent-sessions-header-actions')); - // On phone, the desktop header content (label + new button + filter/find toolbar) // is hidden in favor of the mobile filter chip row + the (+) button in the // MobileTitlebarPart. We still create the row container because the find // widget mounts inside it. const phoneLayout = isPhoneLayout(this.layoutService); - if (!phoneLayout) { - headerLabel.textContent = localize('sessionsHeader', "Sessions"); - - // Header actions (visual order: New, Filter, Search). The "New" button is - // contributed to Menus.SidebarSessionsHeader and rendered as a compact pill - // by NewSessionActionViewItem. - const scopedInstantiationService = this._register(this.instantiationService.createChild(new ServiceCollection([IContextKeyService, this.scopedContextKeyService]))); - this._register(scopedInstantiationService.createInstance(MenuWorkbenchToolBar, headerActions, Menus.SidebarSessionsHeader, { - hiddenItemStrategy: HiddenItemStrategy.NoHide, - telemetrySource: 'sessionsView.header', - toolbarOptions: { primaryGroup: () => true }, - })); - } else { - headerRow.classList.add('phone-layout-empty'); - } + const header = renderSessionsHeader(sessionsContent, phoneLayout, this.instantiationService, this.scopedContextKeyService, this._register(new DisposableStore())); + const headerRow = this.headerRow = header.row; + this.headerLabel = header.label; + this.headerActions = header.actions; // Container for the tree's find widget (toggled by the toolbar's Find action) const findWidgetContainer = this.findWidgetContainer = DOM.append(headerRow, $('.agent-sessions-find-widget-container')); diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts index d878e9c6f118a..4fcb2e70c0d84 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts @@ -9,10 +9,12 @@ import { KeyChord, KeyCode, KeyMod } from '../../../../../base/common/keyCodes.j import { Disposable, DisposableStore } from '../../../../../base/common/lifecycle.js'; import { isMobile, isWeb } from '../../../../../base/common/platform.js'; import { localize, localize2 } from '../../../../../nls.js'; +import { Categories } from '../../../../../platform/action/common/actionCommonCategories.js'; import { Action2, MenuId, MenuRegistry, registerAction2 } from '../../../../../platform/actions/common/actions.js'; import { CommandsRegistry, ICommandService } from '../../../../../platform/commands/common/commands.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { ContextKeyExpr, IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js'; +import { IsDevelopmentContext } from '../../../../../platform/contextkey/common/contextkeys.js'; import { IDialogService } from '../../../../../platform/dialogs/common/dialogs.js'; import { ServicesAccessor } from '../../../../../platform/instantiation/common/instantiation.js'; import { IQuickInputService } from '../../../../../platform/quickinput/common/quickInput.js'; @@ -40,6 +42,7 @@ import { IWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase import { registerExternalSessionsFilterMenu } from '../../../../../workbench/contrib/chat/browser/agentSessions/externalSessionsFilterMenu.js'; import { ICustomViewService } from '../../../../services/customView/browser/customViewService.js'; import { IAutomationService } from '../../../../../workbench/contrib/chat/common/automations/automationService.js'; +import { ChatAutomationsEnabledContext } from '../../../../../workbench/contrib/chat/common/automations/automationsEnabled.js'; import { AUTOMATIONS_CUSTOM_VIEW_ID } from '../automationsConstants.js'; const CLOSE_SESSION_COMMAND_ID = 'sessionsViewPane.closeSession'; @@ -1336,6 +1339,23 @@ registerAction2(class ManageAutomationsAction extends Action2 { } }); +registerAction2(class ResetAutomationsNewBadgeAction extends Action2 { + constructor() { + super({ + id: 'sessions.developer.resetAutomationsNewBadge', + title: localize2('resetAutomationsNewBadge', "Reset Automations New Badge"), + category: Categories.Developer, + f1: true, + precondition: ContextKeyExpr.and(IsDevelopmentContext, IsSessionsWindowContext, ChatAutomationsEnabledContext), + }); + } + + override async run(accessor: ServicesAccessor): Promise { + const view = await accessor.get(IViewsService).openView(SessionsViewId, false); + await view?.sessionsControl?.resetAutomationsNewBadge(); + } +}); + const MARK_ALL_AUTOMATION_RUNS_READ_COMMAND_ID = 'sessionsView.markAllAutomationRunsRead'; registerAction2(class MarkAllAutomationRunsReadAction extends Action2 { diff --git a/src/vs/sessions/contrib/sessions/test/browser/automationsNewBadge.test.ts b/src/vs/sessions/contrib/sessions/test/browser/automationsNewBadge.test.ts new file mode 100644 index 0000000000000..a95823f476db6 --- /dev/null +++ b/src/vs/sessions/contrib/sessions/test/browser/automationsNewBadge.test.ts @@ -0,0 +1,236 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { Emitter } from '../../../../../base/common/event.js'; +import { observableValue } from '../../../../../base/common/observable.js'; +import { mock, upcastPartial } from '../../../../../base/test/common/mock.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import type { IConfigurationChangeEvent } from '../../../../../platform/configuration/common/configuration.js'; +import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { NullLogService } from '../../../../../platform/log/common/log.js'; +import { InMemoryStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; +import type { IAutomationDescriptor, IAutomationRun } from '../../../../../workbench/contrib/chat/common/automations/automation.js'; +import { IAutomationService } from '../../../../../workbench/contrib/chat/common/automations/automationService.js'; +import { IWorkbenchAssignmentService } from '../../../../../workbench/services/assignment/common/assignmentService.js'; +import type { ICustomViewDescriptor } from '../../../../services/customView/browser/customView.js'; +import { ICustomViewService } from '../../../../services/customView/browser/customViewService.js'; +import { AUTOMATIONS_CUSTOM_VIEW_ID } from '../../browser/automationsConstants.js'; +import { AUTOMATIONS_NEW_BADGE_SEEN_STORAGE_KEY, AUTOMATIONS_NEW_BADGE_STYLE_SETTING, AUTOMATIONS_NEW_BADGE_STYLE_TREATMENT, AutomationsNewBadgeState, type AutomationsNewBadgeStyle } from '../../browser/automationsNewBadge.js'; + +suite('AutomationsNewBadgeState', () => { + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + + class TestAssignmentService extends mock() { + readonly treatments: string[] = []; + override readonly onDidRefetchAssignments; + + constructor( + private readonly style: AutomationsNewBadgeStyle | undefined, + refetchAssignments: Emitter, + private readonly error?: Error, + ) { + super(); + this.onDidRefetchAssignments = refetchAssignments.event; + } + + override async getTreatment(name: string): Promise { + this.treatments.push(name); + if (this.error) { + throw this.error; + } + return this.style as T | undefined; + } + } + + function createState(options: { + readonly automations?: readonly IAutomationDescriptor[]; + readonly runs?: readonly IAutomationRun[]; + readonly activeView?: ICustomViewDescriptor; + readonly seen?: boolean; + readonly style?: AutomationsNewBadgeStyle; + readonly configuredStyle?: AutomationsNewBadgeStyle; + readonly treatmentError?: Error; + } = {}) { + const storageService = disposables.add(new InMemoryStorageService()); + if (options.seen) { + storageService.store(AUTOMATIONS_NEW_BADGE_SEEN_STORAGE_KEY, true, StorageScope.APPLICATION, StorageTarget.MACHINE); + } + const automations = observableValue(disposables, options.automations ?? []); + const runs = observableValue(disposables, options.runs ?? []); + const activeView = observableValue(disposables, options.activeView); + const automationService = new class extends mock() { + override readonly automations = automations; + override readonly runs = runs; + }; + const customViewService = new class extends mock() { + override readonly activeCustomView = activeView; + }; + const refetchAssignments = disposables.add(new Emitter()); + const assignmentService = new TestAssignmentService(options.style, refetchAssignments, options.treatmentError); + const configurationService = new TestConfigurationService(); + if (options.configuredStyle) { + void configurationService.setUserConfiguration(AUTOMATIONS_NEW_BADGE_STYLE_SETTING, options.configuredStyle); + } + const state = disposables.add(new AutomationsNewBadgeState( + automationService, + customViewService, + storageService, + assignmentService, + configurationService, + new NullLogService(), + )); + return { state, storageService, automations, runs, activeView, assignmentService, configurationService, refetchAssignments }; + } + + test('keeps the resolved style stable until Automations is activated', async () => { + const { state, storageService, automations, runs, activeView } = createState(); + + await state.initialize(); + automations.set([upcastPartial({ id: 'late-automation' })], undefined); + runs.set([upcastPartial({ id: 'late-run' })], undefined); + const beforeActivation = { + showNewBadge: state.showNewBadge.get(), + style: state.presentation.get(), + stored: storageService.get(AUTOMATIONS_NEW_BADGE_SEEN_STORAGE_KEY, StorageScope.APPLICATION), + }; + + activeView.set(upcastPartial({ id: AUTOMATIONS_CUSTOM_VIEW_ID }), undefined); + const afterActivation = { + showNewBadge: state.showNewBadge.get(), + stored: storageService.get(AUTOMATIONS_NEW_BADGE_SEEN_STORAGE_KEY, StorageScope.APPLICATION), + }; + + assert.deepStrictEqual({ beforeActivation, afterActivation }, { + beforeActivation: { showNewBadge: true, style: 'outline', stored: undefined }, + afterActivation: { + showNewBadge: false, + stored: 'true', + }, + }); + }); + + test('resolves accent, soft, and outline from the hidden treatment', async () => { + const snapshots = []; + for (const style of ['accent', 'soft', 'outline'] as const) { + const fixture = createState({ style }); + await fixture.state.initialize(); + snapshots.push({ + style: fixture.state.presentation.get(), + treatments: fixture.assignmentService.treatments, + }); + } + + assert.deepStrictEqual(snapshots, [ + { style: 'accent', treatments: [AUTOMATIONS_NEW_BADGE_STYLE_TREATMENT] }, + { style: 'soft', treatments: [AUTOMATIONS_NEW_BADGE_STYLE_TREATMENT] }, + { style: 'outline', treatments: [AUTOMATIONS_NEW_BADGE_STYLE_TREATMENT] }, + ]); + }); + + test('falls back to outline when treatment resolution fails', async () => { + const fixture = createState({ treatmentError: new Error('Unavailable') }); + + await fixture.state.initialize(); + + assert.deepStrictEqual({ + style: fixture.state.presentation.get(), + treatments: fixture.assignmentService.treatments, + }, { + style: 'outline', + treatments: [AUTOMATIONS_NEW_BADGE_STYLE_TREATMENT], + }); + }); + + test('lets the hidden setting override and live-update the treatment', async () => { + const fixture = createState({ style: 'outline', configuredStyle: 'soft' }); + await fixture.state.initialize(); + const initial = fixture.state.presentation.get(); + + await fixture.configurationService.setUserConfiguration(AUTOMATIONS_NEW_BADGE_STYLE_SETTING, 'accent'); + fixture.configurationService.onDidChangeConfigurationEmitter.fire(upcastPartial({ + affectsConfiguration: (key: string) => key === AUTOMATIONS_NEW_BADGE_STYLE_SETTING, + })); + + assert.deepStrictEqual({ + initial, + updated: fixture.state.presentation.get(), + treatments: fixture.assignmentService.treatments, + }, { + initial: 'soft', + updated: 'accent', + treatments: [], + }); + }); + + test('resets seen state for development even when prior Automation evidence exists', async () => { + const fixture = createState({ + automations: [upcastPartial({ id: 'existing-automation' })], + style: 'accent', + }); + await fixture.state.initialize(); + + await fixture.state.reset(); + + assert.deepStrictEqual({ + showNewBadge: fixture.state.showNewBadge.get(), + style: fixture.state.presentation.get(), + stored: fixture.storageService.get(AUTOMATIONS_NEW_BADGE_SEEN_STORAGE_KEY, StorageScope.APPLICATION), + }, { + showNewBadge: true, + style: 'accent', + stored: undefined, + }); + }); + + test('suppresses the badge when synchronous Automation evidence exists', async () => { + const definition = createState({ + automations: [upcastPartial({ id: 'existing-automation' })], + }); + const run = createState({ + runs: [upcastPartial({ id: 'existing-run' })], + }); + + await definition.state.initialize(); + await run.state.initialize(); + + assert.deepStrictEqual({ + definition: { + showNewBadge: definition.state.showNewBadge.get(), + stored: definition.storageService.get(AUTOMATIONS_NEW_BADGE_SEEN_STORAGE_KEY, StorageScope.APPLICATION), + }, + run: { + showNewBadge: run.state.showNewBadge.get(), + stored: run.storageService.get(AUTOMATIONS_NEW_BADGE_SEEN_STORAGE_KEY, StorageScope.APPLICATION), + }, + }, { + definition: { showNewBadge: false, stored: 'true' }, + run: { showNewBadge: false, stored: 'true' }, + }); + }); + + test('honors persisted and restored seen state before the row renders', async () => { + const persisted = createState({ seen: true }); + const restored = createState({ + activeView: upcastPartial({ id: AUTOMATIONS_CUSTOM_VIEW_ID }), + }); + + await persisted.state.initialize(); + await restored.state.initialize(); + + assert.deepStrictEqual({ + persisted: { + showNewBadge: persisted.state.showNewBadge.get(), + }, + restored: { + showNewBadge: restored.state.showNewBadge.get(), + stored: restored.storageService.get(AUTOMATIONS_NEW_BADGE_SEEN_STORAGE_KEY, StorageScope.APPLICATION), + }, + }, { + persisted: { showNewBadge: false }, + restored: { showNewBadge: false, stored: 'true' }, + }); + }); +}); diff --git a/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts b/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts index 526c469fc604b..541fc0caec372 100644 --- a/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts +++ b/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts @@ -19,6 +19,7 @@ import { IMenuService } from '../../../../../platform/actions/common/actions.js' import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; import { ContextKeyService } from '../../../../../platform/contextkey/browser/contextKeyService.js'; import { IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js'; +import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { ILabelService } from '../../../../../platform/label/common/label.js'; @@ -27,12 +28,14 @@ import { IUriIdentityService } from '../../../../../platform/uriIdentity/common/ import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; import { IAutomationRun } from '../../../../../workbench/contrib/chat/common/automations/automation.js'; import { IAutomationService } from '../../../../../workbench/contrib/chat/common/automations/automationService.js'; +import { ChatAutomationsEnabledContext } from '../../../../../workbench/contrib/chat/common/automations/automationsEnabled.js'; import { IPreferencesService, IOpenSettingsOptions } from '../../../../../workbench/services/preferences/common/preferences.js'; import { AgentMergeSessionState } from '../../../../../platform/agentHost/common/agentMerge.js'; import { getSessionChatDragData, isSessionChatDrag, SessionsDataTransfers } from '../../../../browser/dnd.js'; import { IsPhoneLayoutContext } from '../../../../common/contextkeys.js'; import { IAgentHostSessionsProvider, LOCAL_AGENT_HOST_PROVIDER_ID } from '../../../../common/agentHostSessionsProvider.js'; import { ICustomViewService } from '../../../../services/customView/browser/customViewService.js'; +import type { ICustomViewDescriptor } from '../../../../services/customView/browser/customView.js'; import { ISessionsListModelService } from '../../../../services/sessions/browser/sessionsListModelService.js'; import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; import { ChatInteractivity, ChatOriginKind, IChat, ISession, SessionStatus } from '../../../../services/sessions/common/session.js'; @@ -45,6 +48,8 @@ import { getSessionSummaryHoverData } from '../../browser/sessionHoverContent.js import { createListHarness, createTestSession } from './sessionsListTestUtils.js'; import '../../browser/views/sessionsViewActions.js'; import { computePullRequestIcon, GitHubPullRequestState } from '../../../github/common/types.js'; +import { AUTOMATIONS_CUSTOM_VIEW_ID } from '../../browser/automationsConstants.js'; +import { AUTOMATIONS_NEW_BADGE_STYLE_SETTING } from '../../browser/automationsNewBadge.js'; function createSession(id: string, opts: { workspaceLabel?: string; @@ -119,6 +124,7 @@ suite('Sessions - SessionsList', () => { contextKeyService, automationService, constObservable([]), + constObservable(undefined), new class extends mock() { override readonly extUri = new ExtUri(() => true); }, @@ -171,6 +177,7 @@ suite('Sessions - SessionsList', () => { contextKeyService, automationService, constObservable([]), + constObservable(undefined), new class extends mock() { override readonly extUri = new ExtUri(() => true); }, @@ -201,6 +208,106 @@ suite('Sessions - SessionsList', () => { }); }); + test('renders the new badge only on the Automations section when templates are recycled', () => { + const instantiationService = disposables.add(new TestInstantiationService()); + instantiationService.stubInstance(MenuWorkbenchToolBar, new class extends mock() { + override set context(_context: unknown) { } + override dispose(): void { } + }); + instantiationService.stub(IAccessibilityService, new class extends TestAccessibilityService { + override isMotionReduced(): boolean { return false; } + }()); + instantiationService.stub(ISessionsListModelService, new class extends mock() { }); + const contextKeyService = disposables.add(new ContextKeyService(new TestConfigurationService())); + const automationService = new class extends mock() { + override readonly runs = constObservable([]); + }; + const renderer = new SessionSectionRenderer( + true, + () => { }, + instantiationService, + contextKeyService, + automationService, + constObservable([]), + constObservable('outline'), + new class extends mock() { + override readonly extUri = new ExtUri(() => true); + }, + new class extends mock() { + override readonly activeCustomView = constObservable(undefined); + }, + new class extends mock() { }, + ); + const container = document.createElement('div'); + const template = renderer.renderTemplate(container); + disposables.add(template.disposables); + + renderer.renderElement(upcastPartial[0]>({ + element: { id: 'automations', label: 'Automations', sessions: [] }, + collapsible: false, + collapsed: false, + }), 0, template); + const automationSnapshot = { + text: template.newBadge.textContent, + display: template.newBadge.style.display, + ariaHidden: template.newBadge.getAttribute('aria-hidden'), + }; + + renderer.renderElement(upcastPartial[0]>({ + element: { id: 'workspace:test', label: 'Test', sessions: [] }, + collapsible: true, + collapsed: false, + }), 0, template); + + assert.deepStrictEqual({ + automationSnapshot, + recycledDisplay: template.newBadge.style.display, + recycledShortcutClass: template.container.classList.contains('session-section-shortcut'), + }, { + automationSnapshot: { + text: 'New', + display: 'inline-flex', + ariaHidden: 'true', + }, + recycledDisplay: 'none', + recycledShortcutClass: false, + }); + }); + + test('updates the Automations row accessible label when the new badge is dismissed', () => { + const activeCustomView = observableValue(disposables, undefined); + const harness = createListHarness(disposables, [], instantiationService => { + ChatAutomationsEnabledContext.bindTo(instantiationService.get(IContextKeyService)).set(true); + void (instantiationService.get(IConfigurationService) as TestConfigurationService).setUserConfiguration(AUTOMATIONS_NEW_BADGE_STYLE_SETTING, 'outline'); + instantiationService.stub(IAutomationService, new class extends mock() { + override readonly automations = constObservable([]); + override readonly runs = constObservable([]); + }); + instantiationService.stub(ICustomViewService, new class extends mock() { + override readonly activeCustomView = activeCustomView; + }); + }); + const container = harness.createContainer(); + const list = harness.store.add(harness.instantiationService.createInstance(SessionsList, container, { + grouping: () => SessionsGrouping.Date, + sorting: () => SessionsSorting.Created, + onSessionOpen: () => { }, + })); + list.layout(300, 400); + const row = container.querySelector('.monaco-list-row'); + const before = row?.getAttribute('aria-label'); + + activeCustomView.set(upcastPartial({ id: AUTOMATIONS_CUSTOM_VIEW_ID }), undefined); + + assert.deepStrictEqual({ + before, + after: row?.getAttribute('aria-label'), + }, { + before: 'Automations, new feature', + after: 'Automations', + }); + }); + test('derives terminal automation status from the supplied session snapshot', () => { const session = createSession('automation', { isRead: false, @@ -234,6 +341,7 @@ suite('Sessions - SessionsList', () => { new class extends mock() { }, automationService, automationSessions, + constObservable(undefined), uriIdentityService, new class extends mock() { }, new class extends mock() { }, @@ -301,6 +409,7 @@ suite('Sessions - SessionsList', () => { new class extends mock() { }, automationService, constObservable([runningSession, needsInputSession]), + constObservable(undefined), uriIdentityService, new class extends mock() { }, new class extends mock() { }, diff --git a/src/vs/workbench/test/browser/componentFixtures/sessions/sessionsList.fixture.ts b/src/vs/workbench/test/browser/componentFixtures/sessions/sessionsList.fixture.ts index 4600407be5705..b6e02d641eb3a 100644 --- a/src/vs/workbench/test/browser/componentFixtures/sessions/sessionsList.fixture.ts +++ b/src/vs/workbench/test/browser/componentFixtures/sessions/sessionsList.fixture.ts @@ -4,21 +4,26 @@ *--------------------------------------------------------------------------------------------*/ import { Codicon } from '../../../../../base/common/codicons.js'; -import { Event } from '../../../../../base/common/event.js'; +import * as DOM from '../../../../../base/browser/dom.js'; +import { Emitter, Event } from '../../../../../base/common/event.js'; import { IMarkdownString, MarkdownString } from '../../../../../base/common/htmlContent.js'; -import { constObservable, IObservable } from '../../../../../base/common/observable.js'; +import { Disposable, IDisposable, toDisposable } from '../../../../../base/common/lifecycle.js'; +import { constObservable, IObservable, observableValue } from '../../../../../base/common/observable.js'; import { ExtUri } from '../../../../../base/common/resources.js'; import { ThemeIcon, themeColorFromId } from '../../../../../base/common/themables.js'; import { URI } from '../../../../../base/common/uri.js'; import { mock } from '../../../../../base/test/common/mock.js'; +import { IActionViewItemFactory, IActionViewItemService } from '../../../../../platform/actions/browser/actionViewItemService.js'; import { IListService, ListService } from '../../../../../platform/list/browser/listService.js'; import { IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js'; +import { ICommandService } from '../../../../../platform/commands/common/commands.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; -import { IMenu, IMenuService, MenuItemAction } from '../../../../../platform/actions/common/actions.js'; +import { IMenu, IMenuService, MenuId, MenuItemAction } from '../../../../../platform/actions/common/actions.js'; import { EditorMarkdownCodeBlockRenderer } from '../../../../../editor/browser/widget/markdownRenderer/browser/editorMarkdownCodeBlockRenderer.js'; import { IMarkdownRendererService, MarkdownRendererService } from '../../../../../platform/markdown/browser/markdownRenderer.js'; import { IAgentHostConnectionsService } from '../../../../../platform/agentHost/common/agentHostConnectionsService.js'; +import { IProductService } from '../../../../../platform/product/common/productService.js'; import { IUriIdentityService } from '../../../../../platform/uriIdentity/common/uriIdentity.js'; // eslint-disable-next-line local/code-import-patterns import { IAgentHostFilterService } from '../../../../../sessions/services/agentHostFilter/common/agentHostFilter.js'; @@ -35,25 +40,64 @@ import { ISessionsService } from '../../../../../sessions/services/sessions/brow // eslint-disable-next-line local/code-import-patterns import { ICustomViewService } from '../../../../../sessions/services/customView/browser/customViewService.js'; // eslint-disable-next-line local/code-import-patterns +import { Menus } from '../../../../../sessions/browser/menus.js'; +// eslint-disable-next-line local/code-import-patterns import { IChat, ISession, ISessionChangesSummary, ISessionFolder, ISessionWorkspace, SessionStatus, ChatInteractivity } from '../../../../../sessions/services/sessions/common/session.js'; // eslint-disable-next-line local/code-import-patterns import { IActiveSession, ISessionsManagementService } from '../../../../../sessions/services/sessions/common/sessionsManagement.js'; // eslint-disable-next-line local/code-import-patterns import { SessionsGrouping, SessionsList, SessionsSorting } from '../../../../../sessions/contrib/sessions/browser/views/sessionsList.js'; // eslint-disable-next-line local/code-import-patterns +import { AUTOMATIONS_NEW_BADGE_STYLE_SETTING, type AutomationsNewBadgeStyle } from '../../../../../sessions/contrib/sessions/browser/automationsNewBadge.js'; +// eslint-disable-next-line local/code-import-patterns +import { renderSessionsHeader } from '../../../../../sessions/contrib/sessions/browser/views/sessionsView.js'; +// eslint-disable-next-line local/code-import-patterns +import { NewSessionActionViewItemContribution } from '../../../../../sessions/contrib/sessions/browser/sessionsActions.js'; +// eslint-disable-next-line local/code-import-patterns +import { NEW_SESSION_ACTION_ID } from '../../../../../sessions/contrib/chat/common/constants.js'; +// eslint-disable-next-line local/code-import-patterns import { IsPhoneLayoutContext } from '../../../../../sessions/common/contextkeys.js'; import { AgentSessionApprovalKind, AgentSessionApprovalModel, IAgentSessionApprovalInfo } from '../../../../contrib/chat/browser/agentSessions/agentSessionApprovalModel.js'; import { IAgentSessionsService } from '../../../../contrib/chat/browser/agentSessions/agentSessionsService.js'; import { IAgentSession, IAgentSessionsModel } from '../../../../contrib/chat/browser/agentSessions/agentSessionsModel.js'; import { IAutomationService } from '../../../../contrib/chat/common/automations/automationService.js'; +import type { IAutomationRun } from '../../../../contrib/chat/common/automations/automation.js'; +import { ChatAutomationsEnabledContext } from '../../../../contrib/chat/common/automations/automationsEnabled.js'; import { IChatService } from '../../../../contrib/chat/common/chatService/chatService.js'; import { IChatModel } from '../../../../contrib/chat/common/model/chatModel.js'; import { IVoicePlaybackService } from '../../../../contrib/chat/common/voicePlaybackService.js'; import { IWorkbenchAssignmentService } from '../../../../services/assignment/common/assignmentService.js'; +import { TestProductService } from '../../../common/workbenchTestServices.js'; import { ComponentFixtureContext, createEditorServices, defineComponentFixture, defineThemedFixtureGroup, registerWorkbenchServices } from '../fixtureUtils.js'; // eslint-disable-next-line local/code-import-patterns import '../../../../../sessions/contrib/sessions/browser/media/sessionsList.css'; +// eslint-disable-next-line local/code-import-patterns +import '../../../../../sessions/contrib/sessions/browser/media/sessionsViewPane.css'; +// eslint-disable-next-line local/code-import-patterns +import '../../../../../sessions/contrib/sessions/browser/media/newSessionActionViewItem.css'; + +class FixtureActionViewItemService extends Disposable implements IActionViewItemService { + declare _serviceBrand: undefined; + + private readonly providers = new Map(); + private readonly changeEmitter = this._register(new Emitter()); + readonly onDidChange = this.changeEmitter.event; + + register(menu: MenuId, commandId: string | MenuId, provider: IActionViewItemFactory, event?: Event): IDisposable { + const key = `${menu.id}/${commandId instanceof MenuId ? commandId.id : commandId}`; + this.providers.set(key, provider); + const listener = event?.(() => this.changeEmitter.fire(menu)); + return toDisposable(() => { + listener?.dispose(); + this.providers.delete(key); + }); + } + + lookUp(menu: MenuId, commandId: string | MenuId): IActionViewItemFactory | undefined { + return this.providers.get(`${menu.id}/${commandId instanceof MenuId ? commandId.id : commandId}`); + } +} interface IChatSpec { readonly id: string; @@ -172,15 +216,20 @@ interface IRenderOptions { readonly width?: number; readonly phone?: boolean; readonly revealHierarchyGuides?: boolean; + readonly showAutomations?: boolean; + readonly automationRunStatus?: IAutomationRun['status']; + readonly automationBadgeStyle?: AutomationsNewBadgeStyle; readonly showFocusedToolbar?: boolean; } -function renderSessionsList(ctx: ComponentFixtureContext, options: IRenderOptions): void | Promise { +async function renderSessionsList(ctx: ComponentFixtureContext, options: IRenderOptions): Promise { const { container, disposableStore } = ctx; const approvals = new Map(); const sessions = options.sessions.map(spec => createSession(spec, approvals)); const approvalModel = createApprovalModel(approvals); const groups = options.groups ?? []; + const automationRuns = observableValue(disposableStore, []); + const actionViewItemService = disposableStore.add(new FixtureActionViewItemService()); const membership = new Map(); for (const spec of options.sessions) { if (spec.group) { @@ -192,6 +241,7 @@ function renderSessionsList(ctx: ComponentFixtureContext, options: IRenderOption colorTheme: ctx.theme, additionalServices: reg => { registerWorkbenchServices(reg); + reg.defineInstance(IProductService, TestProductService); if (options.showFocusedToolbar) { const archiveAction = new class extends mock() { override readonly id = 'sessions.fixture.archive'; @@ -279,7 +329,8 @@ function renderSessionsList(ctx: ComponentFixtureContext, options: IRenderOption override hasPendingResponse() { return false; } }()); reg.defineInstance(IAutomationService, new class extends mock() { - override readonly runs = constObservable([]); + override readonly automations = constObservable([]); + override readonly runs = automationRuns; }()); reg.defineInstance(IWorkbenchAssignmentService, new class extends mock() { override readonly onDidRefetchAssignments = Event.None; @@ -288,13 +339,40 @@ function renderSessionsList(ctx: ComponentFixtureContext, options: IRenderOption reg.defineInstance(IUriIdentityService, new class extends mock() { override readonly extUri = new ExtUri(() => true); }()); - reg.defineInstance(ICustomViewService, new class extends mock() { }()); + reg.defineInstance(ICustomViewService, new class extends mock() { + override readonly activeCustomView = constObservable(undefined); + }()); }, }); + if (options.showAutomations) { + const contextKeyService = instantiationService.get(IContextKeyService); + const newSessionAction = new MenuItemAction( + { id: NEW_SESSION_ACTION_ID, title: 'New Session' }, + undefined, + undefined, + undefined, + undefined, + contextKeyService, + instantiationService.get(ICommandService), + ); + instantiationService.stub(IActionViewItemService, actionViewItemService); + instantiationService.stub(IMenuService, new class extends mock() { + override createMenu(id: MenuId): IMenu { + return { + onDidChange: Event.None, + getActions: () => id === Menus.SidebarSessionsHeader ? [['navigation', [newSessionAction]]] : [], + dispose: () => { }, + }; + } + }()); + } // Render terminal-approval labels as real (monospace) code blocks — otherwise // the markdown renderer emits empty code-block spans and the command is blank. (instantiationService.get(IConfigurationService) as TestConfigurationService).setUserConfiguration('editor', { fontFamily: 'monospace' }); + if (options.automationBadgeStyle) { + await (instantiationService.get(IConfigurationService) as TestConfigurationService).setUserConfiguration(AUTOMATIONS_NEW_BADGE_STYLE_SETTING, options.automationBadgeStyle); + } instantiationService.get(IMarkdownRendererService).setDefaultCodeBlockRenderer(instantiationService.createInstance(EditorMarkdownCodeBlockRenderer)); // Phone layout is driven by both a CSS class (visual) and a context key (row @@ -303,6 +381,9 @@ function renderSessionsList(ctx: ComponentFixtureContext, options: IRenderOption if (options.phone) { IsPhoneLayoutContext.bindTo(instantiationService.get(IContextKeyService)).set(true); } + if (options.showAutomations) { + ChatAutomationsEnabledContext.bindTo(instantiationService.get(IContextKeyService)).set(true); + } const width = options.width ?? 340; container.style.width = `${width}px`; @@ -312,15 +393,41 @@ function renderSessionsList(ctx: ComponentFixtureContext, options: IRenderOption container.classList.add('agent-sessions-workbench', 'phone-layout'); } - const listHost = container.ownerDocument.createElement('div'); - container.appendChild(listHost); + let listParent = container; + if (options.showAutomations) { + container.classList.add('agent-sessions-viewpane', 'agent-sessions-section'); + const content = DOM.append(container, DOM.$('.agent-sessions-content')); + disposableStore.add(instantiationService.createInstance(NewSessionActionViewItemContribution)); + renderSessionsHeader(content, false, instantiationService, instantiationService.get(IContextKeyService), disposableStore).toolbar?.refresh(); + listParent = content; + } + const listHost = DOM.append(listParent, DOM.$(options.showAutomations ? '.agent-sessions-control-container' : 'div')); const list = disposableStore.add(instantiationService.createInstance(SessionsList, listHost, { grouping: () => options.grouping ?? SessionsGrouping.Workspace, sorting: () => SessionsSorting.Created, onSessionOpen: () => { }, approvalModel, })); - list.layout(options.phone ? 260 : 220, width); + list.layout(options.phone ? 260 : options.showAutomations ? 180 : 220, width); + + if (options.automationRunStatus) { + automationRuns.set([{ + id: 'fixture-run', + automationId: 'fixture-automation', + status: options.automationRunStatus, + trigger: 'schedule', + startedAt: new Date().toISOString(), + leaderWindowId: 1, + }], undefined); + } + await Promise.resolve(); + if (options.showAutomations && !container.querySelector('.agent-sessions-compact-new-button')) { + const menu = instantiationService.get(IMenuService).createMenu(Menus.SidebarSessionsHeader, instantiationService.get(IContextKeyService)); + const actionCount = menu.getActions().flatMap(([, actions]) => actions).length; + menu.dispose(); + const hasProvider = !!instantiationService.get(IActionViewItemService).lookUp(Menus.SidebarSessionsHeader, NEW_SESSION_ACTION_ID); + throw new Error(`Expected the production New Session action; found ${actionCount} menu action(s), provider=${hasProvider}.`); + } if (options.showFocusedToolbar) { return Promise.resolve().then(() => { @@ -413,6 +520,55 @@ export default defineThemedFixtureGroup({ path: 'sessions/' }, { sessions: [{ id: 'c', title: 'Update onboarding copy', workspace: 'vscode-docs', minutesAgo: 180 }], }), }), + SessionsList_AutomationsNewBadge: defineComponentFixture({ + labels: { kind: 'screenshot', blocksCi: true }, + additionalThemes: ['darkHighContrast'], + expectedVisualDescriptions: ['The Sessions header has an outlined New button. Directly below it, the Automations row has a smaller right-aligned outlined NEW capsule that reads as a non-interactive feature badge rather than a second button.'], + render: ctx => renderSessionsList(ctx, { + sessions: [], + showAutomations: true, + }), + }), + SessionsList_AutomationsNewBadge_Accent: defineComponentFixture({ + labels: { kind: 'screenshot', blocksCi: true }, + additionalThemes: ['darkHighContrast'], + expectedVisualDescriptions: ['The Automations row has a compact right-aligned NEW pill using the prominent activity badge colors, while the larger outlined New button remains visually distinct in the Sessions header.'], + render: ctx => renderSessionsList(ctx, { + sessions: [], + showAutomations: true, + automationBadgeStyle: 'accent', + }), + }), + SessionsList_AutomationsNewBadge_Soft: defineComponentFixture({ + labels: { kind: 'screenshot', blocksCi: true }, + additionalThemes: ['darkHighContrast'], + expectedVisualDescriptions: ['The Automations row has a compact right-aligned NEW pill with a subtle neutral fill, while the larger outlined New button remains visually distinct in the Sessions header.'], + render: ctx => renderSessionsList(ctx, { + sessions: [], + showAutomations: true, + automationBadgeStyle: 'soft', + }), + }), + SessionsList_AutomationsNewBadge_Narrow: defineComponentFixture({ + labels: { kind: 'screenshot', blocksCi: true }, + additionalThemes: ['darkHighContrast'], + expectedVisualDescriptions: ['At the 170px minimum sidebar width, the Automations label remains readable and the outlined NEW capsule stays right-aligned without changing the row height.'], + render: ctx => renderSessionsList(ctx, { + sessions: [], + showAutomations: true, + width: 170, + }), + }), + SessionsList_AutomationsNewBadge_Running: defineComponentFixture({ + labels: { kind: 'screenshot', blocksCi: true }, + additionalThemes: ['darkHighContrast'], + expectedVisualDescriptions: ['The Automations row shows its running status icon and the outlined NEW capsule together without overlap or layout shift.'], + render: ctx => renderSessionsList(ctx, { + sessions: [], + showAutomations: true, + automationRunStatus: 'running', + }), + }), SessionsList_CustomGroup_Phone: defineComponentFixture({ render: ctx => renderSessionsList(ctx, { sessions: GROUPED_SESSIONS, groups: [GROUP], phone: true, width: 340 }), }), diff --git a/test/componentFixtures/blocks-ci-screenshots.md b/test/componentFixtures/blocks-ci-screenshots.md index 3e8e18551ddde..822f3dc323dd4 100644 --- a/test/componentFixtures/blocks-ci-screenshots.md +++ b/test/componentFixtures/blocks-ci-screenshots.md @@ -210,6 +210,51 @@ #### sessions/chat/newWidget/newChatWidget/NewSessionWorkspacePicker/Light ![screenshot](https://hediet-screenshots.azurewebsites.net/images/695c9069e5791b7d24b24c1f9db4561755ff5694ad38370a7706426b6d38210a) +#### sessions/sessionsList/SessionsList_AutomationsNewBadge_Accent/Dark +![screenshot](https://hediet-screenshots.azurewebsites.net/images/7d1ed985c2b64d4cb7e9ef6c56df06761e308d0f4c8c49370bfaaf289d11121f) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge_Accent/DarkHighContrast +![screenshot](https://hediet-screenshots.azurewebsites.net/images/dc2d1502e0902545279c33075ad656aff4206049872b18e9cbe87ddd171729f7) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge_Accent/Light +![screenshot](https://hediet-screenshots.azurewebsites.net/images/b338d37441f73f6548b2cc9988d3875e351b914aa87269d06f74f4e844a51681) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge_Narrow/Dark +![screenshot](https://hediet-screenshots.azurewebsites.net/images/be3b3a64f1f39466e2239ab7d884befc5540d6a42253e77ba96408e4957c1404) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge_Narrow/DarkHighContrast +![screenshot](https://hediet-screenshots.azurewebsites.net/images/761d4a6a30b41df0f542aa11ba02fb10896fecdd32fd3723e63b0b9062552d6f) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge_Narrow/Light +![screenshot](https://hediet-screenshots.azurewebsites.net/images/22cf5e117dce9936a9d7189741fbea102c57dea6e5ff9b04f6d9e9532f261ec5) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge_Running/Dark +![screenshot](https://hediet-screenshots.azurewebsites.net/images/8d11fedea833e441d29331a2e51b034f6aeb3de94bb9eb4aa313428007748c2f) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge_Running/DarkHighContrast +![screenshot](https://hediet-screenshots.azurewebsites.net/images/5c1f4114f8de4b3fcd390d0a952440e0107b0b2ecd98c39cf8a24ba54499f23e) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge_Running/Light +![screenshot](https://hediet-screenshots.azurewebsites.net/images/40ebe91ecf151cf2dfaa702cb260805da15d3a9cc89465d353a9d2ea37f61405) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge_Soft/Dark +![screenshot](https://hediet-screenshots.azurewebsites.net/images/806a587cfc90c9fa9b69728137b2a26f0e3827278541f74e50dde685816fae40) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge_Soft/DarkHighContrast +![screenshot](https://hediet-screenshots.azurewebsites.net/images/34b08a9593a152f108dfa5a005d968a2e709a8ae12fcdc2f57fd0a15280a5b72) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge_Soft/Light +![screenshot](https://hediet-screenshots.azurewebsites.net/images/55b9eb610b82aa126043592d1ecc5458ce8f0f4a37b3a9c2db557c60ed39d353) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge/Dark +![screenshot](https://hediet-screenshots.azurewebsites.net/images/11b9c20d4100af6684519045dbef07052202c37041363c888a6cbc4be276b4d4) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge/DarkHighContrast +![screenshot](https://hediet-screenshots.azurewebsites.net/images/dc2d1502e0902545279c33075ad656aff4206049872b18e9cbe87ddd171729f7) + +#### sessions/sessionsList/SessionsList_AutomationsNewBadge/Light +![screenshot](https://hediet-screenshots.azurewebsites.net/images/5db359cb5c1f424f4f5153ad59e633efda657332d2e19049e472f27bd38df64f) + #### sessions/sessionsList/SessionsList_NarrowHoverToolbar/Dark ![screenshot](https://hediet-screenshots.azurewebsites.net/images/8af0c707c9c8e321ac7c8fd792b3c242a0d394cdaf68c3fe1c61804095395030) From 6f7c195f5404c9efd9b9d58622e2bff495a5bbed Mon Sep 17 00:00:00 2001 From: Simon Siefke Date: Thu, 3 Sep 2026 12:06:10 +0200 Subject: [PATCH 12/13] fix: memory leak in extension host comments (#334095) * fix: memory leak in extHostComments * Attestation commit --------- Co-authored-by: Alex Ross <38270282+alexr00@users.noreply.github.com> --- .../workbench/api/common/extHostComments.ts | 17 +++--- .../api/test/common/extHostComments.test.ts | 55 +++++++++++++++++++ 2 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 src/vs/workbench/api/test/common/extHostComments.test.ts diff --git a/src/vs/workbench/api/common/extHostComments.ts b/src/vs/workbench/api/common/extHostComments.ts index 3f1378c31dd1f..7b80cda420158 100644 --- a/src/vs/workbench/api/common/extHostComments.ts +++ b/src/vs/workbench/api/common/extHostComments.ts @@ -12,7 +12,7 @@ import { MarshalledId } from '../../../base/common/marshallingIds.js'; import { URI, UriComponents } from '../../../base/common/uri.js'; import { IRange } from '../../../editor/common/core/range.js'; import * as languages from '../../../editor/common/languages.js'; -import { ExtensionIdentifierMap, IExtensionDescription } from '../../../platform/extensions/common/extensions.js'; +import { IExtensionDescription } from '../../../platform/extensions/common/extensions.js'; import { ExtHostDocuments } from './extHostDocuments.js'; import * as extHostTypeConverter from './extHostTypeConverters.js'; import * as types from './extHostTypes.js'; @@ -38,9 +38,6 @@ export function createExtHostComments(mainContext: IMainContext, commands: ExtHo private _commentControllers: Map = new Map(); - private _commentControllersByExtension: ExtensionIdentifierMap = new ExtensionIdentifierMap(); - - constructor( ) { commands.registerArgumentProcessor({ @@ -150,13 +147,11 @@ export function createExtHostComments(mainContext: IMainContext, commands: ExtHo createCommentController(extension: IExtensionDescription, id: string, label: string): vscode.CommentController { const handle = ExtHostCommentsImpl.handlePool++; - const commentController = new ExtHostCommentController(extension, handle, id, label); + const commentController = new ExtHostCommentController(extension, handle, id, label, () => { + this._commentControllers.delete(handle); + }); this._commentControllers.set(commentController.handle, commentController); - const commentControllers = this._commentControllersByExtension.get(extension.identifier) || []; - commentControllers.push(commentController); - this._commentControllersByExtension.set(extension.identifier, commentControllers); - return commentController.value; } @@ -665,7 +660,8 @@ export function createExtHostComments(mainContext: IMainContext, commands: ExtHo private _extension: IExtensionDescription, private _handle: number, private _id: string, - private _label: string + private _label: string, + onDidDispose: () => void ) { proxy.$registerCommentController(this.handle, _id, _label, this._extension.identifier.value); @@ -693,6 +689,7 @@ export function createExtHostComments(mainContext: IMainContext, commands: ExtHo proxy.$unregisterCommentController(this.handle); } }); + this._localDisposables.push({ dispose: onDidDispose }); } createCommentThread(resource: vscode.Uri, range: vscode.Range | undefined, comments: vscode.Comment[]): ExtHostCommentThread { diff --git a/src/vs/workbench/api/test/common/extHostComments.test.ts b/src/vs/workbench/api/test/common/extHostComments.test.ts new file mode 100644 index 0000000000000..a3612009b55bd --- /dev/null +++ b/src/vs/workbench/api/test/common/extHostComments.test.ts @@ -0,0 +1,55 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { ExtensionIdentifier } from '../../../../platform/extensions/common/extensions.js'; +import { nullExtensionDescription } from '../../../services/extensions/common/extensions.js'; +import { MainContext, MainThreadCommentsShape } from '../../common/extHost.protocol.js'; +import { ArgumentProcessor, ExtHostCommands } from '../../common/extHostCommands.js'; +import { createExtHostComments } from '../../common/extHostComments.js'; +import { ExtHostDocuments } from '../../common/extHostDocuments.js'; +import { TestRPCProtocol } from './testRPCProtocol.js'; + +suite('ExtHostComments', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + test('disposed comment controllers are removed from extension host bookkeeping', async () => { + let controllerHandle = -1; + let createdThreadCount = 0; + const rpcProtocol = new TestRPCProtocol(); + rpcProtocol.set(MainContext.MainThreadComments, new class extends mock() { + override $registerCommentController(handle: number): void { + controllerHandle = handle; + } + override $unregisterCommentController(): void { } + override $createCommentThread(): undefined { + createdThreadCount++; + return undefined; + } + override $deleteCommentThread(): void { } + }); + + const commands = new class extends mock() { + override registerArgumentProcessor(_processor: ArgumentProcessor): void { } + }; + const extension = { + ...nullExtensionDescription, + identifier: new ExtensionIdentifier('test.comments'), + name: 'comments', + displayName: 'Comments', + extensionLocation: URI.file('/extension') + }; + const extHostComments = createExtHostComments(rpcProtocol, commands, {} as ExtHostDocuments); + const controller = extHostComments.createCommentController(extension, 'comments', 'Comments'); + + controller.dispose(); + await extHostComments.$createCommentThreadTemplate(controllerHandle, URI.file('/file'), undefined); + + assert.strictEqual(createdThreadCount, 0); + }); +}); From f13577c8148d0e687623f6dea9868d9d57411393 Mon Sep 17 00:00:00 2001 From: Ulugbek Abdullaev Date: Thu, 3 Sep 2026 15:15:30 +0500 Subject: [PATCH 13/13] Enable automations by default in non-Stable builds (#334208) * Agent Host changes for ulugbekna/agents/enable-default-automations-insiders * automations: test: capture setting before registry cleanup The full browser suite clears global configuration registrations after modules load. Capture the Automations setting schema during module initialization so the regression test remains valid regardless of suite order. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * automations: test: cover excluded setting registry Stable builds place the excluded Automations setting in the excluded configuration registry. Capture the schema from either registry at module load so the test works in all product qualities and remains independent of suite cleanup order. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/automations.contribution.ts | 2 +- .../browser/automations.contribution.test.ts | 31 +++++++++++++++++++ .../common/automations/automationsEnabled.ts | 2 +- 3 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 src/vs/sessions/contrib/automations/test/browser/automations.contribution.test.ts diff --git a/src/vs/sessions/contrib/automations/browser/automations.contribution.ts b/src/vs/sessions/contrib/automations/browser/automations.contribution.ts index e4444e439f314..bd7e01248468c 100644 --- a/src/vs/sessions/contrib/automations/browser/automations.contribution.ts +++ b/src/vs/sessions/contrib/automations/browser/automations.contribution.ts @@ -38,7 +38,7 @@ Registry.as(ConfigurationExtensions.Configuration).regis properties: { [CHAT_AUTOMATIONS_ENABLED_SETTING]: { type: 'boolean', - default: false, + default: product.quality !== 'stable', scope: ConfigurationScope.MACHINE, tags: ['experimental', 'advanced'], description: localize('chat.automations.enabled', "Enables the Automations feature: scheduling agent sessions to run on a cadence. When disabled, the Automations entry in the Customizations sidebar, the Automations section in the Customizations editor, and the Automation option in the new-session composer are hidden, and scheduled automations are not dispatched."), diff --git a/src/vs/sessions/contrib/automations/test/browser/automations.contribution.test.ts b/src/vs/sessions/contrib/automations/test/browser/automations.contribution.test.ts new file mode 100644 index 0000000000000..24d7a7d1f4c1a --- /dev/null +++ b/src/vs/sessions/contrib/automations/test/browser/automations.contribution.test.ts @@ -0,0 +1,31 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { Extensions as ConfigurationExtensions, IConfigurationRegistry } from '../../../../../platform/configuration/common/configurationRegistry.js'; +import product from '../../../../../platform/product/common/product.js'; +import { Registry } from '../../../../../platform/registry/common/platform.js'; +import { CHAT_AUTOMATIONS_ENABLED_SETTING } from '../../../../../workbench/contrib/chat/common/automations/automationsEnabled.js'; + +import '../../browser/automations.contribution.js'; + +const configurationRegistry = Registry.as(ConfigurationExtensions.Configuration); +const automationEnabledProperty = configurationRegistry.getConfigurationProperties()[CHAT_AUTOMATIONS_ENABLED_SETTING] + ?? configurationRegistry.getExcludedConfigurationProperties()[CHAT_AUTOMATIONS_ENABLED_SETTING]; + +suite('Automations Contribution', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + test('defaults Automations to enabled in non-Stable builds', () => { + assert.deepStrictEqual({ + default: automationEnabledProperty.default, + included: automationEnabledProperty.included, + }, { + default: product.quality !== 'stable', + included: product.quality !== 'stable', + }); + }); +}); diff --git a/src/vs/workbench/contrib/chat/common/automations/automationsEnabled.ts b/src/vs/workbench/contrib/chat/common/automations/automationsEnabled.ts index 31efd8bd3cc42..f32e6abe866bb 100644 --- a/src/vs/workbench/contrib/chat/common/automations/automationsEnabled.ts +++ b/src/vs/workbench/contrib/chat/common/automations/automationsEnabled.ts @@ -7,7 +7,7 @@ import { RawContextKey } from '../../../../../platform/contextkey/common/context /** * Gates the entire Automations feature: sidebar entry, editor section, - * session composer option, and scheduled execution. Default `false`. + * session composer option, and scheduled execution. Enabled by default in non-Stable builds. */ export const CHAT_AUTOMATIONS_ENABLED_SETTING = 'chat.automations.enabled';