From 8e8ef8f1125351c43e405205a082ccb8cc2f16fb Mon Sep 17 00:00:00 2001 From: Shlok Zanwar Date: Mon, 13 Jul 2026 14:53:25 +0530 Subject: [PATCH] feat(notifications): add GET/HEAD support to generic webhooks Resolves #123 - Adds GET and HEAD to GenericWebhookSchema enum - Omits request body on GET and HEAD requests in GenericWebhookAdapter - Verifies boolean return value from adapter.send() across SystemNotificationService and job completion pipeline to properly record delivery failures - Adds explicit GET and HEAD unit tests --- src/lib/adapters/definitions/notification.ts | 2 +- .../adapters/notification/generic-webhook.ts | 22 ++++++++++-------- src/lib/runner/steps/04-completion.ts | 5 +++- .../system-notification-service.ts | 5 +++- .../notification/generic-webhook.test.ts | 23 +++++++++++++++++++ 5 files changed, 44 insertions(+), 13 deletions(-) diff --git a/src/lib/adapters/definitions/notification.ts b/src/lib/adapters/definitions/notification.ts index 71ccada6..eb6a9f7c 100644 --- a/src/lib/adapters/definitions/notification.ts +++ b/src/lib/adapters/definitions/notification.ts @@ -19,7 +19,7 @@ export const TeamsSchema = z.object({ export const GenericWebhookSchema = z.object({ webhookUrl: z.string().url("Valid URL is required"), - method: z.enum(["POST", "PUT", "PATCH"]).default("POST").describe("HTTP method"), + method: z.enum(["POST", "PUT", "PATCH", "GET", "HEAD"]).default("POST").describe("HTTP method"), contentType: z.string().default("application/json").describe("Content-Type header"), authHeader: z.string().optional().describe("Authorization header value (e.g. Bearer token)"), customHeaders: z.string().optional().describe("Additional headers (one per line, Key: Value)"), diff --git a/src/lib/adapters/notification/generic-webhook.ts b/src/lib/adapters/notification/generic-webhook.ts index 3f6325d8..f47d5c8e 100644 --- a/src/lib/adapters/notification/generic-webhook.ts +++ b/src/lib/adapters/notification/generic-webhook.ts @@ -58,11 +58,12 @@ export const GenericWebhookAdapter: NotificationAdapter = { fields: [], }); - const response = await fetch(config.webhookUrl, { - method: config.method || "POST", - headers, - body, - }); + const method = (config.method || "POST").toUpperCase(); + const fetchOpts: RequestInit = { method, headers }; + if (method !== "GET" && method !== "HEAD") { + fetchOpts.body = body; + } + const response = await fetch(config.webhookUrl, fetchOpts); if (response.ok) { return { success: true, message: `Webhook returned ${response.status}` }; @@ -114,11 +115,12 @@ export const GenericWebhookAdapter: NotificationAdapter = { }); } - const response = await fetch(config.webhookUrl, { - method: config.method || "POST", - headers, - body, - }); + const method = (config.method || "POST").toUpperCase(); + const fetchOpts: RequestInit = { method, headers }; + if (method !== "GET" && method !== "HEAD") { + fetchOpts.body = body; + } + const response = await fetch(config.webhookUrl, fetchOpts); if (!response.ok) { const responseBody = await response.text().catch(() => ""); diff --git a/src/lib/runner/steps/04-completion.ts b/src/lib/runner/steps/04-completion.ts index 10eaddef..7eeec4f0 100644 --- a/src/lib/runner/steps/04-completion.ts +++ b/src/lib/runner/steps/04-completion.ts @@ -214,7 +214,7 @@ export async function stepFinalize(ctx: RunnerContext) { }); } - await notifyWithTimeout(() => notifyAdapter.send(channelConfig, payload.message, { + const sent = await notifyWithTimeout(() => notifyAdapter.send(channelConfig, payload.message, { success: payload.success, eventType, title: payload.title, @@ -222,6 +222,9 @@ export async function stepFinalize(ctx: RunnerContext) { color: payload.color, badge: payload.badge, })); + if (sent === false) { + throw new Error("Notification delivery failed (adapter returned false)"); + } await recordNotificationLog({ eventType, diff --git a/src/services/notifications/system-notification-service.ts b/src/services/notifications/system-notification-service.ts index e31b63fb..48e28fac 100644 --- a/src/services/notifications/system-notification-service.ts +++ b/src/services/notifications/system-notification-service.ts @@ -196,7 +196,7 @@ async function sendThroughChannel( } try { - await notifyWithTimeout(() => adapter.send(channelConfig, payload.message, { + const sent = await notifyWithTimeout(() => adapter.send(channelConfig, payload.message, { success: payload.success, eventType, title: payload.title, @@ -204,6 +204,9 @@ async function sendThroughChannel( color: payload.color, badge: payload.badge, })); + if (sent === false) { + throw new Error("Notification delivery failed (adapter returned false)"); + } // Record successful send await recordNotificationLog({ diff --git a/tests/unit/adapters/notification/generic-webhook.test.ts b/tests/unit/adapters/notification/generic-webhook.test.ts index 9c100bbc..416faaab 100644 --- a/tests/unit/adapters/notification/generic-webhook.test.ts +++ b/tests/unit/adapters/notification/generic-webhook.test.ts @@ -280,5 +280,28 @@ describe("Generic Webhook Adapter", () => { expect(result).toBe(false); }); + + it("should omit body when method is GET or HEAD", async () => { + mockFetch.mockResolvedValue({ ok: true }); + + await GenericWebhookAdapter.send({ ...baseConfig, method: "GET" as any }, "Test message"); + expect(mockFetch.mock.calls[0][1].method).toBe("GET"); + expect(mockFetch.mock.calls[0][1].body).toBeUndefined(); + + await GenericWebhookAdapter.send({ ...baseConfig, method: "HEAD" as any }, "Test message"); + expect(mockFetch.mock.calls[1][1].method).toBe("HEAD"); + expect(mockFetch.mock.calls[1][1].body).toBeUndefined(); + }); + }); + + describe("test() with GET/HEAD", () => { + it("should omit body when method is GET in test()", async () => { + mockFetch.mockResolvedValue({ ok: true, status: 200 }); + + const result = await GenericWebhookAdapter.test!({ ...baseConfig, method: "GET" as any }); + expect(result.success).toBe(true); + expect(mockFetch.mock.calls[0][1].method).toBe("GET"); + expect(mockFetch.mock.calls[0][1].body).toBeUndefined(); + }); }); });