Mobile: keep id-less saved TVs distinct and honest about identity (#146)

v2/mobile/BACKLOG.md

@@ -75,16 +75,43 @@ 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).
 - Scan rows are built per stored TV, never per address; no synthesized "Saved TV" row (#115).
 - `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 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.
+
 ## Stability reset (2026-09-04)
 
 A four-track audit (feature parity vs v1/desktop, connection lifecycle, screen UX, Rust backend)

v2/mobile/HANDOFF.md

@@ -37,6 +37,23 @@ 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 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.
+
 ---
 
 ## 1. What this is

v2/mobile/src/lib/discoveryRows.ts

@@ -1,6 +1,7 @@
 import type { Discovery, SavedDevice } from "./types";
 import {
   normalizeHardwareId,
+  savedDeviceIsLiveConnection,
   savedDeviceKey,
   savedDeviceMatchesConnection,
 } from "./identity";
@@ -114,9 +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;
   const hosts = new Map<string, HostGroup>();
   for (const discovery of discoveries) {
     const host = discovery.host.trim();
@@ -173,15 +171,12 @@ 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),
-          );
+      savedMatch = savedDevices.find((saved) =>
+        savedDeviceIsLiveConnection(saved, savedDevices, live.host, live.connectPort, live.hardwareId),
+      );
     }
     if (savedMatch) verified.add(savedMatch);
     rows.push({
@@ -248,7 +243,7 @@ export function buildDiscoveryRows(
       pairingPorts: [],
       legacyConnectPorts: [],
       status: atLiveEndpoint
-        ? savedDeviceMatchesConnection(saved, live.host, live.connectPort, live.hardwareId)
+        ? 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.

v2/mobile/src/lib/identity.ts

@@ -31,9 +31,39 @@ 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);
-  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}`;
 }

v2/mobile/src/lib/savedDevices.ts

@@ -11,17 +11,24 @@ 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";
 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<string, unknown>;
@@ -48,10 +55,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 +80,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<string, unknown>;
+  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 +120,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 +198,15 @@ 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 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);
   const list = current.filter((d) => d !== existing);
@@ -176,9 +215,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);

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;
 }

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,

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" });