chat: fix: honor disabled built-in skills in the Agents window - #329786
Merged
Conversation
The Enable/Disable actions on a built-in skill persist to IPromptsService.setDisabledPromptFiles, but nothing in the Agents window read that store, so the toggle was a no-op. Fix both halves. The wire (enumerateLocalCustomizationsForHarness) now treats a file as opted out when either the per-harness sync provider or the prompts service says so, keeping a disabled skill out of the bundle. The list (PureItemProviderItemSource) merges built-in skills in from the prompts service, so a skill that has just left the bundle stays listed as disabled instead of vanishing with no way to re-enable it. Scope the wire gate with isUserToggleableCustomization. getDisabledPromptFiles is a shared store that the chat view agent picker also writes for agents, and the Agents-window lists are derived from the bundle, so hiding an entry the Customizations UI cannot re-enable would strand it permanently. Fixes #329751 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7522648e-1a2f-436a-9f19-ab220d5444f2
Copilot started reviewing on behalf of
Ulugbek Abdullaev (ulugbekna)
August 8, 2026 22:44
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes disabled built-in skills in the Agents window so they remain manageable while being excluded from agent-host bundles.
Changes:
- Honors the prompts-service disabled set when bundling built-in skills.
- Keeps disabled skills listed by merging authoritative built-ins into provider results.
- Adds regression coverage and architecture documentation.
Show a summary per file
| File | Description |
|---|---|
aiCustomizationItemsModel.test.ts |
Tests disabled built-in list entries. |
resolveCustomizationRefs.test.ts |
Tests bundle exclusion. |
enumerateLocalCustomizationsForHarness.test.ts |
Tests disabled-store scope. |
promptsService.ts |
Adds toggleability predicate. |
aiCustomizationItemSource.ts |
Shares built-in merge logic across sources. |
aiCustomizationItemsModel.ts |
Injects merge dependencies. |
agentHostLocalCustomizations.ts |
Applies disabled state during enumeration. |
AI_CUSTOMIZATIONS.md |
Documents enable/disable architecture. |
Review details
Tip
Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 8/8 changed files
- Comments generated: 5
- Review effort level: Balanced
Ulugbek Abdullaev (ulugbekna)
enabled auto-merge (squash)
August 8, 2026 22:47
Consolidate the scope-gate rationale in AI_CUSTOMIZATIONS.md and reduce the three JSDoc blocks to their contracts plus a pointer, so the invariant has one source of truth instead of drifting across four. Correct the spec: it claimed both disabled stores are honored everywhere, but mergeBuiltinSkills intentionally derives list state from the prompts-service store alone. Document that asymmetry and why it is sound. Add a regression test for the onDidChangeSkills refresh path, which was wired but only covered at initial state. The test settles construction-time refetches first, so it fails if the subscription is removed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7522648e-1a2f-436a-9f19-ab220d5444f2
roblourens (roblourens)
approved these changes
Aug 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #329751
Built-in skills in the Agents window's AI Customizations UI show an Enable/Disable action, but clicking Disable did nothing — the skill stayed enabled and kept working.
Root cause
The Enable/Disable actions persist to
IPromptsService.setDisabledPromptFiles, but nothing in the Agents window read that store:enumerateLocalCustomizationsForHarness) consulted only the per-harnessICustomizationSyncProvideropt-out, so a disabled skill was still bundled and shipped to the agent host.PureItemProviderItemSource(isAgentHostTarget()), which derived items purely from the provider and never consulted the disabled set.So the toggle wrote to a store that had no readers on this path.
Fix
The wire now treats a file as opted out when either store disables it, keeping a disabled skill out of the synthetic bundle.
The list merges built-in skills in from
listPromptFilesForStorage(skill, builtIn)(sharedmergeBuiltinSkillshelper, deduped by URI) and derivesenabledfromgetDisabledPromptFiles.Both halves are required. The bundle protocol has no per-file "disabled" flag, so omission is the only way to disable — which means with only the wire fix the provider stops reporting the skill, the row vanishes, and it can never be re-enabled (the Enable action is only rendered for listed rows). The merge keeps it listed as disabled. Dedupe works because
_applySyncedOriginrestores the original built-in URI.ItemProviderItemSourcealready did this; the shared helper bringsPureItemProviderItemSourceto parity.Scope gate
getDisabledPromptFilesis a shared store — the chat view agent picker also writes it forPromptsType.agent("hidden from agent picker"). Since the Agents-window lists are derived from the bundle, honoring it for a(type, storage)the Customizations UI cannot re-enable would strand that entry permanently: the row disappears, and the action that would restore it only renders for listed rows.New predicate
isUserToggleableCustomization(type, storage)mirrors the menuwhenclauses (built-in skills only) and gates the wire accordingly. It lives beside the store it describes so future consumers find the rule at the source. Behavior for every other(type, storage)is byte-for-byte unchanged — the wire never consulted this store before.Testing
typecheck-clientclean (error count matches a stashed baseline);valid-layers-checkclean.src/vs/sessions/AI_CUSTOMIZATIONS.md, including why the gate is load-bearing.Review
Reviewed by four frontier models independently verifying from source. All four confirmed the root cause and the necessity of both halves, and all four rejected the alternatives considered (registering a built-in harness, replacing
PureItemProviderItemSource, consolidating the stores, filtering insidePromptsService— that last one would break the restore path).One reviewer noted
mergeBuiltinSkillsderivesenabledonly from the prompts store, ignoring the sync-provider store. Verified currently unreachable:isDisabledmatches URIs exactly, and its only writer passes plugin URIs, so a built-in skill URI can never enter that set. Threading it through would render a disabled row whose Enable button couldn't clear it (Enable writes only the prompts store), so the invariant is documented in the helper instead, along with what must change if a per-file sync opt-out is ever added.Known follow-ups
isUserToggleableCustomizationentirely.extractSourcebug, deliberately kept to a separate PR (zero file overlap).Not exercised in a running build — evidence is static analysis plus fail-without-fix test verification.