diff --git a/apps/app/src/components/thread/timeline/TimelineTitleView.tsx b/apps/app/src/components/thread/timeline/TimelineTitleView.tsx index d6eaf56605..fbf93ab58f 100644 --- a/apps/app/src/components/thread/timeline/TimelineTitleView.tsx +++ b/apps/app/src/components/thread/timeline/TimelineTitleView.tsx @@ -1,4 +1,4 @@ -import { Fragment, useEffect, useState } from "react"; +import { Fragment } from "react"; import type { KeyboardEvent, MouseEvent, ReactNode } from "react"; import { assertNever, @@ -15,6 +15,7 @@ import { import { cn } from "@bb/shared-ui/lib/utils"; import { DiffStatsTally } from "@/components/ui/diff-stats-tally.js"; import { RouteAnchor } from "@/components/ui/app-route-anchor.js"; +import { useSecondTick } from "@/hooks/useSecondTick"; /** * Resolves a title's declared action to a click callback. Return `null` to @@ -209,26 +210,20 @@ function renderSegment( } /** - * Ticks the displayed elapsed time locally while the row is still active. - * The truth is `startedAt` (the wall-clock when the work began); the App - * derives `now - startedAt` and ticks once per second until the row reaches - * a terminal status (at which point a static `completedAt - startedAt` is - * shown by the caller instead). Stays empty until the elapsed time crosses - * the visible threshold (>1s) to avoid sub-second flicker on row entry. + * Ticks the displayed elapsed time while the row is still active. The truth + * is `startedAt` (the wall-clock when the work began); the App derives + * `now - startedAt` from the shared 1 Hz ticker — one interval for every + * in-flight row on screen, paused while the document is hidden — until the + * row reaches a terminal status (at which point a static + * `completedAt - startedAt` is shown by the caller instead). Stays empty + * until the elapsed time crosses the visible threshold (>1s) to avoid + * sub-second flicker on row entry. */ function LiveDurationText({ startedAt }: { startedAt: number }) { - const [tick, setTick] = useState(() => Date.now() - startedAt); + const elapsedMs = useSecondTick() - startedAt; - useEffect(() => { - setTick(Date.now() - startedAt); - const interval = window.setInterval(() => { - setTick(Date.now() - startedAt); - }, 1_000); - return () => window.clearInterval(interval); - }, [startedAt]); - - if (tick <= 1_000) return null; - return <>{durationToCompactString(tick)}; + if (elapsedMs <= 1_000) return null; + return <>{durationToCompactString(elapsedMs)}; } function renderDecoration( diff --git a/apps/app/src/hooks/cache-owners/realtime-cache-registry.ts b/apps/app/src/hooks/cache-owners/realtime-cache-registry.ts index a4a849dee4..99ed87dd44 100644 --- a/apps/app/src/hooks/cache-owners/realtime-cache-registry.ts +++ b/apps/app/src/hooks/cache-owners/realtime-cache-registry.ts @@ -167,6 +167,17 @@ interface ThrottledActiveRefetchArgs { */ const WORK_STATUS_REFETCH_MIN_INTERVAL_MS = 1_000; +/** + * A `status-changed` push without a row snapshot falls back to refetching the + * active thread lists. Bare pushes arrive in bursts — writers inside a + * transaction publish one per thread, and a host disconnect used to publish + * one per thread on the host — and every full list response is ~1 KB per + * unarchived thread, so the fallback refetch is throttled to one per second + * per query. Rows still go stale immediately; only the active refetch + * coalesces. + */ +const THREAD_LIST_STATUS_FALLBACK_REFETCH_MIN_INTERVAL_MS = 1_000; + /** * The trailing refetch is self-clocking: it fires as soon as the in-flight * fetch settles and any event arrived meanwhile. During a streaming turn events @@ -831,6 +842,54 @@ function dirtyActiveThreadListQueries({ return [sidebarNavigationQueryKey(), threadSearchQueryKeyPrefix()]; } +/** + * Same scope as {@link dirtyActiveThreadListQueries}, but the active refetches + * run through the throttle machinery: everything goes stale immediately (a + * remount refetches), while active list/sidebar/search observers refetch at + * most once per {@link THREAD_LIST_STATUS_FALLBACK_REFETCH_MIN_INTERVAL_MS}, + * with later changes coalescing into one trailing refetch and no fetch in + * flight ever cancelled. Archived pages keep their stale-only treatment. + */ +function dirtyActiveThreadListQueriesWithThrottledRefetch({ + projectId, + queryClient, +}: ThreadRealtimeDirtyContext): void { + const listQueryKeys = projectId + ? [ + ...getCachedProjectThreadListInvalidationQueryKeys({ + projectId, + queryClient, + }), + ...getCachedGlobalThreadListInvalidationQueryKeys({ queryClient }), + ] + : getCachedThreadListQueryKeys(queryClient); + for (const queryKey of listQueryKeys) { + if (isArchivedThreadListQueryKey(queryKey)) { + queryClient.invalidateQueries({ + exact: true, + queryKey, + refetchType: "none", + }); + continue; + } + invalidateQueryKeyWithThrottledActiveRefetch({ + minIntervalMs: THREAD_LIST_STATUS_FALLBACK_REFETCH_MIN_INTERVAL_MS, + queryClient, + queryKey, + }); + } + for (const queryKey of [ + sidebarNavigationQueryKey(), + threadSearchQueryKeyPrefix(), + ]) { + invalidateQueryKeyWithThrottledActiveRefetch({ + minIntervalMs: THREAD_LIST_STATUS_FALLBACK_REFETCH_MIN_INTERVAL_MS, + queryClient, + queryKey, + }); + } +} + function dirtyThreadListQueriesForBackgroundActivity( context: ThreadRealtimeDirtyContext, ): QueryKey[] { @@ -1108,7 +1167,8 @@ function patchThreadListPendingInteractionState({ * patched in place. The alternative is what the fallback still does for * pushes without the row (older servers, writers inside a transaction that * cannot resolve the runtime): refetch every active thread list plus the - * sidebar bootstrap, which is ~1 KB per unarchived thread, twice per turn. + * sidebar bootstrap, which is ~1 KB per unarchived thread — throttled to one + * active refetch per second so a burst of bare pushes coalesces. * * A list fetch already in flight read the database before this transition * and would overwrite the patch when it lands, so those queries are @@ -1116,16 +1176,26 @@ function patchThreadListPendingInteractionState({ */ function patchThreadListStatusState( context: ThreadRealtimeDirtyContext, -): QueryKey[] { - const { queryClient, statusChange, threadId } = context; +): void { + const { flushOnce, queryClient, statusChange, threadId } = context; if (!threadId || !statusChange) { - return dirtyActiveThreadListQueries(context); + dirtyActiveThreadListQueriesWithThrottledRefetch(context); + return; } updateCachedThreadListStatusState(queryClient, threadId, statusChange); for (const queryKey of getFetchingThreadListQueryKeys(queryClient)) { queryClient.invalidateQueries({ exact: true, queryKey }); } - return [threadSearchQueryKeyPrefix()]; // Result rows render status but are not list-shaped. + // Result rows render status but are not list-shaped, so search refreshes + // rather than patches — once per flush and without aborting a request in + // flight: status changes ride the immediate path, and the default + // cancelling invalidation could starve an open search on a slow link. + if (flushOnce("thread-search:status-changed")) { + queryClient.invalidateQueries( + { queryKey: threadSearchQueryKeyPrefix() }, + { cancelRefetch: false }, + ); + } } function dirtyEnvironmentRecordQueries( diff --git a/apps/app/src/hooks/queries/query-policies.ts b/apps/app/src/hooks/queries/query-policies.ts index 803f186ffb..4e6ca17406 100644 --- a/apps/app/src/hooks/queries/query-policies.ts +++ b/apps/app/src/hooks/queries/query-policies.ts @@ -21,12 +21,25 @@ export const SERVER_SESSION_QUERY_POLICY = { staleTime: SERVER_SESSION_STALE_TIME_MS, } as const; +/** + * Live values with no realtime change kind (provider usage limits): focus and + * reconnect are their only freshness sources, so the explicit `true`s + * deliberately bypass the app-level lost-realtime-coverage gate that + * `createAppQueryClient` applies to the defaults. + */ export const FOCUS_OWNED_LIVE_QUERY_POLICY = { refetchOnReconnect: true, refetchOnWindowFocus: true, staleTime: FOCUS_OWNED_LIVE_STALE_TIME_MS, } as const; +/** + * Explicit resume opt-in for queries whose realtime coverage has gaps: thread + * tabs are absent from the reconnect-watermark catch-up list, and the thread + * host file preview backs an open pane that must not keep stale bytes after + * an offline stretch. Deliberately bypasses the app-level + * lost-realtime-coverage gate (per-query options win over the defaults). + */ export const RESUME_REFETCH_QUERY_POLICY = { refetchOnReconnect: true, refetchOnWindowFocus: true, diff --git a/apps/app/src/hooks/realtime-cache-effects.test.ts b/apps/app/src/hooks/realtime-cache-effects.test.ts index 284b7e378e..95b15478fb 100644 --- a/apps/app/src/hooks/realtime-cache-effects.test.ts +++ b/apps/app/src/hooks/realtime-cache-effects.test.ts @@ -42,6 +42,7 @@ import { import { pluginContributionsQueryKey } from "./queries/query-keys"; import { createRealtimeCacheEffects, + resolveThreadInvalidationDebounce, type RealtimeCacheEffectsVisibility, } from "./realtime-cache-effects"; import { @@ -569,6 +570,73 @@ describe("createRealtimeCacheEffects", () => { effects.dispose(); }); + it("does not abort an in-flight search when a status change patches the row", async () => { + vi.useFakeTimers(); + const { effects, queryClient } = createRealtimeEffectsTestContext(); + const threadSearchKey = threadSearchQueryKey({ + limitPerGroup: 20, + query: "needle", + }); + // Cached data matters: the default cancelling invalidation only aborts + // and re-issues a fetch when the query already holds data — exactly the + // open-search-refreshing case a streaming turn's status flips would starve. + queryClient.setQueryData(threadSearchKey, { + active: { results: [], total: 0 }, + archived: { results: [], total: 0 }, + }); + const signals: AbortSignal[] = []; + const resolveFetches: Array<(value: unknown) => void> = []; + const searchQueryFn = vi.fn(({ signal }: { signal: AbortSignal }) => { + signals.push(signal); + return new Promise((resolve) => { + resolveFetches.push(resolve); + }); + }); + const searchObserver = new QueryObserver(queryClient, { + queryKey: threadSearchKey, + queryFn: searchQueryFn, + staleTime: Infinity, + }); + const unsubscribeSearch = searchObserver.subscribe(() => {}); + void searchObserver.refetch(); + await vi.advanceTimersByTimeAsync(0); + expect(searchQueryFn).toHaveBeenCalledTimes(1); + + effects.handleChanged({ + type: "changed", + entity: "thread", + id: "thr_1", + metadata: { + projectId: "project-1", + statusChange: { + activity: NO_THREAD_ACTIVITY, + latestAttentionAt: 100, + runtime: { + displayStatus: "active", + hostReconnectGraceExpiresAt: null, + }, + status: "active", + updatedAt: 200, + }, + }, + changes: ["status-changed"], + }); + await vi.advanceTimersByTimeAsync(0); + + // Status changes ride the immediate path, so a cancelling invalidation + // here could starve an open search forever; the request keeps running. + expect(signals[0]?.aborted).toBe(false); + expect(searchQueryFn).toHaveBeenCalledTimes(1); + + resolveFetches[0]?.({ + active: { results: [], total: 0 }, + archived: { results: [], total: 0 }, + }); + await vi.advanceTimersByTimeAsync(0); + unsubscribeSearch(); + effects.dispose(); + }); + it("marks the timeline of an unviewed thread stale without scheduling a refetch", async () => { vi.useFakeTimers(); const { effects, queryClient } = createRealtimeEffectsTestContext(); @@ -2082,6 +2150,56 @@ describe("createRealtimeCacheEffects", () => { effects.dispose(); }); + it("throttles the metadata-less status fallback to one active refetch per second", async () => { + vi.useFakeTimers(); + const { effects, queryClient } = createRealtimeEffectsTestContext(); + const sidebarNavigationKey = sidebarNavigationQueryKey(); + const sidebarQueryFn = vi.fn(async () => ({ + projects: [{ threads: [{ id: "thr_1", status: "idle" }] }], + personalProject: { threads: [] }, + })); + const observer = new QueryObserver(queryClient, { + queryKey: sidebarNavigationKey, + queryFn: sidebarQueryFn, + staleTime: Infinity, + }); + const unsubscribe = observer.subscribe(() => {}); + await vi.advanceTimersByTimeAsync(0); + expect(sidebarQueryFn).toHaveBeenCalledTimes(1); + + const emitBareStatusChange = () => { + effects.handleChanged({ + type: "changed", + entity: "thread", + id: "thr_1", + metadata: { projectId: "project-1" }, + changes: ["status-changed"], + }); + }; + + // The first bare push after a quiet period still refetches immediately. + emitBareStatusChange(); + await vi.advanceTimersByTimeAsync(0); + expect(sidebarQueryFn).toHaveBeenCalledTimes(2); + + // A second push inside the same second coalesces instead of fanning out… + await vi.advanceTimersByTimeAsync(100); + emitBareStatusChange(); + await vi.advanceTimersByTimeAsync(100); + expect(sidebarQueryFn).toHaveBeenCalledTimes(2); + // …while the row still goes stale immediately for the next mount. + expect(queryClient.getQueryState(sidebarNavigationKey)?.isInvalidated).toBe( + true, + ); + + // The coalesced change lands as one trailing refetch at the 1s boundary. + await vi.advanceTimersByTimeAsync(1_000); + expect(sidebarQueryFn).toHaveBeenCalledTimes(3); + + unsubscribe(); + effects.dispose(); + }); + it("refetches over a patched row when a bare status-changed arrives while visible", async () => { // Stop requests, command failures and host interruptions push the bare // kind. On the visible path status-changed never enters the debounce @@ -2446,6 +2564,71 @@ describe("createRealtimeCacheEffects", () => { effects.dispose(); }); + it("keeps the fine-pointer cadence and widens it for coarse pointers", () => { + expect(resolveThreadInvalidationDebounce(false)).toEqual({ + debounceMs: 50, + maxWaitMs: 200, + }); + expect(resolveThreadInvalidationDebounce(true)).toEqual({ + debounceMs: 150, + maxWaitMs: 400, + }); + }); + + it("widens the thread invalidation debounce on coarse pointers", async () => { + vi.useFakeTimers(); + // The pointer class is read from matchMedia once at module init, so the + // coarse branch needs a fresh module instance with a stubbed window. + // `location` rides along because the re-imported graph reaches the sdk + // module, which resolves its base URL from the window at init. + vi.stubGlobal("window", { + location: { origin: "http://localhost" }, + matchMedia: (query: string) => ({ + matches: query === "(pointer: coarse)", + }), + }); + vi.resetModules(); + try { + const coarseModule = await import("./realtime-cache-effects"); + const queryClient = createAppQueryClient({ + defaultOptions: { + queries: { + gcTime: Infinity, + retry: false, + }, + }, + showMutationErrorToasts: false, + }); + const effects = coarseModule.createRealtimeCacheEffects({ queryClient }); + const timelineKey = threadTimelineQueryKey("thr_1"); + queryClient.setQueryData(timelineKey, { rows: [] }); + + effects.handleChanged({ + type: "changed", + entity: "thread", + id: "thr_1", + metadata: { + eventTypes: ["item/agentMessage/delta"], + projectId: "project-1", + }, + changes: ["events-appended"], + }); + + // The fine-pointer cadence would have flushed at 50 ms. + vi.advanceTimersByTime(50); + expect(queryClient.getQueryState(timelineKey)?.isInvalidated).not.toBe( + true, + ); + vi.advanceTimersByTime(100); + expect(queryClient.getQueryState(timelineKey)?.isInvalidated).toBe(true); + + effects.dispose(); + } finally { + vi.unstubAllGlobals(); + vi.resetModules(); + } + }); + it("applies the reconnect watermark from the connected event", () => { const { effects, queryClient } = createRealtimeEffectsTestContext(); const disconnectedAt = Date.now(); diff --git a/apps/app/src/hooks/realtime-cache-effects.ts b/apps/app/src/hooks/realtime-cache-effects.ts index 215748164a..5e13dc4eff 100644 --- a/apps/app/src/hooks/realtime-cache-effects.ts +++ b/apps/app/src/hooks/realtime-cache-effects.ts @@ -34,8 +34,40 @@ import { partitionThreadChangesByFlushPriority, } from "./cache-owners/realtime-cache-registry"; -const INVALIDATION_DEBOUNCE_MS = 50; -const INVALIDATION_MAX_WAIT_MS = 200; +interface ThreadInvalidationDebounce { + debounceMs: number; + maxWaitMs: number; +} + +/** + * Streaming publishes arrive up to every 50 ms per thread, and each flush + * reconciles the whole unwindowed timeline/list state. Desktops absorb the + * 50/200 cadence (up to 20 reconciles/s); on coarse-pointer (touch) devices + * the same cadence competes with scroll and input handling on a phone core, + * so the window widens to 150/400 — still well inside perceived-live + * territory. Exported for tests; production reads the pointer class once at + * module init (it does not change mid-session). + */ +export function resolveThreadInvalidationDebounce( + isCoarsePointer: boolean, +): ThreadInvalidationDebounce { + return isCoarsePointer + ? { debounceMs: 150, maxWaitMs: 400 } + : { debounceMs: 50, maxWaitMs: 200 }; +} + +function detectCoarsePointer(): boolean { + return ( + typeof window !== "undefined" && + typeof window.matchMedia === "function" && + window.matchMedia("(pointer: coarse)").matches + ); +} + +const { + debounceMs: INVALIDATION_DEBOUNCE_MS, + maxWaitMs: INVALIDATION_MAX_WAIT_MS, +} = resolveThreadInvalidationDebounce(detectCoarsePointer()); const ENVIRONMENT_INVALIDATION_DEBOUNCE_MS = 250; const ENVIRONMENT_INVALIDATION_MAX_WAIT_MS = 500; diff --git a/apps/app/src/hooks/useSecondTick.test.ts b/apps/app/src/hooks/useSecondTick.test.ts new file mode 100644 index 0000000000..edc8ac3383 --- /dev/null +++ b/apps/app/src/hooks/useSecondTick.test.ts @@ -0,0 +1,71 @@ +// @vitest-environment jsdom + +import { act, renderHook } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useSecondTick } from "./useSecondTick"; + +function setDocumentVisibility(state: "hidden" | "visible"): void { + Object.defineProperty(document, "visibilityState", { + configurable: true, + value: state, + }); + document.dispatchEvent(new Event("visibilitychange")); +} + +describe("useSecondTick", () => { + afterEach(() => { + vi.useRealTimers(); + Reflect.deleteProperty(document, "visibilityState"); + }); + + it("pauses the shared ticker while hidden and jumps to now on resume", () => { + vi.useFakeTimers(); + const { result, unmount } = renderHook(() => useSecondTick()); + const initial = result.current; + + act(() => { + vi.advanceTimersByTime(1_000); + }); + expect(result.current).toBe(initial + 1_000); + + // Hidden: the interval stops entirely — no timer wakes a suspended phone + // to re-render durations nothing can see. + act(() => { + setDocumentVisibility("hidden"); + }); + act(() => { + vi.advanceTimersByTime(5_000); + }); + expect(result.current).toBe(initial + 1_000); + + // Visible again: one immediate tick jumps durations to current truth + // instead of waiting out the next second, then the cadence resumes. + act(() => { + setDocumentVisibility("visible"); + }); + expect(result.current).toBe(initial + 6_000); + + act(() => { + vi.advanceTimersByTime(1_000); + }); + expect(result.current).toBe(initial + 7_000); + + unmount(); + }); + + it("shares one interval across subscribers and stops with the last one", () => { + vi.useFakeTimers(); + const first = renderHook(() => useSecondTick()); + const second = renderHook(() => useSecondTick()); + + act(() => { + vi.advanceTimersByTime(1_000); + }); + // One shared tick value, not two phase-shifted timers. + expect(first.result.current).toBe(second.result.current); + + first.unmount(); + second.unmount(); + expect(vi.getTimerCount()).toBe(0); + }); +}); diff --git a/apps/app/src/hooks/useSecondTick.ts b/apps/app/src/hooks/useSecondTick.ts index e96c0b3fe0..aa6c1003e4 100644 --- a/apps/app/src/hooks/useSecondTick.ts +++ b/apps/app/src/hooks/useSecondTick.ts @@ -1,4 +1,8 @@ import { useSyncExternalStore } from "react"; +import { + isDocumentVisible, + subscribeToDocumentVisibility, +} from "@/lib/document-visibility"; /** * One 1 Hz ticker shared by every live-duration label. Each label used to own @@ -6,27 +10,61 @@ import { useSyncExternalStore } from "react"; * mounted that is many timers firing at slightly different phases, each a * separate render. One interval, one notification per second, and it stops * when the last subscriber leaves. + * + * The interval also stops while the document is hidden: nothing it drives can + * be seen, and on phones the pending timer only queues work for the resume. + * The first tick after becoming visible fires immediately so durations jump + * to the current truth instead of waiting out the next second. */ const listeners = new Set<() => void>(); let lastTickMs = 0; let intervalId: ReturnType | null = null; +let unsubscribeVisibility: (() => void) | null = null; function tick(): void { lastTickMs = Date.now(); for (const listener of listeners) listener(); } +function startInterval(): void { + if (intervalId === null && isDocumentVisible()) { + intervalId = setInterval(tick, 1_000); + } +} + +function stopInterval(): void { + if (intervalId !== null) { + clearInterval(intervalId); + intervalId = null; + } +} + +function handleVisibilityChange(): void { + if (!isDocumentVisible()) { + stopInterval(); + return; + } + if (listeners.size > 0 && intervalId === null) { + tick(); + startInterval(); + } +} + function subscribe(listener: () => void): () => void { if (listeners.size === 0) { lastTickMs = Date.now(); - intervalId = setInterval(tick, 1_000); + unsubscribeVisibility = subscribeToDocumentVisibility( + handleVisibilityChange, + ); + startInterval(); } listeners.add(listener); return () => { listeners.delete(listener); - if (listeners.size === 0 && intervalId !== null) { - clearInterval(intervalId); - intervalId = null; + if (listeners.size === 0) { + stopInterval(); + unsubscribeVisibility?.(); + unsubscribeVisibility = null; } }; } diff --git a/apps/app/src/lib/query-client.test.ts b/apps/app/src/lib/query-client.test.ts index 66b1b187da..6d49d3e16f 100644 --- a/apps/app/src/lib/query-client.test.ts +++ b/apps/app/src/lib/query-client.test.ts @@ -194,6 +194,87 @@ describe("createAppQueryClient", () => { queryClient.clear(); }); + it("keeps the default reconnect refetch when no gate is configured", async () => { + const queryClient = createAppQueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + showMutationErrorToasts: false, + }); + queryClient.mount(); + + const queryFn = vi.fn(() => Promise.resolve("data")); + const observer = new QueryObserver(queryClient, { + queryKey: ["reconnect-ungated"], + queryFn, + staleTime: 0, + }); + const unsubscribe = observer.subscribe(() => {}); + + await vi.waitFor(() => { + expect(observer.getCurrentResult().data).toBe("data"); + }); + + window.dispatchEvent(new Event("offline")); + window.dispatchEvent(new Event("online")); + await vi.waitFor(() => { + expect(queryFn).toHaveBeenCalledTimes(2); + }); + + unsubscribe(); + queryClient.unmount(); + queryClient.clear(); + }); + + it("skips the default reconnect refetch while the gate reports realtime coverage", async () => { + let realtimeConnected = true; + const queryClient = createAppQueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + shouldRefetchOnWindowFocus: () => !realtimeConnected, + showMutationErrorToasts: false, + }); + queryClient.mount(); + + const queryFn = vi.fn(() => Promise.resolve("data")); + const observer = new QueryObserver(queryClient, { + queryKey: ["reconnect-gated"], + queryFn, + // Instantly stale so a permitted reconnect refetch always fires. + staleTime: 0, + }); + const unsubscribe = observer.subscribe(() => {}); + + await vi.waitFor(() => { + expect(observer.getCurrentResult().data).toBe("data"); + }); + expect(queryFn).toHaveBeenCalledTimes(1); + + // Connected: realtime owns freshness, so the browser `online` blip that + // mobile Safari fires around suspensions must not refetch. + window.dispatchEvent(new Event("offline")); + window.dispatchEvent(new Event("online")); + await Promise.resolve(); + expect(queryFn).toHaveBeenCalledTimes(1); + + // Coverage lost: the reconnect refetch is the freshness fallback again. + realtimeConnected = false; + window.dispatchEvent(new Event("offline")); + window.dispatchEvent(new Event("online")); + await vi.waitFor(() => { + expect(queryFn).toHaveBeenCalledTimes(2); + }); + + unsubscribe(); + queryClient.unmount(); + queryClient.clear(); + }); + it("resumes a suspend-cancelled fetch that no focus refetch would restart", async () => { const queryClient = createAppQueryClient({ defaultOptions: { diff --git a/apps/app/src/lib/query-client.ts b/apps/app/src/lib/query-client.ts index 5f7c905576..e7eeb00063 100644 --- a/apps/app/src/lib/query-client.ts +++ b/apps/app/src/lib/query-client.ts @@ -18,14 +18,16 @@ interface CreateAppQueryClientOptions { defaultOptions?: QueryClientConfig["defaultOptions"]; showMutationErrorToasts?: boolean; /** - * Gate for the default focus refetch. Focus refetch is the freshness - * fallback for when realtime coverage is lost; while the socket is - * connected, change events keep the cache correct and the reconnect - * watermark repairs any gap, so a focus event (every phone unlock and - * app switch) must not refetch every active query on top of that wave. - * Defaults to always refetching. A `defaultOptions.queries.refetchOnWindowFocus` - * passed alongside this gate wins over it (caller defaults are spread last), - * so pass one or the other. + * Gate for the default focus and reconnect refetches. Both are the + * freshness fallback for when realtime coverage is lost; while the socket + * is connected, change events keep the cache correct and the reconnect + * watermark repairs any gap, so neither a focus event (every phone unlock + * and app switch) nor a browser `online` event (mobile Safari re-fires it + * around the same suspensions) must refetch every active query on top of + * that wave. Defaults to always refetching. A + * `defaultOptions.queries.refetchOnWindowFocus`/`refetchOnReconnect` + * passed alongside this gate wins over it (caller defaults are spread + * last), so pass one or the other. */ shouldRefetchOnWindowFocus?: () => boolean; } @@ -149,6 +151,10 @@ export function createAppQueryClient( shouldRefetchOnWindowFocus === undefined ? true : () => shouldRefetchOnWindowFocus(), + refetchOnReconnect: + shouldRefetchOnWindowFocus === undefined + ? true + : () => shouldRefetchOnWindowFocus(), retry: shouldRetryTransientReadQuery, retryDelay: TRANSIENT_READ_RETRY_DELAY_MS, ...defaultOptions?.queries, diff --git a/apps/app/src/main.tsx b/apps/app/src/main.tsx index 4d1c8fd865..f28358c4db 100644 --- a/apps/app/src/main.tsx +++ b/apps/app/src/main.tsx @@ -31,8 +31,9 @@ Error.stackTraceLimit = 50; const queryClient = createAppQueryClient({ // While the realtime socket is connected, change events and the reconnect - // watermark own cache freshness; a focus refetch on top would re-request - // every active query on each phone unlock and app switch. + // watermark own cache freshness; a focus or browser-online refetch on top + // would re-request every active query on each phone unlock, app switch, + // and mobile-Safari `online` blip. shouldRefetchOnWindowFocus: () => wsManager.getConnectionState() !== "connected", }); diff --git a/apps/server/src/internal/session-owner-side-effects.ts b/apps/server/src/internal/session-owner-side-effects.ts index 7bb6b5d0dc..f509c05dc1 100644 --- a/apps/server/src/internal/session-owner-side-effects.ts +++ b/apps/server/src/internal/session-owner-side-effects.ts @@ -1,6 +1,7 @@ import { eq } from "drizzle-orm"; import { closeSession, + getThread, hostDaemonSessions, listHostThreadIds, type HostDaemonSessionRow, @@ -18,6 +19,7 @@ import { interruptActiveThreadsForHost, reconcileDaemonReportedThreads, } from "../services/threads/thread-lifecycle.js"; +import { buildThreadStatusChangeMetadata } from "../services/threads/thread-runtime-display.js"; import { settleDanglingBackgroundTasks } from "../services/threads/background-task-reconciliation.js"; const DAEMON_RESTARTED_PENDING_INTERACTION_REASON = @@ -32,12 +34,18 @@ type DaemonSocketClosedDeps = Pick< | "hub" | "logger" | "pendingInteractions" + | "providerRegistry" | "sharedPorts" | "terminalSessions" >; type DaemonDisconnectGraceDeps = Pick< AppDeps, - "db" | "hub" | "logger" | "pendingInteractions" | "terminalSessions" + | "db" + | "hub" + | "logger" + | "pendingInteractions" + | "providerRegistry" + | "terminalSessions" >; interface HandleHostSessionOpenedArgs { @@ -236,7 +244,10 @@ function completeDaemonDisconnectGrace( } function completeDaemonActiveWorkDisconnectGrace( - deps: Pick, + deps: Pick< + AppDeps, + "db" | "hub" | "logger" | "pendingInteractions" | "providerRegistry" + >, args: CompleteDaemonActiveWorkDisconnectGraceArgs, ): void { if (deps.hub.hasDaemonForHost(args.hostId)) { @@ -249,12 +260,27 @@ function completeDaemonActiveWorkDisconnectGrace( }); } +/** + * Host connectivity is part of every thread row's displayed runtime, so each + * notification carries the post-change `statusChange` snapshot: without it, + * every client falls back to refetching every active thread list once per + * thread on this host, twice per disconnect (close + grace). + */ function notifyHostThreadRuntimeStatusChanged( - deps: Pick, + deps: Pick, hostId: string, ): void { for (const threadId of listHostThreadIds(deps.db, { hostId })) { - deps.hub.notifyThread(threadId, ["status-changed"]); + const thread = getThread(deps.db, threadId); + if (!thread) { + deps.hub.notifyThread(threadId, ["status-changed"]); + continue; + } + deps.hub.notifyThread( + threadId, + ["status-changed"], + buildThreadStatusChangeMetadata(deps, thread), + ); } } diff --git a/apps/server/src/services/environments/environment-cleanup-internal.ts b/apps/server/src/services/environments/environment-cleanup-internal.ts index 4939310ad4..4f2260a10e 100644 --- a/apps/server/src/services/environments/environment-cleanup-internal.ts +++ b/apps/server/src/services/environments/environment-cleanup-internal.ts @@ -157,6 +157,9 @@ function markLiveThreadsErroredAfterDestroySuccess( threadId: thread.id, }); if (outcome.applied) { + // Bare on purpose: in-transaction producers cannot build `statusChange` + // metadata (see buildThreadStatusChangeMetadata); clients fall back to + // the throttled thread-list refetch. deps.hub.notifyThread(thread.id, ["status-changed"]); } } diff --git a/apps/server/src/services/environments/environment-provisioning-internal.ts b/apps/server/src/services/environments/environment-provisioning-internal.ts index b75e4a0c03..79a8f67129 100644 --- a/apps/server/src/services/environments/environment-provisioning-internal.ts +++ b/apps/server/src/services/environments/environment-provisioning-internal.ts @@ -553,6 +553,9 @@ function recordEnvironmentProvisioningFailureInTransaction( threadId: thread.id, }); if (outcome.applied) { + // Bare on purpose: in-transaction producers cannot build `statusChange` + // metadata (see buildThreadStatusChangeMetadata); clients fall back to + // the throttled thread-list refetch. deps.hub.notifyThread(thread.id, ["status-changed"]); } } diff --git a/apps/server/src/services/threads/thread-lifecycle.ts b/apps/server/src/services/threads/thread-lifecycle.ts index 1768362a8f..b937221579 100644 --- a/apps/server/src/services/threads/thread-lifecycle.ts +++ b/apps/server/src/services/threads/thread-lifecycle.ts @@ -70,6 +70,7 @@ import { applyLoggedThreadLifecycleEvent, applyLoggedThreadLifecycleEventInTransaction, } from "./lifecycle-outcome.js"; +import { buildThreadStatusChangeMetadata } from "./thread-runtime-display.js"; import { addRequestIdToTurnSubmitCommandPayload, buildThreadStartCommand, @@ -535,6 +536,9 @@ function markThreadStoppingWithEventInTransaction( if (!outcome.applied) { return false; } + // Bare on purpose: an in-transaction producer with a buffered notifier + // cannot build `statusChange` metadata (see buildThreadStatusChangeMetadata); + // clients fall back to the throttled thread-list refetch. deps.hub.notifyThread(args.threadId, ["status-changed"]); appendThreadInterruptedEventInTransaction(deps.db, { threadId: args.threadId, @@ -807,6 +811,8 @@ function settleThreadCommandFailure( threadId: thread.id, }); if (outcome.applied) { + // Bare on purpose: in-transaction producers cannot build `statusChange` + // metadata; clients fall back to the throttled thread-list refetch. args.deps.hub.notifyThread(thread.id, ["status-changed"]); } // Forks / side chats are user-initiated branches, not agent-delegated @@ -877,6 +883,8 @@ export function settleThreadStartCommandResult( threadId: currentThread.id, }); if (outcome.applied) { + // Bare on purpose: in-transaction producers cannot build `statusChange` + // metadata; clients fall back to the throttled thread-list refetch. args.deps.hub.notifyThread(currentThread.id, ["status-changed"]); if (shouldAutoSendQueuedMessagesAfterThreadStart(args.command)) { postCommitActions.push({ @@ -1563,6 +1571,8 @@ function interruptActiveTurnForThreadInTransaction( if (appendedThreadInterruptedEvent) { eventTypes.push("system/thread/interrupted"); } + // No `statusChange` on purpose: in-transaction producers cannot build it; + // clients fall back to the throttled thread-list refetch. deps.hub.notifyThread(args.threadId, ["events-appended", "status-changed"], { eventTypes, }); @@ -1576,7 +1586,10 @@ function interruptActiveTurnForThreadInTransaction( * threads with an open turn also get an interrupted turn completion event. */ function interruptActiveThreads( - deps: Pick, + deps: Pick< + AppDeps, + "db" | "hub" | "logger" | "pendingInteractions" | "providerRegistry" + >, args: InterruptActiveThreadsArgs, ): InterruptActiveThreadsResult { if (args.threads.length === 0) { @@ -1673,11 +1686,16 @@ function interruptActiveThreads( if (result.interruptedTurnId !== null) { eventTypes.unshift("turn/completed"); } + // Published after the transaction committed, so the row snapshot is the + // settled post-interruption state and clients patch their list rows + // instead of refetching every thread list once per interrupted thread. + const thread = getThread(deps.db, result.threadId); deps.hub.notifyThread( result.threadId, ["events-appended", "status-changed"], { eventTypes, + ...(thread ? buildThreadStatusChangeMetadata(deps, thread) : {}), }, ); } @@ -1686,7 +1704,10 @@ function interruptActiveThreads( } export function interruptActiveThreadsForHost( - deps: Pick, + deps: Pick< + AppDeps, + "db" | "hub" | "logger" | "pendingInteractions" | "providerRegistry" + >, args: InterruptActiveThreadsForHostArgs, ): InterruptActiveThreadsResult { const activeThreads = deps.db @@ -1773,6 +1794,9 @@ export function finalizeStoppedThreadInTransaction( threadId: currentThread.id, }); if (outcome.applied) { + // Bare on purpose: in-transaction producers cannot build + // `statusChange` metadata; clients fall back to the throttled + // thread-list refetch. deps.hub.notifyThread(currentThread.id, ["status-changed"]); } } @@ -1782,6 +1806,7 @@ export function finalizeStoppedThreadInTransaction( threadId: currentThread.id, }); if (outcome.applied) { + // Bare on purpose: see the active/stopping branch above. deps.hub.notifyThread(currentThread.id, ["status-changed"]); } } diff --git a/apps/server/src/ws/daemon-protocol.ts b/apps/server/src/ws/daemon-protocol.ts index ab19a68938..4b5315bde1 100644 --- a/apps/server/src/ws/daemon-protocol.ts +++ b/apps/server/src/ws/daemon-protocol.ts @@ -239,6 +239,7 @@ export function onDaemonSocketClose( | "hub" | "logger" | "pendingInteractions" + | "providerRegistry" | "sharedPorts" | "terminalSessions" >, diff --git a/apps/server/test/internal/session-owner-runtime-status.test.ts b/apps/server/test/internal/session-owner-runtime-status.test.ts new file mode 100644 index 0000000000..7572ffac83 --- /dev/null +++ b/apps/server/test/internal/session-owner-runtime-status.test.ts @@ -0,0 +1,162 @@ +import { changedMessageSchema, type ThreadChangedMessage } from "@bb/domain"; +import { getThread } from "@bb/db"; +import { describe, expect, it } from "vitest"; +import { + handleDaemonSocketClosed, + handleHostRemoved, +} from "../../src/internal/session-owner-side-effects.js"; +import { createMockHubSocket } from "../helpers/mock-hub-socket.js"; +import { + seedEnvironment, + seedHostSession, + seedProjectWithSource, + seedThread, +} from "../helpers/seed.js"; +import { withTestHarness, type TestAppHarness } from "../helpers/test-app.js"; + +interface HostThreadsFixture { + activeThreadId: string; + hostId: string; + idleThreadId: string; + sessionId: string; +} + +function seedHostThreadsFixture( + harness: TestAppHarness, + value: number, +): HostThreadsFixture { + const { host, session } = seedHostSession(harness.deps, { + id: `host-runtime-status-${value}`, + }); + const { project } = seedProjectWithSource(harness.deps, { + hostId: host.id, + path: `/tmp/runtime-status-${value}`, + }); + const environment = seedEnvironment(harness.deps, { + hostId: host.id, + projectId: project.id, + path: `/tmp/runtime-status-${value}`, + status: "ready", + }); + const activeThread = seedThread(harness.deps, { + projectId: project.id, + environmentId: environment.id, + status: "active", + }); + const idleThread = seedThread(harness.deps, { + projectId: project.id, + environmentId: environment.id, + status: "idle", + }); + return { + activeThreadId: activeThread.id, + hostId: host.id, + idleThreadId: idleThread.id, + sessionId: session.id, + }; +} + +function statusChangedMessagesFor( + messages: readonly string[], + threadId: string, +): ThreadChangedMessage[] { + return messages.flatMap((raw) => { + const message = changedMessageSchema.parse(JSON.parse(raw)); + return message.entity === "thread" && + message.id === threadId && + message.changes.includes("status-changed") + ? [message] + : []; + }); +} + +function lastStatusChange( + messages: readonly string[], + threadId: string, +): ThreadChangedMessage { + const statusMessages = statusChangedMessagesFor(messages, threadId); + const last = statusMessages.at(-1); + if (!last) { + throw new Error(`no status-changed message for thread ${threadId}`); + } + return last; +} + +describe("host thread runtime status notifications", () => { + it("carries a statusChange snapshot for every host thread when the daemon socket closes", async () => { + await withTestHarness(async (harness) => { + const fixture = seedHostThreadsFixture(harness, 1); + const socket = createMockHubSocket(); + harness.hub.subscribe(socket, { kind: "thread-list" }); + + handleDaemonSocketClosed(harness.deps, { sessionId: fixture.sessionId }); + // The disconnect schedules the grace callbacks with real timers; drop + // them so the harness does not fire interruptions after cleanup. + harness.hub.cancelPendingDaemonDisconnect(fixture.sessionId); + + // Bare notifications here would make every client refetch every active + // thread list once per host thread; the snapshot is what lets them + // patch rows in place. + const activeMessage = lastStatusChange( + socket.messages, + fixture.activeThreadId, + ); + expect(activeMessage.metadata?.statusChange).toMatchObject({ + status: "active", + runtime: { + displayStatus: "host-reconnecting", + hostReconnectGraceExpiresAt: expect.any(Number), + }, + }); + const idleMessage = lastStatusChange( + socket.messages, + fixture.idleThreadId, + ); + expect(idleMessage.metadata?.statusChange).toMatchObject({ + status: "idle", + runtime: { displayStatus: "idle", hostReconnectGraceExpiresAt: null }, + }); + }); + }); + + it("carries the settled post-interruption snapshot when the host is removed", async () => { + await withTestHarness(async (harness) => { + const fixture = seedHostThreadsFixture(harness, 2); + const socket = createMockHubSocket(); + harness.hub.subscribe(socket, { kind: "thread-list" }); + + handleHostRemoved(harness.deps, { + hostId: fixture.hostId, + sessionId: fixture.sessionId, + }); + + // Removal interrupts the active thread (run.failed) before the runtime + // fan-out, so every status-changed for it must carry a snapshot and the + // final one must show the settled error state. + const activeMessages = statusChangedMessagesFor( + socket.messages, + fixture.activeThreadId, + ); + expect(activeMessages.length).toBeGreaterThan(0); + for (const message of activeMessages) { + expect(message.metadata?.statusChange).toBeDefined(); + } + expect(getThread(harness.db, fixture.activeThreadId)?.status).toBe( + "error", + ); + expect( + activeMessages.at(-1)?.metadata?.statusChange, + ).toMatchObject({ + status: "error", + runtime: { displayStatus: "error" }, + }); + expect( + lastStatusChange(socket.messages, fixture.idleThreadId).metadata + ?.statusChange, + ).toMatchObject({ + status: "idle", + runtime: { displayStatus: "idle" }, + }); + }); + }); +}); diff --git a/apps/server/test/threads/thread-send-dispatch.test.ts b/apps/server/test/threads/thread-send-dispatch.test.ts index 6d2c5aaa2c..9245cfa05f 100644 --- a/apps/server/test/threads/thread-send-dispatch.test.ts +++ b/apps/server/test/threads/thread-send-dispatch.test.ts @@ -6,7 +6,13 @@ import { listQueuedThreadMessages, markThreadDeleted, } from "@bb/db"; -import { turnScope, type Environment, type Thread } from "@bb/domain"; +import { + changedMessageSchema, + turnScope, + type Environment, + type Thread, + type ThreadChangedMessage, +} from "@bb/domain"; import { describe, expect, it, vi } from "vitest"; import type { TelemetryService } from "../../src/services/system/telemetry.js"; import { sendQueuedMessage } from "../../src/services/threads/queued-messages.js"; @@ -17,6 +23,7 @@ import { reportQueuedCommandError, waitForQueuedCommand, } from "../helpers/commands.js"; +import { createMockHubSocket } from "../helpers/mock-hub-socket.js"; import { textInput } from "../helpers/prompt-input.js"; import { seedEnvironment, @@ -114,6 +121,15 @@ function installTelemetryCaptureSpy(harness: TestAppHarness) { return capture; } +function parseThreadMessages( + messages: readonly string[], +): ThreadChangedMessage[] { + return messages.flatMap((raw) => { + const message = changedMessageSchema.parse(JSON.parse(raw)); + return message.entity === "thread" ? [message] : []; + }); +} + describe("queued message dispatch gate", () => { it("rolls back and sends no host command when the idle thread was archived between claim and dispatch", async () => { await withTestHarness(async (harness) => { @@ -195,6 +211,41 @@ describe("queued message dispatch gate", () => { }); }); +describe("queued message auto-send notification", () => { + it("carries the statusChange row snapshot when the auto-send activates the thread", async () => { + await withTestHarness(async (harness) => { + const { thread } = seedProviderThreadFixture({ harness, value: 41 }); + const queued = seedQueuedMessage(harness.deps, { + threadId: thread.id, + content: textInput("queued while idle"), + }); + const socket = createMockHubSocket(); + harness.hub.subscribe(socket, { kind: "thread-list" }); + + await sendQueuedMessage(harness.deps, { + threadId: thread.id, + queuedMessageId: queued.id, + mode: "auto", + }); + + // Every status flip must carry the row snapshot, or the client falls + // back to refetching every active thread list for this transition. + const statusMessages = parseThreadMessages(socket.messages).filter( + (message) => + message.id === thread.id && + message.changes.includes("status-changed"), + ); + expect(statusMessages.length).toBeGreaterThan(0); + for (const message of statusMessages) { + expect(message.metadata?.statusChange).toMatchObject({ + status: "active", + runtime: { displayStatus: "active" }, + }); + } + }); + }); +}); + describe("user message telemetry", () => { it("captures direct user sends", async () => { await withTestHarness(async (harness) => {