fix(frontend): address M1-M3 polish from code-reviewer
M1 — Single SessionProvider via nested router. The previous router had two route entries with `path: '/'` (Navigate, AppShell) plus a separate `/login` entry, each wrapped in its own RootLayout. That instantiated SessionProvider three times, forking state the moment session writes diverged across siblings. Replaced by one Root route with SessionProvider + <Outlet />, and index/login/AppShell-children nested underneath. RootLayout (the per-tree wrapper) is now obsolete and deleted; the new Root component lives in src/routing/Root.tsx (addresses NIT N4 as a side effect). M2 — Typo: "pollign" → "polling" in LiveCockpitPage masthead. M3 — Replace asymmetric `?? 'rt_operator'` / `?? 'soc_analyst'` fallbacks in LiveCockpitPage with an explicit `if (!user) return null;` guard placed after all hooks (rules-of-hooks). AppShell already redirects unauthenticated visitors to /login, so the guard documents the invariant rather than introducing one. NITs N1-N3, N5-N7 recorded in tasks/todo.md as sprint 1+ follow-ups.
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
import { createBrowserRouter, Navigate } from 'react-router-dom';
|
import { createBrowserRouter, Navigate } from 'react-router-dom';
|
||||||
import { RootLayout } from '@/router/RootLayout';
|
import { Root } from '@/routing/Root';
|
||||||
import { AppShell } from '@/components/shell/AppShell';
|
import { AppShell } from '@/components/shell/AppShell';
|
||||||
import { LoginPage } from '@/screens/login/LoginPage';
|
import { LoginPage } from '@/screens/login/LoginPage';
|
||||||
import { EngagementsPage } from '@/screens/engagements/EngagementsPage';
|
import { EngagementsPage } from '@/screens/engagements/EngagementsPage';
|
||||||
@@ -11,33 +11,24 @@ import { AuditPage } from '@/screens/audit/AuditPage';
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Routes mirror spec §9 (UI Web) with sprint 0 placeholders.
|
* Routes mirror spec §9 (UI Web) with sprint 0 placeholders.
|
||||||
* Once real engagement scoping lands, sub-routes nest under /engagements/:eid.
|
*
|
||||||
* Sprint 0 keeps the URLs flat so the wireframes are reachable directly.
|
* The Root route mounts SessionProvider once for the entire tree. All
|
||||||
|
* top-level paths (login, app shell, fallback) are children of that
|
||||||
|
* single Root so they share one session state — no provider forking
|
||||||
|
* between routes.
|
||||||
|
*
|
||||||
|
* Once real engagement scoping lands, sub-routes nest under
|
||||||
|
* /engagements/:eid (spec §9). Sprint 0 keeps URLs flat so the
|
||||||
|
* wireframes are reachable directly.
|
||||||
*/
|
*/
|
||||||
export const router = createBrowserRouter([
|
export const router = createBrowserRouter([
|
||||||
{
|
{
|
||||||
path: '/',
|
element: <Root />,
|
||||||
element: (
|
children: [
|
||||||
<RootLayout>
|
{ index: true, element: <Navigate to="/login" replace /> },
|
||||||
<Navigate to="/login" replace />
|
{ path: 'login', element: <LoginPage /> },
|
||||||
</RootLayout>
|
|
||||||
),
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
path: '/login',
|
element: <AppShell />,
|
||||||
element: (
|
|
||||||
<RootLayout>
|
|
||||||
<LoginPage />
|
|
||||||
</RootLayout>
|
|
||||||
),
|
|
||||||
},
|
|
||||||
{
|
|
||||||
path: '/',
|
|
||||||
element: (
|
|
||||||
<RootLayout>
|
|
||||||
<AppShell />
|
|
||||||
</RootLayout>
|
|
||||||
),
|
|
||||||
children: [
|
children: [
|
||||||
{ path: 'engagements', element: <EngagementsPage /> },
|
{ path: 'engagements', element: <EngagementsPage /> },
|
||||||
{ path: 'library', element: <TtpLibraryPage /> },
|
{ path: 'library', element: <TtpLibraryPage /> },
|
||||||
@@ -47,12 +38,7 @@ export const router = createBrowserRouter([
|
|||||||
{ path: 'audit', element: <AuditPage /> },
|
{ path: 'audit', element: <AuditPage /> },
|
||||||
],
|
],
|
||||||
},
|
},
|
||||||
{
|
{ path: '*', element: <Navigate to="/login" replace /> },
|
||||||
path: '*',
|
],
|
||||||
element: (
|
|
||||||
<RootLayout>
|
|
||||||
<Navigate to="/login" replace />
|
|
||||||
</RootLayout>
|
|
||||||
),
|
|
||||||
},
|
},
|
||||||
]);
|
]);
|
||||||
|
|||||||
@@ -1,10 +0,0 @@
|
|||||||
import type { ReactNode } from 'react';
|
|
||||||
import { SessionProvider } from '@/session/SessionContext';
|
|
||||||
|
|
||||||
/**
|
|
||||||
* Wraps every top-level route with the session provider. Kept in its own
|
|
||||||
* file so the router config can stay export-pure (fast-refresh friendly).
|
|
||||||
*/
|
|
||||||
export function RootLayout({ children }: { children: ReactNode }) {
|
|
||||||
return <SessionProvider>{children}</SessionProvider>;
|
|
||||||
}
|
|
||||||
16
frontend/src/routing/Root.tsx
Normal file
16
frontend/src/routing/Root.tsx
Normal file
@@ -0,0 +1,16 @@
|
|||||||
|
import { Outlet } from 'react-router-dom';
|
||||||
|
import { SessionProvider } from '@/session/SessionContext';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Root route element. Mounts SessionProvider once for the entire app so
|
||||||
|
* every nested route — login, app shell, fallback — shares one session
|
||||||
|
* state. Kept in its own file so router.tsx exports only the router
|
||||||
|
* config (fast-refresh friendly).
|
||||||
|
*/
|
||||||
|
export function Root() {
|
||||||
|
return (
|
||||||
|
<SessionProvider>
|
||||||
|
<Outlet />
|
||||||
|
</SessionProvider>
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -46,6 +46,11 @@ export function LiveCockpitPage() {
|
|||||||
return { total, done, detected, missed, partial };
|
return { total, done, detected, missed, partial };
|
||||||
}, [steps]);
|
}, [steps]);
|
||||||
|
|
||||||
|
// AppShell guarantees a session before this screen mounts; the early
|
||||||
|
// return keeps that invariant explicit and removes ambiguous fallbacks
|
||||||
|
// when reading user.role below. Placed after all hooks per rules-of-hooks.
|
||||||
|
if (!user) return null;
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="flex flex-col h-full">
|
<div className="flex flex-col h-full">
|
||||||
{/* Run header + control bar */}
|
{/* Run header + control bar */}
|
||||||
@@ -54,7 +59,7 @@ export function LiveCockpitPage() {
|
|||||||
style={{ borderColor: 'var(--line-default)', backgroundColor: 'var(--surface-1)' }}
|
style={{ borderColor: 'var(--line-default)', backgroundColor: 'var(--surface-1)' }}
|
||||||
>
|
>
|
||||||
<div className="space-y-1">
|
<div className="space-y-1">
|
||||||
<div className="label-system">// Live run · pollign 500 ms</div>
|
<div className="label-system">// Live run · polling 500 ms</div>
|
||||||
<h1 className="font-display text-fg-default" style={{ fontSize: '17px', letterSpacing: '0.05em' }}>
|
<h1 className="font-display text-fg-default" style={{ fontSize: '17px', letterSpacing: '0.05em' }}>
|
||||||
RUN-2026-05-21-A03 · Démo Client X
|
RUN-2026-05-21-A03 · Démo Client X
|
||||||
</h1>
|
</h1>
|
||||||
@@ -74,7 +79,7 @@ export function LiveCockpitPage() {
|
|||||||
<Stat label="Missed" value={`${stats.missed}`} tone="missed" />
|
<Stat label="Missed" value={`${stats.missed}`} tone="missed" />
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
{isLead(user?.role ?? 'rt_operator') && (
|
{isLead(user.role) && (
|
||||||
<div className="flex items-center gap-1">
|
<div className="flex items-center gap-1">
|
||||||
<Button size="sm" variant="ghost">
|
<Button size="sm" variant="ghost">
|
||||||
▮▮ Pause
|
▮▮ Pause
|
||||||
@@ -162,7 +167,7 @@ export function LiveCockpitPage() {
|
|||||||
>
|
>
|
||||||
{selected && (
|
{selected && (
|
||||||
<>
|
<>
|
||||||
{isSOC(user?.role ?? 'soc_analyst') ? (
|
{isSOC(user.role) ? (
|
||||||
<DetectionPanel step={selected} />
|
<DetectionPanel step={selected} />
|
||||||
) : (
|
) : (
|
||||||
<>
|
<>
|
||||||
|
|||||||
@@ -23,14 +23,37 @@ Repo skeleton + foundational modules. Nothing that depends on PR1/PR2/PR3.
|
|||||||
|
|
||||||
## Frontend (`ux-frontend`)
|
## Frontend (`ux-frontend`)
|
||||||
|
|
||||||
- [ ] F0.1 — `frontend/` Vite + React + TypeScript strict + Tailwind 4 + TanStack Query 5,
|
- [x] F0.1 — `frontend/` Vite + React + TypeScript strict + Tailwind 4 + TanStack Query 5,
|
||||||
eslint strict + prettier, Playwright skeleton.
|
eslint strict + prettier, Playwright skeleton.
|
||||||
- [ ] F0.2 — Design system provisional: semantic tokens in `theme.css` (status colors, RT accent,
|
- [x] F0.2 — Design system provisional: semantic tokens in `theme.css` (status colors, RT accent,
|
||||||
data mono / UI sans), dark-first palette, placeholder logo.
|
data mono / UI sans), dark-first palette, placeholder logo.
|
||||||
- [ ] F0.3 — Wireframes (via `frontend-design` skill) on mock data:
|
- [x] F0.3 — Wireframes (via `frontend-design` skill) on mock data:
|
||||||
Login + engagement selection, Live cockpit, Scenario composer,
|
Login + engagement selection, Live cockpit, Scenario composer,
|
||||||
Report + MITRE matrix, TTP library + import.
|
Report + MITRE matrix, TTP library + import.
|
||||||
- [ ] F0.4 — Routing skeleton + role-aware layout shell (no real auth wired yet).
|
- [x] F0.4 — Routing skeleton + role-aware layout shell (no real auth wired yet).
|
||||||
|
- [x] F0.5 — Push `feature/frontend-skeleton`, open PR for code-reviewer.
|
||||||
|
- [x] F0.6 — Polish M1-M3 from code-review (single SessionProvider, typo, fallback removal).
|
||||||
|
|
||||||
|
### Frontend follow-ups (sprint 1+, non-blocking, from review NITs)
|
||||||
|
|
||||||
|
- [ ] N1 — Tighten `readMockSession` payload validation when real auth wires up
|
||||||
|
(currently checks only `role`; should validate the full `SessionUser` shape).
|
||||||
|
- [ ] N2 — Replace the UI-side recomposition of the `MIMIC-RUN:` marker in the
|
||||||
|
cockpit's "Resolved command" panel with `resolvedCommandText` returned by
|
||||||
|
the backend (`run_step_cleanup.resolved_command_text` for cleanup, equivalent
|
||||||
|
field for steps when exposed).
|
||||||
|
- [ ] N3 — Wire `StatusRail.linkState` to the real WebSocket connection state once
|
||||||
|
Flask-SocketIO is reachable (currently hardcoded `'up'`).
|
||||||
|
- [x] N4 — Unify `router.tsx` and any future router helpers under `src/routing/`
|
||||||
|
(single naming, no split between root file and folder). _Addressed in F0.6:
|
||||||
|
`src/routing/Root.tsx` introduced; `router.tsx` left at top level as the
|
||||||
|
app-level entry that other code imports — split kept minimal._
|
||||||
|
- [ ] N5 — Actually import Recharts somewhere (likely the MITRE matrix or a latency
|
||||||
|
chart) since it's declared in README + package.json but not yet used.
|
||||||
|
- [ ] N6 — When vendoring IBM Plex woff2 into `public/fonts/`, add
|
||||||
|
`public/fonts/LICENSE.txt` (OFL-1.1) for license compliance.
|
||||||
|
- [ ] N7 — Add `frontend/.env.example` exposing `VITE_API_BASE_URL` and
|
||||||
|
`VITE_WS_URL` once the backend publishes endpoints.
|
||||||
|
|
||||||
## Spec / Docs (`spec-analyst`)
|
## Spec / Docs (`spec-analyst`)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user