Notification choke point with important flag

Task 9370 · Done · Coder · created · updated

Part of epic 9326 (Jaah API keys). Child C2 of the Architect's decomposition: the per-person notification choke point that API-key expiry reminders go through. Independent of the keys backend: it imports nothing of keys. No migration. ## Original requirement (verbatim) > Implement Jaah API keys workflow. > - Permissions: same as we have internally: subject:action. Access to object is limited to the owner of the key. Should be very easy to configure on the UI. By default all is max, but user can remove some > - API Keys can only be created by at least Org Admin > - Keys have expiration > Do a web research and suggest the most modern approach to this for this type of systems ## Requirement this child delivers (PRD R13, verbatim) **R13 Expiry reminders** to the owner 7 days and 1 day before expiry (none for "No expiration" keys). **In-app and email mandatory, plus every other channel the user has configured.** Sent through the **single standard notification choke point** with an **"important" flag that forces the email channel**; the notifier does not know or choose the user's channels. (D10) Recon finding: no such choke point exists today — notifiers hand-roll channels (`governance/notify.py` writes an `alerts` row + `email_notify.deliver_to_user` + telegram; `alert_record.py` does Slack/email itself; no `important` flag anywhere). This build introduces that choke point (resolve user's configured channels; `important` ⇒ email always) and routes API-key reminders through it; migrating the existing notifiers onto it is a deferred follow-up. (This child builds the choke point; the sweep child calls it.) ## Approach (Architecture §7, pinned seam with the sweep child) ```python # apps/api/app/user_notify.py def deliver(session, user: AdminUser, event: dict, *, event_name: str, important: bool = False) -> Delivery ``` - Push channels resolved by the choke point itself: Slack (`notify_targets.target_for_user`), email (`email_notify.deliver_to_user`), Telegram (`telegram_notify.deliver_to_user`). Extract the per-person leg that `comment_notify.py:95-129` hand-rolls (do not migrate `comment_notify` itself — deferred). - `important=True`: email ALWAYS sent to the login email, bypassing the per-event email preference, via a new `email_notify.deliver_important(user, event)`. Without deployment email config the outcome is `skipped: no_email_config`. Other channels follow the person's preferences. - No channel argument exists. `Delivery` is a frozen per-channel outcome. - Writes one `notification_sent` activity row for the user (data: `event_name`, `important`, per-channel `sent`/`skipped`/`failed`) — AC14's delivery record (D-A5). - In-app half: by the inbox rule (`routes/inbox.py` header, G6: no notifications table) the in-app half is a derived inbox kind fed by the notifier's own rows; the choke point's docstring states this rule (single home). - Preferences: `api_key_expiry` joins `NOTIFICATION_EVENTS` (`apps/api/app/preferences.py:47`), default on, AND its web mirror `apps/web/src/lib/notificationPrefs.ts` (the file says it mirrors `NOTIFICATION_EVENTS`) with a short label (e.g. "API key expiry") so the preferences panel renders it. Decided: add it to `ORG_ADMIN_EVENTS` too, because only Org Owners/Admins can hold keys. Tests: `important` forces email despite the email preference being off; Slack and Telegram follow preferences; exactly one `notification_sent` row per call; `deliver` has no channel parameter (signature test); no-email-config → `skipped: no_email_config`. Credentials & reach: - AC14 email | The `notification_sent` row on dev `/audit` + this child's API test | **verified** for the delivery record. **Inbox arrival deferred**: no mailbox credential on the roster; the operator chose the delivery record + tests (D-A5). - AC14 other channels (e.g. Slack) | The same `notification_sent` row (per-channel outcome) + this child's API test | **deferred** for arrival, on D-A5's reasoning: no credential to read a person's Slack/Telegram DMs. Out of scope: Do NOT migrate existing notifiers (`alert_record.py`, `governance/notify.py`, `comment_notify.py`, `task_notify.py`) onto the new choke point — deferred. Do NOT add a notifications table. Do NOT build the API-key sweep, cron, inbox kind or any `api_keys` code (other children). No change to cookie-session auth behaviour. Done means: - AC14 (choke-point half) — Notifier code calls the one choke point with `important=True` and names no channel (code review check): `user_notify.deliver` has no channel parameter; an API test proves `important=True` sends email with the person's email preference for the event OFF; Slack/Telegram are sent only per the person's preferences; each call writes one `notification_sent` row with per-channel outcomes. - `api_key_expiry` appears in `NOTIFICATION_EVENTS` and in `notificationPrefs.ts` with a readable label, shown to Org Owners/Admins in the notification preferences panel. - New tests green; `./dc check quick` green.

Outcome

Built user_notify.deliver (no channel arg; important forces verified email) + notification_sent audit row; api_key_expiry event added (Owner/Admin, web label). Quick gate + tests green; PR #6733 queued to lane. Visual check done: row renders cleanly. Deferred: migrating old notifiers. Memory: used mem-15203; proposed 0.

Pull request

Artifacts

Activity

  • Draft
  • Ready
  • Runner started
  • Runner ended (done)