From 41c7ea6751dcec33cbeeddb0d7c72caf9578e100 Mon Sep 17 00:00:00 2001 From: OrcaWin <293788423+OrcaWin@users.noreply.github.com> Date: Tue, 4 Aug 2026 00:21:19 -0400 Subject: [PATCH] fix(mobile): bound "Last known" counts and survive a path cutover Review found two ways the home host card's stale-count fix misbehaves. 1. A migrateTo cutover (relay->direct probe, forced replacement) rejects in-flight requests with LogicalClientCutoverError and republishes 'connected' from 'connected', so the connect gate never re-arms and the card latched on "Last known: ..." with nothing left to clear it. worktree.ps now re-issues on the authenticated replacement, bounded, like runtime-capability-probe and worktree-create-retry already do. 2. "Last known: N worktrees" had no age bound. The home snapshot is persisted, so a cold start whose first worktree.ps failed rendered counts proven days ago exactly like counts proven seconds ago - the case STA-3123 deliberately rendered as "Worktree list unavailable". Counts now carry countsProvenAt and expire out of the "last known" wording after 10 minutes; counts persisted by an older build count as expired. Also, per review: the card derives its own worktree line from HostWorktreeInfo, so a caller can no longer re-gate the counts away (that was the original defect), and the derivation is covered by a render test - mobile/vitest.config.ts never collected *.test.tsx, so component tests were silently dead. Home stats are keyed by host and summed instead of letting whichever desktop replied last overwrite the shared header row, which the per-reconnect refetch made churn on flaky links. --- mobile/app/index.tsx | 31 ++--- mobile/src/cache/home-snapshot-cache.ts | 19 +-- mobile/src/components/MobileHostCard.test.tsx | 93 +++++++++++++++ mobile/src/components/MobileHostCard.tsx | 11 +- mobile/src/stats/home-stats-total.test.ts | 59 +++++++++ mobile/src/stats/home-stats-total.ts | 45 +++++++ .../worktree/home-host-worktree-fetch.test.ts | 112 ++++++++++++++++-- .../src/worktree/home-host-worktree-fetch.ts | 73 +++++++----- .../src/worktree/home-worktree-info.test.ts | 40 ++++++- mobile/src/worktree/home-worktree-info.ts | 28 ++++- mobile/vitest.config.ts | 4 +- 11 files changed, 426 insertions(+), 89 deletions(-) create mode 100644 mobile/src/components/MobileHostCard.test.tsx create mode 100644 mobile/src/stats/home-stats-total.test.ts create mode 100644 mobile/src/stats/home-stats-total.ts diff --git a/mobile/app/index.tsx b/mobile/app/index.tsx index 29b2e9b298e..9e37e9aa5ce 100644 --- a/mobile/app/index.tsx +++ b/mobile/app/index.tsx @@ -19,11 +19,8 @@ import { loadHosts } from '../src/transport/host-store' import { navigateToMobileHostEdit } from '../src/transport/host-edit-navigation' import { removeHostAndCloseClient } from '../src/transport/host-removal-lifecycle' import { fetchHomeHostWorktreeInfo } from '../src/worktree/home-host-worktree-fetch' -import { - homeHostWorktreeSummary, - type HomeWorktreeSummary, - type HostWorktreeInfo -} from '../src/worktree/home-worktree-info' +import { totalHomeStats, type HomeStatsSummary } from '../src/stats/home-stats-total' +import type { HomeWorktreeSummary, HostWorktreeInfo } from '../src/worktree/home-worktree-info' import type { RpcClient } from '../src/transport/rpc-client' import { createHostConnectRefetchGate } from '../src/transport/host-connect-refetch-gate' import { sendSingleFlightRequest } from '../src/transport/request-single-flight' @@ -68,13 +65,6 @@ function endpointLabel(endpoint: string): string { } } -type StatsSummary = { - totalAgentsSpawned: number - totalPRsCreated: number - totalAgentTimeMs: number - firstEventAt: number | null -} - type HomeTaskSettings = { visibleTaskProviders?: unknown } @@ -123,7 +113,9 @@ function clientKey(client: RpcClient): number { function fetchStats( client: RpcClient, hostId: string, - setStats: (s: StatsSummary) => void, + setStats: ( + updater: (prev: Record) => Record + ) => void, disposed: () => boolean ) { sendSingleFlightRequest(client, hostId, 'stats.summary') @@ -132,7 +124,8 @@ function fetchStats( return } if (response.ok) { - setStats(response.result as StatsSummary) + // Keyed by host: the header totals every desktop instead of showing whoever replied last. + setStats((prev) => ({ ...prev, [hostId]: response.result as HomeStatsSummary })) } }) .catch(() => {}) @@ -223,7 +216,8 @@ export default function HomeScreen() { const [hostStates, setHostStates] = useState>({}) const [hostAttempts, setHostAttempts] = useState>({}) const [hostLastConnected, setHostLastConnected] = useState>({}) - const [stats, setStats] = useState(null) + const [statsByHost, setStatsByHost] = useState>({}) + const stats = useMemo(() => totalHomeStats(statsByHost), [statsByHost]) const [worktreeInfo, setWorktreeInfo] = useState>({}) const [accountsByHost, setAccountsByHost] = useState>({}) const [taskProvidersByHost, setTaskProvidersByHost] = useState>({}) @@ -326,7 +320,7 @@ export default function HomeScreen() { }) for (const entry of allClientsRef.current) { if (entry.client.getState() === 'connected') { - fetchStats(entry.client, entry.hostId, setStats, () => stale) + fetchStats(entry.client, entry.hostId, setStatsByHost, () => stale) void fetchHomeHostWorktreeInfo(entry.client, entry.hostId, setWorktreeInfo, () => stale) fetchAccountsSnapshot(entry.client, entry.hostId, setAccountsByHost, () => stale) fetchTaskProviders(entry.client, entry.hostId, setTaskProvidersByHost, () => stale) @@ -441,7 +435,7 @@ export default function HomeScreen() { // 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, setStats, () => false) + fetchStats(entry.client, entry.hostId, setStatsByHost, () => false) void fetchHomeHostWorktreeInfo(entry.client, entry.hostId, setWorktreeInfo, () => false) fetchTaskProviders(entry.client, entry.hostId, setTaskProvidersByHost, () => false) } @@ -703,7 +697,6 @@ export default function HomeScreen() { const state = hostStates[item.id] ?? 'connecting' const attempts = hostAttempts[item.id] ?? 0 const lastConnectedAt = hostLastConnected[item.id] ?? null - const info = worktreeInfo[item.id] const verdict = classifyConnection({ state, reconnectAttempts: attempts, @@ -716,7 +709,7 @@ export default function HomeScreen() { state={state} verdict={verdict} path={hostPaths[item.id] ?? 'lan'} - worktreeSummary={homeHostWorktreeSummary(info)} + worktreeInfo={worktreeInfo[item.id]} onPress={() => router.push(`/h/${item.id}`)} onLongPress={() => { triggerMediumImpact() diff --git a/mobile/src/cache/home-snapshot-cache.ts b/mobile/src/cache/home-snapshot-cache.ts index a591bb01d70..bbda033735e 100644 --- a/mobile/src/cache/home-snapshot-cache.ts +++ b/mobile/src/cache/home-snapshot-cache.ts @@ -5,25 +5,12 @@ // WebSocket reconnects and the first responses come back. import AsyncStorage from '@react-native-async-storage/async-storage' import type { AccountsSnapshot } from '../components/AccountUsage' +// Why: the canonical shape, so persisted counts keep carrying countsProvenAt — the home card +// needs it to know whether a rehydrated count is minutes or days old. +import type { HostWorktreeInfo } from '../worktree/home-worktree-info' const STORAGE_KEY = 'orca:home-snapshot:v1' -type WorktreeSummary = { - worktreeId: string - repo: string - branch: string - displayName: string - liveTerminalCount: number - status?: 'working' | 'active' | 'permission' | 'done' | 'inactive' -} - -type HostWorktreeInfo = { - hostId: string - totalWorktrees: number - activeCount: number - lastActiveWorktree: WorktreeSummary | null -} - export type HomeSnapshot = { worktreeInfo: Record accountsByHost: Record diff --git a/mobile/src/components/MobileHostCard.test.tsx b/mobile/src/components/MobileHostCard.test.tsx new file mode 100644 index 00000000000..a1889793982 --- /dev/null +++ b/mobile/src/components/MobileHostCard.test.tsx @@ -0,0 +1,93 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ConnectionVerdict } from '../transport/connection-health' +import type { HostProfile } from '../transport/types' +import { + markHomeWorktreeCatalogUnavailable, + type HostWorktreeInfo +} from '../worktree/home-worktree-info' +import { MobileHostCard } from './MobileHostCard' + +vi.mock('react-native', () => ({ + Pressable: 'Pressable', + StyleSheet: { create: (styles: T) => styles }, + Text: 'Text', + View: 'View' +})) +vi.mock('lucide-react-native', () => ({ ChevronRight: 'ChevronRight', Monitor: 'Monitor' })) +vi.mock('./StatusDot', () => ({ StatusDot: 'StatusDot' })) + +const host: HostProfile = { + id: 'host-1', + name: 'Studio', + endpoint: 'ws://studio.local:8765', + deviceToken: 'token', + publicKeyB64: 'key', + lastConnected: 0 +} +const verdict: ConnectionVerdict = { kind: 'normal', label: 'Connected' } +const loaded: HostWorktreeInfo = { + hostId: 'host-1', + totalWorktrees: 12, + activeCount: 2, + lastActiveWorktree: null, + countsProvenAt: Date.now() +} + +describe('MobileHostCard', () => { + let renderer: ReactTestRenderer | null = null + + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + }) + + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + }) + + async function renderCard(worktreeInfo: HostWorktreeInfo | undefined): Promise { + await act(async () => { + renderer = create( + createElement(MobileHostCard, { + host, + state: 'connected', + verdict, + path: 'lan', + worktreeInfo, + onPress: () => {}, + onLongPress: () => {} + }) + ) + }) + return renderer!.root + .findAllByType('Text') + .flatMap((node) => node.children.filter((child) => typeof child === 'string')) + } + + it('renders the counts the host proved', async () => { + expect(await renderCard(loaded)).toContain('12 worktrees · 2 active') + }) + + it('keeps rendering the last proven counts after a failed refresh', async () => { + // The regression this card shipped once: the caller dropped the counts the + // failure path deliberately preserved. + expect(await renderCard(markHomeWorktreeCatalogUnavailable(loaded, 'host-1'))).toContain( + 'Last known: 12 worktrees · 2 active' + ) + }) + + it('never asserts a count for a catalog that failed with nothing proven', async () => { + expect(await renderCard(markHomeWorktreeCatalogUnavailable(undefined, 'host-1'))).toContain( + 'Worktree list unavailable' + ) + }) + + it('shows no worktree line before the first read lands', async () => { + const lines = await renderCard(undefined) + + expect(lines).not.toContain('0 worktrees') + expect(lines).not.toContain('Worktree list unavailable') + }) +}) diff --git a/mobile/src/components/MobileHostCard.tsx b/mobile/src/components/MobileHostCard.tsx index 90460099a0c..79a58bf760e 100644 --- a/mobile/src/components/MobileHostCard.tsx +++ b/mobile/src/components/MobileHostCard.tsx @@ -6,6 +6,7 @@ import { mobileConnectionPathLabel } from '../transport/mobile-connection-path-l import type { MobileConnectionPath } from '../transport/stable-logical-rpc-client' import type { ConnectionState, HostProfile } from '../transport/types' import { colors, radii, spacing } from '../theme/mobile-theme' +import { homeHostWorktreeSummary, type HostWorktreeInfo } from '../worktree/home-worktree-info' import { StatusDot } from './StatusDot' export function MobileHostCard(props: { @@ -13,13 +14,15 @@ export function MobileHostCard(props: { state: ConnectionState verdict: ConnectionVerdict path: MobileConnectionPath - // Prebuilt by homeHostWorktreeSummary so fresh/stale/unavailable wording stays in one place. - worktreeSummary?: string | null + // Why: the card owns the fresh/stale/unavailable wording so no caller can re-gate the counts + // away (STA-3123 shipped that bug once already). + worktreeInfo?: HostWorktreeInfo onPress: () => void onLongPress: () => void }) { const connected = props.state === 'connected' const isError = ['warning', 'unreachable', 'auth-failed'].includes(props.verdict.kind) + const worktreeSummary = homeHostWorktreeSummary(props.worktreeInfo) return ( [styles.card, pressed && styles.cardPressed]} @@ -44,9 +47,9 @@ export function MobileHostCard(props: { {connected ? ` · ${mobileConnectionPathLabel(props.path)}` : ''} - {connected && props.worktreeSummary ? ( + {connected && worktreeSummary ? ( - {props.worktreeSummary} + {worktreeSummary} ) : null} {props.verdict.kind === 'unreachable' && !props.host.relay ? ( diff --git a/mobile/src/stats/home-stats-total.test.ts b/mobile/src/stats/home-stats-total.test.ts new file mode 100644 index 00000000000..79888376914 --- /dev/null +++ b/mobile/src/stats/home-stats-total.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from 'vitest' +import { totalHomeStats, type HomeStatsSummary } from './home-stats-total' + +function stats(overrides: Partial = {}): HomeStatsSummary { + return { + totalAgentsSpawned: 10, + totalPRsCreated: 2, + totalAgentTimeMs: 60_000, + firstEventAt: 1_700_000_000_000, + ...overrides + } +} + +describe('totalHomeStats', () => { + it('has nothing to show before any host answers', () => { + expect(totalHomeStats({})).toBeNull() + }) + + it('passes a single host through unchanged', () => { + expect(totalHomeStats({ 'host-1': stats() })).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 }) + }) + + expect(total).toMatchObject({ + totalAgentsSpawned: 15, + totalPRsCreated: 3, + totalAgentTimeMs: 90_000 + }) + }) + + 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 }) + }) + + 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() + }) + + 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 + }) + + expect(total).toEqual(stats()) + }) +}) diff --git a/mobile/src/stats/home-stats-total.ts b/mobile/src/stats/home-stats-total.ts new file mode 100644 index 00000000000..9a225726619 --- /dev/null +++ b/mobile/src/stats/home-stats-total.ts @@ -0,0 +1,45 @@ +export type HomeStatsSummary = { + totalAgentsSpawned: number + totalPRsCreated: number + totalAgentTimeMs: number + firstEventAt: number | null +} + +/** + * 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. + */ +export function totalHomeStats(byHost: Record): HomeStatsSummary | null { + const hosts = Object.values(byHost) + if (hosts.length === 0) { + return null + } + const total: HomeStatsSummary = { + totalAgentsSpawned: 0, + totalPRsCreated: 0, + totalAgentTimeMs: 0, + firstEventAt: null + } + for (const host of hosts) { + // The rows come straight off the wire unvalidated; a malformed desktop reply must not + // NaN out or crash the header for every other host. + if (!host || typeof host !== 'object') { + continue + } + total.totalAgentsSpawned += finiteOrZero(host.totalAgentsSpawned) + total.totalPRsCreated += finiteOrZero(host.totalPRsCreated) + total.totalAgentTimeMs += finiteOrZero(host.totalAgentTimeMs) + if (typeof host.firstEventAt === 'number' && Number.isFinite(host.firstEventAt)) { + total.firstEventAt = + total.firstEventAt === null + ? host.firstEventAt + : Math.min(total.firstEventAt, host.firstEventAt) + } + } + return total +} + +function finiteOrZero(value: number): number { + return typeof value === 'number' && Number.isFinite(value) ? value : 0 +} diff --git a/mobile/src/worktree/home-host-worktree-fetch.test.ts b/mobile/src/worktree/home-host-worktree-fetch.test.ts index 420c9f9dc15..5799de96366 100644 --- a/mobile/src/worktree/home-host-worktree-fetch.test.ts +++ b/mobile/src/worktree/home-host-worktree-fetch.test.ts @@ -1,19 +1,23 @@ import { describe, expect, it } from 'vitest' import { createHostConnectRefetchGate } from '../transport/host-connect-refetch-gate' import type { RpcClient } from '../transport/rpc-client' -import type { RpcResponse } from '../transport/types' +import { + createStableLogicalRpcClient, + LogicalClientCutoverError +} from '../transport/stable-logical-rpc-client' +import type { ConnectionState, RpcResponse } from '../transport/types' import { fetchHomeHostWorktreeInfo } from './home-host-worktree-fetch' import type { HostWorktreeInfo } from './home-worktree-info' -type FakeClient = { +type FakeSession = { client: RpcClient calls: number settle: (response: RpcResponse | Error) => void } -function fakeClient(): FakeClient { +function fakeSession(): FakeSession { const pending: Array<(response: RpcResponse | Error) => void> = [] - const fake = { + const fake: FakeSession = { calls: 0, settle(response: RpcResponse | Error) { const next = pending.shift() @@ -27,12 +31,23 @@ function fakeClient(): FakeClient { response instanceof Error ? reject(response) : resolve(response) ) }) - } - } as unknown as RpcClient + }, + subscribe: () => () => {}, + updateTerminalSubscriptionViewport: () => {}, + getState: (): ConnectionState => 'connected', + getReconnectAttempt: () => 0, + getLastConnectedAt: () => null, + onStateChange: () => () => {}, + notifyForeground: () => {}, + close: () => {} + } } return fake } +// Drains the promise chain so a retry queued in a .catch has reached the transport. +const flush = () => new Promise((resolve) => setTimeout(resolve, 0)) + function catalogResponse(count: number, active: number): RpcResponse { const worktrees = Array.from({ length: count }, (_, index) => ({ worktreeId: `wt-${index}`, @@ -62,7 +77,7 @@ const notDisposed = () => false describe('fetchHomeHostWorktreeInfo', () => { it('keeps the last proven counts when the in-flight read rejects', async () => { const store = infoStore() - const host = fakeClient() + const host = fakeSession() const loaded = fetchHomeHostWorktreeInfo(host.client, 'host-1', store.setInfo, notDisposed) host.settle(catalogResponse(12, 2)) @@ -78,13 +93,29 @@ describe('fetchHomeHostWorktreeInfo', () => { activeCount: 2, lastActiveWorktree: expect.objectContaining({ worktreeId: 'wt-0' }), catalogUnavailable: true, - staleCounts: true + staleCounts: true, + // Age-stamped so the card can stop calling day-old counts "last known". + countsProvenAt: expect.any(Number) }) }) + it('stamps when the host proved the counts', async () => { + const store = infoStore() + const host = fakeSession() + const before = Date.now() + + const loaded = fetchHomeHostWorktreeInfo(host.client, 'host-1', store.setInfo, notDisposed) + host.settle(catalogResponse(3, 1)) + await loaded + + const provenAt = store.current['host-1'].countsProvenAt + expect(provenAt).toBeGreaterThanOrEqual(before) + expect(provenAt).toBeLessThanOrEqual(Date.now()) + }) + it('marks a host whose catalog never loaded as unavailable, not empty', async () => { const store = infoStore() - const host = fakeClient() + const host = fakeSession() const failed = fetchHomeHostWorktreeInfo(host.client, 'host-1', store.setInfo, notDisposed) host.settle({ ok: false, error: { code: 'internal' } } as RpcResponse) @@ -99,7 +130,7 @@ describe('fetchHomeHostWorktreeInfo', () => { it('re-reads the catalog on reconnect and clears the stale flag', async () => { const store = infoStore() - const host = fakeClient() + const host = fakeSession() const gate = createHostConnectRefetchGate() const connect = async (response: RpcResponse | Error) => { @@ -133,9 +164,68 @@ describe('fetchHomeHostWorktreeInfo', () => { expect(host.calls).toBe(3) }) + it('re-reads through a relay→direct cutover, which never leaves connected', async () => { + const store = infoStore() + const relay = fakeSession() + const direct = fakeSession() + const logical = createStableLogicalRpcClient(relay.client, 'relay') + const gate = createHostConnectRefetchGate() + const gateFires: ConnectionState[] = [] + logical.onStateChange((state) => { + if (gate.observe(state)) { + gateFires.push(state) + } + }) + gate.observe(logical.getState()) + + const loaded = fetchHomeHostWorktreeInfo(logical, 'host-1', store.setInfo, notDisposed) + relay.settle(catalogResponse(12, 2)) + await loaded + + // The supervisor's direct probe migrates while a refresh is on the wire. + const interrupted = fetchHomeHostWorktreeInfo(logical, 'host-1', store.setInfo, notDisposed) + await logical.migrateTo(direct.client, 'lan') + await flush() + + expect(direct.calls).toBe(1) + direct.settle(catalogResponse(13, 3)) + await interrupted + + // The gate can't cover this: migrateTo republishes 'connected' from 'connected'. + expect(gateFires).toEqual([]) + expect(store.current['host-1']).toMatchObject({ totalWorktrees: 13, activeCount: 3 }) + expect(store.current['host-1'].staleCounts).toBeUndefined() + expect(store.current['host-1'].catalogUnavailable).toBeUndefined() + }) + + it('gives up on a host that keeps cutting over so the card still reports the failure', async () => { + const store = infoStore() + const host = fakeSession() + + const loaded = fetchHomeHostWorktreeInfo(host.client, 'host-1', store.setInfo, notDisposed) + host.settle(catalogResponse(12, 2)) + await loaded + + const cutoverStorm = fetchHomeHostWorktreeInfo( + host.client, + 'host-1', + store.setInfo, + notDisposed + ) + for (let attempt = 0; attempt < 6; attempt += 1) { + host.settle(new LogicalClientCutoverError()) + await flush() + } + await cutoverStorm + + // 1 original + CUTOVER_RETRY_LIMIT retries, then the failure is surfaced. + expect(host.calls).toBe(4) + expect(store.current['host-1'].staleCounts).toBe(true) + }) + it('ignores a response that lands after the screen is disposed', async () => { const store = infoStore() - const host = fakeClient() + const host = fakeSession() const done = fetchHomeHostWorktreeInfo(host.client, 'host-1', store.setInfo, () => true) host.settle(new Error('socket closed mid-request')) diff --git a/mobile/src/worktree/home-host-worktree-fetch.ts b/mobile/src/worktree/home-host-worktree-fetch.ts index bbd4865d963..7c6dafe6b14 100644 --- a/mobile/src/worktree/home-host-worktree-fetch.ts +++ b/mobile/src/worktree/home-host-worktree-fetch.ts @@ -1,6 +1,7 @@ import { setCachedWorktrees } from '../cache/worktree-cache' import { sendSingleFlightRequest } from '../transport/request-single-flight' import type { RpcClient } from '../transport/rpc-client' +import { isLogicalClientCutoverError } from '../transport/stable-logical-rpc-client' import { markHomeWorktreeCatalogUnavailable, type HomeWorktreeSummary, @@ -10,6 +11,10 @@ import { pickResumeWorktree } from './resume-worktree' import { WORKTREE_PS_FULL_LIMIT } from './worktree-catalog-snapshot-client' const ACTIVE_STATUSES = new Set(['working', 'active', 'permission']) +// Why: a relay↔direct cutover rejects in-flight reads without ever leaving 'connected', so the +// connect gate never re-arms. Re-issue on the replacement session; cap it so a migration loop +// can't spin. See runtime-capability-probe.ts for the same hazard on status.get. +const CUTOVER_RETRY_LIMIT = 2 export type HostWorktreeInfoSetter = ( updater: (prev: Record) => Record @@ -30,35 +35,45 @@ export function fetchHomeHostWorktreeInfo( }) } - return sendSingleFlightRequest(client, hostId, 'worktree.ps', { limit: WORKTREE_PS_FULL_LIMIT }) - .then((response) => { - if (disposed()) { - return - } - if (!response.ok) { - markUnavailable() - return - } - const result = response.result as { worktrees?: HomeWorktreeSummary[] } - const worktrees = result.worktrees ?? [] - setCachedWorktrees(hostId, worktrees) - const active = worktrees.filter((w) => w.status && ACTIVE_STATUSES.has(w.status)) - // Mirror the desktop's focused workspace (see pickResumeWorktree). - const lastActive = pickResumeWorktree(worktrees) - setInfo((prev) => ({ - ...prev, - [hostId]: { - hostId, - totalWorktrees: worktrees.length, - activeCount: active.length, - lastActiveWorktree: lastActive + const attempt = (cutoverRetriesLeft: number): Promise => + sendSingleFlightRequest(client, hostId, 'worktree.ps', { limit: WORKTREE_PS_FULL_LIMIT }) + .then((response) => { + if (disposed()) { + return } - })) - }) - .catch(() => { - // A rejected in-flight read (socket died mid-request) is a failed refresh, not an empty host. - if (!disposed()) { + if (!response.ok) { + markUnavailable() + return + } + const result = response.result as { worktrees?: HomeWorktreeSummary[] } + const worktrees = result.worktrees ?? [] + setCachedWorktrees(hostId, worktrees) + const active = worktrees.filter((w) => w.status && ACTIVE_STATUSES.has(w.status)) + // Mirror the desktop's focused workspace (see pickResumeWorktree). + const lastActive = pickResumeWorktree(worktrees) + setInfo((prev) => ({ + ...prev, + [hostId]: { + hostId, + totalWorktrees: worktrees.length, + activeCount: active.length, + lastActiveWorktree: lastActive, + countsProvenAt: Date.now() + } + })) + }) + .catch((error: unknown) => { + if (disposed()) { + return + } + // A cutover raises only after migrateTo installed an authenticated replacement, so the + // read was interrupted, not answered — ask again instead of latching "Last known". + if (cutoverRetriesLeft > 0 && isLogicalClientCutoverError(error)) { + return attempt(cutoverRetriesLeft - 1) + } + // Any other rejection (socket died mid-request) is a failed refresh, not an empty host. markUnavailable() - } - }) + }) + + return attempt(CUTOVER_RETRY_LIMIT) } diff --git a/mobile/src/worktree/home-worktree-info.test.ts b/mobile/src/worktree/home-worktree-info.test.ts index fdb6242be74..b9acc722c68 100644 --- a/mobile/src/worktree/home-worktree-info.test.ts +++ b/mobile/src/worktree/home-worktree-info.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import { + HOME_WORKTREE_COUNTS_STALE_MAX_AGE_MS, homeHostWorktreeSummary, markHomeWorktreeCatalogUnavailable, type HostWorktreeInfo @@ -51,24 +52,51 @@ describe('markHomeWorktreeCatalogUnavailable', () => { }) describe('homeHostWorktreeSummary', () => { + const provenAt = 1_700_000_000_000 const loaded: HostWorktreeInfo = { hostId: 'host-1', totalWorktrees: 12, activeCount: 2, - lastActiveWorktree: null + lastActiveWorktree: null, + countsProvenAt: provenAt } it('summarizes a freshly loaded catalog', () => { - expect(homeHostWorktreeSummary(loaded)).toBe('12 worktrees · 2 active') - expect(homeHostWorktreeSummary({ ...loaded, totalWorktrees: 1, activeCount: 0 })).toBe( - '1 worktree' - ) + expect(homeHostWorktreeSummary(loaded, provenAt)).toBe('12 worktrees · 2 active') + expect( + homeHostWorktreeSummary({ ...loaded, totalWorktrees: 1, activeCount: 0 }, provenAt) + ).toBe('1 worktree') }) it('keeps showing the last proven counts after a failed refresh', () => { const afterFailure = markHomeWorktreeCatalogUnavailable(loaded, 'host-1') - expect(homeHostWorktreeSummary(afterFailure)).toBe('Last known: 12 worktrees · 2 active') + expect(homeHostWorktreeSummary(afterFailure, provenAt + 30_000)).toBe( + 'Last known: 12 worktrees · 2 active' + ) + }) + + it('stops calling aged-out counts "last known"', () => { + const afterFailure = markHomeWorktreeCatalogUnavailable(loaded, 'host-1') + + expect( + homeHostWorktreeSummary(afterFailure, provenAt + HOME_WORKTREE_COUNTS_STALE_MAX_AGE_MS) + ).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' + ) + }) + + it('treats counts persisted before age stamping as unproven', () => { + const preUpgrade: HostWorktreeInfo = { ...loaded, countsProvenAt: undefined } + + expect( + homeHostWorktreeSummary(markHomeWorktreeCatalogUnavailable(preUpgrade, 'host-1'), provenAt) + ).toBe('Worktree list unavailable') }) 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 fc222b32d87..00e05640824 100644 --- a/mobile/src/worktree/home-worktree-info.ts +++ b/mobile/src/worktree/home-worktree-info.ts @@ -17,8 +17,15 @@ export type HostWorktreeInfo = { catalogUnavailable?: boolean // The counts are the last proven ones, kept across a failed refresh. staleCounts?: boolean + // When the host last answered with these counts (ms epoch); absent in snapshots an older + // build persisted, which is why an unstamped count counts as expired rather than fresh. + 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 + export function markHomeWorktreeCatalogUnavailable( current: HostWorktreeInfo | undefined, hostId: string @@ -41,12 +48,16 @@ export function markHomeWorktreeCatalogUnavailable( } /** The host card's worktree line, or null when nothing is known yet. */ -export function homeHostWorktreeSummary(info: HostWorktreeInfo | undefined): string | null { +export function homeHostWorktreeSummary( + info: HostWorktreeInfo | undefined, + now: number = Date.now() +): string | null { if (!info) { return null } - // Why (STA-3123): a catalog that never loaded must not assert a count of zero. - if (info.catalogUnavailable && !info.staleCounts) { + // 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)) { return 'Worktree list unavailable' } const counts = `${info.totalWorktrees} worktree${info.totalWorktrees === 1 ? '' : 's'}${ @@ -54,3 +65,14 @@ export function homeHostWorktreeSummary(info: HostWorktreeInfo | undefined): str }` return info.staleCounts ? `Last known: ${counts}` : counts } + +function hasFreshEnoughCounts(info: HostWorktreeInfo, now: number): boolean { + if (!info.staleCounts) { + return false + } + // 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 + ) +} diff --git a/mobile/vitest.config.ts b/mobile/vitest.config.ts index 9e7e3ecc59d..e39f90eedf1 100644 --- a/mobile/vitest.config.ts +++ b/mobile/vitest.config.ts @@ -9,6 +9,8 @@ export default defineConfig({ oxc: vitestOxcConfig, test: { environment: 'node', - include: ['src/**/*.test.ts'] + // .tsx too: component tests exist (react-test-renderer + mocked react-native) and were + // silently never collected, so render-level regressions shipped untested. + include: ['src/**/*.test.ts', 'src/**/*.test.tsx'] } })