Skip to content

Commit d956cdb

Browse files
Fix duplicate rehydrate during reasoning; centralize rehydrate and preserve cancel metadata (RooCodeInc#8171)
Co-authored-by: daniel-lxs <[email protected]>
1 parent d6cb396 commit d956cdb

File tree

20 files changed

+153
-36
lines changed

20 files changed

+153
-36
lines changed

src/core/task/Task.ts

Lines changed: 21 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
212212

213213
didFinishAbortingStream = false
214214
abandoned = false
215+
abortReason?: ClineApiReqCancelReason
215216
isInitialized = false
216217
isPaused: boolean = false
217218
pausedModeSlug: string = defaultModeSlug
@@ -1264,6 +1265,16 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
12641265
modifiedClineMessages.splice(lastRelevantMessageIndex + 1)
12651266
}
12661267

1268+
// Remove any trailing reasoning-only UI messages that were not part of the persisted API conversation
1269+
while (modifiedClineMessages.length > 0) {
1270+
const last = modifiedClineMessages[modifiedClineMessages.length - 1]
1271+
if (last.type === "say" && last.say === "reasoning") {
1272+
modifiedClineMessages.pop()
1273+
} else {
1274+
break
1275+
}
1276+
}
1277+
12671278
// Since we don't use `api_req_finished` anymore, we need to check if the
12681279
// last `api_req_started` has a cost value, if it doesn't and no
12691280
// cancellation reason to present, then we remove it since it indicates
@@ -1884,28 +1895,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
18841895
lastMessage.partial = false
18851896
// instead of streaming partialMessage events, we do a save and post like normal to persist to disk
18861897
console.log("updating partial message", lastMessage)
1887-
// await this.saveClineMessages()
18881898
}
18891899

1890-
// Let assistant know their response was interrupted for when task is resumed
1891-
await this.addToApiConversationHistory({
1892-
role: "assistant",
1893-
content: [
1894-
{
1895-
type: "text",
1896-
text:
1897-
assistantMessage +
1898-
`\n\n[${
1899-
cancelReason === "streaming_failed"
1900-
? "Response interrupted by API Error"
1901-
: "Response interrupted by user"
1902-
}]`,
1903-
},
1904-
],
1905-
})
1906-
19071900
// Update `api_req_started` to have cancelled and cost, so that
1908-
// we can display the cost of the partial stream.
1901+
// we can display the cost of the partial stream and the cancellation reason
19091902
updateApiReqMsg(cancelReason, streamingFailedMessage)
19101903
await this.saveClineMessages()
19111904

@@ -2187,24 +2180,23 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
21872180
// may have executed), so we just resort to replicating a
21882181
// cancel task.
21892182

2190-
// Check if this was a user-initiated cancellation BEFORE calling abortTask
2191-
// If this.abort is already true, it means the user clicked cancel, so we should
2192-
// treat this as "user_cancelled" rather than "streaming_failed"
2193-
const cancelReason = this.abort ? "user_cancelled" : "streaming_failed"
2183+
// Determine cancellation reason BEFORE aborting to ensure correct persistence
2184+
const cancelReason: ClineApiReqCancelReason = this.abort ? "user_cancelled" : "streaming_failed"
21942185

21952186
const streamingFailedMessage = this.abort
21962187
? undefined
21972188
: (error.message ?? JSON.stringify(serializeError(error), null, 2))
21982189

2199-
// Now call abortTask after determining the cancel reason.
2200-
await this.abortTask()
2190+
// Persist interruption details first to both UI and API histories
22012191
await abortStream(cancelReason, streamingFailedMessage)
22022192

2203-
const history = await provider?.getTaskWithId(this.taskId)
2193+
// Record reason for provider to decide rehydration path
2194+
this.abortReason = cancelReason
22042195

2205-
if (history) {
2206-
await provider?.createTaskWithHistoryItem(history.historyItem)
2207-
}
2196+
// Now abort (emits TaskAborted which provider listens to)
2197+
await this.abortTask()
2198+
2199+
// Do not rehydrate here; provider owns rehydration to avoid duplication races
22082200
}
22092201
} finally {
22102202
this.isStreaming = false

src/core/webview/ClineProvider.ts

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ import { Task } from "../task/Task"
8989
import { getSystemPromptFilePath } from "../prompts/sections/custom-system-prompt"
9090

9191
import { webviewMessageHandler } from "./webviewMessageHandler"
92+
import type { ClineMessage } from "@roo-code/types"
93+
import { readApiMessages, saveApiMessages, saveTaskMessages } from "../task-persistence"
9294
import { getNonce } from "./getNonce"
9395
import { getUri } from "./getUri"
9496

@@ -196,7 +198,35 @@ export class ClineProvider
196198
const onTaskStarted = () => this.emit(RooCodeEventName.TaskStarted, instance.taskId)
197199
const onTaskCompleted = (taskId: string, tokenUsage: any, toolUsage: any) =>
198200
this.emit(RooCodeEventName.TaskCompleted, taskId, tokenUsage, toolUsage)
199-
const onTaskAborted = () => this.emit(RooCodeEventName.TaskAborted, instance.taskId)
201+
const onTaskAborted = async () => {
202+
this.emit(RooCodeEventName.TaskAborted, instance.taskId)
203+
204+
try {
205+
// Only rehydrate on genuine streaming failures.
206+
// User-initiated cancels are handled by cancelTask().
207+
if (instance.abortReason === "streaming_failed") {
208+
// Defensive safeguard: if another path already replaced this instance, skip
209+
const current = this.getCurrentTask()
210+
if (current && current.instanceId !== instance.instanceId) {
211+
this.log(
212+
`[onTaskAborted] Skipping rehydrate: current instance ${current.instanceId} != aborted ${instance.instanceId}`,
213+
)
214+
return
215+
}
216+
217+
const { historyItem } = await this.getTaskWithId(instance.taskId)
218+
const rootTask = instance.rootTask
219+
const parentTask = instance.parentTask
220+
await this.createTaskWithHistoryItem({ ...historyItem, rootTask, parentTask })
221+
}
222+
} catch (error) {
223+
this.log(
224+
`[onTaskAborted] Failed to rehydrate after streaming failure: ${
225+
error instanceof Error ? error.message : String(error)
226+
}`,
227+
)
228+
}
229+
}
200230
const onTaskFocused = () => this.emit(RooCodeEventName.TaskFocused, instance.taskId)
201231
const onTaskUnfocused = () => this.emit(RooCodeEventName.TaskUnfocused, instance.taskId)
202232
const onTaskActive = (taskId: string) => this.emit(RooCodeEventName.TaskActive, taskId)
@@ -2525,14 +2555,24 @@ export class ClineProvider
25252555

25262556
console.log(`[cancelTask] cancelling task ${task.taskId}.${task.instanceId}`)
25272557

2528-
const { historyItem } = await this.getTaskWithId(task.taskId)
2558+
const { historyItem, uiMessagesFilePath } = await this.getTaskWithId(task.taskId)
25292559

25302560
// Preserve parent and root task information for history item.
25312561
const rootTask = task.rootTask
25322562
const parentTask = task.parentTask
25332563

2564+
// Mark this as a user-initiated cancellation so provider-only rehydration can occur
2565+
task.abortReason = "user_cancelled"
2566+
2567+
// Capture the current instance to detect if rehydrate already occurred elsewhere
2568+
const originalInstanceId = task.instanceId
2569+
2570+
// Begin abort (non-blocking)
25342571
task.abortTask()
25352572

2573+
// Immediately mark the original instance as abandoned to prevent any residual activity
2574+
task.abandoned = true
2575+
25362576
await pWaitFor(
25372577
() =>
25382578
this.getCurrentTask()! === undefined ||
@@ -2549,11 +2589,24 @@ export class ClineProvider
25492589
console.error("Failed to abort task")
25502590
})
25512591

2552-
if (this.getCurrentTask()) {
2553-
// 'abandoned' will prevent this Cline instance from affecting
2554-
// future Cline instances. This may happen if its hanging on a
2555-
// streaming request.
2556-
this.getCurrentTask()!.abandoned = true
2592+
// Defensive safeguard: if current instance already changed, skip rehydrate
2593+
const current = this.getCurrentTask()
2594+
if (current && current.instanceId !== originalInstanceId) {
2595+
this.log(
2596+
`[cancelTask] Skipping rehydrate: current instance ${current.instanceId} != original ${originalInstanceId}`,
2597+
)
2598+
return
2599+
}
2600+
2601+
// Final race check before rehydrate to avoid duplicate rehydration
2602+
{
2603+
const currentAfterCheck = this.getCurrentTask()
2604+
if (currentAfterCheck && currentAfterCheck.instanceId !== originalInstanceId) {
2605+
this.log(
2606+
`[cancelTask] Skipping rehydrate after final check: current instance ${currentAfterCheck.instanceId} != original ${originalInstanceId}`,
2607+
)
2608+
return
2609+
}
25572610
}
25582611

25592612
// Clears task again, so we need to abortTask manually above.

src/i18n/locales/ca/common.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/i18n/locales/de/common.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/i18n/locales/en/common.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,10 @@
161161
"incomplete": "Task #{{taskNumber}} (Incomplete)",
162162
"no_messages": "Task #{{taskNumber}} (No messages)"
163163
},
164+
"interruption": {
165+
"responseInterruptedByUser": "Response interrupted by user",
166+
"responseInterruptedByApiError": "Response interrupted by API error"
167+
},
164168
"storage": {
165169
"prompt_custom_path": "Enter custom conversation history storage path, leave empty to use default location",
166170
"path_placeholder": "D:\\RooCodeStorage",

src/i18n/locales/es/common.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/i18n/locales/fr/common.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/i18n/locales/hi/common.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/i18n/locales/id/common.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/i18n/locales/it/common.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)