From 877cc873b12dde56481137962561c7fdf0841a3d Mon Sep 17 00:00:00 2001 From: "Brandon Waterloo [MSFT]" <36966225+bwateratmsft@users.noreply.github.com> Date: Wed, 15 Jul 2026 13:32:45 -0400 Subject: [PATCH 01/26] Add extensionEnabled: when-clause context key Mirror the set of installed and enabled extensions into per-extension context keys (extensionEnabled:) so authors can gate UI via when clauses without activating the extension. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cb195182-bee3-4aaf-b33b-ceb8e36e3f18 --- .../browser/extensionEnablementContext.ts | 65 +++++++++++++ .../browser/extensions.contribution.ts | 2 + .../extensionEnablementContext.test.ts | 91 +++++++++++++++++++ 3 files changed, 158 insertions(+) create mode 100644 src/vs/workbench/contrib/extensions/browser/extensionEnablementContext.ts create mode 100644 src/vs/workbench/contrib/extensions/test/browser/extensionEnablementContext.test.ts diff --git a/src/vs/workbench/contrib/extensions/browser/extensionEnablementContext.ts b/src/vs/workbench/contrib/extensions/browser/extensionEnablementContext.ts new file mode 100644 index 00000000000000..d60c411a75285e --- /dev/null +++ b/src/vs/workbench/contrib/extensions/browser/extensionEnablementContext.ts @@ -0,0 +1,65 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { ExtensionIdentifier } from '../../../../platform/extensions/common/extensions.js'; +import { IContextKey, IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; +import { IExtensionService } from '../../../services/extensions/common/extensions.js'; +import { IWorkbenchContribution } from '../../../common/contributions.js'; + +/** + * Prefix for the per-extension when-clause context keys that report whether an + * extension is installed and enabled. The full key is + * `extensionEnabled:`, e.g. `extensionEnabled:ms-python.python`. + * + * Because context keys are matched by exact string, the extension id is always + * lowercased. When clauses must therefore use the lowercased id (for example + * `extensionEnabled:github.copilot`, not `extensionEnabled:GitHub.copilot`). + */ +export const EXTENSION_ENABLED_CONTEXT_KEY_PREFIX = 'extensionEnabled:'; + +/** + * Mirrors the set of installed and enabled extensions into context keys so that + * `when` clauses can gate on the presence of another extension without having to + * activate it. A key `extensionEnabled:` is `true` while the extension is + * registered (installed and enabled) and `false` once it is removed (uninstalled + * or disabled). + */ +export class ExtensionEnablementContextKeysContribution extends Disposable implements IWorkbenchContribution { + + private readonly _contextKeys = new Map>(); + + constructor( + @IContextKeyService private readonly contextKeyService: IContextKeyService, + @IExtensionService private readonly extensionService: IExtensionService, + ) { + super(); + + // Seed context keys for extensions that are already registered. + for (const extension of this.extensionService.extensions) { + this._setEnabled(extension.identifier, true); + } + + // Track extensions as they get registered or de-registered. + this._register(this.extensionService.onDidChangeExtensions(({ added, removed }) => { + for (const extension of removed) { + this._setEnabled(extension.identifier, false); + } + for (const extension of added) { + this._setEnabled(extension.identifier, true); + } + })); + } + + private _setEnabled(identifier: ExtensionIdentifier, enabled: boolean): void { + const key = EXTENSION_ENABLED_CONTEXT_KEY_PREFIX + ExtensionIdentifier.toKey(identifier); + let contextKey = this._contextKeys.get(key); + if (!contextKey) { + contextKey = this.contextKeyService.createKey(key, false); + this._contextKeys.set(key, contextKey); + } + contextKey.set(enabled); + } +} diff --git a/src/vs/workbench/contrib/extensions/browser/extensions.contribution.ts b/src/vs/workbench/contrib/extensions/browser/extensions.contribution.ts index 489257f34083ed..b04aae0a2ff0e4 100644 --- a/src/vs/workbench/contrib/extensions/browser/extensions.contribution.ts +++ b/src/vs/workbench/contrib/extensions/browser/extensions.contribution.ts @@ -79,6 +79,7 @@ import { ExtensionRecommendationsService } from './extensionRecommendationsServi import { ClearLanguageAction, ConfigureWorkspaceFolderRecommendedExtensionsAction, ConfigureWorkspaceRecommendedExtensionsAction, InstallAction, InstallAnotherVersionAction, InstallSpecificVersionOfExtensionAction, SetColorThemeAction, SetFileIconThemeAction, SetProductIconThemeAction, ToggleAutoUpdateForExtensionAction, ToggleAutoUpdatesForPublisherAction, TogglePreReleaseExtensionAction } from './extensionsActions.js'; import { ExtensionActivationProgress } from './extensionsActivationProgress.js'; import { ExtensionsCompletionItemsProvider } from './extensionsCompletionItemsProvider.js'; +import { ExtensionEnablementContextKeysContribution } from './extensionEnablementContext.js'; import { ExtensionDependencyChecker } from './extensionsDependencyChecker.js'; import { clearSearchResultsIcon, configureRecommendedIcon, extensionsViewIcon, filterIcon, installWorkspaceRecommendedIcon, refreshIcon } from './extensionsIcons.js'; import { InstallExtensionQuickAccessProvider, ManageExtensionsQuickAccessProvider } from './extensionsQuickAccess.js'; @@ -2100,6 +2101,7 @@ workbenchRegistry.registerWorkbenchContribution(ExtensionActivationProgress, Lif workbenchRegistry.registerWorkbenchContribution(ExtensionDependencyChecker, LifecyclePhase.Eventually); workbenchRegistry.registerWorkbenchContribution(ExtensionEnablementWorkspaceTrustTransitionParticipant, LifecyclePhase.Restored); workbenchRegistry.registerWorkbenchContribution(ExtensionsCompletionItemsProvider, LifecyclePhase.Restored); +workbenchRegistry.registerWorkbenchContribution(ExtensionEnablementContextKeysContribution, LifecyclePhase.Restored); workbenchRegistry.registerWorkbenchContribution(UnsupportedExtensionsMigrationContrib, LifecyclePhase.Eventually); workbenchRegistry.registerWorkbenchContribution(TrustedPublishersInitializer, LifecyclePhase.Eventually); workbenchRegistry.registerWorkbenchContribution(ExtensionMarketplaceStatusUpdater, LifecyclePhase.Eventually); diff --git a/src/vs/workbench/contrib/extensions/test/browser/extensionEnablementContext.test.ts b/src/vs/workbench/contrib/extensions/test/browser/extensionEnablementContext.test.ts new file mode 100644 index 00000000000000..85692efc859db7 --- /dev/null +++ b/src/vs/workbench/contrib/extensions/test/browser/extensionEnablementContext.test.ts @@ -0,0 +1,91 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { Emitter, Event } from '../../../../../base/common/event.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { ExtensionIdentifier, IExtensionDescription } from '../../../../../platform/extensions/common/extensions.js'; +import { MockContextKeyService } from '../../../../../platform/keybinding/test/common/mockKeybindingService.js'; +import { NullExtensionService, nullExtensionDescription } from '../../../../services/extensions/common/extensions.js'; +import { EXTENSION_ENABLED_CONTEXT_KEY_PREFIX, ExtensionEnablementContextKeysContribution } from '../../browser/extensionEnablementContext.js'; + +type ExtensionsChangeEvent = { readonly added: readonly IExtensionDescription[]; readonly removed: readonly IExtensionDescription[] }; + +function aExtension(id: string): IExtensionDescription { + return { ...nullExtensionDescription, identifier: new ExtensionIdentifier(id) }; +} + +class TestExtensionService extends NullExtensionService { + private readonly _onDidChangeExtensions = new Emitter(); + override readonly onDidChangeExtensions: Event = this._onDidChangeExtensions.event; + + constructor(initial: IExtensionDescription[]) { + super(); + (this.extensions as IExtensionDescription[]).push(...initial); + } + + fireChange(added: IExtensionDescription[], removed: IExtensionDescription[]): void { + const extensions = this.extensions as IExtensionDescription[]; + for (const toRemove of removed) { + const index = extensions.findIndex(e => ExtensionIdentifier.equals(e.identifier, toRemove.identifier)); + if (index !== -1) { + extensions.splice(index, 1); + } + } + extensions.push(...added); + this._onDidChangeExtensions.fire({ added, removed }); + } + + dispose(): void { + this._onDidChangeExtensions.dispose(); + } +} + +suite('ExtensionEnablementContextKeysContribution', () => { + + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + + function snapshot(contextKeyService: MockContextKeyService, ids: string[]): Record { + const result: Record = {}; + for (const id of ids) { + result[id] = contextKeyService.getContextKeyValue(EXTENSION_ENABLED_CONTEXT_KEY_PREFIX + id); + } + return result; + } + + function createContribution(initial: IExtensionDescription[]): { contextKeyService: MockContextKeyService; extensionService: TestExtensionService } { + const extensionService = new TestExtensionService(initial); + disposables.add({ dispose: () => extensionService.dispose() }); + const contextKeyService = disposables.add(new MockContextKeyService()); + disposables.add(new ExtensionEnablementContextKeysContribution(contextKeyService, extensionService)); + return { contextKeyService, extensionService }; + } + + test('reflects installed-and-enabled extensions and reacts to changes', () => { + const { contextKeyService, extensionService } = createContribution([aExtension('pub.a'), aExtension('pub.b')]); + + assert.deepStrictEqual(snapshot(contextKeyService, ['pub.a', 'pub.b', 'pub.c']), { + 'pub.a': true, // seeded from already-registered extensions + 'pub.b': true, + 'pub.c': undefined, // never present -> key was never set + }); + + extensionService.fireChange([aExtension('pub.c')], []); // newly enabled + extensionService.fireChange([], [aExtension('pub.b')]); // disabled or uninstalled + extensionService.fireChange([aExtension('pub.a')], [aExtension('pub.a')]); // update -> removed then re-added + + assert.deepStrictEqual(snapshot(contextKeyService, ['pub.a', 'pub.b', 'pub.c']), { + 'pub.a': true, + 'pub.b': false, + 'pub.c': true, + }); + }); + + test('normalizes extension id to lower case', () => { + const { contextKeyService } = createContribution([aExtension('Pub.MixedCase')]); + + assert.strictEqual(contextKeyService.getContextKeyValue(EXTENSION_ENABLED_CONTEXT_KEY_PREFIX + 'pub.mixedcase'), true); + }); +}); From 6034f527f90e7f08f67dedbb55c7d59750f5b6a2 Mon Sep 17 00:00:00 2001 From: "Brandon Waterloo [MSFT]" <36966225+bwateratmsft@users.noreply.github.com> Date: Tue, 21 Jul 2026 10:27:54 -0400 Subject: [PATCH 02/26] Reconcile extension context keys from the final registry Address CCR feedback: seed on onDidRegisterExtensions (extensions list is empty until initial registration, which the Restored phase does not guarantee) and reconcile against the authoritative extensionService.extensions set instead of trusting change deltas (which can report added extensions that validation later rejects due to dependency loops). Adds a regression test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cb195182-bee3-4aaf-b33b-ceb8e36e3f18 --- .../browser/extensionEnablementContext.ts | 44 +++++++++++-------- .../extensionEnablementContext.test.ts | 24 ++++++++++ 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/src/vs/workbench/contrib/extensions/browser/extensionEnablementContext.ts b/src/vs/workbench/contrib/extensions/browser/extensionEnablementContext.ts index d60c411a75285e..d9334b5ad9f02c 100644 --- a/src/vs/workbench/contrib/extensions/browser/extensionEnablementContext.ts +++ b/src/vs/workbench/contrib/extensions/browser/extensionEnablementContext.ts @@ -37,29 +37,35 @@ export class ExtensionEnablementContextKeysContribution extends Disposable imple ) { super(); - // Seed context keys for extensions that are already registered. + // Reconcile against the authoritative set of registered extensions rather + // than trusting individual change deltas. `IExtensionService.extensions` + // is empty until the initial registration completes (signalled by + // `onDidRegisterExtensions`, which is not guaranteed to have fired by the + // `Restored` phase), and change deltas can also report `added` extensions + // that registry validation then rejects (e.g. dependency loops) without a + // matching `removed` entry. Recomputing from the final collection keeps the + // keys correct in both cases. + this._reconcile(); + this._register(this.extensionService.onDidRegisterExtensions(() => this._reconcile())); + this._register(this.extensionService.onDidChangeExtensions(() => this._reconcile())); + } + + private _reconcile(): void { + const enabledKeys = new Set(); for (const extension of this.extensionService.extensions) { - this._setEnabled(extension.identifier, true); + enabledKeys.add(EXTENSION_ENABLED_CONTEXT_KEY_PREFIX + ExtensionIdentifier.toKey(extension.identifier)); } - // Track extensions as they get registered or de-registered. - this._register(this.extensionService.onDidChangeExtensions(({ added, removed }) => { - for (const extension of removed) { - this._setEnabled(extension.identifier, false); - } - for (const extension of added) { - this._setEnabled(extension.identifier, true); - } - })); - } + // Update every key we already track to reflect the current registry. + for (const [key, contextKey] of this._contextKeys) { + contextKey.set(enabledKeys.has(key)); + } - private _setEnabled(identifier: ExtensionIdentifier, enabled: boolean): void { - const key = EXTENSION_ENABLED_CONTEXT_KEY_PREFIX + ExtensionIdentifier.toKey(identifier); - let contextKey = this._contextKeys.get(key); - if (!contextKey) { - contextKey = this.contextKeyService.createKey(key, false); - this._contextKeys.set(key, contextKey); + // Create keys for extensions that became enabled since the last reconcile. + for (const key of enabledKeys) { + if (!this._contextKeys.has(key)) { + this._contextKeys.set(key, this.contextKeyService.createKey(key, true)); + } } - contextKey.set(enabled); } } diff --git a/src/vs/workbench/contrib/extensions/test/browser/extensionEnablementContext.test.ts b/src/vs/workbench/contrib/extensions/test/browser/extensionEnablementContext.test.ts index 85692efc859db7..8ac2d125907773 100644 --- a/src/vs/workbench/contrib/extensions/test/browser/extensionEnablementContext.test.ts +++ b/src/vs/workbench/contrib/extensions/test/browser/extensionEnablementContext.test.ts @@ -26,6 +26,7 @@ class TestExtensionService extends NullExtensionService { (this.extensions as IExtensionDescription[]).push(...initial); } + /** Applies a delta to the registry and fires the matching change event. */ fireChange(added: IExtensionDescription[], removed: IExtensionDescription[]): void { const extensions = this.extensions as IExtensionDescription[]; for (const toRemove of removed) { @@ -38,6 +39,18 @@ class TestExtensionService extends NullExtensionService { this._onDidChangeExtensions.fire({ added, removed }); } + /** + * Overwrites the final registry and fires an event carrying an arbitrary + * (possibly misleading) delta. Used to simulate the case where registry + * validation rejects an `added` extension without reporting it in `removed`. + */ + setExtensionsAndFire(finalExtensions: IExtensionDescription[], event: ExtensionsChangeEvent): void { + const extensions = this.extensions as IExtensionDescription[]; + extensions.length = 0; + extensions.push(...finalExtensions); + this._onDidChangeExtensions.fire(event); + } + dispose(): void { this._onDidChangeExtensions.dispose(); } @@ -88,4 +101,15 @@ suite('ExtensionEnablementContextKeysContribution', () => { assert.strictEqual(contextKeyService.getContextKeyValue(EXTENSION_ENABLED_CONTEXT_KEY_PREFIX + 'pub.mixedcase'), true); }); + + test('reconciles against the final registry, not the change delta', () => { + const { contextKeyService, extensionService } = createContribution([aExtension('pub.a')]); + + // Registry validation drops pub.a from the final set (e.g. a dependency + // loop) but reports it as `added` and omits it from `removed`. The key + // must follow the final registry and become false, not trust the delta. + extensionService.setExtensionsAndFire([], { added: [aExtension('pub.a')], removed: [] }); + + assert.strictEqual(contextKeyService.getContextKeyValue(EXTENSION_ENABLED_CONTEXT_KEY_PREFIX + 'pub.a'), false); + }); }); From f5465c35fd7c3869acc5bed8721856fedc35c188 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Thu, 23 Jul 2026 12:59:17 -0700 Subject: [PATCH 03/26] agentHost: grant access to local attachments Grant remote Agent Host connections implicit read access to resource attachments before dispatch and reconnect replay. Fixes #326523 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/remoteAgentHostProtocolClient.ts | 54 ++++--- .../remoteAgentHostProtocolClient.test.ts | 149 +++++++++++++++++- 2 files changed, 178 insertions(+), 25 deletions(-) diff --git a/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts b/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts index 6a1496eda96b8f..0c8caec69440c2 100644 --- a/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts +++ b/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts @@ -25,7 +25,7 @@ import { agentHostAuthority, fromAgentHostUri, toAgentHostUri } from '../common/ import { AgentHostResourcePermissionError, IAgentHostResourceService } from '../common/agentHostResourceService.js'; import type { ClientNotificationMap, CommandMap, JsonRpcErrorResponse, JsonRpcRequest } from '../common/state/protocol/messages.js'; import { ActionType, type ActionEnvelope, type INotification, type IRootConfigChangedAction, type SessionAction, type ChatAction, type TerminalAction, type ClientAnnotationsAction } from '../common/state/sessionActions.js'; -import { SessionSummary, SessionStatus, ROOT_STATE_URI, StateComponents, isAhpRootChannel, type ClientPluginCustomization, type RootState } from '../common/state/sessionState.js'; +import { MessageAttachmentKind, SessionSummary, SessionStatus, ROOT_STATE_URI, StateComponents, isAhpRootChannel, type ClientPluginCustomization, type Message, type RootState } from '../common/state/sessionState.js'; import { PROTOCOL_VERSION } from '../common/state/protocol/version/registry.js'; import { isJsonRpcNotification, isJsonRpcRequest, isJsonRpcResponse, ProtocolError, ReconnectResultType, type ProtocolMessage, type IStateSnapshot } from '../common/state/sessionProtocol.js'; import { type IVscodeUpgradeResult } from '../common/state/protocolUpgrade.js'; @@ -44,6 +44,7 @@ import type { InitializeResult } from '../common/state/protocol/common/commands. import { dirname } from '../../../base/common/resources.js'; import { observableValue, type IObservable } from '../../../base/common/observable.js'; import { isFileResourceRead } from '../common/resourceReadLogging.js'; +import { ResourceSet } from '../../../base/common/map.js'; const AHP_CLIENT_CONNECTION_CLOSED = -32000; @@ -261,11 +262,11 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC private readonly _loadEstimator: ILoadEstimator; /** - * Comparison keys of customization URIs we have already granted implicit + * Comparison keys of URIs we have already granted implicit * read access for on this connection. Dedupes repeat sends so we don't * pile up grants per dispatch. Cleared with the connection. */ - private readonly _grantedCustomizationUris = new Set(); + private readonly _grantedImplicitReadUris = new ResourceSet(); get clientId(): string { return this._clientId; @@ -974,14 +975,28 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC } /** - * Inspect an outgoing client-dispatched action and grant implicit reads - * for any customization URIs it carries. Today this covers - * `SessionActiveClientSet`, which is the only client-dispatched - * action that ships customization URIs to the host. + * Inspect an outgoing client-dispatched action and grant implicit reads for + * resources that the host will need to read after receiving the action. */ private _grantImplicitReadsForOutgoingAction(action: SessionAction | ChatAction | TerminalAction | ClientAnnotationsAction | IRootConfigChangedAction): void { - if (action.type === ActionType.SessionActiveClientSet && action.activeClient.customizations) { - this._grantImplicitReadsForCustomizations(action.activeClient.customizations); + switch (action.type) { + case ActionType.SessionActiveClientSet: + if (action.activeClient.customizations) { + this._grantImplicitReadsForCustomizations(action.activeClient.customizations); + } + break; + case ActionType.ChatTurnStarted: + case ActionType.ChatPendingMessageSet: + this._grantImplicitReadsForMessage(action.message); + break; + } + } + + private _grantImplicitReadsForMessage(message: Message): void { + for (const attachment of message.attachments ?? []) { + if (attachment.type === MessageAttachmentKind.Resource) { + this._grantImplicitRead(URI.parse(attachment.uri)); + } } } @@ -999,16 +1014,17 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC } catch { continue; } - const grantUri = dirname(uri); - const key = grantUri.toString(); - if (this._grantedCustomizationUris.has(key)) { - continue; - } - this._grantedCustomizationUris.add(key); - // Disposable is owned by the resource service; cleared on - // connectionClosed. - this._resourceService.grantImplicitRead(this._address, grantUri); + this._grantImplicitRead(dirname(uri)); + } + } + + private _grantImplicitRead(uri: URI): void { + if (this._grantedImplicitReadUris.has(uri)) { + return; } + this._grantedImplicitReadUris.add(uri); + // The resource service also revokes these grants in connectionClosed. + this._resourceService.grantImplicitRead(this._address, uri); } /** @@ -1185,7 +1201,7 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC } this._rejectPendingRequests(error); this._resourceService.connectionClosed(this._address); - this._grantedCustomizationUris.clear(); + this._grantedImplicitReadUris.clear(); this._transitionTo({ kind: AgentHostClientState.Closed, error }); this._onDidClose.fire(); } diff --git a/src/vs/platform/agentHost/test/electron-browser/remoteAgentHostProtocolClient.test.ts b/src/vs/platform/agentHost/test/electron-browser/remoteAgentHostProtocolClient.test.ts index 1387385dc24962..6252a04cb0f2f4 100644 --- a/src/vs/platform/agentHost/test/electron-browser/remoteAgentHostProtocolClient.test.ts +++ b/src/vs/platform/agentHost/test/electron-browser/remoteAgentHostProtocolClient.test.ts @@ -5,6 +5,7 @@ import assert from 'assert'; import { DeferredPromise, timeout } from '../../../../base/common/async.js'; +import { VSBuffer } from '../../../../base/common/buffer.js'; import { CancellationError } from '../../../../base/common/errors.js'; import { Emitter, Event } from '../../../../base/common/event.js'; import { Disposable } from '../../../../base/common/lifecycle.js'; @@ -19,11 +20,11 @@ import { ConfigurationTarget, type IConfigurationValue } from '../../../configur import { ContentEncoding, ReconnectResultType } from '../../common/state/protocol/commands.js'; import { AhpErrorCodes } from '../../common/state/protocol/errors.js'; import { PROTOCOL_VERSION } from '../../common/state/protocol/version/registry.js'; -import { ActionType, type SessionActiveClientSetAction, type SessionActiveClientRemovedAction, type SessionTitleChangedAction } from '../../common/state/sessionActions.js'; +import { ActionType, 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 { CustomizationType, ROOT_STATE_URI, StateComponents, customizationId } from '../../common/state/sessionState.js'; +import { CustomizationType, MessageAttachmentKind, MessageKind, PendingMessageKind, ROOT_STATE_URI, StateComponents, customizationId } from '../../common/state/sessionState.js'; import type { IClientTransport, IProtocolTransport } from '../../common/state/sessionTransport.js'; import { TestConfigurationService } from '../../../configuration/test/common/testConfigurationService.js'; import { TelemetryLevel } from '../../../telemetry/common/telemetry.js'; @@ -145,6 +146,7 @@ suite('RemoteAgentHostProtocolClient', () => { granted?: (address: string, uri: URI, mode: AgentHostPermissionMode) => boolean; onRequest?: (address: string, params: { uri: string; read?: boolean; write?: boolean }) => Promise; onGrantImplicitRead?: (address: string, uri: URI) => void; + readBytes?: VSBuffer; } /** @@ -169,7 +171,13 @@ suite('RemoteAgentHostProtocolClient', () => { _serviceBrand: undefined, check: async (addr, uri, mode) => grant(addr, uri, mode), async list(addr, uri) { await gateRead(addr, uri); return { entries: [] }; }, - async read(addr, uri) { await gateRead(addr, uri); throw new Error('Not implemented in stub'); }, + async read(addr, uri) { + await gateRead(addr, uri); + if (opts.readBytes) { + return { bytes: opts.readBytes }; + } + throw new Error('Not implemented in stub'); + }, async write(addr, params) { await gateWrite(addr, URI.parse(params.uri)); }, async del(addr, params) { await gateWrite(addr, URI.parse(params.uri)); }, async move(addr, params) { await gateWrite(addr, URI.parse(params.source)); await gateWrite(addr, URI.parse(params.destination)); }, @@ -943,7 +951,7 @@ suite('RemoteAgentHostProtocolClient', () => { }); }); - suite('implicit grants for outgoing customization actions', () => { + suite('implicit grants for outgoing actions', () => { function createCapturingPermissionService(): { service: IAgentHostResourceService; calls: { address: string; uri: URI }[] } { const calls: { address: string; uri: URI }[] = []; @@ -979,6 +987,67 @@ suite('RemoteAgentHostProtocolClient', () => { ); }); + test('ChatTurnStarted grants attachment access before reverse resourceRead', async () => { + const granted = new Set(); + const attachmentUri = URI.file('/attachments/example.txt'); + const service = createResourceServiceStub({ + granted: (_address, uri, mode) => mode === AgentHostPermissionMode.Read && granted.has(uri.toString()), + onGrantImplicitRead: (_address, uri) => granted.add(uri.toString()), + readBytes: VSBuffer.fromString('attachment'), + }); + const { client, transport } = createClient(undefined, service); + const action: ChatTurnStartedAction = { + type: ActionType.ChatTurnStarted, + turnId: 'turn-1', + startedAt: '2026-07-23T00:00:00.000Z', + message: { + text: 'Review this file', + origin: { kind: MessageKind.User }, + attachments: [{ + type: MessageAttachmentKind.Resource, + uri: attachmentUri.toString(), + label: 'example.txt', + }], + }, + }; + + client.dispatch('copilot-chat:/test', action); + transport.fireMessage({ + jsonrpc: '2.0', + id: 42, + method: 'resourceRead', + params: { channel: ROOT_STATE_URI, uri: attachmentUri.toString() }, + }); + await timeout(0); + + assert.deepStrictEqual(transport.sentMessages.at(-1), { + jsonrpc: '2.0', + id: 42, + result: { data: 'YXR0YWNobWVudA==', encoding: ContentEncoding.Base64 }, + }); + }); + + test('ChatPendingMessageSet grants resource attachments only', () => { + const { service, calls } = createCapturingPermissionService(); + const { client } = createClient(undefined, service); + + client.dispatch('copilot-chat:/test', { + type: ActionType.ChatPendingMessageSet, + kind: PendingMessageKind.Queued, + id: 'queued-1', + message: { + text: 'Review these attachments', + origin: { kind: MessageKind.User }, + attachments: [ + { type: MessageAttachmentKind.Resource, uri: 'file:///attachments/queued.txt', label: 'queued.txt' }, + { type: MessageAttachmentKind.EmbeddedResource, data: '', contentType: 'text/plain', label: 'inline.txt' }, + ], + }, + }); + + assert.deepStrictEqual(calls.map(call => call.uri.toString()), ['file:///attachments/queued.txt']); + }); + test('multiple customizations in the same directory dedupe to one grant', () => { const { service, calls } = createCapturingPermissionService(); const { client } = createClient(undefined, service); @@ -1129,7 +1198,7 @@ suite('RemoteAgentHostProtocolClient', () => { * client plus a `transports` array recording each transport handed * out, so tests can drive handshake/reconnect interactions. */ - function createFactoryClient(): { client: RemoteAgentHostProtocolClient; transports: TestClientProtocolTransport[] } { + function createFactoryClient(permissionService = createPermissionService()): { client: RemoteAgentHostProtocolClient; transports: TestClientProtocolTransport[] } { const transports: TestClientProtocolTransport[] = []; const factory = () => { const t = disposables.add(new TestClientProtocolTransport()); @@ -1137,7 +1206,7 @@ suite('RemoteAgentHostProtocolClient', () => { return t; }; const client = disposables.add(new RemoteAgentHostProtocolClient( - 'test.example:1234', factory, undefined, new NullLogService(), createPermissionService(), new TestConfigurationService(), + 'test.example:1234', factory, undefined, new NullLogService(), permissionService, new TestConfigurationService(), )); return { client, transports }; } @@ -1234,6 +1303,74 @@ suite('RemoteAgentHostProtocolClient', () => { }); }); + test('attachment grant remains available when a pending turn is replayed after reconnect', async function () { + this.timeout(10_000); + return runWithFakedTimers({ useFakeTimers: true, maxTaskCount: 10_000 }, async () => { + const attachmentUri = URI.file('/attachments/replayed.txt'); + const granted = new Set(); + const permissionService = createResourceServiceStub({ + granted: (_address, uri, mode) => mode === AgentHostPermissionMode.Read && granted.has(uri.toString()), + onGrantImplicitRead: (_address, uri) => granted.add(uri.toString()), + readBytes: VSBuffer.fromString('replayed'), + }); + const { client, transports } = createFactoryClient(permissionService); + const connectPromise = client.connect(); + await completeHandshake(transports[0], connectPromise); + + const chatUri = URI.parse('copilot-chat:/test-chat'); + const subRef = client.getSubscription(StateComponents.Chat, chatUri, 'test'); + const subscribeReq = await waitForRequest(transports[0], 'subscribe'); + transports[0].fireMessage({ + jsonrpc: '2.0', id: subscribeReq.id, + result: { snapshot: { resource: chatUri.toString(), state: { turns: [] }, fromSeq: 5 } }, + }); + await Promise.resolve(); + + client.dispatch(chatUri.toString(), { + type: ActionType.ChatTurnStarted, + turnId: 'turn-1', + startedAt: '2026-07-23T00:00:00.000Z', + message: { + text: 'Review this file', + origin: { kind: MessageKind.User }, + attachments: [{ + type: MessageAttachmentKind.Resource, + uri: attachmentUri.toString(), + label: 'replayed.txt', + }], + }, + }); + + 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, + result: { type: ReconnectResultType.Replay, actions: [], missing: [] }, + }); + await flushMicrotasks(); + + assert.ok(findDispatchAction(reconnectTransport, ActionType.ChatTurnStarted)); + reconnectTransport.fireMessage({ + jsonrpc: '2.0', + id: 42, + method: 'resourceRead', + params: { channel: ROOT_STATE_URI, uri: attachmentUri.toString() }, + }); + await flushMicrotasks(); + assert.deepStrictEqual(reconnectTransport.sentMessages.at(-1), { + jsonrpc: '2.0', + id: 42, + result: { data: 'cmVwbGF5ZWQ=', encoding: ContentEncoding.Base64 }, + }); + + subRef.dispose(); + client.dispose(); + }); + }); + test('skips replay when server already echoed the action in the replay buffer', async function () { this.timeout(10_000); return runWithFakedTimers({ useFakeTimers: true, maxTaskCount: 10_000 }, async () => { From d35865d758d3986bc93f51f51bf596d6089f061d Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Thu, 23 Jul 2026 13:15:17 -0700 Subject: [PATCH 04/26] agentHost: allow dispatching chat actions Align the remote protocol client dispatch signatures with the chat actions handled by outgoing attachment grants. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/browser/remoteAgentHostProtocolClient.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts b/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts index 0c8caec69440c2..0941504b4301da 100644 --- a/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts +++ b/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts @@ -752,7 +752,7 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC return this._subscriptionManager.getActiveSubscriptions(); } - dispatch(channel: string, action: SessionAction | TerminalAction | ClientAnnotationsAction | IRootConfigChangedAction): void { + dispatch(channel: string, action: SessionAction | ChatAction | TerminalAction | ClientAnnotationsAction | IRootConfigChangedAction): void { const seq = this._subscriptionManager.dispatchOptimistic(channel, action); this.dispatchAction(channel, action, this._clientId, seq); } @@ -797,7 +797,7 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC /** * Dispatch a client action to the server. Returns the clientSeq used. */ - private dispatchAction(channel: string, action: SessionAction | TerminalAction | ClientAnnotationsAction | IRootConfigChangedAction, _clientId: string, clientSeq: number): void { + private dispatchAction(channel: string, action: SessionAction | ChatAction | TerminalAction | ClientAnnotationsAction | IRootConfigChangedAction, _clientId: string, clientSeq: number): void { this._grantImplicitReadsForOutgoingAction(action); this._sendNotification('dispatchAction', { channel, clientSeq, action }); } From b80de3d072d80702a462150238df83778ab0d8ef Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Thu, 23 Jul 2026 14:18:18 -0700 Subject: [PATCH 05/26] Fix agent host file completion refresh Mark host-backed completion lists incomplete so Monaco requests updated fuzzy results as the input token changes. Fixes #326474 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../editor/agentHostInputCompletionsBase.ts | 7 +- .../input/editor/chatInputCompletions.test.ts | 70 ++++++++++++++++++- 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/editor/agentHostInputCompletionsBase.ts b/src/vs/workbench/contrib/chat/browser/widget/input/editor/agentHostInputCompletionsBase.ts index eb26b8a2d9ea58..77c4ff96fbf233 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/editor/agentHostInputCompletionsBase.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/editor/agentHostInputCompletionsBase.ts @@ -79,10 +79,7 @@ export abstract class AgentHostInputCompletionsBase e private async _provide(model: ITextModel, position: Position, token: CancellationToken, triggerCharacters: readonly string[], regData: TRegData): Promise { // Only consult the agent host when the cursor sits inside a token - // led by one of the host-announced trigger characters. Without - // this gate Monaco re-invokes the provider on every keystroke - // (for filtering / incomplete-result refresh), which would - // produce an RPC round-trip per character. + // led by one of the host-announced trigger characters. if (!isAtTriggerCharacterToken(model, position, triggerCharacters)) { return null; } @@ -106,7 +103,7 @@ export abstract class AgentHostInputCompletionsBase e suggestions.push(built); } } - return { suggestions }; + return { suggestions, incomplete: true }; } /** diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/input/editor/chatInputCompletions.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/input/editor/chatInputCompletions.test.ts index 9bd2620690c6da..d705097e535250 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/input/editor/chatInputCompletions.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/input/editor/chatInputCompletions.test.ts @@ -4,14 +4,82 @@ *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; +import { CancellationToken } from '../../../../../../../../base/common/cancellation.js'; +import { DisposableStore, IDisposable } from '../../../../../../../../base/common/lifecycle.js'; import { URI } from '../../../../../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../../../base/test/common/utils.js'; import { Position } from '../../../../../../../../editor/common/core/position.js'; import { Range } from '../../../../../../../../editor/common/core/range.js'; +import { CompletionItem, CompletionItemKind, CompletionTriggerKind } from '../../../../../../../../editor/common/languages.js'; +import { ITextModel } from '../../../../../../../../editor/common/model.js'; +import { LanguageFeaturesService } from '../../../../../../../../editor/common/services/languageFeaturesService.js'; import { createTextModel } from '../../../../../../../../editor/test/common/testTextModel.js'; -import { DisposableStore } from '../../../../../../../../base/common/lifecycle.js'; +import { AgentHostInputCompletionsBase } from '../../../../../browser/widget/input/editor/agentHostInputCompletionsBase.js'; import { attachedContextCompletionSortText, computeCompletionRanges, escapeForCharClass, getAttachedContextCompletionFilterText, isAtTriggerCharacterToken } from '../../../../../browser/widget/input/editor/chatInputCompletionUtils.js'; +import { IChatInputCompletionItem, IChatInputCompletionsParams, IChatInputCompletionsResult } from '../../../../../common/chatSessionsService.js'; import { chatAgentLeader, chatVariableLeader } from '../../../../../common/requestParser/chatParserTypes.js'; +import { MockChatSessionsService } from '../../../../common/mockChatSessionsService.js'; + +class TestChatSessionsService extends MockChatSessionsService { + override async provideChatInputCompletions(_sessionResource: URI, _params: IChatInputCompletionsParams, _token: CancellationToken): Promise { + return { + items: [{ + insertText: '#roadmap.md', + attachment: { + kind: 'resource', + uri: URI.file('/workspace/roadmap.md'), + }, + }], + }; + } +} + +class TestAgentHostInputCompletions extends AgentHostInputCompletionsBase { + register(): IDisposable { + return this._registerProvider({ scheme: 'test' }, 'testAgentHostInputCompletions', ['#'], undefined); + } + + protected override _resolveContext(_model: ITextModel): { sessionResource: URI; context: void } { + return { sessionResource: URI.parse('test:session'), context: undefined }; + } + + protected override _buildItem(position: Position, item: IChatInputCompletionItem): CompletionItem { + return { + label: item.insertText, + insertText: item.insertText, + range: Range.fromPositions(position), + kind: CompletionItemKind.File, + }; + } +} + +suite('AgentHostInputCompletionsBase', () => { + + const store = new DisposableStore(); + + teardown(() => store.clear()); + ensureNoDisposablesAreLeakedInTestSuite(); + + test('marks results incomplete so the host is queried as the token changes', async () => { + const languageFeaturesService = new LanguageFeaturesService(); + const completions = store.add(new TestAgentHostInputCompletions(languageFeaturesService, new TestChatSessionsService())); + store.add(completions.register()); + const model = store.add(createTextModel('#', null, undefined, URI.parse('test:input'))); + const provider = languageFeaturesService.completionProvider.ordered(model)[0]; + + const result = await provider.provideCompletionItems(model, new Position(1, 2), { triggerKind: CompletionTriggerKind.TriggerCharacter, triggerCharacter: '#' }, CancellationToken.None); + + assert.deepStrictEqual(result, { + suggestions: [{ + label: '#roadmap.md', + insertText: '#roadmap.md', + range: new Range(1, 2, 1, 2), + kind: CompletionItemKind.File, + }], + incomplete: true, + }); + }); +}); suite('escapeForCharClass', () => { From 5fbec2bfb07f0951051805a46e1c2feb8ae86390 Mon Sep 17 00:00:00 2001 From: BeniBenj Date: Thu, 23 Jul 2026 23:49:04 +0200 Subject: [PATCH 06/26] refactor: update comments for clarity on assignment context identifiers in onboarding tours --- .../browser/tours/newSessionTour.ts | 5 ++-- .../browser/tours/newSessionViewTour.ts | 5 ++-- .../onboarding/browser/onboardingService.ts | 10 +++++++- .../onboarding/common/onboardingScenario.ts | 15 ++++++------ .../test/browser/onboardingService.test.ts | 24 ++++++++++++++----- 5 files changed, 38 insertions(+), 21 deletions(-) diff --git a/src/vs/sessions/contrib/onboardingTours/browser/tours/newSessionTour.ts b/src/vs/sessions/contrib/onboardingTours/browser/tours/newSessionTour.ts index 68eae0b1ca465d..546a61d9cde213 100644 --- a/src/vs/sessions/contrib/onboardingTours/browser/tours/newSessionTour.ts +++ b/src/vs/sessions/contrib/onboardingTours/browser/tours/newSessionTour.ts @@ -39,9 +39,8 @@ export const NEW_SESSION_ONBOARDING_SEEN_KEY = NEW_SESSION_TOUR_ID; * ExP treatment flag names for Tour 2's A/B experiment. * * - `behaviorFlag` — boolean: `true` shows the tour (treatment), `false` is control. - * - `assignmentContextIdFlag` — string: this tour's assignment-context identifier, - * the key its scorecard groups on. Both arms MUST resolve it to the *same* value, - * which MUST start with the reserved `onb-` prefix (see + * - `assignmentContextIdFlag` — string: the current arm's ExP variant name, which + * MUST start with the reserved `onb-` prefix (see * `ONBOARDING_ASSIGNMENT_CONTEXT_PREFIX`). It is distinct from Tour 1's id so the * two tours report into separate scorecards. */ diff --git a/src/vs/sessions/contrib/onboardingTours/browser/tours/newSessionViewTour.ts b/src/vs/sessions/contrib/onboardingTours/browser/tours/newSessionViewTour.ts index 4997f022ed26f9..e420a92d5fa5ad 100644 --- a/src/vs/sessions/contrib/onboardingTours/browser/tours/newSessionViewTour.ts +++ b/src/vs/sessions/contrib/onboardingTours/browser/tours/newSessionViewTour.ts @@ -42,9 +42,8 @@ export const NEW_SESSION_VIEW_TOUR_ID = 'sessions.onboarding.newSessionView'; * ExP treatment flag names for Tour 1's A/B experiment. * * - `behaviorFlag` — boolean: `true` shows the tour (treatment), `false` is control. - * - `assignmentContextIdFlag` — string: this tour's assignment-context identifier, - * the key its scorecard groups on. Both arms MUST resolve it to the *same* value, - * which MUST start with the reserved `onb-` prefix (see + * - `assignmentContextIdFlag` — string: the current arm's ExP variant name, which + * MUST start with the reserved `onb-` prefix (see * `ONBOARDING_ASSIGNMENT_CONTEXT_PREFIX`). It is distinct from Tour 2's id so the * two tours report into separate scorecards. */ diff --git a/src/vs/workbench/contrib/onboarding/browser/onboardingService.ts b/src/vs/workbench/contrib/onboarding/browser/onboardingService.ts index 0d8f905961884c..70097e873605f1 100644 --- a/src/vs/workbench/contrib/onboarding/browser/onboardingService.ts +++ b/src/vs/workbench/contrib/onboarding/browser/onboardingService.ts @@ -105,7 +105,10 @@ export class OnboardingScenarioService extends Disposable implements IOnboarding // gate has already been opened (this session or persisted from a previous one). this.assignmentService.addTelemetryAssignmentFilter({ id: 'onboarding', - exclude: assignment => assignment.startsWith(ONBOARDING_ASSIGNMENT_CONTEXT_PREFIX) && !this._openedAssignmentContextIds.has(assignment), + exclude: assignment => { + const variant = getAssignmentContextVariant(assignment); + return variant.startsWith(ONBOARDING_ASSIGNMENT_CONTEXT_PREFIX) && !this._openedAssignmentContextIds.has(variant); + }, onDidChange: this._onDidChangeOpenedIds.event }); @@ -575,3 +578,8 @@ export class OnboardingScenarioService extends Disposable implements IOnboarding //#endregion } + +function getAssignmentContextVariant(assignment: string): string { + const separatorIndex = assignment.indexOf(':'); + return separatorIndex === -1 ? assignment : assignment.slice(0, separatorIndex); +} diff --git a/src/vs/workbench/contrib/onboarding/common/onboardingScenario.ts b/src/vs/workbench/contrib/onboarding/common/onboardingScenario.ts index 953c73a253a872..da907bce381426 100644 --- a/src/vs/workbench/contrib/onboarding/common/onboardingScenario.ts +++ b/src/vs/workbench/contrib/onboarding/common/onboardingScenario.ts @@ -39,17 +39,17 @@ export interface IOnboardingPresentationRef { * onboarding would be shown — otherwise the scorecard counts activity from before the * experiment could have had any effect. * - * The treatment flags that tell the client *which* identifier belongs to a given tour resolve + * The treatment flags that tell the client *which* variant belongs to a given tour resolve * asynchronously, after the assignment context has already been committed to telemetry, so * they are too late to block the very first events. This prefix solves that race: because it * is a *static, well-known* string, the client can pre-emptively exclude **any** * assignment-context entry that starts with it from the very first event, and then - * selectively unblock the one specific identifier once the user reaches the onboarding moment. + * selectively unblock the one specific variant once the user reaches the onboarding moment. * * ### Contract for experiment authors - * - Every onboarding experiment configured in ExP MUST set its assignment-context identifier - * to a value beginning with this prefix. - * - That same value MUST be returned by the experiment's + * - Every onboarding experiment configured in ExP MUST use variant names beginning with this + * prefix. + * - Each arm's variant name MUST be returned by the experiment's * {@link IOnboardingExperiment.assignmentContextIdFlag} treatment flag. * * The engine enforces this defensively: an id that lacks the prefix is rejected (the experiment @@ -79,9 +79,8 @@ export interface IOnboardingExperiment { readonly behaviorFlag: string; /** - * Name of the string treatment flag whose value is this experiment's assignment-context - * identifier — the exact entry that appears inside the telemetry `abexp.assignmentcontext` - * property and that the scorecard keys on. The value MUST start with + * Name of the string treatment flag whose value is the current arm's ExP variant name. The + * telemetry entry appends `:`; the value itself MUST start with * {@link ONBOARDING_ASSIGNMENT_CONTEXT_PREFIX}; an id without the prefix is rejected (the * experiment is treated as inactive) so it can never leak into telemetry ungated. */ diff --git a/src/vs/workbench/contrib/onboarding/test/browser/onboardingService.test.ts b/src/vs/workbench/contrib/onboarding/test/browser/onboardingService.test.ts index cc5208cc33a700..5fa447d425198c 100644 --- a/src/vs/workbench/contrib/onboarding/test/browser/onboardingService.test.ts +++ b/src/vs/workbench/contrib/onboarding/test/browser/onboardingService.test.ts @@ -399,17 +399,29 @@ suite('OnboardingScenarioService', () => { presentation: { kind: presentation.kind, payload: undefined } }); - // Before resolution the id is blocked from telemetry by the prefix filter. + const assignmentContext = 'onb-tour-q3:12345'; const { service } = createService({}, assignment); - assert.strictEqual(assignment.isExcluded('onb-tour-q3'), true, 'blocked before would-show'); + const excludedBeforeWouldShow = assignment.isExcluded(assignmentContext); service.start(); await timeout(0); await timeout(0); assert.deepStrictEqual( - { runs: presentation.runs, shown: service.hasBeenShown('exp-treat'), excluded: assignment.isExcluded('onb-tour-q3') }, - { runs: ['exp-treat'], shown: true, excluded: false } + { + excludedBeforeWouldShow, + runs: presentation.runs, + shown: service.hasBeenShown('exp-treat'), + excludedAfterWouldShow: assignment.isExcluded(assignmentContext), + otherVariantExcluded: assignment.isExcluded('onb-tour-q3-other:12346') + }, + { + excludedBeforeWouldShow: true, + runs: ['exp-treat'], + shown: true, + excludedAfterWouldShow: false, + otherVariantExcluded: true + } ); }); @@ -431,7 +443,7 @@ suite('OnboardingScenarioService', () => { // No tour shown, not marked shown (re-eligible later), but the id now flows. assert.deepStrictEqual( - { runs: presentation.runs, shown: service.hasBeenShown('exp-control'), excluded: assignment.isExcluded('onb-tour-q3') }, + { runs: presentation.runs, shown: service.hasBeenShown('exp-control'), excluded: assignment.isExcluded('onb-tour-q3:12345') }, { runs: [], shown: false, excluded: false } ); }); @@ -509,7 +521,7 @@ suite('OnboardingScenarioService', () => { const secondAssignment = new FakeAssignmentService({ 'exp.show': false, 'exp.id': 'onb-tour-q3' }); createService({}, secondAssignment, storage); - assert.strictEqual(secondAssignment.isExcluded('onb-tour-q3'), false); + assert.strictEqual(secondAssignment.isExcluded('onb-tour-q3:12345'), false); }); test('a second experiment with a new id is blocked for a user who already saw the tour', async () => { From 33ee03a6acd00081492afcde5ccea345d368f895 Mon Sep 17 00:00:00 2001 From: Benjamin Christopher Simmonds <44439583+benibenj@users.noreply.github.com> Date: Thu, 23 Jul 2026 23:58:55 +0200 Subject: [PATCH 07/26] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../workbench/contrib/onboarding/browser/onboardingService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/onboarding/browser/onboardingService.ts b/src/vs/workbench/contrib/onboarding/browser/onboardingService.ts index 70097e873605f1..6b84d2dd895f8c 100644 --- a/src/vs/workbench/contrib/onboarding/browser/onboardingService.ts +++ b/src/vs/workbench/contrib/onboarding/browser/onboardingService.ts @@ -580,6 +580,6 @@ export class OnboardingScenarioService extends Disposable implements IOnboarding } function getAssignmentContextVariant(assignment: string): string { - const separatorIndex = assignment.indexOf(':'); + const separatorIndex = assignment.lastIndexOf(':'); return separatorIndex === -1 ? assignment : assignment.slice(0, separatorIndex); } From e2e76248f08e74878caf01851fedc0884cb33a2d Mon Sep 17 00:00:00 2001 From: Benjamin Christopher Simmonds <44439583+benibenj@users.noreply.github.com> Date: Fri, 24 Jul 2026 00:45:15 +0200 Subject: [PATCH 08/26] Keep empty session groups visible (#327191) * Keep empty session groups visible Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Show delete action for empty session groups Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/vs/sessions/SESSIONS_LIST.md | 10 ++- .../sessions/browser/views/sessionsList.ts | 62 ++++++++++++++----- .../browser/views/sessionsViewActions.ts | 28 ++++++++- .../sessions/browser/sessionGroupsService.ts | 47 ++------------ .../test/browser/sessionGroupsService.test.ts | 37 +++++------ 5 files changed, 101 insertions(+), 83 deletions(-) diff --git a/src/vs/sessions/SESSIONS_LIST.md b/src/vs/sessions/SESSIONS_LIST.md index 803b73342410d3..2523e61b4b1685 100644 --- a/src/vs/sessions/SESSIONS_LIST.md +++ b/src/vs/sessions/SESSIONS_LIST.md @@ -62,7 +62,7 @@ Two grouping modes (user-switchable): - **By Workspace** (default) — user groups and one section per workspace label share a single, freely-reorderable user-managed order below Pinned. By default groups come first and workspaces are alphabetical ("Unknown" workspace last) until the user drags them. - **By Date** — user groups form a contiguous, user-ordered block directly below Pinned; the non-grouped sessions follow in the fixed date sections (Recent, Older), where Recent holds up to 10 sessions from the last 7 days and Older holds the rest. Groups never mix into the date sections. -User groups are **fully user-managed**: their order is owned by `ISessionSectionOrderService`, defaults to newest-first, and is shared across both grouping modes (it no longer derives from the recency of a group's member sessions). +User groups are **fully user-managed**: their order is owned by `ISessionSectionOrderService`, defaults to newest-first, and is shared across both grouping modes (it no longer derives from the recency of a group's member sessions). Groups remain visible and persisted until explicitly deleted. A group with no currently-visible member rows renders a muted **"No session" placeholder row** like the empty Chats section; this includes genuinely empty groups and groups whose members currently render in Pinned/Done or are hidden by a filter. Archived sessions always go to the "Done" section regardless of grouping mode. Archive wins over pin — an archived session is never shown in Pinned. @@ -118,7 +118,7 @@ Regular sessions can be reordered by dragging them up or down within the list. P - **Grouping by Date** — the regular list is one continuous sequence, so dragging can move a session across date buckets (e.g. to the top makes it "Recent"). - **Grouping by Workspace** — reordering is restricted to within the same workspace group; drops onto another workspace are rejected. - **Pinned** — dropping a non-archived session on the Pinned header pins it and lets it sort naturally. Dropping it on a pinned session shows an insertion line, pins it, and stores the sort key needed to place it at that location. -- **User groups** — dropping a non-archived session on a group header adds or moves it into the group and lets it sort naturally. Dropping it on a session inside the group shows an insertion line for the exact slot and highlights only the group header to indicate the receiving group. +- **User groups** — dropping a non-archived session on a group header or its "No session" placeholder adds or moves it into the group and lets it sort naturally. Dropping it on a session inside the group shows an insertion line for the exact slot and highlights only the group header to indicate the receiving group. - **Scope** — archived (Done) sessions do not reorder or move into groups. Drops onto the Done section, unsupported section headers, and "show more" rows are rejected. - **Multi-selection** — dragging multiple selected sessions moves them as a contiguous block, preserving their relative order. The drag label reads `"N sessions"`. Dragging sessions into the sessions grid opens all of them. @@ -136,6 +136,8 @@ The insertion line relies on the base list widget's `drop-target-before`/`drop-t Archived sessions do not show the session group context menu actions ("Create Group", "Add to Group", "Move to Group", or "Remove from Group"). +The **Create Group** context-menu action is also available from list blank space, section headers, group headers, "show more" rows, and placeholder rows. These non-session entry points create an empty group and immediately start inline renaming; the session-row action creates the group with the selected sessions as before. + ### Read / Unread - Read/unread state is **owned by the sessions provider** and surfaced via `ISession.isRead`. Marking happens through `ISessionsManagementService.markRead` / `markUnread` / `markAllRead`, which route to the provider's `setSessionReadState`. The agent-host provider persists it via the protocol `IsRead` status bit; the Copilot Chat provider via its agent session model (`setRead`); the local chat provider via its persisted session metadata. @@ -183,7 +185,7 @@ The sessions list defines menu IDs that contributions can target to add actions. | Menu | Constant | Where it appears | Use for | |------|----------|------------------|---------| -| `SessionGroupToolbar` | `SessionGroupToolbarMenuId` | Toolbar on user-created group headers | Group-scoped actions: "New Session" (opens the new-session composer and joins the started session to the group), "Rename", and "Delete Group". The "New Session" intent is recorded via `ISessionGroupsService.setPendingNewSessionGroup` and bound to the committed session when it is started; abandoning the new session clears it. | +| `SessionGroupToolbar` | `SessionGroupToolbarMenuId` | Toolbar on user-created group headers | Group-scoped actions: "Mark All as Done" when the group has visible sessions, "Delete Group" when it has no members, and "New Session" (opens the new-session composer and joins the started session to the group). A group whose members are all pinned, archived, or filtered out shows neither destructive action. The "New Session" intent is recorded via `ISessionGroupsService.setPendingNewSessionGroup` and bound to the committed session when it is started; abandoning the new session clears it. | ### View Title Menus @@ -238,6 +240,8 @@ Context keys available for `when` clauses when contributing to session list menu | Key | Type | Description | |-----|------|-------------| | `sessionSection.type` | string | `'pinned'`, `'quickchats'`, `'archived'`, `'workspace: