馃悰 fix: Resolve Admin Capability Gate In Route Loader - #109
Conversation
The layout decided admin access from a component-level query, so while the effective-capabilities request was in flight it fell through and mounted the full panel before replacing it with the access-denied screen. Resolving the check in the route loader and reading it with a suspense query means the layout only ever renders a decided state, with the route pending component covering the wait. Also removes an unreferenced capabilities query factory that declared the same cache key as the hook with an incompatible payload and no error classification, so the two could not safely coexist.
|
@codex review |
There was a problem hiding this comment.
馃挕 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfe15fcbd6
鈩癸笍 About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 馃憤.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. 馃殌 Reviewed commit: 鈩癸笍 About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 馃憤. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The exported query factory's data contract belongs with the rest of the local type definitions rather than inside the hook, so future consumers reach for the shared type instead of redefining the result shape.
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: 鈩癸笍 About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 馃憤. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: 鈩癸笍 About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 馃憤. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
AppLayoutdecided admin access from a component-level query, so while/api/admin/grants/effectivewas in flight the layout fell through and rendered the full panel (sidebar, header, dashboard) before replacing it with the access-denied screen. A non-admin briefly sees the entire admin UI, including its navigation.This resolves the capability check in the
_approute loader withqueryClient.ensureQueryDataand reads it in the component withuseSuspenseQuery, so the layout only ever renders a decided state: authorized or denied. The route'spendingComponentcovers the wait. This is the pattern TanStack documents for data-derived route gating, for exactly this reason ("No flash of loading states").Change Type
What changed
useCapabilities.tsexports acapabilitiesQueryOptions(userId)factory so the loader and the hook share one query definition and one cache entry. The hook's signature and all existing consumers are unchanged._app.tsxprefetches that query inloader, rendersAppPendingwhile it resolves, and derives the admin decision synchronously viauseSuspenseQuery. TheisLoading/isErrorfall-through is gone.__root.tsxusescreateRootRouteWithContext<{ queryClient }>so the loader can reach the query client through route context. The router already supplies it; only the type was missing.effectiveCapabilitiesOptionsfromserver/capabilities.ts. It had no callers anywhere in the repo, and it declared the same['effectiveCapabilities', userId]cache key as the hook with an incompatible payload (string[]versus{ available, capabilities }) and no 401/403/404 classification. Leaving both would mean whichever populated the key first won, and a consumer of the other would read the wrong shape.Testing
Verified in a container against a mock of the admin API, driving the real panel through a genuine login. Because the flash is a sub-frame DOM transition, a
MutationObserverinstalled before any app JS runs timestamps the first appearance of each marker; screenshots and polling are too coarse to catch it.Before, on this branch's base, at real timing (no artificial latency):
The panel is on screen for ~42ms for a user who is about to be denied.
Before: the full panel mounts while capabilities load, then is replaced by the access-denied screen.
clip-base-flash.webm
This clip was captured on a branch that also changes the access-denied screen itself, so its final frame differs from what
mainrenders. The flashing panel is the same code path this PR fixes.After, same harness:
No privileged chrome enters the DOM at any point.
After: neutral loading only, then the access-denied screen.
Local gates:
eslintclean on the changed files,prettier --checkclean,tsc --noEmitclean, and the fullvitestsuite passes.Checklist