Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Jun 22, 2025

Summary

Fixes #4888 by ensuring all pending api_req_started messages are properly finalized when a task completes, preventing UI spinners from persisting in task history.

Changes

Core Implementation

  • Added finish() method to Task.ts: Finds all api_req_started messages without cost or cancelReason and sets their cost to 0
  • Updated attemptCompletionTool.ts: Calls finish() before emitting taskCompleted to ensure all API requests are finalized

Testing

  • Added comprehensive unit tests in Task.spec.ts:
    • Test that all pending api_req_started messages get finalized with cost: 0
    • Test that no unnecessary saves occur when no updates are needed
    • Test edge case with no api_req_started messages
    • Verification that ALL api_req_started messages have either cost or cancelReason after completion

Problem Solved

The issue occurred because when tasks completed, some api_req_started messages in the conversation history lacked both cost and cancelReason values. The UI component (ChatRow.tsx) shows a spinner for ANY api_req_started message missing these values, causing spinners to persist in completed task history.

Solution Approach

While operations run sequentially and theoretically only the last api_req_started could be unfinished, the UI checks ALL messages for the spinner condition. Rather than modifying the UI logic (which would require additional complexity), this solution ensures backend compatibility by finalizing all unfinished messages.

Testing

✅ All existing tests pass
✅ New unit tests cover the finish() method functionality
✅ Linting and type checking pass

Verification

After this fix:

  • Completed tasks will have all api_req_started messages properly finalized
  • No spinners will persist in task history
  • UI behavior remains unchanged for active/streaming tasks

Important

Adds finish() method to Task to finalize pending api_req_started messages, ensuring UI spinners do not persist.

  • Behavior:
    • Adds finish() method to Task class in Task.ts to finalize api_req_started messages with no cost or cancelReason by setting cost to 0.
    • Updates attemptCompletionTool in attemptCompletionTool.ts to call finish() before emitting taskCompleted.
  • Testing:
    • Adds unit tests in Task.spec.ts to verify finish() method finalizes messages correctly, handles cases with no updates, and processes scenarios with no api_req_started messages.

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

- Add finish() method to Task class that finds all api_req_started messages
  without cost or cancelReason and sets their cost to 0
- Call finish() before emitting taskCompleted in attemptCompletionTool
- Add comprehensive tests for the finish() method
- Prevents UI spinners from persisting in completed task history
@daniel-lxs daniel-lxs requested review from cte, jr and mrubens as code owners June 22, 2025 16:13
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jun 22, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 22, 2025
@dosubot dosubot bot added the bug Something isn't working label Jun 22, 2025
@daniel-lxs daniel-lxs closed this Jun 22, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Jun 22, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 22, 2025
@daniel-lxs daniel-lxs reopened this Jun 22, 2025
@github-project-automation github-project-automation bot moved this from Done to Triage in Roo Code Roadmap Jun 22, 2025
@github-project-automation github-project-automation bot moved this from Done to New in Roo Code Roadmap Jun 22, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 22, 2025
@daniel-lxs
Copy link
Member Author

@hannesrudolph Can you check if this PR solves the issue with a task that has any spinner?

I think this solution will only work with new tasks created since this checks the task when saving for any message that still has a spinner.

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Draft / In Progress] in Roo Code Roadmap Jun 22, 2025
@github-project-automation github-project-automation bot moved this from PR [Draft / In Progress] to Done in Roo Code Roadmap Jul 7, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 7, 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 PR - Draft / In Progress size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Spinner persists in completed tasks UI - intermittent state management bug

3 participants