feat(metrics): count Redis webhook dedup hits (#2075)

src/selfhost/redis-cache.ts

@@ -4,6 +4,38 @@
 // ID after a successful processing attempt, the server can return 204 immediately on retries
 // without re-queuing the job. The self-host review runtime requires REDIS_URL.
 import type { Redis } from "ioredis";
+import { incr } from "./metrics";
+
+const WEBHOOK_DELIVERY_CACHE_PREFIX = "delivery:";
+
+export function webhookDeliveryCacheKey(deliveryId: string): string {
+  return `${WEBHOOK_DELIVERY_CACHE_PREFIX}${deliveryId}`;
+}
+
+/** Returns true when this GitHub webhook delivery ID was already processed (Redis dedup hit).
+ *  Increments `gittensory_webhook_dedup_total{backend="redis"}` on a hit. Does NOT mark the
+ *  delivery — the caller marks only after a successful response (#2506 / #2572). */
+export async function isWebhookDeliveryDuplicate(cache: RedisCache, deliveryId: string): Promise<boolean> {
+  try {
+    const seen = await cache.get(webhookDeliveryCacheKey(deliveryId));
+    if (seen) {
+      incr("gittensory_webhook_dedup_total", { backend: "redis" });
+      return true;
+    }
+    return false;
+  } catch {
+    return false;
+  }
+}
+
+/** Best-effort: record a successfully processed webhook delivery for Redis dedup. */
+export async function rememberWebhookDelivery(cache: RedisCache, deliveryId: string, ttlSeconds = 300): Promise<void> {
+  try {
+    await cache.set(webhookDeliveryCacheKey(deliveryId), "1", ttlSeconds);
+  } catch {
+    // best-effort — never block the response on a cache write failure
+  }
+}
 
 export function createRedisCache(redis: Redis) {
   return {

src/server.ts

@@ -481,7 +481,7 @@ async function main(): Promise<void> {
   const { Redis } = await import("ioredis");
   const redisClient = new Redis(redisUrl);
   const { createRedisRateLimiter } = await import("./selfhost/redis-ratelimit");
-  const { createRedisCache, assertSelfhostTransientCacheOwnershipRelease } = await import("./selfhost/redis-cache");
+  const { createRedisCache, assertSelfhostTransientCacheOwnershipRelease, isWebhookDeliveryDuplicate, rememberWebhookDelivery } = await import("./selfhost/redis-cache");
   const rateLimiter = createRedisRateLimiter(redisClient);
   const webhookCache = createRedisCache(redisClient);
   assertSelfhostTransientCacheOwnershipRelease(webhookCache);
@@ -700,13 +700,13 @@ async function main(): Promise<void> {
     "gittensory_jobs_dead_total",
     "gittensory_jobs_rate_limit_deferred_total",
     "gittensory_jobs_recovered_total",
-    "gittensory_webhook_dedup_total",
     "gittensory_qdrant_queries_total",
     "gittensory_qdrant_upserts_total",
     "gittensory_orb_events_exported_total",
     "gittensory_orb_export_errors_total",
   ])
     incr(c, undefined, 0);
+  incr("gittensory_webhook_dedup_total", { backend: "redis" }, 0);
   // Seed gittensory_http_requests_total per status class so the breakdown panel has every series from the
   // first scrape (keeping the metric consistently labeled — never mix labeled and unlabeled samples).
   for (const status of ["2xx", "3xx", "4xx", "5xx"])
@@ -886,18 +886,13 @@ async function main(): Promise<void> {
                 ? request.headers.get("x-github-delivery")
                 : null;
               if (deliveryId) {
-                const seen = await webhookCache!.get(`delivery:${deliveryId}`);
-                if (seen) {
-                  incr("gittensory_webhook_dedup_total");
+                if (await isWebhookDeliveryDuplicate(webhookCache!, deliveryId)) {
                   return finish(new Response(null, { status: 204 }));
                 }
               }
               const response = await worker.fetch(request, env, ctx);
               if (deliveryId && response.ok) {
-                // Best-effort — never block the response on a cache write failure
-                void webhookCache!
-                  .set(`delivery:${deliveryId}`, "1", 300)
-                  .catch(() => undefined);
+                void rememberWebhookDelivery(webhookCache!, deliveryId);
               }
               return finish(response);
             } finally {

test/unit/selfhost-redis-cache.test.ts

@@ -1,6 +1,13 @@
 import type { Redis } from "ioredis";
-import { describe, expect, it } from "vitest";
-import { assertSelfhostTransientCacheOwnershipRelease, createRedisCache } from "../../src/selfhost/redis-cache";
+import { describe, expect, it, afterEach } from "vitest";
+import {
+  assertSelfhostTransientCacheOwnershipRelease,
+  createRedisCache,
+  isWebhookDeliveryDuplicate,
+  rememberWebhookDelivery,
+  webhookDeliveryCacheKey,
+} from "../../src/selfhost/redis-cache";
+import { renderMetrics, resetMetrics } from "../../src/selfhost/metrics";
 
 /** 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
@@ -97,3 +104,33 @@ describe("createRedisCache (#1216 webhook dedup cache)", () => {
     expect(() => assertSelfhostTransientCacheOwnershipRelease(createRedisCache(fakeRedis()))).not.toThrow();
   });
 });
+
+describe("isWebhookDeliveryDuplicate (#2075)", () => {
+  afterEach(() => resetMetrics());
+
+  it("returns false and does not increment on a first-time delivery", async () => {
+    const cache = createRedisCache(fakeRedis());
+    await expect(isWebhookDeliveryDuplicate(cache, "delivery-1")).resolves.toBe(false);
+    expect(await renderMetrics()).not.toContain('gittensory_webhook_dedup_total{backend="redis"}');
+  });
+
+  it("returns true and increments gittensory_webhook_dedup_total{backend=\"redis\"} when already seen", async () => {
+    const cache = createRedisCache(fakeRedis());
+    await cache.set(webhookDeliveryCacheKey("delivery-2"), "1", 300);
+    await expect(isWebhookDeliveryDuplicate(cache, "delivery-2")).resolves.toBe(true);
+    expect(await renderMetrics()).toContain('gittensory_webhook_dedup_total{backend="redis"} 1');
+  });
+
+  it("returns false without incrementing when Redis get throws", async () => {
+    const brokenRedis = { async get() { throw new Error("connection refused"); } } as unknown as Redis;
+    const cache = createRedisCache(brokenRedis);
+    await expect(isWebhookDeliveryDuplicate(cache, "delivery-3")).resolves.toBe(false);
+    expect(await renderMetrics()).not.toContain('gittensory_webhook_dedup_total{backend="redis"}');
+  });
+
+  it("rememberWebhookDelivery stores the delivery key for later dedup", async () => {
+    const cache = createRedisCache(fakeRedis());
+    await rememberWebhookDelivery(cache, "delivery-4");
+    expect(await cache.get(webhookDeliveryCacheKey("delivery-4"))).toBe("1");
+  });
+});