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] 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",