From d936d8da82c7e4deec567fdb52775b2d1b236d61 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Mon, 7 Sep 2026 16:44:26 -0400 Subject: [PATCH] revert(mobile): pull the relay connect-speed mobile pass pending a smaller, verified re-land (#19348) * Revert "feat(mobile): time relay dial stages so diagnostics say where a slow connect went (#19245)" This reverts commit 83b1558ecce27decfe46fe01de8b95a4b4d1337e. * Revert "perf(mobile): race the direct and relay dials from t=0 on every reconnect (#19308)" This reverts commit ceafdcad2f808839c268fa326d5faf89ad580cf0. * Revert "feat(mobile): draw the last known tab strip while a session reconnects (mobile pass) (#19281)" This reverts commit 643571def6804e2b70a0a66fb3e3e82af7509b34. * Revert "perf(mobile): open a session with parallel startup RPCs and a pre-warmed terminal engine (#19260)" This reverts commit c37413271e9b5f90100524052d5ca2cd997ab32f. * Revert "perf(mobile): cut the relay reconnect critical path and admit dead sockets faster (mobile pass) (#19280)" This reverts commit e628090ad4e1a09dd76604642914df73165838d0. * chore: keep the react-doctor suppression for the startup timers The pattern it covers (a variable number of timers cleared through one cleanup) predates #19260 and is unchanged by the revert; dropping the entry only re-exposed a pre-existing finding to the changed-code gate. --- .../src/cache/session-tab-strip-cache.test.ts | 406 ------------------ mobile/src/cache/session-tab-strip-cache.ts | 260 ----------- .../src/components/HostProtocolGate.test.ts | 176 +------- mobile/src/components/HostProtocolGate.tsx | 42 +- .../codex-reset-credit-capability.test.ts | 2 +- .../codex-reset-credit-capability.ts | 2 +- .../connection-diagnostics-report.test.ts | 94 +--- .../connection-diagnostics-report.ts | 2 - .../connection-log-timing-summary.ts | 66 --- .../session/MobileSessionActiveContent.tsx | 78 +--- mobile/src/session/MobileSessionHeader.tsx | 59 ++- .../src/session/TerminalEnginePrewarm.test.ts | 238 ---------- mobile/src/session/TerminalEnginePrewarm.tsx | 111 ----- .../session/mobile-session-frame-styles.ts | 27 +- ...obile-session-reconnect-view-state.test.ts | 155 ------- .../mobile-session-reconnect-view-state.ts | 61 --- .../mobile-session-route-parity.test.ts | 60 ++- ...ession-route-source-family.test-support.ts | 1 - ...mobile-session-startup-parallelism.test.ts | 279 ------------ .../mobile-session-startup-source.test.ts | 112 +---- .../mobile-session-tab-strip-entries.ts | 116 ----- .../terminal-prewarm-frame-geometry.test.ts | 181 -------- .../terminal-prewarm-refit-debt.test.ts | 147 ------- .../session/use-mobile-session-controller.ts | 4 +- .../session/use-mobile-session-foundation.ts | 11 - .../use-mobile-session-presentation.ts | 29 +- .../src/session/use-mobile-session-startup.ts | 118 ++--- .../use-mobile-session-tab-reconciliation.ts | 40 +- .../use-mobile-session-tab-strip-cache.ts | 66 --- ...ession-terminal-subscription-foundation.ts | 26 +- .../transport/connection-log-buffer.test.ts | 23 - .../connection-state-dwell-log.test.ts | 77 ---- mobile/src/transport/direct-connection-log.ts | 28 +- mobile/src/transport/direct-rpc-client.ts | 9 +- .../transport/host-removal-lifecycle.test.ts | 44 -- .../src/transport/host-removal-lifecycle.ts | 9 - mobile/src/transport/host-status-gates.ts | 77 ++-- .../mobile-direct-return-probe.test.ts | 96 ----- .../transport/mobile-direct-return-probe.ts | 95 +--- .../transport/mobile-endpoint-lifecycle.ts | 3 +- .../mobile-endpoint-reconnect-race.test.ts | 362 ---------------- .../mobile-endpoint-supervisor-contract.ts | 4 +- ...e-endpoint-supervisor-direct-probe.test.ts | 114 +---- .../mobile-endpoint-supervisor-test-fakes.ts | 10 - .../mobile-endpoint-supervisor.test.ts | 106 ++++- .../transport/mobile-endpoint-supervisor.ts | 192 ++++----- .../mobile-relay-background-grace.ts | 13 +- .../mobile-relay-credential-refresh.ts | 71 --- .../mobile-relay-credential-rotation.ts | 4 - .../mobile-relay-direct-grace-timer.ts | 48 +++ .../transport/mobile-relay-e2ee-link.test.ts | 32 -- .../mobile-relay-lost-race-damper.ts | 99 ----- .../mobile-relay-rpc-session-liveness.test.ts | 119 +---- .../mobile-relay-rpc-session.test.ts | 236 ++-------- .../src/transport/mobile-relay-rpc-session.ts | 117 +++-- .../mobile-relay-runtime-failover.test.ts | 4 - .../mobile-relay-session-establisher.ts | 38 +- mobile/src/transport/monotonic-clock.ts | 15 - .../persisted-connection-log-store.test.ts | 83 ---- .../persisted-connection-log-store.ts | 29 +- mobile/src/transport/relay-dial-stage-log.ts | 55 --- .../relay-dial-stage-timings.test.ts | 219 ---------- mobile/src/transport/relay-dial-stage.ts | 49 +-- .../transport/relay-recovery-intent-queue.ts | 45 -- .../relay-session-liveness-profile.ts | 54 --- .../transport/rpc-client-connection-state.ts | 19 +- .../rpc-client-log-redaction.test.ts | 4 +- .../rpc-session-liveness-watchdog.test.ts | 93 ---- .../rpc-session-liveness-watchdog.ts | 92 +--- ...st.ts => runtime-capability-probe.test.ts} | 75 +--- ...s-probe.ts => runtime-capability-probe.ts} | 49 +-- mobile/src/transport/types.ts | 24 +- .../unpaired-host-credential-deletion.test.ts | 104 ----- .../unpaired-host-credential-deletion.ts | 13 - .../src/worktree/home-host-worktree-fetch.ts | 2 +- 75 files changed, 657 insertions(+), 5366 deletions(-) delete mode 100644 mobile/src/cache/session-tab-strip-cache.test.ts delete mode 100644 mobile/src/cache/session-tab-strip-cache.ts delete mode 100644 mobile/src/diagnostics/connection-log-timing-summary.ts delete mode 100644 mobile/src/session/TerminalEnginePrewarm.test.ts delete mode 100644 mobile/src/session/TerminalEnginePrewarm.tsx delete mode 100644 mobile/src/session/mobile-session-reconnect-view-state.test.ts delete mode 100644 mobile/src/session/mobile-session-reconnect-view-state.ts delete mode 100644 mobile/src/session/mobile-session-startup-parallelism.test.ts delete mode 100644 mobile/src/session/mobile-session-tab-strip-entries.ts delete mode 100644 mobile/src/session/terminal-prewarm-frame-geometry.test.ts delete mode 100644 mobile/src/session/terminal-prewarm-refit-debt.test.ts delete mode 100644 mobile/src/session/use-mobile-session-tab-strip-cache.ts delete mode 100644 mobile/src/transport/connection-state-dwell-log.test.ts delete mode 100644 mobile/src/transport/mobile-direct-return-probe.test.ts delete mode 100644 mobile/src/transport/mobile-endpoint-reconnect-race.test.ts delete mode 100644 mobile/src/transport/mobile-relay-credential-refresh.ts create mode 100644 mobile/src/transport/mobile-relay-direct-grace-timer.ts delete mode 100644 mobile/src/transport/mobile-relay-lost-race-damper.ts delete mode 100644 mobile/src/transport/monotonic-clock.ts delete mode 100644 mobile/src/transport/relay-dial-stage-log.ts delete mode 100644 mobile/src/transport/relay-dial-stage-timings.test.ts delete mode 100644 mobile/src/transport/relay-recovery-intent-queue.ts delete mode 100644 mobile/src/transport/relay-session-liveness-profile.ts rename mobile/src/transport/{runtime-status-probe.test.ts => runtime-capability-probe.test.ts} (67%) rename mobile/src/transport/{runtime-status-probe.ts => runtime-capability-probe.ts} (51%) delete mode 100644 mobile/src/transport/unpaired-host-credential-deletion.test.ts diff --git a/mobile/src/cache/session-tab-strip-cache.test.ts b/mobile/src/cache/session-tab-strip-cache.test.ts deleted file mode 100644 index f432801a647..00000000000 --- a/mobile/src/cache/session-tab-strip-cache.test.ts +++ /dev/null @@ -1,406 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' - -const asyncStorage = vi.hoisted(() => ({ - getItem: vi.fn(), - setItem: vi.fn(), - removeItem: vi.fn() -})) - -vi.mock('@react-native-async-storage/async-storage', () => ({ default: asyncStorage })) - -import { - deleteCachedSessionTabStripForHost, - getSessionTabStripCacheKey, - loadCachedSessionTabStrip, - readCachedSessionTabStrip, - resetSessionTabStripCacheForTests, - saveCachedSessionTabStrip -} from './session-tab-strip-cache' -import type { MobileSessionTabStripPreview } from '../session/mobile-session-tab-strip-entries' - -const STORAGE_KEY = 'orca:session-tab-strip:v1' - -function preview(...ids: string[]): MobileSessionTabStripPreview { - return { - tabs: ids.map((id) => ({ id, type: 'terminal' as const, title: id, agentId: null })), - activeTabId: ids[0] ?? null - } -} - -function lastWrittenFile(): { workspaces: { key: string }[] } { - const call = asyncStorage.setItem.mock.calls.at(-1) - return JSON.parse(String(call?.[1])) -} - -beforeEach(() => { - vi.useFakeTimers() - asyncStorage.getItem.mockReset().mockResolvedValue(null) - asyncStorage.setItem.mockReset().mockResolvedValue(undefined) - resetSessionTabStripCacheForTests() -}) - -afterEach(() => { - vi.useRealTimers() -}) - -describe('getSessionTabStripCacheKey', () => { - it('digests the workspace id so no filesystem path reaches the key', () => { - const path = '/Users/someone/private-client/worktrees/acquisition' - const key = getSessionTabStripCacheKey('host-1', `repo::${path}`) - - expect(key).not.toContain(path) - expect(key).not.toContain('someone') - expect(key).toMatch(/^\["host-1","[0-9a-f]{32}"\]$/) - }) - - it('joins the two ids unambiguously, whatever a worktree path contains', () => { - expect(getSessionTabStripCacheKey('host', 'a\nb')).not.toBe( - getSessionTabStripCacheKey('host\na', 'b') - ) - expect(getSessionTabStripCacheKey('host-1', 'wt-1')).not.toBe( - getSessionTabStripCacheKey('host-1', 'wt-2') - ) - }) - - it('needs both a host and a workspace', () => { - expect(getSessionTabStripCacheKey(undefined, 'wt-1')).toBeNull() - expect(getSessionTabStripCacheKey('host-1', undefined)).toBeNull() - }) -}) - -describe('session tab strip cache', () => { - it('serves a save back synchronously and persists it once the write settles', async () => { - const key = getSessionTabStripCacheKey('host-1', 'wt-1') - saveCachedSessionTabStrip(key, preview('tab-1', 'tab-2')) - - expect(readCachedSessionTabStrip(key)?.tabs.map((tab) => tab.id)).toEqual(['tab-1', 'tab-2']) - expect(asyncStorage.setItem).not.toHaveBeenCalled() - - await vi.advanceTimersByTimeAsync(300) - - expect(asyncStorage.setItem.mock.calls[0]?.[0]).toBe(STORAGE_KEY) - expect(lastWrittenFile().workspaces.map((w) => w.key)).toEqual([key]) - }) - - it('reads nothing synchronously before the stored file is loaded', async () => { - const key = getSessionTabStripCacheKey('host-1', 'wt-1') - asyncStorage.getItem.mockResolvedValue( - JSON.stringify({ workspaces: [{ key, preview: preview('tab-1') }] }) - ) - - expect(readCachedSessionTabStrip(key)).toBeNull() - expect((await loadCachedSessionTabStrip(key))?.tabs.map((tab) => tab.id)).toEqual(['tab-1']) - expect(readCachedSessionTabStrip(key)?.tabs).toHaveLength(1) - }) - - it('returns null for a workspace with no stored strip', async () => { - expect(await loadCachedSessionTabStrip(getSessionTabStripCacheKey('host-1', 'wt-9'))).toBeNull() - expect(await loadCachedSessionTabStrip(null)).toBeNull() - }) - - it('survives unreadable storage', async () => { - asyncStorage.getItem.mockResolvedValue('{not json') - - expect(await loadCachedSessionTabStrip(getSessionTabStripCacheKey('host-1', 'wt-1'))).toBeNull() - }) - - it('evicts the least recently written workspace past the cap', async () => { - for (let i = 0; i < 14; i++) { - saveCachedSessionTabStrip(getSessionTabStripCacheKey('host-1', `wt-${i}`), preview('tab-1')) - } - await vi.advanceTimersByTimeAsync(300) - - const keys = lastWrittenFile().workspaces.map((w) => w.key) - expect(keys).toHaveLength(12) - expect(keys).not.toContain(getSessionTabStripCacheKey('host-1', 'wt-0')) - expect(keys.at(-1)).toBe(getSessionTabStripCacheKey('host-1', 'wt-13')) - }) - - it('re-writing a workspace makes it the newest, not the oldest', async () => { - for (let i = 0; i < 12; i++) { - saveCachedSessionTabStrip(getSessionTabStripCacheKey('host-1', `wt-${i}`), preview('tab-1')) - } - saveCachedSessionTabStrip(getSessionTabStripCacheKey('host-1', 'wt-0'), preview('tab-2')) - saveCachedSessionTabStrip(getSessionTabStripCacheKey('host-1', 'wt-99'), preview('tab-1')) - await vi.advanceTimersByTimeAsync(300) - - const keys = lastWrittenFile().workspaces.map((w) => w.key) - expect(keys).toContain(getSessionTabStripCacheKey('host-1', 'wt-0')) - expect(keys).not.toContain(getSessionTabStripCacheKey('host-1', 'wt-1')) - }) - - it('records a workspace the host has emptied, so a stale strip cannot outlive it', async () => { - const key = getSessionTabStripCacheKey('host-1', 'wt-1') - saveCachedSessionTabStrip(key, preview('tab-1')) - saveCachedSessionTabStrip(key, { tabs: [], activeTabId: null }) - - expect(readCachedSessionTabStrip(key)).toEqual({ tabs: [], activeTabId: null }) - }) - - it('caps tabs per workspace and title length, and drops an unmatched active id', async () => { - const key = getSessionTabStripCacheKey('host-1', 'wt-1') - saveCachedSessionTabStrip(key, { - // A file tab, because the titles that survive redaction at all are the ones the cap has - // to bound. - tabs: Array.from({ length: 30 }, (_, i) => ({ - id: `tab-${i}`, - type: 'file' as const, - title: 'x'.repeat(200), - agentId: null - })), - activeTabId: 'tab-29' - }) - - const stored = readCachedSessionTabStrip(key) - expect(stored?.tabs).toHaveLength(24) - expect(stored?.tabs[0]?.title).toHaveLength(64) - expect(stored?.activeTabId).toBeNull() - }) - - it('drops fields a future tab type might smuggle into storage', async () => { - const key = getSessionTabStripCacheKey('host-1', 'wt-1') - saveCachedSessionTabStrip(key, { - tabs: [ - { - id: 'tab-1', - type: 'file', - title: 'notes.md', - agentId: null, - filePath: '/Users/someone/secret/notes.md' - } as never - ], - activeTabId: 'tab-1' - }) - await vi.advanceTimersByTimeAsync(300) - - expect(String(asyncStorage.setItem.mock.calls.at(-1)?.[1])).not.toContain('/Users/someone') - }) - - it('drops a stored entry naming a tab type this build cannot draw', async () => { - const key = getSessionTabStripCacheKey('host-1', 'wt-1') - saveCachedSessionTabStrip(key, { - tabs: [ - { id: 'tab-1', type: 'from-a-newer-build', title: 'raw title', agentId: null } as never, - { id: 'tab-2', type: 'file', title: 'notes.md', agentId: null } - ], - activeTabId: 'tab-2' - }) - - expect(readCachedSessionTabStrip(key)?.tabs.map((tab) => tab.id)).toEqual(['tab-2']) - }) - - it('never writes a shell-controlled terminal title, however it arrives', async () => { - const secret = 'psql postgres://admin:hunter2@db.internal/prod' - const key = getSessionTabStripCacheKey('host-1', 'wt-1') - saveCachedSessionTabStrip(key, { - tabs: [ - { id: 'tab-1', type: 'terminal', title: secret, agentId: null }, - { id: 'tab-2', type: 'terminal', title: secret, agentId: 'claude' }, - { id: 'tab-3', type: 'terminal', title: secret, agentId: 'not-a-known-agent' }, - { id: 'tab-4', type: 'browser', title: 'Acme Corp — Q3 layoffs memo', agentId: null } - ], - activeTabId: 'tab-1' - }) - await vi.advanceTimersByTimeAsync(300) - - expect(readCachedSessionTabStrip(key)?.tabs.map((tab) => tab.title)).toEqual([ - 'Terminal', - 'Claude', - 'Terminal', - 'Browser' - ]) - const written = String(asyncStorage.setItem.mock.calls.at(-1)?.[1]) - expect(written).not.toContain('hunter2') - expect(written).not.toContain('postgres://') - expect(written).not.toContain('layoffs') - }) - - it('scrubs a stored title written by an older build on the way back out', async () => { - const key = getSessionTabStripCacheKey('host-1', 'wt-1') - asyncStorage.getItem.mockResolvedValue( - JSON.stringify({ - workspaces: [ - { - key, - preview: { - tabs: [{ id: 'tab-1', type: 'terminal', title: 'curl -H token', agentId: null }], - activeTabId: 'tab-1' - } - } - ] - }) - ) - - expect((await loadCachedSessionTabStrip(key))?.tabs[0]?.title).toBe('Terminal') - }) - - it('forgets an unpaired host and cannot resurrect it from a later save', async () => { - const hostA = getSessionTabStripCacheKey('host-a', 'wt-1') - const hostB = getSessionTabStripCacheKey('host-b', 'wt-1') - saveCachedSessionTabStrip(hostA, preview('tab-a')) - saveCachedSessionTabStrip(hostB, preview('tab-b')) - await vi.advanceTimersByTimeAsync(300) - - await deleteCachedSessionTabStripForHost('host-a') - - expect(readCachedSessionTabStrip(hostA)).toBeNull() - expect(readCachedSessionTabStrip(hostB)?.tabs).toHaveLength(1) - expect(lastWrittenFile().workspaces.map((w) => w.key)).toEqual([hostB]) - - saveCachedSessionTabStrip(hostB, preview('tab-b2')) - await vi.advanceTimersByTimeAsync(300) - - expect(lastWrittenFile().workspaces.map((w) => w.key)).toEqual([hostB]) - }) - - it('forgets a host whose rows are only on disk, never read this session', async () => { - const hostA = getSessionTabStripCacheKey('host-a', 'wt-1') - const hostB = getSessionTabStripCacheKey('host-b', 'wt-1') - asyncStorage.getItem.mockResolvedValue( - JSON.stringify({ - workspaces: [ - { key: hostA, preview: preview('tab-a') }, - { key: hostB, preview: preview('tab-b') } - ] - }) - ) - - await deleteCachedSessionTabStripForHost('host-a') - - expect(lastWrittenFile().workspaces.map((w) => w.key)).toEqual([hostB]) - }) - - it('drops a pending debounced write so it cannot restore the forgotten host', async () => { - const hostA = getSessionTabStripCacheKey('host-a', 'wt-1') - saveCachedSessionTabStrip(hostA, preview('tab-a')) - - await deleteCachedSessionTabStripForHost('host-a') - await vi.advanceTimersByTimeAsync(300) - - expect(lastWrittenFile().workspaces).toEqual([]) - }) - it('rejects a deletion whose write never landed, rather than reporting it as done', async () => { - // A resolved delete over a failed write leaves the forgotten host's tab titles in - // plaintext on disk while every caller believes they are gone. - const hostA = getSessionTabStripCacheKey('host-a', 'wt-1') - saveCachedSessionTabStrip(hostA, preview('tab-a')) - await vi.advanceTimersByTimeAsync(300) - asyncStorage.setItem.mockRejectedValue(new Error('storage full')) - - await expect(deleteCachedSessionTabStripForHost('host-a')).rejects.toThrow('storage full') - }) - - it('keeps a debounced save best effort, so one failed write cannot reject unowned', async () => { - asyncStorage.setItem.mockRejectedValue(new Error('storage full')) - saveCachedSessionTabStrip(getSessionTabStripCacheKey('host-a', 'wt-1'), preview('tab-a')) - - // No throw and no unhandled rejection: the write is fire-and-forget by design. - await vi.advanceTimersByTimeAsync(300) - expect(asyncStorage.setItem).toHaveBeenCalledOnce() - }) - - it('refuses a save for the host it is in the middle of forgetting', async () => { - const hostA = getSessionTabStripCacheKey('host-a', 'wt-1') - saveCachedSessionTabStrip(hostA, preview('tab-a')) - await vi.advanceTimersByTimeAsync(300) - - let releaseWrite!: () => void - asyncStorage.setItem.mockImplementationOnce( - async () => - new Promise((resolve) => { - releaseWrite = () => resolve() - }) - ) - const deletion = deleteCachedSessionTabStripForHost('host-a') - // The purge has run and its write is on the wire; a snapshot queued for the - // workspace the user just unpaired now lands in that window. - await vi.advanceTimersByTimeAsync(0) - saveCachedSessionTabStrip(hostA, preview('tab-a2')) - releaseWrite() - await deletion - await vi.advanceTimersByTimeAsync(300) - - expect(readCachedSessionTabStrip(hostA)).toBeNull() - expect(lastWrittenFile().workspaces).toEqual([]) - }) - - it('cannot be talked back into a host whose deletion write failed', async () => { - const hostA = getSessionTabStripCacheKey('host-a', 'wt-1') - saveCachedSessionTabStrip(hostA, preview('tab-a')) - await vi.advanceTimersByTimeAsync(300) - asyncStorage.setItem.mockRejectedValueOnce(new Error('storage full')) - - await expect(deleteCachedSessionTabStripForHost('host-a')).rejects.toThrow('storage full') - const writesSoFar = asyncStorage.setItem.mock.calls.length - - saveCachedSessionTabStrip(hostA, preview('tab-a3')) - await vi.advanceTimersByTimeAsync(300) - - expect(readCachedSessionTabStrip(hostA)).toBeNull() - expect(asyncStorage.setItem).toHaveBeenCalledTimes(writesSoFar) - }) - it('lets a debounced write that already snapshotted the removed host land first', async () => { - // The tombstone stops new saves, but a debounced write that fired a moment earlier - // built its blob from the map as it was and is still on the wire. Writing over it - // concurrently leaves which blob lands last up to storage. - const hostA = getSessionTabStripCacheKey('host-a', 'wt-1') - const hostB = getSessionTabStripCacheKey('host-b', 'wt-1') - saveCachedSessionTabStrip(hostA, preview('tab-a')) - saveCachedSessionTabStrip(hostB, preview('tab-b')) - - let releaseDebounced!: () => void - asyncStorage.setItem.mockImplementationOnce( - async () => - new Promise((resolve) => { - releaseDebounced = () => resolve() - }) - ) - await vi.advanceTimersByTimeAsync(300) - - const deletion = deleteCachedSessionTabStripForHost('host-a') - await vi.advanceTimersByTimeAsync(0) - expect(asyncStorage.setItem).toHaveBeenCalledOnce() - - releaseDebounced() - await deletion - - expect(asyncStorage.setItem).toHaveBeenCalledTimes(2) - expect(lastWrittenFile().workspaces.map((w) => w.key)).toEqual([hostB]) - }) - it('cannot let an older overlapping write commit after the purge', async () => { - // Why: two debounced writes can sit on the bridge at once, and the second used to replace - // the in-flight handle. The purge then awaited only the newer one, so the older blob -- - // snapshotted while the forgotten host was still in the map -- could commit last. - const hostA = getSessionTabStripCacheKey('host-a', 'wt-1') - const hostB = getSessionTabStripCacheKey('host-b', 'wt-1') - let stored = '' - const gates: Array<() => void> = [] - asyncStorage.setItem.mockImplementation( - (_key: string, value: string) => - new Promise((resolve) => { - gates.push(() => { - stored = value - resolve() - }) - }) - ) - - saveCachedSessionTabStrip(hostA, preview('tab-a')) - await vi.advanceTimersByTimeAsync(300) - saveCachedSessionTabStrip(hostB, preview('tab-b')) - await vi.advanceTimersByTimeAsync(300) - - const deletion = deleteCachedSessionTabStripForHost('host-a') - // Newest released first: only writes that queue behind one another survive this. - for (let step = 0; step < 6 && gates.length > 0; step += 1) { - gates.pop()?.() - await vi.advanceTimersByTimeAsync(0) - } - await deletion - - const keys = (JSON.parse(stored) as { workspaces: { key: string }[] }).workspaces.map( - (workspace) => workspace.key - ) - expect(keys).toEqual([hostB]) - }) -}) diff --git a/mobile/src/cache/session-tab-strip-cache.ts b/mobile/src/cache/session-tab-strip-cache.ts deleted file mode 100644 index d5fef98c75c..00000000000 --- a/mobile/src/cache/session-tab-strip-cache.ts +++ /dev/null @@ -1,260 +0,0 @@ -// Why: reconnecting to a workspace the phone opened a minute ago tears the session screen back -// to an empty strip and a spinner, even though the tab list it is about to be handed is the one -// it just displayed. Persist the shape of the strip per workspace so a reconnect paints the -// known tabs immediately and swaps in live rows under the same keys. -// -// This file is the authority on what reaches plaintext storage, not its callers: every entry is -// rebuilt field by field on the way in, and shell-controlled titles are replaced with fixed -// labels here rather than trusted to have been scrubbed upstream. -import AsyncStorage from '@react-native-async-storage/async-storage' -import { sha256 } from '@noble/hashes/sha256' -import { - getPersistableTabStripTitle, - isDrawableTabStripType, - type MobileSessionTabStripEntry, - type MobileSessionTabStripPreview -} from '../session/mobile-session-tab-strip-entries' - -const STORAGE_KEY = 'orca:session-tab-strip:v1' -// A phone realistically revisits a handful of workspaces; the caps bound both the stored blob -// and the cost of a single write. -const MAX_WORKSPACES = 12 -const MAX_TABS_PER_WORKSPACE = 24 -const MAX_TITLE_LENGTH = 64 -const WRITE_DEBOUNCE_MS = 250 -// 128 bits of a digest: far past collision range for a dozen workspaces, and short enough that -// the stored blob stays small. -const WORKSPACE_DIGEST_LENGTH = 32 - -type StoredWorkspace = { key: string; preview: MobileSessionTabStripPreview } -type StoredFile = { workspaces: StoredWorkspace[] } - -// Insertion-ordered, so the first key is the least recently written one to evict. -let memoryCache: Map | null = null -let loadPromise: Promise> | null = null -let writeTimer: ReturnType | null = null -// Tail of the write chain. Every write queues behind it, so an older setItem can never -// settle after a newer one and make its stale blob the last word on disk. -let writeInFlight: Promise | null = null -// Hosts forgotten this session. A save racing the deletion would re-insert the host and -// the next debounced write would put its tab titles back on disk, so refuse those saves -// outright. Re-pairing the same host caches again from the next app launch — the cheap -// direction for a deletion the user asked for. -const forgottenHosts = new Set() - -/** - * A workspace id ends in a filesystem path, so it is digested rather than stored. The host id - * stays readable because forgetting a host has to be able to find that host's rows, and because - * host ids already key several other entries in this store. - */ -export function getSessionTabStripCacheKey( - hostId: string | undefined, - worktreeId: string | undefined -): string | null { - if (!hostId || !worktreeId) { - return null - } - return JSON.stringify([hostId, digestWorkspaceId(worktreeId)]) -} - -/** Whatever this process already knows, with no await — so a revisit paints on the first frame. */ -export function readCachedSessionTabStrip(key: string | null): MobileSessionTabStripPreview | null { - if (!key || !memoryCache) { - return null - } - return memoryCache.get(key) ?? null -} - -export async function loadCachedSessionTabStrip( - key: string | null -): Promise { - if (!key) { - return null - } - const cache = await loadFile() - return cache.get(key) ?? null -} - -export function saveCachedSessionTabStrip( - key: string | null, - preview: MobileSessionTabStripPreview -): void { - if (!key) { - return - } - const hostId = readHostIdFromKey(key) - if (hostId !== null && forgottenHosts.has(hostId)) { - return - } - const redacted = redactPreview(preview) - const cache = memoryCache ?? new Map() - memoryCache = cache - // Map.set on an existing key keeps its original iteration position, so delete first to make - // the re-inserted key the newest and give the cap true LRU eviction. - cache.delete(key) - cache.set(key, redacted) - while (cache.size > MAX_WORKSPACES) { - const oldest = cache.keys().next().value - if (oldest === undefined) { - break - } - cache.delete(oldest) - } - scheduleWrite(cache) -} - -/** - * Drop every workspace belonging to a host the user has unpaired. Both the in-memory rows and - * the stored blob have to go: leaving either behind means the next save for any other host - * serializes the forgotten host's tabs straight back to disk. - */ -export async function deleteCachedSessionTabStripForHost(hostId: string): Promise { - // Before the first await: a save landing during the load or the write must not - // re-insert the host the caller is in the middle of forgetting. - forgottenHosts.add(hostId) - // Load first so the rewrite below preserves other hosts. If storage is unreadable we still - // rewrite, which can cost another host its rows — the wrong direction for a cache, the right - // one for a deletion the user asked for. - const cache = await loadFile() - // Deleting the entry the iterator is standing on is well-defined for a Map. - for (const key of cache.keys()) { - if (readHostIdFromKey(key) === hostId) { - cache.delete(key) - } - } - if (writeTimer) { - clearTimeout(writeTimer) - writeTimer = null - } - // Queued, not raced: the purge is the last write, and its failure is the caller's. - await enqueueWrite(cache) -} - -export function resetSessionTabStripCacheForTests(): void { - if (writeTimer) { - clearTimeout(writeTimer) - writeTimer = null - } - memoryCache = null - loadPromise = null - writeInFlight = null - forgottenHosts.clear() -} - -function digestWorkspaceId(worktreeId: string): string { - const digest = sha256(new TextEncoder().encode(worktreeId)) - let hex = '' - for (const byte of digest) { - hex += byte.toString(16).padStart(2, '0') - } - return hex.slice(0, WORKSPACE_DIGEST_LENGTH) -} - -function readHostIdFromKey(key: string): string | null { - try { - const parsed = JSON.parse(key) as unknown - return Array.isArray(parsed) && typeof parsed[0] === 'string' ? parsed[0] : null - } catch { - return null - } -} - -async function loadFile(): Promise> { - if (memoryCache) { - return memoryCache - } - loadPromise ??= (async () => { - const parsed = await readStoredFile() - // A save that landed while the read was in flight owns the newer truth. - const cache = memoryCache ?? new Map() - for (const workspace of parsed) { - if (!cache.has(workspace.key)) { - cache.set(workspace.key, workspace.preview) - } - } - memoryCache = cache - return cache - })() - return loadPromise -} - -async function readStoredFile(): Promise { - try { - const raw = await AsyncStorage.getItem(STORAGE_KEY) - if (!raw) { - return [] - } - const parsed = JSON.parse(raw) as StoredFile - if (typeof parsed !== 'object' || parsed === null || !Array.isArray(parsed.workspaces)) { - return [] - } - return parsed.workspaces.flatMap((workspace) => { - if (typeof workspace?.key !== 'string' || !Array.isArray(workspace.preview?.tabs)) { - return [] - } - return [{ key: workspace.key, preview: redactPreview(workspace.preview) }] - }) - } catch { - return [] - } -} - -// Why: a flurry of snapshots (one per desktop republication) must not hammer AsyncStorage. -function scheduleWrite(cache: Map): void { - if (writeTimer) { - clearTimeout(writeTimer) - } - writeTimer = setTimeout(() => { - writeTimer = null - // Best effort by design: a dropped cache refresh costs one repaint, and the next - // save rewrites the whole map. Only the deletion path needs the failure. - void enqueueWrite(cache).catch(() => {}) - }, WRITE_DEBOUNCE_MS) -} - -// Why the chain rather than one handle: two debounced writes can overlap on the bridge, and -// the second overwrote the handle. A deletion then awaited only the newer one, so the older -// write -- serialized before the purge, host rows and all -- could land last and restore them. -function enqueueWrite(cache: Map): Promise { - const queued = (writeInFlight ?? Promise.resolve()).then(() => writeFile(cache)) - // A rejected link must not break the chain for the writes queued behind it. - writeInFlight = queued.catch(() => {}) - return queued -} - -async function writeFile(cache: Map): Promise { - const workspaces: StoredWorkspace[] = [...cache].map(([key, preview]) => ({ key, preview })) - // Throws on purpose: a deletion that only removed the in-memory rows must not be - // reported as a deletion, or the forgotten host's titles stay in plaintext on disk. - await AsyncStorage.setItem(STORAGE_KEY, JSON.stringify({ workspaces })) -} - -// Rebuilt field by field so a field later added to the live tab type cannot ride into storage -// without someone deciding it belongs there. -function redactPreview(preview: MobileSessionTabStripPreview): MobileSessionTabStripPreview { - const tabs: MobileSessionTabStripEntry[] = [] - for (const tab of preview.tabs ?? []) { - if (typeof tab?.id !== 'string' || !isDrawableTabStripType(tab.type)) { - continue - } - const agentId = typeof tab.agentId === 'string' ? tab.agentId : null - const title = typeof tab.title === 'string' ? tab.title : '' - tabs.push({ - id: tab.id, - type: tab.type, - title: getPersistableTabStripTitle({ type: tab.type, title, agentId }).slice( - 0, - MAX_TITLE_LENGTH - ), - agentId - }) - if (tabs.length === MAX_TABS_PER_WORKSPACE) { - break - } - } - const activeTabId = - typeof preview.activeTabId === 'string' && tabs.some((tab) => tab.id === preview.activeTabId) - ? preview.activeTabId - : null - return { tabs, activeTabId } -} diff --git a/mobile/src/components/HostProtocolGate.test.ts b/mobile/src/components/HostProtocolGate.test.ts index b34e4baacaf..44a2265ccb3 100644 --- a/mobile/src/components/HostProtocolGate.test.ts +++ b/mobile/src/components/HostProtocolGate.test.ts @@ -44,12 +44,6 @@ function GateConsumer() { return createElement('GateStatus', null, hostCapabilities.join(',')) } -// Separate from GateStatus so the capability assertions keep their exact rendered shape. -function VerifiedConsumer() { - const { compatVerified } = useHostProtocolGates() - return createElement('GateVerified', null, compatVerified ? 'verified' : 'unverified') -} - // Counts mounts so a test can prove the routes were never torn down, which presence alone can't. const probeMounts = { count: 0 } function MountProbe() { @@ -63,13 +57,7 @@ function gateElement() { return createElement( HostProtocolGate, { hostId: 'host-1' }, - createElement( - 'HostContent', - null, - createElement(GateConsumer), - createElement(VerifiedConsumer), - createElement(MountProbe) - ) + createElement('HostContent', null, createElement(GateConsumer), createElement(MountProbe)) ) } @@ -166,90 +154,13 @@ describe('HostProtocolGate', () => { expect(client.sendRequest).toHaveBeenCalledOnce() }) - it('serves every descendant capability read from the one status.get it issues', async () => { - const client = clientWithStatus({ - protocolVersion: 5, - minCompatibleMobileVersion: 0, - capabilities: ['browser.screencast.v1', 'terminal.queryReplyInput.v1'] - }) - hostClient.current = { client, state: 'connected' } - renderer = await act(async () => { - const created = create( - createElement( - HostProtocolGate, - { hostId: 'host-1' }, - createElement(GateConsumer), - createElement(GateConsumer) - ) - ) - await Promise.resolve() - return created - }) - - // Why: the session route used to run its own retrying status.get on top of this one, so a - // cold open cost two round trips for the same answer. Consumers now read the gate's copy. - expect(client.sendRequest).toHaveBeenCalledOnce() - expect(client.sendRequest).toHaveBeenCalledWith('status.get') - const statuses = renderer.root.findAllByType('GateStatus') - expect(statuses).toHaveLength(2) - for (const status of statuses) { - expect(status.props.children).toBe('browser.screencast.v1,terminal.queryReplyInput.v1') - } - }) - - it('releases the cover on a failed status.get and upgrades when a retry lands', async () => { - vi.useFakeTimers({ shouldAdvanceTime: true }) - const sendRequest = vi - .fn() - .mockRejectedValueOnce(new Error('status.get timed out')) - .mockResolvedValue({ - ok: true, - result: { protocolVersion: 5, minCompatibleMobileVersion: 0, capabilities: ['late.v1'] } - }) - hostClient.current = { client: { sendRequest } as unknown as RpcClient, state: 'connected' } - renderer = await renderGate() - - // Why: a wedged status.get must never trap the routes behind the cover, so the first miss - // settles conservative gates immediately — no capabilities, but a usable UI. - let output = renderedText(renderer) - expect(output).toContain('HostContent') - expect(output).not.toContain('Checking host compatibility') - expect(output).toContain('"type":"GateStatus","props":{},"children":null') - - await act(async () => { - await vi.advanceTimersByTimeAsync(1_100) - }) - - // The probe kept retrying underneath, so the answer arrives without a remount. - expect(sendRequest).toHaveBeenCalledTimes(2) - expect(renderedText(renderer)).toContain('late.v1') - expect(probeMounts.count).toBe(1) - vi.useRealTimers() - }) - - it('blocks a desktop that omits protocolVersion, so a pending verdict is not a formality', async () => { - vi.spyOn(console, 'warn').mockImplementation(() => {}) - // Why this case and not just an explicit old version: evaluateCompat reads a missing - // protocolVersion as 0, so the everyday shape of an old desktop is a blocking one. - hostClient.current = { - client: clientWithStatus({ capabilities: [] }), - state: 'connected' - } - renderer = await renderGate() - const output = renderedText(renderer) - expect(output).toContain('Update Orca on your computer') - expect(output).not.toContain('HostContent') - }) - it('renders the host UI while the host connection is still pending', async () => { hostClient.current = { client: null, state: 'connecting' } renderer = await renderGate() expect(renderedText(renderer)).toContain('HostContent') }) - // Was: the routes were held back until status.get resolved, which serialised every route's - // own startup RPC behind this one round trip. They now mount immediately and are covered. - it('mounts host routes under the pending cover while status.get is still in flight', async () => { + it('does not mount host routes before a connected host passes the compatibility probe', async () => { const client = { sendRequest: vi.fn().mockReturnValue(new Promise(() => {})) } as unknown as RpcClient @@ -257,40 +168,9 @@ describe('HostProtocolGate', () => { renderer = await renderGate() const output = renderedText(renderer) expect(output).toContain('Checking host compatibility') - expect(output).toContain('HostContent') - expect(probeMounts.count).toBe(1) - expect(client.sendRequest).toHaveBeenCalledOnce() - // Why: mounting early must not leak an unproven host's capabilities to the routes below; - // an empty join renders no children, so the consumer saw none. - expect(output).toContain('"type":"GateStatus","props":{},"children":null') - const overlay = renderer.root - .findAllByType('View') - .find((node) => node.props.accessibilityViewIsModal === true) - expect(overlay?.props.pointerEvents).toBe('auto') - }) - - it('unmounts the routes it mounted early when the verdict comes back blocked', async () => { - vi.spyOn(console, 'warn').mockImplementation(() => {}) - let settle: ((response: unknown) => void) | null = null - const client = { - sendRequest: vi.fn().mockReturnValue( - new Promise((resolve) => { - settle = resolve - }) - ) - } as unknown as RpcClient - hostClient.current = { client, state: 'connected' } - renderer = await renderGate() - expect(renderedText(renderer)).toContain('HostContent') - - await act(async () => { - settle?.({ ok: true, result: { protocolVersion: 5, minCompatibleMobileVersion: 999 } }) - await Promise.resolve() - }) - - const output = renderedText(renderer) - expect(output).toContain('Update Orca Mobile') expect(output).not.toContain('HostContent') + expect(probeMounts.count).toBe(0) + expect(client.sendRequest).toHaveBeenCalledOnce() }) it('overlays the pending spinner instead of unmounting routes mounted while connecting', async () => { @@ -379,52 +259,4 @@ describe('HostProtocolGate', () => { renderer = await renderGate() expect(renderedText(renderer)).toContain('HostContent') }) - - it('reports a rejected status.get as unverified, so failing open is not a passing verdict', async () => { - const sendRequest = vi - .fn() - .mockResolvedValue({ ok: false, error: { message: 'no such method' } }) - hostClient.current = { client: { sendRequest } as unknown as RpcClient, state: 'connected' } - renderer = await renderGate() - - // Navigation still works: the host said no, and that must not lock the user out of the route. - const output = renderedText(renderer) - expect(output).toContain('HostContent') - expect(output).not.toContain('Checking host compatibility') - // Why: `compatVerdict` is `ok` here purely as a fallback. Nothing about this host was proven, - // so callers that write to it read this flag instead of the verdict. - expect(output).toContain('["unverified"]') - }) - - it('reports a passing status reply as verified', async () => { - hostClient.current = { - client: clientWithStatus({ protocolVersion: 5, minCompatibleMobileVersion: 0 }), - state: 'connected' - } - renderer = await renderGate() - expect(renderedText(renderer)).toContain('["verified"]') - }) - - it('stays unverified through a failed status.get and flips once a retry answers', async () => { - vi.useFakeTimers({ shouldAdvanceTime: true }) - const sendRequest = vi - .fn() - .mockRejectedValueOnce(new Error('status.get timed out')) - .mockResolvedValue({ - ok: true, - result: { protocolVersion: 5, minCompatibleMobileVersion: 0 } - }) - hostClient.current = { client: { sendRequest } as unknown as RpcClient, state: 'connected' } - renderer = await renderGate() - - expect(renderedText(renderer)).toContain('["unverified"]') - - await act(async () => { - await vi.advanceTimersByTimeAsync(1_100) - }) - - // The retry landed, so the fallback is replaced by a real answer and writes are released. - expect(renderedText(renderer)).toContain('["verified"]') - vi.useRealTimers() - }) }) diff --git a/mobile/src/components/HostProtocolGate.tsx b/mobile/src/components/HostProtocolGate.tsx index d69e4872784..4d9c0c019f1 100644 --- a/mobile/src/components/HostProtocolGate.tsx +++ b/mobile/src/components/HostProtocolGate.tsx @@ -22,26 +22,45 @@ export function useHostProtocolGates(): HostStatusGates { // Why: single choke point above every /h/[hostId] route so a blocked verdict replaces the // whole host UI (sidebar + detail stack) while the host list and other hosts stay usable. -// The routes mount as soon as the connection does, so their startup RPCs (session.tabs.list, -// terminal.list) fly alongside this status.get instead of queueing behind it; a blocked verdict -// then unmounts them and their answers are discarded. export function HostProtocolGate({ hostId, children }: Props) { const { client, state } = useHostClient(hostId) const gates = useHostStatusGates({ hostId, client, connState: state }) const { compatVerdict, statusPending } = gates const resolvedHostIdRef = useRef(null) + const mountedHostIdRef = useRef(null) const hostKey = hostId ?? null const resolvedNow = state === 'connected' && client !== null && !statusPending const blocked = compatVerdict.kind === 'blocked' const pending = statusPending && resolvedHostIdRef.current !== hostKey + const holdBack = pending && mountedHostIdRef.current !== hostKey - // Why: React can replay or discard a render, so the latch records committed outcomes only. + // Why: React can replay or discard a render, so the latches record committed + // outcomes only — a discarded children render must not count as mounted. useEffect(() => { if (resolvedNow) { resolvedHostIdRef.current = hostKey } + if (blocked) { + // Why: the block screen unmounts the routes, so a later pending window + // must not assume a live tree it can overlay. + mountedHostIdRef.current = null + } else if (!holdBack) { + mountedHostIdRef.current = hostKey + } }) + if (holdBack) { + // Why: nothing is mounted yet for this host, so hold the routes back entirely + // rather than letting them mount (and fire their connect RPCs) pre-verdict. + return ( + + + + ) + } if (blocked) { return } @@ -58,11 +77,10 @@ export function HostProtocolGate({ hostId, children }: Props) { {children} {pending ? ( - // Why: cover the stack rather than unmounting it — unmounting for a pending status.get - // destroys in-flight nested navigation, and holding it back would serialise every route's - // startup RPC behind this one. Mount effects underneath run pre-verdict by design; they - // read capabilities from this gate, which reports none until the verdict lands, so every - // capability-dependent surface stays closed rather than guessing. + // Why: once the stack is mounted, unmounting it for a pending status.get destroys + // in-flight nested navigation, so cover it instead. Mount effects underneath still + // run — they wait for connState 'connected' and every capability-dependent call + // re-probes status.get itself, so nothing newer than the baseline fires here. ({ start: vi.fn() })) -vi.mock('../transport/runtime-status-probe', () => ({ +vi.mock('../transport/runtime-capability-probe', () => ({ startRuntimeCapabilityProbe: probe.start })) diff --git a/mobile/src/components/codex-reset-credit-capability.ts b/mobile/src/components/codex-reset-credit-capability.ts index 129dd654ae0..1a32ef37873 100644 --- a/mobile/src/components/codex-reset-credit-capability.ts +++ b/mobile/src/components/codex-reset-credit-capability.ts @@ -1,7 +1,7 @@ import { useEffect, useState } from 'react' import { CODEX_RESET_CREDIT_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version' import type { RpcClient } from '../transport/rpc-client' -import { startRuntimeCapabilityProbe } from '../transport/runtime-status-probe' +import { startRuntimeCapabilityProbe } from '../transport/runtime-capability-probe' // Why: source the capability string from the shared contract so a host bump can never // silently drift from the mobile probe. diff --git a/mobile/src/diagnostics/connection-diagnostics-report.test.ts b/mobile/src/diagnostics/connection-diagnostics-report.test.ts index 8b2128c2d2c..35a1425165b 100644 --- a/mobile/src/diagnostics/connection-diagnostics-report.test.ts +++ b/mobile/src/diagnostics/connection-diagnostics-report.test.ts @@ -1,36 +1,8 @@ import { describe, expect, it } from 'vitest' import { buildConnectionDiagnosticsReport } from './connection-diagnostics-report' -import type { ConnectionLogEntry } from '../transport/types' const NOW = Date.UTC(2026, 6, 9, 22, 0, 0) -function stageEntry( - id: string, - ts: number, - name: string, - ms: number, - complete: boolean -): ConnectionLogEntry { - return { - id, - ts, - level: complete ? 'info' : 'warn', - path: 'relay', - message: `Relay dial stage ${name} ${complete ? 'finished' : 'did not finish'}`, - timing: { kind: 'relay-dial-stage', name, ms, complete } - } -} - -function stateEntry(id: string, ts: number, name: string, ms: number): ConnectionLogEntry { - return { - id, - ts, - level: 'info', - message: `Connection state ${name} → connected`, - timing: { kind: 'connection-state', name, ms, complete: true } - } -} - describe('buildConnectionDiagnosticsReport', () => { it('summarizes a failing Tailscale host with its log', () => { const report = buildConnectionDiagnosticsReport({ @@ -96,28 +68,13 @@ describe('buildConnectionDiagnosticsReport', () => { activePath: 'tailscale', pendingPath: 'relay', entries: [ - { - id: 'relay-stage-opening', - ts: NOW - 6_000, - level: 'info', - path: 'relay', - message: 'Relay dial stage opening finished', - detail: '118ms — resumeToken=secret-resume-token', - timing: { kind: 'relay-dial-stage', name: 'opening', ms: 118, complete: true } - }, { id: 'relay-failure', ts: NOW - 5_000, level: 'error', message: 'Relay: relay dial failed', detail: - 'RelayDirectorHttpError: relay director resolve failed (503); retry after 30000ms; resumeToken=secret-resume-token', - timing: { - kind: 'relay-dial-stage', - name: 'awaiting-hello', - ms: 9_100, - complete: false - } + 'RelayDirectorHttpError: relay director resolve failed (503); retry after 30000ms; resumeToken=secret-resume-token' } ], nowMs: NOW @@ -130,9 +87,6 @@ describe('buildConnectionDiagnosticsReport', () => { expect(report).toContain('Next step: Keep Orca open; recovery should retry automatically.') expect(report).toContain('resumeToken=[redacted]') expect(report).not.toContain('secret-resume-token') - expect(report).toContain( - 'Relay dial stages: opening 118ms · awaiting-hello 9.1s (did not finish) — total 9.2s' - ) }) it('redacts quoted JSON credentials and never echoes an invalid endpoint', () => { @@ -162,52 +116,6 @@ describe('buildConnectionDiagnosticsReport', () => { expect(report).not.toContain('bearer-secret') }) - it('breaks a slow connect down by dial stage and connection state', () => { - const report = buildConnectionDiagnosticsReport({ - hostName: 'Host 6', - endpoint: 'ws://192.168.1.50:6768', - state: 'connected', - reconnectAttempts: 2, - lastConnectedAt: NOW, - platform: 'ios 26.5.1', - appVersion: '0.0.47', - entries: [ - stageEntry('a1', NOW - 30_000, 'opening', 90, false), - stateEntry('s1', NOW - 29_000, 'connecting', 12_000), - stageEntry('b1', NOW - 20_000, 'opening', 120, true), - stageEntry('b2', NOW - 19_000, 'awaiting-hello', 6_400, true), - stageEntry('b3', NOW - 13_000, 'handshaking', 240, true), - stageEntry('b4', NOW - 12_000, 'confirming', 1_180, true), - stateEntry('s2', NOW - 11_000, 'connecting', 8_000) - ], - nowMs: NOW - }) - - // Only the latest dial is broken out, so a reconnect loop cannot average away - // the attempt the reporter is complaining about. - expect(report).toContain( - 'Relay dial stages (latest of 2): opening 120ms · awaiting-hello 6.4s · handshaking 240ms · confirming 1.2s — total 7.9s' - ) - expect(report).toContain('Connection state dwell: connecting 20.0s ×2') - }) - - it('omits the timing lines when nothing recorded a phase duration', () => { - const report = buildConnectionDiagnosticsReport({ - hostName: 'Host 7', - endpoint: 'ws://192.168.1.50:6768', - state: 'connected', - reconnectAttempts: 0, - lastConnectedAt: NOW, - platform: 'ios 26.5.1', - appVersion: '0.0.47', - entries: [{ id: 'plain', ts: NOW, level: 'info', message: 'Authenticated' }], - nowMs: NOW - }) - - expect(report).not.toContain('Relay dial stages') - expect(report).not.toContain('Connection state dwell') - }) - it('bounds a single event line before submission while preserving its identity', () => { const report = buildConnectionDiagnosticsReport({ hostName: 'Host 5', diff --git a/mobile/src/diagnostics/connection-diagnostics-report.ts b/mobile/src/diagnostics/connection-diagnostics-report.ts index 33270b7e4bd..0507b44349a 100644 --- a/mobile/src/diagnostics/connection-diagnostics-report.ts +++ b/mobile/src/diagnostics/connection-diagnostics-report.ts @@ -8,7 +8,6 @@ import { normalizeHostAppVersion } from '../transport/host-app-version-store' import { formatEndpoint } from './host-reachability' import { diagnoseConnection } from './connection-diagnostics-analysis' import { redactConnectionLogEntry, redactConnectionLogText } from './connection-log-redaction' -import { summarizeConnectionLogTimings } from './connection-log-timing-summary' const MAX_EVENT_LINE_BYTES = 2 * 1024 const EVENT_TRUNCATION_MARKER = ' … [truncated]' @@ -60,7 +59,6 @@ export function buildConnectionDiagnosticsReport(args: { ? 'Last connected: never this session' : `Last connected: ${new Date(args.lastConnectedAt).toISOString()} (${formatAgo(now - args.lastConnectedAt)} ago)` ) - lines.push(...summarizeConnectionLogTimings(entries)) lines.push('') lines.push(`Likely cause: ${diagnosis.likelyCause}`) lines.push(`Next step: ${diagnosis.nextStep}`) diff --git a/mobile/src/diagnostics/connection-log-timing-summary.ts b/mobile/src/diagnostics/connection-log-timing-summary.ts deleted file mode 100644 index c9ac38efd54..00000000000 --- a/mobile/src/diagnostics/connection-log-timing-summary.ts +++ /dev/null @@ -1,66 +0,0 @@ -import type { ConnectionLogEntry, ConnectionLogTiming } from '../transport/types' - -// Why: a report that only says "connecting for 10s" cannot be triaged. These lines -// turn the per-phase timings the transport now records into the two questions -// support actually asks: which relay dial stage ate the time, and how long the -// client sat in each connection state. -export function summarizeConnectionLogTimings(entries: readonly ConnectionLogEntry[]): string[] { - const timings = entries.flatMap((entry) => (entry.timing ? [entry.timing] : [])) - const lines: string[] = [] - const dials = groupRelayDials(timings.filter((timing) => timing.kind === 'relay-dial-stage')) - const latestDial = dials.at(-1) - if (latestDial) { - const label = - dials.length > 1 ? `Relay dial stages (latest of ${dials.length})` : 'Relay dial stages' - const total = latestDial.reduce((sum, timing) => sum + timing.ms, 0) - lines.push( - `${label}: ${latestDial.map(formatStageTiming).join(' · ')} — total ${formatDurationMs(total)}` - ) - } - const states = totalPerName(timings.filter((timing) => timing.kind === 'connection-state')) - if (states.length > 0) { - lines.push( - `Connection state dwell: ${states - .map( - ({ name, ms, count }) => `${name} ${formatDurationMs(ms)}${count > 1 ? ` ×${count}` : ''}` - ) - .join(' · ')}` - ) - } - return lines -} - -// Relay dial stages are strictly ordered and every dial starts in 'opening', so an -// 'opening' timing opens a new group. Reporting only the latest keeps a reconnect -// loop from averaging away the attempt the reporter is complaining about. -function groupRelayDials(timings: readonly ConnectionLogTiming[]): ConnectionLogTiming[][] { - const dials: ConnectionLogTiming[][] = [] - for (const timing of timings) { - if (timing.name === 'opening' || dials.length === 0) { - dials.push([]) - } - dials.at(-1)!.push(timing) - } - return dials -} - -function totalPerName( - timings: readonly ConnectionLogTiming[] -): { name: string; ms: number; count: number }[] { - const totals = new Map() - for (const timing of timings) { - const total = totals.get(timing.name) ?? { name: timing.name, ms: 0, count: 0 } - total.ms += timing.ms - total.count += 1 - totals.set(timing.name, total) - } - return [...totals.values()] -} - -function formatStageTiming(timing: ConnectionLogTiming): string { - return `${timing.name} ${formatDurationMs(timing.ms)}${timing.complete ? '' : ' (did not finish)'}` -} - -function formatDurationMs(ms: number): string { - return ms < 1000 ? `${Math.round(ms)}ms` : `${(ms / 1000).toFixed(1)}s` -} diff --git a/mobile/src/session/MobileSessionActiveContent.tsx b/mobile/src/session/MobileSessionActiveContent.tsx index 0ef2a0333b7..019e83c6a99 100644 --- a/mobile/src/session/MobileSessionActiveContent.tsx +++ b/mobile/src/session/MobileSessionActiveContent.tsx @@ -2,8 +2,6 @@ import { Animated, View, Text, Pressable, ActivityIndicator } from 'react-native import { saveTerminalTextScale } from '../storage/preferences' import { MobileBrowserPane } from '../browser/MobileBrowserPane' import { TerminalPaneView } from './TerminalPaneView' -import { TerminalEnginePrewarm } from './TerminalEnginePrewarm' -import { MOBILE_SESSION_TAB_BAR_HEIGHT } from './mobile-session-frame-styles' import { MobileNativeChatOverlay } from './MobileNativeChatOverlay' import { colors } from '../theme/mobile-theme' import { styles } from './mobile-session-styles' @@ -76,38 +74,16 @@ export function MobileSessionActiveContent({ activePendingTerminalTab, isPendingTerminalRecoveryParked, retryPendingTerminalRecovery, - reconnectViewState, - tabStripRows, showLoadingState, - measurePrewarmViewport, showEmptyState, keyboardLift, activeTerminalKeyboardLift, toastAnimatedStyle, createTabBusy } = controller - // Why the same list the header gates on: an unmounted tab bar gives the content row its band - // back, so the pre-warm would measure a taller box than the pane ever gets. Reading the header's - // own rows (live or cached preview) keeps the two from drifting. - const prewarmReservedTabBarHeight = tabStripRows.length > 0 ? 0 : MOBILE_SESSION_TAB_BAR_HEIGHT - // Why: the cached strip in the header is the content during a reconnect; the terminal body - // cannot be, because replaying stored scrollback into the WebView would double-render once the - // live stream replays the same rows. See mobile-session-reconnect-view-state. The engine still - // boots inside the real terminal frame while the startup RPCs are in flight, so the first pane - // inherits a warm WebView and a measured viewport (see prewarm). - return reconnectViewState.kind === 'reconnecting-with-cache' || showLoadingState ? ( - - - - {reconnectViewState.kind === 'reconnecting-with-cache' ? ( - {reconnectViewState.label} - ) : null} - - + return showLoadingState ? ( + + ) : showEmptyState ? ( @@ -195,35 +171,25 @@ export function MobileSessionActiveContent({ )} ) : activePendingTerminalTab ? ( - - - {!isPendingTerminalRecoveryParked && ( - - )} - - {isPendingTerminalRecoveryParked - ? 'Terminal is taking longer than expected' - : activePendingTerminalTab.title || 'Loading terminal'} - - {isPendingTerminalRecoveryParked && ( - [ - styles.createButton, - pressed && styles.newTerminalButtonPressed - ]} - onPress={() => void retryPendingTerminalRecovery()} - > - Retry - - )} - - + + {!isPendingTerminalRecoveryParked && ( + + )} + + {isPendingTerminalRecoveryParked + ? 'Terminal is taking longer than expected' + : activePendingTerminalTab.title || 'Loading terminal'} + + {isPendingTerminalRecoveryParked && ( + [styles.createButton, pressed && styles.newTerminalButtonPressed]} + onPress={() => void retryPendingTerminalRecovery()} + > + Retry + + )} ) : ( - {tabStripRows.length > 0 && ( + {visibleTabs.length > 0 && ( {/* Why: tab taps must register on first press with the keyboard open instead of being eaten by dismissal (#5106). */} - {tabStripRows.map(({ entry, isActive, tab }) => ( + {visibleTabs.map((t) => ( { const { x, width } = e.nativeEvent.layout - tabLayoutsRef.current.set(entry.id, { x, width }) - if (entry.id === activeSessionTabIdRef.current) { - scrollActiveTabIntoView(entry.id, false) + tabLayoutsRef.current.set(t.id, { x, width }) + if (t.id === activeSessionTabIdRef.current) { + scrollActiveTabIntoView(t.id, false) } }} - // A cached preview row has no live tab behind it, so both gestures need the - // reconnect to land first. - disabled={tab === null} - onPress={tab === null ? undefined : () => switchSessionTab(tab)} - onLongPress={ - tab === null - ? undefined - : () => { - triggerMediumImpact() - openSessionTabActionSheetAfterKeyboardDismiss(tab) - } - } + onPress={() => switchSessionTab(t)} + onLongPress={() => { + triggerMediumImpact() + openSessionTabActionSheetAfterKeyboardDismiss(t) + }} delayLongPress={400} > - {entry.type === 'browser' && ( + {t.type === 'browser' && ( )} - {entry.type === 'markdown' && ( + {t.type === 'markdown' && ( )} - {entry.type === 'file' && ( + {t.type === 'file' && ( )} - {entry.agentId !== null && } + {t.type === 'agent-session' && } + {t.type === 'terminal' && + (() => { + const agentId = resolveMobileTerminalTabAgentId(t) + return agentId ? : null + })()} - {entry.title} + {getMobileSessionTabTitle(t)} diff --git a/mobile/src/session/TerminalEnginePrewarm.test.ts b/mobile/src/session/TerminalEnginePrewarm.test.ts deleted file mode 100644 index bdb7ce45bad..00000000000 --- a/mobile/src/session/TerminalEnginePrewarm.test.ts +++ /dev/null @@ -1,238 +0,0 @@ -import { createElement } from 'react' -import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, describe, expect, it, vi } from 'vitest' -import type { TerminalWebViewHandle } from '../terminal/terminal-webview-contract' - -const engine = vi.hoisted(() => ({ - init: vi.fn((_cols: number, _rows: number) => {}), - awaitReady: vi.fn(async () => {}), - measureFitDimensions: vi.fn(async (_containerHeight?: number) => ({ cols: 120, rows: 40 })), - onWebReady: null as (() => void) | null, - textScale: undefined as number | undefined -})) - -vi.mock('react-native', () => ({ - StyleSheet: { - create: (styles: T) => styles, - absoluteFillObject: { position: 'absolute', top: 0, left: 0, right: 0, bottom: 0 } - }, - View: 'View' -})) - -// Stands in for the real engine: records the ref the pre-warm pane holds and the ready callback -// it arms, so a test can drive web-ready and layout in either order. -vi.mock('../terminal/TerminalWebView', async () => { - const { forwardRef, useImperativeHandle } = await import('react') - return { - TerminalWebView: forwardRef< - TerminalWebViewHandle, - { onWebReady?: () => void; textScale?: number } - >(function MockTerminalWebView(props, ref) { - engine.onWebReady = props.onWebReady ?? null - engine.textScale = props.textScale - useImperativeHandle(ref, () => engine as unknown as TerminalWebViewHandle, []) - return createElement('MockTerminalWebView') - }) - } -}) - -import { TerminalEnginePrewarm } from './TerminalEnginePrewarm' - -const FRAME = { x: 0, y: 0, width: 390, height: 700 } - -const TEXT_SCALE = 1.25 - -function renderPrewarm(onEngineMeasured: (ref: TerminalWebViewHandle, height: number) => void): { - renderer: ReactTestRenderer - layout: (frame: { x: number; y: number; width: number; height: number }) => void - webReady: () => void -} { - let renderer: ReactTestRenderer | null = null - act(() => { - renderer = create( - createElement(TerminalEnginePrewarm, { - reservedTabBarHeight: 0, - textScale: TEXT_SCALE, - onEngineMeasured - }) - ) - }) - const created = renderer as unknown as ReactTestRenderer - return { - renderer: created, - layout: (frame) => - act(() => { - created.root.findAllByType('View')[0]?.props.onLayout({ nativeEvent: { layout: frame } }) - }), - webReady: () => - act(() => { - engine.onWebReady?.() - }) - } -} - -// The handoff now waits on the engine's ready promise, so tests have to let microtasks run. -async function flushReady(): Promise { - await act(async () => {}) -} - -afterEach(() => { - engine.measureFitDimensions.mockClear() - engine.init.mockClear() - engine.awaitReady.mockReset() - engine.awaitReady.mockResolvedValue(undefined) - engine.onWebReady = null - engine.textScale = undefined -}) - -describe('TerminalEnginePrewarm', () => { - it('boots the engine without waiting for a terminal to attach', () => { - const measured = vi.fn() - const { renderer } = renderPrewarm(measured) - // The engine mounts on the first render, so its bundle loads while the startup RPCs fly. - expect(renderer.root.findAllByType('MockTerminalWebView')).toHaveLength(1) - expect(measured).not.toHaveBeenCalled() - }) - - it('withholds the measurement until the pane has a real layout', async () => { - const measured = vi.fn() - const { webReady, layout } = renderPrewarm(measured) - - webReady() - // Why: this is the 80x24 trap — an unsized engine answers with xterm's default, and that - // number would ride the first subscribe to the host as the PTY size. - expect(measured).not.toHaveBeenCalled() - - layout({ ...FRAME, width: 0, height: 0 }) - expect(measured).not.toHaveBeenCalled() - - layout(FRAME) - await flushReady() - expect(measured).toHaveBeenCalledOnce() - expect(measured.mock.calls[0]?.[1]).toBe(FRAME.height) - }) - - it('withholds the measurement until the engine reports ready', async () => { - const measured = vi.fn() - const { layout, webReady } = renderPrewarm(measured) - - layout(FRAME) - expect(measured).not.toHaveBeenCalled() - - webReady() - await flushReady() - expect(measured).toHaveBeenCalledOnce() - }) - - it('measures once however many times layout and web-ready repeat', async () => { - const measured = vi.fn() - const { layout, webReady } = renderPrewarm(measured) - - layout(FRAME) - webReady() - webReady() - layout({ ...FRAME, height: 640 }) - layout(FRAME) - await flushReady() - - expect(measured).toHaveBeenCalledOnce() - }) - - it('opens the engine before handing it over, because web-ready alone builds no terminal', async () => { - const measured = vi.fn() - let releaseReady: (() => void) | null = null - engine.awaitReady.mockImplementation( - () => - new Promise((resolve) => { - releaseReady = resolve - }) - ) - const { layout, webReady } = renderPrewarm(measured) - - layout(FRAME) - webReady() - // Why: the WebView answers `measure` with null while it has no terminal, and the pane latches - // once, so handing the engine over before init would spend the one measurement on nothing. - expect(engine.init).toHaveBeenCalledOnce() - expect(measured).not.toHaveBeenCalled() - - releaseReady?.() - await flushReady() - expect(measured).toHaveBeenCalledOnce() - expect(measured.mock.calls[0]?.[0]).toBe(engine) - }) - - it('pre-warms at the text size the first pane will open with', () => { - renderPrewarm(vi.fn()) - // Cell size is what the frame gets divided by, so a default-sized engine would measure a - // different phone than the one the user is looking at. - expect(engine.textScale).toBe(TEXT_SCALE) - }) - - it('reports the frame the pane ended up with when a resize lands during engine start-up', async () => { - const measured = vi.fn() - let releaseReady: (() => void) | null = null - engine.awaitReady.mockImplementation( - () => - new Promise((resolve) => { - releaseReady = resolve - }) - ) - const { layout, webReady } = renderPrewarm(measured) - - layout(FRAME) - webReady() - expect(measured).not.toHaveBeenCalled() - - // A rotation or split-screen resize while the engine is still coming up. The latch has already - // fired, so this is the last chance to correct the height the one measurement is taken against. - const resized = { ...FRAME, width: 700, height: 360 } - layout(resized) - - releaseReady?.() - await flushReady() - - expect(measured).toHaveBeenCalledOnce() - expect(measured.mock.calls[0]?.[1]).toBe(resized.height) - }) - - it('drops the handoff when the pane unmounts before the engine is ready', async () => { - const measured = vi.fn() - let releaseReady: (() => void) | null = null - engine.awaitReady.mockImplementation( - () => - new Promise((resolve) => { - releaseReady = resolve - }) - ) - const { renderer, layout, webReady } = renderPrewarm(measured) - layout(FRAME) - webReady() - - act(() => { - renderer.unmount() - }) - releaseReady?.() - await flushReady() - - // The frame this measurement was taken against is gone, so it describes nothing. - expect(measured).not.toHaveBeenCalled() - }) - - it('is inert: no touches, no accessibility, and nothing sent to a terminal', async () => { - const measured = vi.fn() - const { renderer, layout, webReady } = renderPrewarm(measured) - layout(FRAME) - webReady() - await flushReady() - - const pane = renderer.root.findAllByType('View')[0] - expect(pane?.props.pointerEvents).toBe('none') - expect(pane?.props.accessibilityElementsHidden).toBe(true) - expect(pane?.props.importantForAccessibility).toBe('no-hide-descendants') - // The pane owns no handle, so it has no way to subscribe, send input, or resize a PTY. - // Opening the engine is WebView-local; the measurement itself is the caller's to take. - expect(engine.measureFitDimensions).not.toHaveBeenCalled() - expect(measured.mock.calls[0]?.[0]).toBe(engine) - }) -}) diff --git a/mobile/src/session/TerminalEnginePrewarm.tsx b/mobile/src/session/TerminalEnginePrewarm.tsx deleted file mode 100644 index a98585946e7..00000000000 --- a/mobile/src/session/TerminalEnginePrewarm.tsx +++ /dev/null @@ -1,111 +0,0 @@ -import { useCallback, useRef } from 'react' -import { StyleSheet, View, type LayoutChangeEvent } from 'react-native' -import { TerminalWebView } from '../terminal/TerminalWebView' -import type { TerminalWebViewHandle } from '../terminal/terminal-webview-contract' - -// Diagnostics label for the measurement this pane contributes; it is not a PTY handle. -export const TERMINAL_ENGINE_PREWARM_HANDLE = '(engine-prewarm)' - -// Why: the WebView builds no xterm until it is told to, and `measure` answers null while `term` -// is null, so the engine has to be opened before it can be asked anything. These are placeholder -// dimensions for an empty buffer nobody reads; the measurement derives its own cols and rows from -// the frame and the font's cell size, so nothing downstream inherits them. -const PREWARM_INIT_COLS = 80 -const PREWARM_INIT_ROWS = 24 - -type Props = { - // Height the tab bar will claim from the top of this frame once the session has a tab. The - // loading state has no visible tab, so the bar is not mounted yet and the box the pane will - // finally occupy is this much shorter. Reserving it keeps the measurement honest; measuring - // the taller box would latch too many rows and send them to the host as the PTY size. - reservedTabBarHeight: number - // Why: the first pane opens at the user's saved text size, and cell size is what the - // measurement divides the frame by. Pre-warming at a different size measures a different phone. - textScale: number - onEngineMeasured: (ref: TerminalWebViewHandle, frameHeight: number) => void -} - -// Why: a session still resolving its tabs already knows it is heading for a terminal, so load -// the xterm engine alongside the startup RPCs instead of after terminal.list returns. This pane -// owns no handle: it never subscribes, never sends input, and can never resize a PTY. Its only -// output is the viewport measurement the first real pane would otherwise pay a round trip for. -export function TerminalEnginePrewarm({ - reservedTabBarHeight, - textScale, - onEngineMeasured -}: Props) { - const engineRef = useRef(null) - const frameHeightRef = useRef(0) - const webReadyRef = useRef(false) - const measuredRef = useRef(false) - - // Idempotent by construction: both triggers funnel here and the latch fires once per mount. - const measureWhenSized = useCallback(() => { - const engine = engineRef.current - // Why: an unsized or unmounted WebView measures xterm's 80x24 default, and that number - // rides the first subscribe to the host. Only a laid-out engine is allowed to answer. - if (measuredRef.current || !webReadyRef.current || !engine || frameHeightRef.current <= 0) { - return - } - measuredRef.current = true - // `web-ready` only says the xterm bundle loaded. Opening the engine is what creates `term`, - // and `awaitReady` is what lets its cell dimensions exist before anything reads them. - engine.init(PREWARM_INIT_COLS, PREWARM_INIT_ROWS) - void engine.awaitReady().then(() => { - // React nulls the ref on unmount, so this proves the pane the frame belongs to is still up. - if (engineRef.current !== engine) { - return - } - // Why read the height here and not before the wait: a rotation or split-screen resize during - // engine start-up re-lays out this pane, and the latch above already refused the second - // handoff, so a height captured earlier would be the only one this pane ever reports. - onEngineMeasured(engine, frameHeightRef.current) - }) - }, [onEngineMeasured]) - - const handleLayout = useCallback( - (event: LayoutChangeEvent) => { - const { height, width } = event.nativeEvent.layout - if (width <= 0 || height <= 0) { - return - } - frameHeightRef.current = height - measureWhenSized() - }, - [measureWhenSized] - ) - - const handleWebReady = useCallback(() => { - webReadyRef.current = true - measureWhenSized() - }, [measureWhenSized]) - - return ( - - - - ) -} - -const styles = StyleSheet.create({ - prewarmPane: { - ...StyleSheet.absoluteFillObject, - opacity: 0 - }, - prewarmWebView: { - flex: 1 - } -}) diff --git a/mobile/src/session/mobile-session-frame-styles.ts b/mobile/src/session/mobile-session-frame-styles.ts index cd13bb1173f..a02c14be014 100644 --- a/mobile/src/session/mobile-session-frame-styles.ts +++ b/mobile/src/session/mobile-session-frame-styles.ts @@ -2,20 +2,6 @@ import { StyleSheet } from 'react-native' import { colors, spacing, radii, typography } from '../theme/mobile-theme' -// Why one constant for the whole strip: the terminal frame is whatever the tab bar leaves behind, -// and the engine pre-warm has to reserve exactly that much before the bar exists. Every row child -// is pinned to this height so nothing can grow the bar without moving the reservation with it. -// -// The row deliberately has NO explicit height. React Native lays out border-box, so `height: 36` -// with a 1 px top border would render a 36 px row over a 35 px content area and squeeze children -// that are themselves 36 -- and it would leave this constant one pixel long, which is a whole row -// of drift once a frame sits near a row boundary. Left to size itself the row takes its tallest -// child and adds the border outside it, which is exactly the sum below. -export const MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT = 36 -export const MOBILE_SESSION_TAB_BAR_BORDER_WIDTH = 1 -export const MOBILE_SESSION_TAB_BAR_HEIGHT = - MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT + MOBILE_SESSION_TAB_BAR_BORDER_WIDTH - export const mobileSessionFrameStyles = StyleSheet.create({ container: { flex: 1, @@ -94,12 +80,12 @@ export const mobileSessionFrameStyles = StyleSheet.create({ tabBar: { flexDirection: 'row', alignItems: 'center', - borderTopWidth: MOBILE_SESSION_TAB_BAR_BORDER_WIDTH, + borderTopWidth: 1, borderTopColor: colors.borderSubtle }, tabScroll: { flex: 1, - maxHeight: MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT + maxHeight: 36 }, tabContent: { paddingLeft: spacing.sm, @@ -108,7 +94,7 @@ export const mobileSessionFrameStyles = StyleSheet.create({ tab: { width: 128, maxWidth: 128, - minHeight: MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT, + minHeight: 36, alignItems: 'center', justifyContent: 'center', paddingHorizontal: spacing.sm, @@ -116,11 +102,6 @@ export const mobileSessionFrameStyles = StyleSheet.create({ borderBottomWidth: 2, borderBottomColor: 'transparent' }, - // Why: a cached row is inert until the reconnect lands, so it carries the same de-emphasis as - // the disabled tab-bar buttons beside it rather than passing for a live tab. - tabPreview: { - opacity: 0.45 - }, tabActive: { // Neutral grey underline, matching the desktop terminal tab's active // indicator (a muted foreground/card mix), not a blue accent. @@ -142,7 +123,7 @@ export const mobileSessionFrameStyles = StyleSheet.create({ }, newTerminalButton: { width: 40, - height: MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT, + height: 36, alignItems: 'center', justifyContent: 'center', borderBottomWidth: 2, diff --git a/mobile/src/session/mobile-session-reconnect-view-state.test.ts b/mobile/src/session/mobile-session-reconnect-view-state.test.ts deleted file mode 100644 index 09f9bbb8447..00000000000 --- a/mobile/src/session/mobile-session-reconnect-view-state.test.ts +++ /dev/null @@ -1,155 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { selectMobileSessionReconnectViewState } from './mobile-session-reconnect-view-state' -import { - getMobileSessionTabStripRows, - toMobileSessionTabStripPreview, - type MobileSessionTabStripPreview -} from './mobile-session-tab-strip-entries' -import type { MobileSessionTab } from './mobile-session-route-types' - -function terminalTab(id: string, title: string, isActive = false): MobileSessionTab { - return { type: 'terminal', id, title, terminal: `h-${id}`, isActive } -} - -const cachedPreview: MobileSessionTabStripPreview = { - tabs: [ - { id: 'tab-1', type: 'terminal', title: 'claude', agentId: 'claude' }, - { id: 'tab-2', type: 'terminal', title: 'shell', agentId: null } - ], - activeTabId: 'tab-1' -} - -const base = { - connState: 'reconnecting', - verdictKind: 'normal', - terminalsLoaded: false, - liveTabCount: 0, - activeHandle: null, - cachedPreview: null -} as const - -describe('selectMobileSessionReconnectViewState', () => { - it('renders the cached strip with a progress label while reconnecting', () => { - const state = selectMobileSessionReconnectViewState({ ...base, cachedPreview }) - - expect(state).toEqual({ - kind: 'reconnecting-with-cache', - preview: cachedPreview, - label: 'Reconnecting…' - }) - }) - - it('labels the post-connect hydration gap as loading, not reconnecting', () => { - const state = selectMobileSessionReconnectViewState({ - ...base, - connState: 'connected', - cachedPreview - }) - - expect(state.kind === 'reconnecting-with-cache' && state.label).toBe('Loading tabs…') - }) - - it('blocks when nothing is cached for this workspace', () => { - expect(selectMobileSessionReconnectViewState(base)).toEqual({ kind: 'blocking' }) - expect( - selectMobileSessionReconnectViewState({ - ...base, - cachedPreview: { tabs: [], activeTabId: null } - }) - ).toEqual({ kind: 'blocking' }) - }) - - it('keeps mounted live content instead of swapping in its own cached snapshot', () => { - expect( - selectMobileSessionReconnectViewState({ ...base, liveTabCount: 2, cachedPreview }) - ).toEqual({ kind: 'live' }) - expect( - selectMobileSessionReconnectViewState({ ...base, activeHandle: 'h-1', cachedPreview }) - ).toEqual({ kind: 'live' }) - }) - - it('treats a host-confirmed empty workspace as live', () => { - expect( - selectMobileSessionReconnectViewState({ - ...base, - connState: 'connected', - terminalsLoaded: true, - cachedPreview - }) - ).toEqual({ kind: 'live' }) - }) - - it('falls back to the offline state once the retry loop or the pairing has failed', () => { - expect( - selectMobileSessionReconnectViewState({ ...base, verdictKind: 'unreachable', cachedPreview }) - ).toEqual({ kind: 'offline' }) - expect( - selectMobileSessionReconnectViewState({ ...base, verdictKind: 'auth-failed', cachedPreview }) - ).toEqual({ kind: 'offline' }) - }) - - it('keeps showing the cache through a transient warning verdict', () => { - expect( - selectMobileSessionReconnectViewState({ ...base, verdictKind: 'warning', cachedPreview }).kind - ).toBe('reconnecting-with-cache') - }) -}) - -describe('getMobileSessionTabStripRows', () => { - it('draws disabled preview rows while reconnecting, then the live tabs under the same keys', () => { - const preview = selectMobileSessionReconnectViewState({ ...base, cachedPreview }) - const previewRows = getMobileSessionTabStripRows({ - liveTabs: [], - activeSessionTabId: null, - preview: preview.kind === 'reconnecting-with-cache' ? preview.preview : null - }) - - expect(previewRows.map((row) => row.entry.id)).toEqual(['tab-1', 'tab-2']) - expect(previewRows.map((row) => row.tab)).toEqual([null, null]) - expect(previewRows.map((row) => row.isActive)).toEqual([true, false]) - - const liveTabs = [terminalTab('tab-1', 'claude', true), terminalTab('tab-2', 'shell')] - const liveRows = getMobileSessionTabStripRows({ - liveTabs, - activeSessionTabId: 'tab-1', - preview: null - }) - - expect(liveRows.map((row) => row.entry.id)).toEqual(previewRows.map((row) => row.entry.id)) - expect(liveRows.map((row) => row.isActive)).toEqual(previewRows.map((row) => row.isActive)) - expect(liveRows.every((row) => row.tab !== null)).toBe(true) - }) - - it('prefers live tabs over a preview that is still present', () => { - const rows = getMobileSessionTabStripRows({ - liveTabs: [terminalTab('tab-9', 'fresh', true)], - activeSessionTabId: 'tab-9', - preview: cachedPreview - }) - - expect(rows.map((row) => row.entry.id)).toEqual(['tab-9']) - }) - - it('keeps only the drawn fields when projecting a preview to persist', () => { - const preview = toMobileSessionTabStripPreview( - [ - { - type: 'terminal', - id: 'tab-1', - title: 'claude', - terminal: 'h-1', - launchAgent: 'claude', - launchDraft: 'unsent secret prompt', - isActive: true - } - ], - 'tab-1' - ) - - expect(preview).toEqual({ - tabs: [{ id: 'tab-1', type: 'terminal', title: 'claude', agentId: 'claude' }], - activeTabId: 'tab-1' - }) - expect(JSON.stringify(preview)).not.toContain('unsent secret prompt') - }) -}) diff --git a/mobile/src/session/mobile-session-reconnect-view-state.ts b/mobile/src/session/mobile-session-reconnect-view-state.ts deleted file mode 100644 index fe980676408..00000000000 --- a/mobile/src/session/mobile-session-reconnect-view-state.ts +++ /dev/null @@ -1,61 +0,0 @@ -import type { ConnectionVerdict } from '../transport/connection-health' -import type { ConnectionState } from '../transport/types' -import type { MobileSessionTabStripPreview } from './mobile-session-tab-strip-entries' - -/** - * What the session screen should draw while the phone is not yet serving live tabs. - * - * - `live`: real tabs are mounted (or the host has confirmed there are none). The existing - * loading/empty/content branches own the screen. - * - `reconnecting-with-cache`: nothing live yet, but this workspace's last strip is on the - * device. Draw it, disabled, with a compact progress line instead of a bare spinner. - * - `offline`: the retry loop has given up or the pairing is rejected. A stale strip would - * imply a session we cannot reach, so fall back to the existing offline affordance. - * - `blocking`: nothing live and nothing cached. Unchanged from before this state existed. - */ -export type MobileSessionReconnectViewState = - | { kind: 'live' } - | { kind: 'reconnecting-with-cache'; preview: MobileSessionTabStripPreview; label: string } - | { kind: 'offline' } - | { kind: 'blocking' } - -export function selectMobileSessionReconnectViewState(args: { - connState: ConnectionState - verdictKind: ConnectionVerdict['kind'] - terminalsLoaded: boolean - liveTabCount: number - activeHandle: string | null - cachedPreview: MobileSessionTabStripPreview | null -}): MobileSessionReconnectViewState { - const { connState, verdictKind, terminalsLoaded, liveTabCount, activeHandle, cachedPreview } = - args - // A mounted terminal or tab is the real thing; a mid-session drop must never trade it for a - // snapshot of itself, however the connection is faring. - if (liveTabCount > 0 || activeHandle !== null) { - return { kind: 'live' } - } - // The host has answered and said this workspace is empty — that is live truth, not a gap. - if (connState === 'connected' && terminalsLoaded) { - return { kind: 'live' } - } - if (verdictKind === 'unreachable' || verdictKind === 'auth-failed') { - return { kind: 'offline' } - } - if (cachedPreview && cachedPreview.tabs.length > 0) { - return { - kind: 'reconnecting-with-cache', - preview: cachedPreview, - label: reconnectProgressLabel(connState) - } - } - return { kind: 'blocking' } -} - -function reconnectProgressLabel(connState: ConnectionState): string { - if (connState === 'connected') { - return 'Loading tabs…' - } - return connState === 'reconnecting' || connState === 'disconnected' - ? 'Reconnecting…' - : 'Connecting…' -} diff --git a/mobile/src/session/mobile-session-route-parity.test.ts b/mobile/src/session/mobile-session-route-parity.test.ts index eeeae20ba0c..bc951bfa206 100644 --- a/mobile/src/session/mobile-session-route-parity.test.ts +++ b/mobile/src/session/mobile-session-route-parity.test.ts @@ -37,7 +37,6 @@ const LOGIC_EXPANSION_NAMES = new Set([ 'useMobileSessionContentCreateActions', 'useMobileSessionCloseActions', 'useMobileSessionBulkClose', - 'useMobileSessionTabStripCache', 'useMobileSessionPresentation', 'useMobileSessionPanelRouteActions' ]) @@ -63,32 +62,32 @@ const HOST_COMPONENT_NAMES = new Set([ 'View' ]) -const HEAD_MAIN_HOOK_SHA256 = 'e22e7d3a1147ef19c747f0e216b778e794a73e027e96cf84fac2dde03b37b640' -const HEAD_HOOK_BINDING_SHA256 = '531fe06cf2c261b1346bbc949c9ceba5aea8b8ace2dcb8a1898e9759745e013c' +const HEAD_MAIN_HOOK_SHA256 = '10071240ef9edafc2b9c8bed73be83dceaf7828e3b29f17dab55da020a7697a6' +const HEAD_HOOK_BINDING_SHA256 = '1dadb8c3dc0573ea20659ce7251629669e618dd0effaeac3a4536b29c2e865a1' const HEAD_CALLBACK_IDENTITY_SHA256 = - 'e5df1043256bcb0b3813bf89161d91f5e65c00749fbb6d98176bca82e878d061' -const HEAD_CALLBACK_BODY_SHA256 = '6d9ed614ed139aef5cc911c33ea4220cc1fc5f888a1a564ef85e6910cc118bc3' -const HEAD_EFFECT_SHA256 = 'a6d4d5cb573926f40faa7701cef7885a0f2c7e7c5cfaa91f4e480c29aba44d79' + '2a9e4825df007f6ef53b81aa5004991d6318eee7507b44d625c07e630be432eb' +const HEAD_CALLBACK_BODY_SHA256 = '22103ba85a86e3a3fcb80a7509c7a455d79863010cde3af02db6565b55e3ebe9' +const HEAD_EFFECT_SHA256 = 'd9ebfaabc1e79773cdada7ab370b20459ed972f1f8edce1652199f4d0391cd13' const HEAD_CONTENT_HOOK_SHA256 = '9c3b612fef3f370d66873aefdbe1d701f20cb64ded31fef5cc45fde6f8189581' const HEAD_NESTED_FUNCTION_SHA256 = - '0e553eb5ec7aeda8f8336b8da85ff87eb3657a21fa32d3c75c9cc32e36860244' + '536c72b233c813bb0cea164b090bdce5406ceb965bbc5b83c1f89b89b46f3821' const HEAD_NATIVE_REGISTRATION_SHA256 = 'cab85e4e4a3f43289ba93ddea9ccce57aea83e0bf14fd1620a965aad0c1cb49e' const HEAD_NATIVE_REMOVAL_SHA256 = '4c994574675a2a0f9c607b3ea89ab7a2ed5a83f7c72fa42342ddcb5f00fc3f4f' const HEAD_TIMER_CREATION_SHA256 = - '36c3ccef371698e25cd2eb239df7a8dea6dcc674d9da43cc38cabfa3a8f64929' -const HEAD_TIMER_CLEANUP_SHA256 = '2f41ddc30d0e9c1b6d1d6b5e09d96d1b3facd3133acae1ff7436bb40e4ef39dc' + '1a31b625e2174c3db77272249843196d2b6b06ab1e654a96d8f7858e3082e66b' +const HEAD_TIMER_CLEANUP_SHA256 = 'c73f1d1c2cc89642f3d727d6f3b6b81860a9d6f34234541a2065ec3d1a8cd116' const HEAD_RUNTIME_STRING_SHA256 = - '694a22ed924ebc2a7d380089ff2cfd3e27f5d72d3c4d4b7b06aa3006db93c053' -const HEAD_HOST_JSX_SHA256 = '0aca9fe4b6738228020fe20334fe2716471a2fdf57e4feea6a2a92cac1c04c58' -const HEAD_LEAF_JSX_SHA256 = '2c38e19ffbcaae14f9df2fdb44751546d2b936f9a4b2c5e90727a5f74f3c2665' + '31951b0b83be01ebfa659c4b94df9ad7eaff6404df5338fbade89eb7473a3cb4' +const HEAD_HOST_JSX_SHA256 = '390405926b1695fa3a33686f0bc192b432f5468d8576499d7cafbb4922defbb5' +const HEAD_LEAF_JSX_SHA256 = '21dba981875e173f692590bf910d60964660c5f4cbb79f3a377c7e54f6a1f016' const HEAD_STYLE_REFERENCE_SHA256 = - 'da81d6065c5c1ebafbbd721321023cddd0bfc1afa0325749f736bb97898f9556' + '295a3501c2c6d7bea7c8bbf38b3f3534f01344cd7e1b91bb8e07c040821d596a' const HEAD_IDENTITY_FIELD_SHA256 = - 'a7444b7d0953edb34abc77180ba11d458b02081547b8499249571efd30ac0609' + '91146853930a34dd1f3d80e5c97fbacd7cf19fb93dd26fe8fc6f29169622f9d6' const HEAD_NAVIGATION_SHA256 = '9d96f5dad7de555d6553eac39c0fab00efad507470fd562cb9beaa32db16f512' -const HEAD_CAPABILITY_SHA256 = '54c74cdb468d015c31517004e005187f6cff2ddb07e25fdb4a7060a2fac6b786' +const HEAD_CAPABILITY_SHA256 = 'ca219f7909a091717110b823d5b94a20770ad3ae51894e0fa765e8628309392d' type Definition = { declaration: ts.FunctionDeclaration; sourceFile: ts.SourceFile } type HookFacts = { @@ -457,10 +456,8 @@ function readCompatibilityFacts(definitions: ReadonlyMap): { : '' const callText = canonical(node, sourceFile) if ( - // hostCapabilities.* is included: the session route now reads the gate's shared status.get - // answer instead of running its own probe, and those reads still have to stay ratcheted. - ['useHostProtocolGates', 'supportsMobileQuickCommands'].includes(callName) || - (callName === 'includes' && /[cC]apabilities\.includes/.test(callText)) + ['startRuntimeCapabilityProbe', 'supportsMobileQuickCommands'].includes(callName) || + (callName === 'includes' && callText.includes('capabilities.includes')) ) { capabilities.push(callText) } @@ -475,18 +472,18 @@ describe('mobile session route extraction parity', () => { const contentBindings = CONTENT_COMPONENT_NAMES.flatMap( (name) => readHookFacts(name, definitions).bindings ) - expect(main.hooks).toHaveLength(272) + expect(main.hooks).toHaveLength(266) expect(hash(main.hooks)).toBe(HEAD_MAIN_HOOK_SHA256) expect(hash(main.bindings)).toBe(HEAD_HOOK_BINDING_SHA256) - expect(main.callbacks).toHaveLength(78) + expect(main.callbacks).toHaveLength(77) expect(hash(main.callbacks)).toBe(HEAD_CALLBACK_IDENTITY_SHA256) expect(hash(main.callbackBodies)).toBe(HEAD_CALLBACK_BODY_SHA256) - expect(main.effects).toHaveLength(27) + expect(main.effects).toHaveLength(24) expect(hash(main.effects)).toBe(HEAD_EFFECT_SHA256) expect(contentBindings).toHaveLength(14) expect(hash(contentBindings)).toBe(HEAD_CONTENT_HOOK_SHA256) const nestedFunctions = readNestedFunctions(definitions) - expect(nestedFunctions).toHaveLength(13) + expect(nestedFunctions).toHaveLength(12) expect(hash(nestedFunctions)).toBe(HEAD_NESTED_FUNCTION_SHA256) }) @@ -497,23 +494,20 @@ describe('mobile session route extraction parity', () => { expect(hash(native.registrations)).toBe(HEAD_NATIVE_REGISTRATION_SHA256) expect(native.removals).toHaveLength(9) expect(hash(native.removals)).toBe(HEAD_NATIVE_REMOVAL_SHA256) - expect(native.creations.filter((fact) => fact.startsWith('setTimeout'))).toHaveLength(8) + expect(native.creations.filter((fact) => fact.startsWith('setTimeout'))).toHaveLength(7) expect(native.creations.filter((fact) => fact.startsWith('setInterval'))).toHaveLength(1) expect( native.creations.filter((fact) => fact.startsWith('requestAnimationFrame')) ).toHaveLength(1) expect(hash(native.creations)).toBe(HEAD_TIMER_CREATION_SHA256) - expect(native.cleanups.filter((fact) => fact.startsWith('clearTimeout'))).toHaveLength(12) + expect(native.cleanups.filter((fact) => fact.startsWith('clearTimeout'))).toHaveLength(11) expect(native.cleanups.filter((fact) => fact.startsWith('clearInterval'))).toHaveLength(1) expect(native.cleanups.filter((fact) => fact.startsWith('cancelAnimationFrame'))).toHaveLength( 1 ) expect(hash(native.cleanups)).toBe(HEAD_TIMER_CLEANUP_SHA256) const compatibility = readCompatibilityFacts(definitions) - // 13, not 14: both worktree.activate call sites now share one payload builder, so the - // literal `notifyClients: false` they used to repeat appears once. The guarantee itself is - // pinned in mobile-session-startup-source.test.ts, which requires exactly one call site. - expect(compatibility.identityFields).toHaveLength(13) + expect(compatibility.identityFields).toHaveLength(14) expect(hash(compatibility.identityFields)).toBe(HEAD_IDENTITY_FIELD_SHA256) expect(compatibility.navigation).toHaveLength(6) expect(hash(compatibility.navigation)).toBe(HEAD_NAVIGATION_SHA256) @@ -523,14 +517,14 @@ describe('mobile session route extraction parity', () => { it('preserves runtime strings, styles, and the expanded JSX tree', () => { const strings = readRuntimeStrings() - expect(strings).toHaveLength(545) + expect(strings).toHaveLength(546) expect(hash(strings)).toBe(HEAD_RUNTIME_STRING_SHA256) const jsx = readJsxFacts(readDefinitions()) - expect(jsx.host).toHaveLength(127) + expect(jsx.host).toHaveLength(124) expect(hash(jsx.host)).toBe(HEAD_HOST_JSX_SHA256) - expect(jsx.leaf).toHaveLength(62) + expect(jsx.leaf).toHaveLength(61) expect(hash(jsx.leaf)).toBe(HEAD_LEAF_JSX_SHA256) - expect(jsx.styleReferences).toHaveLength(176) + expect(jsx.styleReferences).toHaveLength(172) expect(hash(jsx.styleReferences)).toBe(HEAD_STYLE_REFERENCE_SHA256) }) }) diff --git a/mobile/src/session/mobile-session-route-source-family.test-support.ts b/mobile/src/session/mobile-session-route-source-family.test-support.ts index acb2bef34a8..41f2d8b9c2f 100644 --- a/mobile/src/session/mobile-session-route-source-family.test-support.ts +++ b/mobile/src/session/mobile-session-route-source-family.test-support.ts @@ -33,7 +33,6 @@ export const MOBILE_SESSION_ROUTE_SOURCE_FILES = [ './use-mobile-session-content-create-actions.ts', './use-mobile-session-close-actions.ts', './use-mobile-session-bulk-close.ts', - './use-mobile-session-tab-strip-cache.ts', './use-mobile-session-presentation.ts', './use-mobile-session-panel-route-actions.tsx', './MobileSessionMarkdownReader.tsx', diff --git a/mobile/src/session/mobile-session-startup-parallelism.test.ts b/mobile/src/session/mobile-session-startup-parallelism.test.ts deleted file mode 100644 index 5ef2c22a483..00000000000 --- a/mobile/src/session/mobile-session-startup-parallelism.test.ts +++ /dev/null @@ -1,279 +0,0 @@ -import { createElement, type ReactElement } from 'react' -import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { useMobileSessionStartup } from './use-mobile-session-startup' -import type { MobileSessionKeyboardStateModel } from './use-mobile-session-keyboard-state' - -type Deferred = { promise: Promise; resolve: (value: T) => void; reject: (e: Error) => void } - -function defer(): Deferred { - let resolve!: (value: T) => void - let reject!: (error: Error) => void - const promise = new Promise((res, rej) => { - resolve = res - reject = rej - }) - return { promise, resolve, reject } -} - -type StartupCall = { rpc: 'session.tabs.list' | 'terminal.list'; worktreeId: string } - -// One session's worth of scope: only the fields useMobileSessionStartup actually reads, plus -// the two reads under test wired to deferreds so a test controls exactly when they settle. -function makeScope(worktreeId: string, calls: StartupCall[], protocolVerified = true) { - const tabs = defer() - const terminals = defer() - const sendRequest = vi.fn().mockResolvedValue({ ok: true, result: {} }) - const scope = { - hostId: 'host-1', - worktreeId, - created: '0', - isFloatingWorkspaceRoute: false, - connState: 'connected', - client: { sendRequest }, - protocolVerified, - setTerminals: vi.fn(), - terminalsRef: { current: [] }, - setSessionTabs: vi.fn(), - appliedSnapshotMarkerRef: { current: { epoch: null, version: -1 } }, - closedTabTombstonesRef: { current: new Map() }, - setTerminalsLoaded: vi.fn(), - setActiveHandle: vi.fn(), - setActiveSessionTabId: vi.fn(), - setMarkdownDocs: vi.fn(), - setFileDocs: vi.fn(), - terminalGestureInputQueuesRef: { current: new Map() }, - terminalGestureInputInFlightRef: { current: new Set() }, - sessionTabActionSheetKeyboardHideSubRef: { current: null }, - sessionTabActionSheetRequestSeqRef: { current: 0 }, - initializedHandlesRef: { current: new Set() }, - terminalDiagnosticsRef: { current: { resetRoute: vi.fn() } }, - activeHandleRef: { current: null }, - activeSessionTabTypeRef: { current: null }, - pendingActiveSessionTabIdRef: { current: null }, - selectedSessionTabIdRef: { current: null }, - pendingActiveTerminalHandleRef: { current: null }, - pendingBrowserFocusPageIdRef: { current: null }, - pendingTerminalActivationAttemptRef: { current: null }, - initialSessionAutoCreateRef: { current: null }, - bufferedTerminalDraftState: { resetDrafts: vi.fn(), clearPendingRestorations: vi.fn() }, - clearPendingLiveInputCommit: vi.fn(), - clearDelayedActionTimers: vi.fn(), - showToast: vi.fn(), - clearTerminalCache: vi.fn(), - fetchTerminals: vi.fn(() => { - calls.push({ rpc: 'terminal.list', worktreeId }) - return terminals.promise - }), - ensureSessionTabs: vi.fn(() => { - calls.push({ rpc: 'session.tabs.list', worktreeId }) - return tabs.promise - }) - } - return { - scope: scope as unknown as MobileSessionKeyboardStateModel, - tabs, - terminals, - sendRequest, - activateCalls: () => - sendRequest.mock.calls.filter(([method]) => method === 'worktree.activate').length - } -} - -function StartupHarness({ - scope -}: { - scope: MobileSessionKeyboardStateModel -}): ReactElement | null { - useMobileSessionStartup(scope) - return null -} - -async function flush(): Promise { - await act(async () => { - await Promise.resolve() - await Promise.resolve() - await Promise.resolve() - }) -} - -describe('mobile session startup parallelism', () => { - let renderer: ReactTestRenderer | null = null - - beforeEach(() => { - vi.useFakeTimers({ shouldAdvanceTime: true }) - }) - - afterEach(() => { - act(() => renderer?.unmount()) - renderer = null - vi.useRealTimers() - }) - - it('puts session.tabs.list and terminal.list on the wire together', async () => { - const calls: StartupCall[] = [] - const { scope } = makeScope('wt-1', calls) - - await act(async () => { - renderer = create(createElement(StartupHarness, { scope })) - await Promise.resolve() - }) - await flush() - - // Neither deferred has settled, so both requests are in flight at the same moment. Under the - // old chain the second call could not have been made until the first resolved. - expect(calls).toEqual([ - { rpc: 'session.tabs.list', worktreeId: 'wt-1' }, - { rpc: 'terminal.list', worktreeId: 'wt-1' } - ]) - }) - - it('isolates each read so one rejection cannot strand the follow-up refreshes', async () => { - const calls: StartupCall[] = [] - const { scope, tabs, terminals } = makeScope('wt-1', calls) - - await act(async () => { - renderer = create(createElement(StartupHarness, { scope })) - await Promise.resolve() - }) - await flush() - - await act(async () => { - tabs.reject(new Error('tabs rejected')) - terminals.reject(new Error('terminals rejected')) - await Promise.resolve() - }) - await flush() - - await act(async () => { - vi.advanceTimersByTime(1600) - await Promise.resolve() - }) - // The 750 ms and 1500 ms follow-up refreshes still armed despite both rejections; an - // unguarded await would have thrown out of the startup block and armed neither. - expect(calls.filter((call) => call.rpc === 'terminal.list')).toHaveLength(3) - }) - - it('drops results that land after the route moved to another session', async () => { - const calls: StartupCall[] = [] - const first = makeScope('wt-1', calls) - const second = makeScope('wt-2', calls) - - await act(async () => { - renderer = create(createElement(StartupHarness, { scope: first.scope })) - await Promise.resolve() - }) - await flush() - - await act(async () => { - renderer?.update(createElement(StartupHarness, { scope: second.scope })) - await Promise.resolve() - }) - await flush() - - // The first session's reads land only now, after its effect was torn down. - await act(async () => { - first.tabs.resolve(undefined) - first.terminals.resolve(true) - await Promise.resolve() - }) - await flush() - await act(async () => { - vi.advanceTimersByTime(1600) - await Promise.resolve() - }) - - // Why: a stale settlement must not schedule refreshes for a worktree the route has left. - expect(calls.filter((call) => call.worktreeId === 'wt-1')).toHaveLength(2) - }) - - it('withholds worktree.activate until the compatibility verdict lands', async () => { - const calls: StartupCall[] = [] - const pending = makeScope('wt-1', calls, false) - - await act(async () => { - renderer = create(createElement(StartupHarness, { scope: pending.scope })) - await Promise.resolve() - }) - await flush() - - // Why: a desktop that omits protocolVersion evaluates as version 0 and IS blocked, so the - // routes that now mount pre-verdict must not mutate a host the gate is about to refuse. - expect(pending.activateCalls()).toBe(0) - // The reads are not held back with it; that is the whole point of mounting early. - expect(calls).toHaveLength(2) - }) - - it('activates once the verdict lands without re-issuing the reads', async () => { - const calls: StartupCall[] = [] - const pending = makeScope('wt-1', calls, false) - - await act(async () => { - renderer = create(createElement(StartupHarness, { scope: pending.scope })) - await Promise.resolve() - }) - await flush() - expect(pending.activateCalls()).toBe(0) - - // Same session, verdict now proven: only the activation effect may re-run. - const verified = { - ...(pending.scope as unknown as Record), - protocolVerified: true - } as unknown as MobileSessionKeyboardStateModel - await act(async () => { - renderer?.update(createElement(StartupHarness, { scope: verified })) - await Promise.resolve() - }) - await flush() - - expect(pending.activateCalls()).toBe(1) - expect(pending.sendRequest).toHaveBeenCalledWith('worktree.activate', { - worktree: 'id:wt-1', - notifyClients: false, - navigation: 'caller' - }) - expect(calls).toHaveLength(2) - }) - - it('discards both parallel results when the session changes mid-flight', async () => { - const calls: StartupCall[] = [] - const first = makeScope('wt-1', calls) - const second = makeScope('wt-2', calls) - - await act(async () => { - renderer = create(createElement(StartupHarness, { scope: first.scope })) - await Promise.resolve() - }) - await flush() - expect(calls.filter((call) => call.worktreeId === 'wt-1')).toHaveLength(2) - - await act(async () => { - renderer?.update(createElement(StartupHarness, { scope: second.scope })) - await Promise.resolve() - }) - await flush() - - // Tabs land late first, then terminals, so each is separately proven inert. - await act(async () => { - first.tabs.resolve(undefined) - await Promise.resolve() - }) - await flush() - await act(async () => { - vi.advanceTimersByTime(1600) - await Promise.resolve() - }) - expect(calls.filter((call) => call.worktreeId === 'wt-1')).toHaveLength(2) - - await act(async () => { - first.terminals.resolve(true) - await Promise.resolve() - }) - await flush() - await act(async () => { - vi.advanceTimersByTime(1600) - await Promise.resolve() - }) - expect(calls.filter((call) => call.worktreeId === 'wt-1')).toHaveLength(2) - }) -}) diff --git a/mobile/src/session/mobile-session-startup-source.test.ts b/mobile/src/session/mobile-session-startup-source.test.ts index d843d3d2d42..83b2021b95e 100644 --- a/mobile/src/session/mobile-session-startup-source.test.ts +++ b/mobile/src/session/mobile-session-startup-source.test.ts @@ -30,10 +30,6 @@ const autoCreateHookSource = readMobileSessionRouteSource( './use-initial-session-terminal-autocreate.ts' ) const foundationSource = readMobileSessionRouteSource('./use-mobile-session-foundation.ts') -const activeContentSource = readMobileSessionRouteSource('./MobileSessionActiveContent.tsx') -const subscriptionFoundationSource = readMobileSessionRouteSource( - './use-mobile-session-terminal-subscription-foundation.ts' -) const terminalRuntimeSource = readMobileSessionRouteSource( './use-mobile-session-terminal-runtime.ts' ) @@ -151,72 +147,35 @@ describe('mobile session startup', () => { ) }) - // Was: one effect that awaited tabs, then terminals, and fired worktree.activate alongside them. - // The reads are now concurrent and unblocked, while the activation moved to its own effect that - // waits for the compatibility verdict, because it writes host state. - it('loads session tabs and terminals concurrently, ahead of any desktop activation', () => { - const readEffect = sliceBetween( + it('loads session tabs without waiting for desktop activation', () => { + const startupEffect = sliceBetween( 'void (async () => {', 'return () => {\n disposed = true', startupSource ) - expect(readEffect).toContain( - 'await Promise.all([\n ensureSessionTabs().catch(() => null),\n fetchTerminals({ allowEmptyLoaded: false }).catch(() => false)\n ])' + expect(startupEffect).toContain("void client\n .sendRequest('worktree.activate'") + expect(startupEffect).toContain("if (client && created !== '1' && !isFloatingWorkspaceRoute)") + expect(startupEffect).toContain("if (client && created === '1' && !isFloatingWorkspaceRoute)") + expect(startupEffect).toContain('notifyClients: false') + expect(startupEffect).toContain("navigation: 'caller'") + expect(startupEffect).not.toContain("await client\n .sendRequest('worktree.activate'") + expect(startupEffect.indexOf("sendRequest('worktree.activate'")).toBeLessThan( + startupEffect.indexOf('await ensureSessionTabs()') ) - // The reads must not wait on the verdict; that is the point of mounting under the gate. - expect(readEffect).not.toContain('protocolVerified') - expect(readEffect).not.toContain('worktree.activate') - expect(startupSource).toContain('}, [connState, fetchTerminals, ensureSessionTabs])') + expect(startupEffect).toContain('headlessActivationNeedsHostRenderer(response.result)') + expect(startupEffect).toContain("showToast('Open Orca on the host to wake sleeping agents.'") }) - it('holds worktree.activate until the compatibility verdict lands', () => { - const activationEffect = sliceBetween( - "if (connState !== 'connected' || !client || !protocolVerified || isFloatingWorkspaceRoute) {", - 'return () => {\n disposed = true', - startupSource.slice(startupSource.indexOf('worktree.activate') - 2000) - ) - - // Why: a desktop that omits protocolVersion reads as version 0 and IS blocked, so mounting - // this route pre-verdict must not let it mutate a host the gate is about to refuse. - expect(activationEffect).toContain("sendRequest('worktree.activate'") - expect(activationEffect).toContain('notifyClients: false') - expect(activationEffect).toContain("navigation: 'caller'") - expect(activationEffect).toContain("if (created !== '1') {") - expect(activationEffect).toContain('headlessActivationNeedsHostRenderer(response.result)') - expect(activationEffect).toContain("showToast('Open Orca on the host to wake sleeping agents.'") - // The only worktree.activate calls in the route are the two this gated effect owns. - expect(startupSource.split("sendRequest('worktree.activate'")).toHaveLength(2) - expect(startupSource).toContain(' protocolVerified,\n showToast,\n worktreeId\n ])') - }) - - // Was: this route ran its own retrying status.get. The gate above every /h/ route already - // holds that answer, so the second request is gone and the gates read it instead. - it('fails runtime capability gates closed until the shared status.get is proven', () => { + it('fails runtime capability gates closed before probing a replacement client', () => { const capabilityEffect = sliceBetween( 'const hostQueryReplyInputSupportedRef = useRef(false)', 'return {\n consumeAcceptedSessionTabs', tabReconciliationSource ) + const probeStart = capabilityEffect.indexOf('startRuntimeCapabilityProbe(client,') - expect(tabReconciliationSource).not.toContain('startRuntimeCapabilityProbe') - expect(tabReconciliationSource).not.toContain('useHostProtocolGates') - // One read of the gate for the whole route, taken in the foundation and passed down. - expect(foundationSource).toContain( - 'const { compatVerdict, compatVerified, hostCapabilities, statusPending } = useHostProtocolGates()' - ) - // Settled is not passing, and passing-by-fallback is not answered. The write gate reads all - // three, so a host that never answered status.get cannot be mistaken for a verified one. - expect(foundationSource).toContain( - "const protocolVerified = !statusPending && compatVerified && compatVerdict.kind === 'ok'" - ) - expect(capabilityEffect).toContain( - "if (!client || connState !== 'connected' || !protocolVerified) {" - ) - const readStart = capabilityEffect.indexOf( - "setBrowserScreencastSupported(hostCapabilities.includes('browser.screencast.v1'))" - ) - expect(readStart).toBeGreaterThanOrEqual(0) + expect(probeStart).toBeGreaterThanOrEqual(0) for (const reset of [ 'setBrowserScreencastSupported(null)', 'setAgentSessionHistorySupported(null)', @@ -226,7 +185,7 @@ describe('mobile session startup', () => { ]) { const resetIndex = capabilityEffect.lastIndexOf(reset) expect(resetIndex).toBeGreaterThanOrEqual(0) - expect(resetIndex).toBeLessThan(readStart) + expect(resetIndex).toBeLessThan(probeStart) } }) @@ -331,43 +290,4 @@ describe('mobile session startup', () => { expect(source).toContain('onPendingTerminalRecoveryParked: setParkedPendingTerminalContext') expect(source).toContain('retryPendingTerminalRecovery()') }) - - it('boots the terminal engine while the startup reads are still in flight', () => { - // Why: the loading and pending-terminal states are exactly the window in which the startup - // RPCs are outstanding, so the engine loads there rather than after terminal.list answers. - const loadingBranch = sliceBetween( - "return reconnectViewState.kind === 'reconnecting-with-cache' || showLoadingState ? (", - ') : showEmptyState ? (', - activeContentSource - ) - const prewarmElement = - '' - expect(loadingBranch).toContain(prewarmElement) - expect(loadingBranch).toContain('') - - const pendingBranch = sliceBetween( - ') : activePendingTerminalTab ? (', - ') : (\n (') - expect(activeContentSource.indexOf(' (') - ) - }) - - it('refuses a pre-warm viewport measured before the frame had a height', () => { - const measure = sliceBetween( - 'const measurePrewarmViewport = useCallback(', - ' return {\n getTerminalRef', - subscriptionFoundationSource - ) - expect(measure).toContain('if (viewportMeasuredRef.current || frameHeight <= 0) {') - expect(measure).toContain('await engine.measureFitDimensions(frameHeight)') - // Why: the latch is re-checked after the await so a real pane that measured first wins. - expect(measure).toContain('if (dims && !viewportMeasuredRef.current) {') - }) }) diff --git a/mobile/src/session/mobile-session-tab-strip-entries.ts b/mobile/src/session/mobile-session-tab-strip-entries.ts deleted file mode 100644 index 5f4569403b0..00000000000 --- a/mobile/src/session/mobile-session-tab-strip-entries.ts +++ /dev/null @@ -1,116 +0,0 @@ -import { TUI_AGENT_DISPLAY_NAMES } from '../../../src/shared/tui-agent-display-names' -import type { MobileSessionTab, MobileSessionTabType } from './mobile-session-route-types' -import { - getMobileSessionTabTitle, - resolveMobileTerminalTabAgentId -} from './mobile-terminal-tab-agent' - -/** - * The only session-tab fields the tab strip draws. Everything else the live tab carries (unsent - * launch drafts, absolute file paths, browser URLs, agent session ids) stays on the wire. - */ -export type MobileSessionTabStripEntry = { - id: string - type: MobileSessionTabType - title: string - agentId: string | null -} - -export type MobileSessionTabStripPreview = { - tabs: readonly MobileSessionTabStripEntry[] - activeTabId: string | null -} - -export type MobileSessionTabStripRow = { - entry: MobileSessionTabStripEntry - isActive: boolean - /** null on a preview row: switching to that tab needs a live connection. */ - tab: MobileSessionTab | null -} - -export function toMobileSessionTabStripEntry(tab: MobileSessionTab): MobileSessionTabStripEntry { - return { - id: tab.id, - type: tab.type, - title: getMobileSessionTabTitle(tab), - agentId: - tab.type === 'agent-session' - ? tab.agent - : tab.type === 'terminal' - ? resolveMobileTerminalTabAgentId(tab) - : null - } -} - -/** - * Every tab type the strip knows how to draw. A stored entry naming anything else is dropped - * rather than trusted, so a type added later fails closed: its rows go missing from the preview - * instead of carrying an unreviewed title into storage. - */ -const drawableTabTypes = new Set([ - 'terminal', - 'markdown', - 'file', - 'browser', - 'agent-session' -] satisfies readonly MobileSessionTabType[]) - -export function isDrawableTabStripType(type: string): type is MobileSessionTabType { - return drawableTabTypes.has(type) -} - -const agentDisplayNames: Readonly> = TUI_AGENT_DISPLAY_NAMES - -/** - * The title a strip entry may be written to disk under. - * - * A terminal's title is whatever the shell last set, which is routinely the command line — - * `psql postgres://user:password@host/db`, `curl -H "Authorization: Bearer ..."`. None of that - * belongs in plaintext storage, and a browser tab's page title is no better. Both collapse to a - * fixed label, so what survives is the shape of the strip, not its contents. A resolved agent - * still names itself, because that lookup is a closed enum: an unrecognised id yields the - * generic label rather than passing text through. - */ -export function getPersistableTabStripTitle( - entry: Pick -): string { - if (entry.type === 'terminal') { - const agentLabel = entry.agentId === null ? undefined : agentDisplayNames[entry.agentId] - return agentLabel ?? 'Terminal' - } - if (entry.type === 'browser') { - return 'Browser' - } - return entry.title -} - -export function toMobileSessionTabStripPreview( - tabs: readonly MobileSessionTab[], - activeTabId: string | null -): MobileSessionTabStripPreview { - return { tabs: tabs.map(toMobileSessionTabStripEntry), activeTabId } -} - -/** - * Rows for the header strip. Live tabs always win; the preview only fills a strip that has no - * live rows yet, and its ids are the live ids, so the swap reuses the same React keys. - */ -export function getMobileSessionTabStripRows(args: { - liveTabs: readonly MobileSessionTab[] - activeSessionTabId: string | null - preview: MobileSessionTabStripPreview | null -}): MobileSessionTabStripRow[] { - const { liveTabs, activeSessionTabId, preview } = args - if (liveTabs.length > 0 || !preview) { - return liveTabs.map((tab) => ({ - entry: toMobileSessionTabStripEntry(tab), - isActive: tab.id === activeSessionTabId, - tab - })) - } - return preview.tabs.map((entry) => ({ - entry, - isActive: entry.id === preview.activeTabId, - tab: null - })) -} diff --git a/mobile/src/session/terminal-prewarm-frame-geometry.test.ts b/mobile/src/session/terminal-prewarm-frame-geometry.test.ts deleted file mode 100644 index 547c94104a5..00000000000 --- a/mobile/src/session/terminal-prewarm-frame-geometry.test.ts +++ /dev/null @@ -1,181 +0,0 @@ -import { createElement } from 'react' -import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, describe, expect, it, vi } from 'vitest' -import type { TerminalWebViewHandle } from '../terminal/terminal-webview-contract' -import { readMobileSessionRouteSource } from './mobile-session-route-source-family.test-support' - -type StyleLayer = { top?: number } - -// The applied top offset, read off the rendered pane rather than assumed. -function appliedTopOffset(style: unknown): number { - const layers = (Array.isArray(style) ? style : [style]) as (StyleLayer | null | undefined)[] - return layers.reduce( - (top, layer) => (typeof layer?.top === 'number' ? layer.top : top), - 0 - ) -} - -const engine = vi.hoisted(() => ({ - init: vi.fn((_cols: number, _rows: number) => {}), - awaitReady: vi.fn(async () => {}), - measureFitDimensions: vi.fn(async (_containerHeight?: number) => ({ cols: 100, rows: 40 })), - onWebReady: null as (() => void) | null -})) - -vi.mock('react-native', () => ({ - StyleSheet: { - create: (styles: T) => styles, - absoluteFillObject: { position: 'absolute', top: 0, left: 0, right: 0, bottom: 0 } - }, - View: 'View' -})) - -vi.mock('../terminal/TerminalWebView', async () => { - const { forwardRef, useImperativeHandle } = await import('react') - return { - TerminalWebView: forwardRef void }>( - function MockTerminalWebView(props, ref) { - engine.onWebReady = props.onWebReady ?? null - useImperativeHandle(ref, () => engine as unknown as TerminalWebViewHandle, []) - return createElement('MockTerminalWebView') - } - ) - } -}) - -import { TerminalEnginePrewarm } from './TerminalEnginePrewarm' -import { - MOBILE_SESSION_TAB_BAR_BORDER_WIDTH, - MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT, - MOBILE_SESSION_TAB_BAR_HEIGHT, - mobileSessionFrameStyles -} from './mobile-session-frame-styles' - -// The box the session content row occupies. Both states below live in it, so it is the one -// number the two frame heights are derived from. -const CONTENT_ROW_HEIGHT = 700 - -// The bar's rendered height, derived from the styles the header actually mounts rather than from -// the constant the pre-warm consumes — otherwise the comparison below would just restate itself. -// React Native sizes a row with no explicit height to its tallest child and puts the border -// outside that, so this is max(children) + border. -function renderedTabBarHeight(): number { - const row = mobileSessionFrameStyles.tabBar as { height?: number; borderTopWidth: number } - // An explicit height here would be border-box and would shrink the row below its children. - expect(row.height).toBeUndefined() - const tallestChild = Math.max( - mobileSessionFrameStyles.tabScroll.maxHeight, - mobileSessionFrameStyles.tab.minHeight, - mobileSessionFrameStyles.newTerminalButton.height, - mobileSessionFrameStyles.tabActionDivider.height - ) - return tallestChild + row.borderTopWidth -} - -// What the first real pane gets once its tab exists and the bar mounts above it. -function firstPaneFrameHeight(): number { - return CONTENT_ROW_HEIGHT - renderedTabBarHeight() -} -const headerSource = readMobileSessionRouteSource('./MobileSessionHeader.tsx') -const activeContentSource = readMobileSessionRouteSource('./MobileSessionActiveContent.tsx') - -// Reproduces React Native's absolute-fill layout: a box pinned to every edge of its parent with -// a top offset gets exactly that much less height. The offset is read off the component, never -// assumed, so a pre-warm that stopped reserving the bar would report the taller box here. -function measuredPrewarmHeight(reservedTabBarHeight: number): number { - let renderer: ReactTestRenderer | null = null - act(() => { - renderer = create( - createElement(TerminalEnginePrewarm, { reservedTabBarHeight, onEngineMeasured: () => {} }) - ) - }) - const created = renderer as unknown as ReactTestRenderer - const applied = appliedTopOffset(created.root.findAllByType('View')[0]?.props.style) - act(() => created.unmount()) - return CONTENT_ROW_HEIGHT - applied -} - -afterEach(() => { - engine.measureFitDimensions.mockClear() - engine.onWebReady = null -}) - -describe('terminal pre-warm frame geometry', () => { - it('states the height the bar actually renders at', () => { - // The constant is what the pre-warm reserves, so it has to equal what the header mounts. - // Deriving the latter from the styles catches the border-box trap: pinning an explicit - // height on the row would render it a pixel short of this sum and drift a whole row. - expect(renderedTabBarHeight()).toBe(MOBILE_SESSION_TAB_BAR_HEIGHT) - expect(MOBILE_SESSION_TAB_BAR_HEIGHT).toBe( - MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT + MOBILE_SESSION_TAB_BAR_BORDER_WIDTH - ) - expect(mobileSessionFrameStyles.tabBar.borderTopWidth).toBe(MOBILE_SESSION_TAB_BAR_BORDER_WIDTH) - // Every child is pinned to the content height, so nothing can grow the row unnoticed. - expect(mobileSessionFrameStyles.tabScroll.maxHeight).toBe(MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT) - expect(mobileSessionFrameStyles.tab.minHeight).toBe(MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT) - expect(mobileSessionFrameStyles.newTerminalButton.height).toBe( - MOBILE_SESSION_TAB_BAR_CONTENT_HEIGHT - ) - }) - - it('mounts the tab bar only once a tab is visible, which is what shortens the pane', () => { - expect(headerSource).toContain( - '{tabStripRows.length > 0 && (\n ' - ) - // So the reservation has to be the exact complement of that condition, read off the same rows - // the header gates on (live tabs or the cached reconnect preview) rather than a proxy for it. - expect(activeContentSource).toContain( - 'const prewarmReservedTabBarHeight = tabStripRows.length > 0 ? 0 : MOBILE_SESSION_TAB_BAR_HEIGHT' - ) - }) - - it('measures the same frame height the first real pane will get', () => { - // Loading: no visible tab, so no tab bar, so the content row is all the pre-warm's to fill, - // minus whatever it reserves. Loaded: the first terminal produces a tab, the bar mounts, and - // the pane gets what is left. The right side is derived from the header's own styles. - expect(measuredPrewarmHeight(MOBILE_SESSION_TAB_BAR_HEIGHT)).toBe(firstPaneFrameHeight()) - }) - - it('would latch a taller frame than the pane if the bar were not reserved', () => { - // Guards the fix rather than the code: without the reservation the pre-warm measures the - // pre-tab-bar box, and every row of that difference is a row the host never had. - const unreserved = measuredPrewarmHeight(0) - expect(unreserved).toBe(CONTENT_ROW_HEIGHT) - expect(unreserved - firstPaneFrameHeight()).toBe(renderedTabBarHeight()) - }) - - it('hands the engine the reserved height, so no refit is owed after the first subscribe', async () => { - let measuredWith: number | null = null - let renderer: ReactTestRenderer | null = null - act(() => { - renderer = create( - createElement(TerminalEnginePrewarm, { - reservedTabBarHeight: MOBILE_SESSION_TAB_BAR_HEIGHT, - textScale: 1, - onEngineMeasured: (_ref: unknown, frameHeight: number) => { - measuredWith = frameHeight - } - }) - ) - }) - const created = renderer as unknown as ReactTestRenderer - const pane = created.root.findAllByType('View')[0] - const applied = appliedTopOffset(pane?.props.style) - act(() => { - pane?.props.onLayout({ - nativeEvent: { layout: { x: 0, y: 0, width: 390, height: CONTENT_ROW_HEIGHT - applied } } - }) - }) - act(() => { - engine.onWebReady?.() - }) - // The handoff waits on the engine's ready promise, so let those microtasks land. - await act(async () => {}) - - // The height the latched viewport is computed from equals the real pane's frame height, so - // the frame-height refit re-measures the same cols/rows and returns before it would send - // terminal.updateViewport (see the prev-dims guard in terminal-viewport-refit.ts). - expect(measuredWith).toBe(firstPaneFrameHeight()) - act(() => created.unmount()) - }) -}) diff --git a/mobile/src/session/terminal-prewarm-refit-debt.test.ts b/mobile/src/session/terminal-prewarm-refit-debt.test.ts deleted file mode 100644 index 57a3567cffa..00000000000 --- a/mobile/src/session/terminal-prewarm-refit-debt.test.ts +++ /dev/null @@ -1,147 +0,0 @@ -import { createElement, useRef, type ReactElement } from 'react' -import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import type { RpcClient } from '../transport/rpc-client' -import type { TerminalWebViewHandle } from '../terminal/terminal-webview-contract' - -vi.mock('react-native', () => ({ - AppState: { currentState: 'active', addEventListener: () => ({ remove: () => {} }) }, - Platform: { OS: 'android' }, - StyleSheet: { - create: (styles: T) => styles, - absoluteFillObject: { position: 'absolute', top: 0, left: 0, right: 0, bottom: 0 }, - hairlineWidth: 1 - }, - useWindowDimensions: () => ({ width: 390, height: 844 }), - View: 'View' -})) - -import { useTerminalViewportRefit } from '../terminal/terminal-viewport-refit' -import { MOBILE_SESSION_TAB_BAR_HEIGHT } from './mobile-session-frame-styles' - -const CONTENT_ROW_HEIGHT = 700 -const CELL_HEIGHT = 17 -const HANDLE = 'term-1' -const REFIT_DEBOUNCE_MS = 150 - -// Stands in for the WebView's fit: the taller the box it is handed, the more rows it reports. -// This is what turns a frame that is one tab bar too tall into a row count the host never had. -function fitDimensions(containerHeight: number): { cols: number; rows: number } { - return { cols: 100, rows: Math.floor(containerHeight / CELL_HEIGHT) } -} - -type ColdOpenResult = { - updateViewportCalls: number - resubscribes: number - latchedRows: number -} - -// Replays a single-terminal cold open: the pre-warm measured `prewarmFrameHeight` and latched it, -// the first pane subscribed with those dims, and only then does the real frame report its layout. -async function runSingleTerminalColdOpen(prewarmFrameHeight: number): Promise { - const firstPaneFrameHeight = CONTENT_ROW_HEIGHT - MOBILE_SESSION_TAB_BAR_HEIGHT - const sendRequest = vi.fn(async () => ({ ok: true, result: { updated: true, applied: true } })) - const client = { - sendRequest, - updateTerminalSubscriptionViewport: vi.fn() - } as unknown as RpcClient - const engine = { - measureFitDimensions: vi.fn(async (containerHeight?: number) => - fitDimensions(containerHeight ?? 0) - ), - reflow: vi.fn() - } as unknown as TerminalWebViewHandle - const subscribeToTerminal = vi.fn() - const unsubscribeTerminal = vi.fn() - const viewport = { current: fitDimensions(prewarmFrameHeight) as { cols: number; rows: number } } - const viewportMeasured = { current: true } - // The real pane's frame, reported by its onLayout once the tab bar has mounted. - const frameHeight = { current: firstPaneFrameHeight } - - let notify: ((height: number) => void) | null = null - function RefitHarness(): ReactElement | null { - const terminalRefs = useRef(new Map([[HANDLE, engine]])) - const { notifyTerminalFrameHeight } = useTerminalViewportRefit({ - activeHandleRef: useRef(HANDLE), - terminalRefs, - terminalFrameHeightRef: frameHeight, - viewportRef: viewport, - viewportMeasuredRef: viewportMeasured, - nativeChatCoveredRef: useRef(false), - clientRef: useRef(client), - deviceTokenRef: useRef('device-1'), - initializedHandlesRef: useRef(new Set([HANDLE])), - connState: 'connected', - // One terminal, so the tab-strip corrector is not armed — this is the case that used to - // fall through to the frame-height reducer and pay for the mis-measurement. - tabStripVisible: false, - textScale: 1, - terminalFrameWidth: 390, - unsubscribeTerminal, - subscribeToTerminal - }) - notify = notifyTerminalFrameHeight - return null - } - - let renderer: ReactTestRenderer | null = null - await act(async () => { - renderer = create(createElement(RefitHarness)) - }) - await act(async () => { - notify?.(firstPaneFrameHeight) - }) - // Why drain microtasks between ticks and before unmount: the refit measures and sends inside an - // async block that bails once disposedRef flips, so tearing down early would fake a clean run. - await act(async () => { - vi.advanceTimersByTime(REFIT_DEBOUNCE_MS + 1) - for (let i = 0; i < 10; i += 1) { - await Promise.resolve() - } - }) - await act(async () => { - vi.advanceTimersByTime(REFIT_DEBOUNCE_MS + 1) - for (let i = 0; i < 10; i += 1) { - await Promise.resolve() - } - }) - act(() => (renderer as unknown as ReactTestRenderer).unmount()) - - return { - updateViewportCalls: sendRequest.mock.calls.filter( - ([method]) => method === 'terminal.updateViewport' - ).length, - resubscribes: subscribeToTerminal.mock.calls.length, - latchedRows: viewport.current.rows - } -} - -describe('terminal pre-warm refit debt', () => { - beforeEach(() => { - vi.useFakeTimers({ shouldAdvanceTime: true }) - }) - - afterEach(() => { - vi.useRealTimers() - }) - - it('owes the host nothing after the first subscribe when the pre-warm reserved the tab bar', async () => { - const reserved = CONTENT_ROW_HEIGHT - MOBILE_SESSION_TAB_BAR_HEIGHT - const result = await runSingleTerminalColdOpen(reserved) - - expect(result.updateViewportCalls).toBe(0) - expect(result.resubscribes).toBe(0) - expect(result.latchedRows).toBe(fitDimensions(reserved).rows) - }) - - it('pays a terminal.updateViewport round trip if the pre-warm measured the pre-tab-bar box', async () => { - // Guards the fix, not the code: this is the frame the pre-warm saw before it reserved the bar. - const result = await runSingleTerminalColdOpen(CONTENT_ROW_HEIGHT) - - expect(result.updateViewportCalls).toBe(1) - // And the rows it had to correct are rows the host was told about and never had. - expect(fitDimensions(CONTENT_ROW_HEIGHT).rows).toBeGreaterThan( - fitDimensions(CONTENT_ROW_HEIGHT - MOBILE_SESSION_TAB_BAR_HEIGHT).rows - ) - }) -}) diff --git a/mobile/src/session/use-mobile-session-controller.ts b/mobile/src/session/use-mobile-session-controller.ts index b2427f806c2..f188b30b17a 100644 --- a/mobile/src/session/use-mobile-session-controller.ts +++ b/mobile/src/session/use-mobile-session-controller.ts @@ -27,7 +27,6 @@ import { useMobileSessionTerminalCreateActions } from './use-mobile-session-term import { useMobileSessionContentCreateActions } from './use-mobile-session-content-create-actions' import { useMobileSessionCloseActions } from './use-mobile-session-close-actions' import { useMobileSessionBulkClose } from './use-mobile-session-bulk-close' -import { useMobileSessionTabStripCache } from './use-mobile-session-tab-strip-cache' import { useMobileSessionPresentation } from './use-mobile-session-presentation' import { useMobileSessionPanelRouteActions } from './use-mobile-session-panel-route-actions' @@ -114,8 +113,7 @@ export function useMobileSessionController() { useMobileSessionCloseActions(contentCreateActions) ) const bulkClose = Object.assign(closeActions, useMobileSessionBulkClose(closeActions)) - const tabStripCache = Object.assign(bulkClose, useMobileSessionTabStripCache(bulkClose)) - const presentation = Object.assign(tabStripCache, useMobileSessionPresentation(tabStripCache)) + const presentation = Object.assign(bulkClose, useMobileSessionPresentation(bulkClose)) const panelRouteActions = Object.assign( presentation, useMobileSessionPanelRouteActions(presentation) diff --git a/mobile/src/session/use-mobile-session-foundation.ts b/mobile/src/session/use-mobile-session-foundation.ts index 6f9e0e849ca..fa2f9607bbc 100644 --- a/mobile/src/session/use-mobile-session-foundation.ts +++ b/mobile/src/session/use-mobile-session-foundation.ts @@ -14,7 +14,6 @@ import { isFloatingWorkspaceWorktreeId } from './floating-workspace' import { useLiveWorktreeName } from './use-live-worktree-name' import { useMissingWorktreeBounce } from './use-missing-worktree-bounce' import { hostRouteWithNotice } from '../host-route-notice' -import { useHostProtocolGates } from '../components/HostProtocolGate' export function useMobileSessionFoundation() { const { @@ -37,14 +36,6 @@ export function useMobileSessionFoundation() { const insets = useSafeAreaInsets() // Why: shared client per host owned by RpcClientProvider (docs/mobile-shared-client-per-host.md). const { client, clientId, state: connState } = useHostClient(hostId) - // Why: HostProtocolGate holds this connection's single status.get. Reading it here gives the - // whole route one source for host capabilities and for whether the compatibility verdict has - // landed — the routes now mount while it is still in flight, so "not yet known" is a real state. - const { compatVerdict, compatVerified, hostCapabilities, statusPending } = useHostProtocolGates() - // Why all three: a settled verdict is not necessarily a passing one, and a settled *passing* - // verdict is not necessarily an answered one — a host that cannot answer status.get fails open - // to `ok` so navigation still works. Writes read this flag, so they wait for a real reply. - const protocolVerified = !statusPending && compatVerified && compatVerdict.kind === 'ok' const reconnectAttempts = useReconnectAttempt(hostId) const lastConnectedAt = useLastConnectedAt(hostId) const forceReconnectHost = useForceReconnect() @@ -107,8 +98,6 @@ export function useMobileSessionFoundation() { client, clientId, connState, - hostCapabilities, - protocolVerified, reconnectAttempts, lastConnectedAt, forceReconnectHost, diff --git a/mobile/src/session/use-mobile-session-presentation.ts b/mobile/src/session/use-mobile-session-presentation.ts index e43b59cabef..2565f729940 100644 --- a/mobile/src/session/use-mobile-session-presentation.ts +++ b/mobile/src/session/use-mobile-session-presentation.ts @@ -3,11 +3,9 @@ import { classifyConnection, verdictDisplayLabel } from '../transport/connection import { computeActiveTerminalKeyboardLift } from '../terminal/terminal-keyboard-avoidance-lift' import { useInitialSessionTerminalAutoCreate } from './use-initial-session-terminal-autocreate' import { MOBILE_SESSION_STATUS_LABELS } from './mobile-session-route-helpers' -import { selectMobileSessionReconnectViewState } from './mobile-session-reconnect-view-state' -import { getMobileSessionTabStripRows } from './mobile-session-tab-strip-entries' -import type { MobileSessionTabStripCacheModel } from './use-mobile-session-tab-strip-cache' +import type { MobileSessionBulkCloseModel } from './use-mobile-session-bulk-close' -export function useMobileSessionPresentation(scope: MobileSessionTabStripCacheModel) { +export function useMobileSessionPresentation(scope: MobileSessionBulkCloseModel) { const { created, worktreeId, @@ -26,8 +24,6 @@ export function useMobileSessionPresentation(scope: MobileSessionTabStripCacheMo terminalKeyboardMetrics, toastOpacityRef, hostEndpoint, - activeSessionTabId, - cachedTabStrip, initialSessionAutoCreateRef, terminalFrameHeightRef, handleCreateTerminal, @@ -62,23 +58,6 @@ export function useMobileSessionPresentation(scope: MobileSessionTabStripCacheMo const showConnectionRetry = connectionVerdict.kind === 'warning' || connectionVerdict.kind === 'unreachable' - // Why: a reconnect to a workspace this phone has already drawn should re-draw it, not blank - // the screen while the RPCs land. See mobile-session-reconnect-view-state. - const reconnectViewState = selectMobileSessionReconnectViewState({ - connState, - verdictKind: connectionVerdict.kind, - terminalsLoaded, - liveTabCount: visibleTabs.length, - activeHandle, - cachedPreview: cachedTabStrip - }) - const tabStripRows = getMobileSessionTabStripRows({ - liveTabs: visibleTabs, - activeSessionTabId, - preview: - reconnectViewState.kind === 'reconnecting-with-cache' ? reconnectViewState.preview : null - }) - const terminalSummary = connState === 'connected' ? showLoadingState @@ -109,8 +88,6 @@ export function useMobileSessionPresentation(scope: MobileSessionTabStripCacheMo return { showLoadingState, showEmptyState, - reconnectViewState, - tabStripRows, connectionVerdict, showConnectionRetry, terminalSummary, @@ -120,5 +97,5 @@ export function useMobileSessionPresentation(scope: MobileSessionTabStripCacheMo } } -export type MobileSessionPresentationModel = MobileSessionTabStripCacheModel & +export type MobileSessionPresentationModel = MobileSessionBulkCloseModel & ReturnType diff --git a/mobile/src/session/use-mobile-session-startup.ts b/mobile/src/session/use-mobile-session-startup.ts index 3c90433e12e..f33d081f2cc 100644 --- a/mobile/src/session/use-mobile-session-startup.ts +++ b/mobile/src/session/use-mobile-session-startup.ts @@ -12,7 +12,6 @@ export function useMobileSessionStartup(scope: MobileSessionKeyboardStateModel) isFloatingWorkspaceRoute, connState, client, - protocolVerified, setTerminals, terminalsRef, setSessionTabs, @@ -96,8 +95,6 @@ export function useMobileSessionStartup(scope: MobileSessionKeyboardStateModel) worktreeId ]) - // Reads only. They carry no side effect on the host, so they do not wait on the compatibility - // verdict — that is the whole point of mounting this route while status.get is still in flight. // Every setTimeout goes through addTimer into `timers`, which the returned cleanup clears. // react-doctor-disable-next-line react-doctor/effect-needs-cleanup useEffect(() => { @@ -119,81 +116,58 @@ export function useMobileSessionStartup(scope: MobileSessionKeyboardStateModel) timers.push(setTimeout(fn, ms)) } void (async () => { - // Why: session.tabs.list and terminal.list are independent reads, so issue both now and - // wait for the pair. Serialising them cost a full extra round trip before the first - // terminal could paint, which on a far relay cell is seconds, not milliseconds. Each - // call keeps its own catch so one rejection cannot strand the other's follow-up refreshes. - await Promise.all([ - ensureSessionTabs().catch(() => null), - fetchTerminals({ allowEmptyLoaded: false }).catch(() => false) - ]) + const reportActivationOutcome = (response: RpcSuccess | null): void => { + if (!disposed && response && headlessActivationNeedsHostRenderer(response.result)) { + showToast('Open Orca on the host to wake sleeping agents.', 3000) + } + } + if (client && created !== '1' && !isFloatingWorkspaceRoute) { + // Why: hydrate host-owned tabs without pulling other paired clients (esp. desktop) into this worktree. + void client + .sendRequest('worktree.activate', { + worktree: `id:${worktreeId}`, + notifyClients: false, + navigation: 'caller' + }) + .then((response) => reportActivationOutcome(response.ok ? response : null)) + .catch(() => null) + } + if (disposed) { + return + } + await ensureSessionTabs().catch(() => null) + if (disposed) { + return + } + await fetchTerminals({ allowEmptyLoaded: false }) if (disposed) { return } addTimer(() => void fetchTerminals({ allowEmptyLoaded: false }), 750) addTimer(() => void fetchTerminals({ allowEmptyLoaded: true }), 1500) - })() - return () => { - disposed = true - for (const t of timers) { - clearTimeout(t) - } - } - // Why no client/worktreeId here: both reads are useCallbacks that already list them, so a - // host or worktree change replaces their identity and re-runs this effect with them. - }, [connState, fetchTerminals, ensureSessionTabs]) - - // worktree.activate writes host state, so unlike the reads above it waits for the compatibility - // verdict. A missing protocolVersion reads as 0 and is blocked, so "pending" is not a formality: - // mounting early must not let this route mutate a host the gate is about to refuse. - // Every setTimeout goes through addTimer into `timers`, which the returned cleanup clears. - // react-doctor-disable-next-line react-doctor/effect-needs-cleanup - useEffect(() => { - if (connState !== 'connected' || !client || !protocolVerified || isFloatingWorkspaceRoute) { - return - } - let disposed = false - const timers: ReturnType[] = [] - function addTimer(fn: () => void, ms: number) { - if (disposed) { - return - } - timers.push(setTimeout(fn, ms)) - } - const activateWorktree = () => - client - .sendRequest('worktree.activate', { - worktree: `id:${worktreeId}`, - notifyClients: false, - navigation: 'caller' - }) - .catch(() => null) - const reportActivationOutcome = (response: RpcSuccess | null): void => { - if (!disposed && response && headlessActivationNeedsHostRenderer(response.result)) { - showToast('Open Orca on the host to wake sleeping agents.', 3000) - } - } - if (created !== '1') { - // Why: hydrate host-owned tabs without pulling other paired clients (esp. desktop) into this worktree. - void activateWorktree().then((response) => - reportActivationOutcome(response?.ok ? response : null) - ) - } else { - addTimer(() => { - if (activeHandleRef.current) { - return - } - void (async () => { - const activationResponse = await activateWorktree() - reportActivationOutcome(activationResponse?.ok ? activationResponse : null) - if (disposed) { + if (client && created === '1' && !isFloatingWorkspaceRoute) { + addTimer(() => { + if (activeHandleRef.current) { return } - await fetchTerminals({ allowEmptyLoaded: true }) - addTimer(() => void fetchTerminals({ allowEmptyLoaded: true }), 750) - })() - }, 1800) - } + void (async () => { + const activationResponse = await client + .sendRequest('worktree.activate', { + worktree: `id:${worktreeId}`, + notifyClients: false, + navigation: 'caller' + }) + .catch(() => null) + reportActivationOutcome(activationResponse?.ok ? activationResponse : null) + if (disposed) { + return + } + await fetchTerminals({ allowEmptyLoaded: true }) + addTimer(() => void fetchTerminals({ allowEmptyLoaded: true }), 750) + })() + }, 1800) + } + })() return () => { disposed = true for (const t of timers) { @@ -205,8 +179,8 @@ export function useMobileSessionStartup(scope: MobileSessionKeyboardStateModel) connState, created, fetchTerminals, + ensureSessionTabs, isFloatingWorkspaceRoute, - protocolVerified, showToast, worktreeId ]) diff --git a/mobile/src/session/use-mobile-session-tab-reconciliation.ts b/mobile/src/session/use-mobile-session-tab-reconciliation.ts index 22e57944a40..be4641dd297 100644 --- a/mobile/src/session/use-mobile-session-tab-reconciliation.ts +++ b/mobile/src/session/use-mobile-session-tab-reconciliation.ts @@ -1,4 +1,5 @@ import { useEffect, useRef, useCallback, useMemo, useState } from 'react' +import { startRuntimeCapabilityProbe } from '../transport/runtime-capability-probe' import { supportsMobileQuickCommands } from '../terminal/quick-commands' import { MOBILE_AI_VAULT_CAPABILITY } from '../agent-history/agent-history-capability' import { TERMINAL_QUERY_REPLY_INPUT_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version' @@ -16,8 +17,6 @@ export function useMobileSessionTabReconciliation(scope: MobileSessionMarkdownAc worktreeId, client, connState, - hostCapabilities, - protocolVerified, sessionTabsRef, activeSessionTabIdRef, terminalsRef, @@ -145,14 +144,8 @@ export function useMobileSessionTabReconciliation(scope: MobileSessionMarkdownAc const hostQueryReplyInputSupportedRef = useRef(false) - // Why: the gate above every /h/ route already holds this connection's status.get answer (and - // retries it until one lands), so the route reads it through the foundation instead of issuing - // a second one. It reports no capabilities until the verdict is proven, which keeps the - // fail-closed reset below identical to the old pre-probe clear. useEffect(() => { - // Why: a client swap can keep the route connected while moving to an older - // host; clear the prior capability before exposing host-specific actions. - if (!client || connState !== 'connected' || !protocolVerified) { + if (!client || connState !== 'connected') { setBrowserScreencastSupported(null) setAgentSessionHistorySupported(null) setQuickCommandsSupported(null) @@ -160,15 +153,26 @@ export function useMobileSessionTabReconciliation(scope: MobileSessionMarkdownAc hostQueryReplyInputSupportedRef.current = false return } - setBrowserScreencastSupported(hostCapabilities.includes('browser.screencast.v1')) - setAgentSessionHistorySupported(hostCapabilities.includes(MOBILE_AI_VAULT_CAPABILITY)) - setQuickCommandsSupported(supportsMobileQuickCommands(hostCapabilities)) - // Why: hosts without this capability strip inputKind from terminal.send, - // so a forwarded xterm reply would become floor-stealing shell input. - hostQueryReplyInputSupportedRef.current = hostCapabilities.includes( - TERMINAL_QUERY_REPLY_INPUT_RUNTIME_CAPABILITY - ) - }, [client, connState, hostCapabilities, protocolVerified]) + // Why: a client swap can keep the route connected while moving to an older + // host; clear the prior capability before exposing host-specific actions. + setBrowserScreencastSupported(null) + setAgentSessionHistorySupported(null) + setQuickCommandsSupported(null) + setShowQuickCommands(false) + hostQueryReplyInputSupportedRef.current = false + // Why: the probe retries — a relay→direct cutover or request timeout rejects + // status.get without changing connState, which used to latch these hidden. + return startRuntimeCapabilityProbe(client, (capabilities) => { + setBrowserScreencastSupported(capabilities.includes('browser.screencast.v1')) + setAgentSessionHistorySupported(capabilities.includes(MOBILE_AI_VAULT_CAPABILITY)) + setQuickCommandsSupported(supportsMobileQuickCommands(capabilities)) + // Why: hosts without this capability strip inputKind from terminal.send, + // so a forwarded xterm reply would become floor-stealing shell input. + hostQueryReplyInputSupportedRef.current = capabilities.includes( + TERMINAL_QUERY_REPLY_INPUT_RUNTIME_CAPABILITY + ) + }) + }, [client, connState]) return { consumeAcceptedSessionTabs, hasSessionTabsRecoveryNeed, diff --git a/mobile/src/session/use-mobile-session-tab-strip-cache.ts b/mobile/src/session/use-mobile-session-tab-strip-cache.ts deleted file mode 100644 index d0207afd83c..00000000000 --- a/mobile/src/session/use-mobile-session-tab-strip-cache.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { useEffect, useState } from 'react' -import { - getSessionTabStripCacheKey, - loadCachedSessionTabStrip, - readCachedSessionTabStrip, - saveCachedSessionTabStrip -} from '../cache/session-tab-strip-cache' -import { - toMobileSessionTabStripPreview, - type MobileSessionTabStripPreview -} from './mobile-session-tab-strip-entries' -import type { MobileSessionBulkCloseModel } from './use-mobile-session-bulk-close' - -/** - * Keeps the last drawn tab strip for this workspace on the device, so a reconnect has something - * to render before the first snapshot lands. See mobile-session-reconnect-view-state. - */ -export function useMobileSessionTabStripCache(scope: MobileSessionBulkCloseModel) { - const { hostId, worktreeId, connState, terminalsLoaded } = scope - const { visibleTabs, activeSessionTabId, activeHandle } = scope - const cacheKey = getSessionTabStripCacheKey(hostId, worktreeId) - // Why: state settles a commit behind the key it was read for, so carry the key with it — - // otherwise the first render after a workspace switch draws the previous workspace's strip. - const [loaded, setLoaded] = useState<{ - key: string | null - preview: MobileSessionTabStripPreview | null - }>(() => ({ key: cacheKey, preview: readCachedSessionTabStrip(cacheKey) })) - - useEffect(() => { - // Synchronous first, so an in-session revisit never blinks through the uncached branch. - setLoaded({ key: cacheKey, preview: readCachedSessionTabStrip(cacheKey) }) - let disposed = false - void loadCachedSessionTabStrip(cacheKey).then((preview) => { - if (!disposed) { - setLoaded({ key: cacheKey, preview }) - } - }) - return () => { - disposed = true - } - }, [cacheKey]) - const cachedTabStrip = loaded.key === cacheKey ? loaded.preview : null - - // Only a host-confirmed strip is worth persisting, and an emptied workspace has to be written - // too — skipping it would leave yesterday's tabs to be drawn over a session that no longer has - // them. The one reading we do not trust is a live terminal with no tab record behind it, which - // is the same case the empty state refuses to claim (use-mobile-session-presentation). - // react-doctor-disable-next-line react-doctor/effect-needs-cleanup - useEffect(() => { - if (connState !== 'connected' || !terminalsLoaded) { - return - } - if (visibleTabs.length === 0 && activeHandle !== null) { - return - } - saveCachedSessionTabStrip( - cacheKey, - toMobileSessionTabStripPreview(visibleTabs, activeSessionTabId) - ) - }, [activeHandle, activeSessionTabId, cacheKey, connState, terminalsLoaded, visibleTabs]) - - return { cachedTabStrip } -} - -export type MobileSessionTabStripCacheModel = MobileSessionBulkCloseModel & - ReturnType diff --git a/mobile/src/session/use-mobile-session-terminal-subscription-foundation.ts b/mobile/src/session/use-mobile-session-terminal-subscription-foundation.ts index 6c833dfe088..76d8229b288 100644 --- a/mobile/src/session/use-mobile-session-terminal-subscription-foundation.ts +++ b/mobile/src/session/use-mobile-session-terminal-subscription-foundation.ts @@ -1,6 +1,4 @@ import { useRef, useCallback } from 'react' -import type { TerminalWebViewHandle } from '../terminal/terminal-webview-contract' -import { TERMINAL_ENGINE_PREWARM_HANDLE } from './TerminalEnginePrewarm' import type { MobileSessionNativeChatDictationModel } from './use-mobile-session-native-chat-dictation' export function useMobileSessionTerminalSubscriptionFoundation( @@ -103,34 +101,12 @@ export function useMobileSessionTerminalSubscriptionFoundation( }, [getTerminalRef] ) - // Why: the pre-warm engine occupies the frame the first pane will occupy, so let it satisfy - // the one-shot measurement. It has no handle, so it is passed its own ref instead of looking - // one up, and it must never latch a measurement taken before the frame has a real height. - const measurePrewarmViewport = useCallback( - async (engine: TerminalWebViewHandle, frameHeight: number) => { - if (viewportMeasuredRef.current || frameHeight <= 0) { - return - } - const dims = await engine.measureFitDimensions(frameHeight) - terminalDiagnosticsRef.current.viewportMeasured( - TERMINAL_ENGINE_PREWARM_HANDLE, - dims, - frameHeight - ) - if (dims && !viewportMeasuredRef.current) { - viewportRef.current = dims - viewportMeasuredRef.current = true - } - }, - [] - ) return { getTerminalRef, unsubscribeTerminal, unsubscribeTerminalRef, clearTerminalCache, - measureViewportOnce, - measurePrewarmViewport + measureViewportOnce } } diff --git a/mobile/src/transport/connection-log-buffer.test.ts b/mobile/src/transport/connection-log-buffer.test.ts index a069feadb07..51d8bbb044d 100644 --- a/mobile/src/transport/connection-log-buffer.test.ts +++ b/mobile/src/transport/connection-log-buffer.test.ts @@ -16,29 +16,6 @@ describe('connection log buffer', () => { expect(store.get('host-b').map((e) => e.id)).toEqual(['log-2']) }) - it('retains phase timings through redaction and evicts them with the cap', () => { - const store = createConnectionLogStore(2) - store.append('host-a', { - ...entry(1), - timing: { kind: 'connection-state', name: 'reconnecting', ms: 800, complete: true } - }) - store.append('host-a', { - ...entry(2), - detail: '4280ms in connecting; resumeToken=secret-resume-token', - timing: { kind: 'connection-state', name: 'connecting', ms: 4_280, complete: true } - }) - store.append('host-a', { - ...entry(3), - timing: { kind: 'relay-dial-stage', name: 'awaiting-hello', ms: 9_100, complete: false } - }) - - expect(store.get('host-a').map((e) => e.timing)).toEqual([ - { kind: 'connection-state', name: 'connecting', ms: 4_280, complete: true }, - { kind: 'relay-dial-stage', name: 'awaiting-hello', ms: 9_100, complete: false } - ]) - expect(store.get('host-a')[0]!.detail).toBe('4280ms in connecting; resumeToken=[redacted]') - }) - it('drops the oldest entries past the cap', () => { const store = createConnectionLogStore(3) for (let i = 1; i <= 5; i++) { diff --git a/mobile/src/transport/connection-state-dwell-log.test.ts b/mobile/src/transport/connection-state-dwell-log.test.ts deleted file mode 100644 index 4e28391cf6f..00000000000 --- a/mobile/src/transport/connection-state-dwell-log.test.ts +++ /dev/null @@ -1,77 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { DirectConnectionLog } from './direct-connection-log' -import { RpcClientConnectionState } from './rpc-client-connection-state' -import type { ConnectionLogEntry, ConnectionState } from './types' - -function openStateWithLog(sink?: (entry: ConnectionLogEntry) => void) { - const entries: ConnectionLogEntry[] = [] - const log = new DirectConnectionLog( - 'ws://192.168.1.50:6768', - sink ?? ((entry) => entries.push(entry)) - ) - let now = 0 - const state = new RpcClientConnectionState({ - endpoint: 'ws://192.168.1.50:6768', - getReconnectAttempt: () => 0, - isClosed: () => false, - onStateDwell: log.stateDwell, - now: () => now - }) - const publishAfter = (elapsedMs: number, next: ConnectionState): void => { - now += elapsedMs - state.publish(next) - } - return { entries, state, publishAfter } -} - -describe('connection state dwell logging', () => { - it('records the time spent in each state as a structured log entry', () => { - const { entries, publishAfter } = openStateWithLog() - - publishAfter(300, 'connecting') - publishAfter(4_200, 'handshaking') - publishAfter(250, 'connected') - - expect(entries.map((entry) => entry.timing)).toEqual([ - { kind: 'connection-state', name: 'disconnected', ms: 300, complete: true }, - { kind: 'connection-state', name: 'connecting', ms: 4_200, complete: true }, - { kind: 'connection-state', name: 'handshaking', ms: 250, complete: true } - ]) - expect(entries[1]!.message).toBe('Connection state connecting → handshaking') - expect(entries[1]!.detail).toBe('4200ms in connecting') - }) - - it('skips transitions too short to explain a slow connect', () => { - const { entries, publishAfter } = openStateWithLog() - - publishAfter(99, 'connecting') - publishAfter(100, 'handshaking') - - expect(entries.map((entry) => entry.timing?.name)).toEqual(['connecting']) - }) - - it('does not log a dwell when the state does not change', () => { - const { entries, publishAfter } = openStateWithLog() - - publishAfter(500, 'connecting') - publishAfter(500, 'connecting') - - expect(entries).toHaveLength(1) - }) - - it('still publishes the state when the log sink throws', () => { - const seen: ConnectionState[] = [] - const { state, publishAfter } = openStateWithLog(() => { - throw new Error('sink exploded') - }) - state.addListener((next) => seen.push(next)) - const connected = state.waitForConnected() - - publishAfter(500, 'connecting') - publishAfter(500, 'connected') - - expect(seen).toEqual(['connecting', 'connected']) - expect(state.get()).toBe('connected') - return expect(connected).resolves.toBeUndefined() - }) -}) diff --git a/mobile/src/transport/direct-connection-log.ts b/mobile/src/transport/direct-connection-log.ts index 43246e51ebc..2630b008459 100644 --- a/mobile/src/transport/direct-connection-log.ts +++ b/mobile/src/transport/direct-connection-log.ts @@ -4,15 +4,9 @@ import type { ConnectionLogEntry, ConnectionLogLevel, ConnectionLogSink, - ConnectionState, MobileConnectionDiagnosticPath } from './types' -// Why: every reconnect cycle walks four states, and the per-host buffer is capped. -// Logging sub-100ms transitions would halve the history a report can show while -// telling support nothing — those states are never where a slow connect spent time. -const MIN_LOGGED_DWELL_MS = 100 - export class DirectConnectionLog { private sequence = 0 private readonly path: MobileConnectionDiagnosticPath @@ -28,7 +22,7 @@ export class DirectConnectionLog { level: ConnectionLogLevel, message: string, detail?: string, - evidence?: Pick + evidence?: Pick ): void => { this.sink?.({ id: `log-${++this.sequence}-${Date.now()}`, @@ -50,26 +44,6 @@ export class DirectConnectionLog { ) } - // Why: how long the client sat in each ConnectionState used to go only to - // console, so a shared diagnostics report could not show where a slow connect - // spent its seconds. - stateDwell = (previous: ConnectionState, next: ConnectionState, dweltMs: number): void => { - if (dweltMs < MIN_LOGGED_DWELL_MS) { - return - } - this.emit('info', `Connection state ${previous} → ${next}`, `${dweltMs}ms in ${previous}`, { - timing: { kind: 'connection-state', name: previous, ms: dweltMs, complete: true } - }) - } - - retryScheduled = (message: string, detail?: string): void => { - this.emit('info', message, detail, { code: 'retry-scheduled' }) - } - - authenticationRejected = (message: string, detail?: string): void => { - this.emit('warn', message, detail, { code: 'authentication-rejected' }) - } - connected = (): void => { this.emit('success', 'Authenticated', 'Channel ready for RPC', { code: 'direct-connected' }) } diff --git a/mobile/src/transport/direct-rpc-client.ts b/mobile/src/transport/direct-rpc-client.ts index a4407035465..16306f95cdd 100644 --- a/mobile/src/transport/direct-rpc-client.ts +++ b/mobile/src/transport/direct-rpc-client.ts @@ -48,14 +48,14 @@ export class DirectRpcClient implements RpcClient { this.reconnect = new RpcClientReconnectSchedule({ openConnection: () => this.openConnection(), rejectConnectWaiters: (reason) => this.connectionState.rejectWaiters(reason), - emitLog: this.connectionLog.retryScheduled + emitLog: (message, detail) => + this.connectionLog.emit('info', message, detail, { code: 'retry-scheduled' }) }) this.connectionState = new RpcClientConnectionState({ endpoint, initialListener: options.onStateChange, getReconnectAttempt: () => this.reconnect.getAttempt(), - isClosed: () => this.intentionallyClosed, - onStateDwell: this.connectionLog.stateDwell + isClosed: () => this.intentionallyClosed }) this.streams = new RpcClientStreamRegistry({ nextId: () => this.nextId(), @@ -102,7 +102,8 @@ export class DirectRpcClient implements RpcClient { this.authenticationRetry = new RpcClientAuthenticationRetry({ endpoint, stopLiveness: () => this.stopLiveness(), - emitWarning: this.connectionLog.authenticationRejected, + emitWarning: (message, detail) => + this.connectionLog.emit('warn', message, detail, { code: 'authentication-rejected' }), retry: (reason) => this.retryAuthentication(reason), latchFailure: (reason) => this.latchAuthenticationFailure(reason) }) diff --git a/mobile/src/transport/host-removal-lifecycle.test.ts b/mobile/src/transport/host-removal-lifecycle.test.ts index 56d38ab0371..6c96ef1c446 100644 --- a/mobile/src/transport/host-removal-lifecycle.test.ts +++ b/mobile/src/transport/host-removal-lifecycle.test.ts @@ -17,12 +17,6 @@ vi.mock('./host-store', () => ({ })) import { removeHostAndCloseClient } from './host-removal-lifecycle' -import { - getSessionTabStripCacheKey, - readCachedSessionTabStrip, - resetSessionTabStripCacheForTests, - saveCachedSessionTabStrip -} from '../cache/session-tab-strip-cache' import { getHostNotificationSession, resetHostNotificationSessionsForTests @@ -32,9 +26,7 @@ describe('host removal lifecycle', () => { beforeEach(() => { removeHostMock.mockReset() asyncStorage.removeItem.mockClear() - asyncStorage.setItem.mockReset().mockResolvedValue(undefined) resetHostNotificationSessionsForTests() - resetSessionTabStripCacheForTests() }) it('closes the client only after metadata removal commits', async () => { @@ -96,40 +88,4 @@ describe('host removal lifecycle', () => { expect(asyncStorage.removeItem).toHaveBeenCalledWith('orca:mobileNotificationsWatermark:host-1') }) - - it('drops the removed host cached tab strip and keeps every other host', async () => { - // Why: the strip is plaintext and nothing else in the app ever expires an entry, so a - // forgotten host would keep its tab titles on disk and get them rewritten by the next - // save for any surviving host. - removeHostMock.mockResolvedValue(undefined) - const removed = getSessionTabStripCacheKey('host-1', 'wt-1') - const kept = getSessionTabStripCacheKey('host-2', 'wt-1') - const strip = { - tabs: [{ id: 'tab-1', type: 'terminal' as const, title: 'Terminal', agentId: null }], - activeTabId: 'tab-1' - } - saveCachedSessionTabStrip(removed, strip) - saveCachedSessionTabStrip(kept, strip) - - await removeHostAndCloseClient('host-1', vi.fn()) - // Fire-and-forget, like clearWatermark above; let its microtasks land. - await vi.waitFor(() => expect(readCachedSessionTabStrip(removed)).toBeNull()) - - expect(readCachedSessionTabStrip(kept)?.tabs).toHaveLength(1) - }) - it('finishes the removal even when the cached tab strip write fails', async () => { - // The metadata removal has already committed and the client is closed by this - // point, so a cache write that fails must be reported, not thrown: surfacing it - // as a failed removal would leave the user staring at a host that is really gone. - removeHostMock.mockResolvedValue(undefined) - asyncStorage.setItem.mockRejectedValue(new Error('storage full')) - const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) - const closeHostClient = vi.fn() - - await expect(removeHostAndCloseClient('host-1', closeHostClient)).resolves.toBeUndefined() - - expect(closeHostClient).toHaveBeenCalledWith('host-1') - expect(warn).toHaveBeenCalled() - warn.mockRestore() - }) }) diff --git a/mobile/src/transport/host-removal-lifecycle.ts b/mobile/src/transport/host-removal-lifecycle.ts index 1608d065719..cd0a09cb67e 100644 --- a/mobile/src/transport/host-removal-lifecycle.ts +++ b/mobile/src/transport/host-removal-lifecycle.ts @@ -1,4 +1,3 @@ -import { deleteCachedSessionTabStripForHost } from '../cache/session-tab-strip-cache' import { clearWatermark, forgetHostNotificationSession @@ -18,12 +17,4 @@ export async function removeHostAndCloseClient( // re-pair of the same host would inherit a watermark for a counter it never saw. forgetHostNotificationSession(hostId) void clearWatermark(hostId) - // Why: the cached tab strip is plaintext and host-scoped, so forgetting the host has to drop - // it here too — nothing else in the app ever expires an entry. Awaited so a storage failure - // is observed rather than swallowed, but never fatal: the metadata removal has already - // committed and the client is closed, so failing here would report a finished removal as - // failed. The cache refuses further saves for this host either way. - await deleteCachedSessionTabStripForHost(hostId).catch((error: unknown) => { - console.warn('[host-removal] cached tab strip delete failed', error) - }) } diff --git a/mobile/src/transport/host-status-gates.ts b/mobile/src/transport/host-status-gates.ts index 4f8bc8fcc52..91f0205a5c7 100644 --- a/mobile/src/transport/host-status-gates.ts +++ b/mobile/src/transport/host-status-gates.ts @@ -1,7 +1,6 @@ import { useEffect, useState } from 'react' import type { RpcClient } from './rpc-client' -import type { ConnectionState } from './types' -import { readRuntimeCapabilities, startRuntimeStatusProbe } from './runtime-status-probe' +import type { ConnectionState, RpcSuccess } from './types' import { evaluateCompat, type CompatVerdict } from './protocol-compat' import type { DesktopStatus } from '../worktree/host-worktree-rpc-types' import { normalizeHostAppVersion, recordHostAppVersion } from './host-app-version-store' @@ -11,10 +10,6 @@ export type HostStatusGates = { floatingWorkspaceEnabled: boolean desktopAppVersion: string | null compatVerdict: CompatVerdict - // Why: `compatVerdict.kind === 'ok'` is not proof. A host that never answers status.get settles - // the same `ok` so navigation is not trapped, and that fallback must not read as a passing - // verdict. Only an evaluated status reply sets this, so writes to the host can gate on it. - compatVerified: boolean statusPending: boolean } @@ -26,11 +21,8 @@ type LoadedHostStatusGates = Omit & { const EMPTY_HOST_CAPABILITIES: string[] = [] -// The route tree's single status.get: it reads capabilities, the protocol-compat verdict, and -// the floating-workspace flag once per connection and publishes them through HostProtocolGate, -// so no descendant issues its own. The verdict really can block — evaluateCompat reads a missing -// protocolVersion as 0, below MIN_COMPATIBLE_DESKTOP_VERSION — so a pending verdict is a real -// state, not a formality, and anything that writes to the host must wait for it. +// Reads status.get on connect for capabilities, protocol-compat verdict, and the +// floating-workspace flag. Compat constants are wide-open today so this never blocks yet. export function useHostStatusGates(args: { hostId: string | undefined client: RpcClient | null @@ -47,33 +39,30 @@ export function useHostStatusGates(args: { setUnverified(true) return } + let cancelled = false const requestClient = client const settle = (gates: Omit) => { setLoaded({ hostId, client: requestClient, ...gates }) setUnverified(false) } - // Why: a transient status failure must not trap navigation, so the first miss settles - // conservative gates and releases the pending overlay; the probe keeps retrying underneath - // so a cutover or timeout no longer latches capability-gated UI hidden until a remount. - // compatVerified stays false: this releases the UI, it proves nothing about the host. - let failedOpen = false - const failOpen = () => { - if (failedOpen) { - return - } - failedOpen = true - settle({ - hostCapabilities: [], - floatingWorkspaceEnabled: false, - desktopAppVersion: null, - compatVerdict: { kind: 'ok' }, - compatVerified: false - }) - } - return startRuntimeStatusProbe(requestClient, { - onUnavailable: failOpen, - onStatus: (result) => { - const status = result as DesktopStatus & { capabilities?: string[] } + void (async () => { + try { + const response = await requestClient.sendRequest('status.get') + if (cancelled) { + return + } + if (!response.ok) { + settle({ + hostCapabilities: [], + floatingWorkspaceEnabled: false, + desktopAppVersion: null, + compatVerdict: { kind: 'ok' } + }) + return + } + const status = (response as RpcSuccess).result as DesktopStatus & { + capabilities?: string[] + } const verdict = evaluateCompat({ desktopProtocolVersion: status.protocolVersion, desktopMinCompatibleMobileVersion: status.minCompatibleMobileVersion @@ -83,11 +72,10 @@ export function useHostStatusGates(args: { void recordHostAppVersion(hostId, desktopAppVersion) } settle({ - hostCapabilities: [...readRuntimeCapabilities(result)], + hostCapabilities: status.capabilities ?? [], floatingWorkspaceEnabled: status.floatingWorkspaceEnabled === true, desktopAppVersion, - compatVerdict: verdict, - compatVerified: true + compatVerdict: verdict }) if (verdict.kind === 'blocked') { // Why: support breadcrumb to confirm a block fired vs a render bug; no PII, just version ints. @@ -98,8 +86,21 @@ export function useHostStatusGates(args: { requiredDesktopVersion: verdict.requiredDesktopVersion }) } + } catch { + // Why: a transient status failure must not trap navigation; conservative feature gates remain disabled. + if (!cancelled) { + settle({ + hostCapabilities: [], + floatingWorkspaceEnabled: false, + desktopAppVersion: null, + compatVerdict: { kind: 'ok' } + }) + } } - }) + })() + return () => { + cancelled = true + } }, [client, connState, hostId]) // Why: effects run after render, so key loaded gates by host and client to fail closed during route reuse. @@ -110,7 +111,6 @@ export function useHostStatusGates(args: { floatingWorkspaceEnabled: false, desktopAppVersion: null, compatVerdict: { kind: 'ok' }, - compatVerified: false, statusPending: connState === 'connected' && client !== null } } @@ -119,7 +119,6 @@ export function useHostStatusGates(args: { floatingWorkspaceEnabled: proven.floatingWorkspaceEnabled, desktopAppVersion: proven.desktopAppVersion, compatVerdict: proven.compatVerdict, - compatVerified: proven.compatVerified, // Why (F10): unchanged pending timing — the reconnect refetch is still "unknown", it just no // longer blanks the capabilities this same host already proved. statusPending: connState === 'connected' && unverified diff --git a/mobile/src/transport/mobile-direct-return-probe.test.ts b/mobile/src/transport/mobile-direct-return-probe.test.ts deleted file mode 100644 index 6e73243cbf9..00000000000 --- a/mobile/src/transport/mobile-direct-return-probe.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -import { afterEach, beforeEach, expect, it, vi } from 'vitest' -import { DirectReturnProbe } from './mobile-direct-return-probe' -import { MobileEndpointHysteresis } from './mobile-endpoint-hysteresis' -import { FakeSession, host } from './mobile-endpoint-supervisor-test-fakes' - -vi.mock('react-native', () => ({ Platform: { OS: 'ios' } })) - -// A LAN that never answers: every dial sits open until the probe's own 12s budget. -function fixture() { - const opened: FakeSession[] = [] - const probe = new DirectReturnProbe( - { - now: Date.now, - setTimer: setTimeout, - clearTimer: clearTimeout, - openDirect: () => { - const candidate = new FakeSession('connecting') - opened.push(candidate) - return candidate - } - }, - { - hysteresis: new MobileEndpointHysteresis(Date.now(), { - directSuccessesRequired: 1, - directObservationMs: 60_000, - failureCooldownMs: 0, - minimumDwellMs: 0 - }), - host: () => host, - canSchedule: () => true, - canDial: () => true, - canAttempt: () => true, - // These cases model a live relay session, so hysteresis still arbitrates. - adoptsOutright: () => false, - beginOperation: () => {}, - migrate: async () => {}, - onDirectMigrated: async () => {}, - afterProbe: () => {} - } - ) - return { opened, probe } -} - -beforeEach(() => vi.useFakeTimers()) -afterEach(() => vi.useRealTimers()) - -it('never opens a second dial while one is still in flight', async () => { - const { opened, probe } = fixture() - probe.schedule(0) - await vi.advanceTimersByTimeAsync(0) - expect(opened).toHaveLength(1) - - // A relay drop and a foreground return both ask for an immediate probe while the - // first dial is still awaiting authentication. - probe.schedule(0) - probe.schedule(0) - await vi.advanceTimersByTimeAsync(0) - expect(opened).toHaveLength(1) - - // Why this is the assertion that matters: a second probe would have overwritten - // activeProbe, so stop() would abort only the newest dial and leave this socket - // open for the rest of its 12s budget. - probe.stop() - await vi.advanceTimersByTimeAsync(0) - expect(opened[0]!.close).toHaveBeenCalledOnce() - expect(vi.getTimerCount()).toBe(0) -}) - -it('honors an urgent reprobe asked for mid-dial instead of dropping it on the 15s floor', async () => { - const { opened, probe } = fixture() - probe.schedule(0) - await vi.advanceTimersByTimeAsync(0) - probe.schedule(0) - await vi.advanceTimersByTimeAsync(0) - expect(opened).toHaveLength(1) - - // The deferred ask survives the dial and runs at once when it settles, so holding - // the slot does not cost the caller the 15s it was trying to skip. - await vi.advanceTimersByTimeAsync(12_000) - await vi.advanceTimersByTimeAsync(1) - expect(opened).toHaveLength(2) - probe.stop() -}) - -it('falls back to the ordinary interval when nothing asked for a sooner probe', async () => { - const { opened, probe } = fixture() - probe.schedule(0) - await vi.advanceTimersByTimeAsync(12_000) - expect(opened).toHaveLength(1) - - await vi.advanceTimersByTimeAsync(14_999) - expect(opened).toHaveLength(1) - await vi.advanceTimersByTimeAsync(1) - expect(opened).toHaveLength(2) - probe.stop() -}) diff --git a/mobile/src/transport/mobile-direct-return-probe.ts b/mobile/src/transport/mobile-direct-return-probe.ts index 5c996a4f001..3ae31edd07f 100644 --- a/mobile/src/transport/mobile-direct-return-probe.ts +++ b/mobile/src/transport/mobile-direct-return-probe.ts @@ -6,18 +6,13 @@ import type { MobileConnectionPath } from './stable-logical-rpc-client' const DIRECT_PROBE_INTERVAL_MS = 15_000 -// Re-acquires the direct endpoint while the runtime channel rides the relay. -// Two adoption policies, because what is at stake differs: -// - against a live relay, hysteresis must prove direct stable before the swap; -// - during a reconnect nothing is live, so this dial races the relay dial from -// t=0 and the first authenticated socket is adopted outright. +// While the runtime channel rides the relay, periodically probe the direct +// endpoint and migrate back once hysteresis proves it stable. export class DirectReturnProbe { private timer: ReturnType | null = null private stopped = false private activeProbe: AbortController | null = null - // Soonest delay a caller asked for while a dial was in flight. - private deferredDelayMs: number | null = null constructor( private readonly deps: { @@ -30,14 +25,7 @@ export class DirectReturnProbe { hysteresis: MobileEndpointHysteresis host: () => HostProfile canSchedule: () => boolean - // A dial is a pure observation on its own socket, so it only needs a live - // supervisor; the cutover is the part that needs the operation mutex. - canDial: () => boolean canAttempt: () => boolean - // True while no session is live: the reconnect is a race, so an - // authenticated direct socket wins without consulting hysteresis. - adoptsOutright: () => boolean - // Takes the supervisor's operation mutex, now held for the cutover only. beginOperation: () => void migrate: ( client: RpcClient, @@ -50,18 +38,7 @@ export class DirectReturnProbe { ) {} schedule(delayMs = DIRECT_PROBE_INTERVAL_MS): void { - if (this.stopped || !this.hooks.canSchedule()) { - return - } - // Why: the dial no longer holds the supervisor's mutex, so nothing else stops a - // second probe from overwriting activeProbe — stop() would then reach only the - // newest socket and leave the earlier one dialing for its full 12s budget. The - // in-flight probe owns the next slot and re-arms it on the soonest ask. - if (this.activeProbe) { - this.deferredDelayMs = Math.min(this.deferredDelayMs ?? delayMs, delayMs) - return - } - if (this.timer) { + if (this.stopped || !this.hooks.canSchedule() || this.timer) { return } this.timer = this.deps.setTimer(() => { @@ -70,18 +47,7 @@ export class DirectReturnProbe { }, delayMs) } - // Why: a reconnect races both paths from t=0, and schedule(0) yields to a - // pending 15s tick — that would hand the relay dial a head start by another name. - probeNow(): void { - if (this.stopped || this.activeProbe) { - return - } - this.clear() - this.schedule(0) - } - clear(): void { - this.deferredDelayMs = null if (this.timer) { this.deps.clearTimer(this.timer) this.timer = null @@ -98,23 +64,15 @@ export class DirectReturnProbe { if (this.stopped) { return } - // Why: the failure cooldown exists to stop a healthy relay flapping onto a - // marginal LAN. With nothing connected there is no session to protect, and - // honouring it would leave the phone waiting on relay alone. - const racing = this.hooks.adoptsOutright() - if (!this.hooks.canDial() || (!racing && !this.hooks.hysteresis.canProbe(this.deps.now()))) { + if (!this.hooks.canAttempt() || !this.hooks.hysteresis.canProbe(this.deps.now())) { this.schedule() return } const controller = new AbortController() this.activeProbe = controller - let owned = false + this.hooks.beginOperation() let successful: Awaited> = null try { - // Why: the dial is a pure observation on its own socket — holding the - // supervisor's mutex across its 12s budget stalled every relay recovery - // that landed during a foreground return, and makes the reconnect race - // unwinnable while a relay dial holds it. successful = await openAuthenticatedDirectEndpoint( this.hooks.host(), this.deps.openDirect, @@ -128,42 +86,17 @@ export class DirectReturnProbe { this.hooks.hysteresis.recordDirectFailure(this.deps.now()) return } - // Both early returns leave the candidate to the finally, which owns it until - // migration takes over — closing here too would double-close it. - const outright = this.hooks.adoptsOutright() - // Why: a socket that entered the race and lost books nothing and leaves the - // promotion streak untouched — the winner is this reconnect's whole verdict. - if (!outright && (racing || !this.hooks.hysteresis.recordDirectSuccess(this.deps.now()))) { + if (!this.hooks.hysteresis.recordDirectSuccess(this.deps.now())) { + successful.client.close() return } - const mutexFree = this.hooks.canAttempt() - if (!mutexFree && !outright) { - // A relay dial owns the mutex; the streak survives, so the next probe - // promotes direct instead of this one. - return - } - if (mutexFree) { - this.hooks.beginOperation() - owned = true - } - // Why: when a relay dial holds the mutex the race still cuts over — that - // dial withdraws itself in migrateTo and books no failure against relay. const candidate = successful // Migration owns the candidate, including closing it if cutover is canceled. successful = null - // Why: the relay dial can authenticate between this socket's authentication - // and the swap. migrateTo re-checks after auth, so the loser withdraws. - const abortCutover = outright - ? (): boolean => this.stopped || !this.hooks.adoptsOutright() - : (): boolean => this.stopped try { - await this.hooks.migrate(candidate.client, candidate.path, abortCutover) + await this.hooks.migrate(candidate.client, candidate.path, () => this.stopped) } catch (error) { - // Why: a withdrawn cutover is the ordinary end of a lost race, and - // migrateTo has already closed the candidate. Only the timer calls this - // method, and it discards the promise, so rethrowing here would surface - // a routine loss as an unhandled rejection. - if (this.stopped || abortCutover()) { + if (this.stopped) { return } throw error @@ -176,14 +109,10 @@ export class DirectReturnProbe { } finally { this.activeProbe = null successful?.client.close() - // Why: a relay drop or backoff timer can arrive while the cutover owns the + // Why: a relay drop or backoff timer can arrive while the probe owns the // operation mutex; afterProbe releases it and replays deferred recovery. - if (owned) { - this.hooks.afterProbe() - } - const deferred = this.deferredDelayMs - this.deferredDelayMs = null - this.schedule(deferred ?? undefined) + this.hooks.afterProbe() + this.schedule() } } } diff --git a/mobile/src/transport/mobile-endpoint-lifecycle.ts b/mobile/src/transport/mobile-endpoint-lifecycle.ts index 1542de9da7d..7ec5f28b945 100644 --- a/mobile/src/transport/mobile-endpoint-lifecycle.ts +++ b/mobile/src/transport/mobile-endpoint-lifecycle.ts @@ -86,7 +86,7 @@ function createSupervisor( ): MobileEndpointSupervisor { return new MobileEndpointSupervisor(logical, host, { openDirect: (endpoint) => connect(endpoint, host.deviceToken, host.publicKeyB64, { onLog }), - openRelay: (relay, credential, confirmReqId, onHostCloseReason, isForeground) => + openRelay: (relay, credential, confirmReqId, onHostCloseReason) => connectMobileRelayRpcSession({ relay, resumeToken: credential.token, @@ -94,7 +94,6 @@ function createSupervisor( resumeConfirmReqId: confirmReqId, deviceToken: host.deviceToken, desktopPublicKeyB64: host.publicKeyB64, - isForeground, onHostCloseReason, onLog }), diff --git a/mobile/src/transport/mobile-endpoint-reconnect-race.test.ts b/mobile/src/transport/mobile-endpoint-reconnect-race.test.ts deleted file mode 100644 index 47f41e0556d..00000000000 --- a/mobile/src/transport/mobile-endpoint-reconnect-race.test.ts +++ /dev/null @@ -1,362 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { MobileEndpointHysteresis } from './mobile-endpoint-hysteresis' -import { MobileEndpointSupervisor } from './mobile-endpoint-supervisor' -import { RelayOuterError } from './mobile-relay-e2ee-link' -import { - dependencies, - FakeLogicalClient, - FakeRelaySession, - FakeSession, - host, - unreachableDirect -} from './mobile-endpoint-supervisor-test-fakes' - -vi.mock('react-native', () => ({ Platform: { OS: 'ios' } })) -vi.mock('expo-secure-store', () => ({ WHEN_UNLOCKED_THIS_DEVICE_ONLY: 'when-unlocked' })) -vi.mock('expo-crypto', () => ({ getRandomBytes: (length: number) => new Uint8Array(length) })) - -// Holds the relay cutover open so the direct path can authenticate mid-dial. The -// fake's migrateTo otherwise settles inside the dial, which no real cell does. -function holdRelayCutover(logical: FakeLogicalClient): () => void { - const settle = logical.migrateTo.getMockImplementation()! - let release!: () => void - const held = new Promise((resolve) => { - release = resolve - }) - logical.migrateTo.mockImplementationOnce(async (session, path, timeoutMs, shouldAbort) => { - await held - // Why: replays the real post-authentication checks, so a superseded dial - // still withdraws instead of stealing the client from the winner. - return await settle(session, path, timeoutMs, shouldAbort) - }) - return release -} - -// One full lost race: the relay dial starts, direct returns mid-cutover and wins. -async function loseOneRace( - logical: FakeLogicalClient, - openRelay: ReturnType -): Promise { - const before = openRelay.mock.calls.length - const release = holdRelayCutover(logical) - logical.publishState('reconnecting') - await vi.advanceTimersByTimeAsync(0) - expect(openRelay.mock.calls.length).toBe(before + 1) - logical.publishState('connected') - release() - await vi.advanceTimersByTimeAsync(0) -} - -function relaySessionsFrom(openRelay: ReturnType): FakeRelaySession[] { - return openRelay.mock.results.map((result) => result.value as FakeRelaySession) -} - -describe('mobile endpoint reconnect race', () => { - beforeEach(() => { - vi.useFakeTimers() - vi.setSystemTime(new Date('2026-07-13T12:00:00Z')) - }) - - afterEach(() => { - vi.useRealTimers() - }) - - it('dials relay at t=0 while the direct dial is still connecting', async () => { - const logical = new FakeLogicalClient('connecting', 'lan') - const deps = dependencies({ openDirect: unreachableDirect() }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - - // No timer advance at all: an unfinished direct dial buys no head start. - await supervisor.start() - - expect(deps.openRelay).toHaveBeenCalledOnce() - expect(logical.getActivePath()).toBe('relay') - expect(logical.migrateTo).toHaveBeenCalledWith( - expect.any(FakeRelaySession), - 'relay', - undefined, - expect.any(Function) - ) - supervisor.stop() - }) - - it('adopts the direct dial and withdraws the slower relay dial without booking it', async () => { - const logical = new FakeLogicalClient('connecting', 'lan') - const openRelay = vi.fn(() => new FakeRelaySession('connected')) - const deps = dependencies({ openRelay, openDirect: unreachableDirect() }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - - const release = holdRelayCutover(logical) - const starting = supervisor.start() - await vi.advanceTimersByTimeAsync(0) - expect(openRelay).toHaveBeenCalledOnce() - - // The direct dial authenticates while the cell is still cutting over. - logical.publishState('connected') - release() - await starting - - expect(logical.getActivePath()).toBe('lan') - expect(relaySessionsFrom(openRelay)[0]!.close).toHaveBeenCalled() - // A withdrawn dial is not a failure: no cooldown is armed, so no redial lands. - await vi.advanceTimersByTimeAsync(60_000) - expect(openRelay).toHaveBeenCalledOnce() - expect(logical.setRecoveryPath).toHaveBeenLastCalledWith(null) - supervisor.stop() - }) - - it('adopts a direct socket that wins a reconnect the relay path started', async () => { - const recordMigration = vi.spyOn(MobileEndpointHysteresis.prototype, 'recordMigration') - const logical = new FakeLogicalClient('connected', 'relay') - const openRelay = vi.fn(() => new FakeRelaySession('connected')) - const deps = dependencies({ openRelay }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - await supervisor.start() - - const release = holdRelayCutover(logical) - logical.publishState('disconnected') - // The direct dial runs while the relay dial is still in flight and wins it. - await vi.advanceTimersByTimeAsync(0) - expect(deps.openDirect).toHaveBeenCalledOnce() - expect(logical.getActivePath()).toBe('lan') - - release() - await vi.advanceTimersByTimeAsync(0) - expect(relaySessionsFrom(openRelay)[0]!.close).toHaveBeenCalled() - // Hysteresis stamps the dwell, and the losing relay dial books no backoff. - expect(recordMigration).toHaveBeenCalled() - await vi.advanceTimersByTimeAsync(60_000) - expect(openRelay).toHaveBeenCalledOnce() - supervisor.stop() - }) - - it('books one backoff, not two, when both paths lose the reconnect', async () => { - const recordDirectFailure = vi.spyOn(MobileEndpointHysteresis.prototype, 'recordDirectFailure') - const logical = new FakeLogicalClient('connected', 'relay') - const openRelay = vi.fn(() => new FakeRelaySession('disconnected', new RelayOuterError(4408))) - const deps = dependencies({ - openRelay, - openDirect: unreachableDirect(), - randomBytes: () => new Uint8Array([128, 0]) - }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - await supervisor.start() - - logical.publishState('disconnected') - await vi.advanceTimersByTimeAsync(0) - expect(openRelay).toHaveBeenCalledOnce() - expect(deps.openDirect).toHaveBeenCalledOnce() - expect(recordDirectFailure).toHaveBeenCalledOnce() - - // One failure, so one 250ms step. A double-booked loss would redial at 500ms. - await vi.advanceTimersByTimeAsync(249) - expect(openRelay).toHaveBeenCalledOnce() - await vi.advanceTimersByTimeAsync(1) - expect(openRelay).toHaveBeenCalledTimes(2) - supervisor.stop() - }) - - it('leaves the promotion streak alone when the direct socket loses the race', async () => { - const recordDirectSuccess = vi.spyOn(MobileEndpointHysteresis.prototype, 'recordDirectSuccess') - const logical = new FakeLogicalClient('connected', 'relay') - const direct = new FakeSession('connecting') - const openRelay = vi.fn(() => new FakeRelaySession('connected')) - const deps = dependencies({ openRelay, openDirect: vi.fn(() => direct) }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - await supervisor.start() - - const release = holdRelayCutover(logical) - logical.publishState('disconnected') - await vi.advanceTimersByTimeAsync(0) - expect(deps.openDirect).toHaveBeenCalledOnce() - - // Relay authenticates first, then the direct socket finally answers. - release() - await vi.advanceTimersByTimeAsync(0) - expect(logical.getActivePath()).toBe('relay') - direct.publishState('connected') - await vi.advanceTimersByTimeAsync(0) - - expect(direct.close).toHaveBeenCalled() - expect(recordDirectSuccess).not.toHaveBeenCalled() - expect(logical.getActivePath()).toBe('relay') - supervisor.stop() - }) - - it('ignores a loser that closes after the winner has been adopted', async () => { - const logical = new FakeLogicalClient('connecting', 'lan') - const openRelay = vi.fn(() => new FakeRelaySession('connected')) - const deps = dependencies({ openRelay, openDirect: unreachableDirect() }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - - const release = holdRelayCutover(logical) - const starting = supervisor.start() - await vi.advanceTimersByTimeAsync(0) - logical.publishState('connected') - release() - await starting - expect(logical.getActivePath()).toBe('lan') - - // The withdrawn cell socket reports its close afterwards. - relaySessionsFrom(openRelay)[0]!.publishState('disconnected') - await vi.advanceTimersByTimeAsync(60_000) - - expect(logical.getState()).toBe('connected') - expect(logical.getActivePath()).toBe('lan') - expect(openRelay).toHaveBeenCalledOnce() - supervisor.stop() - }) - - it('withdraws the relay socket before it authenticates once direct wins', async () => { - const logical = new FakeLogicalClient('connecting', 'lan') - const relaySession = new FakeRelaySession('connecting') - const openRelay = vi.fn(() => relaySession) - const deps = dependencies({ openRelay, openDirect: unreachableDirect() }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - - const release = holdRelayCutover(logical) - const starting = supervisor.start() - await vi.advanceTimersByTimeAsync(0) - expect(openRelay).toHaveBeenCalledOnce() - expect(relaySession.close).not.toHaveBeenCalled() - - // The direct dial authenticates while the cell socket is still pre-handshake. - // migrateTo would not withdraw until after E2EE auth, so the cell would have - // reserved a splice and the desktop would have finished a handshake for it. - logical.publishState('connected') - expect(relaySession.close).toHaveBeenCalled() - expect(relaySession.getState()).not.toBe('connected') - - release() - await starting - await vi.advanceTimersByTimeAsync(60_000) - expect(logical.getActivePath()).toBe('lan') - expect(openRelay).toHaveBeenCalledOnce() - supervisor.stop() - }) - - it('damps the race after a loss so a flapping LAN opens one cell socket', async () => { - const logical = new FakeLogicalClient('connecting', 'lan') - const openRelay = vi.fn(() => new FakeRelaySession('connecting')) - const deps = dependencies({ openRelay, openDirect: unreachableDirect() }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - - // The first blip races, and the returning direct dial wins it. - const release = holdRelayCutover(logical) - const starting = supervisor.start() - await vi.advanceTimersByTimeAsync(0) - logical.publishState('connected') - release() - await starting - expect(openRelay).toHaveBeenCalledOnce() - - // Two more blips inside the damper window open no further cell socket. - for (const _blip of [1, 2]) { - logical.publishState('reconnecting') - await vi.advanceTimersByTimeAsync(100) - logical.publishState('connected') - await vi.advanceTimersByTimeAsync(400) - } - expect(openRelay).toHaveBeenCalledOnce() - - // The window lapses against a live direct path, so it still opens nothing. - await vi.advanceTimersByTimeAsync(10_000) - expect(openRelay).toHaveBeenCalledOnce() - supervisor.stop() - }) - - it('races at once when the LAN dies inside a damper window grown to the cap', async () => { - const logical = new FakeLogicalClient('connecting', 'lan') - const openRelay = vi.fn(() => new FakeRelaySession('connecting')) - const deps = dependencies({ openRelay, openDirect: unreachableDirect() }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - - const release = holdRelayCutover(logical) - const starting = supervisor.start() - await vi.advanceTimersByTimeAsync(0) - logical.publishState('connected') - release() - await starting - - // Four more losses, each once its window has run: 2s, 4s, 8s, 16s, then the - // fifth earns the 30s cap. - for (const window of [2_000, 4_000, 8_000, 16_000]) { - await vi.advanceTimersByTimeAsync(window) - await loseOneRace(logical, openRelay) - } - expect(openRelay).toHaveBeenCalledTimes(5) - - // This time direct does not come back. Waiting out the window a blip earned - // would strand the phone offline for 30s with nothing else scheduled. - logical.publishState('reconnecting') - await vi.advanceTimersByTimeAsync(249) - expect(openRelay).toHaveBeenCalledTimes(5) - await vi.advanceTimersByTimeAsync(1) - expect(openRelay.mock.calls.length).toBeGreaterThan(5) - supervisor.stop() - }) - - it('lets a foreground resume race immediately inside a damper window', async () => { - const logical = new FakeLogicalClient('connecting', 'lan') - const openRelay = vi.fn(() => new FakeRelaySession('connecting')) - const deps = dependencies({ openRelay, openDirect: unreachableDirect() }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - - const release = holdRelayCutover(logical) - const starting = supervisor.start() - await vi.advanceTimersByTimeAsync(0) - logical.publishState('connected') - release() - await starting - - logical.publishState('reconnecting') - await vi.advanceTimersByTimeAsync(100) - expect(openRelay).toHaveBeenCalledOnce() - - // A resume is the user waiting on the screen; it never serves out the window. - supervisor.setForeground(false) - supervisor.setForeground(true) - await vi.advanceTimersByTimeAsync(0) - - expect(openRelay.mock.calls.length).toBeGreaterThan(1) - supervisor.stop() - }) - - it('starts no dial in the background and races both paths on resume', async () => { - const logical = new FakeLogicalClient('connecting', 'lan') - const deps = dependencies({ openDirect: unreachableDirect() }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - - supervisor.setForeground(false) - await supervisor.start() - await vi.advanceTimersByTimeAsync(60_000) - expect(deps.openRelay).not.toHaveBeenCalled() - - supervisor.setForeground(true) - await vi.advanceTimersByTimeAsync(0) - - expect(deps.openRelay).toHaveBeenCalledOnce() - expect(logical.getActivePath()).toBe('relay') - supervisor.stop() - }) - - it('runs the resume probe against a relay that survived the background grace', async () => { - const logical = new FakeLogicalClient('connected', 'relay') - const deps = dependencies() - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - await supervisor.start() - - supervisor.setForeground(false) - await vi.advanceTimersByTimeAsync(1_000) - expect(deps.openDirect).not.toHaveBeenCalled() - - // A live relay is not a reconnect: the resume probe dials direct, but the - // promotion still has to earn its hysteresis streak. - supervisor.setForeground(true) - await vi.advanceTimersByTimeAsync(0) - expect(deps.openDirect).toHaveBeenCalledOnce() - expect(logical.getActivePath()).toBe('relay') - expect(deps.openRelay).not.toHaveBeenCalled() - supervisor.stop() - }) -}) diff --git a/mobile/src/transport/mobile-endpoint-supervisor-contract.ts b/mobile/src/transport/mobile-endpoint-supervisor-contract.ts index 29ec807e649..2a784fd8895 100644 --- a/mobile/src/transport/mobile-endpoint-supervisor-contract.ts +++ b/mobile/src/transport/mobile-endpoint-supervisor-contract.ts @@ -12,9 +12,7 @@ export type MobileEndpointSupervisorDependencies = { relay: MobileRelayEndpoint, credential: { token: string; version: number }, confirmReqId: string, - onHostCloseReason?: (reason: RelayHostCloseReason) => void, - // Gates the session's idle liveness sweep; a backgrounded app spends no probes. - isForeground?: () => boolean + onHostCloseReason?: (reason: RelayHostCloseReason) => void ) => MobileRelayRpcSession resolveRelay: typeof resolveMobileRelayEndpoint readBundle: (hostId: string) => Promise diff --git a/mobile/src/transport/mobile-endpoint-supervisor-direct-probe.test.ts b/mobile/src/transport/mobile-endpoint-supervisor-direct-probe.test.ts index 634953d93ff..3ee52fc7ddf 100644 --- a/mobile/src/transport/mobile-endpoint-supervisor-direct-probe.test.ts +++ b/mobile/src/transport/mobile-endpoint-supervisor-direct-probe.test.ts @@ -1,26 +1,13 @@ import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest' import { MobileEndpointSupervisor } from './mobile-endpoint-supervisor' -import { MobileEndpointHysteresis } from './mobile-endpoint-hysteresis' import { dependencies, FakeLogicalClient, FakeRelaySession, FakeSession, - host, - unreachableDirect + host } from './mobile-endpoint-supervisor-test-fakes' -// A cell that authenticates and then answers the confirm for a different relay host -// — what a rehomed desktop produces. The session fails after the logical cutover. -function confirmRejectingRelaySession(logical: FakeLogicalClient): FakeRelaySession { - const session = new FakeRelaySession('connected', new Error('relay resume confirmation missing')) - session.whenResumeConfirmed = async () => { - session.publishState('disconnected') - logical.publishState('disconnected') - } - return session -} - vi.mock('react-native', () => ({ Platform: { OS: 'ios' } })) vi.mock('expo-secure-store', () => ({ WHEN_UNLOCKED_THIS_DEVICE_ONLY: 'when-unlocked' })) vi.mock('expo-crypto', () => ({ getRandomBytes: (length: number) => new Uint8Array(length) })) @@ -61,103 +48,4 @@ describe('mobile endpoint supervisor direct probe', () => { expect(logical.getActivePath()).toBe('relay') supervisor.stop() }) - - it('recovers the relay at once while the probe is still dialing direct', async () => { - const logical = new FakeLogicalClient('connected', 'relay') - // A black-holed LAN endpoint: the dial sits unanswered for its whole 12s budget. - const direct = new FakeSession('connecting') - const openRelay = vi.fn(() => new FakeRelaySession('connected')) - const deps = dependencies({ openDirect: vi.fn(() => direct), openRelay }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - await supervisor.start() - - await vi.advanceTimersByTimeAsync(15_000) - expect(deps.openDirect).toHaveBeenCalledOnce() - logical.publishState('disconnected') - await vi.advanceTimersByTimeAsync(0) - - // Why: the dial is a pure observation, so it no longer owns the operation - // mutex — recovery does not wait out the probe's budget. - expect(openRelay).toHaveBeenCalledOnce() - expect(logical.getState()).toBe('connected') - expect(logical.getActivePath()).toBe('relay') - supervisor.stop() - }) - - it('backs off a dial whose resume confirm fails after the cutover', async () => { - const recordMigration = vi.spyOn(MobileEndpointHysteresis.prototype, 'recordMigration') - const logical = new FakeLogicalClient('disconnected', 'lan') - const openRelay = vi.fn(() => confirmRejectingRelaySession(logical)) - // No LAN to race: this is about the relay cadence after a confirm failure. - const deps = dependencies({ - openRelay, - openDirect: unreachableDirect(), - randomBytes: () => new Uint8Array([128, 0]) - }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - - await supervisor.start() - // Two sockets per pass: a confirm mismatch reads as a stale cell assignment, so - // the existing director fallback re-resolves and dials the authoritative target. - expect(openRelay).toHaveBeenCalledTimes(2) - expect(logical.migrateTo).toHaveBeenCalledTimes(2) - - // Why: `connected` is published at authentication, so the cutover happens before - // the confirm answers. A confirm that then fails must still book the shared - // cooldown — reporting it as an established dial redials in a tight loop. - await vi.advanceTimersByTimeAsync(0) - expect(openRelay).toHaveBeenCalledTimes(2) - - // 250ms, then 500ms, then 1000ms: the streak grows instead of resetting, which - // it could not do if setActiveSession had run for this dying session. - await vi.advanceTimersByTimeAsync(249) - expect(openRelay).toHaveBeenCalledTimes(2) - await vi.advanceTimersByTimeAsync(1) - expect(openRelay).toHaveBeenCalledTimes(4) - await vi.advanceTimersByTimeAsync(250) - expect(openRelay).toHaveBeenCalledTimes(4) - await vi.advanceTimersByTimeAsync(250) - expect(openRelay).toHaveBeenCalledTimes(6) - await vi.advanceTimersByTimeAsync(999) - expect(openRelay).toHaveBeenCalledTimes(6) - await vi.advanceTimersByTimeAsync(1) - expect(openRelay).toHaveBeenCalledTimes(8) - - // No session whose confirm failed is ever booked as a migration. - expect(recordMigration).not.toHaveBeenCalled() - supervisor.stop() - }) - - it('replays a relay recovery that landed while the direct cutover owned the mutex', async () => { - const logical = new FakeLogicalClient('connected', 'relay') - const openRelay = vi.fn(() => new FakeRelaySession('connected')) - const deps = dependencies({ openDirect: vi.fn(() => new FakeSession('connected')), openRelay }) - const supervisor = new MobileEndpointSupervisor(logical, host, deps) - await supervisor.start() - - let release!: () => void - const cutover = new Promise((resolve) => { - release = resolve - }) - // The candidate loses the cutover, so the logical client stays on the relay path. - logical.migrateTo.mockImplementationOnce(async (candidate) => { - await cutover - candidate.close() - }) - // Three authenticated probes plus the observation and dwell windows. - await vi.advanceTimersByTimeAsync(60_000) - expect(logical.migrateTo).toHaveBeenCalledOnce() - - logical.publishState('disconnected') - await vi.advanceTimersByTimeAsync(0) - expect(openRelay).not.toHaveBeenCalled() - - release() - await vi.advanceTimersByTimeAsync(0) - - // The queued request is replayed by afterProbe, never dropped. - expect(openRelay).toHaveBeenCalledOnce() - expect(logical.getState()).toBe('connected') - supervisor.stop() - }) }) diff --git a/mobile/src/transport/mobile-endpoint-supervisor-test-fakes.ts b/mobile/src/transport/mobile-endpoint-supervisor-test-fakes.ts index c646fc3000c..80f4438c160 100644 --- a/mobile/src/transport/mobile-endpoint-supervisor-test-fakes.ts +++ b/mobile/src/transport/mobile-endpoint-supervisor-test-fakes.ts @@ -65,7 +65,6 @@ export class FakeRelaySession extends FakeSession implements MobileRelayRpcSessi renewed: this.renewed, resumeExpiresAt: this.resumeExpiry }) - whenResumeConfirmed = () => Promise.resolve() getFailure = () => this.failure } @@ -204,15 +203,6 @@ export const bundle: MobileRelayCredentialBundle = { } } -// Why: LAN unreachable. A throwing open beats a never-answering socket — the -// direct dial resolves synchronously, so a relay-only test leaves no probe timer -// behind and the reconnect race has exactly one runner. -export function unreachableDirect(): MobileEndpointSupervisorDependencies['openDirect'] { - return vi.fn(() => { - throw new Error('direct endpoint unreachable') - }) -} - export function dependencies( overrides: Partial = {} ): MobileEndpointSupervisorDependencies { diff --git a/mobile/src/transport/mobile-endpoint-supervisor.test.ts b/mobile/src/transport/mobile-endpoint-supervisor.test.ts index f8f8e50d094..10ef892a479 100644 --- a/mobile/src/transport/mobile-endpoint-supervisor.test.ts +++ b/mobile/src/transport/mobile-endpoint-supervisor.test.ts @@ -10,8 +10,7 @@ import { FakeSession, host, mockCredentialRotation, - relay, - unreachableDirect + relay } from './mobile-endpoint-supervisor-test-fakes' import { MobileEndpointSupervisor } from './mobile-endpoint-supervisor' @@ -49,17 +48,19 @@ describe('mobile endpoint supervisor', () => { supervisor.stop() }) - it('fails over while the direct retry loop is still dialing', async () => { + it('fails over when the direct retry loop publishes reconnecting', async () => { const logical = new FakeLogicalClient('connecting', 'lan') - const deps = dependencies({ openDirect: unreachableDirect() }) + const deps = dependencies() const supervisor = new MobileEndpointSupervisor(logical, host, deps) await supervisor.start() - // An unfinished direct dial no longer holds relay back, so the failover has - // already happened by the time the direct client gives up. logical.publishState('handshaking') await vi.advanceTimersByTimeAsync(0) - expect(deps.openRelay).toHaveBeenCalledOnce() + expect(deps.openRelay).not.toHaveBeenCalled() + + supervisor.setForeground(true) + await vi.advanceTimersByTimeAsync(0) + expect(deps.openRelay).not.toHaveBeenCalled() logical.publishState('reconnecting') await vi.waitFor(() => expect(logical.getActivePath()).toBe('relay')) @@ -147,12 +148,11 @@ describe('mobile endpoint supervisor', () => { expect(logical.getPendingPath()).toBeNull() }) - it('keeps retrying relay on its own cadence while a direct handshake drags on', async () => { + it('does not spend a queued relay retry while direct authentication is progressing', async () => { const logical = new FakeLogicalClient('disconnected', 'lan') const openRelay = vi.fn(() => new FakeRelaySession('disconnected', new RelayOuterError(4408))) const deps = dependencies({ openRelay, - openDirect: unreachableDirect(), randomBytes: () => new Uint8Array([128, 0]) }) const supervisor = new MobileEndpointSupervisor(logical, host, deps) @@ -160,13 +160,12 @@ describe('mobile endpoint supervisor', () => { await supervisor.start() expect(openRelay).toHaveBeenCalledOnce() - // A direct dial that reaches 'handshaking' and stays there used to park relay - // recovery until it gave up; the retry now runs on the failure cadence alone. logical.publishState('handshaking') - await vi.advanceTimersByTimeAsync(249) + await vi.advanceTimersByTimeAsync(250) expect(openRelay).toHaveBeenCalledOnce() - await vi.advanceTimersByTimeAsync(1) - expect(openRelay).toHaveBeenCalledTimes(2) + + logical.publishState('disconnected') + await vi.waitFor(() => expect(openRelay).toHaveBeenCalledTimes(2)) supervisor.stop() }) @@ -190,7 +189,6 @@ describe('mobile endpoint supervisor', () => { resolved, expect.any(Object), expect.any(String), - expect.any(Function), expect.any(Function) ) expect(deps.saveHost).toHaveBeenCalledWith( @@ -245,7 +243,6 @@ describe('mobile endpoint supervisor', () => { const deps = dependencies({ openRelay, onLog, - openDirect: unreachableDirect(), randomBytes: () => new Uint8Array([128, 0]) }) const supervisor = new MobileEndpointSupervisor(logical, host, deps) @@ -290,7 +287,6 @@ describe('mobile endpoint supervisor', () => { const openRelay = vi.fn(() => new FakeRelaySession('connected', new RelayOuterError(4408))) const deps = dependencies({ openRelay, - openDirect: unreachableDirect(), randomBytes: () => new Uint8Array([128, 0]) }) const supervisor = new MobileEndpointSupervisor(logical, host, deps) @@ -473,7 +469,6 @@ describe('mobile endpoint supervisor', () => { .mockImplementation(() => new FakeRelaySession('connected')) const deps = dependencies({ openRelay, - openDirect: unreachableDirect(), writeBundle: vi.fn(() => writePending), randomBytes: () => new Uint8Array([128, 0]) }) @@ -567,7 +562,6 @@ describe('mobile endpoint supervisor', () => { relay, expect.objectContaining({ version: 3 }), expect.any(String), - expect.any(Function), expect.any(Function) ) supervisor.stop() @@ -616,7 +610,6 @@ describe('mobile endpoint supervisor', () => { relay, expect.objectContaining({ version: 3 }), expect.any(String), - expect.any(Function), expect.any(Function) ) supervisor.stop() @@ -812,7 +805,6 @@ describe('mobile endpoint supervisor', () => { .mockImplementation(() => new FakeRelaySession('connected')) const deps = dependencies({ openRelay, - openDirect: unreachableDirect(), randomBytes: () => new Uint8Array([128, 0]) }) const supervisor = new MobileEndpointSupervisor(logical, host, deps) @@ -849,6 +841,43 @@ describe('mobile endpoint supervisor', () => { supervisor.stop() }) + it('races a relay dial when the direct dial stalls unauthenticated', async () => { + const logical = new FakeLogicalClient('connecting', 'lan') + const deps = dependencies() + const supervisor = new MobileEndpointSupervisor(logical, host, deps) + + await supervisor.start() + await vi.advanceTimersByTimeAsync(2_499) + expect(deps.openRelay).not.toHaveBeenCalled() + expect(logical.getState()).toBe('connecting') + + // The direct dial never authenticates; the relay wins the race through migrateTo. + await vi.advanceTimersByTimeAsync(1) + await vi.waitFor(() => expect(logical.getActivePath()).toBe('relay')) + expect(logical.migrateTo).toHaveBeenCalledWith( + expect.any(FakeRelaySession), + 'relay', + undefined, + expect.any(Function) + ) + supervisor.stop() + }) + + it('cancels the grace race when the direct dial authenticates first', async () => { + const logical = new FakeLogicalClient('connecting', 'lan') + const deps = dependencies() + const supervisor = new MobileEndpointSupervisor(logical, host, deps) + + await supervisor.start() + logical.publishState('connected') + expect(vi.getTimerCount()).toBe(0) + + await vi.advanceTimersByTimeAsync(5_000) + expect(deps.openRelay).not.toHaveBeenCalled() + expect(logical.getActivePath()).toBe('lan') + supervisor.stop() + }) + it('never races a relay dial against a desktop with no relay endpoint', async () => { const logical = new FakeLogicalClient('connecting', 'lan') const deps = dependencies() @@ -861,4 +890,39 @@ describe('mobile endpoint supervisor', () => { expect(vi.getTimerCount()).toBe(0) supervisor.stop() }) + + it('drops the pending grace race when the phone backgrounds', async () => { + const logical = new FakeLogicalClient('connecting', 'lan') + const deps = dependencies() + const supervisor = new MobileEndpointSupervisor(logical, host, deps) + + await supervisor.start() + supervisor.setForeground(false) + await vi.advanceTimersByTimeAsync(5_000) + + expect(deps.openRelay).not.toHaveBeenCalled() + expect(vi.getTimerCount()).toBe(0) + supervisor.stop() + }) + + it('books the shared cooldown when the grace race loses its dial', async () => { + const logical = new FakeLogicalClient('connecting', 'lan') + const openRelay = vi.fn(() => new FakeRelaySession('disconnected', new RelayOuterError(4408))) + const deps = dependencies({ openRelay, randomBytes: () => new Uint8Array([128, 0]) }) + const supervisor = new MobileEndpointSupervisor(logical, host, deps) + + await supervisor.start() + await vi.advanceTimersByTimeAsync(2_500) + expect(openRelay).toHaveBeenCalledOnce() + + // The armed retry runs unforced, so it yields to the still-progressing direct + // dial: the race gets one attempt, never a socket-per-cooldown loop. + await vi.advanceTimersByTimeAsync(60_000) + expect(openRelay).toHaveBeenCalledOnce() + + // Direct finally gives up: ordinary recovery still owns the failure. + logical.publishState('reconnecting') + await vi.waitFor(() => expect(openRelay).toHaveBeenCalledTimes(2)) + supervisor.stop() + }) }) diff --git a/mobile/src/transport/mobile-endpoint-supervisor.ts b/mobile/src/transport/mobile-endpoint-supervisor.ts index 450ec656d88..9ba12f35112 100644 --- a/mobile/src/transport/mobile-endpoint-supervisor.ts +++ b/mobile/src/transport/mobile-endpoint-supervisor.ts @@ -10,11 +10,13 @@ import { } from './mobile-endpoint-supervisor-support' import { selectDialableRelayCredentials } from './mobile-relay-credential-selection' import { createRelayRecoveryLog, type RelayRecoveryLog } from './mobile-relay-recovery-log' -import { MobileRelayCredentialRefresh } from './mobile-relay-credential-refresh' +import { + mobileRelayCredentialNeedsRotation, + rotateMobileRelayCredential +} from './mobile-relay-credential-rotation' import type { MobileRelayCredentialBundle } from './mobile-relay-credential-bundle' import { MobileEndpointNudgeRouter } from './mobile-endpoint-nudge-router' -import { RelayRecoveryIntentQueue } from './relay-recovery-intent-queue' -import { RelayLostRaceDamper } from './mobile-relay-lost-race-damper' +import { MobileRelayDirectGraceTimer } from './mobile-relay-direct-grace-timer' import { MobileRelaySessionEstablisher } from './mobile-relay-session-establisher' import * as recoveryPresentation from './mobile-relay-recovery-presentation' import type { StableLogicalRpcClient } from './stable-logical-rpc-client' @@ -36,9 +38,9 @@ export class MobileEndpointSupervisor { private bundle: MobileRelayCredentialBundle | null = null private stopped = false private operationInFlight = false - private readonly pending = new RelayRecoveryIntentQueue() + private pendingReplace = false private readonly nudgeRouter: MobileEndpointNudgeRouter - private readonly credentialRefresh: MobileRelayCredentialRefresh + private credentialRotationInFlight = false private relayRotationPending = false private unsubscribeState: (() => void) | null = null private readonly hysteresis: MobileEndpointHysteresis @@ -46,7 +48,7 @@ export class MobileEndpointSupervisor { private readonly leaseRotation: RelayLeaseRotationTimer private readonly logRelay: RelayRecoveryLog private readonly directProbe: DirectReturnProbe - private readonly lostRace: RelayLostRaceDamper + private readonly directGrace: MobileRelayDirectGraceTimer private readonly backgroundGrace: MobileRelayBackgroundGrace private readonly sessionEstablisher: MobileRelaySessionEstablisher @@ -62,27 +64,6 @@ export class MobileEndpointSupervisor { minimumDwellMs: MINIMUM_DWELL_MS }) this.logRelay = createRelayRecoveryLog(dependencies.now, dependencies.onLog) - this.credentialRefresh = new MobileRelayCredentialRefresh({ - logical, - now: dependencies.now, - randomBytes: dependencies.randomBytes, - writeBundle: dependencies.writeBundle, - bundle: () => this.bundle, - adoptBundle: (bundle) => (this.bundle = bundle), - persistResolvedRelay: async (resolved) => { - this.host = await persistRelayHost(this.host, resolved, dependencies.saveHost) - }, - isStopped: () => this.stopped, - completeRefresh: () => this.relayReconnect.completeCredentialRefresh(), - // Why relayDialAllowed and not the reconnect controller's needsRecovery: a - // refresh that lands while direct is still dialing must start the relay race, - // not wait on the direct retry loop as the pre-race rotation path did. - onRefreshed: () => { - if (this.isActive() && this.relayDialAllowed(false)) { - void this.recoverRelay() - } - } - }) this.relayReconnect = new RelayReconnectController(dependencies, this.recoverRelay.bind(this)) this.relayReconnect.reportRecoveryTo(logical) this.nudgeRouter = new MobileEndpointNudgeRouter({ @@ -92,17 +73,18 @@ export class MobileEndpointSupervisor { isForeground: () => this.backgroundGrace.isForeground(), setForeground: (foreground) => this.setForeground(foreground), replaceRelay: () => void this.recoverRelay(true, true), - scheduleDirectProbe: () => this.directProbe.probeNow() - }) - this.lostRace = new RelayLostRaceDamper(dependencies, () => { - // Why: the window closing is the moment to re-ask. If direct came back the - // guards below no-op; if it never did, relay recovery resumes on its own. - void this.recoverRelay() + scheduleDirectProbe: () => this.directProbe.schedule(0) }) this.leaseRotation = new RelayLeaseRotationTimer(dependencies, () => { this.relayRotationPending = true void this.recoverRelay(true) }) + // Why: the race owns recovery exactly like a network-change replacement — its + // failure must book the shared cooldown. recoverRelay's own guards already + // cover stopped/background/no-relay, so the timer needs no scope check. + this.directGrace = new MobileRelayDirectGraceTimer(dependencies, logical, () => { + void this.recoverRelay(true, true) + }) this.sessionEstablisher = new MobileRelaySessionEstablisher({ logical, controller: this.relayReconnect, @@ -120,7 +102,6 @@ export class MobileEndpointSupervisor { adoptBundle: (bundle) => (this.bundle = bundle), recordMigration: () => { this.relayRotationPending = false - this.lostRace.reset() this.hysteresis.recordMigration(dependencies.now()) logRelayConnected(this.logRelay) }, @@ -137,22 +118,21 @@ export class MobileEndpointSupervisor { hysteresis: this.hysteresis, host: () => this.host, canSchedule: () => this.isActive() && this.logical.getActivePath() === 'relay', - canDial: () => this.isActive(), canAttempt: () => this.isActive() && !this.operationInFlight, - // Why: a reconnect has no session to protect, so the first authenticated - // socket wins it outright — hysteresis only arbitrates against a live relay. - adoptsOutright: () => this.isActive() && this.logical.getState() !== 'connected', beginOperation: () => (this.operationInFlight = true), migrate: (client, path, abort) => this.logical.migrateTo(client, path, undefined, abort), onDirectMigrated: async () => { this.leaseRotation.clear() this.relayRotationPending = false - await this.credentialRefresh.run(this.relayReconnect.resetForDirectConnection()) + await this.rotateCredentialIfNeeded(this.relayReconnect.resetForDirectConnection()) }, afterProbe: () => { this.operationInFlight = false - const queued = this.pending.takeRecovery() || this.pending.hasReplacement() - if (queued || this.relayRotationPending || this.logical.getState() !== 'connected') { + if ( + this.pendingReplace || + this.relayRotationPending || + this.logical.getState() !== 'connected' + ) { void this.recoverRelay(this.relayRotationPending) } } @@ -162,7 +142,8 @@ export class MobileEndpointSupervisor { logical, this.relayReconnect, this.leaseRotation, - this.directProbe + this.directProbe, + this.directGrace ) } @@ -178,18 +159,12 @@ export class MobileEndpointSupervisor { } this.unsubscribeState = this.logical.onStateChange((state) => { if (state === 'connected') { - this.lostRace.noteDirectRestored() + this.directGrace.clear() if (this.logical.getActivePath() !== 'relay') { - void this.credentialRefresh.run(this.relayReconnect.resetForDirectConnection()) + void this.rotateCredentialIfNeeded(this.relayReconnect.resetForDirectConnection()) } this.directProbe.schedule() - return - } - // Why: the path that won the last race is gone, so the window it earned - // must not be served out — a blip that became an outage would otherwise - // strand the user for the whole window with nothing else scheduled. - this.lostRace.clampForLostDirect() - if (!this.backgroundGrace.isForeground()) { + } else if (!this.backgroundGrace.isForeground()) { this.backgroundGrace.handleStateFailure() } else { // Why: the direct client enters reconnecting after its first failed @@ -199,21 +174,17 @@ export class MobileEndpointSupervisor { logRelayDialFailure(this.logRelay, relayFailure, 'active-session') } }) - if (this.logical.getState() === 'connected') { + if (this.relayReconnect.needsRecovery(this.logical.getState())) { + // Why: the first direct dial can fail while encrypted relay credentials + // are still loading, before the supervisor subscribes to state changes. + await this.recoverRelay() + } else { this.directProbe.schedule() - return + this.directGrace.arm() } - // Why: nothing is live, so both paths dial from t=0. This also covers the - // first direct dial failing while encrypted relay credentials are still - // loading, before the supervisor subscribes to state changes. - await this.recoverRelay() } setForeground(foreground: boolean): void { - if (foreground) { - // Why: a resume is the user waiting on the screen, never a blip. - this.lostRace.reset() - } this.backgroundGrace.setForeground(foreground) if (foreground && this.relayRotationPending) { void this.recoverRelay(true) @@ -224,8 +195,6 @@ export class MobileEndpointSupervisor { stop(): void { this.stopped = true - this.pending.clear() - this.lostRace.reset() this.directProbe.stop() this.unsubscribeState?.() this.unsubscribeState = null @@ -236,15 +205,8 @@ export class MobileEndpointSupervisor { return !this.stopped && this.backgroundGrace.isForeground() } - // Why: the relay dial yields to a live session and to nothing else. An - // unfinished direct dial ('connecting'/'handshaking') used to block it behind a - // fixed head start, which bought an off-LAN phone nothing on every reconnect. - private relayDialAllowed(forceReplacement: boolean): boolean { - return forceReplacement || this.logical.getState() !== 'connected' - } - - // forceReplacement: dial past the "a live session already holds the client" - // guard — a lease rotation or a network-change replacement. + // forceReplacement: dial past the "direct still looks live" guard — a lease + // rotation, a network-change replacement, or the happy-eyeballs grace race. // ownsRecovery: this dial is the connection's only hope, so a failure books the // shared cooldown and any session left stale-'connected' by a half-open socket // comes down; lease rotation clears it because armRetry owns its own retry. @@ -252,30 +214,20 @@ export class MobileEndpointSupervisor { if (!this.isActive() || !this.host.relay) { return } - if (this.logical.getState() !== 'connected') { - // Why: both paths race from t=0. This no-ops unless relay owns the logical - // client — when direct owns it, its own session is already redialing. - this.directProbe.probeNow() - } if (this.operationInFlight) { - // Why: a direct cutover or a slow post-migration write can own the mutex when - // a handoff lands. Every request is queued — an owning replacement keeps its - // force/owns intent, anything else replays as a plain recovery — so the - // holder's release replays it instead of dropping it. - this.pending.queue(forceReplacement, ownsRecovery) + // Why: a 12s direct probe can own the mutex when a network handoff lands; + // afterProbe replays the queued replacement so the signal is never lost. + this.pendingReplace ||= forceReplacement && ownsRecovery return } - if (this.pending.takeReplacement()) { + if (this.pendingReplace) { + this.pendingReplace = false forceReplacement = true ownsRecovery = true } - if (!this.relayDialAllowed(forceReplacement)) { - return - } - if (!forceReplacement && this.lostRace.suppresses()) { - // Why: the previous race was lost to direct and booked nothing, so only - // this damper stands between a flapping LAN and a cell socket per blip. - this.logRelay('relay race damped after losing to direct') + // Why: connecting/handshaking is live direct progress; an unforced relay dial + // would race it before the grace timer has given direct its head start. + if (!forceReplacement && !this.relayReconnect.needsRecovery(this.logical.getState())) { return } // Why: revival and lease timers can overlap resume failures; one shared cooldown @@ -284,7 +236,7 @@ export class MobileEndpointSupervisor { if (ownsRecovery) { // Why: never tear down a session no dial has disproven — the intent stays // queued so the armed retry runs forced once the cooldown lapses. - this.pending.holdReplacement() + this.pendingReplace = true } this.logRelay('recovery deferred by cooldown or gate') return @@ -308,18 +260,20 @@ export class MobileEndpointSupervisor { if (ownsRecovery) { // Why: no dial happened — keep the session and the intent; the reprobe // runs forced and replaces make-before-break once a credential exists. - this.pending.holdReplacement() + this.pendingReplace = true } return } - if (!this.isActive() || !this.relayDialAllowed(forceReplacement)) { + const recoveryNeeded = + forceReplacement || this.relayReconnect.needsRecovery(this.logical.getState()) + if (!this.isActive() || !recoveryNeeded) { return } this.logical.setRecoveryPath('relay', this.relayReconnect.getFailureCount()) const dialed = await this.sessionEstablisher.dialEligible(selection.credentials) if (dialed.outcome === 'established') { // Why: a fresh socket satisfies any replacement intent queued mid-dial. - this.pending.clearReplacement() + this.pendingReplace = false retryAfterOperation = this.logical.getState() !== 'connected' return } @@ -327,12 +281,6 @@ export class MobileEndpointSupervisor { this.logical.setRecoveryPath(null) // Why: direct won the race or the supervisor went inactive — not a // failure; booking backoff would delay the next genuine recovery. - // Why: only an unforced race can be blip-driven. A forced replacement - // that stands down is a lease rotation or a network change reconsidered, - // not a LAN that flapped, so it must not grow the streak. - if (!forceReplacement && this.isActive() && this.logical.getState() === 'connected') { - this.lostRace.record() - } return } // Why: cleanup may happen while a relay dial is awaiting the network; @@ -345,12 +293,52 @@ export class MobileEndpointSupervisor { } } finally { this.operationInFlight = false - const queued = this.pending.takeRecovery() if (forceReplacement && this.relayRotationPending && this.isActive()) { this.leaseRotation.armRetry(this.relayReconnect.retryDelayMs(5000)) } // Why: the active relay can drop while migration follow-up still owns the mutex. - if ((retryAfterOperation || queued) && this.isActive()) { + if (retryAfterOperation && this.isActive()) { + void this.recoverRelay() + } + } + } + + private async rotateCredentialIfNeeded(force = false): Promise { + if ( + this.stopped || + this.credentialRotationInFlight || + !this.bundle || + this.logical.getActivePath() === 'relay' || + (!force && !mobileRelayCredentialNeedsRotation(this.bundle, this.dependencies.now())) + ) { + return + } + this.credentialRotationInFlight = true + let credentialRefreshed = false + try { + const result = await rotateMobileRelayCredential({ + client: this.logical, + bundle: this.bundle, + writeBundle: this.dependencies.writeBundle, + randomBytes: this.dependencies.randomBytes + }) + this.bundle = result.bundle + // Why: a scheduled rotation can finish after the old credential enters the rejection gate. + credentialRefreshed = true + this.host = await persistRelayHost(this.host, result.relay, this.dependencies.saveHost) + } catch { + // Why: pending material remains durable; the next authenticated direct + // opportunity must reconcile it before creating another install key. + } finally { + if (credentialRefreshed) { + this.relayReconnect.completeCredentialRefresh() + } + this.credentialRotationInFlight = false + if ( + credentialRefreshed && + this.isActive() && + this.relayReconnect.needsRecovery(this.logical.getState()) + ) { void this.recoverRelay() } } diff --git a/mobile/src/transport/mobile-relay-background-grace.ts b/mobile/src/transport/mobile-relay-background-grace.ts index 061c29a15ed..cdea1374b4f 100644 --- a/mobile/src/transport/mobile-relay-background-grace.ts +++ b/mobile/src/transport/mobile-relay-background-grace.ts @@ -51,7 +51,8 @@ export class MobileRelayBackgroundGraceTimer { } type Clearable = { clear(): void } -type DirectProbe = Clearable & { schedule(delayMs?: number): void; probeNow(): void } +type DirectProbe = Clearable & { schedule(delayMs?: number): void } +type DirectGrace = Clearable & { arm(): void } export class MobileRelayBackgroundGrace { private foregroundState = true @@ -63,7 +64,8 @@ export class MobileRelayBackgroundGrace { private readonly logical: StableLogicalRpcClient, private readonly relayReconnect: RelayReconnectController, private readonly leaseRotation: Clearable, - private readonly directProbe: DirectProbe + private readonly directProbe: DirectProbe, + private readonly directGrace: DirectGrace ) { this.timer = new MobileRelayBackgroundGraceTimer(dependencies, () => this.suspendRelay()) } @@ -78,9 +80,8 @@ export class MobileRelayBackgroundGrace { if (foreground) { this.foreground() this.relayReconnect.handleForeground(this.logical, wasForeground) - // Why: a resume dials direct alongside the relay recovery handleForeground - // just triggered; a pending probe tick must not delay this one. - this.directProbe.probeNow() + this.directProbe.schedule(0) + this.directGrace.arm() } else if (wasForeground) { this.background() } @@ -91,6 +92,7 @@ export class MobileRelayBackgroundGrace { this.directProbe.clear() this.relayReconnect.clear() this.leaseRotation.clear() + this.directGrace.clear() this.logical.setRecoveryPath(null) } @@ -106,6 +108,7 @@ export class MobileRelayBackgroundGrace { const retainsRelay = this.logical.getActivePath() === 'relay' && this.logical.getState() === 'connected' this.directProbe.clear() + this.directGrace.clear() this.logical.setRecoveryPath(null) if (retainsRelay) { this.timer.arm() diff --git a/mobile/src/transport/mobile-relay-credential-refresh.ts b/mobile/src/transport/mobile-relay-credential-refresh.ts deleted file mode 100644 index 647a12a423f..00000000000 --- a/mobile/src/transport/mobile-relay-credential-refresh.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { - mobileRelayCredentialNeedsRotation, - rotateMobileRelayCredential -} from './mobile-relay-credential-rotation' -import type { MobileRelayCredentialBundle } from './mobile-relay-credential-bundle' -import type { MobileRelayEndpoint } from '../../../src/shared/mobile-relay-credential-contract' -import type { StableLogicalRpcClient } from './stable-logical-rpc-client' - -// Mints a replacement relay credential over a live direct connection. That is the -// only moment it can happen: the replacement comes from an authenticated RPC, and -// a phone whose credential the relay has rejected cannot carry one over relay. -export class MobileRelayCredentialRefresh { - private inFlight = false - - constructor( - private readonly args: { - logical: StableLogicalRpcClient - now: () => number - randomBytes: (length: number) => Uint8Array - writeBundle: (bundle: MobileRelayCredentialBundle) => Promise - bundle: () => MobileRelayCredentialBundle | null - adoptBundle: (bundle: MobileRelayCredentialBundle) => void - persistResolvedRelay: (resolved: MobileRelayEndpoint) => Promise - isStopped: () => boolean - // Lifts the controller's fresh-credential gate once the replacement is durable. - completeRefresh: () => void - onRefreshed: () => void - } - ) {} - - // force: the caller already knows the current credential is rejected, so the - // age check would only delay a rotation the relay path is blocked on. - async run(force: boolean): Promise { - const { args } = this - const bundle = args.bundle() - if ( - args.isStopped() || - this.inFlight || - !bundle || - args.logical.getActivePath() === 'relay' || - (!force && !mobileRelayCredentialNeedsRotation(bundle, args.now())) - ) { - return - } - this.inFlight = true - let refreshed = false - try { - const result = await rotateMobileRelayCredential({ - client: args.logical, - bundle, - writeBundle: args.writeBundle, - randomBytes: args.randomBytes - }) - args.adoptBundle(result.bundle) - // Why: a scheduled rotation can finish after the old credential enters the rejection gate. - refreshed = true - await args.persistResolvedRelay(result.relay) - } catch { - // Why: pending material remains durable; the next authenticated direct - // opportunity must reconcile it before creating another install key. - } finally { - if (refreshed) { - args.completeRefresh() - } - this.inFlight = false - if (refreshed) { - args.onRefreshed() - } - } - } -} diff --git a/mobile/src/transport/mobile-relay-credential-rotation.ts b/mobile/src/transport/mobile-relay-credential-rotation.ts index ef2630c8a67..9b8a038e8e4 100644 --- a/mobile/src/transport/mobile-relay-credential-rotation.ts +++ b/mobile/src/transport/mobile-relay-credential-rotation.ts @@ -142,15 +142,11 @@ export async function persistResumeConfirmation(args: { session: { getResumeConfirmation(): DeviceResumeConfirmed | null getResumeExpiresAt(): number | null - whenResumeConfirmed(): Promise } bundle: MobileRelayCredentialBundle usedCredentialVersion: number writeBundle: (bundle: MobileRelayCredentialBundle) => Promise }): Promise<{ bundle: MobileRelayCredentialBundle; leaseExpiry: number | null }> { - // Why: 'connected' is published at E2EE authentication now, so the confirm round - // trip can still be in flight here — its answer is what makes the bundle durable. - await args.session.whenResumeConfirmed() const confirmation = args.session.getResumeConfirmation() let bundle = args.bundle if (confirmation) { diff --git a/mobile/src/transport/mobile-relay-direct-grace-timer.ts b/mobile/src/transport/mobile-relay-direct-grace-timer.ts new file mode 100644 index 00000000000..df3c1ba1428 --- /dev/null +++ b/mobile/src/transport/mobile-relay-direct-grace-timer.ts @@ -0,0 +1,48 @@ +import type { StableLogicalRpcClient } from './stable-logical-rpc-client' + +// Why: on a black-holed LAN endpoint the direct dial sits in 'connecting' for the +// whole 12s connect timeout (rpc-client CONNECT_TIMEOUT_MS), and relay recovery +// cannot even start meanwhile because connecting/handshaking count as live direct +// progress. Happy eyeballs: give direct this much of a head start, then race the +// relay dial — migrateTo hands the logical client to whichever authenticates first. +const DIRECT_DIAL_GRACE_MS = 2500 + +type DirectGraceTimerDependencies = { + setTimer: typeof setTimeout + clearTimer: typeof clearTimeout +} + +// One-shot timer that releases the relay dial when the direct dial has not +// authenticated within the grace. The supervisor arms it at start and on +// foreground restore, and clears it on connect, background, and stop. +export class MobileRelayDirectGraceTimer { + private timer: ReturnType | null = null + + constructor( + private readonly dependencies: DirectGraceTimerDependencies, + private readonly logical: StableLogicalRpcClient, + private readonly dialRelay: () => void + ) {} + + // No-op unless the direct dial is still unauthenticated, so a healthy LAN and + // an already-failed direct path (recovery owns that) never open a relay socket. + arm(): void { + const state = this.logical.getState() + if (this.timer || (state !== 'connecting' && state !== 'handshaking')) { + return + } + this.timer = this.dependencies.setTimer(() => { + this.timer = null + if (this.logical.getState() !== 'connected') { + this.dialRelay() + } + }, DIRECT_DIAL_GRACE_MS) + } + + clear(): void { + if (this.timer) { + this.dependencies.clearTimer(this.timer) + this.timer = null + } + } +} diff --git a/mobile/src/transport/mobile-relay-e2ee-link.test.ts b/mobile/src/transport/mobile-relay-e2ee-link.test.ts index 6f8f0a6d9b4..965135511eb 100644 --- a/mobile/src/transport/mobile-relay-e2ee-link.test.ts +++ b/mobile/src/transport/mobile-relay-e2ee-link.test.ts @@ -195,38 +195,6 @@ describe('MobileRelayE2eeLink', () => { } }) - it('writes no e2ee frame when withdrawn between relay-auth and the hello', () => { - const socket = new ThrowingSocket() - const sent: string[] = [] - socket.send.mockImplementation((frame: string) => { - sent.push(frame) - }) - const link = new MobileRelayE2eeLink({ - endpoint: { - cellUrl: 'https://relay-c1.onorca.dev', - relayHostId: 'AbCdEf0123_-xyZ9' - }, - credential: 'credential', - expectedCredentialKind: 'resume', - deviceToken: 'device-token', - desktopPublicKeyB64: 'desktop-key', - onAuthenticated: vi.fn(), - onText: vi.fn(), - onBinary: vi.fn(), - onError: vi.fn(), - createSocket: () => socket as unknown as WebSocket - }) - socket.onopen?.() - - // The window a lost reconnect race is withdrawn in: the cell has the outer - // credential but has not answered, so no key exchange has started. - link.close() - - expect(sent).toHaveLength(1) - expect(JSON.parse(sent[0]!)).toMatchObject({ type: 'relay-auth' }) - expect(socket.close).toHaveBeenCalledOnce() - }) - it('cancels the missing-close timer when explicitly closed', async () => { vi.useFakeTimers() try { diff --git a/mobile/src/transport/mobile-relay-lost-race-damper.ts b/mobile/src/transport/mobile-relay-lost-race-damper.ts deleted file mode 100644 index b9891b8481e..00000000000 --- a/mobile/src/transport/mobile-relay-lost-race-damper.ts +++ /dev/null @@ -1,99 +0,0 @@ -// Paces the direct-vs-relay reconnect race after the relay dial loses it. A lost -// race books no failure — that is deliberate, since losing is the good outcome — -// so nothing else stops a flapping LAN from opening one cell socket per blip, and -// the relay's per-host rate limiter would eventually turn a benign race into a -// booked relay failure. This is not backoff: it never delays the failure path, -// and its window lapse re-enters recovery so a LAN that dies mid-window still -// reaches relay on its own. -const INITIAL_DAMP_MS = 2_000 -const MAX_DAMP_MS = 30_000 -// How long a lost direct path is given to prove it was only a blip. Long enough -// to absorb one that drops and comes straight back, short enough that a real -// outage never reads as the connection being stuck. -const LOST_DIRECT_FLOOR_MS = 250 - -type LostRaceDamperDependencies = { - now: () => number - setTimer: typeof setTimeout - clearTimer: typeof clearTimeout -} - -export class RelayLostRaceDamper { - private windowMs = 0 - private suppressUntil = 0 - // The window held aside while a lost direct path proves whether it was a blip. - private pendingUntil = 0 - private timer: ReturnType | null = null - - constructor( - private readonly dependencies: LostRaceDamperDependencies, - private readonly onWindowLapse: () => void - ) {} - - suppresses(): boolean { - return this.dependencies.now() < this.suppressUntil - } - - // Each successive loss inside the window doubles it, so a LAN that flaps all - // afternoon settles at one race per 30s instead of one per blip. - record(): void { - this.windowMs = this.windowMs === 0 ? INITIAL_DAMP_MS : Math.min(this.windowMs * 2, MAX_DAMP_MS) - this.suppressUntil = this.dependencies.now() + this.windowMs - this.arm(this.windowMs) - } - - // The direct path that won the last race is gone. Collapse the wait to the - // floor, so an outage is never held off for the window a blip earned, and keep - // the rest of that window aside rather than spending it: one blip must not buy - // a flapping LAN a free pass on every race that follows. - clampForLostDirect(): void { - const floorAt = this.dependencies.now() + LOST_DIRECT_FLOOR_MS - if (this.suppressUntil === 0 || this.pendingUntil !== 0 || this.suppressUntil <= floorAt) { - return - } - this.pendingUntil = this.suppressUntil - this.suppressUntil = floorAt - this.arm(LOST_DIRECT_FLOOR_MS) - } - - // Direct came back inside the floor, so that was the blip this exists for and - // the rest of the window still has to run. - noteDirectRestored(): void { - if (this.pendingUntil === 0) { - return - } - this.suppressUntil = this.pendingUntil - this.pendingUntil = 0 - this.arm(Math.max(0, this.suppressUntil - this.dependencies.now())) - } - - // A relay dial that wins, or the user bringing the app back, ends the streak: - // neither is a blip, and a resume must never wait out a damper window. A relay - // failure deliberately does not — it is not evidence the LAN stopped flapping, - // and its own cooldown runs after this window rather than on top of it, since - // a damped attempt never reaches the dial that would book one. - reset(): void { - this.windowMs = 0 - this.suppressUntil = 0 - this.pendingUntil = 0 - this.clearTimer() - } - - private arm(delayMs: number): void { - this.clearTimer() - this.timer = this.dependencies.setTimer(() => { - this.timer = null - // Why: the floor lapsed with direct still gone, so it was an outage and the - // window held aside is void — a later return must not resurrect it. - this.pendingUntil = 0 - this.onWindowLapse() - }, delayMs) - } - - private clearTimer(): void { - if (this.timer) { - this.dependencies.clearTimer(this.timer) - this.timer = null - } - } -} diff --git a/mobile/src/transport/mobile-relay-rpc-session-liveness.test.ts b/mobile/src/transport/mobile-relay-rpc-session-liveness.test.ts index 958b9e14813..b811721e562 100644 --- a/mobile/src/transport/mobile-relay-rpc-session-liveness.test.ts +++ b/mobile/src/transport/mobile-relay-rpc-session-liveness.test.ts @@ -32,10 +32,7 @@ const relay = { e2eeFraming: 2 as const } -async function authenticateSession( - onLog?: ConnectionLogSink, - isForeground: () => boolean = () => true -) { +async function authenticateSession(onLog?: ConnectionLogSink) { const session = connectMobileRelayRpcSession({ relay, resumeToken: 'resume-secret', @@ -44,7 +41,6 @@ async function authenticateSession( deviceToken: 'device-token', desktopPublicKeyB64: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=', requestTimeoutMs: 30_000, - isForeground, onLog }) fakes.linkOptions!.onHello({ @@ -56,12 +52,12 @@ async function authenticateSession( acceptedAs: 'current', resumeExpiresAt: Date.now() + 300_000 }) - // Authentication publishes 'connected' and puts both advisories on the wire. fakes.linkOptions!.onAuthenticated() - const [confirmation, capabilities] = sentRequests() + await vi.waitFor(() => expect(fakes.sendText).toHaveBeenCalledOnce()) + const confirmation = sentRequests()[0]! fakes.linkOptions!.onText( JSON.stringify({ - id: confirmation!.id, + id: confirmation.id, ok: true, result: { v: 1, @@ -78,16 +74,17 @@ async function authenticateSession( _meta: { runtimeId: 'runtime-1' } }) ) + await vi.waitFor(() => expect(fakes.sendText).toHaveBeenCalledTimes(2)) + const capabilities = sentRequests()[1]! fakes.linkOptions!.onText( JSON.stringify({ - id: capabilities!.id, + id: capabilities.id, ok: true, result: {}, _meta: { runtimeId: 'runtime-1' } }) ) - await session.whenResumeConfirmed() - expect(session.getState()).toBe('connected') + await vi.waitFor(() => expect(session.getState()).toBe('connected')) fakes.sendText.mockClear() return session } @@ -98,13 +95,6 @@ function sentRequests(): Array<{ id: string; method: string }> { ) } -function answerProbe(): void { - const probe = sentRequests().at(-1)! - fakes.linkOptions!.onText( - JSON.stringify({ id: probe.id, ok: true, result: {}, _meta: { runtimeId: 'r1' } }) - ) -} - describe('mobile relay RPC session liveness', () => { beforeEach(() => { vi.useFakeTimers() @@ -114,82 +104,16 @@ describe('mobile relay RPC session liveness', () => { }) afterEach(() => vi.useRealTimers()) - it('sweeps an idle foregrounded relay once per idle interval', async () => { + it('sends no periodic traffic while an authenticated relay is idle', async () => { const session = await authenticateSession() - await vi.advanceTimersByTimeAsync(24_999) - expect(fakes.sendText).not.toHaveBeenCalled() - await vi.advanceTimersByTimeAsync(1) - expect(sentRequests().map(({ method }) => method)).toEqual(['status.get']) - answerProbe() - - // Inbound traffic re-arms the sweep rather than stacking probes on it. - await vi.advanceTimersByTimeAsync(24_999) - expect(fakes.sendText).toHaveBeenCalledOnce() - await vi.advanceTimersByTimeAsync(1) - expect(fakes.sendText).toHaveBeenCalledTimes(2) - expect(session.getState()).toBe('connected') - session.close() - }) - - it('spends no idle probe while the app is backgrounded', async () => { - let foreground = true - const session = await authenticateSession(undefined, () => foreground) - foreground = false - - await vi.advanceTimersByTimeAsync(120_000) + await vi.advanceTimersByTimeAsync(60_000) expect(fakes.sendText).not.toHaveBeenCalled() expect(session.getState()).toBe('connected') - - // The resume that follows probes at once instead of waiting out the sweep. - foreground = true - session.notifyForeground('app-resume') - expect(sentRequests().map(({ method }) => method)).toEqual(['status.get']) session.close() }) - it('terminates a relay whose socket died in the background on two 2s resume misses', async () => { - const onLog = vi.fn() - const session = await authenticateSession(onLog) - - session.notifyForeground('app-resume') - expect(fakes.sendText).toHaveBeenCalledOnce() - // Why: the first frame after a resume rides a cold radio, so one slow answer is - // tolerated — but the verdict still lands at 4s instead of the old 8s. - await vi.advanceTimersByTimeAsync(2_000) - expect(session.getState()).toBe('connected') - expect(fakes.sendText).toHaveBeenCalledTimes(2) - await vi.advanceTimersByTimeAsync(1_999) - expect(session.getState()).toBe('connected') - await vi.advanceTimersByTimeAsync(1) - - expect(session.getState()).toBe('disconnected') - expect(fakes.close).toHaveBeenCalledOnce() - expect(onLog).toHaveBeenCalledWith( - expect.objectContaining({ - code: 'liveness-timeout', - detail: expect.stringMatching(/^probe-timeout; 2\/2 probes missed;/) - }) - ) - }) - - it('still terminates a dead relay when the log sink throws on the timeout line', async () => { - const onLog = vi.fn(() => { - throw new Error('sink exploded') - }) - const session = await authenticateSession(onLog) - - session.notifyForeground('focus') - await vi.advanceTimersByTimeAsync(4_000) - await vi.advanceTimersByTimeAsync(4_000) - - // The line was attempted and threw; the session still came down. - expect(onLog).toHaveBeenCalledWith(expect.objectContaining({ code: 'liveness-timeout' })) - expect(session.getState()).toBe('disconnected') - expect(fakes.close).toHaveBeenCalledOnce() - }) - it('disconnects after two fair foreground misses', async () => { const onLog = vi.fn() const session = await authenticateSession(onLog) @@ -237,25 +161,22 @@ describe('mobile relay RPC session liveness', () => { expect(secondId).not.toBe(firstId) }) - it('rate-limits focus nudges but never an app resume', async () => { + it('rate-limits foreground sequences without suppressing a retry', async () => { const session = await authenticateSession() session.notifyForeground('focus') - answerProbe() + const firstProbe = sentRequests()[0]! + fakes.linkOptions!.onText( + JSON.stringify({ id: firstProbe.id, ok: true, result: {}, _meta: { runtimeId: 'r1' } }) + ) session.notifyForeground('focus') await vi.advanceTimersByTimeAsync(9_999) - expect(fakes.sendText).toHaveBeenCalledOnce() - - // The resume owns the only evidence that the suspended socket is still alive. session.notifyForeground('app-resume') - expect(fakes.sendText).toHaveBeenCalledTimes(2) - answerProbe() - session.notifyForeground('focus') - expect(fakes.sendText).toHaveBeenCalledTimes(2) - await vi.advanceTimersByTimeAsync(10_000) + expect(fakes.sendText).toHaveBeenCalledOnce() + await vi.advanceTimersByTimeAsync(1) session.notifyForeground('focus') - expect(fakes.sendText).toHaveBeenCalledTimes(3) + expect(fakes.sendText).toHaveBeenCalledTimes(2) session.close() }) @@ -268,9 +189,9 @@ describe('mobile relay RPC session liveness', () => { session.close() }) - it('does not probe when work follows inbound silence', async () => { + it('does not probe when work follows prolonged inbound silence', async () => { const session = await authenticateSession() - await vi.advanceTimersByTimeAsync(20_000) + await vi.advanceTimersByTimeAsync(60_000) const pending = session.sendRequest('terminal.send', { terminal: 'term', text: 'hi' }) const outcome = pending.catch(() => undefined) diff --git a/mobile/src/transport/mobile-relay-rpc-session.test.ts b/mobile/src/transport/mobile-relay-rpc-session.test.ts index 05ffce0f1e8..4bf617faf50 100644 --- a/mobile/src/transport/mobile-relay-rpc-session.test.ts +++ b/mobile/src/transport/mobile-relay-rpc-session.test.ts @@ -22,10 +22,6 @@ const fakes = vi.hoisted(() => ({ close: vi.fn() })) -vi.mock('react-native', () => ({ Platform: { OS: 'ios' } })) -vi.mock('expo-secure-store', () => ({ WHEN_UNLOCKED_THIS_DEVICE_ONLY: 'when-unlocked' })) -vi.mock('expo-crypto', () => ({ getRandomBytes: (length: number) => new Uint8Array(length) })) - vi.mock('./mobile-relay-e2ee-link', () => ({ MobileRelayE2eeLink: class { constructor(options: NonNullable) { @@ -37,8 +33,6 @@ vi.mock('./mobile-relay-e2ee-link', () => ({ })) import { connectMobileRelayRpcSession } from './mobile-relay-rpc-session' -import { persistResumeConfirmation } from './mobile-relay-credential-rotation' -import type { MobileRelayCredentialBundle } from './mobile-relay-credential-bundle' const relay = { v: 1 as const, @@ -49,13 +43,6 @@ const relay = { e2eeFraming: 2 as const } -type SentRequest = { - id: string - method: string - deviceToken: string - params: Record | undefined -} - function openSession() { return connectMobileRelayRpcSession({ relay, @@ -68,11 +55,8 @@ function openSession() { }) } -function sentRequests(): SentRequest[] { - return fakes.sendText.mock.calls.map(([value]) => JSON.parse(value as string) as SentRequest) -} - -function receiveHello(): void { +async function confirmResume() { + const session = openSession() fakes.linkOptions!.onHello({ type: 'relay-hello', ok: true, @@ -82,31 +66,21 @@ function receiveHello(): void { acceptedAs: 'current', resumeExpiresAt: Date.now() + 300_000 }) -} - -// E2EE authentication alone publishes 'connected'; the confirm and the capability -// advisory are already on the wire by the time it returns. -function authenticateSession() { - const session = openSession() - receiveHello() expect(session.getState()).toBe('handshaking') fakes.linkOptions!.onAuthenticated() - const [confirmationRequest, capabilityRequest] = sentRequests() - return { - session, - confirmationRequest: confirmationRequest!, - capabilityRequest: capabilityRequest! + await vi.waitFor(() => expect(fakes.sendText).toHaveBeenCalledOnce()) + const request = JSON.parse(fakes.sendText.mock.calls[0]![0] as string) as { + id: string + method: string + params: unknown } -} - -function answerConfirm(request: SentRequest, relayHostId = relay.relayHostId): void { fakes.linkOptions!.onText( JSON.stringify({ id: request.id, ok: true, result: { v: 1, - relay: { ...relay, relayHostId }, + relay, resumeConfirmation: { v: 1, reqId: 'confirm-1', @@ -119,32 +93,39 @@ function answerConfirm(request: SentRequest, relayHostId = relay.relayHostId): v _meta: { runtimeId: 'runtime-1' } }) ) + await vi.waitFor(() => expect(fakes.sendText).toHaveBeenCalledTimes(2)) + const capabilityRequest = JSON.parse(fakes.sendText.mock.calls[1]![0] as string) as { + id: string + method: string + deviceToken: string + params: { clientCapabilities?: string[] } + } + return { session, confirmationRequest: request, capabilityRequest } } -function answerCapability(request: SentRequest, supported = true): void { +async function authenticateSession(capabilitySupported = true) { + const { session, confirmationRequest, capabilityRequest } = await confirmResume() + expect(session.getState()).toBe('handshaking') fakes.linkOptions!.onText( JSON.stringify( - supported - ? { id: request.id, ok: true, result: request.params, _meta: { runtimeId: 'runtime-1' } } + capabilitySupported + ? { + id: capabilityRequest.id, + ok: true, + result: capabilityRequest.params, + _meta: { runtimeId: 'runtime-1' } + } : { - id: request.id, + id: capabilityRequest.id, ok: false, error: { code: 'method_not_found', message: 'Unknown method' }, _meta: { runtimeId: 'runtime-1' } } ) ) -} - -// Both advisories answered and the send log cleared, so a test can read its own frames. -async function settledSession(capabilitySupported = true) { - const authenticated = authenticateSession() - answerConfirm(authenticated.confirmationRequest) - answerCapability(authenticated.capabilityRequest, capabilitySupported) - await authenticated.session.whenResumeConfirmed() - expect(authenticated.session.getState()).toBe('connected') + await vi.waitFor(() => expect(session.getState()).toBe('connected')) fakes.sendText.mockClear() - return authenticated + return { session, confirmationRequest, capabilityRequest } } describe('mobile relay RPC session', () => { @@ -156,7 +137,7 @@ describe('mobile relay RPC session', () => { afterEach(() => vi.useRealTimers()) it('releases stream listeners on failure even when close follows it', async () => { - const { session } = await settledSession() + const { session } = await authenticateSession() const listener = vi.fn() session.subscribe('runtime.clientEvents.subscribe', {}, listener) await Promise.resolve() @@ -185,8 +166,8 @@ describe('mobile relay RPC session', () => { expect(listener).toHaveBeenCalledTimes(1) }) - it('sends the resume confirm by request ID and the capability advisory concurrently', async () => { - const { session, confirmationRequest, capabilityRequest } = await settledSession() + it('requires exact resume observations and confirms by request ID before becoming connected', async () => { + const { session, confirmationRequest, capabilityRequest } = await authenticateSession() expect(fakes.linkOptions).toMatchObject({ endpoint: relay, @@ -211,103 +192,21 @@ describe('mobile relay RPC session', () => { }) it('connects when an older runtime rejects capability negotiation', async () => { - const { session } = await settledSession(false) + const { session } = await authenticateSession(false) expect(session.getState()).toBe('connected') expect(session.getFailure()).toBeNull() }) it('connects when the relay never answers capability negotiation', async () => { - const { session, confirmationRequest } = authenticateSession() - answerConfirm(confirmationRequest) + const { session } = await confirmResume() - // Why: the advisory's own deadline used to fail the confirm, so a link too slow to + // Why: the advisory's own deadline used to fail confirmResume, so a link too slow to // answer within the request timeout never published 'connected' — it just redialled. - await session.whenResumeConfirmed() - expect(session.getState()).toBe('connected') + await vi.waitFor(() => expect(session.getState()).toBe('connected'), { timeout: 5_000 }) expect(session.getFailure()).toBeNull() }) - it('publishes connected at authentication, ahead of the confirm answer', async () => { - const states: string[] = [] - const session = openSession() - session.onStateChange((state) => states.push(state)) - receiveHello() - fakes.linkOptions!.onAuthenticated() - - // Why: the transport carries traffic from here; two serialized advisory round - // trips used to add ~200ms to every phone reconnect before anything rendered. - expect(session.getState()).toBe('connected') - expect(states).toEqual(['handshaking', 'connected']) - expect(session.getResumeConfirmation()).toBeNull() - expect(sentRequests().map(({ method }) => method)).toEqual([ - 'pairing.getEndpoints', - 'runtime.clientCapabilities.update' - ]) - - const [confirmationRequest] = sentRequests() - answerConfirm(confirmationRequest!) - await session.whenResumeConfirmed() - expect(session.getResumeConfirmation()).toMatchObject({ reqId: 'confirm-1' }) - session.close() - }) - - it('fails a session whose confirm answers for another relay host after connected', async () => { - const { session, confirmationRequest } = authenticateSession() - expect(session.getState()).toBe('connected') - - answerConfirm(confirmationRequest, 'ZZZZZZZZZZZZZZZZ') - await session.whenResumeConfirmed() - - // A late failure is fine; a lost one is not. - expect(session.getState()).toBe('disconnected') - expect(session.getFailure()?.message).toBe('relay resume confirmation missing') - expect(fakes.close).toHaveBeenCalledOnce() - }) - - it('fails a session whose confirm never answers', async () => { - vi.useFakeTimers() - try { - const { session } = authenticateSession() - expect(session.getState()).toBe('connected') - - await vi.advanceTimersByTimeAsync(1_000) - - expect(session.getState()).toBe('disconnected') - expect(session.getFailure()?.message).toBe('relay RPC timed out: pairing.getEndpoints') - } finally { - vi.useRealTimers() - } - }) - - it('hands the landed confirmation to resume persistence', async () => { - const { session, confirmationRequest } = authenticateSession() - const bundle: MobileRelayCredentialBundle = { - v: 1, - hostId: 'host-1', - deviceToken: 'device-token', - current: { token: 'A'.repeat(43), hash: 'B'.repeat(43), version: 3, expiresAt: 1 } - } - const writeBundle = vi.fn(async () => {}) - // Why: persistence runs right after the migration, while the confirm is still - // in flight — it must wait for the answer instead of reading a null. - const persisting = persistResumeConfirmation({ - session, - bundle, - usedCredentialVersion: 3, - writeBundle - }) - expect(writeBundle).not.toHaveBeenCalled() - - answerConfirm(confirmationRequest) - const applied = await persisting - - expect(writeBundle).toHaveBeenCalledOnce() - expect(applied.bundle.current.expiresAt).toBe(session.getResumeExpiresAt()) - expect(applied.leaseExpiry).toBe(session.getResumeExpiresAt()) - session.close() - }) - // Why: ConnectionState stays 'connecting' until relay-hello, so the migration bound // needs a separate signal to tell "cell never answered the upgrade" from "cell took // relay-auth and is still resolving the assignment". @@ -332,7 +231,7 @@ describe('mobile relay RPC session', () => { expect(session.getDialStage()).toBe('handshaking') fakes.linkOptions!.onAuthenticated() expect(session.getDialStage()).toBe('confirming') - expect(fakes.sendText).toHaveBeenCalledTimes(2) + await vi.waitFor(() => expect(fakes.sendText).toHaveBeenCalledOnce()) expect(stages).toEqual(['awaiting-hello', 'handshaking', 'confirming']) session.close() }) @@ -355,7 +254,7 @@ describe('mobile relay RPC session', () => { }) it('routes terminal and browser binary streams after confirmation', async () => { - const { session } = await settledSession() + const { session } = await authenticateSession() const terminalListener = vi.fn() session.subscribe('terminal.subscribe', { terminal: 'term-1' }, terminalListener) await vi.waitFor(() => expect(fakes.sendText).toHaveBeenCalledOnce()) @@ -412,7 +311,7 @@ describe('mobile relay RPC session', () => { }) it('rejects pending RPC work when the physical link fails', async () => { - const { session } = await settledSession() + const { session } = await authenticateSession() const pending = session.sendRequest('status.get') await vi.waitFor(() => expect(fakes.sendText).toHaveBeenCalledOnce()) fakes.linkOptions!.onError(new Error('relay transport error')) @@ -424,7 +323,7 @@ describe('mobile relay RPC session', () => { }) it('marks in-flight requests delivery-unknown when the session closes', async () => { - const { session } = await settledSession() + const { session } = await authenticateSession() const pending = session.sendRequest('terminal.send', { terminal: 'term', text: 'hi' }) await vi.waitFor(() => expect(fakes.sendText).toHaveBeenCalledOnce()) session.close() @@ -434,7 +333,7 @@ describe('mobile relay RPC session', () => { }) it('marks a relay RPC timeout delivery-unknown', async () => { - const { session } = await settledSession() + const { session } = await authenticateSession() vi.useFakeTimers() try { const pending = session.sendRequest('terminal.send', { terminal: 'term', text: 'hi' }) @@ -453,57 +352,4 @@ describe('mobile relay RPC session', () => { vi.useRealTimers() } }) - it('keeps whenResumeConfirmed() pending until the session has an answer', async () => { - // The contract callers rely on is "settles when the confirm has answered or the - // session is over". A promise already resolved during the dial would let a caller - // read getResumeConfirmation() as null and persist that as the answer. - const session = openSession() - const settled = vi.fn() - void session.whenResumeConfirmed().then(settled) - receiveHello() - await Promise.resolve() - expect(settled).not.toHaveBeenCalled() - - fakes.linkOptions!.onAuthenticated() - await Promise.resolve() - expect(settled).not.toHaveBeenCalled() - - answerConfirm(sentRequests()[0]!) - await session.whenResumeConfirmed() - expect(settled).toHaveBeenCalled() - expect(session.getResumeConfirmation()).toMatchObject({ reqId: 'confirm-1' }) - }) - - it('settles whenResumeConfirmed() when the session dies before authenticating', async () => { - const session = openSession() - const settled = vi.fn() - void session.whenResumeConfirmed().then(settled) - - // A credential-version mismatch fails the session inside onHello, so no confirm - // is ever sent. Awaiting the answer must not hang a caller forever. - fakes.linkOptions!.onHello({ - type: 'relay-hello', - ok: true, - credentialKind: 'resume', - leaseExpiresAt: Date.now() + 60_000, - acceptedCredentialVersion: 2, - acceptedAs: 'current', - resumeExpiresAt: Date.now() + 300_000 - }) - - await session.whenResumeConfirmed() - expect(settled).toHaveBeenCalled() - expect(session.getState()).toBe('disconnected') - }) - - it('settles whenResumeConfirmed() when a caller closes an unconfirmed session', async () => { - const session = openSession() - const settled = vi.fn() - void session.whenResumeConfirmed().then(settled) - - session.close() - - await session.whenResumeConfirmed() - expect(settled).toHaveBeenCalled() - }) }) diff --git a/mobile/src/transport/mobile-relay-rpc-session.ts b/mobile/src/transport/mobile-relay-rpc-session.ts index 6f49de5c973..67b50ea591e 100644 --- a/mobile/src/transport/mobile-relay-rpc-session.ts +++ b/mobile/src/transport/mobile-relay-rpc-session.ts @@ -9,18 +9,17 @@ import { MobileE2EEAuthenticationError } from './mobile-e2ee-v2-physical-channel import { markRpcDeliveryUnknown } from './rpc-delivery-ambiguity' import { openRpcRequestBudget, resolvePostConnectRequestTimeout } from './rpc-request-budget' import { isRpcResponse } from './rpc-response-shape' -import { RelayDialStageLog } from './relay-dial-stage-log' import { RelayDialStageTracker, type RelayDialStageSource } from './relay-dial-stage' import { RelayPendingRequests } from './relay-pending-requests' -import { createRelaySessionLivenessWatchdog } from './relay-session-liveness-profile' +import { RpcSessionLivenessWatchdog } from './rpc-session-liveness-watchdog' import { settleMobileRuntimeCapabilities } from './mobile-runtime-capability-negotiation' import type { RelayHostCloseReason } from '../../../src/shared/relay-host-close-reason' import type { RpcClient } from './rpc-client' import type { ConnectionLogSink, ConnectionState, RpcResponse } from './types' -// Bounds the confirm exactly as migrateTo's own wait used to, so the supervisor's -// mutex is never held for the full request timeout waiting on a silent cell. -const RELAY_CONFIRM_TIMEOUT_MS = 12_000 +const RELAY_PROBE_TIMEOUT_MS = 4_000 +const RELAY_MISSED_PROBE_LIMIT = 2 +const RELAY_FOREGROUND_PROBE_MIN_INTERVAL_MS = 10_000 let relayRpcSessionSequence = 0 export type MobileRelayRpcSession = RpcClient & @@ -30,10 +29,6 @@ export type MobileRelayRpcSession = RpcClient & getAttachDeadlineAt(): number | null getResumeExpiresAt(): number | null getResumeConfirmation(): DeviceResumeConfirmed | null - // Settles once the resume confirm has answered or failed the session. Never - // rejects. Anyone reading getResumeConfirmation()/getResumeExpiresAt() must - // await it: 'connected' is published at authentication, ahead of the confirm. - whenResumeConfirmed(): Promise getFailure(): Error | null } @@ -45,8 +40,6 @@ export function connectMobileRelayRpcSession(args: { deviceToken: string desktopPublicKeyB64: string requestTimeoutMs?: number - // Gates the idle liveness sweep; a backgrounded app must not spend probes. - isForeground?: () => boolean createSocket?: (url: string) => WebSocket onHostCloseReason?: (reason: RelayHostCloseReason) => void onLog?: ConnectionLogSink @@ -64,16 +57,7 @@ export function connectMobileRelayRpcSession(args: { let logSequence = 0 const logSessionId = `${Date.now().toString(36)}-${(++relayRpcSessionSequence).toString(36)}` const livenessIdentity = {} - // Why created here and not at authentication: handing a pre-auth caller an - // already-resolved promise would let it read getResumeConfirmation() as null and - // treat that as the answer. Every terminal path settles it — the confirm, fail(), - // and close() — so awaiting it can never outlive the session. - let settleResumeConfirmed!: () => void - const resumeConfirmed = new Promise((resolve) => { - settleResumeConfirmed = resolve - }) const dialStage = new RelayDialStageTracker() - const dialStageLog = new RelayDialStageLog(dialStage, logSessionId, args.onLog) const streams = new MobileRelayRpcStreams({ nextId: () => pending.nextId(), sendFrame, @@ -88,7 +72,7 @@ export function connectMobileRelayRpcSession(args: { desktopPublicKeyB64: args.desktopPublicKeyB64, createSocket: args.createSocket, onHostCloseReason: args.onHostCloseReason, - onOpen: () => dialStageLog.enter('awaiting-hello'), + onOpen: () => dialStage.advance('awaiting-hello'), onHello: (hello) => { if ( hello.credentialKind !== 'resume' || @@ -99,10 +83,10 @@ export function connectMobileRelayRpcSession(args: { } attachDeadlineAt = hello.leaseExpiresAt resumeExpiresAt = hello.resumeExpiresAt - dialStageLog.enter('handshaking') + dialStage.advance('handshaking') publishState('handshaking') }, - onAuthenticated: () => publishAuthenticated(), + onAuthenticated: () => void confirmResume(), onText: (plaintext) => { livenessWatchdog.noteAuthenticatedInbound(livenessIdentity) handleText(plaintext) @@ -141,56 +125,58 @@ export function connectMobileRelayRpcSession(args: { }, notifyForeground: (reason) => { if (state === 'connected' && reason !== 'network-change') { - livenessWatchdog.probeNow(livenessIdentity, reason === 'app-resume' ? 'resume' : 'nudge') + livenessWatchdog.probeNow(livenessIdentity) } }, - close: () => terminate(new Error('Client closed')), + close() { + if (closed) { + return + } + closed = true + livenessWatchdog.stop(livenessIdentity) + link.close() + pending.rejectAll(new Error('Client closed')) + streams.clear() + publishState('disconnected') + }, getDialStage: () => dialStage.getDialStage(), onDialStageChange: (listener) => dialStage.onDialStageChange(listener), getAttachDeadlineAt: () => attachDeadlineAt, getResumeExpiresAt: () => resumeExpiresAt, getResumeConfirmation: () => resumeConfirmation, - whenResumeConfirmed: () => resumeConfirmed, getFailure: () => failure } - const livenessWatchdog = createRelaySessionLivenessWatchdog({ - isForeground: args.isForeground, + const livenessWatchdog = new RpcSessionLivenessWatchdog({ + transport: 'relay', + idleProbeMs: null, + probeTimeoutMs: RELAY_PROBE_TIMEOUT_MS, + missedProbeLimit: RELAY_MISSED_PROBE_LIMIT, + voluntaryProbeMinIntervalMs: RELAY_FOREGROUND_PROBE_MIN_INTERVAL_MS, sendProbe: () => state === 'connected' && sendFrame({ id: pending.nextId(), method: 'status.get', params: undefined }), - terminate: () => fail(new Error('relay session liveness timeout')), - onLog: args.onLog, - nextLogId: () => `relay-liveness-${logSessionId}-${++logSequence}` + onTimeout: (evidence) => { + args.onLog?.({ + id: `relay-liveness-${logSessionId}-${++logSequence}`, + ts: Date.now(), + level: 'error', + code: 'liveness-timeout', + path: 'relay', + message: 'Relay health check failed', + detail: `${evidence.reason}; ${evidence.missedProbes}/${evidence.missedProbeLimit} probes missed; last authenticated activity ${evidence.lastInboundAgeMs}ms ago` + }) + }, + terminate: () => fail(new Error('relay session liveness timeout')) }) return client - // Why: the transport carries traffic the moment E2EE authenticates. The resume - // confirm and the capability advisory ride it concurrently instead of putting - // two serialized round trips in front of 'connected'. - function publishAuthenticated(): void { - if (closed) { - return - } - dialStageLog.enter('confirming') - void confirmResume().then(settleResumeConfirmed, settleResumeConfirmed) - // Why: an unanswered advisory says nothing, but a frame that never reached the - // wire proves the socket cannot carry traffic — that alone still fails. - void settleMobileRuntimeCapabilities((method, params) => - sendRpc(method, params, requestTimeoutMs, true) - ).catch((error: unknown) => fail(asError(error))) - lastConnectedAt = Date.now() - livenessWatchdog.start(livenessIdentity) - publishState('connected') - } - - // Off the critical path but never optional: a failed confirm or a relayHostId - // that is not ours still fails the session, only later than it used to. async function confirmResume(): Promise { + dialStage.advance('confirming') try { const response = await sendRpc( 'pairing.getEndpoints', { resumeConfirmReqId: args.resumeConfirmReqId }, - Math.min(requestTimeoutMs, RELAY_CONFIRM_TIMEOUT_MS), + requestTimeoutMs, true ) if (!response.ok) { @@ -202,9 +188,13 @@ export function connectMobileRelayRpcSession(args: { } resumeConfirmation = result.resumeConfirmation resumeExpiresAt = result.resumeConfirmation.resumeExpiresAt - // The dial's last stage ends when the desktop has confirmed the resume, not when - // 'connected' was published at authentication ahead of it. - dialStageLog.settle(true) + lastConnectedAt = Date.now() + // Why: an unanswered advisory must not keep a slow relay from ever reaching connected. + await settleMobileRuntimeCapabilities((method, params) => + sendRpc(method, params, requestTimeoutMs, true) + ) + livenessWatchdog.start(livenessIdentity) + publishState('connected') } catch (error) { fail(asError(error)) } @@ -297,29 +287,18 @@ export function connectMobileRelayRpcSession(args: { } } - // One teardown for both endings; only whether the session is to blame differs, and - // recording a failure for a caller's close would make the establisher report a - // deliberate teardown as a dial error. - function terminate(error: Error): void { + function fail(error: Error): void { if (closed) { return } closed = true - settleResumeConfirmed() - dialStageLog.settle(false, error.message) + failure = error livenessWatchdog.stop(livenessIdentity) streams.clear() link.close() pending.rejectAll(error) publishState(error instanceof MobileE2EEAuthenticationError ? 'auth-failed' : 'disconnected') } - - function fail(error: Error): void { - if (!closed) { - failure = error - } - terminate(error) - } } function asError(error: unknown): Error { diff --git a/mobile/src/transport/mobile-relay-runtime-failover.test.ts b/mobile/src/transport/mobile-relay-runtime-failover.test.ts index 7098746587a..ce7cca3fd9f 100644 --- a/mobile/src/transport/mobile-relay-runtime-failover.test.ts +++ b/mobile/src/transport/mobile-relay-runtime-failover.test.ts @@ -88,7 +88,6 @@ class FakeRelaySession extends FakeSession implements MobileRelayRpcSession { this.dialStage.onDialStageChange(listener) getResumeExpiresAt = () => Date.now() + 30 * 24 * 3_600_000 getResumeConfirmation = () => null - whenResumeConfirmed = () => Promise.resolve() getFailure = () => this.failure } @@ -278,7 +277,6 @@ describe('relay runtime recovery without direct connectivity', () => { relay, expect.objectContaining({ version: 3 }), expect.any(String), - expect.any(Function), expect.any(Function) ) expect(logical.getActivePath()).toBe('relay') @@ -369,7 +367,6 @@ describe('relay runtime recovery without direct connectivity', () => { relay, expect.objectContaining({ version: 2 }), expect.any(String), - expect.any(Function), expect.any(Function) ) expect(logical.getActivePath()).toBe('relay') @@ -400,7 +397,6 @@ describe('relay runtime recovery without direct connectivity', () => { relay, expect.objectContaining({ version: 1 }), expect.any(String), - expect.any(Function), expect.any(Function) ) expect(logical.getActivePath()).toBe('relay') diff --git a/mobile/src/transport/mobile-relay-session-establisher.ts b/mobile/src/transport/mobile-relay-session-establisher.ts index be6130dac1c..9a04ae44137 100644 --- a/mobile/src/transport/mobile-relay-session-establisher.ts +++ b/mobile/src/transport/mobile-relay-session-establisher.ts @@ -19,25 +19,6 @@ function directWon(logical: StableLogicalRpcClient): boolean { return logical.getActivePath() !== 'relay' && logical.getState() === 'connected' } -// Why: migrateTo consults its abort predicate only after E2EE authentication, so -// a dial that has already lost would still make the cell reserve a splice and the -// desktop finish a handshake. Closing the socket withdraws it at whatever stage it -// reached — before any e2ee frame when the hello has not landed yet. The caller -// still reports the dial as aborted, so nothing is booked against relay. -function withdrawWhenDirectWins( - logical: StableLogicalRpcClient, - session: { close(): void } -): () => void { - const withdraw = (): void => { - if (directWon(logical)) { - session.close() - } - } - const unsubscribe = logical.onStateChange(withdraw) - withdraw() - return unsubscribe -} - // Turns one relay credential into the active runtime session: resolve the cell // assignment if the director rejects the cached one, open the cell socket, // migrate the logical client onto it, then persist the resume confirmation and @@ -129,10 +110,8 @@ export class MobileRelaySessionEstablisher { if (reason === RELAY_HOST_CLOSE_REASON.SIGNED_OUT) { args.logical.setHostSignedOut(true) } - }, - args.isForeground + } ) - const stopWithdrawWatch = withdrawWhenDirectWins(args.logical, session) try { // Why: backgrounding or a direct winner withdraws this dial before cutover. await args.logical.migrateTo( @@ -146,21 +125,6 @@ export class MobileRelaySessionEstablisher { return { ok: false, error: new RelayDialAbortedError() } } return { ok: false, error: session.getFailure() ?? toError(error) } - } finally { - // Why: past the cutover this session is the active path, and a later direct - // promotion must not read as a reason to close the client's own socket. - stopWithdrawWatch() - } - // Why: migrateTo now resolves at E2EE authentication, so the resume confirm can - // still fail this session after the cutover. Booking a dying session as an - // established dial skips backoff and redials in a tight loop — the supervisor's - // bookkeeping waits for the verdict even though the UI is already connected. - await session.whenResumeConfirmed() - if (session.getState() !== 'connected') { - if (!args.isActive() || directWon(args.logical)) { - return { ok: false, error: new RelayDialAbortedError() } - } - return { ok: false, error: session.getFailure() ?? new Error('relay lost at confirm') } } args.controller.setActiveSession(session) if (!args.isForeground()) { diff --git a/mobile/src/transport/monotonic-clock.ts b/mobile/src/transport/monotonic-clock.ts deleted file mode 100644 index c9d4294f492..00000000000 --- a/mobile/src/transport/monotonic-clock.ts +++ /dev/null @@ -1,15 +0,0 @@ -// Why: connection phase durations must never go negative. Date.now() can jump -// backwards (NTP or a user clock change) mid-dial, which would turn a slow stage -// into a negative one in the diagnostics report. performance.now() is monotonic -// and Hermes exposes it; hosts without it fall back to wall clock. -const hasPerformanceNow = - typeof performance === 'object' && performance !== null && typeof performance.now === 'function' - -export const monotonicNowMs: () => number = hasPerformanceNow - ? () => performance.now() - : () => Date.now() - -/** Whole milliseconds between two monotonic reads, clamped so a fallback wall-clock jump can't go negative. */ -export function elapsedMs(startedAt: number, endedAt: number = monotonicNowMs()): number { - return Math.max(0, Math.round(endedAt - startedAt)) -} diff --git a/mobile/src/transport/persisted-connection-log-store.test.ts b/mobile/src/transport/persisted-connection-log-store.test.ts index 8f287bf32a1..6635ccec696 100644 --- a/mobile/src/transport/persisted-connection-log-store.test.ts +++ b/mobile/src/transport/persisted-connection-log-store.test.ts @@ -23,89 +23,6 @@ describe('persisted connection log store', () => { vi.resetModules() }) - // 'negotiating' is not a dial stage and 'confirming' is a dial stage rather than a - // connection state; the report echoes the name, so neither may survive. A negative - // duration is corruption too: producers clamp at 0, and the report sums these, so a - // negative would subtract from a dial total. - it('rehydrates well-formed phase timings and drops corrupt names and durations', async () => { - vi.mocked(AsyncStorage.getItem).mockResolvedValue( - JSON.stringify([ - { - id: 'stage-ok', - ts: 900, - level: 'info', - message: 'Relay dial stage awaiting-hello finished', - timing: { kind: 'relay-dial-stage', name: 'awaiting-hello', ms: 6_400, complete: true } - }, - { - id: 'stage-corrupt', - ts: 950, - level: 'info', - message: 'Relay dial stage handshaking finished', - timing: { kind: 'relay-dial-stage', name: 'handshaking', ms: 'soon' } - }, - { - id: 'stage-unknown-name', - ts: 960, - level: 'info', - message: 'Relay dial stage negotiating finished', - timing: { kind: 'relay-dial-stage', name: 'negotiating', ms: 12, complete: true } - }, - { - id: 'state-borrowed-stage-name', - ts: 970, - level: 'info', - message: 'Connection state confirming → connected', - timing: { kind: 'connection-state', name: 'confirming', ms: 12, complete: true } - }, - { - id: 'state-unknown-kind', - ts: 980, - level: 'info', - message: 'Something else', - timing: { kind: 'wall-clock', name: 'connecting', ms: 12, complete: true } - }, - { - id: 'stage-negative-ms', - ts: 985, - level: 'info', - message: 'Relay dial stage opening finished', - timing: { kind: 'relay-dial-stage', name: 'opening', ms: -1, complete: true } - }, - { - id: 'state-negative-ms', - ts: 990, - level: 'info', - message: 'Connection state connecting → connected', - timing: { kind: 'connection-state', name: 'connecting', ms: -0.5, complete: true } - }, - { - id: 'stage-zero-ms', - ts: 995, - level: 'info', - message: 'Relay dial stage confirming finished', - timing: { kind: 'relay-dial-stage', name: 'confirming', ms: 0, complete: true } - } - ]) - ) - vi.resetModules() - const { connectionLogStore } = await import('./persisted-connection-log-store') - - await connectionLogStore.hydrate('host-timings') - - // 0 survives: a stage the dial passed through instantly is real, not corruption. - expect(connectionLogStore.get('host-timings').map((entry) => entry.id)).toEqual([ - 'stage-ok', - 'stage-zero-ms' - ]) - expect(connectionLogStore.get('host-timings')[0]!.timing).toEqual({ - kind: 'relay-dial-stage', - name: 'awaiting-hello', - ms: 6_400, - complete: true - }) - }) - it('keeps a new client-session boundary when a restart shares the prior timestamp', async () => { const stored: ConnectionLogEntry[] = [ { diff --git a/mobile/src/transport/persisted-connection-log-store.ts b/mobile/src/transport/persisted-connection-log-store.ts index b8f4e0ec485..85f9794b284 100644 --- a/mobile/src/transport/persisted-connection-log-store.ts +++ b/mobile/src/transport/persisted-connection-log-store.ts @@ -1,7 +1,6 @@ import AsyncStorage from '@react-native-async-storage/async-storage' import { createConnectionLogStore } from './connection-log-buffer' -import { RELAY_DIAL_STAGE_NAMES } from './relay-dial-stage' -import { CONNECTION_STATE_NAMES, type ConnectionLogEntry, type ConnectionLogTiming } from './types' +import type { ConnectionLogEntry } from './types' const STORAGE_PREFIX = 'orca.mobile.connection-log.v1.' const clientSessionId = `${Date.now().toString(36)}-${Math.random().toString(36).slice(2)}` @@ -73,30 +72,6 @@ function isConnectionLogEntry(value: unknown): value is ConnectionLogEntry { entry.level === 'warn' || entry.level === 'error') && typeof entry.message === 'string' && - (entry.detail === undefined || typeof entry.detail === 'string') && - (entry.timing === undefined || isConnectionLogTiming(entry.timing)) - ) -} - -// Why: the report echoes the phase name and formats the duration directly, so a -// corrupted stored timing must not reach it. The name is checked against the closed -// enum for its kind, not just "is a string", and the duration must be one a producer -// could have written — `elapsedMs` clamps at 0, so a negative is corruption. -function isConnectionLogTiming(value: unknown): value is ConnectionLogTiming { - if (!value || typeof value !== 'object') { - return false - } - const timing = value as Partial - if (timing.kind !== 'relay-dial-stage' && timing.kind !== 'connection-state') { - return false - } - const names = timing.kind === 'relay-dial-stage' ? RELAY_DIAL_STAGE_NAMES : CONNECTION_STATE_NAMES - return ( - typeof timing.name === 'string' && - Object.hasOwn(names, timing.name) && - typeof timing.ms === 'number' && - Number.isFinite(timing.ms) && - timing.ms >= 0 && - typeof timing.complete === 'boolean' + (entry.detail === undefined || typeof entry.detail === 'string') ) } diff --git a/mobile/src/transport/relay-dial-stage-log.ts b/mobile/src/transport/relay-dial-stage-log.ts deleted file mode 100644 index b7ae981bcd1..00000000000 --- a/mobile/src/transport/relay-dial-stage-log.ts +++ /dev/null @@ -1,55 +0,0 @@ -import type { - RelayDialStage, - RelayDialStageTracker, - RelayDialStageTiming -} from './relay-dial-stage' -import type { ConnectionLogSink } from './types' - -// Why: support needs per-stage durations for a slow dial, and the name of the stage -// a failed dial died in, without a debug build. Timing only — advancing the tracker -// stays the session's call. -export class RelayDialStageLog { - private sequence = 0 - - constructor( - private readonly tracker: RelayDialStageTracker, - private readonly sessionId: string, - private readonly sink?: ConnectionLogSink - ) {} - - enter(stage: RelayDialStage): void { - this.record(this.tracker.advance(stage)) - } - - settle(complete: boolean, failureDetail?: string): void { - this.record(this.tracker.settle(complete), failureDetail) - } - - private record(timing: RelayDialStageTiming | null, failureDetail?: string): void { - if (!timing) { - return - } - // Why: this runs inside the dial's success and failure paths. A sink that - // throws must not turn a good connect into a failed one. - try { - this.sink?.({ - id: `relay-dial-stage-${this.sessionId}-${++this.sequence}`, - ts: Date.now(), - level: timing.complete ? 'info' : 'warn', - path: 'relay', - message: `Relay dial stage ${timing.stage} ${ - timing.complete ? 'finished' : 'did not finish' - }`, - detail: `${timing.ms}ms${failureDetail ? ` — ${failureDetail}` : ''}`, - timing: { - kind: 'relay-dial-stage', - name: timing.stage, - ms: timing.ms, - complete: timing.complete - } - }) - } catch { - // Diagnostics only; a broken sink is not worth failing a dial over. - } - } -} diff --git a/mobile/src/transport/relay-dial-stage-timings.test.ts b/mobile/src/transport/relay-dial-stage-timings.test.ts deleted file mode 100644 index a32f34d5e9c..00000000000 --- a/mobile/src/transport/relay-dial-stage-timings.test.ts +++ /dev/null @@ -1,219 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' -import { RelayDialStageTracker } from './relay-dial-stage' -import type { ConnectionLogEntry } from './types' - -const fakes = vi.hoisted(() => ({ - linkOptions: null as null | { - onOpen(): void - onHello(value: unknown): void - onAuthenticated(): void - onText(value: string): void - onBinary(value: Uint8Array): void - onError(error: Error): void - }, - sendText: vi.fn(() => true), - close: vi.fn() -})) - -vi.mock('./mobile-relay-e2ee-link', () => ({ - MobileRelayE2eeLink: class { - constructor(options: NonNullable) { - fakes.linkOptions = options - } - sendText = fakes.sendText - close = fakes.close - } -})) - -import { connectMobileRelayRpcSession } from './mobile-relay-rpc-session' - -const relay = { - v: 1 as const, - directorUrl: 'https://relay.onorca.dev', - cellUrl: 'https://relay-c1.onorca.dev', - assignmentEpoch: 7, - relayHostId: 'AbCdEf0123_-xyZ9', - e2eeFraming: 2 as const -} - -function openSession(entries: ConnectionLogEntry[]) { - return connectMobileRelayRpcSession({ - relay, - resumeToken: 'resume-secret', - resumeCredentialVersion: 3, - resumeConfirmReqId: 'confirm-1', - deviceToken: 'device-token', - desktopPublicKeyB64: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=', - requestTimeoutMs: 1000, - onLog: (entry) => entries.push(entry) - }) -} - -function stageTimings(entries: readonly ConnectionLogEntry[]) { - return entries.flatMap((entry) => - entry.timing?.kind === 'relay-dial-stage' ? [entry.timing] : [] - ) -} - -describe('RelayDialStageTracker timings', () => { - it('times every stage it passes through without going negative', () => { - // A clock that steps backwards proves the report can never show a negative stage. - const reads = [0, 120, 4_400, 4_300, 5_500] - let index = 0 - const tracker = new RelayDialStageTracker(() => reads[index++]!) - - expect(tracker.advance('awaiting-hello')).toEqual({ - stage: 'opening', - ms: 120, - complete: true - }) - expect(tracker.advance('handshaking')).toEqual({ - stage: 'awaiting-hello', - ms: 4_280, - complete: true - }) - expect(tracker.advance('confirming')).toEqual({ - stage: 'handshaking', - ms: 0, - complete: true - }) - expect(tracker.settle(true)).toEqual({ stage: 'confirming', ms: 1_200, complete: true }) - expect(tracker.getDialStage()).toBe('confirming') - }) - - it('re-advancing to the current stage is not a transition', () => { - const tracker = new RelayDialStageTracker(() => 0) - expect(tracker.advance('opening')).toBeNull() - }) - - it('settles once, so a failure after connecting cannot re-time the last stage', () => { - let now = 0 - const tracker = new RelayDialStageTracker(() => now) - tracker.advance('awaiting-hello') - now = 900 - expect(tracker.settle(true)).toEqual({ stage: 'awaiting-hello', ms: 900, complete: true }) - now = 90_000 - expect(tracker.settle(false)).toBeNull() - }) -}) - -function requestIdAt(call: number): string { - return (JSON.parse(fakes.sendText.mock.calls[call]![0] as string) as { id: string }).id -} - -async function driveToConnected(session: { - getState(): string - whenResumeConfirmed(): Promise -}): Promise { - fakes.linkOptions!.onOpen() - fakes.linkOptions!.onHello({ - type: 'relay-hello', - ok: true, - credentialKind: 'resume', - leaseExpiresAt: Date.now() + 60_000, - acceptedCredentialVersion: 3, - acceptedAs: 'current', - resumeExpiresAt: Date.now() + 300_000 - }) - fakes.linkOptions!.onAuthenticated() - // 'connected' is published at authentication; the resume confirm and the capability - // advisory are both already on the wire, so answer them in the order they were sent. - await vi.waitFor(() => expect(session.getState()).toBe('connected')) - expect(fakes.sendText).toHaveBeenCalledTimes(2) - fakes.linkOptions!.onText( - JSON.stringify({ - id: requestIdAt(0), - ok: true, - result: { - v: 1, - relay, - resumeConfirmation: { - v: 1, - reqId: 'confirm-1', - currentVersion: 3, - acceptedAs: 'current', - renewed: true, - resumeExpiresAt: Date.now() + 300_000 - } - }, - _meta: { runtimeId: 'runtime-1' } - }) - ) - fakes.linkOptions!.onText( - JSON.stringify({ id: requestIdAt(1), ok: true, result: {}, _meta: { runtimeId: 'runtime-1' } }) - ) - await session.whenResumeConfirmed() -} - -describe('relay dial stage timings in the connection log', () => { - beforeEach(() => { - fakes.sendText.mockClear() - fakes.close.mockClear() - }) - - it('records the stages a failed dial reached plus the stage it died in', () => { - const entries: ConnectionLogEntry[] = [] - openSession(entries) - fakes.linkOptions!.onOpen() - fakes.linkOptions!.onError(new Error('relay dial failed')) - - const timings = stageTimings(entries) - expect(timings.map((timing) => timing.name)).toEqual(['opening', 'awaiting-hello']) - expect(timings.map((timing) => timing.complete)).toEqual([true, false]) - for (const timing of timings) { - expect(timing.ms).toBeGreaterThanOrEqual(0) - } - expect(entries.at(-1)!.message).toContain('awaiting-hello did not finish') - expect(entries.at(-1)!.detail).toContain('relay dial failed') - expect(entries.at(-1)!.path).toBe('relay') - }) - - it('records every stage of a dial that reaches connected, all complete', async () => { - const entries: ConnectionLogEntry[] = [] - const session = openSession(entries) - await driveToConnected(session) - - const timings = stageTimings(entries) - expect(timings.map((timing) => timing.name)).toEqual([ - 'opening', - 'awaiting-hello', - 'handshaking', - 'confirming' - ]) - expect(timings.every((timing) => timing.complete)).toBe(true) - expect(timings.every((timing) => timing.ms >= 0)).toBe(true) - - // A later teardown must not append a second timing for 'confirming'. - session.close() - expect(stageTimings(entries)).toHaveLength(4) - }) - - it('reaches connected even when the log sink throws on every stage', async () => { - const session = connectMobileRelayRpcSession({ - relay, - resumeToken: 'resume-secret', - resumeCredentialVersion: 3, - resumeConfirmReqId: 'confirm-1', - deviceToken: 'device-token', - desktopPublicKeyB64: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=', - requestTimeoutMs: 1000, - onLog: () => { - throw new Error('sink exploded') - } - }) - await driveToConnected(session) - - expect(session.getState()).toBe('connected') - expect(session.getFailure()).toBeNull() - }) - - it('marks a dial that never opened its socket as stuck in opening', () => { - const entries: ConnectionLogEntry[] = [] - openSession(entries) - fakes.linkOptions!.onError(new Error('websocket refused')) - - expect(stageTimings(entries)).toEqual([ - { kind: 'relay-dial-stage', name: 'opening', ms: expect.any(Number), complete: false } - ]) - }) -}) diff --git a/mobile/src/transport/relay-dial-stage.ts b/mobile/src/transport/relay-dial-stage.ts index 5960cc9a24c..c4a743f84f4 100644 --- a/mobile/src/transport/relay-dial-stage.ts +++ b/mobile/src/transport/relay-dial-stage.ts @@ -1,5 +1,3 @@ -import { elapsedMs, monotonicNowMs } from './monotonic-clock' - // Where a relay dial is waiting, so a bound can tell "the cell never answered the // upgrade" from "the cell took the dial and is slow" — the two look identical from // ConnectionState, which stays 'connecting' until relay-hello arrives. @@ -14,23 +12,6 @@ export type RelayDialStage = // E2EE authenticated; waiting on the desktop's resume confirmation. | 'confirming' -// Exhaustive by construction: adding a stage to the union breaks this table, so a -// persisted-log validator can never silently start accepting an unknown stage. -export const RELAY_DIAL_STAGE_NAMES: Record = { - opening: true, - 'awaiting-hello': true, - handshaking: true, - confirming: true -} - -// How long a dial spent in one stage. `complete` is false when the dial left the -// stage by dying in it, so a report can name the stage that never finished. -export type RelayDialStageTiming = { - stage: RelayDialStage - ms: number - complete: boolean -} - export type RelayDialStageSource = { getDialStage(): RelayDialStage onDialStageChange(listener: (stage: RelayDialStage) => void): () => void @@ -46,14 +27,8 @@ export function relayDialStageSource(session: object): RelayDialStageSource | nu export class RelayDialStageTracker implements RelayDialStageSource { private stage: RelayDialStage = 'opening' - private stageEnteredAt: number - private settled = false private readonly listeners = new Set<(stage: RelayDialStage) => void>() - constructor(private readonly now: () => number = monotonicNowMs) { - this.stageEnteredAt = now() - } - getDialStage(): RelayDialStage { return this.stage } @@ -63,34 +38,14 @@ export class RelayDialStageTracker implements RelayDialStageSource { return () => this.listeners.delete(listener) } - /** Returns the timing of the stage just left, or null when nothing was timed. */ - advance(stage: RelayDialStage): RelayDialStageTiming | null { + advance(stage: RelayDialStage): void { if (this.stage === stage) { - return null + return } - const now = this.now() - const timing = this.settled ? null : this.closeStage(true, now) this.stage = stage - this.stageEnteredAt = now for (const listener of this.listeners) { listener(stage) } - return timing - } - - // Close the stage the dial is sitting in: `true` once it reached the runtime, - // `false` when it died there. Idempotent, so a failure on an already-connected - // session cannot re-time the last dial stage. - settle(complete: boolean): RelayDialStageTiming | null { - if (this.settled) { - return null - } - this.settled = true - return this.closeStage(complete, this.now()) - } - - private closeStage(complete: boolean, now: number): RelayDialStageTiming { - return { stage: this.stage, ms: elapsedMs(this.stageEnteredAt, now), complete } } } diff --git a/mobile/src/transport/relay-recovery-intent-queue.ts b/mobile/src/transport/relay-recovery-intent-queue.ts deleted file mode 100644 index c34e40b8990..00000000000 --- a/mobile/src/transport/relay-recovery-intent-queue.ts +++ /dev/null @@ -1,45 +0,0 @@ -// Recovery requests that arrive while the supervisor's operation mutex is held. -// Two latches, because the intents are not interchangeable: an owning forced -// replacement books the shared cooldown and may bring a stale session down, while -// every other request must replay as a plain recovery. Nothing is ever dropped. -export class RelayRecoveryIntentQueue { - private replacement = false - private recovery = false - - queue(forceReplacement: boolean, ownsRecovery: boolean): void { - if (forceReplacement && ownsRecovery) { - this.replacement = true - return - } - this.recovery = true - } - - holdReplacement(): void { - this.replacement = true - } - - hasReplacement(): boolean { - return this.replacement - } - - clearReplacement(): void { - this.replacement = false - } - - takeReplacement(): boolean { - const queued = this.replacement - this.replacement = false - return queued - } - - takeRecovery(): boolean { - const queued = this.recovery - this.recovery = false - return queued - } - - clear(): void { - this.replacement = false - this.recovery = false - } -} diff --git a/mobile/src/transport/relay-session-liveness-profile.ts b/mobile/src/transport/relay-session-liveness-profile.ts deleted file mode 100644 index e56eb8e81fb..00000000000 --- a/mobile/src/transport/relay-session-liveness-profile.ts +++ /dev/null @@ -1,54 +0,0 @@ -import { - RpcSessionLivenessWatchdog, - type LivenessTimeoutEvidence -} from './rpc-session-liveness-watchdog' -import type { ConnectionLogSink } from './types' - -// Ordinary foreground checks: two 4s misses, at most one voluntary probe per 10s. -const RELAY_PROBE = { timeoutMs: 4_000, missedProbeLimit: 2, minIntervalMs: 10_000 } -// A socket that died while the process was suspended must be admitted before the -// user reads the screen as broken. Two 2s misses, not one: the first frame after a -// resume rides a cold radio, and a single slow answer is not proof of a dead link. -const RELAY_RESUME_PROBE = { timeoutMs: 2_000, missedProbeLimit: 2 } -// Foreground-only sweep so a silently-dead relay surfaces without a user action. -const RELAY_IDLE_PROBE_MS = 25_000 - -// The relay session's probe budget and its timeout log line, kept apart from the -// session so the dial/RPC code and the liveness policy can each be read on its own. -export function createRelaySessionLivenessWatchdog(args: { - isForeground?: () => boolean - sendProbe: () => boolean - terminate: () => void - onLog?: ConnectionLogSink - nextLogId: () => string -}): RpcSessionLivenessWatchdog { - return new RpcSessionLivenessWatchdog({ - transport: 'relay', - idleProbeMs: RELAY_IDLE_PROBE_MS, - probeTimeoutMs: RELAY_PROBE.timeoutMs, - missedProbeLimit: RELAY_PROBE.missedProbeLimit, - voluntaryProbeMinIntervalMs: RELAY_PROBE.minIntervalMs, - urgentProbeTimeoutMs: RELAY_RESUME_PROBE.timeoutMs, - urgentMissedProbeLimit: RELAY_RESUME_PROBE.missedProbeLimit, - shouldIdleProbe: () => args.isForeground?.() ?? true, - sendProbe: args.sendProbe, - onTimeout: (evidence: LivenessTimeoutEvidence) => { - // Why: the watchdog terminates the session right after this returns. A sink - // that throws must not keep a dead relay 'connected'. - try { - args.onLog?.({ - id: args.nextLogId(), - ts: Date.now(), - level: 'error', - code: 'liveness-timeout', - path: 'relay', - message: 'Relay health check failed', - detail: `${evidence.reason}; ${evidence.missedProbes}/${evidence.missedProbeLimit} probes missed; last authenticated activity ${evidence.lastInboundAgeMs}ms ago` - }) - } catch { - // Diagnostics only. - } - }, - terminate: args.terminate - }) -} diff --git a/mobile/src/transport/rpc-client-connection-state.ts b/mobile/src/transport/rpc-client-connection-state.ts index 154acd34340..83714ecd0c0 100644 --- a/mobile/src/transport/rpc-client-connection-state.ts +++ b/mobile/src/transport/rpc-client-connection-state.ts @@ -1,4 +1,3 @@ -import { elapsedMs, monotonicNowMs } from './monotonic-clock' import { redactSocketEndpoint } from './socket-event-debug' import type { ConnectionState } from './types' @@ -13,19 +12,16 @@ type ConnectionStateOptions = { initialListener?: (state: ConnectionState) => void getReconnectAttempt: () => number isClosed: () => boolean - onStateDwell?: (previous: ConnectionState, next: ConnectionState, dweltMs: number) => void - now?: () => number } export class RpcClientConnectionState { private state: ConnectionState = 'disconnected' private lastConnectedAt: number | null = null - private stateEnteredAt: number + private stateEnteredAt = Date.now() private readonly listeners = new Set<(state: ConnectionState) => void>() private readonly waiters: ConnectWaiter[] = [] constructor(private readonly options: ConnectionStateOptions) { - this.stateEnteredAt = this.now() if (options.initialListener) { this.listeners.add(options.initialListener) } @@ -44,14 +40,9 @@ export class RpcClientConnectionState { return } const previous = this.state - const dweltMs = elapsedMs(this.stateEnteredAt, this.now()) + const dweltMs = Date.now() - this.stateEnteredAt this.state = next - this.stateEnteredAt = this.now() - try { - this.options.onStateDwell?.(previous, next, dweltMs) - } catch { - // Diagnostics only; a broken log sink must not abort the state publish. - } + this.stateEnteredAt = Date.now() console.log('[net] state', { from: previous, to: next, @@ -112,10 +103,6 @@ export class RpcClientConnectionState { return () => this.listeners.delete(listener) } - private now(): number { - return (this.options.now ?? monotonicNowMs)() - } - private resolveWaiters(): void { for (const waiter of this.waiters.splice(0)) { if (waiter.timeout) { diff --git a/mobile/src/transport/rpc-client-log-redaction.test.ts b/mobile/src/transport/rpc-client-log-redaction.test.ts index ccf5ec5febf..ff893cdde1a 100644 --- a/mobile/src/transport/rpc-client-log-redaction.test.ts +++ b/mobile/src/transport/rpc-client-log-redaction.test.ts @@ -67,9 +67,7 @@ describe('mobile rpc-client connection logs', () => { onLog: (entry) => logs.push(entry) }) - expect(logs).toContainEqual( - expect.objectContaining({ message: 'Opening WebSocket', detail: 'desktop.example:7443' }) - ) + expect(logs[0]?.detail).toBe('desktop.example:7443') expect(JSON.stringify(logs)).not.toContain('password') client.close() }) diff --git a/mobile/src/transport/rpc-session-liveness-watchdog.test.ts b/mobile/src/transport/rpc-session-liveness-watchdog.test.ts index 4b25aaa1fd5..aa1398e44b6 100644 --- a/mobile/src/transport/rpc-session-liveness-watchdog.test.ts +++ b/mobile/src/transport/rpc-session-liveness-watchdog.test.ts @@ -173,97 +173,4 @@ describe('RpcSessionLivenessWatchdog', () => { watchdog.probeNow(identity) expect(terminate).toHaveBeenCalledWith(identity) }) - function backgroundableFixture() { - const sendProbe = vi.fn(() => true) - const terminate = vi.fn() - const identity = {} - const state = { foreground: true } - const watchdog = new RpcSessionLivenessWatchdog({ - transport: 'relay', - sendProbe, - terminate, - shouldIdleProbe: () => state.foreground, - now: Date.now - }) - watchdog.start(identity) - return { identity, sendProbe, state, terminate, watchdog } - } - - it('stops retrying an idle probe once the app backgrounds under it', async () => { - const { sendProbe, state, terminate } = backgroundableFixture() - await vi.advanceTimersByTimeAsync(LIVENESS_IDLE_MS) - expect(sendProbe).toHaveBeenCalledOnce() - - // iOS suspends the socket in the background, so every further miss is evidence - // about the app and not about the peer. Retrying would spend the whole budget on - // the suspension and terminate a relay that is fine. - state.foreground = false - await vi.advanceTimersByTimeAsync(LIVENESS_PROBE_TIMEOUT_MS * 4) - expect(sendProbe).toHaveBeenCalledOnce() - expect(terminate).not.toHaveBeenCalled() - }) - - it('re-arms the idle sweep with a clean slate after a backgrounded probe', async () => { - const { sendProbe, state, terminate } = backgroundableFixture() - await vi.advanceTimersByTimeAsync(LIVENESS_IDLE_MS) - state.foreground = false - await vi.advanceTimersByTimeAsync(LIVENESS_PROBE_TIMEOUT_MS) - state.foreground = true - - // The abandoned probe must not be carried forward as a miss: the sweep needs its - // full three fair misses again before it may call the session dead. - await vi.advanceTimersByTimeAsync(LIVENESS_IDLE_MS) - expect(sendProbe).toHaveBeenCalledTimes(2) - await vi.advanceTimersByTimeAsync(LIVENESS_PROBE_TIMEOUT_MS * 2) - expect(terminate).not.toHaveBeenCalled() - await vi.advanceTimersByTimeAsync(LIVENESS_PROBE_TIMEOUT_MS) - expect(terminate).toHaveBeenCalledOnce() - }) - - it('gives a resume probe its own miss budget, not the one the ordinary probe spent', async () => { - // Why: the urgent profile exists to tolerate one slow answer from a cold radio. Inheriting - // an ordinary miss spends that tolerance before the resume probe is even sent, so the first - // slow answer on a healthy socket kills the session -- the case the profile was added for. - const terminate = vi.fn() - const sendProbe = vi.fn(() => true) - const identity = {} - const watchdog = new RpcSessionLivenessWatchdog({ - transport: 'relay', - idleProbeMs: 20_000, - probeTimeoutMs: 4_000, - missedProbeLimit: 2, - urgentProbeTimeoutMs: 2_000, - urgentMissedProbeLimit: 2, - shouldIdleProbe: () => true, - sendProbe, - terminate, - now: Date.now - }) - watchdog.start(identity) - - // One ordinary miss on the idle sweep, tolerated, and a second ordinary probe in flight. - await vi.advanceTimersByTimeAsync(20_000) - await vi.advanceTimersByTimeAsync(4_000) - expect(terminate).not.toHaveBeenCalled() - - // Foreground: the resume probe supersedes the ordinary one still in flight. - watchdog.probeNow(identity, 'resume') - await vi.advanceTimersByTimeAsync(2_000) - expect(terminate).not.toHaveBeenCalled() - - // The second urgent miss is the one that may terminate. - await vi.advanceTimersByTimeAsync(2_000) - expect(terminate).toHaveBeenCalledOnce() - }) - - it('still reaches a verdict on a caller probe when the app backgrounds', async () => { - // The gate covers the idle sweep only. A nudge or resume probe was asked for on - // purpose, and abandoning it would leave a genuinely dead socket unreported. - const { identity, state, terminate, watchdog } = backgroundableFixture() - watchdog.probeNow(identity) - state.foreground = false - - await vi.advanceTimersByTimeAsync(LIVENESS_PROBE_TIMEOUT_MS * 3) - expect(terminate).toHaveBeenCalledOnce() - }) }) diff --git a/mobile/src/transport/rpc-session-liveness-watchdog.ts b/mobile/src/transport/rpc-session-liveness-watchdog.ts index e34c47bd67a..36525f60fb0 100644 --- a/mobile/src/transport/rpc-session-liveness-watchdog.ts +++ b/mobile/src/transport/rpc-session-liveness-watchdog.ts @@ -13,19 +13,11 @@ type WatchdogOptions = { probeTimeoutMs?: number missedProbeLimit?: number voluntaryProbeMinIntervalMs?: number - // Bounds for probeImmediately(); default to the ordinary probe bounds. - urgentProbeTimeoutMs?: number - urgentMissedProbeLimit?: number - // Gates the idle sweep only. False re-arms without probing — a backgrounded app - // must not spend a probe, and its resume probes immediately anyway. - shouldIdleProbe?: () => boolean now?: () => number setTimer?: typeof setTimeout clearTimer?: typeof clearTimeout } -type ProbeProfile = { timeoutMs: number; missedProbeLimit: number } - export type LivenessTimeoutEvidence = { transport: 'direct' | 'relay' reason: 'probe-send-failed' | 'probe-timeout' @@ -38,15 +30,12 @@ export class RpcSessionLivenessWatchdog { private identity: RpcSessionIdentity | null = null private timer: ReturnType | null = null private probing = false - // Whether the probe in flight came from the idle sweep rather than a caller. - private idleSweepProbe = false private missedProbes = 0 private lastInboundAt = 0 private lastVoluntaryProbeAt: number | null = null - private profile: ProbeProfile private readonly idleProbeMs: number | null - private readonly ordinaryProfile: ProbeProfile - private readonly urgentProfile: ProbeProfile + private readonly probeTimeoutMs: number + private readonly missedProbeLimit: number private readonly voluntaryProbeMinIntervalMs: number private readonly now: () => number private readonly setTimer: typeof setTimeout @@ -54,15 +43,8 @@ export class RpcSessionLivenessWatchdog { constructor(private readonly options: WatchdogOptions) { this.idleProbeMs = options.idleProbeMs === undefined ? LIVENESS_IDLE_MS : options.idleProbeMs - this.ordinaryProfile = { - timeoutMs: options.probeTimeoutMs ?? LIVENESS_PROBE_TIMEOUT_MS, - missedProbeLimit: options.missedProbeLimit ?? MISSED_PROBE_LIMIT - } - this.urgentProfile = { - timeoutMs: options.urgentProbeTimeoutMs ?? this.ordinaryProfile.timeoutMs, - missedProbeLimit: options.urgentMissedProbeLimit ?? this.ordinaryProfile.missedProbeLimit - } - this.profile = this.ordinaryProfile + this.probeTimeoutMs = options.probeTimeoutMs ?? LIVENESS_PROBE_TIMEOUT_MS + this.missedProbeLimit = options.missedProbeLimit ?? MISSED_PROBE_LIMIT this.voluntaryProbeMinIntervalMs = options.voluntaryProbeMinIntervalMs ?? 0 this.now = options.now ?? Date.now this.setTimer = options.setTimer ?? setTimeout @@ -73,11 +55,9 @@ export class RpcSessionLivenessWatchdog { this.clearActiveTimer() this.identity = identity this.probing = false - this.idleSweepProbe = false this.missedProbes = 0 this.lastInboundAt = this.now() this.lastVoluntaryProbeAt = null - this.profile = this.ordinaryProfile this.armIdle(identity) } @@ -104,28 +84,22 @@ export class RpcSessionLivenessWatchdog { } this.missedProbes = 0 this.probing = false - this.idleSweepProbe = false this.armIdle(identity) } - // 'resume' is evidence the socket may have died while the process was suspended: - // it ignores the voluntary minimum, runs on the urgent bounds, and replaces any - // probe already in flight so the verdict lands on the short clock. - probeNow(identity: RpcSessionIdentity, urgency: 'nudge' | 'resume' = 'nudge'): void { - const urgent = urgency === 'resume' - if (this.identity !== identity || (this.probing && !urgent)) { + probeNow(identity: RpcSessionIdentity): void { + if (this.identity !== identity || this.probing) { return } const now = this.now() if ( - !urgent && this.lastVoluntaryProbeAt !== null && now - this.lastVoluntaryProbeAt < this.voluntaryProbeMinIntervalMs ) { return } this.lastVoluntaryProbeAt = now - this.startProbe(identity, urgent ? this.urgentProfile : this.ordinaryProfile) + this.startProbe(identity) } stop(identity: RpcSessionIdentity): void { @@ -135,11 +109,9 @@ export class RpcSessionLivenessWatchdog { this.clearActiveTimer() this.identity = null this.probing = false - this.idleSweepProbe = false this.missedProbes = 0 this.lastInboundAt = 0 this.lastVoluntaryProbeAt = null - this.profile = this.ordinaryProfile } private armIdle(identity: RpcSessionIdentity, delayMs = this.idleProbeMs): void { @@ -152,37 +124,21 @@ export class RpcSessionLivenessWatchdog { if (this.identity !== identity) { return } - if (this.options.shouldIdleProbe && !this.options.shouldIdleProbe()) { - this.armIdle(identity) - return - } const idleMs = this.now() - this.lastInboundAt if (this.idleProbeMs !== null && idleMs < this.idleProbeMs) { this.armIdle(identity, Math.max(1, this.idleProbeMs - Math.max(0, idleMs))) } else { - this.startProbe(identity, this.ordinaryProfile, true) + this.startProbe(identity) } }, delayMs) } - private startProbe( - identity: RpcSessionIdentity, - profile = this.ordinaryProfile, - fromIdleSweep = false - ): void { + private startProbe(identity: RpcSessionIdentity): void { if (this.identity !== identity) { return } this.clearActiveTimer() - // Why: switching profile starts a new observation window on a different clock. Carrying the - // ordinary probe's misses into the urgent one spends the tolerated slow answer that profile - // exists to give a cold radio, so the first 2s miss would kill a healthy socket. - if (profile !== this.profile) { - this.missedProbes = 0 - } - this.profile = profile this.probing = true - this.idleSweepProbe = fromIdleSweep const sentAt = this.now() let sent = false try { @@ -194,7 +150,7 @@ export class RpcSessionLivenessWatchdog { this.terminateCurrent(identity, 'probe-send-failed') return } - this.timer = this.setTimer(() => this.handleProbeTimeout(identity, sentAt), profile.timeoutMs) + this.timer = this.setTimer(() => this.handleProbeTimeout(identity, sentAt), this.probeTimeoutMs) } private handleProbeTimeout(identity: RpcSessionIdentity, sentAt: number): void { @@ -202,38 +158,27 @@ export class RpcSessionLivenessWatchdog { if (this.identity !== identity) { return } - // Why: the idle sweep is foreground-only because iOS suspends sockets in the - // background, where a miss is not evidence of a dead peer. Retrying here would - // spend the whole miss budget on that suspension and kill a healthy session. - if (this.idleSweepProbe && this.options.shouldIdleProbe && !this.options.shouldIdleProbe()) { - this.probing = false - this.idleSweepProbe = false - this.missedProbes = 0 - this.armIdle(identity) - return - } - const profile = this.profile const elapsedMs = this.now() - sentAt - if (elapsedMs < 0 || elapsedMs > profile.timeoutMs * 1.5) { + if (elapsedMs < 0 || elapsedMs > this.probeTimeoutMs * 1.5) { console.log('[net] activity-probe unfair window skipped', { transport: this.options.transport, elapsedMs, - timeoutMs: profile.timeoutMs + timeoutMs: this.probeTimeoutMs }) - this.startProbe(identity, profile, this.idleSweepProbe) + this.startProbe(identity) return } this.missedProbes += 1 - if (this.missedProbes >= profile.missedProbeLimit) { + if (this.missedProbes >= this.missedProbeLimit) { this.terminateCurrent(identity, 'probe-timeout') return } console.log('[net] activity-probe timeout tolerated', { transport: this.options.transport, missedProbes: this.missedProbes, - missedProbeLimit: profile.missedProbeLimit + missedProbeLimit: this.missedProbeLimit }) - this.startProbe(identity, profile, this.idleSweepProbe) + this.startProbe(identity) } private terminateCurrent( @@ -246,17 +191,16 @@ export class RpcSessionLivenessWatchdog { this.clearActiveTimer() this.identity = null this.probing = false - this.idleSweepProbe = false console.log('[net] activity-probe TIMEOUT — forcing reconnect', { transport: this.options.transport, missedProbes: this.missedProbes, - missedProbeLimit: this.profile.missedProbeLimit + missedProbeLimit: this.missedProbeLimit }) this.options.onTimeout?.({ transport: this.options.transport, reason, missedProbes: this.missedProbes, - missedProbeLimit: this.profile.missedProbeLimit, + missedProbeLimit: this.missedProbeLimit, lastInboundAgeMs: Math.max(0, this.now() - this.lastInboundAt) }) this.options.terminate(identity) diff --git a/mobile/src/transport/runtime-status-probe.test.ts b/mobile/src/transport/runtime-capability-probe.test.ts similarity index 67% rename from mobile/src/transport/runtime-status-probe.test.ts rename to mobile/src/transport/runtime-capability-probe.test.ts index 9b1de5c391e..2272c25610f 100644 --- a/mobile/src/transport/runtime-status-probe.test.ts +++ b/mobile/src/transport/runtime-capability-probe.test.ts @@ -1,9 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { - readRuntimeCapabilities, - startRuntimeCapabilityProbe, - startRuntimeStatusProbe -} from './runtime-status-probe' +import { startRuntimeCapabilityProbe } from './runtime-capability-probe' import { LogicalClientCutoverError } from './stable-logical-rpc-client' import type { RpcClient } from './rpc-client' import type { RpcResponse } from './types' @@ -129,46 +125,19 @@ describe('startRuntimeCapabilityProbe', () => { cancel() }) - // Was: an ok:false response was retried like a timeout. The probe now backs the gate that sits - // above every /h/ route, so polling a host that already answered would run for the life of the - // connection. A reply is an answer; only an unanswered request is retried. - it('settles once on an ok:false response rather than polling the host', async () => { + it('retries an ok:false response instead of settling', async () => { const failure: RpcResponse = { ok: false, id: '1', error: { code: 'internal', message: 'nope' }, _meta: { runtimeId: 'r1' } } - const { client, calls } = makeClient([failure, ok(['a.v1'])]) + const { client } = makeClient([failure, ok(['a.v1'])]) const seen: (readonly string[])[] = [] - const retrying: boolean[] = [] - const cancel = startRuntimeStatusProbe(client, { - onStatus: (status) => seen.push(readRuntimeCapabilities(status)), - onUnavailable: (isRetrying) => retrying.push(isRetrying) - }) + const cancel = startRuntimeCapabilityProbe(client, (capabilities) => seen.push(capabilities)) await flushMicrotasks() - expect(retrying).toEqual([false]) expect(seen).toEqual([]) - - await vi.advanceTimersByTimeAsync(60_000) - expect(calls()).toBe(1) - expect(seen).toEqual([]) - cancel() - }) - - it('still retries a request the host never answered', async () => { - const { client, calls } = makeClient([new Error('timeout'), ok(['a.v1'])]) - const seen: (readonly string[])[] = [] - const retrying: boolean[] = [] - const cancel = startRuntimeStatusProbe(client, { - onStatus: (status) => seen.push(readRuntimeCapabilities(status)), - onUnavailable: (isRetrying) => retrying.push(isRetrying) - }) - await flushMicrotasks() - expect(retrying).toEqual([true]) - await vi.advanceTimersByTimeAsync(1_000) - expect(calls()).toBe(2) expect(seen).toEqual([['a.v1']]) cancel() }) @@ -213,40 +182,4 @@ describe('startRuntimeCapabilityProbe', () => { await flushMicrotasks() expect(seen).toEqual([]) }) - - it('reports the full status, not just capabilities', async () => { - const response: RpcResponse = { - ok: true, - id: '1', - result: { appVersion: '1.4.0', protocolVersion: 7, capabilities: ['a.v1'] }, - _meta: { runtimeId: 'r1' } - } - const { client } = makeClient([response]) - const seen: Record[] = [] - const cancel = startRuntimeStatusProbe(client, { onStatus: (status) => seen.push(status) }) - await flushMicrotasks() - expect(seen).toEqual([{ appVersion: '1.4.0', protocolVersion: 7, capabilities: ['a.v1'] }]) - cancel() - }) - - // Why: the gate needs to release its pending cover on the first miss rather than wait out the - // retries, so a wedged status.get cannot hold the whole host UI behind a spinner. - it('announces each failed attempt while the retry is still pending', async () => { - const { client, calls } = makeClient([new Error('boom'), ok(['a.v1'])]) - const misses: number[] = [] - const seen: Record[] = [] - const cancel = startRuntimeStatusProbe(client, { - onStatus: (status) => seen.push(status), - onUnavailable: () => misses.push(calls()) - }) - await flushMicrotasks() - expect(misses).toEqual([1]) - expect(seen).toEqual([]) - - await vi.advanceTimersByTimeAsync(1_000) - await flushMicrotasks() - expect(seen).toEqual([{ capabilities: ['a.v1'] }]) - expect(misses).toEqual([1]) - cancel() - }) }) diff --git a/mobile/src/transport/runtime-status-probe.ts b/mobile/src/transport/runtime-capability-probe.ts similarity index 51% rename from mobile/src/transport/runtime-status-probe.ts rename to mobile/src/transport/runtime-capability-probe.ts index 03cec914871..ef636552863 100644 --- a/mobile/src/transport/runtime-status-probe.ts +++ b/mobile/src/transport/runtime-capability-probe.ts @@ -9,20 +9,9 @@ const CUTOVER_RETRY_DELAY_MS = 250 const FAILURE_RETRY_BASE_DELAY_MS = 1_000 const FAILURE_RETRY_MAX_DELAY_MS = 15_000 -export type RuntimeStatusProbeHandlers = { - onStatus: (status: Record) => void - // Fires once per attempt that produced no status. `retrying` is false when the host itself - // answered with an error: that is a definitive reply, so the probe stops rather than polling a - // host that has already said no. It is true when nothing reached us and a retry is armed, which - // lets a caller that must not stay blocked fail open on the first miss and be upgraded later. - onUnavailable?: (retrying: boolean) => void -} - -// Single status.get producer for a connected client: one request, retried until it -// lands. Callers share the answer instead of each issuing their own status.get. -export function startRuntimeStatusProbe( - client: Pick, - handlers: RuntimeStatusProbeHandlers +export function startRuntimeCapabilityProbe( + client: RpcClient, + onCapabilities: (capabilities: readonly string[]) => void ): () => void { let cancelled = false let retryTimer: ReturnType | null = null @@ -35,15 +24,20 @@ export function startRuntimeStatusProbe( return } if (!response.ok) { - // Why not retry: the desktop replied. Re-asking every 15 s for the life of a connection - // from a probe mounted above every /h/ route buys nothing a reconnect would not. - handlers.onUnavailable?.(false) + scheduleRetry(false) return } const result = (response as RpcSuccess).result - handlers.onStatus( - result && typeof result === 'object' ? (result as Record) : {} - ) + const rawCapabilities = + result && typeof result === 'object' + ? (result as { capabilities?: unknown }).capabilities + : null + const capabilities = + Array.isArray(rawCapabilities) && + rawCapabilities.every((value) => typeof value === 'string') + ? rawCapabilities + : [] + onCapabilities(capabilities) }, (error: unknown) => { if (cancelled) { @@ -61,7 +55,6 @@ export function startRuntimeStatusProbe( ? CUTOVER_RETRY_DELAY_MS : Math.min(FAILURE_RETRY_BASE_DELAY_MS * 2 ** failureRetries++, FAILURE_RETRY_MAX_DELAY_MS) retryTimer = setTimeout(attempt, delay) - handlers.onUnavailable?.(true) } attempt() @@ -72,17 +65,3 @@ export function startRuntimeStatusProbe( } } } - -export function readRuntimeCapabilities(status: Record): readonly string[] { - const raw = status.capabilities - return Array.isArray(raw) && raw.every((value) => typeof value === 'string') ? raw : [] -} - -export function startRuntimeCapabilityProbe( - client: Pick, - onCapabilities: (capabilities: readonly string[]) => void -): () => void { - return startRuntimeStatusProbe(client, { - onStatus: (status) => onCapabilities(readRuntimeCapabilities(status)) - }) -} diff --git a/mobile/src/transport/types.ts b/mobile/src/transport/types.ts index 2a47d1ca1ea..c40bd979936 100644 --- a/mobile/src/transport/types.ts +++ b/mobile/src/transport/types.ts @@ -58,17 +58,6 @@ export type ConnectionDiagnosticCode = | 'relay-credential-unavailable' | 'host-open-failed' -// Why: a 10s connect used to read as one opaque "connecting" span. Attaching the -// duration of the phase an entry closes out lets the report say where the time -// went. Diagnostics only — nothing schedules from these. -export type ConnectionLogTiming = { - kind: 'relay-dial-stage' | 'connection-state' - name: string - ms: number - // False when the phase never finished (the dial died inside it). - complete: boolean -} - export type ConnectionLogEntry = { id: string ts: number @@ -79,7 +68,6 @@ export type ConnectionLogEntry = { detail?: string code?: ConnectionDiagnosticCode path?: MobileConnectionDiagnosticPath - timing?: ConnectionLogTiming } export type ConnectionLogSink = (entry: ConnectionLogEntry) => void @@ -88,7 +76,7 @@ export type ConnectionLogEmitter = ( level: ConnectionLogLevel, message: string, detail?: string, - evidence?: Pick + evidence?: Pick ) => void export type ConnectionState = @@ -99,16 +87,6 @@ export type ConnectionState = | 'reconnecting' | 'auth-failed' -// Exhaustive by construction; see RELAY_DIAL_STAGE_NAMES for why. -export const CONNECTION_STATE_NAMES: Record = { - connecting: true, - handshaking: true, - connected: true, - disconnected: true, - reconnecting: true, - 'auth-failed': true -} - // Why: a user-attention nudge must not tear down a healthy relay (probe it); only a // network-change nudge marks the socket suspect enough to replace it. export type ForegroundNudgeReason = 'focus' | 'app-resume' | 'network-change' diff --git a/mobile/src/transport/unpaired-host-credential-deletion.test.ts b/mobile/src/transport/unpaired-host-credential-deletion.test.ts deleted file mode 100644 index 38731afec4d..00000000000 --- a/mobile/src/transport/unpaired-host-credential-deletion.test.ts +++ /dev/null @@ -1,104 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' - -const asyncStorage = vi.hoisted(() => ({ - getItem: vi.fn(async () => null), - setItem: vi.fn(async () => undefined), - removeItem: vi.fn(async () => undefined) -})) -const deletions = vi.hoisted(() => ({ - deviceToken: vi.fn(async () => undefined), - credentialBundle: vi.fn(async () => undefined), - directUpgradeJournal: vi.fn(async () => undefined), - clearWriteRevision: vi.fn() -})) - -vi.mock('@react-native-async-storage/async-storage', () => ({ default: asyncStorage })) -vi.mock('./host-device-token-store', () => ({ deleteHostDeviceToken: deletions.deviceToken })) -vi.mock('./mobile-relay-credential-bundle', () => ({ - deleteMobileRelayCredentialBundle: deletions.credentialBundle -})) -vi.mock('./mobile-relay-direct-upgrade-journal', () => ({ - deleteMobileRelayDirectUpgradeJournal: deletions.directUpgradeJournal -})) -vi.mock('./host-credential-write-revision', () => ({ - clearHostCredentialWriteRevision: deletions.clearWriteRevision, - getHostCredentialWriteRevision: () => 0 -})) - -import { createUnpairedHostCredentialDeletion } from './unpaired-host-credential-deletion' -import { - getSessionTabStripCacheKey, - readCachedSessionTabStrip, - resetSessionTabStripCacheForTests, - saveCachedSessionTabStrip -} from '../cache/session-tab-strip-cache' - -const strip = { - tabs: [{ id: 'tab-1', type: 'terminal' as const, title: 'Terminal', agentId: null }], - activeTabId: 'tab-1' -} - -function createDeletion(storedHostIds: string[] = []) { - return createUnpairedHostCredentialDeletion({ - waitForHostMutations: async () => undefined, - hasStoredHost: async (hostId) => storedHostIds.includes(hostId), - onDeleted: vi.fn() - }) -} - -beforeEach(() => { - asyncStorage.getItem.mockClear() - asyncStorage.setItem.mockClear() - for (const mock of Object.values(deletions)) { - mock.mockClear() - } - resetSessionTabStripCacheForTests() -}) - -describe('unpaired host credential deletion', () => { - it('takes the cached tab strip with the credentials, leaving other hosts alone', async () => { - // Why: the strip is not a credential, but it is host-scoped plaintext written from the - // session screen. Without this sweep it outlives the pairing that produced it. - const unpaired = getSessionTabStripCacheKey('host-1', 'wt-1') - const other = getSessionTabStripCacheKey('host-2', 'wt-1') - saveCachedSessionTabStrip(unpaired, strip) - saveCachedSessionTabStrip(other, strip) - - await createDeletion()('host-1', 0) - - expect(readCachedSessionTabStrip(unpaired)).toBeNull() - expect(readCachedSessionTabStrip(other)?.tabs).toHaveLength(1) - }) - - it('finishes the cleanup when the cache purge fails', async () => { - // Why: every credential above is already deleted by this point. Aborting on the cache - // would strand the write revision and leave onDeleted's token cache holding a host whose - // credentials are gone, retried only by an explicit Settings action. - const onDeleted = vi.fn() - const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) - asyncStorage.setItem.mockRejectedValueOnce(new Error('disk full')) - - await expect( - createUnpairedHostCredentialDeletion({ - waitForHostMutations: async () => undefined, - hasStoredHost: async () => false, - onDeleted - })('host-1', 0) - ).resolves.toBeUndefined() - - expect(deletions.clearWriteRevision).toHaveBeenCalledWith('host-1') - expect(onDeleted).toHaveBeenCalledWith('host-1') - expect(warn).toHaveBeenCalled() - warn.mockRestore() - }) - - it('leaves the strip alone when the host turned out to still be paired', async () => { - const stillPaired = getSessionTabStripCacheKey('host-1', 'wt-1') - saveCachedSessionTabStrip(stillPaired, strip) - - await createDeletion(['host-1'])('host-1', 0) - - expect(readCachedSessionTabStrip(stillPaired)?.tabs).toHaveLength(1) - expect(deletions.deviceToken).not.toHaveBeenCalled() - }) -}) diff --git a/mobile/src/transport/unpaired-host-credential-deletion.ts b/mobile/src/transport/unpaired-host-credential-deletion.ts index 6b7ef55aa31..cc9c27e49ad 100644 --- a/mobile/src/transport/unpaired-host-credential-deletion.ts +++ b/mobile/src/transport/unpaired-host-credential-deletion.ts @@ -1,4 +1,3 @@ -import { deleteCachedSessionTabStripForHost } from '../cache/session-tab-strip-cache' import { deleteHostDeviceToken } from './host-device-token-store' import { clearHostCredentialWriteRevision, @@ -53,18 +52,6 @@ export function createUnpairedHostCredentialDeletion(dependencies: DeletionDepen return } assertWriteRevisionUnchanged(hostId, writeRevision) - // The cached tab strip is not a credential, but it is host-scoped plaintext that outlives - // the pairing unless this sweep takes it too. Warned rather than thrown, as - // removeHostAndCloseClient does: every credential above is already gone, so aborting here - // would strand the write revision and leave onDeleted's token cache holding a host whose - // credentials no longer exist. The cache refuses further saves for this host either way. - await deleteCachedSessionTabStripForHost(hostId).catch((error: unknown) => { - console.warn('[unpaired-host-cleanup] cached tab strip delete failed', error) - }) - if (await shouldSkip(hostId, writeRevision)) { - return - } - assertWriteRevisionUnchanged(hostId, writeRevision) clearHostCredentialWriteRevision(hostId) dependencies.onDeleted(hostId) } diff --git a/mobile/src/worktree/home-host-worktree-fetch.ts b/mobile/src/worktree/home-host-worktree-fetch.ts index 4e9cfa3b2ab..72b9e572ba1 100644 --- a/mobile/src/worktree/home-host-worktree-fetch.ts +++ b/mobile/src/worktree/home-host-worktree-fetch.ts @@ -13,7 +13,7 @@ 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-status-probe.ts for the same hazard on status.get. +// can't spin. See runtime-capability-probe.ts for the same hazard on status.get. const CUTOVER_RETRY_LIMIT = 2 export type HostWorktreeInfoSetter = (