fix(desktop): stop dropping live events behind the reader clock - #3762
Open
tfournet wants to merge 1 commit into
Open
fix(desktop): stop dropping live events behind the reader clock#3762tfournet wants to merge 1 commit into
tfournet wants to merge 1 commit into
Conversation
Signed-off-by: Tim Fournet <timfournet@gmail.com>
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
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.
Summary
The desktop channel live subscription derived its
sincefilter from the reader's clock. Nostrcreated_atis author-assigned, and the relay accepts timestamps up to 900s from server time (MAX_TIMESTAMP_DRIFT_SECS,crates/buzz-relay/src/handlers/ingest.rs). Becausesinceis re-applied at live fan-out (crates/buzz-relay/src/subscription.rs→filters_match→crates/buzz-core/src/filter.rs), a valid message from a peer whose clock is behind the reader's was dropped — silently, and for the entire lifetime of that subscription. Nothing recovered it except a reconnect or channel re-entry.The fix removes the reader-clock
sinceand bounds initial replay withlimitinstead, which does not constrain future fan-out.Removing
sincealone would have traded one loss window for another: theCLOSEDretry path resent the original filter unchanged, so after a backoff of up to 30s a subscription would resume with only the newestlimitevents and permanently miss anything older in the gap. This PR therefore pairs the filter change with paged recovery, deriving the resume point fromlastSeenCreatedAt— an author timestamp the relay already accepted, which is exactly why it is safe where a reader clock is not. That machinery already existed for reconnect (relayReconnectReplay.ts) and is reused rather than reinvented.Changes, all desktop TypeScript (no Rust, no relay changes):
relayChannelFilters.ts— newbuildChannelLiveFilter: nosince,limit: 50. The limit must stay above zero:shouldPageReconnectReplaygates paged reconnect recovery onfilter.limit > 0, solimit: 0would silently disable reconnect history recovery. Documented at the call site.relayClientSession.ts—subscribeToChannelLiveuses the new builder; adds an optional per-flush callback so replay projection can be batched.relayClosedRecovery.ts—CLOSEDretry now performs paged catch-up fromlastSeenCreatedAt - skewwhen a last-seen timestamp exists. TerminalCLOSEDhandling (auth/access/filter failure) and rate-limit backoff are unchanged.relayReconnectReplay.ts— replayed events route through the buffered event path rather than bypassing it.hooks.ts— window projection is batched per relay flush and runs only when an event actually mutated the window. Without this, removingsincewould trigger up to 50 full-store flatten-and-reconcile passes on a single channel switch, sincemainprojects once per event.Related issue
None found for this bug. Searched open and closed issues and PRs for
clock skew,created_at since,live subscription since,message loss subscription, anddrops messages.Related but not duplicate: #3104 ("Desktop thread can render agent reply before triggering message under client clock skew"). Same root cause — author-assigned
created_atunder clock skew — but the opposite symptom. #3104 is about messages that arrive and sort into the wrong position; this PR is about messages that never arrive at all because the reader's clock acts as a delivery filter. Fixing one does not fix the other.Adjacent instance not fixed here:
buildChannelMentionFilter(relayChannelFilters.ts) still usessince: Math.floor(Date.now() / 1_000)from the reader's clock, so channel mentions from a peer with a lagging clock can be dropped the same way. It is left alone deliberately — it needs its own check on whether a barelimitis safe there and whether that subscription has a recovery path. Happy to fold it into this PR or file a follow-up, whichever the maintainers prefer.Testing
No UI change, so no screenshots.
New unit coverage in
relayChannelFilters.test.mjs:sincekey (the regression guard for this bug)limit > 0, andshouldPageReconnectReplay(liveFilter) === true— pinning the coupling a futurelimit: 0would silently break#hscoping and the kinds set, includingKIND_CHANNEL_THREAD_SUMMARYThe existing
relay-reconnect.spec.ts:216("reconnect backfills more missed channel messages than the live subscription limit") is the end-to-end proof for the recovery half: it injects 260 messages during a disconnect and requires the oldest to remain reachable, which fails if recovery is bounded by the livelimit. It passes on this branch.Local verification (Fedora 44, x86_64):
just desktop-check(incl. file-size, px-text, pubkey-truncation gates)just desktop-testjust desktop-typecheckjust desktop-buildpnpm exec playwright test --project=smoke(all 4 shards)Two disclosures so they are not discovered in review rather than read here:
just cidoes not go green on my machine, because ofbuzz-db'sreplica_fence::tests::fence_starts_closed_and_opens_on_advance— a nanosecond-vs-microsecond timestamp assertion. It fails identically on unmodifiedmain, and this branch changes zero.rsfiles (git diff --name-only origin/main..HEAD | grep -c '\.rs$'→ 0), so it cannot be caused by this change. Path filtering means Rust Lint and Unit Tests do not run for a desktop-only PR anyway.relay-reconnect.spec.ts:97("failed initial relay dial retries automatically") flaked once under shard parallelism withRelay state seam is not installed— the spec's own instrumentation hook was not yet installed when it polled. It passes in isolation on bothmainand this branch, and CI runs withretries: 2.Performance is unchanged, as expected — this is a correctness fix, not a latency change. Cold-switch and warm-switch longtask harnesses are at parity with
mainwithin run-to-run spread.