From 22ca50c7d19475912633cb04ae96bcb847c3fdb9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 07:48:24 +0000 Subject: [PATCH 01/17] Initial plan From 9c2b7035f2227ba48d1647d839c75ce0c8a0c001 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 08:09:33 +0000 Subject: [PATCH 02/17] Fix contextview viewport calculation to use container's window instead of active window The ContextView.doLayout() method was using DOM.getActiveWindow() to obtain viewport dimensions for positioning. In multi-window scenarios (e.g. when an editor is in a floating/auxiliary window), getActiveWindow() may return the main window instead of the window containing the context view, causing incorrect positioning of code action widgets, context menus, and other popups anchored through the ContextView. Changed to use DOM.getWindow(this.container) which resolves the window from the actual container element, ensuring correct viewport dimensions regardless of which window currently has focus. Co-authored-by: mjbvz <12821956+mjbvz@users.noreply.github.com> --- src/vs/base/browser/ui/contextview/contextview.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/base/browser/ui/contextview/contextview.ts b/src/vs/base/browser/ui/contextview/contextview.ts index b3bfc63cb79ca..3aa5f3828dcfc 100644 --- a/src/vs/base/browser/ui/contextview/contextview.ts +++ b/src/vs/base/browser/ui/contextview/contextview.ts @@ -235,8 +235,8 @@ export class ContextView extends Disposable { // Get anchor const anchor = getAnchorRect(this.delegate!.getAnchor()); - const activeWindow = DOM.getActiveWindow(); - const viewport = { top: activeWindow.pageYOffset, left: activeWindow.pageXOffset, width: activeWindow.innerWidth, height: activeWindow.innerHeight }; + const containerWindow = this.container ? DOM.getWindow(this.container) : DOM.getActiveWindow(); + const viewport = { top: containerWindow.pageYOffset, left: containerWindow.pageXOffset, width: containerWindow.innerWidth, height: containerWindow.innerHeight }; const view = { width: DOM.getTotalWidth(this.view), height: DOM.getTotalHeight(this.view) }; const anchorPosition = this.delegate!.anchorPosition; const anchorAlignment = this.delegate!.anchorAlignment; From 5367dafe06fb81b0fe53b019fc8b6899356bbed1 Mon Sep 17 00:00:00 2001 From: Isidor Date: Thu, 26 Mar 2026 13:45:16 +0100 Subject: [PATCH 03/17] feat(telemetry): log terminal sandbox setting changes --- .../contrib/telemetry/browser/telemetry.contribution.ts | 8 ++++++++ .../workbench/contrib/terminal/terminalContribExports.ts | 1 + 2 files changed, 9 insertions(+) diff --git a/src/vs/workbench/contrib/telemetry/browser/telemetry.contribution.ts b/src/vs/workbench/contrib/telemetry/browser/telemetry.contribution.ts index 399ff208b721a..f0baac310882e 100644 --- a/src/vs/workbench/contrib/telemetry/browser/telemetry.contribution.ts +++ b/src/vs/workbench/contrib/telemetry/browser/telemetry.contribution.ts @@ -444,6 +444,14 @@ class ConfigurationTelemetryContribution extends Disposable implements IWorkbenc source: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'source of the setting' }; }>('terminal.integrated.suggest.enabled', { settingValue: this.getValueToReport(key, target), source }); return; + case TerminalContribSettingId.TerminalSandboxEnabled: + this.telemetryService.publicLog2('chat.tools.terminal.sandbox.enabled', { settingValue: this.getValueToReport(key, target), source }); + return; } } diff --git a/src/vs/workbench/contrib/terminal/terminalContribExports.ts b/src/vs/workbench/contrib/terminal/terminalContribExports.ts index babbac632fcc8..3988435a01cd2 100644 --- a/src/vs/workbench/contrib/terminal/terminalContribExports.ts +++ b/src/vs/workbench/contrib/terminal/terminalContribExports.ts @@ -46,6 +46,7 @@ export const enum TerminalContribSettingId { EnableAutoApprove = TerminalChatAgentToolsSettingId.EnableAutoApprove, ShellIntegrationTimeout = TerminalChatAgentToolsSettingId.ShellIntegrationTimeout, OutputLocation = TerminalChatAgentToolsSettingId.OutputLocation, + TerminalSandboxEnabled = TerminalChatAgentToolsSettingId.TerminalSandboxEnabled, DeprecatedTerminalSandboxNetwork = TerminalChatAgentToolsSettingId.DeprecatedTerminalSandboxNetwork, TerminalSandboxNetworkAllowedDomains = TerminalChatAgentToolsSettingId.TerminalSandboxNetworkAllowedDomains, TerminalSandboxNetworkDeniedDomains = TerminalChatAgentToolsSettingId.TerminalSandboxNetworkDeniedDomains, From 228f1b6987b862a6ad6b5d88333a62e1a60da115 Mon Sep 17 00:00:00 2001 From: Yogeshwaran C <84272111+yogeshwaran-c@users.noreply.github.com> Date: Fri, 27 Mar 2026 11:16:35 +0530 Subject: [PATCH 04/17] Merge pull request #304959 from yogeshwaran-c/fix/testing-icon-color-inheritance fix: make testing icon colors inherit from list error/warning foreground --- .../workbench/contrib/testing/browser/theme.ts | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/contrib/testing/browser/theme.ts b/src/vs/workbench/contrib/testing/browser/theme.ts index b41f5828a9771..d2caa0673bc8b 100644 --- a/src/vs/workbench/contrib/testing/browser/theme.ts +++ b/src/vs/workbench/contrib/testing/browser/theme.ts @@ -5,22 +5,13 @@ import { localize } from '../../../../nls.js'; import { activityErrorBadgeBackground, activityErrorBadgeForeground, badgeBackground, badgeForeground, chartsGreen, chartsRed, contrastBorder, diffInserted, diffRemoved, editorBackground, editorErrorForeground, editorForeground, editorInfoForeground, opaque, registerColor, transparent } from '../../../../platform/theme/common/colorRegistry.js'; +import { listErrorForeground, listWarningForeground } from '../../../../platform/theme/common/colors/listColors.js'; import { registerThemingParticipant } from '../../../../platform/theme/common/themeService.js'; import { TestResultState } from '../common/testTypes.js'; -export const testingColorIconFailed = registerColor('testing.iconFailed', { - dark: '#f14c4c', - light: '#f14c4c', - hcDark: '#f14c4c', - hcLight: '#B5200D' -}, localize('testing.iconFailed', "Color for the 'failed' icon in the test explorer.")); +export const testingColorIconFailed = registerColor('testing.iconFailed', listErrorForeground, localize('testing.iconFailed', "Color for the 'failed' icon in the test explorer.")); -export const testingColorIconErrored = registerColor('testing.iconErrored', { - dark: '#f14c4c', - light: '#f14c4c', - hcDark: '#f14c4c', - hcLight: '#B5200D' -}, localize('testing.iconErrored', "Color for the 'Errored' icon in the test explorer.")); +export const testingColorIconErrored = registerColor('testing.iconErrored', listErrorForeground, localize('testing.iconErrored', "Color for the 'Errored' icon in the test explorer.")); export const testingColorIconPassed = registerColor('testing.iconPassed', { dark: '#73c991', @@ -31,7 +22,7 @@ export const testingColorIconPassed = registerColor('testing.iconPassed', { export const testingColorRunAction = registerColor('testing.runAction', testingColorIconPassed, localize('testing.runAction', "Color for 'run' icons in the editor.")); -export const testingColorIconQueued = registerColor('testing.iconQueued', '#cca700', localize('testing.iconQueued', "Color for the 'Queued' icon in the test explorer.")); +export const testingColorIconQueued = registerColor('testing.iconQueued', listWarningForeground, localize('testing.iconQueued', "Color for the 'Queued' icon in the test explorer.")); export const testingColorIconUnset = registerColor('testing.iconUnset', '#848484', localize('testing.iconUnset', "Color for the 'Unset' icon in the test explorer.")); From 1d62cc626a51b25c83b2f4bad00e225449d9e275 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Thu, 26 Mar 2026 22:46:59 -0700 Subject: [PATCH 05/17] agentPlugins: normalize to user data dir storage (#304977) * agentPlugins: normalize to user data dir storage Previously we stored plugins in a very internal way that was inaccessible by other tooling. This sets up a `agent-plugins` folder beside `extensions` and creates an `installed.json` which is easy to integrate with. * comments * fix compile --------- Co-authored-by: Raymond Zhao <7199958+rzhao271@users.noreply.github.com> --- src/vs/platform/environment/common/argv.ts | 1 + .../environment/common/environmentService.ts | 20 ++ src/vs/platform/environment/node/argv.ts | 1 + .../browser/agentPluginRepositoryService.ts | 56 +++- .../chat/browser/pluginInstallService.ts | 2 +- .../plugins/agentPluginRepositoryService.ts | 7 + .../fileBackedInstalledPluginsStore.ts | 276 ++++++++++++++++ .../plugins/pluginMarketplaceService.ts | 301 ++++++++++-------- .../agentPluginRepositoryService.test.ts | 10 +- .../fileBackedInstalledPluginsStore.test.ts | 208 ++++++++++++ .../plugins/pluginMarketplaceService.test.ts | 125 +++++++- .../environment/browser/environmentService.ts | 3 + .../environment/common/environmentService.ts | 1 + .../electron-browser/environmentService.ts | 3 + 14 files changed, 876 insertions(+), 138 deletions(-) create mode 100644 src/vs/workbench/contrib/chat/common/plugins/fileBackedInstalledPluginsStore.ts create mode 100644 src/vs/workbench/contrib/chat/test/common/plugins/fileBackedInstalledPluginsStore.test.ts diff --git a/src/vs/platform/environment/common/argv.ts b/src/vs/platform/environment/common/argv.ts index 72752cf9ed1af..b391d4e6a915e 100644 --- a/src/vs/platform/environment/common/argv.ts +++ b/src/vs/platform/environment/common/argv.ts @@ -75,6 +75,7 @@ export interface NativeParsedArgs { 'extensions-dir'?: string; 'extensions-download-dir'?: string; 'builtin-extensions-dir'?: string; + 'agent-plugins-dir'?: string; extensionDevelopmentPath?: string[]; // undefined or array of 1 or more local paths or URIs extensionTestsPath?: string; // either a local path or a URI extensionDevelopmentKind?: string[]; diff --git a/src/vs/platform/environment/common/environmentService.ts b/src/vs/platform/environment/common/environmentService.ts index 3502a718fa0ca..718f2cc83c4b5 100644 --- a/src/vs/platform/environment/common/environmentService.ts +++ b/src/vs/platform/environment/common/environmentService.ts @@ -147,6 +147,26 @@ export abstract class AbstractNativeEnvironmentService implements INativeEnviron return joinPath(this.userHome, this.productService.dataFolderName, 'extensions').fsPath; } + @memoize + get agentPluginsPath(): string { + const cliAgentPluginsDir = this.args['agent-plugins-dir']; + if (cliAgentPluginsDir) { + return resolve(cliAgentPluginsDir); + } + + const vscodeAgentPlugins = env['VSCODE_AGENT_PLUGINS']; + if (vscodeAgentPlugins) { + return vscodeAgentPlugins; + } + + const vscodePortable = env['VSCODE_PORTABLE']; + if (vscodePortable) { + return join(vscodePortable, 'agent-plugins'); + } + + return joinPath(this.userHome, this.productService.dataFolderName, 'agent-plugins').fsPath; + } + @memoize get extensionDevelopmentLocationURI(): URI[] | undefined { const extensionDevelopmentPaths = this.args.extensionDevelopmentPath; diff --git a/src/vs/platform/environment/node/argv.ts b/src/vs/platform/environment/node/argv.ts index 3b7f625a6ab8e..dcb2b33a1f2de 100644 --- a/src/vs/platform/environment/node/argv.ts +++ b/src/vs/platform/environment/node/argv.ts @@ -120,6 +120,7 @@ export const OPTIONS: OptionDescriptions> = { 'extensions-download-dir': { type: 'string' }, 'builtin-extensions-dir': { type: 'string' }, 'list-extensions': { type: 'boolean', cat: 'e', description: localize('listExtensions', "List the installed extensions.") }, + 'agent-plugins-dir': { type: 'string' }, 'show-versions': { type: 'boolean', cat: 'e', description: localize('showVersions', "Show versions of installed extensions, when using --list-extensions.") }, 'category': { type: 'string', allowEmptyValue: true, cat: 'e', description: localize('category', "Filters installed extensions by provided category, when using --list-extensions."), args: 'category' }, 'install-extension': { type: 'string[]', cat: 'e', args: 'ext-id | path', description: localize('installExtension', "Installs or updates an extension. The argument is either an extension id or a path to a VSIX. The identifier of an extension is '${publisher}.${name}'. Use '--force' argument to update to latest version. To install a specific version provide '@${version}'. For example: 'vscode.csharp@1.2.3'.") }, diff --git a/src/vs/workbench/contrib/chat/browser/agentPluginRepositoryService.ts b/src/vs/workbench/contrib/chat/browser/agentPluginRepositoryService.ts index 155b5b2317811..4ceb53bc152b0 100644 --- a/src/vs/workbench/contrib/chat/browser/agentPluginRepositoryService.ts +++ b/src/vs/workbench/contrib/chat/browser/agentPluginRepositoryService.ts @@ -11,7 +11,7 @@ import { dirname, isEqual, isEqualOrParent, joinPath } from '../../../../base/co import { URI } from '../../../../base/common/uri.js'; import { localize } from '../../../../nls.js'; import { ICommandService } from '../../../../platform/commands/common/commands.js'; -import { IEnvironmentService } from '../../../../platform/environment/common/environment.js'; +import { IWorkbenchEnvironmentService } from '../../../services/environment/common/environmentService.js'; import { IFileService } from '../../../../platform/files/common/files.js'; import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; import { ILogService } from '../../../../platform/log/common/log.js'; @@ -36,14 +36,16 @@ type IStoredMarketplaceIndex = Dto>; export class AgentPluginRepositoryService implements IAgentPluginRepositoryService { declare readonly _serviceBrand: undefined; + readonly agentPluginsHome: URI; private readonly _cacheRoot: URI; private readonly _marketplaceIndex = new Lazy>(() => this._loadMarketplaceIndex()); private readonly _pluginSources: ReadonlyMap; private readonly _cloneSequencer = new SequencerByKey(); + private readonly _migrationDone: Promise; constructor( @ICommandService private readonly _commandService: ICommandService, - @IEnvironmentService environmentService: IEnvironmentService, + @IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService, @IFileService private readonly _fileService: IFileService, @IInstantiationService instantiationService: IInstantiationService, @ILogService private readonly _logService: ILogService, @@ -51,7 +53,23 @@ export class AgentPluginRepositoryService implements IAgentPluginRepositoryServi @IProgressService private readonly _progressService: IProgressService, @IStorageService private readonly _storageService: IStorageService, ) { - this._cacheRoot = joinPath(environmentService.cacheHome, 'agentPlugins'); + // On native, use the well-known ~/{dataFolderName}/agent-plugins/ path + // so that external tools can discover it. On web, fall back to the + // internal cache location. + this.agentPluginsHome = environmentService.agentPluginsHome; + const legacyCacheRoot = joinPath(environmentService.cacheHome, 'agentPlugins'); + const oldCacheRoot = environmentService.cacheHome.scheme === 'file' + ? legacyCacheRoot + : this.agentPluginsHome; + this._cacheRoot = this.agentPluginsHome; + + // Migrate plugin files from the old internal cache directory to the + // new well-known location. This is a one-time operation. + if (!isEqual(oldCacheRoot, this.agentPluginsHome)) { + this._migrationDone = this._migrateDirectory(oldCacheRoot); + } else { + this._migrationDone = Promise.resolve(); + } // Build per-kind source repository map via instantiation service so // each repository can inject its own dependencies. @@ -91,6 +109,7 @@ export class AgentPluginRepositoryService implements IAgentPluginRepositoryServi } async ensureRepository(marketplace: IMarketplaceReference, options?: IEnsureRepositoryOptions): Promise { + await this._migrationDone; const repoDir = this.getRepositoryUri(marketplace, options?.marketplaceType); return this._cloneSequencer.queue(repoDir.fsPath, async () => { const repoExists = await this._fileService.exists(repoDir); @@ -254,6 +273,7 @@ export class AgentPluginRepositoryService implements IAgentPluginRepositoryServi } async ensurePluginSource(plugin: IMarketplacePlugin, options?: IEnsureRepositoryOptions): Promise { + await this._migrationDone; const repo = this.getPluginSource(plugin.sourceDescriptor.kind); if (plugin.sourceDescriptor.kind === PluginSourceKind.RelativePath) { return this.ensureRepository(plugin.marketplaceReference, options); @@ -351,4 +371,34 @@ export class AgentPluginRepositoryService implements IAgentPluginRepositoryServi } } + /** + * One-time migration of plugin files from the old internal cache + * directory (`{cacheHome}/agentPlugins/`) to the new well-known + * location (`~/{dataFolderName}/agent-plugins/`). + */ + private async _migrateDirectory(oldCacheRoot: URI): Promise { + try { + const oldExists = await this._fileService.exists(oldCacheRoot); + if (!oldExists) { + return; + } + + const newExists = await this._fileService.exists(this.agentPluginsHome); + if (newExists) { + this._logService.info('[AgentPluginRepositoryService] Both old and new agent-plugins directories exist; skipping directory migration'); + return; + } + + this._logService.info(`[AgentPluginRepositoryService] Migrating agent plugins from ${oldCacheRoot.toString()} to ${this.agentPluginsHome.toString()}`); + await this._fileService.move(oldCacheRoot, this.agentPluginsHome, false); + + // Clear the marketplace index — it caches repository URIs that + // pointed to the old location and would cause path mismatches. + this._storageService.remove(MARKETPLACE_INDEX_STORAGE_KEY, StorageScope.APPLICATION); + this._marketplaceIndex.value.clear(); + } catch (error) { + this._logService.error('[AgentPluginRepositoryService] Directory migration failed', error); + } + } + } diff --git a/src/vs/workbench/contrib/chat/browser/pluginInstallService.ts b/src/vs/workbench/contrib/chat/browser/pluginInstallService.ts index 44fccdc7c0d76..9ee703e61e008 100644 --- a/src/vs/workbench/contrib/chat/browser/pluginInstallService.ts +++ b/src/vs/workbench/contrib/chat/browser/pluginInstallService.ts @@ -238,7 +238,7 @@ export class PluginInstallService implements IPluginInstallService { } async updateAllPlugins(options: IUpdateAllPluginsOptions, token: CancellationToken): Promise { - const installed = this._pluginMarketplaceService.installedPlugins.get().filter(e => e.enabled); + const installed = this._pluginMarketplaceService.installedPlugins.get(); if (installed.length === 0) { return { updatedNames: [], failedNames: [] }; } diff --git a/src/vs/workbench/contrib/chat/common/plugins/agentPluginRepositoryService.ts b/src/vs/workbench/contrib/chat/common/plugins/agentPluginRepositoryService.ts index 7ad90d2fd26ba..9d43e4f8bd0c7 100644 --- a/src/vs/workbench/contrib/chat/common/plugins/agentPluginRepositoryService.ts +++ b/src/vs/workbench/contrib/chat/common/plugins/agentPluginRepositoryService.ts @@ -43,6 +43,13 @@ export interface IPullRepositoryOptions { export interface IAgentPluginRepositoryService { readonly _serviceBrand: undefined; + /** + * Root directory where agent plugins are stored on disk. + * On native this is `~/{dataFolderName}/agent-plugins/`; on web it + * falls back to `{cacheHome}/agentPlugins/`. + */ + readonly agentPluginsHome: URI; + /** * Returns the local cache URI for a marketplace repository reference. * Uses a storage-backed marketplace index when available. diff --git a/src/vs/workbench/contrib/chat/common/plugins/fileBackedInstalledPluginsStore.ts b/src/vs/workbench/contrib/chat/common/plugins/fileBackedInstalledPluginsStore.ts new file mode 100644 index 0000000000000..4a3c94e7fa75f --- /dev/null +++ b/src/vs/workbench/contrib/chat/common/plugins/fileBackedInstalledPluginsStore.ts @@ -0,0 +1,276 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { RunOnceScheduler, ThrottledDelayer } from '../../../../../base/common/async.js'; +import { VSBuffer } from '../../../../../base/common/buffer.js'; +import { Disposable } from '../../../../../base/common/lifecycle.js'; +import { revive } from '../../../../../base/common/marshalling.js'; +import { IObservable, ITransaction, observableValue } from '../../../../../base/common/observable.js'; +import { isEqual, joinPath } from '../../../../../base/common/resources.js'; +import { URI, UriComponents } from '../../../../../base/common/uri.js'; +import { IFileService } from '../../../../../platform/files/common/files.js'; +import { ILogService } from '../../../../../platform/log/common/log.js'; +import { IStorageService, StorageScope } from '../../../../../platform/storage/common/storage.js'; + +const INSTALLED_JSON_FILENAME = 'installed.json'; +const INSTALLED_JSON_VERSION = 1; + +/** Legacy storage key used before migration to file-backed store. */ +const LEGACY_INSTALLED_PLUGINS_STORAGE_KEY = 'chat.plugins.installed.v1'; +/** Legacy storage key for the marketplace index that cached old URI paths. */ +const LEGACY_MARKETPLACE_INDEX_STORAGE_KEY = 'chat.plugins.marketplaces.index.v1'; + +/** + * Minimal entry stored in `installed.json`. URIs are serialised as strings + * so that external tools can read and write the file without depending on + * VS Code internal URI representations. + */ +interface IInstalledJsonEntry { + readonly pluginUri: string; + readonly marketplace: string; +} + +/** + * On-disk schema for `installed.json`. + */ +interface IInstalledJson { + readonly version: number; + readonly installed: readonly IInstalledJsonEntry[]; +} + +/** + * In-memory representation of an installed plugin entry. + */ +export interface IStoredInstalledPlugin { + readonly pluginUri: URI; + readonly marketplace: string; +} + +/** + * An observable store for installed agent plugins that is backed by a + * `installed.json` file within the agent-plugins directory. This makes + * the installed-plugin manifest discoverable by external tools (CLIs, + * other editors, etc.) without depending on VS Code internals. + * + * The on-disk format stores only the plugin URI (as a string) and the + * marketplace identifier. Plugin metadata (name, description, etc.) is + * read from the plugin manifest on disk by the discovery layer - + * keeping a single source of truth. + * + * On construction the store: + * 1. Attempts to read `installed.json` from the agent-plugins directory. + * 2. If no file exists, migrates data from the legacy {@link StorageService} + * key (`chat.plugins.installed.v1`), rebasing plugin URIs from the old + * cache directory to the new agent-plugins directory. + * 3. Sets up a correlated file watcher so that external edits to + * `installed.json` are picked up automatically. + * + * Write operations update the in-memory observable synchronously and + * schedule a debounced file write so that rapid successive mutations + * (e.g. batch enables) are coalesced into a single I/O operation. + */ +export class FileBackedInstalledPluginsStore extends Disposable { + private readonly _installed = observableValue('file/installed.json', []); + private readonly _fileUri: URI; + private readonly _writeDelayer: ThrottledDelayer; + private _suppressFileWatch = false; + private _initialized = false; + + readonly value: IObservable = this._installed; + + constructor( + private readonly _agentPluginsHome: URI, + private readonly _oldCacheRoot: URI | undefined, + private readonly _fileService: IFileService, + private readonly _logService: ILogService, + private readonly _storageService: IStorageService, + ) { + super(); + this._fileUri = joinPath(_agentPluginsHome, INSTALLED_JSON_FILENAME); + this._writeDelayer = this._register(new ThrottledDelayer(100)); + void this._initialize(); + } + + get(): readonly IStoredInstalledPlugin[] { + return this._installed.get(); + } + + set(newValue: readonly IStoredInstalledPlugin[], tx: ITransaction | undefined): void { + this._setValue(newValue, tx, true); + } + + private async _initialize(): Promise { + try { + const read = await this._readFromFile(); + if (read !== undefined) { + this._setValue(read, undefined, false); + } else { + // No installed.json yet — attempt migration from legacy storage. + await this._migrateFromStorage(); + } + } catch (error) { + this._logService.error('[FileBackedInstalledPluginsStore] Initialization failed', error); + } + + this._initialized = true; + this._setupFileWatcher(); + } + + // --- File I/O ---------------------------------------------------------------- + + private async _readFromFile(): Promise { + try { + const exists = await this._fileService.exists(this._fileUri); + if (!exists) { + return undefined; + } + + const content = await this._fileService.readFile(this._fileUri); + const json: IInstalledJson = JSON.parse(content.value.toString()); + if (!json || !Array.isArray(json.installed)) { + this._logService.warn('[FileBackedInstalledPluginsStore] installed.json has unexpected format, ignoring'); + return undefined; + } + + // Each entry is { pluginUri: string, enabled: boolean }. + return json.installed + .filter((entry): entry is IInstalledJsonEntry => typeof entry.pluginUri === 'string' && typeof entry.marketplace === 'string') + .map(entry => ({ pluginUri: URI.parse(entry.pluginUri), marketplace: entry.marketplace })); + } catch { + return undefined; + } + } + + private _scheduleWrite(): void { + void this._writeDelayer.trigger(async () => { + await this._writeToFile(); + }); + } + + private async _writeToFile(): Promise { + const entries: IInstalledJsonEntry[] = this.get().map(e => ({ + pluginUri: e.pluginUri.toString(), + marketplace: e.marketplace, + })); + + const data: IInstalledJson = { + version: INSTALLED_JSON_VERSION, + installed: entries, + }; + + try { + this._suppressFileWatch = true; + const content = JSON.stringify(data, undefined, '\t'); + await this._fileService.createFolder(this._agentPluginsHome); + await this._fileService.writeFile(this._fileUri, VSBuffer.fromString(content)); + return true; + } catch (error) { + this._logService.error('[FileBackedInstalledPluginsStore] Failed to write installed.json', error); + return false; + } finally { + this._suppressFileWatch = false; + } + } + + // --- File watching ------------------------------------------------------------ + + private _setupFileWatcher(): void { + if (typeof this._fileService.createWatcher !== 'function') { + return; + } + const dir = this._agentPluginsHome; + const watcher = this._fileService.createWatcher(dir, { recursive: false, excludes: [] }); + this._register(watcher); + + const scheduler = this._register(new RunOnceScheduler(() => this._onFileChanged(), 100)); + this._register(watcher.onDidChange(e => { + if (!this._suppressFileWatch && e.affects(this._fileUri)) { + scheduler.schedule(); + } + })); + } + + private async _onFileChanged(): Promise { + const read = await this._readFromFile(); + if (read !== undefined) { + // Suppress file write for externally triggered updates. + this._suppressFileWatch = true; + try { + this._setValue(read, undefined, false); + } finally { + this._suppressFileWatch = false; + } + } + } + + // --- Write-through to file ---------------------------------------------------- + + private _setValue(newValue: readonly IStoredInstalledPlugin[], tx: ITransaction | undefined, scheduleWrite: boolean): void { + this._installed.set(newValue, tx); + // Only schedule writes after initialization and when not processing + // an external file change. + if (scheduleWrite && this._initialized && !this._suppressFileWatch) { + this._scheduleWrite(); + } + } + + // --- Migration from legacy storage ------------------------------------------- + + private async _migrateFromStorage(): Promise { + const raw = this._storageService.get(LEGACY_INSTALLED_PLUGINS_STORAGE_KEY, StorageScope.APPLICATION); + if (!raw) { + return; + } + + try { + const parsed = JSON.parse(raw); + if (!Array.isArray(parsed) || parsed.length === 0) { + return; + } + + const migrated: IStoredInstalledPlugin[] = (revive(parsed) as { pluginUri: UriComponents; plugin?: { marketplaceReference?: { rawValue?: string } } }[]).map(entry => { + const uri = URI.revive(entry.pluginUri); + const rebased = this._rebasePluginUri(uri); + return { + pluginUri: rebased ?? uri, + marketplace: entry.plugin?.marketplaceReference?.rawValue ?? '', + }; + }).filter(e => !!e.marketplace); + + this._logService.info(`[FileBackedInstalledPluginsStore] Migrating ${migrated.length} plugin(s) from storage to installed.json`); + + // Set in memory and persist to file before removing legacy keys. + this._setValue(migrated, undefined, false); + const didPersist = await this._writeToFile(); + if (!didPersist) { + return; + } + + // Clean up legacy keys. + this._storageService.remove(LEGACY_INSTALLED_PLUGINS_STORAGE_KEY, StorageScope.APPLICATION); + this._storageService.remove(LEGACY_MARKETPLACE_INDEX_STORAGE_KEY, StorageScope.APPLICATION); + } catch (error) { + this._logService.error('[FileBackedInstalledPluginsStore] Migration from storage failed', error); + } + } + + /** + * If the plugin URI was under the old cache root, rebase it to the + * new agent-plugins directory. Otherwise, return `undefined` to keep + * the original. + */ + private _rebasePluginUri(uri: URI): URI | undefined { + if (!this._oldCacheRoot) { + return undefined; + } + + const oldRoot = this._oldCacheRoot; + if (!isEqual(uri, oldRoot) && uri.scheme === oldRoot.scheme && uri.path.startsWith(oldRoot.path + '/')) { + const relativePart = uri.path.substring(oldRoot.path.length); + return uri.with({ path: this._agentPluginsHome.path + relativePart }); + } + return undefined; + } +} diff --git a/src/vs/workbench/contrib/chat/common/plugins/pluginMarketplaceService.ts b/src/vs/workbench/contrib/chat/common/plugins/pluginMarketplaceService.ts index 22b67da4ff24f..814c4f943a63f 100644 --- a/src/vs/workbench/contrib/chat/common/plugins/pluginMarketplaceService.ts +++ b/src/vs/workbench/contrib/chat/common/plugins/pluginMarketplaceService.ts @@ -10,10 +10,11 @@ import { parse as parseJSONC } from '../../../../../base/common/json.js'; import { Lazy } from '../../../../../base/common/lazy.js'; import { Disposable } from '../../../../../base/common/lifecycle.js'; import { revive } from '../../../../../base/common/marshalling.js'; -import { derived, IObservable, observableFromEvent, observableValue } from '../../../../../base/common/observable.js'; +import { autorun, derived, IObservable, observableFromEvent, observableValue } from '../../../../../base/common/observable.js'; import { isEqual, isEqualOrParent, joinPath, normalizePath, relativePath } from '../../../../../base/common/resources.js'; -import { URI, UriComponents } from '../../../../../base/common/uri.js'; +import { URI } from '../../../../../base/common/uri.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; +import { IEnvironmentService } from '../../../../../platform/environment/common/environment.js'; import { IFileService } from '../../../../../platform/files/common/files.js'; import { createDecorator } from '../../../../../platform/instantiation/common/instantiation.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; @@ -24,6 +25,7 @@ import type { Dto } from '../../../../services/extensions/common/proxyIdentifier import { AutoUpdateConfigurationKey, AutoUpdateConfigurationValue } from '../../../extensions/common/extensions.js'; import { ChatConfiguration } from '../constants.js'; import { IAgentPluginRepositoryService } from './agentPluginRepositoryService.js'; +import { FileBackedInstalledPluginsStore, IStoredInstalledPlugin } from './fileBackedInstalledPluginsStore.js'; import { IWorkspacePluginSettingsService } from './workspacePluginSettingsService.js'; import { IWorkspaceTrustManagementService } from '../../../../../platform/workspace/common/workspaceTrust.js'; import { type IMarketplaceReference, deduplicateMarketplaceReferences, MarketplaceReferenceKind, parseMarketplaceReference, parseMarketplaceReferences } from './marketplaceReference.js'; @@ -134,7 +136,6 @@ interface IMarketplaceJson { export interface IMarketplaceInstalledPlugin { readonly pluginUri: URI; readonly plugin: IMarketplacePlugin; - readonly enabled: boolean; } export const IPluginMarketplaceService = createDecorator('pluginMarketplaceService'); @@ -168,7 +169,6 @@ export interface IPluginMarketplaceService { getMarketplacePluginMetadata(pluginUri: URI): IMarketplacePlugin | undefined; addInstalledPlugin(pluginUri: URI, plugin: IMarketplacePlugin): void; removeInstalledPlugin(pluginUri: URI): void; - setInstalledPluginEnabled(pluginUri: URI, enabled: boolean): void; /** Returns whether the given marketplace has been explicitly trusted by the user. */ isMarketplaceTrusted(ref: IMarketplaceReference): boolean; /** Records that the user trusts the given marketplace, persisted permanently. */ @@ -207,12 +207,6 @@ interface IGitHubMarketplaceCacheEntry { type IStoredGitHubMarketplaceCache = Dto>; -interface IStoredInstalledPlugin { - readonly pluginUri: UriComponents; - readonly plugin: IMarketplacePlugin; - readonly enabled: boolean; -} - /** * Ensures that an {@link IMarketplacePlugin} loaded from storage has a * {@link IMarketplacePlugin.sourceDescriptor sourceDescriptor}. Plugins @@ -230,16 +224,6 @@ function ensureSourceDescriptor(plugin: IMarketplacePlugin): IMarketplacePlugin }; } -const installedPluginsMemento = observableMemento({ - defaultValue: [], - key: 'chat.plugins.installed.v1', - toStorage: value => JSON.stringify(value), - fromStorage: value => { - const parsed = JSON.parse(value); - return Array.isArray(parsed) ? parsed : []; - }, -}); - const trustedMarketplacesMemento = observableMemento({ defaultValue: [], key: 'chat.plugins.trustedMarketplaces.v1', @@ -271,7 +255,8 @@ const lastFetchedPluginsMemento = observableMemento({ export class PluginMarketplaceService extends Disposable implements IPluginMarketplaceService { declare readonly _serviceBrand: undefined; private readonly _gitHubMarketplaceCache = new Lazy>(() => this._loadPersistedGitHubMarketplaceCache()); - private readonly _installedPluginsStore: ObservableMemento; + private readonly _installedPluginsStore: FileBackedInstalledPluginsStore; + private readonly _pluginMetadata = new Map(); private readonly _trustedMarketplacesStore: ObservableMemento; private readonly _lastFetchedPluginsStore: ObservableMemento; private readonly _hasUpdatesAvailable = observableValue('hasUpdatesAvailable', false); @@ -287,6 +272,7 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke constructor( @IConfigurationService private readonly _configurationService: IConfigurationService, @IRequestService private readonly _requestService: IRequestService, + @IEnvironmentService environmentService: IEnvironmentService, @IFileService private readonly _fileService: IFileService, @IAgentPluginRepositoryService private readonly _pluginRepositoryService: IAgentPluginRepositoryService, @ILogService private readonly _logService: ILogService, @@ -296,8 +282,17 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke ) { super(); + // File-backed store for installed plugins. The old cache location + // is passed so the store can rebase URIs during migration. + const oldCacheRoot = joinPath(environmentService.cacheHome, 'agentPlugins'); this._installedPluginsStore = this._register( - installedPluginsMemento(StorageScope.APPLICATION, StorageTarget.MACHINE, _storageService) + new FileBackedInstalledPluginsStore( + _pluginRepositoryService.agentPluginsHome, + oldCacheRoot, + _fileService, + _logService, + _storageService, + ) ); this._trustedMarketplacesStore = this._register( @@ -313,12 +308,16 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke return revived.plugins.map(ensureSourceDescriptor); }); - this.installedPlugins = this._installedPluginsStore.map(s => - (revive(s) as readonly IMarketplaceInstalledPlugin[]).map(e => ({ - ...e, - plugin: ensureSourceDescriptor(e.plugin), - })) - ); + this.installedPlugins = this._installedPluginsStore.value.map(entries => { + const result: IMarketplaceInstalledPlugin[] = []; + for (const e of entries) { + const plugin = this._pluginMetadata.get(e.pluginUri.toString()); + if (plugin) { + result.push({ pluginUri: e.pluginUri, plugin }); + } + } + return result; + }); // Aggregate recommended plugin keys from all providers. // Currently sourced from Claude workspace settings; more providers can be @@ -356,6 +355,18 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke e => e.affectsConfiguration(AutoUpdateConfigurationKey), )(() => this._scheduleUpdateCheck())); })); + + // Hydrate plugin metadata for installed entries that are not yet in + // the in-memory cache (e.g. after restart when installed.json is read + // but the metadata map is empty). Walks up from each plugin URI to + // find the marketplace.json in the enclosing repository directory. + this._register(autorun(reader => { + const entries = this._installedPluginsStore.value.read(reader); + const unhydrated = entries.filter(e => !this._pluginMetadata.has(e.pluginUri.toString())); + if (unhydrated.length > 0) { + this._hydratePluginMetadata(unhydrated); + } + })); } override dispose(): void { @@ -423,65 +434,34 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke let repoMayBePrivate = true; - for (const def of MARKETPLACE_DEFINITIONS) { + const plugins = await this._readPluginsFromDefinitions(reference, async (defPath) => { if (token.isCancellationRequested) { - return []; + return undefined; } - const url = `https://raw.githubusercontent.com/${repo}/main/${def.path}`; + const url = `https://raw.githubusercontent.com/${repo}/main/${defPath}`; try { const context = await this._requestService.request({ type: 'GET', url, callSite: 'pluginMarketplaceService.fetchPluginList' }, token); const statusCode = context.res.statusCode; if (statusCode !== 200) { repoMayBePrivate &&= statusCode !== undefined && statusCode >= 400 && statusCode < 500; this._logService.debug(`[PluginMarketplaceService] ${url} returned status ${statusCode}, skipping`); - continue; - } - const json = await asJson(context); - if (!json?.plugins || !Array.isArray(json.plugins)) { - this._logService.debug(`[PluginMarketplaceService] ${url} did not contain a valid plugins array, skipping`); - continue; + return undefined; } - const plugins = json.plugins - .filter((p): p is { name: string; description?: string; version?: string; source?: string | IJsonPluginSource } => - typeof p.name === 'string' && !!p.name - ) - .flatMap(p => { - const sourceDescriptor = parsePluginSource(p.source, json.metadata?.pluginRoot, { - pluginName: p.name, - logService: this._logService, - logPrefix: `[PluginMarketplaceService]`, - }); - if (!sourceDescriptor) { - return []; - } - - const source = sourceDescriptor.kind === PluginSourceKind.RelativePath ? sourceDescriptor.path : ''; - - return [{ - name: p.name, - description: p.description ?? '', - version: p.version ?? '', - source, - sourceDescriptor, - marketplace: reference.displayLabel, - marketplaceReference: reference, - marketplaceType: def.type, - readmeUri: getMarketplaceReadmeUri(repo, source), - }]; - }); - - cache.set(reference.canonicalId, { - plugins, - expiresAt: Date.now() + GITHUB_MARKETPLACE_CACHE_TTL_MS, - referenceRawValue: reference.rawValue, - }); - this._savePersistedGitHubMarketplaceCache(cache); - - return plugins; + return await asJson(context) ?? undefined; } catch (err) { this._logService.debug(`[PluginMarketplaceService] Failed to fetch marketplace.json from ${url}:`, err); - continue; + return undefined; } + }); + + if (plugins.length > 0) { + cache.set(reference.canonicalId, { + plugins, + expiresAt: Date.now() + GITHUB_MARKETPLACE_CACHE_TTL_MS, + referenceRawValue: reference.rawValue, + }); + this._savePersistedGitHubMarketplaceCache(cache); + return plugins; } if (repoMayBePrivate) { @@ -572,38 +552,122 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke } getMarketplacePluginMetadata(pluginUri: URI): IMarketplacePlugin | undefined { - const installed = this.installedPlugins.get(); - return installed.find(e => isEqualOrParent(pluginUri, e.pluginUri))?.plugin; + return this._pluginMetadata.get(pluginUri.toString()) + ?? [...this._pluginMetadata.entries()].find(([key]) => isEqualOrParent(pluginUri, URI.parse(key)))?.[1]; } addInstalledPlugin(pluginUri: URI, plugin: IMarketplacePlugin): void { - const current = this.installedPlugins.get(); + this._pluginMetadata.set(pluginUri.toString(), plugin); + const current = this._installedPluginsStore.get(); const existing = current.find(e => isEqual(e.pluginUri, pluginUri)); if (existing) { // Still update to trigger watchers to re-check, something might have happened that we want to know about - this._installedPluginsStore.set(current.map(c => c === existing ? { pluginUri, plugin, enabled: existing.enabled } : c), undefined); + this._installedPluginsStore.set(current.map(c => c === existing ? { pluginUri, marketplace: plugin.marketplaceReference.rawValue } : c), undefined); } else { - this._installedPluginsStore.set([...current, { pluginUri, plugin, enabled: true }], undefined); + this._installedPluginsStore.set([...current, { pluginUri, marketplace: plugin.marketplaceReference.rawValue }], undefined); } } removeInstalledPlugin(pluginUri: URI): void { - const current = this.installedPlugins.get(); + this._pluginMetadata.delete(pluginUri.toString()); + const current = this._installedPluginsStore.get(); this._installedPluginsStore.set(current.filter(e => !isEqual(e.pluginUri, pluginUri)), undefined); } - setInstalledPluginEnabled(pluginUri: URI, enabled: boolean): void { - const current = this.installedPlugins.get(); - this._installedPluginsStore.set( - current.map(e => isEqual(e.pluginUri, pluginUri) ? { ...e, enabled } : e), - undefined, - ); - } - isMarketplaceTrusted(ref: IMarketplaceReference): boolean { return this._trustedMarketplacesStore.get().includes(ref.canonicalId); } + // --- Plugin metadata hydration ----------------------------------------------- + + /** + * For each plugin URI that has no cached metadata, walk up the directory + * tree from the plugin towards the agent-plugins root looking for a + * marketplace definition file. When found, read the marketplace plugins + * and match by source path to populate {@link _pluginMetadata}. + * + * After hydration completes the installed-plugins store is "touched" so + * that the derived {@link installedPlugins} observable re-evaluates with + * the newly available metadata. + */ + private async _hydratePluginMetadata(entries: readonly IStoredInstalledPlugin[]): Promise { + let hydrated = 0; + + for (const entry of entries) { + const key = entry.pluginUri.toString(); + if (this._pluginMetadata.has(key)) { + continue; + } + + const reference = parseMarketplaceReference(entry.marketplace); + if (!reference) { + this._logService.debug(`[PluginMarketplaceService] Cannot parse marketplace reference '${entry.marketplace}' for ${key}`); + continue; + } + + try { + const repoDir = this._pluginRepositoryService.getRepositoryUri(reference); + const plugins = await this._readPluginsFromDirectory(repoDir, reference); + const match = plugins.find(p => { + const installUri = this._pluginRepositoryService.getPluginInstallUri(p); + return isEqual(installUri, entry.pluginUri); + }); + if (match) { + this._pluginMetadata.set(key, match); + hydrated++; + } + } catch (err) { + this._logService.debug(`[PluginMarketplaceService] Failed to hydrate metadata for ${key}:`, err); + } + } + + if (hydrated > 0) { + // Touch the store to trigger the derived observable to re-evaluate + // now that _pluginMetadata has new entries. + const current = this._installedPluginsStore.get(); + this._installedPluginsStore.set([...current], undefined); + } + } + + /** + * Shared logic to parse a marketplace.json into {@link IMarketplacePlugin} + * objects. Used by both fetch and hydration paths. + */ + private _parseMarketplacePlugins(json: IMarketplaceJson, reference: IMarketplaceReference, marketplaceType: MarketplaceType, repoDir?: URI): IMarketplacePlugin[] { + if (!json.plugins || !Array.isArray(json.plugins)) { + return []; + } + + return json.plugins + .filter((p): p is { name: string; description?: string; version?: string; source?: string | IJsonPluginSource } => + typeof p.name === 'string' && !!p.name + ) + .flatMap(p => { + const sourceDescriptor = parsePluginSource(p.source, json.metadata?.pluginRoot, { + pluginName: p.name, + logService: this._logService, + logPrefix: '[PluginMarketplaceService]', + }); + if (!sourceDescriptor) { + return []; + } + + const source = sourceDescriptor.kind === PluginSourceKind.RelativePath ? sourceDescriptor.path : ''; + + return [{ + name: p.name, + description: p.description ?? '', + version: p.version ?? '', + source, + sourceDescriptor, + marketplace: reference.displayLabel, + marketplaceReference: reference, + marketplaceType, + readmeUri: repoDir ? getMarketplaceReadmeFileUri(repoDir, source) : getMarketplaceReadmeUri(reference.githubRepo ?? '', source), + }]; + }); + } + trustMarketplace(ref: IMarketplaceReference): void { const current = this._trustedMarketplacesStore.get(); if (!current.includes(ref.canonicalId)) { @@ -646,7 +710,7 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke this._updateCheckTimer = undefined; try { - const installed = this.installedPlugins.get().filter(e => e.enabled); + const installed = this.installedPlugins.get(); if (installed.length === 0) { return; } @@ -706,53 +770,36 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke } private async _readPluginsFromDirectory(repoDir: URI, reference: IMarketplaceReference, token?: CancellationToken): Promise { - for (const def of MARKETPLACE_DEFINITIONS) { + return this._readPluginsFromDefinitions(reference, async (defPath) => { if (token?.isCancellationRequested) { - return []; + return undefined; } - - const definitionUri = joinPath(repoDir, def.path); - let json: IMarketplaceJson | undefined; + const definitionUri = joinPath(repoDir, defPath); try { const contents = await this._fileService.readFile(definitionUri); - json = parseJSONC(contents.value.toString()) as IMarketplaceJson | undefined; + return parseJSONC(contents.value.toString()) as IMarketplaceJson | undefined; } catch { - continue; + return undefined; } + }, repoDir); + } + /** + * Iterates over {@link MARKETPLACE_DEFINITIONS} paths, calling + * {@link readJson} for each to obtain the parsed JSON. Returns the + * plugins from the first definition that yields a valid result. + */ + private async _readPluginsFromDefinitions( + reference: IMarketplaceReference, + readJson: (defPath: string) => Promise, + repoDir?: URI, + ): Promise { + for (const def of MARKETPLACE_DEFINITIONS) { + const json = await readJson(def.path); if (!json?.plugins || !Array.isArray(json.plugins)) { - this._logService.debug(`[PluginMarketplaceService] ${definitionUri.toString()} did not contain a valid plugins array, skipping`); continue; } - - return json.plugins - .filter((p): p is { name: string; description?: string; version?: string; source?: string | IJsonPluginSource } => - typeof p.name === 'string' && !!p.name - ) - .flatMap(p => { - const sourceDescriptor = parsePluginSource(p.source, json.metadata?.pluginRoot, { - pluginName: p.name, - logService: this._logService, - logPrefix: `[PluginMarketplaceService]`, - }); - if (!sourceDescriptor) { - return []; - } - - const source = sourceDescriptor.kind === PluginSourceKind.RelativePath ? sourceDescriptor.path : ''; - - return [{ - name: p.name, - description: p.description ?? '', - version: p.version ?? '', - source, - sourceDescriptor, - marketplace: reference.displayLabel, - marketplaceReference: reference, - marketplaceType: def.type, - readmeUri: getMarketplaceReadmeFileUri(repoDir, source), - }]; - }); + return this._parseMarketplacePlugins(json, reference, def.type, repoDir); } this._logService.debug(`[PluginMarketplaceService] No marketplace.json found in ${reference.rawValue}`); diff --git a/src/vs/workbench/contrib/chat/test/browser/plugins/agentPluginRepositoryService.test.ts b/src/vs/workbench/contrib/chat/test/browser/plugins/agentPluginRepositoryService.test.ts index 3e94219d5aed2..83057f077cb5c 100644 --- a/src/vs/workbench/contrib/chat/test/browser/plugins/agentPluginRepositoryService.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/plugins/agentPluginRepositoryService.test.ts @@ -59,7 +59,7 @@ suite('AgentPluginRepositoryService', () => { return undefined; }, } as unknown as ICommandService); - instantiationService.stub(IEnvironmentService, { cacheHome: URI.file('/cache') } as unknown as IEnvironmentService); + instantiationService.stub(IEnvironmentService, { cacheHome: URI.file('/cache'), agentPluginsHome: URI.file('/cache/agentPlugins') } as unknown as IEnvironmentService); instantiationService.stub(IFileService, fileService); instantiationService.stub(ILogService, new NullLogService()); instantiationService.stub(INotificationService, { notify: () => undefined } as unknown as INotificationService); @@ -136,7 +136,7 @@ suite('AgentPluginRepositoryService', () => { return undefined; }, } as unknown as ICommandService); - instantiationService.stub(IEnvironmentService, { cacheHome: URI.file('/cache') } as unknown as IEnvironmentService); + instantiationService.stub(IEnvironmentService, { cacheHome: URI.file('/cache'), agentPluginsHome: URI.file('/cache/agentPlugins') } as unknown as IEnvironmentService); instantiationService.stub(IFileService, fileService); instantiationService.stub(ILogService, new NullLogService()); instantiationService.stub(INotificationService, { notify: () => undefined } as unknown as INotificationService); @@ -176,7 +176,7 @@ suite('AgentPluginRepositoryService', () => { const instantiationService = store.add(new TestInstantiationService()); instantiationService.stub(ICommandService, { executeCommand: async () => undefined } as unknown as ICommandService); - instantiationService.stub(IEnvironmentService, { cacheHome: URI.file('/cache') } as unknown as IEnvironmentService); + instantiationService.stub(IEnvironmentService, { cacheHome: URI.file('/cache'), agentPluginsHome: URI.file('/cache/agentPlugins') } as unknown as IEnvironmentService); instantiationService.stub(IFileService, { exists: async () => true } as unknown as IFileService); instantiationService.stub(ILogService, new NullLogService()); instantiationService.stub(INotificationService, { notify: () => undefined } as unknown as INotificationService); @@ -270,7 +270,7 @@ suite('AgentPluginRepositoryService', () => { ) { const instantiationService = store.add(new TestInstantiationService()); instantiationService.stub(ICommandService, { executeCommand: async () => undefined } as unknown as ICommandService); - instantiationService.stub(IEnvironmentService, { cacheHome: URI.file('/cache') } as unknown as IEnvironmentService); + instantiationService.stub(IEnvironmentService, { cacheHome: URI.file('/cache'), agentPluginsHome: URI.file('/cache/agentPlugins') } as unknown as IEnvironmentService); instantiationService.stub(IFileService, { exists: async () => true, del: async (resource: URI) => { onDel(resource); }, @@ -363,7 +363,7 @@ suite('AgentPluginRepositoryService', () => { test('does not throw when delete fails', async () => { const instantiationService = store.add(new TestInstantiationService()); instantiationService.stub(ICommandService, { executeCommand: async () => undefined } as unknown as ICommandService); - instantiationService.stub(IEnvironmentService, { cacheHome: URI.file('/cache') } as unknown as IEnvironmentService); + instantiationService.stub(IEnvironmentService, { cacheHome: URI.file('/cache'), agentPluginsHome: URI.file('/cache/agentPlugins') } as unknown as IEnvironmentService); instantiationService.stub(IFileService, { exists: async () => true, del: async () => { throw new Error('permission denied'); }, diff --git a/src/vs/workbench/contrib/chat/test/common/plugins/fileBackedInstalledPluginsStore.test.ts b/src/vs/workbench/contrib/chat/test/common/plugins/fileBackedInstalledPluginsStore.test.ts new file mode 100644 index 0000000000000..30e966f9f69ef --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/common/plugins/fileBackedInstalledPluginsStore.test.ts @@ -0,0 +1,208 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { timeout } from '../../../../../../base/common/async.js'; +import { VSBuffer } from '../../../../../../base/common/buffer.js'; +import { Event } from '../../../../../../base/common/event.js'; +import { URI } from '../../../../../../base/common/uri.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; +import { IFileService, IFileSystemWatcher } from '../../../../../../platform/files/common/files.js'; +import { NullLogService } from '../../../../../../platform/log/common/log.js'; +import { InMemoryStorageService, IStorageService, StorageScope, StorageTarget } from '../../../../../../platform/storage/common/storage.js'; +import { FileBackedInstalledPluginsStore } from '../../../common/plugins/fileBackedInstalledPluginsStore.js'; +import { MarketplaceType, PluginSourceKind, parseMarketplaceReference } from '../../../common/plugins/pluginMarketplaceService.js'; + +const LEGACY_INSTALLED_PLUGINS_STORAGE_KEY = 'chat.plugins.installed.v1'; +const LEGACY_MARKETPLACE_INDEX_STORAGE_KEY = 'chat.plugins.marketplaces.index.v1'; + +class TestFileService { + private readonly _files = new Map(); + private readonly _folders = new Set(); + + constructor(private readonly _failWrites = false) { } + + async exists(resource: URI): Promise { + const key = resource.toString(); + return this._files.has(key) || this._folders.has(key); + } + + async readFile(resource: URI): Promise<{ value: VSBuffer }> { + const key = resource.toString(); + const value = this._files.get(key); + if (value === undefined) { + throw new Error(`Missing file: ${key}`); + } + return { value: VSBuffer.fromString(value) }; + } + + async writeFile(resource: URI, content: VSBuffer): Promise { + if (this._failWrites) { + throw new Error('write failed'); + } + + this._files.set(resource.toString(), content.toString()); + return {}; + } + + async createFolder(resource: URI): Promise { + this._folders.add(resource.toString()); + return {}; + } + + createWatcher(): IFileSystemWatcher { + return { + onDidChange: Event.None, + dispose: () => { }, + }; + } + + setFile(resource: URI, content: string): void { + this._files.set(resource.toString(), content); + } + + getFile(resource: URI): string | undefined { + return this._files.get(resource.toString()); + } +} + +suite('FileBackedInstalledPluginsStore', () => { + const store = ensureNoDisposablesAreLeakedInTestSuite(); + + function createLegacyEntry(pluginPath: string) { + const reference = parseMarketplaceReference('microsoft/plugins'); + assert.ok(reference); + + return { + pluginUri: URI.file(pluginPath), + plugin: { + name: 'my-plugin', + description: 'A plugin', + version: '1.0.0', + source: 'plugins/my-plugin', + sourceDescriptor: { kind: PluginSourceKind.RelativePath, path: 'plugins/my-plugin' } as const, + marketplace: reference.displayLabel, + marketplaceReference: reference, + marketplaceType: MarketplaceType.Copilot, + }, + enabled: true, + }; + } + + async function waitFor(predicate: () => boolean): Promise { + for (let i = 0; i < 40; i++) { + if (predicate()) { + return; + } + await timeout(10); + } + + assert.fail('Condition not met in time'); + } + + test('migrates legacy storage to installed.json and removes legacy keys', async () => { + const storageService = store.add(new InMemoryStorageService()); + const fileService = new TestFileService(); + + const legacyEntry = createLegacyEntry('/cache/agentPlugins/github.com/microsoft/plugins/plugins/my-plugin'); + storageService.store( + LEGACY_INSTALLED_PLUGINS_STORAGE_KEY, + JSON.stringify([legacyEntry]), + StorageScope.APPLICATION, + StorageTarget.MACHINE, + ); + + const agentPluginsHome = URI.file('/home/user/.vscode/agent-plugins'); + const installedJson = URI.joinPath(agentPluginsHome, 'installed.json'); + + const pluginsStore = store.add(new FileBackedInstalledPluginsStore( + agentPluginsHome, + URI.file('/cache/agentPlugins'), + fileService as unknown as IFileService, + new NullLogService(), + storageService as unknown as IStorageService, + )); + + await waitFor(() => !!fileService.getFile(installedJson)); + + const serialized = fileService.getFile(installedJson); + assert.ok(serialized); + const parsed = JSON.parse(serialized!); + assert.strictEqual(parsed.version, 1); + assert.strictEqual(parsed.installed.length, 1); + assert.ok(parsed.installed[0].pluginUri.includes('/home/user/.vscode/agent-plugins/github.com/microsoft/plugins/plugins/my-plugin')); + // plugin metadata is NOT stored in the file + assert.strictEqual(parsed.installed[0].plugin, undefined); + assert.strictEqual(pluginsStore.get().length, 1); + + assert.strictEqual(storageService.get(LEGACY_INSTALLED_PLUGINS_STORAGE_KEY, StorageScope.APPLICATION), undefined); + assert.strictEqual(storageService.get(LEGACY_MARKETPLACE_INDEX_STORAGE_KEY, StorageScope.APPLICATION), undefined); + }); + + test('keeps legacy keys when migration write fails', async () => { + const storageService = store.add(new InMemoryStorageService()); + const fileService = new TestFileService(true); + + const legacyEntry = createLegacyEntry('/cache/agentPlugins/github.com/microsoft/plugins/plugins/my-plugin'); + storageService.store( + LEGACY_INSTALLED_PLUGINS_STORAGE_KEY, + JSON.stringify([legacyEntry]), + StorageScope.APPLICATION, + StorageTarget.MACHINE, + ); + + storageService.store( + LEGACY_MARKETPLACE_INDEX_STORAGE_KEY, + JSON.stringify({ any: 'value' }), + StorageScope.APPLICATION, + StorageTarget.MACHINE, + ); + + const agentPluginsHome = URI.file('/home/user/.vscode/agent-plugins'); + const installedJson = URI.joinPath(agentPluginsHome, 'installed.json'); + + store.add(new FileBackedInstalledPluginsStore( + agentPluginsHome, + URI.file('/cache/agentPlugins'), + fileService as unknown as IFileService, + new NullLogService(), + storageService as unknown as IStorageService, + )); + + await timeout(30); + + assert.strictEqual(fileService.getFile(installedJson), undefined); + assert.ok(storageService.get(LEGACY_INSTALLED_PLUGINS_STORAGE_KEY, StorageScope.APPLICATION)); + assert.ok(storageService.get(LEGACY_MARKETPLACE_INDEX_STORAGE_KEY, StorageScope.APPLICATION)); + }); + + test('loads existing installed.json on startup', async () => { + const storageService = store.add(new InMemoryStorageService()); + const fileService = new TestFileService(); + const agentPluginsHome = URI.file('/home/user/.vscode/agent-plugins'); + const installedJson = URI.joinPath(agentPluginsHome, 'installed.json'); + + const existingData = { + version: 1, + installed: [{ + pluginUri: URI.file('/home/user/.vscode/agent-plugins/github.com/microsoft/plugins/plugins/my-plugin').toString(), + marketplace: 'microsoft/plugins', + }], + }; + fileService.setFile(installedJson, JSON.stringify(existingData)); + + const pluginsStore = store.add(new FileBackedInstalledPluginsStore( + agentPluginsHome, + URI.file('/cache/agentPlugins'), + fileService as unknown as IFileService, + new NullLogService(), + storageService as unknown as IStorageService, + )); + + await waitFor(() => pluginsStore.get().length === 1); + + assert.strictEqual(pluginsStore.get()[0].pluginUri.path, '/home/user/.vscode/agent-plugins/github.com/microsoft/plugins/plugins/my-plugin'); + }); +}); diff --git a/src/vs/workbench/contrib/chat/test/common/plugins/pluginMarketplaceService.test.ts b/src/vs/workbench/contrib/chat/test/common/plugins/pluginMarketplaceService.test.ts index 49cda85236d61..90198314f32e0 100644 --- a/src/vs/workbench/contrib/chat/test/common/plugins/pluginMarketplaceService.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/plugins/pluginMarketplaceService.test.ts @@ -16,9 +16,10 @@ import { ILogService, NullLogService } from '../../../../../../platform/log/comm import { IRequestService } from '../../../../../../platform/request/common/request.js'; import { IStorageService, InMemoryStorageService } from '../../../../../../platform/storage/common/storage.js'; import { IWorkspaceTrustManagementService } from '../../../../../../platform/workspace/common/workspaceTrust.js'; +import { IEnvironmentService } from '../../../../../../platform/environment/common/environment.js'; import { ChatConfiguration } from '../../../common/constants.js'; import { IAgentPluginRepositoryService } from '../../../common/plugins/agentPluginRepositoryService.js'; -import { MarketplaceReferenceKind, MarketplaceType, PluginMarketplaceService, PluginSourceKind, getPluginSourceLabel, parseMarketplaceReference, parseMarketplaceReferences, parsePluginSource } from '../../../common/plugins/pluginMarketplaceService.js'; +import { IMarketplacePlugin, MarketplaceReferenceKind, MarketplaceType, PluginMarketplaceService, PluginSourceKind, getPluginSourceLabel, parseMarketplaceReference, parseMarketplaceReferences, parsePluginSource } from '../../../common/plugins/pluginMarketplaceService.js'; import { IWorkspacePluginSettingsService } from '../../../common/plugins/workspacePluginSettingsService.js'; suite('PluginMarketplaceService', () => { @@ -186,8 +187,9 @@ suite('PluginMarketplaceService - getMarketplacePluginMetadata', () => { [ChatConfiguration.PluginMarketplaces]: ['microsoft/plugins'], [ChatConfiguration.PluginsEnabled]: true, })); + instantiationService.stub(IEnvironmentService, { cacheHome: URI.file('/cache') } as Partial as IEnvironmentService); instantiationService.stub(IFileService, {} as unknown as IFileService); - instantiationService.stub(IAgentPluginRepositoryService, {} as unknown as IAgentPluginRepositoryService); + instantiationService.stub(IAgentPluginRepositoryService, { agentPluginsHome: URI.file('/agent-plugins') } as unknown as IAgentPluginRepositoryService); instantiationService.stub(ILogService, new NullLogService()); instantiationService.stub(IRequestService, {} as unknown as IRequestService); instantiationService.stub(IStorageService, store.add(new InMemoryStorageService())); @@ -236,6 +238,125 @@ suite('PluginMarketplaceService - getMarketplacePluginMetadata', () => { }); }); +suite('PluginMarketplaceService - installed plugins lifecycle', () => { + const store = ensureNoDisposablesAreLeakedInTestSuite(); + + const marketplaceRef = parseMarketplaceReference('microsoft/plugins')!; + + function makePlugin(name: string, source: string): IMarketplacePlugin { + return { + name, + description: `${name} description`, + version: '1.0.0', + source, + sourceDescriptor: { kind: PluginSourceKind.RelativePath, path: source } as const, + marketplace: marketplaceRef.displayLabel, + marketplaceReference: marketplaceRef, + marketplaceType: MarketplaceType.Copilot, + }; + } + + function createService(): PluginMarketplaceService { + const instantiationService = store.add(new TestInstantiationService()); + + instantiationService.stub(IConfigurationService, new TestConfigurationService({ + [ChatConfiguration.PluginMarketplaces]: ['microsoft/plugins'], + [ChatConfiguration.PluginsEnabled]: true, + })); + instantiationService.stub(IEnvironmentService, { cacheHome: URI.file('/cache') } as Partial as IEnvironmentService); + instantiationService.stub(IFileService, {} as unknown as IFileService); + instantiationService.stub(IAgentPluginRepositoryService, { agentPluginsHome: URI.file('/agent-plugins') } as unknown as IAgentPluginRepositoryService); + instantiationService.stub(ILogService, new NullLogService()); + instantiationService.stub(IRequestService, {} as unknown as IRequestService); + instantiationService.stub(IStorageService, store.add(new InMemoryStorageService())); + instantiationService.stub(IWorkspacePluginSettingsService, { + extraMarketplaces: observableValue('test.extraMarketplaces', []), + enabledPlugins: observableValue('test.enabledPlugins', new Map()), + } as Partial as IWorkspacePluginSettingsService); + instantiationService.stub(IWorkspaceTrustManagementService, { + isWorkspaceTrusted: () => true, + onDidChangeTrust: Event.None, + } as Partial as IWorkspaceTrustManagementService); + + return store.add(instantiationService.createInstance(PluginMarketplaceService)); + } + + test('installedPlugins observable is empty with no plugins', () => { + const service = createService(); + assert.deepStrictEqual(service.installedPlugins.get(), []); + }); + + test('addInstalledPlugin makes plugin appear in installedPlugins', () => { + const service = createService(); + const uri = URI.file('/agent-plugins/github.com/microsoft/plugins/my-plugin'); + const plugin = makePlugin('my-plugin', 'my-plugin'); + + service.addInstalledPlugin(uri, plugin); + + const installed = service.installedPlugins.get(); + assert.strictEqual(installed.length, 1); + assert.strictEqual(installed[0].plugin.name, 'my-plugin'); + }); + + test('removeInstalledPlugin removes plugin from installedPlugins and metadata', () => { + const service = createService(); + const uri = URI.file('/agent-plugins/github.com/microsoft/plugins/my-plugin'); + const plugin = makePlugin('my-plugin', 'my-plugin'); + + service.addInstalledPlugin(uri, plugin); + assert.strictEqual(service.installedPlugins.get().length, 1); + + service.removeInstalledPlugin(uri); + assert.strictEqual(service.installedPlugins.get().length, 0); + assert.strictEqual(service.getMarketplacePluginMetadata(uri), undefined); + }); + + test('addInstalledPlugin updates metadata for existing entry', () => { + const service = createService(); + const uri = URI.file('/agent-plugins/github.com/microsoft/plugins/my-plugin'); + const v1 = makePlugin('my-plugin', 'my-plugin'); + const v2 = { ...v1, version: '2.0.0', description: 'updated' }; + + service.addInstalledPlugin(uri, v1); + service.addInstalledPlugin(uri, v2); + + const installed = service.installedPlugins.get(); + assert.strictEqual(installed.length, 1); + assert.strictEqual(installed[0].plugin.version, '2.0.0'); + assert.strictEqual(installed[0].plugin.description, 'updated'); + }); + + test('getMarketplacePluginMetadata finds metadata for child URI', () => { + const service = createService(); + const uri = URI.file('/agent-plugins/github.com/microsoft/plugins'); + const plugin = makePlugin('my-plugin', 'my-plugin'); + + service.addInstalledPlugin(uri, plugin); + + const childUri = URI.file('/agent-plugins/github.com/microsoft/plugins/subdir/file.ts'); + const result = service.getMarketplacePluginMetadata(childUri); + assert.strictEqual(result?.name, 'my-plugin'); + }); + + test('multiple plugins can be installed independently', () => { + const service = createService(); + const uri1 = URI.file('/agent-plugins/github.com/microsoft/plugins/plugin-a'); + const uri2 = URI.file('/agent-plugins/github.com/microsoft/plugins/plugin-b'); + const pluginA = makePlugin('plugin-a', 'plugin-a'); + const pluginB = makePlugin('plugin-b', 'plugin-b'); + + service.addInstalledPlugin(uri1, pluginA); + service.addInstalledPlugin(uri2, pluginB); + + assert.strictEqual(service.installedPlugins.get().length, 2); + + service.removeInstalledPlugin(uri1); + const remaining = service.installedPlugins.get(); + assert.strictEqual(remaining.length, 1); + assert.strictEqual(remaining[0].plugin.name, 'plugin-b'); + }); +}); + suite('parsePluginSource', () => { ensureNoDisposablesAreLeakedInTestSuite(); diff --git a/src/vs/workbench/services/environment/browser/environmentService.ts b/src/vs/workbench/services/environment/browser/environmentService.ts index a42fb531bdf51..83d722c7e6ca4 100644 --- a/src/vs/workbench/services/environment/browser/environmentService.ts +++ b/src/vs/workbench/services/environment/browser/environmentService.ts @@ -144,6 +144,9 @@ export class BrowserWorkbenchEnvironmentService implements IBrowserWorkbenchEnvi @memoize get extHostLogsPath(): URI { return joinPath(this.logsHome, 'exthost'); } + @memoize + get agentPluginsHome(): URI { return joinPath(this.userRoamingDataHome, 'agent-plugins'); } + private extensionHostDebugEnvironment: IExtensionHostDebugEnvironment | undefined = undefined; @memoize diff --git a/src/vs/workbench/services/environment/common/environmentService.ts b/src/vs/workbench/services/environment/common/environmentService.ts index 3ad7edaa43405..7f799fa21c28e 100644 --- a/src/vs/workbench/services/environment/common/environmentService.ts +++ b/src/vs/workbench/services/environment/common/environmentService.ts @@ -26,6 +26,7 @@ export interface IWorkbenchEnvironmentService extends IEnvironmentService { readonly logFile: URI; readonly windowLogsPath: URI; readonly extHostLogsPath: URI; + readonly agentPluginsHome: URI; // --- Extensions readonly extensionEnabledProposedApi?: string[]; diff --git a/src/vs/workbench/services/environment/electron-browser/environmentService.ts b/src/vs/workbench/services/environment/electron-browser/environmentService.ts index 9d08b14460784..8c0080ace5d75 100644 --- a/src/vs/workbench/services/environment/electron-browser/environmentService.ts +++ b/src/vs/workbench/services/environment/electron-browser/environmentService.ts @@ -154,6 +154,9 @@ export class NativeWorkbenchEnvironmentService extends AbstractNativeEnvironment @memoize get isSessionsWindow(): boolean { return !!this.configuration.isSessionsWindow; } + @memoize + get agentPluginsHome(): URI { return URI.file(this.agentPluginsPath); } + constructor( private readonly configuration: INativeWindowConfiguration, productService: IProductService From 6afe980a82f8b85e63aba9c4ca428044b02e2e3d Mon Sep 17 00:00:00 2001 From: dileepyavan <52841896+dileepyavan@users.noreply.github.com> Date: Thu, 26 Mar 2026 22:49:29 -0700 Subject: [PATCH 06/17] Preserve $TMPDIR when retrying terminal commands outside the sandbox (#304601) * Changes to include environment variables for retry without sandboxing * adding e2e tests * fixing code formatting --- .../chat.runInTerminal.test.ts | 50 ++++++++++++++++--- .../commandLineSandboxRewriter.ts | 5 +- .../common/terminalSandboxService.ts | 9 +++- .../browser/terminalSandboxService.test.ts | 14 ++++++ .../commandLineSandboxRewriter.test.ts | 15 +++--- .../runInTerminalTool.test.ts | 6 +-- 6 files changed, 76 insertions(+), 23 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/chat.runInTerminal.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/chat.runInTerminal.test.ts index 5d9876eccba91..b70c97299845a 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/chat.runInTerminal.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/chat.runInTerminal.test.ts @@ -71,7 +71,7 @@ function extractTextContent(result: vscode.LanguageModelToolResult): string { participantRegistered = false; pendingResult = undefined; pendingCommand = undefined; - pendingTimeout = undefined; + pendingOptions = undefined; const chatToolsConfig = vscode.workspace.getConfiguration('chat.tools.global'); await chatToolsConfig.update('autoApprove', undefined, vscode.ConfigurationTarget.Global); @@ -82,10 +82,16 @@ function extractTextContent(result: vscode.LanguageModelToolResult): string { * Helper: invokes run_in_terminal via a chat participant and returns the tool result text. * Each call creates a new chat session to avoid participant re-registration issues. */ + interface RunInTerminalOptions { + timeout?: number; + requestUnsandboxedExecution?: boolean; + requestUnsandboxedExecutionReason?: string; + } + let participantRegistered = false; let pendingResult: DeferredPromise | undefined; let pendingCommand: string | undefined; - let pendingTimeout: number | undefined; + let pendingOptions: RunInTerminalOptions | undefined; function setupParticipant() { if (participantRegistered) { @@ -98,10 +104,10 @@ function extractTextContent(result: vscode.LanguageModelToolResult): string { } const currentResult = pendingResult; const currentCommand = pendingCommand; - const currentTimeout = pendingTimeout ?? 15000; + const currentOptions = pendingOptions ?? {}; pendingResult = undefined; pendingCommand = undefined; - pendingTimeout = undefined; + pendingOptions = undefined; try { const result = await vscode.lm.invokeTool('run_in_terminal', { input: { @@ -109,7 +115,11 @@ function extractTextContent(result: vscode.LanguageModelToolResult): string { explanation: 'Integration test command', goal: 'Test run_in_terminal output', isBackground: false, - timeout: currentTimeout + timeout: currentOptions.timeout ?? 15000, + ...currentOptions.requestUnsandboxedExecution ? { + requestUnsandboxedExecution: true, + requestUnsandboxedExecutionReason: currentOptions.requestUnsandboxedExecutionReason, + } : {}, }, toolInvocationToken: request.toolInvocationToken, }); @@ -122,13 +132,18 @@ function extractTextContent(result: vscode.LanguageModelToolResult): string { disposables.push(participant); } - async function invokeRunInTerminal(command: string, timeout = 15000): Promise { + async function invokeRunInTerminal(command: string, options?: RunInTerminalOptions): Promise; + async function invokeRunInTerminal(command: string, timeout?: number): Promise; + async function invokeRunInTerminal(command: string, optionsOrTimeout?: RunInTerminalOptions | number): Promise { setupParticipant(); + const opts: RunInTerminalOptions = typeof optionsOrTimeout === 'number' + ? { timeout: optionsOrTimeout } + : optionsOrTimeout ?? {}; const resultPromise = new DeferredPromise(); pendingResult = resultPromise; pendingCommand = command; - pendingTimeout = timeout; + pendingOptions = opts; await vscode.commands.executeCommand('workbench.action.chat.newChat'); vscode.commands.executeCommand('workbench.action.chat.open', { query: '@participant test' }); @@ -326,6 +341,27 @@ function extractTextContent(result: vscode.LanguageModelToolResult): string { assert.ok(acceptable.includes(output.trim()), `Unexpected output: ${JSON.stringify(output.trim())}`); }); + test('requestUnsandboxedExecution preserves sandbox $TMPDIR', async function () { + this.timeout(60000); + + const marker = `SANDBOX_UNSANDBOX_${Date.now()}`; + const sentinelName = `sentinel-${marker}.txt`; + + // Step 1: Write a sentinel file into the sandbox-provided $TMPDIR. + const writeOutput = await invokeRunInTerminal(`echo ${marker} > "$TMPDIR/${sentinelName}" && echo ${marker}`); + assert.strictEqual(writeOutput.trim(), marker); + + // Step 2: Retry with requestUnsandboxedExecution=true while sandbox + // stays enabled. The tool should preserve $TMPDIR from the sandbox so + // the sentinel file created in step 1 is still accessible. + const retryOutput = await invokeRunInTerminal(`cat "$TMPDIR/${sentinelName}"`, { + timeout: 30000, + requestUnsandboxedExecution: true, + requestUnsandboxedExecutionReason: 'Need to verify $TMPDIR persists on unsandboxed retry', + }); + assert.strictEqual(retryOutput.trim(), marker); + }); + test('cannot write to /tmp', async function () { this.timeout(60000); diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineRewriter/commandLineSandboxRewriter.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineRewriter/commandLineSandboxRewriter.ts index db6e961052696..2a0ff12f2da27 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineRewriter/commandLineSandboxRewriter.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineRewriter/commandLineSandboxRewriter.ts @@ -15,9 +15,6 @@ export class CommandLineSandboxRewriter extends Disposable implements ICommandLi } async rewrite(options: ICommandLineRewriterOptions): Promise { - if (options.requestUnsandboxedExecution) { - return undefined; - } if (!(await this._sandboxService.isEnabled())) { return undefined; @@ -30,7 +27,7 @@ export class CommandLineSandboxRewriter extends Disposable implements ICommandLi return undefined; } - const wrappedCommand = this._sandboxService.wrapCommand(options.commandLine); + const wrappedCommand = this._sandboxService.wrapCommand(options.commandLine, options.requestUnsandboxedExecution); return { rewritten: wrappedCommand, reasoning: 'Wrapped command for sandbox execution', diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandboxService.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandboxService.ts index 71cd4d8fbd571..71b4fe63c5c71 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandboxService.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandboxService.ts @@ -36,7 +36,7 @@ export interface ITerminalSandboxService { readonly _serviceBrand: undefined; isEnabled(): Promise; getOS(): Promise; - wrapCommand(command: string): string; + wrapCommand(command: string, requestUnsandboxedExecution?: boolean): string; getSandboxConfigPath(forceRefresh?: boolean): Promise; getTempDir(): URI | undefined; setNeedsForceUpdateConfigFile(): void; @@ -127,10 +127,15 @@ export class TerminalSandboxService extends Disposable implements ITerminalSandb return this._os; } - public wrapCommand(command: string): string { + public wrapCommand(command: string, requestUnsandboxedExecution?: boolean): string { if (!this._sandboxConfigPath || !this._tempDir) { throw new Error('Sandbox config path or temp dir not initialized'); } + // If requestUnsandboxedExecution is true, need to ensure env variables set during sandbox still apply. + if (requestUnsandboxedExecution) { + return this._tempDir?.path ? `(TMPDIR="${this._tempDir.path}"; export TMPDIR; ${command})` : command; + } + if (!this._execPath) { throw new Error('Executable path not set to run sandbox commands'); } diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/terminalSandboxService.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/terminalSandboxService.test.ts index a85db3988f2eb..d40734b1335be 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/terminalSandboxService.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/terminalSandboxService.test.ts @@ -388,6 +388,20 @@ suite('TerminalSandboxService - allowTrustedDomains', () => { ); }); + test('should preserve TMPDIR when unsandboxed execution is requested', async () => { + const sandboxService = store.add(instantiationService.createInstance(TerminalSandboxService)); + await sandboxService.getSandboxConfigPath(); + + strictEqual(sandboxService.wrapCommand('echo test', true), `(TMPDIR="${sandboxService.getTempDir()?.path}"; export TMPDIR; echo test)`); + }); + + test('should preserve TMPDIR for piped unsandboxed commands', async () => { + const sandboxService = store.add(instantiationService.createInstance(TerminalSandboxService)); + await sandboxService.getSandboxConfigPath(); + + strictEqual(sandboxService.wrapCommand('echo test | cat', true), `(TMPDIR="${sandboxService.getTempDir()?.path}"; export TMPDIR; echo test | cat)`); + }); + test('should pass wrapped command as a single quoted argument', async () => { const sandboxService = store.add(instantiationService.createInstance(TerminalSandboxService)); await sandboxService.getSandboxConfigPath(); diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineSandboxRewriter.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineSandboxRewriter.test.ts index 9a679676dd316..5af9d8f1d5665 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineSandboxRewriter.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineSandboxRewriter.test.ts @@ -22,7 +22,7 @@ suite('CommandLineSandboxRewriter', () => { instantiationService.stub(ITerminalSandboxService, { _serviceBrand: undefined, isEnabled: async () => false, - wrapCommand: command => command, + wrapCommand: (command, _requestUnsandboxedExecution) => command, getSandboxConfigPath: async () => '/tmp/sandbox.json', getTempDir: () => undefined, setNeedsForceUpdateConfigFile: () => { }, @@ -62,7 +62,7 @@ suite('CommandLineSandboxRewriter', () => { const calls: string[] = []; stubSandboxService({ isEnabled: async () => true, - wrapCommand: command => { + wrapCommand: (command, _requestUnsandboxedExecution) => { calls.push('wrapCommand'); return `wrapped:${command}`; }, @@ -79,12 +79,12 @@ suite('CommandLineSandboxRewriter', () => { deepStrictEqual(calls, ['getSandboxConfigPath', 'wrapCommand']); }); - test('does not wrap command when sandbox bypass was explicitly requested', async () => { + test('wraps command and forwards sandbox bypass flag when explicitly requested', async () => { const calls: string[] = []; stubSandboxService({ isEnabled: async () => true, - wrapCommand: command => { - calls.push(`wrap:${command}`); + wrapCommand: (command, requestUnsandboxedExecution) => { + calls.push(`wrap:${command}:${String(requestUnsandboxedExecution)}`); return `wrapped:${command}`; }, getSandboxConfigPath: async () => { @@ -99,7 +99,8 @@ suite('CommandLineSandboxRewriter', () => { requestUnsandboxedExecution: true, }); - strictEqual(result, undefined); - deepStrictEqual(calls, []); + strictEqual(result?.rewritten, 'wrapped:echo hello'); + strictEqual(result?.reasoning, 'Wrapped command for sandbox execution'); + deepStrictEqual(calls, ['config', 'wrap:echo hello:true']); }); }); diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts index 77639dedd5c7b..7ba8cf18603a4 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts @@ -121,7 +121,7 @@ suite('RunInTerminalTool', () => { terminalSandboxService = { _serviceBrand: undefined, isEnabled: async () => sandboxEnabled, - wrapCommand: (command: string) => `sandbox:${command}`, + wrapCommand: (command: string, requestUnsandboxedExecution?: boolean) => requestUnsandboxedExecution ? `unsandboxed:${command}` : `sandbox:${command}`, getSandboxConfigPath: async () => sandboxEnabled ? '/tmp/sandbox.json' : undefined, getTempDir: () => undefined, setNeedsForceUpdateConfigFile: () => { }, @@ -552,7 +552,7 @@ suite('RunInTerminalTool', () => { const terminalData = result?.toolSpecificData as IChatTerminalToolInvocationData; strictEqual(terminalData.requestUnsandboxedExecution, true); strictEqual(terminalData.requestUnsandboxedExecutionReason, 'Needs network access outside the sandbox'); - strictEqual(terminalData.commandLine.toolEdited, undefined); + strictEqual(terminalData.commandLine.toolEdited, 'unsandboxed:echo hello'); const confirmationMessage = result?.confirmationMessages?.message; ok(confirmationMessage && typeof confirmationMessage !== 'string'); @@ -566,7 +566,7 @@ suite('RunInTerminalTool', () => { ok(actions, 'Expected custom actions to be defined'); strictEqual(actions.length, 11); ok(!isSeparator(actions[0])); - strictEqual(actions[0].label, 'Allow `echo …` in this Session'); + strictEqual(actions[0].label, 'Allow `unsandboxed:echo …` in this Session'); ok(!isSeparator(actions[4])); strictEqual(actions[4].label, 'Allow Exact Command Line in this Session'); ok(!isSeparator(actions[10])); From 3bafa7dc2a2e035e832f4c0a21e20dfcf2be8f81 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Fri, 27 Mar 2026 01:51:59 -0400 Subject: [PATCH 07/17] fix for chat tips (#304899) fixes #303283 --- .../contrib/chat/browser/chatTipService.ts | 6 +---- .../chat/test/browser/chatTipService.test.ts | 24 +++++++++++++++---- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatTipService.ts b/src/vs/workbench/contrib/chat/browser/chatTipService.ts index fc596d0a73280..ae39102fb759a 100644 --- a/src/vs/workbench/contrib/chat/browser/chatTipService.ts +++ b/src/vs/workbench/contrib/chat/browser/chatTipService.ts @@ -849,14 +849,10 @@ export class ChatTipService extends Disposable implements IChatTipService { return; } const enabledCommandSet = new Set(enabledCommands); - const dismissCommandSet = new Set(tip.dismissWhenCommandsClicked); this._tipCommandListener.value = this._commandService.onDidExecuteCommand(e => { if (enabledCommandSet.has(e.commandId) && this._shownTip?.id === tip.id) { this._logTipTelemetry(tip.id, 'commandClicked', e.commandId); - if (dismissCommandSet.has(e.commandId)) { - this.dismissTip(); - } - this.hideTipsForSession(); + this.dismissTip(); } }); } diff --git a/src/vs/workbench/contrib/chat/test/browser/chatTipService.test.ts b/src/vs/workbench/contrib/chat/test/browser/chatTipService.test.ts index dc13e3e902e82..21eca814c6297 100644 --- a/src/vs/workbench/contrib/chat/test/browser/chatTipService.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/chatTipService.test.ts @@ -37,7 +37,7 @@ import { Range } from '../../../../../editor/common/core/range.js'; import { ITelemetryService } from '../../../../../platform/telemetry/common/telemetry.js'; import { NullTelemetryService } from '../../../../../platform/telemetry/common/telemetryUtils.js'; import { localChatSessionType } from '../../common/chatSessionsService.js'; -import { GENERATE_AGENT_INSTRUCTIONS_COMMAND_ID } from '../../browser/actions/chatActions.js'; +import { GENERATE_AGENT_INSTRUCTIONS_COMMAND_ID, GENERATE_PROMPT_COMMAND_ID } from '../../browser/actions/chatActions.js'; class MockContextKeyServiceWithRulesMatching extends MockContextKeyService { override contextMatchesRules(rules: ContextKeyExpression): boolean { @@ -1484,13 +1484,29 @@ suite('ChatTipService', () => { commandExecutedEmitter.fire({ commandId: 'workbench.action.openSettings', args: [] }); assert.strictEqual(dismissed, true, `${tipId} should dismiss when its settings command is clicked`); - assert.strictEqual(service.getWelcomeTip(contextKeyService), undefined, 'Tips should hide for the rest of the session after actioning a tip'); + assert.notStrictEqual(service.getWelcomeTip(contextKeyService)?.id, tipId, `${tipId} should not be shown again after actioning its command link`); - service.resetSession(); - assertTipNeverShown(service, tipId); + const nextService = createService(); + assertTipNeverShown(nextService, tipId); }); } + test('dismisses createPrompt tip after clicking its command link', () => { + const service = createService(); + contextKeyService.createKey(ChatContextKeys.chatSessionType.key, localChatSessionType); + + const tip = findTipById(service, 'tip.createPrompt'); + assert.ok(tip, 'Should show tip.createPrompt before command click'); + assert.ok(tip.enabledCommands?.includes(GENERATE_PROMPT_COMMAND_ID), 'Tip should enable the create prompt command'); + + commandExecutedEmitter.fire({ commandId: GENERATE_PROMPT_COMMAND_ID, args: [] }); + + assert.notStrictEqual(service.getWelcomeTip(contextKeyService)?.id, 'tip.createPrompt', 'tip.createPrompt should not be shown again after actioning its command link'); + + const nextService = createService(); + assertTipNeverShown(nextService, 'tip.createPrompt'); + }); + test('logs telemetry when tip is shown', () => { const events: { eventName: string; data: Record }[] = []; instantiationService.stub(ITelemetryService, { From befae3eaffeb27dff48b00a22587bfc2eac73a72 Mon Sep 17 00:00:00 2001 From: xingsy97 <87063252+xingsy97@users.noreply.github.com> Date: Fri, 27 Mar 2026 14:10:14 +0800 Subject: [PATCH 08/17] timeline: fix memory leak when toggling pane visibility (#304668) * timeline: dispose previous visibility subscriptions before recreating * retrigger CI --------- Co-authored-by: Raymond Zhao <7199958+rzhao271@users.noreply.github.com> --- src/vs/workbench/contrib/timeline/browser/timelinePane.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vs/workbench/contrib/timeline/browser/timelinePane.ts b/src/vs/workbench/contrib/timeline/browser/timelinePane.ts index c7c9cc7f99be4..7219c0991f1c7 100644 --- a/src/vs/workbench/contrib/timeline/browser/timelinePane.ts +++ b/src/vs/workbench/contrib/timeline/browser/timelinePane.ts @@ -883,6 +883,7 @@ export class TimelinePane extends ViewPane { override setVisible(visible: boolean): void { if (visible) { this.extensionService.activateByEvent('onView:timeline'); + this.visibilityDisposables?.dispose(); this.visibilityDisposables = new DisposableStore(); this.editorService.onDidActiveEditorChange(this.onActiveEditorChanged, this, this.visibilityDisposables); From d1058a088dd39c83cdde558fcd913f24bb66da5b Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Fri, 27 Mar 2026 07:45:20 +0100 Subject: [PATCH 09/17] debt - clean up some todos (#305530) --- .../browser/configuration.contribution.ts | 2 +- .../browser/workbench.contribution.ts | 2 +- .../agentSessions/agentSessionsActions.ts | 18 +++++++--------- .../agentSessions/agentSessionsModel.ts | 21 ++++--------------- .../electron-browser/chat.contribution.ts | 7 ------- 5 files changed, 13 insertions(+), 37 deletions(-) diff --git a/src/vs/sessions/contrib/configuration/browser/configuration.contribution.ts b/src/vs/sessions/contrib/configuration/browser/configuration.contribution.ts index 04711b328a93e..77f793a49f4f1 100644 --- a/src/vs/sessions/contrib/configuration/browser/configuration.contribution.ts +++ b/src/vs/sessions/contrib/configuration/browser/configuration.contribution.ts @@ -31,7 +31,7 @@ Registry.as(Extensions.Configuration).registerDefaultCon 'files.watcherExclude': { '**/.git/objects/**': true, '**/.git/subtree-cache/**': true, - '**/node_modules/*/**': true /* TODO@bpasero see if this helps improve perf */, + '**/node_modules/*/**': true, '**/.hg/store/**': true }, diff --git a/src/vs/workbench/browser/workbench.contribution.ts b/src/vs/workbench/browser/workbench.contribution.ts index ae3425dec0877..9b12f14ef57e6 100644 --- a/src/vs/workbench/browser/workbench.contribution.ts +++ b/src/vs/workbench/browser/workbench.contribution.ts @@ -359,7 +359,7 @@ const registry = Registry.as(ConfigurationExtensions.Con localize('useModal.all', "All editors open in a centered modal overlay."), ], 'description': localize('useModal', "Controls whether editors open in a modal overlay."), - 'default': product.quality !== 'stable' ? 'some' : 'off', // TODO@bpasero figure out the default + 'default': 'some', tags: ['experimental'], experiment: { mode: 'auto' diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsActions.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsActions.ts index b4805d349a6e6..4b019ac5effe1 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsActions.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsActions.ts @@ -35,7 +35,6 @@ import { KeyCode, KeyMod } from '../../../../../base/common/keyCodes.js'; import { coalesce } from '../../../../../base/common/arrays.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; import { IPaneCompositePartService } from '../../../../services/panecomposite/browser/panecomposite.js'; -import { IWorkbenchEnvironmentService } from '../../../../services/environment/common/environmentService.js'; const AGENT_SESSIONS_CATEGORY = localize2('chatSessions', "Chat Agent Sessions"); @@ -500,19 +499,16 @@ export class ArchiveAgentSessionAction extends BaseAgentSessionAction { async runWithSessions(sessions: IAgentSession[], accessor: ServicesAccessor): Promise { const chatService = accessor.get(IChatService); const dialogService = accessor.get(IDialogService); - const environmentService = accessor.get(IWorkbenchEnvironmentService); // Archive all sessions for (const session of sessions) { - if (!environmentService.isSessionsWindow) { - const chatModel = chatService.getSession(session.resource); - if (chatModel && !await showClearEditingSessionConfirmation(chatModel, dialogService, { - isArchiveAction: true, - titleOverride: localize('archiveSession', "Archive chat with pending edits?"), - messageOverride: localize('archiveSessionDescription', "You have pending changes in this chat session.") - })) { - return; - } + const chatModel = chatService.getSession(session.resource); + if (chatModel && !await showClearEditingSessionConfirmation(chatModel, dialogService, { + isArchiveAction: true, + titleOverride: localize('archiveSession', "Archive chat with pending edits?"), + messageOverride: localize('archiveSessionDescription', "You have pending changes in this chat session.") + })) { + return; } session.setArchived(true); diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsModel.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsModel.ts index 76ee31512fe89..a23f358a8cf06 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsModel.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsModel.ts @@ -23,7 +23,6 @@ import { IStorageService, StorageScope, StorageTarget } from '../../../../../pla import { IWorkspaceContextService } from '../../../../../platform/workspace/common/workspace.js'; import { IWorkspaceTrustManagementService } from '../../../../../platform/workspace/common/workspaceTrust.js'; import { IChatEntitlementService } from '../../../../services/chat/common/chatEntitlementService.js'; -import { IWorkbenchEnvironmentService } from '../../../../services/environment/common/environmentService.js'; import { ILifecycleService } from '../../../../services/lifecycle/common/lifecycle.js'; import { Extensions, IOutputChannelRegistry, IOutputService } from '../../../../services/output/common/output.js'; import { ChatSessionStatus as AgentSessionStatus, IChatSessionFileChange, IChatSessionFileChange2, IChatSessionItem, IChatSessionsService, isSessionInProgressStatus, ResolvedChatSessionsExtensionPoint } from '../../common/chatSessionsService.js'; @@ -451,14 +450,7 @@ export class AgentSessionsModel extends Disposable implements IAgentSessionsMode get resolved(): boolean { return this._resolved; } private _sessions: ResourceMap; - get sessions(): IAgentSession[] { - const sessions = Array.from(this._sessions.values()); - if (this.environmentService.isSessionsWindow) { - return sessions.filter(session => session.providerType !== AgentSessionProviders.Claude && session.providerType !== AgentSessionProviders.Codex); // filter out sessions that can currently not be triggered in the sessions app (TODO@bpasero revisit later) - } - - return sessions; - } + get sessions(): IAgentSession[] { return Array.from(this._sessions.values()); } private readonly resolvers = this._register(new DisposableMap>()); @@ -475,7 +467,6 @@ export class AgentSessionsModel extends Disposable implements IAgentSessionsMode @IWorkspaceContextService private readonly workspaceContextService: IWorkspaceContextService, @IWorkspaceTrustManagementService private readonly workspaceTrustManagementService: IWorkspaceTrustManagementService, @IChatEntitlementService private readonly chatEntitlementService: IChatEntitlementService, - @IWorkbenchEnvironmentService private readonly environmentService: IWorkbenchEnvironmentService, ) { super(); @@ -810,9 +801,6 @@ interface ISerializedAgentSession { readonly created: number; readonly lastRequestStarted?: number; readonly lastRequestEnded?: number; - // Old format for backward compatibility when reading (TODO@bpasero remove eventually) - readonly startTime?: number; - readonly endTime?: number; }; readonly changes?: readonly IChatSessionFileChange[] | readonly IChatSessionFileChange2[] | { @@ -886,10 +874,9 @@ class AgentSessionsCache { archived: session.archived, timing: { - // Support loading both new and old cache formats (TODO@bpasero remove old format support after some time) - created: session.timing.created ?? session.timing.startTime ?? 0, - lastRequestStarted: session.timing.lastRequestStarted ?? session.timing.startTime, - lastRequestEnded: session.timing.lastRequestEnded ?? session.timing.endTime, + created: session.timing.created ?? 0, + lastRequestStarted: session.timing.lastRequestStarted, + lastRequestEnded: session.timing.lastRequestEnded, }, changes: Array.isArray(session.changes) ? session.changes.map((change: IChatSessionFileChange) => ({ diff --git a/src/vs/workbench/contrib/chat/electron-browser/chat.contribution.ts b/src/vs/workbench/contrib/chat/electron-browser/chat.contribution.ts index 76022ee368496..6fddb201f36e2 100644 --- a/src/vs/workbench/contrib/chat/electron-browser/chat.contribution.ts +++ b/src/vs/workbench/contrib/chat/electron-browser/chat.contribution.ts @@ -18,7 +18,6 @@ import { IContextKeyService } from '../../../../platform/contextkey/common/conte import { IDialogService } from '../../../../platform/dialogs/common/dialogs.js'; import { ILogService } from '../../../../platform/log/common/log.js'; import { INativeHostService } from '../../../../platform/native/common/native.js'; -import { IProductService } from '../../../../platform/product/common/productService.js'; import { IWorkspaceTrustRequestService } from '../../../../platform/workspace/common/workspaceTrust.js'; import { WorkbenchPhase, registerWorkbenchContribution2 } from '../../../common/contributions.js'; import { IViewsService } from '../../../services/views/common/viewsService.js'; @@ -120,8 +119,6 @@ class ChatSuspendThrottlingHandler extends Disposable { constructor( @INativeHostService nativeHostService: INativeHostService, @IChatService chatService: IChatService, - @IProductService productService: IProductService, - @ILogService logService: ILogService, ) { super(); @@ -132,10 +129,6 @@ class ChatSuspendThrottlingHandler extends Disposable { // throttling is not applied so that the chat session can continue // even when the window is not in focus. nativeHostService.setBackgroundThrottling(!running); - - if (productService.quality === 'insider') { // TODO@bpasero remove me in the future - logService.info(`[ChatSuspendThrottlingHandler] background throttling ${running ? 'suspended' : 'resumed'}`); - } })); } } From d6638bd41d55884ce92e3357b3c2bb12a45a43dc Mon Sep 17 00:00:00 2001 From: Justin Chen <54879025+justschen@users.noreply.github.com> Date: Fri, 27 Mar 2026 00:37:22 -0700 Subject: [PATCH 10/17] fix some thinking content rendering for edits + lazy markdown not rendering (#305531) * fix some thinking content rendering * don't skip hasValue --- .../chatThinkingContentPart.ts | 30 ++++++++++++------- .../chat/browser/widget/chatListRenderer.ts | 2 +- .../chatThinkingContentPart.test.ts | 6 ++-- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts index a1a053d2968c6..32dc64e4497f9 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts @@ -690,17 +690,21 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen labelElement.appendChild(restSpan); } - // Show aggregated diff stats from edit pills + // Show aggregated diff stats from edit pills (only when there are actual changes) if (this.diffStatsByPartId.size > 0) { const { added, removed } = this._aggregatedDiff; - const diffContainer = $('span.chat-thinking-title-diff'); - diffContainer.appendChild($('span.label-added', {}, `+${added}`)); - diffContainer.appendChild($('span.label-removed', {}, `-${removed}`)); - labelElement.appendChild(diffContainer); - - const insertionsFragment = added === 1 ? localize('chat.thinking.insertions.one', "1 insertion") : localize('chat.thinking.insertions', "{0} insertions", added); - const deletionsFragment = removed === 1 ? localize('chat.thinking.deletions.one', "1 deletion") : localize('chat.thinking.deletions', "{0} deletions", removed); - this._collapseButton.element.ariaLabel = localize('chat.thinking.titleWithDiff', "{0}, {1}, {2}", title, insertionsFragment, deletionsFragment); + if (added > 0 || removed > 0) { + const diffContainer = $('span.chat-thinking-title-diff'); + diffContainer.appendChild($('span.label-added', {}, `+${added}`)); + diffContainer.appendChild($('span.label-removed', {}, `-${removed}`)); + labelElement.appendChild(diffContainer); + + const insertionsFragment = added === 1 ? localize('chat.thinking.insertions.one', "1 insertion") : localize('chat.thinking.insertions', "{0} insertions", added); + const deletionsFragment = removed === 1 ? localize('chat.thinking.deletions.one', "1 deletion") : localize('chat.thinking.deletions', "{0} deletions", removed); + this._collapseButton.element.ariaLabel = localize('chat.thinking.titleWithDiff', "{0}, {1}, {2}", title, insertionsFragment, deletionsFragment); + } else { + this._collapseButton.element.ariaLabel = title; + } } else { this._collapseButton.element.ariaLabel = title; } @@ -1803,7 +1807,13 @@ ${this.hookCount > 0 ? `EXAMPLES WITH BLOCKED CONTENT (from hooks): // Handle tool items if (item.lazy.hasValue) { - return; // Already materialized + // Already evaluated — but may not have been placed in the DOM yet + // (e.g. finalizeTitleIfDefault materialized it before the wrapper existed). + const result = item.lazy.value; + if (!result.domNode.parentElement) { + this.appendItemToDOM(result.domNode, item.toolInvocationId, item.toolInvocationOrMarkdown, item.originalParent); + } + return; } const result = item.lazy.value; diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts index d840ad32a8779..95a7c32fe999e 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts @@ -1193,7 +1193,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer { assert.strictEqual(removedEl?.textContent, '-3'); }); - test('should show +0 -0 when diff parts exist but have no changes', () => { + test('should not show diff stats when diff parts exist but have no changes', () => { const content = createThinkingPart('**Editing files**'); const context = createMockRenderContext(true); @@ -1548,8 +1548,8 @@ suite('ChatThinkingContentPart', () => { const addedEl = part.domNode.querySelector('.label-added'); const removedEl = part.domNode.querySelector('.label-removed'); - assert.strictEqual(addedEl?.textContent, '+0'); - assert.strictEqual(removedEl?.textContent, '-0'); + assert.strictEqual(addedEl, null); + assert.strictEqual(removedEl, null); }); test('should include diff stats in aria-label', () => { From e78b8ebc62cae895bc38cd8a44618bba8417fb1a Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Fri, 27 Mar 2026 07:38:16 +0000 Subject: [PATCH 11/17] Fix inconsistent capitalization in permissions learn-more string (#305535) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Initial plan * Fix inconsistent capitalization: "Learn More about Permissions" → "Learn more about permissions" Agent-Logs-Url: https://github.com/microsoft/vscode/sessions/23fe2cf9-916b-4fea-b014-2d98f21d2386 Co-authored-by: justschen <54879025+justschen@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: justschen <54879025+justschen@users.noreply.github.com> --- .../sessions/contrib/chat/browser/newChatPermissionPicker.ts | 4 ++-- .../chat/browser/widget/input/permissionPickerActionItem.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/newChatPermissionPicker.ts b/src/vs/sessions/contrib/chat/browser/newChatPermissionPicker.ts index 790c7106691c7..365f4747b3a19 100644 --- a/src/vs/sessions/contrib/chat/browser/newChatPermissionPicker.ts +++ b/src/vs/sessions/contrib/chat/browser/newChatPermissionPicker.ts @@ -163,11 +163,11 @@ export class NewChatPermissionPicker extends Disposable { kind: ActionListItemKind.Action, group: { kind: ActionListItemKind.Header, title: '', icon: Codicon.blank }, item: { - label: localize('permissions.learnMore', "Learn More about Permissions"), + label: localize('permissions.learnMore', "Learn more about permissions"), icon: Codicon.blank, checked: false, }, - label: localize('permissions.learnMore', "Learn More about Permissions"), + label: localize('permissions.learnMore', "Learn more about permissions"), hideIcon: false, disabled: false, }); diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts b/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts index 293764ad7397f..f0bad94daa558 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts @@ -191,8 +191,8 @@ export class PermissionPickerActionItem extends ChatInputPickerActionViewItem { actionProvider, actionBarActions: [{ id: 'chat.permissions.learnMore', - label: localize('permissions.learnMore', "Learn More about Permissions"), - tooltip: localize('permissions.learnMore', "Learn More about Permissions"), + label: localize('permissions.learnMore', "Learn more about permissions"), + tooltip: localize('permissions.learnMore', "Learn more about permissions"), class: undefined, enabled: true, run: async () => { From 5f966ed2ed6391b9e20eb8e6bed49b04ee15cd47 Mon Sep 17 00:00:00 2001 From: Matt Bierner <12821956+mjbvz@users.noreply.github.com> Date: Fri, 27 Mar 2026 00:38:30 -0700 Subject: [PATCH 12/17] Also default to treating macos as case insensitive for md file checks --- extensions/markdown-language-features/src/util/resources.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/markdown-language-features/src/util/resources.ts b/extensions/markdown-language-features/src/util/resources.ts index 3edb0bdf48ff4..0cc36e73c6b24 100644 --- a/extensions/markdown-language-features/src/util/resources.ts +++ b/extensions/markdown-language-features/src/util/resources.ts @@ -21,7 +21,7 @@ export function areUrisEqual(uri1: vscode.Uri, uri2: vscode.Uri): boolean { } if (uri1.scheme === 'file') { - if (process.platform === 'win32') { + if (process.platform === 'win32' || process.platform === 'darwin') { return uri1.fsPath.toLowerCase() === uri2.fsPath.toLowerCase(); } From 3b3b067a1d8d5725211b545b38567ba7709a9d91 Mon Sep 17 00:00:00 2001 From: Justin Chen <54879025+justschen@users.noreply.github.com> Date: Fri, 27 Mar 2026 00:39:51 -0700 Subject: [PATCH 13/17] make tool call confirmation content LARGER (#305538) --- .../chatToolConfirmationSubPart.ts | 2 +- .../contrib/chat/browser/widget/media/chat.css | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationSubPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationSubPart.ts index fcd46cea03ee3..44dc256673db9 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationSubPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationSubPart.ts @@ -34,7 +34,7 @@ import { ChatMarkdownContentPart } from '../chatMarkdownContentPart.js'; import { AbstractToolConfirmationSubPart } from './abstractToolConfirmationSubPart.js'; import { EditorPool } from '../chatContentCodePools.js'; -const SHOW_MORE_MESSAGE_HEIGHT_TRIGGER = 45; +const SHOW_MORE_MESSAGE_HEIGHT_TRIGGER = 100; export class ToolConfirmationSubPart extends AbstractToolConfirmationSubPart { private markdownParts: ChatMarkdownContentPart[] = []; diff --git a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css index 5618337afc474..dd77d092198a7 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css +++ b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css @@ -516,7 +516,7 @@ display: none; position: absolute; right: 0; - top: 20px; + bottom: 0; a { color: var(--vscode-textLink-foreground); @@ -529,12 +529,11 @@ position: relative; .message-wrapper { - /* This mask fades out the end of the second line of text so the "see more" message can be displayed over it. */ - mask-image: - linear-gradient(to right, rgba(0, 0, 0, 1) calc(100% - 95px), rgba(0, 0, 0, 0) calc(100% - 72px)), linear-gradient(to top, rgba(0, 0, 0, 0) 0%, rgba(0, 0, 0, 0) 20px, rgba(0, 0, 0, 1) 2px, rgba(0, 0, 0, 1) 100%); - mask-repeat: no-repeat, no-repeat; + /* Fade out the bottom edge so the "see more" link can overlay cleanly. */ + mask-image: linear-gradient(to bottom, rgba(0, 0, 0, 1) calc(100% - 24px), rgba(0, 0, 0, 0) 100%); pointer-events: none; - max-height: 40px; + max-height: 96px; + overflow: hidden; } .see-more { From bd5b480c236751f68f7ba842d63a05f6e7e80962 Mon Sep 17 00:00:00 2001 From: Matt Bierner <12821956+mjbvz@users.noreply.github.com> Date: Fri, 27 Mar 2026 00:42:39 -0700 Subject: [PATCH 14/17] Use `areUrisEqual` helper for better uri checks --- extensions/markdown-language-features/src/preview/preview.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/extensions/markdown-language-features/src/preview/preview.ts b/extensions/markdown-language-features/src/preview/preview.ts index a96a0ae028ff6..4dc949eae0585 100644 --- a/extensions/markdown-language-features/src/preview/preview.ts +++ b/extensions/markdown-language-features/src/preview/preview.ts @@ -129,8 +129,9 @@ class MarkdownPreview extends Disposable implements WebviewResourceProvider { const watcher = this._register(vscode.workspace.createFileSystemWatcher(new vscode.RelativePattern(resource, '*'))); this._register(watcher.onDidChange(uri => { if (this.isPreviewOf(uri)) { - // Only use the file system event when VS Code does not already know about the file - if (!vscode.workspace.textDocuments.some(doc => doc.uri.toString() === uri.toString())) { + // Only use the file system event when VS Code does not already know about the file. + // This is needed to avoid duplicate refreshes + if (!vscode.workspace.textDocuments.some(doc => areUrisEqual(doc.uri, uri))) { this.refresh(); } } From 1049842e2877640c538486e77d5ab3dd0c00719c Mon Sep 17 00:00:00 2001 From: BeniBenj Date: Fri, 27 Mar 2026 10:05:10 +0100 Subject: [PATCH 15/17] use new session icon instead of plus for add chat action --- .../contrib/sessions/browser/views/sessionsViewActions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts index 282653879da59..eda6598522f16 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts @@ -634,7 +634,7 @@ registerAction2(class AddChatAction extends Action2 { super({ id: 'agentSession.addChat', title: localize2('addChat', "Add Chat"), - icon: Codicon.plus, + icon: Codicon.newSession, menu: [{ id: Menus.CommandCenter, order: 102, From 7c89420b9636552800b7d2c646eab272b342bcec Mon Sep 17 00:00:00 2001 From: Isidor Nikolic Date: Fri, 27 Mar 2026 11:01:26 +0100 Subject: [PATCH 16/17] Remove ChatAgentVoteDownReason and voteDownReason (#304878) Remove the ChatAgentVoteDownReason enum and all voteDownReason references from the model, view model, service, telemetry, and UI layers. Fix snapshot test ordering to maintain backward compat for the voteDownReason field position. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../api/common/extHostChatAgents2.ts | 1 - .../chat/browser/actions/chatTitleActions.ts | 11 +-- .../chat/browser/widget/chatListRenderer.ts | 98 +------------------ .../chat/common/chatService/chatService.ts | 13 --- .../chatService/chatServiceTelemetry.ts | 3 - .../contrib/chat/common/model/chatModel.ts | 19 +--- .../common/model/chatSessionOperationLog.ts | 1 - .../chat/common/model/chatViewModel.ts | 13 +-- .../common/chatService/chatService.test.ts | 10 +- .../inlineChat/browser/inlineChatWidget.ts | 9 +- ...ode.proposed.chatParticipantAdditions.d.ts | 4 - 11 files changed, 17 insertions(+), 165 deletions(-) diff --git a/src/vs/workbench/api/common/extHostChatAgents2.ts b/src/vs/workbench/api/common/extHostChatAgents2.ts index a8ca253cb447c..9e7ac0fb7d3af 100644 --- a/src/vs/workbench/api/common/extHostChatAgents2.ts +++ b/src/vs/workbench/api/common/extHostChatAgents2.ts @@ -1038,7 +1038,6 @@ export class ExtHostChatAgents2 extends Disposable implements ExtHostChatAgentsS const feedback: vscode.ChatResultFeedback = { result: ehResult, kind, - unhelpfulReason: isProposedApiEnabled(agent.extension, 'chatParticipantAdditions') ? voteAction.reason : undefined, }; agent.acceptFeedback(Object.freeze(feedback)); } diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatTitleActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatTitleActions.ts index 89475130ce54d..44ce63265ab9e 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatTitleActions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatTitleActions.ts @@ -21,7 +21,7 @@ import { CellEditType, CellKind, NOTEBOOK_EDITOR_ID } from '../../../notebook/co import { NOTEBOOK_IS_ACTIVE_EDITOR } from '../../../notebook/common/notebookContextKeys.js'; import { ChatContextKeys } from '../../common/actions/chatContextKeys.js'; import { applyingChatEditsFailedContextKey, isChatEditingActionContext } from '../../common/editing/chatEditingService.js'; -import { ChatAgentVoteDirection, ChatAgentVoteDownReason, IChatService } from '../../common/chatService/chatService.js'; +import { ChatAgentVoteDirection, IChatService } from '../../common/chatService/chatService.js'; import { isResponseVM } from '../../common/model/chatViewModel.js'; import { ChatModeKind } from '../../common/constants.js'; import { IChatAccessibilityService, IChatWidgetService } from '../chat.js'; @@ -71,11 +71,9 @@ export function registerChatTitleActions() { action: { kind: 'vote', direction: ChatAgentVoteDirection.Up, - reason: undefined } }); item.setVote(ChatAgentVoteDirection.Up); - item.setVoteDownReason(undefined); } }); @@ -108,13 +106,7 @@ export function registerChatTitleActions() { return; } - const reason = args[1]; - if (typeof reason !== 'string') { - return; - } - item.setVote(ChatAgentVoteDirection.Down); - item.setVoteDownReason(reason as ChatAgentVoteDownReason); const chatService = accessor.get(IChatService); chatService.notifyUserAction({ @@ -126,7 +118,6 @@ export function registerChatTitleActions() { action: { kind: 'vote', direction: ChatAgentVoteDirection.Down, - reason: item.voteDownReason } }); } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts index 95a7c32fe999e..d1c8c32b4ecc7 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts @@ -8,7 +8,6 @@ import { renderFormattedText } from '../../../../../base/browser/formattedTextRe import { StandardKeyboardEvent } from '../../../../../base/browser/keyboardEvent.js'; import { IActionViewItemOptions } from '../../../../../base/browser/ui/actionbar/actionViewItems.js'; import { alert } from '../../../../../base/browser/ui/aria/aria.js'; -import { DropdownMenuActionViewItem, IDropdownMenuActionViewItemOptions } from '../../../../../base/browser/ui/dropdown/dropdownActionViewItem.js'; import { getDefaultHoverDelegate } from '../../../../../base/browser/ui/hover/hoverDelegateFactory.js'; import { CachedListVirtualDelegate, IListElementRenderDetails } from '../../../../../base/browser/ui/list/list.js'; import { ITreeNode, ITreeRenderer } from '../../../../../base/browser/ui/tree/tree.js'; @@ -31,13 +30,12 @@ import { clamp } from '../../../../../base/common/numbers.js'; import { ThemeIcon } from '../../../../../base/common/themables.js'; import { URI } from '../../../../../base/common/uri.js'; import { localize } from '../../../../../nls.js'; -import { IMenuEntryActionViewItemOptions, MenuEntryActionViewItem, createActionViewItem } from '../../../../../platform/actions/browser/menuEntryActionViewItem.js'; +import { MenuEntryActionViewItem, createActionViewItem } from '../../../../../platform/actions/browser/menuEntryActionViewItem.js'; import { MenuWorkbenchToolBar } from '../../../../../platform/actions/browser/toolbar.js'; import { MenuId, MenuItemAction } from '../../../../../platform/actions/common/actions.js'; import { ICommandService } from '../../../../../platform/commands/common/commands.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js'; -import { IContextMenuService } from '../../../../../platform/contextview/browser/contextView.js'; import { IHoverService } from '../../../../../platform/hover/browser/hover.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { ServiceCollection } from '../../../../../platform/instantiation/common/serviceCollection.js'; @@ -47,7 +45,6 @@ import { isDark } from '../../../../../platform/theme/common/theme.js'; import { IThemeService } from '../../../../../platform/theme/common/themeService.js'; import { AccessibilitySignal, IAccessibilitySignalService } from '../../../../../platform/accessibilitySignal/browser/accessibilitySignalService.js'; import { IChatEntitlementService } from '../../../../services/chat/common/chatEntitlementService.js'; -import { IWorkbenchIssueService } from '../../../issue/common/issue.js'; import { CodiconActionViewItem } from '../../../notebook/browser/view/cellParts/cellActionView.js'; import { annotateSpecialMarkdownContent, extractSubAgentInvocationIdFromText, hasCodeblockUriTag, hasEditCodeblockUriTag } from '../../common/widget/annotations.js'; import { checkModeOption } from '../../common/chat.js'; @@ -55,7 +52,7 @@ import { IChatAgentMetadata } from '../../common/participants/chatAgents.js'; import { ChatContextKeys } from '../../common/actions/chatContextKeys.js'; import { IChatTextEditGroup } from '../../common/model/chatModel.js'; import { chatSubcommandLeader } from '../../common/requestParser/chatParserTypes.js'; -import { ChatAgentVoteDirection, ChatAgentVoteDownReason, ChatErrorLevel, ChatRequestQueueKind, IChatConfirmation, IChatContentReference, IChatDisabledClaudeHooksPart, IChatElicitationRequest, IChatElicitationRequestSerialized, IChatExtensionsContent, IChatFollowup, IChatHookPart, IChatMarkdownContent, IChatMcpServersStarting, IChatMcpServersStartingSerialized, IChatMultiDiffData, IChatMultiDiffDataSerialized, IChatPullRequestContent, IChatQuestionAnswerValue, IChatQuestionAnswers, IChatQuestionCarousel, IChatService, IChatTask, IChatTaskSerialized, IChatThinkingPart, IChatToolInvocation, IChatToolInvocationSerialized, IChatTreeData, IChatUndoStop, isChatFollowup } from '../../common/chatService/chatService.js'; +import { ChatAgentVoteDirection, ChatErrorLevel, ChatRequestQueueKind, IChatConfirmation, IChatContentReference, IChatDisabledClaudeHooksPart, IChatElicitationRequest, IChatElicitationRequestSerialized, IChatExtensionsContent, IChatFollowup, IChatHookPart, IChatMarkdownContent, IChatMcpServersStarting, IChatMcpServersStartingSerialized, IChatMultiDiffData, IChatMultiDiffDataSerialized, IChatPullRequestContent, IChatQuestionAnswerValue, IChatQuestionAnswers, IChatQuestionCarousel, IChatService, IChatTask, IChatTaskSerialized, IChatThinkingPart, IChatToolInvocation, IChatToolInvocationSerialized, IChatTreeData, IChatUndoStop, isChatFollowup } from '../../common/chatService/chatService.js'; import { ChatQuestionCarouselData } from '../../common/model/chatProgressTypes/chatQuestionCarouselData.js'; import { localChatSessionType } from '../../common/chatSessionsService.js'; import { getChatSessionType } from '../../common/model/chatUri.js'; @@ -65,7 +62,7 @@ import { getNWords } from '../../common/model/chatWordCounter.js'; import { CodeBlockModelCollection } from '../../common/widget/codeBlockModelCollection.js'; import { ChatAgentLocation, ChatConfiguration, ChatModeKind, CollapsedToolsDisplayMode, ThinkingDisplayMode } from '../../common/constants.js'; import { ClickAnimation } from '../../../../../base/browser/ui/animations/animations.js'; -import { MarkHelpfulActionId, MarkUnhelpfulActionId } from '../actions/chatTitleActions.js'; +import { MarkHelpfulActionId } from '../actions/chatTitleActions.js'; import { ChatTreeItem, IChatCodeBlockInfo, IChatFileTreeInfo, IChatListItemRendererOptions, IChatWidgetService } from '../chat.js'; import { ChatAgentHover, getChatAgentHoverOptions } from './chatAgentHover.js'; import { ChatContentMarkdownRenderer } from './chatContentMarkdownRenderer.js'; @@ -541,9 +538,6 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer submenu.actions.length <= 1 }, actionViewItemProvider: (action: IAction, options: IActionViewItemOptions) => { - if (action instanceof MenuItemAction && action.item.id === MarkUnhelpfulActionId) { - return scopedInstantiationService.createInstance(ChatVoteDownButton, action, options as IMenuEntryActionViewItemOptions); - } if (action instanceof MenuItemAction && action.item.id === MarkHelpfulActionId) { const animation = upvoteAnimationSettingToEnum(this.configService.getValue('chat.upvoteAnimation')); return scopedInstantiationService.createInstance(MenuEntryActionViewItem, action, { ...options, onClickAnimation: animation }); @@ -2598,92 +2592,6 @@ export class ChatListDelegate extends CachedListVirtualDelegate { } } -const voteDownDetailLabels: Record = { - [ChatAgentVoteDownReason.IncorrectCode]: localize('incorrectCode', "Suggested incorrect code"), - [ChatAgentVoteDownReason.DidNotFollowInstructions]: localize('didNotFollowInstructions', "Didn't follow instructions"), - [ChatAgentVoteDownReason.MissingContext]: localize('missingContext', "Missing context"), - [ChatAgentVoteDownReason.OffensiveOrUnsafe]: localize('offensiveOrUnsafe', "Offensive or unsafe"), - [ChatAgentVoteDownReason.PoorlyWrittenOrFormatted]: localize('poorlyWrittenOrFormatted', "Poorly written or formatted"), - [ChatAgentVoteDownReason.RefusedAValidRequest]: localize('refusedAValidRequest', "Refused a valid request"), - [ChatAgentVoteDownReason.IncompleteCode]: localize('incompleteCode', "Incomplete code"), - [ChatAgentVoteDownReason.WillReportIssue]: localize('reportIssue', "Report an issue"), - [ChatAgentVoteDownReason.Other]: localize('other', "Other"), -}; - -export class ChatVoteDownButton extends DropdownMenuActionViewItem { - constructor( - action: IAction, - options: IDropdownMenuActionViewItemOptions | undefined, - @ICommandService private readonly commandService: ICommandService, - @IWorkbenchIssueService private readonly issueService: IWorkbenchIssueService, - @ILogService private readonly logService: ILogService, - @IContextMenuService contextMenuService: IContextMenuService, - ) { - super(action, - { getActions: () => this.getActions(), }, - contextMenuService, - { - ...options, - classNames: ThemeIcon.asClassNameArray(Codicon.thumbsdown), - }); - } - - getActions(): readonly IAction[] { - return [ - this.getVoteDownDetailAction(ChatAgentVoteDownReason.IncorrectCode), - this.getVoteDownDetailAction(ChatAgentVoteDownReason.DidNotFollowInstructions), - this.getVoteDownDetailAction(ChatAgentVoteDownReason.IncompleteCode), - this.getVoteDownDetailAction(ChatAgentVoteDownReason.MissingContext), - this.getVoteDownDetailAction(ChatAgentVoteDownReason.PoorlyWrittenOrFormatted), - this.getVoteDownDetailAction(ChatAgentVoteDownReason.RefusedAValidRequest), - this.getVoteDownDetailAction(ChatAgentVoteDownReason.OffensiveOrUnsafe), - this.getVoteDownDetailAction(ChatAgentVoteDownReason.Other), - { - id: 'reportIssue', - label: voteDownDetailLabels[ChatAgentVoteDownReason.WillReportIssue], - tooltip: '', - enabled: true, - class: undefined, - run: async (context: IChatResponseViewModel) => { - if (!isResponseVM(context)) { - this.logService.error('ChatVoteDownButton#run: invalid context'); - return; - } - - await this.commandService.executeCommand(MarkUnhelpfulActionId, context, ChatAgentVoteDownReason.WillReportIssue); - await this.issueService.openReporter({ extensionId: context.agent?.extensionId.value }); - } - } - ]; - } - - override render(container: HTMLElement): void { - super.render(container); - - this.element?.classList.toggle('checked', this.action.checked); - } - - private getVoteDownDetailAction(reason: ChatAgentVoteDownReason): IAction { - const label = voteDownDetailLabels[reason]; - return { - id: MarkUnhelpfulActionId, - label, - tooltip: '', - enabled: true, - checked: (this._context as IChatResponseViewModel).voteDownReason === reason, - class: undefined, - run: async (context: IChatResponseViewModel) => { - if (!isResponseVM(context)) { - this.logService.error('ChatVoteDownButton#getVoteDownDetailAction: invalid context'); - return; - } - - await this.commandService.executeCommand(MarkUnhelpfulActionId, context, reason); - } - }; - } -} - /** * Check if a tool invocation is the parent subagent tool (the tool that spawns a subagent). * A parent subagent tool has subagent toolSpecificData but no subAgentInvocationId. diff --git a/src/vs/workbench/contrib/chat/common/chatService/chatService.ts b/src/vs/workbench/contrib/chat/common/chatService/chatService.ts index 26426bb321338..d748c114514c3 100644 --- a/src/vs/workbench/contrib/chat/common/chatService/chatService.ts +++ b/src/vs/workbench/contrib/chat/common/chatService/chatService.ts @@ -1074,22 +1074,9 @@ export enum ChatAgentVoteDirection { Up = 1 } -export enum ChatAgentVoteDownReason { - IncorrectCode = 'incorrectCode', - DidNotFollowInstructions = 'didNotFollowInstructions', - IncompleteCode = 'incompleteCode', - MissingContext = 'missingContext', - PoorlyWrittenOrFormatted = 'poorlyWrittenOrFormatted', - RefusedAValidRequest = 'refusedAValidRequest', - OffensiveOrUnsafe = 'offensiveOrUnsafe', - Other = 'other', - WillReportIssue = 'willReportIssue' -} - export interface IChatVoteAction { kind: 'vote'; direction: ChatAgentVoteDirection; - reason: ChatAgentVoteDownReason | undefined; } export enum ChatCopyKind { diff --git a/src/vs/workbench/contrib/chat/common/chatService/chatServiceTelemetry.ts b/src/vs/workbench/contrib/chat/common/chatService/chatServiceTelemetry.ts index d524128a23803..352a53bffda2c 100644 --- a/src/vs/workbench/contrib/chat/common/chatService/chatServiceTelemetry.ts +++ b/src/vs/workbench/contrib/chat/common/chatService/chatServiceTelemetry.ts @@ -19,14 +19,12 @@ type ChatVoteEvent = { direction: 'up' | 'down'; agentId: string; command: string | undefined; - reason: string | undefined; }; type ChatVoteClassification = { direction: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Whether the user voted up or down.' }; agentId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'The ID of the chat agent that this vote is for.' }; command: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'The name of the slash command that this vote is for.' }; - reason: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'The reason selected by the user for voting down.' }; owner: 'roblourens'; comment: 'Provides insight into the performance of Chat agents.'; }; @@ -188,7 +186,6 @@ export class ChatServiceTelemetry { direction: action.action.direction === ChatAgentVoteDirection.Up ? 'up' : 'down', agentId: action.agentId ?? '', command: action.command, - reason: action.action.reason, }); } else if (action.action.kind === 'copy') { this.telemetryService.publicLog2('interactiveSessionCopy', { diff --git a/src/vs/workbench/contrib/chat/common/model/chatModel.ts b/src/vs/workbench/contrib/chat/common/model/chatModel.ts index b99e48c11bc20..2fb3d3943ff1d 100644 --- a/src/vs/workbench/contrib/chat/common/model/chatModel.ts +++ b/src/vs/workbench/contrib/chat/common/model/chatModel.ts @@ -30,7 +30,7 @@ import { CellUri, ICellEditOperation } from '../../../notebook/common/notebookCo import { ChatRequestToolReferenceEntry, IChatRequestVariableEntry, isImplicitVariableEntry, isStringImplicitContextValue, isStringVariableEntry } from '../attachments/chatVariableEntries.js'; import { migrateLegacyTerminalToolSpecificData } from '../chat.js'; import { ChatPerfMark, markChat } from '../chatPerf.js'; -import { ChatAgentVoteDirection, ChatAgentVoteDownReason, ChatRequestQueueKind, ChatResponseClearToPreviousToolInvocationReason, ElicitationState, IChatAgentMarkdownContentWithVulnerability, IChatClearToPreviousToolInvocation, IChatCodeCitation, IChatCommandButton, IChatConfirmation, IChatContentInlineReference, IChatContentReference, IChatDisabledClaudeHooksPart, IChatEditingSessionAction, IChatElicitationRequest, IChatElicitationRequestSerialized, IChatExternalToolInvocationUpdate, IChatExtensionsContent, IChatFollowup, IChatHookPart, IChatLocationData, IChatMarkdownContent, IChatMcpServersStarting, IChatMcpServersStartingSerialized, IChatModelReference, IChatMultiDiffData, IChatMultiDiffDataSerialized, IChatNotebookEdit, IChatProgress, IChatProgressMessage, IChatPullRequestContent, IChatQuestionCarousel, IChatResponseCodeblockUriPart, IChatResponseProgressFileTreeData, IChatSendRequestOptions, IChatService, IChatSessionContext, IChatSessionTiming, IChatTask, IChatTaskSerialized, IChatTextEdit, IChatThinkingPart, IChatToolInvocation, IChatToolInvocationSerialized, IChatTreeData, IChatUndoStop, IChatUsage, IChatUsedContext, IChatWarningMessage, IChatWorkspaceEdit, ResponseModelState, isIUsedContext } from '../chatService/chatService.js'; +import { ChatAgentVoteDirection, ChatRequestQueueKind, ChatResponseClearToPreviousToolInvocationReason, ElicitationState, IChatAgentMarkdownContentWithVulnerability, IChatClearToPreviousToolInvocation, IChatCodeCitation, IChatCommandButton, IChatConfirmation, IChatContentInlineReference, IChatContentReference, IChatDisabledClaudeHooksPart, IChatEditingSessionAction, IChatElicitationRequest, IChatElicitationRequestSerialized, IChatExternalToolInvocationUpdate, IChatExtensionsContent, IChatFollowup, IChatHookPart, IChatLocationData, IChatMarkdownContent, IChatMcpServersStarting, IChatMcpServersStartingSerialized, IChatModelReference, IChatMultiDiffData, IChatMultiDiffDataSerialized, IChatNotebookEdit, IChatProgress, IChatProgressMessage, IChatPullRequestContent, IChatQuestionCarousel, IChatResponseCodeblockUriPart, IChatResponseProgressFileTreeData, IChatSendRequestOptions, IChatService, IChatSessionContext, IChatSessionTiming, IChatTask, IChatTaskSerialized, IChatTextEdit, IChatThinkingPart, IChatToolInvocation, IChatToolInvocationSerialized, IChatTreeData, IChatUndoStop, IChatUsage, IChatUsedContext, IChatWarningMessage, IChatWorkspaceEdit, ResponseModelState, isIUsedContext } from '../chatService/chatService.js'; import { ChatAgentLocation, ChatModeKind, ChatPermissionLevel } from '../constants.js'; import { ChatToolInvocation } from './chatProgressTypes/chatToolInvocation.js'; import { ToolDataSource, IToolData } from '../tools/languageModelToolsService.js'; @@ -280,7 +280,6 @@ export interface IChatResponseModel { /** A stale response is one that has been persisted and rehydrated, so e.g. Commands that have their arguments stored in the EH are gone. */ readonly isStale: boolean; readonly vote: ChatAgentVoteDirection | undefined; - readonly voteDownReason: ChatAgentVoteDownReason | undefined; readonly followups?: IChatFollowup[] | undefined; readonly result?: IChatAgentResult; readonly usage?: IChatUsage; @@ -289,7 +288,6 @@ export interface IChatResponseModel { initializeCodeBlockInfos(codeBlockInfo: ICodeBlockInfo[]): void; addUndoStop(undoStop: IChatUndoStop): void; setVote(vote: ChatAgentVoteDirection): void; - setVoteDownReason(reason: ChatAgentVoteDownReason | undefined): void; setUsage(usage: IChatUsage): void; setEditApplied(edit: IChatTextEditGroup, editCount: number): boolean; updateContent(progress: IChatProgressResponseContent | IChatTextEdit | IChatNotebookEdit | IChatTask | IChatExternalToolInvocationUpdate, quiet?: boolean): void; @@ -939,7 +937,6 @@ export interface IChatResponseModelParameters { requestId: string; timestamp?: number; vote?: ChatAgentVoteDirection; - voteDownReason?: ChatAgentVoteDownReason; result?: IChatAgentResult; followups?: ReadonlyArray; isCompleteAddedRequest?: boolean; @@ -970,7 +967,6 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel private _slashCommand: IChatAgentCommand | undefined; private _modelState = observableValue(this, { value: ResponseModelState.Pending }); private _vote?: ChatAgentVoteDirection; - private _voteDownReason?: ChatAgentVoteDownReason; private _result?: IChatAgentResult; private _usage?: IChatUsage; private _shouldBeRemovedOnSend: IChatRequestDisablement | undefined; @@ -1044,10 +1040,6 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel return this._vote; } - public get voteDownReason(): ChatAgentVoteDownReason | undefined { - return this._voteDownReason; - } - public get followups(): IChatFollowup[] | undefined { return this._followups; } @@ -1147,7 +1139,6 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel } this._timeSpentWaitingAccumulator = params.timeSpentWaiting || 0; this._vote = params.vote; - this._voteDownReason = params.voteDownReason; this._result = params.result; this._followups = params.followups ? [...params.followups] : undefined; this.isCompleteAddedRequest = params.isCompleteAddedRequest ?? false; @@ -1318,11 +1309,6 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel this._onDidChange.fire(defaultChatResponseModelChangeReason); } - setVoteDownReason(reason: ChatAgentVoteDownReason | undefined): void { - this._voteDownReason = reason; - this._onDidChange.fire(defaultChatResponseModelChangeReason); - } - setEditApplied(edit: IChatTextEditGroup, editCount: number): boolean { if (!this.response.value.includes(edit)) { return false; @@ -1358,7 +1344,6 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel followups: this.followups, modelState: modelState.value === ResponseModelState.Pending || modelState.value === ResponseModelState.NeedsInput ? { value: ResponseModelState.Cancelled, completedAt: Date.now() } : modelState, vote: this.vote, - voteDownReason: this.voteDownReason, slashCommand: this.slashCommand, usedContext: this.usedContext, contentReferences: this.contentReferences, @@ -1441,7 +1426,6 @@ interface ISerializableChatResponseData { followups?: ReadonlyArray; modelState?: ResponseModelStateT; vote?: ChatAgentVoteDirection; - voteDownReason?: ChatAgentVoteDownReason; timestamp?: number; slashCommand?: IChatAgentCommand; /** For backward compat: should be optional */ @@ -2381,7 +2365,6 @@ export class ChatModel extends Disposable implements IChatModel { modelState, vote: raw.vote, timestamp: raw.timestamp, - voteDownReason: raw.voteDownReason, result, followups: raw.followups, restoredId: raw.responseId, diff --git a/src/vs/workbench/contrib/chat/common/model/chatSessionOperationLog.ts b/src/vs/workbench/contrib/chat/common/model/chatSessionOperationLog.ts index ab760e3bb3e54..bc7b42e663cec 100644 --- a/src/vs/workbench/contrib/chat/common/model/chatSessionOperationLog.ts +++ b/src/vs/workbench/contrib/chat/common/model/chatSessionOperationLog.ts @@ -144,7 +144,6 @@ const requestSchema = Adapt.object m.response?.followups, objectsEqual), modelState: Adapt.v(m => m.response?.stateT, objectsEqual), vote: Adapt.v(m => m.response?.vote), - voteDownReason: Adapt.v(m => m.response?.voteDownReason), slashCommand: Adapt.t(m => m.response?.slashCommand, Adapt.value((a, b) => a?.name === b?.name)), usedContext: Adapt.v(m => m.response?.usedContext, objectsEqual), contentReferences: Adapt.v(m => m.response?.contentReferences, objectsEqual), diff --git a/src/vs/workbench/contrib/chat/common/model/chatViewModel.ts b/src/vs/workbench/contrib/chat/common/model/chatViewModel.ts index 0c799d61f2001..a6fd04dae2b36 100644 --- a/src/vs/workbench/contrib/chat/common/model/chatViewModel.ts +++ b/src/vs/workbench/contrib/chat/common/model/chatViewModel.ts @@ -13,7 +13,7 @@ import { ThemeIcon } from '../../../../../base/common/themables.js'; import { URI } from '../../../../../base/common/uri.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { IChatRequestVariableEntry } from '../attachments/chatVariableEntries.js'; -import { ChatAgentVoteDirection, ChatAgentVoteDownReason, ChatRequestQueueKind, IChatCodeCitation, IChatContentReference, IChatDisabledClaudeHooksPart, IChatFollowup, IChatMcpServersStarting, IChatProgressMessage, IChatQuestionCarousel, IChatResponseErrorDetails, IChatTask, IChatUsedContext } from '../chatService/chatService.js'; +import { ChatAgentVoteDirection, ChatRequestQueueKind, IChatCodeCitation, IChatContentReference, IChatDisabledClaudeHooksPart, IChatFollowup, IChatMcpServersStarting, IChatProgressMessage, IChatQuestionCarousel, IChatResponseErrorDetails, IChatTask, IChatUsedContext } from '../chatService/chatService.js'; import { getFullyQualifiedId, IChatAgentCommand, IChatAgentData, IChatAgentNameService, IChatAgentResult } from '../participants/chatAgents.js'; import { IParsedChatRequest } from '../requestParser/chatParserTypes.js'; import { CodeBlockModelCollection } from '../widget/codeBlockModelCollection.js'; @@ -207,7 +207,6 @@ export interface IChatResponseViewModel { readonly isCanceled: boolean; readonly isStale: boolean; readonly vote: ChatAgentVoteDirection | undefined; - readonly voteDownReason: ChatAgentVoteDownReason | undefined; readonly replyFollowups?: IChatFollowup[]; readonly errorDetails?: IChatResponseErrorDetails; readonly result?: IChatAgentResult; @@ -217,7 +216,6 @@ export interface IChatResponseViewModel { renderData?: IChatResponseRenderData; currentRenderedHeight: number | undefined; setVote(vote: ChatAgentVoteDirection): void; - setVoteDownReason(reason: ChatAgentVoteDownReason | undefined): void; usedReferencesExpanded?: boolean; vulnerabilitiesListExpanded: boolean; setEditApplied(edit: IChatTextEditGroup, editCount: number): void; @@ -587,10 +585,6 @@ export class ChatResponseViewModel extends Disposable implements IChatResponseVi return this._model.vote; } - get voteDownReason() { - return this._model.voteDownReason; - } - get requestId() { return this._model.requestId; } @@ -666,11 +660,6 @@ export class ChatResponseViewModel extends Disposable implements IChatResponseVi this._model.setVote(vote); } - setVoteDownReason(reason: ChatAgentVoteDownReason | undefined): void { - this._modelChangeCount++; - this._model.setVoteDownReason(reason); - } - setEditApplied(edit: IChatTextEditGroup, editCount: number) { this._modelChangeCount++; this._model.setEditApplied(edit, editCount); diff --git a/src/vs/workbench/contrib/chat/test/common/chatService/chatService.test.ts b/src/vs/workbench/contrib/chat/test/common/chatService/chatService.test.ts index c4dacb71de5a9..89a0d4d6d2465 100644 --- a/src/vs/workbench/contrib/chat/test/common/chatService/chatService.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/chatService/chatService.test.ts @@ -952,8 +952,10 @@ function toSnapshotExportData(model: IChatModel) { return { ...exp, requests: exp.requests.map(r => { + // Destructure properties after `vote` so we can insert `voteDownReason` in the correct position for snapshot compat + const { slashCommand, usedContext, contentReferences, codeCitations, timeSpentWaiting, ...rest } = r; return { - ...r, + ...rest, modelState: { ...r.modelState, completedAt: undefined @@ -961,6 +963,12 @@ function toSnapshotExportData(model: IChatModel) { timestamp: undefined, requestId: undefined, // id contains a random part responseId: undefined, // id contains a random part + voteDownReason: undefined, // removed from model, kept for snapshot compat + slashCommand, + usedContext, + contentReferences, + codeCitations, + timeSpentWaiting, }; }) }; diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts index a449ed2a51206..ad2589ec7f1a8 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts @@ -22,9 +22,9 @@ import { localize } from '../../../../nls.js'; import { IAccessibleViewService } from '../../../../platform/accessibility/browser/accessibleView.js'; import { IAccessibilityService } from '../../../../platform/accessibility/common/accessibility.js'; import { IWorkbenchButtonBarOptions, MenuWorkbenchButtonBar } from '../../../../platform/actions/browser/buttonbar.js'; -import { createActionViewItem, IMenuEntryActionViewItemOptions } from '../../../../platform/actions/browser/menuEntryActionViewItem.js'; +import { createActionViewItem } from '../../../../platform/actions/browser/menuEntryActionViewItem.js'; import { MenuWorkbenchToolBar } from '../../../../platform/actions/browser/toolbar.js'; -import { MenuId, MenuItemAction } from '../../../../platform/actions/common/actions.js'; +import { MenuId } from '../../../../platform/actions/common/actions.js'; import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; import { IContextKey, IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; import { IHoverService } from '../../../../platform/hover/browser/hover.js'; @@ -39,9 +39,7 @@ import { EDITOR_DRAG_AND_DROP_BACKGROUND } from '../../../common/theme.js'; import { IChatEntitlementService } from '../../../services/chat/common/chatEntitlementService.js'; import { AccessibilityVerbositySettingId } from '../../accessibility/browser/accessibilityConfiguration.js'; import { AccessibilityCommandId } from '../../accessibility/common/accessibilityCommands.js'; -import { MarkUnhelpfulActionId } from '../../chat/browser/actions/chatTitleActions.js'; import { IChatWidgetViewOptions } from '../../chat/browser/chat.js'; -import { ChatVoteDownButton } from '../../chat/browser/widget/chatListRenderer.js'; import { ChatWidget, IChatWidgetLocationOptions } from '../../chat/browser/widget/chatWidget.js'; import { chatRequestBackground } from '../../chat/common/widget/chatColors.js'; import { ChatContextKeys } from '../../chat/common/actions/chatContextKeys.js'; @@ -251,9 +249,6 @@ export class InlineChatWidget { telemetrySource: _options.chatWidgetViewOptions?.menus?.telemetrySource, menuOptions: { renderShortTitle: true, shouldForwardArgs: true }, actionViewItemProvider: (action: IAction, options: IActionViewItemOptions) => { - if (action instanceof MenuItemAction && action.item.id === MarkUnhelpfulActionId) { - return scopedInstaService.createInstance(ChatVoteDownButton, action, options as IMenuEntryActionViewItemOptions); - } return createActionViewItem(scopedInstaService, action, options); } }); diff --git a/src/vscode-dts/vscode.proposed.chatParticipantAdditions.d.ts b/src/vscode-dts/vscode.proposed.chatParticipantAdditions.d.ts index 286b87dd85cc7..f38544f604c98 100644 --- a/src/vscode-dts/vscode.proposed.chatParticipantAdditions.d.ts +++ b/src/vscode-dts/vscode.proposed.chatParticipantAdditions.d.ts @@ -990,10 +990,6 @@ declare module 'vscode' { readonly toolReferences?: readonly ChatLanguageModelToolReference[]; } - export interface ChatResultFeedback { - readonly unhelpfulReason?: string; - } - export namespace lm { export function fileIsIgnored(uri: Uri, token?: CancellationToken): Thenable; } From 78a390832f8f43747c73397f9139eaf0c288e8e6 Mon Sep 17 00:00:00 2001 From: Abhitej John Date: Fri, 27 Mar 2026 03:17:00 -0700 Subject: [PATCH 17/17] Skills usage telemetry (#303110) * Add per-skill telemetry when skills are loaded into agent context Add a new 'skillLoadedIntoContext' telemetry event that fires for each skill loaded into agent context. This captures: - Skill name - Skill storage source (local, user, extension, plugin, internal) - SHA-1 hash of the SKILL.md file content (integrity verification) The event fires in computeAutomaticInstructions.ts where skills are filtered and loaded into the chat system prompt, providing per-request skill usage visibility. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Replace skillVersion/skillAuthor with extensionVersion in skill provenance telemetry * Add skill telemetry tests for skillLoadedIntoContext events * Send extensionVersion and pluginVersion as plain text instead of hashed * Address PR review feedback: use ResourceMap for URI keys, remove skillContentHash, simplify plugin lookup * Add try/catch for fire-and-forget telemetry, add extension/plugin provenance test * Clearing out an unused import. * update, avoid new IAgentSkillProvenance type * use cheap hash --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Martin Aeschlimann --- .../chat/browser/promptsDebugContribution.ts | 2 +- .../computeAutomaticInstructions.ts | 57 +++- .../config/promptFileLocations.ts | 3 + .../promptSyntax/service/promptsService.ts | 26 +- .../service/promptsServiceImpl.ts | 79 +++--- .../computeAutomaticInstructions.test.ts | 261 +++++++++++++++++- 6 files changed, 384 insertions(+), 44 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/promptsDebugContribution.ts b/src/vs/workbench/contrib/chat/browser/promptsDebugContribution.ts index 85c07a6c599e8..a6445b4df0a1a 100644 --- a/src/vs/workbench/contrib/chat/browser/promptsDebugContribution.ts +++ b/src/vs/workbench/contrib/chat/browser/promptsDebugContribution.ts @@ -104,7 +104,7 @@ export class PromptsDebugContribution extends Disposable implements IWorkbenchCo name: f.name, status: f.status, storage: f.storage, - extensionId: f.extensionId, + extensionId: f.extension?.identifier.value, skipReason: f.skipReason, errorMessage: f.errorMessage, duplicateOf: f.duplicateOf, diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/computeAutomaticInstructions.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/computeAutomaticInstructions.ts index dafa487c65bd7..5f223e524ee6c 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/computeAutomaticInstructions.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/computeAutomaticInstructions.ts @@ -24,11 +24,13 @@ import { ILanguageModelToolsService, IToolData, VSCodeToolReference } from '../t import { PromptsConfig } from './config/config.js'; import { isInClaudeAgentsFolder, isInClaudeRulesFolder, isPromptOrInstructionsFile } from './config/promptFileLocations.js'; import { ParsedPromptFile, PromptHeader } from './promptFileParser.js'; -import { AgentFileType, ICustomAgent, IPromptPath, IPromptsService } from './service/promptsService.js'; +import { AgentFileType, IAgentSkill, ICustomAgent, IPromptPath, IPromptsService } from './service/promptsService.js'; import { AGENT_DEBUG_LOG_ENABLED_SETTING, AGENT_DEBUG_LOG_FILE_LOGGING_ENABLED_SETTING, TROUBLESHOOT_SKILL_PATH } from './promptTypes.js'; import { OffsetRange } from '../../../../../editor/common/core/ranges/offsetRange.js'; import { ChatConfiguration, ChatModeKind } from '../constants.js'; import { UserSelectedTools } from '../participants/chatAgents.js'; +import { hash } from '../../../../../base/common/hash.js'; +import { IAgentPlugin, IAgentPluginService } from '../plugins/agentPluginService.js'; export type InstructionsCollectionEvent = { applyingInstructionsCount: number; @@ -76,6 +78,7 @@ export class ComputeAutomaticInstructions { @IRemoteAgentService private readonly _remoteAgentService: IRemoteAgentService, @ITelemetryService private readonly _telemetryService: ITelemetryService, @ILanguageModelToolsService private readonly _languageModelToolsService: ILanguageModelToolsService, + @IAgentPluginService private readonly _agentPluginService: IAgentPluginService, ) { } @@ -127,6 +130,55 @@ export class ComputeAutomaticInstructions { this._telemetryService.publicLog2('instructionsCollected', telemetryEvent); } + private async _logSkillLoadedTelemetry(skills: readonly IAgentSkill[]): Promise { + type SkillLoadedIntoContextEvent = { + skillNameHash: string; + skillStorage: string; + extensionIdHash: string; + extensionVersion: string; + pluginNameHash: string; + pluginVersion: string; + }; + + type SkillLoadedIntoContextClassification = { + skillNameHash: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'SHA-1 hash of the skill name loaded into the agent context.' }; + skillStorage: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'The storage source of the skill (local, user, extension, plugin, internal).' }; + extensionIdHash: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'SHA-1 hash of the contributing extension identifier, empty if none.' }; + extensionVersion: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Semver version of the contributing extension, empty if none.' }; + pluginNameHash: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'SHA-1 hash of the plugin display name, empty if not from a plugin.' }; + pluginVersion: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Semver version of the plugin, empty if unavailable.' }; + owner: 'manishj, dbreshears'; + comment: 'Tracks individual skill loading into agent context with provenance metadata.'; + }; + + try { + // Build map of plugin URI to plugin metadata for provenance + const pluginByUri = new ResourceMap(); + const allPlugins = this._agentPluginService.plugins.get(); + for (const plugin of allPlugins) { + pluginByUri.set(plugin.uri, plugin); + } + + const hashOrEmpty = (value: string | undefined) => { + return value !== undefined ? String(hash(value)) : ''; + }; + + for (const skill of skills) { + const skillPlugin = skill.pluginUri ? pluginByUri.get(skill.pluginUri) : undefined; + this._telemetryService.publicLog2('skillLoadedIntoContext', { + skillNameHash: hashOrEmpty(skill.name), + skillStorage: skill.storage, + extensionIdHash: hashOrEmpty(skill.extension?.identifier.value), + extensionVersion: skill.extension?.version ?? '', + pluginNameHash: hashOrEmpty(skillPlugin?.label), + pluginVersion: skillPlugin?.fromMarketplace?.version ?? '', + }); + } + } catch (err) { + this._logService.error('[InstructionsContextComputer] Failed to log skill telemetry', err); + } + } + /** public for testing */ public async addApplyingInstructions(instructionFiles: readonly IPromptPath[], context: { files: ResourceSet; instructions: ResourceSet }, variables: ChatRequestVariableSet, telemetryEvent: InstructionsCollectionEvent, token: CancellationToken): Promise { const includeApplyingInstructions = this._configurationService.getValue(PromptsConfig.INCLUDE_APPLYING_INSTRUCTIONS); @@ -361,6 +413,9 @@ export class ComputeAutomaticInstructions { return true; }); if (modelInvocableSkills && modelInvocableSkills.length > 0) { + // Log per-skill telemetry for each skill loaded into context + this._logSkillLoadedTelemetry(modelInvocableSkills); + const useSkillAdherencePrompt = this._configurationService.getValue(PromptsConfig.USE_SKILL_ADHERENCE_PROMPT); entries.push(''); if (useSkillAdherencePrompt) { diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/config/promptFileLocations.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/config/promptFileLocations.ts index bc63b772fe582..12e3eba053f71 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/config/promptFileLocations.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/config/promptFileLocations.ts @@ -7,6 +7,7 @@ import { URI } from '../../../../../../base/common/uri.js'; import { basename, dirname } from '../../../../../../base/common/resources.js'; import { PromptsType } from '../promptTypes.js'; import { PromptsStorage } from '../service/promptsService.js'; +import { IExtensionDescription } from '../../../../../../platform/extensions/common/extensions.js'; /** * File extension for the reusable prompt files. @@ -163,6 +164,8 @@ export interface IResolvedPromptFile { readonly fileUri: URI; readonly source: PromptFileSource; readonly storage: PromptsStorage; + readonly extension?: IExtensionDescription; + readonly pluginUri?: URI; } /** diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsService.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsService.ts index 84a2800ed78ad..7c677a7b3b293 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsService.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsService.ts @@ -114,6 +114,11 @@ export interface IPromptPathBase { */ readonly extension?: IExtensionDescription; + /** + * Identifier of the contributing plugin (only when storage === PromptsStorage.plugin). + */ + readonly pluginUri?: URI; + readonly name?: string; readonly description?: string; @@ -127,6 +132,11 @@ export interface IExtensionPromptPath extends IPromptPathBase { readonly description?: string; readonly when?: string; } + +export function isExtensionPromptPath(obj: IPromptPath): obj is IExtensionPromptPath { + return obj.storage === PromptsStorage.extension; +} + export interface ILocalPromptPath extends IPromptPathBase { readonly storage: PromptsStorage.local; } @@ -259,6 +269,10 @@ export interface IChatPromptSlashCommand { readonly when: ContextKeyExpression | undefined; } +/** + * Supply-chain metadata describing where a skill originated. + */ + export interface IAgentSkill { readonly uri: URI; readonly storage: PromptsStorage; @@ -279,6 +293,14 @@ export interface IAgentSkill { * when this expression evaluates to true against a scoped context. */ readonly when?: ContextKeyExpression; + /** + * Optional plugin URI describing where this skill originated. + */ + readonly pluginUri?: URI; + /** + * Optional extension metadata describing where this skill originated. + */ + readonly extension?: IExtensionDescription; } /** @@ -335,7 +357,9 @@ export interface IPromptFileDiscoveryResult { /** For duplicates, the URI of the file that took precedence */ readonly duplicateOf?: URI; /** Extension ID if from extension */ - readonly extensionId?: string; + readonly extension?: IExtensionDescription; + /** Uri of the plugin, if from a plugin */ + readonly pluginUri?: URI; /** Whether the skill is user-invocable in the / menu (set user-invocable: false to hide it) */ readonly userInvocable?: boolean; /** If true, the skill won't be automatically loaded by the agent (disable-model-invocation: true) */ diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts index 28bd11ed04085..62b0430152f35 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts @@ -1145,6 +1145,8 @@ export class PromptsService extends Disposable implements IPromptsService { disableModelInvocation: file.disableModelInvocation ?? false, userInvocable: file.userInvocable ?? true, when: internalSkill?.when, + pluginUri: file.pluginUri, + extension: file.extension, }); } } @@ -1390,8 +1392,9 @@ export class PromptsService extends Disposable implements IPromptsService { storage: promptPath.storage, status: 'skipped' as const, skipReason: 'disabled' as const, - extensionId: promptPath.extension?.identifier?.value - })); + extension: promptPath.extension, + pluginUri: promptPath.pluginUri + } satisfies IPromptFileDiscoveryResult)); const sourceFolders = await this._collectSourceFolderDiagnostics(PromptsType.skill); return { type: PromptsType.skill, files, sourceFolders }; } @@ -1422,11 +1425,13 @@ export class PromptsService extends Disposable implements IPromptsService { allSkills.push(...discoveredSkills, ...extensionSkills.map((extPath) => ({ fileUri: extPath.uri, storage: extPath.storage, - source: extPath.source === ExtensionAgentSourceType.contribution ? PromptFileSource.ExtensionContribution : PromptFileSource.ExtensionAPI + source: extPath.source === ExtensionAgentSourceType.contribution ? PromptFileSource.ExtensionContribution : PromptFileSource.ExtensionAPI, + extension: extPath.extension })), ...pluginSkills.map((p) => ({ fileUri: p.uri, storage: p.storage, source: PromptFileSource.Plugin, + pluginUri: p.pluginUri, })), ...this.internalCustomizations.getSkills() .map(s => ({ fileUri: s.uri, @@ -1455,17 +1460,9 @@ export class PromptsService extends Disposable implements IPromptsService { // Stable sort; we should keep order consistent to the order in the user's configuration object allSkills.sort((a, b) => getPriority(a) - getPriority(b)); - // Build map of URI to extension ID - const extensionIdByUri = new Map(); - for (const extSkill of extensionSkills) { - extensionIdByUri.set(extSkill.uri.toString(), extSkill.extension.identifier.value); - } - for (const skill of allSkills) { const uri = skill.fileUri; - const storage = skill.storage; - const source = skill.source; - const extensionId = extensionIdByUri.get(uri.toString()); + const { extension, pluginUri, storage, source } = skill; try { const parsedFile = await this.parseNew(uri, token); @@ -1485,7 +1482,7 @@ export class PromptsService extends Disposable implements IPromptsService { if (seenNames.has(sanitizedName)) { this.logger.debug(`[computeSkillDiscoveryInfo] Skipping duplicate agent skill name: ${sanitizedName} at ${uri}`); - files.push({ uri, storage, status: 'skipped', skipReason: 'duplicate-name', name: sanitizedName, duplicateOf: nameToUri.get(sanitizedName), extensionId, source }); + files.push({ uri, storage, status: 'skipped', skipReason: 'duplicate-name', name: sanitizedName, duplicateOf: nameToUri.get(sanitizedName), extension, source, pluginUri }); continue; } @@ -1495,7 +1492,8 @@ export class PromptsService extends Disposable implements IPromptsService { nameToUri.set(sanitizedName, uri); const disableModelInvocation = parsedFile.header?.disableModelInvocation === true; const userInvocable = parsedFile.header?.userInvocable !== false; - files.push({ uri, storage, status: 'loaded', name: sanitizedName, description, extensionId, source, disableModelInvocation, userInvocable }); + + files.push({ uri, storage, status: 'loaded', name: sanitizedName, description, extension, source, disableModelInvocation, userInvocable, pluginUri }); // Track skill type skillsBySource.set(source, (skillsBySource.get(source) || 0) + 1); @@ -1508,8 +1506,9 @@ export class PromptsService extends Disposable implements IPromptsService { status: 'skipped', skipReason: 'parse-error', errorMessage: msg, - extensionId, - source + extension, + source, + pluginUri }); } } @@ -1523,19 +1522,17 @@ export class PromptsService extends Disposable implements IPromptsService { const agentFiles = await this.listPromptFiles(PromptsType.agent, token); for (const promptPath of agentFiles) { - const uri = promptPath.uri; - const storage = promptPath.storage; - const extensionId = promptPath.extension?.identifier?.value; + const { uri, storage, pluginUri, extension } = promptPath; if (disabledAgents.has(uri)) { - files.push({ uri, storage, status: 'skipped', skipReason: 'disabled', extensionId }); + files.push({ uri, storage, status: 'skipped', skipReason: 'disabled', extension, pluginUri }); continue; } try { const ast = await this.parseNew(uri, token); const name = ast.header?.name ?? promptPath.name ?? getCleanPromptName(uri); - files.push({ uri, storage, status: 'loaded', name, extensionId }); + files.push({ uri, storage, status: 'loaded', name, extension, pluginUri }); } catch (e) { files.push({ uri, @@ -1543,7 +1540,8 @@ export class PromptsService extends Disposable implements IPromptsService { status: 'skipped', skipReason: 'parse-error', errorMessage: e instanceof Error ? e.message : String(e), - extensionId + extension, + pluginUri }); } } @@ -1557,14 +1555,12 @@ export class PromptsService extends Disposable implements IPromptsService { const promptFiles = await this.listPromptFiles(PromptsType.prompt, token); for (const promptPath of promptFiles) { - const uri = promptPath.uri; - const storage = promptPath.storage; - const extensionId = promptPath.extension?.identifier?.value; + const { uri, storage, pluginUri, extension } = promptPath; try { const parsedPromptFile = await this.parseNew(uri, token); const name = parsedPromptFile?.header?.name ?? promptPath.name ?? getCleanPromptName(uri); - files.push({ uri, storage, status: 'loaded', name, extensionId }); + files.push({ uri, storage, status: 'loaded', name, extension, pluginUri }); } catch (e) { files.push({ uri, @@ -1572,7 +1568,8 @@ export class PromptsService extends Disposable implements IPromptsService { status: 'skipped', skipReason: 'parse-error', errorMessage: e instanceof Error ? e.message : String(e), - extensionId + extension, + pluginUri }); } } @@ -1588,12 +1585,12 @@ export class PromptsService extends Disposable implements IPromptsService { for (const promptPath of instructionsFiles) { const uri = promptPath.uri; const storage = promptPath.storage; - const extensionId = promptPath.extension?.identifier?.value; + const { pluginUri, extension } = promptPath; try { const parsedPromptFile = await this.parseNew(uri, token); const name = parsedPromptFile?.header?.name ?? promptPath.name ?? getCleanPromptName(uri); - files.push({ uri, storage, status: 'loaded', name, extensionId }); + files.push({ uri, storage, status: 'loaded', name, extension, pluginUri }); } catch (e) { files.push({ uri, @@ -1601,7 +1598,8 @@ export class PromptsService extends Disposable implements IPromptsService { status: 'skipped', skipReason: 'parse-error', errorMessage: e instanceof Error ? e.message : String(e), - extensionId + extension, + pluginUri }); } } @@ -1620,9 +1618,7 @@ export class PromptsService extends Disposable implements IPromptsService { const useClaudeHooks = this.configurationService.getValue(PromptsConfig.USE_CLAUDE_HOOKS); const hookFiles = await this.listPromptFiles(PromptsType.hook, token); for (const promptPath of hookFiles) { - const uri = promptPath.uri; - const storage = promptPath.storage; - const extensionId = promptPath.extension?.identifier?.value; + const { uri, storage, pluginUri, extension } = promptPath; const name = basename(uri); // Ignored if workspace is untrusted @@ -1633,7 +1629,8 @@ export class PromptsService extends Disposable implements IPromptsService { status: 'skipped', skipReason: 'workspace-untrusted', name: basename(promptPath.uri), - extensionId: promptPath.extension?.identifier?.value, + extension, + pluginUri }); continue; } @@ -1646,7 +1643,8 @@ export class PromptsService extends Disposable implements IPromptsService { status: 'skipped', skipReason: 'claude-hooks-disabled', name, - extensionId + extension, + pluginUri }); continue; } @@ -1665,7 +1663,8 @@ export class PromptsService extends Disposable implements IPromptsService { skipReason: 'parse-error', errorMessage: 'Invalid hooks file: must be a JSON object', name, - extensionId + extension, + pluginUri }); continue; } @@ -1685,13 +1684,14 @@ export class PromptsService extends Disposable implements IPromptsService { status: 'skipped', skipReason: 'all-hooks-disabled', name, - extensionId + extension, + pluginUri }); continue; } // File is valid - files.push({ uri, storage, status: 'loaded', name, extensionId }); + files.push({ uri, storage, status: 'loaded', name, extension, pluginUri }); } catch (e) { files.push({ uri, @@ -1700,7 +1700,8 @@ export class PromptsService extends Disposable implements IPromptsService { skipReason: 'parse-error', errorMessage: e instanceof Error ? e.message : String(e), name, - extensionId + extension, + pluginUri }); } } diff --git a/src/vs/workbench/contrib/chat/test/common/promptSyntax/computeAutomaticInstructions.test.ts b/src/vs/workbench/contrib/chat/test/common/promptSyntax/computeAutomaticInstructions.test.ts index 9cf3150e5aec9..738828ed4c228 100644 --- a/src/vs/workbench/contrib/chat/test/common/promptSyntax/computeAutomaticInstructions.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/promptSyntax/computeAutomaticInstructions.test.ts @@ -16,6 +16,7 @@ import { IModelService } from '../../../../../../editor/common/services/model.js import { ModelService } from '../../../../../../editor/common/services/modelService.js'; import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { ExtensionIdentifier, IExtensionDescription } from '../../../../../../platform/extensions/common/extensions.js'; import { IFileService } from '../../../../../../platform/files/common/files.js'; import { FileService } from '../../../../../../platform/files/common/fileService.js'; import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; @@ -33,7 +34,7 @@ import { ComputeAutomaticInstructions, getFilePath, InstructionsCollectionEvent import { PromptsConfig } from '../../../common/promptSyntax/config/config.js'; import { AGENTS_SOURCE_FOLDER, CLAUDE_RULES_SOURCE_FOLDER, INSTRUCTION_FILE_EXTENSION, INSTRUCTIONS_DEFAULT_SOURCE_FOLDER, LEGACY_MODE_DEFAULT_SOURCE_FOLDER, PROMPT_DEFAULT_SOURCE_FOLDER, PROMPT_FILE_EXTENSION } from '../../../common/promptSyntax/config/promptFileLocations.js'; import { INSTRUCTIONS_LANGUAGE_ID, PROMPT_LANGUAGE_ID } from '../../../common/promptSyntax/promptTypes.js'; -import { IPromptsService } from '../../../common/promptSyntax/service/promptsService.js'; +import { IAgentSkill, IPromptsService, PromptsStorage } from '../../../common/promptSyntax/service/promptsService.js'; import { PromptsService } from '../../../common/promptSyntax/service/promptsServiceImpl.js'; import { mockFiles, TestInMemoryFileSystemProviderWithRealPath } from './testUtils/mockFilesystem.js'; import { InMemoryStorageService, IStorageService } from '../../../../../../platform/storage/common/storage.js'; @@ -47,7 +48,7 @@ import { match } from '../../../../../../base/common/glob.js'; import { ChatModeKind } from '../../../common/constants.js'; import { IContextKeyService } from '../../../../../../platform/contextkey/common/contextkey.js'; import { MockContextKeyService } from '../../../../../../platform/keybinding/test/common/mockKeybindingService.js'; -import { IAgentPluginService } from '../../../common/plugins/agentPluginService.js'; +import { IAgentPlugin, IAgentPluginService } from '../../../common/plugins/agentPluginService.js'; import { observableValue } from '../../../../../../base/common/observable.js'; suite('ComputeAutomaticInstructions', () => { @@ -1099,6 +1100,262 @@ suite('ComputeAutomaticInstructions', () => { }); }); + suite('skill telemetry', () => { + test('should emit skillLoadedIntoContext for each loaded skill', async () => { + const rootFolderName = 'skill-telemetry-test'; + const rootFolder = `/${rootFolderName}`; + const rootFolderUri = URI.file(rootFolder); + + workspaceContextService.setWorkspace(testWorkspace(rootFolderUri)); + testConfigService.setUserConfiguration(PromptsConfig.USE_AGENT_SKILLS, true); + + await mockFiles(fileService, [ + { + path: `${rootFolder}/.claude/skills/my-skill/SKILL.md`, + contents: [ + '---', + 'name: \'my-skill\'', + 'description: \'A test skill\'', + '---', + 'Skill content here', + ] + }, + { + path: `${rootFolder}/.claude/skills/other-skill/SKILL.md`, + contents: [ + '---', + 'name: \'other-skill\'', + 'description: \'Another test skill\'', + '---', + 'Other skill content', + ] + }, + ]); + + const telemetryEvents: { eventName: string; data: Record }[] = []; + const mockTelemetryService = { + publicLog2: (eventName: string, data: Record) => { + telemetryEvents.push({ eventName, data }); + } + } as unknown as ITelemetryService; + instaService.stub(ITelemetryService, mockTelemetryService); + + const contextComputer = instaService.createInstance( + ComputeAutomaticInstructions, + ChatModeKind.Agent, + { 'vscode_readFile': true }, + undefined, + undefined + ); + const variables = new ChatRequestVariableSet(); + + await contextComputer.collect(variables, CancellationToken.None); + await new Promise(resolve => setTimeout(resolve, 50)); + + const skillEvents = telemetryEvents.filter(e => e.eventName === 'skillLoadedIntoContext'); + assert.strictEqual(skillEvents.length, 2, 'Should emit one event per skill'); + + // Both events should have hashed skill names (non-empty strings) + for (const event of skillEvents) { + assert.ok(typeof event.data.skillNameHash === 'string' && event.data.skillNameHash.length > 0, 'skillNameHash should be a non-empty string'); + assert.strictEqual(event.data.skillStorage, 'local', 'skillStorage should be local for workspace skills'); + // Local skills have no extension or plugin provenance + assert.strictEqual(event.data.extensionIdHash, '', 'extensionIdHash should be empty for local skills'); + assert.strictEqual(event.data.extensionVersion, '', 'extensionVersion should be empty for local skills'); + assert.strictEqual(event.data.pluginNameHash, '', 'pluginNameHash should be empty for local skills'); + assert.strictEqual(event.data.pluginVersion, '', 'pluginVersion should be empty for local skills'); + } + + // The two events should have different name hashes (different skill names) + assert.notStrictEqual(skillEvents[0].data.skillNameHash, skillEvents[1].data.skillNameHash, 'Different skills should have different name hashes'); + }); + + test('should not emit skillLoadedIntoContext for skills with disableModelInvocation', async () => { + const rootFolderName = 'skill-telemetry-disabled-test'; + const rootFolder = `/${rootFolderName}`; + const rootFolderUri = URI.file(rootFolder); + + workspaceContextService.setWorkspace(testWorkspace(rootFolderUri)); + testConfigService.setUserConfiguration(PromptsConfig.USE_AGENT_SKILLS, true); + + await mockFiles(fileService, [ + { + path: `${rootFolder}/.claude/skills/manual-skill/SKILL.md`, + contents: [ + '---', + 'name: \'manual-skill\'', + 'description: \'A manual-only skill\'', + 'disable-model-invocation: true', + '---', + 'Manual skill content', + ] + }, + { + path: `${rootFolder}/.claude/skills/auto-skill/SKILL.md`, + contents: [ + '---', + 'name: \'auto-skill\'', + 'description: \'An auto-invocable skill\'', + '---', + 'Auto skill content', + ] + }, + ]); + + const telemetryEvents: { eventName: string; data: Record }[] = []; + const mockTelemetryService = { + publicLog2: (eventName: string, data: Record) => { + telemetryEvents.push({ eventName, data }); + } + } as unknown as ITelemetryService; + instaService.stub(ITelemetryService, mockTelemetryService); + + const contextComputer = instaService.createInstance( + ComputeAutomaticInstructions, + ChatModeKind.Agent, + { 'vscode_readFile': true }, + undefined, + undefined + ); + const variables = new ChatRequestVariableSet(); + + await contextComputer.collect(variables, CancellationToken.None); + await new Promise(resolve => setTimeout(resolve, 50)); + + const skillEvents = telemetryEvents.filter(e => e.eventName === 'skillLoadedIntoContext'); + assert.strictEqual(skillEvents.length, 1, 'Should emit only one event (manual skill excluded)'); + assert.strictEqual(skillEvents[0].data.skillStorage, 'local'); + }); + + test('should not emit skillLoadedIntoContext when skills feature is disabled', async () => { + const rootFolderName = 'skill-telemetry-feature-off-test'; + const rootFolder = `/${rootFolderName}`; + const rootFolderUri = URI.file(rootFolder); + + workspaceContextService.setWorkspace(testWorkspace(rootFolderUri)); + testConfigService.setUserConfiguration(PromptsConfig.USE_AGENT_SKILLS, false); + + await mockFiles(fileService, [ + { + path: `${rootFolder}/.claude/skills/some-skill/SKILL.md`, + contents: [ + '---', + 'name: \'some-skill\'', + 'description: \'A skill\'', + '---', + 'Skill content', + ] + }, + ]); + + const telemetryEvents: { eventName: string; data: Record }[] = []; + const mockTelemetryService = { + publicLog2: (eventName: string, data: Record) => { + telemetryEvents.push({ eventName, data }); + } + } as unknown as ITelemetryService; + instaService.stub(ITelemetryService, mockTelemetryService); + + const contextComputer = instaService.createInstance( + ComputeAutomaticInstructions, + ChatModeKind.Agent, + { 'vscode_readFile': true }, + undefined, + undefined + ); + const variables = new ChatRequestVariableSet(); + + await contextComputer.collect(variables, CancellationToken.None); + await new Promise(resolve => setTimeout(resolve, 50)); + + const skillEvents = telemetryEvents.filter(e => e.eventName === 'skillLoadedIntoContext'); + assert.strictEqual(skillEvents.length, 0, 'Should not emit skill telemetry when feature is disabled'); + }); + + test('should emit provenance metadata for extension and plugin skills', async () => { + const rootFolderName = 'skill-telemetry-provenance-test'; + const rootFolder = `/${rootFolderName}`; + const rootFolderUri = URI.file(rootFolder); + + workspaceContextService.setWorkspace(testWorkspace(rootFolderUri)); + testConfigService.setUserConfiguration(PromptsConfig.USE_AGENT_SKILLS, true); + + const stubSkills: IAgentSkill[] = [ + { + uri: URI.file(`${rootFolder}/ext-skills/ext-skill/SKILL.md`), + storage: PromptsStorage.extension, + name: 'ext-skill', + description: 'An extension skill', + disableModelInvocation: false, + userInvocable: true, + extension: { + identifier: new ExtensionIdentifier('publisher.my-extension'), + version: '1.2.3', + } as IExtensionDescription, + }, + { + uri: URI.file(`${rootFolder}/plugin-skills/plugin-skill/SKILL.md`), + storage: PromptsStorage.plugin, + name: 'plugin-skill', + description: 'A plugin skill', + disableModelInvocation: false, + userInvocable: true, + pluginUri: URI.parse('plugin://my-plugin/4.5.6'), + }, + ]; + sinon.stub(service, 'findAgentSkills').resolves(stubSkills); + + // Override the plugin service mock so the plugin skill can be resolved + const pluginUri = URI.parse('plugin://my-plugin/4.5.6'); + instaService.stub(IAgentPluginService, { + plugins: observableValue('testPlugins', [{ + uri: pluginUri, + label: 'my-plugin', + fromMarketplace: { version: '4.5.6' }, + }] as unknown as readonly IAgentPlugin[]), + }); + + const telemetryEvents: { eventName: string; data: Record }[] = []; + const mockTelemetryService = { + publicLog2: (eventName: string, data: Record) => { + telemetryEvents.push({ eventName, data }); + } + } as unknown as ITelemetryService; + instaService.stub(ITelemetryService, mockTelemetryService); + + const contextComputer = instaService.createInstance( + ComputeAutomaticInstructions, + ChatModeKind.Agent, + { 'vscode_readFile': true }, + undefined, + undefined + ); + const variables = new ChatRequestVariableSet(); + + await contextComputer.collect(variables, CancellationToken.None); + await new Promise(resolve => setTimeout(resolve, 50)); + + const skillEvents = telemetryEvents.filter(e => e.eventName === 'skillLoadedIntoContext'); + assert.strictEqual(skillEvents.length, 2, 'Should emit one event per skill'); + + // Extension skill should have extensionId hash and version + const extEvent = skillEvents.find(e => e.data.skillStorage === 'extension'); + assert.ok(extEvent, 'Should have an extension skill event'); + assert.ok(typeof extEvent.data.extensionIdHash === 'string' && extEvent.data.extensionIdHash.length > 0, 'extensionIdHash should be non-empty'); + assert.strictEqual(extEvent.data.extensionVersion, '1.2.3'); + assert.strictEqual(extEvent.data.pluginNameHash, ''); + assert.strictEqual(extEvent.data.pluginVersion, ''); + + // Plugin skill should have plugin name hash and version + const pluginEvent = skillEvents.find(e => e.data.skillStorage === 'plugin'); + assert.ok(pluginEvent, 'Should have a plugin skill event'); + assert.ok(typeof pluginEvent.data.pluginNameHash === 'string' && pluginEvent.data.pluginNameHash.length > 0, 'pluginNameHash should be non-empty'); + assert.strictEqual(pluginEvent.data.pluginVersion, '4.5.6'); + assert.strictEqual(pluginEvent.data.extensionIdHash, ''); + assert.strictEqual(pluginEvent.data.extensionVersion, ''); + }); + }); + suite('instructions list variable', () => { function xmlContents(text: string, tag: string): string[] { const regex = new RegExp(`<${tag}>([\\s\\S]*?)<\\/${tag}>`, 'g');