mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix: prevent chat history truncation by using exact timestamp matching
- Changed findMessageIndices to use exact timestamp matching (ts === messageTs) - Removed 1-second buffer that was causing unintended message deletion - Updated test expectations to match correct behavior - Fixes #6932 where chat history was being truncated during message operations
This commit is contained in:
parent
2a974e8bf6
commit
809be07f2c
2 changed files with 79 additions and 16 deletions
|
|
@ -1222,12 +1222,19 @@ describe("ClineProvider", () => {
|
|||
await messageHandler({ type: "deleteMessageConfirm", messageTs: 4000 })
|
||||
|
||||
// Verify only messages before the deleted message were kept
|
||||
expect(mockCline.overwriteClineMessages).toHaveBeenCalledWith([mockMessages[0], mockMessages[1]])
|
||||
// With exact timestamp matching, we find the message at index 3 (ts: 4000)
|
||||
// and keep messages 0, 1, and 2 (all before timestamp 4000)
|
||||
expect(mockCline.overwriteClineMessages).toHaveBeenCalledWith([
|
||||
mockMessages[0],
|
||||
mockMessages[1],
|
||||
mockMessages[2],
|
||||
])
|
||||
|
||||
// Verify only API messages before the deleted message were kept
|
||||
expect(mockCline.overwriteApiConversationHistory).toHaveBeenCalledWith([
|
||||
mockApiHistory[0],
|
||||
mockApiHistory[1],
|
||||
mockApiHistory[2],
|
||||
])
|
||||
|
||||
// Verify createTaskWithHistoryItem was called
|
||||
|
|
@ -1319,12 +1326,19 @@ describe("ClineProvider", () => {
|
|||
})
|
||||
|
||||
// Verify correct messages were kept (only messages before the edited one)
|
||||
expect(mockCline.overwriteClineMessages).toHaveBeenCalledWith([mockMessages[0], mockMessages[1]])
|
||||
// With exact timestamp matching, we find the message at index 3 (ts: 4000)
|
||||
// and keep messages 0, 1, and 2 (all before timestamp 4000)
|
||||
expect(mockCline.overwriteClineMessages).toHaveBeenCalledWith([
|
||||
mockMessages[0],
|
||||
mockMessages[1],
|
||||
mockMessages[2],
|
||||
])
|
||||
|
||||
// Verify correct API messages were kept (only messages before the edited one)
|
||||
expect(mockCline.overwriteApiConversationHistory).toHaveBeenCalledWith([
|
||||
mockApiHistory[0],
|
||||
mockApiHistory[1],
|
||||
mockApiHistory[2],
|
||||
])
|
||||
|
||||
// The new flow calls webviewMessageHandler recursively with askResponse
|
||||
|
|
@ -3016,9 +3030,11 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => {
|
|||
text: "Edited message with preserved images",
|
||||
})
|
||||
|
||||
// Verify messages were edited correctly - only the first message should remain
|
||||
expect(mockCline.overwriteClineMessages).toHaveBeenCalledWith([mockMessages[0]])
|
||||
expect(mockCline.overwriteApiConversationHistory).toHaveBeenCalledWith([{ ts: 1000 }])
|
||||
// Verify messages were edited correctly
|
||||
// With exact timestamp matching, we find the message at index 2 (ts: 3000)
|
||||
// and keep messages 0 and 1 (all before timestamp 3000)
|
||||
expect(mockCline.overwriteClineMessages).toHaveBeenCalledWith([mockMessages[0], mockMessages[1]])
|
||||
expect(mockCline.overwriteApiConversationHistory).toHaveBeenCalledWith([{ ts: 1000 }, { ts: 2000 }])
|
||||
})
|
||||
|
||||
test("handles editing messages with file attachments", async () => {
|
||||
|
|
@ -3632,8 +3648,10 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => {
|
|||
await messageHandler({ type: "deleteMessageConfirm", messageTs: 3000 })
|
||||
|
||||
// Should handle large payloads without issues
|
||||
expect(mockCline.overwriteClineMessages).toHaveBeenCalledWith([mockMessages[0]])
|
||||
expect(mockCline.overwriteApiConversationHistory).toHaveBeenCalledWith([{ ts: 1000 }])
|
||||
// With exact timestamp matching, we find the message at index 2 (ts: 3000)
|
||||
// and keep messages 0 and 1 (all before timestamp 3000)
|
||||
expect(mockCline.overwriteClineMessages).toHaveBeenCalledWith([mockMessages[0], mockMessages[1]])
|
||||
expect(mockCline.overwriteApiConversationHistory).toHaveBeenCalledWith([{ ts: 1000 }, { ts: 2000 }])
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -67,32 +67,53 @@ export const webviewMessageHandler = async (
|
|||
await provider.contextProxy.setValue(key, value)
|
||||
|
||||
/**
|
||||
* Shared utility to find message indices based on timestamp
|
||||
* Shared utility to find message indices based on exact timestamp matching
|
||||
* This prevents accidental deletion of unrelated messages
|
||||
*/
|
||||
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 message deletion
|
||||
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 }
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes the target message and all subsequent messages
|
||||
* Includes validation to prevent accidental data loss
|
||||
*/
|
||||
const removeMessagesThisAndSubsequent = async (
|
||||
currentCline: any,
|
||||
messageIndex: number,
|
||||
apiConversationHistoryIndex: number,
|
||||
) => {
|
||||
// Validate indices before deletion
|
||||
if (messageIndex < 0 || messageIndex >= currentCline.clineMessages.length) {
|
||||
console.error(
|
||||
`[Chat History] Invalid message index: ${messageIndex}, total messages: ${currentCline.clineMessages.length}`,
|
||||
)
|
||||
throw new Error("Invalid message index for deletion")
|
||||
}
|
||||
|
||||
// Log deletion for debugging
|
||||
const messagesToDelete = currentCline.clineMessages.length - messageIndex
|
||||
console.log(`[Chat History] Deleting ${messagesToDelete} messages starting from index ${messageIndex}`)
|
||||
|
||||
// Delete this message and all that follow
|
||||
await currentCline.overwriteClineMessages(currentCline.clineMessages.slice(0, messageIndex))
|
||||
|
||||
if (apiConversationHistoryIndex !== -1) {
|
||||
await currentCline.overwriteApiConversationHistory(
|
||||
currentCline.apiConversationHistory.slice(0, apiConversationHistoryIndex),
|
||||
)
|
||||
if (
|
||||
apiConversationHistoryIndex >= 0 &&
|
||||
apiConversationHistoryIndex < currentCline.apiConversationHistory.length
|
||||
) {
|
||||
await currentCline.overwriteApiConversationHistory(
|
||||
currentCline.apiConversationHistory.slice(0, apiConversationHistoryIndex),
|
||||
)
|
||||
} else {
|
||||
console.warn(`[Chat History] Invalid API conversation history index: ${apiConversationHistoryIndex}`)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -118,6 +139,16 @@ export const webviewMessageHandler = async (
|
|||
|
||||
if (messageIndex !== -1) {
|
||||
try {
|
||||
// Additional validation: ensure we're not deleting critical messages
|
||||
const messageToDelete = currentCline.clineMessages[messageIndex]
|
||||
if (!messageToDelete) {
|
||||
console.error(`[Chat History] Message not found at index ${messageIndex}`)
|
||||
throw new Error("Message not found for deletion")
|
||||
}
|
||||
|
||||
// Log the message being deleted for debugging
|
||||
console.log(`[Chat History] Deleting message with timestamp ${messageTs} at index ${messageIndex}`)
|
||||
|
||||
const { historyItem } = await provider.getTaskWithId(currentCline.taskId)
|
||||
|
||||
// Delete this message and all subsequent messages
|
||||
|
|
@ -126,11 +157,13 @@ export const webviewMessageHandler = async (
|
|||
// Initialize with history item after deletion
|
||||
await provider.createTaskWithHistoryItem(historyItem)
|
||||
} catch (error) {
|
||||
console.error("Error in delete message:", error)
|
||||
console.error("[Chat History] Error in delete message:", error)
|
||||
vscode.window.showErrorMessage(
|
||||
`Error deleting message: ${error instanceof Error ? error.message : String(error)}`,
|
||||
)
|
||||
}
|
||||
} else {
|
||||
console.warn(`[Chat History] Message with timestamp ${messageTs} not found for deletion`)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -165,6 +198,16 @@ export const webviewMessageHandler = async (
|
|||
|
||||
if (messageIndex !== -1) {
|
||||
try {
|
||||
// Additional validation: ensure we're not editing critical messages
|
||||
const messageToEdit = currentCline.clineMessages[messageIndex]
|
||||
if (!messageToEdit) {
|
||||
console.error(`[Chat History] Message not found at index ${messageIndex}`)
|
||||
throw new Error("Message not found for editing")
|
||||
}
|
||||
|
||||
// Log the message being edited for debugging
|
||||
console.log(`[Chat History] Editing message with timestamp ${messageTs} at index ${messageIndex}`)
|
||||
|
||||
// Edit this message and delete subsequent
|
||||
await removeMessagesThisAndSubsequent(currentCline, messageIndex, apiConversationHistoryIndex)
|
||||
|
||||
|
|
@ -180,11 +223,13 @@ export const webviewMessageHandler = async (
|
|||
// Don't initialize with history item for edit operations
|
||||
// The webviewMessageHandler will handle the conversation state
|
||||
} catch (error) {
|
||||
console.error("Error in edit message:", error)
|
||||
console.error("[Chat History] Error in edit message:", error)
|
||||
vscode.window.showErrorMessage(
|
||||
`Error editing message: ${error instanceof Error ? error.message : String(error)}`,
|
||||
)
|
||||
}
|
||||
} else {
|
||||
console.warn(`[Chat History] Message with timestamp ${messageTs} not found for editing`)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue