@@ -0,0 +1,195 @@ |
| 1 | +# Notifications (S29) |
| 2 | + |
| 3 | +S29 ships in-app notifications + email. The fan-out worker consumes |
| 4 | +from `domain_events` (the unified S26 event log) and materializes per- |
| 5 | +recipient inbox rows + (optionally) sends one email. Per-thread |
| 6 | +coalescing keeps the inbox slim; a per-recipient + per-thread storm |
| 7 | +dampener keeps inboxes sane during bot-spam scenarios. |
| 8 | + |
| 9 | +## Architecture |
| 10 | + |
| 11 | +``` |
| 12 | +internal/notif/ |
| 13 | + notif.go — Deps, FanoutBatch, storm-dampener constants |
| 14 | + routing.go — Reason / Relation / Action; Routing(kind, rel) |
| 15 | + fanout.go — FanoutOnce(ctx, deps): drain N events from cursor |
| 16 | + email.go — sendNotificationEmail + HMAC-signed unsub URL |
| 17 | + emit.go — Emit / EmitTx: insert one domain_events row |
| 18 | + mentions.go — ResolveMentionUserIDs (markdown → user IDs) |
| 19 | + queries/ — sqlc input |
| 20 | + sqlc/ — generated DB code |
| 21 | + |
| 22 | +internal/worker/jobs/notify_fanout.go — wraps FanoutOnce as a worker job |
| 23 | +internal/web/handlers/notifications/ — /notifications + thread sub |
| 24 | +internal/web/templates/notifications/ — inbox.html, unsubscribed.html |
| 25 | +``` |
| 26 | + |
| 27 | +## Migrations |
| 28 | + |
| 29 | +* **0033 — notifications**: |
| 30 | + * `notification_thread_kind` enum (`'issue' | 'pr'`). |
| 31 | + * `notifications` (per-recipient inbox row, with thread-coalesce |
| 32 | + unique partial index on `(recipient_user_id, thread_kind, thread_id)` |
| 33 | + where `thread_id IS NOT NULL`). |
| 34 | + * `notification_threads` (per-recipient, per-thread subscription |
| 35 | + override — the row that the Subscribe / Unsubscribe / Ignore |
| 36 | + handlers write). |
| 37 | + * `notification_email_log` (de-dup + storm dampener input). |
| 38 | + * `domain_events_processed` (cursor table; one row per consumer). |
| 39 | + Seeded with `('notify_fanout', 0)` so the worker's first run |
| 40 | + starts at the head. |
| 41 | + |
| 42 | +## Event flow |
| 43 | + |
| 44 | +``` |
| 45 | +caller (issues/pulls/etc) ──▶ notif.Emit(...) |
| 46 | + │ |
| 47 | + ▼ |
| 48 | + domain_events (one row, NOTIFY shithub_jobs) |
| 49 | + │ |
| 50 | + ▼ |
| 51 | + notify:fanout job |
| 52 | + │ |
| 53 | + ▼ |
| 54 | + FanoutOnce(ctx, deps) |
| 55 | + │ |
| 56 | + ├── drain N events from cursor |
| 57 | + ├── per event, computeRecipients() |
| 58 | + ├── visibility re-check per recipient |
| 59 | + ├── self-suppress |
| 60 | + ├── pickAction() per recipient |
| 61 | + ├── upsertInboxRow (coalesce by thread) |
| 62 | + └── maybeSendEmail (storm-dampened) |
| 63 | +``` |
| 64 | + |
| 65 | +## Routing matrix |
| 66 | + |
| 67 | +`Routing(kind, relation)` is the single source of truth for "who |
| 68 | +gets pinged on what." Today it's a switch; once we have ~30 kinds |
| 69 | +it'll graduate to a table-driven shape. |
| 70 | + |
| 71 | +| Event kind | Relation | Inbox | Email | Override-ignore | Reason | |
| 72 | +|--------------------------------------|-----------------|:----:|:-----:|:---------------:|-------------------| |
| 73 | +| `issue_created`, `pr_opened` | mention | ✓ | ✓ | **yes** | mention | |
| 74 | +| `issue_created`, `pr_opened` | watching=all | ✓ | ✓ | no | watching | |
| 75 | +| `issue_comment_created`, `pr_comment_created` | mention | ✓ | ✓ | **yes** | mention | |
| 76 | +| `issue_comment_created`, `pr_comment_created` | assignee | ✓ | ✓ | no | assignment | |
| 77 | +| `issue_comment_created`, `pr_comment_created` | author | ✓ | ✓ | no | author | |
| 78 | +| `issue_comment_created`, `pr_comment_created` | commenter| ✓ | ✓ | no | commenter | |
| 79 | +| `issue_comment_created`, `pr_comment_created` | subscribed-thread | ✓ | ✓ | no | subscribed | |
| 80 | +| `issue_comment_created`, `pr_comment_created` | watching=all | ✓ | ✓ | no | watching | |
| 81 | +| `issue_assigned`, `pr_assigned` | assignee | ✓ | ✓ | no | assignment | |
| 82 | +| `issue_closed`, `issue_reopened`, `pr_closed`, `pr_reopened`, `pr_merged` | author / assignee / sub / watch | ✓ | ✓/no | no | author/assignment/subscribed/watching | |
| 83 | +| `review_requested` | reviewer | ✓ | ✓ | **yes** | review_requested | |
| 84 | +| `review_submitted` | author / sub | ✓ | ✓/no | no | author/subscribed | |
| 85 | +| `mentioned` (standalone) | mention | ✓ | ✓ | **yes** | mention | |
| 86 | +| `check_failed`, `check_fixed` | author | ✓ | no | no | author | |
| 87 | +| `repo_archived` and S16 lifecycle | repo_owner | ✓ | ✓ | no | repo_admin_action | |
| 88 | + |
| 89 | +Anything else → `Skip` (deny by default). |
| 90 | + |
| 91 | +## Storm dampener |
| 92 | + |
| 93 | +* Per (recipient, thread, time-window): max **1 email per 10 minutes** |
| 94 | + per thread. |
| 95 | +* Per recipient absolute: max **100 emails per hour**. |
| 96 | +* In-app inbox rows are **not** damped (they're cheap and the |
| 97 | + thread-coalesce already collapses noise). |
| 98 | + |
| 99 | +## Auto-subscription rules |
| 100 | + |
| 101 | +When a recipient earns a slot via author / assignee / reviewer / |
| 102 | +mention / commenter, the fan-out worker writes a |
| 103 | +`notification_threads` row with `subscribed=true` if no row exists. |
| 104 | +Explicit user choices (a manual Subscribe / Unsubscribe) win |
| 105 | +because the auto-sub uses `INSERT … ON CONFLICT DO NOTHING`. |
| 106 | + |
| 107 | +## Visibility re-check |
| 108 | + |
| 109 | +The fan-out worker re-checks repo visibility *per recipient* before |
| 110 | +materializing the inbox row. This catches the case where a repo |
| 111 | +flipped from public to private between the event-emit moment and the |
| 112 | +fan-out moment — a stale-read public event must not produce a |
| 113 | +notification for a recipient who no longer has access. |
| 114 | + |
| 115 | +The check uses `policy.IsVisibleTo` (same predicate the rest of the |
| 116 | +runtime uses for "can this viewer see this repo?"). |
| 117 | + |
| 118 | +## One-click unsubscribe |
| 119 | + |
| 120 | +The email body carries an HMAC-signed URL of the shape: |
| 121 | + |
| 122 | +``` |
| 123 | +{baseURL}/notifications/unsubscribe?u={recipientID}&tk={kind}&ti={threadID}&sig={base64(HMAC-SHA256)} |
| 124 | +``` |
| 125 | + |
| 126 | +The handler verifies the HMAC and, on success, writes a |
| 127 | +`notification_threads` row with `subscribed=false, |
| 128 | +reason='email_one_click'`. No session is required — RFC 8058 mailers |
| 129 | +click these from arbitrary agents. |
| 130 | + |
| 131 | +The HMAC key (`Notif.UnsubscribeKeyB64`) is operator-managed; rotating |
| 132 | +it invalidates outstanding unsub links. The dev-mode wiring derives a |
| 133 | +deterministic key from the session secret and logs a WARN — operators |
| 134 | +must set the explicit key in prod. |
| 135 | + |
| 136 | +## Routes |
| 137 | + |
| 138 | +| Method | Path | Auth | Notes | |
| 139 | +|-------|-----------------------------------------|----------|------------------------------------| |
| 140 | +| GET | `/notifications` | required | Inbox (filter=unread tab) | |
| 141 | +| POST | `/notifications/{id}/read` | required | Mark one read | |
| 142 | +| POST | `/notifications/{id}/unread` | required | Mark one unread | |
| 143 | +| POST | `/notifications/mark-all-read` | required | Bounded sweep | |
| 144 | +| POST | `/threads/{kind}/{id}/subscribe` | required | Per-thread subscribe override | |
| 145 | +| POST | `/threads/{kind}/{id}/unsubscribe` | required | Per-thread unsubscribe override | |
| 146 | +| GET | `/notifications/unsubscribe` | none | One-click HMAC-signed unsub | |
| 147 | + |
| 148 | +## What we deferred from the spec |
| 149 | + |
| 150 | +* **API endpoint** `GET /api/v1/notifications` + the per-thread sub |
| 151 | + PUTs. Defer to S33 / S34 API consolidation (matches the S28 |
| 152 | + search-API deferral). |
| 153 | +* **Per-kind HTML email templates**. Today every email uses a |
| 154 | + minimal HTML+plaintext layout that includes the reason, event |
| 155 | + kind, and link. Per-kind templates ride a follow-up that touches |
| 156 | + the email surface. The List-Unsubscribe URL is still wired in |
| 157 | + the body footer. |
| 158 | +* **List-Unsubscribe HTTP header** (RFC 8058). The |
| 159 | + `email.Message` shape from S05 doesn't carry arbitrary headers; |
| 160 | + adding `Headers map[string]string` ripples through SMTP + |
| 161 | + Postmark backends. Tracked as a follow-up; the URL is still |
| 162 | + emitted in the body so the one-click flow works. |
| 163 | +* **PR review submission, PR merge, check-failed/fixed event |
| 164 | + emission**. The routing matrix is wired for these kinds; the |
| 165 | + emit-side hooks land when the spec touches the PR review and |
| 166 | + check pipelines next. |
| 167 | +* **S16 lifecycle email kinds emission** (`repo_archived` etc). |
| 168 | + Routing is wired; the emit-side hook lands when the lifecycle |
| 169 | + ops next get touched. |
| 170 | +* **Daily digest mode**. Schema accommodates (one inbox row per |
| 171 | + thread); the cron + render path is post-MVP. |
| 172 | +* **Reply-by-email** is post-MVP per the spec. |
| 173 | + |
| 174 | +## Pitfalls noted in code |
| 175 | + |
| 176 | +* **Visibility leak**: highest-stakes risk; addressed by the |
| 177 | + per-recipient re-check at fan-out time. Test: |
| 178 | + `TestFanout_VisibilityRecheck_PrivateRepo`. |
| 179 | +* **Email storm**: dampener caps. Test: |
| 180 | + `TestFanout_StormDampener_SingleEmailPerThread`. |
| 181 | +* **Thread coalescing**: 5 events → 1 inbox row. Test: |
| 182 | + `TestFanout_ThreadCoalescing`. |
| 183 | +* **Self-notification suppression**: dropped at recipient-compute |
| 184 | + time AND defense-in-depth at dispatch. Test: |
| 185 | + `TestFanout_SelfSuppression`. |
| 186 | +* **Override-ignore for mentions**: routing flag honored over the |
| 187 | + `watches.level=ignore` gate. Test: |
| 188 | + `TestFanout_MentionOverridesIgnore`. |
| 189 | +* **Unsubscribe HMAC tampering**: signature mismatch invalidates. |
| 190 | + Test: `TestUnsubscribe_HMACRoundtrip`. |
| 191 | +* **Suspended user as actor**: drop at recipient-compute when the |
| 192 | + primary email is unverified or the user is soft-deleted. Mention |
| 193 | + resolver also filters suspended. |
| 194 | +* **Event ordering**: fan-out drains in `id ASC` order; |
| 195 | + `domain_events.id` is bigserial so commit order is preserved. |