Keep the fully-read marker monotonic - #565
Open
ruka-hamanasu wants to merge 3 commits into
Open
Conversation
added 2 commits
August 28, 2026 16:34
A backwards m.fully_read write was accepted verbatim, so any stale device or client could regress the marker another one already advanced, leaving the unread line stuck or jumping backwards between syncs. Resolve both the stored and the incoming marker event to their timeline positions and skip the write unless it advances, matching the existing monotonic gates for m.read and m.read.private. An unresolvable position on either side accepts the write, as for public receipts. Backwards writes are silently accepted-but-ignored to keep the endpoints idempotent. Applies to both /read_markers and /receipt; storage errors in /read_markers now propagate instead of being swallowed. Adds an HTTP-level integration test (fully_read_monotonic) covering forward writes, backwards writes via /read_markers, and backwards writes via /receipt.
repro-fully-read-regression.sh verifies the monotonic fully-read guard against a running server (forward write stores, backwards writes via /read_markers and /receipt are ignored). probe-read-state.sh dumps the server-side signals a client's unread state derives from (notification counts, own receipts, m.fully_read, marked-unread) for a given account and room. Both default to a test server and test accounts; they are diagnostic tooling, not general-purpose tests.
3 tasks
9 tasks
A receipt naming the currently stored event was rejected without allocating a stream position, producing no EDU and no sync traffic at all. A client which missed the original receipt (e.g. read on another device while the room was outside its sliding-sync window) could never repair its derived unread state in a quiet room: re-reading the same position stayed silent, and no new events arrived to force a recomputation. Element X / SchildiChat Next showed such rooms as unread (grey client-side count) forever while other clients correctly showed them read. Identical receipts now rewrite the row under a fresh stream position so the receipt EDU is re-emitted to sync (v3 ephemeral and the MSC4186 receipts extension), but still report no advance: notification counts are not reset and the last-notification-read stamp is untouched, so legitimate unread accumulated after the receipted event is preserved. Strictly older receipts remain rejected (monotonicity, matrix-construct#516). The private read marker path re-announces identically through its sync gate; unannounced identical markers (own-send path) stay a no-op.
jevolk
force-pushed
the
dev
branch
3 times, most recently
from
September 4, 2026 06:52
6ae89f9 to
93a663e
Compare
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.
What does this PR do?
Unlike m.read and m.read.private receipts, which already have monotonic gates, the m.fully_read marker accepted any position, including backwards. In a multi-device setup, any client acting on a stale view (e.g. reposting its last-known position on startup, or a "mark as read" action on an old notification) silently regressed the marker that another device had already advanced. Users saw the unread line stuck in place or jumping backwards between syncs, and no amount of re-reading could be trusted to fix it, since whichever device was stalest would pull it back again. There is also a Matrix Rust SDK quirk that can cause older private read receipts to take precedence over fully-read if they're on events the client doesn't have locally, like when using an existing account on a new client.
Fix: before storing, resolve both the stored and the incoming marker event to their timeline positions and skip the write unless it advances, the same semantics as the existing receipt gates (position_advances): an unresolvable position on either side accepts the write. Backwards writes are accepted-but-ignored (200 OK) so the endpoints stay idempotent. Applies to both POST /read_markers and POST /receipt (m.fully_read). As a side effect, storage errors in /read_markers now propagate instead of being silently swallowed.
The spec doesn't say anything about whether fully-read can or should be allowed to go backward, but there isn't an obvious use case for it, and I'm not sure it can really be implemented coherently since I think it should be order-independent, which it can't be if it's not monotonic.
Testing: new HTTP-level integration test (fully_read_monotonic) — forward write stores; backwards writes via /read_markers (all three fields, as clients send them) and via /receipt leave the marker untouched. Verified to fail without the guard. Also verified live against a production server with a deterministic repro script (included in a separate commit, drop if unwanted).
I currently have this running on matrix.agiadn.org along with #564, and along with some FluffyChat fixes that were preventing it from updating the read marker in some cases, FluffyChat now marks rooms as read when I read them in all cases.
Found and fixed another case (possibly a regression?) for sliding sync clients: the monotonicity check was causing duplicate receipts to be rejected entirely with no new EDU. But the only case where we should be seeing these is if a client thinks it's reading unread messages. So we now send a new EDU in these cases so that those clients (Shildichat Next is the one I've been testing with) mark the room as read.
Checklist
cargo fmtand satisfies clippy andrustc lints; any allowed lint is justified by an obvious reason or a
comment.
are noted in the description above.
src/core/config/mod.rsdoccomments and the regenerated
tuwunel-example.tomlis committed.docs/.and my conduct is in line with the Contributor's Covenant and
Tuwunel's Code of Conduct.