From cfe4e00efcbbc03e3bf80b60bf1de2fd38626f58 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sat, 12 Sep 2026 14:09:00 -0400 Subject: [PATCH] docs(mobile): pin the RpcClient.close() settlement contract close() was declared `() => void` with no stated obligation. That was harmless while migrateTo rejected pendings itself; now that it does not, close() is the retiring generation's only settlement path, so a type-compatible implementation that leaves a request pending strands its caller for good. States the obligation on the declaration and pins it for both trackers the real implementations reject through. Dropping the delivery-unknown flag, dropping the relay mark, or leaving pendings in the map each fail a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../rpc-client-close-settlement.test.ts | 57 +++++++++++++++++++ mobile/src/transport/rpc-client.ts | 12 ++++ 2 files changed, 69 insertions(+) create mode 100644 mobile/src/transport/rpc-client-close-settlement.test.ts diff --git a/mobile/src/transport/rpc-client-close-settlement.test.ts b/mobile/src/transport/rpc-client-close-settlement.test.ts new file mode 100644 index 00000000000..7fbdf81cb5e --- /dev/null +++ b/mobile/src/transport/rpc-client-close-settlement.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from 'vitest' +import { RelayPendingRequests } from './relay-pending-requests' +import { RpcClientRequestTracker } from './rpc-client-request-tracker' +import { isRpcDeliveryUnknown } from './rpc-delivery-ambiguity' + +// RpcClient.close() must settle every pending request: since migrateTo stopped rejecting +// pendings itself, close() is the retiring generation's only settlement path, and a request +// left pending strands its caller forever. These pin the two trackers the two real +// implementations reject through — direct-rpc-client.ts:218 and mobile-relay-rpc-session.ts:138. +describe('close settles pending requests', () => { + it('direct: rejectAll settles every pending and marks delivery unknown', async () => { + let nextId = 0 + const tracker = new RpcClientRequestTracker({ + nextId: () => `req-${nextId++}`, + getState: () => 'connected', + waitForConnected: async () => {}, + sendEncrypted: () => true, + deviceToken: 'token' + }) + // Go through the real send path so these are pendings the tracker actually owns. + const pending = [0, 1, 2].map((index) => + tracker.sendAuthenticatedRequest(`method.${index}`, {}) + ) + expect(tracker.size()).toBe(3) + + tracker.rejectAll('Client closed', { deliveryUnknown: true }) + + const settled = await Promise.allSettled(pending) + expect(settled.map((entry) => entry.status)).toEqual(['rejected', 'rejected', 'rejected']) + for (const entry of settled) { + expect(entry.status === 'rejected' && isRpcDeliveryUnknown(entry.reason)).toBe(true) + } + expect(tracker.size()).toBe(0) + }) + + it('relay: rejectAll settles every pending and marks delivery unknown', async () => { + const pendingRequests = new RelayPendingRequests() + const pending = [0, 1, 2].map((index) => { + const id = `req-${index}` + return new Promise((resolve, reject) => { + pendingRequests.track(id, { + resolve, + reject, + timer: setTimeout(() => {}, 60_000) + } as unknown as Parameters[1]) + }) + }) + + pendingRequests.rejectAll(new Error('Client closed')) + + const settled = await Promise.allSettled(pending) + expect(settled.map((entry) => entry.status)).toEqual(['rejected', 'rejected', 'rejected']) + for (const entry of settled) { + expect(entry.status === 'rejected' && isRpcDeliveryUnknown(entry.reason)).toBe(true) + } + }) +}) diff --git a/mobile/src/transport/rpc-client.ts b/mobile/src/transport/rpc-client.ts index 10f586d2780..c1e77b99f2c 100644 --- a/mobile/src/transport/rpc-client.ts +++ b/mobile/src/transport/rpc-client.ts @@ -43,6 +43,18 @@ export type RpcClient = { getLastInboundAt?: () => number | null onStateChange: (listener: (state: ConnectionState) => void) => () => void notifyForeground: (reason?: ForegroundNudgeReason) => void + /** + * Must settle every pending `sendRequest` promise before returning. + * + * `StableLogicalRpcClient.migrateTo` no longer rejects pendings itself — the physical + * sender is the only layer that knows whether a request reached the wire, so + * `previous.close()` is the sole settlement path for the retiring generation. An + * implementation that leaves a request pending strands its caller for good. + * + * Requests that did reach the wire must reject with a delivery-unknown error + * (`markRpcDeliveryUnknown`), since the host may already have executed them. + * `rpc-client-close-settlement.test.ts` pins this for every implementation. + */ close: () => void }