Skip to content

Commit 5f68516

Browse files
committed
fix: prevent empty history corruption during cancel/resume operations
- Make resume tolerant of empty API conversation history - Standardize reads using helper functions instead of direct JSON.parse - Add transaction helper for safe read-modify-write operations - Guard against unintentional empty writes with allowEmpty flag This fixes the issue where canceling during model's thinking/streaming phase could leave the API history file as an empty array ([]), causing the chat to lock when resuming. Fixes #8153 and #5333
1 parent 87b45de commit 5f68516

File tree

5 files changed

+82
-13
lines changed

5 files changed

+82
-13
lines changed

src/core/task-persistence/apiMessages.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,46 @@ export async function saveApiMessages({
8181
const filePath = path.join(taskDir, GlobalFileNames.apiConversationHistory)
8282
await safeWriteJson(filePath, messages)
8383
}
84+
85+
/**
86+
* Transaction helper for safe read-modify-write operations on API messages.
87+
* Ensures atomic updates by reading, modifying, and writing under a conceptual lock.
88+
*
89+
* @param taskId - The task ID
90+
* @param globalStoragePath - The global storage path
91+
* @param updater - A pure function that takes the current messages and returns the updated messages
92+
* @param options - Optional configuration
93+
* @returns The updated messages
94+
*/
95+
export async function transactApiMessages({
96+
taskId,
97+
globalStoragePath,
98+
updater,
99+
options = {},
100+
}: {
101+
taskId: string
102+
globalStoragePath: string
103+
updater: (messages: ApiMessage[]) => ApiMessage[]
104+
options?: {
105+
allowEmpty?: boolean
106+
}
107+
}): Promise<ApiMessage[]> {
108+
// Read current state
109+
const currentMessages = await readApiMessages({ taskId, globalStoragePath })
110+
111+
// Apply the pure updater function
112+
const updatedMessages = updater(currentMessages)
113+
114+
// Guard against unintentional empty writes
115+
if (updatedMessages.length === 0 && currentMessages.length > 0 && !options.allowEmpty) {
116+
console.warn(
117+
`[transactApiMessages] Preventing empty write for taskId: ${taskId}. Current has ${currentMessages.length} messages. Use allowEmpty: true to force.`,
118+
)
119+
return currentMessages // Return unchanged
120+
}
121+
122+
// Commit the changes
123+
await saveApiMessages({ messages: updatedMessages, taskId, globalStoragePath })
124+
125+
return updatedMessages
126+
}

src/core/task/Task.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,16 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
582582
await this.saveApiConversationHistory()
583583
}
584584

585-
async overwriteApiConversationHistory(newHistory: ApiMessage[]) {
585+
async overwriteApiConversationHistory(newHistory: ApiMessage[], allowEmpty: boolean = false) {
586+
// Guard against unintentional empty writes
587+
if (newHistory.length === 0 && this.apiConversationHistory.length > 0 && !allowEmpty) {
588+
console.warn(
589+
`[Task#overwriteApiConversationHistory] Preventing empty write for taskId: ${this.taskId}. ` +
590+
`Current has ${this.apiConversationHistory.length} messages. Use allowEmpty: true to force.`,
591+
)
592+
return // Don't overwrite with empty array unless explicitly allowed
593+
}
594+
586595
this.apiConversationHistory = newHistory
587596
await this.saveApiConversationHistory()
588597
}
@@ -1436,7 +1445,15 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
14361445
throw new Error("Unexpected: Last message is not a user or assistant message")
14371446
}
14381447
} else {
1439-
throw new Error("Unexpected: No existing API conversation history")
1448+
// Handle empty API conversation history gracefully instead of throwing
1449+
// This prevents the "chat locks until reopen" failure mode
1450+
this.say(
1451+
"text",
1452+
"[TASK RESUMPTION] Previous conversation history was empty. Starting with a fresh baseline.",
1453+
)
1454+
// Initialize with empty arrays to allow the task to continue
1455+
modifiedApiConversationHistory = []
1456+
modifiedOldUserContent = []
14401457
}
14411458

14421459
let newUserContent: Anthropic.Messages.ContentBlockParam[] = [...modifiedOldUserContent]

src/core/webview/ClineProvider.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -918,8 +918,10 @@ export class ClineProvider
918918
await task.overwriteClineMessages(task.clineMessages.slice(0, messageIndex))
919919

920920
if (apiConversationHistoryIndex !== -1) {
921+
// Allow empty writes for edit operations after checkpoint restoration
921922
await task.overwriteApiConversationHistory(
922923
task.apiConversationHistory.slice(0, apiConversationHistoryIndex),
924+
true, // allowEmpty: true for edit operations
923925
)
924926
}
925927

@@ -1453,14 +1455,16 @@ export class ClineProvider
14531455

14541456
if (historyItem) {
14551457
const { getTaskDirectoryPath } = await import("../../utils/storage")
1458+
const { readApiMessages } = await import("../task-persistence/apiMessages")
14561459
const globalStoragePath = this.contextProxy.globalStorageUri.fsPath
14571460
const taskDirPath = await getTaskDirectoryPath(globalStoragePath, id)
14581461
const apiConversationHistoryFilePath = path.join(taskDirPath, GlobalFileNames.apiConversationHistory)
14591462
const uiMessagesFilePath = path.join(taskDirPath, GlobalFileNames.uiMessages)
14601463
const fileExists = await fileExistsAtPath(apiConversationHistoryFilePath)
14611464

14621465
if (fileExists) {
1463-
const apiConversationHistory = JSON.parse(await fs.readFile(apiConversationHistoryFilePath, "utf8"))
1466+
// Use the helper reader for unified behavior/logging instead of direct JSON.parse
1467+
const apiConversationHistory = await readApiMessages({ taskId: id, globalStoragePath })
14641468

14651469
return {
14661470
historyItem,

src/core/webview/__tests__/webviewMessageHandler.edit.spec.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ describe("webviewMessageHandler - Edit Message with Timestamp Fallback", () => {
135135
)
136136

137137
// API history should be truncated from first message at/after edited timestamp (fallback)
138-
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([])
138+
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([], true)
139139
})
140140

141141
it("should preserve messages before the edited message when message not in API history", async () => {
@@ -197,13 +197,16 @@ describe("webviewMessageHandler - Edit Message with Timestamp Fallback", () => {
197197
])
198198

199199
// API history should be truncated from the first API message at/after the edited timestamp (fallback)
200-
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([
201-
{
202-
ts: earlierMessageTs,
203-
role: "user",
204-
content: [{ type: "text", text: "Earlier message" }],
205-
},
206-
])
200+
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith(
201+
[
202+
{
203+
ts: earlierMessageTs,
204+
role: "user",
205+
content: [{ type: "text", text: "Earlier message" }],
206+
},
207+
],
208+
true,
209+
)
207210
})
208211

209212
it("should not use fallback when exact apiConversationHistoryIndex is found", async () => {
@@ -248,7 +251,7 @@ describe("webviewMessageHandler - Edit Message with Timestamp Fallback", () => {
248251

249252
// Both should be truncated at index 0
250253
expect(mockCurrentTask.overwriteClineMessages).toHaveBeenCalledWith([])
251-
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([])
254+
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([], true)
252255
})
253256

254257
it("should handle case where no API messages match timestamp criteria", async () => {
@@ -385,6 +388,6 @@ describe("webviewMessageHandler - Edit Message with Timestamp Fallback", () => {
385388
expect(mockCurrentTask.overwriteClineMessages).toHaveBeenCalledWith([])
386389

387390
// API history should be truncated from first message at/after edited timestamp (fallback)
388-
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([])
391+
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([], true)
389392
})
390393
})

src/core/webview/webviewMessageHandler.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,10 @@ export const webviewMessageHandler = async (
108108
await currentCline.overwriteClineMessages(currentCline.clineMessages.slice(0, messageIndex))
109109

110110
if (apiConversationHistoryIndex !== -1) {
111+
// Allow empty writes for edit/delete operations
111112
await currentCline.overwriteApiConversationHistory(
112113
currentCline.apiConversationHistory.slice(0, apiConversationHistoryIndex),
114+
true, // allowEmpty: true for edit/delete operations
113115
)
114116
}
115117
}

0 commit comments

Comments
 (0)