Skip to content

Conversation

@cte
Copy link
Collaborator

@cte cte commented Nov 10, 2025

Test plan:

  • Basic approval
    • Read
    • Write
    • Browser
    • Retry
    • MCP
    • Mode
    • Subtasks
    • Execute
    • Question
    • Update TODOs
  • Automatically denied command
  • Automatically select question response after n seconds
  • Auto-approval max count
  • Auto-approval max cost
  • Sounds

Important

Refactor auto-approval logic by moving it from ChatView to Task class, centralizing command validation and tool action handling.

  • Behavior:
    • Move auto-approval logic from ChatView to Task class.
    • Task class handles auto-approval for commands, tools, and other actions.
    • ChatView focuses on UI interactions, delegating logic to Task.
  • Command Validation:
    • Remove command validation logic from ChatView.
    • Implement command validation in Task class.
  • Tests:
    • Update tests in ChatView.spec.tsx to reflect new logic.
    • Add tests for Task class to ensure correct auto-approval behavior.
  • Misc:
    • Remove unused imports and code from ChatView.
    • Simplify ChatView component by removing logic now handled by Task.

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

@cte cte requested review from jr and mrubens as code owners November 10, 2025 21:52
@dosubot dosubot bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label Nov 10, 2025
@roomote
Copy link
Contributor

roomote bot commented Nov 10, 2025

Rooviewer Clock   See task on Roo Cloud

Reviewed commit 62cc020. Clean test cleanup that properly removes obsolete tests for auto-approval logic that was moved from ChatView to Task. No new issues found.

  • Missing error handling in isMcpToolAlwaysAllowed - Fixed by moving JSON parsing to caller with try-catch
Previous reviews

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

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 10, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. lgtm This PR has been approved by a maintainer size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants