diff --git a/fjerkroa_bot/discord_bot.py b/fjerkroa_bot/discord_bot.py index f0c54d5..c0ccf44 100644 --- a/fjerkroa_bot/discord_bot.py +++ b/fjerkroa_bot/discord_bot.py @@ -68,11 +68,29 @@ 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). - def on_modified(self, event): - self._on_modified(event) + 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 on_any_event(self, event): + if not event.is_directory and self._hits_config(event): + self._on_change() class FjerkroaBot(commands.Bot): @@ -102,8 +120,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 +435,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 diff --git a/specs/SPEC-008-config.md b/specs/SPEC-008-config.md index 3f3570f..c294be7 100644 --- a/specs/SPEC-008-config.md +++ b/specs/SPEC-008-config.md @@ -31,3 +31,13 @@ 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 any event (modified, created, moved) 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. Events for other files in the +directory, and directory events themselves, are ignored. diff --git a/tests/test_spec_per.py b/tests/test_spec_per.py index 7021ede..dd9d960 100644 --- a/tests/test_spec_per.py +++ b/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,32 @@ 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_any_event(evt(src=str(config))) # in-place modify + handler.on_any_event(evt(src=str(Path(tmp) / "kroa.toml.tmp"), dest=str(config))) # atomic rename over + self.assertEqual(len(hits), 2) + + handler.on_any_event(evt(src=str(Path(tmp) / "other.txt"))) # unrelated file + handler.on_any_event(evt(is_dir=True, src=str(config))) # directory event + self.assertEqual(len(hits), 2) # neither fired