From 7861ea9388f7e2cc54d06fbdd26fa40037b6cf14 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Thu, 1 Oct 2026 13:57:10 -0700 Subject: [PATCH 1/3] automations: add customization selection and save progress Adds a way to select the customizations that an automation syncs, and shows progress while the agent host syncs them. - Adds an "Advanced" section to the automation dialog with a checkbox list of customizations. A saved customization that changed locally shows an "Outdated" pill. Saving the automation updates it. - Keeps the automation dialog open while the agent host saves the change. Shows a progress bar and a status, and shows errors in the dialog. - Adds getCustomizationChoices and customizationIds to the automation store contract. - Uses file plugins at their original paths when a local window saves the automation, so the local agent host does not copy them. (Commit message generated by Copilot) --- .../agentHost/common/agentPluginManager.ts | 5 +- .../node/agentHostAutomationCustomizations.ts | 27 +- .../node/agentHostAutomationService.ts | 4 +- .../node/agentHostClientConnectionService.ts | 13 + .../agentHost/node/agentPluginManager.ts | 4 +- .../agentHost/node/protocolServerHandler.ts | 6 + .../agentHostAutomationCustomizations.test.ts | 60 +++- .../node/agentHostAutomationService.test.ts | 35 ++- .../agentHostClientConnectionService.test.ts | 24 ++ .../node/agentHostToolCallTelemetry.test.ts | 1 + .../node/agentHostTurnHangTelemetry.test.ts | 2 + .../test/node/agentPluginManager.test.ts | 23 ++ .../test/node/protocolServerHandler.test.ts | 31 ++ src/vs/sessions/AUTOMATIONS.md | 4 + .../automations/browser/automationDialog.ts | 245 ++++++++++++++-- .../browser/automationDialogService.ts | 119 +++++--- .../browser/media/automationDialog.css | 145 +++++++++- .../browser/providerAutomationService.ts | 9 +- .../test/browser/automationDialog.test.ts | 241 +++++++++++++++- .../browser/providerAutomationService.test.ts | 27 ++ .../browser/agentHostAutomationStore.ts | 115 ++++++-- .../reconnectableAgentHostAutomationStore.ts | 8 +- .../browser/agentHostAutomationStore.test.ts | 265 +++++++++++++++++- .../sessions/browser/views/automationsView.ts | 147 +++++----- .../test/browser/automationsView.test.ts | 57 ++-- .../automations/automationDialogService.ts | 12 +- .../common/automations/automationService.ts | 30 ++ 27 files changed, 1436 insertions(+), 223 deletions(-) diff --git a/src/vs/platform/agentHost/common/agentPluginManager.ts b/src/vs/platform/agentHost/common/agentPluginManager.ts index ba7e75c88fa0cc..2a49cb1d7b2e12 100644 --- a/src/vs/platform/agentHost/common/agentPluginManager.ts +++ b/src/vs/platform/agentHost/common/agentPluginManager.ts @@ -9,6 +9,9 @@ import type { ClientPluginCustomization, PluginCustomization } from './state/ses export const IAgentPluginManager = createDecorator('agentPluginManager'); +/** Static active-client identity used for host-resolved automation plugins. */ +export const AUTOMATION_ACTIVE_CLIENT_ID = 'vscode.automation'; + /** * A synced customization with its local plugin directory (when available). */ @@ -37,7 +40,7 @@ export interface IAgentPluginManager { */ readonly basePath: URI; - /** Immutable host-owned plugin directories. File URIs equal to or under this path are synced in place, without cache entries, regardless of clientId. */ + /** Immutable host-owned plugin directories. File URIs under this path, or synced for AUTOMATION_ACTIVE_CLIENT_ID, are used in place without cache entries. */ readonly hostPluginsPath: URI; /** diff --git a/src/vs/platform/agentHost/node/agentHostAutomationCustomizations.ts b/src/vs/platform/agentHost/node/agentHostAutomationCustomizations.ts index 786a31c1b1c723..af83ea22e89786 100644 --- a/src/vs/platform/agentHost/node/agentHostAutomationCustomizations.ts +++ b/src/vs/platform/agentHost/node/agentHostAutomationCustomizations.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { createHash } from 'crypto'; +import { Schemas } from '../../../base/common/network.js'; import { extUriBiasedIgnorePathCase, joinPath } from '../../../base/common/resources.js'; import { URI } from '../../../base/common/uri.js'; import { generateUuid } from '../../../base/common/uuid.js'; @@ -12,14 +13,13 @@ import { parsePlugin } from '../../agentPlugins/common/pluginParsers.js'; import { IFileService } from '../../files/common/files.js'; import { ILogService } from '../../log/common/log.js'; import { toAgentClientUri } from '../common/agentClientUri.js'; +import { AUTOMATION_ACTIVE_CLIENT_ID } from '../common/agentPluginManager.js'; import type { AutomationEntry, AutomationSessionTemplate } from '../common/state/protocol/channels-automation/state.js'; import { CustomizationLoadStatus, CustomizationType, type AgentSelection, type ClientPluginCustomization, type PluginCustomization, type SessionActiveClient } from '../common/state/sessionState.js'; import { toChildCustomizations } from './copilot/copilotPluginConverters.js'; -/** Static active-client identity used for captured automation plugins. */ -export const AUTOMATION_ACTIVE_CLIENT_ID = 'vscode.automation'; -/** Owns immutable automation plugin copies, independently of connected clients and the plugin cache. */ +/** Captures automation plugins as host paths, independently of connected clients and the plugin cache. */ export class AgentHostAutomationCustomizations { private readonly _path: URI; /** Copies handed to run sessions in this process; those sessions keep using them in place for follow-up turns. */ @@ -35,7 +35,7 @@ export class AgentHostAutomationCustomizations { } /** Captures changed references atomically for the caller, reusing unchanged entries without contacting their client. */ - async capture(clientId: string | undefined, next: readonly ClientPluginCustomization[] | undefined, previous: AutomationEntry | undefined): Promise { + async capture(clientId: string | undefined, next: readonly ClientPluginCustomization[] | undefined, previous: AutomationEntry | undefined, isLocalClient = false): Promise { if (!next?.length) { return undefined; } @@ -56,18 +56,23 @@ export class AgentHostAutomationCustomizations { if (!clientId) { throw new Error('Capturing automation customizations requires a dispatching client.'); } - const key = ref.nonce === undefined ? generateUuid() : createHash('sha256').update(`${ref.uri}\n${ref.nonce}`).digest('hex'); - const destination = joinPath(this._path, key); - if (!await this._fileService.exists(destination)) { - const staging = joinPath(this._path, `.staging-${generateUuid()}`); - await this._fileService.copy(toAgentClientUri(URI.parse(ref.uri), clientId), staging); - await this._fileService.move(staging, destination); + const uri = URI.parse(ref.uri); + const inPlace = isLocalClient && uri.scheme === Schemas.file; + let destination = uri; + if (!inPlace) { + const key = ref.nonce === undefined ? generateUuid() : createHash('sha256').update(`${ref.uri}\n${ref.nonce}`).digest('hex'); + destination = joinPath(this._path, key); + if (!await this._fileService.exists(destination)) { + const staging = joinPath(this._path, `.staging-${generateUuid()}`); + await this._fileService.copy(toAgentClientUri(uri, clientId), staging); + await this._fileService.move(staging, destination); + } } const parsed = await parsePlugin(destination, this._fileService, undefined, this._userHome, destination); copy = { type: CustomizationType.Plugin, id: ref.id, - uri: destination.toString(), + uri: inPlace ? ref.uri : destination.toString(), name: ref.name, children: toChildCustomizations([parsed]), load: { kind: CustomizationLoadStatus.Loaded }, diff --git a/src/vs/platform/agentHost/node/agentHostAutomationService.ts b/src/vs/platform/agentHost/node/agentHostAutomationService.ts index 45efd2711c22fe..3971e7f07d63fe 100644 --- a/src/vs/platform/agentHost/node/agentHostAutomationService.ts +++ b/src/vs/platform/agentHost/node/agentHostAutomationService.ts @@ -230,7 +230,7 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost } const timestamp = new Date().toISOString(); - const customizations = await this._customizations.capture(clientId, definition.session.customizations, undefined); + const customizations = await this._customizations.capture(clientId, definition.session.customizations, undefined, clientId !== undefined && this._clientConnections.isLocalClient(clientId)); const automation = this._withInitialScheduleState({ resource: action.resource, definition, @@ -274,7 +274,7 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost }; this._validateDefinition(automation.definition); if (action.changes.session !== undefined) { - automation.customizations = await this._customizations.capture(clientId, action.changes.session.customizations, existing); + automation.customizations = await this._customizations.capture(clientId, action.changes.session.customizations, existing, clientId !== undefined && this._clientConnections.isLocalClient(clientId)); } if (action.changes.triggers !== undefined || action.changes.enabled !== undefined) { automation = this._withInitialScheduleState(automation, new Date()); diff --git a/src/vs/platform/agentHost/node/agentHostClientConnectionService.ts b/src/vs/platform/agentHost/node/agentHostClientConnectionService.ts index fde3ff4c85d3a3..7ffd8b48283efe 100644 --- a/src/vs/platform/agentHost/node/agentHostClientConnectionService.ts +++ b/src/vs/platform/agentHost/node/agentHostClientConnectionService.ts @@ -18,6 +18,8 @@ export interface IAgentHostClientConnectionCounts { export interface IAgentHostClientConnectionSource { hasSeenClient(clientId: string): boolean; isClientConnected(clientId: string): boolean; + /** Whether an active connection is local and uses the server's MessagePort transport. */ + isLocalClient(clientId: string): boolean; getConnectedClientTransportCounts(): ReadonlyMap; requestWorkspaceTrust(clientId: string, request: IAgentHostWorkspaceTrustRequest): Promise; /** Requests silent MCP authentication from connected clients. */ @@ -31,6 +33,8 @@ export interface IAgentHostClientConnectionService { registerSource(source: IAgentHostClientConnectionSource): IDisposable; hasSeenClient(clientId: string): boolean; isClientConnected(clientId: string): boolean; + /** Whether any source has an active local MessagePort connection for this client. */ + isLocalClient(clientId: string): boolean; getConnectionCounts(clientId: string): IAgentHostClientConnectionCounts; requestWorkspaceTrust(clientId: string, request: IAgentHostWorkspaceTrustRequest): Promise; /** Requests silent MCP authentication, stopping at the first successful source. */ @@ -72,6 +76,15 @@ export class AgentHostClientConnectionService extends Disposable implements IAge return false; } + isLocalClient(clientId: string): boolean { + for (const source of this._sources) { + if (source.isLocalClient(clientId)) { + return true; + } + } + return false; + } + getConnectionCounts(clientId: string): IAgentHostClientConnectionCounts { const connectedClients = new Set(); let connectedTransportCount = 0; diff --git a/src/vs/platform/agentHost/node/agentPluginManager.ts b/src/vs/platform/agentHost/node/agentPluginManager.ts index 2ce86ab9f17562..d481331a0533aa 100644 --- a/src/vs/platform/agentHost/node/agentPluginManager.ts +++ b/src/vs/platform/agentHost/node/agentPluginManager.ts @@ -10,7 +10,7 @@ import { Schemas } from '../../../base/common/network.js'; import { extUriBiasedIgnorePathCase } from '../../../base/common/resources.js'; import { FileOperationResult, IFileService, toFileOperationResult } from '../../files/common/files.js'; import { ILogService } from '../../log/common/log.js'; -import { IAgentPluginManager, type ISyncedCustomization } from '../common/agentPluginManager.js'; +import { AUTOMATION_ACTIVE_CLIENT_ID, IAgentPluginManager, type ISyncedCustomization } from '../common/agentPluginManager.js'; import { CustomizationLoadStatus, type ClientPluginCustomization, type PluginCustomization } from '../common/state/sessionState.js'; import { toAgentClientUri } from '../common/agentClientUri.js'; @@ -142,7 +142,7 @@ export class AgentPluginManager implements IAgentPluginManager { private async _syncPlugin(clientId: string, ref: ClientPluginCustomization): Promise { const uri = URI.parse(ref.uri); // Normalize so `..` segments cannot escape the host-owned directory. - if (uri.scheme === Schemas.file && extUriBiasedIgnorePathCase.isEqualOrParent(extUriBiasedIgnorePathCase.normalizePath(uri), this.hostPluginsPath)) { + if (uri.scheme === Schemas.file && (clientId === AUTOMATION_ACTIVE_CLIENT_ID || extUriBiasedIgnorePathCase.isEqualOrParent(extUriBiasedIgnorePathCase.normalizePath(uri), this.hostPluginsPath))) { return uri; } const pluginUri = toAgentClientUri(uri, clientId); diff --git a/src/vs/platform/agentHost/node/protocolServerHandler.ts b/src/vs/platform/agentHost/node/protocolServerHandler.ts index 4349fa5e037b26..d3c933f9903244 100644 --- a/src/vs/platform/agentHost/node/protocolServerHandler.ts +++ b/src/vs/platform/agentHost/node/protocolServerHandler.ts @@ -1390,6 +1390,12 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien && record.connections.some(connection => connection.telemetryConnectionActive); } + isLocalClient(clientId: string): boolean { + const record = this._clients.get(clientId); + return record?.state === 'active' + && record.connections.some(connection => connection.telemetryContext.connectionKind === AgentHostClientConnectionKind.Local); + } + getConnectedClientTransportCounts(): ReadonlyMap { const result = new Map(); for (const [clientId, record] of this._clients) { diff --git a/src/vs/platform/agentHost/test/node/agentHostAutomationCustomizations.test.ts b/src/vs/platform/agentHost/test/node/agentHostAutomationCustomizations.test.ts index 9d405c2258f5b3..307cec25461b55 100644 --- a/src/vs/platform/agentHost/test/node/agentHostAutomationCustomizations.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostAutomationCustomizations.test.ts @@ -17,7 +17,8 @@ import { AGENT_CLIENT_SCHEME, toAgentClientUri } from '../../common/agentClientU import { CustomizationType, MessageKind, type ClientPluginCustomization, type PluginCustomization } from '../../common/state/sessionState.js'; import { CustomizationEnablementKind } from '../../common/state/protocol/channels-session/state.js'; import type { AutomationEntry } from '../../common/state/protocol/channels-automation/state.js'; -import { AgentHostAutomationCustomizations, AUTOMATION_ACTIVE_CLIENT_ID } from '../../node/agentHostAutomationCustomizations.js'; +import { AgentHostAutomationCustomizations } from '../../node/agentHostAutomationCustomizations.js'; +import { AUTOMATION_ACTIVE_CLIENT_ID } from '../../common/agentPluginManager.js'; suite('AgentHostAutomationCustomizations', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); @@ -88,6 +89,63 @@ suite('AgentHostAutomationCustomizations', () => { }); }); + test('captures local file plugins in place, reuses them, and never garbage collects their paths', async () => { + const directory = URI.file('/local/bundle'); + const localRef = { ...ref, uri: directory.toString() }; + await fileService.writeFile(URI.joinPath(directory, '.plugin/plugin.json'), VSBuffer.fromString('{"name":"bundle"}')); + await fileService.writeFile(URI.joinPath(directory, 'agents/reviewer.md'), VSBuffer.fromString('---\nname: Reviewer\n---\nReview.')); + const copySpy = sinon.spy(fileService, 'copy'); + const copies = (await customizations.capture('author', [localRef], undefined, true))!; + const reused = await customizations.capture(undefined, [{ ...localRef, name: 'Renamed' }], entry([localRef], copies)); + await fileService.createFolder(URI.joinPath(root, 'unused')); + await customizations.collectGarbage([]); + assert.deepStrictEqual({ + uri: copies[0].uri, + children: copies[0].children?.map(child => ({ type: child.type, name: child.name, uri: child.uri })), + load: copies[0].load, + reused, + copyCount: copySpy.callCount, + collected: (await fileService.resolve(root)).children, + sourceExists: await fileService.exists(directory), + }, { + uri: localRef.uri, + children: [{ type: CustomizationType.Agent, name: 'Reviewer', uri: URI.joinPath(directory, 'agents/reviewer.md').toString() }], + load: { kind: 'loaded' }, + reused: [{ ...copies[0], name: 'Renamed' }], + copyCount: 0, + collected: [], + sourceExists: true, + }); + }); + + test('still copies virtual plugins for local clients', async () => { + await seed(); + const [copy] = (await customizations.capture('author', [ref], undefined, true))!; + const destination = URI.joinPath(root, createHash('sha256').update(`${ref.uri}\n${ref.nonce}`).digest('hex')); + assert.deepStrictEqual({ + uri: copy.uri, + children: copy.children?.map(child => child.uri), + }, { + uri: destination.toString(), + children: [URI.joinPath(destination, 'agents/reviewer.md').toString()], + }); + }); + + test('still copies file plugins for remote clients', async () => { + const remoteRef = { ...ref, uri: URI.file('/remote/bundle').toString() }; + const source = toAgentClientUri(URI.parse(remoteRef.uri), 'author'); + await fileService.writeFile(URI.joinPath(source, '.plugin/plugin.json'), VSBuffer.fromString('{"name":"bundle"}')); + const [copy] = (await customizations.capture('author', [remoteRef], undefined, false))!; + const destination = URI.joinPath(root, createHash('sha256').update(`${remoteRef.uri}\n${remoteRef.nonce}`).digest('hex')); + assert.deepStrictEqual({ + uri: copy.uri, + manifest: (await fileService.readFile(URI.joinPath(destination, '.plugin/plugin.json'))).value.toString(), + sourceExistsOnHost: await fileService.exists(URI.parse(remoteRef.uri)), + }, { + uri: destination.toString(), manifest: '{"name":"bundle"}', sourceExistsOnHost: false, + }); + }); + test('shares nonce revisions across automations but not captures without a nonce', async () => { await seed(); const first = (await customizations.capture('author', [ref], undefined))!; diff --git a/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts b/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts index b13008e47dabb8..396b5c48c8de78 100644 --- a/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts @@ -39,7 +39,7 @@ import { FileService } from '../../../files/common/fileService.js'; import { InMemoryFileSystemProvider } from '../../../files/common/inMemoryFilesystemProvider.js'; import { AGENT_CLIENT_SCHEME, toAgentClientUri } from '../../common/agentClientUri.js'; import { AgentPluginManager } from '../../node/agentPluginManager.js'; -import { AUTOMATION_ACTIVE_CLIENT_ID } from '../../node/agentHostAutomationCustomizations.js'; +import { AUTOMATION_ACTIVE_CLIENT_ID } from '../../common/agentPluginManager.js'; import { AgentHostClientConnectionService } from '../../node/agentHostClientConnectionService.js'; import type { IAgentHostMcpAuthenticationRequest } from '../../common/agentHostExtensionProtocol.js'; import { McpAuthRequiredReason, McpServerStatus, type Customization, type McpAuthRequirement } from '../../common/state/protocol/channels-session/state.js'; @@ -74,6 +74,7 @@ suite('AgentHostAutomationService', () => { disposables.add(clientConnections.registerSource({ hasSeenClient: () => false, isClientConnected: () => false, + isLocalClient: () => false, getConnectedClientTransportCounts: () => new Map(), requestWorkspaceTrust: async () => false, requestMcpAuthentication: request => { @@ -259,6 +260,38 @@ suite('AgentHostAutomationService', () => { }); }); + test('creates and updates local file customizations in place using connection service locality', async () => { + disposables.add(clientConnections.registerSource({ + hasSeenClient: clientId => clientId === 'local', + isClientConnected: clientId => clientId === 'local', + isLocalClient: clientId => clientId === 'local', + getConnectedClientTransportCounts: () => new Map([['local', 1]]), + requestWorkspaceTrust: async () => true, + requestMcpAuthentication: async () => false, + })); + const ref: ClientPluginCustomization = { type: CustomizationType.Plugin, id: 'bundle', uri: URI.file('/local/bundle').toString(), name: 'Bundle', nonce: 'one' }; + await fileService.writeFile(URI.joinPath(URI.parse(ref.uri), '.plugin/plugin.json'), VSBuffer.fromString('{"name":"bundle"}')); + const service = createService(); + const action = createAction(); + action.definition.session.customizations = [ref]; + await service.handleCreate(action, 'local'); + const created = stateManager.getAutomationCatalogState()!.entries[0].customizations; + const updatedRef = { ...ref, nonce: 'two' }; + await service.handleUpdate({ + type: ActionType.AutomationUpdateRequested, resource: action.resource, + changes: { session: { ...action.definition.session, customizations: [updatedRef] } }, + }, 'local'); + const updated = stateManager.getAutomationCatalogState()!.entries[0]; + assert.deepStrictEqual({ + createdUris: created?.map(copy => copy.uri), + updatedUris: updated.customizations?.map(copy => copy.uri), + refs: updated.definition.session.customizations, + copiesExist: await fileService.exists(URI.joinPath(pluginManager.hostPluginsPath, 'automations')), + }, { + createdUris: [ref.uri], updatedUris: [ref.uri], refs: [updatedRef], copiesExist: false, + }); + }); + test('keeps copies on unrelated updates and rejects failed captures atomically', async () => { const service = createService(); const action = createAction(); diff --git a/src/vs/platform/agentHost/test/node/agentHostClientConnectionService.test.ts b/src/vs/platform/agentHost/test/node/agentHostClientConnectionService.test.ts index 090cc6bc5c655b..f97280970524f6 100644 --- a/src/vs/platform/agentHost/test/node/agentHostClientConnectionService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostClientConnectionService.test.ts @@ -12,6 +12,29 @@ import { AgentHostClientConnectionService } from '../../node/agentHostClientConn suite('AgentHostClientConnectionService', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); + test('reports locality from registered sources and drops it when a source is removed', () => { + const service = store.add(new AgentHostClientConnectionService()); + const withoutSources = service.isLocalClient('local'); + for (const local of [false, true]) { + const registration = store.add(service.registerSource({ + hasSeenClient: () => true, + isClientConnected: () => true, + isLocalClient: clientId => local && clientId === 'local', + getConnectedClientTransportCounts: () => new Map(), + requestWorkspaceTrust: async () => false, + requestMcpAuthentication: async () => false, + })); + if (local) { + const withLocalSource = service.isLocalClient('local'); + const otherClient = service.isLocalClient('other'); + registration.dispose(); + assert.deepStrictEqual({ withoutSources, withLocalSource, otherClient, afterRemoval: service.isLocalClient('local') }, { + withoutSources: false, withLocalSource: true, otherClient: false, afterRemoval: false, + }); + } + } + }); + test('requests MCP authentication from sources in order until successful', async () => { const service = store.add(new AgentHostClientConnectionService()); const request: IAgentHostMcpAuthenticationRequest = { @@ -24,6 +47,7 @@ suite('AgentHostClientConnectionService', () => { store.add(service.registerSource({ hasSeenClient: () => false, isClientConnected: () => false, + isLocalClient: () => false, getConnectedClientTransportCounts: () => new Map(), requestWorkspaceTrust: async () => false, requestMcpAuthentication: async request => { diff --git a/src/vs/platform/agentHost/test/node/agentHostToolCallTelemetry.test.ts b/src/vs/platform/agentHost/test/node/agentHostToolCallTelemetry.test.ts index 1a1fe2293d773c..5e3160010f6b35 100644 --- a/src/vs/platform/agentHost/test/node/agentHostToolCallTelemetry.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostToolCallTelemetry.test.ts @@ -173,6 +173,7 @@ suite('AgentSideEffects — tool call telemetry', () => { const source: IAgentHostClientConnectionSource = { hasSeenClient: candidate => candidate === clientId, isClientConnected: candidate => connected && candidate === clientId, + isLocalClient: () => false, getConnectedClientTransportCounts: () => connected ? new Map([[clientId, 1]]) : new Map(), requestWorkspaceTrust: async () => false, requestMcpAuthentication: async () => false, diff --git a/src/vs/platform/agentHost/test/node/agentHostTurnHangTelemetry.test.ts b/src/vs/platform/agentHost/test/node/agentHostTurnHangTelemetry.test.ts index f66479a8de8120..81536552475284 100644 --- a/src/vs/platform/agentHost/test/node/agentHostTurnHangTelemetry.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostTurnHangTelemetry.test.ts @@ -222,6 +222,7 @@ suite('AgentSideEffects — turn hang telemetry', () => { disposables.add(clientConnections.registerSource({ hasSeenClient: clientId => clientId === 'test', isClientConnected: clientId => clientId === 'test', + isLocalClient: () => false, getConnectedClientTransportCounts: () => new Map([['test', 1]]), requestWorkspaceTrust: async () => true, requestMcpAuthentication: async () => false, @@ -595,6 +596,7 @@ suite('AgentSideEffects — turn hang telemetry', () => { disposables.add(clientConnections.registerSource({ hasSeenClient: clientId => clientId === 'connected-client', isClientConnected: clientId => clientId === 'connected-client', + isLocalClient: () => false, getConnectedClientTransportCounts: () => new Map([['connected-client', 1]]), requestWorkspaceTrust: async () => true, requestMcpAuthentication: async () => false, diff --git a/src/vs/platform/agentHost/test/node/agentPluginManager.test.ts b/src/vs/platform/agentHost/test/node/agentPluginManager.test.ts index 402c9943da097d..44023310ec9677 100644 --- a/src/vs/platform/agentHost/test/node/agentPluginManager.test.ts +++ b/src/vs/platform/agentHost/test/node/agentPluginManager.test.ts @@ -15,6 +15,7 @@ import { IFileDeleteOptions } from '../../../files/common/files.js'; import { InMemoryFileSystemProvider } from '../../../files/common/inMemoryFilesystemProvider.js'; import { NullLogService } from '../../../log/common/log.js'; import { AGENT_CLIENT_SCHEME, toAgentClientUri } from '../../common/agentClientUri.js'; +import { AUTOMATION_ACTIVE_CLIENT_ID } from '../../common/agentPluginManager.js'; import { customizationId, type ClientPluginCustomization, type PluginCustomization } from '../../common/state/sessionState.js'; import { CustomizationType } from '../../common/state/protocol/state.js'; import { AgentPluginManager } from '../../node/agentPluginManager.js'; @@ -110,6 +111,28 @@ suite('AgentPluginManager', () => { suite('syncCustomizations', () => { + test('uses file plugins in place only for the automation active client outside host directories', async () => { + disposables.add(fileService.registerProvider(Schemas.file, disposables.add(new InMemoryFileSystemProvider()))); + const directory = URI.file('/local/bundle'); + const ref = { ...makeRef('local', 'revision'), uri: directory.toString() }; + await fileService.writeFile(URI.joinPath(directory, 'index.js'), VSBuffer.fromString('original')); + await fileService.writeFile(URI.joinPath(toAgentClientUri(directory, 'test-client'), 'index.js'), VSBuffer.fromString('client-served')); + const [inPlace] = await manager.syncCustomizations(AUTOMATION_ACTIVE_CLIENT_ID, [ref]); + const cacheExistsBeforeCopy = await fileService.exists(URI.joinPath(manager.basePath, 'cache.json')); + const [copied] = await manager.syncCustomizations('test-client', [ref]); + assert.deepStrictEqual({ + inPlace: inPlace.pluginDir?.toString(), + inPlaceLoad: inPlace.customization.load, + cacheExistsBeforeCopy, + copied: copied.pluginDir?.toString() !== directory.toString(), + copiedLoad: copied.customization.load, + content: (await fileService.readFile(URI.joinPath(copied.pluginDir!, 'index.js'))).value.toString(), + }, { + inPlace: directory.toString(), inPlaceLoad: { kind: 'loaded' }, cacheExistsBeforeCopy: false, + copied: true, copiedLoad: { kind: 'loaded' }, content: 'client-served', + }); + }); + test('uses immutable host directories in place without client reads or cache entries', async () => { disposables.add(fileService.registerProvider(Schemas.file, disposables.add(new InMemoryFileSystemProvider()))); const hostManager = new AgentPluginManager(URI.file('/userData'), fileService, new NullLogService()); diff --git a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts index 37ec197f141692..e8ab41d1e84b56 100644 --- a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts +++ b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts @@ -510,6 +510,37 @@ suite('ProtocolServerHandler', () => { }); }); + test('isLocalClient requires an active Local MessagePort connection', () => { + const results = []; + for (const connectionKind of [AgentHostClientConnectionKind.Local, AgentHostClientConnectionKind.SSH, AgentHostClientConnectionKind.Unknown]) { + for (const transportKind of [AgentHostTransportKind.MessagePort, AgentHostTransportKind.WebSocket, AgentHostTransportKind.Unknown]) { + const clientId = `${connectionKind}-${transportKind}`; + const transport = connectClient(clientId, undefined, undefined, { + 'vscode.clientConnectionKind': connectionKind, + }, transportKind); + results.push(clientConnections.isLocalClient(clientId)); + transport.simulateClose(); + results.push(clientConnections.isLocalClient(clientId)); + } + } + assert.deepStrictEqual({ missing: clientConnections.isLocalClient('missing'), results }, { + missing: false, + results: [true, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false], + }); + }); + + test('isLocalClient follows the qualifying connection when a client has multiple transports', () => { + const meta = { 'vscode.clientConnectionKind': AgentHostClientConnectionKind.Local }; + connectClient('client', undefined, undefined, meta, AgentHostTransportKind.WebSocket); + const remoteOnly = clientConnections.isLocalClient('client'); + const local = connectClient('client', undefined, undefined, meta, AgentHostTransportKind.MessagePort); + const withLocal = clientConnections.isLocalClient('client'); + local.simulateClose(); + assert.deepStrictEqual({ remoteOnly, withLocal, afterLocalCloses: clientConnections.isLocalClient('client'), connected: clientConnections.isClientConnected('client') }, { + remoteOnly: false, withLocal: true, afterLocalCloses: false, connected: true, + }); + }); + test('canvas extension is local-only, publishes full snapshots, and fences source resolution', async () => { const snapshot: IAgentCanvasSnapshot = { chat: URI.parse(defaultChatUri), diff --git a/src/vs/sessions/AUTOMATIONS.md b/src/vs/sessions/AUTOMATIONS.md index 79e010f39f5759..9c0bc8cc42d387 100644 --- a/src/vs/sessions/AUTOMATIONS.md +++ b/src/vs/sessions/AUTOMATIONS.md @@ -64,8 +64,12 @@ Compatibility decoding for configuration values in existing AHP definitions is s When the host advertises `automations.customizations`, the client sends its enabled plugins for the target's harness and workspace in the AHP session template on creation and when the target changes. Ordinary edits resubmit the saved entries unchanged. The client keeps the customization scope alive until the host accepts or rejects the mutation. +The automation dialog's Advanced section lets the user choose which of those plugins to sync. When editing, it marks saved entries whose local `nonce` changed as outdated. Saving always sends the selection, so outdated entries are refreshed, and saved entries that are no longer available locally keep their copy while selected. The dialog stays open with progress until the host accepts the mutation, and shows a rejection inline. + The host copies each new or changed entry from the dispatching client into an immutable host-owned directory, and reuses the existing copy for entries whose `id`, `uri`, and `nonce` are unchanged. Any capture failure rejects the mutation. The catalogue reports the copies in `AutomationEntry.customizations`. +For a local VS Code window connected through MessagePort, `file:` plugins are parsed and used at their original host paths instead of copied; edits to their contents on disk take effect in later runs. Virtual bundles and plugins from remote clients are still copied, and garbage collection only removes host-owned copies, never these in-place paths. + A run session receives the copies as a static, host-owned active client created with the session, so providers load them through the same path as any other client plugin, without contacting the originating client. That client contributes no tools and is not re-attached when a session is restored. A selected custom agent inside a captured plugin is remapped to the copy. After each create, update, or removal, the host deletes copies that no automation references, including those of a rejected capture. Copies used by a run session stay until the host restarts, because that session keeps using them in place for follow-up turns. diff --git a/src/vs/sessions/contrib/automations/browser/automationDialog.ts b/src/vs/sessions/contrib/automations/browser/automationDialog.ts index 380ecd25f86591..6724f4d245ba75 100644 --- a/src/vs/sessions/contrib/automations/browser/automationDialog.ts +++ b/src/vs/sessions/contrib/automations/browser/automationDialog.ts @@ -7,7 +7,8 @@ import * as DOM from '../../../../base/browser/dom.js'; import { raceCancellationError, raceTimeout } from '../../../../base/common/async.js'; import { BaseActionViewItem, IBaseActionViewItemOptions } from '../../../../base/browser/ui/actionbar/actionViewItems.js'; import { renderIcon } from '../../../../base/browser/ui/iconLabel/iconLabels.js'; -import { IButton } from '../../../../base/browser/ui/button/button.js'; +import { Button, IButton, unthemedButtonStyles } from '../../../../base/browser/ui/button/button.js'; +import { Checkbox } from '../../../../base/browser/ui/toggle/toggle.js'; import { InputBox } from '../../../../base/browser/ui/inputbox/inputBox.js'; import { AnchorPosition } from '../../../../base/browser/ui/contextview/contextview.js'; import { ISelectBoxOptions, ISelectOptionItem, SelectBox } from '../../../../base/browser/ui/selectBox/selectBox.js'; @@ -18,7 +19,7 @@ import { Emitter, Event } from '../../../../base/common/event.js'; import { KeyCode } from '../../../../base/common/keyCodes.js'; import { Disposable, DisposableStore, IDisposable, MutableDisposable, toDisposable } from '../../../../base/common/lifecycle.js'; import { autorun, constObservable, derived, disposableObservableValue, IObservable, observableSignalFromEvent, observableValue } from '../../../../base/common/observable.js'; -import { isEqual } from '../../../../base/common/resources.js'; +import { getComparisonKey, isEqual } from '../../../../base/common/resources.js'; import { URI } from '../../../../base/common/uri.js'; import { ICodeEditorService } from '../../../../editor/browser/services/codeEditorService.js'; import { EditorOptions } from '../../../../editor/common/config/editorOptions.js'; @@ -33,12 +34,13 @@ import { IConfigurationService } from '../../../../platform/configuration/common import { ContextKeyExpr, IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; import { IContextViewService } from '../../../../platform/contextview/browser/contextView.js'; import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; +import { IHoverService } from '../../../../platform/hover/browser/hover.js'; import { ServiceCollection } from '../../../../platform/instantiation/common/serviceCollection.js'; import { KeybindingsRegistry, KeybindingWeight } from '../../../../platform/keybinding/common/keybindingsRegistry.js'; import { ILogService } from '../../../../platform/log/common/log.js'; import { HiddenItemStrategy, MenuWorkbenchToolBar } from '../../../../platform/actions/browser/toolbar.js'; import { IWorkspaceTrustRequestService } from '../../../../platform/workspace/common/workspaceTrust.js'; -import { defaultInputBoxStyles, defaultSelectBoxStyles } from '../../../../platform/theme/browser/defaultStyles.js'; +import { defaultCheckboxStyles, defaultInputBoxStyles, defaultSelectBoxStyles } from '../../../../platform/theme/browser/defaultStyles.js'; import { hasNativeContextMenu } from '../../../../platform/window/common/window.js'; import { IWorkspacePickerItem, WorkspacePicker } from '../../chat/browser/sessionWorkspacePicker.js'; import { BranchPicker, IBranchPickerBranch } from '../../chat/browser/branchPicker.js'; @@ -47,7 +49,7 @@ import { isMobilePickerSheetTarget } from '../../../browser/parts/mobile/mobileP import { ISession, ISessionWorkspaceBrowseAction, SESSION_WORKSPACE_GROUP_LOCAL } from '../../../services/sessions/common/session.js'; import { IGitRepository, IGitService } from '../../../../workbench/contrib/git/common/gitService.js'; import { AutomationInterval, AutomationTarget, IAutomationDescriptor } from '../../../../workbench/contrib/chat/common/automations/automation.js'; -import { IAutomationService } from '../../../../workbench/contrib/chat/common/automations/automationService.js'; +import { IAutomationCustomizationChoice, IAutomationService } from '../../../../workbench/contrib/chat/common/automations/automationService.js'; import { DAYS_OF_WEEK } from '../../../../workbench/contrib/chat/common/automations/schedule.js'; import { ChatContextKeys } from '../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; import { ChatAgentLocation } from '../../../../workbench/contrib/chat/common/constants.js'; @@ -244,10 +246,11 @@ interface IRenderFormHandle { readonly getSessionConfiguration: (token: CancellationToken) => Promise; readonly getBranch: () => string | undefined; readonly waitForAutomationSessionSync: (token: CancellationToken) => Promise; - readonly setSaving: (saving: boolean) => void; + readonly setSaving: (saving: boolean, committing?: boolean) => void; + readonly getCustomizationIds: () => readonly string[] | undefined; readonly showTargetValidationError: (message: string | undefined) => void; - readonly showSessionConfigurationError: (message: string | undefined) => void; - readonly focusSessionConfigurationError: () => void; + readonly showSaveError: (message: string | undefined) => void; + readonly focusSaveError: () => void; readonly getFocusableElements: () => readonly HTMLElement[]; readonly acceptPromptSuggestion: () => boolean; readonly cancelPromptSuggestion: () => boolean; @@ -1032,7 +1035,14 @@ export function renderForm( initialTarget: AutomationTarget | undefined, initialSessionConfiguration: IAutomationSessionConfiguration | undefined, allowedProviders: IObservable, + customizations?: { readonly service: IAutomationService; readonly hoverService: IHoverService; readonly existingId?: string }, ): IRenderFormHandle { + let reloadCustomizations: () => void = () => { }; + const validateForm = revalidate; + revalidate = () => { + reloadCustomizations(); + validateForm(); + }; const formContent = DOM.append(form, $('.automation-form-content')); const nameRow = DOM.append(formContent, $('.automation-form-row')); DOM.append(nameRow, $('span.automation-form-label', undefined, localize('automation.form.name', "Name"))); @@ -1405,11 +1415,6 @@ export function renderForm( role: 'status', 'aria-atomic': 'true', })); - const sessionConfigurationError = DOM.append(sessionConfiguration, $('span.automation-session-configuration-error', { - role: 'alert', - tabindex: '-1', - })); - DOM.hide(sessionConfigurationError); disposables.add(autorun(reader => { const hasTarget = isolationModel.isQuickChatObs.read(reader) || isolationModel.folderUriObs.read(reader) !== undefined; setAutomationControlVisible(sessionConfiguration, hasTarget); @@ -1457,16 +1462,41 @@ export function renderForm( }, DOM.getWindow(promptHost))); disposables.add(resizeObserver.observe(promptHost)); + const customizationSelection = customizations ? disposables.add(new AutomationCustomizationSelection( + formContent, customizations.service, customizations.hoverService, logService, customizations.existingId, + )) : undefined; + reloadCustomizations = () => customizationSelection?.updateTarget( + state.providerId && state.sessionTypeId && allowedProviders.get().includes(state.providerId) + ? createAutomationTarget(state, isolationModel.persistedBranch) + : undefined, + ); + disposables.add(onDidChangeSessionTarget.event(reloadCustomizations)); + disposables.add(autorun(reader => { + isolationModel.isQuickChatObs.read(reader); + isolationModel.folderUriObs.read(reader); + allowedProviders.read(reader); + reloadCustomizations(); + })); + const saveStatus = DOM.append(form, $('span.automation-form-save-status', { role: 'status', 'aria-atomic': 'true', })); + const saveIcon = DOM.append(saveStatus, renderIcon(Codicon.loading)); + saveIcon.classList.add('codicon-modifier-spin'); + saveIcon.setAttribute('aria-hidden', 'true'); + const saveStatusText = DOM.append(saveStatus, $('span')); DOM.hide(saveStatus); + const saveError = DOM.append(form, $('.automation-form-save-error', { role: 'alert', tabindex: '-1' })); + DOM.append(saveError, renderIcon(Codicon.error)).setAttribute('aria-hidden', 'true'); + const saveErrorText = DOM.append(saveError, $('span')); + DOM.hide(saveError); return { getPrompt: () => chatInput.inputEditor.getValue(), getSessionConfiguration: token => automationSessionDraftSynchronizer.getSessionConfiguration(token), getBranch: () => isolationModel.persistedBranch, + getCustomizationIds: () => customizationSelection?.getSelectedIds(), showTargetValidationError: message => { const text = message ?? ''; if (targetError.textContent === text) { @@ -1487,28 +1517,30 @@ export function renderForm( updateAutomationSessionTarget(); return automationSessionDraftSynchronizer.waitForSync(token); }, - setSaving: saving => { + setSaving: (saving, committing) => { formContent.toggleAttribute('inert', saving); formContent.setAttribute('aria-busy', String(saving)); form.classList.toggle('saving', saving); if (saving) { DOM.show(saveStatus); - saveStatus.textContent = localize('automation.form.saving', "Saving automation…"); + saveStatusText.textContent = committing && customizationSelection?.getSelectedIds()?.length + ? localize('automation.form.syncingCustomizations', "Syncing customizations…") + : localize('automation.form.saving', "Saving automation…"); } else { DOM.hide(saveStatus); - saveStatus.textContent = ''; + saveStatusText.textContent = ''; } }, - showSessionConfigurationError: message => { + showSaveError: message => { if (message) { - DOM.show(sessionConfigurationError); - sessionConfigurationError.textContent = message; + DOM.show(saveError); + saveErrorText.textContent = message; } else { - DOM.hide(sessionConfigurationError); - sessionConfigurationError.textContent = ''; + DOM.hide(saveError); + saveErrorText.textContent = ''; } }, - focusSessionConfigurationError: () => sessionConfigurationError.focus(), + focusSaveError: () => saveError.focus(), getFocusableElements: () => { // eslint-disable-next-line no-restricted-syntax -- the dialog owns this form subtree and supplies its dynamic focus order. return Array.from(form.querySelectorAll('input, select, textarea, button, a[href], [tabindex]')); @@ -1532,6 +1564,177 @@ export function renderForm( }; } +export class AutomationCustomizationSelection extends Disposable { + private readonly section: HTMLElement; + private readonly disclosure: Button; + private readonly chevron: HTMLElement; + private readonly summary: HTMLElement; + private readonly content: HTMLElement; + private readonly rows = this._register(new DisposableStore()); + private readonly request = this._register(new MutableDisposable()); + private readonly toggles = new Map(); + private choices: readonly IAutomationCustomizationChoice[] | undefined; + private targetKey: string | undefined; + private expanded = false; + + constructor( + container: HTMLElement, + private readonly service: IAutomationService, + private readonly hoverService: IHoverService, + private readonly logService: ILogService, + private readonly existingId?: string, + ) { + super(); + this._register(toDisposable(() => this.request.value?.cancel())); + this.section = DOM.append(container, $('.automation-advanced')); + this.disclosure = this._register(new Button(this.section, { ...unthemedButtonStyles, secondary: true })); + this.disclosure.element.classList.add('automation-advanced-disclosure', 'automation-form-label'); + this.chevron = DOM.append(this.disclosure.element, $('span.automation-advanced-chevron', { 'aria-hidden': 'true' })); + DOM.append(this.chevron, renderIcon(Codicon.chevronRight)); + DOM.append(this.disclosure.element, $('span', undefined, localize('automation.advanced', "Advanced"))); + this.summary = DOM.append(this.disclosure.element, $('span.automation-advanced-summary')); + this.content = DOM.append(this.section, $('.automation-customizations', { id: 'automation-customizations' })); + this.disclosure.element.setAttribute('aria-controls', this.content.id); + this.disclosure.element.setAttribute('aria-expanded', 'false'); + DOM.hide(this.section, this.content); + this._register(this.disclosure.onDidClick(() => { + this.expanded = !this.expanded; + this.disclosure.element.setAttribute('aria-expanded', String(this.expanded)); + DOM.reset(this.chevron, renderIcon(this.expanded ? Codicon.chevronDown : Codicon.chevronRight)); + this.content.hidden = !this.expanded; + this.content.style.display = this.expanded ? '' : 'none'; + this.summary.hidden = this.expanded; + })); + } + + getSelectedIds(): readonly string[] | undefined { + return this.choices?.filter(choice => this.toggles.get(choice.id) ?? choice.selected).map(choice => choice.id); + } + + updateTarget(target: AutomationTarget | undefined): void { + const key = target ? JSON.stringify({ + kind: target.kind, + providerId: target.providerId, + sessionTypeId: target.sessionTypeId, + folder: target.kind === 'workspace' ? getComparisonKey(target.folderUri) : undefined, + }) : undefined; + if (key === this.targetKey) { + return; + } + this.targetKey = key; + this.request.value?.cancel(); + this.request.clear(); + this.choices = undefined; + this.rows.clear(); + DOM.clearNode(this.content); + if (!target || !target.providerId || !target.sessionTypeId || !this.service.getCustomizationChoices) { + DOM.hide(this.section); + return; + } + DOM.show(this.section); + this.summary.textContent = localize('automation.customizations.loadingSummary', "Loading…"); + const loading = DOM.append(this.content, $('.automation-customizations-message', { role: 'status' })); + const icon = DOM.append(loading, renderIcon(Codicon.loading)); + icon.classList.add('codicon-modifier-spin'); + icon.setAttribute('aria-hidden', 'true'); + DOM.append(loading, $('span', undefined, localize('automation.customizations.loading', "Loading customizations…"))); + const request = new CancellationTokenSource(); + this.request.value = request; + void this.load(target, request); + } + + private async load(target: AutomationTarget, request: CancellationTokenSource): Promise { + try { + const choices = await this.service.getCustomizationChoices?.(target, this.existingId, request.token); + if (request.token.isCancellationRequested || this._store.isDisposed) { + return; + } + this.choices = choices; + if (choices === undefined) { + DOM.hide(this.section); + return; + } + this.renderChoices(choices); + } catch (error) { + if (!request.token.isCancellationRequested && !this._store.isDisposed) { + this.logService.error('[AutomationDialog] Failed to load customization choices.', error); + DOM.hide(this.section); + } + } + } + + private updateSummary(): void { + const outdated = this.choices?.filter(choice => choice.outdated).length ?? 0; + this.summary.textContent = outdated + ? localize('automation.customizations.summaryOutdated', "{0} of {1} customizations · {2} outdated", this.getSelectedIds()?.length ?? 0, this.choices?.length ?? 0, outdated) + : localize('automation.customizations.summary', "{0} of {1} customizations", this.getSelectedIds()?.length ?? 0, this.choices?.length ?? 0); + } + + private renderChoices(choices: readonly IAutomationCustomizationChoice[]): void { + DOM.clearNode(this.content); + DOM.append(this.content, $('span.automation-form-label', undefined, localize('automation.customizations.heading', "Customizations"))); + DOM.append(this.content, $('p.automation-customizations-description', undefined, localize('automation.customizations.description', "Choose which plugins and customizations runs of this automation can use."))); + if (!choices.length) { + DOM.append(this.content, $('p.automation-customizations-description', undefined, localize('automation.customizations.empty', "No customizations are available for this target."))); + } + for (const [index, choice] of choices.entries()) { + const row = DOM.append(this.content, $('.automation-customization-row')); + const checkbox = this.rows.add(new Checkbox(choice.label, this.toggles.get(choice.id) ?? choice.selected, defaultCheckboxStyles)); + row.appendChild(checkbox.domNode); + const text = DOM.append(row, $('.automation-customization-text')); + const label = DOM.append(text, $('span.automation-customization-label', { id: `automation-customization-${index}` }, choice.label)); + checkbox.domNode.setAttribute('aria-labelledby', label.id); + this.rows.add(this.hoverService.setupDelayedHover(label, { content: choice.label })); + if (choice.description) { + const description = DOM.append(text, $('span.automation-customization-description', { id: `automation-customization-description-${index}` }, choice.description)); + checkbox.domNode.setAttribute('aria-describedby', description.id); + this.rows.add(this.hoverService.setupDelayedHover(description, { content: choice.description })); + } + if (choice.outdated) { + const outdated = DOM.append(row, $('span.automation-customization-outdated', { id: `automation-customization-outdated-${index}` }, localize('automation.customizations.outdated', "Outdated"))); + checkbox.domNode.setAttribute('aria-describedby', choice.description + ? `automation-customization-description-${index} ${outdated.id}` + : outdated.id); + } + this.rows.add(checkbox.onChange(() => { + this.toggles.set(choice.id, checkbox.checked); + this.updateSummary(); + })); + this.rows.add(DOM.addDisposableListener(text, DOM.EventType.CLICK, () => { + checkbox.checked = !checkbox.checked; + this.toggles.set(choice.id, checkbox.checked); + this.updateSummary(); + checkbox.focus(); + })); + } + if (choices.some(choice => choice.outdated)) { + const hint = DOM.append(this.content, $('.automation-customizations-message')); + DOM.append(hint, renderIcon(Codicon.info)).setAttribute('aria-hidden', 'true'); + DOM.append(hint, $('span', undefined, localize('automation.customizations.outdatedHint', "Outdated customizations will be updated when the automation is saved."))); + } + this.updateSummary(); + } +} + +export function createAutomationTarget(state: IFormState, branch: string | undefined): AutomationTarget | undefined { + if (state.isQuickChat) { + return state.providerId && state.sessionTypeId + ? { kind: 'quickChat', providerId: state.providerId, sessionTypeId: state.sessionTypeId } + : undefined; + } + if (!state.folderUri) { + return undefined; + } + const isolation = state.isolationMode === 'worktree' + ? (branch ? { kind: 'worktree' as const, branch } : undefined) + : state.isolationMode === 'workspace' + ? { kind: 'folder' as const } + : { kind: 'default' as const }; + return isolation + ? { kind: 'workspace', folderUri: state.folderUri, providerId: state.providerId, sessionTypeId: state.sessionTypeId, isolation } + : undefined; +} + interface ITimeOption { readonly hour: number; readonly minute: number; diff --git a/src/vs/sessions/contrib/automations/browser/automationDialogService.ts b/src/vs/sessions/contrib/automations/browser/automationDialogService.ts index 6fe8809bf4f35f..fd7bba81f8312f 100644 --- a/src/vs/sessions/contrib/automations/browser/automationDialogService.ts +++ b/src/vs/sessions/contrib/automations/browser/automationDialogService.ts @@ -7,9 +7,10 @@ import './media/automationDialog.css'; import * as DOM from '../../../../base/browser/dom.js'; import { ButtonBar, IButton } from '../../../../base/browser/ui/button/button.js'; import { Dialog } from '../../../../base/browser/ui/dialog/dialog.js'; +import { ProgressBar } from '../../../../base/browser/ui/progressbar/progressbar.js'; import { DeferredPromise } from '../../../../base/common/async.js'; import { CancellationToken, CancellationTokenSource } from '../../../../base/common/cancellation.js'; -import { isCancellationError } from '../../../../base/common/errors.js'; +import { getErrorMessage, isCancellationError } from '../../../../base/common/errors.js'; import { DisposableStore, MutableDisposable, toDisposable } from '../../../../base/common/lifecycle.js'; import { isWindows } from '../../../../base/common/platform.js'; import { localize } from '../../../../nls.js'; @@ -17,20 +18,21 @@ import { IConfigurationService } from '../../../../platform/configuration/common import { IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; import { IContextViewService } from '../../../../platform/contextview/browser/contextView.js'; import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; +import { IHoverService } from '../../../../platform/hover/browser/hover.js'; import { IKeybindingService } from '../../../../platform/keybinding/common/keybinding.js'; import { ILogService } from '../../../../platform/log/common/log.js'; import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js'; import { IWorkspaceTrustRequestService } from '../../../../platform/workspace/common/workspaceTrust.js'; -import { defaultButtonStyles, defaultDialogStyles } from '../../../../platform/theme/browser/defaultStyles.js'; +import { defaultButtonStyles, defaultDialogStyles, defaultProgressBarStyles } from '../../../../platform/theme/browser/defaultStyles.js'; import { createWorkbenchDialogOptions } from '../../../../workbench/browser/parts/dialogs/dialog.js'; -import { AutomationTarget, IAutomationSchedule } from '../../../../workbench/contrib/chat/common/automations/automation.js'; +import { IAutomationSchedule } from '../../../../workbench/contrib/chat/common/automations/automation.js'; import { IAutomationDialogResult, IAutomationDialogService, IShowAutomationDialogOptions } from '../../../../workbench/contrib/chat/common/automations/automationDialogService.js'; import { IAutomationService, ICreateAutomationOptions, IUpdateAutomationOptions } from '../../../../workbench/contrib/chat/common/automations/automationService.js'; import { IHostService } from '../../../../workbench/services/host/browser/host.js'; import { IWorkbenchLayoutService } from '../../../../workbench/services/layout/browser/layoutService.js'; import { ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js'; import { IAutomationSessionConfiguration } from '../../../services/sessions/common/sessionsProvider.js'; -import { AutomationSessionConfigurationCapture, getAutomationDialogProviders, IFormState, IValidationState, isAutomationDialogPopupTarget, registerAutomationDialogKeyboardNavigation, renderForm, shouldPassThroughAutomationDialogCommand, updateSaveButtonState } from './automationDialog.js'; +import { AutomationSessionConfigurationCapture, createAutomationTarget, getAutomationDialogProviders, IFormState, IValidationState, isAutomationDialogPopupTarget, registerAutomationDialogKeyboardNavigation, renderForm, shouldPassThroughAutomationDialogCommand, updateSaveButtonState } from './automationDialog.js'; import { AutomationDialogTelemetry } from './automationTelemetry.js'; const $ = DOM.$; @@ -82,6 +84,7 @@ export class AutomationDialogService implements IAutomationDialogService { @IWorkspaceTrustRequestService private readonly workspaceTrustRequestService: IWorkspaceTrustRequestService, @IAutomationService private readonly automationService: IAutomationService, @ITelemetryService private readonly telemetryService: ITelemetryService, + @IHoverService private readonly hoverService: IHoverService, ) { } async showAutomationDialog(options: IShowAutomationDialogOptions): Promise { @@ -127,9 +130,14 @@ export class AutomationDialogService implements IAutomationDialogService { let getSessionConfiguration: (token: CancellationToken) => Promise = async () => ({ kind: 'preserved', configuration: initialSessionConfiguration }); let getBranch: () => string | undefined = () => initialWorkspaceTarget?.isolation.kind === 'worktree' ? initialWorkspaceTarget.isolation.branch : undefined; let waitForAutomationSessionSync: (token: CancellationToken) => Promise = async () => { }; - let setSaving: (saving: boolean) => void = () => { }; - let showSessionConfigurationError: (message: string | undefined) => void = () => { }; - let focusSessionConfigurationError: () => void = () => { }; + let setSaving: (saving: boolean, committing?: boolean) => void = () => { }; + let getCustomizationIds: () => readonly string[] | undefined = () => undefined; + let progressBar: ProgressBar | undefined; + let dialogElement: HTMLElement | undefined; + let closeToolbar: HTMLElement | undefined; + let commitInProgress = false; + let showSaveError: (message: string | undefined) => void = () => { }; + let focusSaveError: () => void = () => { }; let getFocusableElements: () => readonly HTMLElement[] = () => []; let focusFirst: () => void = () => { }; let saveInProgress = false; @@ -156,6 +164,7 @@ export class AutomationDialogService implements IAutomationDialogService { const sessionConfiguration = sessionConfigurationCapture.configuration; const sessionTemplate = sessionConfiguration?.sessionTemplate; const target = createAutomationTarget(state, getBranch()); + const customizationIds = getCustomizationIds(); if (!target) { return undefined; } @@ -169,6 +178,7 @@ export class AutomationDialogService implements IAutomationDialogService { sessionTemplate: sessionTemplate ?? null, } : {}), enabled: state.enabled, + ...(customizationIds !== undefined ? { customizationIds } : {}), }; return { kind: 'update', id: existing.id, value: patch }; } @@ -185,12 +195,13 @@ export class AutomationDialogService implements IAutomationDialogService { ...(sessionConfiguration.permissionLevel !== undefined ? { permissionLevel: sessionConfiguration.permissionLevel } : {}), } : {}), enabled: state.enabled, + ...(customizationIds !== undefined ? { customizationIds } : {}), }; return { kind: 'create', value: create }; }; const closeDialog = (result: IAutomationDialogResult | undefined) => { - if (completion.isSettled) { + if (completion.isSettled || (commitInProgress && result === undefined)) { return; } dialogTelemetry.complete(result !== undefined); @@ -214,8 +225,9 @@ export class AutomationDialogService implements IAutomationDialogService { } saveInProgress = true; - showSessionConfigurationError(undefined); + showSaveError(undefined); setSaving(true); + progressBar?.infinite().show(); if (saveButton) { saveButton.enabled = false; saveButton.label = savingButtonLabel; @@ -230,7 +242,7 @@ export class AutomationDialogService implements IAutomationDialogService { const sessionConfigurationCapture = await getSessionConfiguration(cancellation.token); if (sessionConfigurationCapture.kind === 'failed') { dialogTelemetry.captureFailed(); - showSessionConfigurationError(captureErrorMessage); + showSaveError(captureErrorMessage); shouldFocusError = true; return; } @@ -240,14 +252,28 @@ export class AutomationDialogService implements IAutomationDialogService { } const result = buildResult(sessionConfigurationCapture); if (result) { + if (options.commit) { + commitInProgress = true; + setSaving(true, true); + if (cancelButton) { + cancelButton.enabled = false; + } + dialogElement?.classList.add('committing'); + closeToolbar?.setAttribute('inert', ''); + dialogElement?.focus(); + await options.commit(result); + commitInProgress = false; + } shouldClose = true; closeDialog(result); } } catch (error) { - if (!isCancellationError(error) && !cancellation.token.isCancellationRequested) { - this.logService.error('[AutomationDialog] Failed to save the automation session configuration.', error); - dialogTelemetry.captureFailed(); - showSessionConfigurationError(captureErrorMessage); + if (commitInProgress || (!isCancellationError(error) && !cancellation.token.isCancellationRequested)) { + this.logService.error('[AutomationDialog] Failed to save automation.', error); + if (!commitInProgress) { + dialogTelemetry.captureFailed(); + } + showSaveError(commitInProgress ? getErrorMessage(error) : captureErrorMessage); shouldFocusError = true; } } finally { @@ -255,14 +281,21 @@ export class AutomationDialogService implements IAutomationDialogService { saveCancellation.clear(); } saveInProgress = false; + commitInProgress = false; if (!shouldClose && !completion.isSettled) { + dialogElement?.classList.remove('committing'); + closeToolbar?.removeAttribute('inert'); + if (cancelButton) { + cancelButton.enabled = true; + } + progressBar?.stop().hide(); setSaving(false); if (saveButton) { saveButton.label = saveButtonLabel; } revalidate(); if (shouldFocusError) { - focusSessionConfigurationError(); + focusSaveError(); } } } @@ -304,6 +337,10 @@ export class AutomationDialogService implements IAutomationDialogService { }, renderBody: container => { container.classList.add('automation-dialog-body'); + dialogElement = container.closest('.monaco-dialog-box') ?? undefined; + const progressHost = DOM.append(container, $('.automation-dialog-progress')); + progressBar = disposables.add(new ProgressBar(progressHost, defaultProgressBarStyles)); + progressBar.hide(); const titlebar = DOM.append(container, $('.automation-titlebar')); titlebar.setAttribute('aria-hidden', 'true'); @@ -316,14 +353,15 @@ export class AutomationDialogService implements IAutomationDialogService { const formPane = DOM.append(container, $('.automation-form-pane')); const form = DOM.append(formPane, $('.automation-form')); - const handle = renderForm(form, state, disposables, validation, () => revalidate(), this.instantiationService, this.contextKeyService, this.contextViewService, this.configurationService, this.layoutService, this.logService, this.sessionsManagementService, this.workspaceTrustRequestService, initial?.prompt ?? '', initialTarget, initialSessionConfiguration, allowedProviders); + const handle = renderForm(form, state, disposables, validation, () => revalidate(), this.instantiationService, this.contextKeyService, this.contextViewService, this.configurationService, this.layoutService, this.logService, this.sessionsManagementService, this.workspaceTrustRequestService, initial?.prompt ?? '', initialTarget, initialSessionConfiguration, allowedProviders, { service: this.automationService, hoverService: this.hoverService, existingId: existing?.id }); getPrompt = handle.getPrompt; getSessionConfiguration = handle.getSessionConfiguration; getBranch = handle.getBranch; waitForAutomationSessionSync = handle.waitForAutomationSessionSync; setSaving = handle.setSaving; - showSessionConfigurationError = handle.showSessionConfigurationError; - focusSessionConfigurationError = handle.focusSessionConfigurationError; + getCustomizationIds = handle.getCustomizationIds; + showSaveError = handle.showSaveError; + focusSaveError = handle.focusSaveError; getFocusableElements = handle.getFocusableElements; const keyboardNavigation = disposables.add(registerAutomationDialogKeyboardNavigation( DOM.getWindow(container), @@ -333,10 +371,24 @@ export class AutomationDialogService implements IAutomationDialogService { ...(cancelButton ? [cancelButton.element] : []), ], isAutomationDialogPopupTarget, - handle.acceptPromptSuggestion, - handle.cancelPromptSuggestion, + () => !saveInProgress && handle.acceptPromptSuggestion(), + () => !saveInProgress && handle.cancelPromptSuggestion(), )); focusFirst = keyboardNavigation.focusFirst; + for (const type of [DOM.EventType.KEY_DOWN, DOM.EventType.KEY_UP]) { + disposables.add(DOM.addDisposableListener(DOM.getWindow(container), type, (event: KeyboardEvent) => { + if (commitInProgress && event.key === 'Escape') { + DOM.EventHelper.stop(event, true); + event.stopImmediatePropagation(); + } + }, true)); + } + disposables.add(DOM.addDisposableListener(activeContainer, DOM.EventType.CLICK, (event: MouseEvent) => { + if (commitInProgress && DOM.isHTMLElement(event.target) && closeToolbar?.contains(event.target)) { + DOM.EventHelper.stop(event, true); + event.stopImmediatePropagation(); + } + }, true)); revalidate = () => { const providerAvailable = state.providerId !== undefined && allowedProviders.get().includes(state.providerId); updateSaveButtonState(saveButton, state, validation, form, getPrompt, getBranch, this.sessionsManagementService, providerAvailable, existing?.target.providerId, isEdit); @@ -356,6 +408,8 @@ export class AutomationDialogService implements IAutomationDialogService { try { void dialog.show().then(() => closeDialog(undefined)); + // eslint-disable-next-line no-restricted-syntax -- Dialog owns its close toolbar and exposes no enablement API. + closeToolbar = dialogElement?.querySelector('.dialog-toolbar') ?? undefined; focusFirst(); return await completion.p; } finally { @@ -375,28 +429,3 @@ function deriveAutomationName(prompt: string): string { const wordBoundary = text.lastIndexOf(' ', prefix.length); return wordBoundary > 0 ? text.slice(0, wordBoundary) : prefix; } - -function createAutomationTarget(state: IFormState, branch: string | undefined): AutomationTarget | undefined { - if (state.isQuickChat) { - return state.providerId && state.sessionTypeId - ? { kind: 'quickChat', providerId: state.providerId, sessionTypeId: state.sessionTypeId } - : undefined; - } - if (!state.folderUri) { - return undefined; - } - const isolation = state.isolationMode === 'worktree' - ? (branch ? { kind: 'worktree' as const, branch } : undefined) - : state.isolationMode === 'workspace' - ? { kind: 'folder' as const } - : { kind: 'default' as const }; - return isolation - ? { - kind: 'workspace', - folderUri: state.folderUri, - providerId: state.providerId, - sessionTypeId: state.sessionTypeId, - isolation, - } - : undefined; -} diff --git a/src/vs/sessions/contrib/automations/browser/media/automationDialog.css b/src/vs/sessions/contrib/automations/browser/media/automationDialog.css index b9803e305c17b8..2010e8017cb2d2 100644 --- a/src/vs/sessions/contrib/automations/browser/media/automationDialog.css +++ b/src/vs/sessions/contrib/automations/browser/media/automationDialog.css @@ -359,18 +359,153 @@ font-size: var(--vscode-fontSize-body2); } -.automation-session-configuration-error { - flex-basis: 100%; - color: var(--vscode-inputValidation-errorForeground); +.automation-target-error { + color: var(--vscode-errorForeground); font-size: var(--vscode-fontSize-body2); } -.automation-target-error { +.automation-form-save-status { + display: flex; + align-items: center; + gap: var(--vscode-spacing-size80); + color: var(--vscode-descriptionForeground); + font-size: var(--vscode-fontSize-body2); +} + +.automation-dialog-progress { + position: absolute; + top: 0; + left: 0; + right: 0; + z-index: 3; +} + +.automation-dialog.committing .dialog-toolbar { + opacity: 0.5; + pointer-events: none; +} + +.automation-form-save-error { + display: flex; + align-items: flex-start; + gap: var(--vscode-spacing-size80); + padding: var(--vscode-spacing-size80); + border: var(--vscode-strokeThickness) solid var(--vscode-inputValidation-errorBorder); + border-radius: var(--vscode-cornerRadius-small); color: var(--vscode-errorForeground); + background-color: var(--vscode-inputValidation-errorBackground); + font-size: var(--vscode-fontSize-body1); +} + +.automation-form-save-error:focus { + outline: var(--vscode-strokeThickness) solid var(--vscode-focusBorder); + outline-offset: calc(-1 * var(--vscode-strokeThickness)); +} + +.automation-form-save-error > .codicon, +.automation-customizations-message > .codicon { + flex-shrink: 0; +} + +.automation-advanced { + display: flex; + flex-direction: column; + gap: var(--vscode-spacing-size80); +} + +.automation-advanced > .automation-advanced-disclosure { + display: flex; + align-items: center; + gap: var(--vscode-spacing-size40); + /* Pad the hover area while keeping the chevron aligned with the other form labels. */ + margin: 0 calc(-1 * var(--vscode-spacing-size60)); + padding: var(--vscode-spacing-size40) var(--vscode-spacing-size60); + border: var(--vscode-strokeThickness) solid transparent; + border-radius: var(--vscode-cornerRadius-small); + background-color: transparent; + color: var(--vscode-foreground); + text-decoration: none; + cursor: pointer; +} + +.automation-advanced > .automation-advanced-disclosure:hover { + background-color: var(--vscode-toolbar-hoverBackground); +} + +.automation-advanced > .automation-advanced-disclosure:focus-visible { + outline: var(--vscode-strokeThickness) solid var(--vscode-focusBorder); + outline-offset: calc(-1 * var(--vscode-strokeThickness)); +} + +.automation-advanced-chevron { + display: inline-flex; + flex-shrink: 0; +} + +.automation-advanced-summary { + margin-left: auto; + padding-left: var(--vscode-spacing-size80); + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis; + color: var(--vscode-descriptionForeground); font-size: var(--vscode-fontSize-body2); + font-weight: var(--vscode-fontWeight-regular); } -.automation-form-save-status { +.automation-customizations { + display: flex; + flex-direction: column; + gap: var(--vscode-spacing-size80); +} + +.automation-customizations-description { + margin: 0; + color: var(--vscode-descriptionForeground); + font-size: var(--vscode-fontSize-body2); +} + +.automation-customization-row { + display: flex; + align-items: center; + gap: var(--vscode-spacing-size80); +} + +.automation-customization-text { + display: flex; + flex: 1; + min-width: 0; + flex-direction: column; + gap: var(--vscode-spacing-size20); + cursor: pointer; +} + +.automation-customization-label, +.automation-customization-description { + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis; +} + +.automation-customization-description { + color: var(--vscode-descriptionForeground); + font-size: var(--vscode-fontSize-body2); +} + +.automation-customization-outdated { + flex-shrink: 0; + padding: var(--vscode-spacing-size20) var(--vscode-spacing-size60); + border: var(--vscode-strokeThickness) solid var(--vscode-contrastBorder, transparent); + border-radius: var(--vscode-cornerRadius-circle); + background-color: var(--vscode-badge-background); + color: var(--vscode-badge-foreground); + font-size: var(--vscode-fontSize-label2); +} + +.automation-customizations-message { + display: flex; + align-items: flex-start; + gap: var(--vscode-spacing-size80); color: var(--vscode-descriptionForeground); font-size: var(--vscode-fontSize-body2); } diff --git a/src/vs/sessions/contrib/automations/browser/providerAutomationService.ts b/src/vs/sessions/contrib/automations/browser/providerAutomationService.ts index 493a336ef2e256..c7270cfa48e248 100644 --- a/src/vs/sessions/contrib/automations/browser/providerAutomationService.ts +++ b/src/vs/sessions/contrib/automations/browser/providerAutomationService.ts @@ -7,8 +7,8 @@ import { Disposable } from '../../../../base/common/lifecycle.js'; import { CancellationToken } from '../../../../base/common/cancellation.js'; import { derived, IObservable, observableSignalFromEvent } from '../../../../base/common/observable.js'; import { localize } from '../../../../nls.js'; -import { IAutomationDescriptor, IAutomationRun } from '../../../../workbench/contrib/chat/common/automations/automation.js'; -import { AutomationCatalogueState, AutomationMutationGuard, AutomationUnavailableError, assertAutomationTargetAuthority, combineAutomationCatalogueStates, IAutomationProviderDescriptor, IAutomationRunRequestResult, IAutomationService, ICreateAutomationOptions, IGuardedAutomationUpdateResult, serializeAutomationEditableState, IUpdateAutomationOptions } from '../../../../workbench/contrib/chat/common/automations/automationService.js'; +import { AutomationTarget, IAutomationDescriptor, IAutomationRun } from '../../../../workbench/contrib/chat/common/automations/automation.js'; +import { AutomationCatalogueState, AutomationMutationGuard, AutomationUnavailableError, assertAutomationTargetAuthority, combineAutomationCatalogueStates, IAutomationCustomizationChoice, IAutomationProviderDescriptor, IAutomationRunRequestResult, IAutomationService, ICreateAutomationOptions, IGuardedAutomationUpdateResult, serializeAutomationEditableState, IUpdateAutomationOptions } from '../../../../workbench/contrib/chat/common/automations/automationService.js'; import { ISessionsProvidersService } from '../../../services/sessions/browser/sessionsProvidersService.js'; import { ISessionsProviderAutomations } from '../../../services/sessions/common/sessionsProvider.js'; @@ -83,6 +83,11 @@ export class ProviderAutomationService extends Disposable implements IAutomation return this.findAutomationStore(id)?.getAutomation(id); } + async getCustomizationChoices(target: AutomationTarget, existingId: string | undefined, token: CancellationToken): Promise { + const store = target.providerId ? this.sessionsProvidersService.getProvider(target.providerId)?.automations : undefined; + return store?.getCustomizationChoices?.(target, existingId, token); + } + runsFor(automationId: string): IObservable { let result = this.runsForCache.get(automationId); if (!result) { diff --git a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts index c19727fb1518db..d4a67d6884e6a0 100644 --- a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts +++ b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts @@ -14,7 +14,7 @@ import { StandardMouseEvent } from '../../../../../base/browser/mouseEvent.js'; import { Codicon } from '../../../../../base/common/codicons.js'; import { Action, IAction } from '../../../../../base/common/actions.js'; import { Emitter, Event } from '../../../../../base/common/event.js'; -import { getErrorMessage } from '../../../../../base/common/errors.js'; +import { CancellationError, getErrorMessage } from '../../../../../base/common/errors.js'; import { DisposableStore, toDisposable } from '../../../../../base/common/lifecycle.js'; import { autorun, constObservable, observableValue } from '../../../../../base/common/observable.js'; import { isEqual } from '../../../../../base/common/resources.js'; @@ -42,13 +42,14 @@ import { ResultKind } from '../../../../../platform/keybinding/common/keybinding import { KeybindingsRegistry } from '../../../../../platform/keybinding/common/keybindingsRegistry.js'; import { ILayoutService } from '../../../../../platform/layout/browser/layoutService.js'; import { ILogService, NullLogService } from '../../../../../platform/log/common/log.js'; +import { IHoverService } from '../../../../../platform/hover/browser/hover.js'; import { defaultButtonStyles, defaultCheckboxStyles, defaultDialogStyles, defaultInputBoxStyles, defaultSelectBoxStyles } from '../../../../../platform/theme/browser/defaultStyles.js'; import { IWorkspaceTrustRequestService, ResourceTrustRequestOptions } from '../../../../../platform/workspace/common/workspaceTrust.js'; import { createWorkbenchDialogOptions } from '../../../../../workbench/browser/parts/dialogs/dialog.js'; import { ChatContextKeys } from '../../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; import { ChatInputPart } from '../../../../../workbench/contrib/chat/browser/widget/input/chatInputPart.js'; import { IAutomationDescriptor, IAutomationSessionTemplate } from '../../../../../workbench/contrib/chat/common/automations/automation.js'; -import { AutomationCatalogueState, IAutomationService } from '../../../../../workbench/contrib/chat/common/automations/automationService.js'; +import { AutomationCatalogueState, IAutomationCustomizationChoice, IAutomationService } from '../../../../../workbench/contrib/chat/common/automations/automationService.js'; import { IShowAutomationDialogOptions } from '../../../../../workbench/contrib/chat/common/automations/automationDialogService.js'; import { GitRefType, IGitRepository, IGitService } from '../../../../../workbench/contrib/git/common/gitService.js'; import { IHostService } from '../../../../../workbench/services/host/browser/host.js'; @@ -62,7 +63,7 @@ import { IAutomationSessionConfiguration } from '../../../../services/sessions/c import { ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; import { AutomationDialogService } from '../../browser/automationDialogService.js'; -import { AutomationIsolationGroupActionViewItem, AutomationSessionDraftSynchronizer, canSelectAutomationWorkspace, getAutomationDialogProviders, IFormState, IValidationState, isAutomationDialogPopupTarget, MobileAutomationsWorkspacePicker, registerAutomationDialogKeyboardNavigation, renderForm, shouldPassThroughAutomationDialogCommand, updateSaveButtonState } from '../../browser/automationDialog.js'; +import { AutomationCustomizationSelection, AutomationIsolationGroupActionViewItem, AutomationSessionDraftSynchronizer, canSelectAutomationWorkspace, getAutomationDialogProviders, IFormState, IValidationState, isAutomationDialogPopupTarget, MobileAutomationsWorkspacePicker, registerAutomationDialogKeyboardNavigation, renderForm, shouldPassThroughAutomationDialogCommand, updateSaveButtonState } from '../../browser/automationDialog.js'; import { AutomationInputCompletions } from '../../browser/automationInputCompletions.js'; import { AutomationIsolationModel } from '../../common/isolationGroupModel.js'; @@ -71,7 +72,7 @@ const FOLDER = URI.file('/workspace'); suite('Automation dialog creation', () => { const disposables = ensureNoDisposablesAreLeakedInTestSuite(); - function openDialog(options: IShowAutomationDialogOptions = {}) { + function openDialog(options: IShowAutomationDialogOptions = {}, getCustomizationChoices?: IAutomationService['getCustomizationChoices']) { const configurationService = new TestConfigurationService(); const contextKeyService = disposables.add(new ContextKeyService(configurationService)); const instantiationService = workbenchInstantiationService({ @@ -110,6 +111,7 @@ suite('Automation dialog creation', () => { automations: constObservable(options.existing ? [options.existing] : []), catalogueState: constObservable('ready'), canUpdateAutomation: () => true, + getCustomizationChoices, })); ChatContextKeys.enabled.bindTo(contextKeyService).set(true); let targetModel: AutomationIsolationModel | undefined; @@ -152,8 +154,9 @@ suite('Automation dialog creation', () => { disposables.add(toDisposable(() => cancelButton.click())); const nameInput = container.querySelector('.automation-form-input-host input')!; return { - result, saveButton, nameInput, + result, saveButton, cancelButton, nameInput, container, getTarget: () => ({ quickChat: targetModel?.isQuickChat, workspace: selectedWorkspace }), + setWorkspace: (folder: URI) => targetModel?.setQuickChat(false, folder), setPrompt: (prompt: string) => { promptInput.value = prompt; promptChanged.fire(); @@ -165,6 +168,186 @@ suite('Automation dialog creation', () => { }; } + test('keeps the dialog open during commit and ignores cancel, close, and Escape', async () => { + const started = new DeferredPromise(); + const committed = new DeferredPromise(); + const dialog = openDialog({ + commit: async () => { + void started.complete(); + await committed.p; + } + }); + dialog.setPrompt('Review changes'); + dialog.saveButton.click(); + await started.p; + dialog.cancelButton.click(); + dialog.container.querySelector('.dialog-toolbar .action-label')?.click(); + DOM.getWindow(dialog.container).dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); + DOM.getWindow(dialog.container).dispatchEvent(new KeyboardEvent('keyup', { key: 'Escape' })); + assert.deepStrictEqual({ + open: !!dialog.container.querySelector('.automation-dialog'), + save: dialog.saveButton.textContent, + cancelDisabled: dialog.cancelButton.getAttribute('aria-disabled'), + inert: dialog.container.querySelector('.automation-form-content')?.hasAttribute('inert'), + status: dialog.container.querySelector('.automation-form-save-status')?.textContent, + }, { open: true, save: 'Saving…', cancelDisabled: 'true', inert: true, status: 'Saving automation…' }); + void committed.complete(); + assert.deepStrictEqual((await dialog.result)?.kind, 'create'); + }); + + test('shows commit errors inline, preserves input, and supports retry', async () => { + let attempts = 0; + const dialog = openDialog({ + commit: async () => { + if (++attempts === 1) { + throw new Error('Unable to sync plugin'); + } + } + }); + dialog.setPrompt('Review changes'); + dialog.setName('My automation'); + dialog.saveButton.click(); + await timeout(0); + const error = dialog.container.querySelector('.automation-form-save-error')!; + assert.deepStrictEqual({ + text: error.textContent, + role: error.getAttribute('role'), + focused: DOM.getWindow(error).document.activeElement === error, + name: dialog.nameInput.value, + save: dialog.saveButton.textContent, + cancelDisabled: dialog.cancelButton.getAttribute('aria-disabled'), + inert: dialog.container.querySelector('.automation-form-content')?.hasAttribute('inert'), + }, { text: 'Unable to sync plugin', role: 'alert', focused: true, name: 'My automation', save: 'Create', cancelDisabled: 'false', inert: false }); + dialog.saveButton.click(); + assert.deepStrictEqual(error.style.display, 'none'); + assert.deepStrictEqual({ kind: (await dialog.result)?.kind, attempts }, { kind: 'create', attempts: 2 }); + }); + + test('reloads choices from the form target controls and preserves toggles', async () => { + const targets: string[] = []; + const dialog = openDialog({}, async target => { + targets.push(target.kind === 'workspace' ? target.folderUri.toString() : target.kind); + return [ + { id: 'a', label: 'A', selected: true, outdated: false }, + { id: 'b', label: 'B', selected: false, outdated: false }, + ]; + }); + await timeout(0); + dialog.container.querySelector('.automation-advanced-disclosure')!.click(); + dialog.container.querySelectorAll('.automation-customization-row .monaco-checkbox')[1].click(); + dialog.setWorkspace(FOLDER); + await timeout(0); + dialog.setWorkspace(URI.file('/other')); + await timeout(0); + dialog.setPrompt('Review changes'); + dialog.saveButton.click(); + assert.deepStrictEqual({ + targets, + selected: (await dialog.result)?.value.customizationIds, + }, { targets: ['quickChat', FOLDER.toString(), URI.file('/other').toString()], selected: ['a', 'b'] }); + }); + + test('surfaces commit cancellation rejections instead of silently abandoning the save', async () => { + const error = new CancellationError(); + const dialog = openDialog({ commit: async () => { throw error; } }); + dialog.setPrompt('Review changes'); + dialog.saveButton.click(); + await timeout(0); + const message = dialog.container.querySelector('.automation-form-save-error')?.textContent; + dialog.cancelButton.click(); + assert.deepStrictEqual({ message, result: await dialog.result }, { message: getErrorMessage(error), result: undefined }); + }); + + test('refreshes selected outdated choices on edit without expanding Advanced', async () => { + const existing: IAutomationDescriptor = { + id: 'existing', name: 'Review', prompt: 'Review changes', + target: { kind: 'quickChat', providerId: 'host', sessionTypeId: 'copilotcli' }, + schedule: { interval: 'manual', scheduleHour: 9, scheduleMinute: 0, scheduleDay: 1 }, enabled: true, createdAt: '', updatedAt: '', + }; + let existingId: string | undefined; + const dialog = openDialog({ existing }, async (_target, id) => { + existingId = id; + return [{ id: 'outdated', label: 'Outdated plugin', selected: true, outdated: true }]; + }); + await timeout(0); + const expanded = dialog.container.querySelector('.automation-advanced-disclosure')?.getAttribute('aria-expanded'); + dialog.saveButton.click(); + const result = await dialog.result; + assert.deepStrictEqual({ expanded, existingId, kind: result?.kind, selected: result?.value.customizationIds }, { + expanded: 'false', existingId: 'existing', kind: 'update', selected: ['outdated'], + }); + }); + + test('passes selected customization ids in choice order without requiring expansion', async () => { + const choices: IAutomationCustomizationChoice[] = [ + { id: 'second', label: 'Second', selected: true, outdated: true }, + { id: 'first', label: 'First', selected: false, outdated: false }, + { id: 'third', label: 'Third', selected: true, outdated: false }, + ]; + const started = new DeferredPromise(); + const committed = new DeferredPromise(); + const dialog = openDialog({ + commit: async () => { + void started.complete(); + await committed.p; + } + }, async () => choices); + await timeout(0); + const disclosure = dialog.container.querySelector('.automation-advanced-disclosure')!; + disclosure.click(); + const checkboxes = dialog.container.querySelectorAll('.automation-customization-row .monaco-checkbox'); + checkboxes[1].click(); + disclosure.click(); + dialog.setPrompt('Review changes'); + dialog.saveButton.click(); + await started.p; + assert.deepStrictEqual({ + expanded: disclosure.getAttribute('aria-expanded'), + status: dialog.container.querySelector('.automation-form-save-status')?.textContent, + }, { expanded: 'false', status: 'Syncing customizations…' }); + void committed.complete(); + assert.deepStrictEqual((await dialog.result)?.value.customizationIds, ['second', 'first', 'third']); + }); + + test('hides Advanced when customizations are unsupported', async () => { + for (const loader of [undefined, async () => undefined]) { + const dialog = openDialog({}, loader); + await timeout(0); + const display = dialog.container.querySelector('.automation-advanced')?.style.display; + dialog.setPrompt('Review changes'); + dialog.saveButton.click(); + assert.deepStrictEqual({ display, selected: (await dialog.result)?.value.customizationIds }, { display: 'none', selected: undefined }); + } + }); + + test('shows loading then an empty customization list', async () => { + const choices = new DeferredPromise(); + const dialog = openDialog({}, async () => choices.p); + const disclosure = dialog.container.querySelector('.automation-advanced-disclosure')!; + disclosure.click(); + const loading = { + summary: dialog.container.querySelector('.automation-advanced-summary')?.textContent, + message: dialog.container.querySelector('.automation-customizations-message')?.textContent, + expanded: disclosure.getAttribute('aria-expanded'), + controls: disclosure.getAttribute('aria-controls'), + spinners: dialog.container.querySelectorAll('.automation-customizations .codicon-loading.codicon-modifier-spin').length, + chevrons: disclosure.querySelectorAll('.codicon-chevron-down').length, + }; + void choices.complete([]); + await timeout(0); + dialog.setPrompt('Review changes'); + dialog.saveButton.click(); + assert.deepStrictEqual({ + loading, + empty: dialog.container.querySelector('.automation-customizations')?.textContent?.includes('No customizations are available for this target.'), + selected: (await dialog.result)?.value.customizationIds, + }, { + loading: { summary: 'Loading…', message: 'Loading customizations…', expanded: 'true', controls: 'automation-customizations', spinners: 1, chevrons: 1 }, + empty: true, + selected: [], + }); + }); + for (const { label, prompt, name } of [ { label: 'short prompt', prompt: ' Review changes ', name: 'Review changes' }, { label: 'multiline whitespace', prompt: '\n Review\t the changes\n today ', name: 'Review the changes today' }, @@ -208,6 +391,54 @@ suite('Automation dialog creation', () => { assert.strictEqual(result.value.name, 'Summarize changes'); }); + suite('Automation customization selection', () => { + test('cancels target reloads and preserves only explicit toggles by id', async () => { + const requests: { target: string; cancelled: () => boolean; result: DeferredPromise }[] = []; + const container = DOM.$('div'); + const selection = disposables.add(new AutomationCustomizationSelection( + container, + upcastPartial({ + getCustomizationChoices: async (target, existingId, token) => { + assert.strictEqual(existingId, 'existing'); + const result = new DeferredPromise(); + requests.push({ target: target.sessionTypeId ?? '', cancelled: () => token.isCancellationRequested, result }); + return result.p; + }, + }), + upcastPartial({ setupDelayedHover: () => toDisposable(() => { }) }), + new NullLogService(), + 'existing', + )); + selection.updateTarget({ kind: 'quickChat', providerId: 'host', sessionTypeId: 'one' }); + void requests[0].result.complete([ + { id: 'toggled', label: 'Toggled', selected: true, outdated: false }, + { id: 'default', label: 'Default', selected: true, outdated: false }, + ]); + await timeout(0); + container.querySelector('.automation-advanced-disclosure')!.click(); + container.querySelector('.monaco-checkbox')!.click(); + selection.updateTarget({ kind: 'workspace', folderUri: FOLDER, providerId: 'host', sessionTypeId: 'two', isolation: { kind: 'folder' } }); + selection.updateTarget({ kind: 'workspace', folderUri: FOLDER, providerId: 'host2', sessionTypeId: 'three', isolation: { kind: 'folder' } }); + void requests[2].result.complete([ + { id: 'new', label: 'New', selected: true, outdated: false }, + { id: 'toggled', label: 'Toggled', selected: true, outdated: true }, + { id: 'default', label: 'Default', selected: false, outdated: false }, + ]); + await timeout(0); + void requests[1].result.complete([{ id: 'stale', label: 'Stale', selected: true, outdated: false }]); + await timeout(0); + assert.deepStrictEqual({ + requests: requests.map(request => ({ target: request.target, cancelled: request.cancelled() })), + selected: selection.getSelectedIds(), + summary: container.querySelector('.automation-advanced-summary')?.textContent, + }, { + requests: [{ target: 'one', cancelled: true }, { target: 'two', cancelled: true }, { target: 'three', cancelled: false }], + selected: ['new'], + summary: '1 of 3 customizations · 1 outdated', + }); + }); + }); + test('preserves a supplied quick-chat target and custom name', async () => { const initialValues = { name: ' Custom title ', prompt: 'Review changes', diff --git a/src/vs/sessions/contrib/automations/test/browser/providerAutomationService.test.ts b/src/vs/sessions/contrib/automations/test/browser/providerAutomationService.test.ts index 1c54054bac9c92..6c575049c54c40 100644 --- a/src/vs/sessions/contrib/automations/test/browser/providerAutomationService.test.ts +++ b/src/vs/sessions/contrib/automations/test/browser/providerAutomationService.test.ts @@ -5,6 +5,7 @@ import assert from 'assert'; import { timeout } from '../../../../../base/common/async.js'; +import { CancellationToken } from '../../../../../base/common/cancellation.js'; import { Emitter } from '../../../../../base/common/event.js'; import { autorun, derived, observableValue } from '../../../../../base/common/observable.js'; import { URI } from '../../../../../base/common/uri.js'; @@ -123,6 +124,32 @@ suite('ProviderAutomationService', () => { assert.deepStrictEqual(states, ['loading', 'unavailable']); }); + test('routes customization choices to the target owner, not the saved automation owner', async () => { + const local = new TestAuthority('local'); + const remote = new TestAuthority('remote'); + const choices = [{ id: 'plugin', label: 'Plugin', selected: true, outdated: false }]; + const requests: { providerId: string | undefined; existingId: string | undefined; token: CancellationToken }[] = []; + const remoteProvider = upcastPartial({ + id: remote.providerId, label: remote.providerId, + automations: upcastPartial({ + getCustomizationChoices: async (target, existingId, token) => { + requests.push({ providerId: target.providerId, existingId, token }); + return choices; + }, + }), + }); + const { service } = setup([provider(local), remoteProvider]); + assert.deepStrictEqual({ + remote: await service.getCustomizationChoices(automation('remote').target, 'local-automation', CancellationToken.None), + unsupported: await service.getCustomizationChoices(automation('local').target, undefined, CancellationToken.None), + missing: await service.getCustomizationChoices(automation('missing').target, undefined, CancellationToken.None), + requests, + }, { + remote: choices, unsupported: undefined, missing: undefined, + requests: [{ providerId: 'remote', existingId: 'local-automation', token: CancellationToken.None }], + }); + }); + test('aggregates availability while retaining independent multi-host operations', async () => { const local = new TestAuthority('local'); const remote = new TestAuthority('remote'); diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostAutomationStore.ts b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostAutomationStore.ts index 663286fbf88b05..14cdb87d0e01d5 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostAutomationStore.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostAutomationStore.ts @@ -3,13 +3,14 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { disposableTimeout } from '../../../../../base/common/async.js'; +import { disposableTimeout, raceCancellationError } from '../../../../../base/common/async.js'; import { CancellationToken } from '../../../../../base/common/cancellation.js'; import { CancellationError } from '../../../../../base/common/errors.js'; import { Event } from '../../../../../base/common/event.js'; import { Disposable, DisposableMap, DisposableStore, toDisposable, type IReference } from '../../../../../base/common/lifecycle.js'; +import { stableStringify } from '../../../../../base/common/objects.js'; import { derived, type IObservable, observableSignalFromEvent, observableValue } from '../../../../../base/common/observable.js'; -import { isEqual, isEqualOrParent, joinPath, relativePath } from '../../../../../base/common/resources.js'; +import { basename, isEqual, isEqualOrParent, joinPath, relativePath } from '../../../../../base/common/resources.js'; import { hasKey } from '../../../../../base/common/types.js'; import { URI } from '../../../../../base/common/uri.js'; import { generateUuid } from '../../../../../base/common/uuid.js'; @@ -27,7 +28,7 @@ import { IStorageService, StorageScope } from '../../../../../platform/storage/c import { type IAgentCustomizationScope, IAgentHostActiveClientService } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostActiveClientService.js'; import { SYNCED_CUSTOMIZATION_SCHEME } from '../../../../../workbench/services/agentHost/common/agentHostFileSystemService.js'; import { assertAutomationSessionTemplate, type AutomationTarget, type IAutomationDescriptor, type IAutomationRun, type IAutomationSchedule, type IAutomationSessionTemplate } from '../../../../../workbench/contrib/chat/common/automations/automation.js'; -import { AutomationUnavailableError, type AutomationCatalogueState, assertAutomationSessionTemplateAuthority, type AutomationMutationGuard, type IAutomationRunRequestResult, type ICreateAutomationOptions, type IGuardedAutomationUpdateResult, serializeAutomationEditableState, type IUpdateAutomationOptions } from '../../../../../workbench/contrib/chat/common/automations/automationService.js'; +import { AutomationUnavailableError, type AutomationCatalogueState, assertAutomationSessionTemplateAuthority, type AutomationMutationGuard, type IAutomationCustomizationChoice, type IAutomationRunRequestResult, type ICreateAutomationOptions, type IGuardedAutomationUpdateResult, serializeAutomationEditableState, type IUpdateAutomationOptions } from '../../../../../workbench/contrib/chat/common/automations/automationService.js'; import type { ISessionsProviderAutomations } from '../../../../services/sessions/common/sessionsProvider.js'; const MUTATION_TIMEOUT_MS = 30_000; @@ -169,7 +170,7 @@ export class AgentHostAutomationStore extends Disposable implements ISessionsPro createdAt: now.toISOString(), updatedAt: now.toISOString(), }; - const state = await this._createDescriptor(resource, descriptor, mutationGuard); + const state = await this._createDescriptor(resource, descriptor, mutationGuard, options.customizationIds); return this._requireProjectedAutomation(state); } @@ -177,7 +178,7 @@ export class AgentHostAutomationStore extends Disposable implements ISessionsPro this._requireOperation(id, AutomationOperation.Update); const current = this._requireAutomation(id); const updated = this._applyPatch(current, patch); - const state = await this._replaceDescriptor(updated, patch.sessionTemplate === null, mutationGuard); + const state = await this._replaceDescriptor(updated, patch.sessionTemplate === null, mutationGuard, patch.customizationIds); return this._requireProjectedAutomation(state); } @@ -189,6 +190,55 @@ export class AgentHostAutomationStore extends Disposable implements ISessionsPro return { kind: 'updated', automation: await this.updateAutomation(id, patch, mutationGuard) }; } + async getCustomizationChoices(target: AutomationTarget, existingId: string | undefined, token: CancellationToken): Promise { + if (token.isCancellationRequested) { + throw new CancellationError(); + } + const existing = existingId ? this._findAutomationEntry(existingId)?.definition : undefined; + const scope = this._acquireCustomizationScope(target, target.sessionTypeId ?? existing?.session.provider); + if (!scope) { + return undefined; + } + try { + await raceCancellationError(scope.whenResolved(), token); + if (token.isCancellationRequested) { + throw new CancellationError(); + } + const current = scope.customizations.get().filter(isCustomizationEnabled); + const saved = new Map(existing?.session.customizations?.map(plugin => [plugin.id, plugin])); + const sameTarget = existing !== undefined && this._isCustomizationTargetUnchanged(target, existing, target.sessionTypeId ?? existing.session.provider, true); + const currentIds = new Set(current.map(plugin => plugin.id)); + return [ + ...current.map(plugin => ({ + id: plugin.id, + label: this._customizationLabel(plugin), + description: URI.parse(plugin.uri).scheme === SYNCED_CUSTOMIZATION_SCHEME + ? localize('agentHostAutomation.syncedCustomizationDescription', "Customizations synced from VS Code.") + : plugin.version ? localize('agentHostAutomation.customizationVersion', "Version {0}", plugin.version) : undefined, + selected: !sameTarget || saved.has(plugin.id), + outdated: sameTarget && saved.has(plugin.id) + && (saved.get(plugin.id)!.nonce !== plugin.nonce || saved.get(plugin.id)!.uri !== plugin.uri), + })), + ...[...saved.values()].filter(plugin => !currentIds.has(plugin.id)).map(plugin => ({ + id: plugin.id, + label: this._customizationLabel(plugin), + description: localize('agentHostAutomation.savedCustomization', "Not available locally. The saved copy is kept."), + selected: true, + outdated: false, + })), + ]; + } finally { + scope.dispose(); + } + } + + private _customizationLabel(plugin: ClientPluginCustomization): string { + const uri = URI.parse(plugin.uri); + return uri.scheme === SYNCED_CUSTOMIZATION_SCHEME + ? localize('agentHostAutomation.syncedCustomizations', "VS Code Customizations") + : plugin.name || basename(uri); + } + async deleteAutomation(id: string, mutationGuard?: AutomationMutationGuard): Promise { const { resource } = this._requireOperation(id, AutomationOperation.Remove); await this._dispatchAndWait( @@ -398,12 +448,12 @@ export class AgentHostAutomationStore extends Disposable implements ISessionsPro return automation; } - private async _createDescriptor(resource: string, descriptor: IAutomationDescriptor, mutationGuard?: AutomationMutationGuard): Promise { + private async _createDescriptor(resource: string, descriptor: IAutomationDescriptor, mutationGuard?: AutomationMutationGuard, customizationIds?: readonly string[]): Promise { const definition = this._definitionFromDescriptor(descriptor); - const scope = this._acquireCustomizationScope(descriptor, definition); + const scope = this._acquireCustomizationScope(descriptor.target, definition.session.provider); try { if (scope) { - await this._captureCustomizations(definition, scope); + await this._captureCustomizations(definition, scope, undefined, customizationIds); } const state = await this._dispatchAndWait( { type: ActionType.AutomationCreateRequested, resource, definition }, @@ -420,17 +470,19 @@ export class AgentHostAutomationStore extends Disposable implements ISessionsPro } } - private async _replaceDescriptor(descriptor: IAutomationDescriptor, resetSessionTemplate = false, mutationGuard?: AutomationMutationGuard): Promise { + private async _replaceDescriptor(descriptor: IAutomationDescriptor, resetSessionTemplate = false, mutationGuard?: AutomationMutationGuard, customizationIds?: readonly string[]): Promise { const current = this._findAutomationEntry(descriptor.id); if (!current) { throw new Error(`Automation does not exist: ${descriptor.id}`); } const resource = current.resource; const definition = this._definitionFromDescriptor(descriptor, current.definition, resetSessionTemplate); - const scope = this._acquireCustomizationScope(descriptor, definition, current.definition); + const scope = customizationIds !== undefined || !this._isCustomizationTargetUnchanged(descriptor.target, current.definition, definition.session.provider) + ? this._acquireCustomizationScope(descriptor.target, definition.session.provider) + : undefined; try { if (scope) { - await this._captureCustomizations(definition, scope, current.definition); + await this._captureCustomizations(definition, scope, current.definition, customizationIds); } const expected = this._requireProjectedAutomation({ ...current, definition }); const state = await this._dispatchAndWait( @@ -448,6 +500,9 @@ export class AgentHostAutomationStore extends Disposable implements ISessionsPro }, catalog => { const state = catalog.entries.find(automation => automation.resource === resource); + if (scope && stableStringify(state?.definition.session.customizations) !== stableStringify(definition.session.customizations)) { + return false; + } const projected = this._projectAutomation(state); if (projected === undefined || serializeAutomationEditableState(projected) !== serializeAutomationEditableState(expected)) { @@ -456,6 +511,7 @@ export class AgentHostAutomationStore extends Disposable implements ISessionsPro return true; }, mutationGuard, + !!scope, ); if (!state) { throw new Error(`Automation update completed without authoritative state: ${resource}`); @@ -466,31 +522,34 @@ export class AgentHostAutomationStore extends Disposable implements ISessionsPro } } - private _acquireCustomizationScope(descriptor: IAutomationDescriptor, definition: AutomationDefinition, existing?: AutomationDefinition): IAgentCustomizationScope | undefined { - if (!this._connection.initializeResult.get()?.automations?.customizations) { - return undefined; - } - const target = descriptor.target; - const previousTarget = existing && this._projectTarget(existing); - if (existing && previousTarget + private _isCustomizationTargetUnchanged(target: AutomationTarget, existing: AutomationDefinition, provider: string | undefined, compareIsolation = false): boolean { + const previousTarget = this._projectTarget(existing); + return !!previousTarget && previousTarget.providerId === target.providerId && previousTarget.sessionTypeId === target.sessionTypeId - && existing.session.provider === definition.session.provider + && existing.session.provider === provider && previousTarget.kind === target.kind - && (previousTarget.kind !== 'workspace' || target.kind !== 'workspace' || isEqual(previousTarget.folderUri, target.folderUri))) { - return undefined; - } - const provider = definition.session.provider; - if (!provider) { + && (previousTarget.kind !== 'workspace' || target.kind !== 'workspace' + || (isEqual(previousTarget.folderUri, target.folderUri) + && (!compareIsolation || stableStringify(previousTarget.isolation) === stableStringify(target.isolation)))); + } + + private _acquireCustomizationScope(target: AutomationTarget, provider: string | undefined): IAgentCustomizationScope | undefined { + if (!this._connection.initializeResult.get()?.automations?.customizations || !provider) { return undefined; } const sessionType = this._boundaryMapper?.resourceSchemeForProvider(provider) ?? provider; return this._activeClientService.acquireScope(sessionType, target.kind === 'workspace' ? [target.folderUri] : []); } - private async _captureCustomizations(definition: AutomationDefinition, scope: IAgentCustomizationScope, previous?: AutomationDefinition): Promise { + private async _captureCustomizations(definition: AutomationDefinition, scope: IAgentCustomizationScope, previous?: AutomationDefinition, customizationIds?: readonly string[]): Promise { await scope.whenResolved(); - const customizations = scope.customizations.get().filter(isCustomizationEnabled); + const current = scope.customizations.get().filter(isCustomizationEnabled); + const currentIds = new Set(current.map(plugin => plugin.id)); + const selectedIds = customizationIds !== undefined ? new Set(customizationIds) : undefined; + const customizations = selectedIds + ? [...current, ...(previous?.session.customizations ?? []).filter(plugin => !currentIds.has(plugin.id))].filter(plugin => selectedIds.has(plugin.id)) + : current; definition.session.customizations = customizations; if (!definition.session.agent) { return; @@ -638,13 +697,15 @@ export class AgentHostAutomationStore extends Disposable implements ISessionsPro action: Parameters[1] & { readonly resource: string }, predicate: (catalog: AutomationState) => boolean, mutationGuard?: AutomationMutationGuard, + waitForCatalogChange = false, ): Promise { await this._waitForCatalog(() => true); if (this._store.isDisposed) { throw new CancellationError(); } mutationGuard?.(); - const result = this._waitForCatalog(predicate, action); + const beforeDispatch = this._catalog.value; + const result = this._waitForCatalog(catalog => (!waitForCatalogChange || catalog !== beforeDispatch) && predicate(catalog), action); this._connection.dispatch(AUTOMATION_CATALOG_URI, action); const catalog = await result; const state = catalog.entries.find(automation => automation.resource === action.resource); diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/reconnectableAgentHostAutomationStore.ts b/src/vs/sessions/contrib/providers/agentHost/browser/reconnectableAgentHostAutomationStore.ts index 875a33e0df6d0f..9546dc2905f8d8 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/reconnectableAgentHostAutomationStore.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/reconnectableAgentHostAutomationStore.ts @@ -10,8 +10,8 @@ import { localize } from '../../../../../nls.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { supportsAgentHostAutonomousAutomations } from '../../../../../platform/agentHost/common/meta/agentHostAutomationsMeta.js'; -import type { IAutomationDescriptor, IAutomationRun } from '../../../../../workbench/contrib/chat/common/automations/automation.js'; -import { AutomationUnavailableError, type AutomationCatalogueState, type AutomationMutationGuard, type AutomationUnavailableReasonCode, type IAutomationRunRequestResult, type ICreateAutomationOptions, type IGuardedAutomationUpdateResult, type IUpdateAutomationOptions } from '../../../../../workbench/contrib/chat/common/automations/automationService.js'; +import type { AutomationTarget, IAutomationDescriptor, IAutomationRun } from '../../../../../workbench/contrib/chat/common/automations/automation.js'; +import { AutomationUnavailableError, type AutomationCatalogueState, type AutomationMutationGuard, type AutomationUnavailableReasonCode, type IAutomationCustomizationChoice, type IAutomationRunRequestResult, type ICreateAutomationOptions, type IGuardedAutomationUpdateResult, type IUpdateAutomationOptions } from '../../../../../workbench/contrib/chat/common/automations/automationService.js'; import type { ISessionsProviderAutomations } from '../../../../services/sessions/common/sessionsProvider.js'; import { AgentHostAutomationStore, type IAgentHostAutomationBoundaryMapper, type IAgentHostAutomationConnection } from './agentHostAutomationStore.js'; import { CHAT_AUTOMATIONS_ENABLED_SETTING } from '../../../../../workbench/contrib/chat/common/automations/automationsEnabled.js'; @@ -110,6 +110,10 @@ export class ReconnectableAgentHostAutomationStore extends Disposable implements return this.currentStore.get()?.getAutomation(id); } + async getCustomizationChoices(target: AutomationTarget, existingId: string | undefined, token: CancellationToken): Promise { + return this.currentStore.get()?.getCustomizationChoices(target, existingId, token); + } + canRunAutomation(id: string): boolean { return this.currentStore.get()?.canRunAutomation(id) === true; } diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostAutomationStore.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostAutomationStore.test.ts index 3b383016d8cbb5..3ef4cc6c752749 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostAutomationStore.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostAutomationStore.test.ts @@ -5,7 +5,7 @@ import assert from 'assert'; import { DeferredPromise, timeout } from '../../../../../../base/common/async.js'; -import { CancellationTokenSource } from '../../../../../../base/common/cancellation.js'; +import { CancellationToken, CancellationTokenSource } from '../../../../../../base/common/cancellation.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; import { DisposableStore, type IReference } from '../../../../../../base/common/lifecycle.js'; import { ResourceMap } from '../../../../../../base/common/map.js'; @@ -60,8 +60,10 @@ class TestAutomationConnection { readonly runRequested = new DeferredPromise(); runAdmissionBarrier: Promise | undefined; suppressCreatePublication = false; + suppressUpdatePublication = false; updateError: Error | undefined; readonly createRequested = new DeferredPromise(); + readonly updateRequested = new DeferredPromise(); constructor(catalogAvailable = true) { this._catalogAvailable = catalogAvailable; @@ -149,9 +151,13 @@ class TestAutomationConnection { origin: undefined, }); } else if (action.type === ActionType.AutomationUpdateRequested) { + void this.updateRequested.complete(); if (this.updateError) { throw this.updateError; } + if (this.suppressUpdatePublication) { + return; + } const current = this._catalog.entries.find(automation => automation.resource === action.resource); if (!current) { throw new Error(`Missing Automation: ${action.resource}`); @@ -360,6 +366,263 @@ suite('AgentHostAutomationStore', () => { return connection; } + test('customization choices include enabled plugins in scope order, selected for creation', async () => { + const activeClient = new TestActiveClientService(); + const enabled = { ...plugin('virtual://client/plugins/enabled'), version: '1.2.3' }; + const unnamed = { ...plugin('virtual://client/plugins/unnamed'), name: '' }; + const bundle = { ...plugin('vscode-synced-customization:/bundle'), name: 'VS Code Synced Data' }; + activeClient.customizations.set([enabled, plugin('virtual://client/plugins/disabled', false), unnamed, bundle], undefined); + const { store } = reconnectable(true, activeClient); + store.setConnection(customizationConnection()); + assert.deepStrictEqual({ + choices: await store.getCustomizationChoices(createOptions().target, undefined, CancellationToken.None), + scopes: activeClient.scopes, + }, { + choices: [ + { id: enabled.id, label: 'Test Plugin', description: 'Version 1.2.3', selected: true, outdated: false }, + { id: unnamed.id, label: 'unnamed', description: undefined, selected: true, outdated: false }, + { id: bundle.id, label: 'VS Code Customizations', description: 'Customizations synced from VS Code.', selected: true, outdated: false }, + ], + scopes: [{ sessionType: 'copilotcli', roots: [], disposed: true }], + }); + }); + + test('editing customization choices reflects saved selection, changed refs and saved-only plugins', async () => { + const activeClient = new TestActiveClientService(); + const saved = plugin('virtual://client/plugins/saved'); + const moved = plugin('virtual://client/plugins/moved'); + const missing = plugin('virtual://other-client/plugins/missing'); + activeClient.customizations.set([saved, moved, missing], undefined); + const { store } = reconnectable(true, activeClient); + store.setConnection(customizationConnection()); + const automation = await store.createAutomation(createOptions()); + const added = plugin('virtual://client/plugins/added'); + activeClient.customizations.set([{ ...saved, nonce: 'v2' }, { ...moved, uri: 'virtual://client/plugins/new-location' }, added], undefined); + const choices = await store.getCustomizationChoices(automation.target, automation.id, CancellationToken.None); + const retargeted = await store.getCustomizationChoices({ ...automation.target, sessionTypeId: 'claude' }, automation.id, CancellationToken.None); + assert.deepStrictEqual({ choices, retargeted: retargeted?.map(({ id, selected, outdated }) => ({ id, selected, outdated })) }, { + choices: [ + { id: saved.id, label: 'Test Plugin', description: undefined, selected: true, outdated: true }, + { id: moved.id, label: 'Test Plugin', description: undefined, selected: true, outdated: true }, + { id: added.id, label: 'Test Plugin', description: undefined, selected: false, outdated: false }, + { id: missing.id, label: 'Test Plugin', description: 'Not available locally. The saved copy is kept.', selected: true, outdated: false }, + ], + retargeted: [saved, moved, added, missing].map(plugin => ({ id: plugin.id, selected: true, outdated: false })), + }); + }); + + test('customization choices are unsupported without the capability or a connection', async () => { + const activeClient = new TestActiveClientService(); + const { store } = reconnectable(true, activeClient); + const disconnected = await store.getCustomizationChoices(createOptions().target, undefined, CancellationToken.None); + store.setConnection(disposables.add(new TestAutomationConnection())); + assert.deepStrictEqual({ + disconnected, + unsupported: await store.getCustomizationChoices(createOptions().target, undefined, CancellationToken.None), + scopes: activeClient.scopes, + }, { disconnected: undefined, unsupported: undefined, scopes: [] }); + }); + + test('changing workspace isolation selects every enabled customization without marking it outdated', async () => { + const activeClient = new TestActiveClientService(); + const saved = plugin('virtual://client/plugins/saved'); + const added = plugin('virtual://client/plugins/added'); + activeClient.customizations.set([saved], undefined); + const { store } = reconnectable(true, activeClient); + store.setConnection(customizationConnection()); + const target = { + kind: 'workspace', providerId: 'host', sessionTypeId: 'copilotcli', + folderUri: URI.file('/workspace'), isolation: { kind: 'default' }, + } as const; + const automation = await store.createAutomation({ ...createOptions(), target }); + activeClient.customizations.set([{ ...saved, nonce: 'v2' }, added], undefined); + assert.deepStrictEqual(await store.getCustomizationChoices({ ...target, isolation: { kind: 'folder' } }, automation.id, CancellationToken.None), [ + { id: saved.id, label: 'Test Plugin', description: undefined, selected: true, outdated: false }, + { id: added.id, label: 'Test Plugin', description: undefined, selected: true, outdated: false }, + ]); + }); + + test('customization choice cancellation and resolution failures dispose the scope', async () => { + for (const failure of ['cancelled', 'resolution'] as const) { + const activeClient = new TestActiveClientService(); + const { store } = reconnectable(true, activeClient); + store.setConnection(customizationConnection()); + const tokenSource = disposables.add(new CancellationTokenSource()); + const resolution = new DeferredPromise(); + activeClient.resolution = failure === 'resolution' ? Promise.reject(new Error('Resolution failed')) : resolution.p; + const pending = store.getCustomizationChoices(createOptions().target, undefined, tokenSource.token); + if (failure === 'cancelled') { + tokenSource.cancel(); + } + await assert.rejects(pending, failure === 'resolution' ? /Resolution failed/ : /Canceled/); + assert.deepStrictEqual(activeClient.scopes, [{ sessionType: 'copilotcli', roots: [], disposed: true }]); + await resolution.complete(); + } + }); + + test('creation filters customization ids in scope order and ignores disabled and unknown ids', async () => { + const activeClient = new TestActiveClientService(); + const first = plugin('virtual://client/plugins/first'); + const second = plugin('virtual://client/plugins/second'); + const disabled = plugin('virtual://client/plugins/disabled', false); + activeClient.customizations.set([first, second, disabled], undefined); + const { store } = reconnectable(true, activeClient); + const connection = customizationConnection(); + store.setConnection(connection); + await store.createAutomation({ ...createOptions(), customizationIds: [second.id, 'unknown', disabled.id, first.id] }); + await store.createAutomation({ ...createOptions(), customizationIds: [second.id] }); + assert.deepStrictEqual(connection.dispatched.map(({ action }) => { + assert.ok(action.type === ActionType.AutomationCreateRequested); + return action.definition.session.customizations; + }), [[first, second], [second]]); + }); + + test('updating customization ids refreshes current refs and keeps saved-only refs verbatim', async () => { + const activeClient = new TestActiveClientService(); + const saved = plugin('virtual://client/plugins/saved'); + const missing = plugin('virtual://other-client/plugins/missing'); + activeClient.customizations.set([saved, missing], undefined); + const { store } = reconnectable(true, activeClient); + const connection = customizationConnection(); + store.setConnection(connection); + const automation = await store.createAutomation(createOptions()); + const refreshed = { ...saved, nonce: 'v2' }; + activeClient.customizations.set([refreshed, plugin('virtual://client/plugins/unselected')], undefined); + await store.updateAutomation(automation.id, { customizationIds: [missing.id, saved.id, 'unknown'] }); + const update = connection.dispatched[1].action; + assert.ok(update.type === ActionType.AutomationUpdateRequested); + assert.deepStrictEqual({ + customizations: update.changes.session?.customizations, + savedRefReused: update.changes.session?.customizations?.[1] === missing, + scopes: activeClient.scopes, + }, { + customizations: [refreshed, missing], + savedRefReused: true, + scopes: [ + { sessionType: 'copilotcli', roots: [], disposed: true }, + { sessionType: 'copilotcli', roots: [], disposed: true }, + ], + }); + }); + + test('empty customization selection clears captured refs on creation and guarded update', async () => { + const activeClient = new TestActiveClientService(); + activeClient.customizations.set([plugin('virtual://client/plugins/saved')], undefined); + const { store } = reconnectable(true, activeClient); + const connection = customizationConnection(); + store.setConnection(connection); + await store.createAutomation({ ...createOptions(), customizationIds: [] }); + const automation = await store.createAutomation(createOptions()); + await store.updateAutomationIfUnchanged(automation.id, { customizationIds: [] }, automation); + assert.deepStrictEqual(connection.dispatched.map(({ action }) => { + assert.ok(action.type === ActionType.AutomationCreateRequested || action.type === ActionType.AutomationUpdateRequested); + return action.type === ActionType.AutomationCreateRequested ? action.definition.session.customizations : action.changes.session?.customizations; + }), [[], activeClient.customizations.get(), []]); + }); + + test('customization-only updates hold the scope until the refreshed refs are published', async () => { + const activeClient = new TestActiveClientService(); + const saved = plugin('virtual://client/plugins/saved'); + activeClient.customizations.set([saved], undefined); + const { store } = reconnectable(true, activeClient); + const connection = customizationConnection(); + store.setConnection(connection); + const automation = await store.createAutomation(createOptions()); + const refreshed = { ...saved, nonce: 'v2' }; + activeClient.customizations.set([refreshed], undefined); + connection.suppressUpdatePublication = true; + const pending = store.updateAutomation(automation.id, { customizationIds: [saved.id] }); + await connection.updateRequested.p; + await timeout(0); + const duringDispatch = activeClient.scopes[1].disposed; + const create = connection.dispatched[0].action; + const update = connection.dispatched[1].action; + assert.ok(create.type === ActionType.AutomationCreateRequested && update.type === ActionType.AutomationUpdateRequested); + connection.setAutomation({ + resource: create.resource, definition: { ...create.definition, ...update.changes }, runs: [], + operations: [AutomationOperation.Update], createdAt: automation.createdAt, modifiedAt: automation.updatedAt, + }); + await pending; + assert.deepStrictEqual({ duringDispatch, afterResponse: activeClient.scopes[1].disposed }, { duringDispatch: false, afterResponse: true }); + }); + + test('unchanged selections wait for a host response even when the catalogue changes during scope resolution', async () => { + const activeClient = new TestActiveClientService(); + const saved = plugin('virtual://client/plugins/saved'); + activeClient.customizations.set([saved], undefined); + const { store } = reconnectable(true, activeClient); + const connection = customizationConnection(); + store.setConnection(connection); + const automation = await store.createAutomation(createOptions()); + const create = connection.dispatched[0].action; + assert.ok(create.type === ActionType.AutomationCreateRequested); + const resolution = new DeferredPromise(); + activeClient.resolution = resolution.p; + connection.suppressUpdatePublication = true; + const pending = store.updateAutomation(automation.id, { customizationIds: [saved.id] }); + const entry = { + resource: create.resource, definition: create.definition, runs: [], + operations: [AutomationOperation.Update], createdAt: automation.createdAt, modifiedAt: automation.updatedAt, + }; + connection.setAutomation(entry); + await resolution.complete(); + await connection.updateRequested.p; + await timeout(0); + const duringDispatch = activeClient.scopes[1].disposed; + const update = connection.dispatched[1].action; + assert.ok(update.type === ActionType.AutomationUpdateRequested); + connection.setAutomation({ ...entry, definition: { ...create.definition, ...update.changes } }); + await pending; + assert.deepStrictEqual({ duringDispatch, afterResponse: activeClient.scopes[1].disposed }, { duringDispatch: false, afterResponse: true }); + }); + + test('hosts without customization support ignore explicit selections', async () => { + const activeClient = new TestActiveClientService(); + activeClient.customizations.set([plugin('virtual://client/plugins/saved')], undefined); + const { store } = reconnectable(true, activeClient); + const connection = disposables.add(new TestAutomationConnection()); + store.setConnection(connection); + const automation = await store.createAutomation({ ...createOptions(), customizationIds: [] }); + await store.updateAutomation(automation.id, { customizationIds: [] }); + assert.deepStrictEqual({ + customizations: connection.dispatched.map(({ action }) => { + assert.ok(action.type === ActionType.AutomationCreateRequested || action.type === ActionType.AutomationUpdateRequested); + return action.type === ActionType.AutomationCreateRequested ? action.definition.session.customizations : action.changes.session?.customizations; + }), + scopes: activeClient.scopes, + }, { customizations: [undefined, undefined], scopes: [] }); + }); + + test('customization selection remaps bundled agents only when their bundle is selected', async () => { + const activeClient = new TestActiveClientService(); + const source = URI.file('/user/prompts/review.agent.md'); + const bundle = plugin('vscode-synced-customization:/bundle'); + const bundledAgent = URI.joinPath(URI.parse(bundle.uri), 'agents', 'review.agent.md'); + activeClient.customizations.set([bundle], undefined); + activeClient.syncedUris.set(source, bundledAgent); + const { store } = reconnectable(true, activeClient); + const connection = customizationConnection(); + store.setConnection(connection); + const automations: IAutomationDescriptor[] = []; + for (const customizationIds of [[bundle.id], []]) { + automations.push(await store.createAutomation({ ...createOptions(), sessionTemplate: { agent: { uri: source.toString() } }, customizationIds })); + } + const refreshed = { ...bundle, uri: 'vscode-synced-customization:/refreshed', nonce: 'v2' }; + const refreshedAgent = URI.joinPath(URI.parse(refreshed.uri), 'agents', 'review.agent.md'); + activeClient.customizations.set([refreshed], undefined); + activeClient.syncedUris.set(source, refreshedAgent); + await store.updateAutomation(automations[0].id, { customizationIds: [bundle.id] }); + assert.deepStrictEqual(connection.dispatched.map(({ action }) => { + assert.ok(action.type === ActionType.AutomationCreateRequested || action.type === ActionType.AutomationUpdateRequested); + const session = action.type === ActionType.AutomationCreateRequested ? action.definition.session : action.changes.session!; + return { agent: session.agent, customizations: session.customizations }; + }), [ + { agent: { uri: bundledAgent.toString() }, customizations: [bundle] }, + { agent: { uri: source.toString() }, customizations: [] }, + { agent: { uri: refreshedAgent.toString() }, customizations: [refreshed] }, + ]); + }); + test('captures enabled customizations only with negotiated capability', async () => { const enabled = plugin('virtual://client/plugins/enabled'); const inherited = { ...plugin('virtual://client/plugins/inherited'), enablement: undefined }; diff --git a/src/vs/sessions/contrib/sessions/browser/views/automationsView.ts b/src/vs/sessions/contrib/sessions/browser/views/automationsView.ts index 86fc325dbdab01..8890d7b335f19b 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/automationsView.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/automationsView.ts @@ -1013,17 +1013,27 @@ class AutomationCardsSection extends Disposable { return; } try { - const result = await this.automationDialogService.showAutomationDialog(initialValues ? { initialValues } : {}); - if (!result || result.kind !== 'create' || this._store.isDisposed) { - return; - } - const restoreFocus = DOM.isAncestorOfActiveElement(this.focusRoot); - const focusRequestGeneration = this.focusRequestGeneration; - if (!await this.ensureEnabled()) { + let created: IAutomationDescriptor | undefined; + let restoreFocus = false; + let focusRequestGeneration = this.focusRequestGeneration; + const result = await this.automationDialogService.showAutomationDialog({ + ...(initialValues ? { initialValues } : {}), + commit: async result => { + if (result.kind === 'create') { + if (this._store.isDisposed) { + throw new Error(localize('automationViewClosedBeforeSave', "The automations view was closed before the automation could be saved.")); + } + this.throwIfDisabled(); + restoreFocus = DOM.isAncestorOfActiveElement(this.focusRoot) || !!this.focusRoot.closest('.automation-dialog-open'); + focusRequestGeneration = this.focusRequestGeneration; + created = await withAutomationDialogPersistenceTelemetry(this.telemetryService, 'create', () => + this.automationService.createAutomation(result.value, () => this.throwIfDisabled())); + } + }, + }); + if (!result || !created || this._store.isDisposed) { return; } - const created = await withAutomationDialogPersistenceTelemetry(this.telemetryService, 'create', () => - this.automationService.createAutomation(result.value, () => this.throwIfDisabled())); if (restoreFocus && focusRequestGeneration === this.focusRequestGeneration && !this._store.isDisposed && DOM.isAncestorOfActiveElement(this.focusRoot)) { this.pendingFocusAutomationId = created.id; this.focusPendingAutomation(); @@ -1085,20 +1095,25 @@ class AutomationCardsSection extends Disposable { return; } try { - const result = await this.automationDialogService.showAutomationDialog({ existing: automation }); + const result = await this.automationDialogService.showAutomationDialog({ + existing: automation, + commit: async result => { + if (result.kind !== 'update') { + return; + } + this.throwIfDisabled(); + const updateResult = await withAutomationDialogPersistenceTelemetry(this.telemetryService, 'update', () => + this.automationService.updateAutomationIfUnchanged(result.id, result.value, automation, () => this.throwIfDisabled())); + if (updateResult.kind === 'conflict') { + throw new Error(updateResult.current + ? localize('automationChangedDuringEdit', "This automation changed while the dialog was open. Reopen it to review the latest values.") + : localize('automationDeletedDuringEdit', "This automation was deleted while the dialog was open.")); + } + }, + }); if (!result || result.kind !== 'update') { return; } - if (!await this.ensureEnabled()) { - return; - } - const updateResult = await withAutomationDialogPersistenceTelemetry(this.telemetryService, 'update', () => - this.automationService.updateAutomationIfUnchanged(result.id, result.value, automation, () => this.throwIfDisabled())); - if (updateResult.kind === 'conflict') { - throw new Error(updateResult.current - ? localize('automationChangedDuringEdit', "This automation changed while the dialog was open. Reopen it to review the latest values.") - : localize('automationDeletedDuringEdit', "This automation was deleted while the dialog was open.")); - } status(localize('automationUpdatedStatus', "Updated automation {0}", automation.name)); } catch (err) { this.logService.error('[AutomationsCards] Failed to update automation', err); @@ -1725,6 +1740,7 @@ async function importAutomationBlueprint( return; } + let automation: IAutomationDescriptor | undefined; const result = await automationDialogService.showAutomationDialog({ initialValues: { name: blueprint.name, @@ -1732,30 +1748,21 @@ async function importAutomationBlueprint( schedule: blueprint.schedule, enabled: false, }, + commit: async result => { + if (result.kind === 'create') { + automation = await withAutomationDialogPersistenceTelemetry(telemetryService, 'create', () => + automationService.createAutomation(result.value, () => { + if (!isEnabled()) { + throw new Error(localize('automationsDisabledBeforeImport', "Automations were disabled before the imported automation could be saved.")); + } + })); + } + }, }); - if (!result || result.kind !== 'create') { + if (!result || !automation) { return; } - if (!isEnabled()) { - await showAutomationsDisabled(dialogService); - return; - } - - try { - const automation = await withAutomationDialogPersistenceTelemetry(telemetryService, 'create', () => - automationService.createAutomation(result.value, () => { - if (!isEnabled()) { - throw new Error(localize('automationsDisabledBeforeImport', "Automations were disabled before the imported automation could be saved.")); - } - })); - status(localize('automationImportedStatus', "Imported automation {0}", automation.name)); - } catch (error) { - logService.error('[Automations] Failed to import Automation blueprint', error); - await dialogService.error( - localize('automationImportFailed', "Failed to import automation."), - getErrorMessage(error), - ); - } + status(localize('automationImportedStatus', "Imported automation {0}", automation.name)); } async function showAutomationsDisabled(dialogService: IDialogService): Promise { @@ -2026,35 +2033,24 @@ registerAction2(class NewAutomationAction extends Action2 { const automationService = accessor.get(IAutomationService); const configurationService = accessor.get(IConfigurationService); const dialogService = accessor.get(IDialogService); - const logService = accessor.get(ILogService); const telemetryService = accessor.get(ITelemetryService); const isEnabled = () => configurationService.getValue(CHAT_AUTOMATIONS_ENABLED_SETTING) === true; if (!isEnabled()) { await showAutomationsDisabled(dialogService); return; } - const result = await automationDialogService.showAutomationDialog({}); - if (!result || result.kind !== 'create') { - return; - } - if (!isEnabled()) { - await showAutomationsDisabled(dialogService); - return; - } - try { - await withAutomationDialogPersistenceTelemetry(telemetryService, 'create', () => - automationService.createAutomation(result.value, () => { - if (!isEnabled()) { - throw new Error(localize('automationsDisabledBeforeSave', "Automations were disabled before the change could be saved.")); - } - })); - } catch (err) { - logService.error('[Automations] Failed to create automation', err); - await dialogService.error( - localize('automationCreateFailed', "Failed to create automation."), - getErrorMessage(err), - ); - } + await automationDialogService.showAutomationDialog({ + commit: async result => { + if (result.kind === 'create') { + await withAutomationDialogPersistenceTelemetry(telemetryService, 'create', () => + automationService.createAutomation(result.value, () => { + if (!isEnabled()) { + throw new Error(localize('automationsDisabledBeforeSave', "Automations were disabled before the change could be saved.")); + } + })); + } + }, + }); } }); @@ -2140,6 +2136,7 @@ registerAction2(class DuplicateAutomationAction extends Action2 { try { const name = getDuplicateAutomationName(automation.name, automationService.automations.get()); + let duplicate: IAutomationDescriptor | undefined; const result = await automationDialogService.showAutomationDialog({ initialValues: { name, @@ -2155,20 +2152,20 @@ registerAction2(class DuplicateAutomationAction extends Action2 { }), enabled: automation.enabled, }, + commit: async result => { + if (result.kind === 'create') { + duplicate = await withAutomationDialogPersistenceTelemetry(telemetryService, 'create', () => + automationService.createAutomation(result.value, () => { + if (!isEnabled()) { + throw new Error(localize('automationsDisabledBeforeDuplicate', "Automations were disabled before the duplicate could be saved.")); + } + })); + } + }, }); - if (!result || result.kind !== 'create') { - return; - } - if (!isEnabled()) { - await showAutomationsDisabled(dialogService); + if (!result || !duplicate) { return; } - const duplicate = await withAutomationDialogPersistenceTelemetry(telemetryService, 'create', () => - automationService.createAutomation(result.value, () => { - if (!isEnabled()) { - throw new Error(localize('automationsDisabledBeforeDuplicate', "Automations were disabled before the duplicate could be saved.")); - } - })); status(localize('automationDuplicatedStatus', "Created duplicate automation {0}", duplicate.name)); } catch (error) { logService.error('[Automations] Failed to duplicate automation', error); diff --git a/src/vs/sessions/contrib/sessions/test/browser/automationsView.test.ts b/src/vs/sessions/contrib/sessions/test/browser/automationsView.test.ts index ee43b85667042b..32dc8181ab3f07 100644 --- a/src/vs/sessions/contrib/sessions/test/browser/automationsView.test.ts +++ b/src/vs/sessions/contrib/sessions/test/browser/automationsView.test.ts @@ -15,6 +15,7 @@ import { DeferredPromise, timeout } from '../../../../../base/common/async.js'; import { CancellationToken } from '../../../../../base/common/cancellation.js'; import { Codicon } from '../../../../../base/common/codicons.js'; import { Emitter, Event } from '../../../../../base/common/event.js'; +import { getErrorMessage } from '../../../../../base/common/errors.js'; import { constObservable, IObservable, observableValue } from '../../../../../base/common/observable.js'; import { DisposableStore, IDisposable, toDisposable } from '../../../../../base/common/lifecycle.js'; import { URI } from '../../../../../base/common/uri.js'; @@ -298,6 +299,8 @@ class FakeAutomationDialogService extends mock() { beforeReturn: (() => void) | undefined; showCalls = 0; lastOptions: IShowAutomationDialogOptions | undefined; + readonly commitErrors: string[] = []; + readonly commitFailed = new DeferredPromise(); override async showAutomationDialog(options: IShowAutomationDialogOptions): Promise { this.showCalls++; @@ -306,6 +309,15 @@ class FakeAutomationDialogService extends mock() { throw this.error; } this.beforeReturn?.(); + if (this.result && options.commit) { + try { + await options.commit(this.result); + } catch (error) { + this.commitErrors.push(getErrorMessage(error)); + void this.commitFailed.complete(); + return undefined; + } + } return this.result; } } @@ -1576,6 +1588,7 @@ suite('AutomationsCardsWidget', () => { accessibleDescription: describedBy ? widget.element.querySelector(`#${describedBy}`)?.textContent : undefined, }, { dialogOptions: { + commit: automationDialogService.lastOptions?.commit, initialValues: { name: template.name, prompt: template.prompt, @@ -1916,13 +1929,15 @@ suite('AutomationsCardsWidget', () => { automationService.setCatalogueState('ready'); widget.element.querySelector('.automations-template-card')?.click(); - await dialogService.infoCalled.p; + await automationDialogService.commitFailed.p; assert.deepStrictEqual({ info: dialogService.infos, + inlineErrors: automationDialogService.commitErrors, createCalls: automationService.createCalls, }, { - info: ['Automations are disabled.'], + info: [], + inlineErrors: ['Automations were disabled before the change could be saved.'], createCalls: [], }); }); @@ -2065,6 +2080,7 @@ suite('AutomationsCardsWidget', () => { runCount: automationService.runs.get().length, }, { dialogOptions: { + commit: automationDialogService.lastOptions?.commit, initialValues: { name: 'Daily review Copy', prompt: 'Review all open issues', @@ -2168,7 +2184,7 @@ suite('AutomationsCardsWidget', () => { }); }); - test('duplicate creation failures are logged and reported to the user', async () => { + test('duplicate creation failures stay in the automation dialog', async () => { const { automationDialogService, automationService, contextKeyService, contextMenuService, dialogService, instantiationService, logService, widget } = setup(); const source = automation(); const error = new Error('create failed'); @@ -2198,20 +2214,16 @@ suite('AutomationsCardsWidget', () => { const command = CommandsRegistry.getCommand('sessions.automations.duplicate'); assert.ok(command); await instantiationService.invokeFunction(accessor => command.handler(accessor, source)); - await dialogService.errorCalled.p; + await automationDialogService.commitFailed.p; assert.deepStrictEqual({ loggedErrors: logService.errors, dialogErrors: dialogService.errors, + inlineErrors: automationDialogService.commitErrors, }, { - loggedErrors: [{ - message: '[Automations] Failed to duplicate automation', - args: [error], - }], - dialogErrors: [{ - message: 'Failed to duplicate automation.', - detail: 'create failed', - }], + loggedErrors: [], + dialogErrors: [], + inlineErrors: ['create failed'], }); }); @@ -2886,7 +2898,7 @@ suite('AutomationsCardsWidget', () => { assert.ok(!actionIds.includes('sessions.automations.deleteRunSession'), 'delete absent from context menu'); }); - test('edit conflict is reported to the user', async () => { + test('edit conflict stays in the automation dialog', async () => { const { automationDialogService, automationService, dialogService, widget } = setup(); const item = automation(); automationService.setAutomations([item]); @@ -2894,12 +2906,15 @@ suite('AutomationsCardsWidget', () => { automationDialogService.result = { kind: 'update', id: item.id, value: { name: 'Edited' } }; widget.element.querySelector('.automations-card-main')?.click(); - await dialogService.errorCalled.p; + await automationDialogService.commitFailed.p; - assert.deepStrictEqual(dialogService.errors, [{ - message: 'Failed to update automation.', - detail: 'This automation changed while the dialog was open. Reopen it to review the latest values.', - }]); + assert.deepStrictEqual({ + dialogErrors: dialogService.errors, + inlineErrors: automationDialogService.commitErrors, + }, { + dialogErrors: [], + inlineErrors: ['This automation changed while the dialog was open. Reopen it to review the latest values.'], + }); }); test('edit dialog failures are logged and reported to the user', async () => { @@ -2949,13 +2964,15 @@ suite('AutomationsCardsWidget', () => { automationDialogService.beforeReturn = () => configurationService.setUserConfiguration('chat.automations.enabled', false); widget.element.querySelector('.automations-card-main')?.click(); - await dialogService.infoCalled.p; + await automationDialogService.commitFailed.p; assert.deepStrictEqual({ info: dialogService.infos, + inlineErrors: automationDialogService.commitErrors, updateCalls: automationService.updateCalls, }, { - info: ['Automations are disabled.'], + info: [], + inlineErrors: ['Automations were disabled before the change could be saved.'], updateCalls: 0, }); }); diff --git a/src/vs/workbench/contrib/chat/common/automations/automationDialogService.ts b/src/vs/workbench/contrib/chat/common/automations/automationDialogService.ts index 02eb48ab6df112..9b89899f0c2143 100644 --- a/src/vs/workbench/contrib/chat/common/automations/automationDialogService.ts +++ b/src/vs/workbench/contrib/chat/common/automations/automationDialogService.ts @@ -9,9 +9,17 @@ import { ICreateAutomationOptions, IUpdateAutomationOptions } from './automation export type AutomationDialogCreateInitialValues = Omit & { readonly target?: AutomationTarget }; -export type IShowAutomationDialogOptions = +export type IShowAutomationDialogOptions = ( | { readonly existing: IAutomationDescriptor; readonly initialValues?: never } - | { readonly existing?: never; readonly initialValues?: AutomationDialogCreateInitialValues }; + | { readonly existing?: never; readonly initialValues?: AutomationDialogCreateInitialValues } +) & { + /** + * Persists the result while the dialog stays open and shows progress. A rejection + * is shown in the dialog so the user can retry or cancel; the dialog resolves + * only after `commit` succeeds. + */ + readonly commit?: (result: IAutomationDialogResult) => Promise; +}; export type IAutomationDialogResult = | { readonly kind: 'create'; readonly value: ICreateAutomationOptions } diff --git a/src/vs/workbench/contrib/chat/common/automations/automationService.ts b/src/vs/workbench/contrib/chat/common/automations/automationService.ts index 2399a7b46d3a13..d8982b396607d4 100644 --- a/src/vs/workbench/contrib/chat/common/automations/automationService.ts +++ b/src/vs/workbench/contrib/chat/common/automations/automationService.ts @@ -79,6 +79,25 @@ export interface ICreateAutomationOptions { /** @deprecated Compatibility input translated into {@link sessionTemplate}. */ readonly permissionLevel?: string; readonly enabled?: boolean; + /** + * Ids of the {@link IAutomationCustomizationChoice | customizations} to sync. + * Absent means the provider's default: every customization enabled for the target. + */ + readonly customizationIds?: readonly string[]; +} + +/** + * A customization an automation can sync, as offered by + * {@link IAutomationStore.getCustomizationChoices}. + */ +export interface IAutomationCustomizationChoice { + readonly id: string; + readonly label: string; + readonly description?: string; + /** Initially selected: enabled for the target when creating, saved in the definition when editing. */ + readonly selected: boolean; + /** Saved in the definition but changed locally since; saving the automation refreshes it. */ + readonly outdated: boolean; } /** @@ -98,6 +117,11 @@ export interface IUpdateAutomationOptions { /** @deprecated Compatibility input translated into {@link sessionTemplate}. */ readonly permissionLevel?: string | null; readonly enabled?: boolean; + /** + * Ids of the customizations to sync. Selected entries that are outdated are refreshed. + * Absent keeps the saved customizations unless the target changes. + */ + readonly customizationIds?: readonly string[]; } /** @@ -191,6 +215,12 @@ export interface IAutomationStore { updateAutomationIfUnchanged(id: string, patch: IUpdateAutomationOptions, expected: IAutomationDescriptor, mutationGuard?: AutomationMutationGuard): Promise; /** Deletes an automation and its retained run history; missing IDs are ignored. */ deleteAutomation(id: string, mutationGuard?: AutomationMutationGuard): Promise; + /** + * Lists the customizations an automation targeting `target` can sync, including + * those saved in automation `existingId`. Resolves `undefined` when the provider + * does not support choosing customizations for that target. + */ + getCustomizationChoices?(target: AutomationTarget, existingId: string | undefined, token: CancellationToken): Promise; /** Requests a manual run, forwarding supported cancellation after admission even while session creation is pending. */ runAutomation(automationId: string, token?: CancellationToken): Promise; From 650726a5e490cb3c951252dfdf236149de661a02 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Thu, 1 Oct 2026 14:07:41 -0700 Subject: [PATCH 2/3] automations: address review feedback on customization selection - Uses a host-controlled allowlist to decide which file plugins the agent host uses in place, instead of trusting the client ID. A client cannot make the host load an arbitrary host path. - Restores the MessagePort transport check for local agent host clients. - Waits for the customization choices to load before the dialog saves, so the save always includes the selection. - Reloads the customization choices when the workspace isolation changes. (Commit message generated by Copilot) --- .../agentHost/common/agentPluginManager.ts | 9 ++++++- .../node/agentHostAutomationService.ts | 23 +++++++++++++++-- .../agentHost/node/agentPluginManager.ts | 13 ++++++++-- .../agentHost/node/protocolServerHandler.ts | 2 +- .../node/agentHostAutomationService.test.ts | 4 ++- .../test/node/agentPluginManager.test.ts | 24 ++++++++---------- .../agentHost/test/node/claudeAgent.test.ts | 1 + .../agentHost/test/node/copilotAgent.test.ts | 1 + .../automations/browser/automationDialog.ts | 12 ++++++++- .../browser/automationDialogService.ts | 3 +++ .../test/browser/automationDialog.test.ts | 25 +++++++++++++++++++ 11 files changed, 96 insertions(+), 21 deletions(-) diff --git a/src/vs/platform/agentHost/common/agentPluginManager.ts b/src/vs/platform/agentHost/common/agentPluginManager.ts index 2a49cb1d7b2e12..8cb6d656cce02a 100644 --- a/src/vs/platform/agentHost/common/agentPluginManager.ts +++ b/src/vs/platform/agentHost/common/agentPluginManager.ts @@ -40,9 +40,16 @@ export interface IAgentPluginManager { */ readonly basePath: URI; - /** Immutable host-owned plugin directories. File URIs under this path, or synced for AUTOMATION_ACTIVE_CLIENT_ID, are used in place without cache entries. */ + /** Immutable host-owned plugin directories. File URIs under this path are used in place without cache entries. */ readonly hostPluginsPath: URI; + /** + * Marks a host `file:` plugin directory as trusted, so customizations with + * exactly this URI are used in place regardless of which client syncs them. + * Only host-side callers that resolved the path themselves may call this. + */ + trustHostPluginDirectory(uri: URI): void; + /** * Syncs a set of client-provided plugin customizations to local storage. * diff --git a/src/vs/platform/agentHost/node/agentHostAutomationService.ts b/src/vs/platform/agentHost/node/agentHostAutomationService.ts index 3971e7f07d63fe..5af9a3191e1bbe 100644 --- a/src/vs/platform/agentHost/node/agentHostAutomationService.ts +++ b/src/vs/platform/agentHost/node/agentHostAutomationService.ts @@ -8,6 +8,7 @@ import { disposableTimeout } from '../../../base/common/async.js'; import { Disposable, DisposableMap, MutableDisposable, toDisposable } from '../../../base/common/lifecycle.js'; import { equals } from '../../../base/common/objects.js'; import { autorun, type IReader } from '../../../base/common/observable.js'; +import { Schemas } from '../../../base/common/network.js'; import { URI } from '../../../base/common/uri.js'; import { generateUuid } from '../../../base/common/uuid.js'; import { localize } from '../../../nls.js'; @@ -117,13 +118,13 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost @ILogService private readonly _logService: ILogService, @ITelemetryService private readonly _telemetryService: ITelemetryService, @IAgentHostProviderService private readonly _providerService: IAgentHostProviderService, - @IAgentPluginManager pluginManager: IAgentPluginManager, + @IAgentPluginManager private readonly _pluginManager: IAgentPluginManager, @IFileService fileService: IFileService, @INativeEnvironmentService environmentService: INativeEnvironmentService, @IAgentHostClientConnectionService private readonly _clientConnections: IAgentHostClientConnectionService, ) { super(); - this._customizations = new AgentHostAutomationCustomizations(pluginManager.hostPluginsPath, fileService, this._logService, environmentService.userHome); + this._customizations = new AgentHostAutomationCustomizations(_pluginManager.hostPluginsPath, fileService, this._logService, environmentService.userHome); this._register(toDisposable(() => this._cancellations.clear())); this._register(toDisposable(() => this._mcpAuthenticationChallenges.clear())); const stored = this._load(); @@ -137,6 +138,7 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost } : undefined; this._manualRunRequests = new Map(stored?.manualRunRequests?.map(request => [request.requestId, request])); if (this._catalog) { + this._trustCapturedCustomizations(this._catalog.entries); this._stateManager.setAutomationCatalogState(this._catalog); } for (const run of this._runs.values()) { @@ -231,6 +233,7 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost const timestamp = new Date().toISOString(); const customizations = await this._customizations.capture(clientId, definition.session.customizations, undefined, clientId !== undefined && this._clientConnections.isLocalClient(clientId)); + this._trustCapturedCustomizations([{ customizations }]); const automation = this._withInitialScheduleState({ resource: action.resource, definition, @@ -252,6 +255,21 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost this._scheduleNext(); } + /** + * Lets runs use captured local `file:` plugins in place. These paths come + * only from host captures, so a client can never nominate a host path itself. + */ + private _trustCapturedCustomizations(entries: readonly Pick[]): void { + for (const entry of entries) { + for (const customization of entry.customizations ?? []) { + const uri = URI.parse(customization.uri); + if (uri.scheme === Schemas.file) { + this._pluginManager.trustHostPluginDirectory(uri); + } + } + } + } + async handleUpdate(action: AutomationUpdateRequestedAction, clientId?: string): Promise { return this._enqueueMutation(() => this._collectingCustomizations(() => this._handleUpdate(action, clientId))); } @@ -275,6 +293,7 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost this._validateDefinition(automation.definition); if (action.changes.session !== undefined) { automation.customizations = await this._customizations.capture(clientId, action.changes.session.customizations, existing, clientId !== undefined && this._clientConnections.isLocalClient(clientId)); + this._trustCapturedCustomizations([automation]); } if (action.changes.triggers !== undefined || action.changes.enabled !== undefined) { automation = this._withInitialScheduleState(automation, new Date()); diff --git a/src/vs/platform/agentHost/node/agentPluginManager.ts b/src/vs/platform/agentHost/node/agentPluginManager.ts index d481331a0533aa..0981d9c33c3dae 100644 --- a/src/vs/platform/agentHost/node/agentPluginManager.ts +++ b/src/vs/platform/agentHost/node/agentPluginManager.ts @@ -8,9 +8,10 @@ import { SequencerByKey } from '../../../base/common/async.js'; import { URI } from '../../../base/common/uri.js'; import { Schemas } from '../../../base/common/network.js'; import { extUriBiasedIgnorePathCase } from '../../../base/common/resources.js'; +import { ResourceSet } from '../../../base/common/map.js'; import { FileOperationResult, IFileService, toFileOperationResult } from '../../files/common/files.js'; import { ILogService } from '../../log/common/log.js'; -import { AUTOMATION_ACTIVE_CLIENT_ID, IAgentPluginManager, type ISyncedCustomization } from '../common/agentPluginManager.js'; +import { IAgentPluginManager, type ISyncedCustomization } from '../common/agentPluginManager.js'; import { CustomizationLoadStatus, type ClientPluginCustomization, type PluginCustomization } from '../common/state/sessionState.js'; import { toAgentClientUri } from '../common/agentClientUri.js'; @@ -83,6 +84,9 @@ export class AgentPluginManager implements IAgentPluginManager { private _cacheLoadPromise: Promise | undefined; + /** Host plugin directories registered through {@link trustHostPluginDirectory}. */ + private readonly _trustedHostPluginDirectories = new ResourceSet(uri => extUriBiasedIgnorePathCase.getComparisonKey(uri)); + constructor( userDataPath: URI, @IFileService private readonly _fileService: IFileService, @@ -98,6 +102,10 @@ export class AgentPluginManager implements IAgentPluginManager { return this._basePath; } + trustHostPluginDirectory(uri: URI): void { + this._trustedHostPluginDirectories.add(extUriBiasedIgnorePathCase.normalizePath(uri)); + } + get hostPluginsPath(): URI { return URI.joinPath(this.basePath, '.host'); } @@ -142,7 +150,8 @@ export class AgentPluginManager implements IAgentPluginManager { private async _syncPlugin(clientId: string, ref: ClientPluginCustomization): Promise { const uri = URI.parse(ref.uri); // Normalize so `..` segments cannot escape the host-owned directory. - if (uri.scheme === Schemas.file && (clientId === AUTOMATION_ACTIVE_CLIENT_ID || extUriBiasedIgnorePathCase.isEqualOrParent(extUriBiasedIgnorePathCase.normalizePath(uri), this.hostPluginsPath))) { + const normalized = extUriBiasedIgnorePathCase.normalizePath(uri); + if (uri.scheme === Schemas.file && (this._trustedHostPluginDirectories.has(normalized) || extUriBiasedIgnorePathCase.isEqualOrParent(normalized, this.hostPluginsPath))) { return uri; } const pluginUri = toAgentClientUri(uri, clientId); diff --git a/src/vs/platform/agentHost/node/protocolServerHandler.ts b/src/vs/platform/agentHost/node/protocolServerHandler.ts index d3c933f9903244..68af61b6a661d0 100644 --- a/src/vs/platform/agentHost/node/protocolServerHandler.ts +++ b/src/vs/platform/agentHost/node/protocolServerHandler.ts @@ -1393,7 +1393,7 @@ export class ProtocolServerHandler extends Disposable implements IAgentHostClien isLocalClient(clientId: string): boolean { const record = this._clients.get(clientId); return record?.state === 'active' - && record.connections.some(connection => connection.telemetryContext.connectionKind === AgentHostClientConnectionKind.Local); + && record.connections.some(connection => connection.telemetryConnectionActive && this._supportsCanvases(connection)); } getConnectedClientTransportCounts(): ReadonlyMap { diff --git a/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts b/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts index 396b5c48c8de78..5cb14056be0616 100644 --- a/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts @@ -282,13 +282,15 @@ suite('AgentHostAutomationService', () => { changes: { session: { ...action.definition.session, customizations: [updatedRef] } }, }, 'local'); const updated = stateManager.getAutomationCatalogState()!.entries[0]; + const [runSync] = await pluginManager.syncCustomizations(AUTOMATION_ACTIVE_CLIENT_ID, [{ ...updatedRef, clientId: AUTOMATION_ACTIVE_CLIENT_ID }]); assert.deepStrictEqual({ createdUris: created?.map(copy => copy.uri), updatedUris: updated.customizations?.map(copy => copy.uri), refs: updated.definition.session.customizations, copiesExist: await fileService.exists(URI.joinPath(pluginManager.hostPluginsPath, 'automations')), + runPluginDir: runSync.pluginDir?.toString(), }, { - createdUris: [ref.uri], updatedUris: [ref.uri], refs: [updatedRef], copiesExist: false, + createdUris: [ref.uri], updatedUris: [ref.uri], refs: [updatedRef], copiesExist: false, runPluginDir: ref.uri, }); }); diff --git a/src/vs/platform/agentHost/test/node/agentPluginManager.test.ts b/src/vs/platform/agentHost/test/node/agentPluginManager.test.ts index 44023310ec9677..2d44994a26353c 100644 --- a/src/vs/platform/agentHost/test/node/agentPluginManager.test.ts +++ b/src/vs/platform/agentHost/test/node/agentPluginManager.test.ts @@ -111,25 +111,23 @@ suite('AgentPluginManager', () => { suite('syncCustomizations', () => { - test('uses file plugins in place only for the automation active client outside host directories', async () => { + test('uses file plugins in place only when the host trusts their directory', async () => { disposables.add(fileService.registerProvider(Schemas.file, disposables.add(new InMemoryFileSystemProvider()))); const directory = URI.file('/local/bundle'); const ref = { ...makeRef('local', 'revision'), uri: directory.toString() }; await fileService.writeFile(URI.joinPath(directory, 'index.js'), VSBuffer.fromString('original')); - await fileService.writeFile(URI.joinPath(toAgentClientUri(directory, 'test-client'), 'index.js'), VSBuffer.fromString('client-served')); - const [inPlace] = await manager.syncCustomizations(AUTOMATION_ACTIVE_CLIENT_ID, [ref]); - const cacheExistsBeforeCopy = await fileService.exists(URI.joinPath(manager.basePath, 'cache.json')); - const [copied] = await manager.syncCustomizations('test-client', [ref]); + await fileService.writeFile(URI.joinPath(toAgentClientUri(directory, AUTOMATION_ACTIVE_CLIENT_ID), 'index.js'), VSBuffer.fromString('client-served')); + const [untrusted] = await manager.syncCustomizations(AUTOMATION_ACTIVE_CLIENT_ID, [ref]); + manager.trustHostPluginDirectory(directory); + const [trusted] = await manager.syncCustomizations('any-client', [ref]); assert.deepStrictEqual({ - inPlace: inPlace.pluginDir?.toString(), - inPlaceLoad: inPlace.customization.load, - cacheExistsBeforeCopy, - copied: copied.pluginDir?.toString() !== directory.toString(), - copiedLoad: copied.customization.load, - content: (await fileService.readFile(URI.joinPath(copied.pluginDir!, 'index.js'))).value.toString(), + untrustedCopied: untrusted.pluginDir?.toString() !== directory.toString(), + untrustedContent: (await fileService.readFile(URI.joinPath(untrusted.pluginDir!, 'index.js'))).value.toString(), + trusted: trusted.pluginDir?.toString(), + trustedLoad: trusted.customization.load, }, { - inPlace: directory.toString(), inPlaceLoad: { kind: 'loaded' }, cacheExistsBeforeCopy: false, - copied: true, copiedLoad: { kind: 'loaded' }, content: 'client-served', + untrustedCopied: true, untrustedContent: 'client-served', + trusted: directory.toString(), trustedLoad: { kind: 'loaded' }, }); }); diff --git a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts index 785af526d921f5..f2be13a416eda6 100644 --- a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts @@ -313,6 +313,7 @@ class FakeAgentPluginManager implements IAgentPluginManager { declare readonly _serviceBrand: undefined; readonly basePath = URI.from({ scheme: 'inmemory', path: '/agentPlugins' }); readonly hostPluginsPath = URI.joinPath(this.basePath, '.host'); + trustHostPluginDirectory(): void { } syncResult: readonly ISyncedCustomization[] | undefined; syncCalls: { clientId: string; customizations: readonly ClientPluginCustomization[] }[] = []; diff --git a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts index 4ade18b32adeb4..1b37194c01c77f 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts @@ -285,6 +285,7 @@ class TestAgentPluginManager implements IAgentPluginManager { readonly basePath = URI.from({ scheme: 'inmemory', path: '/agentPlugins' }); readonly hostPluginsPath = URI.joinPath(this.basePath, '.host'); + trustHostPluginDirectory(): void { } async syncCustomizations(_clientId: string, _customizations: ClientPluginCustomization[], _progress?: (status: PluginCustomization) => void): Promise { return []; diff --git a/src/vs/sessions/contrib/automations/browser/automationDialog.ts b/src/vs/sessions/contrib/automations/browser/automationDialog.ts index 6724f4d245ba75..27895b66f790f6 100644 --- a/src/vs/sessions/contrib/automations/browser/automationDialog.ts +++ b/src/vs/sessions/contrib/automations/browser/automationDialog.ts @@ -248,6 +248,7 @@ interface IRenderFormHandle { readonly waitForAutomationSessionSync: (token: CancellationToken) => Promise; readonly setSaving: (saving: boolean, committing?: boolean) => void; readonly getCustomizationIds: () => readonly string[] | undefined; + readonly waitForCustomizationChoices: (token: CancellationToken) => Promise; readonly showTargetValidationError: (message: string | undefined) => void; readonly showSaveError: (message: string | undefined) => void; readonly focusSaveError: () => void; @@ -1497,6 +1498,7 @@ export function renderForm( getSessionConfiguration: token => automationSessionDraftSynchronizer.getSessionConfiguration(token), getBranch: () => isolationModel.persistedBranch, getCustomizationIds: () => customizationSelection?.getSelectedIds(), + waitForCustomizationChoices: async token => customizationSelection?.waitForChoices(token), showTargetValidationError: message => { const text = message ?? ''; if (targetError.textContent === text) { @@ -1574,6 +1576,7 @@ export class AutomationCustomizationSelection extends Disposable { private readonly request = this._register(new MutableDisposable()); private readonly toggles = new Map(); private choices: readonly IAutomationCustomizationChoice[] | undefined; + private loading: Promise = Promise.resolve(); private targetKey: string | undefined; private expanded = false; @@ -1611,12 +1614,19 @@ export class AutomationCustomizationSelection extends Disposable { return this.choices?.filter(choice => this.toggles.get(choice.id) ?? choice.selected).map(choice => choice.id); } + /** Waits for the choices of the current target to load, so a save never omits the selection. */ + async waitForChoices(token: CancellationToken): Promise { + await raceCancellationError(this.loading, token); + } + updateTarget(target: AutomationTarget | undefined): void { const key = target ? JSON.stringify({ kind: target.kind, providerId: target.providerId, sessionTypeId: target.sessionTypeId, folder: target.kind === 'workspace' ? getComparisonKey(target.folderUri) : undefined, + // Isolation changes count as a new target for the saved selection, as in the provider. + isolation: target.kind === 'workspace' ? target.isolation : undefined, }) : undefined; if (key === this.targetKey) { return; @@ -1640,7 +1650,7 @@ export class AutomationCustomizationSelection extends Disposable { DOM.append(loading, $('span', undefined, localize('automation.customizations.loading', "Loading customizations…"))); const request = new CancellationTokenSource(); this.request.value = request; - void this.load(target, request); + this.loading = this.load(target, request); } private async load(target: AutomationTarget, request: CancellationTokenSource): Promise { diff --git a/src/vs/sessions/contrib/automations/browser/automationDialogService.ts b/src/vs/sessions/contrib/automations/browser/automationDialogService.ts index fd7bba81f8312f..95dcbeb2470bde 100644 --- a/src/vs/sessions/contrib/automations/browser/automationDialogService.ts +++ b/src/vs/sessions/contrib/automations/browser/automationDialogService.ts @@ -132,6 +132,7 @@ export class AutomationDialogService implements IAutomationDialogService { let waitForAutomationSessionSync: (token: CancellationToken) => Promise = async () => { }; let setSaving: (saving: boolean, committing?: boolean) => void = () => { }; let getCustomizationIds: () => readonly string[] | undefined = () => undefined; + let waitForCustomizationChoices: (token: CancellationToken) => Promise = async () => { }; let progressBar: ProgressBar | undefined; let dialogElement: HTMLElement | undefined; let closeToolbar: HTMLElement | undefined; @@ -239,6 +240,7 @@ export class AutomationDialogService implements IAutomationDialogService { let shouldFocusError = false; try { await waitForAutomationSessionSync(cancellation.token); + await waitForCustomizationChoices(cancellation.token); const sessionConfigurationCapture = await getSessionConfiguration(cancellation.token); if (sessionConfigurationCapture.kind === 'failed') { dialogTelemetry.captureFailed(); @@ -360,6 +362,7 @@ export class AutomationDialogService implements IAutomationDialogService { waitForAutomationSessionSync = handle.waitForAutomationSessionSync; setSaving = handle.setSaving; getCustomizationIds = handle.getCustomizationIds; + waitForCustomizationChoices = handle.waitForCustomizationChoices; showSaveError = handle.showSaveError; focusSaveError = handle.focusSaveError; getFocusableElements = handle.getFocusableElements; diff --git a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts index d4a67d6884e6a0..102a2068b523e7 100644 --- a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts +++ b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts @@ -10,6 +10,7 @@ import { Button } from '../../../../../base/browser/ui/button/button.js'; import { Dialog } from '../../../../../base/browser/ui/dialog/dialog.js'; import { SelectBox } from '../../../../../base/browser/ui/selectBox/selectBox.js'; import { DeferredPromise, timeout } from '../../../../../base/common/async.js'; +import { CancellationToken } from '../../../../../base/common/cancellation.js'; import { StandardMouseEvent } from '../../../../../base/browser/mouseEvent.js'; import { Codicon } from '../../../../../base/common/codicons.js'; import { Action, IAction } from '../../../../../base/common/actions.js'; @@ -437,6 +438,30 @@ suite('Automation dialog creation', () => { summary: '1 of 3 customizations · 1 outdated', }); }); + + test('reloads on isolation changes and waits for a pending load before reporting the selection', async () => { + const requests: DeferredPromise[] = []; + const selection = disposables.add(new AutomationCustomizationSelection( + DOM.$('div'), + upcastPartial({ + getCustomizationChoices: async () => { + const result = new DeferredPromise(); + requests.push(result); + return result.p; + }, + }), + upcastPartial({ setupDelayedHover: () => toDisposable(() => { }) }), + new NullLogService(), + )); + selection.updateTarget({ kind: 'workspace', folderUri: FOLDER, providerId: 'host', sessionTypeId: 'one', isolation: { kind: 'folder' } }); + selection.updateTarget({ kind: 'workspace', folderUri: FOLDER, providerId: 'host', sessionTypeId: 'one', isolation: { kind: 'worktree', branch: 'main' } }); + const beforeLoad = selection.getSelectedIds(); + const waited = selection.waitForChoices(CancellationToken.None).then(() => selection.getSelectedIds()); + void requests[1].complete([{ id: 'plugin', label: 'Plugin', selected: true, outdated: false }]); + assert.deepStrictEqual({ requests: requests.length, beforeLoad, afterLoad: await waited }, { + requests: 2, beforeLoad: undefined, afterLoad: ['plugin'], + }); + }); }); test('preserves a supplied quick-chat target and custom name', async () => { From 1074ad742b904000e36e5a8c2a9c94d70bdc0ee8 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Thu, 1 Oct 2026 14:18:11 -0700 Subject: [PATCH 3/3] agentHost: mark host-local automation plugins with a dedicated scheme Replaces the allowlist of trusted host plugin directories with a URI scheme. A `file:` plugin URI now always names a client resource, so a host path cannot be confused with a client path. - Adds the `vscode-agent-host-file:` scheme for plugin URIs that name a directory on the agent host's own disk. The plugin manager uses these directories in place. - Gives automation run sessions their captured and local in-place plugins with this scheme. - Removes trustHostPluginDirectory and the in-place rule for paths under hostPluginsPath. (Commit message generated by Copilot) --- .../agentHost/common/agentPluginManager.ts | 23 +++++--- .../node/agentHostAutomationCustomizations.ts | 5 +- .../node/agentHostAutomationService.ts | 23 +------- .../agentHost/node/agentPluginManager.ts | 17 ++---- .../agentHostAutomationCustomizations.test.ts | 4 +- .../node/agentHostAutomationService.test.ts | 6 +-- .../test/node/agentPluginManager.test.ts | 52 +++++++------------ .../agentHost/test/node/claudeAgent.test.ts | 1 - .../agentHost/test/node/copilotAgent.test.ts | 1 - src/vs/sessions/AUTOMATIONS.md | 2 +- 10 files changed, 47 insertions(+), 87 deletions(-) diff --git a/src/vs/platform/agentHost/common/agentPluginManager.ts b/src/vs/platform/agentHost/common/agentPluginManager.ts index 8cb6d656cce02a..5f85af165c96d8 100644 --- a/src/vs/platform/agentHost/common/agentPluginManager.ts +++ b/src/vs/platform/agentHost/common/agentPluginManager.ts @@ -12,6 +12,18 @@ export const IAgentPluginManager = createDecorator('agentPl /** Static active-client identity used for host-resolved automation plugins. */ export const AUTOMATION_ACTIVE_CLIENT_ID = 'vscode.automation'; +/** + * Scheme for plugin customization URIs that name a directory on the agent + * host's own disk. Every other URI, including `file:`, names a resource on the + * client that published the customization. + */ +export const AGENT_HOST_FILE_SCHEME = 'vscode-agent-host-file'; + +/** Marks a `file:` URI as a path on the agent host's own disk. */ +export function toAgentHostFileUri(uri: URI): URI { + return uri.with({ scheme: AGENT_HOST_FILE_SCHEME }); +} + /** * A synced customization with its local plugin directory (when available). */ @@ -40,18 +52,13 @@ export interface IAgentPluginManager { */ readonly basePath: URI; - /** Immutable host-owned plugin directories. File URIs under this path are used in place without cache entries. */ + /** Directory for immutable host-owned plugin copies, such as automation captures. */ readonly hostPluginsPath: URI; - /** - * Marks a host `file:` plugin directory as trusted, so customizations with - * exactly this URI are used in place regardless of which client syncs them. - * Only host-side callers that resolved the path themselves may call this. - */ - trustHostPluginDirectory(uri: URI): void; - /** * Syncs a set of client-provided plugin customizations to local storage. + * Customizations with an {@link AGENT_HOST_FILE_SCHEME} URI are already on + * the host's disk and are used in place without a copy or cache entry. * * Each plugin is copied to a local directory, respecting nonce-based * caching. The optional {@link progress} callback fires with the single diff --git a/src/vs/platform/agentHost/node/agentHostAutomationCustomizations.ts b/src/vs/platform/agentHost/node/agentHostAutomationCustomizations.ts index af83ea22e89786..96073737132bad 100644 --- a/src/vs/platform/agentHost/node/agentHostAutomationCustomizations.ts +++ b/src/vs/platform/agentHost/node/agentHostAutomationCustomizations.ts @@ -13,7 +13,7 @@ import { parsePlugin } from '../../agentPlugins/common/pluginParsers.js'; import { IFileService } from '../../files/common/files.js'; import { ILogService } from '../../log/common/log.js'; import { toAgentClientUri } from '../common/agentClientUri.js'; -import { AUTOMATION_ACTIVE_CLIENT_ID } from '../common/agentPluginManager.js'; +import { AUTOMATION_ACTIVE_CLIENT_ID, toAgentHostFileUri } from '../common/agentPluginManager.js'; import type { AutomationEntry, AutomationSessionTemplate } from '../common/state/protocol/channels-automation/state.js'; import { CustomizationLoadStatus, CustomizationType, type AgentSelection, type ClientPluginCustomization, type PluginCustomization, type SessionActiveClient } from '../common/state/sessionState.js'; import { toChildCustomizations } from './copilot/copilotPluginConverters.js'; @@ -97,7 +97,8 @@ export class AgentHostAutomationCustomizations { throw new Error(`Missing captured automation customization: ${ref.id}`); } this._usedByRuns.add(copy.uri); - return { ...ref, uri: copy.uri, clientId: AUTOMATION_ACTIVE_CLIENT_ID }; + // Captured copies and local in-place plugins are host paths, not client resources. + return { ...ref, uri: toAgentHostFileUri(URI.parse(copy.uri)).toString(), clientId: AUTOMATION_ACTIVE_CLIENT_ID }; }); return customizations?.length ? { clientId: AUTOMATION_ACTIVE_CLIENT_ID, diff --git a/src/vs/platform/agentHost/node/agentHostAutomationService.ts b/src/vs/platform/agentHost/node/agentHostAutomationService.ts index 5af9a3191e1bbe..3971e7f07d63fe 100644 --- a/src/vs/platform/agentHost/node/agentHostAutomationService.ts +++ b/src/vs/platform/agentHost/node/agentHostAutomationService.ts @@ -8,7 +8,6 @@ import { disposableTimeout } from '../../../base/common/async.js'; import { Disposable, DisposableMap, MutableDisposable, toDisposable } from '../../../base/common/lifecycle.js'; import { equals } from '../../../base/common/objects.js'; import { autorun, type IReader } from '../../../base/common/observable.js'; -import { Schemas } from '../../../base/common/network.js'; import { URI } from '../../../base/common/uri.js'; import { generateUuid } from '../../../base/common/uuid.js'; import { localize } from '../../../nls.js'; @@ -118,13 +117,13 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost @ILogService private readonly _logService: ILogService, @ITelemetryService private readonly _telemetryService: ITelemetryService, @IAgentHostProviderService private readonly _providerService: IAgentHostProviderService, - @IAgentPluginManager private readonly _pluginManager: IAgentPluginManager, + @IAgentPluginManager pluginManager: IAgentPluginManager, @IFileService fileService: IFileService, @INativeEnvironmentService environmentService: INativeEnvironmentService, @IAgentHostClientConnectionService private readonly _clientConnections: IAgentHostClientConnectionService, ) { super(); - this._customizations = new AgentHostAutomationCustomizations(_pluginManager.hostPluginsPath, fileService, this._logService, environmentService.userHome); + this._customizations = new AgentHostAutomationCustomizations(pluginManager.hostPluginsPath, fileService, this._logService, environmentService.userHome); this._register(toDisposable(() => this._cancellations.clear())); this._register(toDisposable(() => this._mcpAuthenticationChallenges.clear())); const stored = this._load(); @@ -138,7 +137,6 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost } : undefined; this._manualRunRequests = new Map(stored?.manualRunRequests?.map(request => [request.requestId, request])); if (this._catalog) { - this._trustCapturedCustomizations(this._catalog.entries); this._stateManager.setAutomationCatalogState(this._catalog); } for (const run of this._runs.values()) { @@ -233,7 +231,6 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost const timestamp = new Date().toISOString(); const customizations = await this._customizations.capture(clientId, definition.session.customizations, undefined, clientId !== undefined && this._clientConnections.isLocalClient(clientId)); - this._trustCapturedCustomizations([{ customizations }]); const automation = this._withInitialScheduleState({ resource: action.resource, definition, @@ -255,21 +252,6 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost this._scheduleNext(); } - /** - * Lets runs use captured local `file:` plugins in place. These paths come - * only from host captures, so a client can never nominate a host path itself. - */ - private _trustCapturedCustomizations(entries: readonly Pick[]): void { - for (const entry of entries) { - for (const customization of entry.customizations ?? []) { - const uri = URI.parse(customization.uri); - if (uri.scheme === Schemas.file) { - this._pluginManager.trustHostPluginDirectory(uri); - } - } - } - } - async handleUpdate(action: AutomationUpdateRequestedAction, clientId?: string): Promise { return this._enqueueMutation(() => this._collectingCustomizations(() => this._handleUpdate(action, clientId))); } @@ -293,7 +275,6 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost this._validateDefinition(automation.definition); if (action.changes.session !== undefined) { automation.customizations = await this._customizations.capture(clientId, action.changes.session.customizations, existing, clientId !== undefined && this._clientConnections.isLocalClient(clientId)); - this._trustCapturedCustomizations([automation]); } if (action.changes.triggers !== undefined || action.changes.enabled !== undefined) { automation = this._withInitialScheduleState(automation, new Date()); diff --git a/src/vs/platform/agentHost/node/agentPluginManager.ts b/src/vs/platform/agentHost/node/agentPluginManager.ts index 0981d9c33c3dae..d03dfaf95a0cb8 100644 --- a/src/vs/platform/agentHost/node/agentPluginManager.ts +++ b/src/vs/platform/agentHost/node/agentPluginManager.ts @@ -7,11 +7,9 @@ import { VSBuffer } from '../../../base/common/buffer.js'; import { SequencerByKey } from '../../../base/common/async.js'; import { URI } from '../../../base/common/uri.js'; import { Schemas } from '../../../base/common/network.js'; -import { extUriBiasedIgnorePathCase } from '../../../base/common/resources.js'; -import { ResourceSet } from '../../../base/common/map.js'; import { FileOperationResult, IFileService, toFileOperationResult } from '../../files/common/files.js'; import { ILogService } from '../../log/common/log.js'; -import { IAgentPluginManager, type ISyncedCustomization } from '../common/agentPluginManager.js'; +import { AGENT_HOST_FILE_SCHEME, IAgentPluginManager, type ISyncedCustomization } from '../common/agentPluginManager.js'; import { CustomizationLoadStatus, type ClientPluginCustomization, type PluginCustomization } from '../common/state/sessionState.js'; import { toAgentClientUri } from '../common/agentClientUri.js'; @@ -84,9 +82,6 @@ export class AgentPluginManager implements IAgentPluginManager { private _cacheLoadPromise: Promise | undefined; - /** Host plugin directories registered through {@link trustHostPluginDirectory}. */ - private readonly _trustedHostPluginDirectories = new ResourceSet(uri => extUriBiasedIgnorePathCase.getComparisonKey(uri)); - constructor( userDataPath: URI, @IFileService private readonly _fileService: IFileService, @@ -102,10 +97,6 @@ export class AgentPluginManager implements IAgentPluginManager { return this._basePath; } - trustHostPluginDirectory(uri: URI): void { - this._trustedHostPluginDirectories.add(extUriBiasedIgnorePathCase.normalizePath(uri)); - } - get hostPluginsPath(): URI { return URI.joinPath(this.basePath, '.host'); } @@ -149,10 +140,8 @@ export class AgentPluginManager implements IAgentPluginManager { */ private async _syncPlugin(clientId: string, ref: ClientPluginCustomization): Promise { const uri = URI.parse(ref.uri); - // Normalize so `..` segments cannot escape the host-owned directory. - const normalized = extUriBiasedIgnorePathCase.normalizePath(uri); - if (uri.scheme === Schemas.file && (this._trustedHostPluginDirectories.has(normalized) || extUriBiasedIgnorePathCase.isEqualOrParent(normalized, this.hostPluginsPath))) { - return uri; + if (uri.scheme === AGENT_HOST_FILE_SCHEME) { + return uri.with({ scheme: Schemas.file }); } const pluginUri = toAgentClientUri(uri, clientId); const destDir = this._dirFor(ref.uri, ref.nonce); diff --git a/src/vs/platform/agentHost/test/node/agentHostAutomationCustomizations.test.ts b/src/vs/platform/agentHost/test/node/agentHostAutomationCustomizations.test.ts index 307cec25461b55..7d0d67dad502f0 100644 --- a/src/vs/platform/agentHost/test/node/agentHostAutomationCustomizations.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostAutomationCustomizations.test.ts @@ -18,7 +18,7 @@ import { CustomizationType, MessageKind, type ClientPluginCustomization, type Pl import { CustomizationEnablementKind } from '../../common/state/protocol/channels-session/state.js'; import type { AutomationEntry } from '../../common/state/protocol/channels-automation/state.js'; import { AgentHostAutomationCustomizations } from '../../node/agentHostAutomationCustomizations.js'; -import { AUTOMATION_ACTIVE_CLIENT_ID } from '../../common/agentPluginManager.js'; +import { AUTOMATION_ACTIVE_CLIENT_ID, toAgentHostFileUri } from '../../common/agentPluginManager.js'; suite('AgentHostAutomationCustomizations', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); @@ -210,7 +210,7 @@ suite('AgentHostAutomationCustomizations', () => { }, { activeClient: { clientId: AUTOMATION_ACTIVE_CLIENT_ID, displayName: 'Automation', tools: [], - customizations: [{ ...input, uri: copies[0].uri, clientId: AUTOMATION_ACTIVE_CLIENT_ID }], + customizations: [{ ...input, uri: toAgentHostFileUri(URI.parse(copies[0].uri)).toString(), clientId: AUTOMATION_ACTIVE_CLIENT_ID }], }, empty: undefined, }); diff --git a/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts b/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts index 5cb14056be0616..94527f2c5d303c 100644 --- a/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts @@ -39,7 +39,7 @@ import { FileService } from '../../../files/common/fileService.js'; import { InMemoryFileSystemProvider } from '../../../files/common/inMemoryFilesystemProvider.js'; import { AGENT_CLIENT_SCHEME, toAgentClientUri } from '../../common/agentClientUri.js'; import { AgentPluginManager } from '../../node/agentPluginManager.js'; -import { AUTOMATION_ACTIVE_CLIENT_ID } from '../../common/agentPluginManager.js'; +import { AUTOMATION_ACTIVE_CLIENT_ID, toAgentHostFileUri } from '../../common/agentPluginManager.js'; import { AgentHostClientConnectionService } from '../../node/agentHostClientConnectionService.js'; import type { IAgentHostMcpAuthenticationRequest } from '../../common/agentHostExtensionProtocol.js'; import { McpAuthRequiredReason, McpServerStatus, type Customization, type McpAuthRequirement } from '../../common/state/protocol/channels-session/state.js'; @@ -254,7 +254,7 @@ suite('AgentHostAutomationService', () => { copy: { type: CustomizationType.Plugin, id: 'bundle', uri: copy.uri, name: 'Bundle', children: [], load: { kind: 'loaded' }, icons: undefined, range: undefined, version: undefined }, stored: [copy], run: { - activeClient: { clientId: AUTOMATION_ACTIVE_CLIENT_ID, displayName: 'Automation', tools: [], customizations: [{ ...ref, uri: copy.uri, clientId: AUTOMATION_ACTIVE_CLIENT_ID }] }, + activeClient: { clientId: AUTOMATION_ACTIVE_CLIENT_ID, displayName: 'Automation', tools: [], customizations: [{ ...ref, uri: toAgentHostFileUri(URI.parse(copy.uri)).toString(), clientId: AUTOMATION_ACTIVE_CLIENT_ID }] }, agent: URI.joinPath(URI.parse(copy.uri), 'agents/reviewer.md').toString(), }, }); @@ -282,7 +282,7 @@ suite('AgentHostAutomationService', () => { changes: { session: { ...action.definition.session, customizations: [updatedRef] } }, }, 'local'); const updated = stateManager.getAutomationCatalogState()!.entries[0]; - const [runSync] = await pluginManager.syncCustomizations(AUTOMATION_ACTIVE_CLIENT_ID, [{ ...updatedRef, clientId: AUTOMATION_ACTIVE_CLIENT_ID }]); + const [runSync] = await pluginManager.syncCustomizations(AUTOMATION_ACTIVE_CLIENT_ID, [{ ...updatedRef, uri: toAgentHostFileUri(URI.parse(updatedRef.uri)).toString(), clientId: AUTOMATION_ACTIVE_CLIENT_ID }]); assert.deepStrictEqual({ createdUris: created?.map(copy => copy.uri), updatedUris: updated.customizations?.map(copy => copy.uri), diff --git a/src/vs/platform/agentHost/test/node/agentPluginManager.test.ts b/src/vs/platform/agentHost/test/node/agentPluginManager.test.ts index 2d44994a26353c..a927b85cbbc73c 100644 --- a/src/vs/platform/agentHost/test/node/agentPluginManager.test.ts +++ b/src/vs/platform/agentHost/test/node/agentPluginManager.test.ts @@ -15,7 +15,7 @@ import { IFileDeleteOptions } from '../../../files/common/files.js'; import { InMemoryFileSystemProvider } from '../../../files/common/inMemoryFilesystemProvider.js'; import { NullLogService } from '../../../log/common/log.js'; import { AGENT_CLIENT_SCHEME, toAgentClientUri } from '../../common/agentClientUri.js'; -import { AUTOMATION_ACTIVE_CLIENT_ID } from '../../common/agentPluginManager.js'; +import { AUTOMATION_ACTIVE_CLIENT_ID, toAgentHostFileUri } from '../../common/agentPluginManager.js'; import { customizationId, type ClientPluginCustomization, type PluginCustomization } from '../../common/state/sessionState.js'; import { CustomizationType } from '../../common/state/protocol/state.js'; import { AgentPluginManager } from '../../node/agentPluginManager.js'; @@ -111,44 +111,28 @@ suite('AgentPluginManager', () => { suite('syncCustomizations', () => { - test('uses file plugins in place only when the host trusts their directory', async () => { + test('uses agent host file URIs in place and reads file URIs from the client', async () => { disposables.add(fileService.registerProvider(Schemas.file, disposables.add(new InMemoryFileSystemProvider()))); const directory = URI.file('/local/bundle'); - const ref = { ...makeRef('local', 'revision'), uri: directory.toString() }; - await fileService.writeFile(URI.joinPath(directory, 'index.js'), VSBuffer.fromString('original')); + await fileService.writeFile(URI.joinPath(directory, 'index.js'), VSBuffer.fromString('host')); await fileService.writeFile(URI.joinPath(toAgentClientUri(directory, AUTOMATION_ACTIVE_CLIENT_ID), 'index.js'), VSBuffer.fromString('client-served')); - const [untrusted] = await manager.syncCustomizations(AUTOMATION_ACTIVE_CLIENT_ID, [ref]); - manager.trustHostPluginDirectory(directory); - const [trusted] = await manager.syncCustomizations('any-client', [ref]); + const hostRef = { ...makeRef('host', 'revision'), uri: toAgentHostFileUri(directory).toString() }; + const clientRef = { ...makeRef('client', 'revision'), uri: directory.toString() }; + const [host] = await manager.syncCustomizations('not-connected', [hostRef]); + const cacheExistsAfterHost = await fileService.exists(URI.joinPath(manager.basePath, 'cache.json')); + const [client] = await manager.syncCustomizations(AUTOMATION_ACTIVE_CLIENT_ID, [clientRef]); assert.deepStrictEqual({ - untrustedCopied: untrusted.pluginDir?.toString() !== directory.toString(), - untrustedContent: (await fileService.readFile(URI.joinPath(untrusted.pluginDir!, 'index.js'))).value.toString(), - trusted: trusted.pluginDir?.toString(), - trustedLoad: trusted.customization.load, + hostPluginDir: host.pluginDir?.toString(), + hostCustomization: host.customization, + cacheExistsAfterHost, + clientCopied: client.pluginDir?.toString() !== directory.toString(), + clientContent: (await fileService.readFile(URI.joinPath(client.pluginDir!, 'index.js'))).value.toString(), }, { - untrustedCopied: true, untrustedContent: 'client-served', - trusted: directory.toString(), trustedLoad: { kind: 'loaded' }, - }); - }); - - test('uses immutable host directories in place without client reads or cache entries', async () => { - disposables.add(fileService.registerProvider(Schemas.file, disposables.add(new InMemoryFileSystemProvider()))); - const hostManager = new AgentPluginManager(URI.file('/userData'), fileService, new NullLogService()); - const directory = URI.joinPath(hostManager.hostPluginsPath, 'automations', 'captured'); - await fileService.createFolder(directory); - await fileService.writeFile(URI.joinPath(directory, 'index.js'), VSBuffer.fromString('immutable')); - const ref = { ...makeRef('host', 'revision'), uri: directory.toString() }; - const [result] = await hostManager.syncCustomizations('not-connected', [ref]); - assert.deepStrictEqual({ - pluginDir: result.pluginDir?.toString(), - customization: result.customization, - content: (await fileService.readFile(URI.joinPath(directory, 'index.js'))).value.toString(), - cacheExists: await fileService.exists(URI.joinPath(hostManager.basePath, 'cache.json')), - }, { - pluginDir: directory.toString(), - customization: { ...ref, load: { kind: 'loaded' } }, - content: 'immutable', - cacheExists: false, + hostPluginDir: directory.toString(), + hostCustomization: { ...hostRef, load: { kind: 'loaded' } }, + cacheExistsAfterHost: false, + clientCopied: true, + clientContent: 'client-served', }); }); diff --git a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts index f2be13a416eda6..785af526d921f5 100644 --- a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts @@ -313,7 +313,6 @@ class FakeAgentPluginManager implements IAgentPluginManager { declare readonly _serviceBrand: undefined; readonly basePath = URI.from({ scheme: 'inmemory', path: '/agentPlugins' }); readonly hostPluginsPath = URI.joinPath(this.basePath, '.host'); - trustHostPluginDirectory(): void { } syncResult: readonly ISyncedCustomization[] | undefined; syncCalls: { clientId: string; customizations: readonly ClientPluginCustomization[] }[] = []; diff --git a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts index 1b37194c01c77f..4ade18b32adeb4 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts @@ -285,7 +285,6 @@ class TestAgentPluginManager implements IAgentPluginManager { readonly basePath = URI.from({ scheme: 'inmemory', path: '/agentPlugins' }); readonly hostPluginsPath = URI.joinPath(this.basePath, '.host'); - trustHostPluginDirectory(): void { } async syncCustomizations(_clientId: string, _customizations: ClientPluginCustomization[], _progress?: (status: PluginCustomization) => void): Promise { return []; diff --git a/src/vs/sessions/AUTOMATIONS.md b/src/vs/sessions/AUTOMATIONS.md index 9c0bc8cc42d387..7060e6b7c048ba 100644 --- a/src/vs/sessions/AUTOMATIONS.md +++ b/src/vs/sessions/AUTOMATIONS.md @@ -70,7 +70,7 @@ The host copies each new or changed entry from the dispatching client into an im For a local VS Code window connected through MessagePort, `file:` plugins are parsed and used at their original host paths instead of copied; edits to their contents on disk take effect in later runs. Virtual bundles and plugins from remote clients are still copied, and garbage collection only removes host-owned copies, never these in-place paths. -A run session receives the copies as a static, host-owned active client created with the session, so providers load them through the same path as any other client plugin, without contacting the originating client. That client contributes no tools and is not re-attached when a session is restored. A selected custom agent inside a captured plugin is remapped to the copy. +A run session receives the copies as a static, host-owned active client created with the session, so providers load them through the same path as any other client plugin, without contacting the originating client. That client's plugin URIs use the `vscode-agent-host-file:` scheme, which tells the plugin manager to use the host directory in place; a `file:` URI always names a client resource. That client contributes no tools and is not re-attached when a session is restored. A selected custom agent inside a captured plugin is remapped to the copy. After each create, update, or removal, the host deletes copies that no automation references, including those of a rejected capture. Copies used by a run session stay until the host restarts, because that session keeps using them in place for follow-up turns.