Explorar o código

docs: clarify webhook delivery state machine and claim rules

Define pending/processing/success/dead transitions, worker claim leases,
payload id construction, and synthetic test deliveries with send_event_id 0.

AI-Co-Authored-By: Grok
chendeben hai 1 mes
pai
achega
6dd0b28112
Modificáronse 1 ficheiros con 60 adicións e 10 borrados
  1. 60 10
      docs/superpowers/specs/2026-07-09-mailhub-webhooks-design.md

+ 60 - 10
docs/superpowers/specs/2026-07-09-mailhub-webhooks-design.md

@@ -42,10 +42,12 @@ send_event reaches terminal status
 
 ### Trigger points
 
-Whenever `send_events.status` becomes or is set to `sent` | `bounced` | `failed`:
+Concrete call sites in the current codebase (plan should wire both):
 
-1. `updateSendEventDelivery` (Postfix tracker) when computed `nextStatus` is terminal **and** previous status was not the same terminal status.
-2. Direct failure/success writes from mailer / submission / API send paths that set terminal status.
+1. **`updateSendEventDelivery`** (Postfix tracker path) — when computed `nextStatus` is terminal **and** differs from the previous row status.
+2. **`logSendEvent`** (and any wrapper that inserts a terminal status, typically `failed` on immediate send failure) — when the inserted status is already terminal.
+
+Other mailer/submission/API paths only matter if they write through these functions; do not add parallel enqueue call sites without going through DB helpers.
 
 Enqueue is **synchronous DB insert only** (must not await outbound HTTP). Worker performs HTTP.
 
@@ -56,6 +58,40 @@ Enqueue is **synchronous DB insert only** (must not await outbound HTTP). Worker
 - Concurrency: small fixed pool (e.g. 2–3 in-flight POSTs) to protect local Node and remote endpoints.
 - On process restart, pending/retryable rows remain in SQLite and resume via `next_attempt_at`.
 
+### Delivery status machine
+
+`webhook_deliveries.status` values:
+
+| Status | Meaning |
+|--------|---------|
+| `pending` | Not yet successfully delivered; may be due when `next_attempt_at <= now` |
+| `processing` | Claimed by worker for an in-flight HTTP attempt (short-lived lease) |
+| `success` | Last attempt received HTTP 2xx |
+| `dead` | Exhausted max attempts without 2xx |
+
+There is **no** long-lived `failed` status. Transient HTTP/network failures stay **`pending`** with an updated `next_attempt_at`, `attempt_count`, `error`, and `response_*` fields for the UI (“last attempt failed” is derived from `error` / `response_status` while `status=pending` or `dead`).
+
+Transitions:
+
+1. Enqueue → `pending`, `attempt_count=0`, `next_attempt_at=now`.
+2. Worker **claims** due `pending` rows → `processing` (see claim protocol).
+3. HTTP 2xx → `success`.
+4. HTTP non-2xx / timeout / network error:
+   - If `attempt_count < 8` after increment → `pending` + backoff `next_attempt_at`.
+   - Else → `dead`.
+5. Manual replay (from `success` or `dead`, or `pending` with errors) → `pending`, `attempt_count=0`, `next_attempt_at=now`, clear `error` (keep or clear last response preview; MVP may clear both).
+
+### Worker claim protocol
+
+To avoid double POST within one process (and reduce multi-instance races):
+
+1. In a single SQLite transaction, `SELECT` up to N rows where `status='pending' AND next_attempt_at <= now` ordered by `next_attempt_at`, then `UPDATE` those ids to `status='processing'`, set `last_attempt_at=now`, and set `next_attempt_at` to a **lease deadline** (e.g. now + 2 minutes) so a crashed worker does not leave rows stuck forever.
+2. Only claimed rows may perform HTTP POST.
+3. On completion, update to `success` or back to `pending`/`dead` as above; clear lease by writing the final status.
+4. Reaper (same worker loop): if `status='processing'` and `next_attempt_at < now` (lease expired), reset to `pending` with `next_attempt_at=now` so the row can be retried.
+
+MVP assumes a **single app instance** (current Docker deploy). Claim still required for in-process concurrency.
+
 ## Resolution Rules
 
 Given `userId`, `domainId`, `eventType` ∈ {sent, bounced, failed}:
@@ -96,7 +132,7 @@ Indexes: `(user_id)`, `(user_id, domain_id)`.
 | send_event_id | INTEGER NOT NULL | |
 | event_type | TEXT NOT NULL | sent \| bounced \| failed |
 | payload_json | TEXT NOT NULL | Exact body bytes basis (JSON text) |
-| status | TEXT NOT NULL | pending \| success \| failed \| dead |
+| status | TEXT NOT NULL | pending \| processing \| success \| dead |
 | attempt_count | INTEGER NOT NULL DEFAULT 0 | |
 | next_attempt_at | TEXT NOT NULL | |
 | last_attempt_at | TEXT | |
@@ -125,8 +161,8 @@ On webhook delete: CASCADE or mark deliveries orphaned—prefer **ON DELETE CASC
 | 6+ | 12 hours |
 
 - Max attempts: **8** (including first try). Then `status = dead`.
-- Success: HTTP **2xx**.
-- Manual **replay**: set `status=pending`, `attempt_count=0`, `next_attempt_at=now`, clear last error (keep historical response fields or clear—prefer clear error + allow new attempts; do not change `payload_json` so signature body stays consistent for that delivery identity; optional: create a new delivery row for replay to preserve history—**MVP: mutate same row for simplicity**).
+- Success: HTTP **2xx**. All non-2xx and transport errors use the same backoff path (no special-case 4xx in MVP).
+- Manual **replay**: mutate the **same** delivery row — `status=pending`, `attempt_count=0`, `next_attempt_at=now`, clear `error` / response preview; **do not change `payload_json`** so the signed body stays stable for that delivery id.
 
 ## HTTP Contract
 
@@ -172,7 +208,9 @@ On webhook delete: CASCADE or mark deliveries orphaned—prefer **ON DELETE CASC
 
 ### Signature
 
-Signed string: `{t}.{raw_body}` where `raw_body` is the exact JSON string POSTed.
+**Payload id construction:** Insert `webhook_deliveries` first to obtain `id`, then set `payload_json` with `"id": "whd_<id>"` (and stable `created_at`) before the first attempt. Signature always uses that stored `payload_json` as `raw_body`.
+
+Signed string: `{t}.{raw_body}` where `raw_body` is the exact stored JSON string POSTed.
 
 ```text
 v1 = hex(HMAC_SHA256(secret, signed_string))
@@ -200,9 +238,19 @@ All routes require authenticated session (or existing API auth pattern used by o
 | PATCH | `/api/webhooks/:id` | Update name, url, events, enabled, domainId (not secret) |
 | POST | `/api/webhooks/:id/rotate-secret` | New secret, return once |
 | DELETE | `/api/webhooks/:id` | Delete |
-| POST | `/api/webhooks/:id/test` | Enqueue synthetic delivery for first subscribed event or `sent` |
+| POST | `/api/webhooks/:id/test` | Enqueue a **synthetic** test delivery (see below) |
 | GET | `/api/webhook-deliveries` | List with filters: status, webhookId, eventType, limit |
-| POST | `/api/webhook-deliveries/:id/replay` | Replay failed/dead/success (re-queue) |
+| POST | `/api/webhook-deliveries/:id/replay` | Replay dead/success/pending (re-queue per replay rules) |
+
+**Test delivery:** Does not require a real `send_events` row. Use `send_event_id = 0` (allowed only for test deliveries; document as sentinel). Payload shape matches production with:
+
+- `data.test: true`
+- `data.message_id: "mh-test"`
+- `data.send_event_id: 0`
+- `data.status` / `type` from the webhook’s first subscribed event, or `sent` / `email.sent` if all three are subscribed
+- Placeholder from/to/subject/domain from the user’s first domain when available, else fixed examples
+
+Unique key for test rows: still `(webhook_id, send_event_id, event_type)` — concurrent tests of the same event on the same webhook may hit the unique constraint; API should **reuse** the existing test delivery row and reset it to pending (replay semantics) instead of failing.
 
 Validation: URL format + SSRF check on create/update; events non-empty subset of allowed three; domainId must belong to user when set.
 
@@ -255,8 +303,10 @@ Reuse existing design system: PageHeader, SectionCard, StatusPill, CodeBlock for
 
 - Terminal status change creates at most one delivery row per webhook endpoint  
 - Domain override suppresses account endpoints for that event  
-- Failed POSTs retry with backoff and appear in UI  
+- Failed POSTs remain `pending` (or become `dead`) with backoff and appear in UI  
 - Manual replay works  
+- Worker claim uses `processing` lease to avoid double POST  
+
 - Secrets encrypted at rest; shown once on create/rotate  
 - `npm test` / `npm run build` pass