fix(notifications): broadcast live comments to non-member viewers - #1057
Draft
leahpeker wants to merge 3 commits into
Draft
fix(notifications): broadcast live comments to non-member viewers#1057leahpeker wants to merge 3 commits into
leahpeker wants to merge 3 commits into
Conversation
…sue 1043) Non-member/anonymous viewers of a public event page never received live comment updates, since the frontend only opened an SSE subscription from NotificationBell, which is authed-only. The backend broadcast (wildcard event_updates ping) already reached any connected viewer. - SseTicket.user becomes nullable; anonymous viewers mint a ticket too via a new /sse-ticket/ optional-auth path, rate-limited by IP when anon. - notification_stream/_sse_generator accept a null user_id: wildcard event_updated frames still deliver, personal notification frames don't. - EventCommentsCard opens its own SSE subscription when not authed, scoped to invalidate only that event's comment list.
Tighten multi-line comments/docstrings to single lines per repo comment style; drop redundant issue-number test docstrings.
- cap concurrent anonymous SSE connections (per-worker, cache-backed counter) so removing the auth requirement on ticket minting doesn't leave connection concurrency fully unbounded - reuse existing auth_or_ip_key for the ticket-mint rate limit instead of a hand-rolled key that skipped proxy-aware IP resolution - drop the redundant User row fetch in _consume_ticket (on_delete is CASCADE, so the DoesNotExist branch was unreachable); return the user_id string directly instead of routing through a sentinel - note in EventCommentsCard why its SSE subscription intentionally no-ops for authed users
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
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
Non-member (RSVP-token) viewers of a public event page could see comments but never saw new ones appear live, while logged-in members did. The frontend's only SSE subscription lived in
NotificationBell, which is authed-only ({isAuthed ? <NotificationBell /> : null}), so a non-member never opened a stream connection at all — even though the backend'sbroadcast_event_comment_update()already pings a wildcard ("*") channel that any connected viewer, member or not, is designed to receive.Root cause: frontend never attempted the SSE subscription for non-authed viewers. A secondary blocker: even if it tried,
POST /api/notifications/sse-ticket/(which mints the ticketEventSourceneeds, since it can't send an Authorization header) wasauth=gated_jwt— a non-member has no JWT, so ticket-minting itself would 401.Changes
SseTicket.useris now nullable;SseTicket.mint_anonymous()mints a ticket with no user attached.POST /sse-ticket/now usesauth=_optional_jwt(existing optional-JWT auth already used elsewhere for public endpoints) — mints a bound ticket for a logged-in caller, an anonymous one otherwise. Anonymous minting is rate-limited by IP.notification_stream/_sse_generator/_format_notify_for_useracceptuser_id: str | None. Wildcardevent_updatedbroadcasts still deliver to an anonymous connection; the personalnotificationchannel never matches one (an anonymous viewer has no personal notifications to receive).EventCommentsCardnow opens its own SSE subscription when the viewer isn't authed, invalidating only that event'sevent-commentsquery key onevent_updated. Authed members are unaffected — they still get this viaNotificationBell's existing subscription, so no duplicate connection is opened for them.Closes #1043
Test plan
make agent-typecheck(backendty check) — passmake agent-lint(backendruff) — passmake agent-frontend-typecheck(tsc -b) — passmake agent-frontend-lint(eslint) — passbackend/tests/test_sse.pyrun directly against a real per-worktree Postgres DB (SSE/pg_notify needs real Postgres, not sqlite) — 15/15 pass, including new tests for anonymous ticket minting and wildcard delivery to an anonymous connection--reload— the reload file-watcher was found to interrupt long-lived SSE connections in dev, unrelated to this fix): one browser context authenticated as a member, a separate context logged out and RSVP'd anonymously via the public RSVP form to unlock the comments card as a non-member. Confirmed via network trace the anonymous viewer'sPOST /sse-ticket/andGET /stream/?ticket=...both succeeded (200), then posted a comment as the member — the anonymous viewer's comment list updated live with no manual reload or refresh.