Skip to content

Support triggering commands via review assignment in Gitlab#2388

Open
igoraj wants to merge 1 commit into
The-PR-Agent:mainfrom
igoraj:feature/gitlab-reviewer-assignment-trigger
Open

Support triggering commands via review assignment in Gitlab#2388
igoraj wants to merge 1 commit into
The-PR-Agent:mainfrom
igoraj:feature/gitlab-reviewer-assignment-trigger

Conversation

@igoraj
Copy link
Copy Markdown

@igoraj igoraj commented May 12, 2026

Gitlab comes with built-it in commands /request_review, /reviewer, /assign_reviewer which all pop-up when one tries to write /review in comment.

I thought it might come handy if assigning reviewer would actually trigger the same thing, as it is now by default ignored.

I made the change as an togglable setting via handle_reviewer_assignment which is off by default so it does not break current behavior under [gitlab] section.
In addition one can set reviewer_commands = ["/review"] array optionally triggering several commands upon assignment.

Example use in config:

[gitlab]
handle_reviewer_assignment = true
reviewer_commands = ["/review", "/improve"]

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Support auto-triggering commands on GitLab reviewer assignment

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add GitLab reviewer assignment detection via webhook payload
• Auto-trigger configured commands when bot assigned as reviewer
• Implement bot user ID resolution with per-process caching
• Add configurable toggle and command list for reviewer assignment
Diagram
flowchart LR
  A["GitLab Webhook<br/>MR Update Event"] -->|"Check action=update"| B["is_bot_assigned_as_reviewer()"]
  B -->|"Parse reviewers<br/>changes"| C["_get_bot_user_id()"]
  C -->|"Resolve via API<br/>with cache"| D["Bot ID Match"]
  D -->|"If enabled"| E["apply_repo_settings()"]
  E -->|"handle_reviewer_assignment=true"| F["Execute<br/>reviewer_commands"]
  F -->|"Run /review etc"| G["Process MR"]
Loading

Grey Divider

File Changes

1. pr_agent/servers/gitlab_webhook.py ✨ Enhancement +45/-0

Add reviewer assignment detection and command triggering

• Add _get_bot_user_id() function to resolve bot's GitLab user ID via API with global caching
• Add is_bot_assigned_as_reviewer() function to detect when bot is newly assigned as reviewer by
 comparing previous and current reviewer lists
• Add new dispatch branch in merge_request event handler to trigger reviewer_commands when bot
 assignment detected
• Call apply_repo_settings() before checking toggle to enable per-project configuration

pr_agent/servers/gitlab_webhook.py


2. pr_agent/settings/configuration.toml ⚙️ Configuration changes +5/-0

Add GitLab reviewer assignment configuration options

• Add handle_reviewer_assignment boolean toggle (default: false) under [gitlab] section
• Add reviewer_commands array configuration to specify commands triggered on reviewer assignment
• Default reviewer_commands includes "/review" command

pr_agent/settings/configuration.toml


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 12, 2026

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (5)

Grey Divider


Action required

1. Reviewer toggle key casing 🐞 Bug ≡ Correctness ⭐ New
Description
The reviewer-assignment handler reads handle_reviewer_assignment/reviewer_commands via
get_settings().gitlab.get("…") using lowercase keys, which can return defaults when Dynaconf
normalizes stored keys to uppercase. This can prevent the feature from enabling (or from reading the
configured commands) even when configured via file/env/secrets.
Code

pr_agent/servers/gitlab_webhook.py[R357-376]

+                apply_repo_settings(url)
+                handle_assignment = get_settings().gitlab.get("handle_reviewer_assignment", False)
+                if isinstance(handle_assignment, str):
+                    handle_assignment = handle_assignment.lower() in ("true", "1", "yes")
+                if not handle_assignment:
+                    return JSONResponse(status_code=status.HTTP_200_OK,
+                                        content=jsonable_encoder({"message": "success"}))
+
+                # Check PR logic after applying repo settings
+                if not should_process_pr_logic(data):
+                    return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"}))
+
+                if is_draft(data):
+                    get_logger().info(f"Skipping draft MR reviewer assignment: {url}")
+                    return JSONResponse(status_code=status.HTTP_200_OK,
+                                        content=jsonable_encoder({"message": "success"}))
+                if await is_bot_assigned_as_reviewer(data):
+                    reviewer_commands = get_settings().gitlab.get("reviewer_commands", [])
+                    if not isinstance(reviewer_commands, list) or not all(isinstance(c, str) for c in reviewer_commands):
+                        get_logger().warning("gitlab.reviewer_commands is not a list of strings, skipping")
Evidence
The new code path uses lowercase .gitlab.get("…") lookups, while other parts of the repo treat
settings keys as uppercased internally (including explicit uppercasing when applying secrets),
indicating a mismatch that can cause these new lookups to miss configured values.

pr_agent/servers/gitlab_webhook.py[331-380]
pr_agent/config_loader.py[138-150]
tests/unittest/test_fresh_vars_functionality.py[183-188]
tests/unittest/test_fresh_vars_functionality.py[101-111]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The reviewer-assignment branch retrieves settings via `get_settings().gitlab.get("handle_reviewer_assignment")` and `get_settings().gitlab.get("reviewer_commands")`. In this codebase, settings are commonly accessed using `get_settings().get("GITLAB.X")` / `get_settings().get("gitlab.x")`, and other code paths explicitly normalize keys to uppercase, so the lowercase `.gitlab.get()` lookups can miss the configured values.

### Issue Context
This can break the new feature in real deployments (especially when configured via env/secrets), because the toggle/commands may be read as defaults.

### Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[357-376]

### Proposed fix
- Replace:
 - `get_settings().gitlab.get("handle_reviewer_assignment", False)`
 - `get_settings().gitlab.get("reviewer_commands", [])`
 with a single canonical accessor, e.g.:
 - `handle_assignment = get_settings().get("gitlab.handle_reviewer_assignment", False)`
 - `reviewer_commands = get_settings().get("gitlab.reviewer_commands", [])`
 (or the equivalent `GITLAB.HANDLE_REVIEWER_ASSIGNMENT`/`GITLAB.REVIEWER_COMMANDS` form).
- Keep the existing string-to-bool coercion for `handle_assignment` after reading the value.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. ssl_verify path misparsed ✓ Resolved 🐞 Bug ⛨ Security
Description
_get_bot_user_id() converts any string GITLAB.SSL_VERIFY into a boolean, so a custom CA path
(documented as supported) becomes False and disables TLS verification. This can silently weaken
transport security or break GitLab API auth in reviewer-assignment triggering.
Code

pr_agent/servers/gitlab_webhook.py[R131-134]

+        ssl_verify = get_settings().get("GITLAB.SSL_VERIFY", True)
+        if isinstance(ssl_verify, str):
+            ssl_verify = ssl_verify.lower() in ("true", "1", "yes")
+
Evidence
The new code coerces any string ssl_verify to bool, conflicting with the repository’s documented
configuration and the existing GitLabProvider behavior which supports CA-path strings for
ssl_verify.

pr_agent/servers/gitlab_webhook.py[131-147]
pr_agent/settings/configuration.toml[270-276]
pr_agent/git_providers/gitlab_provider.py[33-62]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_get_bot_user_id()` currently coerces any string `GITLAB.SSL_VERIFY` into a boolean by checking only `("true","1","yes")`. This breaks the documented/expected behavior where `ssl_verify` may be a *path to a custom CA bundle* (string), and worse, turns that path into `False` which disables TLS verification.
### Issue Context
Elsewhere in the codebase, `GitLabProvider` passes `ssl_verify` through to `gitlab.Gitlab(..., ssl_verify=ssl_verify)` without coercion, preserving support for both booleans and CA-path strings.
### Fix
Update the normalization logic to:
- Only coerce strings that are explicit boolean-like values (`true/false/1/0/yes/no`).
- Otherwise, keep the original string value (treat as CA path).
Example:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Isort import grouping fails 🐞 Bug ⚙ Maintainability
Description
The new test module mixes standard-library and third-party imports without a separating blank line,
which violates the repo’s enabled Ruff isort rules and can fail lint/CI for this PR.
Code

tests/unittest/test_gitlab_reviewer_assignment.py[R1-4]

+import asyncio
+from unittest import mock
+import pytest
+
Evidence
Ruff is configured to enforce isort import rules (I001/I002). The added test file currently has
stdlib imports (asyncio, unittest) directly adjacent to the third-party import (pytest) with
no blank line, which violates the enforced grouping rules.

tests/unittest/test_gitlab_reviewer_assignment.py[1-4]
pyproject.toml[52-61]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`tests/unittest/test_gitlab_reviewer_assignment.py` groups `pytest` together with stdlib imports without a blank line separator. With Ruff’s isort rules enabled in this repo, this will be reported as an import-order/grouping violation.
### Issue Context
Ruff is configured to lint isort rules (`I001`, `I002`) in `pyproject.toml`.
### Fix
Reorder/group imports into sections and add a blank line between stdlib and third-party imports.
### Fix Focus Areas
- tests/unittest/test_gitlab_reviewer_assignment.py[1-4]
- pyproject.toml[52-61]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (10)
4. Trailing whitespace-only blank lines 📘 Rule violation ⚙ Maintainability
Description
The new reviewer-assignment branch adds blank lines containing trailing whitespace, which can fail
pre-commit whitespace hooks and create noisy diffs. This should be cleaned to keep formatting checks
passing.
Code

pr_agent/servers/gitlab_webhook.py[R333-339]

+                
+                # Fast early-exit: no reviewer changes means nothing to do
+                changes = data.get("changes")
+                if not isinstance(changes, dict) or "reviewers" not in changes:
+                    return JSONResponse(status_code=status.HTTP_200_OK,
+                                        content=jsonable_encoder({"message": "success"}))
+                
Evidence
PR Compliance ID 16 requires pre-commit whitespace/newline checks to pass. The added lines at the
cited locations are blank but contain trailing whitespace.

AGENTS.md
pr_agent/servers/gitlab_webhook.py[333-333]
pr_agent/servers/gitlab_webhook.py[339-339]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Two newly-added blank lines contain trailing whitespace, which can fail pre-commit whitespace checks.
## Issue Context
These are whitespace-only lines in the reviewer-assignment handling block.
## Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[333-333]
- pr_agent/servers/gitlab_webhook.py[339-339]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Unused os import in test 📘 Rule violation ⚙ Maintainability
Description
The new unit test file imports os but never uses it, adding dead code and likely triggering lint
failures. Removing it keeps the test suite clean and compliant.
Code

tests/unittest/test_gitlab_reviewer_assignment.py[R1-4]

+import asyncio
+import os
+from unittest import mock
+import pytest
Evidence
PR Compliance ID 2 prohibits dead/unused code in submitted changes. The cited new test file includes
import os with no usage in the module.

Rule 2: No Dead or Commented-Out Code
tests/unittest/test_gitlab_reviewer_assignment.py[1-4]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test module imports `os` but does not use it.
## Issue Context
This is newly added code and should not introduce unused imports/dead code.
## Fix Focus Areas
- tests/unittest/test_gitlab_reviewer_assignment.py[1-4]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Permanent failure cache ✓ Resolved 🐞 Bug ☼ Reliability
Description
_get_bot_user_id() caches a sentinel (-1) on any exception and then returns None for all future
calls for that URL/token, so one transient GitLab auth/network failure can permanently disable
reviewer-assignment triggering until process restart.
Code

pr_agent/servers/gitlab_webhook.py[R122-151]

+    cache_key = hashlib.sha256(f"{gitlab_url}:{gitlab_token}".encode()).hexdigest()
+
+    cached = _bot_user_id_cache.get(cache_key)
+    if cached is not None:
+        return cached if cached != -1 else None
+
+    def _resolve_sync():
+        import gitlab
+
+        ssl_verify = get_settings().get("GITLAB.SSL_VERIFY", True)
+        auth_method = get_settings().get("GITLAB.AUTH_TYPE", "oauth_token")
+        kwargs = {"url": gitlab_url, "ssl_verify": ssl_verify}
+        if auth_method == "oauth_token":
+            kwargs["oauth_token"] = gitlab_token
+        else:
+            kwargs["private_token"] = gitlab_token
+
+        gl = gitlab.Gitlab(**kwargs)
+        gl.auth()
+        return gl.user.id
+
+    try:
+        user_id = await asyncio.to_thread(_resolve_sync)
+        _bot_user_id_cache[cache_key] = user_id
+        get_logger().info(f"Bot user ID resolved via API: {user_id}")
+        return user_id
+    except Exception as e:
+        _bot_user_id_cache[cache_key] = -1
+        get_logger().error(f"Failed to resolve bot user ID: {e}")
+        return None
Evidence
The function explicitly stores -1 on exception and the cache lookup treats -1 as a permanent
"return None" outcome, with no TTL/expiration mechanism anywhere in the cache handling.

pr_agent/servers/gitlab_webhook.py[113-151]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_get_bot_user_id()` stores `-1` in `_bot_user_id_cache` on *any* exception, and subsequent calls treat that as a permanent failure (returning `None`). This makes reviewer-assignment triggering brittle: a transient outage/rate-limit/timeout can disable the feature until the service is restarted.
### Issue Context
The cache is process-global and has no expiration logic. The code currently uses `-1` as a negative-cache sentinel with no retry window.
### Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[113-151]
### Suggested fix
- Replace the `-1` sentinel with a small structure like `{value, ts, is_failure}` and only honor negative-cache entries for a short TTL (e.g., 30–300 seconds), after which a retry is allowed.
- Alternatively, do not cache failures at all (or cache only specific non-retryable failures), and keep caching only successful resolutions.
- Add/adjust a unit test to verify retries occur after TTL expiry (or that failures are not cached).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Late import after env 📘 Rule violation ⚙ Maintainability
Description
The test sets GITLAB__URL at module import time and then performs a module-level import of
pr_agent.servers.gitlab_webhook, triggering Ruff E402 and making test behavior depend on ambient
environment state. This violates lint/style conventions and the requirement that tests explicitly
control environment variables/process-global state for determinism.
Code

tests/unittest/test_gitlab_reviewer_assignment.py[R4-6]

+os.environ.setdefault("GITLAB__URL", "https://gitlab.example.com")
+import pr_agent.servers.gitlab_webhook as gitlab_webhook
+
Evidence
Rule 10 requires adherence to Ruff conventions; the file executes code
(os.environ.setdefault(...)) before a module-level import, which commonly triggers Ruff E402 and
breaks import-order conventions. Rule 17 requires tests to explicitly control env/global state;
using setdefault at import time does not guarantee the test’s preconditions and can vary depending
on the runner’s environment.

AGENTS.md
tests/unittest/test_gitlab_reviewer_assignment.py[4-6]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`tests/unittest/test_gitlab_reviewer_assignment.py` executes `os.environ.setdefault(...)` at import time and then imports `pr_agent.servers.gitlab_webhook` at module scope, which triggers Ruff `E402` and makes the test rely on implicit/ambient environment state.
## Issue Context
`pr_agent.servers.gitlab_webhook` performs work at import time (e.g., logger setup using `get_settings()`), so tests that need env/config should set env vars explicitly inside a fixture/test, then import/reload the module deterministically.
## Fix Focus Areas
- tests/unittest/test_gitlab_reviewer_assignment.py[1-6]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. Patch targets missing gitlab ✓ Resolved 📘 Rule violation ≡ Correctness
Description
The unit tests patch pr_agent.servers.gitlab_webhook.gitlab.Gitlab, but gitlab_webhook.py is a
single module (not a package with a gitlab submodule) and the production code imports gitlab
only locally inside _get_bot_user_id(), so there is no module-level gitlab attribute to patch
and mock.patch(...) will raise AttributeError/import errors before assertions run. This breaks
the test suite/CI and undermines the requirement to provide working test coverage for the changed
behavior.
Code

tests/unittest/test_gitlab_reviewer_assignment.py[R93-100]

+        with mock.patch("pr_agent.servers.gitlab_webhook.get_settings", return_value=make_settings("https://a.example.com", "token-a")):
+            with mock.patch("pr_agent.servers.gitlab_webhook.gitlab.Gitlab", mock_gitlab_cls):
+                assert gitlab_webhook._get_bot_user_id() == 111
+
+        gl_mock.user.id = 222
+        with mock.patch("pr_agent.servers.gitlab_webhook.get_settings", return_value=make_settings("https://a.example.com", "token-b")):
+            with mock.patch("pr_agent.servers.gitlab_webhook.gitlab.Gitlab", mock_gitlab_cls):
+                assert gitlab_webhook._get_bot_user_id() == 222
Evidence
Rule 13 requires relevant tests for the changed behavior, but the tests currently target
pr_agent.servers.gitlab_webhook.gitlab.Gitlab, which unittest.mock.patch interprets as importing
pr_agent.servers.gitlab_webhook.gitlab and then replacing Gitlab; the codebase instead has
pr_agent/servers/gitlab_webhook.py (a file), so that import path does not exist. In production,
_get_bot_user_id() performs import gitlab inside the function and then calls
gitlab.Gitlab(...), meaning gitlab is not a module-level attribute on
pr_agent.servers.gitlab_webhook, so the patch fails before the tests can execute; consequently,
patching gitlab.Gitlab (or injecting a fake gitlab module) is the correct approach.

AGENTS.md
tests/unittest/test_gitlab_reviewer_assignment.py[93-100]
pr_agent/servers/gitlab_webhook.py[128-141]
pr_agent/servers/gitlab_webhook.py[1-13]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The unit tests currently patch `"pr_agent.servers.gitlab_webhook.gitlab.Gitlab"`, but `gitlab_webhook.py` is a module (not a package with a `gitlab` submodule) and `gitlab` is imported locally inside `_get_bot_user_id()`, so there is no `pr_agent.servers.gitlab_webhook.gitlab` import path (nor a module-level `gitlab` attribute) for `mock.patch` to replace. This causes the patch to fail before assertions run, breaking CI and preventing the tests from providing valid coverage.
## Issue Context
`_get_bot_user_id()` does `import gitlab` and then uses `gitlab.Gitlab(...)`. To avoid requiring the real `python-gitlab` dependency during tests, update the tests to patch the correct target (`gitlab.Gitlab`) or inject a fake `gitlab` module into `sys.modules` (e.g., via `monkeypatch` or `mock.patch.dict`) that provides a `Gitlab` class returning the desired mock client.
## Fix Focus Areas
- tests/unittest/test_gitlab_reviewer_assignment.py[89-100]
- tests/unittest/test_gitlab_reviewer_assignment.py[114-119]
- tests/unittest/test_gitlab_reviewer_assignment.py[134-145]
- pr_agent/servers/gitlab_webhook.py[128-141]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. Unconditional repo settings fetch ✓ Resolved 🐞 Bug ➹ Performance
Description
The reviewer-assignment webhook path calls apply_repo_settings(url) before checking
handle_reviewer_assignment, so even when the feature is disabled (default), MR update events
without oldrev still trigger GitLabProvider construction and GitLab API work. This adds avoidable
latency and rate-limit/load risk on common non-reviewer update events.
Code

pr_agent/servers/gitlab_webhook.py[R321-326]

+            elif object_attributes.get('action') == 'update' and not object_attributes.get('oldrev'):
+                url = object_attributes.get('url')
+                apply_repo_settings(url)
+                if not get_settings().gitlab.get('handle_reviewer_assignment', False):
+                    return JSONResponse(status_code=status.HTTP_200_OK,
+                                        content=jsonable_encoder({"message": "success"}))
Evidence
The new branch calls apply_repo_settings(url) before the feature flag check.
apply_repo_settings() constructs a Git provider via get_git_provider_with_context(), and the
GitLab provider constructor immediately sets the MR and loads diffs (GitLab API calls), so this
becomes expensive work executed even when the feature is off.

pr_agent/servers/gitlab_webhook.py[320-334]
pr_agent/git_providers/utils.py[14-27]
pr_agent/git_providers/init.py[40-65]
pr_agent/git_providers/gitlab_provider.py[32-76]
pr_agent/git_providers/gitlab_provider.py[341-346]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`apply_repo_settings(url)` is invoked unconditionally in the reviewer-assignment update branch, before the feature toggle check. `apply_repo_settings()` constructs a Git provider instance, and for GitLab this initializes a `GitLabProvider` which fetches MR data/diffs via the GitLab API.
## Issue Context
This code runs for `merge_request` webhooks where `action == "update"` and `oldrev` is falsy, which can occur for many non-reviewer updates (title/label/etc). When `handle_reviewer_assignment` is false (the default), the handler should return without fetching repo settings/provider.
## Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[320-334]
- pr_agent/git_providers/utils.py[14-27]
- pr_agent/git_providers/__init__.py[40-65]
- pr_agent/git_providers/gitlab_provider.py[32-76]
- pr_agent/git_providers/gitlab_provider.py[341-346]
### Suggested approach
1) Add a cheap payload guard before any expensive work (e.g., if `changes.reviewers` missing -> return).
2) Check the instance-level/global setting first (e.g., `get_settings().get("gitlab.handle_reviewer_assignment", False)`); if false and repo-settings-file is not used, return.
3) Only then call `apply_repo_settings(url)` and re-check the setting (to support repo overrides), and proceed to `is_bot_assigned_as_reviewer` / `_perform_commands_gitlab`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


10. New strings use single quotes ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
Newly added code uses single-quoted strings (e.g., dict keys like data.get('changes')), which
conflicts with the compliance requirement to use double quotes for strings. This can cause avoidable
Ruff formatting/lint failures.
Code

pr_agent/servers/gitlab_webhook.py[R150-167]

+def is_bot_assigned_as_reviewer(data) -> bool:
+    try:
+        changes = data.get('changes')
+        if not isinstance(changes, dict):
+            return False
+        if 'reviewers' not in changes:
+            return False
+        reviewers_change = changes['reviewers']
+        if not isinstance(reviewers_change, dict):
+            return False
+        previous = reviewers_change.get('previous', [])
+        current = reviewers_change.get('current', [])
+        bot_user_id = _get_bot_user_id()
+        if bot_user_id is None:
+            return False
+        previous_ids = {r.get('id') for r in previous if isinstance(r, dict)}
+        current_ids = {r.get('id') for r in current if isinstance(r, dict)}
+        return bot_user_id in current_ids and bot_user_id not in previous_ids
Evidence
PR Compliance ID 6 explicitly requires double quotes for strings. The added reviewer-assignment
logic introduces multiple single-quoted string literals (e.g., data.get('changes'),
'reviewers'), violating that requirement.

AGENTS.md
pr_agent/servers/gitlab_webhook.py[150-167]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New code uses single-quoted strings, but the compliance checklist requires double quotes for Python strings.
## Issue Context
This affects newly added GitLab reviewer-assignment logic and may trigger Ruff formatting/lint failures.
## Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[150-167]
- pr_agent/servers/gitlab_webhook.py[320-333]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


11. _bot_user_id_cache shared global state ✓ Resolved 📘 Rule violation ⛨ Security
Description
_get_bot_user_id() caches a token-derived GitLab user ID in the process-global
_bot_user_id_cache, which can persist across webhook requests and silently apply the wrong
identity when tokens/instances differ. This can cause incorrect authorization/behavior (e.g.,
reviewer-assignment detection misfiring) and cross-request state leakage in multi-tenant,
rotated-token, or warm AWS Lambda deployments.
Code

pr_agent/servers/gitlab_webhook.py[R111-126]

+_bot_user_id_cache = None
+
+def _get_bot_user_id():
+    global _bot_user_id_cache
+    if _bot_user_id_cache is not None:
+        return _bot_user_id_cache
+    try:
+        import gitlab
+        gl = gitlab.Gitlab(
+            get_settings().get("GITLAB.URL", "https://gitlab.com"),
+            private_token=get_settings().get("GITLAB.PERSONAL_ACCESS_TOKEN", None),
+        )
+        gl.auth()
+        _bot_user_id_cache = gl.user.id
+        get_logger().info(f"Bot user ID resolved via API: {_bot_user_id_cache}")
+        return _bot_user_id_cache
Evidence
PR Compliance ID 18 requires avoiding shared mutable globals for provider scoping and guarding
against silent overrides, yet the code introduces _bot_user_id_cache as a module-global value that
is populated from an authenticated GitLab client and then reused for future calls regardless of
request context. Because the webhook path can set different GitLab tokens per request based on
X-Gitlab-Token (via a secret provider), and AWS Lambda can reuse the same module/router across
warm invocations, the single cached ID can outlive a request and incorrectly apply to subsequent
requests using different credentials or GitLab instances.

pr_agent/servers/gitlab_webhook.py[111-126]
pr_agent/servers/gitlab_webhook.py[111-129]
pr_agent/servers/gitlab_webhook.py[207-243]
pr_agent/servers/gitlab_lambda_webhook.py[2-7]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_bot_user_id_cache` is a module-level cache for a value derived from GitLab credentials (the current user/bot id). In a server handling multiple webhook requests—potentially with different tokens, tenants, or GitLab instances—this shared mutable global can persist across requests (and warm AWS Lambda invocations) and cause incorrect bot identity decisions, including reviewer-assignment detection triggering for the wrong user or failing to trigger.
## Issue Context
The webhook handler can set GitLab credentials dynamically per request (e.g., via secret provider and `X-Gitlab-Token`). The bot user id must be scoped to the effective credentials (and ideally the GitLab URL/instance) rather than stored in a single process-wide variable; otherwise, the cached ID can silently mismatch the current request’s token/instance, violating the intent of PR Compliance ID 18 (avoid shared mutable globals for provider scoping and protect against silent overrides).
## Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[111-129]
- pr_agent/servers/gitlab_webhook.py[207-243]
- pr_agent/servers/gitlab_lambda_webhook.py[2-7]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


12. No tests for reviewer assignment 📘 Rule violation ⚙ Maintainability
Description
The PR adds new behavior to auto-trigger commands when the bot is assigned as a reviewer, but no
corresponding pytest coverage is added/updated. This increases regression risk for webhook parsing
and configuration toggle behavior.
Code

pr_agent/servers/gitlab_webhook.py[R296-302]

+            # for reviewer assignment triggered merge requests
+            elif object_attributes.get('action') == 'update' and is_bot_assigned_as_reviewer(data):
+                url = object_attributes.get('url')
+                apply_repo_settings(url)
+                if get_settings().gitlab.get('handle_reviewer_assignment', False):
+                    get_logger().info(f"Bot was assigned as reviewer on MR: {url}")
+                    await _perform_commands_gitlab("reviewer_commands", PRAgent(), url, log_context, data)
Evidence
PR Compliance ID 13 requires behavior changes to be backed by tests in the appropriate pytest
directories. The diff introduces a new webhook action path that performs commands on reviewer
assignment, but this PR diff does not include any new/updated tests for it.

AGENTS.md
pr_agent/servers/gitlab_webhook.py[296-303]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Reviewer-assignment-triggered command execution is new behavior and needs tests to prevent regressions.
## Issue Context
Add unit tests to validate:
- detection logic for bot assignment based on `changes.reviewers.previous/current`
- `handle_reviewer_assignment` gating
- correct command list used (`reviewer_commands`)
## Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[296-303]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


13. Auth type ignored ✓ Resolved 🐞 Bug ≡ Correctness
Description
_get_bot_user_id always authenticates using private_token and does not pass SSL verification
settings, while the rest of the GitLab integration supports oauth_token/private_token selection via
GITLAB.AUTH_TYPE and SSL_VERIFY. This can prevent bot user-id resolution in oauth_token deployments
and silently disable reviewer-assignment triggering.
Code

pr_agent/servers/gitlab_webhook.py[R118-126]

+        import gitlab
+        gl = gitlab.Gitlab(
+            get_settings().get("GITLAB.URL", "https://gitlab.com"),
+            private_token=get_settings().get("GITLAB.PERSONAL_ACCESS_TOKEN", None),
+        )
+        gl.auth()
+        _bot_user_id_cache = gl.user.id
+        get_logger().info(f"Bot user ID resolved via API: {_bot_user_id_cache}")
+        return _bot_user_id_cache
Evidence
The new bot-id resolver hardcodes private_token without ssl_verify, while the existing GitLab
provider explicitly supports oauth/private tokens via GITLAB.AUTH_TYPE and uses SSL_VERIFY,
demonstrating expected configuration behavior.

pr_agent/servers/gitlab_webhook.py[113-126]
pr_agent/git_providers/gitlab_provider.py[33-65]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_get_bot_user_id()` hardcodes `private_token=...` and omits `ssl_verify`, diverging from `GitLabProvider` which selects auth based on `GITLAB.AUTH_TYPE` and configures SSL verification.
### Issue Context
If `GITLAB.AUTH_TYPE` is set to (or defaults to) oauth token usage, the hardcoded `private_token` path can fail and `is_bot_assigned_as_reviewer()` will always return `False`.
### Fix
- Mirror the `GitLabProvider` client creation logic:
- read `GITLAB.AUTH_TYPE` and pass either `oauth_token=` or `private_token=` accordingly
- pass `ssl_verify=get_settings().get("GITLAB.SSL_VERIFY", True)`
- Prefer extracting a shared helper to create an authenticated python-gitlab client to avoid drift.
### Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[113-126]
- pr_agent/git_providers/gitlab_provider.py[33-65]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

14. Bot-ID resolve stampede 🐞 Bug ☼ Reliability ⭐ New
Description
On a cache miss, _get_bot_user_id() can spawn many parallel gl.auth() calls because concurrent
requests can all pass the unsynchronized cache check and run asyncio.to_thread(_resolve_sync).
Under bursty reviewer-assignment updates or multi-worker startup, this can add latency and trigger
GitLab API rate limiting.
Code

pr_agent/servers/gitlab_webhook.py[R124-166]

+    cached = _bot_user_id_cache.get(cache_key)
+    if cached is not None:
+        return cached
+
+    def _resolve_sync():
+        import gitlab
+
+        ssl_verify = get_settings().get("GITLAB.SSL_VERIFY", True)
+        if isinstance(ssl_verify, str) and ssl_verify.lower() in ("true", "false"):
+            ssl_verify = ssl_verify.lower() == "true"
+
+        auth_method = get_settings().get("GITLAB.AUTH_TYPE", "oauth_token")
+        if auth_method not in ("oauth_token", "private_token"):
+            raise ValueError(
+                f"Unsupported GITLAB.AUTH_TYPE: '{auth_method}'. "
+                f"Must be 'oauth_token' or 'private_token'."
+            )
+
+        if auth_method == "oauth_token":
+            gl = gitlab.Gitlab(
+                url=gitlab_url,
+                oauth_token=gitlab_token,
+                ssl_verify=ssl_verify
+            )
+        else:
+            gl = gitlab.Gitlab(
+                url=gitlab_url,
+                private_token=gitlab_token,
+                ssl_verify=ssl_verify
+            )
+        gl.auth()
+        return gl.user.id
+
+    try:
+        user_id = await asyncio.to_thread(_resolve_sync)
+        if len(_bot_user_id_cache) > 1000:
+            _bot_user_id_cache.clear()
+        _bot_user_id_cache[cache_key] = user_id
+        get_logger().info(f"Bot user ID resolved via API: {user_id}")
+        return user_id
+    except Exception as e:
+        get_logger().error(f"Failed to resolve bot user ID: {e}")
+        return None
Evidence
The function checks _bot_user_id_cache and immediately proceeds to
asyncio.to_thread(_resolve_sync) on a miss with no lock or in-flight future, so concurrent calls
can all execute the same expensive GitLab auth flow.

pr_agent/servers/gitlab_webhook.py[113-166]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`_get_bot_user_id()` uses a global dict cache, but there is no in-flight de-duplication. If multiple webhook tasks call it concurrently before the cache is populated, each call will execute `_resolve_sync()` in a thread and hit GitLab’s `auth()` endpoint.

### Issue Context
This code runs in an async webhook server and can see concurrent tasks for the same GitLab URL/token.

### Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[113-166]

### Proposed fix
Implement in-flight de-duplication per `cache_key`, e.g.:
- Maintain a module-level `asyncio.Lock` per `cache_key`, or a dict `inflight: dict[str, asyncio.Future[int|None]]`.
- On cache miss:
 - Acquire the lock / await the existing inflight future.
 - Re-check the cache after acquiring.
 - Only one task performs `asyncio.to_thread(_resolve_sync)` and populates the cache; others await the result.
- Ensure failures don’t permanently block (clean up inflight entries in `finally`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


15. Missing .pr_agent.toml gitlab settings 📘 Rule violation ⚙ Maintainability
Description
New GitLab behavior is introduced via handle_reviewer_assignment/reviewer_commands defaults, but
the repo’s .pr_agent.toml mirror/example is not updated to include these keys. This can cause
configuration drift and confusion about how to enable the feature consistently across environments.
Code

pr_agent/settings/configuration.toml[R272-276]

+# Auto-trigger commands when the bot is assigned as a reviewer on an MR
+handle_reviewer_assignment = false
+reviewer_commands = [
+    "/review",
+]
Evidence
Rule 14 requires configuration mirrors to stay aligned when behavior changes. The PR adds new GitLab
settings defaults in pr_agent/settings/configuration.toml, but .pr_agent.toml does not include
any corresponding [gitlab] entries, indicating the mirrors are out of sync.

AGENTS.md
pr_agent/settings/configuration.toml[272-276]
.pr_agent.toml[1-20]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR adds new GitLab configuration keys (`handle_reviewer_assignment`, `reviewer_commands`) to `pr_agent/settings/configuration.toml`, but `.pr_agent.toml` is not updated to mirror/document these options. This creates configuration drift and makes it harder for users to discover/enable the feature.
## Issue Context
Compliance requires keeping configuration mirrors in sync when behavior changes.
## Fix Focus Areas
- pr_agent/settings/configuration.toml[272-276]
- .pr_agent.toml[1-20]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


16. Duplicated GitLab auth logic ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
_get_bot_user_id() reimplements GitLab client creation/authentication instead of reusing the
existing GitLabProvider construction path, and the two implementations already differ (e.g.,
auth_type validation/behavior). This duplication increases maintenance risk and can lead to webhook
reviewer-trigger behavior diverging from normal GitLab operations over time.
Code

pr_agent/servers/gitlab_webhook.py[R128-147]

+    def _resolve_sync():
+        import gitlab
+
+        ssl_verify = get_settings().get("GITLAB.SSL_VERIFY", True)
+        if isinstance(ssl_verify, str) and ssl_verify.lower() in ("true", "false"):
+            ssl_verify = ssl_verify.lower() == "true"
+
+        auth_method = get_settings().get("GITLAB.AUTH_TYPE", "oauth_token")
+        if auth_method not in ("oauth_token", "private_token"):
+            auth_method = "oauth_token"
+
+        kwargs = {"url": gitlab_url, "ssl_verify": ssl_verify}
+        if auth_method == "oauth_token":
+            kwargs["oauth_token"] = gitlab_token
+        else:
+            kwargs["private_token"] = gitlab_token
+
+        gl = gitlab.Gitlab(**kwargs)
+        gl.auth()
+        return gl.user.id
Evidence
The webhook adds a new, separate GitLab client/auth implementation inside _get_bot_user_id(),
while the repo already has a GitLabProvider that constructs the client and validates auth
configuration, meaning there are now two implementations to keep consistent.

pr_agent/servers/gitlab_webhook.py[128-147]
pr_agent/git_providers/gitlab_provider.py[32-66]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_get_bot_user_id()` builds and authenticates its own `gitlab.Gitlab(...)` client instead of reusing a shared/canonical GitLab client factory (or `GitLabProvider`). This duplicates configuration handling (SSL verify, auth type, token usage) and invites drift.
### Issue Context
There is already a GitLab client construction path in `GitLabProvider.__init__` that enforces/validates auth configuration. The new webhook-specific client setup is a second source of truth.
### Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[128-147]
- pr_agent/git_providers/gitlab_provider.py[32-66]
### Suggested change
Refactor to a shared helper (e.g., `create_gitlab_client_from_settings()`) used by both `GitLabProvider` and `_get_bot_user_id()`, or instantiate/reuse `GitLabProvider` in `_get_bot_user_id()` to obtain the current user id via the provider’s authenticated client.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (6)
17. Unused -1 cache sentinel ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
_get_bot_user_id() contains logic to treat -1 as a cached failure sentinel, but this PR never
writes -1 into _bot_user_id_cache. This is dead/unused logic that adds confusion and maintenance
risk.
Code

pr_agent/servers/gitlab_webhook.py[R124-126]

+    cached = _bot_user_id_cache.get(cache_key)
+    if cached is not None:
+        return cached if cached != -1 else None
Evidence
PR Compliance ID 2 forbids dead/unused code. The new cache retrieval branch explicitly handles a
-1 sentinel even though the PR contains no corresponding write of -1, making that conditional
path unused.

Rule 2: No Dead or Commented-Out Code
pr_agent/servers/gitlab_webhook.py[124-126]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_get_bot_user_id()` checks for a `-1` sentinel in `_bot_user_id_cache` (`return cached if cached != -1 else None`), but no code path stores `-1` in the cache. This makes the sentinel handling dead code.
## Issue Context
The function now only caches successful `user_id` values and returns `None` on failures without caching them.
## Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[124-126]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


18. Repo settings applied twice 🐞 Bug ➹ Performance
Description
The reviewer-assignment handler calls apply_repo_settings(url) and then calls
_perform_commands_gitlab(), which calls apply_repo_settings(api_url) again, repeating
temp-file/Dynaconf merge work for the same MR event. This adds avoidable latency and I/O on reviewer
assignment updates.
Code

pr_agent/servers/gitlab_webhook.py[R349-372]

+                apply_repo_settings(url)
+                handle_assignment = get_settings().gitlab.get("handle_reviewer_assignment", False)
+                if isinstance(handle_assignment, str):
+                    handle_assignment = handle_assignment.lower() in ("true", "1", "yes")
+                if not handle_assignment:
+                    return JSONResponse(status_code=status.HTTP_200_OK,
+                                        content=jsonable_encoder({"message": "success"}))
+
+                # Check PR logic after applying repo settings
+                if not should_process_pr_logic(data):
+                    return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"}))
+
+                if is_draft(data):
+                    get_logger().info(f"Skipping draft MR reviewer assignment: {url}")
+                    return JSONResponse(status_code=status.HTTP_200_OK,
+                                        content=jsonable_encoder({"message": "success"}))
+                if await is_bot_assigned_as_reviewer(data):
+                    reviewer_commands = get_settings().gitlab.get("reviewer_commands", [])
+                    if not isinstance(reviewer_commands, list) or not all(isinstance(c, str) for c in reviewer_commands):
+                        get_logger().warning("gitlab.reviewer_commands is not a list of strings, skipping")
+                        return JSONResponse(status_code=status.HTTP_200_OK,
+                                            content=jsonable_encoder({"message": "success"}))
+                    get_logger().info(f"Bot was assigned as reviewer on MR: {url}")
+                    await _perform_commands_gitlab("reviewer_commands", PRAgent(), url, log_context, data)
Evidence
The reviewer-assignment branch applies repo settings, then delegates to a helper that applies repo
settings again; the apply_repo_settings implementation performs non-trivial temp-file/Dynaconf work
each invocation.

pr_agent/servers/gitlab_webhook.py[41-49]
pr_agent/servers/gitlab_webhook.py[343-372]
pr_agent/git_providers/utils.py[14-73]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
In the reviewer-assignment update path, repo settings are applied explicitly before reading `gitlab.handle_reviewer_assignment`, and then `_perform_commands_gitlab()` applies repo settings again.
This duplicates expensive work (`mkstemp` + Dynaconf load/merge) per event.
### Issue Context
`_perform_commands_gitlab()` always begins with `apply_repo_settings(api_url)`.
`apply_repo_settings()` creates a temp file and loads Dynaconf settings each time it runs.
### Fix
Ensure repo settings are applied only once per webhook event/URL. Options:
1) Add a parameter to `_perform_commands_gitlab(..., skip_apply_repo_settings: bool = False)` and set it to `True` from the reviewer-assignment path (since settings are already applied).
2) Alternatively, in the reviewer-assignment path, avoid the first `apply_repo_settings(url)` by fetching `handle_reviewer_assignment` using only global settings (if acceptable), and apply only when enabled.
### Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[41-49]
- pr_agent/servers/gitlab_webhook.py[349-372]
- pr_agent/git_providers/utils.py[14-73]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


19. previous/current reviewers not type-checked ✓ Resolved 📘 Rule violation ☼ Reliability
Description
is_bot_assigned_as_reviewer() assumes reviewers.previous and reviewers.current are iterable,
which can raise TypeError (and log noisy errors) if GitLab sends null or a non-list value. This
reduces robustness against schema variations in webhook payloads.
Code

pr_agent/servers/gitlab_webhook.py[R170-177]

+        previous = reviewers_change.get("previous", [])
+        current = reviewers_change.get("current", [])
+        bot_user_id = await _get_bot_user_id()
+        if bot_user_id is None:
+            return False
+        previous_ids = {r.get("id") for r in previous if isinstance(r, dict)}
+        current_ids = {r.get("id") for r in current if isinstance(r, dict)}
+        return bot_user_id in current_ids and bot_user_id not in previous_ids
Evidence
PR Compliance ID 19 requires defensive access/type checks for optional or variable payload
structures. The added code reads previous = reviewers_change.get("previous", []) and then iterates
it in a set-comprehension without validating it is a list, which can fail if the payload contains
null/non-list values.

pr_agent/servers/gitlab_webhook.py[170-177]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`is_bot_assigned_as_reviewer()` iterates over `previous`/`current` without confirming they are lists. If either field is `None` or another unexpected type, the function can throw (caught by the outer `try`, but still produces error logs and may hide real issues).
## Issue Context
GitLab webhook payloads can vary; defensive type checks should be used before iterating or indexing optional fields.
## Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[170-177]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


20. GITLAB.AUTH_TYPE not validated ✓ Resolved ...

Comment thread pr_agent/servers/gitlab_webhook.py Outdated
Comment on lines +296 to +302
# for reviewer assignment triggered merge requests
elif object_attributes.get('action') == 'update' and is_bot_assigned_as_reviewer(data):
url = object_attributes.get('url')
apply_repo_settings(url)
if get_settings().gitlab.get('handle_reviewer_assignment', False):
get_logger().info(f"Bot was assigned as reviewer on MR: {url}")
await _perform_commands_gitlab("reviewer_commands", PRAgent(), url, log_context, data)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. No tests for reviewer assignment 📘 Rule violation ⚙ Maintainability

The PR adds new behavior to auto-trigger commands when the bot is assigned as a reviewer, but no
corresponding pytest coverage is added/updated. This increases regression risk for webhook parsing
and configuration toggle behavior.
Agent Prompt
## Issue description
Reviewer-assignment-triggered command execution is new behavior and needs tests to prevent regressions.

## Issue Context
Add unit tests to validate:
- detection logic for bot assignment based on `changes.reviewers.previous/current`
- `handle_reviewer_assignment` gating
- correct command list used (`reviewer_commands`)

## Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[296-303]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread pr_agent/servers/gitlab_webhook.py Outdated
@igoraj igoraj force-pushed the feature/gitlab-reviewer-assignment-trigger branch from 133a196 to dfe13bf Compare May 12, 2026 09:28
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 12, 2026

Persistent review updated to latest commit dfe13bf

Comment thread pr_agent/servers/gitlab_webhook.py Outdated
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 12, 2026

Persistent review updated to latest commit efbbf0d

Comment thread tests/unittest/test_gitlab_reviewer_assignment.py Outdated
Comment thread tests/unittest/test_gitlab_reviewer_assignment.py Outdated
Comment thread pr_agent/servers/gitlab_webhook.py
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 12, 2026

Persistent review updated to latest commit b00b975

@igoraj igoraj force-pushed the feature/gitlab-reviewer-assignment-trigger branch from b00b975 to 779a192 Compare May 12, 2026 11:02
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 12, 2026

Persistent review updated to latest commit 779a192

Comment thread pr_agent/servers/gitlab_webhook.py
@igoraj igoraj force-pushed the feature/gitlab-reviewer-assignment-trigger branch from 779a192 to c47b201 Compare May 12, 2026 11:17
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 12, 2026

Persistent review updated to latest commit c47b201

Comment thread pr_agent/servers/gitlab_webhook.py Outdated
Comment thread tests/unittest/test_gitlab_reviewer_assignment.py
@igoraj igoraj force-pushed the feature/gitlab-reviewer-assignment-trigger branch from c47b201 to 58c4c52 Compare May 12, 2026 11:39
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 12, 2026

Persistent review updated to latest commit 58c4c52

Comment thread tests/unittest/test_gitlab_reviewer_assignment.py
@igoraj igoraj force-pushed the feature/gitlab-reviewer-assignment-trigger branch from 58c4c52 to c110e50 Compare May 12, 2026 11:57
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 12, 2026

Persistent review updated to latest commit c110e50

Comment thread pr_agent/servers/gitlab_webhook.py
@igoraj igoraj force-pushed the feature/gitlab-reviewer-assignment-trigger branch from c110e50 to 959aacd Compare May 12, 2026 12:04
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 12, 2026

Persistent review updated to latest commit 959aacd

@igoraj igoraj force-pushed the feature/gitlab-reviewer-assignment-trigger branch from 959aacd to c950ba8 Compare May 12, 2026 12:17
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 12, 2026

Persistent review updated to latest commit c950ba8

…b MR

Add support for detecting when the PR-Agent bot is assigned as a
reviewer on a GitLab merge request via the webhook payload, and
automatically running configured commands (e.g. /review) in response.

- Add handle_reviewer_assignment toggle and reviewer_commands config
  under [gitlab] in configuration.toml (disabled by default)
- Add is_bot_assigned_as_reviewer() to parse changes.reviewers from
  the webhook payload and detect initial bot assignment
- Add _get_bot_user_id() to auto-resolve the bot's GitLab user ID via
  API, with per-process caching
- Add new dispatch branch in the merge_request event handler for
  reviewer-assignment updates
- Call apply_repo_settings() before reading the toggle, enabling
  per-project configuration via .pr_agent.toml

Assisted-by: opencode:deepseek-v4-pro
@igoraj igoraj force-pushed the feature/gitlab-reviewer-assignment-trigger branch from c950ba8 to f54d0b9 Compare May 12, 2026 12:28
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 12, 2026

Persistent review updated to latest commit f54d0b9

Comment on lines +357 to +376
apply_repo_settings(url)
handle_assignment = get_settings().gitlab.get("handle_reviewer_assignment", False)
if isinstance(handle_assignment, str):
handle_assignment = handle_assignment.lower() in ("true", "1", "yes")
if not handle_assignment:
return JSONResponse(status_code=status.HTTP_200_OK,
content=jsonable_encoder({"message": "success"}))

# Check PR logic after applying repo settings
if not should_process_pr_logic(data):
return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"}))

if is_draft(data):
get_logger().info(f"Skipping draft MR reviewer assignment: {url}")
return JSONResponse(status_code=status.HTTP_200_OK,
content=jsonable_encoder({"message": "success"}))
if await is_bot_assigned_as_reviewer(data):
reviewer_commands = get_settings().gitlab.get("reviewer_commands", [])
if not isinstance(reviewer_commands, list) or not all(isinstance(c, str) for c in reviewer_commands):
get_logger().warning("gitlab.reviewer_commands is not a list of strings, skipping")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Reviewer toggle key casing 🐞 Bug ≡ Correctness

The reviewer-assignment handler reads handle_reviewer_assignment/reviewer_commands via
get_settings().gitlab.get("…") using lowercase keys, which can return defaults when Dynaconf
normalizes stored keys to uppercase. This can prevent the feature from enabling (or from reading the
configured commands) even when configured via file/env/secrets.
Agent Prompt
### Issue description
The reviewer-assignment branch retrieves settings via `get_settings().gitlab.get("handle_reviewer_assignment")` and `get_settings().gitlab.get("reviewer_commands")`. In this codebase, settings are commonly accessed using `get_settings().get("GITLAB.X")` / `get_settings().get("gitlab.x")`, and other code paths explicitly normalize keys to uppercase, so the lowercase `.gitlab.get()` lookups can miss the configured values.

### Issue Context
This can break the new feature in real deployments (especially when configured via env/secrets), because the toggle/commands may be read as defaults.

### Fix Focus Areas
- pr_agent/servers/gitlab_webhook.py[357-376]

### Proposed fix
- Replace:
  - `get_settings().gitlab.get("handle_reviewer_assignment", False)`
  - `get_settings().gitlab.get("reviewer_commands", [])`
  with a single canonical accessor, e.g.:
  - `handle_assignment = get_settings().get("gitlab.handle_reviewer_assignment", False)`
  - `reviewer_commands = get_settings().get("gitlab.reviewer_commands", [])`
  (or the equivalent `GITLAB.HANDLE_REVIEWER_ASSIGNMENT`/`GITLAB.REVIEWER_COMMANDS` form).
- Keep the existing string-to-bool coercion for `handle_assignment` after reading the value.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant