From af5ee252da6a90e4d34ff5164f686cc547f98a72 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Nov 2025 04:51:15 +0000 Subject: [PATCH] Add tests for Copilot CLI Prompt resolver (#1888) * Add tests for Copilot CLI Prompt resolver * Fix tests for windows * More windows test fixes * Updates * Remove trailing lines --- .../node/copilotcliPromptResolver.ts | 141 +++++++++++---- .../test/copilotcliPromptResolver.spec.ts | 166 ++++++++++++++++++ 2 files changed, 269 insertions(+), 38 deletions(-) create mode 100644 extensions/copilot/src/extension/agents/copilotcli/node/test/copilotcliPromptResolver.spec.ts diff --git a/extensions/copilot/src/extension/agents/copilotcli/node/copilotcliPromptResolver.ts b/extensions/copilot/src/extension/agents/copilotcli/node/copilotcliPromptResolver.ts index 272d9ce8762..1cd19309693 100644 --- a/extensions/copilot/src/extension/agents/copilotcli/node/copilotcliPromptResolver.ts +++ b/extensions/copilot/src/extension/agents/copilotcli/node/copilotcliPromptResolver.ts @@ -9,6 +9,7 @@ import { IFileSystemService } from '../../../../platform/filesystem/common/fileS import { ILogService } from '../../../../platform/log/common/logService'; import { isLocation } from '../../../../util/common/types'; import { raceCancellationError } from '../../../../util/vs/base/common/async'; +import { ResourceSet } from '../../../../util/vs/base/common/map'; import * as path from '../../../../util/vs/base/common/path'; import { URI } from '../../../../util/vs/base/common/uri'; import { ChatReferenceDiagnostic, FileType } from '../../../../vscodeTypes'; @@ -28,46 +29,33 @@ export class CopilotCLIPromptResolver { const allRefsTexts: string[] = []; const diagnosticTexts: string[] = []; const files: { path: string; name: string }[] = []; + const attachedFiles = new ResourceSet(); // TODO@rebornix: filter out implicit references for now. Will need to figure out how to support `` without poluting user prompt - request.references.filter(ref => !ref.id.startsWith('vscode.prompt.instructions')).forEach(ref => { - if (ref.value instanceof ChatReferenceDiagnostic) { - // Handle diagnostic reference - for (const [uri, diagnostics] of ref.value.diagnostics) { - if (uri.scheme !== 'file') { - continue; - } - for (const diagnostic of diagnostics) { - const severityMap: { [key: number]: string } = { - 0: 'error', - 1: 'warning', - 2: 'info', - 3: 'hint' - }; - const severity = severityMap[diagnostic.severity] ?? 'error'; - const code = (typeof diagnostic.code === 'object' && diagnostic.code !== null) ? diagnostic.code.value : diagnostic.code; - const codeStr = code ? ` [${code}]` : ''; - const line = diagnostic.range.start.line + 1; - diagnosticTexts.push(`- ${severity}${codeStr} at ${uri.fsPath}:${line}: ${diagnostic.message}`); - files.push({ path: uri.fsPath, name: path.basename(uri.fsPath) }); - } - } - } else { - const uri = URI.isUri(ref.value) ? ref.value : isLocation(ref.value) ? ref.value.uri : undefined; - if (!uri || uri.scheme !== 'file') { - return; - } - const filePath = uri.fsPath; + request.references.forEach(ref => { + if (shouldExcludeReference(ref)) { + return; + } + if (collectDiagnosticContent(ref.value, diagnosticTexts, files)) { + return; + } + const uri = URI.isUri(ref.value) ? ref.value : isLocation(ref.value) ? ref.value.uri : undefined; + if (!uri || uri.scheme !== 'file') { + return; + } + const filePath = uri.fsPath; + if (!attachedFiles.has(uri)) { + attachedFiles.add(uri); files.push({ path: filePath, name: ref.name || path.basename(filePath) }); - const valueText = URI.isUri(ref.value) ? - ref.value.fsPath : - isLocation(ref.value) ? - `${ref.value.uri.fsPath}:${ref.value.range.start.line + 1}` : - undefined; - if (valueText && ref.range) { - // Keep the original prompt untouched, just collect resolved paths - const variableText = request.prompt.substring(ref.range[0], ref.range[1]); - allRefsTexts.push(`- ${variableText} → ${valueText}`); - } + } + const valueText = URI.isUri(ref.value) ? + ref.value.fsPath : + isLocation(ref.value) ? + `${ref.value.uri.fsPath}:${ref.value.range.start.line + 1}` : + undefined; + if (valueText && ref.range) { + // Keep the original prompt untouched, just collect resolved paths + const variableText = request.prompt.substring(ref.range[0], ref.range[1]); + allRefsTexts.push(`- ${variableText} → ${valueText}`); } }); @@ -105,3 +93,80 @@ export class CopilotCLIPromptResolver { return { prompt, attachments }; } } + +function shouldExcludeReference(ref: vscode.ChatPromptReference): boolean { + return ref.id.startsWith('vscode.prompt.instructions'); +} + +function collectDiagnosticContent(value: unknown, diagnosticTexts: string[], files: { path: string; name: string }[]): boolean { + const attachedFiles = new ResourceSet(); + const diagnosticCollection = getChatReferenceDiagnostics(value); + if (!diagnosticCollection.length) { + return false; + } + + let hasDiagnostics = false; + // Handle diagnostic reference + for (const [uri, diagnostics] of diagnosticCollection) { + if (uri.scheme !== 'file') { + continue; + } + for (const diagnostic of diagnostics) { + const severityMap: { [key: number]: string } = { + 0: 'error', + 1: 'warning', + 2: 'info', + 3: 'hint' + }; + const severity = severityMap[diagnostic.severity] ?? 'error'; + const code = (typeof diagnostic.code === 'object' && diagnostic.code !== null) ? diagnostic.code.value : diagnostic.code; + const codeStr = code ? ` [${code}]` : ''; + const line = diagnostic.range.start.line + 1; + diagnosticTexts.push(`- ${severity}${codeStr} at ${uri.fsPath}:${line}: ${diagnostic.message}`); + hasDiagnostics = true; + if (!attachedFiles.has(uri)) { + attachedFiles.add(uri); + files.push({ path: uri.fsPath, name: path.basename(uri.fsPath) }); + } + } + } + return hasDiagnostics; +} + +function getChatReferenceDiagnostics(value: unknown): [vscode.Uri, readonly vscode.Diagnostic[]][] { + if (isChatReferenceDiagnostic(value)) { + return Array.from(value.diagnostics.values()); + } + if (isDiagnosticCollection(value)) { + const result: [vscode.Uri, readonly vscode.Diagnostic[]][] = []; + value.forEach((uri, diagnostics) => { + result.push([uri, diagnostics]); + }); + return result; + } + return []; +} +function isChatReferenceDiagnostic(value: unknown): value is ChatReferenceDiagnostic { + if (value instanceof ChatReferenceDiagnostic) { + return true; + } + + const possibleDiag = value as ChatReferenceDiagnostic; + if (possibleDiag.diagnostics && Array.isArray(possibleDiag.diagnostics)) { + return true; + } + return false; +} + +function isDiagnosticCollection(value: unknown): value is vscode.DiagnosticCollection { + const possibleDiag = value as vscode.DiagnosticCollection; + if (possibleDiag.clear && typeof possibleDiag.clear === 'function' && + possibleDiag.delete && typeof possibleDiag.delete === 'function' && + possibleDiag.get && typeof possibleDiag.get === 'function' && + possibleDiag.set && typeof possibleDiag.set === 'function' && + possibleDiag.forEach && typeof possibleDiag.forEach === 'function') { + return true; + } + + return false; +} diff --git a/extensions/copilot/src/extension/agents/copilotcli/node/test/copilotcliPromptResolver.spec.ts b/extensions/copilot/src/extension/agents/copilotcli/node/test/copilotcliPromptResolver.spec.ts new file mode 100644 index 00000000000..db3006b0efb --- /dev/null +++ b/extensions/copilot/src/extension/agents/copilotcli/node/test/copilotcliPromptResolver.spec.ts @@ -0,0 +1,166 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type * as vscode from 'vscode'; +import { IFileSystemService } from '../../../../../platform/filesystem/common/fileSystemService'; +import { MockFileSystemService } from '../../../../../platform/filesystem/node/test/mockFileSystemService'; +import { ILogService } from '../../../../../platform/log/common/logService'; +import { CancellationToken } from '../../../../../util/vs/base/common/cancellation'; +import { DisposableStore } from '../../../../../util/vs/base/common/lifecycle'; +import * as path from '../../../../../util/vs/base/common/path'; +import { URI } from '../../../../../util/vs/base/common/uri'; +import { ChatReferenceDiagnostic, Diagnostic, DiagnosticSeverity, FileType, Range } from '../../../../../vscodeTypes'; +import { createExtensionUnitTestingServices } from '../../../../test/node/services'; +import { TestChatRequest } from '../../../../test/node/testHelpers'; +import { CopilotCLIPromptResolver } from '../copilotcliPromptResolver'; + +function makeDiagnostic(line: number, message: string, severity: DiagnosticSeverity = DiagnosticSeverity.Error, code?: string): Diagnostic { + const diag = new Diagnostic( + new Range(line, 0, line, 0), + message, + severity + ); + diag.code = code; + return diag; +} + +// Helper to create a ChatRequest with references array patched +function withReferences(req: TestChatRequest, refs: unknown[]): TestChatRequest { + // vitest doesn't prevent mutation; emulate the readonly property by assignment cast + req.references = refs as vscode.ChatPromptReference[]; + return req; +} + +describe('CopilotCLIPromptResolver', () => { + const store = new DisposableStore(); + let resolver: CopilotCLIPromptResolver; + let fileSystemService: IFileSystemService; + let logService: ILogService; + + beforeEach(() => { + const services = store.add(createExtensionUnitTestingServices()); + const accessor = services.createTestingAccessor(); + fileSystemService = new MockFileSystemService(); + logService = accessor.get(ILogService); + resolver = new CopilotCLIPromptResolver(logService, fileSystemService); + }); + + afterEach(() => { + store.clear(); + vi.resetAllMocks(); + }); + + it('returns original prompt unchanged for slash command', async () => { + const req = new TestChatRequest('/help something'); + const { prompt, attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None); + expect(prompt).toBe('/help something'); + expect(attachments).toHaveLength(0); + }); + + it('collects file references and produces attachments plus reminder block', async () => { + // Spy on stat to simulate file type + const statSpy = vi.spyOn(fileSystemService, 'stat').mockResolvedValue({ type: FileType.File, size: 10 } as any); + + const fileA = URI.file(path.join('tmp', 'a.ts')); + const fileB = URI.file(path.join('tmp', 'b.ts')); + + const req = withReferences(new TestChatRequest('Explain a and b'), [ + { id: 'file-a', value: fileA, name: 'a.ts', range: [8, 9] }, // 'a' + { id: 'file-b', value: fileB, name: 'b.ts', range: [14, 15] } // 'b' + ]); + + const { prompt, attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None); + + // Should have reminder block + expect(prompt).toMatch(//); + expect(prompt).toMatch(/The user provided the following references:/); + expect(prompt).toContain(`- a → ${fileA.fsPath}`); + expect(prompt).toContain(`- b → ${fileB.fsPath}`); + + // Attachments reflect both files + expect(attachments.map(a => a.displayName).sort()).toEqual(['a.ts', 'b.ts']); + expect(attachments.every(a => a.type === 'file')).toBe(true); + // Stat called for each file + expect(statSpy).toHaveBeenCalledTimes(2); + }); + + it('includes diagnostics in reminder block with severity and line', async () => { + const statSpy = vi.spyOn(fileSystemService, 'stat').mockResolvedValue({ type: FileType.File, size: 10 } as any); + const fileUri = URI.file(path.join('workspace', 'src', 'index.ts')); + + const diagnostics = [ + makeDiagnostic(4, 'Unexpected any', 0, 'TS7005'), + makeDiagnostic(9, 'Possible undefined', 1) + ]; + + // ChatReferenceDiagnostic requires a Map of uri -> diagnostics array + const chatRefDiag: ChatReferenceDiagnostic = { diagnostics: [[fileUri, diagnostics]] }; + const req = withReferences(new TestChatRequest('Fix issues'), [ + { id: 'diag-1', value: chatRefDiag } + ]); + + const { prompt, attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None); + + expect(prompt).toMatch(/Fix issues/); + expect(prompt).toMatch(/The user provided the following diagnostics:/); + expect(prompt).toContain(`- error [TS7005] at ${fileUri.fsPath}:5: Unexpected any`); + expect(prompt).toContain(`- warning at ${fileUri.fsPath}:10: Possible undefined`); + // File should be attached once + expect(attachments).toHaveLength(1); + expect(attachments[0].path).toBe(fileUri.fsPath); + expect(statSpy).toHaveBeenCalledTimes(1); + }); + + it('attaches directories correctly', async () => { + const statSpy = vi.spyOn(fileSystemService, 'stat').mockResolvedValueOnce({ type: FileType.Directory, size: 0 } as any); + const dirUri = URI.file('/workspace/src'); + const req = withReferences(new TestChatRequest('List src'), [ + { id: 'src-dir', value: dirUri, name: 'src', range: [5, 8] } + ]); + + const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None); + expect(attachments).toHaveLength(1); + expect(attachments[0].type).toBe('directory'); + expect(attachments[0].displayName).toBe('src'); + expect(statSpy).toHaveBeenCalledTimes(1); + }); + + it('logs and ignores non file/directory stat types', async () => { + // Simulate an unknown type (e.g., FileType.SymbolicLink or other) + const statSpy = vi.spyOn(fileSystemService, 'stat').mockResolvedValue({ type: 99, size: 0 } as any); + const logSpy = vi.spyOn(logService, 'error').mockImplementation(() => { }); + const badUri = URI.file('/workspace/unknown'); + const req = withReferences(new TestChatRequest('Check unknown'), [ + { id: 'bad', value: badUri, name: 'unknown', range: [6, 13] } + ]); + + const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None); + expect(attachments).toHaveLength(0); // ignored + expect(statSpy).toHaveBeenCalledTimes(1); + expect(logSpy).toHaveBeenCalled(); + }); + + it('handles stat failure gracefully and logs error', async () => { + const error = new Error('stat failed'); + const statSpy = vi.spyOn(fileSystemService, 'stat').mockRejectedValue(error); + const logSpy = vi.spyOn(logService, 'error').mockImplementation(() => { }); + const fileUri = URI.file('/workspace/src/index.ts'); + const req = withReferences(new TestChatRequest('Read file'), [ + { id: 'file', value: fileUri, name: 'index.ts', range: [5, 10] } + ]); + + const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None); + expect(attachments).toHaveLength(0); + expect(statSpy).toHaveBeenCalledTimes(1); + expect(logSpy).toHaveBeenCalled(); + }); + + it('no reminder block when there are no references or diagnostics', async () => { + const req = new TestChatRequest('Just a question'); + const { prompt } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None); + expect(prompt).toBe('Just a question'); + }); +});