Merge pull request #365 from agessaman/fix/all-the-things

Fix/all the things

repeater/config_manager.py

@@ -316,6 +316,14 @@ class ConfigManager:
                         self.daemon.repeater_handler.reload_runtime_config()
                         logger.info("Reloaded RepeaterHandler runtime config")
 
+            # Re-apply login security when the repeater section changed; the
+            # repeater ACL captures repeater.security at registration, so a
+            # saved password change must be pushed to the live ACL explicitly.
+            if "repeater" in sections and self.daemon:
+                login_helper = getattr(self.daemon, "login_helper", None)
+                if login_helper is not None and hasattr(login_helper, "refresh_repeater_security"):
+                    login_helper.refresh_repeater_security(daemon_config)
+
             # Also reload advert_helper config if repeater section changed
             if self.daemon and hasattr(self.daemon, "advert_helper") and self.daemon.advert_helper:
                 if "repeater" in sections:

repeater/handler_helpers/login.py

@@ -33,6 +33,9 @@ class LoginHelper:
 
         self.handlers = {}
         self.acls = {}  # Per-identity ACLs keyed by hash_byte
+        # The repeater identity's ACL, kept so live config updates can re-apply
+        # repeater.security without re-registering the identity.
+        self._repeater_acl = None
         self._pending_tasks = set()
         # Shared across all identities so the node's total anon-reply rate is
         # bounded (mirrors firmware anon_limiter: ~4 requests / 2 min).
@@ -116,6 +119,8 @@ class LoginHelper:
         )
 
         self.acls[hash_byte] = identity_acl
+        if identity_type != "room_server":
+            self._repeater_acl = identity_acl
         logger.info(f"Created ACL for {identity_type} '{name}': hash=0x{hash_byte:02X}")
 
         # Create auth callback that uses this identity's ACL
@@ -180,6 +185,34 @@ class LoginHelper:
 
         logger.info(f"Registered {identity_type} '{name}' login handler: hash=0x{hash_byte:02X}")
 
+    def refresh_repeater_security(self, config: dict = None) -> bool:
+        """Re-apply ``repeater.security`` from config to the live repeater ACL.
+
+        The ACL captures its passwords at registration, so without this a saved
+        password change would only take effect after a restart. Room-server ACLs
+        keep their per-identity ``settings`` passwords and are not touched.
+        """
+        acl = self._repeater_acl
+        if acl is None:
+            return False
+
+        cfg = config if isinstance(config, dict) else self.config
+        security = (cfg or {}).get("repeater", {}).get("security", {}) or {}
+
+        acl.admin_password = security.get("admin_password") or ""
+        acl.guest_password = security.get("guest_password") or ""
+        acl.allow_read_only = bool(security.get("allow_read_only", False))
+        try:
+            acl.max_clients = int(security.get("max_clients", acl.max_clients))
+        except (TypeError, ValueError):
+            logger.warning(
+                "Ignoring invalid repeater.security.max_clients=%r during security refresh",
+                security.get("max_clients"),
+            )
+
+        logger.info("Refreshed repeater ACL security settings from config")
+        return True
+
     def _format_region_names(self) -> str:
         """Build the comma-separated region-names string for an anon regions reply.
 

repeater/handler_helpers/mesh_cli.py

@@ -39,6 +39,28 @@ class MeshCLI:
         self.repeater_config = config.get("repeater", {})
         self.mesh_config = config.setdefault("mesh", {})
 
+    def _save_config_and_apply(self, sections=None) -> bool:
+        """Persist the config, then live-apply the changed sections.
+
+        ``ConfigManager.save_to_file`` returns a bare bool; the tuple form is
+        tolerated for older manager doubles. Live update only runs after a
+        successful save so a failed write never half-applies a change. Pass
+        no sections to stage a change: saved to disk, applied on restart
+        (radio parameters, matching firmware's reboot-to-apply).
+        """
+        result = self.config_manager.save_to_file()
+        saved = result[0] if isinstance(result, tuple) else bool(result)
+        if not saved:
+            return False
+        if sections:
+            self.config_manager.live_update_daemon(sections)
+        return True
+
+    def _get_security_config(self) -> Dict[str, Any]:
+        """Return the repeater login security section (the one LoginHelper reads)."""
+        security = self.repeater_config.get("security")
+        return security if isinstance(security, dict) else {}
+
     def _get_node_name(self) -> str:
         """Return the configured node name, preferring the newer key when present."""
         return self.repeater_config.get("node_name") or self.repeater_config.get("name", "Unknown")
@@ -332,12 +354,12 @@ class MeshCLI:
                 "  set allow.read.only on|off  Read-only access\n"
                 "  set advert.interval <min>   60-240 minutes\n"
                 "  set flood.advert.interval <hr>  3-168 hours\n"
-                "  set flood.max <hops>   Max flood hops (max 64)\n"
+                "  set flood.max <hops>   Max flood hops (max 64; 0 = unlimited, unlike firmware)\n"
                 "  set path.hash.mode <0-2>  Path hash mode (0=1B,1=2B,2=3B)\n"
                 "  set loop.detect <off|minimal|moderate|strict>  Flood loop detection\n"
-                "  set rxdelay <val>      RX delay base (>=0)\n"
-                "  set txdelay <val>      TX delay factor (>=0)\n"
-                "  set direct.txdelay <val>  Direct TX delay (>=0)\n"
+                "  set rxdelay <val>      RX delay base (0-20)\n"
+                "  set txdelay <val>      TX delay factor (0-2)\n"
+                "  set direct.txdelay <val>  Direct TX delay (0-2)\n"
                 "  set multi.acks <n>     Multi-ack count\n"
                 "  set int.thresh <dbm>   Interference threshold\n"
                 "  set agc.reset.interval <n>  AGC reset (rounded to x4)"
@@ -447,19 +469,14 @@ class MeshCLI:
         if not new_password:
             return "Error: Password cannot be empty"
 
-        # Update security config
-        if "security" not in self.config:
-            self.config["security"] = {}
-
-        self.config["security"]["password"] = new_password
+        # LoginHelper authenticates from repeater.security.admin_password.
+        self.repeater_config.setdefault("security", {})["admin_password"] = new_password
 
         # Save config and live update
         try:
-            saved, err = self.config_manager.save_to_file()
-            if not saved:
-                logger.error(f"Failed to save password: {err}")
-                return f"Error: Failed to save config: {err}"
-            self.config_manager.live_update_daemon(["security"])
+            if not self._save_config_and_apply(["repeater"]):
+                logger.error("Failed to save password: config save failed")
+                return "Error: Failed to save config"
             return f"password now: {new_password}"
         except Exception as e:
             logger.error(f"Failed to save password: {e}")
@@ -538,7 +555,7 @@ class MeshCLI:
             return f"> {role}"
 
         elif param == "guest.password":
-            guest_pw = self.config.get("security", {}).get("guest_password", "")
+            guest_pw = self._get_security_config().get("guest_password") or ""
             return f"> {guest_pw}"
 
         elif param == "owner.info":
@@ -546,7 +563,7 @@ class MeshCLI:
             return f"> {owner_info}"
 
         elif param == "allow.read.only":
-            allow = self.config.get("security", {}).get("allow_read_only", False)
+            allow = self._get_security_config().get("allow_read_only", False)
             return f"> {'on' if allow else 'off'}"
 
         elif param == "advert.interval":
@@ -554,7 +571,8 @@ class MeshCLI:
             return f"> {interval}"
 
         elif param == "flood.advert.interval":
-            interval = self.repeater_config.get("flood_advert_interval_hours", 24)
+            # The engine's flood-advert timer consumes send_advert_interval_hours.
+            interval = self.repeater_config.get("send_advert_interval_hours", 10)
             return f"> {interval}"
 
         elif param == "flood.max":
@@ -615,87 +633,95 @@ class MeshCLI:
         try:
             if key == "af":
                 self.repeater_config["airtime_factor"] = float(value)
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater"])
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "name":
                 self._set_node_name(value)
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater"])
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "repeat":
                 self.repeater_config["mode"] = "forward" if value.lower() == "on" else "monitor"
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater"])
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return f"OK - repeat is now {'ON' if self.repeater_config['mode'] == 'forward' else 'OFF'}"
 
             elif key == "lat":
                 self.repeater_config["latitude"] = float(value)
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater"])
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "lon":
                 self.repeater_config["longitude"] = float(value)
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater"])
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "radio":
-                # Format: freq bw sf cr
+                # Format: freq(MHz) bw(kHz) sf cr — the config stores Hz.
                 radio_parts = value.split()
                 if len(radio_parts) != 4:
                     return "Error: Expected freq bw sf cr"
 
-                if "radio" not in self.config:
-                    self.config["radio"] = {}
+                freq_mhz = float(radio_parts[0])
+                bw_khz = float(radio_parts[1])
+                sf = int(radio_parts[2])
+                cr = int(radio_parts[3])
+                if not (
+                    150.0 <= freq_mhz <= 2500.0
+                    and 7.0 <= bw_khz <= 500.0
+                    and 5 <= sf <= 12
+                    and 5 <= cr <= 8
+                ):
+                    return "Error, invalid radio params"
 
-                self.config["radio"]["frequency"] = float(radio_parts[0])
-                self.config["radio"]["bandwidth"] = float(radio_parts[1])
-                self.config["radio"]["spreading_factor"] = int(radio_parts[2])
-                self.config["radio"]["coding_rate"] = int(radio_parts[3])
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["radio"])
+                radio_config = self.config.setdefault("radio", {})
+                radio_config["frequency"] = int(round(freq_mhz * 1_000_000))
+                radio_config["bandwidth"] = int(round(bw_khz * 1_000))
+                radio_config["spreading_factor"] = sf
+                radio_config["coding_rate"] = cr
+                if not self._save_config_and_apply():
+                    return "Error: Failed to save config"
                 return "OK - restart repeater to apply"
 
             elif key == "freq":
-                if "radio" not in self.config:
-                    self.config["radio"] = {}
-                self.config["radio"]["frequency"] = float(value)
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["radio"])
+                # CLI input is MHz (firmware parity); the config stores Hz.
+                freq_mhz = float(value)
+                self.config.setdefault("radio", {})["frequency"] = int(round(freq_mhz * 1_000_000))
+                if not self._save_config_and_apply():
+                    return "Error: Failed to save config"
                 return "OK - restart repeater to apply"
 
             elif key == "tx":
-                if "radio" not in self.config:
-                    self.config["radio"] = {}
-                self.config["radio"]["tx_power"] = int(value)
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["radio"])
-                return "OK"
+                self.config.setdefault("radio", {})["tx_power"] = int(value)
+                if not self._save_config_and_apply():
+                    return "Error: Failed to save config"
+                return "OK - restart repeater to apply"
 
             elif key == "guest.password":
-                if "security" not in self.config:
-                    self.config["security"] = {}
-                self.config["security"]["guest_password"] = value
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["security"])
+                # LoginHelper authenticates from repeater.security.guest_password.
+                self.repeater_config.setdefault("security", {})["guest_password"] = value
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "owner.info":
                 self.repeater_config["owner_info"] = value.replace("|", "\n")
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater"])
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "allow.read.only":
-                if "security" not in self.config:
-                    self.config["security"] = {}
-                self.config["security"]["allow_read_only"] = value.lower() == "on"
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["security"])
+                # LoginHelper reads repeater.security.allow_read_only.
+                self.repeater_config.setdefault("security", {})["allow_read_only"] = (
+                    value.lower() == "on"
+                )
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "advert.interval":
@@ -703,17 +729,18 @@ class MeshCLI:
                 if mins > 0 and (mins < 60 or mins > 240):
                     return "Error: interval range is 60-240 minutes"
                 self.repeater_config["advert_interval_minutes"] = mins
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater"])
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "flood.advert.interval":
                 hours = int(value)
                 if (hours > 0 and hours < 3) or hours > 168:
                     return "Error: interval range is 3-168 hours"
-                self.repeater_config["flood_advert_interval_hours"] = hours
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater"])
+                # The engine's flood-advert timer consumes send_advert_interval_hours.
+                self.repeater_config["send_advert_interval_hours"] = hours
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "flood.max":
@@ -721,8 +748,8 @@ class MeshCLI:
                 if max_val > 64:
                     return "Error: max 64"
                 self.repeater_config["max_flood_hops"] = max_val
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater"])
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "path.hash.mode":
@@ -730,8 +757,8 @@ class MeshCLI:
                 if mode not in (0, 1, 2):
                     return "Error: path.hash.mode must be 0, 1, or 2"
                 self.mesh_config["path_hash_mode"] = mode
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["mesh"])
+                if not self._save_config_and_apply(["mesh"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "loop.detect":
@@ -739,47 +766,47 @@ class MeshCLI:
                 if mode not in ("off", "minimal", "moderate", "strict"):
                     return "Error: loop.detect must be off, minimal, moderate, or strict"
                 self.mesh_config["loop_detect"] = mode
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["mesh"])
+                if not self._save_config_and_apply(["mesh"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "rxdelay":
                 delay = float(value)
-                if delay < 0:
-                    return "Error: cannot be negative"
+                if delay < 0 or delay > 20.0:
+                    return "Error, must be 0-20"
                 self.config.setdefault("delays", {})["rx_delay_base"] = delay
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater", "delays"])
+                if not self._save_config_and_apply(["repeater", "delays"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "txdelay":
                 delay = float(value)
-                if delay < 0:
-                    return "Error: cannot be negative"
+                if delay < 0 or delay > 2.0:
+                    return "Error, must be 0-2"
                 self.config.setdefault("delays", {})["tx_delay_factor"] = delay
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater", "delays"])
+                if not self._save_config_and_apply(["repeater", "delays"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "direct.txdelay":
                 delay = float(value)
-                if delay < 0:
-                    return "Error: cannot be negative"
+                if delay < 0 or delay > 2.0:
+                    return "Error, must be 0-2"
                 self.config.setdefault("delays", {})["direct_tx_delay_factor"] = delay
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater", "delays"])
+                if not self._save_config_and_apply(["repeater", "delays"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "multi.acks":
                 self.repeater_config["multi_acks"] = int(value)
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater"])
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "int.thresh":
                 self.repeater_config["interference_threshold"] = int(value)
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater"])
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return "OK"
 
             elif key == "agc.reset.interval":
@@ -787,8 +814,8 @@ class MeshCLI:
                 # Round to nearest multiple of 4
                 rounded = (interval // 4) * 4
                 self.repeater_config["agc_reset_interval"] = rounded
-                saved, _ = self.config_manager.save_to_file()
-                self.config_manager.live_update_daemon(["repeater"])
+                if not self._save_config_and_apply(["repeater"]):
+                    return "Error: Failed to save config"
                 return f"OK - interval rounded to {rounded}"
 
             else:

repeater/handler_helpers/trace.py

@@ -135,11 +135,16 @@ class TraceHelper:
             logger.info(f"Path SNRs: [{', '.join(path_snrs)}], Hashes: [{', '.join(path_hashes)}]")
 
             should_forward = self._should_forward_trace(packet, trace_bytes, flags, hash_width)
+            mode_blocked = should_forward and not self._relay_allowed()
+            if mode_blocked:
+                should_forward = False
+                logger.info("Trace relay suppressed (repeat disabled)")
 
             if should_forward:
                 await self._forward_trace_packet(packet, num_hops)
             else:
-                self._log_no_forward_reason(packet, trace_bytes, hash_width)
+                if not mode_blocked:
+                    self._log_no_forward_reason(packet, trace_bytes, hash_width)
                 if (
                     self.on_trace_complete
                     and self._is_trace_complete(packet, trace_bytes, hash_width)
@@ -271,6 +276,20 @@ class TraceHelper:
 
         return path_snrs, path_hashes
 
+    def _relay_allowed(self) -> bool:
+        """Firmware gates intermediate-hop TRACE relay on ``allowPacketForward``
+        (Mesh.cpp), so a repeater that is not forwarding (monitor / no_tx mode)
+        must not relay traces. Locally originated pings are injected directly
+        and never pass through this gate.
+        """
+        config = getattr(self.repeater_handler, "config", None)
+        mode = "forward"
+        if isinstance(config, dict):
+            mode = config.get("repeater", {}).get("mode", "forward")
+        if mode not in ("forward", "monitor", "no_tx"):
+            mode = "forward"
+        return mode == "forward"
+
     def _should_forward_trace(
         self, packet, trace_bytes: bytes, flags: int, hash_width: int
     ) -> bool:

repeater/packet_router.py

@@ -273,14 +273,21 @@ class PacketRouter:
             return {}
         return companion_bridges
 
-    async def _fan_out_to_bridges(self, packet, bridges, *, context: str) -> bool:
-        """Offer packet to each bridge; True if any bridge authenticated it.
+    async def _fan_out_to_bridges(self, packet, bridges, *, context: str) -> tuple:
+        """Offer packet to each bridge; report ``(delivered, authenticated)``.
 
         Accepts a dict of bridges — pass a single-entry dict for targeted delivery
         to the bridge that owns ``dest_hash``. A bridge that raises is logged and
         skipped; ``result.authenticated`` is read directly (every bridge returns a
         HandlerResult) so a broken contract surfaces instead of being hidden.
+
+        ``delivered`` is True when at least one bridge completed without raising —
+        the signal callers must use before ``_mark_delivered_to_companions`` so a
+        delivery where every bridge raised is retried on the next copy instead of
+        being suppressed for the dedupe TTL. ``authenticated`` is True when any
+        bridge authenticated the packet.
         """
+        delivered = False
         authenticated = False
         for bridge in bridges.values():
             try:
@@ -288,9 +295,10 @@ class PacketRouter:
             except Exception as e:
                 logger.debug("Companion bridge %s error: %s", context, e)
                 continue
+            delivered = True
             if result.authenticated is True:
                 authenticated = True
-        return authenticated
+        return delivered, authenticated
 
     async def _consume_via_local_candidates(
         self, packet, metadata: dict, dest_hash, helper, process_method_name: str
@@ -316,9 +324,15 @@ class PacketRouter:
 
         consumed = False
         if has_companion:
-            bridge_result = await companion_bridges[dest_hash].process_received_packet(packet)
-            if bridge_result.authenticated:
-                consumed = True
+            # A raising bridge must not abort the candidate loop: the colliding
+            # room-server / repeater identity below still gets offered the packet.
+            try:
+                bridge_result = await companion_bridges[dest_hash].process_received_packet(packet)
+            except Exception as e:
+                logger.debug("Companion bridge candidate error: %s", e)
+            else:
+                if bridge_result.authenticated:
+                    consumed = True
         # Offer to the room-server / repeater identity when it shares the hash
         # (collision) or when no local companion claims it at all (normal
         # server-owned + remote-forward handling).
@@ -632,21 +646,21 @@ class PacketRouter:
             companion_bridges = self._companion_bridges_for_packet(packet, metadata)
             if dest_hash is not None and dest_hash in companion_bridges:
                 if not self._was_delivered_to_companions(packet):
-                    consumed = (
-                        await self._fan_out_to_bridges(
-                            packet, {dest_hash: companion_bridges[dest_hash]}, context="PATH"
-                        )
-                        or consumed
+                    delivered, authenticated = await self._fan_out_to_bridges(
+                        packet, {dest_hash: companion_bridges[dest_hash]}, context="PATH"
                     )
-                    self._mark_delivered_to_companions(packet)
+                    consumed = authenticated or consumed
+                    if delivered:
+                        self._mark_delivered_to_companions(packet)
             elif companion_bridges and not self._was_delivered_to_companions(packet):
                 # Dest not in bridges: path-return with ephemeral dest (e.g. multi-hop login).
                 # Deliver to all bridges; each will try to decrypt and ignore if not relevant.
-                consumed = (
-                    await self._fan_out_to_bridges(packet, companion_bridges, context="PATH")
-                    or consumed
+                delivered, authenticated = await self._fan_out_to_bridges(
+                    packet, companion_bridges, context="PATH"
                 )
-                self._mark_delivered_to_companions(packet)
+                consumed = authenticated or consumed
+                if delivered:
+                    self._mark_delivered_to_companions(packet)
                 logger.debug(
                     "PATH dest=0x%02x (anon) delivered to %d bridge(s) for matching",
                     dest_hash or 0,
@@ -668,13 +682,13 @@ class PacketRouter:
             companion_bridges = self._companion_bridges_for_packet(packet, metadata)
             local_hash = getattr(self.daemon, "local_hash", None)
             if dest_hash is not None and dest_hash in companion_bridges:
-                consumed = await self._fan_out_to_bridges(
+                _, consumed = await self._fan_out_to_bridges(
                     packet, {dest_hash: companion_bridges[dest_hash]}, context="RESPONSE"
                 )
                 logger.info("RESPONSE dest=0x%02x delivered to companion bridge", dest_hash)
             elif dest_hash == local_hash and companion_bridges:
                 # Response addressed to this repeater (e.g. path-based reply to first hop)
-                consumed = await self._fan_out_to_bridges(
+                _, consumed = await self._fan_out_to_bridges(
                     packet, companion_bridges, context="RESPONSE"
                 )
                 logger.info(
@@ -686,7 +700,7 @@ class PacketRouter:
                 # Dest not in bridges and not local: likely ANON_REQ response (dest = ephemeral
                 # sender hash). Deliver to all bridges; each will try to decrypt and ignore if
                 # not relevant (firmware-like behavior, works with multiple companion bridges).
-                consumed = await self._fan_out_to_bridges(
+                _, consumed = await self._fan_out_to_bridges(
                     packet, companion_bridges, context="RESPONSE"
                 )
                 logger.debug(
@@ -711,8 +725,11 @@ class PacketRouter:
             companion_bridges = self._companion_bridges_for_packet(packet, metadata)
             final_hop = _is_direct_final_hop(packet)
             if companion_bridges and (final_hop or not self._was_delivered_to_companions(packet)):
-                await self._fan_out_to_bridges(packet, companion_bridges, context="RESPONSE")
-                self._mark_delivered_to_companions(packet)
+                delivered, _ = await self._fan_out_to_bridges(
+                    packet, companion_bridges, context="RESPONSE"
+                )
+                if delivered:
+                    self._mark_delivered_to_companions(packet)
             if companion_bridges and final_hop:
                 # DIRECT with empty path: we're the final hop, so consume after delivery.
                 processed_by_injection = True

tests/test_handler_helpers_mesh_cli.py

@@ -15,6 +15,7 @@ def _base_config():
             "longitude": 3.4,
             "airtime_factor": 1.1,
             "advert_interval_minutes": 120,
+            # Stale orphan key from the old CLI; nothing must read it.
             "flood_advert_interval_hours": 24,
             "max_flood_hops": 20,
             "rx_delay_base": 0.2,
@@ -23,6 +24,12 @@ def _base_config():
             "multi_acks": 2,
             "interference_threshold": -115,
             "agc_reset_interval": 8,
+            "security": {
+                "admin_password": "adminpw",
+                "guest_password": "guest",
+                "allow_read_only": True,
+                "max_clients": 5,
+            },
         },
         "radio": {
             "frequency": 915000000,
@@ -32,13 +39,14 @@ def _base_config():
             "tx_power": 22,
         },
         "mesh": {"path_hash_mode": 0, "loop_detect": "minimal"},
-        "security": {"guest_password": "guest", "allow_read_only": True},
+        # Decoy: the legacy top-level section the CLI must no longer touch.
+        "security": {"guest_password": "stale-top-level", "allow_read_only": False},
     }
 
 
-def _cfg_mgr(save_ok=True, err=None):
+def _cfg_mgr(save_ok=True):
     return SimpleNamespace(
-        save_to_file=MagicMock(return_value=(save_ok, err)),
+        save_to_file=MagicMock(return_value=save_ok),
         live_update_daemon=MagicMock(),
     )
 
@@ -95,11 +103,15 @@ def test_cmd_password_save_success_failure_and_exception():
 
     assert cli_ok._cmd_password("password   ") == "Error: Password cannot be empty"
     assert cli_ok._cmd_password("password newpw") == "password now: newpw"
-    ok_mgr.live_update_daemon.assert_called_once_with(["security"])
+    # The password must land where LoginHelper reads it, not the legacy top-level section.
+    assert cfg["repeater"]["security"]["admin_password"] == "newpw"
+    assert "password" not in cfg["security"]
+    ok_mgr.live_update_daemon.assert_called_once_with(["repeater"])
 
-    bad_mgr = _cfg_mgr(save_ok=False, err="disk")
+    bad_mgr = _cfg_mgr(save_ok=False)
     cli_bad = MeshCLI("/tmp/cfg.yaml", _base_config(), bad_mgr)
     assert "Failed to save config" in cli_bad._cmd_password("password x")
+    bad_mgr.live_update_daemon.assert_not_called()
 
     ex_mgr = SimpleNamespace(
         save_to_file=MagicMock(side_effect=RuntimeError("boom")),
@@ -178,15 +190,26 @@ def test_cmd_set_updates_and_validation_errors():
     assert cli._cmd_set("repeat off").endswith("OFF")
     assert cfg["repeater"]["mode"] == "monitor"
 
-    assert cli._cmd_set("radio 900000000 250000 9 6").startswith("OK")
-    assert cfg["radio"]["frequency"] == 900000000.0
+    # CLI input is MHz/kHz (firmware parity); the config stores Hz.
+    assert cli._cmd_set("radio 900 250 9 6").startswith("OK")
+    assert cfg["radio"]["frequency"] == 900000000
+    assert cfg["radio"]["bandwidth"] == 250000
+    assert cfg["radio"]["spreading_factor"] == 9
+    assert cfg["radio"]["coding_rate"] == 6
+    assert cli._cmd_set("radio 100 250 9 6") == "Error, invalid radio params"
+    assert cli._cmd_set("radio 900 250 4 6") == "Error, invalid radio params"
 
-    assert cli._cmd_set("freq 868000000").startswith("OK")
-    assert cli._cmd_set("tx 17") == "OK"
+    assert cli._cmd_set("freq 868.5").startswith("OK")
+    assert cfg["radio"]["frequency"] == 868500000
+    assert cli._cmd_set("tx 17") == "OK - restart repeater to apply"
+    assert cfg["radio"]["tx_power"] == 17
     assert cli._cmd_set("guest.password g") == "OK"
+    assert cfg["repeater"]["security"]["guest_password"] == "g"
+    assert cfg["security"]["guest_password"] == "stale-top-level"
     assert cli._cmd_set("owner.info Alice|Ops") == "OK"
     assert cfg["repeater"]["owner_info"] == "Alice\nOps"
     assert cli._cmd_set("allow.read.only off") == "OK"
+    assert cfg["repeater"]["security"]["allow_read_only"] is False
     assert cli._cmd_set("path.hash.mode 2") == "OK"
     assert cfg["mesh"]["path_hash_mode"] == 2
     assert cli._cmd_set("path.hash.mode 3") == "Error: path.hash.mode must be 0, 1, or 2"
@@ -199,10 +222,18 @@ def test_cmd_set_updates_and_validation_errors():
 
     assert cli._cmd_set("advert.interval 59").startswith("Error: interval range")
     assert cli._cmd_set("flood.advert.interval 2").startswith("Error: interval range")
+    assert cli._cmd_set("flood.advert.interval 12") == "OK"
+    # The engine's timer key is updated; the stale orphan key is left alone.
+    assert cfg["repeater"]["send_advert_interval_hours"] == 12
+    assert cfg["repeater"]["flood_advert_interval_hours"] == 24
     assert cli._cmd_set("flood.max 100") == "Error: max 64"
-    assert cli._cmd_set("rxdelay -1") == "Error: cannot be negative"
-    assert cli._cmd_set("txdelay -1") == "Error: cannot be negative"
-    assert cli._cmd_set("direct.txdelay -1") == "Error: cannot be negative"
+    assert cli._cmd_set("rxdelay -1") == "Error, must be 0-20"
+    assert cli._cmd_set("rxdelay 20.5") == "Error, must be 0-20"
+    assert cli._cmd_set("rxdelay 20") == "OK"
+    assert cli._cmd_set("txdelay -1") == "Error, must be 0-2"
+    assert cli._cmd_set("txdelay 2.5") == "Error, must be 0-2"
+    assert cli._cmd_set("direct.txdelay -1") == "Error, must be 0-2"
+    assert cli._cmd_set("direct.txdelay 2.5") == "Error, must be 0-2"
 
     assert cli._cmd_set("agc.reset.interval 10") == "OK - interval rounded to 8"
     assert cli._cmd_set("bad") == "Error: Missing value"
@@ -442,3 +473,136 @@ def test_discovery_auto_add_skips_local_node_and_persists_remote():
     )
     assert remote_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)
+
+    for command in ("af 2.0", "name x", "freq 915", "guest.password g", "flood.max 8"):
+        assert cli._cmd_set(command) == "Error: Failed to save config"
+    mgr.live_update_daemon.assert_not_called()
+
+
+def test_cmd_get_reads_repeater_security_section():
+    cli = MeshCLI("/tmp/cfg.yaml", _base_config(), _cfg_mgr())
+
+    # Values come from repeater.security, not the legacy top-level decoy.
+    assert cli._cmd_get("guest.password") == "> guest"
+    assert cli._cmd_get("allow.read.only") == "> on"
+
+
+def test_cli_password_change_applies_to_live_repeater_acl(tmp_path):
+    """A CLI password change must reach the running LoginHelper ACL, so the
+    next login attempt authenticates against the new password."""
+    from repeater.config_manager import ConfigManager
+    from repeater.handler_helpers.login import LoginHelper
+
+    cfg = _base_config()
+    login_helper = LoginHelper(identity_manager=None, config=cfg)
+    identity = SimpleNamespace(get_public_key=lambda: b"\x42" + b"\x00" * 31)
+    login_helper.register_identity(
+        name="repeater", identity=identity, identity_type="repeater", config=cfg
+    )
+    acl = login_helper.get_acl_for_identity(0x42)
+    assert acl.admin_password == "adminpw"
+    assert acl.guest_password == "guest"
+
+    daemon = SimpleNamespace(config=cfg, login_helper=login_helper)
+    manager = ConfigManager(
+        config_path=str(tmp_path / "config.yaml"), config=cfg, daemon_instance=daemon
+    )
+    cli = MeshCLI(str(tmp_path / "config.yaml"), cfg, manager)
+
+    assert cli._cmd_password("password fresh-secret") == "password now: fresh-secret"
+    assert acl.admin_password == "fresh-secret"
+
+    assert cli._cmd_set("guest.password fresh-guest") == "OK"
+    assert acl.guest_password == "fresh-guest"
+
+    assert cli._cmd_set("allow.read.only off") == "OK"
+    assert acl.allow_read_only is False
+
+
+def test_cmd_set_radio_commands_stage_without_live_apply():
+    """Radio changes match firmware reboot-to-apply: saved to disk, never
+    live-applied (a live retune would cut off the admin mid-session)."""
+    cfg = _base_config()
+    mgr = _cfg_mgr()
+    cli = MeshCLI("/tmp/cfg.yaml", cfg, mgr)
+
+    assert cli._cmd_set("radio 900 250 9 6") == "OK - restart repeater to apply"
+    assert cli._cmd_set("freq 868.5") == "OK - restart repeater to apply"
+    assert cli._cmd_set("tx 17") == "OK - restart repeater to apply"
+
+    assert mgr.save_to_file.call_count == 3
+    mgr.live_update_daemon.assert_not_called()
+
+
+def test_cmd_get_flood_advert_interval_reads_engine_key():
+    cfg = _base_config()
+    cli = MeshCLI("/tmp/cfg.yaml", cfg, _cfg_mgr())
+
+    # The orphan flood_advert_interval_hours (24) must be ignored; the engine
+    # key is absent so the engine default (10) is reported.
+    assert cli._cmd_get("flood.advert.interval") == "> 10"
+
+    cfg["repeater"]["send_advert_interval_hours"] = 6
+    assert cli._cmd_get("flood.advert.interval") == "> 6"
+
+
+def test_cli_set_commands_persist_with_real_config_manager(tmp_path):
+    """Every CLI set command must work against the real ConfigManager bool save
+    contract: reply OK and leave the change on disk."""
+    import yaml
+
+    from repeater.config_manager import ConfigManager
+
+    config_path = tmp_path / "config.yaml"
+    cfg = _base_config()
+    manager = ConfigManager(config_path=str(config_path), config=cfg, daemon_instance=None)
+    cli = MeshCLI(str(config_path), cfg, manager)
+
+    commands = [
+        "af 1.5",
+        "name node-b",
+        "repeat off",
+        "lat 10.5",
+        "lon -3.25",
+        "radio 869.618 250 9 6",
+        "freq 915.125",
+        "tx 17",
+        "guest.password gpw",
+        "owner.info Bob|Lab",
+        "allow.read.only on",
+        "advert.interval 90",
+        "flood.advert.interval 12",
+        "flood.max 8",
+        "path.hash.mode 1",
+        "loop.detect moderate",
+        "rxdelay 4.5",
+        "txdelay 1.5",
+        "direct.txdelay 0.25",
+        "multi.acks 1",
+        "int.thresh -110",
+        "agc.reset.interval 8",
+    ]
+    for command in commands:
+        reply = cli._cmd_set(command)
+        assert reply.startswith("OK"), f"set {command!r} replied {reply!r}"
+
+    assert cli._cmd_password("password newadminpw") == "password now: newadminpw"
+
+    saved = yaml.safe_load(config_path.read_text())
+    assert saved["repeater"]["airtime_factor"] == 1.5
+    assert saved["repeater"]["node_name"] == "node-b"
+    assert saved["repeater"]["mode"] == "monitor"
+    assert saved["radio"]["frequency"] == 915125000
+    assert saved["radio"]["bandwidth"] == 250000
+    assert saved["repeater"]["security"]["guest_password"] == "gpw"
+    assert saved["repeater"]["security"]["admin_password"] == "newadminpw"
+    assert saved["repeater"]["security"]["allow_read_only"] is True
+    assert saved["repeater"]["send_advert_interval_hours"] == 12
+    assert saved["delays"]["rx_delay_base"] == 4.5
+    assert saved["delays"]["tx_delay_factor"] == 1.5
+    assert saved["delays"]["direct_tx_delay_factor"] == 0.25

tests/test_handler_helpers_trace_discovery_login.py

@@ -201,6 +201,70 @@ async def test_trace_helper_forward_trace_packet_updates_recent_record_and_injec
     packet_injector.assert_awaited_once()
 
 
+def _trace_relay_helper(mode, forward=True):
+    """TraceHelper wired so _should_forward_trace says yes; mode comes from config."""
+    repeater_handler = MagicMock()
+    repeater_handler.is_duplicate.return_value = False
+    repeater_handler.calculate_packet_score.return_value = 0.9
+    repeater_handler.config = {"repeater": {"mode": mode}}
+    helper = TraceHelper(
+        local_hash=0x42,
+        local_identity=FakeIdentity(0x42),
+        repeater_handler=repeater_handler,
+    )
+    helper._forward_trace_packet = AsyncMock()
+    helper._extract_path_info = MagicMock(return_value=([], []))
+    helper._should_forward_trace = MagicMock(return_value=forward)
+    helper.trace_handler._parse_trace_payload = MagicMock(
+        return_value={
+            "valid": True,
+            "trace_path_bytes": b"\x42\x43",
+            "flags": 0,
+            "trace_hops": [b"\x42", b"\x43"],
+            "trace_path": [0x42, 0x43],
+            "tag": 1234,
+        }
+    )
+    helper.trace_handler._format_trace_response = MagicMock(return_value="trace ok")
+    return helper
+
+
+@pytest.mark.asyncio
+async def test_trace_relay_suppressed_in_monitor_and_no_tx_mode():
+    # Firmware gates TRACE relay on allowPacketForward, so a repeater with
+    # repeat off must not relay an intermediate-hop trace.
+    for mode in ("monitor", "no_tx"):
+        helper = _trace_relay_helper(mode)
+        packet = DummyPacket(path=b"\x01", payload=b"\xaa\xbb\xcc")
+        await helper.process_trace_packet(packet)
+        helper._forward_trace_packet.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+async def test_trace_relay_allowed_in_forward_and_unknown_mode():
+    for mode in ("forward", "weird-mode"):
+        helper = _trace_relay_helper(mode)
+        packet = DummyPacket(path=b"\x01", payload=b"\xaa\xbb\xcc")
+        await helper.process_trace_packet(packet)
+        helper._forward_trace_packet.assert_awaited_once()
+
+
+@pytest.mark.asyncio
+async def test_trace_ping_response_still_matches_in_monitor_mode():
+    # Ping origination/response handling is not relay: a pending ping must
+    # resolve even when the repeater is not forwarding.
+    helper = _trace_relay_helper("monitor", forward=False)
+    tag = 555
+    helper.trace_handler._parse_trace_payload.return_value["tag"] = tag
+    evt = helper.register_ping(tag, 0x42)
+
+    packet = DummyPacket(path=b"\x01", payload=b"\xaa\xbb\xcc")
+    await helper.process_trace_packet(packet)
+
+    assert evt.is_set()
+    helper._forward_trace_packet.assert_not_awaited()
+
+
 def test_trace_helper_cleanup_stale_pings():
     helper = TraceHelper(
         local_hash=0x42, local_identity=FakeIdentity(0x42), repeater_handler=MagicMock()

tests/test_packet_router.py

@@ -1321,3 +1321,106 @@ class TestInjectedTxRawEcho(unittest.IsolatedAsyncioTestCase):
         ok = await router.inject_packet(_make_packet())
 
         self.assertTrue(ok)
+
+
+class TestCompanionDeliveryFailureHandling(unittest.IsolatedAsyncioTestCase):
+    """Dedupe marking and candidate-loop behaviour when companion bridges raise."""
+
+    async def test_path_all_bridges_raising_is_retried_on_next_copy(self):
+        """A PATH delivery where every bridge raised must not be marked
+        delivered; the next mesh copy gets another delivery attempt."""
+        daemon = _make_daemon()
+        bridge = _make_bridge()
+        bridge.process_received_packet = AsyncMock(side_effect=RuntimeError("bridge down"))
+        daemon.companion_bridges = {0x01: bridge}
+        router = PacketRouter(daemon)
+        pkt = _make_packet(PathHandler.payload_type())
+        pkt.payload = bytes([0x01, 0xAA])
+
+        await router._route_packet(pkt)
+        self.assertEqual(bridge.process_received_packet.await_count, 1)
+
+        # Bridge recovers; the second copy must be delivered, not TTL-suppressed.
+        recovered = AsyncMock(return_value=HandlerResult.not_for_us())
+        bridge.process_received_packet = recovered
+        await router._route_packet(pkt)
+        recovered.assert_awaited_once()
+
+    async def test_path_partial_bridge_failure_still_marks_delivered(self):
+        """One healthy bridge is a delivery: the duplicate copy stays suppressed."""
+        daemon = _make_daemon()
+        raising = _make_bridge()
+        raising.process_received_packet = AsyncMock(side_effect=RuntimeError("boom"))
+        healthy = _make_bridge()
+        healthy.process_received_packet = AsyncMock(return_value=HandlerResult.not_for_us())
+        daemon.companion_bridges = {0x01: raising, 0x02: healthy}
+        router = PacketRouter(daemon)
+        pkt = _make_packet(PathHandler.payload_type())
+        # Dest not in bridges: anon path-return, delivered to all bridges.
+        pkt.payload = bytes([0xEE, 0xAA])
+
+        await router._route_packet(pkt)
+        await router._route_packet(pkt)
+
+        healthy.process_received_packet.assert_awaited_once()
+        raising.process_received_packet.assert_awaited_once()
+
+    async def test_protocol_response_all_bridges_raising_is_retried_on_next_copy(self):
+        daemon = _make_daemon()
+        bridge = _make_bridge()
+        bridge.process_received_packet = AsyncMock(side_effect=RuntimeError("bridge down"))
+        daemon.companion_bridges = {0x01: bridge}
+        router = PacketRouter(daemon)
+        pkt = _make_packet(ProtocolResponseHandler.payload_type())
+        pkt.header = ROUTE_TYPE_FLOOD  # not a final hop: dedupe decides delivery
+
+        with patch("repeater.packet_router.PathHandler.payload_type", return_value=0x55):
+            await router._route_packet(pkt)
+            self.assertEqual(bridge.process_received_packet.await_count, 1)
+
+            recovered = AsyncMock(return_value=HandlerResult.not_for_us())
+            bridge.process_received_packet = recovered
+            await router._route_packet(pkt)
+            recovered.assert_awaited_once()
+
+    async def test_login_candidate_bridge_error_still_offers_local_identity(self):
+        """A raising companion bridge must not abort the candidate loop: the
+        hash-colliding room-server/repeater identity still gets the packet."""
+        daemon = _make_daemon()
+        bridge = _make_bridge()
+        bridge.process_received_packet = AsyncMock(side_effect=RuntimeError("bridge down"))
+        daemon.companion_bridges = {0xF5: bridge}
+        daemon.login_helper = MagicMock()
+        daemon.login_helper.handlers = {0xF5: MagicMock()}
+        daemon.login_helper.process_login_packet = AsyncMock(return_value=True)
+        daemon.repeater_handler = AsyncMock()
+        daemon.repeater_handler.storage = MagicMock()
+        daemon.repeater_handler.record_packet_only = MagicMock()
+        router = PacketRouter(daemon)
+        pkt = _make_packet(LoginServerHandler.payload_type())
+        pkt.payload = bytes([0xF5, 0x01])
+
+        await router._route_packet(pkt)
+
+        bridge.process_received_packet.assert_awaited_once()
+        daemon.login_helper.process_login_packet.assert_awaited_once()
+        # The colliding identity consumed it, so the engine must not re-forward.
+        daemon.repeater_handler.assert_not_awaited()
+
+    async def test_text_candidate_bridge_error_leaves_packet_for_engine(self):
+        """Bridge raises and no local identity claims the text: the packet must
+        still reach the forwarding engine instead of dying with the exception."""
+        daemon = _make_daemon()
+        bridge = _make_bridge()
+        bridge.process_received_packet = AsyncMock(side_effect=RuntimeError("bridge down"))
+        daemon.companion_bridges = {0xF5: bridge}
+        daemon.text_helper = MagicMock()
+        daemon.text_helper.handlers = {}
+        daemon.text_helper.process_text_packet = AsyncMock(return_value=False)
+        router = PacketRouter(daemon)
+        pkt = _make_packet(TextMessageHandler.payload_type())
+        pkt.payload = bytes([0xF5, 0x01])
+
+        await router._route_packet(pkt)
+
+        daemon.repeater_handler.assert_awaited_once()