Skip to content
Merged
30 changes: 20 additions & 10 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {

didFinishAbortingStream = false
abandoned = false
abortReason?: ClineApiReqCancelReason
isInitialized = false
isPaused: boolean = false
pausedModeSlug: string = defaultModeSlug
Expand Down Expand Up @@ -1264,6 +1265,16 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
modifiedClineMessages.splice(lastRelevantMessageIndex + 1)
}

// Remove any trailing reasoning-only UI messages that were not part of the persisted API conversation
while (modifiedClineMessages.length > 0) {
const last = modifiedClineMessages[modifiedClineMessages.length - 1]
if (last.type === "say" && last.say === "reasoning") {
modifiedClineMessages.pop()
} else {
break
}
}

// Since we don't use `api_req_finished` anymore, we need to check if the
// last `api_req_started` has a cost value, if it doesn't and no
// cancellation reason to present, then we remove it since it indicates
Expand Down Expand Up @@ -2187,24 +2198,23 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// may have executed), so we just resort to replicating a
// cancel task.

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

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

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

const history = await provider?.getTaskWithId(this.taskId)
// Record reason for provider to decide rehydration path
this.abortReason = cancelReason

if (history) {
await provider?.createTaskWithHistoryItem(history.historyItem)
}
// Now abort (emits TaskAborted which provider listens to)
await this.abortTask()

// Do not rehydrate here; provider owns rehydration to avoid duplication races
}
} finally {
this.isStreaming = false
Expand Down
118 changes: 111 additions & 7 deletions src/core/webview/ClineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ import { Task } from "../task/Task"
import { getSystemPromptFilePath } from "../prompts/sections/custom-system-prompt"

import { webviewMessageHandler } from "./webviewMessageHandler"
import type { ClineMessage } from "@roo-code/types"
import { readApiMessages, saveApiMessages, saveTaskMessages } from "../task-persistence"
import { getNonce } from "./getNonce"
import { getUri } from "./getUri"

Expand Down Expand Up @@ -196,7 +198,35 @@ export class ClineProvider
const onTaskStarted = () => this.emit(RooCodeEventName.TaskStarted, instance.taskId)
const onTaskCompleted = (taskId: string, tokenUsage: any, toolUsage: any) =>
this.emit(RooCodeEventName.TaskCompleted, taskId, tokenUsage, toolUsage)
const onTaskAborted = () => this.emit(RooCodeEventName.TaskAborted, instance.taskId)
const onTaskAborted = async () => {
this.emit(RooCodeEventName.TaskAborted, instance.taskId)

try {
// Only rehydrate on genuine streaming failures.
// User-initiated cancels are handled by cancelTask().
if ((instance as any).abortReason === "streaming_failed") {
Copy link

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using type assertion (instance as any) bypasses type safety. Consider adding abortReason to the Task interface or creating a proper type guard to check for this property safely.

Copilot uses AI. Check for mistakes.
// Defensive safeguard: if another path already replaced this instance, skip
const current = this.getCurrentTask()
if (current && current.instanceId !== instance.instanceId) {
this.log(
`[onTaskAborted] Skipping rehydrate: current instance ${current.instanceId} != aborted ${instance.instanceId}`,
)
return
}

const { historyItem } = await this.getTaskWithId(instance.taskId)
const rootTask = instance.rootTask
const parentTask = instance.parentTask
await this.createTaskWithHistoryItem({ ...historyItem, rootTask, parentTask })
}
} catch (error) {
this.log(
`[onTaskAborted] Failed to rehydrate after streaming failure: ${
error instanceof Error ? error.message : String(error)
}`,
)
}
}
const onTaskFocused = () => this.emit(RooCodeEventName.TaskFocused, instance.taskId)
const onTaskUnfocused = () => this.emit(RooCodeEventName.TaskUnfocused, instance.taskId)
const onTaskActive = (taskId: string) => this.emit(RooCodeEventName.TaskActive, taskId)
Expand Down Expand Up @@ -2525,14 +2555,28 @@ export class ClineProvider

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

const { historyItem } = await this.getTaskWithId(task.taskId)
const { historyItem, uiMessagesFilePath, apiConversationHistoryFilePath } = await this.getTaskWithId(
task.taskId,
)

// Preserve parent and root task information for history item.
const rootTask = task.rootTask
const parentTask = task.parentTask

// Mark this as a user-initiated cancellation so provider-only rehydration can occur
task.abortReason = "user_cancelled"
Copy link

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Direct property assignment to abortReason suggests this property may not be part of the Task interface. Consider adding this property to the Task interface or using a setter method to ensure type safety.

Copilot uses AI. Check for mistakes.

// Capture the current instance to detect if rehydrate already occurred elsewhere
const originalInstanceId = task.instanceId

// Begin abort (non-blocking)
task.abortTask()

// Immediately mark the current instance as abandoned to prevent any residual activity
if (this.getCurrentTask()) {
this.getCurrentTask()!.abandoned = true
}

await pWaitFor(
() =>
this.getCurrentTask()! === undefined ||
Expand All @@ -2549,11 +2593,71 @@ export class ClineProvider
console.error("Failed to abort task")
})

if (this.getCurrentTask()) {
// 'abandoned' will prevent this Cline instance from affecting
// future Cline instances. This may happen if its hanging on a
// streaming request.
this.getCurrentTask()!.abandoned = true
// Defensive safeguard: if current instance already changed, skip rehydrate
const current = this.getCurrentTask()
if (current && current.instanceId !== originalInstanceId) {
this.log(
`[cancelTask] Skipping cancel bookkeeping and rehydrate: current instance ${current.instanceId} != original ${originalInstanceId}`,
)
return
}

// Provider-side cancel bookkeeping to mirror abortStream effects for user_cancelled
try {
// Update ui_messages: add cancelReason to last api_req_started
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor the cancelTask method to extract UI and API history updating logic into smaller helper functions for improved readability and maintainability.

const messagesJson = await fs.readFile(uiMessagesFilePath, "utf8").catch(() => undefined)
if (messagesJson) {
const uiMsgs = JSON.parse(messagesJson) as ClineMessage[]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrap the JSON.parse call on uiMessagesFilePath content in a try/catch to handle potential parsing errors gracefully.

This comment was generated because it violated a code review rule: irule_PTI8rjtnhwrWq6jS.

if (Array.isArray(uiMsgs)) {
const revIdx = uiMsgs
.slice()
.reverse()
.findIndex((m) => m?.type === "say" && (m as any)?.say === "api_req_started")
if (revIdx !== -1) {
const idx = uiMsgs.length - 1 - revIdx
try {
const existing = uiMsgs[idx]?.text ? JSON.parse(uiMsgs[idx].text as string) : {}
uiMsgs[idx].text = JSON.stringify({ ...existing, cancelReason: "user_cancelled" })
await saveTaskMessages({
messages: uiMsgs as any,
taskId: task.taskId,
globalStoragePath: this.contextProxy.globalStorageUri.fsPath,
})
} catch {
// non-fatal
}
}
}
}

// Update api_conversation_history: append assistant interruption if last isn't assistant
try {
const apiMsgs = await readApiMessages({
taskId: task.taskId,
globalStoragePath: this.contextProxy.globalStorageUri.fsPath,
})
const last = apiMsgs.at(-1)
if (!last || last.role !== "assistant") {
apiMsgs.push({
role: "assistant",
content: [{ type: "text", text: "[Response interrupted by user]" }],
ts: Date.now(),
} as any)
await saveApiMessages({
messages: apiMsgs as any,
taskId: task.taskId,
globalStoragePath: this.contextProxy.globalStorageUri.fsPath,
})
}
} catch (e) {
this.log(
`[cancelTask] Failed to update API history for user_cancelled: ${
e instanceof Error ? e.message : String(e)
}`,
)
}
} catch (e) {
this.log(`[cancelTask] Cancel bookkeeping failed: ${e instanceof Error ? e.message : String(e)}`)
}

// Clears task again, so we need to abortTask manually above.
Expand Down
Loading