Skip to content

feat(notifications): in-app notification center backend (phase d.3) - #60

Open
axelhamil wants to merge 32 commits into
devfrom
feat/notifications
Open

feat(notifications): in-app notification center backend (phase d.3)#60
axelhamil wants to merge 32 commits into
devfrom
feat/notifications

Conversation

@axelhamil

Copy link
Copy Markdown
Owner

Backend of the in-app notification center. Front (<NotificationBell />, /settings/notifications, createBroadcastChannel promotion) is a separate follow-up plan.

What ships

  • notification-map.ts in @packages/events — third projection of the event catalog, after visibility-map (webhooks) and retention-map (purge). 21 of 67 events are notifiable; an absent event produces nothing.
  • NotificationFanoutSubscriber — runs inside the outbox dispatch TX beside the audit and webhook subscribers, so a notification is never lost to a best-effort handler. One INSERT ... SELECT joining members, never a loop.
  • Capability-based recipientstype Audience = "self" | "actor" | "org:all" | { can: OrgPermissions }. Roles are static code, so rolesWith() resolves at boot and leaves WHERE member.role = ANY($1) at runtime: same query a hardcoded role tuple would have produced, without duplicating a decision owned by @packages/access-control.
  • Two tablesnotification + notification_preference, with 3 partial indexes (unread count, dedup, email pending).
  • Per-channel batching — in-app writes immediately and groups on read; email batches on write through emailPendingAt / emailSentAt. No batch table: we own the storage, so the batch stays a query instead of state that can desync.
  • SSE streamGET /notifications/stream, one LISTEN connection per instance (never per client), heartbeat 25 s, cap 5 streams per user. The stream carries a signal, never data: a reconnect just fires invalidateQueries, which kills Last-Event-ID, replay and merge logic in one stroke.
  • 8 HTTP routes, all gated; writes carry denyImpersonated, org preferences carry requireOrgPermission.
  • Preference cascade — org lock → user preference → map default, with forced bypassing all three.
  • Two crons on the existing /internal/* rail, documented in docs/CRON.md.

Catalog

65 → 67 / 28 public / 39 internal. Notification creation emits nothing (it is a read projection of an already-audited event, and emitting would loop with its own subscriber). Preference mutations do emit — an admin silencing billing alerts for a whole org must leave an audit trail (rule 7).

Notable decisions

  • organizationId nullable on notification is a documented exception to org-scoping rule 3: a notification is user-scoped by nature, user.password_changed belongs to no org.
  • Raw SQL for the org fan-out path (INSERT ... SELECT + ON CONFLICT on a partial index has no ORM equivalent) — column references stay typed via sql.identifier, all external values are bound parameters.
  • Resend Broadcasts/Audiences and Topics rejected: marketing one-to-many, and Topics would hand a product decision to the vendor while covering only one channel.

Verification

  • pnpm test (monorepo): 17/17 tasks
  • pnpm ci:check: clean (biome, knip, jscpd, type-check)
  • apps/api: 688 pass / 0 fail
  • Fan-out proven against the real DB: org with 3 members + billing.payment.failed → exactly 2 rows (owner + admin), member excluded

Bugs caught in review that no test would have found

  • ownerUserId missing from the actor priority chain — would have notified the wrong account.
  • emailPendingAt: forced ? null : ... made the 11 forced notifications invisible to the email cron, including payment failure and password change.
  • Idempotency key concatenated every batch id, exceeding the 8191-byte btree index limit past ~221 pending rows: the cron would have broken exactly under load.

Known debt

  • Org fan-out path has no permanent unit test (functionally proven, script not kept).
  • notification_preference.scope_id is polymorphic, so no FK: orphans after user/org deletion. Risk is nil (unreadable without an authenticated session); fix is two DELETE ... WHERE NOT EXISTS in the retention cron on a later pass.

https://claude.ai/code/session_01SHZVb6cDRHaSNMJwxqQ8ec

D.3 (in-app notification center) design decisions, benchmarked against
Knock / Novu / Courier / SuprSend:

- fan-out as an OutboxSubscriber in the dispatch TX, not a post-commit
  onEvent handler: batching needs a transactional write, and onEvent
  fails silently
- recipients resolved by capability instead of hardcoded role tuples
  (org-scoping rule #6); roles are static code, so resolution happens
  at boot and costs nothing at runtime
- batching split per channel: in-app groups on read, email batches on
  write through two columns instead of a batch table
- SSE stream carries a signal, never data, which removes Last-Event-ID,
  replay and merge logic; polling survives only as fallback
- one LISTEN connection per instance, never one per client
- no new events: a notification is a read projection of an audited one

D.2 (OpenAPI auto-docs) deferred: no third-party consumer yet, and
every SOTA approach requires rewriting route registration for docs
nobody reads. Activation trigger documented.
- add sql.identifier and notificationSchema stubs to all 16 drizzle mocks
  so the bun parallel mock.module leak exposes the full export superset
- convert notification-trigger.test.ts from db integration test to unit test:
  passes a fake client, inspects emitted DDL for CREATE OR REPLACE,
  notification_created channel, NEW.user_id, and idempotency

Claude-Session: https://claude.ai/code/session_01Qua7utUwpyb5XJmfQb8DNd
Ajoute GET/POST /notifications (liste, unread-count, read, read-all),
GET/PUT /notifications/preferences et GET/PUT /notifications/org-preferences.
Fix les codes d'erreur NotificationError pour matcher AppError (suffix _PROVIDER_FAILURE).

Claude-Session: https://claude.ai/code/session_018jZCHWpXTTbQVbWL1CU58q
la route get org-preferences requiert organization:update comme le put.
un test verifie le refus pour un membre sans la capability.

Claude-Session: https://claude.ai/code/session_018jZCHWpXTTbQVbWL1CU58q
A batch of ~221+ UUIDs joined with " |" exceeds btree's 8191-byte limit,
crashing the entire flush TX. Replace the raw join with its SHA-256 hex
digest (64 chars, constant size). Extract digestIdempotencyKey() for
direct unit-testing; add invariant test that verifies constant length for
2 and 500 ids.

Claude-Session: https://claude.ai/code/session_01ReWK29neXcRT793zPJMjuk
Adds POST /internal/sweep-notifications: purges read notifications older
than NOTIFICATION_RETENTION_DAYS (default 30d). Unread notifications are
preserved regardless of age. Enriches notificationSchema in all drizzle
mocks to expose readAt/createdAt for the test superset rule. Documents
flush-notification-emails and sweep-notifications in docs/CRON.md.
Add notification-map section to docs/EVENTS.md as third catalog
projection after visibility-map (webhooks) and retention-map (purge).
Tick verified D.3 backend checkboxes in ROADMAP.md. Fix em-dashes
in routes.test.ts describe labels.

Claude-Session: https://claude.ai/code/session_01XdAe6D7fZ3zYgrpunUZdZE
forced means "bypass preference cascade and batching window", not
"skip email". a null email_pending_at is invisible to the flush cron
because the partial index filters WHERE email_pending_at IS NOT NULL.
payment failure, password change, and 2fa mutations were silently
never sent by email.

add a test that locks the invariant: a forced event must carry a
non-null email_pending_at equal to event.occurred_at.

Claude-Session: https://claude.ai/code/session_01M3fXj8cAzmhnPjABgFhmYY
add notification.preference.updated and
notification.org_preference.updated to the catalog. both are
internal/compliance -- preference changes are persistent state
mutations that were not audited (rule 6 gap).

for org preferences, actorUserId is a distinct field from
organizationId because the admin acting is not the subject (rule 7).

emit both events from the put /preferences and /org-preferences
routes via emitEvent, consistent with the audit-log route pattern.

Claude-Session: https://claude.ai/code/session_01M3fXj8cAzmhnPjABgFhmYY
…e call

a second call to start() opened a new postgres listen connection while
leaking the first one. the started flag makes subsequent calls no-ops;
stop() resets it so restart is possible.

Claude-Session: https://claude.ai/code/session_01M3fXj8cAzmhnPjABgFhmYY
50 000 rows under a 30-second statement_timeout would reliably abort
the transaction and send nothing. 5 000 is still 10x the default
of 500 and safe within the timeout budget.

Claude-Session: https://claude.ai/code/session_01M3fXj8cAzmhnPjABgFhmYY
the cursor-based list endpoint returned items only; callers had to
derive the next-page cursor from the last item themselves. returning
nextcursor explicitly makes pagination self-describing and removes a
fragile client-side assumption. null when no further page exists.

Claude-Session: https://claude.ai/code/session_01M3fXj8cAzmhnPjABgFhmYY
two preference audit events (notification.preference.updated and
notification.org_preference.updated) were added post-ship. update
all catalog references in the d.3 section and the c.4 as-built row.
reformulate the "no new events" criterion to be accurate: creation
fan-out emits no event; preference mutations do (they are persistent
state changes, not read projections).

Claude-Session: https://claude.ai/code/session_01M3fXj8cAzmhnPjABgFhmYY
put /preferences and /org-preferences now call emitEvent which needs
di.ioutboxrepository. add a mock enqueue so the tests can run.

Claude-Session: https://claude.ai/code/session_01M3fXj8cAzmhnPjABgFhmYY
Two notification preference audit events were legitimately added (D.3).
The guard in webhook-events.test.ts is intentional and must stay strict.
Docs updated: EVENTS.md, FEATURES.md, REMOVABILITY.md (28 public / 39 internal).

Claude-Session: https://claude.ai/code/session_01ScRHBjs4ZRemABn7zdc1Gv
Remove the local-handler registry that caused the emitting tab to
notify itself on post(). The primitive is now a thin wrapper: post
delegates to BroadcastChannel.postMessage, subscribe wires a message
event listener, nothing more.

Rewrite the delivery and unsubscription tests to use two distinct
channel instances (publisher / receiver), matching the native API
contract. Delivery assertions now await a promise resolved by the
handler; a 500 ms guard prevents the suite from hanging if nothing
arrives. The SSR no-BroadcastChannel test is preserved unchanged.

Claude-Session: https://claude.ai/code/session_01W6XKJwLDoxs3ZcMm9uNRvo
…ackages/events

NOTIFICATION_CHANNELS, NOTIFICATION_FREQUENCIES, NOTIFICATION_PREFERENCE_SCOPES
were declared inline in packages/drizzle/src/schema/notification.ts, unreachable
by the front-end. Moving them to @packages/events (already imported by the front)
lets the RPC type inference name them portably, fixing TS2883 on InferResponseType.

- packages/events: export the three const arrays + their derived types
- packages/drizzle: import from @packages/events (new dep), remove inline decls
- api port: import NotificationChannel/Frequency/PreferenceScope from @packages/events
  so Hono RPC inference resolves to a nameable, shared type
- api schema: use NOTIFICATION_CHANNELS/FREQUENCIES from @packages/events
- app queries: replace hardcoded NotificationPreference literal with
  InferResponseType<typeof $preferences, 200>, following the api-tokens pattern
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant