diff --git a/extensions/copilot/src/extension/agents/copilotcli/node/copilotCli.ts b/extensions/copilot/src/extension/agents/copilotcli/node/copilotCli.ts index d4ed5225db4..50ee8a9195e 100644 --- a/extensions/copilot/src/extension/agents/copilotcli/node/copilotCli.ts +++ b/extensions/copilot/src/extension/agents/copilotcli/node/copilotCli.ts @@ -98,13 +98,17 @@ export class CopilotCLISDK implements ICopilotCLISDK { public async getPackage(): Promise { try { // Ensure the node-pty shim exists before importing the SDK (required for CLI sessions) - await ensureNodePtyShim(this.extensionContext.extensionPath, this.envService.appRoot, this.logService); + await this.ensureNodePtyShim(); return await import('@github/copilot/sdk'); } catch (error) { this.logService.error(`[CopilotCLISDK] Failed to load @github/copilot/sdk: ${error}`); throw error; } } + + protected async ensureNodePtyShim(): Promise { + await ensureNodePtyShim(this.extensionContext.extensionPath, this.envService.appRoot, this.logService); + } } export interface ICopilotCLISessionOptionsService { diff --git a/extensions/copilot/src/extension/agents/copilotcli/node/copilotcliSessionService.ts b/extensions/copilot/src/extension/agents/copilotcli/node/copilotcliSessionService.ts index d8d089d2d78..893336e0a6a 100644 --- a/extensions/copilot/src/extension/agents/copilotcli/node/copilotcliSessionService.ts +++ b/extensions/copilot/src/extension/agents/copilotcli/node/copilotcliSessionService.ts @@ -84,7 +84,7 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS }); } - private monitorSessionFiles() { + protected monitorSessionFiles() { try { const sessionDir = joinPath(this.nativeEnv.userHome, '.copilot', 'session-state'); const watcher = this._register(this.fileSystem.createFileSystemWatcher(new RelativePattern(sessionDir, '*.jsonl'))); diff --git a/extensions/copilot/test/e2e/cli.stest.ts b/extensions/copilot/test/e2e/cli.stest.ts new file mode 100644 index 00000000000..938e06e93f1 --- /dev/null +++ b/extensions/copilot/test/e2e/cli.stest.ts @@ -0,0 +1,204 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import * as path from 'path'; +import type { ChatPromptReference } from 'vscode'; +import { CopilotCLIModels, CopilotCLISDK, CopilotCLISessionOptionsService, ICopilotCLIModels, ICopilotCLISDK, ICopilotCLISessionOptionsService } from '../../src/extension/agents/copilotcli/node/copilotCli'; +import { CopilotCLIPromptResolver } from '../../src/extension/agents/copilotcli/node/copilotcliPromptResolver'; +import { ICopilotCLISession } from '../../src/extension/agents/copilotcli/node/copilotcliSession'; +import { CopilotCLISessionService, ICopilotCLISessionService } from '../../src/extension/agents/copilotcli/node/copilotcliSessionService'; +import { PermissionRequest } from '../../src/extension/agents/copilotcli/node/permissionHelpers'; +import { ILanguageModelServer, LanguageModelServer } from '../../src/extension/agents/node/langModelServer'; +import { MockChatResponseStream, TestChatRequest } from '../../src/extension/test/node/testHelpers'; +import { TestingServiceCollection } from '../../src/platform/test/node/services'; +import { disposableTimeout, IntervalTimer } from '../../src/util/vs/base/common/async'; +import { CancellationToken } from '../../src/util/vs/base/common/cancellation'; +import { DisposableStore } from '../../src/util/vs/base/common/lifecycle'; +import { SyncDescriptor } from '../../src/util/vs/platform/instantiation/common/descriptors'; +import { IInstantiationService } from '../../src/util/vs/platform/instantiation/common/instantiation'; +import { ChatRequest, ChatSessionStatus, Uri } from '../../src/vscodeTypes'; +import { ssuite, stest } from '../base/stest'; + +function registerChatServices(testingServiceCollection: TestingServiceCollection) { + class TestCopilotCLISDK extends CopilotCLISDK { + override async ensureNodePtyShim(): Promise { + // Override to do nothing in tests + } + } + class TestCopilotCLISessionService extends CopilotCLISessionService { + override async monitorSessionFiles() { + // Override to do nothing in tests + } + } + + testingServiceCollection.define(ICopilotCLISessionService, new SyncDescriptor(TestCopilotCLISessionService)); + testingServiceCollection.define(ICopilotCLISessionOptionsService, new SyncDescriptor(CopilotCLISessionOptionsService)); + testingServiceCollection.define(ICopilotCLIModels, new SyncDescriptor(CopilotCLIModels)); + testingServiceCollection.define(ICopilotCLISDK, new SyncDescriptor(TestCopilotCLISDK)); + testingServiceCollection.define(ILanguageModelServer, new SyncDescriptor(LanguageModelServer)); + + const accessor = testingServiceCollection.createTestingAccessor(); + const copilotCLISessionService = accessor.get(ICopilotCLISessionService); + const instaService = accessor.get(IInstantiationService); + const promptResolver = instaService.createInstance(CopilotCLIPromptResolver); + + return { sessionService: copilotCLISessionService, promptResolver }; +} + +function testRunner(cb: (services: { sessionService: ICopilotCLISessionService; promptResolver: CopilotCLIPromptResolver }, stream: MockChatResponseStream, disposables: DisposableStore) => Promise) { + return async (testingServiceCollection: TestingServiceCollection) => { + const disposables = new DisposableStore(); + try { + const services = registerChatServices(testingServiceCollection); + const stream = new MockChatResponseStream(); + + await cb(services, stream, disposables); + } finally { + disposables.dispose(); + } + }; +} + +const scenariosPath = path.join(__dirname, '..', 'test/scenarios/test-cli'); + +ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { + stest({ description: 'can start a session' }, + testRunner(async ({ sessionService }, stream, disposables) => { + const session = await sessionService.createSession('What is 1+8?', undefined, undefined, CancellationToken.None); + disposables.add(session); + disposables.add(session.attachStream(stream)); + + await session.handleRequest('What is 1+8?', [], undefined, CancellationToken.None); + + // Verify we have a response of 9. + assert.strictEqual(session.status, ChatSessionStatus.Completed); + assert.ok(stream.output.join('\n').includes('9'), 'Expected response to include "9"'); + + // Can send a subsequent request. + await session.handleRequest('What is 11+25?', [], undefined, CancellationToken.None); + + // Verify we have a response of 36. + assert.strictEqual(session.status, ChatSessionStatus.Completed); + assert.ok(stream.output.join('\n').includes('36'), 'Expected response to include "36"'); + }) + ); + + stest({ description: 'can resume a session' }, + testRunner(async ({ sessionService }, stream, disposables) => { + let sessionId = ''; + { + const session = await sessionService.createSession('What is 1+8?', undefined, undefined, CancellationToken.None); + sessionId = session.sessionId; + disposables.add(session); + + await session.handleRequest('What is 1+8?', [], undefined, CancellationToken.None); + session.dispose(); + } + + { + const session = await new Promise((resolve, reject) => { + const interval = disposables.add(new IntervalTimer()); + interval.cancelAndSet(async () => { + const session = await sessionService.getSession(sessionId, undefined, undefined, false, CancellationToken.None); + if (session) { + interval.dispose(); + resolve(session); + } + }, 50); + disposables.add(disposableTimeout(() => reject(new Error('Timed out waiting for session')), 5_000)); + }); + disposables.add(session); + disposables.add(session.attachStream(stream)); + + await session.handleRequest('What was my previous question?', [], undefined, CancellationToken.None); + + // Verify we have a response of 9. + assert.strictEqual(session.status, ChatSessionStatus.Completed); + assert.ok(stream.output.join('\n').includes('8'), 'Expected response to include "8"'); + } + }) + ); + stest({ description: 'can read file without permission' }, + testRunner(async ({ sessionService }, stream, disposables) => { + const workingDirectory = path.join(scenariosPath, 'wkspc1'); + const file = path.join(workingDirectory, 'sample.js'); + const prompt = `Explain the contents of the file '${path.basename(file)}'. There is no need to check for contents in the directory. This file exists on disc.`; + const session = await sessionService.createSession(prompt, undefined, workingDirectory, CancellationToken.None); + disposables.add(session); + disposables.add(session.attachStream(stream)); + + await session.handleRequest(prompt, [], undefined, CancellationToken.None); + + assert.strictEqual(session.status, ChatSessionStatus.Completed); + assert.ok(stream.output.join('\n').includes('add'), 'Expected response to include "add"'); + }) + ); + stest({ description: 'request permission when reading file outside workspace' }, + testRunner(async ({ sessionService }, stream, disposables) => { + const workingDirectory = path.join(scenariosPath, 'wkspc1'); + const externalFile = path.join(scenariosPath, 'wkspc2', 'foobar.js'); + const prompt = `Explain the contents of the file '${path.basename(externalFile)}'. This file exists on disc but not in the current working directory.`; + const session = await sessionService.createSession(prompt, undefined, workingDirectory, CancellationToken.None); + disposables.add(session); + disposables.add(session.attachStream(stream)); + let permissionRequested = false; + + session.attachPermissionHandler(async (permission: PermissionRequest) => { + if (permission.kind === 'read' && permission.path.toLowerCase() === externalFile.toLowerCase()) { + permissionRequested = true; + return true; + } else if (permission.kind === 'shell' && (permission.intention.toLowerCase().includes('search') || permission.intention.toLowerCase().includes('find'))) { + permissionRequested = true; + return true; + } else { + return false; + } + }); + + await session.handleRequest(prompt, [], undefined, CancellationToken.None); + + assert.strictEqual(session.status, ChatSessionStatus.Completed); + assert.ok(permissionRequested, 'Expected permission to be requested for external file'); + }) + ); + stest({ description: 'can read attachment without permission' }, + testRunner(async ({ sessionService, promptResolver }, stream, disposables) => { + const workingDirectory = path.join(scenariosPath, 'wkspc1'); + const file = path.join(workingDirectory, 'sample.js'); + const { prompt, attachments } = await resolvePromptWithFileReferences( + `Explain the contents of the attached file. There is no need to check for contents in the directory. This file exists on disc.`, + [file], + promptResolver + ); + + const session = await sessionService.createSession(prompt, undefined, workingDirectory, CancellationToken.None); + disposables.add(session); + disposables.add(session.attachStream(stream)); + + await session.handleRequest(prompt, attachments, undefined, CancellationToken.None); + + assert.strictEqual(session.status, ChatSessionStatus.Completed); + assert.ok(stream.output.join('\n').includes('add'), 'Expected response to include "add"'); + }) + ); +}); + +function createWithRequestWithFileReference(prompt: string, files: string[]): ChatRequest { + const request = new TestChatRequest(prompt); + request.references = files.map(file => ({ + id: `file-${file}`, + name: path.basename(file), + value: Uri.file(file), + } satisfies ChatPromptReference)); + return request; +} + +function resolvePromptWithFileReferences(prompt: string, files: string[], promptResolver: CopilotCLIPromptResolver): Promise<{ prompt: string; attachments: any[] }> { + return promptResolver.resolvePrompt( + createWithRequestWithFileReference(prompt, files), + CancellationToken.None + ); +} \ No newline at end of file diff --git a/extensions/copilot/test/scenarios/test-cli/wkspc1/sample.js b/extensions/copilot/test/scenarios/test-cli/wkspc1/sample.js new file mode 100644 index 00000000000..cc811b6023a --- /dev/null +++ b/extensions/copilot/test/scenarios/test-cli/wkspc1/sample.js @@ -0,0 +1,6 @@ +/** + * Sample function to add two values + */ +function add(a, b) { + return a + b; +} \ No newline at end of file diff --git a/extensions/copilot/test/scenarios/test-cli/wkspc2/foobar.js b/extensions/copilot/test/scenarios/test-cli/wkspc2/foobar.js new file mode 100644 index 00000000000..e8ac482d60f --- /dev/null +++ b/extensions/copilot/test/scenarios/test-cli/wkspc2/foobar.js @@ -0,0 +1,6 @@ +/** + * Function baz logs 'baz' to the console. + */ +function baz() { + return console.log('baz'); +} \ No newline at end of file diff --git a/extensions/copilot/test/simulationTests.ts b/extensions/copilot/test/simulationTests.ts index 4ae759d5805..1679c6ed335 100644 --- a/extensions/copilot/test/simulationTests.ts +++ b/extensions/copilot/test/simulationTests.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ import './codeMapper/codeMapper.stest'; +import './e2e/cli.stest'; import './e2e/edit.stest'; import './e2e/explain.stest'; import './e2e/fetchWebPageTool.stest';