From b0070e37203ee68cd771ef68be32eec46d04ded1 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Sun, 13 Sep 2026 17:53:58 -0400 Subject: [PATCH] refactor(mobile): migrate settings reads to RpcOperation (#20499) * refactor(mobile): migrate settings reads to RpcOperation Replay the settings slice on the landed RPC foundation after rebasing onto main. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): refresh task parity snapshots after main rebase Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): correct rebased declaration parity hash Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): account for main task declaration Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): preserve raw RPC rejection timing Return the transport promise directly and interpret replies separately so sibling Promise.all rejection order cannot change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): refresh parity hashes after timing fix Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): use operation interpreter after raw request * test(mobile): refresh settings migration parity hashes Refresh hook and statement parity hashes for the two task declarations whose settings reads now use RpcOperation request and interpretation. Changed declarations: - useMobileTasksRuntimeHydration: settings.get replaced by settingsRead request/interpret. - useMobileTasksWorkspaceCreateActions: settings.get response handling replaced by settingsRead request/interpret. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../MobileAgentSessionHistoryPanel.tsx | 16 +- .../use-new-workspace-create-submit.ts | 15 +- .../use-new-workspace-runtime-context.ts | 15 +- mobile/src/home/mobile-home-host-requests.ts | 10 +- .../src/host-screen/use-host-repo-metadata.ts | 40 ++-- .../session/mobile-new-tab-agent-loader.ts | 15 +- .../session/use-pr-bot-author-overrides.ts | 22 +- .../mobile-tasks-refactor-parity.test.ts | 32 +-- .../use-mobile-tasks-runtime-hydration.tsx | 10 +- ...-mobile-tasks-workspace-create-actions.tsx | 11 +- .../src/transport/rpc-acceptance-policies.ts | 15 ++ .../transport/rpc-operation-compile-fence.ts | 38 +++- .../src/transport/rpc-operation-contract.ts | 51 +++-- mobile/src/transport/rpc-operation-reply.ts | 92 ++++++++ mobile/src/transport/rpc-operation.ts | 175 +++++++-------- .../settings-read-operations.test.ts | 200 ++++++++++++++++++ .../src/transport/settings-read-operations.ts | 93 ++++++++ .../unvalidated-rpc-request-port-inventory.ts | 18 +- 18 files changed, 666 insertions(+), 202 deletions(-) create mode 100644 mobile/src/transport/rpc-operation-reply.ts create mode 100644 mobile/src/transport/settings-read-operations.test.ts create mode 100644 mobile/src/transport/settings-read-operations.ts diff --git a/mobile/src/agent-history/MobileAgentSessionHistoryPanel.tsx b/mobile/src/agent-history/MobileAgentSessionHistoryPanel.tsx index 4f1d9ecf656..6d1ea4303fa 100644 --- a/mobile/src/agent-history/MobileAgentSessionHistoryPanel.tsx +++ b/mobile/src/agent-history/MobileAgentSessionHistoryPanel.tsx @@ -1,3 +1,4 @@ +import { optionalSettingsRead } from '../transport/settings-read-operations' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { ActivityIndicator, Pressable, Text, TextInput, View } from 'react-native' import { SafeAreaView } from 'react-native-safe-area-context' @@ -384,8 +385,8 @@ async function loadMobileResumeMetadata(client: Pick): client .sendRequest('projectGroup.list', undefined, { timeoutMs: RESUME_RPC_TIMEOUT_MS }) .catch(() => null), - client - .sendRequest('settings.get', undefined, { timeoutMs: RESUME_RPC_TIMEOUT_MS }) + optionalSettingsRead + .request(client, undefined, { timeoutMs: RESUME_RPC_TIMEOUT_MS }) .catch(() => null), client .sendRequest('worktree.ps', { limit: 10000 }, { timeoutMs: RESUME_RPC_TIMEOUT_MS }) @@ -405,17 +406,18 @@ async function loadMobileResumeMetadata(client: Pick): projectGroupResponse?.ok === true ? (projectGroupResponse.result as { groups?: MobileAiVaultResumeProjectGroup[] }) : null - const settingsResult = - settingsResponse?.ok === true - ? (settingsResponse.result as { settings?: MobileAiVaultResumeSettings }) - : null + const settingsResult = settingsResponse ? optionalSettingsRead.interpret(settingsResponse) : null + const settings = settingsResult?.accepted + ? // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. + (settingsResult.value as MobileAiVaultResumeSettings | null | undefined) + : null const worktreeResult = worktreeResponse?.ok === true ? (worktreeResponse.result as { worktrees?: Worktree[] }) : null return { repos: repoResult.repos ?? [], folderWorkspaces: folderWorkspaceResult?.folderWorkspaces ?? [], projectGroups: projectGroupResult?.groups ?? [], - settings: settingsResult?.settings ?? null, + settings: settings ?? null, worktrees: worktreeResult?.worktrees ?? null } } diff --git a/mobile/src/components/use-new-workspace-create-submit.ts b/mobile/src/components/use-new-workspace-create-submit.ts index 1a7ba078b27..c9265f8ed21 100644 --- a/mobile/src/components/use-new-workspace-create-submit.ts +++ b/mobile/src/components/use-new-workspace-create-submit.ts @@ -1,8 +1,8 @@ +import { settingsRead } from '../transport/settings-read-operations' import { useRef, useState, type Dispatch, type SetStateAction } from 'react' import type { PersistedTrustedOrcaHooks } from '../../../src/shared/orca-yaml-hook-types' import type { RetiredNameRegistry } from '../../../src/shared/worktree/retired-name-registry' import type { RpcClient } from '../transport/rpc-client' -import type { RpcSuccess } from '../transport/types' import { createBlankWorkspace } from '../tasks/blank-workspace-create' import { isMobileTuiAgentEnabled } from '../tasks/mobile-tui-agents' import { @@ -88,13 +88,12 @@ export function useNewWorkspaceCreateSubmit(args: { } let latestRuntimeSettings = args.runtimeSettings try { - const settingsResponse = await client.sendRequest('settings.get') - if (settingsResponse.ok) { - const result = (settingsResponse as RpcSuccess).result as { - settings: NewWorktreeRuntimeSettings - } - latestRuntimeSettings = result.settings - args.setRuntimeSettings(result.settings) + const settingsReply = await settingsRead.request(client) + const settings = settingsRead.interpret(settingsReply) + if (settings.accepted) { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. + latestRuntimeSettings = settings.value as NewWorktreeRuntimeSettings + args.setRuntimeSettings(latestRuntimeSettings) } } catch { // The runtime validates the same setting before spawning. diff --git a/mobile/src/components/use-new-workspace-runtime-context.ts b/mobile/src/components/use-new-workspace-runtime-context.ts index 05df7b53fe3..e178d714ce8 100644 --- a/mobile/src/components/use-new-workspace-runtime-context.ts +++ b/mobile/src/components/use-new-workspace-runtime-context.ts @@ -1,3 +1,4 @@ +import { settingsRead } from '../transport/settings-read-operations' import { useEffect, useState } from 'react' import type { PersistedTrustedOrcaHooks } from '../../../src/shared/orca-yaml-hook-types' import type { RpcClient } from '../transport/rpc-client' @@ -39,20 +40,18 @@ export function useNewWorkspaceRuntimeContext( client.sendRequest('linear.status') ]) const [settingsRes, uiRes] = await Promise.allSettled([ - client.sendRequest('settings.get'), + settingsRead.request(client), client.sendRequest('ui.get') ]) if (stale) { return } - const settingsResult = settledSuccess(settingsRes) - const settingsValue = settingsResult - ? ( - settingsResult.result as { - settings: NewWorktreeRuntimeSettings & { visibleTaskProviders?: unknown } - } - ).settings + const settingsResult = + settingsRes.status === 'fulfilled' ? settingsRead.interpret(settingsRes.value) : null + const settingsValue = settingsResult?.accepted + ? // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. + (settingsResult.value as NewWorktreeRuntimeSettings & { visibleTaskProviders?: unknown }) : null if (settingsValue) { setRuntimeSettings(settingsValue) diff --git a/mobile/src/home/mobile-home-host-requests.ts b/mobile/src/home/mobile-home-host-requests.ts index e0e19a036d3..cf4c45cf4a8 100644 --- a/mobile/src/home/mobile-home-host-requests.ts +++ b/mobile/src/home/mobile-home-host-requests.ts @@ -1,3 +1,4 @@ +import { settingsRead } from '../transport/settings-read-operations' import { decodeAccountsSnapshot, type AccountsSnapshot } from '../components/AccountUsage' import type { HomeStatsSummary } from '../stats/home-stats-total' import { @@ -73,7 +74,7 @@ export function fetchMobileHomeTaskProviders( disposed: () => boolean ): void { Promise.all([ - sendSingleFlightRequest(client, hostId, 'settings.get'), + settingsRead.requestSingleFlight(client, hostId), sendSingleFlightRequest(client, hostId, 'preflight.check'), sendSingleFlightRequest(client, hostId, 'linear.status') ]) @@ -81,9 +82,10 @@ export function fetchMobileHomeTaskProviders( if (disposed()) { return } - const settings = settingsResponse.ok - ? (((settingsResponse.result as { settings?: HomeTaskSettings }).settings ?? - {}) as HomeTaskSettings) + const settingsResult = settingsRead.interpret(settingsResponse) + const settings = settingsResult.accepted + ? // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. + ((settingsResult.value ?? {}) as HomeTaskSettings) : {} const preflight = preflightResponse.ok ? (preflightResponse.result as HomePreflightStatus) diff --git a/mobile/src/host-screen/use-host-repo-metadata.ts b/mobile/src/host-screen/use-host-repo-metadata.ts index 6840184c3a0..64ece7b700e 100644 --- a/mobile/src/host-screen/use-host-repo-metadata.ts +++ b/mobile/src/host-screen/use-host-repo-metadata.ts @@ -1,8 +1,9 @@ +import { optionalSettingsRead } from '../transport/settings-read-operations' import { useCallback } from 'react' import { getRepoExecutionHostId } from '../../../src/shared/execution-host' import { setCachedRepos } from '../cache/repo-cache' import type { RpcClient } from '../transport/rpc-client' -import type { ConnectionState, RpcSuccess } from '../transport/types' +import type { ConnectionState, RpcResponse, RpcSuccess } from '../transport/types' import type { RepoSummary } from '../worktree/host-worktree-rpc-types' import { repoColor } from '../worktree/repo-color' import { @@ -15,10 +16,12 @@ const REPO_METADATA_REFRESH_MS = 60_000 type SshTargetSummaryRow = { id: string; label: string } -async function requestResult(client: RpcClient, method: string): Promise { +async function requestMetadataResponse( + client: RpcClient, + method: 'repo.list' | 'ssh.listTargetSummaries' | 'host.platform' +): Promise { try { - const response = await client.sendRequest(method) - return response.ok ? response.result : null + return await client.sendRequest(method) } catch { // Best-effort: hosts that predate a method still list repos; labels degrade to host ids. return null @@ -45,8 +48,8 @@ function readHostPlatform(result: unknown): NodeJS.Platform | null { } function readHostSettingOverrides(result: unknown): unknown { - return (result as { settings?: { hostSettingOverrides?: unknown } } | null)?.settings - ?.hostSettingOverrides + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. + return (result as { hostSettingOverrides?: unknown } | null)?.hostSettingOverrides } export function useHostRepoMetadata(args: { @@ -90,8 +93,12 @@ export function useHostRepoMetadata(args: { try { do { fetchRepoMetadataPendingRef.current.delete(requestClient) - const repoResponse = await requestClient.sendRequest('repo.list') - if (clientRef.current !== requestClient || hostId !== requestHostId || !repoResponse.ok) { + const repoResponse = await requestMetadataResponse(requestClient, 'repo.list') + if ( + clientRef.current !== requestClient || + hostId !== requestHostId || + !repoResponse?.ok + ) { return } const repoResult = (repoResponse as RpcSuccess).result as { repos: RepoSummary[] } @@ -120,20 +127,25 @@ export function useHostRepoMetadata(args: { const hostIds = new Set(repoResult.repos.map((repo) => getRepoExecutionHostId(repo))) if (hostIds.size > 1) { const [sshTargets, hostSettings, hostPlatform] = await Promise.all([ - requestResult(requestClient, 'ssh.listTargetSummaries'), - requestResult(requestClient, 'settings.get'), - requestResult(requestClient, 'host.platform') + requestMetadataResponse(requestClient, 'ssh.listTargetSummaries'), + optionalSettingsRead.request(requestClient).catch(() => null), + requestMetadataResponse(requestClient, 'host.platform') ]) if (clientRef.current !== requestClient || hostId !== requestHostId) { return } + const hostSettingsResult = hostSettings + ? optionalSettingsRead.interpret(hostSettings) + : null setHostLabelById( buildHostLabelById({ - sshTargets: readSshTargets(sshTargets), - hostSettingOverrides: readHostSettingOverrides(hostSettings) + sshTargets: readSshTargets(sshTargets?.ok ? sshTargets.result : null), + hostSettingOverrides: readHostSettingOverrides( + hostSettingsResult?.accepted ? hostSettingsResult.value : undefined + ) }) ) - setHostPlatform(readHostPlatform(hostPlatform)) + setHostPlatform(readHostPlatform(hostPlatform?.ok ? hostPlatform.result : null)) } } while (fetchRepoMetadataPendingRef.current.has(requestClient)) } catch { diff --git a/mobile/src/session/mobile-new-tab-agent-loader.ts b/mobile/src/session/mobile-new-tab-agent-loader.ts index ea87e74ba0e..22ec2957df6 100644 --- a/mobile/src/session/mobile-new-tab-agent-loader.ts +++ b/mobile/src/session/mobile-new-tab-agent-loader.ts @@ -1,3 +1,4 @@ +import { newTabSettingsRead } from '../transport/settings-read-operations' import type { RpcClient } from '../transport/rpc-client' import type { RpcFailure, RpcSuccess } from '../transport/types' import { isFloatingWorkspaceWorktreeId } from './floating-workspace' @@ -23,22 +24,16 @@ export async function loadMobileNewTabAgentOptions(args: { ? client.sendRequest('preflight.detectAgents') : loadWorkspaceDetectedAgents(client, worktreeId) const [settingsResponse, detectedResponse] = await Promise.all([ - client.sendRequest('settings.get'), + newTabSettingsRead.request(client), detectedAgentsRequest ]) - if (!settingsResponse.ok) { - throw new Error((settingsResponse as RpcFailure).error.message) - } + const readSettings = newTabSettingsRead.interpret(settingsResponse) if (!detectedResponse.ok) { throw new Error((detectedResponse as RpcFailure).error.message) } - const settings = ( - (settingsResponse as RpcSuccess).result as { - settings?: MobileNewTabAgentSettings - } - ).settings return buildMobileNewTabAgentOptions( - settings, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. + readSettings() as MobileNewTabAgentSettings | undefined, (detectedResponse as RpcSuccess).result as unknown[] ) } diff --git a/mobile/src/session/use-pr-bot-author-overrides.ts b/mobile/src/session/use-pr-bot-author-overrides.ts index 94e39794616..5bce59d6d3c 100644 --- a/mobile/src/session/use-pr-bot-author-overrides.ts +++ b/mobile/src/session/use-pr-bot-author-overrides.ts @@ -1,5 +1,6 @@ +import { botOverridesRead } from '../transport/settings-read-operations' import { useEffect, useMemo, useRef, useState } from 'react' -import type { ConnectionState, RpcSuccess } from '../transport/types' +import type { ConnectionState } from '../transport/types' import type { RpcClient } from '../transport/rpc-client' import { createBotAuthorOverrideSet } from '../../../src/shared/pr-bot-author-overrides' @@ -32,21 +33,16 @@ export function usePRBotAuthorOverrides( return } let stale = false - void client - .sendRequest('settings.get') + void botOverridesRead + .request(client) .then((response) => { - if (stale || !response.ok) { + if (stale) { return } - const result = (response as RpcSuccess).result as { - settings?: { prBotAuthorOverrides?: unknown } - } | null - const overrides = result?.settings?.prBotAuthorOverrides - setLogins( - Array.isArray(overrides) - ? overrides.filter((login): login is string => typeof login === 'string') - : [] - ) + const overrides = botOverridesRead.interpret(response) + if (overrides.accepted) { + setLogins(overrides.value) + } }) .catch(() => { // Best-effort: without the setting the heuristics still classify most bots. diff --git a/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts b/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts index 09fa587e0b1..111b119db8e 100644 --- a/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts +++ b/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts @@ -16,51 +16,51 @@ const hash = (parts: string[] | string): string => .update(Array.isArray(parts) ? parts.join('\n') : parts) .digest('hex') -// Task and Linear sort tests cover computation changes; render/style guards remain. -const EXPECTED_SCREEN_HOOKS = '25c9a72805e48caa9c6758d14a128fea1bdc6cfdea3e4e39a7f0026c5933c8d7' -const EXPECTED_DIFF_HOOKS = '93c7189b32bed8456cc51814fffa8ce80cf62011ef968a9d53ddec2b9686f58f' -const EXPECTED_STATEMENTS = '1413f6e843f7a7ae849767b26b25d5eafc7fffda1fc37f283fbd883d9d8a4bda' -const EXPECTED_DECLARATIONS = '6ad0397123e59fc1047a14049c86ff31d81723673a7a7f5c41677471aec58415' -const EXPECTED_SEMANTICS = '4758ba019e4ff7cadd7ee02338719fa4fc4e1443e34cc290842819cfa1a70181' -const EXPECTED_STYLES = '1db6af69c791d9963928541ad5310942fcbda6d984b422c90b6eb92b6816579a' -const EXPECTED_RENDER_TREE = '2111145136b1e4fbca150d4792d735a90e992488e9934cfc1a8b8f3be981f39f' +// Bound settings requests change source signatures; their behavior is covered by settings-read-operations.test.ts. +const SETTINGS_RPC_SCREEN_HOOKS = 'fb2d873e06001fbae7cee78d079b3df9dc2eedb56ab2f03c7ffb431bc8666191' +const PRE_REFACTOR_DIFF_HOOKS = '93c7189b32bed8456cc51814fffa8ce80cf62011ef968a9d53ddec2b9686f58f' +const SETTINGS_RPC_STATEMENTS = '1c99d6382f74c37c0ff896dfa634fb503c9fe8062e2280328d0b82f79f658fdb' +const MAIN_REBASED_DECLARATIONS = '6ad0397123e59fc1047a14049c86ff31d81723673a7a7f5c41677471aec58415' +const SETTINGS_RPC_SEMANTICS = '2431b1c07dfe9a9c94f5d3f4e91415ed99bd9e1bce3794f8bd5f094a29134d77' +const PRE_REFACTOR_STYLES = '1db6af69c791d9963928541ad5310942fcbda6d984b422c90b6eb92b6816579a' +const PRE_REFACTOR_RENDER_TREE = '2111145136b1e4fbca150d4792d735a90e992488e9934cfc1a8b8f3be981f39f' describe('Mobile Tasks refactor parity', () => { it('preserves recursively flattened hook and dependency order', () => { const screenHooks = readFlattenedMobileTasksHookSignatures('MobileTasksScreen') expect(screenHooks).toHaveLength(350) - expect(hash(screenHooks)).toBe(EXPECTED_SCREEN_HOOKS) + expect(hash(screenHooks)).toBe(SETTINGS_RPC_SCREEN_HOOKS) const diffHooks = readFlattenedMobileTasksHookSignatures('GitHubPrFileDiff') expect(diffHooks).toHaveLength(3) - expect(hash(diffHooks)).toBe(EXPECTED_DIFF_HOOKS) + expect(hash(diffHooks)).toBe(PRE_REFACTOR_DIFF_HOOKS) }) it('preserves every screen statement in execution order', () => { const statements = readFlattenedMobileTasksCoreStatements() expect(statements).toHaveLength(417) - expect(hash(statements)).toBe(EXPECTED_STATEMENTS) + expect(hash(statements)).toBe(SETTINGS_RPC_STATEMENTS) }) it('preserves every moved top-level declaration', () => { const declarations = readMobileTasksDeclarationSignatures() expect(declarations).toHaveLength(194) - expect(hash(declarations)).toBe(EXPECTED_DECLARATIONS) + expect(hash(declarations)).toBe(MAIN_REBASED_DECLARATIONS) }) it('preserves RPC calls, runtime strings, and JSX host signatures', () => { const semantics = readMobileTasksSemanticSource() - expect(semantics.split('\n')).toHaveLength(3_500) - expect(hash(semantics)).toBe(EXPECTED_SEMANTICS) + expect(semantics.split('\n')).toHaveLength(3_496) + expect(hash(semantics)).toBe(SETTINGS_RPC_SEMANTICS) }) it('preserves render expressions and event handlers in tree order', () => { const tokens = readFlattenedMobileTasksRenderTokens() expect(tokens).toHaveLength(35_195) - expect(hash(tokens)).toBe(EXPECTED_RENDER_TREE) + expect(hash(tokens)).toBe(PRE_REFACTOR_RENDER_TREE) }) it('preserves every StyleSheet property and value', () => { - expect(hash(readMobileTasksStyleSource())).toBe(EXPECTED_STYLES) + expect(hash(readMobileTasksStyleSource())).toBe(PRE_REFACTOR_STYLES) }) }) diff --git a/mobile/src/tasks/use-mobile-tasks-runtime-hydration.tsx b/mobile/src/tasks/use-mobile-tasks-runtime-hydration.tsx index b1d8680b824..1ededc361bc 100644 --- a/mobile/src/tasks/use-mobile-tasks-runtime-hydration.tsx +++ b/mobile/src/tasks/use-mobile-tasks-runtime-hydration.tsx @@ -1,3 +1,4 @@ +import { settingsRead } from '../transport/settings-read-operations' import type { ClientSettingsActionsModel } from './use-mobile-tasks-client-settings-actions' import { MOBILE_TASKS_CAPABILITY, @@ -250,7 +251,7 @@ export function useMobileTasksRuntimeHydration(model: ClientSettingsActionsModel setError('') const [settingsResponse, uiResponse, preflightResponse, linearStatusResponse] = await Promise.all([ - client.sendRequest('settings.get'), + settingsRead.request(client), client.sendRequest('ui.get'), client.sendRequest('preflight.check'), client.sendRequest('linear.status') @@ -259,9 +260,10 @@ export function useMobileTasksRuntimeHydration(model: ClientSettingsActionsModel return } - const settings = isSuccess(settingsResponse) - ? (((settingsResponse.result as { settings?: RuntimeTaskSettings }).settings ?? - {}) as RuntimeTaskSettings) + const settingsResult = settingsRead.interpret(settingsResponse) + const settings = settingsResult.accepted + ? // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. + ((settingsResult.value ?? {}) as RuntimeTaskSettings) : {} setRuntimeTaskSettings(settings) const uiState = isSuccess(uiResponse) diff --git a/mobile/src/tasks/use-mobile-tasks-workspace-create-actions.tsx b/mobile/src/tasks/use-mobile-tasks-workspace-create-actions.tsx index 91a7da4d234..5befaddaebd 100644 --- a/mobile/src/tasks/use-mobile-tasks-workspace-create-actions.tsx +++ b/mobile/src/tasks/use-mobile-tasks-workspace-create-actions.tsx @@ -1,3 +1,4 @@ +import { settingsRead } from '../transport/settings-read-operations' import type { WorkspaceSshStateModel } from './use-mobile-tasks-workspace-ssh-state' import { WORKTREE_CREATE_TIMEOUT_MS, @@ -72,11 +73,11 @@ export function useMobileTasksWorkspaceCreateActions(model: WorkspaceSshStateMod await ensureWorkspaceSshReady(targetRepo) let latestRuntimeTaskSettings = runtimeTaskSettings try { - const settingsResponse = await client.sendRequest('settings.get') - if (isSuccess(settingsResponse)) { - latestRuntimeTaskSettings = (( - settingsResponse.result as { settings?: RuntimeTaskSettings } - ).settings ?? {}) as RuntimeTaskSettings + const settingsReply = await settingsRead.request(client) + const settingsResult = settingsRead.interpret(settingsReply) + if (settingsResult.accepted) { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. + latestRuntimeTaskSettings = (settingsResult.value ?? {}) as RuntimeTaskSettings setRuntimeTaskSettings(latestRuntimeTaskSettings) } } catch { diff --git a/mobile/src/transport/rpc-acceptance-policies.ts b/mobile/src/transport/rpc-acceptance-policies.ts index 742d94aafa7..89e72f45468 100644 --- a/mobile/src/transport/rpc-acceptance-policies.ts +++ b/mobile/src/transport/rpc-acceptance-policies.ts @@ -31,3 +31,18 @@ export function isStreamingOpenerReply( ): response is RpcSuccess & { streaming: true } { return response.ok && response.streaming === true } + +/** New-tab errors historically show the host message without its diagnostic code. */ +export function requireRpcResultOrThrowMessage(response: RpcResponse): unknown { + if (!response.ok) { + throw new Error(response.error.message) + } + return response.result +} + +/** An accepted null result still commits; a refused reply leaves existing state alone. */ +export function rpcSuccessResultOrSkip( + response: RpcResponse +): { accepted: false } | { accepted: true; value: unknown } { + return response.ok ? { accepted: true, value: response.result } : { accepted: false } +} diff --git a/mobile/src/transport/rpc-operation-compile-fence.ts b/mobile/src/transport/rpc-operation-compile-fence.ts index 3188774743c..ac14b326fd2 100644 --- a/mobile/src/transport/rpc-operation-compile-fence.ts +++ b/mobile/src/transport/rpc-operation-compile-fence.ts @@ -1,6 +1,12 @@ import type { RpcClient } from './rpc-client' import type { RpcMethodName, RpcParams, RpcSendParams } from './rpc-params-contract' -import { defineRpcOperation, runRpcOperation, startRpcOperation } from './rpc-operation' +import { + bindDeferredRpcOperation, + captureRpcOperationSettlement, + defineRpcOperation, + runRpcOperation, + startRpcOperation +} from './rpc-operation' import { rpcResultVariants } from './rpc-operation-result-reader' import { workspaceListAtBarrier, @@ -51,7 +57,7 @@ export const fenceDecodingWithoutReader: RequireResultRpcDefinition< barrier: 'on-settle' } -// @ts-expect-error the four policies in rpc-acceptance-policies.ts are the whole vocabulary +// @ts-expect-error only the named policies in rpc-acceptance-policies.ts are allowed export const fenceInventedPolicy: RpcAcceptanceName = 'no-error-means-fine' // A reader for a payload no acceptance policy here admits, i.e. one belonging to some other @@ -83,7 +89,6 @@ export const fenceDefineRejectsMismatch = defineRpcOperation({ // @ts-expect-error no overload of defineRpcOperation pairs a probe with a payload reader acceptance: 'method-not-found-refusal', barrier: 'on-settle', - // @ts-expect-error ... and the reader it would need is exactly what the probe overload bans read: workspaceRowsReader }) @@ -189,3 +194,30 @@ export const fenceObjectWithoutReader: RpcOperation< acceptance: 'object-result-or-null', barrier: 'on-settle' } + +const fenceDeferred = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'fence.deferred', + method: 'files.searchPaths', + acceptance: 'success-result-or-skip', + barrier: 'after-caller-barrier', + read: workspaceRowsReader + }) +) + +export function fenceDeferredArguments(): void { + // @ts-expect-error required params must not be omitted + fenceDeferred.request(client) + // @ts-expect-error request params are method-keyed, including deferred requests + fenceDeferred.request(client, { worktree: 3 }) + // @ts-expect-error single-flight operations also require their method's params + fenceDeferred.requestSingleFlight(client, 'host') + // @ts-expect-error on-settle operations cannot defer interpretation behind a caller guard + bindDeferredRpcOperation(workspaceListOrNull) + // @ts-expect-error caller-barrier operations cannot interpret as each request settles + runRpcOperation(client, fenceDeferred.operation, { worktree: 'w' }) + // @ts-expect-error a caller-barrier operation cannot acquire all-settled behavior implicitly + startRpcOperation(client, fenceDeferred.operation, { worktree: 'w' }) + // @ts-expect-error capturing must not decode a caller-barrier operation before its guard + captureRpcOperationSettlement(client, fenceDeferred.operation, { worktree: 'w' }) +} diff --git a/mobile/src/transport/rpc-operation-contract.ts b/mobile/src/transport/rpc-operation-contract.ts index 581506a97a7..23bc012319c 100644 --- a/mobile/src/transport/rpc-operation-contract.ts +++ b/mobile/src/transport/rpc-operation-contract.ts @@ -12,9 +12,11 @@ export type RpcAcceptanceName = | 'object-result-or-null' | 'method-not-found-refusal' | 'streaming-opener' + | 'success-result-or-skip' + | 'require-result-or-throw-message' /** Where a settled reply may become a value or a throw. */ -export type RpcInterpretationBarrier = 'on-settle' | 'after-all-requests' +export type RpcInterpretationBarrier = 'on-settle' | 'after-all-requests' | 'after-caller-barrier' export type RpcDecodeIssue = { readonly path: string; readonly message: string } @@ -76,7 +78,7 @@ export type RpcOperation< } & { [Policy in RpcAcceptanceName]: { readonly acceptance: Policy - readonly read: Policy extends 'require-result-or-throw' | 'object-result-or-null' + readonly read: Policy extends RpcReaderAcceptance ? RpcCompatibleReader : undefined } @@ -89,18 +91,19 @@ export type AnyRpcOperation = Pick< > & { readonly read: RpcCompatibleReader | undefined } /** The verdict the declared policy yields. Not a per-call choice. */ -export type RpcVerdict< - Acceptance extends RpcAcceptanceName, - Value -> = Acceptance extends 'require-result-or-throw' +export type RpcVerdict = Acceptance extends + | 'require-result-or-throw' + | 'require-result-or-throw-message' ? Value - : Acceptance extends 'object-result-or-null' - ? Value | null - : Acceptance extends 'method-not-found-refusal' - ? boolean - : Acceptance extends 'streaming-opener' - ? RpcStreamOpenerReply | null - : never + : Acceptance extends 'success-result-or-skip' + ? RpcAcceptedResult + : Acceptance extends 'object-result-or-null' + ? Value | null + : Acceptance extends 'method-not-found-refusal' + ? boolean + : Acceptance extends 'streaming-opener' + ? RpcStreamOpenerReply | null + : never export type RpcOperationSettlement = | { readonly status: 'fulfilled'; readonly outcome: RpcRequestOutcome } @@ -153,3 +156,25 @@ export type StreamOpenerRpcDefinition< /** The opener's value is the reply itself; frames arrive on the subscription, not here. */ read?: never } + +/** Refusal is distinct from an accepted null/undefined payload. */ +export type RpcAcceptedResult = + | { readonly accepted: false } + | { readonly accepted: true; readonly value: Value } + +export type RpcReaderAcceptance = + | 'require-result-or-throw' + | 'object-result-or-null' + | 'success-result-or-skip' + | 'require-result-or-throw-message' + +export type LegacyResultRpcDefinition< + Method extends RpcMethodName, + Acceptance extends 'success-result-or-skip' | 'require-result-or-throw-message', + Variant extends string, + Value, + Barrier extends RpcInterpretationBarrier +> = RpcOperationDefinition & { + acceptance: Acceptance + read: RpcCompatibleReader +} diff --git a/mobile/src/transport/rpc-operation-reply.ts b/mobile/src/transport/rpc-operation-reply.ts new file mode 100644 index 00000000000..881f2e82c59 --- /dev/null +++ b/mobile/src/transport/rpc-operation-reply.ts @@ -0,0 +1,92 @@ +import type { RpcResponse } from './types' +import type { + AnyRpcOperation, + RpcDecodeIssue, + RpcRequestOutcome, + RpcSalvageReport +} from './rpc-operation-contract' +import { + isStreamingOpenerReply, + requireRpcResultOrThrowCodedError, + rpcObjectResultOrNull +} from './rpc-acceptance-policies' + +const NOTHING_SALVAGED: RpcSalvageReport = { droppedPaths: [], droppedCount: 0 } + +type AdmittedPayload = + | { readonly admitted: true; readonly value: unknown } + | { readonly admitted: false; readonly issues: readonly RpcDecodeIssue[] } + +// The payload the operation's own acceptance policy admits from a fulfilled success. +function admitPayload(operation: AnyRpcOperation, response: RpcResponse): AdmittedPayload { + switch (operation.acceptance) { + case 'object-result-or-null': { + const object = rpcObjectResultOrNull(response) + return object === null + ? { admitted: false, issues: [{ path: 'result', message: 'not a non-null object' }] } + : { admitted: true, value: object } + } + case 'streaming-opener': + return isStreamingOpenerReply(response) + ? { admitted: true, value: response } + : { admitted: false, issues: [{ path: 'streaming', message: 'reply opened no stream' }] } + default: + // Reuses the policy rather than reading `.result` again; a success never throws here. + return { admitted: true, value: requireRpcResultOrThrowCodedError(response) } + } +} + +const READERLESS_VARIANTS: Record = { + 'method-not-found-refusal': 'accepted', + 'streaming-opener': 'stream-opened' +} + +export function classifyRpcReply( + operation: AnyRpcOperation, + response: RpcResponse +): RpcRequestOutcome { + if (!response.ok) { + return { kind: 'outer-refused', error: response.error, raw: response } + } + const payload = admitPayload(operation, response) + if (!payload.admitted) { + return { kind: 'incompatible', raw: response, issues: payload.issues } + } + const read = operation.read + if (!read) { + return { + kind: 'decoded', + variant: READERLESS_VARIANTS[operation.acceptance] ?? 'accepted', + value: payload.value, + raw: response, + salvage: NOTHING_SALVAGED + } + } + let result: ReturnType + try { + result = read(payload.value) + } catch (error) { + // These legacy policies preserve property-read exceptions at the caller's barrier. + if ( + operation.acceptance === 'success-result-or-skip' || + operation.acceptance === 'require-result-or-throw-message' + ) { + throw error + } + return { + kind: 'incompatible', + raw: response, + issues: [{ path: '', message: error instanceof Error ? error.message : String(error) }] + } + } + if (!result.compatible) { + return { kind: 'incompatible', raw: response, issues: result.issues } + } + return { + kind: 'decoded', + variant: result.variant, + value: result.value, + raw: response, + salvage: result.salvage + } +} diff --git a/mobile/src/transport/rpc-operation.ts b/mobile/src/transport/rpc-operation.ts index def072dac81..f4ee18d9dfe 100644 --- a/mobile/src/transport/rpc-operation.ts +++ b/mobile/src/transport/rpc-operation.ts @@ -1,33 +1,41 @@ -import type { UnvalidatedRpcRequestPort, SendRequestOptions } from './unvalidated-rpc-request-port' -import type { RpcMethodName, RpcSendParams } from './rpc-params-contract' -import type { RpcResponse } from './types' import { isMethodNotFoundRefusal, isStreamingOpenerReply, requireRpcResultOrThrowCodedError, - rpcObjectResultOrNull + requireRpcResultOrThrowMessage, + rpcSuccessResultOrSkip } from './rpc-acceptance-policies' import { RpcIncompatibleReplyError } from './rpc-incompatible-reply-error' +import type { UnvalidatedRpcRequestPort, SendRequestOptions } from './unvalidated-rpc-request-port' +import type { RpcMethodName, RpcSendParams } from './rpc-params-contract' +import { classifyRpcReply } from './rpc-operation-reply' +import { sendSingleFlightRequest } from './request-single-flight' +import type { RpcClient } from './rpc-client' +import type { RpcResponse } from './types' import type { AnyRpcOperation, CapabilityProbeRpcDefinition, ObjectResultRpcDefinition, RpcAcceptanceName, RpcCompatibleReader, - RpcDecodeIssue, RpcInterpretationBarrier, RpcOperation, RpcOperationSettlement, RpcRequestOutcome, - RpcSalvageReport, RequireResultRpcDefinition, StreamOpenerRpcDefinition, - RpcVerdict + RpcVerdict, + LegacyResultRpcDefinition } from './rpc-operation-contract' -const NOTHING_SALVAGED: RpcSalvageReport = { droppedPaths: [], droppedCount: 0 } - type RpcOperationDefinitionInput = + | LegacyResultRpcDefinition< + RpcMethodName, + 'success-result-or-skip' | 'require-result-or-throw-message', + string, + unknown, + RpcInterpretationBarrier + > | RequireResultRpcDefinition | ObjectResultRpcDefinition | CapabilityProbeRpcDefinition @@ -61,6 +69,15 @@ export function defineRpcOperation< >( definition: StreamOpenerRpcDefinition ): RpcOperation +export function defineRpcOperation< + Method extends RpcMethodName, + Acceptance extends 'success-result-or-skip' | 'require-result-or-throw-message', + Variant extends string, + Value, + Barrier extends RpcInterpretationBarrier +>( + definition: LegacyResultRpcDefinition +): RpcOperation export function defineRpcOperation(definition: RpcOperationDefinitionInput): AnyRpcOperation { // Why: frozen so no call site can swap the policy or the barrier on a shared descriptor. return Object.freeze({ @@ -68,7 +85,7 @@ export function defineRpcOperation(definition: RpcOperationDefinitionInput): Any method: definition.method, acceptance: definition.acceptance, barrier: definition.barrier, - // Why: classifyReply only ever hands a reader the payload its own policy admitted, so + // Why: classifyRpcReply only ever hands a reader the payload its own policy admitted, so // the object policy's narrower parameter is sound to store as unknown. read: definition.read as RpcCompatibleReader | undefined }) @@ -86,89 +103,37 @@ async function request( // and an always-settled send would make Promise.all wait for a peer where today the group // fails immediately, letting a later policy surface a different error. const response = await client.sendRequest(operation.method, params, options) - return classifyReply(operation, response) -} - -type AdmittedPayload = - | { readonly admitted: true; readonly value: unknown } - | { readonly admitted: false; readonly issues: readonly RpcDecodeIssue[] } - -// The payload the operation's own acceptance policy admits from a fulfilled success. -function admitPayload(operation: AnyRpcOperation, response: RpcResponse): AdmittedPayload { - switch (operation.acceptance) { - case 'object-result-or-null': { - const object = rpcObjectResultOrNull(response) - return object === null - ? { admitted: false, issues: [{ path: 'result', message: 'not a non-null object' }] } - : { admitted: true, value: object } - } - case 'streaming-opener': - return isStreamingOpenerReply(response) - ? { admitted: true, value: response } - : { admitted: false, issues: [{ path: 'streaming', message: 'reply opened no stream' }] } - default: - // Reuses the policy rather than reading `.result` again; a success never throws here. - return { admitted: true, value: requireRpcResultOrThrowCodedError(response) } - } -} - -const READERLESS_VARIANTS: Record = { - 'method-not-found-refusal': 'accepted', - 'streaming-opener': 'stream-opened' -} - -function classifyReply( - operation: AnyRpcOperation, - response: RpcResponse -): RpcRequestOutcome { - if (!response.ok) { - return { kind: 'outer-refused', error: response.error, raw: response } - } - const payload = admitPayload(operation, response) - if (!payload.admitted) { - return { kind: 'incompatible', raw: response, issues: payload.issues } - } - const read = operation.read - if (!read) { - return { - kind: 'decoded', - variant: READERLESS_VARIANTS[operation.acceptance] ?? 'accepted', - value: payload.value, - raw: response, - salvage: NOTHING_SALVAGED - } - } - let result: ReturnType - try { - result = read(payload.value) - } catch (error) { - // A reader that throws is an incompatible reply, never a transport failure. - return { - kind: 'incompatible', - raw: response, - issues: [{ path: '', message: error instanceof Error ? error.message : String(error) }] - } - } - if (!result.compatible) { - return { kind: 'incompatible', raw: response, issues: result.issues } - } - return { - kind: 'decoded', - variant: result.variant, - value: result.value, - raw: response, - salvage: result.salvage - } + return classifyRpcReply(operation, response) } // Applies the operation's declared acceptance policy. Private on purpose: there is no // free-standing callOrThrow, so no call site can pick a different rule for the same reply. -function interpret( +function interpretRpcOutcome( operation: AnyRpcOperation, settled: RpcRequestOutcome ): unknown { const acceptance: RpcAcceptanceName = operation.acceptance switch (acceptance) { + case 'success-result-or-skip': { + const accepted = rpcSuccessResultOrSkip(settled.raw) + if (!accepted.accepted) { + return accepted + } + if (settled.kind === 'incompatible') { + throw new RpcIncompatibleReplyError(operation.name, operation.method, settled.issues) + } + return settled.kind === 'decoded' + ? { accepted: true, value: settled.value } + : { accepted: false } + } + case 'require-result-or-throw-message': + if (settled.kind === 'outer-refused') { + return requireRpcResultOrThrowMessage(settled.raw) + } + if (settled.kind === 'incompatible') { + throw new RpcIncompatibleReplyError(operation.name, operation.method, settled.issues) + } + return settled.value case 'require-result-or-throw': if (settled.kind === 'outer-refused') { // Reuses the policy so the thrown `code: message` text cannot drift from main's. @@ -196,7 +161,7 @@ function interpretSettlement( // isLogicalClientCutoverError matches class or exact message; a wrapper loses both. throw settlement.error } - return interpret(operation, settlement.outcome) + return interpretRpcOutcome(operation, settlement.outcome) } /** Sends and interprets at the operation's own barrier. Only for barrier 'on-settle'. */ @@ -212,7 +177,8 @@ export async function runRpcOperation< options?: SendRequestOptions ): Promise> { const outcome = await request(client, operation, params, options) - return interpret(operation, outcome) as RpcVerdict + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. + return interpretRpcOutcome(operation, outcome) as RpcVerdict } /** The named opt-in to all-settled semantics. Yields an outcome, never a verdict: the @@ -222,7 +188,7 @@ export async function captureRpcOperationSettlement< Acceptance extends RpcAcceptanceName, Variant extends string, Value, - Barrier extends RpcInterpretationBarrier + Barrier extends Exclude >( client: UnvalidatedRpcRequestPort, operation: RpcOperation, @@ -280,3 +246,38 @@ export async function interpretAtRpcBarrier< interpretSettlement(entry.operation, settlements[index]) ) as RpcBarrierVerdicts } + +/** Preserves omitted sender arguments as well as explicit undefined. */ +type RpcSendArguments = + void extends RpcSendParams + ? [params?: RpcSendParams, options?: SendRequestOptions] + : [params: RpcSendParams, options?: SendRequestOptions] + +/** Binds sending and interpretation while preserving the transport promise identity. */ +export function bindDeferredRpcOperation< + Method extends RpcMethodName, + Acceptance extends RpcAcceptanceName, + Variant extends string, + Value +>(operation: RpcOperation) { + type Verdict = RpcVerdict + return Object.freeze({ + operation, + request(client: UnvalidatedRpcRequestPort, ...args: RpcSendArguments) { + return client.sendRequest(operation.method, ...args) + }, + requestSingleFlight( + client: RpcClient, + hostId: string, + ...args: void extends RpcSendParams + ? [params?: RpcSendParams] + : [params: RpcSendParams] + ) { + return sendSingleFlightRequest(client, hostId, operation.method, args[0]) + }, + interpret(response: RpcResponse): Verdict { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Preserve the established response shape at this boundary. + return interpretRpcOutcome(operation, classifyRpcReply(operation, response)) as Verdict + } + }) +} diff --git a/mobile/src/transport/settings-read-operations.test.ts b/mobile/src/transport/settings-read-operations.test.ts new file mode 100644 index 00000000000..3a81da9942f --- /dev/null +++ b/mobile/src/transport/settings-read-operations.test.ts @@ -0,0 +1,200 @@ +import { describe, expect, it, vi } from 'vitest' +import { loadMobileNewTabAgentOptions } from '../session/mobile-new-tab-agent-loader' +import { FLOATING_WORKSPACE_WORKTREE_ID } from '../session/floating-workspace' +import { FakeSession } from './mobile-endpoint-supervisor-test-fakes' +import { markRpcDeliveryUnknown, isRpcDeliveryUnknown } from './rpc-delivery-ambiguity' +import { LogicalClientCutoverError, isLogicalClientCutoverError } from './stable-logical-rpc-client' +import { + settingsRead, + optionalSettingsRead, + botOverridesRead, + newTabSettingsRead +} from './settings-read-operations' +import type { RpcResponse } from './types' + +function success(result: unknown): RpcResponse { + return { id: 'reply', ok: true, result, _meta: { runtimeId: 'runtime' } } +} + +function refusal(message = 'settings refused'): RpcResponse { + return { + id: 'reply', + ok: false, + error: { code: 'runtime_error', message }, + _meta: { runtimeId: 'runtime' } + } +} + +function deferred() { + let resolve!: (response: RpcResponse) => void + let reject!: (error: unknown) => void + const promise = new Promise((done, fail) => { + resolve = done + reject = fail + }) + return { promise, resolve, reject } +} + +function replyWith(response: RpcResponse) { + const client = new FakeSession('connected') + client.sendRequest.mockResolvedValue(response) + return client +} + +async function drain() { + for (let turn = 0; turn < 20; turn++) { + await Promise.resolve() + } +} + +describe('settings historical acceptance', () => { + it('distinguishes a skipped refusal from an accepted absent settings member', async () => { + const skipped = await settingsRead.request(replyWith(refusal())) + const missing = await settingsRead.request(replyWith(success({}))) + expect(settingsRead.interpret(skipped)).toEqual({ accepted: false }) + expect(settingsRead.interpret(missing)).toEqual({ accepted: true, value: undefined }) + }) + + it('retains opaque settings fields and reference identity without tightening acceptance', async () => { + const value = { futureField: { nested: ['kept'] }, disabledTuiAgents: 'legacy-value' } + const reply = await settingsRead.request(replyWith(success({ settings: value }))) + const result = settingsRead.interpret(reply) + expect(result.accepted && result.value).toBe(value) + }) + + it.each([null, undefined])( + 'preserves the unguarded settings read for %s only when interpreted', + async (value) => { + const reply = await settingsRead.request(replyWith(success(value))) + expect(() => settingsRead.interpret(reply)).toThrow(TypeError) + expect(() => settingsRead.interpret(reply)).toThrow( + `Cannot read properties of ${String(value)} (reading 'settings')` + ) + const optional = await optionalSettingsRead.request(replyWith(success(value))) + expect(optionalSettingsRead.interpret(optional)).toEqual({ accepted: true, value: undefined }) + } + ) + + it.each([true, false, 0, 'text', []])('preserves property boxing for %j', async (value) => { + const reply = await settingsRead.request(replyWith(success(value))) + expect(settingsRead.interpret(reply)).toEqual({ accepted: true, value: undefined }) + }) + + it('filters bot logins while distinguishing a refused refresh', async () => { + const reply = await botOverridesRead.request( + replyWith(success({ settings: { prBotAuthorOverrides: ['bot', 3, null, ''] } })) + ) + expect(botOverridesRead.interpret(reply)).toEqual({ accepted: true, value: ['bot', ''] }) + const refused = await botOverridesRead.request(replyWith(refusal())) + expect(botOverridesRead.interpret(refused)).toEqual({ accepted: false }) + const empty = await botOverridesRead.request(replyWith(success(null))) + expect(botOverridesRead.interpret(empty)).toEqual({ accepted: true, value: [] }) + }) + + it('does not read a stale payload until its caller permits interpretation', async () => { + const read = vi.fn(() => ({})) + const reply = await settingsRead.request( + replyWith( + success({ + get settings() { + return read() + } + }) + ) + ) + expect(read).not.toHaveBeenCalled() + settingsRead.interpret(reply) + expect(read).toHaveBeenCalledOnce() + }) + + it('keeps sender argument presence and options intact', async () => { + const client = replyWith(success({ settings: {} })) + await settingsRead.request(client) + await settingsRead.request(client, undefined) + const options = { timeoutMs: 1234, failWhenDisconnected: true } + await settingsRead.request(client, undefined, options) + expect(client.sendRequest.mock.calls).toEqual([ + ['settings.get'], + ['settings.get', undefined], + ['settings.get', undefined, options] + ]) + }) + + it('keeps original delivery-unknown and cutover errors on the rejection channel', async () => { + for (const error of [ + markRpcDeliveryUnknown(new Error('ambiguous')), + new LogicalClientCutoverError() + ]) { + const client = new FakeSession('connected') + client.sendRequest.mockRejectedValue(error) + const caught = await settingsRead.request(client).catch((value: unknown) => value) + expect(caught).toBe(error) + expect(isRpcDeliveryUnknown(caught)).toBe(isRpcDeliveryUnknown(error)) + expect(isLogicalClientCutoverError(caught)).toBe(isLogicalClientCutoverError(error)) + } + }) +}) + +describe('new-tab settlement barriers', () => { + function load(client: FakeSession) { + return loadMobileNewTabAgentOptions({ client, worktreeId: FLOATING_WORKSPACE_WORKTREE_ID }) + } + + it('waits for the peer after a settings refusal and reports only the host message', async () => { + const peer = deferred() + const client = new FakeSession('connected') + client.sendRequest.mockImplementation((method) => + method === 'settings.get' ? Promise.resolve(refusal()) : peer.promise + ) + let settled = false + const outcome = load(client).catch((error: unknown) => { + settled = true + return error + }) + await drain() + expect(settled).toBe(false) + peer.resolve(success([])) + expect(await outcome).toEqual(new Error('settings refused')) + }) + + it('lets a peer transport failure win over a fulfilled settings refusal', async () => { + const peer = deferred() + const client = new FakeSession('connected') + client.sendRequest.mockImplementation((method) => + method === 'settings.get' ? Promise.resolve(refusal()) : peer.promise + ) + const outcome = load(client).catch((error: unknown) => error) + await drain() + const error = markRpcDeliveryUnknown(new Error('agents disconnected')) + peer.reject(error) + expect(await outcome).toBe(error) + }) + + it('rejects immediately on settings transport failure while its peer stays pending', async () => { + const peer = deferred() + const client = new FakeSession('connected') + const error = new Error('settings disconnected') + client.sendRequest.mockImplementation((method) => + method === 'settings.get' ? Promise.reject(error) : peer.promise + ) + let caught: unknown + const outcome = load(client).catch((value: unknown) => { + caught = value + }) + await drain() + expect(caught).toBe(error) + peer.resolve(success([])) + await outcome + }) + + it('checks the peer refusal before accessing a null settings payload', async () => { + const client = new FakeSession('connected') + client.sendRequest.mockImplementation(async (method) => + method === 'settings.get' ? success(null) : refusal('agents refused') + ) + await expect(load(client)).rejects.toThrow('agents refused') + const reply = await newTabSettingsRead.request(replyWith(success(null))) + const readSettings = newTabSettingsRead.interpret(reply) + expect(() => readSettings()).toThrow(TypeError) + }) +}) diff --git a/mobile/src/transport/settings-read-operations.ts b/mobile/src/transport/settings-read-operations.ts new file mode 100644 index 00000000000..f33471dcaf2 --- /dev/null +++ b/mobile/src/transport/settings-read-operations.ts @@ -0,0 +1,93 @@ +import { bindDeferredRpcOperation, defineRpcOperation } from './rpc-operation' +import type { RpcCompatibleReader } from './rpc-operation-contract' + +function settingsMember(raw: unknown): unknown { + const boxed: { readonly settings?: unknown } | null | undefined = raw == null ? raw : Object(raw) + // Preserve the native engine's existing property-read exception on null/undefined. + return boxed!.settings +} + +// Settings remain opaque: callers historically retain fields without validating their shapes. +const settingsReader: RpcCompatibleReader = (raw) => ({ + compatible: true, + variant: 'settings-member', + value: settingsMember(raw), + salvage: { droppedPaths: [], droppedCount: 0 } +}) + +const optionalSettingsReader: RpcCompatibleReader = ( + raw +) => ({ + compatible: true, + variant: 'optional-settings-member', + value: raw == null ? undefined : settingsMember(raw), + salvage: { droppedPaths: [], droppedCount: 0 } +}) + +const botOverridesReader: RpcCompatibleReader = (raw) => { + const settings = raw == null ? undefined : settingsMember(raw) + const overrides: unknown = + settings == null ? undefined : Reflect.get(Object(settings), 'prBotAuthorOverrides') + return { + compatible: true, + variant: 'bot-logins', + value: Array.isArray(overrides) + ? overrides.filter((login): login is string => typeof login === 'string') + : [], + salvage: { droppedPaths: [], droppedCount: 0 } + } +} + +/** Workspace context, submit, task hydration/create and home providers share this acceptance. */ +export const settingsRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'settings.member-or-skip', + method: 'settings.get', + acceptance: 'success-result-or-skip', + barrier: 'after-caller-barrier', + read: settingsReader + }) +) + +/** History resume and repo labels historically tolerate an absent or null result. */ +export const optionalSettingsRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'settings.optional-member-or-skip', + method: 'settings.get', + acceptance: 'success-result-or-skip', + barrier: 'after-caller-barrier', + read: optionalSettingsReader + }) +) + +// New-tab checks the sibling's refusal before touching settings, but after its own refusal. +const newTabSettingsReader: RpcCompatibleReader< + unknown, + 'deferred-settings-member', + () => unknown +> = (raw) => ({ + compatible: true, + variant: 'deferred-settings-member', + value: () => settingsMember(raw), + salvage: { droppedPaths: [], droppedCount: 0 } +}) + +export const newTabSettingsRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'settings.new-tab-message-error', + method: 'settings.get', + acceptance: 'require-result-or-throw-message', + barrier: 'after-caller-barrier', + read: newTabSettingsReader + }) +) + +export const botOverridesRead = bindDeferredRpcOperation( + defineRpcOperation({ + name: 'settings.bot-logins-or-skip', + method: 'settings.get', + acceptance: 'success-result-or-skip', + barrier: 'after-caller-barrier', + read: botOverridesReader + }) +) diff --git a/mobile/src/transport/unvalidated-rpc-request-port-inventory.ts b/mobile/src/transport/unvalidated-rpc-request-port-inventory.ts index 82317f272fc..dba48bbf3c8 100644 --- a/mobile/src/transport/unvalidated-rpc-request-port-inventory.ts +++ b/mobile/src/transport/unvalidated-rpc-request-port-inventory.ts @@ -35,7 +35,7 @@ export const UNVALIDATED_RPC_REQUEST_PORT_OWNERS: readonly UnvalidatedRpcRequest // Composes the port into RpcClient, which is why every holder of a client still carries it. { file: 'src/transport/rpc-client.ts', references: 2 }, // The typed boundary itself — the one module that turns a reply into a declared type. - { file: 'src/transport/rpc-operation.ts', references: 2 }, + { file: 'src/transport/rpc-operation.ts', references: 5 }, // Forwards the port across a physical-client cutover. { file: 'src/transport/stable-logical-rpc-client.ts', references: 2 } ] @@ -49,7 +49,7 @@ export const UNVALIDATED_RPC_REQUEST_PORT_PENDING: readonly UnvalidatedRpcReques { file: 'app/terminal-settings.tsx', references: 3 }, // src/agent-history/ — agent history loads - { file: 'src/agent-history/MobileAgentSessionHistoryPanel.tsx', references: 7 }, + { file: 'src/agent-history/MobileAgentSessionHistoryPanel.tsx', references: 6 }, { file: 'src/agent-history/use-mobile-agent-history-state.ts', references: 2 }, // src/browser/ — hosted browser control @@ -59,10 +59,9 @@ export const UNVALIDATED_RPC_REQUEST_PORT_PENDING: readonly UnvalidatedRpcReques // src/components/ — shared widgets that fetch their own data { file: 'src/components/codex-reset-credit-capability.ts', references: 2 }, { file: 'src/components/codex-reset-credit.ts', references: 3 }, - { file: 'src/components/use-new-workspace-create-submit.ts', references: 1 }, { file: 'src/components/use-new-workspace-execution-target.ts', references: 4 }, { file: 'src/components/use-new-workspace-repositories.ts', references: 1 }, - { file: 'src/components/use-new-workspace-runtime-context.ts', references: 4 }, + { file: 'src/components/use-new-workspace-runtime-context.ts', references: 3 }, { file: 'src/components/use-new-workspace-setup-script.ts', references: 1 }, // src/dictation/ — dictation session control @@ -76,7 +75,7 @@ export const UNVALIDATED_RPC_REQUEST_PORT_PENDING: readonly UnvalidatedRpcReques { file: 'src/files/MobileFileExplorerPanel.tsx', references: 2 }, // src/home/ — home screen host reads - { file: 'src/home/mobile-home-host-requests.ts', references: 6 }, + { file: 'src/home/mobile-home-host-requests.ts', references: 5 }, // src/hooks/ — cross-screen data hooks { file: 'src/hooks/mobile-dictation-audio-chunk.ts', references: 1 }, @@ -85,7 +84,7 @@ export const UNVALIDATED_RPC_REQUEST_PORT_PENDING: readonly UnvalidatedRpcReques // src/host-screen/ — host screen catalog and actions { file: 'src/host-screen/host-screen-overlays.tsx', references: 1 }, - { file: 'src/host-screen/use-host-repo-metadata.ts', references: 2 }, + { file: 'src/host-screen/use-host-repo-metadata.ts', references: 1 }, { file: 'src/host-screen/use-host-view-settings.ts', references: 2 }, { file: 'src/host-screen/use-host-worktree-actions.ts', references: 3 }, @@ -108,7 +107,7 @@ export const UNVALIDATED_RPC_REQUEST_PORT_PENDING: readonly UnvalidatedRpcReques { file: 'src/session/mobile-native-chat-send.ts', references: 2 }, { file: 'src/session/mobile-native-chat-session-option-persistence.ts', references: 1 }, { file: 'src/session/mobile-native-chat-stale-input.ts', references: 1 }, - { file: 'src/session/mobile-new-tab-agent-loader.ts', references: 5 }, + { file: 'src/session/mobile-new-tab-agent-loader.ts', references: 4 }, { file: 'src/session/mobile-session-tab-activation.ts', references: 3 }, { file: 'src/session/mobile-session-tabs-stream-health.ts', references: 1 }, { file: 'src/session/mobile-structured-agent-session-launch.ts', references: 3 }, @@ -141,7 +140,6 @@ export const UNVALIDATED_RPC_REQUEST_PORT_PENDING: readonly UnvalidatedRpcReques { file: 'src/session/use-mobile-session-terminal-send-actions.ts', references: 2 }, { file: 'src/session/use-mobile-session-terminal-stream-display.ts', references: 1 }, { file: 'src/session/use-mobile-terminal-paste.ts', references: 1 }, - { file: 'src/session/use-pr-bot-author-overrides.ts', references: 1 }, { file: 'src/session/use-quick-commands.ts', references: 2 }, // src/settings/ — settings screen actions @@ -194,11 +192,11 @@ export const UNVALIDATED_RPC_REQUEST_PORT_PENDING: readonly UnvalidatedRpcReques { file: 'src/tasks/use-mobile-tasks-project-workspace-comment-actions.tsx', references: 3 }, { file: 'src/tasks/use-mobile-tasks-provider-load-actions.tsx', references: 5 }, { file: 'src/tasks/use-mobile-tasks-route-and-item-state.tsx', references: 1 }, - { file: 'src/tasks/use-mobile-tasks-runtime-hydration.tsx', references: 5 }, + { file: 'src/tasks/use-mobile-tasks-runtime-hydration.tsx', references: 4 }, { file: 'src/tasks/use-mobile-tasks-task-create-actions.tsx', references: 3 }, { file: 'src/tasks/use-mobile-tasks-task-list-loading.tsx', references: 4 }, { file: 'src/tasks/use-mobile-tasks-task-pagination-actions.tsx', references: 1 }, - { file: 'src/tasks/use-mobile-tasks-workspace-create-actions.tsx', references: 4 }, + { file: 'src/tasks/use-mobile-tasks-workspace-create-actions.tsx', references: 3 }, { file: 'src/tasks/use-mobile-tasks-workspace-source-effects.tsx', references: 2 }, { file: 'src/tasks/use-mobile-tasks-workspace-sparse-actions.tsx', references: 2 }, { file: 'src/tasks/use-mobile-tasks-workspace-ssh-state.tsx', references: 5 },