feat: gitlab skip source, target and labels

[paolomainardi]

User description

Related to #1190

This is a first attempt to support on the gitlab server the ability to skip automatic commands for source, target branches and labels.

---

PR Type

Enhancement

---

Description

• Implemented new features to ignore PRs/MRs based on title, labels, source branches, and target branches for both GitHub and GitLab
• Added configuration options in configuration.toml to support these new ignore features
• Updated GitHub and GitLab webhook handlers to check for ignore conditions before processing PRs/MRs
• Added comprehensive documentation explaining how to use the new ignore features
• Moved some configuration settings to a common section for better organization
• Standardized the implementation across GitHub and GitLab platforms

---

Changes walkthrough 📝

	Relevant files
Enhancement	github_app.py Enhance GitHub PR filtering capabilities                                  pr_agent/servers/github_app.py Moved configuration for ignoring PR titles to a common config section Added logic to ignore PRs with specific labels, source branches, or target branches Implemented checks for these new ignore conditions +24/-1    gitlab_webhook.py Implement GitLab MR filtering features                                      pr_agent/servers/gitlab_webhook.py Added logic to ignore merge requests based on title, labels, source branches, and target branches Implemented checks for these new ignore conditions Imported 're' module for regex operations +32/-1
Documentation	additional_configurations.md Document new PR/MR ignore configurations                                  docs/docs/usage-guide/additional_configurations.md Added documentation for new PR/MR ignoring features Provided examples of how to configure ignore settings in the configuration.toml file +33/-0
Configuration changes	configuration.toml Update configuration with new ignore options                          pr_agent/settings/configuration.toml Added new configuration options for ignoring PRs/MRs based on title, labels, source branches, and target branches Moved some GitHub-specific settings to a common config section Added similar ignore settings for GitLab +18/-2

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[qodo-merge-pro]

PR Reviewer Guide 🔍

(Review updated until commit d2a744e)

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🏅 Score: 85

🧪 No relevant tests

🔒 No security concerns identified

🔀 Multiple PR themes Sub-PR theme: Implement PR/MR filtering for GitHub and GitLab Relevant files: pr_agent/servers/github_app.py pr_agent/servers/gitlab_webhook.py Sub-PR theme: Update configuration and documentation for PR/MR filtering Relevant files: docs/docs/usage-guide/additional_configurations.md pr_agent/settings/configuration.toml

⚡ Key issues to review Code Duplication The logic for ignoring PRs based on title, labels, source branches, and target branches is duplicated in both GitHub and GitLab server files. Consider extracting this logic into a shared utility function to improve maintainability. Inconsistent Configuration Keys The configuration keys used for GitLab (e.g., ignore_mr_title) are different from those used for GitHub (e.g., ignore_pr_title). This inconsistency might lead to confusion and maintenance issues. Consider unifying the configuration keys across both platforms.

[mrT23]

This PR in its current state is far from being considered to merge, for several reasons:

1. The need is not strong enough. I have not encountered requests from users for this specific ability (ignore branches), and I don't think there is a real need for this specific degree-of-freedom. More parameters create overload, and each one needs solid justification
2. Its gitlab only. New abilities, if possible, should be for all providers.
3. undocumented. Adding new params to configuration Should always be accompanied by proper documentation, and cannot be reviewed without it.

What can be done instead:

• Currently, there is some semi-maintained parameter, only for github app: GITHUB_APP.IGNORE_PR_TITLE
  https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/servers/github_app.py#L141
• This parameter in practice is quite useful (for example, ignoring PRs that contain [bump], or other keywords that symbolize something without real content).
• I would consider a PR for upgrading IGNORE_PR_TITLE:
  • moving it to the config
  • implementing it also for gitlab and bitbucket
  • documenting it

[paolomainardi]

Thanks for your review, but I was pretty sure of the answer.

Let me reply to each point:

1. I think excluding branches is helpful in many cases, such as merging from one branch to another or releasing to the main. Labels can also exclude MR opened by bots, such as RenovateBot. I don't think I am the only one who needs this; they are common scenarios.

We are discussing three parameters and two lines of code to handle them; it would be more or less the same for the other providers.

2. and 3. This is right, but the intention was to start discussing it first and then spend some more time on it after an initial discussion.

I instead noticed that the GitHub provider can automatically skip "bot" PRs. This isn't something that the Gitlab server handles. It would be helpful to add it, too, and maybe it can partially reduce the need for this MR, just partially.

I'll look into the title stuff, but not in this PR, as they are unrelated.

[paolomainardi]

Ok, I found where Github handles the bot users: https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/servers/github_app.py#L263 (anyway this is dead configuration), we cannot do the same on Gitlab has the concept of bot is not represented in any form in the webhook events, it should be just guessed: https://docs.gitlab.com/ee/user/project/integrations/webhook_events.html#merge-request-events

So this means not having a way on Gitlab to exclude RenovateBot automatic MR, the only way would be to exclude the the source branch dependencies/* or the label dependencies always added.

Could this be a good justification to implement it?

[paolomainardi]

/describe

[paolomainardi]

/review

[paolomainardi]

/improve

[qodo-merge-pro]

PR Description updated to latest commit (adce357)

• Copy walkthrough table to "Files Changed" Tab

[qodo-merge-pro]

Persistent review updated to latest commit adce357

[paolomainardi]

@mrT23, just in case, I made some improvements:

1. Implemented the same logic for Github to skip source, target (list of regexp), labels
2. Added to Gitlab the title skip feature

[mrT23]

@paolomainardi
ok, this is an improvement. To be able to merge the PR, make the following changes:

1. remove the 'label'. it's not useful, and also bug-prone (some users define labels only after the PR is opened, and its a race condition)
2. move the 3 new parameters to a unified [config] section, since they are the same for all platforms (I will later apply them later also to bitbucket)
3. add 2-3 lines documenting the params in:
   https://pr-agent-docs.codium.ai/usage-guide/additional_configurations/
   (https://github.com/Codium-ai/pr-agent/blob/main/docs/docs/usage-guide/additional_configurations.md)

thanks for the contribution

[paolomainardi]

Thanks a lot, @mrT23. I just pushed the requested changes but labels, which I pray you keep as they are very useful. Automated systems like RenovateBot may open PRs with attached labels that can be easier to identify instead of using regexp titles.

Just as a reference, this is how I am currently using the feature:

config__ignore_pr_source_branches="['develop', 'main', 'master', 'stage']"
config__ignore_pr_labels="['dependencies']"
config__ignore_pr_title="['\\[AUTO\\]', '\\[SKIP-PR-AGENT\\]']"

[paolomainardi]

/describe

[paolomainardi]

/review

[paolomainardi]

/improve

[qodo-merge-pro]

PR Code Suggestions ✨

Latest suggestions up to d2a744e

Category	Suggestion	Score
Maintainability	Extract the MR ignore logic into a separate function for better code organization Consider extracting the logic for checking and ignoring MRs based on various criteria into a separate function to improve code readability and maintainability. pr_agent/servers/gitlab_webhook.py [143-165] -if ignore_mr_source_branches: - source_branch = data['object_attributes'].get('source_branch') - if any(re.search(regex, source_branch) for regex in ignore_mr_source_branches): - get_logger().info(f"Ignoring MR with source branch '{source_branch}' due to gitlab.ignore_mr_source_branches settings") - return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) +def should_ignore_mr(data, title, ignore_mr_source_branches, ignore_mr_target_branches, ignore_mr_labels, ignore_mr_title): + if ignore_mr_source_branches: + source_branch = data['object_attributes'].get('source_branch') + if any(re.search(regex, source_branch) for regex in ignore_mr_source_branches): + get_logger().info(f"Ignoring MR with source branch '{source_branch}' due to gitlab.ignore_mr_source_branches settings") + return True -if ignore_mr_target_branches: - target_branch = data['object_attributes'].get('target_branch') - if any(re.search(regex, target_branch) for regex in ignore_mr_target_branches): - get_logger().info(f"Ignoring MR with target branch '{target_branch}' due to gitlab.ignore_mr_target_branches settings") - return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) + if ignore_mr_target_branches: + target_branch = data['object_attributes'].get('target_branch') + if any(re.search(regex, target_branch) for regex in ignore_mr_target_branches): + get_logger().info(f"Ignoring MR with target branch '{target_branch}' due to gitlab.ignore_mr_target_branches settings") + return True -if ignore_mr_labels: - labels = [label['title'] for label in data['object_attributes'].get('labels', [])] - if any(label in ignore_mr_labels for label in labels): - labels_str = ", ".join(labels) - get_logger().info(f"Ignoring MR with labels '{labels_str}' due to gitlab.ignore_mr_labels settings") - return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) + if ignore_mr_labels: + labels = [label['title'] for label in data['object_attributes'].get('labels', [])] + if any(label in ignore_mr_labels for label in labels): + labels_str = ", ".join(labels) + get_logger().info(f"Ignoring MR with labels '{labels_str}' due to gitlab.ignore_mr_labels settings") + return True -if ignore_mr_title: - if any(re.search(regex, title) for regex in ignore_mr_title): - get_logger().info(f"Ignoring MR with title '{title}' due to gitlab.ignore_mr_title settings") - return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) + if ignore_mr_title: + if any(re.search(regex, title) for regex in ignore_mr_title): + get_logger().info(f"Ignoring MR with title '{title}' due to gitlab.ignore_mr_title settings") + return True + return False + +if should_ignore_mr(data, title, ignore_mr_source_branches, ignore_mr_target_branches, ignore_mr_labels, ignore_mr_title): + return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) + Apply this suggestion Suggestion importance[1-10]: 8 Why: This suggestion significantly improves code organization and maintainability by extracting repetitive logic into a separate function.	8
	Use a more descriptive variable name for clarity Consider using a more descriptive variable name instead of regex. For example, title_pattern would be more clear about what the regex is matching against. pr_agent/servers/github_app.py [145] -if ignore_pr_title_re and any(re.search(regex, title) for regex in ignore_pr_title_re): +if ignore_pr_title_re and any(re.search(title_pattern, title) for title_pattern in ignore_pr_title_re): Apply this suggestion Suggestion importance[1-10]: 5 Why: The suggestion improves code readability by using a more descriptive variable name, but it's a minor change that doesn't significantly impact functionality.	5
	Use more descriptive variable names in loops for better code readability Consider using a more descriptive variable name instead of regex in the loop checking source and target branches. For example, branch_pattern would be more clear about what the regex is matching against. pr_agent/servers/github_app.py [164-169] -if any(re.search(regex, source_branch) for regex in ignore_pr_source_branches): +if any(re.search(branch_pattern, source_branch) for branch_pattern in ignore_pr_source_branches): get_logger().info(f"Ignoring PR with source branch '{source_branch}' due to github_app.ignore_pr_source_branches settings") return {} -if any(re.search(regex, target_branch) for regex in ignore_pr_target_branches): +if any(re.search(branch_pattern, target_branch) for branch_pattern in ignore_pr_target_branches): get_logger().info(f"Ignoring PR with target branch '{target_branch}' due to github_app.ignore_pr_target_branches settings") return {} Apply this suggestion Suggestion importance[1-10]: 5 Why: Similar to the first suggestion, this improves code readability with more descriptive variable names, but it's a minor change with limited impact on overall code quality.	5
Best practice	Use consistent naming convention for configuration keys Consider using a consistent naming convention for the configuration keys. Currently, some keys use "MR" (Merge Request) while others use "PR" (Pull Request). It would be more consistent to use either "MR" or "PR" throughout the code. pr_agent/servers/gitlab_webhook.py [139-142] -ignore_mr_title = get_settings().get("CONFIG.IGNORE_PR_TITLE", []) -ignore_mr_labels = get_settings().get("CONFIG.IGNORE_PR_LABELS", []) -ignore_mr_source_branches = get_settings().get("CONFIG.IGNORE_PR_SOURCE_BRANCHES", []) -ignore_mr_target_branches = get_settings().get("CONFIG.IGNORE_PR_TARGET_BRANCHES", []) +ignore_mr_title = get_settings().get("CONFIG.IGNORE_MR_TITLE", []) +ignore_mr_labels = get_settings().get("CONFIG.IGNORE_MR_LABELS", []) +ignore_mr_source_branches = get_settings().get("CONFIG.IGNORE_MR_SOURCE_BRANCHES", []) +ignore_mr_target_branches = get_settings().get("CONFIG.IGNORE_MR_TARGET_BRANCHES", []) Apply this suggestion Suggestion importance[1-10]: 7 Why: Consistency in naming conventions is important for code maintainability and readability. This suggestion addresses a meaningful inconsistency in the codebase.	7

---

Previous suggestions

✅ Suggestions up to commit adce357

Category	Suggestion	Score
Maintainability	Extract ignore logic into a separate function to improve code organization and readability Consider extracting the ignore logic for source branches, target branches, labels, and titles into separate functions to improve code readability and maintainability. pr_agent/servers/gitlab_webhook.py [146-168] -if ignore_mr_source_branches: - source_branch = data['object_attributes'].get('source_branch') - if any(re.search(regex, source_branch) for regex in ignore_mr_source_branches): - get_logger().info(f"Ignoring MR with source branch '{source_branch}' due to gitlab.ignore_mr_source_branches settings") - return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) +def should_ignore_mr(data, title, ignore_mr_source_branches, ignore_mr_target_branches, ignore_mr_labels, ignore_mr_title): + if ignore_mr_source_branches: + source_branch = data['object_attributes'].get('source_branch') + if any(re.search(regex, source_branch) for regex in ignore_mr_source_branches): + get_logger().info(f"Ignoring MR with source branch '{source_branch}' due to gitlab.ignore_mr_source_branches settings") + return True -if ignore_mr_target_branches: - target_branch = data['object_attributes'].get('target_branch') - if any(re.search(regex, target_branch) for regex in ignore_mr_target_branches): - get_logger().info(f"Ignoring MR with target branch '{target_branch}' due to gitlab.ignore_mr_target_branches settings") - return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) + if ignore_mr_target_branches: + target_branch = data['object_attributes'].get('target_branch') + if any(re.search(regex, target_branch) for regex in ignore_mr_target_branches): + get_logger().info(f"Ignoring MR with target branch '{target_branch}' due to gitlab.ignore_mr_target_branches settings") + return True -if ignore_mr_labels: - labels = [label['title'] for label in data['object_attributes'].get('labels', [])] - if any(label in ignore_mr_labels for label in labels): - labels_str = ", ".join(labels) - get_logger().info(f"Ignoring MR with labels '{labels_str}' due to gitlab.ignore_mr_labels settings") - return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) + if ignore_mr_labels: + labels = [label['title'] for label in data['object_attributes'].get('labels', [])] + if any(label in ignore_mr_labels for label in labels): + labels_str = ", ".join(labels) + get_logger().info(f"Ignoring MR with labels '{labels_str}' due to gitlab.ignore_mr_labels settings") + return True -if ignore_mr_title: - if any([re.search(regex, title) for regex in ignore_mr_title]): - get_logger().info(f"Ignoring MR with title '{title}' due to gitlab.ignore_mr_title settings") - return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) + if ignore_mr_title: + if any([re.search(regex, title) for regex in ignore_mr_title]): + get_logger().info(f"Ignoring MR with title '{title}' due to gitlab.ignore_mr_title settings") + return True + return False + +if should_ignore_mr(data, title, ignore_mr_source_branches, ignore_mr_target_branches, ignore_mr_labels, ignore_mr_title): + return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) + Suggestion importance[1-10]: 8 Why: This suggestion significantly improves code organization, maintainability, and readability by extracting repetitive logic into a separate function.	8
Consistency	✅ Standardize the approach for checking regex matches across different ignore conditions Suggestion Impact: The commit directly implemented the suggested change by removing the square brackets in the list comprehension for the ignore_mr_title check, making it consistent with other similar checks in the code. code diff: - if any([re.search(regex, title) for regex in ignore_mr_title]): + if any(re.search(regex, title) for regex in ignore_mr_title): Consider using a consistent approach for checking regex matches. Currently, re.search() is used for branches, while a list comprehension with re.search() is used for titles. Standardizing this approach can improve code consistency and readability. pr_agent/servers/gitlab_webhook.py [148-168] if any(re.search(regex, source_branch) for regex in ignore_mr_source_branches): get_logger().info(f"Ignoring MR with source branch '{source_branch}' due to gitlab.ignore_mr_source_branches settings") return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) if any(re.search(regex, target_branch) for regex in ignore_mr_target_branches): get_logger().info(f"Ignoring MR with target branch '{target_branch}' due to gitlab.ignore_mr_target_branches settings") return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) ... -if any([re.search(regex, title) for regex in ignore_mr_title]): +if any(re.search(regex, title) for regex in ignore_mr_title): get_logger().info(f"Ignoring MR with title '{title}' due to gitlab.ignore_mr_title settings") return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) Suggestion importance[1-10]: 7 Why: The suggestion improves code consistency and readability by standardizing the approach for regex matching, which is a good practice but not a critical issue.	7
Enhancement	Use a list comprehension to improve readability and potentially performance when checking for matching labels Consider using a list comprehension instead of a for loop with any() for better readability and potentially improved performance when checking if any label matches the ignore list. pr_agent/servers/github_app.py [154-159] if ignore_pr_labels: labels = [label['name'] for label in pull_request.get("labels", [])] - if any(label in ignore_pr_labels for label in labels): - labels_str = ", ".join(labels) + matching_labels = [label for label in labels if label in ignore_pr_labels] + if matching_labels: + labels_str = ", ".join(matching_labels) get_logger().info(f"Ignoring PR with labels '{labels_str}' due to github_app.ignore_pr_labels settings") return {} Suggestion importance[1-10]: 6 Why: The suggestion improves code readability and potentially performance, but the improvement is minor and doesn't address a critical issue.	6

✅ Suggestions up to commit 23af1af

Category	Suggestion	Score
Maintainability	Combine checks for excluded branches and labels to reduce code duplication Consider combining the checks for excluded branches and labels into a single if statement to reduce code duplication and improve readability. pr_agent/servers/gitlab_webhook.py [138-144] -if target_branch in excluded_target_branches or source_branch in excluded_source_branches: - get_logger().info(f"Skipping excluded branch MR: {url}") +if (target_branch in excluded_target_branches or + source_branch in excluded_source_branches or + labels.intersection(excluded_labels)): + reason = "branch" if target_branch in excluded_target_branches or source_branch in excluded_source_branches else "label" + get_logger().info(f"Skipping excluded {reason} MR: {url}") return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) -if labels.intersection(excluded_labels): - get_logger().info(f"Skipping excluded label MR: {url}") - return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) - Suggestion importance[1-10]: 7 Why: This suggestion improves code maintainability by reducing duplication and enhancing readability. It's a good refactoring that makes the code more concise and easier to maintain.	7
Performance	Use a set for excluded labels to improve intersection performance Consider using a set instead of a list for excluded_labels to improve the performance of the intersection operation. pr_agent/servers/gitlab_webhook.py [129-142] -excluded_labels = get_settings().get("gitlab.excluded_labels", []) +excluded_labels = set(get_settings().get("gitlab.excluded_labels", [])) ... -if labels.intersection(excluded_labels): +if set(labels) & excluded_labels: Suggestion importance[1-10]: 6 Why: Using a set for excluded_labels can improve performance for intersection operations, especially for larger lists. However, the improvement might be negligible for small lists.	6
	✅ Use any() with a generator expression for more efficient label checking Suggestion Impact: The commit implemented the suggestion by using any() with a generator expression for checking excluded labels, although with slightly different variable names and structure. code diff: + labels = [label['title'] for label in data['object_attributes'].get('labels', [])] + if any(label in ignore_mr_labels for label in labels): + labels_str = ", ".join(labels) + get_logger().info(f"Ignoring MR with labels '{labels_str}' due to gitlab.ignore_mr_labels settings") + return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) Consider using any() with a generator expression instead of intersection() for checking excluded labels, as it can be more efficient for small lists. pr_agent/servers/gitlab_webhook.py [142] -if labels.intersection(excluded_labels): +if any(label in excluded_labels for label in labels): Suggestion importance[1-10]: 5 Why: Using any() with a generator expression can be more efficient for small lists, but the performance difference may be minimal. It's a valid optimization but not crucial.	5

[qodo-merge-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

Persistent review updated to latest commit d2a744e

[qodo-merge-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Description updated to latest commit (d2a744e)

• Copy walkthrough table to "Files Changed" Tab

[mrT23]

@paolomainardi remove this
I want to have unified params, via the [config]. they are easier to maintain, track an use.
This is a deal breaker.

look how long and cumbersome are the current instructions

[paolomainardi]

Ooops, it's a leftover, I'll remove it.

[mrT23]

ok, i will merge, but i will later remove 'labels' from the instructions.
Its just a plain mistake - all the bots will compete against PR-agent, and their feedback will probably won't be available at the beginning of the PR, when PR-Agent pulls the PR. So problems and race conditions are almost guaranteed.

[paolomainardi]

Thanks for merging @mrT23.

Still don't understand what's the point with labels, the idea behind this is that a PR is opened with certain labels, it will be skipped, exactly as with the title.

Why it should be confusing or misleading ? Genuinely curious, maybe I am missing something.