sessions - resolve by provider#302160
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adjusts chat/agent session resolution so sessions can be refreshed and materialized per provider, aiming to avoid a single slow provider blocking the overall sessions UI.
Changes:
- Filters
ChatSessionsService.refreshChatSessionItemsby the resolved (primary) session type when a provider filter is supplied. - Updates
AgentSessionsModel.doResolveto resolve providers in parallel, refreshing and updating items per provider. - Updates
AgentSessionsModel.updateItemsto fully collect provider results before applying model updates.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/browser/chatSessions/chatSessions.contribution.ts | Provider-filtered refresh now matches against primary/resolved session types. |
| src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsModel.ts | Resolves sessions per provider (intended parallelization) and adjusts how provider results are collected/applied. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
joshspicer
previously approved these changes
Mar 16, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Updates agent sessions resolution to be per-provider (instead of “resolve everything at once”), and threads the provider identifier through resolve lifecycle events to support provider-scoped UI/logic.
Changes:
- Change
IAgentSessionsModel.onWillResolve/onDidResolveto carry the provider string, and update affected tests/stubs. - Refactor
AgentSessionsModelto use per-providerThrottledDelayers and to merge session results across providers. - Adjust
ChatSessionsServicerefresh logic to filter by provider using “primary” type resolution, and update/extend tests to reflect per-provider semantics.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/test/browser/agentSessions/agentSessionsDataSource.test.ts | Updates test stub event typings for provider-carrying resolve events. |
| src/vs/workbench/contrib/chat/test/browser/agentSessions/agentSessionViewModel.test.ts | Updates expectations for per-provider resolution semantics and adds coverage for independent provider refresh behavior. |
| src/vs/workbench/contrib/chat/browser/chatSessions/chatSessions.contribution.ts | Filters refresh by resolved/primary provider type and improves provider naming in logs. |
| src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsModel.ts | Implements per-provider resolution/delayers and provider-carrying resolve events; changes merge behavior across providers. |
| src/vs/sessions/contrib/codeReview/test/browser/codeReviewService.test.ts | Updates test stub event typings for provider-carrying resolve events. |
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsModel.ts:556
- Phase 2 currently preserves all existing sessions from other providers unconditionally. This can keep sessions from providers that are no longer known/available (e.g. cached sessions from an uninstalled/disabled provider) forever, since they will never be replaced by any resolveProvider call. Consider preserving only sessions for providers that are still known (built-in, contributed/available, or currently registered controllers) and dropping unknown provider types during merges to avoid showing stale sessions.
for (const [, session] of this._sessions) {
if (session.providerType !== provider && !sessions.has(session.resource)) {
sessions.set(session.resource, session);
}
}
added 2 commits
March 17, 2026 11:40
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors Agent Sessions resolution to operate per provider, enabling independent throttling and preventing “resolve X clears Y” behavior. This fits into the chat sessions infrastructure by extending IChatSessionsService with provider enumeration and updating the Agent Sessions model/tests accordingly.
Changes:
- Add
getRegisteredChatSessionItemProviders()toIChatSessionsService(and implement it in the real and mock services). - Update
ChatSessionsService.refreshChatSessionItems()to support provider filtering via primary-type resolution. - Refactor
AgentSessionsModel.resolve()to resolve providers independently using per-providerThrottledDelayers and preserve sessions from other providers; update tests to match.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/test/common/mockChatSessionsService.ts | Adds mock implementation of getRegisteredChatSessionItemProviders(). |
| src/vs/workbench/contrib/chat/test/browser/agentSessions/agentSessionsDataSource.test.ts | Updates mocked model event types to Event<string>. |
| src/vs/workbench/contrib/chat/test/browser/agentSessions/agentSessionViewModel.test.ts | Updates expectations for per-provider resolution behavior and adds coverage for independent provider delayers. |
| src/vs/workbench/contrib/chat/common/chatSessionsService.ts | Extends IChatSessionsService with provider enumeration API. |
| src/vs/workbench/contrib/chat/browser/chatSessions/chatSessions.contribution.ts | Implements provider enumeration and filters refresh by resolved provider type. |
| src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsModel.ts | Moves to per-provider resolve/refresh logic with per-provider throttling and provider-carrying resolve events. |
| src/vs/sessions/contrib/codeReview/test/browser/codeReviewService.test.ts | Updates mocked model event types to Event<string>. |
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsModel.ts:568
- The Phase 2 comment has a missing closing parenthesis and still references “updateItems calls”, but the logic now lives in doResolve/resolveProvider. Please fix the typo and update the wording so the comment matches the current control flow.
private toAgentSession(data: IInternalAgentSessionData): IInternalAgentSession {
added 4 commits
March 17, 2026 11:46
sandy081
approved these changes
Mar 17, 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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
fix #298507
improves #300770