diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b91c05..f70149d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,49 @@ so this log can be regenerated from history (e.g. with `git-cliff`). ## [Unreleased] +### Added +- **Notifications and an inbox.** An `@mention` in a comment used to fire a + `pg_notify('comment_mentions', …)` that no process in the codebase ever + listened for, into a `MSG_MENTION` frame the frontend reserved and never + received. Mentioning a colleague did nothing they would ever see. Five events + now write to a `notifications` table — mentions, replies in threads you are + part of, task assignment, overdue tasks, and documents shared with you — read + through an Inbox in the sidebar with an unread badge and a `/notifications` + page. Delivery is a 30-second poll that also refreshes on window focus; the + table is shaped as an outbox (`emailed_at`) so email can arrive later without + a migration. Idempotency is a unique index on `(user_id, dedupe_key)` rather + than application logic, which is what lets the overdue sweep run on every + replica with no leader election. +- **Two checklist items with identical text assigned to the same person notify + once.** The notification's dedupe key is content-addressed precisely so that + reordering a checklist does not re-notify everyone; the cost is that identical + text collides. The assignee still sees both items on `/tasks`. +- **Self-assignment is not suppressed when you assign yourself by typing.** The + guard that drops self-notifications needs to know who acted, but the path a + live edit takes does not carry that: WebSocket updates are persisted with no + user id, and the channel feeding the reindex worker carries only a document + id. Editing a checklist item that assigns you a task will notify you about it; + the guard does work on the import paths. + +### Fixed +- **Members whose display name contains a space could not be mentioned when + opening a thread or replying.** Comment mentions were resolved server-side + by matching `@(\w+)` against member display names, so `@Christian Hüning` + captured `Christian`, matched nobody, and silently notified no one. The + mention picker now sends the user ids it resolved, unioned with whatever the + regex still matches so a hand-typed second name is never dropped; the regex + remains as the sole path for comments written before this release. Editing + an existing comment does not yet carry a `mentions` field, so adding + `@Christian Hüning` in an edit still notifies nobody — only the create path + is fixed here. +- **A document title longer than about fifteen characters was cut off behind + the tool icons.** On the desktop column the title shared its line with the + header's action row, and the row's ~490px left the title only 208px of the + 712px measure — an `` never wraps, so everything past that was + simply clipped. The title now takes the whole column and the action row + sits beneath it, so any title up to roughly 45 characters is shown in full. + Narrow screens already laid the header out this way; it is now the layout + at every width. (#29) ## [0.7.0] - 2026-10-02 ### Added diff --git a/Cargo.lock b/Cargo.lock index 243081a..f3a0e90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2117,6 +2117,7 @@ dependencies = [ "rand 0.10.2", "rust-s3", "serde", + "serde_json", "sha2 0.10.9", "sqlx", "thiserror 2.0.20", diff --git a/crates/knot-obs/src/metrics.rs b/crates/knot-obs/src/metrics.rs index a4f9b53..3f7335e 100644 --- a/crates/knot-obs/src/metrics.rs +++ b/crates/knot-obs/src/metrics.rs @@ -70,6 +70,12 @@ pub fn init(addr: &str) -> Result<(), MetricsError> { "Collab socket upgraded -> initial document state queued to the client" ); + // Notifications + describe_counter!( + "knot_notifications_emitted_total", + "Notifications written to the inbox, by kind" + ); + // Storage / pool describe_gauge!("knot_db_pool_size", "Total connections in the pool"); describe_gauge!("knot_db_pool_idle", "Idle connections in the pool"); diff --git a/crates/knot-server/src/lib.rs b/crates/knot-server/src/lib.rs index 1aa33d6..06be8cb 100644 --- a/crates/knot-server/src/lib.rs +++ b/crates/knot-server/src/lib.rs @@ -12,10 +12,10 @@ use knot_auth::{Hasher, Throttle}; use knot_config::Config; use knot_docs::AclCache; use knot_storage::{ - BlobMeta, BlobStore, CommentStore, DocStore, GrantStore, MarkdownCacheStore, PgBytesStore, - PgCommentStore, PgDocStore, PgGrantStore, PgMarkdownCache, PgSearchStore, PgSessionStore, - PgShareTokenStore, PgUserStore, PgWorkspaceStore, Pool, SearchStore, SessionStore, - ShareTokenStore, UserStore, WorkspaceStore, + BlobMeta, BlobStore, CommentStore, DocStore, GrantStore, MarkdownCacheStore, NotificationStore, + PgBytesStore, PgCommentStore, PgDocStore, PgGrantStore, PgMarkdownCache, PgNotificationStore, + PgSearchStore, PgSessionStore, PgShareTokenStore, PgUserStore, PgWorkspaceStore, Pool, + SearchStore, SessionStore, ShareTokenStore, UserStore, WorkspaceStore, }; use uuid::Uuid; @@ -29,6 +29,7 @@ pub mod board_room_shim; pub mod comments_listener; pub mod http_error; pub mod metrics; +pub mod notifications_sweep; pub mod protocol; pub mod reindex; pub mod room; @@ -59,6 +60,7 @@ pub struct AppState { pub boards: Option>, pub board_rooms: Option>, pub tasks: Option>, + pub notifications: Option>, pub hasher: Arc, pub throttle: Arc, pub session_key: Vec, @@ -95,6 +97,7 @@ impl AppState { boards: None, board_rooms: None, tasks: None, + notifications: None, hasher: Arc::new(Hasher::new()), throttle: Arc::new(Throttle::new()), session_key: Vec::new(), @@ -136,6 +139,8 @@ impl AppState { Arc::new(knot_storage::PgBoardStore::new(pool.clone())); let tasks: Arc = Arc::new(knot_storage::PgTaskStore::new(pool.clone())); + let notifications: Arc = + Arc::new(PgNotificationStore::new(pool.clone())); Self { pool: Some(pool), users: Some(users), @@ -156,6 +161,7 @@ impl AppState { boards: Some(boards), board_rooms: None, tasks: Some(tasks), + notifications: Some(notifications), hasher: Arc::new(Hasher::new()), throttle: Arc::new(Throttle::new()), session_key: Vec::new(), diff --git a/crates/knot-server/src/main.rs b/crates/knot-server/src/main.rs index a555245..f5bad84 100644 --- a/crates/knot-server/src/main.rs +++ b/crates/knot-server/src/main.rs @@ -274,6 +274,10 @@ async fn run_server(cfg: Config) { let _handle = knot_server::comments_listener::spawn(pool, rooms); tracing::info!("comments listener spawned"); } + if let Some(pool) = state.pool.clone() { + let _handle = knot_server::notifications_sweep::spawn(pool); + tracing::info!("notification sweep spawned"); + } // Token shared with every collab socket; cancelled on SIGTERM so they // send a clean 1001 Close and drain instead of being severed mid-rollout. diff --git a/crates/knot-server/src/notifications_sweep.rs b/crates/knot-server/src/notifications_sweep.rs new file mode 100644 index 0000000..88e8311 --- /dev/null +++ b/crates/knot-server/src/notifications_sweep.rs @@ -0,0 +1,116 @@ +//! Periodic notification work: overdue tasks, and retention. +//! +//! Runs on every replica with no leader election. Both halves are safe to +//! run concurrently — the overdue emit is `ON CONFLICT DO NOTHING` against +//! `notifications_dedupe`, and the prune (delegated to +//! `PgNotificationStore::prune`) is idempotent by construction. + +use chrono::{DateTime, Utc}; +use knot_storage::{NotificationStore, NotificationStoreError, PgNotificationStore}; +use sqlx::PgPool; +use tokio::task::JoinHandle; + +const INTERVAL: std::time::Duration = std::time::Duration::from_secs(15 * 60); + +#[derive(Debug, Default, PartialEq, Eq)] +pub struct SweepOutcome { + pub due_emitted: u64, + pub pruned: u64, +} + +/// Emit `task_due` for every open, assigned, task that went overdue in the +/// last 7 days, then prune. +/// +/// The dedupe key is content-addressed exactly like `task_assigned`'s +/// (`task_due::::`, computed with +/// the same SQL sha256 expression `PgTaskStore` uses at +/// `crates/knot-storage/src/tasks.rs`) rather than keyed on `doc_tasks.id`. +/// `doc_tasks.id` is `":"`, so inserting a checklist +/// item above an overdue one shifts every later item's id — an id-keyed +/// dedupe key would treat that reorder as a brand-new task and re-notify +/// every overdue assignee the same day. The date suffix still makes the key +/// a pure function of `(task content, assignee, day)`, so an overdue task +/// notifies once a day rather than once every fifteen minutes. +/// +/// The `t.due_at > $1 - interval '7 days'` floor exists so that on an +/// existing deployment — where `doc_tasks` already has rows with `due_at` +/// stretching back to whenever the workspace was created, and +/// `notifications` starts out empty — the first sweep after this feature +/// ships does not treat every task that has *ever* been overdue as newly +/// overdue and fire a `task_due` burst for the entire historical backlog. +/// Only work that went overdue recently is worth a push; anything older +/// than that is still visible on `/tasks` without a notification. +pub async fn run_once(pool: &PgPool, now: DateTime) -> Result { + // The date half of the dedupe key is computed here, in Rust, from `now` + // — not with `to_char(...)` in SQL. `to_char` renders using the + // connection's session `TimeZone` GUC, which nothing in this repo ever + // sets; today every replica happens to inherit the same server default + // so the key is stable, but that's incidental, not enforced. Binding a + // pre-formatted date string makes the key a pure function of `now`, + // matching the design's claim that correctness lives entirely in the + // unique index, not in ambient session state. + let date = now.format("%Y-%m-%d").to_string(); + let emitted = sqlx::query( + "INSERT INTO notifications \ + (workspace_id, user_id, actor_id, kind, doc_id, target_kind, target_id, dedupe_key, data) \ + SELECT t.workspace_id, t.assignee_user_id, NULL, 'task_due', t.doc_id, 'task', t.id, \ + 'task_due:' || t.doc_id || ':' || \ + encode(substring(sha256(convert_to(t.text, 'UTF8')) from 1 for 8), 'hex') || \ + ':' || t.assignee_user_id || ':' || $2, \ + jsonb_build_object('excerpt', t.text, 'due_at', t.due_at) \ + FROM doc_tasks t \ + WHERE t.assignee_user_id IS NOT NULL \ + AND t.checked = false \ + AND t.due_at IS NOT NULL \ + AND t.due_at < $1 \ + AND t.due_at > $1 - interval '7 days' \ + ON CONFLICT (user_id, dedupe_key) DO NOTHING", + ) + .bind(now) + .bind(&date) + .execute(pool) + .await? + .rows_affected(); + + // Retention is `PgNotificationStore::prune`'s rule, not a second copy of + // it — the store already implements "read rows die after 90 days, all + // rows after 180" for the inbox-mutation endpoints, and duplicating + // that DELETE here would leave two places to keep in sync. + // `prune` returns the store's own error type; unwrap to the `sqlx::Error` + // this function's signature carries — `NotificationStoreError` only + // ever wraps one. + let pruned = match PgNotificationStore::new(pool.clone()).prune(now).await { + Ok(n) => n, + Err(NotificationStoreError::Sqlx(e)) => return Err(e), + }; + + if emitted > 0 { + metrics::counter!("knot_notifications_emitted_total", "kind" => "task_due") + .increment(emitted); + } + Ok(SweepOutcome { + due_emitted: emitted, + pruned, + }) +} + +/// Spawn the 15-minute loop. One per process; every replica runs its own. +pub fn spawn(pool: PgPool) -> JoinHandle<()> { + tokio::spawn(async move { + let mut ticker = tokio::time::interval(INTERVAL); + // The first tick fires immediately; skip it so a rolling restart + // doesn't have every pod sweep at once on boot. + ticker.tick().await; + loop { + ticker.tick().await; + match run_once(&pool, Utc::now()).await { + Ok(out) => tracing::debug!( + due_emitted = out.due_emitted, + pruned = out.pruned, + "notification sweep" + ), + Err(e) => tracing::warn!(error=?e, "notification sweep failed"), + } + } + }) +} diff --git a/crates/knot-server/src/reindex.rs b/crates/knot-server/src/reindex.rs index d9e584c..5727cc3 100644 --- a/crates/knot-server/src/reindex.rs +++ b/crates/knot-server/src/reindex.rs @@ -52,7 +52,14 @@ pub fn spawn(state: AppState, mut rx: mpsc::Receiver) { } let to_flush = std::mem::take(&mut pending); for doc_id in to_flush { - if let Err(e) = refresh_markdown_and_index(&state, doc_id).await { + // The dirty-notification channel carries only a + // doc-id (see the module docs above), so there is + // no editor identity to forward here. That means a + // `task_assigned` notification triggered purely by + // this worker's periodic flush has no actor — it + // is never suppressed as a "self-assignment" and + // never attributed to anyone in particular. + if let Err(e) = refresh_markdown_and_index(&state, doc_id, None).await { tracing::warn!(error=?e, %doc_id, "reindex worker: refresh failed"); } } diff --git a/crates/knot-server/src/routes/api/comments.rs b/crates/knot-server/src/routes/api/comments.rs index e66bea3..22086b1 100644 --- a/crates/knot-server/src/routes/api/comments.rs +++ b/crates/knot-server/src/routes/api/comments.rs @@ -59,11 +59,19 @@ struct CreateThreadBody { position_y_end: Option, #[serde(default)] anchor_text: Option, + /// User ids the client's mention picker resolved. Preferred over the + /// display-name regex, which cannot match a name containing a space. + #[serde(default)] + mentions: Vec, } #[derive(Deserialize)] struct CreateReplyBody { body: String, + /// User ids the client's mention picker resolved. Preferred over the + /// display-name regex, which cannot match a name containing a space. + #[serde(default)] + mentions: Vec, } #[derive(Deserialize)] @@ -138,60 +146,129 @@ fn extract_mentions(body: &str) -> Vec { .collect() } -/// Fire-and-forget mention notification via Postgres LISTEN/NOTIFY channel -/// `comment_mentions`. Payload: JSON `{type, doc_id, comment_id, user_ids}`. -async fn broadcast_mentions(state: &AppState, doc_id: Uuid, comment_id: Uuid, body: &str) { - let handles = extract_mentions(body); - if handles.is_empty() { - return; - } - let Some(workspaces) = state.workspaces.clone() else { - return; - }; - let Some(ctx) = state.pool.as_ref() else { +/// Distinguishes a brand-new comment (thread open or reply) from an edit of +/// an existing one, for `emit_comment_notifications`. An edit is not a +/// reply — nobody said anything new to the thread — so it must not fan out +/// `reply` rows the way a new comment does; it can still add `mention` +/// rows, since the edited body may name someone for the first time. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum CommentWrite { + Created, + Edited, +} + +/// Write inbox rows for a comment write: `mention` for everyone named in the +/// body, plus (for a newly created comment only — see [`CommentWrite`]) +/// `reply` for the thread's other participants. A user who is both gets the +/// mention only. +/// +/// Runs after the comment has committed, so a crash in between loses the +/// notification. That is the same at-most-once behaviour the previous +/// `pg_notify` had, and a trait object cannot join the caller's transaction. +#[allow(clippy::too_many_arguments)] // cohesive set of comment-write context +async fn emit_comment_notifications( + state: &AppState, + doc_id: Uuid, + thread_id: Uuid, + comment_id: Uuid, + author_id: Uuid, + body: &str, + write: CommentWrite, + explicit: &[Uuid], +) { + let (Some(notifications), Some(docs), Some(workspaces), Some(comments)) = ( + state.notifications.clone(), + state.docs.clone(), + state.workspaces.clone(), + state.comments.clone(), + ) else { return; }; - // We need the workspace_id. Fetch from doc state via the docs store. - // Actually we need workspace_id for list_members; look it up via the doc. - let Some(docs) = state.docs.clone() else { + let Ok(Some(doc)) = docs.get(doc_id).await else { return; }; - let ws_id = match docs.get(doc_id).await { - Ok(Some(d)) => d.workspace_id, - Ok(None) => return, - Err(_) => return, - }; - let members = match workspaces.list_members(ws_id).await { + + // Explicit ids from the picker UNION the regex's display-name matches + // — not either/or. Closing the picker (e.g. typing a trailing space) + // and then hand-typing a second `@name` used to be silently dropped + // whenever `explicit` was non-empty, because the regex fallback only + // ran when it was empty. Either way, membership decides — an id or a + // handle for a non-member is dropped rather than trusted. + let members = match workspaces.list_members(doc.workspace_id).await { Ok(m) => m, Err(_) => return, }; - let user_ids: Vec = members - .into_iter() - .filter(|m| handles.contains(&m.display_name.to_lowercase())) + let handles = extract_mentions(body); + let mentioned: Vec = members + .iter() + .filter(|m| { + explicit.contains(&m.user_id) || handles.contains(&m.display_name.to_lowercase()) + }) .map(|m| m.user_id) .collect(); - if user_ids.is_empty() { + + let excerpt: String = body.chars().take(140).collect(); + let base_data = serde_json::json!({ + "excerpt": excerpt, + "doc_title": doc.title, + "thread_id": thread_id.to_string(), + }); + + let mut batch: Vec = mentioned + .iter() + .map(|&uid| knot_storage::NewNotification { + workspace_id: doc.workspace_id, + user_id: uid, + actor_id: Some(author_id), + kind: knot_storage::NotificationKind::Mention, + doc_id: Some(doc_id), + target_kind: "comment".into(), + target_id: comment_id.to_string(), + dedupe_key: format!("mention:{comment_id}"), + data: base_data.clone(), + }) + .collect(); + + // Thread participants, minus the author and minus anyone already + // receiving a mention for this comment. Only for a newly created + // comment: editing an existing one is not a reply, and re-running this + // fan-out on every edit would falsely tell participants someone just + // replied. + if write == CommentWrite::Created + && let Ok(thread) = comments.list(doc_id, true).await + { + let mut seen: Vec = mentioned.clone(); + seen.push(author_id); + for c in thread.into_iter().filter(|c| c.thread_id == thread_id) { + if seen.contains(&c.author_id) { + continue; + } + seen.push(c.author_id); + batch.push(knot_storage::NewNotification { + workspace_id: doc.workspace_id, + user_id: c.author_id, + actor_id: Some(author_id), + kind: knot_storage::NotificationKind::Reply, + doc_id: Some(doc_id), + target_kind: "comment".into(), + target_id: comment_id.to_string(), + dedupe_key: format!("reply:{comment_id}"), + data: base_data.clone(), + }); + } + } + + if batch.is_empty() { return; } - let payload = serde_json::json!({ - "type": "mention", - "doc_id": doc_id, - "comment_id": comment_id, - "user_ids": user_ids, - }); - let payload_str = payload.to_string(); - // Fire and forget — don't fail the request on notify errors. - let pool = ctx.clone(); - tokio::spawn(async move { - let _ = sqlx::query("SELECT pg_notify('comment_mentions', $1)") - .bind(&payload_str) - .execute(&pool) - .await; - }); + if let Err(e) = notifications.emit_many(&batch).await { + tracing::warn!(error=?e, %comment_id, "emit comment notifications"); + } } /// Fire-and-forget: tell any active room for `doc_id` that its comments changed, -/// so connected clients refetch. Mirrors `broadcast_mentions`' pool access. +/// so connected clients refetch. Reads `state.pool` directly, same as the +/// notification path reads its individual stores. fn notify_comment_change(state: &AppState, doc_id: Uuid) { let Some(pool) = state.pool.as_ref().cloned() else { return; @@ -271,6 +348,7 @@ async fn create_thread( let Some(comments) = state.comments.clone() else { return internal(); }; + let explicit = body_req.mentions.clone(); match comments .create_thread( doc_id, @@ -284,9 +362,20 @@ async fn create_thread( { Ok(c) => { let comment_id = c.id; + let c_thread_id = c.thread_id; let body_text = c.body.clone(); let response = (StatusCode::CREATED, Json(c)).into_response(); - broadcast_mentions(&state, doc_id, comment_id, &body_text).await; + emit_comment_notifications( + &state, + doc_id, + c_thread_id, + comment_id, + ctx.user_id, + &body_text, + CommentWrite::Created, + &explicit, + ) + .await; notify_comment_change(&state, doc_id); response } @@ -330,15 +419,27 @@ async fn create_reply( let Some(comments) = state.comments.clone() else { return internal(); }; + let explicit = body_req.mentions.clone(); match comments .create_reply(doc_id, thread_id, ctx.user_id, &body_req.body) .await { Ok(c) => { let comment_id = c.id; + let c_thread_id = c.thread_id; let body_text = c.body.clone(); let response = (StatusCode::CREATED, Json(c)).into_response(); - broadcast_mentions(&state, doc_id, comment_id, &body_text).await; + emit_comment_notifications( + &state, + doc_id, + c_thread_id, + comment_id, + ctx.user_id, + &body_text, + CommentWrite::Created, + &explicit, + ) + .await; notify_comment_change(&state, doc_id); response } @@ -604,9 +705,20 @@ async fn edit_comment( Ok(c) => { let comment_id_val = c.id; let doc_id_val = c.doc_id; + let thread_id_val = c.thread_id; let body_text = c.body.clone(); let response = Json(c).into_response(); - broadcast_mentions(&state, doc_id_val, comment_id_val, &body_text).await; + emit_comment_notifications( + &state, + doc_id_val, + thread_id_val, + comment_id_val, + ctx.user_id, + &body_text, + CommentWrite::Edited, + &[], + ) + .await; notify_comment_change(&state, doc_id); response } diff --git a/crates/knot-server/src/routes/api/export_import.rs b/crates/knot-server/src/routes/api/export_import.rs index faf9257..944c0f9 100644 --- a/crates/knot-server/src/routes/api/export_import.rs +++ b/crates/knot-server/src/routes/api/export_import.rs @@ -686,7 +686,9 @@ async fn import( if matches!(rx.await, Ok(Ok(_))) { // Best-effort: kick the indexer so /tasks reflects the imported // tree without waiting for someone to hit each markdown export. - let _ = super::markdown::refresh_markdown_and_index(&state, new_doc_id).await; + let _ = + super::markdown::refresh_markdown_and_index(&state, new_doc_id, Some(ctx.user_id)) + .await; } } diff --git a/crates/knot-server/src/routes/api/grants.rs b/crates/knot-server/src/routes/api/grants.rs index 8289525..8121229 100644 --- a/crates/knot-server/src/routes/api/grants.rs +++ b/crates/knot-server/src/routes/api/grants.rs @@ -117,7 +117,14 @@ pub(super) async fn put_inline( ) .await { - Ok(()) => StatusCode::NO_CONTENT.into_response(), + Ok(()) => { + if let Some(rest) = principal.strip_prefix("user:") + && let Ok(grantee) = Uuid::parse_str(rest) + { + emit_doc_shared(&state, doc_id, grantee, ctx.user_id).await; + } + StatusCode::NO_CONTENT.into_response() + } Err(e) => { tracing::error!(error=?e, "grants put"); internal() @@ -174,3 +181,29 @@ async fn read_json(req: Request) -> Result Response { json_err(StatusCode::INTERNAL_SERVER_ERROR, "internal", "") } + +/// Tell the grantee a document was shared with them. Best-effort: a failure +/// here must not fail the grant that already committed. +async fn emit_doc_shared(state: &AppState, doc_id: Uuid, grantee: Uuid, actor: Uuid) { + let (Some(notifications), Some(docs)) = (state.notifications.clone(), state.docs.clone()) + else { + return; + }; + let Ok(Some(doc)) = docs.get(doc_id).await else { + return; + }; + let n = knot_storage::NewNotification { + workspace_id: doc.workspace_id, + user_id: grantee, + actor_id: Some(actor), + kind: knot_storage::NotificationKind::DocShared, + doc_id: Some(doc_id), + target_kind: "document".into(), + target_id: doc_id.to_string(), + dedupe_key: format!("share:{doc_id}:{grantee}"), + data: serde_json::json!({ "doc_title": doc.title }), + }; + if let Err(e) = notifications.emit(&n).await { + tracing::warn!(error=?e, %doc_id, "emit doc_shared"); + } +} diff --git a/crates/knot-server/src/routes/api/markdown.rs b/crates/knot-server/src/routes/api/markdown.rs index 8fde96d..474b88d 100644 --- a/crates/knot-server/src/routes/api/markdown.rs +++ b/crates/knot-server/src/routes/api/markdown.rs @@ -59,6 +59,11 @@ pub enum RefreshError { /// reflected on `/tasks` (markdown export, full-doc import via /// ApplyUpdate/ReplaceWithMarkdown, individual task patch). /// +/// `actor_id` is whoever's edit triggered this refresh, forwarded to the +/// task indexer so a fresh `task_assigned` notification can name an actor +/// and self-assignment can be suppressed. Pass `None` when the caller has +/// no edit to attribute (e.g. a plain export) or genuinely doesn't know. +/// /// Best-effort: cache-put + indexer failures are logged but never /// propagated. The Result reports only the steps before the cache write /// (state export + markdown serialise), because failures there mean @@ -66,21 +71,23 @@ pub enum RefreshError { pub async fn refresh_markdown_and_index( state: &AppState, doc_id: Uuid, + actor_id: Option, ) -> Result { - refresh_markdown_inner(state, doc_id, true).await + refresh_markdown_inner(state, doc_id, true, actor_id).await } /// Export the doc to markdown WITHOUT re-running the task indexer. /// Used by the from-template flow so cloning a template doesn't /// trigger a write to the template's own task rows. pub async fn export_markdown_only(state: &AppState, doc_id: Uuid) -> Result { - refresh_markdown_inner(state, doc_id, false).await + refresh_markdown_inner(state, doc_id, false, None).await } async fn refresh_markdown_inner( state: &AppState, doc_id: Uuid, reindex_tasks: bool, + actor_id: Option, ) -> Result { let rooms = state.rooms_v2.clone().ok_or(RefreshError::NoRooms)?; let room = rooms @@ -130,7 +137,7 @@ async fn refresh_markdown_inner( match docs.get(doc_id).await { Ok(Some(doc)) => { if let Err(e) = tasks - .upsert_for_doc(doc.workspace_id, doc_id, &inputs) + .upsert_for_doc(doc.workspace_id, doc_id, &inputs, actor_id) .await { tracing::warn!(error=?e, "task reindex failed"); @@ -153,7 +160,9 @@ pub(super) async fn export_inline( if req.extensions().get::().is_none() { return json_err(StatusCode::FORBIDDEN, "acl.no_grant", ""); } - let text = match refresh_markdown_and_index(&state, doc_id).await { + // A plain export doesn't edit the doc, so there's no actor to + // attribute a `task_assigned` notification to. + let text = match refresh_markdown_and_index(&state, doc_id, None).await { Ok(t) => t, Err(e) => { tracing::error!(error=?e, %doc_id, "md export refresh"); @@ -276,7 +285,7 @@ pub(super) async fn import_inline( match applied { Ok(_seq) => { - let _ = refresh_markdown_and_index(&state, doc_id).await; + let _ = refresh_markdown_and_index(&state, doc_id, Some(ctx.user_id)).await; StatusCode::NO_CONTENT.into_response() } Err(e) => { diff --git a/crates/knot-server/src/routes/api/mod.rs b/crates/knot-server/src/routes/api/mod.rs index f47f1dd..3280d4c 100644 --- a/crates/knot-server/src/routes/api/mod.rs +++ b/crates/knot-server/src/routes/api/mod.rs @@ -13,6 +13,7 @@ pub mod export_import; pub mod grants; pub mod history; pub mod markdown; +pub mod notifications; pub mod search; pub mod shares; pub mod tasks; @@ -27,6 +28,7 @@ pub fn router(state: AppState) -> Router { .merge(shares::router()) .merge(boards::router()) .merge(tasks::router()) + .merge(notifications::router()) .merge(export_import::router()) .layer(middleware::from_fn(csrf_mw)) .layer(middleware::from_fn(require_session_mw)) diff --git a/crates/knot-server/src/routes/api/notifications.rs b/crates/knot-server/src/routes/api/notifications.rs new file mode 100644 index 0000000..c569348 --- /dev/null +++ b/crates/knot-server/src/routes/api/notifications.rs @@ -0,0 +1,246 @@ +//! The per-user notification inbox. +//! +//! GET /api/notifications?filter=unread|all&limit=&cursor= → { items, next_cursor } +//! GET /api/notifications/unread_count → { count, capped } +//! POST /api/notifications/read { ids: [] } | { all: true } → 204 +//! +//! Rows are addressed to exactly one recipient, so ownership needs no join. +//! Document *access* is a separate question: a notification can outlive the +//! grant that made it visible, so rows carrying a `doc_id` are re-checked +//! against `effective_role` and filtered, never 403'd. + +use axum::{ + Json, Router, + body::Body, + extract::{Query, Request, State}, + http::StatusCode, + response::{IntoResponse, Response}, + routing::{get, post}, +}; +use serde::{Deserialize, Serialize}; + +use crate::AppState; +use crate::auth::AuthContext; +use crate::http_error::json_err; + +const DEFAULT_LIMIT: i64 = 50; +const MAX_LIMIT: i64 = 100; +const UNREAD_CAP: i64 = 100; + +pub fn router() -> Router { + Router::new() + .route("/api/notifications", get(list)) + .route("/api/notifications/unread_count", get(unread_count)) + .route("/api/notifications/read", post(mark_read)) +} + +#[derive(Deserialize)] +struct ListQuery { + #[serde(default)] + filter: Option, + #[serde(default)] + limit: Option, + #[serde(default)] + cursor: Option, +} + +#[derive(Serialize)] +struct NotificationRow { + id: i64, + kind: String, + doc_id: Option, + doc_title: Option, + target_kind: String, + target_id: String, + actor_display_name: Option, + data: serde_json::Value, + created_at: String, + read: bool, +} + +#[derive(Serialize)] +struct ListResponse { + items: Vec, + next_cursor: Option, +} + +#[derive(Serialize)] +struct CountResponse { + count: i64, + capped: bool, +} + +#[derive(Deserialize)] +struct ReadBody { + #[serde(default)] + ids: Vec, + #[serde(default)] + all: bool, +} + +fn internal() -> Response { + json_err(StatusCode::INTERNAL_SERVER_ERROR, "internal", "") +} + +async fn list(State(state): State, Query(q): Query, req: Request) -> Response { + let Some(ctx) = req.extensions().get::().cloned() else { + return json_err(StatusCode::UNAUTHORIZED, "auth.session_required", ""); + }; + let (Some(notifications), Some(acl)) = (state.notifications.clone(), state.acl.clone()) else { + return internal(); + }; + let unread_only = q.filter.as_deref() == Some("unread"); + let limit = q.limit.unwrap_or(DEFAULT_LIMIT).clamp(1, MAX_LIMIT); + + let rows = match notifications + .list(ctx.user_id, unread_only, limit, q.cursor) + .await + { + Ok(r) => r, + Err(e) => { + tracing::error!(error=?e, "notifications list"); + return internal(); + } + }; + let next_cursor = if rows.len() as i64 == limit { + rows.last().map(|r| r.id) + } else { + None + }; + + // Filter, don't fail: access can be revoked after the row is written. + // Rows are independent (each keyed by its own doc_id), so the checks run + // concurrently rather than one `.await` per row in sequence — a + // cache-cold page of up to `MAX_LIMIT` rows would otherwise serialize + // that many round trips through `acl::resolve` behind one response. + let checks = futures::future::join_all(rows.iter().map(|r| { + let acl = &acl; + async move { + match r.doc_id { + Some(doc_id) => Some( + acl.effective_role(ctx.workspace_id, doc_id, ctx.user_id) + .await, + ), + None => None, + } + } + })) + .await; + + let mut items = Vec::with_capacity(rows.len()); + for (r, check) in rows.into_iter().zip(checks) { + match check { + None => {} // no doc_id: nothing to re-check, keep. + Some(Ok(Some(_))) => {} // access confirmed, keep. + Some(Ok(None)) => continue, // access revoked: drop the row silently. + Some(Err(e)) => { + // An ACL lookup failure is a database problem, not a stale + // grant — fail the whole request rather than silently + // dropping the row, which would look like data loss to the + // user. + tracing::error!(error=?e, "notifications acl check"); + return internal(); + } + } + items.push(NotificationRow { + id: r.id, + kind: r.kind, + doc_id: r.doc_id.map(|d| d.to_string()), + doc_title: r.doc_title, + target_kind: r.target_kind, + target_id: r.target_id, + actor_display_name: r.actor_display_name, + data: r.data, + created_at: r.created_at.to_rfc3339(), + read: r.read_at.is_some(), + }); + } + + Json(ListResponse { items, next_cursor }).into_response() +} + +// Unlike `list`, this does not re-check `effective_role` per row, so the +// count can in principle include rows for documents the caller can no +// longer read — the badge could read higher than what `list` returns. +// That's unreachable today: v0.1 is single-workspace-per-deployment, and +// workspace membership alone grants a role on every doc in it, so a +// request that reaches this handler can never hit the `effective_role -> +// None` branch. It becomes reachable once a deployment can hold multiple +// workspaces; the fix then is a `workspace_members` join inside the +// existing `LIMIT` subquery in the store, not per-row ACL calls here, +// which would reintroduce the sequential-scan risk the cap exists to +// avoid. +async fn unread_count(State(state): State, req: Request) -> Response { + let Some(ctx) = req.extensions().get::().cloned() else { + return json_err(StatusCode::UNAUTHORIZED, "auth.session_required", ""); + }; + let Some(notifications) = state.notifications.clone() else { + return internal(); + }; + match notifications.unread_count(ctx.user_id, UNREAD_CAP).await { + Ok(n) => Json(CountResponse { + count: n, + capped: is_capped(n, UNREAD_CAP), + }) + .into_response(), + Err(e) => { + tracing::error!(error=?e, "notifications unread_count"); + internal() + } + } +} + +/// Whether the badge should show the "+" suffix: `n` hit the `LIMIT` the +/// store's `unread_count` query imposes, so the true count might be +/// higher. Pulled out of the handler so the `true` branch has direct unit +/// coverage — exercising it through the HTTP stack would mean actually +/// creating `UNREAD_CAP` (100) rows for a user, which no existing test does. +fn is_capped(n: i64, cap: i64) -> bool { + n >= cap +} + +async fn mark_read(State(state): State, req: Request) -> Response { + let Some(ctx) = req.extensions().get::().cloned() else { + return json_err(StatusCode::UNAUTHORIZED, "auth.session_required", ""); + }; + let Some(notifications) = state.notifications.clone() else { + return internal(); + }; + let bytes = match axum::body::to_bytes(req.into_body(), 64 * 1024).await { + Ok(b) => b, + Err(_) => return json_err(StatusCode::PAYLOAD_TOO_LARGE, "bad_request", ""), + }; + let body: ReadBody = match serde_json::from_slice(&bytes) { + Ok(b) => b, + Err(_) => return json_err(StatusCode::BAD_REQUEST, "bad_request", ""), + }; + + let res = if body.all { + notifications.mark_all_read(ctx.user_id).await + } else { + notifications.mark_read(ctx.user_id, &body.ids).await + }; + match res { + Ok(_) => StatusCode::NO_CONTENT.into_response(), + Err(e) => { + tracing::error!(error=?e, "notifications mark_read"); + internal() + } + } +} + +#[cfg(test)] +mod tests { + use super::is_capped; + + #[test] + fn not_capped_below_the_limit() { + assert!(!is_capped(4, 5)); + } + + #[test] + fn capped_at_and_above_the_limit() { + assert!(is_capped(5, 5), "count == cap must already read as capped"); + assert!(is_capped(6, 5)); + } +} diff --git a/crates/knot-server/tests/notifications_integration.rs b/crates/knot-server/tests/notifications_integration.rs new file mode 100644 index 0000000..40eedb5 --- /dev/null +++ b/crates/knot-server/tests/notifications_integration.rs @@ -0,0 +1,807 @@ +//! Integration: comment writes produce inbox rows for the right people. + +use std::sync::Arc; + +use axum::body::Body; +use axum::http::{Request, StatusCode}; +use http_body_util::BodyExt; +use knot_auth::{Hasher, Throttle}; +use knot_server::{AppState, router_with_state}; +use knot_storage::WorkspaceRole; +use tower::ServiceExt; +use uuid::Uuid; + +/// Seed: workspace + alice (owner) + bob (editor) + a doc owned by alice. +/// Returns (state, ws_id, doc_id, alice_id, bob_id). +async fn seeded() -> (AppState, Uuid, Uuid, Uuid, Uuid) { + let pool = knot_test_support::fresh_db().await.pool; + let mut s = AppState::with_pool(pool.clone()); + s.hasher = Arc::new(Hasher::fast_for_tests()); + s.throttle = Arc::new(Throttle::new()); + s.session_key = b"test-key-32-bytes-aaaaaaaaaaaaaa".to_vec(); + + let hash = s.hasher.hash("hunter22").unwrap(); + let ws = s + .workspaces + .as_ref() + .unwrap() + .create("default", "W") + .await + .unwrap(); + let alice = s + .users + .as_ref() + .unwrap() + .create_local("alice@example.com", "Alice", &hash) + .await + .unwrap(); + let bob = s + .users + .as_ref() + .unwrap() + .create_local("bob@example.com", "Bob", &hash) + .await + .unwrap(); + s.workspaces + .as_ref() + .unwrap() + .add_member(ws.id, alice.id, WorkspaceRole::Owner) + .await + .unwrap(); + s.workspaces + .as_ref() + .unwrap() + .add_member(ws.id, bob.id, WorkspaceRole::Editor) + .await + .unwrap(); + let doc = s + .docs + .as_ref() + .unwrap() + .create(ws.id, None, "Test Doc", "m", alice.id) + .await + .unwrap(); + (s, ws.id, doc.id, alice.id, bob.id) +} + +/// Log in as `email` and return the Cookie header value to replay. +async fn login(state: &AppState, email: &str) -> String { + let app = router_with_state(state.clone()); + let res = app + .oneshot( + Request::builder() + .method("POST") + .uri("/auth/login") + .header("content-type", "application/json") + .body(Body::from( + serde_json::json!({ "email": email, "password": "hunter22" }).to_string(), + )) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!( + res.status(), + StatusCode::NO_CONTENT, + "login failed for {email}" + ); + res.headers() + .get_all("set-cookie") + .iter() + .map(|v| v.to_str().unwrap().split(';').next().unwrap().to_string()) + .collect::>() + .join("; ") +} + +fn csrf_from(cookie: &str) -> String { + cookie + .split("; ") + .find_map(|c| c.strip_prefix("csrf=")) + .unwrap_or_default() + .to_string() +} + +async fn post_json( + state: &AppState, + cookie: &str, + uri: &str, + body: serde_json::Value, +) -> (StatusCode, serde_json::Value) { + let app = router_with_state(state.clone()); + let res = app + .oneshot( + Request::builder() + .method("POST") + .uri(uri) + .header("cookie", cookie) + .header("x-csrf-token", csrf_from(cookie)) + .header("content-type", "application/json") + .body(Body::from(body.to_string())) + .unwrap(), + ) + .await + .unwrap(); + let status = res.status(); + let bytes = res.into_body().collect().await.unwrap().to_bytes(); + let json = if bytes.is_empty() { + serde_json::Value::Null + } else { + serde_json::from_slice(&bytes).unwrap_or(serde_json::Value::Null) + }; + (status, json) +} + +/// Same as `post_json` but for the `PATCH` comment-edit endpoint. +async fn patch_json( + state: &AppState, + cookie: &str, + uri: &str, + body: serde_json::Value, +) -> (StatusCode, serde_json::Value) { + let app = router_with_state(state.clone()); + let res = app + .oneshot( + Request::builder() + .method("PATCH") + .uri(uri) + .header("cookie", cookie) + .header("x-csrf-token", csrf_from(cookie)) + .header("content-type", "application/json") + .body(Body::from(body.to_string())) + .unwrap(), + ) + .await + .unwrap(); + let status = res.status(); + let bytes = res.into_body().collect().await.unwrap().to_bytes(); + let json = if bytes.is_empty() { + serde_json::Value::Null + } else { + serde_json::from_slice(&bytes).unwrap_or(serde_json::Value::Null) + }; + (status, json) +} + +#[tokio::test(flavor = "multi_thread")] +async fn mention_in_a_comment_notifies_the_mentioned_user_only() { + let (state, _ws, doc, alice, bob) = seeded().await; + let cookie = login(&state, "alice@example.com").await; + + let (status, _) = post_json( + &state, + &cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "please look @Bob" }), + ) + .await; + assert_eq!(status, StatusCode::CREATED); + + let notifications = state.notifications.as_ref().unwrap(); + let bobs = notifications.list(bob, false, 50, None).await.unwrap(); + assert_eq!(bobs.len(), 1); + assert_eq!(bobs[0].kind, "mention"); + assert_eq!(bobs[0].doc_id, Some(doc)); + + // The author gets nothing. + assert!( + notifications + .list(alice, false, 50, None) + .await + .unwrap() + .is_empty() + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn reply_notifies_thread_participants_but_not_the_replier() { + let (state, _ws, doc, alice, bob) = seeded().await; + let alice_cookie = login(&state, "alice@example.com").await; + let bob_cookie = login(&state, "bob@example.com").await; + + // Alice opens a thread with no mention. + let (_, thread) = post_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "what do we think?" }), + ) + .await; + let thread_id = thread["thread_id"].as_str().unwrap(); + + // Bob replies. + let (status, _) = post_json( + &state, + &bob_cookie, + &format!("/api/docs/{doc}/comments/{thread_id}/replies"), + serde_json::json!({ "body": "looks fine" }), + ) + .await; + assert_eq!(status, StatusCode::CREATED); + + let notifications = state.notifications.as_ref().unwrap(); + let alices = notifications.list(alice, false, 50, None).await.unwrap(); + assert_eq!(alices.len(), 1); + assert_eq!(alices[0].kind, "reply"); + assert!( + notifications + .list(bob, false, 50, None) + .await + .unwrap() + .is_empty() + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_mentioned_participant_gets_one_row_not_two() { + let (state, _ws, doc, alice, bob) = seeded().await; + let alice_cookie = login(&state, "alice@example.com").await; + let bob_cookie = login(&state, "bob@example.com").await; + + let (_, thread) = post_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "opening" }), + ) + .await; + let thread_id = thread["thread_id"].as_str().unwrap(); + + // Bob replies AND mentions Alice — she is a participant and mentioned. + post_json( + &state, + &bob_cookie, + &format!("/api/docs/{doc}/comments/{thread_id}/replies"), + serde_json::json!({ "body": "done @Alice" }), + ) + .await; + + let rows = state + .notifications + .as_ref() + .unwrap() + .list(alice, false, 50, None) + .await + .unwrap(); + assert_eq!(rows.len(), 1, "mention wins; no duplicate reply row"); + assert_eq!(rows[0].kind, "mention"); + let _ = bob; +} + +#[tokio::test(flavor = "multi_thread")] +async fn editing_a_comment_does_not_notify_thread_participants() { + let (state, _ws, doc, alice, bob) = seeded().await; + let alice_cookie = login(&state, "alice@example.com").await; + let bob_cookie = login(&state, "bob@example.com").await; + + // Alice opens a thread. + let (_, thread) = post_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "original text" }), + ) + .await; + let thread_id = thread["thread_id"].as_str().unwrap(); + let comment_id = thread["id"].as_str().unwrap().to_string(); + + // Bob replies, becoming a thread participant. This is the one + // legitimate reply notification in this test, and it goes to Alice — + // Bob himself gets nothing from his own reply. + post_json( + &state, + &bob_cookie, + &format!("/api/docs/{doc}/comments/{thread_id}/replies"), + serde_json::json!({ "body": "looks fine" }), + ) + .await; + + let notifications = state.notifications.as_ref().unwrap(); + assert_eq!( + notifications + .list(alice, false, 50, None) + .await + .unwrap() + .len(), + 1, + "Alice should have exactly the one reply notification, from Bob's reply" + ); + + // Alice edits her original comment — a typo fix, no new mention. This + // must NOT tell Bob "Alice replied": nobody replied, she edited. + let (status, _) = patch_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments/{comment_id}"), + serde_json::json!({ "body": "original text, fixed" }), + ) + .await; + assert_eq!(status, StatusCode::OK); + + let bobs = notifications.list(bob, false, 50, None).await.unwrap(); + assert!( + bobs.is_empty(), + "editing a comment must not fan out reply notifications to thread participants; got {bobs:?}" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn editing_a_comment_to_add_a_mention_notifies_exactly_once() { + let (state, _ws, doc, alice, bob) = seeded().await; + let _ = alice; + let alice_cookie = login(&state, "alice@example.com").await; + + // Alice opens a thread with no mention — solo thread, so no reply + // fan-out is in play either; this isolates the mention-on-edit path. + let (_, thread) = post_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "no mentions here" }), + ) + .await; + let comment_id = thread["id"].as_str().unwrap().to_string(); + + let notifications = state.notifications.as_ref().unwrap(); + assert!( + notifications + .list(bob, false, 50, None) + .await + .unwrap() + .is_empty() + ); + + // Edit adds a mention: the newly-mentioned Bob is notified exactly once. + let (status, _) = patch_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments/{comment_id}"), + serde_json::json!({ "body": "please look @Bob" }), + ) + .await; + assert_eq!(status, StatusCode::OK); + + let bobs = notifications.list(bob, false, 50, None).await.unwrap(); + assert_eq!(bobs.len(), 1); + assert_eq!(bobs[0].kind, "mention"); + + // Editing again while keeping the same mention must not add a second + // row for Bob — he is already mentioned on this comment. + let (status, _) = patch_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments/{comment_id}"), + serde_json::json!({ "body": "please look @Bob, thanks" }), + ) + .await; + assert_eq!(status, StatusCode::OK); + + let bobs = notifications.list(bob, false, 50, None).await.unwrap(); + assert_eq!( + bobs.len(), + 1, + "no duplicate mention row when a second edit keeps the same mention" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn granting_access_notifies_the_grantee() { + let (state, _ws, doc, _alice, bob) = seeded().await; + let cookie = login(&state, "alice@example.com").await; + + let app = router_with_state(state.clone()); + let res = app + .oneshot( + Request::builder() + .method("PUT") + .uri(format!("/api/docs/{doc}/grants/user:{bob}")) + .header("cookie", &cookie) + .header("x-csrf-token", csrf_from(&cookie)) + .header("content-type", "application/json") + .body(Body::from( + serde_json::json!({ "role": "editor", "inherit": true }).to_string(), + )) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT); + + let rows = state + .notifications + .as_ref() + .unwrap() + .list(bob, false, 50, None) + .await + .unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].kind, "doc_shared"); + assert_eq!(rows[0].doc_id, Some(doc)); +} + +#[tokio::test(flavor = "multi_thread")] +async fn explicit_mention_ids_reach_a_user_whose_name_has_a_space() { + let (state, ws, doc, _alice, _bob) = seeded().await; + let cookie = login(&state, "alice@example.com").await; + + // A member the display-name regex can never match. + let hash = state.hasher.hash("hunter22").unwrap(); + let carol = state + .users + .as_ref() + .unwrap() + .create_local("carol@example.com", "Carol Danvers", &hash) + .await + .unwrap(); + state + .workspaces + .as_ref() + .unwrap() + .add_member(ws, carol.id, WorkspaceRole::Editor) + .await + .unwrap(); + + let (status, _) = post_json( + &state, + &cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ + "body": "over to you @Carol Danvers", + "mentions": [carol.id.to_string()], + }), + ) + .await; + assert_eq!(status, StatusCode::CREATED); + + let rows = state + .notifications + .as_ref() + .unwrap() + .list(carol.id, false, 50, None) + .await + .unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].kind, "mention"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn explicit_and_typed_mentions_union_instead_of_either_or() { + // Finding 6: picking one person from the mention picker then + // hand-typing a second `@name` (e.g. after a trailing space silently + // closed the picker) used to drop the typed name entirely, because a + // non-empty `mentions` list disabled the regex fallback outright. + let (state, ws, doc, _alice, bob) = seeded().await; + let cookie = login(&state, "alice@example.com").await; + + // A member the display-name regex can never match on its own — proves + // the explicit id still works when unioned, not just when alone. + let hash = state.hasher.hash("hunter22").unwrap(); + let carol = state + .users + .as_ref() + .unwrap() + .create_local("carol@example.com", "Carol Danvers", &hash) + .await + .unwrap(); + state + .workspaces + .as_ref() + .unwrap() + .add_member(ws, carol.id, WorkspaceRole::Editor) + .await + .unwrap(); + + let (status, _) = post_json( + &state, + &cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ + "body": "over to you @Carol Danvers, and also @Bob", + "mentions": [carol.id.to_string()], + }), + ) + .await; + assert_eq!(status, StatusCode::CREATED); + + let notifications = state.notifications.as_ref().unwrap(); + assert_eq!( + notifications + .list(carol.id, false, 50, None) + .await + .unwrap() + .len(), + 1, + "the picked id must still notify" + ); + assert_eq!( + notifications + .list(bob, false, 50, None) + .await + .unwrap() + .len(), + 1, + "the hand-typed name must not be dropped just because `mentions` was non-empty" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn an_id_for_a_non_member_is_ignored() { + let (state, _ws, doc, _alice, _bob) = seeded().await; + let cookie = login(&state, "alice@example.com").await; + + // A real user who is not a member of this workspace. + let hash = state.hasher.hash("hunter22").unwrap(); + let outsider = state + .users + .as_ref() + .unwrap() + .create_local("mallory@example.com", "Mallory", &hash) + .await + .unwrap(); + + let (status, _) = post_json( + &state, + &cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "hi", "mentions": [outsider.id.to_string()] }), + ) + .await; + assert_eq!(status, StatusCode::CREATED); + + assert!( + state + .notifications + .as_ref() + .unwrap() + .list(outsider.id, false, 50, None) + .await + .unwrap() + .is_empty(), + "a non-member must not be notified" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_comment_without_the_field_still_resolves_by_display_name() { + let (state, _ws, doc, _alice, bob) = seeded().await; + let cookie = login(&state, "alice@example.com").await; + + let (status, _) = post_json( + &state, + &cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "ping @Bob" }), + ) + .await; + assert_eq!(status, StatusCode::CREATED); + assert_eq!( + state + .notifications + .as_ref() + .unwrap() + .list(bob, false, 50, None) + .await + .unwrap() + .len(), + 1 + ); +} + +async fn get_json(state: &AppState, cookie: &str, uri: &str) -> (StatusCode, serde_json::Value) { + let app = router_with_state(state.clone()); + let res = app + .oneshot( + Request::builder() + .method("GET") + .uri(uri) + .header("cookie", cookie) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + let status = res.status(); + let bytes = res.into_body().collect().await.unwrap().to_bytes(); + let json = if bytes.is_empty() { + serde_json::Value::Null + } else { + serde_json::from_slice(&bytes).unwrap_or(serde_json::Value::Null) + }; + (status, json) +} + +#[tokio::test(flavor = "multi_thread")] +async fn inbox_lists_counts_and_marks_read() { + let (state, _ws, doc, _alice, _bob) = seeded().await; + let alice_cookie = login(&state, "alice@example.com").await; + let bob_cookie = login(&state, "bob@example.com").await; + + post_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "hey @Bob" }), + ) + .await; + + let (status, count) = get_json(&state, &bob_cookie, "/api/notifications/unread_count").await; + assert_eq!(status, StatusCode::OK); + assert_eq!(count["count"], 1); + assert_eq!(count["capped"], false); + + let (status, list) = get_json(&state, &bob_cookie, "/api/notifications?filter=unread").await; + assert_eq!(status, StatusCode::OK); + let items = list["items"].as_array().unwrap(); + assert_eq!(items.len(), 1); + assert_eq!(items[0]["kind"], "mention"); + assert_eq!(items[0]["doc_title"], "Test Doc"); + assert_eq!(items[0]["actor_display_name"], "Alice"); + assert_eq!(items[0]["read"], false); + let id = items[0]["id"].as_i64().unwrap(); + + let (status, _) = post_json( + &state, + &bob_cookie, + "/api/notifications/read", + serde_json::json!({ "ids": [id] }), + ) + .await; + assert_eq!(status, StatusCode::NO_CONTENT); + + let (_, count) = get_json(&state, &bob_cookie, "/api/notifications/unread_count").await; + assert_eq!(count["count"], 0); +} + +#[tokio::test(flavor = "multi_thread")] +async fn the_inbox_is_per_user() { + let (state, _ws, doc, _alice, _bob) = seeded().await; + let alice_cookie = login(&state, "alice@example.com").await; + post_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "hey @Bob" }), + ) + .await; + + // Alice sees nothing — the row belongs to Bob. + let (_, list) = get_json(&state, &alice_cookie, "/api/notifications").await; + assert!(list["items"].as_array().unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_row_for_a_doc_the_user_cannot_read_is_filtered_out() { + let (state, _ws, doc, _alice, bob) = seeded().await; + let alice_cookie = login(&state, "alice@example.com").await; + let bob_cookie = login(&state, "bob@example.com").await; + + post_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "hey @Bob" }), + ) + .await; + assert_eq!( + get_json(&state, &bob_cookie, "/api/notifications").await.1["items"] + .as_array() + .unwrap() + .len(), + 1 + ); + + // A notification pointing at a document Bob cannot read. Workspace + // membership grants a role on every doc in that workspace + // (knot_docs::acl::resolve), so the way to be unable to read a doc is + // for it to live in another workspace — which is exactly the tenancy + // guard at acl.rs:59-62, and the same `effective_role` -> None branch a + // revoked grant produces. + let other_ws = state + .workspaces + .as_ref() + .unwrap() + .create("other", "Other") + .await + .unwrap(); + let other_doc = state + .docs + .as_ref() + .unwrap() + .create(other_ws.id, None, "Elsewhere", "m", bob) + .await + .unwrap(); + state + .notifications + .as_ref() + .unwrap() + .emit(&knot_storage::NewNotification { + workspace_id: other_ws.id, + user_id: bob, + actor_id: None, + kind: knot_storage::NotificationKind::DocShared, + doc_id: Some(other_doc.id), + target_kind: "document".into(), + target_id: other_doc.id.to_string(), + dedupe_key: format!("share:{}:{bob}", other_doc.id), + data: serde_json::json!({}), + }) + .await + .unwrap(); + + // Two rows exist for Bob; the endpoint returns only the readable one. + assert_eq!( + state + .notifications + .as_ref() + .unwrap() + .list(bob, false, 50, None) + .await + .unwrap() + .len(), + 2 + ); + let items = get_json(&state, &bob_cookie, "/api/notifications").await.1; + let items = items["items"].as_array().unwrap(); + assert_eq!(items.len(), 1, "the unreadable row is filtered, not 403'd"); + assert_eq!(items[0]["kind"], "mention"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mark_read_with_all_true_clears_the_whole_inbox() { + // `POST /api/notifications/read {"all": true}` had no coverage at any + // level. `ids` defaults to `[]` and `all` to `false`, so a body that + // fails to deserialize as expected (e.g. a wrong key name) silently + // degrades to "mark nothing" and still returns 204 — the "Mark all + // read" button would appear to work and do nothing. + let (state, _ws, doc, _alice, bob) = seeded().await; + let alice_cookie = login(&state, "alice@example.com").await; + let bob_cookie = login(&state, "bob@example.com").await; + + // Two distinct unread rows for Bob: a mention, and a direct grant. + post_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "hey @Bob" }), + ) + .await; + let app = router_with_state(state.clone()); + let res = app + .oneshot( + Request::builder() + .method("PUT") + .uri(format!("/api/docs/{doc}/grants/user:{bob}")) + .header("cookie", &alice_cookie) + .header("x-csrf-token", csrf_from(&alice_cookie)) + .header("content-type", "application/json") + .body(Body::from( + serde_json::json!({ "role": "editor", "inherit": true }).to_string(), + )) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT); + + let (_, count) = get_json(&state, &bob_cookie, "/api/notifications/unread_count").await; + assert_eq!(count["count"], 2, "sanity: two unread rows before marking"); + + let (status, _) = post_json( + &state, + &bob_cookie, + "/api/notifications/read", + serde_json::json!({ "all": true }), + ) + .await; + assert_eq!(status, StatusCode::NO_CONTENT); + + let (_, count) = get_json(&state, &bob_cookie, "/api/notifications/unread_count").await; + assert_eq!(count["count"], 0, "all: true must clear every unread row"); + + let (_, list) = get_json(&state, &bob_cookie, "/api/notifications").await; + let items = list["items"].as_array().unwrap(); + assert_eq!(items.len(), 2); + assert!( + items.iter().all(|i| i["read"] == true), + "every row must now be marked read: {items:?}" + ); +} diff --git a/crates/knot-server/tests/notifications_sweep.rs b/crates/knot-server/tests/notifications_sweep.rs new file mode 100644 index 0000000..f5964d0 --- /dev/null +++ b/crates/knot-server/tests/notifications_sweep.rs @@ -0,0 +1,308 @@ +//! The overdue-task sweep emits once per task per day and is safe to run +//! concurrently on every replica. + +use chrono::{Duration, Utc}; +use knot_server::notifications_sweep; +use knot_storage::{ + DocStore, DocTaskInput, NewNotification, NotificationKind, NotificationStore, PgDocStore, + PgNotificationStore, PgTaskStore, PgUserStore, PgWorkspaceStore, TaskStore, UserStore, + WorkspaceRole, WorkspaceStore, sort_key_between, +}; +use uuid::Uuid; + +#[tokio::test(flavor = "multi_thread")] +async fn overdue_task_emits_once_per_day_even_across_concurrent_sweeps() { + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()) + .create("default", "W") + .await + .unwrap(); + let alice = PgUserStore::new(pool.clone()) + .create_local("alice@x.test", "Alice", "$h$") + .await + .unwrap(); + let bob = PgUserStore::new(pool.clone()) + .create_local("bob@x.test", "Bob", "$h$") + .await + .unwrap(); + let ws_store = PgWorkspaceStore::new(pool.clone()); + ws_store + .add_member(ws.id, alice.id, WorkspaceRole::Owner) + .await + .unwrap(); + ws_store + .add_member(ws.id, bob.id, WorkspaceRole::Editor) + .await + .unwrap(); + let doc = PgDocStore::new(pool.clone()) + .create(ws.id, None, "Doc", &sort_key_between(None, None), alice.id) + .await + .unwrap(); + + PgTaskStore::new(pool.clone()) + .upsert_for_doc( + ws.id, + doc.id, + &[DocTaskInput { + item_index: 0, + text: "overdue thing".into(), + assignee_user_id: Some(bob.id), + checked: false, + due_at: Some(Utc::now() - Duration::days(1)), + }], + Some(alice.id), + ) + .await + .unwrap(); + + let notifications = PgNotificationStore::new(pool.clone()); + // The assignment itself notified once; count from there. + let before = notifications + .list(bob.id, false, 50, None) + .await + .unwrap() + .len(); + + let now = Utc::now(); + // Two replicas sweeping at the same moment. + let (a, b) = tokio::join!( + notifications_sweep::run_once(&pool, now), + notifications_sweep::run_once(&pool, now), + ); + let total = a.unwrap().due_emitted + b.unwrap().due_emitted; + assert_eq!(total, 1, "concurrent sweeps must produce exactly one row"); + + let rows = notifications.list(bob.id, false, 50, None).await.unwrap(); + assert_eq!(rows.len(), before + 1); + assert!(rows.iter().any(|r| r.kind == "task_due")); + + // Same day again: nothing new. + let again = notifications_sweep::run_once(&pool, now).await.unwrap(); + assert_eq!(again.due_emitted, 0); + + // Tomorrow: one more. + let tomorrow = notifications_sweep::run_once(&pool, now + Duration::days(1)) + .await + .unwrap(); + assert_eq!(tomorrow.due_emitted, 1); +} + +#[tokio::test(flavor = "multi_thread")] +async fn task_due_survives_a_reorder_without_re_notifying() { + // Finding 5: `doc_tasks.id` is ":". Keying + // `task_due` on it meant inserting an item above an already-overdue + // one shifted the id of everything below, so the dedupe index saw what + // looked like a brand-new overdue task and fired a second `task_due` + // the same day — the exact reorder hazard the design's §2 warns about + // for `task_assigned`. The content-addressed key must survive this. + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()) + .create("default", "W") + .await + .unwrap(); + let alice = PgUserStore::new(pool.clone()) + .create_local("alice@x.test", "Alice", "$h$") + .await + .unwrap(); + let bob = PgUserStore::new(pool.clone()) + .create_local("bob@x.test", "Bob", "$h$") + .await + .unwrap(); + let ws_store = PgWorkspaceStore::new(pool.clone()); + ws_store + .add_member(ws.id, alice.id, WorkspaceRole::Owner) + .await + .unwrap(); + ws_store + .add_member(ws.id, bob.id, WorkspaceRole::Editor) + .await + .unwrap(); + let doc = PgDocStore::new(pool.clone()) + .create(ws.id, None, "Doc", &sort_key_between(None, None), alice.id) + .await + .unwrap(); + + let tasks = PgTaskStore::new(pool.clone()); + let overdue = DocTaskInput { + item_index: 0, + text: "ship it".into(), + assignee_user_id: Some(bob.id), + checked: false, + due_at: Some(Utc::now() - Duration::days(1)), + }; + tasks + .upsert_for_doc( + ws.id, + doc.id, + std::slice::from_ref(&overdue), + Some(alice.id), + ) + .await + .unwrap(); + + let now = Utc::now(); + let first = notifications_sweep::run_once(&pool, now).await.unwrap(); + assert_eq!(first.due_emitted, 1, "the overdue task notifies once"); + + // Insert an unrelated item above it: "ship it" moves from doc_tasks id + // ":0" to ":1" even though nothing about the task itself — + // text, assignee, due date — changed. + let inserted_above = DocTaskInput { + item_index: 0, + text: "an unrelated new item".into(), + assignee_user_id: None, + checked: false, + due_at: None, + }; + let shifted = DocTaskInput { + item_index: 1, + ..overdue.clone() + }; + tasks + .upsert_for_doc(ws.id, doc.id, &[inserted_above, shifted], Some(alice.id)) + .await + .unwrap(); + + let second = notifications_sweep::run_once(&pool, now).await.unwrap(); + assert_eq!( + second.due_emitted, 0, + "a reorder that only changes doc_tasks.id must not look like a new overdue task" + ); + + let due_rows = PgNotificationStore::new(pool) + .list(bob.id, false, 50, None) + .await + .unwrap() + .into_iter() + .filter(|r| r.kind == "task_due") + .count(); + assert_eq!( + due_rows, 1, + "bob must have exactly one task_due row across both sweeps" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_checked_task_is_never_overdue() { + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()) + .create("default", "W") + .await + .unwrap(); + let alice = PgUserStore::new(pool.clone()) + .create_local("alice@x.test", "Alice", "$h$") + .await + .unwrap(); + let bob = PgUserStore::new(pool.clone()) + .create_local("bob@x.test", "Bob", "$h$") + .await + .unwrap(); + let ws_store = PgWorkspaceStore::new(pool.clone()); + ws_store + .add_member(ws.id, alice.id, WorkspaceRole::Owner) + .await + .unwrap(); + ws_store + .add_member(ws.id, bob.id, WorkspaceRole::Editor) + .await + .unwrap(); + let doc = PgDocStore::new(pool.clone()) + .create(ws.id, None, "Doc", &sort_key_between(None, None), alice.id) + .await + .unwrap(); + + PgTaskStore::new(pool.clone()) + .upsert_for_doc( + ws.id, + doc.id, + &[DocTaskInput { + item_index: 0, + text: "already done".into(), + assignee_user_id: Some(bob.id), + checked: true, + due_at: Some(Utc::now() - Duration::days(3)), + }], + Some(alice.id), + ) + .await + .unwrap(); + + let out = notifications_sweep::run_once(&pool, Utc::now()) + .await + .unwrap(); + assert_eq!(out.due_emitted, 0); +} + +#[tokio::test(flavor = "multi_thread")] +async fn run_once_prunes_via_the_notification_store_and_reports_the_count() { + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()) + .create("default", "W") + .await + .unwrap(); + let alice = PgUserStore::new(pool.clone()) + .create_local("alice@x.test", "Alice", "$h$") + .await + .unwrap(); + let bob = PgUserStore::new(pool.clone()) + .create_local("bob@x.test", "Bob", "$h$") + .await + .unwrap(); + let ws_store = PgWorkspaceStore::new(pool.clone()); + ws_store + .add_member(ws.id, alice.id, WorkspaceRole::Owner) + .await + .unwrap(); + ws_store + .add_member(ws.id, bob.id, WorkspaceRole::Editor) + .await + .unwrap(); + let doc = PgDocStore::new(pool.clone()) + .create(ws.id, None, "Doc", &sort_key_between(None, None), alice.id) + .await + .unwrap(); + + let notifications = PgNotificationStore::new(pool.clone()); + notifications + .emit(&NewNotification { + workspace_id: ws.id, + user_id: bob.id, + actor_id: Some(alice.id), + kind: NotificationKind::Mention, + doc_id: Some(doc.id), + target_kind: "comment".into(), + target_id: Uuid::new_v4().to_string(), + dedupe_key: format!("mention:{}", Uuid::new_v4()), + data: serde_json::json!({}), + }) + .await + .unwrap(); + let row_id = notifications.list(bob.id, false, 50, None).await.unwrap()[0].id; + + // Backdate past the 180-day everything-goes threshold. Issued as raw + // SQL against the pool directly — mirroring the `backdate` helper in + // crates/knot-storage/tests/notifications.rs — rather than + // reintroducing a test-only method on `PgNotificationStore`, which was + // deliberately removed from that production type in Task 1. + sqlx::query("UPDATE notifications SET created_at = now() - interval '200 days' WHERE id = $1") + .bind(row_id) + .execute(&pool) + .await + .unwrap(); + + let out = notifications_sweep::run_once(&pool, Utc::now()) + .await + .unwrap(); + assert_eq!( + out.pruned, 1, + "SweepOutcome.pruned must reflect the row prune deleted" + ); + assert!( + notifications + .list(bob.id, false, 50, None) + .await + .unwrap() + .is_empty(), + "the backdated row must actually be gone" + ); +} diff --git a/crates/knot-storage/Cargo.toml b/crates/knot-storage/Cargo.toml index a9fe130..b7c0b9f 100644 --- a/crates/knot-storage/Cargo.toml +++ b/crates/knot-storage/Cargo.toml @@ -8,6 +8,7 @@ publish = false [dependencies] sqlx.workspace = true serde.workspace = true +serde_json.workspace = true tokio.workspace = true tracing.workspace = true thiserror.workspace = true diff --git a/crates/knot-storage/src/lib.rs b/crates/knot-storage/src/lib.rs index 76db9fa..5e0592a 100644 --- a/crates/knot-storage/src/lib.rs +++ b/crates/knot-storage/src/lib.rs @@ -9,6 +9,7 @@ pub mod grant_store; pub mod invalidations; pub mod lexorank; pub mod markdown_cache; +pub mod notifications; pub mod pool; pub mod search; pub mod session_store; @@ -28,6 +29,10 @@ pub use lexorank::between as sort_key_between; pub use markdown_cache::{ MarkdownCacheEntry, MarkdownCacheError, MarkdownCacheStore, PgMarkdownCache, }; +pub use notifications::{ + NewNotification, Notification, NotificationKind, NotificationStore, NotificationStoreError, + PgNotificationStore, +}; pub use pool::{Pool, PoolError, begin, connect}; pub use search::{PgSearchStore, SearchHit, SearchStore, SearchStoreError}; pub use session_store::{PgSessionStore, Session, SessionStore, SessionStoreError}; @@ -35,7 +40,9 @@ pub use share_tokens::{PgShareTokenStore, ShareStoreError, ShareToken, ShareToke pub use snapshot_store::{ DocSnapshot, PgSnapshotStore, SnapshotMeta, SnapshotStore, SnapshotStoreError, }; -pub use tasks::{DocTask, DocTaskInput, PgTaskStore, TaskStore, TaskStoreError}; +pub use tasks::{ + DocTask, DocTaskInput, PgTaskStore, TaskStore, TaskStoreError, task_assigned_dedupe_key, +}; pub use updates_store::{DocUpdate, PgUpdatesStore, UpdatesStore, UpdatesStoreError}; pub use user_store::{PgUserStore, User, UserStore, UserStoreError}; pub use workspace_store::{ diff --git a/crates/knot-storage/src/notifications.rs b/crates/knot-storage/src/notifications.rs new file mode 100644 index 0000000..297620d --- /dev/null +++ b/crates/knot-storage/src/notifications.rs @@ -0,0 +1,260 @@ +//! In-app notification inbox. +//! +//! One table serves as inbox and outbox. Idempotency lives in the +//! `notifications_dedupe` unique index rather than in application logic: +//! every write is `ON CONFLICT DO NOTHING`, so concurrent emits from +//! several replicas converge on one row without coordination. +//! +//! `emailed_at` is untouched by this module. It exists so a future mailer +//! can poll `WHERE emailed_at IS NULL` without a migration. + +use async_trait::async_trait; +use chrono::{DateTime, Utc}; +use sqlx::PgPool; +use thiserror::Error; +use uuid::Uuid; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NotificationKind { + Mention, + Reply, + TaskAssigned, + TaskDue, + DocShared, +} + +impl NotificationKind { + pub fn as_str(&self) -> &'static str { + match self { + Self::Mention => "mention", + Self::Reply => "reply", + Self::TaskAssigned => "task_assigned", + Self::TaskDue => "task_due", + Self::DocShared => "doc_shared", + } + } +} + +#[derive(Debug, Clone)] +pub struct NewNotification { + pub workspace_id: Uuid, + /// Recipient. + pub user_id: Uuid, + /// Who caused it. `None` for system events (`task_due`). + pub actor_id: Option, + pub kind: NotificationKind, + pub doc_id: Option, + pub target_kind: String, + pub target_id: String, + pub dedupe_key: String, + pub data: serde_json::Value, +} + +#[derive(Debug, Clone, sqlx::FromRow)] +pub struct Notification { + pub id: i64, + pub kind: String, + pub doc_id: Option, + pub target_kind: String, + pub target_id: String, + pub data: serde_json::Value, + pub created_at: DateTime, + pub read_at: Option>, + pub actor_id: Option, + pub actor_display_name: Option, + pub doc_title: Option, +} + +#[derive(Debug, Error)] +pub enum NotificationStoreError { + #[error("sqlx: {0}")] + Sqlx(#[from] sqlx::Error), +} + +pub type Result = std::result::Result; + +/// Columns every read shares. `doc_title` is NULL for notifications with no +/// document (none today) and for archived documents, which still resolve. +const SELECT_COLS: &str = "n.id, n.kind, n.doc_id, n.target_kind, n.target_id, n.data, \ + n.created_at, n.read_at, n.actor_id, \ + u.display_name AS actor_display_name, d.title AS doc_title"; + +const FROM_JOINS: &str = "FROM notifications n \ + LEFT JOIN users u ON u.id = n.actor_id \ + LEFT JOIN documents d ON d.id = n.doc_id"; + +#[async_trait] +pub trait NotificationStore: Send + Sync + 'static { + /// Insert one notification. Returns `false` when it was deduped or + /// dropped as a self-notification — never an error in either case. + async fn emit(&self, n: &NewNotification) -> Result; + + /// Insert several. Returns how many rows were actually created. + async fn emit_many(&self, ns: &[NewNotification]) -> Result; + + /// Newest first. `cursor` is the last id of the previous page. + async fn list( + &self, + user_id: Uuid, + unread_only: bool, + limit: i64, + cursor: Option, + ) -> Result>; + + /// Unread rows, counted no further than `cap` so an ignored inbox + /// cannot turn the polled endpoint into a sequential scan. + async fn unread_count(&self, user_id: Uuid, cap: i64) -> Result; + + async fn mark_read(&self, user_id: Uuid, ids: &[i64]) -> Result; + async fn mark_all_read(&self, user_id: Uuid) -> Result; + + /// Delete read rows older than 90 days and any row older than 180. + async fn prune(&self, now: DateTime) -> Result; +} + +pub struct PgNotificationStore { + pool: PgPool, +} + +impl PgNotificationStore { + pub fn new(pool: PgPool) -> Self { + Self { pool } + } + + /// Exposed so tests can issue ad hoc SQL (backdating rows, hard-deleting + /// a document to prove the FK cascade) without this store carrying + /// test-only production methods. + pub fn pool(&self) -> &PgPool { + &self.pool + } +} + +#[async_trait] +impl NotificationStore for PgNotificationStore { + async fn emit(&self, n: &NewNotification) -> Result { + if n.actor_id == Some(n.user_id) { + return Ok(false); + } + let res = sqlx::query( + "INSERT INTO notifications \ + (workspace_id, user_id, actor_id, kind, doc_id, target_kind, target_id, dedupe_key, data) \ + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) \ + ON CONFLICT (user_id, dedupe_key) DO NOTHING", + ) + .bind(n.workspace_id) + .bind(n.user_id) + .bind(n.actor_id) + .bind(n.kind.as_str()) + .bind(n.doc_id) + .bind(&n.target_kind) + .bind(&n.target_id) + .bind(&n.dedupe_key) + .bind(&n.data) + .execute(&self.pool) + .await?; + let created = res.rows_affected() == 1; + if created { + metrics::counter!("knot_notifications_emitted_total", "kind" => n.kind.as_str()) + .increment(1); + } + Ok(created) + } + + async fn emit_many(&self, ns: &[NewNotification]) -> Result { + let mut created = 0; + for n in ns { + if self.emit(n).await? { + created += 1; + } + } + Ok(created) + } + + async fn list( + &self, + user_id: Uuid, + unread_only: bool, + limit: i64, + cursor: Option, + ) -> Result> { + let sql = format!( + "SELECT {SELECT_COLS} {FROM_JOINS} \ + WHERE n.user_id = $1 \ + AND ($2 = false OR n.read_at IS NULL) \ + AND ($3::bigint IS NULL OR n.id < $3) \ + ORDER BY n.id DESC LIMIT $4" + ); + let rows = sqlx::query_as::<_, Notification>(sqlx::AssertSqlSafe(sql)) + .bind(user_id) + .bind(unread_only) + .bind(cursor) + .bind(limit) + .fetch_all(&self.pool) + .await?; + Ok(rows) + } + + async fn unread_count(&self, user_id: Uuid, cap: i64) -> Result { + let (n,): (i64,) = sqlx::query_as( + "SELECT count(*)::bigint FROM \ + (SELECT 1 FROM notifications WHERE user_id = $1 AND read_at IS NULL LIMIT $2) t", + ) + .bind(user_id) + .bind(cap) + .fetch_one(&self.pool) + .await?; + Ok(n) + } + + async fn mark_read(&self, user_id: Uuid, ids: &[i64]) -> Result { + if ids.is_empty() { + return Ok(0); + } + let res = sqlx::query( + "UPDATE notifications SET read_at = now() \ + WHERE user_id = $1 AND id = ANY($2) AND read_at IS NULL", + ) + .bind(user_id) + .bind(ids) + .execute(&self.pool) + .await?; + Ok(res.rows_affected()) + } + + async fn mark_all_read(&self, user_id: Uuid) -> Result { + let res = sqlx::query( + "UPDATE notifications SET read_at = now() WHERE user_id = $1 AND read_at IS NULL", + ) + .bind(user_id) + .execute(&self.pool) + .await?; + Ok(res.rows_affected()) + } + + async fn prune(&self, now: DateTime) -> Result { + // `task_assigned` is excluded: unlike the other four kinds, its + // persisted row is not just an inbox entry, it is the *only* thing + // suppressing a re-notification. `PgTaskStore::upsert_for_doc` + // re-derives `task_assigned` from `doc_tasks` on every reindex and + // relies on `notifications_dedupe` to no-op when it already + // notified about an assignment. Pruning a read `task_assigned` row + // after 90 days would let the next edit of that document — however + // unrelated to this task — re-notify the assignee about an + // unchanged, months-old assignment. The other four kinds derive + // from a one-time write event and never re-emit, so ordinary + // retention is safe for them. These rows are tiny, so keeping them + // indefinitely costs nothing worth reclaiming. + let res = sqlx::query( + "DELETE FROM notifications \ + WHERE kind <> 'task_assigned' \ + AND ( \ + (read_at IS NOT NULL AND created_at < $1 - interval '90 days') \ + OR created_at < $1 - interval '180 days' \ + )", + ) + .bind(now) + .execute(&self.pool) + .await?; + Ok(res.rows_affected()) + } +} diff --git a/crates/knot-storage/src/tasks.rs b/crates/knot-storage/src/tasks.rs index daa2942..adbb1fb 100644 --- a/crates/knot-storage/src/tasks.rs +++ b/crates/knot-storage/src/tasks.rs @@ -50,11 +50,23 @@ pub trait TaskStore: Send + Sync + 'static { /// Replace the task set for `doc_id` with `items`. Rows that fell out /// of the new set are deleted. `completed_at` is preserved across /// re-indexing when the checked status doesn't change. + /// + /// `actor_id` is whoever's edit triggered the reindex, when known; it + /// becomes the actor on any `task_assigned` notification, and + /// suppresses the notification when someone assigns a task to + /// themselves. It is `None` on the live-editing path — the CRDT room + /// persists updates without an editor identity, and the reindex worker + /// that drains its dirty-doc channel receives only a doc id — so + /// self-assignment is **not** suppressed there: a task assigned to + /// yourself while co-editing still produces a `task_assigned` row with + /// a NULL actor. The guard only has an effect on paths that pass a real + /// actor, currently the markdown/workspace import handlers. async fn upsert_for_doc( &self, workspace_id: Uuid, doc_id: Uuid, items: &[DocTaskInput], + actor_id: Option, ) -> Result<()>; /// All open (uncompleted) tasks for a user across the workspace. @@ -90,6 +102,7 @@ impl TaskStore for PgTaskStore { workspace_id: Uuid, doc_id: Uuid, items: &[DocTaskInput], + actor_id: Option, ) -> Result<()> { let mut tx = crate::begin(&self.pool).await?; @@ -143,6 +156,53 @@ impl TaskStore for PgTaskStore { .bind(item.due_at) .execute(&mut *tx) .await?; + + // Notify a new assignee — but key on the task's *content*, not + // its id. Ids are ":" and every reorder + // rewrites them, so an id-keyed notification would re-fire for + // every assignee each time anyone moved a list item. + // + // `!item.checked` matters beyond "don't notify about a task + // someone already finished": it is also what keeps an existing + // deployment's first reindex after this feature ships from + // notifying every assignee about their entire historical + // backlog of *completed* work. The still-open backlog is + // handled separately, by the seed migration that pre-populates + // dedupe rows for currently-unchecked assigned tasks (see + // migrations/20260907120000_notifications_kind_check_and_task_assigned_seed.sql). + if let Some(assignee) = item.assignee_user_id + && Some(assignee) != actor_id + && !item.checked + { + let dedupe_key = task_assigned_dedupe_key(doc_id, &item.text, assignee); + let res = sqlx::query( + "INSERT INTO notifications \ + (workspace_id, user_id, actor_id, kind, doc_id, target_kind, target_id, dedupe_key, data) \ + VALUES ($1, $2, $3, 'task_assigned', $4, 'task', $5, $6, $7) \ + ON CONFLICT (user_id, dedupe_key) DO NOTHING", + ) + .bind(workspace_id) + .bind(assignee) + .bind(actor_id) + .bind(doc_id) + .bind(&id) + .bind(&dedupe_key) + .bind(serde_json::json!({ "excerpt": item.text })) + .execute(&mut *tx) + .await?; + + // This insert bypasses `NotificationStore::emit`, which is + // the only other place this counter is incremented — so + // without this, `task_assigned` never shows up in + // `knot_notifications_emitted_total` at all. Count only + // rows actually inserted, matching `emit`'s + // `rows_affected() == 1` check, so a deduped no-op doesn't + // inflate the counter. + if res.rows_affected() == 1 { + metrics::counter!("knot_notifications_emitted_total", "kind" => "task_assigned") + .increment(1); + } + } } tx.commit().await?; @@ -193,6 +253,32 @@ impl TaskStore for PgTaskStore { } } +/// First 8 bytes of a digest as lowercase hex — 16 characters, enough to +/// key a notification without carrying the whole hash in every row. +fn hex_prefix(digest: &[u8]) -> String { + digest[..8].iter().map(|b| format!("{b:02x}")).collect() +} + +/// The content-addressed `dedupe_key` for a `task_assigned` notification: +/// `task_assigned:::`. Reordering a checklist changes `doc_tasks.id` but not +/// this key; editing the text does. +/// +/// `pub` (and re-exported from the crate root) purely so +/// `crates/knot-storage/tests/notifications.rs` can assert this agrees, +/// byte for byte, with the SQL expression the seed migration +/// (`migrations/20260907120000_notifications_kind_check_and_task_assigned_seed.sql`) +/// uses to pre-populate the same keys: `encode(substring(sha256(convert_to(text, +/// 'UTF8')) from 1 for 8), 'hex')`. If the two ever disagree the seed +/// migration is silently useless — it inserts rows under keys the runtime +/// will never look up again. +pub fn task_assigned_dedupe_key(doc_id: Uuid, text: &str, assignee: Uuid) -> String { + use sha2::{Digest, Sha256}; + let digest = Sha256::digest(text.as_bytes()); + let short = hex_prefix(&digest); + format!("task_assigned:{doc_id}:{short}:{assignee}") +} + impl<'r> sqlx::FromRow<'r, sqlx::postgres::PgRow> for DocTask { fn from_row(row: &'r sqlx::postgres::PgRow) -> std::result::Result { use sqlx::Row; diff --git a/crates/knot-storage/tests/migrations_apply.rs b/crates/knot-storage/tests/migrations_apply.rs index 68676d8..b418802 100644 --- a/crates/knot-storage/tests/migrations_apply.rs +++ b/crates/knot-storage/tests/migrations_apply.rs @@ -38,6 +38,7 @@ async fn migrations_apply_cleanly() { "doc_updates", "document_grants", "documents", + "notifications", "sessions", "share_tokens", "users", diff --git a/crates/knot-storage/tests/notifications.rs b/crates/knot-storage/tests/notifications.rs new file mode 100644 index 0000000..ebec0ec --- /dev/null +++ b/crates/knot-storage/tests/notifications.rs @@ -0,0 +1,629 @@ +//! Integration tests for `PgNotificationStore`. Uses +//! `knot_test_support::fresh_db` against the dev compose Postgres. + +use knot_storage::{ + DocStore, DocTaskInput, NewNotification, NotificationKind, NotificationStore, PgDocStore, + PgNotificationStore, PgTaskStore, PgUserStore, PgWorkspaceStore, TaskStore, UserStore, + WorkspaceRole, WorkspaceStore, sort_key_between, task_assigned_dedupe_key, +}; +use uuid::Uuid; + +/// Returns (store, workspace_id, doc_id, alice_id, bob_id). +async fn setup() -> (PgNotificationStore, Uuid, Uuid, Uuid, Uuid) { + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()) + .create("default", "W") + .await + .unwrap(); + let users = PgUserStore::new(pool.clone()); + let alice = users + .create_local("alice@x.test", "Alice", "$h$") + .await + .unwrap(); + let bob = users + .create_local("bob@x.test", "Bob Smith", "$h$") + .await + .unwrap(); + let ws_store = PgWorkspaceStore::new(pool.clone()); + ws_store + .add_member(ws.id, alice.id, WorkspaceRole::Owner) + .await + .unwrap(); + ws_store + .add_member(ws.id, bob.id, WorkspaceRole::Editor) + .await + .unwrap(); + let docs = PgDocStore::new(pool.clone()); + let sk = sort_key_between(None, None); + let doc = docs + .create(ws.id, None, "Doc", &sk, alice.id) + .await + .unwrap(); + ( + PgNotificationStore::new(pool), + ws.id, + doc.id, + alice.id, + bob.id, + ) +} + +/// Backdate a row and optionally mark it read, so `prune` can be exercised +/// without waiting 90 days. Issued directly against the pool rather than +/// through a test-only method on `PgNotificationStore`, since that store is +/// production API surface. +async fn backdate(store: &PgNotificationStore, id: i64, days_old: i64, read: bool) { + sqlx::query( + "UPDATE notifications \ + SET created_at = now() - ($2 || ' days')::interval, \ + read_at = CASE WHEN $3 THEN now() - ($2 || ' days')::interval ELSE NULL END \ + WHERE id = $1", + ) + .bind(id) + .bind(days_old.to_string()) + .bind(read) + .execute(store.pool()) + .await + .unwrap(); +} + +/// Hard-delete a document to prove the FK cascade, issued directly against +/// the pool rather than through a test-only method on `PgNotificationStore`. +async fn hard_delete_doc(store: &PgNotificationStore, doc_id: Uuid) { + sqlx::query("DELETE FROM documents WHERE id = $1") + .bind(doc_id) + .execute(store.pool()) + .await + .unwrap(); +} + +fn mention_for( + ws: Uuid, + doc: Uuid, + recipient: Uuid, + actor: Uuid, + comment: Uuid, +) -> NewNotification { + NewNotification { + workspace_id: ws, + user_id: recipient, + actor_id: Some(actor), + kind: NotificationKind::Mention, + doc_id: Some(doc), + target_kind: "comment".into(), + target_id: comment.to_string(), + dedupe_key: format!("mention:{comment}"), + data: serde_json::json!({ "excerpt": "hello @Bob Smith" }), + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn emit_then_list_returns_the_row_with_actor_and_doc_title() { + let (store, ws, doc, alice, bob) = setup().await; + let comment = Uuid::new_v4(); + assert!( + store + .emit(&mention_for(ws, doc, bob, alice, comment)) + .await + .unwrap() + ); + + let rows = store.list(bob, false, 50, None).await.unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].kind, "mention"); + assert_eq!(rows[0].actor_display_name.as_deref(), Some("Alice")); + assert_eq!(rows[0].doc_title.as_deref(), Some("Doc")); + assert!(rows[0].read_at.is_none()); + + // Alice is the actor, not a recipient. + assert!(store.list(alice, false, 50, None).await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn emit_is_idempotent_on_dedupe_key() { + let (store, ws, doc, alice, bob) = setup().await; + let comment = Uuid::new_v4(); + let n = mention_for(ws, doc, bob, alice, comment); + + assert!(store.emit(&n).await.unwrap(), "first emit inserts"); + assert!(!store.emit(&n).await.unwrap(), "second emit is deduped"); + assert_eq!(store.list(bob, false, 50, None).await.unwrap().len(), 1); +} + +#[tokio::test(flavor = "multi_thread")] +async fn self_notification_is_dropped() { + let (store, ws, doc, alice, _bob) = setup().await; + let comment = Uuid::new_v4(); + // Alice mentions herself. + assert!( + !store + .emit(&mention_for(ws, doc, alice, alice, comment)) + .await + .unwrap() + ); + assert!(store.list(alice, false, 50, None).await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn unread_count_caps_and_mark_read_clears() { + let (store, ws, doc, alice, bob) = setup().await; + for _ in 0..3 { + store + .emit(&mention_for(ws, doc, bob, alice, Uuid::new_v4())) + .await + .unwrap(); + } + assert_eq!(store.unread_count(bob, 100).await.unwrap(), 3); + // The cap actually binds: with 3 unread rows, a cap below that count + // must clip the result rather than merely bounding it from above. + assert_eq!(store.unread_count(bob, 2).await.unwrap(), 2); + + let rows = store.list(bob, true, 50, None).await.unwrap(); + let first = rows[0].id; + assert_eq!(store.mark_read(bob, &[first]).await.unwrap(), 1); + assert_eq!(store.unread_count(bob, 100).await.unwrap(), 2); + + assert_eq!(store.mark_all_read(bob).await.unwrap(), 2); + assert_eq!(store.unread_count(bob, 100).await.unwrap(), 0); + // Reading does not delete. + assert_eq!(store.list(bob, false, 50, None).await.unwrap().len(), 3); + assert!(store.list(bob, true, 50, None).await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mark_read_cannot_touch_another_users_rows() { + let (store, ws, doc, alice, bob) = setup().await; + store + .emit(&mention_for(ws, doc, bob, alice, Uuid::new_v4())) + .await + .unwrap(); + let bobs = store.list(bob, true, 50, None).await.unwrap(); + + assert_eq!(store.mark_read(alice, &[bobs[0].id]).await.unwrap(), 0); + assert_eq!(store.unread_count(bob, 100).await.unwrap(), 1); +} + +#[tokio::test(flavor = "multi_thread")] +async fn list_pages_backwards_on_the_cursor() { + let (store, ws, doc, alice, bob) = setup().await; + for _ in 0..3 { + store + .emit(&mention_for(ws, doc, bob, alice, Uuid::new_v4())) + .await + .unwrap(); + } + let page1 = store.list(bob, false, 2, None).await.unwrap(); + assert_eq!(page1.len(), 2); + let page2 = store.list(bob, false, 2, Some(page1[1].id)).await.unwrap(); + assert_eq!(page2.len(), 1); + assert!(page2[0].id < page1[1].id, "ids descend across pages"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn prune_drops_read_rows_past_ninety_days_and_everything_past_one_eighty() { + let (store, ws, doc, alice, bob) = setup().await; + for _ in 0..3 { + store + .emit(&mention_for(ws, doc, bob, alice, Uuid::new_v4())) + .await + .unwrap(); + } + let rows = store.list(bob, false, 50, None).await.unwrap(); + // Row 0: read, 100 days old -> pruned. + // Row 1: unread, 100 days old -> kept. + // Row 2: unread, 200 days old -> pruned. + backdate(&store, rows[0].id, 100, true).await; + backdate(&store, rows[1].id, 100, false).await; + backdate(&store, rows[2].id, 200, false).await; + + assert_eq!(store.prune(chrono::Utc::now()).await.unwrap(), 2); + let left = store.list(bob, false, 50, None).await.unwrap(); + assert_eq!(left.len(), 1); + assert_eq!(left[0].id, rows[1].id); +} + +#[tokio::test(flavor = "multi_thread")] +async fn prune_never_touches_task_assigned_even_when_read_and_old() { + // task_assigned is the one kind whose persisted row is the *only* + // thing suppressing a re-notification: PgTaskStore::upsert_for_doc + // re-derives it from doc_tasks on every reindex and relies on the + // dedupe row already existing. If retention pruned it like every other + // kind, the next edit of a document — any edit, unrelated to the task + // — would re-notify the assignee about a months-old, unchanged + // assignment. A same-age `mention` row is the control: it must still + // go, proving this isn't just a floor that swallowed everything. + let (store, ws, doc, alice, bob) = setup().await; + store + .emit(&mention_for(ws, doc, bob, alice, Uuid::new_v4())) + .await + .unwrap(); + store + .emit(&NewNotification { + workspace_id: ws, + user_id: bob, + actor_id: None, + kind: NotificationKind::TaskAssigned, + doc_id: Some(doc), + target_kind: "task".into(), + target_id: format!("{doc}:0"), + dedupe_key: format!("task_assigned:{doc}:deadbeefcafebabe:{bob}"), + data: serde_json::json!({ "excerpt": "old task" }), + }) + .await + .unwrap(); + + let rows = store.list(bob, false, 50, None).await.unwrap(); + let task_assigned = rows.iter().find(|r| r.kind == "task_assigned").unwrap().id; + let mention = rows.iter().find(|r| r.kind == "mention").unwrap().id; + // Both read, both well past the 90-day read-row threshold. + backdate(&store, task_assigned, 200, true).await; + backdate(&store, mention, 200, true).await; + + assert_eq!( + store.prune(chrono::Utc::now()).await.unwrap(), + 1, + "only the mention row should be deleted" + ); + let left = store.list(bob, false, 50, None).await.unwrap(); + assert_eq!(left.len(), 1); + assert_eq!( + left[0].id, task_assigned, + "the read, 200-day-old task_assigned row must survive prune" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn deleting_a_doc_cascades_its_notifications() { + let (store, ws, doc, alice, bob) = setup().await; + store + .emit(&mention_for(ws, doc, bob, alice, Uuid::new_v4())) + .await + .unwrap(); + hard_delete_doc(&store, doc).await; + assert!(store.list(bob, false, 50, None).await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn assigning_a_task_notifies_once_and_survives_a_reorder() { + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()) + .create("default", "W") + .await + .unwrap(); + let users = PgUserStore::new(pool.clone()); + let alice = users + .create_local("alice@x.test", "Alice", "$h$") + .await + .unwrap(); + let bob = users + .create_local("bob@x.test", "Bob", "$h$") + .await + .unwrap(); + let carol = users + .create_local("carol@x.test", "Carol", "$h$") + .await + .unwrap(); + let ws_store = PgWorkspaceStore::new(pool.clone()); + ws_store + .add_member(ws.id, alice.id, WorkspaceRole::Owner) + .await + .unwrap(); + ws_store + .add_member(ws.id, bob.id, WorkspaceRole::Editor) + .await + .unwrap(); + ws_store + .add_member(ws.id, carol.id, WorkspaceRole::Editor) + .await + .unwrap(); + let docs = PgDocStore::new(pool.clone()); + let doc = docs + .create(ws.id, None, "Doc", &sort_key_between(None, None), alice.id) + .await + .unwrap(); + + let tasks = PgTaskStore::new(pool.clone()); + let notifications = PgNotificationStore::new(pool.clone()); + + // Different assignees per item on purpose: after the swap below, each + // task id (":") is paired with a (text, assignee) + // combination it has never held before. If both items shared one + // assignee, an id-keyed dedupe scheme would report the same + // notification counts as the content-keyed one and this test would + // pass even against that regression. + let ship = DocTaskInput { + item_index: 0, + text: "ship it".into(), + assignee_user_id: Some(bob.id), + checked: false, + due_at: None, + }; + let review = DocTaskInput { + item_index: 1, + text: "review it".into(), + assignee_user_id: Some(carol.id), + checked: false, + due_at: None, + }; + + tasks + .upsert_for_doc( + ws.id, + doc.id, + &[ship.clone(), review.clone()], + Some(alice.id), + ) + .await + .unwrap(); + assert_eq!( + notifications + .list(bob.id, false, 50, None) + .await + .unwrap() + .len(), + 1 + ); + assert_eq!( + notifications + .list(carol.id, false, 50, None) + .await + .unwrap() + .len(), + 1 + ); + + // Swap the two items: "ship it" moves to index 1, "review it" moves to + // index 0. Every task id changes, and — because the assignees differ — + // each id is now paired with an assignee it never held before either + // (id 0 was ship/bob, is now review/carol; id 1 was review/carol, is + // now ship/bob). An id-keyed dedupe would treat both as brand-new + // assignments and re-notify; content-keyed dedupe must not. + let swapped = vec![ + DocTaskInput { + item_index: 0, + ..review.clone() + }, + DocTaskInput { + item_index: 1, + ..ship.clone() + }, + ]; + tasks + .upsert_for_doc(ws.id, doc.id, &swapped, Some(alice.id)) + .await + .unwrap(); + assert_eq!( + notifications + .list(bob.id, false, 50, None) + .await + .unwrap() + .len(), + 1, + "reordering a list must not re-notify" + ); + assert_eq!( + notifications + .list(carol.id, false, 50, None) + .await + .unwrap() + .len(), + 1, + "reordering a list must not re-notify" + ); + + // Editing the text is a new task as far as the key is concerned. + // "ship it" now lives at item_index 1, still assigned to bob. + let edited = vec![DocTaskInput { + text: "ship it today".into(), + ..swapped[1].clone() + }]; + tasks + .upsert_for_doc(ws.id, doc.id, &edited, Some(alice.id)) + .await + .unwrap(); + assert_eq!( + notifications + .list(bob.id, false, 50, None) + .await + .unwrap() + .len(), + 2 + ); + assert_eq!( + notifications + .list(carol.id, false, 50, None) + .await + .unwrap() + .len(), + 1 + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn assignment_with_no_known_actor_still_notifies() { + // Live edits arrive through the reindex worker (crates/knot-server/src/ + // reindex.rs), which only ever has a doc id, never an editor identity — + // so `refresh_markdown_and_index` always calls `upsert_for_doc` with + // `actor_id: None` on that path. The self-assignment guard compares + // the assignee against `actor_id`, so with `actor_id: None` it can + // never suppress: this pins that deliberate limitation, not a bug. + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()) + .create("default", "W") + .await + .unwrap(); + let alice = PgUserStore::new(pool.clone()) + .create_local("alice@x.test", "Alice", "$h$") + .await + .unwrap(); + PgWorkspaceStore::new(pool.clone()) + .add_member(ws.id, alice.id, WorkspaceRole::Owner) + .await + .unwrap(); + let doc = PgDocStore::new(pool.clone()) + .create(ws.id, None, "Doc", &sort_key_between(None, None), alice.id) + .await + .unwrap(); + + PgTaskStore::new(pool.clone()) + .upsert_for_doc( + ws.id, + doc.id, + &[DocTaskInput { + item_index: 0, + text: "assigned to myself with no known actor".into(), + assignee_user_id: Some(alice.id), + checked: false, + due_at: None, + }], + None, + ) + .await + .unwrap(); + + assert_eq!( + PgNotificationStore::new(pool) + .list(alice.id, false, 50, None) + .await + .unwrap() + .len(), + 1, + "actor_id: None can't suppress self-assignment — that's the live-edit path" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn assigning_a_task_to_yourself_notifies_nobody() { + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()) + .create("default", "W") + .await + .unwrap(); + let alice = PgUserStore::new(pool.clone()) + .create_local("alice@x.test", "Alice", "$h$") + .await + .unwrap(); + PgWorkspaceStore::new(pool.clone()) + .add_member(ws.id, alice.id, WorkspaceRole::Owner) + .await + .unwrap(); + let doc = PgDocStore::new(pool.clone()) + .create(ws.id, None, "Doc", &sort_key_between(None, None), alice.id) + .await + .unwrap(); + + PgTaskStore::new(pool.clone()) + .upsert_for_doc( + ws.id, + doc.id, + &[DocTaskInput { + item_index: 0, + text: "mine".into(), + assignee_user_id: Some(alice.id), + checked: false, + due_at: None, + }], + Some(alice.id), + ) + .await + .unwrap(); + + assert!( + PgNotificationStore::new(pool) + .list(alice.id, false, 50, None) + .await + .unwrap() + .is_empty() + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn assigning_an_already_checked_task_notifies_nobody() { + // Finding 1a: without a `checked` guard, a checklist item that was + // completed months ago — and is only unchecked in the sense that it + // was never notified about before this feature existed — would still + // fire a `task_assigned` row the first time its document gets + // reindexed. Landing a task as checked from the start (import, or any + // path that never went through the unchecked state) must notify + // nobody, matching a task that's simply done. + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()) + .create("default", "W") + .await + .unwrap(); + let alice = PgUserStore::new(pool.clone()) + .create_local("alice@x.test", "Alice", "$h$") + .await + .unwrap(); + let bob = PgUserStore::new(pool.clone()) + .create_local("bob@x.test", "Bob", "$h$") + .await + .unwrap(); + PgWorkspaceStore::new(pool.clone()) + .add_member(ws.id, alice.id, WorkspaceRole::Owner) + .await + .unwrap(); + PgWorkspaceStore::new(pool.clone()) + .add_member(ws.id, bob.id, WorkspaceRole::Editor) + .await + .unwrap(); + let doc = PgDocStore::new(pool.clone()) + .create(ws.id, None, "Doc", &sort_key_between(None, None), alice.id) + .await + .unwrap(); + + PgTaskStore::new(pool.clone()) + .upsert_for_doc( + ws.id, + doc.id, + &[DocTaskInput { + item_index: 0, + text: "already done on arrival".into(), + assignee_user_id: Some(bob.id), + checked: true, + due_at: None, + }], + Some(alice.id), + ) + .await + .unwrap(); + + assert!( + PgNotificationStore::new(pool) + .list(bob.id, false, 50, None) + .await + .unwrap() + .is_empty(), + "a task that lands checked must not notify its assignee" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn sql_and_rust_task_assigned_dedupe_keys_agree() { + // Finding 1c's seed migration + // (migrations/20260907120000_notifications_kind_check_and_task_assigned_seed.sql) + // reproduces `task_assigned_dedupe_key` in raw SQL — `encode(substring( + // sha256(convert_to(text, 'UTF8')) from 1 for 8), 'hex')` — so it can + // pre-populate dedupe rows for tasks that already existed when this + // feature ships. If the SQL and Rust ever disagree, the seed inserts + // rows under keys the runtime will never look up again, and the whole + // point of the migration (a silent upgrade) silently fails instead. + // The non-ASCII and empty-string cases matter because sha256 operates + // on UTF-8 bytes, not chars, and an empty task text is a valid input. + let (store, _ws, doc, _alice, bob) = setup().await; + for text in ["follow up", "Jörg follow up 日本語 🎉", ""] { + let rust_key = task_assigned_dedupe_key(doc, text, bob); + let (sql_key,): (String,) = sqlx::query_as( + "SELECT 'task_assigned:' || $1::uuid || ':' || \ + encode(substring(sha256(convert_to($2, 'UTF8')) from 1 for 8), 'hex') || \ + ':' || $3::uuid", + ) + .bind(doc) + .bind(text) + .bind(bob) + .fetch_one(store.pool()) + .await + .unwrap(); + assert_eq!(rust_key, sql_key, "mismatch for text = {text:?}"); + } +} diff --git a/crates/knot-storage/tests/tasks.rs b/crates/knot-storage/tests/tasks.rs index 07aec9c..e673b09 100644 --- a/crates/knot-storage/tests/tasks.rs +++ b/crates/knot-storage/tests/tasks.rs @@ -44,7 +44,10 @@ async fn upsert_then_list_returns_rows() { due_at: None, }, ]; - store.upsert_for_doc(ws_id, doc_id, &items).await.unwrap(); + store + .upsert_for_doc(ws_id, doc_id, &items, None) + .await + .unwrap(); let in_doc = store.list_for_doc(doc_id).await.unwrap(); assert_eq!(in_doc.len(), 2); let mine = store.list_for_assignee(ws_id, user_id, true).await.unwrap(); @@ -70,7 +73,10 @@ async fn list_excludes_completed_by_default() { due_at: None, }, ]; - store.upsert_for_doc(ws_id, doc_id, &items).await.unwrap(); + store + .upsert_for_doc(ws_id, doc_id, &items, None) + .await + .unwrap(); let open_only = store .list_for_assignee(ws_id, user_id, false) .await @@ -106,7 +112,10 @@ async fn upsert_replaces_set_dropping_removed_items() { due_at: None, }, ]; - store.upsert_for_doc(ws_id, doc_id, &v1).await.unwrap(); + store + .upsert_for_doc(ws_id, doc_id, &v1, None) + .await + .unwrap(); assert_eq!(store.list_for_doc(doc_id).await.unwrap().len(), 3); // Second pass: only index 0 and 2 remain. Index 1 must be deleted. let v2 = vec![ @@ -125,7 +134,10 @@ async fn upsert_replaces_set_dropping_removed_items() { due_at: None, }, ]; - store.upsert_for_doc(ws_id, doc_id, &v2).await.unwrap(); + store + .upsert_for_doc(ws_id, doc_id, &v2, None) + .await + .unwrap(); let after = store.list_for_doc(doc_id).await.unwrap(); assert_eq!(after.len(), 2); assert!(after.iter().any(|t| t.item_index == 0)); @@ -146,9 +158,15 @@ async fn empty_upsert_clears_all_doc_tasks() { checked: false, due_at: None, }]; - store.upsert_for_doc(ws_id, doc_id, &v1).await.unwrap(); + store + .upsert_for_doc(ws_id, doc_id, &v1, None) + .await + .unwrap(); assert_eq!(store.list_for_doc(doc_id).await.unwrap().len(), 1); - store.upsert_for_doc(ws_id, doc_id, &[]).await.unwrap(); + store + .upsert_for_doc(ws_id, doc_id, &[], None) + .await + .unwrap(); assert_eq!(store.list_for_doc(doc_id).await.unwrap().len(), 0); } @@ -167,6 +185,7 @@ async fn checked_transition_stamps_completed_at_and_clears_on_uncheck() { checked: false, due_at: None, }], + None, ) .await .unwrap(); @@ -187,6 +206,7 @@ async fn checked_transition_stamps_completed_at_and_clears_on_uncheck() { checked: true, due_at: None, }], + None, ) .await .unwrap(); @@ -207,6 +227,7 @@ async fn checked_transition_stamps_completed_at_and_clears_on_uncheck() { checked: false, due_at: None, }], + None, ) .await .unwrap(); @@ -232,6 +253,7 @@ async fn unchanged_checked_preserves_completed_at_across_reindex() { checked: true, due_at: None, }], + None, ) .await .unwrap(); @@ -250,6 +272,7 @@ async fn unchanged_checked_preserves_completed_at_across_reindex() { checked: true, due_at: None, }], + None, ) .await .unwrap(); diff --git a/docs/superpowers/plans/2026-09-06-notifications-inbox.md b/docs/superpowers/plans/2026-09-06-notifications-inbox.md new file mode 100644 index 0000000..ece4bd7 --- /dev/null +++ b/docs/superpowers/plans/2026-09-06-notifications-inbox.md @@ -0,0 +1,3082 @@ +# Notifications and Inbox Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Give knot an in-app notification inbox — mentions, replies, task assignment, overdue tasks and document shares land in a per-user list with an unread badge. + +**Architecture:** A single `notifications` table acts as both inbox and outbox: every source writes a row with a `dedupe_key` protected by a unique index, which makes emits idempotent across replicas without coordination. Delivery is polling — TanStack Query refetches an unread count every 30s — so no new transport or connection lifecycle is introduced. The doc-scoped `MSG_MENTION` plumbing reserved in June is deleted rather than completed, because an inbox must reach a user who has no document open. + +**Tech Stack:** Rust (axum, sqlx 0.9, async-trait, metrics), PostgreSQL 18, React 19 + TanStack Query + Tailwind, Playwright. + +**Spec:** `docs/superpowers/specs/2026-09-06-notifications-inbox-design.md` + +## Global Constraints + +- **Dev Postgres must be running:** `make compose.up` before any `cargo` test. Tests use `knot_test_support::fresh_db()` — never testcontainers. +- **Rust gate for every task:** `cargo clippy --workspace --all-targets --all-features -- -D warnings` and `cargo fmt --all -- --check` must pass before commit. +- **Web gate for every web task:** `cd web && pnpm tsc --noEmit && pnpm lint` must pass before commit. +- **Test commands:** Rust `cargo nextest run --workspace --all-features`; web `cd web && pnpm test`; e2e `cd e2e && pnpm playwright test`. +- **Commits:** Conventional Commits. Every task ends in exactly one commit. +- **Any new table must be added** to the `expected` list in `crates/knot-storage/tests/migrations_apply.rs` (alphabetically sorted) or that test fails. +- **Notification kinds are exactly:** `mention | reply | task_assigned | task_due | doc_shared`. +- **Retention:** read rows deleted after 90 days, all rows after 180 days. +- **Unread count cap:** 100 (UI renders `99+`). +- **Poll interval:** 30_000 ms. + +--- + +### Task 1: Notifications table and store + +**Files:** + +- Create: `migrations/_notifications.sql` +- Create: `crates/knot-storage/src/notifications.rs` +- Modify: `crates/knot-storage/src/lib.rs:3-20` (module list), `:22-42` (re-exports) +- Modify: `crates/knot-storage/Cargo.toml` (add `serde_json.workspace = true`) +- Modify: `crates/knot-storage/tests/migrations_apply.rs:27-46` (expected table list) +- Modify: `crates/knot-obs/src/metrics.rs` (describe the counter) +- Test: `crates/knot-storage/tests/notifications.rs` + +**Interfaces:** + +- Consumes: `knot_storage::begin`, `knot_test_support::fresh_db`. +- Produces: + - `knot_storage::NotificationKind` — enum `{Mention, Reply, TaskAssigned, TaskDue, DocShared}`, `fn as_str(&self) -> &'static str`. + - `knot_storage::NewNotification { workspace_id: Uuid, user_id: Uuid, actor_id: Option, kind: NotificationKind, doc_id: Option, target_kind: String, target_id: String, dedupe_key: String, data: serde_json::Value }` + - `knot_storage::Notification { id: i64, kind: String, doc_id: Option, target_kind: String, target_id: String, data: serde_json::Value, created_at: DateTime, read_at: Option>, actor_id: Option, actor_display_name: Option, doc_title: Option }` + - `knot_storage::NotificationStore` trait with `emit`, `emit_many`, `list`, `unread_count`, `mark_read`, `mark_all_read`, `prune`. + - `knot_storage::PgNotificationStore::new(pool: PgPool)`. +- [ ] **Step 1: Scaffold the migration** + +```bash +make migrate.create NAME=notifications +``` + +Fill the created file with: + +```sql +CREATE TABLE notifications ( + id bigserial PRIMARY KEY, + workspace_id uuid NOT NULL REFERENCES workspaces(id) ON DELETE CASCADE, + user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE, + actor_id uuid NULL REFERENCES users(id) ON DELETE SET NULL, + kind text NOT NULL, + doc_id uuid NULL REFERENCES documents(id) ON DELETE CASCADE, + target_kind text NOT NULL, + target_id text NOT NULL, + dedupe_key text NOT NULL, + data jsonb NOT NULL DEFAULT '{}'::jsonb, + created_at timestamptz NOT NULL DEFAULT now(), + read_at timestamptz NULL, + emailed_at timestamptz NULL +); + +-- Idempotency: every emit is ON CONFLICT DO NOTHING against this index. +CREATE UNIQUE INDEX notifications_dedupe ON notifications(user_id, dedupe_key); +-- Inbox listing, newest first, keyset-paged on id. +CREATE INDEX notifications_inbox ON notifications(user_id, id DESC); +-- The polled badge query. +CREATE INDEX notifications_unread ON notifications(user_id) WHERE read_at IS NULL; +``` + +- [ ] **Step 2: Add the table to the migration characterisation test** + +In `crates/knot-storage/tests/migrations_apply.rs`, insert `"notifications",` into `expected` between `"documents",` and `"sessions",` (the list is alphabetical). + +- [ ] **Step 3: Run it to verify it fails** + +Run: `cargo nextest run -p knot-storage --test migrations_apply` +Expected: FAIL — the expected list contains `notifications` but the migration has not been applied to a fresh DB yet by this test run. (If it passes immediately, the migration is already picked up — that is fine, continue.) + +- [ ] **Step 4: Add the serde_json dependency** + +In `crates/knot-storage/Cargo.toml`, under `[dependencies]`, after `serde.workspace = true`: + +```toml +serde_json.workspace = true +``` + +- [ ] **Step 5: Write the failing store test** + +Create `crates/knot-storage/tests/notifications.rs`: + +```rust +//! Integration tests for `PgNotificationStore`. Uses +//! `knot_test_support::fresh_db` against the dev compose Postgres. + +use knot_storage::{ + NewNotification, NotificationKind, NotificationStore, PgDocStore, PgNotificationStore, + PgUserStore, PgWorkspaceStore, DocStore, UserStore, WorkspaceRole, WorkspaceStore, + sort_key_between, +}; +use uuid::Uuid; + +/// Returns (store, workspace_id, doc_id, alice_id, bob_id). +async fn setup() -> (PgNotificationStore, Uuid, Uuid, Uuid, Uuid) { + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()) + .create("default", "W") + .await + .unwrap(); + let users = PgUserStore::new(pool.clone()); + let alice = users.create_local("alice@x.test", "Alice", "$h$").await.unwrap(); + let bob = users.create_local("bob@x.test", "Bob Smith", "$h$").await.unwrap(); + let ws_store = PgWorkspaceStore::new(pool.clone()); + ws_store.add_member(ws.id, alice.id, WorkspaceRole::Owner).await.unwrap(); + ws_store.add_member(ws.id, bob.id, WorkspaceRole::Editor).await.unwrap(); + let docs = PgDocStore::new(pool.clone()); + let sk = sort_key_between(None, None); + let doc = docs.create(ws.id, None, "Doc", &sk, alice.id).await.unwrap(); + (PgNotificationStore::new(pool), ws.id, doc.id, alice.id, bob.id) +} + +fn mention_for(ws: Uuid, doc: Uuid, recipient: Uuid, actor: Uuid, comment: Uuid) -> NewNotification { + NewNotification { + workspace_id: ws, + user_id: recipient, + actor_id: Some(actor), + kind: NotificationKind::Mention, + doc_id: Some(doc), + target_kind: "comment".into(), + target_id: comment.to_string(), + dedupe_key: format!("mention:{comment}"), + data: serde_json::json!({ "excerpt": "hello @Bob Smith" }), + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn emit_then_list_returns_the_row_with_actor_and_doc_title() { + let (store, ws, doc, alice, bob) = setup().await; + let comment = Uuid::new_v4(); + assert!(store.emit(&mention_for(ws, doc, bob, alice, comment)).await.unwrap()); + + let rows = store.list(bob, false, 50, None).await.unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].kind, "mention"); + assert_eq!(rows[0].actor_display_name.as_deref(), Some("Alice")); + assert_eq!(rows[0].doc_title.as_deref(), Some("Doc")); + assert!(rows[0].read_at.is_none()); + + // Alice is the actor, not a recipient. + assert!(store.list(alice, false, 50, None).await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn emit_is_idempotent_on_dedupe_key() { + let (store, ws, doc, alice, bob) = setup().await; + let comment = Uuid::new_v4(); + let n = mention_for(ws, doc, bob, alice, comment); + + assert!(store.emit(&n).await.unwrap(), "first emit inserts"); + assert!(!store.emit(&n).await.unwrap(), "second emit is deduped"); + assert_eq!(store.list(bob, false, 50, None).await.unwrap().len(), 1); +} + +#[tokio::test(flavor = "multi_thread")] +async fn self_notification_is_dropped() { + let (store, ws, doc, alice, _bob) = setup().await; + let comment = Uuid::new_v4(); + // Alice mentions herself. + assert!(!store.emit(&mention_for(ws, doc, alice, alice, comment)).await.unwrap()); + assert!(store.list(alice, false, 50, None).await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn unread_count_caps_and_mark_read_clears() { + let (store, ws, doc, alice, bob) = setup().await; + for _ in 0..3 { + store.emit(&mention_for(ws, doc, bob, alice, Uuid::new_v4())).await.unwrap(); + } + assert_eq!(store.unread_count(bob, 100).await.unwrap(), 3); + + let rows = store.list(bob, true, 50, None).await.unwrap(); + let first = rows[0].id; + assert_eq!(store.mark_read(bob, &[first]).await.unwrap(), 1); + assert_eq!(store.unread_count(bob, 100).await.unwrap(), 2); + + assert_eq!(store.mark_all_read(bob).await.unwrap(), 2); + assert_eq!(store.unread_count(bob, 100).await.unwrap(), 0); + // Reading does not delete. + assert_eq!(store.list(bob, false, 50, None).await.unwrap().len(), 3); + assert!(store.list(bob, true, 50, None).await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mark_read_cannot_touch_another_users_rows() { + let (store, ws, doc, alice, bob) = setup().await; + store.emit(&mention_for(ws, doc, bob, alice, Uuid::new_v4())).await.unwrap(); + let bobs = store.list(bob, true, 50, None).await.unwrap(); + + assert_eq!(store.mark_read(alice, &[bobs[0].id]).await.unwrap(), 0); + assert_eq!(store.unread_count(bob, 100).await.unwrap(), 1); +} + +#[tokio::test(flavor = "multi_thread")] +async fn list_pages_backwards_on_the_cursor() { + let (store, ws, doc, alice, bob) = setup().await; + for _ in 0..3 { + store.emit(&mention_for(ws, doc, bob, alice, Uuid::new_v4())).await.unwrap(); + } + let page1 = store.list(bob, false, 2, None).await.unwrap(); + assert_eq!(page1.len(), 2); + let page2 = store.list(bob, false, 2, Some(page1[1].id)).await.unwrap(); + assert_eq!(page2.len(), 1); + assert!(page2[0].id < page1[1].id, "ids descend across pages"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn prune_drops_read_rows_past_ninety_days_and_everything_past_one_eighty() { + let (store, ws, doc, alice, bob) = setup().await; + for _ in 0..3 { + store.emit(&mention_for(ws, doc, bob, alice, Uuid::new_v4())).await.unwrap(); + } + let rows = store.list(bob, false, 50, None).await.unwrap(); + // Row 0: read, 100 days old -> pruned. + // Row 1: unread, 100 days old -> kept. + // Row 2: unread, 200 days old -> pruned. + store.set_ages_for_test(rows[0].id, 100, true).await.unwrap(); + store.set_ages_for_test(rows[1].id, 100, false).await.unwrap(); + store.set_ages_for_test(rows[2].id, 200, false).await.unwrap(); + + assert_eq!(store.prune(chrono::Utc::now()).await.unwrap(), 2); + let left = store.list(bob, false, 50, None).await.unwrap(); + assert_eq!(left.len(), 1); + assert_eq!(left[0].id, rows[1].id); +} + +#[tokio::test(flavor = "multi_thread")] +async fn deleting_a_doc_cascades_its_notifications() { + let (store, ws, doc, alice, bob) = setup().await; + store.emit(&mention_for(ws, doc, bob, alice, Uuid::new_v4())).await.unwrap(); + store.hard_delete_doc_for_test(doc).await.unwrap(); + assert!(store.list(bob, false, 50, None).await.unwrap().is_empty()); +} +``` + +- [ ] **Step 6: Run it to verify it fails** + +Run: `cargo nextest run -p knot-storage --test notifications` +Expected: FAIL to compile — `unresolved import knot_storage::NotificationStore`. + +- [ ] **Step 7: Write the store** + +Create `crates/knot-storage/src/notifications.rs`: + +```rust +//! In-app notification inbox. +//! +//! One table serves as inbox and outbox. Idempotency lives in the +//! `notifications_dedupe` unique index rather than in application logic: +//! every write is `ON CONFLICT DO NOTHING`, so concurrent emits from +//! several replicas converge on one row without coordination. +//! +//! `emailed_at` is untouched by this module. It exists so a future mailer +//! can poll `WHERE emailed_at IS NULL` without a migration. + +use async_trait::async_trait; +use chrono::{DateTime, Utc}; +use sqlx::PgPool; +use thiserror::Error; +use uuid::Uuid; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NotificationKind { + Mention, + Reply, + TaskAssigned, + TaskDue, + DocShared, +} + +impl NotificationKind { + pub fn as_str(&self) -> &'static str { + match self { + Self::Mention => "mention", + Self::Reply => "reply", + Self::TaskAssigned => "task_assigned", + Self::TaskDue => "task_due", + Self::DocShared => "doc_shared", + } + } +} + +#[derive(Debug, Clone)] +pub struct NewNotification { + pub workspace_id: Uuid, + /// Recipient. + pub user_id: Uuid, + /// Who caused it. `None` for system events (`task_due`). + pub actor_id: Option, + pub kind: NotificationKind, + pub doc_id: Option, + pub target_kind: String, + pub target_id: String, + pub dedupe_key: String, + pub data: serde_json::Value, +} + +#[derive(Debug, Clone, sqlx::FromRow)] +pub struct Notification { + pub id: i64, + pub kind: String, + pub doc_id: Option, + pub target_kind: String, + pub target_id: String, + pub data: serde_json::Value, + pub created_at: DateTime, + pub read_at: Option>, + pub actor_id: Option, + pub actor_display_name: Option, + pub doc_title: Option, +} + +#[derive(Debug, Error)] +pub enum NotificationStoreError { + #[error("sqlx: {0}")] + Sqlx(#[from] sqlx::Error), +} + +pub type Result = std::result::Result; + +/// Columns every read shares. `doc_title` is NULL for notifications with no +/// document (none today) and for archived documents, which still resolve. +const SELECT_COLS: &str = "n.id, n.kind, n.doc_id, n.target_kind, n.target_id, n.data, \ + n.created_at, n.read_at, n.actor_id, \ + u.display_name AS actor_display_name, d.title AS doc_title"; + +const FROM_JOINS: &str = "FROM notifications n \ + LEFT JOIN users u ON u.id = n.actor_id \ + LEFT JOIN documents d ON d.id = n.doc_id"; + +#[async_trait] +pub trait NotificationStore: Send + Sync + 'static { + /// Insert one notification. Returns `false` when it was deduped or + /// dropped as a self-notification — never an error in either case. + async fn emit(&self, n: &NewNotification) -> Result; + + /// Insert several. Returns how many rows were actually created. + async fn emit_many(&self, ns: &[NewNotification]) -> Result; + + /// Newest first. `cursor` is the last id of the previous page. + async fn list( + &self, + user_id: Uuid, + unread_only: bool, + limit: i64, + cursor: Option, + ) -> Result>; + + /// Unread rows, counted no further than `cap` so an ignored inbox + /// cannot turn the polled endpoint into a sequential scan. + async fn unread_count(&self, user_id: Uuid, cap: i64) -> Result; + + async fn mark_read(&self, user_id: Uuid, ids: &[i64]) -> Result; + async fn mark_all_read(&self, user_id: Uuid) -> Result; + + /// Delete read rows older than 90 days and any row older than 180. + async fn prune(&self, now: DateTime) -> Result; +} + +pub struct PgNotificationStore { + pool: PgPool, +} + +impl PgNotificationStore { + pub fn new(pool: PgPool) -> Self { + Self { pool } + } + + pub fn pool(&self) -> &PgPool { + &self.pool + } + + /// Test-only: backdate a row and optionally mark it read, so `prune` + /// can be exercised without waiting 90 days. + pub async fn set_ages_for_test(&self, id: i64, days_old: i64, read: bool) -> Result<()> { + sqlx::query( + "UPDATE notifications \ + SET created_at = now() - ($2 || ' days')::interval, \ + read_at = CASE WHEN $3 THEN now() - ($2 || ' days')::interval ELSE NULL END \ + WHERE id = $1", + ) + .bind(id) + .bind(days_old.to_string()) + .bind(read) + .execute(&self.pool) + .await?; + Ok(()) + } + + /// Test-only: hard-delete a document to prove the FK cascade. + pub async fn hard_delete_doc_for_test(&self, doc_id: Uuid) -> Result<()> { + sqlx::query("DELETE FROM documents WHERE id = $1") + .bind(doc_id) + .execute(&self.pool) + .await?; + Ok(()) + } +} + +#[async_trait] +impl NotificationStore for PgNotificationStore { + async fn emit(&self, n: &NewNotification) -> Result { + if n.actor_id == Some(n.user_id) { + return Ok(false); + } + let res = sqlx::query( + "INSERT INTO notifications \ + (workspace_id, user_id, actor_id, kind, doc_id, target_kind, target_id, dedupe_key, data) \ + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) \ + ON CONFLICT (user_id, dedupe_key) DO NOTHING", + ) + .bind(n.workspace_id) + .bind(n.user_id) + .bind(n.actor_id) + .bind(n.kind.as_str()) + .bind(n.doc_id) + .bind(&n.target_kind) + .bind(&n.target_id) + .bind(&n.dedupe_key) + .bind(&n.data) + .execute(&self.pool) + .await?; + let created = res.rows_affected() == 1; + if created { + metrics::counter!("knot_notifications_emitted_total", "kind" => n.kind.as_str()) + .increment(1); + } + Ok(created) + } + + async fn emit_many(&self, ns: &[NewNotification]) -> Result { + let mut created = 0; + for n in ns { + if self.emit(n).await? { + created += 1; + } + } + Ok(created) + } + + async fn list( + &self, + user_id: Uuid, + unread_only: bool, + limit: i64, + cursor: Option, + ) -> Result> { + let sql = format!( + "SELECT {SELECT_COLS} {FROM_JOINS} \ + WHERE n.user_id = $1 \ + AND ($2 = false OR n.read_at IS NULL) \ + AND ($3::bigint IS NULL OR n.id < $3) \ + ORDER BY n.id DESC LIMIT $4" + ); + let rows = sqlx::query_as::<_, Notification>(sqlx::AssertSqlSafe(sql)) + .bind(user_id) + .bind(unread_only) + .bind(cursor) + .bind(limit) + .fetch_all(&self.pool) + .await?; + Ok(rows) + } + + async fn unread_count(&self, user_id: Uuid, cap: i64) -> Result { + let (n,): (i64,) = sqlx::query_as( + "SELECT count(*)::bigint FROM \ + (SELECT 1 FROM notifications WHERE user_id = $1 AND read_at IS NULL LIMIT $2) t", + ) + .bind(user_id) + .bind(cap) + .fetch_one(&self.pool) + .await?; + Ok(n) + } + + async fn mark_read(&self, user_id: Uuid, ids: &[i64]) -> Result { + if ids.is_empty() { + return Ok(0); + } + let res = sqlx::query( + "UPDATE notifications SET read_at = now() \ + WHERE user_id = $1 AND id = ANY($2) AND read_at IS NULL", + ) + .bind(user_id) + .bind(ids) + .execute(&self.pool) + .await?; + Ok(res.rows_affected()) + } + + async fn mark_all_read(&self, user_id: Uuid) -> Result { + let res = sqlx::query( + "UPDATE notifications SET read_at = now() WHERE user_id = $1 AND read_at IS NULL", + ) + .bind(user_id) + .execute(&self.pool) + .await?; + Ok(res.rows_affected()) + } + + async fn prune(&self, now: DateTime) -> Result { + let res = sqlx::query( + "DELETE FROM notifications \ + WHERE (read_at IS NOT NULL AND created_at < $1 - interval '90 days') \ + OR created_at < $1 - interval '180 days'", + ) + .bind(now) + .execute(&self.pool) + .await?; + Ok(res.rows_affected()) + } +} +``` + +- [ ] **Step 8: Export from the crate root** + +In `crates/knot-storage/src/lib.rs`, add `pub mod notifications;` to the module list (alphabetically, after `pub mod markdown_cache;`) and this re-export after the `markdown_cache` one: + +```rust +pub use notifications::{ + NewNotification, Notification, NotificationKind, NotificationStore, NotificationStoreError, + PgNotificationStore, +}; +``` + +- [ ] **Step 9: Describe the metric** + +In `crates/knot-obs/src/metrics.rs`, beside the other `describe_counter!` calls: + +```rust + describe_counter!( + "knot_notifications_emitted_total", + "Notifications written to the inbox, by kind" + ); +``` + +- [ ] **Step 10: Run the tests to verify they pass** + +Run: `cargo nextest run -p knot-storage --test notifications --test migrations_apply` +Expected: PASS — 8 notification tests plus the migration test. + +- [ ] **Step 11: Lint** + +Run: `cargo clippy --workspace --all-targets --all-features -- -D warnings && cargo fmt --all -- --check` +Expected: no output, exit 0. + +- [ ] **Step 12: Commit** + +```bash +git add migrations crates/knot-storage crates/knot-obs +git commit -m "feat(storage): notifications table and store + +Idempotency lives in the notifications_dedupe unique index rather than in +application logic, so concurrent emits from several replicas converge on +one row without coordination." +``` + +--- + +### Task 2: Wire the store into AppState + +**Files:** + +- Modify: `crates/knot-server/src/lib.rs:14-16` (imports), `:42-72` (struct + `in_memory`), `:115-170` (`with_pool`) + +**Interfaces:** + +- Consumes: `knot_storage::{NotificationStore, PgNotificationStore}` from Task 1. +- Produces: `AppState.notifications: Option>`, populated by `with_pool`, `None` in `in_memory`. +- [ ] **Step 1: Add the import** + +In `crates/knot-server/src/lib.rs`, extend the existing `knot_storage::{…}` import list with `NotificationStore` and `PgNotificationStore`. + +- [ ] **Step 2: Add the field** + +In `pub struct AppState`, after `pub tasks: Option>,`: + +```rust + pub notifications: Option>, +``` + +- [ ] **Step 3: Set it in both constructors** + +In `in_memory()`, after `tasks: None,` add `notifications: None,`. + +In `with_pool()`, after the `tasks` binding: + +```rust + let notifications: Arc = + Arc::new(PgNotificationStore::new(pool.clone())); +``` + +and in the returned struct, after `tasks: Some(tasks),` add `notifications: Some(notifications),`. + +- [ ] **Step 4: Verify it compiles** + +Run: `cargo check -p knot-server --all-features` +Expected: success. Any other `AppState { … }` literal in the workspace that fails here must gain the field too — fix each before continuing. + +- [ ] **Step 5: Commit** + +```bash +git add crates/knot-server/src/lib.rs +git commit -m "feat(server): wire the notification store into AppState" +``` + +--- + +### Task 3: Mention and reply notifications on comments + +Replaces `broadcast_mentions` and the `comment_mentions` channel. Recipients still come from the display-name regex in this task; Task 5 adds explicit ids. + +**Files:** + +- Modify: `crates/knot-server/src/routes/api/comments.rs:141-188` (delete `broadcast_mentions`, add `emit_comment_notifications`), and its two call sites in `create_thread` and `create_reply` +- Test: `crates/knot-server/tests/notifications_integration.rs` (create) + +**Interfaces:** + +- Consumes: `AppState.notifications` (Task 2), `CommentStore::list`, `WorkspaceStore::list_members`, `DocStore::get`. +- Produces: `async fn emit_comment_notifications(state: &AppState, doc_id: Uuid, thread_id: Uuid, comment_id: Uuid, author_id: Uuid, body: &str)` — private to the module; called after a comment commits. +- [ ] **Step 1: Write the failing integration test** + +Create `crates/knot-server/tests/notifications_integration.rs`: + +```rust +//! Integration: comment writes produce inbox rows for the right people. + +use std::sync::Arc; + +use axum::body::Body; +use axum::http::{Request, StatusCode}; +use http_body_util::BodyExt; +use knot_auth::{Hasher, Throttle}; +use knot_server::{AppState, router_with_state}; +use knot_storage::{NotificationStore, WorkspaceRole}; +use tower::ServiceExt; +use uuid::Uuid; + +/// Seed: workspace + alice (owner) + bob (editor) + a doc owned by alice. +/// Returns (state, ws_id, doc_id, alice_id, bob_id). +async fn seeded() -> (AppState, Uuid, Uuid, Uuid, Uuid) { + let pool = knot_test_support::fresh_db().await.pool; + let mut s = AppState::with_pool(pool.clone()); + s.hasher = Arc::new(Hasher::fast_for_tests()); + s.throttle = Arc::new(Throttle::new()); + s.session_key = b"test-key-32-bytes-aaaaaaaaaaaaaa".to_vec(); + + let hash = s.hasher.hash("hunter22").unwrap(); + let ws = s.workspaces.as_ref().unwrap().create("default", "W").await.unwrap(); + let alice = s.users.as_ref().unwrap() + .create_local("alice@example.com", "Alice", &hash).await.unwrap(); + let bob = s.users.as_ref().unwrap() + .create_local("bob@example.com", "Bob", &hash).await.unwrap(); + s.workspaces.as_ref().unwrap() + .add_member(ws.id, alice.id, WorkspaceRole::Owner).await.unwrap(); + s.workspaces.as_ref().unwrap() + .add_member(ws.id, bob.id, WorkspaceRole::Editor).await.unwrap(); + let doc = s.docs.as_ref().unwrap() + .create(ws.id, None, "Test Doc", "m", alice.id).await.unwrap(); + (s, ws.id, doc.id, alice.id, bob.id) +} + +/// Log in as `email` and return the Cookie header value to replay. +async fn login(state: &AppState, email: &str) -> String { + let app = router_with_state(state.clone()); + let res = app + .oneshot( + Request::builder() + .method("POST") + .uri("/auth/login") + .header("content-type", "application/json") + .body(Body::from( + serde_json::json!({ "email": email, "password": "hunter22" }).to_string(), + )) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(res.status(), StatusCode::OK, "login failed for {email}"); + res.headers() + .get_all("set-cookie") + .iter() + .map(|v| v.to_str().unwrap().split(';').next().unwrap().to_string()) + .collect::>() + .join("; ") +} + +fn csrf_from(cookie: &str) -> String { + cookie + .split("; ") + .find_map(|c| c.strip_prefix("csrf=")) + .unwrap_or_default() + .to_string() +} + +async fn post_json( + state: &AppState, + cookie: &str, + uri: &str, + body: serde_json::Value, +) -> (StatusCode, serde_json::Value) { + let app = router_with_state(state.clone()); + let res = app + .oneshot( + Request::builder() + .method("POST") + .uri(uri) + .header("cookie", cookie) + .header("x-csrf-token", csrf_from(cookie)) + .header("content-type", "application/json") + .body(Body::from(body.to_string())) + .unwrap(), + ) + .await + .unwrap(); + let status = res.status(); + let bytes = res.into_body().collect().await.unwrap().to_bytes(); + let json = if bytes.is_empty() { + serde_json::Value::Null + } else { + serde_json::from_slice(&bytes).unwrap_or(serde_json::Value::Null) + }; + (status, json) +} + +#[tokio::test(flavor = "multi_thread")] +async fn mention_in_a_comment_notifies_the_mentioned_user_only() { + let (state, _ws, doc, alice, bob) = seeded().await; + let cookie = login(&state, "alice@example.com").await; + + let (status, _) = post_json( + &state, + &cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "please look @Bob" }), + ) + .await; + assert_eq!(status, StatusCode::CREATED); + + let notifications = state.notifications.as_ref().unwrap(); + let bobs = notifications.list(bob, false, 50, None).await.unwrap(); + assert_eq!(bobs.len(), 1); + assert_eq!(bobs[0].kind, "mention"); + assert_eq!(bobs[0].doc_id, Some(doc)); + + // The author gets nothing. + assert!(notifications.list(alice, false, 50, None).await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn reply_notifies_thread_participants_but_not_the_replier() { + let (state, _ws, doc, alice, bob) = seeded().await; + let alice_cookie = login(&state, "alice@example.com").await; + let bob_cookie = login(&state, "bob@example.com").await; + + // Alice opens a thread with no mention. + let (_, thread) = post_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "what do we think?" }), + ) + .await; + let thread_id = thread["thread_id"].as_str().unwrap(); + + // Bob replies. + let (status, _) = post_json( + &state, + &bob_cookie, + &format!("/api/docs/{doc}/comments/{thread_id}/replies"), + serde_json::json!({ "body": "looks fine" }), + ) + .await; + assert_eq!(status, StatusCode::CREATED); + + let notifications = state.notifications.as_ref().unwrap(); + let alices = notifications.list(alice, false, 50, None).await.unwrap(); + assert_eq!(alices.len(), 1); + assert_eq!(alices[0].kind, "reply"); + assert!(notifications.list(bob, false, 50, None).await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_mentioned_participant_gets_one_row_not_two() { + let (state, _ws, doc, alice, bob) = seeded().await; + let alice_cookie = login(&state, "alice@example.com").await; + let bob_cookie = login(&state, "bob@example.com").await; + + let (_, thread) = post_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "opening" }), + ) + .await; + let thread_id = thread["thread_id"].as_str().unwrap(); + + // Bob replies AND mentions Alice — she is a participant and mentioned. + post_json( + &state, + &bob_cookie, + &format!("/api/docs/{doc}/comments/{thread_id}/replies"), + serde_json::json!({ "body": "done @Alice" }), + ) + .await; + + let rows = state.notifications.as_ref().unwrap() + .list(alice, false, 50, None).await.unwrap(); + assert_eq!(rows.len(), 1, "mention wins; no duplicate reply row"); + assert_eq!(rows[0].kind, "mention"); + let _ = bob; +} +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `cargo nextest run -p knot-server --test notifications_integration` +Expected: FAIL — `assert_eq!(bobs.len(), 1)` sees 0, because nothing writes notifications yet. + +- [ ] **Step 3: Replace `broadcast_mentions`** + +In `crates/knot-server/src/routes/api/comments.rs`, delete the whole `broadcast_mentions` function (the doc comment, the body, and the `pg_notify('comment_mentions', …)` spawn) and put this in its place. Keep `MENTION_RE` and `extract_mentions` — they are the fallback path. + +```rust +/// Write inbox rows for a new comment: `mention` for everyone named in the +/// body, `reply` for the thread's other participants. A user who is both +/// gets the mention only. +/// +/// Runs after the comment has committed, so a crash in between loses the +/// notification. That is the same at-most-once behaviour the previous +/// `pg_notify` had, and a trait object cannot join the caller's transaction. +async fn emit_comment_notifications( + state: &AppState, + doc_id: Uuid, + thread_id: Uuid, + comment_id: Uuid, + author_id: Uuid, + body: &str, +) { + let (Some(notifications), Some(docs), Some(workspaces), Some(comments)) = ( + state.notifications.clone(), + state.docs.clone(), + state.workspaces.clone(), + state.comments.clone(), + ) else { + return; + }; + let Ok(Some(doc)) = docs.get(doc_id).await else { + return; + }; + + // Mentioned users: match handles against member display names. + let handles = extract_mentions(body); + let mut mentioned: Vec = Vec::new(); + if !handles.is_empty() { + let Ok(members) = workspaces.list_members(doc.workspace_id).await else { + return; + }; + mentioned = members + .into_iter() + .filter(|m| handles.contains(&m.display_name.to_lowercase())) + .map(|m| m.user_id) + .collect(); + } + + let excerpt: String = body.chars().take(140).collect(); + let base_data = serde_json::json!({ + "excerpt": excerpt, + "doc_title": doc.title, + "thread_id": thread_id.to_string(), + }); + + let mut batch: Vec = mentioned + .iter() + .map(|&uid| knot_storage::NewNotification { + workspace_id: doc.workspace_id, + user_id: uid, + actor_id: Some(author_id), + kind: knot_storage::NotificationKind::Mention, + doc_id: Some(doc_id), + target_kind: "comment".into(), + target_id: comment_id.to_string(), + dedupe_key: format!("mention:{comment_id}"), + data: base_data.clone(), + }) + .collect(); + + // Thread participants, minus the author and minus anyone already + // receiving a mention for this comment. + if let Ok(thread) = comments.list(doc_id, true).await { + let mut seen: Vec = mentioned.clone(); + seen.push(author_id); + for c in thread.into_iter().filter(|c| c.thread_id == thread_id) { + if seen.contains(&c.author_id) { + continue; + } + seen.push(c.author_id); + batch.push(knot_storage::NewNotification { + workspace_id: doc.workspace_id, + user_id: c.author_id, + actor_id: Some(author_id), + kind: knot_storage::NotificationKind::Reply, + doc_id: Some(doc_id), + target_kind: "comment".into(), + target_id: comment_id.to_string(), + dedupe_key: format!("reply:{comment_id}"), + data: base_data.clone(), + }); + } + } + + if batch.is_empty() { + return; + } + if let Err(e) = notifications.emit_many(&batch).await { + tracing::warn!(error=?e, %comment_id, "emit comment notifications"); + } +} +``` + +- [ ] **Step 4: Update the two call sites** + +In `create_thread`, replace `broadcast_mentions(&state, doc_id, comment_id, &body_text).await;` with: + +```rust + emit_comment_notifications(&state, doc_id, c_thread_id, comment_id, ctx.user_id, &body_text) + .await; +``` + +and capture the thread id alongside the comment id in the same `Ok(c)` arm: + +```rust + Ok(c) => { + let comment_id = c.id; + let c_thread_id = c.thread_id; + let body_text = c.body.clone(); +``` + +Apply the identical change in `create_reply` — find its `Ok(c)` arm and its `broadcast_mentions` call and mirror both edits. + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `cargo nextest run -p knot-server --test notifications_integration --test comments_integration` +Expected: PASS. `comments_integration` must stay green — it covers the `@mention` extraction that is still in use. + +- [ ] **Step 6: Lint** + +Run: `cargo clippy --workspace --all-targets --all-features -- -D warnings && cargo fmt --all -- --check` + +- [ ] **Step 7: Commit** + +```bash +git add crates/knot-server +git commit -m "feat(comments): write mention and reply notifications + +Replaces the pg_notify('comment_mentions') call nothing ever listened to." +``` + +--- + +### Task 4: Doc-shared notifications on grants + +**Files:** + +- Modify: `crates/knot-server/src/routes/api/grants.rs` (the `put_inline` handler's success arm, around `:108-125`) +- Test: `crates/knot-server/tests/notifications_integration.rs` (append) + +**Interfaces:** + +- Consumes: `AppState.notifications`, `DocStore::get`. +- Produces: nothing new; a `doc_shared` row keyed `share::`. +- [ ] **Step 1: Write the failing test** + +Append to `crates/knot-server/tests/notifications_integration.rs`: + +```rust +#[tokio::test(flavor = "multi_thread")] +async fn granting_access_notifies_the_grantee() { + let (state, _ws, doc, _alice, bob) = seeded().await; + let cookie = login(&state, "alice@example.com").await; + + let app = router_with_state(state.clone()); + let res = app + .oneshot( + Request::builder() + .method("PUT") + .uri(format!("/api/docs/{doc}/grants/user:{bob}")) + .header("cookie", &cookie) + .header("x-csrf-token", csrf_from(&cookie)) + .header("content-type", "application/json") + .body(Body::from( + serde_json::json!({ "role": "editor", "inherit": true }).to_string(), + )) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(res.status(), StatusCode::NO_CONTENT); + + let rows = state.notifications.as_ref().unwrap() + .list(bob, false, 50, None).await.unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].kind, "doc_shared"); + assert_eq!(rows[0].doc_id, Some(doc)); +} +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `cargo nextest run -p knot-server --test notifications_integration granting_access` +Expected: FAIL — `rows.len()` is 0. + +- [ ] **Step 3: Emit on a successful grant** + +In `crates/knot-server/src/routes/api/grants.rs`, in `put_inline`, replace the success arm: + +```rust + Ok(()) => StatusCode::NO_CONTENT.into_response(), +``` + +with: + +```rust + Ok(()) => { + if let Some(rest) = principal.strip_prefix("user:") + && let Ok(grantee) = Uuid::parse_str(rest) + { + emit_doc_shared(&state, doc_id, grantee, ctx.user_id).await; + } + StatusCode::NO_CONTENT.into_response() + } +``` + +and add at the bottom of the file: + +```rust +/// Tell the grantee a document was shared with them. Best-effort: a failure +/// here must not fail the grant that already committed. +async fn emit_doc_shared(state: &AppState, doc_id: Uuid, grantee: Uuid, actor: Uuid) { + let (Some(notifications), Some(docs)) = (state.notifications.clone(), state.docs.clone()) + else { + return; + }; + let Ok(Some(doc)) = docs.get(doc_id).await else { + return; + }; + let n = knot_storage::NewNotification { + workspace_id: doc.workspace_id, + user_id: grantee, + actor_id: Some(actor), + kind: knot_storage::NotificationKind::DocShared, + doc_id: Some(doc_id), + target_kind: "document".into(), + target_id: doc_id.to_string(), + dedupe_key: format!("share:{doc_id}:{grantee}"), + data: serde_json::json!({ "doc_title": doc.title }), + }; + if let Err(e) = notifications.emit(&n).await { + tracing::warn!(error=?e, %doc_id, "emit doc_shared"); + } +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `cargo nextest run -p knot-server --test notifications_integration` +Expected: PASS — all four tests. + +- [ ] **Step 5: Lint and commit** + +```bash +cargo clippy --workspace --all-targets --all-features -- -D warnings && cargo fmt --all -- --check +git add crates/knot-server +git commit -m "feat(grants): notify a user when a document is shared with them" +``` + +--- + +### Task 5: Explicit mention ids on the comment API + +The backend half of spec §3. `@Christian Hüning` cannot be resolved from text; the client sends the ids it picked. + +**Files:** + +- Modify: `crates/knot-server/src/routes/api/comments.rs` (`CreateThreadBody`, `CreateReplyBody`, `emit_comment_notifications`, both call sites) +- Test: `crates/knot-server/tests/notifications_integration.rs` (append) + +**Interfaces:** + +- Consumes: `WorkspaceStore::list_members`. +- Produces: `emit_comment_notifications` gains a parameter — final signature: + `async fn emit_comment_notifications(state: &AppState, doc_id: Uuid, thread_id: Uuid, comment_id: Uuid, author_id: Uuid, body: &str, explicit: &[Uuid])`. + `POST /api/docs/{id}/comments` and `…/replies` accept an optional `mentions: [uuid]` field. +- [ ] **Step 1: Write the failing tests** + +Append to `crates/knot-server/tests/notifications_integration.rs`: + +```rust +#[tokio::test(flavor = "multi_thread")] +async fn explicit_mention_ids_reach_a_user_whose_name_has_a_space() { + let (state, ws, doc, _alice, _bob) = seeded().await; + let cookie = login(&state, "alice@example.com").await; + + // A member the display-name regex can never match. + let hash = state.hasher.hash("hunter22").unwrap(); + let carol = state.users.as_ref().unwrap() + .create_local("carol@example.com", "Carol Danvers", &hash).await.unwrap(); + state.workspaces.as_ref().unwrap() + .add_member(ws, carol.id, WorkspaceRole::Editor).await.unwrap(); + + let (status, _) = post_json( + &state, + &cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ + "body": "over to you @Carol Danvers", + "mentions": [carol.id.to_string()], + }), + ) + .await; + assert_eq!(status, StatusCode::CREATED); + + let rows = state.notifications.as_ref().unwrap() + .list(carol.id, false, 50, None).await.unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].kind, "mention"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn an_id_for_a_non_member_is_ignored() { + let (state, _ws, doc, _alice, _bob) = seeded().await; + let cookie = login(&state, "alice@example.com").await; + + // A real user who is not a member of this workspace. + let hash = state.hasher.hash("hunter22").unwrap(); + let outsider = state.users.as_ref().unwrap() + .create_local("mallory@example.com", "Mallory", &hash).await.unwrap(); + + let (status, _) = post_json( + &state, + &cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "hi", "mentions": [outsider.id.to_string()] }), + ) + .await; + assert_eq!(status, StatusCode::CREATED); + + assert!( + state.notifications.as_ref().unwrap() + .list(outsider.id, false, 50, None).await.unwrap().is_empty(), + "a non-member must not be notified" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_comment_without_the_field_still_resolves_by_display_name() { + let (state, _ws, doc, _alice, bob) = seeded().await; + let cookie = login(&state, "alice@example.com").await; + + let (status, _) = post_json( + &state, + &cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "ping @Bob" }), + ) + .await; + assert_eq!(status, StatusCode::CREATED); + assert_eq!( + state.notifications.as_ref().unwrap() + .list(bob, false, 50, None).await.unwrap().len(), + 1 + ); +} +``` + +- [ ] **Step 2: Run to verify the first two fail** + +Run: `cargo nextest run -p knot-server --test notifications_integration` +Expected: `explicit_mention_ids_reach_a_user_whose_name_has_a_space` FAILS (0 rows). The other two pass already. + +- [ ] **Step 3: Accept the field** + +In `crates/knot-server/src/routes/api/comments.rs`, add to both `CreateThreadBody` and `CreateReplyBody`: + +```rust + /// User ids the client's mention picker resolved. Preferred over the + /// display-name regex, which cannot match a name containing a space. + #[serde(default)] + mentions: Vec, +``` + +- [ ] **Step 4: Use it in the emit path** + +Change the signature and the mention-resolution block of `emit_comment_notifications`: + +```rust +async fn emit_comment_notifications( + state: &AppState, + doc_id: Uuid, + thread_id: Uuid, + comment_id: Uuid, + author_id: Uuid, + body: &str, + explicit: &[Uuid], +) { +``` + +and replace the block that starts `let handles = extract_mentions(body);` through the end of the `if !handles.is_empty() { … }` with: + +```rust + // Explicit ids from the picker win; the regex is the fallback for + // clients that don't send them (and every comment written before this + // shipped). Either way, membership decides — an id for a non-member is + // dropped rather than trusted. + let members = match workspaces.list_members(doc.workspace_id).await { + Ok(m) => m, + Err(_) => return, + }; + let mentioned: Vec = if explicit.is_empty() { + let handles = extract_mentions(body); + members + .iter() + .filter(|m| handles.contains(&m.display_name.to_lowercase())) + .map(|m| m.user_id) + .collect() + } else { + members + .iter() + .filter(|m| explicit.contains(&m.user_id)) + .map(|m| m.user_id) + .collect() + }; +``` + +- [ ] **Step 5: Pass it at both call sites** + +In `create_thread` and `create_reply`, add `&body_req.mentions` as the final argument to `emit_comment_notifications`. Note `body_req` is moved by the store call in some arms — bind `let explicit = body_req.mentions.clone();` before the store call and pass `&explicit`. + +- [ ] **Step 6: Run to verify all pass** + +Run: `cargo nextest run -p knot-server --test notifications_integration --test comments_integration` +Expected: PASS — seven notification tests plus the comments suite. + +- [ ] **Step 7: Lint and commit** + +```bash +cargo clippy --workspace --all-targets --all-features -- -D warnings && cargo fmt --all -- --check +git add crates/knot-server +git commit -m "feat(comments): accept explicit mention ids + +The display-name regex captures @Christian and matches no member, so +anyone whose name contains a space was unmentionable. The picker knows +who it picked; it now says so. Regex kept for existing clients." +``` + +--- + +### Task 6: Task-assignment notifications inside the reindex + +**Files:** + +- Modify: `crates/knot-storage/src/tasks.rs:88-145` (`upsert_for_doc`) +- Test: `crates/knot-storage/tests/notifications.rs` (append) + +**Interfaces:** + +- Consumes: the `notifications` table (Task 1) directly — this write joins the transaction `upsert_for_doc` already opens, so it does not go through `NotificationStore`. +- Produces: `task_assigned` rows keyed `task_assigned:::`. +- Signature change: `upsert_for_doc` gains a trailing parameter `actor_id: Option` — the user whose edit triggered the reindex, `None` when unknown. +- [ ] **Step 1: Write the failing test** + +Append to `crates/knot-storage/tests/notifications.rs`: + +```rust +use knot_storage::{DocTaskInput, PgTaskStore, TaskStore}; + +#[tokio::test(flavor = "multi_thread")] +async fn assigning_a_task_notifies_once_and_survives_a_reorder() { + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()).create("default", "W").await.unwrap(); + let users = PgUserStore::new(pool.clone()); + let alice = users.create_local("alice@x.test", "Alice", "$h$").await.unwrap(); + let bob = users.create_local("bob@x.test", "Bob", "$h$").await.unwrap(); + let ws_store = PgWorkspaceStore::new(pool.clone()); + ws_store.add_member(ws.id, alice.id, WorkspaceRole::Owner).await.unwrap(); + ws_store.add_member(ws.id, bob.id, WorkspaceRole::Editor).await.unwrap(); + let docs = PgDocStore::new(pool.clone()); + let doc = docs + .create(ws.id, None, "Doc", &sort_key_between(None, None), alice.id) + .await + .unwrap(); + + let tasks = PgTaskStore::new(pool.clone()); + let notifications = PgNotificationStore::new(pool.clone()); + + let ship = DocTaskInput { + item_index: 0, + text: "ship it".into(), + assignee_user_id: Some(bob.id), + checked: false, + due_at: None, + }; + let review = DocTaskInput { + item_index: 1, + text: "review it".into(), + assignee_user_id: Some(bob.id), + checked: false, + due_at: None, + }; + + tasks + .upsert_for_doc(ws.id, doc.id, &[ship.clone(), review.clone()], Some(alice.id)) + .await + .unwrap(); + assert_eq!(notifications.list(bob.id, false, 50, None).await.unwrap().len(), 2); + + // Swap the two items. Every task id changes; no new notifications. + let swapped = vec![ + DocTaskInput { item_index: 0, ..review.clone() }, + DocTaskInput { item_index: 1, ..ship.clone() }, + ]; + tasks.upsert_for_doc(ws.id, doc.id, &swapped, Some(alice.id)).await.unwrap(); + assert_eq!( + notifications.list(bob.id, false, 50, None).await.unwrap().len(), + 2, + "reordering a list must not re-notify" + ); + + // Editing the text is a new task as far as the key is concerned. + let edited = vec![DocTaskInput { text: "ship it today".into(), ..ship.clone() }]; + tasks.upsert_for_doc(ws.id, doc.id, &edited, Some(alice.id)).await.unwrap(); + assert_eq!(notifications.list(bob.id, false, 50, None).await.unwrap().len(), 3); +} + +#[tokio::test(flavor = "multi_thread")] +async fn assigning_a_task_to_yourself_notifies_nobody() { + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()).create("default", "W").await.unwrap(); + let alice = PgUserStore::new(pool.clone()) + .create_local("alice@x.test", "Alice", "$h$").await.unwrap(); + PgWorkspaceStore::new(pool.clone()) + .add_member(ws.id, alice.id, WorkspaceRole::Owner).await.unwrap(); + let doc = PgDocStore::new(pool.clone()) + .create(ws.id, None, "Doc", &sort_key_between(None, None), alice.id) + .await + .unwrap(); + + PgTaskStore::new(pool.clone()) + .upsert_for_doc( + ws.id, + doc.id, + &[DocTaskInput { + item_index: 0, + text: "mine".into(), + assignee_user_id: Some(alice.id), + checked: false, + due_at: None, + }], + Some(alice.id), + ) + .await + .unwrap(); + + assert!( + PgNotificationStore::new(pool) + .list(alice.id, false, 50, None).await.unwrap().is_empty() + ); +} +``` + +Add `#[derive(Clone)]` usage support: `DocTaskInput` already derives `Clone`. + +- [ ] **Step 2: Run to verify it fails** + +Run: `cargo nextest run -p knot-storage --test notifications` +Expected: FAIL to compile — `upsert_for_doc` takes 3 arguments, not 4. + +- [ ] **Step 3: Extend the trait and implementation** + +In `crates/knot-storage/src/tasks.rs`, change the trait method: + +```rust + /// Replace the task set for `doc_id` with `items`. Rows that fell out + /// of the new set are deleted. `completed_at` is preserved across + /// re-indexing when the checked status doesn't change. + /// + /// `actor_id` is whoever's edit triggered the reindex; it becomes the + /// actor on any `task_assigned` notification, and suppresses the + /// notification when someone assigns a task to themselves. + async fn upsert_for_doc( + &self, + workspace_id: Uuid, + doc_id: Uuid, + items: &[DocTaskInput], + actor_id: Option, + ) -> Result<()>; +``` + +Mirror the signature on the `impl`. Inside, after the per-item `INSERT INTO doc_tasks … ON CONFLICT …` `.execute(&mut *tx).await?;`, add: + +```rust + // Notify a new assignee — but key on the task's *content*, not + // its id. Ids are ":" and every reorder + // rewrites them, so an id-keyed notification would re-fire for + // every assignee each time anyone moved a list item. + if let Some(assignee) = item.assignee_user_id + && Some(assignee) != actor_id + { + use sha2::{Digest, Sha256}; + let digest = Sha256::digest(item.text.as_bytes()); + let short = hex_prefix(&digest); + sqlx::query( + "INSERT INTO notifications \ + (workspace_id, user_id, actor_id, kind, doc_id, target_kind, target_id, dedupe_key, data) \ + VALUES ($1, $2, $3, 'task_assigned', $4, 'task', $5, $6, $7) \ + ON CONFLICT (user_id, dedupe_key) DO NOTHING", + ) + .bind(workspace_id) + .bind(assignee) + .bind(actor_id) + .bind(doc_id) + .bind(&id) + .bind(format!("task_assigned:{doc_id}:{short}:{assignee}")) + .bind(serde_json::json!({ "text": item.text })) + .execute(&mut *tx) + .await?; + } +``` + +and at the bottom of the file: + +```rust +/// First 8 bytes of a digest as lowercase hex — 16 characters, enough to +/// key a notification without carrying the whole hash in every row. +fn hex_prefix(digest: &[u8]) -> String { + digest[..8].iter().map(|b| format!("{b:02x}")).collect() +} +``` + +- [ ] **Step 4: Fix the callers** + +Run `cargo check --workspace --all-features` and add the new argument at each call site. Expect `crates/knot-server/src/reindex.rs` (pass the user whose edit dirtied the doc if the worker has it, otherwise `None`) and `crates/knot-storage/tests/tasks.rs` (pass `None`). + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `cargo nextest run -p knot-storage` +Expected: PASS — including the pre-existing `tasks.rs` suite. + +- [ ] **Step 6: Lint and commit** + +```bash +cargo clippy --workspace --all-targets --all-features -- -D warnings && cargo fmt --all -- --check +git add crates +git commit -m "feat(tasks): notify a new assignee, keyed on task content + +doc_tasks ids are ':' and every reindex rewrites +them, so an id-keyed notification would re-fire on every reorder." +``` + +--- + +### Task 7: The sweep — overdue tasks and retention + +**Files:** + +- Create: `crates/knot-server/src/notifications_sweep.rs` +- Modify: `crates/knot-server/src/lib.rs` (module list, beside `pub mod comments_listener;`) +- Modify: `crates/knot-server/src/main.rs:274-276` (spawn beside the comments listener) +- Test: `crates/knot-server/tests/notifications_sweep.rs` (create) + +**Interfaces:** + +- Consumes: `PgNotificationStore::prune`, the `doc_tasks` and `notifications` tables. +- Produces: + - `knot_server::notifications_sweep::run_once(pool: &PgPool, now: DateTime) -> Result` + - `pub struct SweepOutcome { pub due_emitted: u64, pub pruned: u64 }` + - `knot_server::notifications_sweep::spawn(pool: PgPool) -> JoinHandle<()>` — 15-minute interval. +- [ ] **Step 1: Write the failing test** + +Create `crates/knot-server/tests/notifications_sweep.rs`: + +```rust +//! The overdue-task sweep emits once per task per day and is safe to run +//! concurrently on every replica. + +use chrono::{Duration, Utc}; +use knot_server::notifications_sweep; +use knot_storage::{ + DocStore, DocTaskInput, NotificationStore, PgDocStore, PgNotificationStore, PgTaskStore, + PgUserStore, PgWorkspaceStore, TaskStore, UserStore, WorkspaceRole, WorkspaceStore, + sort_key_between, +}; + +#[tokio::test(flavor = "multi_thread")] +async fn overdue_task_emits_once_per_day_even_across_concurrent_sweeps() { + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()).create("default", "W").await.unwrap(); + let alice = PgUserStore::new(pool.clone()) + .create_local("alice@x.test", "Alice", "$h$").await.unwrap(); + let bob = PgUserStore::new(pool.clone()) + .create_local("bob@x.test", "Bob", "$h$").await.unwrap(); + let ws_store = PgWorkspaceStore::new(pool.clone()); + ws_store.add_member(ws.id, alice.id, WorkspaceRole::Owner).await.unwrap(); + ws_store.add_member(ws.id, bob.id, WorkspaceRole::Editor).await.unwrap(); + let doc = PgDocStore::new(pool.clone()) + .create(ws.id, None, "Doc", &sort_key_between(None, None), alice.id) + .await + .unwrap(); + + PgTaskStore::new(pool.clone()) + .upsert_for_doc( + ws.id, + doc.id, + &[DocTaskInput { + item_index: 0, + text: "overdue thing".into(), + assignee_user_id: Some(bob.id), + checked: false, + due_at: Some(Utc::now() - Duration::days(1)), + }], + Some(alice.id), + ) + .await + .unwrap(); + + let notifications = PgNotificationStore::new(pool.clone()); + // The assignment itself notified once; count from there. + let before = notifications.list(bob.id, false, 50, None).await.unwrap().len(); + + let now = Utc::now(); + // Two replicas sweeping at the same moment. + let (a, b) = tokio::join!( + notifications_sweep::run_once(&pool, now), + notifications_sweep::run_once(&pool, now), + ); + let total = a.unwrap().due_emitted + b.unwrap().due_emitted; + assert_eq!(total, 1, "concurrent sweeps must produce exactly one row"); + + let rows = notifications.list(bob.id, false, 50, None).await.unwrap(); + assert_eq!(rows.len(), before + 1); + assert!(rows.iter().any(|r| r.kind == "task_due")); + + // Same day again: nothing new. + let again = notifications_sweep::run_once(&pool, now).await.unwrap(); + assert_eq!(again.due_emitted, 0); + + // Tomorrow: one more. + let tomorrow = notifications_sweep::run_once(&pool, now + Duration::days(1)) + .await + .unwrap(); + assert_eq!(tomorrow.due_emitted, 1); +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_checked_task_is_never_overdue() { + let pool = knot_test_support::fresh_db().await.pool; + let ws = PgWorkspaceStore::new(pool.clone()).create("default", "W").await.unwrap(); + let alice = PgUserStore::new(pool.clone()) + .create_local("alice@x.test", "Alice", "$h$").await.unwrap(); + let bob = PgUserStore::new(pool.clone()) + .create_local("bob@x.test", "Bob", "$h$").await.unwrap(); + let ws_store = PgWorkspaceStore::new(pool.clone()); + ws_store.add_member(ws.id, alice.id, WorkspaceRole::Owner).await.unwrap(); + ws_store.add_member(ws.id, bob.id, WorkspaceRole::Editor).await.unwrap(); + let doc = PgDocStore::new(pool.clone()) + .create(ws.id, None, "Doc", &sort_key_between(None, None), alice.id) + .await + .unwrap(); + + PgTaskStore::new(pool.clone()) + .upsert_for_doc( + ws.id, + doc.id, + &[DocTaskInput { + item_index: 0, + text: "already done".into(), + assignee_user_id: Some(bob.id), + checked: true, + due_at: Some(Utc::now() - Duration::days(3)), + }], + Some(alice.id), + ) + .await + .unwrap(); + + let out = notifications_sweep::run_once(&pool, Utc::now()).await.unwrap(); + assert_eq!(out.due_emitted, 0); +} +``` + +- [ ] **Step 2: Run to verify it fails** + +Run: `cargo nextest run -p knot-server --test notifications_sweep` +Expected: FAIL to compile — `notifications_sweep` does not exist. + +- [ ] **Step 3: Write the sweep** + +Create `crates/knot-server/src/notifications_sweep.rs`: + +```rust +//! Periodic notification work: overdue tasks, and retention. +//! +//! Runs on every replica with no leader election. Both halves are safe to +//! run concurrently — the overdue emit is `ON CONFLICT DO NOTHING` against +//! `notifications_dedupe`, and the prune is idempotent by construction. + +use chrono::{DateTime, Utc}; +use sqlx::PgPool; +use tokio::task::JoinHandle; + +const INTERVAL: std::time::Duration = std::time::Duration::from_secs(15 * 60); + +#[derive(Debug, Default, PartialEq, Eq)] +pub struct SweepOutcome { + pub due_emitted: u64, + pub pruned: u64, +} + +/// Emit `task_due` for every open, assigned, overdue task, then prune. +/// The dedupe key carries the date, so an overdue task notifies once a day +/// rather than once every fifteen minutes. +pub async fn run_once(pool: &PgPool, now: DateTime) -> Result { + let emitted = sqlx::query( + "INSERT INTO notifications \ + (workspace_id, user_id, actor_id, kind, doc_id, target_kind, target_id, dedupe_key, data) \ + SELECT t.workspace_id, t.assignee_user_id, NULL, 'task_due', t.doc_id, 'task', t.id, \ + 'task_due:' || t.id || ':' || to_char($1::timestamptz, 'YYYY-MM-DD'), \ + jsonb_build_object('text', t.text, 'due_at', t.due_at) \ + FROM doc_tasks t \ + WHERE t.assignee_user_id IS NOT NULL \ + AND t.checked = false \ + AND t.due_at IS NOT NULL \ + AND t.due_at < $1 \ + ON CONFLICT (user_id, dedupe_key) DO NOTHING", + ) + .bind(now) + .execute(pool) + .await? + .rows_affected(); + + let pruned = sqlx::query( + "DELETE FROM notifications \ + WHERE (read_at IS NOT NULL AND created_at < $1 - interval '90 days') \ + OR created_at < $1 - interval '180 days'", + ) + .bind(now) + .execute(pool) + .await? + .rows_affected(); + + if emitted > 0 { + metrics::counter!("knot_notifications_emitted_total", "kind" => "task_due") + .increment(emitted); + } + Ok(SweepOutcome { + due_emitted: emitted, + pruned, + }) +} + +/// Spawn the 15-minute loop. One per process; every replica runs its own. +pub fn spawn(pool: PgPool) -> JoinHandle<()> { + tokio::spawn(async move { + let mut ticker = tokio::time::interval(INTERVAL); + // The first tick fires immediately; skip it so a rolling restart + // doesn't have every pod sweep at once on boot. + ticker.tick().await; + loop { + ticker.tick().await; + match run_once(&pool, Utc::now()).await { + Ok(out) => tracing::debug!( + due_emitted = out.due_emitted, + pruned = out.pruned, + "notification sweep" + ), + Err(e) => tracing::warn!(error=?e, "notification sweep failed"), + } + } + }) +} +``` + +- [ ] **Step 4: Register the module and spawn it** + +In `crates/knot-server/src/lib.rs`, beside `pub mod comments_listener;`: + +```rust +pub mod notifications_sweep; +``` + +In `crates/knot-server/src/main.rs`, immediately after the comments-listener spawn block (`tracing::info!("comments listener spawned");`), inside the same `if let Some(pool) = …` scope that provides a pool: + +```rust + let _handle = knot_server::notifications_sweep::spawn(pool.clone()); + tracing::info!("notification sweep spawned"); +``` + +If the surrounding block moved `pool`, clone it before the earlier use. + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `cargo nextest run -p knot-server --test notifications_sweep` +Expected: PASS — both tests. + +- [ ] **Step 6: Lint and commit** + +```bash +cargo clippy --workspace --all-targets --all-features -- -D warnings && cargo fmt --all -- --check +git add crates/knot-server +git commit -m "feat(server): sweep overdue tasks and prune old notifications + +Every replica runs it; the dedupe index makes concurrent sweeps +idempotent, so there is no leader election to operate." +``` + +--- + +### Task 8: HTTP endpoints + +**Files:** + +- Create: `crates/knot-server/src/routes/api/notifications.rs` +- Modify: `crates/knot-server/src/routes/api/mod.rs:20-31` (module + `.merge`) +- Test: `crates/knot-server/tests/notifications_integration.rs` (append) + +**Interfaces:** + +- Consumes: `AppState.notifications`, `AppState.acl` (`AclCache::effective_role`). +- Produces: + - `GET /api/notifications?filter=unread|all&limit=&cursor=` → `{ "items": [NotificationRow], "next_cursor": }` + - `GET /api/notifications/unread_count` → `{ "count": n, "capped": bool }` + - `POST /api/notifications/read` — body `{"ids":[…]}` or `{"all":true}` → 204 + - `NotificationRow { id: i64, kind: String, doc_id: Option, doc_title: Option, target_kind: String, target_id: String, actor_display_name: Option, data: serde_json::Value, created_at: String, read: bool }` +- [ ] **Step 1: Write the failing tests** + +Append to `crates/knot-server/tests/notifications_integration.rs`: + +```rust +async fn get_json(state: &AppState, cookie: &str, uri: &str) -> (StatusCode, serde_json::Value) { + let app = router_with_state(state.clone()); + let res = app + .oneshot( + Request::builder() + .method("GET") + .uri(uri) + .header("cookie", cookie) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + let status = res.status(); + let bytes = res.into_body().collect().await.unwrap().to_bytes(); + let json = if bytes.is_empty() { + serde_json::Value::Null + } else { + serde_json::from_slice(&bytes).unwrap_or(serde_json::Value::Null) + }; + (status, json) +} + +#[tokio::test(flavor = "multi_thread")] +async fn inbox_lists_counts_and_marks_read() { + let (state, _ws, doc, _alice, _bob) = seeded().await; + let alice_cookie = login(&state, "alice@example.com").await; + let bob_cookie = login(&state, "bob@example.com").await; + + post_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "hey @Bob" }), + ) + .await; + + let (status, count) = get_json(&state, &bob_cookie, "/api/notifications/unread_count").await; + assert_eq!(status, StatusCode::OK); + assert_eq!(count["count"], 1); + assert_eq!(count["capped"], false); + + let (status, list) = get_json(&state, &bob_cookie, "/api/notifications?filter=unread").await; + assert_eq!(status, StatusCode::OK); + let items = list["items"].as_array().unwrap(); + assert_eq!(items.len(), 1); + assert_eq!(items[0]["kind"], "mention"); + assert_eq!(items[0]["doc_title"], "Test Doc"); + assert_eq!(items[0]["actor_display_name"], "Alice"); + assert_eq!(items[0]["read"], false); + let id = items[0]["id"].as_i64().unwrap(); + + let (status, _) = post_json( + &state, + &bob_cookie, + "/api/notifications/read", + serde_json::json!({ "ids": [id] }), + ) + .await; + assert_eq!(status, StatusCode::NO_CONTENT); + + let (_, count) = get_json(&state, &bob_cookie, "/api/notifications/unread_count").await; + assert_eq!(count["count"], 0); +} + +#[tokio::test(flavor = "multi_thread")] +async fn the_inbox_is_per_user() { + let (state, _ws, doc, _alice, _bob) = seeded().await; + let alice_cookie = login(&state, "alice@example.com").await; + post_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "hey @Bob" }), + ) + .await; + + // Alice sees nothing — the row belongs to Bob. + let (_, list) = get_json(&state, &alice_cookie, "/api/notifications").await; + assert!(list["items"].as_array().unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_row_for_a_doc_the_user_cannot_read_is_filtered_out() { + let (state, _ws, doc, _alice, bob) = seeded().await; + let alice_cookie = login(&state, "alice@example.com").await; + let bob_cookie = login(&state, "bob@example.com").await; + + post_json( + &state, + &alice_cookie, + &format!("/api/docs/{doc}/comments"), + serde_json::json!({ "body": "hey @Bob" }), + ) + .await; + assert_eq!( + get_json(&state, &bob_cookie, "/api/notifications").await.1["items"] + .as_array() + .unwrap() + .len(), + 1 + ); + + // A notification pointing at a document Bob cannot read. Workspace + // membership grants a role on every doc in that workspace + // (knot_docs::acl::resolve), so the way to be unable to read a doc is + // for it to live in another workspace — which is exactly the tenancy + // guard at acl.rs:59-62, and the same `effective_role` -> None branch a + // revoked grant produces. + let other_ws = state.workspaces.as_ref().unwrap() + .create("other", "Other").await.unwrap(); + let other_doc = state.docs.as_ref().unwrap() + .create(other_ws.id, None, "Elsewhere", "m", bob).await.unwrap(); + state.notifications.as_ref().unwrap() + .emit(&knot_storage::NewNotification { + workspace_id: other_ws.id, + user_id: bob, + actor_id: None, + kind: knot_storage::NotificationKind::DocShared, + doc_id: Some(other_doc.id), + target_kind: "document".into(), + target_id: other_doc.id.to_string(), + dedupe_key: format!("share:{}:{bob}", other_doc.id), + data: serde_json::json!({}), + }) + .await + .unwrap(); + + // Two rows exist for Bob; the endpoint returns only the readable one. + assert_eq!( + state.notifications.as_ref().unwrap() + .list(bob, false, 50, None).await.unwrap().len(), + 2 + ); + let items = get_json(&state, &bob_cookie, "/api/notifications").await.1; + let items = items["items"].as_array().unwrap(); + assert_eq!(items.len(), 1, "the unreadable row is filtered, not 403'd"); + assert_eq!(items[0]["kind"], "mention"); +} +``` + +- [ ] **Step 2: Run to verify it fails** + +Run: `cargo nextest run -p knot-server --test notifications_integration inbox_lists` +Expected: FAIL — 404, because the route does not exist. + +- [ ] **Step 3: Write the route** + +Create `crates/knot-server/src/routes/api/notifications.rs`: + +```rust +//! The per-user notification inbox. +//! +//! GET /api/notifications?filter=unread|all&limit=&cursor= → { items, next_cursor } +//! GET /api/notifications/unread_count → { count, capped } +//! POST /api/notifications/read { ids: [] } | { all: true } → 204 +//! +//! Rows are addressed to exactly one recipient, so ownership needs no join. +//! Document *access* is a separate question: a notification can outlive the +//! grant that made it visible, so rows carrying a `doc_id` are re-checked +//! against `effective_role` and filtered, never 403'd. + +use axum::{ + Json, Router, + body::Body, + extract::{Query, Request, State}, + http::StatusCode, + response::{IntoResponse, Response}, + routing::{get, post}, +}; +use serde::{Deserialize, Serialize}; + +use crate::AppState; +use crate::auth::AuthContext; +use crate::http_error::json_err; + +const DEFAULT_LIMIT: i64 = 50; +const MAX_LIMIT: i64 = 100; +const UNREAD_CAP: i64 = 100; + +pub fn router() -> Router { + Router::new() + .route("/api/notifications", get(list)) + .route("/api/notifications/unread_count", get(unread_count)) + .route("/api/notifications/read", post(mark_read)) +} + +#[derive(Deserialize)] +struct ListQuery { + #[serde(default)] + filter: Option, + #[serde(default)] + limit: Option, + #[serde(default)] + cursor: Option, +} + +#[derive(Serialize)] +struct NotificationRow { + id: i64, + kind: String, + doc_id: Option, + doc_title: Option, + target_kind: String, + target_id: String, + actor_display_name: Option, + data: serde_json::Value, + created_at: String, + read: bool, +} + +#[derive(Serialize)] +struct ListResponse { + items: Vec, + next_cursor: Option, +} + +#[derive(Serialize)] +struct CountResponse { + count: i64, + capped: bool, +} + +#[derive(Deserialize)] +struct ReadBody { + #[serde(default)] + ids: Vec, + #[serde(default)] + all: bool, +} + +fn internal() -> Response { + json_err(StatusCode::INTERNAL_SERVER_ERROR, "internal", "") +} + +async fn list(State(state): State, Query(q): Query, req: Request) -> Response { + let Some(ctx) = req.extensions().get::().cloned() else { + return json_err(StatusCode::UNAUTHORIZED, "auth.session_required", ""); + }; + let (Some(notifications), Some(acl)) = (state.notifications.clone(), state.acl.clone()) else { + return internal(); + }; + let unread_only = q.filter.as_deref() == Some("unread"); + let limit = q.limit.unwrap_or(DEFAULT_LIMIT).clamp(1, MAX_LIMIT); + + let rows = match notifications + .list(ctx.user_id, unread_only, limit, q.cursor) + .await + { + Ok(r) => r, + Err(e) => { + tracing::error!(error=?e, "notifications list"); + return internal(); + } + }; + let next_cursor = if rows.len() as i64 == limit { + rows.last().map(|r| r.id) + } else { + None + }; + + let mut items = Vec::with_capacity(rows.len()); + for r in rows { + // Filter, don't fail: access can be revoked after the row is written. + if let Some(doc_id) = r.doc_id { + match acl.effective_role(ctx.workspace_id, doc_id, ctx.user_id).await { + Ok(Some(_)) => {} + Ok(None) => continue, + Err(e) => { + tracing::error!(error=?e, "notifications acl check"); + return internal(); + } + } + } + items.push(NotificationRow { + id: r.id, + kind: r.kind, + doc_id: r.doc_id.map(|d| d.to_string()), + doc_title: r.doc_title, + target_kind: r.target_kind, + target_id: r.target_id, + actor_display_name: r.actor_display_name, + data: r.data, + created_at: r.created_at.to_rfc3339(), + read: r.read_at.is_some(), + }); + } + + Json(ListResponse { items, next_cursor }).into_response() +} + +async fn unread_count(State(state): State, req: Request) -> Response { + let Some(ctx) = req.extensions().get::().cloned() else { + return json_err(StatusCode::UNAUTHORIZED, "auth.session_required", ""); + }; + let Some(notifications) = state.notifications.clone() else { + return internal(); + }; + match notifications.unread_count(ctx.user_id, UNREAD_CAP).await { + Ok(n) => Json(CountResponse { + count: n, + capped: n >= UNREAD_CAP, + }) + .into_response(), + Err(e) => { + tracing::error!(error=?e, "notifications unread_count"); + internal() + } + } +} + +async fn mark_read(State(state): State, req: Request) -> Response { + let Some(ctx) = req.extensions().get::().cloned() else { + return json_err(StatusCode::UNAUTHORIZED, "auth.session_required", ""); + }; + let Some(notifications) = state.notifications.clone() else { + return internal(); + }; + let bytes = match axum::body::to_bytes(req.into_body(), 64 * 1024).await { + Ok(b) => b, + Err(_) => return json_err(StatusCode::PAYLOAD_TOO_LARGE, "bad_request", ""), + }; + let body: ReadBody = match serde_json::from_slice(&bytes) { + Ok(b) => b, + Err(_) => return json_err(StatusCode::BAD_REQUEST, "bad_request", ""), + }; + + let res = if body.all { + notifications.mark_all_read(ctx.user_id).await + } else { + notifications.mark_read(ctx.user_id, &body.ids).await + }; + match res { + Ok(_) => StatusCode::NO_CONTENT.into_response(), + Err(e) => { + tracing::error!(error=?e, "notifications mark_read"); + internal() + } + } +} +``` + +- [ ] **Step 4: Register the router** + +In `crates/knot-server/src/routes/api/mod.rs`, add `pub mod notifications;` beside the other module declarations and `.merge(notifications::router())` into the chain beside `.merge(tasks::router())`. + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `cargo nextest run -p knot-server --test notifications_integration` +Expected: PASS — all tests in the file. + +- [ ] **Step 6: Lint and commit** + +```bash +cargo clippy --workspace --all-targets --all-features -- -D warnings && cargo fmt --all -- --check +git add crates/knot-server +git commit -m "feat(api): notification list, unread count and mark-read" +``` + +--- + +### Task 9: Web API client and the unread badge + +**Files:** + +- Create: `web/src/lib/notifications.api.ts` +- Create: `web/src/features/notifications/useNotifications.ts` +- Modify: `web/src/features/workspace/WorkspaceHeader.tsx:1-2` (imports), `:38-46` (nav) +- Test: `web/src/features/notifications/useNotifications.test.ts` + +**Interfaces:** + +- Consumes: `apiFetch`, `ApiResult` from `web/src/lib/api.ts`. +- Produces: + - `notificationsApi.list(filter, cursor?)`, `notificationsApi.unreadCount()`, `notificationsApi.markRead(ids)`, `notificationsApi.markAllRead()` + - `type Notification` and `type NotificationList` in `notifications.api.ts` + - `useUnreadCount()`, `useNotificationList(filter)`, `useMarkRead()` in `useNotifications.ts` + - `formatBadge(count: number, capped: boolean): string` — exported for the unit test. +- [ ] **Step 1: Write the failing test** + +Create `web/src/features/notifications/useNotifications.test.ts`: + +```ts +import { describe, expect, it } from "vitest"; + +import { formatBadge } from "./useNotifications"; + +describe("formatBadge", () => { + it("renders a plain count under the cap", () => { + expect(formatBadge(3, false)).toBe("3"); + }); + + it("renders 99+ once capped", () => { + expect(formatBadge(100, true)).toBe("99+"); + }); + + it("renders an empty string at zero so the badge can hide", () => { + expect(formatBadge(0, false)).toBe(""); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `cd web && pnpm test -- useNotifications` +Expected: FAIL — cannot resolve `./useNotifications`. + +- [ ] **Step 3: Write the API client** + +Create `web/src/lib/notifications.api.ts`: + +```ts +import { apiFetch, type ApiResult } from "./api"; + +export type Notification = { + id: number; + kind: "mention" | "reply" | "task_assigned" | "task_due" | "doc_shared"; + doc_id: string | null; + doc_title: string | null; + target_kind: string; + target_id: string; + actor_display_name: string | null; + data: Record; + created_at: string; + read: boolean; +}; + +export type NotificationList = { + items: Notification[]; + next_cursor: number | null; +}; + +export type UnreadCount = { count: number; capped: boolean }; + +export const notificationsApi = { + async list(filter: "all" | "unread" = "all", cursor?: number): Promise> { + const qs = new URLSearchParams({ filter }); + if (cursor !== undefined) qs.set("cursor", String(cursor)); + return apiFetch(`/api/notifications?${qs.toString()}`); + }, + async unreadCount(): Promise> { + return apiFetch("/api/notifications/unread_count"); + }, + async markRead(ids: number[]): Promise> { + return apiFetch("/api/notifications/read", { method: "POST", body: { ids } }); + }, + async markAllRead(): Promise> { + return apiFetch("/api/notifications/read", { method: "POST", body: { all: true } }); + }, +}; +``` + +- [ ] **Step 4: Write the hooks** + +Create `web/src/features/notifications/useNotifications.ts`: + +```ts +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; + +import { notificationsApi, type NotificationList, type UnreadCount } from "../../lib/notifications.api"; + +/** Delivery is polling, not push — see the spec's §6. Window focus covers + * most of the perceived latency; this is the ceiling. */ +export const POLL_INTERVAL_MS = 30_000; + +export function formatBadge(count: number, capped: boolean): string { + if (count <= 0) return ""; + return capped ? "99+" : String(count); +} + +export function useUnreadCount() { + return useQuery({ + queryKey: ["notifications", "unread_count"], + queryFn: async () => { + const res = await notificationsApi.unreadCount(); + if ("error" in res) throw new Error(res.error.message); + return res.ok; + }, + refetchInterval: POLL_INTERVAL_MS, + refetchOnWindowFocus: true, + }); +} + +export function useNotificationList(filter: "all" | "unread") { + return useQuery({ + queryKey: ["notifications", "list", filter], + queryFn: async () => { + const res = await notificationsApi.list(filter); + if ("error" in res) throw new Error(res.error.message); + return res.ok; + }, + }); +} + +export function useMarkRead() { + const qc = useQueryClient(); + return useMutation({ + mutationFn: async (ids: number[] | "all") => { + const res = ids === "all" + ? await notificationsApi.markAllRead() + : await notificationsApi.markRead(ids); + if ("error" in res) throw new Error(res.error.message); + }, + onSuccess: () => { + void qc.invalidateQueries({ queryKey: ["notifications"] }); + }, + }); +} +``` + +- [ ] **Step 5: Run the test to verify it passes** + +Run: `cd web && pnpm test -- useNotifications` +Expected: PASS — 3 tests. + +- [ ] **Step 6: Add the badge to the sidebar** + +In `web/src/features/workspace/WorkspaceHeader.tsx`, extend the lucide import to include `Bell`, add the hook import: + +```tsx +import { useUnreadCount, formatBadge } from "../notifications/useNotifications"; +``` + +Inside the component, before `return`: + +```tsx + const unread = useUnreadCount(); + const badge = unread.data ? formatBadge(unread.data.count, unread.data.capped) : ""; +``` + +and add this `Link` as the first child of the `