diff --git a/docs/11-risks-and-technical-debt/README.md b/docs/11-risks-and-technical-debt/README.md index 1b5653f2..065230f2 100644 --- a/docs/11-risks-and-technical-debt/README.md +++ b/docs/11-risks-and-technical-debt/README.md @@ -145,6 +145,38 @@ write the lesson here so the next contributor doesn't repeat it. Format: short title + **What happened** + **Why it happened** + **How to avoid it next time**. +### A rebase dropped two i18n keys, and `t()` renders the key path instead of failing — the degraded-mode banner read `common.heartbeatDataIncomplete` in production (2026-08 audit, #238) + +**What happened:** `common.unknown` and `common.heartbeatDataIncomplete` +were added in `ff1ada1` (#50) and removed again in `8c1be9c` (#52) in a +hunk unrelated to that PR's subject — an accidental merge/rebase +regression. Four call sites kept referencing them. Because +`LanguageContext.t()` returns the key path when the leaf is not +renderable, the dashboard rendered the literal string +`common.heartbeatDataIncomplete` to users, in both locales, precisely +when the backend was degraded and a readable message mattered most. +Nothing failed: not the type checker, not the 191-test homepage suite, +not the build. + +**Why:** `t()` takes a `string` path, so a key that does not exist is +indistinguishable at compile time from one that does — there is no +type-level link between a call site and the translations object. The +graceful fallback that makes `t()` safe to call is exactly what makes a +missing key invisible: it degrades to something renderable instead of +throwing, so the failure only ever shows up on screen. + +**How to avoid it next time:** a graceful fallback needs a test that +notices when it fires. `homepage/src/__tests__/i18nKeyCompleteness.test.ts` +scans every `t('...')` literal in the homepage source and asserts each +resolves to renderable text in **every** locale, and that the locales +stay structurally in step. Two things it has to get right, both learned +the hard way while writing it: it must accept plural forms +(`{ one, other }`) as renderable, or it reports the intentional +`dashboard.modulesListed` as missing; and it must assert it found call +sites at all, or a scan that silently matches nothing passes every other +assertion vacuously — which is what happened on the first run, when +`__dirname` was undefined under vitest's ESM loader. + ### The delete path trusted a client filename the read path didn't — and fleet filenames were silently clobbering each other (2026-07 audit, #202) **What happened:** `image-service`'s `delete_image` joined the diff --git a/homepage/src/__tests__/i18nKeyCompleteness.test.ts b/homepage/src/__tests__/i18nKeyCompleteness.test.ts new file mode 100644 index 00000000..8e2f4329 --- /dev/null +++ b/homepage/src/__tests__/i18nKeyCompleteness.test.ts @@ -0,0 +1,107 @@ +import { describe, it, expect } from 'vitest'; +import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { join, resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import translations from '../i18n/translations'; + +/** + * Guards the failure in #238: a `t('...')` call site whose key does not exist + * in translations.ts. LanguageContext.t() returns the key path when the leaf + * is not renderable, so the UI shows the literal `common.unknown` instead of + * text, and nothing failed. This scans the source for `t('...')` literals and + * resolves each against every locale. + */ + +const SRC = resolve(dirname(fileURLToPath(import.meta.url)), '..'); + +function sourceFiles(dir: string): string[] { + return readdirSync(dir).flatMap((entry) => { + const full = join(dir, entry); + if (statSync(full).isDirectory()) { + // The tests themselves may reference deliberately-absent keys. + return entry === '__tests__' ? [] : sourceFiles(full); + } + return /\.tsx?$/.test(entry) ? [full] : []; + }); +} + +/** + * Only single-quoted string literals. A template literal or a variable is not + * statically resolvable, and is out of scope for this check. + */ +function keysIn(source: string): string[] { + return [...source.matchAll(/\bt\(\s*'([^']+)'/g)].map((match) => match[1]); +} + +function resolveKey(locale: Record, key: string): unknown { + return key + .split('.') + .reduce( + (node, part) => + node !== null && typeof node === 'object' + ? (node as Record)[part] + : undefined, + locale + ); +} + +/** + * Mirrors what LanguageContext.t() can actually render: a string, or a plural + * form — an object carrying a string `other` branch, which t() selects with + * Intl.PluralRules. Anything else (an array such as setup.stepLabels, or a + * nested group) collapses to the key path on screen, which is the bug being + * guarded. Those are read with useTranslationRaw(), not t(). + */ +function isRenderable(value: unknown): boolean { + if (typeof value === 'string') return true; + return ( + value !== null && + typeof value === 'object' && + typeof (value as Record).other === 'string' + ); +} + +const locales = Object.keys(translations) as (keyof typeof translations)[]; + +const callSites = sourceFiles(SRC).flatMap((file) => + keysIn(readFileSync(file, 'utf8')).map((key) => ({ file, key })) +); + +describe('i18n key completeness', () => { + it('finds t() call sites to check', () => { + // Guards the scan itself: a regex or a directory walk that silently stops + // matching would otherwise make every assertion below vacuously true. + expect(callSites.length).toBeGreaterThan(20); + expect(locales.length).toBeGreaterThan(1); + }); + + it('accepts strings and plural forms, and rejects what t() cannot render', () => { + expect(isRenderable('Unknown')).toBe(true); + expect(isRenderable({ one: '1 module', other: '{count} modules' })).toBe(true); + expect(isRenderable(undefined)).toBe(false); + expect(isRenderable(['a', 'b'])).toBe(false); + expect(isRenderable({ nested: { deeper: 'x' } })).toBe(false); + }); + + it.each(locales)('every t() key resolves to renderable text in %s', (locale) => { + const missing = callSites + .filter(({ key }) => !isRenderable(resolveKey(translations[locale], key))) + .map(({ file, key }) => `${key} (${file.slice(SRC.length + 1)})`); + + expect([...new Set(missing)]).toEqual([]); + }); + + it('keeps every locale structurally in step', () => { + const flatten = (node: unknown, prefix = ''): string[] => + node !== null && typeof node === 'object' + ? Object.entries(node as Record).flatMap(([k, v]) => + flatten(v, prefix ? `${prefix}.${k}` : k) + ) + : [prefix]; + + const [first, ...rest] = locales.map((locale) => flatten(translations[locale]).sort()); + for (const other of rest) { + expect(other).toEqual(first); + } + }); +}); diff --git a/homepage/src/i18n/translations.ts b/homepage/src/i18n/translations.ts index e05d37ab..66268f82 100644 --- a/homepage/src/i18n/translations.ts +++ b/homepage/src/i18n/translations.ts @@ -6,6 +6,9 @@ const translations = { next: 'Next', online: 'Online', offline: 'Offline', + unknown: 'Unknown', + heartbeatDataIncomplete: + 'Heartbeat data unavailable — some module statuses may be incomplete. Refresh in a moment.', loading: 'Loading...', error: 'Error', tryAgain: 'Try Again', @@ -479,6 +482,9 @@ const translations = { next: 'Weiter', online: 'Online', offline: 'Offline', + unknown: 'Unbekannt', + heartbeatDataIncomplete: + 'Heartbeat-Daten nicht verfügbar — einige Modul-Status können unvollständig sein. Bitte gleich neu laden.', loading: 'Laden...', error: 'Fehler', tryAgain: 'Erneut versuchen',