|
| 1 | +import type { Logger } from "pino"; |
| 2 | + |
| 3 | +import { getValkeyClient, isValkeyHealthy } from "../orchestrator/valkey"; |
| 4 | + |
| 5 | +/** |
| 6 | + * Webhook delivery idempotency (issue #202). |
| 7 | + * |
| 8 | + * GitHub replays a delivery (automatic retry or operator-driven manual |
| 9 | + * redelivery) with the SAME `X-GitHub-Delivery` header for up to 3 days, so |
| 10 | + * webhooks are at-least-once. Without a dedup gate, a redelivery re-runs the |
| 11 | + * full handler, including the triage/intent LLM calls and any chat-thread turn, |
| 12 | + * double-billing and posting duplicate replies. `claimDelivery` is the |
| 13 | + * canonical SET-NX-with-TTL idempotency claim, evaluated at the top of each |
| 14 | + * handler's dispatch path before any side-effect. |
| 15 | + */ |
| 16 | + |
| 17 | +// 3 days, matching GitHub's redelivery window. |
| 18 | +const TTL_SECONDS = 259_200; |
| 19 | +const KEY_PREFIX = "idemp:webhook:"; |
| 20 | + |
| 21 | +/** |
| 22 | + * Claim a webhook delivery for processing. |
| 23 | + * |
| 24 | + * Returns `true` exactly once per `deliveryId` within the TTL window (the first |
| 25 | + * caller proceeds); a redelivery gets `false` and must skip. |
| 26 | + * |
| 27 | + * Fail-OPEN: if Valkey is unavailable or errors, returns `true` so an outage |
| 28 | + * degrades to at-least-once processing rather than dropping every webhook. The |
| 29 | + * `idx_workflow_runs_inflight` partial-unique index remains the durable backstop |
| 30 | + * against duplicate work when this best-effort layer is skipped: the dispatcher |
| 31 | + * rejects a second in-flight run for the same workflow+target. (The |
| 32 | + * tracking-comment marker scan via `isAlreadyProcessed` is NOT a backstop here: |
| 33 | + * it runs only on the legacy `router.ts processRequest` path that production |
| 34 | + * handlers bypass, issue #202.) |
| 35 | + */ |
| 36 | +export async function claimDelivery(deliveryId: string, log: Logger): Promise<boolean> { |
| 37 | + const client = getValkeyClient(); |
| 38 | + // `getValkeyClient()` returns a non-null client even while the TCP connection |
| 39 | + // is down (it is null only when VALKEY_URL is unset). Bun's RedisClient |
| 40 | + // defaults to `enableOfflineQueue: true`, so issuing SET against a |
| 41 | + // disconnected client would QUEUE and block (up to the 10s connectionTimeout) |
| 42 | + // instead of failing open. Gate on `isValkeyHealthy()` (the same liveness |
| 43 | + // signal `router.ts` dispatch guards use) so a configured-but-down Valkey |
| 44 | + // takes the immediate fail-open path, leaving the durable backstops |
| 45 | + // (`idx_workflow_runs_inflight` + tracking-comment marker scan) to dedup. |
| 46 | + if (client === null || !isValkeyHealthy()) { |
| 47 | + log.warn({ deliveryId }, "claimDelivery: Valkey unavailable, proceeding (fail-open)"); |
| 48 | + return true; |
| 49 | + } |
| 50 | + try { |
| 51 | + // SET key 1 NX EX <ttl>: returns "OK" iff the key did not exist (we won the |
| 52 | + // claim); returns null when it already exists (a redelivery). |
| 53 | + // `RedisClient.send` is typed `Promise<any>`; SET-NX returns "OK" or null. |
| 54 | + const res = (await client.send("SET", [ |
| 55 | + `${KEY_PREFIX}${deliveryId}`, |
| 56 | + "1", |
| 57 | + "NX", |
| 58 | + "EX", |
| 59 | + String(TTL_SECONDS), |
| 60 | + ])) as string | null; |
| 61 | + if (res === "OK") return true; |
| 62 | + log.info( |
| 63 | + { deliveryId, event: "dedup-skip" }, |
| 64 | + "claimDelivery: duplicate webhook delivery, skipping", |
| 65 | + ); |
| 66 | + return false; |
| 67 | + } catch (err) { |
| 68 | + log.warn( |
| 69 | + { deliveryId, err: err instanceof Error ? err.message : String(err) }, |
| 70 | + "claimDelivery: Valkey error, proceeding (fail-open)", |
| 71 | + ); |
| 72 | + return true; |
| 73 | + } |
| 74 | +} |
0 commit comments