From b7ebca4bfb127a42315d6b76ff1f37dbed66a41b Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 18 Feb 2026 10:59:44 +1100 Subject: [PATCH] Add authorization checks in Background agents (#3705) * Add authorization checks in Background agents * fix tests * Fixes * Fixes with tests * Updates * Fix tests --- .../copilotcli/node/copilotcliSession.ts | 12 +-- .../node/test/copilotcliSession.spec.ts | 39 +++++---- .../copilotCLIChatSessionsContribution.ts | 37 ++++---- .../copilotCLIChatSessionParticipant.spec.ts | 85 ++++++++++++++++--- extensions/copilot/test/e2e/cli.stest.ts | 82 +++++++++--------- 5 files changed, 156 insertions(+), 99 deletions(-) diff --git a/extensions/copilot/src/extension/agents/copilotcli/node/copilotcliSession.ts b/extensions/copilot/src/extension/agents/copilotcli/node/copilotcliSession.ts index 44716549c0e..54b86f06f53 100644 --- a/extensions/copilot/src/extension/agents/copilotcli/node/copilotcliSession.ts +++ b/extensions/copilot/src/extension/agents/copilotcli/node/copilotcliSession.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import type { Attachment, Session } from '@github/copilot/sdk'; +import type { Attachment, Session, SessionOptions } from '@github/copilot/sdk'; import * as l10n from '@vscode/l10n'; import type * as vscode from 'vscode'; import { ILogService } from '../../../../platform/log/common/logService'; @@ -78,6 +78,7 @@ export interface ICopilotCLISession extends IDisposable { input: CopilotCLISessionInput, attachments: Attachment[], modelId: string | undefined, + authInfo: NonNullable, token: vscode.CancellationToken ): Promise; addUserMessage(content: string): void; @@ -177,12 +178,13 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes input: CopilotCLISessionInput, attachments: Attachment[], modelId: string | undefined, + authInfo: NonNullable, token: vscode.CancellationToken ): Promise { const label = 'prompt' in input ? input.prompt : `/${input.command}`; const promptLabel = label.length > 50 ? label.substring(0, 47) + '...' : label; const capturingToken = new CapturingToken(`Background Agent | ${promptLabel}`, 'worktree', false, true); - return this._requestLogger.captureInvocation(capturingToken, () => this._handleRequestImpl(requestId, input, attachments, modelId, token)); + return this._requestLogger.captureInvocation(capturingToken, () => this._handleRequestImpl(requestId, input, attachments, modelId, authInfo, token)); } private async _handleRequestImpl( @@ -190,6 +192,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes input: CopilotCLISessionInput, attachments: Attachment[], modelId: string | undefined, + authInfo: NonNullable, token: vscode.CancellationToken ): Promise { if (this.isDisposed) { @@ -272,10 +275,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes const logStartTime = Date.now(); try { // Where possible try to avoid an extra call to getSelectedModel by using cached value. - const [currentModel, authInfo] = await Promise.all([ - modelId ? (this._lastUsedModel ?? raceCancellation(this._sdkSession.getSelectedModel(), token)) : undefined, - raceCancellation(this.copilotCLISDK.getAuthInfo(), token) - ]); + const currentModel = await modelId ? (this._lastUsedModel ?? raceCancellation(this._sdkSession.getSelectedModel(), token)) : undefined; if (authInfo) { this._sdkSession.setAuthInfo(authInfo); } diff --git a/extensions/copilot/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts b/extensions/copilot/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts index 05da7cc315e..fbb650200aa 100644 --- a/extensions/copilot/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts +++ b/extensions/copilot/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts @@ -92,18 +92,20 @@ describe('CopilotCLISession', () => { return undefined; } }(); + let authInfo: NonNullable; beforeEach(async () => { const services = disposables.add(createExtensionUnitTestingServices()); const accessor = services.createTestingAccessor(); logger = accessor.get(ILogService); requestLogger = new NullRequestLogger(); + authInfo = { + type: 'token', + token: '', + host: 'https://github.com' + }; sdk = new class extends mock() { override async getAuthInfo(): Promise> { - return { - type: 'token', - token: '', - host: 'https://github.com' - }; + return authInfo; } }; sdkSession = new MockSdkSession(); @@ -138,7 +140,7 @@ describe('CopilotCLISession', () => { // Attach stream first, then invoke with new signature (no stream param) session.attachStream(stream); - await session.handleRequest('', { prompt: 'Hello' }, [], undefined, CancellationToken.None); + await session.handleRequest('', { prompt: 'Hello' }, [], undefined, authInfo, CancellationToken.None); expect(session.status).toBe(ChatSessionStatus.Completed); expect(stream.output.join('\n')).toContain('Echo: Hello'); @@ -149,7 +151,7 @@ describe('CopilotCLISession', () => { const session = await createSession(); const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('', { prompt: 'Hi' }, [], 'modelB', CancellationToken.None); + await session.handleRequest('', { prompt: 'Hi' }, [], 'modelB', authInfo, CancellationToken.None); expect(sdkSession._selectedModel).toBe('modelB'); }); @@ -160,7 +162,7 @@ describe('CopilotCLISession', () => { const session = await createSession(); const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('', { prompt: 'Boom' }, [], undefined, CancellationToken.None); + await session.handleRequest('', { prompt: 'Boom' }, [], undefined, authInfo, CancellationToken.None); expect(session.status).toBe(ChatSessionStatus.Failed); expect(stream.output.join('\n')).toContain('Error: network'); @@ -172,7 +174,7 @@ describe('CopilotCLISession', () => { const listener = disposables.add(session.onDidChangeStatus(s => statuses.push(s))); const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('', { prompt: 'Status OK' }, [], 'modelA', CancellationToken.None); + await session.handleRequest('', { prompt: 'Status OK' }, [], 'modelA', authInfo, CancellationToken.None); listener.dispose?.(); expect(statuses).toEqual([ChatSessionStatus.InProgress, ChatSessionStatus.Completed]); @@ -187,11 +189,8 @@ describe('CopilotCLISession', () => { const listener = disposables.add(session.onDidChangeStatus(s => statuses.push(s))); const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('', { prompt: 'Will Fail' }, [], undefined, CancellationToken.None); + await session.handleRequest('', { prompt: 'Will Fail' }, [], undefined, authInfo, CancellationToken.None); listener.dispose?.(); - - expect(statuses).toEqual([ChatSessionStatus.InProgress, ChatSessionStatus.Failed]); - expect(session.status).toBe(ChatSessionStatus.Failed); expect(stream.output.join('\n')).toContain('Error: boom'); }); @@ -209,7 +208,7 @@ describe('CopilotCLISession', () => { session.attachStream(stream); // Path must be absolute within workspace, should auto-approve - await session.handleRequest('', { prompt: 'Test' }, [], undefined, CancellationToken.None); + await session.handleRequest('', { prompt: 'Test' }, [], undefined, authInfo, CancellationToken.None); expect(result).toEqual({ kind: 'approved' }); }); @@ -228,7 +227,7 @@ describe('CopilotCLISession', () => { session.attachStream(stream); // Path must be absolute within workspace, should auto-approve - await session.handleRequest('', { prompt: 'Test' }, [], undefined, CancellationToken.None); + await session.handleRequest('', { prompt: 'Test' }, [], undefined, authInfo, CancellationToken.None); expect(result).toEqual({ kind: 'approved' }); }); @@ -253,7 +252,7 @@ describe('CopilotCLISession', () => { })); // Path must be absolute within workspace, should auto-approve - await session.handleRequest('', { prompt: 'Test' }, [], undefined, CancellationToken.None); + await session.handleRequest('', { prompt: 'Test' }, [], undefined, authInfo, CancellationToken.None); const file = path.join('/workingDirectory', 'file.ts'); expect(result).toEqual({ kind: 'denied-interactively-by-user' }); expect(askedForPermission).not.toBeUndefined(); @@ -276,7 +275,7 @@ describe('CopilotCLISession', () => { const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('', { prompt: 'Write' }, [], undefined, CancellationToken.None); + await session.handleRequest('', { prompt: 'Write' }, [], undefined, authInfo, CancellationToken.None); expect(result).toEqual({ kind: 'approved' }); }); @@ -294,7 +293,7 @@ describe('CopilotCLISession', () => { }; const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('', { prompt: 'Write' }, [], undefined, CancellationToken.None); + await session.handleRequest('', { prompt: 'Write' }, [], undefined, authInfo, CancellationToken.None); expect(result).toEqual({ kind: 'denied-interactively-by-user' }); }); @@ -312,7 +311,7 @@ describe('CopilotCLISession', () => { }; const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('', { prompt: 'Write' }, [], undefined, CancellationToken.None); + await session.handleRequest('', { prompt: 'Write' }, [], undefined, authInfo, CancellationToken.None); expect(result).toEqual({ kind: 'denied-interactively-by-user' }); }); @@ -334,7 +333,7 @@ describe('CopilotCLISession', () => { }); // Act: start handling request (do not await yet) - const requestPromise = session.handleRequest('', { prompt: 'Edits' }, [], undefined, CancellationToken.None); + const requestPromise = session.handleRequest('', { prompt: 'Edits' }, [], undefined, authInfo, CancellationToken.None); // Wait a tick to ensure event listeners are registered inside handleRequest await new Promise(r => setTimeout(r, 0)); diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts index 070f6f4271b..406ace8dd09 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Attachment, SweCustomAgent } from '@github/copilot/sdk'; +import { Attachment, SessionOptions, SweCustomAgent } from '@github/copilot/sdk'; import * as l10n from '@vscode/l10n'; import * as vscode from 'vscode'; import { ChatExtendedRequestHandler, ChatSessionProviderOptionItem, Uri } from 'vscode'; @@ -28,7 +28,7 @@ import { URI } from '../../../util/vs/base/common/uri'; import { IInstantiationService } from '../../../util/vs/platform/instantiation/common/instantiation'; import { ToolCall } from '../../agents/copilotcli/common/copilotCLITools'; import { IChatDelegationSummaryService } from '../../agents/copilotcli/common/delegationSummaryService'; -import { ICopilotCLIAgents, ICopilotCLIModels } from '../../agents/copilotcli/node/copilotCli'; +import { ICopilotCLIAgents, ICopilotCLIModels, ICopilotCLISDK } from '../../agents/copilotcli/node/copilotCli'; import { CopilotCLIPromptResolver } from '../../agents/copilotcli/node/copilotcliPromptResolver'; import { CopilotCLICommand, copilotCLICommands, ICopilotCLISession } from '../../agents/copilotcli/node/copilotcliSession'; import { ICopilotCLISessionItem, ICopilotCLISessionService } from '../../agents/copilotcli/node/copilotcliSessionService'; @@ -810,6 +810,7 @@ export class CopilotCLIChatSessionParticipant extends Disposable { @IChatDelegationSummaryService private readonly chatDelegationSummaryService: IChatDelegationSummaryService, @IFolderRepositoryManager private readonly folderRepositoryManager: IFolderRepositoryManager, @IConfigurationService private readonly configurationService: IConfigurationService, + @ICopilotCLISDK private readonly copilotCLISDK: ICopilotCLISDK, ) { super(); } @@ -862,7 +863,15 @@ export class CopilotCLIChatSessionParticipant extends Disposable { } } - await this.lockRepoOptionForSession(context, token); + const [authInfo,] = await Promise.all([this.copilotCLISDK.getAuthInfo().catch((ex) => this.logService.error(ex, 'Authorization failed')), this.lockRepoOptionForSession(context, token)]); + if (!authInfo) { + this.logService.error(`Authorization failed`); + throw new Error(vscode.l10n.t('Authorization failed. Please sign into GitHub and try again.')); + } + if ((authInfo.type === 'token' && !authInfo.token) && !this.configurationService.getConfig(ConfigKey.Shared.DebugOverrideProxyUrl)) { + this.logService.error(`Authorization failed`); + throw new Error(vscode.l10n.t('Authorization failed. Please sign into GitHub and try again.')); + } if (!chatSessionContext && SessionIdForCLI.isCLIResource(request.sessionResource)) { /** @@ -891,7 +900,7 @@ export class CopilotCLIChatSessionParticipant extends Disposable { if (!chatSessionContext) { // Delegating from another chat session - return await this.handleDelegationFromAnotherChat(request, context, stream, token); + return await this.handleDelegationFromAnotherChat(request, undefined, request.references, context, stream, authInfo, token); } const { resource } = chatSessionContext.chatSessionItem; @@ -944,18 +953,18 @@ export class CopilotCLIChatSessionParticipant extends Disposable { // This is a request that was created in createCLISessionAndSubmitRequest with attachments already resolved. const { prompt, attachments } = contextForRequest; this.contextForRequest.delete(session.object.sessionId); - await session.object.handleRequest(request.id, { prompt }, attachments, modelId, token); + await session.object.handleRequest(request.id, { prompt }, attachments, modelId, authInfo, token); await this.commitWorktreeChangesIfNeeded(session.object, token); } else if (request.command && !request.prompt && !isUntitled) { const input = (copilotCLICommands as readonly string[]).includes(request.command) ? { command: request.command as CopilotCLICommand } : { prompt: `/${request.command}` }; - await session.object.handleRequest(request.id, input, [], modelId, token); + await session.object.handleRequest(request.id, input, [], modelId, authInfo, token); await this.commitWorktreeChangesIfNeeded(session.object, token); } else { // Construct the full prompt with references to be sent to CLI. const { prompt, attachments } = await this.promptResolver.resolvePrompt(request, undefined, [], session.object.options.isolationEnabled, session.object.options.workingDirectory, token); - await session.object.handleRequest(request.id, { prompt }, attachments, modelId, token); + await session.object.handleRequest(request.id, { prompt }, attachments, modelId, authInfo, token); await this.commitWorktreeChangesIfNeeded(session.object, token); } @@ -1167,15 +1176,6 @@ export class CopilotCLIChatSessionParticipant extends Disposable { } - private async handleDelegationFromAnotherChat( - request: vscode.ChatRequest, - context: vscode.ChatContext, - stream: vscode.ChatResponseStream, - token: vscode.CancellationToken, - ): Promise { - return await this.createCLISessionAndSubmitRequest(request, undefined, request.references, context, stream, token); - } - private async getOrInitializeWorkingDirectory( chatSessionContext: vscode.ChatSessionContext | undefined, stream: vscode.ChatResponseStream, @@ -1235,12 +1235,13 @@ export class CopilotCLIChatSessionParticipant extends Disposable { return { isolationEnabled, workingDirectory, worktreeProperties, cancelled: false, trusted: true }; } - private async createCLISessionAndSubmitRequest( + private async handleDelegationFromAnotherChat( request: vscode.ChatRequest, userPrompt: string | undefined, otherReferences: readonly vscode.ChatPromptReference[] | undefined, context: vscode.ChatContext, stream: vscode.ChatResponseStream, + authInfo: NonNullable, token: vscode.CancellationToken ): Promise { let summary: string | undefined; @@ -1300,7 +1301,7 @@ export class CopilotCLIChatSessionParticipant extends Disposable { // The caller is most likely a chat editor or the like. // Now that we've delegated it to a session, we can get out of here. // Else if the request takes say 10 minutes, the caller would be blocked for that long. - session.object.handleRequest(request.id, { prompt }, attachments, model, token) + session.object.handleRequest(request.id, { prompt }, attachments, model, authInfo, token) .then(() => this.commitWorktreeChangesIfNeeded(session.object, token)) .catch(error => { this.logService.error(`Failed to handle CLI session request: ${error}`); diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts index 5baae06a730..7487c6ded89 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Attachment } from '@github/copilot/sdk'; +import { Attachment, SessionOptions } from '@github/copilot/sdk'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as vscode from 'vscode'; import { Uri } from 'vscode'; @@ -185,9 +185,9 @@ function createChatContext(sessionId: string, isUntitled: boolean): vscode.ChatC } class TestCopilotCLISession extends CopilotCLISession { - public requests: Array<{ input: CopilotCLISessionInput; attachments: Attachment[]; modelId: string | undefined; token: vscode.CancellationToken }> = []; - override handleRequest(requestId: string, input: CopilotCLISessionInput, attachments: Attachment[], modelId: string | undefined, token: vscode.CancellationToken): Promise { - this.requests.push({ input, attachments, modelId, token }); + public requests: Array<{ input: CopilotCLISessionInput; attachments: Attachment[]; modelId: string | undefined; authInfo: NonNullable; token: vscode.CancellationToken }> = []; + override handleRequest(requestId: string, input: CopilotCLISessionInput, attachments: Attachment[], modelId: string | undefined, authInfo: NonNullable, token: vscode.CancellationToken): Promise { + this.requests.push({ input, attachments, modelId, authInfo, token }); return Promise.resolve(); } } @@ -226,15 +226,18 @@ describe('CopilotCLIChatSessionParticipant.handleRequest', () => { let folderRepositoryManager: CopilotCLIFolderRepositoryManager; let cliSessionServiceForFolderManager: FakeCopilotCLISessionService; let contentProvider: CopilotCLIChatSessionContentProvider; + let sdk: ICopilotCLISDK; const cliSessions: TestCopilotCLISession[] = []; beforeEach(async () => { cliSessions.length = 0; - // Reset executeCommand to throw by default — existing delegation tests rely on this - // falling into the catch block of createCLISessionAndSubmitRequest. - mockExecuteCommand.mockImplementation(() => { throw new Error('vscode.commands.executeCommand not available in test'); }); - const sdk = { - getPackage: vi.fn(async () => ({ internal: { LocalSessionManager: MockCliSdkSessionManager } })) + // By default, simulate the command not being available so that + // handleDelegationFromAnotherChat falls into its catch block and + // calls handleRequest directly. The workaround tests override this. + mockExecuteCommand.mockRejectedValue(new Error('command not available')); + sdk = { + getPackage: vi.fn(async () => ({ internal: { LocalSessionManager: MockCliSdkSessionManager } })), + getAuthInfo: vi.fn(async () => ({ type: 'token' as const, token: 'valid-token', host: 'https://github.com' })), } as unknown as ICopilotCLISDK; const services = disposables.add(createExtensionUnitTestingServices()); const accessor = services.createTestingAccessor(); @@ -335,7 +338,8 @@ describe('CopilotCLIChatSessionParticipant.handleRequest', () => { new PromptsServiceImpl(new NullWorkspaceService()), delegationService, folderRepositoryManager, - mockConfigurationService + mockConfigurationService, + sdk ); }); @@ -349,13 +353,14 @@ describe('CopilotCLIChatSessionParticipant.handleRequest', () => { const context = createChatContext('temp-new', true); const stream = new MockChatResponseStream(); const token = disposables.add(new CancellationTokenSource()).token; + const authInfo = await sdk.getAuthInfo(); expect(cliSessions.length).toBe(0); await participant.createHandler()(request, context, stream, token); expect(cliSessions.length).toBe(1); expect(cliSessions[0].requests.length).toBe(1); - expect(cliSessions[0].requests[0]).toEqual({ input: { prompt: 'Say hi' }, attachments: [], modelId: 'base', token }); + expect(cliSessions[0].requests[0]).toEqual({ input: { prompt: 'Say hi' }, attachments: [], modelId: 'base', authInfo, token }); }); it('uses worktree workingDirectory when isolation is enabled for a new untitled session', async () => { @@ -414,7 +419,7 @@ describe('CopilotCLIChatSessionParticipant.handleRequest', () => { const sessionId = 'existing-123'; const sdkSession = new MockCliSdkSession(sessionId, new Date()); manager.sessions.set(sessionId, sdkSession); - + const authInfo = await sdk.getAuthInfo(); const request = new TestChatRequest('Continue'); const context = createChatContext(sessionId, false); const stream = new MockChatResponseStream(); @@ -427,7 +432,7 @@ describe('CopilotCLIChatSessionParticipant.handleRequest', () => { expect(cliSessions.length).toBe(1); expect(cliSessions[0].sessionId).toBe(sessionId); expect(cliSessions[0].requests.length).toBe(1); - expect(cliSessions[0].requests[0]).toEqual({ input: { prompt: 'Continue' }, attachments: [], modelId: 'base', token }); + expect(cliSessions[0].requests[0]).toEqual({ input: { prompt: 'Continue' }, attachments: [], modelId: 'base', authInfo, token }); expect(itemProvider.swap).not.toHaveBeenCalled(); }); @@ -795,6 +800,60 @@ describe('CopilotCLIChatSessionParticipant.handleRequest', () => { expect(cliSessions[0].requests[1].input).toEqual({ prompt: 'Second request' }); }); + describe('Authorization check', () => { + it('throws when auth token is empty and no proxy URL configured', async () => { + (sdk.getAuthInfo as ReturnType).mockResolvedValue({ type: 'token', token: '', host: 'https://github.com' }); + + const request = new TestChatRequest('Say hi'); + const context = createChatContext('temp-new', true); + const stream = new MockChatResponseStream(); + const token = disposables.add(new CancellationTokenSource()).token; + + await expect(participant.createHandler()(request, context, stream, token)).rejects.toThrow('Authorization failed'); + expect(cliSessions.length).toBe(0); + }); + + it('proceeds normally when auth token is valid', async () => { + (sdk.getAuthInfo as ReturnType).mockResolvedValue({ type: 'token', token: 'valid-token', host: 'https://github.com' }); + + const request = new TestChatRequest('Say hi'); + const context = createChatContext('temp-new', true); + const stream = new MockChatResponseStream(); + const token = disposables.add(new CancellationTokenSource()).token; + + await participant.createHandler()(request, context, stream, token); + + expect(cliSessions.length).toBe(1); + expect(cliSessions[0].requests.length).toBe(1); + }); + + it('proceeds when auth type is not token even if token is empty', async () => { + (sdk.getAuthInfo as ReturnType).mockResolvedValue({ type: 'oauth', token: '', host: 'https://github.com' }); + + const request = new TestChatRequest('Say hi'); + const context = createChatContext('temp-new', true); + const stream = new MockChatResponseStream(); + const token = disposables.add(new CancellationTokenSource()).token; + + await participant.createHandler()(request, context, stream, token); + + expect(cliSessions.length).toBe(1); + expect(cliSessions[0].requests.length).toBe(1); + }); + + it('throws when getAuthInfo rejects', async () => { + (sdk.getAuthInfo as ReturnType).mockRejectedValue(new Error('network error')); + + const request = new TestChatRequest('Say hi'); + const context = createChatContext('temp-new', true); + const stream = new MockChatResponseStream(); + const token = disposables.add(new CancellationTokenSource()).token; + + await expect(participant.createHandler()(request, context, stream, token)).rejects.toThrow('Authorization failed'); + expect(cliSessions.length).toBe(0); + }); + }); + describe('Repository option locking behavior', () => { it('locks repository option on request start for untitled sessions', async () => { // Setup folder repository manager to return valid folder data diff --git a/extensions/copilot/test/e2e/cli.stest.ts b/extensions/copilot/test/e2e/cli.stest.ts index af380f59ca3..d83cd211b87 100644 --- a/extensions/copilot/test/e2e/cli.stest.ts +++ b/extensions/copilot/test/e2e/cli.stest.ts @@ -73,7 +73,7 @@ function restoreEnvVariablesAfterTests() { } } -function registerChatServices(testingServiceCollection: TestingServiceCollection) { +async function registerChatServices(testingServiceCollection: TestingServiceCollection) { const ITestSessionOptionsProvider = createServiceIdentifier('ITestSessionOptionsProvider'); class TestSessionOptionsProvider { declare _serviceBrand: undefined; @@ -325,7 +325,8 @@ function registerChatServices(testingServiceCollection: TestingServiceCollection await populateWorkspaceFiles(workingDirectory.fsPath); await sdk.getPackage(); - } + }, + authInfo: await sdk.getAuthInfo() }; } @@ -333,7 +334,7 @@ const vscCopilotRoot = path.join(__dirname, '..'); // NOTE: Ensure all files/folders/workingDirectories are under test/scenarios/test-cli for path replacements to work correctly. const sourcePath = path.join(__dirname, '..', 'test', 'scenarios', 'test-cli'); let tmpDirCounter = 0; -function testRunner(cb: (services: { sessionService: ICopilotCLISessionService; promptResolver: CopilotCLIPromptResolver; init: (workingDirectory: URI) => Promise }, scenariosPath: string, toolInvocations: ChatToolInvocationPart[], stream: MockChatResponseStream, disposables: DisposableStore) => Promise) { +function testRunner(cb: (services: { sessionService: ICopilotCLISessionService; promptResolver: CopilotCLIPromptResolver; init: (workingDirectory: URI) => Promise; authInfo: NonNullable }, scenariosPath: string, toolInvocations: ChatToolInvocationPart[], stream: MockChatResponseStream, disposables: DisposableStore) => Promise) { return async (testingServiceCollection: TestingServiceCollection) => { trackEnvVariablesBeforeTests(); const disposables = new DisposableStore(); @@ -345,13 +346,12 @@ function testRunner(cb: (services: { sessionService: ICopilotCLISessionService; await fs.cp(sourcePath, scenariosPath, { recursive: true, force: true, errorOnExist: false }); const toolInvocations: ChatToolInvocationPart[] = []; try { - const services = registerChatServices(testingServiceCollection); + const services = await registerChatServices(testingServiceCollection); const stream = new MockChatResponseStream((part) => { if (part instanceof ChatToolInvocationPart) { toolInvocations.push(part); } }); - await cb(services, await fs.realpath(scenariosPath), toolInvocations, stream, disposables); } finally { await fs.rm(scenariosPath, { recursive: true }).catch(() => { /* Ignore */ }); @@ -404,14 +404,14 @@ function assertToolInvocationMessageContains(invocation: ChatToolInvocationPart, ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { stest({ description: 'can start a session' }, - testRunner(async ({ sessionService, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); const session = await sessionService.createSession({ workingDirectory }, CancellationToken.None); disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt: 'What is 1+8?' }, [], undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt: 'What is 1+8?' }, [], undefined, authInfo, CancellationToken.None); // Verify we have a response of 9. assert.strictEqual(session.object.status, ChatSessionStatus.Completed); @@ -419,16 +419,14 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { assertStreamContains(stream, '9'); // Can send a subsequent request. - await session.object.handleRequest('', { prompt: 'What is 11+25?' }, [], undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt: 'What is 11+25?' }, [], undefined, authInfo, CancellationToken.None); // Verify we have a response of 36. - assert.strictEqual(session.object.status, ChatSessionStatus.Completed); - assertNoErrorsInStream(stream); assertStreamContains(stream, '36'); }) ); stest({ description: 'can resume a session' }, - testRunner(async ({ sessionService, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); @@ -438,7 +436,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { const session = await sessionService.createSession({ workingDirectory }, CancellationToken.None); sessionId = session.object.sessionId; - await session.object.handleRequest('', { prompt: 'What is 1+8?' }, [], undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt: 'What is 1+8?' }, [], undefined, authInfo, CancellationToken.None); session.dispose(); } @@ -458,7 +456,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt: 'What was my previous question?' }, [], undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt: 'What was my previous question?' }, [], undefined, authInfo, CancellationToken.None); // Verify we have a response of 9. assert.strictEqual(session.object.status, ChatSessionStatus.Completed); @@ -468,7 +466,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { }) ); stest({ description: 'can read file without permission' }, - testRunner(async ({ sessionService, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); const file = URI.joinPath(workingDirectory, 'sample.js'); @@ -477,7 +475,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt }, [], undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, [], undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -485,7 +483,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { }) ); stest({ description: 'request permission when reading file outside workspace' }, - testRunner(async ({ sessionService, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); @@ -508,7 +506,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { } })); - await session.object.handleRequest('', { prompt }, [], undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, [], undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -517,7 +515,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { }) ); stest({ description: 'can read attachment without permission' }, - testRunner(async ({ sessionService, promptResolver, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, promptResolver, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); const file = URI.joinPath(workingDirectory, 'sample.js').fsPath; @@ -531,7 +529,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt }, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, attachments, undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -539,7 +537,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { }) ); stest({ description: 'can edit file' }, - testRunner(async ({ sessionService, promptResolver, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, promptResolver, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); const file = URI.joinPath(workingDirectory, 'sample.js').fsPath; @@ -553,7 +551,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt }, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, attachments, undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -567,7 +565,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { [], promptResolver )); - await session.object.handleRequest('', { prompt }, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, attachments, undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -578,7 +576,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { }) ); stest({ description: 'explain selection' }, - testRunner(async ({ sessionService, promptResolver, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, promptResolver, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); const file = URI.joinPath(workingDirectory, 'utils.js').fsPath; @@ -593,14 +591,14 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt }, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, attachments, undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertStreamContains(stream, 'throw'); }) ); stest({ description: 'can create a file' }, - testRunner(async ({ sessionService, promptResolver, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, promptResolver, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); const { prompt, attachments } = await resolvePromptWithFileReferences( @@ -613,7 +611,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt }, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, attachments, undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -621,7 +619,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { }) ); stest({ description: 'can list files in directory' }, - testRunner(async ({ sessionService, promptResolver, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, promptResolver, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); const { prompt, attachments } = await resolvePromptWithFileReferences( @@ -634,7 +632,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt }, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, attachments, undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -645,7 +643,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { }) ); stest({ description: 'can fix problems' }, - testRunner(async ({ sessionService, promptResolver, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, promptResolver, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); const file = URI.joinPath(workingDirectory, 'stringUtils.js').fsPath; @@ -661,7 +659,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt }, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, attachments, undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -671,7 +669,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { ); stest({ description: 'can fix multiple problems in multiple files' }, - testRunner(async ({ sessionService, promptResolver, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, promptResolver, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); const tsFile = URI.joinPath(workingDirectory, 'stringUtils.js').fsPath; @@ -689,7 +687,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt }, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, attachments, undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); const tsContents = await fs.readFile(tsFile, 'utf-8'); @@ -700,7 +698,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { ); stest({ description: 'can run terminal commands' }, - testRunner(async ({ sessionService, promptResolver, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, promptResolver, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); @@ -723,7 +721,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { } })); - await session.object.handleRequest('', { prompt }, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, attachments, undefined, authInfo, CancellationToken.None); assertNoErrorsInStream(stream); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); @@ -732,7 +730,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { ); stest({ description: 'glob tool returns files with correct toolSpecificData' }, - testRunner(async ({ sessionService, promptResolver, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, promptResolver, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); const { prompt, attachments } = await resolvePromptWithFileReferences( @@ -745,7 +743,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt }, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, attachments, undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -760,7 +758,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { ); stest({ description: 'glob tool with no matches has empty toolSpecificData' }, - testRunner(async ({ sessionService, promptResolver, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, promptResolver, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); const { prompt, attachments } = await resolvePromptWithFileReferences( @@ -773,7 +771,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt }, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, attachments, undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -789,7 +787,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { ); stest({ description: 'grep tool returns files with correct toolSpecificData' }, - testRunner(async ({ sessionService, promptResolver, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, promptResolver, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); const { prompt, attachments } = await resolvePromptWithFileReferences( @@ -802,7 +800,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt }, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, attachments, undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -818,7 +816,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { ); stest({ description: 'grep tool with no matches has empty toolSpecificData' }, - testRunner(async ({ sessionService, promptResolver, init }, scenariosPath, toolInvocations, stream, disposables) => { + testRunner(async ({ sessionService, promptResolver, init, authInfo }, scenariosPath, toolInvocations, stream, disposables) => { const workingDirectory = URI.file(path.join(scenariosPath, 'wkspc1')); await init(workingDirectory); const { prompt, attachments } = await resolvePromptWithFileReferences( @@ -831,7 +829,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('', { prompt }, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', { prompt }, attachments, undefined, authInfo, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream);