✨ feat(gitlab): add issue sync outbound support#107216
✨ feat(gitlab): add issue sync outbound support#107216iamrajjoshi wants to merge 2 commits intomasterfrom
Conversation
760e657 to
b4293d0
Compare
c582fc5 to
5790fac
Compare
b4293d0 to
5458f2d
Compare
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
5790fac to
778b71d
Compare
5458f2d to
c8dfe02
Compare
c8dfe02 to
e5e81c2
Compare
5afe7f2 to
f492317
Compare
f492317 to
aeb4885
Compare
e5e81c2 to
430424c
Compare
430424c to
dad6dc5
Compare
dad6dc5 to
9c2591b
Compare
Backend Test FailuresFailures on
|
9c2591b to
9424c5b
Compare
Backend Test FailuresFailures on
|
9424c5b to
3f8cf58
Compare
| IntegrationExternalProject.objects.filter( | ||
| organization_integration_id=self.org_integration.id | ||
| ).delete() | ||
|
|
There was a problem hiding this comment.
Bug: The update_organization_config method deletes existing project mappings before validating new data, causing data loss if validation fails, as the operation is not in a transaction.
Severity: HIGH
Suggested Fix
Wrap the delete and create operations within a transaction.atomic() block to ensure that the entire update is rolled back if any part of it fails. Additionally, move all validation logic to execute before any database write operations, such as the delete() call, to prevent unnecessary database changes on invalid input.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/sentry/integrations/gitlab/integration.py#L358-L361
Potential issue: The `update_organization_config` method in the GitLab integration
deletes all existing `IntegrationExternalProject` records before fully validating the
new configuration. Because this operation is not wrapped in a database transaction, if a
validation check fails after the deletion has occurred, the process aborts, resulting in
the permanent loss of all previously configured project mappings. This happens when a
user provides a configuration that passes initial checks but fails later validation,
such as providing an invalid status value like `"invalid_status"` for `on_resolve`.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Search value change breaks existing issue creation flow
- Reverted search endpoint to return numeric project IDs instead of path_with_namespace to maintain backward compatibility with existing issue creation and linking flows.
- ✅ Fixed: Status validation occurs after deleting existing records
- Moved status value validation before the deletion of existing IntegrationExternalProject records to prevent data loss if validation fails.
Or push these changes by commenting:
@cursor push b751f2f38c
Preview (b751f2f38c)
diff --git a/src/sentry/integrations/gitlab/integration.py b/src/sentry/integrations/gitlab/integration.py
--- a/src/sentry/integrations/gitlab/integration.py
+++ b/src/sentry/integrations/gitlab/integration.py
@@ -353,15 +353,9 @@
):
raise IntegrationError("Resolve and unresolve status are required.")
- data["sync_status_forward"] = bool(project_mappings)
-
- IntegrationExternalProject.objects.filter(
- organization_integration_id=self.org_integration.id
- ).delete()
-
+ # Validate status values before deleting existing records
+ valid_statuses = {GitLabIssueStatus.OPENED.value, GitLabIssueStatus.CLOSED.value}
for project_path, statuses in project_mappings.items():
- # Validate status values
- valid_statuses = {GitLabIssueStatus.OPENED.value, GitLabIssueStatus.CLOSED.value}
if statuses["on_resolve"] not in valid_statuses:
raise IntegrationError(
f"Invalid resolve status: {statuses['on_resolve']}. Must be 'opened' or 'closed'."
@@ -371,6 +365,13 @@
f"Invalid unresolve status: {statuses['on_unresolve']}. Must be 'opened' or 'closed'."
)
+ data["sync_status_forward"] = bool(project_mappings)
+
+ IntegrationExternalProject.objects.filter(
+ organization_integration_id=self.org_integration.id
+ ).delete()
+
+ for project_path, statuses in project_mappings.items():
IntegrationExternalProject.objects.create(
organization_integration_id=self.org_integration.id,
external_id=project_path,
diff --git a/src/sentry/integrations/gitlab/search.py b/src/sentry/integrations/gitlab/search.py
--- a/src/sentry/integrations/gitlab/search.py
+++ b/src/sentry/integrations/gitlab/search.py
@@ -79,7 +79,7 @@
[
{
"label": project["name_with_namespace"],
- "value": project["path_with_namespace"],
+ "value": project["id"],
}
for project in response
]This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
| name=project_path, | ||
| resolved_status=statuses["on_resolve"], | ||
| unresolved_status=statuses["on_unresolve"], | ||
| ) |
There was a problem hiding this comment.
Status validation occurs after deleting existing records
Medium Severity
In update_organization_config, existing IntegrationExternalProject records are deleted on line 358–360 before the per-mapping status value validation runs inside the loop on lines 364–372. If an invalid status value (not "opened" or "closed") is submitted for any project mapping, the IntegrationError is raised after all existing records have already been permanently deleted, causing data loss of the previous configuration.
There was a problem hiding this comment.
@iamrajjoshi reminder that we need to wrap these in transactions as a cleanup PR to ensure this isn't a problem.
There was a problem hiding this comment.
Will follow up in a cleanup PR to wrap the delete+create in transaction.atomic().
| name=project_path, | ||
| resolved_status=statuses["on_resolve"], | ||
| unresolved_status=statuses["on_unresolve"], | ||
| ) |
There was a problem hiding this comment.
@iamrajjoshi reminder that we need to wrap these in transactions as a cleanup PR to ensure this isn't a problem.
|
|
||
| # Verify task was NOT called | ||
| mock_task.delay.assert_not_called() | ||
| mock_task.assert_not_called() |
There was a problem hiding this comment.
Instead of mocking the service, should we continue to assert that the task is correctly queued? This presumes that the RPC call is correct instead of testing that the parameters we're passing are correct.
If this is tested elsewhere, can you update the mock names to correctly reflect the change please?
There was a problem hiding this comment.
Renamed mock_task → mock_schedule_webhooks throughout — the variable was a leftover from when we mocked the Celery task directly. The RPC call tests do assert the correct org/integration args on the call sites that trigger it. Happy to discuss if you think we should go deeper and test at the task-queue level instead.
Propagate Sentry issue resolution and assignment to linked GitLab issues. Includes sync_status_outbound and sync_assignee_outbound implementations, status mapping via IntegrationExternalProject, and a schedule_update_gitlab_project_webhooks RPC method. Co-Authored-By: Claude <noreply@anthropic.com>
- URL-encode project_id in client.py to support path_with_namespace values (e.g. getsentry/sentry → getsentry%2Fsentry) when querying GitLab project issues API - Rename mock_task → mock_schedule_webhooks in webhook update tests to accurately reflect that the RPC service method is being mocked, not a Celery task directly Co-Authored-By: Claude <noreply@anthropic.com>
f054e15 to
c0b439e
Compare



Outbound status sync support. It also uses the async repo loading pattern as github.
Configuring a Repo
Screen.Recording.2026-01-28.at.8.35.29.PM.mov
Outbound Status Sync
Screen.Recording.2026-01-28.at.9.00.14.PM.mov
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.