(modeling-commons): Durable Notifications + In-App Notifications

.claude/DECISIONS.md

@@ -181,3 +181,14 @@ Critical, non-obvious decisions made while working in this repo. Newest first.
 3. **"Nothing loaded yet" is `lastPage: null`, not `0`.** A node at the nesting limit ships `{ count, data: [] }`, so `toPagination` maps any empty page to `null` and `nextPageToLoad()` (in `comment-tree.ts`) starts it at page 0 instead of skipping page 0 outright.
 4. **Frontend tree defaults are display-only.** `COMMENT_TREE_DEFAULTS` → `COMMENT_VIEW_DEFAULTS` with just `maximumNested`; the `maximumShownRepliesPerLevel` prop is deleted from `CommentView`/`CommentsPanel` (it sliced nothing — it only leaked a page size into the request path).
 5. **`installCommentFetchMock` now pages like the server:** `toApiComment` takes `{page, limit}`, slices replies at `offset = page * limit`, embeds nested levels at the server's size (2), and the thread GET honors the querystring. A `roots` option serves custom trees. The old mock returned every fixture reply at page 0, which is why the bug was invisible to the component suite.
+
+## 2026-07-28 — Notification feed: `GET /v1/me/notifications` + mark-read
+
+**Context:** `apps/modeling-commons-backend/src/modules/user-notification`. The durable pipeline (2026-07-27) deliberately shipped without a read layer — `doc/notification-pipeline-plan.md` lists the in-app feed API as out of scope, with the ledger already carrying `title`/`body`/`url`/`readAt` so it could be added without schema churn. That read layer now exists.
+
+**Decisions:**
+1. **The ledger is not the feed.** `UserNotification` rows are written whenever *any* channel is on (the unique key is the email-idempotency guard), so the feed re-derives the visible set: `listUserNotificationsQuery` resolves the caller's preferences through `userNotificationDomain.inAppEnabledCategories(overrides)` and passes that list as a `category IN (...)` filter. A category muted in-app is still emailed and still leaves a row, but never appears in the feed. Muting everything short-circuits to an empty page with no DB read.
+2. **`unreadCount` rides along in the paginated response** (`user-notification.paginated.response.dto.ts`) and counts every unread in-app notification, ignoring `since`/`unreadOnly`/paging — a poll for the bell badge is one request, not two.
+3. **A read receipt is not an audit event.** `userNotificationService.markRead` does a single `update` outside `transactionManager`/the outbox, like `markEmailSent`. Routing it through the event table would feed the event processor its own noise for zero audit value. It is idempotent: an already-read row is a no-op.
+4. **Mark-read 404s on someone else's notification** rather than 403 (`assertOwnedByRecipient` → `NotificationNotFoundError` for both missing and not-owned), so the endpoint doesn't confirm that an id exists.
+5. **`NOTIFICATION_CATEGORIES` is the const tuple behind `NotificationCategory`**, and the response DTO's `category` is `Type.Enum(NOTIFICATION_CATEGORIES)` — typebox v1.1.x's `Type.Enum` takes a readonly value tuple and infers the literal union, so the catalog stays the single source of truth. (`Type.Union([...arr.map(Type.Literal)])` does *not* infer — it widens to `never`.)

apps/modeling-commons-backend/.dependency-cruiser.cjs

@@ -7,6 +7,7 @@ const applicationLayerPaths = [
   '\\.service\\.ts$',
   '\\.handler\\.ts$',
   '\\.event-handler\\.ts$',
+  '\\.notifier\\.ts$',
 ];
 
 const infrastructureLayerPaths = ['infrastructure', 'infra', 'database', 'repository'];

apps/modeling-commons-backend/.ongoing/README.md

@@ -0,0 +1,29 @@
+# .ongoing
+
+Plans for future work. Nothing here is implemented until its task file is picked up and approved.
+
+## Layout
+
+`.ongoing/<project>/<task>/<pr>-<n>.md`
+
+- `<project>` — optional, nestable (`billing/invoicing/...`).
+- `<task>` — one coherent unit of work.
+- `<pr>-<n>.md` — one file per planned PR; `<n>` is the intended merge order within the task (`schema-1.md`, `endpoints-2.md`).
+
+## PR file contract
+
+A PR is a very small part of the task with one clear goal. Required fields:
+
+- **Goal** — one sentence.
+- **In scope** / **Out of scope**
+- **Description**
+- **Acceptance criteria** — verifiable.
+- **Depends on** — other PR files, or `none`.
+- **Notes**
+
+Additional fields (Risks, Rollback, Open questions, Status, …) at the author's discretion.
+
+## Rules
+
+- Can't state acceptance criteria → it's a task, not a PR. Split it.
+- Delete a PR file when it merges. Stale plans are noise.

apps/modeling-commons-backend/.ongoing/durable-notifications/comment-notifier-5.md

@@ -0,0 +1,95 @@
+# comment-notifier-5 — Move comment notifications onto the durable pipeline
+
+**Goal** — Move comment notification wording into a notifier owned by `model-comment`, and
+delete the inline fire-and-forget send.
+
+## In scope
+
+- `src/modules/model-comment/notifications/model-comment.notifier.ts` (new).
+- `src/modules/model-comment/notifications/model-comment.notifier.spec.ts` (new).
+- `src/modules/model-comment/index.ts` — declare `modelCommentNotifier`.
+- `src/modules/user-notification/user-notification.service.ts` — add `modelCommentNotifier` to
+  the notifiers array.
+- `src/modules/model-comment/model-comment.service.ts` — delete the notification code.
+- `src/modules/model-comment/model-comment.service.spec.ts` — drop the notification block.
+- `tests/api/model-comment.feature` / `.steps.ts` and `tests/api/user-notification.feature` /
+  `.steps.ts` — move the two notification scenarios.
+
+## Out of scope
+
+- Changing who gets notified, or the wording of either email. This is a move, not a redesign.
+- Notifiers for `model_comment.updated` or `model_comment.deleted`. Both events are emitted;
+  neither notifies anyone today and neither should start here.
+- Comment likes. They send nothing and write no event row today; unchanged.
+
+## Description
+
+The notifier subscribes to `model_comment.created` and resolves the comment from
+`event.payload.commentId` — the event row is now the only input, so it no longer receives the
+entity from the caller.
+
+Everything moves across with semantics unchanged:
+
+- `truncatePreview` (`model-comment.service.ts:9-12`), now using
+  `rules.limits.notification.previewLength`.
+- `buildEmailModel` (`:55-76`), including its `getModelCardQuery` failure fallback to
+  `{ name: 'a model', url }`.
+- The thread deep link (`:81-89`): a reply's own URL opens detached, so the URL roots at
+  `parent?.id ?? entity.id` and sets `highlightedCommentId` to the new comment.
+- The recipient set (`:99-105`): the parent comment's author gets
+  `createRepliedToCommentEmail` under `comment.reply_to_you` unless they are the commenter;
+  every other `ModelAuthor`, minus the commenter and minus the parent author, gets
+  `createCommentedOnModelEmail` under `comment.on_your_model`.
+
+Two things change. The notifier returns `NotificationIntent[]` instead of calling `mailService`,
+and `unsubscribeUrl` comes from the `links` argument instead of
+`` `mailto:${env.product.supportEmail}` ``. Each intent's `buildEmail` closes over the already-
+computed model card and preview, so the card query runs once per event rather than once per
+recipient.
+
+Then `model-comment.service.ts` loses `truncatePreview`, `buildEmailModel`,
+`notifyOnNewComment`, and the `void notifyOnNewComment(entity, parent)` call at line 209, plus
+the now-unused dependencies `modelAuthorRepository`, `userRepository`, `getModelCardQuery`,
+`mailService`, `mailDomain` and the `env` / `EmailModel` imports. It becomes purely
+transactional.
+
+## Acceptance criteria
+
+- Posting a comment on a model with an owner and a contributor, then draining the queue,
+  produces exactly two emails; neither goes to the commenter.
+- Commenting on your own model produces no email.
+- Replying to someone else's comment sends them the reply template exactly once, and they do
+  not also receive the commented-on-model template.
+- The `commentUrl` on every intent contains `highlightedCommentId=<new comment id>` and roots
+  at the parent's id for a reply, the comment's own id for a top-level comment.
+- A `getModelCardQuery` failure still yields an email, with the model name `'a model'`.
+- Recipients who opted out of the relevant category receive nothing, and no ledger row is
+  written for them.
+- `getModelCardQuery.execute` is called once per event, not once per recipient.
+- `model-comment.service.ts` no longer references `mailService`, `mailDomain`, or `env`.
+- `model-comment.service.spec.ts` retains only transactional assertions and no longer defines
+  `flushMicrotasks`.
+- `yarn run check`, `yarn run test:unit`, `yarn run test:e2e` pass.
+
+## Depends on
+
+`dispatch-4.md`
+
+## Notes
+
+The notifier spec should carry over the assertions currently in `model-comment.service.spec.ts`
+rather than being written fresh — they encode behavior worth preserving exactly, including the
+positional-argument assertion on `commentUrl`.
+
+The two e2e scenarios at `tests/api/model-comment.feature:179` ("Commenting notifies other
+authors but never the commenter") and `:193` ("Commenting on your own model does not notify
+yourself") assert inline sending and will fail as written. Move them into
+`user-notification.feature` and trigger the batch with `boss.send('process-events', {})`, as
+`tests/api/workers.steps.ts:42-45` already does. The mail-capturing monkey-patch and
+`waitForMailCalls` helper at `tests/api/model-comment.steps.ts:314-344` move with them.
+
+Delivery is now up to ~60s slower (the cron interval). Expected and accepted.
+
+Once this merges, `mailDomain.createCommentedOnModelEmail` and `createRepliedToCommentEmail`
+have exactly one caller each — the notifier — which is the intended shape: the producing module
+picks its own template.

apps/modeling-commons-backend/.ongoing/durable-notifications/dispatch-4.md

@@ -0,0 +1,101 @@
+# dispatch-4 — Event dispatch to the notification service
+
+**Goal** — Wire the event processor to a dispatcher that fans durable events out to the
+notification service, with preference gating and ledger idempotency.
+
+## In scope
+
+- `src/modules/user-notification/domain/user-notification.types.ts` — add `NotificationIntent`,
+  `NotificationRecipient`, `NotificationLinks`, `Notifier`, `EventSubscriber`.
+- `src/modules/user-notification/user-notification.service.ts` — `handles` + `handleEvent`.
+- `src/modules/user-notification/user-notification.service.ts` — build `NotificationLinks`:
+  `unsubscribeUrl` is `` `mailto:${env.product.supportEmail}` `` (today's behaviour, unchanged),
+  `preferencesUrl` is `${env.product.website}/settings/notifications`.
+- `src/modules/event/event-dispatcher.service.ts`
+- `src/workers/event-processor.ts` — call `dispatch`, record `markFailed` on throw.
+- `src/workers/index.ts` — thread `eventDispatcherService` through.
+- `src/config/rules.ts` — `limits.notification`: `eventBatchSize: 50`, `maxEventAttempts: 5`,
+  `previewLength: 280`.
+- `src/server/di/index.ts` — add `notifier` to the first glob; delete the second glob.
+- `.dependency-cruiser.cjs` — add `'\\.notifier\\.ts$'` to `applicationLayerPaths`.
+
+## Out of scope
+
+- Any actual notifier. `notifiers` is an empty array in this PR, so `handles` returns false for
+  everything and dispatch is a no-op end to end.
+- Touching `model-comment.service.ts`. The inline send keeps working until `comment-notifier-5`.
+- A ledger sweeper for rows with a null `emailSentAt`.
+
+## Description
+
+`eventDispatcherService` holds an explicit subscriber array — `[userNotificationService]` —
+filters by `handles(event.type)`, runs the survivors under `Promise.allSettled` so one failure
+cannot block another, logs each rejection, and rethrows an `AggregateError` if any failed so the
+processor records the attempt.
+
+`userNotificationService.handleEvent` resolves intents from its notifiers, then per intent:
+
+1. Load the recipient; skip if missing, soft-deleted, banned, or without an email — no ledger
+   row for someone who could never receive it.
+2. Resolve the preference; skip the email channel on `email: false`, skip the ledger row on
+   `inApp: false`.
+3. Insert the ledger row inside `transactionManager.run`. A unique violation on
+   `(eventId, recipientId, category)` means an earlier pass already delivered this — skip.
+4. Invoke `intent.buildEmail(recipient, links)` and send.
+5. Stamp `emailSentAt` on success; on failure log and leave it null.
+
+`buildEmail` is a thunk rather than a rendered email so nothing is rendered for a recipient who
+opted out. `links` is passed in rather than built by each notifier so that the deferred
+unsubscribe-token work changes one file instead of every notifier — see "Unsubscribe: deferred"
+in `doc/notification-pipeline-plan.md`. Its values are constants for now.
+
+The processor changes from unconditional `markProcessed` to:
+
+```ts
+try { await eventDispatcherService.dispatch(event); await eventRepository.markProcessed(event.id); }
+catch (error) { await eventRepository.markFailed(event.id, error); }
+```
+
+so a failed event stays unprocessed and is retried on the next tick until `attempts` reaches
+`maxEventAttempts`.
+
+The second `loadModules` call in `src/server/di/index.ts` (lines 33-44) is deleted rather than
+extended. It globs `*.{handler,event-handler}`, matches zero files, and its `asyncInit: 'init'`
+resolver option fires inside Fastify's `onReady` hook — which runs *after* `startWorkers` at
+`src/server/index.ts:82`. Anything that self-registered there would race the worker it registers
+with. Notifiers go in the first glob as plain singletons instead.
+
+## Acceptance criteria
+
+- The processor calls `dispatch` once per fetched event, then `markProcessed` on success.
+- A subscriber that throws produces `markFailed` with `attempts` incremented and `lastError`
+  populated, and does not abort the remaining events in the batch.
+- An event whose `attempts` has reached `maxEventAttempts` is not returned by `findUnprocessed`.
+- Two subscribers, one throwing: the other still runs to completion.
+- `handleEvent` with a recipient who has `email: false` for the category sends no mail.
+- `handleEvent` on an event whose ledger row already exists sends no mail and does not throw.
+- `buildEmail` is never invoked for an intent filtered out by preferences or recipient state.
+- A `sendMail` rejection leaves `emailSentAt` null, is logged, and does not throw out of
+  `handleEvent`.
+- `yarn run check` passes, including `deps:validate` with the new `applicationLayerPaths` entry.
+- `yarn run test:unit` and `yarn run test:e2e` pass; existing `workers.feature` still goes green.
+
+## Depends on
+
+`mail-await-1.md`, `preferences-3.md`
+
+## Notes
+
+Nothing is observable to a user after this PR. That is intentional — every seam is unit-tested
+in isolation before the cutover in `comment-notifier-5` gives it real traffic.
+
+The subscriber array is explicit rather than discovered by container enumeration. With one
+subscriber, enumeration would be indirection without payoff; adding the FTS indexer from
+`[[legacy-migration-search-spec]]` later is one line.
+
+`userNotificationService` will resolve notifiers by *name* from the awilix cradle in the next
+PR, so there is no static import edge from `user-notification` to any producing module. Keep it
+that way — the `Notifier` type is imported by producers, never the reverse.
+
+`previewLength: 280` in `rules.ts` is unused until `comment-notifier-5` consumes it; it is added
+here to keep all three constants in one commit.

apps/modeling-commons-backend/.ongoing/durable-notifications/mail-await-1.md

@@ -0,0 +1,63 @@
+# mail-await-1 — Await SMTP delivery in mailService
+
+**Goal** — Make `mailService.sendMail` resolve only after nodemailer reports the delivery
+outcome, so callers can observe failures.
+
+This PR is done.
+
+## In scope
+
+- `src/modules/mail/mail.service.ts` — promisify the nodemailer callback.
+- A new `src/modules/mail/mail.service.spec.ts` covering both outcomes.
+
+## Out of scope
+
+- Retry logic, queueing, or backoff. This PR only makes the outcome *knowable*.
+- The three direct `transporter.sendMail` call sites in `src/lib/auth.ts` (lines 59-69, 85-90,
+  98-103). They bypass DI and use `void … .catch(console.error)`; leave them alone.
+- Changing any caller's behavior.
+
+## Description
+
+`sendMail` is declared `async` but wraps nodemailer's callback API without bridging it:
+
+```ts
+async sendMail(content: Mail.Options) {
+  transporter.sendMail(content, (error, info) => {
+    if (error) { logger.error({ name: 'Mail Service', message: 'Failed to send email', error, info }); }
+    else { logger.info({ name: 'Mail Service', message: 'Email sent successfully', info }); }
+  });
+}
+```
+
+The returned promise resolves as soon as `transporter.sendMail` is *called*, not when it
+completes. Every `await mailService.sendMail(...)` in the codebase is therefore a no-op wait,
+and the delivery result is only ever visible in the logs.
+
+Wrap the callback in a promise that resolves on success and rejects on error, keeping both log
+lines exactly as they are so log output does not change.
+
+This is a prerequisite for the durable pipeline: `dispatch-4` stamps `emailSentAt` on a ledger
+row after `sendMail` resolves, which is meaningless while it resolves unconditionally.
+
+## Acceptance criteria
+
+- `await mailService.sendMail(...)` rejects when the transporter invokes its callback with an
+  error, and resolves when it invokes it with `info`.
+- Both existing log lines still fire, with unchanged `name` and `message` values.
+- `mail.service.spec.ts` covers success and failure, stubbing the transporter.
+- `yarn run check` and `yarn run test:unit` pass.
+
+## Depends on
+
+none
+
+## Notes
+
+Low risk. The only DI caller today is `notifyOnNewComment` in `model-comment.service.ts`, which
+already awaits inside `Promise.allSettled` and logs rejections at lines 147-156 — so the change
+activates error handling that was written but unreachable, rather than introducing a new
+failure path.
+
+`src/lib/auth.ts` imports `transporter` directly rather than going through `mailService`, so it
+is unaffected.