Refactor SPA into feature modules with enterprise contract#32
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 26 minutes and 4 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (21)
📝 WalkthroughWalkthroughThe pull request restructures the frontend application to use a feature module system. It introduces centralized router and authentication handling, moves shared utilities to a common location, creates feature-specific API surface files, and implements access control logic. Application shell responsibilities are extracted from Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser as Browser
participant App as App.vue<br/>(thin wrapper)
participant Shell as AppShell.vue<br/>(Layout)
participant Router as Router<br/>(beforeEach guard)
participant AuthStore as Auth Store<br/>(Pinia)
participant SharedAuth as `@/shared/auth`<br/>(Token & Session)
participant API as API<br/>(HTTP layer)
participant FeatureRegistry as Feature<br/>Registry
User->>Browser: Load app
Browser->>App: Mount App.vue
App->>Shell: Render AppShell
Shell->>AuthStore: loadAuthConfig()
AuthStore->>API: fetchAuthConfig()
API-->>AuthStore: AuthMode, WorkOS URL, etc.
AuthStore-->>Shell: Config loaded
Shell->>SharedAuth: getUsableAccessToken()
alt Token exists and valid
SharedAuth-->>Shell: Return access token
else Token missing or expired
SharedAuth->>API: POST /auth/token/refresh/
API-->>SharedAuth: New tokens
SharedAuth->>SharedAuth: setStoredTokens()
SharedAuth->>Browser: Emit AUTH_TOKENS_UPDATED_EVENT
SharedAuth-->>Shell: Return new token
end
Shell->>AuthStore: loadCurrentUser()
AuthStore->>API: fetchCurrentUserContext()
API-->>AuthStore: User profile
AuthStore-->>Shell: User loaded
User->>Browser: Navigate to route
Browser->>Router: beforeEach guard
Router->>Router: Check auth (token valid?)
alt Not authenticated
Router->>AuthStore: clearTokens()
Router->>Browser: Redirect /login
else Authenticated
Router->>Router: Check canAccessRequirement()
alt Access denied
Router->>Browser: Redirect /home
else Access allowed
Router->>FeatureRegistry: resolve() if /settings
FeatureRegistry->>FeatureRegistry: getFirstVisibleSettingsItem()
Router->>Browser: Allow navigation
end
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (10)
frontend/src/shared/components/state/PageStateCard.vue (1)
10-16: Consider exposing slots for richer state content.As a shared "state card" meant for empty/error/loading contexts, consumers will typically want to render an icon, CTA button, or supplementary content. Currently the card is fixed to title + description only. Adding a default slot (and optionally a header/icon slot) would make it considerably more reusable without changing current call sites.
♻️ Proposed refactor
<template> <Card class="border-slate-200"> <CardHeader> <CardTitle>{{ title }}</CardTitle> <CardDescription>{{ description }}</CardDescription> </CardHeader> + <slot /> </Card> </template>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/shared/components/state/PageStateCard.vue` around lines 10 - 16, The PageStateCard component currently renders only CardHeader with CardTitle and CardDescription from props (title, description); update it to expose a default slot for richer body content and an optional named slot (e.g., "header" or "icon") for custom header/icon rendering while preserving backward compatibility by falling back to the existing CardTitle/CardDescription when those slots are not provided; modify the template inside PageStateCard.vue to check for the presence of the named slots and render them instead of the props, and keep props title/description as the default content when slots are absent.frontend/src/features/reports/pages/ReportsPage.vue (1)
14-23: Avoid duplicating the page title and description.Line 14 adds a page-level header, but Lines 21-22 still repeat the same title and description inside the card. Consider making the card header section-specific so the page does not render duplicate header copy.
♻️ Proposed copy adjustment
<Card> <CardHeader> - <CardTitle>Operational Reports</CardTitle> - <CardDescription>Platform-level overview for audit, review, and execution performance.</CardDescription> + <CardTitle>Performance metrics</CardTitle> + <CardDescription>Current approval, review, and query execution health.</CardDescription> </CardHeader>As per coding guidelines,
frontend/**/*.{tsx,jsx,vue}: Do not add hero topbars or oversized hero headers for internal product pages unless explicitly requested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/reports/pages/ReportsPage.vue` around lines 14 - 23, The page currently renders duplicate header copy: the PageHeader component (PageHeader title/description) and the CardHeader block (CardTitle/CardDescription) both show the same "Operational Reports" title and description; update the CardHeader inside ReportsPage.vue to be section-specific (e.g., replace CardTitle/CardDescription with a contextual subtitle or remove them) so only PageHeader contains the global page hero while CardHeader contains a concise section label or is omitted for this card (locate PageHeader, CardHeader, CardTitle, CardDescription in the file to implement the change).frontend/package.json (1)
16-16: Consider using generic test discovery for future test files.The current script lists only two specific test files. While all existing tests are included today, this hardcoded approach requires manual updates when new unit tests are added. Using
vitest runwithout explicit paths ensures new tests are automatically discovered and run:♻️ Proposed script update
- "test:unit": "vitest run src/app/feature-registry.test.ts src/shared/auth/access.test.ts", + "test:unit": "vitest run",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/package.json` at line 16, The "test:unit" npm script currently hardcodes two test file paths which will require manual updates; update the "test:unit" script in package.json (the "test:unit" entry) to use vitest's automatic discovery (e.g., run vitest without explicit file paths or with a glob) so new unit tests are picked up automatically — replace the hardcoded command with a generic vitest invocation like "vitest run" or "vitest run <glob>" in the "test:unit" script.frontend/src/stores/auth.ts (1)
4-4: Keep the global auth store off feature-scoped API barrels.
useAuthStoreis foundational app state, so importing from@/features/auth/apimakes shared auth state depend on a feature module. Prefer a shared/app auth API entrypoint and let the auth feature consume that same lower-level surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/stores/auth.ts` at line 4, The auth store is importing feature-scoped API symbols (fetchAuthConfig, fetchCurrentUserContext, AuthMode, CurrentUserContext) from the auth feature barrel which couples global state to a feature; change the import to the shared/app-level auth API entrypoint (the common auth API surface used by the rest of the app) so useAuthStore depends on the core auth API instead of '@/features/auth/api', and update any re-exports if necessary so the same symbols are imported from the shared auth entrypoint.frontend/src/features/workflows/pages/WorkflowSqlEditorPage.vue (1)
6-6: TreatSqlCodeEditoras shared UI instead of a private queries component.
workflowsnow imports fromfeatures/queries/components/..., which couples two feature modules through an internal path. Since workflow pages also need this editor, consider moving it tosharedor exposing it through a public queries component entrypoint.Possible direction after extracting the shared editor
-import SqlCodeEditor from '@/features/queries/components/SqlCodeEditor.vue' +import SqlCodeEditor from '@/shared/components/sql/SqlCodeEditor.vue'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/workflows/pages/WorkflowSqlEditorPage.vue` at line 6, The import of SqlCodeEditor in WorkflowSqlEditorPage.vue couples workflows to queries' internal components; move the editor to a shared UI location or add a public export from the queries feature and update imports. Specifically, extract the component symbol SqlCodeEditor into a shared/components (or shared/ui) module and update WorkflowSqlEditorPage.vue to import from the new shared path, or alternatively add an index export (e.g., queries/components/index.ts) that re-exports SqlCodeEditor and update the import to use that public entrypoint so workflows no longer reference an internal path.frontend/src/shared/auth/access.test.ts (1)
25-67: Consider adding a few more coverage cases.Solid baseline tests. A couple of worthwhile additions when time permits:
requiresSuperuser: truepath (covered byhasPermissionbut not directly viacanAccessRequirement).anyPermissionswithout anyrequiredPermissions(i.e., only the "any" branch matters).- Empty/undefined requirement object should return
true(baseline behavior).hasPermissionfor a non-superuser with a missing permission returningfalse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/shared/auth/access.test.ts` around lines 25 - 67, Add unit tests covering the missing branches in access helpers: create tests that verify canAccessRequirement with requiresSuperuser: true returns true for buildUser({is_superuser: true}) and false otherwise; add a test where only anyPermissions is provided (no requiredPermissions) and ensure canAccessRequirement returns true when user has one of the anyPermissions and false when they don't; add a test with an empty/undefined requirement object (call canAccessRequirement(user, {}) or canAccessRequirement(user, undefined)) and assert it returns true; and add a negative hasPermission test asserting hasPermission(buildUser({is_superuser: false}), 'some.missing_permission') returns false. Reference the functions hasPermission, canAccessRequirement and helper buildUser to locate where to add these cases.frontend/src/app/AppShell.vue (1)
120-135: Verify logout path whenloadAuthConfig()is slow/unreachable.
logout()awaitsauthStore.loadAuthConfig()before clearing tokens. If this network call hangs or is slow, logout is delayed and the user retains a valid session in the meantime. Consider clearing tokens first (optimistic local sign-out) and using a cachedauthModeto decide between WorkOS redirect and/login, or bounding the config load with a timeout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/AppShell.vue` around lines 120 - 135, In logout(), avoid awaiting authStore.loadAuthConfig() before clearing tokens; instead perform an immediate local sign-out by calling authStore.clearTokens() first, then attempt to load or refresh auth config with a bounded timeout (or read a cached authStore.authMode) to decide whether to redirect to publicApiUrl('/auth/workos/logout/') or router.push('/login'); update the control flow in the logout function to use the cached authMode or a timed promise for loadAuthConfig() so a slow/unreachable network call cannot delay clearing tokens or the redirect decision.frontend/src/shared/api/http.ts (1)
58-141: 401 refresh/retry flow looks correct.The auth gating (
requiresAuth = options.token !== undefined), the single-retry guard viaskipAuthRetry, and the separation between "refresh succeeded but server still 401s" (firesnotifyUnauthorizedand throws) vs. "refresh failed withAuthSessionExpiredError" (firesnotifyUnauthorizedand throws a formatted 401) are all handled cleanly. Callers that omitoptions.tokenget a pure unauthenticated request, which matches thepublicApiUrluse case.One small note worth being aware of: because
requiresAuthis driven by the presence ofoptions.token(even""), every caller must pass sometokenvalue to get auth + retry behavior — that contract is implicit, so a brief comment aboveRequestOptions.tokenwould help future feature-module authors avoid silently landing unauthenticated calls.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/shared/api/http.ts` around lines 58 - 141, Add a short comment to the RequestOptions/InternalRequestOptions definition explaining that auth gating uses the presence of options.token (even an empty string) to enable authentication and the automatic refresh/retry flow; reference the options.token field and the request function's requiresAuth check so future callers know they must pass a token value to get auth+retry behavior and won't silently make unauthenticated requests.frontend/src/shared/auth/auth.ts (2)
14-48: Duplicate of helpers now inshared/api/http.ts.
isRecord,flattenErrorMessage, andparseResponseMessagehere are near-identical to the ones exported fromfrontend/src/shared/api/http.ts(the http version additionally prefixes object keys). Consider importing the shared versions instead to keep error-message formatting consistent across the refresh path and the main request path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/shared/auth/auth.ts` around lines 14 - 48, This file defines local duplicates of isRecord, flattenErrorMessage, and parseResponseMessage; remove these local implementations and instead import and use the shared implementations from the central HTTP helpers, replacing all references to the local functions (isRecord, flattenErrorMessage, parseResponseMessage) with the imported ones so error-message formatting is consistent across refresh and main request paths. Ensure you import the exported names exactly (isRecord, flattenErrorMessage, parseResponseMessage) and run a quick build/test to verify no other local usages remain.
136-145: Client-side JWT validation of refresh token assumes backend uses JWTs.
isAccessTokenExpired(refreshToken)callsdecodeJwtPayload, which requires a 3-segment JWT format with numericexpclaim (line 80–87). If the token isn't a valid JWT, it immediately returnstrue, triggeringAuthSessionExpiredErrorwithout attempting the server refresh (line 139).Backend verification confirms this is safe:
SIMPLE_JWTconfiguration (archery/settings.py) withRefreshToken.for_user()(api_auth/views.py) always produces proper JWTs withexpclaims. However, the architecture relies on an implicit client-side assumption. Consider letting the server validate and reject invalid refresh tokens (returning 401 from/auth/token/refresh/) rather than preemptively checking JWT format on the client.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/shared/auth/auth.ts` around lines 136 - 145, The client currently treats the stored refresh token as a JWT by calling isAccessTokenExpired(decodeJwtPayload) inside refreshAccessToken and throws AuthSessionExpiredError when decodeJwtPayload fails; instead, change refreshAccessToken to only check for presence of a refresh token (via getStoredRefreshToken) and not call isAccessTokenExpired/decodeJwtPayload on the refresh token so the server can validate format/expiry and return 401 if invalid; keep the existing refreshRequest concurrency guard and ensure any AuthSessionExpiredError is only thrown when there is no refresh token at all (or after a server 401), referencing the refreshAccessToken, getStoredRefreshToken, isAccessTokenExpired and AuthSessionExpiredError symbols to locate the spots to remove the preemptive JWT validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/app/AppShell.vue`:
- Around line 192-210: The Settings toggle button lacks ARIA state and control
linkage; update the button (the element using isSettingsRouteActive,
toggleSettingsMenu, isSidebarCollapsed and isSettingsMenuOpen) to include
aria-expanded bound to isSettingsMenuOpen and aria-controls referencing the id
of the submenu region; ensure the submenu element (the sibling that is
shown/hidden by isSettingsMenuOpen) has a matching unique id (e.g.,
settings-submenu or generated id) and appropriate role (region or menu) so
assistive tech can discover and announce the expanded/collapsed state and the
controlled region.
In `@frontend/src/app/feature-registry.ts`:
- Around line 55-66: The current matchesNavigationItem function uses
currentPath.startsWith(targetPath) which can incorrectly mark sibling routes as
active (e.g., "/workflow" matching "/workflows"); update the logic in
matchesNavigationItem to treat a prefix match as valid only when currentPath
equals targetPath or when currentPath starts with targetPath followed by a '/'
(i.e., check currentPath === targetPath || currentPath.startsWith(targetPath +
'/')), keep the existing special-case for '/' and continue deriving targetPath
from item.matchPrefix ?? item.to to locate the code to change.
In `@frontend/src/app/router.ts`:
- Around line 63-90: The catch blocks around ensureCurrentUser() (which calls
authStore.loadCurrentUser()) are currently treating any error as session expiry;
change them to only clear tokens and redirect to the login-expired flow when the
error is an AuthSessionExpiredError or an HTTP 401 response from
loadCurrentUser(), otherwise rethrow or navigate to a generic error route.
Concretely, update both try/catch blocks that call ensureCurrentUser() (used by
the settings branch that uses getFirstVisibleSettingsItem() and the meta.access
branch that uses canAccessRequirement()) so the catch inspects the error (e.g.,
instanceof AuthSessionExpiredError or error.status === 401) before calling
clearStoredTokens() and authStore.clearTokens() and returning { name: 'login',
query: { reason: 'expired' } }; for non-expiry errors either rethrow the error
or return a generic error navigation (e.g., { name: 'error' }) so
transient/network/server errors don't log users out.
In `@frontend/src/features/archives/manifest.ts`:
- Around line 11-22: The three route objects for Archives (route names
'archives', 'archive-new', 'archive-detail' with components ArchivesPage,
ArchiveCreatePage, ArchiveDetailPage) lack route-level access metadata while the
navigation item is gated by sql.menu_archive; add meta.access to each route
matching the nav guard (e.g., meta: { title: '...', access: { anyPermissions:
['sql.menu_archive'] } }) so the router authorization prevents direct access to
/archives, /archives/new, and /archives/:archiveId for users without the menu
permission.
In `@frontend/src/features/permissions/manifest.ts`:
- Around line 9-18: The route for '/permission-management' is unprotected while
the navigation item has an access guard; add the same access requirement to the
route's meta so direct URL access is guarded. Update the route entry that maps
path '/permission-management' / component PermissionManagementPage to include
meta.access with the same object used in navigation (anyPermissions:
['sql.menu_queryapplylist']), ensuring the route's meta contains title and the
new access field.
In `@frontend/src/features/settings/manifest.ts`:
- Line 16: The /settings landing route currently defined as { path: '/settings',
name: 'settings', component: SettingsLandingPage, meta: { title: 'Settings' } }
exposes the settings index without any access gate; either add an access
requirement to this route consistent with the sub-pages (e.g., same
staff/superuser/permission checks used by those sub-routes) or change
SettingsLandingPage to immediately redirect to the first visible/authorized
settings nav entry resolved by your registry logic; locate the route definition
(path '/settings', component SettingsLandingPage) and implement one of these two
fixes so users cannot land on an inaccessible overview page.
In `@frontend/src/shared/auth/access.ts`:
- Around line 11-20: hasPermission currently assumes currentUser.permissions
exists and calls .includes, which can throw if permissions is null/undefined;
update hasPermission to first check currentUser?.is_superuser, then verify that
currentUser.permissions is an array (e.g.,
Array.isArray(currentUser.permissions)) before calling .includes, and return
false if permissions is missing or not an array so callers (like
canAccessRequirement) won't crash; keep the function name hasPermission and
existing return behavior for superusers.
In `@frontend/src/shared/components/feedback/FeedbackAlert.vue`:
- Around line 2-8: hasError and hasSuccess are plain booleans computed once from
defineProps and won't update when props change; make them reactive by replacing
the current const hasError and const hasSuccess with Vue computed properties
(e.g., use computed(() => Boolean(props.error)) and computed(() =>
!hasError.value && Boolean(props.success))) or by inlining Boolean(props.error)
/ Boolean(props.success) checks in the template so the alert updates when
props.error or props.success change; update references to use .value if you
choose computed.
In `@frontend/src/shared/composables/use-route-query-sync.ts`:
- Around line 26-33: routeQueriesMatch currently compares compacted queries
using JSON.stringify which can differ when object key orders vary; to fix,
normalize key order before comparing by deep-sorting object keys (e.g.,
implement a small helper like deepSortKeys and call it on normalizedRouteQuery
and normalizedCurrentQuery) and then compare the results with JSON.stringify (or
use a stable stringify that sorts keys). Update routeQueriesMatch to use
compactRouteQuery, then deepSortKeys(compactResult) for both inputs (or reuse a
stable serializer) so equivalent queries with different insertion orders compare
equal.
---
Nitpick comments:
In `@frontend/package.json`:
- Line 16: The "test:unit" npm script currently hardcodes two test file paths
which will require manual updates; update the "test:unit" script in package.json
(the "test:unit" entry) to use vitest's automatic discovery (e.g., run vitest
without explicit file paths or with a glob) so new unit tests are picked up
automatically — replace the hardcoded command with a generic vitest invocation
like "vitest run" or "vitest run <glob>" in the "test:unit" script.
In `@frontend/src/app/AppShell.vue`:
- Around line 120-135: In logout(), avoid awaiting authStore.loadAuthConfig()
before clearing tokens; instead perform an immediate local sign-out by calling
authStore.clearTokens() first, then attempt to load or refresh auth config with
a bounded timeout (or read a cached authStore.authMode) to decide whether to
redirect to publicApiUrl('/auth/workos/logout/') or router.push('/login');
update the control flow in the logout function to use the cached authMode or a
timed promise for loadAuthConfig() so a slow/unreachable network call cannot
delay clearing tokens or the redirect decision.
In `@frontend/src/features/reports/pages/ReportsPage.vue`:
- Around line 14-23: The page currently renders duplicate header copy: the
PageHeader component (PageHeader title/description) and the CardHeader block
(CardTitle/CardDescription) both show the same "Operational Reports" title and
description; update the CardHeader inside ReportsPage.vue to be section-specific
(e.g., replace CardTitle/CardDescription with a contextual subtitle or remove
them) so only PageHeader contains the global page hero while CardHeader contains
a concise section label or is omitted for this card (locate PageHeader,
CardHeader, CardTitle, CardDescription in the file to implement the change).
In `@frontend/src/features/workflows/pages/WorkflowSqlEditorPage.vue`:
- Line 6: The import of SqlCodeEditor in WorkflowSqlEditorPage.vue couples
workflows to queries' internal components; move the editor to a shared UI
location or add a public export from the queries feature and update imports.
Specifically, extract the component symbol SqlCodeEditor into a
shared/components (or shared/ui) module and update WorkflowSqlEditorPage.vue to
import from the new shared path, or alternatively add an index export (e.g.,
queries/components/index.ts) that re-exports SqlCodeEditor and update the import
to use that public entrypoint so workflows no longer reference an internal path.
In `@frontend/src/shared/api/http.ts`:
- Around line 58-141: Add a short comment to the
RequestOptions/InternalRequestOptions definition explaining that auth gating
uses the presence of options.token (even an empty string) to enable
authentication and the automatic refresh/retry flow; reference the options.token
field and the request function's requiresAuth check so future callers know they
must pass a token value to get auth+retry behavior and won't silently make
unauthenticated requests.
In `@frontend/src/shared/auth/access.test.ts`:
- Around line 25-67: Add unit tests covering the missing branches in access
helpers: create tests that verify canAccessRequirement with requiresSuperuser:
true returns true for buildUser({is_superuser: true}) and false otherwise; add a
test where only anyPermissions is provided (no requiredPermissions) and ensure
canAccessRequirement returns true when user has one of the anyPermissions and
false when they don't; add a test with an empty/undefined requirement object
(call canAccessRequirement(user, {}) or canAccessRequirement(user, undefined))
and assert it returns true; and add a negative hasPermission test asserting
hasPermission(buildUser({is_superuser: false}), 'some.missing_permission')
returns false. Reference the functions hasPermission, canAccessRequirement and
helper buildUser to locate where to add these cases.
In `@frontend/src/shared/auth/auth.ts`:
- Around line 14-48: This file defines local duplicates of isRecord,
flattenErrorMessage, and parseResponseMessage; remove these local
implementations and instead import and use the shared implementations from the
central HTTP helpers, replacing all references to the local functions (isRecord,
flattenErrorMessage, parseResponseMessage) with the imported ones so
error-message formatting is consistent across refresh and main request paths.
Ensure you import the exported names exactly (isRecord, flattenErrorMessage,
parseResponseMessage) and run a quick build/test to verify no other local usages
remain.
- Around line 136-145: The client currently treats the stored refresh token as a
JWT by calling isAccessTokenExpired(decodeJwtPayload) inside refreshAccessToken
and throws AuthSessionExpiredError when decodeJwtPayload fails; instead, change
refreshAccessToken to only check for presence of a refresh token (via
getStoredRefreshToken) and not call isAccessTokenExpired/decodeJwtPayload on the
refresh token so the server can validate format/expiry and return 401 if
invalid; keep the existing refreshRequest concurrency guard and ensure any
AuthSessionExpiredError is only thrown when there is no refresh token at all (or
after a server 401), referencing the refreshAccessToken, getStoredRefreshToken,
isAccessTokenExpired and AuthSessionExpiredError symbols to locate the spots to
remove the preemptive JWT validation.
In `@frontend/src/shared/components/state/PageStateCard.vue`:
- Around line 10-16: The PageStateCard component currently renders only
CardHeader with CardTitle and CardDescription from props (title, description);
update it to expose a default slot for richer body content and an optional named
slot (e.g., "header" or "icon") for custom header/icon rendering while
preserving backward compatibility by falling back to the existing
CardTitle/CardDescription when those slots are not provided; modify the template
inside PageStateCard.vue to check for the presence of the named slots and render
them instead of the props, and keep props title/description as the default
content when slots are absent.
In `@frontend/src/stores/auth.ts`:
- Line 4: The auth store is importing feature-scoped API symbols
(fetchAuthConfig, fetchCurrentUserContext, AuthMode, CurrentUserContext) from
the auth feature barrel which couples global state to a feature; change the
import to the shared/app-level auth API entrypoint (the common auth API surface
used by the rest of the app) so useAuthStore depends on the core auth API
instead of '@/features/auth/api', and update any re-exports if necessary so the
same symbols are imported from the shared auth entrypoint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 94219d8f-2b44-4046-8e76-71a8d26539aa
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (83)
frontend/docs/enterprise-feature-contract.mdfrontend/package.jsonfrontend/src/App.vuefrontend/src/app/AppShell.vuefrontend/src/app/feature-contract.tsfrontend/src/app/feature-registry.test.tsfrontend/src/app/feature-registry.tsfrontend/src/app/install-auth-session.tsfrontend/src/app/router.tsfrontend/src/extensions/enterprise-feature-modules.tsfrontend/src/features/archives/api.tsfrontend/src/features/archives/manifest.tsfrontend/src/features/archives/pages/ArchiveCreatePage.vuefrontend/src/features/archives/pages/ArchiveDetailPage.vuefrontend/src/features/archives/pages/ArchivesPage.vuefrontend/src/features/auth/api.tsfrontend/src/features/auth/manifest.tsfrontend/src/features/auth/pages/LoginCallbackPage.vuefrontend/src/features/auth/pages/LoginPage.vuefrontend/src/features/auth/pages/ProfilePage.vuefrontend/src/features/dashboard/api.tsfrontend/src/features/dashboard/manifest.tsfrontend/src/features/dashboard/pages/HomePage.vuefrontend/src/features/inventory/api.tsfrontend/src/features/inventory/manifest.tsfrontend/src/features/inventory/pages/InventoryEditorPage.vuefrontend/src/features/inventory/pages/InventoryListPage.vuefrontend/src/features/permissions/api.tsfrontend/src/features/permissions/manifest.tsfrontend/src/features/permissions/pages/PermissionManagementPage.vuefrontend/src/features/queries/api.tsfrontend/src/features/queries/components/QueryMetadataExplorer.vuefrontend/src/features/queries/components/SqlCodeEditor.vuefrontend/src/features/queries/components/query-metadata-explorer.tsfrontend/src/features/queries/manifest.tsfrontend/src/features/queries/pages/QueriesPage.vuefrontend/src/features/reports/manifest.tsfrontend/src/features/reports/pages/ReportsPage.vuefrontend/src/features/settings/api.tsfrontend/src/features/settings/manifest.tsfrontend/src/features/settings/pages/SettingsGroupDetailPage.vuefrontend/src/features/settings/pages/SettingsGroupsPage.vuefrontend/src/features/settings/pages/SettingsInstanceTagDetailPage.vuefrontend/src/features/settings/pages/SettingsInstanceTagsPage.vuefrontend/src/features/settings/pages/SettingsLandingPage.vuefrontend/src/features/settings/pages/SettingsResourceGroupDetailPage.vuefrontend/src/features/settings/pages/SettingsResourceGroupsPage.vuefrontend/src/features/settings/pages/SettingsSystemPage.vuefrontend/src/features/settings/pages/SettingsUserDetailPage.vuefrontend/src/features/settings/pages/SettingsUsersPage.vuefrontend/src/features/settings/system-settings.tsfrontend/src/features/workflows/api.tsfrontend/src/features/workflows/manifest.tsfrontend/src/features/workflows/pages/DdlWorkflowCreatePage.vuefrontend/src/features/workflows/pages/DmlWorkflowCreatePage.vuefrontend/src/features/workflows/pages/ExportWorkflowCreatePage.vuefrontend/src/features/workflows/pages/WorkflowCreatePage.vuefrontend/src/features/workflows/pages/WorkflowDetailPage.vuefrontend/src/features/workflows/pages/WorkflowSqlEditorPage.vuefrontend/src/features/workflows/pages/WorkflowsPage.vuefrontend/src/lib/api.tsfrontend/src/lib/auth-session.tsfrontend/src/lib/auth.tsfrontend/src/lib/system-settings.tsfrontend/src/lib/utils.tsfrontend/src/main.tsfrontend/src/router/index.tsfrontend/src/shared/api/http.tsfrontend/src/shared/auth/access.test.tsfrontend/src/shared/auth/access.tsfrontend/src/shared/auth/auth.tsfrontend/src/shared/components/feedback/FeedbackAlert.vuefrontend/src/shared/components/layout/FormActions.vuefrontend/src/shared/components/layout/PageHeader.vuefrontend/src/shared/components/state/PageStateCard.vuefrontend/src/shared/composables/use-auth-token.tsfrontend/src/shared/composables/use-list-query-state.tsfrontend/src/shared/composables/use-route-query-sync.tsfrontend/src/shared/composables/use-user-facing-message.tsfrontend/src/shared/utils/cn.tsfrontend/src/stores/auth.tsfrontend/tsconfig.app.jsonfrontend/vite.config.ts
This was referenced Apr 23, 2026
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
1 participant
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.
Summary
app,shared, andfeatureslayers with manifest-driven routes and navigation.Testing
npm run test:unitnpm run buildnpm run e2eSummary by CodeRabbit
New Features
UI Components
Tests