From 8ca562ca63db44085a82392ae544da3bea8954f2 Mon Sep 17 00:00:00 2001 From: RealDiligent Date: Sun, 26 Jul 2026 21:38:10 +0800 Subject: [PATCH] fix(db): add a monotonic tiebreak to listNotificationDeliveriesForRecipient's ORDER BY listNotificationDeliveriesForRecipient ordered only by `createdAt DESC`. A fan-out (e.g. issue_watch_match to many miners) stamps createdAt via nowIso() per row, so several deliveries can tie on the millisecond timestamp, leaving display order and the limit-boundary row engine-defined. Add `desc(notificationDeliveries.id)` as a deterministic secondary sort, matching the ai_review_cache tiebreak fix. Closes #8895 Co-Authored-By: Claude Opus 4.8 --- src/db/repositories.ts | 5 ++++- test/unit/notifications-service.test.ts | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 5dc1e44da2..6974960384 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -2250,7 +2250,10 @@ export async function listNotificationDeliveriesForRecipient( .select() .from(notificationDeliveries) .where(and(...conditions)) - .orderBy(desc(notificationDeliveries.createdAt)) + // #8895: a fan-out (e.g. issue_watch_match to many miners) stamps createdAt via nowIso() per row, so + // several deliveries can tie on the millisecond timestamp; the unique text id is a deterministic + // secondary sort so display order and the limit-boundary row are stable, not engine-defined. + .orderBy(desc(notificationDeliveries.createdAt), desc(notificationDeliveries.id)) .limit(Math.min(Math.max(options.limit ?? 50, 1), 100)); return rows.map(toNotificationDeliveryRecord); } diff --git a/test/unit/notifications-service.test.ts b/test/unit/notifications-service.test.ts index 24e48318fe..00bf94a944 100644 --- a/test/unit/notifications-service.test.ts +++ b/test/unit/notifications-service.test.ts @@ -241,6 +241,24 @@ describe("evaluateNotificationEvent", () => { const rows = await listNotificationDeliveriesForRecipient(env, "miner"); expect(rows.find((row) => row.dedupKey === "over-limit")?.status).toBe("suppressed"); }); + + it("returns a deterministic order for deliveries that tie on createdAt (#8895)", async () => { + const env = createTestEnv(); + // Two deliveries stamped with an identical createdAt (a fan-out within one millisecond). Without a + // secondary sort key the tie order is engine-defined; desc(id) makes it deterministic. Controlled text + // ids (inserted a-before-z) prove the query reorders to id-desc rather than echoing insertion order. + const ts = "2026-07-10T00:00:00.000Z"; + const insert = + "INSERT INTO notification_deliveries (id, dedup_key, channel, recipient_login, event_type, repo_full_name, pull_number, title, body, deeplink, actor_login, status, created_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"; + await env.DB.prepare(insert) + .bind("delivery-a", "k-a", "badge", "miner", "pull_request_changes_requested", "owner/repo", 1, "t", "b", "https://x", "reviewer", "delivered", ts) + .run(); + await env.DB.prepare(insert) + .bind("delivery-z", "k-z", "badge", "miner", "pull_request_changes_requested", "owner/repo", 2, "t", "b", "https://x", "reviewer", "delivered", ts) + .run(); + const rows = await listNotificationDeliveriesForRecipient(env, "miner"); + expect(rows.map((row) => row.id)).toEqual(["delivery-z", "delivery-a"]); + }); }); describe("deliverNotification", () => {