sessions: enable customizations UI for all session types with deep-link navigation#312398
sessions: enable customizations UI for all session types with deep-link navigation#312398TylerLeonhardt merged 2 commits intomainfrom
Conversation
…nk navigation Previously the customizations sidebar only worked for copilotcli sessions. This change extends support to any session type that has a registered content provider, making it work for claude-code and other AHP harnesses. - Remove hardcoded copilotcli gate in mainThreadChatAgents2; now any session type with a registered content provider schema is accepted - Wire ICustomizationHarnessService into count widgets so they use the active harness's itemProvider when available (avoids count mismatches for remote item providers) - Add getActiveItemProvider() utility and update getCustomizationTotalCount() to accept an optional itemProvider for harness-backed sessions - Each sidebar customization link now deep-links to its section in the management editor via selectSectionById - Use reader.store pattern instead of MutableDisposable + manual rebind for itemProvider change subscriptions - Add tests for getActiveItemProvider and getCustomizationTotalCount with itemProvider; add ICustomizationHarnessService mock to widget fixture - Update AI_CUSTOMIZATIONS.md to reflect the new multi-harness sessions behavior
There was a problem hiding this comment.
Pull request overview
Enables the Sessions window customizations UI for any session type that has an actual chat session content provider, and updates the Sessions sidebar customizations widgets to deep-link into the management editor and to use harness-backed item providers for counts.
Changes:
- Relaxed Sessions-window gating for extension-registered customization harnesses to allow any session type with a registered content provider scheme.
- Updated Sessions customizations sidebar widgets to (a) use the active harness’s
itemProviderfor counts where available and (b) deep-link into the management editor viaselectSectionById. - Added shared
getActiveItemProvider()utility plus unit tests and updated widget test fixtures to includeICustomizationHarnessService.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/api/browser/mainThreadChatAgents2.ts | Changes Sessions-window gating for extension customization providers from a hardcoded type to “has content provider scheme”. |
| src/vs/sessions/contrib/sessions/browser/customizationCounts.ts | Adds getActiveItemProvider() and updates total counting to optionally use an item provider. |
| src/vs/sessions/contrib/sessions/browser/customizationsToolbar.contribution.ts | Updates sidebar link widgets to use item provider counts and deep-link into editor sections. |
| src/vs/sessions/contrib/sessions/browser/aiCustomizationShortcutsWidget.ts | Updates header total count to use item provider (and subscribes to provider change events). |
| src/vs/sessions/contrib/sessions/test/browser/customizationCounts.test.ts | Adds unit tests for getActiveItemProvider() and item-provider-backed totals. |
| src/vs/sessions/contrib/sessions/test/browser/aiCustomizationShortcutsWidget.fixture.ts | Adds ICustomizationHarnessService to the Sessions widget fixture DI setup. |
| src/vs/sessions/AI_CUSTOMIZATIONS.md | Updates documentation to reflect expanded harness acceptance and item-provider-based counting behavior. |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 5
| if (itemProvider) { | ||
| const allItems = await itemProvider.provideChatSessionCustomizations(CancellationToken.None); | ||
| if (requestId !== this._updateCountsRequestId) { | ||
| return; | ||
| } | ||
| const total = allItems?.filter(item => item.type === this._config.promptType).length ?? 0; | ||
| this._renderTotalCount(this._countContainer, total); | ||
| } else { | ||
| const type = this._config.promptType; | ||
| const filter = this._workspaceService.getStorageSourceFilter(type); | ||
| const counts = await getSourceCounts(this._promptsService, type, filter, this._workspaceContextService, this._workspaceService, this._fileService); | ||
| if (requestId !== this._updateCountsRequestId) { | ||
| return; | ||
| } | ||
| const total = getSourceCountsTotal(counts, filter); | ||
| this._renderTotalCount(this._countContainer, total); | ||
| } |
There was a problem hiding this comment.
When itemProvider is present, this path ignores harnesses that also provide local syncable items via a syncProvider (those items appear in the management editor list) and also fails for harnesses whose itemProvider doesn't report prompt types at all (e.g. remote agent harness itemProvider reports only type: 'plugin'). This can make the sidebar prompt counts show 0 even when the list shows items to sync. Consider basing counts on the active harness descriptor (itemProvider + syncProvider) and mirroring the list's item-source logic for blending in local syncable items.
| if (itemProvider) { | |
| const allItems = await itemProvider.provideChatSessionCustomizations(CancellationToken.None); | |
| if (requestId !== this._updateCountsRequestId) { | |
| return; | |
| } | |
| const total = allItems?.filter(item => item.type === this._config.promptType).length ?? 0; | |
| this._renderTotalCount(this._countContainer, total); | |
| } else { | |
| const type = this._config.promptType; | |
| const filter = this._workspaceService.getStorageSourceFilter(type); | |
| const counts = await getSourceCounts(this._promptsService, type, filter, this._workspaceContextService, this._workspaceService, this._fileService); | |
| if (requestId !== this._updateCountsRequestId) { | |
| return; | |
| } | |
| const total = getSourceCountsTotal(counts, filter); | |
| this._renderTotalCount(this._countContainer, total); | |
| } | |
| const type = this._config.promptType; | |
| const filter = this._workspaceService.getStorageSourceFilter(type); | |
| const counts = await getSourceCounts(this._promptsService, type, filter, this._workspaceContextService, this._workspaceService, this._fileService); | |
| if (requestId !== this._updateCountsRequestId) { | |
| return; | |
| } | |
| const localTotal = getSourceCountsTotal(counts, filter); | |
| let providerTotal = 0; | |
| if (itemProvider) { | |
| const allItems = await itemProvider.provideChatSessionCustomizations(CancellationToken.None); | |
| if (requestId !== this._updateCountsRequestId) { | |
| return; | |
| } | |
| providerTotal = allItems?.filter(item => item.type === type).length ?? 0; | |
| } | |
| this._renderTotalCount(this._countContainer, localTotal + providerTotal); |
| const requestId = ++this._updateCountsRequestId; | ||
| const itemProvider = getActiveItemProvider(this._activeSessionService, this._harnessService); | ||
|
|
||
| if (this._config.promptType) { | ||
| const type = this._config.promptType; | ||
| const filter = this._workspaceService.getStorageSourceFilter(type); | ||
| const counts = await getSourceCounts(this._promptsService, type, filter, this._workspaceContextService, this._workspaceService, this._fileService); | ||
| if (requestId !== this._updateCountsRequestId) { | ||
| return; | ||
| if (itemProvider) { | ||
| const allItems = await itemProvider.provideChatSessionCustomizations(CancellationToken.None); |
There was a problem hiding this comment.
provideChatSessionCustomizations() is called inside each individual link view item whenever counts refresh. With 4 prompt-type links, a single refresh cycle can trigger multiple full provider scans (and prompts-service change listeners still trigger these even when provider-backed). Consider caching the provider result per refresh tick (or centralizing provider fetch + per-type counts) to avoid repeated remote/disk work and make updates cheaper.
| const updateHeaderTotalCount = async () => { | ||
| const requestId = ++updateCountRequestId; | ||
| const totalCount = await getCustomizationTotalCount(this.promptsService, this.mcpService, this.workspaceService, this.workspaceContextService, this.agentPluginService); | ||
| const totalCount = await getCustomizationTotalCount(this.promptsService, this.mcpService, this.workspaceService, this.workspaceContextService, this.agentPluginService, getActiveItemProvider(this.sessionsManagementService, this.harnessService)); | ||
| if (requestId !== updateCountRequestId) { |
There was a problem hiding this comment.
The header total now uses itemProvider-based counting, but that can diverge from what the management editor shows for harnesses with a syncProvider (local syncable items are added in the list) and for hook files (the list expands hooks into multiple entries). Consider computing the total from the active harness descriptor using the same item-source logic as the management editor so the header matches the list contents.
| // AHP remote servers register directly via registerExternalHarness. | ||
| if (this._environmentService.isSessionsWindow && chatSessionType !== 'copilotcli') { | ||
| if (this._environmentService.isSessionsWindow && !this._chatSessionService.getContentProviderSchemes().includes(chatSessionType)) { | ||
| return; |
There was a problem hiding this comment.
In Sessions window, this hard drop means an extension that registers a customization provider before its chat session content provider is registered will never get its harness registered (the provider is discarded and not retried when content providers appear). Consider storing these registrations and registering the harness once onDidChangeContentProviderSchemes includes the type, or gating UI visibility later instead of returning early here.
| return; | |
| const contentProviderReady = new DeferredPromise<void>(); | |
| let isDisposed = false; | |
| const waitForContentProviderRegistration = this._chatSessionService.onDidChangeContentProviderSchemes(schemes => { | |
| if (schemes.includes(chatSessionType)) { | |
| waitForContentProviderRegistration.dispose(); | |
| contentProviderReady.complete(); | |
| } | |
| }); | |
| this._customizationProviders.set(handle, { | |
| dispose: () => { | |
| isDisposed = true; | |
| waitForContentProviderRegistration.dispose(); | |
| contentProviderReady.complete(); | |
| } | |
| }); | |
| await contentProviderReady.p; | |
| if (isDisposed) { | |
| return; | |
| } |
| if (itemProvider) { | ||
| const allItems = await itemProvider.provideChatSessionCustomizations(CancellationToken.None); | ||
| promptTotal = allItems?.filter(item => PROMPT_TYPE_SET.has(item.type)).length ?? 0; | ||
| } else { |
There was a problem hiding this comment.
itemProvider counts currently treat hooks as 1 item per hook file, but the customization list expands hook files into individual hook entries (see expandHookFileItems usage in the list item source). This will make totals diverge from what the management editor shows for providers that return hook files. Consider mirroring the same hook-expansion logic here when computing counts from an itemProvider.
rzhao271
left a comment
There was a problem hiding this comment.
LGTM though check CCR comments.
Summary
copilotcligate inmainThreadChatAgents2; accept any session type with a registered content provider schemaICustomizationHarnessServiceinto sidebar count widgets so they use the active harness'sitemProviderwhen available (fixes count mismatches for remote item providers like claude-code)getActiveItemProvider()shared utility; usereader.storepattern instead ofMutableDisposable+ manual rebind for itemProvider subscriptionsselectSectionByIdgetActiveItemProviderandgetCustomizationTotalCountwithitemProvider; addICustomizationHarnessServicemock to widget fixtureTest plan
customizationCounts.test.ts— 41 tests including 8 new)