-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Restore edits for background sessions #2180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| currentResponseParts.push(new ChatResponseCodeblockUriPart(uri, true, editId)); | ||
| currentResponseParts.push(new ChatResponseMarkdownPart('\n````\n')); | ||
| currentResponseParts.push(new ChatResponseTextEditPart(uri, [])); | ||
| currentResponseParts.push(new ChatResponseTextEditPart(uri, true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-emit the edits with the undo stop ids
| } | ||
| } | ||
|
|
||
| const requestDetails: { requestId: string; toolIdEditMap: Record<string, string> } = { requestId, toolIdEditMap: {} }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping a map of request Id and undo stop ids (that map to SDK requests and tool calls)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements functionality to restore edits for background chat sessions by introducing request ID tracking and undo stop ID support. This allows the system to correlate VS Code chat request IDs with SDK request IDs and track edit operations across session boundaries, which is necessary for restoring edits when sessions are reopened.
Key changes:
- Added
requestIdparameter tohandleRequestmethod to track VS Code chat request IDs through the session lifecycle - Introduced
undoStopIdtoChatResponseCodeblockUriPartfor tracking individual edit operations - Changed
externalEditreturn type from genericTtostringto return edit IDs for tracking purposes
Reviewed changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/cli.stest.ts | Updated all test calls to handleRequest to include the new requestId parameter (empty string for tests) |
| src/util/common/test/shims/chatTypes.ts | Added isEdit and undoStopId properties to ChatResponseCodeblockUriPart and updated constructor signature; modified ChatResponseExternalEditPart.applied to return Thenable<string> |
| src/util/common/chatResponseStreamImpl.ts | Changed externalEdit to return Promise<string> instead of generic Thenable<T> and updated callback parameter type to Thenable<unknown> |
| src/extension/vscode.proposed.chatParticipantPrivate.d.ts | Added optional id field to ChatRequestTurn2 for tracking chat request IDs and updated constructor signature |
| src/extension/vscode.proposed.chatParticipantAdditions.d.ts | Added undoStopId to ChatResponseCodeblockUriPart, changed externalEdit return type to Thenable<string>, and updated ChatResponseExternalEditPart.applied type |
| src/extension/test/node/testHelpers.ts | Updated MockChatResponseStream.externalEdit to match new signature returning Promise<string> |
| src/extension/replay/vscode-node/chatReplaySessionProvider.ts | Updated ChatRequestTurn2 constructor call to include undefined for the new id parameter |
| src/extension/prompt/node/chatParticipantRequestHandler.ts | Added type casting to access editedFileEvents from ChatRequestTurn2 while maintaining compatibility with base ChatRequestTurn |
| src/extension/linkify/common/responseStreamWithLinkification.ts | Updated externalEdit signature to match new return type |
| src/extension/chatSessions/vscode-node/test/copilotCLIChatSessionParticipant.spec.ts | Updated test mock to include requestId parameter in handleRequest signature |
| src/extension/chatSessions/vscode-node/copilotCloudSessionContentBuilder.ts | Updated ChatRequestTurn2 constructor call to include undefined for the new id parameter |
| src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts | Updated to pass request.id as the first parameter to handleRequest |
| src/extension/chatSessions/vscode-node/claudeChatSessionContentProvider.ts | Updated ChatRequestTurn2 constructor call to include undefined for the new id parameter |
| src/extension/agents/copilotcli/node/test/copilotcliSession.spec.ts | Updated all test calls to handleRequest to include the new requestId parameter (empty string for tests) |
| src/extension/agents/copilotcli/node/copilotcliSession.ts | Added requestId parameter to handleRequest, implemented edit ID tracking with toolIdEditMap, and updated getChatHistory to pass request ID lookup function; stores request details in SDK after completion |
| src/extension/agents/copilotcli/node/copilotCli.ts | Added request map storage in workspace state with getRequestId and setRequestId methods, includes 7-day pruning of old entries |
| src/extension/agents/copilotcli/common/test/copilotCLITools.spec.ts | Updated test expectations to handle new tuple return type from processToolExecutionComplete |
| src/extension/agents/copilotcli/common/copilotCLITools.ts | Modified buildChatHistoryFromEvents to accept optional request ID lookup function; updated tool execution tracking to use tuples; added logic to insert ChatResponseCodeblockUriPart with edit IDs for completed edits; updated formatter signatures to accept optional editId parameter |
| src/extension/agents/common/externalEditTracker.ts | Changed completeEdit to return Promise<string | undefined> instead of Promise<void> to propagate the edit ID from VS Code |
Comments suppressed due to low confidence (2)
src/extension/agents/copilotcli/common/copilotCLITools.ts:590
- The
editIdparameter is declared but never used in this function. Either remove it if it's not needed, or implement the intended functionality if it was meant to be used.
function formatCreateToolInvocation(invocation: ChatToolInvocationPart, toolCall: CreateTool, editId?: string): void {
const args = toolCall.arguments;
const display = args.path ? formatUriForFileWidget(Uri.file(args.path)) : '';
if (display) {
invocation.invocationMessage = new MarkdownString(l10n.t("Created {0}", display));
} else {
invocation.invocationMessage = new MarkdownString(l10n.t("Created file"));
}
}
src/extension/agents/copilotcli/common/copilotCLITools.ts:555
- The
editIdparameter is declared but never used in this function. Either remove it if it's not needed, or implement the intended functionality if it was meant to be used.
function formatStrReplaceEditorInvocation(invocation: ChatToolInvocationPart, toolCall: StringReplaceEditorTool, editId?: string): void {
if (!toolCall.arguments.path) {
return;
}
const args = toolCall.arguments;
const display = formatUriForFileWidget(Uri.file(args.path));
switch (args.command) {
case 'view':
formatViewToolInvocation(invocation, { toolName: 'view', arguments: args } as ViewTool);
break;
case 'edit':
formatEditToolInvocation(invocation, { toolName: 'edit', arguments: args } as EditTool);
break;
case 'insert':
formatInsertToolInvocation(invocation, { toolName: 'insert', arguments: args } as InsertTool);
break;
case 'create':
formatCreateToolInvocation(invocation, { toolName: 'create', arguments: args } as CreateTool);
break;
case 'undo_edit':
formatUndoEdit(invocation, { toolName: 'undo_edit', arguments: args } as UndoEditTool);
break;
default:
invocation.invocationMessage = new MarkdownString(l10n.t("Modified {0}", display));
}
}
Related PR microsoft/vscode#279270