From 38831923e10b4f183f7279272eb2934da06a9568 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 03:13:14 +0000 Subject: [PATCH 1/2] fix(app-shell): the bell's Approvals and Activity tabs fill in off-app, from one shared fetch (#4197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AppHeader` gated the pending-approvals poll and the `sys_activity` read on `variant === 'app'`. The bell renders on Home, Organizations and the full-page AI screen too, so off-app both tabs were permanently empty — on the very page whose own To-do and activity cards (`useHomeInbox`, ungated) were listing the same rows from the same endpoint and the same object. Since #4199 un-gated the inbox half, the badge (`unreadTopics + pendingApprovalsCount`) fetched only its first addend off-app: one user with one set of data read 1 on Home and 3 inside an app. Neither feed is app-scoped — approvals are scoped to the user, activity to the tenant — so both are un-gated. To avoid paying for that with duplicate reads (on `/home` the bell and the cards mount in one tree), a module-scoped store `hooks/sharedUserFeeds` now owns each feed: one in-flight request, one 30s approvals poll, one 404-retires-the-feature rule, with the bell and `useHomeInbox` both subscribing. Home keeps its narrower cut of the activity rows (human actors only) by filtering the shared feed at its call site. Presence stays app-scoped: the avatars and the connection dot remain behind `isApp`, and presence was never a read at all — it is a transport-level subscription (`useTenantPresence`), which is why the effect formerly named `fetchPresenceAndActivities` only ever fetched `sys_activity`. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- .../bell-approvals-activity-off-app-4197.md | 13 + .../app-shell/src/hooks/sharedUserFeeds.ts | 383 ++++++++++++++++++ packages/app-shell/src/hooks/useHomeInbox.ts | 97 +---- packages/app-shell/src/layout/AppHeader.tsx | 182 ++------- .../__tests__/AppHeader.inboxVariant.test.tsx | 269 +++++++++++- 5 files changed, 721 insertions(+), 223 deletions(-) create mode 100644 .changeset/bell-approvals-activity-off-app-4197.md create mode 100644 packages/app-shell/src/hooks/sharedUserFeeds.ts diff --git a/.changeset/bell-approvals-activity-off-app-4197.md b/.changeset/bell-approvals-activity-off-app-4197.md new file mode 100644 index 0000000000..8f140601d4 --- /dev/null +++ b/.changeset/bell-approvals-activity-off-app-4197.md @@ -0,0 +1,13 @@ +--- +'@object-ui/app-shell': patch +--- + +The bell's Approvals and Activity tabs fill in on Home, Organizations and the AI screen — from the same fetch the cards below them already use + +The top bar's bell renders on every console surface, but two of the three streams that fill it were gated on `variant === 'app'`: the pending-approvals poll and the `sys_activity` read. Off-app the popover therefore held nothing to show — the Approvals tab read "No pending approvals" and the Activity tab "No recent activity" — on the very page whose own To-do and activity cards were listing both, from the same endpoint and the same object. Neither stream is app-scoped: approvals are scoped to the signed-in user, activity to the tenant. Whichever app happened to be in the URL was never part of either query. + +The badge made the inconsistency arithmetic. It is `unreadTopics + pendingApprovalsCount`, and after objectui#4199 un-gated the inbox half, only the first addend was fetched outside an app — so one user with one set of data read 1 on Home and 3 inside an app, and the popover's own breakdown line disagreed with the number on the bell. + +Un-gating alone would have fixed the emptiness by paying for it twice: on `/home` the bell and the cards mount in one tree, so each owning its own effect means the approvals request and the `sys_activity` read both go out twice per page. They now share one fetch. A module-scoped store (`hooks/sharedUserFeeds`) owns each feed — one in-flight request, one 30s approvals poll, one 404-retires-the-feature rule — and both the bell and `useHomeInbox` subscribe to it. The dedupe is structural rather than agreed: there is no longer a second producer that could drift, so the badge and the card cannot show different numbers. Home's card keeps its narrower cut of the rows (human actors only, `sys_*`/`ai_*` churn dropped) by filtering the shared feed at its own call site rather than by issuing its own query. + +`isApp` keeps the meaning it was introduced for. The presence avatars and the connection dot are app-shell chrome and stay behind it — and presence was never a read to begin with: it is a transport-level subscription (`useTenantPresence`), which is why the effect that used to be called `fetchPresenceAndActivities` only ever fetched `sys_activity`. The boundary this draws is data scope, not surface: user- and tenant-scoped feeds follow the bell everywhere it renders, app-scoped chrome does not. diff --git a/packages/app-shell/src/hooks/sharedUserFeeds.ts b/packages/app-shell/src/hooks/sharedUserFeeds.ts new file mode 100644 index 0000000000..96ed95a28b --- /dev/null +++ b/packages/app-shell/src/hooks/sharedUserFeeds.ts @@ -0,0 +1,383 @@ +/** + * sharedUserFeeds — ONE fetch per user-scoped feed, however many consumers mount + * + * Two console surfaces read the same two user-scoped streams: + * + * | feed | producer | consumers | + * | ------------------------ | ------------------------------------------ | ---------------------------------- | + * | pending approvals count | `GET /api/v1/approvals/requests?status=…` | AppHeader bell badge + Approvals | + * | | | tab; Home's To-do card | + * | recent activity | `find('sys_activity', top 20, desc)` | AppHeader bell Activity tab; | + * | | | Home's activity card | + * + * Both consumers live in this package and, on `/home`, mount in the same tree + * (`HomeLayout` renders the bell, `HomePage` renders the cards) — so each of + * them owning its own effect meant the same read went out twice per page. That + * is exactly the trade-off #4197 refused to accept as the price of un-gating + * the bell: the fix is one fetch feeding both, not two fetches agreeing. + * + * Neither feed is app-scoped, so neither is gated on the header's `isApp` + * flag. `isApp` still means something — it hides genuinely app-shell chrome + * (presence avatars, the connection dot) — but the approvals inbox and the + * activity feed are scoped to the *user* and the *tenant*, not to whichever + * app happens to be in the URL. Gating them there is what left the bell's + * Approvals and Activity tabs permanently empty on Home / Organizations / the + * full-page AI screen, and what made the badge (`unread + approvals`) read a + * different number on Home than inside an app for the same user. + * + * Why a module-scoped store rather than a context provider: both consumers are + * already inside `@object-ui/app-shell`, so sharing needs no new dependency + * edge — and a store needs no provider mounted above every call site, so the + * one-fetch guarantee holds no matter where a consumer is rendered (the bell + * is mounted by four different layouts). The dedupe is structural: consumers + * cannot opt out of it by mounting somewhere unexpected. + * + * @module + */ +import { useEffect, useMemo, useRef, useSyncExternalStore } from 'react'; +import { useAuth } from '@object-ui/auth'; +import { errorCodeIs } from '@object-ui/types'; +// Re-exported from `@object-ui/react` — import it through the provider module +// so a consumer that stubs the provider stubs this too. +import { useAdapter } from '../providers/AdapterProvider'; +import { bearerAuthHeaders } from '../utils/authToken'; +import type { ActivityItem } from '../layout/ActivityFeed'; + +/** Approvals poll cadence — the bell's original 30s (M11.C15). */ +const APPROVALS_POLL_MS = 30_000; +/** + * How long a fetched value stays authoritative. It is the dedupe window: a + * second consumer mounting inside it is served the cached value instead of + * issuing its own read, which covers the common case where the header mounts + * a beat before the page body does. + */ +const FRESH_MS = 30_000; + +/** + * Stable empty value — `useSyncExternalStore` re-renders in a loop if + * `getSnapshot` hands back a fresh reference each call, so the "nothing yet" + * value must be one shared array (cf. `EMPTY_PRESENCE_USERS` in AppHeader). + */ +const NO_ACTIVITIES: ActivityItem[] = []; + +/** + * The runner produces the feed's next value, or `undefined` to leave the last + * one in place (a transient error, a non-OK response). `markUnavailable()` + * retires the feed for the rest of the page — the deployment does not have the + * approvals plugin / the `sys_activity` object, so retrying is pure noise. + */ +type FeedRunner = (ctx: { markUnavailable: () => void }) => Promise; + +/** + * One feed's shared state. Consumers `attach` (from an effect) and read via + * `useSyncExternalStore`; the first one in starts the fetch and the poll, the + * last one out stops it. + */ +class SharedFeed { + private value: T; + private key: string | null = null; + private readonly listeners = new Set<() => void>(); + private runner: FeedRunner | null = null; + private consumers = 0; + private inFlight = false; + private unavailable = false; + private fetchedAt = 0; + private timer: ReturnType | null = null; + + constructor( + private readonly empty: T, + /** Re-fetch cadence while at least one consumer is mounted; 0 = fetch once. */ + private readonly pollMs: number, + ) { + this.value = empty; + } + + subscribe = (onStoreChange: () => void): (() => void) => { + this.listeners.add(onStoreChange); + return () => { + this.listeners.delete(onStoreChange); + }; + }; + + getSnapshot = (): T => this.value; + + /** + * Register a consumer. `key` identifies *whose* feed this is (the approver + * identity list / the adapter instance); a different key means the previous + * value belongs to someone else and is dropped rather than shown. + * + * Every consumer of a given feed derives its key from the same auth/adapter + * context, so concurrent consumers always agree on it. + */ + attach(key: string, runner: FeedRunner): () => void { + if (key !== this.key) { + this.key = key; + this.unavailable = false; + this.fetchedAt = 0; + this.publish(this.empty); + } + // Freshest closure wins — it holds the current adapter / identities. + this.runner = runner; + this.consumers += 1; + if (this.consumers === 1) this.schedule(); + void this.refresh(); + return () => { + this.consumers = Math.max(0, this.consumers - 1); + if (this.consumers === 0) this.stopPolling(); + }; + } + + /** + * `force` is the poll tick: it bypasses the freshness window (which exists + * to collapse mounts, not to defeat the cadence). Concurrent callers are + * collapsed by `inFlight`, which is set synchronously before the first + * `await` — so two consumers attaching in the same commit issue one read. + */ + private async refresh(force = false): Promise { + const runner = this.runner; + if (!runner || this.unavailable || this.inFlight) return; + if (!force && this.fetchedAt && Date.now() - this.fetchedAt < FRESH_MS) return; + this.inFlight = true; + try { + const next = await runner({ + markUnavailable: () => { + this.unavailable = true; + this.stopPolling(); + }, + }); + if (next !== undefined) { + this.fetchedAt = Date.now(); + this.publish(next); + } + } catch { + // Transient — keep the last value; the next poll / mount retries. + } finally { + this.inFlight = false; + } + } + + private schedule(): void { + if (this.pollMs <= 0 || this.unavailable || this.timer) return; + this.timer = setTimeout(() => { + this.timer = null; + void this.refresh(true).finally(() => { + if (this.consumers > 0) this.schedule(); + }); + }, this.pollMs); + } + + private stopPolling(): void { + if (this.timer) { + clearTimeout(this.timer); + this.timer = null; + } + } + + private publish(next: T): void { + if (Object.is(next, this.value)) return; + this.value = next; + for (const listener of [...this.listeners]) listener(); + } + + /** Test seam — drop all cached state between cases. Listeners are left alone. */ + reset(): void { + this.stopPolling(); + this.value = this.empty; + this.key = null; + this.runner = null; + this.consumers = 0; + this.inFlight = false; + this.unavailable = false; + this.fetchedAt = 0; + } +} + +/** + * Subscribe to a shared feed. A `null` key means "nothing to fetch yet" (no + * signed-in user, no adapter) — the consumer still reads the snapshot, it just + * does not drive a fetch. + */ +function useSharedFeed(feed: SharedFeed, key: string | null, runner: FeedRunner): T { + const value = useSyncExternalStore(feed.subscribe, feed.getSnapshot, feed.getSnapshot); + // Latest-ref: the runner closes over values that change every render, but + // only `key` may re-drive the attach effect. Declared first so it lands + // before the attach effect on every commit. + const runnerRef = useRef(runner); + useEffect(() => { + runnerRef.current = runner; + }); + useEffect(() => { + if (!key) return; + return feed.attach(key, (ctx) => runnerRef.current(ctx)); + }, [feed, key]); + return value; +} + +// ── Pending approvals ──────────────────────────────────────────────────────── + +const approvalsFeed = new SharedFeed(0, APPROVALS_POLL_MS); + +/** + * The identities the endpoint matches a pending approver against: the user id, + * their email, and `role:` for each role. Sent as one comma-separated + * `approverId` so this is ONE request rather than one per identity. + */ +function approverIdentities(user: unknown): string[] { + const u = user as { id?: string; email?: string; roles?: string[] } | null | undefined; + const identities: string[] = []; + if (u?.id) identities.push(u.id); + if (u?.email) identities.push(u.email); + for (const role of u?.roles ?? []) if (role) identities.push(`role:${role}`); + return identities; +} + +/** + * Count of approval requests waiting on the signed-in user. + * + * Feeds the bell's badge (second addend of `unread + approvals`) and its + * Approvals tab, and Home's To-do card — from one polled request. Degrades to + * 0 on 404 (approvals plugin not installed) and retires the poll. + */ +export function useSharedPendingApprovalsCount(): number { + const { user } = useAuth(); + const identities = approverIdentities(user); + // `user?.id` is the sign-in gate; identities is what the query needs. + const key = user?.id && identities.length > 0 ? identities.join(',') : null; + + return useSharedFeed(approvalsFeed, key, async ({ markUnavailable }) => { + const serverUrl = (import.meta.env?.VITE_SERVER_URL || '').replace(/\/$/, ''); + const qs = new URLSearchParams({ status: 'pending', approverId: identities.join(',') }); + const res = await fetch(`${serverUrl}/api/v1/approvals/requests?${qs}`, { + credentials: 'include', + // Bearer too — see utils/authToken (#2548 split-origin fix). + headers: bearerAuthHeaders(), + }); + if (res.status === 404) { + markUnavailable(); + return undefined; + } + if (!res.ok) return undefined; + const payload = await res.json().catch(() => null); + const seen = new Set(); + for (const row of (payload?.data || []) as { id: string }[]) seen.add(row.id); + return seen.size; + }); +} + +// ── Recent activity ────────────────────────────────────────────────────────── + +const activityFeed = new SharedFeed(NO_ACTIVITIES, 0); + +/** + * Stable string id per adapter instance, so swapping the adapter (tenant + * switch) drops the previous tenant's rows instead of serving them from cache. + */ +const adapterKeys = new WeakMap(); +let adapterSeq = 0; +function adapterKey(adapter: unknown): string | null { + if (!adapter || typeof adapter !== 'object') return null; + let key = adapterKeys.get(adapter as object); + if (!key) { + key = `sys_activity@${++adapterSeq}`; + adapterKeys.set(adapter as object, key); + } + return key; +} + +/** + * Raw `sys_activity` rows carry plugin-audit's column names + * (`summary` / `actor_name` / `object_name` / `timestamp`); casting them + * straight through leaves every `ActivityItem` field undefined, which is what + * once rendered the Activity tab as blank rows showing only a relative time. + * + * This is the shared superset: rows that name an action and say something. + * Home narrows it further (human actors only) at its own call site. + */ +function mapActivityRows(rows: unknown[]): ActivityItem[] { + return rows + .filter((row): row is Record => { + if (!row || typeof row !== 'object') return false; + const r = row as Record; + return typeof r.type === 'string' && String(r.summary ?? '').trim().length > 0; + }) + .map((r) => { + let when = r.timestamp as string | undefined; + if (!when || when === 'NOW()' || Number.isNaN(Date.parse(when))) { + when = r.created_at as string | undefined; + } + const raw = String(r.type); + const type: ActivityItem['type'] = + raw === 'commented' || raw === 'mentioned' + ? 'comment' + : raw === 'deleted' + ? 'delete' + : raw === 'created' + ? 'create' + : 'update'; + return { + id: String(r.id), + type, + objectName: String(r.object_name ?? ''), + recordId: r.record_id != null ? String(r.record_id) : undefined, + user: String(r.actor_name ?? ''), + description: String(r.summary ?? ''), + timestamp: when ?? '', + }; + }); +} + +/** The ObjectStack client throws `httpStatus` (not `status`) with an error code. */ +function isMissingResource(err: unknown): boolean { + const e = err as { httpStatus?: number; status?: number } | null; + return e?.httpStatus === 404 || e?.status === 404 || errorCodeIs(err, 'OBJECT_NOT_FOUND'); +} + +/** + * The 20 most recent activity rows, tenant-wide, mapped onto `ActivityItem`. + * + * Not polled — it is a landing-surface feed on both consumers, and the bell + * never polled it either. Degrades to empty when `sys_activity` is absent + * (no plugin-audit) and retires the feed for the rest of the page. + */ +export function useSharedActivityFeed(): ActivityItem[] { + const dataSource = useAdapter(); + + return useSharedFeed(activityFeed, adapterKey(dataSource), async ({ markUnavailable }) => { + if (!dataSource) return undefined; + const res = await Promise.resolve( + dataSource.find('sys_activity', { $orderby: { timestamp: 'desc' }, $top: 20 }) as Promise<{ + data?: unknown[]; + }>, + ).catch((err: unknown) => { + if (isMissingResource(err)) markUnavailable(); + return null; + }); + if (!res) return undefined; + return mapActivityRows(Array.isArray(res.data) ? res.data : []); + }); +} + +/** + * Home's narrower cut of the same rows: real human actions only — drop the + * `sys_*` / `ai_*` system churn (actor "System", UUID titles) that the bell's + * full feed still shows — capped at `limit`. + */ +export function useHumanActivityFeed(limit: number): ActivityItem[] { + const all = useSharedActivityFeed(); + return useMemo(() => { + const human = all.filter((a) => { + const actor = a.user.trim(); + return actor.length > 0 && actor.toLowerCase() !== 'system'; + }); + return human.slice(0, limit); + }, [all, limit]); +} + +/** + * Test seam: drop every shared feed's cached value, key and in-flight state so + * cases do not inherit each other's reads. Not part of the public surface. + */ +export function __resetSharedUserFeeds(): void { + approvalsFeed.reset(); + activityFeed.reset(); +} diff --git a/packages/app-shell/src/hooks/useHomeInbox.ts b/packages/app-shell/src/hooks/useHomeInbox.ts index d7ef687bb0..f96b8046fa 100644 --- a/packages/app-shell/src/hooks/useHomeInbox.ts +++ b/packages/app-shell/src/hooks/useHomeInbox.ts @@ -1,23 +1,29 @@ /** * useHomeInbox * - * One-shot fetch of the inbox streams the Home work-dashboard surfaces: + * The inbox streams the Home work-dashboard surfaces: * - pendingApprovalsCount — items waiting on the user (REST endpoint) * - notifications — latest in-app inbox messages (assignments/@mentions) * - activities — recent human activity feed (sys_activity) * * Everything degrades silently to empty on 404 / error so deployments without * the approvals plugin, the inbox pipeline, or a `sys_activity` object still - * render Home. Unlike the top-bar bell (AppHeader) this does NOT poll — Home is - * a landing surface, one fetch on mount is enough; the bell stays the live - * source of truth. Query shapes mirror AppHeader so the two never diverge. + * render Home. + * + * Approvals and activity are NOT fetched here (#4197). Both come from + * `sharedUserFeeds`, which the top-bar bell reads too — on `/home` the bell and + * these cards mount in one tree, so two owners meant the same read went out + * twice per page. One fetch now feeds both, which is also what makes the bell's + * badge and this card structurally incapable of showing different numbers. + * What is still fetched here is the inbox-message list, whose query is Home's + * own (top-`limit` titles, no read-state receipts). * * @module */ import { useEffect, useRef, useState } from 'react'; import { useAdapter } from '../providers/AdapterProvider'; import { useAuth } from '@object-ui/auth'; -import { bearerAuthHeaders } from '../utils/authToken'; +import { useHumanActivityFeed, useSharedPendingApprovalsCount } from './sharedUserFeeds'; import type { ActivityItem } from '../layout/ActivityFeed'; export interface HomeNotification { @@ -36,62 +42,20 @@ export interface HomeInboxData { export function useHomeInbox(limit = 5): HomeInboxData { const dataSource = useAdapter(); const { user } = useAuth(); - const [pendingApprovalsCount, setPendingApprovalsCount] = useState(0); const [notifications, setNotifications] = useState([]); - const [activities, setActivities] = useState([]); const mountedRef = useRef(true); + // Shared with the top-bar bell — one read each, not one per consumer (#4197). + // `useHumanActivityFeed` is Home's narrower cut of the bell's rows: real + // human actions only, dropping the sys_*/ai_* churn (actor "System"). + const pendingApprovalsCount = useSharedPendingApprovalsCount(); + const activities = useHumanActivityFeed(limit); + useEffect(() => { mountedRef.current = true; return () => { mountedRef.current = false; }; }, []); - // Recent activity (sys_activity). Raw rows use plugin-audit's column names - // (actor_name / summary / object_name / timestamp); map onto ActivityItem and - // keep only real human actions — drop sys_*/ai_* system churn (UUID-titled, - // actor "System"). Degrades to [] if the object is absent. - useEffect(() => { - if (!dataSource) return; - let cancelled = false; - Promise.resolve( - dataSource.find('sys_activity', { $orderby: { timestamp: 'desc' }, $top: 20 }) as Promise, - ) - .then((res) => { - if (cancelled || !mountedRef.current) return; - const rows: any[] = Array.isArray(res?.data) ? res.data : []; - const mapped: ActivityItem[] = rows - .filter((r) => { - if (!r || typeof r.type !== 'string') return false; - if (!(r.summary ?? '').toString().trim()) return false; - const actor = String(r.actor_name ?? '').trim(); - return actor.length > 0 && actor.toLowerCase() !== 'system'; - }) - .map((r) => { - let when = r.timestamp; - if (!when || when === 'NOW()' || Number.isNaN(Date.parse(when))) when = r.created_at; - const raw = String(r.type); - const type: ActivityItem['type'] = - raw === 'commented' || raw === 'mentioned' ? 'comment' - : raw === 'deleted' ? 'delete' - : raw === 'created' ? 'create' - : 'update'; - return { - id: String(r.id), - type, - objectName: r.object_name ?? '', - recordId: r.record_id ?? undefined, - user: r.actor_name ?? 'System', - description: r.summary ?? '', - timestamp: when ?? '', - }; - }) - .slice(0, limit); - setActivities(mapped); - }) - .catch(() => { /* missing / error → empty */ }); - return () => { cancelled = true; }; - }, [dataSource, limit]); - // Latest in-app inbox messages (assignments / @mentions / alerts). useEffect(() => { if (!dataSource || !user?.id) return; @@ -124,32 +88,5 @@ export function useHomeInbox(limit = 5): HomeInboxData { return () => { cancelled = true; }; }, [dataSource, user?.id, limit]); - // Pending-approvals count (framework REST endpoint). 404 / error → 0. - useEffect(() => { - if (!user?.id) return; - const serverUrl = (import.meta.env?.VITE_SERVER_URL || '').replace(/\/$/, ''); - const identities: string[] = []; - if (user.id) identities.push(user.id); - if ((user as any).email) identities.push((user as any).email); - for (const r of (((user as any).roles || []) as string[])) { if (r) identities.push(`role:${r}`); } - if (identities.length === 0) return; - let cancelled = false; - const qs = new URLSearchParams({ status: 'pending', approverId: identities.join(',') }); - fetch(`${serverUrl}/api/v1/approvals/requests?${qs}`, { - credentials: 'include', - // Bearer too — see utils/authToken (#2548 split-origin fix). - headers: bearerAuthHeaders(), - }) - .then(async (res) => { - if (!res.ok) return; - const payload = await res.json().catch(() => null); - const seen = new Set(); - for (const row of ((payload?.data || []) as { id: string }[])) seen.add(row.id); - if (!cancelled && mountedRef.current) setPendingApprovalsCount(seen.size); - }) - .catch(() => { /* transient / 404 → keep 0 */ }); - return () => { cancelled = true; }; - }, [user?.id]); - return { pendingApprovalsCount, notifications, activities }; } diff --git a/packages/app-shell/src/layout/AppHeader.tsx b/packages/app-shell/src/layout/AppHeader.tsx index 96e47e5f6d..45ba788035 100644 --- a/packages/app-shell/src/layout/AppHeader.tsx +++ b/packages/app-shell/src/layout/AppHeader.tsx @@ -75,6 +75,7 @@ import { useCommandPalette } from '../context/CommandPaletteProvider'; import { useUrlOverlay } from '../hooks/useUrlOverlay'; import { KEYBOARD_SHORTCUTS_PARAM, RECORD_TRAIL_PARAM, decodeRecordTrail, buildRecordTrailHref } from '../urlParams'; import { useAiSurfaceEnabled } from '../hooks/useAiSurface'; +import { useSharedActivityFeed, useSharedPendingApprovalsCount } from '../hooks/sharedUserFeeds'; import { getProductName, getLogoUrl } from '../runtime-config'; import { LocalizedSidebarTrigger } from './LocalizedSidebarTrigger'; import { PreviewBadge } from './PreviewBadge'; @@ -192,7 +193,18 @@ export function AppHeader({ } }, [helpDocs, dataSource]); - const [apiActivities, setApiActivities] = useState(null); + /** + * Recent activity for the bell's Activity tab (#4197). + * + * Read from the shared user-scoped feed rather than a local effect: Home's + * activity card reads the very same `sys_activity` rows, and on `/home` the + * bell and the card mount in one tree — so an effect here would have made + * that page issue the read twice. Also NOT gated on `isApp`: the feed is + * tenant-scoped, not app-scoped, and gating it left this tab reading "No + * recent activity" on Home / Organizations / the AI screen while the card + * two hundred pixels below listed the rows. + */ + const apiActivities = useSharedActivityFeed(); /** * In-header notifications (ADR-0030). Polled from `sys_inbox_message` (the L5 * in-app materialization, `mine` scope) joined with `sys_notification_receipt` @@ -219,7 +231,8 @@ export function AppHeader({ // Once the server returns 404 for these collections we stop retrying for // the lifetime of the page — they're optional features and re-requesting // on every navigation creates console noise + wasted round trips. - const activityUnavailableRef = useRef(false); + // (`sys_activity` and the approvals endpoint carry the same rule inside + // `sharedUserFeeds`, which retires a feed for every consumer at once.) const notificationsUnavailableRef = useRef(false); // Tracks whether the component is still mounted. Used by the pollers to @@ -235,83 +248,36 @@ export function AppHeader({ return () => { mountedRef.current = false; }; }, []); - // In-flight guards: during bootstrap the poller effects re-run several - // times as `dataSource` / `isApp` / `user.id` settle, and each run kicks - // an immediate fetch. Without these the same query fired 5× concurrently - // (nothing cached yet) and flooded the backend. They coalesce to one. + // In-flight guard: during bootstrap the poller effect re-runs several times + // as `dataSource` / `user.id` settle, and each run kicks an immediate fetch. + // Without it the same query fired 5× concurrently (nothing cached yet) and + // flooded the backend. It coalesces them to one. (The approvals and activity + // feeds carry the equivalent guard inside `sharedUserFeeds`, where it also + // collapses the *other* consumer's mount, not just this one's re-runs.) const notifInFlightRef = useRef(false); - const approvalsInFlightRef = useRef(false); - const activityInFlightRef = useRef(false); - - /** M11.C15: pending approvals count for the topbar shortcut. */ - const [pendingApprovalsCount, setPendingApprovalsCount] = useState(0); - const approvalsUnavailableRef = useRef(false); - const fetchPresenceAndActivities = useCallback(async () => { - if (!dataSource || !isApp) return; - // ObjectStack client throws Error objects with `httpStatus` (not `status`) - // and a `code` like `object_not_found` when the underlying object isn't - // registered on the server. Either signal means the feature is - // unavailable — disable it for the rest of the page. - const isMissingResource = (err: any): boolean => - err?.httpStatus === 404 || err?.status === 404 || errorCodeIs(err, 'OBJECT_NOT_FOUND'); - - // Tenant-wide presence ("who else is online?") is intentionally NOT - // probed here. Presence is real-time ephemeral state that does not - // belong in a regular REST collection. The feature is staged behind a - // transport-level provider () which is not yet - // wired — see ROADMAP for the realtime plan. - if (activityUnavailableRef.current) return; - // In-flight dedupe: this callback's identity changes as dataSource/isApp - // settle during bootstrap, re-firing the mount effect below; coalesce the - // immediate fetches into one instead of N (sys_activity fired 3×+). - if (activityInFlightRef.current) return; - activityInFlightRef.current = true; - try { - const activityResult = await dataSource - .find('sys_activity', { $orderby: { timestamp: 'desc' }, $top: 20 }) - .catch((err: any) => { - if (isMissingResource(err)) activityUnavailableRef.current = true; - return { data: [] as Record[] }; - }); - if (activityResult.data?.length) { - // Raw sys_activity rows use plugin-audit's column names - // (summary / actor_name / object_name / timestamp). Map them onto - // ActivityItem's shape (description / user / objectName) — casting the - // raw row straight through left every field undefined, so the popover - // Activity tab rendered blank rows (only the relative time showed). - // Mirrors the mapping in `useHomeInbox` so the bell and Home never diverge. - const items: ActivityItem[] = (activityResult.data as Record[]) - .filter((r) => typeof r.type === 'string' && String(r.summary ?? '').trim()) - .map((r) => { - let when = r.timestamp as string | undefined; - if (!when || when === 'NOW()' || Number.isNaN(Date.parse(when))) { - when = r.created_at as string | undefined; - } - const raw = String(r.type); - const type: ActivityItem['type'] = - raw === 'commented' || raw === 'mentioned' ? 'comment' - : raw === 'deleted' ? 'delete' - : raw === 'created' ? 'create' - : 'update'; - return { - id: String(r.id), - type, - objectName: (r.object_name as string) ?? '', - recordId: (r.record_id as string) ?? undefined, - user: (r.actor_name as string) ?? '', - description: (r.summary as string) ?? '', - timestamp: when ?? '', - }; - }); - if (items.length) setApiActivities(items); - } - } catch { /* fallback below */ } finally { - activityInFlightRef.current = false; - } - }, [dataSource, isApp]); + /** + * M11.C15: pending approvals count — the topbar shortcut, and the second + * addend of the bell badge (`unreadTopics + pendingApprovalsCount`). + * + * Shared with Home's To-do card (#4197): one polled request serves both, so + * the badge and the card can no longer disagree. Formerly a local effect + * gated on `isApp`, which meant the badge silently dropped this addend + * everywhere outside an app — the same user with the same data read 1 on + * Home and 3 inside an app. + */ + const pendingApprovalsCount = useSharedPendingApprovalsCount(); - useEffect(() => { fetchPresenceAndActivities(); }, [fetchPresenceAndActivities]); + /** + * Presence is the OTHER half of what this component used to fetch here, and + * it stays app-scoped (#4197). Tenant-wide presence ("who else is online?") + * is never *read* — it is not a REST collection but a transport-level + * subscription (`useTenantPresence`, ), and the avatars + * plus the connection dot render only under `isApp` below, which is the + * reason that flag exists. So un-gating the two user/tenant-scoped feeds + * above does not drag app-shell chrome off-app with them: the boundary is + * data scope, not surface. + */ /** * Poll the signed-in user's in-app inbox (ADR-0030 L5). @@ -443,66 +409,6 @@ export function AppHeader({ }; }, [dataSource, user?.id]); - /** - * M11.C15: poll pending-approvals count for the topbar shortcut badge. - * Hits the framework's `/api/v1/approvals/requests?status=pending` - * endpoint with the user's identities (id, email, role:). Degrades - * silently to zero on 404 (approvals plugin not installed). - * - * The endpoint accepts a comma-separated `approverId` and matches a - * request when ANY identity is a pending approver, so this issues ONE - * request per poll. (It previously looped one fetch per identity, firing - * N near-simultaneous calls every cycle — the dominant duplicate-request - * offender on the control plane. Requires framework with multi-approverId - * support; ship the framework + console SHA bumps together.) - */ - useEffect(() => { - if (!isApp || !user?.id) return; - if (approvalsUnavailableRef.current) return; - const serverUrl = (import.meta.env?.VITE_SERVER_URL || '').replace(/\/$/, ''); - const base = `${serverUrl}/api/v1/approvals/requests`; - const identities: string[] = []; - if (user.id) identities.push(user.id); - if ((user as any).email) identities.push((user as any).email); - for (const r of ((user as any).roles || []) as string[]) { - if (r) identities.push(`role:${r}`); - } - let cancelled = false; - let timer: ReturnType | null = null; - const POLL_MS = 30_000; - const fetchOnce = async () => { - if (identities.length === 0) return; - // In-flight dedupe: bootstrap re-runs this effect a few times; coalesce - // the immediate fetches into one instead of firing them concurrently. - if (approvalsInFlightRef.current) return; - approvalsInFlightRef.current = true; - try { - const qs = new URLSearchParams({ status: 'pending', approverId: identities.join(',') }); - const res = await fetch(`${base}?${qs}`, { - credentials: 'include', - // Bearer too — see utils/authToken (#2548 split-origin fix). - headers: bearerAuthHeaders(), - }); - if (res.status === 404) { approvalsUnavailableRef.current = true; return; } - if (!res.ok) return; - const payload = await res.json().catch(() => null); - const seen = new Set(); - for (const row of (payload?.data || []) as { id: string }[]) seen.add(row.id); - // Apply if still mounted (not gated on this run's `cancelled`, so the - // single in-flight fetch survives a bootstrap re-run mid-flight). - if (mountedRef.current) setPendingApprovalsCount(seen.size); - } catch { /* transient — keep last value */ } finally { - approvalsInFlightRef.current = false; - } - }; - const schedule = () => { - if (cancelled || approvalsUnavailableRef.current) return; - timer = setTimeout(async () => { await fetchOnce(); schedule(); }, POLL_MS); - }; - fetchOnce().finally(schedule); - return () => { cancelled = true; if (timer) clearTimeout(timer); }; - }, [isApp, user?.id]); - const unreadCount = notifications.reduce((n, x) => n + (x.is_read ? 0 : 1), 0); // Read-state lives in `sys_notification_receipt`, keyed @@ -558,7 +464,9 @@ export function AppHeader({ const tenantPresence = useTenantPresence(); const activeUsers = presenceUsers ?? (tenantPresence.length > 0 ? tenantPresence : EMPTY_PRESENCE_USERS); - const activeActivities = activities ?? apiActivities ?? []; + // The `activities` prop still wins where a host passes one; otherwise the + // shared feed, which is `[]` (not null) until the first read lands. + const activeActivities = activities ?? apiActivities; const orgList = organizations ?? []; const hasOrgSection = isOrganizationsLoading || orgList.length > 0 || !!activeOrganization; // Mirror the server's `beforeCreateOrganization` gate so the "Create diff --git a/packages/app-shell/src/layout/__tests__/AppHeader.inboxVariant.test.tsx b/packages/app-shell/src/layout/__tests__/AppHeader.inboxVariant.test.tsx index 0e3a3aa2fe..b2d2624a6c 100644 --- a/packages/app-shell/src/layout/__tests__/AppHeader.inboxVariant.test.tsx +++ b/packages/app-shell/src/layout/__tests__/AppHeader.inboxVariant.test.tsx @@ -114,15 +114,21 @@ vi.mock('@object-ui/react', async (importOriginal) => ({ useOffline: () => ({ isOnline: true }), })); +// Presence renders identifiably (and with a non-empty tenant list) so the +// #4197 boundary case can assert that presence stays app-only chrome while the +// activity feed goes user-scoped. `useTenantPresence` is a transport +// subscription, not a `dataSource.find` — there is no presence *read* to count. vi.mock('@object-ui/collaboration', () => ({ - PresenceAvatars: () => null, - useTenantPresence: () => [], + PresenceAvatars: () =>
, + useTenantPresence: () => [{ id: 'u2', name: 'Wang Wu' }], })); vi.mock('../ModeToggle', () => ({ ModeToggle: () => null })); vi.mock('../WorkspaceSwitcher', () => ({ WorkspaceSwitcher: () => null })); vi.mock('../LocaleSwitcher', () => ({ LocaleSwitcher: () => null })); -vi.mock('../ConnectionStatus', () => ({ ConnectionStatus: () => null })); +vi.mock('../ConnectionStatus', () => ({ + ConnectionStatus: () =>
, +})); vi.mock('../AppSwitcher', () => ({ AppSwitcher: () => null })); vi.mock('../LocalizedSidebarTrigger', () => ({ LocalizedSidebarTrigger: () => null })); vi.mock('../PreviewBadge', () => ({ PreviewBadge: () => null })); @@ -176,6 +182,31 @@ const finds: Array<{ object: string; query: unknown }> = []; /** What the fake `sys_inbox_message` collection holds for the current test. */ let inboxRows: Array> = []; +/** + * #4197 — one `sys_activity` row in plugin-audit's raw column names, the shape + * both the bell's Activity tab and Home's activity card map onto `ActivityItem`. + * A named human actor, so it survives BOTH mappings (Home additionally drops + * `System`-actor churn). + */ +const ACTIVITY_ROW = { + id: 'act_1', + type: 'updated', + actor_name: 'Li Si', + summary: 'updated Contract C-1', + object_name: 'hr_contract', + record_id: 'c_1', + timestamp: '2026-08-10T08:00:00Z', +}; + +/** What the fake `sys_activity` collection holds for the current test. */ +let activityRows: Array> = []; + +/** + * What `/api/v1/approvals/requests?status=pending` answers for the current + * test. Two rows ⇒ `pendingApprovalsCount` 2, which is the second badge addend. + */ +let approvalRows: Array<{ id: string }> = []; + const fakeAdapter = { find: (object: string, query: unknown) => { finds.push({ object, query }); @@ -183,6 +214,7 @@ const fakeAdapter = { if (object === 'sys_notification_receipt') { return Promise.resolve({ data: inboxRows.length ? [DELIVERED_RECEIPT] : [] }); } + if (object === 'sys_activity') return Promise.resolve({ data: activityRows }); return Promise.resolve({ data: [] }); }, getClient: () => undefined, @@ -193,13 +225,41 @@ vi.mock('../../providers/AdapterProvider', () => ({ })); import { AppHeader } from '../AppHeader'; +import { useHomeInbox } from '../../hooks/useHomeInbox'; +import { __resetSharedUserFeeds } from '../../hooks/sharedUserFeeds'; + +/** Every URL passed to `fetch`, in order — the approvals reads are counted here. */ +let fetchUrls: string[] = []; beforeEach(() => { finds.length = 0; + fetchUrls = []; inboxRows = [INBOX_ROW]; - // The approvals count + auth-config reads are not under test; keep them from - // reaching the network (both are already soft-degrading). - vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response('{}', { status: 404 }))); + // Default: no activity, no approvals — so the #4110 cases above keep the + // exact badge arithmetic they were written against (approvals addend 0). + activityRows = []; + approvalRows = []; + // Drop the shared feeds' cache and poll timer so cases do not inherit each + // other's reads — the store deliberately outlives any one render tree. + __resetSharedUserFeeds(); + // Route by URL: the approvals endpoint answers from `approvalRows`; every + // other request (auth config, mark-read) stays a soft-degrading 404. + vi.stubGlobal( + 'fetch', + vi.fn((input: RequestInfo | URL) => { + const url = String(input); + fetchUrls.push(url); + if (url.includes('/api/v1/approvals/requests')) { + return Promise.resolve( + new Response(JSON.stringify({ data: approvalRows }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + } + return Promise.resolve(new Response('{}', { status: 404 })); + }), + ); }); afterEach(() => { @@ -209,6 +269,10 @@ afterEach(() => { /** Objects the bell's poller reads, in the order it issued them. */ const inboxReads = () => finds.filter((f) => f.object === 'sys_inbox_message'); +/** `sys_activity` reads issued by anyone in the tree — the dedupe assertion. */ +const activityReads = () => finds.filter((f) => f.object === 'sys_activity'); +/** Approvals-endpoint requests issued by anyone in the tree. */ +const approvalReads = () => fetchUrls.filter((u) => u.includes('/api/v1/approvals/requests')); describe('AppHeader — the bell polls the inbox in every variant (#4110)', () => { it('lists a canonical inbox row on Home, where the To-do card already shows it', async () => { @@ -261,3 +325,196 @@ describe('AppHeader — the bell polls the inbox in every variant (#4110)', () = expect(screen.queryByTestId('inbox-bell-badge')).not.toBeInTheDocument(); }); }); + +/** + * #4197 — the two sibling effects #4110 left behind. The pending-approvals poll + * (`if (!isApp || !user?.id) return;`) and the activity read + * (`if (!dataSource || !isApp) return;`) carried the same variant gate, so + * off-app the Approvals tab read "No pending approvals" and the Activity tab + * "No recent activity" — on the very page whose own cards (`useHomeInbox`, + * ungated) were showing both. Worse, the bell badge is + * `unreadTopics + pendingApprovalsCount`, so after #4110 only the FIRST addend + * was fetched off-app: the same user with the same data read 1 on Home and 3 + * inside an app. + * + * `variant="home"` is also the AI screen — `AiChatPage` renders + * ``; there is no separate `ai-chat` variant + * (`AppHeaderVariant = 'app' | 'home' | 'orgs'`). + */ +describe('AppHeader — Approvals + Activity fill in every variant (#4197)', () => { + beforeEach(() => { + approvalRows = [{ id: 'apr_1' }, { id: 'apr_2' }]; + activityRows = [ACTIVITY_ROW]; + }); + + // 'app' is the control: it was green before the fix and must stay green. + for (const variant of ['home', 'orgs', 'app'] as const) { + it(`fills the Approvals tab on the ${variant} variant`, async () => { + render(); + + // The count reached the popover: the breakdown line spells the second + // addend out unclamped, and the tab body offers the drill-in. + await waitFor(() => + expect(screen.getByTestId('inbox-badge-breakdown-approvals')).toHaveTextContent( + '2 pending approvals', + ), + ); + expect(screen.getByText('View approvals')).toBeInTheDocument(); + expect(screen.queryByText('No pending approvals')).not.toBeInTheDocument(); + }); + + it(`fills the Activity tab on the ${variant} variant`, async () => { + render(); + + expect(await screen.findByText('updated Contract C-1')).toBeInTheDocument(); + expect(screen.getByText('Li Si')).toBeInTheDocument(); + expect(screen.queryByText('No recent activity')).not.toBeInTheDocument(); + }); + + it(`reads the same badge on ${variant} as anywhere else — 1 unread topic + 2 approvals`, async () => { + render(); + + // Both addends are fetched, so the badge reconciles against the + // breakdown line instead of against an unasked question (#4073). + await waitFor(() => + expect(screen.getByTestId('inbox-bell-badge')).toHaveTextContent('3'), + ); + expect(screen.getByTestId('inbox-badge-breakdown-total')).toHaveTextContent('3 total'); + expect(screen.getByTestId('inbox-badge-breakdown-notifications')).toHaveTextContent( + '1 notifications', + ); + }); + } + + it('still reports an empty approvals inbox as an answer, not a gate', async () => { + approvalRows = []; + render(); + + await waitFor(() => expect(approvalReads().length).toBeGreaterThan(0)); + expect(await screen.findByText('No pending approvals')).toBeInTheDocument(); + // Badge falls back to the notifications addend alone — correctly this time. + expect(screen.getByTestId('inbox-bell-badge')).toHaveTextContent('1'); + }); +}); + +/** + * The To-do card's real data path. `HomePage` renders this hook's output + * alongside the bell inside `HomeLayout`, so on Home both consumers mount in + * the same commit — the duplicate-read site the card called out. + */ +function HomeTodoCardProbe() { + const { pendingApprovalsCount, activities } = useHomeInbox(); + return ( + // Fenced off with a testid so the assertions below can tell the card's + // copy of a row apart from the bell's — when the fix works, BOTH render + // the same text and an unscoped `getByText` is ambiguous by construction. +
+ {pendingApprovalsCount} + {activities[0]?.description ?? ''} +
+ ); +} + +/** + * The pin on the triage ruling (#4197): consistency is the acceptance + * criterion and ONE fetch feeds both consumers. Naively dropping `isApp` would + * make Home issue the approvals read and the `sys_activity` read twice — once + * for the bell, once for `useHomeInbox` — which is precisely the trade-off the + * card refused to resolve by duplicate polling. Both consumers now read one + * shared store, so the read count stays 1 no matter how many mount. + */ +describe('AppHeader + Home cards share one fetch, not two (#4197)', () => { + beforeEach(() => { + approvalRows = [{ id: 'apr_1' }, { id: 'apr_2' }]; + activityRows = [ACTIVITY_ROW]; + }); + + it('issues ONE approvals read for the bell and the To-do card together', async () => { + render( + <> + + + , + ); + + await waitFor(() => + expect(screen.getByTestId('inbox-badge-breakdown-approvals')).toHaveTextContent( + '2 pending approvals', + ), + ); + await waitFor(() => expect(screen.getByTestId('home-approvals-count')).toHaveTextContent('2')); + + // …and exactly one request went out for the two of them. + expect(approvalReads()).toHaveLength(1); + }); + + it('issues ONE sys_activity read for the bell and the activity card together', async () => { + render( + <> + + + , + ); + + // The one row surfaces in BOTH places — the bell's Activity tab and Home's + // card — so it is matched twice and each match is attributed explicitly. + await waitFor(() => { + const homeCards = screen.getByTestId('home-cards'); + const shown = screen.getAllByText('updated Contract C-1'); + expect(shown.some((el) => !homeCards.contains(el))).toBe(true); // the bell's tab + expect(shown.some((el) => homeCards.contains(el))).toBe(true); // Home's card + }); + + expect(activityReads()).toHaveLength(1); + }); + + it('serves both consumers the same numbers — the consistency criterion', async () => { + render( + <> + + + , + ); + + await waitFor(() => expect(screen.getByTestId('home-approvals-count')).toHaveTextContent('2')); + // The bell's second badge addend IS the card's number, from one read. + expect(screen.getByTestId('inbox-badge-breakdown-approvals')).toHaveTextContent( + '2 pending approvals', + ); + }); +}); + +/** + * The other half of the boundary. `isApp` still means something — it is the + * flag that hides genuinely app-scoped chrome — so un-gating the two + * user/org-scoped reads must NOT drag presence off-app with them. + * + * Note what is being pinned: presence never was a *read*. `useTenantPresence` + * is a transport subscription (`PresenceProvider.subscribeTenant`) and the + * effect formerly named `fetchPresenceAndActivities` explicitly never probed + * it — its one and only read is `sys_activity`. So the boundary is a RENDER + * gate, and these assert it stayed put. + */ +describe('AppHeader — presence stays app-only chrome (#4197 boundary)', () => { + it('renders no presence avatars and no connection dot off-app', async () => { + render(); + + await waitFor(() => expect(activityReads().length).toBeGreaterThan(0)); + expect(screen.queryByTestId('presence-avatars')).not.toBeInTheDocument(); + expect(screen.queryByTestId('connection-dot')).not.toBeInTheDocument(); + }); + + it('renders both inside an app (the control)', async () => { + render(); + + expect(await screen.findByTestId('presence-avatars')).toBeInTheDocument(); + expect(screen.getByTestId('connection-dot')).toBeInTheDocument(); + }); + + it('never issues a presence read in any variant — presence is a subscription', async () => { + render(); + + await waitFor(() => expect(activityReads().length).toBeGreaterThan(0)); + expect(finds.some((f) => String(f.object).includes('presence'))).toBe(false); + }); +}); From 8bc784179c5b85f247376baf7afd65fcc67e28b3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 03:23:33 +0000 Subject: [PATCH 2/2] test(app-shell): anchor the presence boundary pin on a read both sides issue (#4197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asserting an absence needs the tree to have settled first, and the case used the `sys_activity` read as that settle point — which the `home` variant does not issue on `origin/main`. So under reverse verification the presence pin went red for a timeout that says nothing about presence, when the whole point of a boundary pin is to be green on BOTH sides. Anchored on the inbox read instead: ungated in every variant since #4199, so it arrives before and after, and what the case reports is the render gate it was written to watch. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- .../layout/__tests__/AppHeader.inboxVariant.test.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/app-shell/src/layout/__tests__/AppHeader.inboxVariant.test.tsx b/packages/app-shell/src/layout/__tests__/AppHeader.inboxVariant.test.tsx index b2d2624a6c..a1778381b9 100644 --- a/packages/app-shell/src/layout/__tests__/AppHeader.inboxVariant.test.tsx +++ b/packages/app-shell/src/layout/__tests__/AppHeader.inboxVariant.test.tsx @@ -499,7 +499,14 @@ describe('AppHeader — presence stays app-only chrome (#4197 boundary)', () => it('renders no presence avatars and no connection dot off-app', async () => { render(); - await waitFor(() => expect(activityReads().length).toBeGreaterThan(0)); + // Settle on the INBOX read, not the activity read. Asserting an absence + // needs the tree to have done its work first, and the inbox poll is the + // one read this variant issues on both sides of this change (#4199 + // un-gated it) — so this case stays green before AND after, which is what + // a boundary pin is for. Anchored on the activity read it went red on + // `origin/main` for the settle point never arriving, which would have + // dressed an unrelated timeout up as evidence about presence. + await waitFor(() => expect(inboxReads().length).toBeGreaterThan(0)); expect(screen.queryByTestId('presence-avatars')).not.toBeInTheDocument(); expect(screen.queryByTestId('connection-dot')).not.toBeInTheDocument(); }); @@ -515,6 +522,7 @@ describe('AppHeader — presence stays app-only chrome (#4197 boundary)', () => render(); await waitFor(() => expect(activityReads().length).toBeGreaterThan(0)); + // Not one read names presence — in the variant that DOES render it. expect(finds.some((f) => String(f.object).includes('presence'))).toBe(false); }); });