Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): inbox/activity "see all" links open in an app the user can open (#4074) - #4242
Merged
Merged
Conversation
…r can open (#4074) Four producers hardcoded `/apps/setup/...` for pages that are not bound to the setup app: Home's notification fallback and "View all activity", and the bell popover's two footer drills. `sys_inbox_message` and `sys_activity` are framework-owned objects reachable at `/apps/{any app}/{object}` — exactly like `system/approvals`, whose entry in the same popover already resolved the current app. Being app-independent is why they render under whatever app the user can open, not a reason to render them in `setup`. Measured in a browser on the issue: a business user without setup access gets "You don't have access" (the shell renders the target inside their own app), and every user is switched out of the app they were in — URL, sidebar and app switcher flip to Setup with no announcement. All four now resolve through one shared helper, `resolveHostAppSegment` in `utils/appRoute.ts`, generalizing the resolution objectstack#7231 introduced for the approvals entry so Home and the popover cannot answer the same question differently: current/last-open app re-checked against the live active-app list, then the first active app, then the caller's own hint when the list has not loaded, then `setup`. The admin-scoped `/apps/setup/system/marketplace` and `/apps/setup/system/apps` links are deliberately untouched and pinned as such. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 11, 2026 06:26
Uh oh!
There was an error while loading. Please reload this page.
os-help added a commit
that referenced
this pull request
Aug 11, 2026
One conflict, in `packages/app-shell/src/console/home/HomePage.tsx`, between two changes to the same home-grid app filter: - main (#4242 / objectui#4074) extracted the predicate `active !== false && hidden !== true` into a shared `filterActiveApps()` in `utils/appRoute.ts`, so Home's launcher and every producer that builds a link into an app answer "an app the user can open" identically; - this branch (objectstack#6955) added a ⛔ note to that same inline predicate recording that it must keep filtering `hidden` and must NEVER gain an `_unpublished` clause. Both intents are kept and neither is behavioral, so this is not a trade-off: HomePage takes main's `filterActiveApps(apps)` call, and the ⛔ note moves onto `filterActiveApps` itself. The pin is strictly stronger there — it is now the single definition, so it covers Home's grid AND `resolveHostAppSegment`'s link producers, which main newly routed through the same predicate. `filterActiveApps` does not read `_unpublished`, so the #6955 contract is unchanged by the merge: the banner still reads `_unpublished`, the per-app publish body is still `{"_unpublished": false}`, and every launcher surface still filters `hidden` alone. The ⛔ pin gains a test case in `utils/__tests__/appRoute.test.ts` alongside the existing AppSwitcher and RootLandingRedirect pins; reverse-verified by injecting the guarded clause into `filterActiveApps` (that case, and only that case, went red).
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes#4074
Four navigation producers hardcoded
/apps/setup/...for two pages that are not bound to the setup app.sys_inbox_messageandsys_activityare framework-owned objects reachable at/apps/{any app}/{object}, exactly likesystem/approvals— whose entry in the same popover, one line above, was already resolving the current app (objectstack#7234). The in-code comment defending the hardcoded target argued the OBJECT is app-independent, which is true, and which is precisely why it renders under whatever app the user can open rather than undersetup.Both browser verifications on the card measured the cost, and it is two defects, not one:
setupgets the target rendered inside their own app's shell with a "You don't have access" empty state (softer, and more confusing, than a hard app guard);The change
One shared helper,
resolveHostAppSegmentinpackages/app-shell/src/utils/appRoute.ts, generalizing the resolution objectstack#7231 landed inline inHomePagefor the approvals entry. Home and the bell popover both call it, so the two surfaces cannot drift into different answers to the same question. Resolution order:/apps/crm/...tosetupwould reintroduce this very defect in a loading race;setuplast — what is left is a degenerate app carrying neither_packageIdnorname, so the historical target beats a broken link.The four anchors now resolve through it:
HomePageonOpenNotificationfallback (noaction_url)/apps/setup/sys_inbox_message?view=mine/apps/{host}/sys_inbox_message?view=mineHomePageHomeActivity onViewAll/apps/setup/sys_activity/apps/{host}/sys_activityInboxPopovergoToAllNotifications/apps/setup/sys_inbox_message?view=mine/apps/{host}/sys_inbox_message?view=mineInboxPopovergoToAllActivity/apps/setup/sys_activity/apps/{host}/sys_activityHomePage's inlineactiveAppspredicate moved into the same module asfilterActiveApps, so "an app the user can open" means one thing to Home's launcher and to every producer building a link into an app.Deliberately unchanged, and pinned as such
/apps/setup/system/marketplaceand/apps/setup/system/apps— admin-scoped surfaces wheresetupis the target rather than an oversight (the card's own scope guard). A case asserts the marketplace link still emits/apps/setup/system/marketplace.goToApprovalsandhandleNotificationClickin the popover — untouched; cases assert they still resolve the current app and still follow an explicitaction_url.setupstill lands insetup. The fallback order's tail is not "never setup", it is "an app this user can actually open".Verification
Red-first, then reverse-verified. With the four sites reverted on top of the finished branch (helper left in place), 13 of 63 cases went red — every case asserting a resolved target — while all 12 helper unit cases and every CONTROL stayed green, which is the intended split: the helper is right in isolation, the four anchors are the wiring. Restored: 63 passed.
No copy changed — the four edits are navigation targets and comments only — so there is no i18n work in this PR. Changeset:
@object-ui/app-shellpatch.What this fix does NOT close — do not misread the empty state
objectstack#7344 is a second, independent cause: no shipped permission set grants
sys_inbox_message(member_default, the additiveeveryonebaseline, does not name it). Re-pointing these four anchors at an app the user can open therefore leaves them on the same "You don't have access" state until that grant question is settled — verified on the card against the Account app's own Notifications entry, which reachessys_inbox_messagewith nosetupinvolved anywhere and renders the identical empty state.The routing fix is necessary, not sufficient. Accordingly every case here asserts the resolved target — the argument
navigate()receives — and none asserts what renders at the far end; pinning the far end would pin someone else's defect.Generated by Claude Code