From 495df80e2f98ecdaac365c16721ef9b44e536524 Mon Sep 17 00:00:00 2001 From: agessaman Date: Sat, 3 Oct 2026 22:43:24 -0700 Subject: [PATCH] refactor(commands): choose the help call by signature instead of retrying on TypeError Codex found that after 7aba432b a TypeError raised inside one of the 40 newly widened help bodies reached the retry in help and command_manager, so the body ran a second time (and could then succeed); before, the no-message call ran it once and the error propagated. Both call sites now go through base_command.help_text_for(command, message), which binds the message against get_help_text's signature first: commands that accept it get get_help_text(message), local plugins written as get_help_text(self) get get_help_text(), and an error from the body propagates after one call. A callable with no introspectable signature keeps the old try-and-retry. Accepted residual difference: joke and rain already took the message, and a TypeError raised inside their help bodies used to be retried without it; it now propagates. help's no-message test now uses a real get_help_text(self) instead of a Mock that raised TypeError, and a new test pins the single call. --- modules/command_manager.py | 7 ++----- modules/commands/base_command.py | 22 ++++++++++++++++++++++ modules/commands/help_command.py | 9 +++------ tests/test_help_command.py | 27 +++++++++++++++++++++++---- 4 files changed, 50 insertions(+), 15 deletions(-) diff --git a/modules/command_manager.py b/modules/command_manager.py index 0c3447d..9427acb 100644 --- a/modules/command_manager.py +++ b/modules/command_manager.py @@ -27,7 +27,7 @@ from .command_prefix import ( from .command_prefix import ( normalize_command_content as normalize_command_content_text, ) -from .commands.base_command import BaseCommand +from .commands.base_command import BaseCommand, help_text_for from .config_validation import ( PUBLIC_CHANNEL_KEY_HEX, # noqa: F401 — re-exported; used by core.py PUBLIC_CHANNEL_OVERRIDE_KEY, @@ -1699,10 +1699,7 @@ class CommandManager: if not command and requested_name: command = self._find_help_command(requested_name.split(maxsplit=1)[0]) if command: - try: - help_text = command.get_help_text(message) - except TypeError: - help_text = command.get_help_text() + help_text = help_text_for(command, message) if hasattr(self.bot, 'translator'): return self.bot.translator.translate('commands.help.specific', command=command_name, help_text=help_text) return f"Help {command_name}: {help_text}" diff --git a/modules/commands/base_command.py b/modules/commands/base_command.py index b6a58da..90b577b 100644 --- a/modules/commands/base_command.py +++ b/modules/commands/base_command.py @@ -4,6 +4,7 @@ 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 @@ -49,6 +50,27 @@ _CONFIG_TYPED_GETTERS = {'bool': 'getboolean', 'int': 'getint', 'float': 'getflo _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. + + 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. + """ + get_help_text = command.get_help_text + try: + inspect.signature(get_help_text).bind(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): """Base class for all bot commands - Plugin Interface. diff --git a/modules/commands/help_command.py b/modules/commands/help_command.py index bcb084c..ee064f7 100644 --- a/modules/commands/help_command.py +++ b/modules/commands/help_command.py @@ -11,7 +11,7 @@ from typing import ( ) from ..models import MeshMessage -from .base_command import BaseCommand +from .base_command import BaseCommand, help_text_for class HelpCommand(BaseCommand): @@ -120,11 +120,8 @@ class HelpCommand(BaseCommand): if command: # Pass message context to get_help_text if the method supports it if hasattr(command, 'get_help_text') and callable(command.get_help_text): - try: - help_text = command.get_help_text(message) - except TypeError: - # Fallback for commands that don't accept message parameter - help_text = command.get_help_text() + # Commands that don't accept the message get the no-argument call + help_text = help_text_for(command, message) else: help_text = self.translate('commands.help.no_help') return self.translate('commands.help.specific', command=command_name, help_text=help_text) diff --git a/tests/test_help_command.py b/tests/test_help_command.py index e37dd9b..cd47657 100644 --- a/tests/test_help_command.py +++ b/tests/test_help_command.py @@ -205,15 +205,34 @@ class TestGetSpecificHelp: assert "commands.help.specific" in result or result != "" def test_known_command_help_text_no_message_param(self): - """Falls back to no-argument get_help_text when TypeError is raised.""" + """A local command whose get_help_text takes no message still gets help.""" bot = _make_bot() - mock_cmd = MagicMock() - mock_cmd.get_help_text = Mock(side_effect=[TypeError("no param"), "Simple help"]) - bot.command_manager.commands = {"foo": mock_cmd} + + class LocalCommand: + def get_help_text(self): + return "Simple help" + + bot.command_manager.commands = {"foo": LocalCommand()} cmd = HelpCommand(bot) 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.""" + bot = _make_bot() + calls = [] + + class Broken: + def get_help_text(self, message=None): + calls.append(message) + raise TypeError("bug in help body") + + bot.command_manager.commands = {"foo": Broken()} + cmd = HelpCommand(bot) + with pytest.raises(TypeError): + cmd.get_specific_help("foo", None) + assert len(calls) == 1 + def test_unknown_command_returns_unknown_key(self): bot = _make_bot() bot.command_manager.commands = {}