Skip to content

Commit 3bbf984

Browse files
committed
fix(webview/task): harden cancel/rehydrate flow and type-safety
- Mark original task instance abandoned instead of getCurrentTask() - Add parse guards and centralize UI/API cancel bookkeeping helpers - Replace any-cast with typed instance.abortReason - Centralize interruption strings and reuse in Task/Provider - Add final instanceId race-check before rehydrate - Remove unused variable in cancelTask()
1 parent bcf2241 commit 3bbf984

File tree

4 files changed

+116
-60
lines changed

4 files changed

+116
-60
lines changed
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import type { ClineMessage } from "@roo-code/types"
2+
import { readTaskMessages, saveTaskMessages } from "./taskMessages"
3+
import { readApiMessages, saveApiMessages } from "./apiMessages"
4+
import type { ClineApiReqCancelReason } from "../../shared/ExtensionMessage"
5+
6+
// Safely add cancelReason to the last api_req_started UI message
7+
export async function addCancelReasonToLastApiReqStarted(args: {
8+
taskId: string
9+
globalStoragePath: string
10+
reason: ClineApiReqCancelReason
11+
}): Promise<void> {
12+
const { taskId, globalStoragePath, reason } = args
13+
14+
try {
15+
const uiMsgs = (await readTaskMessages({ taskId, globalStoragePath })) as ClineMessage[]
16+
17+
if (!Array.isArray(uiMsgs) || uiMsgs.length === 0) {
18+
return
19+
}
20+
21+
// Find last api_req_started
22+
const revIdx = uiMsgs
23+
.slice()
24+
.reverse()
25+
.findIndex((m) => m?.type === "say" && (m as any)?.say === "api_req_started")
26+
27+
if (revIdx === -1) {
28+
return
29+
}
30+
31+
const idx = uiMsgs.length - 1 - revIdx
32+
33+
try {
34+
const existing = uiMsgs[idx]?.text ? JSON.parse(uiMsgs[idx].text as string) : {}
35+
uiMsgs[idx].text = JSON.stringify({ ...existing, cancelReason: reason })
36+
await saveTaskMessages({ messages: uiMsgs as any, taskId, globalStoragePath })
37+
} catch {
38+
// Non-fatal parse or write failure
39+
return
40+
}
41+
} catch {
42+
// Non-fatal read failure
43+
return
44+
}
45+
}
46+
47+
// Append an assistant interruption marker to API conversation history
48+
// only if the last message isn't already an assistant.
49+
export async function appendAssistantInterruptionIfNeeded(args: {
50+
taskId: string
51+
globalStoragePath: string
52+
text: string
53+
}): Promise<void> {
54+
const { taskId, globalStoragePath, text } = args
55+
56+
try {
57+
const apiMsgs = await readApiMessages({ taskId, globalStoragePath })
58+
const last = apiMsgs.at(-1)
59+
60+
if (!last || last.role !== "assistant") {
61+
apiMsgs.push({
62+
role: "assistant",
63+
content: [{ type: "text", text }],
64+
ts: Date.now(),
65+
} as any)
66+
67+
await saveApiMessages({ messages: apiMsgs as any, taskId, globalStoragePath })
68+
}
69+
} catch {
70+
// Non-fatal read/write failure
71+
return
72+
}
73+
}

src/core/task/Task.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ import { type AssistantMessageContent, presentAssistantMessage } from "../assist
8888
import { AssistantMessageParser } from "../assistant-message/AssistantMessageParser"
8989
import { truncateConversationIfNeeded } from "../sliding-window"
9090
import { ClineProvider } from "../webview/ClineProvider"
91+
import { RESPONSE_INTERRUPTED_BY_API_ERROR, RESPONSE_INTERRUPTED_BY_USER } from "../../shared/messages"
9192
import { MultiSearchReplaceDiffStrategy } from "../diff/strategies/multi-search-replace"
9293
import { MultiFileSearchReplaceDiffStrategy } from "../diff/strategies/multi-file-search-replace"
9394
import {
@@ -1908,8 +1909,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
19081909
assistantMessage +
19091910
`\n\n[${
19101911
cancelReason === "streaming_failed"
1911-
? "Response interrupted by API Error"
1912-
: "Response interrupted by user"
1912+
? RESPONSE_INTERRUPTED_BY_API_ERROR
1913+
: RESPONSE_INTERRUPTED_BY_USER
19131914
}]`,
19141915
},
19151916
],

src/core/webview/ClineProvider.ts

Lines changed: 31 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ import { readApiMessages, saveApiMessages, saveTaskMessages } from "../task-pers
9494
import { getNonce } from "./getNonce"
9595
import { getUri } from "./getUri"
9696

97+
import {
98+
addCancelReasonToLastApiReqStarted,
99+
appendAssistantInterruptionIfNeeded,
100+
} from "../task-persistence/cancelBookkeeping"
101+
import { RESPONSE_INTERRUPTED_BY_USER } from "../../shared/messages"
97102
/**
98103
* https://github.com/microsoft/vscode-webview-ui-toolkit-samples/blob/main/default/weather-webview/src/providers/WeatherViewProvider.ts
99104
* https://github.com/KumarVariable/vscode-extension-sidebar-html/blob/master/src/customSidebarViewProvider.ts
@@ -204,7 +209,7 @@ export class ClineProvider
204209
try {
205210
// Only rehydrate on genuine streaming failures.
206211
// User-initiated cancels are handled by cancelTask().
207-
if ((instance as any).abortReason === "streaming_failed") {
212+
if (instance.abortReason === "streaming_failed") {
208213
// Defensive safeguard: if another path already replaced this instance, skip
209214
const current = this.getCurrentTask()
210215
if (current && current.instanceId !== instance.instanceId) {
@@ -2555,9 +2560,7 @@ export class ClineProvider
25552560

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

2558-
const { historyItem, uiMessagesFilePath, apiConversationHistoryFilePath } = await this.getTaskWithId(
2559-
task.taskId,
2560-
)
2563+
const { historyItem, uiMessagesFilePath } = await this.getTaskWithId(task.taskId)
25612564

25622565
// Preserve parent and root task information for history item.
25632566
const rootTask = task.rootTask
@@ -2572,10 +2575,8 @@ export class ClineProvider
25722575
// Begin abort (non-blocking)
25732576
task.abortTask()
25742577

2575-
// Immediately mark the current instance as abandoned to prevent any residual activity
2576-
if (this.getCurrentTask()) {
2577-
this.getCurrentTask()!.abandoned = true
2578-
}
2578+
// Immediately mark the original instance as abandoned to prevent any residual activity
2579+
task.abandoned = true
25792580

25802581
await pWaitFor(
25812582
() =>
@@ -2604,60 +2605,32 @@ export class ClineProvider
26042605

26052606
// Provider-side cancel bookkeeping to mirror abortStream effects for user_cancelled
26062607
try {
2607-
// Update ui_messages: add cancelReason to last api_req_started
2608-
const messagesJson = await fs.readFile(uiMessagesFilePath, "utf8").catch(() => undefined)
2609-
if (messagesJson) {
2610-
const uiMsgs = JSON.parse(messagesJson) as ClineMessage[]
2611-
if (Array.isArray(uiMsgs)) {
2612-
const revIdx = uiMsgs
2613-
.slice()
2614-
.reverse()
2615-
.findIndex((m) => m?.type === "say" && (m as any)?.say === "api_req_started")
2616-
if (revIdx !== -1) {
2617-
const idx = uiMsgs.length - 1 - revIdx
2618-
try {
2619-
const existing = uiMsgs[idx]?.text ? JSON.parse(uiMsgs[idx].text as string) : {}
2620-
uiMsgs[idx].text = JSON.stringify({ ...existing, cancelReason: "user_cancelled" })
2621-
await saveTaskMessages({
2622-
messages: uiMsgs as any,
2623-
taskId: task.taskId,
2624-
globalStoragePath: this.contextProxy.globalStorageUri.fsPath,
2625-
})
2626-
} catch {
2627-
// non-fatal
2628-
}
2629-
}
2630-
}
2631-
}
2608+
// Persist cancelReason to last api_req_started in UI messages
2609+
await addCancelReasonToLastApiReqStarted({
2610+
taskId: task.taskId,
2611+
globalStoragePath: this.contextProxy.globalStorageUri.fsPath,
2612+
reason: "user_cancelled",
2613+
})
26322614

2633-
// Update api_conversation_history: append assistant interruption if last isn't assistant
2634-
try {
2635-
const apiMsgs = await readApiMessages({
2636-
taskId: task.taskId,
2637-
globalStoragePath: this.contextProxy.globalStorageUri.fsPath,
2638-
})
2639-
const last = apiMsgs.at(-1)
2640-
if (!last || last.role !== "assistant") {
2641-
apiMsgs.push({
2642-
role: "assistant",
2643-
content: [{ type: "text", text: "[Response interrupted by user]" }],
2644-
ts: Date.now(),
2645-
} as any)
2646-
await saveApiMessages({
2647-
messages: apiMsgs as any,
2648-
taskId: task.taskId,
2649-
globalStoragePath: this.contextProxy.globalStorageUri.fsPath,
2650-
})
2651-
}
2652-
} catch (e) {
2615+
// Append assistant interruption marker to API conversation history if needed
2616+
await appendAssistantInterruptionIfNeeded({
2617+
taskId: task.taskId,
2618+
globalStoragePath: this.contextProxy.globalStorageUri.fsPath,
2619+
text: `[${RESPONSE_INTERRUPTED_BY_USER}]`,
2620+
})
2621+
} catch (e) {
2622+
this.log(`[cancelTask] Cancel bookkeeping failed: ${e instanceof Error ? e.message : String(e)}`)
2623+
}
2624+
2625+
// Final race check before rehydrate to avoid duplicate rehydration
2626+
{
2627+
const currentAfterBookkeeping = this.getCurrentTask()
2628+
if (currentAfterBookkeeping && currentAfterBookkeeping.instanceId !== originalInstanceId) {
26532629
this.log(
2654-
`[cancelTask] Failed to update API history for user_cancelled: ${
2655-
e instanceof Error ? e.message : String(e)
2656-
}`,
2630+
`[cancelTask] Skipping rehydrate after bookkeeping: current instance ${currentAfterBookkeeping.instanceId} != original ${originalInstanceId}`,
26572631
)
2632+
return
26582633
}
2659-
} catch (e) {
2660-
this.log(`[cancelTask] Cancel bookkeeping failed: ${e instanceof Error ? e.message : String(e)}`)
26612634
}
26622635

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

src/shared/messages.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/**
2+
* Centralized user-facing message constants for interruption labels.
3+
* TODO: Consider moving these to i18n JSON in src/i18n/locales/* and wiring through t()
4+
* so they can be localized consistently across the UI.
5+
*
6+
* Note: These are plain phrases (no surrounding brackets). Call sites add any desired decoration.
7+
*/
8+
export const RESPONSE_INTERRUPTED_BY_USER = "Response interrupted by user"
9+
export const RESPONSE_INTERRUPTED_BY_API_ERROR = "Response interrupted by API Error"

0 commit comments

Comments
 (0)