diff --git a/mobile/src/transport/mobile-direct-return-probe.test.ts b/mobile/src/transport/mobile-direct-return-probe.test.ts index 8be5c00c805..6e73243cbf9 100644 --- a/mobile/src/transport/mobile-direct-return-probe.test.ts +++ b/mobile/src/transport/mobile-direct-return-probe.test.ts @@ -28,7 +28,10 @@ function fixture() { }), 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 () => {}, diff --git a/mobile/src/transport/mobile-direct-return-probe.ts b/mobile/src/transport/mobile-direct-return-probe.ts index 7bb4d97ce7a..5c996a4f001 100644 --- a/mobile/src/transport/mobile-direct-return-probe.ts +++ b/mobile/src/transport/mobile-direct-return-probe.ts @@ -6,8 +6,11 @@ import type { MobileConnectionPath } from './stable-logical-rpc-client' const DIRECT_PROBE_INTERVAL_MS = 15_000 -// While the runtime channel rides the relay, periodically probe the direct -// endpoint and migrate back once hysteresis proves it stable. +// 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. export class DirectReturnProbe { private timer: ReturnType | null = null @@ -27,7 +30,13 @@ 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: ( @@ -61,6 +70,16 @@ 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) { @@ -79,7 +98,11 @@ export class DirectReturnProbe { if (this.stopped) { return } - if (!this.hooks.canAttempt() || !this.hooks.hysteresis.canProbe(this.deps.now())) { + // 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()))) { this.schedule() return } @@ -90,7 +113,8 @@ export class DirectReturnProbe { 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. Only the cutover needs the mutex. + // 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, @@ -106,23 +130,40 @@ export class DirectReturnProbe { } // Both early returns leave the candidate to the finally, which owns it until // migration takes over — closing here too would double-close it. - if (!this.hooks.hysteresis.recordDirectSuccess(this.deps.now())) { + 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()))) { return } - if (!this.hooks.canAttempt()) { + 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 } - this.hooks.beginOperation() - owned = true + 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, () => this.stopped) + await this.hooks.migrate(candidate.client, candidate.path, abortCutover) } catch (error) { - if (this.stopped) { + // 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()) { return } throw error diff --git a/mobile/src/transport/mobile-endpoint-reconnect-race.test.ts b/mobile/src/transport/mobile-endpoint-reconnect-race.test.ts new file mode 100644 index 00000000000..47f41e0556d --- /dev/null +++ b/mobile/src/transport/mobile-endpoint-reconnect-race.test.ts @@ -0,0 +1,362 @@ +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-direct-probe.test.ts b/mobile/src/transport/mobile-endpoint-supervisor-direct-probe.test.ts index 0e8f32ee5e3..634953d93ff 100644 --- a/mobile/src/transport/mobile-endpoint-supervisor-direct-probe.test.ts +++ b/mobile/src/transport/mobile-endpoint-supervisor-direct-probe.test.ts @@ -6,7 +6,8 @@ import { FakeLogicalClient, FakeRelaySession, FakeSession, - host + host, + unreachableDirect } from './mobile-endpoint-supervisor-test-fakes' // A cell that authenticates and then answers the confirm for a different relay host @@ -87,7 +88,12 @@ describe('mobile endpoint supervisor direct probe', () => { const recordMigration = vi.spyOn(MobileEndpointHysteresis.prototype, 'recordMigration') const logical = new FakeLogicalClient('disconnected', 'lan') const openRelay = vi.fn(() => confirmRejectingRelaySession(logical)) - const deps = dependencies({ openRelay, randomBytes: () => new Uint8Array([128, 0]) }) + // 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() diff --git a/mobile/src/transport/mobile-endpoint-supervisor-test-fakes.ts b/mobile/src/transport/mobile-endpoint-supervisor-test-fakes.ts index cc4d91ea9da..c646fc3000c 100644 --- a/mobile/src/transport/mobile-endpoint-supervisor-test-fakes.ts +++ b/mobile/src/transport/mobile-endpoint-supervisor-test-fakes.ts @@ -204,6 +204,15 @@ 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 028387d8232..f8f8e50d094 100644 --- a/mobile/src/transport/mobile-endpoint-supervisor.test.ts +++ b/mobile/src/transport/mobile-endpoint-supervisor.test.ts @@ -10,7 +10,8 @@ import { FakeSession, host, mockCredentialRotation, - relay + relay, + unreachableDirect } from './mobile-endpoint-supervisor-test-fakes' import { MobileEndpointSupervisor } from './mobile-endpoint-supervisor' @@ -48,19 +49,17 @@ describe('mobile endpoint supervisor', () => { supervisor.stop() }) - it('fails over when the direct retry loop publishes reconnecting', async () => { + it('fails over while the direct retry loop is still dialing', async () => { const logical = new FakeLogicalClient('connecting', 'lan') - const deps = dependencies() + const deps = dependencies({ openDirect: unreachableDirect() }) 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).not.toHaveBeenCalled() - - supervisor.setForeground(true) - await vi.advanceTimersByTimeAsync(0) - expect(deps.openRelay).not.toHaveBeenCalled() + expect(deps.openRelay).toHaveBeenCalledOnce() logical.publishState('reconnecting') await vi.waitFor(() => expect(logical.getActivePath()).toBe('relay')) @@ -148,11 +147,12 @@ describe('mobile endpoint supervisor', () => { expect(logical.getPendingPath()).toBeNull() }) - it('does not spend a queued relay retry while direct authentication is progressing', async () => { + it('keeps retrying relay on its own cadence while a direct handshake drags on', 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,12 +160,13 @@ 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(250) + await vi.advanceTimersByTimeAsync(249) expect(openRelay).toHaveBeenCalledOnce() - - logical.publishState('disconnected') - await vi.waitFor(() => expect(openRelay).toHaveBeenCalledTimes(2)) + await vi.advanceTimersByTimeAsync(1) + expect(openRelay).toHaveBeenCalledTimes(2) supervisor.stop() }) @@ -244,6 +245,7 @@ describe('mobile endpoint supervisor', () => { const deps = dependencies({ openRelay, onLog, + openDirect: unreachableDirect(), randomBytes: () => new Uint8Array([128, 0]) }) const supervisor = new MobileEndpointSupervisor(logical, host, deps) @@ -288,6 +290,7 @@ 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) @@ -470,6 +473,7 @@ describe('mobile endpoint supervisor', () => { .mockImplementation(() => new FakeRelaySession('connected')) const deps = dependencies({ openRelay, + openDirect: unreachableDirect(), writeBundle: vi.fn(() => writePending), randomBytes: () => new Uint8Array([128, 0]) }) @@ -808,6 +812,7 @@ 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) @@ -844,43 +849,6 @@ 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() @@ -893,39 +861,4 @@ 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 372fd7372a2..450ec656d88 100644 --- a/mobile/src/transport/mobile-endpoint-supervisor.ts +++ b/mobile/src/transport/mobile-endpoint-supervisor.ts @@ -10,14 +10,11 @@ import { } from './mobile-endpoint-supervisor-support' import { selectDialableRelayCredentials } from './mobile-relay-credential-selection' import { createRelayRecoveryLog, type RelayRecoveryLog } from './mobile-relay-recovery-log' -import { - mobileRelayCredentialNeedsRotation, - rotateMobileRelayCredential -} from './mobile-relay-credential-rotation' +import { MobileRelayCredentialRefresh } from './mobile-relay-credential-refresh' import type { MobileRelayCredentialBundle } from './mobile-relay-credential-bundle' import { MobileEndpointNudgeRouter } from './mobile-endpoint-nudge-router' import { RelayRecoveryIntentQueue } from './relay-recovery-intent-queue' -import { MobileRelayDirectGraceTimer } from './mobile-relay-direct-grace-timer' +import { RelayLostRaceDamper } from './mobile-relay-lost-race-damper' import { MobileRelaySessionEstablisher } from './mobile-relay-session-establisher' import * as recoveryPresentation from './mobile-relay-recovery-presentation' import type { StableLogicalRpcClient } from './stable-logical-rpc-client' @@ -41,7 +38,7 @@ export class MobileEndpointSupervisor { private operationInFlight = false private readonly pending = new RelayRecoveryIntentQueue() private readonly nudgeRouter: MobileEndpointNudgeRouter - private credentialRotationInFlight = false + private readonly credentialRefresh: MobileRelayCredentialRefresh private relayRotationPending = false private unsubscribeState: (() => void) | null = null private readonly hysteresis: MobileEndpointHysteresis @@ -49,7 +46,7 @@ export class MobileEndpointSupervisor { private readonly leaseRotation: RelayLeaseRotationTimer private readonly logRelay: RelayRecoveryLog private readonly directProbe: DirectReturnProbe - private readonly directGrace: MobileRelayDirectGraceTimer + private readonly lostRace: RelayLostRaceDamper private readonly backgroundGrace: MobileRelayBackgroundGrace private readonly sessionEstablisher: MobileRelaySessionEstablisher @@ -65,6 +62,27 @@ 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({ @@ -74,18 +92,17 @@ export class MobileEndpointSupervisor { isForeground: () => this.backgroundGrace.isForeground(), setForeground: (foreground) => this.setForeground(foreground), replaceRelay: () => void this.recoverRelay(true, true), - scheduleDirectProbe: () => this.directProbe.schedule(0) + 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() }) 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, @@ -103,6 +120,7 @@ export class MobileEndpointSupervisor { adoptBundle: (bundle) => (this.bundle = bundle), recordMigration: () => { this.relayRotationPending = false + this.lostRace.reset() this.hysteresis.recordMigration(dependencies.now()) logRelayConnected(this.logRelay) }, @@ -119,13 +137,17 @@ 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.rotateCredentialIfNeeded(this.relayReconnect.resetForDirectConnection()) + await this.credentialRefresh.run(this.relayReconnect.resetForDirectConnection()) }, afterProbe: () => { this.operationInFlight = false @@ -140,8 +162,7 @@ export class MobileEndpointSupervisor { logical, this.relayReconnect, this.leaseRotation, - this.directProbe, - this.directGrace + this.directProbe ) } @@ -157,12 +178,18 @@ export class MobileEndpointSupervisor { } this.unsubscribeState = this.logical.onStateChange((state) => { if (state === 'connected') { - this.directGrace.clear() + this.lostRace.noteDirectRestored() if (this.logical.getActivePath() !== 'relay') { - void this.rotateCredentialIfNeeded(this.relayReconnect.resetForDirectConnection()) + void this.credentialRefresh.run(this.relayReconnect.resetForDirectConnection()) } this.directProbe.schedule() - } else if (!this.backgroundGrace.isForeground()) { + 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()) { this.backgroundGrace.handleStateFailure() } else { // Why: the direct client enters reconnecting after its first failed @@ -172,17 +199,21 @@ export class MobileEndpointSupervisor { logRelayDialFailure(this.logRelay, relayFailure, 'active-session') } }) - 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 { + if (this.logical.getState() === 'connected') { this.directProbe.schedule() - this.directGrace.arm() + return } + // 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) @@ -194,6 +225,7 @@ export class MobileEndpointSupervisor { stop(): void { this.stopped = true this.pending.clear() + this.lostRace.reset() this.directProbe.stop() this.unsubscribeState?.() this.unsubscribeState = null @@ -204,8 +236,15 @@ export class MobileEndpointSupervisor { return !this.stopped && this.backgroundGrace.isForeground() } - // forceReplacement: dial past the "direct still looks live" guard — a lease - // rotation, a network-change replacement, or the happy-eyeballs grace race. + // 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. // 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. @@ -213,6 +252,11 @@ 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 @@ -225,9 +269,13 @@ export class MobileEndpointSupervisor { forceReplacement = true ownsRecovery = true } - // 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())) { + 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') return } // Why: revival and lease timers can overlap resume failures; one shared cooldown @@ -264,9 +312,7 @@ export class MobileEndpointSupervisor { } return } - const recoveryNeeded = - forceReplacement || this.relayReconnect.needsRecovery(this.logical.getState()) - if (!this.isActive() || !recoveryNeeded) { + if (!this.isActive() || !this.relayDialAllowed(forceReplacement)) { return } this.logical.setRecoveryPath('relay', this.relayReconnect.getFailureCount()) @@ -281,6 +327,12 @@ 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; @@ -303,45 +355,4 @@ export class MobileEndpointSupervisor { } } } - - 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 cdea1374b4f..061c29a15ed 100644 --- a/mobile/src/transport/mobile-relay-background-grace.ts +++ b/mobile/src/transport/mobile-relay-background-grace.ts @@ -51,8 +51,7 @@ export class MobileRelayBackgroundGraceTimer { } type Clearable = { clear(): void } -type DirectProbe = Clearable & { schedule(delayMs?: number): void } -type DirectGrace = Clearable & { arm(): void } +type DirectProbe = Clearable & { schedule(delayMs?: number): void; probeNow(): void } export class MobileRelayBackgroundGrace { private foregroundState = true @@ -64,8 +63,7 @@ export class MobileRelayBackgroundGrace { private readonly logical: StableLogicalRpcClient, private readonly relayReconnect: RelayReconnectController, private readonly leaseRotation: Clearable, - private readonly directProbe: DirectProbe, - private readonly directGrace: DirectGrace + private readonly directProbe: DirectProbe ) { this.timer = new MobileRelayBackgroundGraceTimer(dependencies, () => this.suspendRelay()) } @@ -80,8 +78,9 @@ export class MobileRelayBackgroundGrace { if (foreground) { this.foreground() this.relayReconnect.handleForeground(this.logical, wasForeground) - this.directProbe.schedule(0) - this.directGrace.arm() + // Why: a resume dials direct alongside the relay recovery handleForeground + // just triggered; a pending probe tick must not delay this one. + this.directProbe.probeNow() } else if (wasForeground) { this.background() } @@ -92,7 +91,6 @@ export class MobileRelayBackgroundGrace { this.directProbe.clear() this.relayReconnect.clear() this.leaseRotation.clear() - this.directGrace.clear() this.logical.setRecoveryPath(null) } @@ -108,7 +106,6 @@ 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 new file mode 100644 index 00000000000..647a12a423f --- /dev/null +++ b/mobile/src/transport/mobile-relay-credential-refresh.ts @@ -0,0 +1,71 @@ +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-direct-grace-timer.ts b/mobile/src/transport/mobile-relay-direct-grace-timer.ts deleted file mode 100644 index df3c1ba1428..00000000000 --- a/mobile/src/transport/mobile-relay-direct-grace-timer.ts +++ /dev/null @@ -1,48 +0,0 @@ -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 965135511eb..6f8f0a6d9b4 100644 --- a/mobile/src/transport/mobile-relay-e2ee-link.test.ts +++ b/mobile/src/transport/mobile-relay-e2ee-link.test.ts @@ -195,6 +195,38 @@ 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 new file mode 100644 index 00000000000..b9891b8481e --- /dev/null +++ b/mobile/src/transport/mobile-relay-lost-race-damper.ts @@ -0,0 +1,99 @@ +// 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-session-establisher.ts b/mobile/src/transport/mobile-relay-session-establisher.ts index 9ec8ebb3a37..be6130dac1c 100644 --- a/mobile/src/transport/mobile-relay-session-establisher.ts +++ b/mobile/src/transport/mobile-relay-session-establisher.ts @@ -19,6 +19,25 @@ 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 @@ -113,6 +132,7 @@ export class MobileRelaySessionEstablisher { }, args.isForeground ) + const stopWithdrawWatch = withdrawWhenDirectWins(args.logical, session) try { // Why: backgrounding or a direct winner withdraws this dial before cutover. await args.logical.migrateTo( @@ -126,6 +146,10 @@ 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