Skip to content

fix(servicehooks): Make servicehook updating idempotent#108415

Open
Christinarlong wants to merge 2 commits intomasterfrom
crl/fix-duplicate-servicehooks
Open

fix(servicehooks): Make servicehook updating idempotent#108415
Christinarlong wants to merge 2 commits intomasterfrom
crl/fix-duplicate-servicehooks

Conversation

@Christinarlong
Copy link
Contributor

@Christinarlong Christinarlong commented Feb 17, 2026

Description

Fixes a MultipleObjectsReturned error in create_or_update_webhook_and_events_for_installation caused by duplicate ServiceHook rows for the same (installation_id, application_id) pair.

Due to concurrency issues, some installations ended up with multiple ServiceHook objects. The previous implementation used update_or_create() and get(), which both fail when duplicates exist. This changes the function to a delete-and-recreate pattern, making it fully idempotent regardless of how many hooks exist.

Test info

  • Updated existing test to not assert on hook ID stability (since we now delete and recreate)
  • Added test for duplicate hooks with webhook_url provided — verifies duplicates are cleaned up and exactly one hook remains
  • Added test for duplicate hooks with webhook_url=None — verifies all duplicates are deleted

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 17, 2026
@Christinarlong Christinarlong marked this pull request as ready for review February 18, 2026 00:00
@Christinarlong Christinarlong requested review from a team as code owners February 18, 2026 00:00
Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

Copy link
Member

@GabeVillalobos GabeVillalobos left a comment

Choose a reason for hiding this comment

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

@markstory Any idea why ServiceHook was given an explicit deletion task? The comment dates back to this PR #45656, but it's unclear why we have this behavior.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants