diff --git a/src/core/task-persistence/apiMessages.ts b/src/core/task-persistence/apiMessages.ts index f846aaf13f..cb019e94b1 100644 --- a/src/core/task-persistence/apiMessages.ts +++ b/src/core/task-persistence/apiMessages.ts @@ -77,7 +77,43 @@ export async function saveApiMessages({ taskId: string globalStoragePath: string }) { + // Validate messages before saving to prevent data corruption + if (!Array.isArray(messages)) { + console.error( + `[Roo-Debug] saveApiMessages: Invalid messages format - expected array, got ${typeof messages}. TaskId: ${taskId}`, + ) + throw new Error("Invalid messages format for saving") + } + + // Log warning for unusually large conversations + if (messages.length > 1000) { + console.warn( + `[Roo-Debug] saveApiMessages: Saving large conversation with ${messages.length} messages. TaskId: ${taskId}`, + ) + } + + // Validate message structure + for (let i = 0; i < messages.length; i++) { + const msg = messages[i] + if (!msg || typeof msg !== "object") { + console.error( + `[Roo-Debug] saveApiMessages: Invalid message at index ${i} - expected object, got ${typeof msg}. TaskId: ${taskId}`, + ) + throw new Error(`Invalid message structure at index ${i}`) + } + if (!msg.role || (msg.role !== "user" && msg.role !== "assistant")) { + console.error( + `[Roo-Debug] saveApiMessages: Invalid message role at index ${i} - got "${msg.role}". TaskId: ${taskId}`, + ) + throw new Error(`Invalid message role at index ${i}`) + } + } + const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId) const filePath = path.join(taskDir, GlobalFileNames.apiConversationHistory) + + // Log the save operation for debugging + console.log(`[Roo-Debug] saveApiMessages: Saving ${messages.length} messages to ${filePath}. TaskId: ${taskId}`) + await safeWriteJson(filePath, messages) } diff --git a/src/core/task-persistence/taskMessages.ts b/src/core/task-persistence/taskMessages.ts index 63a2eefbaa..f2e8d05766 100644 --- a/src/core/task-persistence/taskMessages.ts +++ b/src/core/task-persistence/taskMessages.ts @@ -36,7 +36,26 @@ export type SaveTaskMessagesOptions = { } export async function saveTaskMessages({ messages, taskId, globalStoragePath }: SaveTaskMessagesOptions) { + // Validate messages before saving to prevent data corruption + if (!Array.isArray(messages)) { + console.error( + `[Roo-Debug] saveTaskMessages: Invalid messages format - expected array, got ${typeof messages}. TaskId: ${taskId}`, + ) + throw new Error("Invalid messages format for saving") + } + + // Log warning for unusually large conversations + if (messages.length > 1000) { + console.warn( + `[Roo-Debug] saveTaskMessages: Saving large conversation with ${messages.length} messages. TaskId: ${taskId}`, + ) + } + const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId) const filePath = path.join(taskDir, GlobalFileNames.uiMessages) + + // Log the save operation for debugging + console.log(`[Roo-Debug] saveTaskMessages: Saving ${messages.length} UI messages to ${filePath}. TaskId: ${taskId}`) + await safeWriteJson(filePath, messages) } diff --git a/src/core/webview/__tests__/webviewMessageHandler.findMessageIndices.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.findMessageIndices.spec.ts new file mode 100644 index 0000000000..1426b6a592 --- /dev/null +++ b/src/core/webview/__tests__/webviewMessageHandler.findMessageIndices.spec.ts @@ -0,0 +1,229 @@ +import { describe, it, expect, vi, beforeEach } from "vitest" +import { webviewMessageHandler } from "../webviewMessageHandler" +import type { ClineProvider } from "../ClineProvider" +import type { ClineMessage } from "@roo-code/types" + +describe("webviewMessageHandler - findMessageIndices", () => { + let mockClineProvider: any + let mockTask: any + + beforeEach(() => { + // Create mock messages with specific timestamps + const mockMessages: ClineMessage[] = [ + { ts: 1000, type: "say", say: "user_feedback", text: "Message 1" }, + { ts: 1500, type: "say", say: "user_feedback", text: "Message 2" }, + { ts: 2000, type: "say", say: "user_feedback", text: "Message 3" }, + { ts: 2999, type: "say", say: "user_feedback", text: "Message 4" }, // Within 1 second of message 3 + { ts: 3000, type: "say", say: "user_feedback", text: "Message 5" }, + ] + + const mockApiHistory = [ + { ts: 1000, role: "user", content: "API Message 1" }, + { ts: 1500, role: "assistant", content: "API Message 2" }, + { ts: 2000, role: "user", content: "API Message 3" }, + { ts: 2999, role: "assistant", content: "API Message 4" }, + { ts: 3000, role: "user", content: "API Message 5" }, + ] + + mockTask = { + taskId: "test-task-id", + clineMessages: mockMessages, + apiConversationHistory: mockApiHistory, + overwriteClineMessages: vi.fn(), + overwriteApiConversationHistory: vi.fn(), + handleWebviewAskResponse: vi.fn(), + } + + mockClineProvider = { + getCurrentTask: vi.fn(() => mockTask), + getTaskWithId: vi.fn().mockResolvedValue({ + historyItem: { ts: Date.now(), task: "Test", tokensIn: 0, tokensOut: 0 }, + }), + createTaskWithHistoryItem: vi.fn(), + postMessageToWebview: vi.fn(), + } as unknown as ClineProvider + }) + + describe("deleteMessage with exact timestamp matching", () => { + it("should delete only the message with exact timestamp match", async () => { + // First, show the delete dialog + await webviewMessageHandler(mockClineProvider, { + type: "deleteMessage", + value: 2000, // Delete message at timestamp 2000 + }) + + // Verify dialog was shown + expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({ + type: "showDeleteMessageDialog", + messageTs: 2000, + }) + + // Now confirm the deletion + await webviewMessageHandler(mockClineProvider, { + type: "deleteMessageConfirm", + messageTs: 2000, + }) + + // Should delete from index 2 onwards (messages 3, 4, 5) + expect(mockTask.overwriteClineMessages).toHaveBeenCalledWith([ + mockTask.clineMessages[0], + mockTask.clineMessages[1], + ]) + + expect(mockTask.overwriteApiConversationHistory).toHaveBeenCalledWith([ + mockTask.apiConversationHistory[0], + mockTask.apiConversationHistory[1], + ]) + }) + + it("should not delete messages within 1 second buffer when using exact matching", async () => { + // Delete message at timestamp 3000 + await webviewMessageHandler(mockClineProvider, { + type: "deleteMessage", + value: 3000, + }) + + await webviewMessageHandler(mockClineProvider, { + type: "deleteMessageConfirm", + messageTs: 3000, + }) + + // Should delete only from index 4 (message 5), NOT from index 3 (message 4 at 2999) + expect(mockTask.overwriteClineMessages).toHaveBeenCalledWith([ + mockTask.clineMessages[0], + mockTask.clineMessages[1], + mockTask.clineMessages[2], + mockTask.clineMessages[3], // Message at 2999 should be preserved + ]) + + expect(mockTask.overwriteApiConversationHistory).toHaveBeenCalledWith([ + mockTask.apiConversationHistory[0], + mockTask.apiConversationHistory[1], + mockTask.apiConversationHistory[2], + mockTask.apiConversationHistory[3], // API message at 2999 should be preserved + ]) + }) + + it("should handle case when message timestamp is not found", async () => { + // Try to delete a message with non-existent timestamp + await webviewMessageHandler(mockClineProvider, { + type: "deleteMessage", + value: 9999, + }) + + await webviewMessageHandler(mockClineProvider, { + type: "deleteMessageConfirm", + messageTs: 9999, + }) + + // Should not call overwrite methods since message wasn't found + expect(mockTask.overwriteClineMessages).not.toHaveBeenCalled() + expect(mockTask.overwriteApiConversationHistory).not.toHaveBeenCalled() + expect(mockClineProvider.createTaskWithHistoryItem).not.toHaveBeenCalled() + }) + }) + + describe("editMessage with exact timestamp matching", () => { + it("should edit only the message with exact timestamp match", async () => { + // First, show the edit dialog + await webviewMessageHandler(mockClineProvider, { + type: "submitEditedMessage", + value: 2000, + editedMessageContent: "Edited message content", + }) + + // Verify dialog was shown + expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({ + type: "showEditMessageDialog", + messageTs: 2000, + text: "Edited message content", + images: undefined, + }) + + // Now confirm the edit + await webviewMessageHandler(mockClineProvider, { + type: "editMessageConfirm", + messageTs: 2000, + text: "Edited message content", + }) + + // Should delete from index 2 onwards before adding the edited message + expect(mockTask.overwriteClineMessages).toHaveBeenCalledWith([ + mockTask.clineMessages[0], + mockTask.clineMessages[1], + ]) + + expect(mockTask.overwriteApiConversationHistory).toHaveBeenCalledWith([ + mockTask.apiConversationHistory[0], + mockTask.apiConversationHistory[1], + ]) + }) + + it("should not affect messages within 1 second buffer when editing", async () => { + // Edit message at timestamp 3000 + await webviewMessageHandler(mockClineProvider, { + type: "submitEditedMessage", + value: 3000, + editedMessageContent: "Edited message at 3000", + }) + + await webviewMessageHandler(mockClineProvider, { + type: "editMessageConfirm", + messageTs: 3000, + text: "Edited message at 3000", + }) + + // Should delete only from index 4, preserving message at 2999 + expect(mockTask.overwriteClineMessages).toHaveBeenCalledWith([ + mockTask.clineMessages[0], + mockTask.clineMessages[1], + mockTask.clineMessages[2], + mockTask.clineMessages[3], // Message at 2999 should be preserved + ]) + }) + }) + + describe("edge cases", () => { + it("should handle empty message arrays gracefully", async () => { + mockTask.clineMessages = [] + mockTask.apiConversationHistory = [] + + await webviewMessageHandler(mockClineProvider, { + type: "deleteMessage", + value: 1000, + }) + + await webviewMessageHandler(mockClineProvider, { + type: "deleteMessageConfirm", + messageTs: 1000, + }) + + // Should not throw errors and should not call overwrite methods + expect(mockTask.overwriteClineMessages).not.toHaveBeenCalled() + expect(mockTask.overwriteApiConversationHistory).not.toHaveBeenCalled() + }) + + it("should handle messages with duplicate timestamps correctly", async () => { + // Create messages with duplicate timestamps + mockTask.clineMessages = [ + { ts: 1000, type: "say", say: "user_feedback", text: "Message 1" }, + { ts: 2000, type: "say", say: "user_feedback", text: "Message 2a" }, + { ts: 2000, type: "say", say: "user_feedback", text: "Message 2b" }, // Duplicate timestamp + { ts: 3000, type: "say", say: "user_feedback", text: "Message 3" }, + ] + + await webviewMessageHandler(mockClineProvider, { + type: "deleteMessage", + value: 2000, + }) + + await webviewMessageHandler(mockClineProvider, { + type: "deleteMessageConfirm", + messageTs: 2000, + }) + + // Should delete from the first occurrence of timestamp 2000 + expect(mockTask.overwriteClineMessages).toHaveBeenCalledWith([mockTask.clineMessages[0]]) + }) + }) +}) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 4dd0fee75e..9c29f25416 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -70,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) + // Use exact timestamp matching to prevent unintended deletion of unrelated messages + 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 } } @@ -86,6 +86,25 @@ export const webviewMessageHandler = async ( messageIndex: number, apiConversationHistoryIndex: number, ) => { + // Validate indices before deletion to prevent accidental data loss + if (messageIndex < 0 || messageIndex >= currentCline.clineMessages.length) { + console.error( + `[Chat History Protection] Invalid message index ${messageIndex} for clineMessages array of length ${currentCline.clineMessages.length}`, + ) + throw new Error("Invalid message index for deletion") + } + + // Log the deletion for debugging + const messagesToDelete = currentCline.clineMessages.length - messageIndex + const apiMessagesToDelete = + apiConversationHistoryIndex !== -1 + ? currentCline.apiConversationHistory.length - apiConversationHistoryIndex + : 0 + + console.log( + `[Chat History] Deleting ${messagesToDelete} UI messages and ${apiMessagesToDelete} API messages from index ${messageIndex}`, + ) + // Delete this message and all that follow await currentCline.overwriteClineMessages(currentCline.clineMessages.slice(0, messageIndex)) @@ -118,6 +137,11 @@ export const webviewMessageHandler = async ( if (messageIndex !== -1) { try { + // Log the operation for debugging + console.log( + `[Chat History] Delete operation requested for message at timestamp ${messageTs}, found at index ${messageIndex}`, + ) + const { historyItem } = await provider.getTaskWithId(currentCline.taskId) // Delete this message and all subsequent messages @@ -131,6 +155,10 @@ export const webviewMessageHandler = async ( `Error deleting message: ${error instanceof Error ? error.message : String(error)}`, ) } + } else { + console.warn( + `[Chat History] Message with timestamp ${messageTs} not found for deletion. Total messages: ${currentCline.clineMessages.length}`, + ) } } } @@ -165,6 +193,11 @@ export const webviewMessageHandler = async ( if (messageIndex !== -1) { try { + // Log the operation for debugging + console.log( + `[Chat History] Edit operation requested for message at timestamp ${messageTs}, found at index ${messageIndex}`, + ) + // Edit this message and delete subsequent await removeMessagesThisAndSubsequent(currentCline, messageIndex, apiConversationHistoryIndex) @@ -185,6 +218,10 @@ export const webviewMessageHandler = async ( `Error editing message: ${error instanceof Error ? error.message : String(error)}`, ) } + } else { + console.warn( + `[Chat History] Message with timestamp ${messageTs} not found for editing. Total messages: ${currentCline.clineMessages.length}`, + ) } } }