sessions: clean up ISessionsProvider chat API#317875
Merged
Merged
Conversation
Simplify the provider contract and remove provider access to the chat widget service so widget opening is owned exclusively by the management service. Provider API - Remove `sendAndCreateChat` and `addChat` from `ISessionsProvider`. - Add `createNewChat(sessionId)` returning the committed `IChat` so the management service can open the widget on the real backend resource before sending. - `sendRequest(sessionId, chatResource, options)` now handles both the first-send (new session) and subsequent-send paths; providers dispatch internally based on `_currentNewSession`. - For multi-chat sessions, `createNewChat` on an existing session gates on `_isMultiChatEnabled()` and creates a fresh chat in the group. Session model - `ISession.mainChat` changes from `IChat` to `IObservable<IChat>` so providers can swap the chat when an untitled new session commits to a real backend resource (e.g. Claude). - `ICopilotChatSession` owns its own `ISettableObservable<IChat>` `mainChat` field; the provider no longer maintains a parallel map. Management service - `sendNewChatRequest` (renamed from `sendAndCreateChat`) calls `createNewChat`, opens the widget on the returned chat resource, then calls `sendRequest`. - `openNewChatInSession` is now async and opens the widget after `createNewChat` returns. Copilot provider - Drops the `IChatWidgetService` injection; the provider never opens widgets directly. - Drops `userSelectedTools` from the local send path (no widget available there). Spec - `SESSIONS.md`: document that `ISessionsProvider` must not have optional methods and that every addition to `ISession` or `ISessionsProvider` must be consumed by the agents-window core workbench (outside `contrib/providers/`). - `COPILOT_CHAT_SESSIONS_PROVIDER.md`: update the send-flow section to describe the new `createNewChat` + `sendRequest` two-step contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
roblourens
previously approved these changes
May 21, 2026
Contributor
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @lszomoruMatched files:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request refactors the Agents Window sessions/provider chat contract so providers own backend chat lifecycle while SessionsManagementService becomes the sole owner of opening chat widgets. It updates the session model to allow providers to swap the “main chat” when an untitled session commits to a real backend resource.
Changes:
- Replaces
ISessionsProvider.sendAndCreateChat/addChatwith a two-stepcreateNewChat+sendRequestflow. - Changes
ISession.mainChatfromIChattoIObservable<IChat>to support main-chat replacement on commit. - Updates management-service send/open flows plus broad test and documentation updates to match the new contract.
Show a summary per file
| File | Description |
|---|---|
| src/vs/sessions/SESSIONS.md | Updates conceptual model and documents new provider/management responsibilities and interface design rules. |
| src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts | Updates test provider stubs for createNewChat and mainChat observable. |
| src/vs/sessions/services/sessions/test/browser/sessionNavigation.test.ts | Updates management service mock to new method names/async signatures and mainChat observable. |
| src/vs/sessions/services/sessions/common/sessionsProvider.ts | Replaces provider API with createNewChat(sessionId, prompt?) and updated sendRequest contract. |
| src/vs/sessions/services/sessions/common/sessionsManagement.ts | Renames/adjusts management service APIs (sendNewChatRequest, async openNewChatInSession). |
| src/vs/sessions/services/sessions/common/session.ts | Updates ISession.mainChat to IObservable<IChat> and documents replacement semantics. |
| src/vs/sessions/services/sessions/browser/sessionsManagementService.ts | Moves new-chat send flow to createNewChat → open widget → sendRequest; makes openNewChatInSession async. |
| src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts | Updates session stubs to mainChat observable. |
| src/vs/sessions/contrib/terminal/test/browser/agentHostSessionTaskRunner.test.ts | Updates session stubs to mainChat observable. |
| src/vs/sessions/contrib/sessions/test/browser/sessionsListModelService.test.ts | Adjusts tests for mainChat observable usage. |
| src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts | Updates session test helper to provide observable mainChat. |
| src/vs/sessions/contrib/sessions/browser/views/sessionsViewActions.ts | Updates rename/add-chat actions for observable mainChat and async openNewChatInSession. |
| src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts | Updates provider tests to the new createNewChat + sendRequest flow. |
| src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts | Updates Copilot provider tests for new chat creation and observable mainChat. |
| src/vs/sessions/contrib/providers/copilotChatSessions/COPILOT_CHAT_SESSIONS_PROVIDER.md | Rewrites send-flow documentation to match createNewChat + sendRequest contract. |
| src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts | Refactors Copilot provider to support createNewChat, observable main-chat replacement, and management-owned widget opening. |
| src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts | Updates agent-host provider tests to the new chat API. |
| src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostSkillButtons.test.ts | Updates session stubs to mainChat observable. |
| src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts | Implements createNewChat and shifts widget opening out of the provider; updates ISession.mainChat. |
| src/vs/sessions/contrib/layout/test/browser/sessionLayoutController.test.ts | Updates session stubs to mainChat observable. |
| src/vs/sessions/contrib/github/test/browser/githubContribution.test.ts | Updates test session implementation for observable mainChat. |
| src/vs/sessions/contrib/chat/test/browser/worktreeCreatedTaskDispatcher.test.ts | Updates session stubs to mainChat observable. |
| src/vs/sessions/contrib/chat/test/browser/workbenchSessionTaskRunner.test.ts | Updates session stubs to mainChat observable. |
| src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts | Updates provider mock to include createNewChat and new sendRequest signature. |
| src/vs/sessions/contrib/chat/test/browser/sessionsTaskService.test.ts | Updates session stubs to mainChat observable. |
| src/vs/sessions/contrib/chat/browser/runScriptAction.ts | Updates widget lookup and send flow to use observable mainChat and sendNewChatRequest. |
| src/vs/sessions/contrib/chat/browser/newChatViewPane.ts | Updates send path to call sendNewChatRequest. |
| src/vs/sessions/browser/parts/chatCompositeBar.ts | Updates reactive tab rebuild logic to read mainChat via observable. |
Copilot's findings
- Files reviewed: 28/28 changed files
- Comments generated: 5
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- sessionsManagementService.sendNewChatRequest: open the chat widget with ChatViewPaneTarget so it lands in the chat view (matches the other openSession call sites in this service). - copilotChatSessionsProvider.createNewChat & _createNewSubsequentChat: dispose the IDisposable returned by _createChatSession so the model reference acquired via acquireOrLoadSession is not leaked. - copilotChatSessionsProvider._sendExistingChat: capture the disposable from _updateChatSessionState and release it in a finally so the model reference does not leak on every send. - copilotChatSessionsProvider._sendFirstChat: when chatService.sendRequest returns kind 'rejected', clean up the temp session (cache, group cache, current-new-session, fire removed event, dispose) before throwing so the UI does not keep showing a stuck InProgress session. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dmitrivMS
approved these changes
May 21, 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.
Cleans up
ISessionsProviderand the management-service send flow so providers own only backend chat lifecycle, and the management service is the sole owner of widget opening.Provider API
sendAndCreateChatandaddChatfromISessionsProvider.createNewChat(sessionId)that returns the committedIChat, so the management service can open the widget on the real backend resource before sending.sendRequest(sessionId, chatResource, options)now handles both first-send (new session) and subsequent-send paths; providers dispatch internally based on_currentNewSession.createNewChaton an existing session gates on_isMultiChatEnabled()and creates a fresh chat in the group.Session model
ISession.mainChatchanges fromIChattoIObservable<IChat>so providers can swap the chat when an untitled new session commits to a real backend resource (e.g. Claude).ICopilotChatSessionowns its ownISettableObservable<IChat>mainChatfield; the provider no longer keeps a parallel_mainChatBySessionIdmap.Management service
sendNewChatRequest(renamed fromsendAndCreateChat) callscreateNewChat, opens the widget on the returnedchat.resource, then callssendRequest.openNewChatInSessionis now async and opens the widget aftercreateNewChatreturns.Copilot provider
IChatWidgetServiceinjection — the provider never opens widgets directly.userSelectedToolsfrom the local send path (no widget available there).Spec updates
SESSIONS.md: documents two new interface-design rules:ISessionsProvidermust have no optional methods.ISessionorISessionsProvidermust be consumed in the agents-window core workbench (anything outsidecontrib/providers/).COPILOT_CHAT_SESSIONS_PROVIDER.md: send-flow section rewritten to describe the newcreateNewChat+sendRequesttwo-step contract.Validation
npm run compile-check-ts-native— clean (sessions code; only pre-existing unrelatedproxyResolver.tserrors remain).npm run valid-layers-check— clean.scripts/test.sh --glob "**/sessions/**/*.test.js"— 753 passing.