fix(ui): audit batch — auth, sessions, settings, admin pages, mocks#270
Closed
JoshuaAFerguson wants to merge 5 commits into
Closed
fix(ui): audit batch — auth, sessions, settings, admin pages, mocks#270JoshuaAFerguson wants to merge 5 commits into
JoshuaAFerguson wants to merge 5 commits into
Conversation
Four real bugs surfaced + fixed during a Playwright-driven audit pass
through Login → Dashboard → My Sessions → Session Viewer.
ui/src/store/userStore.ts — defensive expiresAt fallback
TypeScript LoginResponse.expiresAt is required, but TS contracts
aren't enforced at the network boundary. A backend (or MSW mock)
returning {token, user} without expiresAt left the field undefined,
isTokenExpired() returned true on the next render, and the route
guard silently bounced the user back to /login with no error shown.
Default to 24h ahead when missing so the session is at least usable
until the next /auth/me call refreshes state.
ui/src/pages/Sessions.tsx — initial REST fetch + correct empty-state
Page relied entirely on useSessionsWebSocket() for data with no
initial REST seed, so a failing WS (mock mode, network glitch, or
any race) left the page silently empty forever. Wired up the
existing useSessions() hook to seed `sessions` on mount; WS pushes
still win the race when faster.
Also fixed empty-state copy that pointed at a non-existent
"Template Catalog" — there's no Catalog link in the sidebar.
Replaced with a "Browse Applications" inline action that routes to
the My Applications page (the actual launch surface).
ui/src/lib/toast.ts — dedupe identical toasts within 2s window
A single page navigation that triggers N parallel API requests all
returning 5xx (e.g. Sessions page hitting /sessions/:id/connect plus
/users/me/quota during a brief backend hiccup) stacked the same red
"Server error. Please try again later." 3-4× simultaneously, which
looked like the app was dying. 2s is long enough to absorb a request
burst but short enough that a real second occurrence still shows up.
ui/src/mocks/handlers.ts — mock infra completeness pass
- /auth/login: include expiresAt (paired with userStore fix above)
- /sessions: wrap in {sessions, total} envelope to match real
backend response shape (api/internal/api/handlers.go ListSessions);
bare-array response made api.listSessions() return undefined
- Removed undefined MOCK_SESSIONS.vnc reference that produced a
trailing null in the array
- Added stub handlers for ~25 endpoints called by the UI but never
mocked, returning [] / {} so pages render their empty states
instead of stacking 500s from MSW's default unhandled-request
behavior. Real backend returns real data; mocks just need to not
crash the audit.
Long-term follow-up: generate the MSW handlers + the API client from
a single OpenAPI spec exposed by the Go backend so they can never
drift. Three of the four bugs above are the same class — TS contract
not enforced at the network boundary because mock and backend shapes
were hand-maintained separately.
Card layout had three accumulated bugs: 1. Three stacked status pills per card (state Chip + phase Chip + ActivityIndicator) all conveying the same conceptual state. Consolidated to a single phase Chip; ActivityIndicator now renders inline below only when isIdle && state === 'running'. 2. Long template names, session names, and URLs overflowed the card. Added flex ellipsis (minWidth: 0 on parent + overflow/textOverflow/ whiteSpace on the text node) with title= attributes for full value on hover. 3. CardActions wrapped inconsistently — Connect button on its own line on narrow cards. flexWrap: 'wrap' + gap keeps the row coherent and wraps cleanly when needed. Added title= to icon-only buttons for a11y.
QuotaCard imported `UserQuota as _UserQuota` (type-only) and tried to suppress the unused-import lint with `void (_UserQuota)`. TS erases type-only imports entirely, but Vite/esbuild kept the void expression, so the runtime threw `ReferenceError: _UserQuota is not defined` and ErrorBoundary caught the entire Settings route — Settings was 100% broken. The type wasn't actually used in the file. Removing both lines unblocks the page. The correct way to silence the lint for genuinely needed type-only imports is an eslint-disable comment, not a runtime void.
UI audit batch 2 — fixes uncovered while walking admin routes in MSW mode.
- Add /api/v1/metrics handler returning the ClusterMetrics shape the
admin Dashboard requires (cluster.{nodes,sessions,resources,users}).
Without this, useMetrics() resolves undefined and Dashboard renders
"Failed to load metrics".
- Add /api/v1/admin/agents handler with the {agents,total,page,limit}
envelope the admin Agents page expects (the /agents handler was on a
different path; admin/agents was 500-ing).
- Wrap /api/v1/users response in {users,total} envelope to match the
api.listUsers contract — bare array deserialized as [] and the User
Management page showed "No users found" with two seeded users.
- Rename MOCK_SESSIONS timestamp fields from snake_case (created_at,
last_activity) to camelCase (createdAt, lastActivity) to match the
Session interface in api.ts. Was rendering "Invalid Date" in the
Recent Sessions table on the admin Dashboard.
The backend route is GET /sessions/:id/connect (api/cmd/main.go:637) and the API client uses GET (api.connectSession). The mock used POST, so the SessionViewer flow 500'd on every load with "Failed to connect to session". Switching the mock to GET unblocks the viewer chrome.
5 tasks
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| <Chip | ||
| label={session.status.phase || session.state} | ||
| size="small" | ||
| color={getPhaseColor(session.status.phase) || getStateColor(session.state)} |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac2e9fe761
ℹ️ 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".
Comment on lines
+153
to
+157
| if (initialSessions && sessions.length === 0) { | ||
| const filtered = username | ||
| ? initialSessions.filter((s) => s.user === username) | ||
| : initialSessions; | ||
| setSessions(filtered); |
There was a problem hiding this comment.
The new seeding effect only copies initialSessions when sessions.length === 0, so once local state is populated it stops tracking React Query updates. That breaks the explicit WebSocket-fallback path: useUpdateSessionState/useDeleteSession invalidate ['sessions'], but when the socket is down the refetched REST data is ignored and the UI can keep showing stale or deleted sessions. This regression is introduced here because the page now relies on local state that no longer updates from query results after the first seed.
Useful? React with 👍 / 👎.
Contributor
|
This pull request has been automatically marked as stale because it has not had recent activity. Action Required:
|
github-actions
Bot
added
the
stale
Contributor
|
This pull request was automatically closed due to inactivity. If you believe this was closed in error, please reopen it. |
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.
Summary
Live-walk audit of every UI route through MSW + Playwright surfaced 5 distinct bugs ranging from a P0 page-killer to mock-data fidelity gaps. All fixed in this batch:
Verification
Walked all major routes in Playwright after each fix:
All routes render without ErrorBoundary catches.
Out of scope
Three known cosmetic issues noted but not fixed (low priority):
Test plan