Skip to content

Commit 7015608

Browse files
committed
fix race cond and lint
1 parent caee1dc commit 7015608

File tree

5 files changed

+285
-191
lines changed

5 files changed

+285
-191
lines changed

src/core/checkpoints/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ export async function checkpointSave(cline: Task, force = false) {
179179
} catch (err) {
180180
console.error("[Task#checkpointSave] caught unexpected error, disabling checkpoints", err)
181181
cline.enableCheckpoints = false
182+
return undefined
182183
}
183184
})()
184185

Lines changed: 243 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,243 @@
1+
import { describe, it, expect, vi, beforeEach } from "vitest"
2+
import { handleCheckpointRestoreOperation } from "../checkpointRestoreHandler"
3+
import { saveTaskMessages } from "../../task-persistence"
4+
import pWaitFor from "p-wait-for"
5+
import * as vscode from "vscode"
6+
7+
// Mock dependencies
8+
vi.mock("../../task-persistence", () => ({
9+
saveTaskMessages: vi.fn(),
10+
}))
11+
vi.mock("p-wait-for")
12+
vi.mock("vscode", () => ({
13+
window: {
14+
showErrorMessage: vi.fn(),
15+
},
16+
}))
17+
18+
describe("checkpointRestoreHandler", () => {
19+
let mockProvider: any
20+
let mockCline: any
21+
22+
beforeEach(() => {
23+
vi.clearAllMocks()
24+
25+
// Setup mock Cline instance
26+
mockCline = {
27+
taskId: "test-task-123",
28+
abort: false,
29+
abortTask: vi.fn(() => {
30+
mockCline.abort = true
31+
}),
32+
checkpointRestore: vi.fn(),
33+
clineMessages: [
34+
{ ts: 1, type: "user", say: "user", text: "First message" },
35+
{ ts: 2, type: "assistant", say: "assistant", text: "Response" },
36+
{
37+
ts: 3,
38+
type: "user",
39+
say: "user",
40+
text: "Checkpoint message",
41+
checkpoint: { hash: "abc123" },
42+
},
43+
{ ts: 4, type: "assistant", say: "assistant", text: "After checkpoint" },
44+
],
45+
}
46+
47+
// Setup mock provider
48+
mockProvider = {
49+
getCurrentCline: vi.fn(() => mockCline),
50+
postMessageToWebview: vi.fn(),
51+
getTaskWithId: vi.fn(() => ({
52+
historyItem: { id: "test-task-123", messages: mockCline.clineMessages },
53+
})),
54+
initClineWithHistoryItem: vi.fn(),
55+
setPendingEditOperation: vi.fn(),
56+
contextProxy: {
57+
globalStorageUri: { fsPath: "/test/storage" },
58+
},
59+
}
60+
61+
// Mock pWaitFor to resolve immediately
62+
;(pWaitFor as any).mockImplementation(async (condition: () => boolean) => {
63+
// Simulate the condition being met
64+
return Promise.resolve()
65+
})
66+
})
67+
68+
describe("handleCheckpointRestoreOperation", () => {
69+
it("should abort task before checkpoint restore for delete operations", async () => {
70+
// Simulate a task that hasn't been aborted yet
71+
mockCline.abort = false
72+
73+
await handleCheckpointRestoreOperation({
74+
provider: mockProvider,
75+
currentCline: mockCline,
76+
messageTs: 3,
77+
messageIndex: 2,
78+
checkpoint: { hash: "abc123" },
79+
operation: "delete",
80+
})
81+
82+
// Verify abortTask was called before checkpointRestore
83+
expect(mockCline.abortTask).toHaveBeenCalled()
84+
expect(mockCline.checkpointRestore).toHaveBeenCalled()
85+
86+
// Verify the order of operations
87+
const abortOrder = mockCline.abortTask.mock.invocationCallOrder[0]
88+
const restoreOrder = mockCline.checkpointRestore.mock.invocationCallOrder[0]
89+
expect(abortOrder).toBeLessThan(restoreOrder)
90+
})
91+
92+
it("should not abort task if already aborted", async () => {
93+
// Simulate a task that's already aborted
94+
mockCline.abort = true
95+
96+
await handleCheckpointRestoreOperation({
97+
provider: mockProvider,
98+
currentCline: mockCline,
99+
messageTs: 3,
100+
messageIndex: 2,
101+
checkpoint: { hash: "abc123" },
102+
operation: "delete",
103+
})
104+
105+
// Verify abortTask was not called
106+
expect(mockCline.abortTask).not.toHaveBeenCalled()
107+
expect(mockCline.checkpointRestore).toHaveBeenCalled()
108+
})
109+
110+
it("should handle edit operations with pending edit data", async () => {
111+
const editData = {
112+
editedContent: "Edited content",
113+
images: ["image1.png"],
114+
apiConversationHistoryIndex: 2,
115+
}
116+
117+
await handleCheckpointRestoreOperation({
118+
provider: mockProvider,
119+
currentCline: mockCline,
120+
messageTs: 3,
121+
messageIndex: 2,
122+
checkpoint: { hash: "abc123" },
123+
operation: "edit",
124+
editData,
125+
})
126+
127+
// Verify abortTask was NOT called for edit operations
128+
expect(mockCline.abortTask).not.toHaveBeenCalled()
129+
130+
// Verify pending edit operation was set
131+
expect(mockProvider.setPendingEditOperation).toHaveBeenCalledWith("task-test-task-123", {
132+
messageTs: 3,
133+
editedContent: "Edited content",
134+
images: ["image1.png"],
135+
messageIndex: 2,
136+
apiConversationHistoryIndex: 2,
137+
originalCheckpoint: { hash: "abc123" },
138+
})
139+
140+
// Verify checkpoint restore was called with edit operation
141+
expect(mockCline.checkpointRestore).toHaveBeenCalledWith({
142+
ts: 3,
143+
commitHash: "abc123",
144+
mode: "restore",
145+
operation: "edit",
146+
})
147+
})
148+
149+
it("should save messages after delete operation", async () => {
150+
// Mock the checkpoint restore to simulate message deletion
151+
mockCline.checkpointRestore.mockImplementation(async () => {
152+
mockCline.clineMessages = mockCline.clineMessages.slice(0, 2)
153+
})
154+
155+
await handleCheckpointRestoreOperation({
156+
provider: mockProvider,
157+
currentCline: mockCline,
158+
messageTs: 3,
159+
messageIndex: 2,
160+
checkpoint: { hash: "abc123" },
161+
operation: "delete",
162+
})
163+
164+
// Verify saveTaskMessages was called
165+
expect(saveTaskMessages).toHaveBeenCalledWith({
166+
messages: mockCline.clineMessages,
167+
taskId: "test-task-123",
168+
globalStoragePath: "/test/storage",
169+
})
170+
171+
// Verify initClineWithHistoryItem was called
172+
expect(mockProvider.initClineWithHistoryItem).toHaveBeenCalled()
173+
})
174+
175+
it("should reinitialize task with correct history item after delete", async () => {
176+
const expectedHistoryItem = {
177+
id: "test-task-123",
178+
messages: mockCline.clineMessages,
179+
}
180+
181+
await handleCheckpointRestoreOperation({
182+
provider: mockProvider,
183+
currentCline: mockCline,
184+
messageTs: 3,
185+
messageIndex: 2,
186+
checkpoint: { hash: "abc123" },
187+
operation: "delete",
188+
})
189+
190+
// Verify getTaskWithId was called
191+
expect(mockProvider.getTaskWithId).toHaveBeenCalledWith("test-task-123")
192+
193+
// Verify initClineWithHistoryItem was called with the correct history item
194+
expect(mockProvider.initClineWithHistoryItem).toHaveBeenCalledWith(expectedHistoryItem)
195+
})
196+
197+
it("should not save messages or reinitialize for edit operation", async () => {
198+
const editData = {
199+
editedContent: "Edited content",
200+
images: [],
201+
apiConversationHistoryIndex: 2,
202+
}
203+
204+
await handleCheckpointRestoreOperation({
205+
provider: mockProvider,
206+
currentCline: mockCline,
207+
messageTs: 3,
208+
messageIndex: 2,
209+
checkpoint: { hash: "abc123" },
210+
operation: "edit",
211+
editData,
212+
})
213+
214+
// Verify saveTaskMessages was NOT called for edit operation
215+
expect(saveTaskMessages).not.toHaveBeenCalled()
216+
217+
// Verify initClineWithHistoryItem was NOT called for edit operation
218+
expect(mockProvider.initClineWithHistoryItem).not.toHaveBeenCalled()
219+
})
220+
221+
it("should handle errors gracefully", async () => {
222+
// Mock checkpoint restore to throw an error
223+
mockCline.checkpointRestore.mockRejectedValue(new Error("Checkpoint restore failed"))
224+
225+
// The function should throw and show an error message
226+
await expect(
227+
handleCheckpointRestoreOperation({
228+
provider: mockProvider,
229+
currentCline: mockCline,
230+
messageTs: 3,
231+
messageIndex: 2,
232+
checkpoint: { hash: "abc123" },
233+
operation: "delete",
234+
}),
235+
).rejects.toThrow("Checkpoint restore failed")
236+
237+
// Verify error message was shown
238+
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(
239+
"Error during checkpoint restore: Checkpoint restore failed",
240+
)
241+
})
242+
})
243+
})

0 commit comments

Comments
 (0)