Skip to content

Add onDidDispose event for chat sessions input states #312433

Merged
mjbvz merged 3 commits intomicrosoft:mainfrom
mjbvz:dev/mjbvz/complete-gibbon
Apr 24, 2026
Merged

Add onDidDispose event for chat sessions input states #312433
mjbvz merged 3 commits intomicrosoft:mainfrom
mjbvz:dev/mjbvz/complete-gibbon

Conversation

@mjbvz
Copy link
Copy Markdown
Collaborator

@mjbvz mjbvz commented Apr 24, 2026

No description provided.

mjbvz and others added 2 commits April 21, 2026 23:58
This isn't fully hooked up since input states don't have a full lifecycle yet in the service

Co-authored-by: Copilot <copilot@github.com>
Copilot AI review requested due to automatic review settings April 24, 2026 20:48
@mjbvz mjbvz enabled auto-merge April 24, 2026 20:48
TylerLeonhardt
TylerLeonhardt previously approved these changes Apr 24, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a disposal lifecycle hook to chat session input states so extensions can react when an input state is no longer valid.

Changes:

  • Introduce ChatSessionInputState.onDidDispose in the proposed API.
  • Implement onDidDispose in ChatSessionInputStateImpl and dispose input states when sessions/controllers are disposed.
  • Add logic to dispose prior input states associated with a session resource.
Show a summary per file
File Description
src/vscode-dts/vscode.proposed.chatSessionsProvider.d.ts Adds onDidDispose to the proposed ChatSessionInputState API surface.
src/vs/workbench/api/common/extHostChatSessions.ts Implements the new event and wires disposal of input states to session/controller lifecycle paths.

Copilot's findings

  • Files reviewed: 1/2 changed files
  • Comments generated: 1

Comment on lines +963 to +967
// Dispose any previous input states for this session resource
if (sessionResource && controllerData) {
this._disposeInputStatesForResource(controllerData.inputStates, sessionResource);
}

Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disposing prior input states here (inside getInputStateForSession, which is used by the option-group proxy command wrapper) can dispose the active ChatSessionInputStateImpl that the UI/extension is still using to receive onDidChange updates for the session. This call path is triggered when a user runs an option-group command and should not tear down the session’s live input state. Suggest removing the _disposeInputStatesForResource(...) call from this method (or changing the command wrapper to use an untracked/snapshot input state) and only disposing/replacing states in the session lifecycle paths ($provideChatSessionContent / $provideChatSessionInputState / $disposeChatSessionContent).

Suggested change
// Dispose any previous input states for this session resource
if (sessionResource && controllerData) {
this._disposeInputStatesForResource(controllerData.inputStates, sessionResource);
}

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <copilot@github.com>
@mjbvz mjbvz merged commit fe9a973 into microsoft:main Apr 24, 2026
27 checks passed
@vs-code-engineering vs-code-engineering Bot added this to the 1.118.0 milestone Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants