From 14d3c0ca2d73a596fa8a260db43d0bdd730a3adf Mon Sep 17 00:00:00 2001
From: Stacy Olivas <kg7qin@arrl.net>
Date: Tue, 17 Mar 2026 19:56:11 -0700
Subject: [PATCH] refactor: move command aliases to per-command config section

Each command's aliases are now configured as an `aliases` key in its own
config section (e.g. [Wx_Command] aliases = !weather, !w) rather than a
separate [Aliases] section. BaseCommand._load_aliases_from_config() reads
and injects them into keywords at startup. CommandManager.load_aliases()
and _apply_aliases() are removed. No behaviour change for commands without
aliases configured.
---
 BUGS.md                          |  2 +
 TODO.md                          |  6 +-
 config.ini.example               | 22 +-------
 modules/command_manager.py       | 47 ----------------
 modules/commands/base_command.py | 24 ++++++++
 tests/test_command_manager.py    | 97 ++++++++++++--------------------
 6 files changed, 69 insertions(+), 129 deletions(-)

diff --git a/BUGS.md b/BUGS.md
index 3891044..9add941 100644
--- a/BUGS.md
+++ b/BUGS.md
@@ -10,6 +10,8 @@ Tracking of known bugs, fixed issues, and outstanding defects in meshcore-bot.
 
 | Commit | Summary |
 |--------|---------|
+| pending | Refactored command aliases from global `[Aliases]` config section to per-command `aliases =` key in each command's own config section; `BaseCommand._load_aliases_from_config()` reads and injects aliases at startup; `CommandManager.load_aliases()` and `_apply_aliases()` removed |
+| pending | Fixed pre-existing test failure in `test_discord_bridge_multi_webhooks.py`: `ConfigParser` lowercases all config keys so `bridge.Public` is stored as `"public"` — test assertions updated to match actual lowercase key behaviour; runtime matching was already case-insensitive |
 | `26d18c1` | Fixed BUG-029 (third pass): Realtime monitor panels stuck at "Connecting…" — root cause was `<script type="module">` in `realtime.html` creating a second Socket.IO manager that raced with `base.html`'s `forceNew: true` manager; the module-socket's `connect` event never fired. Also: `ping_timeout=5` (5 s) was too short for subscribe handlers that replay DB history; `subscribed_messages` key was missing from `connected_clients` initial dict. Fixed: changed `<script type="module">` to regular `<script>` with dynamic `import()` for the decoder; removed `forceNew: true` from `base.html` so both pages share one Socket.IO manager; raised `ping_timeout` 5→20 s; added `subscribed_messages: False` to client tracking dict. |
 | `26d18c1` | Fixed BUG-029 (second pass): `config_base` was a local variable — stored as `self._config_base` instance attribute; removed dead `_get_db_path()` method that still used `self.bot_root`; fixed `subscribe_logs` and `_start_log_tailing` to resolve log file path via `self._config_base` instead of `self.bot_root`; fixed misleading hardcoded "Connected"/"Active" status badges in `realtime.html` (now start as "Connecting…" and update dynamically on actual SocketIO connect). |
 | `26d18c1` | Fixed BUG-029 (first pass): `app.py` resolved `db_path` relative to the code root (2 dirs above `app.py`) instead of relative to the config file's parent directory, causing the web viewer and bot to open different database files. Fixed: `config_base = Path(config_path).parent.resolve()` used as base for `resolve_path()`; also elevated subscribe-handler replay errors from DEBUG to WARNING and added INFO log of resolved db_path on startup. 4 new tests in `TestDbPathResolutionFromConfigDir`. |
diff --git a/TODO.md b/TODO.md
index 54f3b9a..69f6e48 100644
--- a/TODO.md
+++ b/TODO.md
@@ -3,7 +3,7 @@
 Task list for meshcore-bot development. Auto-updated sections are regenerated
 by running `python scripts/update_todos.py` (see [Auto-Update](#auto-update)).
 
-**Last updated:** 2026-03-16 — coverage at 36.72% (2,140 passed / 29 skipped); `fail_under=35`; target 40%
+**Last updated:** 2026-03-17 — coverage at 36.72% (2,140 passed / 29 skipped); `fail_under=35`; target 40%; 22 PR branches pushed to KG7QIN fork
 
 ---
 
@@ -176,6 +176,10 @@ by running `python scripts/update_todos.py` (see [Auto-Update](#auto-update)).
 
 ## Recently Completed
 
+- [x] (2026-03-17) **PR split** — 22 logical PR branches created from `dev-kg7qin-changes` commits and pushed to `KG7QIN/meshcore-bot`; stacked on `pr-base` (upstream/dev + 2 catch-up commits); each targets `agessaman/meshcore-bot:dev`
+- [x] (2026-03-17) **Alias refactor** — aliases moved from global `[Aliases]` config section to per-command `aliases =` key in each command's own section; loaded by `BaseCommand._load_aliases_from_config()` at startup; `CommandManager.load_aliases()` and `_apply_aliases()` removed
+- [x] (2026-03-17) **Discord bridge test fix** — `test_discord_bridge_multi_webhooks.py` assertions corrected: `ConfigParser` lowercases all keys so `bridge.Public` stores as `"public"`; test expectations updated to match actual (correct) lowercase key behaviour
+
 - [x] (2026-03-15) Radio firmware config UI — Migration 6 (`payload_data`); `firmware_read`/`firmware_write` op types; `POST /api/radio/firmware/config/read|write`; Firmware Configuration card (path.hash.mode + loop.detect)
 - [x] (2026-03-15) APScheduler migration — `BackgroundScheduler` + `CronTrigger`; removes `schedule` lib dependency
 - [x] (2026-03-15) Rate-limiter observability — `GET /api/stats/rate_limiters`; all 4 limiter types exposed
diff --git a/config.ini.example b/config.ini.example
index 710ce1f..9c86fcf 100644
--- a/config.ini.example
+++ b/config.ini.example
@@ -386,26 +386,9 @@ enabled = true
 # dm_only = true: only respond to direct messages (recommended)
 # dm_only = false: allow in monitored channels
 dm_only = true
+# aliases = comma-separated list of additional trigger words for this command
+# aliases = !s, !sched
 
-# ---------------------------------------------------------------------------
-# Command aliases
-# ---------------------------------------------------------------------------
-# Map short aliases to canonical command names.
-# Format:  <alias> = <command_name>
-# Both the alias and command name are case-insensitive.
-# The alias is injected into the command's keyword list at startup, so it
-# responds exactly the same as typing the full command name.
-# Unknown aliases (command not loaded) are logged and silently ignored.
-#
-# Examples:
-#   s  = schedule
-#   wx = weather
-#   h  = help
-#   e  = earthquake
-#
-# [Aliases]
-# s  = schedule
-# wx = weather
 
 [Logging]
 # Log level: DEBUG, INFO, WARNING, ERROR, CRITICAL
@@ -1122,6 +1105,7 @@ url_timeout = 10
 [Wx_Command]
 enabled = true
 # channels =
+# aliases = comma-separated additional trigger words, e.g.: aliases = !weather, !w
 
 [WebViewer_Command]
 enabled = true
diff --git a/modules/command_manager.py b/modules/command_manager.py
index 31f7cc3..1b5394c 100644
--- a/modules/command_manager.py
+++ b/modules/command_manager.py
@@ -95,10 +95,6 @@ class CommandManager:
         self.plugin_loader = PluginLoader(bot, local_commands_dir=local_commands_dir)
         self.commands = self.plugin_loader.load_all_plugins()
 
-        # Load aliases and inject them into command keyword lists
-        self.aliases = self.load_aliases()
-        self._apply_aliases()
-
         # Cache for internet connectivity status to avoid checking on every command
         # Thread-safe cache with asyncio.Lock
         self._internet_cache = InternetStatusCache(has_internet=True, timestamp=0)
@@ -500,49 +496,6 @@ class CommandManager:
         prefix = self.bot.config.get('Bot', 'command_prefix', fallback='')
         return prefix.strip() if prefix else ''
 
-    def load_aliases(self) -> dict[str, str]:
-        """Load command aliases from the ``[Aliases]`` config section.
-
-        Each entry maps a short alias to a canonical command name, e.g.::
-
-            [Aliases]
-            s  = schedule
-            wx = weather
-
-        Returns:
-            Dict[str, str]: alias → canonical command name (both lowercase).
-        """
-        aliases: dict[str, str] = {}
-        if not self.bot.config.has_section('Aliases'):
-            return aliases
-        for alias, canonical in self.bot.config.items('Aliases'):
-            alias = alias.strip().lower()
-            canonical = canonical.strip().lower()
-            if alias and canonical:
-                aliases[alias] = canonical
-        return aliases
-
-    def _apply_aliases(self) -> None:
-        """Inject each alias into the keyword list of its canonical command.
-
-        This lets the existing ``matches_keyword()`` dispatch handle aliases
-        transparently — no changes needed in ``check_keywords()``.
-
-        Unknown aliases (pointing to commands that are not loaded) are logged
-        and skipped.
-        """
-        for alias, canonical in self.aliases.items():
-            command = self.commands.get(canonical)
-            if command is None:
-                self.logger.warning(
-                    f"Alias '{alias}' → '{canonical}' skipped: "
-                    f"command '{canonical}' is not loaded"
-                )
-                continue
-            if alias not in [k.lower() for k in command.keywords]:
-                command.keywords.append(alias)
-                self.logger.debug(f"Alias '{alias}' → '{canonical}' registered")
-
     def format_keyword_response(self, response_format: str, message: MeshMessage) -> str:
         """Format a keyword response string with message data.
 
diff --git a/modules/commands/base_command.py b/modules/commands/base_command.py
index b01326a..625acc5 100644
--- a/modules/commands/base_command.py
+++ b/modules/commands/base_command.py
@@ -48,6 +48,9 @@ class BaseCommand(ABC):
         # Load allowed channels from config (standardized channel override)
         self.allowed_channels = self._load_allowed_channels()
 
+        # Load aliases from this command's config section and extend keywords
+        self._load_aliases_from_config()
+
         # Load translated keywords after initialization
         self._load_translated_keywords()
 
@@ -356,6 +359,27 @@ class BaseCommand(ABC):
         channels = [ch.strip() for ch in channels_str.split(',') if ch.strip()]
         return channels if channels else None
 
+    def _load_aliases_from_config(self) -> None:
+        """Load aliases from this command's own config section and extend keywords.
+
+        Config format::
+
+            [Wx_Command]
+            aliases = !weather, !w
+
+        Each alias is appended to ``self.keywords`` if not already present.
+        """
+        section_name = self._derive_config_section_name()
+        aliases_str = self.get_config_value(section_name, 'aliases', fallback=None, value_type='str')
+        if not aliases_str:
+            return
+        for alias in aliases_str.split(','):
+            alias = alias.strip().lower()
+            if alias and alias not in [k.lower() for k in self.keywords]:
+                self.keywords = list(self.keywords)  # ensure instance-level list
+                self.keywords.append(alias)
+                self.logger.debug(f"Alias '{alias}' registered for command '{self.name}'")
+
     def is_channel_allowed(self, message: MeshMessage) -> bool:
         """Check if this command is allowed in the message's channel.
 
diff --git a/tests/test_command_manager.py b/tests/test_command_manager.py
index 906579a..04f18da 100644
--- a/tests/test_command_manager.py
+++ b/tests/test_command_manager.py
@@ -452,81 +452,54 @@ class TestSendChannelMessagesChunked:
 
 
 # ---------------------------------------------------------------------------
-# TestLoadAliases
+# TestCommandAliases (per-command config)
 # ---------------------------------------------------------------------------
 
 
-class TestLoadAliases:
-    """Tests for load_aliases() config parsing."""
+class TestCommandAliases:
+    """Tests for per-command aliases via BaseCommand._load_aliases_from_config()."""
 
-    def test_empty_when_no_section(self, cm_bot):
-        manager = make_manager(cm_bot)
-        assert manager.aliases == {}
+    def _make_command(self, bot, section, aliases_value=None):
+        """Create a minimal concrete BaseCommand subclass with aliases config."""
+        if not bot.config.has_section(section):
+            bot.config.add_section(section)
+        if aliases_value is not None:
+            bot.config.set(section, "aliases", aliases_value)
 
-    def test_reads_alias_entries(self, cm_bot):
-        cm_bot.config.add_section("Aliases")
-        cm_bot.config.set("Aliases", "s", "schedule")
-        cm_bot.config.set("Aliases", "wx", "weather")
-        manager = make_manager(cm_bot)
-        assert manager.aliases == {"s": "schedule", "wx": "weather"}
+        from modules.commands.base_command import BaseCommand
 
-    def test_aliases_are_lowercased(self, cm_bot):
-        cm_bot.config.add_section("Aliases")
-        cm_bot.config.set("Aliases", "S", "Schedule")
-        manager = make_manager(cm_bot)
-        assert "s" in manager.aliases
-        assert manager.aliases["s"] == "schedule"
+        class _Cmd(BaseCommand):
+            name = section.lower().replace("_command", "")
+            keywords: list = [name]
+            description = "test"
 
-    def test_empty_alias_key_ignored(self, cm_bot):
-        # ConfigParser won't allow a truly empty key, so this tests whitespace values
-        cm_bot.config.add_section("Aliases")
-        cm_bot.config.set("Aliases", "wx", "")  # empty canonical
-        manager = make_manager(cm_bot)
-        assert "wx" not in manager.aliases
+            async def execute(self, message):  # type: ignore[override]
+                return True
 
+        return _Cmd(bot)
 
-# ---------------------------------------------------------------------------
-# TestApplyAliases
-# ---------------------------------------------------------------------------
+    def test_alias_added_to_keywords(self, cm_bot):
+        cmd = self._make_command(cm_bot, "Schedule_Command", "!s, !sched")
+        assert "!s" in cmd.keywords
+        assert "!sched" in cmd.keywords
 
+    def test_no_aliases_key_leaves_keywords_unchanged(self, cm_bot):
+        cmd = self._make_command(cm_bot, "Schedule_Command")
+        assert cmd.keywords == ["schedule"]
 
-class TestApplyAliases:
-    """Tests for _apply_aliases() keyword injection."""
+    def test_empty_aliases_value_leaves_keywords_unchanged(self, cm_bot):
+        cmd = self._make_command(cm_bot, "Schedule_Command", "")
+        assert cmd.keywords == ["schedule"]
 
-    def _make_mock_command(self, name, keywords):
-        cmd = Mock()
-        cmd.name = name
-        cmd.keywords = list(keywords)
-        return cmd
+    def test_alias_already_present_not_duplicated(self, cm_bot):
+        cmd = self._make_command(cm_bot, "Schedule_Command", "schedule, !s")
+        assert cmd.keywords.count("schedule") == 1
+        assert "!s" in cmd.keywords
 
-    def test_alias_injected_into_command_keywords(self, cm_bot):
-        sched_cmd = self._make_mock_command("schedule", ["schedule"])
-        cm_bot.config.add_section("Aliases")
-        cm_bot.config.set("Aliases", "s", "schedule")
-        make_manager(cm_bot, commands={"schedule": sched_cmd})
-        assert "s" in sched_cmd.keywords
-
-    def test_unknown_alias_logs_warning_and_skipped(self, cm_bot):
-        cm_bot.config.add_section("Aliases")
-        cm_bot.config.set("Aliases", "x", "nonexistent")
-        make_manager(cm_bot)
-        cm_bot.logger.warning.assert_called()
-
-    def test_duplicate_alias_not_added_twice(self, cm_bot):
-        sched_cmd = self._make_mock_command("schedule", ["schedule", "s"])
-        cm_bot.config.add_section("Aliases")
-        cm_bot.config.set("Aliases", "s", "schedule")
-        make_manager(cm_bot, commands={"schedule": sched_cmd})
-        assert sched_cmd.keywords.count("s") == 1
-
-    def test_multiple_aliases_for_same_command(self, cm_bot):
-        wx_cmd = self._make_mock_command("weather", ["weather"])
-        cm_bot.config.add_section("Aliases")
-        cm_bot.config.set("Aliases", "wx", "weather")
-        cm_bot.config.set("Aliases", "w", "weather")
-        make_manager(cm_bot, commands={"weather": wx_cmd})
-        assert "wx" in wx_cmd.keywords
-        assert "w" in wx_cmd.keywords
+    def test_aliases_lowercased(self, cm_bot):
+        cmd = self._make_command(cm_bot, "Schedule_Command", "!S, !Sched")
+        assert "!s" in cmd.keywords
+        assert "!sched" in cmd.keywords
 
 
 class TestSendChannelMessageRetry: