From 814d4cdbe6dae03384beb6af780aa4ac5dd1acd5 Mon Sep 17 00:00:00 2001 From: Krasimir Kostadinov <12051991+krazymir@users.noreply.github.com> Date: Wed, 21 Oct 2026 11:27:44 +0300 Subject: [PATCH] perf(notifications): one pulse poll, indexable inbox gates, one-statement roles (v1.18.0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cuts the steady-state request and query load without changing what anyone sees (INNOBOX_SPEC.md v0.19: §4.2 Resolution, §5 notifications, §12.1, §12.2 pulse). - One pulse per tab: GET /api/me/pulse returns the bell's unread count, the new-challenges count, the triage attention count (null for non-admins) and the system banner. The shell polls it every 30 s while the tab is visible and not at all while hidden; a returning tab catches up at once. The bell list loads on open. The new-count and triage/attention endpoints are retired. - Indexable inbox gates: migration 0051 adds notifications.challenge_id and solution_id, derived from the payload target by a trigger and backfilled once. The visibility and trash gates, the delete cascade and mark-read-on-open now join by primary key instead of matching payload text against every challenge. One partial index replaces the two per-type payload indexes. - Role resolution: the user row, active/session check, grants, bootstrap group and people access list come back from one SQL statement, memoized for one request only. No role cache across requests; invariant 1 unchanged. Tests: unit tests for the pulse helpers and the request memo; DB tests for the one-statement resolver, the derived columns and gates, and the pulse. --- ENTRA_AUTH_SPEC.md | 2 +- INNOBOX_SPEC.md | 138 +++++-- .../0051_notification_item_columns.sql | 110 ++++++ docs/performance-audit-2026-10-21.md | 341 ++++++++++++++++++ packages/shared/src/item-delete.ts | 11 +- packages/shared/src/version.ts | 2 +- packages/web/package.json | 4 +- .../app/api/admin/triage/attention/route.ts | 26 -- .../src/app/api/challenges/new-count/route.ts | 19 - packages/web/src/app/api/me/pulse/route.ts | 20 + .../web/src/app/api/me/pulse/store.dbtest.ts | 102 ++++++ packages/web/src/app/api/me/pulse/store.ts | 28 ++ .../api/notifications/item-columns.dbtest.ts | 168 +++++++++ .../web/src/app/api/notifications/route.ts | 11 +- .../web/src/app/api/notifications/store.ts | 72 ++-- packages/web/src/app/whats-new/changelog.ts | 6 + packages/web/src/components/AppShell.tsx | 79 +--- .../web/src/components/NotificationBell.tsx | 50 ++- packages/web/src/components/usePulse.ts | 101 ++++++ packages/web/src/lib/auth.ts | 142 ++------ packages/web/src/lib/notify.ts | 5 +- packages/web/src/lib/presence.test.ts | 8 +- packages/web/src/lib/presence.ts | 17 +- packages/web/src/lib/pulse.test.ts | 64 ++++ packages/web/src/lib/pulse.ts | 70 ++++ packages/web/src/lib/request-memo.test.ts | 45 +++ packages/web/src/lib/request-memo.ts | 20 + .../web/src/lib/role-resolution.dbtest.ts | 149 ++++++++ packages/web/src/lib/role-resolution.ts | 135 +++++++ 29 files changed, 1623 insertions(+), 322 deletions(-) create mode 100644 db/migrations/0051_notification_item_columns.sql create mode 100644 docs/performance-audit-2026-10-21.md delete mode 100644 packages/web/src/app/api/admin/triage/attention/route.ts delete mode 100644 packages/web/src/app/api/challenges/new-count/route.ts create mode 100644 packages/web/src/app/api/me/pulse/route.ts create mode 100644 packages/web/src/app/api/me/pulse/store.dbtest.ts create mode 100644 packages/web/src/app/api/me/pulse/store.ts create mode 100644 packages/web/src/app/api/notifications/item-columns.dbtest.ts create mode 100644 packages/web/src/components/usePulse.ts create mode 100644 packages/web/src/lib/pulse.test.ts create mode 100644 packages/web/src/lib/pulse.ts create mode 100644 packages/web/src/lib/request-memo.test.ts create mode 100644 packages/web/src/lib/request-memo.ts create mode 100644 packages/web/src/lib/role-resolution.dbtest.ts create mode 100644 packages/web/src/lib/role-resolution.ts diff --git a/ENTRA_AUTH_SPEC.md b/ENTRA_AUTH_SPEC.md index 3d7d22b..0d9b353 100644 --- a/ENTRA_AUTH_SPEC.md +++ b/ENTRA_AUTH_SPEC.md @@ -31,7 +31,7 @@ open; no user table exists. No local accounts will ever exist (INNOBOX_SPEC.md | Role vocabulary | `platform_admin` (scope: platform), `namespace_admin`, `committee`, `member` (per-namespace). `member` grants namespace membership only — visibility + submission targeting (§4.3). **Assignee is per-challenge (`challenges.assignee_id`), never group-based.** Baseline: every active authenticated user is an implicit `member` of the built-in `global` namespace | | Mapping management | Platform admins only, every change audited. Phase 1 ships a minimal `/admin` surface (namespaces CRUD-lite + role mappings list/add/remove, dead-mapping flag); the full §14.3 settings page is Phase 4 polish | | Bootstrap admins | `INNOBOX_BOOTSTRAP_ADMIN_GROUP` (Entra group object id, already in `.env.example`): while set, treated as an implicit `(group → platform_admin)` mapping. Its membership is synced by reconciliation from worker boot, so bootstrap works before any SCIM assignment | -| Role staleness | **Per-request resolution** (one indexed join); `users.active` checked on every request at session validation. No role cache in v1 | +| Role staleness | **Per-request resolution** — the user row, the `active`/session-validity check, the role grants (including the bootstrap group) and the `people` access list in **one SQL statement**, evaluated at most once per request (INNOBOX_SPEC.md §4.2 *Resolution*); `users.active` checked on every request at session validation. No role cache in v1 — nothing is reused across requests | | Session | Auth.js JWT cookie session (`HttpOnly`, `Secure`, `SameSite=Lax`), rolling, **7-day** max age. The JWT carries `oid` + display basics only — **never roles** | | Sign-out | Local only (destroy the InnoBox session and expire every Auth.js cookie the request carried — INNOBOX_SPEC.md §3); no Entra front-channel logout | | Rollout | Greenfield hard cutover: once Phase 1 deploys, all routes require sign-in. Local dev/e2e use the `INNOBOX_DEV_AUTH` bypass (never set in production — §2.3 release checklist) | diff --git a/INNOBOX_SPEC.md b/INNOBOX_SPEC.md index ec72c16..90fff1b 100644 --- a/INNOBOX_SPEC.md +++ b/INNOBOX_SPEC.md @@ -4,7 +4,7 @@ > is reviewed and approved, and only then gets implemented. Code follows spec, never > the reverse. `§n` references are internal to this document. > -> Status: **v0.18 — draft for review** (2026-10-19; v0.2 added the UI and CI/CD +> Status: **v0.19 — draft for review** (2026-10-21; v0.2 added the UI and CI/CD > sections; v0.3 pinned every externally-referenced convention inline, so the > document is fully self-contained; v0.4 pinned the canonical production URL as > deployment configuration (`PUBLIC_BASE_URL`), aligned §12.1 with the bundled @@ -53,7 +53,12 @@ > (2026-10-19) specified **hype** — a scarce weekly endorsement of 3 per person on open > challenges and solutions, counting double in an item's first week, with a per-viewer > "Hyped this week" board on Home, author points and a Talent Scout badge for the hyper -> (§26)). +> (§26); v0.19 (2026-10-21) cut the steady-state request and query load without changing +> what anyone sees: the bell, the new-challenges bubble, the triage attention bubble and +> the system banner now share **one pulse poll** that pauses while the tab is hidden +> (§12.2), inbox rows carry the challenge and solution they are about as real columns so +> the read-time gates are index lookups (§5, §12.1), and a request's identity, roles and +> access list resolve in **one statement**, at most once per request (§4.2)). > Derived from the legacy > Power Apps "InnoBox" canvas app (solution export `innobox-solution-master@e67e8ca94e4`) > and a requirements interview with the product owner. This is a **brand-new @@ -819,7 +824,8 @@ Entra runbook); on conflict this document wins. re-assembled with a newer one. Non-auth browser state (theme, collapsed admin cards, the §13.1 view choice) is untouched. Sessions: rolling 7-day cookie; the user's `active` flag and roles are resolved - from the DB on every request — the session token never carries roles. + from the DB on every request — the session token never carries roles — in one + statement, at most once per request (§4.2 *Resolution*). - **Sign out everywhere (added in v1.16).** Plain sign-out is per device: a session cookie on another browser stays valid until its 7-day cap. The account menu's **Sign out everywhere** item (§2.2) — a confirm dialog, then @@ -1076,6 +1082,26 @@ shown (§13.6). A user may hold different roles in different namespaces. Roles are additive. +**Resolution (one statement, once per request — v1.18).** Roles come from SCIM-synced +group membership and `role_mappings` on every request, never from the session token +(invariant 1), and nothing is cached across requests. Within that rule the lookup is +bounded: + +- **One statement.** The user row (by the token's `oid`), the `active` and + `scrubbed_at` check, the §3 `sessions_valid_from` comparison inputs, the role grants + (group membership ⋈ `role_mappings`, plus the `INNOBOX_BOOTSTRAP_ADMIN_GROUP` + implicit mapping while it is set) and the §4.3 `people` access list (`challenge_access`) + are read in **one SQL statement** — one database round trip, not one per source. The + grants and the access list come back as arrays on the user row. The `global` + namespace's id, which never changes after migration, may be held in process memory. +- **At most once per request.** The result is memoized for the lifetime of a single + request only, so a handler, the helpers it calls and the §14.5 presence stamp share + one resolution. The memo dies with the request; a role, membership or access change + is visible on the very next request, exactly as before. +- **Bulk resolution** for an event's recipient set (§12.1 dispatch) uses the same + statement over a list of user ids — one round trip whatever the count — and returns the + same grants a per-request resolution would. + ### §4.3 Visibility - Every challenge has **`visibility: org | namespace | people`**. The submitter @@ -1290,7 +1316,26 @@ Entities (Postgres; key fields only — types/constraints finalized in migration and may address an outside submitter by e-mail instead of a user (nullable `recipient_email` with `user_id` null, a check constraint requiring exactly one — §24.6). Rows targeting a challenge/solution are deleted with it (§10.3), so no - inbox item ever points at a vanished entity. **Comment rows coalesce** (§12.1): a + inbox item ever points at a vanished entity. **What a row is about (v1.18):** + `notifications` carries two nullable columns, `challenge_id` and `solution_id`, + written at insert from the dispatch target — a challenge row sets `challenge_id`; a + solution row sets `solution_id` **and** its parent's `challenge_id`; a comment or + hype row sets them from its parent the same way; a scan-failure row about a bound + attachment sets them from the attachment's parent. Rows that name no item + (achievements, a staged attachment's scan failure, `system.error`, + `assignments_transferred`) leave both null. A database trigger derives both columns from + the row's payload target on every insert (and on a payload update, as coalescing does), + so no writer can forget them. They are lookup keys, not foreign keys: the §10.3 cascade deletes the rows + explicitly, as it always has. Migration 0051 backfills existing rows once, from the + payload's `parentType`/`parentId` and `challengeId` keys and, for rows that carry only + a deep link, from the `/challenges/` and `#SOL-` link forms; a row whose item no + longer resolves stays null. One index serves the inbox gates, the cascade and the + "opening the challenge reads its rows" update: `(user_id, challenge_id) WHERE read_at + IS NULL`, replacing the two per-type expression indexes on `payload->>'challengeId'` + (0026, 0050); `(challenge_id)` and `(solution_id)` partial indexes (`WHERE … IS NOT + NULL`) serve the cascade and trash lookups. The payload keys stay in the payload — the + rendered message, the link and the coalescing keys still use them — but no filter + matches on them any more. **Comment rows coalesce** (§12.1): a partial unique index on `(user_id, payload->>'parentType', payload->>'parentId') WHERE read_at IS NULL AND type = 'comment_posted'` lets the insert become an atomic update-in-place while the row is unread; the payload carries `count`, `latestBy`, @@ -2464,10 +2509,14 @@ row written while the recipient could see the item is **hidden** from the bell l the unread count once they no longer can — the challenge was narrowed to `namespace` or `people` (§4.3) after the row was written, or the recipient left the namespace — and comes back if access returns, exactly as rows about the trash are kept but hidden -(§10.4). The read-time predicate is the §4.3 SQL twin on the row's `challengeId` / -`parentType` + `parentId` payload keys (the keys every item-bound row carries); rows -that name no item (achievements, scan failures) are unaffected. The §25.6 agent view -already filters this way; the web inbox now matches it. **A deactivated user (`active = false`, the §3 leaver) or an erased +(§10.4). The read-time predicate is the §4.3 SQL twin evaluated on the challenge the +row is about — the row's `challenge_id` column (§5), joined to `challenges` by primary +key — and the trash filter looks up `challenge_id` and `solution_id` the same way; rows +that name no item (both columns null — achievements, scan failures) are unaffected. +**Since v1.18 neither gate matches on payload keys or the deep link**, so each costs one +index lookup per row rather than a scan of the challenges table. The web inbox list, +the §12.2 pulse count and the §25.6 agent view share this one predicate, so they can +never disagree. **A deactivated user (`active = false`, the §3 leaver) or an erased one (`scrubbed_at` set) is never a recipient**, on any channel: the dispatch drops them at insert time, beside the visibility drop, so neither the inbox row nor the outbox row is written; the role-resolving recipient helpers (namespace admins, @@ -2629,8 +2678,48 @@ state, mark-read and mark-all-read. In-app notifications are always on; the per- opt-out (§13.5 profile) affects e-mail only, and the §12.1 per-event preferences remove a recipient before any row exists. A coalesced comment row (§12.1) renders its count and latest commenter; the platform admins' `system.error` alert (§14.7) is an -ordinary inbox row that is never e-mailed. The 30-second unread poll also carries the -active §14.6 system banner, so no second poll exists for it. +ordinary inbox row that is never e-mailed. + +**The pulse — one poll per tab (v1.18).** Every signed-in tab keeps its live counters +current through **one** request, `GET /api/me/pulse`, instead of one poller per +widget: + +```json +{ "unreadCount": 3, "newChallenges": 5, "attention": 2, "banner": null } +``` + +- `unreadCount` — the bell's unread count, under the §12.1 read-time gates. +- `newChallenges` — the §13.1 "new since your last visit" count. +- `attention` — the §14.4 triage attention count for a namespace or platform admin; + **`null`** for everyone else (the key is present, the value is null, so the shape + never varies by role and nothing about it is computed for a non-admin). +- `banner` — the active §14.6 system banner, `{ message, tone, url, expiresAt }` or + `null`. + +All four are bare values with the same visibility and namespace scoping as the surfaces +they feed (invariant 2): no titles, authors or namespaces. The four reads run +concurrently after one §4.2 resolution. + +- **Cadence.** Every **30 s** while the tab is **visible**. While + `document.visibilityState` is `hidden` the tab **does not poll at all**; on becoming + visible again it polls **at once** (if the last pulse is older than 30 s) and resumes + the cadence. A hidden tab therefore shows values from when it was last visible, and + catches up the moment the user looks at it. +- **The bell list is not polled.** `GET /api/notifications` runs when the bell is + opened, and again after a mark-read or mark-all-read. Its response carries + `unreadCount` too, which the shell adopts, so the badge and the open list agree. +- **Immediate refresh.** Actions that change a counter re-read the pulse at once + rather than waiting for the next tick: mark-read and mark-all-read, opening the + triage queue (§14.4 *Clearing*), and leaving the Challenges surface (§13.1). +- **No needless re-render.** The shell keeps the previous values when a pulse returns + the same ones, so an idle tick re-renders nothing. +- **Failure.** A failed or non-2xx pulse keeps the last values and retries on the next + tick; a 401 means the session has ended and stops polling (§3). +- **Presence.** The pulse is a background poller: it is **not** on the §14.5 presence + allowlist and never stamps `last_seen_at`. + +`GET /api/challenges/new-count` and `GET /api/admin/triage/attention` are **retired** +— the pulse is their only reader. ### §12.3 Follows @@ -2965,8 +3054,8 @@ challenges_seen_at` **and** it is visible to them (invariant 2). A new solution comment on an existing challenge does **not** make it new, and the viewer's own just-submitted items count like anyone else's. The **Challenges** nav item shows a superscript `1`–`9` / `9+` count of new items (hidden at zero; the §14.4 bubble -style), polled on the 30-second bell cadence via `GET /api/challenges/new-count → -{ count }` — a bare integer, no titles or namespaces. Each matching card, on **any** +style), kept current by the §12.2 pulse (`newChallenges`) — a bare integer, no titles +or namespaces. Each matching card, on **any** tab and in either view, carries a small **"new" corner tag**, so the tagged cards are exactly the ones the count refers to. The marker **advances when the user leaves the Challenges surface** — `/challenges` and its `/challenges/:number` detail pages share it — via @@ -4562,8 +4651,8 @@ namespace-admin function, §7.2). `POST /api/admin/triage/seen`, stamping `triage_seen_at = now()` for the actor; the count then reads 0 on both nav items until a **new** actionable item arrives. Merely opening the queue clears it — acting on the items is not required. -- **Freshness.** Polled on the same 30-second cadence as the notification bell - (§12.2) via `GET /api/admin/triage/attention → { count }`. +- **Freshness.** Carried by the §12.2 pulse (`attention`, `null` for non-admins), so it + follows the pulse cadence and pauses with it while the tab is hidden. - **Safety.** The value is a bare integer — no title, author, or namespace leaks — and obeys the same namespace scoping and visibility filtering as the queue (invariant 2). @@ -4579,8 +4668,8 @@ non-admin surface for presence anywhere in the app (§13.8). #### What counts as activity - **User-initiated requests only.** Page navigations and mutations stamp - `users.last_seen_at`; **background pollers never do** — the notification bell (§12.2) - and the triage attention badge (§14.4) both poll every 30 s, so counting them would + `users.last_seen_at`; **background pollers never do** — the §12.2 pulse, which feeds + the bell, the attention badge (§14.4) and the banner, polls every 30 s, so counting it would make "online" mean "has a tab open somewhere" and every user who ever left InnoBox open would read as permanently active. Reading counts as using: a challenge detail `GET` is activity, `GET /api/notifications` is not. @@ -4827,10 +4916,9 @@ structural rather than a filter. - **Audience & scope:** every authenticated user, org-wide. No namespace scoping (namespace admins have no authority here), **no per-user dismiss**, and the unauthenticated Home (§13.2) never shows it. -- **Delivery:** folded into the existing 30-second bell poll — the unread-count - response gains `banner: { message, tone, url, expiresAt } | null` (§12.2) — so an - open tab picks up a new, replaced or cleared banner within one poll and no second - transport exists. +- **Delivery:** carried by the §12.2 pulse — `banner: { message, tone, url, expiresAt } + | null` — so a visible tab picks up a new, replaced or cleared banner within one pulse, + a hidden tab on its return, and no second transport exists. - **Rendering:** desktop — a width-capped pill that truncates on one line with an ellipsis (full text via `title`) and **never** grows under the bell/theme controls; mobile (the §2.2 narrow topbar) — its own full-width line below the search row, @@ -5421,8 +5509,8 @@ bulk download of identities is never silent. REST under `/api`, session-authenticated, JSON, UTC ISO timestamps. Resource groups: - `challenges` (list/search/detail/create/edit/withdraw/transition/assign/visibility/ - **delete**; `challenges/similar` — the §6.1 duplicate check; `challenges/new-count` - — the §13.1 bare integer; `challenges/:number/featured` `PUT`/`DELETE` — platform + **delete**; `challenges/similar` — the §6.1 duplicate check (the §13.1 new count + rides on `me/pulse`); `challenges/:number/featured` `PUT`/`DELETE` — platform admin, §13.2, 409 at the cap or on an ineligible status; the detail payload carries `featured`/`canFeature`; `challenges/:number/visibility` `POST { visibility: "org" | "namespace" | "people", access?: { userIds, emails } }` — namespace/platform admin, @@ -5446,7 +5534,9 @@ REST under `/api`, session-authenticated, JSON, UTC ISO timestamps. Resource gro `hype`, `hyped`, `canHype`, `hypeReason` and `hypers`, the detail payload and `me` the viewer's `hypesLeft`; `challenges` accepts `sort=most_hyped`, §26.11) - `attachments` (single-shot upload — bound or **staged via `draftKey`**; **chunked upload** via `attachments/uploads` — initiate → parts → complete/abort — for files over the chunk size; list own staged by `draftKey`; gateway download; `challenges`/`solutions` create accept a `draftKey` to bind staged files, **gated on a clean scan when a scanner is available**) -- `notifications` (inbox, mark read; the unread poll carries the §14.6 banner), `me` +- `notifications` (inbox list with its `unreadCount`, fetched when the bell opens; mark + read, mark all read), `me/pulse` (`GET` the §12.2 pulse — `unreadCount`, + `newChallenges`, `attention`, `banner`; the shell's only poll), `me` (`GET` identity, roles and flags, including the §13.9 `whatsNewSeenVersion` and `whatsNewSinceDate`; `PATCH` sets the §13.7 quick-start flag, which also stamps the §13.9 marker; `me/challenges-seen` advances the §13.1 marker; `me/whats-new-seen` @@ -5467,7 +5557,7 @@ REST under `/api`, session-authenticated, JSON, UTC ISO timestamps. Resource gro - `leaderboards` (incl. `metric=points` and each row's level, §23.12), `dashboard` (KPIs, spotlights, **featured** — visibility-filtered pins, §13.2 — and **`hyped`**, the per-viewer "Hyped this week" board with the viewer's budget, `null` while hype is off, §26.5) -- `admin/*` (queue incl. proposed-solutions tab, bulk, export, settings, namespaces, role-mappings, audit; **triage attention count + mark-seen**, §14.4; +- `admin/*` (queue incl. proposed-solutions tab, bulk, export, settings, namespaces, role-mappings, audit; **triage mark-seen**, §14.4 — the attention count rides on `me/pulse`; **`admin/presence?window=5m|1h|8h|24h|30d` → `{ asOf, dau, wau, mau, total, users[] }` and `admin/presence/history?range=7d|30d|90d|all` → `{ range, enoughHistory, points[{ day, activeUsers, wau, mau }] }`** (one point per UTC day, contiguous; diff --git a/db/migrations/0051_notification_item_columns.sql b/db/migrations/0051_notification_item_columns.sql new file mode 100644 index 0000000..93df3b5 --- /dev/null +++ b/db/migrations/0051_notification_item_columns.sql @@ -0,0 +1,110 @@ +-- 0051_notification_item_columns.sql — what an inbox row is about, as real columns +-- (INNOBOX_SPEC.md §5 `notifications`, §12.1 read-time gate, §10.4 trash, v1.18). +-- +-- notifications gains challenge_id and solution_id (nullable uuid). A challenge row sets +-- challenge_id; a solution row sets solution_id AND its parent's challenge_id; comment and hype +-- rows the same way from their parent; a bound attachment's scan failure from the attachment's +-- parent. Rows that name no item (achievements, staged scan failures, system.error, +-- assignments_transferred) leave both null. Lookup keys, not foreign keys: the §10.3 cascade +-- deletes the rows explicitly, as it always has. +-- +-- They are DERIVED, never supplied: the BEFORE INSERT / UPDATE OF payload trigger +-- notifications_item_ids computes both from the payload's target keys on every write (any +-- value a caller passes is overwritten), so no writer — web, worker, shared, a test fixture — +-- can forget them, and a coalescing upsert keeps them right. The derivation, in order: +-- 1. parentType 'solution' + parentId → that solution and its challenge; +-- 2. parentType 'challenge' + parentId → that challenge; +-- 3. challengeId (comment/hype rows) → that challenge; +-- 4. the deep link, for rows that carry nothing else: '/challenges/' → challenge n, +-- '…#SOL-' → solution m under it. +-- An id that does not resolve to an existing row leaves the column null. +-- +-- The read-time gates (§12.1 visibility, §10.4 trash) and the §10.3 cascade now join on these +-- columns by primary key instead of matching payload text against every challenge. +-- +-- Indexes: (user_id, challenge_id) WHERE read_at IS NULL serves "opening the challenge reads its +-- rows" (comment_posted + hyped) and replaces the two per-type payload-expression indexes from +-- 0026 and 0050; (challenge_id) and (solution_id) partials serve the cascade and trash lookups. +-- The coalescing UNIQUE indexes on the payload keys stay — they key the upsert, not a filter. +-- +-- Idempotent: IF NOT EXISTS columns/indexes, CREATE OR REPLACE function, DROP IF EXISTS + CREATE +-- trigger, and the backfill only touches rows whose columns are still both null. + +BEGIN; + +ALTER TABLE notifications ADD COLUMN IF NOT EXISTS challenge_id uuid; +ALTER TABLE notifications ADD COLUMN IF NOT EXISTS solution_id uuid; + +-- The derivation, shared by the trigger and the backfill. STABLE: reads challenges/solutions only. +-- OUT parameters are prefixed so they can never be confused with the challenge_id columns. +CREATE OR REPLACE FUNCTION notification_item_ids(p jsonb, OUT out_challenge_id uuid, OUT out_solution_id uuid) +LANGUAGE plpgsql STABLE AS $$ +DECLARE + uuid_re constant text := '^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$'; + ptype text := p->>'parentType'; + pid uuid; + cid uuid; + link text := p->>'link'; + ch_num text; + sol_num text; +BEGIN + IF p->>'parentId' ~ uuid_re THEN pid := (p->>'parentId')::uuid; END IF; + IF p->>'challengeId' ~ uuid_re THEN cid := (p->>'challengeId')::uuid; END IF; + + IF ptype = 'solution' AND pid IS NOT NULL THEN + SELECT s.id, s.challenge_id INTO out_solution_id, out_challenge_id FROM solutions s WHERE s.id = pid; + ELSIF ptype = 'challenge' AND pid IS NOT NULL THEN + SELECT c.id INTO out_challenge_id FROM challenges c WHERE c.id = pid; + END IF; + + IF out_challenge_id IS NULL AND cid IS NOT NULL THEN + SELECT c.id INTO out_challenge_id FROM challenges c WHERE c.id = cid; + END IF; + + IF out_challenge_id IS NULL AND link IS NOT NULL THEN + ch_num := substring(link from '^/challenges/([0-9]{1,18})(?:#|$)'); + IF ch_num IS NOT NULL THEN + SELECT c.id INTO out_challenge_id FROM challenges c WHERE c.number = ch_num::bigint; + sol_num := substring(link from '#SOL-([0-9]{1,18})$'); + IF out_challenge_id IS NOT NULL AND out_solution_id IS NULL AND sol_num IS NOT NULL THEN + SELECT s.id INTO out_solution_id FROM solutions s + WHERE s.number = sol_num::bigint AND s.challenge_id = out_challenge_id; + END IF; + END IF; + END IF; +END $$; + +CREATE OR REPLACE FUNCTION notifications_item_ids() RETURNS trigger +LANGUAGE plpgsql AS $$ +BEGIN + SELECT d.out_challenge_id, d.out_solution_id INTO NEW.challenge_id, NEW.solution_id + FROM notification_item_ids(NEW.payload) d; + RETURN NEW; +END $$; + +DROP TRIGGER IF EXISTS notifications_item_ids ON notifications; +CREATE TRIGGER notifications_item_ids + BEFORE INSERT OR UPDATE OF payload ON notifications + FOR EACH ROW EXECUTE FUNCTION notifications_item_ids(); + +-- Backfill once. Only the id columns are written, so the UPDATE OF payload trigger does not fire. +UPDATE notifications n + SET challenge_id = d.out_challenge_id, solution_id = d.out_solution_id + FROM notifications src + CROSS JOIN LATERAL notification_item_ids(src.payload) d + WHERE src.id = n.id + AND n.challenge_id IS NULL AND n.solution_id IS NULL + AND (d.out_challenge_id IS NOT NULL OR d.out_solution_id IS NOT NULL); + +CREATE INDEX IF NOT EXISTS notifications_user_challenge_unread_idx + ON notifications (user_id, challenge_id) WHERE read_at IS NULL; +CREATE INDEX IF NOT EXISTS notifications_challenge_idx + ON notifications (challenge_id) WHERE challenge_id IS NOT NULL; +CREATE INDEX IF NOT EXISTS notifications_solution_idx + ON notifications (solution_id) WHERE solution_id IS NOT NULL; + +-- Superseded by notifications_user_challenge_unread_idx (no query filters on payload->>'challengeId'). +DROP INDEX IF EXISTS notifications_comment_challenge_idx; +DROP INDEX IF EXISTS notifications_hyped_challenge_idx; + +COMMIT; diff --git a/docs/performance-audit-2026-10-21.md b/docs/performance-audit-2026-10-21.md new file mode 100644 index 0000000..824b8d4 --- /dev/null +++ b/docs/performance-audit-2026-10-21.md @@ -0,0 +1,341 @@ +# InnoBox performance audit — 2026-10-21 + +Read-only analysis of the web tier, database access layer, worker, and client bundle. +Every finding below was checked against the source; line numbers are as of commit `8be26ea`. +Nothing here is implemented — each item is a candidate change that goes through the +spec gate (CLAUDE.md) before any code lands. + +## How to read the priorities + +- **P1** — scales badly with data or users and is on a hot path (every request, every poll). +- **P2** — measurable waste on a common path; fix when touching the area. +- **P3** — small, cheap, or admin-only. + +The short version: the app does little per page server-side (nearly every page is a client +component), so the real cost is in **how many API calls a page makes × what each call costs**. +Three things dominate: the per-call auth queries, the three 30-second pollers per tab, and a +notifications query that cannot use an index. + +--- + +## P1 — fix first + +### 1. Notification inbox and unread count scan every challenge per notification row +`packages/web/src/app/api/notifications/store.ts:32-73` + +The visibility gate and the trash filter are correlated `not exists` subqueries over +`challenges` whose join predicate casts the indexed side: + +```sql +c.id::text = n.payload->>'challengeId' +s.id::text = n.payload->>'parentId' +``` + +The primary-key index cannot be used, so for each of the viewer's notification rows Postgres +scans all challenges. `getInbox` evaluates it twice (list and unread count), `markAllRead` a +third time. The bell polls this every 30 s per open tab, and nothing ever purges +`notifications` (only the item-delete cascade removes rows), so per-user row counts grow +without bound. Cost ≈ notifications-per-user × challenges, per tab, every 30 s. This is the +query most likely to become the dominant steady-state DB load. + +**Fix:** flip the casts so the index applies — +`c.id = (n.payload->>'challengeId')::uuid`, `s.id = (n.payload->>'parentId')::uuid`, +`tc.number = (…)::bigint` for the link form. Better: add a real `notifications.challenge_id` +column written at dispatch, backfilled from the payload, indexed `(user_id, challenge_id)`. +Get the unread count from the list query with `count(*) filter (where read_at is null) over()` +instead of a second full evaluation. Consider a retention purge for read rows older than N days. + +### 2. Three 30-second pollers per tab, each re-running full auth, never pausing when hidden +`packages/web/src/components/NotificationBell.tsx:36-47`, +`packages/web/src/components/AppShell.tsx:440-450, 465-476` + +`/api/notifications`, `/api/challenges/new-count`, and (admins) `/api/admin/triage/attention` +each poll on their own 30 s interval. There is no `visibilitychange` handling anywhere in +`src` — background tabs keep polling forever. Each call pays the 3–4 auth queries from +finding 3 before its own work: 9–12 auth queries + 3 payload queries per tab per 30 s. +A hundred open tabs is 400–600 requests per minute at idle. Each bell tick also sets a fresh +array/object into shell state, re-rendering the shell tree every 30 s. + +**Fix:** one shell-level poller against one endpoint returning +`{ unreadCount, newChallenges, attention, banner }`; fetch the bell list only when opened; +stop on `document.visibilityState === "hidden"` and refetch on return; back off to 2–5 min +after idle ticks; compare values before `setState` so unchanged ticks do not re-render. + +### 3. `getSessionUser` is 3–4 sequential round trips, repeated on every API call +`packages/web/src/lib/auth.ts:48-96, 123-171` + +Per call: users row by `external_id` → group/role-mapping grants → (bootstrap admin group, +when configured) → `challenge_access`. All awaited in sequence, no per-request memo, 132 call +sites. A `/challenges` page load runs it 4–5 times (`/api/me`, `/api/notifications`, +`/api/challenges`, `new-count`, `attention`), resolving the same roles each time. Roughly +40–50 % of all queries the app issues are this. + +**Fix (spec-neutral):** fold the three role queries into one statement (CTEs returning arrays, +or `left join lateral`), or at minimum run them with `Promise.all` after the user row. The +"roles from SCIM groups, never claims" invariant is untouched; this is the same data in fewer +hops. A short per-user role cache would remove most of the rest, but the spec currently rules +that out — a spec decision, not an implementation one. + +### 4. Global advisory lock in the audit-chain trigger serializes every write, and transactions do a lot of work after taking it +`db/migrations/0028_audit_chain.sql` (`audit_log_chain()`), +`packages/web/src/app/api/challenges/store.ts:1318-1395` and other write paths + +The trigger takes `pg_advisory_xact_lock(hashtextextended('innobox:audit_log_chain', 0))`, +held until COMMIT, so at most one audit-writing transaction is in flight platform-wide (web +and worker). That is by design for the hash chain. The problem is lock *scope*: write paths +call `appendAudit` early and keep going. `setSolutionStatus` writes its first audit row, then +runs awards, the sibling-close loop (one audit each), `writeLiveAwards` (per recipient: +advisory lock, points, daily-cap count, badge checks, achievement notification), webhook +enqueue, and finally `fetchSolutionRows` — 20–40 round trips under the global lock. +`createChallenge`, `toggleLike`, `giveHype`, `createComment` follow the same shape. +`bulkSetStatus` with `BULK_CONCURRENCY = 10` just queues ten connections on the lock. + +**Fix:** make `appendAudit` the last statement of each transaction and move the post-write +refetches (`CHALLENGE_SELECT`, `fetchSolutionRows`, attachment lists) outside it. The +structural fix (sequence-allocated `chain_seq`, hashing off the critical path) is a spec change. + +### 5. `/api/me` issues roughly 23 queries, twelve of them `platform_settings` point reads +`packages/web/src/app/api/me/route.ts:30-43`, `api/me/survey/store.ts:171-190` + +A 10-wide `Promise.all` (date format, AI display-name setting, open survey, self survey, bug +reports, gamification, new achievements, MCP settings, live connection, hype enabled) followed +by a serial hype-budget read. Several readers re-read the users row or the same settings keys. +On the default 10-connection pool, one `/api/me` monopolises the pool for its duration, and it +runs on every full page load. + +**Fix:** one `select key, value from platform_settings where key = any($1)` for all keys, +passed into the readers; widen the `getSessionUser` select to carry the survey and +achievement columns; move the hype budget into the parallel block and compute "now" in SQL. +Target ≤ 6 queries. + +### 6. Worker: outbox, digest, and scan sweeps can overlap and claim no rows +`packages/worker/src/index.ts:346-347, 364-365`, +`packages/worker/src/notifications/dispatch.ts:194` + +Webhook delivery and triage assist have a `running` guard; the e-mail outbox, digest, and +ClamAV sweeps do not, and the outbox SELECT neither locks nor marks rows. A sweep of 50 rows +with Graph at its 15 s timeout can run 12 minutes; every 30 s a new sweep re-selects the same +still-`pending` rows and sends them again. Zero cost at idle; at the first Graph slowdown, +duplicate e-mails scale with sweep time ÷ 30 s. The scan sweep similarly re-streams a large +object to clamd when a scan exceeds 15 s (the DB write is idempotent, the S3 egress and CPU +are not). + +**Fix:** add the `running` guard to all three; claim rows with +`update … set status='sending', locked_at=now() where id in (select … for update skip locked) returning *` +(needs a `locked_at` column and a reclaim rule for stale locks). + +--- + +## P2 — measurable waste on common paths + +### 7. Challenge detail: N+1 attachments query plus five serial steps +`api/challenges/store.ts:600-613`, `api/challenges/[number]/route.ts:24-39` + +`buildDetailWithAttachments` runs `listAttachmentsForParent` once per solution in a `for` +loop, then once for the challenge, then `listAccessEntries`. The route then awaits the +mark-read UPDATE, the hype switch, and `dbNow` in sequence. Total ≈ 12 + N round trips. +**Fix:** one attachments query with `parent_id = any($1::uuid[])` grouped in JS; run the +mark-read UPDATE, hype switch, and detail fetch in parallel; use `now()` in SQL. + +### 8. Challenge detail page: fetch waterfall, N+1 comment requests, full re-read per micro-action +`packages/web/src/app/challenges/[number]/page.tsx:170-191, 239-287, 675, 992`, +`components/CommentThread.tsx:81-91` + +The page is a client component that fetches the detail on mount; each `CommentThread` +(one for the challenge, one per solution) then fetches `/api/comments` on mount — 1 + N +requests that start only after the detail renders. Like, hype, and follow each trigger a full +detail refetch. The `[number]` effect has no abort, so a quick CH-1 → CH-2 navigation can +commit CH-1's late response. +**Fix:** include comments in the detail payload or batch them; have like/hype/follow return +new counts and patch state locally; abort on route change. Consider a server component for +the first paint. + +### 9. Attachment scan poll re-reads the whole challenge every 4 seconds +`components/AttachmentControl.tsx:72, 143-146, 179` + +In bound mode, `refresh` calls `boundOnChanged`, which on the detail page is a full +`GET /api/challenges/:n`. Fifteen full-detail reads per minute per pending file per open +control, for as long as ClamAV is slow or down. +**Fix:** poll a status-only endpoint (`/api/attachments/:id`) and patch the row; back off +4 → 8 → 16 s and stop after a few minutes. + +### 10. `CHALLENGE_SELECT` carries seven correlated subqueries per row and the full description into list views +`api/challenges/store.ts:311-335`, sort at `:879-890` + +`like_count`, `liked_by_viewer`, `followed_by_viewer`, `solution_count`, `is_new` (a +per-row users lookup for the viewer's own `challenges_seen_at`), `featured_by_name`, +`trashed_by_name`, plus `c.description` and contact columns, for the gallery (limit 100), +search (with OFFSET), and featured. With a count-based sort the subquery is the ORDER BY key, +so it is evaluated for every visible challenge before the top-100 sort; `most_hyped` adds a +`sum(weight)` subquery per row and then `withHypePoints` re-queries hype for the same ids. +**Fix:** a `CHALLENGE_LIST_SELECT` without body/contact/trash columns; pass +`challenges_seen_at` as a parameter from the session user; fold hype into the select when on; +longer term, maintained `like_count` / `solution_count` columns updated in the transactions +that already audit those changes (then the sorts are indexable). + +### 11. Triage queue evaluates the hype subquery for every matching row before paging +`api/admin/triage/store.ts:156-169, 200-207, 456-471` + +`hype_points` sits in the inner select; with `count(*) over()` and OFFSET, it runs for all +challenges in the admin's namespaces per page, hype on or off. +**Fix:** attach hype only to the returned page; skip when hype is off; add +`challenges (namespace_id, created_at desc) where trashed_at is null`. + +### 12. Worker: one Graph token transaction and one SMTP connection per e-mail +`worker/src/notifications/dispatch.ts:94-101, 149`, `shared/src/email-graph.ts:95-110` + +`ensureFreshAccessToken` per row = connect, BEGIN, `select … for update`, COMMIT; a fresh +nodemailer transport per message. At 50 rows per tick that is ~250 DB round trips and 50 TLS +handshakes per sweep, serial. +**Fix:** resolve the token once per sweep (refresh only on 401), one pooled transport per +sweep, bounded concurrency (~5) keeping the 429 early exit. + +### 13. Worker: digest sweep is O(users × held rows) serial queries +`worker/src/notifications/digest.ts:363, 440-513` + +Per due recipient: held rows, user row, role resolution (4 queries including a constant +global-namespace lookup), then one `isVisibleTo` query **per held row**, then updates and a +Graph send with its own token transaction. 500 daily-digest users × 10 rows ≈ 9,000 queries +plus 500 serial sends at 06:00 UTC, with the overlap bug from finding 6 on top. +**Fix:** one query for all due users' rows, bulk role resolution (`resolveRoles.many` already +exists on the web side), parent visibility by `id = any($1)`, concurrency 3–5. + +### 14. SCIM group membership writes are one round trip per member +`worker/src/scim/router.ts:417-427, 450-459, 735-736` + +`addMembersTolerantly` does `userExists` + `addGroupMember` per id; removal is one DELETE per +member; neither is in a transaction, so a mid-way failure leaves a half-applied roster while +Entra retries the whole request. A PUT of a 500-member group ≈ 1,000 sequential round trips. +`GET /Groups` fans out one `listGroupMemberIds` per group in an unbounded `Promise.all` +(up to 200 → bursts past the 10-connection pool). +**Fix:** set-based `insert … select from users where id = any($2) … on conflict do nothing +returning user_id` (unknown = input minus returned), `delete … where user_id = any($2)`, +wrapped in `atomically`; one `select group_id, user_id … where group_id = any($1)` for lists. + +### 15. Entra reconciliation: two serial Graph calls per active user, hourly, no overlap guard +`worker/src/recon/reconcile.ts:103, 174, 290`, `index.ts:385` + +`getUser` + photo metadata per user, sequential, 15 s timeout each, no 429 backoff; a +per-oid `select` per membership add. 2,000 users ≈ 4,000 serial Graph calls ≈ 10+ min/hour, +and a pass longer than an hour overlaps the next. +**Fix:** `running` guard; `/users/delta` or `$batch`; bounded concurrency for photo checks; +load local users once into a Map. + +### 16. Chunked upload is strictly serial +`lib/chunked-upload.ts:138-143`, `components/AttachmentControl.tsx:71` + +One 5 MB part at a time: a 200 MB file is 40 sequential PUTs, each paying connection ramp-up +on a high-RTT VPN. The server already accepts parts independently. +**Fix:** 2–3 parts in flight with ordered progress. + +### 17. Webhook delivery: serial with 10 s timeout per row → head-of-line blocking +`worker/src/webhooks/deliver.ts:127-210`, `shared/src/webhook-send.ts:204` + +One dead receiver with 50 queued rows holds the sweep for 8+ minutes; every other webhook +waits. `keepAlive: false` means a fresh DNS + TLS per row. +**Fix:** group the batch by `webhook_id`, run webhooks concurrently (limit ~5), rows serial +within a webhook; after a timeout/5xx, skip that webhook's remaining rows this sweep. + +### 18. Pool sizing and timeouts +`packages/web/src/lib/db.ts:15`, `packages/worker/src/index.ts:100` + +Both pools use the pg defaults: `max` 10, no `statement_timeout`, no +`idle_in_transaction_session_timeout`. Finding 5's 10-wide burst plus a few concurrent API +calls already queues on the web pool; symptoms are random latency, not errors. The worker +pool serves SCIM HTTP and every sweep, so a SCIM group-list burst can stall `/readyz`'s +`select 1`. Postgres itself runs with image defaults (no `shared_buffers` or +`work_mem` tuning in `deploy/docker-compose.yml`). +**Fix:** set `max` explicitly (20–30 web, 10–15 worker), a `statement_timeout` (e.g. 30 s) +and `idle_in_transaction_session_timeout` on the app role, and a basic Postgres config for the +host's RAM. Reduce the bursts via findings 3 and 5. + +--- + +## P3 — small, cheap, or admin-only + +19. **Shell cluster on every route** — `layout.tsx:81-85`, `AppShell.tsx` (1014 lines): + ≈546 KB uncompressed JS before any page code (react-dom + Next runtime 433 KB, next-auth + client 40 KB, shell 35 KB + 38 KB). Survey, bug-report, What's-new, and achievement + machinery could be a lazily imported `ShellExtras`. `SessionProvider` refetches + `/api/auth/session` on every window focus (`refetchOnWindowFocus` default) while the app + already polls; `DateFormat.tsx:22-33` fires `/api/me` before the session resolves (a 401 + on the signed-out home). +20. **Unused dependencies** — the five `@tiptap/*` packages in `packages/web/package.json` + have zero imports in `src` (13 MB in the install, nothing in the bundle). JetBrains Mono + is loaded in `layout.tsx` but `--font-mono` is used only by dead `.dropzone-hint` / + `.code-cmd` rules. +21. **Dead CSS from the starter kit** — `globals.css` (123 KB source, 79 KB built, every + route): ~99 class names never referenced (`skill-row*`, `rating-hist*`, `install-*`, + `md-*`, `dropzone-*`, `filepick-*`, `taginput-*`, `facet*`, `metric-strip`, …). This is + the Phase 4 pruning CLAUDE.md mentions, still pending. ~3 KB gzipped. +22. **`listComments` is 4 serial round trips** — `api/comments/store.ts:105-110`: + `getParentAccess` already reads `namespace_id`; `getParentNamespaceId` re-queries it; + `committeeMemberIds` can run alongside the list. +23. **Dashboard runs its two KPI aggregates serially before its parallel block** — + `api/dashboard/store.ts:76-93, 144-150`. Four stages → two. +24. **`markCommentNotificationsReadForChallenge` matches no partial index** — + `lib/notify.ts:353-359`: `type in ('comment_posted','hyped')` implies neither of the + single-type partial indexes from 0026/0050, so each challenge open filters all the user's + rows. One partial index on `(user_id, (payload->>'challengeId')) where read_at is null and + type in (…)`. +25. **Index gaps and one redundant index** — no `challenges (created_at desc) where + trashed_at is null` for the gallery's default order; `solutions (challenge_id, + created_at)` would replace `solutions_challenge_idx` for the always-ordered + `fetchSolutionRows`; `attachments_scan_status_idx` (0012) is a strict prefix of + `attachments_scan_status_next_idx` (0021) and can be dropped. +26. **Audit browser search** — `api/admin/audit/store.ts:75-86, 122`: ILIKE over an + expression containing correlated subqueries with `c.id::text = a.target_id`; each + candidate audit row scans `challenges`. Cast the other side inside the type-guarded CASE + branches; pre-parse `CH-12` / `SOL-34` queries into id lookups. +27. **Write paths load every sibling solution to return one** — + `api/challenges/store.ts:1099, 1390, 2006`: `fetchSolutionRows(...).find(...)` inside the + transaction (under the audit lock). A single-row variant. +28. **Coalesced comment/hype dispatch is two queries per recipient, serial** — + `lib/notify.ts:308-347, 406-440`. One multi-row upsert with `returning (xmax = 0)` and + one multi-row outbox insert, as `writeNotifications` already does. +29. **Leaderboard cold cache** — `lib/gamification-read.ts:101-110`: ten `(metric, window)` + boards computed sequentially on a cold 30 s cache; `likes_received` scans all `likes` with + no `likes (created_at)` index for the 30-day window. `Promise.all` + the index. +30. **Admin console** — `admin/page.tsx:77-250`: eight cards each fetching on mount; + `CurrentlyOnline.tsx:103, 112` fetches both the window and the full history regardless of + the selected range. One overview endpoint; history for the visible range only. +31. **`withSystemLog` amplifies 4xx cost** — `lib/system-log-core.ts:95-150`: on recorded + error statuses it clones and parses the body, re-reads the session, and queries the actor + and anonymity target. Pass the already-resolved user; skip the anonymity check on 401/403. +32. **Triage assist throughput** — `worker/src/ai/triage.ts:295`: batch of 3, serial, per + minute → ≤180 challenges/hour; enabling it on a 1,000-item backlog takes ~6 h. + `api/ai/similar.ts:54` has no content-hash cache, unlike summary and triage. +33. **ClamAV scan wall-clock cap** — `worker/src/attachments/scan.ts`: the 180 s socket + timeout is an idle timeout; a clamd that keeps reading but never answers can hold the + sweep indefinitely. Streaming itself is correct (64 KB frames, backpressure, no buffering). +34. **Minor client** — `TopbarSearch.tsx:57-58` and `challenges/page.tsx:79-95` lack + `AbortController` (out-of-order responses on fast typing / filter changes); the `✕` + glyph in the shell pulls Open Sans' 26 KB symbols subset (use an SVG). + +--- + +## Verified clean (no action) + +- ECharts is `echarts/core` with tree-shaken modules, loaded via dynamic import only on + `/admin` (591 KB chunk behind a 1 KB loader). +- `@aws-sdk/client-s3` is server-only; `@modelcontextprotocol/sdk` is test-only. +- Avatar photos ship `Cache-Control: private, max-age=3600` with ETag/304. +- `cachedGet` dedupes `/api/me` across the shell, date provider, and pages. +- Triage, audit, and system-log lists are paginated; the gallery is capped at 100. +- Middleware does no DB work (JWT verify + nonce only). +- Rate limiter is a bounded LRU (50,000 keys); presence throttle map self-sweeps. +- Search uses the GIN tsvector indexes with the visibility predicate inside the candidate limit. +- Notification fan-out (`writeNotifications`) is batched; recipient roles resolve in bulk. +- ClamAV streaming, MinIO puts, and the ClamAV probe run outside transactions. +- Presence rollup is incremental and bounded to a 3-day window. +- Webhook retry schedule honours `Retry-After`; no retry storms. + +## Suggested order of work + +1. Findings 1 + 2 + 3 together (one spec change: "pulse" endpoint, visibility-gated polling, + indexable notification predicates, single-statement role resolution). This removes most of + the steady-state load. +2. Finding 6 (worker sweep guards + row claims) — a correctness issue as much as performance. +3. Findings 4, 5, 7, 10 (audit-lock scope, `/api/me`, detail N+1, list select). +4. Findings 12–15 (worker throughput) when e-mail volume, digests, or SCIM group sizes grow. +5. P3 items opportunistically, in the commits that touch those files. diff --git a/packages/shared/src/item-delete.ts b/packages/shared/src/item-delete.ts index 5098c93..5dde1d1 100644 --- a/packages/shared/src/item-delete.ts +++ b/packages/shared/src/item-delete.ts @@ -96,7 +96,8 @@ export interface Subtree { } /** - * The notification rows that target a subtree. A row targets it when EITHER + * The OUTBOX rows that target a subtree (inbox rows match on their challenge_id / solution_id + * columns instead — §5, migration 0051). A row targets it when EITHER * - its payload names an item of the subtree as its target, `parentType`/`parentId` — every * notification about a challenge or solution carries one (web lib/notify.ts `withTarget`, the * scan-failure writer) — or, for a comment row (event 6), names a challenge of the subtree as @@ -203,7 +204,13 @@ async function deleteChildren( // §12.1: no inbox item may survive pointing at an entity that no longer exists; unsent e-mail // about it never goes out. - const inbox = await client.query(`delete from notifications where ${SUBTREE_NOTIFICATION_MATCH}`, subtreeNotificationParams(subtree)); + // Inbox rows carry what they are about as columns (§5, migration 0051 — a solution row carries its + // challenge too), so the inbox delete is two index lookups; the outbox has no such columns and + // keeps the payload/link match. + const inbox = await client.query( + `delete from notifications where challenge_id = any($1::uuid[]) or solution_id = any($2::uuid[])`, + [subtree.challengeIds, subtree.solutionIds], + ); const outbox = await client.query( `delete from notification_outbox where ${SUBTREE_NOTIFICATION_MATCH} and status <> 'sent'`, subtreeNotificationParams(subtree), diff --git a/packages/shared/src/version.ts b/packages/shared/src/version.ts index 7157efe..7009e5f 100644 --- a/packages/shared/src/version.ts +++ b/packages/shared/src/version.ts @@ -5,4 +5,4 @@ // patch — fixes, styling, copy, small tweaks // minor — new features / behaviors / endpoints / migrations // major — breaking changes (API shapes, required config) -export const APP_VERSION = "1.17.2"; +export const APP_VERSION = "1.18.0"; diff --git a/packages/web/package.json b/packages/web/package.json index 569f761..20c4375 100644 --- a/packages/web/package.json +++ b/packages/web/package.json @@ -10,9 +10,9 @@ "start": "node .next/standalone/server.js", "typecheck": "next typegen && tsc --noEmit", "lint": "eslint .", - "test": "node --import tsx --test src/lib/ui.test.ts src/lib/attribution.test.ts src/lib/spec-refs.test.ts src/lib/date-format-usage.test.ts src/lib/audit.test.ts src/lib/users.test.ts src/lib/routeAccess.test.ts src/lib/presence.test.ts src/lib/presence-touch.test.ts src/lib/charts/presence-chart.test.ts src/lib/charts/survey-trend-chart.test.ts src/app/api/admin/validation.test.ts src/app/api/challenges/validation.test.ts src/app/api/comments/validation.test.ts src/app/api/admin/triage/validation.test.ts src/app/api/admin/settings/validation.test.ts src/app/api/admin/settings/bootstrap-admin.test.ts src/app/api/admin/email/validation.test.ts src/components/breadcrumb-crumbs.test.ts src/components/toggle-switch.test.ts src/lib/http.test.ts src/lib/rate-limit.test.ts src/lib/csrf.test.ts src/lib/security-headers.test.ts src/lib/dev-auth-guard.test.ts src/lib/entra-tenant.test.ts src/lib/system-log.test.ts src/lib/challenges-surface.test.ts src/lib/form-lock.test.ts src/lib/popovers.test.ts src/lib/auth-cookie-sweep.test.ts src/lib/signin-relink.test.ts src/lib/client-ip.test.ts src/lib/csp-mode.test.ts src/lib/csp-report.test.ts src/app/api/users/erasure-validation.test.ts src/app/api/admin/audit/csv.test.ts src/lib/challenges-view.test.ts src/app/api/admin/webhooks/routes.test.ts src/app/api/challenges/featured-rules.test.ts src/lib/auth-pages.test.ts src/lib/photo-type.test.ts src/lib/search-rate-limit.test.ts src/lib/chunked-upload.test.ts src/lib/attachment-control.test.ts src/lib/attachment-intake.test.ts src/lib/challenge-detail.test.ts src/lib/deep-link.test.ts src/app/api/challenges/delete-routes.test.ts src/app/api/follows/routes.test.ts src/lib/clamav.test.ts src/lib/email.test.ts src/lib/avatar-palette.test.ts src/lib/quick-start.test.ts src/lib/directory-card.test.ts src/app/whats-new/upcoming.test.ts src/lib/whats-new.test.ts src/app/api/me/whats-new-seen/routes.test.ts src/app/api/me/achievements-seen/routes.test.ts src/app/api/admin/ai/routes.test.ts src/app/api/admin/ai/validation.test.ts src/app/api/ai/draft/validation.test.ts src/lib/survey-client.test.ts src/app/api/public/handlers.test.ts src/app/api/public-submissions/link/routes.test.ts src/lib/public-intake.test.ts src/lib/pow-solver.test.ts src/lib/mcp/oauth.test.ts src/lib/mcp/consent-page.test.ts src/lib/mcp/protocol.test.ts src/lib/mcp/conformance.test.ts src/lib/session-revocation.test.ts src/app/api/me/sign-out-everywhere/routes.test.ts src/app/api/admin/users/[userId]/scrub/routes.test.ts scripts/e2e-specs.test.mjs", + "test": "node --import tsx --test src/lib/ui.test.ts src/lib/attribution.test.ts src/lib/spec-refs.test.ts src/lib/date-format-usage.test.ts src/lib/audit.test.ts src/lib/users.test.ts src/lib/routeAccess.test.ts src/lib/presence.test.ts src/lib/presence-touch.test.ts src/lib/pulse.test.ts src/lib/request-memo.test.ts src/lib/charts/presence-chart.test.ts src/lib/charts/survey-trend-chart.test.ts src/app/api/admin/validation.test.ts src/app/api/challenges/validation.test.ts src/app/api/comments/validation.test.ts src/app/api/admin/triage/validation.test.ts src/app/api/admin/settings/validation.test.ts src/app/api/admin/settings/bootstrap-admin.test.ts src/app/api/admin/email/validation.test.ts src/components/breadcrumb-crumbs.test.ts src/components/toggle-switch.test.ts src/lib/http.test.ts src/lib/rate-limit.test.ts src/lib/csrf.test.ts src/lib/security-headers.test.ts src/lib/dev-auth-guard.test.ts src/lib/entra-tenant.test.ts src/lib/system-log.test.ts src/lib/challenges-surface.test.ts src/lib/form-lock.test.ts src/lib/popovers.test.ts src/lib/auth-cookie-sweep.test.ts src/lib/signin-relink.test.ts src/lib/client-ip.test.ts src/lib/csp-mode.test.ts src/lib/csp-report.test.ts src/app/api/users/erasure-validation.test.ts src/app/api/admin/audit/csv.test.ts src/lib/challenges-view.test.ts src/app/api/admin/webhooks/routes.test.ts src/app/api/challenges/featured-rules.test.ts src/lib/auth-pages.test.ts src/lib/photo-type.test.ts src/lib/search-rate-limit.test.ts src/lib/chunked-upload.test.ts src/lib/attachment-control.test.ts src/lib/attachment-intake.test.ts src/lib/challenge-detail.test.ts src/lib/deep-link.test.ts src/app/api/challenges/delete-routes.test.ts src/app/api/follows/routes.test.ts src/lib/clamav.test.ts src/lib/email.test.ts src/lib/avatar-palette.test.ts src/lib/quick-start.test.ts src/lib/directory-card.test.ts src/app/whats-new/upcoming.test.ts src/lib/whats-new.test.ts src/app/api/me/whats-new-seen/routes.test.ts src/app/api/me/achievements-seen/routes.test.ts src/app/api/admin/ai/routes.test.ts src/app/api/admin/ai/validation.test.ts src/app/api/ai/draft/validation.test.ts src/lib/survey-client.test.ts src/app/api/public/handlers.test.ts src/app/api/public-submissions/link/routes.test.ts src/lib/public-intake.test.ts src/lib/pow-solver.test.ts src/lib/mcp/oauth.test.ts src/lib/mcp/consent-page.test.ts src/lib/mcp/protocol.test.ts src/lib/mcp/conformance.test.ts src/lib/session-revocation.test.ts src/app/api/me/sign-out-everywhere/routes.test.ts src/app/api/admin/users/[userId]/scrub/routes.test.ts scripts/e2e-specs.test.mjs", "test:db": "node scripts/test-db-isolated.mjs", - "test:db:run": "node --import tsx --test --test-concurrency=1 src/lib/audit.dbtest.ts src/app/api/admin/store.dbtest.ts src/app/api/admin/settings/bootstrap-admin.dbtest.ts src/app/api/challenges/store.dbtest.ts src/app/api/challenges/delete.dbtest.ts src/app/api/challenges/trash.dbtest.ts src/app/api/comments/store.dbtest.ts src/app/api/admin/triage/store.dbtest.ts src/app/api/admin/presence/store.dbtest.ts src/app/api/users/store.dbtest.ts src/app/api/users/directory-search.dbtest.ts src/app/api/notifications/store.dbtest.ts src/app/api/notifications/inbox-visibility.dbtest.ts src/app/api/attachments/store.dbtest.ts src/app/api/me/store.dbtest.ts src/app/api/me/whats-new.dbtest.ts src/app/api/profile/store.dbtest.ts src/app/api/admin/system-log/store.dbtest.ts src/app/api/admin/audit/store.dbtest.ts src/app/api/admin/system-banner/store.dbtest.ts src/app/api/challenges/new-markers.dbtest.ts src/app/api/challenges/similar.dbtest.ts src/lib/notify-preferences.dbtest.ts src/lib/notify-digest.dbtest.ts src/lib/notify-coalesce.dbtest.ts src/lib/signin-relink.dbtest.ts src/app/api/users/erasure-reassignment.dbtest.ts src/app/api/admin/identity-sync/store.dbtest.ts src/app/api/admin/audit/verify/verify.dbtest.ts src/app/api/admin/webhooks/store.dbtest.ts src/lib/webhooks.dbtest.ts src/app/api/challenges/featured.dbtest.ts src/app/api/challenges/likes-frozen.dbtest.ts src/app/api/challenges/namespace-archived.dbtest.ts src/lib/notify-events.dbtest.ts src/lib/notify-fanout.dbtest.ts src/lib/notify-inactive.dbtest.ts src/app/api/solutions/store.dbtest.ts src/app/api/search/store.dbtest.ts src/app/api/challenges/visibility-sql.dbtest.ts src/app/api/challenges/people-visibility.dbtest.ts src/app/api/dashboard/store.dbtest.ts src/app/api/leaderboards/store.dbtest.ts src/app/api/admin/ai/store.dbtest.ts src/app/api/ai/draft/store.dbtest.ts src/app/api/ai/triage/store.dbtest.ts src/app/api/ai/similar.dbtest.ts src/app/api/ai/summary/store.dbtest.ts src/app/api/me/survey/store.dbtest.ts src/app/api/admin/survey/store.dbtest.ts src/lib/gamification.dbtest.ts src/app/api/hypes/store.dbtest.ts src/app/api/public/store.dbtest.ts src/app/api/public/visibility-request.dbtest.ts src/lib/mcp/store.dbtest.ts src/lib/mcp/tools.dbtest.ts src/app/api/users/scrub-refusals.dbtest.ts src/app/api/me/sign-out-everywhere/sign-out-everywhere.dbtest.ts", + "test:db:run": "node --import tsx --test --test-concurrency=1 src/lib/audit.dbtest.ts src/app/api/admin/store.dbtest.ts src/app/api/admin/settings/bootstrap-admin.dbtest.ts src/app/api/challenges/store.dbtest.ts src/app/api/challenges/delete.dbtest.ts src/app/api/challenges/trash.dbtest.ts src/app/api/comments/store.dbtest.ts src/app/api/admin/triage/store.dbtest.ts src/app/api/admin/presence/store.dbtest.ts src/app/api/users/store.dbtest.ts src/app/api/users/directory-search.dbtest.ts src/app/api/notifications/store.dbtest.ts src/app/api/notifications/inbox-visibility.dbtest.ts src/app/api/notifications/item-columns.dbtest.ts src/app/api/me/pulse/store.dbtest.ts src/lib/role-resolution.dbtest.ts src/app/api/attachments/store.dbtest.ts src/app/api/me/store.dbtest.ts src/app/api/me/whats-new.dbtest.ts src/app/api/profile/store.dbtest.ts src/app/api/admin/system-log/store.dbtest.ts src/app/api/admin/audit/store.dbtest.ts src/app/api/admin/system-banner/store.dbtest.ts src/app/api/challenges/new-markers.dbtest.ts src/app/api/challenges/similar.dbtest.ts src/lib/notify-preferences.dbtest.ts src/lib/notify-digest.dbtest.ts src/lib/notify-coalesce.dbtest.ts src/lib/signin-relink.dbtest.ts src/app/api/users/erasure-reassignment.dbtest.ts src/app/api/admin/identity-sync/store.dbtest.ts src/app/api/admin/audit/verify/verify.dbtest.ts src/app/api/admin/webhooks/store.dbtest.ts src/lib/webhooks.dbtest.ts src/app/api/challenges/featured.dbtest.ts src/app/api/challenges/likes-frozen.dbtest.ts src/app/api/challenges/namespace-archived.dbtest.ts src/lib/notify-events.dbtest.ts src/lib/notify-fanout.dbtest.ts src/lib/notify-inactive.dbtest.ts src/app/api/solutions/store.dbtest.ts src/app/api/search/store.dbtest.ts src/app/api/challenges/visibility-sql.dbtest.ts src/app/api/challenges/people-visibility.dbtest.ts src/app/api/dashboard/store.dbtest.ts src/app/api/leaderboards/store.dbtest.ts src/app/api/admin/ai/store.dbtest.ts src/app/api/ai/draft/store.dbtest.ts src/app/api/ai/triage/store.dbtest.ts src/app/api/ai/similar.dbtest.ts src/app/api/ai/summary/store.dbtest.ts src/app/api/me/survey/store.dbtest.ts src/app/api/admin/survey/store.dbtest.ts src/lib/gamification.dbtest.ts src/app/api/hypes/store.dbtest.ts src/app/api/public/store.dbtest.ts src/app/api/public/visibility-request.dbtest.ts src/lib/mcp/store.dbtest.ts src/lib/mcp/tools.dbtest.ts src/app/api/users/scrub-refusals.dbtest.ts src/app/api/me/sign-out-everywhere/sign-out-everywhere.dbtest.ts", "e2e": "playwright test -c e2e/playwright.config.ts" }, "dependencies": { diff --git a/packages/web/src/app/api/admin/triage/attention/route.ts b/packages/web/src/app/api/admin/triage/attention/route.ts deleted file mode 100644 index 57ad82e..0000000 --- a/packages/web/src/app/api/admin/triage/attention/route.ts +++ /dev/null @@ -1,26 +0,0 @@ -// GET /api/admin/triage/attention (INNOBOX_SPEC.md §14.4): the "unseen actionable items" count -// behind the Triage/Administration nav bubble. Admin-only (namespace admins → their namespaces, -// platform admins → everywhere); polled by the app shell on the notification-bell cadence. -import { requireUser } from "@/lib/auth"; -import { pool } from "@/lib/db"; -import { adminNamespaceIds, countTriageAttention } from "../store"; -import { withSystemLog } from "@/lib/system-log"; - -export const dynamic = "force-dynamic"; - -async function handleGET(): Promise { - const gate = await requireUser(); - if (!gate.ok) return gate.response; - - const viewer = { userId: gate.user.id, roles: gate.user.roles }; - const adminNs = adminNamespaceIds(viewer.roles); - if (adminNs !== "all" && adminNs.length === 0) { - return Response.json({ error: "forbidden" }, { status: 403 }); - } - - const count = await countTriageAttention(pool, viewer); - return Response.json({ count }); -} - -// §14.7: every handler is wrapped so refused requests and failures are recorded in the system log. -export const GET = withSystemLog("/api/admin/triage/attention", handleGET); diff --git a/packages/web/src/app/api/challenges/new-count/route.ts b/packages/web/src/app/api/challenges/new-count/route.ts deleted file mode 100644 index 9a95e1c..0000000 --- a/packages/web/src/app/api/challenges/new-count/route.ts +++ /dev/null @@ -1,19 +0,0 @@ -// GET /api/challenges/new-count (INNOBOX_SPEC.md §13.1): how many challenges, visible to the -// caller, were created since they last left the Challenges surface. A bare integer — no titles, -// no namespaces — polled by the app shell on the bell's 30-second cadence for the nav bubble. -import { requireUser } from "@/lib/auth"; -import { pool } from "@/lib/db"; -import { withSystemLog } from "@/lib/system-log"; -import { countNewChallenges } from "../store"; - -export const dynamic = "force-dynamic"; - -async function handleGET(): Promise { - const gate = await requireUser(); - if (!gate.ok) return gate.response; - const count = await countNewChallenges(pool, { userId: gate.user.id, roles: gate.user.roles }); - return Response.json({ count }); -} - -// §14.7: every handler is wrapped so refused requests and failures are recorded in the system log. -export const GET = withSystemLog("/api/challenges/new-count", handleGET); diff --git a/packages/web/src/app/api/me/pulse/route.ts b/packages/web/src/app/api/me/pulse/route.ts new file mode 100644 index 0000000..c56a0be --- /dev/null +++ b/packages/web/src/app/api/me/pulse/route.ts @@ -0,0 +1,20 @@ +// GET /api/me/pulse (INNOBOX_SPEC.md §12.2, v1.18): the app shell's only poll — the bell's unread +// count, the §13.1 new-challenges count, the §14.4 triage attention count (null for non-admins) and +// the active §14.6 system banner, in one round trip. A background poller: it is not on the §14.5 +// presence allowlist, so it never stamps last_seen_at. +import { requireUser } from "@/lib/auth"; +import { pool } from "@/lib/db"; +import { withSystemLog } from "@/lib/system-log"; +import { readPulse } from "./store"; + +export const dynamic = "force-dynamic"; + +async function handleGET(): Promise { + const gate = await requireUser(); + if (!gate.ok) return gate.response; + const pulse = await readPulse(pool, { userId: gate.user.id, roles: gate.user.roles }); + return Response.json(pulse, { headers: { "Cache-Control": "no-store" } }); +} + +// §14.7: every handler is wrapped so refused requests and failures are recorded in the system log. +export const GET = withSystemLog("/api/me/pulse", handleGET); diff --git a/packages/web/src/app/api/me/pulse/store.dbtest.ts b/packages/web/src/app/api/me/pulse/store.dbtest.ts new file mode 100644 index 0000000..261bfa4 --- /dev/null +++ b/packages/web/src/app/api/me/pulse/store.dbtest.ts @@ -0,0 +1,102 @@ +// Live-DB integration test (gated) for the §12.2 pulse (INNOBOX_SPEC.md §12.2, §13.1, §14.4, §14.6, +// invariant 2; v1.18): +// - one read returns the bell's unread count, the new-challenges count, the triage attention +// count and the active banner — each equal to its surface's own count; +// - attention is null for a non-admin and a number for a namespace admin; +// - the counts obey visibility: a namespace-restricted challenge reaches neither count of a +// non-member; +// - the banner appears while active and is null once cleared. +// Self-skips when DATABASE_URL is unset. +import { test } from "node:test"; +import assert from "node:assert/strict"; + +const url = process.env.DATABASE_URL; + +test( + "§12.2 pulse: four bare values, scoped like their surfaces, attention only for admins", + { skip: url ? false : "DATABASE_URL not set — live-DB suite self-skips" }, + async () => { + const { Pool } = await import("pg"); + const { randomUUID } = await import("node:crypto"); + const { buildRoleSet } = await import("@innobox/shared"); + const { readPulse, hasTriageAttention } = await import("./store"); + const { countNewChallenges } = await import("../../challenges/store"); + const { countUnread } = await import("../../notifications/store"); + const { countTriageAttention } = await import("../../admin/triage/store"); + const { setSystemBanner, clearSystemBanner } = await import("../../admin/system-banner/store"); + + const pool = new Pool({ connectionString: url }); + try { + const stamp = randomUUID().slice(0, 8); + const { rows: g } = await pool.query<{ id: string }>(`select id from namespaces where slug = 'global'`); + const globalId = g[0]!.id; + const { rows: ns } = await pool.query<{ id: string }>(`insert into namespaces (slug, display_name) values ($1, 'Dbtest Pulse NS') returning id`, [ + `dbtest-pulse-${stamp}`, + ]); + const nsId = ns[0]!.id; + const { rows: ia } = await pool.query<{ id: string }>(`select id from impact_areas where active and name <> 'Client' limit 1`); + const mkUser = async (label: string) => { + const { rows } = await pool.query<{ id: string }>( + `insert into users (external_id, user_name, display_name, email) values ($1, $2, $3, $4) returning id`, + [`dbtest-pulse-${label}-${stamp}`, `dbtest-pulse-${label}-${stamp}@example.test`, `Dbtest Pulse ${label}`, `pulse-${label}-${stamp}@example.test`], + ); + return rows[0]!.id; + }; + const roles = (grants: { role: "member" | "namespace_admin"; namespaceId: string }[]) => buildRoleSet(grants, { globalNamespaceId: globalId }); + const authorId = await mkUser("author"); + const outsider = { userId: await mkUser("outsider"), roles: roles([]) }; + const nsAdmin = { userId: await mkUser("nsadmin"), roles: roles([{ role: "namespace_admin", namespaceId: nsId }]) }; + // Both viewers have "seen" the Challenges surface before the fixtures exist. + await pool.query(`update users set challenges_seen_at = now() - interval '1 minute' where id = any($1::uuid[])`, [[outsider.userId, nsAdmin.userId]]); + + assert.equal(hasTriageAttention(outsider), false); + assert.equal(hasTriageAttention(nsAdmin), true); + + // One namespace-restricted challenge awaiting triage, one org-visible valid one. + const mkChallenge = async (visibility: "org" | "namespace", status: string) => { + const { rows } = await pool.query<{ id: string; number: string }>( + `insert into challenges (namespace_id, visibility, title, description, impact_area_id, author_id, status) + values ($1, $2, $3, 'pulse fixture', $4, $5, $6) returning id, number::text`, + [nsId, visibility, `Pulse ${visibility} ${stamp}`, ia[0]!.id, authorId, status], + ); + return rows[0]!; + }; + const restricted = await mkChallenge("namespace", "awaiting_triage"); + const open = await mkChallenge("org", "valid"); + for (const v of [outsider, nsAdmin]) { + for (const c of [restricted, open]) { + await pool.query(`insert into notifications (user_id, type, payload) values ($1, 'status_changed', $2)`, [ + v.userId, + JSON.stringify({ message: `pulse ${stamp}`, link: `/challenges/${c.number}`, parentType: "challenge", parentId: c.id }), + ]); + } + } + + const p1 = await readPulse(pool, outsider); + assert.equal(p1.attention, null, "a non-admin gets null, and nothing is computed for them"); + assert.equal(p1.unreadCount, await countUnread(pool, outsider)); + assert.equal(p1.newChallenges, await countNewChallenges(pool, outsider)); + assert.ok(p1.unreadCount >= 1, "the row about the org challenge counts"); + + const p2 = await readPulse(pool, nsAdmin); + assert.equal(typeof p2.attention, "number"); + assert.equal(p2.attention, await countTriageAttention(pool, nsAdmin)); + assert.ok((p2.attention ?? 0) >= 1, "the awaiting_triage challenge is unseen for its namespace admin"); + assert.equal(p2.unreadCount, await countUnread(pool, nsAdmin)); + assert.equal(p2.newChallenges, await countNewChallenges(pool, nsAdmin)); + + // Invariant 2: the restricted challenge reaches neither of the outsider's counts. + assert.equal(p2.unreadCount - p1.unreadCount, 1, "exactly the restricted row separates them"); + assert.equal(p2.newChallenges - p1.newChallenges, 1, "…and exactly the restricted challenge"); + + // The banner rides along while active (a singleton — cleared again below). + const banner = await setSystemBanner(pool, { message: `Pulse banner ${stamp}`, tone: "info", url: null, duration: "1h" }, nsAdmin.userId); + const p3 = await readPulse(pool, outsider); + assert.deepEqual(p3.banner, banner); + await clearSystemBanner(pool, nsAdmin.userId); + assert.equal((await readPulse(pool, outsider)).banner, null); + } finally { + await pool.end(); + } + }, +); diff --git a/packages/web/src/app/api/me/pulse/store.ts b/packages/web/src/app/api/me/pulse/store.ts new file mode 100644 index 0000000..5ad4cc6 --- /dev/null +++ b/packages/web/src/app/api/me/pulse/store.ts @@ -0,0 +1,28 @@ +// Data layer for GET /api/me/pulse (INNOBOX_SPEC.md §12.2, v1.18): the four live values the app +// shell shows, read concurrently after one §4.2 resolution. Every value is a bare count or the +// banner — no titles, authors or namespaces — under the same visibility and namespace scoping as +// the surface it feeds (invariant 2). Relative imports only (no `@/`) so the gated .dbtest.ts suite +// runs under the plain node runner. +import type { Pool } from "pg"; +import type { Pulse } from "../../../../lib/pulse"; +import { countNewChallenges, type Viewer } from "../../challenges/store"; +import { countUnread } from "../../notifications/store"; +import { adminNamespaceIds, countTriageAttention } from "../../admin/triage/store"; +import { getActiveSystemBanner } from "../../admin/system-banner/store"; + +/** Whether the §14.4 attention bubble applies: a platform admin, or a namespace admin somewhere. */ +export function hasTriageAttention(viewer: Viewer): boolean { + const ns = adminNamespaceIds(viewer.roles); + return ns === "all" || ns.length > 0; +} + +export async function readPulse(pool: Pool, viewer: Viewer): Promise { + // Nothing about attention is computed for a non-admin: the key is present, the value null. + const [unreadCount, newChallenges, attention, banner] = await Promise.all([ + countUnread(pool, viewer), + countNewChallenges(pool, viewer), + hasTriageAttention(viewer) ? countTriageAttention(pool, viewer) : Promise.resolve(null), + getActiveSystemBanner(pool), + ]); + return { unreadCount, newChallenges, attention, banner }; +} diff --git a/packages/web/src/app/api/notifications/item-columns.dbtest.ts b/packages/web/src/app/api/notifications/item-columns.dbtest.ts new file mode 100644 index 0000000..fc88d58 --- /dev/null +++ b/packages/web/src/app/api/notifications/item-columns.dbtest.ts @@ -0,0 +1,168 @@ +// Live-DB integration test (gated) for what an inbox row is about, as columns (INNOBOX_SPEC.md §5 +// `notifications`, §12.1 read-time gate, §10.4, migration 0051, v1.18): +// - the trigger derives challenge_id / solution_id from every payload shape a writer uses — +// solution parent (both ids), challenge parent, a comment row's challengeId, and the bare deep +// link (`/challenges/`, `…#SOL-`) — and leaves both null for a row that names no item or +// names one that does not exist; +// - a value a caller supplies is overwritten, and a payload update (coalescing) re-derives; +// - the bell list's unreadCount equals the §12.2 pulse count, under the same gates; +// - a link-only row now follows the visibility and trash gates like a keyed one; +// - opening a challenge marks its comment/hype rows read — the challenge's own AND its solutions'. +// Self-skips when DATABASE_URL is unset. +import { test } from "node:test"; +import assert from "node:assert/strict"; + +const url = process.env.DATABASE_URL; + +test( + "§5/§12.1 notification item columns: derived on write, gate by primary key, same count everywhere", + { skip: url ? false : "DATABASE_URL not set — live-DB suite self-skips" }, + async () => { + const { Pool } = await import("pg"); + const { randomUUID } = await import("node:crypto"); + const { buildRoleSet } = await import("@innobox/shared"); + const { getInbox, countUnread, listInboxPage } = await import("./store"); + const { markCommentNotificationsReadForChallenge } = await import("../../../lib/notify"); + + const pool = new Pool({ connectionString: url }); + try { + const stamp = randomUUID().slice(0, 8); + const { rows: g } = await pool.query<{ id: string }>(`select id from namespaces where slug = 'global'`); + const globalId = g[0]!.id; + const { rows: ns } = await pool.query<{ id: string }>(`insert into namespaces (slug, display_name) values ($1, 'Dbtest ItemCols NS') returning id`, [ + `dbtest-itemcols-${stamp}`, + ]); + const nsId = ns[0]!.id; + const { rows: ia } = await pool.query<{ id: string }>(`select id from impact_areas where active and name <> 'Client' limit 1`); + const mkUser = async (label: string) => { + const { rows } = await pool.query<{ id: string }>( + `insert into users (external_id, user_name, display_name, email) values ($1, $2, $3, $4) returning id`, + [`dbtest-itemcols-${label}-${stamp}`, `dbtest-itemcols-${label}-${stamp}@example.test`, `Dbtest ItemCols ${label}`, `itemcols-${label}-${stamp}@example.test`], + ); + return rows[0]!.id; + }; + const authorId = await mkUser("author"); + const viewer = { userId: await mkUser("viewer"), roles: buildRoleSet([], { globalNamespaceId: globalId }) }; + + const mkChallenge = async (title: string) => { + const { rows } = await pool.query<{ id: string; number: string }>( + `insert into challenges (namespace_id, visibility, title, description, impact_area_id, author_id, status) + values ($1, 'org', $2, 'item columns fixture', $3, $4, 'valid') returning id, number::text`, + [nsId, `${title} ${stamp}`, ia[0]!.id, authorId], + ); + return rows[0]!; + }; + const ch = await mkChallenge("ItemCols"); + const other = await mkChallenge("ItemCols other"); + const { rows: sol } = await pool.query<{ id: string; number: string }>( + `insert into solutions (challenge_id, description, author_id, status) values ($1, 'sol', $2, 'in_review') returning id, number::text`, + [ch.id, authorId], + ); + const solution = sol[0]!; + + const insert = async (type: string, payload: Record, extra: { challengeId?: string; solutionId?: string } = {}) => { + const { rows } = await pool.query<{ id: string; challenge_id: string | null; solution_id: string | null }>( + `insert into notifications (user_id, type, payload, challenge_id, solution_id) values ($1, $2, $3, $4, $5) + returning id, challenge_id, solution_id`, + [viewer.userId, type, JSON.stringify(payload), extra.challengeId ?? null, extra.solutionId ?? null], + ); + return rows[0]!; + }; + const msg = (m: string) => ({ message: `${m} ${stamp}` }); + + // ── derivation, one payload shape at a time ────────────────────────────────────────── + const bySolution = await insert("status_changed", { ...msg("sol"), link: `/challenges/${ch.number}#SOL-${solution.number}`, parentType: "solution", parentId: solution.id }); + assert.deepEqual([bySolution.challenge_id, bySolution.solution_id], [ch.id, solution.id], "a solution row carries its challenge too"); + + const byChallenge = await insert("status_changed", { ...msg("ch"), link: `/challenges/${ch.number}`, parentType: "challenge", parentId: ch.id }); + assert.deepEqual([byChallenge.challenge_id, byChallenge.solution_id], [ch.id, null]); + + const commentOnSolution = await insert("comment_posted", { + ...msg("comment"), + link: `/challenges/${ch.number}#SOL-${solution.number}`, + parentType: "solution", + parentId: solution.id, + challengeId: ch.id, + count: 1, + }); + assert.deepEqual([commentOnSolution.challenge_id, commentOnSolution.solution_id], [ch.id, solution.id]); + + const challengeIdOnly = await insert("comment_posted", { ...msg("cid"), link: "/x", challengeId: ch.id, count: 1 }); + assert.deepEqual([challengeIdOnly.challenge_id, challengeIdOnly.solution_id], [ch.id, null], "challengeId alone"); + + const linkOnly = await insert("status_changed", { ...msg("link"), link: `/challenges/${ch.number}` }); + assert.deepEqual([linkOnly.challenge_id, linkOnly.solution_id], [ch.id, null], "a pre-target row resolves by its link"); + + const solLinkOnly = await insert("status_changed", { ...msg("sollink"), link: `/challenges/${ch.number}#SOL-${solution.number}` }); + assert.deepEqual([solLinkOnly.challenge_id, solLinkOnly.solution_id], [ch.id, solution.id], "…and a solution link to both"); + + const wrongSolLink = await insert("status_changed", { ...msg("wrongsol"), link: `/challenges/${other.number}#SOL-${solution.number}` }); + assert.deepEqual([wrongSolLink.challenge_id, wrongSolLink.solution_id], [other.id, null], "a SOL number under the wrong challenge is not that solution"); + + const noItem = await insert("achievement_earned", { ...msg("ach"), link: "/me" }); + assert.deepEqual([noItem.challenge_id, noItem.solution_id], [null, null]); + + const ghost = await insert("status_changed", { ...msg("ghost"), link: "/challenges/999999999999", parentType: "challenge", parentId: randomUUID() }); + assert.deepEqual([ghost.challenge_id, ghost.solution_id], [null, null], "an item that does not exist resolves to nothing"); + + const junk = await insert("status_changed", { ...msg("junk"), link: "/challenges/12abc", parentType: "solution", parentId: "not-a-uuid", challengeId: 42 }); + assert.deepEqual([junk.challenge_id, junk.solution_id], [null, null], "malformed keys never raise, never match"); + + const forged = await insert("achievement_earned", { ...msg("forged"), link: "/me" }, { challengeId: other.id, solutionId: solution.id }); + assert.deepEqual([forged.challenge_id, forged.solution_id], [null, null], "a supplied value is overwritten — the columns are derived, never written"); + + // A payload update (what coalescing does) re-derives; a read_at update leaves them alone. + const { rows: moved } = await pool.query<{ challenge_id: string | null }>( + `update notifications set payload = $2::jsonb where id = $1 returning challenge_id`, + [noItem.id, JSON.stringify({ ...msg("moved"), link: `/challenges/${other.number}` })], + ); + assert.equal(moved[0]!.challenge_id, other.id); + + // ── the list's unreadCount is the pulse count, under the same gates ────────────────── + const inbox = await getInbox(pool, viewer); + assert.equal(inbox.unreadCount, await countUnread(pool, viewer)); + assert.equal(inbox.unreadCount, inbox.notifications.filter((n) => !n.read).length, "fewer than 50 rows: the page holds them all"); + + // ── a link-only row now follows the gates like a keyed one ─────────────────────────── + await pool.query(`update challenges set visibility = 'namespace' where id = $1`, [ch.id]); + const hidden = new Set((await getInbox(pool, viewer)).notifications.map((n) => n.id)); + for (const r of [bySolution, byChallenge, commentOnSolution, challengeIdOnly, linkOnly, solLinkOnly]) { + assert.equal(hidden.has(r.id), false, "every row about the narrowed challenge is hidden from a non-member"); + } + assert.equal(hidden.has(noItem.id), true, "a row about another, visible challenge stays"); + assert.equal(hidden.has(ghost.id), true, "a row that names nothing resolvable is never gated"); + assert.equal((await getInbox(pool, viewer)).unreadCount, await countUnread(pool, viewer)); + // §25.5 agent view: the same column, restricted challenges left out. + const agent = await listInboxPage(pool, viewer.userId, { limit: 50, offset: 0, orgVisibleOnly: true }); + assert.equal(agent.notifications.some((n) => n.id === linkOnly.id), false); + await pool.query(`update challenges set visibility = 'org' where id = $1`, [ch.id]); + + // Trash: a trashed solution hides its rows; its challenge's own rows stay. + await pool.query(`update solutions set trashed_at = now(), trashed_by = $2, trash_reason = 'item columns fixture' where id = $1`, [solution.id, authorId]); + const afterTrash = new Set((await getInbox(pool, viewer)).notifications.map((n) => n.id)); + assert.equal(afterTrash.has(bySolution.id), false); + assert.equal(afterTrash.has(solLinkOnly.id), false); + assert.equal(afterTrash.has(byChallenge.id), true); + await pool.query(`update solutions set trashed_at = null, trashed_by = null, trash_reason = null where id = $1`, [solution.id]); + + // ── opening the challenge reads its comment rows, the solution's included ──────────── + await markCommentNotificationsReadForChallenge(pool, viewer.userId, ch.id); + const { rows: readState } = await pool.query<{ id: string; read: boolean }>( + `select id, read_at is not null as read from notifications where id = any($1::uuid[])`, + [[commentOnSolution.id, challengeIdOnly.id, bySolution.id]], + ); + const read = new Map(readState.map((r) => [r.id, r.read])); + assert.equal(read.get(commentOnSolution.id), true, "a comment on one of its solutions"); + assert.equal(read.get(challengeIdOnly.id), true, "a comment on the challenge"); + assert.equal(read.get(bySolution.id), false, "not a comment row — untouched"); + + // The old per-type payload indexes are gone; the column indexes exist. + const { rows: idx } = await pool.query<{ indexname: string }>(`select indexname from pg_indexes where tablename = 'notifications'`); + const names = new Set(idx.map((r) => r.indexname)); + for (const n of ["notifications_user_challenge_unread_idx", "notifications_challenge_idx", "notifications_solution_idx"]) assert.ok(names.has(n), n); + for (const n of ["notifications_comment_challenge_idx", "notifications_hyped_challenge_idx"]) assert.equal(names.has(n), false, n); + } finally { + await pool.end(); + } + }, +); diff --git a/packages/web/src/app/api/notifications/route.ts b/packages/web/src/app/api/notifications/route.ts index e117138..b530f2a 100644 --- a/packages/web/src/app/api/notifications/route.ts +++ b/packages/web/src/app/api/notifications/route.ts @@ -1,11 +1,11 @@ // GET/PATCH /api/notifications (INNOBOX_SPEC.md §12.2): the in-app inbox. Newest-first, -// with unread count. PATCH marks all read (single-notification mark-read is -// /api/notifications/:id). +// with unread count — fetched when the bell opens and after a mark action, never polled (the +// §12.2 pulse, GET /api/me/pulse, carries the live count and the §14.6 banner). PATCH marks all +// read (single-notification mark-read is /api/notifications/:id). import { requireUser } from "@/lib/auth"; import { pool } from "@/lib/db"; import { getInbox, markAllRead } from "./store"; import { rateLimit } from "@/lib/rate-limit"; -import { getActiveSystemBanner } from "../admin/system-banner/store"; import { withSystemLog } from "@/lib/system-log"; export const dynamic = "force-dynamic"; @@ -14,12 +14,9 @@ async function handleGET(): Promise { const gate = await requireUser(); if (!gate.ok) return gate.response; - // §14.6: the active system banner rides the bell's 30-second poll — one round trip, no second - // transport. Null when none is active (lazy expiry judged here, at read time). // §12.1: the inbox is visibility-gated at read time, so the viewer's roles ride along. const viewer = { userId: gate.user.id, roles: gate.user.roles }; - const [inbox, banner] = await Promise.all([getInbox(pool, viewer), getActiveSystemBanner(pool)]); - return Response.json({ ...inbox, banner }); + return Response.json(await getInbox(pool, viewer)); } async function handlePATCH(): Promise { diff --git a/packages/web/src/app/api/notifications/store.ts b/packages/web/src/app/api/notifications/store.ts index 06c6211..76d9b68 100644 --- a/packages/web/src/app/api/notifications/store.ts +++ b/packages/web/src/app/api/notifications/store.ts @@ -25,31 +25,15 @@ const INBOX_LIMIT = 50; /** * §10.4: an inbox row about a trashed item — or about anything under a trashed challenge — is kept * but hidden: the list and the unread count skip it, mark-all-read leaves it alone, and a restore - * brings it back exactly as it was. A row is "about" an item by the same matching the §10.3 - * purge uses: the target it carries (`parentType`/`parentId`, or `challengeId` on a comment row), - * or its deep link (`/challenges/`, `…#SOL-`). Over `notifications n`. + * brings it back exactly as it was. Since v1.18 a row is "about" an item by its `challenge_id` / + * `solution_id` columns (§5, migration 0051 — derived from the payload target by a trigger, so a + * solution row carries its parent challenge too), looked up by primary key: one index probe per + * row instead of a scan of every trashed item's link and payload keys. Over `notifications n`. */ export const NOT_ABOUT_TRASH = `not exists ( - select 1 from challenges tc - where tc.trashed_at is not null - and (n.payload->>'link' = '/challenges/' || tc.number - or n.payload->>'link' like '/challenges/' || tc.number || '#%' - or (n.payload->>'parentType' = 'challenge' and n.payload->>'parentId' = tc.id::text) - or n.payload->>'challengeId' = tc.id::text)) + select 1 from challenges tc where tc.id = n.challenge_id and tc.trashed_at is not null) and not exists ( - select 1 from solutions ts join challenges sc on sc.id = ts.challenge_id - where ts.trashed_at is not null - and ((n.payload->>'parentType' = 'solution' and n.payload->>'parentId' = ts.id::text) - or n.payload->>'link' = '/challenges/' || sc.number || '#SOL-' || ts.number))`; - -/** The challenge an inbox row is about, by the payload keys every item-bound row carries - * (§12.1): `challengeId` (comment rows), or `parentType` + `parentId` for a challenge or a - * solution (resolved to its challenge). Over `notifications n` and `challenges c`. A row that - * names no item matches no challenge. */ -const ROW_IS_ABOUT_CHALLENGE = `(c.id::text = n.payload->>'challengeId' - or (n.payload->>'parentType' = 'challenge' and c.id::text = n.payload->>'parentId') - or (n.payload->>'parentType' = 'solution' - and c.id = (select s.challenge_id from solutions s where s.id::text = n.payload->>'parentId')))`; + select 1 from solutions ts where ts.id = n.solution_id and ts.trashed_at is not null)`; /** * §12.1 read-time visibility gate (v1.16): hide a row about a challenge — or a solution on one — @@ -57,7 +41,8 @@ const ROW_IS_ABOUT_CHALLENGE = `(c.id::text = n.payload->>'challengeId' * platform admins see everything; awaiting_triage/withdrawn only the author and the namespace's * admins; `people` the access list, reviewers and assignee; else org-wide or members), so the * bell can never disagree with the detail page. A read-time predicate only — nothing is deleted, - * and a row comes back as it was once access returns. Rows that name no item pass. Over + * and a row comes back as it was once access returns. Rows that name no item (`challenge_id` null) + * pass. The web list, the §12.2 pulse count and the §25.6 agent view all go through it. Over * `notifications n`; parameters go through `push`. */ function pushInboxVisibilityCondition(viewer: Viewer, push: (v: unknown) => string, conditions: string[]): void { @@ -66,9 +51,11 @@ function pushInboxVisibilityCondition(viewer: Viewer, push: (v: unknown) => stri // coalesce: the twin is written for a plain WHERE, where SQL NULL (e.g. `c.assignee_id = $viewer` // on a challenge with no assignee) already means "not visible". Negated inside `not exists` a // NULL would mean "not hidden" instead, so it is pinned to false before the negation. + // §12.1 (v1.18): the challenge the row is about is its `challenge_id` column — a primary-key join, + // where matching payload text against `c.id::text` used to scan every challenge per row. conditions.push(`not exists ( select 1 from challenges c - where ${ROW_IS_ABOUT_CHALLENGE} + where c.id = n.challenge_id and not coalesce((${visible.join(" and ")}), false))`); } @@ -86,16 +73,24 @@ function inboxScope(viewer: Viewer, extra: string[] = []): { where: string; para } export async function getInbox(pool: Pool, viewer: Viewer): Promise { + // One evaluation of the gates: the window count runs over every row the WHERE keeps, before the + // LIMIT, so the list and its unread count come from the same pass (§12.2 — the list response + // carries `unreadCount`, which the shell adopts). const list = inboxScope(viewer); - const unread = inboxScope(viewer, [`n.read_at is null`]); - const [{ rows }, { rows: countRows }] = await Promise.all([ - pool.query<{ id: string; type: string; payload: { message: string; link: string }; read_at: Date | null; created_at: Date }>( - `select n.id, n.type, n.payload, n.read_at, n.created_at from notifications n - where ${list.where} order by n.created_at desc limit ${INBOX_LIMIT}`, - list.params, - ), - pool.query<{ count: string }>(`select count(*)::text as count from notifications n where ${unread.where}`, unread.params), - ]); + const { rows } = await pool.query<{ + id: string; + type: string; + payload: { message: string; link: string }; + read_at: Date | null; + created_at: Date; + unread_count: string; + }>( + `select n.id, n.type, n.payload, n.read_at, n.created_at, + (count(*) filter (where n.read_at is null) over ())::text as unread_count + from notifications n + where ${list.where} order by n.created_at desc limit ${INBOX_LIMIT}`, + list.params, + ); return { notifications: rows.map((r) => ({ @@ -106,10 +101,17 @@ export async function getInbox(pool: Pool, viewer: Viewer): Promise { read: r.read_at !== null, createdAt: r.created_at.toISOString(), })), - unreadCount: Number(countRows[0]?.count ?? 0), + unreadCount: Number(rows[0]?.unread_count ?? 0), }; } +/** §12.2 pulse: the bell's unread count alone, under the same gates as the list. */ +export async function countUnread(pool: Pool, viewer: Viewer): Promise { + const scope = inboxScope(viewer, [`n.read_at is null`]); + const { rows } = await pool.query<{ count: string }>(`select count(*)::text as count from notifications n where ${scope.where}`, scope.params); + return Number(rows[0]?.count ?? 0); +} + /** * §25.6 list_my_notifications: a page of the caller's inbox, newest first. With `orgVisibleOnly` * (§25.5) a row about a restricted challenge (namespace-only or shared with specific people, §4.3) @@ -127,7 +129,7 @@ export async function listInboxPage( if (opts.orgVisibleOnly) { conditions.push(`not exists ( select 1 from challenges c - where c.visibility <> 'org' and ${ROW_IS_ABOUT_CHALLENGE})`); + where c.id = n.challenge_id and c.visibility <> 'org')`); } const { rows } = await pool.query<{ id: string; type: string; payload: { message: string; link: string }; read_at: Date | null; created_at: Date }>( `select n.id, n.type, n.payload, n.read_at, n.created_at from notifications n diff --git a/packages/web/src/app/whats-new/changelog.ts b/packages/web/src/app/whats-new/changelog.ts index c9bf835..4a7ae68 100644 --- a/packages/web/src/app/whats-new/changelog.ts +++ b/packages/web/src/app/whats-new/changelog.ts @@ -10,6 +10,12 @@ export interface ChangelogEntry { } export const CHANGELOG: ChangelogEntry[] = [ + { + version: "1.18.0", + date: "2026-10-21", + summary: + "Faster and lighter: the bell, the new-challenges and triage bubbles and the system banner now update through one check every 30 seconds, and a tab you are not looking at stops checking until you return. Notifications load faster as your inbox grows, and every page spends less time confirming who you are. Nothing changes in what you see.", + }, { version: "1.17.2", date: "2026-10-20", diff --git a/packages/web/src/components/AppShell.tsx b/packages/web/src/components/AppShell.tsx index 2eed31c..dbbd88e 100644 --- a/packages/web/src/components/AppShell.tsx +++ b/packages/web/src/components/AppShell.tsx @@ -19,7 +19,7 @@ import { QUICK_START_PATH, shouldRedirectToQuickStart } from "../lib/quick-start import { ThemeToggle } from "./ThemeToggle"; import { NotificationBell } from "./NotificationBell"; import { SystemBanner } from "./SystemBanner"; -import type { SystemBanner as Banner } from "@innobox/shared/system-banner"; +import { usePulse } from "./usePulse"; import { TopbarSearch, type TopbarSearchHandle } from "./TopbarSearch"; import { AvatarBubble } from "./AvatarBubble"; import { SurveyCard, type BugCardOthers, type StartSelfOutcome } from "./SurveyCard"; @@ -102,13 +102,12 @@ export function AppShell({ children }: { children: ReactNode }) { // Role/identity hints for the nav + account menu. Fetched only when authenticated — the // signed-out landing shows no user data (INNOBOX_SPEC.md §2.1 invariant 2, §13.2). const [me, setMe] = useState(null); - // §14.4 attention count for the Triage/Administration nav bubbles — polled while an admin is - // signed in (see the effect below). - const [attention, setAttention] = useState(0); - // §14.6 system banner — delivered on the bell's poll, rendered in the topbar. - const [banner, setBanner] = useState(null); - // §13.1 "new since your last visit" — the count behind the Challenges nav bubble. - const [newChallenges, setNewChallenges] = useState(0); + // §12.2 pulse (v1.18): the shell's ONLY poll — the bell's unread count, the §13.1 "new since + // your last visit" count behind the Challenges bubble, the §14.4 attention count behind the + // Triage/Administration bubbles (null for non-admins) and the §14.6 banner. Visible tabs only. + const { pulse, adoptUnreadCount } = usePulse(authed); + const attention = pulse.attention ?? 0; + const newChallenges = pulse.newChallenges; const previousPath = useRef(null); // §13.10 feedback survey: the open offer, the card (and how it was opened), the bell's open // state (its mobile sheet blocks the card), and the shell's toast. @@ -429,60 +428,12 @@ export function AppShell({ children }: { children: ReactNode }) { showToast("Got it — no more surveys. You can turn them back on in your profile."); }, [refreshSurvey, showToast]); - // §14.4: poll the unseen-actionable count while an admin is signed in, on the same 30s cadence - // as the notification bell. The triage-queue page dispatches `innobox:triage-seen` after it - // stamps triage_seen_at, so the bubble clears promptly on open instead of waiting for the tick. - useEffect(() => { - if (!authed || !isAdmin) { - setAttention(0); - return; - } - let live = true; - const fetchAttention = () => { - fetch("/api/admin/triage/attention", { headers: { accept: "application/json" } }) - .then((res) => (res.ok ? res.json() : null)) - .then((json) => { - if (live && json) setAttention(json.count ?? 0); - }) - .catch(() => {}); - }; - fetchAttention(); - const interval = window.setInterval(fetchAttention, 30_000); - window.addEventListener("innobox:triage-seen", fetchAttention); - return () => { - live = false; - window.clearInterval(interval); - window.removeEventListener("innobox:triage-seen", fetchAttention); - }; - }, [authed, isAdmin]); - - // §13.1: poll the new-challenge count on the bell's 30 s cadence while signed in. Leaving the - // Challenges surface (gallery or a detail page → anywhere else) advances the marker, then the - // bubble is refreshed so it clears at once; entering never touches it, so the count and the - // card tags stay stable for the whole visit. A tab closed while on the surface counts as a - // leave too (a beacon survives page hide where a fetch would not). - useEffect(() => { - if (!authed) return; - let live = true; - const fetchCount = () => { - fetch("/api/challenges/new-count", { headers: { accept: "application/json" } }) - .then((res) => (res.ok ? res.json() : null)) - .then((json) => { - if (live && json) setNewChallenges(json.count ?? 0); - }) - .catch(() => {}); - }; - fetchCount(); - const interval = window.setInterval(fetchCount, 30_000); - const onSeen = () => fetchCount(); - window.addEventListener("innobox:challenges-seen", onSeen); - return () => { - live = false; - window.clearInterval(interval); - window.removeEventListener("innobox:challenges-seen", onSeen); - }; - }, [authed]); - + // §14.4 / §13.1: the attention and new-challenges counts ride on the §12.2 pulse (usePulse). The + // triage queue dispatches `innobox:triage-seen` after stamping triage_seen_at, and leaving the + // Challenges surface dispatches `innobox:challenges-seen` below — both re-read the pulse at once. + // A tab closed while on the surface counts as a leave too (a beacon survives page hide where a + // fetch would not). Entering never touches the marker, so the count and the card tags stay + // stable for the whole visit. useEffect(() => { if (!authed) return; const prev = previousPath.current; @@ -632,8 +583,8 @@ export function AppShell({ children }: { children: ReactNode }) { {authed && }
- {authed && } - {authed && } + {authed && } + {authed && }
{quickStartPending ? null : children}
diff --git a/packages/web/src/components/NotificationBell.tsx b/packages/web/src/components/NotificationBell.tsx index 391741a..a22589e 100644 --- a/packages/web/src/components/NotificationBell.tsx +++ b/packages/web/src/components/NotificationBell.tsx @@ -1,11 +1,10 @@ "use client"; // In-app notification bell (INNOBOX_SPEC.md §12.2): unread count, newest-first list, -// mark-read (click) and mark-all-read. Reuses the .bell/.msg-* class contract from -// globals.css (carried-over messaging pattern) — no new CSS. +// mark-read (click) and mark-all-read. The count comes from the shell's §12.2 pulse. Reuses the +// .bell/.msg-* class contract from globals.css (carried-over messaging pattern) — no new CSS. import Link from "next/link"; import { useEffect, useRef, useState } from "react"; import { useRouter } from "next/navigation"; -import type { SystemBanner } from "@innobox/shared/system-banner"; import { useDateFmt } from "@/components/DateFormat"; interface NotificationItem { @@ -17,43 +16,41 @@ interface NotificationItem { createdAt: string; } -/** `onBanner` receives the §14.6 system banner (or null) that rides every inbox poll, so the - * shell can render the topbar pill without a second poller. `onOpenChange` reports the panel's - * open state — on mobile it is a full sheet, which keeps the §13.10 survey card off screen. */ +/** §12.2 (v1.18): the badge shows `unreadCount` from the shell's pulse — the bell itself never + * polls. Its list is fetched when the panel opens and again after mark-all-read; that response + * carries the unread count too, handed back through `onUnreadCount` so badge and list agree. A + * mark action also dispatches `innobox:pulse`, re-reading the pulse at once. `onOpenChange` + * reports the panel's open state — on mobile it is a full sheet, which keeps the §13.10 survey + * card off screen. */ export function NotificationBell({ - onBanner, + unreadCount, + onUnreadCount, onOpenChange, -}: { onBanner?: (banner: SystemBanner | null) => void; onOpenChange?: (open: boolean) => void } = {}) { +}: { unreadCount: number; onUnreadCount?: (count: number) => void; onOpenChange?: (open: boolean) => void }) { const router = useRouter(); const fmt = useDateFmt(); const [open, setOpen] = useState(false); const [items, setItems] = useState(null); - const [unreadCount, setUnreadCount] = useState(0); const rootRef = useRef(null); - const refresh = () => { + const loadList = () => { fetch("/api/notifications", { headers: { accept: "application/json" } }) - .then((res) => res.json()) + .then((res) => (res.ok ? res.json() : null)) .then((json) => { + if (!json) return; setItems(json.notifications); - setUnreadCount(json.unreadCount); - onBanner?.((json.banner as SystemBanner | null | undefined) ?? null); + if (typeof json.unreadCount === "number") onUnreadCount?.(json.unreadCount); }) .catch(() => {}); }; + const refreshPulse = () => window.dispatchEvent(new Event("innobox:pulse")); + + // The list is not polled: it loads each time the panel opens. useEffect(() => { - refresh(); - const interval = window.setInterval(refresh, 30_000); - // The Administration card dispatches this after a save/clear so the admin's own header - // reflects the change at once instead of at the next tick. - window.addEventListener("innobox:banner-changed", refresh); - return () => { - window.clearInterval(interval); - window.removeEventListener("innobox:banner-changed", refresh); - }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + if (open) loadList(); + // eslint-disable-next-line react-hooks/exhaustive-deps -- load on open only + }, [open]); useEffect(() => { onOpenChange?.(open); @@ -71,14 +68,15 @@ export function NotificationBell({ const markAllRead = async () => { await fetch("/api/notifications", { method: "PATCH" }); - refresh(); + loadList(); + refreshPulse(); }; const openItem = async (item: NotificationItem) => { setOpen(false); if (!item.read) { await fetch(`/api/notifications/${item.id}`, { method: "PATCH" }); - refresh(); + refreshPulse(); } router.push(item.link); }; diff --git a/packages/web/src/components/usePulse.ts b/packages/web/src/components/usePulse.ts new file mode 100644 index 0000000..358c6ac --- /dev/null +++ b/packages/web/src/components/usePulse.ts @@ -0,0 +1,101 @@ +"use client"; +// The §12.2 pulse poller (INNOBOX_SPEC.md §12.2, v1.18): ONE request per tab, every 30 s while the +// tab is visible and never while it is hidden; a tab that comes back polls at once when its last +// pulse is stale, then resumes the cadence. The window events in PULSE_REFRESH_EVENTS re-read it +// immediately. An unchanged pulse keeps the previous state object, so an idle tick re-renders +// nothing. A 401 ends the session: polling stops. +import { useCallback, useEffect, useRef, useState } from "react"; +import { + EMPTY_PULSE, + PULSE_INTERVAL_MS, + PULSE_REFRESH_EVENTS, + parsePulse, + pulseOutcome, + samePulse, + shouldPollOnVisible, + type Pulse, +} from "../lib/pulse"; + +export interface PulseControls { + pulse: Pulse; + /** The bell's list response carries the unread count too; the shell adopts it (§12.2). */ + adoptUnreadCount: (unreadCount: number) => void; +} + +export function usePulse(enabled: boolean): PulseControls { + const [pulse, setPulse] = useState(EMPTY_PULSE); + const lastAt = useRef(null); + const stopped = useRef(false); + const inFlight = useRef(false); + + const apply = useCallback((next: Pulse) => { + setPulse((prev) => (samePulse(prev, next) ? prev : next)); + }, []); + + const adoptUnreadCount = useCallback((unreadCount: number) => { + setPulse((prev) => (prev.unreadCount === unreadCount ? prev : { ...prev, unreadCount })); + }, []); + + useEffect(() => { + if (!enabled) { + lastAt.current = null; + return; + } + stopped.current = false; + let live = true; + let timer: number | null = null; + + const poll = async () => { + if (!live || stopped.current || inFlight.current) return; + if (document.visibilityState === "hidden") return; + inFlight.current = true; + try { + const res = await fetch("/api/me/pulse", { headers: { accept: "application/json" }, cache: "no-store" }); + const outcome = pulseOutcome(res.status); + if (outcome === "stop") stopped.current = true; + if (outcome === "apply" && live) { + apply(parsePulse(await res.json())); + lastAt.current = Date.now(); + } + } catch { + /* network blip: keep the last values, retry on the next tick */ + } finally { + inFlight.current = false; + } + }; + + const start = () => { + if (timer === null) timer = window.setInterval(() => void poll(), PULSE_INTERVAL_MS); + }; + const stop = () => { + if (timer !== null) window.clearInterval(timer); + timer = null; + }; + + const onVisibility = () => { + if (document.visibilityState === "hidden") { + stop(); + return; + } + if (shouldPollOnVisible(lastAt.current, Date.now())) void poll(); + start(); + }; + const onRefresh = () => void poll(); + + if (document.visibilityState !== "hidden") { + void poll(); + start(); + } + document.addEventListener("visibilitychange", onVisibility); + for (const name of PULSE_REFRESH_EVENTS) window.addEventListener(name, onRefresh); + return () => { + live = false; + stop(); + document.removeEventListener("visibilitychange", onVisibility); + for (const name of PULSE_REFRESH_EVENTS) window.removeEventListener(name, onRefresh); + }; + }, [enabled, apply]); + + // Signed out (or not yet signed in): nothing to show, whatever an earlier session left in state. + return { pulse: enabled ? pulse : EMPTY_PULSE, adoptUnreadCount }; +} diff --git a/packages/web/src/lib/auth.ts b/packages/web/src/lib/auth.ts index 94c9716..83ccfb6 100644 --- a/packages/web/src/lib/auth.ts +++ b/packages/web/src/lib/auth.ts @@ -2,14 +2,17 @@ // cookie only proves authentication happened; identity, `active`, and roles come from // the database on EVERY request — a deactivated user holding a live cookie loses access // at the next call (§2 leaver semantics), and roles NEVER come from token claims -// (INNOBOX_SPEC.md invariant 1; per-request resolution, no role cache in v1). +// (INNOBOX_SPEC.md invariant 1; per-request resolution, no role cache in v1). §4.2 *Resolution*: +// one statement per resolution, at most one resolution per request. import { headers } from "next/headers"; import { getServerSession } from "next-auth"; -import { buildRoleSet, type Role, type RoleGrant, type RoleSet } from "@innobox/shared"; +import type { RoleSet } from "@innobox/shared"; import { authOptions } from "./authOptions"; import { pool } from "./db"; import { PRESENCE_METHOD_HEADER, PRESENCE_PATH_HEADER, touchPresence } from "./presence-touch"; import { isSessionRevoked } from "./session-revocation"; +import { requestMemo } from "./request-memo"; +import { bootstrapAdminGroup, loadSessionRow, resolveRoleSets } from "./role-resolution"; export interface SessionUser { id: string; @@ -26,73 +29,13 @@ export interface SessionUser { roles: RoleSet; } -// The built-in 'global' namespace is seeded by migration 0003 and can never be deleted -// (namespaces are archive-only), so its id is safe to cache for the process lifetime. -let globalNamespaceId: string | null = null; -async function getGlobalNamespaceId(): Promise { - if (!globalNamespaceId) { - const { rows } = await pool.query<{ id: string }>( - `select id from namespaces where slug = 'global'`, - ); - if (!rows[0]) throw new Error("built-in 'global' namespace missing — are migrations applied?"); - globalNamespaceId = rows[0].id; - } - return globalNamespaceId; -} - -/** Resolves a RoleSet for each of many user ids in a fixed number of queries, whatever the count — - * notification dispatch (lib/notify.ts) visibility-checks every candidate recipient of an event, - * and a per-user lookup made that cost grow with the recipient list. Every id gets an entry; a - * user with no grants gets the implicit-member RoleSet. The ONE place roles are resolved - * (invariant 1): resolveRolesForUser and getSessionUser go through it too. */ +/** Resolves a RoleSet for each of many user ids in ONE statement, whatever the count (§4.2 + * *Resolution*) — notification dispatch (lib/notify.ts) visibility-checks every candidate + * recipient of an event at once. Every id gets an entry; a user with no grants gets the + * implicit-member RoleSet. The ONE place roles are resolved (invariant 1): resolveRolesForUser and + * getSessionUser go through the same SQL (lib/role-resolution.ts). */ export async function resolveRolesForUsers(userIds: readonly string[]): Promise> { - const ids = [...new Set(userIds)]; - if (ids.length === 0) return new Map(); - - const grants = new Map(ids.map((id) => [id, []])); - const { rows: grantRows } = await pool.query<{ user_id: string; role: Role; namespace_id: string | null }>( - `select gm.user_id, rm.role, rm.namespace_id - from group_members gm - join groups g on g.id = gm.group_id - join role_mappings rm on rm.group_external_id = g.external_id - where gm.user_id = any($1::uuid[])`, - [ids], - ); - for (const g of grantRows) grants.get(g.user_id)!.push({ role: g.role, namespaceId: g.namespace_id }); - - const bootstrapAdmins = new Set(); - const bootstrapGroup = process.env.INNOBOX_BOOTSTRAP_ADMIN_GROUP; - if (bootstrapGroup) { - const { rows: b } = await pool.query<{ user_id: string }>( - `select distinct gm.user_id - from group_members gm - join groups g on g.id = gm.group_id - where gm.user_id = any($1::uuid[]) and g.external_id = $2`, - [ids, bootstrapGroup], - ); - for (const r of b) bootstrapAdmins.add(r.user_id); - } - - // §4.3 `people`: the challenges whose access list names the user — resolved per request with the - // grants, so the visibility gate and its SQL twin read it exactly like namespace membership. - const access = new Map(ids.map((id) => [id, []])); - const { rows: accessRows } = await pool.query<{ user_id: string; challenge_id: string }>( - `select user_id, challenge_id from challenge_access where user_id = any($1::uuid[])`, - [ids], - ); - for (const r of accessRows) access.get(r.user_id)!.push(r.challenge_id); - - const globalNamespaceId = await getGlobalNamespaceId(); - return new Map( - ids.map((id) => [ - id, - buildRoleSet(grants.get(id)!, { - bootstrapPlatformAdmin: bootstrapAdmins.has(id), - globalNamespaceId, - accessListChallengeIds: access.get(id)!, - }), - ]), - ); + return resolveRoleSets(pool, userIds, bootstrapAdminGroup()); } /** Resolves a RoleSet for an arbitrary user id (not just the session user) — the assignee and @@ -117,56 +60,51 @@ async function stampPresence(userId: string): Promise { } } +/** The request's scope object for the §4.2 per-request memo: the headers object Next hands out + * once per request (lib/request-memo.ts). Null outside a request scope — then nothing is memoized. */ +async function requestScope(): Promise { + try { + return await headers(); + } catch { + return null; + } +} + +const sessionMemo = requestMemo(); + /** The signed-in, ACTIVE user with DB-resolved roles — or null (unauthenticated, or the * user row is missing/deactivated, or the token was issued before the row's - * `sessions_valid_from` — "Sign out everywhere", INNOBOX_SPEC.md §3: same outcome, no session). */ + * `sessions_valid_from` — "Sign out everywhere", INNOBOX_SPEC.md §3: same outcome, no session). + * §4.2 *Resolution*: the user row and its roles come back from ONE statement, and a request that + * asks twice (a handler and a helper it calls) shares one resolution — never across requests. */ export async function getSessionUser(): Promise { + return sessionMemo(await requestScope(), resolveSessionUser); +} + +async function resolveSessionUser(): Promise { const session = await getServerSession(authOptions); const oid = session?.oid; if (!oid) return null; - const { rows } = await pool.query<{ - id: string; - external_id: string; - user_name: string; - email: string | null; - display_name: string; - active: boolean; - sessions_valid_from: Date | null; - email_notifications_enabled: boolean; - quick_start_seen_at: Date | null; - whats_new_seen_version: string | null; - whats_new_since_date: string | null; - }>( - // The rollout boundary is a bare calendar date: read it as text, never through a JS Date, - // which would shift it by the server's offset. - `select id, external_id, user_name, email, display_name, active, sessions_valid_from, email_notifications_enabled, quick_start_seen_at, - whats_new_seen_version, to_char(whats_new_since_date, 'YYYY-MM-DD') as whats_new_since_date - from users - where external_id = $1`, - [oid], - ); - const row = rows[0]; + const row = await loadSessionRow(pool, oid, bootstrapAdminGroup()); if (!row || !row.active) return null; // §3 Sign out everywhere / erasure / SCIM deactivation stamp: a token minted before it is no // session, whatever the cookie says — the API answers 401, a page redirects to Home. - if (isSessionRevoked({ issuedAt: session.issuedAt, sessionsValidFrom: row.sessions_valid_from })) return null; + if (isSessionRevoked({ issuedAt: session.issuedAt, sessionsValidFrom: row.sessionsValidFrom })) return null; await stampPresence(row.id); - const roles = await resolveRolesForUser(row.id); - return { id: row.id, - externalId: row.external_id, - userName: row.user_name, - displayName: row.display_name, + externalId: row.externalId, + userName: row.userName, + displayName: row.displayName, email: row.email, - emailNotificationsEnabled: row.email_notifications_enabled, - quickStartSeenAt: row.quick_start_seen_at, - whatsNewSeenVersion: row.whats_new_seen_version, - whatsNewSinceDate: row.whats_new_since_date, - roles, + emailNotificationsEnabled: row.emailNotificationsEnabled, + quickStartSeenAt: row.quickStartSeenAt, + whatsNewSeenVersion: row.whatsNewSeenVersion, + whatsNewSinceDate: row.whatsNewSinceDate, + roles: row.roles, }; } diff --git a/packages/web/src/lib/notify.ts b/packages/web/src/lib/notify.ts index 8d99312..8952f3b 100644 --- a/packages/web/src/lib/notify.ts +++ b/packages/web/src/lib/notify.ts @@ -349,11 +349,12 @@ export async function dispatchCoalescedComment( /** §12.1 read action: opening a challenge's page reads the viewer's unread comment rows — and, * since §26.6, their unread "hyped" rows — for the challenge itself AND its solutions (the page - * shows both). */ + * shows both). A solution row carries its challenge in `challenge_id` (§5, migration 0051), so one + * probe of the (user_id, challenge_id) unread index covers both. */ export async function markCommentNotificationsReadForChallenge(db: Db, userId: string, challengeId: string): Promise { await db.query( `update notifications set read_at = now() - where user_id = $1 and type in ('comment_posted', 'hyped') and read_at is null and payload->>'challengeId' = $2`, + where user_id = $1 and type in ('comment_posted', 'hyped') and read_at is null and challenge_id = $2::uuid`, [userId, challengeId], ); } diff --git a/packages/web/src/lib/presence.test.ts b/packages/web/src/lib/presence.test.ts index d4c7673..452e58e 100644 --- a/packages/web/src/lib/presence.test.ts +++ b/packages/web/src/lib/presence.test.ts @@ -26,9 +26,11 @@ import { } from "./presence"; test("the 30-second pollers are NOT activity — otherwise 'online' means 'left a tab open'", () => { - // The notification bell (§12.2) and the triage attention badge (§14.4) both poll every - // 30s from any open tab. If either counted, a user who opened InnoBox on Monday would - // still read as "active just now" on Friday. + // The §12.2 pulse (v1.18) polls every 30s from every visible tab, carrying the bell, the + // attention badge (§14.4), the new-challenges badge (§13.1) and the banner. If it counted, a + // user who opened InnoBox on Monday would still read as "active just now" on Friday. + assert.equal(presenceTouchFor("/api/me/pulse", "GET"), null); + // The bell's list (fetched on open) and the retired pre-pulse pollers stay silent too. assert.equal(presenceTouchFor("/api/notifications", "GET"), null); assert.equal(presenceTouchFor("/api/admin/triage/attention", "GET"), null); // The shell's new-challenges badge polls on every page — silent, so Home stays "Overview". diff --git a/packages/web/src/lib/presence.ts b/packages/web/src/lib/presence.ts index 153b592..2e093e7 100644 --- a/packages/web/src/lib/presence.ts +++ b/packages/web/src/lib/presence.ts @@ -71,11 +71,12 @@ export const PRESENCE_THROTTLE_MS = 60_000; // --------------------------------------------------------------------------------------- // EXPLICIT ALLOWLIST, never a denylist (§14.5): a path that isn't listed here is silent. // A future polling widget therefore cannot quietly pollute presence by being forgotten — -// the failure mode is an under-count, which is the safe direction. The two pollers that -// exist today (GET /api/notifications on the bell's 30 s cadence, §12.2, and -// GET /api/admin/triage/attention on the same cadence, §14.4) are absent by construction: -// counting them would make "online" mean "left a tab open", and every user who ever opened -// InnoBox would read as permanently active. +// the failure mode is an under-count, which is the safe direction. The one poller that +// exists today — the §12.2 pulse, GET /api/me/pulse, every 30 s from every visible tab, which +// carries the bell count, the §13.1 new-challenges count, the §14.4 attention count and the +// banner (v1.18) — is absent by construction, as is the bell's list (GET /api/notifications, +// fetched when the bell opens): counting them would make "online" mean "left a tab open", and +// every user who ever opened InnoBox would read as permanently active. // // Two tiers: // "locate" — the request tells us where the user is; it stamps last_seen_at AND last_route. @@ -160,9 +161,9 @@ export function presenceTouchFor(pathname: string, method: string): PresenceTouc // plus an anonymous challenge appearing moments later is the §9 correlation with // extra steps (§14.5). if (!second) return { tier: "locate", route: ROUTE_CHALLENGES }; - // The shell's "new since your last visit" badge (§13.1) polls this every 30 s on every - // page — a background poller, never activity (§14.5); counting it would also relocate a - // user on Home to "Challenges" whenever it raced the dashboard read. + // "new-count" was the §13.1 badge's own poller before the §12.2 pulse absorbed it (v1.18). + // The route is gone; a stale tab still asking for it must not relocate a user on Home to + // "Challenges", so it stays silent. if (second === "new-count") return null; const token = entityToken("challenge", second); // A nested solutions listing (…/challenges/412/solutions) is still "on CH-412". diff --git a/packages/web/src/lib/pulse.test.ts b/packages/web/src/lib/pulse.test.ts new file mode 100644 index 0000000..add1710 --- /dev/null +++ b/packages/web/src/lib/pulse.test.ts @@ -0,0 +1,64 @@ +// Unit tests for the §12.2 pulse helpers (INNOBOX_SPEC.md §12.2, v1.18): response parsing, the +// "no needless re-render" equality, the visibility catch-up rule and the failure handling. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { EMPTY_PULSE, PULSE_INTERVAL_MS, PULSE_REFRESH_EVENTS, parsePulse, pulseOutcome, samePulse, shouldPollOnVisible, type Pulse } from "./pulse"; + +const banner = { message: "Maintenance tonight", tone: "info", url: null, expiresAt: "2026-10-22T18:00:00.000Z" } as Pulse["banner"]; + +test("parsePulse reads the four values the route returns", () => { + assert.deepEqual(parsePulse({ unreadCount: 3, newChallenges: 5, attention: 2, banner }), { unreadCount: 3, newChallenges: 5, attention: 2, banner }); +}); + +test("parsePulse keeps a non-admin's attention null — the key is present, the value is null", () => { + assert.equal(parsePulse({ unreadCount: 0, newChallenges: 0, attention: null, banner: null }).attention, null); + assert.equal(parsePulse({ unreadCount: 0, newChallenges: 0, banner: null }).attention, null); +}); + +test("parsePulse degrades malformed input to nothing-to-show, never to a crash or a negative bubble", () => { + assert.deepEqual(parsePulse(null), EMPTY_PULSE); + assert.deepEqual(parsePulse("nope"), EMPTY_PULSE); + assert.deepEqual(parsePulse({ unreadCount: -4, newChallenges: "7", attention: Number.NaN, banner: { message: 1 } }), { + unreadCount: 0, + newChallenges: 0, + attention: 0, + banner: null, + }); + assert.equal(parsePulse({ unreadCount: 2.9 }).unreadCount, 2); +}); + +test("samePulse: identical values are the same pulse, so an idle tick keeps the previous state", () => { + const a: Pulse = { unreadCount: 1, newChallenges: 2, attention: null, banner }; + assert.equal(samePulse(a, { ...a, banner: banner && { ...banner } }), true); + assert.equal(samePulse(EMPTY_PULSE, { ...EMPTY_PULSE }), true); +}); + +test("samePulse: any change in a count, the attention role or the banner is a new pulse", () => { + const a: Pulse = { unreadCount: 1, newChallenges: 2, attention: 0, banner }; + assert.equal(samePulse(a, { ...a, unreadCount: 2 }), false); + assert.equal(samePulse(a, { ...a, newChallenges: 3 }), false); + assert.equal(samePulse(a, { ...a, attention: null }), false, "0 (an admin with nothing unseen) is not null (not an admin)"); + assert.equal(samePulse(a, { ...a, banner: null }), false); + assert.equal(samePulse(a, { ...a, banner: banner && { ...banner, expiresAt: "2026-10-23T18:00:00.000Z" } }), false, "a replaced banner"); +}); + +test("shouldPollOnVisible: a returning tab catches up at once only when its last pulse is stale", () => { + const now = 1_000_000; + assert.equal(shouldPollOnVisible(null, now), true, "never polled"); + assert.equal(shouldPollOnVisible(now - PULSE_INTERVAL_MS, now), true, "exactly one interval old"); + assert.equal(shouldPollOnVisible(now - 5 * 60_000, now), true, "hidden for minutes"); + assert.equal(shouldPollOnVisible(now - 5_000, now), false, "a quick tab switch does not double-poll"); +}); + +test("pulseOutcome: 2xx applies, 401 stops polling, anything else keeps the last values", () => { + assert.equal(pulseOutcome(200), "apply"); + assert.equal(pulseOutcome(401), "stop"); + assert.equal(pulseOutcome(403), "keep"); + assert.equal(pulseOutcome(429), "keep"); + assert.equal(pulseOutcome(500), "keep"); +}); + +test("the immediate-refresh events cover mark-read, the banner card, the triage queue and leaving Challenges", () => { + assert.deepEqual([...PULSE_REFRESH_EVENTS].sort(), ["innobox:banner-changed", "innobox:challenges-seen", "innobox:pulse", "innobox:triage-seen"]); + assert.equal(PULSE_INTERVAL_MS, 30_000); +}); diff --git a/packages/web/src/lib/pulse.ts b/packages/web/src/lib/pulse.ts new file mode 100644 index 0000000..b89cda9 --- /dev/null +++ b/packages/web/src/lib/pulse.ts @@ -0,0 +1,70 @@ +// The §12.2 pulse — one poll per tab for every live counter the shell shows (INNOBOX_SPEC.md §12.2, +// v1.18): the bell's unread count, the §13.1 new-challenges bubble, the §14.4 triage attention +// bubble and the §14.6 system banner. Pure helpers shared by the route, the shell hook and tests — +// no React, no DB. +import type { SystemBanner } from "@innobox/shared/system-banner"; + +export interface Pulse { + unreadCount: number; + newChallenges: number; + /** null for anyone who is not a namespace or platform admin — the key is always present. */ + attention: number | null; + banner: SystemBanner | null; +} + +export const PULSE_INTERVAL_MS = 30_000; + +/** Window events that ask the shell to re-read the pulse at once instead of at the next tick + * (§12.2 *Immediate refresh*). The first three predate the pulse and keep their names. */ +export const PULSE_REFRESH_EVENTS = [ + "innobox:pulse", // mark-read / mark-all-read in the bell + "innobox:banner-changed", // the admin saved or cleared the §14.6 banner + "innobox:triage-seen", // the triage queue stamped triage_seen_at (§14.4 clearing) + "innobox:challenges-seen", // leaving the Challenges surface advanced the §13.1 marker +] as const; + +export const EMPTY_PULSE: Pulse = { unreadCount: 0, newChallenges: 0, attention: null, banner: null }; + +function count(v: unknown): number { + return typeof v === "number" && Number.isFinite(v) && v >= 0 ? Math.floor(v) : 0; +} + +/** Reads a pulse response defensively: anything malformed degrades to "nothing to show". */ +export function parsePulse(json: unknown): Pulse { + const o = (json && typeof json === "object" ? json : {}) as Record; + const b = o.banner as Record | null | undefined; + const banner = + b && typeof b === "object" && typeof b.message === "string" && typeof b.expiresAt === "string" + ? (b as unknown as SystemBanner) + : null; + return { + unreadCount: count(o.unreadCount), + newChallenges: count(o.newChallenges), + attention: o.attention === null || o.attention === undefined ? null : count(o.attention), + banner, + }; +} + +function sameBanner(a: SystemBanner | null, b: SystemBanner | null): boolean { + if (a === null || b === null) return a === b; + return a.message === b.message && a.tone === b.tone && a.url === b.url && a.expiresAt === b.expiresAt; +} + +/** §12.2 *No needless re-render*: true when a fresh pulse shows exactly what the shell already has. */ +export function samePulse(a: Pulse, b: Pulse): boolean { + return a.unreadCount === b.unreadCount && a.newChallenges === b.newChallenges && a.attention === b.attention && sameBanner(a.banner, b.banner); +} + +/** §12.2 *Cadence*: a tab that becomes visible polls at once when its last pulse is older than the + * interval (or it never had one); otherwise it just resumes the cadence. */ +export function shouldPollOnVisible(lastPulseAt: number | null, now: number, intervalMs: number = PULSE_INTERVAL_MS): boolean { + return lastPulseAt === null || now - lastPulseAt >= intervalMs; +} + +/** §12.2 *Failure*: what a pulse response means for the poller. A 401 ends the session — stop; any + * other non-2xx keeps the last values and retries on the next tick. */ +export function pulseOutcome(status: number): "apply" | "stop" | "keep" { + if (status >= 200 && status < 300) return "apply"; + if (status === 401) return "stop"; + return "keep"; +} diff --git a/packages/web/src/lib/request-memo.test.ts b/packages/web/src/lib/request-memo.test.ts new file mode 100644 index 0000000..7fd5c84 --- /dev/null +++ b/packages/web/src/lib/request-memo.test.ts @@ -0,0 +1,45 @@ +// Unit tests for the per-request memo behind getSessionUser (INNOBOX_SPEC.md §4.2 *Resolution*: +// at most once per request, never reused across requests). +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { requestMemo } from "./request-memo"; + +test("one request scope: the computation runs once, every caller shares its result", async () => { + const memo = requestMemo(); + const scope = {}; + let calls = 0; + const compute = async () => ++calls; + const [a, b, c] = await Promise.all([memo(scope, compute), memo(scope, compute), memo(scope, compute)]); + assert.deepEqual([a, b, c], [1, 1, 1]); + assert.equal(await memo(scope, compute), 1, "a later call in the same request still shares it"); + assert.equal(calls, 1); +}); + +test("two requests never share a resolution — a role change is visible on the very next request", async () => { + const memo = requestMemo(); + let role = "member"; + const compute = async () => role; + assert.equal(await memo({}, compute), "member"); + role = "namespace_admin"; + assert.equal(await memo({}, compute), "namespace_admin"); +}); + +test("no request scope (build, tests): nothing is memoized", async () => { + const memo = requestMemo(); + let calls = 0; + await memo(null, async () => ++calls); + await memo(null, async () => ++calls); + assert.equal(calls, 2); +}); + +test("a failed resolution is not pinned for the rest of the request", async () => { + const memo = requestMemo(); + const scope = {}; + let calls = 0; + await assert.rejects(memo(scope, async () => { + calls++; + throw new Error("db blip"); + })); + assert.equal(await memo(scope, async () => (calls++, "ok")), "ok"); + assert.equal(calls, 2); +}); diff --git a/packages/web/src/lib/request-memo.ts b/packages/web/src/lib/request-memo.ts new file mode 100644 index 0000000..dfaee7c --- /dev/null +++ b/packages/web/src/lib/request-memo.ts @@ -0,0 +1,20 @@ +// A memo whose lifetime is ONE request (INNOBOX_SPEC.md §4.2 *Resolution*: "at most once per +// request"). Keyed on a per-request scope object — in production the request's headers object, +// which Next hands out once per request — and held in a WeakMap, so the entry dies with the +// request and nothing is ever reused across requests. A rejected computation is evicted, so a +// transient failure is not pinned for the rest of the request. + +export function requestMemo(): (scope: object | null, compute: () => Promise) => Promise { + const entries = new WeakMap>(); + return (scope, compute) => { + if (scope === null) return compute(); // no request scope (build, tests): never memoize + const hit = entries.get(scope); + if (hit) return hit; + const pending = compute(); + entries.set(scope, pending); + pending.catch(() => { + if (entries.get(scope) === pending) entries.delete(scope); + }); + return pending; + }; +} diff --git a/packages/web/src/lib/role-resolution.dbtest.ts b/packages/web/src/lib/role-resolution.dbtest.ts new file mode 100644 index 0000000..4f1e12a --- /dev/null +++ b/packages/web/src/lib/role-resolution.dbtest.ts @@ -0,0 +1,149 @@ +// Live-DB integration test (gated) for role resolution in ONE statement (INNOBOX_SPEC.md §4.2 +// *Resolution*, ENTRA_AUTH_SPEC.md "Role staleness", invariant 1): +// - the session row and its roles come back from a single round trip — grants from SCIM-synced +// group membership + role_mappings, the bootstrap group's implicit platform_admin while it is +// set, the §4.3 `people` access list, and the implicit `global` membership; +// - the bulk resolver returns the same RoleSet for each id, in one round trip whatever the count, +// and an id with no user row still gets the implicit-member RoleSet; +// - a role or access change is visible on the next resolution (nothing is cached); +// - an unknown oid resolves to null. +// Self-skips when DATABASE_URL is unset. +import { test } from "node:test"; +import assert from "node:assert/strict"; + +const url = process.env.DATABASE_URL; + +test( + "§4.2 role resolution: one statement per resolution, same grants as before, nothing cached", + { skip: url ? false : "DATABASE_URL not set — live-DB suite self-skips" }, + async () => { + const { Pool } = await import("pg"); + const { randomUUID } = await import("node:crypto"); + const { loadSessionRow, resolveRoleSets, bootstrapAdminGroup } = await import("./role-resolution"); + + const pool = new Pool({ connectionString: url }); + // Counts round trips without changing behaviour. + let roundTrips = 0; + const counted = { + query: (...args: unknown[]) => { + roundTrips++; + return (pool.query as (...a: unknown[]) => unknown)(...args); + }, + } as unknown as import("pg").Pool; + + try { + const stamp = randomUUID().slice(0, 8); + const { rows: g } = await pool.query<{ id: string }>(`select id from namespaces where slug = 'global'`); + const globalId = g[0]!.id; + const { rows: ns } = await pool.query<{ id: string }>(`insert into namespaces (slug, display_name) values ($1, 'Dbtest Roles NS') returning id`, [ + `dbtest-roles-${stamp}`, + ]); + const nsId = ns[0]!.id; + const { rows: ia } = await pool.query<{ id: string }>(`select id from impact_areas where active and name <> 'Client' limit 1`); + + const mkUser = async (label: string) => { + const oid = `dbtest-roles-${label}-${stamp}`; + const { rows } = await pool.query<{ id: string }>( + `insert into users (external_id, user_name, display_name, email) values ($1, $2, $3, $4) returning id`, + [oid, `${oid}@example.test`, `Dbtest Roles ${label}`, `roles-${label}-${stamp}@example.test`], + ); + return { id: rows[0]!.id, oid }; + }; + const mkGroup = async (label: string) => { + const ext = `dbtest-roles-grp-${label}-${stamp}`; + const { rows } = await pool.query<{ id: string }>(`insert into groups (external_id, display_name) values ($1, $2) returning id`, [ext, ext]); + return { id: rows[0]!.id, ext }; + }; + const join = (groupId: string, userId: string) => pool.query(`insert into group_members (group_id, user_id) values ($1, $2)`, [groupId, userId]); + + const admins = await mkGroup("nsadmins"); + const committee = await mkGroup("committee"); + const boot = await mkGroup("bootstrap"); + await pool.query(`insert into role_mappings (group_external_id, role, namespace_id) values ($1, 'namespace_admin', $2)`, [admins.ext, nsId]); + await pool.query(`insert into role_mappings (group_external_id, role, namespace_id) values ($1, 'committee', $2)`, [committee.ext, nsId]); + // The bootstrap group has NO role mapping: it grants platform_admin only through the env. + + const alice = await mkUser("alice"); // namespace admin + committee, on one access list + const bob = await mkUser("bob"); // bootstrap group member only + const carol = await mkUser("carol"); // nothing at all + await join(admins.id, alice.id); + await join(committee.id, alice.id); + await join(boot.id, bob.id); + + const { rows: ch } = await pool.query<{ id: string }>( + `insert into challenges (namespace_id, visibility, title, description, impact_area_id, author_id, status) + values ($1, 'people', $2, 'roles fixture', $3, $4, 'valid') returning id`, + [nsId, `Roles ${stamp}`, ia[0]!.id, carol.id], + ); + const challengeId = ch[0]!.id; + await pool.query(`insert into challenge_access (challenge_id, user_id) values ($1, $2)`, [challengeId, alice.id]); + + // ── the session row: one round trip, everything on it ─────────────────────────────── + roundTrips = 0; + const a = await loadSessionRow(counted, alice.oid, null); + assert.equal(roundTrips, 1, "the user row and its roles are ONE statement"); + assert.ok(a); + assert.equal(a.id, alice.id); + assert.equal(a.active, true); + assert.equal(a.roles.isPlatformAdmin, false); + assert.equal(a.roles.isNamespaceAdmin(nsId), true); + assert.equal(a.roles.isCommittee(nsId), true); + assert.equal(a.roles.isMemberOf(globalId), true, "every user is an implicit member of global"); + assert.deepEqual(a.roles.accessListChallenges(), [challengeId]); + assert.equal(a.roles.isOnAccessList(challengeId), true); + + // The bootstrap group counts only while the env names it. + roundTrips = 0; + const bOff = await loadSessionRow(counted, bob.oid, null); + const bOn = await loadSessionRow(counted, bob.oid, boot.ext); + assert.equal(roundTrips, 2, "one statement each, bootstrap or not"); + assert.equal(bOff!.roles.isPlatformAdmin, false); + assert.equal(bOn!.roles.isPlatformAdmin, true); + assert.equal(bootstrapAdminGroup({}), null); + assert.equal(bootstrapAdminGroup({ INNOBOX_BOOTSTRAP_ADMIN_GROUP: "" }), null); + assert.equal(bootstrapAdminGroup({ INNOBOX_BOOTSTRAP_ADMIN_GROUP: boot.ext }), boot.ext); + + const c = await loadSessionRow(pool, carol.oid, null); + assert.deepEqual( + c!.roles.grants.map((x) => x.role), + ["member"], + "no grants → just the implicit global membership", + ); + assert.equal(await loadSessionRow(pool, `dbtest-roles-nobody-${stamp}`, null), null); + + // A deactivated row still loads — the caller (lib/auth.ts) applies the active check. + await pool.query(`update users set active = false where id = $1`, [carol.id]); + assert.equal((await loadSessionRow(pool, carol.oid, null))!.active, false); + + // ── bulk: one round trip for many ids, identical RoleSets ──────────────────────────── + const ghost = randomUUID(); // no user row at all + roundTrips = 0; + const many = await resolveRoleSets(counted, [alice.id, bob.id, carol.id, ghost, alice.id], boot.ext); + assert.equal(roundTrips, 1, "bulk resolution is ONE statement whatever the count"); + assert.equal(many.size, 4, "deduplicated, and every id gets an entry"); + const describe = (r: import("@innobox/shared").RoleSet) => ({ + pa: r.isPlatformAdmin, + nsAdmin: r.isNamespaceAdmin(nsId), + committee: r.isCommittee(nsId), + access: r.accessListChallenges(), + global: r.isMemberOf(globalId), + }); + assert.deepEqual(describe(many.get(alice.id)!), describe(a.roles)); + assert.deepEqual(describe(many.get(bob.id)!), describe(bOn!.roles)); + assert.deepEqual(describe(many.get(ghost)!), { pa: false, nsAdmin: false, committee: false, access: [], global: true }); + roundTrips = 0; + assert.equal((await resolveRoleSets(counted, [], null)).size, 0); + assert.equal(roundTrips, 0, "an empty list costs nothing"); + + // ── nothing is cached: a change shows on the next resolution ───────────────────────── + await pool.query(`delete from group_members where group_id = $1 and user_id = $2`, [admins.id, alice.id]); + await pool.query(`delete from challenge_access where challenge_id = $1 and user_id = $2`, [challengeId, alice.id]); + const after = await loadSessionRow(pool, alice.oid, null); + assert.equal(after!.roles.isNamespaceAdmin(nsId), false); + assert.equal(after!.roles.isCommittee(nsId), true); + assert.deepEqual(after!.roles.accessListChallenges(), []); + } finally { + await pool.end(); + } + }, +); diff --git a/packages/web/src/lib/role-resolution.ts b/packages/web/src/lib/role-resolution.ts new file mode 100644 index 0000000..0fa3584 --- /dev/null +++ b/packages/web/src/lib/role-resolution.ts @@ -0,0 +1,135 @@ +// Role resolution in ONE statement (INNOBOX_SPEC.md §4.2 *Resolution*, ENTRA_AUTH_SPEC.md "Role +// staleness"). Roles come from SCIM-synced group membership + role_mappings on every request, never +// from token claims (invariant 1), and nothing here is cached across requests. What this module +// bounds is the cost: the grants, the bootstrap-group implicit mapping, the §4.3 `people` access +// list and the built-in `global` namespace id are read in a single round trip — as arrays on one +// row per user — where they used to be three or four sequential queries. +// +// Kept free of next/* and next-auth so the gated `.dbtest.ts` suites can run it under the plain +// node test runner; lib/auth.ts is the session-facing wrapper. +import type { Pool, PoolClient } from "pg"; +import { buildRoleSet, type Role, type RoleGrant, type RoleSet } from "@innobox/shared"; + +type Db = Pool | PoolClient; + +/** The per-user role columns, correlated on `uid` (a uuid expression in the enclosing query). + * `$bootstrap` is the parameter holding INNOBOX_BOOTSTRAP_ADMIN_GROUP (null when unset). The + * global namespace is an uncorrelated scalar subquery — Postgres evaluates it once per statement. */ +function roleColumns(uid: string, bootstrap: string): string { + return ` + coalesce(( + select json_agg(json_build_object('role', rm.role, 'namespace_id', rm.namespace_id)) + from group_members gm + join groups g on g.id = gm.group_id + join role_mappings rm on rm.group_external_id = g.external_id + where gm.user_id = ${uid} + ), '[]'::json) as grants, + (${bootstrap}::text is not null and exists ( + select 1 from group_members gm + join groups g on g.id = gm.group_id + where gm.user_id = ${uid} and g.external_id = ${bootstrap}::text + )) as bootstrap_admin, + coalesce(( + select array_agg(ca.challenge_id::text order by ca.challenge_id) from challenge_access ca where ca.user_id = ${uid} + ), '{}'::text[]) as access, + (select ns.id from namespaces ns where ns.slug = 'global') as global_namespace_id`; +} + +interface RoleRow { + grants: { role: Role; namespace_id: string | null }[]; + bootstrap_admin: boolean; + access: string[]; + global_namespace_id: string | null; +} + +function toRoleSet(row: RoleRow): RoleSet { + if (!row.global_namespace_id) throw new Error("built-in 'global' namespace missing — are migrations applied?"); + const grants: RoleGrant[] = row.grants.map((g) => ({ role: g.role, namespaceId: g.namespace_id })); + return buildRoleSet(grants, { + bootstrapPlatformAdmin: row.bootstrap_admin, + globalNamespaceId: row.global_namespace_id, + accessListChallengeIds: row.access, + }); +} + +/** Resolves a RoleSet for each of many user ids in ONE statement, whatever the count — notification + * dispatch visibility-checks every candidate recipient of an event at once. Every id gets an entry + * (driven by the id list, not the users table); a user with no grants gets the implicit-member + * RoleSet. */ +export async function resolveRoleSets(db: Db, userIds: readonly string[], bootstrapGroup: string | null): Promise> { + const ids = [...new Set(userIds)]; + if (ids.length === 0) return new Map(); + const { rows } = await db.query( + `select ids.id::text as user_id, ${roleColumns("ids.id", "$2")} + from unnest($1::uuid[]) as ids(id)`, + [ids, bootstrapGroup], + ); + return new Map(rows.map((r) => [r.user_id, toRoleSet(r)])); +} + +/** The session-facing user row, before the active / revocation checks lib/auth.ts applies. */ +export interface SessionRow { + id: string; + externalId: string; + userName: string; + email: string | null; + displayName: string; + active: boolean; + sessionsValidFrom: Date | null; + emailNotificationsEnabled: boolean; + quickStartSeenAt: Date | null; + whatsNewSeenVersion: string | null; + whatsNewSinceDate: string | null; + roles: RoleSet; +} + +/** The user row by Entra `oid` AND its roles, in one statement (§4.2). Null when no row exists. */ +export async function loadSessionRow(db: Db, oid: string, bootstrapGroup: string | null): Promise { + const { rows } = await db.query< + RoleRow & { + id: string; + external_id: string; + user_name: string; + email: string | null; + display_name: string; + active: boolean; + sessions_valid_from: Date | null; + email_notifications_enabled: boolean; + quick_start_seen_at: Date | null; + whats_new_seen_version: string | null; + whats_new_since_date: string | null; + } + >( + // The rollout boundary is a bare calendar date: read it as text, never through a JS Date, + // which would shift it by the server's offset. + `select u.id, u.external_id, u.user_name, u.email, u.display_name, u.active, u.sessions_valid_from, + u.email_notifications_enabled, u.quick_start_seen_at, u.whats_new_seen_version, + to_char(u.whats_new_since_date, 'YYYY-MM-DD') as whats_new_since_date, + ${roleColumns("u.id", "$2")} + from users u + where u.external_id = $1`, + [oid, bootstrapGroup], + ); + const row = rows[0]; + if (!row) return null; + return { + id: row.id, + externalId: row.external_id, + userName: row.user_name, + email: row.email, + displayName: row.display_name, + active: row.active, + sessionsValidFrom: row.sessions_valid_from, + emailNotificationsEnabled: row.email_notifications_enabled, + quickStartSeenAt: row.quick_start_seen_at, + whatsNewSeenVersion: row.whats_new_seen_version, + whatsNewSinceDate: row.whats_new_since_date, + roles: toRoleSet(row), + }; +} + +/** INNOBOX_BOOTSTRAP_ADMIN_GROUP, or null when unset/blank — read per call, like before. */ +export function bootstrapAdminGroup(env: Record = process.env): string | null { + const v = env.INNOBOX_BOOTSTRAP_ADMIN_GROUP; + return v ? v : null; +}