🤖 Add Completion Notifications (Desktop + Web + Mobile PWA) #445
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add NotificationService with VAPID key generation and push support - Desktop: Native Electron notifications with click-to-focus - Web: Browser notifications when tab is backgrounded - Mobile PWA: Push notifications when app is closed - Command palette toggle: Enable/Disable Completion Notifications - Notifications trigger on stream-end if enabled and document hidden - Service worker handles push events and notification clicks - Push subscriptions managed per-workspace with auto-cleanup Implementation: - Backend: NotificationService generates VAPID keys, stores subscriptions - IPC: 4 new channels (subscribe, unsubscribe, getVapidKey, send) - Frontend: Push subscription utilities with permission handling - Service Worker: Push event handlers with notification display - WorkspaceStore: Checks preference and triggers on stream-end - Command Palette: Settings section with notification toggle Generated with `cmux`
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Fix subscription filtering to preserve correct indexes - Move notification trigger to server-side for PWA support - Remove frontend notification logic (now server-side only)
…ed IPC - Move desktop notification logic into NotificationService (was broken) - Desktop notifications now show properly via server-side trigger - Remove unused NOTIFICATION_SEND IPC channel and handler - Remove send() method from notification API (not needed) - Use dynamic import instead of require for electron module - Update all comments to reflect server-side architecture
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Adds optional notifications when agent streams complete. Works across:
Implementation
Backend (NotificationService)
IPC Layer
Added 4 new channels:
NOTIFICATION_SUBSCRIBE_PUSH- Register push subscriptionNOTIFICATION_UNSUBSCRIBE_PUSH- Remove push subscriptionNOTIFICATION_GET_VAPID_KEY- Get public VAPID keyNOTIFICATION_SEND- Trigger notificationFrontend
src/utils/notifications/pushSubscription.ts)public/service-worker.js)Trigger Logic
stream-endKey Design Decisions
Backend vs Frontend Trigger
Frontend triggers notifications after checking localStorage preference. Keeps backend agnostic to UI preferences.
Push Subscription Storage
In-memory Map (not persisted). Frontend re-subscribes on app load if permission already granted. Avoids stale subscriptions.
Global vs Per-Workspace Subscriptions
Global subscriptions (not per-workspace). Simpler UX - one device receives all completion notifications.
VAPID Key Storage
Stored in
~/.cmux/vapid.json. Generated once on first notification enable. Persists across restarts.Testing
Manual testing checklist:
Files Changed
New Files
src/services/NotificationService.ts(151 lines) - VAPID keys, push notificationssrc/utils/notifications/pushSubscription.ts(99 lines) - Push subscription managementsrc/types/notification.ts(32 lines) - Type definitionsModified Files
public/service-worker.js(+43 lines) - Push event handlerssrc/constants/storage.ts(+6 lines) - Notification preference keysrc/constants/ipc-constants.ts(+4 lines) - IPC channelssrc/services/ipcMain.ts(+62 lines) - IPC handlers, NotificationService integrationsrc/stores/WorkspaceStore.ts(+13 lines) - Trigger logicsrc/components/CommandPalette.tsx- Command palette toggle (via sources.ts)src/utils/commands/sources.ts(+40 lines) - Settings section with togglesrc/browser/api.ts(+9 lines) - Web API implementationsrc/preload.ts(+9 lines) - Preload API exposuresrc/types/ipc.ts(+6 lines) - API type definitionssrc/App.stories.tsx(+6 lines) - Mock API for Storybooksrc/main-server.ts(+1 line) - Pass isDesktop=falseTotal: ~370 lines added
Future Enhancements
Generated with
cmux