From c7c0dca66931213465140842b3af27fd39bad21d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 15 Jul 2026 00:14:18 -0700 Subject: [PATCH] fix(mobile): dedupe host cards by pinned public key on re-pair (STA-1840) Re-pairing an already-paired desktop minted a fresh host-${now} id and appended a second host row, because persistHost dedups only on id and the pairing coordinator always mints a new id. The desktop CTA to re-scan for relay made this reachable in normal use, so one desktop showed as multiple hosts. Reuse the existing host id and name when the desktop pinned publicKeyB64 is already stored, so the pairing merges into the existing card. Every downstream write (journal, credential bundle, saveHost) then keys to the same id and persistHost updates in place. A new desktop key falls through to a fresh id. --- mobile/src/transport/host-store.test.ts | 13 ++++++++ mobile/src/transport/host-store.ts | 19 ++++++++++++ .../pre-profile-pairing-coordinator.test.ts | 31 +++++++++++++++++++ .../pre-profile-pairing-coordinator.ts | 16 ++++++++-- 4 files changed, 76 insertions(+), 3 deletions(-) diff --git a/mobile/src/transport/host-store.test.ts b/mobile/src/transport/host-store.test.ts index a1aa3d677a1..22ec7267785 100644 --- a/mobile/src/transport/host-store.test.ts +++ b/mobile/src/transport/host-store.test.ts @@ -34,6 +34,7 @@ vi.mock('./host-credential-cleanup', () => ({ })) import { + findStoredHostByPublicKey, loadHosts, MobileRelayUpgradeHostRemovedError, removeHost, @@ -92,6 +93,18 @@ describe('host-store list mutations', () => { ) }) + it('finds a stored host by its pinned public key (STA-1840 re-pair dedup)', async () => { + await expect(findStoredHostByPublicKey(HOST_TWO.publicKeyB64)).resolves.toEqual({ + id: HOST_TWO.id, + name: HOST_TWO.name + }) + }) + + it('returns null when no stored host has the public key, or the key is empty', async () => { + await expect(findStoredHostByPublicKey('unpaired-key')).resolves.toBeNull() + await expect(findStoredHostByPublicKey('')).resolves.toBeNull() + }) + it('commits the removal when credential cleanup scheduling rejects', async () => { scheduleCleanupMock.mockRejectedValue(new Error('intent storage unavailable')) diff --git a/mobile/src/transport/host-store.ts b/mobile/src/transport/host-store.ts index a17e6f57a1b..790eb8dc990 100644 --- a/mobile/src/transport/host-store.ts +++ b/mobile/src/transport/host-store.ts @@ -190,6 +190,25 @@ async function loadStoredHosts(): Promise { } } +// Why: re-pairing the same desktop must merge into its existing card, not mint +// a new host id and duplicate the row (STA-1840). publicKeyB64 is the desktop's +// pinned identity key — stable across re-pairs, unlike id/endpoint/relay — so +// it is the right dedup key. Reads the durable stored list (not loadHosts) so a +// host whose keychain token read is transiently failing still matches and is +// not duplicated. +export async function findStoredHostByPublicKey( + publicKeyB64: string +): Promise<{ id: string; name: string } | null> { + if (!publicKeyB64) { + return null + } + // Why: settle any in-flight save/remove first so a re-pair racing its own + // prior write matches the just-persisted host rather than an empty snapshot. + await hostListMutation + const match = (await loadStoredHosts()).find((h) => h.publicKeyB64 === publicKeyB64) + return match ? { id: match.id, name: match.name } : null +} + async function readStoredHostsForMutation(): Promise { try { const parsed = parseStoredHosts(await AsyncStorage.getItem(STORAGE_KEY)) diff --git a/mobile/src/transport/pre-profile-pairing-coordinator.test.ts b/mobile/src/transport/pre-profile-pairing-coordinator.test.ts index 16c833bf4f2..8f39e2b36a2 100644 --- a/mobile/src/transport/pre-profile-pairing-coordinator.test.ts +++ b/mobile/src/transport/pre-profile-pairing-coordinator.test.ts @@ -65,6 +65,7 @@ function dependencies(client: RpcClient, events: string[]) { throw new Error('director unavailable') }), getNextHostName: vi.fn(async () => 'Blue Whale'), + findExistingHost: vi.fn(async (_publicKeyB64: string) => null), saveHost: vi.fn(async (_host: HostProfile) => { events.push('save-host') }), @@ -136,6 +137,36 @@ 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.findExistingHost = vi.fn(async (publicKeyB64: string) => { + expect(publicKeyB64).toBe(directOffer.publicKeyB64) + 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 + }) + // Existing host keeps its name — no fresh name minted. + expect(deps.getNextHostName).not.toHaveBeenCalled() + }) + it('journals before connecting and publishes only after authoritative direct install', async () => { const events: string[] = [] let journal: MobileRelayPairingJournal | null = null diff --git a/mobile/src/transport/pre-profile-pairing-coordinator.ts b/mobile/src/transport/pre-profile-pairing-coordinator.ts index 9d2a3b48ddb..b66aeeb12e7 100644 --- a/mobile/src/transport/pre-profile-pairing-coordinator.ts +++ b/mobile/src/transport/pre-profile-pairing-coordinator.ts @@ -6,7 +6,7 @@ import { type MobileRelayEndpoint } from '../../../src/shared/mobile-relay-credential-contract' import { connect, type ConnectOptions } from './rpc-client' -import { getNextHostName, saveHost } from './host-store' +import { findStoredHostByPublicKey, getNextHostName, saveHost } from './host-store' import type { HostProfile, PairingOffer, RpcResponse } from './types' import { createMobileRelayPairingJournal, @@ -40,6 +40,7 @@ type Dependencies = { connectRelay: typeof connectMobileRelayForPairing resolveInviteDirector: typeof resolvePairingInviteThroughDirector getNextHostName: typeof getNextHostName + findExistingHost: typeof findStoredHostByPublicKey saveHost: typeof saveHost saveJournal: typeof saveMobileRelayPairingJournal updateJournal: typeof updateMobileRelayPairingJournal @@ -54,6 +55,7 @@ const defaultDependencies: Dependencies = { connectRelay: connectMobileRelayForPairing, resolveInviteDirector: resolvePairingInviteThroughDirector, getNextHostName, + findExistingHost: findStoredHostByPublicKey, saveHost, saveJournal: saveMobileRelayPairingJournal, updateJournal: updateMobileRelayPairingJournal, @@ -130,8 +132,16 @@ async function runPairing( isDisposed: () => boolean ): Promise<{ hostId: string }> { const now = dependencies.now() - const hostId = `host-${now}` - const hostName = await dependencies.getNextHostName() + // Why: if this desktop (by its pinned public key) is already paired, reuse + // that host's id and name so the pairing merges into the existing card + // instead of minting a fresh host-${now} and duplicating the row (STA-1840). + // Every downstream write (journal, credential bundle, saveHost) then keys to + // the same id, so persistHost updates in place. A new desktop key falls + // through to a fresh id/name. + const existing = await dependencies.findExistingHost(offer.publicKeyB64) + assertActive(isDisposed) + const hostId = existing?.id ?? `host-${now}` + const hostName = existing?.name ?? (await dependencies.getNextHostName()) assertActive(isDisposed) let journal: MobileRelayPairingJournal | null = null if (offer.relay && dependencies.platform !== 'web') {