Add WebSocket notification system for real-time BBS and mail mentions#905
Add WebSocket notification system for real-time BBS and mail mentions#905
Conversation
Replaces focus-based polling with persistent WebSocket connection to the backend notification service. Notifications now arrive within 1-2 seconds with automatic reconnection and exponential backoff. Maintains polling as fallback for reliability when WebSocket is unavailable. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fa3423595
ℹ️ 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".
There was a problem hiding this comment.
4 issues found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="XMOJ.user.js">
<violation number="1" location="XMOJ.user.js:628">
P1: Race condition: `ReconnectNotificationSocket` schedules a `setTimeout` but never stores the timer ID. When the `visibilitychange` handler also calls `ConnectNotificationSocket()`, both can fire, creating duplicate WebSocket connections. Store the timer ID in a module-level variable and clear it before creating a new connection.</violation>
<violation number="2" location="XMOJ.user.js:667">
P2: Matching mail mentions only by `FromUserID` is insufficient when the same sender has multiple unread mentions. This loop will match the first (potentially older) mention in the list rather than the newly pushed one, causing the wrong `MentionID` to be marked as read on click while the actual new mention remains unread. Consider matching by a unique identifier like `MentionID` from the notification payload instead.</violation>
<violation number="3" location="XMOJ.user.js:709">
P1: XSS vulnerability: `mention.PostTitle` is inserted directly via `innerHTML` without sanitization. If a malicious user crafts a post title containing `<script>` or event handler attributes, it will execute in the context of the victim's browser session. Use `escapeHTML()` or `textContent` to safely render user-supplied data.</violation>
<violation number="4" location="XMOJ.user.js:770">
P2: Bug: `innerHTML +=` after `appendChild(ToastUser)` destroys the DOM node created by the async `GetUsernameHTML()`. When you use `innerHTML +=`, the browser serializes existing children to HTML, appends the string, and re-parses everything—destroying event listeners and breaking async updates in progress. Use `appendChild(document.createTextNode(...))` instead.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
The backend now includes all required fields (PostTitle, PageNumber, MentionID) in WebSocket notifications, eliminating the need for additional API calls to fetch mention details. This reduces latency and server load for real-time notifications. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…issues 1. Race condition (P1): Store reconnect timer ID to prevent duplicate WebSocket connections when visibilitychange handler and delayed reconnect fire simultaneously 2. XSS vulnerability (P1): Sanitize user-supplied PostTitle with escapeHTML() before rendering to prevent script injection attacks 3. DOM destruction (P2): Replace innerHTML += with appendChild to preserve async GetUsernameHTML() results in mail mention toasts Note: Mail mention matching issue (violation #2) was already resolved by previous commit that passes notification.data directly Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc2099ddcc
ℹ️ 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".
There was a problem hiding this comment.
2 issues found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="XMOJ.user.js">
<violation number="1" location="XMOJ.user.js:793">
P2: Race condition in `PollNotifications`: when both BBSPopup and MessagePopup are enabled, the BBS callback unconditionally clears `ToastContainer.innerHTML`, but if the mail response arrives first, its toasts will be destroyed when the BBS response arrives later. Consider clearing the container once before both requests, or chaining the requests sequentially.</violation>
<violation number="2" location="XMOJ.user.js:1540">
P2: The `visibilitychange` handler doesn't account for `WebSocket.CONNECTING` state. If the socket is already connecting, this will trigger another `ConnectNotificationSocket()` call, creating a duplicate connection. Add a check for `CONNECTING` state.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="Update.json">
<violation number="1" location="Update.json:3293">
P2: Avoid manually editing Update.json version metadata; the UpdateVersion workflow owns these timestamps and will overwrite or conflict with hand-edits. Revert this change and let the automation update the entry.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…ns[bot] This prevents infinite loops where the bot commits version updates, which triggers the workflow again, causing another commit. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replaces focus-based polling with persistent WebSocket connection to the backend notification service. Notifications now arrive within 1-2 seconds with automatic reconnection and exponential backoff. Maintains polling as fallback for reliability when WebSocket is unavailable.
What does this PR aim to accomplish?:
This PR introduces a WebSocket-based real-time notification system to replace the existing focus-based polling mechanism for BBS mentions and mail notifications.
This closes #335.
Related PRs:
Key Goals:
Benefits:
How does this PR accomplish the above?:
Implementation Details:
WebSocket Client (
ws://127.0.0.1:8787for debug,wss://api.xmoj-bbs.me/ws/notificationsfor production)visibilitychangeevent when tab becomes visibleReal-time Notification Handling
bbs_mentionandmail_mentionpayloads from backendescapeHTML()to prevent XSS attacksappendChild()in mail toastsFallback Mechanisms
Bug Fixes
Changelog:
Files Modified:
XMOJ.user.js: +306/-128 lines (WebSocket implementation, notification handlers)Update.json: Version bump and release notespackage.json: Version update to 2.7.3By submitting this pull request, I confirm the following:
git rebase)Summary by cubic
Adds a persistent WebSocket for real-time BBS and mail mention notifications with enriched payloads for instant toasts. Ships in v2.7.3 (prerelease) with auto-reconnect, 30s keepalive pings, and guardrails to prevent XSS, duplicate connections, and duplicate toasts.
New Features
Bug Fixes
Written for commit 25ab8d7. Summary will update on new commits.