Goodyslim · Sep 29, 2026
diff --git a/listener/docs/WEBHOOK_RESPONSE_CLASSIFICATION.md b/listener/docs/WEBHOOK_RESPONSE_CLASSIFICATION.md
@@ -0,0 +1,39 @@
+# Webhook Response Classification
+
+Webhook deliveries are classified into exactly one of three categories by
+`listener/src/services/webhook-response-classifier.ts`. That module is the single
+source of truth for retry decisions; both the bounded retry helper
+(`webhook-retry-helper.ts`) and the delivery service
+(`webhook-delivery-service.ts`) defer to it so they can never disagree.
+
+| Category    | Meaning                                                               | Retried?                                  |
+| ----------- | --------------------------------------------------------------------- | ----------------------------------------- |
+| `success`   | HTTP 2xx; the receiver accepted the payload.                          | No                                        |
+| `retryable` | Transient failure (HTTP 429, any 5xx, network / timeout errors).      | Yes, within the bounded retry budget.     |
+| `permanent` | Failure that will not change on retry (3xx redirects, other 4xx, 1xx). | No                                        |
+
+## Status code rules
+
+| HTTP status | Category    | Rationale                                  |
+| ----------- | ----------- | ------------------------------------------ |
+| 1xx         | `permanent` | Unexpected for a webhook POST.             |
+| 200-299     | `success`   | Payload accepted.                          |
+| 3xx         | `permanent` | Redirects are not followed for webhooks.   |
+| 429         | `retryable` | Rate limited; back off and try again.      |
+| Other 4xx   | `permanent` | Bad request, auth failure, not found, ...  |
+| 5xx         | `retryable` | Server-side / transient.                   |
+
+## Error rules
+
+Network-level failures and request timeouts (`AbortError`) are classified as
+`retryable`. Any unrecognised outcome defaults to `permanent`, so a bug can only
+ever under-retry rather than loop forever.
+
+## Usage
+
+```ts
+import { classifyWebhookStatus, classifyWebhookError } from './webhook-response-classifier';
+
+const category = classifyWebhookStatus(response.status); // 'success' | 'retryable' | 'permanent'
+const retryable = category === 'retryable';
+```
diff --git a/listener/src/services/webhook-delivery-service.test.ts b/listener/src/services/webhook-delivery-service.test.ts
@@ -241,6 +241,50 @@ describe('WebhookDeliveryService', () => {
     });
   });
 
+  // ── Classification (issue #643) ───────────────────────────────────────────
+
+  describe('response category', () => {
+    it('reports success for 2xx responses', async () => {
+      mockSendWebhook.mockResolvedValue(makeResponse(200));
+
+      const result = await service.deliver(TARGET_URL, PAYLOAD, REQUEST_ID);
+
+      expect(result.category).toBe('success');
+    });
+
+    it.each([429, 500, 502, 503, 504])('reports retryable for HTTP %i', async (status) => {
+      mockSendWebhook.mockResolvedValue(makeResponse(status, false));
+
+      const result = await service.deliver(TARGET_URL, PAYLOAD, REQUEST_ID);
+
+      expect(result.category).toBe('retryable');
+    });
+
+    it.each([400, 401, 403, 404, 422])('reports permanent for HTTP %i', async (status) => {
+      mockSendWebhook.mockResolvedValue(makeResponse(status, false));
+
+      const result = await service.deliver(TARGET_URL, PAYLOAD, REQUEST_ID);
+
+      expect(result.category).toBe('permanent');
+    });
+
+    it('reports retryable for a request timeout', async () => {
+      mockSendWebhook.mockRejectedValue(makeAbortError());
+
+      const result = await service.deliver(TARGET_URL, PAYLOAD, REQUEST_ID);
+
+      expect(result.category).toBe('retryable');
+    });
+
+    it('reports retryable for a network error', async () => {
+      mockSendWebhook.mockRejectedValue(new Error('ECONNREFUSED'));
+
+      const result = await service.deliver(TARGET_URL, PAYLOAD, REQUEST_ID);
+
+      expect(result.category).toBe('retryable');
+    });
+  });
+
   // ── Default options ───────────────────────────────────────────────────────
 
   describe('default options', () => {

diff --git a/listener/src/services/webhook-delivery-service.ts b/listener/src/services/webhook-delivery-service.ts
@@ -17,6 +17,11 @@
  */
 
 import logger from '../utils/logger';
+import {
+  classifyWebhookError,
+  classifyWebhookStatus,
+  type WebhookResponseCategory,
+} from './webhook-response-classifier';
 import { sendWebhook, WebhookSendOptions } from './webhook-sender';
 
 export interface WebhookDeliveryOptions {
@@ -33,6 +38,8 @@ export interface WebhookDeliveryResult {
   statusCode?: number;
   /** Human-readable failure reason for logging. */
   errorReason?: string;
+  /** Classified outcome: `success`, `retryable`, or `permanent`. */
+  category: WebhookResponseCategory;
 }
 
 export class WebhookDeliveryService {
@@ -73,19 +80,24 @@ export class WebhookDeliveryService {
     try {
       const response = await sendWebhook(targetUrl, payload, sendOpts);
       const durationMs = Date.now() - startMs;
+      const category = classifyWebhookStatus(response.status);
 
-      if (response.ok) {
+      if (category === 'success') {
         logger.info('Webhook delivered successfully', {
           ...logCtx,
           statusCode: response.status,
           durationMs,
         });
-        return { success: true, statusCode: response.status };
+        return { success: true, statusCode: response.status, category };
       }
 
-      // 5xx — transient server error, worth retrying
-      if (response.status >= 500) {
-        logger.warn('Webhook delivery failed with server error (5xx) — will retry', {
+      if (category === 'retryable') {
+        // 5xx is the common transient case; 429 (rate limited) is also retryable.
+        const message =
+          response.status >= 500
+            ? 'Webhook delivery failed with server error (5xx) — will retry'
+            : 'Webhook delivery failed with a transient error — will retry';
+        logger.warn(message, {
           ...logCtx,
           statusCode: response.status,
           durationMs,
@@ -94,6 +106,7 @@ export class WebhookDeliveryService {
           success: false,
           statusCode: response.status,
           errorReason: `HTTP ${response.status}`,
+          category,
         };
       }
 
@@ -107,6 +120,7 @@ export class WebhookDeliveryService {
         success: false,
         statusCode: response.status,
         errorReason: `HTTP ${response.status}`,
+        category,
       };
     } catch (err) {
       const durationMs = Date.now() - startMs;
@@ -131,7 +145,7 @@ export class WebhookDeliveryService {
         });
       }
 
-      return { success: false, errorReason };
+      return { success: false, errorReason, category: classifyWebhookError(err) };
     }
   }
 }
diff --git a/listener/src/services/webhook-response-classifier.test.ts b/listener/src/services/webhook-response-classifier.test.ts
@@ -0,0 +1,56 @@
+/**
+ * Tests for the canonical webhook response classifier (issue #643).
+ *
+ * Covers representative HTTP status codes for each category, plus the error
+ * classifier used for network / timeout failures.
+ */
+
+import { describe, it, expect } from '@jest/globals';
+import {
+  classifyWebhookError,
+  classifyWebhookStatus,
+  isRetryableStatus,
+} from './webhook-response-classifier';
+
+describe('classifyWebhookStatus', () => {
+  it.each([200, 201, 202, 204, 299])('classifies HTTP %i as success', (status) => {
+    expect(classifyWebhookStatus(status)).toBe('success');
+  });
+
+  it.each([429, 500, 501, 502, 503, 504, 507])('classifies HTTP %i as retryable', (status) => {
+    expect(classifyWebhookStatus(status)).toBe('retryable');
+  });
+
+  it.each([100, 301, 302, 400, 401, 403, 404, 408, 418, 422])(
+    'classifies HTTP %i as permanent',
+    (status) => {
+      expect(classifyWebhookStatus(status)).toBe('permanent');
+    },
+  );
+});
+
+describe('classifyWebhookError', () => {
+  it('treats network errors as retryable', () => {
+    expect(classifyWebhookError(new Error('ECONNREFUSED'))).toBe('retryable');
+  });
+
+  it('treats request timeouts (AbortError) as retryable', () => {
+    const abort = new Error('The operation was aborted');
+    abort.name = 'AbortError';
+    expect(classifyWebhookError(abort)).toBe('retryable');
+  });
+
+  it('treats an absent error as permanent', () => {
+    expect(classifyWebhookError(undefined)).toBe('permanent');
+    expect(classifyWebhookError(null)).toBe('permanent');
+  });
+});
+
+describe('isRetryableStatus', () => {
+  it('mirrors classifyWebhookStatus for representative codes', () => {
+    expect(isRetryableStatus(429)).toBe(true);
+    expect(isRetryableStatus(503)).toBe(true);
+    expect(isRetryableStatus(200)).toBe(false);
+    expect(isRetryableStatus(404)).toBe(false);
+  });
+});
diff --git a/listener/src/services/webhook-response-classifier.ts b/listener/src/services/webhook-response-classifier.ts
@@ -0,0 +1,71 @@
+/**
+ * Canonical classification of webhook delivery responses (issue #643).
+ *
+ * Every webhook attempt has exactly one of three outcomes:
+ *
+ *   - `success`   - the receiver accepted the payload (HTTP 2xx). Stop.
+ *   - `retryable` - a transient failure worth another attempt: HTTP 429, any
+ *                   5xx (500/502/503/504/...), and network / timeout errors.
+ *   - `permanent` - a client error that will not change on retry: redirects
+ *                   (3xx), other 4xx, and 1xx. Stop and surface the failure.
+ *
+ * Status -> category:
+ *
+ *   1xx       -> permanent (unexpected for a webhook POST)
+ *   2xx       -> success
+ *   3xx       -> permanent (redirects are not followed for webhooks)
+ *   429       -> retryable (rate limited; back off and try again)
+ *   other 4xx -> permanent (bad request, auth failure, not found, ...)
+ *   5xx       -> retryable (server-side / transient)
+ *
+ * Network-level failures and timeouts (AbortError) are retryable; see
+ * {@link classifyWebhookError}.
+ *
+ * This module is the single source of truth for retry decisions so the retry
+ * helper and the delivery service can never disagree. It never mutates the
+ * response and defaults anything unrecognised to `permanent`, so a bug can only
+ * ever under-retry, never loop forever.
+ */
+
+export type WebhookResponseCategory = 'success' | 'retryable' | 'permanent';
+
+/** Explicitly retryable status codes (5xx is covered by the generic 5xx rule). */
+export const RETRYABLE_STATUS_CODES: ReadonlySet<number> = new Set([429, 500, 502, 503, 504]);
+
+/** Permanent client-error status codes (never retried). */
+export const PERMANENT_STATUS_CODES: ReadonlySet<number> = new Set([400, 401, 403, 404, 422]);
+
+/**
+ * Classify an HTTP status code into a webhook delivery category.
+ *
+ * @param status - HTTP response status code.
+ * @returns The category that describes whether delivery succeeded, should be
+ *          retried, or has permanently failed.
+ */
+export function classifyWebhookStatus(status: number): WebhookResponseCategory {
+  if (status >= 200 && status < 300) {
+    return 'success';
+  }
+  if (status === 429 || status >= 500) {
+    return 'retryable';
+  }
+  return 'permanent';
+}
+
+/**
+ * Classify a thrown webhook error.
+ *
+ * Network-level failures and request timeouts (AbortError) are transient and
+ * therefore retryable; anything else is treated as a permanent failure.
+ *
+ * @param error - The error thrown by the webhook attempt, if any.
+ * @returns `'retryable'` for any thrown failure, else `'permanent'`.
+ */
+export function classifyWebhookError(error: unknown): WebhookResponseCategory {
+  return error ? 'retryable' : 'permanent';
+}
+
+/** True when the HTTP status code represents a retryable failure. */
+export function isRetryableStatus(status: number): boolean {
+  return classifyWebhookStatus(status) === 'retryable';
+}
diff --git a/listener/src/services/webhook-retry-helper.ts b/listener/src/services/webhook-retry-helper.ts
@@ -14,6 +14,7 @@
  * A delay is added between attempts to reduce load on failing services.
  */
 
+import { classifyWebhookError, classifyWebhookStatus } from './webhook-response-classifier';
 import { sendWebhook, WebhookSendOptions } from './webhook-sender';
 
 /** Maximum number of retry attempts (not counting the initial attempt). */
@@ -22,16 +23,6 @@ const MAX_RETRY_ATTEMPTS = 2;
 /** Delay in milliseconds between retry attempts. */
 const RETRY_DELAY_MS = 1000;
 
-/**
- * HTTP status codes that are considered retryable (transient failures).
- */
-const RETRYABLE_STATUS_CODES = new Set([429, 500, 502, 503, 504]);
-
-/**
- * HTTP status codes that are permanent client errors (not retryable).
- */
-const PERMANENT_CLIENT_ERRORS = new Set([400, 401, 403, 404, 422]);
-
 /**
  * Determines if an error or response should trigger a retry.
  *
@@ -40,35 +31,14 @@ const PERMANENT_CLIENT_ERRORS = new Set([400, 401, 403, 404, 422]);
  * @returns true if the failure is retryable
  */
 function isRetryable(response?: Response, error?: unknown): boolean {
-  // Network errors and timeouts are retryable
+  // Network errors and timeouts are retryable; the classifier owns the policy.
   if (error) {
-    return true;
+    return classifyWebhookError(error) === 'retryable';
   }
 
-  // Check HTTP status codes
+  // Delegate the HTTP status policy to the shared classifier.
   if (response) {
-    // Success responses don't need retry
-    if (response.ok) {
-      return false;
-    }
-
-    // Permanent client errors should not be retried
-    if (PERMANENT_CLIENT_ERRORS.has(response.status)) {
-      return false;
-    }
-
-    // Explicit retryable status codes
-    if (RETRYABLE_STATUS_CODES.has(response.status)) {
-      return true;
-    }
-
-    // Any other 5xx error is retryable
-    if (response.status >= 500) {
-      return true;
-    }
-
-    // Other status codes (e.g., redirects, other 4xx) are not retried
-    return false;
+    return classifyWebhookStatus(response.status) === 'retryable';
   }
 
   return false;