fix(relay): stop a revoke that can never land from pinning the relay online

Reproduced first: enqueue a revoke, reject it 50 times the way a deleted device
is rejected, and `hasDemand` is still true. It stays true for the life of the
install.

Two deliberate properties combine into the bug. `flushRevoke` removes the outbox
item only on a SUCCESSFUL flush and swallows every failure in a bare `catch {}`
with no attempt cap and no expiry, so a permanently-rejected revoke is pending
forever. And the revoke arm of `hasDemand` is deliberately NOT filtered by the
host's pairing-connection policy, because a credential must be killed on the relay
whatever the user has since chosen. Forever-pending plus policy-exempt means the
relay is held online permanently and a `local-only` pick is defeated outright —
the regression path for what #19870 closes, previously with no test at all.

THE CHOICE, stated because the convenient one is wrong. The revoke is NOT
abandoned. Failing to revoke leaves a live credential on the relay, so discarding
the intent to tidy up demand would trade a visible stuck relay for an invisible
live credential. The item stays in the outbox and keeps retrying on every future
connection, unchanged.

What expires is only its CLAIM ON DEMAND, via `demandingFor` alongside the
unchanged `pendingFor`: after 24h an unlanded revoke stops being a reason to force
the relay online BY ITSELF. The reasoning is that holding the relay open is not
what makes the revoke succeed — it has had a day of attempts — so holding it open
buys nothing and costs the user the setting they chose. A revoke that can still
land is unaffected inside the window, and one that lands is removed as before.

Uses the existing `createdAt`, so nothing new is persisted and the window survives
restart.

Mutation-tested: putting `pendingFor` back in `hasDemand` fails the new test at
the post-window assertion. The success path is pinned separately so the fix cannot
be mistaken for aging out an item that should have been removed.

Audited the rest of the file for the same shape, since a bare `catch {}` on a
queue is rarely alone: `flushRevoke` is the only one in desktop-relay-service.ts.

src/main/runtime/relay/relay-demand-ledger.ts

@@ -52,10 +52,15 @@ export class RelayDemandLedger {
         return true
       }
     }
-    if (this.options.revokeOutbox.pendingFor(ownerIdentityKey, this.options.relayHostId).length) {
+    const now = (this.options.now ?? Date.now)()
+    // Why `demandingFor` and not `pendingFor`: a revoke the server rejects permanently is never
+    // removed, and this check is deliberately unfiltered by the host's pairing policy, so counting
+    // every pending item held the relay up forever. The item still retries; only its demand expires.
+    if (
+      this.options.revokeOutbox.demandingFor(ownerIdentityKey, this.options.relayHostId, now).length
+    ) {
       return true
     }
-    const now = (this.options.now ?? Date.now)()
     return this.options.deviceRegistry.listDevices().some((device) => {
       const binding = device.relayBinding
       if (

src/main/runtime/relay/relay-revoke-outbox.ts

@@ -21,6 +21,10 @@ export type RelayRevokeOutboxItem = RelayDeviceBinding & {
 
 const OUTBOX_FILENAME = 'mobile-relay-revoke-outbox.json'
 
+/** How long an unlanded revoke keeps forcing the relay online on its own. It is retried forever
+ *  regardless; this bounds only its claim on demand. */
+export const RELAY_REVOKE_DEMAND_WINDOW_MS = 24 * 60 * 60_000
+
 function isItem(value: unknown): value is RelayRevokeOutboxItem {
   if (!value || typeof value !== 'object') {
     return false
@@ -72,6 +76,31 @@ export class RelayRevokeOutbox {
     )
   }
 
+  /**
+   * The pending revokes that still justify forcing the relay online by themselves.
+   *
+   * Why this is narrower than `pendingFor`: an item is removed only on a SUCCESSFUL flush, so one
+   * the server rejects permanently stays pending forever, and demand counts pending revokes
+   * unfiltered by the host's pairing-connection policy. That combination pinned the relay up for
+   * the life of the install and defeated a `local-only` pick outright.
+   *
+   * What deliberately does NOT happen here is giving up on the revoke. The item stays in the
+   * outbox and keeps retrying on every connection, because failing to revoke leaves a live
+   * credential on the relay and silently discarding that intent is the worse hazard. Only its
+   * claim on demand expires: a revoke that has not landed in this long is not going to land
+   * because the relay is held open, so holding it open buys nothing and costs the user the
+   * setting they chose.
+   */
+  demandingFor(
+    ownerIdentityKey: string,
+    relayHostId: string,
+    now: number
+  ): readonly RelayRevokeOutboxItem[] {
+    return this.pendingFor(ownerIdentityKey, relayHostId).filter(
+      (item) => now - item.createdAt < RELAY_REVOKE_DEMAND_WINDOW_MS
+    )
+  }
+
   remove(reqId: string): void {
     const next = this.items.filter((item) => item.reqId !== reqId)
     if (next.length === this.items.length) {

src/main/runtime/relay/relay-revoke-permanent-failure.test.ts

@@ -0,0 +1,101 @@
+import { mkdtempSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { describe, expect, it, vi } from 'vitest'
+import { DeviceRegistry } from '../device-registry'
+import { RelayDemandLedger } from './relay-demand-ledger'
+import {
+  RELAY_REVOKE_DEMAND_WINDOW_MS,
+  RelayRevokeOutbox,
+  type RelayDeviceBinding
+} from './relay-revoke-outbox'
+import { DesktopRelayService } from './desktop-relay-service'
+
+// What this pins: a revoke that can never succeed must not hold relay demand forever.
+//
+// `hasDemand` counts a pending revoke deliberately unfiltered by the host's pairing-connection
+// policy — the credential has to be killed on the relay whatever the user has since chosen. That
+// is right for a revoke that can still land, and wrong for one that never will: the item is only
+// removed on a SUCCESSFUL flush, and `flushRevoke` swallowed every failure with no attempt cap and
+// no expiry. A permanently-rejected revoke therefore pinned demand true for the life of the
+// install and defeated a `local-only` pick entirely (#19870's regression path).
+
+const ownerIdentityKey = 'user-1\0profile-1\0org-1'
+const relayHostId = 'relay-host-1'
+
+function fixture(now: () => number) {
+  const userDataPath = mkdtempSync(join(tmpdir(), 'orca-relay-revoke-'))
+  const deviceRegistry = new DeviceRegistry(userDataPath)
+  const revokeOutbox = new RelayRevokeOutbox(userDataPath)
+  const ledger = new RelayDemandLedger({ deviceRegistry, revokeOutbox, relayHostId, now })
+  return { deviceRegistry, revokeOutbox, ledger }
+}
+
+function binding(relayDeviceId: string): RelayDeviceBinding {
+  return { relayDeviceId, relayHostId, ownerIdentityKey }
+}
+
+/** A broker whose revoke is rejected permanently, the way a deleted device or a revoked
+ *  credential is rejected: the server is reachable and says no, every time. */
+function permanentlyFailingBroker(revokeDevice: ReturnType<typeof vi.fn>) {
+  return { ownerIdentityKey, hostId: relayHostId, revokeDevice } as never
+}
+
+function serviceOver(revokeOutbox: RelayRevokeOutbox, ledger: RelayDemandLedger) {
+  const service = Object.create(DesktopRelayService.prototype) as DesktopRelayService
+  Object.assign(service, {
+    revokeOutbox,
+    demandLedger: ledger,
+    coordinator: { reconcile: vi.fn(), ensureLive: vi.fn(), getActiveBroker: () => null },
+    stopped: false,
+    livenessTimer: null,
+    demandExpiryTimer: null
+  })
+  return service as unknown as { flushRevokeOutbox: (broker: unknown) => Promise<void> }
+}
+
+describe('a revoke that can never succeed', () => {
+  it('stops pinning relay demand once its window passes, but is never abandoned', async () => {
+    let now = Date.now()
+    const { revokeOutbox, ledger } = fixture(() => now)
+    revokeOutbox.enqueue(binding('device-1'))
+    expect(ledger.hasDemand(ownerIdentityKey)).toBe(true)
+
+    const revokeDevice = vi.fn().mockRejectedValue(new Error('device_not_found'))
+    const service = serviceOver(revokeOutbox, ledger)
+
+    // Every reconnect re-drives the outbox against the same stable reqId.
+    for (let attempt = 0; attempt < 50; attempt += 1) {
+      await service.flushRevokeOutbox(permanentlyFailingBroker(revokeDevice))
+    }
+    // Inside the window the relay is still held up on purpose: the revoke may yet land.
+    expect(ledger.hasDemand(ownerIdentityKey)).toBe(true)
+
+    now += RELAY_REVOKE_DEMAND_WINDOW_MS + 1
+    expect(ledger.hasDemand(ownerIdentityKey)).toBe(false)
+
+    // The intent is NOT discarded. Dropping it would leave a live credential on the relay with
+    // nothing left to kill it, so the item stays and keeps retrying on any future connection.
+    expect(revokeOutbox.pendingFor(ownerIdentityKey, relayHostId)).toHaveLength(1)
+    const callsBefore = revokeDevice.mock.calls.length
+    await service.flushRevokeOutbox(permanentlyFailingBroker(revokeDevice))
+    expect(revokeDevice.mock.calls.length).toBe(callsBefore + 1)
+  })
+
+  it('still lets a revoke that lands remove the item and release demand', async () => {
+    let now = Date.now()
+    const { revokeOutbox, ledger } = fixture(() => now)
+    revokeOutbox.enqueue(binding('device-1'))
+    const revokeDevice = vi.fn().mockResolvedValue(undefined)
+    const service = serviceOver(revokeOutbox, ledger)
+
+    await service.flushRevokeOutbox(permanentlyFailingBroker(revokeDevice))
+
+    expect(revokeDevice).toHaveBeenCalledTimes(1)
+    expect(revokeOutbox.pendingFor(ownerIdentityKey, relayHostId)).toHaveLength(0)
+    expect(ledger.hasDemand(ownerIdentityKey)).toBe(false)
+    // And it was removed on success, not merely aged out.
+    now += RELAY_REVOKE_DEMAND_WINDOW_MS + 1
+    expect(ledger.hasDemand(ownerIdentityKey)).toBe(false)
+  })
+})