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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,)
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user