working version, still some default checkpointing bugs

This commit is contained in:
Will Li 2025-07-13 00:19:49 -07:00
parent 63c7c7c230
commit d3c655d351
7 changed files with 527 additions and 57 deletions

View file

@ -0,0 +1,169 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
import { Task } from "../../task/Task"
import { ClineProvider } from "../../webview/ClineProvider"
import { checkpointSave } from "../index"
import * as vscode from "vscode"
// Mock vscode
vi.mock("vscode", () => ({
window: {
showErrorMessage: vi.fn(),
},
Uri: {
file: vi.fn((path: string) => ({ fsPath: path })),
},
}))
// Mock other dependencies
vi.mock("@roo-code/telemetry", () => ({
TelemetryService: {
instance: {
captureCheckpointCreated: vi.fn(),
},
},
}))
vi.mock("../../../utils/path", () => ({
getWorkspacePath: vi.fn(() => "/test/workspace"),
}))
describe("Checkpoint after message deletion", () => {
let mockProvider: any
let mockTask: any
let mockCheckpointService: any
beforeEach(() => {
// Create mock checkpoint service
mockCheckpointService = {
isInitialized: false,
saveCheckpoint: vi.fn().mockResolvedValue({ commit: "test-commit-hash" }),
on: vi.fn(),
initShadowGit: vi.fn().mockResolvedValue(undefined),
}
// Create mock provider
mockProvider = {
context: {
globalStorageUri: { fsPath: "/test/storage" },
},
log: vi.fn(),
postMessageToWebview: vi.fn(),
postStateToWebview: vi.fn(),
}
// Create mock task
mockTask = {
taskId: "test-task-id",
enableCheckpoints: true,
checkpointService: undefined,
checkpointServiceInitializing: false,
providerRef: {
deref: () => mockProvider,
},
clineMessages: [],
pendingUserMessageCheckpoint: undefined,
}
// Mock the RepoPerTaskCheckpointService.create to return our mock
vi.mock("../../../services/checkpoints", () => ({
RepoPerTaskCheckpointService: {
create: vi.fn(() => mockCheckpointService),
},
}))
})
afterEach(() => {
vi.clearAllMocks()
})
it("should wait for checkpoint service initialization before saving", async () => {
// Simulate service initialization after a delay
setTimeout(() => {
mockCheckpointService.isInitialized = true
}, 100)
// Call checkpointSave
const savePromise = checkpointSave(mockTask, true)
// Initially, service should not be initialized
expect(mockCheckpointService.isInitialized).toBe(false)
// Wait for the save to complete
const result = await savePromise
// Service should now be initialized
expect(mockCheckpointService.isInitialized).toBe(true)
// saveCheckpoint should have been called
expect(mockCheckpointService.saveCheckpoint).toHaveBeenCalledWith(
expect.stringContaining("Task: test-task-id"),
{ allowEmpty: true },
)
// Result should contain the commit hash
expect(result).toEqual({ commit: "test-commit-hash" })
// Task should still have checkpoints enabled
expect(mockTask.enableCheckpoints).toBe(true)
})
it("should handle timeout when service doesn't initialize", async () => {
// Service never initializes
mockCheckpointService.isInitialized = false
// Call checkpointSave with a task that has no checkpoint service
const taskWithNoService = {
...mockTask,
checkpointService: undefined,
enableCheckpoints: false,
}
const result = await checkpointSave(taskWithNoService, true)
// Result should be undefined
expect(result).toBeUndefined()
// saveCheckpoint should not have been called
expect(mockCheckpointService.saveCheckpoint).not.toHaveBeenCalled()
})
it("should preserve checkpoint data through message deletion flow", async () => {
// Initialize service
mockCheckpointService.isInitialized = true
// Simulate saving checkpoint before user message
const checkpointResult = await checkpointSave(mockTask, true)
expect(checkpointResult).toEqual({ commit: "test-commit-hash" })
// Simulate setting pendingUserMessageCheckpoint
if (checkpointResult && "commit" in checkpointResult) {
mockTask.pendingUserMessageCheckpoint = {
hash: checkpointResult.commit,
timestamp: Date.now(),
type: "user_message",
}
}
// Verify checkpoint data is preserved
expect(mockTask.pendingUserMessageCheckpoint).toBeDefined()
expect(mockTask.pendingUserMessageCheckpoint.hash).toBe("test-commit-hash")
// Simulate message deletion and reinitialization
mockTask.clineMessages = []
mockTask.checkpointService = undefined
mockTask.checkpointServiceInitializing = false
// Re-initialize checkpoint service
setTimeout(() => {
mockCheckpointService.isInitialized = true
mockTask.checkpointService = mockCheckpointService
}, 50)
// Save checkpoint again after deletion
const newCheckpointResult = await checkpointSave(mockTask, true)
// Should still work after reinitialization
expect(newCheckpointResult).toEqual({ commit: "test-commit-hash" })
expect(mockTask.enableCheckpoints).toBe(true)
})
})

View file

@ -150,19 +150,13 @@ async function getInitializedCheckpointService(
}
export async function checkpointSave(cline: Task, force = false) {
const service = getCheckpointService(cline)
// Use getInitializedCheckpointService to wait for initialization
const service = await getInitializedCheckpointService(cline)
if (!service) {
return
}
if (!service.isInitialized) {
const provider = cline.providerRef.deref()
provider?.log("[checkpointSave] checkpoints didn't initialize in time, disabling checkpoints for this task")
cline.enableCheckpoints = false
return
}
TelemetryService.instance.captureCheckpointCreated(cline.taskId)
// Start the checkpoint process in the background.
@ -176,9 +170,13 @@ export type CheckpointRestoreOptions = {
ts: number
commitHash: string
mode: "preview" | "restore"
operation?: "delete" | "edit" // Optional to maintain backward compatibility
}
export async function checkpointRestore(cline: Task, { ts, commitHash, mode }: CheckpointRestoreOptions) {
export async function checkpointRestore(
cline: Task,
{ ts, commitHash, mode, operation = "delete" }: CheckpointRestoreOptions,
) {
const service = await getInitializedCheckpointService(cline)
if (!service) {
@ -207,7 +205,10 @@ export async function checkpointRestore(cline: Task, { ts, commitHash, mode }: C
cline.combineMessages(deletedMessages),
)
await cline.overwriteClineMessages(cline.clineMessages.slice(0, index + 1))
// For delete operations, exclude the checkpoint message itself
// For edit operations, include the checkpoint message (to be edited)
const endIndex = operation === "edit" ? index + 1 : index
await cline.overwriteClineMessages(cline.clineMessages.slice(0, endIndex))
// TODO: Verify that this is working as expected.
await cline.say(

View file

@ -552,39 +552,10 @@ export class Task extends EventEmitter<ClineEvents> {
}
async handleWebviewAskResponse(askResponse: ClineAskResponse, text?: string, images?: string[]) {
// Save checkpoint BEFORE setting the response to ensure it's ready when the user_feedback message is created
if (this.enableCheckpoints && askResponse === "messageResponse") {
console.log("[Task#handleWebviewAskResponse] Saving checkpoint for user message")
try {
const checkpointResult = await this.checkpointSave(true) // Force checkpoint save
console.log("[Task#handleWebviewAskResponse] Checkpoint result:", checkpointResult)
if (checkpointResult?.commit) {
// Store checkpoint data temporarily to be used when creating the user_feedback message
this.pendingUserMessageCheckpoint = {
hash: checkpointResult.commit,
timestamp: Date.now(),
type: "user_message",
}
console.log(
"[Task#handleWebviewAskResponse] Set pendingUserMessageCheckpoint:",
this.pendingUserMessageCheckpoint,
)
} else {
console.log("[Task#handleWebviewAskResponse] No commit in checkpoint result")
}
} catch (error) {
console.error("[Task#handleWebviewAskResponse] Failed to save checkpoint after user message:", error)
}
} else {
console.log(
"[Task#handleWebviewAskResponse] Skipping checkpoint save - enableCheckpoints:",
this.enableCheckpoints,
"askResponse:",
askResponse,
)
}
// Checkpoint saving is now handled in webviewMessageHandler before this method is called
console.log("[Task#handleWebviewAskResponse] Processing askResponse:", askResponse)
// Now set the response, which will trigger the ask promise to resolve
// Set the response, which will trigger the ask promise to resolve
this.askResponse = askResponse
this.askResponseText = text
this.askResponseImages = images

View file

@ -605,6 +605,54 @@ export class ClineProvider
this.log(
`[subtasks] ${cline.parentTask ? "child" : "parent"} task ${cline.taskId}.${cline.instanceId} instantiated`,
)
// Check if there's a pending edit after checkpoint restoration
if ((this as any).pendingEditAfterRestore) {
const pendingEdit = (this as any).pendingEditAfterRestore
;(this as any).pendingEditAfterRestore = undefined // Clear the pending edit
this.log(`[initClineWithHistoryItem] Processing pending edit after checkpoint restoration`)
// Process the pending edit after a short delay to ensure the task is fully initialized
setTimeout(async () => {
try {
// Find the message index in the restored state
const { messageIndex, apiConversationHistoryIndex } = (() => {
const messageIndex = cline.clineMessages.findIndex((msg) => msg.ts === pendingEdit.messageTs)
const apiConversationHistoryIndex = cline.apiConversationHistory.findIndex(
(msg) => msg.ts === pendingEdit.messageTs,
)
return { messageIndex, apiConversationHistoryIndex }
})()
if (messageIndex !== -1) {
// Remove the target message and all subsequent messages
await cline.overwriteClineMessages(cline.clineMessages.slice(0, messageIndex))
if (apiConversationHistoryIndex !== -1) {
await cline.overwriteApiConversationHistory(
cline.apiConversationHistory.slice(0, apiConversationHistoryIndex),
)
}
// If there was an original checkpoint, preserve it for the new message
if (pendingEdit.originalCheckpoint) {
cline.pendingUserMessageCheckpoint = pendingEdit.originalCheckpoint
}
// Process the edited message
await cline.handleWebviewAskResponse(
"messageResponse",
pendingEdit.editedContent,
pendingEdit.images,
)
}
} catch (error) {
this.log(`[initClineWithHistoryItem] Error processing pending edit: ${error}`)
}
}, 100) // Small delay to ensure task is fully ready
}
return cline
}

View file

@ -0,0 +1,150 @@
import { describe, it, expect, vi, beforeEach } from "vitest"
import { webviewMessageHandler } from "../webviewMessageHandler"
import { saveTaskMessages } from "../../task-persistence"
import { checkpointRestore } from "../../checkpoints"
// Mock dependencies
vi.mock("../../task-persistence")
vi.mock("../../checkpoints")
vi.mock("vscode", () => ({
window: {
showErrorMessage: vi.fn(),
},
}))
describe("webviewMessageHandler - checkpoint operations", () => {
let mockProvider: any
let mockCline: any
beforeEach(() => {
vi.clearAllMocks()
// Setup mock Cline instance
mockCline = {
taskId: "test-task-123",
clineMessages: [
{ ts: 1, type: "user", say: "user", text: "First message" },
{ ts: 2, type: "assistant", say: "assistant", text: "Response" },
{
ts: 3,
type: "user",
say: "user",
text: "Checkpoint message",
checkpoint: { hash: "abc123", label: "Test checkpoint" },
},
{ ts: 4, type: "assistant", say: "assistant", text: "After checkpoint" },
],
apiConversationHistory: [
{ ts: 1, role: "user", content: [{ type: "text", text: "First message" }] },
{ ts: 2, role: "assistant", content: [{ type: "text", text: "Response" }] },
{ ts: 3, role: "user", content: [{ type: "text", text: "Checkpoint message" }] },
{ ts: 4, role: "assistant", content: [{ type: "text", text: "After checkpoint" }] },
],
checkpointRestore: vi.fn(),
overwriteClineMessages: vi.fn(),
overwriteApiConversationHistory: vi.fn(),
}
// Setup mock provider
mockProvider = {
getCurrentCline: vi.fn(() => mockCline),
postMessageToWebview: vi.fn(),
getTaskWithId: vi.fn(() => ({
historyItem: { id: "test-task-123", messages: mockCline.clineMessages },
})),
initClineWithHistoryItem: vi.fn(),
contextProxy: {
globalStorageUri: { fsPath: "/test/storage" },
},
}
})
describe("delete operations with checkpoint restoration", () => {
it("should save messages to disk after checkpoint restoration", async () => {
// Simulate checkpoint restoration that removes messages
mockCline.checkpointRestore.mockImplementation(async () => {
// Simulate the effect of checkpoint restoration
mockCline.clineMessages = mockCline.clineMessages.slice(0, 2)
})
// Call the handler with delete confirmation
await webviewMessageHandler(mockProvider, {
type: "deleteMessageConfirm",
messageTs: 3,
restoreCheckpoint: true,
})
// Verify checkpoint restore was called with delete operation
expect(mockCline.checkpointRestore).toHaveBeenCalledWith({
ts: 3,
commitHash: "abc123",
mode: "restore",
operation: "delete",
})
// Verify saveTaskMessages was called after checkpoint restoration
expect(saveTaskMessages).toHaveBeenCalledWith({
messages: mockCline.clineMessages,
taskId: "test-task-123",
globalStoragePath: "/test/storage",
})
// Verify the save happened after the checkpoint restore
const checkpointRestoreOrder = mockCline.checkpointRestore.mock.invocationCallOrder[0]
const saveTaskMessagesOrder = (saveTaskMessages as any).mock.invocationCallOrder[0]
expect(saveTaskMessagesOrder).toBeGreaterThan(checkpointRestoreOrder)
})
it("should save messages for non-checkpoint deletes", async () => {
// Call the handler with delete confirmation (no checkpoint restoration)
await webviewMessageHandler(mockProvider, {
type: "deleteMessageConfirm",
messageTs: 2,
restoreCheckpoint: false,
})
// Verify saveTaskMessages was called
expect(saveTaskMessages).toHaveBeenCalledWith({
messages: expect.any(Array),
taskId: "test-task-123",
globalStoragePath: "/test/storage",
})
// Verify checkpoint restore was NOT called
expect(mockCline.checkpointRestore).not.toHaveBeenCalled()
})
})
describe("edit operations with checkpoint restoration", () => {
it("should call checkpoint restore with edit operation", async () => {
// Mock the pending edit storage
mockCline.pendingEditOperation = null
// Call the handler with edit confirmation
await webviewMessageHandler(mockProvider, {
type: "editMessageConfirm",
messageTs: 3,
text: "Edited checkpoint message",
restoreCheckpoint: true,
})
// Verify checkpoint restore was called with edit operation
expect(mockCline.checkpointRestore).toHaveBeenCalledWith({
ts: 3,
commitHash: "abc123",
mode: "restore",
operation: "edit",
})
// Verify the pending edit operation was stored
expect(mockCline.pendingEditOperation).toEqual({
messageTs: 3,
editedContent: "Edited checkpoint message",
images: undefined,
messageIndex: 2,
apiConversationHistoryIndex: 2,
originalCheckpoint: { hash: "abc123", label: "Test checkpoint" },
})
})
})
})

View file

@ -16,6 +16,7 @@ import {
import { CloudService } from "@roo-code/cloud"
import { TelemetryService } from "@roo-code/telemetry"
import { type ApiMessage } from "../task-persistence/apiMessages"
import { saveTaskMessages } from "../task-persistence"
import { ClineProvider } from "./ClineProvider"
import { changeLanguage, t } from "../../i18n"
@ -69,10 +70,10 @@ export const webviewMessageHandler = async (
* Shared utility to find message indices based on timestamp
*/
const findMessageIndices = (messageTs: number, currentCline: any) => {
const timeCutoff = messageTs - 1000 // 1 second buffer before the message
const messageIndex = currentCline.clineMessages.findIndex((msg: ClineMessage) => msg.ts && msg.ts >= timeCutoff)
// Find the exact message by timestamp, not the first one after a cutoff
const messageIndex = currentCline.clineMessages.findIndex((msg: ClineMessage) => msg.ts === messageTs)
const apiConversationHistoryIndex = currentCline.apiConversationHistory.findIndex(
(msg: ApiMessage) => msg.ts && msg.ts >= timeCutoff,
(msg: ApiMessage) => msg.ts === messageTs,
)
return { messageIndex, apiConversationHistoryIndex }
}
@ -165,17 +166,53 @@ export const webviewMessageHandler = async (
ts: targetMessage.ts!,
commitHash: targetMessage.checkpoint.hash as string,
mode: "restore",
operation: "delete",
})
// Save the updated messages to disk after checkpoint restoration
// This ensures the deleted messages are persisted before reinitialization
await saveTaskMessages({
messages: currentCline.clineMessages,
taskId: currentCline.taskId,
globalStoragePath: provider.contextProxy.globalStorageUri.fsPath,
})
}
} else {
// For non-checkpoint deletes, preserve checkpoint associations for remaining messages
// Store checkpoints from messages that will be preserved
const preservedCheckpoints = new Map<number, any>()
for (let i = 0; i < messageIndex; i++) {
const msg = currentCline.clineMessages[i]
if (msg?.checkpoint && msg.ts) {
preservedCheckpoints.set(msg.ts, msg.checkpoint)
}
}
// Delete this message and all subsequent messages
await removeMessagesThisAndSubsequent(currentCline, messageIndex, apiConversationHistoryIndex)
// Restore checkpoint associations for preserved messages
for (const [ts, checkpoint] of preservedCheckpoints) {
const msgIndex = currentCline.clineMessages.findIndex((msg) => msg.ts === ts)
if (msgIndex !== -1) {
currentCline.clineMessages[msgIndex].checkpoint = checkpoint
}
}
// Save the updated messages with restored checkpoints
await saveTaskMessages({
messages: currentCline.clineMessages,
taskId: currentCline.taskId,
globalStoragePath: provider.contextProxy.globalStorageUri.fsPath,
})
}
const { historyItem } = await provider.getTaskWithId(currentCline.taskId)
// Delete this message and all subsequent messages
await removeMessagesThisAndSubsequent(currentCline, messageIndex, apiConversationHistoryIndex)
// Initialize with history item after deletion
await provider.initClineWithHistoryItem(historyItem)
// Initialize with history item after deletion (only for checkpoint restores)
if (restoreCheckpoint) {
await provider.initClineWithHistoryItem(historyItem)
}
} catch (error) {
console.error("Error in delete message:", error)
vscode.window.showErrorMessage(
@ -264,27 +301,79 @@ export const webviewMessageHandler = async (
if (messageIndex !== -1) {
try {
const targetMessage = currentCline.clineMessages[messageIndex]
// Preserve the original checkpoint data for the edited message
const originalCheckpoint = targetMessage?.checkpoint
// If checkpoint restoration is requested, restore to the checkpoint first
if (restoreCheckpoint) {
const targetMessage = currentCline.clineMessages[messageIndex]
if (
targetMessage?.checkpoint &&
typeof targetMessage.checkpoint === "object" &&
"hash" in targetMessage.checkpoint
originalCheckpoint &&
typeof originalCheckpoint === "object" &&
"hash" in originalCheckpoint
) {
// Store the edited content and images for after restoration
const editData = { text: editedContent, images }
// Set a flag on the provider to indicate we need to process an edit after restoration
;(provider as any).pendingEditAfterRestore = {
messageTs,
editedContent,
images,
messageIndex,
apiConversationHistoryIndex,
originalCheckpoint, // Preserve the checkpoint for the new message
}
await currentCline.checkpointRestore({
ts: targetMessage.ts!,
commitHash: targetMessage.checkpoint.hash as string,
commitHash: originalCheckpoint.hash as string,
mode: "restore",
operation: "edit",
})
// The task will be cancelled and reinitialized by checkpointRestore
// The pending edit will be processed in the reinitialized task
return
}
}
// For non-checkpoint edits, preserve checkpoint associations for remaining messages
// Store checkpoints from messages that will be preserved
const preservedCheckpoints = new Map<number, any>()
for (let i = 0; i < messageIndex; i++) {
const msg = currentCline.clineMessages[i]
if (msg?.checkpoint && msg.ts) {
preservedCheckpoints.set(msg.ts, msg.checkpoint)
}
}
// Edit this message and delete subsequent
await removeMessagesThisAndSubsequent(currentCline, messageIndex, apiConversationHistoryIndex)
// Restore checkpoint associations for preserved messages
for (const [ts, checkpoint] of preservedCheckpoints) {
const msgIndex = currentCline.clineMessages.findIndex((msg) => msg.ts === ts)
if (msgIndex !== -1) {
currentCline.clineMessages[msgIndex].checkpoint = checkpoint
}
}
// Save the updated messages with restored checkpoints
await saveTaskMessages({
messages: currentCline.clineMessages,
taskId: currentCline.taskId,
globalStoragePath: provider.contextProxy.globalStorageUri.fsPath,
})
// Process the edited message as a regular user message
// This will add it to the conversation and trigger an AI response
// Preserve the original checkpoint for the new message
if (originalCheckpoint) {
// Store the checkpoint to be attached to the new message
currentCline.pendingUserMessageCheckpoint = originalCheckpoint
}
webviewMessageHandler(provider, {
type: "askResponse",
askResponse: "messageResponse",
@ -457,7 +546,33 @@ export const webviewMessageHandler = async (
await provider.postStateToWebview()
break
case "askResponse":
provider.getCurrentCline()?.handleWebviewAskResponse(message.askResponse!, message.text, message.images)
// Save checkpoint BEFORE processing the user message if checkpoints are enabled
const currentCline = provider.getCurrentCline()
if (currentCline && currentCline.enableCheckpoints && message.askResponse === "messageResponse") {
console.log("[webviewMessageHandler] Saving checkpoint before user message processing")
try {
const checkpointResult = await currentCline.checkpointSave(true) // Force checkpoint save
console.log("[webviewMessageHandler] Checkpoint result:", checkpointResult)
if (checkpointResult?.commit) {
// Store checkpoint data temporarily to be used when creating the user_feedback message
currentCline.pendingUserMessageCheckpoint = {
hash: checkpointResult.commit,
timestamp: Date.now(),
type: "user_message",
}
console.log(
"[webviewMessageHandler] Set pendingUserMessageCheckpoint:",
currentCline.pendingUserMessageCheckpoint,
)
} else {
console.log("[webviewMessageHandler] No commit in checkpoint result")
}
} catch (error) {
console.error("[webviewMessageHandler] Failed to save checkpoint before user message:", error)
}
}
currentCline?.handleWebviewAskResponse(message.askResponse!, message.text, message.images)
break
case "autoCondenseContext":
await updateGlobalState("autoCondenseContext", message.bool)

View file

@ -783,7 +783,23 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
)
const visibleMessages = useMemo(() => {
// First, collect all checkpoint hashes that are associated with user_feedback messages
const userMessageCheckpointHashes = new Set<string>()
modifiedMessages.forEach((message) => {
if (message.say === "user_feedback" && message.checkpoint) {
const checkpoint = message.checkpoint as any
if (checkpoint.type === "user_message" && checkpoint.hash) {
userMessageCheckpointHashes.add(checkpoint.hash)
}
}
})
const newVisibleMessages = modifiedMessages.filter((message) => {
// Filter out checkpoint_saved messages that are associated with user messages
if (message.say === "checkpoint_saved" && message.text && userMessageCheckpointHashes.has(message.text)) {
return false
}
if (everVisibleMessagesTsRef.current.has(message.ts)) {
// If it was ever visible, and it's not one of the types that should always be hidden once processed, keep it.
// This helps prevent flickering for messages like 'api_req_retry_delayed' if they are no longer the absolute last.