diff --git a/bert_e/docs/USER_DOC.md b/bert_e/docs/USER_DOC.md index 54635034..dc4f4fcd 100644 --- a/bert_e/docs/USER_DOC.md +++ b/bert_e/docs/USER_DOC.md @@ -161,6 +161,14 @@ __Bert-E__: | reset | Let __Bert-E__ reset the integration branches associated to the current pull request with a warning if the developer manually modified one of the the integration branches | no | force_reset | Let __Bert-E__ reset the integration branches associated to the current pull request **without warning**. | no +Commands can also be triggered with the ``/`` shorthand at the start +of a comment (e.g. ``/help``). Unknown ``/`` comments are silently +ignored when the keyword doesn't look like a typo of a known command or +option -- that way, comments addressed to other bots (e.g. +``/coderabbit review``) don't trigger an "unknown command" reply. Comments +that explicitly address __Bert-E__ via its ``@`` mention are always +dispatched normally and will still report unknown commands. + Integration branches... ----------------------- __*Bert-E* creates temporary branches during the merge process. These are diff --git a/bert_e/reactor.py b/bert_e/reactor.py index d5720923..e28ebb6e 100644 --- a/bert_e/reactor.py +++ b/bert_e/reactor.py @@ -125,6 +125,7 @@ # Its only non-standard dependencies are the Dispatcher utility mixin, # and the fact that a 'job' must have a 'settings' dictionary-like attribute. +import difflib import logging import re from collections import namedtuple @@ -178,6 +179,9 @@ def normalize_whitespace(msg): return ' '.join(msg.strip().split()) +_CLOSE_MATCH_CUTOFF = 0.6 + + class Reactor(Dispatcher): """Central dispatching class for comment-based commands and options.""" @@ -279,6 +283,19 @@ def init_settings(self, job): for key, option in self.get_options().items(): job.settings[key] = copy(option.default) + def _has_close_match(self, key): + """Return True if ``key`` (case-insensitive) is close enough to any + registered command or option name to plausibly be a typo of it. + + Used on the ``/`` shorthand path to distinguish a plausible typo + (``/apporve`` -> ``/approve``) from a comment addressed to another + bot (``/coderabbit review``): the former should still surface an + "unknown command" reply, the latter should be dropped silently. + """ + known = [k.lower() for k in self.__callbacks__.keys()] + return bool(difflib.get_close_matches( + key.lower(), known, n=1, cutoff=_CLOSE_MATCH_CUTOFF)) + def handle_options(self, job, text, prefix, privileged=False, authored=False): """Find option calls in given text string, and execute the @@ -292,7 +309,10 @@ def handle_options(self, job, text, prefix, privileged=False, The text is ignored if: * the option declaration is actually a command call, - * there is no option declaration in it. + * there is no option declaration in it, + * it uses the ``/`` shorthand with an unknown keyword that isn't + a close match to any registered option (e.g. comments addressed + to another bot such as ``/coderabbit``). Args: job: the job to run the handlers on. @@ -303,7 +323,9 @@ def handle_options(self, job, text, prefix, privileged=False, Raises: NotFound: if the option declaration has the right syntax but calls - an unknown option. + an unknown option (via the ``@`` prefix, or via + the ``/`` shorthand with a keyword close enough to a + registered option to be considered a typo). NotPrivileged: when a privileged option declaration is found and the method is called with privileged=False. NotAuthored: when an authored option declaration is found and the @@ -313,10 +335,12 @@ def handle_options(self, job, text, prefix, privileged=False, raw = text.strip() canonical_raw = None canonical_prefix = None + slash_shorthand = False if raw.startswith(prefix): canonical_raw = raw canonical_prefix = prefix elif re.match(r'^/[\w=]+([\s,.\-:;|+]+/[\w=]+)*\s*$', raw): + slash_shorthand = True canonical_raw = " " + raw canonical_prefix = "" if not canonical_raw: @@ -337,6 +361,11 @@ def handle_options(self, job, text, prefix, privileged=False, key, *args = kwd.split('=') option = self.dispatch(key) if option is None: + if slash_shorthand and not self._has_close_match(key): + LOG.debug( + 'Ignoring unknown /-shorthand option %r ' + '(not close to any registered option)', key) + return raise NotFound(key) if not isinstance(option, Option): if idx == 0: @@ -363,19 +392,24 @@ def handle_commands(self, job, text, prefix, privileged=False): {prefix} command arg1 arg2 ... The text is ignored if: - * the command call is actually an opotion declaration, - * there is no command call in it. + * the command call is actually an option declaration, + * there is no command call in it, + * it uses the ``/`` shorthand with an unknown keyword that isn't + a close match to any registered command (e.g. comments + addressed to another bot such as ``/coderabbit review``). Args: job: the job to run the handlers on. text: the text to look for command calls in. prefix: the prefix of commands. - privileged: run the command handler in privileged mode. Defaults to - False. + privileged: run the command handler in privileged mode. Defaults + to False. Raises: NotFound: if the command call has the right syntax but calls - an unknown command. + an unknown command (via the ``@`` prefix, or via + the ``/`` shorthand with a keyword close enough to a + registered command to be considered a typo). NotPrivileged: when a privileged command call is found and the method is called with privileged=False. @@ -383,10 +417,12 @@ def handle_commands(self, job, text, prefix, privileged=False): raw = text.strip() canonical_raw = None canonical_prefix = None + slash_shorthand = False if raw.startswith(prefix): canonical_raw = raw canonical_prefix = prefix elif re.match(r'^/\w', raw): + slash_shorthand = True canonical_raw = raw.replace("/", "/ ", 1) canonical_prefix = "/" if not canonical_raw: @@ -402,6 +438,11 @@ def handle_commands(self, job, text, prefix, privileged=False): key, args = match.group('command'), match.group('args').split() command = self.dispatch(key) if command is None: + if slash_shorthand and not self._has_close_match(key): + LOG.debug( + 'Ignoring unknown /-shorthand command %r ' + '(not close to any registered command)', key) + return raise NotFound(key) if not isinstance(command, Command): return diff --git a/bert_e/tests/unit/test_reactor.py b/bert_e/tests/unit/test_reactor.py index b3fc9084..dd55dc0b 100644 --- a/bert_e/tests/unit/test_reactor.py +++ b/bert_e/tests/unit/test_reactor.py @@ -283,3 +283,111 @@ def cmd(job, *args): privileged=True) assert call.value.args == ('arg',) + + +def test_handle_commands_slash_shorthand(reactor_cls, job): + """Test the ``/keyword`` shorthand supported by handle_commands.""" + + class CommandCalled(Exception): + def __init__(self, args): + self.args = args + + @reactor_cls.command + def help(job, *args): + raise CommandCalled(args) + + reactor = reactor_cls() + + with pytest.raises(CommandCalled): + reactor.handle_commands(job, '/help', '@bert-e') + + +def test_handle_commands_slash_shorthand_unknown_dropped(reactor_cls, job): + """Unknown ``/keyword`` comments that aren't close to any registered + command or option are dropped silently so foreign-bot mentions don't + produce noisy "unknown command" replies.""" + + @reactor_cls.command + def help(job, *args): + pass + + reactor = reactor_cls() + + # Comments addressed to other bots don't resemble any bert-e command + # and are silently ignored. + reactor.handle_commands(job, '/coderabbit review', '@bert-e') + reactor.handle_commands(job, '/gemini review this PR', '@bert-e') + reactor.handle_commands(job, '/copilot summary', '@bert-e') + + # Same on the options path: the strict options regex matches a bare + # ``/coderabbit`` and previously raised NotFound. With the fuzzy-match + # guard, unknown foreign keywords are dropped silently. + reactor.handle_options(job, '/coderabbit', '@bert-e') + + +def test_handle_commands_slash_shorthand_typo_raises(reactor_cls, job): + """``/keyword`` typos of a registered command still raise NotFound so + the caller can post a "did you mean?" reply.""" + + @reactor_cls.command + def help(job, *args): + pass + + reactor = reactor_cls() + + # ``hlep`` is a close difflib match to ``help`` -> still surfaces as + # an unknown command. + with pytest.raises(NotFound): + reactor.handle_commands(job, '/hlep', '@bert-e') + + +def test_handle_options_slash_shorthand_typo_raises(reactor_cls, job): + """Typos of a registered option on the ``/`` path still raise NotFound.""" + + reactor_cls.add_option('approve', authored=True) + + reactor = reactor_cls() + reactor.init_settings(job) + + with pytest.raises(NotFound): + reactor.handle_options(job, '/apporve', '@bert-e', authored=True) + + +def test_at_prefix_unknown_still_raises(reactor_cls, job): + """Comments that explicitly address the bot with ``@`` never + silently drop unknown commands, even when the keyword isn't close to + any registered one.""" + + @reactor_cls.command + def help(job, *args): + pass + + reactor = reactor_cls() + + # ``@bert-e coderabbit`` is not close to any bert-e command, but because + # the user is explicitly addressing the bot, we still raise NotFound. + with pytest.raises(NotFound): + reactor.handle_commands(job, '@bert-e coderabbit', '@bert-e') + + +def test_reactor_has_close_match(reactor_cls, job): + """Directly exercise the fuzzy-match helper on a controlled registry.""" + + reactor_cls.add_option('approve', authored=True) + + @reactor_cls.command + def help(job, *args): + pass + + reactor = reactor_cls() + + # Typos of registered keywords are recognised (case-insensitive). + assert reactor._has_close_match('apporve') is True + assert reactor._has_close_match('APPORVE') is True + assert reactor._has_close_match('hlep') is True + + # Foreign-bot names bear no resemblance to registered keywords. + assert reactor._has_close_match('coderabbit') is False + assert reactor._has_close_match('gemini') is False + assert reactor._has_close_match('copilot') is False + assert reactor._has_close_match('other-bot-name') is False diff --git a/bert_e/workflow/gitwaterflow/__init__.py b/bert_e/workflow/gitwaterflow/__init__.py index 771ee42e..2fd750a0 100644 --- a/bert_e/workflow/gitwaterflow/__init__.py +++ b/bert_e/workflow/gitwaterflow/__init__.py @@ -314,7 +314,8 @@ def handle_comments(job): authored = author == pr_author text = comment.text try: - reactor.handle_options(job, text, prefix, privileged, authored) + reactor.handle_options( + job, text, prefix, privileged, authored) except NotFound as err: raise messages.UnknownCommand( active_options=job.active_options, command=err.keyword, @@ -344,7 +345,8 @@ def handle_comments(job): privileged = author in admins and author != pr_author text = comment.text try: - reactor.handle_commands(job, text, prefix, privileged) + reactor.handle_commands( + job, text, prefix, privileged) except NotFound as err: raise messages.UnknownCommand( active_options=job.active_options, command=err.keyword,