fix(mobile): preserve delivery ambiguity across transport cutover (#20280)

* fix(mobile): preserve delivery ambiguity across transport cutover

Let physical close settle requests and retain its error as the cutover cause, copying only an existing delivery-unknown mark. Pin sent and unsent caller outcomes and both cutover predicate carriers.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* 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

* test(mobile): read the cutover cause without a type assertion

main's new casting gate rejects `(error as Error).cause`; narrow instead so the
assertion still distinguishes a missing cause from an unmarked one.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
Jinwoo Hong
2026-09-13 18:23:13 -04:00
committed by GitHub
parent 2fc84cb492
commit 85d7cf3cc1
5 changed files with 84 additions and 18 deletions
@@ -1,6 +1,11 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { connect } from './rpc-client'
import { isRpcDeliveryUnknown } from './rpc-delivery-ambiguity'
import {
createStableLogicalRpcClient,
isLogicalClientCutoverError,
LogicalClientCutoverError
} from './stable-logical-rpc-client'
vi.mock('./e2ee', () => ({
generateKeyPair: () => ({
@@ -72,7 +77,7 @@ function hasSentRequest(socket: MockWebSocket, method: string): boolean {
function connectAuthenticated(): { client: ReturnType<typeof connect>; socket: MockWebSocket } {
const client = connect('ws://desktop.invalid', 'token', 'server-key')
const socket = mockSockets[0]!
const socket = mockSockets[mockSockets.length - 1]!
socket.open()
socket.receive(JSON.stringify({ type: 'e2ee_ready' }))
socket.receive('encrypted:{"type":"e2ee_authenticated"}')
@@ -94,6 +99,52 @@ describe('mobile rpc-client delivery ambiguity marking', () => {
globalThis.WebSocket = originalWebSocket
})
it.each([true, false])(
'preserves physical delivery evidence at the cutover caller (sent=%s)',
async (sent) => {
const physical = sent
? connectAuthenticated()
: {
client: connect('ws://desktop.invalid', 'token', 'server-key'),
socket: mockSockets[0]!
}
const client = createStableLogicalRpcClient(physical.client, 'lan')
const replacement = connectAuthenticated()
const requestError = client
.sendRequest('worktree.create', { name: 'new' })
.catch((error: unknown) => error)
await Promise.resolve()
expect(hasSentRequest(physical.socket, 'worktree.create')).toBe(sent)
await client.migrateTo(replacement.client, 'relay')
const error = await requestError
expect(isLogicalClientCutoverError(error)).toBe(true)
expect(isRpcDeliveryUnknown(error)).toBe(sent)
expect(error).toBeInstanceOf(LogicalClientCutoverError)
expect(isRpcDeliveryUnknown(error instanceof Error ? error.cause : null)).toBe(sent)
expect(hasSentRequest(replacement.socket, 'worktree.create')).toBe(false)
expect(
physical.socket.sent.filter((payload) => payload.includes('worktree.create'))
).toHaveLength(sent ? 1 : 0)
client.close()
}
)
it('recognizes a cutover by class even when its message changes', () => {
const error = new LogicalClientCutoverError()
error.message = 'wrapped migration'
expect(isLogicalClientCutoverError(error)).toBe(true)
})
it('recognizes a cutover message from another bundle copy', () => {
expect(isLogicalClientCutoverError(new Error('RPC interrupted by connection migration'))).toBe(
true
)
expect(isLogicalClientCutoverError(new Error('Client closed'))).toBe(false)
expect(isLogicalClientCutoverError('RPC interrupted by connection migration')).toBe(false)
})
it('marks in-flight requests as delivery-unknown when the socket drops', async () => {
const { client, socket } = connectAuthenticated()
const requestError = client.sendRequest('terminal.send', { terminal: 't' }).then(
+13
View File
@@ -32,6 +32,19 @@ export type RpcClient = UnvalidatedRpcRequestPort & {
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. Pinned
* against the real clients in `rpc-client-delivery-ambiguity.test.ts` (direct) and
* `mobile-relay-rpc-session.test.ts` (relay) — a new implementation needs its own case.
*/
close: () => void
}
@@ -90,6 +90,7 @@ describe('stable logical RPC client', () => {
const nextSession = new FakeSession('connecting')
const pending = deferred<RpcResponse>()
oldSession.sendRequest.mockReturnValue(pending.promise)
oldSession.close.mockImplementation(() => pending.reject(new Error('Client closed')))
nextSession.sendRequest.mockResolvedValue(success('next'))
const client = createStableLogicalRpcClient(oldSession, 'lan')
const stream = vi.fn()
@@ -7,12 +7,16 @@ import {
import { waitForAuthenticated } from './replacement-session-authentication'
import { projectMobileRpcRequestParams } from './mobile-rpc-request-projection'
import { LogicalClientConnectionPath } from './logical-client-connection-path'
import { isRpcDeliveryUnknown, markRpcDeliveryUnknown } from './rpc-delivery-ambiguity'
export type MobileConnectionPath = 'lan' | 'tailscale' | 'relay'
export class LogicalClientCutoverError extends Error {
constructor() {
super('RPC interrupted by connection migration')
constructor(cause?: unknown) {
super('RPC interrupted by connection migration', { cause })
if (isRpcDeliveryUnknown(cause)) {
markRpcDeliveryUnknown(this)
}
}
}
@@ -33,10 +37,6 @@ type SubscriptionRecord = {
cancelled: boolean
}
type PendingRequest = {
reject: (error: Error) => void
}
export type StableLogicalRpcClient = RpcClient & {
migrateTo(
session: RpcClient,
@@ -75,7 +75,6 @@ export function createStableLogicalRpcClient(
let nextSubscriptionId = 0
let activeStateUnsubscribe: (() => void) | null = null
const subscriptions = new Map<number, SubscriptionRecord>()
const pendingRequests = new Set<PendingRequest>()
const stateListeners = new Set<(state: ConnectionState) => void>()
let state = initialSession.getState()
const connectionPath = new LogicalClientConnectionPath(() => state === 'connected')
@@ -90,22 +89,23 @@ export function createStableLogicalRpcClient(
if (suspended) {
return Promise.reject(new Error('Client suspended'))
}
const requestGeneration = generation
const session = activeSession
return new Promise<RpcResponse>((resolve, reject) => {
const pending = { reject }
pendingRequests.add(pending)
void session
.sendRequest(method, projectMobileRpcRequestParams(method, params), options)
.then(
(response) => {
pendingRequests.delete(pending)
// A correlated response is definitive even if close/cutover won the
// callback race after the physical promise had already settled.
resolve(response)
},
(error: unknown) => {
pendingRequests.delete(pending)
reject(error)
// Why: the retiring physical session settles this, so keep its error as the
// cause — it is the only evidence of whether the frame reached the wire.
reject(
requestGeneration !== generation ? new LogicalClientCutoverError(error) : error
)
}
)
})
@@ -260,15 +260,12 @@ export function createStableLogicalRpcClient(
suspended = false
previousStateUnsubscribe?.()
bindActiveState(nextSession, nextGeneration)
for (const pending of pendingRequests) {
pending.reject(new LogicalClientCutoverError())
}
pendingRequests.clear()
state = nextSession.getState()
connectionPath.clearAfterConnected()
for (const listener of stateListeners) {
listener(state)
}
// Only the physical sender knows whether a pending request reached the wire.
previous.close()
},
@@ -39,7 +39,11 @@ function fakeSession(): FakeSession {
getLastConnectedAt: () => null,
onStateChange: () => () => {},
notifyForeground: () => {},
close: () => {}
close: () => {
for (const settle of pending.splice(0)) {
settle(new Error('Client closed'))
}
}
}
}
return fake