diff --git a/server/src/__tests__/routines-service.test.ts b/server/src/__tests__/routines-service.test.ts index 8886c75c60..aa5d16dd33 100644 --- a/server/src/__tests__/routines-service.test.ts +++ b/server/src/__tests__/routines-service.test.ts @@ -2363,7 +2363,7 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => { expect(run.status).toBe("issue_created"); }); - it("accepts Sentry signatures and deduplicates retries by immutable issue id", async () => { + it("deduplicates identical Sentry event retries but delivers distinct issue actions", async () => { const { routine, svc } = await seedFixture(); const { trigger, secretMaterial } = await svc.createTrigger( routine.id, @@ -2394,30 +2394,74 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => { payload, }; - const retryPayload = { + const resolvedPayload = { ...payload, action: "resolved", actor: { type: "application", name: "Sentry" }, }; - const retryRawBody = Buffer.from(JSON.stringify(retryPayload)); - const retryRequest = { + const resolvedRawBody = Buffer.from(JSON.stringify(resolvedPayload)); + const resolvedRequest = { sentrySignatureHeader: createHmac("sha256", secretMaterial!.webhookSecret) - .update(retryRawBody) + .update(resolvedRawBody) .digest("hex"), - rawBody: retryRawBody, - payload: retryPayload, + rawBody: resolvedRawBody, + payload: resolvedPayload, }; const first = await svc.firePublicTrigger(trigger.publicId!, request); - const retry = await svc.firePublicTrigger(trigger.publicId!, retryRequest); + const retry = await svc.firePublicTrigger(trigger.publicId!, request); + const resolved = await svc.firePublicTrigger(trigger.publicId!, resolvedRequest); expect(first).toMatchObject({ source: "webhook", status: "issue_created" }); expect(retry.id).toBe(first.id); expect(retry.linkedIssueId).toBe(first.linkedIssueId); - expect(await db.select().from(routineRuns).where(eq(routineRuns.triggerId, trigger.id))).toHaveLength(1); + expect(resolved.id).not.toBe(first.id); + expect(resolved.linkedIssueId).not.toBe(first.linkedIssueId); + expect(await db.select().from(routineRuns).where(eq(routineRuns.triggerId, trigger.id))).toHaveLength(2); expect( await db.select().from(issues).where(eq(issues.originId, routine.id)), - ).toHaveLength(1); + ).toHaveLength(2); + }); + + it("prefers a provider delivery id when deduplicating Sentry webhook retries", async () => { + const { routine, svc } = await seedFixture(); + const { trigger, secretMaterial } = await svc.createTrigger( + routine.id, + { kind: "webhook", signingMode: "github_hmac" }, + {}, + ); + const payload = { + action: "created", + data: { issue: { id: "7625432288", project: { slug: "api" } } }, + }; + const requestFor = (idempotencyKey: string, body: Record) => { + const rawBody = Buffer.from(JSON.stringify(body)); + return { + idempotencyKey, + sentrySignatureHeader: createHmac("sha256", secretMaterial!.webhookSecret) + .update(rawBody) + .digest("hex"), + rawBody, + payload: body, + }; + }; + + const first = await svc.firePublicTrigger( + trigger.publicId!, + requestFor("sentry-delivery-1", payload), + ); + const retry = await svc.firePublicTrigger( + trigger.publicId!, + requestFor("sentry-delivery-1", { ...payload, actor: { type: "application" } }), + ); + const separateDelivery = await svc.firePublicTrigger( + trigger.publicId!, + requestFor("sentry-delivery-2", payload), + ); + + expect(retry.id).toBe(first.id); + expect(separateDelivery.id).not.toBe(first.id); + expect(await db.select().from(routineRuns).where(eq(routineRuns.triggerId, trigger.id))).toHaveLength(2); }); it("rejects invalid signature for github_hmac signing mode", async () => { diff --git a/server/src/services/routines.ts b/server/src/services/routines.ts index 30666297ee..e2006b39dc 100644 --- a/server/src/services/routines.ts +++ b/server/src/services/routines.ts @@ -319,6 +319,11 @@ function sentryIssueIdFromWebhookPayload(payload: Record | null return typeof id === "string" || typeof id === "number" ? String(id) : null; } +function sentryActionFromWebhookPayload(payload: Record | null | undefined) { + const action = payload?.action; + return typeof action === "string" && action.trim().length > 0 ? action.trim() : null; +} + function parseBooleanVariableValue(name: string, raw: unknown) { if (typeof raw === "boolean") return raw; if (typeof raw === "number" && (raw === 0 || raw === 1)) return raw === 1; @@ -2920,9 +2925,14 @@ export function routineService( if (input.sentrySignatureHeader) { const sentryIssueId = sentryIssueIdFromWebhookPayload(input.payload); if (sentryIssueId) { - hmacReplayKey = `webhook-sentry-issue:${crypto + const providerDeliveryId = input.idempotencyKey?.trim(); + const action = sentryActionFromWebhookPayload(input.payload); + const deliveryIdentity = providerDeliveryId + ? `delivery:${providerDeliveryId}` + : `issue:${sentryIssueId}:action:${action ?? "unknown"}`; + hmacReplayKey = `webhook-sentry-event:${crypto .createHash("sha256") - .update(`${trigger.id}:${sentryIssueId}`) + .update(`${trigger.id}:${deliveryIdentity}`) .digest("hex")}`; } } @@ -2989,7 +2999,7 @@ export function routineService( : null, idempotencyKey: hmacReplayKey ?? input.idempotencyKey, rejectIdempotencyReplay: - hmacReplayKey !== null && !hmacReplayKey.startsWith("webhook-sentry-issue:"), + hmacReplayKey !== null && !hmacReplayKey.startsWith("webhook-sentry-event:"), }); },