From d60e7cf1351d58e65b17c84fe4af532be668d5b9 Mon Sep 17 00:00:00 2001 From: agessaman Date: Sat, 3 Oct 2026 23:19:00 -0700 Subject: [PATCH] refactor(commands): tell bundled help methods apart by their defining class, not __module__ Codex on ba293bb2: functools.wraps copies __module__, so a local subclass that wraps PingCommand.get_help_text with a no-message override looked bundled and lost the retry it needs (it raised instead of returning its help), and callables with odd __module__ values could fail or be misclassified. help_text_for now treats a method as bundled only when it is a plain bound method of this command whose function is defined, per the command class's MRO, by a class whose own __module__ is under modules. Everything else (local classes and overrides, partials, Mocks, other callables) gets dev's call-then-retry. New test: a wrapped local override of PingCommand still gets its help. Accepted residual, besides joke and rain: an instance-level wrapper (functools.partial, Mock(wraps=...)) around a bundled get_help_text whose body raises TypeError now runs the body twice, since the bundled method accepts the message. --- modules/commands/base_command.py | 29 ++++++++++++++++++++++------- tests/test_help_command.py | 16 ++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/modules/commands/base_command.py b/modules/commands/base_command.py index 92aad60..f0b4b7f 100644 --- a/modules/commands/base_command.py +++ b/modules/commands/base_command.py @@ -49,18 +49,33 @@ _CONFIG_TYPED_GETTERS = {'bool': 'getboolean', 'int': 'getint', 'float': 'getflo _CONFIG_VALUE_TYPES = frozenset({'str', 'list', *_CONFIG_TYPED_GETTERS}) +def _defined_by_bundled_class(command: Any, method: Any) -> bool: + """Whether ``method`` is a plain bound method from a class defined under ``modules.``. + + Looks the function up in the command class's MRO rather than trusting the + function's own ``__module__``, which ``functools.wraps`` copies. + """ + func = getattr(method, "__func__", None) + if func is None or getattr(method, "__self__", None) is not command: + return False + for klass in type(command).__mro__: + if klass.__dict__.get("get_help_text") is func: + module = klass.__dict__.get("__module__") + return isinstance(module, str) and module.startswith("modules.") + return False + + def help_text_for(command: Any, message: Any) -> str: """``command.get_help_text(message)``, retrying without the message for local plugins. - 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. + Every bundled command's get_help_text takes the message, so when the + method comes from a bundled class it is called once and an error from its + body propagates. Anything else (a local plugin or subclass override, which + may implement ``get_help_text(self)``, or any other callable) 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."): + if _defined_by_bundled_class(command, get_help_text): return get_help_text(message) try: return get_help_text(message) diff --git a/tests/test_help_command.py b/tests/test_help_command.py index 8243e60..6fa2d57 100644 --- a/tests/test_help_command.py +++ b/tests/test_help_command.py @@ -236,6 +236,22 @@ class TestGetSpecificHelp: cmd.get_specific_help("ping", None) assert len(calls) == 1 + def test_wrapped_local_override_of_a_bundled_command_keeps_the_retry(self): + """functools.wraps copies the bundled __module__; the override is still local.""" + import functools + + from modules.commands.ping_command import PingCommand + + class LocalPing(PingCommand): + @functools.wraps(PingCommand.get_help_text) + def get_help_text(self): + return "local help" + + bot = _make_bot() + bot.command_manager.commands = {"ping": object.__new__(LocalPing)} + cmd = HelpCommand(bot) + cmd.get_specific_help("ping", Mock()) # must not raise + 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()