diff --git a/app/components/editor/FooterBar.tsx b/app/components/editor/FooterBar.tsx index 2f6253bd..69a57355 100644 --- a/app/components/editor/FooterBar.tsx +++ b/app/components/editor/FooterBar.tsx @@ -2,6 +2,7 @@ import { useState } from "react"; import { Button } from "@core/shared-ui"; import type { PresenceUser, PresenceStatus } from "~/hooks/usePresence"; import { m } from "~/paraglide/messages"; +import { SignInAgainLink } from "./SignInAgainLink"; interface FooterBarProps { connected?: boolean; @@ -91,12 +92,16 @@ export function FooterBar({ connected = false, status, roster = [] }: FooterBarP {effectiveStatus === 'connected' ? m.editor_footer_status_connected() + : effectiveStatus === 'expired' + ? {m.editor_session_expired()} : effectiveStatus === 'reconnecting' ? m.editor_footer_status_reconnecting() : m.editor_footer_status_connecting()} diff --git a/app/components/editor/MobileSyncPill.test.tsx b/app/components/editor/MobileSyncPill.test.tsx index 37717a65..b40c556c 100644 --- a/app/components/editor/MobileSyncPill.test.tsx +++ b/app/components/editor/MobileSyncPill.test.tsx @@ -11,6 +11,7 @@ */ import { describe, it, expect, vi } from "vitest"; import { render, screen } from "@testing-library/react"; +import { MemoryRouter } from "react-router"; import { MobileSyncPill } from "./MobileSyncPill"; @@ -31,4 +32,16 @@ describe("MobileSyncPill", () => { rerender(); expect(state()).toBe("connecting"); }); + + it("turns into the way back to this item when the session has expired", () => { + render( + + + , + ); + expect(state()).toBe("expired"); + const href = screen.getByRole("link", { name: "Session expired" }).getAttribute("href") ?? ""; + expect(href.startsWith("/login?")).toBe(true); + expect(new URLSearchParams(href.slice("/login?".length)).get("returnTo")).toBe("/inspections/i1/edit?section=s1&item=a1"); + }); }); diff --git a/app/components/editor/MobileSyncPill.tsx b/app/components/editor/MobileSyncPill.tsx index b51a65fd..b15cbee2 100644 --- a/app/components/editor/MobileSyncPill.tsx +++ b/app/components/editor/MobileSyncPill.tsx @@ -2,6 +2,7 @@ import { useEffect, useState } from "react"; import { listPendingMedia } from "~/lib/collab/media-pending-store"; import type { PresenceStatus } from "~/hooks/usePresence"; import { m } from "~/paraglide/messages"; +import { SignInAgainLink } from "./SignInAgainLink"; /** Poll interval while photos are queued. Slow on purpose: the pill is a * reassurance readout, not a progress bar, and this runs on a phone. */ @@ -24,7 +25,9 @@ const POLL_MS = 3000; * "Synced". `navigator.onLine` only reports that the phone has a network, not * that this server answers on it (one bar in a crawlspace, a captive portal, a * deploy in progress), and the edit made in that window is on the phone only. - * `connection` is the same status the desktop `FooterBar` shows. + * `connection` is the same status the desktop `FooterBar` shows. Its "expired" + * is the one state the inspector can end themselves, so there the label is the + * link that does it. */ export function MobileSyncPill({ inspectionId, connection = "connected" }: { inspectionId: string; connection?: PresenceStatus }) { const [online, setOnline] = useState(true); @@ -63,6 +66,8 @@ export function MobileSyncPill({ inspectionId, connection = "connected" }: { ins const label = state === "offline" ? m.editor_mobile_sync_offline({ count: pending }) + : state === "expired" + ? {m.editor_session_expired_short()} : state === "reconnecting" ? m.editor_footer_status_reconnecting() : state === "connecting" @@ -73,14 +78,14 @@ export function MobileSyncPill({ inspectionId, connection = "connected" }: { ins // A fresh page is "connecting" for a moment and that is not a warning, so // it stays neutral — the same reading the desktop footer gives it. const tone = - state === "offline" + state === "offline" || state === "expired" ? "bg-ih-bad-bg text-ih-bad-fg" : state === "connecting" ? "bg-ih-bg-muted text-ih-fg-2" : state === "synced" ? "bg-ih-ok-bg text-ih-ok-fg" : "bg-ih-watch-bg text-ih-watch-fg"; - const dot = state === "offline" ? "bg-ih-bad" : state === "connecting" ? "bg-ih-fg-3" : state === "synced" ? "bg-ih-ok" : "bg-ih-watch"; + const dot = state === "offline" || state === "expired" ? "bg-ih-bad" : state === "connecting" ? "bg-ih-fg-3" : state === "synced" ? "bg-ih-ok" : "bg-ih-watch"; return ( `: the cookies change underneath the page, and a full + * document load is the one navigation that cannot carry stale state across. + * + * The padding with its equal negative margin makes the tap target at least + * 44px tall (an 18px line plus 2 x 14px) without moving the 24px pill it sits in. + */ +export function SignInAgainLink({ children }: { children: ReactNode }) { + const { pathname, search } = useLocation(); + return ( + + {children} + + ); +} diff --git a/app/hooks/usePresence.test.ts b/app/hooks/usePresence.test.ts new file mode 100644 index 00000000..5a350376 --- /dev/null +++ b/app/hooks/usePresence.test.ts @@ -0,0 +1,105 @@ +// @vitest-environment happy-dom +/** + * A refused WebSocket handshake and a dead network look the same to a browser, + * so "expired" has to come from somewhere other than the socket. These pin where: + * a socket that closed without opening, plus a 401 from the API on the same + * cookie. Each "is not expired" case is the control for the first test: a hook + * that said "expired" on every failed handshake would pass it alone. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { usePresence } from "./usePresence"; + +/** A socket the test settles by hand: `open()` accepts the handshake, `close()` before it refuses. */ +class StubSocket extends EventTarget { + static readonly OPEN = 1; + static last: StubSocket; + readyState = 0; + constructor() { + super(); + StubSocket.last = this; + } + send(): void { /* presence frames are not under test */ } + open(): void { + this.readyState = 1; + this.dispatchEvent(new Event("open")); + } + close(): void { + this.readyState = 3; + this.dispatchEvent(new Event("close")); + } +} + +const fetchMock = vi.fn(); +const mount = () => renderHook(() => usePresence({ inspectionId: "i1", userId: "u1", userName: "Inspector" })); +/** Refuse the newest socket's handshake, then let the session check answer. */ +const refuse = () => act(async () => { + StubSocket.last.close(); + await vi.advanceTimersByTimeAsync(0); +}); + +beforeEach(() => { + vi.useFakeTimers(); + vi.stubGlobal("WebSocket", StubSocket); + vi.stubGlobal("fetch", fetchMock); + fetchMock.mockReset(); +}); + +afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); +}); + +describe("usePresence session expiry", () => { + it("reads expired when the handshake is refused and the API answers 401, until a retry opens", async () => { + fetchMock.mockResolvedValue({ status: 401 }); + const { result } = mount(); + const first = StubSocket.last; + + await refuse(); + expect(fetchMock).toHaveBeenCalledWith("/api/auth/me"); + expect(result.current.status).toBe("expired"); + + // The retry is refused too. The reading holds from the moment it closes, + // before its own check has answered: no flash of "Reconnecting…". + await act(() => vi.advanceTimersByTimeAsync(400)); + expect(StubSocket.last).not.toBe(first); + act(() => StubSocket.last.close()); + expect(result.current.status).toBe("expired"); + + // Signed in from another tab: the next retry is let in. + await act(() => vi.advanceTimersByTimeAsync(800)); + act(() => StubSocket.last.open()); + expect(result.current.status).toBe("connected"); + }); + + it("is not expired when the handshake is refused but the session is live", async () => { + fetchMock.mockResolvedValue({ status: 200 }); + const { result } = mount(); + await refuse(); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(result.current.status).toBe("connecting"); + }); + + it("is not expired when the API cannot be reached, and does not ask while offline", async () => { + fetchMock.mockRejectedValue(new TypeError("Failed to fetch")); + const { result } = mount(); + await refuse(); + expect(result.current.status).toBe("connecting"); + + fetchMock.mockClear(); + vi.spyOn(navigator, "onLine", "get").mockReturnValue(false); + await act(() => vi.advanceTimersByTimeAsync(400)); + await refuse(); + expect(fetchMock).not.toHaveBeenCalled(); + expect(result.current.status).toBe("connecting"); + }); + + it("does not ask when an open socket drops: that one was let in", async () => { + const { result } = mount(); + act(() => StubSocket.last.open()); + await refuse(); + expect(fetchMock).not.toHaveBeenCalled(); + expect(result.current.status).toBe("reconnecting"); + }); +}); diff --git a/app/hooks/usePresence.ts b/app/hooks/usePresence.ts index bc6f597f..567ce4e8 100644 --- a/app/hooks/usePresence.ts +++ b/app/hooks/usePresence.ts @@ -1,4 +1,5 @@ import { useState, useEffect, useCallback, useRef } from "react"; +import { isOffline } from "~/lib/network-failure"; /* ------------------------------------------------------------------ */ /* Types */ @@ -11,8 +12,12 @@ export interface PresenceUser { focusItemId?: string | null; } -/** FE-5 — tri-state so the UI can distinguish "still connecting" from "lost". */ -export type PresenceStatus = "connecting" | "connected" | "reconnecting"; +/** + * FE-5 — tri-state so the UI can distinguish "still connecting" from "lost". + * "expired" is a lost connection with a known cause: the server refuses the + * socket because there is no session, so no amount of retrying will reopen it. + */ +export type PresenceStatus = "connecting" | "connected" | "reconnecting" | "expired"; /* ------------------------------------------------------------------ */ /* Constants */ @@ -44,6 +49,9 @@ export function usePresence(options: { // read as data loss by field users. Track connecting/connected/reconnecting. const [status, setStatus] = useState("connecting"); const everConnectedRef = useRef(false); + // Held in a ref so each failed retry keeps the reading instead of flashing + // "Reconnecting…" until its own session check answers. + const expiredRef = useRef(false); const wsRef = useRef(null); const heartbeatRef = useRef | null>(null); const backoffRef = useRef(400); @@ -65,8 +73,13 @@ export function usePresence(options: { try { const ws = new WebSocket(wsUrl); wsRef.current = ws; + let opened = false; + const downStatus = (): PresenceStatus => + expiredRef.current ? "expired" : everConnectedRef.current ? "reconnecting" : "connecting"; ws.addEventListener("open", () => { + opened = true; + expiredRef.current = false; backoffRef.current = 400; everConnectedRef.current = true; setStatus("connected"); @@ -97,7 +110,22 @@ export function usePresence(options: { if (heartbeatRef.current) clearInterval(heartbeatRef.current); heartbeatRef.current = null; setConnected(false); - setStatus(everConnectedRef.current ? "reconnecting" : "connecting"); + setStatus(downStatus()); + // A browser is never told why a WebSocket handshake was refused: a 401 + // and a dead network both arrive as a bare `close`. So a socket that + // closed without opening asks the API, with the cookie the socket + // itself sends, whether there is still a session. Only a 401 reads as + // "expired"; the retries below carry on either way, so signing in from + // another tab reconnects this one. + if (!opened && !closedRef.current && !isOffline()) { + void fetch("/api/auth/me").then( + (res) => { + expiredRef.current = res.status === 401; + if (!closedRef.current && wsRef.current?.readyState !== WebSocket.OPEN) setStatus(downStatus()); + }, + () => { /* unreachable: a network problem, not a session one */ }, + ); + } // Schedule reconnect if (!closedRef.current) { setTimeout(() => connect(), backoffRef.current); diff --git a/app/lib/session.server.ts b/app/lib/session.server.ts index e371fd58..a50732be 100644 --- a/app/lib/session.server.ts +++ b/app/lib/session.server.ts @@ -135,7 +135,7 @@ export async function getToken(context: LoadContext, request: Request): Promise< * cheaper question: is it even worth sending? A token whose payload cannot be * read counts as expired, so an unreadable value can never be passed on. */ -function isTokenExpired(token: string, nowMs: number): boolean { +export function isTokenExpired(token: string, nowMs: number): boolean { const payload = token.split(".")[1]; if (!payload) return true; try { diff --git a/app/routes/login-saas-bounce.test.ts b/app/routes/login-saas-bounce.test.ts index 73fa1689..9effadd9 100644 --- a/app/routes/login-saas-bounce.test.ts +++ b/app/routes/login-saas-bounce.test.ts @@ -30,6 +30,11 @@ beforeEach(() => { type LoaderArgs = Parameters[0]; +// The loader reads `exp` from the token's payload and nothing else; the API is +// what verifies a signature. So a session is a payload with a future `exp`. +const cookieExpiring = (exp: number) => `__Host-inspector_token=h.${btoa(JSON.stringify({ exp }))}.s`; +const LIVE_COOKIE = cookieExpiring(4102444800); // 2100-01-01 + // happy-dom enforces the Fetch spec's forbidden-header list and silently drops a // `Cookie` header set on `new Request(...)`, so we hand-roll a minimal request // object whose `headers.get('Cookie')` we fully control (the loader only reads @@ -89,15 +94,22 @@ describe('login loader — returnTo (B3)', () => { it('redirects an already-authenticated user to the same-origin returnTo', async () => { const res = await loader( - args({}, { cookie: '__Host-inspector_token=fake.jwt.value', returnTo: '/oauth/authorize?x=1' }), + args({}, { cookie: LIVE_COOKIE, returnTo: '/oauth/authorize?x=1' }), ); expect(res).toBeInstanceOf(Response); expect((res as Response).headers.get('Location')).toBe('/oauth/authorize?x=1'); }); + it('shows the form, returnTo kept, when the token is past its exp (NET-06)', async () => { + // Bounced to returnTo, that page's loader would end the session and send + // the visitor back here with the destination gone. + const res = await loader(args({}, { cookie: cookieExpiring(1), returnTo: '/inspections/i1/edit?item=a' })); + expect(res).toEqual({ returnTo: '/inspections/i1/edit?item=a' }); + }); + it('ignores an off-origin returnTo for an authenticated user (no open redirect)', async () => { const res = await loader( - args({}, { cookie: '__Host-inspector_token=fake.jwt.value', returnTo: '//evil.test/phish' }), + args({}, { cookie: LIVE_COOKIE, returnTo: '//evil.test/phish' }), ); expect(res).toBeInstanceOf(Response); expect((res as Response).headers.get('Location')).toBe('/inspections'); @@ -129,7 +141,7 @@ describe('login loader — first run on a standalone instance (ONB-01)', () => { it('a signed-in visitor is not asked, and goes to the app', async () => { setupStatus.mockResolvedValue(status(false)); - const res = await loader(args({}, { cookie: '__Host-inspector_token=fake.jwt.value' })); + const res = await loader(args({}, { cookie: LIVE_COOKIE })); expect(setupStatus).not.toHaveBeenCalled(); expect((res as Response).headers.get('Location')).toBe('/inspections'); }); diff --git a/app/routes/login.tsx b/app/routes/login.tsx index 279b3f9b..269dec0e 100644 --- a/app/routes/login.tsx +++ b/app/routes/login.tsx @@ -2,7 +2,7 @@ import { Form, useActionData, useLoaderData, useNavigation, redirect } from "rea import { useForm } from "@conform-to/react"; import { parseWithZod } from "@conform-to/zod/v4"; import type { Route } from "./+types/login"; -import { getToken, createSessionWithToken } from "~/lib/session.server"; +import { getToken, isTokenExpired, createSessionWithToken } from "~/lib/session.server"; import { createApi } from "~/lib/api-client.server"; import { makeLoginSchema } from "~/lib/forms/auth.schema"; import { AuthShell } from "~/components/AuthShell"; @@ -37,8 +37,10 @@ export async function loader({ request, context }: Route.LoaderArgs) { // it to same-origin paths, so an attacker can't turn this into an open // redirect. Absent/invalid → /inspections (unchanged behavior). const returnTo = new URL(request.url).searchParams.get("returnTo"); + // A token past its `exp` is not a session: bouncing it to `returnTo` only has + // that page's loader send it straight back here with the destination dropped. const token = await getToken(context, request); - if (token) return redirect(safeReturnTo(returnTo, "/inspections")); + if (token && !isTokenExpired(token, Date.now())) return redirect(safeReturnTo(returnTo, "/inspections")); // ONB-01 — a standalone instance with no users has nobody who could sign in, // and `/setup` is only findable by already knowing the URL. Ask the question diff --git a/docs/develop/fork-log.md b/docs/develop/fork-log.md index f0098c24..3db73c96 100644 --- a/docs/develop/fork-log.md +++ b/docs/develop/fork-log.md @@ -319,6 +319,7 @@ 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 | ## Rules diff --git a/messages/en/editor-2.json b/messages/en/editor-2.json index 9f706ebd..e9c00609 100644 --- a/messages/en/editor-2.json +++ b/messages/en/editor-2.json @@ -257,5 +257,7 @@ "editor_limitation_reason_unsafe": "Unsafe to access", "editor_mobile_sync_ok": "Synced", "editor_mobile_sync_uploading": "Uploading {count}", - "editor_mobile_sync_offline": "Offline · {count} queued" + "editor_mobile_sync_offline": "Offline · {count} queued", + "editor_session_expired": "Session expired: log in to continue", + "editor_session_expired_short": "Session expired" } diff --git a/messages/es-419/editor-2.json b/messages/es-419/editor-2.json index f01ef2b1..ad752f75 100644 --- a/messages/es-419/editor-2.json +++ b/messages/es-419/editor-2.json @@ -257,5 +257,7 @@ "editor_limitation_reason_unsafe": "Acceso inseguro", "editor_mobile_sync_ok": "Sincronizado", "editor_mobile_sync_uploading": "Subiendo {count}", - "editor_mobile_sync_offline": "Sin conexión · {count} en cola" + "editor_mobile_sync_offline": "Sin conexión · {count} en cola", + "editor_session_expired": "La sesión expiró: inicia sesión para continuar", + "editor_session_expired_short": "Sesión expirada" }