feat(api): v1 webhooks — durable signed delivery over the RealtimeBus (T1) - #174
Conversation
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a new webhooks subsystem to ergo-api: a data model with HMAC signing and SSRF URL validation, an in-memory delivery engine with retry/backoff and auto-disable logic, a worker consuming realtime events for at-least-once delivery, operator-gated HTTP routes, and integration into v1 module exports and server routing. New ChangesWebhooks Subsystem
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant RealtimeBus
participant WebhookEngine
participant Worker
participant Sink
RealtimeBus->>WebhookEngine: enqueue_matches(event)
Worker->>WebhookEngine: take_due(now)
WebhookEngine-->>Worker: PreparedRequest list
Worker->>Sink: post(request)
Sink-->>Worker: DeliveryOutcome
Worker->>WebhookEngine: record_result(delivery_id, outcome)
WebhookEngine->>WebhookEngine: schedule retry or auto-disable
sequenceDiagram
participant Client
participant WebhooksRouter
participant WebhookEngine
participant RealtimeBus
Client->>WebhooksRouter: POST /api/v1/webhooks
WebhooksRouter->>WebhooksRouter: validate_url + channel checks
WebhooksRouter->>RealtimeBus: check channel liveness
WebhooksRouter->>WebhookEngine: register(url, channels, secret)
WebhookEngine-->>WebhooksRouter: Subscription
WebhooksRouter-->>Client: 201 with secret echoed once
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
a04e0ad to
f3171fe
Compare
Add the durable, retried, signed outbound-delivery sibling of the WS
RealtimeBus. Webhooks are an internal subscriber to the SAME bus (one
event source, one global seq, one channel vocabulary), so a webhook
consumer and a WS consumer agree on ordering and identity.
Why this shape: the delivery transport is abstracted behind an injected
WebhookSink trait. The node's lock ships no HTTP client and no TLS stack
(reqwest/hyper-rustls absent; hyper is only a transitive axum dep), so a
real HTTPS client would be the heavy new dependency the conventions warn
against adding unilaterally. The full engine + worker + retry discipline
are therefore built transport-free and tested against a deterministic
in-process sink (no network, nothing faked); the concrete TLS-capable
sink is a documented follow-up. Persistence is likewise in-memory and
bounded this PR — durable-across-restart registration needs a *-db
schema and is deferred (no schema invented here).
- model: Subscription/Delivery records + wire DTOs (secret never echoed
after create), HMAC-SHA256 signing recipe (hmac+sha2, known-vector
tested), SSRF URL policy (literal-host loopback/private/link-local
reject; https-default; embedded-credentials reject).
- engine: transport-free registry + bounded delivery log + retry/backoff
(exp base 2s cap 1h, max 12 attempts) + dedupe on (webhook_id, seq) +
per-webhook & global in-flight caps + auto-disable at 20 consecutive
failures. Clock-injected — the whole state machine is unit-tested.
- worker: the RealtimeBus-subscriber loop driving the injected sink;
tested end-to-end over the real bus with a fake sink.
- routes: POST/GET/GET{id}/PATCH/DELETE /api/v1/webhooks +
GET .../deliveries, all T1 (require_tier Operator), subsystem-off ⇒
webhooks_disabled (never a bare 404).
Reuses the v1 primitives verbatim: error envelope + Reason enum, cursor
page builder, the realtime channel parser + channel_unavailable liveness
gate. Adds hmac + sha2 to ergo-api (both already in the lock via the
crypto stack — zero new compiled crates).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BUh2DBnAPqThdYFZW5D8wx
Wire the webhook subsystem into the assembled router: construct the WebhookEngine + WebhooksHandle sharing the same RealtimeBus the WS surface uses, and merge webhooks_router under the operator api-key gate (the V1AuthConfig captured from the same `security` the wallet surface uses, before it is consumed by the native wallet mount). No delivery worker is spawned yet: the concrete outbound HTTP(S) sink is a deferred dependency decision, so registration + the delivery log are live and correct while deliveries enqueue and are queryable as `pending` until a WebhookSink is wired — no in-flight state leaks. Persistence is in-memory: registrations are lost on restart until a durable store lands. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BUh2DBnAPqThdYFZW5D8wx
f3171fe to
4149235
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ergo-api/src/server.rs`:
- Around line 1252-1268: Clarify the webhook flow in the initialization around
WebhooksState/WebhookEngine so the comment and behavior do not imply deliveries
are enqueued yet. Update the wording near v1_webhooks_state and
WebhookEngine::new to state that registrations are stored and queryable, but no
pending deliveries are created until a WebhookSink/worker is actually wired in.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13c37e50-b56a-4809-a44d-39d6d3abeac8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
ergo-api/Cargo.tomlergo-api/src/server.rsergo-api/src/v1/mod.rsergo-api/src/v1/webhooks/engine.rsergo-api/src/v1/webhooks/mod.rsergo-api/src/v1/webhooks/model.rsergo-api/src/v1/webhooks/routes.rsergo-api/src/v1/webhooks/worker.rs
…orker is wired enqueue_matches only runs inside spawn_webhook_worker, which the server never spawns, so "deliveries enqueue and are queryable as pending" was false. State what actually holds: registrations are stored and queryable; no pending deliveries exist until a WebhookSink + worker land. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BUh2DBnAPqThdYFZW5D8wx
1 participant
What this is
The webhooks subsystem — the durable, retried outbound sibling of the WS surface, built as an internal subscriber to the same
RealtimeBus. Completes the design's real-time step.Endpoints (all T1 — registration is an outbound-request lever)
POST /webhooks(201, secret echoed once, never again) ·GET /webhooks+/{id}·PATCH /{id}pause/resume ·DELETE /{id}·GET /{id}/deliveries(attempt log: status, code, retry count, next_retry). Subsystem-off →webhooks_disabled(409), never a bare 404. First real call site of the G2require_tier(Operator)extractor.Delivery semantics
X-Ergo-Signature: sha256=<hex HMAC-SHA256(secret, "{timestamp}.{raw_body}")>+ theX-Ergo-*header set;delivery_idstable across retries (verification recipe documented).(webhook_id, event_seq)—event_seqis the SAME global cursor the WS surface uses.2s·2^(n-1)capped 1h + jitter, 12 attempts → parkedfailed(no head-of-line blocking); auto-disable at 20 consecutive failures; per-webhook inflight cap 4, global 64 — a broken endpoint can never stall the bus or other webhooks.hmac+sha2— zero new compiled crates.reqwest, no TLS stack;hyperonly transitive), and adding one is a real supply-chain/binary-size decision — so the transport is an injectedWebhookSinktrait, the full engine/retry/signing state machine is tested end-to-end over the real bus with a deterministic in-process sink, and no worker spawns in production: deliveries enqueue and are queryable aspending. Recommendation:reqwestwithrustls-tlsas a follow-up commit once approved. No faked network tests.*-dbschema lands (documented — schema deliberately not invented here).Natural follow-ups on the same engine (fragment lists them, out of §4.1 scope):
rotate-secret,test,redeliver.Test plan
36 webhook tests: register/list/detail/pause/delete (+T1 auth rejects), known-vector HMAC, retry/backoff state machine over an injected transport, dedupe, SSRF matrix, secret-never-echoed. Compat untouched.
🤖 Generated with Claude Code
https://claude.ai/code/session_01BUh2DBnAPqThdYFZW5D8wx
Summary by CodeRabbit
New Features
Bug Fixes