From 4aaed5c3121026531bf3556e999fa47f86dc66cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 3 Sep 2026 05:19:52 +0200 Subject: [PATCH 1/4] fix(dashboard): give the time-of-day greeting one clock Three pieces of code answered "what hour is it" for the salutation. The snapshot builder computed a `greetingHour` in the user's zone and put it on the payload so clients would not have to; no client ever read it. The insights hero derived one through the shared `hourInTz`. The dashboard header derived one through a local helper that fell back to the DEVICE clock whenever the runtime could not resolve the configured zone, which is what happens to an IANA entry the browser's data does not yet carry. At one instant, for one account, those three returned 20, 21 and 05. Keep the client end and drop the server one. The hour is a clock reading, not a fact about the record, and the snapshot body serves stale for up to an hour past its fresh TTL, so a stored hour can name a salutation the clock has left behind. What the server IS authoritative for is the zone, and that already travels on the same payload and is already what both surfaces read. Both greetings now share `hourInTz`, so an unresolvable zone falls back to the same default the rest of the app uses instead of to the device. The header's private helper is gone; it had no other caller. Two tests pinned the old shape by asserting the payload carried an hour. Inverted rather than deleted: the payload must now be shown NOT to carry one, which is the property that was missing when the field went unread for months. `greetingHour` leaves the response contract, so a native client decoding it as required needs the field made optional or dropped. --- docs/api/openapi.yaml | 5 - e2e/utils/mock-dashboard-snapshot.ts | 1 - .../greeting-hour-single-source.test.ts | 131 ++++++++++++++++++ .../snapshot/__tests__/route.test.ts | 8 +- src/components/dashboard/dashboard-header.tsx | 11 +- src/components/dashboard/range-display.tsx | 18 --- src/lib/dashboard/__tests__/snapshot.test.ts | 8 +- src/lib/dashboard/snapshot.ts | 31 ++--- src/lib/openapi/routes/insights/schemas.ts | 1 - 9 files changed, 159 insertions(+), 55 deletions(-) create mode 100644 src/__tests__/greeting-hour-single-source.test.ts diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 168c050e7..c08418792 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -48650,10 +48650,6 @@ components: - type: "null" onboardingTourCompleted: type: boolean - greetingHour: - type: integer - minimum: -9007199254740991 - maximum: 9007199254740991 required: - username - timezone @@ -48662,7 +48658,6 @@ components: - gender - glucoseUnit - onboardingTourCompleted - - greetingHour additionalProperties: false layout: type: object diff --git a/e2e/utils/mock-dashboard-snapshot.ts b/e2e/utils/mock-dashboard-snapshot.ts index 77e4a11dd..c8df7de9c 100644 --- a/e2e/utils/mock-dashboard-snapshot.ts +++ b/e2e/utils/mock-dashboard-snapshot.ts @@ -258,7 +258,6 @@ export function buildMockSnapshot( gender: "MALE", glucoseUnit: "mg/dL", onboardingTourCompleted: true, - greetingHour: 9, }, layout: DEFAULT_DASHBOARD_LAYOUT, layoutCatalogue: buildLayoutCatalogue(), diff --git a/src/__tests__/greeting-hour-single-source.test.ts b/src/__tests__/greeting-hour-single-source.test.ts new file mode 100644 index 000000000..98766ce71 --- /dev/null +++ b/src/__tests__/greeting-hour-single-source.test.ts @@ -0,0 +1,131 @@ +/** + * The time-of-day greeting has ONE evaluator. + * + * The dashboard header and the insights hero both open with a salutation + * bucketed by the wall-clock hour. For a long time three different pieces + * of code answered "what hour is it": + * + * - the dashboard snapshot builder, which computed a `greetingHour` in + * the user's zone and shipped it on the payload "so the client never + * has to run its own Intl" — and which no client ever read; + * - the insights hero, through the shared `hourInTz` (profile zone, + * Europe/Berlin when the zone will not resolve); + * - the dashboard header, through a local helper that fell back to the + * DEVICE clock whenever the zone would not resolve. + * + * At one instant, for one account, those three returned 20, 21 and 05. + * + * The server value lost. It is not a derived fact about the record, it is + * a clock read, and the snapshot body is served stale-while-revalidate for + * up to an hour — long enough for a baked-in hour to name a salutation the + * clock had left behind. What the server IS authoritative for is the zone, + * and that already reaches every client on the same payload. + * + * This guard fails if either end drifts back: + * T1 — the snapshot builder or its OpenAPI schema starts carrying a + * greeting hour again. + * T2 — a greeting surface reads a clock that is not `hourInTz` — the + * device zone via `Date#getHours`, or a hand-rolled `Intl` walk. + * T3 — behaviourally, the shared helper stays on the profile zone even + * when the device sits elsewhere and the zone will not resolve. + * + * T1/T2 are grep-shaped and assert a non-zero match count, so an empty + * sweep fails loudly instead of passing silently. + */ +import { describe, it, expect, afterAll, vi } from "vitest"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { hourInTz, DEFAULT_TIMEZONE } from "@/lib/tz/format"; + +const SRC = join(process.cwd(), "src"); +const read = (rel: string) => readFileSync(join(SRC, rel), "utf8"); + +const SERVER_FILES = [ + "lib/dashboard/snapshot.ts", + "lib/openapi/routes/insights/schemas.ts", +] as const; + +/** Every surface that paints a time-of-day salutation. */ +const GREETING_SURFACES = [ + "components/dashboard/dashboard-header.tsx", + "components/insights/hero-strip.tsx", +] as const; + +describe("T1 — the snapshot payload carries no wall-clock hour", () => { + it.each(SERVER_FILES)("%s declares no greeting hour", (rel) => { + const src = read(rel); + expect(src.length).toBeGreaterThan(0); + expect(src).not.toMatch(/greetingHour/); + expect(src).not.toMatch(/hourInTimezone/); + }); + + it("still resolves the zone the clients read", () => { + const src = read("lib/dashboard/snapshot.ts"); + expect(src).toMatch(/timezone:\s*userTz/); + }); +}); + +describe("T2 — every greeting surface reads the shared helper", () => { + it.each(GREETING_SURFACES)("%s derives its hour from hourInTz", (rel) => { + const src = read(rel); + expect(src.length).toBeGreaterThan(0); + // Imported from the shared module, not re-implemented next door. + expect(src).toMatch( + /import[\s\S]*?\bhourInTz\b[\s\S]*?from "@\/lib\/tz\/format"/, + ); + const calls = src.match(/\bhourInTz\s*\(/g) ?? []; + expect(calls.length).toBeGreaterThan(0); + }); + + it.each(GREETING_SURFACES)("%s reads no device-local clock", (rel) => { + const src = read(rel); + // `getHours()` is the device zone by definition; a hand-rolled + // `Intl.DateTimeFormat` for the hour is the second evaluator this + // guard exists to prevent. + expect(src).not.toMatch(/\.getHours\s*\(/); + expect(src).not.toMatch(/Intl\.DateTimeFormat/); + expect(src).not.toMatch(/getHourForTimeZone/); + }); + + it("the retired header helper is gone from the tree", () => { + const src = read("components/dashboard/range-display.tsx"); + expect(src.length).toBeGreaterThan(0); + expect(src).not.toMatch(/getHourForTimeZone/); + }); +}); + +describe("T3 — the profile zone beats the device zone", () => { + const INSTANT = new Date("2026-03-01T20:30:00Z"); + const originalTz = process.env.TZ; + + afterAll(() => { + vi.useRealTimers(); + process.env.TZ = originalTz; + }); + + it("follows the configured zone while the device sits ten hours away", () => { + process.env.TZ = "Asia/Tokyo"; + vi.useFakeTimers(); + vi.setSystemTime(INSTANT); + + // The device says 05:30 (morning). The account is configured for + // Berlin, where it is 21:30 (evening). + expect(new Date().getHours()).toBe(5); + expect(hourInTz(new Date(), "Europe/Berlin")).toBe(21); + }); + + it("falls back to the shared default, never to the device clock", () => { + process.env.TZ = "Asia/Tokyo"; + vi.useFakeTimers(); + vi.setSystemTime(INSTANT); + + // A zone this runtime cannot resolve — a recently added IANA entry on + // an older engine is the realistic case. The header helper used to + // answer with the device hour here, splitting the two greetings by + // sixteen hours for the same account at the same instant. + const unresolvable = hourInTz(new Date(), "Europe/Atlantis"); + expect(unresolvable).toBe(hourInTz(new Date(), DEFAULT_TIMEZONE)); + expect(unresolvable).not.toBe(new Date().getHours()); + }); +}); diff --git a/src/app/api/dashboard/snapshot/__tests__/route.test.ts b/src/app/api/dashboard/snapshot/__tests__/route.test.ts index 634de7dc8..7d68a97f1 100644 --- a/src/app/api/dashboard/snapshot/__tests__/route.test.ts +++ b/src/app/api/dashboard/snapshot/__tests__/route.test.ts @@ -76,7 +76,6 @@ const SNAPSHOT_BODY = { gender: "MALE", glucoseUnit: "mg/dL", onboardingTourCompleted: true, - greetingHour: 9, }, layout: { version: 1, widgets: [] }, tiles: { @@ -121,7 +120,12 @@ describe("GET /api/dashboard/snapshot", () => { expect(json.data.briefingState).toBe("preparing"); expect(json.data.tiles).toBeDefined(); expect(json.data.extras).toBeNull(); - expect(json.data.user.greetingHour).toBe(9); + // Inverted 2026-09: the payload no longer carries a wall-clock hour. + // The body is served stale for up to an hour, so a baked-in hour could + // name a salutation the clock had long left behind. Clients read the + // resolved `timezone` and their own live clock instead. + expect(json.data.user).not.toHaveProperty("greetingHour"); + expect(json.data.user.timezone).toBe("Europe/Berlin"); // bfcache-friendly directive present. expect(res.headers.get("Cache-Control")).toContain("private"); }); diff --git a/src/components/dashboard/dashboard-header.tsx b/src/components/dashboard/dashboard-header.tsx index e502f9456..4440bab28 100644 --- a/src/components/dashboard/dashboard-header.tsx +++ b/src/components/dashboard/dashboard-header.tsx @@ -25,7 +25,7 @@ import { useTranslations } from "@/lib/i18n/context"; import { useAuth } from "@/hooks/use-auth"; import { useMounted } from "@/hooks/use-mounted"; import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; -import { getHourForTimeZone } from "@/components/dashboard/range-display"; +import { hourInTz } from "@/lib/tz/format"; import type { QuickEntryDialog } from "@/components/dashboard/quick-entry-sheets"; export function DashboardHeader({ @@ -47,9 +47,16 @@ export function DashboardHeader({ // hydrates — the text must match the name-less SSR output during // hydration (React #418) and may only personalise from the first // client re-render. + // + // The hour comes from the shared `hourInTz`, the same helper the + // insights hero greeting uses. Its predecessor fell back to the + // DEVICE clock whenever the runtime could not resolve the profile + // zone, so a zone this browser's ICU does not carry (a recently added + // one, an older engine) split the two greetings by hours while every + // other date on the page stayed on the profile zone. const userTimezone = mounted ? user?.timezone : undefined; const hour = useMemo( - () => (userTimezone ? getHourForTimeZone(userTimezone) : null), + () => (userTimezone ? hourInTz(new Date(), userTimezone) : null), [userTimezone], ); const timeGreeting = diff --git a/src/components/dashboard/range-display.tsx b/src/components/dashboard/range-display.tsx index 66256bac1..58268184f 100644 --- a/src/components/dashboard/range-display.tsx +++ b/src/components/dashboard/range-display.tsx @@ -46,24 +46,6 @@ export function toHoursSummary(s: DataSummary): DataSummary { }; } -export function getHourForTimeZone(timeZone?: string): number { - const now = new Date(); - if (!timeZone) return now.getHours(); - - try { - const parts = new Intl.DateTimeFormat("en-US", { - hour: "2-digit", - hour12: false, - timeZone, - }).formatToParts(now); - const hourPart = parts.find((part) => part.type === "hour")?.value; - const parsed = hourPart ? Number(hourPart) : Number.NaN; - return Number.isNaN(parsed) ? now.getHours() : parsed; - } catch { - return now.getHours(); - } -} - export function getRangeColorClass( value: number | null | undefined, config: RangeDisplayConfig, diff --git a/src/lib/dashboard/__tests__/snapshot.test.ts b/src/lib/dashboard/__tests__/snapshot.test.ts index 257da8fdd..f98613d9b 100644 --- a/src/lib/dashboard/__tests__/snapshot.test.ts +++ b/src/lib/dashboard/__tests__/snapshot.test.ts @@ -258,9 +258,11 @@ describe("buildDashboardSnapshot — envelope shape", () => { expect(snap.user.timezone).toBe("Europe/Berlin"); expect(snap.user.heightCm).toBe(180); expect(snap.user.gender).toBe("MALE"); - expect(typeof snap.user.greetingHour).toBe("number"); - expect(snap.user.greetingHour).toBeGreaterThanOrEqual(0); - expect(snap.user.greetingHour).toBeLessThan(24); + // Inverted 2026-09: the builder used to compute a wall-clock hour for + // the greeting that no client ever read, and that a stale-while- + // revalidate serve could carry an hour past its bucket. The zone is + // the resolved value; the hour belongs to whoever holds a live clock. + expect(snap.user).not.toHaveProperty("greetingHour"); // tiles (fast phase) always present expect(snap.tiles.summaries.WEIGHT.latest).toBe(80); diff --git a/src/lib/dashboard/snapshot.ts b/src/lib/dashboard/snapshot.ts index dc22484f6..ea56aa949 100644 --- a/src/lib/dashboard/snapshot.ts +++ b/src/lib/dashboard/snapshot.ts @@ -212,6 +212,14 @@ export type BriefingState = "ready" | "preparing" | "disabled" | "no-provider"; export interface DashboardSnapshotUser { username: string; + /** + * The user's configured zone — the ONE server-resolved input the + * greeting needs. The wall-clock hour itself is deliberately not on + * this payload: the body is served stale-while-revalidate for up to an + * hour, so a baked-in hour can name the wrong salutation long after + * the boundary passed, and every client already reads a live clock. + * Resolve the zone here, read the hour there. + */ timezone: string; heightCm: number | null; dateOfBirth: string | null; @@ -223,12 +231,6 @@ export interface DashboardSnapshotUser { gender: ProfileSex; glucoseUnit: string | null; onboardingTourCompleted: boolean; - /** - * Server-computed wall-clock hour in the user's timezone so the - * client greeting never has to run its own `Intl.DateTimeFormat` - * before first paint. - */ - greetingHour: number; } /** @@ -530,22 +532,6 @@ export interface SnapshotUserInput { healthScoreConfigJson: unknown; } -/** Wall-clock hour in `tz`, used for the server-side greeting. */ -function hourInTimezone(now: Date, tz: string): number { - try { - const fmt = new Intl.DateTimeFormat("en-US", { - hour: "numeric", - hour12: false, - timeZone: tz, - }); - const parsed = parseInt(fmt.format(now), 10); - // Intl emits "24" for midnight under hour12:false on some ICU builds. - return Number.isFinite(parsed) ? parsed % 24 : now.getUTCHours(); - } catch { - return now.getUTCHours(); - } -} - function enrichLastSeen( raw: Record, nowMs: number, @@ -1457,7 +1443,6 @@ export async function buildDashboardSnapshot( gender, glucoseUnit: user.glucoseUnit ?? null, onboardingTourCompleted: user.onboardingTourCompleted, - greetingHour: hourInTimezone(now, userTz), }, layout, layoutCatalogue: buildLayoutCatalogue(layout), diff --git a/src/lib/openapi/routes/insights/schemas.ts b/src/lib/openapi/routes/insights/schemas.ts index 98c96f8aa..6e76f57ee 100644 --- a/src/lib/openapi/routes/insights/schemas.ts +++ b/src/lib/openapi/routes/insights/schemas.ts @@ -1030,7 +1030,6 @@ export const dashboardSnapshotResponse = z gender: z.enum(["MALE", "FEMALE", "OTHER"]).nullable(), glucoseUnit: z.string().nullable(), onboardingTourCompleted: z.boolean(), - greetingHour: z.number().int(), }), layout: z.record(z.string(), z.unknown()), // v1.7.0 — full 27-id widget catalogue (16 server-known + 11 From 86f826351f247a616d679230618a28bf921aa74c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 3 Sep 2026 05:20:00 +0200 Subject: [PATCH 2/4] fix(dashboard): resolve the aggregate greeting through the shared locale order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The salutation and the streak label are the two strings this route still translates server-side, and the native client is its only caller: a Bearer request carries no language cookie, so `User.locale` is the sole per-user signal. The coercion here stopped at English when that column was NULL, so an account that never opened the language picker was greeted in English on an instance configured for German, while every string beside it was German. `resolveJobLocale` is where the fallback order lives — stored preference, then the operator default, then English — and every background writer was moved onto it earlier. This was the last copy of the old coercion outside that tree. The structural guard covers `lib/jobs` and `lib/daily` only, so it never saw this one; the two tests added here pin the behaviour directly instead. --- .../dashboard/summary/__tests__/route.test.ts | 46 +++++++++++++++++++ src/app/api/dashboard/summary/route.ts | 31 +++++++------ 2 files changed, 62 insertions(+), 15 deletions(-) diff --git a/src/app/api/dashboard/summary/__tests__/route.test.ts b/src/app/api/dashboard/summary/__tests__/route.test.ts index 1005fd8ea..57fb570ad 100644 --- a/src/app/api/dashboard/summary/__tests__/route.test.ts +++ b/src/app/api/dashboard/summary/__tests__/route.test.ts @@ -45,6 +45,12 @@ vi.mock("@/lib/db", () => ({ user: { findUnique: vi.fn(), }, + // The greeting salutation is one of the two strings this route still + // translates server-side; `resolveJobLocale` reads the operator's + // configured default from here when the user's column is NULL. + appSettings: { + findUnique: vi.fn(async () => null), + }, // v1.4.38 W-F — the route now uses `$queryRaw` for the per-type // latest reading, the 7-day rollup-bucket sparkline, and the // 365-day distinct-day streak set. Three raw calls per request, @@ -1089,6 +1095,46 @@ describe("GET /api/dashboard/summary", () => { * source — `buildMoodDailySeries` for mood, `computeBmi` for BMI — so the * client consumes a finished figure instead of re-deriving one. */ +describe("GET /api/dashboard/summary — greeting locale", () => { + // The salutation and the streak label are translated on the server, and + // the native client is this route's only caller: a Bearer request carries + // no locale cookie, so `User.locale` is the sole per-user signal. When it + // is NULL the operator's configured default is the answer — the same + // fallback order every background writer uses. The coercion this replaces + // stopped at English, so a German instance greeted a user who never opened + // the language picker in English while the rest of their screen was German. + it("falls back to the operator default, not to English", async () => { + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue({ + defaultLocale: "de", + } as never); + + const res = await callGet(makeReq()); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { greeting: { salutation: string } }; + }; + expect(body.data.greeting.salutation).toBe("Hallo, testuser"); + }); + + it("still prefers the user's own stored locale", async () => { + vi.mocked(getSession).mockResolvedValue({ + ...SESSION_OK, + user: { ...SESSION_OK.user, locale: "en" }, + } as never); + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue({ + defaultLocale: "de", + } as never); + + const res = await callGet(makeReq()); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { greeting: { salutation: string } }; + }; + expect(body.data.greeting.salutation).toBe("Hi, testuser"); + }); +}); + describe("GET /api/dashboard/summary — mood + BMI cards", () => { function cardsOf(body: unknown): Array> { return (body as { data: { metrics: Array> } }).data diff --git a/src/app/api/dashboard/summary/route.ts b/src/app/api/dashboard/summary/route.ts index b596e17fb..a80dbc6de 100644 --- a/src/app/api/dashboard/summary/route.ts +++ b/src/app/api/dashboard/summary/route.ts @@ -61,7 +61,8 @@ import type { MeasurementType } from "@/generated/prisma/client"; import { measurementTypeEnum } from "@/lib/validations/measurement"; import { CUMULATIVE_HK_TYPES } from "@/lib/measurements/apple-health-mapping"; import { getServerTranslator } from "@/lib/i18n/server-translator"; -import { defaultLocale, locales, type Locale } from "@/lib/i18n/config"; +import { type Locale } from "@/lib/i18n/config"; +import { resolveJobLocale } from "@/lib/i18n/job-locale"; import { userDayKey, DEFAULT_TIMEZONE } from "@/lib/tz/resolver"; import { cachedSwr, caches, type ServerCache } from "@/lib/cache/server-cache"; import { buildMedsTodayBlock } from "@/lib/dashboard/meds-today"; @@ -343,7 +344,8 @@ export const GET = apiHandler(async () => { Awaited> >, `${user.id}|dashboard-summary`, - () => buildDashboardSummary(user.id, userTz, buildContext(user)), + async () => + buildDashboardSummary(user.id, userTz, await buildContext(user)), annotate, ); @@ -383,23 +385,22 @@ interface SummaryBuilderContext { locale: Locale; } -function buildContext(user: { +async function buildContext(user: { displayName: string | null; username: string; locale: string | null; -}): SummaryBuilderContext { +}): Promise { const greetingName = user.displayName ?? user.username; - // v1.5.x — accept every shipped locale (de / en / es / fr / it / pl) - // so the greeting + streak label resolve against the user's - // configured language. The legacy narrowing dropped fr/es/it/pl back - // to the default; that bug is masked for the title/unit fields by - // the new key-based wire shape but still mattered for the strings - // that stay translated server-side. - const locale: Locale = (locales as readonly string[]).includes( - user.locale ?? "", - ) - ? (user.locale as Locale) - : defaultLocale; + // The greeting salutation and the streak label are the two strings this + // route still translates server-side, and the native client is its only + // caller — a Bearer request carries no locale cookie and no meaningful + // Accept-Language, so the stored column is the only per-user signal. + // That is exactly the shape `resolveJobLocale` exists for: stored user + // locale, then the operator's configured default, then English. The + // hand-rolled coercion this replaces stopped at English, so a German + // instance whose user never touched the language picker greeted them in + // English while every other string on the same screen was German. + const locale: Locale = await resolveJobLocale(user.locale); return { greetingName, locale }; } From 1850e2f56435956d3c8ef6dd7d480bcf8502083e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 3 Sep 2026 05:20:11 +0200 Subject: [PATCH 3/4] fix(i18n): reconcile the stored locale from the request, not from a lost PUT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `User.locale` is the only signal a cookie-blind path has. The nightly briefing warm, the Coach nudge, the reminder senders and the native aggregate all resolve from that column; the screen in front of the user resolves from the `healthlog-locale` cookie first, then the column, then Accept-Language. So a browser can read the whole app in German while the column says English, and while it does, nothing on screen looks wrong: the divergence surfaces as one generated paragraph in the wrong language inside an otherwise correct page. Keeping the two in step was a fire-and-forget PUT from a mount effect. A 401 on the paint before sign-in, an offline blip, a tab closed mid-flight — each loses the write, and the next mount repeats the same unchecked attempt rather than noticing the column is still wrong, so a stale column can stand for the life of the account. `getSession` now corrects the column when the request carries a valid locale cookie the column disagrees with. It is ordered by the request pipeline, needs nothing from the client, and is self-limiting: once the two agree the condition is false. Only a cookie counts, and a cookie is only ever written alongside a committed UI locale, so Accept-Language never stamps a guess onto a column that generated prose depends on. The mount PUT stays for the server-set cookie it gets in reply, which Safari's 7-day cap on script-written cookies makes worth having; it is no longer the mechanism. The obvious suspect was a first-paint race between two unordered writes. It is not that: the provider takes the locale the server already resolved and nothing on the mount path changes it, so the effect fires once with the right value. `locale-first-paint-single-write` pins both properties so a future mount-time flip has to face them. --- .../session-locale-reconcile.test.ts | 171 ++++++++++++++++++ src/lib/auth/session.ts | 66 +++++-- .../locale-column-divergence.test.ts | 100 ++++++++++ .../locale-first-paint-single-write.test.tsx | 70 +++++++ src/lib/i18n/context.tsx | 20 +- 5 files changed, 409 insertions(+), 18 deletions(-) create mode 100644 src/lib/auth/__tests__/session-locale-reconcile.test.ts create mode 100644 src/lib/i18n/__tests__/locale-column-divergence.test.ts create mode 100644 src/lib/i18n/__tests__/locale-first-paint-single-write.test.tsx diff --git a/src/lib/auth/__tests__/session-locale-reconcile.test.ts b/src/lib/auth/__tests__/session-locale-reconcile.test.ts new file mode 100644 index 000000000..4766eaf1f --- /dev/null +++ b/src/lib/auth/__tests__/session-locale-reconcile.test.ts @@ -0,0 +1,171 @@ +/** + * `getSession` keeps `User.locale` in step with the language the browser is + * actually reading. + * + * The column is the only signal a cookie-blind path has — the nightly + * briefing warm, the Coach nudge, the reminder senders, the native dashboard + * aggregate. The screen in front of the user resolves from the + * `healthlog-locale` cookie first. When those two disagree nothing looks + * wrong until a job writes a sentence, and then one paragraph of an + * otherwise correct page comes back in the wrong language. + * + * Closing that used to be a fire-and-forget PUT from a mount effect, so a + * lost request left the divergence standing for the life of the account. + * These pin the server-side reconcile that replaced it as the mechanism. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const cookieState = vi.hoisted(() => ({ + sessionId: undefined as string | undefined, + locale: undefined as string | undefined, + set: vi.fn(), + delete: vi.fn(), +})); + +const store = vi.hoisted(() => ({ + row: null as Record | null, +})); + +vi.mock("@/lib/db", () => ({ + prisma: { + session: { + findUnique: vi.fn(async () => store.row), + update: vi.fn(async () => ({})), + deleteMany: vi.fn(async () => ({})), + }, + user: { update: vi.fn(async () => ({})) }, + }, +})); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/logging/context", () => ({ getEvent: () => undefined })); + +vi.mock("@/lib/auth/hmac", () => ({ + hashToken: (raw: string) => `hash:${raw}`, +})); + +vi.mock("next/headers", () => ({ + cookies: vi.fn(async () => ({ + get: (name: string) => { + if (name === "healthlog_session" && cookieState.sessionId) { + return { value: cookieState.sessionId }; + } + if (name === "healthlog-locale" && cookieState.locale !== undefined) { + return { value: cookieState.locale }; + } + return undefined; + }, + set: cookieState.set, + delete: cookieState.delete, + })), +})); + +import { prisma } from "@/lib/db"; +import { getSession } from "../session"; + +const THIRTY_DAYS_MS = 30 * 24 * 60 * 60 * 1000; + +function seedSession(userLocale: string | null) { + cookieState.sessionId = "hls_raw-secret"; + store.row = { + id: "sess-1", + userId: "user-1", + tokenHash: "hash:hls_raw-secret", + // Far enough out that the sliding-expiry refresh stays quiet. + expiresAt: new Date(Date.now() + THIRTY_DAYS_MS), + lastActiveAt: new Date(), + actingAsUserId: null, + recordEpoch: 0, + user: { id: "user-1", locale: userLocale }, + }; +} + +beforeEach(() => { + vi.clearAllMocks(); + cookieState.locale = undefined; + store.row = null; +}); + +describe("getSession — locale column reconcile", () => { + it("writes the column when the browser reads a different language", async () => { + // The row the production instance actually held: a German reader whose + // column said English, so every generated sentence came back English. + seedSession("en"); + cookieState.locale = "de"; + + const result = await getSession(); + + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: "user-1" }, + data: { locale: "de" }, + }); + // The same request must not answer with the value just replaced. + expect(result?.user.locale).toBe("de"); + }); + + it("backfills a column that was never written at all", async () => { + // The account that never opened the language picker: the cookie is the + // client's own mount-time write of the Accept-Language paint. + seedSession(null); + cookieState.locale = "de"; + + await getSession(); + + expect(prisma.user.update).toHaveBeenCalledWith({ + where: { id: "user-1" }, + data: { locale: "de" }, + }); + }); + + it("writes nothing once the two agree", async () => { + seedSession("de"); + cookieState.locale = "de"; + + await getSession(); + + expect(prisma.user.update).not.toHaveBeenCalled(); + }); + + it("ignores a cookie value that is not a shipped locale", async () => { + // The cookie is not HttpOnly, so anything can be in it. A value the + // resolver would later have to defend against never reaches the column. + seedSession("de"); + cookieState.locale = "tlh"; + + const result = await getSession(); + + expect(prisma.user.update).not.toHaveBeenCalled(); + expect(result?.user.locale).toBe("de"); + }); + + it("re-emits the cookie instead of writing when the cookie is gone", async () => { + // Safari ITP expired the script-written copy; the column is the + // survivor, so it is the one that seeds the cookie, not the reverse. + seedSession("de"); + cookieState.locale = undefined; + + await getSession(); + + expect(prisma.user.update).not.toHaveBeenCalled(); + expect(cookieState.set).toHaveBeenCalledWith( + "healthlog-locale", + "de", + expect.objectContaining({ httpOnly: false }), + ); + }); + + it("does not block the request on the write", async () => { + // Fire-and-forget like the `lastActiveAt` stamp: a failing update must + // not turn locale drift into a failed session resolution. + seedSession("en"); + cookieState.locale = "de"; + vi.mocked(prisma.user.update).mockRejectedValueOnce( + new Error("db down") as never, + ); + + await expect(getSession()).resolves.not.toBeNull(); + }); +}); diff --git a/src/lib/auth/session.ts b/src/lib/auth/session.ts index d55186487..c8899fadb 100644 --- a/src/lib/auth/session.ts +++ b/src/lib/auth/session.ts @@ -325,26 +325,70 @@ export async function getSession(): Promise<{ .catch(() => {}); } - // Locale-cookie refresh: Safari's ITP expires the script-written - // `healthlog-locale` cookie after 7 days, so a returning user's first - // paint fell back to the browser language once a week. When the user - // has a persisted locale but the cookie is gone, re-emit it here — - // Set-Cookie is exempt from the ITP cap. Absent-only on purpose: a - // locale switch writes the cookie client-side before the PUT persists - // the column, and overwriting a *differing* cookie here would revert - // that fresh choice mid-flight. Fail-soft: in a server-component - // render the cookie store is read-only and `set` throws — the next - // route-handler request re-attempts. + const rawLocaleCookie = cookieStore.get(LOCALE_COOKIE)?.value; + const cookieLocale = + rawLocaleCookie && (locales as readonly string[]).includes(rawLocaleCookie) + ? (rawLocaleCookie as Locale) + : null; + if ( session.user.locale && (locales as readonly string[]).includes(session.user.locale) && - !cookieStore.get(LOCALE_COOKIE) + !cookieLocale ) { + // Locale-cookie refresh: Safari's ITP expires the script-written + // `healthlog-locale` cookie after 7 days, so a returning user's first + // paint fell back to the browser language once a week. When the user + // has a persisted locale but the cookie is gone, re-emit it here — + // Set-Cookie is exempt from the ITP cap. Fail-soft: in a + // server-component render the cookie store is read-only and `set` + // throws — the next route-handler request re-attempts. try { setLocaleCookie(cookieStore, session.user.locale as Locale); } catch { // Read-only cookie context — skip; nothing depends on this write. } + } else if (cookieLocale && cookieLocale !== session.user.locale) { + // The column follows the language the browser is actually reading. + // + // `User.locale` is the ONLY signal every cookie-blind path has: the + // nightly briefing warm, the Coach nudge, the reminder senders and the + // native aggregate all resolve from the column alone, while the screen + // in front of the user resolves from this cookie first. The two can + // therefore disagree indefinitely, and the disagreement is invisible — + // it surfaces as one generated paragraph in the wrong language inside + // an otherwise correct page. + // + // Keeping them in step used to be the client's job: a fire-and-forget + // `PUT /api/auth/me/locale` from a mount effect, best-effort by + // construction. A 401 on the paint before sign-in, an offline blip, a + // tab closed mid-flight — every one of those loses the write silently, + // and the next mount repeats the same unreliable attempt rather than + // noticing the column is still wrong. So the column stayed on a + // language the user had left behind, for as long as the account existed. + // + // This arm closes it from the side that cannot miss. The cookie is only + // ever written alongside a committed UI locale (client `persistLocale`, + // or this function's own refresh above), so it is a statement about what + // the person is reading, not a guess: Accept-Language never reaches the + // column this way. Fire-and-forget like the `lastActiveAt` stamp, and + // self-limiting — once the two agree the condition is false and no + // further write is attempted. + // + // A second browser whose cookie still carries the older choice will move + // the column back. That is the same last-writer-wins the client PUT + // already had, and it is coherent: that browser is showing the older + // language too, because the cookie outranks the column on its first + // paint as well. + void prisma.user + .update({ + where: { id: session.user.id }, + data: { locale: cookieLocale }, + }) + .catch(() => {}); + // Keep THIS request coherent — a handler resolving the locale from the + // returned row must not answer with the value we just replaced. + session.user.locale = cookieLocale; } return { diff --git a/src/lib/i18n/__tests__/locale-column-divergence.test.ts b/src/lib/i18n/__tests__/locale-column-divergence.test.ts new file mode 100644 index 000000000..5d7aeffb4 --- /dev/null +++ b/src/lib/i18n/__tests__/locale-column-divergence.test.ts @@ -0,0 +1,100 @@ +/** + * The UI language and the stored language are resolved from different + * evidence, and only one of them is ever shown to the person reading. + * + * `resolveServerLocale` / `resolveInitialLocale` answer a REQUEST: the + * `healthlog-locale` cookie first, then `User.locale`, then Accept-Language. + * Two of those three rungs are invisible to anything without a request, so + * a browser can render the whole app in German while the column says + * English. + * + * Everything written outside a request reads the column alone — + * `resolveJobLocale` for the nightly writers, `resolveLocaleForUser` for the + * Coach tool executor and the MCP reads. So a diverged column does not show + * up as a wrong menu or a wrong button. It shows up as one paragraph of + * generated prose in the wrong language, sitting in a page that is otherwise + * correct, which is exactly how it was reported. + * + * These are the two ways a browser gets a language the column never hears + * about. They are the reason `getSession` reconciles the column from the + * cookie: the client's fire-and-forget PUT is the only thing that ever + * closed this gap, and a request that never arrives leaves the divergence + * standing for as long as the account exists. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const mockCookies = vi.fn(); +const mockHeaders = vi.fn(); +vi.mock("next/headers", () => ({ + cookies: (...args: unknown[]) => mockCookies(...args), + headers: (...args: unknown[]) => mockHeaders(...args), +})); + +const mockGetSessionUserLocale = vi.fn(); +vi.mock("@/lib/auth/session", () => ({ + getSessionUserLocale: (...args: unknown[]) => + mockGetSessionUserLocale(...args), +})); + +vi.mock("@/lib/app-settings", () => ({ + getOperatorDefaultLocale: vi.fn(async () => null), +})); + +import { resolveInitialLocale } from "@/lib/i18n/resolve-initial-locale"; +import { resolveJobLocale } from "@/lib/i18n/job-locale"; + +function cookieStoreWith(value: string | undefined) { + return { + get: (name: string) => + name === "healthlog-locale" && value !== undefined + ? { name, value } + : undefined, + }; +} + +function headersWith(acceptLanguage: string | null) { + return { + get: (name: string) => + name.toLowerCase() === "accept-language" ? acceptLanguage : null, + }; +} + +beforeEach(() => { + vi.clearAllMocks(); + mockCookies.mockResolvedValue(cookieStoreWith(undefined)); + mockHeaders.mockResolvedValue(headersWith(null)); + mockGetSessionUserLocale.mockResolvedValue(null); +}); + +describe("a browser can read German while the column says English", () => { + it("Accept-Language alone: German UI, English background prose", async () => { + // The account has never opened the language picker, so the column is + // NULL, and the browser asks for German. + mockGetSessionUserLocale.mockResolvedValue(null); + mockHeaders.mockResolvedValue(headersWith("de-DE,de;q=0.9,en;q=0.5")); + + await expect(resolveInitialLocale()).resolves.toBe("de"); + await expect(resolveJobLocale(null)).resolves.toBe("en"); + }); + + it("cookie alone: German UI, English background prose", async () => { + // The picker was used at some point, so the cookie carries German — + // but the column was left holding the language of the paint that + // preceded the choice. + mockCookies.mockResolvedValue(cookieStoreWith("de")); + mockGetSessionUserLocale.mockResolvedValue("en"); + + await expect(resolveInitialLocale()).resolves.toBe("de"); + await expect(resolveJobLocale("en")).resolves.toBe("en"); + }); + + it("the cookie outranks the column on every request that has one", async () => { + // Which is why the divergence is invisible: nothing the user looks at + // reads the column, so a wrong column produces no wrong screen until a + // job writes a sentence with it. + mockCookies.mockResolvedValue(cookieStoreWith("de")); + mockGetSessionUserLocale.mockResolvedValue("en"); + await expect(resolveInitialLocale()).resolves.toBe("de"); + expect(mockGetSessionUserLocale).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/i18n/__tests__/locale-first-paint-single-write.test.tsx b/src/lib/i18n/__tests__/locale-first-paint-single-write.test.tsx new file mode 100644 index 000000000..7ca105bff --- /dev/null +++ b/src/lib/i18n/__tests__/locale-first-paint-single-write.test.tsx @@ -0,0 +1,70 @@ +/** + * Why the obvious suspect is not the cause. + * + * The tempting explanation for a column stuck on the wrong language is a + * race: first paint renders the default, the mount effect persists THAT, + * the client then flips to the real locale and persists again, and two + * unordered `keepalive` PUTs land in the wrong order. It would explain the + * symptom exactly, and it would mean the write needs ordering. + * + * It is not what happens. The provider takes the locale the SERVER already + * resolved — cookie, then column, then Accept-Language — and nothing on the + * mount path changes it afterwards, so the effect fires once, with the right + * value, and there is no second write to race with. The write is lost a + * simpler way: it is fire-and-forget, so a 401 on the paint before sign-in, + * an offline blip or a tab closed mid-flight drops it, and the next mount + * repeats the same unreliable attempt rather than noticing. + * + * These pin the two properties that rule the race out, so that a future + * change which does introduce a mount-time flip has to face this file. + */ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { renderToStaticMarkup } from "react-dom/server"; + +import { I18nProvider, useTranslations } from "@/lib/i18n/context"; +import deMessages from "../../../../messages/de.json"; + +function Read({ k }: { k: string }) { + const { t } = useTranslations(); + return {t(k)}; +} + +describe("first paint carries the resolved locale, not a default", () => { + it("renders the server-resolved locale on the very first pass", () => { + // If this ever rendered English first, the mount effect would persist + // English before the flip, and the race above would be real. + const html = renderToStaticMarkup( + + + , + ); + expect(html).toContain("Speichern"); + expect(html).not.toContain(">Save<"); + }); + + it("changes the active locale only through the switcher", () => { + const src = readFileSync( + join(process.cwd(), "src/lib/i18n/context.tsx"), + "utf8", + ); + expect(src.length).toBeGreaterThan(0); + + // Every `setActive` call in the file. One belongs to `setLocale` (the + // explicit switch); the others are the bundle backfill, which must + // carry the locale it was given and never a different one. + const setActiveCalls = src.match(/setActive\(/g) ?? []; + expect(setActiveCalls.length).toBeGreaterThan(0); + + // The backfill keeps `prev` whenever the locale moved under it, so it + // can only ever swap MESSAGES, never the locale. + expect(src).toMatch( + /prev\.locale === locale \? \{ locale, messages: loaded \} : prev/, + ); + + // And the persistence effect keys on the locale alone, so one mount + // with one locale means one write. + expect(src).toMatch(/persistLocaleToServer\(locale\);\s*\}, \[locale\]\);/); + }); +}); diff --git a/src/lib/i18n/context.tsx b/src/lib/i18n/context.tsx index 45149980c..446f60042 100644 --- a/src/lib/i18n/context.tsx +++ b/src/lib/i18n/context.tsx @@ -109,13 +109,19 @@ function persistLocale(newLocale: Locale) { document.documentElement.lang = newLocale; } -// v1.25.0 — mirror the active locale onto `User.locale` so server-side -// background work (the proactive Coach nudge, the Telegram test message) -// renders in the user's language. The cookie/localStorage choice is -// invisible to a cron that has no request context; without this write the -// column stays null and those messages fall back to English. Fire-and-forget -// and idempotent on the server: a 401 on a public page, an offline blip or a -// no-op equal value all fail silently and never block the UI flip. +// Mirror the active locale onto `User.locale` so server-side background work +// (the proactive Coach nudge, the Telegram test message) renders in the +// user's language. The cookie/localStorage choice is invisible to a cron that +// has no request context. +// +// This is no longer what the column depends on. Being fire-and-forget, it +// loses the write on a 401, an offline blip or a tab closed mid-flight, and +// nothing retried — the column then held a language the user had left behind +// for good. `getSession` reconciles the column from the locale cookie on the +// next request instead, which is ordered by the request pipeline and needs no +// cooperation from the client. What this call still earns its place for is +// the server-set cookie the route emits in reply: Safari's ITP caps the +// script-written copy at 7 days, and a Set-Cookie is exempt. function persistLocaleToServer(newLocale: Locale) { void apiFetchRaw("/api/auth/me/locale", { method: "PUT", From 40ff39aaa5fef2ed6844547886b6cf2417435959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 3 Sep 2026 05:20:14 +0200 Subject: [PATCH 4/4] docs: record the greeting and locale fixes under Unreleased --- CHANGELOG.md | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab82d4fe4..2b24a6cc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,68 @@ ## [Unreleased] +### Changed + +- `greetingHour` is no longer part of the dashboard snapshot response. Any + client that decoded it as a required field needs the field made optional or + dropped; the `timezone` it sits beside is unchanged and is what the greeting + should be derived from. + +### Fixed + +- **The greeting could contradict the rest of the page.** Three separate + pieces of code answered the question "what hour is it" for the time-of-day + salutation. The dashboard snapshot computed one server-side and put it on + the payload so that clients would not have to; no client ever read it. The + insights hero worked it out from the profile timezone. The dashboard header + worked it out from a helper of its own that quietly fell back to the + device's clock whenever the runtime could not resolve the configured zone, + which is what happens to a zone the browser's timezone data does not yet + carry. At one instant, for one account, those three returned 20, 21 and 05, + so the header said good morning while the hero said good evening and every + date beneath them belonged to a third zone. The two greetings now read the + same helper, which stays on the configured zone and falls back to the same + default as the rest of the app rather than to the device. The server value + is gone: it is a clock reading, not a fact about the record, and the + snapshot body is served from cache for up to an hour, long enough for a + stored hour to name a salutation the clock had already left behind. What + the server resolves and every client reads is the timezone itself, which + travels on the same payload as before. +- **Some accounts had their generated text written in a language they do not + read.** Everything the app writes without a request in front of it — the + nightly briefing, the Coach nudge, the reminder messages — resolves its + language from one column on the user row, because a background pass has no + cookie and no browser to ask. Everything the user looks at resolves from the + request instead, and the request prefers the language cookie, then the + column, then what the browser asked for. So the two can disagree, and while + they do, nothing on screen looks wrong: the menus, the buttons and the + labels are all correct, and the disagreement shows up as a single generated + paragraph in the wrong language sitting inside an otherwise correct page. + That is how it was reported, on a page where the sentence above the health + score came back in English on an account read entirely in German. + Keeping the column in step was the browser's job, through a best-effort + request sent once when a page mounted and never checked. Anything that + interrupts it — the paint before sign-in, an offline moment, a tab closed + while it was still in flight — loses the write, and the next page load makes + the same unchecked attempt rather than noticing the column is still wrong, + so the wrong language can stand for the life of the account. The server now + keeps the two in step itself: when a request arrives carrying a language the + column does not agree with, the column is corrected, once, on the spot. It + needs nothing from the browser, it cannot be dropped in transit, and it + fixes accounts that have been diverged for months on their next page load. + Accounts that never opened the language picker at all are covered the same + way. Nobody's stored preference is guessed at: only a language the app was + actually being read in can reach the column. +- **The dashboard aggregate greeted some users in English on a German + instance.** The salutation and the streak label are translated on the + server for the native client, and that request carries no language cookie, + so the stored preference is the only signal. When an account had never + opened the language picker the column is empty, and the fallback stopped at + English instead of asking what language the instance is configured for. + It now follows the same order every background writer uses: the user's own + preference, then the operator's default, then English. This was the last + copy of the fallback that the nightly writers were moved off earlier. + ### Internal - The column-reader sweep was wrong in both directions and had been for as long as it existed. It answers "does this column have a consumer" for every column in the schema, and it is the tool this project leans on to catch a schema change that ships its writer and forgets its reader. Its idea of a write was any `field:` key anywhere, so `where: { ticketHash }` counted as writing `ticketHash`: all twelve columns it reported were a lookup key or a cron's discovery predicate being filtered on, every one a false alarm. And because it only ever reported a column that HAD a write, a column with no write at all was the one thing it could not see. It now understands which Prisma argument a key sits in, follows a payload assembled into a variable before the call, credits raw SQL against the mapped column name, and reports a column no code touches at all as its own category. Twelve false alarms became nineteen findings that each hold up, four of them worth acting on. The matcher underneath is pinned by a test that was checked by breaking it three ways.