Skip to content

Commit adcc10a

Browse files
daniel-lxsroomote
authored andcommitted
fix: address PR review feedback for task cancellation
- Add proper resource disposal in resetToResumableState() to prevent memory leaks - Add JSDoc documentation for skipSave parameter in abortTask() - Add error handling for resetToResumableState() and ask() operations - Extract resumption logic into dedicated handleUserCancellationResume() method - Add 30-second timeout safety net for cancellation in ClineProvider These changes ensure proper cleanup of resources (browsers, terminals) during task cancellation while maintaining the no-rerender behavior.
1 parent c93d690 commit adcc10a

File tree

2 files changed

+90
-6
lines changed

2 files changed

+90
-6
lines changed

src/core/task/Task.ts

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1489,7 +1489,13 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
14891489
}
14901490
}
14911491

1492-
public async abortTask(isAbandoned = false, skipSave = false) {
1492+
/**
1493+
* Aborts the current task and optionally saves messages.
1494+
*
1495+
* @param isAbandoned - If true, marks the task as abandoned (no cleanup needed)
1496+
* @param skipSave - If true, skips saving messages (used during user cancellation when messages are already saved)
1497+
*/
1498+
public async abortTask(isAbandoned = false, skipSave = false) {
14931499
console.log(`[subtasks] aborting task ${this.taskId}.${this.instanceId}`)
14941500

14951501
// Will stop any autonomously running promises.
@@ -1521,6 +1527,9 @@ public async abortTask(isAbandoned = false, skipSave = false) {
15211527
/**
15221528
* Reset the task to a resumable state without recreating the instance.
15231529
* This is used when canceling a task to avoid unnecessary rerenders.
1530+
*
1531+
* IMPORTANT: This method cleans up resources to prevent memory leaks
1532+
* while preserving the task instance for resumption.
15241533
*/
15251534
public async resetToResumableState() {
15261535
console.log(`[subtasks] resetting task ${this.taskId}.${this.instanceId} to resumable state`)
@@ -1553,7 +1562,6 @@ public async abortTask(isAbandoned = false, skipSave = false) {
15531562
this.askResponse = undefined
15541563
this.askResponseText = undefined
15551564
this.askResponseImages = undefined
1556-
this.blockingAsk = undefined
15571565

15581566
// Reset parser if exists
15591567
if (this.assistantMessageParser) {
@@ -1566,9 +1574,27 @@ public async abortTask(isAbandoned = false, skipSave = false) {
15661574
await this.diffViewProvider.reset()
15671575
}
15681576

1569-
// The task is now ready to be resumed
1570-
// The API request status has already been updated by abortStream
1571-
// We don't add the resume_task message here because ask() will add it
1577+
// Dispose of resources that could accumulate if tasks are repeatedly cancelled
1578+
// These will be recreated as needed when the task resumes
1579+
try {
1580+
// Close browser sessions to free memory and browser processes
1581+
if (this.urlContentFetcher) {
1582+
this.urlContentFetcher.closeBrowser()
1583+
}
1584+
if (this.browserSession) {
1585+
this.browserSession.closeBrowser()
1586+
}
1587+
1588+
// Release any terminals associated with this task
1589+
// They will be recreated if needed when the task resumes
1590+
if (this.terminalProcess) {
1591+
this.terminalProcess.abort()
1592+
this.terminalProcess = undefined
1593+
}
1594+
} catch (error) {
1595+
console.error(`Error disposing resources during resetToResumableState: ${error}`)
1596+
// Continue even if resource cleanup fails
1597+
}
15721598

15731599
// Keep messages and history intact for resumption
15741600
// The task is now ready to be resumed without recreation
@@ -2771,6 +2797,45 @@ if (this.abort) {
27712797
}
27722798
}
27732799

2800+
/**
2801+
* Handles the resumption flow after a user cancels a task.
2802+
* This method resets the task state, shows the resume prompt,
2803+
* and continues with new user input if provided.
2804+
*
2805+
* @returns Promise<boolean> - true if the task should end, false to continue
2806+
*/
2807+
private async handleUserCancellationResume(): Promise<boolean> {
2808+
try {
2809+
// Reset the task to a resumable state
2810+
this.abort = false
2811+
await this.resetToResumableState()
2812+
2813+
// Show the resume prompt
2814+
const { response, text, images } = await this.ask("resume_task")
2815+
2816+
if (response === "messageResponse") {
2817+
await this.say("user_feedback", text, images)
2818+
// Continue with the new user input
2819+
const newUserContent: Anthropic.Messages.ContentBlockParam[] = []
2820+
if (text) {
2821+
newUserContent.push({ type: "text", text })
2822+
}
2823+
if (images && images.length > 0) {
2824+
newUserContent.push(...formatResponse.imageBlocks(images))
2825+
}
2826+
// Recursively continue with the new content
2827+
return await this.recursivelyMakeClineRequests(newUserContent)
2828+
}
2829+
// If not messageResponse, the task will end
2830+
return true
2831+
} catch (error) {
2832+
// If there's an error during resumption, log it and end the task
2833+
console.error(`Error during user cancellation resume: ${error}`)
2834+
// Re-throw to maintain existing error handling behavior
2835+
throw error
2836+
}
2837+
}
2838+
27742839
// Getters
27752840

27762841
public get cwd() {

src/core/webview/ClineProvider.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1282,8 +1282,27 @@ export class ClineProvider
12821282
// Just set the abort flag - the task will handle its own resumption
12831283
cline.abort = true
12841284

1285+
// Add a timeout safety net to ensure the task doesn't hang indefinitely
1286+
// If the task doesn't respond to cancellation within 30 seconds, force abort it
1287+
const timeoutMs = 30000 // 30 seconds
1288+
const timeoutPromise = new Promise<void>((resolve) => {
1289+
setTimeout(async () => {
1290+
// Check if the task is still in an aborted state
1291+
if (cline.abort && !cline.abandoned) {
1292+
console.log(
1293+
`[subtasks] task ${cline.taskId}.${cline.instanceId} did not respond to cancellation within ${timeoutMs}ms, forcing abort`,
1294+
)
1295+
// Force abandon the task to ensure cleanup
1296+
cline.abandoned = true
1297+
// Remove it from the stack
1298+
await this.removeClineFromStack()
1299+
resolve()
1300+
}
1301+
}, timeoutMs)
1302+
})
1303+
12851304
// The task's streaming loop will detect the abort flag and handle the resumption
1286-
// No need to wait or do anything else here
1305+
// The timeout ensures we don't wait indefinitely
12871306
}
12881307

12891308
async updateCustomInstructions(instructions?: string) {

0 commit comments

Comments
 (0)