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(); } } 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(); } 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/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; 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/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/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/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, 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/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/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/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/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/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/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/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/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/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/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts index d840ad32a8779..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 }); @@ -1193,7 +1187,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer { } } -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/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 () => { 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 { 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/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/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/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'}`); - } })); } } 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, { 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/browser/widget/chatContentParts/chatThinkingContentPart.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatThinkingContentPart.test.ts index 8717a5dee30b8..f8be60f7623fc 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatThinkingContentPart.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatThinkingContentPart.test.ts @@ -1518,7 +1518,7 @@ suite('ChatThinkingContentPart', () => { 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', () => { 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/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/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'); 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/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, 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])); 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.")); 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); 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 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; }