fix(auth): a session the API refuses ends at /login, not at a default shell (QA sweep ONB-04)

app/routes/auth-layout.loader.test.ts

@@ -0,0 +1,71 @@
+// ONB-04 — a session the API no longer honours (issued before a password change
+// or reset, or its user was removed) used to load the workspace shell at 200
+// with default branding: "Unnamed", openinspection.dev, no Dispatch. The layout
+// loader swallowed the 401 from the session-context call as a "graceful
+// fallback". It must end the session and send the visitor to /login instead.
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { loader } from '~/routes/auth-layout';
+import { createLoadContext } from '~/lib/load-context';
+
+const { contextGet } = vi.hoisted(() => ({ contextGet: vi.fn() }));
+vi.mock('~/lib/api-client.server', () => ({
+    createApi: () => ({ sessionContext: { context: { $get: contextGet } } }),
+}));
+
+// Braces matter: vitest calls a function returned from a hook as its teardown,
+// and `mockReset()` returns the mock itself.
+beforeEach(() => {
+    contextGet.mockReset();
+});
+
+type LoaderArgs = Parameters<typeof loader>[0];
+
+// `requireToken` reads `exp` from the payload and nothing else (the API is what
+// verifies the signature), so a live session is a payload with a future `exp`.
+// Same hand-rolled request as login-saas-bounce.test.ts: happy-dom drops a
+// `Cookie` header set on `new Request(...)`.
+const LIVE_COOKIE = `__Host-inspector_token=h.${btoa(JSON.stringify({ exp: 4102444800 }))}.s`;
+
+function args(pathname = '/inspections'): LoaderArgs {
+    const request = {
+        url: `http://app.example.com${pathname}`,
+        headers: { get: (name: string) => (name.toLowerCase() === 'cookie' ? LIVE_COOKIE : null) },
+    };
+    return { request, context: createLoadContext({}), params: {} } as unknown as LoaderArgs;
+}
+
+const json = (body: unknown, status = 200) => new Response(JSON.stringify(body), { status });
+
+describe('auth layout loader — session the API refuses (ONB-04)', () => {
+    it('a 401 clears the session and redirects to /login instead of rendering a default shell', async () => {
+        contextGet.mockResolvedValue(json({ success: false, error: 'Token has been invalidated' }, 401));
+        const thrown = await loader(args()).then(
+            () => null,
+            (e: unknown) => e,
+        );
+        expect(thrown).toBeInstanceOf(Response);
+        const res = thrown as Response;
+        expect(res.status).toBe(302);
+        expect(res.headers.get('Location')).toBe('/login');
+        expect(res.headers.get('Set-Cookie')).toMatch(/__Host-inspector_token=;.*Max-Age=0/);
+    });
+
+    it('a server error keeps the defaults: the token may be fine, so the session is not ended', async () => {
+        contextGet.mockResolvedValue(json({ success: false }, 500));
+        const res = await loader(args());
+        expect((res as { data: unknown }).data).toEqual({ context: null });
+    });
+
+    it('an unreachable API keeps the defaults too', async () => {
+        contextGet.mockRejectedValue(new TypeError('fetch failed'));
+        const res = await loader(args());
+        expect((res as { data: unknown }).data).toEqual({ context: null });
+    });
+
+    it('a 200 hands the session context to the page', async () => {
+        const ctx = { user: { locale: null }, branding: { defaultLocale: null } };
+        contextGet.mockResolvedValue(json({ success: true, data: ctx }));
+        const res = await loader(args());
+        expect((res as { data: unknown }).data).toEqual({ context: ctx });
+    });
+});

app/routes/auth-layout.tsx

@@ -2,7 +2,7 @@ import { useCallback, useEffect, useMemo, useState } from "react";
 import { data, Outlet, useLoaderData, useLocation, useNavigate, useNavigation } from "react-router";
 import { uiLocaleStampFor } from "../../server/lib/i18n/ui-locale";
 import type { Route } from "./+types/auth-layout";
-import { requireToken } from "~/lib/session.server";
+import { destroyUserSession, requireToken } from "~/lib/session.server";
 import { createApi } from "~/lib/api-client.server";
 import { CommandPalette, CommandPaletteProvider } from "~/components/CommandPalette";
 import { Sidebar, MobileHeader } from "~/components/Sidebar";
@@ -35,16 +35,25 @@ function useDelayedFlag(active: boolean, delayMs: number): boolean {
 export async function loader({ request, context }: Route.LoaderArgs) {
   const token = await requireToken(context, request);
   let sessionContext: SessionContext | null = null;
+  let refused = false;
   try {
     const api = createApi(context, { token });
     const res = await api.sessionContext.context.$get();
+    refused = res.status === 401;
     if (res.ok) {
       const body = (await res.json()) as Record<string, unknown>;
       sessionContext = body.data as SessionContext;
     }
   } catch {
     // Graceful fallback — layout renders with defaults
   }
+  // A 401 is the API saying this token is no longer a session: issued before a
+  // password change or reset, or its user was removed. `requireToken` cannot see
+  // that (the payload still reads as live), and the fallback above would paint
+  // an "Unnamed" shell at 200 for a signed-out visitor. Only 401 ends the
+  // session; a 5xx or a throw keeps the defaults, since the token may be fine.
+  // Outside the try so the thrown redirect is not swallowed by its catch.
+  if (refused) throw await destroyUserSession(context, request);
 
   // i18n activation (#269) — the DATABASE half of locale resolution.
   //

docs/develop/fork-log.md

@@ -319,7 +319,8 @@ deliberate departure rather than work upstream simply has not done yet.
 | `editor` notes reach the doc on idle (2026-10-12 QA sweep CORE-01; no upstream fix) | `setNotes` still echoes into `results` per keystroke, but now also schedules the doc write 600 ms after the last change, and `pagehide` / `visibilitychange: hidden` flush it; blur (`commitNotes`) writes at once. The path moved out of `collab-findings-api.ts` into `app/lib/collab/finding-notes.ts`. Dictation (`DictationButton`) and the comment typeahead pick call `setNotes`, so they ride the same timer; `insertComment` and `cloneLast` flush a pending note before they write. Spec: `app/lib/collab/finding-notes.test.tsx` | The doc write waited for the textarea's blur, so a reload, a discarded tab or a crash before blur lost the note while the header said "Synced". `git log main..upstream/main -- app/lib/collab/` is empty, so upstream has the same blur-only design. The caret-jump concern behind blur-only still holds for per-keystroke writes: the idle write commits the text the textarea already shows, so the rebuilt `results` carries an identical string. Not changed: the "Synced" indicator itself (NET-06) |
 | `auth` first-run link (2026-10-12 QA sweep ONB-01; no upstream fix) | The `/login` loader in `app/routes/login.tsx`, on a profile with `hasSetupWizard`, asks `GET /api/auth/setup-status` and redirects to `/setup` when the answer is an explicit `isSetUp: false`. Any other outcome (refusal, unreachable API, malformed body) shows the login form. `/` already lands on `/login`, so one loader covers both. Spec: the ONB-01 block in `app/routes/login-saas-bounce.test.ts` | A fresh instance's `/` went to a login form with no way to `/setup`, which was findable only by already knowing the URL. `git log main..upstream/main` on `app/routes/login.tsx` and `setup.tsx` has no setup-link change (the hits are 2FA, invite links and submit latches). `/setup` already redirects to `/login` once users exist; the explicit-`false` rule keeps the two from looping. The loader's call is one more in-process API call on an unauthenticated standalone `/login`, so `lint:middleware-budget` was re-baselined (`login.tsx` fan-out 2 -> 3, total 541 -> 542); that gate runs only in CI, which is where it was caught. Not changed: ONB-02 (operator-only agent-terms note on `/setup`) |
 | `wizard` network drop (2026-10-12 QA sweep NET-02; no upstream fix) | `app/routes/inspections.tsx` exports a `clientAction` that runs `guardWizardNetwork` (`app/lib/wizard-submit.ts`): a create whose fetch dies comes back as `{ ok: false, error: { code: "NETWORK" } }`, the shape the wizard already holds open on (`8632b8f7`), so the draft stays and Create is the retry; the agent and client typeaheads get an empty list; any other intent, a redirect or a non-network error is rethrown. The wizard submits nothing when `isOffline()` (`app/lib/network-failure.ts`, which `isNetworkFailure` now also uses) and skips the conflict and holiday loads and the typeahead while offline (read when the debounce fires, not when it is scheduled). Spec: `guardWizardNetwork` in `app/lib/wizard-submit.test.ts`, the offline case in `NewInspectionWizard.test.tsx` | A failed fetcher is routed by React Router to the route's error boundary, which unmounted the wizard with the inspector's draft. `git log main..upstream/main` on the wizard files has only focus-latch and overlay work. The `clientAction` is its own chunk (`splitRouteModules: "enforce"`), fetched on first use, hence the `isOffline()` pre-check. **Known ceiling:** a conflict or holiday load that fails mid-flight while `navigator.onLine` still says true (a bar of signal, no route) still unmounts the wizard. `file-size-baseline.json` raised for `inspections.tsx` (929 -> 934) and `NewInspectionWizard.tsx` (562 -> 571) |
-| `editor` session expiry (2026-10-13 QA sweep NET-06; no upstream fix) | `usePresence` (`app/hooks/usePresence.ts`) has a fourth status, `"expired"`: a presence socket that closes without having opened asks `GET /api/auth/me` with the browser's own cookie, and a 401 reads as expired. A 200, an unreachable API or `isOffline()` leaves "Connecting…" / "Reconnecting…". The retries carry on, so logging in from another tab reconnects this one. `MobileSyncPill` and `FooterBar` show that status as `SignInAgainLink`, an anchor to `/login?returnTo=` this page with its `?section=` / `?item=`. The `/login` loader no longer bounces a token past its `exp` to `returnTo` (`isTokenExpired`, now exported from `app/lib/session.server.ts`). Specs: `app/hooks/usePresence.test.ts`, the expired case in `MobileSyncPill.test.tsx`, the NET-06 case in `app/routes/login-saas-bounce.test.ts` | A browser is never told the status of a refused WebSocket handshake, so once the 24-hour token ran out the editor retried under "Reconnecting…" with the edits held in IndexedDB, and the next navigation went to a bare `/login`. `git log main..upstream/main` by keyword (401, session, socket, collab) and on `app/lib/collab` and `app/hooks` has no such change. The check hangs off the presence socket, not the Yjs one: it is the status both readouts already show, and both sockets are refused by the same cookie check (`server/api/inspections/core.ts`, `collab.ts`). The API's JWT middleware lets a request with no cookie through, and `collab/ws` answers a plain GET with 426 before it checks auth, so the socket's own URL cannot be the probe. **Known ceilings:** a token invalidated by a password change but not yet past `exp` is still bounced from `/login` to `returnTo`; under saas `/login` redirects to the portal and drops `returnTo`; on an origin where the browser refuses the `__Host-` cookie (plain http that is not `localhost`) the readout says expired |
+| `editor` session expiry (2026-10-13 QA sweep NET-06; no upstream fix) | `usePresence` (`app/hooks/usePresence.ts`) has a fourth status, `"expired"`: a presence socket that closes without having opened asks `GET /api/auth/me` with the browser's own cookie, and a 401 reads as expired. A 200, an unreachable API or `isOffline()` leaves "Connecting…" / "Reconnecting…". The retries carry on, so logging in from another tab reconnects this one. `MobileSyncPill` and `FooterBar` show that status as `SignInAgainLink`, an anchor to `/login?returnTo=` this page with its `?section=` / `?item=`. The `/login` loader no longer bounces a token past its `exp` to `returnTo` (`isTokenExpired`, now exported from `app/lib/session.server.ts`). Specs: `app/hooks/usePresence.test.ts`, the expired case in `MobileSyncPill.test.tsx`, the NET-06 case in `app/routes/login-saas-bounce.test.ts` | A browser is never told the status of a refused WebSocket handshake, so once the 24-hour token ran out the editor retried under "Reconnecting…" with the edits held in IndexedDB, and the next navigation went to a bare `/login`. `git log main..upstream/main` by keyword (401, session, socket, collab) and on `app/lib/collab` and `app/hooks` has no such change. The check hangs off the presence socket, not the Yjs one: it is the status both readouts already show, and both sockets are refused by the same cookie check (`server/api/inspections/core.ts`, `collab.ts`). The API's JWT middleware lets a request with no cookie through, and `collab/ws` answers a plain GET with 426 before it checks auth, so the socket's own URL cannot be the probe. **Known ceilings:** a token invalidated by a password change but not yet past `exp` is still bounced from `/login` to `returnTo`, where the auth layout now ends the session (ONB-04 row below) and the visitor lands on a bare `/login`, `returnTo` dropped; under saas `/login` redirects to the portal and drops `returnTo`; on an origin where the browser refuses the `__Host-` cookie (plain http that is not `localhost`) the readout says expired |
+| `auth` stale session ends at /login (2026-10-13 QA sweep ONB-04; no upstream fix) | The loader in `app/routes/auth-layout.tsx` ends the session (`destroyUserSession`: both cookies cleared, 302 to `/login`) when the API answers its session-context call with 401. Only 401: a 5xx or a thrown fetch still falls back to the default shell. Specs: `app/routes/auth-layout.loader.test.ts`; the end-to-end case sits in `tests/e2e/auth-password-reset.spec.ts` (a token from before a reset gets 302 to `/login`) | A token issued before a password change or reset passes `requireToken` (its `exp` is live) and is refused by the API (`pwchanged:{userId}`), but the loader swallowed that refusal as a "graceful fallback", so the page came back 200 with "Unnamed", `openinspection.dev` and no Dispatch, and a reload did the same. `git log main..upstream/main` by keyword (stale session, pwchanged, password reset) and on the auth-boundary files finds only the TOTP and invite-link fixes, neither of which touches this. **Known ceilings:** only a document load runs the layout loader, so a stale session that navigates client-side between child routes sees each child's own best-effort fallback until the next full load; `returnTo` is not carried; no `?reason=` notice on `/login` (the upstream plan floated `password-changed`, which would need a message in both catalogs) |
 
 ## Rules
 

tests/e2e/auth-password-reset.spec.ts

@@ -123,6 +123,9 @@ test.describe('Reset password', () => {
     expect(adminToken, 'admin must authenticate').toBeTruthy();
     const joinToken = await inviteMember(request, adminToken, email);
     await acceptInvite(request, joinToken, initialPassword);
+    // A session from BEFORE the reset, kept to prove the reset ends it (ONB-04).
+    const staleToken = await loginApi(request, email, initialPassword);
+    expect(staleToken, 'initial password must authenticate').toBeTruthy();
 
     // 2. Request a reset through the real UI; the email sink captures the link.
     await page.goto(`${BASE_URL}/forgot-password`, { timeout: NAV_TIMEOUT, waitUntil: 'networkidle' });
@@ -154,5 +157,20 @@ test.describe('Reset password', () => {
       headers: { 'Content-Type': 'application/json', ...csrfHeaders().headers },
     });
     expect(stale.status(), 'old password must be rejected after reset').toBe(401);
+
+    // 7. A session from before the reset is ended at the page, not rendered as a
+    // default shell (ONB-04). Asserted on status and Location, like
+    // route-auth-boundary.spec.ts. The fresh session is the control: if every
+    // cookie bounced to /login this would say nothing about the stale one.
+    const asPage = (token: string) =>
+      request.get(`${BASE_URL}/inspections`, {
+        maxRedirects: 0,
+        headers: { Cookie: `__Host-inspector_token=${token}` },
+      });
+    const fresh = await asPage(newToken);
+    expect(fresh.headers()['location'] ?? '', 'a live session must not be bounced').not.toMatch(/\/login/);
+    const old = await asPage(staleToken);
+    expect(old.status(), 'a session from before the reset must not render').toBe(302);
+    expect(old.headers()['location']).toMatch(/\/login/);
   });
 });