diff --git a/src/main/runtime/relay/relay-auth-coordinator-wait-budget.test.ts b/src/main/runtime/relay/relay-auth-coordinator-wait-budget.test.ts new file mode 100644 index 00000000000..e46431f42f6 --- /dev/null +++ b/src/main/runtime/relay/relay-auth-coordinator-wait-budget.test.ts @@ -0,0 +1,57 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { RelayAuthCoordinator, type RelayAuthContext } from './relay-auth-coordinator' + +const context: RelayAuthContext = { + identity: { userId: 'user-1', profileId: 'profile-1', organizationId: 'org-1' }, + accessToken: 'access-1', + relayEntitled: true +} + +/** + * What the live-broker wait budget does and does not bound. + * + * `LIVE_BROKER_WAIT_BUDGET_MS` is 20s and the deadline is checked only AFTER `await pending`, which + * is deliberately unbounded — cutting a slow-but-succeeding open short would fail a pairing that + * was about to work. So the budget bounds the armed-retry chain and nothing else. + * + * That is worth a test rather than a comment because the consequence lands on the phone: + * `pairing.provisionRelay` reaches this through `requireActiveBroker`, and `readContext`'s own + * ceiling is the cloud refresh timeout (60s, with one retry for a definitive 5xx) — several times + * the phone's request budget. The phone gives up and retries while the desktop is still holding a + * transient demand ref for the call it abandoned. + */ +describe('live-broker wait budget', () => { + afterEach(() => { + vi.useRealTimers() + }) + + it('does not bound a reconcile already in flight, so it can outlast the phone request budget', async () => { + vi.useFakeTimers() + let releaseContext = (): void => {} + const coordinator = new RelayAuthCoordinator({ + readContext: () => + new Promise((resolve) => { + releaseContext = (): void => resolve(context) + }), + openBroker: async () => ({ closeNow: vi.fn() }), + onStatus: vi.fn() + }) + coordinator.reconcile() + await Promise.resolve() + + let settled = false + const wait = coordinator.waitForLiveBrokerResult().then((result) => { + settled = true + return result + }) + + // Well past both the 20s budget and the phone's 30s request budget. + await vi.advanceTimersByTimeAsync(45_000) + expect(settled).toBe(false) + + releaseContext() + await vi.advanceTimersByTimeAsync(0) + await expect(wait).resolves.toEqual({ broker: expect.anything() }) + expect(settled).toBe(true) + }) +}) diff --git a/src/main/runtime/relay/relay-auth-coordinator.ts b/src/main/runtime/relay/relay-auth-coordinator.ts index a284618ac72..c5f82395af4 100644 --- a/src/main/runtime/relay/relay-auth-coordinator.ts +++ b/src/main/runtime/relay/relay-auth-coordinator.ts @@ -31,10 +31,18 @@ type BrokerOwnership = { } export class RelayAuthCoordinator { - // Why 20s: bounds only how long a waiter sits through armed retries, never - // an open already in flight. It spans the first few rungs of the backoff - // ladder and stays inside the phone's 30s request budget, so a sustained - // outage fails the caller with its cause instead of parking the demand ref. + // Why 20s: bounds only how long a waiter sits through armed retries, never a reconcile already + // in flight. It spans the first few rungs of the backoff ladder, so a sustained outage fails the + // caller with its cause instead of parking the demand ref. + // + // It does NOT bound the call. The deadline is only consulted after `await pending` below, and a + // reconcile's own ceiling is `readContext`'s cloud-refresh timeout (60s, plus one retry for a + // definitive 5xx) followed by the broker open — several times the phone's 30s request budget, + // which an earlier version of this comment claimed it stayed inside. A phone whose + // pairing.provisionRelay reaches this through requireActiveBroker can therefore give up and + // retry while the desktop still holds a transient demand ref for the call it abandoned. + // Bounding the reconcile instead would fail a slow-but-succeeding open, which is the worse trade; + // relay-auth-coordinator-wait-budget.test.ts pins the behaviour so the claim cannot drift back. private static readonly LIVE_BROKER_WAIT_BUDGET_MS = 20_000 private readonly options: RelayAuthCoordinatorOptions private authEpoch = 0 @@ -145,9 +153,11 @@ export class RelayAuthCoordinator { return { broker } } const pending = this.latestReconcile - // Why unbounded: a reconcile always settles (opens carry HTTP deadlines), - // and cutting a slow-but-succeeding open short would fail a pairing that - // was about to work. The budget bounds only the retry chain below. + // Why unbounded: a reconcile always settles — BOTH its awaits carry a deadline, the broker + // open and `readContext`'s cloud refresh (the larger of the two, and the one the original + // parenthetical here left out) — and cutting a slow-but-succeeding open short would fail a + // pairing that was about to work. "Settles" is not "settles soon": see the ceiling noted on + // LIVE_BROKER_WAIT_BUDGET_MS. The budget bounds only the retry chain below. await pending if (pending !== this.latestReconcile) { continue