From 06d75cce8a6a8fde3c1a70c7d24ab7bd7c26cfe8 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Thu, 14 Aug 2025 03:59:31 +0000 Subject: [PATCH] feat: save checkpoints immediately when users submit messages - Move checkpoint saving from recursivelyMakeClineRequests to handleWebviewAskResponse - Checkpoints are now saved when users click send, not when API requests are made - This allows users to easily revert to the state right before they typed their message - Update tests to reflect the new checkpoint behavior - Make handleWebviewAskResponse async to support checkpoint saving --- src/core/task/Task.ts | 30 +++--- src/core/task/__tests__/Task.spec.ts | 122 ++++++++++++---------- src/core/webview/webviewMessageHandler.ts | 5 +- 3 files changed, 86 insertions(+), 71 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index d4faca84f9..b8e2ba00f8 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -741,7 +741,21 @@ export class Task extends EventEmitter implements TaskLike { this.handleWebviewAskResponse("messageResponse", text, images) } - handleWebviewAskResponse(askResponse: ClineAskResponse, text?: string, images?: string[]) { + async handleWebviewAskResponse(askResponse: ClineAskResponse, text?: string, images?: string[]) { + // Save checkpoint immediately when user submits a message + // This allows users to easily revert to the state right before they typed their message + if (this.enableCheckpoints && askResponse === "messageResponse") { + try { + await this.checkpointSave(true) + } catch (error) { + console.error( + `[Task#handleWebviewAskResponse] Error saving checkpoint before user message: ${error.message}`, + error, + ) + // Don't block the user message if checkpoint fails + } + } + this.askResponse = askResponse this.askResponseText = text this.askResponseImages = images @@ -1533,17 +1547,9 @@ export class Task extends EventEmitter implements TaskLike { // results. const finalUserContent = [...parsedUserContent, { type: "text" as const, text: environmentDetails }] - // Save checkpoint before adding user message to conversation history - if (this.enableCheckpoints) { - try { - await this.checkpointSave(true) - } catch (error) { - console.error( - `[Task#recursivelyMakeClineRequests] Error saving checkpoint before user message: ${error.message}`, - error, - ) - } - } + // Note: Checkpoint is now saved in handleWebviewAskResponse when user submits a message, + // not here before the API request. This allows users to easily revert to the state + // right before they typed their message. await this.addToApiConversationHistory({ role: "user", content: finalUserContent }) TelemetryService.instance.captureConversationMessage(this.taskId, "user") diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 57aa1ba1be..c3e90f7a10 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -1615,7 +1615,7 @@ describe("Cline", () => { }) describe("Checkpoint before user messages", () => { - it("should save checkpoint before adding user message to conversation history", async () => { + it("should save checkpoint when user submits a message via handleWebviewAskResponse", async () => { const task = new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, @@ -1627,39 +1627,41 @@ describe("Cline", () => { // Mock checkpointSave method const checkpointSaveSpy = vi.spyOn(task, "checkpointSave").mockResolvedValue(undefined) - // Mock addToApiConversationHistory - const addToApiConversationHistorySpy = vi - .spyOn(task as any, "addToApiConversationHistory") - .mockResolvedValue(undefined) + // Call handleWebviewAskResponse with a user message + await task.handleWebviewAskResponse("messageResponse", "test user message", ["image.png"]) - // Mock other required methods - vi.spyOn(task as any, "saveClineMessages").mockResolvedValue(undefined) - vi.spyOn(task.api, "createMessage").mockReturnValue({ - async *[Symbol.asyncIterator]() { - yield { type: "text", text: "response" } - }, - } as any) - - // Mock clineMessages to avoid errors - task.clineMessages = [ - { - ts: Date.now(), - type: "say", - say: "api_req_started", - text: JSON.stringify({ request: "test" }), - }, - ] - - // Call recursivelyMakeClineRequests which should trigger checkpoint save - await task.recursivelyMakeClineRequests([{ type: "text", text: "test user message" }]) - - // Verify checkpoint was saved before adding to conversation history + // Verify checkpoint was saved expect(checkpointSaveSpy).toHaveBeenCalledWith(true) - expect(checkpointSaveSpy).toHaveBeenCalledBefore(addToApiConversationHistorySpy) - expect(addToApiConversationHistorySpy).toHaveBeenCalled() + + // Verify the response was set + expect(task["askResponse"]).toBe("messageResponse") + expect(task["askResponseText"]).toBe("test user message") + expect(task["askResponseImages"]).toEqual(["image.png"]) }) - it("should handle checkpoint save errors gracefully", async () => { + it("should not save checkpoint for non-messageResponse ask responses", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + enableCheckpoints: true, + startTask: false, + }) + + // Mock checkpointSave method + const checkpointSaveSpy = vi.spyOn(task, "checkpointSave").mockResolvedValue(undefined) + + // Call handleWebviewAskResponse with a non-message response + await task.handleWebviewAskResponse("yesButtonClicked", undefined, undefined) + + // Verify checkpoint was NOT saved + expect(checkpointSaveSpy).not.toHaveBeenCalled() + + // Verify the response was set + expect(task["askResponse"]).toBe("yesButtonClicked") + }) + + it("should handle checkpoint save errors gracefully in handleWebviewAskResponse", async () => { const task = new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, @@ -1675,31 +1677,8 @@ describe("Cline", () => { // Mock console.error to verify error logging const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) - // Mock addToApiConversationHistory to verify it still gets called - const addToApiConversationHistorySpy = vi - .spyOn(task as any, "addToApiConversationHistory") - .mockResolvedValue(undefined) - - // Mock other required methods - vi.spyOn(task as any, "saveClineMessages").mockResolvedValue(undefined) - vi.spyOn(task.api, "createMessage").mockReturnValue({ - async *[Symbol.asyncIterator]() { - yield { type: "text", text: "response" } - }, - } as any) - - // Mock clineMessages - task.clineMessages = [ - { - ts: Date.now(), - type: "say", - say: "api_req_started", - text: JSON.stringify({ request: "test" }), - }, - ] - - // Call recursivelyMakeClineRequests - await task.recursivelyMakeClineRequests([{ type: "text", text: "test user message" }]) + // Call handleWebviewAskResponse + await task.handleWebviewAskResponse("messageResponse", "test user message", ["image.png"]) // Verify checkpoint save was attempted expect(checkpointSaveSpy).toHaveBeenCalledWith(true) @@ -1710,8 +1689,10 @@ describe("Cline", () => { checkpointError, ) - // Verify conversation history was still updated despite checkpoint error - expect(addToApiConversationHistorySpy).toHaveBeenCalled() + // Verify the response was still set despite checkpoint error + expect(task["askResponse"]).toBe("messageResponse") + expect(task["askResponseText"]).toBe("test user message") + expect(task["askResponseImages"]).toEqual(["image.png"]) // Restore console.error consoleErrorSpy.mockRestore() @@ -1729,6 +1710,30 @@ describe("Cline", () => { // Mock checkpointSave method const checkpointSaveSpy = vi.spyOn(task, "checkpointSave").mockResolvedValue(undefined) + // Call handleWebviewAskResponse with a user message + await task.handleWebviewAskResponse("messageResponse", "test user message", ["image.png"]) + + // Verify checkpoint was NOT saved + expect(checkpointSaveSpy).not.toHaveBeenCalled() + + // Verify the response was still set + expect(task["askResponse"]).toBe("messageResponse") + expect(task["askResponseText"]).toBe("test user message") + expect(task["askResponseImages"]).toEqual(["image.png"]) + }) + + it("should not save checkpoint in recursivelyMakeClineRequests anymore", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + enableCheckpoints: true, + startTask: false, + }) + + // Mock checkpointSave method + const checkpointSaveSpy = vi.spyOn(task, "checkpointSave").mockResolvedValue(undefined) + // Mock other required methods vi.spyOn(task as any, "addToApiConversationHistory").mockResolvedValue(undefined) vi.spyOn(task as any, "saveClineMessages").mockResolvedValue(undefined) @@ -1751,7 +1756,8 @@ describe("Cline", () => { // Call recursivelyMakeClineRequests await task.recursivelyMakeClineRequests([{ type: "text", text: "test user message" }]) - // Verify checkpoint was NOT saved + // Verify checkpoint was NOT saved in recursivelyMakeClineRequests + // (it should only be saved in handleWebviewAskResponse now) expect(checkpointSaveSpy).not.toHaveBeenCalled() }) }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 7e25ae14dc..76f3ae09c5 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -346,7 +346,10 @@ export const webviewMessageHandler = async ( await provider.postStateToWebview() break case "askResponse": - provider.getCurrentCline()?.handleWebviewAskResponse(message.askResponse!, message.text, message.images) + // handleWebviewAskResponse is now async to support checkpoint saving + await provider + .getCurrentCline() + ?.handleWebviewAskResponse(message.askResponse!, message.text, message.images) break case "autoCondenseContext": await updateGlobalState("autoCondenseContext", message.bool)