diff --git a/.changeset/9318-field-rule-verdict-from-lint.md b/.changeset/9318-field-rule-verdict-from-lint.md new file mode 100644 index 0000000000..d648ccf474 --- /dev/null +++ b/.changeset/9318-field-rule-verdict-from-lint.md @@ -0,0 +1,43 @@ +--- +'@object-ui/app-shell': minor +--- + +The wrong-layer root advisory asks the platform for its verdict instead of keeping +a second copy of it (objectui#9318). + +`rowCanonAdvisory` answered "is this root bound on this surface?" from objectui's +own knowledge — `@object-ui/core`'s detector hard-codes the single root `data`, and +the docblock justified that from `ROW_PREDICATE_ROOTS` / `FIELD_RULE_ROOTS` / +`FORMULA_ROOTS`. `@objectstack/lint` publishes the same judgement as +`fieldRuleRootIssue` / `FIELD_RULE_BOUND_ROOTS`, pinned upstream. Two hand-maintained +copies of one judgement, agreeing today, with nothing keeping them agreeing: the next +root the platform binds or unbinds moves one and not the other, silently. + +A `CelSchemaHint.slot` (and the matching `CelPredicateField` prop) names the authored +key, and on the three slots whose bound set IS the field-rule set — `visibleWhen`, +`readonlyWhen`, `requiredWhen` — the verdict and the message now come from the helper. +That set is objectui's own answer, not the helper's: `fieldRuleRootIssue` has no slot +vocabulary at all — hand it any slot name and it still judges, gating on the ROOT and +interpolating whatever name it was given. Which surfaces it may answer for is therefore +this repo's call. Both are internal to `@object-ui/app-shell`; no package export moves. + +**Behaviour change, deliberate and warning-only.** Those three editors now advise on +every root the field level leaves unbound — except `app` — where before only `data` +was reported, and each carries the engine's own per-root diagnostic rather than +objectui's single sentence. `app` is the one exception, and for a structural reason +rather than as a special case: the advisory runs only on an error-free predicate, and +`app` is the single judged root the platform does not declare, so the pre-existing +bare-reference error fires first and the advisory never runs. Severity stays +objectui's own `warning` — every save gate on this tier counts `severity === 'error'`, +so no accept set moves and no predicate already stored in customer metadata is refused. + +**Coverage is not shrunk to fit the helper.** Two guarded surfaces bind a set that is +not the field-rule set, and neither is comparable to it — they overlap on `record` +alone. A `formula` field's `expression` binds `FORMULA_ROOTS` (`record`), a proper +subset. A conditional-formatting `condition` binds `ROW_PREDICATE_ROOTS` (`record`, +`current_user`, `user`, `features`, `os`, `ctx`) — five roots the field tier does not +bind, but not a superset of it either, since it lacks `previous` and `parent`. Routed +through the helper, the condition would be told to rewrite five roots that work there, +and the formula would be told `previous` and `parent` are bound when it binds neither. +Those keep the local instrument unchanged, and both are pinned as live controls against +a later tidy-up that routes them through the helper anyway. diff --git a/packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx b/packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx index 07c1001e15..0e6247b57f 100644 --- a/packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx +++ b/packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx @@ -87,6 +87,14 @@ export interface CelPredicateFieldProps { scope?: 'record' | 'flattened'; /** Override the scope roots offered by autocomplete (see CelSchemaHint.roots). */ roots?: string[]; + /** + * The authored key this editor writes (`visibleWhen`, `readonlyWhen`, + * `requiredWhen`) — see `CelSchemaHint.slot`. Naming it lets the wrong-layer + * advisory take the platform's published per-slot verdict (objectui#9318); + * leaving it unset keeps the local one, which is the right answer for the + * surfaces whose bound roots are not the field-rule set. + */ + slot?: string; /** * Engine field role (see CelSchemaHint.role). `'predicate'` (default) for * boolean conditions; `'value'` for formula expressions — which also turns @@ -118,6 +126,7 @@ export function CelPredicateField({ clause, scope, roots, + slot, role, onLintChange, onInferredTypeChange, @@ -168,7 +177,7 @@ export function CelPredicateField({ React.useEffect(() => { let cancelled = false; const handle = setTimeout(() => { - const hint = { objectName, fields: fieldNames, clause, scope, role }; + const hint = { objectName, fields: fieldNames, clause, scope, slot, role }; lintCelPredicate(value, hint).then((res) => { if (cancelled) return; setIssues(res); @@ -188,7 +197,7 @@ export function CelPredicateField({ clearTimeout(handle); }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [value, objectName, fieldsKey, clause, scope, role]); + }, [value, objectName, fieldsKey, clause, scope, slot, role]); /* Report "clean" upward when the field empties (no debounce needed). */ React.useEffect(() => { diff --git a/packages/app-shell/src/views/metadata-admin/celAuthoring.fieldRuleVerdict-9318.test.ts b/packages/app-shell/src/views/metadata-admin/celAuthoring.fieldRuleVerdict-9318.test.ts new file mode 100644 index 0000000000..cb1a4ea4c5 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/celAuthoring.fieldRuleVerdict-9318.test.ts @@ -0,0 +1,212 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#9318 — the wrong-layer verdict comes from `@objectstack/lint`, not + * from a second hand-maintained copy of it. + * + * ## What moved + * + * `rowCanonAdvisory` used to answer "is this root bound on this surface?" from + * objectui's own knowledge: `@object-ui/core`'s `detectNonCanonicalRowSpelling` + * hard-codes ONE root (`data`) and objectui's docblock justified that from + * `ROW_PREDICATE_ROOTS` / `FIELD_RULE_ROOTS` / `FORMULA_ROOTS`. The platform + * publishes the same judgement as `fieldRuleRootIssue` / `FIELD_RULE_BOUND_ROOTS`. + * Two copies of one judgement agree today and nothing keeps them agreeing. + * + * ## What did NOT move, and is pinned here as such + * + * The helper judges against `FIELD_RULE_BOUND_ROOTS` = `record` / `previous` / + * `parent` — whatever slot name it is handed. It has no vocabulary gate of its + * own (measured: an unrecognised slot still returns a finding, not `null`), so + * WHICH surfaces it may answer for is objectui's call, not the platform's. Two + * of the surfaces `rowCanonAdvisory` guards bind a set that is not that one, and + * neither is comparable to it — they overlap on `record` alone: + * + * - the conditional-formatting `condition` binds `ROW_PREDICATE_ROOTS` + * (`record`, `current_user`, `user`, `features`, `os`, `ctx`) — five roots + * the field tier does not bind, but NOT a superset: no `previous`, no + * `parent`; + * - a `formula` field's `expression` binds `FORMULA_ROOTS` (`record`) — a + * proper subset. + * + * So the helper's verdict is adopted exactly on the three slots whose bound set + * IS the field-rule set, and the local fallback is KEPT for the rest. The two + * "uncovered surface" pins below are live controls for the failure this card + * exists to stop, in miniature: routing every surface through the helper to + * make the code tidier would redden them. + * + * Severity stays objectui's own mapping (`warning`, never `error`) on both + * paths — every save gate on this tier counts `severity === 'error'`. + */ + +import { describe, it, expect } from 'vitest'; +import { fieldRuleRootIssue, FIELD_RULE_BOUND_ROOTS } from '@objectstack/lint'; +import { SCOPE_ROOTS } from '@objectstack/formula'; +import { lintCelPredicate } from './celAuthoring'; + +const HINT = { objectName: 'account', fields: ['organization_id', 'owner_id', 'status', 'amount'] }; +/** A field conditional rule — a slot the published helper's vocabulary covers. */ +const RULE_SLOT_HINT = { ...HINT, scope: 'record' as const, slot: 'visibleWhen' as const }; +/** The same tier with no slot named — the local fallback, unchanged. */ +const UNSLOTTED_HINT = { ...HINT, scope: 'record' as const }; + +/** objectui's own advisory sentence; the engine's never contains it. */ +const OBJECTUI_MESSAGE_MARKER = /Re-root/; + +describe('celAuthoring · the field-rule verdict is the published one (objectui#9318)', () => { + it('TRUE POSITIVE — a covered slot takes BOTH the verdict and the message from `fieldRuleRootIssue`', async () => { + const source = "data.status == 'x'"; + const issues = await lintCelPredicate(source, RULE_SLOT_HINT); + const engine = fieldRuleRootIssue('visibleWhen', source); + // The helper has something to say about this source — if it ever stops, + // this pin is measuring nothing and must be re-derived, not relaxed. + expect(engine).not.toBeNull(); + const advisory = issues.filter((i) => i.severity === 'warning' && i.message === engine!.message); + // Verbatim equality against the helper's OWN output, evaluated here rather + // than transcribed: this asserts "objectui ships the engine's message" and + // can never drift with upstream wording the way a quoted string would. + expect(advisory).toHaveLength(1); + }); + + it('ships ONE message, never both — objectui\'s sentence is gone from the covered path', async () => { + const issues = await lintCelPredicate("data.status == 'x'", RULE_SLOT_HINT); + expect(issues.filter((i) => OBJECTUI_MESSAGE_MARKER.test(i.message))).toEqual([]); + expect(issues.filter((i) => i.severity === 'warning')).toHaveLength(1); + }); + + it('LIVE CONTROL — the ACCEPT SET is not narrowed: the covered path raises no error', async () => { + // Every save gate on this tier counts `severity === 'error'` and nothing + // else, so "zero errors" IS "still accepted". Reddens on exactly one + // change — promoting the advisory to `error`. + const issues = await lintCelPredicate("data.status == 'x'", RULE_SLOT_HINT); + expect(issues.filter((i) => i.severity === 'error')).toEqual([]); + }); + + it('a root the platform DOES bind comes back clean THROUGH the new path', async () => { + // The ablation target: `previous` and `parent` are clean here because + // `FIELD_RULE_BOUND_ROOTS` says the field level binds them, not because + // objectui's detector only ever looked at `data`. + expect(await lintCelPredicate("previous.status == 'x'", RULE_SLOT_HINT)).toEqual([]); + expect(await lintCelPredicate('parent.status == "paid"', RULE_SLOT_HINT)).toEqual([]); + expect(await lintCelPredicate("record.status == 'x'", RULE_SLOT_HINT)).toEqual([]); + }); + + it('takes the WIDER verdict on a covered slot: a root no field rule binds is advised too', async () => { + // A declared behaviour change, and the substance of adopting the published + // verdict: `current_user` is in the engine's baseline `SCOPE_ROOTS`, so + // `validateExpression` says nothing about it, while the FIELD level does + // not bind it. objectui's one-root detector could never reach this. + // "WIDER" in the name is this verdict against the ONE-ROOT detector it + // replaces — ⛔ not a set relation between the two surfaces' bound roots, + // which are incomparable (see the header). + const issues = await lintCelPredicate('current_user.isAdmin', RULE_SLOT_HINT); + const advisory = issues.filter((i) => i.severity === 'warning' && /current_user/.test(i.message)); + expect(advisory).toHaveLength(1); + expect(issues.filter((i) => i.severity === 'error')).toEqual([]); + }); + + it('stands down on a parse error and on a non-CEL dialect, like the path it replaces', async () => { + const broken = await lintCelPredicate('data.status ==', RULE_SLOT_HINT); + expect(broken.some((i) => i.severity === 'error')).toBe(true); + expect(broken.filter((i) => i.severity === 'warning' && /\bdata\b/.test(i.message))).toEqual([]); + const legacy = await lintCelPredicate('${data.status}', RULE_SLOT_HINT); + expect(legacy.filter((i) => i.severity === 'warning' && /\bdata\b/.test(i.message))).toEqual([]); + }); +}); + +describe('celAuthoring · the UNCOVERED surfaces keep the local fallback (objectui#9318 part 3)', () => { + it('LIVE CONTROL — a formula `expression` still gets objectui\'s message, not the engine\'s', async () => { + // `FORMULA_ROOTS` is `['record']` — a proper SUBSET of + // `FIELD_RULE_BOUND_ROOTS`. Routing this through the helper would not shrink + // coverage: measured, `previous.*` / `parent.*` report nothing on this + // surface today either. It would report them CLEAN on the helper's own + // authority — asserting they are bound where this surface binds only + // `record`. A false green, which is worse than the silence it replaces. + const issues = await lintCelPredicate('data.amount * 0.2', { ...UNSLOTTED_HINT, role: 'value' as const }); + const advisory = issues.filter((i) => i.severity === 'warning' && OBJECTUI_MESSAGE_MARKER.test(i.message)); + expect(advisory).toHaveLength(1); + expect(issues.filter((i) => i.severity === 'error')).toEqual([]); + }); + + it('LIVE CONTROL — a conditional-formatting condition is NOT advised for the roots it binds', async () => { + // `ROW_PREDICATE_ROOTS` carries `current_user`, `user`, `features`, `os`, + // `ctx` on top of `record` — five roots the field tier does not bind. It is + // NOT a superset of `FIELD_RULE_BOUND_ROOTS` though: it lacks `previous` and + // `parent`, so the two sets overlap on `record` alone. This is the pin that + // reddens if someone routes every `scope: 'record'` surface through the + // helper: the author would be told to rewrite five roots that work here. + expect(await lintCelPredicate('current_user.isAdmin', UNSLOTTED_HINT)).toEqual([]); + expect(await lintCelPredicate("os.name == 'x'", UNSLOTTED_HINT)).toEqual([]); + }); + + it('an unknown slot name falls back rather than guessing', async () => { + const issues = await lintCelPredicate('current_user.isAdmin', { ...UNSLOTTED_HINT, slot: 'someFutureWhen' }); + expect(issues).toEqual([]); + }); +}); + +describe('celAuthoring · platform drift tripwire (objectui#9318)', () => { + it('the published bound set is still the one objectui measured its coverage answer against', async () => { + // NOT a second copy of the verdict — the verdict is read from the helper at + // runtime and this assertion is never consulted by product code. It exists + // so that the next root the platform binds or unbinds arrives as a RED test + // in objectui, with the instruction to re-derive which of this repo's + // surfaces the helper's vocabulary still covers (PR objectui#9318 part 3). + expect([...FIELD_RULE_BOUND_ROOTS]).toEqual(['record', 'previous', 'parent']); + }); + + it('SWEEPS the whole unbound population: every root is advised EXCEPT `app`', async () => { + // The instrument behind the docblock's universal. The published prose says + // "every root the field level leaves unbound is advised, except `app`" and + // ⛔ names no others — because any written-down list goes stale the next + // time the platform moves a root. This sweep re-derives the population on + // every run instead, so the claim cannot rot silently: it reddens when a + // root stops being advised, when a SECOND root starts being blocked, or + // when any root goes silent. + // + // ⚠️ The universe is RECONSTRUCTED, and that is a declared limit, not an + // oversight: `@objectstack/lint` keeps both `FIELD_RULE_JUDGED_ROOTS` and + // `FIELD_RULE_AMBIENT_ROOTS` module-private (neither is in its export list + // at 17.4.0), so the judged set is rebuilt here as `SCOPE_ROOTS` plus the + // one ambient root. A NEW ambient root added upstream would be invisible to + // this sweep until that literal is updated — nothing in this repo can see + // it. The `app` leg below pins the reconstruction itself. + const judged: readonly string[] = [...SCOPE_ROOTS, 'app']; + const bound: readonly string[] = FIELD_RULE_BOUND_ROOTS; + const candidates = judged.filter((r) => !bound.includes(r)); + expect(candidates.length).toBeGreaterThan(1); // the sweep is measuring something + + const advised: string[] = []; + const blocked: string[] = []; + const silent: string[] = []; + for (const root of candidates) { + const source = `${root}.x == 1`; + const issues = await lintCelPredicate(source, RULE_SLOT_HINT); + const engine = fieldRuleRootIssue('visibleWhen', source); + // Verbatim equality with the helper's own output, as elsewhere in this + // file — never a transcribed string. + if (engine && issues.some((i) => i.severity === 'warning' && i.message === engine.message)) advised.push(root); + else if (issues.some((i) => i.severity === 'error')) blocked.push(root); + else silent.push(root); + } + + // The universal, and the single documented exception. + expect(silent).toEqual([]); + expect(blocked).toEqual(['app']); + expect(advised).toEqual(candidates.filter((r) => r !== 'app')); + }); + + it('WHY `app` is the exception: the helper judges it, the platform does not DECLARE it', async () => { + // The exception is a mechanism, not a special case: the advisory is gated + // behind `issues.every((i) => i.severity !== 'error')`, and a root the + // engine does not declare raises a bare-reference ERROR first. `app` is the + // only judged root in that position — it is AMBIENT (renderer-mounted), + // which is exactly why the engine's own message for it refuses the + // `record.app` rewrite by name. + expect(SCOPE_ROOTS).not.toContain('app'); + expect(fieldRuleRootIssue('visibleWhen', 'app.theme == "dark"')).not.toBeNull(); + const issues = await lintCelPredicate('app.theme == "dark"', RULE_SLOT_HINT); + expect(issues.some((i) => i.severity === 'error')).toBe(true); + expect(issues.filter((i) => i.severity === 'warning')).toEqual([]); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/celAuthoring.ts b/packages/app-shell/src/views/metadata-admin/celAuthoring.ts index 94d8e4d6ac..0120d42aba 100644 --- a/packages/app-shell/src/views/metadata-admin/celAuthoring.ts +++ b/packages/app-shell/src/views/metadata-admin/celAuthoring.ts @@ -68,6 +68,19 @@ export interface CelSchemaHint { * would author a predicate that silently never fires. */ roots?: string[]; + /** + * The authored KEY this source is the value of — `visibleWhen`, + * `readonlyWhen`, `requiredWhen` (objectui#9318). Naming it lets the + * wrong-layer advisory take its verdict from `@objectstack/lint`'s published + * `fieldRuleRootIssue`, which judges per SLOT, instead of from a second copy + * of that judgement maintained here. + * + * Optional, and an unrecognised value is not an error: a surface that does + * not name a slot — or names one the published helper's vocabulary does not + * cover — keeps the local fallback. See {@link FIELD_RULE_VERDICT_SLOTS} for + * why that is a real set and not a formality. + */ + slot?: string; /** * The engine field role of the authoring site (mirrors `FieldRole` minus * `template`, which no CEL editor hosts): @@ -194,6 +207,84 @@ interface RowCanonModule { let rowCanonCached: Promise | null = null; +/** `@objectstack/lint`'s published per-slot verdict (objectui#9318). */ +type FieldRuleRootIssue = (slot: string, source: string) => { root: string; message: string } | null; + +let fieldRuleVerdictCached: Promise | null = null; + +/** + * Feature-detect `fieldRuleRootIssue` on the installed `@objectstack/lint`. + * + * The `import()` must stay DYNAMIC for the same reason `securityPostureLint.ts` + * keeps its own dynamic — `@objectstack/lint` is the one `@objectstack/*` + * package the console's `vendor-objectstack` chunk group does not claim + * (objectui#5266), so a static import would pull the whole lint bundle onto the + * eager graph. Progressive enhancement, like every other entry point in this + * module: a lint package without the export degrades to the local fallback, + * never to an exception and never to silence. + */ +function loadFieldRuleVerdict(): Promise { + if (!fieldRuleVerdictCached) { + fieldRuleVerdictCached = import('@objectstack/lint') + .then((m) => { + const fn = (m as unknown as Record)?.fieldRuleRootIssue; + return typeof fn === 'function' ? (fn as FieldRuleRootIssue) : null; + }) + .catch(() => null); + } + return fieldRuleVerdictCached; +} + +/** + * The authored slots whose bound-root set IS the platform's field-rule set, so + * `fieldRuleRootIssue`'s verdict answers THIS surface's question (objectui#9318). + * + * ⚠️ This is a list of objectui's own surfaces, not a copy of the platform's + * judgement — the judgement itself is read from `@objectstack/lint` at call + * time. Membership is measured, not assumed: `ObjectFieldInspector` offers + * exactly `FIELD_RULE_ROOTS` (`record` / `previous` / `parent`) on these three + * editors, which is `FIELD_RULE_BOUND_ROOTS` verbatim. + * + * ## What is deliberately NOT here, and why the local fallback stays + * + * ⚠️ `fieldRuleRootIssue` does NOT gate on the slot name. Hand it any string and + * it judges against `FIELD_RULE_BOUND_ROOTS` and interpolates the name into its + * message — measured on 17.4.0, `('expression', 'current_user.x')` and + * `('condition', 'current_user.x')` both return a finding, not `null`. The + * helper never declines a surface, so this list is objectui's OWN coverage + * answer and it is the only thing standing between a wrong surface and a + * confident wrong verdict. + * + * Two guarded surfaces bind a set that is not the field-rule set, and neither + * is comparable to it — they overlap on `record` alone: + * + * - a `formula` field's `expression` binds `FORMULA_ROOTS` — `['record']`, a + * proper SUBSET. Routed through the helper, `previous.*` / `parent.*` come + * back clean — the helper reports them BOUND at the field tier, and on a + * formula they are not bound at all. A false green; and the message it does + * print for other roots names `previous` and `parent` as available here, + * which on this surface is wrong prose as well as a wrong verdict; + * - a conditional-formatting `condition` binds `ROW_PREDICATE_ROOTS` — + * `record`, `current_user`, `user`, `features`, `os`, `ctx`. That is five + * roots the field tier does not bind, but it is NOT a superset of the + * field-rule set: it lacks `previous` and `parent`. Routed through the + * helper, all five would be advised as unbound where the surface binds them + * — an author told to rewrite a predicate that works. + * + * ⛔ Neither miss is a coverage SHRINK, and it is worth being exact about that: + * measured on this tree, both surfaces report nothing at all for `previous.*` / + * `parent.*` today, so nothing currently advised would stop being advised. The + * hazard is the other one — a verdict that is wrong in a DIFFERENT direction on + * each surface: too loud on the condition, too quiet on the formula. + * + * So those keep the local instrument. ⛔ Do not extend this list to "tidy up" + * the branch below without re-measuring the surface's bound roots first — + * narrowing a consumer to fit the API it adopted is the drift objectui#9318 + * exists to stop, and `celAuthoring.fieldRuleVerdict-9318.test.ts` pins both + * uncovered surfaces as live controls against exactly that edit. + */ +const FIELD_RULE_VERDICT_SLOTS: readonly string[] = ['visibleWhen', 'readonlyWhen', 'requiredWhen']; + /** * Load `@object-ui/core`'s row-spelling detector the same way the engine is * loaded: lazily, feature-detected, swallowing every failure. The detector @@ -267,13 +358,88 @@ function loadRowCanon(): Promise { * (ADR-0089 D3) is `views/metadata-admin/SchemaForm.tsx`, which evaluates through * `views/metadata-admin/predicate.ts` and never reaches this function — which is * why the gate below is `scope === 'record'` and not a source pattern. + * + * ## Where the VERDICT comes from since objectui#9318 + * + * "Is this root bound on this surface?" is a judgement the platform publishes: + * `@objectstack/lint` exports `fieldRuleRootIssue` / `FIELD_RULE_BOUND_ROOTS`, + * pinned upstream by a test named for rejecting `data` — the LEGAL root of the + * same key one layer over. objectui derived the same answer independently, from + * `ROW_PREDICATE_ROOTS` / `FIELD_RULE_ROOTS` / `FORMULA_ROOTS` plus the single + * root `@object-ui/core`'s detector hard-codes. Two hand-maintained copies of + * one judgement: they agree today, and the next root the platform binds or + * unbinds moves one and not the other, silently, in the direction objectui#8166 + * already paid for once. + * + * So on the slots {@link FIELD_RULE_VERDICT_SLOTS} names, the verdict is ASKED, + * not re-derived — and the engine's own message ships with it. + * + * ⚠️ The reason is NOT that the engine's message refuses a `record.` + * rewrite. Measured against the installed 17.4.0, it mostly PRESCRIBES one: the + * `current_user` text CONTAINS "To gate on record state, rewrite the predicate + * against `record`." — one of the three remedies it offers, and NOT where it + * ends (measured: `includes` true, `endsWith` false; every covered slot's text + * closes on "it is not a fourth answer"). (Exactly one root's message does + * refuse the rewrite by name — `app`'s, + * with "⛔ Do NOT write `record.app`" — and that one never reaches this function; + * see the gate below.) The real reason is that the verdict widens and objectui + * has no message for most of what it now judges: + * + * - the local instrument produces a sentence for exactly ONE root — + * `@object-ui/core`'s `METADATA_LAYER_ROOT`, `data`. For every OTHER root a + * covered slot now reports there is no objectui sentence to keep; the + * alternative is hand-writing one per root, which is the second copy this + * card exists to delete. ⛔ Do not write that population down — it is + * `FIELD_RULE_JUDGED_ROOTS` minus `FIELD_RULE_BOUND_ROOTS` minus `data`, and + * it moves whenever the platform moves either set. And they would have to be + * per-root: the engine's texts for `data`, the user-root family, the + * platform-wide family and the ambient family are four different remedies, + * not one sentence with the root substituted; + * - the one message actually SWAPPED is `data`'s, and objectui's tail there — + * "Re-root the reference on `record`" — is the half that does not generalise. + * It is right for `data` and wrong for `current_user`, which is not a field of + * the record. Reading a verdict from one authority and explaining it from + * another drifts exactly the way two verdicts do. + * + * ⛔ Exactly one message ships per finding; the two are never concatenated. + * + * Two consequences, both deliberate and both pinned: + * + * - a covered slot now advises on EVERY root the field level leaves unbound — + * except `app`. ⛔ The exception is a MECHANISM, not a list, and this comment + * deliberately enumerates neither side of it: the advisory runs only once the + * predicate is error-free (`issues.every((i) => i.severity !== 'error')`, + * below), and `app` is the single root the helper judges that the platform + * does not DECLARE (`FIELD_RULE_JUDGED_ROOTS` is `SCOPE_ROOTS` plus the + * ambient `app`), so for `app` alone the pre-existing bare-reference ERROR + * fires first and the advisory never runs — even though the helper judges it + * and carries a bespoke message for it. A name the helper does not judge at + * all is stopped by that same error, and was never a candidate. ⛔ Writing + * the advised roots out here would go stale the next time the platform moves + * either set; `celAuthoring.fieldRuleVerdict-9318.test.ts` sweeps the whole + * population instead. Still `warning`, so no save gate's accept set moves; + * - the surfaces {@link FIELD_RULE_VERDICT_SLOTS} does not name keep this + * function's own reading, unchanged. ⛔ Their coverage is not shrunk to match + * the helper. + * + * Severity is the one thing that stays objectui's: `warning`, never `error`. */ -function rowCanonAdvisory(finding: { - kind: string; - identifier: string; - canonical: string; -}): CelLintIssue | null { - if (finding.kind !== 'metadata-layer-root') return null; +async function rowCanonAdvisory(source: string, slot: string | undefined): Promise { + if (slot !== undefined && FIELD_RULE_VERDICT_SLOTS.includes(slot)) { + const fieldRuleRootIssue = await loadFieldRuleVerdict(); + if (fieldRuleRootIssue) { + const issue = fieldRuleRootIssue(slot, source); + // `null` = the source does not parse, or every root it reads is bound + // here. Both are "nothing to report" upstream and here. + return issue ? { severity: 'warning', message: issue.message } : null; + } + // An older/absent `@objectstack/lint` has no verdict to give. Fall through + // to the local instrument rather than going quiet — progressive + // enhancement never costs an author a diagnostic they had yesterday. + } + const canon = await loadRowCanon(); + const finding = canon?.detectNonCanonicalRowSpelling?.(source, null, true); + if (!finding || finding.kind !== 'metadata-layer-root') return null; return { severity: 'warning', message: @@ -302,8 +468,10 @@ function rowCanonAdvisory(finding: { * expression (usually paired with `scope: 'record'`, where a bare field ref IS * a hard error — it silently evaluates to null at runtime). * - * At `scope: 'record'` one finding comes from outside the engine: the - * wrong-layer `data.*` advisory described on {@link rowCanonAdvisory}. It is + * At `scope: 'record'` one finding comes from outside `validateExpression`: the + * wrong-layer root advisory described on {@link rowCanonAdvisory}, whose verdict + * comes from `@objectstack/lint` when {@link CelSchemaHint.slot} names a slot + * that helper's vocabulary covers and from the local instrument otherwise. It is * always a `warning`, so it never narrows what this surface accepts. * * Empty input is always clean. @@ -352,13 +520,12 @@ export async function lintCelPredicate( /* advisory only — never let it break the lint */ } } - // Wrong-layer `data.*` advisory (objectui#8972) — see `rowCanonAdvisory`. - // Only in `record` scope, only once the predicate parses, only a WARNING. + // Wrong-layer root advisory (objectui#8972, verdict re-homed by objectui#9318) + // — see `rowCanonAdvisory`. Only in `record` scope, only once the predicate + // parses, only a WARNING. if (issues.every((i) => i.severity !== 'error') && hint.scope === 'record') { try { - const canon = await loadRowCanon(); - const finding = canon?.detectNonCanonicalRowSpelling?.(source, null, true); - const advisory = finding ? rowCanonAdvisory(finding) : null; + const advisory = await rowCanonAdvisory(source, hint.slot); if (advisory) issues.push(advisory); } catch { /* advisory only — never let it break the lint */ diff --git a/packages/app-shell/src/views/metadata-admin/clientValidation.ts b/packages/app-shell/src/views/metadata-admin/clientValidation.ts index b2871dac6f..39e9975e70 100644 --- a/packages/app-shell/src/views/metadata-admin/clientValidation.ts +++ b/packages/app-shell/src/views/metadata-admin/clientValidation.ts @@ -810,6 +810,12 @@ async function validateObjectFieldRules(draft: unknown): Promise e.name)} scope="record" roots={FIELD_RULE_ROOTS} + // The authored key, so the wrong-layer advisory reads the + // platform's published per-slot verdict (objectui#9318). Sound + // here because `FIELD_RULE_ROOTS` above IS that helper's + // `FIELD_RULE_BOUND_ROOTS`; the formula editor above deliberately + // names no slot, because `FORMULA_ROOTS` is not. + slot="visibleWhen" t={tr} /> e.name)} scope="record" roots={FIELD_RULE_ROOTS} + // The authored key, so the wrong-layer advisory reads the + // platform's published per-slot verdict (objectui#9318). Sound + // here because `FIELD_RULE_ROOTS` above IS that helper's + // `FIELD_RULE_BOUND_ROOTS`; the formula editor above deliberately + // names no slot, because `FORMULA_ROOTS` is not. + slot="readonlyWhen" t={tr} /> e.name)} scope="record" roots={FIELD_RULE_ROOTS} + // The authored key, so the wrong-layer advisory reads the + // platform's published per-slot verdict (objectui#9318). Sound + // here because `FIELD_RULE_ROOTS` above IS that helper's + // `FIELD_RULE_BOUND_ROOTS`; the formula editor above deliberately + // names no slot, because `FORMULA_ROOTS` is not. + slot="requiredWhen" t={tr} />