test(app-shell): pin the bell panel's Unread/All render oracle — #4230's console predates #4199 (#4230) - #4284
Merged
Conversation
#4230 reported the console bell dead again and filed it as a regression of #4110 / PR #4199. It is not a regression: the QA run vendored console 09987b6 (2026-08-09 02:35:56Z) and #4199 landed as 7b07832 (2026-08-10 20:39:10Z), 42 hours later. `git merge-base --is-ancestor 7b07832 09987b6` exits non-zero, and that console's AppHeader.tsx line 338 still reads `if (!dataSource || !isApp || !user?.id) return;` — the exact gate #4199 removed. No production code needed changing. What the round did expose is a hole in #4199's own pin: every case in AppHeader.inboxVariant.test.tsx held ONE row and never touched the popover's Unread/All sub-filter, so nothing could tell "Unread is empty because every row is read" apart from "the panel was handed nothing" — and the reading under All is the load-bearing half of the reported symptom, because All applies no predicate at all. Adds eight cases driving the QA payload (ten sys_inbox_message rows, mixed read-state, including the approval.reminder #4230 names) through the real AppHeader + InboxPopover pair under BOTH filters, plus the grouping fold (a repeated topic shows x2 unread / x3 all rather than vanishing) and the badge reconciliation. Reverse-verified: restoring the isApp gate turns all eight red on home/orgs and leaves variant="app" green — #4110's signature. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 11, 2026 11:17
This was referenced Aug 11, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #4230. Deliberately not
Fixes— no production code changed, and whether the card closes as not-reproducible (or stays open pending a QA re-run on a current console) is a triage call, not this PR's to make.The premise did not survive the re-check
#4230 is filed as a regression: "the symptom is back on console
09987b68… whatever #4199 fixed, it does not hold on this build." #4199 was never in that build.09987b680d53801c79f67d969b14e9bb732b8a227b07832327ea07af01208309b0e0d9c60df3645fLine 338 is the exact statement #4199 rewrote to
if (!dataSource || !user?.id) return;. And #4199 is the only commit touching the bell path in the whole range09987b68..7b0783232:So: #4230 is #4110, observed on a build that predates #4110's fix. Not a regression, and #4199 did not regress — it was simply absent. Every corroborating detail in the report fits the
isAppgate exactly: the bell empty under both filters, the home To-do card (useHomeInbox, never gated) listing the same rows, and zero/api/v1/*requests on open (a gated poll issues none, and post-#4199 a quiet open is expected anyway, per #4225).The symptom does not reproduce at
origin/main(436681e72): #4199's un-gated poll is intact atAppHeader.tsx:314, and #4197 / PR #4223'ssharedUserFeeds.tsrefactor — the change the dispatch flagged as the regression risk on this data path — deliberately left the inbox read alone (it shared only approvals andsys_activity), so it could not have re-broken it.What this PR actually adds, and why it is still worth landing
A hole in #4199's own pin. Every case in
AppHeader.inboxVariant.test.tsxholds one row and never touches the popover's Unread/All sub-filter. But the symptom both cards report is a pair of readings — "You're all caught up" under Unread and "No notifications" under All — and All is the load-bearing half, because it applies no predicate at all:Nothing in the file could tell those apart. A recurrence that left the poll running but dropped rows between the poll and the popover — the other branch #4110's investigation named, and the one #4230 left explicitly open — would have found every existing case green.
Eight new cases drive the QA payload through the real
AppHeader+InboxPopoverpair: tensys_inbox_messagerows for one user, newest-first, five already read, including theapproval.reminder#4230 names. They assert the unread cut under Unread, all ten titles under All, the round trip back to Unread, that the repeated-topic fold showsx2unread /x3under All rather than vanishing (the #2765 grouping is the one transform between poll and list), and that the badge reconciles with the rows on show. One case pins the distinction neither #4110 nor #4230 could make from outside: with every row read, Unread is correctly empty while All must still hold ten.Oracle boundary, stated rather than implied: this mounts the real components over a fake adapter. It pins everything from
dataSource.findto the rendered row — the whole span both cards localise the defect to — and it does not pin the adapter, the REST layer, or/api/v1/notificationsitself. No live console or backend was driven in this round.Reverse verification
Prediction, stated before running: restoring the
isAppgate on the poll turns everyvariant="home"/"orgs"case red and leavesvariant="app"green — #4110's exact signature. That is what happened.All eight new cases red; the
variant="app"control cases green throughout:Taken out with
git checkout --, nevergit stash.#4235 — one fix or two? Two.
#4230 asked whether the home-header panel (#4235) shares this root cause. It does not, and the reasoning is now sharper than when #4235 was filed:
/api/v1/notificationsreturns 10 rows (#4110 / #4156 back on console 09987b68) #4230's root cause is theisAppgate, which is by construction not The console HOME header renders a second, different inbox polling the wrong source —sys_inbox_messageinstead of/api/v1/notifications, no badge, "You're all caught up" with 9 unread #4235's. The home card was the working control in [console] InboxPopover 通知页签恒为空:sys_inbox_message 有未读行、REST 正常返回、Home 卡片可渲染,弹层打开不发请求且过滤成空(rc.5 回归,rc.3 正常) #4110, Console: notification bell dropdown always empty while list page / API / inbox tables show unread notifications #4156 and REGRESSION: the console header bell panel is dead again — "No notifications" under both Unread and All while/api/v1/notificationsreturns 10 rows (#4110 / #4156 back on console 09987b68) #4230 precisely becauseuseHomeInboxwas never gated — a variant gate cannot explain a panel that renders on exactly one variant.sys_inbox_messageinstead of/api/v1/notifications, no badge, "You're all caught up" with 9 unread #4235's complaint is a source choice (sys_inbox_messagerather than/api/v1/notifications), which is still true at main and is untouched by anything here. Note for whoever takes it: readingsys_inbox_message+ receipts is what ADR-0030 / feat(app-shell): repoint Console bell to sys_inbox_message + receipts (ADR-0030) #1429 deliberately repointed the bell to, so "wrong source" is a design question for that card to settle, not a self-evident defect.sys_inbox_messageinstead of/api/v1/notifications, no badge, "You're all caught up" with 9 unread #4235's own recorded cross-run contradiction — REGRESSION: the console header bell panel is dead again — "No notifications" under both Unread and All while/api/v1/notificationsreturns 10 rows (#4110 / #4156 back on console 09987b68) #4230 (run objectstack#7517) saw the home card working while objectstack#7514 saw it empty, same console pin — is unresolved and stays The console HOME header renders a second, different inbox polling the wrong source —sys_inbox_messageinstead of/api/v1/notifications, no badge, "You're all caught up" with 9 unread #4235's to resolve. Nothing in this round bears on it.Sequencing, per #4225: whoever extends
hooks/sharedUserFeeds.tswith an inbox feed will be editing this exact path, and the eight cases added here are what will tell them if they re-break the bell.Verification
npx vitest run packages/app-shell/src/layout/__tests__/AppHeader.inboxVariant.test.tsx— 30 passed (22 pre-existing, 8 new)npx vitest run packages/app-shell(repo-root vitest perpnpm --filter @object-ui/app-shell test跑的是 @object-ui/console 的 22 个文件,app-shell 自己的 276 个一个没跑,却报绿 #3378) — 344 files, 3294 passed, 1 skippedpnpm --filter @object-ui/app-shell type-check— green (both tsc passes:tsc --noEmitandtsc -p tsconfig.typetests.json), after building the dependency closure--filter '@object-ui/app-shell^...' buildpnpm --filter @object-ui/app-shell lint— 0 errors (2286 pre-existing warnings, baseline unchanged)check-control-bytes,check-changeset-presence,check-changeset-no-major,check-changeset-fixed,check-i18n-call-site-keys,check-i18n-en-drift— all greenChangeset: empty frontmatter (the gate's documented exemption). Tests and comments only, no source change, so nothing to release.
Refs #4110, #4156, #4199, #4225, #4235, objectstack#7517.
Generated by Claude Code