From 56be3ffa1bfc45d97f31cba7d027b3a426508c9b Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 20 Mar 2025 07:19:30 +0100 Subject: [PATCH] chat - setup tweaks (#244019) * chat - setup tweaks * clear hacks * sort * fix it * await agent * . * . --- .../contrib/chat/browser/chatSetup.ts | 112 +++++++++++------- .../contrib/chat/common/chatAgents.ts | 17 ++- .../chat/common/chatEntitlementService.ts | 8 -- .../contrib/chat/common/chatServiceImpl.ts | 13 +- 4 files changed, 85 insertions(+), 65 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatSetup.ts b/src/vs/workbench/contrib/chat/browser/chatSetup.ts index 6f6d9cc1d60..fb2d5616d73 100644 --- a/src/vs/workbench/contrib/chat/browser/chatSetup.ts +++ b/src/vs/workbench/contrib/chat/browser/chatSetup.ts @@ -142,8 +142,9 @@ class SetupChatAgentImplementation extends Disposable implements IChatAgentImple disposable.add(chatAgentService.registerAgent(id, { id, - name: `${defaultChat.providerName} Copilot`, + name: 'Copilot', // intentionally not using exact same name as extension to avoid conflict with IChatAgentService.getAgentsByName() isDefault: true, + isCore: true, isToolsAgent, slashCommands: [], disambiguation: [], @@ -158,7 +159,7 @@ class SetupChatAgentImplementation extends Disposable implements IChatAgentImple extensionPublisherId: nullExtensionDescription.publisher })); - const agent = disposable.add(instantiationService.createInstance(SetupChatAgentImplementation, context, controller)); + const agent = disposable.add(instantiationService.createInstance(SetupChatAgentImplementation, context, controller, location)); disposable.add(chatAgentService.registerAgentImplementation(id, agent)); return { agent, disposable }; @@ -173,6 +174,7 @@ class SetupChatAgentImplementation extends Disposable implements IChatAgentImple constructor( private readonly context: ChatEntitlementContext, private readonly controller: Lazy, + private readonly location: ChatAgentLocation, @IInstantiationService private readonly instantiationService: IInstantiationService, @ILogService private readonly logService: ILogService, @IConfigurationService private readonly configurationService: IConfigurationService, @@ -186,20 +188,21 @@ class SetupChatAgentImplementation extends Disposable implements IChatAgentImple const chatService = accessor.get(IChatService); // use accessor for lazy loading const languageModelsService = accessor.get(ILanguageModelsService); // of chat related services const chatWidgetService = accessor.get(IChatWidgetService); + const chatAgentService = accessor.get(IChatAgentService); - return this.doInvoke(request, progress, chatService, languageModelsService, chatWidgetService); + return this.doInvoke(request, progress, chatService, languageModelsService, chatWidgetService, chatAgentService); }); } - private async doInvoke(request: IChatAgentRequest, progress: (part: IChatProgress) => void, chatService: IChatService, languageModelsService: ILanguageModelsService, chatWidgetService: IChatWidgetService): Promise { + private async doInvoke(request: IChatAgentRequest, progress: (part: IChatProgress) => void, chatService: IChatService, languageModelsService: ILanguageModelsService, chatWidgetService: IChatWidgetService, chatAgentService: IChatAgentService): Promise { if (this.context.state.installed && (this.context.state.entitlement === ChatEntitlement.Pro || this.context.state.entitlement === ChatEntitlement.Limited)) { - return this.doInvokeWithoutSetup(request, progress, chatService, languageModelsService, chatWidgetService); + return this.doInvokeWithoutSetup(request, progress, chatService, languageModelsService, chatWidgetService, chatAgentService); } - return this.doInvokeWithSetup(request, progress, chatService, languageModelsService, chatWidgetService); + return this.doInvokeWithSetup(request, progress, chatService, languageModelsService, chatWidgetService, chatAgentService); } - private async doInvokeWithoutSetup(request: IChatAgentRequest, progress: (part: IChatProgress) => void, chatService: IChatService, languageModelsService: ILanguageModelsService, chatWidgetService: IChatWidgetService): Promise { + private async doInvokeWithoutSetup(request: IChatAgentRequest, progress: (part: IChatProgress) => void, chatService: IChatService, languageModelsService: ILanguageModelsService, chatWidgetService: IChatWidgetService, chatAgentService: IChatAgentService): Promise { const requestModel = chatWidgetService.getWidgetBySessionId(request.sessionId)?.viewModel?.model.getRequests().at(-1); if (!requestModel) { this.logService.error('[chat setup] Request model not found, cannot redispatch request.'); @@ -211,33 +214,27 @@ class SetupChatAgentImplementation extends Disposable implements IChatAgentImple content: new MarkdownString(localize('waitingCopilot', "Getting Copilot ready.")), }); - await this.forwardRequestToCopilot(requestModel, progress, chatService, languageModelsService); + await this.forwardRequestToCopilot(requestModel, progress, chatService, languageModelsService, chatAgentService); return {}; } - private async forwardRequestToCopilot(requestModel: IChatRequestModel, progress: (part: IChatProgress) => void, chatService: IChatService, languageModelsService: ILanguageModelsService): Promise { + private async forwardRequestToCopilot(requestModel: IChatRequestModel, progress: (part: IChatProgress) => void, chatService: IChatService, languageModelsService: ILanguageModelsService, chatAgentService: IChatAgentService): Promise { // We need a signal to know when we can resend the request to // Copilot. Waiting for the registration of the agent is not // enough, we also need a language model to be available. - let isCopilotReady = false; - for (const id of languageModelsService.getLanguageModelIds()) { - const model = languageModelsService.lookupLanguageModel(id); - if (model && model.isDefault) { - isCopilotReady = true; - break; - } - } + const whenLanguageModelReady = this.whenLanguageModelReady(languageModelsService); + const whenAgentReady = this.whenAgentReady(chatAgentService); - if (!isCopilotReady) { - const hasDefaultModel = await Promise.race([ + if (whenLanguageModelReady instanceof Promise || whenAgentReady instanceof Promise) { + const ready = await Promise.race([ timeout(10000), - Event.toPromise(Event.filter(languageModelsService.onDidChangeLanguageModels, e => e.added?.some(added => added.metadata.isDefault) ?? false)) + Promise.allSettled([whenLanguageModelReady, whenAgentReady]) ]); - if (!hasDefaultModel) { + if (!ready) { progress({ kind: 'warning', content: new MarkdownString(localize('copilotTookLongWarning', "Copilot took too long to get ready. Please try again later.")) @@ -253,7 +250,30 @@ class SetupChatAgentImplementation extends Disposable implements IChatAgentImple chatService.resendRequest(requestModel); } - private async doInvokeWithSetup(request: IChatAgentRequest, progress: (part: IChatProgress) => void, chatService: IChatService, languageModelsService: ILanguageModelsService, chatWidgetService: IChatWidgetService): Promise { + private whenLanguageModelReady(languageModelsService: ILanguageModelsService): Promise | void { + for (const id of languageModelsService.getLanguageModelIds()) { + const model = languageModelsService.lookupLanguageModel(id); + if (model && model.isDefault) { + return; // we have language models! + } + } + + return Event.toPromise(Event.filter(languageModelsService.onDidChangeLanguageModels, e => e.added?.some(added => added.metadata.isDefault) ?? false)); + } + + private whenAgentReady(chatAgentService: IChatAgentService): Promise | void { + const defaultAgent = chatAgentService.getDefaultAgent(this.location); + if (defaultAgent && !defaultAgent.isCore) { + return; // we have a default agent from an extension! + } + + return Event.toPromise(Event.filter(chatAgentService.onDidChangeAgents, () => { + const defaultAgent = chatAgentService.getDefaultAgent(this.location); + return Boolean(defaultAgent && !defaultAgent.isCore); + })); + } + + private async doInvokeWithSetup(request: IChatAgentRequest, progress: (part: IChatProgress) => void, chatService: IChatService, languageModelsService: ILanguageModelsService, chatWidgetService: IChatWidgetService, chatAgentService: IChatAgentService): Promise { this.telemetryService.publicLog2('workbenchActionExecuted', { id: CHAT_SETUP_ACTION_ID, from: 'chat' }); const requestModel = chatWidgetService.getWidgetBySessionId(request.sessionId)?.viewModel?.model.getRequests().at(-1); @@ -288,7 +308,7 @@ class SetupChatAgentImplementation extends Disposable implements IChatAgentImple if (typeof success === 'boolean') { if (success) { if (requestModel) { - await this.forwardRequestToCopilot(requestModel, progress, chatService, languageModelsService); + await this.forwardRequestToCopilot(requestModel, progress, chatService, languageModelsService, chatAgentService); } } else { progress({ @@ -371,13 +391,13 @@ class ChatSetup { try { switch (setupStrategy) { case ChatSetupStrategy.SetupWithEnterpriseProvider: - success = await this.controller.value.setupWithProvider({ disableCopilotViewReveal: true, useEnterpriseProvider: true }); + success = await this.controller.value.setupWithProvider({ setupFromDialog: true, useEnterpriseProvider: true }); break; case ChatSetupStrategy.SetupWithoutEnterpriseProvider: - success = await this.controller.value.setupWithProvider({ disableCopilotViewReveal: true, useEnterpriseProvider: false }); + success = await this.controller.value.setupWithProvider({ setupFromDialog: true, useEnterpriseProvider: false }); break; case ChatSetupStrategy.DefaultSetup: - success = await this.controller.value.setup({ disableCopilotViewReveal: true }); + success = await this.controller.value.setup({ setupFromDialog: true }); break; } } catch (error) { @@ -810,7 +830,7 @@ class ChatSetupController extends Disposable { this._onDidChange.fire(); } - async setup(options?: { forceSignIn?: boolean; disableCopilotViewReveal?: boolean }): Promise { + async setup(options?: { forceSignIn?: boolean; setupFromDialog?: boolean }): Promise { const watch = new StopWatch(false); const title = localize('setupChatProgress', "Getting Copilot ready..."); const badge = this.activityService.showViewContainerActivity(preferCopilotEditsView(this.viewsService) ? CHAT_EDITING_SIDEBAR_PANEL_ID : CHAT_SIDEBAR_PANEL_ID, { @@ -828,13 +848,12 @@ class ChatSetupController extends Disposable { } } - private async doSetup(options: { forceSignIn?: boolean; disableCopilotViewReveal?: boolean }, watch: StopWatch): Promise { + private async doSetup(options: { forceSignIn?: boolean; setupFromDialog?: boolean }, watch: StopWatch): Promise { this.context.suspend(); // reduces flicker let focusChatInput = false; let success = false; try { - const setupFromDialog = Boolean(this.configurationService.getValue('chat.experimental.setupFromDialog')); const providerId = ChatEntitlementRequests.providerId(this.configurationService); let session: AuthenticationSession | undefined; let entitlement: ChatEntitlement | undefined; @@ -844,7 +863,7 @@ class ChatSetupController extends Disposable { this.setStep(ChatSetupStep.SigningIn); const result = await this.signIn(providerId, options); if (!result.session) { - this.telemetryService.publicLog2('commandCenter.chatInstall', { installResult: 'failedNotSignedIn', installDuration: watch.elapsed(), signUpErrorCode: undefined, setupFromDialog }); + this.telemetryService.publicLog2('commandCenter.chatInstall', { installResult: 'failedNotSignedIn', installDuration: watch.elapsed(), signUpErrorCode: undefined, setupFromDialog: Boolean(options.setupFromDialog) }); return false; } @@ -856,7 +875,7 @@ class ChatSetupController extends Disposable { message: localize('copilotWorkspaceTrust', "Copilot is currently only supported in trusted workspaces.") }); if (!trusted) { - this.telemetryService.publicLog2('commandCenter.chatInstall', { installResult: 'failedNotTrusted', installDuration: watch.elapsed(), signUpErrorCode: undefined, setupFromDialog }); + this.telemetryService.publicLog2('commandCenter.chatInstall', { installResult: 'failedNotTrusted', installDuration: watch.elapsed(), signUpErrorCode: undefined, setupFromDialog: Boolean(options.setupFromDialog) }); return false; } @@ -873,18 +892,18 @@ class ChatSetupController extends Disposable { this.context.resume(); } - if (focusChatInput && !options.disableCopilotViewReveal) { + if (focusChatInput && !options.setupFromDialog) { (await showCopilotView(this.viewsService, this.layoutService))?.focusInput(); } return success; } - private async signIn(providerId: string, options?: { disableCopilotViewReveal?: boolean }): Promise<{ session: AuthenticationSession | undefined; entitlement: ChatEntitlement | undefined }> { + private async signIn(providerId: string, options?: { setupFromDialog?: boolean }): Promise<{ session: AuthenticationSession | undefined; entitlement: ChatEntitlement | undefined }> { let session: AuthenticationSession | undefined; let entitlements; try { - if (!options?.disableCopilotViewReveal) { + if (!options?.setupFromDialog) { showCopilotView(this.viewsService, this.layoutService); } @@ -909,13 +928,12 @@ class ChatSetupController extends Disposable { return { session, entitlement: entitlements?.entitlement }; } - private async install(session: AuthenticationSession | undefined, entitlement: ChatEntitlement, providerId: string, options: { disableCopilotViewReveal?: boolean }, watch: StopWatch): Promise { + private async install(session: AuthenticationSession | undefined, entitlement: ChatEntitlement, providerId: string, options: { setupFromDialog?: boolean }, watch: StopWatch): Promise { const wasInstalled = this.context.state.installed; let signUpResult: boolean | { errorCode: number } | undefined = undefined; - const setupFromDialog = Boolean(this.configurationService.getValue('chat.experimental.setupFromDialog')); try { - if (!options?.disableCopilotViewReveal) { + if (!options?.setupFromDialog) { showCopilotView(this.viewsService, this.layoutService); } @@ -932,7 +950,7 @@ class ChatSetupController extends Disposable { } if (!session) { - this.telemetryService.publicLog2('commandCenter.chatInstall', { installResult: 'failedNoSession', installDuration: watch.elapsed(), signUpErrorCode: undefined, setupFromDialog }); + this.telemetryService.publicLog2('commandCenter.chatInstall', { installResult: 'failedNoSession', installDuration: watch.elapsed(), signUpErrorCode: undefined, setupFromDialog: Boolean(options.setupFromDialog) }); return false; // unexpected } } @@ -940,27 +958,29 @@ class ChatSetupController extends Disposable { signUpResult = await this.requests.signUpLimited(session); if (typeof signUpResult !== 'boolean' /* error */) { - this.telemetryService.publicLog2('commandCenter.chatInstall', { installResult: 'failedSignUp', installDuration: watch.elapsed(), signUpErrorCode: signUpResult.errorCode, setupFromDialog }); + this.telemetryService.publicLog2('commandCenter.chatInstall', { installResult: 'failedSignUp', installDuration: watch.elapsed(), signUpErrorCode: signUpResult.errorCode, setupFromDialog: Boolean(options.setupFromDialog) }); } } await this.doInstall(); } catch (error) { this.logService.error(`[chat setup] install: error ${error}`); - this.telemetryService.publicLog2('commandCenter.chatInstall', { installResult: isCancellationError(error) ? 'cancelled' : 'failedInstall', installDuration: watch.elapsed(), signUpErrorCode: undefined, setupFromDialog }); + this.telemetryService.publicLog2('commandCenter.chatInstall', { installResult: isCancellationError(error) ? 'cancelled' : 'failedInstall', installDuration: watch.elapsed(), signUpErrorCode: undefined, setupFromDialog: Boolean(options.setupFromDialog) }); return false; } - this.telemetryService.publicLog2('commandCenter.chatInstall', { installResult: 'installed', installDuration: watch.elapsed(), signUpErrorCode: undefined, setupFromDialog }); + this.telemetryService.publicLog2('commandCenter.chatInstall', { installResult: 'installed', installDuration: watch.elapsed(), signUpErrorCode: undefined, setupFromDialog: Boolean(options.setupFromDialog) }); if (wasInstalled && signUpResult === true) { refreshTokens(this.commandService); } - await Promise.race([ - timeout(5000), // helps prevent flicker with sign-in welcome view - Event.toPromise(this.chatAgentService.onDidChangeAgents) // https://github.com/microsoft/vscode-copilot/issues/9274 - ]); + if (!options?.setupFromDialog) { + await Promise.race([ + timeout(5000), // helps prevent flicker with sign-in welcome view + Event.toPromise(this.chatAgentService.onDidChangeAgents) // https://github.com/microsoft/vscode-copilot/issues/9274 + ]); + } return true; } @@ -998,7 +1018,7 @@ class ChatSetupController extends Disposable { } } - async setupWithProvider(options: { useEnterpriseProvider: boolean; disableCopilotViewReveal?: boolean }): Promise { + async setupWithProvider(options: { useEnterpriseProvider: boolean; setupFromDialog?: boolean }): Promise { const registry = Registry.as(ConfigurationExtensions.Configuration); registry.registerConfiguration({ 'id': 'copilot.setup', diff --git a/src/vs/workbench/contrib/chat/common/chatAgents.ts b/src/vs/workbench/contrib/chat/common/chatAgents.ts index d7debe39b9f..1b46b4cdbd8 100644 --- a/src/vs/workbench/contrib/chat/common/chatAgents.ts +++ b/src/vs/workbench/contrib/chat/common/chatAgents.ts @@ -54,6 +54,8 @@ export interface IChatAgentData { isToolsAgent?: boolean; /** This agent is not contributed in package.json, but is registered dynamically */ isDynamic?: boolean; + /** This agent is contributed from core and not from an extension */ + isCore?: boolean; metadata: IChatAgentMetadata; slashCommands: IChatAgentCommand[]; locations: ChatAgentLocation[]; @@ -383,13 +385,13 @@ export class ChatAgentService extends Disposable implements IChatAgentService { location = ChatAgentLocation.EditingSession; } - return findLast(this.getActivatedAgents(), a => { + return this._preferExtensionAgent(this.getActivatedAgents().filter(a => { if ((mode === ChatMode.Agent) !== !!a.isToolsAgent) { return false; } return !!a.isDefault && a.locations.includes(location); - }); + })); } public get hasToolsAgent(): boolean { @@ -397,7 +399,15 @@ export class ChatAgentService extends Disposable implements IChatAgentService { } getContributedDefaultAgent(location: ChatAgentLocation): IChatAgentData | undefined { - return this.getAgents().find(a => !!a.isDefault && a.locations.includes(location)); + return this._preferExtensionAgent(this.getAgents().filter(a => !!a.isDefault && a.locations.includes(location))); + } + + private _preferExtensionAgent(agents: T[]): T | undefined { + // We potentially have multiple agents on the same location, + // contributed from core and from extensions. + // This method will prefer the last extensions provided agent + // falling back to the last core agent if no extension agent is found. + return findLast(agents, agent => !agent.isCore) ?? agents.at(-1); } getAgent(id: string, includeDisabled = false): IChatAgentData | undefined { @@ -568,6 +578,7 @@ export class MergedChatAgent implements IChatAgent { get extensionDisplayName(): string { return this.data.extensionDisplayName; } get isDefault(): boolean | undefined { return this.data.isDefault; } get isToolsAgent(): boolean | undefined { return this.data.isToolsAgent; } + get isCore(): boolean | undefined { return this.data.isCore; } get metadata(): IChatAgentMetadata { return this.data.metadata; } get slashCommands(): IChatAgentCommand[] { return this.data.slashCommands; } get locations(): ChatAgentLocation[] { return this.data.locations; } diff --git a/src/vs/workbench/contrib/chat/common/chatEntitlementService.ts b/src/vs/workbench/contrib/chat/common/chatEntitlementService.ts index b68c6a1d03b..66ea4b5a150 100644 --- a/src/vs/workbench/contrib/chat/common/chatEntitlementService.ts +++ b/src/vs/workbench/contrib/chat/common/chatEntitlementService.ts @@ -21,7 +21,6 @@ import { IProductService } from '../../../../platform/product/common/productServ import { asText, IRequestService } from '../../../../platform/request/common/request.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js'; import { ITelemetryService, TelemetryLevel } from '../../../../platform/telemetry/common/telemetry.js'; -import { IWorkspaceContextService } from '../../../../platform/workspace/common/workspace.js'; import { AuthenticationSession, IAuthenticationExtensionsService, IAuthenticationService } from '../../../services/authentication/common/authentication.js'; import { IWorkbenchExtensionEnablementService } from '../../../services/extensionManagement/common/extensionManagement.js'; import { IExtension, IExtensionsWorkbenchService } from '../../extensions/common/extensions.js'; @@ -785,7 +784,6 @@ export class ChatEntitlementContext extends Disposable { constructor( @IContextKeyService contextKeyService: IContextKeyService, @IStorageService private readonly storageService: IStorageService, - @IWorkspaceContextService private readonly workspaceContextService: IWorkspaceContextService, @IWorkbenchExtensionEnablementService private readonly extensionEnablementService: IWorkbenchExtensionEnablementService, @ILogService private readonly logService: ILogService, @IExtensionsWorkbenchService private readonly extensionsWorkbenchService: IExtensionsWorkbenchService, @@ -863,12 +861,6 @@ export class ChatEntitlementContext extends Disposable { private updateContextSync(): void { this.logService.trace(`[chat entitlement context] updateContext(): ${JSON.stringify(this._state)}`); - if (!this._state.hidden && !this._state.installed) { - // this is ugly but fixes flicker from a previous chat install - this.storageService.remove('chat.welcomeMessageContent.panel', StorageScope.APPLICATION); - this.storageService.remove('interactive.sessions', this.workspaceContextService.getWorkspace().folders.length ? StorageScope.WORKSPACE : StorageScope.APPLICATION); - } - this.signedOutContextKey.set(this._state.entitlement === ChatEntitlement.Unknown); this.canSignUpContextKey.set(this._state.entitlement === ChatEntitlement.Available); this.limitedContextKey.set(this._state.entitlement === ChatEntitlement.Limited); diff --git a/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts b/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts index 2bd26b5333a..deccb104d3b 100644 --- a/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts @@ -492,15 +492,12 @@ export class ChatService extends Disposable implements IChatService { throw new ErrorNoTelemetry('No default agent contributed'); } - await this.extensionService.activateByEvent(`onChatParticipant:${defaultAgentData.id}`); + // Activate the default extension provided agent but do not wait + // for it to be ready so that the session can be used immediately + // without having to wait for the agent to be ready. + this.extensionService.activateByEvent(`onChatParticipant:${defaultAgentData.id}`); - const defaultAgent = this.chatAgentService.getActivatedAgents().find(agent => agent.id === defaultAgentData.id); - if (!defaultAgent) { - throw new ErrorNoTelemetry('No default agent registered'); - } - - const sampleQuestions = await defaultAgent.provideSampleQuestions?.(model.initialLocation, token) ?? undefined; - model.initialize(sampleQuestions); + model.initialize(); } catch (err) { this.trace('startSession', `initializeSession failed: ${err}`); model.setInitializationError(err);