From ba293bb26266bc921e13e50047383182bc4f6e4d Mon Sep 17 00:00:00 2001 From: agessaman Date: Sat, 3 Oct 2026 22:57:26 -0700 Subject: [PATCH] refactor(commands): keep the help TypeError retry for local plugins; fix two doc claims Codex on 495df80e: choosing the help call by signature also dropped the TypeError retry for local plugins written as get_help_text(self, message=None), which used to fall back to generic help when the message call raised, and inspect.signature itself can raise TypeError on odd callables and then take the wrong branch. help_text_for now decides by where get_help_text is defined: bundled commands (defined under modules.) are called once with the message, so an error from their body propagates as it did before 7aba432b; anything else gets exactly the old call-then-retry. Tests pin both: a bundled body's TypeError runs once, a local plugin's is retried without the message. joke and rain stay the accepted residual (no retry for a TypeError inside their help bodies). Codex also found two inaccuracies in b569ed5d's docs: ChannelBridgeBase subclasses must also supply _open_http_session and _init_queues, and an empty [Keywords] format means no reply. --- docs/local-plugins.md | 4 ++-- modules/commands/base_command.py | 23 +++++++++----------- tests/test_help_command.py | 37 +++++++++++++++++++++++++------- 3 files changed, 41 insertions(+), 23 deletions(-) diff --git a/docs/local-plugins.md b/docs/local-plugins.md index c7c8da4..9a0afac 100644 --- a/docs/local-plugins.md +++ b/docs/local-plugins.md @@ -82,7 +82,7 @@ Restart the bot so the service is loaded and started. |---|---| | `enabled_attr = "my_enabled"` | `can_execute` refuses while `self.my_enabled` is false, before any other check. Set the attribute in `__init__`, for example from `self.get_config_value('MyCommand_Command', 'enabled', fallback=True, value_type='bool')`. | | `admin_only = True` | The command always requires admin access, whatever `[Admin_ACL] admin_commands` lists. `requires_admin_access()` is still what callers ask. | -| `keywords_format_key = "mycommand"` | `get_response_format()` returns `[Keywords] mycommand` (surrounding double quotes stripped) when it is set; `handle_keyword_match` then replies with it through `format_response`. | +| `keywords_format_key = "mycommand"` | `get_response_format()` returns `[Keywords] mycommand` (surrounding double quotes stripped). When that is non-empty, `handle_keyword_match` replies with it through `format_response`; unset or empty means no reply. | | `get_help_text(self, message=None)` | The signature every bundled command uses; `help` passes the requesting message so the text can depend on it (DM vs channel). A plugin written as `get_help_text(self)` still works. | | `self.translated_or(key, fallback, kind=list)` | The translation at `key` when it is a non-empty `kind` (a list of greetings, a dict), else your built-in `fallback`. | @@ -94,7 +94,7 @@ Restart the bot so the service is loaded and started. | `await self._cancel_tasks(task_a, task_b)` | Cancels each task (None is skipped) and waits for it; use it in `stop()`. | | `self._subscribe(meshcore, event_type, handler)` / `self._unsubscribe_all()` | Subscribe to meshcore events through `_subscribe` and call `_unsubscribe_all()` in `stop()`: a health restart calls `start()` again, and a subscription left behind would deliver every event twice. | -A service that forwards channel messages to another chat system can subclass `ChannelBridgeBase` (`modules/service_plugins/channel_bridge_utils.py`), which the Discord and Telegram bridges use for start/stop, message intake and the retrying send queue; the subclass supplies `_bridge_mappings`, `_targets_for`, `_deliver`, `_throttled` and `_send_queued`. Files named `*_utils.py` are never loaded as services, so shared code like this can live next to your plugins. +A service that forwards channel messages to another chat system can subclass `ChannelBridgeBase` (`modules/service_plugins/channel_bridge_utils.py`), which the Discord and Telegram bridges use for start/stop, message intake and the retrying send queue; the subclass supplies `_bridge_mappings`, `_open_http_session`, `_init_queues`, `_targets_for`, `_deliver`, `_throttled` and `_send_queued` (the base versions raise `NotImplementedError`). Files named `*_utils.py` are never loaded as services, so shared code like this can live next to your plugins. ## Configuration diff --git a/modules/commands/base_command.py b/modules/commands/base_command.py index 90b577b..92aad60 100644 --- a/modules/commands/base_command.py +++ b/modules/commands/base_command.py @@ -4,7 +4,6 @@ Base command class for all MeshCore Bot commands Provides common functionality and interface for command implementations """ -import inspect import re from abc import ABC, abstractmethod from collections.abc import Callable, Iterator @@ -51,24 +50,22 @@ _CONFIG_VALUE_TYPES = frozenset({'str', 'list', *_CONFIG_TYPED_GETTERS}) def help_text_for(command: Any, message: Any) -> str: - """``command.get_help_text(message)``, or ``get_help_text()`` when it takes no message. + """``command.get_help_text(message)``, retrying without the message for local plugins. - Bundled commands all take the message; a local plugin may implement - ``get_help_text(self)``. The signature decides which call to make, so an - error raised inside a help body propagates instead of being retried. + Every bundled command's get_help_text takes the message, so it is called + once and an error from its body propagates. Anything defined outside + ``modules`` (a local plugin, which may implement ``get_help_text(self)``) + keeps the old contract: a TypeError from the message call is retried + without the message. """ get_help_text = command.get_help_text + defined_in = getattr(get_help_text, "__module__", None) or "" + if defined_in.startswith("modules."): + return get_help_text(message) try: - inspect.signature(get_help_text).bind(message) + return get_help_text(message) except TypeError: return get_help_text() - except ValueError: - # No introspectable signature: try the message form, as before. - try: - return get_help_text(message) - except TypeError: - return get_help_text() - return get_help_text(message) class BaseCommand(ABC): diff --git a/tests/test_help_command.py b/tests/test_help_command.py index cd47657..8243e60 100644 --- a/tests/test_help_command.py +++ b/tests/test_help_command.py @@ -217,21 +217,42 @@ class TestGetSpecificHelp: result = cmd.get_specific_help("foo") assert isinstance(result, str) - def test_error_inside_help_body_is_not_retried(self): - """A TypeError raised by the help body itself propagates after one call.""" + def test_error_inside_bundled_help_body_is_not_retried(self): + """A TypeError raised by a bundled command's help body propagates after one call.""" + from modules.commands.ping_command import PingCommand + + bot = _make_bot() + ping = object.__new__(PingCommand) + calls = [] + + def broken_translate(key, **kwargs): + calls.append(key) + raise TypeError("bug in help body") + + ping.translate = broken_translate + bot.command_manager.commands = {"ping": ping} + cmd = HelpCommand(bot) + with pytest.raises(TypeError): + cmd.get_specific_help("ping", None) + assert len(calls) == 1 + + def test_local_plugin_help_keeps_the_typeerror_retry(self): + """A local get_help_text(message=None) that raises TypeError is retried without it.""" bot = _make_bot() calls = [] - class Broken: + class LocalCommand: def get_help_text(self, message=None): calls.append(message) - raise TypeError("bug in help body") + if message is not None: + raise TypeError("message-specific help failed") + return "Generic help" - bot.command_manager.commands = {"foo": Broken()} + bot.command_manager.commands = {"foo": LocalCommand()} cmd = HelpCommand(bot) - with pytest.raises(TypeError): - cmd.get_specific_help("foo", None) - assert len(calls) == 1 + message = Mock() + cmd.get_specific_help("foo", message) + assert calls == [message, None] def test_unknown_command_returns_unknown_key(self): bot = _make_bot()