From 4f7224fa969d05485ef076dfb6f9bbbb8ed45ef1 Mon Sep 17 00:00:00 2001 From: bot Date: Wed, 30 Sep 2026 13:00:15 +0000 Subject: [PATCH] fix: resolve issue #284 --- bert_e/reactor.py | 12 +++++++++++ bert_e/templates/incorrect_command_syntax.md | 5 ++++- bert_e/tests/test_bert_e.py | 16 ++++++++++++++ bert_e/tests/unit/test_reactor.py | 22 +++++++++++++++++++- bert_e/workflow/gitwaterflow/__init__.py | 9 +++++++- 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/bert_e/reactor.py b/bert_e/reactor.py index e28ebb6e..936f9197 100644 --- a/bert_e/reactor.py +++ b/bert_e/reactor.py @@ -153,6 +153,13 @@ def __init__(self, keyword: str): self.keyword = keyword +class IncorrectSyntax(Error): + """An option was called with arguments using a wrong syntax.""" + def __init__(self, keyword: str): + super().__init__() + self.keyword = keyword + + class NotFound(Error): """The requested command or option doesn't exist.""" def __init__(self, keyword: str): @@ -445,6 +452,11 @@ def handle_commands(self, job, text, prefix, privileged=False): return raise NotFound(key) if not isinstance(command, Command): + if slash_shorthand and args and not args[0].startswith("="): + # An option followed by free-form arguments (e.g. + # ``/after_pull_request 1509``) is not a valid option + # declaration: options take their argument as ``key=value``. + raise IncorrectSyntax(key) return if command.privileged and not privileged: diff --git a/bert_e/templates/incorrect_command_syntax.md b/bert_e/templates/incorrect_command_syntax.md index d4c1b5e6..fdf74d98 100644 --- a/bert_e/templates/incorrect_command_syntax.md +++ b/bert_e/templates/incorrect_command_syntax.md @@ -10,7 +10,10 @@ It seems that your command syntax is incorrect. The correct usage is: ``` @{{ robot }} option[=argument] ``` - +{% if keyword is defined and keyword %} +Options take their argument after an equal sign, without spaces, e.g. +`/{{ keyword }}=` or `@{{ robot }} {{ keyword }}=`. +{% endif %} Please **edit** or **delete** the corresponding comment so I can move on. {% endblock %} diff --git a/bert_e/tests/test_bert_e.py b/bert_e/tests/test_bert_e.py index 8b6126c5..5007dd15 100644 --- a/bert_e/tests/test_bert_e.py +++ b/bert_e/tests/test_bert_e.py @@ -4358,6 +4358,22 @@ def test_after_pull_request_wrong_syntax(self): with self.assertRaises(exns.IncorrectCommandSyntax): self.handle(blocked_pr.id, options=self.bypass_all, backtrace=True) + def test_after_pull_request_slash_space_syntax(self): + pr_declined = self.create_pr('bugfix/TEST-00002', 'development/4.3') + pr_declined.decline() + blocked_pr = self.create_pr('bugfix/TEST-00003', 'development/4.3') + + # Commands are only read after Bert-E's last message + with self.assertRaises(exns.ApprovalRequired): + self.handle(blocked_pr.id, options=['bypass_jira_check', + 'bypass_build_status'], + backtrace=True) + + blocked_pr.add_comment('/after_pull_request %s' % pr_declined.id) + + with self.assertRaises(exns.IncorrectCommandSyntax): + self.handle(blocked_pr.id, options=self.bypass_all, backtrace=True) + def test_after_pull_request_wrong_pr_id(self): blocked_pr = self.create_pr('bugfix/TEST-00003', 'development/4.3') diff --git a/bert_e/tests/unit/test_reactor.py b/bert_e/tests/unit/test_reactor.py index dd55dc0b..678f959f 100644 --- a/bert_e/tests/unit/test_reactor.py +++ b/bert_e/tests/unit/test_reactor.py @@ -15,7 +15,8 @@ import pytest -from bert_e.reactor import Command, NotFound, NotPrivileged, Option, Reactor +from bert_e.reactor import (Command, IncorrectSyntax, NotFound, NotPrivileged, + Option, Reactor) # All tests are run on a Reactor subclass to avoid sharing state. @@ -391,3 +392,22 @@ def help(job, *args): 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 + + +def test_handle_commands_slash_option_with_space_arg_raises(reactor_cls, job): + """``/after_pull_request 1509`` (space instead of ``=``) must not be + silently ignored.""" + + @reactor_cls.option(default=set()) + def after_pull_request(job, pr_id=None): + pass + + reactor = reactor_cls() + + with pytest.raises(IncorrectSyntax) as exc: + reactor.handle_commands(job, '/after_pull_request 1509', '@bert-e') + assert exc.value.keyword == 'after_pull_request' + + # Valid syntaxes are still fine. + reactor.handle_commands(job, '/after_pull_request', '@bert-e') + reactor.handle_commands(job, '/after_pull_request=1509', '@bert-e') diff --git a/bert_e/workflow/gitwaterflow/__init__.py b/bert_e/workflow/gitwaterflow/__init__.py index 3ad5c435..87fe5c43 100644 --- a/bert_e/workflow/gitwaterflow/__init__.py +++ b/bert_e/workflow/gitwaterflow/__init__.py @@ -23,7 +23,9 @@ from bert_e.job import handler, CommitJob, PullRequestJob, QueuesJob from bert_e.lib.cli import confirm from bert_e.lib.simplecmd import CommandError -from bert_e.reactor import Reactor, NotFound, NotPrivileged, NotAuthored +from bert_e.reactor import ( + Reactor, NotFound, NotPrivileged, NotAuthored, IncorrectSyntax +) from ..git_utils import push, clone_git_repo from ..pr_utils import find_comment, notify_user from .branches import ( @@ -360,6 +362,11 @@ def handle_comments(job): active_options=job.active_options, command=err.keyword, author=author, self_pr=(author == pr_author), comment=text ) from err + except IncorrectSyntax as err: + raise messages.IncorrectCommandSyntax( + robot=job.settings.robot, keyword=err.keyword, + active_options=job.active_options + ) from err def check_commit_diff(job):