diff --git a/mobile/app/index.tsx b/mobile/app/index.tsx index 9e37e9aa5ce..57532a4a8ae 100644 --- a/mobile/app/index.tsx +++ b/mobile/app/index.tsx @@ -217,7 +217,6 @@ export default function HomeScreen() { const [hostAttempts, setHostAttempts] = useState>({}) const [hostLastConnected, setHostLastConnected] = useState>({}) const [statsByHost, setStatsByHost] = useState>({}) - const stats = useMemo(() => totalHomeStats(statsByHost), [statsByHost]) const [worktreeInfo, setWorktreeInfo] = useState>({}) const [accountsByHost, setAccountsByHost] = useState>({}) const [taskProvidersByHost, setTaskProvidersByHost] = useState>({}) @@ -230,6 +229,8 @@ export default function HomeScreen() { // Why: shared clients from the per-host store, not N independent WebSockets. See docs/mobile-shared-client-per-host.md. const hostIds = useMemo(() => hosts.map((h) => h.id), [hosts]) + // Why: scoped to the paired hosts so an unpaired desktop's cached reply leaves the header total. + const stats = useMemo(() => totalHomeStats(statsByHost, hostIds), [statsByHost, hostIds]) const allClients = useAllHostClients(hostIds) const hostPaths = useMemo( () => Object.fromEntries(allClients.map(({ hostId, path }) => [hostId, path])), @@ -402,62 +403,70 @@ export default function HomeScreen() { }) }, [allClients, hosts]) - // Per-host notif/accounts subs + a snapshot read per connect; re-runs per (hostId, client) pair, socket stays open so it's cheap. - useEffect(() => { - const cleanups: Array<() => void> = [] - for (const entry of allClients) { - let unsubNotif: (() => void) | null = null - let unsubAccounts: (() => void) | null = null - const refetchGate = createHostConnectRefetchGate() - const wireUp = (state: ConnectionState) => { - const reconnected = refetchGate.observe(state) - if (state === 'connected') { - if (!unsubNotif) { - unsubNotif = subscribeToDesktopNotifications(entry.client, entry.hostId) - } - if (!unsubAccounts) { - unsubAccounts = entry.client.subscribe('accounts.subscribe', null, (payload) => { - if (!payload || typeof payload !== 'object') { - return + // Notif/accounts subs + a snapshot read per connect for one host. Lives outside the effect body + // because react-doctor's effect-needs-cleanup false-positives on `subscribe` inside one; the + // returned disposer owns every handle allocated here. + const wireHostSubscriptions = (entry: { + hostId: string + client: RpcClient + state: ConnectionState + }) => { + let unsubNotif: (() => void) | null = null + let unsubAccounts: (() => void) | null = null + const refetchGate = createHostConnectRefetchGate() + const wireUp = (state: ConnectionState) => { + const reconnected = refetchGate.observe(state) + if (state === 'connected') { + if (!unsubNotif) { + unsubNotif = subscribeToDesktopNotifications(entry.client, entry.hostId) + } + if (!unsubAccounts) { + unsubAccounts = entry.client.subscribe('accounts.subscribe', null, (payload) => { + if (!payload || typeof payload !== 'object') { + return + } + const evt = payload as { type?: string; snapshot?: unknown } + if (evt.type === 'ready' || evt.type === 'snapshot') { + try { + const snapshot = decodeAccountsSnapshot(evt.snapshot) + setAccountsByHost((prev) => ({ ...prev, [entry.hostId]: snapshot })) + } catch { + // Keep the last proven snapshot; malformed remote data must + // not enter render state or crash the home host cards. } - const evt = payload as { type?: string; snapshot?: unknown } - if (evt.type === 'ready' || evt.type === 'snapshot') { - try { - const snapshot = decodeAccountsSnapshot(evt.snapshot) - setAccountsByHost((prev) => ({ ...prev, [entry.hostId]: snapshot })) - } catch { - // Keep the last proven snapshot; malformed remote data must - // not enter render state or crash the home host cards. - } - } - }) - } - // Why: the socket survives backgrounding/handoffs by reconnecting, so re-read the host - // snapshot on every reconnect — a one-shot latch left the card on stale data forever. - if (reconnected) { - fetchStats(entry.client, entry.hostId, setStatsByHost, () => false) - void fetchHomeHostWorktreeInfo(entry.client, entry.hostId, setWorktreeInfo, () => false) - fetchTaskProviders(entry.client, entry.hostId, setTaskProvidersByHost, () => false) - } - } else { - if (unsubNotif) { - unsubNotif() - unsubNotif = null - } - if (unsubAccounts) { - unsubAccounts() - unsubAccounts = null - } + } + }) + } + // Why: the socket survives backgrounding/handoffs by reconnecting, so re-read the host + // snapshot on every reconnect — a one-shot latch left the card on stale data forever. + if (reconnected) { + fetchStats(entry.client, entry.hostId, setStatsByHost, () => false) + void fetchHomeHostWorktreeInfo(entry.client, entry.hostId, setWorktreeInfo, () => false) + fetchTaskProviders(entry.client, entry.hostId, setTaskProvidersByHost, () => false) + } + } else { + if (unsubNotif) { + unsubNotif() + unsubNotif = null + } + if (unsubAccounts) { + unsubAccounts() + unsubAccounts = null } } - wireUp(entry.state) - const unsubState = entry.client.onStateChange(wireUp) - cleanups.push(() => { - unsubState() - unsubNotif?.() - unsubAccounts?.() - }) } + wireUp(entry.state) + const unsubState = entry.client.onStateChange(wireUp) + return () => { + unsubState() + unsubNotif?.() + unsubAccounts?.() + } + } + + // Re-runs per (hostId, client) pair; the socket stays open so it's cheap. + useEffect(() => { + const cleanups = allClients.map((entry) => wireHostSubscriptions(entry)) return () => { for (const c of cleanups) { c() diff --git a/mobile/src/stats/home-stats-total.test.ts b/mobile/src/stats/home-stats-total.test.ts index 79888376914..035f6c7476f 100644 --- a/mobile/src/stats/home-stats-total.test.ts +++ b/mobile/src/stats/home-stats-total.test.ts @@ -13,18 +13,21 @@ function stats(overrides: Partial = {}): HomeStatsSummary { describe('totalHomeStats', () => { it('has nothing to show before any host answers', () => { - expect(totalHomeStats({})).toBeNull() + expect(totalHomeStats({}, ['host-1'])).toBeNull() }) it('passes a single host through unchanged', () => { - expect(totalHomeStats({ 'host-1': stats() })).toEqual(stats()) + expect(totalHomeStats({ 'host-1': stats() }, ['host-1'])).toEqual(stats()) }) it('sums every desktop instead of letting the last reply win', () => { - const total = totalHomeStats({ - 'host-1': stats(), - 'host-2': stats({ totalAgentsSpawned: 5, totalPRsCreated: 1, totalAgentTimeMs: 30_000 }) - }) + const total = totalHomeStats( + { + 'host-1': stats(), + 'host-2': stats({ totalAgentsSpawned: 5, totalPRsCreated: 1, totalAgentTimeMs: 30_000 }) + }, + ['host-1', 'host-2'] + ) expect(total).toMatchObject({ totalAgentsSpawned: 15, @@ -33,26 +36,45 @@ describe('totalHomeStats', () => { }) }) + it('drops a removed desktop from the total', () => { + // Replies are cached for the life of the process, so the entry outlives the pairing. + const byHost = { + 'host-1': stats(), + 'host-2': stats({ totalAgentsSpawned: 5, totalPRsCreated: 1, totalAgentTimeMs: 30_000 }) + } + + expect(totalHomeStats(byHost, ['host-1'])).toEqual(stats()) + expect(totalHomeStats(byHost, [])).toBeNull() + }) + it('keeps the earliest known first event', () => { - const total = totalHomeStats({ - 'host-1': stats({ firstEventAt: 2_000 }), - 'host-2': stats({ firstEventAt: null }), - 'host-3': stats({ firstEventAt: 1_000 }) - }) + const total = totalHomeStats( + { + 'host-1': stats({ firstEventAt: 2_000 }), + 'host-2': stats({ firstEventAt: null }), + 'host-3': stats({ firstEventAt: 1_000 }) + }, + ['host-1', 'host-2', 'host-3'] + ) expect(total?.firstEventAt).toBe(1_000) }) it('reports no first event when no host has one', () => { - expect(totalHomeStats({ 'host-1': stats({ firstEventAt: null }) })?.firstEventAt).toBeNull() + expect( + totalHomeStats({ 'host-1': stats({ firstEventAt: null }) }, ['host-1'])?.firstEventAt + ).toBeNull() }) it('ignores a malformed reply instead of poisoning the header', () => { - const total = totalHomeStats({ - 'host-1': stats(), - 'host-2': null as unknown as HomeStatsSummary, - 'host-3': { totalAgentsSpawned: 'lots' } as unknown as HomeStatsSummary - }) + const total = totalHomeStats( + { + 'host-1': stats(), + 'host-2': null as unknown as HomeStatsSummary, + 'host-3': { totalAgentsSpawned: 'lots' } as unknown as HomeStatsSummary + }, + ['host-1', 'host-2', 'host-3'] + ) expect(total).toEqual(stats()) }) diff --git a/mobile/src/stats/home-stats-total.ts b/mobile/src/stats/home-stats-total.ts index 9a225726619..72264872561 100644 --- a/mobile/src/stats/home-stats-total.ts +++ b/mobile/src/stats/home-stats-total.ts @@ -9,9 +9,15 @@ export type HomeStatsSummary = { * Why: the home header shows one lifetime-usage row for every paired desktop. Each host answers * stats.summary for itself, so a single shared slot made the row flip to whichever host replied * last — visible churn now that every reconnect re-reads. Sum instead; one host still totals itself. + * + * Summing only `hostIds` keeps an unpaired desktop out of the total: replies are cached per host + * for the life of the process, so an entry outlives the host it describes. */ -export function totalHomeStats(byHost: Record): HomeStatsSummary | null { - const hosts = Object.values(byHost) +export function totalHomeStats( + byHost: Record, + hostIds: readonly string[] +): HomeStatsSummary | null { + const hosts = hostIds.filter((id) => id in byHost).map((id) => byHost[id]) if (hosts.length === 0) { return null } diff --git a/mobile/src/worktree/home-worktree-info.test.ts b/mobile/src/worktree/home-worktree-info.test.ts index b9acc722c68..5cb80d007dd 100644 --- a/mobile/src/worktree/home-worktree-info.test.ts +++ b/mobile/src/worktree/home-worktree-info.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' import { - HOME_WORKTREE_COUNTS_STALE_MAX_AGE_MS, + HOME_WORKTREE_COUNTS_LIVE_MAX_AGE_MS, homeHostWorktreeSummary, markHomeWorktreeCatalogUnavailable, type HostWorktreeInfo @@ -76,27 +76,41 @@ describe('homeHostWorktreeSummary', () => { ) }) - it('stops calling aged-out counts "last known"', () => { + it('keeps the last proven counts however long ago the refresh failed', () => { const afterFailure = markHomeWorktreeCatalogUnavailable(loaded, 'host-1') + // A session that stays connected past the live window then hits one failed refresh is the + // headline case: the counts are history, not nothing. expect( - homeHostWorktreeSummary(afterFailure, provenAt + HOME_WORKTREE_COUNTS_STALE_MAX_AGE_MS) + homeHostWorktreeSummary(afterFailure, provenAt + HOME_WORKTREE_COUNTS_LIVE_MAX_AGE_MS + 1) ).toBe('Last known: 12 worktrees · 2 active') - // A snapshot rehydrated from a previous app session describes a host we have not reached. - expect( - homeHostWorktreeSummary(afterFailure, provenAt + HOME_WORKTREE_COUNTS_STALE_MAX_AGE_MS + 1) - ).toBe('Worktree list unavailable') expect(homeHostWorktreeSummary(afterFailure, provenAt + 3 * 86_400_000)).toBe( - 'Worktree list unavailable' + 'Last known: 12 worktrees · 2 active' ) }) - it('treats counts persisted before age stamping as unproven', () => { + it('stops calling aged-out counts live even while a refresh is only pending', () => { + // A snapshot rehydrated from a previous app session describes a host we have not reached yet. + expect(homeHostWorktreeSummary(loaded, provenAt + HOME_WORKTREE_COUNTS_LIVE_MAX_AGE_MS)).toBe( + '12 worktrees · 2 active' + ) + expect( + homeHostWorktreeSummary(loaded, provenAt + HOME_WORKTREE_COUNTS_LIVE_MAX_AGE_MS + 1) + ).toBe('Last known: 12 worktrees · 2 active') + expect(homeHostWorktreeSummary(loaded, provenAt + 3 * 86_400_000)).toBe( + 'Last known: 12 worktrees · 2 active' + ) + }) + + it('treats counts persisted before age stamping as no longer live', () => { const preUpgrade: HostWorktreeInfo = { ...loaded, countsProvenAt: undefined } + expect(homeHostWorktreeSummary(preUpgrade, provenAt)).toBe( + 'Last known: 12 worktrees · 2 active' + ) expect( homeHostWorktreeSummary(markHomeWorktreeCatalogUnavailable(preUpgrade, 'host-1'), provenAt) - ).toBe('Worktree list unavailable') + ).toBe('Last known: 12 worktrees · 2 active') }) it('reports unavailable only when no catalog ever loaded', () => { diff --git a/mobile/src/worktree/home-worktree-info.ts b/mobile/src/worktree/home-worktree-info.ts index 00e05640824..a417636dcaf 100644 --- a/mobile/src/worktree/home-worktree-info.ts +++ b/mobile/src/worktree/home-worktree-info.ts @@ -22,9 +22,9 @@ export type HostWorktreeInfo = { countsProvenAt?: number } -// Why: the home snapshot is persisted across launches, so "Last known" could otherwise describe a -// workspace set proven days ago. Past this window the counts stop being evidence about the host. -export const HOME_WORKTREE_COUNTS_STALE_MAX_AGE_MS = 10 * 60_000 +// Why: the home snapshot is persisted across launches, so counts could otherwise be rendered as +// live while describing a workspace set proven days ago. Past this window they are only history. +export const HOME_WORKTREE_COUNTS_LIVE_MAX_AGE_MS = 10 * 60_000 export function markHomeWorktreeCatalogUnavailable( current: HostWorktreeInfo | undefined, @@ -55,24 +55,23 @@ export function homeHostWorktreeSummary( if (!info) { return null } - // Why (STA-3123): a catalog that never loaded — or whose counts have aged out of the - // "last known" window — must not assert a count the host has not proven. - if (info.catalogUnavailable && !hasFreshEnoughCounts(info, now)) { + // Why (STA-3123): a catalog that never loaded must not assert a count the host has not proven. + if (info.catalogUnavailable && !info.staleCounts) { return 'Worktree list unavailable' } const counts = `${info.totalWorktrees} worktree${info.totalWorktrees === 1 ? '' : 's'}${ info.activeCount > 0 ? ` · ${info.activeCount} active` : '' }` - return info.staleCounts ? `Last known: ${counts}` : counts + // Age bounds liveness, not the counts themselves: a failed refresh — or a rehydrated snapshot the + // host has not re-confirmed — is still the last thing it told us, so keep it and drop the claim + // that it is current. + return info.staleCounts || !provenRecently(info, now) ? `Last known: ${counts}` : counts } -function hasFreshEnoughCounts(info: HostWorktreeInfo, now: number): boolean { - if (!info.staleCounts) { - return false - } +function provenRecently(info: HostWorktreeInfo, now: number): boolean { // A snapshot written before countsProvenAt existed has an unknowable age; treat it as expired. return ( typeof info.countsProvenAt === 'number' && - now - info.countsProvenAt <= HOME_WORKTREE_COUNTS_STALE_MAX_AGE_MS + now - info.countsProvenAt <= HOME_WORKTREE_COUNTS_LIVE_MAX_AGE_MS ) }