Skip to content

Commit 58c4c52

Browse files
committed
feat: auto-trigger commands when bot is assigned as reviewer on GitLab MR
GitLab provides built-in commands like /request_review and /assign_reviewer which currently do not trigger the PR-Agent. This PR adds webhook handling for reviewer assignment events to automatically trigger commands (default: /review). Features: - Adds handle_reviewer_assignment toggle under [gitlab] section (default: false) - Supports configurable reviewer_commands array (default: ['/review']) - Bot identifies itself dynamically using the GitLab API - Safely skips draft MRs and invalid events - Validates configurations and payload structures to avoid exceptions Assisted-by: opencode:deepseek-v4-pro
1 parent 133a196 commit 58c4c52

2 files changed

Lines changed: 270 additions & 25 deletions

File tree

pr_agent/servers/gitlab_webhook.py

Lines changed: 91 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
import asyncio
12
import copy
3+
import hashlib
24
import json
35
import os
46
import re
@@ -108,38 +110,70 @@ def is_draft_ready(data) -> bool:
108110
get_logger().error(f"Failed 'is_draft_ready' logic: {e}")
109111
return False
110112

111-
_bot_user_id_cache = None
113+
_bot_user_id_cache = {}
112114

113-
def _get_bot_user_id():
114-
global _bot_user_id_cache
115-
if _bot_user_id_cache is not None:
116-
return _bot_user_id_cache
117-
try:
115+
async def _get_bot_user_id():
116+
gitlab_url = get_settings().get("GITLAB.URL", "https://gitlab.com")
117+
gitlab_token = get_settings().get("GITLAB.PERSONAL_ACCESS_TOKEN", None)
118+
if not gitlab_token:
119+
get_logger().error("No GitLab token available for bot user ID resolution")
120+
return None
121+
122+
cache_key = hashlib.sha256(f"{gitlab_url}:{gitlab_token}".encode()).hexdigest()
123+
124+
cached = _bot_user_id_cache.get(cache_key)
125+
if cached is not None:
126+
return cached if cached != -1 else None
127+
128+
def _resolve_sync():
118129
import gitlab
119-
gl = gitlab.Gitlab(
120-
get_settings().get("GITLAB.URL", "https://gitlab.com"),
121-
private_token=get_settings().get("GITLAB.PERSONAL_ACCESS_TOKEN", None),
122-
)
130+
131+
ssl_verify = get_settings().get("GITLAB.SSL_VERIFY", True)
132+
if isinstance(ssl_verify, str):
133+
ssl_verify = ssl_verify.lower() in ("true", "1", "yes")
134+
135+
auth_method = get_settings().get("GITLAB.AUTH_TYPE", "oauth_token")
136+
if auth_method not in ("oauth_token", "private_token"):
137+
auth_method = "oauth_token"
138+
139+
kwargs = {"url": gitlab_url, "ssl_verify": ssl_verify}
140+
if auth_method == "oauth_token":
141+
kwargs["oauth_token"] = gitlab_token
142+
else:
143+
kwargs["private_token"] = gitlab_token
144+
145+
gl = gitlab.Gitlab(**kwargs)
123146
gl.auth()
124-
_bot_user_id_cache = gl.user.id
125-
get_logger().info(f"Bot user ID resolved via API: {_bot_user_id_cache}")
126-
return _bot_user_id_cache
147+
return gl.user.id
148+
149+
try:
150+
user_id = await asyncio.to_thread(_resolve_sync)
151+
if len(_bot_user_id_cache) > 1000:
152+
_bot_user_id_cache.clear()
153+
_bot_user_id_cache[cache_key] = user_id
154+
get_logger().info(f"Bot user ID resolved via API: {user_id}")
155+
return user_id
127156
except Exception as e:
128157
get_logger().error(f"Failed to resolve bot user ID: {e}")
129158
return None
130159

131-
def is_bot_assigned_as_reviewer(data) -> bool:
160+
async def is_bot_assigned_as_reviewer(data) -> bool:
132161
try:
133-
if 'reviewers' not in data.get('changes', {}):
162+
changes = data.get("changes")
163+
if not isinstance(changes, dict):
134164
return False
135-
reviewers_change = data['changes']['reviewers']
136-
previous = reviewers_change.get('previous', [])
137-
current = reviewers_change.get('current', [])
138-
bot_user_id = _get_bot_user_id()
165+
if "reviewers" not in changes:
166+
return False
167+
reviewers_change = changes["reviewers"]
168+
if not isinstance(reviewers_change, dict):
169+
return False
170+
previous = reviewers_change.get("previous", [])
171+
current = reviewers_change.get("current", [])
172+
bot_user_id = await _get_bot_user_id()
139173
if bot_user_id is None:
140174
return False
141-
previous_ids = {r.get('id') for r in previous} if isinstance(previous, list) else set()
142-
current_ids = {r.get('id') for r in current} if isinstance(current, list) else set()
175+
previous_ids = {r.get("id") for r in previous if isinstance(r, dict)}
176+
current_ids = {r.get("id") for r in current if isinstance(r, dict)}
143177
return bot_user_id in current_ids and bot_user_id not in previous_ids
144178
except Exception as e:
145179
get_logger().error(f"Failed 'is_bot_assigned_as_reviewer' logic: {e}")
@@ -254,7 +288,9 @@ async def inner(data: dict):
254288
# ignore MRs based on title, labels, source and target branches
255289
if not should_process_pr_logic(data):
256290
return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"}))
257-
object_attributes = data.get('object_attributes', {})
291+
object_attributes = data.get('object_attributes')
292+
if not isinstance(object_attributes, dict):
293+
object_attributes = {}
258294
if object_attributes.get('action') in ['open', 'reopen']:
259295
url = object_attributes.get('url')
260296
get_logger().info(f"New merge request: {url}")
@@ -284,7 +320,7 @@ async def inner(data: dict):
284320

285321
get_logger().debug(f'A push event has been received: {url}')
286322
await _perform_commands_gitlab("push_commands", PRAgent(), url, log_context, data)
287-
323+
288324
# for draft to ready triggered merge requests
289325
elif object_attributes.get('action') == 'update' and is_draft_ready(data):
290326
url = object_attributes.get('url')
@@ -294,10 +330,40 @@ async def inner(data: dict):
294330
await _perform_commands_gitlab("pr_commands", PRAgent(), url, log_context, data)
295331

296332
# for reviewer assignment triggered merge requests
297-
elif object_attributes.get('action') == 'update' and is_bot_assigned_as_reviewer(data):
333+
elif object_attributes.get('action') == 'update' and not object_attributes.get('oldrev'):
298334
url = object_attributes.get('url')
335+
if not url:
336+
return JSONResponse(status_code=status.HTTP_200_OK,
337+
content=jsonable_encoder({"message": "success"}))
338+
339+
# Fast early-exit: no reviewer changes means nothing to do
340+
changes = data.get("changes")
341+
if not isinstance(changes, dict) or "reviewers" not in changes:
342+
return JSONResponse(status_code=status.HTTP_200_OK,
343+
content=jsonable_encoder({"message": "success"}))
344+
299345
apply_repo_settings(url)
300-
if get_settings().gitlab.get('handle_reviewer_assignment', False):
346+
handle_assignment = get_settings().gitlab.get("handle_reviewer_assignment", False)
347+
if isinstance(handle_assignment, str):
348+
handle_assignment = handle_assignment.lower() in ("true", "1", "yes")
349+
if not handle_assignment:
350+
return JSONResponse(status_code=status.HTTP_200_OK,
351+
content=jsonable_encoder({"message": "success"}))
352+
353+
# Check PR logic after applying repo settings
354+
if not should_process_pr_logic(data):
355+
return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"}))
356+
357+
if is_draft(data):
358+
get_logger().info(f"Skipping draft MR reviewer assignment: {url}")
359+
return JSONResponse(status_code=status.HTTP_200_OK,
360+
content=jsonable_encoder({"message": "success"}))
361+
if await is_bot_assigned_as_reviewer(data):
362+
reviewer_commands = get_settings().gitlab.get("reviewer_commands", [])
363+
if not isinstance(reviewer_commands, list) or not all(isinstance(c, str) for c in reviewer_commands):
364+
get_logger().warning("gitlab.reviewer_commands is not a list of strings, skipping")
365+
return JSONResponse(status_code=status.HTTP_200_OK,
366+
content=jsonable_encoder({"message": "success"}))
301367
get_logger().info(f"Bot was assigned as reviewer on MR: {url}")
302368
await _perform_commands_gitlab("reviewer_commands", PRAgent(), url, log_context, data)
303369

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
import asyncio
2+
from unittest import mock
3+
import pytest
4+
5+
6+
@pytest.fixture(autouse=True)
7+
def setup_env(monkeypatch):
8+
monkeypatch.setenv("GITLAB__URL", "https://gitlab.example.com")
9+
10+
11+
class TestIsBotAssignedAsReviewer:
12+
BOT_ID = 516
13+
14+
@mock.patch("pr_agent.servers.gitlab_webhook._get_bot_user_id", new_callable=mock.AsyncMock)
15+
def test_detects_new_assignment(self, mock_bot_id):
16+
from pr_agent.servers.gitlab_webhook import is_bot_assigned_as_reviewer
17+
mock_bot_id.return_value = self.BOT_ID
18+
data = {
19+
"changes": {
20+
"reviewers": {
21+
"previous": [],
22+
"current": [{"id": self.BOT_ID, "username": "k2so-bot"}],
23+
}
24+
}
25+
}
26+
assert asyncio.run(is_bot_assigned_as_reviewer(data)) is True
27+
28+
@mock.patch("pr_agent.servers.gitlab_webhook._get_bot_user_id", new_callable=mock.AsyncMock)
29+
def test_ignores_already_assigned(self, mock_bot_id):
30+
from pr_agent.servers.gitlab_webhook import is_bot_assigned_as_reviewer
31+
mock_bot_id.return_value = self.BOT_ID
32+
data = {
33+
"changes": {
34+
"reviewers": {
35+
"previous": [{"id": self.BOT_ID, "username": "k2so-bot"}],
36+
"current": [{"id": self.BOT_ID, "username": "k2so-bot"}],
37+
}
38+
}
39+
}
40+
assert asyncio.run(is_bot_assigned_as_reviewer(data)) is False
41+
42+
@mock.patch("pr_agent.servers.gitlab_webhook._get_bot_user_id", new_callable=mock.AsyncMock)
43+
def test_no_reviewers_key(self, mock_bot_id):
44+
from pr_agent.servers.gitlab_webhook import is_bot_assigned_as_reviewer
45+
mock_bot_id.return_value = self.BOT_ID
46+
data = {"changes": {"updated_at": {"previous": "old", "current": "new"}}}
47+
assert asyncio.run(is_bot_assigned_as_reviewer(data)) is False
48+
49+
def test_changes_not_dict(self):
50+
from pr_agent.servers.gitlab_webhook import is_bot_assigned_as_reviewer
51+
data = {"changes": "not-a-dict"}
52+
assert asyncio.run(is_bot_assigned_as_reviewer(data)) is False
53+
54+
@mock.patch("pr_agent.servers.gitlab_webhook._get_bot_user_id", new_callable=mock.AsyncMock)
55+
def test_reviewers_not_dict(self, mock_bot_id):
56+
from pr_agent.servers.gitlab_webhook import is_bot_assigned_as_reviewer
57+
mock_bot_id.return_value = self.BOT_ID
58+
data = {"changes": {"reviewers": "not-a-dict"}}
59+
assert asyncio.run(is_bot_assigned_as_reviewer(data)) is False
60+
61+
def test_no_changes_key(self):
62+
from pr_agent.servers.gitlab_webhook import is_bot_assigned_as_reviewer
63+
data = {"object_kind": "merge_request"}
64+
assert asyncio.run(is_bot_assigned_as_reviewer(data)) is False
65+
66+
@mock.patch("pr_agent.servers.gitlab_webhook._get_bot_user_id", new_callable=mock.AsyncMock)
67+
def test_bot_id_unresolvable(self, mock_bot_id):
68+
from pr_agent.servers.gitlab_webhook import is_bot_assigned_as_reviewer
69+
mock_bot_id.return_value = None
70+
data = {
71+
"changes": {
72+
"reviewers": {
73+
"previous": [],
74+
"current": [{"id": self.BOT_ID}],
75+
}
76+
}
77+
}
78+
assert asyncio.run(is_bot_assigned_as_reviewer(data)) is False
79+
80+
@mock.patch("pr_agent.servers.gitlab_webhook._get_bot_user_id", new_callable=mock.AsyncMock)
81+
def test_previous_with_non_dict_entries(self, mock_bot_id):
82+
from pr_agent.servers.gitlab_webhook import is_bot_assigned_as_reviewer
83+
mock_bot_id.return_value = self.BOT_ID
84+
data = {
85+
"changes": {
86+
"reviewers": {
87+
"previous": [{"id": 100}, "not-a-dict"],
88+
"current": [{"id": self.BOT_ID}],
89+
}
90+
}
91+
}
92+
assert asyncio.run(is_bot_assigned_as_reviewer(data)) is True
93+
94+
95+
class TestGetBotUserId:
96+
@staticmethod
97+
def _make_fake_gitlab(user_id):
98+
fake = mock.MagicMock()
99+
fake.Gitlab.return_value.auth.return_value = None
100+
fake.Gitlab.return_value.user.id = user_id
101+
return fake
102+
103+
@staticmethod
104+
def _make_settings(url, token):
105+
s = mock.MagicMock()
106+
s.get.side_effect = lambda k, d=None: {
107+
"GITLAB.URL": url,
108+
"GITLAB.PERSONAL_ACCESS_TOKEN": token,
109+
"GITLAB.SSL_VERIFY": True,
110+
"GITLAB.AUTH_TYPE": "oauth_token",
111+
}.get(k, d)
112+
return s
113+
114+
def test_caches_by_credential(self):
115+
from pr_agent.servers.gitlab_webhook import _bot_user_id_cache, _get_bot_user_id
116+
_bot_user_id_cache.clear()
117+
118+
with mock.patch("pr_agent.servers.gitlab_webhook.get_settings",
119+
return_value=self._make_settings("https://a.example.com", "token-a")):
120+
with mock.patch.dict("sys.modules", {"gitlab": self._make_fake_gitlab(111)}):
121+
assert asyncio.run(_get_bot_user_id()) == 111
122+
123+
with mock.patch("pr_agent.servers.gitlab_webhook.get_settings",
124+
return_value=self._make_settings("https://a.example.com", "token-b")):
125+
with mock.patch.dict("sys.modules", {"gitlab": self._make_fake_gitlab(222)}):
126+
assert asyncio.run(_get_bot_user_id()) == 222
127+
128+
assert len(_bot_user_id_cache) >= 2
129+
130+
def test_no_cache_on_failure(self):
131+
from pr_agent.servers.gitlab_webhook import _bot_user_id_cache, _get_bot_user_id
132+
_bot_user_id_cache.clear()
133+
134+
fake = self._make_fake_gitlab(0)
135+
fake.Gitlab.side_effect = RuntimeError("auth failed")
136+
137+
with mock.patch("pr_agent.servers.gitlab_webhook.get_settings",
138+
return_value=self._make_settings("https://x.example.com", "fail-token")):
139+
with mock.patch.dict("sys.modules", {"gitlab": fake}):
140+
assert asyncio.run(_get_bot_user_id()) is None
141+
142+
assert len(_bot_user_id_cache) == 0
143+
144+
def test_respects_auth_type_private_token(self):
145+
from pr_agent.servers.gitlab_webhook import _bot_user_id_cache, _get_bot_user_id
146+
_bot_user_id_cache.clear()
147+
148+
s = mock.MagicMock()
149+
s.get.side_effect = lambda k, d=None: {
150+
"GITLAB.URL": "https://x.example.com",
151+
"GITLAB.PERSONAL_ACCESS_TOKEN": "tok",
152+
"GITLAB.SSL_VERIFY": True,
153+
"GITLAB.AUTH_TYPE": "private_token",
154+
}.get(k, d)
155+
156+
fake_gitlab = self._make_fake_gitlab(99)
157+
158+
with mock.patch("pr_agent.servers.gitlab_webhook.get_settings", return_value=s):
159+
with mock.patch.dict("sys.modules", {"gitlab": fake_gitlab}):
160+
assert asyncio.run(_get_bot_user_id()) == 99
161+
162+
call_kwargs = fake_gitlab.Gitlab.call_args.kwargs
163+
assert "private_token" in call_kwargs
164+
assert call_kwargs["private_token"] == "tok"
165+
166+
def test_no_token_returns_none(self):
167+
from pr_agent.servers.gitlab_webhook import _bot_user_id_cache, _get_bot_user_id
168+
_bot_user_id_cache.clear()
169+
170+
s = mock.MagicMock()
171+
s.get.side_effect = lambda k, d=None: {
172+
"GITLAB.URL": "https://x.example.com",
173+
"GITLAB.PERSONAL_ACCESS_TOKEN": None,
174+
"GITLAB.SSL_VERIFY": True,
175+
"GITLAB.AUTH_TYPE": "oauth_token",
176+
}.get(k, d)
177+
178+
with mock.patch("pr_agent.servers.gitlab_webhook.get_settings", return_value=s):
179+
assert asyncio.run(_get_bot_user_id()) is None

0 commit comments

Comments
 (0)