Skip to content

Commit 86debee

Browse files
committed
fix: preserve message history after condense and rewind operations
- Fixed bug where rewinding after condensing would lose intervening message history - Updated handleDeleteMessageConfirm to check for condense_context in preserved messages - Prevents incorrect API history truncation when condensed context exists - Added comprehensive test coverage for condense-rewind scenarios Fixes #8295
1 parent d3d0967 commit 86debee

File tree

3 files changed

+302
-2
lines changed

3 files changed

+302
-2
lines changed

.tmp/review/Roo-Code

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Subproject commit 8dbd8c4b1b72fb48be3990a8e78285a787a1828c
Lines changed: 288 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,288 @@
1+
import { describe, it, expect, vi, beforeEach } from "vitest"
2+
import { webviewMessageHandler } from "../webviewMessageHandler"
3+
import { ClineProvider } from "../ClineProvider"
4+
import { Task } from "../../task/Task"
5+
import { ClineMessage } from "@roo-code/types"
6+
7+
// Mock vscode module
8+
vi.mock("vscode", () => ({
9+
window: {
10+
showErrorMessage: vi.fn(),
11+
showWarningMessage: vi.fn(),
12+
},
13+
workspace: {
14+
workspaceFolders: [],
15+
getConfiguration: vi.fn(() => ({
16+
get: vi.fn(),
17+
})),
18+
},
19+
}))
20+
21+
describe("webviewMessageHandler - Condense and Rewind", () => {
22+
let mockProvider: any
23+
let mockTask: any
24+
25+
beforeEach(() => {
26+
// Create mock task
27+
mockTask = {
28+
taskId: "test-task-id",
29+
clineMessages: [] as ClineMessage[],
30+
apiConversationHistory: [],
31+
overwriteClineMessages: vi.fn(),
32+
overwriteApiConversationHistory: vi.fn(),
33+
saveClineMessages: vi.fn(),
34+
skipPrevResponseIdOnce: false,
35+
}
36+
37+
// Create mock provider
38+
mockProvider = {
39+
getCurrentTask: vi.fn().mockReturnValue(mockTask),
40+
postMessageToWebview: vi.fn(),
41+
postStateToWebview: vi.fn(),
42+
contextProxy: {
43+
globalStorageUri: { fsPath: "/test/storage" },
44+
},
45+
log: vi.fn(),
46+
}
47+
})
48+
49+
describe("Bug #8295: Rewind after manual condense keeps only initial + new message", () => {
50+
it("should preserve messages up to rewind point after condense", async () => {
51+
// Setup: Create messages 1-11 with a condense_context message after message 10
52+
const messages: ClineMessage[] = []
53+
54+
// Add initial message
55+
messages.push({
56+
ts: 1000,
57+
type: "say",
58+
say: "text",
59+
text: "Initial message",
60+
})
61+
62+
// Add messages 1-10 (jokes)
63+
for (let i = 1; i <= 10; i++) {
64+
messages.push({
65+
ts: 1000 + i * 100,
66+
type: "say",
67+
say: "user_feedback",
68+
text: `tell me a joke ${i}`,
69+
})
70+
messages.push({
71+
ts: 1000 + i * 100 + 50,
72+
type: "say",
73+
say: "text",
74+
text: `Here's joke ${i}...`,
75+
})
76+
}
77+
78+
// Add condense_context message after message 10
79+
messages.push({
80+
ts: 3000,
81+
type: "say",
82+
say: "condense_context",
83+
contextCondense: {
84+
summary: "Conversation condensed",
85+
cost: 0.001,
86+
prevContextTokens: 5000,
87+
newContextTokens: 1000,
88+
},
89+
})
90+
91+
// Add message 11
92+
messages.push({
93+
ts: 3100,
94+
type: "say",
95+
say: "user_feedback",
96+
text: "tell me a joke 11",
97+
})
98+
messages.push({
99+
ts: 3150,
100+
type: "say",
101+
say: "text",
102+
text: "Here's joke 11...",
103+
})
104+
105+
mockTask.clineMessages = [...messages]
106+
107+
// Find the timestamp of message 8 (the user feedback for joke 8)
108+
const message8Index = messages.findIndex(
109+
(m) => m.type === "say" && m.say === "user_feedback" && m.text === "tell me a joke 8",
110+
)
111+
const message8Ts = messages[message8Index].ts
112+
113+
// Act: Delete message 8 and all subsequent messages
114+
await webviewMessageHandler(mockProvider, {
115+
type: "deleteMessageConfirm",
116+
messageTs: message8Ts,
117+
})
118+
119+
// Assert: Check that messages up to joke 7 are preserved
120+
expect(mockTask.overwriteClineMessages).toHaveBeenCalled()
121+
const preservedMessages = mockTask.overwriteClineMessages.mock.calls[0][0]
122+
123+
// Should have initial message + jokes 1-7 (each joke is 2 messages: user + assistant)
124+
// That's 1 + (7 * 2) = 15 messages
125+
expect(preservedMessages.length).toBe(15)
126+
127+
// Verify the last preserved message is the assistant response to joke 7
128+
const lastPreserved = preservedMessages[preservedMessages.length - 1]
129+
expect(lastPreserved.text).toBe("Here's joke 7...")
130+
131+
// Verify skipPrevResponseIdOnce is NOT set (since last message is not condense_context)
132+
expect(mockTask.skipPrevResponseIdOnce).toBe(false)
133+
})
134+
135+
it("should set skipPrevResponseIdOnce when deletion leaves condense_context as last message", async () => {
136+
// Setup: Create a simpler scenario with condense_context as the last message after deletion
137+
const messages: ClineMessage[] = [
138+
{
139+
ts: 1000,
140+
type: "say",
141+
say: "text",
142+
text: "Initial message",
143+
},
144+
{
145+
ts: 1100,
146+
type: "say",
147+
say: "user_feedback",
148+
text: "First request",
149+
},
150+
{
151+
ts: 1200,
152+
type: "say",
153+
say: "text",
154+
text: "First response",
155+
},
156+
{
157+
ts: 1300,
158+
type: "say",
159+
say: "condense_context",
160+
contextCondense: {
161+
summary: "Condensed",
162+
cost: 0.001,
163+
prevContextTokens: 1000,
164+
newContextTokens: 200,
165+
},
166+
},
167+
{
168+
ts: 1400,
169+
type: "say",
170+
say: "user_feedback",
171+
text: "Second request",
172+
},
173+
{
174+
ts: 1500,
175+
type: "say",
176+
say: "text",
177+
text: "Second response",
178+
},
179+
]
180+
181+
mockTask.clineMessages = [...messages]
182+
183+
// Act: Delete the message after condense_context
184+
await webviewMessageHandler(mockProvider, {
185+
type: "deleteMessageConfirm",
186+
messageTs: 1400, // Delete "Second request" and everything after
187+
})
188+
189+
// Assert: Verify the preserved messages
190+
expect(mockTask.overwriteClineMessages).toHaveBeenCalled()
191+
const preservedMessages = mockTask.overwriteClineMessages.mock.calls[0][0]
192+
193+
// Should preserve up to and including the condense_context message
194+
expect(preservedMessages.length).toBe(4)
195+
expect(preservedMessages[preservedMessages.length - 1].say).toBe("condense_context")
196+
})
197+
198+
it("should handle rewind to specific message correctly after condense", async () => {
199+
// This test specifically reproduces the bug scenario from issue #8295
200+
// Setup: Messages 1-11 with condense after 10, then rewind to 8
201+
const messages: ClineMessage[] = []
202+
203+
// Initial message
204+
messages.push({
205+
ts: 1000,
206+
type: "say",
207+
say: "text",
208+
text: "Initial task",
209+
})
210+
211+
// Messages 1-10
212+
for (let i = 1; i <= 10; i++) {
213+
messages.push({
214+
ts: 1000 + i * 200,
215+
type: "say",
216+
say: "user_feedback",
217+
text: `tell me a joke ${i}`,
218+
})
219+
messages.push({
220+
ts: 1000 + i * 200 + 100,
221+
type: "say",
222+
say: "text",
223+
text: `Joke ${i}: Why did the ${i} cross the road?`,
224+
})
225+
}
226+
227+
// Condense after message 10
228+
messages.push({
229+
ts: 4000,
230+
type: "say",
231+
say: "condense_context",
232+
contextCondense: {
233+
summary: "User asked for 10 jokes, all delivered successfully",
234+
cost: 0.002,
235+
prevContextTokens: 3000,
236+
newContextTokens: 500,
237+
},
238+
})
239+
240+
// Message 11
241+
messages.push({
242+
ts: 4100,
243+
type: "say",
244+
say: "user_feedback",
245+
text: "tell me a joke 11",
246+
})
247+
messages.push({
248+
ts: 4200,
249+
type: "say",
250+
say: "text",
251+
text: "Joke 11: Why did the 11 cross the road?",
252+
})
253+
254+
mockTask.clineMessages = [...messages]
255+
256+
// Find message 8's timestamp
257+
const joke8UserIndex = messages.findIndex(
258+
(m) => m.type === "say" && m.say === "user_feedback" && m.text === "tell me a joke 8",
259+
)
260+
const joke8UserTs = messages[joke8UserIndex].ts
261+
262+
// Act: Rewind to message 8 (delete this and after)
263+
await webviewMessageHandler(mockProvider, {
264+
type: "deleteMessageConfirm",
265+
messageTs: joke8UserTs,
266+
})
267+
268+
// Assert: Messages 1-7 should be preserved
269+
expect(mockTask.overwriteClineMessages).toHaveBeenCalled()
270+
const preservedMessages = mockTask.overwriteClineMessages.mock.calls[0][0]
271+
272+
// Expected: initial + 7 jokes (each joke = 2 messages) = 1 + 14 = 15 messages
273+
expect(preservedMessages.length).toBe(15)
274+
275+
// Verify we have jokes 1-7 but not 8-11
276+
const joke7Present = preservedMessages.some((m: ClineMessage) => m.text?.includes("Joke 7:"))
277+
const joke8Present = preservedMessages.some((m: ClineMessage) => m.text?.includes("Joke 8:"))
278+
const condensePresent = preservedMessages.some((m: ClineMessage) => m.say === "condense_context")
279+
280+
expect(joke7Present).toBe(true)
281+
expect(joke8Present).toBe(false)
282+
expect(condensePresent).toBe(false)
283+
284+
// The last message should be the response to joke 7
285+
expect(preservedMessages[preservedMessages.length - 1].text).toContain("Joke 7:")
286+
})
287+
})
288+
})

src/core/webview/webviewMessageHandler.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,20 @@ export const webviewMessageHandler = async (
156156
}
157157

158158
const { messageIndex, apiConversationHistoryIndex } = findMessageIndices(messageTs, currentCline)
159-
// Determine API truncation index with timestamp fallback if exact match not found
159+
160+
// Check if there's a condense_context message in the preserved messages
161+
const hasCondenseInPreservedMessages =
162+
messageIndex > 0 &&
163+
currentCline.clineMessages.slice(0, messageIndex).some((msg) => msg.say === "condense_context")
164+
165+
// Determine API truncation index with special handling for post-condense scenarios
160166
let apiIndexToUse = apiConversationHistoryIndex
161167
const tsThreshold = currentCline.clineMessages[messageIndex]?.ts
162-
if (apiIndexToUse === -1 && typeof tsThreshold === "number") {
168+
169+
// After condensing, the API conversation history is already condensed
170+
// We should not use findFirstApiIndexAtOrAfter when there's a condense in preserved messages
171+
// as it would incorrectly truncate the condensed context
172+
if (apiIndexToUse === -1 && typeof tsThreshold === "number" && !hasCondenseInPreservedMessages) {
163173
apiIndexToUse = findFirstApiIndexAtOrAfter(tsThreshold, currentCline)
164174
}
165175

@@ -206,6 +216,7 @@ export const webviewMessageHandler = async (
206216
}
207217

208218
// Delete this message and all subsequent messages
219+
// The overwriteClineMessages method in Task will handle setting skipPrevResponseIdOnce if needed
209220
await removeMessagesThisAndSubsequent(currentCline, messageIndex, apiIndexToUse)
210221

211222
// Restore checkpoint associations for preserved messages

0 commit comments

Comments
 (0)