feat(notifications): in-app notification center backend (phase d.3) - #60
Open
axelhamil wants to merge 32 commits into
Open
feat(notifications): in-app notification center backend (phase d.3)#60axelhamil wants to merge 32 commits into
axelhamil wants to merge 32 commits into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backend of the in-app notification center. Front (
<NotificationBell />,/settings/notifications,createBroadcastChannelpromotion) is a separate follow-up plan.What ships
notification-map.tsin@packages/events— third projection of the event catalog, aftervisibility-map(webhooks) andretention-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. OneINSERT ... SELECTjoining members, never a loop.type Audience = "self" | "actor" | "org:all" | { can: OrgPermissions }. Roles are static code, sorolesWith()resolves at boot and leavesWHERE member.role = ANY($1)at runtime: same query a hardcoded role tuple would have produced, without duplicating a decision owned by@packages/access-control.notification+notification_preference, with 3 partial indexes (unread count, dedup, email pending).emailPendingAt/emailSentAt. No batch table: we own the storage, so the batch stays a query instead of state that can desync.GET /notifications/stream, oneLISTENconnection per instance (never per client), heartbeat 25 s, cap 5 streams per user. The stream carries a signal, never data: a reconnect just firesinvalidateQueries, which killsLast-Event-ID, replay and merge logic in one stroke.denyImpersonated, org preferences carryrequireOrgPermission.forcedbypassing all three./internal/*rail, documented indocs/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
organizationIdnullable onnotificationis a documented exception to org-scoping rule 3: a notification is user-scoped by nature,user.password_changedbelongs to no org.INSERT ... SELECT+ON CONFLICTon a partial index has no ORM equivalent) — column references stay typed viasql.identifier, all external values are bound parameters.Verification
pnpm test(monorepo): 17/17 taskspnpm ci:check: clean (biome, knip, jscpd, type-check)apps/api: 688 pass / 0 failbilling.payment.failed→ exactly 2 rows (owner + admin), member excludedBugs caught in review that no test would have found
ownerUserIdmissing 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.Known debt
notification_preference.scope_idis polymorphic, so no FK: orphans after user/org deletion. Risk is nil (unreadable without an authenticated session); fix is twoDELETE ... WHERE NOT EXISTSin the retention cron on a later pass.https://claude.ai/code/session_01SHZVb6cDRHaSNMJwxqQ8ec