Skip to content

Allow fully read markers to "go backwards" #19635

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
anoadragon453 wants to merge 2 commits into
base: develop
Choose a base branch
from
+17 −22
Draft

Allow fully read markers to "go backwards" #19635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16409c9
Allow fully read markers to "go backwards"
anoadragon453 Mar 31, 2026
5c2ec68
newsfile
anoadragon453 Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/19635.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow clients to set their user's `m.fully_read` marker back to older events.
38 changes: 16 additions & 22 deletions synapse/handlers/read_marker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
from typing import TYPE_CHECKING

from synapse.api.constants import ReceiptTypes
from synapse.api.errors import SynapseError
from synapse.util.async_helpers import Linearizer

if TYPE_CHECKING:
Expand Down Expand Up @@ -55,25 +54,20 @@ async def received_client_read_marker(
user_id, room_id, ReceiptTypes.FULLY_READ
)

should_update = True
# Get event ordering, this also ensures we know about the event
event_ordering = await self.store.get_event_ordering(event_id, room_id)
# Ignore subsequent attempts to apply the same read marker.
if (
existing_read_marker is not None
and not isinstance(existing_read_marker.get("event_id"), str)
Copy link
Copy Markdown
Contributor

@SpiritCroc SpiritCroc Apr 1, 2026

Choose a reason for hiding this comment

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

Wondering, shouldn't we check the isinstance without not here?

Copy link
Copy Markdown
Member Author

@anoadragon453 anoadragon453 Apr 1, 2026

Choose a reason for hiding this comment

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

Oops, good catch. This is why this PR should be paired with some tests!

and event_id == existing_read_marker.get("event_id")
):
return

if existing_read_marker:
try:
old_event_ordering = await self.store.get_event_ordering(
existing_read_marker["event_id"], room_id
)
except SynapseError:
# Old event no longer exists, assume new is ahead. This may
# happen if the old event was removed due to retention.
pass
else:
# Only update if the new marker is ahead in the stream
should_update = event_ordering > old_event_ordering

if should_update:
content = {"event_id": event_id}
await self.account_data_handler.add_account_data_to_room(
user_id, room_id, ReceiptTypes.FULLY_READ, content
)
# Update the user's account data with the new read marker.
#
# Note: it's OK if this update refers to an *older* event than the
# previous read marker; a user may have accidentally marked some
# messages as read, and want to undo it.
content = {"event_id": event_id}
await self.account_data_handler.add_account_data_to_room(
user_id, room_id, ReceiptTypes.FULLY_READ, content
)
Loading