From dc5a5460f42df342abea739b0edb6080b545faba Mon Sep 17 00:00:00 2001 From: Josh Spicer <23246594+joshspicer@users.noreply.github.com> Date: Mon, 6 Apr 2026 16:03:04 -0700 Subject: [PATCH] ChatSessionCustomizationProvider testing fixes (#308071) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Delete individual hooks instead of entire hooks file When deleting a hook from the Chat Customizations UI, parse the hook item ID to identify the specific entry within the JSON file. Remove only that entry and write the file back, instead of deleting the entire hooks file. If no hooks remain after removal, the file is deleted. Also passes the item ID through the action context so the delete handler can distinguish individual hook entries from whole files. Fixes microsoft/vscode-internalbacklog#7381 * Throw CancellationError when plugin trust dialog is declined When the user cancels the trust dialog during plugin installation, throw a CancellationError instead of silently returning. This allows the install button's catch handler to properly reset to 'Install' state instead of incorrectly showing 'Installed'. Fixes microsoft/vscode-internalbacklog#7378 * Check installed state when rendering marketplace plugin items When rendering marketplace plugin items in the plugin list, check whether the plugin is already installed by comparing install URIs with the agent plugin service's plugin list. Show 'Installed' with the button disabled instead of offering a redundant install option. Fixes microsoft/vscode-internalbacklog#7379 * Replace floppy disk icon with newFile icon for create plugin button The save/floppy disk icon is misleading for a create action. Use the newFile codicon instead, which better communicates the intent and is consistent with other create actions in the UI. Fixes microsoft/vscode-internalbacklog#7373 * Fix plugin page CSS: padding, back button, and focus outlines - Add padding-bottom to marketplace plugin footer for proper Install button alignment (fixes #7377) - Restyle back-to-installed link as a button with muted foreground color and rounded hover background instead of link styling (fixes #7384) - Add outline-offset: -1px to button group buttons to prevent focus outline clipping by parent container (fixes #7385) Fixes microsoft/vscode-internalbacklog#7377 Fixes microsoft/vscode-internalbacklog#7384 Fixes microsoft/vscode-internalbacklog#7385 * Show workspace folder name in provider path for multi-root workspaces When rendering items from external customization providers (e.g. Copilot CLI), use the label service to produce workspace-relative paths for file: URIs instead of bare filenames. This shows the folder name (e.g. 'tankgame • AGENTS.md') in multi-root workspaces, matching the core (Local) path behavior. Fixes microsoft/vscode-internalbacklog#7330 --- .../aiCustomizationListWidget.ts | 6 +- .../aiCustomizationManagement.contribution.ts | 64 ++++++++++++++++++- .../media/aiCustomizationManagement.css | 19 ++++-- .../aiCustomization/pluginListWidget.ts | 26 +++++++- .../chat/browser/pluginInstallService.ts | 3 +- .../plugins/pluginInstallService.test.ts | 5 +- ...aiCustomizationManagementEditor.fixture.ts | 39 +++++++++-- 7 files changed, 145 insertions(+), 17 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts b/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts index 40cdca1609a..5577bb957cb 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts @@ -490,6 +490,7 @@ class AICustomizationItemRenderer implements IListRenderer/SKILL.md. const fileName = isSkill ? basename(dirname(uri)) : basename(uri); @@ -291,9 +323,13 @@ registerAction2(class extends Action2 { } // Confirm deletion + const hookInfo = isHook && itemId ? parseHookItemId(itemId) : undefined; + const hookName = typeof context !== 'string' && !URI.isUri(context) ? context.name : undefined; const message = isSkill ? localize('confirmDeleteSkill', "Are you sure you want to delete skill '{0}' and its folder?", fileName) - : localize('confirmDelete', "Are you sure you want to delete '{0}'?", fileName); + : hookInfo && hookName + ? localize('confirmDeleteHook', "Are you sure you want to delete the '{0}' hook?", hookName) + : localize('confirmDelete', "Are you sure you want to delete '{0}'?", fileName); const confirmation = await dialogService.confirm({ message, detail: localize('confirmDeleteDetail', "This action cannot be undone."), @@ -311,6 +347,32 @@ registerAction2(class extends Action2 { // Telemetry must not block deletion } + // For hooks with a specific hook ID, remove only that entry from the file. + // Uses JSONC edits to preserve user comments and formatting. + if (hookInfo) { + try { + const content = await fileService.readFile(uri); + const text = content.value.toString(); + const edits = removeProperty(text, ['hooks', hookInfo.originalId, hookInfo.index], { tabSize: 1, insertSpaces: false }); + if (edits.length > 0) { + const updated = applyEdits(text, edits); + await fileService.writeFile(uri, VSBuffer.fromString(updated)); + if (storage === PromptsStorage.local) { + const projectRoot = workspaceService.getActiveProjectRoot(); + if (projectRoot) { + await workspaceService.commitFiles(projectRoot, [uri]); + } + } + } + } catch { + await dialogService.error( + localize('deleteHookItemFailed', "Unable to delete this hook entry because the file contents have changed."), + localize('deleteHookItemFailedDetail', "Refresh the view and try again."), + ); + } + return; + } + // For skills, delete the parent folder (e.g. .github/skills/my-skill/) // since each skill is a folder containing SKILL.md. const deleteTarget = isSkill ? dirname(uri) : uri; diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/media/aiCustomizationManagement.css b/src/vs/workbench/contrib/chat/browser/aiCustomization/media/aiCustomizationManagement.css index a3baa33eef2..35551590845 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/media/aiCustomizationManagement.css +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/media/aiCustomizationManagement.css @@ -909,24 +909,32 @@ flex-shrink: 0; } +.mcp-list-widget .list-button-group .monaco-button:focus-visible { + outline-offset: -1px; +} + /* Back to installed link */ .mcp-list-widget .mcp-back-link { display: flex; align-items: center; gap: 4px; - padding: 4px 0; + padding: 4px 6px; cursor: pointer; font-size: 12px; - color: var(--vscode-textLink-foreground); + color: var(--vscode-descriptionForeground); flex-shrink: 0; + border-radius: 4px; + margin: 4px 0; } .mcp-list-widget .mcp-back-link:hover { - color: var(--vscode-textLink-activeForeground); + background-color: var(--vscode-toolbar-hoverBackground); + color: var(--vscode-foreground); } -.mcp-list-widget .mcp-back-link:hover span:last-child { - text-decoration: underline; +.mcp-list-widget .mcp-back-link:focus-visible { + outline: 1px solid var(--vscode-focusBorder); + outline-offset: -1px; } /* Gallery item specifics */ @@ -956,6 +964,7 @@ .mcp-gallery-item.extension-list-item > .details > .footer { display: flex; align-items: center; + padding-bottom: 4px; } .mcp-gallery-item.extension-list-item .mcp-gallery-action { diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/pluginListWidget.ts b/src/vs/workbench/contrib/chat/browser/aiCustomization/pluginListWidget.ts index fa38fa51778..d2fed8eaa28 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/pluginListWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/pluginListWidget.ts @@ -24,7 +24,7 @@ import { ICommandService } from '../../../../../platform/commands/common/command import { CancellationTokenSource } from '../../../../../base/common/cancellation.js'; import { Delayer } from '../../../../../base/common/async.js'; import { IAction, Separator } from '../../../../../base/common/actions.js'; -import { basename, dirname } from '../../../../../base/common/resources.js'; +import { basename, dirname, isEqual } from '../../../../../base/common/resources.js'; import { getDefaultHoverDelegate } from '../../../../../base/browser/ui/hover/hoverDelegateFactory.js'; import { IHoverService } from '../../../../../platform/hover/browser/hover.js'; import { IAgentPlugin, IAgentPluginService } from '../../common/plugins/agentPluginService.js'; @@ -207,6 +207,7 @@ class PluginMarketplaceItemRenderer implements IListRenderer isEqual(p.uri, installUri)); + + if (isAlreadyInstalled) { + templateData.installButton.label = localize('installed', "Installed"); + templateData.installButton.enabled = false; + return; + } + templateData.installButton.label = localize('install', "Install"); templateData.installButton.enabled = true; @@ -398,7 +418,7 @@ export class PluginListWidget extends Disposable { })); const createPluginButton = this._register(new Button(buttonContainer, { ...defaultButtonStyles, secondary: true, supportIcons: true })); - createPluginButton.label = `$(${Codicon.save.id})`; + createPluginButton.label = `$(${Codicon.newFile.id})`; createPluginButton.setTitle(localize('createPlugin', "Create Plugin")); createPluginButton.element.classList.add('list-icon-button'); this._register(this.hoverService.setupManagedHover(getDefaultHoverDelegate('element'), createPluginButton.element, localize('createPluginTooltip', "Create Plugin"))); @@ -456,7 +476,7 @@ export class PluginListWidget extends Disposable { const delegate = new PluginItemDelegate(); const groupHeaderRenderer = new CustomizationGroupHeaderRenderer('pluginGroupHeader', this.hoverService); const installedRenderer = new PluginInstalledItemRenderer(this.harnessService); - const marketplaceRenderer = new PluginMarketplaceItemRenderer(this.pluginInstallService); + const marketplaceRenderer = new PluginMarketplaceItemRenderer(this.pluginInstallService, this.agentPluginService); this.list = this._register(this.instantiationService.createInstance( WorkbenchList, diff --git a/src/vs/workbench/contrib/chat/browser/pluginInstallService.ts b/src/vs/workbench/contrib/chat/browser/pluginInstallService.ts index 9ee703e61e0..32f74585505 100644 --- a/src/vs/workbench/contrib/chat/browser/pluginInstallService.ts +++ b/src/vs/workbench/contrib/chat/browser/pluginInstallService.ts @@ -6,6 +6,7 @@ import { Action } from '../../../../base/common/actions.js'; import { CancellationToken } from '../../../../base/common/cancellation.js'; import { Codicon } from '../../../../base/common/codicons.js'; +import { CancellationError } from '../../../../base/common/errors.js'; import { URI } from '../../../../base/common/uri.js'; import { localize } from '../../../../nls.js'; import { ICommandService } from '../../../../platform/commands/common/commands.js'; @@ -39,7 +40,7 @@ export class PluginInstallService implements IPluginInstallService { async installPlugin(plugin: IMarketplacePlugin): Promise { if (!await this._ensureMarketplaceTrusted(plugin)) { - return; + throw new CancellationError(); } const kind = plugin.sourceDescriptor.kind; diff --git a/src/vs/workbench/contrib/chat/test/browser/plugins/pluginInstallService.test.ts b/src/vs/workbench/contrib/chat/test/browser/plugins/pluginInstallService.test.ts index db448e31734..0ff8f541020 100644 --- a/src/vs/workbench/contrib/chat/test/browser/plugins/pluginInstallService.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/plugins/pluginInstallService.test.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; +import { isCancellationError } from '../../../../../../base/common/errors.js'; import { URI } from '../../../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; @@ -802,7 +803,7 @@ suite('PluginInstallService', () => { sourceDescriptor: { kind: PluginSourceKind.RelativePath, path: 'plugins/myPlugin' }, }); - await service.installPlugin(plugin); + await assert.rejects(() => service.installPlugin(plugin), (err: unknown) => isCancellationError(err as Error)); assert.strictEqual(state.trustedMarketplaces.length, 0); assert.strictEqual(state.addedPlugins.length, 0); @@ -820,7 +821,7 @@ suite('PluginInstallService', () => { ]; for (const sourceDescriptor of kinds) { - await service.installPlugin(createPlugin({ sourceDescriptor })); + await assert.rejects(() => service.installPlugin(createPlugin({ sourceDescriptor })), (err: unknown) => isCancellationError(err as Error)); } assert.strictEqual(state.addedPlugins.length, 0, 'no plugins should be installed when trust is declined'); diff --git a/src/vs/workbench/test/browser/componentFixtures/aiCustomizationManagementEditor.fixture.ts b/src/vs/workbench/test/browser/componentFixtures/aiCustomizationManagementEditor.fixture.ts index 4621ef37369..45051546e30 100644 --- a/src/vs/workbench/test/browser/componentFixtures/aiCustomizationManagementEditor.fixture.ts +++ b/src/vs/workbench/test/browser/componentFixtures/aiCustomizationManagementEditor.fixture.ts @@ -717,13 +717,33 @@ async function renderPluginBrowseMode(ctx: ComponentFixtureContext): Promise([ + ['example/linear-plugin', URI.file('/home/dev/.vscode/agent-plugins/example/linear-plugin')], + ['example/sentry-plugin', URI.file('/home/dev/.vscode/agent-plugins/example/sentry-plugin')], + ['example/datadog-plugin', URI.file('/home/dev/.vscode/agent-plugins/example/datadog-plugin')], + ]); + const instantiationService = createEditorServices(ctx.disposableStore, { colorTheme: ctx.theme, additionalServices: (reg) => { registerWorkbenchServices(reg); reg.define(IListService, ListService); + reg.defineInstance(ICustomizationHarnessService, new class extends mock() { + override readonly activeHarness = observableValue('activeHarness', CustomizationHarness.VSCode); + override getActiveDescriptor() { return createVSCodeHarnessDescriptor([PromptsStorage.extension, BUILTIN_STORAGE]); } + override registerExternalHarness() { return { dispose() { } }; } + }()); reg.defineInstance(IAgentPluginService, new class extends mock() { - override readonly plugins = constObservable([] as readonly IAgentPlugin[]); + override readonly plugins = constObservable(browseInstalledPlugins as readonly IAgentPlugin[]); override readonly enablementModel = undefined!; }()); reg.defineInstance(IPluginMarketplaceService, new class extends mock() { @@ -732,7 +752,10 @@ async function renderPluginBrowseMode(ctx: ComponentFixtureContext): Promise() { - override getPluginInstallUri() { return URI.file('/dev/null'); } + override getPluginInstallUri(plugin: IMarketplacePlugin) { + const repo = plugin.sourceDescriptor.kind === PluginSourceKind.GitHub ? plugin.sourceDescriptor.repo : undefined; + return repo ? (pluginInstallUris.get(repo) ?? URI.file('/dev/null')) : URI.file('/dev/null'); + } }()); }, }); @@ -747,8 +770,16 @@ async function renderPluginBrowseMode(ctx: ComponentFixtureContext): Promise setTimeout(resolve, 50)); + // Wait for the marketplace query to resolve, then wait for scrollbar fade transition + // (visible → invisible takes ~2s after programmatic scroll/list populate) + await new Promise(resolve => setTimeout(resolve, 100)); + // Blur the search input to prevent cursor blink instability in screenshots + (widget.element.querySelector('input') as HTMLElement)?.blur(); + // Force-hide scrollbars to avoid fade-transition instability + for (const scrollbar of widget.element.querySelectorAll('.scrollbar')) { + scrollbar.style.visibility = 'hidden'; + } + await new Promise(resolve => setTimeout(resolve, 200)); } // ============================================================================