From ca7f82a5e5ed5670d8b82db2fa5c99ce924a0b4a Mon Sep 17 00:00:00 2001 From: kolaente Date: Thu, 9 Apr 2026 11:10:24 +0200 Subject: [PATCH] fix(webhook): order matching webhooks by id for deterministic fan-out Previously WebhookListener.Handle fetched matching webhooks with Find(&ws) without an explicit ORDER BY, so iteration order depended on the DB driver. Add ORDER BY id ASC so the fan-out order is stable for both project- and user-level webhooks, and update the sibling-blocking regression test to insert webhooks with explicit ids so its ordering assumption is robust to autoincrement state. --- pkg/e2etests/webhook_multi_test.go | 10 +++++++--- pkg/models/listeners.go | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/e2etests/webhook_multi_test.go b/pkg/e2etests/webhook_multi_test.go index 12df29eac..2a779b9c6 100644 --- a/pkg/e2etests/webhook_multi_test.go +++ b/pkg/e2etests/webhook_multi_test.go @@ -69,15 +69,18 @@ func TestWebhookFailingSiblingDoesNotBlockOthers(t *testing.T) { defer bad.Close() // Clean slate + insert two webhooks listening on task.updated for project 1. - // Insert bad FIRST so it has the lower id and would be iterated before good. - // Delete the fixture webhook id=1 (example.com target) first so it - // does not pollute this test with unrelated delivery failures. + // Use explicit ids so the bad webhook (id=10) is strictly ordered before + // the good one (id=11) once WebhookListener.Handle applies its ORDER BY id. + // This makes the test independent of auto-increment state and DB insert + // ordering. Delete the fixture webhook id=1 (example.com target) first so + // it does not pollute this test with unrelated delivery failures. require.NoError(t, db.LoadFixtures()) s := db.NewSession() defer s.Close() _, err = s.Where("id = ?", 1).Delete(&models.Webhook{}) require.NoError(t, err) _, err = s.Insert(&models.Webhook{ + ID: 10, TargetURL: bad.URL, Events: []string{"task.updated"}, ProjectID: 1, @@ -85,6 +88,7 @@ func TestWebhookFailingSiblingDoesNotBlockOthers(t *testing.T) { }) require.NoError(t, err) _, err = s.Insert(&models.Webhook{ + ID: 11, TargetURL: good.URL, Events: []string{"task.updated"}, ProjectID: 1, diff --git a/pkg/models/listeners.go b/pkg/models/listeners.go index fa97fa600..86707c94e 100644 --- a/pkg/models/listeners.go +++ b/pkg/models/listeners.go @@ -1121,6 +1121,7 @@ func (wl *WebhookListener) Handle(msg *message.Message) (err error) { ws := []*Webhook{} err = s.In("project_id", projectIDs). + OrderBy("id ASC"). Find(&ws) if err != nil { return err @@ -1142,6 +1143,7 @@ func (wl *WebhookListener) Handle(msg *message.Message) (err error) { if userID > 0 { userWebhooks := []*Webhook{} err = s.Where("user_id = ? AND (project_id IS NULL OR project_id = 0)", userID). + OrderBy("id ASC"). Find(&userWebhooks) if err != nil { return err