Add ability to disable individual customizations#311806
Add ability to disable individual customizations#311806
Conversation
|
Base:
|
Replace the per-URI checkbox sync model with an auto-sync approach where all eligible local customizations are automatically synced to remote agents. Users opt OUT of syncing specific files rather than opting in. Changes: AgentCustomizationDisableProvider The new provider tracks per-URI `disabled` state instead of per-URI `synced` state. Storage key migrated from `<harnessId>.customizationDisabled`. - Remove ICustomizationSyncProvider from IHarnessDescriptor; replace with ICustomizationDisableProvider (isDisabled/setDisabled API) - Remove Checkbox UI from the syncaiCustomizationListWidget checkboxes, template data, and toggle logic are all removed. A future PR (#311806) will add a proper disable toggle UI. - Clean up remove syncProviderProviderCustomizationItemSource coupling; the item source no longer merges synced/unsynced state. - pluginListWidget: remove syncProvider references Tests: delete agentCustomizationSyncProvider.test.ts (old opt-in model), add agentCustomizationDisableProvider.test.ts (new opt-out model). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
pwang347
requested review from
DonJayamanne,
TylerLeonhardt,
aeschli and
joshspicer
| * instead of the individual item, and the item's own | ||
| * {@link enablementScope} is ignored. | ||
| */ | ||
| readonly pluginUri?: Uri; |
There was a problem hiding this comment.
(shared with you IRL too) This feels like the odd man out in the API, especially since a plugin itself is a ChatSessionCustomizationType
| * Whether this customization is currently enabled. | ||
| * Defaults to `true` when omitted. | ||
| */ | ||
| readonly enabled?: boolean; |
There was a problem hiding this comment.
I generally don't like default to true as it means conditional checks don't work nicely in js. Could this be something like: disabled?: { reason: string }
There was a problem hiding this comment.
Part of what is confusing me is that there are a few different concepts here:
- Is the contribution actually enabled / disabled?
- Can the contribution be enabled / disabled by the user?
I feel like we need clearer names around this
Also making it more complicated is if a setting is user configurable, then enabled just reflects the initial state, right? Or is the extension supposed to update this value in response to when the command is fired?
There was a problem hiding this comment.
Thanks @mjbvz, agree that disabled?: { reason: string } would be a cleaner pattern and removes the need for an extra property.
To be clear, the API here is for presentation purposes only and to allow a user to configure settings from this panel. I don't think it is a goal for this to be always 100% in-sync with the actual contents, rather it would update when the view is refreshed (e.g., by changing tabs). Maybe @joshspicer might have some more thoughts on the lifecycle of this.
Another thing we could do to better group the "Can the contribution be enabled / disabled by the user?" is to put it under a grouped object like disableAction?: { command: Command, scopeHint: ChatSessionCustomizationEnablementScope } but that doesn't seem to be a common pattern in our API.
| * {@link ChatSessionCustomizationProvider.onDidChange} so the UI | ||
| * re-queries the updated state. | ||
| */ | ||
| readonly enablementCommand?: string; |
Fixes #290290