diff --git a/solstone/apps/network/routes.py b/solstone/apps/network/routes.py index 8dae09544..ab75cc450 100644 --- a/solstone/apps/network/routes.py +++ b/solstone/apps/network/routes.py @@ -177,14 +177,6 @@ def _default_device_label() -> str: ) -def _display_label(assigned: str, client: str) -> str: - assigned = (assigned or "").strip() - client = (client or "").strip() - if assigned and client and assigned != client: - return f"{assigned} ({client})" - return assigned or client - - NETWORK_HOME = "home" @@ -814,7 +806,7 @@ def _complete_pairing( *, network: str, sender_instance_id: str | None = None, -) -> tuple[dict[str, Any], str, str]: +) -> tuple[dict[str, Any], ClientEntry, str]: ca = load_or_generate_ca(ca_dir()) cert_label = client_label or assigned_label or _default_device_label() client_cert_pem, fingerprint = sign_csr(ca, csr_pem, cert_label) @@ -845,7 +837,7 @@ def _complete_pairing( peer_instance_id=sender_instance_id, ) create_state_directory(Path(get_journal()), journal_source_record_path.stem) - _authorized().add( + entry = _authorized().add( fingerprint=fingerprint, device_label=assigned_label, instance_id=state.instance_id, @@ -900,7 +892,7 @@ def _complete_pairing( "pair: auto-retire failed for label %r: %s", assigned_label, exc ) - return response, fingerprint, paired_at + return response, entry, paired_at def _emit_pair_complete( @@ -980,7 +972,7 @@ def pair() -> Any: # the peer would therefore read as `anywhere` for the owner's own machine. network = NETWORK_HOME if consumed.same_machine else _rough_network(g.identity.mode) try: - response, fingerprint, paired_at = _complete_pairing( + response, entry, paired_at = _complete_pairing( consumed, csr_pem, assigned_label, @@ -992,8 +984,8 @@ def pair() -> Any: logger.info("pair: bad csr: %s", exc) return error_response(PAIRING_KEY_INVALID, detail=f"bad csr: {exc}") _emit_pair_complete( - _display_label(assigned_label, client_label), - fingerprint, + entry.display_label, + entry.fingerprint, paired_at, network=network, ) @@ -1041,7 +1033,13 @@ def rename() -> Any: ) if not updated: return error_response(PAIRED_DEVICE_NOT_FOUND, detail="fingerprint not paired") - return jsonify({"fingerprint": fingerprint, "label": label.strip()}) + return jsonify( + { + "fingerprint": updated.fingerprint, + "device_label": updated.device_label, + "display_label": updated.display_label, + } + ) @network_bp.route("/unpair", methods=["POST"]) @@ -1140,7 +1138,7 @@ def _entry_to_json(entry: ClientEntry) -> dict[str, Any]: "fingerprint": entry.fingerprint, "fingerprint_short": short_fp, "device_label": entry.device_label, - "display_label": _display_label(entry.device_label, entry.client_label), + "display_label": entry.display_label, "paired_at": entry.paired_at, "last_seen_at": entry.last_seen_at, "role": entry.role, diff --git a/solstone/apps/network/tests/test_api_devices_role.py b/solstone/apps/network/tests/test_api_devices_role.py index caa1432bf..e3663587b 100644 --- a/solstone/apps/network/tests/test_api_devices_role.py +++ b/solstone/apps/network/tests/test_api_devices_role.py @@ -32,7 +32,7 @@ def test_devices_api_includes_role_field(link_env) -> None: @pytest.mark.parametrize( ("assigned", "client", "expected"), [ - ("assigned", "client", "assigned (client)"), + ("assigned", "client", "assigned"), ("same", "same", "same"), ("assigned", "", "assigned"), ("", "client", "client"), diff --git a/solstone/apps/network/tests/test_label_ordinal.py b/solstone/apps/network/tests/test_label_ordinal.py new file mode 100644 index 000000000..3205dd605 --- /dev/null +++ b/solstone/apps/network/tests/test_label_ordinal.py @@ -0,0 +1,91 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright (c) 2026 sol pbc + +from __future__ import annotations + +import json + +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import ec +from cryptography.x509.oid import NameOID + +from solstone.apps.network import routes as link_routes +from solstone.think.link.paths import authorized_clients_path + + +def _make_csr(label: str = "test") -> str: + key = ec.generate_private_key(ec.SECP256R1()) + csr = ( + x509.CertificateSigningRequestBuilder() + .subject_name(x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, label)])) + .sign(key, hashes.SHA256()) + ) + return csr.public_bytes(serialization.Encoding.PEM).decode("ascii") + + +def _pair( + env, + *, + label: str, + client_label: str | None = None, +) -> dict: + started = env.client.post( + "/app/network/pair-start", + json={"device_label": label, "role": ""}, + ) + assert started.status_code == 200 + + body = {"csr": _make_csr(client_label or label or "test")} + if client_label is not None: + body["device_label"] = client_label + response = env.client.post( + f"/app/network/pair?token={started.get_json()['nonce']}", + json=body, + ) + assert response.status_code == 200 + return response.get_json() + + +def test_blank_assigned_label_collides_on_client_label( + link_env, + monkeypatch, +) -> None: + env = link_env() + events: list[tuple[str, str, dict]] = [] + + def record_emit(tract: str, event: str, **payload: object) -> None: + events.append((tract, event, payload)) + + monkeypatch.setattr(link_routes, "emit", record_emit) + + first = _pair(env, label="", client_label="iPhone") + second = _pair(env, label="", client_label="iPhone") + + response = env.client.get("/app/network/api/devices") + assert response.status_code == 200 + devices = { + device["fingerprint"]: device for device in response.get_json()["devices"] + } + assert devices[first["fingerprint"]]["device_label"] == "" + assert devices[first["fingerprint"]]["display_label"] == "iPhone" + assert devices[second["fingerprint"]]["device_label"] == "" + assert devices[second["fingerprint"]]["display_label"] == "iPhone (2)" + + persisted = { + item["fingerprint"]: item + for item in json.loads(authorized_clients_path().read_text("utf-8")) + } + assert persisted[first["fingerprint"]]["device_label"] == "" + assert persisted[first["fingerprint"]]["client_label"] == "iPhone" + assert "label_ordinal" not in persisted[first["fingerprint"]] + assert persisted[second["fingerprint"]]["device_label"] == "" + assert persisted[second["fingerprint"]]["client_label"] == "iPhone" + assert persisted[second["fingerprint"]]["label_ordinal"] == 2 + + pair_complete_labels = [ + payload["device_label"] + for tract, event, payload in events + if tract == "link" and event == "pair_complete" + ] + assert pair_complete_labels == ["iPhone", "iPhone (2)"] diff --git a/solstone/apps/network/tests/test_rename_route.py b/solstone/apps/network/tests/test_rename_route.py index b03e43a27..985e45a78 100644 --- a/solstone/apps/network/tests/test_rename_route.py +++ b/solstone/apps/network/tests/test_rename_route.py @@ -8,6 +8,7 @@ from solstone.think.link.paths import authorized_clients_path FINGERPRINT = "sha256:" + ("a" * 64) UNKNOWN_FINGERPRINT = "sha256:" + ("b" * 64) +SIBLING_FINGERPRINT = "sha256:" + ("c" * 64) PAIRED_AT = "2026-05-20T00:00:00Z" @@ -35,13 +36,52 @@ def test_rename_updates_paired_device_label(link_env) -> None: ) assert response.status_code == 200 - assert response.get_json() == {"fingerprint": FINGERPRINT, "label": "new name"} + assert response.get_json() == { + "fingerprint": FINGERPRINT, + "device_label": "new name", + "display_label": "new name", + } entry = _authorized().get(FINGERPRINT) assert entry is not None assert entry.device_label == "new name" assert entry.client_label == "client-host" +def test_rename_display_label_round_trip_stores_base_label(link_env) -> None: + env = link_env() + authorized = _authorized() + authorized.add( + SIBLING_FINGERPRINT, + "iPhone", + "inst-1", + paired_at="2026-05-20T00:00:00Z", + ) + target = authorized.add( + FINGERPRINT, + "iPhone", + "inst-1", + paired_at="2026-05-20T00:00:01Z", + ) + assert target.display_label == "iPhone (2)" + + response = env.client.post( + "/app/network/rename", + json={"fingerprint": FINGERPRINT, "label": "iPhone (2)"}, + ) + + assert response.status_code == 200 + assert response.get_json() == { + "fingerprint": FINGERPRINT, + "device_label": "iPhone", + "display_label": "iPhone (2)", + } + entry = _authorized().get(FINGERPRINT) + assert entry is not None + assert entry.device_label == "iPhone" + assert entry.label_ordinal == 2 + assert entry.display_label == "iPhone (2)" + + def test_rename_empty_label_returns_invalid_request(link_env) -> None: env = link_env() _add_device() diff --git a/solstone/apps/network/workspace.html b/solstone/apps/network/workspace.html index 0e8322985..12def2dee 100644 --- a/solstone/apps/network/workspace.html +++ b/solstone/apps/network/workspace.html @@ -1383,7 +1383,7 @@ throw new Error(`rename failed: ${response.status}`); } const data = await response.json(); - const persisted = typeof data.label === 'string' ? data.label : input.value.trim(); + const persisted = typeof data.device_label === 'string' ? data.device_label : input.value.trim(); device.device_label = persisted; replaceRenameInput(input, persisted); refreshDevices(); diff --git a/solstone/think/link/auth.py b/solstone/think/link/auth.py index 419575449..a2215315d 100644 --- a/solstone/think/link/auth.py +++ b/solstone/think/link/auth.py @@ -6,7 +6,8 @@ The spl-protocol-fixed core shape is unchanged: `fingerprint`, `device_label`, `paired_at`, `instance_id`, and `role`. `device_label` is the home-assigned, renameable label for the paired client. Solstone also stores local-only -`last_seen_at`, `network`, `client_label`, and browser-uplink fields for UX: +`last_seen_at`, `network`, `client_label`, `label_ordinal`, and +browser-uplink fields for UX: { "fingerprint": "sha256:", @@ -17,6 +18,8 @@ renameable label for the paired client. Solstone also stores local-only "last_seen_at": "2026-04-19T18:03:12Z", // optional; null/absent = never "network": "network", // optional; local display label source "client_label": "jer-laptop", // optional; client self-name/hostname + "label_ordinal": 1, // optional; local display + // ordinal, 1 = no suffix "kind": "cert", // cert | browser "pubkey_spki": "", // browser only "observer_handle": "" // browser only @@ -33,8 +36,8 @@ file write. Convey's pair and unpair routes own the pairing writer surface; the secure listener updates `last_seen_at` and uses this ledger for TLS verification and per-request authorization. -`last_seen_at`, `network`, `client_label`, `kind`, `pubkey_spki`, and -`observer_handle` are local-only — never transmitted externally. +`last_seen_at`, `network`, `client_label`, `label_ordinal`, `kind`, +`pubkey_spki`, and `observer_handle` are local-only — never transmitted externally. """ from __future__ import annotations @@ -64,10 +67,22 @@ class ClientEntry: last_seen_at: str | None = None network: str | None = None client_label: str = "" + label_ordinal: int = 1 kind: str = "cert" pubkey_spki: str | None = None observer_handle: str | None = None + @property + def base_label(self) -> str: + return self.device_label or self.client_label + + @property + def display_label(self) -> str: + base = self.base_label + if self.label_ordinal == 1 or not base: + return base + return f"{base} ({self.label_ordinal})" + class AuthorizedClients: """In-memory view of authorized_clients.json with mtime-based reload.""" @@ -115,24 +130,35 @@ class AuthorizedClients: paired_at: str | None = None, network: str | None = None, client_label: str = "", - ) -> None: - paired_at = paired_at or dt.datetime.now(dt.UTC).strftime("%Y-%m-%dT%H:%M:%SZ") - entry = ClientEntry( - fingerprint=fingerprint, - device_label=device_label, - paired_at=paired_at, - instance_id=instance_id, - role=role, - last_seen_at=None, - network=network, - client_label=client_label, - ) + ) -> ClientEntry: with self._lock: with hold_lock(self._path): current = self._load_file_locked() + paired_at = paired_at or dt.datetime.now(dt.UTC).strftime( + "%Y-%m-%dT%H:%M:%SZ" + ) + entry = ClientEntry( + fingerprint=fingerprint, + device_label=device_label, + paired_at=paired_at, + instance_id=instance_id, + role=role, + last_seen_at=None, + network=network, + client_label=client_label, + ) + entry = replace( + entry, + label_ordinal=self._allocate_label_ordinal_locked( + current, + entry.base_label, + fingerprint, + ), + ) current[fingerprint] = entry self._write(current) self._entries = current + return entry def add_browser( self, @@ -145,27 +171,37 @@ class AuthorizedClients: paired_at: str | None = None, network: str | None = None, ) -> ClientEntry: - paired_at = paired_at or dt.datetime.now(dt.UTC).strftime("%Y-%m-%dT%H:%M:%SZ") - entry = ClientEntry( - fingerprint=fingerprint, - device_label=device_label, - paired_at=paired_at, - instance_id=instance_id, - role="", - last_seen_at=None, - network=network, - client_label="", - kind="browser", - pubkey_spki=pubkey_spki, - observer_handle=observer_handle, - ) with self._lock: with hold_lock(self._path): current = self._load_file_locked() + paired_at = paired_at or dt.datetime.now(dt.UTC).strftime( + "%Y-%m-%dT%H:%M:%SZ" + ) + entry = ClientEntry( + fingerprint=fingerprint, + device_label=device_label, + paired_at=paired_at, + instance_id=instance_id, + role="", + last_seen_at=None, + network=network, + client_label="", + kind="browser", + pubkey_spki=pubkey_spki, + observer_handle=observer_handle, + ) + entry = replace( + entry, + label_ordinal=self._allocate_label_ordinal_locked( + current, + entry.base_label, + fingerprint, + ), + ) current[fingerprint] = entry self._write(current) self._entries = current - return entry + return entry def attach_observer_handle(self, fingerprint: str, observer_handle: str) -> bool: normalized = observer_handle.strip() @@ -209,23 +245,34 @@ class AuthorizedClients: self._entries = current return True - def update_label(self, fingerprint: str, label: str) -> bool: - """Update device_label for a paired device. Returns False if not paired.""" + def update_label(self, fingerprint: str, label: str) -> ClientEntry | None: + """Update device_label for a paired device. Returns None if not paired.""" normalized = label.strip() if not normalized: raise ValueError("label must not be empty") - if len(normalized) > MAX_DEVICE_LABEL_LEN: - raise ValueError("label too long") with self._lock: with hold_lock(self._path): current = self._load_file_locked() existing = current.get(fingerprint) if existing is None: - return False - current[fingerprint] = replace(existing, device_label=normalized) + return None + if normalized == existing.display_label: + normalized = existing.base_label + if len(normalized) > MAX_DEVICE_LABEL_LEN: + raise ValueError("label too long") + updated = replace(existing, device_label=normalized) + updated = replace( + updated, + label_ordinal=self._allocate_label_ordinal_locked( + current, + updated.base_label, + fingerprint, + ), + ) + current[fingerprint] = updated self._write(current) self._entries = current - return True + return updated def snapshot(self) -> list[ClientEntry]: self.reload_if_stale() @@ -253,13 +300,56 @@ class AuthorizedClients: except FileNotFoundError: self._mtime_ns = 0 + def backfill_label_ordinals(self) -> bool: + """Repair duplicate label ordinals in authorized_clients.json.""" + with self._lock: + with hold_lock(self._path): + current = self._load_file_locked() + groups: dict[str, list[ClientEntry]] = {} + for entry in current.values(): + base = entry.base_label + if not base: + continue + groups.setdefault(base, []).append(entry) + + changed = False + for entries in groups.values(): + seen: set[int] = set() + needs_repair = False + for entry in entries: + if entry.label_ordinal in seen: + needs_repair = True + break + seen.add(entry.label_ordinal) + if not needs_repair: + continue + + for ordinal, entry in enumerate( + sorted(entries, key=lambda e: (e.paired_at, e.fingerprint)), + start=1, + ): + if entry.label_ordinal == ordinal: + continue + current[entry.fingerprint] = replace( + entry, + label_ordinal=ordinal, + ) + changed = True + + if not changed: + return False + self._write(current) + self._entries = current + return True + def _load_file_locked(self) -> dict[str, ClientEntry]: if not self._path.exists(): return {} try: raw = json.loads(self._path.read_text("utf-8")) except (json.JSONDecodeError, OSError): - # Unreadable authorized_clients.json means no clients are authorized. There is no last-good authorization cache. + # Unreadable authorized_clients.json means no clients are authorized. + # There is no last-good authorization cache. return {} out: dict[str, ClientEntry] = {} if isinstance(raw, list): @@ -275,6 +365,14 @@ class AuthorizedClients: kind = item.get("kind") pubkey_spki = item.get("pubkey_spki") observer_handle = item.get("observer_handle") + raw_label_ordinal = item.get("label_ordinal", 1) + label_ordinal = ( + raw_label_ordinal + if isinstance(raw_label_ordinal, int) + and not isinstance(raw_label_ordinal, bool) + and raw_label_ordinal > 0 + else 1 + ) out[fp] = ClientEntry( fingerprint=fp, device_label=str(item.get("device_label", "")), @@ -284,6 +382,7 @@ class AuthorizedClients: last_seen_at=last_seen if isinstance(last_seen, str) else None, network=network if isinstance(network, str) else None, client_label=client_label if isinstance(client_label, str) else "", + label_ordinal=label_ordinal, kind=kind if isinstance(kind, str) and kind else "cert", pubkey_spki=pubkey_spki if isinstance(pubkey_spki, str) else None, observer_handle=( @@ -292,6 +391,24 @@ class AuthorizedClients: ) return out + def _allocate_label_ordinal_locked( + self, + entries: dict[str, ClientEntry], + base_label: str, + fingerprint: str, + ) -> int: + if not base_label: + return 1 + held = { + entry.label_ordinal + for entry in entries.values() + if entry.fingerprint != fingerprint and entry.base_label == base_label + } + ordinal = 1 + while ordinal in held: + ordinal += 1 + return ordinal + def _write(self, entries: dict[str, ClientEntry]) -> None: payload = [ { @@ -304,6 +421,7 @@ class AuthorizedClients: **({"last_seen_at": e.last_seen_at} if e.last_seen_at else {}), **({"network": e.network} if e.network else {}), **({"client_label": e.client_label} if e.client_label else {}), + **({"label_ordinal": e.label_ordinal} if e.label_ordinal != 1 else {}), **({"pubkey_spki": e.pubkey_spki} if e.pubkey_spki else {}), **({"observer_handle": e.observer_handle} if e.observer_handle else {}), } diff --git a/solstone/think/link/runtime.py b/solstone/think/link/runtime.py index 464e0e99b..37d69566a 100644 --- a/solstone/think/link/runtime.py +++ b/solstone/think/link/runtime.py @@ -12,10 +12,12 @@ import threading from dataclasses import dataclass, field from typing import Any +from .auth import AuthorizedClients from .interface_watcher import ( InterfaceWatcher, set_interface_watcher, ) +from .paths import authorized_clients_path logger = logging.getLogger("link.runtime") @@ -32,6 +34,7 @@ class RuntimeState: _RUNTIME_LOCK = threading.Lock() _runtime: RuntimeState | None = None _atexit_registered = False +_label_ordinal_backfill_ran = False def get_runtime_state() -> RuntimeState | None: @@ -58,11 +61,28 @@ def _thread_main(runtime: RuntimeState) -> None: loop.close() +def _backfill_label_ordinals_once() -> None: + global _label_ordinal_backfill_ran + + if _label_ordinal_backfill_ran: + return + _label_ordinal_backfill_ran = True + try: + authorized = AuthorizedClients(authorized_clients_path()) + repaired = authorized.backfill_label_ordinals() + except Exception: + logger.exception("link runtime label ordinal backfill failed") + return + if repaired: + logger.info("link runtime repaired duplicate label ordinals") + + def start_link_runtime(app: Any) -> None: """Start the interface watcher in the Convey process.""" global _runtime, _atexit_registered with _RUNTIME_LOCK: + _backfill_label_ordinals_once() if _runtime is None: runtime = RuntimeState() thread = threading.Thread( @@ -99,11 +119,12 @@ def stop_link_runtime(app: Any) -> None: def stop_all_link_runtime() -> None: - global _runtime + global _label_ordinal_backfill_ran, _runtime with _RUNTIME_LOCK: runtime = _runtime _runtime = None + _label_ordinal_backfill_ran = False if runtime is None: return for app in list(runtime.apps): diff --git a/tests/link/test_auth.py b/tests/link/test_auth.py index 4e583c7a3..9c8df5dd6 100644 --- a/tests/link/test_auth.py +++ b/tests/link/test_auth.py @@ -7,11 +7,15 @@ from __future__ import annotations import datetime as dt import json +import threading import time +from collections.abc import Iterator +from contextlib import contextmanager from pathlib import Path import pytest +import solstone.think.link.auth as auth_module from solstone.think.link.auth import MAX_DEVICE_LABEL_LEN, AuthorizedClients @@ -98,6 +102,7 @@ def test_add_then_last_seen_key_absent_in_payload(tmp_path: Path) -> None: assert payload[0]["role"] == "" assert "last_seen_at" not in payload[0] assert "client_label" not in payload[0] + assert "label_ordinal" not in payload[0] def test_network_round_trips(tmp_path: Path) -> None: @@ -128,6 +133,57 @@ def test_client_label_round_trips(tmp_path: Path) -> None: assert reloaded.client_label == "client-host" +def test_label_ordinal_round_trips_and_invalid_values_default( + tmp_path: Path, +) -> None: + path = tmp_path / "auth.json" + path.write_text( + json.dumps( + [ + { + "fingerprint": "sha256:good", + "device_label": "phone", + "paired_at": "2026-04-19T00:00:00Z", + "instance_id": "inst-1", + "label_ordinal": 3, + }, + { + "fingerprint": "sha256:bool", + "device_label": "phone", + "paired_at": "2026-04-19T00:00:01Z", + "instance_id": "inst-1", + "label_ordinal": True, + }, + { + "fingerprint": "sha256:zero", + "device_label": "phone", + "paired_at": "2026-04-19T00:00:02Z", + "instance_id": "inst-1", + "label_ordinal": 0, + }, + { + "fingerprint": "sha256:string", + "device_label": "phone", + "paired_at": "2026-04-19T00:00:03Z", + "instance_id": "inst-1", + "label_ordinal": "2", + }, + ], + indent=2, + ) + + "\n", + encoding="utf-8", + ) + + entries = {entry.fingerprint: entry for entry in AuthorizedClients(path).snapshot()} + + assert entries["sha256:good"].label_ordinal == 3 + assert entries["sha256:good"].display_label == "phone (3)" + assert entries["sha256:bool"].label_ordinal == 1 + assert entries["sha256:zero"].label_ordinal == 1 + assert entries["sha256:string"].label_ordinal == 1 + + def test_missing_network_defaults_to_none(tmp_path: Path) -> None: path = tmp_path / "auth.json" path.write_text( @@ -231,6 +287,70 @@ def test_touch_last_seen_preserves_network(tmp_path: Path) -> None: assert _load_payload(path)[0]["network"] == "anywhere" +def test_add_allocates_ordinals_and_touch_preserves_after_reload( + tmp_path: Path, +) -> None: + path = tmp_path / "auth.json" + store = AuthorizedClients(path) + + first = store.add( + "sha256:a", + "iPhone", + "inst-1", + paired_at="2026-04-19T00:00:00Z", + ) + second = store.add( + "sha256:b", + "iPhone", + "inst-1", + paired_at="2026-04-19T00:00:01Z", + ) + + assert first.display_label == "iPhone" + assert second.label_ordinal == 2 + assert second.display_label == "iPhone (2)" + payload = {item["fingerprint"]: item for item in _load_payload(path)} + assert "label_ordinal" not in payload["sha256:a"] + assert payload["sha256:b"]["label_ordinal"] == 2 + + assert store.touch_last_seen( + "sha256:b", + now=dt.datetime(2026, 4, 19, 18, 3, 12, tzinfo=dt.UTC), + ) + reloaded = AuthorizedClients(path).get("sha256:b") + + assert reloaded is not None + assert reloaded.label_ordinal == 2 + assert reloaded.display_label == "iPhone (2)" + + +def test_blank_base_entries_keep_ordinal_one(tmp_path: Path) -> None: + path = tmp_path / "auth.json" + store = AuthorizedClients(path) + + first = store.add("sha256:a", "", "inst-1") + second = store.add("sha256:b", "", "inst-1") + + assert first.display_label == "" + assert second.display_label == "" + assert {entry.label_ordinal for entry in AuthorizedClients(path).snapshot()} == {1} + assert all("label_ordinal" not in item for item in _load_payload(path)) + + +def test_removed_sibling_does_not_renumber_sticky_ordinal(tmp_path: Path) -> None: + path = tmp_path / "auth.json" + store = AuthorizedClients(path) + store.add("sha256:a", "iPhone", "inst-1") + store.add("sha256:b", "iPhone", "inst-1") + + assert store.remove("sha256:a") is True + reloaded = AuthorizedClients(path).get("sha256:b") + + assert reloaded is not None + assert reloaded.label_ordinal == 2 + assert reloaded.display_label == "iPhone (2)" + + def test_update_label_updates_and_persists(tmp_path: Path) -> None: path = tmp_path / "auth.json" store = AuthorizedClients(path) @@ -238,7 +358,10 @@ def test_update_label_updates_and_persists(tmp_path: Path) -> None: store.add(fingerprint, "old name", "inst-1") - assert store.update_label(fingerprint, " new name ") is True + updated = store.update_label(fingerprint, " new name ") + assert updated is not None + assert updated.device_label == "new name" + assert updated.display_label == "new name" entry = store.get(fingerprint) assert entry is not None assert entry.device_label == "new name" @@ -252,7 +375,7 @@ def test_update_label_preserves_network(tmp_path: Path) -> None: store = AuthorizedClients(path) store.add("sha256:abc", "old name", "inst-1", network="anywhere") - assert store.update_label("sha256:abc", "new name") is True + assert store.update_label("sha256:abc", "new name") is not None entry = store.get("sha256:abc") assert entry is not None @@ -266,7 +389,7 @@ def test_update_label_preserves_client_label(tmp_path: Path) -> None: store = AuthorizedClients(path) store.add("sha256:abc", "old name", "inst-1", client_label="client-host") - assert store.update_label("sha256:abc", "new name") is True + assert store.update_label("sha256:abc", "new name") is not None entry = store.get("sha256:abc") assert entry is not None @@ -275,6 +398,42 @@ def test_update_label_preserves_client_label(tmp_path: Path) -> None: assert _load_payload(path)[0]["client_label"] == "client-host" +def test_update_label_allocates_for_new_base_not_old_ordinal(tmp_path: Path) -> None: + path = tmp_path / "auth.json" + store = AuthorizedClients(path) + store.add("sha256:a", "iPhone", "inst-1") + store.add("sha256:b", "iPhone", "inst-1") + + updated = store.update_label("sha256:b", "Work Phone") + + assert updated is not None + assert updated.label_ordinal == 1 + assert updated.display_label == "Work Phone" + payload = {item["fingerprint"]: item for item in _load_payload(path)} + assert "label_ordinal" not in payload["sha256:b"] + + +def test_update_label_display_round_trip_promotes_base_label( + tmp_path: Path, +) -> None: + path = tmp_path / "auth.json" + store = AuthorizedClients(path) + base = "x" * MAX_DEVICE_LABEL_LEN + store.add("sha256:a", base, "inst-1") + target = store.add("sha256:b", "", "inst-1", client_label=base) + assert len(target.display_label) == MAX_DEVICE_LABEL_LEN + 4 + + updated = store.update_label("sha256:b", target.display_label) + + assert updated is not None + assert updated.device_label == base + assert updated.label_ordinal == 2 + assert updated.display_label == target.display_label + reloaded = AuthorizedClients(path).get("sha256:b") + assert reloaded is not None + assert reloaded.device_label == base + + @pytest.mark.parametrize( ("label", "message"), [ @@ -298,11 +457,11 @@ def test_update_label_rejects_invalid_labels( def test_update_label_unknown_fp_returns_false(tmp_path: Path) -> None: store = AuthorizedClients(tmp_path / "auth.json") - assert store.update_label("sha256:deadbeef", "new name") is False + assert store.update_label("sha256:deadbeef", "new name") is None store.add("sha256:abc", "old name", "inst-1") - assert store.update_label("sha256:deadbeef", "new name") is False + assert store.update_label("sha256:deadbeef", "new name") is None def test_update_label_rereads_file_and_preserves_interleaved_last_seen( @@ -316,7 +475,7 @@ def test_update_label_rereads_file_and_preserves_interleaved_last_seen( first.add(fingerprint, "old name", "inst-1") assert second.touch_last_seen(fingerprint, now=seen_at) is True - assert first.update_label(fingerprint, "new name") is True + assert first.update_label(fingerprint, "new name") is not None final = AuthorizedClients(path).get(fingerprint) assert final is not None @@ -324,6 +483,161 @@ def test_update_label_rereads_file_and_preserves_interleaved_last_seen( assert final.last_seen_at == "2026-04-19T18:03:12Z" +def test_backfill_label_ordinals_repairs_duplicates_idempotently( + tmp_path: Path, +) -> None: + path = tmp_path / "auth.json" + path.write_text( + json.dumps( + [ + { + "fingerprint": "sha256:c", + "device_label": "iPhone", + "paired_at": "2026-04-19T00:00:03Z", + "instance_id": "inst-1", + }, + { + "fingerprint": "sha256:a", + "device_label": "iPhone", + "paired_at": "2026-04-19T00:00:01Z", + "instance_id": "inst-1", + }, + { + "fingerprint": "sha256:b", + "device_label": "iPhone", + "paired_at": "2026-04-19T00:00:02Z", + "instance_id": "inst-1", + }, + { + "fingerprint": "sha256:blank-a", + "device_label": "", + "paired_at": "2026-04-19T00:00:01Z", + "instance_id": "inst-1", + }, + { + "fingerprint": "sha256:blank-b", + "device_label": "", + "paired_at": "2026-04-19T00:00:02Z", + "instance_id": "inst-1", + }, + ], + indent=2, + ) + + "\n", + encoding="utf-8", + ) + before_load = path.read_bytes() + store = AuthorizedClients(path) + + assert path.read_bytes() == before_load + assert store.backfill_label_ordinals() is True + payload = {item["fingerprint"]: item for item in _load_payload(path)} + assert "label_ordinal" not in payload["sha256:a"] + assert payload["sha256:b"]["label_ordinal"] == 2 + assert payload["sha256:c"]["label_ordinal"] == 3 + assert "label_ordinal" not in payload["sha256:blank-a"] + assert "label_ordinal" not in payload["sha256:blank-b"] + + after_backfill = path.read_bytes() + assert store.backfill_label_ordinals() is False + assert path.read_bytes() == after_backfill + + assert store.touch_last_seen( + "sha256:b", + now=dt.datetime(2026, 4, 19, 18, 3, 12, tzinfo=dt.UTC), + ) + after_touch = path.read_bytes() + assert store.backfill_label_ordinals() is False + assert path.read_bytes() == after_touch + reloaded = AuthorizedClients(path).get("sha256:b") + assert reloaded is not None + assert reloaded.label_ordinal == 2 + + +def test_backfill_label_ordinals_preserves_sticky_non_duplicates( + tmp_path: Path, +) -> None: + path = tmp_path / "auth.json" + path.write_text( + json.dumps( + [ + { + "fingerprint": "sha256:a", + "device_label": "iPhone", + "paired_at": "2026-04-19T00:00:01Z", + "instance_id": "inst-1", + "label_ordinal": 1, + }, + { + "fingerprint": "sha256:b", + "device_label": "iPhone", + "paired_at": "2026-04-19T00:00:02Z", + "instance_id": "inst-1", + "label_ordinal": 3, + }, + { + "fingerprint": "sha256:c", + "device_label": "iPad", + "paired_at": "2026-04-19T00:00:03Z", + "instance_id": "inst-1", + "label_ordinal": 2, + }, + ], + indent=2, + ) + + "\n", + encoding="utf-8", + ) + before = path.read_bytes() + + assert AuthorizedClients(path).backfill_label_ordinals() is False + assert path.read_bytes() == before + + +def test_concurrent_add_allocates_distinct_ordinals( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + path = tmp_path / "auth.json" + first = AuthorizedClients(path) + second = AuthorizedClients(path) + barrier = threading.Barrier(2) + real_hold_lock = auth_module.hold_lock + + @contextmanager + def hold_lock_with_barrier(path_arg: Path, **kwargs: object) -> Iterator[None]: + barrier.wait(timeout=5) + with real_hold_lock(path_arg, **kwargs): + yield + + monkeypatch.setattr(auth_module, "hold_lock", hold_lock_with_barrier) + errors: list[Exception] = [] + + def add(store: AuthorizedClients, fingerprint: str) -> None: + try: + store.add(fingerprint, "iPhone", "inst-1") + except Exception as exc: + errors.append(exc) + + threads = [ + threading.Thread(target=add, args=(first, "sha256:a")), + threading.Thread(target=add, args=(second, "sha256:b")), + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=10) + + assert not any(thread.is_alive() for thread in threads) + assert not errors + entries = {entry.fingerprint: entry for entry in AuthorizedClients(path).snapshot()} + assert {entry.label_ordinal for entry in entries.values()} == {1, 2} + assert sorted(entry.display_label for entry in entries.values()) == [ + "iPhone", + "iPhone (2)", + ] + + def test_find_by_label(tmp_path: Path) -> None: path = tmp_path / "auth.json" store = AuthorizedClients(path) diff --git a/tests/link/test_runtime.py b/tests/link/test_runtime.py new file mode 100644 index 000000000..70f451f14 --- /dev/null +++ b/tests/link/test_runtime.py @@ -0,0 +1,76 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright (c) 2026 sol pbc + +from __future__ import annotations + +import json +from types import SimpleNamespace + +import pytest + +from solstone.think.link import runtime as link_runtime +from solstone.think.link.auth import AuthorizedClients +from solstone.think.link.paths import authorized_clients_path + + +def test_start_link_runtime_backfills_label_ordinals_once( + tmp_path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + journal = tmp_path / "journal" + monkeypatch.setenv("SOLSTONE_JOURNAL", str(journal)) + link_runtime.stop_all_link_runtime() + path = authorized_clients_path() + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps( + [ + { + "fingerprint": "sha256:a", + "device_label": "iPhone", + "paired_at": "2026-04-19T00:00:01Z", + "instance_id": "inst-1", + }, + { + "fingerprint": "sha256:b", + "device_label": "iPhone", + "paired_at": "2026-04-19T00:00:02Z", + "instance_id": "inst-1", + }, + ], + indent=2, + ) + + "\n", + encoding="utf-8", + ) + calls = [] + real_backfill = AuthorizedClients.backfill_label_ordinals + + def record_backfill(self: AuthorizedClients) -> bool: + calls.append(self.path) + return real_backfill(self) + + monkeypatch.setattr(AuthorizedClients, "backfill_label_ordinals", record_backfill) + monkeypatch.setattr( + link_runtime, + "_thread_main", + lambda runtime: runtime.started_event.set(), + ) + + try: + first_app = SimpleNamespace() + second_app = SimpleNamespace() + + link_runtime.start_link_runtime(first_app) + link_runtime.start_link_runtime(second_app) + + assert calls == [path] + entries = { + item["fingerprint"]: item for item in json.loads(path.read_text("utf-8")) + } + assert "label_ordinal" not in entries["sha256:a"] + assert entries["sha256:b"]["label_ordinal"] == 2 + assert first_app.link_runtime_started is True + assert second_app.link_runtime_started is True + finally: + link_runtime.stop_all_link_runtime()