fix(services): back off a service that does not start instead of retiring it

Review found that marking a not-running service inactive for good also retired services that only returned early for a transient reason: packet capture and the map uploader give up after waiting 30 s for the radio, and the bridges return when meshcore is momentarily unset, which is exactly when the health loop restarts them. Such a service now records a restart failure, so the next attempt waits out service_restart_backoff_seconds; a transient failure recovers, a missing configuration costs one attempt per backoff instead of one every 5 s. The restart decision is _service_restart_due, which the tests now exercise directly.

Also from the review: packet capture and the map uploader never cleared the should_exit flag stop() sets, so after a restart their background loops exited at once while the service reported healthy; start() now clears it. The Discord and Telegram bridges close the HTTP session they open when they return early because meshcore is not available.

modules/core.py

@@ -433,9 +433,6 @@ class MeshCoreBot:
         # Service plugin restart state (name -> timestamp of last failed restart)
         self._service_restart_failures: dict[str, float] = {}
         self._service_restarting: set = set()
-        # Services whose start() returned without running (missing config, nothing
-        # to do): restarting them cannot help, so the health loop leaves them alone.
-        self._services_inactive: set[str] = set()
 
         # Transport reconnect (serial/BLE/TCP) — lock created when event loop runs
         self._transport_reconnect_lock: asyncio.Lock | None = None
@@ -2402,7 +2399,7 @@ long_jokes = false
         if webhook_service is not None and getattr(webhook_service, 'enabled', False):
             try:
                 await webhook_service.start()
-                if not self._note_if_service_inactive('webhook', webhook_service):
+                if not self._note_if_service_not_running('webhook', webhook_service):
                     self.logger.info("Service 'webhook' started (early, before radio connect)")
             except Exception as e:
                 self.logger.error(f"Failed to start service 'webhook' early: {e}")
@@ -2455,7 +2452,7 @@ long_jokes = false
                 continue
             try:
                 await service_instance.start()
-                if not self._note_if_service_inactive(service_name, service_instance):
+                if not self._note_if_service_not_running(service_name, service_instance):
                     self.logger.info(f"Service '{service_name}' started")
             except Exception as e:
                 self.logger.error(f"Failed to start service '{service_name}': {e}")
@@ -2537,19 +2534,7 @@ long_jokes = false
                     )
                     now = time.time()
                     for name, service in self.services.items():
-                        if not getattr(service, 'enabled', True):
-                            continue
-                        try:
-                            healthy = service.is_healthy()
-                        except Exception:
-                            healthy = False
-                        if healthy:
-                            continue
-                        if name in self._service_restarting or name in self._services_inactive:
-                            continue
-                        if name in self._service_restart_failures and (
-                            now - self._service_restart_failures[name]
-                        ) < restart_backoff:
+                        if not self._service_restart_due(name, service, now, restart_backoff):
                             continue
                         self.logger.warning(
                             f"Service '{name}' unhealthy, attempting restart..."
@@ -2651,21 +2636,41 @@ long_jokes = false
         finally:
             self._shutdown_complete = True
 
-    def _note_if_service_inactive(self, service_name: str, service_instance: Any) -> bool:
-        """Record a service whose start() returned without running; True if so."""
+    def _service_restart_due(self, name: str, service: Any, now: float, backoff: float) -> bool:
+        """Whether the health loop should restart *service* now."""
+        if not getattr(service, 'enabled', True):
+            return False
+        try:
+            if service.is_healthy():
+                return False
+        except Exception:
+            pass
+        if name in self._service_restarting:
+            return False
+        last_failure = self._service_restart_failures.get(name)
+        return last_failure is None or now - last_failure >= backoff
+
+    def _note_if_service_not_running(self, service_name: str, service_instance: Any) -> bool:
+        """Back off a service whose start() returned without running; True if so.
+
+        That happens for missing configuration (Discord or Telegram with no
+        channels, the map uploader with no key) and for transient conditions
+        (the radio not connected yet). Either way, restarting it on every
+        health tick cannot help; after the backoff the health loop tries again,
+        which recovers the transient case.
+        """
         try:
             running = service_instance.is_running()
         except Exception:
             running = True
         if running:
-            self._services_inactive.discard(service_name)
             return False
-        if service_name not in self._services_inactive:
-            self.logger.info(
-                f"Service '{service_name}' did not start (check its configuration); "
-                "it will not be restarted automatically"
-            )
-        self._services_inactive.add(service_name)
+        backoff = self.config.getint('Bot', 'service_restart_backoff_seconds', fallback=300)
+        self.logger.info(
+            f"Service '{service_name}' did not start (check its configuration); "
+            f"next attempt in {backoff}s"
+        )
+        self._service_restart_failures[service_name] = time.time()
         return True
 
     async def _restart_service(self, service_name: str, service_instance: Any) -> bool:
@@ -2676,7 +2681,7 @@ long_jokes = false
         try:
             await service_instance.stop()
             await service_instance.start()
-            if self._note_if_service_inactive(service_name, service_instance):
+            if self._note_if_service_not_running(service_name, service_instance):
                 return False
             if not service_instance.is_healthy():
                 # Restarted but still unhealthy: wait out the backoff before the

modules/service_plugins/discord_bridge_service.py

@@ -338,6 +338,9 @@ class DiscordBridgeService(BaseServicePlugin):
             self.logger.info("Subscribed to CHANNEL_MSG_RECV events")
         else:
             self.logger.error("Cannot subscribe to events - meshcore not available")
+            if self.http_session is not None:
+                await self.http_session.close()
+                self.http_session = None
             return
 
         # Register for bot-sent channel messages so bot responses are bridged too

modules/service_plugins/map_uploader_service.py

@@ -221,6 +221,9 @@ class MapUploaderService(BaseServicePlugin):
             self.logger.info("Map uploader service is disabled")
             return
 
+        # stop() sets this; clear it so a restart's background work actually runs.
+        self.should_exit = False
+
         # Check dependencies
         if not AIOHTTP_AVAILABLE:
             self.logger.error("aiohttp is required for map uploader service. Install with: pip install aiohttp")

modules/service_plugins/packet_capture_service.py

@@ -944,6 +944,9 @@ class PacketCaptureService(BaseServicePlugin):
             self.logger.info("Packet capture service is disabled")
             return
 
+        # stop() sets this; clear it so a restart's background loops actually run.
+        self.should_exit = False
+
         # Wait for bot to be connected (with timeout)
         max_wait = 30  # seconds
         wait_time: float = 0

modules/service_plugins/telegram_bridge_service.py

@@ -239,6 +239,9 @@ class TelegramBridgeService(BaseServicePlugin):
             self.logger.info("Subscribed to CHANNEL_MSG_RECV events")
         else:
             self.logger.error("Cannot subscribe to events - meshcore not available")
+            if self.http_session is not None:
+                await self.http_session.close()
+                self.http_session = None
             return
 
         # Register for bot-sent channel messages so bot responses are bridged too