Skip to content

Conversation

@iamrajjoshi
Copy link
Collaborator

@iamrajjoshi iamrajjoshi commented Nov 9, 2025

Problem

When switching assignees on a GitHub issue externally (e.g., changing from @billybobthebot to @iamrajjoshi), Sentry would sometimes end up with the wrong assignee or no assignee at all.

Root cause:

GitHub sends two webhooks when switching assignees:

  • issues.unassigned (for the person being removed)
  • issues.assigned (for the person being added)

These webhooks arrive in non-deterministic order, creating a race condition. The old implementation used the assignee field at the root of the webhook payload, which represents the delta (the person being added/removed), not the current state.

Race condition scenarios:

Scenario 1: assigned arrives first, then unassigned

  1. assigned webhook (assignee=user2) → syncs to user2 ✓
  2. unassigned webhook (assignee=user1) → syncs to user1 ✗

Scenario 2: unassigned arrives first, then assigned

  1. unassigned webhook (assignee=user1) → syncs to user1 ✗
  2. assigned webhook (assignee=user2) → syncs to user2 ✓

In both cases, there's a window where the wrong assignee gets synced, and depending on timing, the final state could be incorrect.

Solution

Use issue.assignees (current state after the action) instead of assignee (the delta) when syncing assignments. This ensures both webhooks converge to the same final state regardless of arrival order.

After fix:

  • Both assigned and unassigned webhooks now look at issue.assignees[]
  • If issue.assignees is empty → deassign
  • If issue.assignees has users → assign to the first user (Sentry only supports single assignee)
  • Both webhooks now produce the same result regardless of order

Note

GitHub supports multiple assignees per issue, but Sentry currently only supports a single assignee per issue, so we take the first user Github returns. It is generally the oldest user to be assigned (based on what I have seen)

Screen.Recording.2025-11-08.at.4.21.02.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.

@iamrajjoshi iamrajjoshi self-assigned this Nov 9, 2025
@iamrajjoshi iamrajjoshi requested a review from a team as a code owner November 9, 2025 00:36
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 9, 2025
@codecov
Copy link

codecov bot commented Nov 9, 2025

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/integrations/github/webhook.py 0.00% 8 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #103008      +/-   ##
===========================================
- Coverage   80.65%    80.65%   -0.01%     
===========================================
  Files        9190      9190              
  Lines      392895    392901       +6     
  Branches    24975     24975              
===========================================
  Hits       316903    316903              
- Misses      75575     75581       +6     
  Partials      417       417              

if not assignees:
sync_group_assignee_inbound_by_external_actor(
integration=integration,
external_user_name="", # Not used for deassignment
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird that this isn't an optional param....

@GabeVillalobos GabeVillalobos added the Trigger: getsentry tests Once code is reviewed: apply label to PR to trigger getsentry tests label Nov 10, 2025
@iamrajjoshi iamrajjoshi force-pushed the raj/fix-gh-assignment-sync branch from 91fea3a to ed9680a Compare November 10, 2025 16:25
@GabeVillalobos GabeVillalobos merged commit 0d9a78b into master Nov 10, 2025
66 checks passed
@GabeVillalobos GabeVillalobos deleted the raj/fix-gh-assignment-sync branch November 10, 2025 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components Trigger: getsentry tests Once code is reviewed: apply label to PR to trigger getsentry tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants