Skip to content

Commit 5ab3c4d

Browse files
committed
fix: prevent subtask restart on cancellation
- Added wasSubtaskCancelled flag to Task class to track cancelled subtasks - Modified completeSubtask to accept wasCancelled parameter - Updated cancelTask to detect subtasks and handle them differently - Subtasks now notify parent of cancellation instead of triggering rehydration - Added comprehensive tests for subtask cancellation behavior Fixes #8504
1 parent 97f9686 commit 5ab3c4d

File tree

3 files changed

+513
-20
lines changed

3 files changed

+513
-20
lines changed

src/core/task/Task.ts

Lines changed: 42 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
146146
readonly rootTaskId?: string
147147
readonly parentTaskId?: string
148148
childTaskId?: string
149+
wasSubtaskCancelled?: boolean
149150

150151
readonly instanceId: string
151152
readonly metadata: TaskMetadata
@@ -1649,32 +1650,55 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
16491650
})
16501651
}
16511652

1652-
public async completeSubtask(lastMessage: string) {
1653+
public async completeSubtask(lastMessage: string, wasCancelled: boolean = false) {
16531654
this.isPaused = false
16541655
this.childTaskId = undefined
1656+
this.wasSubtaskCancelled = wasCancelled
16551657

16561658
this.emit(RooCodeEventName.TaskUnpaused, this.taskId)
16571659

1658-
// Fake an answer from the subtask that it has completed running and
1659-
// this is the result of what it has done add the message to the chat
1660-
// history and to the webview ui.
1661-
try {
1662-
await this.say("subtask_result", lastMessage)
1660+
// Only add the subtask result if it wasn't cancelled
1661+
if (!wasCancelled) {
1662+
// Fake an answer from the subtask that it has completed running and
1663+
// this is the result of what it has done add the message to the chat
1664+
// history and to the webview ui.
1665+
try {
1666+
await this.say("subtask_result", lastMessage)
16631667

1664-
await this.addToApiConversationHistory({
1665-
role: "user",
1666-
content: [{ type: "text", text: `[new_task completed] Result: ${lastMessage}` }],
1667-
})
1668+
await this.addToApiConversationHistory({
1669+
role: "user",
1670+
content: [{ type: "text", text: `[new_task completed] Result: ${lastMessage}` }],
1671+
})
16681672

1669-
// Set skipPrevResponseIdOnce to ensure the next API call sends the full conversation
1670-
// including the subtask result, not just from before the subtask was created
1671-
this.skipPrevResponseIdOnce = true
1672-
} catch (error) {
1673-
this.providerRef
1674-
.deref()
1675-
?.log(`Error failed to add reply from subtask into conversation of parent task, error: ${error}`)
1673+
// Set skipPrevResponseIdOnce to ensure the next API call sends the full conversation
1674+
// including the subtask result, not just from before the subtask was created
1675+
this.skipPrevResponseIdOnce = true
1676+
} catch (error) {
1677+
this.providerRef
1678+
.deref()
1679+
?.log(`Error failed to add reply from subtask into conversation of parent task, error: ${error}`)
16761680

1677-
throw error
1681+
throw error
1682+
}
1683+
} else {
1684+
// If the subtask was cancelled, add a message indicating that
1685+
try {
1686+
await this.say("subtask_result", "Subtask was cancelled by user")
1687+
1688+
await this.addToApiConversationHistory({
1689+
role: "user",
1690+
content: [{ type: "text", text: `[new_task cancelled] The subtask was cancelled by the user.` }],
1691+
})
1692+
1693+
// Set skipPrevResponseIdOnce to ensure the next API call sends the full conversation
1694+
this.skipPrevResponseIdOnce = true
1695+
} catch (error) {
1696+
this.providerRef
1697+
.deref()
1698+
?.log(`Error failed to add cancellation message to parent task, error: ${error}`)
1699+
1700+
throw error
1701+
}
16781702
}
16791703
}
16801704

src/core/webview/ClineProvider.ts

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -481,13 +481,13 @@ export class ClineProvider
481481
// exists).
482482
// This is used when a subtask is finished and the parent task needs to be
483483
// resumed.
484-
async finishSubTask(lastMessage: string) {
484+
async finishSubTask(lastMessage: string, wasCancelled: boolean = false) {
485485
// Remove the last cline instance from the stack (this is the finished
486486
// subtask).
487487
await this.removeClineFromStack()
488488
// Resume the last cline instance in the stack (if it exists - this is
489489
// the 'parent' calling task).
490-
await this.getCurrentTask()?.completeSubtask(lastMessage)
490+
await this.getCurrentTask()?.completeSubtask(lastMessage, wasCancelled)
491491
}
492492
// Pending Edit Operations Management
493493

@@ -2582,6 +2582,44 @@ export class ClineProvider
25822582

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

2585+
// Check if this is a subtask
2586+
const isSubtask = task.parentTask !== undefined
2587+
2588+
if (isSubtask) {
2589+
// For subtasks, we handle cancellation differently to prevent automatic restart
2590+
console.log(`[cancelTask] Cancelling subtask ${task.taskId}.${task.instanceId}`)
2591+
2592+
// Mark this as a user-initiated cancellation
2593+
task.abortReason = "user_cancelled"
2594+
2595+
// Begin abort (non-blocking)
2596+
task.abortTask()
2597+
2598+
// Mark as abandoned to prevent residual activity
2599+
task.abandoned = true
2600+
2601+
// Wait for the task to finish aborting
2602+
await pWaitFor(
2603+
() =>
2604+
this.getCurrentTask()! === undefined ||
2605+
this.getCurrentTask()!.isStreaming === false ||
2606+
this.getCurrentTask()!.didFinishAbortingStream ||
2607+
this.getCurrentTask()!.isWaitingForFirstChunk,
2608+
{
2609+
timeout: 3_000,
2610+
},
2611+
).catch(() => {
2612+
console.error("Failed to abort subtask")
2613+
})
2614+
2615+
// Notify the parent task that the subtask was cancelled
2616+
await this.finishSubTask("Subtask was cancelled by user", true)
2617+
2618+
// Don't rehydrate - let the parent task handle what to do next
2619+
return
2620+
}
2621+
2622+
// For non-subtasks, use the original cancellation logic with rehydration
25852623
const { historyItem, uiMessagesFilePath } = await this.getTaskWithId(task.taskId)
25862624

25872625
// Preserve parent and root task information for history item.

0 commit comments

Comments
 (0)