feat(app-shell,i18n): announce inbox arrivals with a toast and a desktop notification (#7011) - #8668
Conversation
…top notification (objectui#7011) The inbox was completely silent about arrivals: `sharedUserFeeds` polls `sys_inbox_message`, the rows landed in the store, the bell badge counted them, and a user not staring at the bell learned nothing — so approvals and @-mentions were routinely missed. Presentation layer only. The transport is untouched: same two reads, same 10s/60s cadence, same backoff, no push channel. A backgrounded tab can therefore be up to a minute late, which is accepted rather than worked around. - `inboxArrivals` — the pure diff: a session-scoped seen set, `(topic, title)` collapse reused from the inbox's own `groupNotifications`, bounded memory. - `useInboxArrivalNotifier` — mounted from `useInboxBell`, the one wiring of the shared feed onto a bell, so the header bell and the `global:notifications` block announce by the same rules and through the same `markRead`. - `desktopNotifications` — the single door to the browser Notification API. - `NotificationPreferencesMenu` — two switches in the account menu, stored per user in localStorage: in-app alerts (on), desktop notifications (off). The negative rules are the load-bearing ones: the first answered read primes and announces nothing, several rows in one cycle announce once, an already-read row never announces, and a hidden tab gets the desktop notification instead of the toast (never both). `Notification.requestPermission()` is called from the settings toggle's change handler and from nowhere else — never on mount, on a feed refresh or on a first message. That prompt is answered once per origin and `denied` is permanent, so a load-time request spends the channel for every user who reflexively blocks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
ℹ️ Console Performance Budget — not measuredThis run did not produce a console bundle to measure, so there is no pass/fail verdict for the performance budget. This is not a budget violation. Nothing was measured — the numbers a real violation would carry are simply absent.
See the workflow run for details. 📦 Bundle Size Report
Size Limits
|
… and repair the fixture's imports (objectui#7011) Two CI reds on PR #8668, both this branch's own. `check:spec-symbols` — `@objectstack/spec/api` already owns `NotificationPreferences`, and it is a different layer: the account's server-persisted delivery routing (`email`, `push`, `inApp`, `digest`, `channels`), moved by the `getNotificationPreferences` API pair. The local interface is this browser's presentation switches (`toast`, `desktop`). Zero keys in common, and the spec's schema strips both of ours, so importing or deriving it cannot express the two switches — it would change what the feature stores. Renamed to `BrowserNotificationPreferences`, with the measurement in the declaration's doc comment and a tripwire row in `spec-symbol-parity.test.ts` so the new name cannot silently re-collide. `Bundle Analysis` — the job never reached the bundle. The console build failed with six TS2307s: the browser fixture imported `@object-ui/app-shell/hooks/…` and `@object-ui/auth/AuthContext`, and neither package publishes a subpath. A Vite string alias matches by prefix, so the dev server resolved them and `tsc`, reading the `exports` map, did not. The fixture now imports the workspace sources those aliases already resolved to; nothing is added to either package's published surface. `apps/console/tsconfig.json` gains `node` in `types` for the `typeof process` guard in the app-shell source this pulls into the program. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
CI repair — both reds diagnosed and repairedPushed 1.
|
| keys | |
|---|---|
@objectstack/spec/api NotificationPreferences |
email, push, inApp, digest, channels |
| this branch's local interface | toast, desktop |
Zero keys in common, and they are two different layers under one word:
- The spec's is the account's server-persisted delivery routing — which transports a notification is sent over and how often. It has a server API pair beside it (
getNotificationPreferences/updateNotificationPreferences) andRegisterDevice/UnregisterDevicenearby. - This one is this browser's presentation of a row that has already been delivered: may a toast cover this screen, may this browser's Notification API be used.
The direction that settles it is not the key count but the parse:
NotificationPreferencesSchema.parse({ toast: true, desktop: false })
-> { email: true, push: true, inApp: true, digest: 'none' }
The spec's schema strips both of our keys. So importing or deriving it cannot express the two switches at all — binding to it would change what the feature stores, not merely what the type is called. The doctrine's preferred arm (import/derive) is genuinely unavailable here.
Repair — rename to a dialect, with the tripwire. NotificationPreferences -> BrowserNotificationPreferences, confined to that one file (the type had no consumer outside it; the exported functions, the const and the localStorage key are untouched, so nothing stored or read moved). No ALLOW entry: the collision is gone rather than excused. The measurement above is written into the declaration's doc comment so the next reader does not have to re-derive it.
The tripwire is a row in the existing packages/app-shell/src/__tests__/spec-symbol-parity.test.ts RENAMES table, which arms both ratchets: the spec must not own BrowserNotificationPreferences, and the spec must still own NotificationPreferences — so if upstream ever retires the name, the rename stops being load-bearing and this fails rather than outliving its reason.
The tripwire was observed red, not merely written: swapping the row's local name to the spec-owned NotificationPreferences on disk (blob aa85daf -> f0ed71a) turns exactly that assertion red —
× the spec does not own `NotificationPreferences`
AssertionError: @objectstack/spec now exports `NotificationPreferences`. ... expected true to be false
Tests 1 failed | 31 passed (32)
— and the restore is byte-identical (git hash-object back to aa85daf, git diff HEAD empty).
The header census (twenty-eight / twenty / eight) was not re-counted in place: it is a measurement of the batch-3 burn-down, and a note records that later arrivals are appended instead.
Verified: check:spec-symbols exit 0 — 1361 files scanned against 5050 spec export names; 17 declared dialects, 14 untriaged collisions in 7 packages.
2. Bundle Analysis
The ceiling hypothesis is falsified — the job never reached the bundle. The log ends in the Build Console step, not the budget step, and the budget step's own env shows BUDGET_STEP_OUTCOME: skipped with every BUDGET_* variable empty (Rendered performance budget comment (kind: not-measured)).
The real failure is six TS2307s from apps/console/src/inbox-arrival-preview.tsx, i.e. the console build's tsc:
Cannot find module '@object-ui/auth/AuthContext'
Cannot find module '@object-ui/app-shell/hooks/useInboxArrivalNotifier'
Cannot find module '@object-ui/app-shell/hooks/inboxArrivals'
Cannot find module '@object-ui/app-shell/layout/NotificationPreferencesMenu'
Cannot find module '@object-ui/app-shell/layout/inboxGrouping'
Cannot find module '@object-ui/app-shell/hooks/sharedUserFeeds'
Root cause. Neither package publishes a subpath — . is the whole exports map for both @object-ui/app-shell and @object-ui/auth. The browser fixture looked fine locally because a Vite string alias matches by prefix: @object-ui/app-shell/hooks/x resolved through the @object-ui/app-shell -> packages/app-shell/src alias in apps/console/vite.config.ts, so the dev server and the browser check were both green. tsc resolves the same specifier through the exports map instead, finds no subpath, and fails. That is exactly the check nobody could run before pushing (it needs a built tree), so this is a precondition gap, not negligence.
Repair. The fixture now imports the workspace sources that alias already resolved to (../../../packages/app-shell/src/hooks/...), so the modules loaded at runtime are the same objects as before. Two arms were rejected on purpose:
- Adding the names to the barrels would publish six internals permanently — including
__resetInboxArrivals, which is a declared test seam. Widening two packages' public API so a dev-only fixture can pretend to be a consumer is the wrong trade. tsconfigpathsmapping the subpath specifiers would teach that@object-ui/app-shell/hooks/xis importable when it is not — a planted premise for the next reader.
One config line follows from this: apps/console/tsconfig.json gains node in types. The app-shell source this pulls into the console's type program guards on typeof process for the non-browser case, and app-shell's own tsconfig declares types: ["node", "vite/client"]. Without it that legitimate guard is TS2591. It is additive — it declares globals, it suppresses no check. Measured coupling: 30 workspace source files enter the console's program (27 app-shell, 3 auth) out of 3712 total.
And the ceiling, now that it was actually measured. node scripts/check-eager-closure-budget.mjs exit 0 on the built tree, with this branch's 7 keys across all ten locale packs already in:
✅ Console eager closure is 3480.4 KB gzipped across 50 of 518 chunks (budget: 3512.7 KB, headroom: 32.3 KB).
✅ vendor-objectstack 1206.4 KB / 1224.6 KB ceiling (headroom 18.2 KB)
✅ i18n-locales 442.9 KB / 444.3 KB ceiling (headroom 1.4 KB)
✅ ui-components 384.1 KB / 389.6 KB ceiling (headroom 5.5 KB)
✅ framework 70.6 KB / 97.7 KB ceiling (headroom 27.1 KB)
i18n-locales moved 441.5 -> 442.9 KB: the ten packs cost 1.4 KB of the 2.8 KB that was there, and the chunk still fits. PER_CHUNK_GZIP_CEILINGS is untouched. Worth saying out loud anyway: that chunk now has 1.4 KB of headroom, 0.02x the 89 KB regression this gate exists to catch, so the next locale-touching change is likely to be the one that has the objectui#8542 conversation. That is a maintainer call, not this branch's.
What was run
| check | result |
|---|---|
node scripts/check-spec-symbol-derivation.mjs |
exit 0 (was exit 1) |
apps/console tsc --noEmit |
exit 0, 0 errors (was exit 2, 6 errors) |
pnpm --filter @object-ui/console build |
exit 0 (tsc && vite build && build:plugin) |
node scripts/check-eager-closure-budget.mjs |
exit 0, all four per-chunk ceilings green |
pnpm check:sdui-registration-pins |
exit 0, 16/16 registrations present |
pnpm --filter @object-ui/app-shell type-check |
exit 0 (tsc --noEmit && tsc -p tsconfig.test.json) |
pnpm exec vitest run packages/app-shell/ packages/i18n/ |
724 files, 7477 passed, 1 skipped (4 shards, all exit 0) |
pnpm exec vitest run apps/console/ |
93 files, 1102 passed |
node scripts/inbox-arrival-browser-check.mjs |
20/20 checks passed in real Chromium |
node scripts/check-changeset-presence.mjs |
exit 0 |
node scripts/check-type-check-coverage.mjs |
exit 0 |
eslint on the changed files |
0 errors (2 pre-existing react-refresh warnings) |
The real-browser check passing 20/20 after the import rewrite is the load-bearing one for "no behaviour change": it drives the same modules through a real Notification grant and a real visibilityState, and every acceptance row still reads the same.
Nothing was skipped, disabled or quarantined; no empty commit; no ceiling raised; draft and auto-merge state untouched.
Generated by Claude Code
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
2 participants
Fixes #7011
The inbox was completely silent about arrivals.
sharedUserFeedspollssys_inbox_messageevery 10s, the rows landed in the store, the bell badge counted them — and a user not staring at the bell learned nothing, so approvals and @-mentions were routinely missed. All three candidate popup paths existed and none was connected: the feed had no diff logic, the console's sonner bridge only serves notifications that explicitly declaredisplayType: 'toast', and there was nonew Notification(call anywhere inpackages/orapps/.⛔ Transport untouched
Presentation layer only. Same two reads, same 10s / 60s cadence, same failure backoff, no WebSocket / SSE / long-poll. The accepted consequence is that a backgrounded tab can be up to a minute late — speeding the poll up to shave that would trade a server-wide cost for one surface's latency.
git difftouches no file underhooks/sharedUserFeeds.ts's scheduling, and none of its cadence constants moved.What lands
hooks/inboxArrivals.ts(topic, title)collapse reused from the inbox's owngroupNotifications, bounded memoryhooks/useInboxArrivalNotifier.tsuseInboxBellhooks/desktopNotifications.tshooks/notificationPreferences.tslayout/NotificationPreferencesMenu.tsxMounted from
useInboxBellrather than fromAppHeader, because that hook is the ONE wiring of the shared feed onto a bell: the header bell and theglobal:notificationspage block then announce by the same rules and through the samemarkRead. Mounting both at once is safe by construction — the seen set is module-scoped, so the first scan of a snapshot takes its arrivals and the second finds none.The toast entry point was re-located by symbol
The card named
presentNotificationToastand triage could not find it in source, flagging it as unverified. It does exist:packages/app-shell/src/chrome/notificationToast.tsx, exported fromchrome/index.ts. It is used as-is — that module's own contract is "the ONLY place a notification becomes a sonner call", so the announcement goes through it rather than around it.The five constraints
readysnapshot for a session identity primes the seen set and announces nothing; historical unread at login or refresh updates the badge only. Onlyreadyis scanned — aloading/errorsnapshot carries the last value, and priming off one would either swallow the inbox or announce all of it.(topic, title)collapse: one group says its own title, several groups summarize.sys_inbox_messagecarries no actor column at all (inbox-channel.tswrites the row field by field and includes none;listInbox's REST view has none either). The actor exists only one FK hop up onsys_notification.actor_id, and the shipped permission set does not grant a normal user read on that object. This repo has also already ruled the consumer side cannot go first: objectui#5203 retiredInboxNotification.actor_namefor exactly this reason and left two pins forbidding its re-declaration, one of which asserts the produced row's exact key set. Filed rather than guessed — messaging:sys_inbox_message不带 actor,消费端无法识别「自己触发的回执」 objectstack#16974 (add the column at materialization) and app-shell: 站内信到达提醒缺「自己触发的不弹」—— 等收件箱行带上 actor #8667 (Blocked-by:it, wire the suppression, ~4 lines).Notification.requestPermission()is reachable only from the settings toggle's change handler.document.visibilityState === 'visible'decides, in one place.⭐ The non-regression axis, and its lit control
The plausible wrong fix presents correctly but asks for permission at startup to make the desktop path work. A browser answers that prompt once and
deniedis permanent for the origin, so that fix spends the channel for every user who reflexively blocks and no later release undoes it.Pinned in both directions, because a counter that only ever reads zero also passes on an implementation that deleted the call:
useInboxArrivalNotifier.permissioncase — not on mount, not on a feed refresh, not on the first message, visible or hidden. Verified RED by moving the call to mount (leg A5).NotificationPreferencesMenucase — the toggle does prompt, exactly once.Verification
Unit — 42 new pins, all seven ablation legs RED. Every mutation was proved on disk by anchored grep counts plus a blob-hash comparison (never an editor's exit code), and every restore proved by
git diff HEADbeing empty. Classified from vitest's JSON reporter.!is_readhalf of what an arrival isThe caricature is pinned in both directions, and "pops for none" does not read as success: A2 turns the harness's own lit control (
the Notification stub is live) red along with the positive cases.Suites:
pnpm exec vitest run packages/i18n/ packages/app-shell/src/hooks/ packages/app-shell/src/layout/ packages/app-shell/src/views/__tests__/global-page-blocks.render.test.tsx packages/app-shell/src/console/home/— 149 files, 1746 tests passed.pnpm --filter @object-ui/app-shell run type-check— 0error TS(it runstsc --noEmitandtsc -p tsconfig.test.json, so the new pins are type-checked too). eslint on every changed file — 0 errors. The full affected closure (i18n is depended on by ~25 packages) is declared to CI rather than run here.Real browser — 20/20, and it found a bug the unit pins could not. happy-dom implements neither
Notificationnor a settabledocument.visibilityState, so the unit pins measure a simulation of both.apps/console/inbox-arrival-preview.html(dev-server only, like the five sibling*-preview.htmlpages — the production build's single rollup input isindex.html) mounts the real presenter, the real sonner toaster and the real preferences menu;scripts/inbox-arrival-browser-check.mjsdrives it in Chromium across two contexts, one with notifications granted and one without. This is the test instance kept as a fixture.It caught this: the settings menu and the presenter each held their own
useStatecopy of the preferences, so switching desktop notifications on did nothing until the page was reloaded — the switch said granted, the presenter still believeddesktop: false, the tab stayed silent.useStorageSynccould not cover it, because thestorageevent fires only in other tabs by design. Both surfaces now read one module-scoped store, and leg A7 reddens the new pin on the old shape.The card's
examples/app-showcaseroute was not taken: that app lives in the framework repo and needs a live backend, and the repo's ownverifyskill covers the metadata-admin preview gallery, which does not mount the shell chrome. Reported rather than skipped silently.Notes for review
notifications.*keys added to all ten locale packs;check-i18n-call-site-keysandcheck-i18n-en-driftboth green.✅ 12 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s).minor, per the fixed-group rule.useInboxBellnow also returns the feed'sstatus(additive) — the presenter may only scan an answer.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code