fix(mqtt): capture the radio status baseline at boot, not on first publish

The snapshot that keeps non-default radios reporting the band they are
actually on was seeded lazily, on the first status publish. That publish
waits for a broker to connect, so a node whose MQTT is briefly unreachable
could take a live radio save first and bake the pending settings in as
though they were what the hardware got -- reproducing the wrong-band
attribution the snapshot exists to prevent.

Capture it in MeshCoreToMqttPusher.__init__ instead, together with the
default radio id. That id was being recomputed from live config on every
publish, but fabric.default_radio is itself restart-required: a pending
change would refresh the radio that was not retuned and freeze the one that
was, reporting both wrongly where rebuilding everything got one right.

Seeding at construction also removes the unlocked check-then-set on
_applied_radio_map, which the status-publish timer thread shared with
whatever triggered the first publish.

repeater/config.py

@@ -1312,12 +1312,19 @@ def build_metering_profiles(config: dict) -> list:
 
 
 def _default_radio_id(config: dict, radio_ids: list) -> Optional[str]:
-    """The radio a live config save actually retunes.
+    """The radio a live config save retunes, read from a config mapping.
 
-    Mirrors build_radio_stack's TX default -- ``fabric.default_radio`` when it
-    names a radio that exists, else the first configured one. A default naming
-    a radio that is not there cannot be the one that got retuned, so it falls
-    back rather than freezing every entry.
+    Follows build_radio_stack's TX default -- ``fabric.default_radio``, else the
+    first configured radio -- with one deliberate difference: a default naming a
+    radio that is not in ``radio_ids`` falls back to the first instead of being
+    returned as-is. build_radio_stack hands an unknown id straight to FabricRadio
+    and the node fails to start, so that config never reaches a status publish;
+    here the fallback only has to avoid freezing every entry.
+
+    Callers reporting on a *running* node should capture this at boot rather
+    than recompute it: ``fabric.default_radio`` is restart-required, so a live
+    edit changes this answer while the running Fabric keeps its original
+    default. See ``capture_radio_status_baseline``.
     """
     fabric_cfg = config.get("fabric") if isinstance(config.get("fabric"), dict) else {}
     default_radio = fabric_cfg.get("default_radio") or fabric_cfg.get("default_radio_id")
@@ -1326,7 +1333,35 @@ def _default_radio_id(config: dict, radio_ids: list) -> Optional[str]:
     return radio_ids[0] if radio_ids else None
 
 
-def build_radio_status_entries(config: dict, *, applied: Optional[dict] = None) -> list:
+def capture_radio_status_baseline(config: dict) -> tuple:
+    """The radio map and default radio id as built, for a reporter to hold.
+
+    Call this once, from boot config, before anything can edit it. Returns
+    ``(applied, default_radio_id)`` to hand back to
+    ``build_radio_status_entries`` on every later build.
+
+    Both halves have to be captured together and early. The map is what
+    non-default radios keep until they are really reconfigured, and the default
+    id is which entry is allowed to move -- and that id is itself restart-required,
+    so reading it from live config would let a pending ``fabric.default_radio``
+    edit refresh the radio that was not retuned while freezing the one that was,
+    reporting both radios wrongly where rebuilding everything got one right.
+
+    Returns ``({}, None)`` if the config cannot be read, which leaves the caller
+    rebuilding from config exactly as it did before.
+    """
+    try:
+        entries = build_radio_status_entries(config)
+    except Exception as exc:  # pragma: no cover - reporting must not break startup
+        logger.debug("Could not capture the radio status baseline: %s", exc)
+        return {}, None
+    applied = {entry["id"]: entry for entry in entries}
+    return applied, _default_radio_id(config, list(applied))
+
+
+def build_radio_status_entries(
+    config: dict, *, applied: Optional[dict] = None, default_radio_id: Optional[str] = None
+) -> list:
     """Map every radio id to its air settings, for the MQTT status message.
 
     Observers join this against the ``rx_radio_id`` / ``tx_radio_ids`` fields
@@ -1347,9 +1382,12 @@ def build_radio_status_entries(config: dict, *, applied: Optional[dict] = None)
     SX1262 holds -9..22 dBm).
 
     ``applied`` maps radio id to the entry built when that radio was last really
-    configured, and makes the map survive a live save. Only the default radio is
-    retuned without a restart: a change to any other sits in ``radios[]`` marked
-    restart-required. Because ``radios[]`` entries inherit the top-level
+    configured, and ``default_radio_id`` says which entry a live save is allowed
+    to move. Both come from ``capture_radio_status_baseline`` at boot; passing
+    ``applied`` without ``default_radio_id`` falls back to reading the default
+    from ``config``, which is only right if nothing has edited it yet. Only the
+    default radio is retuned without a restart: a change to any other sits in
+    ``radios[]`` marked restart-required. Because ``radios[]`` entries inherit the top-level
     ``radio`` block key by key, a save that edits the default radio also moves
     the *configured* values of every radio whose entry omits that key, so
     rebuilding all of them from config would publish a bandwidth the hardware
@@ -1387,7 +1425,9 @@ def build_radio_status_entries(config: dict, *, applied: Optional[dict] = None)
         entries.append(entry)
 
     if applied:
-        default_id = _default_radio_id(config, [entry["id"] for entry in entries])
+        default_id = default_radio_id
+        if default_id is None:
+            default_id = _default_radio_id(config, [entry["id"] for entry in entries])
         entries = [
             entry if entry["id"] == default_id else applied.get(entry["id"], entry)
             for entry in entries

repeater/data_acquisition/mqtt_handler.py

@@ -669,10 +669,16 @@ class MeshCoreToMqttPusher:
         self._shutdown_requested = False
         self._lock = threading.Lock()
         self._connect_timers: List[threading.Timer] = []
-        # The radio map as built when the radios were last really configured.
-        # Seeded on the first build, which is this process's start -- see
-        # _radio_map for why a later rebuild must not replace it wholesale.
-        self._applied_radio_map: Optional[dict] = None
+        # The radio map and default radio id as the daemon built them, captured
+        # here rather than on the first publish: that publish waits for a broker
+        # to connect, and a node whose MQTT is briefly unreachable can take a
+        # live radio save first, which would bake the pending settings in as
+        # though they were what the hardware got. See _radio_map.
+        from ..config import capture_radio_status_baseline
+
+        self._applied_radio_map, self._applied_default_radio_id = capture_radio_status_baseline(
+            config
+        )
 
         # Initialize brokers list
         mqtt_brokers_config = config.get("mqtt_brokers", {})
@@ -1051,25 +1057,32 @@ class MeshCoreToMqttPusher:
         ``radio`` field already says everything, so the map is published only
         when there is genuinely more than one band to tell apart.
 
-        Rebuilt per status publish rather than cached at construction, because
-        air settings can be changed from the web UI while the node runs. Only
-        the default radio is retuned by such a save, though, and radios[] entries
+        Rebuilt per status publish rather than cached whole, because the default
+        radio's air settings can be changed from the web UI while the node runs.
+        Only that radio is retuned by such a save, though, and radios[] entries
         inherit the top-level ``radio`` block key by key -- so a rebuild that
         took every entry from the current config would report a new bandwidth
         for radios still transmitting on the old one, and observers would
-        attribute their packets to a band they are not on. The first build is
-        kept and handed back in as ``applied`` so every non-default radio keeps
-        what it was really configured with until the service restarts.
+        attribute their packets to a band they are not on. The baseline captured
+        at construction is handed back in, so every non-default radio keeps what
+        it was really configured with until the service restarts.
+
+        The default radio id comes from that same baseline. ``fabric.default_radio``
+        is itself restart-required, so reading it from the live config would let
+        a pending change refresh the radio that was not retuned and freeze the
+        one that was.
         """
         from ..config import build_radio_status_entries
 
         try:
-            entries = build_radio_status_entries(self.config, applied=self._applied_radio_map)
+            entries = build_radio_status_entries(
+                self.config,
+                applied=self._applied_radio_map,
+                default_radio_id=self._applied_default_radio_id,
+            )
         except Exception as exc:  # pragma: no cover - reporting must not break status
             logger.debug(f"Could not build radio map for status: {exc}")
             return []
-        if self._applied_radio_map is None and entries:
-            self._applied_radio_map = {entry["id"]: entry for entry in entries}
         return entries if len(entries) > 1 else []
 
     def publish(self, subtopic: str, payload: dict, retain: bool = False, qos: int = 0):

tests/test_observer_radio_attribution.py

@@ -18,7 +18,12 @@ from unittest.mock import MagicMock, patch
 
 import pytest
 
-from repeater.config import build_radio_status_entries, get_node_info, get_radio_for_board
+from repeater.config import (
+    build_radio_status_entries,
+    capture_radio_status_baseline,
+    get_node_info,
+    get_radio_for_board,
+)
 from repeater.data_acquisition.mqtt_handler import MeshCoreToMqttPusher
 from repeater.data_acquisition.storage_utils import PacketRecord
 
@@ -503,8 +508,14 @@ def test_applied_map_holds_non_default_radios_across_a_live_edit():
 
 
 def test_applied_map_is_ignored_for_a_radio_the_config_has_just_added():
-    """A radio with no applied entry has no band to preserve, so it reports
-    what it is configured with."""
+    """A radio with no applied entry reports what it is configured with.
+
+    Adding a radio is restart-required, so until then it has no hardware on the
+    air at all and arguably should not be published. This pins the behaviour
+    that was already there -- the map has always been built from the current
+    radios[] list -- rather than endorsing it; fixing it means publishing the
+    boot topology, which is a change of its own.
+    """
     config = {
         "radio_type": "sx1262",
         "radio": LOCAL_RADIO["radio"],
@@ -565,3 +576,77 @@ def test_published_status_holds_a_non_retuned_radio_across_a_live_edit():
         "local": "869.618,500.0,8,8",  # retuned
         "link": "864.2,62.5,8,8",  # untouched, still reported as it is
     }
+
+
+def test_a_live_save_before_the_first_publish_does_not_poison_the_baseline():
+    """The first status publish waits for a broker to connect. A node whose MQTT
+    is briefly unreachable can take a radio save first, so a baseline captured
+    on that first publish would bake the pending settings in as though they were
+    what the hardware got."""
+    config = _make_config(
+        radios=[copy.deepcopy(LOCAL_RADIO), {"id": "link", "radio": {"frequency": 864200000}}]
+    )
+    config["radio"] = copy.deepcopy(LOCAL_RADIO["radio"])
+    config["fabric"]["default_radio"] = "local"
+
+    pusher = MeshCoreToMqttPusher(local_identity=_FakeIdentity("AB" * 32), config=config)
+
+    # The save lands while MQTT is still down, before any status was published.
+    config["radios"][0]["radio"]["bandwidth"] = 500000
+    config["radio"]["bandwidth"] = 500000
+
+    conn = pusher.connections[0]
+    captured = []
+    conn._running = True
+    conn.client = MagicMock()
+    conn.client.publish = lambda topic, payload, retain=False, qos=0: captured.append(payload)
+    pusher.publish_status(state="online")
+
+    by_id = {e["id"]: e["radio"] for e in json.loads(captured[0])["radios"]}
+    assert by_id["local"] == "869.618,500.0,8,8"  # really retuned
+    assert by_id["link"] == "864.2,62.5,8,8"  # never was
+
+
+def test_a_pending_default_radio_change_does_not_move_which_entry_is_refreshed():
+    """``fabric.default_radio`` is restart-required, so the running Fabric keeps
+    the default it booted with. Reading the default from live config would
+    refresh the radio that was not retuned and freeze the one that was --
+    reporting both wrongly, where rebuilding everything at least got one right.
+    """
+    config = {
+        "radio_type": "sx1262",
+        "radio": {
+            "frequency": 869618000,
+            "bandwidth": 62500,
+            "spreading_factor": 8,
+            "coding_rate": 8,
+            "preamble_length": 32,
+        },
+        "fabric": {"default_radio": "link"},
+        "radios": [
+            {"id": "local", "radio": {"frequency": 869618000}},
+            {"id": "link", "radio": {"frequency": 864200000}},
+        ],
+    }
+
+    applied, default_id = capture_radio_status_baseline(config)
+    assert default_id == "link"
+
+    # A save retunes the running default (link), and an import queues a change
+    # of default that only takes effect on restart.
+    config["radios"][1]["radio"]["bandwidth"] = 500000
+    config["radio"]["bandwidth"] = 500000
+    config["fabric"]["default_radio"] = "local"
+
+    by_id = {
+        e["id"]: e["radio"]
+        for e in build_radio_status_entries(config, applied=applied, default_radio_id=default_id)
+    }
+
+    assert by_id["link"] == "864.2,500.0,8,8"  # the radio that was retuned
+    assert by_id["local"] == "869.618,62.5,8,8"  # the radio that was not
+
+    # Following the pending default instead gets both radios wrong.
+    naive = {e["id"]: e["radio"] for e in build_radio_status_entries(config, applied=applied)}
+    assert naive["link"] == "864.2,62.5,8,8"
+    assert naive["local"] == "869.618,500.0,8,8"