-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: resolve phantom subtask display on cancel during API retry (#4602) #4893
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.
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
Conversation
- Fixed clearTask handler to check for actual parent task existence - Changed condition from getClineStackSize() > 1 to checking currentTask.parentTask - Prevents finishSubTask() from being called on tasks without parents - Added comprehensive test coverage for clearTask message handler - Resolves infinite checkpoint initialization loop issue
- Modified clearTask handler to check for parentTask property instead of stack size - This prevents single tasks from being incorrectly treated as subtasks - Added comprehensive test coverage for the fix - Fixes issue where canceling during API retry would show phantom 'Subtask Results' and create checkpoint initialization loop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Fixes an issue where canceling during an API retry incorrectly treated single tasks as subtasks.
- Replace
getClineStackSize()check with explicitparentTaskexistence in theclearTaskhandler - Introduce a new
clearTask()method onClineProviderfor non-subtask cancellations - Add comprehensive tests for both subtask and single-task cancellation flows
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/core/webview/webviewMessageHandler.ts | Updated clearTask case to branch on currentTask.parentTask and call finishSubTask or clearTask |
| src/core/webview/ClineProvider.ts | Added clearTask() helper to remove the current task without subtask logic |
| src/core/webview/tests/ClineProvider.spec.ts | Added tests covering the new clearTask logic, including the fix for issue #4602 |
Comments suppressed due to low confidence (2)
src/core/webview/tests/ClineProvider.spec.ts:675
- This test does not mock or assert
postStateToWebview; consider spying onprovider.postStateToWebviewto both avoid unintended side effects and verify that state is posted in this scenario.
await messageHandler({ type: "clearTask" })
src/core/webview/webviewMessageHandler.ts:207
- The translation function
tis used here but not imported in this module. Add the appropriate import (e.g.import { t } from 'i18n';) to prevent a runtime error.
await provider.finishSubTask(t("common:tasks.canceled"))
daniel-lxs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT
Description
Fixes #4602
This PR resolves an issue where canceling a task during API retry would incorrectly display "Subtask Results" and create a checkpoint initialization loop, even though no subtask was actually created.
Changes Made
clearTaskhandler inwebviewMessageHandler.tsto check for the existence of aparentTaskproperty instead of relying on stack sizefinishSubTask()methodclearTask()insteadTesting
ClineProvider.spec.ts:clearTaskfinishSubTaskVerification of Acceptance Criteria
Checklist
Technical Details
The root cause was that the previous implementation checked
provider.getClineStackSize() > 1to determine if a task was a subtask. However, during API retry scenarios, the stack could have multiple entries even for a single task, leading to incorrect behavior.The fix now properly checks for the existence of a
parentTaskproperty on the current task, which is the definitive way to determine if a task is actually a subtask.Important
Fixes phantom subtask display issue by checking
parentTaskproperty inwebviewMessageHandler.tsduring task cancellation.clearTaskinwebviewMessageHandler.tsnow checks forparentTaskproperty instead of stack size to determine subtask status.parentTasktriggerfinishSubTask(); others useclearTask().ClineProvider.spec.tsfor single tasks, child tasks, and no current task scenarios.This description was created by
for 0cc9eca. You can customize this summary. It will automatically update as commits are pushed.