refactor(selfhost): remove the dead checkAndMarkDelivery helper

checkAndMarkDelivery (the documented webhook-delivery-dedup helper in
redis-cache.ts) was exported but never called anywhere in src/. The
real webhook dedup path in server.ts implements its own get-then-set
inline, duplicating the same logic with a deliberate and CORRECT
mark-after-success ordering difference: it marks a delivery ID as seen
only once the response is confirmed ok, so a failed/rejected webhook
stays retryable. checkAndMarkDelivery marked on the FIRST call
regardless of outcome, which would have incorrectly suppressed a retry
of a webhook whose processing actually failed, had anything called it.

Delete it -- the inline version in server.ts is already correct and
live, and two divergent implementations of the same dedup logic can
silently drift out of sync.

Author: JSONbored

src/selfhost/redis-cache.ts

@@ -27,21 +27,3 @@ export function createRedisCache(redis: Redis) {
 }
 
 export type RedisCache = ReturnType<typeof createRedisCache>;
-
-/**
- * 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<boolean> {
-  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;
-  }
-}

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);
-  });
-});