Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Nov 27, 2025

Problem

The new_task tool for native protocol was broken - it was not returning a tool_result, which broke the conversation history structure.

Root Cause

In NewTaskTool.ts, the pendingNewTaskToolCallId was being set after startSubtask() returned. However, startSubtask() contains a 500ms delay during which the subtask could complete. If the subtask completed during this window, completeSubtask() would be called before pendingNewTaskToolCallId was set, causing it to fall through to the XML protocol path and add a text message instead of a proper tool_result block.

This race condition would break the API conversation structure because the native protocol API expects a tool_result block for every tool_use block.

Fix

Move the pendingNewTaskToolCallId assignment to happen before calling startSubtask(), ensuring the ID is set before the subtask starts. If the subtask creation fails, the pending ID is cleared.

Changes

  • src/core/tools/NewTaskTool.ts: Reordered the logic to set pendingNewTaskToolCallId before startSubtask() and clear it if subtask creation fails.

Testing

All 4325 tests pass.


Important

Fixes race condition in NewTaskTool.ts by setting pendingNewTaskToolCallId before startSubtask() for native protocol.

  • Behavior:
    • Fixes race condition in NewTaskTool.ts by setting pendingNewTaskToolCallId before startSubtask() for native protocol.
    • Clears pendingNewTaskToolCallId if subtask creation fails.
  • Testing:
    • All 4325 tests pass.

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

The pendingNewTaskToolCallId was being set AFTER startSubtask() returned.
However, startSubtask() contains a 500ms delay during which the subtask
could complete. If the subtask completed during this window, completeSubtask()
would be called before pendingNewTaskToolCallId was set, causing it to
fall through to the XML protocol path and add a text message instead of
a proper tool_result block, breaking the API conversation structure.

This fix moves the pendingNewTaskToolCallId assignment to happen BEFORE
calling startSubtask(), ensuring the ID is set before the subtask starts.
If the subtask creation fails, the pending ID is cleared.
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Nov 27, 2025
@roomote
Copy link
Contributor

roomote bot commented Nov 27, 2025

Rooviewer Clock   See task on Roo Cloud

Review completed - no issues found. The fix correctly addresses the race condition by setting pendingNewTaskToolCallId before calling startSubtask(), ensuring the ID is available when completeSubtask() is called during the 500ms delay window.

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 27, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 27, 2025
@mrubens mrubens merged commit d2d311e into main Nov 27, 2025
24 checks passed
@mrubens mrubens deleted the fix/new-task-native-protocol-race-condition branch November 27, 2025 17:30
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Nov 27, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Nov 27, 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. lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants