Skip to content

Conversation

@DonJayamanne
Copy link
Contributor

Ability to dispose the contributed copilot chat sessions.

For #270852
For https://github.com/microsoft/vscode-internalbacklog/issues/5982

Copilot AI review requested due to automatic review settings October 27, 2025 20:51
Copy link
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

This PR adds the ability to dispose contributed Copilot chat sessions by invoking an optional close method when a session is being disposed.

Key Changes

  • Added a call to close() method (if available) during chat session disposal

/**
* Implement to handle when the Chat Session is closed.
*/
close?(): void;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going with something similar to CustomDocument & PseudoTerminal
Didn't know whether to go with close or dispose
Went with close as that feels the right terminology for chat sessions.

@DonJayamanne DonJayamanne requested a review from mjbvz October 27, 2025 20:52
@DonJayamanne DonJayamanne marked this pull request as ready for review October 27, 2025 20:52
@vs-code-engineering vs-code-engineering bot added this to the October 2025 milestone Oct 27, 2025
Copy link
Collaborator

@mjbvz mjbvz left a comment

Choose a reason for hiding this comment

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

Can you add more info about the use case? A close method isn't a great fit for the current api design but maybe we can come up with something better

@DonJayamanne
Copy link
Contributor Author

@mjbvz Sure.

ose method isn't a great fit for the current api design but maybe we can come up with something better

When creating a chat editor, we call the get content API. which in turn creates corresponding CLI session object.
This has some resources that need to be disposed when the chat session is closed.

Basically each chat editor has a corresponding session object in extension.
& the CLI sdk also has its own resources that need to be cleaned up when the chat editor is closed.

When these editors are closed extension has no way of knowing that they're closed.
I agree its not the best approach (vscode doesn't generally dispose objects returned by extensions), from what I can tell we only do someting similar with CustomDocument and PseudoTerminal that is exposed by extensions.

Optionally an event onDidCloseChatSession would also work.

@mjbvz
Copy link
Collaborator

mjbvz commented Oct 28, 2025

Thanks. Are there resource tied to having a session opened in an editor, or are they tied to the chat session item existing?

If possible, can you link them to the chat session items? That's what we intended for managing the lifecycle of sessions, while the content provider just provides the initial content for a chat session

@DonJayamanne
Copy link
Contributor Author

DonJayamanne commented Oct 28, 2025

Are there resource tied to having a session opened in an editor,

Yes. when we open a chat in the editor, then we need to use a separate API that constructs the chat object via the SDK & this uses some resources & requires proper teardown.

or are they tied to the chat session item existing?

Not sure what you mean.

If possible, can you link them to the chat session items?

Unfortunately this isn't an efficient approach for Copilot CLI
As the session items will be treated as a light weight service (at least for copilot cli) that merely sends a list of sessions.
We're not planning on constructing the entire session object and attaching it to this list, that woudl be expense..
Assume we have 100s of sessions, each session has its own file, loading a session requires reading the entire file, re-constructing the session history, creating a session object with a backing file storage & the like (at least thats inner workings of the sdk that I'm aware of, & the inner workings are changing )..

Also please note when opening a single session in session editor, the contributed get session items gets called around 6-8 times. Similarly when starting a session also results in multiple calls to this method.

while the content provider just provides the initial content for a chat session

From what I understand the content is required only when opening a chat editor.
Thats why I treated this the same as CustomDocument, but lets ignore that suggestion for now.

@mjbvz
Copy link
Collaborator

mjbvz commented Oct 28, 2025

What does the chat object do? Is it something like an event listen that gets updates from a server?

@DonJayamanne
Copy link
Contributor Author

The SDK object has event listeners and writes to the session files.
The SDK holds references to all chat objects opened.

On top of this, we have a delete icon in the chat sessions view that allows one to delete a chat session.
Deleting an already opened chat causes issues as the chat object (SDK) is no longer valid leaving user in a weird state

Ideally we'd like to either disabled this icon of the chat is opened or closed the chat editor.

Perhaps list of opened chat editors and closed event could work here.

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