Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Dec 4, 2025

Problem

When editing a user_feedback message that corresponds to a tool_result in the API conversation history, the previous behavior would delete the message and create a new one. This caused the tool_use_id to be lost, creating a mismatch between the tool_call and tool_result that caused errors with native tool protocols.

Solution

This PR implements in-place editing for tool_result messages:

Changes to Task.ts

  • Added resumeAfterMessageEdit() method that properly resets task state and resumes the task loop without creating a new user message
  • Clears pending ask states and promises
  • Resets abort, streaming, and tool execution flags
  • Updates environment details in the last user message
  • Ensures the next API call includes full context

Changes to webviewMessageHandler.ts

  • Detect when the edited message is a tool_result message in the API history
  • Handle timestamp mismatch between clineMessages and apiConversationHistory using position-based matching
  • Edit the tool_result content in place while preserving the tool_use_id
  • Delete only messages after the edited one (not the edited message itself)
  • Resume task loop using resumeAfterMessageEdit() instead of creating a new message

Testing

Manual testing confirmed that editing a tool result message now:

  1. Preserves the original tool_use_id
  2. Updates the content correctly
  3. Resumes the task loop without API errors
  4. Works with both text-only and image-containing edits

Important

Implements in-place editing for tool_result messages to preserve tool_use_id, ensuring task loop resumes correctly without creating new messages.

  • Behavior:
    • Implements in-place editing for tool_result messages in Task.ts and webviewMessageHandler.ts to preserve tool_use_id.
    • resumeAfterMessageEdit() in Task.ts resets task state and resumes task loop without creating a new user message.
    • Handles timestamp mismatch and position-based matching for tool_result messages in webviewMessageHandler.ts.
  • Task.ts:
    • Adds resumeAfterMessageEdit() to clear pending states, reset flags, update environment details, and ensure full context in next API call.
  • webviewMessageHandler.ts:
    • Detects tool_result messages and edits content in place while preserving tool_use_id.
    • Deletes only messages after the edited one and resumes task loop using resumeAfterMessageEdit().
  • Testing:
    • Manual testing confirms preservation of tool_use_id, correct content update, and task loop resumption without errors.

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

When editing a user_feedback message that corresponds to a tool_result in the API
conversation history, we now edit the message in place rather than creating a
new one. This preserves the tool_use_id, preventing the mismatch between
tool_call and tool_result that was causing errors with native tool protocols.

Changes:
- Add resumeAfterMessageEdit() method to Task.ts that properly resets state
  and resumes the task loop without creating a new user message
- Update webviewMessageHandler.ts to detect tool_result messages and edit
  them in place, preserving the tool_use_id while updating the content
- Handle the case where timestamps don't match between clineMessages and
  apiConversationHistory by using position-based matching
@daniel-lxs daniel-lxs requested review from cte, jr and mrubens as code owners December 4, 2025 18:25
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Dec 4, 2025
@roomote
Copy link
Contributor

roomote bot commented Dec 4, 2025

Rooviewer Clock   See task on Roo Cloud

All previously flagged issues have been resolved. No new issues found.

  • Remove empty debug if-block at lines 455-456
  • Fix potential bug where multiple tool_result blocks would all be overwritten with identical content
  • Add cleanup of orphaned condenseParent/truncationParent references when deleting subsequent messages
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 Dec 4, 2025
- Remove empty debug if-block (leftover code)
- Only update first tool_result block to avoid overwriting multiple blocks with identical content
- Add cleanup of orphaned condenseParent/truncationParent references using cleanupAfterTruncation
Copy link
Collaborator

@mrubens mrubens left a comment

Choose a reason for hiding this comment

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

This all just feels really complicated... is it because of choices we made in how we implemented message editing?

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:L This PR changes 100-499 lines, ignoring generated files.

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

4 participants