Restore activationBlockers on auth #1878
Merged
Conversation
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
Restore activationBlockers on auth
roblourens
changed the title
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an activation blocker mechanism to prevent the extension from completing activation until critical async operations (authentication and embeddings registration) are complete. Previously, chat requests could fail if they arrived before authentication was ready.
Key Changes:
- Added
activationBlockerproperty toIExtensionContributioninterface that delays extension activation - Modified
ConversationFeatureandLanguageModelAccessto wait for authentication before completing activation - Removed redundant request blocking logic from
ChatAgents(formerlyChatParticipants) class
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/extension/common/contributions.ts | Added activationBlocker property to contribution interface and logic to wait for all blockers before activation completes |
| src/extension/extension/vscode/extension.ts | Updated to await waitForActivationBlockers() and removed unused log service import |
| src/extension/conversation/vscode-node/conversationFeature.ts | Implemented activationBlocker that resolves when authentication token is available |
| src/extension/conversation/vscode-node/languageModelAccess.ts | Implemented activationBlocker for chat provider and embeddings registration, improved embeddings re-registration on auth change |
| src/extension/conversation/vscode-node/chatParticipants.ts | Removed redundant request blocking logic (now handled by ConversationFeature), renamed class from ChatParticipants to ChatAgents |
| src/extension/conversation/vscode-node/test/conversationFeature.test.ts | Added test to verify feature behavior when interactive version doesn't match |
| this.activated = false; | ||
| } | ||
|
|
||
| activationBlockerDeferred.complete(); |
There was a problem hiding this comment.
The activationBlockerDeferred.complete() can be called multiple times if the authentication changes multiple times. DeferredPromise.complete() should only be called once. Consider adding a check to ensure it's only called once, or restructure to complete the promise on the first authentication event (whether token exists or not).
| const sw = StopWatch.create(); | ||
| const id = instance.id || 'UNKNOWN'; | ||
| this.allActivationBlockers.push(instance.activationBlocker.finally(() => { | ||
| logService.info(`activationBlocker from '${id}' took for ${Math.round(sw.elapsed())}ms`); |
There was a problem hiding this comment.
Corrected grammar: 'took for' should be 'took'.
Suggested change
| logService.info(`activationBlocker from '${id}' took for ${Math.round(sw.elapsed())}ms`); | |
| logService.info(`activationBlocker from '${id}' took ${Math.round(sw.elapsed())}ms`); |
lszomoru
approved these changes
Nov 8, 2025
DonJayamanne
pushed a commit
that referenced
this pull request
Nov 10, 2025
…#1878) Restore activationBlockers on auth Co-authored-by: Logan Ramos <loganramos@microsoft.com>
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.
Merge candidate PR #1860 to main