Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions .changeset/16310-orphan-locale-key-gates.md
Original file line number Diff line number Diff line change
@@ -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.

<!-- adr-0087: not-required (no-migration-prescription) nothing an author writes moves: no authorable key is renamed, retired or reshaped, `packages/spec/**` is untouched, no Zod schema and no stored metadata shape changes, and the diff moves one lint severity literal plus the exported TS union that types it. `objectstack migrate meta` has nothing to reach and the ledger serves nobody affected. The author action this release can imply is not an upgrade step either: an orphan key resolved to nothing before this release and resolves to nothing after it, and the rule has been naming each one, with its remedy, in every release that shipped it — what changes is that the report is no longer ignorable, not what the metadata is supposed to say. -->
7 changes: 5 additions & 2 deletions packages/lint/src/reference-integrity-suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
108 changes: 100 additions & 8 deletions packages/lint/src/validate-translation-references.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down Expand Up @@ -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.');
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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"?');
Expand All @@ -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');
Expand All @@ -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',
);
Expand Down Expand Up @@ -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');
});
Expand Down Expand Up @@ -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.');
Expand Down
61 changes: 50 additions & 11 deletions packages/lint/src/validate-translation-references.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ───────────────────────────
*
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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<string, FlowFacts>();
for (const flow of recordsOf(stack.flows)) {
const flowName = strName(flow.name);
Expand Down Expand Up @@ -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++) {
Expand Down Expand Up @@ -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}`,
Expand Down
Loading