Heal the inbox after a missed broadcast (chats--refresh-inbox)#2
Open
rameerez wants to merge 1 commit into
Open
Heal the inbox after a missed broadcast (chats--refresh-inbox)#2rameerez wants to merge 1 commit into
rameerez wants to merge 1 commit into
Conversation
The inbox already receives Turbo 8 page refreshes (broadcast_refresh_later_to), but Action Cable has no replay: a refresh sent while the client's socket was down (backgrounded tab/app, network blip, laptop asleep) is lost and the inbox sits stale until the user navigates. The thread got stale-catch-up in 0.1.1 (the single biggest reliability pattern from the Campfire review); the inbox was the remaining gap. Add a tiny chats--refresh-inbox controller that re-runs the same page refresh on the two moments we might have missed one — cable reconnect and return-to-visible — reusing the thread's channel-free reconnect detection (observe the <turbo-cable-stream-source> `connected` attribute; no new Action Cable channel). The recovery action is Turbo.session.refresh() rather than the thread's `?since=` delta, because inbox broadcasts already ARE page refreshes. Auto-registered via the engine importmap pin, so hosts need zero changes. It guards against morphing the list out from under someone typing in search. - engine_test: assert the new controller is pinned for auto-registration - conversations_flow_test: assert the inbox stream source is wrapped by the reconciler
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.
Gap
The thread got stale-catch-up in 0.1.1 — the reliability pattern
docs/campfire_review.mdcalls "the single biggest ... in [Campfire's] codebase," because mobile WebViews reap sockets constantly. The inbox never got the same treatment.The inbox updates via Turbo 8 page-refresh broadcasts (
Chats::Broadcasts.refresh_inbox_of→broadcast_refresh_later_to). Action Cable has no replay: a refresh broadcast sent while this client's socket was down (backgrounded tab/app, network blip, laptop asleep) is lost, and the inbox silently sits at its last render until the user happens to navigate. Same failure mode the thread already guards against — just on the other list.(Found while auditing a production host app whose inbox is a native tab root with pull-to-refresh disabled, so there was no user-reachable recovery at all.)
Fix
A tiny
chats--refresh-inboxcontroller that re-runs the same page refresh on the two moments we might have missed one:connectedattribute turbo-rails toggles on<turbo-cable-stream-source>; no new Action Cable channel), andThe recovery action is
Turbo.session.refresh()(not the thread's?since=HTTP delta) because inbox broadcasts already are whole-page refreshes — idempotent, morphing, scroll-preserving. It skips refreshing while the search box is focused so it never yanks the list out from under someone typing.Auto-registered via the engine importmap pin, so hosts need zero changes; a host can still override it by pinning the same key.
Tests
engine_test: the controller is pinned undercontrollers/chats/for stimulus auto-registration.conversations_flow_test: the inbox stream source is wrapped by the reconciler.Full suite green (the one unrelated red in my local run is a pre-existing asset-path assertion that hard-codes the checkout dir name
chats; it passes in CI / a normally-named checkout).Docs
CHANGELOG (Unreleased), README (both surfaces self-heal now), and the
campfire_review.mdledger updated to note inbox parity.