From 5083b58b9c5e366e53ce79de2651989fce730f8d Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Fri, 4 Sep 2026 18:05:10 -0400 Subject: [PATCH] fix(desktop): never replay a refresh token after a timeout; jitter relay lease renewal (#18719) Two independent desktop hardening changes that share one incident date. 1. Refresh-token replay. A refresh POST that aborts client-side leaves the token's fate unknown: the server may already have rotated it before the reply was lost. Every caller above this module (the relay auth coordinator most of all) then re-entered and resent the same stored token, which the server reads as reuse and answers by revoking the whole token family. On 2026-09-04 that turned a slow refresh endpoint into 21,605 sign-outs. - The refresh endpoint now gets one 60s attempt instead of a 30s attempt plus a replayed retry. - A failure with no status line is ambiguous: it is never retried, and the token is recorded so a re-entry within 30s is refused outright rather than replayed. The block is bounded, not permanent -- the token is only possibly spent, and a permanent block would sign out every desktop whose refresh merely timed out. - Before any retry or replay the stored session is re-read; if another caller already rotated it, that result is adopted and the old token is never sent again. - Only a definitive 5xx, which proves the server rejected without rotating, is retried, and exactly once. - A 401 on a token whose earlier attempt never answered now logs orca_cloud_refresh_possible_replay so support can tell a real revocation from a sign-out we caused ourselves. clearCloudSessionIfUnchanged semantics are unchanged and still emit the invalidation event. 2. Lease renewal jitter. Both desktop renewal timers took the mean of a 60s-wide window before expiry. A cell recreate reconnects a whole cohort inside one second, so every host in it renewed inside the same second ~54 minutes later, re-bursting the fleet every ~54 minutes. Renewal now carries full +/-10% jitter, spreading the same cohort over ~10 minutes. Early renewal is free on the server side: the rebind branch resets the full 55-minute TTL from whenever it arrives, with no minimum-age or early-renewal restriction (cloud/apps/relay/src/host-session-registry.ts :736-743, and :794 for a fresh session). Only lateness is fatal -- :997 drains and closes a lease that has expired. The base interval is therefore shrunk to fit the upward jitter rather than clipping the jittered value at the margin, which keeps the distribution unbiased and keeps every sample at least 90s before expiry. No wire change. --- .../orca-profiles/profile-cloud-client.ts | 44 +++++-- .../profile-cloud-refresh-replay-guard.ts | 55 ++++++++ .../profile-cloud-service-auth-retry.test.ts | 1 + .../profile-cloud-service-refresh.test.ts | 1 + .../profile-cloud-session-refresh.test.ts | 122 ++++++++++++++++++ .../profile-cloud-session-refresh.ts | 92 ++++++++++++- src/main/runtime/relay/relay-origin-pool.ts | 4 +- .../relay/relay-renewal-jitter.test.ts | 57 ++++++++ .../runtime/relay/relay-renewal-jitter.ts | 25 ++++ .../runtime/relay/relay-session-broker.ts | 4 +- 10 files changed, 389 insertions(+), 16 deletions(-) create mode 100644 src/main/orca-profiles/profile-cloud-refresh-replay-guard.ts create mode 100644 src/main/runtime/relay/relay-renewal-jitter.test.ts create mode 100644 src/main/runtime/relay/relay-renewal-jitter.ts diff --git a/src/main/orca-profiles/profile-cloud-client.ts b/src/main/orca-profiles/profile-cloud-client.ts index e7657bbdb87..5893c8d109a 100644 --- a/src/main/orca-profiles/profile-cloud-client.ts +++ b/src/main/orca-profiles/profile-cloud-client.ts @@ -159,19 +159,37 @@ function normalizeSessionResponse(value: unknown): OrcaCloudSessionExchangeRespo const CLOUD_REQUEST_TIMEOUT_MS = 30_000 -async function postJson(url: string, body: unknown, accessToken?: string): Promise { +// Why: refresh tokens rotate, so an aborted refresh is ambiguous — the server +// may have rotated ours before the reply was lost, and the only recovery is a +// replay the server reads as reuse. One long attempt beats a short attempt plus +// a replayed retry. +const CLOUD_REFRESH_TIMEOUT_MS = 60_000 + +type PostJsonOptions = { + accessToken?: string + timeoutMs?: number +} + +// Only a status line proves the server rejected the request without consuming +// what was in it. Everything else — an abort, a dropped socket, a 200 we could +// not parse — leaves a rotating credential possibly already spent. +export function isAmbiguousCloudRequestFailure(error: unknown): boolean { + return !(error instanceof OrcaCloudRequestError) +} + +async function postJson(url: string, body: unknown, options?: PostJsonOptions): Promise { const response = await fetch(url, { method: 'POST', headers: { 'content-type': 'application/json', - ...(accessToken ? { authorization: `Bearer ${accessToken}` } : {}) + ...(options?.accessToken ? { authorization: `Bearer ${options.accessToken}` } : {}) }, body: JSON.stringify(body), // Why: these are fixed first-party token endpoints; following a redirect // would re-send refresh tokens/code verifiers to another origin, and a // stalled server must not hang the renderer's awaited IPC call forever. redirect: 'error', - signal: AbortSignal.timeout(CLOUD_REQUEST_TIMEOUT_MS) + signal: AbortSignal.timeout(options?.timeoutMs ?? CLOUD_REQUEST_TIMEOUT_MS) }) if (!response.ok) { await cancelUnreadResponseBody(response) @@ -204,7 +222,7 @@ export async function refreshOrcaCloudCapabilities( cloud?: unknown organizations?: unknown capabilities: unknown - }>(config.capabilitiesEndpoint, {}, session.accessToken) + }>(config.capabilitiesEndpoint, {}, { accessToken: session.accessToken }) return { cloud: response.cloud === undefined ? undefined : normalizeCloudSummary(response.cloud), organizations: normalizeOrganizations(response.organizations), @@ -217,9 +235,11 @@ export async function refreshOrcaCloudSession( session: OrcaCloudSession ): Promise { return normalizeSessionResponse( - await postJson(config.refreshEndpoint, { - refreshToken: session.refreshToken - }) + await postJson( + config.refreshEndpoint, + { refreshToken: session.refreshToken }, + { timeoutMs: CLOUD_REFRESH_TIMEOUT_MS } + ) ) } @@ -235,7 +255,7 @@ export async function createOrcaCloudProfile( orgId: args.orgId, name: args.name }, - session.accessToken + { accessToken: session.accessToken } ) ) } @@ -249,7 +269,7 @@ export async function selectOrcaCloudOrg( cloud: unknown organizations?: unknown capabilities: unknown - }>(config.orgEndpoint, { orgId }, session.accessToken) + }>(config.orgEndpoint, { orgId }, { accessToken: session.accessToken }) return { cloud: normalizeCloudSummary(response.cloud), organizations: normalizeOrganizations(response.organizations), @@ -261,5 +281,9 @@ export async function revokeOrcaCloudSession( config: OrcaCloudAuthConfig, session: OrcaCloudSession ): Promise { - await postJson(config.logoutEndpoint, { refreshToken: session.refreshToken }, session.accessToken) + await postJson( + config.logoutEndpoint, + { refreshToken: session.refreshToken }, + { accessToken: session.accessToken } + ) } diff --git a/src/main/orca-profiles/profile-cloud-refresh-replay-guard.ts b/src/main/orca-profiles/profile-cloud-refresh-replay-guard.ts new file mode 100644 index 00000000000..d5c22f6b527 --- /dev/null +++ b/src/main/orca-profiles/profile-cloud-refresh-replay-guard.ts @@ -0,0 +1,55 @@ +// Refresh tokens whose server-side fate is unknown: the POST left the client but +// no status came back, so the server may already have rotated the token before +// the reply was lost. Sending it again reads as reuse and revokes the whole +// token family — on 2026-09-04 that turned one slow refresh endpoint into 21,605 +// sign-outs, because every caller's retry loop replayed the same stored token. + +// A replay this soon after the ambiguous attempt is a retry loop, not a person +// asking again; holding it back keeps one lost reply from becoming a storm. +export const AMBIGUOUS_REFRESH_REPLAY_DELAY_MS = 30_000 + +type AmbiguousRefreshAttempt = { + refreshToken: string + attemptedAt: number +} + +const ambiguousRefreshAttempts = new Map() + +export class AmbiguousRefreshReplayBlockedError extends Error { + constructor() { + super('orca_cloud_refresh_replay_blocked') + this.name = 'AmbiguousRefreshReplayBlockedError' + } +} + +export function recordAmbiguousRefreshAttempt( + key: string, + refreshToken: string, + now = Date.now() +): void { + ambiguousRefreshAttempts.set(key, { refreshToken, attemptedAt: now }) +} + +// Call once the token's fate is known: it rotated, or the session it belonged to +// is gone. Leaving the record would mislabel a later, unrelated 401. +export function forgetAmbiguousRefreshAttempt(key: string): void { + ambiguousRefreshAttempts.delete(key) +} + +export function wasRefreshTokenAmbiguouslyAttempted(key: string, refreshToken: string): boolean { + return ambiguousRefreshAttempts.get(key)?.refreshToken === refreshToken +} + +export function blocksAmbiguousRefreshReplay( + key: string, + refreshToken: string, + now = Date.now() +): boolean { + const attempt = ambiguousRefreshAttempts.get(key) + if (!attempt || attempt.refreshToken !== refreshToken) { + return false + } + // Why bounded rather than permanent: the token is only *possibly* spent. A + // permanent block would sign out every desktop whose refresh merely timed out. + return now - attempt.attemptedAt < AMBIGUOUS_REFRESH_REPLAY_DELAY_MS +} diff --git a/src/main/orca-profiles/profile-cloud-service-auth-retry.test.ts b/src/main/orca-profiles/profile-cloud-service-auth-retry.test.ts index 9db1d830986..53b26a75595 100644 --- a/src/main/orca-profiles/profile-cloud-service-auth-retry.test.ts +++ b/src/main/orca-profiles/profile-cloud-service-auth-retry.test.ts @@ -53,6 +53,7 @@ vi.mock('./profile-cloud-pkce', () => ({ vi.mock('./profile-cloud-client', () => ({ OrcaCloudRequestError: OrcaCloudRequestErrorMock, + isAmbiguousCloudRequestFailure: (error: unknown) => !(error instanceof OrcaCloudRequestErrorMock), createOrcaCloudProfile: createOrcaCloudProfileMock, exchangeOrcaCloudAuthCode: exchangeOrcaCloudAuthCodeMock, refreshOrcaCloudCapabilities: refreshOrcaCloudCapabilitiesMock, diff --git a/src/main/orca-profiles/profile-cloud-service-refresh.test.ts b/src/main/orca-profiles/profile-cloud-service-refresh.test.ts index 075573eea3a..877c74c3c46 100644 --- a/src/main/orca-profiles/profile-cloud-service-refresh.test.ts +++ b/src/main/orca-profiles/profile-cloud-service-refresh.test.ts @@ -51,6 +51,7 @@ vi.mock('./profile-cloud-pkce', () => ({ vi.mock('./profile-cloud-client', () => ({ OrcaCloudRequestError: OrcaCloudRequestErrorMock, + isAmbiguousCloudRequestFailure: (error: unknown) => !(error instanceof OrcaCloudRequestErrorMock), createOrcaCloudProfile: createOrcaCloudProfileMock, exchangeOrcaCloudAuthCode: exchangeOrcaCloudAuthCodeMock, refreshOrcaCloudCapabilities: refreshOrcaCloudCapabilitiesMock, diff --git a/src/main/orca-profiles/profile-cloud-session-refresh.test.ts b/src/main/orca-profiles/profile-cloud-session-refresh.test.ts index 3b869beb6de..2aa3dc0266e 100644 --- a/src/main/orca-profiles/profile-cloud-session-refresh.test.ts +++ b/src/main/orca-profiles/profile-cloud-session-refresh.test.ts @@ -38,6 +38,7 @@ vi.mock('./profile-cloud-index', () => ({ linkOrcaProfileToCloud: linkMock })) import { readFreshOrcaCloudSession } from './profile-cloud-session-refresh' import { OrcaCloudRequestError } from './profile-cloud-client' import { onOrcaCloudSessionInvalidated } from './profile-cloud-session-invalidation' +import { forgetAmbiguousRefreshAttempt } from './profile-cloud-refresh-replay-guard' const config = {} as OrcaCloudAuthConfig const active = { @@ -61,6 +62,8 @@ const staleSession = { describe('profile cloud session refresh', () => { beforeEach(() => { vi.clearAllMocks() + vi.restoreAllMocks() + forgetAmbiguousRefreshAttempt('/data\0profile-1') saveIfCurrentMock.mockReturnValue('memory-only') readMock.mockReturnValue({ status: 'found', session: staleSession, persistence: 'memory-only' }) }) @@ -156,3 +159,122 @@ describe('profile cloud session refresh', () => { unsubscribe() }) }) + +describe('refresh-token replay after an ambiguous attempt', () => { + const timeout = (): Error => + Object.assign(new Error('The operation timed out.'), { name: 'TimeoutError' }) + const rotatedResponse = { + accessToken: 'new-access', + refreshToken: 'new-refresh', + expiresAt: 4_000_000, + organizations: [], + capabilities: { flags: { 'relay.use': true }, refreshedAt: 2 }, + cloud: { userId: 'user-1', cloudProfileId: 'cloud-profile-1', activeOrgId: 'org-1' } + } + + beforeEach(() => { + vi.clearAllMocks() + vi.restoreAllMocks() + forgetAmbiguousRefreshAttempt('/data\0profile-1') + saveIfCurrentMock.mockReturnValue('memory-only') + readMock.mockReturnValue({ status: 'found', session: staleSession, persistence: 'memory-only' }) + }) + + it('never resends a refresh token whose attempt timed out', async () => { + refreshMock.mockRejectedValue(timeout()) + + await expect(readFreshOrcaCloudSession(config, active, '/data')).rejects.toThrow( + 'The operation timed out.' + ) + expect(refreshMock).toHaveBeenCalledTimes(1) + + // The retry loop above this module re-enters immediately; it must not turn + // one lost reply into a second POST of the same token. + await expect(readFreshOrcaCloudSession(config, active, '/data')).rejects.toThrow( + 'orca_cloud_refresh_replay_blocked' + ) + expect(refreshMock).toHaveBeenCalledTimes(1) + expect(clearMock).not.toHaveBeenCalled() + }) + + it('adopts the stored session when a timed-out attempt was rotated elsewhere', async () => { + const rotated = { ...staleSession, refreshToken: 'rotated-refresh', expiresAt: 4_000_000 } + refreshMock.mockRejectedValue(timeout()) + readMock + .mockReturnValueOnce({ status: 'found', session: staleSession, persistence: 'memory-only' }) + .mockReturnValueOnce({ status: 'found', session: staleSession, persistence: 'memory-only' }) + .mockReturnValue({ status: 'found', session: rotated, persistence: 'memory-only' }) + + await expect(readFreshOrcaCloudSession(config, active, '/data')).resolves.toEqual({ + status: 'found', + session: rotated + }) + expect(refreshMock).toHaveBeenCalledTimes(1) + expect(saveIfCurrentMock).not.toHaveBeenCalled() + }) + + it('retries once after a definitive 5xx, which cannot have rotated the token', async () => { + refreshMock + .mockRejectedValueOnce(new OrcaCloudRequestError(503)) + .mockResolvedValueOnce(rotatedResponse) + + const result = await readFreshOrcaCloudSession(config, active, '/data') + + expect(refreshMock).toHaveBeenCalledTimes(2) + expect(refreshMock).toHaveBeenNthCalledWith(2, config, staleSession) + expect(result).toEqual({ + status: 'found', + session: expect.objectContaining({ + accessToken: 'new-access', + refreshToken: 'new-refresh' + }) + }) + }) + + it('gives a definitive 5xx exactly one retry', async () => { + refreshMock.mockRejectedValue(new OrcaCloudRequestError(503)) + + await expect(readFreshOrcaCloudSession(config, active, '/data')).rejects.toThrow( + 'orca_cloud_request_failed_503' + ) + expect(refreshMock).toHaveBeenCalledTimes(2) + expect(clearMock).not.toHaveBeenCalled() + }) + + it('marks a 401 that follows an ambiguous attempt as a possible self-replay', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const now = vi.spyOn(Date, 'now').mockReturnValue(1_000_000) + const invalidated = vi.fn() + const unsubscribe = onOrcaCloudSessionInvalidated(invalidated) + refreshMock.mockRejectedValueOnce(timeout()) + + await expect(readFreshOrcaCloudSession(config, active, '/data')).rejects.toThrow( + 'The operation timed out.' + ) + + now.mockReturnValue(1_000_000 + 31_000) + refreshMock.mockRejectedValueOnce(new OrcaCloudRequestError(401)) + + await expect(readFreshOrcaCloudSession(config, active, '/data')).resolves.toEqual({ + status: 'reconnect-required' + }) + + expect(refreshMock).toHaveBeenCalledTimes(2) + expect(warn.mock.calls.flat().join(' ')).toContain('orca_cloud_refresh_possible_replay') + expect(clearMock).toHaveBeenCalledTimes(1) + expect(invalidated).toHaveBeenCalledTimes(1) + unsubscribe() + }) + + it('does not mark a 401 that follows no ambiguous attempt', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + refreshMock.mockRejectedValue(new OrcaCloudRequestError(401)) + + await expect(readFreshOrcaCloudSession(config, active, '/data')).resolves.toEqual({ + status: 'reconnect-required' + }) + + expect(warn.mock.calls.flat().join(' ')).not.toContain('orca_cloud_refresh_possible_replay') + expect(clearMock).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/orca-profiles/profile-cloud-session-refresh.ts b/src/main/orca-profiles/profile-cloud-session-refresh.ts index 32e2e39f6e0..221b4908bee 100644 --- a/src/main/orca-profiles/profile-cloud-session-refresh.ts +++ b/src/main/orca-profiles/profile-cloud-session-refresh.ts @@ -6,8 +6,20 @@ import { readOrcaCloudSession, saveOrcaCloudSessionIfCurrent } from './profile-cloud-session-store' -import { OrcaCloudRequestError, refreshOrcaCloudSession } from './profile-cloud-client' +import { + isAmbiguousCloudRequestFailure, + OrcaCloudRequestError, + refreshOrcaCloudSession +} from './profile-cloud-client' import { linkOrcaProfileToCloud } from './profile-cloud-index' +import type { OrcaCloudSessionExchangeResponse } from './profile-cloud-session-exchange' +import { + AmbiguousRefreshReplayBlockedError, + blocksAmbiguousRefreshReplay, + forgetAmbiguousRefreshAttempt, + recordAmbiguousRefreshAttempt, + wasRefreshTokenAmbiguouslyAttempted +} from './profile-cloud-refresh-replay-guard' import { captureCloudSessionMutation, cloudSessionIdentity, @@ -67,11 +79,78 @@ function clearCloudSessionIfUnchanged( ) } clearOrcaCloudSession(profileId, userDataPath) + forgetAmbiguousRefreshAttempt(cloudSessionRefreshKey(profileId, userDataPath)) // Why: the renderer cached auth status at startup; without this it keeps // showing "Connected" until the app restarts. emitOrcaCloudSessionInvalidated() } +// Why: support cannot otherwise tell a genuine revocation from a sign-out we +// caused ourselves by resending a refresh token whose first attempt never +// answered. Never log the token itself. +function warnIfPossibleRefreshReplay( + profileId: string, + userDataPath: string, + failed: OrcaCloudSession, + error: unknown +): void { + if (!(error instanceof OrcaCloudRequestError) || error.statusCode !== 401) { + return + } + const key = cloudSessionRefreshKey(profileId, userDataPath) + if (!wasRefreshTokenAmbiguouslyAttempted(key, failed.refreshToken)) { + return + } + console.warn( + '[orca-cloud] orca_cloud_refresh_possible_replay: refresh rejected 401 for a token whose earlier attempt never answered' + ) +} + +type CloudSessionRefreshAttempt = + | { status: 'refreshed'; response: OrcaCloudSessionExchangeResponse } + | { status: 'rotated-elsewhere'; session: OrcaCloudSession } + +function isRetryableCloudRefreshRejection(error: unknown): boolean { + return error instanceof OrcaCloudRequestError && error.statusCode >= 500 +} + +async function attemptCloudSessionRefresh( + key: string, + config: OrcaCloudAuthConfig, + active: ActiveOrcaProfileState, + userDataPath: string, + session: OrcaCloudSession +): Promise { + for (let attempt = 0; ; attempt++) { + try { + const response = await refreshOrcaCloudSession(config, session) + forgetAmbiguousRefreshAttempt(key) + return { status: 'refreshed', response } + } catch (error) { + const ambiguous = isAmbiguousCloudRequestFailure(error) + if (ambiguous) { + recordAmbiguousRefreshAttempt(key, session.refreshToken) + } + // Only a status line proves the server rejected this token without + // rotating it, so a definitive 5xx is the only failure worth retrying. + const retryable = !ambiguous && attempt === 0 && isRetryableCloudRefreshRejection(error) + if (!ambiguous && !retryable) { + throw error + } + // Another caller may have rotated the stored session while this attempt + // was in flight; that result is the one to use, and the token this attempt + // held is no longer ours to send again. + const current = readOrcaCloudSession(active.profile.id, userDataPath) + if (current.status === 'found' && current.session.refreshToken !== session.refreshToken) { + return { status: 'rotated-elsewhere', session: current.session } + } + if (ambiguous) { + throw error + } + } + } +} + async function refreshStoredCloudSession( config: OrcaCloudAuthConfig, active: ActiveOrcaProfileState, @@ -95,9 +174,16 @@ async function refreshStoredCloudSession( if (!active.profile.cloud) { throw new StaleCloudSessionMutationError() } + if (blocksAmbiguousRefreshReplay(key, session.refreshToken)) { + throw new AmbiguousRefreshReplayBlockedError() + } const expectedIdentity = cloudSessionIdentity(active.profile.id, active.profile.cloud) const snapshot = captureCloudSessionMutation(expectedIdentity, userDataPath) - const refreshed = await refreshOrcaCloudSession(config, session) + const attempt = await attemptCloudSessionRefresh(key, config, active, userDataPath, session) + if (attempt.status === 'rotated-elsewhere') { + return attempt.session + } + const refreshed = attempt.response const refreshedIdentity = cloudSessionIdentity(active.profile.id, refreshed.cloud) if ( refreshedIdentity.cloudUserId !== expectedIdentity.cloudUserId || @@ -148,6 +234,7 @@ export async function readFreshOrcaCloudSession( } } catch (error) { if (isOrcaCloudAuthFailure(error)) { + warnIfPossibleRefreshReplay(active.profile.id, userDataPath, session.session, error) clearCloudSessionIfUnchanged(active.profile.id, userDataPath, session.session, active) return { status: 'reconnect-required' } } @@ -168,6 +255,7 @@ export async function forceRefreshOrcaCloudSession( } } catch (error) { if (isOrcaCloudAuthFailure(error)) { + warnIfPossibleRefreshReplay(active.profile.id, userDataPath, session, error) clearCloudSessionIfUnchanged(active.profile.id, userDataPath, session, active) return { status: 'reconnect-required' } } diff --git a/src/main/runtime/relay/relay-origin-pool.ts b/src/main/runtime/relay/relay-origin-pool.ts index ca29c41e8a7..acd2f90292e 100644 --- a/src/main/runtime/relay/relay-origin-pool.ts +++ b/src/main/runtime/relay/relay-origin-pool.ts @@ -7,6 +7,7 @@ import type { RelayDrainMessage } from './relay-control-protocol' import type { RelayHostCloseReason } from '../../../shared/relay-host-close-reason' import { RelayDrainRetrySchedule } from './relay-drain-retry-schedule' import { RelayHttpError, requestRelayAssignment, type RelayAssignment } from './relay-http-client' +import { relayRenewalDelayMs } from './relay-renewal-jitter' import type { RelayBrokerStatus, RelayIdentity } from './relay-session-broker-contract' import type { RelayRegion } from './relay-region-preference' @@ -245,8 +246,7 @@ export class RelayOriginPool { } const now = (this.options.now ?? Date.now)() const random = this.options.random ?? Math.random - const earlyMs = 60_000 + Math.floor(random() * 60_001) - const delay = Math.max(0, origin.controlLeaseExpiresAt - earlyMs - now) + const delay = relayRenewalDelayMs(origin.controlLeaseExpiresAt, now, random) this.rotationTimer = setTimeout(() => void this.rebindActiveControl(origin), delay) } diff --git a/src/main/runtime/relay/relay-renewal-jitter.test.ts b/src/main/runtime/relay/relay-renewal-jitter.test.ts new file mode 100644 index 00000000000..5bbc0994836 --- /dev/null +++ b/src/main/runtime/relay/relay-renewal-jitter.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from 'vitest' +import { + RELAY_RENEWAL_JITTER_RATIO, + RELAY_RENEWAL_SAFETY_MARGIN_MS, + relayRenewalDelayMs +} from './relay-renewal-jitter' + +const LEASE_MS = 55 * 60_000 +const latest = LEASE_MS - RELAY_RENEWAL_SAFETY_MARGIN_MS +const base = latest / (1 + RELAY_RENEWAL_JITTER_RATIO) + +describe('relay renewal jitter', () => { + it('keeps every sample inside the jitter band and before the safety margin', () => { + let seed = 1 + const random = (): number => { + seed = (seed * 1103515245 + 12345) % 2147483648 + return seed / 2147483648 + } + const samples: number[] = [] + for (let i = 0; i < 20_000; i++) { + samples.push(relayRenewalDelayMs(LEASE_MS, 0, random)) + } + for (const sample of samples) { + expect(sample).toBeGreaterThanOrEqual(Math.floor(base * (1 - RELAY_RENEWAL_JITTER_RATIO))) + expect(sample).toBeLessThanOrEqual(latest) + // The renewal never lands inside the margin, so it never races expiry. + expect(LEASE_MS - sample).toBeGreaterThanOrEqual(RELAY_RENEWAL_SAFETY_MARGIN_MS) + } + const mean = samples.reduce((total, sample) => total + sample, 0) / samples.length + expect(Math.abs(mean - base) / base).toBeLessThan(0.005) + }) + + it('spreads a same-second cohort over minutes instead of one second', () => { + const delays = Array.from({ length: 1000 }, (_, index) => + relayRenewalDelayMs(LEASE_MS, 0, () => index / 999) + ) + const spread = Math.max(...delays) - Math.min(...delays) + expect(spread).toBeGreaterThan(9 * 60_000) + }) + + it('pins the band ends to the base interval', () => { + expect(relayRenewalDelayMs(LEASE_MS, 0, () => 0)).toBe( + Math.floor(base * (1 - RELAY_RENEWAL_JITTER_RATIO)) + ) + expect(relayRenewalDelayMs(LEASE_MS, 0, () => 0.5)).toBe(Math.floor(base)) + expect(relayRenewalDelayMs(LEASE_MS, 0, () => 1)).toBe(latest) + }) + + it('renews immediately once the lease is inside the safety margin', () => { + expect(relayRenewalDelayMs(RELAY_RENEWAL_SAFETY_MARGIN_MS, 0, () => 1)).toBe(0) + expect(relayRenewalDelayMs(0, 60_000, () => 1)).toBe(0) + }) + + it('measures the delay from now, not from the epoch', () => { + expect(relayRenewalDelayMs(LEASE_MS + 1_000_000, 1_000_000, () => 0.5)).toBe(Math.floor(base)) + }) +}) diff --git a/src/main/runtime/relay/relay-renewal-jitter.ts b/src/main/runtime/relay/relay-renewal-jitter.ts new file mode 100644 index 00000000000..8b1f1ec6412 --- /dev/null +++ b/src/main/runtime/relay/relay-renewal-jitter.ts @@ -0,0 +1,25 @@ +// Why: a cell recreate reconnects a whole cohort inside one second. Every host +// in it then took its lease from the same second and, with only a 60s-wide +// spread, renewed inside the same second ~54 minutes later — a self-sustaining +// fleet-wide reconnect burst. Full +/-10% jitter spreads that cohort over +// minutes instead. +export const RELAY_RENEWAL_JITTER_RATIO = 0.1 + +// The latest jittered renewal still lands this far before expiry. +export const RELAY_RENEWAL_SAFETY_MARGIN_MS = 90_000 + +// Why: the relay accepts a rebind at any point in the lease and resets the full +// TTL from it (cloud/apps/relay/src/host-session-registry.ts:736-743), so +// renewing early is free; only renewing late is fatal (:997 drains an expired +// lease). That asymmetry is why the base is shrunk to fit the upward jitter +// rather than the jittered value being clipped at the margin. +export function relayRenewalDelayMs(expiresAt: number, now: number, random: () => number): number { + const remaining = expiresAt - now + const latest = remaining - RELAY_RENEWAL_SAFETY_MARGIN_MS + if (latest <= 0) { + return 0 + } + const base = latest / (1 + RELAY_RENEWAL_JITTER_RATIO) + const jittered = base * (1 + (random() * 2 - 1) * RELAY_RENEWAL_JITTER_RATIO) + return Math.max(0, Math.min(Math.floor(jittered), latest)) +} diff --git a/src/main/runtime/relay/relay-session-broker.ts b/src/main/runtime/relay/relay-session-broker.ts index d8d0ec45047..cd83545e9ca 100644 --- a/src/main/runtime/relay/relay-session-broker.ts +++ b/src/main/runtime/relay/relay-session-broker.ts @@ -16,6 +16,7 @@ import { type RelayAssignment } from './relay-http-client' import { RelayOriginPool } from './relay-origin-pool' +import { relayRenewalDelayMs } from './relay-renewal-jitter' import type { RelayBrokerStatus, RelaySessionBrokerOptions } from './relay-session-broker-contract' export type { RelayBrokerStatus } from './relay-session-broker-contract' @@ -243,8 +244,7 @@ export class RelaySessionBroker { } const now = (this.options.now ?? Date.now)() const random = this.options.random ?? Math.random - const earlyMs = 60_000 + Math.floor(random() * 60_001) - const delay = Math.max(0, authorization.expiresAt - earlyMs - now) + const delay = relayRenewalDelayMs(authorization.expiresAt, now, random) this.refreshTimer = setTimeout(() => void this.refreshAuthorization(), delay) }