fix(relay): the demand wake signal must not fail the grant it wakes for

`withTransientDemand` called `refreshDemand()` bare on both sides of the
operation. It reaches the device registry (`nextPendingExpiry` -> `listDevices`)
and the settings store (`hasDemand` -> `isRelayAllowedForDevice`), so it can
fail on its own — and measured, each call site failed the operation instead:

- the pre-call threw with the transient ref already acquired, so the ref was
  never released and the operation never ran. Transient refs have no expiry, so
  that ref holds relay demand for the rest of the process.
- the teardown call replaced the operation's own result in both directions: a
  named mint failure arrived as `listDevices exploded`, and a SUCCESSFUL mint
  arrived as a rejection.

Both are wake signals; the liveness tick and the next refresh re-ask, so a lost
signal is recoverable where a stranded ref is not. The containment lives beside
the ledger that owns the ref, because desktop-relay-service.ts is at its
max-lines ceiling and this must not cost it a line.

Also carries the original error as the `cause` of the mid-operation policy flip
rewrite, which was discarding it — the one place in a commit about naming causes
that destroyed one.
This commit is contained in:
Neil
2026-09-10 17:56:17 -07:00
parent 1bb7489a92
commit 25096b39d8
3 changed files with 136 additions and 4 deletions
@@ -0,0 +1,113 @@
import { describe, expect, it, vi } from 'vitest'
import { DesktopRelayService } from './desktop-relay-service'
import type { MobilePairingConnectionMode } from '../../../shared/mobile-pairing-connection-mode'
/**
* `withTransientDemand`'s teardown runs work that can fail on its own.
*
* `refreshDemand` reaches the device registry (`nextPendingExpiry` -> `listDevices`) and the
* settings store (`hasDemand` -> `isRelayAllowedForDevice`). Running it bare in the `finally` let
* its failure replace the operation's result in BOTH directions: a named mint failure arrived as
* an unrelated message, and a successful mint arrived as a rejection. Before the operation it was
* worse still: the throw landed after the transient ref was acquired, so the ref was never
* released, and transient refs have no expiry.
*/
type TransientDemandHost = {
withTransientDemand: (
kind: string,
deviceId: string,
operation: () => Promise<unknown>
) => Promise<unknown>
}
function serviceWithTeardown(options: {
release?: () => void
refreshDemand?: () => void
}): (operation: () => Promise<unknown>) => Promise<unknown> {
const service = Object.create(DesktopRelayService.prototype) as DesktopRelayService
Object.assign(service, {
hostMobilePairingConnectionMode: () => 'automatic' as MobilePairingConnectionMode,
runtimeRpc: {
getDeviceRegistry: () => ({ getMobilePairingConnectionMode: () => 'automatic' })
},
demandLedger: { acquireTransient: () => options.release ?? ((): void => {}) },
refreshDemand: options.refreshDemand ?? ((): void => {})
})
const host = service as unknown as TransientDemandHost
return (operation) => host.withTransientDemand.call(service, 'pairing', 'device-1', operation)
}
describe('DesktopRelayService transient-demand teardown', () => {
it('keeps the operation failure when the demand refresh throws', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
const run = serviceWithTeardown({
refreshDemand: () => {
throw new Error('listDevices exploded')
}
})
await expect(
run(async () => {
throw new Error('relay_broker_rejected')
})
).rejects.toThrow('relay_broker_rejected')
expect(warn).toHaveBeenCalled()
warn.mockRestore()
})
it('does not strand the demand ref when the pre-operation refresh throws', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
const release = vi.fn()
const operation = vi.fn(async () => 'minted')
const run = serviceWithTeardown({
release,
refreshDemand: () => {
throw new Error('listDevices exploded')
}
})
// The ref has no expiry, so failing the wake signal must not take the release with it.
await expect(run(operation)).resolves.toBe('minted')
expect(operation).toHaveBeenCalledTimes(1)
expect(release).toHaveBeenCalledTimes(1)
warn.mockRestore()
})
it('keeps a successful mint successful when the demand refresh throws', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
const run = serviceWithTeardown({
refreshDemand: () => {
throw new Error('listDevices exploded')
}
})
await expect(run(async () => 'minted')).resolves.toBe('minted')
warn.mockRestore()
})
it('carries the original failure as the cause of a mid-operation policy flip', async () => {
const mode = { current: 'automatic' as MobilePairingConnectionMode }
const service = Object.create(DesktopRelayService.prototype) as DesktopRelayService
Object.assign(service, {
hostMobilePairingConnectionMode: () => mode.current,
runtimeRpc: {
getDeviceRegistry: () => ({ getMobilePairingConnectionMode: () => 'automatic' })
},
demandLedger: { acquireTransient: () => (): void => {} },
refreshDemand: () => {}
})
const host = service as unknown as TransientDemandHost
const rejection = await host.withTransientDemand
.call(service, 'pairing', 'device-1', async () => {
mode.current = 'local-only'
throw new Error('relay_token_exchange_failed_503')
})
.catch((error: unknown) => error)
expect((rejection as Error).message).toBe('relay_disabled_for_device')
expect(((rejection as Error).cause as Error | undefined)?.message).toBe(
'relay_token_exchange_failed_503'
)
})
})
@@ -18,7 +18,7 @@ import type {
RelayRevokeOutboxItem
} from './relay-revoke-outbox'
import { deriveRelayHostId } from './relay-http-client'
import { RelayDemandLedger } from './relay-demand-ledger'
import { RelayDemandLedger, refreshRelayDemandBestEffort } from './relay-demand-ledger'
import { createRelayRegionPreferenceReader } from './relay-region-preference'
import { pairingAuthorizationForContext } from './relay-pairing-authorization'
import { buildPairingEndpointsResult } from './relay-pairing-endpoints-result'
@@ -292,7 +292,7 @@ export class DesktopRelayService {
throw new Error('relay_disabled_for_device')
}
const release = this.demandLedger.acquireTransient(`${kind}:${deviceId}`, deviceId)
this.refreshDemand()
refreshRelayDemandBestEffort(() => this.refreshDemand())
try {
return await operation()
} catch (error) {
@@ -301,12 +301,12 @@ export class DesktopRelayService {
// reaches `standby` and clears the offline reason. The wait then ends with no cause at all
// — the generic `relay_control_not_active` — when the flip is exactly the cause.
if (!this.isRelayAllowedForDevice(deviceId)) {
throw new Error('relay_disabled_for_device')
throw new Error('relay_disabled_for_device', { cause: error })
}
throw error
} finally {
release()
this.refreshDemand()
refreshRelayDemandBestEffort(() => this.refreshDemand())
}
}
@@ -13,6 +13,25 @@ type RelayDemandLedgerOptions = {
type TransientRef = { deviceId: string; count: number }
/**
* Run a demand refresh as the wake signal it is.
*
* A refresh reaches the device registry (`nextPendingExpiry` -> `listDevices`) and the settings
* store (`hasDemand` -> the host pairing mode), so it can fail on its own. Bare, it failed the
* caller it was waking for: before an operation it threw with a transient ref already acquired —
* and those have no expiry, so the ref held relay demand for the rest of the process — and after
* one it replaced the operation's own result, turning a named mint failure into an unrelated
* message and a successful mint into a rejection. A lost wake signal is recoverable; the liveness
* tick and the next refresh both re-ask.
*/
export function refreshRelayDemandBestEffort(refresh: () => void): void {
try {
refresh()
} catch (error) {
console.warn('[relay] demand refresh failed:', error)
}
}
export class RelayDemandLedger {
private readonly options: RelayDemandLedgerOptions
private readonly transientRefs = new Map<string, TransientRef>()