From c42df8cbc34e54c9e963511e734d6b7707816720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 6 Aug 2026 14:32:32 +0200 Subject: [PATCH] fix(02-12): gate record preload scope - Wait for a validated account scope before warming record reads - Abort scoped preloads and discard handoffs across record transitions - Cover malformed scope reloads and fresh own-record recovery --- e2e/v137-sharing-managed-profiles.spec.ts | 66 +++++++++++++-- .../providers-record-preloader.test.ts | 80 +++++++++++++++++++ src/components/providers.tsx | 62 ++++++++++++-- .../__tests__/prefetch-medications.test.ts | 72 +++++++++++++++++ .../__tests__/use-dashboard-snapshot.test.tsx | 31 +++++++ src/lib/queries/prefetch-medications.ts | 9 ++- src/lib/queries/use-dashboard-snapshot.ts | 29 +++++-- src/lib/query-keys/record-scope.ts | 33 ++++++++ 8 files changed, 361 insertions(+), 21 deletions(-) create mode 100644 src/components/__tests__/providers-record-preloader.test.ts create mode 100644 src/lib/queries/__tests__/prefetch-medications.test.ts diff --git a/e2e/v137-sharing-managed-profiles.spec.ts b/e2e/v137-sharing-managed-profiles.spec.ts index 2a2a503d2..8850556f1 100644 --- a/e2e/v137-sharing-managed-profiles.spec.ts +++ b/e2e/v137-sharing-managed-profiles.spec.ts @@ -138,7 +138,7 @@ test.describe.serial("scoped sharing browser journeys", () => { }); } - test("refuses a malformed switched payload before owner controls or reads mount", async ({ + test("refuses a malformed switched payload before target or owner reads mount", async ({ page, }) => { await page.goto("/"); @@ -149,14 +149,20 @@ test.describe.serial("scoped sharing browser journeys", () => { ); await expect(entry).toHaveCount(1); - let corruptNextAuthPayload = true; + // The switch normally writes this mirror before its full reload. Remove it + // in the new document so the global preloaders cannot guess an own-record + // scope while `/api/auth/me` is still deciding whether the target is safe. + await page.addInitScript(() => { + localStorage.removeItem("healthlog-record-scope"); + }); + + let corruptAuthPayload = true; await page.route("**/api/auth/me", async (route) => { const response = await route.fetch(); - if (!corruptNextAuthPayload) { + if (!corruptAuthPayload) { await route.fulfill({ response }); return; } - corruptNextAuthPayload = false; await route.fulfill({ response, json: withDivergentActiveAccountAccess(await response.json()), @@ -182,6 +188,24 @@ test.describe.serial("scoped sharing browser journeys", () => { } }; page.on("request", trackOwnerOnlyRead); + const targetPreloads: string[] = []; + const trackTargetPreload = (request: { + method(): string; + url(): string; + }) => { + if (request.method() !== "GET") return; + const path = new URL(request.url()).pathname; + if ( + [ + "/api/dashboard/snapshot", + "/api/medications", + "/api/medications/compliance", + ].includes(path) + ) { + targetPreloads.push(path); + } + }; + page.on("request", trackTargetPreload); try { await entry.click(); @@ -196,14 +220,46 @@ test.describe.serial("scoped sharing browser journeys", () => { await expect( page.locator('[data-slot="shared-record-banner"]'), ).toHaveCount(0); - expect(ownerOnlyReads).toEqual([]); + // Both global preloader routes must remain inert while the malformed + // response is refused, even though the scope mirror was intentionally + // cleared before each reload. + await page.goto("/"); + await expect( + page.locator('[data-slot="invalid-record-access-refusal"]'), + ).toBeVisible(); + await page.goto("/medications"); + await expect( + page.locator('[data-slot="invalid-record-access-refusal"]'), + ).toBeVisible(); + expect(ownerOnlyReads).toEqual([]); + expect(targetPreloads).toEqual([]); + + // Leave the refused record, then demand fresh own-record reads. This + // catches a response that was parked under the owner hash while the + // refusal was on screen and would otherwise paint without a new request. + corruptAuthPayload = false; + const freshDashboard = page.waitForResponse( + (response) => + response.request().method() === "GET" && + new URL(response.url()).pathname === "/api/dashboard/snapshot", + ); await page .locator('[data-slot="shared-record-unavailable-leave"]') .click(); await expect(page).toHaveURL(/\/$/); + await freshDashboard; + + const freshMedications = page.waitForResponse( + (response) => + response.request().method() === "GET" && + new URL(response.url()).pathname === "/api/medications", + ); + await page.goto("/medications"); + await freshMedications; } finally { page.off("request", trackOwnerOnlyRead); + page.off("request", trackTargetPreload); await page.unroute("**/api/auth/me"); } }); diff --git a/src/components/__tests__/providers-record-preloader.test.ts b/src/components/__tests__/providers-record-preloader.test.ts new file mode 100644 index 000000000..f33ab9434 --- /dev/null +++ b/src/components/__tests__/providers-record-preloader.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from "vitest"; +import { isRecordPreloadReady } from "@/components/providers"; +import { + __resetRecordScopeForTests, + setRecordScope, + setRefusedRecordScope, +} from "@/lib/query-keys/record-scope"; + +describe("isRecordPreloadReady", () => { + it("waits for the account response before warming record reads", () => { + __resetRecordScopeForTests(); + + expect( + isRecordPreloadReady({ isLoading: true, accountAccessStatus: "valid" }), + ).toBe(false); + expect( + isRecordPreloadReady({ + isLoading: false, + accountAccessStatus: undefined, + }), + ).toBe(false); + }); + + it("accepts only a resolved own-record response with an own-record scope", () => { + __resetRecordScopeForTests(); + setRecordScope(null); + + expect( + isRecordPreloadReady({ + isLoading: false, + accountAccessStatus: "valid", + activeAccountId: null, + }), + ).toBe(true); + + setRecordScope("shared-record"); + expect( + isRecordPreloadReady({ + isLoading: false, + accountAccessStatus: "valid", + activeAccountId: null, + }), + ).toBe(false); + }); + + it("accepts a shared-record response only after its exact scope is mirrored", () => { + __resetRecordScopeForTests(); + setRecordScope("shared-record"); + + expect( + isRecordPreloadReady({ + isLoading: false, + accountAccessStatus: "valid", + activeAccountId: "shared-record", + }), + ).toBe(true); + + setRecordScope(null); + expect( + isRecordPreloadReady({ + isLoading: false, + accountAccessStatus: "valid", + activeAccountId: "shared-record", + }), + ).toBe(false); + }); + + it("never opens a preload window for an explicitly refused record", () => { + __resetRecordScopeForTests(); + setRefusedRecordScope(); + + expect( + isRecordPreloadReady({ + isLoading: false, + accountAccessStatus: "invalid", + activeAccountId: null, + }), + ).toBe(false); + }); +}); diff --git a/src/components/providers.tsx b/src/components/providers.tsx index 37dfa184d..39407f5a6 100644 --- a/src/components/providers.tsx +++ b/src/components/providers.tsx @@ -22,9 +22,14 @@ import { Toaster } from "@/components/ui/sonner"; import { VersionPoller } from "@/components/version-poller"; import { ServiceWorkerRegistrar } from "@/components/service-worker-registrar"; import { SharedRecordGrantLossBridge } from "@/components/layout/shared-record-grant-loss-bridge"; +import { useAuth, type AccountAccessStatus } from "@/hooks/use-auth"; import { isDashboardSnapshotEnabled } from "@/lib/dashboard/snapshot-flag"; import { prefetchDashboardSnapshot } from "@/lib/queries/use-dashboard-snapshot"; import { prefetchMedicationsList } from "@/lib/queries/prefetch-medications"; +import { + getRecordScope, + subscribeToRecordScope, +} from "@/lib/query-keys/record-scope"; import { restorePersistedQueryCache, startPersistingQueryCache, @@ -168,15 +173,57 @@ function ThemeProvider({ children }: { children: ReactNode }) { // the router commits to "/" instead of waiting for the dashboard page // chunk to download + mount (~450 ms later on a 4G / 4x-CPU profile). // The proxy has already enforced auth + onboarding for "/" before any -// client code runs here, so the prefetch cannot fire for an -// unauthenticated visitor; a racing edge case just yields a swallowed -// 401 prefetch and the mounted cell re-resolves normally. +// client code runs here. It does not, however, resolve the active shared +// record: that verdict arrives from `/api/auth/me`. Do not begin a record read +// until that answer and the local cache scope agree. +export function isRecordPreloadReady({ + isLoading, + accountAccessStatus, + activeAccountId = null, +}: { + isLoading: boolean; + accountAccessStatus: AccountAccessStatus | undefined; + activeAccountId?: string | null; +}): boolean { + return ( + !isLoading && + (accountAccessStatus === "absent" || accountAccessStatus === "valid") && + getRecordScope() === activeAccountId + ); +} + +function subscribeToClientRecordScope(onStoreChange: () => void): () => void { + return subscribeToRecordScope(onStoreChange); +} + +function getClientRecordScope(): string | null { + return getRecordScope(); +} + +function getServerRecordScope(): string | null { + return null; +} + function DashboardSnapshotPreloader() { const queryClient = useQueryClient(); const pathname = usePathname(); + const { user, isLoading } = useAuth(); + const recordScope = useSyncExternalStore( + subscribeToClientRecordScope, + getClientRecordScope, + getServerRecordScope, + ); + const ready = isRecordPreloadReady({ + isLoading, + accountAccessStatus: user?.accountAccessStatus, + activeAccountId: user?.accountAccess?.active?.accountId ?? null, + }); + useEffect(() => { + if (!ready) return; + const controller = new AbortController(); if (pathname === "/" && isDashboardSnapshotEnabled()) { - prefetchDashboardSnapshot(queryClient); + prefetchDashboardSnapshot(queryClient, controller.signal); } // v1.16.7 — same waterfall cut for the medications page: its list // query (which carries the per-medication `nextDueAt` the due cells @@ -185,9 +232,12 @@ function DashboardSnapshotPreloader() { // download; the nav links additionally prefetch on hover/touch // intent, so this is the fallback for direct loads + reloads. if (pathname === "/medications") { - prefetchMedicationsList(queryClient); + prefetchMedicationsList(queryClient, controller.signal); } - }, [pathname, queryClient]); + // A transition to an unresolved, refused, or different record scope must + // abort a preloader before its response can be adopted by the next route. + return () => controller.abort(); + }, [pathname, queryClient, ready, recordScope]); return null; } diff --git a/src/lib/queries/__tests__/prefetch-medications.test.ts b/src/lib/queries/__tests__/prefetch-medications.test.ts new file mode 100644 index 000000000..bf718c69c --- /dev/null +++ b/src/lib/queries/__tests__/prefetch-medications.test.ts @@ -0,0 +1,72 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { QueryClient } from "@tanstack/react-query"; +import { QUERY_CLIENT_DEFAULT_OPTIONS } from "@/lib/pwa/query-client-options"; +import { queryKeys } from "@/lib/query-keys"; +import { + __resetRecordScopeForTests, + setRecordScope, +} from "@/lib/query-keys/record-scope"; +import { prefetchMedicationsList } from "../prefetch-medications"; + +function envelopeResponse(data: unknown): Response { + return new Response(JSON.stringify({ data, error: null }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); +} + +afterEach(() => { + __resetRecordScopeForTests(); + vi.unstubAllGlobals(); +}); + +describe("prefetchMedicationsList", () => { + it("does not park an aborted shared-record preload in the own-record cache", async () => { + setRecordScope("shared-record"); + const controller = new AbortController(); + const fetchMock = vi.fn( + (_path: string, init?: RequestInit) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => { + reject(new DOMException("Aborted", "AbortError")); + }); + }), + ); + vi.stubGlobal("fetch", fetchMock); + const client = new QueryClient({ + defaultOptions: QUERY_CLIENT_DEFAULT_OPTIONS, + }); + + prefetchMedicationsList(client, controller.signal); + controller.abort(); + await vi.waitFor(() => { + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + setRecordScope(null); + + expect(client.getQueryData(queryKeys.medications())).toBeUndefined(); + expect( + client.getQueryData(queryKeys.medicationComplianceSummary()), + ).toBeUndefined(); + }); + + it("passes a lifecycle signal to both record reads", async () => { + const controller = new AbortController(); + const fetchMock = vi.fn().mockResolvedValue(envelopeResponse([])); + vi.stubGlobal("fetch", fetchMock); + const client = new QueryClient({ + defaultOptions: QUERY_CLIENT_DEFAULT_OPTIONS, + }); + + prefetchMedicationsList(client, controller.signal); + await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); + + expect(fetchMock.mock.calls[0]?.[1]).toMatchObject({ + signal: controller.signal, + }); + expect(fetchMock.mock.calls[1]?.[1]).toMatchObject({ + signal: controller.signal, + }); + }); +}); diff --git a/src/lib/queries/__tests__/use-dashboard-snapshot.test.tsx b/src/lib/queries/__tests__/use-dashboard-snapshot.test.tsx index ba22b54c1..9205493a0 100644 --- a/src/lib/queries/__tests__/use-dashboard-snapshot.test.tsx +++ b/src/lib/queries/__tests__/use-dashboard-snapshot.test.tsx @@ -43,6 +43,10 @@ import { } from "../use-dashboard-snapshot"; import { retryOnceOnTransientError } from "../retry-transient"; import { queryKeys } from "@/lib/query-keys"; +import { + __resetRecordScopeForTests, + setRecordScope, +} from "@/lib/query-keys/record-scope"; import type { QueryClient } from "@tanstack/react-query"; /** Envelope-shaped Response — the snapshot fetch rides `apiGet` now. */ @@ -58,6 +62,7 @@ afterEach(() => { getQueryDataMock.mockReset(); setQueryDataMock.mockClear(); _resetDashboardSnapshotPreloadForTests(); + __resetRecordScopeForTests(); vi.unstubAllGlobals(); }); @@ -257,6 +262,32 @@ describe("prefetchDashboardSnapshot — hydration-safe promise handoff", () => { expect(snap.layout).toEqual(layout); expect(fetchMock).toHaveBeenCalledTimes(2); }); + + it("drops a parked shared-record response after the record scope changes", async () => { + setRecordScope("shared-record"); + const fetchMock = vi + .fn() + .mockResolvedValueOnce( + envelopeResponse({ layout: { owner: true }, tiles: {} }), + ) + .mockResolvedValueOnce( + envelopeResponse({ layout: { owner: false }, tiles: {} }), + ); + vi.stubGlobal("fetch", fetchMock); + getQueryDataMock.mockReturnValue({ widgets: [{ id: "existing" }] }); + + prefetchDashboardSnapshot(fakeQueryClient()); + setRecordScope(null); + + useDashboardSnapshot(); + const opts = lastOptsOf(useQueryMock); + const snapshot = await ( + opts.queryFn as () => Promise<{ layout: unknown }> + )(); + + expect(snapshot.layout).toEqual({ owner: false }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); }); function lastOptsOf(mock: typeof useQueryMock): Record { diff --git a/src/lib/queries/prefetch-medications.ts b/src/lib/queries/prefetch-medications.ts index 4ac9bd05a..b9902f6e3 100644 --- a/src/lib/queries/prefetch-medications.ts +++ b/src/lib/queries/prefetch-medications.ts @@ -29,15 +29,18 @@ import { apiGet } from "@/lib/api/api-fetch"; */ export const MEDICATIONS_LIST_STALE_TIME_MS = 15_000; -export function prefetchMedicationsList(queryClient: QueryClient): void { +export function prefetchMedicationsList( + queryClient: QueryClient, + signal?: AbortSignal, +): void { void queryClient.prefetchQuery({ queryKey: queryKeys.medications(), - queryFn: () => apiGet("/api/medications"), + queryFn: () => apiGet("/api/medications", { signal }), staleTime: MEDICATIONS_LIST_STALE_TIME_MS, }); void queryClient.prefetchQuery({ queryKey: queryKeys.medicationComplianceSummary(), - queryFn: () => apiGet("/api/medications/compliance"), + queryFn: () => apiGet("/api/medications/compliance", { signal }), staleTime: MEDICATIONS_LIST_STALE_TIME_MS, }); } diff --git a/src/lib/queries/use-dashboard-snapshot.ts b/src/lib/queries/use-dashboard-snapshot.ts index 62c99c199..30b476ed8 100644 --- a/src/lib/queries/use-dashboard-snapshot.ts +++ b/src/lib/queries/use-dashboard-snapshot.ts @@ -40,16 +40,19 @@ import { import { apiGet } from "@/lib/api/api-fetch"; import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; +import { getRecordScope } from "@/lib/query-keys/record-scope"; import { DASHBOARD_REFETCH_INTERVAL_MS } from "@/lib/queries/refetch-interval"; import { retryOnceOnTransientError } from "@/lib/queries/retry-transient"; import type { DashboardSnapshot } from "@/lib/dashboard/snapshot"; -async function fetchDashboardSnapshot(): Promise { +async function fetchDashboardSnapshot( + signal?: AbortSignal, +): Promise { // Routed through the typed wrapper so a failure carries its HTTP // status (`ApiError`) for the transient-retry predicate, and the // default 15 s timeout keeps a stalled response from pinning the // whole dashboard on its skeleton. - return apiGet("/api/dashboard/snapshot"); + return apiGet("/api/dashboard/snapshot", { signal }); } /** Handoff freshness window — mirrors the hook's `staleTime`. */ @@ -68,6 +71,7 @@ const PRELOAD_MAX_AGE_MS = 60_000; let preloadedSnapshot: { promise: Promise; startedAt: number; + recordScope: string | null; } | null = null; export function _resetDashboardSnapshotPreloadForTests(): void { @@ -88,7 +92,10 @@ export function _resetDashboardSnapshotPreloadForTests(): void { * `queryFn` consumes it; preload errors are swallowed by design — the * mounted cell owns error surfacing with its own fresh attempt. */ -export function prefetchDashboardSnapshot(queryClient: QueryClient) { +export function prefetchDashboardSnapshot( + queryClient: QueryClient, + signal?: AbortSignal, +) { // Expired handoff from a previous visit — drop it so a long-idle slot // can never serve stale data. if ( @@ -128,22 +135,30 @@ export function prefetchDashboardSnapshot(queryClient: QueryClient) { if (freshest > 0) { return; } - const promise = fetchDashboardSnapshot(); + const recordScope = getRecordScope(); + const promise = fetchDashboardSnapshot(signal); promise.catch(() => { // Swallow + clear so an unconsumed failed preload (racing 401 on // login, transient network) never becomes an unhandled rejection and // never gets handed to a later mount. if (preloadedSnapshot?.promise === promise) preloadedSnapshot = null; }); - preloadedSnapshot = { promise, startedAt: Date.now() }; + preloadedSnapshot = { promise, startedAt: Date.now(), recordScope }; } /** Single-use consume of the preload handoff; `null` when absent/expired. */ function takePreloadedSnapshot(): Promise | null { if (!preloadedSnapshot) return null; - const { promise, startedAt } = preloadedSnapshot; + const { promise, startedAt, recordScope } = preloadedSnapshot; preloadedSnapshot = null; - return Date.now() - startedAt < PRELOAD_MAX_AGE_MS ? promise : null; + // A handoff is not query-cached, so it needs the same record boundary as a + // query cache entry. A scope move while it is in flight must make the next + // mounted dashboard fetch afresh instead of adopting a response from the + // record that just ended. + return Date.now() - startedAt < PRELOAD_MAX_AGE_MS && + recordScope === getRecordScope() + ? promise + : null; } function makeSnapshotQueryFn(queryClient: QueryClient) { diff --git a/src/lib/query-keys/record-scope.ts b/src/lib/query-keys/record-scope.ts index bad85d65c..35c5d74b6 100644 --- a/src/lib/query-keys/record-scope.ts +++ b/src/lib/query-keys/record-scope.ts @@ -60,6 +60,11 @@ const REFUSED_RECORD_SCOPE = "__healthlog_refused_record__"; */ let scope: string | null = null; let seeded = false; +const scopeListeners = new Set<() => void>(); + +function notifyScopeListeners(): void { + for (const listener of scopeListeners) listener(); +} function readMirror(): string | null { try { @@ -112,6 +117,34 @@ export function setRecordScope(accountId: string | null): void { } catch { /* storage unavailable — the in-process copy still partitions this tab */ } + notifyScopeListeners(); +} + +/** + * React bridge for consumers that must stop work as soon as another tab moves + * the session to a different record. `storage` never fires in the tab that + * wrote the mirror, so local writes notify directly above; other tabs adopt + * the new value before notifying their subscribers. + */ +export function subscribeToRecordScope(listener: () => void): () => void { + scopeListeners.add(listener); + if (typeof window === "undefined") { + return () => scopeListeners.delete(listener); + } + const onStorage = (event: StorageEvent) => { + if (event.key !== STORAGE_KEY) return; + scope = + event.newValue === null || event.newValue.length === 0 + ? null + : event.newValue; + seeded = true; + notifyScopeListeners(); + }; + window.addEventListener("storage", onStorage); + return () => { + scopeListeners.delete(listener); + window.removeEventListener("storage", onStorage); + }; } /**