Merged
Conversation
- Now that edit sessions are sync, we don't need to wait for them (just guard edits) as in microsoft/vscode-copilot#16060 - I noticed `widget.onDidClear()` listeners were actually async and we do need to wait for them before we can safely make new input. Revised this to a delegate pattern. - Wait for a view model when sending chat. Also, improved Event.toPromise so it can safely be reused -- it now cleans up its own disposable from the disposable store or array after it settles. Resolves #247484
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @jriekenMatched files:
@bpaseroMatched files:
@lszomoruMatched files:
|
connor4312
commented
Nov 17, 2025
|
|
||
| if (!opts.isPartialQuery) { | ||
| await chatWidget.waitForReady(); | ||
| if (!chatWidget.viewModel) { |
Member
Author
There was a problem hiding this comment.
This is the case where we do need to wait still, because we might change agents I believe
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR removes the waitForReady() method from chat widgets now that edit sessions are synchronous. The key changes include:
- Replaced the event-based
onDidClear()pattern with a delegate-based approach for better async handling - Added view model waiting logic directly in
acceptInput()instead of relying on externalwaitForReady()calls - Enhanced
Event.toPromiseto properly clean up disposables after settling, preventing resource leaks
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
chatWidget.ts |
Removed waitForReady() and onDidClear event; made clear() async and added view model waiting in acceptInput() |
chat.ts |
Updated IChatWidget interface to remove waitForReady() and add clear delegate option |
event.ts |
Enhanced Event.toPromise to clean up disposables from stores/arrays after resolution or cancellation |
event.test.ts |
Added comprehensive tests for improved Event.toPromise cleanup behavior |
chatViewPane.ts |
Migrated from onDidClear listener to clear delegate |
chatQuick.ts |
Migrated from onDidClear listener to clear delegate |
chatEditor.ts |
Migrated from onDidClear listener to clear delegate |
inlineChatZoneWidget.ts |
Added clearDelegate parameter to support delegate pattern |
inlineChatController.ts |
Refactored to pass clear logic as delegate instead of event listener |
chatNewActions.ts |
Replaced clear() + waitForReady() with single await clear() call |
chatMoveActions.ts |
Replaced clear() + waitForReady() with single await clear() call |
chatExecuteActions.ts |
Replaced clear() + waitForReady() with single await clear() call |
chatActions.ts |
Replaced waitForReady() with direct view model waiting; made clear operations properly await |
chat.contribution.ts |
Removed unused waitForReady() call and IViewsService import |
scmHistoryChatContext.ts |
Removed waitForReady() call before adding attachment |
chatSetup.ts |
Made clear() call async |
chatEditingSession.ts |
Added chatEditingSessionIsReady calls to guard edits as mentioned in PR description |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
21 tasks
roblourens
approved these changes
Nov 17, 2025
pwang347
pushed a commit
that referenced
this pull request
Nov 18, 2025
* chat: remove `waitForReady` - Now that edit sessions are sync, we don't need to wait for them (just guard edits) as in microsoft/vscode-copilot#16060 - I noticed `widget.onDidClear()` listeners were actually async and we do need to wait for them before we can safely make new input. Revised this to a delegate pattern. - Wait for a view model when sending chat. Also, improved Event.toPromise so it can safely be reused -- it now cleans up its own disposable from the disposable store or array after it settles. Resolves #247484 * Update src/vs/base/common/event.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
guard edits) as in https://github.com/microsoft/vscode-copilot/issues/16060
widget.onDidClear()listeners were actually async and wedo need to wait for them before we can safely make new input. Revised
this to a delegate pattern.
so it can safely be reused -- it now cleans up its own disposable from
the disposable store or array after it settles.
Resolves #247484
Refs #277318