diff --git a/mobile/app/h/[hostId]/edit.tsx b/mobile/app/h/[hostId]/edit.tsx index 447a2ab2a3d..af15f28ad20 100644 --- a/mobile/app/h/[hostId]/edit.tsx +++ b/mobile/app/h/[hostId]/edit.tsx @@ -51,7 +51,8 @@ export default function EditHostScreen() { return } setHost(found) - setName(found.name) + // The field edits the phone's override; an empty field means "use the desktop's name". + setName(found.personalName ?? '') setAddress(displayHostEndpoint(found.endpoint)) setLoadError(null) } catch (err) { @@ -70,12 +71,11 @@ export default function EditHostScreen() { ) const nameTrimmed = name.trim() - const nameChanged = host != null && nameTrimmed.length > 0 && nameTrimmed !== host.name + const nameChanged = host != null && nameTrimmed !== (host.personalName ?? '') const endpointChanged = endpointEdit?.kind === 'changed' const canSave = host != null && endpointEdit != null && - nameTrimmed.length > 0 && endpointEdit.kind !== 'invalid' && (nameChanged || endpointChanged) && !saving @@ -85,16 +85,12 @@ export default function EditHostScreen() { return } const nextName = name.trim() - if (!nextName) { - setSaveError('Enter a name.') - return - } if (endpointEdit.kind === 'invalid') { setSaveError(endpointEdit.error) return } - const willRename = nextName !== host.name + const willRename = nextName !== (host.personalName ?? '') const nextEndpoint = endpointEdit.kind === 'changed' ? endpointEdit.endpoint : undefined if (!willRename && nextEndpoint === undefined) { router.back() @@ -109,7 +105,7 @@ export default function EditHostScreen() { // atomically — a mid-save failure can never persist one without the // other, and a host removed mid-edit throws instead of no-oping. await updateHostNameAndEndpoint(host.id, { - ...(willRename ? { name: nextName } : {}), + ...(willRename ? { personalName: nextName || null } : {}), ...(nextEndpoint !== undefined ? { endpoint: nextEndpoint } : {}) }) } catch (err) { @@ -192,9 +188,10 @@ export default function EditHostScreen() { keyboardShouldPersistTaps="handled" > - Change the display name or connection address. Address edits only switch where this - phone connects — they do not re-pair. Use this when the same desktop is reachable at a - different IP (for example home LAN vs Tailscale). + Change the display name or connection address. Leave the name empty to use the name + the desktop reports. Address edits only switch where this phone connects — they do not + re-pair. Use this when the same desktop is reachable at a different IP (for example + home LAN vs Tailscale). Name @@ -206,7 +203,7 @@ export default function EditHostScreen() { setName(value) setSaveError(null) }} - placeholder="Host name" + placeholder={host.lastKnownMachineName ?? 'Host name'} placeholderTextColor={colors.textMuted} autoCapitalize="words" autoCorrect={false} diff --git a/mobile/src/components/HostProtocolGate.test.ts b/mobile/src/components/HostProtocolGate.test.ts index a7be7b6e5a4..d3a94f3a7df 100644 --- a/mobile/src/components/HostProtocolGate.test.ts +++ b/mobile/src/components/HostProtocolGate.test.ts @@ -37,6 +37,10 @@ const hostClient = vi.hoisted(() => ({ vi.mock('../transport/client-context', () => ({ useHostClient: () => hostClient.current })) +// Descriptor bookkeeping only; the real recorder reaches the native keychain through host-store. +vi.mock('../transport/host-descriptor-recorder', () => ({ + recordHostDescriptorFromStatus: vi.fn() +})) function clientWithStatus(result: Record): RpcClient { return { sendRequest: vi.fn().mockResolvedValue({ ok: true, result }) } as unknown as RpcClient diff --git a/mobile/src/components/MobileHostCard.test.ts b/mobile/src/components/MobileHostCard.test.ts index be692835d65..e29ab50bfab 100644 --- a/mobile/src/components/MobileHostCard.test.ts +++ b/mobile/src/components/MobileHostCard.test.ts @@ -2,6 +2,10 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' import { afterEach, describe, expect, it, vi } from 'vitest' import { MobileHostCard } from './MobileHostCard' +import { + recordHostDescriptor, + resetHostDescriptorStoreForTests +} from '../transport/host-descriptor-store' vi.mock('react-native', () => ({ Pressable: 'Pressable', @@ -33,6 +37,7 @@ describe('MobileHostCard', () => { afterEach(() => { act(() => renderer?.unmount()) renderer = null + resetHostDescriptorStoreForTests() vi.restoreAllMocks() }) @@ -121,6 +126,134 @@ describe('MobileHostCard', () => { ) }) + it('keeps a phone rename above a disagreeing machine descriptor', async () => { + const consoleError = suppressRendererDeprecation() + recordHostDescriptor('desk', { machineName: 'm4airs-Air', platform: 'darwin' }) + await act(async () => { + renderer = create( + createElement(MobileHostCard, { + host: { + id: 'desk', + name: 'Windows-Low Spec', + personalName: 'Windows-Low Spec', + endpoint: 'ws://192.168.1.2:6768', + deviceToken: 'token', + publicKeyB64: 'key', + lastConnected: 1 + }, + state: 'connected', + verdict: { kind: 'normal', label: 'Connected' }, + path: 'lan', + onPress: vi.fn(), + onLongPress: vi.fn(), + onOpenActions: vi.fn() + }) + ) + }) + consoleError.mockRestore() + + const texts = renderer.root.findAllByType('Text').map((node) => node.children.join('')) + expect(texts).toContain('Windows-Low Spec') + expect(texts).toContain('macOS · m4airs-Air') + }) + + it('titles an unrenamed host with the live machine name before its stored name catches up', async () => { + // The desktop was renamed; the stored `name` still holds the old machine name until reload. + const consoleError = suppressRendererDeprecation() + recordHostDescriptor('desk', { machineName: 'Studio 2', platform: 'darwin' }) + await act(async () => { + renderer = create( + createElement(MobileHostCard, { + host: { + id: 'desk', + name: 'Studio', + lastKnownMachineName: 'Studio', + lastKnownHostPlatform: 'darwin', + endpoint: 'ws://192.168.1.2:6768', + deviceToken: 'token', + publicKeyB64: 'key', + lastConnected: 1 + }, + state: 'connected', + verdict: { kind: 'normal', label: 'Connected' }, + path: 'lan', + onPress: vi.fn(), + onLongPress: vi.fn(), + onOpenActions: vi.fn() + }) + ) + }) + consoleError.mockRestore() + + const texts = renderer.root.findAllByType('Text').map((node) => node.children.join('')) + expect(texts).toContain('Studio 2') + expect(texts).toContain('macOS') + expect(texts).not.toContain('Studio') + }) + + it('shows the stored descriptor when the host is offline, as after a restart', async () => { + // No live descriptor is recorded: the row has only what the stored profile carries. + const consoleError = suppressRendererDeprecation() + await act(async () => { + renderer = create( + createElement(MobileHostCard, { + host: { + id: 'desk', + name: 'Desk', + personalName: 'Desk', + lastKnownMachineName: 'm4airs-Air', + lastKnownHostPlatform: 'darwin', + endpoint: 'ws://192.168.1.2:6768', + deviceToken: 'token', + publicKeyB64: 'key', + lastConnected: 1 + }, + state: 'disconnected', + verdict: { kind: 'normal', label: 'Disconnected' }, + path: 'lan', + onPress: vi.fn(), + onLongPress: vi.fn(), + onOpenActions: vi.fn() + }) + ) + }) + consoleError.mockRestore() + + expect(renderer.root.findAllByType('Text').map((node) => node.children.join(''))).toContain( + 'macOS · m4airs-Air' + ) + }) + + it('collapses the machine name into the OS line when it is the shown name', async () => { + const consoleError = suppressRendererDeprecation() + recordHostDescriptor('desk', { machineName: 'Desk', platform: 'darwin' }) + await act(async () => { + renderer = create( + createElement(MobileHostCard, { + host: { + id: 'desk', + name: 'Desk', + endpoint: 'ws://192.168.1.2:6768', + deviceToken: 'token', + publicKeyB64: 'key', + lastConnected: 1 + }, + state: 'connected', + verdict: { kind: 'normal', label: 'Connected' }, + path: 'lan', + onPress: vi.fn(), + onLongPress: vi.fn(), + onOpenActions: vi.fn() + }) + ) + }) + consoleError.mockRestore() + + const texts = renderer.root.findAllByType('Text').map((node) => node.children.join('')) + expect(texts).toContain('macOS') + expect(texts).not.toContain('macOS · Desk') + }) + it('preserves the connected worktree-catalog failure state', async () => { const consoleError = suppressRendererDeprecation() await act(async () => { diff --git a/mobile/src/components/MobileHostCard.tsx b/mobile/src/components/MobileHostCard.tsx index 938ab9fa6ee..cadaf186fb2 100644 --- a/mobile/src/components/MobileHostCard.tsx +++ b/mobile/src/components/MobileHostCard.tsx @@ -3,6 +3,7 @@ import { Pressable, StyleSheet, Text, View } from 'react-native' import type { ConnectionVerdict } from '../transport/connection-health' import { verdictDisplayLabel } from '../transport/connection-health' import { mobileConnectionPathLabel } from '../transport/mobile-connection-path-label' +import { useHostDisplay } from '../transport/use-host-display' import type { MobileConnectionPath } from '../transport/stable-logical-rpc-client' import type { ConnectionState, HostCatalogEntry, HostProfile } from '../transport/types' import { colors, radii, spacing } from '../theme/mobile-theme' @@ -38,6 +39,8 @@ export function MobileHostCard(props: { ? { kind: 'warning', label: statusLabel } : props.verdict const worktreeSummary = homeHostWorktreeSummary(props.worktreeInfo) + const display = useHostDisplay(props.host) + const descriptorText = display.descriptorLine const connectionPathLabel = !credentialMissing && !credentialUnavailable && connected ? mobileConnectionPathLabel(props.path) @@ -55,7 +58,8 @@ export function MobileHostCard(props: { const verdictDetail = credentialHint === null && 'detail' in props.verdict ? (props.verdict.detail ?? null) : null const accessibilityLabel = [ - `Open ${props.host.name}`, + `Open ${display.title}`, + descriptorText, statusLabel, connectionPathLabel?.replace(' · ', ' via '), connected ? worktreeSummary?.replace(' · ', ', ') : null, @@ -83,8 +87,13 @@ export function MobileHostCard(props: { style={[styles.name, !connected && { color: colors.textSecondary }]} numberOfLines={1} > - {props.host.name} + {display.title} + {descriptorText ? ( + + {descriptorText} + + ) : null} [styles.actionButton, pressed && styles.actionButtonPressed]} onPress={props.onOpenActions} @@ -164,6 +173,7 @@ const styles = StyleSheet.create({ }, main: { flex: 1, minWidth: 0, marginRight: spacing.sm }, name: { color: colors.textPrimary, fontSize: 15, fontWeight: '600', lineHeight: 20 }, + platformText: { color: colors.textSecondary, fontSize: 12, lineHeight: 16 }, meta: { flexDirection: 'row', alignItems: 'center', gap: 6, marginTop: 3, minWidth: 0 }, metaText: { flex: 1, fontSize: 12, color: colors.textSecondary }, worktreeMetaText: { diff --git a/mobile/src/diagnostics/use-mobile-web-bundle-probe.test.tsx b/mobile/src/diagnostics/use-mobile-web-bundle-probe.test.tsx index 8113e09356e..c7878e65ea5 100644 --- a/mobile/src/diagnostics/use-mobile-web-bundle-probe.test.tsx +++ b/mobile/src/diagnostics/use-mobile-web-bundle-probe.test.tsx @@ -18,6 +18,8 @@ vi.mock('../transport/host-logical-client', () => ({ openHostLogicalClient: (...args: unknown[]) => connectMock(...args) })) vi.mock('../transport/host-store', () => ({ loadHosts: () => loadHostsMock() })) +// Why: the opener starts a descriptor status probe per connection; these fakes have no RPC surface. +vi.mock('../transport/runtime-status-probe', () => ({ startRuntimeStatusProbe: () => () => {} })) vi.mock('../transport/connection-revival-triggers', () => ({ subscribeConnectionRevivalTriggers: () => () => {} })) diff --git a/mobile/src/host-edit-save-flow.test.ts b/mobile/src/host-edit-save-flow.test.ts index a0a4561b2b2..835af6330f3 100644 --- a/mobile/src/host-edit-save-flow.test.ts +++ b/mobile/src/host-edit-save-flow.test.ts @@ -141,7 +141,7 @@ describe('edit host handleSave', () => { await pressSave(renderer) expect(dependencies.updateHostNameAndEndpoint).toHaveBeenCalledWith('host-1', { - name: 'Home Desk' + personalName: 'Home Desk' }) expect(dependencies.forceReconnectHost).not.toHaveBeenCalled() expect(dependencies.back).toHaveBeenCalledTimes(1) @@ -171,7 +171,7 @@ describe('edit host handleSave', () => { expect(dependencies.updateHostNameAndEndpoint).toHaveBeenCalledTimes(1) expect(dependencies.updateHostNameAndEndpoint).toHaveBeenCalledWith('host-1', { - name: 'Home Desk', + personalName: 'Home Desk', endpoint: 'ws://192.168.1.20:6768' }) expect(dependencies.forceReconnectHost).toHaveBeenCalledWith('host-1') diff --git a/mobile/src/host-screen/host-screen-header-reconnect.test.tsx b/mobile/src/host-screen/host-screen-header-reconnect.test.tsx index 8c13e6b2145..54274baf90f 100644 --- a/mobile/src/host-screen/host-screen-header-reconnect.test.tsx +++ b/mobile/src/host-screen/host-screen-header-reconnect.test.tsx @@ -23,6 +23,7 @@ vi.mock('lucide-react-native', () => ({ X: 'Icon' })) +import { resolveHostDisplay } from '../../../src/shared/host-display-resolution' import { HostScreenHeader } from './host-screen-header' /** A host the shell's client has failed to reach twenty times, which is the verdict that offers @@ -39,6 +40,7 @@ function controllerWith(forceReconnectHost: ForceReconnect) { floatingWorkspaceEnabled: false, forceReconnectHost, hostId: 'host-a', + hostDisplay: resolveHostDisplay({ name: 'Desk' }), lastConnectedAt: null, onHideSidebar: undefined, reconnectAttempts: 20, diff --git a/mobile/src/host-screen/host-screen-header.tsx b/mobile/src/host-screen/host-screen-header.tsx index 33283be05bc..6a2601a981e 100644 --- a/mobile/src/host-screen/host-screen-header.tsx +++ b/mobile/src/host-screen/host-screen-header.tsx @@ -30,6 +30,7 @@ export function HostScreenHeader({ controller }: { controller: HostScreenControl floatingWorkspaceEnabled, forceReconnectHost, hostId, + hostDisplay, lastConnectedAt, onHideSidebar, reconnectAttempts, @@ -61,10 +62,17 @@ export function HostScreenHeader({ controller }: { controller: HostScreenControl return ( <> - - - {state.hostName || 'Host'} - + + + + {hostDisplay.title} + + + {hostDisplay.descriptorLine ? ( + + {hostDisplay.descriptorLine} + + ) : null} {connState !== 'connected' && (() => { diff --git a/mobile/src/host-screen/host-screen-primary-styles.ts b/mobile/src/host-screen/host-screen-primary-styles.ts index c499ad2afe6..1f13dd17b16 100644 --- a/mobile/src/host-screen/host-screen-primary-styles.ts +++ b/mobile/src/host-screen/host-screen-primary-styles.ts @@ -36,11 +36,16 @@ export const hostScreenPrimaryStyles = StyleSheet.create({ }, hostIdentity: { flex: 1, - flexDirection: 'row', - alignItems: 'center', minWidth: 0, marginRight: spacing.md }, + hostIdentityLine: { flexDirection: 'row', alignItems: 'center', minWidth: 0 }, + hostPlatformText: { + marginLeft: 16, + color: colors.textSecondary, + fontSize: 12, + lineHeight: 16 + }, hostNameText: { flex: 1, fontSize: 15, diff --git a/mobile/src/host-screen/use-host-screen-controller.ts b/mobile/src/host-screen/use-host-screen-controller.ts index 36ba705bf1f..66ab8119941 100644 --- a/mobile/src/host-screen/use-host-screen-controller.ts +++ b/mobile/src/host-screen/use-host-screen-controller.ts @@ -23,6 +23,7 @@ import { useHostScreenState } from './use-host-screen-state' import { useHostViewSettings } from './use-host-view-settings' import { useHostWorktreeActions } from './use-host-worktree-actions' import { useHostWorktreeCatalog } from './use-host-worktree-catalog' +import { useHostDisplay } from '../transport/use-host-display' export type HostScreenProps = { // When true, rendered as the persistent tablet sidebar by the host layout, not as its own routed screen. @@ -65,6 +66,11 @@ export function useHostScreenController({ const { hostCapabilities, floatingWorkspaceEnabled } = useHostProtocolGates() const state = useHostScreenState(hostId, action) const settings = useHostViewSettings({ client, connState, hostId, state }) + const hostDisplay = useHostDisplay( + hostId && state.hostName + ? { id: hostId, name: state.hostName, ...state.hostStoredDescriptor } + : null + ) useHostScreenIdentity({ client, hostId, state }) const fetchRepoMetadata = useHostRepoMetadata({ client, connState, hostId, state }) @@ -146,6 +152,7 @@ export function useHostScreenController({ floatingWorkspaceEnabled, forceReconnectHost, hostCapabilities, + hostDisplay, hostId, insets, isReadOnly: connState === 'auth-failed', diff --git a/mobile/src/host-screen/use-host-screen-identity.ts b/mobile/src/host-screen/use-host-screen-identity.ts index 0a45502090f..324f2d11c6d 100644 --- a/mobile/src/host-screen/use-host-screen-identity.ts +++ b/mobile/src/host-screen/use-host-screen-identity.ts @@ -20,6 +20,7 @@ export function useHostScreenIdentity(args: { setHostLabelById, setHostName, setHostPlatform, + setHostStoredDescriptor, setLastKnownWorktrees, setPinnedIds, setRepoColorsByName, @@ -54,6 +55,7 @@ export function useHostScreenIdentity(args: { useEffect(() => { setHostName('') + setHostStoredDescriptor(null) setError('') setRepoColorsByName(new Map()) setRepoIconsByName(new Map()) @@ -87,6 +89,15 @@ export function useHostScreenIdentity(args: { return } setHostName(host.name) + setHostStoredDescriptor({ + ...(host.personalName !== undefined ? { personalName: host.personalName } : {}), + ...(host.lastKnownMachineName !== undefined + ? { lastKnownMachineName: host.lastKnownMachineName } + : {}), + ...(host.lastKnownHostPlatform !== undefined + ? { lastKnownHostPlatform: host.lastKnownHostPlatform } + : {}) + }) void updateLastConnected(host.id) }) return () => { diff --git a/mobile/src/host-screen/use-host-screen-state.ts b/mobile/src/host-screen/use-host-screen-state.ts index 715645cc385..8e020fba446 100644 --- a/mobile/src/host-screen/use-host-screen-state.ts +++ b/mobile/src/host-screen/use-host-screen-state.ts @@ -40,6 +40,12 @@ export function useHostScreenState(hostId: string | undefined, action: string | const [repoColorsByName, setRepoColorsByName] = useState>(new Map()) const [repoIconsByName, setRepoIconsByName] = useState>(new Map()) const [hostName, setHostName] = useState('') + // The stored name identity, loaded with the name so the header can label an offline host. + const [hostStoredDescriptor, setHostStoredDescriptor] = useState<{ + personalName?: string + lastKnownMachineName?: string + lastKnownHostPlatform?: NodeJS.Platform + } | null>(null) const [error, setError] = useState('') // An action that did not happen, said above the list rather than instead of it. Separate from // `error`, which is the screen's identity and is the one thing worth taking the whole view for. @@ -107,6 +113,7 @@ export function useHostScreenState(hostId: string | undefined, action: string | hostLabelById, hostName, hostPlatform, + hostStoredDescriptor, lastKnownWorktrees, newWorktreeModalRef, newWorktreeModalVisibleRef, @@ -131,6 +138,7 @@ export function useHostScreenState(hostId: string | undefined, action: string | setHostLabelById, setHostName, setHostPlatform, + setHostStoredDescriptor, setLastKnownWorktrees, setOptimisticActiveWorktreeIdentity, setPinnedIds, diff --git a/mobile/src/mobile-web-shell/bridge/bridge-envelope.test.ts b/mobile/src/mobile-web-shell/bridge/bridge-envelope.test.ts index 5428b2642b8..056dcf7bb18 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-envelope.test.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-envelope.test.ts @@ -365,6 +365,53 @@ describe('host messages', () => { }) } + it("keeps the host's stored name identity, which the page's title rule reads", () => { + const host = { + id: 'host-a', + name: 'm4airs-Air', + lastKnownMachineName: 'm4airs-Air', + lastKnownHostPlatform: 'darwin', + endpoint: 'ws://host-a', + lastConnected: 0 + } + const read = readHost( + client({ + type: 'init', + sessionId: 's1', + buildId: 'b1', + connection: CONNECTION, + grants: GRANTS, + host + }) + ) + expect(read.ok && read.message.type === 'init' ? read.message.host : null).toEqual(host) + }) + + it('drops a name-identity value this page cannot read instead of refusing the init', () => { + const read = readHost( + client({ + type: 'init', + sessionId: 's1', + buildId: 'b1', + connection: CONNECTION, + grants: GRANTS, + host: { + id: 'host-a', + name: 'Studio', + personalName: '', + lastKnownMachineName: 'Studio', + lastKnownHostPlatform: 'plan9', + endpoint: 'ws://host-a', + lastConnected: 0 + } + }) + ) + const host = read.ok && read.message.type === 'init' ? read.message.host : null + expect(host).toMatchObject({ id: 'host-a', name: 'Studio', lastKnownMachineName: 'Studio' }) + expect(host?.personalName).toBeUndefined() + expect(host?.lastKnownHostPlatform).toBeUndefined() + }) + const refused = [ [ 'an init without a build id', diff --git a/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts b/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts index 7ea94531c02..ae8729f96ce 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts @@ -1,4 +1,6 @@ import { z } from 'zod' +import { salvagedOptional } from '../../../../src/shared/zod-salvage' +import { NODE_PLATFORM_NAMES } from '../../transport/mobile-runtime-host-platform' import { isRpcResponse } from '../../transport/rpc-response-shape' import type { RpcResponse } from '../../transport/types' import { BridgeErrorCaptureSchema } from './bridge-error-capture' @@ -90,6 +92,22 @@ export { BridgeInitRouteSchema, type BridgeInitRoute } export const BridgeInitHostSchema = z.object({ id: z.string().min(1).max(BRIDGE_MAX_HOST_FIELD_CHARS), name: z.string().min(1).max(BRIDGE_MAX_HOST_FIELD_CHARS), + // Why: `name` alone cannot say whether it is the phone's override or the desktop's name. + // Optional and additive: an older shell sends none and the page falls back to classifying `name`. + // Salvaged so a value this page cannot read (a newer shell's platform) drops the field, not `init`; + // the outer `.optional()` keeps the inferred key optional rather than required `T | undefined`. + personalName: salvagedOptional( + 'personalName', + z.string().min(1).max(BRIDGE_MAX_HOST_FIELD_CHARS) + ).optional(), + lastKnownMachineName: salvagedOptional( + 'lastKnownMachineName', + z.string().min(1).max(BRIDGE_MAX_HOST_FIELD_CHARS) + ).optional(), + lastKnownHostPlatform: salvagedOptional( + 'lastKnownHostPlatform', + z.enum(NODE_PLATFORM_NAMES) + ).optional(), endpoint: z.string().min(1).max(BRIDGE_MAX_HOST_FIELD_CHARS), lastConnected: z.number().finite() }) diff --git a/mobile/src/mobile-web-shell/use-page-host-snapshot.test.tsx b/mobile/src/mobile-web-shell/use-page-host-snapshot.test.tsx index 3c8cd7b5652..e74ad11181e 100644 --- a/mobile/src/mobile-web-shell/use-page-host-snapshot.test.tsx +++ b/mobile/src/mobile-web-shell/use-page-host-snapshot.test.tsx @@ -6,6 +6,8 @@ type Doubles = { store: Map writes: { key: string; value: string | null }[] hostsReject: boolean + /** The stored name identity the app's host record carries, beside its resolved name. */ + hostIdentity: Record /** Holds every store read open, which is how the two reads are made to answer out of order. */ holdReads: boolean releaseReads: (() => void)[] @@ -15,6 +17,7 @@ const doubles = vi.hoisted((): Doubles => ({ store: new Map(), writes: [], hostsReject: false, + hostIdentity: {}, holdReads: false, releaseReads: [] })) @@ -48,7 +51,15 @@ vi.mock('../transport/host-store', () => ({ if (doubles.hostsReject) { throw new Error('the keychain would not answer') } - return [{ id: 'host-1', name: 'Host One', endpoint: 'ws://host-1', lastConnected: 3 }] + return [ + { + id: 'host-1', + name: 'Host One', + ...doubles.hostIdentity, + endpoint: 'ws://host-1', + lastConnected: 3 + } + ] } })) @@ -87,6 +98,7 @@ beforeEach(() => { doubles.store.clear() doubles.writes.length = 0 doubles.hostsReject = false + doubles.hostIdentity = {} doubles.holdReads = false doubles.releaseReads.length = 0 }) @@ -243,6 +255,24 @@ describe('the host the page is handed', () => { expect(mounted.view().snapshot?.host.id).toBe('host-1') expect(mounted.view().readStorage().storage).toEqual({ [PINS]: '["one"]' }) }) + + it("carries the stored name identity, so the page can tell the phone's override from the desktop's name", async () => { + doubles.hostIdentity = { + personalName: 'Host One', + lastKnownMachineName: 'm4airs-Air', + lastKnownHostPlatform: 'darwin' + } + const mounted = await mount() + expect(mounted.view().snapshot?.host).toEqual({ + id: 'host-1', + name: 'Host One', + personalName: 'Host One', + lastKnownMachineName: 'm4airs-Air', + lastKnownHostPlatform: 'darwin', + endpoint: 'ws://host-1', + lastConnected: 3 + }) + }) }) describe('a host the app store would not answer for', () => { diff --git a/mobile/src/mobile-web-shell/use-page-host-snapshot.ts b/mobile/src/mobile-web-shell/use-page-host-snapshot.ts index f14d1147130..ff7dd2dd542 100644 --- a/mobile/src/mobile-web-shell/use-page-host-snapshot.ts +++ b/mobile/src/mobile-web-shell/use-page-host-snapshot.ts @@ -78,6 +78,13 @@ export function usePageHostSnapshot(hostId: string, routePathname: string): Page host: { id: found.id, name: found.name, + ...(found.personalName !== undefined ? { personalName: found.personalName } : {}), + ...(found.lastKnownMachineName !== undefined + ? { lastKnownMachineName: found.lastKnownMachineName } + : {}), + ...(found.lastKnownHostPlatform !== undefined + ? { lastKnownHostPlatform: found.lastKnownHostPlatform } + : {}), endpoint: found.endpoint, lastConnected: found.lastConnected } diff --git a/mobile/src/transport/client-context.test.ts b/mobile/src/transport/client-context.test.ts index a2effecd870..0d65be5a63f 100644 --- a/mobile/src/transport/client-context.test.ts +++ b/mobile/src/transport/client-context.test.ts @@ -8,6 +8,23 @@ import type { MobileConnectionPath } from './stable-logical-rpc-client' const push = vi.hoisted(() => ({ attach: vi.fn(), detach: vi.fn() })) vi.mock('../notifications/push-registration', () => ({ attachPushRegistration: push.attach })) +// Why: the opener starts a descriptor status probe per connection; these fakes have no RPC surface. +const descriptorProbe = vi.hoisted(() => { + const stop = vi.fn() + return { + stop, + start: vi.fn((_client: unknown, _onStatus: (status: unknown) => void) => stop), + record: vi.fn() + } +}) +vi.mock('./runtime-status-probe', () => ({ + startRuntimeStatusProbe: (client: unknown, onStatus: (status: unknown) => void) => + descriptorProbe.start(client, onStatus) +})) +vi.mock('./host-descriptor-recorder', () => ({ + recordHostDescriptorFromStatus: (...args: unknown[]) => descriptorProbe.record(...args) +})) + const connectMock = vi.fn() const loadHostsMock = vi.fn() @@ -739,6 +756,33 @@ it('owns push registration for a paired host without mounting the home screen', expect(push.detach).toHaveBeenCalledTimes(2) }) +it('reads each connected host descriptor once per connection and records what lands', async () => { + descriptorProbe.start.mockClear() + descriptorProbe.stop.mockClear() + descriptorProbe.record.mockClear() + const client = makeFakeClient('handshaking') + connectMock.mockReturnValue(client) + loadHostsMock.mockResolvedValue([HOST]) + const harness = await renderHarness(HOST.id) + expect(descriptorProbe.start).not.toHaveBeenCalled() + await act(async () => client.emitState('connected')) + await act(async () => client.emitState('connected')) + expect(descriptorProbe.start).toHaveBeenCalledOnce() + const onStatus = descriptorProbe.start.mock.calls[0]![1] + onStatus({ machineName: 'Studio', hostPlatform: 'darwin' }) + onStatus(null) + expect(descriptorProbe.record).toHaveBeenCalledExactlyOnceWith(HOST.id, { + machineName: 'Studio', + hostPlatform: 'darwin' + }) + await act(async () => client.emitState('disconnected')) + expect(descriptorProbe.stop).toHaveBeenCalledOnce() + await act(async () => client.emitState('connected')) + expect(descriptorProbe.start).toHaveBeenCalledTimes(2) + await act(async () => harness.unmount()) + expect(descriptorProbe.stop).toHaveBeenCalledTimes(2) +}) + it('registers an already authenticated host and detaches on explicit disconnect', async () => { const client = makeFakeClient('connected') connectMock.mockReturnValue(client) diff --git a/mobile/src/transport/host-descriptor-persistence.ts b/mobile/src/transport/host-descriptor-persistence.ts new file mode 100644 index 00000000000..ab0b80ce94a --- /dev/null +++ b/mobile/src/transport/host-descriptor-persistence.ts @@ -0,0 +1,30 @@ +import { mutateStoredHosts } from './host-list-mutation-queue' +import { withReportedDescriptor, type ReportedHostDescriptor } from './host-name-identity' + +/** + * Records what the desktop reported over an authenticated status read. Best-effort: descriptor + * upkeep must never gate connecting or acting on a host. A reply carrying neither field (a desktop + * from before either existed) writes nothing, so "Host N" rows for old desktops never churn. + */ +export async function updateHostDescriptor( + hostId: string, + descriptor: ReportedHostDescriptor +): Promise { + if (descriptor.machineName === null && descriptor.platform === null) { + return + } + try { + await mutateStoredHosts((hosts) => { + const index = hosts.findIndex((host) => host.id === hostId) + const updated = index === -1 ? null : withReportedDescriptor(hosts[index]!, descriptor) + if (updated === null || updated === hosts[index]) { + return hosts + } + const next = hosts.slice() + next[index] = updated + return next + }) + } catch { + // Unreadable storage is the next read's problem, not this bookkeeping's. + } +} diff --git a/mobile/src/transport/host-descriptor-recorder.test.ts b/mobile/src/transport/host-descriptor-recorder.test.ts new file mode 100644 index 00000000000..039f2f8ef33 --- /dev/null +++ b/mobile/src/transport/host-descriptor-recorder.test.ts @@ -0,0 +1,40 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const recordHostDescriptorMock = vi.hoisted(() => vi.fn()) +const updateHostDescriptorMock = vi.hoisted(() => vi.fn()) + +vi.mock('./host-descriptor-store', () => ({ + recordHostDescriptor: (...args: unknown[]) => recordHostDescriptorMock(...args) +})) + +vi.mock('./host-store', () => ({ + updateHostDescriptor: (...args: unknown[]) => updateHostDescriptorMock(...args) +})) + +import { recordHostDescriptorFromStatus } from './host-descriptor-recorder' +import { hostStatusSchema } from './host-status-reply-schema' + +describe('recordHostDescriptorFromStatus', () => { + beforeEach(() => { + recordHostDescriptorMock.mockClear() + updateHostDescriptorMock.mockReset() + updateHostDescriptorMock.mockResolvedValue(undefined) + }) + + it('writes the normalized descriptor to both the live store and the durable one', () => { + recordHostDescriptorFromStatus( + 'host-1', + hostStatusSchema.parse({ machineName: ' m4airs-Air ', hostPlatform: 'darwin' }) + ) + const descriptor = { machineName: 'm4airs-Air', platform: 'darwin' } + expect(recordHostDescriptorMock).toHaveBeenCalledWith('host-1', descriptor) + expect(updateHostDescriptorMock).toHaveBeenCalledWith('host-1', descriptor) + }) + + it('records an answered status that carried neither field as nulls', () => { + recordHostDescriptorFromStatus('host-1', hostStatusSchema.parse({ machineName: ' ' })) + const descriptor = { machineName: null, platform: null } + expect(recordHostDescriptorMock).toHaveBeenCalledWith('host-1', descriptor) + expect(updateHostDescriptorMock).toHaveBeenCalledWith('host-1', descriptor) + }) +}) diff --git a/mobile/src/transport/host-descriptor-recorder.ts b/mobile/src/transport/host-descriptor-recorder.ts new file mode 100644 index 00000000000..8379431e0c3 --- /dev/null +++ b/mobile/src/transport/host-descriptor-recorder.ts @@ -0,0 +1,18 @@ +import { recordHostDescriptor } from './host-descriptor-store' +import { updateHostDescriptor } from './host-store' +import type { HostStatusReply } from './host-status-reply-schema' + +/** + * The one writer of host descriptor state. Every readable status reply that arrives with a known + * host id lands here: the in-memory store repaints connected rows immediately, and the persisted + * copy is what an offline or freshly restarted app shows as "Last known". Persistence is + * best-effort inside `updateHostDescriptor`; recording must never gate a connection. + */ +export function recordHostDescriptorFromStatus(hostId: string, status: HostStatusReply): void { + const descriptor = { + machineName: status.machineName?.trim() || null, + platform: status.hostPlatform ?? null + } + recordHostDescriptor(hostId, descriptor) + void updateHostDescriptor(hostId, descriptor) +} diff --git a/mobile/src/transport/host-descriptor-store.test.ts b/mobile/src/transport/host-descriptor-store.test.ts new file mode 100644 index 00000000000..d356c02ea45 --- /dev/null +++ b/mobile/src/transport/host-descriptor-store.test.ts @@ -0,0 +1,86 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { + recordHostDescriptor, + resetHostDescriptorStoreForTests, + useHostDescriptor +} from './host-descriptor-store' + +describe('host descriptor store', () => { + let renderer: ReactTestRenderer | null = null + const seen: Record[]> = {} + + function Row({ hostId }: { hostId: string }): null { + const descriptor = useHostDescriptor(hostId) + ;(seen[hostId] ??= []).push(descriptor) + return null + } + + beforeEach(() => { + resetHostDescriptorStoreForTests() + for (const hostId of Object.keys(seen)) { + delete seen[hostId] + } + }) + + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + }) + + it('has nothing to show before the host answers a status read', async () => { + await act(async () => { + renderer = create(createElement(Row, { hostId: 'host-1' })) + }) + + expect(seen['host-1']?.at(-1)).toBeNull() + }) + + it('publishes the latest reply, including one that omitted both fields', async () => { + await act(async () => { + renderer = create(createElement(Row, { hostId: 'host-1' })) + }) + await act(async () => { + recordHostDescriptor('host-1', { machineName: 'Old', platform: 'win32' }) + }) + expect(seen['host-1']?.at(-1)).toEqual({ machineName: 'Old', platform: 'win32' }) + + await act(async () => { + recordHostDescriptor('host-1', { machineName: null, platform: null }) + }) + expect(seen['host-1']?.at(-1)).toEqual({ machineName: null, platform: null }) + }) + + it('does not re-render for an unchanged reply', async () => { + recordHostDescriptor('host-1', { machineName: 'Desk', platform: 'darwin' }) + await act(async () => { + renderer = create(createElement(Row, { hostId: 'host-1' })) + }) + const renders = seen['host-1']?.length ?? 0 + await act(async () => { + recordHostDescriptor('host-1', { machineName: 'Desk', platform: 'darwin' }) + }) + + expect(seen['host-1']).toHaveLength(renders) + }) + + it("does not re-render another host's row", async () => { + await act(async () => { + renderer = create( + createElement( + 'rows', + null, + createElement(Row, { hostId: 'host-1' }), + createElement(Row, { hostId: 'host-2' }) + ) + ) + }) + const host2Renders = seen['host-2']?.length ?? 0 + await act(async () => { + recordHostDescriptor('host-1', { machineName: 'Desk', platform: 'darwin' }) + }) + + expect(seen['host-2']).toHaveLength(host2Renders) + }) +}) diff --git a/mobile/src/transport/host-descriptor-store.ts b/mobile/src/transport/host-descriptor-store.ts new file mode 100644 index 00000000000..610c310fa5c --- /dev/null +++ b/mobile/src/transport/host-descriptor-store.ts @@ -0,0 +1,66 @@ +import { useCallback, useSyncExternalStore } from 'react' + +export type HostMachineDescriptor = { + machineName: string | null + platform: NodeJS.Platform | null +} + +// The live half of descriptor state: what this process last decoded from each host. The durable +// half is the stored host profile's lastKnown* fields; host-descriptor-recorder.ts writes both. +const descriptorByHostId = new Map() +const listenersByHostId = new Map void>>() + +/** Records every readable status reply, including one that omitted either descriptor field. */ +export function recordHostDescriptor(hostId: string, descriptor: HostMachineDescriptor): void { + const previous = descriptorByHostId.get(hostId) + if ( + previous?.machineName === descriptor.machineName && + previous.platform === descriptor.platform + ) { + return + } + descriptorByHostId.set(hostId, descriptor) + for (const listener of listenersByHostId.get(hostId) ?? []) { + listener() + } +} + +/** Drops a removed host's entry so a later re-pair cannot inherit the dead pairing's descriptor. */ +export function forgetHostDescriptor(hostId: string): void { + if (!descriptorByHostId.delete(hostId)) { + return + } + for (const listener of listenersByHostId.get(hostId) ?? []) { + listener() + } +} + +function subscribe(hostId: string, listener: () => void): () => void { + const listeners = listenersByHostId.get(hostId) ?? new Set<() => void>() + listeners.add(listener) + listenersByHostId.set(hostId, listeners) + return () => { + listeners.delete(listener) + if (listeners.size === 0) { + listenersByHostId.delete(hostId) + } + } +} + +/** The descriptor this host last reported to this app process, or null before any read. */ +export function useHostDescriptor(hostId: string | undefined): HostMachineDescriptor | null { + const read = useCallback( + () => (hostId ? (descriptorByHostId.get(hostId) ?? null) : null), + [hostId] + ) + const subscribeToHost = useCallback( + (listener: () => void) => (hostId ? subscribe(hostId, listener) : () => {}), + [hostId] + ) + return useSyncExternalStore(subscribeToHost, read, read) +} + +export function resetHostDescriptorStoreForTests(): void { + descriptorByHostId.clear() + listenersByHostId.clear() +} diff --git a/mobile/src/transport/host-entry-opener.ts b/mobile/src/transport/host-entry-opener.ts index 6a22d29bf5e..ae920186f64 100644 --- a/mobile/src/transport/host-entry-opener.ts +++ b/mobile/src/transport/host-entry-opener.ts @@ -1,4 +1,6 @@ import { attachPushRegistration } from '../notifications/push-registration' +import { recordHostDescriptorFromStatus } from './host-descriptor-recorder' +import { startRuntimeStatusProbe } from './runtime-status-probe' import { connectionLogStore, recordConnectionClientSessionStart @@ -123,12 +125,29 @@ export async function openHostClientEntry( detachPushRegistration = null } } + // Why here: the connection layer owns descriptor recording for every host client — home rows + // and host screens alike — so no screen has to re-ask, and the probe's cutover retry means a + // relay<->direct switch cannot lose the read. One extra status.get per connect is the cost. + let stopDescriptorProbe: (() => void) | null = null + const syncDescriptorProbe = (next: ConnectionState): void => { + if (next === 'connected') { + stopDescriptorProbe ??= startRuntimeStatusProbe(client, (status) => { + if (status) { + recordHostDescriptorFromStatus(hostId, status) + } + }) + } else { + stopDescriptorProbe?.() + stopDescriptorProbe = null + } + } const unsubscribeState = client.onStateChange((next) => { const current = state.store.get(hostId) if (!current) { return } syncPushRegistration(next) + syncDescriptorProbe(next) current.state = next state.notifyHostState(hostId, next) }) @@ -149,12 +168,15 @@ export async function openHostClientEntry( unsubscribeState() detachPushRegistration?.() detachPushRegistration = null + stopDescriptorProbe?.() + stopDescriptorProbe = null }, unsubConnectionPath } state.pendingAcquisitions.delete(hostId) state.store.set(hostId, entry) syncPushRegistration(entry.state) + syncDescriptorProbe(entry.state) settle() const priorFailureCount = state.retryScheduler.recordSuccess(hostId) if (priorFailureCount > 0) { diff --git a/mobile/src/transport/host-list-mutation-queue.ts b/mobile/src/transport/host-list-mutation-queue.ts new file mode 100644 index 00000000000..5ed1a06f5ec --- /dev/null +++ b/mobile/src/transport/host-list-mutation-queue.ts @@ -0,0 +1,38 @@ +import * as hostListLoads from './host-list-load-sharing' +import { readStoredHostProfilesForMutation, writeStoredHostProfiles } from './host-metadata-store' +import type { StoredHostProfile } from './types' + +// Why: serialize host metadata RMW so concurrent writers cannot drop updates. +let hostListMutation: Promise = Promise.resolve() + +/** Settles once every mutation queued so far has, so a reader never sees a half-written list. */ +export function hostListMutationsSettled(): Promise { + return hostListMutation +} + +export function enqueueHostListMutation(operation: () => Promise): Promise { + const mutation = hostListMutation.then(operation) + hostListMutation = mutation.catch(() => {}) + return mutation +} + +export async function mutateStoredHosts( + update: (hosts: StoredHostProfile[]) => StoredHostProfile[] | Promise +): Promise { + return enqueueHostListMutation(async () => { + const current = await readStoredHostProfilesForMutation() + const next = await update(current) + // Why: an update handing back the list it read changed nothing; a per-connect descriptor + // read must not rewrite storage and invalidate every shared host-list load. + if (next === current) { + return + } + await writeStoredHostProfiles(next) + hostListLoads.dropSharedHostListLoad() + }) +} + +/** Test-only: drain the mutation chain between cases. */ +export function resetHostListMutationQueueForTests(): void { + hostListMutation = Promise.resolve() +} diff --git a/mobile/src/transport/host-metadata-store.ts b/mobile/src/transport/host-metadata-store.ts index 7962f43dc37..303fbdb0d06 100644 --- a/mobile/src/transport/host-metadata-store.ts +++ b/mobile/src/transport/host-metadata-store.ts @@ -1,5 +1,6 @@ import AsyncStorage from '@react-native-async-storage/async-storage' import { StoredHostProfileSchema, type HostProfile, type StoredHostProfile } from './types' +import { classifyLegacyHostName } from './host-name-identity' const STORAGE_KEY = 'orca:hosts' @@ -24,8 +25,26 @@ export function writeStoredHostProfiles(hosts: readonly StoredHostProfile[]): Pr } export function toStoredHostProfile(host: HostProfile): StoredHostProfile { - const { id, name, endpoint, publicKeyB64, lastConnected } = host - return { id, name, endpoint, publicKeyB64, lastConnected } + const { + id, + name, + personalName, + lastKnownMachineName, + lastKnownHostPlatform, + endpoint, + publicKeyB64, + lastConnected + } = host + return { + id, + name, + ...(personalName !== undefined ? { personalName } : {}), + ...(lastKnownMachineName !== undefined ? { lastKnownMachineName } : {}), + ...(lastKnownHostPlatform !== undefined ? { lastKnownHostPlatform } : {}), + endpoint, + publicKeyB64, + lastConnected + } } function parseStoredHostProfiles(raw: string | null): StoredHostProfile[] | null { @@ -43,7 +62,7 @@ function parseStoredHostProfiles(raw: string | null): StoredHostProfile[] | null return [] } const result = StoredHostProfileSchema.safeParse(item) - return result.success ? [result.data] : [] + return result.success ? [classifyLegacyHostName(result.data)] : [] }) } catch { return null diff --git a/mobile/src/transport/host-name-identity.test.ts b/mobile/src/transport/host-name-identity.test.ts new file mode 100644 index 00000000000..8c67d86bdec --- /dev/null +++ b/mobile/src/transport/host-name-identity.test.ts @@ -0,0 +1,259 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const asyncStorageMock = vi.hoisted(() => ({ + getItem: vi.fn(), + setItem: vi.fn(), + removeItem: vi.fn() +})) + +const secureStoreMock = vi.hoisted(() => ({ + deleteItemAsync: vi.fn(), + getItemAsync: vi.fn(), + setItemAsync: vi.fn() +})) + +vi.mock('@react-native-async-storage/async-storage', () => ({ + default: asyncStorageMock +})) + +vi.mock('expo-secure-store', () => ({ + WHEN_UNLOCKED_THIS_DEVICE_ONLY: 'WHEN_UNLOCKED_THIS_DEVICE_ONLY', + ...secureStoreMock +})) + +vi.mock('react-native', () => ({ + Platform: { OS: 'ios' } +})) + +vi.mock('./host-credential-cleanup', () => ({ + cancelPendingHostCredentialCleanup: vi.fn().mockResolvedValue(undefined), + recordHostCredentialCleanupIntent: vi.fn().mockResolvedValue(undefined), + scheduleHostCredentialCleanup: vi.fn().mockResolvedValue(undefined), + retryPendingHostCredentialCleanups: vi.fn() +})) + +import { + loadHosts, + resetHostStoreForTests, + saveHost, + updateHostDescriptor, + updateHostNameAndEndpoint +} from './host-store' +import { resetMobileRelayHostOverlayStoreForTests } from './mobile-relay-host-overlay-store' +import { StoredHostProfileSchema, type StoredHostProfile } from './types' + +const HOSTS_STORAGE_KEY = 'orca:hosts' + +const GENERATED_HOST = { + id: 'host-1', + name: 'Host 1', + endpoint: 'ws://127.0.0.1:1', + publicKeyB64: 'key-1', + lastConnected: 0 +} + +const TYPED_HOST = { + id: 'host-2', + name: 'Windows-Low Spec', + endpoint: 'ws://127.0.0.1:2', + publicKeyB64: 'key-2', + lastConnected: 0 +} + +describe('host name identity', () => { + let storedHostsRaw: string + + beforeEach(() => { + vi.clearAllMocks() + resetHostStoreForTests() + resetMobileRelayHostOverlayStoreForTests() + storedHostsRaw = JSON.stringify([GENERATED_HOST, TYPED_HOST]) + asyncStorageMock.getItem.mockImplementation(async (key: string) => + key === HOSTS_STORAGE_KEY ? storedHostsRaw : null + ) + asyncStorageMock.setItem.mockImplementation(async (key: string, raw: string) => { + if (key === HOSTS_STORAGE_KEY) { + storedHostsRaw = raw + } + }) + secureStoreMock.setItemAsync.mockResolvedValue(undefined) + secureStoreMock.getItemAsync.mockResolvedValue('device-token') + }) + + function stored(): StoredHostProfile[] { + return StoredHostProfileSchema.array().parse(JSON.parse(storedHostsRaw)) + } + + describe('legacy classification', () => { + it('treats a typed legacy name as a phone override and a generated one as none', async () => { + const hosts = await loadHosts() + expect(hosts.find(({ id }) => id === GENERATED_HOST.id)?.personalName).toBeUndefined() + expect(hosts.find(({ id }) => id === TYPED_HOST.id)?.personalName).toBe('Windows-Low Spec') + }) + + it('does not re-classify an adopted machine name once identity fields exist', async () => { + // A record the descriptor writer already touched: non-generated name, no override. + storedHostsRaw = JSON.stringify([ + { + ...GENERATED_HOST, + name: 'm4airs-Air', + lastKnownMachineName: 'm4airs-Air', + lastKnownHostPlatform: 'darwin' + } + ]) + const hosts = await loadHosts() + expect(hosts[0]?.personalName).toBeUndefined() + }) + + it('drops only an identity value this build cannot read, never the paired host', async () => { + // A newer build's platform, or an empty string, as a rolled-back app would find them. + storedHostsRaw = JSON.stringify([ + { ...GENERATED_HOST, lastKnownHostPlatform: 'plan9', lastKnownMachineName: 'Studio' }, + { ...TYPED_HOST, personalName: '', lastKnownMachineName: '' } + ]) + const loaded = await loadHosts() + expect(loaded.map(({ id }) => id)).toEqual([GENERATED_HOST.id, TYPED_HOST.id]) + expect(loaded[0]?.lastKnownHostPlatform).toBeUndefined() + expect(loaded[0]?.lastKnownMachineName).toBe('Studio') + // The next write persists the list it parsed, so a dropped record would be gone for good. + await updateHostNameAndEndpoint(TYPED_HOST.id, { endpoint: 'ws://10.0.0.9:6768' }) + const records: Record[] = JSON.parse(storedHostsRaw) + expect(records.map(({ id }) => id)).toEqual([GENERATED_HOST.id, TYPED_HOST.id]) + expect(records[0]).not.toHaveProperty('lastKnownHostPlatform') + expect(records[1]).not.toHaveProperty('lastKnownMachineName') + expect(records[1]?.personalName).toBe('Windows-Low Spec') + }) + }) + + describe('updateHostDescriptor', () => { + it('adopts a reported machine name as the display name of an unoverridden host', async () => { + await updateHostDescriptor(GENERATED_HOST.id, { + machineName: 'm4airs-Air', + platform: 'darwin' + }) + expect(stored().find(({ id }) => id === GENERATED_HOST.id)).toMatchObject({ + name: 'm4airs-Air', + lastKnownMachineName: 'm4airs-Air', + lastKnownHostPlatform: 'darwin' + }) + }) + + it('records the descriptor under an override without touching the display name', async () => { + await updateHostDescriptor(TYPED_HOST.id, { machineName: 'm4airs-Air', platform: 'darwin' }) + expect(stored().find(({ id }) => id === TYPED_HOST.id)).toMatchObject({ + name: 'Windows-Low Spec', + personalName: 'Windows-Low Spec', + lastKnownMachineName: 'm4airs-Air' + }) + }) + + it('writes nothing for a desktop that reports neither field', async () => { + await updateHostDescriptor(GENERATED_HOST.id, { machineName: null, platform: null }) + expect(asyncStorageMock.setItem).not.toHaveBeenCalled() + expect(stored().find(({ id }) => id === GENERATED_HOST.id)?.name).toBe('Host 1') + }) + + it('clears a stale machine name when a live host stops reporting one', async () => { + await updateHostDescriptor(GENERATED_HOST.id, { + machineName: 'm4airs-Air', + platform: 'darwin' + }) + await updateHostDescriptor(GENERATED_HOST.id, { machineName: null, platform: 'darwin' }) + const record = stored().find(({ id }) => id === GENERATED_HOST.id) + expect(record?.lastKnownMachineName).toBeUndefined() + // The display name is kept: there is nothing better to fall back to. + expect(record?.name).toBe('m4airs-Air') + }) + + it('swallows unreadable storage instead of surfacing an error', async () => { + storedHostsRaw = '{' + await expect( + updateHostDescriptor(GENERATED_HOST.id, { machineName: 'm4airs-Air', platform: 'darwin' }) + ).resolves.toBeUndefined() + }) + }) + + describe('re-pair', () => { + it('preserves the override and descriptor a pairing save knows nothing about', async () => { + await updateHostDescriptor(TYPED_HOST.id, { machineName: 'm4airs-Air', platform: 'darwin' }) + await saveHost({ + id: TYPED_HOST.id, + name: 'Windows-Low Spec', + endpoint: 'ws://10.0.0.9:6768', + deviceToken: 'fresh-token', + publicKeyB64: TYPED_HOST.publicKeyB64, + lastConnected: 99 + }) + expect(stored().find(({ id }) => id === TYPED_HOST.id)).toMatchObject({ + name: 'Windows-Low Spec', + personalName: 'Windows-Low Spec', + lastKnownMachineName: 'm4airs-Air', + lastKnownHostPlatform: 'darwin', + endpoint: 'ws://10.0.0.9:6768' + }) + }) + + it('re-resolves an adopted name after a re-pair that offered the stored one', async () => { + await updateHostDescriptor(GENERATED_HOST.id, { + machineName: 'm4airs-Air', + platform: 'darwin' + }) + await saveHost({ + id: GENERATED_HOST.id, + name: 'Host 1', + endpoint: GENERATED_HOST.endpoint, + deviceToken: 'fresh-token', + publicKeyB64: GENERATED_HOST.publicKeyB64, + lastConnected: 99 + }) + expect(stored().find(({ id }) => id === GENERATED_HOST.id)?.name).toBe('m4airs-Air') + }) + + it('does not roll back identity changed since a connection took its profile snapshot', async () => { + await updateHostDescriptor(TYPED_HOST.id, { machineName: 'Studio', platform: 'darwin' }) + // A connection holds this for its lifetime and re-saves it on relay credential rotation. + const snapshot = (await loadHosts()).find(({ id }) => id === TYPED_HOST.id)! + await updateHostNameAndEndpoint(TYPED_HOST.id, { personalName: null }) + await updateHostDescriptor(TYPED_HOST.id, { machineName: 'Studio 2', platform: 'darwin' }) + await saveHost({ ...snapshot, lastConnected: 99 }) + const record = stored().find(({ id }) => id === TYPED_HOST.id) + expect(record?.personalName).toBeUndefined() + expect(record).toMatchObject({ + name: 'Studio 2', + lastKnownMachineName: 'Studio 2', + lastConnected: 99 + }) + }) + }) + + describe('phone override edit', () => { + it('sets the override and the display name together', async () => { + await updateHostNameAndEndpoint(GENERATED_HOST.id, { personalName: 'Basement Rig' }) + expect(stored().find(({ id }) => id === GENERATED_HOST.id)).toMatchObject({ + name: 'Basement Rig', + personalName: 'Basement Rig' + }) + }) + + it('clearing the override returns to the desktop-reported name', async () => { + await updateHostDescriptor(TYPED_HOST.id, { machineName: 'm4airs-Air', platform: 'darwin' }) + await updateHostNameAndEndpoint(TYPED_HOST.id, { personalName: null }) + const record = stored().find(({ id }) => id === TYPED_HOST.id) + expect(record?.personalName).toBeUndefined() + expect(record?.name).toBe('m4airs-Air') + }) + + it('clearing with no known machine name regenerates a Host N name', async () => { + await updateHostNameAndEndpoint(TYPED_HOST.id, { personalName: null }) + const record = stored().find(({ id }) => id === TYPED_HOST.id) + expect(record?.personalName).toBeUndefined() + // "Host 2" is taken by nothing here but "Host 1" exists, so the counter lands on 2. + expect(record?.name).toBe('Host 2') + }) + + it('keeps an already-generated name when clearing a no-op override', async () => { + await updateHostNameAndEndpoint(GENERATED_HOST.id, { personalName: null }) + expect(stored().find(({ id }) => id === GENERATED_HOST.id)?.name).toBe('Host 1') + }) + }) +}) diff --git a/mobile/src/transport/host-name-identity.ts b/mobile/src/transport/host-name-identity.ts new file mode 100644 index 00000000000..dc5d5bef9a1 --- /dev/null +++ b/mobile/src/transport/host-name-identity.ts @@ -0,0 +1,109 @@ +import { GENERATED_HOST_NAME_PATTERN, getNextHostNameFromHosts } from './host-names' +import type { StoredHostProfile } from './types' + +/** + * The rules that keep a stored host's `name` equal to + * `personalName ?? lastKnownMachineName ?? "Host N"`. Pure: host-store applies them inside its + * serialized mutation pass, so the resolved name and its sources are always written together. + * One exception: see `withReportedDescriptor`. + */ + +type HostNameIdentity = Pick< + StoredHostProfile, + 'name' | 'personalName' | 'lastKnownMachineName' | 'lastKnownHostPlatform' +> + +export type ReportedHostDescriptor = { + machineName: string | null + platform: NodeJS.Platform | null +} + +/** + * Classifies a record written before the name-identity fields existed. Legacy storage held one + * `name` that was either the generated "Host N" or typed by the user, and only the typed one is an + * override. Safe to re-run on every parse: any record a current build has written carries at least + * one of the three fields (clearing an override either restores the machine name — a field — or + * keeps a generated "Host N", which this pattern skips), so only true legacy records are classified. + */ +export function classifyLegacyHostName(profile: T): T { + if ( + profile.personalName !== undefined || + profile.lastKnownMachineName !== undefined || + profile.lastKnownHostPlatform !== undefined || + GENERATED_HOST_NAME_PATTERN.test(profile.name) + ) { + return profile + } + return { ...profile, personalName: profile.name } +} + +/** + * Why: a save rebuilds the record from a pairing offer (no name identity) or from a connection's + * profile snapshot (identity as of connect). Only the stored record reflects later renames and + * descriptor reads, so it keeps the name identity; the save supplies everything else. + */ +export function mergeHostNameIdentity( + incoming: StoredHostProfile, + existing: StoredHostProfile +): StoredHostProfile { + const { + personalName: _personalName, + lastKnownMachineName: _machineName, + lastKnownHostPlatform: _platform, + ...rest + } = incoming + const { personalName, lastKnownMachineName, lastKnownHostPlatform } = existing + return { + ...rest, + name: existing.name, + ...(personalName !== undefined ? { personalName } : {}), + ...(lastKnownMachineName !== undefined ? { lastKnownMachineName } : {}), + ...(lastKnownHostPlatform !== undefined ? { lastKnownHostPlatform } : {}) + } +} + +/** Sets the phone's override, or clears it (`null`) to return the row to the desktop's name. */ +export function withPersonalName( + current: StoredHostProfile, + personalName: string | null, + hosts: readonly StoredHostProfile[] +): StoredHostProfile { + if (personalName !== null) { + return { ...current, personalName, name: personalName } + } + const { personalName: _cleared, ...rest } = current + // Why: with no machine name to fall back to, an already-generated name is kept, not renumbered. + const fallback = GENERATED_HOST_NAME_PATTERN.test(current.name) + ? current.name + : getNextHostNameFromHosts(hosts) + return { ...rest, name: current.lastKnownMachineName ?? fallback } +} + +/** + * Applies what the desktop reported; returns `current` itself when nothing changed. An answered + * status is authoritative, so an omitted half clears its last-known value, and an unoverridden + * row adopts a newly reported machine name as its display name. + * Exception: an OS reported without a machine name keeps the previously adopted name as `name`. + */ +export function withReportedDescriptor( + current: StoredHostProfile, + descriptor: ReportedHostDescriptor +): StoredHostProfile { + const { lastKnownMachineName: _machineName, lastKnownHostPlatform: _platform, ...rest } = current + const lastKnownMachineName = descriptor.machineName ?? undefined + const lastKnownHostPlatform = descriptor.platform ?? undefined + const name = current.personalName ?? lastKnownMachineName ?? current.name + if ( + name === current.name && + lastKnownMachineName === current.lastKnownMachineName && + lastKnownHostPlatform === current.lastKnownHostPlatform + ) { + return current + } + return { + ...rest, + name, + ...(lastKnownMachineName !== undefined ? { lastKnownMachineName } : {}), + ...(lastKnownHostPlatform !== undefined ? { lastKnownHostPlatform } : {}) + } +} diff --git a/mobile/src/transport/host-names.ts b/mobile/src/transport/host-names.ts index 8ac9ef83e85..33b0fffc753 100644 --- a/mobile/src/transport/host-names.ts +++ b/mobile/src/transport/host-names.ts @@ -2,13 +2,14 @@ type HostNameSource = { readonly name: string } -const HOST_NUMBER_PATTERN = /^Host (\d+)$/ +/** The generated "Host N" shape; a stored name matching it was never typed by the user. */ +export const GENERATED_HOST_NAME_PATTERN = /^Host (\d+)$/ export function getNextHostNameFromHosts(hosts: readonly HostNameSource[]): string { let largestHostNumber = 0 for (const host of hosts) { - const match = HOST_NUMBER_PATTERN.exec(host.name) + const match = GENERATED_HOST_NAME_PATTERN.exec(host.name) if (!match) { continue } diff --git a/mobile/src/transport/host-open-recovery.test.tsx b/mobile/src/transport/host-open-recovery.test.tsx index 90e79c13976..972a30dce1c 100644 --- a/mobile/src/transport/host-open-recovery.test.tsx +++ b/mobile/src/transport/host-open-recovery.test.tsx @@ -11,6 +11,12 @@ const openHostLogicalClientMock = vi.fn() const loadHostsMock = vi.fn() const revival = vi.hoisted(() => ({ callback: null as null | ((reason: 'focus') => void) })) +// Why: the opener starts a descriptor status probe per connection; these fakes have no RPC surface. +const descriptorProbe = vi.hoisted(() => ({ start: vi.fn((..._args: unknown[]) => vi.fn()) })) +vi.mock('./runtime-status-probe', () => ({ + startRuntimeStatusProbe: (...args: unknown[]) => descriptorProbe.start(...args) +})) + vi.mock('./host-logical-client', () => ({ openHostLogicalClient: (...args: unknown[]) => openHostLogicalClientMock(...args) })) diff --git a/mobile/src/transport/host-removal-lifecycle.ts b/mobile/src/transport/host-removal-lifecycle.ts index 876fa0aa969..493cf54412f 100644 --- a/mobile/src/transport/host-removal-lifecycle.ts +++ b/mobile/src/transport/host-removal-lifecycle.ts @@ -3,6 +3,7 @@ import { forgetHostUpdateFailures } from '../mobile-web-shell/removed-host-shell-cache' import { unregisterPushForRemovedHost } from '../notifications/push-registration' +import { forgetHostDescriptor } from './host-descriptor-store' import { removeHost } from './host-store' export async function removeHostAndCloseClient( @@ -21,6 +22,7 @@ export async function removeHostAndCloseClient( throw error } forgetHostClient(hostId) + forgetHostDescriptor(hostId) // Why after the commit and not awaited: state about a host that is gone, never a reason to hold // the removal or fail it. A cache that fails to delete is reclaimed by the next eviction. void forgetHostUpdateFailures(hostId).catch(() => undefined) diff --git a/mobile/src/transport/host-status-capability-ignorability.test.ts b/mobile/src/transport/host-status-capability-ignorability.test.ts index b0714d09b33..03477a3a918 100644 --- a/mobile/src/transport/host-status-capability-ignorability.test.ts +++ b/mobile/src/transport/host-status-capability-ignorability.test.ts @@ -24,6 +24,11 @@ vi.mock('./host-app-version-store', () => ({ recordHostAppVersion: (...args: unknown[]) => recordHostAppVersionMock(...args) })) +// Why: the recorder reaches the durable host store, which this hook-level suite never exercises. +vi.mock('./host-descriptor-recorder', () => ({ + recordHostDescriptorFromStatus: vi.fn() +})) + /** * What a desktop that ships a mobile web bundle now answers. The new name sits among real ones * rather than alone, so a reader that keeps only the head or the tail of the list cannot look diff --git a/mobile/src/transport/host-status-gates.test.ts b/mobile/src/transport/host-status-gates.test.ts index 983f8146391..0711d3dfa0e 100644 --- a/mobile/src/transport/host-status-gates.test.ts +++ b/mobile/src/transport/host-status-gates.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import type { RpcClient } from './rpc-client' import { useHostStatusGates, type HostStatusGates } from './host-status-gates' @@ -10,7 +10,16 @@ vi.mock('./host-app-version-store', () => ({ recordHostAppVersion: (...args: unknown[]) => recordHostAppVersionMock(...args) })) +const recordDescriptorFromStatusMock = vi.hoisted(() => vi.fn()) + +vi.mock('./host-descriptor-recorder', () => ({ + recordHostDescriptorFromStatus: (...args: unknown[]) => recordDescriptorFromStatusMock(...args) +})) + describe('useHostStatusGates', () => { + beforeEach(() => { + recordDescriptorFromStatusMock.mockClear() + }) it('clears every prior-host gate and ignores its late response while the client is replaced', async () => { let resolveOldStatus: ((response: unknown) => void) | null = null const pendingOldStatus = new Promise((resolve) => { @@ -84,7 +93,9 @@ describe('useHostStatusGates', () => { result: { appVersion: '1.4.191', capabilities: ['browser.screencast.v1'], - floatingWorkspaceEnabled: true + floatingWorkspaceEnabled: true, + machineName: 'studio', + hostPlatform: 'darwin' } }) const client = { sendRequest } as unknown as RpcClient @@ -109,6 +120,10 @@ describe('useHostStatusGates', () => { expect(sendRequest).toHaveBeenCalledOnce() expect(recordHostAppVersionMock).toHaveBeenCalledWith('host-1', '1.4.191') + expect(recordDescriptorFromStatusMock).toHaveBeenCalledWith( + 'host-1', + expect.objectContaining({ machineName: 'studio', hostPlatform: 'darwin' }) + ) } finally { renderer?.unmount() } diff --git a/mobile/src/transport/host-status-gates.ts b/mobile/src/transport/host-status-gates.ts index f8e4817ec67..21407d80912 100644 --- a/mobile/src/transport/host-status-gates.ts +++ b/mobile/src/transport/host-status-gates.ts @@ -6,6 +6,7 @@ import { evaluateCompat, type CompatVerdict } from './protocol-compat' import type { HostStatusReply } from './host-status-reply-schema' import { normalizeHostAppVersion } from './host-app-version' import { recordHostAppVersion } from './host-app-version-store' +import { recordHostDescriptorFromStatus } from './host-descriptor-recorder' export type HostStatusGates = { hostCapabilities: string[] @@ -95,6 +96,11 @@ export function useHostStatusGates(args: { if (hostId && desktopAppVersion) { void recordHostAppVersion(hostId, desktopAppVersion) } + if (hostId) { + // Why also here: the web page never runs the connection layer's probe, and the host + // screen should not wait for it; the recorder is idempotent across the two feeds. + recordHostDescriptorFromStatus(hostId, status) + } settle({ hostCapabilities: status.capabilities ?? [], floatingWorkspaceEnabled: status.floatingWorkspaceEnabled === true, diff --git a/mobile/src/transport/host-status-probe-operations.ts b/mobile/src/transport/host-status-probe-operations.ts index d9663930cfe..ec9d1ac0e58 100644 --- a/mobile/src/transport/host-status-probe-operations.ts +++ b/mobile/src/transport/host-status-probe-operations.ts @@ -42,22 +42,31 @@ export function readHostStatusGates(reply: RpcResponse): HostStatusReply | null } /** - * The capabilities the retrying probe publishes, or `null` for a refusal it should back off from. + * The status the retrying probe delivers, or `null` for a refusal it should back off from. * - * An unreadable status publishes the empty set rather than backing off, because that is exactly - * what main did: `Array.isArray(result?.capabilities)` was false for a null, absent or foreign - * result and the probe published `[]` and stopped. Swallowing the error here rather than at the - * call site keeps that decision beside the operation whose reader produces it. + * An unreadable status lands as `{ status: null }` rather than backing off, because that is + * exactly what main did for the capability read: `Array.isArray(result?.capabilities)` was false + * for a null, absent or foreign result and the probe published `[]` and stopped. Swallowing the + * error here rather than at the call site keeps that decision beside the operation whose reader + * produces it. The wrapper object separates "landed but unreadable" from "refused". */ -export function readProbedHostCapabilities(reply: RpcResponse): readonly string[] | null { +export function readProbedHostStatus( + reply: RpcResponse +): { status: HostStatusReply | null } | null { try { const accepted = hostStatusProbe.interpret(reply) - return accepted.accepted ? (accepted.value.capabilities ?? []) : null + return accepted.accepted ? { status: accepted.value } : null } catch { - return [] + return { status: null } } } +/** The capability projection of `readProbedHostStatus`, kept for callers that only ask that much. */ +export function readProbedHostCapabilities(reply: RpcResponse): readonly string[] | null { + const landed = readProbedHostStatus(reply) + return landed ? (landed.status?.capabilities ?? []) : null +} + /** * Whether the host answered the probe at all, which is the whole of what the pairing race asks. * @@ -71,3 +80,12 @@ export function hostAnsweredStatusProbe(reply: RpcResponse): boolean { return true } } + +/** + * The status the pairing race attaches to its winner, or `null` when the host's answer is + * unreadable. Never throws: it runs in the race's fulfilment handler, where a throw would strand + * the candidate exactly as `hostAnsweredStatusProbe` describes. + */ +export function readPairingCandidateStatus(reply: RpcResponse): HostStatusReply | null { + return readProbedHostStatus(reply)?.status ?? null +} diff --git a/mobile/src/transport/host-status-reply-schema.test.ts b/mobile/src/transport/host-status-reply-schema.test.ts index 54a55b62c6e..d55d953784b 100644 --- a/mobile/src/transport/host-status-reply-schema.test.ts +++ b/mobile/src/transport/host-status-reply-schema.test.ts @@ -21,12 +21,14 @@ describe('the host status decodes from every version the gate admits', () => { minCompatibleMobileVersion: 1, appVersion: '1.4.200', capabilities: ['mobile.tasks.v1', 'push.v1'], - floatingWorkspaceEnabled: true + floatingWorkspaceEnabled: true, + hostPlatform: 'darwin', + machineName: 'Studio' } expect(reads(hostStatusSchema, status)).toMatchObject(status) }) - it('reads a host that answers none of the five fields', () => { + it('reads a host that answers none of the status fields', () => { expect(reads(hostStatusSchema, {})).toEqual({}) expect(reads(hostStatusSchema, { error: 'refused' })).toMatchObject({ error: 'refused' }) }) diff --git a/mobile/src/transport/host-status-reply-schema.ts b/mobile/src/transport/host-status-reply-schema.ts index aea9f705f40..041cbf6a6b9 100644 --- a/mobile/src/transport/host-status-reply-schema.ts +++ b/mobile/src/transport/host-status-reply-schema.ts @@ -1,5 +1,6 @@ import { z } from 'zod' import { salvagedOptional } from '../../../src/shared/zod-salvage' +import { NODE_PLATFORM_NAMES } from './mobile-runtime-host-platform' /** * The status the transport's own `status.get` reads. @@ -27,7 +28,9 @@ export const hostStatusSchema = z.looseObject({ minCompatibleMobileVersion: salvagedOptional('minCompatibleMobileVersion', z.number()), appVersion: salvagedOptional('appVersion', z.string()), floatingWorkspaceEnabled: salvagedOptional('floatingWorkspaceEnabled', z.boolean()), - capabilities: salvagedOptional('capabilities', z.array(z.string())) + capabilities: salvagedOptional('capabilities', z.array(z.string())), + hostPlatform: salvagedOptional('hostPlatform', z.enum(NODE_PLATFORM_NAMES)), + machineName: salvagedOptional('machineName', z.string()) }) export type HostStatusReply = z.output diff --git a/mobile/src/transport/host-store-endpoint.test.ts b/mobile/src/transport/host-store-endpoint.test.ts index 95d9c364b88..8553f66e851 100644 --- a/mobile/src/transport/host-store-endpoint.test.ts +++ b/mobile/src/transport/host-store-endpoint.test.ts @@ -44,50 +44,61 @@ describe('updateHostNameAndEndpoint', () => { } ] - it('commits name and endpoint together in a single write', async () => { + // Legacy typed names parse as phone overrides, so the untouched row keeps its override too. + const legacyOverride = (host: (typeof stored)[number]) => ({ ...host, personalName: host.name }) + + function writtenHosts(): unknown { + expect(AsyncStorage.setItem).toHaveBeenCalledTimes(1) + const [key, value] = vi.mocked(AsyncStorage.setItem).mock.calls[0]! + expect(key).toBe('orca:hosts') + return JSON.parse(value) + } + + it('commits the phone name and endpoint together in a single write', async () => { vi.mocked(AsyncStorage.getItem).mockResolvedValue(JSON.stringify(stored)) await updateHostNameAndEndpoint('host-1', { - name: 'Home Desk', + personalName: 'Home Desk', endpoint: 'ws://192.168.1.10:6768' }) - expect(AsyncStorage.setItem).toHaveBeenCalledTimes(1) - expect(AsyncStorage.setItem).toHaveBeenCalledWith( - 'orca:hosts', - JSON.stringify([ - { ...stored[0], name: 'Home Desk', endpoint: 'ws://192.168.1.10:6768' }, - stored[1] - ]) - ) + expect(writtenHosts()).toEqual([ + { + ...stored[0], + name: 'Home Desk', + personalName: 'Home Desk', + endpoint: 'ws://192.168.1.10:6768' + }, + legacyOverride(stored[1]!) + ]) }) it('updates only the provided field', async () => { vi.mocked(AsyncStorage.getItem).mockResolvedValue(JSON.stringify(stored)) - await updateHostNameAndEndpoint('host-1', { name: 'Home Desk' }) + await updateHostNameAndEndpoint('host-1', { personalName: 'Home Desk' }) - expect(AsyncStorage.setItem).toHaveBeenCalledWith( - 'orca:hosts', - JSON.stringify([{ ...stored[0], name: 'Home Desk' }, stored[1]]) - ) + expect(writtenHosts()).toEqual([ + { ...stored[0], name: 'Home Desk', personalName: 'Home Desk' }, + legacyOverride(stored[1]!) + ]) }) - it('rewrites only the endpoint when name is omitted', async () => { + it('rewrites only the endpoint when the name is omitted', async () => { vi.mocked(AsyncStorage.getItem).mockResolvedValue(JSON.stringify(stored)) await updateHostNameAndEndpoint('host-1', { endpoint: 'ws://192.168.1.10:6768' }) - expect(AsyncStorage.setItem).toHaveBeenCalledWith( - 'orca:hosts', - JSON.stringify([{ ...stored[0], endpoint: 'ws://192.168.1.10:6768' }, stored[1]]) - ) + expect(writtenHosts()).toEqual([ + { ...legacyOverride(stored[0]!), endpoint: 'ws://192.168.1.10:6768' }, + legacyOverride(stored[1]!) + ]) }) it('throws and writes nothing when the host is missing', async () => { vi.mocked(AsyncStorage.getItem).mockResolvedValue('[]') - await expect(updateHostNameAndEndpoint('missing', { name: 'Renamed' })).rejects.toThrow( + await expect(updateHostNameAndEndpoint('missing', { personalName: 'Renamed' })).rejects.toThrow( 'Host not found' ) expect(AsyncStorage.setItem).not.toHaveBeenCalled() diff --git a/mobile/src/transport/host-store.test.ts b/mobile/src/transport/host-store.test.ts index ddb8f92a92b..bb13e23dc4d 100644 --- a/mobile/src/transport/host-store.test.ts +++ b/mobile/src/transport/host-store.test.ts @@ -615,7 +615,7 @@ describe('host-store list mutations', () => { return storedHostsRaw }) - const rename = updateHostNameAndEndpoint(HOST_ONE.id, { name: 'Renamed Host' }) + const rename = updateHostNameAndEndpoint(HOST_ONE.id, { personalName: 'Renamed Host' }) const remove = removeHost(HOST_TWO.id) // Both writers have started their RMW and are blocked on the shared read // gate; without a mutation queue the second would clobber the first. @@ -626,19 +626,16 @@ describe('host-store list mutations', () => { await Promise.all([rename, remove]) expect(JSON.parse(storedHostsRaw)).toEqual([ - { - ...HOST_ONE, - name: 'Renamed Host' - } + { ...HOST_ONE, name: 'Renamed Host', personalName: 'Renamed Host' } ]) }) it('preserves a rename when lastConnected updates race it', async () => { const before = Date.now() await Promise.all([ - updateHostNameAndEndpoint(HOST_ONE.id, { name: 'Alpha' }), + updateHostNameAndEndpoint(HOST_ONE.id, { personalName: 'Alpha' }), updateLastConnected(HOST_ONE.id), - updateHostNameAndEndpoint(HOST_TWO.id, { name: 'Beta' }) + updateHostNameAndEndpoint(HOST_TWO.id, { personalName: 'Beta' }) ]) const stored = JSON.parse(storedHostsRaw) as Array @@ -654,7 +651,7 @@ describe('host-store list mutations', () => { it('does not wipe the host list when storage is unreadable during mutation', async () => { storedHostsRaw = '{' - await expect(updateHostNameAndEndpoint(HOST_ONE.id, { name: 'Nope' })).rejects.toThrow( + await expect(updateHostNameAndEndpoint(HOST_ONE.id, { personalName: 'Nope' })).rejects.toThrow( /unreadable/ ) expect(asyncStorageMock.setItem).not.toHaveBeenCalled() @@ -699,7 +696,7 @@ describe('host-store list mutations', () => { expect(secureStoreMock.getItemAsync).toHaveBeenCalled() }) - await updateHostNameAndEndpoint(HOST_ONE.id, { name: 'Living Room Mac' }) + await updateHostNameAndEndpoint(HOST_ONE.id, { personalName: 'Living Room Mac' }) const afterRename = loadHosts() releaseKeychain() diff --git a/mobile/src/transport/host-store.ts b/mobile/src/transport/host-store.ts index c32ac44ac77..32f910cd27d 100644 --- a/mobile/src/transport/host-store.ts +++ b/mobile/src/transport/host-store.ts @@ -1,6 +1,7 @@ import { HostProfileSchema } from './types' import type { HostCatalogEntry, HostProfile, StoredHostProfile } from './types' import { getNextHostNameFromHosts } from './host-names' +import { mergeHostNameIdentity, withPersonalName } from './host-name-identity' import * as hostListLoads from './host-list-load-sharing' import { joinHostCatalogCredentials } from './host-catalog-credential-join' import { resetPairingKeychainForTests } from './pairing-keychain' @@ -27,9 +28,14 @@ import { createUnpairedHostCredentialDeletion } from './unpaired-host-credential import { loadStoredHostProfiles, readStoredHostProfilesForMutation, - toStoredHostProfile, - writeStoredHostProfiles + toStoredHostProfile } from './host-metadata-store' +import { + enqueueHostListMutation, + hostListMutationsSettled, + mutateStoredHosts, + resetHostListMutationQueueForTests +} from './host-list-mutation-queue' async function commitDeviceToken(hostId: string, token: string): Promise { markHostCredentialWrite(hostId) @@ -40,8 +46,6 @@ async function commitDeviceToken(hostId: string, token: string): Promise { // Why: Keychain reads are slow (50-200ms) and loadHosts() runs on every screen mount; cache per-hostId in memory, invalidate on save/remove. const tokenCache = new Map() -// Why: serialize host metadata RMW so concurrent writers cannot drop updates. -let hostListMutation: Promise = Promise.resolve() export const loadHosts = async (): Promise => (await loadHostListSnapshot()).profiles export const loadHostCatalog = async (): Promise => @@ -49,7 +53,7 @@ export const loadHostCatalog = async (): Promise => async function loadHostListSnapshot(): Promise { // Why: writers hold the mutation chain across their full RMW; wait so a load doesn't race a half-written list. - await hostListMutation + await hostListMutationsSettled() // Why: deduplicate concurrent loadHosts() calls so simultaneously mounting screens share one Keychain read pass. return hostListLoads.shareHostListLoad(doLoadHostListSnapshot) } @@ -85,7 +89,7 @@ export async function resolvePairingHostIdentity( newHostId: string ): Promise<{ id: string; name: string }> { // Why: one durable read both preserves an existing identity and names a new host, avoiding duplicate cards. - await hostListMutation + await hostListMutationsSettled() const hosts = await readStoredHostProfilesForMutation() const match = hosts.find((host) => host.publicKeyB64 === publicKeyB64) return match @@ -94,7 +98,7 @@ export async function resolvePairingHostIdentity( } const deleteUnpairedHostCredentials = createUnpairedHostCredentialDeletion({ - waitForHostMutations: () => hostListMutation, + waitForHostMutations: hostListMutationsSettled, hasStoredHost: async (hostId) => (await readStoredHostProfilesForMutation()).some(({ id }) => id === hostId), onDeleted: (hostId) => { @@ -111,14 +115,13 @@ function scheduleUnpairedHostCredentialCleanup(hostId: string): Promise { } function cancelCleanupForStoredHost(hostId: string): void { - const cancellation = hostListMutation.then(async () => { + void enqueueHostListMutation(async () => { const hosts = await readStoredHostProfilesForMutation() if (hosts.some(({ id }) => id === hostId)) { // Register before later removals enqueue their intent, without blocking host loads on cleanup storage. void cancelPendingHostCredentialCleanup(hostId).catch(() => undefined) } - }) - hostListMutation = cancellation.catch(() => {}) + }).catch(() => {}) } async function cancelCleanupForDurablyStoredHosts(hostIds: Iterable): Promise { @@ -133,12 +136,6 @@ async function cancelCleanupForDurablyStoredHosts(hostIds: Iterable): Pr }).catch(() => undefined) } -function enqueueHostListMutation(operation: () => Promise): Promise { - const mutation = hostListMutation.then(operation) - hostListMutation = mutation.catch(() => {}) - return mutation -} - function removeOrphanOverlayIfUnpaired(hostId: string): Promise { return enqueueHostListMutation(async () => { const hosts = await readStoredHostProfilesForMutation() @@ -148,16 +145,8 @@ function removeOrphanOverlayIfUnpaired(hostId: string): Promise { }) } -async function mutateStoredHosts( - update: (hosts: StoredHostProfile[]) => StoredHostProfile[] | Promise -): Promise { - return enqueueHostListMutation(async () => { - const current = await readStoredHostProfilesForMutation() - const next = await update(current) - await writeStoredHostProfiles(next) - hostListLoads.dropSharedHostListLoad() - }) -} +// The page's host-store sibling keeps its own no-op, so only the native store reaches persistence. +export { updateHostDescriptor } from './host-descriptor-persistence' export class MobileRelayUpgradeHostRemovedError extends Error {} @@ -187,7 +176,9 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise // Why: an authoritative save is the safe point to collapse pre-existing duplicate rows to the preserved host id. next = hosts .filter(({ id }) => !duplicateHostIds.has(id)) - .map((candidate) => (candidate.id === stored.id ? stored : candidate)) + .map((candidate) => + candidate.id === stored.id ? mergeHostNameIdentity(stored, candidate) : candidate + ) } else if (requireExisting) { // Why: an in-flight relay upgrade must not resurrect a host the user removed. throw new MobileRelayUpgradeHostRemovedError('mobile relay upgrade host was removed') @@ -298,21 +289,25 @@ export async function retryPendingHostCredentialCleanup(): Promise<{ } // Why: single mutation pass commits name + endpoint atomically so a mid-save failure can't persist one without the other. +// `personalName: null` clears the phone's override, returning the row to the desktop-reported name. export async function updateHostNameAndEndpoint( hostId: string, - updates: { name?: string; endpoint?: string } + updates: { personalName?: string | null; endpoint?: string } ): Promise { await mutateStoredHosts((hosts) => { const index = hosts.findIndex((host) => host.id === hostId) if (index === -1) { throw new Error('Host not found') } - const next = hosts.slice() - next[index] = { - ...next[index]!, - ...(updates.name !== undefined ? { name: updates.name } : {}), + let updated: StoredHostProfile = { + ...hosts[index]!, ...(updates.endpoint !== undefined ? { endpoint: updates.endpoint } : {}) } + if (updates.personalName !== undefined) { + updated = withPersonalName(updated, updates.personalName, hosts) + } + const next = hosts.slice() + next[index] = updated return next }) } @@ -335,7 +330,7 @@ export async function updateLastConnected(hostId: string): Promise { /** Test-only: drain module mutation chain between cases. */ export function resetHostStoreForTests(): void { - hostListMutation = Promise.resolve() + resetHostListMutationQueueForTests() tokenCache.clear() resetHostCredentialWriteRevisionsForTests() hostListLoads.dropSharedHostListLoad() diff --git a/mobile/src/transport/host-store.web.ts b/mobile/src/transport/host-store.web.ts index 88d539d5865..8c9d54e588c 100644 --- a/mobile/src/transport/host-store.web.ts +++ b/mobile/src/transport/host-store.web.ts @@ -34,8 +34,15 @@ export const updateLastConnected = (_hostId: string): Promise => Promise.r export function updateHostNameAndEndpoint( _hostId: string, - _name: string, - _endpoint: string + _updates: { personalName?: string | null; endpoint?: string } +): Promise { + return Promise.resolve() +} + +/** A native write the page drops: last-known descriptors belong to the app's own host list. */ +export function updateHostDescriptor( + _hostId: string, + _descriptor: { machineName: string | null; platform: NodeJS.Platform | null } ): Promise { return Promise.resolve() } diff --git a/mobile/src/transport/pairing-candidate-race.ts b/mobile/src/transport/pairing-candidate-race.ts index 4fd016b2c54..3a7201af2c8 100644 --- a/mobile/src/transport/pairing-candidate-race.ts +++ b/mobile/src/transport/pairing-candidate-race.ts @@ -1,5 +1,10 @@ -import { hostAnsweredStatusProbe, hostStatusProbe } from './host-status-probe-operations' +import { + hostAnsweredStatusProbe, + hostStatusProbe, + readPairingCandidateStatus +} from './host-status-probe-operations' import type { PairingCandidateClient } from './mobile-relay-physical-client' +import type { HostStatusReply } from './host-status-reply-schema' export type PairingCandidatePath = 'direct' | 'relay' @@ -8,11 +13,15 @@ export type PairingCandidate = { client: PairingCandidateClient } +export type PairingCandidateWinner = PairingCandidate & { + status: HostStatusReply | null +} + export function racePairingCandidates( candidates: readonly PairingCandidate[] -): Promise { +): Promise { return new Promise((resolve, reject) => { - const successes: PairingCandidate[] = [] + const successes: PairingCandidateWinner[] = [] let failures = 0 let settled = false let selectionQueued = false @@ -24,7 +33,7 @@ export function racePairingCandidates( rejectIfFinished() return } - successes.push(candidate) + successes.push({ ...candidate, status: readPairingCandidateStatus(reply) }) if (selectionQueued) { return } @@ -38,7 +47,7 @@ export function racePairingCandidates( settled = true const winner = successes.find(({ path }) => path === 'direct') ?? successes[0]! for (const loser of candidates) { - if (loser !== winner) { + if (loser.client !== winner.client) { loser.client.close() } } diff --git a/mobile/src/transport/pairing-relay-host.ts b/mobile/src/transport/pairing-relay-host.ts new file mode 100644 index 00000000000..4954b079fc9 --- /dev/null +++ b/mobile/src/transport/pairing-relay-host.ts @@ -0,0 +1,46 @@ +import type { + DeviceCredentialInstalled, + MobileRelayEndpoint +} from '../../../src/shared/mobile-relay-credential-contract' +import type { MobileRelayPairingJournal } from './mobile-relay-pairing-journal' +import type { HostProfile } from './types' + +export function relayHost( + journal: MobileRelayPairingJournal, + relay: MobileRelayEndpoint +): HostProfile { + const host = journal.metadata.host + return { + ...host, + deviceToken: journal.secrets.deviceToken, + endpoints: [ + { id: 'direct-primary', kind: 'lan', url: host.endpoint }, + { id: 'relay-primary', kind: 'relay', url: relayWebSocketUrl(relay) } + ], + relayHostId: relay.relayHostId, + relay + } +} + +export function relayWebSocketUrl(relay: MobileRelayEndpoint): string { + const url = new URL(relay.cellUrl) + url.protocol = 'wss:' + url.pathname = `/v1/connect/${encodeURIComponent(relay.relayHostId)}` + return url.toString() +} + +export function assertCommittedInstall( + status: + | { state: 'not-found' } + | { state: 'committed'; result: DeviceCredentialInstalled } + | undefined, + installed: DeviceCredentialInstalled +): void { + if ( + !status || + status.state !== 'committed' || + JSON.stringify(status.result) !== JSON.stringify(installed) + ) { + throw new Error('relay credential install was not authoritatively reconciled') + } +} diff --git a/mobile/src/transport/pre-profile-pairing-coordinator.test.ts b/mobile/src/transport/pre-profile-pairing-coordinator.test.ts index c99307a923d..bff67eadf2c 100644 --- a/mobile/src/transport/pre-profile-pairing-coordinator.test.ts +++ b/mobile/src/transport/pre-profile-pairing-coordinator.test.ts @@ -123,6 +123,9 @@ function dependencies(client: RpcClient, events: string[]) { writeCredentialBundle: vi.fn(async (_bundle: MobileRelayCredentialBundle) => { events.push('write-credential') }), + recordDescriptorFromStatus: vi.fn(() => { + events.push('record-descriptor') + }), now: () => now, platform: 'ios' } @@ -176,7 +179,7 @@ describe('pre-profile pairing coordinator', () => { publicKeyB64: directOffer.publicKeyB64, lastConnected: now }) - expect(events).toEqual(['connect', 'save-host']) + expect(events).toEqual(['connect', 'save-host', 'record-descriptor']) }) it('reuses the existing host id and name when re-pairing the same desktop key (no duplicate)', async () => { @@ -208,6 +211,54 @@ describe('pre-profile pairing coordinator', () => { }) }) + it('hands the winning status to the descriptor recorder only after the host is saved', async () => { + const events: string[] = [] + const client = fakeClient([success({ machineName: 'm4airs-Air', hostPlatform: 'darwin' })]) + const deps = dependencies(client, events) + const attempt = startPreProfilePairing({ + offer: directOffer, + timeoutMs: 5_000, + dependencies: deps + }) + + await expect(attempt.result).resolves.toEqual({ hostId: `host-${now}` }) + expect(events).toEqual(['connect', 'save-host', 'record-descriptor']) + expect(deps.recordDescriptorFromStatus).toHaveBeenCalledWith( + `host-${now}`, + expect.objectContaining({ machineName: 'm4airs-Air', hostPlatform: 'darwin' }) + ) + }) + + it('pairs a desktop whose status reply is unreadable, recording no descriptor', async () => { + const deps = dependencies(fakeClient([success(null)]), []) + const attempt = startPreProfilePairing({ + offer: directOffer, + timeoutMs: 5_000, + dependencies: deps + }) + + await expect(attempt.result).resolves.toEqual({ hostId: `host-${now}` }) + expect(deps.saveHost).toHaveBeenCalledOnce() + expect(deps.recordDescriptorFromStatus).not.toHaveBeenCalled() + }) + + it('still resolves a saved pairing when descriptor recording throws', async () => { + const events: string[] = [] + const client = fakeClient([success({ machineName: 'm4airs-Air', hostPlatform: 'darwin' })]) + const deps = dependencies(client, events) + deps.recordDescriptorFromStatus.mockImplementation(() => { + throw new Error('storage unavailable') + }) + const attempt = startPreProfilePairing({ + offer: directOffer, + timeoutMs: 5_000, + dependencies: deps + }) + + await expect(attempt.result).resolves.toEqual({ hostId: `host-${now}` }) + expect(deps.saveHost).toHaveBeenCalledOnce() + }) + it('journals before connecting and publishes only after authoritative direct install', async () => { const events: string[] = [] let journal: MobileRelayPairingJournal | null = null @@ -269,7 +320,8 @@ describe('pre-profile pairing coordinator', () => { 'update-journal', 'write-credential', 'save-host', - 'clear-journal' + 'clear-journal', + 'record-descriptor' ]) expect(client.sendRequest).toHaveBeenNthCalledWith(2, 'pairing.provisionRelay', { reqId: journal!.metadata.installReqId, @@ -312,7 +364,8 @@ describe('pre-profile pairing coordinator', () => { 'connect', 'update-journal', 'save-host', - 'clear-journal' + 'clear-journal', + 'record-descriptor' ]) }) @@ -343,7 +396,8 @@ describe('pre-profile pairing coordinator', () => { 'connect', 'update-journal', 'save-host', - 'clear-journal' + 'clear-journal', + 'record-descriptor' ]) expect(entries).toContainEqual( expect.objectContaining({ diff --git a/mobile/src/transport/pre-profile-pairing-coordinator.ts b/mobile/src/transport/pre-profile-pairing-coordinator.ts index 472648b1dbb..0939938bec3 100644 --- a/mobile/src/transport/pre-profile-pairing-coordinator.ts +++ b/mobile/src/transport/pre-profile-pairing-coordinator.ts @@ -1,8 +1,4 @@ import { Platform } from 'react-native' -import type { - DeviceCredentialInstalled, - MobileRelayEndpoint -} from '../../../src/shared/mobile-relay-credential-contract' import { connect, type ConnectOptions } from './rpc-client' import { resolvePairingHostIdentity, saveHost } from './host-store' import type { HostProfile, PairingOffer } from './types' @@ -34,6 +30,9 @@ import { resolvePairingInviteThroughDirector } from './mobile-relay-invite-direc import { createRecoveringPairingRelayCandidate } from './pairing-relay-candidate' import { createPairingRelayLogger } from './pairing-relay-log' import { redactSocketEndpoint } from './socket-event-debug' +import { assertCommittedInstall, relayHost } from './pairing-relay-host' +import { recordHostDescriptorFromStatus } from './host-descriptor-recorder' +import type { HostStatusReply } from './host-status-reply-schema' export type PreProfilePairingAttempt = { readonly result: Promise<{ hostId: string }> @@ -51,6 +50,7 @@ type Dependencies = { updateJournal: typeof updateMobileRelayPairingJournal clearJournal: typeof clearMobileRelayPairingJournal writeCredentialBundle: typeof writeMobileRelayCredentialBundle + recordDescriptorFromStatus: typeof recordHostDescriptorFromStatus now: () => number platform: string } @@ -65,6 +65,7 @@ const defaultDependencies: Dependencies = { updateJournal: updateMobileRelayPairingJournal, clearJournal: clearMobileRelayPairingJournal, writeCredentialBundle: writeMobileRelayCredentialBundle, + recordDescriptorFromStatus: recordHostDescriptorFromStatus, now: Date.now, platform: Platform.OS } @@ -206,6 +207,7 @@ async function runPairing( if (!journal) { await dependencies.saveHost(baseHost(offer, hostId, hostName, now)) + recordWinnerDescriptor(dependencies, hostId, winner.status) return { hostId } } @@ -231,6 +233,7 @@ async function runPairing( log('info', 'Relay: desktop will not serve relay pairing', provision.error.code) await dependencies.saveHost(baseHost(offer, hostId, hostName, now)) await dependencies.clearJournal(journal.metadata.journalId) + recordWinnerDescriptor(dependencies, hostId, winner.status) return { hostId } } const installed = relayCredentialProvision.interpret(provision) @@ -246,9 +249,31 @@ async function runPairing( await dependencies.writeCredentialBundle(promotePairingJournalCredential({ journal, installed })) await dependencies.saveHost(relayHost(journal, endpoints.relay)) await dependencies.clearJournal(journal.metadata.journalId) + recordWinnerDescriptor(dependencies, hostId, winner.status) return { hostId } } +/** + * Why after the save: the saved row starts as its existing name (or "Host N") and the descriptor + * writer adopt-renames it to the desktop's machine name in the same serialized store chain, so a + * load issued after pairing returns the desktop-reported name. A recording failure is swallowed — + * descriptor upkeep must never fail a pairing that already saved. + */ +function recordWinnerDescriptor( + dependencies: Dependencies, + hostId: string, + status: HostStatusReply | null +): void { + if (!status) { + return + } + try { + dependencies.recordDescriptorFromStatus(hostId, status) + } catch { + // Best-effort bookkeeping; the host is already saved. + } +} + function baseHost( offer: PairingOffer, hostId: string, @@ -265,43 +290,6 @@ function baseHost( } } -function relayHost(journal: MobileRelayPairingJournal, relay: MobileRelayEndpoint): HostProfile { - const host = journal.metadata.host - return { - ...host, - deviceToken: journal.secrets.deviceToken, - endpoints: [ - { id: 'direct-primary', kind: 'lan', url: host.endpoint }, - { id: 'relay-primary', kind: 'relay', url: relayWebSocketUrl(relay) } - ], - relayHostId: relay.relayHostId, - relay - } -} - -function relayWebSocketUrl(relay: MobileRelayEndpoint): string { - const url = new URL(relay.cellUrl) - url.protocol = 'wss:' - url.pathname = `/v1/connect/${encodeURIComponent(relay.relayHostId)}` - return url.toString() -} - -function assertCommittedInstall( - status: - | { state: 'not-found' } - | { state: 'committed'; result: DeviceCredentialInstalled } - | undefined, - installed: DeviceCredentialInstalled -): void { - if ( - !status || - status.state !== 'committed' || - JSON.stringify(status.result) !== JSON.stringify(installed) - ) { - throw new Error('relay credential install was not authoritatively reconciled') - } -} - function assertActive(isDisposed: () => boolean): void { if (isDisposed()) { throw new Error('mobile pairing cancelled') diff --git a/mobile/src/transport/runtime-capability-probe.ts b/mobile/src/transport/runtime-capability-probe.ts index 3461a12765e..946b99c7c9d 100644 --- a/mobile/src/transport/runtime-capability-probe.ts +++ b/mobile/src/transport/runtime-capability-probe.ts @@ -1,60 +1,16 @@ -import type { UnvalidatedRpcRequestPort } from './unvalidated-rpc-request-port' -import { hostStatusProbe, readProbedHostCapabilities } from './host-status-probe-operations' -import { isLogicalClientCutoverError } from './stable-logical-rpc-client' +import { startRuntimeStatusProbe } from './runtime-status-probe' -// Why: a relay→direct cutover or request timeout can reject an in-flight -// status.get without ever changing connState, so a one-shot probe would latch -// capability-gated UI hidden until the screen remounts; retry until one lands. -const CUTOVER_RETRY_DELAY_MS = 250 -const FAILURE_RETRY_BASE_DELAY_MS = 1_000 -const FAILURE_RETRY_MAX_DELAY_MS = 15_000 - -// The parameter names the raw port rather than RpcClient because one of the four callers holds -// only the sender; the request itself goes through hostStatusProbe. +/** + * The retrying status probe projected to its capability list. An unreadable status publishes the + * empty set rather than backing off, because that is exactly what main did before the reply gained + * a schema: `Array.isArray(result?.capabilities)` was false for a null, absent or foreign result + * and the probe published `[]` and stopped. + */ export function startRuntimeCapabilityProbe( - client: UnvalidatedRpcRequestPort, + client: Parameters[0], onCapabilities: (capabilities: readonly string[]) => void ): () => void { - let cancelled = false - let retryTimer: ReturnType | null = null - let failureRetries = 0 - - function attempt(): void { - void hostStatusProbe.request(client).then( - (reply) => { - if (cancelled) { - return - } - const capabilities = readProbedHostCapabilities(reply) - if (!capabilities) { - scheduleRetry(false) - return - } - onCapabilities(capabilities) - }, - (error: unknown) => { - if (cancelled) { - return - } - scheduleRetry(isLogicalClientCutoverError(error)) - } - ) - } - - function scheduleRetry(cutover: boolean): void { - // Why: cutover means the replacement transport is already authenticated — - // re-ask promptly; other failures back off so a wedged host isn't hammered. - const delay = cutover - ? CUTOVER_RETRY_DELAY_MS - : Math.min(FAILURE_RETRY_BASE_DELAY_MS * 2 ** failureRetries++, FAILURE_RETRY_MAX_DELAY_MS) - retryTimer = setTimeout(attempt, delay) - } - - attempt() - return () => { - cancelled = true - if (retryTimer) { - clearTimeout(retryTimer) - } - } + return startRuntimeStatusProbe(client, (status) => { + onCapabilities(status?.capabilities ?? []) + }) } diff --git a/mobile/src/transport/runtime-status-probe.test.ts b/mobile/src/transport/runtime-status-probe.test.ts new file mode 100644 index 00000000000..93b36d6dc2f --- /dev/null +++ b/mobile/src/transport/runtime-status-probe.test.ts @@ -0,0 +1,95 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { startRuntimeStatusProbe } from './runtime-status-probe' +import { LogicalClientCutoverError } from './stable-logical-rpc-client' +import type { HostStatusReply } from './host-status-reply-schema' +import type { UnvalidatedRpcRequestPort } from './unvalidated-rpc-request-port' +import type { RpcResponse } from './types' + +type ProbeOutcome = RpcResponse | Error + +function makeClient(outcomes: ProbeOutcome[]): { + client: UnvalidatedRpcRequestPort + calls: () => number +} { + let calls = 0 + const client: UnvalidatedRpcRequestPort = { + sendRequest: () => { + const outcome = outcomes[Math.min(calls, outcomes.length - 1)]! + calls += 1 + return outcome instanceof Error ? Promise.reject(outcome) : Promise.resolve(outcome) + } + } + return { client, calls: () => calls } +} + +const ok = (result: unknown): RpcResponse => ({ + ok: true, + id: '1', + result, + _meta: { runtimeId: 'r1' } +}) + +async function flushMicrotasks(): Promise { + await Promise.resolve() + await Promise.resolve() +} + +describe('startRuntimeStatusProbe', () => { + beforeEach(() => { + vi.useFakeTimers() + }) + afterEach(() => { + vi.useRealTimers() + }) + + it('delivers the parsed status once and stops', async () => { + const { client, calls } = makeClient([ + ok({ machineName: 'm4airs-Air', hostPlatform: 'darwin', capabilities: ['a.v1'] }) + ]) + const seen: (HostStatusReply | null)[] = [] + const cancel = startRuntimeStatusProbe(client, (status) => seen.push(status)) + await flushMicrotasks() + expect(seen).toHaveLength(1) + expect(seen[0]).toMatchObject({ machineName: 'm4airs-Air', hostPlatform: 'darwin' }) + expect(calls()).toBe(1) + await vi.advanceTimersByTimeAsync(60_000) + expect(calls()).toBe(1) + cancel() + }) + + it('delivers null for an answer this build cannot decode, without retrying', async () => { + const { client, calls } = makeClient([ok(null)]) + const seen: (HostStatusReply | null)[] = [] + const cancel = startRuntimeStatusProbe(client, (status) => seen.push(status)) + await flushMicrotasks() + expect(seen).toEqual([null]) + expect(calls()).toBe(1) + cancel() + }) + + it('retries a refusal and a cutover rejection until a status lands', async () => { + const refusal: RpcResponse = { + ok: false, + id: '1', + error: { code: 'unavailable', message: 'no' }, + _meta: { runtimeId: 'r1' } + } + const { client, calls } = makeClient([ + refusal, + new LogicalClientCutoverError(), + ok({ machineName: 'Studio' }) + ]) + const seen: (HostStatusReply | null)[] = [] + const cancel = startRuntimeStatusProbe(client, (status) => seen.push(status)) + await flushMicrotasks() + expect(seen).toEqual([]) + await vi.advanceTimersByTimeAsync(1_000) + await flushMicrotasks() + await vi.advanceTimersByTimeAsync(250) + await flushMicrotasks() + expect(seen).toHaveLength(1) + expect(seen[0]).toMatchObject({ machineName: 'Studio' }) + expect(calls()).toBe(3) + cancel() + }) +}) diff --git a/mobile/src/transport/runtime-status-probe.ts b/mobile/src/transport/runtime-status-probe.ts new file mode 100644 index 00000000000..1d748e1e685 --- /dev/null +++ b/mobile/src/transport/runtime-status-probe.ts @@ -0,0 +1,67 @@ +import type { UnvalidatedRpcRequestPort } from './unvalidated-rpc-request-port' +import { hostStatusProbe, readProbedHostStatus } from './host-status-probe-operations' +import type { HostStatusReply } from './host-status-reply-schema' +import { isLogicalClientCutoverError } from './stable-logical-rpc-client' + +// Why: a relay→direct cutover or request timeout can reject an in-flight +// status.get without ever changing connState, so a one-shot probe would latch +// its consumers on nothing until the screen remounts; retry until one lands. +const CUTOVER_RETRY_DELAY_MS = 250 +const FAILURE_RETRY_BASE_DELAY_MS = 1_000 +const FAILURE_RETRY_MAX_DELAY_MS = 15_000 + +/** + * Asks status.get until an answer lands, then delivers it once and stops. `null` is a host that + * answered something this build cannot decode — an answer, not a failure, so it is delivered + * rather than retried. A refusal or transport rejection schedules a retry instead. + * + * The parameter names the raw port rather than RpcClient because one of the capability callers + * holds only the sender; the request itself goes through hostStatusProbe. + */ +export function startRuntimeStatusProbe( + client: UnvalidatedRpcRequestPort, + onStatus: (status: HostStatusReply | null) => void +): () => void { + let cancelled = false + let retryTimer: ReturnType | null = null + let failureRetries = 0 + + function attempt(): void { + void hostStatusProbe.request(client).then( + (reply) => { + if (cancelled) { + return + } + const landed = readProbedHostStatus(reply) + if (!landed) { + scheduleRetry(false) + return + } + onStatus(landed.status) + }, + (error: unknown) => { + if (cancelled) { + return + } + scheduleRetry(isLogicalClientCutoverError(error)) + } + ) + } + + function scheduleRetry(cutover: boolean): void { + // Why: cutover means the replacement transport is already authenticated — + // re-ask promptly; other failures back off so a wedged host isn't hammered. + const delay = cutover + ? CUTOVER_RETRY_DELAY_MS + : Math.min(FAILURE_RETRY_BASE_DELAY_MS * 2 ** failureRetries++, FAILURE_RETRY_MAX_DELAY_MS) + retryTimer = setTimeout(attempt, delay) + } + + attempt() + return () => { + cancelled = true + if (retryTimer) { + clearTimeout(retryTimer) + } + } +} diff --git a/mobile/src/transport/settings-host-client-lifecycle.test.ts b/mobile/src/transport/settings-host-client-lifecycle.test.ts index bd0c2048637..e6c55117e0d 100644 --- a/mobile/src/transport/settings-host-client-lifecycle.test.ts +++ b/mobile/src/transport/settings-host-client-lifecycle.test.ts @@ -23,6 +23,12 @@ const routeFocus = vi.hoisted(() => ({ effect: null as null | (() => void | (() => void)) })) +// Why: the opener starts a descriptor status probe per connection; these fakes have no RPC surface. +const descriptorProbe = vi.hoisted(() => ({ start: vi.fn(() => vi.fn()) })) +vi.mock('./runtime-status-probe', () => ({ + startRuntimeStatusProbe: (...args: unknown[]) => descriptorProbe.start(...args) +})) + vi.mock('expo-router', () => ({ useFocusEffect: (effect: () => void | (() => void)) => { routeFocus.effect = effect diff --git a/mobile/src/transport/types.ts b/mobile/src/transport/types.ts index bb853ba260f..0ed86a7a986 100644 --- a/mobile/src/transport/types.ts +++ b/mobile/src/transport/types.ts @@ -9,6 +9,8 @@ import { type MobileRelayHostOverlay } from './mobile-relay-host-overlay' import { MobileRelayEndpointSchema } from '../../../src/shared/mobile-relay-credential-contract' +import { NODE_PLATFORM_NAMES } from './mobile-runtime-host-platform' +import { salvagedOptional } from '../../../src/shared/zod-salvage' export { PairingOfferSchema } export type { PairingOffer } @@ -102,7 +104,14 @@ export type ForegroundNudgeReason = 'focus' | 'app-resume' | 'network-change' export type HostProfile = { id: string + /** Resolved display value; the store maintains `name === personalName ?? lastKnownMachineName ?? "Host N"`, + * except that a desktop reporting its OS without a machine name leaves the last adopted one here. */ name: string + /** The name typed on this phone. Present only when the user renamed the host here; it overrides the desktop's. */ + personalName?: string + /** What the desktop last called itself over status.get, kept for offline and post-restart rows. */ + lastKnownMachineName?: string + lastKnownHostPlatform?: NodeJS.Platform endpoint: string deviceToken: string publicKeyB64: string @@ -119,9 +128,22 @@ export type HostCatalogEntry = Omit & { profile: HostProfile | null } +// Why: salvaged so a value this build cannot read (a newer build's platform, an empty string) +// drops only that field; a failed record parse would drop the whole paired host on the next write. +// The outer `.optional()` keeps the inferred key optional rather than required `T | undefined`. +const hostNameIdentityFields = { + personalName: salvagedOptional('personalName', z.string().min(1)).optional(), + lastKnownMachineName: salvagedOptional('lastKnownMachineName', z.string().min(1)).optional(), + lastKnownHostPlatform: salvagedOptional( + 'lastKnownHostPlatform', + z.enum(NODE_PLATFORM_NAMES) + ).optional() +} + export const HostProfileSchema = z.object({ id: z.string().min(1), name: z.string().min(1), + ...hostNameIdentityFields, endpoint: z.string().min(1), deviceToken: z.string().min(1), publicKeyB64: z.string().min(1), @@ -140,6 +162,7 @@ export const HostProfileSchema = z.object({ export const StoredHostProfileSchema = z.object({ id: z.string().min(1), name: z.string().min(1), + ...hostNameIdentityFields, endpoint: z.string().min(1), publicKeyB64: z.string().min(1), lastConnected: z.number().finite() diff --git a/mobile/src/transport/unvalidated-rpc-request-port-inventory.ts b/mobile/src/transport/unvalidated-rpc-request-port-inventory.ts index 724eaedee66..89f5a1f911f 100644 --- a/mobile/src/transport/unvalidated-rpc-request-port-inventory.ts +++ b/mobile/src/transport/unvalidated-rpc-request-port-inventory.ts @@ -182,6 +182,7 @@ export const UNVALIDATED_RPC_REQUEST_PORT_PENDING: readonly UnvalidatedRpcReques { file: 'src/transport/mobile-runtime-capability-negotiation.ts', references: 2 }, // Sends through hostStatusProbe; the one reference left is its parameter type. Its callers do // not share a client type — push-registration.ts holds only the sender — so the parameter names - // the port itself. It reaches zero when the last such caller migrates. - { file: 'src/transport/runtime-capability-probe.ts', references: 1 } + // the port itself. It reaches zero when the last such caller migrates. Moved here from + // runtime-capability-probe.ts, which is now a projection of this probe and names no port. + { file: 'src/transport/runtime-status-probe.ts', references: 1 } ] diff --git a/mobile/src/transport/use-host-display.test.ts b/mobile/src/transport/use-host-display.test.ts new file mode 100644 index 00000000000..780bd089ada --- /dev/null +++ b/mobile/src/transport/use-host-display.test.ts @@ -0,0 +1,56 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { HostDisplayResolution } from '../../../src/shared/host-display-resolution' +import { recordHostDescriptor, resetHostDescriptorStoreForTests } from './host-descriptor-store' +import { useHostDisplay, type HostDisplaySource } from './use-host-display' + +function renderDisplay(host: HostDisplaySource): HostDisplayResolution | null { + let display: HostDisplayResolution | null = null + function Probe(): null { + display = useHostDisplay(host) + return null + } + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + act(() => { + renderer = create(createElement(Probe)) + }) + consoleError.mockRestore() + return display +} + +let renderer: ReactTestRenderer | null = null + +describe('useHostDisplay', () => { + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + resetHostDescriptorStoreForTests() + }) + + it('keeps a label that arrives without identity fields above the live machine name', () => { + // The web page receives only the app's resolved `name`, which may be the user's rename. + recordHostDescriptor('desk', { machineName: 'm4airs-Air', platform: 'darwin' }) + expect(renderDisplay({ id: 'desk', name: 'Windows-Low Spec' })).toEqual({ + title: 'Windows-Low Spec', + descriptorLine: 'macOS · m4airs-Air' + }) + }) + + it('lets the live machine name replace a generated name that arrives without identity fields', () => { + recordHostDescriptor('desk', { machineName: 'm4airs-Air', platform: 'darwin' }) + expect(renderDisplay({ id: 'desk', name: 'Host 2' })?.title).toBe('m4airs-Air') + }) + + it('follows a desktop rename past a stored machine name that arrives with its identity fields', () => { + recordHostDescriptor('desk', { machineName: 'Brennans-M4', platform: 'darwin' }) + expect( + renderDisplay({ + id: 'desk', + name: 'm4airs-Air', + lastKnownMachineName: 'm4airs-Air', + lastKnownHostPlatform: 'darwin' + }) + ).toEqual({ title: 'Brennans-M4', descriptorLine: 'macOS' }) + }) +}) diff --git a/mobile/src/transport/use-host-display.ts b/mobile/src/transport/use-host-display.ts new file mode 100644 index 00000000000..c0ef7b5acf3 --- /dev/null +++ b/mobile/src/transport/use-host-display.ts @@ -0,0 +1,32 @@ +import { + resolveHostDisplay, + type HostDisplayResolution +} from '../../../src/shared/host-display-resolution' +import { useHostDescriptor } from './host-descriptor-store' +import { classifyLegacyHostName } from './host-name-identity' + +export type HostDisplaySource = { + id: string + name: string + personalName?: string + lastKnownMachineName?: string + lastKnownHostPlatform?: NodeJS.Platform +} + +/** + * The display precedence, stated once: a live-reported descriptor beats the persisted last-known + * one — including a live reply that reported nothing, which is the desktop's own answer. The title + * is the phone's override, else the live machine name, else the stored resolved `name`; the live + * name leads only because the stored copy catches up on the next host-list load. + * Pass null while the host record is still loading. + */ +export function useHostDisplay(host: HostDisplaySource | null): HostDisplayResolution { + const live = useHostDescriptor(host?.id) + // Why: a source without identity fields (a page handed its host by an older shell) holds a label. + const identity = host ? classifyLegacyHostName(host) : null + return resolveHostDisplay({ + name: identity?.personalName ?? live?.machineName ?? identity?.name ?? '', + machineName: live ? live.machineName : (host?.lastKnownMachineName ?? null), + platform: live ? live.platform : (host?.lastKnownHostPlatform ?? null) + }) +} diff --git a/src/renderer/src/components/settings/runtime-server-row-unverifiable-host.test.tsx b/src/renderer/src/components/settings/runtime-server-row-unverifiable-host.test.tsx index 6a1ba57dd0e..4dc1382729a 100644 --- a/src/renderer/src/components/settings/runtime-server-row-unverifiable-host.test.tsx +++ b/src/renderer/src/components/settings/runtime-server-row-unverifiable-host.test.tsx @@ -127,3 +127,23 @@ it('still offers Disconnect for a verified host', () => { expect(screen.queryByRole('button', { name: /disconnect/i })).not.toBeNull() }) + +it('keeps the machine under the label once the host is unreachable', () => { + const status = { ...answeredStatus(), machineName: 'Studio', hostPlatform: 'darwin' as const } + setEntry({ + status: null, + checkedAt: 2, + snapshot: snapshot({ status, verification: 'unavailable' }) + }) + renderRow() + expect(screen.queryByText('macOS · Studio')).not.toBeNull() + cleanup() + + setEntry({ + status: null, + checkedAt: 2, + snapshot: snapshot({ status, verification: 'unavailable', transport: 'disconnected' }) + }) + renderRow() + expect(screen.queryByText('macOS · Studio')).not.toBeNull() +}) diff --git a/src/renderer/src/components/settings/runtime-server-row.tsx b/src/renderer/src/components/settings/runtime-server-row.tsx index 989e6a67d54..3a8f30deecf 100644 --- a/src/renderer/src/components/settings/runtime-server-row.tsx +++ b/src/renderer/src/components/settings/runtime-server-row.tsx @@ -3,6 +3,8 @@ import type { PublicKnownRuntimeEnvironment } from '../../../../shared/runtime-e import type { RemoteServerUpdateEntry } from '@/runtime/remote-server-update-coordinator' import { translate } from '@/i18n/i18n' import { cn } from '@/lib/utils' +import { resolveHostDisplay } from '../../../../shared/host-display-resolution' +import { lastVerifiedRuntimeStatus } from '../../../../shared/runtime-host-status' import { isConnectedRuntimeHostState, runtimeHostConnectionStateForEntry @@ -93,13 +95,21 @@ export function RuntimeServerRow({ // A connected host exposes Disconnect; otherwise Connect. const isReachable = isRuntimeServerTransportConnected(connectionState) const actionBusy = connecting || switching || disconnecting || removing + // Why: the snapshot keeps the last answered status across a lost probe; `status` is only the latest answer. + const descriptorStatus = lastVerifiedRuntimeStatus(runtimeStatusEntry) + const hostDisplay = resolveHostDisplay({ + name: environment.name, + machineName: descriptorStatus?.machineName, + platform: descriptorStatus?.hostPlatform + }) + const hostDescriptorText = hostDisplay.descriptorLine return (
-
{environment.name}
+
{hostDisplay.title}
) : null}
+ {hostDescriptorText ? ( +

{hostDescriptorText}

+ ) : null}

{environment.connectionDependency === 'ssh-tunnel' ? translate( diff --git a/src/shared/host-display-resolution.test.ts b/src/shared/host-display-resolution.test.ts new file mode 100644 index 00000000000..f13f54d11c0 --- /dev/null +++ b/src/shared/host-display-resolution.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'vitest' +import { resolveHostDisplay } from './host-display-resolution' + +describe('resolveHostDisplay', () => { + it('titles with the caller label and shows a disagreeing machine name beneath it', () => { + expect( + resolveHostDisplay({ + name: 'Windows-Low Spec', + machineName: 'm4airs-Air', + platform: 'darwin' + }) + ).toEqual({ + title: 'Windows-Low Spec', + descriptorLine: 'macOS · m4airs-Air' + }) + }) + + it('never titles with the machine name, whatever the label state', () => { + // The title-fallback bug class: a late descriptor must not be able to retitle a row. + for (const name of ['Host 2', 'Desk']) { + expect( + resolveHostDisplay({ name, machineName: 'm4airs-Air', platform: 'darwin' }).title + ).toBe(name) + } + }) + + it('keeps the OS visible when the shown name is the machine name', () => { + expect( + resolveHostDisplay({ + name: 'm4airs-Air', + machineName: 'm4airs-Air', + platform: 'darwin' + }) + ).toEqual({ title: 'm4airs-Air', descriptorLine: 'macOS' }) + }) + + it('labels whichever descriptor half an older host reported', () => { + expect(resolveHostDisplay({ name: 'Desk', platform: 'linux' })).toEqual({ + title: 'Desk', + descriptorLine: 'Linux' + }) + expect(resolveHostDisplay({ name: 'Desk', machineName: 'build-box' })).toEqual({ + title: 'Desk', + descriptorLine: 'build-box' + }) + expect(resolveHostDisplay({ name: 'Desk' })).toEqual({ + title: 'Desk', + descriptorLine: null + }) + }) + + it('normalizes blank inputs', () => { + expect(resolveHostDisplay({ name: ' ', machineName: ' ' })).toEqual({ + title: 'Host', + descriptorLine: null + }) + }) +}) diff --git a/src/shared/host-display-resolution.ts b/src/shared/host-display-resolution.ts new file mode 100644 index 00000000000..b35dce5e5e1 --- /dev/null +++ b/src/shared/host-display-resolution.ts @@ -0,0 +1,37 @@ +import { hostPlatformDisplayName } from './host-platform-label' + +export type HostDisplayResolutionInput = { + /** The label the caller already owns: a stored resolved name, or a desktop-local server label. */ + name: string + machineName?: string | null + platform?: NodeJS.Platform | null +} + +export type HostDisplayResolution = { + title: string + /** + * "OS · machine name" when the machine name differs from the shown title, the OS alone when it + * matches (the OS is always shown when known), or null when the host reported neither. + */ + descriptorLine: string | null +} + +/** + * The one display rule for host rows and headers. The title is always the caller's own label — + * a machine name can appear only on the descriptor line, never as the title, so a late-arriving + * descriptor can never retitle a row. + */ +export function resolveHostDisplay(input: HostDisplayResolutionInput): HostDisplayResolution { + const title = normalize(input.name) ?? 'Host' + const machineName = normalize(input.machineName) + const descriptorLine = + [hostPlatformDisplayName(input.platform ?? null), machineName === title ? null : machineName] + .filter(Boolean) + .join(' · ') || null + return { title, descriptorLine } +} + +function normalize(value: string | null | undefined): string | null { + const trimmed = value?.trim() + return trimmed ? trimmed : null +} diff --git a/src/shared/host-platform-label.ts b/src/shared/host-platform-label.ts new file mode 100644 index 00000000000..5df8f78d1d0 --- /dev/null +++ b/src/shared/host-platform-label.ts @@ -0,0 +1,19 @@ +const PLATFORM_LABELS: Record = { + aix: 'AIX', + android: 'Android', + cygwin: 'Cygwin', + darwin: 'macOS', + freebsd: 'FreeBSD', + haiku: 'Haiku', + linux: 'Linux', + netbsd: 'NetBSD', + openbsd: 'OpenBSD', + sunos: 'Solaris', + win32: 'Windows' +} + +export function hostPlatformDisplayName( + platform: NodeJS.Platform | null | undefined +): string | null { + return platform ? PLATFORM_LABELS[platform] : null +}