From 59487e32d23dd94352fb471dfa2fe2b28ec3e1e9 Mon Sep 17 00:00:00 2001 From: Bryan Roscoe Date: Thu, 1 Oct 2026 11:57:00 -0500 Subject: [PATCH 1/3] Mobile: keep id-less saved TVs distinct and honest about identity (#146) Id-less saved rows (no hardware id) were keyed everywhere by their bare host:port, so two genuinely different TVs that had shared an endpoint at different times collapsed into one storage slot -- the older one silently discarded -- and buildDiscoveryRows could mark more than one of them "connected" at a shared live endpoint. Give every id-less row a stable random localId the first time it is persisted; key it on that instead of the endpoint (identity.ts, savedDevices.ts). rememberDevice only refreshes an existing id-less row when exactly one saved row matches the connecting endpoint -- with two or more, which one reconnected is unknowable, so a new row is saved rather than guessing. discoveryRows.ts never attributes a shared, id-less live endpoint to a specific saved row when more than one claims it: the discovery row falls back to a generic name and the saved rows report "saved-address" instead of claiming the connection. Existing stored rows migrate to the new key on first read without losing any. Approach (a) from #146, chosen over honesty-messaging-only merging because it keeps the two TVs' names and reconnect targets instead of discarding one. --- v2/mobile/BACKLOG.md | 27 ++++++++++++-- v2/mobile/HANDOFF.md | 14 ++++++++ v2/mobile/src/lib/discoveryRows.ts | 38 +++++++++++++++----- v2/mobile/src/lib/identity.ts | 9 +++-- v2/mobile/src/lib/savedDevices.ts | 49 ++++++++++++++++++++++---- v2/mobile/src/lib/types.ts | 5 +++ v2/mobile/tests/discoveryRows.test.mjs | 45 +++++++++++++++++++++++ v2/mobile/tests/savedDevices.test.mjs | 47 +++++++++++++++++++++--- 8 files changed, 208 insertions(+), 26 deletions(-) diff --git a/v2/mobile/BACKLOG.md b/v2/mobile/BACKLOG.md index e477c03..8077295 100644 --- a/v2/mobile/BACKLOG.md +++ b/v2/mobile/BACKLOG.md @@ -75,9 +75,10 @@ Fixed GitHub #115–#118, all found by a code-read audit. Each repro is now a te **none has run on a phone or TV.** - One hardware-id rule (`lib/identity.ts`, matching desktop `idKey`): trim, and empty or any - casing of `unknown` is no id. Stored placeholder ids migrate to id-less rows, and rows that then - share one identity collapse to the newest, so saved-TV list keys are unique and Forget removes - exactly one TV (#116). A placeholder is no longer a wildcard for `adb-unknown-*` adverts. + casing of `unknown` is no id. Stored placeholder ids migrate to id-less rows, each keyed by its + own stable random local id rather than its bare endpoint (see #146 below), so saved-TV list keys + are unique and Forget removes exactly one TV (#116). A placeholder is no longer a wildcard for + `adb-unknown-*` adverts. - `cachedDeviceName` matches on verified id, or an id-less row at the exact endpoint; an identified row never names a TV that reports no id. Advertised serials match only exactly or with adbd's six-character suffix, so `shield` no longer verifies `adb-shield-a` (#117). @@ -85,6 +86,26 @@ Fixed GitHub #115–#118, all found by a code-read audit. Each repro is now a te - `loadHealth` cannot wedge after its device vanishes mid-load, Cancel no longer shows a reconnect failure, and Diagnostics keys its safety lookup on the sorted package set (#118). +## Id-less saved TVs stay distinct (2026-10-01) + +Fixed GitHub #146, raised by Codex on PR #144 and declined there as an edge case at the time. +Two id-less saved TVs (no hardware id) that had shared one `host:port` used to collapse into a +single storage row on migration/read -- the older one silently discarded -- because every id-less +row was keyed by its bare endpoint. Fixed in `savedDevices.ts`/`identity.ts`/`discoveryRows.ts`: + +- Every id-less row gets a stable random `localId` the first time it is persisted; `savedDeviceKey` + keys on that instead of the endpoint, so two id-less TVs stay two rows no matter what address + they shared. Existing stored rows are migrated on first read without losing any. +- `rememberDevice` only refreshes an existing id-less row when exactly one saved row matches the + connecting endpoint. When two or more already share it, which one just reconnected is unknowable, + so a new row is saved rather than silently overwriting one of the existing guesses. +- `buildDiscoveryRows` never marks more than one id-less saved row "connected" at a shared live + endpoint: the discovery row falls back to a generic name and both saved rows report + "saved-address" instead of one of them claiming the connection. + +Covered by new tests in `tests/savedDevices.test.mjs` and `tests/discoveryRows.test.mjs`; none has +run on a device. + ## Stability reset (2026-09-04) A four-track audit (feature parity vs v1/desktop, connection lifecycle, screen UX, Rust backend) diff --git a/v2/mobile/HANDOFF.md b/v2/mobile/HANDOFF.md index 6c4f3d3..e5fd76b 100644 --- a/v2/mobile/HANDOFF.md +++ b/v2/mobile/HANDOFF.md @@ -37,6 +37,20 @@ exactly or with adbd's 6-character suffix. `loadHealth` can no longer wedge, Can reports a reconnect failure, and Diagnostics no longer blanks its safety badges when two apps swap rank. Every fix is covered by a test reproducing the issue; none has run on a device. +**Id-less saved rows stay distinct (2026-10-01, GitHub #146).** An id-less row (no hardware id) +used to be keyed everywhere by its bare `host:port`, so two genuinely different TVs that had +shared an endpoint at different times collapsed into one storage slot — the older one silently +lost. Every id-less row now gets its own random `localId` the first time it is persisted +(`savedDevices.ts`), and `savedDeviceKey`/`identity.ts` key id-less rows on that instead of the +endpoint; existing stored rows are migrated in place on first read, losing nothing. +`rememberDevice` only refreshes an existing id-less row when exactly one saved row matches the +connecting endpoint — when two or more already share it, which one just reconnected is not +knowable, so a new row is saved rather than overwriting a guess. `discoveryRows.ts` never marks +more than one id-less saved row "connected" at a shared live endpoint either; an ambiguous +endpoint's discovery row shows a generic name and both saved rows report "saved-address" instead. +Covered by new tests in `savedDevices.test.mjs` and `discoveryRows.test.mjs`; unverified on a +device. + --- ## 1. What this is diff --git a/v2/mobile/src/lib/discoveryRows.ts b/v2/mobile/src/lib/discoveryRows.ts index 0f57c3b..f459403 100644 --- a/v2/mobile/src/lib/discoveryRows.ts +++ b/v2/mobile/src/lib/discoveryRows.ts @@ -117,6 +117,18 @@ export function buildDiscoveryRows( const liveSaved = liveId ? savedDevices.find((saved) => normalizeHardwareId(saved.hardwareId) === liveId) : undefined; + // How many id-less saved rows claim a given endpoint. More than one means + // an id-less live connection there cannot be attributed to either of + // them -- the bare address is all any id-less row has, and both have it + // equally, so neither may be shown as the one that is connected. + const idlessCountAt = new Map(); + for (const saved of savedDevices) { + if (normalizeHardwareId(saved.hardwareId) !== undefined) continue; + const endpoint = `${saved.host}:${saved.connectPort}`; + idlessCountAt.set(endpoint, (idlessCountAt.get(endpoint) ?? 0) + 1); + } + const idlessAmbiguousAt = (host: string, port: number): boolean => + (idlessCountAt.get(`${host}:${port}`) ?? 0) > 1; const hosts = new Map(); for (const discovery of discoveries) { const host = discovery.host.trim(); @@ -173,15 +185,20 @@ export function buildDiscoveryRows( ); // On the live row only the live TV's own identity counts. When it reports // no id, an advert there may be stale, so the row is named only after an - // id-less saved TV at this exact endpoint, or after nothing. + // id-less saved TV at this exact endpoint -- and only when that endpoint + // has exactly one such TV saved, never a guess between several. if (isLive) { - savedMatch = liveId - ? liveSaved - : savedDevices.find( - (saved) => - normalizeHardwareId(saved.hardwareId) === undefined && - savedDeviceMatchesConnection(saved, live.host, live.connectPort), - ); + if (liveId) { + savedMatch = liveSaved; + } else if (!idlessAmbiguousAt(live.host, live.connectPort)) { + savedMatch = savedDevices.find( + (saved) => + normalizeHardwareId(saved.hardwareId) === undefined && + savedDeviceMatchesConnection(saved, live.host, live.connectPort), + ); + } else { + savedMatch = undefined; + } } if (savedMatch) verified.add(savedMatch); rows.push({ @@ -248,7 +265,10 @@ export function buildDiscoveryRows( pairingPorts: [], legacyConnectPorts: [], status: atLiveEndpoint - ? savedDeviceMatchesConnection(saved, live.host, live.connectPort, live.hardwareId) + ? savedDeviceMatchesConnection(saved, live.host, live.connectPort, live.hardwareId) && + // An id-less match at an endpoint shared by another id-less saved + // TV is not evidence this particular row is the live one. + !(normalizeHardwareId(saved.hardwareId) === undefined && idlessAmbiguousAt(saved.host, saved.connectPort)) ? "connected" // Something answers at this saved address, but nothing shows it is // this TV, so the row claims the address and not the connection. diff --git a/v2/mobile/src/lib/identity.ts b/v2/mobile/src/lib/identity.ts index 82fa427..46b3cb4 100644 --- a/v2/mobile/src/lib/identity.ts +++ b/v2/mobile/src/lib/identity.ts @@ -33,7 +33,12 @@ export function savedDeviceMatchesConnection( export function savedDeviceKey(device: SavedDevice): string { const hardwareId = normalizeHardwareId(device.hardwareId); - return hardwareId - ? `hardware:${hardwareId}` + if (hardwareId) return `hardware:${hardwareId}`; + // An id-less row is keyed by its own stable local id once it has one + // (assigned by savedDevices.ts the first time it is persisted), so two + // id-less TVs that share an endpoint stay distinct rows. The bare endpoint + // is only a fallback for a row that has not gone through that migration. + return device.localId + ? `local:${device.localId}` : `idless:${device.host}:${device.connectPort}`; } diff --git a/v2/mobile/src/lib/savedDevices.ts b/v2/mobile/src/lib/savedDevices.ts index 85430c5..d890d6a 100644 --- a/v2/mobile/src/lib/savedDevices.ts +++ b/v2/mobile/src/lib/savedDevices.ts @@ -22,6 +22,12 @@ const AUTO_KEY = "atv.autoConnect.v1"; const MAX = 16; const EPOCH = new Date(0).toISOString(); +/// Not a security identifier -- just random enough that two rows saved in the +/// same process tick never collide. +function randomLocalId(): string { + return `${Date.now().toString(36)}${Math.random().toString(36).slice(2, 10)}`; +} + function normalizeSavedDevice(value: unknown): SavedDevice | null { if (!value || typeof value !== "object") return null; const d = value as Record; @@ -48,10 +54,20 @@ function normalizeSavedDevice(value: unknown): SavedDevice | null { const hardwareId = normalizeHardwareId( typeof d.hardwareId === "string" ? d.hardwareId : undefined, ); + // Every id-less row needs a stable key of its own (see identity.ts); carry + // an existing one over, or mint one now if this row has never had one (an + // older record, or one whose only id was a placeholder that just migrated + // away above). + const localId = hardwareId + ? undefined + : typeof d.localId === "string" && d.localId.trim() !== "" + ? d.localId.trim() + : randomLocalId(); return { host: d.host.trim(), connectPort: d.connectPort, ...(hardwareId ? { hardwareId } : {}), + ...(localId ? { localId } : {}), name: typeof d.name === "string" && d.name.trim() !== "" ? d.name.trim() @@ -63,6 +79,19 @@ function normalizeSavedDevice(value: unknown): SavedDevice | null { }; } +/// Whether a raw stored record will come out of `normalizeSavedDevice` as +/// id-less and without an existing local key -- i.e. it is about to be +/// assigned a fresh one, which must be persisted so it stays stable. +function rawNeedsLocalId(raw: unknown): boolean { + if (!raw || typeof raw !== "object") return false; + const d = raw as Record; + const hardwareId = normalizeHardwareId( + typeof d.hardwareId === "string" ? d.hardwareId : undefined, + ); + if (hardwareId) return false; + return typeof d.localId !== "string" || d.localId.trim() === ""; +} + function read(): SavedDevice[] { try { const raw = localStorage.getItem(KEY); @@ -90,7 +119,10 @@ function read(): SavedDevice[] { const stored = d.hardwareId; return normalizeHardwareId(typeof stored === "string" ? stored : undefined) !== stored; }); - if (unnormalizedId || unique.length !== normalized.length) write(unique); + const needsLocalIdMigration = parsed.some(rawNeedsLocalId); + if (unnormalizedId || needsLocalIdMigration || unique.length !== normalized.length) { + write(unique); + } return unique; } catch { return []; @@ -165,9 +197,13 @@ export function rememberDevice( ): void { const current = read(); const hardwareId = hardwareIdOf(device); - const existing = current.find((d) => - sameTv(d, host, connectPort, hardwareId), - ); + const matches = current.filter((d) => sameTv(d, host, connectPort, hardwareId)); + // An id-less connection can match more than one saved row only when several + // id-less TVs have shared this exact endpoint over time. Which one just + // answered is not knowable from the endpoint alone, so none of them is + // claimed -- a new row is saved rather than overwriting a guess. + const existing = matches.length === 1 ? matches[0] : undefined; + const combinedHardwareId = hardwareId ?? existing?.hardwareId; const reportedFriendlyName = device?.properties?.friendly_name?.trim(); const name = reportedFriendlyName || existing?.name || deviceLabelOf(device); const list = current.filter((d) => d !== existing); @@ -176,9 +212,8 @@ export function rememberDevice( connectPort, name, deviceType: device?.device_type ?? existing?.deviceType ?? "unknown", - ...(hardwareId || existing?.hardwareId - ? { hardwareId: hardwareId ?? existing?.hardwareId } - : {}), + ...(combinedHardwareId ? { hardwareId: combinedHardwareId } : {}), + ...(combinedHardwareId ? {} : { localId: existing?.localId ?? randomLocalId() }), lastUsed: new Date().toISOString(), }); write(list); diff --git a/v2/mobile/src/lib/types.ts b/v2/mobile/src/lib/types.ts index edf3407..50e1128 100644 --- a/v2/mobile/src/lib/types.ts +++ b/v2/mobile/src/lib/types.ts @@ -391,6 +391,11 @@ export interface SavedDevice { /// Hardware serial when the TV reported one. Lets a TV keep its row and /// name across an IP change, and stops a reused IP from inheriting a name. hardwareId?: string; + /// Stable random id assigned the first time a row with no hardware id is + /// saved. Two TVs that never reported a serial and happened to share a + /// host:port are still distinct rows -- the address alone cannot tell them + /// apart, so each gets its own key instead of collapsing into one. + localId?: string; /// ISO timestamp of the last successful connect, for "last used" copy. lastUsed: string; } diff --git a/v2/mobile/tests/discoveryRows.test.mjs b/v2/mobile/tests/discoveryRows.test.mjs index 91f6c75..8daba19 100644 --- a/v2/mobile/tests/discoveryRows.test.mjs +++ b/v2/mobile/tests/discoveryRows.test.mjs @@ -156,6 +156,51 @@ test("only the saved TV whose id is live claims a shared live endpoint (#115)", assert.deepEqual(idless.map((row) => [row.name, row.status]), [["No id", "connected"]]); }); +test("two id-less saved TVs sharing the live endpoint never both claim connected (#146)", () => { + const first = saved({ + hardwareId: undefined, + localId: "guest-a", + name: "Guest A", + lastUsed: "2026-09-01T00:00:00.000Z", + }); + const second = saved({ + hardwareId: undefined, + localId: "guest-b", + name: "Guest B", + lastUsed: "2026-09-02T00:00:00.000Z", + }); + + // With an advert, the live address still gets a generic connected row, but + // neither saved TV's name is attached to it -- the address cannot say which + // of them answered. + const withAdvert = buildDiscoveryRows( + [advert("192.168.1.10", 5555, LEGACY, "Android TV")], + [first, second], + liveAt("192.168.1.10", 5555), + ); + assert.deepEqual( + withAdvert.map((row) => [row.key, row.name, row.status]), + [ + ["discovery:192.168.1.10", "Android TV", "connected"], + ["saved:local:guest-b", "Guest B", "saved-address"], + ["saved:local:guest-a", "Guest A", "saved-address"], + ], + ); + assert.equal(withAdvert[0].savedTarget, undefined); + + // With no advert at all, the live endpoint has no discovery row to stand + // in as an honest "connected, but unidentified" placeholder, so neither + // saved row may claim it -- both report the address as previously used. + const silent = buildDiscoveryRows([], [first, second], liveAt("192.168.1.10", 5555)); + assert.deepEqual( + silent.map((row) => [row.key, row.status]), + [ + ["saved:local:guest-b", "saved-address"], + ["saved:local:guest-a", "saved-address"], + ], + ); +}); + test("a stale advert never names the live row after a different saved TV (#115)", () => { const shieldA = saved({ hardwareId: "shield-a", name: "Shield A" }); const googleB = saved({ hardwareId: "google-b", name: "Google B", lastUsed: "2026-08-01T00:00:00.000Z" }); diff --git a/v2/mobile/tests/savedDevices.test.mjs b/v2/mobile/tests/savedDevices.test.mjs index 8540efe..b46ceee 100644 --- a/v2/mobile/tests/savedDevices.test.mjs +++ b/v2/mobile/tests/savedDevices.test.mjs @@ -340,9 +340,11 @@ test("a reused address never lends an identified TV's name to a TV that reports assert.equal(savedDevices.cachedDeviceName("192.168.1.10", 41234), null); }); -test("a stored placeholder id becomes an id-less row with one key (#116)", () => { +test("a stored placeholder id becomes an id-less row with its own stable key, never merged with another (#116, #146)", () => { seed([ saved({ hardwareId: undefined, name: "Real", lastUsed: "2026-09-02T00:00:00.000Z" }), + // Shares "Real"'s exact host:port once its placeholder id normalizes + // away -- a genuinely different TV, not a duplicate of "Real". saved({ hardwareId: "unknown", name: "Ghost", lastUsed: "2026-09-01T00:00:00.000Z" }), saved({ host: "192.168.1.20", hardwareId: " UNKNOWN ", name: "Other", lastUsed: "2026-08-01T00:00:00.000Z" }), ]); @@ -350,15 +352,24 @@ test("a stored placeholder id becomes an id-less row with one key (#116)", () => const rows = savedDevices.listSavedDevices(); const keys = rows.map(savedDevices.savedDeviceKey); assert.equal(new Set(keys).size, keys.length); + // All three survive -- the placeholder migration must not discard a TV + // just because it now shares an address with another id-less row. assert.deepEqual( - rows.map((row) => [row.name, row.hardwareId]), - [["Real", undefined], ["Other", undefined]], + new Set(rows.map((row) => `${row.name}:${row.hardwareId}`)), + new Set(["Real:undefined", "Ghost:undefined", "Other:undefined"]), ); // The placeholder is migrated out of storage, not just hidden on read. assert.equal(rawRows().some((row) => "hardwareId" in row), false); + // Each row's freshly-minted local key is itself persisted, so it stays + // stable across reads instead of being re-rolled every time. + assert.deepEqual(savedDevices.listSavedDevices().map(savedDevices.savedDeviceKey), keys); - savedDevices.forgetSavedDevice(rows[0]); - assert.deepEqual(savedDevices.listSavedDevices().map((row) => row.name), ["Other"]); + const ghost = rows.find((row) => row.name === "Ghost"); + savedDevices.forgetSavedDevice(ghost); + assert.deepEqual( + new Set(savedDevices.listSavedDevices().map((row) => row.name)), + new Set(["Real", "Other"]), + ); }); test("reconnecting a TV that reports a placeholder id refreshes its row instead of appending (#116)", () => { @@ -373,6 +384,32 @@ test("reconnecting a TV that reports a placeholder id refreshes its row instead assert.equal(rows[0].name, "Ghost"); }); +test("never guesses which of two id-less TVs at one endpoint just reconnected (#146)", () => { + seed([ + saved({ hardwareId: undefined, localId: "first-tv", name: "First TV", lastUsed: "2026-09-01T00:00:00.000Z" }), + saved({ hardwareId: undefined, localId: "second-tv", name: "Second TV", lastUsed: "2026-09-02T00:00:00.000Z" }), + ]); + + // An id-less connection lands on the one shared endpoint of two already + // distinct saved TVs. Which one it is cannot be told from the address + // alone, so neither existing row is claimed (and so overwritten with a + // possibly-wrong identity) -- a third, honestly unidentified row is saved. + savedDevices.rememberDevice("192.168.1.10", 5555, device(undefined, { + name: "Just reported", + properties: { friendly_name: null, serial_number: undefined }, + })); + + const rows = savedDevices.listSavedDevices(); + assert.equal(rows.length, 3); + assert.deepEqual( + new Set(rows.map((row) => row.name)), + new Set(["First TV", "Second TV", "Just reported"]), + ); + // Both original rows are untouched -- neither lost its name nor its key. + assert.equal(rows.some((row) => row.name === "First TV" && savedDevices.savedDeviceKey(row) === "local:first-tv"), true); + assert.equal(rows.some((row) => row.name === "Second TV" && savedDevices.savedDeviceKey(row) === "local:second-tv"), true); +}); + test("sorts by lastUsed and truncates the oldest saved rows", () => { seed(Array.from({ length: 18 }, (_, index) => saved({ host: `192.168.1.${index + 1}`, From d129c7b0f9f885cd72156c87c7f692d8ea4eda9b Mon Sep 17 00:00:00 2001 From: Bryan Roscoe Date: Thu, 1 Oct 2026 12:07:18 -0500 Subject: [PATCH 2/3] Mobile: fix Codex round-1 findings on #146 (proliferation, Devices visibility) - rememberDevice no longer saves a new row on every ambiguous id-less reconnect: with two or more existing matches it writes nothing, since claiming one would be a guess and a fresh row per reconnect would eventually evict a genuine saved TV once MAX is reached. - Add shared savedDeviceIsLiveConnection (identity.ts): a hardware-id match is always unambiguous, an id-less match only counts when it is the single id-less row at that endpoint. discoveryRows.ts now uses this one helper instead of its own duplicated ambiguity bookkeeping. - Devices.svelte's "Other TVs" filter used the plain endpoint-based savedDeviceMatchesConnection, which hid every ambiguous id-less row (including their Forget controls) once the live TV reported no serial. Switched to savedDeviceIsLiveConnection so an ambiguous row stays visible. --- v2/mobile/src/lib/discoveryRows.ts | 35 +++---------------- v2/mobile/src/lib/identity.ts | 25 ++++++++++++++ v2/mobile/src/lib/savedDevices.ts | 11 +++--- v2/mobile/src/screens/Devices.svelte | 9 +++-- v2/mobile/tests/savedDevices.test.mjs | 50 +++++++++++++++++++++++++-- 5 files changed, 91 insertions(+), 39 deletions(-) diff --git a/v2/mobile/src/lib/discoveryRows.ts b/v2/mobile/src/lib/discoveryRows.ts index f459403..4c3ea6a 100644 --- a/v2/mobile/src/lib/discoveryRows.ts +++ b/v2/mobile/src/lib/discoveryRows.ts @@ -1,6 +1,7 @@ import type { Discovery, SavedDevice } from "./types"; import { normalizeHardwareId, + savedDeviceIsLiveConnection, savedDeviceKey, savedDeviceMatchesConnection, } from "./identity"; @@ -114,21 +115,6 @@ export function buildDiscoveryRows( // other host is stale. Only that advert is dropped, never the other // devices answering from the same address. const liveId = live.connected ? normalizeHardwareId(live.hardwareId) : undefined; - const liveSaved = liveId - ? savedDevices.find((saved) => normalizeHardwareId(saved.hardwareId) === liveId) - : undefined; - // How many id-less saved rows claim a given endpoint. More than one means - // an id-less live connection there cannot be attributed to either of - // them -- the bare address is all any id-less row has, and both have it - // equally, so neither may be shown as the one that is connected. - const idlessCountAt = new Map(); - for (const saved of savedDevices) { - if (normalizeHardwareId(saved.hardwareId) !== undefined) continue; - const endpoint = `${saved.host}:${saved.connectPort}`; - idlessCountAt.set(endpoint, (idlessCountAt.get(endpoint) ?? 0) + 1); - } - const idlessAmbiguousAt = (host: string, port: number): boolean => - (idlessCountAt.get(`${host}:${port}`) ?? 0) > 1; const hosts = new Map(); for (const discovery of discoveries) { const host = discovery.host.trim(); @@ -188,17 +174,9 @@ export function buildDiscoveryRows( // id-less saved TV at this exact endpoint -- and only when that endpoint // has exactly one such TV saved, never a guess between several. if (isLive) { - if (liveId) { - savedMatch = liveSaved; - } else if (!idlessAmbiguousAt(live.host, live.connectPort)) { - savedMatch = savedDevices.find( - (saved) => - normalizeHardwareId(saved.hardwareId) === undefined && - savedDeviceMatchesConnection(saved, live.host, live.connectPort), - ); - } else { - savedMatch = undefined; - } + savedMatch = savedDevices.find((saved) => + savedDeviceIsLiveConnection(saved, savedDevices, live.host, live.connectPort, live.hardwareId), + ); } if (savedMatch) verified.add(savedMatch); rows.push({ @@ -265,10 +243,7 @@ export function buildDiscoveryRows( pairingPorts: [], legacyConnectPorts: [], status: atLiveEndpoint - ? savedDeviceMatchesConnection(saved, live.host, live.connectPort, live.hardwareId) && - // An id-less match at an endpoint shared by another id-less saved - // TV is not evidence this particular row is the live one. - !(normalizeHardwareId(saved.hardwareId) === undefined && idlessAmbiguousAt(saved.host, saved.connectPort)) + ? savedDeviceIsLiveConnection(saved, savedDevices, live.host, live.connectPort, live.hardwareId) ? "connected" // Something answers at this saved address, but nothing shows it is // this TV, so the row claims the address and not the connection. diff --git a/v2/mobile/src/lib/identity.ts b/v2/mobile/src/lib/identity.ts index 46b3cb4..7a9fcaa 100644 --- a/v2/mobile/src/lib/identity.ts +++ b/v2/mobile/src/lib/identity.ts @@ -31,6 +31,31 @@ export function savedDeviceMatchesConnection( ); } +/// Whether a saved row can be told apart as *the specific TV* live on this +/// connection, not just a row that matches the bare address. A verified +/// hardware id is always unambiguous -- ids are unique by construction. An +/// id-less match is only unambiguous when it is the single id-less row saved +/// at that exact endpoint; when another id-less TV shares it, the address +/// alone cannot say which of them answered, so neither counts as the live +/// row here. +export function savedDeviceIsLiveConnection( + device: SavedDevice, + allSaved: SavedDevice[], + host: string, + connectPort: number, + hardwareId?: string | null, +): boolean { + if (!savedDeviceMatchesConnection(device, host, connectPort, hardwareId)) return false; + if (normalizeHardwareId(hardwareId)) return true; + const idlessAtEndpoint = allSaved.filter( + (other) => + normalizeHardwareId(other.hardwareId) === undefined && + other.host === host && + other.connectPort === connectPort, + ).length; + return idlessAtEndpoint <= 1; +} + export function savedDeviceKey(device: SavedDevice): string { const hardwareId = normalizeHardwareId(device.hardwareId); if (hardwareId) return `hardware:${hardwareId}`; diff --git a/v2/mobile/src/lib/savedDevices.ts b/v2/mobile/src/lib/savedDevices.ts index d890d6a..1ab8bf1 100644 --- a/v2/mobile/src/lib/savedDevices.ts +++ b/v2/mobile/src/lib/savedDevices.ts @@ -11,11 +11,12 @@ import type { Device, SavedDevice } from "./types"; import { deviceLabelOf } from "./types"; import { normalizeHardwareId, + savedDeviceIsLiveConnection, savedDeviceKey, savedDeviceMatchesConnection, } from "./identity"; -export { savedDeviceKey, savedDeviceMatchesConnection }; +export { savedDeviceIsLiveConnection, savedDeviceKey, savedDeviceMatchesConnection }; const KEY = "atv.savedDevices.v1"; const AUTO_KEY = "atv.autoConnect.v1"; @@ -200,9 +201,11 @@ export function rememberDevice( const matches = current.filter((d) => sameTv(d, host, connectPort, hardwareId)); // An id-less connection can match more than one saved row only when several // id-less TVs have shared this exact endpoint over time. Which one just - // answered is not knowable from the endpoint alone, so none of them is - // claimed -- a new row is saved rather than overwriting a guess. - const existing = matches.length === 1 ? matches[0] : undefined; + // answered is not knowable from the endpoint alone, so nothing is written: + // claiming one would be a guess, and saving a fresh row on every repeat + // reconnect would eventually evict a genuine saved TV once MAX is reached. + if (matches.length > 1) return; + const existing = matches[0]; const combinedHardwareId = hardwareId ?? existing?.hardwareId; const reportedFriendlyName = device?.properties?.friendly_name?.trim(); const name = reportedFriendlyName || existing?.name || deviceLabelOf(device); diff --git a/v2/mobile/src/screens/Devices.svelte b/v2/mobile/src/screens/Devices.svelte index 59f3044..d34b249 100644 --- a/v2/mobile/src/screens/Devices.svelte +++ b/v2/mobile/src/screens/Devices.svelte @@ -6,8 +6,8 @@ forgetSavedDevice, lastUsedLabel, listSavedDevices, + savedDeviceIsLiveConnection, savedDeviceKey, - savedDeviceMatchesConnection, savedHostHasMultipleIdentities, } from "../lib/savedDevices"; import { deviceTypeLabel } from "../lib/types"; @@ -75,8 +75,13 @@ saved.filter( (device) => !session.connectedDevice || - !savedDeviceMatchesConnection( + // An id-less row at the live endpoint is only "the current TV" when + // it is the only id-less row saved there -- otherwise which one is + // actually connected cannot be told, so none of them is hidden from + // this list (and its Forget control stays reachable). + !savedDeviceIsLiveConnection( device, + saved, session.host, session.connectPort, currentHardwareId, diff --git a/v2/mobile/tests/savedDevices.test.mjs b/v2/mobile/tests/savedDevices.test.mjs index b46ceee..3d790fc 100644 --- a/v2/mobile/tests/savedDevices.test.mjs +++ b/v2/mobile/tests/savedDevices.test.mjs @@ -393,17 +393,28 @@ test("never guesses which of two id-less TVs at one endpoint just reconnected (# // An id-less connection lands on the one shared endpoint of two already // distinct saved TVs. Which one it is cannot be told from the address // alone, so neither existing row is claimed (and so overwritten with a - // possibly-wrong identity) -- a third, honestly unidentified row is saved. + // possibly-wrong identity) -- and nothing new is persisted either, since a + // fresh unidentified row on every repeat reconnect would eventually evict a + // genuine saved TV once MAX is reached. The connection is simply not + // recorded against any saved identity. savedDevices.rememberDevice("192.168.1.10", 5555, device(undefined, { name: "Just reported", properties: { friendly_name: null, serial_number: undefined }, })); + // Repeating the ambiguous reconnect many times still does not grow storage. + for (let i = 0; i < 20; i++) { + savedDevices.rememberDevice("192.168.1.10", 5555, device(undefined, { + name: "Just reported", + properties: { friendly_name: null, serial_number: undefined }, + })); + } + const rows = savedDevices.listSavedDevices(); - assert.equal(rows.length, 3); + assert.equal(rows.length, 2); assert.deepEqual( new Set(rows.map((row) => row.name)), - new Set(["First TV", "Second TV", "Just reported"]), + new Set(["First TV", "Second TV"]), ); // Both original rows are untouched -- neither lost its name nor its key. assert.equal(rows.some((row) => row.name === "First TV" && savedDevices.savedDeviceKey(row) === "local:first-tv"), true); @@ -565,6 +576,39 @@ test("matches the current TV by verified id or an exact id-less endpoint", () => ); }); +test("only an unambiguous match counts as the live connection (#146)", () => { + const shieldA = saved({ hardwareId: "shield-a" }); + const lone = saved({ hardwareId: undefined, localId: "lone-tv" }); + const first = saved({ hardwareId: undefined, localId: "first-tv" }); + const second = saved({ hardwareId: undefined, localId: "second-tv", name: "Second TV" }); + + // A verified hardware id is unambiguous regardless of what else is saved. + assert.equal( + savedDevices.savedDeviceIsLiveConnection(shieldA, [shieldA, first, second], "192.168.1.10", 5555, "shield-a"), + true, + ); + // A single id-less row at the endpoint is as good as this app's identity + // story gets, so it counts. + assert.equal( + savedDevices.savedDeviceIsLiveConnection(lone, [lone], "192.168.1.10", 5555, undefined), + true, + ); + // Two id-less rows sharing the endpoint: neither may claim the connection. + assert.equal( + savedDevices.savedDeviceIsLiveConnection(first, [first, second], "192.168.1.10", 5555, undefined), + false, + ); + assert.equal( + savedDevices.savedDeviceIsLiveConnection(second, [first, second], "192.168.1.10", 5555, undefined), + false, + ); + // A row that does not even match the endpoint is never live, ambiguous or not. + assert.equal( + savedDevices.savedDeviceIsLiveConnection(first, [first, second], "192.168.1.99", 5555, undefined), + false, + ); +}); + test("forgets the exact selected identity at an identical endpoint", () => { seed([ saved({ hardwareId: "shield-a", lastUsed: "2026-09-03T00:00:00.000Z" }), From eff495c34bf9126bc60a3e567e5f4e14d5826ceb Mon Sep 17 00:00:00 2001 From: Bryan Roscoe Date: Thu, 1 Oct 2026 12:14:23 -0500 Subject: [PATCH 3/3] Mobile: correct #146 docs after the proliferation fix (Codex round 2) HANDOFF.md and BACKLOG.md still described the pre-fix rememberDevice behavior ("a new row is saved" on an ambiguous reconnect). Update both to say the connection is deliberately not persisted at all in that case, and note the shared savedDeviceIsLiveConnection check used by both discoveryRows.ts and the Devices screen. --- v2/mobile/BACKLOG.md | 14 ++++++++++---- v2/mobile/HANDOFF.md | 13 ++++++++----- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/v2/mobile/BACKLOG.md b/v2/mobile/BACKLOG.md index 8077295..c5c824d 100644 --- a/v2/mobile/BACKLOG.md +++ b/v2/mobile/BACKLOG.md @@ -98,10 +98,16 @@ row was keyed by its bare endpoint. Fixed in `savedDevices.ts`/`identity.ts`/`di they shared. Existing stored rows are migrated on first read without losing any. - `rememberDevice` only refreshes an existing id-less row when exactly one saved row matches the connecting endpoint. When two or more already share it, which one just reconnected is unknowable, - so a new row is saved rather than silently overwriting one of the existing guesses. -- `buildDiscoveryRows` never marks more than one id-less saved row "connected" at a shared live - endpoint: the discovery row falls back to a generic name and both saved rows report - "saved-address" instead of one of them claiming the connection. + so the connection is deliberately not persisted against any of them -- it writes nothing, rather + than either overwriting one of them with a guess or saving a new row on every repeat reconnect + (the latter would eventually evict a genuine saved TV once `MAX` is reached). +- A shared `savedDeviceIsLiveConnection` check (`identity.ts`) is the one place that decides + whether a saved row is unambiguously the live connection: always true for a verified hardware id, + true for an id-less match only when it is the single id-less row at that endpoint. Both + `buildDiscoveryRows` and the Devices screen's "Other TVs" filter use it, so neither marks more + than one id-less saved row "connected" at a shared live endpoint, and neither hides an ambiguous + row (and its Forget control) from the list -- the discovery row falls back to a generic name and + both saved rows report "saved-address" instead of one of them claiming the connection. Covered by new tests in `tests/savedDevices.test.mjs` and `tests/discoveryRows.test.mjs`; none has run on a device. diff --git a/v2/mobile/HANDOFF.md b/v2/mobile/HANDOFF.md index e5fd76b..cb6e16e 100644 --- a/v2/mobile/HANDOFF.md +++ b/v2/mobile/HANDOFF.md @@ -45,11 +45,14 @@ lost. Every id-less row now gets its own random `localId` the first time it is p endpoint; existing stored rows are migrated in place on first read, losing nothing. `rememberDevice` only refreshes an existing id-less row when exactly one saved row matches the connecting endpoint — when two or more already share it, which one just reconnected is not -knowable, so a new row is saved rather than overwriting a guess. `discoveryRows.ts` never marks -more than one id-less saved row "connected" at a shared live endpoint either; an ambiguous -endpoint's discovery row shows a generic name and both saved rows report "saved-address" instead. -Covered by new tests in `savedDevices.test.mjs` and `discoveryRows.test.mjs`; unverified on a -device. +knowable, so the connection is deliberately not persisted against any of them (writing nothing, +not a new row every time, is what stops repeat reconnects from eventually evicting a genuine saved +TV once `MAX` is reached). `discoveryRows.ts` and the Devices screen's "Other TVs" filter both go +through one shared `savedDeviceIsLiveConnection` check (`identity.ts`) so neither ever marks more +than one id-less saved row "connected" at a shared live endpoint, or hides an ambiguous row from +the list; the discovery row shows a generic name and both saved rows report "saved-address" +instead. Covered by new tests in `savedDevices.test.mjs` and `discoveryRows.test.mjs`; unverified +on a device. ---