chat: unify customization item paths via PromptsServiceCustomizationProvider#308618
chat: unify customization item paths via PromptsServiceCustomizationProvider#308618joshspicer wants to merge 1 commit intomainfrom
Conversation
Screenshot ChangesBase: Changed (24) |
There was a problem hiding this comment.
Pull request overview
This PR refactors the Chat Customizations management UI to use a single provider-based item loading path by extracting the legacy “core discovery” logic into a new PromptsServiceCustomizationProvider that wraps IPromptsService. This reduces complexity in the list widget and aligns Local/CLI harnesses with extension-contributed harness behavior.
Changes:
- Added
PromptsServiceCustomizationProviderimplementingIExternalCustomizationItemProviderto emit customization items fromIPromptsService. - Updated core (workbench) and sessions harness services to attach the new provider to their static harness descriptors.
- Simplified
AICustomizationListWidgetby removing the legacy core-fetch/filter pipeline and always using the provider-based path.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/browser/aiCustomization/promptsServiceCustomizationProvider.ts | New provider that produces IExternalCustomizationItem[] from IPromptsService and emits change events. |
| src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts | Removes legacy core discovery path and routes everything through provider-based loading/grouping. |
| src/vs/workbench/contrib/chat/browser/aiCustomization/customizationHarnessService.ts | Core harness now creates/registers the new provider and attaches it to the Local descriptor. |
| src/vs/sessions/contrib/chat/browser/customizationHarnessService.ts | Sessions harness now creates/registers the new provider and attaches it to the CLI descriptor. |
| src/vs/workbench/contrib/chat/common/customizationHarnessService.ts | CustomizationHarnessServiceBase now extends Disposable for lifecycle management. |
| src/vs/workbench/contrib/chat/test/common/customizationHarnessService.test.ts | Test updated to dispose the base service via store.add(). |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 4
| } | ||
|
|
||
| /** | ||
| /** |
There was a problem hiding this comment.
There’s an extra nested doc-comment opener (/**) here, which makes the comment block look malformed and is likely accidental. Please remove the redundant /** so the JSDoc is a single well-formed block.
| /** |
| // --- Post-processing: storage source filter --- | ||
| const filteredItems = this._applyFilters(items); | ||
|
|
||
| return filteredItems; |
There was a problem hiding this comment.
The comment says this is a “Post-processing: storage source filter”, but _applyFilters currently only applies workspaceSubpaths/instructionFileFilter and never applies the harness’s storage source filter (sources + includedUserFileRoots). This is important for restricted harnesses like CLI, which should not surface user items outside the allowed roots. Please apply descriptor.getStorageSourceFilter(...) filtering (potentially per-item-type) before returning the list.
| result = result.filter(item => { | ||
| if (item.uri.scheme !== Schemas.file || !projectRoot || !item.uri.path.startsWith(projectRoot.path)) { | ||
| return true; | ||
| } | ||
| if (matchesWorkspaceSubpath(item.uri.path, subpaths)) { | ||
| return true; |
There was a problem hiding this comment.
Workspace-subpath filtering is using item.uri.scheme === file plus item.uri.path.startsWith(projectRoot.path). This can fail for remote/workspace URIs (e.g. vscode-remote:) and can produce false positives on path prefixes (e.g. /proj/foo vs /proj/foobar). Consider using isEqualOrParent(item.uri, projectRoot) (or an equivalent URI-aware parent check) and avoid hard-coding the scheme to file so the filter works consistently in remote workspaces too.
| private async fetchItemsForSection(section: AICustomizationManagementSection): Promise<IAICustomizationListItem[]> { | ||
| const promptType = sectionToPromptType(section); | ||
| const activeDescriptor = this.harnessService.getActiveDescriptor(); | ||
|
|
||
| if (activeDescriptor.itemProvider && promptType) { | ||
| return this.fetchProviderItemsForSection(activeDescriptor, promptType); | ||
| } | ||
|
|
||
| return this.fetchCoreItemsForSection(promptType); | ||
| return this.fetchProviderItemsForSection(this.harnessService.getActiveDescriptor(), promptType); | ||
| } |
There was a problem hiding this comment.
Now that all harnesses (including Local/CLI) go through the provider-based path, items coming from PromptsService will frequently have groupKey set (built-in, instruction categories, etc.). In fetchItemsFromProvider, the current mapping skips _inferStorageAndGroup when groupKey is present, leaving storage undefined, which can break context-key when-clauses that rely on AI_CUSTOMIZATION_ITEM_STORAGE_KEY (e.g. read-only/delete/enable-disable actions). Please ensure storage is still inferred/set even when a provider supplies a custom groupKey (grouping and storage shouldn’t be mutually exclusive).
981665a to
1aa6371
Compare
…rovider Extract fetchCoreItemsForSection from the list widget into a new PromptsServiceCustomizationProvider class that implements IExternalCustomizationItemProvider. It emits IExternalCustomizationItem[] with rich metadata (groupKey, badge, description, enabled) so the widget's single provider-based code path handles all harnesses. The Local (VSCode) and Sessions CLI harness services now create a PromptsServiceCustomizationProvider and wire it as the itemProvider on their static harness descriptors. This eliminates the if/else branching in fetchItemsForSection and filterItems, and removes ~500 lines of legacy core-path code from the widget: - fetchCoreItemsForSection (~300 lines) - filterItemsForCore (~40 lines) - applyBuiltinGroupKeys (~25 lines) - storageToIcon, findPluginUri, isBuiltin/extensionLabel fields - Unused imports (dirname, ResourceSet, applyStorageSourceFilter, etc.) CustomizationHarnessServiceBase now extends Disposable for lifecycle management of the provider's event subscriptions.
1aa6371 to
757195e
Compare
Summary
Extracts the ~300-line
fetchCoreItemsForSectionfrom the AI Customizations list widget into a newPromptsServiceCustomizationProviderclass that implementsIExternalCustomizationItemProvider. The widget now uses a single provider-based code path for all harnesses — static (Local, CLI) and extension-contributed.Follows #308590 which removed the kill-switch setting and Claude harness.
What changed (6 files, +406 / -509 lines)
promptsServiceCustomizationProvider.tsIPromptsService. EmitsIExternalCustomizationItem[]with semanticgroupKey(agent-instructions,context-instructions,on-demand-instructions,agents,BUILTIN_STORAGE),badge/badgeTooltipfor applyTo patterns,enabled: falsefor disabled itemsaiCustomizationListWidget.tsfetchCoreItemsForSection(~300 lines),filterItemsForCore(~40),applyBuiltinGroupKeys(~25),storageToIcon,isBuiltin/extensionLabelfields,if/elsebranching infilterItems(). Simplified hover tooltips and copy actions. Cleaned up unused importsbrowser/customizationHarnessService.tsPromptsServiceCustomizationProviderand spreadsitemProvideronto Local descriptorsessions/.../customizationHarnessService.tscommon/customizationHarnessService.tsCustomizationHarnessServiceBasenow extendsDisposablefor lifecycle managementcustomizationHarnessService.test.tsstore.add()Widget size reduction
~2250 lines → ~1770 lines (-480 lines). The deleted code was the complete "core discovery path" that only the Local harness used. It's now handled by the provider class (~340 lines) which emits items in the same format as extension-contributed providers.
Verification