From b505a654f80f099e610208c1e21672a41ad9819c Mon Sep 17 00:00:00 2001 From: ux-frontend Date: Thu, 21 May 2026 20:44:32 +0200 Subject: [PATCH] fix(frontend): address M1-M3 polish from code-reviewer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 + , 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. --- frontend/src/router.tsx | 64 ++++++++----------- frontend/src/router/RootLayout.tsx | 10 --- frontend/src/routing/Root.tsx | 16 +++++ .../src/screens/cockpit/LiveCockpitPage.tsx | 11 +++- tasks/todo.md | 31 +++++++-- 5 files changed, 76 insertions(+), 56 deletions(-) delete mode 100644 frontend/src/router/RootLayout.tsx create mode 100644 frontend/src/routing/Root.tsx diff --git a/frontend/src/router.tsx b/frontend/src/router.tsx index f3a7754..14acf40 100644 --- a/frontend/src/router.tsx +++ b/frontend/src/router.tsx @@ -1,5 +1,5 @@ import { createBrowserRouter, Navigate } from 'react-router-dom'; -import { RootLayout } from '@/router/RootLayout'; +import { Root } from '@/routing/Root'; import { AppShell } from '@/components/shell/AppShell'; import { LoginPage } from '@/screens/login/LoginPage'; import { EngagementsPage } from '@/screens/engagements/EngagementsPage'; @@ -11,48 +11,34 @@ import { AuditPage } from '@/screens/audit/AuditPage'; /** * 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([ { - path: '/', - element: ( - - - - ), - }, - { - path: '/login', - element: ( - - - - ), - }, - { - path: '/', - element: ( - - - - ), + element: , children: [ - { path: 'engagements', element: }, - { path: 'library', element: }, - { path: 'scenarios', element: }, - { path: 'runs', element: }, - { path: 'reports', element: }, - { path: 'audit', element: }, + { index: true, element: }, + { path: 'login', element: }, + { + element: , + children: [ + { path: 'engagements', element: }, + { path: 'library', element: }, + { path: 'scenarios', element: }, + { path: 'runs', element: }, + { path: 'reports', element: }, + { path: 'audit', element: }, + ], + }, + { path: '*', element: }, ], }, - { - path: '*', - element: ( - - - - ), - }, ]); diff --git a/frontend/src/router/RootLayout.tsx b/frontend/src/router/RootLayout.tsx deleted file mode 100644 index bca8088..0000000 --- a/frontend/src/router/RootLayout.tsx +++ /dev/null @@ -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 {children}; -} diff --git a/frontend/src/routing/Root.tsx b/frontend/src/routing/Root.tsx new file mode 100644 index 0000000..ec978f8 --- /dev/null +++ b/frontend/src/routing/Root.tsx @@ -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 ( + + + + ); +} diff --git a/frontend/src/screens/cockpit/LiveCockpitPage.tsx b/frontend/src/screens/cockpit/LiveCockpitPage.tsx index 8d987e6..9886469 100644 --- a/frontend/src/screens/cockpit/LiveCockpitPage.tsx +++ b/frontend/src/screens/cockpit/LiveCockpitPage.tsx @@ -46,6 +46,11 @@ export function LiveCockpitPage() { return { total, done, detected, missed, partial }; }, [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 (
{/* Run header + control bar */} @@ -54,7 +59,7 @@ export function LiveCockpitPage() { style={{ borderColor: 'var(--line-default)', backgroundColor: 'var(--surface-1)' }} >
-
// Live run · pollign 500 ms
+
// Live run · polling 500 ms

RUN-2026-05-21-A03 · Démo Client X

@@ -74,7 +79,7 @@ export function LiveCockpitPage() {
- {isLead(user?.role ?? 'rt_operator') && ( + {isLead(user.role) && (