Skip to content

Conversation

@chrarnoldus
Copy link
Contributor

@chrarnoldus chrarnoldus commented Jul 7, 2025

Thanks @philipvas for this fix!


Important

Fixes browser_action_result messages appearing in chat by ensuring they are grouped into browser sessions in ChatRow.tsx and ChatView.tsx.

  • Behavior:
    • Fixes browser_action_result messages appearing in chat by ensuring they are grouped into browser sessions in ChatRow.tsx.
    • Adds logic in ChatView.tsx to handle browser_action_result and browser_action messages, ensuring they start a browser session if not already in one.
  • Components:
    • Updates ChatRowContent in ChatRow.tsx to display a warning if browser_action_result is not grouped.
    • Modifies ChatViewComponent in ChatView.tsx to handle browser session grouping logic for browser_action_result and browser_action messages.

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

@chrarnoldus chrarnoldus requested review from cte, jr and mrubens as code owners July 7, 2025 19:20
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Jul 7, 2025
// Check if the previous browser_action was a close action
const lastBrowserAction = [...currentGroup].reverse().find((m) => m.say === "browser_action")
if (lastBrowserAction) {
const browserAction = JSON.parse(lastBrowserAction.text || "{}") as ClineSayBrowserAction
Copy link
Contributor

Choose a reason for hiding this comment

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

JSON.parse is used here on lastBrowserAction.text without error handling. To avoid runtime exceptions on malformed JSON, consider wrapping this call in a try/catch (or use safeJsonParse).

This comment was generated because it violated a code review rule: irule_PTI8rjtnhwrWq6jS.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 7, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jul 7, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jul 7, 2025
@daniel-lxs
Copy link
Member

I noticed that when browser_action_result or browser_action messages appear outside of an existing browser session, the code creates a new empty browser session for them.

My concern is that this could lead to multiple empty or incomplete browser sessions being created if these orphaned messages appear frequently, which could impact performance and create a confusing UI with many partial browser sessions. Since ChatRow.tsx already shows a warning for ungrouped browser_action_result messages indicating it's a bug in the grouping logic, would it be better to handle these orphaned messages similarly - perhaps by skipping them or logging warnings rather than creating new sessions?

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jul 9, 2025
@github-project-automation github-project-automation bot moved this from PR [Changes Requested] to Done in Roo Code Roadmap Sep 23, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Sep 23, 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 - Changes Requested size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants