fix(discovery): persist discover.neighbors results with the storage actually wired in

MeshCLI._auto_add_discovery_result reached for storage_handler.record_advert and
returned early when it was absent. record_advert lives on StorageCollector, but
MeshCLI is constructed with the SQLiteHandler (main.py -> TextHelper ->
MeshCLI), which exposes only store_advert. The guard therefore always failed in
production: `discover.neighbors` discovered nodes and recorded none of them, so
`neighbors` listed whatever adverts happened to arrive rather than what
discovery found. Tests missed it because they injected a hand-rolled double that
did have record_advert.

Extract the shared persistence into discovery.persist_discovery_result, which
accepts either storage object its callers actually hold: record_advert when
present (store plus the MQTT/Glass advert publish), falling back to
store_advert. neighbors_publisher._enrich_discovery_result, which had its own
working copy against the StorageCollector, now calls it too, so the two paths
build an identical advert row from one implementation.

The regression test specs its mock off the real SQLiteHandler class rather than
hand-rolling the surface, so a double can no longer claim a method the wired-in
object does not have. It fails against the previous code with KeyError
'auto_added'.

repeater/handler_helpers/discovery.py

@@ -36,6 +36,71 @@ NODE_TYPE_NAMES = {
 }
 
 
+def build_discovery_advert_record(result: dict) -> Optional[dict]:
+    """Turn a discovery response into a zero-hop advert row, or None if unusable.
+
+    A node-discover response is only ever answered by a node that heard our
+    zero-hop broadcast directly, so the row is recorded with ``zero_hop=True``
+    and ``route_type=2`` — the same thing firmware ``putNeighbour()`` records
+    when a discovery response arrives.
+    """
+    pubkey = str(result.get("pub_key") or "").strip().lower()
+    if pubkey.startswith("0x"):
+        pubkey = pubkey[2:]
+    if not pubkey:
+        return None
+
+    node_type = int(result.get("node_type", 0) or 0)
+    rssi = result.get("rssi")
+    # response_snr is our RX of their reply; plain snr is the fallback shape.
+    snr = result.get("response_snr", result.get("snr"))
+
+    return {
+        "timestamp": time.time(),
+        "pubkey": pubkey,
+        "node_name": result.get("node_name"),
+        "is_repeater": node_type == 2,
+        "route_type": 2,
+        "contact_type": NODE_TYPE_NAMES.get(node_type, "Unknown"),
+        "latitude": None,
+        "longitude": None,
+        "rssi": int(rssi) if rssi is not None else None,
+        "snr": float(snr) if snr is not None else None,
+        "is_new_neighbor": True,
+        "zero_hop": True,
+    }
+
+
+def persist_discovery_result(storage, result: dict) -> bool:
+    """Store one discovery response, returning whether it was written.
+
+    Accepts either storage object the callers actually hold: ``StorageCollector``
+    exposes ``record_advert`` (store plus the MQTT/Glass advert publish), while
+    ``SQLiteHandler`` exposes only ``store_advert``. Preferring the first and
+    falling back to the second is what makes this work from the mesh CLI, which
+    is constructed with the SQLiteHandler — checking for ``record_advert`` alone
+    made ``discover.neighbors`` silently persist nothing.
+    """
+    if storage is None:
+        return False
+
+    record = build_discovery_advert_record(result)
+    if record is None:
+        return False
+
+    writer = getattr(storage, "record_advert", None) or getattr(storage, "store_advert", None)
+    if not callable(writer):
+        logger.debug("Storage backend cannot persist discovery results")
+        return False
+
+    try:
+        writer(record)
+        return True
+    except Exception as e:
+        logger.debug("Failed to persist discovery result for %s: %s", record["pubkey"][:8], e)
+        return False
+
+
 class DiscoveryHelper:
     """Helper class for processing discovery requests in the repeater."""
 

repeater/handler_helpers/mesh_cli.py

@@ -129,41 +129,11 @@ class MeshCLI:
         if not self.storage_handler:
             return enriched
 
-        record_advert = getattr(self.storage_handler, "record_advert", None)
-        if not callable(record_advert):
-            return enriched
+        from repeater.handler_helpers.discovery import persist_discovery_result
 
-        try:
-            import time
-
-            node_type = int(enriched.get("node_type", 0) or 0)
-            contact_type = {
-                1: "Chat Node",
-                2: "Repeater",
-                3: "Room Server",
-            }.get(node_type, "Unknown")
-
-            rssi = enriched.get("rssi")
-            snr = enriched.get("response_snr", enriched.get("snr"))
-            advert_record = {
-                "timestamp": time.time(),
-                "pubkey": pub_key,
-                "node_name": enriched.get("node_name"),
-                "is_repeater": node_type == 2,
-                "route_type": 2,
-                "contact_type": contact_type,
-                "latitude": None,
-                "longitude": None,
-                "rssi": int(rssi) if rssi is not None else None,
-                "snr": float(snr) if snr is not None else None,
-                "is_new_neighbor": True,
-                "zero_hop": True,
-            }
-            record_advert(advert_record)
+        if persist_discovery_result(self.storage_handler, enriched):
             enriched["known_neighbor"] = True
             enriched["auto_added"] = True
-        except Exception as exc:
-            logger.debug("Auto-add discovery result failed for %s: %s", pub_key, exc)
 
         return enriched
 

repeater/neighbors_publisher.py

@@ -25,6 +25,7 @@ import time
 from datetime import datetime, timezone
 from typing import Any, Callable, Dict, List, Optional
 
+from repeater.handler_helpers.discovery import persist_discovery_result
 from repeater.handler_helpers.neighbor_scopes import (
     STATUS_RESPONDED,
     STATUS_TIMEOUT,
@@ -431,36 +432,7 @@ class NeighborsPublisher:
                 "snr": float(snr_raw) if snr_raw is not None else 0.0,
             }
 
-        storage = self._storage()
-        record_advert = getattr(storage, "record_advert", None) if storage else None
-        if not callable(record_advert):
-            return result
-
-        node_type = int(result.get("node_type", 0) or 0)
-        rssi = result.get("rssi")
-        snr = result.get("response_snr", result.get("snr"))
-        try:
-            record_advert(
-                {
-                    "timestamp": time.time(),
-                    "pubkey": pubkey,
-                    "node_name": result.get("node_name"),
-                    "is_repeater": node_type == 2,
-                    "route_type": 2,
-                    "contact_type": {1: "Chat Node", 2: "Repeater", 3: "Room Server"}.get(
-                        node_type, "Unknown"
-                    ),
-                    "latitude": None,
-                    "longitude": None,
-                    "rssi": int(rssi) if rssi is not None else None,
-                    "snr": float(snr) if snr is not None else None,
-                    "is_new_neighbor": True,
-                    "zero_hop": True,
-                }
-            )
-        except Exception as e:
-            logger.debug(f"Could not persist discovery result for {pubkey[:8]}: {e}")
-
+        persist_discovery_result(self._storage(), result)
         return result
 
     def _snapshot_neighbors(self) -> List[NeighborSnapshot]:

tests/test_handler_helpers_mesh_cli.py

@@ -475,6 +475,60 @@ def test_discovery_auto_add_skips_local_node_and_persists_remote():
     storage.record_advert.assert_called_once()
 
 
+def test_discovery_auto_add_persists_through_the_storage_actually_wired_in():
+    """MeshCLI is constructed with the SQLiteHandler, not the StorageCollector.
+
+    See repeater/main.py:516 -> TextHelper(sqlite_handler=...storage.sqlite_handler)
+    -> MeshCLI(storage_handler=self.sqlite_handler). SQLiteHandler exposes
+    store_advert but no record_advert, so a persistence path that only looked for
+    record_advert made `discover.neighbors` silently record nothing in production
+    while passing tests that injected a hand-rolled double. Spec'ing the mock off
+    the real class is what keeps this honest.
+    """
+    from repeater.data_acquisition.sqlite_handler import SQLiteHandler
+
+    identity = SimpleNamespace(get_public_key=lambda: bytes.fromhex("11" * 32))
+    storage = MagicMock(spec=SQLiteHandler)
+    cli = MeshCLI(
+        "/tmp/cfg.yaml", _base_config(), _cfg_mgr(), identity=identity, storage_handler=storage
+    )
+
+    result = cli._auto_add_discovery_result(
+        {
+            "pub_key": "22" * 32,
+            "node_name": "Remote Repeater",
+            "node_type": 2,
+            "rssi": -70,
+            "response_snr": 4.25,
+        }
+    )
+
+    assert result["auto_added"] is True
+    storage.store_advert.assert_called_once()
+    record = storage.store_advert.call_args.args[0]
+    assert record["pubkey"] == "22" * 32
+    assert record["is_repeater"] is True
+    assert record["zero_hop"] is True
+    assert record["snr"] == 4.25
+    assert record["rssi"] == -70
+
+
+def test_discovery_auto_add_prefers_record_advert_when_the_collector_is_wired_in():
+    """StorageCollector.record_advert also publishes the advert; prefer it."""
+    from repeater.data_acquisition.storage_collector import StorageCollector
+
+    identity = SimpleNamespace(get_public_key=lambda: bytes.fromhex("11" * 32))
+    storage = MagicMock(spec=StorageCollector)
+    cli = MeshCLI(
+        "/tmp/cfg.yaml", _base_config(), _cfg_mgr(), identity=identity, storage_handler=storage
+    )
+
+    result = cli._auto_add_discovery_result({"pub_key": "22" * 32, "node_type": 2})
+
+    assert result["auto_added"] is True
+    storage.record_advert.assert_called_once()
+
+
 def test_cmd_set_save_failure_reports_error_and_skips_live_update():
     mgr = _cfg_mgr(save_ok=False)
     cli = MeshCLI("/tmp/cfg.yaml", _base_config(), mgr)