ChatSessionCustomizationProvider testing fixes (#308071)

* 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
This commit is contained in:
Josh Spicer
2026-04-06 16:03:04 -07:00
committed by GitHub
parent d843b57fb4
commit dc5a5460f4
7 changed files with 145 additions and 17 deletions

View File

@@ -490,6 +490,7 @@ class AICustomizationItemRenderer implements IListRenderer<IFileItemEntry, IAICu
promptType: element.promptType,
storage: element.storage,
pluginUri: element.pluginUri?.toString(),
itemId: element.id,
};
// Create scoped context key service with item-specific keys for when-clause filtering
@@ -824,6 +825,7 @@ export class AICustomizationListWidget extends Disposable {
promptType: item.promptType,
storage: item.storage,
pluginUri: item.pluginUri?.toString(),
itemId: item.id,
};
// Create scoped context key service with item-specific keys for when-clause filtering
@@ -1677,7 +1679,9 @@ export class AICustomizationListWidget extends Disposable {
id: item.uri.toString(),
uri: item.uri,
name: item.name,
filename: basename(item.uri),
filename: item.uri.scheme === Schemas.file
? this.labelService.getUriLabel(item.uri, { relative: true })
: basename(item.uri),
description: item.description ?? descriptionsByUri.get(item.uri),
promptType,
disabled: item.enabled === false,

View File

@@ -5,11 +5,13 @@
import { Codicon } from '../../../../../base/common/codicons.js';
import { MarkdownString } from '../../../../../base/common/htmlContent.js';
import { applyEdits, removeProperty } from '../../../../../base/common/jsonEdit.js';
import { Disposable } from '../../../../../base/common/lifecycle.js';
import { Schemas } from '../../../../../base/common/network.js';
import { isMacintosh, isWindows } from '../../../../../base/common/platform.js';
import { basename, dirname, isEqualOrParent } from '../../../../../base/common/resources.js';
import { URI } from '../../../../../base/common/uri.js';
import { VSBuffer } from '../../../../../base/common/buffer.js';
import { getCodeEditor } from '../../../../../editor/browser/editorBrowser.js';
import { localize, localize2 } from '../../../../../nls.js';
import { Categories } from '../../../../../platform/action/common/actionCommonCategories.js';
@@ -171,6 +173,34 @@ function extractPluginUri(context: AICustomizationContext): URI | undefined {
return URI.isUri(raw) ? raw : typeof raw === 'string' ? URI.parse(raw) : undefined;
}
/**
* Extracts the item ID from context (used for identifying individual hooks within a file).
*/
function extractItemId(context: AICustomizationContext): string | undefined {
if (URI.isUri(context) || typeof context === 'string') {
return undefined;
}
return typeof context.itemId === 'string' ? context.itemId : undefined;
}
/**
* Parses a hook item ID to extract the original hook type ID and array index.
* Hook item IDs have the format: `fileUri#originalId[index]`
* Returns undefined if the ID does not match this format.
*/
function parseHookItemId(itemId: string): { originalId: string; index: number } | undefined {
const hashIndex = itemId.lastIndexOf('#');
if (hashIndex < 0) {
return undefined;
}
const fragment = itemId.substring(hashIndex + 1);
const match = /^([^[]+)\[(\d+)\]$/.exec(fragment);
if (!match) {
return undefined;
}
return { originalId: match[1], index: parseInt(match[2], 10) };
}
// Open file action
const OPEN_AI_CUSTOMIZATION_MGMT_FILE_ID = 'aiCustomizationManagement.openFile';
registerAction2(class extends Action2 {
@@ -259,7 +289,9 @@ registerAction2(class extends Action2 {
const uri = extractURI(context);
const storage = extractStorage(context);
const promptType = extractPromptType(context);
const itemId = extractItemId(context);
const isSkill = promptType === PromptsType.skill;
const isHook = promptType === PromptsType.hook;
// For skills, use the parent folder name since skills are structured as <skillname>/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;

View File

@@ -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 {

View File

@@ -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<IPluginMarketplaceI
constructor(
private readonly pluginInstallService: IPluginInstallService,
private readonly agentPluginService: IAgentPluginService,
) { }
renderTemplate(container: HTMLElement): IPluginMarketplaceItemTemplateData {
@@ -236,6 +237,25 @@ class PluginMarketplaceItemRenderer implements IListRenderer<IPluginMarketplaceI
templateData.publisher.textContent = element.item.marketplace ? localize('byPublisher', "by {0}", element.item.marketplace) : '';
templateData.description.textContent = element.item.description || '';
// Check if the plugin is already installed by comparing install URIs
const installUri = this.pluginInstallService.getPluginInstallUri({
name: element.item.name,
description: element.item.description,
version: '',
sourceDescriptor: element.item.sourceDescriptor,
source: element.item.source,
marketplace: element.item.marketplace,
marketplaceReference: element.item.marketplaceReference,
marketplaceType: element.item.marketplaceType,
});
const isAlreadyInstalled = this.agentPluginService.plugins.get().some(p => 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<IPluginGroupHeaderEntry>('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<IPluginListEntry>,

View File

@@ -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<void> {
if (!await this._ensureMarketplaceTrusted(plugin)) {
return;
throw new CancellationError();
}
const kind = plugin.sourceDescriptor.kind;

View File

@@ -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');

View File

@@ -717,13 +717,33 @@ async function renderPluginBrowseMode(ctx: ComponentFixtureContext): Promise<voi
ctx.container.style.width = `${width}px`;
ctx.container.style.height = `${height}px`;
// Some marketplace plugins match installed plugins by URI so the renderer
// shows them as "Installed" (exercises the installed-state check from #7379).
const browseInstalledPlugins = [
makeInstalledPlugin('Linear', URI.file('/home/dev/.vscode/agent-plugins/example/linear-plugin'), true),
makeInstalledPlugin('Sentry', URI.file('/home/dev/.vscode/agent-plugins/example/sentry-plugin'), true),
makeInstalledPlugin('Datadog', URI.file('/home/dev/.vscode/agent-plugins/example/datadog-plugin'), false),
];
// Map plugin source descriptors to install URIs, matching installed URIs above
const pluginInstallUris = new Map<string, URI>([
['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<ICustomizationHarnessService>() {
override readonly activeHarness = observableValue<string>('activeHarness', CustomizationHarness.VSCode);
override getActiveDescriptor() { return createVSCodeHarnessDescriptor([PromptsStorage.extension, BUILTIN_STORAGE]); }
override registerExternalHarness() { return { dispose() { } }; }
}());
reg.defineInstance(IAgentPluginService, new class extends mock<IAgentPluginService>() {
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<IPluginMarketplaceService>() {
@@ -732,7 +752,10 @@ async function renderPluginBrowseMode(ctx: ComponentFixtureContext): Promise<voi
override async fetchMarketplacePlugins() { return marketplacePlugins; }
}());
reg.defineInstance(IPluginInstallService, new class extends mock<IPluginInstallService>() {
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<voi
const browseButton = widget.element.querySelector('.list-add-button') as HTMLElement;
browseButton?.click();
// Wait for the marketplace query to resolve
await new Promise(resolve => 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<HTMLElement>('.scrollbar')) {
scrollbar.style.visibility = 'hidden';
}
await new Promise(resolve => setTimeout(resolve, 200));
}
// ============================================================================