diff --git a/src/selfhost/redis-cache.ts b/src/selfhost/redis-cache.ts index b38297ad8d..4b55aeeea0 100644 --- a/src/selfhost/redis-cache.ts +++ b/src/selfhost/redis-cache.ts @@ -27,21 +27,3 @@ export function createRedisCache(redis: Redis) { } export type RedisCache = ReturnType; - -/** - * Idempotency check for GitHub webhook deliveries. Returns true if the delivery was - * already seen (caller should short-circuit with 204). Marks the delivery as seen - * for `ttlSeconds` (default 5 min — covers GitHub's retry window) on the FIRST call. - * Best-effort: a Redis error is swallowed to avoid blocking webhook processing. - */ -export async function checkAndMarkDelivery(cache: RedisCache, deliveryId: string, ttlSeconds = 300): Promise { - try { - const seen = await cache.get(`delivery:${deliveryId}`); - if (seen) return true; - await cache.set(`delivery:${deliveryId}`, "1", ttlSeconds); - return false; - } catch { - // Redis unavailable → treat as first-time (never block processing on cache failure) - return false; - } -} diff --git a/test/unit/selfhost-redis-cache.test.ts b/test/unit/selfhost-redis-cache.test.ts index b4bf3d5afd..f12499dcf7 100644 --- a/test/unit/selfhost-redis-cache.test.ts +++ b/test/unit/selfhost-redis-cache.test.ts @@ -1,6 +1,6 @@ import type { Redis } from "ioredis"; import { describe, expect, it } from "vitest"; -import { checkAndMarkDelivery, createRedisCache } from "../../src/selfhost/redis-cache"; +import { createRedisCache } from "../../src/selfhost/redis-cache"; /** Minimal in-memory stand-in for the ioredis methods the cache uses. Emulates real Redis SET NX * semantics (refuse + return null when NX is requested and the key already exists) so a test @@ -64,38 +64,3 @@ describe("createRedisCache (#1216 webhook dedup cache)", () => { await expect(cache.claim("lock", "1", 60)).rejects.toThrow("connection refused"); }); }); - -describe("checkAndMarkDelivery (#1216 webhook idempotency)", () => { - it("returns false (first-time) for a new delivery ID and marks it as seen", async () => { - const cache = createRedisCache(fakeRedis()); - const result = await checkAndMarkDelivery(cache, "delivery-abc", 300); - expect(result).toBe(false); - // second call with the same ID should be a duplicate - const duplicate = await checkAndMarkDelivery(cache, "delivery-abc", 300); - expect(duplicate).toBe(true); - }); - - it("returns true (duplicate) for an already-seen delivery ID", async () => { - const r = fakeRedis(); - r._store.set("delivery:existing-id", "1"); - const cache = createRedisCache(r); - expect(await checkAndMarkDelivery(cache, "existing-id")).toBe(true); - }); - - it("different delivery IDs are tracked independently", async () => { - const cache = createRedisCache(fakeRedis()); - expect(await checkAndMarkDelivery(cache, "id-A")).toBe(false); - expect(await checkAndMarkDelivery(cache, "id-B")).toBe(false); // different ID → first-time - expect(await checkAndMarkDelivery(cache, "id-A")).toBe(true); // id-A seen before - }); - - it("swallows Redis errors and returns false (never blocks processing)", async () => { - const brokenRedis = { - async get() { throw new Error("connection refused"); }, - async set() { throw new Error("connection refused"); }, - } as unknown as Redis; - const cache = createRedisCache(brokenRedis); - // Must not throw — error is swallowed, returns false (first-time / let it through) - expect(await checkAndMarkDelivery(cache, "any-id")).toBe(false); - }); -});