refactor(mobile): name the RPC acceptance policies call sites hand-rolled (#19960)

This commit is contained in:
Jinwoo Hong
2026-09-10 19:37:52 -07:00
committed by GitHub
parent 54216a7868
commit 58ff95becb
12 changed files with 278 additions and 62 deletions
@@ -2,6 +2,7 @@ import { useEffect, useState } from 'react'
import { CODEX_RESET_CREDIT_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version'
import type { RpcClient } from '../transport/rpc-client'
import { startRuntimeCapabilityProbe } from '../transport/runtime-capability-probe'
import { rpcObjectResultOrNull } from '../transport/rpc-acceptance-policies'
// Why: source the capability string from the shared contract so a host bump can never
// silently drift from the mobile probe.
@@ -12,10 +13,7 @@ export async function readCodexResetCreditCapability(
): Promise<boolean> {
try {
const response = await client.sendRequest('status.get')
if (!response.ok || !response.result || typeof response.result !== 'object') {
return false
}
const capabilities = (response.result as { capabilities?: unknown }).capabilities
const capabilities = rpcObjectResultOrNull(response)?.capabilities
return (
Array.isArray(capabilities) && capabilities.includes(MOBILE_CODEX_RESET_CREDIT_CAPABILITY)
)
@@ -18,7 +18,7 @@ type SshTargetSummaryRow = { id: string; label: string }
async function requestResult(client: RpcClient, method: string): Promise<unknown> {
try {
const response = await client.sendRequest(method)
return response.ok ? (response as RpcSuccess).result : null
return response.ok ? response.result : null
} catch {
// Best-effort: hosts that predate a method still list repos; labels degrade to host ids.
return null
@@ -228,6 +228,34 @@ describe('mobile structured agent-session launch', () => {
})
})
describe.each(['top-level', 'nested'])('%s refusal messages', (location) => {
function refusalClient(message: unknown) {
const refusal = { code: 'method_not_found', ...(message === undefined ? {} : { message }) }
return clientReturning(
{ ok: true, result: { supported: true } },
location === 'top-level'
? { ok: false, error: refusal }
: { ok: true, result: { ok: false, refusal } }
)
}
it.each(
[undefined, null, 42, false, { text: 'unavailable' }, ['unavailable']].map((message) => ({
message
}))
)('keeps a malformed message $message unknown', async ({ message }) => {
await expect(
createMobileStructuredAgentSession(refusalClient(message), 'workspace-1', 'codex')
).resolves.toMatchObject({ kind: 'unknown' })
})
it('preserves the fallback for an empty string message', async () => {
await expect(
createMobileStructuredAgentSession(refusalClient(''), 'workspace-1', 'codex')
).resolves.toEqual({ kind: 'failed', message: 'Could not open Codex chat.' })
})
})
it.each(['structured_agent_session_unsupported', 'method_not_found'])(
'treats a top-level %s as a definitive refusal',
async (code) => {
@@ -137,18 +137,22 @@ export async function createMobileStructuredAgentSession(
}
}
// Why: this path distrusts the declared RpcResponse type — a malformed reply must read as
// unconfirmed, not as a refusal we can classify.
if (!response || typeof response !== 'object' || typeof response.ok !== 'boolean') {
return unknownCreateResult(agent, new Error(unconfirmedMessage(agent)))
}
if (!response.ok) {
const error = response.error as { code?: unknown; message?: unknown } | null | undefined
if (
!response.error ||
typeof response.error !== 'object' ||
typeof response.error.code !== 'string'
!error ||
typeof error !== 'object' ||
typeof error.code !== 'string' ||
typeof error.message !== 'string'
) {
return unknownCreateResult(agent, new Error(unconfirmedMessage(agent)))
}
return classifyCreateRefusal(agent, response.error.code, response.error.message)
return classifyCreateRefusal(agent, error.code, error.message)
}
const result = response.result as AgentSessionMutationResult<AgentSessionAttachResult>
if (!result || typeof result !== 'object' || typeof result.ok !== 'boolean') {
@@ -158,7 +162,8 @@ export async function createMobileStructuredAgentSession(
if (
!result.refusal ||
typeof result.refusal !== 'object' ||
typeof result.refusal.code !== 'string'
typeof result.refusal.code !== 'string' ||
typeof result.refusal.message !== 'string'
) {
return unknownCreateResult(agent, new Error(unconfirmedMessage(agent)))
}
@@ -1,4 +1,8 @@
import type { RpcResponse } from '../transport/types'
import {
isMethodNotFoundRefusal,
rpcObjectResultOrNull
} from '../transport/rpc-acceptance-policies'
export type TerminalUpdateViewportCapability = 'unknown' | 'supported' | 'unsupported'
@@ -14,17 +18,11 @@ export type TerminalViewportRefitTargetState = {
}
export function isTerminalUpdateViewportUpdated(response: RpcResponse): boolean {
if (!response.ok || typeof response.result !== 'object' || response.result == null) {
return false
}
return (response.result as { updated?: unknown }).updated === true
return rpcObjectResultOrNull(response)?.updated === true
}
export function isTerminalUpdateViewportApplied(response: RpcResponse): boolean {
if (!response.ok || typeof response.result !== 'object' || response.result == null) {
return false
}
return (response.result as { applied?: unknown }).applied === true
return rpcObjectResultOrNull(response)?.applied === true
}
export function resolveTerminalUpdateViewportCapability(
@@ -33,7 +31,7 @@ export function resolveTerminalUpdateViewportCapability(
if (response.ok) {
return 'supported'
}
return response.error.code === 'method_not_found' ? 'unsupported' : 'unknown'
return isMethodNotFoundRefusal(response) ? 'unsupported' : 'unknown'
}
// Why: defer height refits while typing, then coalesce every skipped layout
@@ -21,7 +21,11 @@ import {
type MobileRelayDirectUpgradeJournal
} from './mobile-relay-direct-upgrade-journal'
import type { RpcClient } from './rpc-client'
import type { HostProfile, RpcResponse } from './types'
import type { HostProfile } from './types'
import {
isMethodNotFoundRefusal,
requireRpcResultOrThrowCodedError
} from './rpc-acceptance-policies'
export type MobileRelayDirectUpgradeResult = {
host: HostProfile
@@ -79,11 +83,13 @@ export async function upgradeDirectMobileRelay(args: {
reqId: journal.reqId,
newResumeTokenHash: journal.pendingResumeTokenHash
})
if (isMethodNotFound(provisionResponse)) {
if (isMethodNotFoundRefusal(provisionResponse)) {
await dependencies.clearJournal(args.host.id)
return null
}
const installed = DeviceCredentialInstalledSchema.parse(requireSuccess(provisionResponse))
const installed = DeviceCredentialInstalledSchema.parse(
requireRpcResultOrThrowCodedError(provisionResponse)
)
assertDirectInstall(journal, installed)
const reconciled = await getEndpoints(args.client, journal.reqId)
if (reconciled === 'method-not-found') {
@@ -136,10 +142,10 @@ async function getEndpoints(
installReqId: string
): Promise<PairingGetEndpointsResult | 'method-not-found'> {
const response = await client.sendRequest('pairing.getEndpoints', { installReqId })
if (isMethodNotFound(response)) {
if (isMethodNotFoundRefusal(response)) {
return 'method-not-found'
}
return PairingGetEndpointsResultSchema.parse(requireSuccess(response))
return PairingGetEndpointsResultSchema.parse(requireRpcResultOrThrowCodedError(response))
}
function assertDirectInstall(
@@ -162,14 +168,3 @@ function assertCommitted(
throw new Error('relay credential install was not authoritatively reconciled')
}
}
function requireSuccess(response: RpcResponse): unknown {
if (!response.ok) {
throw new Error(`${response.error.code}: ${response.error.message}`)
}
return response.result
}
function isMethodNotFound(response: RpcResponse): boolean {
return !response.ok && response.error.code === 'method_not_found'
}
@@ -25,7 +25,8 @@ import {
type PairingCandidateClient
} from './mobile-relay-physical-client'
import { createRecoveringPairingRelayCandidate } from './pairing-relay-candidate'
import type { HostProfile, RpcResponse } from './types'
import type { HostProfile } from './types'
import { requireRpcResultOrThrowCodedError } from './rpc-acceptance-policies'
export type MobileRelayPairingRecoveryResult = 'none' | 'recovered' | 'deferred' | 'abandoned'
@@ -128,7 +129,7 @@ async function runRecovery(
if (credential.kind === 'invite' && endpoints.installStatus?.state === 'not-found') {
journal = await transitionToInviteAuthorization(journal, dependencies)
const installed = DeviceCredentialInstalledSchema.parse(
requireSuccess(
requireRpcResultOrThrowCodedError(
await client.sendRequest('pairing.provisionRelay', {
reqId: journal.metadata.installReqId,
newResumeTokenHash: journal.metadata.pendingResumeTokenHash
@@ -220,7 +221,7 @@ async function getRecoveryStatus(
kind: 'resume' | 'invite'
) {
return PairingGetEndpointsResultSchema.parse(
requireSuccess(
requireRpcResultOrThrowCodedError(
await client.sendRequest('pairing.getEndpoints', {
installReqId: journal.metadata.installReqId,
...(kind === 'resume' ? { resumeConfirmReqId: journal.metadata.resumeConfirmReqId } : {})
@@ -294,13 +295,6 @@ function pairingRelay(journal: MobileRelayPairingJournal): PairingRelay {
return { ...journal.metadata.relay, inviteToken: journal.secrets.inviteToken }
}
function requireSuccess(response: RpcResponse): unknown {
if (!response.ok) {
throw new Error(`${response.error.code}: ${response.error.message}`)
}
return response.result
}
function assertCommitted(
endpoints: ReturnType<typeof PairingGetEndpointsResultSchema.parse>,
installed: DeviceCredentialInstalled
@@ -9,6 +9,7 @@ import {
updateTerminalSubscriptionViewport
} from './rpc-client-terminal-subscription'
import { buildReadyStreamUnsubscribe } from './rpc-client-server-subscription'
import { isStreamingOpenerReply } from './rpc-acceptance-policies'
import type { RpcClient } from './rpc-client'
import type { RpcResponse, RpcSuccess } from './types'
@@ -119,7 +120,7 @@ export class MobileRelayRpcStreams {
}
}
}
if (response.ok && response.streaming !== true) {
if (response.ok && !isStreamingOpenerReply(response)) {
this.cancelledSubscriptions.delete(response.id)
}
return true
@@ -7,7 +7,11 @@ import {
} from '../../../src/shared/mobile-relay-credential-contract'
import { connect, type ConnectOptions } from './rpc-client'
import { resolvePairingHostIdentity, saveHost } from './host-store'
import type { HostProfile, PairingOffer, RpcResponse } from './types'
import type { HostProfile, PairingOffer } from './types'
import {
isMethodNotFoundRefusal,
requireRpcResultOrThrowCodedError
} from './rpc-acceptance-policies'
import {
createMobileRelayPairingJournal,
type MobileRelayPairingJournal
@@ -219,7 +223,7 @@ async function runPairing(
reqId: journal.metadata.installReqId,
newResumeTokenHash: journal.metadata.pendingResumeTokenHash
})
if (isMethodNotFound(provision)) {
if (isMethodNotFoundRefusal(provision)) {
if (winner.path !== 'direct') {
throw new Error('relay pairing RPC unavailable after relay path authentication')
}
@@ -227,9 +231,11 @@ async function runPairing(
await dependencies.clearJournal(journal.metadata.journalId)
return { hostId }
}
const installed = DeviceCredentialInstalledSchema.parse(requireSuccess(provision))
const installed = DeviceCredentialInstalledSchema.parse(
requireRpcResultOrThrowCodedError(provision)
)
const endpoints = PairingGetEndpointsResultSchema.parse(
requireSuccess(
requireRpcResultOrThrowCodedError(
await winner.client.sendRequest('pairing.getEndpoints', {
installReqId: journal.metadata.installReqId
})
@@ -283,17 +289,6 @@ function relayWebSocketUrl(relay: MobileRelayEndpoint): string {
return url.toString()
}
function requireSuccess(response: RpcResponse): unknown {
if (!response.ok) {
throw new Error(`${response.error.code}: ${response.error.message}`)
}
return response.result
}
function isMethodNotFound(response: RpcResponse): boolean {
return !response.ok && response.error.code === 'method_not_found'
}
function assertCommittedInstall(
status:
| { state: 'not-found' }
@@ -0,0 +1,168 @@
import { describe, expect, it } from 'vitest'
import type { RpcResponse } from './types'
import {
isMethodNotFoundRefusal,
isStreamingOpenerReply,
requireRpcResultOrThrowCodedError,
rpcObjectResultOrNull
} from './rpc-acceptance-policies'
const meta = { runtimeId: 'runtime-1' }
function success(result: unknown, streaming?: true): RpcResponse {
return { id: 'rpc-1', ok: true, result, _meta: meta, ...(streaming ? { streaming } : {}) }
}
function refusal(code: string, message = 'Nope'): RpcResponse {
return { id: 'rpc-1', ok: false, error: { code, message }, _meta: meta }
}
/** Every result partition a policy has to survive. */
const resultPartitions: [string, unknown][] = [
['object result', { value: 1 }],
['undefined result', undefined],
['null result', null],
['empty object result', {}],
['numeric result', 7],
['zero result', 0],
['string result', 'done'],
['empty string result', ''],
['boolean result', false],
['array result', [1, 2]],
['empty array result', []]
]
describe('requireRpcResultOrThrowCodedError', () => {
it.each(resultPartitions)('returns the %s untouched', (_label, result) => {
expect(requireRpcResultOrThrowCodedError(success(result))).toEqual(result)
})
it('returns an absent result field as undefined', () => {
const response = { id: 'rpc-1', ok: true, _meta: meta } as unknown as RpcResponse
expect(requireRpcResultOrThrowCodedError(response)).toBeUndefined()
})
it('throws a code-prefixed message on refusal', () => {
expect(() => requireRpcResultOrThrowCodedError(refusal('method_not_found', 'no such'))).toThrow(
'method_not_found: no such'
)
})
it('throws even when the refusal carries an empty message', () => {
expect(() => requireRpcResultOrThrowCodedError(refusal('runtime_error', ''))).toThrow(
'runtime_error: '
)
})
})
describe('rpcObjectResultOrNull', () => {
it('accepts a plain object result', () => {
expect(rpcObjectResultOrNull(success({ value: 1 }))).toEqual({ value: 1 })
})
it('accepts an empty object result', () => {
expect(rpcObjectResultOrNull(success({}))).toEqual({})
})
it('accepts an array result, because arrays are objects', () => {
expect(rpcObjectResultOrNull(success([1, 2]))).toEqual([1, 2])
})
it.each([
['null', null],
['undefined', undefined],
['numeric', 7],
['zero', 0],
['string', 'done'],
['empty string', ''],
['boolean', false],
['true', true]
])('refuses a %s result', (_label, result) => {
expect(rpcObjectResultOrNull(success(result))).toBeNull()
})
it('refuses a refusal regardless of its code', () => {
expect(rpcObjectResultOrNull(refusal('method_not_found'))).toBeNull()
expect(rpcObjectResultOrNull(refusal('runtime_error'))).toBeNull()
})
})
// A refusal that illegally carries success-shaped fields: without the `ok` check each of these
// would read the stray field and answer as if the call had succeeded.
describe('a refusal carrying stray success fields', () => {
const strayRefusal = {
id: 'rpc-1',
ok: false,
error: { code: 'method_not_found', message: 'Nope' },
result: { value: 1 },
streaming: true,
_meta: meta
} as unknown as RpcResponse
it('yields null rather than the stray result', () => {
expect(rpcObjectResultOrNull(strayRefusal)).toBeNull()
})
it('is still recognised as method-not-found', () => {
expect(isMethodNotFoundRefusal(strayRefusal)).toBe(true)
})
// The mirror case: a success carrying a stray error must not read as a refusal.
it('does not read a success carrying a stray error as a refusal', () => {
const straySuccess = {
id: 'rpc-1',
ok: true,
result: { value: 1 },
error: { code: 'method_not_found', message: 'Nope' },
_meta: meta
} as unknown as RpcResponse
expect(isMethodNotFoundRefusal(straySuccess)).toBe(false)
})
})
describe('isMethodNotFoundRefusal', () => {
it('matches only the method_not_found code', () => {
expect(isMethodNotFoundRefusal(refusal('method_not_found'))).toBe(true)
expect(isMethodNotFoundRefusal(refusal('runtime_error'))).toBe(false)
expect(isMethodNotFoundRefusal(refusal('METHOD_NOT_FOUND'))).toBe(false)
})
it('never matches a success, including one with a null result', () => {
expect(isMethodNotFoundRefusal(success(null))).toBe(false)
expect(isMethodNotFoundRefusal(success({ code: 'method_not_found' }))).toBe(false)
})
})
describe('isStreamingOpenerReply', () => {
it('accepts a success flagged streaming', () => {
expect(isStreamingOpenerReply(success({ subscriptionId: 's1' }, true))).toBe(true)
})
it('refuses a success with no streaming flag', () => {
expect(isStreamingOpenerReply(success({ subscriptionId: 's1' }))).toBe(false)
})
// A truthy non-boolean off the wire must not open a stream: the registry would route it to
// handleStreamingResponse and wait for frames that never come.
it.each([['yes'], [1], [{}]])('refuses a truthy non-boolean streaming flag %j', (flag) => {
const response = {
id: 'rpc-1',
ok: true,
result: { subscriptionId: 's1' },
streaming: flag,
_meta: meta
} as unknown as RpcResponse
expect(isStreamingOpenerReply(response)).toBe(false)
})
it('refuses a refusal even when it carries a streaming flag', () => {
const response = {
id: 'rpc-1',
ok: false,
error: { code: 'runtime_error', message: 'Nope' },
streaming: true,
_meta: meta
} as unknown as RpcResponse
expect(isStreamingOpenerReply(response)).toBe(false)
})
})
@@ -0,0 +1,33 @@
import type { RpcResponse, RpcSuccess } from './types'
// Named acceptance policies for RPC replies. Call sites used to hand-roll these
// predicates and did not agree with each other; each policy here preserves one
// call site's existing acceptance exactly. Do not merge two policies without
// proving every caller of both tolerates the wider or narrower set.
/** Throws `code: message` on refusal. Diagnostic text; not user-facing. */
export function requireRpcResultOrThrowCodedError(response: RpcResponse): unknown {
if (!response.ok) {
throw new Error(`${response.error.code}: ${response.error.message}`)
}
return response.result
}
/** Accepts only a success whose result is a non-null object. Arrays qualify. */
export function rpcObjectResultOrNull(response: RpcResponse): Record<string, unknown> | null {
if (!response.ok || typeof response.result !== 'object' || response.result === null) {
return null
}
return response.result as Record<string, unknown>
}
export function isMethodNotFoundRefusal(response: RpcResponse): boolean {
return !response.ok && response.error.code === 'method_not_found'
}
/** A success that opened a stream rather than delivering a terminal result. */
export function isStreamingOpenerReply(
response: RpcResponse
): response is RpcSuccess & { streaming: true } {
return response.ok && response.streaming === true
}
@@ -8,6 +8,7 @@ import {
updateTerminalSubscriptionViewport
} from './rpc-client-terminal-subscription'
import { buildReadyStreamUnsubscribe } from './rpc-client-server-subscription'
import { isStreamingOpenerReply } from './rpc-acceptance-policies'
import {
isStreamingSubscriptionReadyResult,
isTerminalSubscribedResult
@@ -112,7 +113,7 @@ export class RpcClientStreamRegistry {
}
handleResponse(response: RpcResponse): boolean {
if (response.ok && response.streaming === true) {
if (isStreamingOpenerReply(response)) {
this.handleStreamingResponse(response)
return true
}