Skip to content

Drop chat notifications for an account that is not signed in - #29804

Merged
chrisnojima merged 2 commits into
masterfrom
nojima/HOTPOT-chat-notify-uid-cleanup
Oct 20, 2026
Merged

chrisnojima merged 2 commits into
masterfrom
nojima/HOTPOT-chat-notify-uid-cleanup

Conversation

@chrisnojima

Copy link
Copy Markdown
Contributor

What

Every chat.1.NotifyChat.* notification now names the account it was made for (#29708). The GUI uses that: _onEngineIncoming drops any NotifyChat whose uid is not the signed-in account's, before the chat notification router or any engine action listener sees it. While no account is signed in (current uid '', from the start of a switch or a logout until the next bootstrap) all of them are dropped. chat.1.chatUi.* calls carry no uid and pass through untouched.

Why

Login and logout are not serialized against chat notifications, so through an account switch the previous account's can still arrive. Until now the GUI ignored the uid on all but two of them, so a late notification could write the previous account's metas, typing, participants or archive progress into the next account's stores, or into the next account's thread for a team channel both accounts share.

Changes

  • chat/notification-router.tsx: isChatNotificationNotForSignedInAccount, the predicate, covering every NotifyChat type (including ones the router does not route: archive progress/complete, welcome message, team retention).
  • constants/init/shared.tsx: _onEngineIncoming returns early on it.
  • chat/inbox/selection.tsx: conversationGoneForUser is gone; the router calls conversationGone for left/reset conversations, since the central filter already checks the account.
  • chat/inbox/layout-state.tsx: the inbox layout reset also clears waitingKeyChatInboxSyncStarted. That key is held by sync notifications rather than a call, and the leaving account's closing ChatInboxSynced is now dropped, so nothing else would release it and the inbox would show loading.
  • chat/readme.md: describes the filter.

Judgment calls

  • Strict while nobody is signed in. Dropping everything while the current uid is '' also drops the new account's earliest notifications (before its bootstrap lands). The inbox does a full refresh once the session is ready, and letting notifications through in that window would let the previous account's through too.
  • What this does not retire. The account-keyed thread provider, thread retirement (isRetired, makeThreadChatRpc) and the engine's account-generation checks stay. They guard screens kept mounted through a switch from starting calls as the next account and late call results from the previous one, which a notification filter does not cover. The per-thread notification guard (useConversationThreadNotifications) also stays: it stops the new account's notifications for a shared channel reaching the old account's still-mounted thread, and those carry the current uid.
  • Requires a service with include uid on all local rpcs #29708. Against an older service, the six notifications that had no uid (typing, participants, identify, archive progress/complete, welcome message) would all be dropped.

Tests

chat/notification-effects.test.tsx, through the real _onEngineIncoming and engine listener bus:

  • another account's typing update and archive progress reach neither the router nor a listener
  • the signed-in account's do
  • while no account is signed in every one is dropped, an empty uid included
  • chatUi calls pass whoever is signed in
  • a sync the leaving account started does not keep the next account's inbox loading

Each was checked to fail with its fix removed. The suite's notification builder now stamps the signed-in uid on NotifyChat params. auto-select-after-switch.test.tsx drops its other-account step, which tested the removed per-handler check by calling the router directly.

yarn lint:all clean, yarn test:unit passing.

Every NotifyChat notification now names the account it was made for
(#29708). Login and logout are not serialized against them, so an account
switch can deliver the previous account's late. _onEngineIncoming drops any
NotifyChat whose uid is not the signed-in account's, and all of them while
no account is signed in, before the chat router or any engine action
listener sees it. chatUi calls carry no uid and pass through.

The per-handler uid check for left/reset conversations is now redundant and
removed.
The leaving account's ChatInboxSynced is now dropped as another account's, so
the inbox layout reset clears the sync key itself. Rename the filter to say
it also drops everything while no account is signed in.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No blocking issues remain; regression coverage addresses account filtering, chatUi passthrough, and sync cleanup.

No open issues 🎉

What changed in this PR

Prevents late chat notifications from a signed-out account from reaching the GUI’s chat stores or engine listeners.

Changes:

  • Filters NotifyChat events by signed-in UID while preserving chatUi calls.
  • Clears inbox sync waiting state during account changes.
  • Updates regression tests and documents the filtering behavior.
File Description
shared/​constants/​init/​shared.tsx Filters notifications before routing or listener delivery.
shared/​chat/​readme.md Documents account filtering.
shared/​chat/​notification-router.tsx Adds the UID predicate and simplifies conversation removal.
shared/​chat/​notification-effects.test.tsx Tests filtering, listener delivery, and sync cleanup.
shared/​chat/​inbox/​selection.tsx Removes the redundant per-handler UID check.
shared/​chat/​inbox/​layout-state.tsx Clears sync waiting state on reset.
shared/​chat/​inbox/​auto-select-after-switch.test.tsx Updates selection coverage for centralized filtering.

Review effort: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants