diff --git a/src/github/webhook.ts b/src/github/webhook.ts index e2c9c0f992..0691c77428 100644 --- a/src/github/webhook.ts +++ b/src/github/webhook.ts @@ -37,7 +37,11 @@ export async function handleGitHubWebhook(c: Context<{ Bindings: Env }>): Promis const payloadHash = await sha256Hex(rawBody); const existingEvent = await getWebhookEvent(c.env, deliveryId); - if (existingEvent && existingEvent.payloadHash === payloadHash && existingEvent.status !== "error") { + // Suppress redelivery of an already-processed event (on success its payloadHash is overwritten to a + // "processed" sentinel, so a hash match alone misses it and the event re-runs its side effects) or one + // still in flight with the same payload. "error" rows are never suppressed so a failed enqueue/processing + // can still be retried (#789). + if (existingEvent && existingEvent.status !== "error" && (existingEvent.status === "processed" || existingEvent.payloadHash === payloadHash)) { return c.json({ ok: true, deliveryId, eventName, status: "duplicate" }, 202); } diff --git a/test/unit/webhook.test.ts b/test/unit/webhook.test.ts index 70774b6323..9ebf7ddb8c 100644 --- a/test/unit/webhook.test.ts +++ b/test/unit/webhook.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import type { Context } from "hono"; import { handleGitHubWebhook } from "../../src/github/webhook"; -import { getWebhookEvent } from "../../src/db/repositories"; +import { getWebhookEvent, recordWebhookEvent } from "../../src/db/repositories"; import { createTestEnv } from "../helpers/d1"; describe("github webhook body reader edge cases", () => { @@ -76,6 +76,46 @@ describe("github webhook enqueue failure (#786)", () => { }); }); +describe("github webhook dedup (#789)", () => { + it("suppresses redelivery of an already-processed event instead of re-running side effects", async () => { + const env = createTestEnv(); + let sendCount = 0; + env.JOBS = { + send: async () => { + sendCount += 1; + }, + } as unknown as typeof env.JOBS; + // Seed a fully-processed event: on success the queue overwrites payloadHash with the "processed" + // sentinel, so a redelivery carries the real hash and a hash-only dedup would miss it. + await recordWebhookEvent(env, { deliveryId: "redelivery-1", eventName: "pull_request", payloadHash: "processed", status: "processed" }); + const rawBody = JSON.stringify({ action: "opened", repository: { full_name: "JSONbored/gittensory" } }); + const signature = await signWebhook(rawBody, env.GITHUB_WEBHOOK_SECRET); + const request = new Request("https://example.com/webhook", { method: "POST", body: rawBody }); + const headers: Record = { + "x-github-delivery": "redelivery-1", + "x-github-event": "pull_request", + "x-hub-signature-256": signature, + }; + const context = { + req: { + raw: request, + header(name: string) { + return headers[name.toLowerCase()] ?? null; + }, + }, + env, + json(payload: unknown, status?: number) { + return Response.json(payload, status === undefined ? undefined : { status }); + }, + } as unknown as Context<{ Bindings: Env }>; + + const response = await handleGitHubWebhook(context); + expect(response.status).toBe(202); + await expect(response.json()).resolves.toMatchObject({ status: "duplicate" }); + expect(sendCount).toBe(0); // not re-enqueued + }); +}); + async function signWebhook(body: string, secret: string): Promise { const key = await crypto.subtle.importKey("raw", new TextEncoder().encode(secret), { name: "HMAC", hash: "SHA-256" }, false, ["sign"]); const signed = await crypto.subtle.sign("HMAC", key, new TextEncoder().encode(body));