From a6332d4852d84436023d5ed7d311d0df2ec95d09 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Sat, 8 Nov 2025 17:39:24 +1100 Subject: [PATCH 1/6] More Copilot CLI tests and refactor permissions --- .../agents/copilotcli/node/copilotCli.ts | 70 ++-- .../copilotcli/node/copilotcliSession.ts | 123 +++++-- .../node/copilotcliSessionService.ts | 45 ++- .../copilotcli/node/permissionHelpers.ts | 17 + .../test/copilotCliSessionService.spec.ts | 17 +- .../node/test/copilotcliSession.spec.ts | 211 ++++++----- .../copilotCLIChatSessionsContribution.ts | 191 +++++----- .../copilotCloudSessionsProvider.ts | 5 +- .../copilotCLIChatSessionParticipant.spec.ts | 347 ++++++++++++++++++ 9 files changed, 752 insertions(+), 274 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..6de36436f0 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,16 @@ export class CopilotCLISessionOptionsService implements ICopilotCLISessionOption if (workingDirectory) { allOptions.workingDirectory = workingDirectory; } - return allOptions; + + return { + addPermissionHandler: (handler: NonNullable) => { + permissionHandler.requestPermission = handler; + return toDisposable(() => { + permissionHandler.requestPermission = requestPermissionRejected; + }); + }, + toSessionOptions: () => allOptions + } satisfies CopilotCLISessionOptions; } private async getWorkspaceFolderPath() { if (this.workspaceService.getWorkspaceFolders().length === 0) { @@ -154,32 +179,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..5675377745 100644 --- a/src/extension/agents/copilotcli/node/copilotcliSession.ts +++ b/src/extension/agents/copilotcli/node/copilotcliSession.ts @@ -3,35 +3,41 @@ * 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; + attchStream(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; @@ -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; } + attchStream(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,26 @@ 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, sessionOptions] = await Promise.all([ modelId ? this._sdkSession.getSelectedModel() : undefined, - this.cliSessionOptions.createOptions(this._options, this._permissionHandler) + this.cliSessionOptions.createOptions({}) ]); - if (sessionOptions.authInfo) { - this._sdkSession.setAuthInfo(sessionOptions.authInfo); + const autoInfo = sessionOptions.toSessionOptions().authInfo; + if (autoInfo) { + this._sdkSession.setAuthInfo(autoInfo); } if (modelId && modelId !== currentModel) { await this._sdkSession.setSelectedModel(modelId); @@ -116,7 +146,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 +166,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 +181,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 +193,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 +205,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(); } @@ -197,17 +227,16 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes } public async getChatHistory(): Promise<(ChatRequestTurn2 | ChatResponseTurn2)[]> { - const events = await this._sdkSession.getEvents(); + 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 +268,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..0531218d1a 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; } @@ -132,6 +131,9 @@ 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 || !label) { + return 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) || 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,11 +232,10 @@ 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); if (!sdkSession) { @@ -250,14 +244,14 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS 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..ac5a6dccaf 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 () => []), + addPermissiongHandler: 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..6f382194cf 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.attchStream(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.attchStream(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.attchStream(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.attchStream(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.attchStream(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.attchStream(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.attchStream(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.attchStream(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.attchStream(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.attchStream(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.attchStream(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.attchStream(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 b11d6531d5..927900128c 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'; @@ -16,9 +17,11 @@ 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 { PermissionRequest, requestPermission } from '../../agents/copilotcli/node/permissionHelpers'; import { ChatSummarizerProvider } from '../../prompt/node/summarizer'; +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; @@ -44,7 +48,7 @@ export class CopilotCLIWorktreeManager { 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; + const worktreePath = await this.commandExecutionService.executeCommand('git.createWorktreeWithDefaults') as string | undefined; if (worktreePath) { return vscode.l10n.t('Created isolated worktree at {0}', worktreePath); } else { @@ -138,6 +142,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); @@ -159,7 +164,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; } @@ -302,6 +307,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 { @@ -310,81 +317,98 @@ 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." } + } + */ + 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) { + /* Invoked from a 'normal' chat or 'cloud button' without CLI session context */ + // Handle confirmation data + return await this.handlePushConfirmationData(request, context, token); + } + 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) + ]); - /* __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." } + const session = await this.getOrCreateSession(request, chatSessionContext, prompt, modelId, stream, disposables, token); + if (!session) { + return {}; } - */ - 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.')); - return {}; + if (!isUntitled && confirmationResults.length) { + return await this.handleConfirmationData(session, request.prompt, confirmationResults, context, stream, token); } - /* Invoked from a 'normal' chat or 'cloud button' without CLI session context */ - // Handle confirmation data - return await this.handlePushConfirmationData(request, context, stream, token); + if (request.prompt.startsWith('/delegate')) { + await this.handleDelegateCommand(session, request, context, stream, token); + } else { + await session.handleRequest(prompt, attachments, modelId, token); + } + + if (isUntitled) { + this.sessionItemProvider.swap(chatSessionContext.chatSessionItem, { resource: SessionIdForCLI.getResource(session.sessionId), label: request.prompt ?? 'CopilotCLI' }); + } + return {}; } + finally { + disposables.dispose(); + } + } - const defaultModel = await this.copilotCLIModels.getDefaultModel(); + 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 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 (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); - } - - await session.handleRequest(prompt, attachments, modelId, stream, request.toolInvocationToken, token); - this.sessionItemProvider.swap(chatSessionContext.chatSessionItem, { resource: SessionIdForCLI.getResource(session.sessionId), label: request.prompt ?? 'CopilotCLI' }); + const workingDirectory = chatSessionContext.isUntitled ? + await this.worktreeManager.createWorktreeIfNeeded(id, stream) : + this.worktreeManager.getWorktreePath(id); + const session = chatSessionContext.isUntitled ? + await this.sessionService.createSession(prompt, modelId, workingDirectory, token) : + await this.sessionService.getSession(id, undefined, workingDirectory, false, token); - return {}; - } - - 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.attchStream(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 @@ -408,38 +432,39 @@ 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 {}; } @@ -447,7 +472,6 @@ export class CopilotCLIChatSessionParticipant { private async handlePushConfirmationData( request: vscode.ChatRequest, context: vscode.ChatContext, - stream: vscode.ChatResponseStream, token: vscode.CancellationToken ): Promise { const prompt = request.prompt; @@ -456,16 +480,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 1df1bc2db1..9b7ac7d179 100644 --- a/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts +++ b/src/extension/chatSessions/vscode-node/copilotCloudSessionsProvider.ts @@ -20,6 +20,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; @@ -468,7 +469,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.')); @@ -552,7 +553,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..e834e5b38b --- /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 attchStream = 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(async () => []); + 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 ec07334b8342775c0fad620fcee9627a78e034ef Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Sat, 8 Nov 2025 19:24:44 +1100 Subject: [PATCH 2/6] Updates --- src/extension/agents/copilotcli/node/copilotCli.ts | 4 +++- src/extension/agents/copilotcli/node/copilotcliSession.ts | 4 ++-- .../agents/copilotcli/node/copilotcliSessionService.ts | 4 ++-- .../vscode-node/copilotCLIChatSessionsContribution.ts | 7 +++++-- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/extension/agents/copilotcli/node/copilotCli.ts b/src/extension/agents/copilotcli/node/copilotCli.ts index 6de36436f0..d4ed5225db 100644 --- a/src/extension/agents/copilotcli/node/copilotCli.ts +++ b/src/extension/agents/copilotcli/node/copilotCli.ts @@ -162,7 +162,9 @@ export class CopilotCLISessionOptionsService implements ICopilotCLISessionOption addPermissionHandler: (handler: NonNullable) => { permissionHandler.requestPermission = handler; return toDisposable(() => { - permissionHandler.requestPermission = requestPermissionRejected; + if (permissionHandler.requestPermission === handler) { + permissionHandler.requestPermission = requestPermissionRejected; + } }); }, toSessionOptions: () => allOptions diff --git a/src/extension/agents/copilotcli/node/copilotcliSession.ts b/src/extension/agents/copilotcli/node/copilotcliSession.ts index 5675377745..fd22503194 100644 --- a/src/extension/agents/copilotcli/node/copilotcliSession.ts +++ b/src/extension/agents/copilotcli/node/copilotcliSession.ts @@ -131,11 +131,11 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes })); try { - const [currentModel, sessionOptions] = await Promise.all([ + const [currentModel, options] = await Promise.all([ modelId ? this._sdkSession.getSelectedModel() : undefined, this.cliSessionOptions.createOptions({}) ]); - const autoInfo = sessionOptions.toSessionOptions().authInfo; + const autoInfo = options.toSessionOptions().authInfo; if (autoInfo) { this._sdkSession.setAuthInfo(autoInfo); } diff --git a/src/extension/agents/copilotcli/node/copilotcliSessionService.ts b/src/extension/agents/copilotcli/node/copilotcliSessionService.ts index 0531218d1a..6501486802 100644 --- a/src/extension/agents/copilotcli/node/copilotcliSessionService.ts +++ b/src/extension/agents/copilotcli/node/copilotcliSessionService.ts @@ -130,10 +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 || !label) { + if (noUserMessages) { return undefined; } + const label = await this._generateSessionLabel(session.sessionId, chatMessages, undefined); // Get timestamp from last SDK event, or fallback to metadata.startTime const sdkEvents = session.getEvents(); diff --git a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts index 927900128c..0eeb889024 100644 --- a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts +++ b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts @@ -337,6 +337,10 @@ export class CopilotCLIChatSessionParticipant { 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); @@ -359,7 +363,7 @@ export class CopilotCLIChatSessionParticipant { return await this.handleConfirmationData(session, request.prompt, confirmationResults, context, stream, token); } - if (request.prompt.startsWith('/delegate')) { + if (!isUntitled && request.prompt.startsWith('/delegate')) { await this.handleDelegateCommand(session, request, context, stream, token); } else { await session.handleRequest(prompt, attachments, modelId, token); @@ -462,7 +466,6 @@ export class CopilotCLIChatSessionParticipant { history: uncommittedChangesData.metadata.history, chatContext: context }, stream, token); - if (prInfo) { await this.recordPushToSession(session, prompt, prInfo); } From cb94146df4433361a3d7a415577f6389a83b54ed Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Sat, 8 Nov 2025 20:51:21 +1100 Subject: [PATCH 3/6] Address review comments --- .../copilotcli/node/copilotcliSession.ts | 17 +++++++------ .../node/test/copilotcliSession.spec.ts | 24 +++++++++---------- .../copilotCLIChatSessionsContribution.ts | 4 ++-- .../copilotCLIChatSessionParticipant.spec.ts | 4 ++-- 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/src/extension/agents/copilotcli/node/copilotcliSession.ts b/src/extension/agents/copilotcli/node/copilotcliSession.ts index fd22503194..d11d45633b 100644 --- a/src/extension/agents/copilotcli/node/copilotcliSession.ts +++ b/src/extension/agents/copilotcli/node/copilotcliSession.ts @@ -31,7 +31,7 @@ export interface ICopilotCLISession extends IDisposable { readonly onPermissionRequested: vscode.Event; attachPermissionHandler(handler: PermissionHandler): IDisposable; - attchStream(stream: vscode.ChatResponseStream): IDisposable; + attachStream(stream: vscode.ChatResponseStream): IDisposable; handleRequest( prompt: string, attachments: Attachment[], @@ -41,7 +41,7 @@ export interface ICopilotCLISession extends IDisposable { addUserMessage(content: string): void; addUserAssistantMessage(content: string): void; getSelectedModelId(): Promise; - getChatHistory(): Promise<(ChatRequestTurn2 | ChatResponseTurn2)[]>; + getChatHistory(): (ChatRequestTurn2 | ChatResponseTurn2)[]; } export class CopilotCLISession extends DisposableStore implements ICopilotCLISession { @@ -75,7 +75,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes this.sessionId = _sdkSession.sessionId; } - attchStream(stream: vscode.ChatResponseStream): IDisposable { + attachStream(stream: vscode.ChatResponseStream): IDisposable { this._stream = stream; return toDisposable(() => { if (this._stream === stream) { @@ -131,13 +131,12 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes })); try { - const [currentModel, options] = await Promise.all([ + const [currentModel, authInfo] = await Promise.all([ modelId ? this._sdkSession.getSelectedModel() : undefined, - this.cliSessionOptions.createOptions({}) + this.cliSessionOptions.createOptions({}).then(opts => opts.toSessionOptions().authInfo) ]); - const autoInfo = options.toSessionOptions().authInfo; - if (autoInfo) { - this._sdkSession.setAuthInfo(autoInfo); + if (authInfo) { + this._sdkSession.setAuthInfo(authInfo); } if (modelId && modelId !== currentModel) { await this._sdkSession.setSelectedModel(modelId); @@ -226,7 +225,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes return this._sdkSession.getSelectedModel(); } - public async getChatHistory(): Promise<(ChatRequestTurn2 | ChatResponseTurn2)[]> { + public getChatHistory(): (ChatRequestTurn2 | ChatResponseTurn2)[] { const events = this._sdkSession.getEvents(); return buildChatHistoryFromEvents(events); } diff --git a/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts b/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts index 6f382194cf..e27bd354cf 100644 --- a/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts +++ b/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts @@ -139,7 +139,7 @@ describe('CopilotCLISession', () => { const stream = new MockChatResponseStream(); // Attach stream first, then invoke with new signature (no stream param) - session.attchStream(stream); + session.attachStream(stream); await session.handleRequest('Hello', [], undefined, CancellationToken.None); expect(session.status).toBe(ChatSessionStatus.Completed); @@ -150,7 +150,7 @@ describe('CopilotCLISession', () => { it('switches model when different modelId provided', async () => { const session = await createSession(); const stream = new MockChatResponseStream(); - session.attchStream(stream); + session.attachStream(stream); await session.handleRequest('Hi', [], 'modelB', CancellationToken.None); expect(sdkSession._selectedModel).toBe('modelB'); @@ -161,7 +161,7 @@ describe('CopilotCLISession', () => { sdkSession.send = async () => { throw new Error('network'); }; const session = await createSession(); const stream = new MockChatResponseStream(); - session.attchStream(stream); + session.attachStream(stream); await session.handleRequest('Boom', [], undefined, CancellationToken.None); expect(session.status).toBe(ChatSessionStatus.Failed); @@ -173,7 +173,7 @@ describe('CopilotCLISession', () => { const statuses: (ChatSessionStatus | undefined)[] = []; const listener = disposables.add(session.onDidChangeStatus(s => statuses.push(s))); const stream = new MockChatResponseStream(); - session.attchStream(stream); + session.attachStream(stream); await session.handleRequest('Status OK', [], 'modelA', CancellationToken.None); listener.dispose?.(); @@ -188,7 +188,7 @@ describe('CopilotCLISession', () => { const statuses: (ChatSessionStatus | undefined)[] = []; const listener = disposables.add(session.onDidChangeStatus(s => statuses.push(s))); const stream = new MockChatResponseStream(); - session.attchStream(stream); + session.attachStream(stream); await session.handleRequest('Will Fail', [], undefined, CancellationToken.None); listener.dispose?.(); @@ -207,7 +207,7 @@ describe('CopilotCLISession', () => { }); const session = await createSession(); const stream = new MockChatResponseStream(); - session.attchStream(stream); + session.attachStream(stream); const handlePromise = session.handleRequest('Test', [], undefined, CancellationToken.None); // Path must be absolute within workspace, should auto-approve @@ -228,7 +228,7 @@ describe('CopilotCLISession', () => { }); const session = await createSession(); const stream = new MockChatResponseStream(); - session.attchStream(stream); + session.attachStream(stream); const handlePromise = session.handleRequest('Test', [], undefined, CancellationToken.None); // Path must be absolute within workspace, should auto-approve @@ -249,7 +249,7 @@ describe('CopilotCLISession', () => { }); const session = await createSession(); const stream = new MockChatResponseStream(); - session.attchStream(stream); + session.attachStream(stream); disposables.add(session.attachPermissionHandler((permission) => { askedForPermission = permission; @@ -279,7 +279,7 @@ describe('CopilotCLISession', () => { sdkSession.emit('assistant.turn_end', {}); }); const stream = new MockChatResponseStream(); - session.attchStream(stream); + 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: '' }); @@ -298,7 +298,7 @@ describe('CopilotCLISession', () => { sdkSession.emit('assistant.turn_end', {}); }); const stream = new MockChatResponseStream(); - session.attchStream(stream); + session.attachStream(stream); const handlePromise = session.handleRequest('Write', [], undefined, CancellationToken.None); const result = await sessionOptions.toSessionOptions().requestPermission!({ kind: 'write', fileName: 'b.ts', intention: 'Update file', diff: '' }); @@ -317,7 +317,7 @@ describe('CopilotCLISession', () => { sdkSession.emit('assistant.turn_end', {}); }); const stream = new MockChatResponseStream(); - session.attchStream(stream); + session.attachStream(stream); const handlePromise = session.handleRequest('Write', [], undefined, CancellationToken.None); const result = await sessionOptions.toSessionOptions().requestPermission!({ kind: 'write', fileName: 'err.ts', intention: 'Update file', diff: '' }); @@ -333,7 +333,7 @@ describe('CopilotCLISession', () => { const session = await createSession(); session.attachPermissionHandler(async () => true); const stream = new MockChatResponseStream(); - session.attchStream(stream); + 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) { diff --git a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts index 0eeb889024..7de1c724c6 100644 --- a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts +++ b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts @@ -236,7 +236,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); @@ -395,7 +395,7 @@ export class CopilotCLIChatSessionParticipant { stream.warning(vscode.l10n.t('Chat session not found.')); return undefined; } - disposables.add(session.attchStream(stream)); + disposables.add(session.attachStream(stream)); disposables.add(session.attachPermissionHandler(async (permissionRequest: PermissionRequest) => requestPermission(permissionRequest, this.toolsService, request.toolInvocationToken, token))); diff --git a/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts b/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts index e834e5b38b..274e908d86 100644 --- a/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts +++ b/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts @@ -71,12 +71,12 @@ class FakeCopilotCLISession implements ICopilotCLISession { public onDidChangeStatus: any = () => ({ dispose() { } }); public attachPermissionHandler = vi.fn(() => ({ dispose() { } })); // Implementation uses the (typo'd) method name `attchStream`. - public attchStream = vi.fn(() => ({ dispose() { } })); + 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(async () => []); + public getChatHistory = vi.fn(() => []); constructor(id: string) { this.sessionId = id; } onPermissionRequested: vscode.Event = () => ({ dispose() { } }); dispose(): void { From d388ffde0dd5d5a227c8e03f92918aee685badfc Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Sat, 8 Nov 2025 20:53:18 +1100 Subject: [PATCH 4/6] Updates --- .../agents/copilotcli/node/copilotcliSessionService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/extension/agents/copilotcli/node/copilotcliSessionService.ts b/src/extension/agents/copilotcli/node/copilotcliSessionService.ts index 6501486802..c7e3bb94c7 100644 --- a/src/extension/agents/copilotcli/node/copilotcliSessionService.ts +++ b/src/extension/agents/copilotcli/node/copilotcliSessionService.ts @@ -133,7 +133,7 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS if (noUserMessages) { return undefined; } - const label = await this._generateSessionLabel(session.sessionId, chatMessages, undefined); + const label = this._generateSessionLabel(session.sessionId, chatMessages, undefined); // Get timestamp from last SDK event, or fallback to metadata.startTime const sdkEvents = session.getEvents(); @@ -195,7 +195,7 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS const sessionManager = await raceCancellationError(this.getSessionManager(), token); const sdkSession = await sessionManager.createSession(options.toSessionOptions()); const chatMessages = await sdkSession.getChatContextMessages(); - const label = this._generateSessionLabel(sdkSession.sessionId, chatMessages, prompt) || prompt; + const label = this._generateSessionLabel(sdkSession.sessionId, chatMessages, prompt); const newSession: ICopilotCLISessionItem = { id: sdkSession.sessionId, label, From 04f99ceb0dc7115d2be5707f8ed43d6ba7fe797c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Sat, 8 Nov 2025 20:54:57 +1100 Subject: [PATCH 5/6] Address review comments --- .../copilotcli/node/test/copilotCliSessionService.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts b/src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts index ac5a6dccaf..63f6ec2bd1 100644 --- a/src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts +++ b/src/extension/agents/copilotcli/node/test/copilotCliSessionService.spec.ts @@ -110,7 +110,7 @@ describe('CopilotCLISessionService', () => { addUserAssistantMessage: vi.fn(), getSelectedModelId: vi.fn(async () => 'gpt-test'), getChatHistory: vi.fn(async () => []), - addPermissiongHandler: vi.fn(() => ({ dispose() { } })), + attachPermissionHandler: vi.fn(() => ({ dispose() { } })), get isDisposed() { return disposables.isDisposed; }, dispose: () => { disposables.dispose(); }, add: (d: IDisposable) => disposables.add(d) From d71058912ca073158df84a63cbf63f628ca6f7bd Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Sat, 8 Nov 2025 20:55:48 +1100 Subject: [PATCH 6/6] Address review comments --- .../agents/copilotcli/node/copilotcliSessionService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extension/agents/copilotcli/node/copilotcliSessionService.ts b/src/extension/agents/copilotcli/node/copilotcliSessionService.ts index c7e3bb94c7..3c15f1397e 100644 --- a/src/extension/agents/copilotcli/node/copilotcliSessionService.ts +++ b/src/extension/agents/copilotcli/node/copilotcliSessionService.ts @@ -237,7 +237,7 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS workingDirectory }), 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();