fix(webhooks): read every Ashby error shape when webhook registration fails

apps/sim/lib/webhooks/providers/ashby.test.ts

@@ -3,7 +3,7 @@
  */
 import crypto from 'crypto'
 import { createMockRequest } from '@sim/testing'
-import { describe, expect, it } from 'vitest'
+import { afterEach, describe, expect, it, vi } from 'vitest'
 import { ashbyHandler } from '@/lib/webhooks/providers/ashby'
 
 describe('ashbyHandler', () => {
@@ -136,6 +136,135 @@ describe('ashbyHandler', () => {
     })
   })
 
+  describe('createSubscription error reporting', () => {
+    const realFetch = globalThis.fetch
+    afterEach(() => {
+      globalThis.fetch = realFetch
+    })
+
+    const ctx = {
+      requestId: 'req-1',
+      webhook: {
+        id: 'wh-1',
+        path: '/api/webhooks/trigger/abc',
+        providerConfig: { apiKey: 'k', triggerId: 'ashby_job_create' },
+      },
+    } as never
+
+    const respondWith = (body: unknown, status = 200) => {
+      globalThis.fetch = vi.fn().mockResolvedValue(
+        new Response(JSON.stringify(body), {
+          status,
+          headers: { 'content-type': 'application/json' },
+        })
+      ) as never
+    }
+
+    it('surfaces the object-shaped errors array Ashby documents', async () => {
+      // Reading only errorInfo.message misses this form, which is what a
+      // missing-permission failure arrives in - the user would see
+      // 'Unknown Ashby API error' instead of the actual cause.
+      respondWith({ success: false, errors: [{ message: 'missing_endpoint_permission' }] })
+      await expect(ashbyHandler.createSubscription?.(ctx)).rejects.toThrow(
+        /missing_endpoint_permission/
+      )
+    })
+
+    it('surfaces the plain-string errors array Ashby also returns', async () => {
+      respondWith({ success: false, errors: ['webhook_not_found'] })
+      await expect(ashbyHandler.createSubscription?.(ctx)).rejects.toThrow(/webhook_not_found/)
+    })
+
+    it('still prefers errorInfo.message when Ashby sends both shapes at once', async () => {
+      respondWith({
+        success: false,
+        errors: ['webhook_not_found'],
+        errorInfo: { code: 'webhook_not_found', message: 'Webhook not found' },
+      })
+      await expect(ashbyHandler.createSubscription?.(ctx)).rejects.toThrow(/Webhook not found/)
+    })
+
+    it('keeps the actionable duplicate-webhook guidance reachable', async () => {
+      // The duplicate branch only fires when the message was extracted, so an
+      // unparsed error costs the user the instructions for fixing it.
+      respondWith({ success: false, errors: [{ message: 'duplicate webhook for this url' }] })
+      await expect(ashbyHandler.createSubscription?.(ctx)).rejects.toThrow(
+        /Ashby Settings > API\/Webhooks/
+      )
+    })
+  })
+
+  describe('deleteSubscription', () => {
+    const realFetch = globalThis.fetch
+    afterEach(() => {
+      globalThis.fetch = realFetch
+    })
+
+    const ctx = (strict: boolean) =>
+      ({
+        requestId: 'req-1',
+        strict,
+        webhook: {
+          id: 'wh-1',
+          providerConfig: { apiKey: 'k', externalId: 'ext-1' },
+        },
+      }) as never
+
+    const respondWith = (body: unknown, status = 200) => {
+      globalThis.fetch = vi.fn().mockResolvedValue(
+        new Response(JSON.stringify(body), {
+          status,
+          headers: { 'content-type': 'application/json' },
+        })
+      ) as never
+    }
+
+    it('treats a 200 carrying success:false as a failed delete', async () => {
+      // Ashby returns what would be a 4XX as HTTP 200. Branching on
+      // response.ok alone reported the leak as a successful cleanup.
+      respondWith({ success: false, errors: [{ message: 'missing_endpoint_permission' }] })
+      await expect(ashbyHandler.deleteSubscription?.(ctx(true))).rejects.toThrow(
+        /missing_endpoint_permission/
+      )
+    })
+
+    it('stays non-fatal for a failed delete when not strict', async () => {
+      respondWith({ success: false, errors: [{ message: 'missing_endpoint_permission' }] })
+      await expect(ashbyHandler.deleteSubscription?.(ctx(false))).resolves.toBeUndefined()
+    })
+
+    it('treats an already-removed webhook as done even in strict mode', async () => {
+      respondWith({ success: false, errors: ['webhook_not_found'] })
+      await expect(ashbyHandler.deleteSubscription?.(ctx(true))).resolves.toBeUndefined()
+    })
+
+    it('recognizes the not-found envelope Ashby actually sends for a repeat delete', async () => {
+      // errorInfo.message wins over the code array in the extractor, so it reads
+      // 'Webhook not found' - matching that against `webhook_not_found` would
+      // turn idempotent cleanup into a strict-mode throw.
+      respondWith({
+        success: false,
+        errors: ['webhook_not_found'],
+        errorInfo: {
+          code: 'webhook_not_found',
+          message: 'Webhook not found',
+          requestId: '01JSJ8FEK5ZN4XQBZP7DBKK7ZC',
+        },
+      })
+      await expect(ashbyHandler.deleteSubscription?.(ctx(true))).resolves.toBeUndefined()
+    })
+
+    it('recognizes a not-found reported only as prose', async () => {
+      respondWith({ success: false, errorInfo: { message: 'Webhook not found' } })
+      await expect(ashbyHandler.deleteSubscription?.(ctx(true))).resolves.toBeUndefined()
+    })
+
+    it('accepts a successful delete', async () => {
+      respondWith({ success: true, results: { webhookId: 'ext-1' } })
+      await expect(ashbyHandler.deleteSubscription?.(ctx(true))).resolves.toBeUndefined()
+    })
+  })
+
   describe('extractIdempotencyId', () => {
     it('derives a stable key from application id + updatedAt', () => {
       const body = {

apps/sim/lib/webhooks/providers/ashby.ts

@@ -17,6 +17,80 @@ import type {
 } from '@/lib/webhooks/providers/types'
 import { buildFallbackDeliveryFingerprint } from '@/lib/webhooks/providers/utils'
 
+/**
+ * Kept local rather than imported from `@/tools/ashby/utils`, which has the same
+ * logic. The webhook providers are reachable from workspace page graphs, and the
+ * knowledge page graph currently sits exactly at the ceiling
+ * `check:tool-registry-boundary` allows - so neither an import edge into
+ * `@/tools/**` nor an extra module in this directory fits. Both copies derive
+ * from the same three documented Ashby error shapes and are covered
+ * independently by `tools/ashby/utils.test.ts` and `ashby.test.ts` here.
+ */
+/**
+ * Extract a human-readable error message from an Ashby error response. Ashby
+ * documents two shapes and uses three in practice:
+ *
+ * - `errorInfo: { code, message, requestId }`
+ * - `errors: ['webhook_not_found']` - plain strings
+ * - `errors: [{ message, parameter }]` - objects, which is the form a 403 for a
+ *   missing module permission arrives in, and which stringifies to
+ *   `[object Object]` unless the message is read explicitly
+ *
+ * A single response can carry more than one of these at once.
+ */
+function ashbyErrorMessage(data: unknown, fallback: string): string {
+  if (!data || typeof data !== 'object') return fallback
+  const d = data as Record<string, unknown>
+  const info = d.errorInfo as Record<string, unknown> | undefined
+  if (info && typeof info.message === 'string' && info.message) return info.message
+  if (Array.isArray(d.errors) && d.errors.length > 0) {
+    const messages = d.errors
+      .map((e) => {
+        if (typeof e === 'string') return e
+        if (e && typeof e === 'object') {
+          const entry = e as Record<string, unknown>
+          const message = typeof entry.message === 'string' ? entry.message : ''
+          const parameter = typeof entry.parameter === 'string' ? entry.parameter : ''
+          if (message && parameter) return `${message} (${parameter})`
+          if (message) return message
+        }
+        return ''
+      })
+      .filter(Boolean)
+    if (messages.length > 0) return messages.join('; ')
+  }
+  return fallback
+}
+
+/**
+ * Whether an Ashby error response means the webhook id no longer exists.
+ *
+ * Ashby signals this as the machine code `webhook_not_found`, carried on
+ * `errorInfo.code` and/or as an `errors` entry — but the same envelope's
+ * `errorInfo.message` reads `Webhook not found`, and that is what
+ * `ashbyErrorMessage` returns, since message wins over the deprecated code
+ * array. Matching the extracted message against the code therefore misses the
+ * envelope Ashby actually sends for a repeat delete, and idempotent cleanup
+ * would be reported as a real failure. Read the codes directly, and keep a
+ * prose fallback for the message-only form.
+ */
+function isAshbyWebhookNotFound(data: Record<string, unknown>, message: string): boolean {
+  const info = data.errorInfo as Record<string, unknown> | undefined
+  if (typeof info?.code === 'string' && /webhook_not_found/i.test(info.code)) return true
+
+  if (Array.isArray(data.errors)) {
+    for (const entry of data.errors) {
+      if (typeof entry === 'string' && /webhook_not_found/i.test(entry)) return true
+      if (entry && typeof entry === 'object') {
+        const entryMessage = (entry as Record<string, unknown>).message
+        if (typeof entryMessage === 'string' && /webhook_not_found/i.test(entryMessage)) return true
+      }
+    }
+  }
+
+  return /webhook[\s_]not[\s_]found/i.test(message)
+}
+
 const logger = createLogger('WebhookProvider:Ashby')
 
 function validateAshbySignature(secretToken: string, signature: string, body: string): boolean {
@@ -212,9 +286,15 @@ export const ashbyHandler: WebhookProviderHandler = {
       const responseBody = (await ashbyResponse.json().catch(() => ({}))) as Record<string, unknown>
 
       if (!ashbyResponse.ok || !responseBody.success) {
-        const errorInfo = responseBody.errorInfo as Record<string, string> | undefined
-        const errorMessage =
-          errorInfo?.message || (responseBody.message as string) || 'Unknown Ashby API error'
+        // Ashby documents two error shapes and uses both. Reading only
+        // `errorInfo.message` misses the `errors: [{ message, parameter }]` form,
+        // which is what a missing-permission failure arrives in - and the
+        // duplicate-webhook branch below only fires when the message was
+        // extracted, so losing it costs the user the actionable guidance.
+        const errorMessage = ashbyErrorMessage(
+          responseBody,
+          (responseBody.message as string) || 'Unknown Ashby API error'
+        )
 
         let userFriendlyMessage = 'Failed to create webhook subscription in Ashby'
         if (ashbyResponse.status === 401) {
@@ -289,23 +369,43 @@ export const ashbyHandler: WebhookProviderHandler = {
         body: JSON.stringify({ webhookId: externalId }),
       })
 
-      if (ashbyResponse.ok) {
-        await ashbyResponse.body?.cancel()
+      const responseBody = (await ashbyResponse.json().catch(() => ({}))) as Record<string, unknown>
+
+      /**
+       * Ashby returns what would be a 4XX elsewhere as HTTP 200 with
+       * `success: false`, so the status alone cannot separate a completed
+       * delete from a rejected one. Branching on `ashbyResponse.ok` reported
+       * every rejection as a successful cleanup while Sim dropped its own row
+       * — and with no `webhook.list` endpoint, an orphan left behind that way
+       * cannot be enumerated afterwards.
+       *
+       * Unlike `createSubscription`, an absent `success` field is treated as
+       * success rather than failure: teardown runs on the undeploy path, and
+       * failing closed on an unparseable body would wedge cleanup on a
+       * response shape Ashby does not document.
+       */
+      const rejected = !ashbyResponse.ok || responseBody.success === false
+      const errorMessage = ashbyErrorMessage(responseBody, `HTTP ${ashbyResponse.status}`)
+
+      if (!rejected) {
         logger.info(
           `[${ctx.requestId}] Successfully deleted Ashby webhook subscription ${externalId}`
         )
-      } else if (ashbyResponse.status === 404) {
-        await ashbyResponse.body?.cancel()
+      } else if (
+        ashbyResponse.status === 404 ||
+        isAshbyWebhookNotFound(responseBody, errorMessage)
+      ) {
         logger.info(
           `[${ctx.requestId}] Ashby webhook ${externalId} not found during deletion (already removed)`
         )
       } else {
-        const responseBody = await ashbyResponse.json().catch(() => ({}))
         logger.warn(
-          `[${ctx.requestId}] Failed to delete Ashby webhook (non-fatal): ${ashbyResponse.status}`,
-          { response: responseBody }
+          `[${ctx.requestId}] Failed to delete Ashby webhook (non-fatal): ${errorMessage}`,
+          { status: ashbyResponse.status, response: responseBody }
         )
-        if (ctx.strict) throw new Error(`Failed to delete Ashby webhook: ${ashbyResponse.status}`)
+        if (ctx.strict) {
+          throw new Error(`Failed to delete Ashby webhook: ${errorMessage}`)
+        }
       }
     } catch (error) {
       logger.warn(`[${ctx.requestId}] Error deleting Ashby webhook (non-fatal)`, error)

apps/sim/triggers/ashby/utils.ts

@@ -43,9 +43,9 @@ export function isAshbyEventMatch(triggerId: string, action: string): boolean {
  */
 export function ashbySetupInstructions(eventType: string): string {
   const instructions = [
-    'Enter your Ashby API Key above. You can find your API key in Ashby at <strong>Settings &gt; API Keys</strong>.',
-    `The webhook for <strong>${eventType}</strong> events will be automatically created in Ashby when you save the trigger.`,
-    'The webhook will be automatically deleted if you remove this trigger.',
+    'Enter your Ashby API Key above. You can find your API key in Ashby at <strong>Settings &gt; API Keys</strong>. It needs the <strong>apiKeysWrite</strong> permission.',
+    `The webhook for <strong>${eventType}</strong> events is created in Ashby when you deploy the workflow, not when you save the trigger.`,
+    'The webhook is deleted from Ashby when you remove this trigger and redeploy.',
   ]
 
   return instructions
@@ -275,7 +275,7 @@ export function buildJobCreateOutputs(): Record<string, TriggerOutput> {
       status: { type: 'string', description: 'Job status (Open, Closed, Draft, Archived)' },
       employmentType: {
         type: 'string',
-        description: 'Employment type (FullTime, PartTime, Intern, Contract)',
+        description: 'Employment type (FullTime, PartTime, Intern, Contract, Temporary)',
       },
     },
   } as Record<string, TriggerOutput>