-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat: add push notifications for approval requests #8723
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add NotificationType enum and NotificationEvent interface - Implement CloudAPI.notifyEvent() for backend communication - Add CloudService.sendNotification() with user preference validation - Integrate notifications in Task.ask() for tool/command/browser/MCP approvals - Add notifications in askFollowupQuestionTool for follow-up questions - Add notifications in attemptCompletionTool for task completion - Add user settings configuration for enabling/disabling notification types - Ensure non-blocking notifications that don't interrupt main functionality Addresses #8721
Review SummaryI've reviewed the pull request and found 1 issue that should be addressed: Issues to Address
Overall AssessmentThe implementation looks solid overall. The notification system is well-integrated with proper error handling and non-blocking behavior. Once the metadata consistency issue is addressed, this PR will be ready to merge. |
| await cloudService | ||
| .sendNotification({ | ||
| timestamp: Date.now(), | ||
| type: notificationType, | ||
| title: "Approval Required", | ||
| message: notificationMessage, | ||
| taskId: this.taskId, | ||
| metadata: { | ||
| askType: type, | ||
| content: text, | ||
| mode: taskMode, | ||
| isProtected, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistency in notification metadata: The approval notifications in Task.ts don't populate userId and organizationId fields, while the follow-up question notification in askFollowupQuestionTool.ts does (lines 85-94). For consistency and to ensure proper routing/filtering on the backend, all notification types should populate these fields. Consider adding:
const userInfo = CloudService.instance.getUserInfo()
// ... then in the notification object:
userId: userInfo?.id,
organizationId: userInfo?.organizationId,
Summary
This PR addresses Issue #8721 by implementing push notifications for approval scenarios in Roo Code. Users will now receive notifications when Roo needs approval for actions like tool usage, command execution, and follow-up questions.
Changes
Core Implementation
NotificationTypeenum andNotificationEventinterface topackages/types/src/cloud.tsCloudAPI.notifyEvent()method for backend communicationCloudService.sendNotification()with user preference validationUserSettingsConfigfor granular controlIntegration Points
Features
Testing
Related Issue
Closes #8721
Notes
The implementation ensures that notification failures are non-blocking and won't interrupt Roo's main functionality. Users can enable/disable specific notification types through their settings configuration.
Feedback and suggestions are welcome!
Important
Adds push notifications for task-related events, including approvals, follow-up questions, and completions, with user preference controls and non-blocking error handling.
Task.ts,askFollowupQuestionTool.ts, andattemptCompletionTool.ts.UserSettingsConfigcontrol notification types.NotificationTypeenum andNotificationEventinterface incloud.ts.CloudAPI.notifyEvent()inCloudAPI.tsfor backend communication.CloudService.sendNotification()inCloudService.tswith user preference validation.Task.ask(): Sends notifications for tool, command, browser_action_launch, and use_mcp_server approvals.askFollowupQuestionTool: Triggers notifications for follow-up questions.attemptCompletionTool: Sends notifications for task completions.This description was created by
for 41fbb2e. You can customize this summary. It will automatically update as commits are pushed.