From 12bcafb52ca46c3e27099f4768d054549e018a93 Mon Sep 17 00:00:00 2001 From: ymichael <610102+ymichael@users.noreply.github.com> Date: Mon, 21 Sep 2026 10:37:12 -0700 Subject: [PATCH] Resync the sidebar after a mobile background gap ## Root cause Backgrounding the app on a phone kills the realtime socket. Recovery runs through `invalidateRealtimeQueriesAfterServerReconnect`, which refetches any realtime-owned query whose `dataUpdatedAt` predates `disconnectedAt`. Two independent defects stopped that from happening. `setQueryData` always stamps `dataUpdatedAt` with the write time, so the local cache patches that realtime pushes apply moved the sidebar's watermark forward. `disconnectedAt` is always pinned at or before the moment the page resumes, while the hidden-change flush stamps the sidebar at resume time, so the strict comparison always failed and the catch-up fetch was skipped. The skip required that patches landed during the gap, which required that a thread was running: the wave was suppressed precisely when it was needed. The sidebar kept rendering the last status it received before the socket died. `probeOrReconnect` then took ~30s rather than 5s to notice the dead socket. `sendPing` skips the probe when a frame arrived within `REALTIME_PONG_TIMEOUT_MS`, but `lastServerActivityAt` records when the app read the bytes, not when they crossed the network. On resume the OS delivers everything it buffered during the freeze, so the guard saw fresh activity from stale data and suppressed the probe that exists for that transition, deferring detection to the next 25s interval tick. ## Change `patchCachedQueryData` writes cache patches without advancing `dataUpdatedAt`, keeping that timestamp meaning "when this data last came from the server" as the reconnect watermark assumes. The sidebar navigation, thread list and timeline patch helpers use it, which covers the realtime and optimistic-mutation writers that funnel through them. Status pushes still patch one row instead of refetching the bootstrap document, so #2169 is unaffected. `probeOrReconnect` passes `ignoreRecentActivity`, since frame recency is only evidence of liveness if the page was awake for that window. The 25s interval keeps the guard. Only a `pong` now clears an outstanding probe. Frames are ordered on one connection, so a frame the server sent after our ping cannot overtake the pong; a non-pong frame arriving with the probe still outstanding was therefore sent before the ping landed and proves nothing about the socket now. The server answers every ping unconditionally (`client-protocol.ts`), and it serves the app bundle, so the two cannot be version-skewed. This narrows the "any inbound frame is liveness" rule from #1883 to the probe window and its test is updated accordingly; without it a straggler buffered frame cancels the reconnect and detection falls back to ~30s. ## Verification - `pnpm exec turbo run typecheck lint test --filter=@bb/app`: 552 files, 4959 tests pass, 0 lint errors (191 pre-existing warnings in untouched files). - Three regression tests, each confirmed failing before its fix: a hidden flush followed by a reconnect still invalidates the sidebar; the resume probe fires despite buffered frames; a stale frame does not answer a probe. > AGENT GENERATED Co-Authored-By: Claude Opus 5 (1M context) --- .../hooks/cache-owners/cache-effect-utils.ts | 14 +++++ .../hooks/cache-owners/project-cache-owner.ts | 7 ++- .../app/src/hooks/cache-owners/query-cache.ts | 17 ++++-- .../cache-owners/thread-list-cache-data.ts | 6 +- .../src/hooks/realtime-cache-effects.test.ts | 60 +++++++++++++++++++ apps/app/src/lib/ws.test.ts | 44 ++++++++++++-- apps/app/src/lib/ws.ts | 11 ++-- 7 files changed, 140 insertions(+), 19 deletions(-) diff --git a/apps/app/src/hooks/cache-owners/cache-effect-utils.ts b/apps/app/src/hooks/cache-owners/cache-effect-utils.ts index 68ea09b5231..019872340f8 100644 --- a/apps/app/src/hooks/cache-owners/cache-effect-utils.ts +++ b/apps/app/src/hooks/cache-owners/cache-effect-utils.ts @@ -1,3 +1,4 @@ +import type { QueryClient, QueryKey, Updater } from "@tanstack/react-query"; import type { QueryKeysArg } from "../cache-effect-types"; export function invalidateQueryKeys({ @@ -24,3 +25,16 @@ export function refetchFailedActiveQueryKeys({ .catch(() => {}); } } + +export function patchCachedQueryData( + queryClient: QueryClient, + queryKey: QueryKey, + updater: Updater, +): void { + const fetchedAt = queryClient.getQueryState(queryKey)?.dataUpdatedAt; + queryClient.setQueryData( + queryKey, + updater, + fetchedAt ? { updatedAt: fetchedAt } : undefined, + ); +} diff --git a/apps/app/src/hooks/cache-owners/project-cache-owner.ts b/apps/app/src/hooks/cache-owners/project-cache-owner.ts index 9603f8afc57..443e13f964a 100644 --- a/apps/app/src/hooks/cache-owners/project-cache-owner.ts +++ b/apps/app/src/hooks/cache-owners/project-cache-owner.ts @@ -9,6 +9,7 @@ import { sidebarNavigationQueryKey, } from "../queries/query-keys"; import { invalidateProjectDeleteQueries } from "./mutation-cache-effects"; +import { patchCachedQueryData } from "./cache-effect-utils"; interface ApplyProjectCreateResultArgs { project: ProjectResponse; @@ -115,7 +116,8 @@ export function applyProjectCreateResult({ ? applyProjectToProjectList(currentProjects, project) : [project], ); - queryClient.setQueryData( + patchCachedQueryData( + queryClient, sidebarNavigationQueryKey(), (currentNavigation) => currentNavigation @@ -135,7 +137,8 @@ export function applyProjectDeleteResult({ ? removeProjectFromProjectList(currentProjects, projectId) : currentProjects, ); - queryClient.setQueryData( + patchCachedQueryData( + queryClient, sidebarNavigationQueryKey(), (currentNavigation) => currentNavigation diff --git a/apps/app/src/hooks/cache-owners/query-cache.ts b/apps/app/src/hooks/cache-owners/query-cache.ts index 7e3a40b8460..bd0566d3a88 100644 --- a/apps/app/src/hooks/cache-owners/query-cache.ts +++ b/apps/app/src/hooks/cache-owners/query-cache.ts @@ -10,6 +10,7 @@ import { iterateThreadListCacheEntries, } from "./thread-list-cache-data"; import { bumpDiffPatchEvictionGeneration } from "./environment-diff-patch-cache-owner"; +import { patchCachedQueryData } from "./cache-effect-utils"; import { readCachedSidebarBootstrap } from "@/lib/sidebar-bootstrap-cache"; import type { SidebarBootstrapResponse, @@ -310,7 +311,8 @@ export function applyToCachedSidebarNavigationThreads({ mapper, queryClient, }: ApplyToCachedSidebarNavigationThreadsArgs): void { - queryClient.setQueryData( + patchCachedQueryData( + queryClient, sidebarNavigationQueryKey(), (currentNavigation) => { if (!currentNavigation) { @@ -398,7 +400,7 @@ export function restoreCachedSidebarNavigation( queryClient: QueryClient, snapshot: CachedSidebarNavigationSnapshot, ): void { - queryClient.setQueryData(sidebarNavigationQueryKey(), snapshot); + patchCachedQueryData(queryClient, sidebarNavigationQueryKey(), snapshot); } export function getEnvironmentRecordInvalidationQueryKeys({ @@ -627,14 +629,19 @@ export function optimisticallyInsertThread( continue; } - queryClient.setQueryData(queryKey, upsertThread(data)); + patchCachedQueryData( + queryClient, + queryKey, + upsertThread(data), + ); } if (thread.visibility === "hidden" || thread.archivedAt !== null) { return; } - queryClient.setQueryData( + patchCachedQueryData( + queryClient, sidebarNavigationQueryKey(), (navigation) => { if (!navigation) { @@ -680,7 +687,7 @@ function updateCachedTimelineRows({ continue; } - queryClient.setQueryData(queryKey, { + patchCachedQueryData(queryClient, queryKey, { ...response, rows: [...nextRows], }); diff --git a/apps/app/src/hooks/cache-owners/thread-list-cache-data.ts b/apps/app/src/hooks/cache-owners/thread-list-cache-data.ts index 206be41de7e..ae1dc972e07 100644 --- a/apps/app/src/hooks/cache-owners/thread-list-cache-data.ts +++ b/apps/app/src/hooks/cache-owners/thread-list-cache-data.ts @@ -4,6 +4,7 @@ import type { QueryKey, } from "@tanstack/react-query"; import type { ThreadListEntry } from "@bb/domain"; +import { patchCachedQueryData } from "./cache-effect-utils"; export type ThreadListCacheData = | ThreadListEntry[] @@ -93,7 +94,7 @@ export function restoreCachedThreadLists( snapshot: CachedThreadListSnapshot, ): void { for (const { queryKey, data } of snapshot) { - queryClient.setQueryData(queryKey, data); + patchCachedQueryData(queryClient, queryKey, data); } } @@ -104,7 +105,8 @@ export function applyToCachedThreadLists( for (const { queryKey, data } of getCachedThreadLists(queryClient, { queryKey: options.queryKey, })) { - queryClient.setQueryData( + patchCachedQueryData( + queryClient, queryKey, mapThreadListCacheData(data, options.mapper), ); diff --git a/apps/app/src/hooks/realtime-cache-effects.test.ts b/apps/app/src/hooks/realtime-cache-effects.test.ts index d075a6403b3..c0882681210 100644 --- a/apps/app/src/hooks/realtime-cache-effects.test.ts +++ b/apps/app/src/hooks/realtime-cache-effects.test.ts @@ -3010,6 +3010,66 @@ describe("createRealtimeCacheEffects", () => { effects.dispose(); }); + it("still resyncs the sidebar when a hidden flush precedes the reconnect", () => { + vi.useFakeTimers(); + const visibility = createFakeVisibility(); + const { effects, queryClient } = + createRealtimeEffectsTestContext(visibility); + const sidebarNavigationKey = sidebarNavigationQueryKey(); + queryClient.setQueryData(sidebarNavigationKey, { + projects: [ + { + threads: [ + { + activity: NO_THREAD_ACTIVITY, + id: "thr_1", + latestAttentionAt: 100, + runtime: { + displayStatus: "idle", + hostReconnectGraceExpiresAt: null, + }, + status: "idle", + updatedAt: 100, + }, + ], + }, + ], + personalProject: { threads: [] }, + }); + vi.advanceTimersByTime(1000); + const disconnectedAt = Date.now(); + + visibility.setVisible(false); + vi.advanceTimersByTime(1000); + effects.handleChanged({ + type: "changed", + entity: "thread", + id: "thr_1", + metadata: { + projectId: "project-1", + statusChange: { + activity: NO_THREAD_ACTIVITY, + latestAttentionAt: 200, + runtime: { + displayStatus: "active", + hostReconnectGraceExpiresAt: null, + }, + status: "active", + updatedAt: 200, + }, + }, + changes: ["status-changed"], + }); + vi.advanceTimersByTime(60_000); + visibility.setVisible(true); + effects.handleConnected({ reconnected: true, disconnectedAt }); + + expect( + queryClient.getQueryState(sidebarNavigationKey)?.isInvalidated, + ).toBe(true); + effects.dispose(); + }); + it("holds a debounce that elapses hidden and non-thread changes until visible", async () => { vi.useFakeTimers(); const visibility = createFakeVisibility(); diff --git a/apps/app/src/lib/ws.test.ts b/apps/app/src/lib/ws.test.ts index 39a47f7159b..4ea75ec7829 100644 --- a/apps/app/src/lib/ws.test.ts +++ b/apps/app/src/lib/ws.test.ts @@ -400,12 +400,7 @@ describe("WebSocketManager liveness", () => { vi.advanceTimersByTime(REALTIME_PING_INTERVAL_MS); expect(pingCount(socket)).toBe(1); - receive(socket, { - type: "changed", - entity: "thread", - id: "thr_1", - changes: ["events-appended"], - }); + receive(socket, { type: "pong" }); vi.advanceTimersByTime(REALTIME_PONG_TIMEOUT_MS); expect(fakeSocketState.instances).toHaveLength(1); }); @@ -424,6 +419,43 @@ describe("WebSocketManager liveness", () => { expect(pingCount(socket)).toBe(2); }); + it("probes on becoming visible even when buffered frames land on resume", () => { + const { browserEvents, socket } = createLiveManager(); + + browserEvents.setVisible(false); + vi.advanceTimersByTime(REALTIME_PING_INTERVAL_MS * 4); + receive(socket, { + type: "changed", + entity: "thread", + id: "thr_1", + changes: ["events-appended"], + }); + browserEvents.setVisible(true); + + expect(pingCount(socket)).toBe(1); + vi.advanceTimersByTime(REALTIME_PONG_TIMEOUT_MS); + expect(fakeSocketState.instances).toHaveLength(2); + }); + + it("replaces the socket when a stale frame arrives instead of a pong", () => { + const { browserEvents, socket } = createLiveManager(); + + browserEvents.setVisible(false); + vi.advanceTimersByTime(REALTIME_PING_INTERVAL_MS * 4); + browserEvents.setVisible(true); + expect(pingCount(socket)).toBe(1); + + receive(socket, { + type: "changed", + entity: "thread", + id: "thr_1", + changes: ["events-appended"], + }); + vi.advanceTimersByTime(REALTIME_PONG_TIMEOUT_MS); + + expect(fakeSocketState.instances).toHaveLength(2); + }); + it("reconnects immediately on visible or online when the socket is closed", () => { const { browserEvents, connectedEvents, socket } = createLiveManager(); diff --git a/apps/app/src/lib/ws.ts b/apps/app/src/lib/ws.ts index b8d84210db3..6bf5d8ecc0b 100644 --- a/apps/app/src/lib/ws.ts +++ b/apps/app/src/lib/ws.ts @@ -201,7 +201,7 @@ export class WebSocketManager { } switch (this.socket.readyState) { case WebSocket.OPEN: - this.sendPing(); + this.sendPing({ ignoreRecentActivity: true }); return; case WebSocket.CONNECTING: return; @@ -230,11 +230,14 @@ export class WebSocketManager { this.clearPongTimer(); } - private sendPing(): void { + private sendPing(options?: { ignoreRecentActivity: boolean }): void { if (this.socket?.readyState !== WebSocket.OPEN) { return; } - if (Date.now() - this.lastServerActivityAt < REALTIME_PONG_TIMEOUT_MS) { + if ( + !options?.ignoreRecentActivity && + Date.now() - this.lastServerActivityAt < REALTIME_PONG_TIMEOUT_MS + ) { return; } this.sendMessage({ type: "ping" }); @@ -256,7 +259,6 @@ export class WebSocketManager { private noteServerActivity(): void { this.lastServerActivityAt = Date.now(); - this.clearPongTimer(); } private markSocketLost(at: number): void { @@ -276,6 +278,7 @@ export class WebSocketManager { } if (pongMessageLenientSchema.safeParse(parsed).success) { + this.clearPongTimer(); return; }