fix(config): a bare '%' in a value no longer blocks config reload

_validate_config_snapshot expanded '%' interpolation on every value, so a value read raw with a literal '%' (an [MqttWeather] json_template with "% RH", which config.ini.example documents as fine) failed every reload, although startup accepts the same file. Such values are now returned and named in a warning, and the reload goes ahead. Typed schema keys are still checked as before.

CHANGELOG.md

@@ -39,6 +39,7 @@ semantic versioning.
 
 ### Fixed
 
+- Reloading the config (from the web viewer, the `reload` command or the admin API) no longer fails when any value contains a bare `%`, such as `% RH` in an `[MqttWeather] json_template`, which `config.ini.example` documents as fine. The reload check expanded `%` interpolation on every value, although those values are read raw and startup accepts the same file, so every reload was rejected and changes saved from the web viewer only took effect after a restart. Such values are now named in a warning and the reload goes ahead.
 - The Telegram bridge no longer logs "Failed to stop service" at shutdown when it is enabled without an `api_token`. It disabled itself before setting up the state `stop()` uses, so stopping it raised `AttributeError`. The Discord bridge had the same gap when neither aiohttp nor requests is installed.
 - `trace` no longer gives up on a reply that is on its way. Each attempt listened only for its own tag, so when the first trace's reply arrived after its timeout, the retry had already replaced it and the reply was dropped; on a mesh where repeaters wait a second or more before forwarding, a three-hop trace always failed. A reply to any attempt now counts until the last attempt times out, and one that arrives during the retry delay ends the trace without sending again. The timeout defaults are now 2.0 s base and 1.5 s per hop (were 1.0 s and 0.5 s, about 2.5 s for three hops); a config that sets them keeps its values.
 - Repeat counts for the bot's own messages (the web viewer's command log, the DARC MoWaS retry check) count the bot's own packets. A sent message was matched to the first packet the radio heard within 30 seconds, whatever it was, so on a busy mesh it usually took someone else's packet: its repeats were counted as the bot's, and the bot's real repeats were not. A channel message is now matched only by a packet that decrypts on the same channel to the radio's name and the same text, a DM by a direct message addressed to the recipient from the bot, and a trace by its tag. The repeat counts are written to the database off the event loop, and each repeat no longer adds an entry to Python's import path.

modules/core.py

@@ -981,8 +981,13 @@ class MeshCoreBot:
         return changed
 
     @staticmethod
-    def _validate_config_snapshot(config: configparser.ConfigParser) -> None:
-        """Validate the fully merged candidate before it can be published."""
+    def _validate_config_snapshot(config: configparser.ConfigParser) -> list[str]:
+        """Validate the fully merged candidate before it can be published.
+
+        Returns the options whose values would fail '%' interpolation. They do not
+        block the reload: a literal '%' is legal in values read raw (templates,
+        strftime formats), and startup accepts the same file.
+        """
         from .config_schema import SECTIONS
         from .config_validation import REQUIRED_SECTIONS
 
@@ -992,10 +997,15 @@ class MeshCoreBot:
                 "Missing required configuration section(s): " + ", ".join(missing)
             )
 
-        # Expand interpolation across every final value, not just the base file.
-        # This catches malformed '%' expressions in an overlay before publish.
+        # Expand interpolation across every final value, not just the base file,
+        # so a malformed '%' expression in an overlay is reported before publish.
+        uninterpolatable = []
         for section in config.sections():
-            list(config.items(section))
+            for key in config.options(section):
+                try:
+                    config.get(section, key)
+                except configparser.InterpolationError:
+                    uninterpolatable.append(f"[{section}] {key}")
 
         # Validate all typed keys currently covered by the project schema.
         for section, section_meta in SECTIONS.items():
@@ -1015,6 +1025,7 @@ class MeshCoreBot:
                             f"[{section}] {key} must be one of "
                             f"{', '.join(meta.values)} (got {value!r})"
                         )
+        return uninterpolatable
 
     _COMMAND_CONFIG_STATE = (
         "keywords",
@@ -1059,7 +1070,13 @@ class MeshCoreBot:
                 if not Path(self.config_file).exists():
                     return (False, "Config file not found")
                 new_config, new_local_root = self._read_config_snapshot()
-                self._validate_config_snapshot(new_config)
+                uninterpolatable = self._validate_config_snapshot(new_config)
+                if uninterpolatable:
+                    self.logger.warning(
+                        "Config values with a bare '%%' (fine where they are read raw, such as "
+                        "templates; use '%%%%' elsewhere): %s",
+                        ", ".join(uninterpolatable),
+                    )
 
                 old_radio_settings = self._get_radio_settings(old_config)
                 new_radio_settings = self._get_radio_settings(new_config)

tests/test_config_merge.py

@@ -127,6 +127,32 @@ class TestReloadConfigMerge:
         assert bot.config is old_config
         assert bot.config.get("LocalExtra", "value") == "original"
 
+    def test_literal_percent_in_a_raw_read_value_does_not_block_reload(self, tmp_path):
+        db_path = tmp_path / "bot.db"
+        main_config = tmp_path / "config.ini"
+        template = "{humidity_percent}% RH | {pressure_hpa:.0f} hPa"
+        main_config.write_text(
+            _minimal_main_config(tmp_path, db_path)
+            + f"\n[MqttWeather]\njson_template = {template}\n",
+            encoding="utf-8",
+        )
+        bot = MeshCoreBot(config_file=str(main_config))
+
+        main_config.write_text(
+            _minimal_main_config(tmp_path, db_path)
+            + f"\n[MqttWeather]\njson_template = {template}\n"
+            + "\n[LocalExtra]\nvalue = after_reload\n",
+            encoding="utf-8",
+        )
+        with patch.object(bot.logger, "warning") as warning:
+            success, message = bot.reload_config()
+
+        assert success, message
+        assert bot.config.get("LocalExtra", "value") == "after_reload"
+        assert bot.config.get("MqttWeather", "json_template", raw=True) == template
+        logged = [call.args[0] % call.args[1:] for call in warning.call_args_list]
+        assert any("[MqttWeather] json_template" in line for line in logged)
+
     def test_invalid_typed_value_in_overlay_leaves_snapshot_untouched(self, tmp_path):
         db_path = tmp_path / "bot.db"
         main_config = tmp_path / "config.ini"