Improvements to the fix for microsoft/vscode#275641 (#1843)

fixes microsoft/vscode#275641

This includes confirmation handling in chat and also prevents some random modals from poping up when they shouldn't.

I should clean up the auth upgrade service next week to handle metadata better... but this works for the release to keep the change small.

Author: TylerLeonhardt

src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts

@@ -16,6 +16,8 @@ import { Disposable, toDisposable } from '../../../util/vs/base/common/lifecycle
 import { body_suffix, CONTINUE_TRUNCATION, extractTitle, formatBodyPlaceholder, getAuthorDisplayName, getRepoId, JOBS_API_VERSION, RemoteAgentResult, SessionIdForPr, toOpenPullRequestWebviewUri, truncatePrompt } from '../vscode/copilotCodingAgentUtils';
 import { ChatSessionContentBuilder } from './copilotCloudSessionContentBuilder';
 import { IPullRequestFileChangesService } from './pullRequestFileChangesService';
+import { IAuthenticationChatUpgradeService } from '../../../platform/authentication/common/authenticationUpgrade';
+import { IAuthenticationService } from '../../../platform/authentication/common/authentication';
 
 export type ConfirmationResult = { step: string; accepted: boolean; metadata?: ConfirmationMetadata };
 
@@ -80,11 +82,17 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C
 		@ILogService private readonly logService: ILogService,
 		@IGitExtensionService private readonly _gitExtensionService: IGitExtensionService,
 		@IPullRequestFileChangesService private readonly _prFileChangesService: IPullRequestFileChangesService,
+		@IAuthenticationService private readonly _authenticationService: IAuthenticationService,
+		@IAuthenticationChatUpgradeService private readonly _authenticationUpgradeService: IAuthenticationChatUpgradeService,
 	) {
 		super();
 		const interval = setInterval(async () => {
 			const repoId = await getRepoId(this._gitService);
 			if (repoId) {
+				// TODO: handle no auth token case more gracefully
+				if (!this._authenticationService.permissiveGitHubSession) {
+					return;
+				}
 				const sessions = await this._octoKitService.getAllOpenSessions(`${repoId.org}/${repoId.repo}`);
 				if (this.cachedSessionsSize !== sessions.length) {
 					this.refresh();
@@ -104,6 +112,10 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C
 			return { optionGroups: [] };
 		}
 
+		// TODO: handle no auth token case more gracefully
+		if (!this._authenticationService.permissiveGitHubSession) {
+			return { optionGroups: [] };
+		}
 		try {
 			const customAgents = await this._octoKitService.getCustomAgents(repoId.org, repoId.repo);
 			const agentItems: vscode.ChatSessionProviderOptionItem[] = [
@@ -153,6 +165,10 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C
 				return [];
 			}
 
+			// TODO: handle no auth token case more gracefully
+			if (!this._authenticationService.permissiveGitHubSession) {
+				return [];
+			}
 			const sessions = await this._octoKitService.getAllOpenSessions(`${repoId.org}/${repoId.repo}`);
 			this.cachedSessionsSize = sessions.length;
 
@@ -430,9 +446,9 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C
 	}
 
 
-	private async handleConfirmationData(request: vscode.ChatRequest, stream: vscode.ChatResponseStream, token: vscode.CancellationToken) {
+	private async handleConfirmationData(request: vscode.ChatRequest, stream: vscode.ChatResponseStream, context: vscode.ChatContext, token: vscode.CancellationToken) {
 		const results: ConfirmationResult[] = [];
-		results.push(...(request.acceptedConfirmationData?.map(data => ({ step: data.step, accepted: true, metadata: data?.metadata })) ?? []));
+		results.push(...(request.acceptedConfirmationData?.filter(data => !data?.authPermissionPrompted).map(data => ({ step: data.step, accepted: true, metadata: data?.metadata })) ?? []));
 		results.push(...((request.rejectedConfirmationData ?? []).filter(data => !results.some(r => r.step === data.step)).map(data => ({ step: data.step, accepted: false, metadata: data?.metadata }))));
 		for (const data of results) {
 			switch (data.step) {
@@ -576,7 +592,26 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C
 
 	private async chatParticipantImpl(request: vscode.ChatRequest, context: vscode.ChatContext, stream: vscode.ChatResponseStream, token: vscode.CancellationToken) {
 		if (request.acceptedConfirmationData || request.rejectedConfirmationData) {
-			return await this.handleConfirmationData(request, stream, token);
+			const findConfirmRequest = request.acceptedConfirmationData?.find(ref => ref?.authPermissionPrompted);
+			if (findConfirmRequest) {
+				const result = await this._authenticationUpgradeService.handleConfirmationRequestWithContext(stream, request, context.history);
+				request = result.request;
+				context = result.context ?? context;
+			} else {
+				return await this.handleConfirmationData(request, stream, context, token);
+			}
+		}
+
+		const accessToken = this._authenticationService.permissiveGitHubSession;
+		if (!accessToken) {
+			// Otherwise, show the permissive session upgrade prompt because it's required
+			this._authenticationUpgradeService.showPermissiveSessionUpgradeInChat(
+				stream,
+				request,
+				vscode.l10n.t('GitHub Copilot Cloud Agent requires access to your repositories on GitHub for handling requests.'),
+				context
+			);
+			return {};
 		}
 
 		/* __GDPR__

src/platform/authentication/common/authenticationUpgrade.ts

@@ -33,7 +33,7 @@ export interface IAuthenticationChatUpgradeService {
 	 * @param data - The initial chat request data for context.
 	 * @param detail - Optional detail overriding
 	 */
-	showPermissiveSessionUpgradeInChat(stream: ChatResponseStream, data: ChatRequest, detail?: string): void;
+	showPermissiveSessionUpgradeInChat(stream: ChatResponseStream, data: ChatRequest, detail?: string, context?: ChatContext): void;
 
 	/**
 	 * Manages the user's input regarding the confirmation request for a session upgrade.
@@ -42,4 +42,13 @@ export interface IAuthenticationChatUpgradeService {
 	 * was passed in if we don't detect that the confirmation was presented.
 	 */
 	handleConfirmationRequest(stream: ChatResponseStream, request: ChatRequest, history: ChatContext['history']): Promise<ChatRequest>;
+
+	/**
+	 * TODO: Fold this into one API with the above
+	 * Manages the user's input regarding the confirmation request for a session upgrade.
+	 * @param request - The chat request object containing details necessary for the upgrade flow.
+	 * @returns Promise<ChatRequest> - The ChatRequest that was originally presented the confirmation, or the request that
+	 * was passed in if we don't detect that the confirmation was presented.
+	 */
+	handleConfirmationRequestWithContext(stream: ChatResponseStream, request: ChatRequest, history: ChatContext['history']): Promise<{ request: ChatRequest; context?: ChatContext }>;
 }

src/platform/authentication/common/authenticationUpgradeService.ts

@@ -109,14 +109,16 @@ export class AuthenticationChatUpgradeService extends Disposable implements IAut
 	showPermissiveSessionUpgradeInChat(
 		stream: ChatResponseStream,
 		data: ChatRequest,
-		detail?: string
+		detail?: string,
+		context?: ChatContext
 	): void {
 		this.logService.trace('Requesting permissive session upgrade in chat');
 		this.hasRequestedPermissiveSessionUpgrade = true;
 		stream.confirmation(
 			this._permissionRequest,
 			detail || l10n.t('To get more relevant Chat results, we need permission to read the contents of your repository on GitHub.'),
-			{ authPermissionPrompted: true, ...data },
+			// TODO: Change this shape to include request via a dedicated field
+			{ authPermissionPrompted: true, ...data, context },
 			[
 				this._permissionRequestGrant,
 				this._permissionRequestNotNow,
@@ -125,6 +127,17 @@ export class AuthenticationChatUpgradeService extends Disposable implements IAut
 		);
 	}
 
+	async handleConfirmationRequestWithContext(stream: ChatResponseStream, request: ChatRequest, history: ChatContext['history']): Promise<{ request: ChatRequest; context?: ChatContext }> {
+		const findConfirmationRequested: (ChatRequest & { context?: ChatContext }) | undefined = request.acceptedConfirmationData?.find(ref => ref?.authPermissionPrompted);
+		if (!findConfirmationRequested) {
+			return { request, context: undefined };
+		}
+		const context = findConfirmationRequested.context;
+
+		const updatedRequest = await this.handleConfirmationRequest(stream, request, history);
+		return { request: updatedRequest, context };
+	}
+
 	async handleConfirmationRequest(stream: ChatResponseStream, request: ChatRequest, history: ChatContext['history']): Promise<ChatRequest> {
 		const findConfirmationRequested: ChatRequest | undefined = request.acceptedConfirmationData?.find(ref => ref?.authPermissionPrompted);
 		if (!findConfirmationRequested) {