Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR separates delegated-task eviction from delegation repair, preserves interrupted parent-child links, adds explicit subtask abandonment through the API and webview, updates delegated/interrupted task UI, and adds unit and end-to-end coverage. ChangesSubtask lifecycle and abandonment
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
webview-ui/src/i18n/locales/ca/chat.json (1)
21-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTranslate new localization keys.
The new localization keys for subtask states were added with English fallback strings across non-English locales. Updating them to their respective translated values ensures a consistent localized user experience.
webview-ui/src/i18n/locales/ca/chat.json#L21-L22: Update to Catalan:"waitingOnSubtask": "Esperant la subtasca","goToSubtask": "Anar a la subtasca".webview-ui/src/i18n/locales/ca/history.json#L58-L59: Update to Catalan:"delegatedTag": "Esperant la subtasca","interruptedTag": "Interromput".webview-ui/src/i18n/locales/de/chat.json#L21-L22: Update to German:"waitingOnSubtask": "Warten auf Teilaufgabe","goToSubtask": "Zur Teilaufgabe".webview-ui/src/i18n/locales/de/history.json#L58-L59: Update to German:"delegatedTag": "Warten auf Teilaufgabe","interruptedTag": "Unterbrochen".webview-ui/src/i18n/locales/es/chat.json#L21-L22: Update to Spanish:"waitingOnSubtask": "Esperando subtarea","goToSubtask": "Ir a la subtarea".webview-ui/src/i18n/locales/es/history.json#L58-L59: Update to Spanish:"delegatedTag": "Esperando subtarea","interruptedTag": "Interrumpido".webview-ui/src/i18n/locales/fr/chat.json#L21-L22: Update to French:"waitingOnSubtask": "En attente de la sous-tâche","goToSubtask": "Aller à la sous-tâche".webview-ui/src/i18n/locales/fr/history.json#L58-L59: Update to French:"delegatedTag": "En attente de la sous-tâche","interruptedTag": "Interrompu".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/vscode-e2e/src/fixtures/subtasks.ts`:
- Around line 388-392: Update the child fixture’s predicate to exclude requests
containing SUBTASK_ABANDON_PARENT_PROMPT, matching the interrupted scenario’s
parent-request exclusion. Keep the existing child marker, tool-call, and
follow-up answer checks unchanged so only the intended child request is
accepted.
In `@src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts`:
- Around line 809-810: Replace the fixed 10 ms sleeps after TaskCompleted
emissions in the task-history tests with awaited completion of the listener
promises. Update the fake emitter used by ClineProvider task-history tests to
collect listener return promises and make emit() await them, then await emit()
directly at the affected cases around task-cb-1 and the additional TaskCompleted
scenarios.
In `@src/core/webview/ClineProvider.ts`:
- Around line 533-540: Update provider disposal to call evictCurrentTask()
instead of removeClineFromStack(), ensuring an active delegated child is marked
interrupted during shutdown while preserving the existing disposal flow.
- Around line 600-609: The in-lock transition in markDelegatedChildInterrupted
must proceed only when childHistory.status is exactly "active". Update the guard
to skip every other status, including "completed", then validate the
active-to-interrupted transition before calling
updateTaskHistory(interruptedChild).
- Around line 3599-3615: Update the delegation flow around atomicReadAndUpdate
to execute under runDelegationTransition(), removing the pre-lock
existingAwaitedChildStatus snapshot. Inside the updater, read
historyItem.awaitingChildId and its current status together, and only sever the
link when that exact child is confirmed interrupted; otherwise throw and
preserve rollback behavior.
- Around line 541-553: Ensure interruption-persistence failures are propagated
instead of swallowed: update markDelegatedChildInterrupted and its callers,
including evictCurrentTask and the corresponding paths near createTask and
abandonSubtask, to preserve rejection. Remove ineffective non-fatal catches and
prevent createTask from opening a replacement task when eviction fails, leaving
the live child/history state consistent.
In `@webview-ui/src/i18n/locales/nl/chat.json`:
- Around line 21-22: Replace the English fallback values for waitingOnSubtask
and goToSubtask with accurate Dutch translations in
webview-ui/src/i18n/locales/nl/chat.json:21-22, Polish translations in
webview-ui/src/i18n/locales/pl/chat.json:21-22, Brazilian Portuguese
translations in webview-ui/src/i18n/locales/pt-BR/chat.json:21-22, Russian
translations in webview-ui/src/i18n/locales/ru/chat.json:21-22, and Turkish
translations in webview-ui/src/i18n/locales/tr/chat.json:21-22. Also localize
the newly added status-label keys in
webview-ui/src/i18n/locales/nl/history.json:51-52,
webview-ui/src/i18n/locales/pl/history.json:51-52,
webview-ui/src/i18n/locales/pt-BR/history.json:51-52, and
webview-ui/src/i18n/locales/ru/history.json:51-52, preserving the existing keys
and JSON structure.
In `@webview-ui/src/i18n/locales/tr/history.json`:
- Around line 51-52: Replace the English placeholder values for delegatedTag and
interruptedTag with Turkish translations in
webview-ui/src/i18n/locales/tr/history.json:51-52. Add Vietnamese navigation
labels in webview-ui/src/i18n/locales/vi/chat.json:21-22 and Vietnamese status
labels in webview-ui/src/i18n/locales/vi/history.json:51-52. Add Simplified
Chinese navigation and status labels in
webview-ui/src/i18n/locales/zh-CN/chat.json:21-22 and
webview-ui/src/i18n/locales/zh-CN/history.json:51-52, and Traditional Chinese
equivalents in webview-ui/src/i18n/locales/zh-TW/chat.json:21-22 and
webview-ui/src/i18n/locales/zh-TW/history.json:51-52; preserve the existing
translation keys and JSON structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 285a6a43-99ec-47fe-afa2-80fc1187e121
📒 Files selected for processing (61)
apps/vscode-e2e/src/fixtures/subtasks.tsapps/vscode-e2e/src/suite/subtasks.test.tspackages/types/src/api.tspackages/types/src/vscode-extension-host.tssrc/__tests__/abandonSubtask.spec.tssrc/__tests__/api-subtask.spec.tssrc/__tests__/helpers/provider-stub.tssrc/__tests__/history-resume-delegation.spec.tssrc/__tests__/provider-delegation.spec.tssrc/__tests__/removeClineFromStack-delegation.spec.tssrc/__tests__/single-open-invariant.spec.tssrc/activate/__tests__/registerCommands.spec.tssrc/activate/registerCommands.tssrc/core/webview/ClineProvider.tssrc/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.tssrc/core/webview/__tests__/ClineProvider.taskHistory.spec.tssrc/core/webview/__tests__/webviewMessageHandler.abandonSubtask.spec.tssrc/core/webview/webviewMessageHandler.tssrc/extension/api.tswebview-ui/src/components/chat/TaskHeader.tsxwebview-ui/src/components/chat/__tests__/TaskHeader.spec.tsxwebview-ui/src/components/history/SubtaskRow.tsxwebview-ui/src/components/history/TaskItemFooter.tsxwebview-ui/src/components/history/TaskStatusBadge.tsxwebview-ui/src/components/history/__tests__/TaskItemFooter.spec.tsxwebview-ui/src/i18n/locales/ca/chat.jsonwebview-ui/src/i18n/locales/ca/history.jsonwebview-ui/src/i18n/locales/de/chat.jsonwebview-ui/src/i18n/locales/de/history.jsonwebview-ui/src/i18n/locales/en/chat.jsonwebview-ui/src/i18n/locales/en/history.jsonwebview-ui/src/i18n/locales/es/chat.jsonwebview-ui/src/i18n/locales/es/history.jsonwebview-ui/src/i18n/locales/fr/chat.jsonwebview-ui/src/i18n/locales/fr/history.jsonwebview-ui/src/i18n/locales/hi/chat.jsonwebview-ui/src/i18n/locales/hi/history.jsonwebview-ui/src/i18n/locales/id/chat.jsonwebview-ui/src/i18n/locales/id/history.jsonwebview-ui/src/i18n/locales/it/chat.jsonwebview-ui/src/i18n/locales/it/history.jsonwebview-ui/src/i18n/locales/ja/chat.jsonwebview-ui/src/i18n/locales/ja/history.jsonwebview-ui/src/i18n/locales/ko/chat.jsonwebview-ui/src/i18n/locales/ko/history.jsonwebview-ui/src/i18n/locales/nl/chat.jsonwebview-ui/src/i18n/locales/nl/history.jsonwebview-ui/src/i18n/locales/pl/chat.jsonwebview-ui/src/i18n/locales/pl/history.jsonwebview-ui/src/i18n/locales/pt-BR/chat.jsonwebview-ui/src/i18n/locales/pt-BR/history.jsonwebview-ui/src/i18n/locales/ru/chat.jsonwebview-ui/src/i18n/locales/ru/history.jsonwebview-ui/src/i18n/locales/tr/chat.jsonwebview-ui/src/i18n/locales/tr/history.jsonwebview-ui/src/i18n/locales/vi/chat.jsonwebview-ui/src/i18n/locales/vi/history.jsonwebview-ui/src/i18n/locales/zh-CN/chat.jsonwebview-ui/src/i18n/locales/zh-CN/history.jsonwebview-ui/src/i18n/locales/zh-TW/chat.jsonwebview-ui/src/i18n/locales/zh-TW/history.json
|
7 issues outstanding. Findings verified against the code were posted as replies on the matching review threads; the one finding with no existing thread is carried below.
Reviewed c13507c |
…skCompleted paths
…ildInterrupted, evict on disposal, fix delegateParentAndOpenChild TOCTOU, translate i18n keys
There was a problem hiding this comment.
🧹 Nitpick comments (2)
webview-ui/src/components/history/TaskItemFooter.tsx (1)
40-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd local coverage for both new status branches.
Please ensure
webview-ui/src/components/history/__tests__/TaskItemFooter.spec.tsxasserts both"delegated"and"interrupted"render the corresponding status badge and label.As per coding guidelines, React/webview behavior should use package-local
webview-uitests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/history/TaskItemFooter.tsx` around lines 40 - 46, Add package-local tests in TaskItemFooter.spec.tsx covering both delegated and interrupted item statuses, asserting each renders its corresponding TaskStatusBadge and visible status label. Keep the tests focused on the conditional branches in TaskItemFooter.Source: Coding guidelines
webview-ui/src/components/chat/__tests__/TaskHeader.spec.tsx (1)
228-279: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the delegated-without-child state.
The suite checks non-delegated and delegated-with-child cases, but not
status: "delegated"withoutawaitingChildId. Add a local webview test asserting that the banner remains hidden in that state.As per coding guidelines, React/webview behavior should be covered by local
webview-uitests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/chat/__tests__/TaskHeader.spec.tsx` around lines 228 - 279, Add a test in the “Delegated parent waiting-on-subtask banner” suite covering a currentTaskItem with status "delegated" but no awaitingChildId. Render via renderTaskHeader and assert the waiting-on-subtask banner is not present, following the existing non-delegated visibility test.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@webview-ui/src/components/chat/__tests__/TaskHeader.spec.tsx`:
- Around line 228-279: Add a test in the “Delegated parent waiting-on-subtask
banner” suite covering a currentTaskItem with status "delegated" but no
awaitingChildId. Render via renderTaskHeader and assert the waiting-on-subtask
banner is not present, following the existing non-delegated visibility test.
In `@webview-ui/src/components/history/TaskItemFooter.tsx`:
- Around line 40-46: Add package-local tests in TaskItemFooter.spec.tsx covering
both delegated and interrupted item statuses, asserting each renders its
corresponding TaskStatusBadge and visible status label. Keep the tests focused
on the conditional branches in TaskItemFooter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c2a3e1a2-44bb-475b-bff8-992bdef918f0
📒 Files selected for processing (61)
apps/vscode-e2e/src/fixtures/subtasks.tsapps/vscode-e2e/src/suite/subtasks.test.tspackages/types/src/api.tspackages/types/src/vscode-extension-host.tssrc/__tests__/abandonSubtask.spec.tssrc/__tests__/api-subtask.spec.tssrc/__tests__/helpers/provider-stub.tssrc/__tests__/history-resume-delegation.spec.tssrc/__tests__/provider-delegation.spec.tssrc/__tests__/removeClineFromStack-delegation.spec.tssrc/__tests__/single-open-invariant.spec.tssrc/activate/__tests__/registerCommands.spec.tssrc/activate/registerCommands.tssrc/core/webview/ClineProvider.tssrc/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.tssrc/core/webview/__tests__/ClineProvider.taskHistory.spec.tssrc/core/webview/__tests__/webviewMessageHandler.abandonSubtask.spec.tssrc/core/webview/webviewMessageHandler.tssrc/extension/api.tswebview-ui/src/components/chat/TaskHeader.tsxwebview-ui/src/components/chat/__tests__/TaskHeader.spec.tsxwebview-ui/src/components/history/SubtaskRow.tsxwebview-ui/src/components/history/TaskItemFooter.tsxwebview-ui/src/components/history/TaskStatusBadge.tsxwebview-ui/src/components/history/__tests__/TaskItemFooter.spec.tsxwebview-ui/src/i18n/locales/ca/chat.jsonwebview-ui/src/i18n/locales/ca/history.jsonwebview-ui/src/i18n/locales/de/chat.jsonwebview-ui/src/i18n/locales/de/history.jsonwebview-ui/src/i18n/locales/en/chat.jsonwebview-ui/src/i18n/locales/en/history.jsonwebview-ui/src/i18n/locales/es/chat.jsonwebview-ui/src/i18n/locales/es/history.jsonwebview-ui/src/i18n/locales/fr/chat.jsonwebview-ui/src/i18n/locales/fr/history.jsonwebview-ui/src/i18n/locales/hi/chat.jsonwebview-ui/src/i18n/locales/hi/history.jsonwebview-ui/src/i18n/locales/id/chat.jsonwebview-ui/src/i18n/locales/id/history.jsonwebview-ui/src/i18n/locales/it/chat.jsonwebview-ui/src/i18n/locales/it/history.jsonwebview-ui/src/i18n/locales/ja/chat.jsonwebview-ui/src/i18n/locales/ja/history.jsonwebview-ui/src/i18n/locales/ko/chat.jsonwebview-ui/src/i18n/locales/ko/history.jsonwebview-ui/src/i18n/locales/nl/chat.jsonwebview-ui/src/i18n/locales/nl/history.jsonwebview-ui/src/i18n/locales/pl/chat.jsonwebview-ui/src/i18n/locales/pl/history.jsonwebview-ui/src/i18n/locales/pt-BR/chat.jsonwebview-ui/src/i18n/locales/pt-BR/history.jsonwebview-ui/src/i18n/locales/ru/chat.jsonwebview-ui/src/i18n/locales/ru/history.jsonwebview-ui/src/i18n/locales/tr/chat.jsonwebview-ui/src/i18n/locales/tr/history.jsonwebview-ui/src/i18n/locales/vi/chat.jsonwebview-ui/src/i18n/locales/vi/history.jsonwebview-ui/src/i18n/locales/zh-CN/chat.jsonwebview-ui/src/i18n/locales/zh-CN/history.jsonwebview-ui/src/i18n/locales/zh-TW/chat.jsonwebview-ui/src/i18n/locales/zh-TW/history.json
🚧 Files skipped from review as they are similar to previous changes (53)
- webview-ui/src/i18n/locales/nl/chat.json
- webview-ui/src/i18n/locales/zh-CN/chat.json
- webview-ui/src/components/history/TaskStatusBadge.tsx
- webview-ui/src/i18n/locales/ko/history.json
- webview-ui/src/i18n/locales/it/history.json
- webview-ui/src/components/history/tests/TaskItemFooter.spec.tsx
- webview-ui/src/i18n/locales/ja/chat.json
- src/tests/history-resume-delegation.spec.ts
- webview-ui/src/i18n/locales/ca/chat.json
- packages/types/src/api.ts
- src/tests/single-open-invariant.spec.ts
- src/activate/tests/registerCommands.spec.ts
- webview-ui/src/i18n/locales/pl/history.json
- webview-ui/src/i18n/locales/ca/history.json
- src/tests/api-subtask.spec.ts
- webview-ui/src/i18n/locales/es/history.json
- webview-ui/src/i18n/locales/it/chat.json
- src/tests/helpers/provider-stub.ts
- webview-ui/src/components/history/SubtaskRow.tsx
- webview-ui/src/i18n/locales/ru/history.json
- webview-ui/src/i18n/locales/tr/history.json
- webview-ui/src/i18n/locales/ko/chat.json
- webview-ui/src/i18n/locales/fr/chat.json
- src/extension/api.ts
- webview-ui/src/i18n/locales/vi/chat.json
- webview-ui/src/i18n/locales/id/chat.json
- src/core/webview/webviewMessageHandler.ts
- webview-ui/src/i18n/locales/fr/history.json
- webview-ui/src/i18n/locales/de/chat.json
- webview-ui/src/i18n/locales/es/chat.json
- webview-ui/src/i18n/locales/zh-TW/history.json
- src/core/webview/tests/ClineProvider.flicker-free-cancel.spec.ts
- webview-ui/src/i18n/locales/ja/history.json
- webview-ui/src/i18n/locales/en/history.json
- webview-ui/src/i18n/locales/ru/chat.json
- webview-ui/src/i18n/locales/vi/history.json
- webview-ui/src/i18n/locales/tr/chat.json
- webview-ui/src/i18n/locales/de/history.json
- src/activate/registerCommands.ts
- webview-ui/src/i18n/locales/hi/history.json
- apps/vscode-e2e/src/suite/subtasks.test.ts
- webview-ui/src/i18n/locales/zh-CN/history.json
- webview-ui/src/i18n/locales/nl/history.json
- src/tests/abandonSubtask.spec.ts
- webview-ui/src/i18n/locales/pt-BR/chat.json
- webview-ui/src/i18n/locales/zh-TW/chat.json
- src/core/webview/tests/webviewMessageHandler.abandonSubtask.spec.ts
- packages/types/src/vscode-extension-host.ts
- src/tests/provider-delegation.spec.ts
- webview-ui/src/components/chat/TaskHeader.tsx
- webview-ui/src/i18n/locales/hi/chat.json
- src/tests/removeClineFromStack-delegation.spec.ts
- src/core/webview/ClineProvider.ts
Related GitHub Issue
Closes: #559
Description
This PR implements the Abandon Subtask flow end-to-end: a parent task that is waiting on a delegated subtask can sever the link if that subtask is interrupted, restoring the parent to active without losing the subtask's history.
Key changes:
Task lifecycle (
ClineProvider,webviewMessageHandler,extension/api):abandonSubtask()atomically severs the parent-child link in the history store, transitions parentdelegated → active, and closes the live child instance before the write so a latersaveClineMessagescan't silently reattach it. A TOCTOU guard re-checks child status inside the per-parent delegation lock.evictCurrentTask(): public method that snapshots the current task, pops it viaremoveClineFromStack, then callsmarkDelegatedChildInterruptedif the child was active+delegated — so the+button andclearCurrentTaskboth correctly interrupt an active delegated child rather than losing the delegation state.onTaskCompletedclosure: explicitly transitions the task tocompletedin the history store before re-emitting the event, ensuring a priorinterruptedstatus (from a cancel followed by resumption and completion) is overwritten rather than preserved by the store's merge.History UI (
HistoryView,TaskGroupItem,TaskItem,TaskItemFooter,SubtaskRow): delegation status badges (delegatedTag,interruptedTag).i18n: all new keys present in all 18 locale files (
en+ 17 others).sequenceIndexexhaustion across sibling tests."delegated child completion persists parent and child history state"where the parent task could complete beforegetTaskHistoryItemasserts the parent status (activeorcompleted).Test coverage additions:
evictCurrentTask,markDelegatedChildInterrupted(TOCTOU + error paths),onTaskCompleted(real closure viataskCreationCallback)abandonSubtask: sever, ordering (close before write), TOCTOU, active-child guard, stale-parent guardabandonSubtaskWithIdwebview message handler: happy path + error catch/logAPI.clearCurrentTaskandAPI.abandonSubtaskpassthroughplusButtonClickedcommand handler callsevictCurrentTaskTest Procedure
new_taskto delegate to a subtask, then navigate away (interrupting the subtask). In History, open the parent — it should show a "Delegated" badge and an "Abandon Subtask" button. Confirm the dialog; the parent returns to active and the subtask is no longer linked.pnpm --filter zoo-code testPre-Submission Checklist
🤖 Generated with Claude Code