Skip to content

fix: resolve orchestrator subtask cancellation during API streaming error (#4896) #4905

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

fix: resolve orchestrator subtask cancellation during API streaming error (#4896) #4905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/core/task/Task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,14 @@ export class Task extends EventEmitter<ClineEvents> {
// this is the result of what it has done add the message to the chat
// history and to the webview ui.
try {
// Check if the task is aborted or abandoned before trying to add messages
if (this.abort || this.abandoned) {
this.providerRef
.deref()
?.log(`[subtasks] Parent task ${this.taskId} is aborted/abandoned, skipping resume message`)
return
}

await this.say("subtask_result", lastMessage)

await this.addToApiConversationHistory({
Expand All @@ -769,7 +777,8 @@ export class Task extends EventEmitter<ClineEvents> {
.deref()
?.log(`Error failed to add reply from subtask into conversation of parent task, error: ${error}`)

throw error
// Don't throw the error - we still want the parent task to be unpaused
// even if we couldn't add the message
}
}

Expand Down
16 changes: 15 additions & 1 deletion src/core/webview/webviewMessageHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,21 @@ export const webviewMessageHandler = async (
// Check if the current task actually has a parent task
const currentTask = provider.getCurrentCline()
if (currentTask && currentTask.parentTask) {
await provider.finishSubTask(t("common:tasks.canceled"))
// For subtasks, we need to ensure the parent task is resumed even if the subtask is in an error state
try {
// First abort the current subtask if it's still running
if (currentTask.isStreaming || !currentTask.didFinishAbortingStream) {
currentTask.abortTask()
Copy link

Copilot AI Jun 19, 2025

Choose a reason for hiding this comment

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

If abortTask returns a promise, consider awaiting it (e.g., await currentTask.abortTask()) to ensure the abort completes before continuing.

Suggested change
currentTask.abortTask()
await currentTask.abortTask()

Copilot uses AI. Check for mistakes.
// Small delay to ensure abort signal is processed before cleanup
// This prevents race conditions where the stream might still be processing
await new Promise((resolve) => setTimeout(resolve, 100))
Copy link

Copilot AI Jun 19, 2025

Choose a reason for hiding this comment

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

The hardcoded 100ms delay is a magic number; extract it into a named constant (e.g., ABORT_DELAY_MS) to clarify its purpose and make tuning easier.

Suggested change
await new Promise((resolve) => setTimeout(resolve, 100))
await new Promise((resolve) => setTimeout(resolve, ABORT_DELAY_MS))

Copilot uses AI. Check for mistakes.
}
await provider.finishSubTask(t("common:tasks.canceled"))
} catch (error) {
// If there's an error, still try to resume the parent task
provider.log(`Error during subtask cancellation: ${error}`)
Comment on lines +216 to +219
Copy link

Copilot AI Jun 19, 2025

Choose a reason for hiding this comment

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

This finishSubTask call appears both inside the try and catch blocks; consider moving it after the try/catch to avoid duplication and centralize cleanup.

Suggested change
await provider.finishSubTask(t("common:tasks.canceled"))
} catch (error) {
// If there's an error, still try to resume the parent task
provider.log(`Error during subtask cancellation: ${error}`)
} catch (error) {
// If there's an error, still try to resume the parent task
provider.log(`Error during subtask cancellation: ${error}`)
} finally {

Copilot uses AI. Check for mistakes.
await provider.finishSubTask(t("common:tasks.canceled"))
}
} else {
// Regular task - just clear it
await provider.clearTask()
Expand Down
Loading