From 4eab4042e5814c2921da2d236ef73086dc5629d0 Mon Sep 17 00:00:00 2001 From: Josh Spicer <23246594+joshspicer@users.noreply.github.com> Date: Tue, 31 Mar 2026 17:07:00 -0700 Subject: [PATCH] Harness fixes (#306978) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: hide hardcoded CLI/Claude harnesses when providerApi is enabled When `chat.customizations.providerApi.enabled` is true, the browser harness service now only registers the Local (VS Code) harness statically. All additional harnesses are expected to come from extensions via the provider API rather than the hardcoded built-ins. - CustomizationHarnessService reads the providerApi setting at construction time and only includes CLI/Claude harnesses when the setting is off - Simplify _getAllHarnesses(): external harnesses no longer shadow static ones with the same id (the overlap case can't happen when providerApi is on and only Local is registered) - Simplify registerExternalHarness dispose: drop the 'restored static harness' fallback guard — always fall back to first available harness - Remove tests that verified the now-deleted external-overrides-static behaviour * fix: extension menu contributions now evaluated for the hooks section buildCreateActions() had an early 'return actions' for the hooks code path that exited before the extension menu contribution check. This meant extensions contributing to chat/customizations/create with a 'aiCustomizationManagementSection == hooks' when-clause were silently ignored. Move the AICustomizationManagementCreateMenuId evaluation (and its 'if (extensionCreateActions.length > 0) return extensionCreateActions' short-circuit) to immediately after the commandId override check and before the hooks-specific block, so extension-contributed actions take precedence for all section types including hooks. --- .../aiCustomizationListWidget.ts | 57 ++++++++++--------- .../customizationHarnessService.ts | 38 ++++++++++--- .../common/customizationHarnessService.ts | 5 +- .../customizationHarnessService.test.ts | 16 +++--- 4 files changed, 69 insertions(+), 47 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts b/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts index 19ef96d30ae..de35ad390b7 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts @@ -968,6 +968,35 @@ export class AICustomizationListWidget extends Disposable { }]; } + // Check for menu-contributed create actions from extensions. + // Extensions contribute to AICustomizationManagementCreateMenuId with + // when-clauses targeting aiCustomizationManagementHarness and + // aiCustomizationManagementSection context keys. + // When a harness contributes create actions, they REPLACE the built-in ones + // for all section types, including hooks. + const menuActions = this.menuService.getMenuActions( + AICustomizationManagementCreateMenuId, + this.contextKeyService, + { shouldForwardArgs: true }, + ); + const extensionCreateActions: ICreateAction[] = []; + for (const [, group] of menuActions) { + for (const menuItem of group) { + if (menuItem instanceof MenuItemAction) { + const icon = ThemeIcon.isThemeIcon(menuItem.item.icon) ? menuItem.item.icon.id : Codicon.add.id; + extensionCreateActions.push({ + label: `$(${icon}) ${typeof menuItem.item.title === 'string' ? menuItem.item.title : menuItem.item.title.value}`, + enabled: menuItem.enabled, + run: () => { menuItem.run(); }, + }); + } + } + } + + if (extensionCreateActions.length > 0) { + return extensionCreateActions; + } + const createTypeLabel = override?.typeLabel ?? typeLabel; const actions: ICreateAction[] = []; const addedTargets = new Set(); @@ -1069,34 +1098,6 @@ export class AICustomizationListWidget extends Disposable { } } - // Check for menu-contributed create actions from extensions. - // Extensions contribute to AICustomizationManagementCreateMenuId with - // when-clauses targeting aiCustomizationManagementHarness and - // aiCustomizationManagementSection context keys. - // When a harness contributes create actions, they REPLACE the built-in ones. - const menuActions = this.menuService.getMenuActions( - AICustomizationManagementCreateMenuId, - this.contextKeyService, - { shouldForwardArgs: true }, - ); - const extensionCreateActions: ICreateAction[] = []; - for (const [, group] of menuActions) { - for (const menuItem of group) { - if (menuItem instanceof MenuItemAction) { - const icon = ThemeIcon.isThemeIcon(menuItem.item.icon) ? menuItem.item.icon.id : Codicon.add.id; - extensionCreateActions.push({ - label: `$(${icon}) ${typeof menuItem.item.title === 'string' ? menuItem.item.title : menuItem.item.title.value}`, - enabled: menuItem.enabled, - run: () => { menuItem.run(); }, - }); - } - } - } - - if (extensionCreateActions.length > 0) { - return extensionCreateActions; - } - return actions; } diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/customizationHarnessService.ts b/src/vs/workbench/contrib/chat/browser/aiCustomization/customizationHarnessService.ts index 417546fdca2..5eaf863df09 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/customizationHarnessService.ts +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/customizationHarnessService.ts @@ -18,26 +18,46 @@ import { import { PromptsStorage } from '../../common/promptSyntax/service/promptsService.js'; import { BUILTIN_STORAGE } from '../../common/aiCustomizationWorkspaceService.js'; import { IPathService } from '../../../../services/path/common/pathService.js'; +import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; +import { ChatConfiguration } from '../../common/constants.js'; /** * Core implementation of the customization harness service. - * Exposes VS Code, CLI, and Claude harnesses for filtering customizations. + * + * When `chat.customizations.providerApi.enabled` is true, only the Local + * harness is registered statically. All other harnesses are contributed by + * extensions via the provider API, so the hardcoded CLI/Claude harnesses are + * intentionally omitted. + * + * When the setting is false, the full set of built-in harnesses (Local, Copilot + * CLI, Claude) is registered for backwards compat. */ class CustomizationHarnessService extends CustomizationHarnessServiceBase { constructor( @IPathService pathService: IPathService, + @IConfigurationService configurationService: IConfigurationService, ) { - const userHome = pathService.userHome({ preferLocal: true }); // The Local harness includes extension-contributed and built-in customizations. // Built-in items come from the default chat extension (productService.defaultChatAgent). - // CLI and Claude harnesses don't consume extension contributions. const localExtras = [PromptsStorage.extension, BUILTIN_STORAGE]; - const restrictedExtras: readonly string[] = []; - const allHarnesses: readonly IHarnessDescriptor[] = [ - createVSCodeHarnessDescriptor(localExtras), - createCliHarnessDescriptor(getCliUserRoots(userHome), restrictedExtras), - createClaudeHarnessDescriptor(getClaudeUserRoots(userHome), restrictedExtras), - ]; + + const providerApiEnabled = configurationService.getValue(ChatConfiguration.CustomizationsProviderApi); + + let allHarnesses: readonly IHarnessDescriptor[]; + if (providerApiEnabled) { + // When the provider API is enabled, only expose the Local harness. + // CLI and Claude harnesses don't consume extension contributions. + // Additional harnesses are contributed entirely via the provider API. + allHarnesses = [createVSCodeHarnessDescriptor(localExtras)]; + } else { + const userHome = pathService.userHome({ preferLocal: true }); + const restrictedExtras: readonly string[] = []; + allHarnesses = [ + createVSCodeHarnessDescriptor(localExtras), + createCliHarnessDescriptor(getCliUserRoots(userHome), restrictedExtras), + createClaudeHarnessDescriptor(getClaudeUserRoots(userHome), restrictedExtras), + ]; + } super( allHarnesses, diff --git a/src/vs/workbench/contrib/chat/common/customizationHarnessService.ts b/src/vs/workbench/contrib/chat/common/customizationHarnessService.ts index 1a8a1dd1637..9be9b54433c 100644 --- a/src/vs/workbench/contrib/chat/common/customizationHarnessService.ts +++ b/src/vs/workbench/contrib/chat/common/customizationHarnessService.ts @@ -483,7 +483,8 @@ export class CustomizationHarnessServiceBase implements ICustomizationHarnessSer } private _getAllHarnesses(): readonly IHarnessDescriptor[] { - // External harnesses override static ones with the same id + // External harnesses shadow static ones with the same id so that + // extension-contributed harnesses can upgrade a built-in entry. const externalIds = new Set(this._externalHarnesses.map(h => h.id)); return [ ...this._staticHarnesses.filter(h => !externalIds.has(h.id)), @@ -505,7 +506,7 @@ export class CustomizationHarnessServiceBase implements ICustomizationHarnessSer this._externalHarnesses.splice(idx, 1); this._refreshAvailableHarnesses(); // If the removed harness was active, only fall back when no - // remaining harness (e.g. a restored static one) shares the id. + // remaining harness (e.g. the restored static one) shares the id. if (this._activeHarness.get() === descriptor.id) { const all = this._getAllHarnesses(); if (!all.some(h => h.id === descriptor.id) && all.length > 0) { diff --git a/src/vs/workbench/contrib/chat/test/common/customizationHarnessService.test.ts b/src/vs/workbench/contrib/chat/test/common/customizationHarnessService.test.ts index 350b43e7a35..f9016d83264 100644 --- a/src/vs/workbench/contrib/chat/test/common/customizationHarnessService.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/customizationHarnessService.test.ts @@ -197,7 +197,7 @@ suite('CustomizationHarnessService', () => { test('external harness with same id as static harness replaces it', () => { const staticDescriptor: IHarnessDescriptor = { id: 'cli', - label: 'Copilot CLI', + label: 'Copilot CLI (static)', icon: ThemeIcon.fromId('extensions'), getStorageSourceFilter: () => ({ sources: [PromptsStorage.local] }), }; @@ -223,16 +223,16 @@ suite('CustomizationHarnessService', () => { const reg = service.registerExternalHarness(externalDescriptor); store.add(reg); - // Should still be 2, not 3 — the external replaces the static + // Should still be 2, not 3 — the external shadows the static assert.strictEqual(service.availableHarnesses.get().length, 2); const cliHarness = service.availableHarnesses.get().find(h => h.id === 'cli')!; assert.strictEqual(cliHarness.label, 'Copilot CLI (from API)'); }); - test('static harness reappears when replacing external harness is disposed', () => { + test('static harness reappears when shadowing external harness is disposed', () => { const staticDescriptor: IHarnessDescriptor = { id: 'cli', - label: 'Copilot CLI', + label: 'Copilot CLI (static)', icon: ThemeIcon.fromId('extensions'), getStorageSourceFilter: () => ({ sources: [PromptsStorage.local] }), }; @@ -260,13 +260,13 @@ suite('CustomizationHarnessService', () => { // Static harness should be back assert.strictEqual(service.availableHarnesses.get().length, 2); const cliHarness = service.availableHarnesses.get().find(h => h.id === 'cli')!; - assert.strictEqual(cliHarness.label, 'Copilot CLI'); + assert.strictEqual(cliHarness.label, 'Copilot CLI (static)'); }); - test('active harness stays when overriding external harness is disposed', () => { + test('active harness stays when shadowing external harness is disposed (static restored)', () => { const staticDescriptor: IHarnessDescriptor = { id: 'cli', - label: 'Copilot CLI', + label: 'Copilot CLI (static)', icon: ThemeIcon.fromId('extensions'), getStorageSourceFilter: () => ({ sources: [PromptsStorage.local] }), }; @@ -294,7 +294,7 @@ suite('CustomizationHarnessService', () => { reg.dispose(); - // Active harness should stay on 'cli' — the static one is restored + // Active stays on 'cli' because the static harness with the same id is restored assert.strictEqual(service.activeHarness.get(), 'cli'); }); });