From 819307572fec1e230b1e8d2359c5c43ba0b77ccb Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 25 Nov 2025 14:21:54 +1100 Subject: [PATCH] Restore edits for background sessions --- .../agents/common/externalEditTracker.ts | 6 +- .../copilotcli/common/copilotCLITools.ts | 58 ++++++++++++------- .../common/test/copilotCLITools.spec.ts | 11 ++-- .../agents/copilotcli/node/copilotCli.ts | 26 ++++++++- .../copilotcli/node/copilotcliSession.ts | 33 ++++++++--- .../node/test/copilotcliSession.spec.ts | 24 ++++---- .../claudeChatSessionContentProvider.ts | 2 +- .../copilotCLIChatSessionsContribution.ts | 2 +- .../copilotCloudSessionContentBuilder.ts | 3 +- .../copilotCLIChatSessionParticipant.spec.ts | 2 +- .../common/responseStreamWithLinkification.ts | 2 +- .../node/chatParticipantRequestHandler.ts | 5 +- .../vscode-node/chatReplaySessionProvider.ts | 2 +- src/extension/test/node/testHelpers.ts | 5 +- ...ode.proposed.chatParticipantAdditions.d.ts | 7 ++- ...scode.proposed.chatParticipantPrivate.d.ts | 6 +- src/util/common/chatResponseStreamImpl.ts | 4 +- src/util/common/test/shims/chatTypes.ts | 11 ++-- test/e2e/cli.stest.ts | 30 +++++----- 19 files changed, 154 insertions(+), 85 deletions(-) diff --git a/src/extension/agents/common/externalEditTracker.ts b/src/extension/agents/common/externalEditTracker.ts index 0f7a0054f1..bbf7a7391b 100644 --- a/src/extension/agents/common/externalEditTracker.ts +++ b/src/extension/agents/common/externalEditTracker.ts @@ -14,7 +14,7 @@ import { IDisposable } from '../../../util/vs/base/common/lifecycle'; * externalEdit API to ensure proper tracking and attribution of file changes. */ export class ExternalEditTracker { - private _ongoingEdits = new Map void; onDidComplete: Thenable }>(); + private _ongoingEdits = new Map void; onDidComplete: Thenable }>(); /** * Starts tracking an external edit operation. @@ -65,12 +65,12 @@ export class ExternalEditTracker { * @param editKey Unique identifier for the edit operation to complete * @returns Promise that resolves when VS Code has finished tracking the edit */ - public async completeEdit(editKey: string): Promise { + public async completeEdit(editKey: string): Promise { const ongoingEdit = this._ongoingEdits.get(editKey); if (ongoingEdit) { this._ongoingEdits.delete(editKey); ongoingEdit.complete(); - await ongoingEdit.onDidComplete; + return await ongoingEdit.onDidComplete; } } } diff --git a/src/extension/agents/copilotcli/common/copilotCLITools.ts b/src/extension/agents/copilotcli/common/copilotCLITools.ts index f46af0366e..e3876e7daa 100644 --- a/src/extension/agents/copilotcli/common/copilotCLITools.ts +++ b/src/extension/agents/copilotcli/common/copilotCLITools.ts @@ -9,7 +9,7 @@ import type { ChatPromptReference, ChatTerminalToolInvocationData, ExtendedChatR import { isLocation } from '../../../../util/common/types'; import { ResourceSet } from '../../../../util/vs/base/common/map'; import { URI } from '../../../../util/vs/base/common/uri'; -import { ChatRequestTurn2, ChatResponseMarkdownPart, ChatResponsePullRequestPart, ChatResponseThinkingProgressPart, ChatResponseTurn2, ChatToolInvocationPart, MarkdownString, Uri } from '../../../../vscodeTypes'; +import { ChatRequestTurn2, ChatResponseCodeblockUriPart, ChatResponseMarkdownPart, ChatResponsePullRequestPart, ChatResponseTextEditPart, ChatResponseThinkingProgressPart, ChatResponseTurn2, ChatToolInvocationPart, MarkdownString, Uri } from '../../../../vscodeTypes'; import { formatUriForFileWidget } from '../../../tools/common/toolUtils'; import { extractChatPromptReferences, getFolderAttachmentPath } from './copilotCLIPrompt'; @@ -283,12 +283,14 @@ function extractPRMetadata(content: string): { cleanedContent: string; prPart?: * Build chat history from SDK events for VS Code chat session * Converts SDKEvents into ChatRequestTurn2 and ChatResponseTurn2 objects */ -export function buildChatHistoryFromEvents(events: readonly SessionEvent[]): (ChatRequestTurn2 | ChatResponseTurn2)[] { +export function buildChatHistoryFromEvents(events: readonly SessionEvent[], getVSCodeRequestId?: (sdkRequestId: string) => { requestId: string; toolIdEditMap: Record } | undefined): (ChatRequestTurn2 | ChatResponseTurn2)[] { const turns: (ChatRequestTurn2 | ChatResponseTurn2)[] = []; let currentResponseParts: ExtendedChatResponsePart[] = []; - const pendingToolInvocations = new Map(); + const pendingToolInvocations = new Map(); + let details: { requestId: string; toolIdEditMap: Record } | undefined; for (const event of events) { + details = getVSCodeRequestId?.(event.id) ?? details; switch (event.type) { case 'user.message': { // Flush any pending response parts before adding user message @@ -338,7 +340,7 @@ export function buildChatHistoryFromEvents(events: readonly SessionEvent[]): (Ch range }); }); - turns.push(new ChatRequestTurn2(stripReminders(event.data.content || ''), undefined, references, '', [], undefined)); + turns.push(new ChatRequestTurn2(stripReminders(event.data.content || ''), undefined, references, '', [], undefined, details?.requestId)); break; } case 'assistant.message': { @@ -367,9 +369,21 @@ export function buildChatHistoryFromEvents(events: readonly SessionEvent[]): (Ch break; } case 'tool.execution_complete': { - const responsePart = processToolExecutionComplete(event, pendingToolInvocations); - if (responsePart && !(responsePart instanceof ChatResponseThinkingProgressPart)) { - currentResponseParts.push(responsePart); + const [responsePart, toolCall] = processToolExecutionComplete(event, pendingToolInvocations) ?? [undefined, undefined]; + if (responsePart && toolCall && !(responsePart instanceof ChatResponseThinkingProgressPart)) { + const editId = details?.toolIdEditMap ? details.toolIdEditMap[toolCall.toolCallId] : undefined; + const editedUris = getAffectedUrisForEditTool(toolCall); + if (isCopilotCliEditToolCall(toolCall) && editId && editedUris.length > 0) { + for (const uri of editedUris) { + currentResponseParts.push(new ChatResponseMarkdownPart('\n````\n')); + currentResponseParts.push(new ChatResponseCodeblockUriPart(uri, true, editId)); + currentResponseParts.push(new ChatResponseMarkdownPart('\n````\n')); + currentResponseParts.push(new ChatResponseTextEditPart(uri, [])); + currentResponseParts.push(new ChatResponseTextEditPart(uri, true)); + } + } else { + currentResponseParts.push(responsePart); + } } break; } @@ -393,27 +407,27 @@ function getRangeInPrompt(prompt: string, referencedName: string): [number, numb return undefined; } -export function processToolExecutionStart(event: ToolExecutionStartEvent, pendingToolInvocations: Map): ChatToolInvocationPart | ChatResponseThinkingProgressPart | undefined { +export function processToolExecutionStart(event: ToolExecutionStartEvent, pendingToolInvocations: Map): ChatToolInvocationPart | ChatResponseThinkingProgressPart | undefined { const toolInvocation = createCopilotCLIToolInvocation(event.data as ToolCall); if (toolInvocation) { // Store pending invocation to update with result later - pendingToolInvocations.set(event.data.toolCallId, toolInvocation); + pendingToolInvocations.set(event.data.toolCallId, [toolInvocation, event.data as ToolCall]); } return toolInvocation; } -export function processToolExecutionComplete(event: ToolExecutionCompleteEvent, pendingToolInvocations: Map): ChatToolInvocationPart | ChatResponseThinkingProgressPart | undefined { +export function processToolExecutionComplete(event: ToolExecutionCompleteEvent, pendingToolInvocations: Map): [ChatToolInvocationPart | ChatResponseThinkingProgressPart, toolData: ToolCall] | undefined { const invocation = pendingToolInvocations.get(event.data.toolCallId); pendingToolInvocations.delete(event.data.toolCallId); - if (invocation && invocation instanceof ChatToolInvocationPart) { - invocation.isComplete = true; - invocation.isError = !!event.data.error; - invocation.invocationMessage = event.data.error?.message || invocation.invocationMessage; + if (invocation && invocation[0] instanceof ChatToolInvocationPart) { + invocation[0].isComplete = true; + invocation[0].isError = !!event.data.error; + invocation[0].invocationMessage = event.data.error?.message || invocation[0].invocationMessage; if (!event.data.success && (event.data.error?.code === 'rejected' || event.data.error?.code === 'denied')) { - invocation.isConfirmed = false; + invocation[0].isConfirmed = false; } else { - invocation.isConfirmed = true; + invocation[0].isConfirmed = true; } } @@ -423,7 +437,7 @@ export function processToolExecutionComplete(event: ToolExecutionCompleteEvent, /** * Creates a formatted tool invocation part for CopilotCLI tools */ -export function createCopilotCLIToolInvocation(data: { toolCallId: string; toolName: string; arguments?: unknown }): ChatToolInvocationPart | ChatResponseThinkingProgressPart | undefined { +export function createCopilotCLIToolInvocation(data: { toolCallId: string; toolName: string; arguments?: unknown }, editId?: string): ChatToolInvocationPart | ChatResponseThinkingProgressPart | undefined { if (!Object.hasOwn(ToolFriendlyNameAndHandlers, data.toolName)) { const invocation = new ChatToolInvocationPart(data.toolName ?? 'unknown', data.toolCallId ?? '', false); invocation.isConfirmed = false; @@ -450,11 +464,11 @@ export function createCopilotCLIToolInvocation(data: { toolCallId: string; toolN invocation.isConfirmed = false; invocation.isComplete = false; - (formatter as Formatter)(invocation, toolCall); + (formatter as Formatter)(invocation, toolCall, editId); return invocation; } -type Formatter = (invocation: ChatToolInvocationPart, toolCall: ToolCall) => void; +type Formatter = (invocation: ChatToolInvocationPart, toolCall: ToolCall, editId?: string) => void; type ToolCallFor = Extract; const ToolFriendlyNameAndHandlers: { [K in ToolCall['toolName']]: [string, (invocation: ChatToolInvocationPart, toolCall: ToolCallFor) => void] } = { @@ -513,7 +527,7 @@ function formatViewToolInvocation(invocation: ChatToolInvocationPart, toolCall: } } -function formatStrReplaceEditorInvocation(invocation: ChatToolInvocationPart, toolCall: StringReplaceEditorTool): void { +function formatStrReplaceEditorInvocation(invocation: ChatToolInvocationPart, toolCall: StringReplaceEditorTool, editId?: string): void { if (!toolCall.arguments.path) { return; } @@ -554,7 +568,7 @@ function formatUndoEdit(invocation: ChatToolInvocationPart, toolCall: UndoEditTo } } -function formatEditToolInvocation(invocation: ChatToolInvocationPart, toolCall: EditTool): void { +function formatEditToolInvocation(invocation: ChatToolInvocationPart, toolCall: EditTool, editId?: string): void { const args = toolCall.arguments; const display = args.path ? formatUriForFileWidget(Uri.file(args.path)) : ''; @@ -564,7 +578,7 @@ function formatEditToolInvocation(invocation: ChatToolInvocationPart, toolCall: } -function formatCreateToolInvocation(invocation: ChatToolInvocationPart, toolCall: CreateTool): void { +function formatCreateToolInvocation(invocation: ChatToolInvocationPart, toolCall: CreateTool, editId?: string): void { const args = toolCall.arguments; const display = args.path ? formatUriForFileWidget(Uri.file(args.path)) : ''; diff --git a/src/extension/agents/copilotcli/common/test/copilotCLITools.spec.ts b/src/extension/agents/copilotcli/common/test/copilotCLITools.spec.ts index 54f8471be4..b3cd77bb54 100644 --- a/src/extension/agents/copilotcli/common/test/copilotCLITools.spec.ts +++ b/src/extension/agents/copilotcli/common/test/copilotCLITools.spec.ts @@ -20,7 +20,8 @@ import { isCopilotCliEditToolCall, processToolExecutionComplete, processToolExecutionStart, - stripReminders + stripReminders, + ToolCall } from '../copilotCLITools'; // Helper to extract invocation message text independent of MarkdownString vs string @@ -160,21 +161,21 @@ describe('CopilotCLITools', () => { describe('process tool execution lifecycle', () => { it('marks tool invocation complete and confirmed on success', () => { - const pending = new Map(); + const pending = new Map(); const startEvent: any = { type: 'tool.execution_start', data: { toolName: 'bash', toolCallId: 'bash-1', arguments: { command: 'echo hi' } } }; const part = processToolExecutionStart(startEvent, pending); expect(part).toBeInstanceOf(ChatToolInvocationPart); const completeEvent: any = { type: 'tool.execution_complete', data: { toolName: 'bash', toolCallId: 'bash-1', success: true } }; - const completed = processToolExecutionComplete(completeEvent, pending) as ChatToolInvocationPart; + const [completed,] = processToolExecutionComplete(completeEvent, pending)! as [ChatToolInvocationPart, ToolCall]; expect(completed.isComplete).toBe(true); expect(completed.isError).toBe(false); expect(completed.isConfirmed).toBe(true); }); it('marks tool invocation error and unconfirmed when denied', () => { - const pending = new Map(); + const pending = new Map(); processToolExecutionStart({ type: 'tool.execution_start', data: { toolName: 'bash', toolCallId: 'bash-2', arguments: { command: 'rm *' } } } as any, pending); const completeEvent: any = { type: 'tool.execution_complete', data: { toolName: 'bash', toolCallId: 'bash-2', success: false, error: { message: 'Denied', code: 'denied' } } }; - const completed = processToolExecutionComplete(completeEvent, pending) as ChatToolInvocationPart; + const [completed,] = processToolExecutionComplete(completeEvent, pending)! as [ChatToolInvocationPart, ToolCall]; expect(completed.isComplete).toBe(true); expect(completed.isError).toBe(true); expect(completed.isConfirmed).toBe(false); diff --git a/src/extension/agents/copilotcli/node/copilotCli.ts b/src/extension/agents/copilotcli/node/copilotCli.ts index 1ad1318e78..4100f1b425 100644 --- a/src/extension/agents/copilotcli/node/copilotCli.ts +++ b/src/extension/agents/copilotcli/node/copilotCli.ts @@ -19,6 +19,7 @@ import { PermissionRequest } from './permissionHelpers'; import { ensureRipgrepShim } from './ripgrepShim'; const COPILOT_CLI_MODEL_MEMENTO_KEY = 'github.copilot.cli.sessionModel'; +const COPILOT_CLI_REQUEST_MAP_KEY = 'github.copilot.cli.requestMap'; export class CopilotCLISessionOptions { public readonly isolationEnabled: boolean; @@ -142,10 +143,15 @@ export interface ICopilotCLISDK { readonly _serviceBrand: undefined; getPackage(): Promise; getAuthInfo(): Promise>; + + getRequestId(sdkRequestId: string): RequestDetails['details'] | undefined; + setRequestId(sdkRequestId: string, details: { requestId: string; toolIdEditMap: Record }): void; } +type RequestDetails = { details: { requestId: string; toolIdEditMap: Record }; createdDateTime: number }; export class CopilotCLISDK implements ICopilotCLISDK { declare _serviceBrand: undefined; + private requestMap: Record = {}; constructor( @IVSCodeExtensionContext private readonly extensionContext: IVSCodeExtensionContext, @@ -153,7 +159,25 @@ export class CopilotCLISDK implements ICopilotCLISDK { @ILogService private readonly logService: ILogService, @IInstantiationService protected readonly instantiationService: IInstantiationService, @IAuthenticationService private readonly authentService: IAuthenticationService, - ) { } + ) { + this.requestMap = this.extensionContext.workspaceState.get>(COPILOT_CLI_REQUEST_MAP_KEY, {}); + } + + getRequestId(sdkRequestId: string): RequestDetails['details'] | undefined { + return this.requestMap[sdkRequestId]?.details; + } + + setRequestId(sdkRequestId: string, details: { requestId: string; toolIdEditMap: Record }): void { + this.requestMap[sdkRequestId] = { details, createdDateTime: Date.now() }; + // Prune entries older than 7 days + const sevenDaysAgo = Date.now() - 7 * 24 * 60 * 60 * 1000; + for (const [key, value] of Object.entries(this.requestMap)) { + if (value.createdDateTime < sevenDaysAgo) { + delete this.requestMap[key]; + } + } + this.extensionContext.workspaceState.update(COPILOT_CLI_REQUEST_MAP_KEY, this.requestMap); + } public async getPackage(): Promise { try { diff --git a/src/extension/agents/copilotcli/node/copilotcliSession.ts b/src/extension/agents/copilotcli/node/copilotcliSession.ts index c2a0896765..9affe19fd2 100644 --- a/src/extension/agents/copilotcli/node/copilotcliSession.ts +++ b/src/extension/agents/copilotcli/node/copilotcliSession.ts @@ -15,7 +15,7 @@ import { DisposableStore, IDisposable, toDisposable } from '../../../../util/vs/ import { ResourceMap } from '../../../../util/vs/base/common/map'; import { extUriBiasedIgnorePathCase } from '../../../../util/vs/base/common/resources'; import { IInstantiationService } from '../../../../util/vs/platform/instantiation/common/instantiation'; -import { ChatRequestTurn2, ChatResponseThinkingProgressPart, ChatResponseTurn2, ChatSessionStatus, EventEmitter, Uri } from '../../../../vscodeTypes'; +import { ChatRequestTurn2, ChatResponseThinkingProgressPart, ChatResponseTurn2, ChatSessionStatus, ChatToolInvocationPart, EventEmitter, Uri } from '../../../../vscodeTypes'; import { ExternalEditTracker } from '../../common/externalEditTracker'; import { buildChatHistoryFromEvents, getAffectedUrisForEditTool, isCopilotCliEditToolCall, processToolExecutionComplete, processToolExecutionStart, ToolCall, UnknownToolCall } from '../common/copilotCLITools'; import { CopilotCLISessionOptions, ICopilotCLISDK } from './copilotCli'; @@ -37,6 +37,7 @@ export interface ICopilotCLISession extends IDisposable { attachPermissionHandler(handler: PermissionHandler): IDisposable; attachStream(stream: vscode.ChatResponseStream): IDisposable; handleRequest( + requestId: string, prompt: string, attachments: Attachment[], modelId: string | undefined, @@ -49,7 +50,6 @@ export interface ICopilotCLISession extends IDisposable { } export class CopilotCLISession extends DisposableStore implements ICopilotCLISession { - private _pendingToolInvocations = new Map(); public readonly sessionId: string; private _status?: vscode.ChatSessionStatus; public get status(): vscode.ChatSessionStatus | undefined { @@ -106,6 +106,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes } public async handleRequest( + requestId: string, prompt: string, attachments: Attachment[], modelId: string | undefined, @@ -124,11 +125,14 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes abortController.abort(); })); disposables.add(toDisposable(() => abortController.abort())); + const pendingToolInvocations = new Map(); const toolNames = new Map(); const editToolIds = new Set(); const toolCalls = new Map(); const editTracker = new ExternalEditTracker(); + let sdkRequestId: string | undefined; + const toolIdEditMap = new Map>(); const editFilesAndToolCallIds = new ResourceMap(); disposables.add(this._options.addPermissionHandler(async (permissionRequest) => { // Need better API from SDK to correlate file edits in permission requests to tool invocations. @@ -154,6 +158,9 @@ 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('user.message', (event) => { + sdkRequestId = event.id; + }))); disposables.add(toDisposable(this._sdkSession.on('assistant.message', (event) => { if (typeof event.data.content === 'string' && event.data.content.length) { this._stream?.markdown(event.data.content); @@ -175,7 +182,7 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes }); } } else { - const responsePart = processToolExecutionStart(event, this._pendingToolInvocations); + const responsePart = processToolExecutionStart(event, pendingToolInvocations); if (responsePart instanceof ChatResponseThinkingProgressPart) { this._stream?.push(responsePart); this._stream?.push(new ChatResponseThinkingProgressPart('', '', { vscodeReasoningDone: true })); @@ -185,13 +192,13 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes }))); disposables.add(toDisposable(this._sdkSession.on('tool.execution_complete', (event) => { // Mark the end of the edit if this was an edit tool. - editTracker.completeEdit(event.data.toolCallId); + toolIdEditMap.set(event.data.toolCallId, editTracker.completeEdit(event.data.toolCallId)); if (editToolIds.has(event.data.toolCallId)) { this.logService.trace(`[CopilotCLISession] Completed edit tracking for toolCallId ${event.data.toolCallId}`); return; } - const responsePart = processToolExecutionComplete(event, this._pendingToolInvocations); + const [responsePart,] = processToolExecutionComplete(event, pendingToolInvocations) ?? []; if (responsePart && !(responsePart instanceof ChatResponseThinkingProgressPart)) { this._stream?.push(responsePart); } @@ -222,7 +229,16 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes this.logService.trace(`[CopilotCLISession] Staged all changes in working directory ${workingDirectory}`); } } - + const requestDetails: { requestId: string; toolIdEditMap: Record } = { requestId, toolIdEditMap: {} }; + await Promise.all(Array.from(toolIdEditMap.entries()).map(async ([toolId, editFilePromise]) => { + const editId = await editFilePromise.catch(() => undefined); + if (editId) { + requestDetails.toolIdEditMap[toolId] = editId; + } + })); + if (Object.keys(requestDetails.toolIdEditMap).length > 0 && sdkRequestId) { + this.copilotCLISDK.setRequestId(sdkRequestId, requestDetails); + } this._status = ChatSessionStatus.Completed; this._statusChange.fire(this._status); } catch (error) { @@ -252,7 +268,10 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes public getChatHistory(): (ChatRequestTurn2 | ChatResponseTurn2)[] { const events = this._sdkSession.getEvents(); - return buildChatHistoryFromEvents(events); + const getVSCodeRequestId = (sdkRequestId: string) => { + return this.copilotCLISDK.getRequestId(sdkRequestId); + }; + return buildChatHistoryFromEvents(events, getVSCodeRequestId); } private async requestPermission( diff --git a/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts b/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts index adee891aba..dde8a3d362 100644 --- a/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts +++ b/src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts @@ -126,7 +126,7 @@ describe('CopilotCLISession', () => { // Attach stream first, then invoke with new signature (no stream param) session.attachStream(stream); - await session.handleRequest('Hello', [], undefined, CancellationToken.None); + await session.handleRequest('', 'Hello', [], undefined, CancellationToken.None); expect(session.status).toBe(ChatSessionStatus.Completed); expect(stream.output.join('\n')).toContain('Echo: Hello'); @@ -137,7 +137,7 @@ describe('CopilotCLISession', () => { const session = await createSession(); const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('Hi', [], 'modelB', CancellationToken.None); + await session.handleRequest('', 'Hi', [], 'modelB', CancellationToken.None); expect(sdkSession._selectedModel).toBe('modelB'); }); @@ -148,7 +148,7 @@ describe('CopilotCLISession', () => { const session = await createSession(); const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('Boom', [], undefined, CancellationToken.None); + await session.handleRequest('', 'Boom', [], undefined, CancellationToken.None); expect(session.status).toBe(ChatSessionStatus.Failed); expect(stream.output.join('\n')).toContain('Error: network'); @@ -160,7 +160,7 @@ describe('CopilotCLISession', () => { const listener = disposables.add(session.onDidChangeStatus(s => statuses.push(s))); const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('Status OK', [], 'modelA', CancellationToken.None); + await session.handleRequest('', 'Status OK', [], 'modelA', CancellationToken.None); listener.dispose?.(); expect(statuses).toEqual([ChatSessionStatus.InProgress, ChatSessionStatus.Completed]); @@ -175,7 +175,7 @@ describe('CopilotCLISession', () => { const listener = disposables.add(session.onDidChangeStatus(s => statuses.push(s))); const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('Will Fail', [], undefined, CancellationToken.None); + await session.handleRequest('', 'Will Fail', [], undefined, CancellationToken.None); listener.dispose?.(); expect(statuses).toEqual([ChatSessionStatus.InProgress, ChatSessionStatus.Failed]); @@ -197,7 +197,7 @@ describe('CopilotCLISession', () => { session.attachStream(stream); // Path must be absolute within workspace, should auto-approve - await session.handleRequest('Test', [], undefined, CancellationToken.None); + await session.handleRequest('', 'Test', [], undefined, CancellationToken.None); expect(result).toEqual({ kind: 'approved' }); }); @@ -216,7 +216,7 @@ describe('CopilotCLISession', () => { session.attachStream(stream); // Path must be absolute within workspace, should auto-approve - await session.handleRequest('Test', [], undefined, CancellationToken.None); + await session.handleRequest('', 'Test', [], undefined, CancellationToken.None); expect(result).toEqual({ kind: 'approved' }); }); @@ -241,7 +241,7 @@ describe('CopilotCLISession', () => { })); // Path must be absolute within workspace, should auto-approve - await session.handleRequest('Test', [], undefined, CancellationToken.None); + await session.handleRequest('', 'Test', [], undefined, CancellationToken.None); const file = path.join('/workingDirectory', 'file.ts'); expect(result).toEqual({ kind: 'denied-interactively-by-user' }); expect(askedForPermission).not.toBeUndefined(); @@ -264,7 +264,7 @@ describe('CopilotCLISession', () => { const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('Write', [], undefined, CancellationToken.None); + await session.handleRequest('', 'Write', [], undefined, CancellationToken.None); expect(result).toEqual({ kind: 'approved' }); }); @@ -282,7 +282,7 @@ describe('CopilotCLISession', () => { }; const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('Write', [], undefined, CancellationToken.None); + await session.handleRequest('', 'Write', [], undefined, CancellationToken.None); expect(result).toEqual({ kind: 'denied-interactively-by-user' }); }); @@ -300,7 +300,7 @@ describe('CopilotCLISession', () => { }; const stream = new MockChatResponseStream(); session.attachStream(stream); - await session.handleRequest('Write', [], undefined, CancellationToken.None); + await session.handleRequest('', 'Write', [], undefined, CancellationToken.None); expect(result).toEqual({ kind: 'denied-interactively-by-user' }); }); @@ -322,7 +322,7 @@ describe('CopilotCLISession', () => { }); // Act: start handling request (do not await yet) - const requestPromise = session.handleRequest('Edits', [], undefined, 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)); diff --git a/src/extension/chatSessions/vscode-node/claudeChatSessionContentProvider.ts b/src/extension/chatSessions/vscode-node/claudeChatSessionContentProvider.ts index 7df77a8069..0f548877e8 100644 --- a/src/extension/chatSessions/vscode-node/claudeChatSessionContentProvider.ts +++ b/src/extension/chatSessions/vscode-node/claudeChatSessionContentProvider.ts @@ -46,7 +46,7 @@ export class ClaudeChatSessionContentProvider implements vscode.ChatSessionConte return; } - return new ChatRequestTurn2(textContent, undefined, [], '', [], undefined); + return new ChatRequestTurn2(textContent, undefined, [], '', [], undefined, undefined); } private _assistantMessageToResponse(message: SDKAssistantMessage['message'], toolContext: ToolContext): vscode.ChatResponseTurn2 { diff --git a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts index cfe612d79a..c57019f1be 100644 --- a/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts +++ b/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts @@ -442,7 +442,7 @@ export class CopilotCLIChatSessionParticipant extends Disposable { if (request.prompt.startsWith('/delegate')) { await this.handleDelegateCommand(session.object, request, context, stream, token); } else { - await session.object.handleRequest(prompt, attachments, modelId, token); + await session.object.handleRequest(request.id, prompt, attachments, modelId, token); } if (isUntitled && !token.isCancellationRequested) { diff --git a/src/extension/chatSessions/vscode-node/copilotCloudSessionContentBuilder.ts b/src/extension/chatSessions/vscode-node/copilotCloudSessionContentBuilder.ts index 024cb7ba7e..6dd9e9e496 100644 --- a/src/extension/chatSessions/vscode-node/copilotCloudSessionContentBuilder.ts +++ b/src/extension/chatSessions/vscode-node/copilotCloudSessionContentBuilder.ts @@ -128,7 +128,8 @@ export class ChatSessionContentBuilder { references, // references this.type, [], // toolReferences - [] + [], + undefined )); // Create the PR card right after problem statement for first session diff --git a/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts b/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts index 770e31e178..73178e9d1c 100644 --- a/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts +++ b/src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts @@ -94,7 +94,7 @@ function createChatContext(sessionId: string, isUntitled: boolean): vscode.ChatC class TestCopilotCLISession extends CopilotCLISession { public requests: Array<{ prompt: string; attachments: Attachment[]; modelId: string | undefined; token: vscode.CancellationToken }> = []; - override handleRequest(prompt: string, attachments: Attachment[], modelId: string | undefined, token: vscode.CancellationToken): Promise { + override handleRequest(requestId: string, prompt: string, attachments: Attachment[], modelId: string | undefined, token: vscode.CancellationToken): Promise { this.requests.push({ prompt, attachments, modelId, token }); return Promise.resolve(); } diff --git a/src/extension/linkify/common/responseStreamWithLinkification.ts b/src/extension/linkify/common/responseStreamWithLinkification.ts index e6e8f33457..36c18671ac 100644 --- a/src/extension/linkify/common/responseStreamWithLinkification.ts +++ b/src/extension/linkify/common/responseStreamWithLinkification.ts @@ -93,7 +93,7 @@ export class ResponseStreamWithLinkification implements FinalizableChatResponseS return this; } - externalEdit(target: Uri | Uri[], callback: () => Thenable): Thenable { + externalEdit(target: Uri | Uri[], callback: () => Thenable): Thenable { return this.enqueue(() => this._progress.externalEdit(target, callback), true); } diff --git a/src/extension/prompt/node/chatParticipantRequestHandler.ts b/src/extension/prompt/node/chatParticipantRequestHandler.ts index ba66ccfdb1..3b1406baad 100644 --- a/src/extension/prompt/node/chatParticipantRequestHandler.ts +++ b/src/extension/prompt/node/chatParticipantRequestHandler.ts @@ -381,19 +381,20 @@ function getResponseIdFromVSCodeChatHistoryTurn(turn: ChatRequestTurn | ChatResp */ function createTurnFromVSCodeChatHistoryTurns( accessor: ServicesAccessor, - chatRequestTurn: ChatRequestTurn2, + chatRequestTurn: ChatRequestTurn, chatResponseTurn: ChatResponseTurn ): Turn { const commandService = accessor.get(ICommandService); const workspaceService = accessor.get(IWorkspaceService); const instaService = accessor.get(IInstantiationService); + const chatRequestAsTurn2 = chatRequestTurn as ChatRequestTurn2; const currentTurn = new Turn( undefined, { message: chatRequestTurn.prompt, type: 'user' }, new ChatVariablesCollection(chatRequestTurn.references), chatRequestTurn.toolReferences.map(InternalToolReference.from), - chatRequestTurn.editedFileEvents + chatRequestAsTurn2.editedFileEvents ); // Take just the content messages diff --git a/src/extension/replay/vscode-node/chatReplaySessionProvider.ts b/src/extension/replay/vscode-node/chatReplaySessionProvider.ts index 5ee8d70b51..ff44870c9b 100644 --- a/src/extension/replay/vscode-node/chatReplaySessionProvider.ts +++ b/src/extension/replay/vscode-node/chatReplaySessionProvider.ts @@ -63,7 +63,7 @@ export class ChatReplaySessionProvider extends Disposable implements ChatSession } private createRequestTurn(step: ChatStep & { kind: 'userQuery' }): ChatRequestTurn2 { - return new ChatRequestTurn2(step.query, undefined, [], 'copilot', [], undefined); + return new ChatRequestTurn2(step.query, undefined, [], 'copilot', [], undefined, undefined); } fireSessionsChanged(): void { diff --git a/src/extension/test/node/testHelpers.ts b/src/extension/test/node/testHelpers.ts index e3e1d7d0a1..2ec7f61771 100644 --- a/src/extension/test/node/testHelpers.ts +++ b/src/extension/test/node/testHelpers.ts @@ -51,12 +51,13 @@ export class MockChatResponseStream extends ChatResponseStreamImpl { this.uris.push(uri.toString()); } - override externalEdit(target: Uri | Uri[], callback: () => Thenable): Thenable { + override async externalEdit(target: Uri | Uri[], callback: () => Thenable): Promise { if (Array.isArray(target)) { this.externalEditUris.push(...target); } else { this.externalEditUris.push(target); } - return callback(); + await callback(); + return ''; } } diff --git a/src/extension/vscode.proposed.chatParticipantAdditions.d.ts b/src/extension/vscode.proposed.chatParticipantAdditions.d.ts index f4ce44cd9e..86af47f60d 100644 --- a/src/extension/vscode.proposed.chatParticipantAdditions.d.ts +++ b/src/extension/vscode.proposed.chatParticipantAdditions.d.ts @@ -32,7 +32,8 @@ declare module 'vscode' { export class ChatResponseCodeblockUriPart { isEdit?: boolean; value: Uri; - constructor(value: Uri, isEdit?: boolean); + undoStopId?: string; + constructor(value: Uri, isEdit?: boolean, undoStopId?: string); } /** @@ -170,7 +171,7 @@ declare module 'vscode' { export class ChatResponseExternalEditPart { uris: Uri[]; callback: () => Thenable; - applied: Thenable; + applied: Thenable; constructor(uris: Uri[], callback: () => Thenable); } @@ -314,7 +315,7 @@ declare module 'vscode' { * tracked as agent edits. This can be used to track edits made from * external tools that don't generate simple {@link textEdit textEdits}. */ - externalEdit(target: Uri | Uri[], callback: () => Thenable): Thenable; + externalEdit(target: Uri | Uri[], callback: () => Thenable): Thenable; markdownWithVulnerabilities(value: string | MarkdownString, vulnerabilities: ChatVulnerability[]): void; codeblockUri(uri: Uri, isEdit?: boolean): void; diff --git a/src/extension/vscode.proposed.chatParticipantPrivate.d.ts b/src/extension/vscode.proposed.chatParticipantPrivate.d.ts index 9f9d0a4b6a..b3850fcdd3 100644 --- a/src/extension/vscode.proposed.chatParticipantPrivate.d.ts +++ b/src/extension/vscode.proposed.chatParticipantPrivate.d.ts @@ -106,6 +106,10 @@ declare module 'vscode' { * ChatRequestTurn + private additions. Note- at runtime this is the SAME as ChatRequestTurn and instanceof is safe. */ export class ChatRequestTurn2 { + /** + * The id of the chat request. Used to identity an interaction with any of the chat surfaces. + */ + readonly id?: string; /** * The prompt as entered by the user. * @@ -144,7 +148,7 @@ declare module 'vscode' { /** * @hidden */ - constructor(prompt: string, command: string | undefined, references: ChatPromptReference[], participant: string, toolReferences: ChatLanguageModelToolReference[], editedFileEvents: ChatRequestEditedFileEvent[] | undefined); + constructor(prompt: string, command: string | undefined, references: ChatPromptReference[], participant: string, toolReferences: ChatLanguageModelToolReference[], editedFileEvents: ChatRequestEditedFileEvent[] | undefined, id: string | undefined); } export class ChatResponseTurn2 { diff --git a/src/util/common/chatResponseStreamImpl.ts b/src/util/common/chatResponseStreamImpl.ts index 2af318c688..4cd501e0d9 100644 --- a/src/util/common/chatResponseStreamImpl.ts +++ b/src/util/common/chatResponseStreamImpl.ts @@ -99,10 +99,10 @@ export class ChatResponseStreamImpl implements FinalizableChatResponseStream { this._push(new ChatResponseFileTreePart(value, baseUri)); } - externalEdit(target: Uri | Uri[], callback: () => Thenable): Thenable { + async externalEdit(target: Uri | Uri[], callback: () => Thenable): Promise { const part = new ChatResponseExternalEditPart(target instanceof Array ? target : [target], callback); this._push(part); - return part.applied as Thenable; + return part.applied; } progress(value: string, task?: (progress: Progress) => Thenable): void { diff --git a/src/util/common/test/shims/chatTypes.ts b/src/util/common/test/shims/chatTypes.ts index 5cd3f32114..3e32164a2a 100644 --- a/src/util/common/test/shims/chatTypes.ts +++ b/src/util/common/test/shims/chatTypes.ts @@ -15,9 +15,12 @@ export class ChatResponseMarkdownPart { } export class ChatResponseCodeblockUriPart { + isEdit?: boolean; value: vscode.Uri; - constructor(value: vscode.Uri) { + undoStopId?: string; + constructor(value: vscode.Uri, isEdit?: boolean, undoStopId?: string) { this.value = value; + this.undoStopId = undoStopId; } } @@ -58,14 +61,14 @@ export class ChatResponseThinkingProgressPart { } export class ChatResponseExternalEditPart { - applied: Thenable; - didGetApplied!: () => void; + applied: Thenable; + didGetApplied!: (value: string) => void; constructor( public uris: vscode.Uri[], public callback: () => Thenable, ) { - this.applied = new Promise((resolve) => { + this.applied = new Promise((resolve) => { this.didGetApplied = resolve; }); } diff --git a/test/e2e/cli.stest.ts b/test/e2e/cli.stest.ts index 0f62cfb059..05a8893017 100644 --- a/test/e2e/cli.stest.ts +++ b/test/e2e/cli.stest.ts @@ -322,7 +322,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('What is 1+8?', [], undefined, CancellationToken.None); + await session.object.handleRequest('', 'What is 1+8?', [], undefined, CancellationToken.None); // Verify we have a response of 9. assert.strictEqual(session.object.status, ChatSessionStatus.Completed); @@ -330,7 +330,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { assertStreamContains(stream, '9'); // Can send a subsequent request. - await session.object.handleRequest('What is 11+25?', [], undefined, CancellationToken.None); + await session.object.handleRequest('', 'What is 11+25?', [], undefined, CancellationToken.None); // Verify we have a response of 36. assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -349,7 +349,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { const session = await sessionService.createSession('What is 1+8?', { workingDirectory }, CancellationToken.None); sessionId = session.object.sessionId; - await session.object.handleRequest('What is 1+8?', [], undefined, CancellationToken.None); + await session.object.handleRequest('', 'What is 1+8?', [], undefined, CancellationToken.None); session.dispose(); } @@ -369,7 +369,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest('What was my previous question?', [], undefined, CancellationToken.None); + await session.object.handleRequest('', 'What was my previous question?', [], undefined, CancellationToken.None); // Verify we have a response of 9. assert.strictEqual(session.object.status, ChatSessionStatus.Completed); @@ -388,7 +388,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest(prompt, [], undefined, CancellationToken.None); + await session.object.handleRequest('', prompt, [], undefined, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -419,7 +419,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { } })); - await session.object.handleRequest(prompt, [], undefined, CancellationToken.None); + await session.object.handleRequest('', prompt, [], undefined, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -442,7 +442,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest(prompt, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', prompt, attachments, undefined, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -464,7 +464,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest(prompt, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', prompt, attachments, undefined, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -478,7 +478,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { [], promptResolver )); - await session.object.handleRequest(prompt, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', prompt, attachments, undefined, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -504,7 +504,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest(prompt, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', prompt, attachments, undefined, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertStreamContains(stream, 'throw'); @@ -524,7 +524,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest(prompt, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', prompt, attachments, undefined, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -545,7 +545,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest(prompt, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', prompt, attachments, undefined, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -572,7 +572,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest(prompt, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', prompt, attachments, undefined, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertNoErrorsInStream(stream); @@ -600,7 +600,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { disposables.add(session); disposables.add(session.object.attachStream(stream)); - await session.object.handleRequest(prompt, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', prompt, attachments, undefined, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); const tsContents = await fs.readFile(tsFile, 'utf-8'); @@ -634,7 +634,7 @@ ssuite.skip({ title: '@cli', location: 'external' }, async (_) => { } })); - await session.object.handleRequest(prompt, attachments, undefined, CancellationToken.None); + await session.object.handleRequest('', prompt, attachments, undefined, CancellationToken.None); assert.strictEqual(session.object.status, ChatSessionStatus.Completed); assertStreamContains(stream, 'wkspc1');