Skip to content

Conversation

@Mnehmos
Copy link
Contributor

@Mnehmos Mnehmos commented Nov 6, 2025

Most of y'all know me. I dont code, so I'd appreciate a review by an expert as well please. My tests work both manual and automated.


Related GitHub Issue

Closes: #8081

Roo Code Task Context (Optional)

N/A - Developed using Roo Code's Orchestrator mode with multi-agent collaboration.

Description

This PR fixes a recurring regression where subtasks completing after interruption fail to properly return control to parent agents, leaving the webview stuck displaying the subtask instead of switching to the resumed parent task.

Key Implementation Details:

  1. UI Navigation Fix: Added explicit chatButtonClicked action in ClineProvider.finishSubTask() after state posting. The previous implementation only called postStateToWebview() which broadcasts state changes but doesn't trigger UI navigation. This mirrors the pattern used in showTaskWithId() for consistency.

  2. Subtask Metadata Persistence: Implemented a new SubtaskMetadata schema (Zod-validated) that stores parent-child relationships in message metadata. This ensures task hierarchy survives VSCode restarts and enables proper parent restoration after interruptions.

  3. Enhanced Error Handling: Added verifyParentExists() method to check parent accessibility before completion, gracefully handling orphaned subtasks. Also added race tolerance for resume asks by treating ignored promises as implicit approval.

  4. Non-Interactive Injection: Made subtask_result message injection non-interactive to avoid interfering with pending resume asks, preventing timing conflicts.

Reviewers should pay attention to:

  • Lines 550-553 in ClineProvider.ts: The UI navigation action that fixes the core regression
  • Lines 3071-3112 in Task.ts: Metadata persistence implementation
  • Lines 1366-1380 in Task.ts: Resume ask race tolerance handling
  • Test mocking changes in ClineProvider.spec.ts (lines 210-224): Proper tuple return from Task.create()

Test Procedure

Automated Testing:

cd src
npx vitest run core/webview/__tests__/ClineProvider.finishSubTask.spec.ts
npx vitest run core/webview/__tests__/ClineProvider.spec.ts
npx vitest run __tests__/subtask-metadata-tracking.test.ts

Test Results:

  • ClineProvider.finishSubTask.spec.ts: 98 tests passing
  • ClineProvider.spec.ts: 92 passed, 6 skipped
  • subtask-metadata-tracking.test.ts: All tests passing

Manual Testing Procedure:

  1. Create a parent task with Orchestrator mode
  2. Create a subtask from the parent using new_task tool
  3. Interrupt the subtask (close VSCode or kill terminal mid-execution)
  4. Reopen VSCode and resume the subtask
  5. Complete the subtask using attempt_completion
  6. Verify: The webview should automatically switch to display the parent task
  7. Verify: Parent task should show "Subtask Completed" message
  8. Verify: Parent task should be in resume-ask state

Edge Case Testing:

  • Test with deleted parent task (should show error message, not crash)
  • Test with subtasks created before this PR (backward compatibility)
  • Test with multiple nested subtasks (depth > 1) (completed and working)

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes (if applicable).
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Before (Bug):

  • After subtask completion, webview remains stuck on subtask view
  • User must manually click through task history to find resumed parent

After (Fixed):

  • Subtask completion automatically switches webview to parent task
  • Parent task immediately visible with "Subtask Completed" notification
  • Seamless user experience with proper task flow

(https://vimeo.com/1134407282?share=copy&fl=sv&fe=ci)

Documentation Updates

  • No documentation updates are required.

Rationale: This is a bug fix that restores expected behavior. The feature (subtask completion returning to parent) already exists and is documented; this PR simply fixes the regression where the UI wasn't properly switching views.

Additional Notes

Backward Compatibility:

  • All changes are backward compatible with existing tasks
  • Legacy subtasks without metadata are handled gracefully via optional fields
  • No breaking changes to public APIs

Performance Impact:

  • Minimal: Metadata operations are lightweight and non-blocking
  • File system operations use existing infrastructure
  • No additional API calls or network requests

Code Quality:

  • No superfluous or placeholder code
  • Clean error handling with detailed logging
  • Follows existing patterns and conventions (e.g., mirrors showTaskWithId() navigation pattern)
  • Well-documented with inline comments

Related Changes:

  • Fixed Task.create() mock in test suite to return proper tuple structure
  • This was necessary to support existing tests after implementation changes

Get in Touch

Discord: @Mnehmos


Important

Fixes subtask completion flow to ensure UI returns to parent task, adds metadata persistence, and enhances error handling in ClineProvider and Task classes.

  • Behavior:
    • Fixes subtask completion flow in ClineProvider.finishSubTask() to ensure UI switches back to parent task.
    • Adds chatButtonClicked action in ClineProvider.finishSubTask() to trigger UI navigation.
    • Implements SubtaskMetadata schema in history.ts for parent-child task tracking.
    • Adds verifyParentExists() in Task.ts to check parent task accessibility.
    • Makes subtask_result injection non-interactive in Task.ts.
  • Testing:
    • Adds ClineProvider.finishSubTask.spec.ts to test subtask completion scenarios.
    • Updates ClineProvider.spec.ts to mock Task.create() and test task stack behavior.
  • Misc:
    • Updates Task.ts to persist subtask metadata and handle resume ask race conditions.

This description was created by Ellipsis for db9d2b1. You can customize this summary. It will automatically update as commits are pushed.

- Add SubtaskMetadata schema to history.ts for validating parent-child relationships
- Implement subtask metadata persistence in Task to track parentTaskId, depth, and return requirements
- Add verifyParentExists() method to check parent task accessibility before completion
- Enhance attemptCompletionTool with parent verification and orphaned subtask handling
- Make subtask_result injection non-interactive to avoid interfering with resume asks
- Add subtask metadata tracking test suite

Ensures subtasks maintain proper parent relationships across VSCode restarts and handle edge cases like missing/deleted parents gracefully.
@Mnehmos Mnehmos requested review from cte, jr and mrubens as code owners November 6, 2025 23:23
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. bug Something isn't working labels Nov 6, 2025
@roomote
Copy link
Contributor

roomote bot commented Nov 6, 2025

Rooviewer Clock   See task on Roo Cloud

Re-review complete. All previously identified issues have been resolved. No new issues found.

Issues Resolved:

Previous reviews

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Nov 6, 2025
Fixes identified by @roomote bot code review:

1. ClineProvider.finishSubTask() - Unhandled Promise Rejection
   - Added .catch handler to createTaskWithHistoryItem() restore call
   - Prevents silent failures when parent task initialization fails
   - Logs error without breaking subtask completion flow
   - Location: src/core/webview/ClineProvider.ts:503-506

2. ClineProvider.createTaskWithHistoryItem() - Missing Parameter
   - Added optional 'options' parameter with viewOnly flag
   - Fixed undefined reference to options.viewOnly (was options?.viewOnly)
   - Ensures backward compatibility with existing callers
   - Location: src/core/webview/ClineProvider.ts:982-984,1117

3. attemptCompletionTool() - Time-of-Check-Time-of-Use Race
   - Removed pre-verification of parent existence (lines 98-121)
   - Delegates parent validation to ClineProvider.finishSubTask()
   - finishSubTask() already handles missing parents gracefully
   - Eliminates race where parent deleted between check and restore
   - Location: src/core/tools/attemptCompletionTool.ts:98-139

Impact: Improves reliability of resumed subtask → parent return flow
Tests: 46 tests passing (finishSubTask + metadata tracking)
Risk: Low - defensive programming, no breaking changes
References: roomote feedback on PR fix/8081-subtask-parent-return-v2
@hannesrudolph
Copy link
Collaborator

@Mnehmos
Closing in favor of PR #9090.

Thank you for the work in PR 9086. After comparing both branches, PR 9090 delivers the same delegation resume improvement with a smaller and more focused scope. It keeps the changes limited to the backend, avoids UI and localization updates, and provides a clearer resume path.

@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Nov 13, 2025
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Nov 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[BUG] Subtask completes but doesn’t return to parent after interruptions

2 participants