diff --git a/src/vs/platform/agentHost/common/agentPluginManager.ts b/src/vs/platform/agentHost/common/agentPluginManager.ts index ba7e75c88fa0cc..5f85af165c96d8 100644 --- a/src/vs/platform/agentHost/common/agentPluginManager.ts +++ b/src/vs/platform/agentHost/common/agentPluginManager.ts @@ -9,6 +9,21 @@ 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'; + +/** + * 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). */ @@ -37,11 +52,13 @@ 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. */ + /** Directory for immutable host-owned plugin copies, such as automation captures. */ readonly hostPluginsPath: URI; /** * 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 786a31c1b1c723..96073737132bad 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, 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'; -/** 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 }, @@ -92,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 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..d03dfaf95a0cb8 100644 --- a/src/vs/platform/agentHost/node/agentPluginManager.ts +++ b/src/vs/platform/agentHost/node/agentPluginManager.ts @@ -7,10 +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 { 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'; @@ -141,9 +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. - if (uri.scheme === Schemas.file && extUriBiasedIgnorePathCase.isEqualOrParent(extUriBiasedIgnorePathCase.normalizePath(uri), 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/node/protocolServerHandler.ts b/src/vs/platform/agentHost/node/protocolServerHandler.ts index 4349fa5e037b26..68af61b6a661d0 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.telemetryConnectionActive && this._supportsCanvases(connection)); + } + 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..7d0d67dad502f0 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, toAgentHostFileUri } 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))!; @@ -152,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 b13008e47dabb8..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 '../../node/agentHostAutomationCustomizations.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'; @@ -74,6 +74,7 @@ suite('AgentHostAutomationService', () => { disposables.add(clientConnections.registerSource({ hasSeenClient: () => false, isClientConnected: () => false, + isLocalClient: () => false, getConnectedClientTransportCounts: () => new Map(), requestWorkspaceTrust: async () => false, requestMcpAuthentication: request => { @@ -253,12 +254,46 @@ 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(), }, }); }); + 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]; + 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), + 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, runPluginDir: ref.uri, + }); + }); + 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..a927b85cbbc73c 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, 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'; @@ -110,24 +111,28 @@ suite('AgentPluginManager', () => { suite('syncCustomizations', () => { - test('uses immutable host directories in place without client reads or cache entries', 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 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]); + const directory = URI.file('/local/bundle'); + 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 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({ - 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')), + 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(), }, { - 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/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..7060e6b7c048ba 100644 --- a/src/vs/sessions/AUTOMATIONS.md +++ b/src/vs/sessions/AUTOMATIONS.md @@ -64,9 +64,13 @@ 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`. -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. +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'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. diff --git a/src/vs/sessions/contrib/automations/browser/automationDialog.ts b/src/vs/sessions/contrib/automations/browser/automationDialog.ts index 380ecd25f86591..27895b66f790f6 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,12 @@ 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 waitForCustomizationChoices: (token: CancellationToken) => Promise; 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 +1036,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 +1416,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 +1463,42 @@ 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(), + waitForCustomizationChoices: async token => customizationSelection?.waitForChoices(token), showTargetValidationError: message => { const text = message ?? ''; if (targetError.textContent === text) { @@ -1487,28 +1519,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 +1566,185 @@ 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 loading: Promise = Promise.resolve(); + 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); + } + + /** 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; + } + 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; + this.loading = 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..95dcbeb2470bde 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,15 @@ 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 waitForCustomizationChoices: (token: CancellationToken) => Promise = async () => { }; + 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 +165,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 +179,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 +196,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 +226,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; @@ -227,10 +240,11 @@ 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(); - showSessionConfigurationError(captureErrorMessage); + showSaveError(captureErrorMessage); shouldFocusError = true; return; } @@ -240,14 +254,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 +283,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 +339,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 +355,16 @@ 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; + waitForCustomizationChoices = handle.waitForCustomizationChoices; + showSaveError = handle.showSaveError; + focusSaveError = handle.focusSaveError; getFocusableElements = handle.getFocusableElements; const keyboardNavigation = disposables.add(registerAutomationDialogKeyboardNavigation( DOM.getWindow(container), @@ -333,10 +374,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 +411,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 +432,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..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,11 +10,12 @@ 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'; 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 +43,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 +64,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 +73,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 +112,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 +155,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 +169,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 +392,78 @@ 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('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 () => { 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;