Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions mobile/app/pair-confirm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
type PreProfilePairingAttempt
} from '../src/transport/pre-profile-pairing-coordinator'
import type { ConnectionLogEntry } from '../src/transport/types'
import { useCloseHost } from '../src/transport/client-context'
import { colors, spacing, radii, typography } from '../src/theme/mobile-theme'
import { ConnectionLog } from '../src/components/ConnectionLog'
import { shouldPresentNotificationOptIn } from '../src/notifications/notification-opt-in-gate'
Expand All @@ -24,6 +25,7 @@ const PAIRING_OVERALL_TIMEOUT_MS = 25_000

export default function PairConfirmScreen() {
const router = useRouter()
const closeHost = useCloseHost()
const insets = useSafeAreaInsets()
const params = useLocalSearchParams<{ code?: string }>()
const [status, setStatus] = useState<Status>('awaiting-confirm')
Expand Down Expand Up @@ -104,6 +106,13 @@ export default function PairConfirmScreen() {
if (!mountedRef.current || !attemptIsCurrent) {
return
}
// Why: re-pairing the same desktop now reuses its existing host id
// (STA-1840 dedup), so a client cached under that id from an earlier
// pairing would keep the stale endpoint/relay. Close it so the
// destination screen opens a fresh client with the newly-paired
// profile — the removeHost() path already refreshes on re-pair, and a
// brand-new host has no cached entry so this is a no-op.
closeHost(hostId)
const showNotificationOptIn = await shouldPresentNotificationOptIn()
if (!mountedRef.current) {
return
Expand Down
9 changes: 9 additions & 0 deletions mobile/app/pair-scan.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
type PreProfilePairingAttempt
} from '../src/transport/pre-profile-pairing-coordinator'
import type { ConnectionLogEntry, PairingOffer } from '../src/transport/types'
import { useCloseHost } from '../src/transport/client-context'
import { colors, spacing, radii, typography } from '../src/theme/mobile-theme'
import { TextInputModal } from '../src/components/TextInputModal'
import { ConnectionLog } from '../src/components/ConnectionLog'
Expand All @@ -43,6 +44,7 @@ function Step({ number, text }: { number: number; text: string }) {

export default function PairScanScreen() {
const router = useRouter()
const closeHost = useCloseHost()
const insets = useSafeAreaInsets()
const [permission, requestPermission] = useCameraPermissions()
const [status, setStatus] = useState<'scanning' | 'connecting' | 'error'>('scanning')
Expand Down Expand Up @@ -148,6 +150,13 @@ export default function PairScanScreen() {
if (!mountedRef.current || !attemptIsCurrent) {
return
}
// Why: re-pairing the same desktop now reuses its existing host id
// (STA-1840 dedup), so a client cached under that id from an earlier
// pairing would keep the stale endpoint/relay. Close it so the
// destination screen opens a fresh client with the newly-paired
// profile — the removeHost() path already refreshes on re-pair, and a
// brand-new host has no cached entry so this is a no-op.
closeHost(hostId)
const showNotificationOptIn = await shouldPresentNotificationOptIn()
if (!mountedRef.current) {
return
Expand Down
87 changes: 87 additions & 0 deletions mobile/src/transport/host-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ import {
MobileRelayUpgradeHostRemovedError,
removeHost,
renameHost,
resolvePairingHostIdentity,
resetHostStoreForTests,
saveHost,
saveExistingHostRelayUpgrade,
updateLastConnected
} from './host-store'
Expand Down Expand Up @@ -85,13 +87,98 @@ describe('host-store list mutations', () => {
asyncStorageMock.setItem.mockImplementation(async (key: string, raw: string) => {
if (key === HOSTS_STORAGE_KEY) {
storedHostsRaw = raw
} else if (key === OVERLAY_STORAGE_KEY) {
storedOverlayRaw = raw
}
})
secureStoreMock.getItemAsync.mockImplementation(async (key: string) =>
key.endsWith(HOST_ONE.id) || key.endsWith(HOST_TWO.id) ? `token-${key.at(-1)}` : null
)
})

it('resolves an existing host by pinned key with one durable read', async () => {
await expect(resolvePairingHostIdentity(HOST_TWO.publicKeyB64, 'host-new')).resolves.toEqual({
id: HOST_TWO.id,
name: HOST_TWO.name
})
expect(asyncStorageMock.getItem).toHaveBeenCalledOnce()
})

it('names a new host from the same durable read used for identity lookup', async () => {
await expect(resolvePairingHostIdentity('unpaired-key', 'host-new')).resolves.toEqual({
id: 'host-new',
name: 'Host 3'
})
expect(asyncStorageMock.getItem).toHaveBeenCalledOnce()
})

it('fails closed when durable host identity storage is unreadable', async () => {
asyncStorageMock.getItem.mockRejectedValueOnce(new Error('storage unavailable'))

await expect(resolvePairingHostIdentity('key-new', 'host-new')).rejects.toThrow(/unreadable/)
expect(asyncStorageMock.setItem).not.toHaveBeenCalled()
})

it('collapses already-persisted duplicates when the desktop is re-paired', async () => {
storedHostsRaw = JSON.stringify([
HOST_ONE,
{ ...HOST_TWO, id: 'host-duplicate', publicKeyB64: HOST_ONE.publicKeyB64 }
])

await saveHost({
...HOST_ONE,
endpoint: 'ws://127.0.0.1:3',
deviceToken: 'replacement-token'
})

expect(JSON.parse(storedHostsRaw)).toEqual([{ ...HOST_ONE, endpoint: 'ws://127.0.0.1:3' }])
expect(scheduleCleanupMock).toHaveBeenCalledWith('host-duplicate', expect.any(Function))
})

it('clears stale relay state when an existing host is re-paired direct-only', async () => {
const overlay: MobileRelayHostOverlay = {
v: 2,
hostId: HOST_ONE.id,
endpoints: [
{ id: 'direct-primary', kind: 'lan', url: HOST_ONE.endpoint },
{
id: 'relay-primary',
kind: 'relay',
url: 'wss://relay-c1.onorca.dev/v1/connect/AbCdEf0123_-xyZ9'
}
],
relayHostId: 'AbCdEf0123_-xyZ9',
relay: {
v: 1,
directorUrl: 'https://relay.onorca.dev',
cellUrl: 'https://relay-c1.onorca.dev',
assignmentEpoch: 7,
relayHostId: 'AbCdEf0123_-xyZ9',
e2eeFraming: 2
}
}
storedOverlayRaw = JSON.stringify([overlay])

await saveHost({ ...HOST_ONE, deviceToken: 'replacement-token' })

expect(JSON.parse(storedOverlayRaw!)).toEqual([])
expect(secureStoreMock.deleteItemAsync).not.toHaveBeenCalled()
})

it('does not touch relay storage when saving a new direct-only host', async () => {
await saveHost({
id: 'host-new',
name: 'New Host',
endpoint: 'ws://127.0.0.1:3',
publicKeyB64: 'key-new',
deviceToken: 'new-token',
lastConnected: 0
})

expect(asyncStorageMock.getItem).not.toHaveBeenCalledWith(OVERLAY_STORAGE_KEY)
expect(secureStoreMock.deleteItemAsync).not.toHaveBeenCalled()
})

it('commits the removal when credential cleanup scheduling rejects', async () => {
scheduleCleanupMock.mockRejectedValue(new Error('intent storage unavailable'))

Expand Down
60 changes: 44 additions & 16 deletions mobile/src/transport/host-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
import {
loadMobileRelayHostOverlayState,
removeMobileRelayHostOverlay,
removeMobileRelayHostOverlays,
saveMobileRelayHostOverlay
} from './mobile-relay-host-overlay-store'
import { deleteMobileRelayCredentialBundle } from './mobile-relay-credential-bundle'
Expand Down Expand Up @@ -182,12 +183,18 @@ async function doLoadHosts(): Promise<HostProfile[]> {
return out
}

async function loadStoredHosts(): Promise<StoredHostProfile[]> {
try {
return parseStoredHosts(await AsyncStorage.getItem(STORAGE_KEY)) ?? []
} catch {
return []
}
export async function resolvePairingHostIdentity(
publicKeyB64: string,
newHostId: string
): Promise<{ id: string; name: string }> {
// Why: one durable read both preserves an existing identity and names a new host,
// avoiding duplicate cards and a second serial storage read before connecting.
await hostListMutation
const hosts = await readStoredHostsForMutation()
const match = hosts.find((host) => host.publicKeyB64 === publicKeyB64)
return match
? { id: match.id, name: match.name }
: { id: newHostId, name: getNextHostNameFromHosts(hosts) }
}

async function readStoredHostsForMutation(): Promise<StoredHostProfile[]> {
Expand Down Expand Up @@ -242,18 +249,28 @@ export async function saveExistingHostRelayUpgrade(host: HostProfile): Promise<v
async function persistHost(host: HostProfile, requireExisting: boolean): Promise<void> {
const validated = HostProfileSchema.parse(host)
const stored = toStored(validated)
const duplicateHostIds = new Set<string>()
let updatedExistingHost = false
await mutateStoredHosts((hosts) => {
const index = hosts.findIndex((h) => h.id === stored.id)
for (const candidate of hosts) {
if (candidate.id !== stored.id && candidate.publicKeyB64 === stored.publicKeyB64) {
duplicateHostIds.add(candidate.id)
}
}
if (index >= 0) {
const next = hosts.slice()
next[index] = stored
return next
updatedExistingHost = true
// Why: affected installs may already contain duplicate rows; an authoritative
// save is the safe point to collapse them to the preserved host id.
return hosts
.filter(({ id }) => !duplicateHostIds.has(id))
.map((candidate) => (candidate.id === stored.id ? stored : candidate))
}
if (requireExisting) {
// Why: an in-flight relay upgrade must not resurrect a host the user removed.
throw new MobileRelayUpgradeHostRemovedError('mobile relay upgrade host was removed')
}
return [...hosts, stored]
return [...hosts.filter(({ id }) => !duplicateHostIds.has(id)), stored]
})
// Why: write metadata BEFORE the keychain token so a crash between the two
// leaves orphaned metadata (which loadHosts skips and removeHost can clean
Expand All @@ -271,6 +288,23 @@ async function persistHost(host: HostProfile, requireExisting: boolean): Promise
relay: validated.relay
})
}
const overlayRemovalIds = [...duplicateHostIds]
if (!validated.endpoints && updatedExistingHost) {
overlayRemovalIds.push(stored.id)
}
if (overlayRemovalIds.length > 0) {
// Why: reusing an id for direct-only re-pairing must not retain routing
// metadata from the host's previous transport state.
await removeMobileRelayHostOverlays(overlayRemovalIds)
}
for (const duplicateHostId of duplicateHostIds) {
tokenCache.delete(duplicateHostId)
try {
await scheduleHostCredentialCleanup(duplicateHostId, deleteHostCredentials)
} catch {
// Metadata is already deduplicated; orphan-token recovery is best-effort.
}
}
}

export async function removeHost(hostId: string): Promise<void> {
Expand Down Expand Up @@ -311,12 +345,6 @@ export async function renameHost(hostId: string, newName: string): Promise<void>
})
}

export async function getNextHostName(): Promise<string> {
await hostListMutation
const hosts = await loadStoredHosts()
return getNextHostNameFromHosts(hosts)
}

export async function updateLastConnected(hostId: string): Promise<void> {
try {
await mutateStoredHosts((hosts) => {
Expand Down
21 changes: 21 additions & 0 deletions mobile/src/transport/mobile-relay-host-overlay-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ vi.mock('@react-native-async-storage/async-storage', () => ({ default: asyncStor

import {
loadMobileRelayHostOverlays,
removeMobileRelayHostOverlays,
resetMobileRelayHostOverlayStoreForTests,
saveMobileRelayHostOverlay
} from './mobile-relay-host-overlay-store'
Expand Down Expand Up @@ -77,4 +78,24 @@ describe('mobile relay host overlay store', () => {
await expect(saveMobileRelayHostOverlay(OVERLAY)).rejects.toThrow(/unreadable/)
expect(asyncStorage.setItem).not.toHaveBeenCalled()
})

it('removes requested overlays in one storage write', async () => {
const second = { ...OVERLAY, hostId: 'host-2' }
stored = JSON.stringify([OVERLAY, second])

await expect(removeMobileRelayHostOverlays(['host-1', 'host-missing'])).resolves.toBeUndefined()

expect(JSON.parse(stored!)).toEqual([second])
expect(asyncStorage.getItem).toHaveBeenCalledOnce()
expect(asyncStorage.setItem).toHaveBeenCalledOnce()
})

it('skips the storage write when no requested overlay exists', async () => {
stored = JSON.stringify([OVERLAY])

await expect(removeMobileRelayHostOverlays(['host-missing'])).resolves.toBeUndefined()

expect(asyncStorage.getItem).toHaveBeenCalledOnce()
expect(asyncStorage.setItem).not.toHaveBeenCalled()
})
})
24 changes: 22 additions & 2 deletions mobile/src/transport/mobile-relay-host-overlay-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,12 @@ async function mutateOverlays(
): Promise<void> {
const mutation = overlayMutation.then(async () => {
const current = await readOverlaysForMutation()
await AsyncStorage.setItem(OVERLAY_STORAGE_KEY, JSON.stringify(update(current)))
const next = update(current)
// Why: direct-only saves commonly have no overlay to remove; avoid a full
// AsyncStorage write when cleanup leaves the durable list unchanged.
if (next !== current) {
await AsyncStorage.setItem(OVERLAY_STORAGE_KEY, JSON.stringify(next))
}
})
overlayMutation = mutation.catch(() => {})
return mutation
Expand Down Expand Up @@ -85,7 +90,22 @@ export async function saveMobileRelayHostOverlay(overlay: MobileRelayHostOverlay
}

export function removeMobileRelayHostOverlay(hostId: string): Promise<void> {
return mutateOverlays((overlays) => overlays.filter((overlay) => overlay.hostId !== hostId))
return removeMobileRelayHostOverlays([hostId])
}

export function removeMobileRelayHostOverlays(hostIds: readonly string[]): Promise<void> {
const targets = new Set(hostIds)
let removed = false
return mutateOverlays((overlays) => {
const next = overlays.filter((overlay) => {
if (!targets.has(overlay.hostId)) {
return true
}
removed = true
return false
})
return removed ? next : overlays
})
}

/** Test-only: drain the module mutation chain between cases. */
Expand Down
34 changes: 33 additions & 1 deletion mobile/src/transport/pre-profile-pairing-coordinator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ function dependencies(client: RpcClient, events: string[]) {
resolveInviteDirector: vi.fn(async () => {
throw new Error('director unavailable')
}),
getNextHostName: vi.fn(async () => 'Blue Whale'),
resolveHostIdentity: vi.fn(async (_publicKeyB64: string, hostId: string) => ({
id: hostId,
name: 'Blue Whale'
})),
saveHost: vi.fn(async (_host: HostProfile) => {
events.push('save-host')
}),
Expand Down Expand Up @@ -136,6 +139,35 @@ describe('pre-profile pairing coordinator', () => {
expect(events).toEqual(['connect', 'save-host'])
})

it('reuses the existing host id and name when re-pairing the same desktop key (no duplicate)', async () => {
// STA-1840: re-pairing a desktop already stored under a different id must
// merge into that card, not mint a new host-${now} and duplicate the row.
const events: string[] = []
const client = fakeClient([success({ version: '1.0.0' })])
const deps = dependencies(client, events)
deps.resolveHostIdentity = vi.fn(async (publicKeyB64: string, newHostId: string) => {
expect(publicKeyB64).toBe(directOffer.publicKeyB64)
expect(newHostId).toBe(`host-${now}`)
return { id: 'host-existing', name: 'Studio Mac' }
})

const attempt = startPreProfilePairing({
offer: directOffer,
timeoutMs: 5_000,
dependencies: deps
})

await expect(attempt.result).resolves.toEqual({ hostId: 'host-existing' })
expect(deps.saveHost).toHaveBeenCalledWith({
id: 'host-existing',
name: 'Studio Mac',
endpoint: directOffer.endpoint,
deviceToken: directOffer.deviceToken,
publicKeyB64: directOffer.publicKeyB64,
lastConnected: now
})
})

it('journals before connecting and publishes only after authoritative direct install', async () => {
const events: string[] = []
let journal: MobileRelayPairingJournal | null = null
Expand Down
Loading
Loading