From 02a417c04b495c2c3a2daea4a83b5fe3fa8fc688 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:35:48 -0700 Subject: [PATCH] perf(renderer): stop six timers from ticking behind a hidden window (#18134) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf(renderer): stop six timers from ticking behind a hidden window IntensiveWakeUpThrottling is disabled in this app, so a renderer interval really does fire at full rate with the window hidden. Six of them had nothing to observe them: - NativeChatWorkingStatus ran a 1s interval + setState per in-flight turn purely to advance an elapsed-seconds counter. Deleted the effect and derived elapsed during render from the shared, visibility-gated useNow(1_000) clock, so N turns collapse onto one tick. - The chromium-error fallback poll (250ms) kept probing a stuck-loading guest to write a loadError nobody could see. - The contextual-tour full-pass interval (500ms) woke twice a second to queue a rAF a hidden window never paints. - Three feature-wall animation timers (3600/2400/2400ms) kept committing React renders for animations nobody was watching. - The landing preflight poll (30s) kept forcing IPC refreshes. All five gated timers reuse installWindowVisibilityInterval. Each either resumes where it left off (animations) or re-derives from durable state on the becoming-visible run, so hiding and re-showing is observationally identical to never hiding. * test(git): stop two empty commits in the divergence fixture from hashing alike `counts drift in both directions` builds 100 empty commits, resets to the fork point, then adds one more — expecting 100 ahead + 1 behind to clear the cap of 100. An empty commit's hash covers only parent, tree, message and a one-second-granularity timestamp, and every commit in the fixture reuses `commit ${index}` starting from 0. On a runner fast enough to finish the whole build inside one wall-clock second (CI: 1059ms for the case, ~7ms per commit), the post-reset `commit 0` hashed identically to the first `commit 0` of the chain, so Git handed back that same object and left the branch 99/0 apart instead of 100/1 — `within`, not `exceeded`. Numbering the empty commits across calls makes the fixture build the 101 distinct commits it already claimed to. Reproduced deterministically by pinning GIT_AUTHOR_DATE/GIT_COMMITTER_DATE, which forces the timestamp collision the fast runner hits by chance: fails with the exact CI assertion before, passes after. --- .../worktree-base-divergence-real-git.test.ts | 10 +- ...hromium-error-page-poll-visibility.test.ts | 97 +++++++++++++++ .../use-browser-page-webview-url-sync.ts | 9 +- .../ContextualTourOverlay.tsx | 11 +- .../ContextualTourOverlay.visibility.test.tsx | 106 ++++++++++++++++ .../feature-wall/WorkspacesAnimatedVisual.tsx | 30 +++-- .../agents-orchestration/StatusesPage.tsx | 28 +++-- ...feature-wall-animation-visibility.test.tsx | 115 ++++++++++++++++++ .../use-workbench-terminal-storyboard.ts | 12 +- ...landing-preflight-runtime-boundary.test.ts | 30 +++++ .../components/landing-preflight-runtime.ts | 15 ++- .../native-chat/NativeChatWorkingStatus.tsx | 26 ++-- ...-chat-working-status-shared-clock.test.tsx | 94 ++++++++++++++ 13 files changed, 534 insertions(+), 49 deletions(-) create mode 100644 src/renderer/src/components/browser-pane/navigate/chromium-error-page-poll-visibility.test.ts create mode 100644 src/renderer/src/components/contextual-tours/ContextualTourOverlay.visibility.test.tsx create mode 100644 src/renderer/src/components/feature-wall/feature-wall-animation-visibility.test.tsx create mode 100644 src/renderer/src/components/native-chat/native-chat-working-status-shared-clock.test.tsx diff --git a/src/main/git/worktree-base-divergence-real-git.test.ts b/src/main/git/worktree-base-divergence-real-git.test.ts index e17192cf914..377b63b48f9 100644 --- a/src/main/git/worktree-base-divergence-real-git.test.ts +++ b/src/main/git/worktree-base-divergence-real-git.test.ts @@ -32,9 +32,17 @@ async function createRepo(): Promise { return repoPath } +// Why unique across calls: an empty commit's hash covers only parent, tree, message and a +// one-second-granularity timestamp. On a fast runner the whole 100-commit build finishes inside +// one second, so a post-reset `commit 0` off the same fork point hashed identically to the first +// `commit 0` of the chain and Git handed back that same object — leaving the branch 99/0 apart +// instead of 100/1. +let emptyCommitSequence = 0 + function commitEmpty(repoPath: string, count: number): void { for (let index = 0; index < count; index += 1) { - git(repoPath, ['commit', '--quiet', '--allow-empty', '-m', `commit ${index}`]) + emptyCommitSequence += 1 + git(repoPath, ['commit', '--quiet', '--allow-empty', '-m', `commit ${emptyCommitSequence}`]) } } diff --git a/src/renderer/src/components/browser-pane/navigate/chromium-error-page-poll-visibility.test.ts b/src/renderer/src/components/browser-pane/navigate/chromium-error-page-poll-visibility.test.ts new file mode 100644 index 00000000000..2256c4171f7 --- /dev/null +++ b/src/renderer/src/components/browser-pane/navigate/chromium-error-page-poll-visibility.test.ts @@ -0,0 +1,97 @@ +// @vitest-environment happy-dom + +import { act, cleanup, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { BrowserTabPageState } from '../describe-page/browser-page-types' +import { useBrowserPageWebviewUrlSync } from './use-browser-page-webview-url-sync' + +const CHROMIUM_ERROR_URL = 'chrome-error://chromewebdata/' + +function setDocumentVisibility(state: 'visible' | 'hidden'): void { + Object.defineProperty(document, 'visibilityState', { configurable: true, get: () => state }) + act(() => { + document.dispatchEvent(new Event('visibilitychange')) + }) +} + +function renderUrlSync(guestUrl: () => string): { + updates: [string, BrowserTabPageState][] + unmount: () => void +} { + const updates: [string, BrowserTabPageState][] = [] + const webview = { getURL: () => guestUrl(), src: '' } as unknown as Electron.WebviewTag + const view = renderHook(() => + useBrowserPageWebviewUrlSync({ + browserTabId: 'tab-1', + browserTabUrl: 'https://example.test/slow', + browserTabLoading: true, + isActive: true, + isPaintable: true, + slotViewport: null, + webviewRef: { current: webview }, + chromeHeaderRef: { current: null }, + lastKnownWebviewUrlRef: { current: 'https://example.test/slow' }, + trackNextLoadingEventRef: { current: false }, + keepAddressBarFocusRef: { current: false }, + addressBarInputRef: { current: null }, + browserTabUrlRef: { current: 'https://example.test/slow' }, + addressBarValueRef: { current: 'https://example.test/slow' }, + onUpdatePageStateRef: { + current: (tabId, patch) => { + updates.push([tabId, patch]) + } + }, + focusWebviewNow: () => false + }) + ) + return { updates, unmount: view.unmount } +} + +beforeEach(() => { + vi.useFakeTimers() + setDocumentVisibility('visible') +}) + +afterEach(() => { + cleanup() + setDocumentVisibility('visible') + vi.useRealTimers() +}) + +describe('chromium error page poll visibility gate', () => { + it('stops the 250ms poll while hidden and re-detects the error page on return', () => { + let guestUrl = 'https://example.test/slow' + const { updates, unmount } = renderUrlSync(() => guestUrl) + + // Visible and still loading: the fallback poll is armed. + expect(vi.getTimerCount()).toBe(1) + act(() => vi.advanceTimersByTime(1_000)) + expect(updates).toHaveLength(0) + + setDocumentVisibility('hidden') + expect(vi.getTimerCount()).toBe(0) + + // The guest lands on a chrome-error page while nobody can see the surface. + guestUrl = CHROMIUM_ERROR_URL + act(() => vi.advanceTimersByTime(10_000)) + expect(updates).toHaveLength(0) + + // Returning re-reads the durable guest URL, so the loadError is not lost. + setDocumentVisibility('visible') + expect(updates).toHaveLength(1) + expect(updates[0]?.[1].loadError?.validatedUrl).toBe('https://example.test/slow') + expect(updates[0]?.[1].loading).toBe(false) + + unmount() + expect(vi.getTimerCount()).toBe(0) + }) + + it('polls unchanged while the window stays visible', () => { + let guestUrl = 'https://example.test/slow' + const { updates } = renderUrlSync(() => guestUrl) + + guestUrl = CHROMIUM_ERROR_URL + act(() => vi.advanceTimersByTime(250)) + expect(updates).toHaveLength(1) + }) +}) diff --git a/src/renderer/src/components/browser-pane/navigate/use-browser-page-webview-url-sync.ts b/src/renderer/src/components/browser-pane/navigate/use-browser-page-webview-url-sync.ts index 0c75ddefce0..bfd93702440 100644 --- a/src/renderer/src/components/browser-pane/navigate/use-browser-page-webview-url-sync.ts +++ b/src/renderer/src/components/browser-pane/navigate/use-browser-page-webview-url-sync.ts @@ -9,6 +9,7 @@ import { applyBrowserPageViewportLayout, syncBrowserPageChromeInset } from '../host-guest/browser-page-viewport' +import { installWindowVisibilityInterval } from '@/lib/window-visibility-interval' import { shouldPollChromiumErrorPage } from './chromium-error-page-polling' import { isChromiumErrorPage } from '../describe-page/browser-page-url-display' import type { BrowserTabPageState } from '../describe-page/browser-page-types' @@ -152,9 +153,11 @@ export function useBrowserPageWebviewUrlSync({ } // Why: some Electron builds paint chrome-error pages without a did-fail-load event; poll only while the active tab loads as a fallback. - detectChromiumErrorPage() - const intervalId = window.setInterval(detectChromiumErrorPage, 250) - return () => window.clearInterval(intervalId) + // Why gated: a page stuck loading would otherwise poll 4x/sec forever behind a hidden window. The guest URL is durable state, so the becoming-visible run re-derives anything a hidden window skipped. + return installWindowVisibilityInterval({ + run: detectChromiumErrorPage, + intervalMs: 250 + }) }, [ addressBarValueRef, browserTabId, diff --git a/src/renderer/src/components/contextual-tours/ContextualTourOverlay.tsx b/src/renderer/src/components/contextual-tours/ContextualTourOverlay.tsx index 243bfcc48c4..119ca73d451 100644 --- a/src/renderer/src/components/contextual-tours/ContextualTourOverlay.tsx +++ b/src/renderer/src/components/contextual-tours/ContextualTourOverlay.tsx @@ -24,6 +24,7 @@ import { handleContextualTourOverlayKeyDown, type ActiveTourRenderState } from './ContextualTourOverlaySurface' +import { installWindowVisibilityInterval } from '@/lib/window-visibility-interval' import { requestActiveTerminalPaneSplit } from '@/components/tab-bar/request-active-terminal-pane-split' import { performContextualTourStepAction } from './contextual-tour-step-actions' import { openWorkspaceCreationComposerWithTourHandoff } from './workspace-creation-tour-handoff' @@ -228,14 +229,20 @@ export function ContextualTourOverlay(): JSX.Element | null { const scheduleFullMeasure = (): void => scheduleMeasure(true) window.addEventListener('resize', scheduleFullMeasure) window.addEventListener('scroll', scheduleTargetMeasure, true) - const interval = window.setInterval(scheduleFullMeasure, 500) + // Why gated: a hidden window paints no frames, so the queued rAF never runs + // and the pass is pure wakeup. The becoming-visible run re-queues it, and + // the layout effect measures on every render, so nothing is missed. + const stopFullPassInterval = installWindowVisibilityInterval({ + run: scheduleFullMeasure, + intervalMs: 500 + }) return () => { if (frame !== null) { window.cancelAnimationFrame(frame) } window.removeEventListener('resize', scheduleFullMeasure) window.removeEventListener('scroll', scheduleTargetMeasure, true) - window.clearInterval(interval) + stopFullPassInterval() } }, [activeTourId, measureTourOverlay]) diff --git a/src/renderer/src/components/contextual-tours/ContextualTourOverlay.visibility.test.tsx b/src/renderer/src/components/contextual-tours/ContextualTourOverlay.visibility.test.tsx new file mode 100644 index 00000000000..f8b6fe0094d --- /dev/null +++ b/src/renderer/src/components/contextual-tours/ContextualTourOverlay.visibility.test.tsx @@ -0,0 +1,106 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { ContextualTourOverlay } from './ContextualTourOverlay' +import { useAppStore } from '@/store' + +let container: HTMLDivElement +let root: Root + +function tourTarget(name: string, top: number): { moveTo: (top: number) => void } { + let currentTop = top + const element = document.createElement('div') + element.setAttribute('data-contextual-tour-target', name) + Object.defineProperty(element, 'getBoundingClientRect', { + configurable: true, + value: () => ({ + left: 100, + right: 220, + top: currentTop, + bottom: currentTop + 40, + width: 120, + height: 40, + x: 100, + y: currentTop + }) + }) + document.body.appendChild(element) + return { + moveTo: (next) => { + currentTop = next + } + } +} + +async function settle(ms: number): Promise { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, ms)) + }) +} + +async function setDocumentVisibility(state: 'visible' | 'hidden'): Promise { + Object.defineProperty(document, 'visibilityState', { configurable: true, get: () => state }) + await act(async () => { + document.dispatchEvent(new Event('visibilitychange')) + await new Promise((resolve) => setTimeout(resolve, 0)) + }) +} + +function ringsTop(): string | undefined { + return container.querySelector('[data-contextual-tour-target-rings]')?.style.top +} + +beforeEach(async () => { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + ;(window as unknown as { api: unknown }).api = { ui: { set: () => Promise.resolve() } } + Object.defineProperty(window, 'innerWidth', { configurable: true, value: 1280 }) + Object.defineProperty(window, 'innerHeight', { configurable: true, value: 960 }) + await setDocumentVisibility('visible') + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(async () => { + act(() => root.unmount()) + container.remove() + document.querySelectorAll('[data-contextual-tour-target]').forEach((node) => node.remove()) + await setDocumentVisibility('visible') + useAppStore.setState({ activeContextualTourId: null, activeContextualTourStepIndex: 0 }) +}) + +describe('ContextualTourOverlay full-pass interval visibility gate', () => { + it('pauses the 500ms full pass while hidden and re-measures on return', async () => { + const target = tourTarget('workspace-create-control', 300) + useAppStore.setState({ + activeContextualTourId: 'workspace-agent-sessions', + activeContextualTourStepIndex: 1, + activeModal: 'none', + contextualToursOnboardingVisible: false, + contextualToursBlockingSurfaceVisible: false, + activeContextualTourSuppressed: false + }) + await act(async () => { + root.render() + await new Promise((resolve) => setTimeout(resolve, 50)) + }) + expect(ringsTop()).toBe('300px') + + // Baseline: while visible, the periodic full pass follows a silent move. + target.moveTo(640) + await settle(700) + expect(ringsTop()).toBe('640px') + + await setDocumentVisibility('hidden') + target.moveTo(900) + await settle(1_500) + expect(ringsTop()).toBe('640px') + + // Returning runs the pass immediately, so the overlay is never left stale. + await setDocumentVisibility('visible') + await settle(50) + expect(ringsTop()).toBe('900px') + }) +}) diff --git a/src/renderer/src/components/feature-wall/WorkspacesAnimatedVisual.tsx b/src/renderer/src/components/feature-wall/WorkspacesAnimatedVisual.tsx index 7240684aeb8..bc4260612fe 100644 --- a/src/renderer/src/components/feature-wall/WorkspacesAnimatedVisual.tsx +++ b/src/renderer/src/components/feature-wall/WorkspacesAnimatedVisual.tsx @@ -1,6 +1,7 @@ import { useEffect, useMemo, useState } from 'react' import type { JSX } from 'react' import { AgentStateDot } from '@/components/AgentStateDot' +import { installWindowVisibilityInterval } from '@/lib/window-visibility-interval' import { ClaudeIcon, OpenCodeGoIcon } from '../status-bar/icons' type AgentKind = 'claude' | 'codex' | 'opencode' @@ -87,18 +88,23 @@ export function WorkspacesAnimatedVisual(props: { reducedMotion: boolean }): JSX if (reducedMotion) { return } - const id = window.setInterval(() => { - setVisualState((current) => { - const next = current.order.slice() - const finishing = next.pop() - if (!finishing) { - return current - } - next.unshift(finishing) - return { order: next, promotedWorkspaceId: finishing.id } - }) - }, STEP_MS) - return () => window.clearInterval(id) + // Why: nobody watches an animation in a hidden window. `runOnVisible` is a + // no-op so revealing the window resumes the cycle instead of skipping a card. + return installWindowVisibilityInterval({ + run: () => { + setVisualState((current) => { + const next = current.order.slice() + const finishing = next.pop() + if (!finishing) { + return current + } + next.unshift(finishing) + return { order: next, promotedWorkspaceId: finishing.id } + }) + }, + runOnVisible: () => {}, + intervalMs: STEP_MS + }) }, [reducedMotion]) // Why: reduced-motion mode should display the static stack without a // post-render repair; only the animated interval needs promoted z-order. diff --git a/src/renderer/src/components/feature-wall/agents-orchestration/StatusesPage.tsx b/src/renderer/src/components/feature-wall/agents-orchestration/StatusesPage.tsx index acbfcb889ff..dd4412cf282 100644 --- a/src/renderer/src/components/feature-wall/agents-orchestration/StatusesPage.tsx +++ b/src/renderer/src/components/feature-wall/agents-orchestration/StatusesPage.tsx @@ -5,6 +5,7 @@ import { Wrench } from 'lucide-react' import { AgentStateDot } from '@/components/AgentStateDot' import { getAgentCatalog, AgentIcon, type AgentCatalogEntry } from '@/lib/agent-catalog' import { ClaudeIcon, OpenAIIcon } from '../../status-bar/icons' +import { installWindowVisibilityInterval } from '@/lib/window-visibility-interval' import { cn } from '@/lib/utils' import { translate } from '@/i18n/i18n' @@ -48,18 +49,25 @@ export function StatusesPage(props: { active: boolean; reducedMotion: boolean }) schedule(() => setRevealed((r) => ({ ...r, codex: true })), 1900) let idx = 0 - const cycleId = window.setInterval(() => { - setClaudeFading(true) - const swap = window.setTimeout(() => { - idx = (idx + 1) % CLAUDE_ACTIVITIES.length - setClaudeIdx(idx) - setClaudeFading(false) - }, 280) - timeouts.push(swap) - }, 2400) + // Why: nobody watches an animation in a hidden window. `runOnVisible` is a + // no-op so revealing the window resumes the cycle instead of skipping an + // activity; `idx` lives outside the timer, so the reveal picks up where it left off. + const stopCycle = installWindowVisibilityInterval({ + run: () => { + setClaudeFading(true) + const swap = window.setTimeout(() => { + idx = (idx + 1) % CLAUDE_ACTIVITIES.length + setClaudeIdx(idx) + setClaudeFading(false) + }, 280) + timeouts.push(swap) + }, + runOnVisible: () => {}, + intervalMs: 2400 + }) return () => { timeouts.forEach((id) => window.clearTimeout(id)) - window.clearInterval(cycleId) + stopCycle() } }, [active, reducedMotion]) diff --git a/src/renderer/src/components/feature-wall/feature-wall-animation-visibility.test.tsx b/src/renderer/src/components/feature-wall/feature-wall-animation-visibility.test.tsx new file mode 100644 index 00000000000..334356ef1a4 --- /dev/null +++ b/src/renderer/src/components/feature-wall/feature-wall-animation-visibility.test.tsx @@ -0,0 +1,115 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { renderHook } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { TooltipProvider } from '@/components/ui/tooltip' +import { StatusesPage } from './agents-orchestration/StatusesPage' +import { useWorkbenchTerminalStoryboard } from './use-workbench-terminal-storyboard' +import { WorkspacesAnimatedVisual } from './WorkspacesAnimatedVisual' + +let container: HTMLDivElement +let root: Root + +function setDocumentVisibility(state: 'visible' | 'hidden'): void { + Object.defineProperty(document, 'visibilityState', { configurable: true, get: () => state }) + act(() => { + document.dispatchEvent(new Event('visibilitychange')) + }) +} + +function cardOrder(): string[] { + return Array.from(container.querySelectorAll('[data-ws-id]')) + .map((node) => ({ + id: node.dataset.wsId ?? '', + top: Number.parseFloat(node.style.transform.replace(/[^\d.-]/g, '')) || 0 + })) + .sort((left, right) => left.top - right.top) + .map((card) => card.id) +} + +beforeEach(() => { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + vi.useFakeTimers() + setDocumentVisibility('visible') + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(() => { + act(() => root.unmount()) + container.remove() + setDocumentVisibility('visible') + vi.useRealTimers() +}) + +describe('feature wall animation timers', () => { + it('pauses the workspaces card rotation while hidden and resumes without skipping', () => { + act(() => + root.render( + + + + ) + ) + const initialOrder = cardOrder() + expect(vi.getTimerCount()).toBe(1) + + act(() => vi.advanceTimersByTime(3_600)) + const afterOneStep = cardOrder() + expect(afterOneStep).not.toEqual(initialOrder) + + setDocumentVisibility('hidden') + expect(vi.getTimerCount()).toBe(0) + act(() => vi.advanceTimersByTime(3_600 * 10)) + expect(cardOrder()).toEqual(afterOneStep) + + // Revealing resumes the cycle rather than jumping a card forward. + setDocumentVisibility('visible') + expect(cardOrder()).toEqual(afterOneStep) + expect(vi.getTimerCount()).toBe(1) + act(() => vi.advanceTimersByTime(3_600)) + expect(cardOrder()).not.toEqual(afterOneStep) + }) + + it('pauses the workbench run queue while hidden and resumes from the same entry', () => { + const view = renderHook(() => useWorkbenchTerminalStoryboard('tour', false)) + const first = view.result.current.running + act(() => vi.advanceTimersByTime(2_400)) + const second = view.result.current.running + expect(second).not.toBe(first) + + setDocumentVisibility('hidden') + act(() => vi.advanceTimersByTime(2_400 * 10)) + expect(view.result.current.running).toBe(second) + + setDocumentVisibility('visible') + expect(view.result.current.running).toBe(second) + act(() => vi.advanceTimersByTime(2_400)) + expect(view.result.current.running).not.toBe(second) + view.unmount() + }) + + it('pauses the agent-status activity cycle while hidden and resumes in place', () => { + act(() => + root.render( + + + + ) + ) + act(() => vi.advanceTimersByTime(2_400 + 280)) + const afterOneCycle = container.textContent ?? '' + + setDocumentVisibility('hidden') + act(() => vi.advanceTimersByTime((2_400 + 280) * 10)) + expect(container.textContent).toBe(afterOneCycle) + + setDocumentVisibility('visible') + expect(container.textContent).toBe(afterOneCycle) + act(() => vi.advanceTimersByTime(2_400 + 280)) + expect(container.textContent).not.toBe(afterOneCycle) + }) +}) diff --git a/src/renderer/src/components/feature-wall/use-workbench-terminal-storyboard.ts b/src/renderer/src/components/feature-wall/use-workbench-terminal-storyboard.ts index 919d3928500..4ee95a9d428 100644 --- a/src/renderer/src/components/feature-wall/use-workbench-terminal-storyboard.ts +++ b/src/renderer/src/components/feature-wall/use-workbench-terminal-storyboard.ts @@ -1,4 +1,5 @@ import { useEffect, useState } from 'react' +import { installWindowVisibilityInterval } from '@/lib/window-visibility-interval' import { WORKBENCH_RUN_QUEUE, WORKBENCH_RUN_TICK_MS, @@ -38,10 +39,13 @@ export function useWorkbenchTerminalStoryboard( if (reducedMotion || isTwoAgentsChecklist) { return } - const id = window.setInterval(() => { - setRunIdx((index) => (index + 1) % WORKBENCH_RUN_QUEUE.length) - }, WORKBENCH_RUN_TICK_MS) - return () => window.clearInterval(id) + // Why: nobody watches an animation in a hidden window. `runOnVisible` is a + // no-op so revealing the window resumes the queue instead of skipping an entry. + return installWindowVisibilityInterval({ + run: () => setRunIdx((index) => (index + 1) % WORKBENCH_RUN_QUEUE.length), + runOnVisible: () => {}, + intervalMs: WORKBENCH_RUN_TICK_MS + }) }, [isTwoAgentsChecklist, reducedMotion]) useEffect(() => { diff --git a/src/renderer/src/components/landing-preflight-runtime-boundary.test.ts b/src/renderer/src/components/landing-preflight-runtime-boundary.test.ts index f282ca5f15a..f6426c9e3cf 100644 --- a/src/renderer/src/components/landing-preflight-runtime-boundary.test.ts +++ b/src/renderer/src/components/landing-preflight-runtime-boundary.test.ts @@ -15,6 +15,11 @@ const status = (overrides: Partial = {}): PreflightStatus => ({ ...overrides }) +function setDocumentVisibility(state: 'visible' | 'hidden'): void { + Object.defineProperty(document, 'visibilityState', { configurable: true, get: () => state }) + document.dispatchEvent(new Event('visibilitychange')) +} + const githubRepo: Repo = { id: 'github', path: '/repos/github', @@ -27,6 +32,7 @@ const githubRepo: Repo = { beforeEach(() => { vi.useFakeTimers() + setDocumentVisibility('visible') refresh.mockClear() invalidate.mockClear() useAppStore.setState(useAppStore.getInitialState(), true) @@ -35,6 +41,7 @@ beforeEach(() => { afterEach(() => { cleanup() + setDocumentVisibility('visible') vi.useRealTimers() useAppStore.setState(useAppStore.getInitialState(), true) }) @@ -108,6 +115,29 @@ describe('landing preflight runtime boundary', () => { view.unmount() }) + it('stops the 30s preflight poll while hidden and lets the reveal refresh instead', () => { + useAppStore.setState({ repos: [githubRepo], preflightStatus: status() }) + const view = renderHook(() => useLandingPreflightRuntime()) + refresh.mockClear() + + expect(vi.getTimerCount()).toBe(1) + act(() => setDocumentVisibility('hidden')) + expect(vi.getTimerCount()).toBe(0) + + // Five poll windows pass behind a hidden window with no IPC at all. + act(() => vi.advanceTimersByTime(150_000)) + expect(refresh).not.toHaveBeenCalled() + + // The sibling visibilitychange handler force-refreshes on reveal, so the + // banner is current the moment it can be seen; the poll re-arms behind it. + act(() => setDocumentVisibility('visible')) + expect(refresh).toHaveBeenCalledTimes(1) + expect(refresh).toHaveBeenCalledWith({ force: true }) + expect(vi.getTimerCount()).toBe(1) + + view.unmount() + }) + it('keeps one active interval and removes listeners and polling on cleanup', () => { useAppStore.setState({ repos: [githubRepo], preflightStatus: status() }) const addEventListener = vi.spyOn(document, 'addEventListener') diff --git a/src/renderer/src/components/landing-preflight-runtime.ts b/src/renderer/src/components/landing-preflight-runtime.ts index 57cc9d1c31d..6df10613668 100644 --- a/src/renderer/src/components/landing-preflight-runtime.ts +++ b/src/renderer/src/components/landing-preflight-runtime.ts @@ -1,5 +1,6 @@ import { useEffect, useMemo } from 'react' import { useAppStore } from '../store' +import { installWindowVisibilityInterval } from '@/lib/window-visibility-interval' import { getLandingPreflightIssues, hasGitHubBackedProject, @@ -60,10 +61,16 @@ export function useLandingPreflightRuntime(): { preflightIssues: PreflightIssue[ if (preflightIssues.length === 0) { return } - const intervalId = window.setInterval(() => { - void refreshPreflightStatus({ force: true }) - }, 30000) - return () => window.clearInterval(intervalId) + // Why gated: the effect above already force-refreshes on visibilitychange + // and focus, so a revealed window has fresh data without this poll firing + // while hidden — hence the no-op `runOnVisible`. + return installWindowVisibilityInterval({ + run: () => { + void refreshPreflightStatus({ force: true }) + }, + runOnVisible: () => {}, + intervalMs: 30000 + }) }, [preflightIssues.length, refreshPreflightStatus]) return { preflightIssues } diff --git a/src/renderer/src/components/native-chat/NativeChatWorkingStatus.tsx b/src/renderer/src/components/native-chat/NativeChatWorkingStatus.tsx index 5aef81ba51d..a883429f658 100644 --- a/src/renderer/src/components/native-chat/NativeChatWorkingStatus.tsx +++ b/src/renderer/src/components/native-chat/NativeChatWorkingStatus.tsx @@ -1,6 +1,7 @@ -import { useEffect, useState } from 'react' +import { useState } from 'react' import { ChevronRight } from 'lucide-react' import { translate } from '@/i18n/i18n' +import { useNow } from '@/hooks/use-now' export function NativeChatWorkingStatus({ startedAt, @@ -15,18 +16,17 @@ export function NativeChatWorkingStatus({ expanded?: boolean onToggleExpanded?: () => void }): React.JSX.Element { - const [elapsedSeconds, setElapsedSeconds] = useState(0) - - useEffect(() => { - if (thinking || workedSeconds != null) { - return - } - const epoch = startedAt ?? Date.now() - setElapsedSeconds(Math.max(0, Math.floor((Date.now() - epoch) / 1000))) - const update = () => setElapsedSeconds(Math.max(0, Math.floor((Date.now() - epoch) / 1000))) - const timer = window.setInterval(update, 1000) - return () => window.clearInterval(timer) - }, [startedAt, thinking, workedSeconds]) + // Why: elapsed seconds is ordinary render dataflow, not an external system. + // The shared 1s clock is visibility-gated and collapses every in-flight turn + // onto one tick, instead of one interval plus one commit per turn. + const counting = !thinking && workedSeconds == null + const now = useNow(1_000, counting) + // Why: preserves the old effect's `startedAt ?? Date.now()` epoch for the + // single frame before the turn's startedAt lands. + const [mountedAt] = useState(() => Date.now()) + const elapsedSeconds = counting + ? Math.max(0, Math.floor((now - (startedAt ?? mountedAt)) / 1000)) + : 0 const label = workedSeconds != null diff --git a/src/renderer/src/components/native-chat/native-chat-working-status-shared-clock.test.tsx b/src/renderer/src/components/native-chat/native-chat-working-status-shared-clock.test.tsx new file mode 100644 index 00000000000..dfea16ae0e4 --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-working-status-shared-clock.test.tsx @@ -0,0 +1,94 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { NativeChatWorkingStatus } from './NativeChatWorkingStatus' + +let container: HTMLDivElement +let root: Root + +function setDocumentVisibility(state: 'visible' | 'hidden'): void { + Object.defineProperty(document, 'visibilityState', { configurable: true, get: () => state }) + act(() => { + document.dispatchEvent(new Event('visibilitychange')) + }) +} + +function renderTurns(count: number, startedAt: number): void { + act(() => { + root.render( + <> + {Array.from({ length: count }, (_, index) => ( + + ))} + + ) + }) +} + +function elapsedLabels(): string[] { + return Array.from(container.querySelectorAll('[aria-label]'), (node) => node.textContent ?? '') +} + +beforeEach(() => { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + vi.useFakeTimers() + vi.setSystemTime(1_000_000) + setDocumentVisibility('visible') + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(() => { + act(() => root.unmount()) + container.remove() + setDocumentVisibility('visible') + vi.useRealTimers() +}) + +describe('native chat working status elapsed clock', () => { + it('collapses every in-flight turn onto one shared visibility-gated timer', () => { + renderTurns(3, 1_000_000) + + // One shared 1s clock for all three turns, not one interval per turn. + expect(vi.getTimerCount()).toBe(1) + act(() => vi.advanceTimersByTime(3_000)) + expect(elapsedLabels()).toEqual([ + 'Working for 3 seconds', + 'Working for 3 seconds', + 'Working for 3 seconds' + ]) + }) + + it('stops ticking while hidden and re-syncs the elapsed value on return', () => { + renderTurns(1, 1_000_000) + act(() => vi.advanceTimersByTime(3_000)) + expect(elapsedLabels()).toEqual(['Working for 3 seconds']) + + setDocumentVisibility('hidden') + expect(vi.getTimerCount()).toBe(0) + + // A minute of hidden wall-clock: no callbacks, no commits, label frozen. + act(() => vi.advanceTimersByTime(60_000)) + expect(elapsedLabels()).toEqual(['Working for 3 seconds']) + + // Returning re-derives elapsed from startedAt, so nothing was lost. + setDocumentVisibility('visible') + expect(elapsedLabels()).toEqual(['Working for 63 seconds']) + expect(vi.getTimerCount()).toBe(1) + }) + + it('holds no timer for a thinking turn or a completed turn', () => { + act(() => { + root.render( + <> + + + + ) + }) + expect(vi.getTimerCount()).toBe(0) + }) +})