From f6d0bde6fb5badbf1949bc8d00a4dea3230ed617 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 6 Aug 2026 11:15:36 -0700 Subject: [PATCH] fix(mobile): choose host for new workspace (#11647) * fix(mobile): choose host for new workspace * fix(mobile): close stale workspace host picker * fix(mobile): disambiguate workspace host choices * fix(mobile): keep host endpoint paths private * fix(mobile): redact invalid host endpoints * fix(mobile): handle opaque host endpoints * fix(mobile): announce host picker options * fix(mobile): harden workspace host picker * fix(mobile): preserve host through workspace creation --- mobile/app/h/[hostId]/index.tsx | 6 +- mobile/app/index.tsx | 93 ++------ .../components/MobileHomeQuickActions.test.ts | 223 ++++++++++++++++++ .../src/components/MobileHomeQuickActions.tsx | 159 +++++++++++++ .../PickerModal.accessibility.test.ts | 66 ++++++ mobile/src/components/PickerModal.tsx | 7 +- mobile/src/host-route-action-state.test.ts | 12 + mobile/src/host-route-action-state.ts | 13 + mobile/src/transport/host-endpoint-label.ts | 11 + 9 files changed, 508 insertions(+), 82 deletions(-) create mode 100644 mobile/src/components/MobileHomeQuickActions.test.ts create mode 100644 mobile/src/components/MobileHomeQuickActions.tsx create mode 100644 mobile/src/components/PickerModal.accessibility.test.ts create mode 100644 mobile/src/transport/host-endpoint-label.ts diff --git a/mobile/app/h/[hostId]/index.tsx b/mobile/app/h/[hostId]/index.tsx index debd57a92..a6660ffe7 100644 --- a/mobile/app/h/[hostId]/index.tsx +++ b/mobile/app/h/[hostId]/index.tsx @@ -67,6 +67,7 @@ import { leaveHostRoute } from '../../../src/host-route-exit' import { loadPinnedIds, savePinnedIds } from '../../../src/storage/preferences' import { createInitialHostRouteActionState, + hostNewWorktreeSessionRoute, resolveHostRouteActionState, setHostRouteNewWorktreeVisible } from '../../../src/host-route-action-state' @@ -1403,10 +1404,7 @@ export function HostScreen({ }} onCreated={(worktreeId, worktreeName) => { void fetchWorktrees({ allowDuringModal: true }) - const params = new URLSearchParams({ name: worktreeName, created: '1' }) - navigateFromHostList( - `/h/${hostId}/session/${encodeURIComponent(worktreeId)}?${params.toString()}` - ) + navigateFromHostList(hostNewWorktreeSessionRoute(hostId, worktreeId, worktreeName)) }} onRouteVisibleChange={setShowNewWorktreeVisible} /> diff --git a/mobile/app/index.tsx b/mobile/app/index.tsx index 707e8ddb0..3edd5cf42 100644 --- a/mobile/app/index.tsx +++ b/mobile/app/index.tsx @@ -2,7 +2,7 @@ import { useState, useCallback, useEffect, useMemo, useRef } from 'react' import { View, Text, StyleSheet, Pressable, FlatList, Alert } from 'react-native' import { SafeAreaView, useSafeAreaInsets } from 'react-native-safe-area-context' import { useRouter, useFocusEffect } from 'expo-router' -import { QrCode, Settings, ChevronRight, Terminal, Plus, ListTodo } from 'lucide-react-native' +import { QrCode, Settings, ChevronRight, Terminal, ListTodo } from 'lucide-react-native' import { ClaudeIcon, OpenAIIcon } from '../src/components/AgentIcons' import { type AccountsSnapshot, @@ -41,6 +41,7 @@ import type { ConnectionState, HostCatalogEntry, HostProfile } from '../src/tran import { triggerMediumImpact } from '../src/platform/haptics' import { OrcaLogo } from '../src/components/OrcaLogo' import { MobileHostCard } from '../src/components/MobileHostCard' +import { MobileHomeQuickActions } from '../src/components/MobileHomeQuickActions' import { TaskProviderLogo } from '../src/components/TaskProviderLogo' import { ActionSheetModal } from '../src/components/ActionSheetModal' import { getHostListActionSheetActions } from '../src/host-list-action-sheet-actions' @@ -71,15 +72,8 @@ import { type HomeResumeCard } from '../src/worktree/home-resume-card' import { hostRouteWithNotice } from '../src/host-route-notice' - -function endpointLabel(endpoint: string): string { - try { - const url = new URL(endpoint) - return `${url.hostname}${url.port ? `:${url.port}` : ''}` - } catch { - return endpoint - } -} +import { hostNewWorktreeRoute } from '../src/host-route-action-state' +import { hostEndpointLabel } from '../src/transport/host-endpoint-label' type HomeTaskSettings = { visibleTaskProviders?: unknown @@ -572,10 +566,11 @@ export default function HomeScreen() { return items }, [sortedHosts, hostStates, accountsByHost]) - const primaryConnectedHost = useMemo( - () => sortedHosts.find((host) => hostStates[host.id] === 'connected') ?? null, + const connectedHosts = useMemo( + () => sortedHosts.filter((host) => hostStates[host.id] === 'connected'), [sortedHosts, hostStates] ) + const primaryConnectedHost = connectedHosts[0] ?? null const primaryTaskProviders = primaryConnectedHost ? (taskProvidersByHost[primaryConnectedHost.id] ?? ['github']) : [] @@ -847,36 +842,11 @@ export default function HomeScreen() { {renderTaskHomeCard()} {/* ─── Quick actions ─── */} - Quick Actions - - [styles.quickAction, pressed && styles.hostCardPressed]} - onPress={() => router.push('/pair-scan')} - > - - - - Pair Desktop - - [ - styles.quickAction, - !primaryConnectedHost && styles.cardDisabled, - pressed && styles.hostCardPressed - ]} - onPress={() => { - if (primaryConnectedHost) { - router.push(`/h/${primaryConnectedHost.id}?action=newWorktree`) - } - }} - > - - - - New Workspace - - + router.push('/pair-scan')} + onCreateWorkspace={(hostId) => router.push(hostNewWorktreeRoute(hostId))} + /> {/* ─── Account usage ─── */} {accountsHosts.length > 0 ? ( @@ -964,7 +934,7 @@ export default function HomeScreen() { ({ + Pressable: 'Pressable', + StyleSheet: { create: (styles: unknown) => styles }, + Text: 'Text', + View: 'View' +})) + +vi.mock('lucide-react-native', () => ({ + Plus: 'Plus', + QrCode: 'QrCode' +})) + +vi.mock('./PickerModal', async () => { + const React = await import('react') + return { + PickerModal: (props: unknown) => React.createElement('PickerModal', props) + } +}) + +function host(id: string, name: string, endpoint: string): HostProfile { + return { + id, + name, + endpoint, + deviceToken: `token-${id}`, + publicKeyB64: `key-${id}`, + lastConnected: 1 + } +} + +describe('MobileHomeQuickActions', () => { + let renderer: ReactTestRenderer | null = null + + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + }) + + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + vi.restoreAllMocks() + }) + + async function renderQuickActions(connectedHosts: HostProfile[]) { + const onPairDesktop = vi.fn() + const onCreateWorkspace = vi.fn() + const quickActions = (hosts: HostProfile[]) => + createElement(MobileHomeQuickActions, { + connectedHosts: hosts, + onPairDesktop, + onCreateWorkspace + }) + const consoleError = vi.spyOn(console, 'error').mockImplementation((...args) => { + if (typeof args[0] !== 'string' || !args[0].includes('react-test-renderer is deprecated')) { + throw new Error(String(args[0])) + } + }) + await act(async () => { + renderer = create(quickActions(connectedHosts)) + }) + consoleError.mockRestore() + return { + onCreateWorkspace, + rerender: async (hosts: HostProfile[]) => { + await act(async () => renderer!.update(quickActions(hosts))) + } + } + } + + function newWorkspaceButton() { + return renderer!.root.findAllByType('Pressable')[1] + } + + function picker() { + return renderer!.root.findByType('PickerModal') + } + + it('disables workspace creation without a connected host', async () => { + await renderQuickActions([]) + + expect(newWorkspaceButton().props.disabled).toBe(true) + expect(newWorkspaceButton().props.accessibilityState).toEqual({ disabled: true }) + expect(picker().props.visible).toBe(false) + }) + + it('opens the only connected host directly', async () => { + const desk = host('desk', 'Desk', 'ws://192.168.1.2:6768') + const callbacks = await renderQuickActions([desk]) + + act(() => newWorkspaceButton().props.onPress()) + + expect(callbacks.onCreateWorkspace).toHaveBeenCalledWith('desk') + expect(picker().props.visible).toBe(false) + }) + + it('asks which host to use when multiple are connected', async () => { + const callbacks = await renderQuickActions([ + host('desk', 'Desk', 'ws://192.168.1.2:6768'), + host('laptop', 'Laptop', 'wss://relay.example.com/mobile') + ]) + + act(() => newWorkspaceButton().props.onPress()) + + expect(callbacks.onCreateWorkspace).not.toHaveBeenCalled() + expect(picker().props.visible).toBe(true) + expect(picker().props.title).toBe('Create Workspace On') + expect(picker().props.options).toEqual([ + { value: 'desk', label: 'Desk', subtitle: '192.168.1.2:6768' }, + { value: 'laptop', label: 'Laptop', subtitle: 'relay.example.com' } + ]) + + act(() => picker().props.onSelect('laptop')) + expect(callbacks.onCreateWorkspace).not.toHaveBeenCalled() + expect(picker().props.visible).toBe(false) + + act(() => picker().props.onAfterClose()) + expect(callbacks.onCreateWorkspace).toHaveBeenCalledWith('laptop') + }) + + it('disambiguates path-routed hosts without exposing endpoint paths', async () => { + await renderQuickActions([ + host('desk-a', 'Desk', 'wss://gateway.example.com/v1/connect/bearer-secret-a'), + host('desk-b', 'Desk', 'wss://gateway.example.com/v1/connect/bearer-secret-b') + ]) + + act(() => newWorkspaceButton().props.onPress()) + + expect(picker().props.options).toEqual([ + { + value: 'desk-a', + label: 'Desk', + subtitle: 'gateway.example.com · desk-a' + }, + { + value: 'desk-b', + label: 'Desk', + subtitle: 'gateway.example.com · desk-b' + } + ]) + expect(JSON.stringify(picker().props.options)).not.toContain('bearer-secret') + }) + + it('does not expose malformed legacy endpoint details', async () => { + await renderQuickActions([ + host('desk-a', 'Desk', 'gateway.example.com/v1/connect/bearer-secret?token=query-secret'), + host('desk-b', 'Desk', 'localhost:6768/private-secret') + ]) + + act(() => newWorkspaceButton().props.onPress()) + + expect(picker().props.options).toEqual([ + { value: 'desk-a', label: 'Desk', subtitle: 'Unknown endpoint · desk-a' }, + { value: 'desk-b', label: 'Desk', subtitle: 'Unknown endpoint · desk-b' } + ]) + expect(JSON.stringify(picker().props.options)).not.toContain('secret') + }) + + it('closes a stale picker when fewer than two hosts remain connected', async () => { + const desk = host('desk', 'Desk', 'ws://192.168.1.2:6768') + const laptop = host('laptop', 'Laptop', 'wss://relay.example.com/mobile') + const callbacks = await renderQuickActions([desk, laptop]) + + act(() => newWorkspaceButton().props.onPress()) + expect(picker().props.visible).toBe(true) + + await callbacks.rerender([desk]) + expect(picker().props.visible).toBe(false) + act(() => picker().props.onAfterClose()) + + await callbacks.rerender([desk, laptop]) + expect(picker().props.visible).toBe(false) + }) + + it('does not reopen a stale picker if its old host set returns while closing', async () => { + const desk = host('desk', 'Desk', 'ws://192.168.1.2:6768') + const laptop = host('laptop', 'Laptop', 'wss://relay.example.com/mobile') + const callbacks = await renderQuickActions([desk, laptop]) + + act(() => newWorkspaceButton().props.onPress()) + await callbacks.rerender([desk]) + await callbacks.rerender([desk, laptop]) + + expect(picker().props.visible).toBe(false) + act(() => picker().props.onAfterClose()) + expect(callbacks.onCreateWorkspace).not.toHaveBeenCalled() + }) + + it('keeps a selected host through an unrelated topology change while closing', async () => { + const desk = host('desk', 'Desk', 'ws://192.168.1.2:6768') + const laptop = host('laptop', 'Laptop', 'wss://relay.example.com/mobile') + const callbacks = await renderQuickActions([desk, laptop]) + + act(() => newWorkspaceButton().props.onPress()) + act(() => picker().props.onSelect('laptop')) + await callbacks.rerender([ + desk, + laptop, + host('server', 'Server', 'wss://ssh.example.com/mobile') + ]) + act(() => picker().props.onAfterClose()) + + expect(callbacks.onCreateWorkspace).toHaveBeenCalledWith('laptop') + }) + + it('drops a selection that disconnects while the picker is closing', async () => { + const desk = host('desk', 'Desk', 'ws://192.168.1.2:6768') + const laptop = host('laptop', 'Laptop', 'wss://relay.example.com/mobile') + const callbacks = await renderQuickActions([desk, laptop]) + + act(() => newWorkspaceButton().props.onPress()) + act(() => picker().props.onSelect('laptop')) + await callbacks.rerender([desk]) + act(() => picker().props.onAfterClose()) + + expect(callbacks.onCreateWorkspace).not.toHaveBeenCalled() + }) +}) diff --git a/mobile/src/components/MobileHomeQuickActions.tsx b/mobile/src/components/MobileHomeQuickActions.tsx new file mode 100644 index 000000000..9dca88997 --- /dev/null +++ b/mobile/src/components/MobileHomeQuickActions.tsx @@ -0,0 +1,159 @@ +import { useRef, useState } from 'react' +import { Plus, QrCode } from 'lucide-react-native' +import { Pressable, StyleSheet, Text, View } from 'react-native' +import type { HostProfile } from '../transport/types' +import { hostEndpointLabel } from '../transport/host-endpoint-label' +import { colors, radii, spacing } from '../theme/mobile-theme' +import { PickerModal } from './PickerModal' + +type Props = { + connectedHosts: HostProfile[] + onPairDesktop: () => void + onCreateWorkspace: (hostId: string) => void +} + +function hostPickerOptions(hosts: HostProfile[]) { + const entries = hosts.map((host) => ({ + host, + endpointLabel: hostEndpointLabel(host.endpoint) + })) + const endpointCounts = new Map() + for (const entry of entries) { + const endpointKey = JSON.stringify([entry.host.name, entry.endpointLabel]) + endpointCounts.set(endpointKey, (endpointCounts.get(endpointKey) ?? 0) + 1) + } + return entries.map((entry) => { + const endpointKey = JSON.stringify([entry.host.name, entry.endpointLabel]) + const endpointCollides = (endpointCounts.get(endpointKey) ?? 0) > 1 + const subtitle = endpointCollides + ? `${entry.endpointLabel} · ${entry.host.id}` + : entry.endpointLabel + return { value: entry.host.id, label: entry.host.name, subtitle } + }) +} + +export function MobileHomeQuickActions(props: Props) { + const [hostPickerForHostSet, setHostPickerForHostSet] = useState(null) + const pendingHostIdRef = useRef(null) + const canCreateWorkspace = props.connectedHosts.length > 0 + const hostSetKey = JSON.stringify(props.connectedHosts.map((host) => host.id)) + const hostPickerVisible = hostPickerForHostSet === hostSetKey + if (hostPickerForHostSet !== null && !hostPickerVisible) { + setHostPickerForHostSet(null) + } + + function handleCreateWorkspace() { + if (props.connectedHosts.length === 1) { + props.onCreateWorkspace(props.connectedHosts[0].id) + return + } + if (props.connectedHosts.length > 1) { + setHostPickerForHostSet(hostSetKey) + } + } + + function handleHostSelect(hostId: string) { + pendingHostIdRef.current = hostId + setHostPickerForHostSet(null) + } + + function handleHostPickerClosed() { + setHostPickerForHostSet(null) + const hostId = pendingHostIdRef.current + pendingHostIdRef.current = null + if (hostId && props.connectedHosts.some((host) => host.id === hostId)) { + props.onCreateWorkspace(hostId) + } + } + + return ( + <> + Quick Actions + + [styles.quickAction, pressed && styles.quickActionPressed]} + onPress={props.onPairDesktop} + > + + + + Pair Desktop + + [ + styles.quickAction, + !canCreateWorkspace && styles.quickActionDisabled, + pressed && styles.quickActionPressed + ]} + onPress={handleCreateWorkspace} + > + + + + New Workspace + + + setHostPickerForHostSet(null)} + onAfterClose={handleHostPickerClosed} + /> + + ) +} + +const styles = StyleSheet.create({ + sectionHeading: { + marginTop: spacing.xl, + marginBottom: spacing.sm, + paddingHorizontal: spacing.xs, + color: colors.textMuted, + fontSize: 11, + fontWeight: '600', + textTransform: 'uppercase', + letterSpacing: 0.6 + }, + quickActions: { + flexDirection: 'row', + gap: spacing.sm + }, + quickAction: { + flex: 1, + flexDirection: 'row', + alignItems: 'center', + gap: spacing.sm + 2, + paddingHorizontal: spacing.md, + paddingVertical: spacing.sm + 2, + borderWidth: 1, + borderColor: colors.borderSubtle, + borderRadius: radii.card, + backgroundColor: colors.bgPanel + }, + quickActionPressed: { + backgroundColor: colors.bgRaised + }, + quickActionDisabled: { + opacity: 0.45 + }, + quickActionIcon: { + width: 28, + height: 28, + borderRadius: 9, + alignItems: 'center', + justifyContent: 'center', + backgroundColor: colors.bgRaised + }, + quickActionLabel: { + color: colors.textSecondary, + fontSize: 12, + fontWeight: '600' + } +}) diff --git a/mobile/src/components/PickerModal.accessibility.test.ts b/mobile/src/components/PickerModal.accessibility.test.ts new file mode 100644 index 000000000..4508f2275 --- /dev/null +++ b/mobile/src/components/PickerModal.accessibility.test.ts @@ -0,0 +1,66 @@ +import { createElement, type ReactNode } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { PickerModal } from './PickerModal' + +vi.mock('react-native', () => ({ + Pressable: 'Pressable', + StyleSheet: { create: (styles: unknown) => styles, hairlineWidth: 1 }, + Text: 'Text', + View: 'View' +})) + +vi.mock('lucide-react-native', () => ({ Check: 'Check' })) + +vi.mock('./BottomDrawer', async () => { + const React = await import('react') + return { + BottomDrawer: ({ children }: { children?: ReactNode }) => + React.createElement('BottomDrawer', null, children) + } +}) + +describe('PickerModal accessibility', () => { + let renderer: ReactTestRenderer | null = null + + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + vi.spyOn(console, 'error').mockImplementation((...args) => { + if (typeof args[0] !== 'string' || !args[0].includes('react-test-renderer is deprecated')) { + throw new Error(String(args[0])) + } + }) + }) + + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + vi.restoreAllMocks() + }) + + it('announces option rows as actionable with their selection and disabled state', async () => { + await act(async () => { + renderer = create( + createElement(PickerModal, { + visible: true, + title: 'Create Workspace On', + options: [ + { value: 'desk', label: 'Desk' }, + { value: 'laptop', label: 'Laptop', disabled: true } + ], + selected: 'desk', + onSelect: vi.fn(), + onClose: vi.fn() + }) + ) + }) + + const rows = renderer!.root.findAllByType('Pressable') + expect(rows.map((row) => row.props.accessible)).toEqual([true, true]) + expect(rows.map((row) => row.props.accessibilityRole)).toEqual(['button', 'button']) + expect(rows.map((row) => row.props.accessibilityState)).toEqual([ + { disabled: false, selected: true }, + { disabled: true, selected: false } + ]) + }) +}) diff --git a/mobile/src/components/PickerModal.tsx b/mobile/src/components/PickerModal.tsx index 25da40463..945630709 100644 --- a/mobile/src/components/PickerModal.tsx +++ b/mobile/src/components/PickerModal.tsx @@ -20,6 +20,7 @@ type Props = { onSelect: (value: T) => void onLongSelect?: (value: T) => void onClose: () => void + onAfterClose?: () => void zIndex?: number } @@ -36,10 +37,11 @@ export function PickerModal({ onSelect, onLongSelect, onClose, + onAfterClose, zIndex }: Props) { return ( - + {title} @@ -72,6 +74,9 @@ function PickerModalContent({ {i > 0 && } [ styles.row, diff --git a/mobile/src/host-route-action-state.test.ts b/mobile/src/host-route-action-state.test.ts index c662d2d34..7f0ff6375 100644 --- a/mobile/src/host-route-action-state.test.ts +++ b/mobile/src/host-route-action-state.test.ts @@ -2,11 +2,23 @@ import { describe, expect, it } from 'vitest' import { createInitialHostRouteActionState, + hostNewWorktreeRoute, + hostNewWorktreeSessionRoute, resolveHostRouteActionState, setHostRouteNewWorktreeVisible } from './host-route-action-state' describe('host route action state', () => { + it('encodes opaque host ids in the new-worktree route segment', () => { + expect(hostNewWorktreeRoute('relay/one#50%')).toBe('/h/relay%2Fone%2350%25?action=newWorktree') + }) + + it('preserves opaque host and worktree ids after creation', () => { + expect(hostNewWorktreeSessionRoute('relay/one#50%', 'repo/one#20%', 'Relay workspace')).toBe( + '/h/relay%2Fone%2350%25/session/repo%2Fone%2320%25?name=Relay+workspace&created=1' + ) + }) + it('opens new worktree modal on an initial newWorktree action', () => { expect(createInitialHostRouteActionState('newWorktree')).toEqual({ routeAction: 'newWorktree', diff --git a/mobile/src/host-route-action-state.ts b/mobile/src/host-route-action-state.ts index bdcb7a524..a89c03fb4 100644 --- a/mobile/src/host-route-action-state.ts +++ b/mobile/src/host-route-action-state.ts @@ -3,6 +3,19 @@ export type HostRouteActionState = { showNewWorktree: boolean } +export function hostNewWorktreeRoute(hostId: string): `/h/${string}?action=newWorktree` { + return `/h/${encodeURIComponent(hostId)}?action=newWorktree` +} + +export function hostNewWorktreeSessionRoute( + hostId: string, + worktreeId: string, + worktreeName: string +): `/h/${string}/session/${string}?${string}` { + const params = new URLSearchParams({ name: worktreeName, created: '1' }) + return `/h/${encodeURIComponent(hostId)}/session/${encodeURIComponent(worktreeId)}?${params}` +} + export function createInitialHostRouteActionState( routeAction: string | undefined ): HostRouteActionState { diff --git a/mobile/src/transport/host-endpoint-label.ts b/mobile/src/transport/host-endpoint-label.ts new file mode 100644 index 000000000..e326a650a --- /dev/null +++ b/mobile/src/transport/host-endpoint-label.ts @@ -0,0 +1,11 @@ +export function hostEndpointLabel(endpoint: string): string { + try { + const url = new URL(endpoint) + if (!url.hostname) { + return 'Unknown endpoint' + } + return `${url.hostname}${url.port ? `:${url.port}` : ''}` + } catch { + return 'Unknown endpoint' + } +}