fix(gitlab): discard label removal events on merge requests#2785
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the GitLab provider's event detection logic to determine if only labels have changed by comparing the length of current and previous labels. It also introduces a helper function and several test cases to verify different label change scenarios. The review feedback points out a critical bug in this implementation: comparing only the slice lengths fails to detect label changes when a label is added and another is removed simultaneously (as the total count remains unchanged). A code suggestion is provided to correctly identify when at least one new label has been added, along with a recommendation to add a corresponding test case for this scenario.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| changes := gitEvent.Changes | ||
|
|
||
| labelsChanged := len(changes.Labels.Previous) > 0 || len(changes.Labels.Current) > 0 | ||
| labelsChanged := len(changes.Labels.Current) > len(changes.Labels.Previous) |
There was a problem hiding this comment.
Comparing only the lengths of the current and previous label slices (len(changes.Labels.Current) > len(changes.Labels.Previous)) introduces a bug when a label is added and another is removed in the same action (e.g., changing a label from status/review to status/approved). In this scenario, the total count of labels remains the same, so labelsChanged incorrectly evaluates to false, and the event is discarded.
To fix this, we should check if there is at least one label in changes.Labels.Current that is not present in changes.Labels.Previous.
| labelsChanged := len(changes.Labels.Current) > len(changes.Labels.Previous) | |
| labelsChanged := false | |
| for _, c := range changes.Labels.Current { | |
| if c == nil { | |
| continue | |
| } | |
| found := false | |
| for _, p := range changes.Labels.Previous { | |
| if p != nil && c.ID == p.ID { | |
| found = true | |
| break | |
| } | |
| } | |
| if !found { | |
| labelsChanged = true | |
| break | |
| } | |
| } |
| name: "good/mergeRequest update Event with label addition", | ||
| event: mrEventWithChanges(sample, `"labels": {"previous": [], "current": [{"id": 1, "title": "bug"}]}`), | ||
| eventType: gitlab.EventTypeMergeRequest, | ||
| isGL: true, | ||
| processReq: true, | ||
| }, |
There was a problem hiding this comment.
Add a test case to verify that simultaneous label addition and removal (where the total label count remains unchanged) is correctly processed as a valid label addition event.
{
name: "good/mergeRequest update Event with label addition",
event: mrEventWithChanges(sample, "\"labels\": {\"previous\": [], \"current\": [{\"id\": 1, \"title\": \"bug\"}]}"),
eventType: gitlab.EventTypeMergeRequest,
isGL: true,
processReq: true,
},
{
name: "good/mergeRequest update Event with label addition and removal simultaneously",
event: mrEventWithChanges(sample, "\"labels\": {\"previous\": [{\"id\": 2, \"title\": \"feature\"}], \"current\": [{\"id\": 1, \"title\": \"bug\"}]}"),
eventType: gitlab.EventTypeMergeRequest,
isGL: true,
processReq: true,
},
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2785 +/- ##
=======================================
Coverage 59.73% 59.73%
=======================================
Files 210 210
Lines 21117 21117
=======================================
Hits 12615 12615
Misses 7707 7707
Partials 795 795 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
why lint rules are forced in this PR 🤔 |
Label removal events on GitLab merge requests were incorrectly triggering pipeline runs. The hasOnlyLabelsChanged check used an OR condition that matched both additions and removals. Changed to compare current vs previous label count so only label additions are processed. Signed-off-by: Zaki Shaikh <zashaikh@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zakisk
force-pushed
the
SRVKP-12428-discard-label-removal-events-gitlab
branch
from
e0d0fff to
494b7fc
Compare
| changes := gitEvent.Changes | ||
|
|
||
| labelsChanged := len(changes.Labels.Previous) > 0 || len(changes.Labels.Current) > 0 | ||
| labelsChanged := len(changes.Labels.Current) > len(changes.Labels.Previous) |
There was a problem hiding this comment.
What if one label is added and one is removed?
Say an MR has label wip and the user removes the wip label and adds ready-for-review, len(labels-previous) equals len(labels-current) in this scenario. Or would this show up as 2 separate events or a single one?
There was a problem hiding this comment.
yeah, it would be single event and this would not work in that scenario, I will update the PR
2 participants
📝 Description of the Change
Label removal events on GitLab merge requests were incorrectly triggering pipeline runs. The hasOnlyLabelsChanged check used an OR condition that matched both additions and removals. Changed to compare current vs previous label count so only label additions are processed.
🔗 Linked GitHub Issue
Fixes #
🧪 Testing Strategy
🤖 AI Assistance
AI assistance can be used for various tasks, such as code generation,
documentation, or testing.
Please indicate whether you have used AI assistance
for this PR and provide details if applicable.
Important
Slop will be simply rejected, if you are using AI assistance you need to make sure you
understand the code generated and that it meets the project's standards. you
need at least know how to run the code and deploy it (if needed). See
startpaac to make it easy
to deploy and test your code changes.
If the majority of the code in this PR was generated by an AI, please add a
Co-authored-bytrailer to your commit message.For example:
Co-authored-by: Claude noreply@anthropic.com
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.