From 79551c38f5ad8bb6d16e13ddfc276fc24d3b65c0 Mon Sep 17 00:00:00 2001 From: BingZ Date: Thu, 16 Jul 2026 07:49:21 +0800 Subject: [PATCH] feat(mobile): edit saved host endpoints (#8294) * feat(mobile): edit saved host endpoints * fix(mobile): reject ambiguous numeric host addresses * fix(mobile): label edit host inputs * fix(mobile): make host edit save atomic and remove superseded mutators Two independent review rounds found the same class of foot-gun: a superseded mutator (updateHostEndpoint, then renameHost) left in host-store.ts after the atomic updateHostNameAndEndpoint refactor, with zero remaining callers. Either could be reintroduced by a future caller and silently regress the non-atomic name/endpoint race the atomic function was written to close, so both are removed. Also covers reconnect-rejection and endpoint-only save paths that were missing test coverage, and merges origin/main (#8789) so this lands without reverting the mobile terminal restore fix. Co-authored-by: Orca * Simplify save-race comment and reword host-removed error message - Trims the redundant comment explaining the savingRef race guard down to one line. - Changes the "no longer saved" load-error copy to "was removed" for clearer phrasing, updating the matching test expectation. --------- Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Co-authored-by: Orca --- mobile/app/h/[hostId]/edit.tsx | 382 ++++++++++++++++++ mobile/app/h/_layout.tsx | 1 + mobile/app/index.tsx | 32 +- .../src/host-edit-route-accessibility.test.ts | 106 +++++ mobile/src/host-edit-save-flow.test.ts | 325 +++++++++++++++ mobile/src/transport/host-endpoint.test.ts | 257 ++++++++++++ mobile/src/transport/host-endpoint.ts | 365 +++++++++++++++++ .../src/transport/host-store-endpoint.test.ts | 95 +++++ mobile/src/transport/host-store.test.ts | 12 +- mobile/src/transport/host-store.ts | 19 +- 10 files changed, 1557 insertions(+), 37 deletions(-) create mode 100644 mobile/app/h/[hostId]/edit.tsx create mode 100644 mobile/src/host-edit-route-accessibility.test.ts create mode 100644 mobile/src/host-edit-save-flow.test.ts create mode 100644 mobile/src/transport/host-endpoint.test.ts create mode 100644 mobile/src/transport/host-endpoint.ts create mode 100644 mobile/src/transport/host-store-endpoint.test.ts diff --git a/mobile/app/h/[hostId]/edit.tsx b/mobile/app/h/[hostId]/edit.tsx new file mode 100644 index 000000000..790c755cd --- /dev/null +++ b/mobile/app/h/[hostId]/edit.tsx @@ -0,0 +1,382 @@ +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { + View, + Text, + TextInput, + StyleSheet, + Pressable, + ActivityIndicator, + KeyboardAvoidingView, + Platform, + ScrollView +} from 'react-native' +import { useSafeAreaInsets } from 'react-native-safe-area-context' +import { useLocalSearchParams, useRouter } from 'expo-router' +import { ChevronLeft } from 'lucide-react-native' +import { colors, radii, spacing, typography } from '../../../src/theme/mobile-theme' +import { loadHosts, updateHostNameAndEndpoint } from '../../../src/transport/host-store' +import { + displayHostEndpoint, + endpointPort, + endpointScheme, + normalizeHostEndpoint +} from '../../../src/transport/host-endpoint' +import { useForceReconnect, usePrimeHosts } from '../../../src/transport/client-context' +import type { HostProfile } from '../../../src/transport/types' + +export default function EditHostScreen() { + const router = useRouter() + const insets = useSafeAreaInsets() + const { hostId } = useLocalSearchParams<{ hostId: string }>() + const primeHosts = usePrimeHosts() + const forceReconnectHost = useForceReconnect() + + const [host, setHost] = useState(null) + const [loadError, setLoadError] = useState(null) + const [name, setName] = useState('') + const [address, setAddress] = useState('') + const [saveError, setSaveError] = useState(null) + const [saving, setSaving] = useState(false) + // Why: setSaving is async, so a second trigger before the re-render could + // still read stale state and re-enter handleSave; the ref closes that race. + const savingRef = useRef(false) + + const load = useCallback(async () => { + if (!hostId) { + setLoadError('Missing host.') + return + } + try { + const hosts = await loadHosts() + const found = hosts.find((h) => h.id === hostId) ?? null + if (!found) { + setLoadError('This host was removed from this phone.') + setHost(null) + return + } + setHost(found) + setName(found.name) + setAddress(displayHostEndpoint(found.endpoint)) + setLoadError(null) + } catch (err) { + setLoadError(err instanceof Error ? err.message : 'Failed to load host.') + setHost(null) + } + }, [hostId]) + + useEffect(() => { + void load() + }, [load]) + + const fallbackPort = host ? endpointPort(host.endpoint) : undefined + const fallbackScheme = host ? endpointScheme(host.endpoint) : 'ws' + + const normalizedEndpoint = useMemo( + () => normalizeHostEndpoint(address, { fallbackPort, fallbackScheme }), + [address, fallbackPort, fallbackScheme] + ) + + const nameTrimmed = name.trim() + const nameChanged = host != null && nameTrimmed.length > 0 && nameTrimmed !== host.name + const endpointChanged = + host != null && normalizedEndpoint.ok && normalizedEndpoint.endpoint !== host.endpoint + const canSave = + host != null && + nameTrimmed.length > 0 && + normalizedEndpoint.ok && + (nameChanged || endpointChanged) && + !saving + + async function handleSave() { + if (!host || !hostId || savingRef.current) { + return + } + const nextName = name.trim() + if (!nextName) { + setSaveError('Enter a name.') + return + } + if (!normalizedEndpoint.ok) { + setSaveError(normalizedEndpoint.error) + return + } + + const willRename = nextName !== host.name + const willUpdateEndpoint = normalizedEndpoint.endpoint !== host.endpoint + if (!willRename && !willUpdateEndpoint) { + router.back() + return + } + + savingRef.current = true + setSaving(true) + setSaveError(null) + try { + // Why: a single mutateStoredHosts pass so name + endpoint commit + // 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 } : {}), + ...(willUpdateEndpoint ? { endpoint: normalizedEndpoint.endpoint } : {}) + }) + } catch (err) { + setSaveError(err instanceof Error ? err.message : 'Failed to save host.') + savingRef.current = false + setSaving(false) + return + } + + try { + // Why: the write already committed above; a re-prime failure here + // must not be reported as a save failure — the next loadHosts() call + // elsewhere in the app picks up the fresh state regardless. + const hosts = await loadHosts() + primeHosts(hosts) + } catch { + // best-effort re-prime; persisted data is unaffected + } + + savingRef.current = false + setSaving(false) + router.back() + + if (willUpdateEndpoint) { + // Why: reconnect is a follow-on side effect of a save that already + // committed — its failure or a hang must not be reported as a save + // failure or block navigating back. + void forceReconnectHost(host.id).catch(() => {}) + } + } + + return ( + + + router.back()} + accessibilityRole="button" + accessibilityLabel="Back" + > + + + Edit host + [ + styles.saveButton, + (!canSave || pressed) && styles.saveButtonDisabled + ]} + onPress={() => void handleSave()} + disabled={!canSave} + accessibilityRole="button" + accessibilityLabel="Save host" + > + {saving ? ( + + ) : ( + Save + )} + + + + {loadError ? ( + + {loadError} + router.back()}> + Go back + + + ) : !host ? ( + + + + ) : ( + + + + 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). + + + Name + { + setName(value) + setSaveError(null) + }} + placeholder="Host name" + placeholderTextColor={colors.textMuted} + autoCapitalize="words" + autoCorrect={false} + returnKeyType="next" + /> + + Address + { + setAddress(value) + setSaveError(null) + }} + placeholder="192.168.1.10:6768" + placeholderTextColor={colors.textMuted} + autoCapitalize="none" + autoCorrect={false} + autoComplete="off" + keyboardType="url" + returnKeyType="done" + onSubmitEditing={() => { + if (canSave) { + void handleSave() + } + }} + /> + + Accepts IP, host:port, or ws:// / wss://. Missing port defaults to the current port + (or 6768). + + + {normalizedEndpoint.ok ? ( + + Connects to {normalizedEndpoint.endpoint} + + ) : address.trim().length > 0 ? ( + {normalizedEndpoint.error} + ) : null} + + {saveError ? {saveError} : null} + + + )} + + ) +} + +const styles = StyleSheet.create({ + container: { + flex: 1, + backgroundColor: colors.bgBase + }, + flex: { + flex: 1 + }, + topRow: { + flexDirection: 'row', + alignItems: 'center', + paddingHorizontal: spacing.md, + paddingBottom: spacing.md, + gap: spacing.sm + }, + backButton: { + width: 36, + height: 36, + alignItems: 'center', + justifyContent: 'center' + }, + heading: { + flex: 1, + color: colors.textPrimary, + fontSize: 20, + fontWeight: '700' + }, + saveButton: { + minWidth: 64, + height: 34, + paddingHorizontal: spacing.md, + borderRadius: radii.button, + backgroundColor: colors.surfaceBright, + alignItems: 'center', + justifyContent: 'center' + }, + saveButtonDisabled: { + opacity: 0.4 + }, + saveButtonText: { + color: colors.bgBase, + fontSize: typography.bodySize, + fontWeight: '600' + }, + form: { + paddingHorizontal: spacing.lg, + gap: spacing.sm + }, + help: { + color: colors.textSecondary, + fontSize: typography.bodySize, + lineHeight: 20, + marginBottom: spacing.sm + }, + label: { + color: colors.textSecondary, + fontSize: typography.metaSize, + fontWeight: '500', + marginTop: spacing.sm, + textTransform: 'uppercase', + letterSpacing: 0.4 + }, + input: { + backgroundColor: colors.bgPanel, + borderWidth: 1, + borderColor: colors.borderSubtle, + borderRadius: radii.row, + color: colors.textPrimary, + fontSize: typography.bodySize, + paddingHorizontal: spacing.md, + paddingVertical: Platform.OS === 'ios' ? 12 : 10 + }, + hint: { + color: colors.textMuted, + fontSize: typography.metaSize, + lineHeight: 16 + }, + preview: { + marginTop: spacing.sm, + color: colors.textSecondary, + fontSize: typography.metaSize, + fontFamily: Platform.OS === 'ios' ? 'Menlo' : typography.monoFamily + }, + previewError: { + marginTop: spacing.sm, + color: colors.statusRed, + fontSize: typography.bodySize + }, + errorText: { + color: colors.statusRed, + fontSize: typography.bodySize, + marginTop: spacing.md + }, + errorState: { + flex: 1, + paddingHorizontal: spacing.lg, + paddingTop: spacing.xl, + gap: spacing.md + }, + loadingState: { + flex: 1, + alignItems: 'center', + justifyContent: 'center' + }, + secondaryButton: { + alignSelf: 'flex-start', + paddingHorizontal: spacing.md, + paddingVertical: spacing.sm, + borderRadius: radii.button, + backgroundColor: colors.bgRaised + }, + secondaryButtonText: { + color: colors.textPrimary, + fontSize: typography.bodySize, + fontWeight: '500' + } +}) diff --git a/mobile/app/h/_layout.tsx b/mobile/app/h/_layout.tsx index 55eade98e..f832b7c39 100644 --- a/mobile/app/h/_layout.tsx +++ b/mobile/app/h/_layout.tsx @@ -39,6 +39,7 @@ function HostStack({ animation }: { animation: 'none' | 'default' }) { }} > + diff --git a/mobile/app/index.tsx b/mobile/app/index.tsx index e26893afd..b33aac126 100644 --- a/mobile/app/index.tsx +++ b/mobile/app/index.tsx @@ -24,7 +24,7 @@ import { UsageBar } from '../src/components/AccountUsage' import AsyncStorage from '@react-native-async-storage/async-storage' -import { loadHosts, renameHost } from '../src/transport/host-store' +import { loadHosts } from '../src/transport/host-store' import { removeHostAndCloseClient } from '../src/transport/host-removal-lifecycle' import { pickResumeWorktree } from '../src/worktree/resume-worktree' import type { RpcClient } from '../src/transport/rpc-client' @@ -42,7 +42,6 @@ import { triggerMediumImpact } from '../src/platform/haptics' import { OrcaLogo } from '../src/components/OrcaLogo' import { MobileHostCard } from '../src/components/MobileHostCard' import { TaskProviderLogo } from '../src/components/TaskProviderLogo' -import { TextInputModal } from '../src/components/TextInputModal' import { ActionSheetModal, type ActionSheetAction } from '../src/components/ActionSheetModal' import { ConfirmModal } from '../src/components/ConfirmModal' import { setCachedWorktrees, getCachedWorktrees } from '../src/cache/worktree-cache' @@ -306,7 +305,6 @@ export default function HomeScreen() { const { isWideLayout, contentMaxWidth } = useResponsiveLayout() const [hosts, setHosts] = useState([]) const [actionTarget, setActionTarget] = useState(null) - const [renameTarget, setRenameTarget] = useState(null) const [confirmRemove, setConfirmRemove] = useState(null) const [hostStates, setHostStates] = useState>({}) const [hostAttempts, setHostAttempts] = useState>({}) @@ -708,19 +706,6 @@ export default function HomeScreen() { ) - async function handleRename(newName: string) { - if (!renameTarget) { - return - } - try { - await renameHost(renameTarget.id, newName) - setRenameTarget(null) - setHosts(await loadHosts()) - } catch { - setRenameTarget(null) - } - } - async function handleRemove() { if (!confirmRemove) { return @@ -1068,11 +1053,12 @@ export default function HomeScreen() { }) } items.push({ - label: 'Rename', + label: 'Edit host', icon: Edit3, closeBeforePress: true, onPress: () => { - setRenameTarget(host) + setActionTarget(null) + router.push(`/h/${host.id}/edit`) } }) items.push({ @@ -1088,16 +1074,6 @@ export default function HomeScreen() { onClose={() => setActionTarget(null)} /> - void handleRename(name)} - onCancel={() => setRenameTarget(null)} - /> - ({ + back: vi.fn(), + forceReconnectHost: vi.fn(), + loadHosts: vi.fn(), + primeHosts: vi.fn(), + updateHostNameAndEndpoint: vi.fn() +})) + +vi.mock('react-native', () => ({ + ActivityIndicator: 'ActivityIndicator', + KeyboardAvoidingView: 'KeyboardAvoidingView', + Platform: { OS: 'ios' }, + Pressable: 'Pressable', + ScrollView: 'ScrollView', + StyleSheet: { create: (styles: unknown) => styles }, + Text: 'Text', + TextInput: 'TextInput', + View: 'View' +})) + +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => ({ bottom: 0, left: 0, right: 0, top: 0 }) +})) + +vi.mock('expo-router', () => ({ + useLocalSearchParams: () => ({ hostId: 'host-1' }), + useRouter: () => ({ back: dependencies.back }) +})) + +vi.mock('lucide-react-native', () => ({ + ChevronLeft: 'ChevronLeft' +})) + +vi.mock('./transport/host-store', () => ({ + loadHosts: dependencies.loadHosts, + updateHostNameAndEndpoint: dependencies.updateHostNameAndEndpoint +})) + +vi.mock('./transport/client-context', () => ({ + useForceReconnect: () => dependencies.forceReconnectHost, + usePrimeHosts: () => dependencies.primeHosts +})) + +function suppressReactTestRendererDeprecationWarning(): () => void { + const originalConsoleError = console.error + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { + const firstArg = args[0] + if (typeof firstArg === 'string' && firstArg.includes('react-test-renderer is deprecated')) { + return + } + originalConsoleError(...args) + }) + return () => consoleErrorSpy.mockRestore() +} + +async function renderEditHostRoute(): Promise { + let renderer: ReactTestRenderer | null = null + const restoreConsoleError = suppressReactTestRendererDeprecationWarning() + try { + await act(async () => { + renderer = create(createElement(EditHostScreen)) + await Promise.resolve() + }) + } finally { + restoreConsoleError() + } + if (!renderer) { + throw new Error('Edit host route did not render') + } + return renderer +} + +describe('edit host route accessibility', () => { + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + dependencies.loadHosts.mockReset().mockResolvedValue([ + { + id: 'host-1', + name: 'Desk', + endpoint: 'ws://192.168.1.10:6768', + deviceToken: 'token', + publicKeyB64: 'public-key', + lastConnected: 1 + } + ]) + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + it('exposes stable accessible names for both editable fields', async () => { + const renderer = await renderEditHostRoute() + + const inputs = renderer.root.findAllByType('TextInput') + expect(inputs).toHaveLength(2) + expect(inputs.map((input) => input.props.accessibilityLabel)).toEqual(['Name', 'Address']) + + act(() => renderer.unmount()) + }) +}) diff --git a/mobile/src/host-edit-save-flow.test.ts b/mobile/src/host-edit-save-flow.test.ts new file mode 100644 index 000000000..1e1c25240 --- /dev/null +++ b/mobile/src/host-edit-save-flow.test.ts @@ -0,0 +1,325 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import EditHostScreen from '../app/h/[hostId]/edit' + +const dependencies = vi.hoisted(() => ({ + back: vi.fn(), + forceReconnectHost: vi.fn(), + loadHosts: vi.fn(), + primeHosts: vi.fn(), + updateHostNameAndEndpoint: vi.fn(), + hostId: 'host-1' as string | undefined +})) + +vi.mock('react-native', () => ({ + ActivityIndicator: 'ActivityIndicator', + KeyboardAvoidingView: 'KeyboardAvoidingView', + Platform: { OS: 'ios' }, + Pressable: 'Pressable', + ScrollView: 'ScrollView', + StyleSheet: { create: (styles: unknown) => styles }, + Text: 'Text', + TextInput: 'TextInput', + View: 'View' +})) + +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => ({ bottom: 0, left: 0, right: 0, top: 0 }) +})) + +vi.mock('expo-router', () => ({ + useLocalSearchParams: () => ({ hostId: dependencies.hostId }), + useRouter: () => ({ back: dependencies.back }) +})) + +vi.mock('lucide-react-native', () => ({ + ChevronLeft: 'ChevronLeft' +})) + +vi.mock('./transport/host-store', () => ({ + loadHosts: dependencies.loadHosts, + updateHostNameAndEndpoint: dependencies.updateHostNameAndEndpoint +})) + +vi.mock('./transport/client-context', () => ({ + useForceReconnect: () => dependencies.forceReconnectHost, + usePrimeHosts: () => dependencies.primeHosts +})) + +const HOST_FIXTURE = { + id: 'host-1', + name: 'Desk', + endpoint: 'ws://192.168.1.10:6768', + deviceToken: 'token', + publicKeyB64: 'public-key', + lastConnected: 1 +} + +function suppressReactTestRendererDeprecationWarning(): () => void { + const originalConsoleError = console.error + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { + const firstArg = args[0] + if (typeof firstArg === 'string' && firstArg.includes('react-test-renderer is deprecated')) { + return + } + originalConsoleError(...args) + }) + return () => consoleErrorSpy.mockRestore() +} + +async function renderEditHostRoute(): Promise { + let renderer: ReactTestRenderer | null = null + const restoreConsoleError = suppressReactTestRendererDeprecationWarning() + try { + await act(async () => { + renderer = create(createElement(EditHostScreen)) + await Promise.resolve() + }) + } finally { + restoreConsoleError() + } + if (!renderer) { + throw new Error('Edit host route did not render') + } + return renderer +} + +function setFieldValue( + renderer: ReactTestRenderer, + accessibilityLabel: 'Name' | 'Address', + value: string +): void { + const input = renderer.root + .findAllByType('TextInput') + .find((node) => node.props.accessibilityLabel === accessibilityLabel) + if (!input) { + throw new Error(`${accessibilityLabel} input not found`) + } + act(() => { + input.props.onChangeText(value) + }) +} + +function findSaveButton(renderer: ReactTestRenderer) { + const button = renderer.root + .findAllByType('Pressable') + .find((node) => node.props.accessibilityLabel === 'Save host') + if (!button) { + throw new Error('Save button not found') + } + return button +} + +async function pressSave(renderer: ReactTestRenderer): Promise { + const button = findSaveButton(renderer) + await act(async () => { + button.props.onPress() + await Promise.resolve() + await Promise.resolve() + }) +} + +function findText(renderer: ReactTestRenderer, match: string): boolean { + return renderer.root.findAllByType('Text').some((node) => { + const children = node.props.children + if (typeof children === 'string') { + return children.includes(match) + } + if (Array.isArray(children)) { + return children.join('').includes(match) + } + return false + }) +} + +describe('edit host handleSave', () => { + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + dependencies.hostId = 'host-1' + dependencies.back.mockReset() + dependencies.forceReconnectHost.mockReset().mockResolvedValue(undefined) + dependencies.loadHosts.mockReset().mockResolvedValue([HOST_FIXTURE]) + dependencies.primeHosts.mockReset() + dependencies.updateHostNameAndEndpoint.mockReset().mockResolvedValue(undefined) + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + it('rename-only save updates only the name and does not reconnect', async () => { + const renderer = await renderEditHostRoute() + setFieldValue(renderer, 'Name', 'Home Desk') + await pressSave(renderer) + + expect(dependencies.updateHostNameAndEndpoint).toHaveBeenCalledWith('host-1', { + name: 'Home Desk' + }) + expect(dependencies.forceReconnectHost).not.toHaveBeenCalled() + expect(dependencies.back).toHaveBeenCalledTimes(1) + + act(() => renderer.unmount()) + }) + + it('endpoint-only save updates only the endpoint and reconnects', async () => { + const renderer = await renderEditHostRoute() + setFieldValue(renderer, 'Address', '192.168.1.20:6768') + await pressSave(renderer) + + expect(dependencies.updateHostNameAndEndpoint).toHaveBeenCalledWith('host-1', { + endpoint: 'ws://192.168.1.20:6768' + }) + expect(dependencies.forceReconnectHost).toHaveBeenCalledWith('host-1') + expect(dependencies.back).toHaveBeenCalledTimes(1) + + act(() => renderer.unmount()) + }) + + it('saves name and endpoint together in one call, then reconnects', async () => { + const renderer = await renderEditHostRoute() + setFieldValue(renderer, 'Name', 'Home Desk') + setFieldValue(renderer, 'Address', '192.168.1.20:6768') + await pressSave(renderer) + + expect(dependencies.updateHostNameAndEndpoint).toHaveBeenCalledTimes(1) + expect(dependencies.updateHostNameAndEndpoint).toHaveBeenCalledWith('host-1', { + name: 'Home Desk', + endpoint: 'ws://192.168.1.20:6768' + }) + expect(dependencies.forceReconnectHost).toHaveBeenCalledWith('host-1') + expect(dependencies.back).toHaveBeenCalledTimes(1) + + act(() => renderer.unmount()) + }) + + it('navigates back without saving or reconnecting when nothing changed', async () => { + const renderer = await renderEditHostRoute() + await pressSave(renderer) + + expect(dependencies.updateHostNameAndEndpoint).not.toHaveBeenCalled() + expect(dependencies.forceReconnectHost).not.toHaveBeenCalled() + expect(dependencies.back).toHaveBeenCalledTimes(1) + + act(() => renderer.unmount()) + }) + + it('shows the error and does not navigate back or reconnect when the save rejects', async () => { + dependencies.updateHostNameAndEndpoint.mockRejectedValueOnce(new Error('Host not found')) + const renderer = await renderEditHostRoute() + setFieldValue(renderer, 'Name', 'Home Desk') + await pressSave(renderer) + + expect(findText(renderer, 'Host not found')).toBe(true) + expect(dependencies.forceReconnectHost).not.toHaveBeenCalled() + expect(dependencies.back).not.toHaveBeenCalled() + + act(() => renderer.unmount()) + }) + + it('still navigates back when the post-save re-prime fails', async () => { + dependencies.loadHosts + .mockResolvedValueOnce([HOST_FIXTURE]) + .mockRejectedValueOnce(new Error('boom')) + const renderer = await renderEditHostRoute() + setFieldValue(renderer, 'Name', 'Home Desk') + await pressSave(renderer) + + expect(dependencies.primeHosts).not.toHaveBeenCalled() + expect(dependencies.back).toHaveBeenCalledTimes(1) + + act(() => renderer.unmount()) + }) + + it('still navigates back and shows no error when the post-save reconnect rejects', async () => { + dependencies.forceReconnectHost.mockRejectedValueOnce(new Error('connect failed')) + const renderer = await renderEditHostRoute() + setFieldValue(renderer, 'Address', '192.168.1.20:6768') + await pressSave(renderer) + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + + expect(dependencies.forceReconnectHost).toHaveBeenCalledWith('host-1') + expect(dependencies.back).toHaveBeenCalledTimes(1) + expect(findText(renderer, 'connect failed')).toBe(false) + + act(() => renderer.unmount()) + }) + + it('ignores a second Save trigger while a save is already in flight', async () => { + let resolveSave: () => void = () => {} + dependencies.updateHostNameAndEndpoint.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSave = resolve + }) + ) + const renderer = await renderEditHostRoute() + setFieldValue(renderer, 'Name', 'Home Desk') + const button = findSaveButton(renderer) + + await act(async () => { + button.props.onPress() + button.props.onPress() + await Promise.resolve() + }) + + expect(dependencies.updateHostNameAndEndpoint).toHaveBeenCalledTimes(1) + + await act(async () => { + resolveSave() + await Promise.resolve() + await Promise.resolve() + }) + + act(() => renderer.unmount()) + }) +}) + +describe('edit host load() error states', () => { + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + dependencies.hostId = 'host-1' + dependencies.back.mockReset() + dependencies.forceReconnectHost.mockReset().mockResolvedValue(undefined) + dependencies.loadHosts.mockReset().mockResolvedValue([HOST_FIXTURE]) + dependencies.primeHosts.mockReset() + dependencies.updateHostNameAndEndpoint.mockReset().mockResolvedValue(undefined) + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + it('shows "Missing host." when hostId is absent', async () => { + dependencies.hostId = undefined + const renderer = await renderEditHostRoute() + + expect(findText(renderer, 'Missing host.')).toBe(true) + expect(renderer.root.findAllByType('TextInput')).toHaveLength(0) + + act(() => renderer.unmount()) + }) + + it('shows a not-saved message when the host is not in the loaded list', async () => { + dependencies.loadHosts.mockReset().mockResolvedValue([]) + const renderer = await renderEditHostRoute() + + expect(findText(renderer, 'This host was removed from this phone.')).toBe(true) + expect(renderer.root.findAllByType('TextInput')).toHaveLength(0) + + act(() => renderer.unmount()) + }) + + it('surfaces the error message when loadHosts rejects', async () => { + dependencies.loadHosts.mockReset().mockRejectedValue(new Error('storage unreadable')) + const renderer = await renderEditHostRoute() + + expect(findText(renderer, 'storage unreadable')).toBe(true) + expect(renderer.root.findAllByType('TextInput')).toHaveLength(0) + + act(() => renderer.unmount()) + }) +}) diff --git a/mobile/src/transport/host-endpoint.test.ts b/mobile/src/transport/host-endpoint.test.ts new file mode 100644 index 000000000..ee904278e --- /dev/null +++ b/mobile/src/transport/host-endpoint.test.ts @@ -0,0 +1,257 @@ +import { describe, expect, it } from 'vitest' +import { + displayHostEndpoint, + endpointPort, + endpointScheme, + normalizeHostEndpoint +} from './host-endpoint' + +describe('displayHostEndpoint', () => { + it('shows host:port for websocket URLs', () => { + expect(displayHostEndpoint('ws://192.168.1.10:6768')).toBe('192.168.1.10:6768') + // Why: URL omits default scheme ports (443 for wss); use a non-default port. + expect(displayHostEndpoint('wss://desk.example:8443')).toBe('desk.example:8443') + }) + + it('returns the raw string when not a URL', () => { + expect(displayHostEndpoint('not-a-url')).toBe('not-a-url') + }) + + it('brackets IPv6 hostnames for round-trip safety', () => { + expect(displayHostEndpoint('ws://[fd7a:115c:a1e0::1]:6768')).toBe('[fd7a:115c:a1e0::1]:6768') + }) +}) + +describe('endpointPort', () => { + it('reads the port when present', () => { + expect(endpointPort('ws://192.168.1.10:6768')).toBe('6768') + }) + + it('returns undefined when the port is omitted', () => { + expect(endpointPort('ws://192.168.1.10')).toBeUndefined() + }) + + it('preserves scheme-default ports that URL.port hides', () => { + expect(endpointPort('ws://192.168.1.10:80')).toBe('80') + expect(endpointPort('wss://desk.example:443')).toBe('443') + }) +}) + +describe('endpointScheme', () => { + it('returns wss for a wss:// endpoint', () => { + expect(endpointScheme('wss://desk.example:8443')).toBe('wss') + }) + + it('returns ws for a ws:// endpoint', () => { + expect(endpointScheme('ws://192.168.1.10:6768')).toBe('ws') + }) + + it('falls back to ws for a non-URL endpoint', () => { + expect(endpointScheme('not-a-url')).toBe('ws') + }) +}) + +describe('normalizeHostEndpoint', () => { + it('accepts a full ws URL', () => { + expect(normalizeHostEndpoint('ws://100.64.0.5:6768')).toEqual({ + ok: true, + endpoint: 'ws://100.64.0.5:6768' + }) + }) + + it('accepts wss and preserves scheme', () => { + expect(normalizeHostEndpoint('wss://desk.example:8443')).toEqual({ + ok: true, + endpoint: 'wss://desk.example:8443' + }) + }) + + it('accepts bare host:port', () => { + expect(normalizeHostEndpoint('192.168.1.10:6768')).toEqual({ + ok: true, + endpoint: 'ws://192.168.1.10:6768' + }) + }) + + it('defaults missing port to 6768', () => { + expect(normalizeHostEndpoint('192.168.1.10')).toEqual({ + ok: true, + endpoint: 'ws://192.168.1.10:6768' + }) + }) + + it('uses fallbackPort when the user omits the port', () => { + expect(normalizeHostEndpoint('mac-mini.local', { fallbackPort: '7777' })).toEqual({ + ok: true, + endpoint: 'ws://mac-mini.local:7777' + }) + }) + + it('fills missing port on scheme URLs from fallbackPort', () => { + expect(normalizeHostEndpoint('ws://192.168.1.10', { fallbackPort: '9000' })).toEqual({ + ok: true, + endpoint: 'ws://192.168.1.10:9000' + }) + }) + + it('preserves explicit ws :80 and wss :443 instead of rewriting to fallback', () => { + expect(normalizeHostEndpoint('ws://192.168.1.10:80', { fallbackPort: '6768' })).toEqual({ + ok: true, + endpoint: 'ws://192.168.1.10:80' + }) + expect(normalizeHostEndpoint('wss://desk.example:443', { fallbackPort: '6768' })).toEqual({ + ok: true, + endpoint: 'wss://desk.example:443' + }) + expect(displayHostEndpoint('ws://192.168.1.10:80')).toBe('192.168.1.10:80') + expect(displayHostEndpoint('wss://desk.example:443')).toBe('desk.example:443') + }) + + it('trims whitespace', () => { + expect(normalizeHostEndpoint(' 10.0.0.2:6768 ')).toEqual({ + ok: true, + endpoint: 'ws://10.0.0.2:6768' + }) + }) + + it('rejects empty input', () => { + expect(normalizeHostEndpoint(' ')).toEqual({ + ok: false, + error: 'Enter a host address.' + }) + }) + + it('rejects non-websocket schemes', () => { + expect(normalizeHostEndpoint('http://192.168.1.10:6768')).toEqual({ + ok: false, + error: 'Use ws:// or wss:// (or host:port).' + }) + }) + + it('rejects explicit invalid ports without falling back', () => { + for (const input of [ + '192.168.1.10:0', + '192.168.1.10:99999', + 'ws://192.168.1.10:0', + 'ws://192.168.1.10:99999' + ]) { + expect(normalizeHostEndpoint(input, { fallbackPort: '6768' })).toEqual({ + ok: false, + error: 'Port must be 1–65535.' + }) + } + }) + + it('rejects bare hosts with path, query, or spaces', () => { + expect(normalizeHostEndpoint('desk/path')).toEqual({ + ok: false, + error: 'Not a valid hostname.' + }) + expect(normalizeHostEndpoint('desk?route')).toEqual({ + ok: false, + error: 'Not a valid hostname.' + }) + expect(normalizeHostEndpoint('desk name')).toEqual({ + ok: false, + error: 'Not a valid hostname.' + }) + }) + + it('rejects scheme URLs with path or query', () => { + expect(normalizeHostEndpoint('ws://desk.example/path')).toEqual({ + ok: false, + error: 'Host must not include a path or query.' + }) + expect(normalizeHostEndpoint('ws://desk.example?route=1')).toEqual({ + ok: false, + error: 'Host must not include a path or query.' + }) + }) + + it('accepts bracketed IPv6 with port', () => { + expect(normalizeHostEndpoint('[fd7a:115c:a1e0::1]:6768')).toEqual({ + ok: true, + endpoint: 'ws://[fd7a:115c:a1e0::1]:6768' + }) + }) + + it('rejects malformed IPv6 that a WebSocket URL cannot parse', () => { + expect(normalizeHostEndpoint('[1::2::3]:6768')).toEqual({ + ok: false, + error: 'Not a valid hostname.' + }) + }) + + it('rejects non-canonical numeric IPv4 forms in bare addresses', () => { + for (const host of ['999.999.999.999', '010.0.0.1', '127.1', '0x7f000001']) { + expect(normalizeHostEndpoint(`${host}:6768`)).toEqual({ + ok: false, + error: 'Not a valid hostname.' + }) + } + }) + + it('rejects non-canonical numeric IPv4 forms before URL normalization', () => { + for (const host of ['999.999.999.999', '010.0.0.1', '127.1', '0x7f000001']) { + expect(normalizeHostEndpoint(`ws://${host}:6768`)).toEqual({ + ok: false, + error: 'Not a valid hostname.' + }) + } + }) + + it('rejects encoded and trailing-dot numeric IPv4 aliases', () => { + for (const input of ['ws://127.1.:6768', 'wss://127.1.:6768', 'ws://%31%32%37.1:6768']) { + expect(normalizeHostEndpoint(input)).toEqual({ + ok: false, + error: 'Not a valid hostname.' + }) + } + }) + + it('rejects empty userinfo before URL normalization', () => { + expect(normalizeHostEndpoint('ws://@127.1:6768')).toEqual({ + ok: false, + error: 'Not a valid address.' + }) + }) + + it('reports a path or query before validating a numeric hostname alias', () => { + for (const input of ['ws://127.1/path', 'ws://127.1?route=1']) { + expect(normalizeHostEndpoint(input)).toEqual({ + ok: false, + error: 'Host must not include a path or query.' + }) + } + }) + + it('keeps ordinary DNS names that contain numeric labels', () => { + expect(normalizeHostEndpoint('desk123.local:6768')).toEqual({ + ok: true, + endpoint: 'ws://desk123.local:6768' + }) + expect(normalizeHostEndpoint('ws://123.example:6768')).toEqual({ + ok: true, + endpoint: 'ws://123.example:6768' + }) + }) + + it('preserves wss when re-normalizing bare host:port via fallbackScheme', () => { + expect( + normalizeHostEndpoint('desk.example:8443', { fallbackScheme: 'wss', fallbackPort: '8443' }) + ).toEqual({ + ok: true, + endpoint: 'wss://desk.example:8443' + }) + }) + + it('round-trips displayed IPv6 endpoints without port corruption', () => { + const original = 'ws://[fd7a:115c:a1e0::1]:6768' + const displayed = displayHostEndpoint(original) + expect(displayed).toBe('[fd7a:115c:a1e0::1]:6768') + expect(normalizeHostEndpoint(displayed, { fallbackPort: '6768' })).toEqual({ + ok: true, + endpoint: original + }) + }) +}) diff --git a/mobile/src/transport/host-endpoint.ts b/mobile/src/transport/host-endpoint.ts new file mode 100644 index 000000000..68d77aa4b --- /dev/null +++ b/mobile/src/transport/host-endpoint.ts @@ -0,0 +1,365 @@ +// Why: mobile host profiles store a single websocket endpoint fixed at pair +// time. Edit-host lets the user rewrite host/port without re-pairing; this +// helper accepts phone-friendly input (bare IP, host:port, or full ws URL) +// and normalizes to the ws(s):// form RpcClient expects. + +export type NormalizeHostEndpointResult = + | { ok: true; endpoint: string } + | { ok: false; error: string } + +type WebsocketUrlPortResolution = + | { kind: 'missing' } + | { kind: 'valid'; port: string } + | { kind: 'invalid' } + +type RawSchemeAuthority = { + hostname: string | null + hasUserInfo: boolean + hasPathOrQuery: boolean +} + +const DEFAULT_PORT = '6768' +const NUMERIC_IPV4_CANDIDATE = /^(?:0[xX][0-9a-fA-F]+|\d+)(?:\.(?:0[xX][0-9a-fA-F]+|\d+))*$/ + +export function displayHostEndpoint(endpoint: string): string { + try { + const url = new URL(endpoint) + // Why: some URL parsers leave IPv6 brackets on hostname, others strip them. + // Normalize once so round-trip through normalizeHostEndpoint stays stable. + const host = formatHostForUrl(unwrapHostname(url.hostname)) + const port = resolveWebsocketUrlPort(endpoint, url) + if (port.kind === 'invalid') { + return endpoint + } + return port.kind === 'valid' ? `${host}:${port.port}` : host + } catch { + return endpoint + } +} + +function unwrapHostname(hostname: string): string { + return hostname.startsWith('[') && hostname.endsWith(']') ? hostname.slice(1, -1) : hostname +} + +/** + * Recover an explicitly written port from a ws(s) URL authority. + * Why: `new URL('ws://host:80').port` and `wss://host:443` are empty — the + * URL parser hides scheme-default ports, so callers that need the user's + * literal :80/:443 must re-parse the original string. + */ +function extractExplicitPortFromWebsocketUrl(input: string): string | null { + const withoutScheme = input.replace(/^[a-zA-Z][a-zA-Z0-9+.-]*:\/\//, '') + if (withoutScheme.startsWith('[')) { + const close = withoutScheme.indexOf(']') + if (close <= 1) { + return null + } + const rest = withoutScheme.slice(close + 1) + const match = /^:(\d+)(?=[/?#]|$)/.exec(rest) + return match?.[1] ?? null + } + const end = withoutScheme.search(/[/?#]/) + const authority = end === -1 ? withoutScheme : withoutScheme.slice(0, end) + const at = authority.lastIndexOf('@') + const hostPort = at === -1 ? authority : authority.slice(at + 1) + const match = /:(\d+)$/.exec(hostPort) + return match?.[1] ?? null +} + +function resolveWebsocketUrlPort(input: string, url?: URL): WebsocketUrlPortResolution { + const explicit = extractExplicitPortFromWebsocketUrl(input) + // Why: missing and invalid are different states. Treating both as null lets + // an explicit :0/:99999 silently inherit fallbackPort on permissive parsers. + if (explicit !== null && !isValidPort(explicit)) { + return { kind: 'invalid' } + } + if (url?.port) { + return isValidPort(url.port) ? { kind: 'valid', port: url.port } : { kind: 'invalid' } + } + if (explicit !== null) { + return { kind: 'valid', port: explicit } + } + return { kind: 'missing' } +} + +export function endpointPort(endpoint: string): string | undefined { + try { + const url = new URL(endpoint) + const port = resolveWebsocketUrlPort(endpoint, url) + return port.kind === 'valid' ? port.port : undefined + } catch { + return undefined + } +} + +export function endpointScheme(endpoint: string): 'ws' | 'wss' { + try { + const protocol = new URL(endpoint).protocol.replace(':', '') + return protocol === 'wss' ? 'wss' : 'ws' + } catch { + return 'ws' + } +} + +export function normalizeHostEndpoint( + input: string, + options?: { fallbackPort?: string | number; fallbackScheme?: 'ws' | 'wss' } +): NormalizeHostEndpointResult { + const trimmed = input.trim() + if (!trimmed) { + return { ok: false, error: 'Enter a host address.' } + } + + const fallbackPort = resolveFallbackPort(options?.fallbackPort) + const fallbackScheme = options?.fallbackScheme === 'wss' ? 'wss' : 'ws' + + if (/^[a-zA-Z][a-zA-Z0-9+.-]*:\/\//.test(trimmed)) { + return normalizeSchemeUrl(trimmed, fallbackPort) + } + + return normalizeHostPort(trimmed, fallbackPort, fallbackScheme) +} + +function resolveFallbackPort(value: string | number | undefined): string { + if (value == null) { + return DEFAULT_PORT + } + const asString = String(value).trim() + if (!asString || !isValidPort(asString)) { + return DEFAULT_PORT + } + return asString +} + +function normalizeSchemeUrl(input: string, fallbackPort: string): NormalizeHostEndpointResult { + const explicitPort = resolveWebsocketUrlPort(input) + if (explicitPort.kind === 'invalid') { + return { ok: false, error: 'Port must be 1–65535.' } + } + const scheme = /^([a-zA-Z][a-zA-Z0-9+.-]*):\/\//.exec(input)?.[1]?.toLowerCase() + if (scheme !== 'ws' && scheme !== 'wss') { + return { ok: false, error: 'Use ws:// or wss:// (or host:port).' } + } + + const rawAuthority = parseRawSchemeAuthority(input) + if (rawAuthority.hasUserInfo) { + return { ok: false, error: 'Not a valid address.' } + } + if (rawAuthority.hasPathOrQuery) { + return { ok: false, error: 'Host must not include a path or query.' } + } + if ( + rawAuthority.hostname && + validateNumericIpv4Candidate(normalizeRawNumericIpv4Candidate(rawAuthority.hostname)) + ) { + return { ok: false, error: 'Not a valid hostname.' } + } + + let url: URL + try { + url = new URL(input) + } catch { + return { ok: false, error: 'Not a valid address.' } + } + + if (url.protocol !== 'ws:' && url.protocol !== 'wss:') { + return { ok: false, error: 'Use ws:// or wss:// (or host:port).' } + } + if (!url.hostname) { + return { ok: false, error: 'Missing hostname.' } + } + + // Why: edit-host persists a bare host:port WebSocket endpoint. Path/query/ + // userinfo are not part of the pairing contract — reject rather than strip + // so typos like desk/path or desk?route cannot be saved silently. + if (url.username || url.password) { + return { ok: false, error: 'Not a valid address.' } + } + if ((url.pathname && url.pathname !== '/') || url.search || url.hash) { + return { ok: false, error: 'Host must not include a path or query.' } + } + + const hostname = unwrapHostname(url.hostname) + // Why: WHATWG URL accepts legacy aliases and rewrites them to a different + // IPv4 address. Only an already-canonical raw dotted quad may become IPv4. + if (rawAuthority.hostname && isCanonicalIpv4(hostname) && rawAuthority.hostname !== hostname) { + return { ok: false, error: 'Not a valid hostname.' } + } + const hostError = validateHostname(hostname) + if (hostError) { + return { ok: false, error: hostError } + } + + // Why: keep explicit :80/:443 (URL.port is empty for scheme defaults) instead + // of rewriting them to fallbackPort (usually 6768). + const resolvedPort = resolveWebsocketUrlPort(input, url) + if (resolvedPort.kind === 'invalid') { + return { ok: false, error: 'Port must be 1–65535.' } + } + const port = resolvedPort.kind === 'valid' ? resolvedPort.port : fallbackPort + + // Why: rebuild so accidental whitespace never reaches the WebSocket constructor. + return { ok: true, endpoint: `${url.protocol}//${formatHostForUrl(hostname)}:${port}` } +} + +function parseRawSchemeAuthority(input: string): RawSchemeAuthority { + const schemeEnd = input.indexOf('://') + const remainder = input.slice(schemeEnd + 3) + const authorityEnd = remainder.search(/[/?#]/) + const authority = authorityEnd === -1 ? remainder : remainder.slice(0, authorityEnd) + const suffix = authorityEnd === -1 ? '' : remainder.slice(authorityEnd) + const hasUserInfo = authority.includes('@') + const hostPort = hasUserInfo ? authority.slice(authority.lastIndexOf('@') + 1) : authority + if (!hostPort) { + return { hostname: null, hasUserInfo, hasPathOrQuery: suffix !== '' && suffix !== '/' } + } + if (hostPort.startsWith('[')) { + const close = hostPort.indexOf(']') + return { + hostname: close > 1 ? hostPort.slice(1, close) : null, + hasUserInfo, + hasPathOrQuery: suffix !== '' && suffix !== '/' + } + } + const lastColon = hostPort.lastIndexOf(':') + return { + hostname: lastColon === -1 ? hostPort : hostPort.slice(0, lastColon), + hasUserInfo, + hasPathOrQuery: suffix !== '' && suffix !== '/' + } +} + +function normalizeHostPort( + input: string, + fallbackPort: string, + fallbackScheme: 'ws' | 'wss' +): NormalizeHostEndpointResult { + let host: string + let port: string | undefined + + if (input.startsWith('[')) { + const close = input.indexOf(']') + if (close <= 1) { + return { ok: false, error: 'Not a valid address.' } + } + host = input.slice(1, close) + const rest = input.slice(close + 1) + if (rest.startsWith(':')) { + port = rest.slice(1) + } else if (rest.length > 0) { + return { ok: false, error: 'Not a valid address.' } + } + } else { + const firstColon = input.indexOf(':') + const lastColon = input.lastIndexOf(':') + if (firstColon !== -1 && firstColon === lastColon) { + host = input.slice(0, firstColon) + port = input.slice(firstColon + 1) + } else { + // No port, or bare IPv6 (multiple colons, no brackets). + host = input + } + } + + host = host.trim() + if (!host) { + return { ok: false, error: 'Missing hostname.' } + } + + // Why: bare input is not a URL, so characters that only make sense in a URL + // (path, query, fragment, whitespace) must not be treated as hostname bytes. + const hostError = validateHostname(host) + if (hostError) { + return { ok: false, error: hostError } + } + + if (port !== undefined) { + port = port.trim() + if (!isValidPort(port)) { + return { ok: false, error: 'Port must be 1–65535.' } + } + } + + const finalPort = port ?? fallbackPort + return { ok: true, endpoint: `${fallbackScheme}://${formatHostForUrl(host)}:${finalPort}` } +} + +function formatHostForUrl(host: string): string { + return host.includes(':') && !host.startsWith('[') ? `[${host}]` : host +} + +/** + * Reject hostnames that would be illegal or ambiguous in a websocket URL. + * Allows DNS labels, `.local` mDNS, IPv4, and IPv6 hex forms. + */ +function validateHostname(host: string): string | null { + if (!host) { + return 'Missing hostname.' + } + // Spaces, path/query/fragment separators, userinfo separators, brackets. + if (/[\s/?#@[\]]/.test(host)) { + return 'Not a valid hostname.' + } + const numericIpv4Error = validateNumericIpv4Candidate(host) + if (numericIpv4Error) { + return numericIpv4Error + } + if (host.includes(':')) { + // Why: a hex/colon regex accepts malformed forms such as two `::` runs. + // Reuse the URL parser that WebSocket will ultimately use. + if (!/^[0-9a-fA-F:]+$/.test(host)) { + return 'Not a valid hostname.' + } + try { + new URL(`ws://[${host}]:${DEFAULT_PORT}`) + } catch { + return 'Not a valid hostname.' + } + return null + } + // DNS / IPv4 / mDNS: labels of alnum and hyphen, dots between, no empty labels. + if ( + !/^[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/.test( + host + ) + ) { + return 'Not a valid hostname.' + } + return null +} + +function validateNumericIpv4Candidate(host: string): string | null { + if (!NUMERIC_IPV4_CANDIDATE.test(host)) { + return null + } + if (!isCanonicalIpv4(host)) { + return 'Not a valid hostname.' + } + return null +} + +function normalizeRawNumericIpv4Candidate(host: string): string { + let decoded = host + try { + decoded = decodeURIComponent(host) + } catch { + // The URL parser will reject malformed escapes; keep them untouched here. + } + return decoded.endsWith('.') ? decoded.slice(0, -1) : decoded +} + +function isCanonicalIpv4(host: string): boolean { + const octets = host.split('.') + return ( + octets.length === 4 && + octets.every((octet) => /^(?:0|[1-9]\d{0,2})$/.test(octet) && Number(octet) <= 255) + ) +} + +function isValidPort(port: string): boolean { + if (!/^\d+$/.test(port)) { + return false + } + const n = Number(port) + return n >= 1 && n <= 65535 +} diff --git a/mobile/src/transport/host-store-endpoint.test.ts b/mobile/src/transport/host-store-endpoint.test.ts new file mode 100644 index 000000000..95d9c364b --- /dev/null +++ b/mobile/src/transport/host-store-endpoint.test.ts @@ -0,0 +1,95 @@ +import AsyncStorage from '@react-native-async-storage/async-storage' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { updateHostNameAndEndpoint } from './host-store' + +vi.mock('@react-native-async-storage/async-storage', () => ({ + default: { + getItem: vi.fn(), + setItem: vi.fn(), + removeItem: vi.fn() + } +})) + +vi.mock('expo-secure-store', () => ({ + getItemAsync: vi.fn(), + setItemAsync: vi.fn(), + deleteItemAsync: vi.fn(), + WHEN_UNLOCKED_THIS_DEVICE_ONLY: 'WHEN_UNLOCKED_THIS_DEVICE_ONLY' +})) + +vi.mock('react-native', () => ({ + Platform: { OS: 'ios' } +})) + +describe('updateHostNameAndEndpoint', () => { + beforeEach(() => { + vi.mocked(AsyncStorage.getItem).mockReset() + vi.mocked(AsyncStorage.setItem).mockReset() + }) + + const stored = [ + { + id: 'host-1', + name: 'Desk', + endpoint: 'ws://100.64.0.5:6768', + publicKeyB64: 'pk', + lastConnected: 1 + }, + { + id: 'host-2', + name: 'Laptop', + endpoint: 'wss://laptop.example:8443', + publicKeyB64: 'pk-2', + lastConnected: 2 + } + ] + + it('commits name and endpoint together in a single write', async () => { + vi.mocked(AsyncStorage.getItem).mockResolvedValue(JSON.stringify(stored)) + + await updateHostNameAndEndpoint('host-1', { + name: '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] + ]) + ) + }) + + it('updates only the provided field', async () => { + vi.mocked(AsyncStorage.getItem).mockResolvedValue(JSON.stringify(stored)) + + await updateHostNameAndEndpoint('host-1', { name: 'Home Desk' }) + + expect(AsyncStorage.setItem).toHaveBeenCalledWith( + 'orca:hosts', + JSON.stringify([{ ...stored[0], name: 'Home Desk' }, stored[1]]) + ) + }) + + it('rewrites only the endpoint when 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]]) + ) + }) + + it('throws and writes nothing when the host is missing', async () => { + vi.mocked(AsyncStorage.getItem).mockResolvedValue('[]') + + await expect(updateHostNameAndEndpoint('missing', { name: '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 17246f57e..918e6e92f 100644 --- a/mobile/src/transport/host-store.test.ts +++ b/mobile/src/transport/host-store.test.ts @@ -37,11 +37,11 @@ import { loadHosts, MobileRelayUpgradeHostRemovedError, removeHost, - renameHost, resolvePairingHostIdentity, resetHostStoreForTests, saveHost, saveExistingHostRelayUpgrade, + updateHostNameAndEndpoint, updateLastConnected } from './host-store' import { resetMobileRelayHostOverlayStoreForTests } from './mobile-relay-host-overlay-store' @@ -273,7 +273,7 @@ describe('host-store list mutations', () => { return storedHostsRaw }) - const rename = renameHost(HOST_ONE.id, 'Renamed Host') + const rename = updateHostNameAndEndpoint(HOST_ONE.id, { name: '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. @@ -294,9 +294,9 @@ describe('host-store list mutations', () => { it('preserves a rename when lastConnected updates race it', async () => { const before = Date.now() await Promise.all([ - renameHost(HOST_ONE.id, 'Alpha'), + updateHostNameAndEndpoint(HOST_ONE.id, { name: 'Alpha' }), updateLastConnected(HOST_ONE.id), - renameHost(HOST_TWO.id, 'Beta') + updateHostNameAndEndpoint(HOST_TWO.id, { name: 'Beta' }) ]) const stored = JSON.parse(storedHostsRaw) as Array @@ -312,7 +312,9 @@ describe('host-store list mutations', () => { it('does not wipe the host list when storage is unreadable during mutation', async () => { storedHostsRaw = '{' - await expect(renameHost(HOST_ONE.id, 'Nope')).rejects.toThrow(/unreadable/) + await expect(updateHostNameAndEndpoint(HOST_ONE.id, { name: 'Nope' })).rejects.toThrow( + /unreadable/ + ) expect(asyncStorageMock.setItem).not.toHaveBeenCalled() expect(storedHostsRaw).toBe('{') }) diff --git a/mobile/src/transport/host-store.ts b/mobile/src/transport/host-store.ts index ad2d7ebc5..a404379e5 100644 --- a/mobile/src/transport/host-store.ts +++ b/mobile/src/transport/host-store.ts @@ -333,14 +333,25 @@ export async function retryPendingHostCredentialCleanup(): Promise<{ return retryPendingHostCredentialCleanups(deleteHostCredentials) } -export async function renameHost(hostId: string, newName: string): Promise { +// Why: Edit host can change name and endpoint together; a single +// mutateStoredHosts pass keeps both fields committed atomically so a +// mid-save failure can never persist one change without the other, and a +// host removed mid-edit throws consistently instead of silently no-oping. +export async function updateHostNameAndEndpoint( + hostId: string, + updates: { name?: string; endpoint?: string } +): Promise { await mutateStoredHosts((hosts) => { - const index = hosts.findIndex((h) => h.id === hostId) + const index = hosts.findIndex((host) => host.id === hostId) if (index < 0) { - return hosts + throw new Error('Host not found') } const next = hosts.slice() - next[index] = { ...next[index]!, name: newName } + next[index] = { + ...next[index]!, + ...(updates.name !== undefined ? { name: updates.name } : {}), + ...(updates.endpoint !== undefined ? { endpoint: updates.endpoint } : {}) + } return next }) }