diff --git a/.changeset/16310-orphan-locale-key-gates.md b/.changeset/16310-orphan-locale-key-gates.md new file mode 100644 index 0000000000..106ea55eea --- /dev/null +++ b/.changeset/16310-orphan-locale-key-gates.md @@ -0,0 +1,63 @@ +--- +'@objectstack/lint': minor +--- + +fix(lint)!: an orphaned locale key now FAILS the run — `translation-target-unknown` is an `error` (#16310) + +`validate-translation-references` reported every orphan translation key precisely +— the id named, the locale named, the remedy printed — and failed nothing. +`os lint` exits 0 on warnings, the rule hard-coded `severity: 'warning'`, and no +per-rule severity is configurable by a consuming app. So a PR that deletes a +navigation entry, a form section or a view and leaves its locale keys behind was +green on every pipeline on the platform, and the dead keys are actively +misleading afterwards: grepping the id returns a confident-looking hit in every +locale, which reads as "this exists and is translated". + +The forward half of this parity — `i18n/missing-*`, an authored surface with no +translation — already fails, and apps already gate on it. The orphan half now +fails too, so the two halves of one parity have the same enforceability instead +of opposite ones. + +**BREAKING** — a stack carrying an orphan locale key stops passing `os lint`, +`os validate` and `os build`. Measured on one stack with 8 orphan keys planted, +`objectstack lint --json`: + +| `@objectstack/lint` | findings | errors | warnings | `passed` | exit | +| :-- | --: | --: | --: | :-- | --: | +| before this release | 20 | 0 | 18 | `true` | 0 | +| after this release | 20 | 8 | 10 | `false` | 1 | + +The findings themselves are unchanged — same count, same paths, same message and +hint text. Only the severity moves, and with it the exit code. + +**What an author does about it.** In a clean stack, nothing: a tree with no +orphan key reports exactly what it reported before, at the same severities, with +the same exit code (measured — the report is identical field for field apart +from its wall-clock `duration`). In a stack the rule already names findings on, +delete each locale key it names. The key resolves to nothing — the object, +field, view, section, tab, action, param, app, nav item, dashboard, widget or +flow screen it was written for is not in the stack — so removing it changes no +rendered string in any locale. Where the target was renamed rather than removed, +key the translation to the new name instead; the finding prints the declared +names to choose from. + +**This is ONE rule, not "warnings are errors now".** Measured on a planted tree +carrying findings from 13 distinct rules: exactly 1 changed severity, 12 did not, +and the finding set is identical modulo that one severity. +`translation-option-key-unknown` — raised by the same function — stays `warning` +on purpose: a mis-keyed option translation names something real and its remedy is +a rename, not a deletion. `validateTranslatableSections`, the sibling asking "is +there a key at all?", is untouched. + +**Unchanged: the runtime publish gate.** `validateTranslationReferences` reaches +the runtime door on a `flow` write, but the per-write snapshot carries only +`objects` / `permissions` / `books` / `datasets` — `RuntimeStackContext` has no +`translations` member for a host to fill — so the rule sees no bundle and returns +nothing there. Measured: a flow write through `runRuntimeAuthoringRules` yields +0 errors and 0 advisories from this rule. No publish that used to succeed is +refused. + +`TranslationRefSeverity` widens from `'warning'` to `'warning' | 'error'` +accordingly. + + diff --git a/packages/lint/src/reference-integrity-suite.ts b/packages/lint/src/reference-integrity-suite.ts index 0e259f6a3f..458c433694 100644 --- a/packages/lint/src/reference-integrity-suite.ts +++ b/packages/lint/src/reference-integrity-suite.ts @@ -387,8 +387,11 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [ // section authored with a `label` and no `name` renders a heading that // `_sections` (keyed by name) can never address, so neither the orphan check // nor the coverage walk can see it. A reference that cannot be written is - // still a reference question, and warning-only for the same reason its - // sibling is: one heading stays in the source locale, nothing breaks. + // still a reference question, and warning-only on its own reading — NOT on + // its sibling's any more: `translation-target-unknown` gates, because an + // orphan key is a confident-looking grep hit for a surface that no longer + // exists. Here the surface is real and present; only its heading stays in the + // source locale, so nothing is misdescribed and nothing breaks. { name: 'validateTranslatableSections', run: validateTranslatableSections }, { name: 'validateFlowTemplatePaths', run: validateFlowTemplatePaths }, { name: 'validateAiSurfaceAffinity', run: validateAiSurfaceAffinity }, diff --git a/packages/lint/src/validate-translation-references.test.ts b/packages/lint/src/validate-translation-references.test.ts index b31baa023d..e4972419c2 100644 --- a/packages/lint/src/validate-translation-references.test.ts +++ b/packages/lint/src/validate-translation-references.test.ts @@ -18,6 +18,33 @@ import { ContactViews } from '../../../examples/app-showcase/src/ui/views/contac // a section that has no name. import { SnapshotContact, SnapshotContactViews } from './showcase-shape.fixtures.js'; +/** + * ⭐ Severity, re-judged in place (#16310) — ⛔ not deleted. + * + * Eight assertions in this file pinned `severity: 'warning'` on + * `translation-target-unknown`, and that silence was deliberate: the rule's own + * Severity note argued an orphan key is inert — a few bytes and one untranslated + * string, nothing crashes — so gating on it would be the over-statement + * ADR-0072 D1 forbids. + * + * The reading was measured wrong in the one direction that matters. An orphan + * key is not inert; it is a confident-looking grep hit, in every locale, for a + * surface that was deleted — which reads as "this exists and is translated" to + * the next author, human or AI. Reported-but-unfailable meant a PR that deletes + * a navigation entry, a form section or a view and leaves its locale keys behind + * was green on every pipeline on the platform (measured: eight planted orphans + * moved `os lint --json` from 12/10 to 20/18 findings, `passed: true`, exit 0). + * + * So every one of those eight now pins `error`. They are the SAME assertions + * making the same statement one severity later, and they stay because the + * severity is exactly what is worth pinning here. + * + * ⚠️ The neighbours are untouched on purpose: `translation-option-key-unknown` + * still pins `warning` (see the "option keys" and "severity is narrow" blocks) — + * a mis-keyed option names something real and its remedy is a rename, not a + * deletion. + */ + /** A stack shaped like the HotCRM lead surface: fields, options, a view, an action. */ const leadStack = (translations: unknown[]) => ({ objects: [ @@ -67,7 +94,7 @@ describe('validateTranslationReferences — orphan keys', () => { ]), ); expect(findings).toHaveLength(1); - expect(findings[0].severity).toBe('warning'); + expect(findings[0].severity).toBe('error'); expect(findings[0].rule).toBe(TRANSLATION_TARGET_UNKNOWN); expect(findings[0].path).toBe('translations[0]["zh-CN"].objects.crm_lead.fields.assigned_to'); expect(findings[0].hint).toContain('Declared fields: name, source, status.'); @@ -128,7 +155,7 @@ describe('validateTranslationReferences — orphan keys', () => { 'translations[0].en.objects.crm_lead._sections.deal_info', 'translations[0].en.objects.crm_lead._actions.mass_update', ]); - expect(findings.every((f) => f.severity === 'warning')).toBe(true); + expect(findings.every((f) => f.severity === 'error')).toBe(true); }); it('flags an action parameter the action does not declare', () => { @@ -266,6 +293,71 @@ describe('validateTranslationReferences — nested conditional validation branch }); }); +describe('validateTranslationReferences — the severity split is narrow (#16310)', () => { + /** + * The gating claim, pinned from the consumer's side rather than from the + * rule's: a consumer selects this rule by its EXACT id, because the id is the + * bare string the registry publishes — no namespace, and none added here (the + * id shape was ruled out of scope: a namespace for this one rule would make it + * the sixth prefixed id among 200 exported rule-id constants, or a migration + * across two producers). So the two things a gate needs are the id string and + * the severity, and both are asserted here together. + */ + it('raises `translation-target-unknown` at `error`, selectable by its exact id', () => { + const findings = validateTranslationReferences( + leadStack([ + { 'zh-CN': { objects: { crm_lead: { fields: { assigned_to: { label: '负责人' } } } } } }, + ]), + ); + const selected = findings.filter((f) => f.rule === 'translation-target-unknown'); + expect(selected).toHaveLength(1); + expect(selected[0].severity).toBe('error'); + // The id is the literal a consumer's filter can be written against — the + // constant and the wire string are the same value, asserted both ways so a + // rename cannot pass this test by moving the constant alone. + expect(TRANSLATION_TARGET_UNKNOWN).toBe('translation-target-unknown'); + }); + + /** + * ⛔ The promotion is ONE rule's, not "every warning becomes an error". The + * sibling raised by the very same function keeps `warning`, so a tree whose + * only translation defect is a mis-keyed option is unchanged — same finding, + * same severity, same exit code as before. + */ + it('leaves `translation-option-key-unknown` at `warning`', () => { + const findings = validateTranslationReferences( + leadStack([ + { + 'zh-CN': { + objects: { + crm_lead: { fields: { source: { label: '来源', options: { 'direct-mail': '直邮' } } } }, + }, + }, + }, + ]), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(TRANSLATION_OPTION_KEY_UNKNOWN); + expect(findings[0].severity).toBe('warning'); + }); + + /** The negative control: a clean bundle still reports nothing, of any severity. */ + it('reports nothing on a bundle whose every key resolves', () => { + const findings = validateTranslationReferences( + leadStack([ + { + 'zh-CN': { + objects: { + crm_lead: { label: '线索', fields: { name: { label: '名称' } } }, + }, + }, + }, + ]), + ); + expect(findings).toEqual([]); + }); +}); + describe('validateTranslationReferences — option keys', () => { it('flags an option key that is a near-miss of the stored value', () => { // The HotCRM instance: `direct-mail` for the value `direct_mail`. @@ -398,7 +490,7 @@ describe('validateTranslationReferences — cross-package objects (§4 ladder)', translations: bundleFor('sys_approval_process'), }); expect(findings).toHaveLength(1); - expect(findings[0].severity).toBe('warning'); + expect(findings[0].severity).toBe('error'); expect(findings[0].message).toContain('platform namespace'); // The object key is reported once; its subtree is not half-checked. expect(findings[0].path).toBe('translations[0]["zh-CN"].objects.sys_approval_process'); @@ -585,7 +677,7 @@ describe('validateTranslationReferences — flows (#7646 / #11287)', () => { bundle({ lead_conversions: { label: 'x', screens: { details: { title: 'y' } } } }), ); expect(findings).toHaveLength(1); - expect(findings[0].severity).toBe('warning'); + expect(findings[0].severity).toBe('error'); expect(findings[0].rule).toBe(TRANSLATION_TARGET_UNKNOWN); expect(findings[0].path).toBe('translations[0]["zh-CN"].flows.lead_conversions'); expect(findings[0].message).toContain('Did you mean "lead_conversion"?'); @@ -598,7 +690,7 @@ describe('validateTranslationReferences — flows (#7646 / #11287)', () => { bundle({ lead_conversion: { label: '线索转换', screens: { detail: { title: 'y' } } } }), ); expect(findings).toHaveLength(1); - expect(findings[0].severity).toBe('warning'); + expect(findings[0].severity).toBe('error'); expect(findings[0].path).toBe('translations[0]["zh-CN"].flows.lead_conversion.screens.detail'); expect(findings[0].message).toContain('Did you mean "details"?'); expect(findings[0].hint).toContain('ScreenSpec.nodeId'); @@ -614,7 +706,7 @@ describe('validateTranslationReferences — flows (#7646 / #11287)', () => { }), ); expect(findings).toHaveLength(1); - expect(findings[0].severity).toBe('warning'); + expect(findings[0].severity).toBe('error'); expect(findings[0].path).toBe( 'translations[0]["zh-CN"].flows.lead_conversion.screens.details.fields.opportunity', ); @@ -703,7 +795,7 @@ describe('validateTranslationReferences — flows (#7646 / #11287)', () => { ], }); expect(findings).toHaveLength(1); - expect(findings[0].severity).toBe('warning'); + expect(findings[0].severity).toBe('error'); expect(findings[0].message).toContain('OBJECT-FORM screen'); expect(findings[0].hint).toContain('objects.crm_lead.fields.owner'); }); @@ -1295,7 +1387,7 @@ describe('validateTranslationReferences — filter-preset tabs (#13835)', () => ]), ); expect(findings).toHaveLength(1); - expect(findings[0].severity).toBe('warning'); + expect(findings[0].severity).toBe('error'); expect(findings[0].rule).toBe(TRANSLATION_TARGET_UNKNOWN); expect(findings[0].path).toBe('translations[0]["zh-CN"].objects.crm_lead._tabs.overdue'); expect(findings[0].hint).toContain('Declared tabs: mine, urgent.'); diff --git a/packages/lint/src/validate-translation-references.ts b/packages/lint/src/validate-translation-references.ts index 0bb4a8c08d..872ee9d329 100644 --- a/packages/lint/src/validate-translation-references.ts +++ b/packages/lint/src/validate-translation-references.ts @@ -28,11 +28,29 @@ * * ── Severity ───────────────────────────────────────────────────────────── * - * All findings are **warnings**. An orphan key is inert, not broken: it costs a - * few bytes and one untranslated string, and nothing crashes. That is a weaker - * failure than the dead references `validate-object-references` / - * `validate-action-name-refs` report as errors, and the severity should say so - * (ADR-0072 D1 — a linter that over-states is a linter authors stop reading). + * `translation-target-unknown` is an **error**; `translation-option-key-unknown` + * stays a **warning**. The split is the difference between a key that points at + * nothing and a key that points at the wrong spelling of something. + * + * The orphan direction used to be a warning on the reading that an orphan key is + * inert — a few bytes and one untranslated string, nothing crashes. That reading + * was measured wrong in the direction that matters: the key is not inert, it is + * **actively misleading**. Grepping its id returns a confident-looking hit in + * every locale, which reads as "this surface exists and is translated" — to a + * human reviewer and to an AI author alike — long after the navigation entry, + * form section or view it was written for was deleted. + * + * Reported-but-unfailable is the shape that let that happen: the rule named + * every orphan, printed the remedy, and exited 0, so a PR that deletes a surface + * and leaves its locale keys behind was green on every pipeline on the platform. + * The forward half of the same parity (`i18n/missing-*` — an authored surface + * with no translation) already fails; the two halves of one parity now have the + * same enforceability instead of opposite ones. + * + * ⚠️ ADR-0072 D1 (a linter that over-states is a linter authors stop reading) is + * what keeps this narrow. It is not a licence to promote the neighbours: a + * mis-keyed OPTION translation resolves to a declared option's near-miss and its + * remedy is a rename, not a deletion, so `checkOptionKeys` keeps `warning`. * * ── What this rule deliberately does NOT check ─────────────────────────── * @@ -121,10 +139,24 @@ const SCREEN_NODE_TYPE = 'screen'; export const TRANSLATION_TARGET_UNKNOWN = 'translation-target-unknown'; export const TRANSLATION_OPTION_KEY_UNKNOWN = 'translation-option-key-unknown'; -export type TranslationRefSeverity = 'warning'; +export type TranslationRefSeverity = 'warning' | 'error'; + +/** + * The severity every `translation-target-unknown` finding carries. + * + * Declared once so the two sites that raise the rule cannot drift apart, and so + * the rule's gating claim is readable as a value rather than inferred from two + * scattered string literals. + */ +const TRANSLATION_TARGET_UNKNOWN_SEVERITY = 'error' as const; export interface TranslationRefFinding { - /** Always `warning` — an orphan translation key is inert, not broken. */ + /** + * `error` for `translation-target-unknown` (an orphan key resolves to nothing + * and gates the run), `warning` for `translation-option-key-unknown` (a + * mis-keyed option resolves to nothing but names something real). See the + * module's Severity note. + */ severity: TranslationRefSeverity; /** Diagnostic rule id. */ rule: string; @@ -726,8 +758,8 @@ function buildUniverse(stack: AnyRec): Universe { // real screen the runner pauses on and hands the client a `ScreenSpec.nodeId` // for, so its translation key resolves. Reading the flat array would leave // every nested screen out of the universe and report each of its keys as an - // orphan — a warning-severity false positive, which is exactly the - // over-stating ADR-0072 D1 forbids. + // orphan — a false positive that now FAILS the run, which is exactly the + // over-stating ADR-0072 D1 forbids, at the cost the gating severity sets. const flows = new Map(); for (const flow of recordsOf(stack.flows)) { const flowName = strName(flow.name); @@ -777,7 +809,14 @@ export function validateTranslationReferences(stack: AnyRec): TranslationRefFind const universe = buildUniverse(stack); const orphan = (where: string, path: string, message: string, hint: string) => { - findings.push({ severity: 'warning', rule: TRANSLATION_TARGET_UNKNOWN, where, path, message, hint }); + findings.push({ + severity: TRANSLATION_TARGET_UNKNOWN_SEVERITY, + rule: TRANSLATION_TARGET_UNKNOWN, + where, + path, + message, + hint, + }); }; for (let bi = 0; bi < bundles.length; bi++) { @@ -1223,7 +1262,7 @@ function checkActionParams( for (const paramName of rawParams) { if (declared.has(paramName)) continue; findings.push({ - severity: 'warning', + severity: TRANSLATION_TARGET_UNKNOWN_SEVERITY, rule: TRANSLATION_TARGET_UNKNOWN, where: `${ctx.where} · param "${paramName}"`, path: `${ctx.path}.params.${paramName}`,