fix race cond and lint

This commit is contained in:
Will Li 2025-07-14 15:26:05 -07:00
parent caee1dc87f
commit 701560823e
5 changed files with 285 additions and 191 deletions

View file

@ -179,6 +179,7 @@ export async function checkpointSave(cline: Task, force = false) {
} catch (err) {
console.error("[Task#checkpointSave] caught unexpected error, disabling checkpoints", err)
cline.enableCheckpoints = false
return undefined
}
})()

View file

@ -0,0 +1,243 @@
import { describe, it, expect, vi, beforeEach } from "vitest"
import { handleCheckpointRestoreOperation } from "../checkpointRestoreHandler"
import { saveTaskMessages } from "../../task-persistence"
import pWaitFor from "p-wait-for"
import * as vscode from "vscode"
// Mock dependencies
vi.mock("../../task-persistence", () => ({
saveTaskMessages: vi.fn(),
}))
vi.mock("p-wait-for")
vi.mock("vscode", () => ({
window: {
showErrorMessage: vi.fn(),
},
}))
describe("checkpointRestoreHandler", () => {
let mockProvider: any
let mockCline: any
beforeEach(() => {
vi.clearAllMocks()
// Setup mock Cline instance
mockCline = {
taskId: "test-task-123",
abort: false,
abortTask: vi.fn(() => {
mockCline.abort = true
}),
checkpointRestore: vi.fn(),
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" },
},
{ ts: 4, type: "assistant", say: "assistant", text: "After checkpoint" },
],
}
// 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(),
setPendingEditOperation: vi.fn(),
contextProxy: {
globalStorageUri: { fsPath: "/test/storage" },
},
}
// Mock pWaitFor to resolve immediately
;(pWaitFor as any).mockImplementation(async (condition: () => boolean) => {
// Simulate the condition being met
return Promise.resolve()
})
})
describe("handleCheckpointRestoreOperation", () => {
it("should abort task before checkpoint restore for delete operations", async () => {
// Simulate a task that hasn't been aborted yet
mockCline.abort = false
await handleCheckpointRestoreOperation({
provider: mockProvider,
currentCline: mockCline,
messageTs: 3,
messageIndex: 2,
checkpoint: { hash: "abc123" },
operation: "delete",
})
// Verify abortTask was called before checkpointRestore
expect(mockCline.abortTask).toHaveBeenCalled()
expect(mockCline.checkpointRestore).toHaveBeenCalled()
// Verify the order of operations
const abortOrder = mockCline.abortTask.mock.invocationCallOrder[0]
const restoreOrder = mockCline.checkpointRestore.mock.invocationCallOrder[0]
expect(abortOrder).toBeLessThan(restoreOrder)
})
it("should not abort task if already aborted", async () => {
// Simulate a task that's already aborted
mockCline.abort = true
await handleCheckpointRestoreOperation({
provider: mockProvider,
currentCline: mockCline,
messageTs: 3,
messageIndex: 2,
checkpoint: { hash: "abc123" },
operation: "delete",
})
// Verify abortTask was not called
expect(mockCline.abortTask).not.toHaveBeenCalled()
expect(mockCline.checkpointRestore).toHaveBeenCalled()
})
it("should handle edit operations with pending edit data", async () => {
const editData = {
editedContent: "Edited content",
images: ["image1.png"],
apiConversationHistoryIndex: 2,
}
await handleCheckpointRestoreOperation({
provider: mockProvider,
currentCline: mockCline,
messageTs: 3,
messageIndex: 2,
checkpoint: { hash: "abc123" },
operation: "edit",
editData,
})
// Verify abortTask was NOT called for edit operations
expect(mockCline.abortTask).not.toHaveBeenCalled()
// Verify pending edit operation was set
expect(mockProvider.setPendingEditOperation).toHaveBeenCalledWith("task-test-task-123", {
messageTs: 3,
editedContent: "Edited content",
images: ["image1.png"],
messageIndex: 2,
apiConversationHistoryIndex: 2,
originalCheckpoint: { hash: "abc123" },
})
// Verify checkpoint restore was called with edit operation
expect(mockCline.checkpointRestore).toHaveBeenCalledWith({
ts: 3,
commitHash: "abc123",
mode: "restore",
operation: "edit",
})
})
it("should save messages after delete operation", async () => {
// Mock the checkpoint restore to simulate message deletion
mockCline.checkpointRestore.mockImplementation(async () => {
mockCline.clineMessages = mockCline.clineMessages.slice(0, 2)
})
await handleCheckpointRestoreOperation({
provider: mockProvider,
currentCline: mockCline,
messageTs: 3,
messageIndex: 2,
checkpoint: { hash: "abc123" },
operation: "delete",
})
// Verify saveTaskMessages was called
expect(saveTaskMessages).toHaveBeenCalledWith({
messages: mockCline.clineMessages,
taskId: "test-task-123",
globalStoragePath: "/test/storage",
})
// Verify initClineWithHistoryItem was called
expect(mockProvider.initClineWithHistoryItem).toHaveBeenCalled()
})
it("should reinitialize task with correct history item after delete", async () => {
const expectedHistoryItem = {
id: "test-task-123",
messages: mockCline.clineMessages,
}
await handleCheckpointRestoreOperation({
provider: mockProvider,
currentCline: mockCline,
messageTs: 3,
messageIndex: 2,
checkpoint: { hash: "abc123" },
operation: "delete",
})
// Verify getTaskWithId was called
expect(mockProvider.getTaskWithId).toHaveBeenCalledWith("test-task-123")
// Verify initClineWithHistoryItem was called with the correct history item
expect(mockProvider.initClineWithHistoryItem).toHaveBeenCalledWith(expectedHistoryItem)
})
it("should not save messages or reinitialize for edit operation", async () => {
const editData = {
editedContent: "Edited content",
images: [],
apiConversationHistoryIndex: 2,
}
await handleCheckpointRestoreOperation({
provider: mockProvider,
currentCline: mockCline,
messageTs: 3,
messageIndex: 2,
checkpoint: { hash: "abc123" },
operation: "edit",
editData,
})
// Verify saveTaskMessages was NOT called for edit operation
expect(saveTaskMessages).not.toHaveBeenCalled()
// Verify initClineWithHistoryItem was NOT called for edit operation
expect(mockProvider.initClineWithHistoryItem).not.toHaveBeenCalled()
})
it("should handle errors gracefully", async () => {
// Mock checkpoint restore to throw an error
mockCline.checkpointRestore.mockRejectedValue(new Error("Checkpoint restore failed"))
// The function should throw and show an error message
await expect(
handleCheckpointRestoreOperation({
provider: mockProvider,
currentCline: mockCline,
messageTs: 3,
messageIndex: 2,
checkpoint: { hash: "abc123" },
operation: "delete",
}),
).rejects.toThrow("Checkpoint restore failed")
// Verify error message was shown
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(
"Error during checkpoint restore: Checkpoint restore failed",
)
})
})
})

View file

@ -1,148 +0,0 @@
import { describe, it, expect, vi, beforeEach } from "vitest"
import { handleCheckpointRestoreOperation } from "../checkpointRestoreHandler"
import { hasValidCheckpoint } from "../../checkpoints/utils"
import { saveTaskMessages } from "../../task-persistence"
import * as vscode from "vscode"
vi.mock("../../task-persistence", () => ({
saveTaskMessages: vi.fn(),
}))
vi.mock("vscode", () => ({
window: {
showErrorMessage: vi.fn(),
},
}))
describe("checkpointRestoreHandler", () => {
let mockProvider: any
let mockCline: any
beforeEach(() => {
vi.clearAllMocks()
mockProvider = {
contextProxy: {
globalStorageUri: {
fsPath: "/test/global/storage",
},
},
getTaskWithId: vi.fn().mockResolvedValue({
historyItem: { id: "task123", messages: [] },
}),
initClineWithHistoryItem: vi.fn(),
setPendingEditOperation: vi.fn(),
}
mockCline = {
taskId: "task123",
clineMessages: [
{ ts: 1, text: "Message 1" },
{ ts: 2, text: "Message 2", checkpoint: { hash: "abc123" } },
{ ts: 3, text: "Message 3" },
],
checkpointRestore: vi.fn(),
}
})
describe("handleCheckpointRestoreOperation", () => {
describe("delete operation", () => {
it("should handle delete operation correctly", async () => {
await handleCheckpointRestoreOperation({
provider: mockProvider,
currentCline: mockCline,
messageTs: 2,
messageIndex: 1,
checkpoint: { hash: "abc123" },
operation: "delete",
})
// Should call checkpointRestore with correct params
expect(mockCline.checkpointRestore).toHaveBeenCalledWith({
ts: 2,
commitHash: "abc123",
mode: "restore",
operation: "delete",
})
// Should save messages after restoration
expect(saveTaskMessages).toHaveBeenCalledWith({
messages: mockCline.clineMessages,
taskId: "task123",
globalStoragePath: "/test/global/storage",
})
// Should reinitialize the task
expect(mockProvider.getTaskWithId).toHaveBeenCalledWith("task123")
expect(mockProvider.initClineWithHistoryItem).toHaveBeenCalledWith({
id: "task123",
messages: [],
})
})
})
describe("edit operation", () => {
it("should handle edit operation correctly", async () => {
const editData = {
editedContent: "Edited content",
images: ["image1.png"],
apiConversationHistoryIndex: 1,
}
await handleCheckpointRestoreOperation({
provider: mockProvider,
currentCline: mockCline,
messageTs: 2,
messageIndex: 1,
checkpoint: { hash: "abc123" },
operation: "edit",
editData,
})
// Should call setPendingEditOperation on provider
expect(mockProvider.setPendingEditOperation).toHaveBeenCalledWith("task-task123", {
messageTs: 2,
editedContent: "Edited content",
images: ["image1.png"],
messageIndex: 1,
apiConversationHistoryIndex: 1,
originalCheckpoint: { hash: "abc123" },
})
// Should call checkpointRestore with correct params
expect(mockCline.checkpointRestore).toHaveBeenCalledWith({
ts: 2,
commitHash: "abc123",
mode: "restore",
operation: "edit",
})
// Should NOT save messages or reinitialize for edit
expect(saveTaskMessages).not.toHaveBeenCalled()
expect(mockProvider.initClineWithHistoryItem).not.toHaveBeenCalled()
})
})
describe("error handling", () => {
it("should handle errors and show error message", async () => {
const error = new Error("Checkpoint restore failed")
mockCline.checkpointRestore.mockRejectedValue(error)
await expect(
handleCheckpointRestoreOperation({
provider: mockProvider,
currentCline: mockCline,
messageTs: 2,
messageIndex: 1,
checkpoint: { hash: "abc123" },
operation: "delete",
}),
).rejects.toThrow("Checkpoint restore failed")
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(
"Error during checkpoint restore: Checkpoint restore failed",
)
})
})
})
})

View file

@ -1,11 +1,11 @@
import { describe, it, expect, vi, beforeEach } from "vitest"
import { webviewMessageHandler } from "../webviewMessageHandler"
import { saveTaskMessages } from "../../task-persistence"
import { checkpointRestore } from "../../checkpoints"
import { handleCheckpointRestoreOperation } from "../checkpointRestoreHandler"
// Mock dependencies
vi.mock("../../task-persistence")
vi.mock("../../checkpoints")
vi.mock("../checkpointRestoreHandler")
vi.mock("vscode", () => ({
window: {
showErrorMessage: vi.fn(),
@ -33,7 +33,7 @@ describe("webviewMessageHandler - checkpoint operations", () => {
type: "user",
say: "user",
text: "Checkpoint message",
checkpoint: { hash: "abc123", label: "Test checkpoint" },
checkpoint: { hash: "abc123" },
},
{ ts: 4, type: "assistant", say: "assistant", text: "After checkpoint" },
],
@ -64,12 +64,9 @@ describe("webviewMessageHandler - checkpoint operations", () => {
})
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)
})
it("should call handleCheckpointRestoreOperation for checkpoint deletes", async () => {
// Mock handleCheckpointRestoreOperation
;(handleCheckpointRestoreOperation as any).mockResolvedValue(undefined)
// Call the handler with delete confirmation
await webviewMessageHandler(mockProvider, {
@ -78,25 +75,15 @@ describe("webviewMessageHandler - checkpoint operations", () => {
restoreCheckpoint: true,
})
// Verify checkpoint restore was called with delete operation
expect(mockCline.checkpointRestore).toHaveBeenCalledWith({
ts: 3,
commitHash: "abc123",
mode: "restore",
// Verify handleCheckpointRestoreOperation was called with correct parameters
expect(handleCheckpointRestoreOperation).toHaveBeenCalledWith({
provider: mockProvider,
currentCline: mockCline,
messageTs: 3,
messageIndex: 2,
checkpoint: { hash: "abc123" },
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 () => {
@ -120,9 +107,9 @@ describe("webviewMessageHandler - checkpoint operations", () => {
})
describe("edit operations with checkpoint restoration", () => {
it("should call checkpoint restore with edit operation", async () => {
// Mock the pending edit storage
mockCline.pendingEditOperation = null
it("should call handleCheckpointRestoreOperation for checkpoint edits", async () => {
// Mock handleCheckpointRestoreOperation
;(handleCheckpointRestoreOperation as any).mockResolvedValue(undefined)
// Call the handler with edit confirmation
await webviewMessageHandler(mockProvider, {
@ -132,22 +119,19 @@ describe("webviewMessageHandler - checkpoint operations", () => {
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 on the provider
expect(mockProvider.setPendingEditOperation).toHaveBeenCalledWith("task-test-task-123", {
// Verify handleCheckpointRestoreOperation was called with correct parameters
expect(handleCheckpointRestoreOperation).toHaveBeenCalledWith({
provider: mockProvider,
currentCline: mockCline,
messageTs: 3,
editedContent: "Edited checkpoint message",
images: undefined,
messageIndex: 2,
apiConversationHistoryIndex: 2,
originalCheckpoint: { hash: "abc123", label: "Test checkpoint" },
checkpoint: { hash: "abc123" },
operation: "edit",
editData: {
editedContent: "Edited checkpoint message",
images: undefined,
apiConversationHistoryIndex: 2,
},
})
})
})

View file

@ -28,6 +28,20 @@ export async function handleCheckpointRestoreOperation(config: CheckpointRestore
const { provider, currentCline, messageTs, checkpoint, operation, editData } = config
try {
// For delete operations, ensure the task is properly aborted to handle any pending ask operations
// This prevents "Current ask promise was ignored" errors
// For edit operations, we don't abort because the checkpoint restore will handle it
if (operation === "delete" && currentCline && !currentCline.abort) {
currentCline.abortTask()
// Wait a bit for the abort to complete
await pWaitFor(() => currentCline.abort === true, {
timeout: 1000,
interval: 50,
}).catch(() => {
// Continue even if timeout - the abort flag should be set
})
}
// For edit operations, set up pending edit data before restoration
if (operation === "edit" && editData) {
const operationId = `task-${currentCline.taskId}`