diff --git a/src/__tests__/unit/components/ConnectionProvider.test.tsx b/src/__tests__/unit/components/ConnectionProvider.test.tsx index 3793c7f..4d223c7 100644 --- a/src/__tests__/unit/components/ConnectionProvider.test.tsx +++ b/src/__tests__/unit/components/ConnectionProvider.test.tsx @@ -5,8 +5,14 @@ * connection lifecycle handling, and notification processing. */ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { Capacitor } from "@capacitor/core"; import { Preferences } from "@capacitor/preferences"; +import { + __simulateDeviceDiscovered, + ZeroConf, + type ZeroConfService, +} from "../../../../__mocks__/capacitor-zeroconf"; import { QueryClient } from "@tanstack/react-query"; import { act, render, screen, waitFor } from "../../../test-utils"; import { ConnectionProvider } from "../../../components/ConnectionProvider"; @@ -178,13 +184,10 @@ vi.mock("@capacitor/app", () => ({ }, })); -vi.mock("@capacitor/core", () => ({ - Capacitor: { - isNativePlatform: vi.fn(() => false), - isPluginAvailable: vi.fn(() => true), - }, -})); - +// `@capacitor/core` deliberately uses the shared `__mocks__` module rather than +// a local factory: test-setup imports `useNetworkScan`, so that hook resolves +// the shared mock, and a local factory here would leave the provider and the +// hook disagreeing about whether the platform is native. vi.mock("@capacitor/network", () => ({ Network: { addListener: vi.fn().mockResolvedValue({ remove: vi.fn() }), @@ -408,6 +411,251 @@ describe("ConnectionProvider", () => { }); }); + // iOS WebSockets do not reliably bootstrap `.local` resolution themselves, so + // the provider browses mDNS and dials the address it resolves while the record + // keeps the hostname that survives the device moving. + describe("resolving a .local device", () => { + const MDNS_RECORD_ID = "record-mdns"; + + const advertise = ( + overrides: Partial = {}, + ): ZeroConfService => ({ + domain: "local.", + type: "_zaparoo._tcp.", + name: "Steam Deck", + port: 7497, + hostname: "steamdeck.local", + ipv4Addresses: ["10.0.0.206"], + ipv6Addresses: [], + txtRecord: { id: "core-id" }, + ...overrides, + }); + + async function seedMdnsDevice() { + await seedDeviceRegistry( + [ + mockDeviceRecord({ + recordId: MDNS_RECORD_ID, + address: "steamdeck.local", + source: "mdns", + discoveryId: "core-id", + }), + ], + MDNS_RECORD_ID, + ); + } + + beforeEach(async () => { + vi.mocked(Capacitor.isNativePlatform).mockReturnValue(true); + vi.mocked(connectionManager.getActiveDeviceId).mockReturnValue( + MDNS_RECORD_ID, + ); + await seedMdnsDevice(); + }); + + afterEach(() => { + vi.mocked(Capacitor.isNativePlatform).mockReturnValue(false); + }); + + it("should browse for a hostname it has not resolved yet", async () => { + render( + +
Test
+
, + ); + + await waitFor(() => { + expect(ZeroConf.watch).toHaveBeenCalled(); + }); + }); + + it("should not browse for a device reached by address", async () => { + await resetStore(); + + render( + +
Test
+
, + ); + + await waitFor(() => { + expect(connectionManager.addDevice).toHaveBeenCalled(); + }); + expect(ZeroConf.watch).not.toHaveBeenCalled(); + }); + + it("should dial the resolved address while the record keeps its hostname", async () => { + render( + +
Test
+
, + ); + + await waitFor(() => { + expect(ZeroConf.watch).toHaveBeenCalled(); + }); + act(() => { + __simulateDeviceDiscovered(advertise()); + }); + + await waitFor(() => { + expect(connectionManager.addDevice).toHaveBeenCalledWith( + expect.objectContaining({ + deviceId: MDNS_RECORD_ID, + address: "ws://10.0.0.206:7497/api/v0.1", + }), + ); + }); + expect(deviceRegistry.activeEndpoint()?.address).toBe("steamdeck.local"); + }); + + it("should follow the hostname to a new address", async () => { + render( + +
Test
+
, + ); + + await waitFor(() => { + expect(ZeroConf.watch).toHaveBeenCalled(); + }); + act(() => { + __simulateDeviceDiscovered(advertise()); + }); + await waitFor(() => { + expect(connectionManager.addDevice).toHaveBeenCalledWith( + expect.objectContaining({ address: "ws://10.0.0.206:7497/api/v0.1" }), + ); + }); + + act(() => { + __simulateDeviceDiscovered( + advertise({ ipv4Addresses: ["10.0.0.219"] }), + ); + }); + + await waitFor(() => { + expect(connectionManager.addDevice).toHaveBeenCalledWith( + expect.objectContaining({ address: "ws://10.0.0.219:7497/api/v0.1" }), + ); + }); + }); + + // DHCP hands leases around, so adopting a stranger's resolution because it + // happens to answer on a familiar address would point the socket at the + // wrong box entirely. + it("should ignore an announcement from a different device", async () => { + render( + +
Test
+
, + ); + + await waitFor(() => { + expect(ZeroConf.watch).toHaveBeenCalled(); + }); + act(() => { + __simulateDeviceDiscovered( + advertise({ + name: "Someone else", + hostname: "basement.local", + txtRecord: { id: "other-core" }, + }), + ); + }); + + await waitFor(() => { + expect(connectionManager.addDevice).toHaveBeenCalled(); + }); + expect(connectionManager.addDevice).not.toHaveBeenCalledWith( + expect.objectContaining({ address: "ws://10.0.0.206:7497/api/v0.1" }), + ); + }); + + it("should match an announcement without a device id on hostname and port", async () => { + await seedDeviceRegistry( + [ + mockDeviceRecord({ + recordId: MDNS_RECORD_ID, + address: "steamdeck.local", + source: "mdns", + }), + ], + MDNS_RECORD_ID, + ); + + render( + +
Test
+
, + ); + + await waitFor(() => { + expect(ZeroConf.watch).toHaveBeenCalled(); + }); + act(() => { + __simulateDeviceDiscovered(advertise({ txtRecord: {} })); + }); + + await waitFor(() => { + expect(connectionManager.addDevice).toHaveBeenCalledWith( + expect.objectContaining({ address: "ws://10.0.0.206:7497/api/v0.1" }), + ); + }); + }); + + // Browsing costs battery and multicast traffic, so it runs only until the + // hostname is resolved on a live connection. + it("should stop browsing once the connection is up", async () => { + render( + +
Test
+
, + ); + + await waitFor(() => { + expect(ZeroConf.watch).toHaveBeenCalled(); + }); + act(() => { + __simulateDeviceDiscovered(advertise()); + }); + + act(() => { + capturedEventHandlers.onConnectionChange!(MDNS_RECORD_ID, { + state: "connected", + hasData: false, + hasConnectedBefore: false, + }); + }); + + await waitFor(() => { + expect(ZeroConf.unwatch).toHaveBeenCalled(); + }); + }); + + it("should keep browsing while the connection is still down", async () => { + render( + +
Test
+
, + ); + + await waitFor(() => { + expect(ZeroConf.watch).toHaveBeenCalled(); + }); + act(() => { + __simulateDeviceDiscovered(advertise()); + }); + + await waitFor(() => { + expect(connectionManager.addDevice).toHaveBeenCalledWith( + expect.objectContaining({ address: "ws://10.0.0.206:7497/api/v0.1" }), + ); + }); + expect(ZeroConf.unwatch).not.toHaveBeenCalled(); + }); + }); + describe("cleanup", () => { it("should remove device on unmount", () => { const { unmount } = render( diff --git a/src/__tests__/unit/hooks/useNetworkScan.test.ts b/src/__tests__/unit/hooks/useNetworkScan.test.ts index ddc8c1c..7848c71 100644 --- a/src/__tests__/unit/hooks/useNetworkScan.test.ts +++ b/src/__tests__/unit/hooks/useNetworkScan.test.ts @@ -293,6 +293,7 @@ describe("useNetworkScan", () => { expect(result.current.devices[0]).toEqual({ name: "test-device", address: "192.168.1.100", + addresses: ["192.168.1.100"], hostname: "test-device.local", port: 7497, deviceId: "device-123", @@ -327,6 +328,59 @@ describe("useNetworkScan", () => { expect(device?.address).toBe("fe80::1"); }); + // The first address is the one the socket dials, and IPv6 link-local needs + // a scope id the WebSocket URL has nowhere to put. + it("should prefer IPv4 over IPv6 for the address it hands out", async () => { + const { useNetworkScan: hook } = + await import("../../../hooks/useNetworkScan"); + const { result } = renderHook(() => hook()); + + await act(async () => { + await result.current.startScan(); + }); + + act(() => { + watchCallback?.({ + action: "resolved", + service: { + name: "dual-stack", + port: 7497, + ipv4Addresses: ["192.168.1.100"], + ipv6Addresses: ["fe80::1"], + }, + }); + }); + + expect(result.current.devices[0]).toMatchObject({ + address: "192.168.1.100", + addresses: ["192.168.1.100", "fe80::1"], + }); + }); + + it("should not repeat an address advertised on both stacks", async () => { + const { useNetworkScan: hook } = + await import("../../../hooks/useNetworkScan"); + const { result } = renderHook(() => hook()); + + await act(async () => { + await result.current.startScan(); + }); + + act(() => { + watchCallback?.({ + action: "resolved", + service: { + name: "duplicated", + port: 7497, + ipv4Addresses: ["192.168.1.100"], + ipv6Addresses: ["192.168.1.100"], + }, + }); + }); + + expect(result.current.devices[0]?.addresses).toEqual(["192.168.1.100"]); + }); + it("should ignore service without IP address", async () => { const { useNetworkScan: hook } = await import("../../../hooks/useNetworkScan"); @@ -745,12 +799,100 @@ describe("useNetworkScan", () => { expect(result.current.devices[0]).toEqual({ name: "basic-device", address: "192.168.1.100", + addresses: ["192.168.1.100"], port: 7497, // No deviceId, version, platform }); }); }); + // A multi-homed device announces one interface at a time, so a re-announcement + // is usually a partial view rather than a correction. + describe("re-announcements from a multi-homed device", () => { + const announce = ( + ipv4Addresses: string[], + hostname = "steamdeck.local.", + ) => ({ + action: "resolved" as const, + service: { + name: "steamdeck", + hostname, + port: 7497, + ipv4Addresses, + ipv6Addresses: [], + txtRecord: { id: "device-123" }, + }, + }); + + it("should collect every interface it hears about", async () => { + const { useNetworkScan: hook } = + await import("../../../hooks/useNetworkScan"); + const { result } = renderHook(() => hook()); + + await act(async () => { + await result.current.startScan(); + }); + + act(() => { + watchCallback?.(announce(["192.168.1.100"])); + }); + act(() => { + watchCallback?.(announce(["192.168.1.100", "10.0.0.5"])); + }); + + expect(result.current.devices).toHaveLength(1); + expect(result.current.devices[0]?.addresses).toEqual([ + "192.168.1.100", + "10.0.0.5", + ]); + }); + + // Flipping `address` here would repoint a live socket at the other + // interface for no reason. + it("should keep the address in use while it is still advertised", async () => { + const { useNetworkScan: hook } = + await import("../../../hooks/useNetworkScan"); + const { result } = renderHook(() => hook()); + + await act(async () => { + await result.current.startScan(); + }); + + act(() => { + watchCallback?.(announce(["192.168.1.100"])); + }); + act(() => { + watchCallback?.(announce(["10.0.0.5", "192.168.1.100"])); + }); + + expect(result.current.devices[0]?.address).toBe("192.168.1.100"); + }); + + it("should follow the device when its address stops being advertised", async () => { + const { useNetworkScan: hook } = + await import("../../../hooks/useNetworkScan"); + const { result } = renderHook(() => hook()); + + await act(async () => { + await result.current.startScan(); + }); + + act(() => { + watchCallback?.(announce(["192.168.1.100"])); + }); + // The DHCP lease moved, so the old address is dropped rather than kept + // around as an address that no longer answers. + act(() => { + watchCallback?.(announce(["10.0.0.5"])); + }); + + expect(result.current.devices[0]).toMatchObject({ + address: "10.0.0.5", + addresses: ["10.0.0.5"], + }); + }); + }); + describe("cleanup", () => { it("should stop scan on unmount", async () => { const { useNetworkScan: hook } = @@ -769,26 +911,96 @@ describe("useNetworkScan", () => { }); }); - describe("restarting scan", () => { - it("should stop existing scan before starting new one", async () => { + // There is one ZeroConf watch for the whole app. The connection provider + // browses to resolve a `.local` hostname at the same time the scan modal + // browses to list devices, so ownership is counted rather than toggled. + describe("sharing one watch between callers", () => { + it("should not restart the watch when the same caller scans again", async () => { const { useNetworkScan: hook } = await import("../../../hooks/useNetworkScan"); const { result } = renderHook(() => hook()); - // Start first scan await act(async () => { await result.current.startScan(); }); - - // Start second scan await act(async () => { await result.current.startScan(); }); - // unwatch should have been called for the first scan - expect(mockUnwatch).toHaveBeenCalledTimes(1); - // watch should have been called twice - expect(mockWatch).toHaveBeenCalledTimes(2); + expect(mockWatch).toHaveBeenCalledTimes(1); + expect(mockUnwatch).not.toHaveBeenCalled(); + }); + + it("should keep watching while a second caller is still scanning", async () => { + const { useNetworkScan: hook } = + await import("../../../hooks/useNetworkScan"); + const { result: modalScan } = renderHook(() => hook()); + const { result: providerScan } = renderHook(() => hook()); + + await act(async () => { + await modalScan.current.startScan(); + await providerScan.current.startScan(); + }); + + expect(mockWatch).toHaveBeenCalledTimes(1); + + await act(async () => { + modalScan.current.stopScan(); + }); + + expect(mockUnwatch).not.toHaveBeenCalled(); + expect(providerScan.current.isScanning).toBe(true); + }); + + it("should stop watching once the last caller stops", async () => { + const { useNetworkScan: hook } = + await import("../../../hooks/useNetworkScan"); + const { result: modalScan } = renderHook(() => hook()); + const { result: providerScan } = renderHook(() => hook()); + + await act(async () => { + await modalScan.current.startScan(); + await providerScan.current.startScan(); + }); + + await act(async () => { + modalScan.current.stopScan(); + providerScan.current.stopScan(); + }); + + await waitFor(() => { + expect(mockUnwatch).toHaveBeenCalledTimes(1); + }); + }); + + it("should deliver discoveries to every scanning caller", async () => { + const { useNetworkScan: hook } = + await import("../../../hooks/useNetworkScan"); + const { result: modalScan } = renderHook(() => hook()); + const { result: providerScan } = renderHook(() => hook()); + + await act(async () => { + await modalScan.current.startScan(); + await providerScan.current.startScan(); + }); + + act(() => { + watchCallback?.({ + action: "resolved", + service: { + name: "test-device", + hostname: "test-device.local", + port: 7497, + ipv4Addresses: ["192.168.1.100"], + ipv6Addresses: [], + }, + }); + }); + + await waitFor(() => { + expect(modalScan.current.devices).toHaveLength(1); + expect(providerScan.current.devices).toHaveLength(1); + }); }); }); }); diff --git a/src/__tests__/unit/lib/devices/deviceRegistry.test.ts b/src/__tests__/unit/lib/devices/deviceRegistry.test.ts index bf8a7f1..0acf202 100644 --- a/src/__tests__/unit/lib/devices/deviceRegistry.test.ts +++ b/src/__tests__/unit/lib/devices/deviceRegistry.test.ts @@ -15,6 +15,8 @@ import { DEVICE_REGISTRY_KEY, deviceRegistry, parseDeviceRegistry, + parsedEndpointForRecord, + resolvedEndpointForRecord, type DeviceRecord, } from "@/lib/devices/deviceRegistry"; import { @@ -41,6 +43,11 @@ function records(): DeviceRecord[] { return Object.values(deviceRegistry.getSnapshot().records); } +/** Re-read a record from the live snapshot rather than trusting a stale copy. */ +function recordById(recordId: string): DeviceRecord | undefined { + return deviceRegistry.getSnapshot().records[recordId]; +} + beforeEach(async () => { __resetDeviceRegistryForTests(); localStorage.clear(); @@ -591,6 +598,123 @@ describe("selecting a discovered device", () => { await deviceRegistry.selectDiscovered({ addresses: [], port: 7497 }), ).toBeNull(); }); + + it("should rewrite the record when the announced addresses change", async () => { + await deviceRegistry.selectDiscovered(announcement); + vi.clearAllMocks(); + + const moved = await deviceRegistry.selectDiscovered({ + ...announcement, + addresses: ["10.0.0.219"], + }); + + expect(Preferences.set).toHaveBeenCalled(); + expect(resolvedEndpointForRecord(moved)?.host).toBe("10.0.0.219"); + }); +}); + +// iOS WebSockets don't reliably resolve `.local` themselves, so the socket +// dials a resolved address while the record keeps the hostname that survives +// the device moving. +describe("resolving an mDNS hostname to an address", () => { + const announcement = { + discoveryId: "core-id", + hostname: "steamdeck.local", + addresses: ["10.0.0.206", "fe80::1"], + port: 7497, + }; + + it("should dial the resolved address while displaying the hostname", async () => { + const record = await deviceRegistry.selectDiscovered(announcement); + + expect(parsedEndpointForRecord(record)?.address).toBe("steamdeck.local"); + expect(resolvedEndpointForRecord(record)?.host).toBe("10.0.0.206"); + expect(resolvedEndpointForRecord(record)?.wsUrl).toBe( + "ws://10.0.0.206:7497/api/v0.1", + ); + }); + + it("should keep the endpoint's port and scheme when swapping the host", async () => { + const record = await deviceRegistry.selectDiscovered({ + ...announcement, + port: 8080, + }); + + expect(resolvedEndpointForRecord(record)?.wsUrl).toBe( + "ws://10.0.0.206:8080/api/v0.1", + ); + }); + + it("should dial the hostname when nothing has resolved it", async () => { + const record = await deviceRegistry.selectAddress("steamdeck.local"); + + expect(resolvedEndpointForRecord(record)?.host).toBe("steamdeck.local"); + }); + + it("should attach resolution to a record the user typed by hand", async () => { + const typed = await deviceRegistry.selectAddress("steamdeck.local"); + + await deviceRegistry.noteResolvedAddresses(typed!.recordId, ["10.0.0.206"]); + + expect(resolvedEndpointForRecord(recordById(typed!.recordId))?.host).toBe( + "10.0.0.206", + ); + }); + + // A background browse runs behind a live connection, so it must never create + // a record or move the user off the one they selected. + it("should not create or activate a record it does not already know", async () => { + const typed = await deviceRegistry.selectAddress("steamdeck.local"); + + await deviceRegistry.noteResolvedAddresses("no-such-record", [ + "10.0.0.206", + ]); + + expect(records()).toHaveLength(1); + expect(deviceRegistry.getSnapshot().activeRecordId).toBe(typed!.recordId); + }); + + it("should not write when the resolution is unchanged", async () => { + const typed = await deviceRegistry.selectAddress("steamdeck.local"); + await deviceRegistry.noteResolvedAddresses(typed!.recordId, ["10.0.0.206"]); + vi.clearAllMocks(); + + await deviceRegistry.noteResolvedAddresses(typed!.recordId, ["10.0.0.206"]); + + expect(Preferences.set).not.toHaveBeenCalled(); + }); + + it("should ignore an announcement that resolved to nothing", async () => { + const typed = await deviceRegistry.selectAddress("steamdeck.local"); + await deviceRegistry.noteResolvedAddresses(typed!.recordId, ["10.0.0.206"]); + + await deviceRegistry.noteResolvedAddresses(typed!.recordId, []); + + expect(resolvedEndpointForRecord(recordById(typed!.recordId))?.host).toBe( + "10.0.0.206", + ); + }); + + it("should follow the device to its new address", async () => { + const typed = await deviceRegistry.selectAddress("steamdeck.local"); + await deviceRegistry.noteResolvedAddresses(typed!.recordId, ["10.0.0.206"]); + + await deviceRegistry.noteResolvedAddresses(typed!.recordId, ["10.0.0.219"]); + + expect(resolvedEndpointForRecord(recordById(typed!.recordId))?.host).toBe( + "10.0.0.219", + ); + }); + + it("should survive a reload", async () => { + const discovered = await deviceRegistry.selectDiscovered(announcement); + __resetDeviceRegistryForTests(); + await deviceRegistry.hydrate(); + + expect( + resolvedEndpointForRecord(recordById(discovered!.recordId))?.host, + ).toBe("10.0.0.206"); + }); }); describe("recording a successful connection", () => { diff --git a/src/__tests__/unit/lib/devices/endpoint.test.ts b/src/__tests__/unit/lib/devices/endpoint.test.ts index 218e795..8375c26 100644 --- a/src/__tests__/unit/lib/devices/endpoint.test.ts +++ b/src/__tests__/unit/lib/devices/endpoint.test.ts @@ -12,6 +12,7 @@ import { formatDeviceEndpoint, isValidHost, parseDeviceEndpoint, + replaceDeviceEndpointHost, } from "@/lib/devices/endpoint"; function endpointOf(input: string) { @@ -201,6 +202,45 @@ describe("formatDeviceEndpoint", () => { }); }); +describe("replaceDeviceEndpointHost", () => { + it("should keep the port and scheme when swapping the host", () => { + const swapped = replaceDeviceEndpointHost( + endpointOf("wss://steamdeck.local:8080"), + "10.0.0.206", + ); + + expect(swapped.wsUrl).toBe("wss://10.0.0.206:8080/api/v0.1"); + }); + + it("should bracket an IPv6 replacement", () => { + const swapped = replaceDeviceEndpointHost( + endpointOf("steamdeck.local"), + "fe80::1", + ); + + expect(swapped.address).toBe("[fe80::1]"); + }); + + it("should give the swapped host its own endpoint id", () => { + const original = endpointOf("steamdeck.local"); + + const swapped = replaceDeviceEndpointHost(original, "10.0.0.206"); + + expect(swapped.endpointId).not.toBe(original.endpointId); + }); + + // A bad advertisement degrades to the hostname the record already had rather + // than pointing the socket at nonsense. + it.each(["", "999.999.999.999", "not a host"])( + "should keep the original endpoint when handed %s", + (host) => { + const original = endpointOf("steamdeck.local"); + + expect(replaceDeviceEndpointHost(original, host)).toEqual(original); + }, + ); +}); + describe("isValidHost", () => { it.each(["192.168.1.100", "127.0.0.1", "::1", "mydevice.local", "core"])( "should accept %s", diff --git a/src/__tests__/unit/routes/settings.licenses.test.tsx b/src/__tests__/unit/routes/settings.licenses.test.tsx index 72eecfe..6afb5ba 100644 --- a/src/__tests__/unit/routes/settings.licenses.test.tsx +++ b/src/__tests__/unit/routes/settings.licenses.test.tsx @@ -78,7 +78,10 @@ const getLicenses = (): ComponentType => { return componentRef.current; }; -describe("Settings Licenses Route", () => { +// Every test here renders the full production dependency list, which is slow +// enough on its own to reach the default 10s timeout when the suite runs all +// files in parallel. +describe("Settings Licenses Route", { timeout: 30000 }, () => { beforeEach(() => { vi.clearAllMocks(); vi.stubGlobal( diff --git a/src/components/ConnectionProvider.tsx b/src/components/ConnectionProvider.tsx index facdeb8..a4b4c0d 100644 --- a/src/components/ConnectionProvider.tsx +++ b/src/components/ConnectionProvider.tsx @@ -70,9 +70,9 @@ import { credentialStore, } from "@/lib/crypto/credentials"; import { - activeAddressOf, deviceRegistry, parsedEndpointForRecord, + resolvedEndpointForRecord, useDeviceRegistry, type DeviceRegistrySnapshot, } from "@/lib/devices/deviceRegistry"; @@ -82,6 +82,7 @@ import { ConnectionContext, type ConnectionContextValue, } from "@/hooks/useConnection"; +import { useNetworkScan } from "@/hooks/useNetworkScan"; import { useAnnouncer } from "./A11yAnnouncer"; import { PairingModal } from "./PairingModal"; @@ -92,18 +93,44 @@ interface ConnectionProviderProps { const selectActiveRecordId = (state: DeviceRegistrySnapshot) => state.activeRecordId; +const activeRecordOf = (state: DeviceRegistrySnapshot) => + state.activeRecordId ? (state.records[state.activeRecordId] ?? null) : null; + /** * The WebSocket URL the active record connects through, or `""` when there is * no usable device. Selected as a primitive so a metadata-only registry write * cannot tear the socket down and rebuild it. + * + * Resolved rather than canonical: on iOS a WebSocket to a `.local` name does + * not reliably bootstrap multicast resolution, so the socket dials the address + * mDNS resolved while the record keeps the hostname. + */ +const selectConnectionWsUrl = (state: DeviceRegistrySnapshot) => + resolvedEndpointForRecord(activeRecordOf(state))?.wsUrl ?? ""; + +/** The address pairing runs against — resolved, for the same reason. */ +const selectConnectionAddress = (state: DeviceRegistrySnapshot) => + resolvedEndpointForRecord(activeRecordOf(state))?.address ?? ""; + +/** + * The `.local` name the active record connects through, or `""` when it does + * not use one. mDNS browsing only exists to resolve these. */ -const selectActiveWsUrl = (state: DeviceRegistrySnapshot) => { - const record = state.activeRecordId - ? state.records[state.activeRecordId] - : null; - return parsedEndpointForRecord(record)?.wsUrl ?? ""; +const selectMdnsHostname = (state: DeviceRegistrySnapshot) => { + const host = parsedEndpointForRecord(activeRecordOf(state))?.host ?? ""; + return host.endsWith(".local") ? host : ""; }; +const selectMdnsPort = (state: DeviceRegistrySnapshot) => + parsedEndpointForRecord(activeRecordOf(state))?.port ?? 0; + +const selectActiveDiscoveryId = (state: DeviceRegistrySnapshot) => + activeRecordOf(state)?.discoveryId ?? ""; + +function normalizeMdnsHostname(hostname: string): string { + return hostname.trim().toLowerCase().replace(/\.+$/, ""); +} + /** * Absorb a registry write failure that has already been reported. * @@ -248,8 +275,16 @@ export function ConnectionProvider({ children }: ConnectionProviderProps) { ); const activeRecordId = useDeviceRegistry(selectActiveRecordId); - const connectionWsUrl = useDeviceRegistry(selectActiveWsUrl); - const connectionAddress = useDeviceRegistry(activeAddressOf); + const connectionWsUrl = useDeviceRegistry(selectConnectionWsUrl); + const connectionAddress = useDeviceRegistry(selectConnectionAddress); + const mdnsHostname = useDeviceRegistry(selectMdnsHostname); + const mdnsPort = useDeviceRegistry(selectMdnsPort); + const activeDiscoveryId = useDeviceRegistry(selectActiveDiscoveryId); + const { + devices: discoveredDevices, + startScan: startMdnsScan, + stopScan: stopMdnsScan, + } = useNetworkScan(); // Connection state tracked via useState to prevent unnecessary re-renders // These are updated via onConnectionChange callback @@ -274,6 +309,57 @@ export function ConnectionProvider({ children }: ConnectionProviderProps) { // Show "Reconnecting..." for devices that had prior successful connection const showReconnecting = !isConnected && (hasData || hasConnectedBefore); + // The announcement for the device this record names, matched on its service + // name when it has one and on hostname + port otherwise. An address is never + // matched on: DHCP hands leases around, and adopting the wrong device's + // resolution would point the socket at a stranger. + const resolvedMdnsDevice = useMemo(() => { + if (mdnsHostname === "") return null; + const discoveryId = activeDiscoveryId.toLowerCase(); + + return ( + discoveredDevices.find((device) => { + if ( + discoveryId && + device.deviceId?.trim().toLowerCase() === discoveryId + ) { + return true; + } + return ( + device.hostname !== undefined && + normalizeMdnsHostname(device.hostname) === mdnsHostname && + device.port === mdnsPort + ); + }) ?? null + ); + }, [activeDiscoveryId, discoveredDevices, mdnsHostname, mdnsPort]); + + useEffect(() => { + if (!resolvedMdnsDevice || !activeRecordId) return; + void deviceRegistry + .noteResolvedAddresses(activeRecordId, resolvedMdnsDevice.addresses) + .catch(ignoreReportedRegistryFailure); + }, [activeRecordId, resolvedMdnsDevice]); + + // Browsing costs battery and multicast traffic, so it runs only until this + // device's hostname has been resolved on a live connection. + // + // Gate on the boolean, not on `resolvedMdnsDevice` itself: a re-announcement + // produces a fresh object every time, and depending on the object would tear + // the watch down and rebuild it on each one. The module-level device cache in + // useNetworkScan outlives stopScan(), so the resolution survives the teardown + // and a later reconnect does not have to rediscover it. + const mdnsBrowseSettled = resolvedMdnsDevice !== null && isConnected; + useEffect(() => { + if (mdnsHostname === "" || mdnsBrowseSettled) return; + if (!Capacitor.isNativePlatform()) return; + + void startMdnsScan(); + return () => { + stopMdnsScan(); + }; + }, [mdnsBrowseSettled, mdnsHostname, startMdnsScan, stopMdnsScan]); + // Keep refs updated with latest callbacks (but don't trigger effect re-runs) useEffect(() => { tRef.current = t; diff --git a/src/components/NetworkScanModal.tsx b/src/components/NetworkScanModal.tsx index 64c72b6..ce7b023 100644 --- a/src/components/NetworkScanModal.tsx +++ b/src/components/NetworkScanModal.tsx @@ -26,7 +26,7 @@ function toRegistration( return { discoveryId: device.deviceId, hostname: device.hostname, - addresses: [device.address], + addresses: device.addresses, port: device.port, name: device.name, platform: device.platform, diff --git a/src/hooks/useNetworkScan.ts b/src/hooks/useNetworkScan.ts index ca7fffb..094b88d 100644 --- a/src/hooks/useNetworkScan.ts +++ b/src/hooks/useNetworkScan.ts @@ -1,6 +1,10 @@ import { useState, useCallback, useRef, useEffect } from "react"; import { Capacitor } from "@capacitor/core"; -import { ZeroConf, ZeroConfService } from "capacitor-zeroconf"; +import { + ZeroConf, + type ZeroConfService, + type ZeroConfWatchResult, +} from "capacitor-zeroconf"; import { logger } from "@/lib/logger"; const ZAPAROO_SERVICE_TYPE = "_zaparoo._tcp."; @@ -8,6 +12,16 @@ const ZAPAROO_SERVICE_DOMAIN = "local."; // Session cache for discovered devices (persists across hook instances until app restart) let deviceCache: DiscoveredDevice[] = []; +const deviceListeners = new Set<(devices: DiscoveredDevice[]) => void>(); + +// There is one ZeroConf watch for the whole app, shared by however many callers +// want discovery at once — the scan modal and the connection provider both do. +// Owners are counted rather than toggled so the modal closing cannot stop a +// browse the connection provider still needs. +const scanOwners = new Set(); +let watchPromise: Promise | null = null; +let unwatchPromise: Promise | null = null; +let isWatching = false; /** * Reset the device cache. Used for testing to prevent cache pollution between tests. @@ -15,13 +29,20 @@ let deviceCache: DiscoveredDevice[] = []; */ export function __resetDeviceCache(): void { deviceCache = []; + deviceListeners.clear(); + scanOwners.clear(); + watchPromise = null; + unwatchPromise = null; + isWatching = false; } export interface DiscoveredDevice { /** Instance name (usually hostname) */ name: string; - /** IP address to use when no service hostname is available */ + /** Preferred resolved IP address */ address: string; + /** Every resolved IP address the service advertised */ + addresses: string[]; /** mDNS hostname shared across the device's network interfaces */ hostname?: string; /** Port number */ @@ -115,8 +136,21 @@ function parseTxtRecord(txtRecord: Record | undefined): { * Convert a ZeroConfService to our DiscoveredDevice format. * Returns null if the service doesn't have a valid IP address. */ +/** + * IPv4 before IPv6, deliberately: callers take `[0]` as the address to dial, + * and IPv4 is the one that works on every network the app is likely to meet. + */ +function serviceAddresses(service: ZeroConfService): string[] { + return [ + ...new Set([ + ...(service.ipv4Addresses ?? []), + ...(service.ipv6Addresses ?? []), + ]), + ]; +} + function serviceToIdentity(service: ZeroConfService): DeviceIdentity { - const address = service.ipv4Addresses?.[0] || service.ipv6Addresses?.[0]; + const address = serviceAddresses(service)[0]; const hostname = normalizeHostname(service.hostname); const deviceId = normalizeIdentityValue(service.txtRecord?.["id"]); @@ -138,12 +172,156 @@ function serviceToDevice(service: ZeroConfService): DiscoveredDevice | null { return { name: service.name, address: identity.address, + addresses: serviceAddresses(service), ...(identity.hostname ? { hostname: identity.hostname } : {}), port: service.port, ...txtData, }; } +/** + * Fold a re-announcement into the cached device. + * + * If the address already in use is still advertised, this is an additive + * announcement from a multi-homed device: union the sets and do not move, so a + * partial announcement cannot flip `address` and tear down a live connection. + * If it is no longer advertised the device genuinely moved, so take the new set + * wholesale rather than keeping addresses that no longer answer. + */ +function mergeDiscoveredDevice( + existing: DiscoveredDevice, + incoming: DiscoveredDevice, +): DiscoveredDevice { + const stillAdvertised = incoming.addresses.includes(existing.address); + const addresses = stillAdvertised + ? [...new Set([...existing.addresses, ...incoming.addresses])] + : incoming.addresses; + + return { + ...existing, + ...incoming, + addresses, + address: stillAdvertised ? existing.address : incoming.address, + }; +} + +function publishDevices(devices: DiscoveredDevice[]): void { + deviceCache = devices; + deviceListeners.forEach((listener) => listener(devices)); +} + +function handleDiscoveryResult(result: ZeroConfWatchResult): void { + if (result.action === "resolved") { + const device = serviceToDevice(result.service); + if (!device) return; + + const existingIndex = deviceCache.findIndex((existing) => + isSameDiscoveredDevice(existing, device), + ); + const existing = deviceCache[existingIndex]; + const updated = [...deviceCache]; + if (existing === undefined) { + updated.push(device); + } else { + updated[existingIndex] = mergeDiscoveredDevice(existing, device); + } + publishDevices(updated); + return; + } + + if (result.action === "removed") { + const removedIdentity = serviceToIdentity(result.service); + if ( + !removedIdentity.deviceId && + !removedIdentity.hostname && + !removedIdentity.address + ) { + return; + } + + const updated = deviceCache.filter( + (device) => !isSameDiscoveredDevice(device, removedIdentity), + ); + if (updated.length !== deviceCache.length) { + publishDevices(updated); + } + } +} + +/** + * Register `owner` as needing discovery, starting the shared watch if it is not + * already running. + * + * A teardown in flight is awaited first: `unwatch` and `watch` against the same + * service type race inside the plugin, and losing that race leaves the watch + * believing it is running with no listener attached. + */ +async function acquireNetworkScan(owner: symbol): Promise { + scanOwners.add(owner); + + if (unwatchPromise) { + await unwatchPromise; + } + // Released while we waited, or someone else already started the watch. + if (!scanOwners.has(owner) || isWatching) return; + + if (!watchPromise) { + watchPromise = ZeroConf.watch( + { + type: ZAPAROO_SERVICE_TYPE, + domain: ZAPAROO_SERVICE_DOMAIN, + }, + handleDiscoveryResult, + ) + .then(() => { + isWatching = true; + }) + .finally(() => { + watchPromise = null; + }); + } + + await watchPromise; +} + +/** Drop `owner`'s claim, stopping the shared watch once nobody holds one. */ +function releaseNetworkScan(owner: symbol): void { + scanOwners.delete(owner); + if (scanOwners.size > 0 || unwatchPromise) return; + + const stopPromise = (async () => { + if (watchPromise) { + try { + await watchPromise; + } catch { + // The watch never started, so there is nothing to unwatch. + return; + } + } + // A new owner may have arrived while the watch was still starting. + if (scanOwners.size > 0 || !isWatching) return; + + isWatching = false; + try { + // unwatch() rather than close(): it only removes the service listener, + // where close() tears down JmDNS and makes the next scan much slower. + await ZeroConf.unwatch({ + type: ZAPAROO_SERVICE_TYPE, + domain: ZAPAROO_SERVICE_DOMAIN, + }); + } catch (e) { + logger.debug("Error stopping zeroconf watch", e); + } + })(); + + unwatchPromise = stopPromise; + void stopPromise.finally(() => { + if (unwatchPromise === stopPromise) { + unwatchPromise = null; + } + }); +} + /** * Hook for scanning the local network for Zaparoo Core devices using mDNS. * Only works on native platforms (iOS/Android). @@ -166,31 +344,12 @@ export function useNetworkScan(): UseNetworkScanResult { const [error, setError] = useState(null); const isScanningRef = useRef(false); - - useEffect(() => { - deviceCache = devices; - }, [devices]); + const scanOwnerRef = useRef(Symbol("network-scan")); const stopScan = useCallback(() => { - // Update UI state immediately - const wasScanning = isScanningRef.current; isScanningRef.current = false; setIsScanning(false); - - // Stop watching if we were scanning - if (wasScanning) { - // Use unwatch() instead of close() - it's much faster because it only - // removes the service listener without tearing down JmDNS. - // Note: Due to a bug in capacitor-zeroconf, the BrowserManager is reused - // and subsequent watch() calls won't re-discover already-cached devices. - // This is acceptable since we maintain a session cache. - ZeroConf.unwatch({ - type: ZAPAROO_SERVICE_TYPE, - domain: ZAPAROO_SERVICE_DOMAIN, - }).catch((e) => { - logger.debug("Error stopping zeroconf watch", e); - }); - } + releaseNetworkScan(scanOwnerRef.current); }, []); const startScan = useCallback(async () => { @@ -200,63 +359,23 @@ export function useNetworkScan(): UseNetworkScanResult { return; } - // Stop any existing scan - stopScan(); - - // Keep cached devices (due to plugin bug, already-discovered devices - // won't be re-announced, so we rely on the cache) + // Keep cached devices (due to a plugin bug, already-discovered devices + // aren't re-announced to a second watch, so we rely on the cache) setDevices(deviceCache); setError(null); setIsScanning(true); isScanningRef.current = true; try { - // Start watching for Zaparoo services with callback - // Note: This plugin uses the callback parameter, not addListener events - await ZeroConf.watch( - { - type: ZAPAROO_SERVICE_TYPE, - domain: ZAPAROO_SERVICE_DOMAIN, - }, - (result) => { - if (result.action === "resolved") { - const device = serviceToDevice(result.service); - if (device) { - setDevices((prev) => { - const existingIndex = prev.findIndex((existing) => - isSameDiscoveredDevice(existing, device), - ); - const updated = [...prev]; - if (existingIndex === -1) { - updated.push(device); - } else { - updated[existingIndex] = { - ...updated[existingIndex], - ...device, - }; - } - return updated; - }); - } - } else if (result.action === "removed") { - const removedIdentity = serviceToIdentity(result.service); - if ( - removedIdentity.deviceId || - removedIdentity.hostname || - removedIdentity.address - ) { - setDevices((prev) => { - return prev.filter( - (device) => !isSameDiscoveredDevice(device, removedIdentity), - ); - }); - } - } - }, - ); - - // Scan continuously until stopScan() is called (no auto-timeout) + // Scans continuously until stopScan() is called (no auto-timeout) + await acquireNetworkScan(scanOwnerRef.current); } catch (e) { + releaseNetworkScan(scanOwnerRef.current); + // stopScan() already ran, so this failure belongs to a scan the caller + // has abandoned — reporting it would surface an error for a scan the + // user is no longer waiting on. + if (!isScanningRef.current) return; + logger.error("Failed to start network scan", e, { category: "connection", action: "networkScan", @@ -266,11 +385,16 @@ export function useNetworkScan(): UseNetworkScanResult { setIsScanning(false); isScanningRef.current = false; } - }, [stopScan]); + }, []); - // Cleanup on unmount useEffect(() => { + const handleDevicesChanged = (updated: DiscoveredDevice[]) => { + setDevices(updated); + }; + deviceListeners.add(handleDevicesChanged); + return () => { + deviceListeners.delete(handleDevicesChanged); stopScan(); }; }, [stopScan]); diff --git a/src/lib/devices/deviceRegistry.ts b/src/lib/devices/deviceRegistry.ts index 554feab..49e27ab 100644 --- a/src/lib/devices/deviceRegistry.ts +++ b/src/lib/devices/deviceRegistry.ts @@ -9,6 +9,7 @@ import { } from "@/lib/crypto/credentials"; import { parseDeviceEndpoint, + replaceDeviceEndpointHost, type DeviceEndpointScheme, type ParsedDeviceEndpoint, } from "@/lib/devices/endpoint"; @@ -28,6 +29,15 @@ export interface DeviceEndpoint { port: number; source: DeviceEndpointSource; lastSeenAt?: number; + /** + * Addresses the last mDNS advertisement for this endpoint resolved to. + * + * The endpoint keeps its `.local` hostname as the canonical identity — that + * is what survives the device changing IP — but iOS WebSockets do not + * reliably resolve `.local` themselves, so the socket dials one of these + * instead when they are known. + */ + resolvedAddresses?: string[]; } export interface DeviceRecord { @@ -104,10 +114,29 @@ function normalizeDiscoveryId(value: string | undefined): string | undefined { return normalized || undefined; } +function unique(values: readonly string[]): string[] { + return [...new Set(values)]; +} + +/** + * Set equality, not sequence equality. Both sides are deduped, so matching + * lengths plus containment is enough. An mDNS re-announcement is free to list + * the same addresses in a different order, and treating that as a change would + * rewrite the record and republish the registry for nothing. + */ +function sameAddressSet( + left: readonly string[], + right: readonly string[], +): boolean { + if (left.length !== right.length) return false; + const existing = new Set(left); + return right.every((address) => existing.has(address)); +} + function endpointFromParsed( endpoint: ParsedDeviceEndpoint, source: DeviceEndpointSource, - lastSeenAt?: number, + details: Pick = {}, ): DeviceEndpoint { return { endpointId: endpoint.endpointId, @@ -115,13 +144,22 @@ function endpointFromParsed( host: endpoint.host, port: endpoint.port, source, - ...(lastSeenAt !== undefined ? { lastSeenAt } : {}), + ...(details.lastSeenAt !== undefined + ? { lastSeenAt: details.lastSeenAt } + : {}), + ...(details.resolvedAddresses + ? { resolvedAddresses: unique(details.resolvedAddresses) } + : {}), }; } /** * The endpoint a record connects through, falling back to the first one when * `preferredEndpointId` no longer names an endpoint the record holds. + * + * Both public accessors go through here so they can never disagree about which + * endpoint is preferred — resolving it twice once dropped `resolvedAddresses` + * in exactly the case the fallback exists for. */ function preferredEndpointFor( record: DeviceRecord | null | undefined, @@ -145,6 +183,26 @@ export function parsedEndpointForRecord( return result.ok ? result.endpoint : null; } +/** + * The endpoint to actually dial: the preferred one, with its host swapped for + * a resolved address when mDNS supplied one. + * + * The record keeps the `.local` hostname — that is the identity that survives + * a DHCP move — while the socket gets an address iOS can reach without doing + * its own multicast resolution. + */ +export function resolvedEndpointForRecord( + record: DeviceRecord | null | undefined, +): ParsedDeviceEndpoint | null { + const preferred = preferredEndpointFor(record); + const parsed = parsedEndpointForRecord(record); + if (!parsed || !preferred) return parsed; + const resolvedAddress = preferred.resolvedAddresses?.[0]; + return resolvedAddress + ? replaceDeviceEndpointHost(parsed, resolvedAddress) + : parsed; +} + /** The active record's display address, or `""` before hydration. */ export function activeAddressOf(snapshot: DeviceRegistrySnapshot): string { const record = snapshot.activeRecordId @@ -170,7 +228,12 @@ function validEndpoint(value: unknown): value is DeviceEndpoint { parsed.ok && parsed.endpoint.scheme === endpoint.scheme && parsed.endpoint.host === endpoint.host && - parsed.endpoint.port === endpoint.port + parsed.endpoint.port === endpoint.port && + (endpoint.resolvedAddresses === undefined || + (Array.isArray(endpoint.resolvedAddresses) && + endpoint.resolvedAddresses.every( + (address) => typeof address === "string", + ))) ); } @@ -635,6 +698,11 @@ class DeviceRegistryRepository { ), ); + const resolvedAddresses = unique(device.addresses); + const existingEndpoint = existing?.endpoints.find( + (endpoint) => endpoint.endpointId === parsed.endpoint.endpointId, + ); + // Compare against what the commit below would actually write: a custom name // is never overwritten and empty metadata is never applied, so testing // fields the commit refuses to change would make this permanently false and @@ -643,6 +711,11 @@ class DeviceRegistryRepository { existing !== undefined && existing.discoveryId === discoveryId && existing.preferredEndpointId === parsed.endpoint.endpointId && + existingEndpoint?.source === "mdns" && + sameAddressSet( + existingEndpoint.resolvedAddresses ?? [], + resolvedAddresses, + ) && (!device.name || existing.nameIsCustom === true || existing.name === device.name) && @@ -655,11 +728,10 @@ class DeviceRegistryRepository { return existing; } - const discoveredEndpoint = endpointFromParsed( - parsed.endpoint, - "mdns", - Date.now(), - ); + const discoveredEndpoint = endpointFromParsed(parsed.endpoint, "mdns", { + lastSeenAt: Date.now(), + resolvedAddresses, + }); const base = existing ?? // Before the registry, picking a device from a scan saved it by IP even @@ -792,6 +864,49 @@ class DeviceRegistryRepository { })); } + /** + * Record the addresses an mDNS advertisement resolved this record's preferred + * endpoint to, so the socket can dial one instead of a `.local` hostname. + * + * Deliberately narrower than `selectDiscovered`: this only annotates the + * record the caller names, where `selectDiscovered` matches an announcement + * against the whole registry and may create a record or switch the active + * one. A background browse running behind a live connection must never do + * either of those things. + */ + async noteResolvedAddresses( + recordId: string, + addresses: readonly string[], + ): Promise { + await this.hydrate(); + const record = this.snapshot.records[recordId]; + const preferred = preferredEndpointFor(record); + if (!record || !preferred) return; + + const resolved = unique(addresses.filter((address) => address.length > 0)); + if ( + resolved.length === 0 || + sameAddressSet(preferred.resolvedAddresses ?? [], resolved) + ) { + // mDNS re-announces constantly; only a genuine change is worth a write. + return; + } + + const next: DeviceRecord = { + ...record, + endpoints: record.endpoints.map((endpoint) => + endpoint.endpointId === preferred.endpointId + ? { ...endpoint, resolvedAddresses: resolved } + : endpoint, + ), + }; + + await this.commit((registry) => ({ + ...registry, + records: { ...registry.records, [recordId]: next }, + })); + } + /** Set or clear the user's own name for a record. Blank clears it. */ async setCustomName(recordId: string, name: string): Promise { await this.hydrate(); diff --git a/src/lib/devices/endpoint.ts b/src/lib/devices/endpoint.ts index 1945a3c..be5e3be 100644 --- a/src/lib/devices/endpoint.ts +++ b/src/lib/devices/endpoint.ts @@ -117,6 +117,20 @@ export function formatDeviceEndpoint( * authority is inspected, and a bracketed literal is stepped over first, so * `http://[::80]` is read as a host rather than as port 80. */ +/** + * The same endpoint reached through a different host — an mDNS `.local` name + * swapped for the IP the advertisement resolved to. An unusable host is + * ignored rather than thrown so a bad advertisement degrades to the hostname + * the record already had. + */ +export function replaceDeviceEndpointHost( + endpoint: ParsedDeviceEndpoint, + host: string, +): ParsedDeviceEndpoint { + if (!isValidHost(host.toLowerCase())) return endpoint; + return formatDeviceEndpoint(host, endpoint.port, endpoint.scheme); +} + function explicitAuthorityPort(input: string): string | undefined { const authority = input.slice(input.indexOf("://") + 3).split(/[/?#]/)[0]; if (authority === undefined) return undefined;