mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-08 03:07:53 +00:00
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
This commit is contained in:
parent
87b45def18
commit
5f68516936
5 changed files with 82 additions and 13 deletions
|
|
@ -81,3 +81,46 @@ export async function saveApiMessages({
|
|||
const filePath = path.join(taskDir, GlobalFileNames.apiConversationHistory)
|
||||
await safeWriteJson(filePath, messages)
|
||||
}
|
||||
|
||||
/**
|
||||
* Transaction helper for safe read-modify-write operations on API messages.
|
||||
* Ensures atomic updates by reading, modifying, and writing under a conceptual lock.
|
||||
*
|
||||
* @param taskId - The task ID
|
||||
* @param globalStoragePath - The global storage path
|
||||
* @param updater - A pure function that takes the current messages and returns the updated messages
|
||||
* @param options - Optional configuration
|
||||
* @returns The updated messages
|
||||
*/
|
||||
export async function transactApiMessages({
|
||||
taskId,
|
||||
globalStoragePath,
|
||||
updater,
|
||||
options = {},
|
||||
}: {
|
||||
taskId: string
|
||||
globalStoragePath: string
|
||||
updater: (messages: ApiMessage[]) => ApiMessage[]
|
||||
options?: {
|
||||
allowEmpty?: boolean
|
||||
}
|
||||
}): Promise<ApiMessage[]> {
|
||||
// Read current state
|
||||
const currentMessages = await readApiMessages({ taskId, globalStoragePath })
|
||||
|
||||
// Apply the pure updater function
|
||||
const updatedMessages = updater(currentMessages)
|
||||
|
||||
// Guard against unintentional empty writes
|
||||
if (updatedMessages.length === 0 && currentMessages.length > 0 && !options.allowEmpty) {
|
||||
console.warn(
|
||||
`[transactApiMessages] Preventing empty write for taskId: ${taskId}. Current has ${currentMessages.length} messages. Use allowEmpty: true to force.`,
|
||||
)
|
||||
return currentMessages // Return unchanged
|
||||
}
|
||||
|
||||
// Commit the changes
|
||||
await saveApiMessages({ messages: updatedMessages, taskId, globalStoragePath })
|
||||
|
||||
return updatedMessages
|
||||
}
|
||||
|
|
|
|||
|
|
@ -582,7 +582,16 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
await this.saveApiConversationHistory()
|
||||
}
|
||||
|
||||
async overwriteApiConversationHistory(newHistory: ApiMessage[]) {
|
||||
async overwriteApiConversationHistory(newHistory: ApiMessage[], allowEmpty: boolean = false) {
|
||||
// Guard against unintentional empty writes
|
||||
if (newHistory.length === 0 && this.apiConversationHistory.length > 0 && !allowEmpty) {
|
||||
console.warn(
|
||||
`[Task#overwriteApiConversationHistory] Preventing empty write for taskId: ${this.taskId}. ` +
|
||||
`Current has ${this.apiConversationHistory.length} messages. Use allowEmpty: true to force.`,
|
||||
)
|
||||
return // Don't overwrite with empty array unless explicitly allowed
|
||||
}
|
||||
|
||||
this.apiConversationHistory = newHistory
|
||||
await this.saveApiConversationHistory()
|
||||
}
|
||||
|
|
@ -1436,7 +1445,15 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
throw new Error("Unexpected: Last message is not a user or assistant message")
|
||||
}
|
||||
} else {
|
||||
throw new Error("Unexpected: No existing API conversation history")
|
||||
// Handle empty API conversation history gracefully instead of throwing
|
||||
// This prevents the "chat locks until reopen" failure mode
|
||||
this.say(
|
||||
"text",
|
||||
"[TASK RESUMPTION] Previous conversation history was empty. Starting with a fresh baseline.",
|
||||
)
|
||||
// Initialize with empty arrays to allow the task to continue
|
||||
modifiedApiConversationHistory = []
|
||||
modifiedOldUserContent = []
|
||||
}
|
||||
|
||||
let newUserContent: Anthropic.Messages.ContentBlockParam[] = [...modifiedOldUserContent]
|
||||
|
|
|
|||
|
|
@ -918,8 +918,10 @@ export class ClineProvider
|
|||
await task.overwriteClineMessages(task.clineMessages.slice(0, messageIndex))
|
||||
|
||||
if (apiConversationHistoryIndex !== -1) {
|
||||
// Allow empty writes for edit operations after checkpoint restoration
|
||||
await task.overwriteApiConversationHistory(
|
||||
task.apiConversationHistory.slice(0, apiConversationHistoryIndex),
|
||||
true, // allowEmpty: true for edit operations
|
||||
)
|
||||
}
|
||||
|
||||
|
|
@ -1453,6 +1455,7 @@ export class ClineProvider
|
|||
|
||||
if (historyItem) {
|
||||
const { getTaskDirectoryPath } = await import("../../utils/storage")
|
||||
const { readApiMessages } = await import("../task-persistence/apiMessages")
|
||||
const globalStoragePath = this.contextProxy.globalStorageUri.fsPath
|
||||
const taskDirPath = await getTaskDirectoryPath(globalStoragePath, id)
|
||||
const apiConversationHistoryFilePath = path.join(taskDirPath, GlobalFileNames.apiConversationHistory)
|
||||
|
|
@ -1460,7 +1463,8 @@ export class ClineProvider
|
|||
const fileExists = await fileExistsAtPath(apiConversationHistoryFilePath)
|
||||
|
||||
if (fileExists) {
|
||||
const apiConversationHistory = JSON.parse(await fs.readFile(apiConversationHistoryFilePath, "utf8"))
|
||||
// Use the helper reader for unified behavior/logging instead of direct JSON.parse
|
||||
const apiConversationHistory = await readApiMessages({ taskId: id, globalStoragePath })
|
||||
|
||||
return {
|
||||
historyItem,
|
||||
|
|
|
|||
|
|
@ -135,7 +135,7 @@ describe("webviewMessageHandler - Edit Message with Timestamp Fallback", () => {
|
|||
)
|
||||
|
||||
// API history should be truncated from first message at/after edited timestamp (fallback)
|
||||
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([])
|
||||
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([], true)
|
||||
})
|
||||
|
||||
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", () => {
|
|||
])
|
||||
|
||||
// API history should be truncated from the first API message at/after the edited timestamp (fallback)
|
||||
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([
|
||||
{
|
||||
ts: earlierMessageTs,
|
||||
role: "user",
|
||||
content: [{ type: "text", text: "Earlier message" }],
|
||||
},
|
||||
])
|
||||
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith(
|
||||
[
|
||||
{
|
||||
ts: earlierMessageTs,
|
||||
role: "user",
|
||||
content: [{ type: "text", text: "Earlier message" }],
|
||||
},
|
||||
],
|
||||
true,
|
||||
)
|
||||
})
|
||||
|
||||
it("should not use fallback when exact apiConversationHistoryIndex is found", async () => {
|
||||
|
|
@ -248,7 +251,7 @@ describe("webviewMessageHandler - Edit Message with Timestamp Fallback", () => {
|
|||
|
||||
// Both should be truncated at index 0
|
||||
expect(mockCurrentTask.overwriteClineMessages).toHaveBeenCalledWith([])
|
||||
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([])
|
||||
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([], true)
|
||||
})
|
||||
|
||||
it("should handle case where no API messages match timestamp criteria", async () => {
|
||||
|
|
@ -385,6 +388,6 @@ describe("webviewMessageHandler - Edit Message with Timestamp Fallback", () => {
|
|||
expect(mockCurrentTask.overwriteClineMessages).toHaveBeenCalledWith([])
|
||||
|
||||
// API history should be truncated from first message at/after edited timestamp (fallback)
|
||||
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([])
|
||||
expect(mockCurrentTask.overwriteApiConversationHistory).toHaveBeenCalledWith([], true)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -108,8 +108,10 @@ export const webviewMessageHandler = async (
|
|||
await currentCline.overwriteClineMessages(currentCline.clineMessages.slice(0, messageIndex))
|
||||
|
||||
if (apiConversationHistoryIndex !== -1) {
|
||||
// Allow empty writes for edit/delete operations
|
||||
await currentCline.overwriteApiConversationHistory(
|
||||
currentCline.apiConversationHistory.slice(0, apiConversationHistoryIndex),
|
||||
true, // allowEmpty: true for edit/delete operations
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue