Private
Public Access
Fix desktop messages not appearing live until refocus (#59)
Desktop mode's focus gating (from #49) made losing OS focus send "leave" for every open room, which stopped live message delivery to that room, not just notification eligibility -- so a message wouldn't render until the room was manually left and rejoined. Room join/leave is now gated on visibility alone, matching the browser; notification eligibility gets its own separate signal (a "focus"/"blur" WS frame tracked by a new Redis-backed FocusPresence), so a connected-but-unfocused desktop member still gets notified without losing live delivery. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -27,6 +27,7 @@ from app.routers import (
|
||||
from app.ws.broadcaster import Broadcaster
|
||||
from app.ws.chat import router as ws_router
|
||||
from app.ws.connection_manager import ConnectionManager
|
||||
from app.ws.focus_presence import FocusPresence
|
||||
from app.ws.global_presence import GlobalPresence
|
||||
from app.ws.presence import Presence
|
||||
|
||||
@@ -82,6 +83,7 @@ def create_app() -> FastAPI:
|
||||
app.state.redis = Redis.from_url(settings.redis_url, decode_responses=True)
|
||||
app.state.presence = Presence(app.state.redis)
|
||||
app.state.global_presence = GlobalPresence(app.state.redis)
|
||||
app.state.focus_presence = FocusPresence(app.state.redis)
|
||||
app.state.broadcaster = Broadcaster(app.state.redis, app.state.connection_manager)
|
||||
|
||||
app.include_router(health.router)
|
||||
|
||||
@@ -25,4 +25,5 @@ async def incoming_webhook_endpoint(
|
||||
|
||||
broadcaster = request.app.state.broadcaster
|
||||
presence = request.app.state.presence
|
||||
await broadcast_new_message(db, broadcaster, presence, room.id, message, sender)
|
||||
focus_presence = request.app.state.focus_presence
|
||||
await broadcast_new_message(db, broadcaster, presence, focus_presence, room.id, message, sender)
|
||||
|
||||
@@ -10,6 +10,7 @@ from app.services.link_preview_service import fetch_and_broadcast_link_preview
|
||||
from app.services.push_service import send_push_to_user
|
||||
from app.services.webhook_service import dispatch_event
|
||||
from app.ws.broadcaster import Broadcaster
|
||||
from app.ws.focus_presence import FocusPresence
|
||||
from app.ws.presence import Presence
|
||||
|
||||
|
||||
@@ -17,6 +18,7 @@ async def _notify_offline_members(
|
||||
db: AsyncSession,
|
||||
broadcaster: Broadcaster,
|
||||
presence: Presence,
|
||||
focus_presence: FocusPresence,
|
||||
room_id: uuid.UUID,
|
||||
sender: User,
|
||||
message: Message,
|
||||
@@ -25,12 +27,28 @@ async def _notify_offline_members(
|
||||
select(RoomMembership.user_id).where(RoomMembership.room_id == room_id)
|
||||
)
|
||||
member_ids = {row[0] for row in result.all()}
|
||||
connected_ids = await presence.connected_user_ids(room_id)
|
||||
# Subtract the sender explicitly rather than relying on them being
|
||||
# "connected" (true for the WS path, since they just sent this over an
|
||||
# active connection -- not true for the incoming-webhook REST path,
|
||||
# which has no WS connection for the attributed sender at all).
|
||||
offline_ids = member_ids - await presence.connected_user_ids(room_id) - {sender.id}
|
||||
if not offline_ids:
|
||||
offline_ids = member_ids - connected_ids - {sender.id}
|
||||
|
||||
# #59: a desktop-mode member can be *connected* to this room's channel
|
||||
# (it's open on screen, live messages are rendering) while their window
|
||||
# sits unfocused behind something else -- still exactly the situation a
|
||||
# desktop notification should fire for, same as #49's original intent.
|
||||
# This used to be handled by the client faking "offline" (leaving the
|
||||
# room's channel on blur), which also silently stopped live delivery to
|
||||
# that room; FocusPresence is a separate signal so notification
|
||||
# eligibility no longer has to ride on room-connection state at all.
|
||||
connected_but_unfocused_ids = {
|
||||
user_id
|
||||
for user_id in connected_ids - {sender.id}
|
||||
if await focus_presence.is_unfocused(user_id)
|
||||
}
|
||||
notify_ids = offline_ids | connected_but_unfocused_ids
|
||||
if not notify_ids:
|
||||
return
|
||||
|
||||
result = await db.execute(
|
||||
@@ -38,12 +56,10 @@ async def _notify_offline_members(
|
||||
)
|
||||
mentioned_ids = {row[0] for row in result.all()}
|
||||
|
||||
# This is also exactly the right audience for "give this room an unread
|
||||
# dot": presence.connected_user_ids(room_id) means "has this room's
|
||||
# channel joined right now" -- which the client only does while the tab
|
||||
# is genuinely foregrounded (see useChatSocket.ts's visibility-gated
|
||||
# join/leave), so a backgrounded-but-open room correctly lands here too,
|
||||
# not just rooms that aren't open at all.
|
||||
# Unread-dot audience stays exactly offline_ids, not notify_ids: a
|
||||
# connected-but-unfocused member still has the room open and rendering
|
||||
# on screen right now, so it isn't actually "unread" for them the way a
|
||||
# room they haven't got open at all is.
|
||||
for user_id in offline_ids:
|
||||
await broadcaster.publish_to_user(
|
||||
user_id,
|
||||
@@ -56,7 +72,7 @@ async def _notify_offline_members(
|
||||
|
||||
room = await db.get(Room, room_id)
|
||||
title = f"#{room.name}" if room else "New message"
|
||||
for user_id in offline_ids:
|
||||
for user_id in notify_ids:
|
||||
mentioned = user_id in mentioned_ids
|
||||
if message.content:
|
||||
prefix = f"{sender.username} mentioned you: " if mentioned else f"{sender.username}: "
|
||||
@@ -131,6 +147,7 @@ async def broadcast_new_message(
|
||||
db: AsyncSession,
|
||||
broadcaster: Broadcaster,
|
||||
presence: Presence,
|
||||
focus_presence: FocusPresence,
|
||||
room_id: uuid.UUID,
|
||||
message: Message,
|
||||
sender: User,
|
||||
@@ -161,7 +178,7 @@ async def broadcast_new_message(
|
||||
await broadcaster.publish_to_user(
|
||||
unhidden_user_id, {"type": "room_added", "room_id": str(room_id)}
|
||||
)
|
||||
await _notify_offline_members(db, broadcaster, presence, room_id, sender, message)
|
||||
await _notify_offline_members(db, broadcaster, presence, focus_presence, room_id, sender, message)
|
||||
await dispatch_event(db, "message.created", room_id, payload)
|
||||
_maybe_fetch_link_preview(broadcaster, room_id, message)
|
||||
|
||||
|
||||
+31
-1
@@ -36,6 +36,7 @@ class ClientEnvelope(BaseModel):
|
||||
file_id: uuid.UUID | None = None
|
||||
message_id: uuid.UUID | None = None
|
||||
emoji: str | None = None
|
||||
focused: bool | None = None
|
||||
|
||||
|
||||
async def _is_room_member(db: AsyncSession, room_id: uuid.UUID, user_id: uuid.UUID) -> bool:
|
||||
@@ -79,8 +80,14 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db)
|
||||
manager = websocket.app.state.connection_manager
|
||||
presence = websocket.app.state.presence
|
||||
global_presence = websocket.app.state.global_presence
|
||||
focus_presence = websocket.app.state.focus_presence
|
||||
broadcaster = websocket.app.state.broadcaster
|
||||
joined_rooms: set[uuid.UUID] = set()
|
||||
# Tracks this connection's last-reported focus state (see the "focus"
|
||||
# envelope below) so the disconnect cleanup can release FocusPresence's
|
||||
# refcount if the socket closes while still blurred -- mirroring how
|
||||
# joined_rooms tracks per-connection room membership for its own cleanup.
|
||||
is_blurred = False
|
||||
manager.register_user(user.id, websocket)
|
||||
# Only broadcast on a genuine offline->online transition (this user's
|
||||
# first open connection), not for every extra tab -- broadcast_member_
|
||||
@@ -133,6 +140,25 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db)
|
||||
await presence.leave(envelope.room_id, user.id)
|
||||
joined_rooms.discard(envelope.room_id)
|
||||
|
||||
elif envelope.type == "focus":
|
||||
# Sent only by the desktop client (#59), independent of
|
||||
# room join/leave -- see FocusPresence's docstring for
|
||||
# why widening desktop_notification eligibility this
|
||||
# way no longer needs to touch live room delivery at
|
||||
# all, unlike the "leave the room's channel on blur"
|
||||
# approach this replaced.
|
||||
if envelope.focused is None:
|
||||
await websocket.send_json({"type": "error", "detail": "focused required"})
|
||||
continue
|
||||
if envelope.focused:
|
||||
if is_blurred:
|
||||
await focus_presence.mark_focused(user.id)
|
||||
is_blurred = False
|
||||
else:
|
||||
if not is_blurred:
|
||||
await focus_presence.mark_blurred(user.id)
|
||||
is_blurred = True
|
||||
|
||||
elif envelope.type == "message":
|
||||
if envelope.room_id is None or (
|
||||
not envelope.content
|
||||
@@ -185,7 +211,9 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db)
|
||||
# incoming-webhook path also calls it, and a webhook's
|
||||
# attributed sender may not actually be watching.
|
||||
await mark_room_read(db, envelope.room_id, user.id)
|
||||
await broadcast_new_message(db, broadcaster, presence, envelope.room_id, message, user)
|
||||
await broadcast_new_message(
|
||||
db, broadcaster, presence, focus_presence, envelope.room_id, message, user
|
||||
)
|
||||
|
||||
elif envelope.type == "edit":
|
||||
if envelope.room_id is None or envelope.message_id is None or not envelope.content:
|
||||
@@ -268,5 +296,7 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db)
|
||||
manager.unregister_user(user.id, websocket)
|
||||
for room_id in joined_rooms:
|
||||
await presence.leave(room_id, user.id)
|
||||
if is_blurred:
|
||||
await focus_presence.mark_focused(user.id)
|
||||
if await global_presence.disconnect(user.id):
|
||||
await broadcast_member_updated(db, broadcaster, user.id)
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
import uuid
|
||||
|
||||
from redis.asyncio import Redis
|
||||
|
||||
|
||||
class FocusPresence:
|
||||
"""Cross-instance "is this desktop-mode user's window currently
|
||||
unfocused," used only to widen desktop_notification/push eligibility
|
||||
beyond plain room-connection state (#59). A Redis hash (field =
|
||||
user_id, value = refcount of that user's currently-blurred desktop
|
||||
connections), parallel to Presence/GlobalPresence.
|
||||
|
||||
Absence from this hash is the default and means "focused." That default
|
||||
is also exactly right for every browser-tab connection: only the
|
||||
desktop client ever sends focus/blur frames at all (see chat.py's
|
||||
"focus" envelope handling), so a browser user never appears here --
|
||||
their attention is already fully captured by Presence's room-connection
|
||||
state, which stays visibility-gated with no separate focus signal.
|
||||
|
||||
Refcounted for the same multi-connection reason as Presence/
|
||||
GlobalPresence, with the same known simplification: two desktop windows
|
||||
for one user, one focused and one blurred, count as "unfocused" here
|
||||
(refcount > 0) even though the user does have attention somewhere. That
|
||||
errs toward notifying rather than silently missing one, which is the
|
||||
safer failure mode for a notification.
|
||||
"""
|
||||
|
||||
def __init__(self, redis: Redis) -> None:
|
||||
self._redis = redis
|
||||
|
||||
def _key(self) -> str:
|
||||
return "presence:unfocused"
|
||||
|
||||
async def mark_blurred(self, user_id: uuid.UUID) -> None:
|
||||
await self._redis.hincrby(self._key(), str(user_id), 1)
|
||||
|
||||
async def mark_focused(self, user_id: uuid.UUID) -> None:
|
||||
key = self._key()
|
||||
field = str(user_id)
|
||||
remaining = await self._redis.hincrby(key, field, -1)
|
||||
if remaining <= 0:
|
||||
await self._redis.hdel(key, field)
|
||||
|
||||
async def is_unfocused(self, user_id: uuid.UUID) -> bool:
|
||||
return await self._redis.hexists(self._key(), str(user_id))
|
||||
@@ -128,6 +128,87 @@ def test_desktop_notification_not_sent_to_room_member_who_is_present(ws_client_f
|
||||
assert live_message["type"] == "message"
|
||||
|
||||
|
||||
def test_desktop_notification_sent_to_connected_but_blurred_member(ws_client_factory):
|
||||
# #59: bob keeps the room's channel joined (so live delivery to a room
|
||||
# actually open on screen never stops) but reports his desktop window
|
||||
# as unfocused via a "focus" frame -- the whole point of this fix is
|
||||
# that notification eligibility no longer needs the client to fake
|
||||
# "offline" by leaving the room's channel, which used to also break
|
||||
# live delivery until the room was manually left and rejoined.
|
||||
instance1 = ws_client_factory()
|
||||
instance2 = ws_client_factory()
|
||||
|
||||
alice = _register_ws(instance1, _unique("alice"))
|
||||
room = instance1.post("/api/rooms", json={"name": _unique("general")}).json()
|
||||
|
||||
_register_ws(instance2, _unique("bob"))
|
||||
instance2.post(f"/api/rooms/{room['id']}/join")
|
||||
|
||||
with instance2.websocket_connect("/ws/chat") as bob_ws:
|
||||
bob_ws.send_json({"type": "join", "room_id": room["id"]})
|
||||
assert bob_ws.receive_json()["type"] == "joined"
|
||||
|
||||
bob_ws.send_json({"type": "focus", "focused": False})
|
||||
# Sync barrier -- see test_mentions.py's identical pattern: a
|
||||
# second (idempotent) join only acks once the prior "focus"
|
||||
# frame's own handling (and commit) has completed.
|
||||
bob_ws.send_json({"type": "join", "room_id": room["id"]})
|
||||
assert bob_ws.receive_json()["type"] == "joined"
|
||||
|
||||
with instance1.websocket_connect("/ws/chat") as alice_ws:
|
||||
alice_ws.send_json({"type": "join", "room_id": room["id"]})
|
||||
assert alice_ws.receive_json()["type"] == "joined"
|
||||
message = _send_and_sync(alice_ws, room["id"], "you awake?")
|
||||
assert message["type"] == "message"
|
||||
|
||||
# Live delivery still works -- bob's room channel was never left.
|
||||
live_message = _recv(bob_ws)
|
||||
assert live_message["type"] == "message"
|
||||
assert live_message["content"] == "you awake?"
|
||||
|
||||
# ...and he's still notified, despite being "connected" to the room.
|
||||
desktop_note = _recv(bob_ws)
|
||||
assert desktop_note == {
|
||||
"type": "desktop_notification",
|
||||
"id": message["id"],
|
||||
"room_id": room["id"],
|
||||
"title": f"#{room['name']}",
|
||||
"body": f"{alice['username']}: you awake?",
|
||||
}
|
||||
|
||||
|
||||
def test_desktop_notification_not_sent_after_refocus(ws_client_factory):
|
||||
# Proves the "focus" signal is a live toggle, not one-way -- blurring
|
||||
# and then refocusing before the message arrives must fully cancel out.
|
||||
instance1 = ws_client_factory()
|
||||
instance2 = ws_client_factory()
|
||||
|
||||
alice = _register_ws(instance1, _unique("alice"))
|
||||
room = instance1.post("/api/rooms", json={"name": _unique("general")}).json()
|
||||
|
||||
_register_ws(instance2, _unique("bob"))
|
||||
instance2.post(f"/api/rooms/{room['id']}/join")
|
||||
|
||||
with instance2.websocket_connect("/ws/chat") as bob_ws:
|
||||
bob_ws.send_json({"type": "join", "room_id": room["id"]})
|
||||
assert bob_ws.receive_json()["type"] == "joined"
|
||||
|
||||
bob_ws.send_json({"type": "focus", "focused": False})
|
||||
bob_ws.send_json({"type": "focus", "focused": True})
|
||||
bob_ws.send_json({"type": "join", "room_id": room["id"]})
|
||||
assert bob_ws.receive_json()["type"] == "joined"
|
||||
|
||||
with instance1.websocket_connect("/ws/chat") as alice_ws:
|
||||
alice_ws.send_json({"type": "join", "room_id": room["id"]})
|
||||
assert alice_ws.receive_json()["type"] == "joined"
|
||||
_send_and_sync(alice_ws, room["id"], "hello again")
|
||||
|
||||
# Refocused before the message arrived -- only the live broadcast,
|
||||
# same as test_desktop_notification_not_sent_to_room_member_who_is_present.
|
||||
live_message = _recv(bob_ws)
|
||||
assert live_message["type"] == "message"
|
||||
|
||||
|
||||
def test_desktop_notification_not_sent_to_non_member(ws_client_factory):
|
||||
instance1 = ws_client_factory()
|
||||
instance2 = ws_client_factory()
|
||||
|
||||
Reference in New Issue
Block a user