fix(webhook): report failed deliveries in webhook test and log the last attempt

src/__tests__/webhook.test.ts

@@ -108,6 +108,17 @@ describe('webhook', () => {
       expect(mockFetch).not.toHaveBeenCalled();
     });
 
+    it('logs a delivery that still fails on its last attempt', async () => {
+      const { sendWebhook } = await import('../webhook.js');
+      const { log } = await import('../utils/logger.js');
+      mockAutoDetectInit.mockResolvedValueOnce(singleEndpointConfig(0));
+      mockFetch.mockResolvedValue({ ok: false, status: 503 });
+
+      await sendWebhook('push', { tool: 'claude', data: {} });
+
+      expect(log.warn).toHaveBeenCalledWith('Webhook to https://example.com/hook failed after 1 attempt(s): status 503');
+    });
+
     it('should handle fetch errors gracefully', async () => {
       mockFetch.mockRejectedValueOnce(new Error('Network error'));
 
@@ -147,6 +158,21 @@ describe('webhook', () => {
     });
   });
 
+  function singleEndpointConfig(retries: number) {
+    return {
+      localConfig: { repo: { localPath: '/tmp', remote: '' }, username: 'test', scope: 'user' },
+      teamConfig: {
+        team: 'test',
+        sharing: {
+          webhooks: {
+            enabled: true,
+            endpoints: [{ url: 'https://example.com/hook', type: 'json', events: ['*'], timeout: 5000, retries }],
+          },
+        },
+      },
+    };
+  }
+
   describe('testWebhook', () => {
     it('should send test event to all endpoints', async () => {
       const { testWebhook } = await import('../webhook.js');
@@ -156,6 +182,66 @@ describe('webhook', () => {
       expect(mockFetch).toHaveBeenCalledTimes(2);
     });
 
+    it('reports success only when the endpoint accepts the event', async () => {
+      const { testWebhook } = await import('../webhook.js');
+      const { log } = await import('../utils/logger.js');
+      mockAutoDetectInit.mockResolvedValueOnce(singleEndpointConfig(0));
+
+      await testWebhook();
+
+      expect(log.success).toHaveBeenCalledWith('Webhook test successful: https://example.com/hook');
+      expect(log.error).not.toHaveBeenCalled();
+    });
+
+    it('reports failure when the endpoint keeps answering 5xx', async () => {
+      const { testWebhook } = await import('../webhook.js');
+      const { log } = await import('../utils/logger.js');
+      mockAutoDetectInit.mockResolvedValueOnce(singleEndpointConfig(0));
+      mockFetch.mockResolvedValue({ ok: false, status: 500 });
+
+      await testWebhook();
+
+      expect(log.success).not.toHaveBeenCalled();
+      expect(log.error).toHaveBeenCalledWith('Webhook test failed: https://example.com/hook');
+      expect(log.warn).toHaveBeenCalledWith('Webhook to https://example.com/hook failed after 1 attempt(s): status 500');
+    });
+
+    it('reports failure on a non-retryable 4xx', async () => {
+      const { testWebhook } = await import('../webhook.js');
+      const { log } = await import('../utils/logger.js');
+      mockAutoDetectInit.mockResolvedValueOnce(singleEndpointConfig(3));
+      mockFetch.mockResolvedValue({ ok: false, status: 404 });
+
+      await testWebhook();
+
+      expect(mockFetch).toHaveBeenCalledTimes(1);
+      expect(log.success).not.toHaveBeenCalled();
+      expect(log.error).toHaveBeenCalledWith('Webhook test failed: https://example.com/hook');
+    });
+
+    it('retries a timed-out request with backoff and reports failure after the last attempt', async () => {
+      vi.useFakeTimers();
+      try {
+        const { testWebhook } = await import('../webhook.js');
+        const { log } = await import('../utils/logger.js');
+        mockAutoDetectInit.mockResolvedValueOnce(singleEndpointConfig(1));
+        const abort = Object.assign(new Error('aborted'), { name: 'AbortError' });
+        mockFetch.mockRejectedValue(abort);
+
+        const run = testWebhook();
+        await vi.advanceTimersByTimeAsync(999);
+        expect(mockFetch).toHaveBeenCalledTimes(1);
+        await vi.advanceTimersByTimeAsync(1);
+        await run;
+
+        expect(mockFetch).toHaveBeenCalledTimes(2);
+        expect(log.warn).toHaveBeenCalledWith('Webhook to https://example.com/hook failed after 2 attempt(s): timed out after 5000ms');
+        expect(log.error).toHaveBeenCalledWith('Webhook test failed: https://example.com/hook');
+      } finally {
+        vi.useRealTimers();
+      }
+    });
+
     it('should send test event to specific endpoint', async () => {
       const { testWebhook } = await import('../webhook.js');
 

src/webhook.ts

@@ -43,12 +43,14 @@ export async function sendWebhook(
 }
 
 /**
- * Send webhook to a single endpoint with retry logic.
+ * Send webhook to a single endpoint with retry logic. Resolves to whether the
+ * endpoint accepted the event; every failure is logged here and never thrown,
+ * so a webhook can never fail the command that fired it.
  */
 async function sendToEndpoint(
   endpoint: WebhookEndpoint,
   payload: WebhookPayload,
-): Promise<void> {
+): Promise<boolean> {
   const { url, type, secret, timeout, retries } = endpoint;
 
   const body = formatMessage(type, payload);
@@ -72,46 +74,44 @@ async function sendToEndpoint(
   }
 
   for (let attempt = 0; attempt <= retries; attempt++) {
+    let failure: string;
+    const controller = new AbortController();
+    const timeoutId = setTimeout(() => controller.abort(), timeout);
     try {
-      const controller = new AbortController();
-      const timeoutId = setTimeout(() => controller.abort(), timeout);
-
       const response = await fetch(url, {
         method: 'POST',
         headers,
         body: outbound,
         signal: controller.signal,
       });
 
-      clearTimeout(timeoutId);
-
       if (response.ok) {
         log.debug(`Webhook sent successfully to ${url}`);
-        return;
+        return true;
       }
 
       if (response.status >= 400 && response.status < 500 && response.status !== 429) {
         log.warn(`Webhook to ${url} failed with status ${response.status} (not retrying)`);
-        return;
-      }
-
-      if (attempt < retries) {
-        const delay = Math.pow(2, attempt) * 1000;
-        log.debug(`Webhook to ${url} failed, retrying in ${delay}ms...`);
-        await new Promise((resolve) => setTimeout(resolve, delay));
+        return false;
       }
+      failure = `status ${response.status}`;
     } catch (error) {
-      if (error instanceof Error && error.name === 'AbortError') {
-        log.warn(`Webhook to ${url} timed out after ${timeout}ms`);
-      } else if (attempt < retries) {
-        const delay = Math.pow(2, attempt) * 1000;
-        log.debug(`Webhook to ${url} failed, retrying in ${delay}ms...`);
-        await new Promise((resolve) => setTimeout(resolve, delay));
-      } else {
-        log.warn(`Webhook to ${url} failed after ${retries + 1} attempts: ${(error as Error).message}`);
-      }
+      failure = error instanceof Error && error.name === 'AbortError'
+        ? `timed out after ${timeout}ms`
+        : (error as Error).message;
+    } finally {
+      clearTimeout(timeoutId);
+    }
+
+    if (attempt < retries) {
+      const delay = Math.pow(2, attempt) * 1000;
+      log.debug(`Webhook to ${url} failed (${failure}), retrying in ${delay}ms...`);
+      await new Promise((resolve) => setTimeout(resolve, delay));
+    } else {
+      log.warn(`Webhook to ${url} failed after ${retries + 1} attempt(s): ${failure}`);
     }
   }
+  return false;
 }
 
 /**
@@ -177,11 +177,10 @@ export async function testWebhook(url?: string): Promise<void> {
 
   for (const endpoint of endpoints) {
     log.info(`Testing webhook to ${endpoint.url}...`);
-    try {
-      await sendToEndpoint(endpoint, testPayload);
+    if (await sendToEndpoint(endpoint, testPayload)) {
       log.success(`Webhook test successful: ${endpoint.url}`);
-    } catch (error) {
-      log.error(`Webhook test failed: ${endpoint.url} - ${(error as Error).message}`);
+    } else {
+      log.error(`Webhook test failed: ${endpoint.url}`);
     }
   }
 }