mirror of
https://github.com/stablyai/orca.git
synced 2026-10-17 08:03:14 +00:00
Add host removal lifecycle safeguards and credential cleanup retry UI
- Sequence host removal so metadata commits before the client socket closes, avoiding a stranded host when storage fails, and add a cancellable open-registry to stop races between host-client opens and closes/unmounts. - Queue AsyncStorage host-list mutations (rename/removal/lastConnected) to prevent concurrent writers from clobbering each other's changes. - Track keychain credential cleanups that fail or time out as durable pending intents, surfaced with a manual retry affordance in Settings.
This commit is contained in:
@@ -20,7 +20,8 @@ import {
|
||||
PanelLeftClose
|
||||
} from 'lucide-react-native'
|
||||
import type { RpcClient } from '../../../src/transport/rpc-client'
|
||||
import { loadHosts, updateLastConnected, removeHost } from '../../../src/transport/host-store'
|
||||
import { loadHosts, updateLastConnected } from '../../../src/transport/host-store'
|
||||
import { removeHostAndCloseClient } from '../../../src/transport/host-removal-lifecycle'
|
||||
import {
|
||||
useHostClient,
|
||||
useCloseHost,
|
||||
@@ -684,13 +685,13 @@ export function HostScreen({
|
||||
if (!hostId) {
|
||||
return
|
||||
}
|
||||
// Why: close the shared client first so its WebSocket is gone before
|
||||
// the host record disappears; otherwise the next loadHosts() the
|
||||
// provider does (e.g. on remount) wouldn't find this host but the
|
||||
// socket would still be open, leaking state.
|
||||
closeHostClient(hostId)
|
||||
await removeHost(hostId)
|
||||
leaveHost()
|
||||
try {
|
||||
await removeHostAndCloseClient(hostId, closeHostClient)
|
||||
leaveHost()
|
||||
} catch {
|
||||
// Why: metadata commit can fail while the host is still paired; keep the
|
||||
// screen mounted so the user can retry instead of navigating away empty.
|
||||
}
|
||||
}, [hostId, leaveHost, closeHostClient])
|
||||
|
||||
const navigateFromHostList = useCallback(
|
||||
|
||||
@@ -25,7 +25,8 @@ import {
|
||||
UsageBar
|
||||
} from '../src/components/AccountUsage'
|
||||
import AsyncStorage from '@react-native-async-storage/async-storage'
|
||||
import { loadHosts, removeHost, renameHost } from '../src/transport/host-store'
|
||||
import { loadHosts, renameHost } 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'
|
||||
import {
|
||||
@@ -711,10 +712,7 @@ export default function HomeScreen() {
|
||||
return
|
||||
}
|
||||
try {
|
||||
// Why: close the shared client first so the WebSocket is gone
|
||||
// before the host record disappears from loadHosts().
|
||||
closeHostClient(confirmRemove.id)
|
||||
await removeHost(confirmRemove.id)
|
||||
await removeHostAndCloseClient(confirmRemove.id, closeHostClient)
|
||||
setConfirmRemove(null)
|
||||
setHosts(await loadHosts())
|
||||
} catch {
|
||||
|
||||
+218
-77
@@ -1,6 +1,15 @@
|
||||
import { View, Text, StyleSheet, Pressable, Linking } from 'react-native'
|
||||
import { useCallback, useRef, useState } from 'react'
|
||||
import {
|
||||
View,
|
||||
Text,
|
||||
StyleSheet,
|
||||
Pressable,
|
||||
Linking,
|
||||
ActivityIndicator,
|
||||
ScrollView
|
||||
} from 'react-native'
|
||||
import { useSafeAreaInsets } from 'react-native-safe-area-context'
|
||||
import { useRouter } from 'expo-router'
|
||||
import { useFocusEffect, useRouter } from 'expo-router'
|
||||
import {
|
||||
ChevronLeft,
|
||||
ChevronRight,
|
||||
@@ -11,13 +20,69 @@ import {
|
||||
LifeBuoy,
|
||||
Mic,
|
||||
Globe,
|
||||
Terminal as TerminalIcon
|
||||
Terminal as TerminalIcon,
|
||||
KeyRound
|
||||
} from 'lucide-react-native'
|
||||
import { colors, spacing, typography } from '../src/theme/mobile-theme'
|
||||
import { colors, radii, spacing, typography } from '../src/theme/mobile-theme'
|
||||
import {
|
||||
loadPendingHostCredentialCleanupIds,
|
||||
subscribePendingHostCredentialCleanup
|
||||
} from '../src/transport/host-credential-cleanup'
|
||||
import { retryPendingHostCredentialCleanup } from '../src/transport/host-store'
|
||||
|
||||
export default function SettingsScreen() {
|
||||
const router = useRouter()
|
||||
const insets = useSafeAreaInsets()
|
||||
const [pendingCredentialIds, setPendingCredentialIds] = useState<string[]>([])
|
||||
const [retryingCredentialCleanup, setRetryingCredentialCleanup] = useState(false)
|
||||
const [credentialRetryFailed, setCredentialRetryFailed] = useState(false)
|
||||
const credentialRefreshGenerationRef = useRef(0)
|
||||
|
||||
useFocusEffect(
|
||||
useCallback(() => {
|
||||
let active = true
|
||||
setCredentialRetryFailed(false)
|
||||
const refresh = () => {
|
||||
const generation = ++credentialRefreshGenerationRef.current
|
||||
void loadPendingHostCredentialCleanupIds().then((ids) => {
|
||||
if (active && generation === credentialRefreshGenerationRef.current) {
|
||||
setPendingCredentialIds(ids)
|
||||
// Why: neutral copy once the queue is empty so a later pending
|
||||
// set does not inherit a previous Retry failure message.
|
||||
if (ids.length === 0) {
|
||||
setCredentialRetryFailed(false)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
const unsubscribe = subscribePendingHostCredentialCleanup(refresh)
|
||||
refresh()
|
||||
return () => {
|
||||
active = false
|
||||
credentialRefreshGenerationRef.current += 1
|
||||
unsubscribe()
|
||||
}
|
||||
}, [])
|
||||
)
|
||||
|
||||
const retryCredentialCleanup = useCallback(async () => {
|
||||
if (retryingCredentialCleanup) {
|
||||
return
|
||||
}
|
||||
setCredentialRetryFailed(false)
|
||||
setRetryingCredentialCleanup(true)
|
||||
try {
|
||||
const result = await retryPendingHostCredentialCleanup()
|
||||
setPendingCredentialIds(result.remainingIds)
|
||||
setCredentialRetryFailed(result.remainingIds.length > 0)
|
||||
} catch {
|
||||
setCredentialRetryFailed(true)
|
||||
} finally {
|
||||
setRetryingCredentialCleanup(false)
|
||||
}
|
||||
}, [retryingCredentialCleanup])
|
||||
|
||||
const pendingCredentialCount = pendingCredentialIds.length
|
||||
|
||||
return (
|
||||
<View style={[styles.container, { paddingTop: insets.top + spacing.sm }]}>
|
||||
@@ -28,79 +93,121 @@ export default function SettingsScreen() {
|
||||
<Text style={styles.heading}>Settings</Text>
|
||||
</View>
|
||||
|
||||
<View style={styles.section}>
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => router.push('/terminal-settings')}
|
||||
>
|
||||
<TerminalIcon size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Terminal</Text>
|
||||
<ChevronRight size={16} color={colors.textMuted} />
|
||||
</Pressable>
|
||||
<View style={styles.separator} />
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => router.push('/browser-settings')}
|
||||
>
|
||||
<Globe size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Browser</Text>
|
||||
<ChevronRight size={16} color={colors.textMuted} />
|
||||
</Pressable>
|
||||
<View style={styles.separator} />
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => router.push('/voice-settings')}
|
||||
>
|
||||
<Mic size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Voice</Text>
|
||||
<ChevronRight size={16} color={colors.textMuted} />
|
||||
</Pressable>
|
||||
<View style={styles.separator} />
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => router.push('/notifications')}
|
||||
>
|
||||
<Bell size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Notifications</Text>
|
||||
<ChevronRight size={16} color={colors.textMuted} />
|
||||
</Pressable>
|
||||
<View style={styles.separator} />
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => router.push('/troubleshoot')}
|
||||
>
|
||||
<Wrench size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Troubleshooting</Text>
|
||||
<ChevronRight size={16} color={colors.textMuted} />
|
||||
</Pressable>
|
||||
<View style={styles.separator} />
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => router.push('/about')}
|
||||
>
|
||||
<Info size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>About</Text>
|
||||
<ChevronRight size={16} color={colors.textMuted} />
|
||||
</Pressable>
|
||||
</View>
|
||||
<ScrollView
|
||||
contentContainerStyle={{ paddingBottom: insets.bottom + spacing.lg }}
|
||||
showsVerticalScrollIndicator={false}
|
||||
>
|
||||
<View style={styles.section}>
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => router.push('/terminal-settings')}
|
||||
>
|
||||
<TerminalIcon size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Terminal</Text>
|
||||
<ChevronRight size={16} color={colors.textMuted} />
|
||||
</Pressable>
|
||||
<View style={styles.separator} />
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => router.push('/browser-settings')}
|
||||
>
|
||||
<Globe size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Browser</Text>
|
||||
<ChevronRight size={16} color={colors.textMuted} />
|
||||
</Pressable>
|
||||
<View style={styles.separator} />
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => router.push('/voice-settings')}
|
||||
>
|
||||
<Mic size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Voice</Text>
|
||||
<ChevronRight size={16} color={colors.textMuted} />
|
||||
</Pressable>
|
||||
<View style={styles.separator} />
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => router.push('/notifications')}
|
||||
>
|
||||
<Bell size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Notifications</Text>
|
||||
<ChevronRight size={16} color={colors.textMuted} />
|
||||
</Pressable>
|
||||
<View style={styles.separator} />
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => router.push('/troubleshoot')}
|
||||
>
|
||||
<Wrench size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Troubleshooting</Text>
|
||||
<ChevronRight size={16} color={colors.textMuted} />
|
||||
</Pressable>
|
||||
<View style={styles.separator} />
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => router.push('/about')}
|
||||
>
|
||||
<Info size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>About</Text>
|
||||
<ChevronRight size={16} color={colors.textMuted} />
|
||||
</Pressable>
|
||||
</View>
|
||||
|
||||
<View style={[styles.section, styles.sectionSpacer]}>
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => void Linking.openURL('https://www.onorca.dev/privacy')}
|
||||
>
|
||||
<Shield size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Privacy Policy</Text>
|
||||
</Pressable>
|
||||
<View style={styles.separator} />
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => void Linking.openURL('https://github.com/stablyai/orca/issues')}
|
||||
>
|
||||
<LifeBuoy size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Support</Text>
|
||||
</Pressable>
|
||||
</View>
|
||||
{pendingCredentialCount > 0 ? (
|
||||
<View style={[styles.section, styles.sectionSpacer]}>
|
||||
<View style={styles.credentialCleanupRow}>
|
||||
<KeyRound size={16} color={colors.statusAmber} />
|
||||
<View style={styles.credentialCleanupCopy}>
|
||||
<Text style={styles.credentialCleanupTitle}>Pairing credential cleanup</Text>
|
||||
<Text accessibilityLiveRegion="polite" style={styles.rowHint}>
|
||||
{credentialRetryFailed
|
||||
? "Cleanup still couldn't be confirmed. Try again later."
|
||||
: `Couldn't confirm cleanup for ${pendingCredentialCount} credential${pendingCredentialCount === 1 ? '' : 's'} on this device.`}
|
||||
</Text>
|
||||
</View>
|
||||
<Pressable
|
||||
accessibilityRole="button"
|
||||
accessibilityLabel="Retry clearing pairing credentials"
|
||||
accessibilityState={{
|
||||
busy: retryingCredentialCleanup,
|
||||
disabled: retryingCredentialCleanup
|
||||
}}
|
||||
disabled={retryingCredentialCleanup}
|
||||
hitSlop={8}
|
||||
style={({ pressed }) => [
|
||||
styles.retryButton,
|
||||
pressed && !retryingCredentialCleanup && styles.rowPressed
|
||||
]}
|
||||
onPress={() => void retryCredentialCleanup()}
|
||||
>
|
||||
{retryingCredentialCleanup ? (
|
||||
<ActivityIndicator size="small" color={colors.textSecondary} />
|
||||
) : (
|
||||
<Text style={styles.retryButtonText}>Retry</Text>
|
||||
)}
|
||||
</Pressable>
|
||||
</View>
|
||||
</View>
|
||||
) : null}
|
||||
|
||||
<View style={[styles.section, styles.sectionSpacer]}>
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => void Linking.openURL('https://www.onorca.dev/privacy')}
|
||||
>
|
||||
<Shield size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Privacy Policy</Text>
|
||||
</Pressable>
|
||||
<View style={styles.separator} />
|
||||
<Pressable
|
||||
style={({ pressed }) => [styles.row, pressed && styles.rowPressed]}
|
||||
onPress={() => void Linking.openURL('https://github.com/stablyai/orca/issues')}
|
||||
>
|
||||
<LifeBuoy size={16} color={colors.textSecondary} />
|
||||
<Text style={styles.rowLabel}>Support</Text>
|
||||
</Pressable>
|
||||
</View>
|
||||
</ScrollView>
|
||||
</View>
|
||||
)
|
||||
}
|
||||
@@ -109,7 +216,7 @@ const styles = StyleSheet.create({
|
||||
container: {
|
||||
flex: 1,
|
||||
backgroundColor: colors.bgBase,
|
||||
padding: spacing.lg
|
||||
paddingHorizontal: spacing.lg
|
||||
},
|
||||
topRow: {
|
||||
flexDirection: 'row',
|
||||
@@ -153,6 +260,40 @@ const styles = StyleSheet.create({
|
||||
fontWeight: '500',
|
||||
color: colors.textPrimary
|
||||
},
|
||||
credentialCleanupRow: {
|
||||
flexDirection: 'row',
|
||||
alignItems: 'center',
|
||||
gap: spacing.sm + 2,
|
||||
paddingVertical: spacing.md,
|
||||
paddingHorizontal: spacing.md + 2
|
||||
},
|
||||
credentialCleanupCopy: {
|
||||
flex: 1,
|
||||
gap: spacing.xs
|
||||
},
|
||||
credentialCleanupTitle: {
|
||||
fontSize: typography.bodySize,
|
||||
fontWeight: '500',
|
||||
color: colors.textPrimary
|
||||
},
|
||||
rowHint: {
|
||||
fontSize: typography.metaSize,
|
||||
color: colors.textSecondary,
|
||||
lineHeight: 17
|
||||
},
|
||||
retryButton: {
|
||||
width: 72,
|
||||
height: 32,
|
||||
borderRadius: radii.button,
|
||||
backgroundColor: colors.bgRaised,
|
||||
alignItems: 'center',
|
||||
justifyContent: 'center'
|
||||
},
|
||||
retryButtonText: {
|
||||
fontSize: typography.metaSize,
|
||||
fontWeight: '600',
|
||||
color: colors.textPrimary
|
||||
},
|
||||
separator: {
|
||||
height: StyleSheet.hairlineWidth,
|
||||
backgroundColor: colors.borderSubtle,
|
||||
|
||||
@@ -119,6 +119,7 @@ async function renderHarness(hostId: string): Promise<Harness> {
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
globalThis.IS_REACT_ACT_ENVIRONMENT = true
|
||||
connectMock.mockReset()
|
||||
loadHostsMock.mockReset()
|
||||
})
|
||||
@@ -156,4 +157,76 @@ describe('useHostClient', () => {
|
||||
|
||||
harness.unmount()
|
||||
})
|
||||
|
||||
it('does not open a client after the host is closed during an in-flight lookup', async () => {
|
||||
let resolveHosts: ((hosts: (typeof HOST)[]) => void) | null = null
|
||||
const hostLookup = new Promise<(typeof HOST)[]>((resolve) => {
|
||||
resolveHosts = resolve
|
||||
})
|
||||
const fake = makeFakeClient('connected')
|
||||
connectMock.mockReturnValue(fake)
|
||||
loadHostsMock.mockReturnValue(hostLookup)
|
||||
|
||||
let closeHost: ((hostId: string) => void) | null = null
|
||||
let renderer: ReactTestRenderer | null = null
|
||||
function Probe(): null {
|
||||
closeHost = useCloseHost()
|
||||
useHostClient(HOST.id)
|
||||
return null
|
||||
}
|
||||
|
||||
const restore = suppressReactTestRendererDeprecationWarning()
|
||||
try {
|
||||
act(() => {
|
||||
renderer = create(createElement(RpcClientProvider, null, createElement(Probe)))
|
||||
})
|
||||
} finally {
|
||||
restore()
|
||||
}
|
||||
expect(loadHostsMock).toHaveBeenCalledOnce()
|
||||
if (!closeHost || !resolveHosts || !renderer) {
|
||||
throw new Error('pending-open harness did not initialize')
|
||||
}
|
||||
|
||||
act(() => closeHost?.(HOST.id))
|
||||
await act(async () => {
|
||||
resolveHosts?.([HOST])
|
||||
await hostLookup
|
||||
})
|
||||
|
||||
expect(connectMock).not.toHaveBeenCalled()
|
||||
expect(fake.closeMock).not.toHaveBeenCalled()
|
||||
act(() => renderer.unmount())
|
||||
})
|
||||
|
||||
it('does not open a client after provider unmount during an in-flight lookup', async () => {
|
||||
let resolveHosts: ((hosts: (typeof HOST)[]) => void) | null = null
|
||||
const hostLookup = new Promise<(typeof HOST)[]>((resolve) => {
|
||||
resolveHosts = resolve
|
||||
})
|
||||
connectMock.mockReturnValue(makeFakeClient('connected'))
|
||||
loadHostsMock.mockReturnValue(hostLookup)
|
||||
|
||||
let renderer: ReactTestRenderer | null = null
|
||||
function Probe(): null {
|
||||
useHostClient(HOST.id)
|
||||
return null
|
||||
}
|
||||
const restore = suppressReactTestRendererDeprecationWarning()
|
||||
try {
|
||||
act(() => {
|
||||
renderer = create(createElement(RpcClientProvider, null, createElement(Probe)))
|
||||
})
|
||||
} finally {
|
||||
restore()
|
||||
}
|
||||
expect(loadHostsMock).toHaveBeenCalledOnce()
|
||||
act(() => renderer?.unmount())
|
||||
await act(async () => {
|
||||
resolveHosts?.([HOST])
|
||||
await hostLookup
|
||||
})
|
||||
|
||||
expect(connectMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
import { connect, type RpcClient } from './rpc-client'
|
||||
import { connectionLogStore } from './connection-log-buffer'
|
||||
import { subscribeConnectionRevivalTriggers } from './connection-revival-triggers'
|
||||
import { HostClientOpenRegistry } from './host-client-open-registry'
|
||||
import { loadHosts } from './host-store'
|
||||
import type { ConnectionState, HostProfile } from './types'
|
||||
|
||||
@@ -66,7 +67,7 @@ export function RpcClientProvider({ children }: { children: ReactNode }) {
|
||||
// Pending opens (avoid two acquire() callers in the same render racing the
|
||||
// host lookup). Keyed by hostId, value is a sentinel resolved when the
|
||||
// entry materialises.
|
||||
const pendingOpensRef = useRef<Map<string, Promise<void>>>(new Map())
|
||||
const pendingOpensRef = useRef(new HostClientOpenRegistry())
|
||||
|
||||
// Why: a fast-path cache of already-loaded HostProfiles. Screens that
|
||||
// have run loadHosts() can call primeHosts() to populate this and skip
|
||||
@@ -91,19 +92,18 @@ export function RpcClientProvider({ children }: { children: ReactNode }) {
|
||||
}
|
||||
|
||||
const closeEntry = useCallback((hostId: string) => {
|
||||
pendingOpensRef.current.cancel(hostId)
|
||||
primedHostsRef.current.delete(hostId)
|
||||
const entry = storeRef.current.get(hostId)
|
||||
if (!entry) {
|
||||
return
|
||||
}
|
||||
entry.unsubState()
|
||||
entry.client.close()
|
||||
entry?.unsubState()
|
||||
storeRef.current.delete(hostId)
|
||||
entry?.client.close()
|
||||
notifyHostState(hostId, 'disconnected')
|
||||
notifyAllHosts()
|
||||
}, [])
|
||||
|
||||
const openEntry = useCallback(async (hostId: string): Promise<StoreEntry | null> => {
|
||||
const existing = pendingOpensRef.current.get(hostId)
|
||||
const existing = pendingOpensRef.current.getActivePromise(hostId)
|
||||
if (existing) {
|
||||
await existing
|
||||
return storeRef.current.get(hostId) ?? null
|
||||
@@ -112,7 +112,7 @@ export function RpcClientProvider({ children }: { children: ReactNode }) {
|
||||
const promise = new Promise<void>((res) => {
|
||||
resolve = res
|
||||
})
|
||||
pendingOpensRef.current.set(hostId, promise)
|
||||
const pendingOpen = pendingOpensRef.current.register(hostId, promise)
|
||||
|
||||
try {
|
||||
// Why: prefer the primed cache (populated by primeHosts when the
|
||||
@@ -143,6 +143,10 @@ export function RpcClientProvider({ children }: { children: ReactNode }) {
|
||||
}
|
||||
}
|
||||
|
||||
if (pendingOpen.cancelled) {
|
||||
return null
|
||||
}
|
||||
|
||||
// Re-check after any await — another acquire() may have completed.
|
||||
const after = storeRef.current.get(hostId)
|
||||
if (after) {
|
||||
@@ -184,7 +188,7 @@ export function RpcClientProvider({ children }: { children: ReactNode }) {
|
||||
notifyAllHosts()
|
||||
return entry
|
||||
} finally {
|
||||
pendingOpensRef.current.delete(hostId)
|
||||
pendingOpensRef.current.deleteIfCurrent(hostId, pendingOpen)
|
||||
resolve()
|
||||
}
|
||||
}, [])
|
||||
@@ -326,6 +330,7 @@ export function RpcClientProvider({ children }: { children: ReactNode }) {
|
||||
useEffect(() => {
|
||||
const store = storeRef.current
|
||||
return () => {
|
||||
pendingOpensRef.current.cancelAll()
|
||||
for (const [hostId] of store) {
|
||||
closeEntry(hostId)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,42 @@
|
||||
export type HostClientOpenTicket = {
|
||||
cancelled: boolean
|
||||
promise: Promise<void>
|
||||
}
|
||||
|
||||
export class HostClientOpenRegistry {
|
||||
private readonly pending = new Map<string, HostClientOpenTicket>()
|
||||
|
||||
getActivePromise(hostId: string): Promise<void> | null {
|
||||
const ticket = this.pending.get(hostId)
|
||||
return ticket && !ticket.cancelled ? ticket.promise : null
|
||||
}
|
||||
|
||||
register(hostId: string, promise: Promise<void>): HostClientOpenTicket {
|
||||
const ticket = { cancelled: false, promise }
|
||||
this.pending.set(hostId, ticket)
|
||||
return ticket
|
||||
}
|
||||
|
||||
cancel(hostId: string): void {
|
||||
const ticket = this.pending.get(hostId)
|
||||
if (ticket) {
|
||||
ticket.cancelled = true
|
||||
// Why: the host lookup may never settle; release the registry's strong
|
||||
// reference immediately while the ticket still cancels its continuation.
|
||||
this.pending.delete(hostId)
|
||||
}
|
||||
}
|
||||
|
||||
deleteIfCurrent(hostId: string, ticket: HostClientOpenTicket): void {
|
||||
if (this.pending.get(hostId) === ticket) {
|
||||
this.pending.delete(hostId)
|
||||
}
|
||||
}
|
||||
|
||||
cancelAll(): void {
|
||||
for (const ticket of this.pending.values()) {
|
||||
ticket.cancelled = true
|
||||
}
|
||||
this.pending.clear()
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,227 @@
|
||||
import AsyncStorage from '@react-native-async-storage/async-storage'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
loadPendingHostCredentialCleanupIds,
|
||||
resetHostCredentialCleanupForTests,
|
||||
retryPendingHostCredentialCleanups,
|
||||
scheduleHostCredentialCleanup,
|
||||
subscribePendingHostCredentialCleanup
|
||||
} from './host-credential-cleanup'
|
||||
|
||||
const asyncStorageMock = vi.hoisted(() => ({
|
||||
getItem: vi.fn(),
|
||||
setItem: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('@react-native-async-storage/async-storage', () => ({
|
||||
default: asyncStorageMock
|
||||
}))
|
||||
|
||||
const PENDING_STORAGE_KEY = 'orca:pending-host-credential-cleanups'
|
||||
let storedPendingIds: string[]
|
||||
let readShouldFail = false
|
||||
|
||||
async function flushMicrotasks(): Promise<void> {
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
}
|
||||
|
||||
describe('host credential cleanup', () => {
|
||||
beforeEach(() => {
|
||||
vi.useRealTimers()
|
||||
vi.clearAllMocks()
|
||||
resetHostCredentialCleanupForTests()
|
||||
storedPendingIds = []
|
||||
readShouldFail = false
|
||||
asyncStorageMock.getItem.mockImplementation(async (key: string) => {
|
||||
if (key !== PENDING_STORAGE_KEY) {
|
||||
return null
|
||||
}
|
||||
if (readShouldFail) {
|
||||
throw new Error('async storage unavailable')
|
||||
}
|
||||
return JSON.stringify(storedPendingIds)
|
||||
})
|
||||
asyncStorageMock.setItem.mockImplementation(async (key: string, raw: string) => {
|
||||
if (key === PENDING_STORAGE_KEY) {
|
||||
storedPendingIds = JSON.parse(raw) as string[]
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
resetHostCredentialCleanupForTests()
|
||||
})
|
||||
|
||||
it('clears the durable intent after a successful cleanup', async () => {
|
||||
const deleteCredential = vi.fn().mockResolvedValue(undefined)
|
||||
|
||||
await scheduleHostCredentialCleanup('host-1', deleteCredential, 20)
|
||||
await vi.waitFor(async () => {
|
||||
await expect(loadPendingHostCredentialCleanupIds()).resolves.toEqual([])
|
||||
})
|
||||
expect(deleteCredential).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('persists cleanup intent before a rejected delete for manual retry', async () => {
|
||||
const deleteCredential = vi.fn().mockRejectedValue(new Error('keychain unavailable'))
|
||||
const listener = vi.fn()
|
||||
const unsubscribe = subscribePendingHostCredentialCleanup(listener)
|
||||
|
||||
await scheduleHostCredentialCleanup('host-1', deleteCredential, 20)
|
||||
await vi.waitFor(() => expect(deleteCredential).toHaveBeenCalledOnce())
|
||||
await expect(loadPendingHostCredentialCleanupIds()).resolves.toEqual(['host-1'])
|
||||
expect(listener).toHaveBeenCalled()
|
||||
unsubscribe()
|
||||
})
|
||||
|
||||
it('records intent before a stalled delete times out', async () => {
|
||||
vi.useFakeTimers()
|
||||
const deleteCredential = vi.fn(() => new Promise<void>(() => undefined))
|
||||
|
||||
await scheduleHostCredentialCleanup('host-1', deleteCredential, 3_000)
|
||||
await flushMicrotasks()
|
||||
await expect(loadPendingHostCredentialCleanupIds()).resolves.toEqual(['host-1'])
|
||||
|
||||
await vi.advanceTimersByTimeAsync(3_000)
|
||||
await expect(loadPendingHostCredentialCleanupIds()).resolves.toEqual(['host-1'])
|
||||
expect(deleteCredential).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('returns after durable intent without waiting for the native delete', async () => {
|
||||
vi.useFakeTimers()
|
||||
const deleteCredential = vi.fn(() => new Promise<void>(() => undefined))
|
||||
|
||||
await expect(
|
||||
scheduleHostCredentialCleanup('host-1', deleteCredential, 3_000)
|
||||
).resolves.toBeUndefined()
|
||||
await flushMicrotasks()
|
||||
|
||||
expect(storedPendingIds).toEqual(['host-1'])
|
||||
expect(deleteCredential).toHaveBeenCalledOnce()
|
||||
|
||||
await vi.advanceTimersByTimeAsync(3_000)
|
||||
})
|
||||
|
||||
it('clears a timed-out pending entry when the native delete later succeeds', async () => {
|
||||
vi.useFakeTimers()
|
||||
let resolveDelete: (() => void) | null = null
|
||||
const deleteCredential = vi.fn(
|
||||
() =>
|
||||
new Promise<void>((resolve) => {
|
||||
resolveDelete = resolve
|
||||
})
|
||||
)
|
||||
|
||||
await scheduleHostCredentialCleanup('host-1', deleteCredential, 3_000)
|
||||
await flushMicrotasks()
|
||||
expect(deleteCredential).toHaveBeenCalledOnce()
|
||||
await vi.advanceTimersByTimeAsync(3_000)
|
||||
await expect(loadPendingHostCredentialCleanupIds()).resolves.toEqual(['host-1'])
|
||||
expect(resolveDelete).not.toBeNull()
|
||||
|
||||
resolveDelete?.()
|
||||
await vi.waitFor(async () => {
|
||||
await expect(loadPendingHostCredentialCleanupIds()).resolves.toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
it('joins a concurrently observed delete instead of stacking native calls', async () => {
|
||||
let resolveDelete: (() => void) | null = null
|
||||
const deleteCredential = vi.fn(
|
||||
() =>
|
||||
new Promise<void>((resolve) => {
|
||||
resolveDelete = resolve
|
||||
})
|
||||
)
|
||||
|
||||
await scheduleHostCredentialCleanup('host-1', deleteCredential, 1_000)
|
||||
await vi.waitFor(() => expect(deleteCredential).toHaveBeenCalledOnce())
|
||||
const readsBeforeSecondAttempt = asyncStorageMock.getItem.mock.calls.length
|
||||
await scheduleHostCredentialCleanup('host-1', deleteCredential, 1_000)
|
||||
await vi.waitFor(() => {
|
||||
expect(asyncStorageMock.getItem.mock.calls.length).toBeGreaterThan(readsBeforeSecondAttempt)
|
||||
})
|
||||
await flushMicrotasks()
|
||||
expect(deleteCredential).toHaveBeenCalledOnce()
|
||||
|
||||
resolveDelete?.()
|
||||
await vi.waitFor(async () => {
|
||||
await expect(loadPendingHostCredentialCleanupIds()).resolves.toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
it('starts a fresh native attempt when the user retries after a timeout', async () => {
|
||||
vi.useFakeTimers()
|
||||
const deleteCredential = vi
|
||||
.fn()
|
||||
.mockImplementationOnce(() => new Promise<void>(() => undefined))
|
||||
.mockResolvedValueOnce(undefined)
|
||||
|
||||
await scheduleHostCredentialCleanup('host-1', deleteCredential, 3_000)
|
||||
await flushMicrotasks()
|
||||
expect(deleteCredential).toHaveBeenCalledOnce()
|
||||
await vi.advanceTimersByTimeAsync(3_000)
|
||||
|
||||
await expect(retryPendingHostCredentialCleanups(deleteCredential)).resolves.toEqual({
|
||||
clearedCount: 1,
|
||||
remainingIds: []
|
||||
})
|
||||
expect(deleteCredential).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('retries only on an explicit call and clears successful ids', async () => {
|
||||
storedPendingIds = ['host-1', 'host-2']
|
||||
const deleteCredential = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new Error('still unavailable'))
|
||||
.mockResolvedValue(undefined)
|
||||
|
||||
expect(deleteCredential).not.toHaveBeenCalled()
|
||||
await expect(retryPendingHostCredentialCleanups(deleteCredential)).resolves.toEqual({
|
||||
clearedCount: 1,
|
||||
remainingIds: ['host-1']
|
||||
})
|
||||
expect(deleteCredential).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('does not wipe existing pending ids when storage read fails during mutation', async () => {
|
||||
storedPendingIds = ['host-a']
|
||||
const deleteCredential = vi.fn().mockRejectedValue(new Error('keychain unavailable'))
|
||||
|
||||
readShouldFail = true
|
||||
await scheduleHostCredentialCleanup('host-b', deleteCredential, 20)
|
||||
await vi.waitFor(() => expect(deleteCredential).toHaveBeenCalledOnce())
|
||||
readShouldFail = false
|
||||
|
||||
// Native delete still runs; durable queue is left untouched (not clobbered).
|
||||
expect(deleteCredential).toHaveBeenCalledOnce()
|
||||
await expect(loadPendingHostCredentialCleanupIds()).resolves.toEqual(['host-a'])
|
||||
expect(asyncStorageMock.setItem).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('cleans a new credential when the same host id is paired and removed again', async () => {
|
||||
const deleteCredential = vi.fn().mockResolvedValue(undefined)
|
||||
|
||||
await scheduleHostCredentialCleanup('host-1', deleteCredential, 20)
|
||||
await vi.waitFor(async () => {
|
||||
await expect(loadPendingHostCredentialCleanupIds()).resolves.toEqual([])
|
||||
})
|
||||
await scheduleHostCredentialCleanup('host-1', deleteCredential, 20)
|
||||
await vi.waitFor(async () => {
|
||||
expect(deleteCredential).toHaveBeenCalledTimes(2)
|
||||
await expect(loadPendingHostCredentialCleanupIds()).resolves.toEqual([])
|
||||
})
|
||||
|
||||
expect(deleteCredential).toHaveBeenCalledTimes(2)
|
||||
await expect(loadPendingHostCredentialCleanupIds()).resolves.toEqual([])
|
||||
})
|
||||
|
||||
it('soft-loads empty when pending-cleanup storage is malformed', async () => {
|
||||
vi.mocked(AsyncStorage.getItem).mockResolvedValueOnce('{')
|
||||
|
||||
await expect(loadPendingHostCredentialCleanupIds()).resolves.toEqual([])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,199 @@
|
||||
import AsyncStorage from '@react-native-async-storage/async-storage'
|
||||
|
||||
const PENDING_STORAGE_KEY = 'orca:pending-host-credential-cleanups'
|
||||
const CLEANUP_CONFIRM_TIMEOUT_MS = 3_000
|
||||
|
||||
type DeleteHostCredential = (hostId: string) => Promise<void>
|
||||
type CleanupAttemptResult = 'cleared' | 'pending'
|
||||
type CleanupOutcome = 'cleared' | 'failed' | 'timed-out'
|
||||
type PendingIdsRead = { ok: true; ids: string[] } | { ok: false }
|
||||
|
||||
let pendingMutation: Promise<void> = Promise.resolve()
|
||||
const pendingListeners = new Set<() => void>()
|
||||
// Why: concurrent taps/callers share one native operation while it is being
|
||||
// confirmed. A timed-out operation is released so the next user tap can retry.
|
||||
const inflightDeletes = new Map<string, Promise<void>>()
|
||||
|
||||
function parsePendingIds(raw: string): string[] | null {
|
||||
try {
|
||||
const parsed = JSON.parse(raw) as unknown
|
||||
if (!Array.isArray(parsed)) {
|
||||
return null
|
||||
}
|
||||
return [...new Set(parsed.filter((value): value is string => typeof value === 'string'))]
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
function sameIdList(a: string[], b: string[]): boolean {
|
||||
return a.length === b.length && a.every((id, index) => id === b[index])
|
||||
}
|
||||
|
||||
async function readPendingIdsForMutation(): Promise<PendingIdsRead> {
|
||||
try {
|
||||
const raw = await AsyncStorage.getItem(PENDING_STORAGE_KEY)
|
||||
if (raw === null) {
|
||||
return { ok: true, ids: [] }
|
||||
}
|
||||
const ids = parsePendingIds(raw)
|
||||
if (!ids) {
|
||||
// Why: refuse to RMW over unreadable payload — treating it as [] would
|
||||
// wipe durable pending ids on the next add/remove.
|
||||
return { ok: false }
|
||||
}
|
||||
return { ok: true, ids }
|
||||
} catch {
|
||||
return { ok: false }
|
||||
}
|
||||
}
|
||||
|
||||
async function readPendingIdsSoft(): Promise<string[]> {
|
||||
const result = await readPendingIdsForMutation()
|
||||
return result.ok ? result.ids : []
|
||||
}
|
||||
|
||||
async function mutatePendingIds(update: (ids: string[]) => string[]): Promise<void> {
|
||||
const mutation = pendingMutation.then(async () => {
|
||||
const current = await readPendingIdsForMutation()
|
||||
if (!current.ok) {
|
||||
throw new Error('pending host credential cleanup storage unreadable')
|
||||
}
|
||||
const next = update(current.ids)
|
||||
if (sameIdList(current.ids, next)) {
|
||||
return
|
||||
}
|
||||
await AsyncStorage.setItem(PENDING_STORAGE_KEY, JSON.stringify(next))
|
||||
for (const listener of pendingListeners) {
|
||||
listener()
|
||||
}
|
||||
})
|
||||
pendingMutation = mutation.catch(() => {})
|
||||
return mutation
|
||||
}
|
||||
|
||||
async function addPendingId(hostId: string): Promise<void> {
|
||||
await mutatePendingIds((ids) => (ids.includes(hostId) ? ids : [...ids, hostId]))
|
||||
}
|
||||
|
||||
async function removePendingId(hostId: string): Promise<void> {
|
||||
await mutatePendingIds((ids) => ids.filter((id) => id !== hostId))
|
||||
}
|
||||
|
||||
function observeCleanup(cleanup: Promise<void>, timeoutMs: number): Promise<CleanupOutcome> {
|
||||
return new Promise((resolve) => {
|
||||
let settled = false
|
||||
const finish = (outcome: CleanupOutcome) => {
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
settled = true
|
||||
clearTimeout(timeout)
|
||||
resolve(outcome)
|
||||
}
|
||||
const timeout = setTimeout(() => finish('timed-out'), timeoutMs)
|
||||
cleanup.then(
|
||||
() => finish('cleared'),
|
||||
() => finish('failed')
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
function startOrJoinDelete(hostId: string, deleteCredential: DeleteHostCredential): Promise<void> {
|
||||
const existing = inflightDeletes.get(hostId)
|
||||
if (existing) {
|
||||
return existing
|
||||
}
|
||||
const cleanup = Promise.resolve()
|
||||
.then(() => deleteCredential(hostId))
|
||||
.finally(() => {
|
||||
if (inflightDeletes.get(hostId) === cleanup) {
|
||||
inflightDeletes.delete(hostId)
|
||||
}
|
||||
})
|
||||
inflightDeletes.set(hostId, cleanup)
|
||||
return cleanup
|
||||
}
|
||||
|
||||
async function recordCleanupIntent(hostId: string): Promise<void> {
|
||||
try {
|
||||
await addPendingId(hostId)
|
||||
} catch {
|
||||
// Best-effort native delete can still proceed without a durable row.
|
||||
}
|
||||
}
|
||||
|
||||
async function confirmNativeCleanup(
|
||||
hostId: string,
|
||||
deleteCredential: DeleteHostCredential,
|
||||
timeoutMs: number
|
||||
): Promise<CleanupAttemptResult> {
|
||||
const cleanup = startOrJoinDelete(hostId, deleteCredential)
|
||||
// Why: attach before observing so a success that races the confirm timeout
|
||||
// still clears the durable queue entry (including after timed-out returns).
|
||||
const clearWhenDeleted = cleanup.then(
|
||||
() => removePendingId(hostId).catch(() => undefined),
|
||||
() => undefined
|
||||
)
|
||||
const outcome = await observeCleanup(cleanup, timeoutMs)
|
||||
if (outcome === 'cleared') {
|
||||
await clearWhenDeleted
|
||||
return 'cleared'
|
||||
}
|
||||
|
||||
if (outcome === 'timed-out' && inflightDeletes.get(hostId) === cleanup) {
|
||||
// Why: timing out is the boundary between automatic work and a future
|
||||
// user-owned retry. Releasing only the dedupe entry does not start work.
|
||||
inflightDeletes.delete(hostId)
|
||||
}
|
||||
|
||||
// Why: failed/unconfirmed attempts stay user-owned; nothing auto-retries.
|
||||
void clearWhenDeleted
|
||||
return 'pending'
|
||||
}
|
||||
|
||||
export async function loadPendingHostCredentialCleanupIds(): Promise<string[]> {
|
||||
await pendingMutation
|
||||
return readPendingIdsSoft()
|
||||
}
|
||||
|
||||
export function subscribePendingHostCredentialCleanup(listener: () => void): () => void {
|
||||
pendingListeners.add(listener)
|
||||
return () => pendingListeners.delete(listener)
|
||||
}
|
||||
|
||||
/**
|
||||
* Await only durable intent (AsyncStorage). Native keychain delete is
|
||||
* fire-and-forget so removeHost never blocks on SecureStore.
|
||||
*/
|
||||
export async function scheduleHostCredentialCleanup(
|
||||
hostId: string,
|
||||
deleteCredential: DeleteHostCredential,
|
||||
timeoutMs = CLEANUP_CONFIRM_TIMEOUT_MS
|
||||
): Promise<void> {
|
||||
await recordCleanupIntent(hostId)
|
||||
void confirmNativeCleanup(hostId, deleteCredential, timeoutMs).catch(() => {})
|
||||
}
|
||||
|
||||
export async function retryPendingHostCredentialCleanups(
|
||||
deleteCredential: DeleteHostCredential
|
||||
): Promise<{ clearedCount: number; remainingIds: string[] }> {
|
||||
const ids = await loadPendingHostCredentialCleanupIds()
|
||||
const outcomes = await Promise.all(
|
||||
// Why: these ids are already durable. Re-adding intent can race a late
|
||||
// success and recreate a ghost row after the credential was deleted.
|
||||
ids.map((id) => confirmNativeCleanup(id, deleteCredential, CLEANUP_CONFIRM_TIMEOUT_MS))
|
||||
)
|
||||
const remainingIds = await loadPendingHostCredentialCleanupIds()
|
||||
return {
|
||||
clearedCount: outcomes.filter((outcome) => outcome === 'cleared').length,
|
||||
remainingIds
|
||||
}
|
||||
}
|
||||
|
||||
/** Test-only: drop module listeners/in-flight state between cases. */
|
||||
export function resetHostCredentialCleanupForTests(): void {
|
||||
inflightDeletes.clear()
|
||||
pendingListeners.clear()
|
||||
pendingMutation = Promise.resolve()
|
||||
}
|
||||
@@ -0,0 +1,42 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const removeHostMock = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('./host-store', () => ({
|
||||
removeHost: (hostId: string) => removeHostMock(hostId)
|
||||
}))
|
||||
|
||||
import { removeHostAndCloseClient } from './host-removal-lifecycle'
|
||||
|
||||
describe('host removal lifecycle', () => {
|
||||
beforeEach(() => {
|
||||
removeHostMock.mockReset()
|
||||
})
|
||||
|
||||
it('closes the client only after metadata removal commits', async () => {
|
||||
let commitRemoval: (() => void) | null = null
|
||||
removeHostMock.mockReturnValue(
|
||||
new Promise<void>((resolve) => {
|
||||
commitRemoval = resolve
|
||||
})
|
||||
)
|
||||
const closeHostClient = vi.fn()
|
||||
|
||||
const removal = removeHostAndCloseClient('host-1', closeHostClient)
|
||||
expect(closeHostClient).not.toHaveBeenCalled()
|
||||
commitRemoval?.()
|
||||
await removal
|
||||
|
||||
expect(closeHostClient).toHaveBeenCalledWith('host-1')
|
||||
})
|
||||
|
||||
it('keeps the client open when metadata removal fails', async () => {
|
||||
removeHostMock.mockRejectedValue(new Error('storage unavailable'))
|
||||
const closeHostClient = vi.fn()
|
||||
|
||||
await expect(removeHostAndCloseClient('host-1', closeHostClient)).rejects.toThrow(
|
||||
'storage unavailable'
|
||||
)
|
||||
expect(closeHostClient).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,11 @@
|
||||
import { removeHost } from './host-store'
|
||||
|
||||
export async function removeHostAndCloseClient(
|
||||
hostId: string,
|
||||
closeHostClient: (hostId: string) => void
|
||||
): Promise<void> {
|
||||
// Why: closing before the metadata commit can strand a still-paired host on
|
||||
// storage failure; closing immediately after success prevents socket leaks.
|
||||
await removeHost(hostId)
|
||||
closeHostClient(hostId)
|
||||
}
|
||||
@@ -0,0 +1,167 @@
|
||||
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()
|
||||
}))
|
||||
|
||||
const scheduleCleanupMock = vi.hoisted(() => 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', () => ({
|
||||
scheduleHostCredentialCleanup: (...args: unknown[]) => scheduleCleanupMock(...args),
|
||||
retryPendingHostCredentialCleanups: vi.fn()
|
||||
}))
|
||||
|
||||
import { removeHost, renameHost, resetHostStoreForTests, updateLastConnected } from './host-store'
|
||||
|
||||
const HOSTS_STORAGE_KEY = 'orca:hosts'
|
||||
const HOST_ONE = {
|
||||
id: 'host-1',
|
||||
name: 'Host 1',
|
||||
endpoint: 'ws://127.0.0.1:1',
|
||||
publicKeyB64: 'key',
|
||||
lastConnected: 0
|
||||
}
|
||||
const HOST_TWO = {
|
||||
id: 'host-2',
|
||||
name: 'Host 2',
|
||||
endpoint: 'ws://127.0.0.1:2',
|
||||
publicKeyB64: 'key-2',
|
||||
lastConnected: 0
|
||||
}
|
||||
|
||||
describe('host-store list mutations', () => {
|
||||
let storedHostsRaw: string
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
resetHostStoreForTests()
|
||||
scheduleCleanupMock.mockReset()
|
||||
scheduleCleanupMock.mockResolvedValue(undefined)
|
||||
storedHostsRaw = JSON.stringify([HOST_ONE, HOST_TWO])
|
||||
asyncStorageMock.getItem.mockImplementation(async (key: string) => {
|
||||
if (key === HOSTS_STORAGE_KEY) {
|
||||
return storedHostsRaw
|
||||
}
|
||||
return null
|
||||
})
|
||||
asyncStorageMock.setItem.mockImplementation(async (key: string, raw: string) => {
|
||||
if (key === HOSTS_STORAGE_KEY) {
|
||||
storedHostsRaw = raw
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
it('commits the removal when credential cleanup scheduling rejects', async () => {
|
||||
scheduleCleanupMock.mockRejectedValue(new Error('intent storage unavailable'))
|
||||
|
||||
await expect(removeHost(HOST_ONE.id)).resolves.toBeUndefined()
|
||||
|
||||
expect(JSON.parse(storedHostsRaw)).toEqual([HOST_TWO])
|
||||
expect(scheduleCleanupMock).toHaveBeenCalledWith(HOST_ONE.id, expect.any(Function))
|
||||
})
|
||||
|
||||
it('awaits cleanup scheduling after metadata commit', async () => {
|
||||
let resolveSchedule: (() => void) | null = null
|
||||
scheduleCleanupMock.mockReturnValue(
|
||||
new Promise<void>((resolve) => {
|
||||
resolveSchedule = resolve
|
||||
})
|
||||
)
|
||||
|
||||
let settled = false
|
||||
const removal = removeHost(HOST_ONE.id).then(() => {
|
||||
settled = true
|
||||
})
|
||||
await vi.waitFor(() => {
|
||||
expect(JSON.parse(storedHostsRaw)).toEqual([HOST_TWO])
|
||||
})
|
||||
expect(settled).toBe(false)
|
||||
expect(scheduleCleanupMock).toHaveBeenCalledOnce()
|
||||
|
||||
resolveSchedule?.()
|
||||
await removal
|
||||
expect(settled).toBe(true)
|
||||
})
|
||||
|
||||
it('applies concurrent rename and remove without clobbering either', async () => {
|
||||
let releaseReads: (() => void) | null = null
|
||||
const readsReleased = new Promise<void>((resolve) => {
|
||||
releaseReads = resolve
|
||||
})
|
||||
let pendingReads = 0
|
||||
asyncStorageMock.getItem.mockImplementation(async (key: string) => {
|
||||
if (key !== HOSTS_STORAGE_KEY) {
|
||||
return null
|
||||
}
|
||||
pendingReads += 1
|
||||
if (pendingReads <= 2) {
|
||||
await readsReleased
|
||||
}
|
||||
return storedHostsRaw
|
||||
})
|
||||
|
||||
const rename = renameHost(HOST_ONE.id, '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.
|
||||
await vi.waitFor(() => {
|
||||
expect(pendingReads).toBeGreaterThanOrEqual(1)
|
||||
})
|
||||
releaseReads?.()
|
||||
await Promise.all([rename, remove])
|
||||
|
||||
expect(JSON.parse(storedHostsRaw)).toEqual([
|
||||
{
|
||||
...HOST_ONE,
|
||||
name: 'Renamed Host'
|
||||
}
|
||||
])
|
||||
})
|
||||
|
||||
it('preserves a rename when lastConnected updates race it', async () => {
|
||||
const before = Date.now()
|
||||
await Promise.all([
|
||||
renameHost(HOST_ONE.id, 'Alpha'),
|
||||
updateLastConnected(HOST_ONE.id),
|
||||
renameHost(HOST_TWO.id, 'Beta')
|
||||
])
|
||||
|
||||
const stored = JSON.parse(storedHostsRaw) as Array<typeof HOST_ONE>
|
||||
expect(stored).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ id: HOST_ONE.id, name: 'Alpha' }),
|
||||
expect.objectContaining({ id: HOST_TWO.id, name: 'Beta' })
|
||||
])
|
||||
)
|
||||
const hostOne = stored.find((host) => host.id === HOST_ONE.id)
|
||||
expect(hostOne?.lastConnected).toBeGreaterThanOrEqual(before)
|
||||
})
|
||||
|
||||
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/)
|
||||
expect(asyncStorageMock.setItem).not.toHaveBeenCalled()
|
||||
expect(storedHostsRaw).toBe('{')
|
||||
})
|
||||
})
|
||||
@@ -8,6 +8,10 @@ import {
|
||||
type StoredHostProfile
|
||||
} from './types'
|
||||
import { getNextHostNameFromHosts } from './host-names'
|
||||
import {
|
||||
retryPendingHostCredentialCleanups,
|
||||
scheduleHostCredentialCleanup
|
||||
} from './host-credential-cleanup'
|
||||
|
||||
const STORAGE_KEY = 'orca:hosts'
|
||||
// Why: SecureStore keys must match [A-Za-z0-9._-]; colons are rejected.
|
||||
@@ -66,8 +70,39 @@ async function deleteDeviceToken(hostId: string): Promise<void> {
|
||||
// (cleared on app uninstall, persisted across foreground/background).
|
||||
const tokenCache = new Map<string, string>()
|
||||
let inflightLoad: Promise<HostProfile[]> | null = null
|
||||
// Why: rename / lastConnected / remove / save all RMW the same hosts JSON.
|
||||
// Without a queue, concurrent writers re-read a stale snapshot and the last
|
||||
// setItem wins — resurrecting a removed host or dropping a rename.
|
||||
let hostListMutation: Promise<void> = Promise.resolve()
|
||||
|
||||
function parseStoredHosts(raw: string | null): StoredHostProfile[] | null {
|
||||
if (!raw) {
|
||||
return []
|
||||
}
|
||||
try {
|
||||
const parsed = JSON.parse(raw) as unknown
|
||||
if (!Array.isArray(parsed)) {
|
||||
return null
|
||||
}
|
||||
return parsed.flatMap((item) => {
|
||||
// Why: pre-v0.0.3 records carry the deviceToken in AsyncStorage.
|
||||
// Drop them silently — the three pre-launch users will re-pair on
|
||||
// first run rather than carry a migration shim through the auth path.
|
||||
if (item && typeof item === 'object' && 'deviceToken' in item) {
|
||||
return []
|
||||
}
|
||||
const result = StoredHostProfileSchema.safeParse(item)
|
||||
return result.success ? [result.data] : []
|
||||
})
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
export async function loadHosts(): Promise<HostProfile[]> {
|
||||
// Why: writers hold the mutation chain across their full RMW; wait so a
|
||||
// load right after rename/remove does not race a half-written list.
|
||||
await hostListMutation
|
||||
// Why: deduplicate concurrent loadHosts() calls so multiple screens
|
||||
// mounting simultaneously share one Keychain read pass.
|
||||
if (inflightLoad) {
|
||||
@@ -81,38 +116,18 @@ export async function loadHosts(): Promise<HostProfile[]> {
|
||||
|
||||
async function doLoadHosts(): Promise<HostProfile[]> {
|
||||
const raw = await AsyncStorage.getItem(STORAGE_KEY)
|
||||
if (!raw) {
|
||||
return []
|
||||
}
|
||||
let parsed: unknown
|
||||
try {
|
||||
parsed = JSON.parse(raw)
|
||||
} catch {
|
||||
return []
|
||||
}
|
||||
if (!Array.isArray(parsed)) {
|
||||
const storedHosts = parseStoredHosts(raw)
|
||||
if (!storedHosts) {
|
||||
return []
|
||||
}
|
||||
|
||||
const out: HostProfile[] = []
|
||||
for (const item of parsed) {
|
||||
// Why: pre-v0.0.3 records carry the deviceToken in AsyncStorage.
|
||||
// Drop them silently — the three pre-launch users will re-pair on
|
||||
// first run rather than carry a migration shim through the auth
|
||||
// path.
|
||||
if (item && typeof item === 'object' && 'deviceToken' in item) {
|
||||
continue
|
||||
}
|
||||
const stored = StoredHostProfileSchema.safeParse(item)
|
||||
if (!stored.success) {
|
||||
continue
|
||||
}
|
||||
|
||||
let token = tokenCache.get(stored.data.id)
|
||||
for (const stored of storedHosts) {
|
||||
let token = tokenCache.get(stored.id)
|
||||
if (!token) {
|
||||
let fetched: string | null
|
||||
try {
|
||||
fetched = await readDeviceToken(stored.data.id)
|
||||
fetched = await readDeviceToken(stored.id)
|
||||
} catch {
|
||||
// Why: a transient Keychain failure for one entry (e.g.
|
||||
// errSecInteractionNotAllowed while the device is briefly locked,
|
||||
@@ -127,37 +142,50 @@ async function doLoadHosts(): Promise<HostProfile[]> {
|
||||
continue
|
||||
}
|
||||
token = fetched
|
||||
tokenCache.set(stored.data.id, token)
|
||||
tokenCache.set(stored.id, token)
|
||||
}
|
||||
out.push({ ...stored.data, deviceToken: token })
|
||||
out.push({ ...stored, deviceToken: token })
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
async function loadStoredHosts(): Promise<StoredHostProfile[]> {
|
||||
const raw = await AsyncStorage.getItem(STORAGE_KEY)
|
||||
if (!raw) {
|
||||
return []
|
||||
}
|
||||
try {
|
||||
const parsed = JSON.parse(raw) as unknown
|
||||
if (!Array.isArray(parsed)) {
|
||||
return []
|
||||
}
|
||||
return parsed.flatMap((item) => {
|
||||
// Why: same drop-old-records rule as loadHosts; keeps internal
|
||||
// mutators from re-persisting pre-v0.0.3 entries.
|
||||
if (item && typeof item === 'object' && 'deviceToken' in item) {
|
||||
return []
|
||||
}
|
||||
const result = StoredHostProfileSchema.safeParse(item)
|
||||
return result.success ? [result.data] : []
|
||||
})
|
||||
return parseStoredHosts(await AsyncStorage.getItem(STORAGE_KEY)) ?? []
|
||||
} catch {
|
||||
return []
|
||||
}
|
||||
}
|
||||
|
||||
async function readStoredHostsForMutation(): Promise<StoredHostProfile[]> {
|
||||
try {
|
||||
const parsed = parseStoredHosts(await AsyncStorage.getItem(STORAGE_KEY))
|
||||
if (!parsed) {
|
||||
// Why: refuse to RMW over unreadable payload — treating it as [] would
|
||||
// wipe the durable host list on the next rename/remove/save.
|
||||
throw new Error('host list storage unreadable')
|
||||
}
|
||||
return parsed
|
||||
} catch (error) {
|
||||
if (error instanceof Error && error.message === 'host list storage unreadable') {
|
||||
throw error
|
||||
}
|
||||
throw new Error('host list storage unreadable')
|
||||
}
|
||||
}
|
||||
|
||||
async function mutateStoredHosts(
|
||||
update: (hosts: StoredHostProfile[]) => StoredHostProfile[]
|
||||
): Promise<void> {
|
||||
const mutation = hostListMutation.then(async () => {
|
||||
const current = await readStoredHostsForMutation()
|
||||
const next = update(current)
|
||||
await AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(next))
|
||||
})
|
||||
hostListMutation = mutation.catch(() => {})
|
||||
return mutation
|
||||
}
|
||||
|
||||
function toStored(host: HostProfile): StoredHostProfile {
|
||||
return {
|
||||
id: host.id,
|
||||
@@ -170,51 +198,77 @@ function toStored(host: HostProfile): StoredHostProfile {
|
||||
|
||||
export async function saveHost(host: HostProfile): Promise<void> {
|
||||
const validated = HostProfileSchema.parse(host)
|
||||
const hosts = await loadStoredHosts()
|
||||
const stored = toStored(validated)
|
||||
const index = hosts.findIndex((h) => h.id === stored.id)
|
||||
if (index >= 0) {
|
||||
hosts[index] = stored
|
||||
} else {
|
||||
hosts.push(stored)
|
||||
}
|
||||
await mutateStoredHosts((hosts) => {
|
||||
const index = hosts.findIndex((h) => h.id === stored.id)
|
||||
if (index >= 0) {
|
||||
const next = hosts.slice()
|
||||
next[index] = stored
|
||||
return next
|
||||
}
|
||||
return [...hosts, stored]
|
||||
})
|
||||
// Why: write metadata BEFORE the keychain token so a crash between the two
|
||||
// leaves orphaned metadata (which loadHosts skips and removeHost can clean
|
||||
// up) rather than an orphaned keychain token with no metadata pointer —
|
||||
// the latter would persist forever since removeHost only deletes by hostId
|
||||
// from current metadata.
|
||||
await AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(hosts))
|
||||
await writeDeviceToken(stored.id, validated.deviceToken)
|
||||
tokenCache.set(stored.id, validated.deviceToken)
|
||||
}
|
||||
|
||||
export async function removeHost(hostId: string): Promise<void> {
|
||||
const hosts = await loadStoredHosts()
|
||||
const filtered = hosts.filter((h) => h.id !== hostId)
|
||||
await AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(filtered))
|
||||
await deleteDeviceToken(hostId)
|
||||
await mutateStoredHosts((hosts) => hosts.filter((h) => h.id !== hostId))
|
||||
tokenCache.delete(hostId)
|
||||
}
|
||||
|
||||
export async function renameHost(hostId: string, newName: string): Promise<void> {
|
||||
const hosts = await loadStoredHosts()
|
||||
const host = hosts.find((h) => h.id === hostId)
|
||||
if (host) {
|
||||
host.name = newName
|
||||
await AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(hosts))
|
||||
// Why: await only the durable cleanup intent (AsyncStorage). Native keychain
|
||||
// delete can reject or stall and must not freeze removeHost / the UI.
|
||||
try {
|
||||
await scheduleHostCredentialCleanup(hostId, deleteDeviceToken)
|
||||
} catch {
|
||||
// Metadata is already committed; orphan-token recovery is best-effort.
|
||||
}
|
||||
}
|
||||
|
||||
export async function retryPendingHostCredentialCleanup(): Promise<{
|
||||
clearedCount: number
|
||||
remainingIds: string[]
|
||||
}> {
|
||||
return retryPendingHostCredentialCleanups(deleteDeviceToken)
|
||||
}
|
||||
|
||||
export async function renameHost(hostId: string, newName: string): Promise<void> {
|
||||
await mutateStoredHosts((hosts) => {
|
||||
const index = hosts.findIndex((h) => h.id === hostId)
|
||||
if (index < 0) {
|
||||
return hosts
|
||||
}
|
||||
const next = hosts.slice()
|
||||
next[index] = { ...next[index]!, name: newName }
|
||||
return next
|
||||
})
|
||||
}
|
||||
|
||||
export async function getNextHostName(): Promise<string> {
|
||||
await hostListMutation
|
||||
const hosts = await loadStoredHosts()
|
||||
return getNextHostNameFromHosts(hosts)
|
||||
}
|
||||
|
||||
export async function updateLastConnected(hostId: string): Promise<void> {
|
||||
const hosts = await loadStoredHosts()
|
||||
const host = hosts.find((h) => h.id === hostId)
|
||||
if (host) {
|
||||
host.lastConnected = Date.now()
|
||||
await AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(hosts))
|
||||
}
|
||||
await mutateStoredHosts((hosts) => {
|
||||
const index = hosts.findIndex((h) => h.id === hostId)
|
||||
if (index < 0) {
|
||||
return hosts
|
||||
}
|
||||
const next = hosts.slice()
|
||||
next[index] = { ...next[index]!, lastConnected: Date.now() }
|
||||
return next
|
||||
})
|
||||
}
|
||||
|
||||
/** Test-only: drain module mutation chain between cases. */
|
||||
export function resetHostStoreForTests(): void {
|
||||
hostListMutation = Promise.resolve()
|
||||
tokenCache.clear()
|
||||
inflightLoad = null
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user