Add notification when closing chat session in progress #278993
Conversation
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @bpaseroMatched files:
|
There was a problem hiding this comment.
Pull request overview
This PR adds a notification to inform users when they close a chat session that has an active request in progress. The notification tells users the chat will continue running in the background and provides a button to open the Agent Sessions view where they can monitor it.
Key Changes
- Created a new notification function that shows an info message with a "Don't Show Again" option and an "Open Agent Sessions" button
- Added logic in
ChatViewPaneto detect when a model with an active request is being disposed - Added logic in
ChatEditorInputto show the notification when the editor input is disposed with an active request - Removed unused
Emitterimport andonWillDisposeevent fromChatEditorModel(no longer needed)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/vs/workbench/contrib/chat/browser/agentSessions/chatCloseNotification.ts |
New file implementing the notification function with proper localization and "Don't Show Again" functionality |
src/vs/workbench/contrib/chat/browser/chatViewPane.ts |
Added check for active requests before disposing model and imports notification function |
src/vs/workbench/contrib/chat/browser/chatEditorInput.ts |
Added dispose override to check for active requests, removed unused Emitter import and onWillDispose event from ChatEditorModel |
| // Check if we're disposing a model with an active request | ||
| if (this.modelRef.value?.object.requestInProgress.get()) { |
There was a problem hiding this comment.
The notification is shown before checking if a new model will be assigned. When updateModel is called with a modelRef parameter, the code disposes the old model and immediately assigns a new one. The notification should only be shown when actually closing/disposing a chat session, not when switching between sessions. Consider checking if !modelRef or moving the check after confirming there's no replacement model.
| // Check if we're disposing a model with an active request | |
| if (this.modelRef.value?.object.requestInProgress.get()) { | |
| // Only show the notification if we're disposing a model with an active request and not replacing it | |
| if (!modelRef && this.modelRef.value?.object.requestInProgress.get()) { |
| { | ||
| label: nls.localize('chat.openAgentSessions', "Open Agent Sessions"), | ||
| run: async () => { | ||
| await viewsService.openView(AGENT_SESSIONS_VIEW_ID, true); |
There was a problem hiding this comment.
do we need to wait here or can fire & forget?
There was a problem hiding this comment.
I am not sure about this: once the agent sessions view is merged with the Chat view, there is no real support for "opening" a view that shows all sessions unless we force the Chat view into the empty state.
This might need another form of UI, like a quick pick to allow to quickly glance over all sessions. Something we do not have yet. Or alternatively, something in the Chat view to bring up a picker (e.g. over the chat session title).
For #277318