config reload: fix feedback loop — react to modified/created/moved only, not open/close (v3.12.1 spun cpu on read-opens)

fjerkroa_bot/discord_bot.py

@@ -68,11 +68,41 @@ def split_answer(text: str, threshold: int, max_parts: int) -> list:
 
 
 class ConfigFileHandler(FileSystemEventHandler):
-    def __init__(self, on_modified):
-        self._on_modified = on_modified
+    """Rename-safe config watch (CFG-05).
 
+    Editors and tools save atomically — write a temp file, then rename it
+    over the target — which fires a *moved*/*created* event (not
+    *modified*) and swaps the inode, so watching the file directly goes
+    deaf after the first save. We watch the config's *directory* and react
+    to any event whose src or dest path is the config file.
+    """
+
+    def __init__(self, config_path: str, on_change):
+        self._config_path = str(Path(config_path).resolve())
+        self._on_change = on_change
+
+    def _hits_config(self, event) -> bool:
+        for attr in ("src_path", "dest_path"):
+            path = getattr(event, attr, "")
+            if path and str(Path(path).resolve()) == self._config_path:
+                return True
+        return False
+
+    def _dispatch(self, event):
+        if not event.is_directory and self._hits_config(event):
+            self._on_change()
+
+    # Only write/rename events — NOT on_opened/on_closed, whose read-opens
+    # (our own load_config re-reads the file) would otherwise feed back into
+    # a reload loop (CFG-05).
     def on_modified(self, event):
-        self._on_modified(event)
+        self._dispatch(event)
+
+    def on_created(self, event):
+        self._dispatch(event)
+
+    def on_moved(self, event):
+        self._dispatch(event)
 
 
 class FjerkroaBot(commands.Bot):
@@ -102,8 +132,10 @@ class FjerkroaBot(commands.Bot):
 
     def init_observer(self):
         self.observer = Observer()
-        self.file_handler = ConfigFileHandler(self.on_config_file_modified)
-        self.observer.schedule(self.file_handler, path=self.config_file, recursive=False)
+        config_path = Path(self.config_file).resolve()
+        self.file_handler = ConfigFileHandler(str(config_path), self.on_config_file_changed)
+        # Watch the directory, not the file — atomic saves replace the inode (CFG-05)
+        self.observer.schedule(self.file_handler, path=str(config_path.parent), recursive=False)
         self.observer.start()
 
     def init_aichannels(self):
@@ -415,12 +447,10 @@ class FjerkroaBot(commands.Bot):
             airesponder.image_cache.purge_message(str(message.id))  # IMG-14
         await airesponder.observe_event(message.author.name, "delete", f"deleted: {message.content}")
 
-    def on_config_file_modified(self, event):
+    def on_config_file_changed(self):
         # Runs on the watchdog observer thread — the swap itself is
         # scheduled onto the event loop so no request reads a
         # half-swapped config (CFG-04 / D9)
-        if event.src_path != self.config_file:
-            return
         new_config = self.load_config(self.config_file)
         if repr(new_config) == repr(self.config):
             return

specs/SPEC-008-config.md

@@ -31,3 +31,16 @@ and all responder `.config` references is scheduled onto the event
 loop (`call_soon_threadsafe`), so no request ever reads a
 half-swapped config (D9). Before the loop runs (startup), the swap
 applies directly — there are no concurrent readers yet.
+
+### CFG-05 — Hot-reload is rename-safe (coverage: test)
+
+The watcher observes the config file's **directory**, not the file, and
+reacts to a **modified, created, or moved** event whose source or
+destination path is the config file. This catches atomic saves — write
+a temp file, then rename it over the target — which replace the inode
+and fire a move/create rather than a modify; watching the file directly
+would go deaf after the first such save. Open/close events are
+deliberately not handled: reloading re-opens the file to read it, so
+reacting to opens would feed back into an endless reload loop. Events
+for other files in the directory, and directory events themselves, are
+ignored.

tests/test_spec_per.py

@@ -120,9 +120,7 @@ class TestConfigReloadRace(TestBotBase):
         new_config["history-limit"] = 99
         self.bot.load_config = lambda path: new_config
         self.bot.loop = MagicMock()
-        event = MagicMock()
-        event.src_path = self.bot.config_file
-        self.bot.on_config_file_modified(event)
+        self.bot.on_config_file_changed()
         self.bot.loop.call_soon_threadsafe.assert_called_once()
         apply_fn = self.bot.loop.call_soon_threadsafe.call_args.args[0]
         apply_fn()
@@ -137,7 +135,37 @@ class TestConfigReloadRace(TestBotBase):
         loop = MagicMock()
         loop.call_soon_threadsafe.side_effect = RuntimeError("no running loop")
         self.bot.loop = loop
-        event = MagicMock()
-        event.src_path = self.bot.config_file
-        self.bot.on_config_file_modified(event)
+        self.bot.on_config_file_changed()
         self.assertEqual(self.bot.config["history-limit"], 42)
+
+
+class TestConfigReloadRenameSafe(unittest.TestCase):
+    def test_atomic_rename_and_modify_trigger_reload(self):
+        """CFG-05: a modified OR a renamed-into-place config fires the reload; unrelated files do not."""
+        from fjerkroa_bot.discord_bot import ConfigFileHandler
+
+        with tempfile.TemporaryDirectory() as tmp:
+            config = Path(tmp) / "kroa.toml"
+            config.write_text("x = 1\n")
+            hits = []
+            handler = ConfigFileHandler(str(config), lambda: hits.append(1))
+
+            def evt(is_dir=False, src=None, dest=None):
+                event = MagicMock()
+                event.is_directory = is_dir
+                event.src_path = src if src is not None else ""
+                event.dest_path = dest if dest is not None else ""
+                return event
+
+            handler.on_modified(evt(src=str(config)))  # in-place modify
+            handler.on_moved(evt(src=str(Path(tmp) / "kroa.toml.tmp"), dest=str(config)))  # atomic rename over
+            handler.on_created(evt(src=str(config)))  # write-new
+            self.assertEqual(len(hits), 3)
+
+            handler.on_modified(evt(src=str(Path(tmp) / "other.txt")))  # unrelated file
+            handler.on_modified(evt(is_dir=True, src=str(config)))  # directory event
+            self.assertEqual(len(hits), 3)  # neither fired
+
+            # open/close of the config (our own load_config re-reads) must NOT be handled — else a reload loop.
+            self.assertNotIn("on_opened", vars(ConfigFileHandler))
+            self.assertNotIn("on_closed", vars(ConfigFileHandler))