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
This commit is contained in:
Marc-André Bombeck committed 2026-08-06 14:32:32 +02:00
1 parent 21e111c06c
commit c42df8cbc3
8 files changed
+361 -21

No files matched your search

+61 -5
View File
@@ -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");
}
});
@@ -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);
});
});
+56 -6
View File
@@ -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;
}
@@ -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<Response>((_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,
});
});
});
@@ -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<string, unknown> {
+6 -3
View File
@@ -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,
});
}
+22 -7
View File
@@ -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<DashboardSnapshot> {
async function fetchDashboardSnapshot(
signal?: AbortSignal,
): Promise<DashboardSnapshot> {
// 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<DashboardSnapshot>("/api/dashboard/snapshot");
return apiGet<DashboardSnapshot>("/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<DashboardSnapshot>;
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<DashboardSnapshot> | 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) {
+33
View File
@@ -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);
};
}
/**