Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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! |
775a5d4 to
6afd917
Compare
…ail-closed error handling
|
Closing in favor of landing #355 Epic 1+2 foundation first. The multi-agent code review surfaced 3 blocking concurrency bugs from unserialized delegation-state mutations — a structural problem that the transition guards (Story 2.3, #366) and The behavioral design, test coverage (39 unit + 4 E2E tests), and review fixes on this branch are preserved for reuse when |
Related GitHub Issue
Closes: #560
Description
When a delegated subtask is interrupted mid-execution (user hits stop),
cancelTaskimmediately severed the parent-child link — parent went from"delegated"→"active"withawaitingChildIdcleared. When the user resumed the child and it calledattempt_completion, the parent no longer awaited it, so it fell through to the standalone "Start New Task" flow instead of reporting back.Changes
Core fix — 4 files:
packages/types/src/history.ts— Add"interrupted"to status enumpackages/types/src/task.ts/src/core/task/Task.ts/src/core/task-persistence/taskMetadata.ts— Propagate"interrupted"throughinitialStatustypessrc/core/webview/ClineProvider.ts—cancelTaskandremoveClineFromStacknow mark the child as"interrupted"instead of detaching the parent. Parent stays"delegated"withawaitingChildIdintact.src/core/tools/AttemptCompletionTool.ts— Accept"interrupted"alongside"active"so resumed children can still delegate back to parentCLI type sync — 2 files:
apps/cli/src/ui/types.ts/apps/cli/src/ui/components/autocomplete/triggers/HistoryTrigger.tsx— Add"interrupted"to CLI status types to prevent type driftTest flake fix — 1 file:
src/services/mcp/__tests__/McpHub.spec.ts— Replace 20 instances ofsetTimeout(resolve, 100)withmcpHub.waitUntilReady()to fix a pre-existing timing race that surfaces under thread-pool pressure in the full test suiteHow it works
Test Procedure
Unit tests (3 files updated, 1 new test added):
src/__tests__/removeClineFromStack-delegation.spec.ts— Updated to assert child marked interrupted (not parent detached)src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts— Updated for new cancel behaviorsrc/core/tools/__tests__/attemptCompletionTool.spec.ts— Added test for interrupted child delegating back to parent; added test for unexpected child status branchE2E test (1 test updated):
apps/vscode-e2e/src/suite/subtasks.test.ts— Updated "cancelled child" test to assert the new behavior: interrupted child resumes → reports back to parent → parent completes. Validated locally viaxvfb-run.All tests pass:
pnpm exec vitest run(unit),xvfb-run pnpm --filter @roo-code/vscode-e2e test:ci:mock(e2e).Pre-Submission Checklist
Additional Notes
historyItemSchemainstead of independent unions)setTimeout(resolve, 100)timing races under vitest thread-pool pressure