mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix: resolve timestamp collision in condense + rewind scenario
- Make summary timestamp unique by using keepMessages[0].ts - 1 to avoid collision - Change checkpoint restore filter from < to <= to include target message - Add regression test for condense + rewind scenario Fixes #8295
This commit is contained in:
parent
d3d0967fc3
commit
2961380e58
5 changed files with 180 additions and 2 deletions
1
.review/pr-8274
Submodule
1
.review/pr-8274
Submodule
|
|
@ -0,0 +1 @@
|
|||
Subproject commit e46929b8d8add0cd3c412d69f8ac882c405a4ba9
|
||||
|
|
@ -226,7 +226,8 @@ export async function checkpointRestore(
|
|||
await provider?.postMessageToWebview({ type: "currentCheckpointUpdated", text: commitHash })
|
||||
|
||||
if (mode === "restore") {
|
||||
await task.overwriteApiConversationHistory(task.apiConversationHistory.filter((m) => !m.ts || m.ts < ts))
|
||||
// Include the target turn during checkpoint restore (use <= instead of <)
|
||||
await task.overwriteApiConversationHistory(task.apiConversationHistory.filter((m) => !m.ts || m.ts <= ts))
|
||||
|
||||
const deletedMessages = task.clineMessages.slice(index + 1)
|
||||
|
||||
|
|
|
|||
|
|
@ -181,10 +181,16 @@ export async function summarizeConversation(
|
|||
return { ...response, cost, error }
|
||||
}
|
||||
|
||||
// Make the summary timestamp unique to avoid collision with the first kept message
|
||||
// Use keepMessages[0].ts - 1, but ensure we don't go before the first message
|
||||
const firstMessageTs = firstMessage.ts || Date.now()
|
||||
const firstKeptTs = keepMessages[0].ts || Date.now()
|
||||
const summaryTs = Math.max(firstMessageTs + 1, firstKeptTs - 1)
|
||||
|
||||
const summaryMessage: ApiMessage = {
|
||||
role: "assistant",
|
||||
content: summary,
|
||||
ts: keepMessages[0].ts,
|
||||
ts: summaryTs,
|
||||
isSummary: true,
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1616,6 +1616,175 @@ describe("Cline", () => {
|
|||
})
|
||||
|
||||
describe("Conversation continuity after condense and deletion", () => {
|
||||
it("should preserve messages up to rewind target after manual condense", async () => {
|
||||
// This test reproduces the bug scenario: messages 1-10 → manual condense → 11 → rewind to 8 → send 12
|
||||
// Expected: Keep messages through 8, drop only 9-11
|
||||
// Bug behavior: Only initial message and 12+ remain
|
||||
|
||||
// Arrange: create task
|
||||
const task = new Task({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: mockApiConfig,
|
||||
task: "initial task",
|
||||
startTask: false,
|
||||
})
|
||||
|
||||
// Simulate messages 1-10
|
||||
const messages = []
|
||||
const apiHistory = []
|
||||
const baseTime = Date.now() - 10000
|
||||
|
||||
for (let i = 1; i <= 10; i++) {
|
||||
const ts = baseTime + i * 100
|
||||
messages.push({
|
||||
ts,
|
||||
type: "say" as const,
|
||||
say: "user_feedback" as const,
|
||||
text: `tell me a joke ${i}`,
|
||||
})
|
||||
apiHistory.push({
|
||||
role: "user" as const,
|
||||
content: [{ type: "text" as const, text: `tell me a joke ${i}` }],
|
||||
ts,
|
||||
})
|
||||
|
||||
messages.push({
|
||||
ts: ts + 50,
|
||||
type: "say" as const,
|
||||
say: "text" as const,
|
||||
text: `Here's joke ${i}...`,
|
||||
})
|
||||
apiHistory.push({
|
||||
role: "assistant" as const,
|
||||
content: [{ type: "text" as const, text: `Here's joke ${i}...` }],
|
||||
ts: ts + 50,
|
||||
})
|
||||
}
|
||||
|
||||
await task.overwriteClineMessages(messages)
|
||||
await task.overwriteApiConversationHistory(apiHistory)
|
||||
|
||||
// Simulate manual condense - this creates a summary with timestamp collision
|
||||
const keepMessages = messages.slice(-6) // Keep last 3 user-assistant pairs
|
||||
const keepApiHistory = apiHistory.slice(-6)
|
||||
|
||||
// Bug: summary gets the same timestamp as first kept message
|
||||
const summaryTs = keepMessages[0].ts // This causes the collision
|
||||
|
||||
const condensedMessages = [
|
||||
messages[0], // First message
|
||||
{
|
||||
ts: summaryTs,
|
||||
type: "say" as const,
|
||||
say: "text" as const,
|
||||
text: "Summary of jokes 1-7...",
|
||||
isSummary: true,
|
||||
},
|
||||
...keepMessages,
|
||||
]
|
||||
|
||||
const condensedApiHistory = [
|
||||
apiHistory[0], // First message
|
||||
{
|
||||
role: "assistant" as const,
|
||||
content: [{ type: "text" as const, text: "Summary of jokes 1-7..." }],
|
||||
ts: summaryTs,
|
||||
isSummary: true,
|
||||
},
|
||||
...keepApiHistory,
|
||||
]
|
||||
|
||||
await task.overwriteClineMessages(condensedMessages)
|
||||
await task.overwriteApiConversationHistory(condensedApiHistory)
|
||||
|
||||
// Add message 11
|
||||
const msg11Ts = baseTime + 1100
|
||||
await task.overwriteClineMessages([
|
||||
...condensedMessages,
|
||||
{
|
||||
ts: msg11Ts,
|
||||
type: "say" as const,
|
||||
say: "user_feedback" as const,
|
||||
text: "tell me a joke 11",
|
||||
},
|
||||
{
|
||||
ts: msg11Ts + 50,
|
||||
type: "say" as const,
|
||||
say: "text" as const,
|
||||
text: "Here's joke 11...",
|
||||
},
|
||||
])
|
||||
|
||||
await task.overwriteApiConversationHistory([
|
||||
...condensedApiHistory,
|
||||
{
|
||||
role: "user" as const,
|
||||
content: [{ type: "text" as const, text: "tell me a joke 11" }],
|
||||
ts: msg11Ts,
|
||||
},
|
||||
{
|
||||
role: "assistant" as const,
|
||||
content: [{ type: "text" as const, text: "Here's joke 11..." }],
|
||||
ts: msg11Ts + 50,
|
||||
},
|
||||
])
|
||||
|
||||
// Simulate rewind to message 8 (delete this and after)
|
||||
const msg8Ts = baseTime + 800 // Timestamp of message 8
|
||||
|
||||
// Find the message index for message 8
|
||||
const messageIndex = task.clineMessages.findIndex(
|
||||
(m) => m.say === "user_feedback" && m.text === "tell me a joke 8",
|
||||
)
|
||||
|
||||
// With the fix, checkpoint restore should use <= instead of <
|
||||
// This ensures message 8 is kept
|
||||
const filteredApiHistory = task.apiConversationHistory.filter((m) => !m.ts || m.ts <= msg8Ts)
|
||||
|
||||
// Verify the fix: message 8 should be included
|
||||
const hasMessage8 = filteredApiHistory.some((m) => {
|
||||
if (
|
||||
Array.isArray(m.content) &&
|
||||
m.content[0] &&
|
||||
typeof m.content[0] === "object" &&
|
||||
"text" in m.content[0]
|
||||
) {
|
||||
return m.content[0].text === "tell me a joke 8"
|
||||
}
|
||||
return false
|
||||
})
|
||||
|
||||
expect(hasMessage8).toBe(true)
|
||||
expect(filteredApiHistory.length).toBeGreaterThan(2) // Should have more than just initial + summary
|
||||
|
||||
// Verify that messages 9-11 are excluded
|
||||
const hasMessage9 = filteredApiHistory.some((m) => {
|
||||
if (
|
||||
Array.isArray(m.content) &&
|
||||
m.content[0] &&
|
||||
typeof m.content[0] === "object" &&
|
||||
"text" in m.content[0]
|
||||
) {
|
||||
return m.content[0].text.includes("joke 9")
|
||||
}
|
||||
return false
|
||||
})
|
||||
const hasMessage11 = filteredApiHistory.some((m) => {
|
||||
if (
|
||||
Array.isArray(m.content) &&
|
||||
m.content[0] &&
|
||||
typeof m.content[0] === "object" &&
|
||||
"text" in m.content[0]
|
||||
) {
|
||||
return m.content[0].text.includes("joke 11")
|
||||
}
|
||||
return false
|
||||
})
|
||||
|
||||
expect(hasMessage9).toBe(false)
|
||||
expect(hasMessage11).toBe(false)
|
||||
})
|
||||
|
||||
it("should set suppressPreviousResponseId when last message is condense_context", async () => {
|
||||
// Arrange: create task
|
||||
const task = new Task({
|
||||
|
|
|
|||
1
tmp/pr-8287-Roo-Code
Submodule
1
tmp/pr-8287-Roo-Code
Submodule
|
|
@ -0,0 +1 @@
|
|||
Subproject commit 88a473b017af37091c85ce3056e444e856f80d6e
|
||||
Loading…
Add table
Reference in a new issue