From 90a94dc238ed3d88b95f4c5482abb330ffa9481f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 19 Nov 2025 19:10:48 +0000 Subject: [PATCH 1/3] Initial plan From b4b3ac9851e01966a3abf4009e7c9ccf73f47cd1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 19 Nov 2025 19:21:13 +0000 Subject: [PATCH 2/3] Implement single confirmation flow refactoring Co-authored-by: joshspicer <23246594+joshspicer@users.noreply.github.com> --- .../copilotCloudSessionsProvider.ts | 187 +++++++++++++++--- 1 file changed, 155 insertions(+), 32 deletions(-) diff --git a/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts b/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts index e814720ec4..de2e34ad85 100644 --- a/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts +++ b/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts @@ -26,13 +26,16 @@ import { ChatSessionContentBuilder } from './copilotCloudSessionContentBuilder'; import { IPullRequestFileChangesService } from './pullRequestFileChangesService'; export type ConfirmationResult = { step: string; accepted: boolean; metadata?: ConfirmationMetadata }; -export const UncommittedChangesStep = 'uncommitted-changes'; +export const UncommittedChangesStep = 'uncommitted-changes'; // Deprecated: kept for backward compatibility +export const DelegateConfirmationStep = 'delegate'; interface ConfirmationMetadata { prompt: string; references?: readonly vscode.ChatPromptReference[]; chatContext: vscode.ChatContext; autoPushAndCommit?: boolean; + hasUncommittedChanges?: boolean; + needsAuthUpgrade?: boolean; } export interface PullRequestInfo { @@ -640,8 +643,49 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C 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) { + case DelegateConfirmationStep: + { + if (!data.accepted || !data.metadata) { + stream.markdown(vscode.l10n.t('Cloud agent request cancelled.')); + return {}; + } + + // Extract button text from request.prompt which has format: "ButtonText: \"ConfirmationTitle\"" + const promptLower = request.prompt.toLowerCase(); + const needsAuthAction = promptLower.includes('allow'); + const needsCommitAction = promptLower.includes('commit') || promptLower.includes('push'); + + // Handle auth upgrade if button indicates it + if (needsAuthAction) { + try { + const accessToken = await this._authenticationService.getPermissiveGitHubSession({ createIfNone: true }); + if (!accessToken) { + stream.markdown(vscode.l10n.t('Cloud agent authentication requirements not met. Please allow access to proceed.')); + return {}; + } + } catch (error) { + this.logService.error(`Failed to get permissive GitHub session: ${error}`); + stream.markdown(vscode.l10n.t('Failed to authenticate. Please try again.')); + return {}; + } + } + + // Handle commit and push if button indicates it + if (needsCommitAction) { + data.metadata.autoPushAndCommit = true; + } + + // Route to appropriate creation method + if (data.metadata.chatContext?.chatSessionContext?.isUntitled) { + await this.doUntitledCreation({ ...data.metadata, chatContext: context }, stream, token); + } else { + await this.createDelegatedChatSession({ ...data.metadata, chatContext: context }, stream, token); + } + break; + } case 'create': { + // Backward compatibility: keep old flow for existing sessions if (!data.accepted || !data.metadata) { stream.markdown(vscode.l10n.t('Cloud agent request cancelled.')); return {}; @@ -655,6 +699,7 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C } case UncommittedChangesStep: { + // Backward compatibility: keep old flow for existing sessions if (!data.accepted || !data.metadata) { stream.markdown(vscode.l10n.t('Cloud agent request cancelled due to uncommitted changes.')); return {}; @@ -714,6 +759,101 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C }; } + /** + * Prepares and shows a single unified confirmation that checks all prerequisites upfront + * (git state, auth state) and presents dynamic buttons based on the current state. + * @returns 'true' if confirmation was pushed (caller should stop processing), 'false' otherwise + */ + async prepareAndShowDelegateConfirmation(metadata: ConfirmationMetadata, stream: vscode.ChatResponseStream, token: vscode.CancellationToken): Promise { + let hasUncommittedChanges = false; + let needsAuthUpgrade = false; + + // Check git state for uncommitted changes + try { + const repoId = await getRepoId(this._gitService); + if (repoId) { + const currentRepository = this._gitService.activeRepository.get(); + if (currentRepository) { + const git = this._gitExtensionService.getExtensionApi(); + const repo = git?.getRepository(currentRepository?.rootUri); + if (repo) { + hasUncommittedChanges = repo.state.workingTreeChanges.length > 0 || repo.state.indexChanges.length > 0; + } + } + } + } catch (error) { + this.logService.warn(`Failed to check git state: ${error}`); + } + + // Check auth state - determine if permissive session upgrade is needed + try { + needsAuthUpgrade = await this._authenticationUpgradeService.shouldRequestPermissiveSessionUpgrade(); + } catch (error) { + this.logService.warn(`Failed to check auth state: ${error}`); + } + + // If auto-commit is enabled and we have changes, skip git-related confirmation + if (hasUncommittedChanges && this.autoCommitAndPushEnabled) { + hasUncommittedChanges = false; + metadata.autoPushAndCommit = true; + } + + // Build confirmation message and buttons based on state + let confirmationTitle: string; + let confirmationMessage: string; + let buttons: string[]; + + if (!hasUncommittedChanges && !needsAuthUpgrade) { + // Simple case: no special requirements + confirmationTitle = vscode.l10n.t('Delegate to cloud agent'); + confirmationMessage = this.DELEGATE_MODAL_DETAILS; + buttons = [vscode.l10n.t('Delegate'), vscode.l10n.t('Cancel')]; + } else if (hasUncommittedChanges && !needsAuthUpgrade) { + // Only uncommitted changes + confirmationTitle = vscode.l10n.t('Delegate to cloud agent'); + confirmationMessage = vscode.l10n.t('You have uncommitted changes in your workspace. Consider committing them if you would like to include them in the cloud agent\'s work.'); + buttons = [ + vscode.l10n.t('Commit and Delegate'), + vscode.l10n.t('Delegate without committing'), + vscode.l10n.t('Cancel') + ]; + } else if (!hasUncommittedChanges && needsAuthUpgrade) { + // Only auth upgrade needed + confirmationTitle = vscode.l10n.t('Delegate to cloud agent'); + confirmationMessage = vscode.l10n.t('GitHub Copilot Cloud Agent requires access to your repositories on GitHub for handling requests.'); + buttons = [ + vscode.l10n.t('Allow and Delegate'), + vscode.l10n.t('Cancel') + ]; + } else { + // Both uncommitted changes and auth upgrade needed + confirmationTitle = vscode.l10n.t('Delegate to cloud agent'); + confirmationMessage = vscode.l10n.t('You have uncommitted changes and GitHub Copilot Cloud Agent requires access to your repositories.'); + buttons = [ + vscode.l10n.t('Commit and Allow'), + vscode.l10n.t('Allow without committing'), + vscode.l10n.t('Cancel') + ]; + } + + // Store state in metadata for later processing + metadata.hasUncommittedChanges = hasUncommittedChanges; + metadata.needsAuthUpgrade = needsAuthUpgrade; + + // Show unified confirmation + stream.confirmation( + confirmationTitle, + confirmationMessage, + { + step: DelegateConfirmationStep, + metadata: metadata satisfies ConfirmationMetadata, + }, + buttons + ); + + return true; // Confirmation was pushed + } + /** * Checks for uncommitted changes in the current repository and prompts the user for confirmation if any are found. * @returns 'true' if handling was performed. This will push a chat confirmation and initiate a new chat request (handled in handleConfirmationData()) @@ -830,6 +970,7 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C } private async chatParticipantImpl(request: vscode.ChatRequest, context: vscode.ChatContext, stream: vscode.ChatResponseStream, token: vscode.CancellationToken) { + // Handle confirmation responses (both old and new flow) if (request.acceptedConfirmationData || request.rejectedConfirmationData) { const findAuthConfirmRequest = request.acceptedConfirmationData?.find(ref => ref?.authPermissionPrompted); const findAuthRejectRequest = request.rejectedConfirmationData?.find(ref => ref?.authPermissionPrompted); @@ -838,17 +979,20 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C return {}; } if (findAuthConfirmRequest) { + // Backward compatibility: handle old auth confirmations const result = await this._authenticationUpgradeService.handleConfirmationRequestWithContext(stream, request, context.history); request = result.request; context = result.context ?? context; } else { + // Handle both new and old confirmation flows return await this.handleConfirmationData(request, stream, context, token); } } + // Check if user has permissive token (only required for new sessions, not follow-ups) const accessToken = this._authenticationService.permissiveGitHubSession; - if (!accessToken) { - // Otherwise, show the permissive session upgrade prompt because it's required + if (!accessToken && !context.chatSessionContext) { + // For new sessions without token, show auth upgrade in chat (old flow for backward compatibility) this._authenticationUpgradeService.showPermissiveSessionUpgradeInChat( stream, request, @@ -873,26 +1017,12 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C if (context.chatSessionContext?.isUntitled) { /* Generate new cloud agent session from an 'untitled' session */ - - const handledUncommittedChanges = await this.tryHandleUncommittedChanges({ + await this.prepareAndShowDelegateConfirmation({ prompt: request.prompt, references: request.references, chatContext: context }, stream, token); - - // If uncommitted changes were detected and a confirmation was shown, - // don't proceed with creation yet - wait for user response - if (handledUncommittedChanges) { - return {}; - } - - const { prompt, references } = request; - await this.doUntitledCreation({ - prompt, - references, - chatContext: context, - }, stream, token); - + return {}; } else if (context.chatSessionContext) { /* Follow up to an existing cloud agent session */ try { @@ -956,19 +1086,12 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C } } else { /* @copilot invoked from a 'normal' chat or 'cloud button' */ - stream.confirmation( - vscode.l10n.t('Delegate to cloud agent'), - this.DELEGATE_MODAL_DETAILS, - { - step: 'create', - metadata: { - prompt: request.prompt, - references: request.references, - chatContext: context, - } satisfies ConfirmationMetadata - }, - ['Delegate', 'Cancel'] - ); + await this.prepareAndShowDelegateConfirmation({ + prompt: request.prompt, + references: request.references, + chatContext: context + }, stream, token); + return {}; } } From a2a52ff0d455459f5a6d38d1b84a902f2040d321 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 19 Nov 2025 19:26:44 +0000 Subject: [PATCH 3/3] Add unit tests for single confirmation flow logic Co-authored-by: joshspicer <23246594+joshspicer@users.noreply.github.com> --- .../test/copilotCloudSessionsProvider.spec.ts | 144 ++++++++++++++++++ src/extension/test/node/testHelpers.ts | 6 + 2 files changed, 150 insertions(+) create mode 100644 src/extension/chatSessions/vscode-node/test/copilotCloudSessionsProvider.spec.ts diff --git a/src/extension/chatSessions/vscode-node/test/copilotCloudSessionsProvider.spec.ts b/src/extension/chatSessions/vscode-node/test/copilotCloudSessionsProvider.spec.ts new file mode 100644 index 0000000000..087c4c4c9b --- /dev/null +++ b/src/extension/chatSessions/vscode-node/test/copilotCloudSessionsProvider.spec.ts @@ -0,0 +1,144 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { describe, expect, it, vi } from 'vitest'; +import * as vscode from 'vscode'; +import { DelegateConfirmationStep } from '../copilotCloudSessionsProvider'; + +describe('CopilotCloudSessionsProvider - Single Confirmation Flow', () => { + + describe('DelegateConfirmationStep constant', () => { + it('should be defined as "delegate"', () => { + expect(DelegateConfirmationStep).toBe('delegate'); + }); + }); + + describe('Button text detection logic', () => { + it('should detect "Allow" in button text (case insensitive)', () => { + const prompts = [ + 'Allow and Delegate: "Delegate to cloud agent"', + 'allow: "test"', + 'ALLOW AND COMMIT: "title"' + ]; + + prompts.forEach(prompt => { + expect(prompt.toLowerCase().includes('allow')).toBe(true); + }); + }); + + it('should detect "Commit" or "Push" in button text (case insensitive)', () => { + const prompts = [ + 'Commit and Delegate: "Delegate to cloud agent"', + 'commit: "test"', + 'Push changes: "title"', + 'COMMIT AND ALLOW: "test"' + ]; + + prompts.forEach(prompt => { + const promptLower = prompt.toLowerCase(); + expect(promptLower.includes('commit') || promptLower.includes('push')).toBe(true); + }); + }); + + it('should not detect allow/commit in regular prompts', () => { + const prompts = [ + 'Delegate: "Delegate to cloud agent"', + 'Cancel: "test"' + ]; + + prompts.forEach(prompt => { + const promptLower = prompt.toLowerCase(); + expect(promptLower.includes('allow')).toBe(false); + expect(promptLower.includes('commit')).toBe(false); + }); + }); + }); + + describe('Confirmation metadata structure', () => { + it('should support required metadata fields', () => { + const metadata = { + prompt: 'test prompt', + references: [] as readonly vscode.ChatPromptReference[], + chatContext: {} as vscode.ChatContext, + autoPushAndCommit: false, + hasUncommittedChanges: true, + needsAuthUpgrade: false + }; + + expect(metadata.prompt).toBe('test prompt'); + expect(metadata.hasUncommittedChanges).toBe(true); + expect(metadata.needsAuthUpgrade).toBe(false); + expect(metadata.autoPushAndCommit).toBe(false); + }); + + it('should allow optional fields to be undefined', () => { + const metadata = { + prompt: 'test prompt', + chatContext: {} as vscode.ChatContext + }; + + expect(metadata.prompt).toBe('test prompt'); + expect(metadata.hasOwnProperty('hasUncommittedChanges')).toBe(false); + expect(metadata.hasOwnProperty('needsAuthUpgrade')).toBe(false); + }); + }); + + describe('Button combinations', () => { + it('should generate correct buttons for clean repo with no auth', () => { + const hasUncommittedChanges = false; + const needsAuthUpgrade = false; + + if (!hasUncommittedChanges && !needsAuthUpgrade) { + const buttons = ['Delegate', 'Cancel']; + expect(buttons).toEqual(['Delegate', 'Cancel']); + } + }); + + it('should generate correct buttons for uncommitted changes only', () => { + const hasUncommittedChanges = true; + const needsAuthUpgrade = false; + + if (hasUncommittedChanges && !needsAuthUpgrade) { + const buttons = [ + 'Commit and Delegate', + 'Delegate without committing', + 'Cancel' + ]; + expect(buttons).toHaveLength(3); + expect(buttons[0]).toContain('Commit'); + } + }); + + it('should generate correct buttons for auth upgrade only', () => { + const hasUncommittedChanges = false; + const needsAuthUpgrade = true; + + if (!hasUncommittedChanges && needsAuthUpgrade) { + const buttons = [ + 'Allow and Delegate', + 'Cancel' + ]; + expect(buttons).toHaveLength(2); + expect(buttons[0]).toContain('Allow'); + } + }); + + it('should generate correct buttons for both conditions', () => { + const hasUncommittedChanges = true; + const needsAuthUpgrade = true; + + if (hasUncommittedChanges && needsAuthUpgrade) { + const buttons = [ + 'Commit and Allow', + 'Allow without committing', + 'Cancel' + ]; + expect(buttons).toHaveLength(3); + expect(buttons[0]).toContain('Commit'); + expect(buttons[0]).toContain('Allow'); + } + }); + }); +}); diff --git a/src/extension/test/node/testHelpers.ts b/src/extension/test/node/testHelpers.ts index 3d2296df77..ecbbfe202d 100644 --- a/src/extension/test/node/testHelpers.ts +++ b/src/extension/test/node/testHelpers.ts @@ -24,6 +24,8 @@ export class TestChatRequest implements ChatRequest { public tools = new Map(); public id = generateUuid(); public sessionId = generateUuid(); + public acceptedConfirmationData?: any[]; + public rejectedConfirmationData?: any[]; constructor( public prompt: string @@ -40,6 +42,7 @@ export class MockChatResponseStream extends ChatResponseStreamImpl { public output: string[] = []; public uris: string[] = []; + public confirmations: Array<{ title: string; message: string; data: any; buttons?: string[] }> = []; constructor() { super(() => { }, () => { }); @@ -50,4 +53,7 @@ export class MockChatResponseStream extends ChatResponseStreamImpl { override codeblockUri(uri: URI): void { this.uris.push(uri.toString()); } + override confirmation(title: string, message: string, data: any, buttons?: string[]): void { + this.confirmations.push({ title, message, data, buttons }); + } } \ No newline at end of file