Tr/more protections gitlab

[mrT23]

PR Type

Bug fix, Enhancement

---

Description

• Fixed GitLab provider to use the default branch for repo settings.

• Added protections to prevent GitHub quick actions in GitLab PR questions.

• Enhanced PR answer sanitization for GitLab compatibility.

---

Changes walkthrough 📝

	Relevant files
Bug fix	gitlab_provider.py Use default branch for GitLab repo settings                            pr_agent/git_providers/gitlab_provider.py Changed to use the default branch instead of the target branch. Improved repository settings retrieval logic. +2/-1
Enhancement	pr_questions.py Add GitLab protections and improve answer sanitization      pr_agent/tools/pr_questions.py Added a method to detect and block GitHub quick actions in GitLab. Enhanced PR answer sanitization for GitLab-specific protections. Updated imports to include GitLabProvider. +12/-1

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-for-open-source]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The gitlab_protctions method returns an error string when GitHub quick actions are detected, but doesn't provide a way to recover or continue with a sanitized response. def gitlab_protctions(self, model_answer: str) -> str: github_quick_actions_MR = ["/approve", "/close", "/merge", "/reopen", "/unapprove", "/title", "/assign", "/copy_metadata", "/target_branch"] if any(action in model_answer for action in github_quick_actions_MR): str_err = "Model answer contains GitHub quick actions, which are not supported in GitLab" get_logger().error(str_err) return str_err Typo The method name 'gitlab_protctions' has a typo and should be 'gitlab_protections'. def gitlab_protctions(self, model_answer: str) -> str:

[qodo-merge-for-open-source]

PR Code Suggestions ✨

Latest suggestions up to 7bd0fef

Category	Suggestion	Impact
Possible issue	行の先頭のクイックアクションのみ検出 現在の実装では、GitLabのクイックアクションが文字列内の任意の場所に存在する場合にエラーを返しますが、これは誤検出を引き起こす可能性があります。クイックアクションは行の先頭にある場合のみ有効なので、行の先頭にあるパターンのみをチェックするべきです。 pr_agent/tools/pr_questions.py [119-126] def gitlab_protections(self, model_answer: str) -> str: github_quick_actions_MR = ["/approve", "/close", "/merge", "/reopen", "/unapprove", "/title", "/assign", "/copy_metadata", "/target_branch"] - if any(action in model_answer for action in github_quick_actions_MR): - str_err = "Model answer contains GitHub quick actions, which are not supported in GitLab" - get_logger().error(str_err) - return str_err + lines = model_answer.split('\n') + for line in lines: + if any(line.strip().startswith(action) for action in github_quick_actions_MR): + str_err = "Model answer contains GitHub quick actions, which are not supported in GitLab" + get_logger().error(str_err) + return str_err return model_answer Apply this suggestion Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential bug in the GitLab quick actions detection. The current implementation checks if actions appear anywhere in the text, but GitLab only processes them when they appear at the beginning of a line. The improved implementation properly splits the text into lines and checks each line's start, preventing false positives.	Medium
Learned best practice	Add null safety checks and proper error handling when accessing project attributes to prevent potential runtime errors The code is missing null safety checks for main_branch before using it as a reference. If default_branch is None or if the project retrieval fails, this could lead to runtime errors. Add proper null checks and error handling. pr_agent/git_providers/gitlab_provider.py [518-519] -main_branch = self.gl.projects.get(self.id_project).default_branch -contents = self.gl.projects.get(self.id_project).files.get(file_path='.pr_agent.toml', ref=main_branch).decode() +project = self.gl.projects.get(self.id_project) +if not project or not project.default_branch: + return "" +main_branch = project.default_branch +try: + contents = project.files.get(file_path='.pr_agent.toml', ref=main_branch).decode() +except Exception as e: + get_logger().debug(f"Failed to get .pr_agent.toml: {e}") + return "" Apply this suggestion Suggestion importance[1-10]: 6	Low
More

• Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

---

Previous suggestions

Suggestions up to commit 7bd0fef

Category	Suggestion	Impact
Possible issue	Fix quick action detection The current implementation checks if any quick action string is contained anywhere in the model answer, which could lead to false positives. Instead, check if any line starts with a quick action to properly identify GitLab quick actions. pr_agent/tools/pr_questions.py [119-126] def gitlab_protections(self, model_answer: str) -> str: github_quick_actions_MR = ["/approve", "/close", "/merge", "/reopen", "/unapprove", "/title", "/assign", "/copy_metadata", "/target_branch"] - if any(action in model_answer for action in github_quick_actions_MR): - str_err = "Model answer contains GitHub quick actions, which are not supported in GitLab" - get_logger().error(str_err) - return str_err + lines = model_answer.split('\n') + for line in lines: + if any(line.strip().startswith(action) for action in github_quick_actions_MR): + str_err = "Model answer contains GitHub quick actions, which are not supported in GitLab" + get_logger().error(str_err) + return str_err return model_answer Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential bug in the GitLab quick actions detection. The current implementation checks if actions appear anywhere in the text, which could flag false positives in normal text. The improved code properly checks if lines start with these actions, which is more accurate for detecting actual command usage.	Medium
General	Use specific exception handling The exception handling is too broad. Catching all exceptions can hide unexpected errors and make debugging difficult. Consider catching specific exceptions like gitlab.exceptions.GitlabGetError for missing files or access issues. pr_agent/git_providers/gitlab_provider.py [517-522] try: main_branch = self.gl.projects.get(self.id_project).default_branch contents = self.gl.projects.get(self.id_project).files.get(file_path='.pr_agent.toml', ref=main_branch).decode() return contents -except Exception: +except gitlab.exceptions.GitlabGetError: + return "" +except Exception as e: + get_logger().error(f"Unexpected error retrieving .pr_agent.toml: {str(e)}") return "" Suggestion importance[1-10]: 7 __ Why: The suggestion improves error handling by catching specific exceptions rather than using a broad catch-all. This makes the code more maintainable and helps with debugging by logging unexpected errors while still handling expected failures gracefully.	Medium

✅ Suggestions up to commit 02d9aed

Category	Suggestion	Impact
Possible issue	✅ Fix function name and return Suggestion Impact: The commit implemented both changes suggested: 1) Fixed the function name typo from 'gitlab_protctions' to 'gitlab_protections' in both the function definition and where it's called, and 2) Added the missing return statement to return model_answer when no quick actions are detected code diff: - def gitlab_protctions(self, model_answer: str) -> str: + def gitlab_protections(self, model_answer: str) -> str: github_quick_actions_MR = ["/approve", "/close", "/merge", "/reopen", "/unapprove", "/title", "/assign", "/copy_metadata", "/target_branch"] if any(action in model_answer for action in github_quick_actions_MR): str_err = "Model answer contains GitHub quick actions, which are not supported in GitLab" get_logger().error(str_err) return str_err + return model_answer def _prepare_pr_answer(self) -> str: model_answer = self.prediction.strip() @@ -130,7 +131,7 @@ model_answer_sanitized = model_answer.replace("\n/", "\n /") model_answer_sanitized = model_answer_sanitized.replace("\r/", "\r /") if isinstance(self.git_provider, GitLabProvider): - model_answer_sanitized = self.gitlab_protctions(model_answer_sanitized) + model_answer_sanitized = self.gitlab_protections(model_answer_sanitized) The function has a typo in its name and doesn't handle the case where no quick actions are found. It should return the original model_answer when no quick actions are detected, otherwise it will return None or the error message, causing unexpected behavior. pr_agent/tools/pr_questions.py [119-125] -def gitlab_protctions(self, model_answer: str) -> str: +def gitlab_protections(self, model_answer: str) -> str: github_quick_actions_MR = ["/approve", "/close", "/merge", "/reopen", "/unapprove", "/title", "/assign", "/copy_metadata", "/target_branch"] if any(action in model_answer for action in github_quick_actions_MR): str_err = "Model answer contains GitHub quick actions, which are not supported in GitLab" get_logger().error(str_err) return str_err + return model_answer [Suggestion has been applied] Suggestion importance[1-10]: 9 __ Why: The suggestion fixes two critical issues: a typo in the function name and the missing return statement for the normal case. Without the return statement, the function would return None when no quick actions are found, causing unexpected behavior or potential errors.	High
	✅ Fix function name typo Suggestion Impact: The commit fixed the typo in both the function definition (line 5-6) and the function call (line 21-22), exactly as suggested code diff: - def gitlab_protctions(self, model_answer: str) -> str: + def gitlab_protections(self, model_answer: str) -> str: github_quick_actions_MR = ["/approve", "/close", "/merge", "/reopen", "/unapprove", "/title", "/assign", "/copy_metadata", "/target_branch"] if any(action in model_answer for action in github_quick_actions_MR): str_err = "Model answer contains GitHub quick actions, which are not supported in GitLab" get_logger().error(str_err) return str_err + return model_answer def _prepare_pr_answer(self) -> str: model_answer = self.prediction.strip() @@ -130,7 +131,7 @@ model_answer_sanitized = model_answer.replace("\n/", "\n /") model_answer_sanitized = model_answer_sanitized.replace("\r/", "\r /") if isinstance(self.git_provider, GitLabProvider): - model_answer_sanitized = self.gitlab_protctions(model_answer_sanitized) + model_answer_sanitized = self.gitlab_protections(model_answer_sanitized) The function name has a typo ('protctions' instead of 'protections'). This will cause a runtime error when the code is executed with a GitLab provider. pr_agent/tools/pr_questions.py [132-133] if isinstance(self.git_provider, GitLabProvider): - model_answer_sanitized = self.gitlab_protctions(model_answer_sanitized) + model_answer_sanitized = self.gitlab_protections(model_answer_sanitized) [Suggestion has been applied] Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a critical typo in the function call that would cause a runtime error when executed with a GitLab provider. The function is called as "gitlab_protctions" but defined as "gitlab_protctions", and this mismatch would break the code.	High

[mrT23]

/improve --CONFIG.RESPONSE_LANGUAGE="ja-JP"

[mrT23]

/scan_repo_discussions

[qodo-merge-for-open-source]

🚀 I've created a PR with an auto-generated best practices file:
#1699