From c254d487ba116416ebd8d5b6142c1c65bce6d611 Mon Sep 17 00:00:00 2001 From: Melvin Strobl Date: Sat, 15 Aug 2026 19:19:57 +0200 Subject: [PATCH] Recover from a dead session, and report a node problem once An expired or orphaned session left the app on a half-rendered page instead of the login screen: a properly signed token naming a user who no longer exists answered 404, which the client does not treat as an authentication failure. All failures in get_current_user are 401 now, and the client stops retrying them, so a dead session goes straight back to the login screen. - Clicking an edge names the two nodes it runs between, not just the message. - A node with a problem shows one badge carrying the whole explanation, rather than a corner badge and a status dot saying the same thing twice. The dot is back to what it is good at: whether the node ran. - The sidebar's collapse control sits in the sidebar, where a phone still finds one in the chrome because there is no sidebar on screen to hold it. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016WzrvW7rjQbynnhF6pxh6i --- backend/app/api/deps.py | 17 ++++--- backend/tests/api/routes/test_session.py | 45 +++++++++++++++++++ .../src/components/Flow/EdgeInspector.tsx | 19 ++++++-- frontend/src/components/Flow/FlowEditor.tsx | 7 ++- frontend/src/components/Flow/FlowNode.tsx | 31 ++++++++----- frontend/src/components/Flow/FlowTabs.tsx | 2 +- .../src/components/Sidebar/AppSidebar.tsx | 11 ++++- frontend/src/main.tsx | 16 ++++++- frontend/src/routes/_layout.tsx | 4 +- 9 files changed, 128 insertions(+), 24 deletions(-) create mode 100644 backend/tests/api/routes/test_session.py diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index c25ebd2..54c47a2 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -47,6 +47,12 @@ def user_from_token(session: Session, token: str) -> User | None: def get_current_user(session: SessionDep, token: TokenDep) -> User: + """Resolve the bearer token to its user. + + Every failure here is an authentication failure, so all of them answer 401: + a token naming a user who no longer exists is a session to log in again, + not a missing resource to report. + """ try: payload = jwt.decode( token, settings.SECRET_KEY, algorithms=[security.ALGORITHM] @@ -54,14 +60,15 @@ def get_current_user(session: SessionDep, token: TokenDep) -> User: token_data = TokenPayload(**payload) except (InvalidTokenError, ValidationError): raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, + status_code=status.HTTP_401_UNAUTHORIZED, detail="Could not validate credentials", ) user = session.get(User, token_data.sub) - if not user: - raise HTTPException(status_code=404, detail="User not found") - if not user.is_active: - raise HTTPException(status_code=400, detail="Inactive user") + if user is None or not user.is_active: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="This session is no longer valid — please log in again", + ) return user diff --git a/backend/tests/api/routes/test_session.py b/backend/tests/api/routes/test_session.py new file mode 100644 index 0000000..9d482bf --- /dev/null +++ b/backend/tests/api/routes/test_session.py @@ -0,0 +1,45 @@ +"""A session the server will not accept must say so as an auth failure. + +The client clears the stored token and returns to the login screen on 401. Any +other status leaves the browser holding a token that can never work, stuck on a +page that will not load. +""" + +import uuid +from datetime import timedelta + +from fastapi.testclient import TestClient + +from app.core import security +from app.core.config import settings + +PROTECTED = [ + f"{settings.API_V1_STR}/users/me", + f"{settings.API_V1_STR}/flows/", +] + + +def token_for(subject: str) -> str: + return security.create_access_token(subject, timedelta(days=1)) + + +def test_no_token_is_unauthorised(client: TestClient) -> None: + for path in PROTECTED: + assert client.get(path).status_code == 401 + + +def test_a_token_we_did_not_sign_is_unauthorised(client: TestClient) -> None: + headers = {"Authorization": "Bearer not.a.real.token"} + for path in PROTECTED: + assert client.get(path, headers=headers).status_code == 401 + + +def test_a_token_for_a_user_who_is_gone_is_unauthorised( + client: TestClient, +) -> None: + # Properly signed, but the user it names no longer exists — which is what a + # browser holds after the database is reset. + headers = {"Authorization": f"Bearer {token_for(str(uuid.uuid4()))}"} + for path in PROTECTED: + response = client.get(path, headers=headers) + assert response.status_code == 401, path diff --git a/frontend/src/components/Flow/EdgeInspector.tsx b/frontend/src/components/Flow/EdgeInspector.tsx index f2c8808..51b5105 100644 --- a/frontend/src/components/Flow/EdgeInspector.tsx +++ b/frontend/src/components/Flow/EdgeInspector.tsx @@ -1,3 +1,5 @@ +import { ArrowRight } from "lucide-react" + import { Button } from "@/components/ui/button" import { Popover, PopoverAnchor, PopoverContent } from "@/components/ui/popover" import { ScrollArea } from "@/components/ui/scroll-area" @@ -15,13 +17,15 @@ function relativeTime(ts: number | null | undefined): string { export type InspectedEdge = { message: string + /** Node titles, so the popover names the two ends in the user's own words. */ + from: string + to: string x: number y: number } /** - * What last travelled along an edge. The value is whatever the producing node - * published, shown as it was serialised. + * What last travelled along an edge, and between which two nodes. */ export function EdgeInspector({ edge, @@ -48,7 +52,16 @@ export function EdgeInspector({ className="w-72 p-3" data-testid="edge-inspector" > -

+

+ + {edge.from} + + + + {edge.to} + +

+

{displayName(flow, edge.message)}

diff --git a/frontend/src/components/Flow/FlowEditor.tsx b/frontend/src/components/Flow/FlowEditor.tsx index 1652d04..8c19884 100644 --- a/frontend/src/components/Flow/FlowEditor.tsx +++ b/frontend/src/components/Flow/FlowEditor.tsx @@ -166,7 +166,6 @@ function FlowEditorInner({ flowName }: { flowName: string }) { flow: flowName, typeLabel: typeLabels.get(definition?.type ?? "") ?? definition?.type ?? "", - issues: nodeIssues.length, issueText: nodeIssues.join("\n"), } satisfies FlowNodeData, } @@ -413,8 +412,14 @@ function FlowEditorInner({ flowName }: { flowName: string }) { setInspected(null) }} onEdgeClick={(event, edge) => { + const label = (id: string) => { + const node = definitions.find((entry) => entry.id === id) + return node?.title || node?.id || id + } setInspected({ message: (edge.data as { message: string }).message, + from: label(edge.source), + to: label(edge.target), x: event.clientX, y: event.clientY, }) diff --git a/frontend/src/components/Flow/FlowNode.tsx b/frontend/src/components/Flow/FlowNode.tsx index a230bd4..0d3da85 100644 --- a/frontend/src/components/Flow/FlowNode.tsx +++ b/frontend/src/components/Flow/FlowNode.tsx @@ -29,17 +29,17 @@ const NODE_ICONS = { mlp: Braces, } as const +// Only the states worth a quiet marker. Anything wrong goes to the badge +// instead, so a problem is never reported twice on the same node. const STATUS_STYLES = { running: { dot: "bg-primary animate-pulse", label: "Running" }, success: { dot: "bg-status-success", label: "Last run succeeded" }, - error: { dot: "bg-destructive", label: "Failed" }, } as const export type FlowNodeData = { definition: NodeDef_Input flow: string typeLabel: string - issues: number issueText: string [key: string]: unknown } @@ -83,15 +83,18 @@ function PortHandles({ } function FlowNodeComponent({ data, selected }: NodeProps) { - const { definition, flow, typeLabel, issues, issueText } = - data as FlowNodeData + const { definition, flow, typeLabel, issueText } = data as FlowNodeData const live = useNodeStatus(`${flow}.${definition.id}`) const Icon = NODE_ICONS[definition.type as keyof typeof NODE_ICONS] ?? Code2 - const status = live?.status === "active" ? undefined : live?.status - const style = status - ? STATUS_STYLES[status as keyof typeof STATUS_STYLES] - : undefined + // Whatever is wrong — it failed to load, it failed to run, or the graph + // around it does not add up — is one badge with one explanation. + const problem = [live?.status === "error" ? live.error : null, issueText] + .filter(Boolean) + .join("\n") + const style = problem + ? undefined + : STATUS_STYLES[live?.status as keyof typeof STATUS_STYLES] return (
- {issues > 0 ? ( + {problem ? ( - + - {issueText} + + {problem} + ) : null} diff --git a/frontend/src/components/Flow/FlowTabs.tsx b/frontend/src/components/Flow/FlowTabs.tsx index fb96651..046c981 100644 --- a/frontend/src/components/Flow/FlowTabs.tsx +++ b/frontend/src/components/Flow/FlowTabs.tsx @@ -131,7 +131,7 @@ export function FlowTabs({ transition={transitions.emphasized} className="pointer-events-auto absolute left-1/2 top-4 z-10 flex max-w-[calc(100vw-2rem)] -translate-x-1/2 items-center gap-1 rounded-full border border-border bg-card/80 px-1.5 py-1 shadow-e2 backdrop-blur-md" > - +
{flows.map((flow) => ( diff --git a/frontend/src/components/Sidebar/AppSidebar.tsx b/frontend/src/components/Sidebar/AppSidebar.tsx index 5298490..7f91df6 100644 --- a/frontend/src/components/Sidebar/AppSidebar.tsx +++ b/frontend/src/components/Sidebar/AppSidebar.tsx @@ -7,6 +7,7 @@ import { SidebarContent, SidebarFooter, SidebarHeader, + SidebarTrigger, } from "@/components/ui/sidebar" import useAuth from "@/hooks/useAuth" import { type Item, Main } from "./Main" @@ -33,7 +34,15 @@ export function AppSidebar() { className="[&>[data-sidebar=sidebar]]:bg-card/80 [&>[data-sidebar=sidebar]]:backdrop-blur-md [&>[data-sidebar=sidebar]]:shadow-e2" > - +
+ {/* Collapsed, the rail has room for one thing, and that is the way + back out. */} + + + + {/* On a phone the sidebar is a sheet with its own way in and out. */} + +
diff --git a/frontend/src/main.tsx b/frontend/src/main.tsx index 2ae6c66..5041c31 100644 --- a/frontend/src/main.tsx +++ b/frontend/src/main.tsx @@ -19,12 +19,17 @@ OpenAPI.TOKEN = async () => { return localStorage.getItem("access_token") || "" } +/** A session the server will not accept, whatever we do next. */ +const isAuthFailure = (error: unknown) => + error instanceof ApiError && [401, 403].includes(error.status) + const handleApiError = (error: Error) => { - if (error instanceof ApiError && [401, 403].includes(error.status)) { + if (isAuthFailure(error)) { localStorage.removeItem("access_token") window.location.href = "/login" } } + const queryClient = new QueryClient({ queryCache: new QueryCache({ onError: handleApiError, @@ -32,6 +37,15 @@ const queryClient = new QueryClient({ mutationCache: new MutationCache({ onError: handleApiError, }), + defaultOptions: { + queries: { + // Retrying an expired session only delays the trip to the login screen. + retry: (count, error) => !isAuthFailure(error) && count < 3, + }, + mutations: { + retry: false, + }, + }, }) const router = createRouter({ routeTree }) diff --git a/frontend/src/routes/_layout.tsx b/frontend/src/routes/_layout.tsx index fff838d..23421e9 100644 --- a/frontend/src/routes/_layout.tsx +++ b/frontend/src/routes/_layout.tsx @@ -25,7 +25,9 @@ function Layout() { -
+ {/* The sidebar carries its own collapse control; a phone has no + sidebar on screen to carry it. */} +