Skip to content

Conversation

@daniel-lxs
Copy link
Member

Problem

Fixes a race condition where the first tool in a parallel batch could be presented twice due to tools retaining partial=true after finalization.

Changes

  • Explicitly marks all tool_use blocks as non-partial after finalization completes

Testing

⚠️ Not yet tested - This is a draft PR for review and discussion.

Related

  • Fixes EXT-634
  • Related to the parallel tool calling feature (EXT-629)

@roomote
Copy link
Contributor

roomote bot commented Jan 27, 2026

Rooviewer Clock   See task on Roo Cloud

I've reviewed this PR and found no issues. The implementation looks sound:

  • The main fix (marking all tool_use blocks as non-partial after finalization) correctly prevents the race condition that caused duplicated tool rendering
  • The fix is well-placed after finalizeRawChunks() and catches edge cases where toolUseIndex was undefined or finalizeStreamingToolCall returned null
  • The CI/CD workflow changes add reasonable improvements (concurrency, lint/type-check/build steps)
  • The Cerebras provider config change adds temperature support

Note: The PR is marked as a draft with "Not yet tested" - consider adding integration testing before merging.

Previous reviews

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

@daniel-lxs daniel-lxs force-pushed the feature/ext-629-enable-parallel-tool-calling-with-new_task-isolation branch from e621c39 to d0a45a9 Compare January 27, 2026 06:12
@daniel-lxs daniel-lxs marked this pull request as ready for review January 27, 2026 06:14
@daniel-lxs daniel-lxs requested review from cte, jr and mrubens as code owners January 27, 2026 06:14
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Jan 27, 2026
@roomote
Copy link
Contributor

roomote bot commented Jan 27, 2026

🔄 Review in Progress - View Job

Summary

No issues found - The PR is ready for approval.

Changes Reviewed

This PR fixes a race condition where the first tool in a parallel batch could be presented twice due to tools retaining partial=true after finalization. It also implements new_task isolation logic to prevent orphaned tools when delegation disposes the parent task.

Key Changes:

  1. Task.ts (lines 3286-3295): Explicitly marks ALL tool_use and mcp_tool_use blocks as non-partial after finalization, preventing the double-presentation race condition.
  2. Task.ts (lines 3433-3466): Implements new_task isolation enforcement - truncates subsequent tools when new_task is called alongside other tools and injects error tool_result blocks.
  3. presentAssistantMessage.ts (line 878): Adds checkpoint save before new_task execution to ensure state is saved before delegation.
  4. NewTaskTool.ts: Removed checkpoint logic (moved to presentAssistantMessage.ts).
  5. new-task-isolation.spec.ts: New comprehensive test file for new_task isolation enforcement.

TODO

  • Verify existing comments against current code
  • Review changes and enumerate net new issues
  • Submit comments
  • Summarize issues flagged and update top-level comment
  • Approve the pull request

📋 View Job Details | Roo Code Reviewer

@daniel-lxs
Copy link
Member Author

Note: There are still concerns about the task.ask() method design being fragile when multiple asks of the same type occur in quick succession. This catch-all approach should handle the immediate race condition, but will do further testing to verify edge cases.

Copy link
Contributor

@roomote roomote bot left a comment

Choose a reason for hiding this comment

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

The changes look correct. The PR properly fixes the race condition where tools could retain partial=true after finalization, and implements proper new_task isolation to prevent orphaned tools when delegation disposes the parent task. The test coverage is comprehensive.

Explicitly marks all tool_use blocks as non-partial after finalization
to prevent race conditions where tools might still have partial=true.

Fixes EXT-634
@hannesrudolph hannesrudolph force-pushed the feature/ext-629-enable-parallel-tool-calling-with-new_task-isolation branch from d0a45a9 to 6eafee9 Compare January 27, 2026 15:11
@hannesrudolph
Copy link
Collaborator

Tested. LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:S This PR changes 10-29 lines, ignoring generated files.

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

3 participants