Skip to content

Commit 6dcddc8

Browse files
committed
Merge branch 'main' into lszomoru/wonderful-jackal
2 parents 0da6357 + c2c693d commit 6dcddc8

File tree

11 files changed

+518
-52
lines changed

11 files changed

+518
-52
lines changed

src/extension/agents/copilotcli/node/copilotCli.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,17 @@ export class CopilotCLISDK implements ICopilotCLISDK {
9999
public async getPackage(): Promise<typeof import('@github/copilot/sdk')> {
100100
try {
101101
// Ensure the node-pty shim exists before importing the SDK (required for CLI sessions)
102-
await ensureNodePtyShim(this.extensionContext.extensionPath, this.envService.appRoot, this.logService);
102+
await this.ensureNodePtyShim();
103103
return await import('@github/copilot/sdk');
104104
} catch (error) {
105105
this.logService.error(`[CopilotCLISDK] Failed to load @github/copilot/sdk: ${error}`);
106106
throw error;
107107
}
108108
}
109+
110+
protected async ensureNodePtyShim(): Promise<void> {
111+
await ensureNodePtyShim(this.extensionContext.extensionPath, this.envService.appRoot, this.logService);
112+
}
109113
}
110114

111115
export interface ICopilotCLISessionOptionsService {

src/extension/agents/copilotcli/node/copilotcliPromptResolver.ts

Lines changed: 103 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { IFileSystemService } from '../../../../platform/filesystem/common/fileS
99
import { ILogService } from '../../../../platform/log/common/logService';
1010
import { isLocation } from '../../../../util/common/types';
1111
import { raceCancellationError } from '../../../../util/vs/base/common/async';
12+
import { ResourceSet } from '../../../../util/vs/base/common/map';
1213
import * as path from '../../../../util/vs/base/common/path';
1314
import { URI } from '../../../../util/vs/base/common/uri';
1415
import { ChatReferenceDiagnostic, FileType } from '../../../../vscodeTypes';
@@ -28,46 +29,33 @@ export class CopilotCLIPromptResolver {
2829
const allRefsTexts: string[] = [];
2930
const diagnosticTexts: string[] = [];
3031
const files: { path: string; name: string }[] = [];
32+
const attachedFiles = new ResourceSet();
3133
// TODO@rebornix: filter out implicit references for now. Will need to figure out how to support `<reminder>` without poluting user prompt
32-
request.references.filter(ref => !ref.id.startsWith('vscode.prompt.instructions')).forEach(ref => {
33-
if (ref.value instanceof ChatReferenceDiagnostic) {
34-
// Handle diagnostic reference
35-
for (const [uri, diagnostics] of ref.value.diagnostics) {
36-
if (uri.scheme !== 'file') {
37-
continue;
38-
}
39-
for (const diagnostic of diagnostics) {
40-
const severityMap: { [key: number]: string } = {
41-
0: 'error',
42-
1: 'warning',
43-
2: 'info',
44-
3: 'hint'
45-
};
46-
const severity = severityMap[diagnostic.severity] ?? 'error';
47-
const code = (typeof diagnostic.code === 'object' && diagnostic.code !== null) ? diagnostic.code.value : diagnostic.code;
48-
const codeStr = code ? ` [${code}]` : '';
49-
const line = diagnostic.range.start.line + 1;
50-
diagnosticTexts.push(`- ${severity}${codeStr} at ${uri.fsPath}:${line}: ${diagnostic.message}`);
51-
files.push({ path: uri.fsPath, name: path.basename(uri.fsPath) });
52-
}
53-
}
54-
} else {
55-
const uri = URI.isUri(ref.value) ? ref.value : isLocation(ref.value) ? ref.value.uri : undefined;
56-
if (!uri || uri.scheme !== 'file') {
57-
return;
58-
}
59-
const filePath = uri.fsPath;
34+
request.references.forEach(ref => {
35+
if (shouldExcludeReference(ref)) {
36+
return;
37+
}
38+
if (collectDiagnosticContent(ref.value, diagnosticTexts, files)) {
39+
return;
40+
}
41+
const uri = URI.isUri(ref.value) ? ref.value : isLocation(ref.value) ? ref.value.uri : undefined;
42+
if (!uri || uri.scheme !== 'file') {
43+
return;
44+
}
45+
const filePath = uri.fsPath;
46+
if (!attachedFiles.has(uri)) {
47+
attachedFiles.add(uri);
6048
files.push({ path: filePath, name: ref.name || path.basename(filePath) });
61-
const valueText = URI.isUri(ref.value) ?
62-
ref.value.fsPath :
63-
isLocation(ref.value) ?
64-
`${ref.value.uri.fsPath}:${ref.value.range.start.line + 1}` :
65-
undefined;
66-
if (valueText && ref.range) {
67-
// Keep the original prompt untouched, just collect resolved paths
68-
const variableText = request.prompt.substring(ref.range[0], ref.range[1]);
69-
allRefsTexts.push(`- ${variableText}${valueText}`);
70-
}
49+
}
50+
const valueText = URI.isUri(ref.value) ?
51+
ref.value.fsPath :
52+
isLocation(ref.value) ?
53+
`${ref.value.uri.fsPath}:${ref.value.range.start.line + 1}` :
54+
undefined;
55+
if (valueText && ref.range) {
56+
// Keep the original prompt untouched, just collect resolved paths
57+
const variableText = request.prompt.substring(ref.range[0], ref.range[1]);
58+
allRefsTexts.push(`- ${variableText}${valueText}`);
7159
}
7260
});
7361

@@ -105,3 +93,80 @@ export class CopilotCLIPromptResolver {
10593
return { prompt, attachments };
10694
}
10795
}
96+
97+
function shouldExcludeReference(ref: vscode.ChatPromptReference): boolean {
98+
return ref.id.startsWith('vscode.prompt.instructions');
99+
}
100+
101+
function collectDiagnosticContent(value: unknown, diagnosticTexts: string[], files: { path: string; name: string }[]): boolean {
102+
const attachedFiles = new ResourceSet();
103+
const diagnosticCollection = getChatReferenceDiagnostics(value);
104+
if (!diagnosticCollection.length) {
105+
return false;
106+
}
107+
108+
let hasDiagnostics = false;
109+
// Handle diagnostic reference
110+
for (const [uri, diagnostics] of diagnosticCollection) {
111+
if (uri.scheme !== 'file') {
112+
continue;
113+
}
114+
for (const diagnostic of diagnostics) {
115+
const severityMap: { [key: number]: string } = {
116+
0: 'error',
117+
1: 'warning',
118+
2: 'info',
119+
3: 'hint'
120+
};
121+
const severity = severityMap[diagnostic.severity] ?? 'error';
122+
const code = (typeof diagnostic.code === 'object' && diagnostic.code !== null) ? diagnostic.code.value : diagnostic.code;
123+
const codeStr = code ? ` [${code}]` : '';
124+
const line = diagnostic.range.start.line + 1;
125+
diagnosticTexts.push(`- ${severity}${codeStr} at ${uri.fsPath}:${line}: ${diagnostic.message}`);
126+
hasDiagnostics = true;
127+
if (!attachedFiles.has(uri)) {
128+
attachedFiles.add(uri);
129+
files.push({ path: uri.fsPath, name: path.basename(uri.fsPath) });
130+
}
131+
}
132+
}
133+
return hasDiagnostics;
134+
}
135+
136+
function getChatReferenceDiagnostics(value: unknown): [vscode.Uri, readonly vscode.Diagnostic[]][] {
137+
if (isChatReferenceDiagnostic(value)) {
138+
return Array.from(value.diagnostics.values());
139+
}
140+
if (isDiagnosticCollection(value)) {
141+
const result: [vscode.Uri, readonly vscode.Diagnostic[]][] = [];
142+
value.forEach((uri, diagnostics) => {
143+
result.push([uri, diagnostics]);
144+
});
145+
return result;
146+
}
147+
return [];
148+
}
149+
function isChatReferenceDiagnostic(value: unknown): value is ChatReferenceDiagnostic {
150+
if (value instanceof ChatReferenceDiagnostic) {
151+
return true;
152+
}
153+
154+
const possibleDiag = value as ChatReferenceDiagnostic;
155+
if (possibleDiag.diagnostics && Array.isArray(possibleDiag.diagnostics)) {
156+
return true;
157+
}
158+
return false;
159+
}
160+
161+
function isDiagnosticCollection(value: unknown): value is vscode.DiagnosticCollection {
162+
const possibleDiag = value as vscode.DiagnosticCollection;
163+
if (possibleDiag.clear && typeof possibleDiag.clear === 'function' &&
164+
possibleDiag.delete && typeof possibleDiag.delete === 'function' &&
165+
possibleDiag.get && typeof possibleDiag.get === 'function' &&
166+
possibleDiag.set && typeof possibleDiag.set === 'function' &&
167+
possibleDiag.forEach && typeof possibleDiag.forEach === 'function') {
168+
return true;
169+
}
170+
171+
return false;
172+
}

src/extension/agents/copilotcli/node/copilotcliSession.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -271,10 +271,25 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes
271271
// TODO:@rebornix @lszomoru
272272
// If user is writing a file in the working directory configured for the session, AND the working directory is not a workspace folder,
273273
// auto-approve the write request. Currently we only set non-workspace working directories when using git worktrees.
274-
const data = Uri.file(permissionRequest.fileName);
274+
const editFile = Uri.file(permissionRequest.fileName);
275+
276+
const isWorkspaceFile = this.workspaceService.getWorkspaceFolder(editFile);
277+
const isWorkingDirectoryFile = !this.workspaceService.getWorkspaceFolder(Uri.file(workingDirectory)) && extUriBiasedIgnorePathCase.isEqualOrParent(editFile, Uri.file(workingDirectory));
278+
279+
if (isWorkspaceFile || isWorkingDirectoryFile) {
280+
if (isWorkspaceFile) {
281+
this.logService.trace(`[CopilotCLISession] Auto Approving request to write file in workspace ${permissionRequest.fileName}`);
282+
} else {
283+
this.logService.trace(`[CopilotCLISession] Auto Approving request to write file in working directory ${permissionRequest.fileName}`);
284+
}
285+
const editKey = getEditKeyForFile(editFile);
286+
287+
// If we're editing a file, start tracking the edit & wait for core to acknowledge it.
288+
if (editKey && this._stream) {
289+
this.logService.trace(`[CopilotCLISession] Starting to track edit for toolCallId ${editKey} & file ${editFile.fsPath}`);
290+
await editTracker.trackEdit(editKey, [editFile], this._stream);
291+
}
275292

276-
if (!this.workspaceService.getWorkspaceFolder(Uri.file(workingDirectory)) && extUriBiasedIgnorePathCase.isEqualOrParent(data, Uri.file(workingDirectory))) {
277-
this.logService.trace(`[CopilotCLISession] Auto Approving request to write file in working directory ${permissionRequest.fileName}`);
278293
return { kind: 'approved' };
279294
}
280295
}

src/extension/agents/copilotcli/node/copilotcliSessionService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS
8484
});
8585
}
8686

87-
private monitorSessionFiles() {
87+
protected monitorSessionFiles() {
8888
try {
8989
const sessionDir = joinPath(this.nativeEnv.userHome, '.copilot', 'session-state');
9090
const watcher = this._register(this.fileSystem.createFileSystemWatcher(new RelativePattern(sessionDir, '*.jsonl')));
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
7+
import type * as vscode from 'vscode';
8+
import { IFileSystemService } from '../../../../../platform/filesystem/common/fileSystemService';
9+
import { MockFileSystemService } from '../../../../../platform/filesystem/node/test/mockFileSystemService';
10+
import { ILogService } from '../../../../../platform/log/common/logService';
11+
import { CancellationToken } from '../../../../../util/vs/base/common/cancellation';
12+
import { DisposableStore } from '../../../../../util/vs/base/common/lifecycle';
13+
import * as path from '../../../../../util/vs/base/common/path';
14+
import { URI } from '../../../../../util/vs/base/common/uri';
15+
import { ChatReferenceDiagnostic, Diagnostic, DiagnosticSeverity, FileType, Range } from '../../../../../vscodeTypes';
16+
import { createExtensionUnitTestingServices } from '../../../../test/node/services';
17+
import { TestChatRequest } from '../../../../test/node/testHelpers';
18+
import { CopilotCLIPromptResolver } from '../copilotcliPromptResolver';
19+
20+
function makeDiagnostic(line: number, message: string, severity: DiagnosticSeverity = DiagnosticSeverity.Error, code?: string): Diagnostic {
21+
const diag = new Diagnostic(
22+
new Range(line, 0, line, 0),
23+
message,
24+
severity
25+
);
26+
diag.code = code;
27+
return diag;
28+
}
29+
30+
// Helper to create a ChatRequest with references array patched
31+
function withReferences(req: TestChatRequest, refs: unknown[]): TestChatRequest {
32+
// vitest doesn't prevent mutation; emulate the readonly property by assignment cast
33+
req.references = refs as vscode.ChatPromptReference[];
34+
return req;
35+
}
36+
37+
describe('CopilotCLIPromptResolver', () => {
38+
const store = new DisposableStore();
39+
let resolver: CopilotCLIPromptResolver;
40+
let fileSystemService: IFileSystemService;
41+
let logService: ILogService;
42+
43+
beforeEach(() => {
44+
const services = store.add(createExtensionUnitTestingServices());
45+
const accessor = services.createTestingAccessor();
46+
fileSystemService = new MockFileSystemService();
47+
logService = accessor.get(ILogService);
48+
resolver = new CopilotCLIPromptResolver(logService, fileSystemService);
49+
});
50+
51+
afterEach(() => {
52+
store.clear();
53+
vi.resetAllMocks();
54+
});
55+
56+
it('returns original prompt unchanged for slash command', async () => {
57+
const req = new TestChatRequest('/help something');
58+
const { prompt, attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None);
59+
expect(prompt).toBe('/help something');
60+
expect(attachments).toHaveLength(0);
61+
});
62+
63+
it('collects file references and produces attachments plus reminder block', async () => {
64+
// Spy on stat to simulate file type
65+
const statSpy = vi.spyOn(fileSystemService, 'stat').mockResolvedValue({ type: FileType.File, size: 10 } as any);
66+
67+
const fileA = URI.file(path.join('tmp', 'a.ts'));
68+
const fileB = URI.file(path.join('tmp', 'b.ts'));
69+
70+
const req = withReferences(new TestChatRequest('Explain a and b'), [
71+
{ id: 'file-a', value: fileA, name: 'a.ts', range: [8, 9] }, // 'a'
72+
{ id: 'file-b', value: fileB, name: 'b.ts', range: [14, 15] } // 'b'
73+
]);
74+
75+
const { prompt, attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None);
76+
77+
// Should have reminder block
78+
expect(prompt).toMatch(/<reminder>/);
79+
expect(prompt).toMatch(/The user provided the following references:/);
80+
expect(prompt).toContain(`- a → ${fileA.fsPath}`);
81+
expect(prompt).toContain(`- b → ${fileB.fsPath}`);
82+
83+
// Attachments reflect both files
84+
expect(attachments.map(a => a.displayName).sort()).toEqual(['a.ts', 'b.ts']);
85+
expect(attachments.every(a => a.type === 'file')).toBe(true);
86+
// Stat called for each file
87+
expect(statSpy).toHaveBeenCalledTimes(2);
88+
});
89+
90+
it('includes diagnostics in reminder block with severity and line', async () => {
91+
const statSpy = vi.spyOn(fileSystemService, 'stat').mockResolvedValue({ type: FileType.File, size: 10 } as any);
92+
const fileUri = URI.file(path.join('workspace', 'src', 'index.ts'));
93+
94+
const diagnostics = [
95+
makeDiagnostic(4, 'Unexpected any', 0, 'TS7005'),
96+
makeDiagnostic(9, 'Possible undefined', 1)
97+
];
98+
99+
// ChatReferenceDiagnostic requires a Map of uri -> diagnostics array
100+
const chatRefDiag: ChatReferenceDiagnostic = { diagnostics: [[fileUri, diagnostics]] };
101+
const req = withReferences(new TestChatRequest('Fix issues'), [
102+
{ id: 'diag-1', value: chatRefDiag }
103+
]);
104+
105+
const { prompt, attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None);
106+
107+
expect(prompt).toMatch(/Fix issues/);
108+
expect(prompt).toMatch(/The user provided the following diagnostics:/);
109+
expect(prompt).toContain(`- error [TS7005] at ${fileUri.fsPath}:5: Unexpected any`);
110+
expect(prompt).toContain(`- warning at ${fileUri.fsPath}:10: Possible undefined`);
111+
// File should be attached once
112+
expect(attachments).toHaveLength(1);
113+
expect(attachments[0].path).toBe(fileUri.fsPath);
114+
expect(statSpy).toHaveBeenCalledTimes(1);
115+
});
116+
117+
it('attaches directories correctly', async () => {
118+
const statSpy = vi.spyOn(fileSystemService, 'stat').mockResolvedValueOnce({ type: FileType.Directory, size: 0 } as any);
119+
const dirUri = URI.file('/workspace/src');
120+
const req = withReferences(new TestChatRequest('List src'), [
121+
{ id: 'src-dir', value: dirUri, name: 'src', range: [5, 8] }
122+
]);
123+
124+
const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None);
125+
expect(attachments).toHaveLength(1);
126+
expect(attachments[0].type).toBe('directory');
127+
expect(attachments[0].displayName).toBe('src');
128+
expect(statSpy).toHaveBeenCalledTimes(1);
129+
});
130+
131+
it('logs and ignores non file/directory stat types', async () => {
132+
// Simulate an unknown type (e.g., FileType.SymbolicLink or other)
133+
const statSpy = vi.spyOn(fileSystemService, 'stat').mockResolvedValue({ type: 99, size: 0 } as any);
134+
const logSpy = vi.spyOn(logService, 'error').mockImplementation(() => { });
135+
const badUri = URI.file('/workspace/unknown');
136+
const req = withReferences(new TestChatRequest('Check unknown'), [
137+
{ id: 'bad', value: badUri, name: 'unknown', range: [6, 13] }
138+
]);
139+
140+
const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None);
141+
expect(attachments).toHaveLength(0); // ignored
142+
expect(statSpy).toHaveBeenCalledTimes(1);
143+
expect(logSpy).toHaveBeenCalled();
144+
});
145+
146+
it('handles stat failure gracefully and logs error', async () => {
147+
const error = new Error('stat failed');
148+
const statSpy = vi.spyOn(fileSystemService, 'stat').mockRejectedValue(error);
149+
const logSpy = vi.spyOn(logService, 'error').mockImplementation(() => { });
150+
const fileUri = URI.file('/workspace/src/index.ts');
151+
const req = withReferences(new TestChatRequest('Read file'), [
152+
{ id: 'file', value: fileUri, name: 'index.ts', range: [5, 10] }
153+
]);
154+
155+
const { attachments } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None);
156+
expect(attachments).toHaveLength(0);
157+
expect(statSpy).toHaveBeenCalledTimes(1);
158+
expect(logSpy).toHaveBeenCalled();
159+
});
160+
161+
it('no reminder block when there are no references or diagnostics', async () => {
162+
const req = new TestChatRequest('Just a question');
163+
const { prompt } = await resolver.resolvePrompt(req as unknown as vscode.ChatRequest, CancellationToken.None);
164+
expect(prompt).toBe('Just a question');
165+
});
166+
});

0 commit comments

Comments
 (0)