From 29ae49887d5111c2aa9e1e66e025e3fe4dd379ee Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Wed, 22 Oct 2025 23:32:03 -0700 Subject: [PATCH 01/16] image attachment support for CLI --- package.json | 3 +- .../copilotcli/node/copilotcliAgentManager.ts | 14 ++- .../copilotCLIChatSessionsContribution.ts | 20 ++++- .../vscode-node/copilotCLIImageSupport.ts | 90 +++++++++++++++++++ 4 files changed, 120 insertions(+), 7 deletions(-) create mode 100644 src/extension/chatSessions/vscode-node/copilotCLIImageSupport.ts diff --git a/package.json b/package.json index 8e7dbace11..4001500248 100644 --- a/package.json +++ b/package.json @@ -4173,7 +4173,8 @@ "capabilities": { "supportsFileAttachments": true, "supportsProblemAttachments": true, - "supportsToolAttachments": false + "supportsToolAttachments": false, + "supportsImageAttachments": true }, "commands": [ { diff --git a/src/extension/agents/copilotcli/node/copilotcliAgentManager.ts b/src/extension/agents/copilotcli/node/copilotcliAgentManager.ts index 5e4901b67d..702d2cc5d8 100644 --- a/src/extension/agents/copilotcli/node/copilotcliAgentManager.ts +++ b/src/extension/agents/copilotcli/node/copilotcliAgentManager.ts @@ -47,13 +47,14 @@ export class CopilotCLIAgentManager extends Disposable { context: vscode.ChatContext, stream: vscode.ChatResponseStream, modelId: ModelProvider | undefined, + imageAttachmentPaths: string[], token: vscode.CancellationToken ): Promise<{ copilotcliSessionId: string | undefined }> { const isNewSession = !copilotcliSessionId; const sessionIdForLog = copilotcliSessionId ?? 'new'; this.logService.trace(`[CopilotCLIAgentManager] Handling request for sessionId=${sessionIdForLog}.`); - const { prompt, attachments } = await this.resolvePrompt(request); + const { prompt, attachments } = await this.resolvePrompt(request, imageAttachmentPaths); // Check if we already have a session wrapper let session = copilotcliSessionId ? this.sessionService.findSessionWrapper(copilotcliSessionId) : undefined; @@ -74,7 +75,7 @@ export class CopilotCLIAgentManager extends Disposable { return { copilotcliSessionId: session.sessionId }; } - private async resolvePrompt(request: vscode.ChatRequest): Promise<{ prompt: string; attachments: Attachment[] }> { + private async resolvePrompt(request: vscode.ChatRequest, imageAttachmentPaths: string[]): Promise<{ prompt: string; attachments: Attachment[] }> { if (request.prompt.startsWith('/')) { return { prompt: request.prompt, attachments: [] }; // likely a slash command, don't modify } @@ -151,6 +152,15 @@ export class CopilotCLIAgentManager extends Disposable { if (diagnosticTexts.length > 0) { reminderParts.push(`The user provided the following diagnostics:\n${diagnosticTexts.join('\n')}`); } + if (imageAttachmentPaths.length > 0) { + const imageAttachmentsText = imageAttachmentPaths.map(p => `- ${p}`).join('\n'); + reminderParts.push(`The user provided the following image attachments:\n${imageAttachmentsText}`); + attachments.push(...imageAttachmentPaths.map(p => ({ + type: 'file', + displayName: path.basename(p), + path: p + }))); + } let prompt = request.prompt; if (reminderParts.length > 0) { diff --git a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts index 19ac6f7978..fd3e98992c 100644 --- a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts +++ b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts @@ -14,6 +14,7 @@ import { CopilotCLIAgentManager } from '../../agents/copilotcli/node/copilotcliA import { ICopilotCLISessionService } from '../../agents/copilotcli/node/copilotcliSessionService'; import { buildChatHistoryFromEvents } from '../../agents/copilotcli/node/copilotcliToolInvocationFormatter'; import { ChatSummarizerProvider } from '../../prompt/node/summarizer'; +import { ImageStorage } from './copilotCLIImageSupport'; import { ICopilotCLITerminalIntegration } from './copilotCLITerminalIntegration'; import { CopilotChatSessionsProvider } from './copilotCloudSessionsProvider'; @@ -195,6 +196,7 @@ export class CopilotCLIChatSessionContentProvider implements vscode.ChatSessionC } export class CopilotCLIChatSessionParticipant { + private readonly imageStore: ImageStorage; constructor( private readonly sessionType: string, private readonly copilotcliAgentManager: CopilotCLIAgentManager, @@ -202,18 +204,28 @@ export class CopilotCLIChatSessionParticipant { private readonly sessionItemProvider: CopilotCLIChatSessionItemProvider, private readonly cloudSessionProvider: CopilotChatSessionsProvider | undefined, private readonly summarizer: ChatSummarizerProvider, - @IGitService private readonly gitService: IGitService - ) { } + @IGitService private readonly gitService: IGitService, + @IVSCodeExtensionContext context: IVSCodeExtensionContext, + ) { + this.imageStore = new ImageStorage(context); + } createHandler(): ChatExtendedRequestHandler { return this.handleRequest.bind(this); } private async handleRequest(request: vscode.ChatRequest, context: vscode.ChatContext, stream: vscode.ChatResponseStream, token: vscode.CancellationToken): Promise { + const imageAttachmentPaths = await Promise.all(request.references.filter(ref => ref.value instanceof vscode.ChatReferenceBinaryData).map(ref => { + const binaryData = ref.value as vscode.ChatReferenceBinaryData; + return binaryData.data().then(buffer => { + return this.imageStore.storeImage(buffer, binaryData.mimeType).then(uri => uri.fsPath); + }); + })); + const { chatSessionContext } = context; if (chatSessionContext) { if (chatSessionContext.isUntitled) { - const { copilotcliSessionId } = await this.copilotcliAgentManager.handleRequest(undefined, request, context, stream, undefined, token); + const { copilotcliSessionId } = await this.copilotcliAgentManager.handleRequest(undefined, request, context, stream, undefined, imageAttachmentPaths, token); if (!copilotcliSessionId) { stream.warning(localize('copilotcli.failedToCreateSession', "Failed to create a new CopilotCLI session.")); return {}; @@ -255,7 +267,7 @@ export class CopilotCLIChatSessionParticipant { } this.sessionService.setSessionStatus(id, vscode.ChatSessionStatus.InProgress); - await this.copilotcliAgentManager.handleRequest(id, request, context, stream, getModelProvider(_sessionModel.get(id)?.id), token); + await this.copilotcliAgentManager.handleRequest(id, request, context, stream, getModelProvider(_sessionModel.get(id)?.id), imageAttachmentPaths, token); this.sessionService.setSessionStatus(id, vscode.ChatSessionStatus.Completed); return {}; } diff --git a/src/extension/chatSessions/vscode-node/copilotCLIImageSupport.ts b/src/extension/chatSessions/vscode-node/copilotCLIImageSupport.ts new file mode 100644 index 0000000000..156d278bbd --- /dev/null +++ b/src/extension/chatSessions/vscode-node/copilotCLIImageSupport.ts @@ -0,0 +1,90 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as vscode from 'vscode'; +import { IVSCodeExtensionContext } from '../../../platform/extContext/common/extensionContext'; +import { URI } from '../../../util/vs/base/common/uri'; + +export class ImageStorage { + private readonly storageDir: URI; + + constructor(private readonly context: IVSCodeExtensionContext) { + this.storageDir = URI.joinPath(this.context.globalStorageUri, 'copilot-cli-images'); + this.initialize(); + } + + private async initialize(): Promise { + try { + await vscode.workspace.fs.createDirectory(this.storageDir); + await this.cleanupOldImages(); + } catch (error) { + console.error('ImageStorage: Failed to initialize', error); + } + } + + async storeImage(imageData: Uint8Array, mimeType: string): Promise { + const timestamp = Date.now(); + const randomId = Math.random().toString(36).substring(2, 10); + const extension = this.getExtension(mimeType); + const filename = `${timestamp}-${randomId}${extension}`; + const imageUri = URI.joinPath(this.storageDir, filename); + + await vscode.workspace.fs.writeFile(imageUri, imageData); + return imageUri; + } + + async getImage(uri: URI): Promise { + try { + const data = await vscode.workspace.fs.readFile(uri); + return data; + } catch { + return undefined; + } + } + + async deleteImage(uri: URI): Promise { + try { + await vscode.workspace.fs.delete(uri); + } catch { + // Already deleted + } + } + + async cleanupOldImages(maxAgeMs: number = 7 * 24 * 60 * 60 * 1000): Promise { + try { + const entries = await vscode.workspace.fs.readDirectory(this.storageDir); + const now = Date.now(); + const cutoff = now - maxAgeMs; + + for (const [filename, fileType] of entries) { + if (fileType === vscode.FileType.File) { + const fileUri = URI.joinPath(this.storageDir, filename); + try { + const stat = await vscode.workspace.fs.stat(fileUri); + if (stat.mtime < cutoff) { + await vscode.workspace.fs.delete(fileUri); + } + } catch { + // Skip files we can't access + } + } + } + } catch (error) { + console.error('ImageStorage: Failed to cleanup old images', error); + } + } + + private getExtension(mimeType: string): string { + const map: Record = { + 'image/png': '.png', + 'image/jpeg': '.jpg', + 'image/jpg': '.jpg', + 'image/gif': '.gif', + 'image/webp': '.webp', + 'image/bmp': '.bmp', + }; + return map[mimeType.toLowerCase()] || '.bin'; + } +} \ No newline at end of file From 102450e64dc4590caea14875aee1cca0cbdf9ed2 Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Sat, 8 Nov 2025 10:26:46 -0800 Subject: [PATCH 02/16] :lipstick: proper layering for prompt parser. --- .../chatSessions/vscode-node/chatSessions.ts | 2 +- .../copilotCLIChatSessionsContribution.ts | 17 +----- .../vscode-node/copilotCLIPromptResolver.ts} | 52 +++++++++++++++---- 3 files changed, 46 insertions(+), 25 deletions(-) rename src/extension/{agents/copilotcli/node/copilotcliPromptResolver.ts => chatSessions/vscode-node/copilotCLIPromptResolver.ts} (68%) diff --git a/src/extension/chatSessions/vscode-node/chatSessions.ts b/src/extension/chatSessions/vscode-node/chatSessions.ts index c9b582acbb..d5e11b4407 100644 --- a/src/extension/chatSessions/vscode-node/chatSessions.ts +++ b/src/extension/chatSessions/vscode-node/chatSessions.ts @@ -15,7 +15,6 @@ import { ClaudeAgentManager } from '../../agents/claude/node/claudeCodeAgent'; import { ClaudeCodeSdkService, IClaudeCodeSdkService } from '../../agents/claude/node/claudeCodeSdkService'; import { ClaudeCodeSessionService, IClaudeCodeSessionService } from '../../agents/claude/node/claudeCodeSessionService'; import { CopilotCLIModels, CopilotCLISDK, CopilotCLISessionOptionsService, ICopilotCLIModels, ICopilotCLISDK, ICopilotCLISessionOptionsService } from '../../agents/copilotcli/node/copilotCli'; -import { CopilotCLIPromptResolver } from '../../agents/copilotcli/node/copilotcliPromptResolver'; import { CopilotCLISessionService, ICopilotCLISessionService } from '../../agents/copilotcli/node/copilotcliSessionService'; import { ILanguageModelServer, LanguageModelServer } from '../../agents/node/langModelServer'; import { IExtensionContribution } from '../../common/contributions'; @@ -24,6 +23,7 @@ import { ClaudeChatSessionContentProvider } from './claudeChatSessionContentProv import { ClaudeChatSessionItemProvider } from './claudeChatSessionItemProvider'; import { ClaudeChatSessionParticipant } from './claudeChatSessionParticipant'; import { CopilotCLIChatSessionContentProvider, CopilotCLIChatSessionItemProvider, CopilotCLIChatSessionParticipant, CopilotCLIWorktreeManager, registerCLIChatCommands } from './copilotCLIChatSessionsContribution'; +import { CopilotCLIPromptResolver } from './copilotCLIPromptResolver'; import { CopilotCLITerminalIntegration, ICopilotCLITerminalIntegration } from './copilotCLITerminalIntegration'; import { CopilotCloudSessionsProvider } from './copilotCloudSessionsProvider'; import { PRContentProvider } from './prContentProvider'; diff --git a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts index 90c24b171c..4c7c8c9bd7 100644 --- a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts +++ b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts @@ -13,11 +13,10 @@ import { Emitter, Event } from '../../../util/vs/base/common/event'; import { Disposable, DisposableStore, IDisposable } from '../../../util/vs/base/common/lifecycle'; import { localize } from '../../../util/vs/nls'; import { ICopilotCLIModels } from '../../agents/copilotcli/node/copilotCli'; -import { CopilotCLIPromptResolver } from '../../agents/copilotcli/node/copilotcliPromptResolver'; import { ICopilotCLISession } from '../../agents/copilotcli/node/copilotcliSession'; import { ICopilotCLISessionService } from '../../agents/copilotcli/node/copilotcliSessionService'; import { ChatSummarizerProvider } from '../../prompt/node/summarizer'; -import { ImageStorage } from './copilotCLIImageSupport'; +import { CopilotCLIPromptResolver } from './copilotCLIPromptResolver'; import { ICopilotCLITerminalIntegration } from './copilotCLITerminalIntegration'; import { ConfirmationResult, CopilotCloudSessionsProvider } from './copilotCloudSessionsProvider'; @@ -293,7 +292,6 @@ export class CopilotCLIChatSessionContentProvider implements vscode.ChatSessionC } export class CopilotCLIChatSessionParticipant { - private readonly imageStore: ImageStorage; constructor( private readonly promptResolver: CopilotCLIPromptResolver, private readonly sessionItemProvider: CopilotCLIChatSessionItemProvider, @@ -304,26 +302,15 @@ export class CopilotCLIChatSessionParticipant { @ICopilotCLIModels private readonly copilotCLIModels: ICopilotCLIModels, @ICopilotCLISessionService private readonly sessionService: ICopilotCLISessionService, @ITelemetryService private readonly telemetryService: ITelemetryService, - @IVSCodeExtensionContext context: IVSCodeExtensionContext, - ) { - this.imageStore = new ImageStorage(context); - } + ) { } createHandler(): ChatExtendedRequestHandler { return this.handleRequest.bind(this); } private async handleRequest(request: vscode.ChatRequest, context: vscode.ChatContext, stream: vscode.ChatResponseStream, token: vscode.CancellationToken): Promise { - const imageAttachmentPaths = await Promise.all(request.references.filter(ref => ref.value instanceof vscode.ChatReferenceBinaryData).map(ref => { - const binaryData = ref.value as vscode.ChatReferenceBinaryData; - return binaryData.data().then(buffer => { - return this.imageStore.storeImage(buffer, binaryData.mimeType).then(uri => uri.fsPath); - }); - })); - const { chatSessionContext } = context; - /* __GDPR__ "copilotcli.chat.invoke" : { "owner": "joshspicer", diff --git a/src/extension/agents/copilotcli/node/copilotcliPromptResolver.ts b/src/extension/chatSessions/vscode-node/copilotCLIPromptResolver.ts similarity index 68% rename from src/extension/agents/copilotcli/node/copilotcliPromptResolver.ts rename to src/extension/chatSessions/vscode-node/copilotCLIPromptResolver.ts index 272d9ce876..0d93547e57 100644 --- a/src/extension/agents/copilotcli/node/copilotcliPromptResolver.ts +++ b/src/extension/chatSessions/vscode-node/copilotCLIPromptResolver.ts @@ -5,19 +5,26 @@ import type { Attachment } from '@github/copilot/sdk'; import type * as vscode from 'vscode'; -import { IFileSystemService } from '../../../../platform/filesystem/common/fileSystemService'; -import { ILogService } from '../../../../platform/log/common/logService'; -import { isLocation } from '../../../../util/common/types'; -import { raceCancellationError } from '../../../../util/vs/base/common/async'; -import * as path from '../../../../util/vs/base/common/path'; -import { URI } from '../../../../util/vs/base/common/uri'; -import { ChatReferenceDiagnostic, FileType } from '../../../../vscodeTypes'; +import { IVSCodeExtensionContext } from '../../../platform/extContext/common/extensionContext'; +import { IFileSystemService } from '../../../platform/filesystem/common/fileSystemService'; +import { ILogService } from '../../../platform/log/common/logService'; +import { isLocation } from '../../../util/common/types'; +import { raceCancellationError } from '../../../util/vs/base/common/async'; +import * as path from '../../../util/vs/base/common/path'; +import { URI } from '../../../util/vs/base/common/uri'; +import { ChatReferenceBinaryData, ChatReferenceDiagnostic, FileType } from '../../../vscodeTypes'; +import { ImageStorage } from './copilotCLIImageSupport'; export class CopilotCLIPromptResolver { + private readonly imageStorage: ImageStorage; + constructor( @ILogService private readonly logService: ILogService, @IFileSystemService private readonly fileSystemService: IFileSystemService, - ) { } + @IVSCodeExtensionContext extensionContext: IVSCodeExtensionContext, + ) { + this.imageStorage = new ImageStorage(extensionContext); + } public async resolvePrompt(request: vscode.ChatRequest, token: vscode.CancellationToken): Promise<{ prompt: string; attachments: Attachment[] }> { if (request.prompt.startsWith('/')) { @@ -28,9 +35,27 @@ export class CopilotCLIPromptResolver { const allRefsTexts: string[] = []; const diagnosticTexts: string[] = []; const files: { path: string; name: string }[] = []; + const imageAttachmentPaths: string[] = []; + + // Handle image attachments + const imageRefs = request.references.filter(ref => ref.value instanceof ChatReferenceBinaryData); + await Promise.all(imageRefs.map(async ref => { + const binaryData = ref.value as ChatReferenceBinaryData; + try { + const buffer = await binaryData.data(); + const uri = await this.imageStorage.storeImage(buffer, binaryData.mimeType); + imageAttachmentPaths.push(uri.fsPath); + } catch (error) { + this.logService.error(`[CopilotCLIPromptResolver] Failed to store image: ${error}`); + } + })); + // 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) { + if (ref.value instanceof ChatReferenceBinaryData) { + // Already handled above + return; + } else if (ref.value instanceof ChatReferenceDiagnostic) { // Handle diagnostic reference for (const [uri, diagnostics] of ref.value.diagnostics) { if (uri.scheme !== 'file') { @@ -96,6 +121,15 @@ export class CopilotCLIPromptResolver { if (diagnosticTexts.length > 0) { reminderParts.push(`The user provided the following diagnostics:\n${diagnosticTexts.join('\n')}`); } + if (imageAttachmentPaths.length > 0) { + const imageAttachmentsText = imageAttachmentPaths.map(p => `- ${p}`).join('\n'); + reminderParts.push(`The user provided the following image attachments:\n${imageAttachmentsText}`); + attachments.push(...imageAttachmentPaths.map(p => ({ + type: 'file' as const, + displayName: path.basename(p), + path: p + }))); + } let prompt = request.prompt; if (reminderParts.length > 0) { From 5dc18fd4a0de319e200d3d60aa58abc524330b7b Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Sat, 8 Nov 2025 10:38:56 -0800 Subject: [PATCH 03/16] improve prompt resolver --- .../vscode-node/copilotCLIPromptResolver.ts | 89 +++++++++---------- 1 file changed, 44 insertions(+), 45 deletions(-) diff --git a/src/extension/chatSessions/vscode-node/copilotCLIPromptResolver.ts b/src/extension/chatSessions/vscode-node/copilotCLIPromptResolver.ts index 0d93547e57..85acd73db0 100644 --- a/src/extension/chatSessions/vscode-node/copilotCLIPromptResolver.ts +++ b/src/extension/chatSessions/vscode-node/copilotCLIPromptResolver.ts @@ -34,27 +34,25 @@ export class CopilotCLIPromptResolver { const attachments: Attachment[] = []; const allRefsTexts: string[] = []; const diagnosticTexts: string[] = []; - const files: { path: string; name: string }[] = []; - const imageAttachmentPaths: string[] = []; - - // Handle image attachments - const imageRefs = request.references.filter(ref => ref.value instanceof ChatReferenceBinaryData); - await Promise.all(imageRefs.map(async ref => { - const binaryData = ref.value as ChatReferenceBinaryData; - try { - const buffer = await binaryData.data(); - const uri = await this.imageStorage.storeImage(buffer, binaryData.mimeType); - imageAttachmentPaths.push(uri.fsPath); - } catch (error) { - this.logService.error(`[CopilotCLIPromptResolver] Failed to store image: ${error}`); - } - })); // 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 => { + const relevantRefs = request.references.filter(ref => !ref.id.startsWith('vscode.prompt.instructions')); + + // Process all references in parallel + await Promise.all(relevantRefs.map(async ref => { if (ref.value instanceof ChatReferenceBinaryData) { - // Already handled above - return; + // Handle image attachments + try { + const buffer = await ref.value.data(); + const uri = await this.imageStorage.storeImage(buffer, ref.value.mimeType); + attachments.push({ + type: 'file', + displayName: path.basename(uri.fsPath), + path: uri.fsPath + }); + } catch (error) { + this.logService.error(`[CopilotCLIPromptResolver] Failed to store image: ${error}`); + } } else if (ref.value instanceof ChatReferenceDiagnostic) { // Handle diagnostic reference for (const [uri, diagnostics] of ref.value.diagnostics) { @@ -73,7 +71,21 @@ export class CopilotCLIPromptResolver { 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) }); + + // Add the file from diagnostic + try { + const stat = await raceCancellationError(this.fileSystemService.stat(uri), token); + const type = stat.type === FileType.Directory ? 'directory' : stat.type === FileType.File ? 'file' : undefined; + if (type) { + attachments.push({ + type, + displayName: path.basename(uri.fsPath), + path: uri.fsPath + }); + } + } catch (error) { + this.logService.error(`[CopilotCLIPromptResolver] Failed to attach ${uri.fsPath}: ${error}`); + } } } } else { @@ -82,7 +94,6 @@ export class CopilotCLIPromptResolver { return; } const filePath = uri.fsPath; - files.push({ path: filePath, name: ref.name || path.basename(filePath) }); const valueText = URI.isUri(ref.value) ? ref.value.fsPath : isLocation(ref.value) ? @@ -93,24 +104,21 @@ export class CopilotCLIPromptResolver { const variableText = request.prompt.substring(ref.range[0], ref.range[1]); allRefsTexts.push(`- ${variableText} → ${valueText}`); } - } - }); - await Promise.all(files.map(async (file) => { - try { - const stat = await raceCancellationError(this.fileSystemService.stat(URI.file(file.path)), token); - const type = stat.type === FileType.Directory ? 'directory' : stat.type === FileType.File ? 'file' : undefined; - if (!type) { - this.logService.error(`[CopilotCLIAgentManager] Ignoring attachment as its not a file/directory (${file.path})`); - return; + // Add the file reference + try { + const stat = await raceCancellationError(this.fileSystemService.stat(uri), token); + const type = stat.type === FileType.Directory ? 'directory' : stat.type === FileType.File ? 'file' : undefined; + if (type) { + attachments.push({ + type, + displayName: ref.name || path.basename(filePath), + path: filePath + }); + } + } catch (error) { + this.logService.error(`[CopilotCLIPromptResolver] Failed to attach ${filePath}: ${error}`); } - attachments.push({ - type, - displayName: file.name, - path: file.path - }); - } catch (error) { - this.logService.error(`[CopilotCLIAgentManager] Failed to attach ${file.path}: ${error}`); } })); @@ -121,15 +129,6 @@ export class CopilotCLIPromptResolver { if (diagnosticTexts.length > 0) { reminderParts.push(`The user provided the following diagnostics:\n${diagnosticTexts.join('\n')}`); } - if (imageAttachmentPaths.length > 0) { - const imageAttachmentsText = imageAttachmentPaths.map(p => `- ${p}`).join('\n'); - reminderParts.push(`The user provided the following image attachments:\n${imageAttachmentsText}`); - attachments.push(...imageAttachmentPaths.map(p => ({ - type: 'file' as const, - displayName: path.basename(p), - path: p - }))); - } let prompt = request.prompt; if (reminderParts.length > 0) { From 0e9ec7546a5b702baea435b917a80d669e063e4c Mon Sep 17 00:00:00 2001 From: Ladislau Szomoru <3372902+lszomoru@users.noreply.github.com> Date: Sat, 8 Nov 2025 13:53:18 +0000 Subject: [PATCH 04/16] CCA - provide number of files modified in the pull request (#1872) --- .../vscode-node/copilotCloudSessionsProvider.ts | 4 ++++ src/extension/vscode.proposed.chatSessionsProvider.d.ts | 5 +++++ src/platform/github/common/githubAPI.ts | 9 +++++++++ 3 files changed, 18 insertions(+) diff --git a/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts b/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts index 1df1bc2db1..d22c200080 100644 --- a/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts +++ b/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts @@ -213,11 +213,15 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C startTime: new Date(sessionItem.last_updated_at).getTime(), }, statistics: { + files: pr.files.totalCount, insertions: pr.additions, deletions: pr.deletions }, fullDatabaseId: pr.fullDatabaseId.toString(), pullRequestDetails: pr, + } satisfies vscode.ChatSessionItem & { + fullDatabaseId: string; + pullRequestDetails: PullRequestSearchItem; }; this.chatSessions.set(pr.number, pr); return session; diff --git a/src/extension/vscode.proposed.chatSessionsProvider.d.ts b/src/extension/vscode.proposed.chatSessionsProvider.d.ts index 3e7c795fbe..489e5f952c 100644 --- a/src/extension/vscode.proposed.chatSessionsProvider.d.ts +++ b/src/extension/vscode.proposed.chatSessionsProvider.d.ts @@ -123,6 +123,11 @@ declare module 'vscode' { * Statistics about the chat session. */ statistics?: { + /** + * Number of files edited during the session. + */ + files: number; + /** * Number of insertions made during the session. */ diff --git a/src/platform/github/common/githubAPI.ts b/src/platform/github/common/githubAPI.ts index 746dcb7f7a..5d86268cfb 100644 --- a/src/platform/github/common/githubAPI.ts +++ b/src/platform/github/common/githubAPI.ts @@ -26,6 +26,9 @@ export interface PullRequestSearchItem { }; additions: number; deletions: number; + files: { + totalCount: number; + }; fullDatabaseId: number; headRefOid: string; baseRefOid?: string; @@ -198,6 +201,9 @@ export async function makeSearchGraphQLRequest( updatedAt additions deletions + files { + totalCount + } author { login } @@ -255,6 +261,9 @@ export async function getPullRequestFromGlobalId( updatedAt additions deletions + files { + totalCount + } author { login } From 7b1c61c8bfb01c902373f91353f540176affb232 Mon Sep 17 00:00:00 2001 From: Ladislau Szomoru <3372902+lszomoru@users.noreply.github.com> Date: Sat, 8 Nov 2025 18:07:00 +0000 Subject: [PATCH 05/16] Revert "improve worktree creation progress (#1869)" (#1873) This reverts commit f1da07ec02f7c1013e899eec9ae05c3d6a5e0985. --- .../copilotCLIChatSessionsContribution.ts | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts index 4c7c8c9bd7..e9b10ea762 100644 --- a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts +++ b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts @@ -42,19 +42,17 @@ export class CopilotCLIWorktreeManager { return undefined; } - await stream.progress(vscode.l10n.t('Creating isolated worktree for session...'), async (progress) => { - try { - const worktreePath = await vscode.commands.executeCommand('git.createWorktreeWithDefaults') as string | undefined; - if (worktreePath) { - return vscode.l10n.t('Created isolated worktree at {0}', worktreePath); - } else { - progress.report(new vscode.ChatResponseWarningPart(vscode.l10n.t('Failed to create worktree for isolation, using default workspace directory'))); - } - } catch (error) { - progress.report(new vscode.ChatResponseWarningPart(vscode.l10n.t('Error creating worktree for isolation: {0}', error instanceof Error ? error.message : String(error)))); + try { + const worktreePath = await vscode.commands.executeCommand('git.createWorktreeWithDefaults') as string | undefined; + if (worktreePath) { + stream.progress(vscode.l10n.t('Created isolated worktree at {0}', worktreePath)); + return worktreePath; + } else { + stream.warning(vscode.l10n.t('Failed to create worktree for isolation, using default workspace directory')); } - }); - + } catch (error) { + stream.warning(vscode.l10n.t('Error creating worktree for isolation: {0}', error instanceof Error ? error.message : String(error))); + } return undefined; } From 9bcf2429f278ed5c28e54d327cde8953b7e534d5 Mon Sep 17 00:00:00 2001 From: Vijay Upadya <41652029+vijayupadya@users.noreply.github.com> Date: Sat, 8 Nov 2025 10:27:31 -0800 Subject: [PATCH 06/16] Add tooltip for pr chat items in chat sessions view (#1842) * Enable tooltip for CP chat pr items * minor update * update comment * comment update * Copilot PR feedback --------- Co-authored-by: vijay upadya --- .../copilotCloudSessionsProvider.ts | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts b/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts index d22c200080..6e22d31e5a 100644 --- a/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts +++ b/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import MarkdownIt from 'markdown-it'; import * as pathLib from 'path'; import * as vscode from 'vscode'; import { Uri } from 'vscode'; @@ -60,6 +61,72 @@ const AGENTS_OPTION_GROUP_ID = 'agents'; const DEFAULT_AGENT_ID = '___vscode_default___'; const BACKGROUND_REFRESH_INTERVAL_MS = 5 * 60 * 1000; // 5 minutes +/** + * Custom renderer for markdown-it that converts markdown to plain text + */ +class PlainTextRenderer { + private md: MarkdownIt; + + constructor() { + this.md = new MarkdownIt(); + } + + /** + * Renders markdown text as plain text by extracting text content from all tokens + */ + render(markdown: string): string { + const tokens = this.md.parse(markdown, {}); + return this.renderTokens(tokens).trim(); + } + + private renderTokens(tokens: MarkdownIt.Token[]): string { + let result = ''; + for (const token of tokens) { + // Process child tokens recursively + if (token.children) { + result += this.renderTokens(token.children); + } + + // Handle different token types + switch (token.type) { + case 'text': + case 'code_inline': + // Only add content if no children were processed + if (!token.children) { + result += token.content; + } + break; + + case 'softbreak': + case 'hardbreak': + result += ' '; // Space instead of newline to match original + break; + + case 'paragraph_close': + result += '\n'; // Newline after paragraphs for separation + break; + + case 'heading_close': + result += '\n'; // Newline after headings + break; + + case 'list_item_close': + result += '\n'; // Newline after list items + break; + + case 'fence': + case 'code_block': + case 'hr': + // Skip these entirely + break; + + // Don't add default case - only explicitly handle what we want + } + } + return result; + } +} + export class CopilotCloudSessionsProvider extends Disposable implements vscode.ChatSessionContentProvider, vscode.ChatSessionItemProvider { public static readonly TYPE = 'copilot-cloud-agent'; private readonly DELEGATE_MODAL_DETAILS = vscode.l10n.t('The agent will work asynchronously to create a pull request with your requested changes. This chat\'s history will be summarized and appended to the pull request as context.'); @@ -74,6 +141,7 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C await this.chatParticipantImpl(request, context, stream, token) ); private cachedSessionsSize: number = 0; + private readonly plainTextRenderer = new PlainTextRenderer(); constructor( @IOctoKitService private readonly _octoKitService: IOctoKitService, @@ -203,12 +271,14 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C const uri = await toOpenPullRequestWebviewUri({ owner: repoId.org, repo: repoId.repo, pullRequestNumber: pr.number }); const prLinkTitle = vscode.l10n.t('Open pull request in VS Code'); const description = new vscode.MarkdownString(`[#${pr.number}](${uri.toString()} "${prLinkTitle}")`); + const tooltip = this.createPullRequestTooltip(pr); const session = { resource: vscode.Uri.from({ scheme: CopilotCloudSessionsProvider.TYPE, path: '/' + pr.number }), label: pr.title, status: this.getSessionStatusFromSession(sessionItem), description, + tooltip, timing: { startTime: new Date(sessionItem.last_updated_at).getTime(), }, @@ -415,6 +485,46 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C } } + private createPullRequestTooltip(pr: PullRequestSearchItem): vscode.MarkdownString { + const markdown = new vscode.MarkdownString(undefined, true); + markdown.supportHtml = true; + + // Repository and date + const date = new Date(pr.createdAt); + const ownerName = `${pr.repository.owner.login}/${pr.repository.name}`; + markdown.appendMarkdown( + `[${ownerName}](https://github.com/${ownerName}) on ${date.toLocaleString('default', { + day: 'numeric', + month: 'short', + year: 'numeric', + })} \n` + ); + + // Icon, title, and PR number + const icon = this.getIconMarkdown(pr); + // Strip markdown from title for plain text display + const title = this.plainTextRenderer.render(pr.title); + markdown.appendMarkdown( + `${icon} **${title}** [#${pr.number}](${pr.url}) \n` + ); + + // Body/Description (truncated if too long) + markdown.appendMarkdown(' \n'); + const maxBodyLength = 200; + let body = this.plainTextRenderer.render(pr.body || ''); + // Convert plain text newlines to markdown line breaks (two spaces + newline) + body = body.replace(/\n/g, ' \n'); + body = body.length > maxBodyLength ? body.substring(0, maxBodyLength) + '...' : body; + markdown.appendMarkdown(body + ' \n'); + + return markdown; + } + + private getIconMarkdown(pr: PullRequestSearchItem): string { + const state = pr.state.toUpperCase(); + return state === 'MERGED' ? '$(git-merge)' : '$(git-pull-request)'; + } + private async startSession(stream: vscode.ChatResponseStream, token: vscode.CancellationToken, source: string, prompt: string, history?: string, references?: readonly vscode.ChatPromptReference[], customAgentName?: string) { /* __GDPR__ "copilot.codingAgent.editor.invoke" : { From 8e3e319934cd5292d402654e55194a2f9876cd27 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Sat, 8 Nov 2025 21:39:29 +0000 Subject: [PATCH 07/16] More tests for Copilot CLI and refactor permission handling (#1871) * More Copilot CLI tests and refactor permissions * Updates * Address review comments * Updates * Address review comments * Address review comments --- .../agents/copilotcli/node/copilotCli.ts | 72 ++-- .../copilotcli/node/copilotcliSession.ts | 126 ++++--- .../node/copilotcliSessionService.ts | 49 ++- .../copilotcli/node/permissionHelpers.ts | 17 + .../test/copilotCliSessionService.spec.ts | 17 +- .../node/test/copilotcliSession.spec.ts | 211 ++++++----- .../copilotCLIChatSessionsContribution.ts | 195 +++++----- .../copilotCloudSessionsProvider.ts | 5 +- .../copilotCLIChatSessionParticipant.spec.ts | 347 ++++++++++++++++++ 9 files changed, 761 insertions(+), 278 deletions(-) create mode 100644 src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts diff --git a/src/extension/agents/copilotcli/node/copilotCli.ts b/src/extension/agents/copilotcli/node/copilotCli.ts index 3b1af7158c..d4ed5225db 100644 --- a/src/extension/agents/copilotcli/node/copilotCli.ts +++ b/src/extension/agents/copilotcli/node/copilotCli.ts @@ -12,13 +12,19 @@ import { ILogService } from '../../../../platform/log/common/logService'; import { IWorkspaceService } from '../../../../platform/workspace/common/workspaceService'; import { createServiceIdentifier } from '../../../../util/common/services'; import { Lazy } from '../../../../util/vs/base/common/lazy'; -import { Disposable, IDisposable, toDisposable } from '../../../../util/vs/base/common/lifecycle'; +import { IDisposable, toDisposable } from '../../../../util/vs/base/common/lifecycle'; import { getCopilotLogger } from './logger'; import { ensureNodePtyShim } from './nodePtyShim'; +import { PermissionRequest } from './permissionHelpers'; const COPILOT_CLI_MODEL_MEMENTO_KEY = 'github.copilot.cli.sessionModel'; const DEFAULT_CLI_MODEL = 'claude-sonnet-4'; +export interface CopilotCLISessionOptions { + addPermissionHandler(handler: SessionOptions['requestPermission']): IDisposable; + toSessionOptions(): SessionOptions; +} + export interface ICopilotCLIModels { _serviceBrand: undefined; toModelProvider(modelId: string): string; @@ -104,9 +110,8 @@ export class CopilotCLISDK implements ICopilotCLISDK { export interface ICopilotCLISessionOptionsService { readonly _serviceBrand: undefined; createOptions( - options: SessionOptions, - permissionHandler: CopilotCLIPermissionsHandler - ): Promise; + options: SessionOptions + ): Promise; } export const ICopilotCLISessionOptionsService = createServiceIdentifier('ICopilotCLISessionOptionsService'); @@ -118,17 +123,28 @@ export class CopilotCLISessionOptionsService implements ICopilotCLISessionOption @ILogService private readonly logService: ILogService, ) { } - public async createOptions(options: SessionOptions, permissionHandler: CopilotCLIPermissionsHandler) { + public async createOptions(options: SessionOptions) { const copilotToken = await this._authenticationService.getAnyGitHubSession(); const workingDirectory = options.workingDirectory ?? await this.getWorkspaceFolderPath(); + const logger = this.logService; + const requestPermissionRejected = async (permission: PermissionRequest): ReturnType> => { + logger.info(`[CopilotCLISessionOptionsService] Permission request denied for permission as no handler was set: ${permission.kind}`); + return { + kind: "denied-interactively-by-user" + }; + }; + const permissionHandler: Required> = { + requestPermission: requestPermissionRejected + }; + const allOptions: SessionOptions = { env: { ...process.env, COPILOTCLI_DISABLE_NONESSENTIAL_TRAFFIC: '1' }, logger: getCopilotLogger(this.logService), - requestPermission: async (permissionRequest) => { - return await permissionHandler.getPermissions(permissionRequest); + requestPermission: async (request: PermissionRequest) => { + return await permissionHandler.requestPermission(request); }, authInfo: { type: 'token', @@ -141,7 +157,18 @@ export class CopilotCLISessionOptionsService implements ICopilotCLISessionOption if (workingDirectory) { allOptions.workingDirectory = workingDirectory; } - return allOptions; + + return { + addPermissionHandler: (handler: NonNullable) => { + permissionHandler.requestPermission = handler; + return toDisposable(() => { + if (permissionHandler.requestPermission === handler) { + permissionHandler.requestPermission = requestPermissionRejected; + } + }); + }, + toSessionOptions: () => allOptions + } satisfies CopilotCLISessionOptions; } private async getWorkspaceFolderPath() { if (this.workspaceService.getWorkspaceFolders().length === 0) { @@ -154,32 +181,3 @@ export class CopilotCLISessionOptionsService implements ICopilotCLISessionOption return folder?.uri?.fsPath; } } - - -/** - * Perhaps temporary interface to handle permission requests from the Copilot CLI SDK - * Perhaps because the SDK needs a better way to handle this in long term per session. - */ -export interface ICopilotCLIPermissions { - onDidRequestPermissions(handler: SessionOptions['requestPermission']): IDisposable; -} - -export class CopilotCLIPermissionsHandler extends Disposable implements ICopilotCLIPermissions { - private _handler: SessionOptions['requestPermission'] | undefined; - - public onDidRequestPermissions(handler: SessionOptions['requestPermission']): IDisposable { - this._handler = handler; - return this._register(toDisposable(() => { - this._handler = undefined; - })); - } - - public async getPermissions(permission: Parameters>[0]): Promise>> { - if (!this._handler) { - return { - kind: "denied-interactively-by-user" - }; - } - return await this._handler(permission); - } -} \ No newline at end of file diff --git a/src/extension/agents/copilotcli/node/copilotcliSession.ts b/src/extension/agents/copilotcli/node/copilotcliSession.ts index a481ba0240..d11d45633b 100644 --- a/src/extension/agents/copilotcli/node/copilotcliSession.ts +++ b/src/extension/agents/copilotcli/node/copilotcliSession.ts @@ -3,39 +3,45 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import type { Attachment, Session, SessionOptions } from '@github/copilot/sdk'; +import type { Attachment, Session } from '@github/copilot/sdk'; import type * as vscode from 'vscode'; import { ILogService } from '../../../../platform/log/common/logService'; import { IWorkspaceService } from '../../../../platform/workspace/common/workspaceService'; import { CancellationToken } from '../../../../util/vs/base/common/cancellation'; +import { Emitter, Event } from '../../../../util/vs/base/common/event'; import { DisposableStore, IDisposable, toDisposable } from '../../../../util/vs/base/common/lifecycle'; import { ResourceMap } from '../../../../util/vs/base/common/map'; import { extUriBiasedIgnorePathCase } from '../../../../util/vs/base/common/resources'; -import { ChatRequestTurn2, ChatResponseThinkingProgressPart, ChatResponseTurn2, ChatSessionStatus, EventEmitter, LanguageModelTextPart, Uri } from '../../../../vscodeTypes'; -import { IToolsService } from '../../../tools/common/toolsService'; +import { ChatRequestTurn2, ChatResponseThinkingProgressPart, ChatResponseTurn2, ChatSessionStatus, EventEmitter, Uri } from '../../../../vscodeTypes'; import { ExternalEditTracker } from '../../common/externalEditTracker'; -import { CopilotCLIPermissionsHandler, ICopilotCLISessionOptionsService } from './copilotCli'; +import { CopilotCLISessionOptions, ICopilotCLISessionOptionsService } from './copilotCli'; import { buildChatHistoryFromEvents, getAffectedUrisForEditTool, isCopilotCliEditToolCall, processToolExecutionComplete, processToolExecutionStart } from './copilotcliToolInvocationFormatter'; -import { getConfirmationToolParams, PermissionRequest } from './permissionHelpers'; +import { PermissionRequest } from './permissionHelpers'; + +type PermissionHandler = ( + permissionRequest: PermissionRequest, + token: CancellationToken, +) => Promise; export interface ICopilotCLISession extends IDisposable { readonly sessionId: string; readonly status: vscode.ChatSessionStatus | undefined; readonly onDidChangeStatus: vscode.Event; + readonly permissionRequested?: PermissionRequest; + readonly onPermissionRequested: vscode.Event; + attachPermissionHandler(handler: PermissionHandler): IDisposable; + attachStream(stream: vscode.ChatResponseStream): IDisposable; handleRequest( prompt: string, attachments: Attachment[], modelId: string | undefined, - stream: vscode.ChatResponseStream, - toolInvocationToken: vscode.ChatParticipantToolToken, token: vscode.CancellationToken ): Promise; - addUserMessage(content: string): void; addUserAssistantMessage(content: string): void; getSelectedModelId(): Promise; - getChatHistory(): Promise<(ChatRequestTurn2 | ChatResponseTurn2)[]>; + getChatHistory(): (ChatRequestTurn2 | ChatResponseTurn2)[]; } export class CopilotCLISession extends DisposableStore implements ICopilotCLISession { @@ -49,25 +55,49 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes public readonly onDidChangeStatus = this._statusChange.event; + private _permissionRequested?: PermissionRequest; + public get permissionRequested(): PermissionRequest | undefined { + return this._permissionRequested; + } + private readonly _onPermissionRequested = this.add(new EventEmitter()); + public readonly onPermissionRequested = this._onPermissionRequested.event; + private _permissionHandler?: PermissionHandler; + private readonly _permissionHandlerSet = this.add(new Emitter()); + private _stream?: vscode.ChatResponseStream; constructor( + private readonly _options: CopilotCLISessionOptions, private readonly _sdkSession: Session, - private readonly _options: SessionOptions, - private readonly _permissionHandler: CopilotCLIPermissionsHandler, @ILogService private readonly logService: ILogService, @IWorkspaceService private readonly workspaceService: IWorkspaceService, - @IToolsService private readonly toolsService: IToolsService, @ICopilotCLISessionOptionsService private readonly cliSessionOptions: ICopilotCLISessionOptionsService, ) { super(); this.sessionId = _sdkSession.sessionId; } + attachStream(stream: vscode.ChatResponseStream): IDisposable { + this._stream = stream; + return toDisposable(() => { + if (this._stream === stream) { + this._stream = undefined; + } + }); + } + + attachPermissionHandler(handler: PermissionHandler): IDisposable { + this._permissionHandler = handler; + this._permissionHandlerSet.fire(); + return toDisposable(() => { + if (this._permissionHandler === handler) { + this._permissionHandler = undefined; + } + }); + } + public async handleRequest( prompt: string, attachments: Attachment[], modelId: string | undefined, - stream: vscode.ChatResponseStream, - toolInvocationToken: vscode.ChatParticipantToolToken, token: vscode.CancellationToken ): Promise { if (this.isDisposed) { @@ -88,26 +118,25 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes const editToolIds = new Set(); const editTracker = new ExternalEditTracker(); const editFilesAndToolCallIds = new ResourceMap(); - disposables.add(this._permissionHandler.onDidRequestPermissions(async (permissionRequest) => { - return await this.requestPermission(permissionRequest, stream, editTracker, + disposables.add(this._options.addPermissionHandler(async (permissionRequest) => { + // Need better API from SDK to correlate file edits in permission requests to tool invocations. + return await this.requestPermission(permissionRequest, editTracker, (file: Uri) => { const ids = editFilesAndToolCallIds.get(file); return ids?.shift(); }, - toolInvocationToken, - this._options.workingDirectory + this._options.toSessionOptions().workingDirectory, + token ); })); try { - const [currentModel, - sessionOptions - ] = await Promise.all([ + const [currentModel, authInfo] = await Promise.all([ modelId ? this._sdkSession.getSelectedModel() : undefined, - this.cliSessionOptions.createOptions(this._options, this._permissionHandler) + this.cliSessionOptions.createOptions({}).then(opts => opts.toSessionOptions().authInfo) ]); - if (sessionOptions.authInfo) { - this._sdkSession.setAuthInfo(sessionOptions.authInfo); + if (authInfo) { + this._sdkSession.setAuthInfo(authInfo); } if (modelId && modelId !== currentModel) { await this._sdkSession.setSelectedModel(modelId); @@ -116,7 +145,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes disposables.add(toDisposable(this._sdkSession.on('*', (event) => this.logService.trace(`[CopilotCLISession]CopilotCLI Event: ${JSON.stringify(event, null, 2)}`)))); disposables.add(toDisposable(this._sdkSession.on('assistant.message', (event) => { if (typeof event.data.content === 'string' && event.data.content.length) { - stream.markdown(event.data.content); + this._stream?.markdown(event.data.content); } }))); disposables.add(toDisposable(this._sdkSession.on('tool.execution_start', (event) => { @@ -136,7 +165,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes } else { const responsePart = processToolExecutionStart(event, this._pendingToolInvocations); if (responsePart instanceof ChatResponseThinkingProgressPart) { - stream.push(responsePart); + this._stream?.push(responsePart); } } this.logService.trace(`[CopilotCLISession] Start Tool ${event.data.toolName || ''}`); @@ -151,7 +180,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes const responsePart = processToolExecutionComplete(event, this._pendingToolInvocations); if (responsePart && !(responsePart instanceof ChatResponseThinkingProgressPart)) { - stream.push(responsePart); + this._stream?.push(responsePart); } const toolName = toolNames.get(event.data.toolCallId) || ''; @@ -163,7 +192,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes }))); disposables.add(toDisposable(this._sdkSession.on('session.error', (event) => { this.logService.error(`[CopilotCLISession]CopilotCLI error: (${event.data.errorType}), ${event.data.message}`); - stream.markdown(`\n\n❌ Error: (${event.data.errorType}) ${event.data.message}`); + this._stream?.markdown(`\n\n❌ Error: (${event.data.errorType}) ${event.data.message}`); }))); await this._sdkSession.send({ prompt, attachments, abortController }); @@ -175,7 +204,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes this._status = ChatSessionStatus.Failed; this._statusChange.fire(this._status); this.logService.error(`[CopilotCLISession] Invoking session (error) ${this.sessionId}`, error); - stream.markdown(`\n\n❌ Error: ${error instanceof Error ? error.message : String(error)}`); + this._stream?.markdown(`\n\n❌ Error: ${error instanceof Error ? error.message : String(error)}`); } finally { disposables.dispose(); } @@ -196,18 +225,17 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes return this._sdkSession.getSelectedModel(); } - public async getChatHistory(): Promise<(ChatRequestTurn2 | ChatResponseTurn2)[]> { - const events = await this._sdkSession.getEvents(); + public getChatHistory(): (ChatRequestTurn2 | ChatResponseTurn2)[] { + const events = this._sdkSession.getEvents(); return buildChatHistoryFromEvents(events); } private async requestPermission( permissionRequest: PermissionRequest, - stream: vscode.ChatResponseStream, editTracker: ExternalEditTracker, getEditKeyForFile: (file: Uri) => string | undefined, - toolInvocationToken: vscode.ChatParticipantToolToken, - workingDirectory?: string + workingDirectory: string | undefined, + token: vscode.CancellationToken ): Promise<{ kind: 'approved' } | { kind: 'denied-interactively-by-user' }> { if (permissionRequest.kind === 'read') { // If user is reading a file in the working directory or workspace, auto-approve @@ -239,26 +267,40 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes } try { - const { tool, input } = getConfirmationToolParams(permissionRequest); - const result = await this.toolsService.invokeTool(tool, - { input, toolInvocationToken }, - CancellationToken.None); + const permissionHandler = await this.waitForPermissionHandler(permissionRequest); + if (!permissionHandler) { + this.logService.warn(`[CopilotCLISession] No permission handler registered, denying request for ${permissionRequest.kind} permission.`); + return { kind: 'denied-interactively-by-user' }; + } - const firstResultPart = result.content.at(0); - if (firstResultPart instanceof LanguageModelTextPart && firstResultPart.value === 'yes') { + if (await permissionHandler(permissionRequest, token)) { // If we're editing a file, start tracking the edit & wait for core to acknowledge it. const editFile = permissionRequest.kind === 'write' ? Uri.file(permissionRequest.fileName) : undefined; const editKey = editFile ? getEditKeyForFile(editFile) : undefined; - if (editFile && editKey) { + if (editFile && editKey && this._stream) { this.logService.trace(`[CopilotCLISession] Starting to track edit for toolCallId ${editKey} & file ${editFile.fsPath}`); - await editTracker.trackEdit(editKey, [editFile], stream); + await editTracker.trackEdit(editKey, [editFile], this._stream); } return { kind: 'approved' }; } } catch (error) { this.logService.error(`[CopilotCLISession] Permission request error: ${error}`); + } finally { + this._permissionRequested = undefined; } return { kind: 'denied-interactively-by-user' }; } + + private async waitForPermissionHandler(permissionRequest: PermissionRequest): Promise { + if (!this._permissionHandler) { + this._permissionRequested = permissionRequest; + this._onPermissionRequested.fire(permissionRequest); + const disposables = this.add(new DisposableStore()); + await Event.toPromise(this._permissionHandlerSet.event, disposables); + disposables.dispose(); + this._permissionRequested = undefined; + } + return this._permissionHandler; + } } diff --git a/src/extension/agents/copilotcli/node/copilotcliSessionService.ts b/src/extension/agents/copilotcli/node/copilotcliSessionService.ts index b80ae5c554..3c15f1397e 100644 --- a/src/extension/agents/copilotcli/node/copilotcliSessionService.ts +++ b/src/extension/agents/copilotcli/node/copilotcliSessionService.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import type { ModelMetadata, Session, SessionOptions, internal } from '@github/copilot/sdk'; +import type { ModelMetadata, Session, internal } from '@github/copilot/sdk'; import { ChatCompletionMessageParam } from 'openai/resources/chat/completions'; import type { CancellationToken, ChatRequest } from 'vscode'; import { INativeEnvService } from '../../../../platform/env/common/envService'; @@ -19,7 +19,7 @@ import { Disposable, DisposableMap, DisposableStore, IDisposable, toDisposable } import { joinPath } from '../../../../util/vs/base/common/resources'; import { IInstantiationService } from '../../../../util/vs/platform/instantiation/common/instantiation'; import { ChatSessionStatus } from '../../../../vscodeTypes'; -import { CopilotCLIPermissionsHandler, ICopilotCLISDK, ICopilotCLISessionOptionsService } from './copilotCli'; +import { CopilotCLISessionOptions, ICopilotCLISDK, ICopilotCLISessionOptionsService } from './copilotCli'; import { CopilotCLISession, ICopilotCLISession } from './copilotcliSession'; import { stripReminders } from './copilotcliToolInvocationFormatter'; import { getCopilotLogger } from './logger'; @@ -27,7 +27,6 @@ import { getCopilotLogger } from './logger'; export interface ICopilotCLISessionItem { readonly id: string; readonly label: string; - readonly isEmpty: boolean; readonly timestamp: Date; readonly status?: ChatSessionStatus; } @@ -131,7 +130,10 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS } const chatMessages = await raceCancellationError(session.getChatContextMessages(), token); const noUserMessages = !chatMessages.find(message => message.role === 'user'); - const label = await this._generateSessionLabel(session.sessionId, chatMessages, undefined); + if (noUserMessages) { + return undefined; + } + const label = this._generateSessionLabel(session.sessionId, chatMessages, undefined); // Get timestamp from last SDK event, or fallback to metadata.startTime const sdkEvents = session.getEvents(); @@ -148,7 +150,6 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS id: metadata.sessionId, label, timestamp, - isEmpty: noUserMessages } satisfies ICopilotCLISessionItem; } catch (error) { this.logService.warn(`Failed to load session ${metadata.sessionId}: ${error}`); @@ -161,7 +162,6 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS // Merge with cached sessions (new sessions not yet persisted by SDK) const allSessions = diskSessions .filter(session => !this._newActiveSessions.has(session.id)) - .filter(session => !session.isEmpty) .map(session => { return { ...session, @@ -187,31 +187,26 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS public async createSession(prompt: string, model: string | undefined, workingDirectory: string | undefined, token: CancellationToken): Promise { const sessionDisposables = this._register(new DisposableStore()); - const permissionHandler = sessionDisposables.add(new CopilotCLIPermissionsHandler()); try { - const options = await raceCancellationError(this.optionsService.createOptions( - { - model: model as unknown as ModelMetadata['model'], - workingDirectory - - }, permissionHandler), token); + const options = await raceCancellationError(this.optionsService.createOptions({ + model: model as unknown as ModelMetadata['model'], + workingDirectory + }), token); const sessionManager = await raceCancellationError(this.getSessionManager(), token); - const sdkSession = await sessionManager.createSession(options); + const sdkSession = await sessionManager.createSession(options.toSessionOptions()); const chatMessages = await sdkSession.getChatContextMessages(); - const noUserMessages = !chatMessages.find(message => message.role === 'user'); - const label = this._generateSessionLabel(sdkSession.sessionId, chatMessages as any, prompt); + const label = this._generateSessionLabel(sdkSession.sessionId, chatMessages, prompt); const newSession: ICopilotCLISessionItem = { id: sdkSession.sessionId, label, - timestamp: sdkSession.startTime, - isEmpty: noUserMessages + timestamp: sdkSession.startTime }; this._newActiveSessions.set(sdkSession.sessionId, newSession); this.logService.trace(`[CopilotCLIAgentManager] Created new CopilotCLI session ${sdkSession.sessionId}.`); sessionDisposables.add(toDisposable(() => this._newActiveSessions.delete(sdkSession.sessionId))); - const session = await this.createCopilotSession(sdkSession, options, sessionManager, permissionHandler, sessionDisposables); + const session = await this.createCopilotSession(sdkSession, options, sessionManager, sessionDisposables); sessionDisposables.add(session.onDidChangeStatus(() => { // This will get swapped out as soon as the session has completed. @@ -237,27 +232,26 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS const sessionDisposables = this._register(new DisposableStore()); try { const sessionManager = await raceCancellationError(this.getSessionManager(), token); - const permissionHandler = sessionDisposables.add(new CopilotCLIPermissionsHandler()); const options = await raceCancellationError(this.optionsService.createOptions({ model: model as unknown as ModelMetadata['model'], workingDirectory - }, permissionHandler), token); + }), token); - const sdkSession = await sessionManager.getSession({ ...options, sessionId }, !readonly); + const sdkSession = await sessionManager.getSession({ ...options.toSessionOptions(), sessionId }, !readonly); if (!sdkSession) { this.logService.error(`[CopilotCLIAgentManager] CopilotCLI failed to get session ${sessionId}.`); sessionDisposables.dispose(); return undefined; } - return this.createCopilotSession(sdkSession, options, sessionManager, permissionHandler, sessionDisposables); + return this.createCopilotSession(sdkSession, options, sessionManager, sessionDisposables); } catch (error) { sessionDisposables.dispose(); throw error; } } - private async createCopilotSession(sdkSession: Session, options: SessionOptions, sessionManager: internal.CLISessionManager, permissionHandler: CopilotCLIPermissionsHandler, disposables: IDisposable,): Promise { + private async createCopilotSession(sdkSession: Session, options: CopilotCLISessionOptions, sessionManager: internal.CLISessionManager, disposables: IDisposable,): Promise { const sessionDisposables = this._register(new DisposableStore()); sessionDisposables.add(disposables); try { @@ -267,7 +261,7 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS void sessionManager.closeSession(sdkSession.sessionId); })); - const session = this.instantiationService.createInstance(CopilotCLISession, sdkSession, options, permissionHandler); + const session = this.instantiationService.createInstance(CopilotCLISession, options, sdkSession); session.add(sessionDisposables); session.add(session.onDidChangeStatus(() => this._onDidChangeSessions.fire())); @@ -277,7 +271,10 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS // When vscode shuts the sessions will be disposed anyway. // This code is to avoid leaving these sessions alive forever in memory. session.add(session.onDidChangeStatus(e => { - if (session.status === undefined || session.status === ChatSessionStatus.Completed || session.status === ChatSessionStatus.Failed) { + // If we're waiting for a permission, then do not start the timeout. + if (session.permissionRequested) { + this.sessionTerminators.deleteAndDispose(session.sessionId); + } else if (session.status === undefined || session.status === ChatSessionStatus.Completed || session.status === ChatSessionStatus.Failed) { // We're done with this session, start timeout to dispose it this.sessionTerminators.set(session.sessionId, disposableTimeout(() => { session.dispose(); diff --git a/src/extension/agents/copilotcli/node/permissionHelpers.ts b/src/extension/agents/copilotcli/node/permissionHelpers.ts index f0327097c2..92437471e4 100644 --- a/src/extension/agents/copilotcli/node/permissionHelpers.ts +++ b/src/extension/agents/copilotcli/node/permissionHelpers.ts @@ -4,7 +4,10 @@ *--------------------------------------------------------------------------------------------*/ import type { SessionOptions } from '@github/copilot/sdk'; +import type { CancellationToken, ChatParticipantToolToken } from 'vscode'; +import { LanguageModelTextPart } from '../../../../vscodeTypes'; import { ToolName } from '../../../tools/common/toolNames'; +import { IToolsService } from '../../../tools/common/toolsService'; type CoreTerminalConfirmationToolParams = { tool: ToolName.CoreTerminalConfirmationTool; @@ -24,6 +27,20 @@ type CoreConfirmationToolParams = { }; } +export async function requestPermission( + permissionRequest: PermissionRequest, + toolsService: IToolsService, + toolInvocationToken: ChatParticipantToolToken, + token: CancellationToken, +): Promise { + + const { tool, input } = getConfirmationToolParams(permissionRequest); + const result = await toolsService.invokeTool(tool, { input, toolInvocationToken }, token); + + const firstResultPart = result.content.at(0); + return (firstResultPart instanceof LanguageModelTextPart && firstResultPart.value === 'yes'); +} + /** * Pure function mapping a Copilot CLI permission request -> tool invocation params. * Keeps logic out of session class for easier unit testing. diff --git a/src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts b/src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts index dae3e93633..63f6ec2bd1 100644 --- a/src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts +++ b/src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts @@ -81,7 +81,12 @@ describe('CopilotCLISessionService', () => { vi.useRealTimers(); optionsService = { _serviceBrand: undefined, - createOptions: vi.fn(async (opts: any) => opts), + createOptions: vi.fn(async (opts: any) => { + return { + addPermissionHandler: () => ({ dispose() { /* noop */ } }), + toSessionOptions: () => ({ ...opts, requestPermission: async () => ({ kind: 'denied-interactively-by-user' }) }) + }; + }), }; const sdk = { getPackage: vi.fn(async () => ({ internal: { CLISessionManager: FakeCLISessionManager } })) @@ -92,18 +97,20 @@ describe('CopilotCLISessionService', () => { const accessor = services.createTestingAccessor(); logService = accessor.get(ILogService); instantiationService = { - createInstance: (_: unknown, { sessionId }: { sessionId: string }) => { + createInstance: (_ctor: unknown, _options: any, sdkSession: { sessionId: string }) => { const disposables = new DisposableStore(); const _onDidChangeStatus = disposables.add(new EventEmitter()); const cliSession: (ICopilotCLISession & DisposableStore) = { - sessionId, + sessionId: sdkSession.sessionId, status: undefined, onDidChangeStatus: _onDidChangeStatus.event, + permissionRequested: undefined, handleRequest: vi.fn(async () => { }), addUserMessage: vi.fn(), addUserAssistantMessage: vi.fn(), getSelectedModelId: vi.fn(async () => 'gpt-test'), getChatHistory: vi.fn(async () => []), + attachPermissionHandler: vi.fn(() => ({ dispose() { } })), get isDisposed() { return disposables.isDisposed; }, dispose: () => { disposables.dispose(); }, add: (d: IDisposable) => disposables.add(d) @@ -131,7 +138,7 @@ describe('CopilotCLISessionService', () => { describe('CopilotCLISessionService.createSession', () => { it('get session will return the same session created using createSession', async () => { const session = await service.createSession(' ', 'gpt-test', '/tmp', createToken().token); - expect(optionsService.createOptions).toHaveBeenCalledWith({ model: 'gpt-test', workingDirectory: '/tmp' }, expect.anything()); + expect(optionsService.createOptions).toHaveBeenCalledWith({ model: 'gpt-test', workingDirectory: '/tmp' }); const existingSession = await service.getSession(session.sessionId, undefined, undefined, false, createToken().token); @@ -139,7 +146,7 @@ describe('CopilotCLISessionService', () => { }); it('get session will return new once previous session is disposed', async () => { const session = await service.createSession(' ', 'gpt-test', '/tmp', createToken().token); - expect(optionsService.createOptions).toHaveBeenCalledWith({ model: 'gpt-test', workingDirectory: '/tmp' }, expect.anything()); + expect(optionsService.createOptions).toHaveBeenCalledWith({ model: 'gpt-test', workingDirectory: '/tmp' }); session.dispose(); await new Promise(resolve => setTimeout(resolve, 0)); // allow dispose async cleanup to run diff --git a/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts b/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts index 01a1fe933f..e27bd354cf 100644 --- a/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts +++ b/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts @@ -5,21 +5,20 @@ import type { Session, SessionOptions } from '@github/copilot/sdk'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import type { ChatParticipantToolToken, LanguageModelToolInvocationOptions, LanguageModelToolResult2 } from 'vscode'; import { ILogService } from '../../../../../platform/log/common/logService'; import { TestWorkspaceService } from '../../../../../platform/test/node/testWorkspaceService'; import { IWorkspaceService } from '../../../../../platform/workspace/common/workspaceService'; 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 { ChatSessionStatus, LanguageModelTextPart, Uri } from '../../../../../vscodeTypes'; +import { ChatSessionStatus, Uri } from '../../../../../vscodeTypes'; import { createExtensionUnitTestingServices } from '../../../../test/node/services'; import { MockChatResponseStream } from '../../../../test/node/testHelpers'; -import { IToolsService, NullToolsService } from '../../../../tools/common/toolsService'; import { ExternalEditTracker } from '../../../common/externalEditTracker'; -import { CopilotCLIPermissionsHandler, ICopilotCLISessionOptionsService } from '../copilotCli'; +import { CopilotCLISessionOptions, ICopilotCLISessionOptionsService } from '../copilotCli'; import { CopilotCLISession } from '../copilotcliSession'; import { CopilotCLIToolNames } from '../copilotcliToolInvocationFormatter'; +import { PermissionRequest } from '../permissionHelpers'; // Minimal shapes for types coming from the Copilot SDK we interact with interface MockSdkEventHandler { (payload: unknown): void } @@ -29,7 +28,7 @@ class MockSdkSession { onHandlers: MockSdkEventMap = new Map(); public sessionId = 'mock-session-id'; public _selectedModel: string | undefined = 'modelA'; - public authInfo: any; + public authInfo: unknown; on(event: string, handler: MockSdkEventHandler) { if (!this.onHandlers.has(event)) { @@ -39,7 +38,7 @@ class MockSdkSession { return () => this.onHandlers.get(event)!.delete(handler); } - emit(event: string, data: any) { + emit(event: string, data: unknown) { this.onHandlers.get(event)?.forEach(h => h({ data })); } @@ -58,19 +57,31 @@ class MockSdkSession { // Mocks for services function createSessionOptionsService() { - const auth: Partial = { - createOptions: async () => { - return { - authInfo: { - token: 'copilot-token', - tokenType: 'test', - expiresAt: Date.now() + 60_000, - copilotPlan: 'pro' + const svc: Partial = { + createOptions: async (_options: SessionOptions) => { + let permissionHandler: SessionOptions['requestPermission'] | undefined; + const requestPermission: SessionOptions['requestPermission'] = async (req) => { + if (!permissionHandler) { + return { kind: 'denied-interactively-by-user' } as const; } - } as unknown as SessionOptions; + return await permissionHandler(req); + }; + const allOptions: SessionOptions = { + env: {}, + logger: { trace() { }, error() { }, info() { }, warn() { } } as unknown as SessionOptions['logger'], + authInfo: { type: 'token', token: 'copilot-token', host: 'https://github.com' }, + requestPermission + }; + return { + addPermissionHandler: (h: SessionOptions['requestPermission']) => { + permissionHandler = h; + return { dispose: () => { permissionHandler = undefined; } }; + }, + toSessionOptions: () => allOptions + } satisfies CopilotCLISessionOptions; } }; - return auth as ICopilotCLISessionOptionsService; + return svc as ICopilotCLISessionOptionsService; } function createWorkspaceService(root: string): IWorkspaceService { @@ -86,40 +97,25 @@ function createWorkspaceService(root: string): IWorkspaceService { } }; } -function createToolsService(invocationBehavior: { approve: boolean; throws?: boolean } | undefined, logger: ILogService,): IToolsService { - return new class extends NullToolsService { - override invokeTool = vi.fn(async (_tool: string, _options: LanguageModelToolInvocationOptions, _token: CancellationToken): Promise => { - if (invocationBehavior?.throws) { - throw new Error('tool failed'); - } - return { - content: [new LanguageModelTextPart(invocationBehavior?.approve ? 'yes' : 'no')] - }; - }); - }(logger); -} describe('CopilotCLISession', () => { - const invocationToken: ChatParticipantToolToken = {} as never; const disposables = new DisposableStore(); let sdkSession: MockSdkSession; - let permissionHandler: CopilotCLIPermissionsHandler; let workspaceService: IWorkspaceService; - let toolsService: IToolsService; let logger: ILogService; let sessionOptionsService: ICopilotCLISessionOptionsService; + let sessionOptions: CopilotCLISessionOptions; - beforeEach(() => { + beforeEach(async () => { const services = disposables.add(createExtensionUnitTestingServices()); const accessor = services.createTestingAccessor(); logger = accessor.get(ILogService); sdkSession = new MockSdkSession(); - permissionHandler = new CopilotCLIPermissionsHandler(); sessionOptionsService = createSessionOptionsService(); + sessionOptions = await sessionOptionsService.createOptions({} as SessionOptions); workspaceService = createWorkspaceService('/workspace'); - toolsService = createToolsService({ approve: true }, logger); }); afterEach(() => { @@ -128,23 +124,23 @@ describe('CopilotCLISession', () => { }); - function createSession() { + async function createSession() { return disposables.add(new CopilotCLISession( + sessionOptions, sdkSession as unknown as Session, - {} as unknown as SessionOptions, - permissionHandler, logger, workspaceService, - toolsService, sessionOptionsService, )); } it('handles a successful request and streams assistant output', async () => { - const session = createSession(); + const session = await createSession(); const stream = new MockChatResponseStream(); - await session.handleRequest('Hello', [], undefined, stream, invocationToken, CancellationToken.None); + // Attach stream first, then invoke with new signature (no stream param) + session.attachStream(stream); + await session.handleRequest('Hello', [], undefined, CancellationToken.None); expect(session.status).toBe(ChatSessionStatus.Completed); expect(stream.output.join('\n')).toContain('Echo: Hello'); @@ -152,10 +148,10 @@ describe('CopilotCLISession', () => { }); it('switches model when different modelId provided', async () => { - const session = createSession(); + const session = await createSession(); const stream = new MockChatResponseStream(); - - await session.handleRequest('Hi', [], 'modelB', stream, invocationToken, CancellationToken.None); + session.attachStream(stream); + await session.handleRequest('Hi', [], 'modelB', CancellationToken.None); expect(sdkSession._selectedModel).toBe('modelB'); }); @@ -163,22 +159,22 @@ describe('CopilotCLISession', () => { it('fails request when underlying send throws', async () => { // Force send to throw sdkSession.send = async () => { throw new Error('network'); }; - const session = createSession(); + const session = await createSession(); const stream = new MockChatResponseStream(); - - await session.handleRequest('Boom', [], undefined, stream, invocationToken, CancellationToken.None); + session.attachStream(stream); + await session.handleRequest('Boom', [], undefined, CancellationToken.None); expect(session.status).toBe(ChatSessionStatus.Failed); expect(stream.output.join('\n')).toContain('Error: network'); }); it('emits status events on successful request', async () => { - const session = createSession(); + const session = await createSession(); const statuses: (ChatSessionStatus | undefined)[] = []; const listener = disposables.add(session.onDidChangeStatus(s => statuses.push(s))); const stream = new MockChatResponseStream(); - - await session.handleRequest('Status OK', [], 'modelA', stream, invocationToken, CancellationToken.None); + session.attachStream(stream); + await session.handleRequest('Status OK', [], 'modelA', CancellationToken.None); listener.dispose?.(); expect(statuses).toEqual([ChatSessionStatus.InProgress, ChatSessionStatus.Completed]); @@ -188,12 +184,12 @@ describe('CopilotCLISession', () => { it('emits status events on failed request', async () => { // Force failure sdkSession.send = async () => { throw new Error('boom'); }; - const session = createSession(); + const session = await createSession(); const statuses: (ChatSessionStatus | undefined)[] = []; const listener = disposables.add(session.onDidChangeStatus(s => statuses.push(s))); const stream = new MockChatResponseStream(); - - await session.handleRequest('Will Fail', [], undefined, stream, invocationToken, CancellationToken.None); + session.attachStream(stream); + await session.handleRequest('Will Fail', [], undefined, CancellationToken.None); listener.dispose?.(); expect(statuses).toEqual([ChatSessionStatus.InProgress, ChatSessionStatus.Failed]); @@ -201,48 +197,100 @@ describe('CopilotCLISession', () => { expect(stream.output.join('\n')).toContain('Error: boom'); }); - it('auto-approves read permission inside workspace without invoking tool', async () => { + it('auto-approves read permission inside workspace without external handler', async () => { + // Keep session active while requesting permission + let resolveSend: () => void; + sdkSession.send = async ({ prompt }: any) => new Promise(r => { resolveSend = r; }).then(() => { + sdkSession.emit('assistant.turn_start', {}); + sdkSession.emit('assistant.message', { content: `Echo: ${prompt}` }); + sdkSession.emit('assistant.turn_end', {}); + }); + const session = await createSession(); + const stream = new MockChatResponseStream(); + session.attachStream(stream); + const handlePromise = session.handleRequest('Test', [], undefined, CancellationToken.None); + + // Path must be absolute within workspace, should auto-approve + const result = await sessionOptions.toSessionOptions().requestPermission!({ kind: 'read', path: path.join('/workspace', 'file.ts'), intention: 'Read file' }); + resolveSend!(); + await handlePromise; + expect(result).toEqual({ kind: 'approved' }); + }); + + it('auto-approves read permission inside working directory without external handler', async () => { // Keep session active while requesting permission let resolveSend: () => void; + sessionOptions.toSessionOptions().workingDirectory = '/workingDirectory'; sdkSession.send = async ({ prompt }: any) => new Promise(r => { resolveSend = r; }).then(() => { sdkSession.emit('assistant.turn_start', {}); sdkSession.emit('assistant.message', { content: `Echo: ${prompt}` }); sdkSession.emit('assistant.turn_end', {}); }); - const session = createSession(); + const session = await createSession(); const stream = new MockChatResponseStream(); - const handlePromise = session.handleRequest('Test', [], undefined, stream, invocationToken, CancellationToken.None); + session.attachStream(stream); + const handlePromise = session.handleRequest('Test', [], undefined, CancellationToken.None); - // Path must be absolute within workspace - const result = await permissionHandler.getPermissions({ kind: 'read', path: path.join('/workspace', 'file.ts'), intention: 'Read file' }); + // Path must be absolute within workspace, should auto-approve + const result = await sessionOptions.toSessionOptions().requestPermission!({ kind: 'read', path: path.join('/workingDirectory', 'file.ts'), intention: 'Read file' }); resolveSend!(); await handlePromise; expect(result).toEqual({ kind: 'approved' }); - expect(toolsService.invokeTool).not.toHaveBeenCalled(); }); - it('prompts for write permission and approves when tool returns yes', async () => { - toolsService = createToolsService({ approve: true }, logger); - const session = createSession(); + it('requires read permission outside workspace and working directory', async () => { + // Keep session active while requesting permission let resolveSend: () => void; + let askedForPermission: PermissionRequest | undefined = undefined; sdkSession.send = async ({ prompt }: any) => new Promise(r => { resolveSend = r; }).then(() => { sdkSession.emit('assistant.turn_start', {}); sdkSession.emit('assistant.message', { content: `Echo: ${prompt}` }); sdkSession.emit('assistant.turn_end', {}); }); + const session = await createSession(); const stream = new MockChatResponseStream(); - const handlePromise = session.handleRequest('Write', [], undefined, stream, invocationToken, CancellationToken.None); + session.attachStream(stream); - const result = await permissionHandler.getPermissions({ kind: 'write', fileName: 'a.ts', intention: 'Update file', diff: '' }); + disposables.add(session.attachPermissionHandler((permission) => { + askedForPermission = permission; + return Promise.resolve(false); + })); + const handlePromise = session.handleRequest('Test', [], undefined, CancellationToken.None); + + // Path must be absolute within workspace, should auto-approve + const file = path.join('/workingDirectory', 'file.ts'); + const result = await sessionOptions.toSessionOptions().requestPermission!({ kind: 'read', path: file, intention: 'Read file' }); + resolveSend!(); + await handlePromise; + expect(result).toEqual({ kind: 'denied-interactively-by-user' }); + expect(askedForPermission).not.toBeUndefined(); + expect(askedForPermission!.kind).toBe('read'); + expect((askedForPermission as unknown as { path: string })!.path).toBe(file); + }); + + it('approves write permission when handler returns true', async () => { + const session = await createSession(); + // Register approval handler + disposables.add(session.attachPermissionHandler(async () => true)); + let resolveSend: () => void; + sdkSession.send = async ({ prompt }: any) => new Promise(r => { resolveSend = r; }).then(() => { + sdkSession.emit('assistant.turn_start', {}); + sdkSession.emit('assistant.message', { content: `Echo: ${prompt}` }); + sdkSession.emit('assistant.turn_end', {}); + }); + const stream = new MockChatResponseStream(); + session.attachStream(stream); + const handlePromise = session.handleRequest('Write', [], undefined, CancellationToken.None); + + const result = await sessionOptions.toSessionOptions().requestPermission!({ kind: 'write', fileName: 'a.ts', intention: 'Update file', diff: '' }); resolveSend!(); await handlePromise; - expect(toolsService.invokeTool).toHaveBeenCalled(); expect(result).toEqual({ kind: 'approved' }); }); - it('denies write permission when tool returns no', async () => { - toolsService = createToolsService({ approve: false }, logger); - const session = createSession(); + it('denies write permission when handler returns false', async () => { + const session = await createSession(); + session.attachPermissionHandler(async () => false); let resolveSend: () => void; sdkSession.send = async ({ prompt }: any) => new Promise(r => { resolveSend = r; }).then(() => { sdkSession.emit('assistant.turn_start', {}); @@ -250,18 +298,18 @@ describe('CopilotCLISession', () => { sdkSession.emit('assistant.turn_end', {}); }); const stream = new MockChatResponseStream(); - const handlePromise = session.handleRequest('Write', [], undefined, stream, invocationToken, CancellationToken.None); + session.attachStream(stream); + const handlePromise = session.handleRequest('Write', [], undefined, CancellationToken.None); - const result = await permissionHandler.getPermissions({ kind: 'write', fileName: 'b.ts', intention: 'Update file', diff: '' }); + const result = await sessionOptions.toSessionOptions().requestPermission!({ kind: 'write', fileName: 'b.ts', intention: 'Update file', diff: '' }); resolveSend!(); await handlePromise; - expect(toolsService.invokeTool).toHaveBeenCalled(); expect(result).toEqual({ kind: 'denied-interactively-by-user' }); }); - it('denies permission when tool invocation throws', async () => { - toolsService = createToolsService({ approve: true, throws: true }, logger); - const session = createSession(); + it('denies write permission when handler throws', async () => { + const session = await createSession(); + session.attachPermissionHandler(async () => { throw new Error('oops'); }); let resolveSend: () => void; sdkSession.send = async ({ prompt }: any) => new Promise(r => { resolveSend = r; }).then(() => { sdkSession.emit('assistant.turn_start', {}); @@ -269,12 +317,12 @@ describe('CopilotCLISession', () => { sdkSession.emit('assistant.turn_end', {}); }); const stream = new MockChatResponseStream(); - const handlePromise = session.handleRequest('Write', [], undefined, stream, invocationToken, CancellationToken.None); + session.attachStream(stream); + const handlePromise = session.handleRequest('Write', [], undefined, CancellationToken.None); - const result = await permissionHandler.getPermissions({ kind: 'write', fileName: 'err.ts', intention: 'Update file', diff: '' }); + const result = await sessionOptions.toSessionOptions().requestPermission!({ kind: 'write', fileName: 'err.ts', intention: 'Update file', diff: '' }); resolveSend!(); await handlePromise; - expect(toolsService.invokeTool).toHaveBeenCalled(); expect(result).toEqual({ kind: 'denied-interactively-by-user' }); }); @@ -282,11 +330,10 @@ describe('CopilotCLISession', () => { // Arrange a deferred send so we can emit tool events before request finishes let resolveSend: () => void; sdkSession.send = async () => new Promise(r => { resolveSend = r; }); - // Use approval for write permissions - toolsService = createToolsService({ approve: true }, logger); - const session = createSession(); + const session = await createSession(); + session.attachPermissionHandler(async () => true); const stream = new MockChatResponseStream(); - + session.attachStream(stream); // Spy on trackEdit to capture ordering (we don't want to depend on externalEdit mechanics here) const trackedOrder: string[] = []; const trackSpy = vi.spyOn(ExternalEditTracker.prototype, 'trackEdit').mockImplementation(async function (this: any, editKey: string) { @@ -296,7 +343,7 @@ describe('CopilotCLISession', () => { }); // Act: start handling request (do not await yet) - const requestPromise = session.handleRequest('Edits', [], undefined, stream, invocationToken, CancellationToken.None); + const requestPromise = session.handleRequest('Edits', [], undefined, CancellationToken.None); // Wait a tick to ensure event listeners are registered inside handleRequest await new Promise(r => setTimeout(r, 0)); @@ -315,7 +362,7 @@ describe('CopilotCLISession', () => { const permissionResults: any[] = []; for (let i = 1; i <= 10; i++) { // Each permission request should dequeue the next toolCallId for the file - const result = await permissionHandler.getPermissions({ + const result = await sessionOptions.toSessionOptions().requestPermission!({ kind: 'write', fileName: filePath, intention: 'Apply edit', diff --git a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts index e9b10ea762..754c61231d 100644 --- a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts +++ b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts @@ -5,6 +5,7 @@ import * as vscode from 'vscode'; import { ChatExtendedRequestHandler, l10n, Uri } from 'vscode'; +import { IRunCommandExecutionService } from '../../../platform/commands/common/runCommandExecutionService'; import { ConfigKey, IConfigurationService } from '../../../platform/configuration/common/configurationService'; import { IVSCodeExtensionContext } from '../../../platform/extContext/common/extensionContext'; import { IGitService } from '../../../platform/git/common/gitService'; @@ -15,10 +16,12 @@ import { localize } from '../../../util/vs/nls'; import { ICopilotCLIModels } from '../../agents/copilotcli/node/copilotCli'; import { ICopilotCLISession } from '../../agents/copilotcli/node/copilotcliSession'; import { ICopilotCLISessionService } from '../../agents/copilotcli/node/copilotcliSessionService'; +import { PermissionRequest, requestPermission } from '../../agents/copilotcli/node/permissionHelpers'; import { ChatSummarizerProvider } from '../../prompt/node/summarizer'; import { CopilotCLIPromptResolver } from './copilotCLIPromptResolver'; +import { IToolsService } from '../../tools/common/toolsService'; import { ICopilotCLITerminalIntegration } from './copilotCLITerminalIntegration'; -import { ConfirmationResult, CopilotCloudSessionsProvider } from './copilotCloudSessionsProvider'; +import { ConfirmationResult, CopilotCloudSessionsProvider, UncommittedChangesStep } from './copilotCloudSessionsProvider'; const MODELS_OPTION_ID = 'model'; const ISOLATION_OPTION_ID = 'isolation'; @@ -34,7 +37,8 @@ export class CopilotCLIWorktreeManager { private _sessionIsolation: Map = new Map(); private _sessionWorktrees: Map = new Map(); constructor( - @IVSCodeExtensionContext private readonly extensionContext: IVSCodeExtensionContext) { } + @IVSCodeExtensionContext private readonly extensionContext: IVSCodeExtensionContext, + @IRunCommandExecutionService private readonly commandExecutionService: IRunCommandExecutionService) { } async createWorktreeIfNeeded(sessionId: string, stream: vscode.ChatResponseStream): Promise { const isolationEnabled = this._sessionIsolation.get(sessionId) ?? false; @@ -43,7 +47,7 @@ export class CopilotCLIWorktreeManager { } try { - const worktreePath = await vscode.commands.executeCommand('git.createWorktreeWithDefaults') as string | undefined; + const worktreePath = await this.commandExecutionService.executeCommand('git.createWorktreeWithDefaults') as string | undefined; if (worktreePath) { stream.progress(vscode.l10n.t('Created isolated worktree at {0}', worktreePath)); return worktreePath; @@ -136,6 +140,7 @@ export class CopilotCLIChatSessionItemProvider extends Disposable implements vsc private readonly worktreeManager: CopilotCLIWorktreeManager, @ICopilotCLISessionService private readonly copilotcliSessionService: ICopilotCLISessionService, @ICopilotCLITerminalIntegration private readonly terminalIntegration: ICopilotCLITerminalIntegration, + @IRunCommandExecutionService private readonly commandExecutionService: IRunCommandExecutionService ) { super(); this._register(this.terminalIntegration); @@ -157,7 +162,7 @@ export class CopilotCLIChatSessionItemProvider extends Disposable implements vsc const diskSessions = sessions.map(session => this._toChatSessionItem(session)); const count = diskSessions.length; - vscode.commands.executeCommand('setContext', 'github.copilot.chat.cliSessionsEmpty', count === 0); + this.commandExecutionService.executeCommand('setContext', 'github.copilot.chat.cliSessionsEmpty', count === 0); return diskSessions; } @@ -229,7 +234,7 @@ export class CopilotCLIChatSessionContentProvider implements vscode.ChatSessionC const isolationEnabled = this.worktreeManager.getIsolationPreference(copilotcliSessionId); options[ISOLATION_OPTION_ID] = isolationEnabled ? 'enabled' : 'disabled'; } - const history = await existingSession?.getChatHistory() || []; + const history = existingSession?.getChatHistory() || []; if (!_sessionModel.get(copilotcliSessionId)) { _sessionModel.set(copilotcliSessionId, selectedModel ?? preferredModel); @@ -300,6 +305,8 @@ export class CopilotCLIChatSessionParticipant { @ICopilotCLIModels private readonly copilotCLIModels: ICopilotCLIModels, @ICopilotCLISessionService private readonly sessionService: ICopilotCLISessionService, @ITelemetryService private readonly telemetryService: ITelemetryService, + @IToolsService private readonly toolsService: IToolsService, + @IRunCommandExecutionService private readonly commandExecutionService: IRunCommandExecutionService, ) { } createHandler(): ChatExtendedRequestHandler { @@ -308,80 +315,102 @@ export class CopilotCLIChatSessionParticipant { private async handleRequest(request: vscode.ChatRequest, context: vscode.ChatContext, stream: vscode.ChatResponseStream, token: vscode.CancellationToken): Promise { const { chatSessionContext } = context; + const disposables = new DisposableStore(); + try { - /* __GDPR__ - "copilotcli.chat.invoke" : { - "owner": "joshspicer", - "comment": "Event sent when a CopilotCLI chat request is made.", - "hasChatSessionItem": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "comment": "Invoked with a chat session item." }, - "isUntitled": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "comment": "Indicates if the chat session is untitled." }, - "hasDelegatePrompt": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "comment": "Indicates if the prompt is a /delegate command." } + /* __GDPR__ + "copilotcli.chat.invoke" : { + "owner": "joshspicer", + "comment": "Event sent when a CopilotCLI chat request is made.", + "hasChatSessionItem": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "comment": "Invoked with a chat session item." }, + "isUntitled": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "comment": "Indicates if the chat session is untitled." }, + "hasDelegatePrompt": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "comment": "Indicates if the prompt is a /delegate command." } + } + */ + this.telemetryService.sendMSFTTelemetryEvent('copilotcli.chat.invoke', { + hasChatSessionItem: String(!!chatSessionContext?.chatSessionItem), + isUntitled: String(chatSessionContext?.isUntitled), + hasDelegatePrompt: String(request.prompt.startsWith('/delegate')) + }); + + const confirmationResults = this.getAcceptedRejectedConfirmationData(request); + if (!chatSessionContext) { + if (confirmationResults.length) { + stream.warning(vscode.l10n.t('No chat session context available for confirmation data handling.')); + return {}; + } + /* Invoked from a 'normal' chat or 'cloud button' without CLI session context */ + // Handle confirmation data + return await this.handlePushConfirmationData(request, context, token); } - */ - this.telemetryService.sendMSFTTelemetryEvent('copilotcli.chat.invoke', { - hasChatSessionItem: String(!!chatSessionContext?.chatSessionItem), - isUntitled: String(chatSessionContext?.isUntitled), - hasDelegatePrompt: String(request.prompt.startsWith('/delegate')) - }); - if (!chatSessionContext) { - if (request.acceptedConfirmationData || request.rejectedConfirmationData) { - stream.warning(vscode.l10n.t('No chat session context available for confirmation data handling.')); + const isUntitled = chatSessionContext.isUntitled; + const { resource } = chatSessionContext.chatSessionItem; + const id = SessionIdForCLI.parse(resource); + const [{ prompt, attachments }, modelId] = await Promise.all([ + this.promptResolver.resolvePrompt(request, token), + this.getModelId(id) + ]); + + const session = await this.getOrCreateSession(request, chatSessionContext, prompt, modelId, stream, disposables, token); + if (!session) { return {}; } - /* Invoked from a 'normal' chat or 'cloud button' without CLI session context */ - // Handle confirmation data - return await this.handlePushConfirmationData(request, context, stream, token); - } - - const defaultModel = await this.copilotCLIModels.getDefaultModel(); - const { resource } = chatSessionContext.chatSessionItem; - const id = SessionIdForCLI.parse(resource); - const preferredModel = _sessionModel.get(id); - // For existing sessions we cannot fall back, as the model info would be updated in _sessionModel - const modelId = this.copilotCLIModels.toModelProvider(preferredModel?.id || defaultModel.id); - const { prompt, attachments } = await this.promptResolver.resolvePrompt(request, token); + if (!isUntitled && confirmationResults.length) { + return await this.handleConfirmationData(session, request.prompt, confirmationResults, context, stream, token); + } - if (chatSessionContext.isUntitled) { - const workingDirectory = await this.worktreeManager.createWorktreeIfNeeded(id, stream); - const session = await this.sessionService.createSession(prompt, modelId, workingDirectory, token); - if (workingDirectory) { - await this.worktreeManager.storeWorktreePath(session.sessionId, workingDirectory); + if (!isUntitled && request.prompt.startsWith('/delegate')) { + await this.handleDelegateCommand(session, request, context, stream, token); + } else { + await session.handleRequest(prompt, attachments, modelId, token); } - await session.handleRequest(prompt, attachments, modelId, stream, request.toolInvocationToken, token); + if (isUntitled) { + this.sessionItemProvider.swap(chatSessionContext.chatSessionItem, { resource: SessionIdForCLI.getResource(session.sessionId), label: request.prompt ?? 'CopilotCLI' }); + } + return {}; + } + finally { + disposables.dispose(); + } + } - this.sessionItemProvider.swap(chatSessionContext.chatSessionItem, { resource: SessionIdForCLI.getResource(session.sessionId), label: request.prompt ?? 'CopilotCLI' }); + private async getOrCreateSession(request: vscode.ChatRequest, chatSessionContext: vscode.ChatSessionContext, prompt: string, modelId: string | undefined, stream: vscode.ChatResponseStream, disposables: DisposableStore, token: vscode.CancellationToken): Promise { + const { resource } = chatSessionContext.chatSessionItem; + const id = SessionIdForCLI.parse(resource); + const workingDirectory = chatSessionContext.isUntitled ? + await this.worktreeManager.createWorktreeIfNeeded(id, stream) : + this.worktreeManager.getWorktreePath(id); - return {}; - } + const session = chatSessionContext.isUntitled ? + await this.sessionService.createSession(prompt, modelId, workingDirectory, token) : + await this.sessionService.getSession(id, undefined, workingDirectory, false, token); - const workingDirectory = this.worktreeManager.getWorktreePath(id); - const session = await this.sessionService.getSession(id, undefined, workingDirectory, false, token); if (!session) { stream.warning(vscode.l10n.t('Chat session not found.')); - return {}; + return undefined; } + disposables.add(session.attachStream(stream)); + disposables.add(session.attachPermissionHandler(async (permissionRequest: PermissionRequest) => requestPermission(permissionRequest, this.toolsService, request.toolInvocationToken, token))); - if (request.acceptedConfirmationData || request.rejectedConfirmationData) { - return await this.handleConfirmationData(session, request, context, stream, token); - } - if (request.prompt.startsWith('/delegate')) { - await this.handleDelegateCommand(session, request, context, stream, token); - return {}; - } + return session; + } - await session.handleRequest(prompt, attachments, modelId, stream, request.toolInvocationToken, token); - return {}; + private async getModelId(sessionId: string): Promise { + const defaultModel = await this.copilotCLIModels.getDefaultModel(); + const preferredModel = _sessionModel.get(sessionId); + // For existing sessions we cannot fall back, as the model info would be updated in _sessionModel + return this.copilotCLIModels.toModelProvider(preferredModel?.id || defaultModel.id); } private async handleDelegateCommand(session: ICopilotCLISession, request: vscode.ChatRequest, context: vscode.ChatContext, stream: vscode.ChatResponseStream, token: vscode.CancellationToken) { if (!this.cloudSessionProvider) { stream.warning(localize('copilotcli.missingCloudAgent', "No cloud agent available")); - return {}; + return; } // Check for uncommitted changes @@ -405,38 +434,38 @@ export class CopilotCLIChatSessionParticipant { chatContext: context }, stream, token); if (prInfo) { - await this.recordPushToSession(session, request.prompt, prInfo, token); + await this.recordPushToSession(session, request.prompt, prInfo); } } } - private async handleConfirmationData(session: ICopilotCLISession, request: vscode.ChatRequest, context: vscode.ChatContext, stream: vscode.ChatResponseStream, token: vscode.CancellationToken) { + private getAcceptedRejectedConfirmationData(request: vscode.ChatRequest): ConfirmationResult[] { const results: ConfirmationResult[] = []; results.push(...(request.acceptedConfirmationData?.map(data => ({ step: data.step, accepted: true, metadata: data?.metadata })) ?? [])); results.push(...((request.rejectedConfirmationData ?? []).filter(data => !results.some(r => r.step === data.step)).map(data => ({ step: data.step, accepted: false, metadata: data?.metadata })))); - for (const data of results) { - switch (data.step) { - case 'uncommitted-changes': - { - if (!data.accepted || !data.metadata) { - stream.markdown(vscode.l10n.t('Cloud agent delegation request cancelled.')); - return {}; - } - const prInfo = await this.cloudSessionProvider?.createDelegatedChatSession({ - prompt: data.metadata.prompt, - history: data.metadata.history, - chatContext: context - }, stream, token); - if (prInfo) { - await this.recordPushToSession(session, request.prompt, prInfo, token); - } - return {}; - } - default: - stream.warning(`Unknown confirmation step: ${data.step}\n\n`); - break; - } + return results; + } + + private async handleConfirmationData(session: ICopilotCLISession, prompt: string, results: ConfirmationResult[], context: vscode.ChatContext, stream: vscode.ChatResponseStream, token: vscode.CancellationToken) { + const uncommittedChangesData = results.find(data => data.step === UncommittedChangesStep); + if (!uncommittedChangesData) { + stream.warning(`Unknown confirmation step: ${results.map(r => r.step).join(', ')}\n\n`); + return {}; + } + + if (!uncommittedChangesData.accepted || !uncommittedChangesData.metadata) { + stream.markdown(vscode.l10n.t('Cloud agent delegation request cancelled.')); + return {}; + } + + const prInfo = await this.cloudSessionProvider?.createDelegatedChatSession({ + prompt: uncommittedChangesData.metadata.prompt, + history: uncommittedChangesData.metadata.history, + chatContext: context + }, stream, token); + if (prInfo) { + await this.recordPushToSession(session, prompt, prInfo); } return {}; } @@ -444,7 +473,6 @@ export class CopilotCLIChatSessionParticipant { private async handlePushConfirmationData( request: vscode.ChatRequest, context: vscode.ChatContext, - stream: vscode.ChatResponseStream, token: vscode.CancellationToken ): Promise { const prompt = request.prompt; @@ -453,16 +481,15 @@ export class CopilotCLIChatSessionParticipant { const requestPrompt = history ? `${prompt}\n**Summary**\n${history}` : prompt; const session = await this.sessionService.createSession(requestPrompt, undefined, undefined, token); - await vscode.commands.executeCommand('vscode.open', SessionIdForCLI.getResource(session.sessionId)); - await vscode.commands.executeCommand('workbench.action.chat.submit', { inputValue: requestPrompt }); + await this.commandExecutionService.executeCommand('vscode.open', SessionIdForCLI.getResource(session.sessionId)); + await this.commandExecutionService.executeCommand('workbench.action.chat.submit', { inputValue: requestPrompt }); return {}; } private async recordPushToSession( session: ICopilotCLISession, userPrompt: string, - prInfo: { uri: string; title: string; description: string; author: string; linkTag: string }, - token: vscode.CancellationToken + prInfo: { uri: string; title: string; description: string; author: string; linkTag: string } ): Promise { // Add user message event session.addUserMessage(userPrompt); diff --git a/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts b/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts index 6e22d31e5a..bbb40665e3 100644 --- a/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts +++ b/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts @@ -21,6 +21,7 @@ import { ChatSessionContentBuilder } from './copilotCloudSessionContentBuilder'; import { IPullRequestFileChangesService } from './pullRequestFileChangesService'; export type ConfirmationResult = { step: string; accepted: boolean; metadata?: ConfirmationMetadata }; +export const UncommittedChangesStep = 'uncommitted-changes'; interface ConfirmationMetadata { prompt: string; @@ -582,7 +583,7 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C } break; } - case 'uncommitted-changes': + case UncommittedChangesStep: { if (!data.accepted || !data.metadata) { stream.markdown(vscode.l10n.t('Cloud agent request cancelled due to uncommitted changes.')); @@ -666,7 +667,7 @@ export class CopilotCloudSessionsProvider extends Disposable implements vscode.C vscode.l10n.t('Uncommitted changes detected'), vscode.l10n.t('You have uncommitted changes in your workspace. Consider committing them if you would like to include them in the cloud agent\'s work.'), { - step: 'uncommitted-changes', + step: UncommittedChangesStep, metadata: metadata satisfies ConfirmationMetadata, // Forward metadata }, ['Proceed', 'Cancel'] diff --git a/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts b/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts new file mode 100644 index 0000000000..274e908d86 --- /dev/null +++ b/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts @@ -0,0 +1,347 @@ +/*--------------------------------------------------------------------------------------------- + * 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 * as vscode from 'vscode'; +import { IRunCommandExecutionService } from '../../../../platform/commands/common/runCommandExecutionService'; +import type { IGitService } from '../../../../platform/git/common/gitService'; +import type { ITelemetryService } from '../../../../platform/telemetry/common/telemetry'; +import { mock } from '../../../../util/common/test/simpleMock'; +import { CancellationTokenSource } from '../../../../util/vs/base/common/cancellation'; +import { Emitter } from '../../../../util/vs/base/common/event'; +import { DisposableStore } from '../../../../util/vs/base/common/lifecycle'; +import type { ICopilotCLIModels } from '../../../agents/copilotcli/node/copilotCli'; +import { CopilotCLIPromptResolver } from '../../../agents/copilotcli/node/copilotcliPromptResolver'; +import type { ICopilotCLISession } from '../../../agents/copilotcli/node/copilotcliSession'; +import type { ICopilotCLISessionService } from '../../../agents/copilotcli/node/copilotcliSessionService'; +import { PermissionRequest } from '../../../agents/copilotcli/node/permissionHelpers'; +import { ChatSummarizerProvider } from '../../../prompt/node/summarizer'; +import { MockChatResponseStream, TestChatRequest } from '../../../test/node/testHelpers'; +import type { IToolsService } from '../../../tools/common/toolsService'; +import { CopilotCLIChatSessionItemProvider, CopilotCLIChatSessionParticipant, CopilotCLIWorktreeManager } from '../copilotCLIChatSessionsContribution'; +import { CopilotCloudSessionsProvider } from '../copilotCloudSessionsProvider'; +// Mock terminal integration to avoid importing PowerShell asset (.ps1) which Vite cannot parse during tests +vi.mock('../copilotCLITerminalIntegration', () => { + // Minimal stand-in for createServiceIdentifier + const createServiceIdentifier = (name: string) => { + const fn: any = () => { /* decorator no-op */ }; + fn.toString = () => name; + return fn; + }; + class CopilotCLITerminalIntegration { + dispose() { } + openTerminal = vi.fn(async () => { }); + } + return { + ICopilotCLITerminalIntegration: createServiceIdentifier('ICopilotCLITerminalIntegration'), + CopilotCLITerminalIntegration + }; +}); + +// Minimal fake implementations for dependencies used by the participant + +class FakePromptResolver extends mock() { + override resolvePrompt(request: vscode.ChatRequest) { + return Promise.resolve({ prompt: request.prompt, attachments: [] }); + } +} + +class FakeSessionItemProvider extends mock() { + override swap = vi.fn(); +} + +class FakeSummarizerProvider extends mock() { + override provideChatSummary(_context: vscode.ChatContext) { return Promise.resolve('summary text'); } +} + +class FakeWorktreeManager extends mock() { + override createWorktreeIfNeeded = vi.fn(async () => undefined); + override storeWorktreePath = vi.fn(async () => { }); + override getWorktreePath = vi.fn((_id: string) => undefined); +} + +interface CreateSessionArgs { prompt: string | undefined; modelId: string | undefined; workingDirectory: string | undefined } + +class FakeCopilotCLISession implements ICopilotCLISession { + public sessionId: string; + public status: any; + public permissionRequested: any; + public onDidChangeStatus: any = () => ({ dispose() { } }); + public attachPermissionHandler = vi.fn(() => ({ dispose() { } })); + // Implementation uses the (typo'd) method name `attchStream`. + public attachStream = vi.fn(() => ({ dispose() { } })); + public handleRequest = vi.fn(async () => { }); + public addUserMessage = vi.fn(); + public addUserAssistantMessage = vi.fn(); + public getSelectedModelId = vi.fn(async () => 'model-default'); + public getChatHistory = vi.fn(() => []); + constructor(id: string) { this.sessionId = id; } + onPermissionRequested: vscode.Event = () => ({ dispose() { } }); + dispose(): void { + throw new Error('Method not implemented.'); + } +} + +class FakeSessionService extends DisposableStore implements ICopilotCLISessionService { + _serviceBrand: undefined; + public createdArgs: CreateSessionArgs | undefined; + public createSession = vi.fn(async (prompt: string | undefined, modelId: string | undefined, workingDirectory: string | undefined) => { + this.createdArgs = { prompt, modelId, workingDirectory }; + const s = new FakeCopilotCLISession('new-session-id'); + return s as unknown as ICopilotCLISession; + }); + private existing: Map = new Map(); + public getSession = vi.fn(async (id: string) => this.existing.get(id)); + public getAllSessions = vi.fn(async () => []); + public deleteSession = vi.fn(async () => { }); + public onDidChangeSessions = this.add(new Emitter()).event; + // helper + setExisting(id: string, session: ICopilotCLISession) { this.existing.set(id, session); } +} + +class FakeModels implements ICopilotCLIModels { + _serviceBrand: undefined; + getDefaultModel = vi.fn(async () => ({ id: 'base', name: 'Base' })); + getAvailableModels = vi.fn(async () => [{ id: 'base', name: 'Base' }]); + setDefaultModel = vi.fn(async () => { }); + toModelProvider = vi.fn((id: string) => id); // passthrough +} + +class FakeTelemetry extends mock() { + override sendMSFTTelemetryEvent = vi.fn(); +} + +class FakeToolsService extends mock() { } + +class FakeGitService extends mock() { + override activeRepository = { get: () => undefined } as unknown as IGitService['activeRepository']; +} + +// Cloud provider fake for delegate scenario +class FakeCloudProvider extends mock() { + override tryHandleUncommittedChanges = vi.fn(async () => false); + override createDelegatedChatSession = vi.fn(async () => ({ uri: 'pr://1', title: 'PR Title', description: 'Desc', author: 'Me', linkTag: 'tag' })) as unknown as CopilotCloudSessionsProvider['createDelegatedChatSession']; +} + +class FakeCommandExecutionService extends mock() { + override executeCommand = vi.fn(async () => { }); +} + +function createChatContext(sessionId: string, isUntitled: boolean): vscode.ChatContext { + return { + chatSessionContext: { + chatSessionItem: { resource: vscode.Uri.from({ scheme: 'copilotcli', path: `/${sessionId}` }), label: 'temp' } as vscode.ChatSessionItem, + isUntitled + } as vscode.ChatSessionContext, + chatSummary: undefined + } as vscode.ChatContext; +} + +describe('CopilotCLIChatSessionParticipant.handleRequest', () => { + const disposables = new DisposableStore(); + let promptResolver: FakePromptResolver; + let itemProvider: FakeSessionItemProvider; + let cloudProvider: FakeCloudProvider; + let summarizer: FakeSummarizerProvider; + let worktree: FakeWorktreeManager; + let git: FakeGitService; + let models: FakeModels; + let sessionService: FakeSessionService; + let telemetry: FakeTelemetry; + let tools: FakeToolsService; + let participant: CopilotCLIChatSessionParticipant; + let commandExecutionService: FakeCommandExecutionService; + + beforeEach(() => { + promptResolver = new FakePromptResolver(); + itemProvider = new FakeSessionItemProvider(); + cloudProvider = new FakeCloudProvider(); + summarizer = new FakeSummarizerProvider(); + worktree = new FakeWorktreeManager(); + git = new FakeGitService(); + models = new FakeModels(); + sessionService = disposables.add(new FakeSessionService()); + telemetry = new FakeTelemetry(); + tools = new FakeToolsService(); + commandExecutionService = new FakeCommandExecutionService(); + participant = new CopilotCLIChatSessionParticipant( + promptResolver, + itemProvider, + cloudProvider, + summarizer, + worktree, + git, + models, + sessionService, + telemetry, + tools, + commandExecutionService + ); + }); + + afterEach(() => { + vi.restoreAllMocks(); + disposables.clear(); + }); + + it('creates new session for untitled context and invokes request', async () => { + 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); + + // createSession was used + expect(sessionService.createSession).toHaveBeenCalled(); + const createdSession = sessionService.createSession.mock.results[0].value as Promise; + const session = await createdSession; + // handleRequest on the returned session + // handleRequest signature: (prompt, attachments, modelId, token) + expect(session.handleRequest).toHaveBeenCalledWith('Say hi', [], 'base', token); + // Swap should have been called to replace the untitled item + expect(itemProvider.swap).toHaveBeenCalled(); + }); + + it('reuses existing session (non-untitled) and does not create new one', async () => { + const existing = new FakeCopilotCLISession('existing-123'); + sessionService.setExisting('existing-123', existing as unknown as ICopilotCLISession); + const request = new TestChatRequest('Continue'); + const context = createChatContext('existing-123', false); + const stream = new MockChatResponseStream(); + const token = disposables.add(new CancellationTokenSource()).token; + + await participant.createHandler()(request, context, stream, token); + + expect(sessionService.getSession).toHaveBeenCalledWith('existing-123', undefined, undefined, false, token); + expect(sessionService.createSession).not.toHaveBeenCalled(); + expect(existing.handleRequest).toHaveBeenCalledWith('Continue', [], 'base', token); + // No swap for existing session + expect(itemProvider.swap).not.toHaveBeenCalled(); + }); + + it('handles /delegate command for existing session (no session.handleRequest)', async () => { + const existing = new FakeCopilotCLISession('existing-delegate'); + sessionService.setExisting('existing-delegate', existing as unknown as ICopilotCLISession); + // Simulate uncommitted changes + git.activeRepository = { get: () => ({ changes: { indexChanges: [{ path: 'file.ts' }] } }) } as unknown as IGitService['activeRepository']; + const request = new TestChatRequest('/delegate Build feature'); + const context = createChatContext('existing-delegate', false); + const stream = new MockChatResponseStream(); + const token = disposables.add(new CancellationTokenSource()).token; + + await participant.createHandler()(request, context, stream, token); + + expect(sessionService.getSession).toHaveBeenCalled(); + expect(existing.handleRequest).not.toHaveBeenCalled(); + expect(cloudProvider.tryHandleUncommittedChanges).toHaveBeenCalled(); + expect(cloudProvider.createDelegatedChatSession).toHaveBeenCalled(); + // PR metadata recorded + expect(existing.addUserMessage).toHaveBeenCalledWith('/delegate Build feature'); + const assistantArg = existing.addUserAssistantMessage.mock.calls[0][0]; + expect(assistantArg).toContain('pr://1'); + // Uncommitted changes warning surfaced + // Warning should appear (we emitted stream.warning). The mock stream only records markdown. + // Delegate path adds assistant PR metadata; ensure output contains PR metadata tag instead of relying on warning capture. + expect(assistantArg).toMatch(/ { + const request = new TestChatRequest('Push this'); + const context = { chatSessionContext: undefined, chatSummary: undefined } as unknown as vscode.ChatContext; + const stream = new MockChatResponseStream(); + const token = disposables.add(new CancellationTokenSource()).token; + const summarySpy = vi.spyOn(summarizer, 'provideChatSummary'); + const execSpy = vi.spyOn(commandExecutionService, 'executeCommand'); + sessionService.createSession.mockResolvedValue(new FakeCopilotCLISession('push-session-id') as any); + + await participant.createHandler()(request, context, stream, token); + + const expectedPrompt = 'Push this\n**Summary**\nsummary text'; + expect(sessionService.createSession).toHaveBeenCalledWith(expectedPrompt, undefined, undefined, token); + expect(summarySpy).toHaveBeenCalledTimes(1); + expect(execSpy).toHaveBeenCalledTimes(2); + expect(execSpy.mock.calls[0]).toEqual(['vscode.open', expect.any(Object)]); + expect(String(execSpy.mock.calls[0].at(1))).toContain('copilotcli:/push-session-id'); + expect(execSpy.mock.calls[1]).toEqual(['workbench.action.chat.submit', { inputValue: expectedPrompt }]); + }); + it('invokes handlePushConfirmationData using existing chatSummary and skips summarizer', async () => { + const request = new TestChatRequest('Push that'); + const context = { chatSessionContext: undefined, chatSummary: { history: 'precomputed history' } } as unknown as vscode.ChatContext; + const stream = new MockChatResponseStream(); + const token = disposables.add(new CancellationTokenSource()).token; + const summarySpy = vi.spyOn(summarizer, 'provideChatSummary'); + const execSpy = vi.spyOn(commandExecutionService, 'executeCommand'); + sessionService.createSession.mockResolvedValue(new FakeCopilotCLISession('push2-session-id') as any); + + await participant.createHandler()(request, context, stream, token); + + const expectedPrompt = 'Push that\n**Summary**\nprecomputed history'; + expect(sessionService.createSession).toHaveBeenCalledWith(expectedPrompt, undefined, undefined, token); + expect(summarySpy).not.toHaveBeenCalled(); + expect(execSpy).toHaveBeenCalledTimes(2); + expect(execSpy.mock.calls[0].at(0)).toBe('vscode.open'); + expect(execSpy.mock.calls[1]).toEqual(['workbench.action.chat.submit', { inputValue: expectedPrompt }]); + }); + + it('handleConfirmationData accepts uncommitted-changes and records push', async () => { + // Existing session (non-untitled) so confirmation path is hit + const existing = new FakeCopilotCLISession('existing-confirm'); + sessionService.setExisting('existing-confirm', existing as unknown as ICopilotCLISession); + const request = new TestChatRequest('Apply'); + (request as any).acceptedConfirmationData = [{ step: 'uncommitted-changes', metadata: { prompt: 'delegate work', history: 'hist' } }]; + const context = createChatContext('existing-confirm', false); + const stream = new MockChatResponseStream(); + const token = disposables.add(new CancellationTokenSource()).token; + // Cloud provider will create delegated chat session returning prInfo + (cloudProvider.createDelegatedChatSession as unknown as ReturnType).mockResolvedValue({ uri: 'pr://2', title: 'T', description: 'D', author: 'A', linkTag: 'L' }); + + await participant.createHandler()(request, context, stream, token); + + // Should NOT call session.handleRequest, instead record push messages + expect(existing.handleRequest).not.toHaveBeenCalled(); + expect(existing.addUserMessage).toHaveBeenCalledWith('Apply'); + const assistantArg = existing.addUserAssistantMessage.mock.calls[0][0]; + expect(assistantArg).toContain('pr://2'); + // Cloud provider used with provided metadata + expect(cloudProvider.createDelegatedChatSession).toHaveBeenCalledWith({ prompt: 'delegate work', history: 'hist', chatContext: context }, expect.anything(), token); + }); + + it('handleConfirmationData cancels when uncommitted-changes rejected', async () => { + const existing = new FakeCopilotCLISession('existing-confirm-reject'); + sessionService.setExisting('existing-confirm-reject', existing as unknown as ICopilotCLISession); + const request = new TestChatRequest('Apply'); + (request as any).rejectedConfirmationData = [{ step: 'uncommitted-changes', metadata: { prompt: 'delegate work', history: 'hist' } }]; + const context = createChatContext('existing-confirm-reject', false); + const stream = new MockChatResponseStream(); + const token = disposables.add(new CancellationTokenSource()).token; + + await participant.createHandler()(request, context, stream, token); + + // Should not record push or call delegate session + expect(existing.addUserAssistantMessage).not.toHaveBeenCalled(); + expect(cloudProvider.createDelegatedChatSession).not.toHaveBeenCalled(); + // Cancellation message markdown captured + expect(stream.output.some(o => /Cloud agent delegation request cancelled/i.test(o))).toBe(true); + }); + + it('handleConfirmationData unknown step warns and skips', async () => { + const existing = new FakeCopilotCLISession('existing-confirm-unknown'); + sessionService.setExisting('existing-confirm-unknown', existing as unknown as ICopilotCLISession); + const request = new TestChatRequest('Apply'); + (request as any).acceptedConfirmationData = [{ step: 'mystery-step', metadata: {} }]; + const context = createChatContext('existing-confirm-unknown', false); + const stream = new MockChatResponseStream(); + const token = disposables.add(new CancellationTokenSource()).token; + + await participant.createHandler()(request, context, stream, token); + + // No push recorded, assistant message absent + expect(existing.addUserAssistantMessage).not.toHaveBeenCalled(); + // Warning emitted (MockChatResponseStream records markdown, not warning; plugin emits with stream.warning -> not captured) + // We just assert no PR metadata present and user message not added by recordPushToSession + expect(existing.addUserMessage).not.toHaveBeenCalledWith('Apply'); + }); +}); From 0bedf9f86bbbb04a8397a65b77de4a66f1c654ab Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sat, 8 Nov 2025 22:53:00 +0000 Subject: [PATCH 08/16] Remove dead code (#1879) Missed deleting this when moving subagents to core --- src/extension/prompt/node/subagentLoop.ts | 121 ---------------------- 1 file changed, 121 deletions(-) delete mode 100644 src/extension/prompt/node/subagentLoop.ts diff --git a/src/extension/prompt/node/subagentLoop.ts b/src/extension/prompt/node/subagentLoop.ts deleted file mode 100644 index bdbad0e00f..0000000000 --- a/src/extension/prompt/node/subagentLoop.ts +++ /dev/null @@ -1,121 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { randomUUID } from 'crypto'; -import type { CancellationToken, ChatRequest, ChatResponseStream, LanguageModelToolInformation, Progress } from 'vscode'; -import { IAuthenticationChatUpgradeService } from '../../../platform/authentication/common/authenticationUpgrade'; -import { ChatLocation, ChatResponse } from '../../../platform/chat/common/commonTypes'; -import { IEndpointProvider } from '../../../platform/endpoint/common/endpointProvider'; -import { ILogService } from '../../../platform/log/common/logService'; -import { IRequestLogger } from '../../../platform/requestLogger/node/requestLogger'; -import { ITelemetryService } from '../../../platform/telemetry/common/telemetry'; -import { IInstantiationService } from '../../../util/vs/platform/instantiation/common/instantiation'; -import { ChatResponseProgressPart, ChatResponseReferencePart } from '../../../vscodeTypes'; -import { getAgentTools } from '../../intents/node/agentIntent'; -import { IToolCallingLoopOptions, ToolCallingLoop, ToolCallingLoopFetchOptions } from '../../intents/node/toolCallingLoop'; -import { AgentPrompt } from '../../prompts/node/agent/agentPrompt'; -import { PromptRenderer } from '../../prompts/node/base/promptRenderer'; -import { ToolName } from '../../tools/common/toolNames'; -import { normalizeToolSchema } from '../../tools/common/toolSchemaNormalizer'; -import { ChatVariablesCollection } from '../common/chatVariablesCollection'; -import { IBuildPromptContext } from '../common/intents'; -import { IBuildPromptResult } from './intents'; - -export interface ISubagentToolCallingLoopOptions extends IToolCallingLoopOptions { - request: ChatRequest; - location: ChatLocation; - promptText: string; -} - -export class SubagentToolCallingLoop extends ToolCallingLoop { - - public static readonly ID = 'subagent'; - - constructor( - options: ISubagentToolCallingLoopOptions, - @IInstantiationService private readonly instantiationService: IInstantiationService, - @ILogService logService: ILogService, - @IRequestLogger requestLogger: IRequestLogger, - @IEndpointProvider private readonly endpointProvider: IEndpointProvider, - @IAuthenticationChatUpgradeService authenticationChatUpgradeService: IAuthenticationChatUpgradeService, - @ITelemetryService telemetryService: ITelemetryService, - ) { - super(options, instantiationService, endpointProvider, logService, requestLogger, authenticationChatUpgradeService, telemetryService); - } - - protected override createPromptContext(availableTools: LanguageModelToolInformation[], outputStream: ChatResponseStream | undefined): IBuildPromptContext { - const context = super.createPromptContext(availableTools, outputStream); - if (context.tools) { - context.tools = { - ...context.tools, - toolReferences: [], - inSubAgent: true - }; - } - context.query = this.options.promptText; - context.chatVariables = new ChatVariablesCollection(); - context.conversation = undefined; - return context; - } - - private async getEndpoint(request: ChatRequest) { - let endpoint = await this.endpointProvider.getChatEndpoint(this.options.request); - if (!endpoint.supportsToolCalls) { - endpoint = await this.endpointProvider.getChatEndpoint('gpt-4.1'); - } - return endpoint; - } - - protected async buildPrompt(promptContext: IBuildPromptContext, progress: Progress, token: CancellationToken): Promise { - const endpoint = await this.getEndpoint(this.options.request); - const renderer = PromptRenderer.create( - this.instantiationService, - endpoint, - AgentPrompt, - { - endpoint, - promptContext: promptContext, - location: this.options.location, - enableCacheBreakpoints: false, - } - ); - return await renderer.render(progress, token); - } - - protected async getAvailableTools(): Promise { - const excludedTools = new Set([ToolName.CoreRunSubagent, ToolName.CoreManageTodoList]); - return (await getAgentTools(this.instantiationService, this.options.request)) - .filter(tool => !excludedTools.has(tool.name as ToolName)) - // TODO can't do virtual tools at this level - .slice(0, 128); - } - - protected async fetch({ messages, finishedCb, requestOptions }: ToolCallingLoopFetchOptions, token: CancellationToken): Promise { - const endpoint = await this.getEndpoint(this.options.request); - return endpoint.makeChatRequest2({ - debugName: SubagentToolCallingLoop.ID, - messages, - finishedCb, - location: this.options.location, - requestOptions: { - ...(requestOptions ?? {}), - temperature: 0, - tools: normalizeToolSchema( - endpoint.family, - requestOptions?.tools, - (tool, rule) => { - this._logService.warn(`Tool ${tool} failed validation: ${rule}`); - }, - ), - }, - // This loop is inside a tool called from another request, so never user initiated - userInitiatedRequest: false, - telemetryProperties: { - messageId: randomUUID(), - messageSource: SubagentToolCallingLoop.ID - }, - }, token); - } -} From 4b85c4977ed666b0b6b7abe584d4eac00f11ab13 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sat, 8 Nov 2025 22:53:21 +0000 Subject: [PATCH 09/16] Fix subagent request marked as user-initiated (#1876) Fix #276305 --- src/extension/intents/node/toolCallingLoop.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extension/intents/node/toolCallingLoop.ts b/src/extension/intents/node/toolCallingLoop.ts index fe98ab74fe..3bb46ca025 100644 --- a/src/extension/intents/node/toolCallingLoop.ts +++ b/src/extension/intents/node/toolCallingLoop.ts @@ -462,7 +462,7 @@ export abstract class ToolCallingLoop Date: Sat, 8 Nov 2025 22:53:39 +0000 Subject: [PATCH 10/16] Merge pull request #1867 from microsoft/roblou/well-wolf (#1877) New model prompt updates --- .../prompts/node/agent/openAIPrompts.tsx | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/extension/prompts/node/agent/openAIPrompts.tsx b/src/extension/prompts/node/agent/openAIPrompts.tsx index 36588cd7fb..eb80676839 100644 --- a/src/extension/prompts/node/agent/openAIPrompts.tsx +++ b/src/extension/prompts/node/agent/openAIPrompts.tsx @@ -343,7 +343,7 @@ class ModelBPrompt extends PromptElement { Before making tool calls, send a brief preamble to the user explaining what you're about to do. When sending preamble messages, follow these principles and examples:

- **Logically group related actions**: if you're about to run several related commands, describe them together in one preamble rather than sending a separate note for each.
- - **Keep it concise**: be no more than 1-2 sentences, focused on immediate, tangible next steps. (8-12 words for quick updates).
+ - **Keep it concise**: no more than 1 or maybe 2 sentences, focused on immediate, tangible next steps. (8-12 words for quick updates).
- **Build on prior context**: if this is not your first tool call, use the preamble message to connect the dots with what's been done so far and create a sense of momentum and clarity for the user to understand your next actions.
- **Keep your tone light, friendly and curious**: add small touches of personality in preambles feel collaborative and engaging.
- **Exception**: Avoid adding a preamble for every trivial action (e.g., read a single file) unless it's part of a larger grouped action.
@@ -365,7 +365,7 @@ class ModelBPrompt extends PromptElement {
Note that plans are not for padding out simple work with filler steps or stating the obvious. The content of your plan should not involve doing anything that you aren't capable of doing (i.e. don't try to test things that you can't test). Do not use plans for simple or single-step queries that you can just do or answer immediately.

- Do not repeat the full contents of the plan after an `update_plan` call — the harness already displays it. Instead, summarize the change made and highlight any important context or next step.
+ Do not repeat the full contents of the plan after an `{ToolName.CoreManageTodoList}` call — the harness already displays it. Instead, summarize the change made and highlight any important context or next step.
} {!tools[ToolName.CoreManageTodoList] && <> For complex tasks requiring multiple steps, you should maintain an organized approach. Break down complex work into logical phases and communicate your progress clearly to the user. Use your responses to outline your approach, track what you've completed, and explain what you're working on next. Consider using numbered lists or clear section headers in your responses to help organize multi-step work and keep the user informed of your progress.
@@ -455,7 +455,8 @@ class ModelBPrompt extends PromptElement { - Do not add inline comments within code unless explicitly requested.
- Do not use one-letter variable names unless explicitly requested.
- NEVER output inline citations like "【F:README.md†L5-L14】" in your outputs. The UI is not able to render these so they will just be broken in the UI. Instead, if you output valid filepaths, users will be able to click on them to open the files in their editor.
- - If there is a specific tool available to do a task, prefer using the tool over running a shell command. + - You have access to many tools. If a tool exists to perform a specific task, you MUST use that tool instead of running a terminal command to perform that task.
+ {tools[ToolName.RunTests] && <>- Use the {ToolName.RunTests} tool to run tests instead of running terminal commands.
} If the codebase has tests or the ability to build or run, consider using them to verify that your work is complete.
@@ -472,7 +473,7 @@ class ModelBPrompt extends PromptElement { You should use judicious initiative to decide on the right level of detail and complexity to deliver based on the user's needs. This means showing good judgment that you're capable of doing the right extras without gold-plating. This might be demonstrated by high-value, creative touches when scope of the task is vague; while being surgical and targeted when scope is tightly specified. - For especially longer tasks that you work on (i.e. requiring many tool calls, or a plan with multiple steps), you should provide progress updates back to the user at reasonable intervals. These updates should be structured as a concise sentence or two (no more than 8-10 words long) recapping progress so far in plain language: this update demonstrates your understanding of what needs to be done, progress so far (i.e. files explores, subtasks complete), and where you're going next.
+ For especially longer tasks that you work on (i.e. requiring many tool calls, or a plan with multiple steps), you should provide progress updates back to the user at reasonable intervals. These updates should be structured as a concise sentence or two (no more than 8-10 words long) recapping progress so far in plain language: this update demonstrates your understanding of what needs to be done, progress so far (i.e. files explored, subtasks complete), and where you're going next.

Before doing large chunks of work that may incur latency as experienced by the user (i.e. writing a new file), you should send a concise message to the user with an update indicating what you're about to do to ensure they know what you're spending time on. Don't start editing or writing large files before informing the user what you are doing and why.

@@ -490,9 +491,9 @@ class ModelBPrompt extends PromptElement { Your final message should read naturally, like an update from a concise teammate. For casual conversation, brainstorming tasks, or quick questions from the user, respond in a friendly, conversational tone. You should ask questions, suggest ideas, and adapt to the user's style. If you've finished a large amount of work, when describing what you've done to the user, you should follow the final answer formatting guidelines to communicate substantive changes. You don't need to add structured formatting for one-word answers, greetings, or purely conversational exchanges.
You can skip heavy formatting for single, simple actions or confirmations. In these cases, respond in plain sentences with any relevant next step or quick option. Reserve multi-section structured responses for results that need grouping or explanation.
- The user is working on the same computer as you, and has access to your work. As such there's no need to show the full contents of large files you have already written unless the user explicitly asks for them. Similarly, if you've created or modified files using `apply_patch`, there's no need to tell users to "save the file" or "copy the code into a file"—just reference the file path.
+ The user is working on the same computer as you, and has access to your work. As such there's no need to show the full contents of large files you have already written or verbatim code snippets unless the user explicitly asks for them. Similarly, if you've created or modified files using `apply_patch`, there's no need to tell users to "save the file" or "copy the code into a file"—just reference the file path.
If there's something that you think you could help with as a logical next step, concisely ask the user if they want you to do so. Good examples of this are running tests, committing changes, or building out the next logical component. If there's something that you couldn't do (even with approval) but that the user might want to do (such as verifying changes by running the app), include those instructions succinctly.
- Brevity is very important as a default. You should be very concise (i.e. no more than 10 lines), but can relax this requirement for tasks where additional detail and comprehensiveness is important for the user's understanding.
+ Brevity is very important as a default. You should be very concise (i.e. no more than 10 lines or around 500 words), but can relax this requirement for tasks where additional detail and comprehensiveness is important for the user's understanding. Don't simply repeat all the changes you made- that is too much detail.

### Final answer structure and style guidelines

@@ -548,7 +549,7 @@ class ModelBPrompt extends PromptElement {
Generally, ensure your final answers adapt their shape and depth to the request. For example, answers to code explanations should have a precise, structured explanation with code references that answer the question directly. For tasks with a simple implementation, lead with the outcome and supplement only with what's needed for clarity. Larger changes can be presented as a logical walkthrough of your approach, grouping related steps, explaining rationale where it adds value, and highlighting next actions to accelerate the user. Your answers should provide the right level of detail while being easily scannable.

- For casual greetings, acknowledgements, or other one-off conversational messages that are not delivering substantive information or structured results, respond naturally without section headers or bullet formatting.
+ For casual greetings, acknowledgements, or other one-off conversational messages that are not delivering substantive information or structured results, respond naturally without section headers or bullet formatting. ; From e2a459fc7a3193f1f190f4a80f72ab40096c9631 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sat, 8 Nov 2025 22:53:54 +0000 Subject: [PATCH 11/16] Merge pull request #1860 from microsoft/roblou/revert-chat-activation (#1878) Restore activationBlockers on auth Co-authored-by: Logan Ramos --- src/extension/common/contributions.ts | 22 ++++++++ .../vscode-node/chatParticipants.ts | 32 ++---------- .../vscode-node/conversationFeature.ts | 22 +++++++- .../vscode-node/languageModelAccess.ts | 50 ++++++++++++------- .../test/conversationFeature.test.ts | 10 ++++ src/extension/extension/vscode/extension.ts | 7 ++- 6 files changed, 94 insertions(+), 49 deletions(-) diff --git a/src/extension/common/contributions.ts b/src/extension/common/contributions.ts index e55beab57d..1d8f3e1b57 100644 --- a/src/extension/common/contributions.ts +++ b/src/extension/common/contributions.ts @@ -5,6 +5,7 @@ import { ILogService } from '../../platform/log/common/logService'; import { Disposable, isDisposable } from '../../util/vs/base/common/lifecycle'; +import { StopWatch } from '../../util/vs/base/common/stopwatch'; import { IInstantiationService, ServicesAccessor } from '../../util/vs/platform/instantiation/common/instantiation'; export interface IExtensionContribution { @@ -15,6 +16,12 @@ export interface IExtensionContribution { * Dispose of the contribution. */ dispose?(): void; + + /** + * A promise that the extension `activate` method will wait on before completing. + * USE this carefully as it will delay startup of our extension. + */ + activationBlocker?: Promise; } export interface IExtensionContributionFactory { @@ -31,6 +38,8 @@ export function asContributionFactory(ctor: { new(...args: any[]): any }): IExte } export class ContributionCollection extends Disposable { + private readonly allActivationBlockers: Promise[] = []; + constructor( contribs: IExtensionContributionFactory[], @ILogService logService: ILogService, @@ -46,9 +55,22 @@ export class ContributionCollection extends Disposable { if (isDisposable(instance)) { this._register(instance); } + + if (instance?.activationBlocker) { + const sw = StopWatch.create(); + const id = instance.id || 'UNKNOWN'; + this.allActivationBlockers.push(instance.activationBlocker.finally(() => { + logService.info(`activationBlocker from '${id}' took for ${Math.round(sw.elapsed())}ms`); + })); + } } catch (error) { logService.error(error, `Error while loading contribution`); } } } + + async waitForActivationBlockers(): Promise { + // WAIT for all activation blockers to complete + await Promise.allSettled(this.allActivationBlockers); + } } diff --git a/src/extension/conversation/vscode-node/chatParticipants.ts b/src/extension/conversation/vscode-node/chatParticipants.ts index 48c06becc9..4c33570ce6 100644 --- a/src/extension/conversation/vscode-node/chatParticipants.ts +++ b/src/extension/conversation/vscode-node/chatParticipants.ts @@ -10,9 +10,7 @@ import { IInteractionService } from '../../../platform/chat/common/interactionSe import { ConfigKey, IConfigurationService } from '../../../platform/configuration/common/configurationService'; import { IEndpointProvider } from '../../../platform/endpoint/common/endpointProvider'; import { IOctoKitService } from '../../../platform/github/common/githubService'; -import { ILogService } from '../../../platform/log/common/logService'; import { IExperimentationService } from '../../../platform/telemetry/common/nullExperimentationService'; -import { DeferredPromise } from '../../../util/vs/base/common/async'; import { Event, Relay } from '../../../util/vs/base/common/event'; import { DisposableStore, IDisposable } from '../../../util/vs/base/common/lifecycle'; import { autorun } from '../../../util/vs/base/common/observableInternal'; @@ -30,18 +28,18 @@ import { getAdditionalWelcomeMessage } from './welcomeMessageProvider'; export class ChatAgentService implements IChatAgentService { declare readonly _serviceBrand: undefined; - private _lastChatAgents: ChatParticipants | undefined; // will be cleared when disposed + private _lastChatAgents: ChatAgents | undefined; // will be cleared when disposed constructor( @IInstantiationService private readonly instantiationService: IInstantiationService, ) { } - public debugGetCurrentChatAgents(): ChatParticipants | undefined { + public debugGetCurrentChatAgents(): ChatAgents | undefined { return this._lastChatAgents; } register(): IDisposable { - const chatAgents = this.instantiationService.createInstance(ChatParticipants); + const chatAgents = this.instantiationService.createInstance(ChatAgents); chatAgents.register(); this._lastChatAgents = chatAgents; return { @@ -53,13 +51,11 @@ export class ChatAgentService implements IChatAgentService { } } -class ChatParticipants implements IDisposable { +class ChatAgents implements IDisposable { private readonly _disposables = new DisposableStore(); private additionalWelcomeMessage: vscode.MarkdownString | undefined; - private requestBlocker: Promise; - constructor( @IOctoKitService private readonly octoKitService: IOctoKitService, @IAuthenticationService private readonly authenticationService: IAuthenticationService, @@ -71,24 +67,7 @@ class ChatParticipants implements IDisposable { @IChatQuotaService private readonly _chatQuotaService: IChatQuotaService, @IConfigurationService private readonly configurationService: IConfigurationService, @IExperimentationService private readonly experimentationService: IExperimentationService, - @ILogService private readonly logService: ILogService, - ) { - // TODO@roblourens This may not be necessary - the point for now is to match the old behavior of blocking chat until auth completes - const requestBlockerDeferred = new DeferredPromise(); - this.requestBlocker = requestBlockerDeferred.p; - if (authenticationService.copilotToken) { - this.logService.debug(`ChatParticipants: Copilot token already available`); - requestBlockerDeferred.complete(); - } else { - this._disposables.add(authenticationService.onDidAuthenticationChange(async () => { - const hasSession = !!authenticationService.copilotToken; - this.logService.debug(`ChatParticipants: onDidAuthenticationChange has token: ${hasSession}`); - if (hasSession) { - requestBlockerDeferred.complete(); - } - })); - } - } + ) { } dispose() { this._disposables.dispose(); @@ -280,7 +259,6 @@ Learn more about [GitHub Copilot](https://docs.github.com/copilot/using-github-c private getChatParticipantHandler(id: string, name: string, defaultIntentIdOrGetter: IntentOrGetter, onRequestPaused: Event): vscode.ChatExtendedRequestHandler { return async (request, context, stream, token): Promise => { - await this.requestBlocker; // If we need privacy confirmation, i.e with 3rd party models. We will return a confirmation response and return early const privacyConfirmation = await this.requestPolicyConfirmation(request, stream); diff --git a/src/extension/conversation/vscode-node/conversationFeature.ts b/src/extension/conversation/vscode-node/conversationFeature.ts index de5595a022..fec26d169a 100644 --- a/src/extension/conversation/vscode-node/conversationFeature.ts +++ b/src/extension/conversation/vscode-node/conversationFeature.ts @@ -18,6 +18,7 @@ import { ILogService } from '../../../platform/log/common/logService'; import { ISettingsEditorSearchService } from '../../../platform/settingsEditor/common/settingsEditorSearchService'; import { ITelemetryService } from '../../../platform/telemetry/common/telemetry'; import { isUri } from '../../../util/common/types'; +import { DeferredPromise } from '../../../util/vs/base/common/async'; import { CancellationToken } from '../../../util/vs/base/common/cancellation'; import { DisposableStore, IDisposable, combinedDisposable } from '../../../util/vs/base/common/lifecycle'; import { URI } from '../../../util/vs/base/common/uri'; @@ -61,6 +62,7 @@ export class ConversationFeature implements IExtensionContribution { private _settingsSearchProviderRegistered = false; readonly id = 'conversationFeature'; + readonly activationBlocker?: Promise; constructor( @IInstantiationService private instantiationService: IInstantiationService, @@ -85,7 +87,25 @@ export class ConversationFeature implements IExtensionContribution { // Register Copilot token listener this.registerCopilotTokenListener(); - this.activated = true; + const activationBlockerDeferred = new DeferredPromise(); + this.activationBlocker = activationBlockerDeferred.p; + if (authenticationService.copilotToken) { + this.logService.debug(`ConversationFeature: Copilot token already available`); + this.activated = true; + activationBlockerDeferred.complete(); + } + + this._disposables.add(authenticationService.onDidAuthenticationChange(async () => { + const hasSession = !!authenticationService.copilotToken; + this.logService.debug(`ConversationFeature: onDidAuthenticationChange has token: ${hasSession}`); + if (hasSession) { + this.activated = true; + } else { + this.activated = false; + } + + activationBlockerDeferred.complete(); + })); } get enabled() { diff --git a/src/extension/conversation/vscode-node/languageModelAccess.ts b/src/extension/conversation/vscode-node/languageModelAccess.ts index 7f502228c5..098e6eff08 100644 --- a/src/extension/conversation/vscode-node/languageModelAccess.ts +++ b/src/extension/conversation/vscode-node/languageModelAccess.ts @@ -29,7 +29,7 @@ import { isEncryptedThinkingDelta } from '../../../platform/thinking/common/thin import { BaseTokensPerCompletion } from '../../../platform/tokenizer/node/tokenizer'; import { TelemetryCorrelationId } from '../../../util/common/telemetryCorrelationId'; import { Emitter } from '../../../util/vs/base/common/event'; -import { Disposable } from '../../../util/vs/base/common/lifecycle'; +import { Disposable, MutableDisposable } from '../../../util/vs/base/common/lifecycle'; import { isDefined, isNumber, isString, isStringArray } from '../../../util/vs/base/common/types'; import { localize } from '../../../util/vs/nls'; import { IInstantiationService } from '../../../util/vs/platform/instantiation/common/instantiation'; @@ -44,6 +44,8 @@ export class LanguageModelAccess extends Disposable implements IExtensionContrib readonly id = 'languageModelAccess'; + readonly activationBlocker?: Promise; + private readonly _onDidChange = this._register(new Emitter()); private _currentModels: vscode.LanguageModelChatInformation[] = []; // Store current models for reference private _chatEndpoints: IChatEndpoint[] = []; @@ -71,8 +73,10 @@ export class LanguageModelAccess extends Disposable implements IExtensionContrib } // initial - this._registerChatProvider(); - this._registerEmbeddings(); + this.activationBlocker = Promise.all([ + this._registerChatProvider(), + this._registerEmbeddings(), + ]); } override dispose(): void { @@ -83,7 +87,7 @@ export class LanguageModelAccess extends Disposable implements IExtensionContrib return this._currentModels; } - private _registerChatProvider(): void { + private async _registerChatProvider(): Promise { const provider: vscode.LanguageModelChatProvider = { onDidChangeLanguageModelChatInformation: this._onDidChange.event, provideLanguageModelChatInformation: this._provideLanguageModelChatInfo.bind(this), @@ -256,22 +260,34 @@ export class LanguageModelAccess extends Disposable implements IExtensionContrib private async _registerEmbeddings(): Promise { - const embeddingsComputer = this._embeddingsComputer; - const embeddingType = EmbeddingType.text3small_512; - const model = getWellKnownEmbeddingTypeInfo(embeddingType)?.model; - if (!model) { - throw new Error(`No model found for embedding type ${embeddingType.id}`); - } + const dispo = this._register(new MutableDisposable()); + - const that = this; - this._register(vscode.lm.registerEmbeddingsProvider(`copilot.${model}`, new class implements vscode.EmbeddingsProvider { - async provideEmbeddings(input: string[], token: vscode.CancellationToken): Promise { - await that._getToken(); + const update = async () => { - const result = await embeddingsComputer.computeEmbeddings(embeddingType, input, {}, new TelemetryCorrelationId('EmbeddingsProvider::provideEmbeddings'), token); - return result.values.map(embedding => ({ values: embedding.value.slice(0) })); + if (!await this._getToken()) { + dispo.clear(); + return; } - })); + + const embeddingsComputer = this._embeddingsComputer; + const embeddingType = EmbeddingType.text3small_512; + const model = getWellKnownEmbeddingTypeInfo(embeddingType)?.model; + if (!model) { + throw new Error(`No model found for embedding type ${embeddingType.id}`); + } + + dispo.clear(); + dispo.value = vscode.lm.registerEmbeddingsProvider(`copilot.${model}`, new class implements vscode.EmbeddingsProvider { + async provideEmbeddings(input: string[], token: vscode.CancellationToken): Promise { + const result = await embeddingsComputer.computeEmbeddings(embeddingType, input, {}, new TelemetryCorrelationId('EmbeddingsProvider::provideEmbeddings'), token); + return result.values.map(embedding => ({ values: embedding.value.slice(0) })); + } + }); + }; + + this._register(this._authenticationService.onDidAuthenticationChange(() => update())); + await update(); } private async _getToken(): Promise { diff --git a/src/extension/conversation/vscode-node/test/conversationFeature.test.ts b/src/extension/conversation/vscode-node/test/conversationFeature.test.ts index 51fc815522..92063931cc 100644 --- a/src/extension/conversation/vscode-node/test/conversationFeature.test.ts +++ b/src/extension/conversation/vscode-node/test/conversationFeature.test.ts @@ -70,6 +70,16 @@ suite('Conversation feature test suite', function () { } }); + test("If the 'interactive' version does not match, the feature is not enabled and not activated", function () { + const conversationFeature = instaService.createInstance(ConversationFeature); + try { + assert.deepStrictEqual(conversationFeature.enabled, false); + assert.deepStrictEqual(conversationFeature.activated, false); + } finally { + conversationFeature.dispose(); + } + }); + test('The feature is enabled and activated in test mode', function () { const conversationFeature = instaService.createInstance(ConversationFeature); try { diff --git a/src/extension/extension/vscode/extension.ts b/src/extension/extension/vscode/extension.ts index cb945b21f4..c9d92d472d 100644 --- a/src/extension/extension/vscode/extension.ts +++ b/src/extension/extension/vscode/extension.ts @@ -9,7 +9,6 @@ import { isScenarioAutomation } from '../../../platform/env/common/envService'; import { isProduction } from '../../../platform/env/common/packagejson'; import { IHeatmapService } from '../../../platform/heatmap/common/heatmapService'; import { IIgnoreService } from '../../../platform/ignore/common/ignoreService'; -import { ILogService } from '../../../platform/log/common/logService'; import { IExperimentationService } from '../../../platform/telemetry/common/nullExperimentationService'; import { IInstantiationServiceBuilder, InstantiationServiceBuilder } from '../../../util/common/services'; import { IInstantiationService } from '../../../util/vs/platform/instantiation/common/instantiation'; @@ -64,16 +63,16 @@ export async function baseActivate(configuration: IExtensionActivationConfigurat await instantiationService.invokeFunction(async accessor => { const expService = accessor.get(IExperimentationService); - const logService = accessor.get(ILogService); // Await intialization of exp service. This ensure cache is fresh. // It will then auto refresh every 30 minutes after that. await expService.hasTreatments(); + // THIS is awaited because some contributions can block activation + // via `IExtensionContribution#activationBlocker` const contributions = instantiationService.createInstance(ContributionCollection, configuration.contributions); context.subscriptions.push(contributions); - - logService.trace('Copilot Chat extension activated'); + await contributions.waitForActivationBlockers(); }); if (ExtensionMode.Test === context.extensionMode && !isScenarioAutomation) { From e5e4a3f66ce00b2d023f6c4351f8900bb68eb581 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Sun, 9 Nov 2025 07:07:04 +0000 Subject: [PATCH 12/16] Faster loading of Copilot CLI session list (#1880) * Faster loading of Copilot CLI Session Info * Updates --- .../node/copilotcliSessionService.ts | 39 ++++++++++--------- .../test/copilotCliSessionService.spec.ts | 1 - 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/extension/agents/copilotcli/node/copilotcliSessionService.ts b/src/extension/agents/copilotcli/node/copilotcliSessionService.ts index 3c15f1397e..d8d089d2d7 100644 --- a/src/extension/agents/copilotcli/node/copilotcliSessionService.ts +++ b/src/extension/agents/copilotcli/node/copilotcliSessionService.ts @@ -119,6 +119,16 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS // This is a new session not yet persisted to disk by SDK return undefined; } + const timestamp = metadata.startTime; + const id = metadata.sessionId; + const label = metadata.summary ? labelFromPrompt(metadata.summary) : undefined; + if (label) { + return { + id, + label, + timestamp, + } satisfies ICopilotCLISessionItem; + } let dispose: (() => Promise) | undefined = undefined; let session: Session | undefined = undefined; try { @@ -134,20 +144,8 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS return undefined; } const label = this._generateSessionLabel(session.sessionId, chatMessages, undefined); - - // Get timestamp from last SDK event, or fallback to metadata.startTime - const sdkEvents = session.getEvents(); - const lastEventWithTimestamp = [...sdkEvents].reverse().find(event => - event.type !== 'session.import_legacy' - && event.type !== 'session.start' - && 'timestamp' in event - ); - const timestamp = lastEventWithTimestamp && 'timestamp' in lastEventWithTimestamp - ? new Date(lastEventWithTimestamp.timestamp) - : metadata.startTime; - return { - id: metadata.sessionId, + id, label, timestamp, } satisfies ICopilotCLISessionItem; @@ -333,13 +331,11 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS : ''; if (content) { - // Strip system reminders and return first line or first 50 characters, whichever is shorter - const cleanContent = stripReminders(content); - const firstLine = cleanContent.split('\n').find((l: string) => l.trim().length > 0) ?? ''; - return firstLine.length > 50 ? firstLine.substring(0, 47) + '...' : firstLine; + return labelFromPrompt(content); } } else if (prompt && prompt.trim().length > 0) { - return prompt.trim().length > 50 ? prompt.trim().substring(0, 47) + '...' : prompt.trim(); + return labelFromPrompt(prompt); + } } catch (error) { this.logService.warn(`Failed to generate session label for ${sessionId}: ${error}`); @@ -349,3 +345,10 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS return `Session ${sessionId.slice(0, 8)}`; } } + +function labelFromPrompt(prompt: string): string { + // Strip system reminders and return first line or first 50 characters, whichever is shorter + const cleanContent = stripReminders(prompt); + const firstLine = cleanContent.split('\n').find((l: string) => l.trim().length > 0) ?? ''; + return firstLine.length > 50 ? firstLine.substring(0, 47) + '...' : firstLine; +} \ No newline at end of file diff --git a/src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts b/src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts index 63f6ec2bd1..1f18414372 100644 --- a/src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts +++ b/src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts @@ -181,7 +181,6 @@ describe('CopilotCLISessionService', () => { expect(item.id).toBe('s1'); expect(item.label.endsWith('...')).toBe(true); // truncated expect(item.label.length).toBeLessThanOrEqual(50); - expect(item.timestamp.toISOString()).toBe(new Date(tsStr).toISOString()); }); }); From 3eec3fa2113c8098a0d73468518d2b69f1826737 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Sun, 9 Nov 2025 07:09:17 +0000 Subject: [PATCH 13/16] Support /delegating to cloud from a new cli session (#1886) --- .../copilotCLIChatSessionsContribution.ts | 2 +- .../copilotCLIChatSessionParticipant.spec.ts | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts index 754c61231d..25706a3f19 100644 --- a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts +++ b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts @@ -361,7 +361,7 @@ export class CopilotCLIChatSessionParticipant { return await this.handleConfirmationData(session, request.prompt, confirmationResults, context, stream, token); } - if (!isUntitled && request.prompt.startsWith('/delegate')) { + if (request.prompt.startsWith('/delegate')) { await this.handleDelegateCommand(session, request, context, stream, token); } else { await session.handleRequest(prompt, attachments, modelId, token); diff --git a/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts b/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts index 274e908d86..1e5aed1269 100644 --- a/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts +++ b/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts @@ -248,6 +248,30 @@ describe('CopilotCLIChatSessionParticipant.handleRequest', () => { expect(assistantArg).toMatch(/ { + const newSession = new FakeCopilotCLISession('push-session-id'); + git.activeRepository = { get: () => ({ changes: { indexChanges: [{ path: 'file.ts' }] } }) } as unknown as IGitService['activeRepository']; + const request = new TestChatRequest('/delegate Build feature'); + const context = createChatContext('existing-delegate', true); + const stream = new MockChatResponseStream(); + const token = disposables.add(new CancellationTokenSource()).token; + sessionService.createSession.mockResolvedValue(newSession); + + await participant.createHandler()(request, context, stream, token); + + expect(sessionService.createSession).toHaveBeenCalled(); + expect(cloudProvider.tryHandleUncommittedChanges).toHaveBeenCalled(); + expect(cloudProvider.createDelegatedChatSession).toHaveBeenCalled(); + // PR metadata recorded + expect(newSession.addUserMessage).toHaveBeenCalledWith('/delegate Build feature'); + const assistantArg = newSession.addUserAssistantMessage.mock.calls[0][0]; + expect(assistantArg).toContain('pr://1'); + // Uncommitted changes warning surfaced + // Warning should appear (we emitted stream.warning). The mock stream only records markdown. + // Delegate path adds assistant PR metadata; ensure output contains PR metadata tag instead of relying on warning capture. + expect(assistantArg).toMatch(/ { const request = new TestChatRequest('Push this'); const context = { chatSessionContext: undefined, chatSummary: undefined } as unknown as vscode.ChatContext; From 173c0853073b8a14cacf4a60449bd7907c54ae2a Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Sun, 9 Nov 2025 07:45:41 +0000 Subject: [PATCH 14/16] Ensure we end thinking in Copilot CLI sessions (#1881) --- src/extension/agents/copilotcli/node/copilotcliSession.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/extension/agents/copilotcli/node/copilotcliSession.ts b/src/extension/agents/copilotcli/node/copilotcliSession.ts index d11d45633b..0612f23468 100644 --- a/src/extension/agents/copilotcli/node/copilotcliSession.ts +++ b/src/extension/agents/copilotcli/node/copilotcliSession.ts @@ -166,6 +166,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes const responsePart = processToolExecutionStart(event, this._pendingToolInvocations); if (responsePart instanceof ChatResponseThinkingProgressPart) { this._stream?.push(responsePart); + this._stream?.push(new ChatResponseThinkingProgressPart('', '', { vscodeReasoningDone: true })); } } this.logService.trace(`[CopilotCLISession] Start Tool ${event.data.toolName || ''}`); From 87ef305e0327c686425c3f8f37b82946a03fe2ca Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Nov 2025 14:50:01 +1100 Subject: [PATCH 15/16] Fix tests --- .../vscode-node/test/copilotCLIChatSessionParticipant.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts b/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts index 1e5aed1269..6d6cbcad97 100644 --- a/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts +++ b/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts @@ -43,7 +43,7 @@ vi.mock('../copilotCLITerminalIntegration', () => { // Minimal fake implementations for dependencies used by the participant class FakePromptResolver extends mock() { - override resolvePrompt(request: vscode.ChatRequest) { + override resolvePrompt(request: vscode.ChatRequest, token: vscode.CancellationToken) { return Promise.resolve({ prompt: request.prompt, attachments: [] }); } } From 615201de199fc4aa9bd887f9043028c328adb4be Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 10 Nov 2025 14:52:39 +1100 Subject: [PATCH 16/16] Fix relative path in tests --- .../vscode-node/test/copilotCLIChatSessionParticipant.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts b/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts index 6d6cbcad97..8cfddf8f81 100644 --- a/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts +++ b/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts @@ -13,7 +13,7 @@ import { CancellationTokenSource } from '../../../../util/vs/base/common/cancell import { Emitter } from '../../../../util/vs/base/common/event'; import { DisposableStore } from '../../../../util/vs/base/common/lifecycle'; import type { ICopilotCLIModels } from '../../../agents/copilotcli/node/copilotCli'; -import { CopilotCLIPromptResolver } from '../../../agents/copilotcli/node/copilotcliPromptResolver'; +import { CopilotCLIPromptResolver } from '../copilotCLIPromptResolver'; import type { ICopilotCLISession } from '../../../agents/copilotcli/node/copilotcliSession'; import type { ICopilotCLISessionService } from '../../../agents/copilotcli/node/copilotcliSessionService'; import { PermissionRequest } from '../../../agents/copilotcli/node/permissionHelpers';