fix(gitlab): add null check for target_file in publish_code_suggestions

[gvago]

Summary

• In GitLabProvider.publish_code_suggestions(), target_file is initialized to None and set only if a matching file is found in the diff. If no match is found, the code proceeds to access target_file.head_file, causing an AttributeError.
• Added a None check after the file-search loop: if target_file is None, log a warning and continue to skip that suggestion gracefully.
• This follows defensive input validation (null check before dereference) and preserves the existing behavior of publishing remaining suggestions even when one fails.

Test plan

• Verify that when a suggestion references a file not present in the diff, the suggestion is skipped with a warning log instead of crashing
• Verify that valid suggestions for files present in the diff are still published normally
• Verify no regression in the existing publish_code_suggestions flow for GitLab MRs

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Add null check for target_file in GitLab publish_code_suggestions

🐞 Bug fix

Walkthroughs

Description

• Add null check for target_file before dereferencing in GitLab provider
• Prevents AttributeError when suggestion file not found in diff
• Logs warning and skips suggestion gracefully instead of crashing
• Allows remaining suggestions to be published successfully

Diagram

flowchart LR
  A["Suggestion file lookup"] --> B{"target_file found?"}
  B -->|Yes| C["Process suggestion"]
  B -->|No| D["Log warning and skip"]
  C --> E["Publish suggestion"]
  D --> F["Continue to next"]

Loading

File Changes

1. pr_agent/git_providers/gitlab_provider.py 🐞 Bug fix +3/-0

Add null check for missing suggestion files

• Added null check after file-search loop to validate target_file is not None
• Logs warning message when suggestion file is not found in diff
• Skips suggestion with continue statement to prevent AttributeError on target_file.head_file
 access
• Preserves existing behavior for valid suggestions while handling missing files gracefully

pr_agent/git_providers/gitlab_provider.py

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)

🐞\ ⛨ Security (1)

1. Unsanitized filename in logs 🐞 ⛨

Description

The new warning logs relevant_file directly; if it contains newline/tab characters it can produce
multi-line/forged log entries and break log parsing. This is avoidable by sanitizing/escaping the
filename (or logging with %r/structured fields) before emitting the warning.

Code

pr_agent/git_providers/gitlab_provider.py[R676-678]

+                if target_file is None:
+                    get_logger().warning(f"Skipping suggestion: file '{relevant_file}' not found in diff")
+                    continue

Evidence

publish_code_suggestions() takes relevant_file directly from the suggestion dict and the PR adds
a warning that interpolates it into the log message without escaping. The repo has tests
demonstrating relevant_file values can include trailing newlines, and other GitLabProvider code
paths already strip/normalize relevant_file, indicating this input can be messy and should be
sanitized before use/logging.

pr_agent/git_providers/gitlab_provider.py[657-683]
tests/unittest/test_try_fix_yaml.py[45-60]
pr_agent/git_providers/gitlab_provider.py[936-940]

Agent prompt

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

### Issue description
A new warning logs `relevant_file` directly. If `relevant_file` contains `\n`/`\t` (or other control characters), the emitted log entry can become multi-line and allow log-forging / break log parsing.

### Issue Context
`relevant_file` originates from suggestion payloads. There are unit tests showing `relevant_file` values can include trailing newlines, and other code in `GitLabProvider` already strips/normalizes `relevant_file`.

### Fix Focus Areas
- pr_agent/git_providers/gitlab_provider.py[664-678]

### Implementation notes
- Normalize `relevant_file` once near assignment (e.g., `relevant_file = str(...).strip('`').strip("'").strip()`), and use the normalized value for both matching and logging.
- For logging, avoid raw interpolation; prefer either:
 - `get_logger().warning("Skipping suggestion: file %r not found in diff", relevant_file)` (so control chars are escaped in repr), or
 - a sanitized version for logs (e.g., replace `\n` with `\\n`, `\t` with `\\t`), or
 - structured logging fields if supported by `get_logger()` in this repo.

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

---

ⓘ The new review experience is currently in Beta. Learn more