feat(server): add outbound webhook dispatcher for note sync events by proscar87 · Pull Request #217 · allenporter/supernote

proscar87 · 2026-08-05T03:06:42Z

What this does

Implements the "Event Webhook Dispatcher" requested in #167: a WebhookService that sends HMAC-signed HTTP POST notifications to configured external endpoints (Home Assistant / n8n / Zapier / etc.) when server-side events happen, without blocking or risking the request that triggered them.

Fixes #167

How it follows your spec

Your comment on the issue included a detailed spec, followed closely:

File location: supernote/server/services/webhook.py, as specified.
Config shape (config.yaml):
```
webhooks:
  enabled: true
  endpoints:
    - url: "https://n8n.homelab.local/webhook/supernote"
      secret: "hmac-secret-key"
      events:
        - "note.sync_completed"
```
Added as WebhookConfig / WebhookEndpointConfig dataclasses on ServerConfig, following the same DataClassYAMLMixin pattern as AuthConfig. Documented (commented-out) in config-example.yaml. enabled also has a SUPERNOTE_WEBHOOKS_ENABLED env override for consistency with the rest of ServerConfig.
Signature header: X-Supernote-Signature: sha256=<hex>, computed with hmac.new(secret, body, hashlib.sha256). Verification helper (verify_signature) uses hmac.compare_digest for constant-time comparison, exposed as public API for receivers/tests.
Dispatcher engine listens on the internal event bus (supernote/server/events.py's LocalEventBus), as specified.
Async, non-blocking dispatch using aiohttp.ClientSession — already a server extra dependency, so no new dependency was added. Each POST has an explicit 10s timeout and retries transient failures (network errors / 5xx) up to 3 attempts with linear backoff; 4xx responses are not retried. Every failure path is caught, logged, and swallowed — a slow or dead endpoint can never delay or fail the request that published the event, matching how LocalEventBus.publish() already fires handlers via asyncio.create_task.
Secret and full signature are never logged.

Where the spec didn't fully specify — decisions made (please weigh in)

note.sync_completed trigger point. Your example payload includes file.page_count and a processing block (tasks_extracted, summary) that are only known after the async OCR/embedding/summary pipeline finishes (ProcessorService.process_file). Wiring the webhook there would mean adding a new "processing completed" event type and touching the processing pipeline — more surface area than a dispatcher module. Instead, I fire note.sync_completed off the existing NoteUpdatedEvent, published the moment a .note file finishes uploading to the server (FileService.upload_finish / upload_finish_web). That's the literal "device synced, note landed on the server" moment and needed zero changes outside the dispatcher + wiring. The payload is therefore minimal:
```
{"event": "note.sync_completed", "timestamp": 1785734400, "user_id": 7, "file": {"id": 42, "path": "Note/Daily/2026-08-04.note"}}
```
Happy to move this to pipeline-completion time (richer payload matching your example exactly) if that's what you intended — let me know and I'll follow up.
task.created. There's no existing domain concept in the codebase for a user-facing "task extracted from a note" (the closest thing, SystemTaskDO, is internal processing-pipeline bookkeeping — hashing/OCR/embedding/summary steps — not something a user would want a webhook for). Rather than invent that feature (out of scope here per the issue title), I left task.created unwired: the dispatcher's dispatch(event_name, payload) API is generic, so wiring it up is a one-line event_bus.subscribe(...) once that feature exists. Flagging this as an open question rather than guessing at a shape.
Retry policy (max attempts / backoff) wasn't specified beyond "retry backoff" — went with 3 attempts, linear backoff (1s, 2s), no retry on 4xx. Easy to tune if you have different numbers in mind.

How it's configured

See config-example.yaml for a documented example. Endpoints are opt-in per-event via the events list, so one config can fan out to multiple receivers with different subscriptions.

What I validated

New tests: tests/server/services/test_webhook.py (HMAC sign/verify incl. tampered-body and wrong-secret rejection, signed dispatch to subscribed endpoints only, no-signature-header when no secret, NoteUpdatedEvent → note.sync_completed end-to-end through the real LocalEventBus, disabled-webhooks no-op, no-raise on repeated 5xx / timeout, no-retry on 4xx, recovery after a transient failure, and that the secret never leaks into logs) plus tests/server/test_config.py additions for the new config section and env override. All HTTP is mocked (a small fake aiohttp.ClientSession) — no real network calls.
uv run ruff check . / uv run ruff format — clean.
uv run ty check — clean.
uv run pytest -n auto -m "not integration" — 585 passed (568 baseline + 17 new), 0 failed. Compared against a git stash baseline run to confirm no pre-existing failures were masked or introduced.
pre-commit run (incl. codespell, yamllint) — clean.
git status clean, uv.lock untouched.

🤖 Generated with Claude Code

Adds a WebhookService that sends HMAC-SHA256 signed HTTP POST notifications to configured endpoints, enabling integrations with Home Assistant, n8n, Zapier, etc. Dispatch is fire-and-forget: it subscribes on the existing LocalEventBus and never blocks or fails the request that triggered the event. - New `webhooks` config section (WebhookConfig/WebhookEndpointConfig) with per-endpoint url/secret/events, matching the config.yaml shape from allenporter#167. - Fires `note.sync_completed` off the existing NoteUpdatedEvent (the point where a synced .note file is durably written), signed via `X-Supernote-Signature: sha256=<hex>` using hmac.compare_digest-safe verification. - Each endpoint POST has an explicit 10s timeout and up to 3 attempts with linear backoff for network errors/5xx; 4xx responses are not retried. All failures are logged (never the secret or signature) and swallowed. Fixes allenporter#167 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

asyncio.gather without return_exceptions propagates the first exception, so an unexpected error while dispatching to one endpoint would surface in the sync flow that triggered it, contradicting the documented never-raises contract. Collect the results instead and log anything unexpected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

proscar87 · 2026-08-05T03:12:01Z

Pushed a follow-up: dispatch() documented a never-raises contract, but asyncio.gather() without return_exceptions=True propagates the first exception — so an unexpected error (e.g. a non-serializable payload) would have surfaced in the sync flow that triggered the dispatch instead of being contained. It now collects results and logs anything unexpected, with a test covering that path through the public API.

allenporter

Really appreciate this stellar contribution. Thank you for proposing a high quality implementation.

allenporter · 2026-08-05T03:48:13Z

+        """Translate a NoteUpdatedEvent into a note.sync_completed webhook."""
+        if not isinstance(event, NoteUpdatedEvent):
+            return
+        payload = {


Can you define a dataclass for this? It could live in model? It can serve as documentation for each field

allenporter · 2026-08-05T03:49:25Z

+        payload = {
+            "event": EVENT_NOTE_SYNC_COMPLETED,
+            "timestamp": int(time.time()),
+            "user_id": event.user_id,


I don't remember, but I believe this is a private field so lets not include it.

I realize a webhook typically is "per user" so maybe we need to consider how this is supposed to work here. I realize in reality, this will probably mostly be used with a single user...

allenporter · 2026-08-05T03:53:47Z

+    )
+    # Retries use a real (but zeroed-out) asyncio.sleep so the suite stays
+    # fast without mocking asyncio.sleep globally.
+    monkeypatch.setattr(webhook_module, "RETRY_BACKOFF_SECONDS", 0.0)


lets use patch and not monkeypatch

allenporter · 2026-08-05T03:54:25Z

+        return False
+
+
+class _FakeTransport:


Can we use an aiohttp app server rather than mocking out the client library? I dont really trust aiohttp mocking and given the fake http server is pretty good I think we should use it.

allenporter · 2026-08-05T03:57:23Z

+    return f"sha256={digest}"
+
+
+def verify_signature(secret: str, body: bytes, signature: str) -> bool:


This appears unused

allenporter · 2026-08-05T03:57:39Z

+        """
+        body = json.dumps(payload).encode("utf-8")
+        headers = {"Content-Type": "application/json", EVENT_HEADER: event_name}
+        if endpoint.secret:


Do you expect this to provide security in practice?

allenporter · 2026-08-05T03:58:20Z

+        Never raises. Endpoints are notified concurrently and independently;
+        one endpoint failing has no effect on delivery to the others.
+        """
+        endpoints = [ep for ep in self.config.endpoints if event_name in ep.events]


how about it's all by default when not specified?

allenporter · 2026-08-05T04:00:26Z

I agree on your decisions of what to implement for v0 vs what can be added later (processing, notes, tasks, etc). This design leaves the door open for adding those things.

Addresses allenporter's review comments on PR allenporter#217: - Extract the note.sync_completed webhook body into a documented NoteSyncCompletedPayload/WebhookFileVO dataclass pair in supernote/models/webhook.py, instead of an inline dict. - Drop the internal, per-user user_id from the outbound payload; it's a private DB identifier that shouldn't leave the server. - Remove verify_signature(), which was dead code (this feature only implements the sending side; nothing in-repo calls it). - An endpoint with no `events` configured now defaults to receiving every event, instead of receiving nothing. - Rewrite the test suite to POST against a real aiohttp.web.Application test receiver (matching the fake-server pattern already used elsewhere in the suite, e.g. tests/server/conftest.py) instead of mocking aiohttp.ClientSession, and switch monkeypatch to unittest.mock.patch throughout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

proscar87 · 2026-08-05T19:02:10Z

Thanks for the thorough review, and for the kind words! Pushed a follow-up commit (820c3ab) addressing all 7 comments:

Dataclass for the payload (webhook.py:72) — done. Added NoteSyncCompletedPayload/WebhookFileVO to supernote/models/webhook.py, following the same DataClassJSONMixin convention as the rest of supernote/models/. _handle_note_updated now builds this typed dataclass and passes payload.to_dict() into dispatch(), so dispatch()/_send() stay generic (they still take dict[str, Any], since dispatch() is meant to be reusable for future event types too).
Drop the private field (webhook.py:75) — removed user_id from the outbound payload; it's the internal DB primary key (see UserService.get_user_id / VirtualFileSystem.create_or_update_file), not something that should leave the server. Added a test asserting it's absent.
On the "webhooks are per-user" question: agreed this deserves real thought, but I don't think there's a small, obvious fix here — today WebhookConfig is server-global (one set of endpoints for everyone), which matches the "mostly single-user" case you mentioned. Doing this properly would mean per-user endpoint config (probably tied to UserService/a new DB table) and touching how WebhookService resolves subscribers per event. That felt like its own design discussion rather than something to bolt on here, so I left it as a follow-up rather than guessing at an API. Happy to file an issue/spec for it if useful.
patch instead of monkeypatch (test_webhook.py:99) — done, unittest.mock.patch throughout (the RETRY_BACKOFF_SECONDS fixture and the new timeout test).
Real aiohttp server instead of mocking the client (test_webhook.py:51) — this was the big one. Rewrote the suite to spin up an actual aiohttp.web.Application (WebhookReceiver) via the aiohttp_client fixture, same pattern already used for the main app in tests/server/conftest.py. The dispatcher now does real HTTP POSTs over loopback; the receiver records calls and can script per-call responses, including a deliberate stall to trigger a genuine client-side timeout for the retry/timeout tests. No more mocking aiohttp.ClientSession.
Unused code (webhook.py:45) — that was verify_signature(). Removed it along with its three dedicated tests; this PR only implements the sending side, and nothing in the repo called it.
Does the signature provide real security? (webhook.py:118) — legitimate question, answered honestly rather than defensively: yes, when a receiver actually verifies X-Supernote-Signature with the shared secret, HMAC-SHA256 gives them real authenticity/integrity guarantees — same pattern GitHub/Stripe/Twilio use for webhooks. It's opt-in per endpoint (only signed if secret is set) since some local-network use cases (e.g. talking to an internal n8n/Home Assistant instance) may not need it; that's documented in config-example.yaml. It doesn't provide confidentiality on its own (that's on the endpoint using HTTPS), and we can't force receivers to verify — but that's inherent to the sender side of any webhook scheme, not something specific to this implementation. I don't think there's security theater here, so I left the mechanism as-is; happy to reconsider if you see it differently.
Default to all events when unspecified (webhook.py:89) — done. An endpoint with an empty/unset events list now receives every event instead of none. Updated the WebhookEndpointConfig.events docstring and config-example.yaml, and added a test.

Full suite (uv run pytest -n auto -m "not integration") still passes: 586 passed, same as on main before this change (net zero test-count delta: -3 removed verify_signature tests, +3 added: default-all-events + 2 new payload model tests). ruff check, ruff format, ty check, and pre-commit run are all clean.

proscar87 and others added 2 commits August 4, 2026 21:06

allenporter requested changes Aug 5, 2026

View reviewed changes

Navigation Menu

feat(server): add outbound webhook dispatcher for note sync events - #217

feat(server): add outbound webhook dispatcher for note sync events#217
proscar87 wants to merge 3 commits into
allenporter:mainfrom
proscar87:feat/event-webhook-dispatcher

proscar87 commented Aug 5, 2026

Uh oh!

proscar87 commented Aug 5, 2026

Uh oh!

allenporter left a comment

Uh oh!

allenporter Aug 5, 2026

Uh oh!

allenporter Aug 5, 2026

Uh oh!

allenporter Aug 5, 2026

Uh oh!

allenporter Aug 5, 2026

Uh oh!

allenporter Aug 5, 2026

Uh oh!

allenporter Aug 5, 2026

Uh oh!

allenporter Aug 5, 2026

Uh oh!

allenporter commented Aug 5, 2026

Uh oh!

proscar87 commented Aug 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		return f"sha256={digest}"


		def verify_signature(secret: str, body: bytes, signature: str) -> bool:

Conversation

proscar87 commented Aug 5, 2026

What this does

How it follows your spec

Where the spec didn't fully specify — decisions made (please weigh in)

How it's configured

What I validated

Uh oh!

proscar87 commented Aug 5, 2026

Uh oh!

allenporter left a comment

Choose a reason for hiding this comment

Uh oh!

allenporter Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

allenporter Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

allenporter Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

allenporter Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

allenporter Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

allenporter Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

allenporter Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

allenporter commented Aug 5, 2026

Uh oh!

proscar87 commented Aug 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants