Fix Notification Tool and add .idea/ to .gitignore#121
Fix Notification Tool and add .idea/ to .gitignore#121Jeomon merged 2 commits intoCursorTouch:mainfrom
.idea/ to .gitignore#121Conversation
Use Windows PowerShell 5.1 explicitly for WinRT toast APIs (unsupported in pwsh 7+), and require a valid Application User Model ID from the caller instead of hardcoding an unregistered one.
There was a problem hiding this comment.
Pull request overview
This PR fixes Windows toast notifications in the Notification MCP tool by ensuring the underlying PowerShell host and AppUserModelID are valid, and also updates repo hygiene by ignoring JetBrains project files.
Changes:
- Update the
NotificationMCP tool and desktop service to require a caller-providedapp_id(AppUserModelID) instead of a hardcoded value. - Extend
Desktop.execute_commandwith an optionalshelloverride and force Windows PowerShell 5.1 for toast notifications (WinRT compatibility). - Add
.idea/to.gitignore.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/windows_mcp/tools/notification.py |
Adds app_id parameter and richer argument metadata for the Notification tool. |
src/windows_mcp/desktop/service.py |
Adds shell override to PowerShell execution and fixes toast creation by using a valid AppUserModelID + PS 5.1. |
.gitignore |
Ignores JetBrains .idea/ directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| app_id: Annotated[ | ||
| str, | ||
| Field( | ||
| description="The valid Application User Model ID of the toast notification. Required to display the notification in a specific app.", | ||
| ), |
There was a problem hiding this comment.
app_id is now a required parameter, but the tool-level description (in the @mcp.tool(...) decorator above) still only mentions a title/message. Please update that description to document the new required AppUserModelID so MCP clients know what to provide.
| description="The valid Application User Model ID of the toast notification. Required to display the notification in a specific app.", | ||
| ), | ||
| ], | ||
| ctx: Context = None, | ||
| ) -> str: |
There was a problem hiding this comment.
Consider adding validation constraints for app_id (e.g., min_length=1 and possibly stripping whitespace) before calling into the desktop service. As-is, an empty string is accepted and will likely recreate the same “silent no-toast” failure mode you’re trying to eliminate.
| description="The valid Application User Model ID of the toast notification. Required to display the notification in a specific app.", | |
| ), | |
| ], | |
| ctx: Context = None, | |
| ) -> str: | |
| description="The valid Application User Model ID of the toast notification. Required to display the notification in a specific app.", | |
| min_length=1, | |
| ), | |
| ], | |
| ctx: Context = None, | |
| ) -> str: | |
| # Normalize and validate app_id to avoid silent no-toast failures. | |
| app_id = app_id.strip() | |
| if not app_id: | |
| return "Error sending notification: app_id must be a non-empty string." |
| def send_notification(self, title: str, message: str, app_id: str) -> str: | ||
| """Send a Windows toast notification with a title and message. | ||
|
|
||
| Args: | ||
| title: The title of the notification. | ||
| message: The message of the notification. | ||
| app_id: The valid Application User Model ID of the toast notification. | ||
| Required to display the notification in a specific app. | ||
|
|
There was a problem hiding this comment.
send_notification now requires app_id, but there’s no guard against app_id being empty/whitespace. Adding an early validation (and returning a clear error) would prevent running PowerShell with an invalid AppUserModelID and avoid reintroducing silent failures.
| "$notifier.Show($toast)" | ||
| ) | ||
| response, status = self.execute_command(ps_script) | ||
| # Use Windows PowerShell (5.1) explicitly because the WinRT toast APIs are not available in PowerShell 7+ (pwsh). | ||
| response, status = self.execute_command(ps_script, shell="powershell") |
There was a problem hiding this comment.
There are existing unit tests that mock Desktop.execute_command (e.g., registry tests), but this new notification behavior isn’t covered. Please add a unit test that calls send_notification(...) and asserts it invokes execute_command with shell="powershell" and that the generated script includes the provided app_id (quoted) to prevent regressions back to pwsh/invalid IDs.
|
Thanks for that, I was working on another project, which is called Operator-Use Please try out. |
|
Thanks for sharing! This looks similar to openclaw, which is exciting. I'll definitely give it a try on my Windows machine. Appreciate your hard work on this! |
Problem of
NotificationToolThe Notification tool was silently failing — no toast notification was ever shown on the desktop. Two root causes were found during testing:
PowerShell 7+ (pwsh) does not support WinRT toast APIs.
The
send_notificationmethod previously used whichever PowerShell was available viaexecute_command, which defaults topwshwhen installed. However, the[Windows.UI.Notifications.ToastNotificationManager]WinRT class is only accessible in Windows PowerShell 5.1, not in PowerShell 7+."Windows MCP"is not a valid Application User Model ID.The
CreateToastNotifier()call was hardcoded with"Windows MCP", which is not a registered AppUserModelID. Windows requires a valid, registered ID to route and display toast notifications.Changes
Explicitly use
powershell(5.1) insend_notification: Added ashellparameter toexecute_commandand passedshell="powershell"fromsend_notificationto ensure the WinRT toast APIs are always available.Require a valid App ID from the MCP client: Replaced the hardcoded
"Windows MCP"string with anapp_idparameter on bothsend_notificationand theNotificationMCP tool, so the caller must provide a registered Application User Model ID (e.g.,Microsoft.Windows.Explorer).Add
.idea/to.gitignore: Exclude JetBrains IDE configuration files from version control.