fix(desktop): stop a future-dated message from marking a channel read

[Chessing234]

Refs #6046 — the desktop half. The issue traces the mobile Dart path and says the desktop write path still needs confirming; it does the same thing, so this fixes that side.

created_at is self-asserted by the sending client, and the relay bounds it for moderation commands but not for ordinary messages. resolveChannelReadMarker took the newest timestamp verbatim, so one event dated ahead of the clock landed in the read marker and every genuinely newer message afterwards failed createdAt > readAt — no unread badge, no divider, and since #5983 no thread resume — until wall-clock time caught up with the bad value.

Where it enters. Both inputs to the marker are event-derived, so both are clamped:

• callerReadAt — a message's own created_at, via channel open, the Esc shortcut's lastMessageAt (useMarkAsReadShortcuts.ts:43), and mark-all-read (AppShell.tsx:415);
• observedLatest — the newest live event this client has seen.

resolveChannelReadMarker is the single funnel every markChannelRead call goes through, so the ceiling lands in one place.

Why a tolerance and not a hard now ceiling. Ordinary skew between two machines is seconds; clamping hard would leave a just-received message unread until this clock caught up, which is a new bug in place of the old one. 120s is the relay's own MAX_COMMAND_SKEW_SECS (crates/buzz-relay/src/handlers/moderation_commands.rs:81), i.e. the number this codebase already uses for "clock difference we accept". NIP-AB (crates/buzz-core/src/pairing/NIP-AB.md:581) already says clients MUST NOT set created_at in the future at all, so nothing legitimate is above the ceiling.

One behavioural detail worth calling out. clearObserved now reports false when the marker was clamped below the observed event. That is deliberate: the future-dated event genuinely is unread, so dropping the observed refs would clear the sidebar dot this change exists to preserve. The poisoning message itself stays unread until its own timestamp arrives — truthful, and it no longer takes every later message down with it.

Tests — 5 new cases in unreadReadMarker.test.mjs, with nowSeconds injected (the existing house pattern from formatDayGroupLabel):

• a caller timestamp a year ahead clamps to now + 120;
• an observed event a year ahead clamps and keeps the observed refs;
• the end-to-end shape: poison → mark read → a genuine later message is still counted unread by computeChannelUnreadMarker, which is the comparison the divider actually uses;
• 30s of ordinary skew passes through unclamped;
• past timestamps and the null paths are unchanged.

Reverting the clamp turns the first three red.

Scope: desktop only, one concern. The mobile Dart path (channel_detail_page.dart / message_read_state.dart) is the issue's own subject and is untouched here — I have no Flutter toolchain on this machine, so I would not be able to run anything I wrote there.

Verified locally in desktop/: pnpm typecheck, pnpm check (2 warnings + 2 infos, all pre-existing on main), pnpm test 4959 passed, git diff --check.

[themiguelamador]

Reviewed exact head e6bb0e9d3c386beecd10d25ae84f3c2d573de90e. I found three release-blocking gaps and fixed them on Complear:review/pr-6063-fix (d2238be590662b4f14c7fa84921bb737b80129f2).

• P1 — the clamp still hides legitimate messages for two minutes. Mapping an outlier to now + 120 manufactures a future read frontier. A correct message arriving at now + 1 still fails createdAt > readAt. The tolerance should decide whether a timestamp is plausible; an implausible timestamp must be repaired to now.
• P1 — Mark all read bypasses the claimed funnel. markAllChannelsRead writes latestByChannelRef straight through markContextRead, then clearAll() deletes the observed evidence. A future event therefore still poisons the marker through the Esc/rail mark-all action and loses its unread dot.
• P1 — persisted and synced poison remains effective. Read markers are monotonic and loaded from local storage plus encrypted NIP-RS events. This patch only constrains new local markChannelRead calls, so a marker written before upgrade or by another unpatched desktop remains year-ahead and unrecoverable in the UI.

The fix branch centralizes the skew policy, caps outliers to the present, retains future observed events during mark-all, drops implausible markers during local hydration and synced-state parsing, and adds regressions for each route. Verification passed: desktop typecheck; full desktop check including file-size, px-text, and pubkey-truncation gates (only the four pre-existing Biome diagnostics); focused 91-test marker/read-state suite; and the full desktop unit suite (4,963 passed). Diff checks and commit signoff also pass.

[Chessing234]

All three confirmed and fixed, one commit each (a828199, 9dcc67d, 8eb0165). (Complear/buzz 404s, so these are written from your description.)

P1 — the clamp. You're right, and it's the same bug in miniature: mapping an outlier to now + 120 manufactures a read frontier and hides everything arriving in the next two minutes. The tolerance now decides plausibility only. One refinement on your framing: I discard each input separately rather than repairing the max, so a real caller position isn't thrown away just because the observed timestamp beside it is poison — resolveChannelReadMarker(realReadAt, YEAR_AHEAD) keeps realReadAt. Only when no input survives does it repair to now. There's now a test asserting a message at now + 1 is still unread against a repaired marker.

P1 — mark-all. Confirmed: markAllChannelsRead wrote latestByChannelRef straight through markContextRead and then clearAll(). It now resolves through the same funnel, and since a repaired marker doesn't cover the future-dated event, clearAll takes a retain set and rebuilds the refs from it instead of wiping the bucket — otherwise the fix would still lose that channel's unread dot.

P1 — persisted and synced poison. Confirmed, and both entry points now share the policy: readStoredReadState skips an implausible persisted marker (hydration is the last chance, since the next write persists whatever was loaded) and sanitizeContexts skips one in a decrypted NIP-RS blob. Both drop rather than clamp — clamping to the present asserts a read position nobody reached, while dropping restores the channel to unread where the user can see it. Markers inside the tolerance are untouched, so ordinary cross-device skew still merges.

The policy itself (isPlausibleReadMarker) now lives in readState/readStateFormat.ts, since all three routes need the same one.

Verification: desktop typecheck, biome, the channels suite (311 passed), and the full desktop suite (4,968 passed).

[mfethe1]

Reviewed at head 8eb0165. I reported #6046, so treat this as an interested party rather than
a maintainer — and first, I owe you a correction that changes the sizing of this work.

The relay already bounds created_at on every event. I claimed in #6046 that only
moderation commands were bounded. That is wrong: crates/buzz-relay/src/handlers/ingest.rs:2005
applies a symmetric MAX_TIMESTAMP_DRIFT_SECS = 900 to every ingested event, after signature
verification and before any kind branch. So the worst poison this relay admits is now + 900,
and it self-heals in ≤15 minutes rather than persisting. The client-side hole is real and worth
closing — markers persist, sync, and can predate the gate — but it is a 15-minute window under
normal operation, not an unbounded one. Details in #6046.

That matters here mainly because it bears on the tolerance constant.

The mechanism looks right to me and the funnel looks genuinely complete — every desktop marker
write goes through resolveChannelReadMarkerUnix, and both bulk ingest routes (NIP-RS
sanitizeContexts, localStorage mergeLocalStorageKey) apply the same predicate. Three things
I think are blocking, and one gap that probably is not yours.

---

1. The tolerance is 120s, but the relay admits ±900s — so this rejects timestamps the relay
legitimately accepted.

MAX_READ_MARKER_SKEW_SECONDS = 120 is justified in-comment by reference to
handlers/moderation_commands.rs. That constant is a replay window for kinds 9040-9044, which
are never stored; it is not the bound on ordinary messages. The real bound is ingest.rs:2005
= 900.

Consequence: a message the relay accepted at now + 300 is judged implausible, the marker
repairs to now, and createdAt > readAt is then still true — so the channel keeps an unread
badge and divider for up to five more minutes, re-arming on every channel open because each
open re-marks at now. The previous review objected that clamping hid messages for two minutes;
discarding surfaces phantom-unread ones for up to fifteen. Suggest 900, or a shared import, with
the comment pointing at ingest.rs:2005.

2. The hydration path deletes rather than quarantines, and the predicate it deletes on cannot
be validated.

This is the one I would most want changed. isPlausibleReadMarker(value, now) compares a
peer-derived timestamp against Date.now() on the local machine, and cannot distinguish "their
clock is fast" from "mine is slow". A machine that boots before its first NTP sync, resumes a
suspended VM, or has a dead CMOS battery will judge every stored marker implausible.

As a filter that is survivable. It is not a filter: hydrateFromLocalStorage drops, then
writeLocalState() serialises the surviving map back (readStateStorage.ts:154). The user's
read state is gone from disk before they have done anything, and NIP-RS cannot restore it
because sanitizeContexts applies the same predicate against the same bad clock. Every channel
lights up unread and nothing clears it.

The comment at the drop site — markers are monotonic, "hydration is the last chance to disarm
it" — is the reason it is necessary and also the reason it is dangerous.

Suggestion: quarantine or clamp on the bulk paths rather than delete. Math.min(value, now)
preserves the invariant you need (a marker at now can never hide a message arriving after
now) with no data loss, and if the local clock was the problem it corrects itself on the next
boot. A cheap extra guard: if a large fraction of hydrated markers read as implausible, assume
the local clock is wrong and fail open to pre-patch behaviour rather than repairing everything
against a bad now.

3. markAllChannelsRead looks like a no-op on exactly the channel it was rewritten to repair.

It passes getEffectiveTimestamp(channelId) — the channel's existing read marker, not a message
created_at — as callerUnix. For any channel the user has ever read, that value is in the past
and therefore always plausible, so the markAt = now repair branch is unreachable.

Worked example: #general has a marker at now−3600, five genuine unread messages from the
last ten minutes, and one future-dated observed event. plausible = [now−3600], so
markAt = now−3600, and markContextRead hits its monotonic early return and does nothing —
the five real messages stay unread and the badge does not clear. The branch is reachable for a
never-read channel, which is probably why it survived. Passing null and letting the existing
marker win through the monotonic path, or flooring at now when the observed value was
rejected, would fix it.

---

Not yours, but it should be named before this merges: mobile is entirely unpatched, and a
desktop-only policy diverges rather than degrades.

#6046 was traced on mobile — channel_detail_page.dart:110-119 still takes unclamped
max(createdAt). All three chokepoints this PR hardens have unguarded Dart equivalents, and
mobile has eleven markContextRead call sites (none clamped) against desktop's single funnel, so
it is not a one-line mirror.

The part worth flagging explicitly: mobile's marker update is monotonic in the same way desktop's
is — read_state_provider.dart:49-53 returns unchanged state for any timestamp <= current. So
once a mobile client holds a future marker, a desktop that has repaired its own copy to now
cannot lower it: mobile discards the smaller value and keeps showing the channel as read. That is
a standing disagreement between the two clients rather than a transient one. I have not traced the
full NIP-RS sync path, so I can't say how the blob itself reconciles — but the monotonic rule on
the receiving side is enough to make a desktop-only policy insufficient. Worth a linked mobile
issue before this lands, and I would not auto-close #6046 with this PR.

---

I have not run your branch — this is from reading the diff at 8eb0165 and the surrounding code,
so treat the worked examples as arguments rather than reproductions. Happy to be wrong on any of
them, particularly (3), where I may be missing a caller that passes something else.