diff --git a/devlog/_plan/260816_gui_loading_performance/030_phase3_hidden_pause.md b/devlog/_plan/260816_gui_loading_performance/030_phase3_hidden_pause.md index d5b3d151cd..9e87c80791 100644 --- a/devlog/_plan/260816_gui_loading_performance/030_phase3_hidden_pause.md +++ b/devlog/_plan/260816_gui_loading_performance/030_phase3_hidden_pause.md @@ -169,3 +169,45 @@ for real; browser check = hidden-tab request count (see below). Request-count reduction on visible tabs (WP4), re-activation staleness (WP4), server push (SSE) migration. + +## D addendum — landed (2026-08-16/17) + +Implementation: commits 955d90b34 (suspension state machine, visibility-poll.ts, +nine poller migrations, 9 new tests) and 794466b10 (round-5 audit fixes). + +Audit history: binding round r5 returned FAIL first — + +- HIGH: the Debug 1s tail poll lost its `useEffectEvent` dispatch in the migration, + so a stream switch (provider→usage, both enabled) kept polling the OLD stream: + wrong entries appended into the new buffer, the shared `logGenerationRef` bumped + by stale ticks cancelling the new stream's initial load, and corrupted `afterRef` + seqs. Fixed by dispatching the tick through `useEffectEvent` again — every tick + reads the latest `fetchLogs` while the interval stays pinned to + `[active, follow, streamEnabled]`. +- MEDIUM: `recomputePoll` never re-evaluated `anyOptOut` in its keep-countdown and + suspended branches, so an opt-out subscriber leaving while hidden left a timer + waking with nothing eligible, and an opt-out joining a suspended store never ran + until visible. Both branches now re-check `documentIsHidden() && !anyOptOut`. + +Round 2 PASS: the reviewer enumerated all four recomputePoll paths (teardown / +unchanged-interval / suspended / changed-interval) against the failure shapes — +no visible store ends timerless, no hidden non-opt-out store keeps a timer, the +listener is never lost while a poll is registered, and the suspended fall-through +cannot double-arm. Also folded in: abort signals for the three remaining 30s +pollers, `loadShadowCall` clearing in `finally`, and a throw guard in +`startVisibilityPoll` (without it a throwing make-up tick would skip `arm()` and +kill the cadence permanently). + +Verification evidence: +- `cd gui && bun test` over the 22 touched-surface suites → 146 pass / 0 fail. +- `bun run lint` (oxlint) and `bun run build` (tsc -b + vite) green. +- Hidden-tab proof is the happy-dom timer probe (`hasPollTimerForTests`), not live + emulation: as 001/E4 predicted, the in-app browser keeps background tabs + `visible`, `Emulation.setPageVisibilityState` is not exposed through the raw CDP + channel, and page-context `visibilityState` patching does not stick because + evaluation runs in an isolated world. Fetch-count assertions alone cannot see + this guarantee (a skipped tick looks identical to no tick), which is exactly why + the timer probe exists. +- Visible-tab baseline re-measured after the change (12s dwell on Dashboard, + sandboxed instance): unchanged cadence, no regression in request volume — the + reduction work is WP4's. diff --git a/gui/src/client-resource.ts b/gui/src/client-resource.ts index 6c9d22efb4..510ada188e 100644 --- a/gui/src/client-resource.ts +++ b/gui/src/client-resource.ts @@ -36,6 +36,12 @@ type Store = { pollTimer: ReturnType | null; /** Currently scheduled poll interval; avoids resetting the countdown on churn. */ pollIntervalMs: number | undefined; + /** + * Hidden with no opt-out subscriber: the timer is GONE (not just skipped) and only + * a visibility transition back to visible re-arms it. pollIntervalMs survives + * suspension so churn cannot re-arm through the changed-interval path. + */ + pollSuspended: boolean; inflight: AbortController | null; /** Subscriber that started the current in-flight request (if any). */ inflightOwner: (() => void) | null; @@ -94,6 +100,7 @@ function getStore(key: string): Store { subscriberCount: 0, pollTimer: null, pollIntervalMs: undefined, + pollSuspended: false, inflight: null, inflightOwner: null, visibilityListener: null, @@ -117,6 +124,30 @@ function clearPollTimer(store: Store) { store.pollIntervalMs = undefined; } +/** + * Clear only the timer and mark the store suspended. Unlike clearPollTimer this + * KEEPS pollIntervalMs — clearing it would let hidden-phase subscriber churn + * (StrictMode, tab mounts) re-arm a fresh interval through recomputePoll's + * changed-interval path. + */ +function suspendPollTimer(store: Store) { + if (store.pollTimer !== null) { + clearInterval(store.pollTimer); + store.pollTimer = null; + } + store.pollSuspended = true; +} + +/** True when any polling subscriber opted out of hidden pausing (e.g. restart watch). */ +function anyOptOut(store: Store): boolean { + for (const [listener, ms] of store.pollByListener) { + if (typeof ms === "number" && ms > 0 && store.pauseWhenHiddenByListener.get(listener) === false) { + return true; + } + } + return false; +} + /** True when the document is currently hidden. Safe on non-browser runtimes. */ function documentIsHidden(): boolean { return typeof document !== "undefined" && document.visibilityState === "hidden"; @@ -167,15 +198,37 @@ function recomputePoll(store: Store) { pollMs = pollMs === undefined ? ms : Math.min(pollMs, ms); } } + if (pollMs === undefined) { + // No subscriber polls any more: full teardown. A suspended store resets here so + // the next polling subscriber starts clean even while the tab is still hidden — + // otherwise the flag would outlive the listener that resumes it. + clearPollTimer(store); + store.pollSuspended = false; + removeVisibilityListener(store); + return; + } // Keep the existing countdown when the effective interval is unchanged. - if (pollMs === store.pollIntervalMs && (pollMs === undefined || store.pollTimer !== null)) { + if (pollMs === store.pollIntervalMs && store.pollTimer !== null) { + // Opt-out churn with an unchanged interval: the last opt-out left while hidden — + // the timer would keep waking every interval with zero eligible entries. + if (documentIsHidden() && !anyOptOut(store)) suspendPollTimer(store); return; } + if (store.pollSuspended) { + store.pollIntervalMs = pollMs; + // Hidden with no opt-out: bookkeeping only — arming is the visibility handler's + // job. An opt-out subscriber joining a suspended store arms below even while + // hidden: noticing the off-screen event is its documented purpose. + if (documentIsHidden() && !anyOptOut(store)) return; + store.pollSuspended = false; + } clearPollTimer(store); store.pollIntervalMs = pollMs; - if (pollMs === undefined) { - // No subscriber polls any more, so there is no skipped tick to make up on return. - removeVisibilityListener(store); + if (documentIsHidden() && !anyOptOut(store)) { + // First subscribed (or re-armed) while already hidden: hold no timer at all. + // The listener still installs so the resume + make-up path stays live. + store.pollSuspended = true; + ensureVisibilityListener(store); return; } store.pollTimer = setInterval(() => { @@ -188,23 +241,34 @@ function recomputePoll(store: Store) { } /** - * One listener per polling store: when the tab comes back, the skipped ticks are made up - * with a single quiet revalidation instead of waiting out the remaining interval. + * One listener per polling store, installed whenever a polling subscriber exists — + * regardless of whether a timer is currently armed (a mount-while-hidden store has + * no timer but MUST still resume). On hidden the timer is suspended outright (zero + * wakeups, unless an opt-out subscriber keeps it); on visible the skipped ticks are + * made up with a single quiet revalidation and the cadence re-arms. * * `replaceInflight: false` keeps this from cancelling work a visible-again mount just * started; if something is already loading, that request is the fresh answer. */ function ensureVisibilityListener(store: Store) { if (typeof document === "undefined" || store.visibilityListener) return; - const onVisible = () => { - if (documentIsHidden()) return; + const onVisibility = () => { if (store.pollIntervalMs === undefined) return; + if (documentIsHidden()) { + if (anyOptOut(store)) return; // opt-out polls keep their timer running + suspendPollTimer(store); + return; + } + if (store.pollSuspended) { + store.pollSuspended = false; + recomputePoll(store); // re-arms: pollIntervalMs survived the suspension + } const entry = pickFetcherEntry(store); if (!entry) return; void runFetch(store, entry.fetcher, { replaceInflight: false, owner: entry.owner, deadlineMs: entry.deadlineMs }); }; - document.addEventListener("visibilitychange", onVisible); - store.visibilityListener = onVisible; + document.addEventListener("visibilitychange", onVisibility); + store.visibilityListener = onVisibility; } function removeVisibilityListener(store: Store) { @@ -322,6 +386,7 @@ function abortInflightOwnedBy(store: Store, owner: () => void): boolean { */ function scheduleStoreEviction(key: string, store: Store) { clearPollTimer(store); + store.pollSuspended = false; // The visibility listener exists to wake a poll; with no poll left there is nothing to // wake, and leaving it attached would leak one handler per evicted store. removeVisibilityListener(store); @@ -553,3 +618,13 @@ export function clearClientResourceStoresForTests(): void { } stores.clear(); } + +/** + * Test-only: whether a live poll timer exists for the key. Suspension (hidden with no + * opt-out subscriber) means NO timer — that absence is the whole hidden-tab guarantee, + * and fetch counts alone cannot see it (skipped ticks look identical). + */ +export function hasPollTimerForTests(key: string): boolean { + const store = stores.get(key); + return store ? store.pollTimer !== null : false; +} diff --git a/gui/src/components/CodexAccountPickerSetting.tsx b/gui/src/components/CodexAccountPickerSetting.tsx index 725712918a..4988b715f3 100644 --- a/gui/src/components/CodexAccountPickerSetting.tsx +++ b/gui/src/components/CodexAccountPickerSetting.tsx @@ -1,5 +1,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { readJsonOrThrow } from "../fetch-json"; +import { startVisibilityPoll } from "../visibility-poll"; +import { createBoundedFetch } from "../bounded-fetch"; import { useT } from "../i18n/shared"; import type { NoticeTone } from "../ui"; @@ -20,8 +22,9 @@ export default function CodexAccountPickerSetting({ apiBase }: { apiBase: string const load = useCallback(async () => { if (savingRef.current) return; const generation = ++loadGenerationRef.current; + const bounded = createBoundedFetch(15_000); try { - const response = await fetch(`${apiBase}/api/settings`); + const response = await fetch(`${apiBase}/api/settings`, { signal: bounded.signal }); if (!response.ok) throw new Error("load"); const payload = await response.json() as { codexAccountPickerEnabled?: unknown }; if (savingRef.current || generation !== loadGenerationRef.current) return; @@ -34,15 +37,17 @@ export default function CodexAccountPickerSetting({ apiBase }: { apiBase: string if (!savingRef.current && generation === loadGenerationRef.current) { setLoadError(true); } + } finally { + bounded.clear(); } }, [apiBase]); useEffect(() => { const timeout = window.setTimeout(() => { void load(); }, 0); - const interval = window.setInterval(() => { void load(); }, 30_000); + const stop = startVisibilityPoll(() => { void load(); }, 30_000); return () => { window.clearTimeout(timeout); - window.clearInterval(interval); + stop(); }; }, [load]); diff --git a/gui/src/components/DefaultModeRequestUserInputSetting.tsx b/gui/src/components/DefaultModeRequestUserInputSetting.tsx index b72fabbff0..d793ac62ab 100644 --- a/gui/src/components/DefaultModeRequestUserInputSetting.tsx +++ b/gui/src/components/DefaultModeRequestUserInputSetting.tsx @@ -1,6 +1,8 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { useT } from "../i18n/shared"; import { readJsonOrThrow } from "../fetch-json"; +import { startVisibilityPoll } from "../visibility-poll"; +import { createBoundedFetch } from "../bounded-fetch"; const FEATURE_ENDPOINT = "/api/codex-auth/features/default-mode-request-user-input"; @@ -28,8 +30,9 @@ export default function DefaultModeRequestUserInputSetting({ apiBase }: { apiBas // GETs that were already in flight when a save started. if (savingRef.current) return; const generation = ++loadGenerationRef.current; + const bounded = createBoundedFetch(15_000); try { - const res = await fetch(`${apiBase}${FEATURE_ENDPOINT}`); + const res = await fetch(`${apiBase}${FEATURE_ENDPOINT}`, { signal: bounded.signal }); if (!res.ok) throw new Error("load"); const payload = await res.json() as { enabled?: unknown }; if (savingRef.current || generation !== loadGenerationRef.current) return; @@ -39,15 +42,17 @@ export default function DefaultModeRequestUserInputSetting({ apiBase }: { apiBas setLoadError(false); } catch { if (!savingRef.current && generation === loadGenerationRef.current) setLoadError(true); + } finally { + bounded.clear(); } }, [apiBase]); useEffect(() => { const timeout = window.setTimeout(() => { void load(); }, 0); - const interval = window.setInterval(() => { void load(); }, 30_000); + const stop = startVisibilityPoll(() => { void load(); }, 30_000); return () => { window.clearTimeout(timeout); - window.clearInterval(interval); + stop(); }; }, [load]); diff --git a/gui/src/components/MemoryObservabilityCard.tsx b/gui/src/components/MemoryObservabilityCard.tsx index da8709c294..d19134c756 100644 --- a/gui/src/components/MemoryObservabilityCard.tsx +++ b/gui/src/components/MemoryObservabilityCard.tsx @@ -3,6 +3,7 @@ import { formatUptime } from "../formatUptime"; import { IconActivity } from "../icons"; import { useI18n, type Locale, type TFn } from "../i18n/shared"; import { createBoundedFetch, type BoundedFetch } from "../bounded-fetch"; +import { startVisibilityPoll } from "../visibility-poll"; /** * Memory observability card. Polls GET /api/system/memory (#314 WP3) every 5s @@ -274,12 +275,16 @@ export default function MemoryObservabilityCard({ apiBase }: { apiBase: string } } }; void fetchMemory(); - const interval = setInterval(() => void fetchMemory(), 5000); + // Hidden tabs show no one the paint: no timer, no /api/system/memory traffic. + // The restart-reconnect loop below is the deliberate exception — it exists to + // notice the server coming back while nobody watches, is bounded, and only runs + // while a restart is actually in progress. + const stop = startVisibilityPoll(() => void fetchMemory(), 5000); return () => { cancelled = true; active?.controller.abort(); active?.clear(); - clearInterval(interval); + stop(); }; }, [apiBase, restartPhase, restartFromPid]); diff --git a/gui/src/components/provider-workspace/ProviderSettings.tsx b/gui/src/components/provider-workspace/ProviderSettings.tsx index 37cf171bed..fe9725e6a5 100644 --- a/gui/src/components/provider-workspace/ProviderSettings.tsx +++ b/gui/src/components/provider-workspace/ProviderSettings.tsx @@ -10,6 +10,8 @@ import { useEffect, useMemo, useRef, useState } from "react"; import { baseUrlForChoice, matchChoiceId, resolvedBaseUrlForChoice } from "../../base-url-choice"; import { readJsonIfOk } from "../../fetch-json"; +import { createBoundedFetch } from "../../bounded-fetch"; +import { startVisibilityPoll } from "../../visibility-poll"; import { useT } from "../../i18n/shared"; import { IconLock } from "../../icons"; import { isCatalogProviderId } from "../../provider-icons"; @@ -152,15 +154,21 @@ export default function ProviderSettings({ useEffect(() => { if (!apiBase) return; let active = true; + let inFlight = false; const load = () => { - fetch(`${apiBase}/api/provider-request-pacing?name=${encodeURIComponent(item.name)}`) + // Guarded + bounded: a hung pacing read must never stack or pin the panel. + if (inFlight) return; + inFlight = true; + const bounded = createBoundedFetch(10_000); + fetch(`${apiBase}/api/provider-request-pacing?name=${encodeURIComponent(item.name)}`, { signal: bounded.signal }) .then(r => readJsonIfOk(r)) .then(status => { if (active && status) setPacingStatus(status); }) - .catch(() => undefined); + .catch(() => undefined) + .finally(() => { bounded.clear(); inFlight = false; }); }; load(); - const timer = window.setInterval(load, 2_000); - return () => { active = false; window.clearInterval(timer); }; + const stop = startVisibilityPoll(load, 2_000); + return () => { active = false; stop(); }; }, [apiBase, item.name]); const pacingDraft = useMemo(() => ({ diff --git a/gui/src/components/use-add-codex-account-oauth.ts b/gui/src/components/use-add-codex-account-oauth.ts index 84626dafff..acff30dd87 100644 --- a/gui/src/components/use-add-codex-account-oauth.ts +++ b/gui/src/components/use-add-codex-account-oauth.ts @@ -7,6 +7,7 @@ import type { } from "./add-codex-account-reducer"; import type { TFn } from "../i18n/shared"; import { readJsonIfOk, readJsonOrThrow } from "../fetch-json"; +import { startVisibilityPoll } from "../visibility-poll"; import { codexAccountMutationCompletion, type CodexAccountMutationCompletion, @@ -34,7 +35,7 @@ export function useAddCodexAccountOAuth({ const aliveRef = useRef(true); const pollErrorStreakRef = useRef(0); const pollInFlightRef = useRef(false); - const pollRef = useRef | null>(null); + const pollRef = useRef<(() => void) | null>(null); const timeoutRef = useRef | null>(null); const pollAbortRef = useRef(null); const flowRef = useRef(null); @@ -55,7 +56,7 @@ export function useAddCodexAccountOAuth({ }, [ui.flowId]); const stopPolling = useCallback(() => { - if (pollRef.current) { clearInterval(pollRef.current); pollRef.current = null; } + if (pollRef.current) { pollRef.current(); pollRef.current = null; } if (timeoutRef.current) { clearTimeout(timeoutRef.current); timeoutRef.current = null; } pollAbortRef.current?.abort(); pollAbortRef.current = null; @@ -174,7 +175,9 @@ export function useAddCodexAccountOAuth({ : `${apiBase}/api/codex-auth/login-status`; const pollSession = new AbortController(); pollAbortRef.current = pollSession; - pollRef.current = setInterval(async () => { + // A hidden tab cannot complete OAuth; the visible make-up tick checks the + // login status the moment the user returns to the modal. + pollRef.current = startVisibilityPoll(async () => { if (pollInFlightRef.current || pollSession.signal.aborted) return; pollInFlightRef.current = true; // Bound each tick and abort it when stopPolling/cleanup cancels the session. diff --git a/gui/src/hooks/useCodexAccountPool.ts b/gui/src/hooks/useCodexAccountPool.ts index 628c8643ad..d9613643a1 100644 --- a/gui/src/hooks/useCodexAccountPool.ts +++ b/gui/src/hooks/useCodexAccountPool.ts @@ -1,5 +1,7 @@ import { usageSummary30dResourceKey } from "../usage-summary-resource"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { createBoundedFetch } from "../bounded-fetch"; +import { startVisibilityPoll } from "../visibility-poll"; import { normalizeAccountPriority } from "../account-priority"; import { useKeyedClientResource } from "../client-resource"; import { extractAutoSwitchThresholdPayload } from "../codex-auto-switch"; @@ -203,6 +205,8 @@ export function useCodexAccountPool(apiBase: string, enabled = true): CodexAccou const load = useCallback(async (refreshQuota = false): Promise => { const generation = ++loadGenerationRef.current; + // Bounded per attempt: a hung accounts/active read must settle, not pin the poll. + const bounded = createBoundedFetch(20_000); setInflightCount(count => count + 1); // The try opens immediately after the increment so even a synchronous throw in the // observer snapshot below cannot leave the counter stuck above zero. @@ -219,7 +223,7 @@ export function useCodexAccountPool(apiBase: string, enabled = true): CodexAccou const accountsTask = (async (): Promise => { try { - const response = await fetch(`${apiBase}/api/codex-auth/accounts${refreshQuota ? "?refresh=1" : ""}`); + const response = await fetch(`${apiBase}/api/codex-auth/accounts${refreshQuota ? "?refresh=1" : ""}`, { signal: bounded.signal }); if (!response.ok) throw new Error("account load failed"); const payload = await response.json(); if (loadGenerationRef.current === generation) { @@ -247,7 +251,7 @@ export function useCodexAccountPool(apiBase: string, enabled = true): CodexAccou const activeTask = (async (): Promise => { try { - const response = await fetch(`${apiBase}/api/codex-auth/active`); + const response = await fetch(`${apiBase}/api/codex-auth/active`, { signal: bounded.signal }); if (!response.ok) throw new Error("active account load failed"); const active = await response.json(); if (loadGenerationRef.current === generation) { @@ -293,6 +297,7 @@ export function useCodexAccountPool(apiBase: string, enabled = true): CodexAccou if (!hasLoadedRef.current) setLoadState("error"); return false; } finally { + bounded.clear(); setInflightCount(count => Math.max(0, count - 1)); setFirstAttemptSettled(true); } @@ -336,11 +341,11 @@ export function useCodexAccountPool(apiBase: string, enabled = true): CodexAccou }; }, [enabled, needsQuotaFill, pauseCount, load]); - // Background refresh, suspended while any pause lease is held. + // Background refresh, suspended while any pause lease is held — and fully paused + // (no timer, no traffic) while the tab is hidden. useEffect(() => { if (!enabled || pauseCount > 0) return; - const interval = window.setInterval(() => { void load(); }, REFRESH_INTERVAL_MS); - return () => window.clearInterval(interval); + return startVisibilityPoll(() => { void load(); }, REFRESH_INTERVAL_MS); }, [enabled, load, pauseCount]); const pauseRefresh = useCallback((): PauseToken => { diff --git a/gui/src/pages/CodexAuth.tsx b/gui/src/pages/CodexAuth.tsx index 17a7b128dd..17239f6acd 100644 --- a/gui/src/pages/CodexAuth.tsx +++ b/gui/src/pages/CodexAuth.tsx @@ -7,6 +7,8 @@ import { codexAccountModeState, type CodexAccountModeState } from "../codex-mult import { navigateHash } from "../hash-routing"; import { ensureOpenAiProvider, openAiAccountProviderState, OpenAiEnableError } from "../provider-payload"; import { readSessionListCache, writeSessionListCache } from "../session-list-cache"; +import { startVisibilityPoll } from "../visibility-poll"; +import { createBoundedFetch } from "../bounded-fetch"; export type OpenAiAccountBannerState = CodexAccountModeState | "invalid" | null; @@ -119,8 +121,9 @@ export default function CodexAuth({ apiBase }: { apiBase: string }) { const [enableError, setEnableError] = useState(""); const loadMode = useCallback(async () => { + const bounded = createBoundedFetch(15_000); try { - const res = await fetch(`${apiBase}/api/config`); + const res = await fetch(`${apiBase}/api/config`, { signal: bounded.signal }); if (!res.ok) throw new Error(String(res.status)); const config = await res.json(); const providerState = openAiAccountProviderState(openaiProviderFromConfig(config)); @@ -138,6 +141,8 @@ export default function CodexAuth({ apiBase }: { apiBase: string }) { writeSessionListCache(configCacheKey, { bannerState: mode, accountModeState: mode }); } catch { // Keep last-good banner on transient config failures. + } finally { + bounded.clear(); } }, [apiBase, configCacheKey]); @@ -151,8 +156,10 @@ export default function CodexAuth({ apiBase }: { apiBase: string }) { initialModeKeyRef.current = apiBase; void Promise.resolve().then(() => { void loadMode(); }); } - const iv = window.setInterval(() => { void loadMode(); }, 30_000); - return () => { window.clearInterval(iv); }; + // Hidden tabs hold no timer and fire nothing; the visible make-up tick re-reads + // the mode the moment the user returns. + const stop = startVisibilityPoll(() => { void loadMode(); }, 30_000); + return () => { stop(); }; }, [apiBase, loadMode]); const enableOpenAi = async () => { diff --git a/gui/src/pages/Debug.tsx b/gui/src/pages/Debug.tsx index a6e4602ac3..05207fbb90 100644 --- a/gui/src/pages/Debug.tsx +++ b/gui/src/pages/Debug.tsx @@ -6,6 +6,8 @@ import { Notice } from "../ui"; import { useDataSurface } from "../data-surface"; import { DataSurfaceSkeleton } from "../components/data-surface"; import { readSessionListCache, writeSessionListCache } from "../session-list-cache"; +import { createBoundedFetch } from "../bounded-fetch"; +import { startVisibilityPoll } from "../visibility-poll"; import { DebugClaudeInboundPanel } from "./debug-claude-inbound-panel"; import { DebugLogViewer } from "./debug-log-viewer"; import { DebugPageHeader, DebugSettingsPanel } from "./debug-settings-panel"; @@ -148,14 +150,25 @@ export default function Debug({ apiBase, embedded, active = true }: { apiBase: s // eslint-disable-next-line react-hooks/exhaustive-deps -- stream identity only }, [active, apiBase, stream, streamEnabled]); - const pollLogs = useEffectEvent((initial: boolean) => { - void fetchLogs(initial); + const pollInFlightRef = useRef(false); + + // useEffectEvent keeps every tick on the LATEST fetchLogs (stream/apiBase identity) + // without re-arming the interval — dropping it would tail the old stream after a switch. + const pollTick = useEffectEvent(() => { + if (pollInFlightRef.current) return; + pollInFlightRef.current = true; + const bounded = createBoundedFetch(10_000); + void fetchLogs(false, bounded.signal).finally(() => { + bounded.clear(); + pollInFlightRef.current = false; + }); }); useEffect(() => { if (!active || !follow || !streamEnabled) return; - const interval = setInterval(() => pollLogs(false), 1000); - return () => clearInterval(interval); + // 1s tail poll: paused entirely while the tab is hidden; each tick is guarded + // and bounded so a hung request never stacks or pins the refreshing indicator. + return startVisibilityPoll(() => pollTick(), 1000); }, [active, follow, streamEnabled]); useEffect(() => { diff --git a/gui/src/pages/Models.tsx b/gui/src/pages/Models.tsx index 4db00f7cac..efa2239971 100644 --- a/gui/src/pages/Models.tsx +++ b/gui/src/pages/Models.tsx @@ -12,6 +12,8 @@ import { formatNamespacedModelId, formatProviderDisplayName, providerDisplaySlug import { readJsonIfOk, readJsonOrThrow } from "../fetch-json"; import { readSessionListCache, writeSessionListCache } from "../session-list-cache"; import { setClientResourceData } from "../client-resource"; +import { createBoundedFetch } from "../bounded-fetch"; +import { startVisibilityPoll } from "../visibility-poll"; import { useDataSurface } from "../data-surface"; import { DataSurfaceSkeleton } from "../components/data-surface"; import ErrorBoundary from "../components/ErrorBoundary"; @@ -284,18 +286,21 @@ export default function Models({ apiBase, restartEpoch = 0 }: { apiBase: string; }, [models, shadowCall?.model, shadowModelOptions]); const loadShadowCall = useCallback(async () => { + const bounded = createBoundedFetch(15_000); try { - const r = await fetch(`${apiBase}/api/shadow-call-settings`); + const r = await fetch(`${apiBase}/api/shadow-call-settings`, { signal: bounded.signal }); const data = await readJsonIfOk(r); if (data) setShadowCall(data); } catch { /* old server / network: keep the section disabled */ } + finally { bounded.clear(); } }, [apiBase]); const loadV2 = useCallback(async () => { // Never let a toggle in flight be clobbered by the poll (same single-flight rule as models). if (v2BusyRef.current) return; + const bounded = createBoundedFetch(15_000); try { - const r = await fetch(`${apiBase}/api/v2`); + const r = await fetch(`${apiBase}/api/v2`, { signal: bounded.signal }); if (!(r.headers.get("content-type") ?? "").includes("application/json")) { setV2(null); return; } const data = await readJsonIfOk(r); if (!data || typeof data.enabled !== "boolean") { setV2(null); return; } @@ -309,6 +314,7 @@ export default function Models({ apiBase, restartEpoch = 0 }: { apiBase: string; } catch { setV2(null); // old server / network: hide the section instead of guessing } finally { + bounded.clear(); setV2Loading(false); } }, [apiBase]); @@ -412,12 +418,13 @@ export default function Models({ apiBase, restartEpoch = 0 }: { apiBase: string; void loadShadowCall(); void loadV2(); }, 0); - const timer = window.setInterval(() => { + // Hidden tab: no timer, no /api/v2 traffic; the make-up tick refreshes on return. + const stop = startVisibilityPoll(() => { if (!v2BusyRef.current) void loadV2(); - }, 10000); + }, 10_000); return () => { window.clearTimeout(timeout); - window.clearInterval(timer); + stop(); }; }, [catalogActive, loadShadowCall, loadV2]); diff --git a/gui/src/visibility-poll.ts b/gui/src/visibility-poll.ts new file mode 100644 index 0000000000..40eed778c3 --- /dev/null +++ b/gui/src/visibility-poll.ts @@ -0,0 +1,110 @@ +/** + * Shared visibility-aware interval for the dashboard's raw pollers. + * + * The client-resource layer already suspends its store timers while hidden; raw + * setInterval pollers (log viewers, settings cards, OAuth status) hand-rolled the + * same pattern nine different ways — most without any visibility handling, so a + * background tab kept paying full poll cost. This helper is the one place that + * owns the rule: while document.hidden there is no interval and no callback; on + * visible-again one make-up tick fires immediately, then the cadence resumes. + */ + +export type VisibilityPollOptions = { + /** + * Default true: hidden tabs neither tick nor hold a timer. Set false only for + * polls whose whole purpose is noticing something happening off-screen (a + * restarted server answering, for instance). + */ + pauseWhenHidden?: boolean; + /** Fire the callback once on start. Default false. */ + immediate?: boolean; +}; + +function hiddenNow(): boolean { + return typeof document !== "undefined" && document.visibilityState === "hidden"; +} + +/** + * Schedule through `window` when it exists. Every migrated poller used + * `window.setInterval`, and their tests intercept it there — the bare global binds + * to a different timer scope than the code this helper replaced, which both hides + * the poll from that instrumentation and changes its lifetime semantics. + */ +function scheduleInterval(fn: () => void, ms: number): ReturnType { + if (typeof window !== "undefined" && typeof window.setInterval === "function") { + return window.setInterval(fn, ms) as unknown as ReturnType; + } + return setInterval(fn, ms); +} + +function cancelInterval(handle: ReturnType): void { + if (typeof window !== "undefined" && typeof window.clearInterval === "function") { + window.clearInterval(handle as unknown as number); + return; + } + clearInterval(handle); +} + +/** + * Start an interval that exists only while the tab is visible (unless opted out). + * Returns stop(), which removes the timer and the visibility listener in any state. + */ +export function startVisibilityPoll( + callback: () => void, + intervalMs: number, + options?: VisibilityPollOptions, +): () => void { + const pauseWhenHidden = options?.pauseWhenHidden !== false; + let timer: ReturnType | null = null; + let stopped = false; + + // A synchronously throwing callback must not kill the poll: without the guard a + // make-up tick that throws would skip arm() and never tick again. + const tick = () => { + try { + callback(); + } catch (error) { + console.error("[visibility-poll]", error); + } + }; + + const arm = () => { + if (timer !== null || stopped) return; + timer = scheduleInterval(tick, intervalMs); + }; + const disarm = () => { + if (timer === null) return; + cancelInterval(timer); + timer = null; + }; + + const onVisibility = () => { + if (!pauseWhenHidden) return; + if (hiddenNow()) { + disarm(); + return; + } + // Make-up tick: the user comes back to fresh data immediately, then cadence. + tick(); + arm(); + }; + + if (pauseWhenHidden && hiddenNow()) { + // Mounted while already hidden: no timer, but the listener must still be there + // or nothing would ever resume this poll. + } else { + arm(); + } + if (pauseWhenHidden && typeof document !== "undefined") { + document.addEventListener("visibilitychange", onVisibility); + } + if (options?.immediate) tick(); + + return () => { + stopped = true; + disarm(); + if (pauseWhenHidden && typeof document !== "undefined") { + document.removeEventListener("visibilitychange", onVisibility); + } + }; +} diff --git a/gui/tests/client-resource-poll.test.tsx b/gui/tests/client-resource-poll.test.tsx index dbfc8e2470..822a897074 100644 --- a/gui/tests/client-resource-poll.test.tsx +++ b/gui/tests/client-resource-poll.test.tsx @@ -4,6 +4,7 @@ import { act, useEffect, useState } from "react"; import type { Root } from "react-dom/client"; import { clearClientResourceStoresForTests, + hasPollTimerForTests, setClientResourceData, useClientResource, useKeyedClientResource, @@ -141,6 +142,226 @@ test("after the latest subscriber unmounts, polling continues with the surviving container.remove(); }); +// Hidden suspension means the timer is GONE, not just skipped. Subscriber churn in the +// hidden phase (unmount one of two) runs recomputePoll and must not re-arm it. +test("subscriber churn while hidden never re-arms a timer", async () => { + const { createRoot } = await import("react-dom/client"); + const container = document.createElement("div"); + document.body.append(container); + const KEY = `poll-hidden-churn-${Date.now()}`; + let fetches = 0; + + function Page({ both }: { both: boolean }) { + useClientResource(KEY, async () => { fetches += 1; return `v${fetches}`; }, { pollMs: 20 }); + const [showSecond] = useState(both); + return showSecond ? : null; + } + function Second() { + useClientResource(KEY, async () => { fetches += 1; return "s"; }, { pollMs: 40 }); + return null; + } + + let root!: Root; + await act(async () => { + root = createRoot(container); + root.render(); + }); + await waitFor(() => fetches >= 1); + expect(hasPollTimerForTests(KEY)).toBe(true); + + await setVisibility("hidden"); + expect(hasPollTimerForTests(KEY)).toBe(false); // suspended: timer gone + + // Churn: drop the second subscriber while hidden. + await act(async () => { + root.render(); + }); + await act(async () => { + await new Promise((resolve) => testWindow.setTimeout(resolve, 60)); + }); + expect(hasPollTimerForTests(KEY)).toBe(false); + + await setVisibility("visible"); + await waitFor(() => hasPollTimerForTests(KEY)); + await act(async () => { root.unmount(); }); + container.remove(); +}); + +// A store first subscribed while the tab is already hidden must not create a live timer, +// but its visibility listener must still be installed or nothing would ever resume it. +test("first subscribe while hidden arms no timer and still makes up on visible", async () => { + const { createRoot } = await import("react-dom/client"); + const container = document.createElement("div"); + document.body.append(container); + const KEY = `poll-mount-hidden-${Date.now()}`; + let fetches = 0; + + await setVisibility("hidden"); + function Page() { + useClientResource(KEY, async () => { fetches += 1; return `v${fetches}`; }, { pollMs: 20 }); + return null; + } + + let root!: Root; + await act(async () => { + root = createRoot(container); + root.render(); + }); + await waitFor(() => fetches >= 1); // the cold-start fetch is unaffected by suspension + expect(hasPollTimerForTests(KEY)).toBe(false); + const atMount = fetches; + await act(async () => { + await new Promise((resolve) => testWindow.setTimeout(resolve, 80)); + }); + expect(fetches).toBe(atMount); // no ticks while hidden + + await setVisibility("visible"); + await waitFor(() => fetches >= atMount + 1); // make-up fetch proves the listener lived + await act(async () => { root.unmount(); }); + container.remove(); + await setVisibility("visible"); +}); + +// The opt-out subscriber is what keeps a hidden store ticking. When it leaves while +// hidden, the remaining paused subscribers must not inherit a timer that wakes every +// interval with nothing eligible to run. +test("opt-out leaving while hidden suspends the timer for the paused remainder", async () => { + const { createRoot } = await import("react-dom/client"); + const container = document.createElement("div"); + document.body.append(container); + const KEY = `poll-optout-churn-${Date.now()}`; + let fetches = 0; + + function Page({ optOut }: { optOut: boolean }) { + useClientResource(KEY, async () => { fetches += 1; return `v${fetches}`; }, { pollMs: 20 }); + return optOut ? : null; + } + function OptOut() { + useClientResource(KEY, async () => { fetches += 1; return "o"; }, { pollMs: 20, pauseWhenHidden: false }); + return null; + } + + let root!: Root; + await act(async () => { + root = createRoot(container); + root.render(); + }); + await waitFor(() => fetches >= 1); + + await setVisibility("hidden"); + // The opt-out keeps the store polling while hidden. + expect(hasPollTimerForTests(KEY)).toBe(true); + const atHide = fetches; + await waitFor(() => fetches > atHide); + + // It leaves; the interval is unchanged, so only an explicit re-check can suspend. + await act(async () => { + root.render(); + }); + expect(hasPollTimerForTests(KEY)).toBe(false); + const atSuspend = fetches; + await act(async () => { + await new Promise((resolve) => testWindow.setTimeout(resolve, 80)); + }); + expect(fetches).toBe(atSuspend); + + await setVisibility("visible"); + await waitFor(() => hasPollTimerForTests(KEY)); + await act(async () => { root.unmount(); }); + container.remove(); + await setVisibility("visible"); +}); + +// The mirror case: an opt-out joining an already-suspended store must arm it, since +// noticing an off-screen event is the only reason that subscriber exists. +test("opt-out joining a suspended store arms it while still hidden", async () => { + const { createRoot } = await import("react-dom/client"); + const container = document.createElement("div"); + document.body.append(container); + const KEY = `poll-optout-join-${Date.now()}`; + let fetches = 0; + + function Page({ optOut }: { optOut: boolean }) { + useClientResource(KEY, async () => { fetches += 1; return `v${fetches}`; }, { pollMs: 20 }); + return optOut ? : null; + } + function OptOut() { + useClientResource(KEY, async () => { fetches += 1; return "o"; }, { pollMs: 20, pauseWhenHidden: false }); + return null; + } + + let root!: Root; + await act(async () => { + root = createRoot(container); + root.render(); + }); + await waitFor(() => fetches >= 1); + + await setVisibility("hidden"); + expect(hasPollTimerForTests(KEY)).toBe(false); + + await act(async () => { + root.render(); + }); + expect(hasPollTimerForTests(KEY)).toBe(true); + const atJoin = fetches; + await waitFor(() => fetches > atJoin); + + await act(async () => { root.unmount(); }); + container.remove(); + await setVisibility("visible"); +}); + +// The last polling subscriber leaving WHILE hidden must not strand the store: the flag +// resets with the teardown, so a later poller resumes cleanly on the next visible. +test("last poller leaves while hidden, then a new poller resumes on visible", async () => { + const { createRoot } = await import("react-dom/client"); + const container = document.createElement("div"); + document.body.append(container); + const KEY = `poll-hidden-teardown-${Date.now()}`; + let fetches = 0; + + function Page({ poller }: { poller: boolean }) { + useClientResource(KEY, async () => { fetches += 1; return `v${fetches}`; }); + return poller ? : null; + } + function Poller() { + useClientResource(KEY, async () => { fetches += 1; return "p"; }, { pollMs: 20 }); + return null; + } + + let root!: Root; + await act(async () => { + root = createRoot(container); + root.render(); + }); + await waitFor(() => fetches >= 1); + + await setVisibility("hidden"); + expect(hasPollTimerForTests(KEY)).toBe(false); + + // The only poller leaves while hidden: full poll teardown, suspension resets. + await act(async () => { + root.render(); + }); + // A NEW poller subscribes, still hidden: the arm-time guard holds, no timer. + await act(async () => { + root.render(); + }); + await act(async () => { + await new Promise((resolve) => testWindow.setTimeout(resolve, 60)); + }); + expect(hasPollTimerForTests(KEY)).toBe(false); + + await setVisibility("visible"); + await waitFor(() => hasPollTimerForTests(KEY)); + const atResume = fetches; + await waitFor(() => fetches > atResume); // cadence alive again + await act(async () => { root.unmount(); }); + container.remove(); + await setVisibility("visible"); +}); + // A hidden tab has nobody reading the paint, so a passive poll there is pure waste. The // interesting part is what happens on the way back: the skipped ticks are made up once, // rather than the user waiting out the rest of the interval on stale content. diff --git a/gui/tests/visibility-poll.test.ts b/gui/tests/visibility-poll.test.ts new file mode 100644 index 0000000000..8c0b32834b --- /dev/null +++ b/gui/tests/visibility-poll.test.ts @@ -0,0 +1,111 @@ +import { afterEach, beforeEach, expect, test as bunTest } from "bun:test"; +import { Window } from "happy-dom"; +import { startVisibilityPoll } from "../src/visibility-poll"; + +function test(name: string, fn: () => void | Promise): void { + bunTest(name, fn, { timeout: 15_000 }); +} + +const globals = ["document", "window", "navigator"] as const; +let previousGlobals: Record<(typeof globals)[number], unknown>; +let testWindow: Window; + +beforeEach(() => { + previousGlobals = Object.fromEntries(globals.map((key) => [key, Reflect.get(globalThis, key)])) as typeof previousGlobals; + testWindow = new Window({ url: "http://localhost/" }); + Object.defineProperties(globalThis, { + document: { configurable: true, value: testWindow.document }, + window: { configurable: true, value: testWindow }, + navigator: { configurable: true, value: testWindow.navigator }, + }); +}); + +afterEach(() => { + testWindow.close(); + for (const key of globals) { + Object.defineProperty(globalThis, key, { configurable: true, value: previousGlobals[key] }); + } +}); + +/** Same driver as client-resource-poll.test.tsx: happy-dom derives visibilityState. */ +function setVisibility(state: "visible" | "hidden"): void { + Object.defineProperty(testWindow.document, "visibilityState", { + configurable: true, + get: () => state, + }); + testWindow.document.dispatchEvent(new testWindow.Event("visibilitychange")); +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +test("ticks on cadence while visible", async () => { + let calls = 0; + const stop = startVisibilityPoll(() => { calls += 1; }, 40); + await sleep(150); + stop(); + expect(calls).toBeGreaterThanOrEqual(2); +}); + +test("hidden holds no timer and fires nothing", async () => { + let calls = 0; + const stop = startVisibilityPoll(() => { calls += 1; }, 30); + await sleep(75); + const beforeHide = calls; + expect(beforeHide).toBeGreaterThanOrEqual(1); + setVisibility("hidden"); + await sleep(160); // >5 intervals: zero calls without the suspension + expect(calls).toBe(beforeHide); + stop(); +}); + +test("visible-again fires exactly one make-up tick, then resumes the cadence", async () => { + let calls = 0; + const stop = startVisibilityPoll(() => { calls += 1; }, 40); + await sleep(50); + setVisibility("hidden"); + await sleep(60); + const atHidden = calls; + setVisibility("visible"); + await sleep(10); + expect(calls).toBe(atHidden + 1); // the make-up tick + await sleep(90); + expect(calls).toBeGreaterThanOrEqual(atHidden + 2); // cadence resumed + stop(); +}); + +test("pauseWhenHidden:false keeps ticking while hidden", async () => { + let calls = 0; + const stop = startVisibilityPoll(() => { calls += 1; }, 30, { pauseWhenHidden: false }); + await sleep(40); + setVisibility("hidden"); + const atHidden = calls; + await sleep(100); + expect(calls).toBeGreaterThan(atHidden); + stop(); +}); + +test("stop() while hidden leaves no zombie wakeup", async () => { + let calls = 0; + const stop = startVisibilityPoll(() => { calls += 1; }, 30); + await sleep(40); + setVisibility("hidden"); + stop(); + const atStop = calls; + setVisibility("visible"); + await sleep(100); + expect(calls).toBe(atStop); +}); + +test("mounted while already hidden: no timer, but the listener still resumes it", async () => { + setVisibility("hidden"); + let calls = 0; + const stop = startVisibilityPoll(() => { calls += 1; }, 30); + await sleep(100); + expect(calls).toBe(0); + setVisibility("visible"); + await sleep(10); + expect(calls).toBe(1); // make-up tick proves the listener was installed + stop(); +});