From 4b9f7cd8faf78d4384034c6a4b41627374c74961 Mon Sep 17 00:00:00 2001 From: can1357 Date: Sun, 21 Jun 2026 19:05:31 +0200 Subject: [PATCH] feat(python/robomp): allowed personal repository owners to authorize implementations - Updated configuration to strip the '@' prefix from bot login names. - Granted personal repository owners authorization to trigger implementations regardless of their GitHub author association. --- python/robomp/src/config.py | 2 +- python/robomp/src/github_events.py | 17 +++++++++++++++-- python/robomp/tests/test_config.py | 7 +++++++ python/robomp/tests/test_github_events.py | 22 ++++++++++++++++++++++ 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/python/robomp/src/config.py b/python/robomp/src/config.py index 1498e86af..97c14bf8b 100644 --- a/python/robomp/src/config.py +++ b/python/robomp/src/config.py @@ -163,7 +163,7 @@ class Settings(BaseSettings): @field_validator("bot_login", mode="after") @classmethod def _require_bot_login(cls, value: str) -> str: - cleaned = value.strip() + cleaned = value.strip().removeprefix("@") if not cleaned: raise ValueError("ROBOMP_BOT_LOGIN must be a non-empty GitHub login") return cleaned diff --git a/python/robomp/src/github_events.py b/python/robomp/src/github_events.py index 75007fff2..a4fa57c08 100644 --- a/python/robomp/src/github_events.py +++ b/python/robomp/src/github_events.py @@ -56,6 +56,18 @@ def _repo_full_name(payload: Mapping[str, Any]) -> str | None: return None +def _login_matches_repo_owner(login: str | None, repo: str | None) -> bool: + """Return whether `login` is the personal-account owner in `owner/repo`.""" + if not isinstance(login, str) or not login: + return False + if not isinstance(repo, str): + return False + owner, sep, _name = repo.partition("/") + if not sep or not owner: + return False + return login.lower() == owner.lower() + + PrIssueResolver = Callable[[str, int], str | None] | None @@ -221,13 +233,14 @@ def route( "directive_pragmas": pragmas, "directive_authorizes_impl": False, } - if not is_maintainer(login, assoc, maintainers=maintainers): + repo_owner_matches = _login_matches_repo_owner(login, repo) + if not repo_owner_matches and not is_maintainer(login, assoc, maintainers=maintainers): return {} stripped = extract_mention(body, bot_login) if stripped is None: return {} cleaned, pragmas = parse_pragmas(stripped) - authorizes_impl = is_implementation_authorizer(login, assoc, maintainers=maintainers) + authorizes_impl = repo_owner_matches or is_implementation_authorizer(login, assoc, maintainers=maintainers) return { "directive": True, "directive_body": cleaned, diff --git a/python/robomp/tests/test_config.py b/python/robomp/tests/test_config.py index 5f1f3e627..6761f7998 100644 --- a/python/robomp/tests/test_config.py +++ b/python/robomp/tests/test_config.py @@ -93,6 +93,13 @@ def test_blank_bot_login_rejected(monkeypatch: pytest.MonkeyPatch, env: dict[str Settings() # type: ignore[call-arg] +def test_bot_login_strips_at_prefix(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> None: + monkeypatch.setenv("ROBOMP_BOT_LOGIN", " @roboomp ") + reset_settings_cache() + cfg = Settings() # type: ignore[call-arg] + assert cfg.bot_login == "roboomp" + + def test_model_pool_single(env: dict[str, str]) -> None: cfg = Settings() # type: ignore[call-arg] assert cfg.model_pool == (cfg.model,) diff --git a/python/robomp/tests/test_github_events.py b/python/robomp/tests/test_github_events.py index 67d51f2dc..d90494806 100644 --- a/python/robomp/tests/test_github_events.py +++ b/python/robomp/tests/test_github_events.py @@ -603,6 +603,28 @@ def test_route_directive_set_when_login_in_maintainers_list() -> None: assert decision.directive_authorizes_impl is True +def test_route_directive_authorizes_personal_repo_owner_without_author_association() -> None: + decision = route( + "issue_comment", + { + "action": "created", + "comment": { + "user": {"login": "can1357"}, + # Some delivery paths omit author_association even for the personal-account repo owner. + "body": "@robomp-bot go ahead and push", + }, + "issue": {"number": 9}, + "repository": {"full_name": "can1357/widget"}, + }, + allowlist=frozenset({"can1357/widget"}), + bot_login=BOT, + ) + assert decision.directive is True + assert decision.directive_body == "go ahead and push" + assert decision.directive_author == "can1357" + assert decision.directive_authorizes_impl is True + + def test_route_directive_from_collaborator_does_not_authorize_impl() -> None: decision = route( "issue_comment",