From 138ae8d6f4b60d02f566a3d5c4e35717f3818e22 Mon Sep 17 00:00:00 2001 From: Arnas Donauskas Date: Thu, 21 May 2026 19:22:03 +0300 Subject: [PATCH] fix(inbox): replace broken updater-closure flag, add reconnect/visibility resync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses #106 (issue stays open until the reporter confirms in prod). #105's `let foundInList = false; setState(p => { foundInList = ...; ... })` pattern is broken: setState updaters run during reconciliation, *after* the synchronous handler returns, so `foundInList` reads `false` in the same tick and `hydrateConversation()` fires on every code path the flag was meant to gate. Every incoming message and every conversation UPDATE (read receipts, status flips, agent assignment) triggered a redundant DB fetch, queuing HTTP behind realtime WS frames on the same supabase client and starving the channel — which surfaced as exactly what #106 reports: messages that never arrive, threads stuck on "No messages yet", events landing out of order. The fix: 1. Synchronous known-IDs ref. `knownConvIdsRef` mirrors the conv ids currently in state, kept in sync via a useEffect on `conversations`. Event handlers check it synchronously to decide between a cheap in-place patch and a hydrate refetch — no more closure-flag pattern. 2. Reconnect + visibility resync. Realtime is best-effort: events sent while the WS was disconnected or while the tab was background-throttled are gone. A `resyncToken` state bumped on (a) `isConnected` false→true transitions after the initial connect and (b) tab visibility → visible drives `ConversationList` and `MessageThread` to refire their fetch effects. The reactions effect in MessageThread is split into fetch + channel-subscribe so a resync only refetches the rows without tearing down and rebuilding the realtime channel. 3. Keep the conv-INSERT hydrate. Still the right fix for #104 (realtime payloads never carry the `contact` join); now it only fires when the conversation is genuinely unknown to state. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/(dashboard)/inbox/page.tsx | 167 +++++++++++++++------ src/components/inbox/conversation-list.tsx | 13 +- src/components/inbox/message-thread.tsx | 36 ++++- 3 files changed, 164 insertions(+), 52 deletions(-) diff --git a/src/app/(dashboard)/inbox/page.tsx b/src/app/(dashboard)/inbox/page.tsx index f8f782b..3d8bb1e 100644 --- a/src/app/(dashboard)/inbox/page.tsx +++ b/src/app/(dashboard)/inbox/page.tsx @@ -30,6 +30,14 @@ export default function InboxPage() { const [whatsappConnected, setWhatsappConnected] = useState( null ); + /** + * Bumped whenever we want children (ConversationList, MessageThread) + * to refetch from the DB — used as a safety net against missed + * realtime events. Bumped on WS reconnect and on tab visibility → + * visible. The initial mount fetches don't depend on this; they fire + * once on conversationId-change as usual. + */ + const [resyncToken, setResyncToken] = useState(0); // Fire the deep-link auto-select exactly once per URL — subsequent // list refreshes (realtime, manual refetch) must not snap the user @@ -43,6 +51,24 @@ export default function InboxPage() { // new conversation even when both events arrive within milliseconds. const hydratingConvIdsRef = useRef>(new Set()); + /** + * Synchronous mirror of the conversation ids currently in `conversations` + * state. Event handlers need to know "do we already have this conv?" + * without waiting for a setState updater to run — updaters fire during + * reconciliation, *after* the synchronous handler code returns, so a + * `let foundInList = false; setState(p => { foundInList = ...; return ... })` + * flag reads as `false` in the same tick (this exact bug shipped in #105 + * and caused #106: every incoming message and every status flip fired a + * redundant DB hydrate, swamping the supabase client and starving the + * realtime channel). The ref is kept in sync via the effect below. + */ + const knownConvIdsRef = useRef>(new Set()); + useEffect(() => { + const next = new Set(); + for (const c of conversations) next.add(c.id); + knownConvIdsRef.current = next; + }, [conversations]); + // Pull the conversation row with its `contact` joined and merge it // into state. Needed because Supabase Realtime payloads only carry the // row's own columns — a brand-new conversation arrives without a @@ -142,32 +168,33 @@ export default function InboxPage() { }); } - // Update conversation list preview. If the conv isn't in state - // yet (its INSERT event was missed, delayed, or this is the - // first message for a conversation created out-of-band) the - // map() below is a no-op and the preview would never appear — - // hydrate the row from the DB instead so the inbox self-heals. - let foundInList = false; - setConversations((prev) => { - if (!prev.some((c) => c.id === newMsg.conversation_id)) { - return prev; - } - foundInList = true; - return prev.map((c) => - c.id === newMsg.conversation_id - ? { - ...c, - last_message_text: newMsg.content_text ?? "", - last_message_at: newMsg.created_at, - unread_count: - activeConversation?.id === newMsg.conversation_id - ? 0 - : c.unread_count + 1, - } - : c, + // Update conversation list preview. We need to know *synchronously* + // whether the conv is already in state to decide between patching + // the preview and triggering a hydrate — see the comment on + // knownConvIdsRef for why a closure flag inside the updater would + // always read false here. + if (knownConvIdsRef.current.has(newMsg.conversation_id)) { + setConversations((prev) => + prev.map((c) => + c.id === newMsg.conversation_id + ? { + ...c, + last_message_text: newMsg.content_text ?? "", + last_message_at: newMsg.created_at, + unread_count: + activeConversation?.id === newMsg.conversation_id + ? 0 + : c.unread_count + 1, + } + : c, + ), ); - }); - if (!foundInList) { + } else { + // First time we're seeing this conv: the conv-INSERT event + // hasn't landed yet, or was missed. Hydrate from the DB so + // the row surfaces with its `contact` joined; the conv-UPDATE + // event the webhook emits right after the message INSERT will + // converge state when it arrives. hydrateConversation(newMsg.conversation_id); } } @@ -192,27 +219,30 @@ export default function InboxPage() { const conv = event.new; if (event.eventType === "INSERT") { - // Prepend immediately for snappy UX, then hydrate to fill in - // the `contact` join (realtime payloads omit it). The dedupe - // inside hydrateConversation keeps the result idempotent if - // the first-message-INSERT handler also triggers a hydrate. - setConversations((prev) => { - if (prev.some((c) => c.id === conv.id)) return prev; - return [conv, ...prev]; - }); - hydrateConversation(conv.id); + // Prepend immediately for snappy UX so the new conv shows in the + // list right away, then hydrate to fill in the `contact` join + // (realtime payloads never include joins). Skip both if we + // already have the row — that shouldn't happen normally, but + // out-of-order delivery would have us prepending a duplicate. + if (!knownConvIdsRef.current.has(conv.id)) { + setConversations((prev) => { + if (prev.some((c) => c.id === conv.id)) return prev; + return [conv, ...prev]; + }); + hydrateConversation(conv.id); + } } if (event.eventType === "UPDATE") { - let foundInList = false; - setConversations((prev) => { - if (!prev.some((c) => c.id === conv.id)) return prev; - foundInList = true; - return prev.map((c) => (c.id === conv.id ? { ...c, ...conv } : c)); - }); - if (!foundInList) { - // UPDATE arrived before the INSERT (or after a missed - // INSERT) — fetch the row so it surfaces with its contact. + if (knownConvIdsRef.current.has(conv.id)) { + setConversations((prev) => + prev.map((c) => (c.id === conv.id ? { ...c, ...conv } : c)), + ); + } else { + // UPDATE arrived before the INSERT (or after a missed INSERT) + // — fetch the row so it surfaces with its contact joined. The + // patch contained in `conv` will already be reflected in what + // the hydrate fetch returns. hydrateConversation(conv.id); } @@ -227,14 +257,59 @@ export default function InboxPage() { [activeConversation, hydrateConversation] ); - // Subscribe to realtime - useRealtime({ + // Subscribe to realtime. The `isConnected` flag below feeds the + // reconnect resync: realtime is best-effort and events sent while the + // WS was disconnected (laptop sleep, network blip, background-tab + // throttle) are simply lost. We need a way to catch up. + const { isConnected } = useRealtime({ channelName: "inbox-realtime", onMessageEvent: handleMessageEvent, onConversationEvent: handleConversationEvent, enabled: true, }); + /** + * Bump `resyncToken` whenever the realtime channel transitions from + * disconnected → connected *after* the initial connect. The initial + * connect is covered by the children's on-mount fetches; only later + * reconnects need a manual refetch to fill the gap. + * + * Tracked via a `was-connected` ref rather than a count so that React + * strict-mode's dev-only effect double-fire doesn't read as a + * reconnect. + */ + const wasConnectedRef = useRef(false); + const initialConnectDoneRef = useRef(false); + useEffect(() => { + if (isConnected && !wasConnectedRef.current) { + // false → true transition + if (initialConnectDoneRef.current) { + setResyncToken((n) => n + 1); + } else { + initialConnectDoneRef.current = true; + } + } + wasConnectedRef.current = isConnected; + }, [isConnected]); + + /** + * Refetch when the tab regains focus. Background tabs may have their + * WS throttled by the browser even without a full disconnect, so a + * visibilitychange → visible is a reliable signal that we may have + * missed events. Cheap to fire; the children dedupe on their own. + */ + useEffect(() => { + const onVisibility = () => { + if (document.visibilityState === "visible") { + setResyncToken((n) => n + 1); + } + }; + document.addEventListener("visibilitychange", onVisibility); + return () => { + document.removeEventListener("visibilitychange", onVisibility); + }; + }, []); + const handleConversationsLoaded = useCallback( (loaded: Conversation[]) => { setConversations(loaded); @@ -397,6 +472,7 @@ export default function InboxPage() { onSelect={handleSelectConversation} conversations={conversations} onConversationsLoaded={handleConversationsLoaded} + resyncToken={resyncToken} /> @@ -420,6 +496,7 @@ export default function InboxPage() { onStatusChange={handleStatusChange} onAssignChange={handleAssignChange} onBack={handleCloseConversation} + resyncToken={resyncToken} /> diff --git a/src/components/inbox/conversation-list.tsx b/src/components/inbox/conversation-list.tsx index e26f718..8e0486d 100644 --- a/src/components/inbox/conversation-list.tsx +++ b/src/components/inbox/conversation-list.tsx @@ -21,6 +21,13 @@ interface ConversationListProps { onSelect: (conversation: Conversation) => void; conversations: Conversation[]; onConversationsLoaded: (conversations: Conversation[]) => void; + /** + * Increment to force the fetch effect below to refire. The parent + * bumps this on realtime reconnect / tab visibility → visible so the + * list catches up on any events sent while the WS was disconnected + * or the tab was throttled. Optional so existing callers keep working. + */ + resyncToken?: number; } const STATUS_COLORS: Record = { @@ -41,6 +48,7 @@ export function ConversationList({ onSelect, conversations, onConversationsLoaded, + resyncToken = 0, }: ConversationListProps) { const [search, setSearch] = useState(""); const [filter, setFilter] = useState("all"); @@ -94,7 +102,10 @@ export function ConversationList({ return () => { cancelled = true; }; - }, []); + // `resyncToken` is included so the parent can force a refetch when + // the realtime channel reconnects or the tab regains focus — catches + // up on any events sent while the WS was disconnected or throttled. + }, [resyncToken]); const filtered = useMemo(() => { let result = conversations; diff --git a/src/components/inbox/message-thread.tsx b/src/components/inbox/message-thread.tsx index 88fec7e..fce4b03 100644 --- a/src/components/inbox/message-thread.tsx +++ b/src/components/inbox/message-thread.tsx @@ -70,6 +70,14 @@ interface MessageThreadProps { * mobile only. */ onBack?: () => void; + /** + * Increment to force the messages + reactions fetch effects to refire. + * Parent bumps this on realtime reconnect / tab visibility → visible + * so the open thread catches up on any events sent while the WS was + * disconnected or the tab was throttled. Optional so existing callers + * keep working. + */ + resyncToken?: number; } function formatDateSeparator(dateStr: string): string { @@ -112,6 +120,7 @@ export function MessageThread({ onStatusChange, onAssignChange, onBack, + resyncToken = 0, }: MessageThreadProps) { const { user } = useAuth(); const [loading, setLoading] = useState(false); @@ -219,12 +228,16 @@ export function MessageThread({ return () => { cancelled = true; }; - }, [conversationId]); + // `resyncToken` is included so the parent can force a refetch when + // the realtime channel reconnects or the tab regains focus — + // realtime is best-effort and any message events sent while the WS + // was disconnected or throttled are otherwise lost. + }, [conversationId, resyncToken]); - // Reactions: fetch + realtime per conversation. Subscribing here (not at - // the page level) keeps the channel scoped to the visible conversation, - // matching the message fetch effect above and avoiding cross-conversation - // chatter on a busy inbox. + // Reactions fetch — pulls the current state from the DB. Kept separate + // from the channel subscription below so a `resyncToken` bump just + // refetches the rows without also tearing down and rebuilding the + // realtime channel. useEffect(() => { if (!conversationId) { setReactions([]); @@ -246,6 +259,18 @@ export function MessageThread({ setReactions((data as MessageReaction[]) ?? []); })(); + return () => { + cancelled = true; + }; + }, [conversationId, resyncToken]); + + // Reactions realtime subscription per conversation. Subscribing here + // (not at the page level) keeps the channel scoped to the visible + // conversation and avoids cross-conversation chatter on a busy inbox. + useEffect(() => { + if (!conversationId) return; + const supabase = createClient(); + const channel = supabase .channel(`reactions:${conversationId}`) .on( @@ -308,7 +333,6 @@ export function MessageThread({ .subscribe(); return () => { - cancelled = true; supabase.removeChannel(channel); }; }, [conversationId]);