On request-removal undo till the point at which the request was made (microsoft#213902)

* On request-removal undo till the point at which the request was made

fixes microsoft/vscode-copilot#5736

* fixed EOL sequence which (hopefully) fixes tests

* give up, don't use new lines...

Author: jrieken

src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts

@@ -439,9 +439,18 @@ export class InlineChatController implements IEditorContribution {
 					}
 				});
 			} else if (e.kind === 'removeRequest') {
-				// TODO@jrieken this is buggy/weird when having code changes from multiple turns because
-				// logically they are all the same hunks
-				this._strategy?.cancel();
+				// TODO@jrieken there is still some work left for when a request "in the middle"
+				// is removed. We will undo all changes till that point but not remove those
+				// later request
+				const exchange = this._session!.exchanges.find(candidate => candidate.prompt.request.id === e.requestId);
+				if (exchange && this._editor.hasModel()) {
+					// undo till this point
+					const model = this._editor.getModel();
+					const targetAltVersion = exchange.prompt.modelAltVersionId;
+					while (targetAltVersion < model.getAlternativeVersionId() && model.canUndo()) {
+						await model.undo();
+					}
+				}
 			}
 		}));
 
@@ -601,7 +610,7 @@ export class InlineChatController implements IEditorContribution {
 		const input = request.message.text;
 		this._ui.value.zone.widget.value = input;
 
-		this._session.addInput(new SessionPrompt(request));
+		this._session.addInput(new SessionPrompt(request, this._editor.getModel()!.getAlternativeVersionId()));
 
 		return State.SHOW_REQUEST;
 	}

src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts

@@ -206,6 +206,10 @@ export class Session {
 		// this._teldata.responseTypes += `${exchange.response instanceof ReplyResponse ? exchange.response.responseType : InlineChatResponseTypes.Empty}|`;
 	}
 
+	get exchanges(): readonly SessionExchange[] {
+		return this._exchange;
+	}
+
 	get lastExchange(): SessionExchange | undefined {
 		return this._exchange[this._exchange.length - 1];
 	}
@@ -273,7 +277,8 @@ export class SessionPrompt {
 	readonly value: string;
 
 	constructor(
-		readonly request: IChatRequestModel
+		readonly request: IChatRequestModel,
+		readonly modelAltVersionId: number,
 	) {
 		this.value = request.message.text;
 	}

src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts

@@ -14,7 +14,7 @@ import { IActiveCodeEditor } from 'vs/editor/browser/editorBrowser';
 import { IDiffProviderFactoryService } from 'vs/editor/browser/widget/diffEditor/diffProviderFactoryService';
 import { EditOperation } from 'vs/editor/common/core/editOperation';
 import { Range } from 'vs/editor/common/core/range';
-import { ITextModel } from 'vs/editor/common/model';
+import { EndOfLineSequence, ITextModel } from 'vs/editor/common/model';
 import { IEditorWorkerService } from 'vs/editor/common/services/editorWorker';
 import { IModelService } from 'vs/editor/common/services/model';
 import { TestDiffProviderFactoryService } from 'vs/editor/test/browser/diff/testDiffProviderFactoryService';
@@ -27,16 +27,16 @@ import { ServiceCollection } from 'vs/platform/instantiation/common/serviceColle
 import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
 import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
 import { IEditorProgressService, IProgressRunner } from 'vs/platform/progress/common/progress';
-import { IViewDescriptorService } from 'vs/workbench/common/views';
+import { IView, IViewDescriptorService } from 'vs/workbench/common/views';
 import { AccessibilityVerbositySettingId } from 'vs/workbench/contrib/accessibility/browser/accessibilityConfiguration';
 import { IAccessibleViewService } from 'vs/platform/accessibility/browser/accessibleView';
-import { IChatAccessibilityService, IChatWidgetService } from 'vs/workbench/contrib/chat/browser/chat';
+import { IChatAccessibilityService, IChatWidget, IChatWidgetService } from 'vs/workbench/contrib/chat/browser/chat';
 import { ChatAgentLocation, ChatAgentService, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';
 import { IChatResponseViewModel } from 'vs/workbench/contrib/chat/common/chatViewModel';
 import { InlineChatController, InlineChatRunOptions, State } from 'vs/workbench/contrib/inlineChat/browser/inlineChatController';
 import { Session } from 'vs/workbench/contrib/inlineChat/browser/inlineChatSession';
 import { CTX_INLINE_CHAT_USER_DID_EDIT, EditMode, InlineChatConfigKeys } from 'vs/workbench/contrib/inlineChat/common/inlineChat';
-import { workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
+import { TestViewsService, workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
 import { IInlineChatSavingService } from '../../browser/inlineChatSavingService';
 import { IInlineChatSessionService } from '../../browser/inlineChatSessionService';
 import { InlineChatSessionServiceImpl } from '../../browser/inlineChatSessionServiceImpl';
@@ -61,6 +61,7 @@ import { ICommandService } from 'vs/platform/commands/common/commands';
 import { TestCommandService } from 'vs/editor/test/browser/editorTestServices';
 import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/services/notebookEditorService';
 import { RerunAction } from 'vs/workbench/contrib/inlineChat/browser/inlineChatActions';
+import { CancellationToken } from 'vs/base/common/cancellation';
 
 suite('InteractiveChatController', function () {
 
@@ -127,10 +128,13 @@ suite('InteractiveChatController', function () {
 	let model: ITextModel;
 	let ctrl: TestController;
 	let contextKeyService: MockContextKeyService;
+	let chatService: IChatService;
 	let chatAgentService: IChatAgentService;
 	let inlineChatSessionService: IInlineChatSessionService;
 	let instaService: TestInstantiationService;
 
+	let chatWidget: IChatWidget;
+
 	setup(function () {
 
 		const serviceCollection = new ServiceCollection(
@@ -141,7 +145,11 @@ suite('InteractiveChatController', function () {
 			[IHoverService, NullHoverService],
 			[IExtensionService, new TestExtensionService()],
 			[IContextKeyService, new MockContextKeyService()],
-			[IViewsService, new TestExtensionService()],
+			[IViewsService, new class extends TestViewsService {
+				override async openView<T extends IView>(id: string, focus?: boolean | undefined): Promise<T | null> {
+					return { widget: chatWidget ?? null } as any;
+				}
+			}()],
 			[IWorkspaceContextService, new TestContextService()],
 			[IChatWidgetHistoryService, new SyncDescriptor(ChatWidgetHistoryService)],
 			[IChatWidgetService, new SyncDescriptor(ChatWidgetService)],
@@ -193,12 +201,13 @@ suite('InteractiveChatController', function () {
 		configurationService.setUserConfiguration('editor', {});
 
 		contextKeyService = instaService.get(IContextKeyService) as MockContextKeyService;
-
+		chatService = instaService.get(IChatService);
 		chatAgentService = instaService.get(IChatAgentService);
 
 		inlineChatSessionService = store.add(instaService.get(IInlineChatSessionService));
 
 		model = store.add(instaService.get(IModelService).createModel('Hello\nWorld\nHello Again\nHello World\n', null));
+		model.setEOL(EndOfLineSequence.LF);
 		editor = store.add(instantiateTestCodeEditor(instaService, model));
 
 		store.add(chatAgentService.registerDynamicAgent({ id: 'testEditorAgent', ...agentData, }, {
@@ -494,4 +503,143 @@ suite('InteractiveChatController', function () {
 		ctrl.finishExistingSession();
 		await r;
 	});
+
+	test('Retry undoes all changes, not just those from the request#5736', async function () {
+
+		const text = [
+			'eins-',
+			'zwei-',
+			'drei-'
+		];
+
+		store.add(chatAgentService.registerDynamicAgent({
+			id: 'testEditorAgent2',
+			...agentData
+		}, {
+			async invoke(request, progress, history, token) {
+				progress({ kind: 'textEdit', uri: model.uri, edits: [{ range: new Range(1, 1, 1, 1), text: text.shift() ?? '' }] });
+				return {};
+			},
+		}));
+
+		ctrl = instaService.createInstance(TestController, editor);
+		const rerun = new RerunAction();
+
+		model.setValue('');
+
+		// REQUEST 1
+		const p = ctrl.waitFor([...TestController.INIT_SEQUENCE, State.SHOW_REQUEST, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
+		const r = ctrl.run({ message: '1', autoSend: true });
+		await p;
+
+		assert.strictEqual(model.getValue(), 'eins-');
+
+		// REQUEST 2
+		const p2 = ctrl.waitFor([State.SHOW_REQUEST, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
+		await ctrl.acceptInput();
+		await p2;
+
+		assert.strictEqual(model.getValue(), 'zwei-eins-');
+
+		// REQUEST 2 - RERUN
+		const p3 = ctrl.waitFor([State.SHOW_REQUEST, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
+		await instaService.invokeFunction(rerun.runInlineChatCommand, ctrl, editor);
+		await p3;
+
+		assert.strictEqual(model.getValue(), 'drei-eins-');
+
+		ctrl.finishExistingSession();
+		await r;
+
+	});
+
+	test('moving inline chat to another model undoes changes', async function () {
+		const text = [
+			'eins\n',
+			'zwei\n'
+		];
+
+		store.add(chatAgentService.registerDynamicAgent({
+			id: 'testEditorAgent2',
+			...agentData
+		}, {
+			async invoke(request, progress, history, token) {
+				progress({ kind: 'textEdit', uri: model.uri, edits: [{ range: new Range(1, 1, 1, 1), text: text.shift() ?? '' }] });
+				return {};
+			},
+		}));
+		ctrl = instaService.createInstance(TestController, editor);
+
+		// REQUEST 1
+		const p = ctrl.waitFor([...TestController.INIT_SEQUENCE, State.SHOW_REQUEST, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
+		ctrl.run({ message: '1', autoSend: true });
+		await p;
+
+
+		assert.strictEqual(model.getValue(), 'eins\nHello\nWorld\nHello Again\nHello World\n');
+
+		const targetModel = chatService.startSession(ChatAgentLocation.Editor, CancellationToken.None)!;
+		store.add(targetModel);
+		chatWidget = new class extends mock<IChatWidget>() {
+			override get viewModel() {
+				return { model: targetModel } as any;
+			}
+			override focusLastMessage() { }
+		};
+
+		const r = ctrl.joinCurrentRun();
+		await ctrl.viewInChat();
+
+		assert.strictEqual(model.getValue(), 'Hello\nWorld\nHello Again\nHello World\n');
+		await r;
+	});
+
+	test('moving inline chat to another model undoes changes (2 requests)', async function () {
+		const text = [
+			'eins\n',
+			'zwei\n'
+		];
+
+		store.add(chatAgentService.registerDynamicAgent({
+			id: 'testEditorAgent2',
+			...agentData
+		}, {
+			async invoke(request, progress, history, token) {
+				progress({ kind: 'textEdit', uri: model.uri, edits: [{ range: new Range(1, 1, 1, 1), text: text.shift() ?? '' }] });
+				return {};
+			},
+		}));
+		ctrl = instaService.createInstance(TestController, editor);
+
+		// REQUEST 1
+		const p = ctrl.waitFor([...TestController.INIT_SEQUENCE, State.SHOW_REQUEST, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
+		ctrl.run({ message: '1', autoSend: true });
+		await p;
+
+		assert.strictEqual(model.getValue(), 'eins\nHello\nWorld\nHello Again\nHello World\n');
+
+		// REQUEST 2
+		const p2 = ctrl.waitFor([State.SHOW_REQUEST, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
+		await ctrl.acceptInput();
+		await p2;
+
+		assert.strictEqual(model.getValue(), 'zwei\neins\nHello\nWorld\nHello Again\nHello World\n');
+
+		const targetModel = chatService.startSession(ChatAgentLocation.Editor, CancellationToken.None)!;
+		store.add(targetModel);
+		chatWidget = new class extends mock<IChatWidget>() {
+			override get viewModel() {
+				return { model: targetModel } as any;
+			}
+			override focusLastMessage() { }
+		};
+
+		const r = ctrl.joinCurrentRun();
+
+		await ctrl.viewInChat();
+
+		assert.strictEqual(model.getValue(), 'Hello\nWorld\nHello Again\nHello World\n');
+
+		await r;
+	});
 });