From e89c2e1dcd08aa7c17a30a08c889222ad58f8ff2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 13:03:39 +0000 Subject: [PATCH] fix(lint): judge a schema-bound metadata form at its own binding layer (#7815) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runtime publish gate calls `validateVisibilityPredicates(stack)` with no options, so every view was judged at the `'runtime'` layer default — including schema-bound metadata forms, which bind the row under edit as `data`. Correct metadata therefore drew a `visibility-root-mislayered` advisory telling its author to write `record.`, and `visibility-bare-identifier`'s hint prescribed `record.`, on a surface that binds no `record` at all. The layer is now read off the metadata where the metadata states it: a form view declaring `data: { provider: 'schema', schemaId }` is judged at `metadata`, and every other site — a plain runtime view, every page component — still takes `opts.layer`. Derived from the same `schemaIdOf` call that decides the #7696 right-hand-literal-slot stand-down, so the two cannot disagree about which surface they are on. The rule itself is unchanged; this is layer plumbing at the invocation side. `visibility-root-mislayered` is `warning` in both directions, so acceptance is untouched — measured as identical error sets across the 19-row controls corpus, and pinned as a property in `runtime-gate.test.ts`. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WocN37om5bw81JDoEEMA2e --- .../visibility-layer-schema-bound-form.md | 36 +++++ packages/lint/src/runtime-gate.test.ts | 92 +++++++++++ .../validate-visibility-predicates.test.ts | 151 +++++++++++++++++- .../src/validate-visibility-predicates.ts | 85 ++++++++-- 4 files changed, 351 insertions(+), 13 deletions(-) create mode 100644 .changeset/visibility-layer-schema-bound-form.md diff --git a/.changeset/visibility-layer-schema-bound-form.md b/.changeset/visibility-layer-schema-bound-form.md new file mode 100644 index 0000000000..7b7f9daef7 --- /dev/null +++ b/.changeset/visibility-layer-schema-bound-form.md @@ -0,0 +1,36 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): judge a schema-bound metadata form at its own binding layer (#7815) + +At the runtime publish gate, `validateVisibilityPredicates` ran at its +`'runtime'` layer default for **every** view, including schema-bound metadata +forms (`data: { provider: 'schema', schemaId }`). Those forms bind the row under +edit as `data`, so correct metadata drew a `visibility-root-mislayered` advisory +telling its author to write `record.` — a root that surface binds nothing under — +and `visibility-bare-identifier`'s hint prescribed `record.` for the same +reason. Nothing went red, which is how it survived: the gate was green, the +advisory was simply wrong, and the only symptom was authors and AI authors being +steered to the wrong root at the publish door. + +The layer is now read off the metadata where the metadata states it. A form view +declaring `data: { provider: 'schema', schemaId }` is judged at `metadata`; every +other site — a plain runtime view, every page component — still takes the +caller's `opts.layer` (default `'runtime'`), so `os validate` / `compile` and any +file-aware caller are unchanged. It is derived from the same `schemaIdOf` call +that already decides the right-hand-literal-slot stand-down (#7696), so the two +verdicts cannot disagree about which surface they are on. + +Three consequences on a schema-bound form, all advisory: + +- a correctly `data.`-rooted predicate no longer draws the mis-layer advisory; +- a `record.`-rooted one now does, in ADR-0089 D3's other direction — that + predicate can never match, and this door was silent about it; +- `visibility-bare-identifier` prescribes `data.`, the root the surface + actually binds. + +`visibility-root-mislayered` is `warning` in both directions and no other rule's +severity or firing condition moves, so **acceptance is untouched**: the same +inputs are refused, with the same ids, at the same paths. That boundary is pinned +as a property in `runtime-gate.test.ts` rather than left as a claim. diff --git a/packages/lint/src/runtime-gate.test.ts b/packages/lint/src/runtime-gate.test.ts index 70e92773fd..e7d228b426 100644 --- a/packages/lint/src/runtime-gate.test.ts +++ b/packages/lint/src/runtime-gate.test.ts @@ -491,3 +491,95 @@ describe('the views[] visibility-predicate family at the runtime publish gate (# expect(sawAdvisory, 'the corpus must contain an advisory').toBe(true); }); }); + +// ───────────────────────────────────────────────────────────────────── +// #7815 — the layer the gate judges a schema-bound form at. +// +// A separate block, added at the end rather than woven into the family suite +// above (#7576 serialization): nothing in that suite is touched. +// ───────────────────────────────────────────────────────────────────── + +describe('the publish gate judges a schema-bound form at its own layer (#7815)', () => { + it('a correctly `data.`-rooted form is SILENT — no advisory, no refusal', () => { + // The finding. `authoring-rules.ts` calls `validateVisibilityPredicates(stack)` + // with no options, so every view was judged at the `'runtime'` default and + // this exact body — correct metadata — came back with an advisory telling + // its author to write `record.`, on a surface that binds no `record` at all. + // Nothing went red then and nothing goes red now; the difference is only + // what the author is told, which is the whole harm of the class. + const result = gateView(schemaBoundForm("data.type == 'text'")); + expect(result.errors, JSON.stringify(result.errors)).toEqual([]); + expect(result.advisories, JSON.stringify(result.advisories)).toEqual([]); + // "clean" and "nothing ran" must stay distinguishable. + expect(result.rulesRun).toEqual([ + 'validateVisibilityPredicates', + 'validatePredicatePathRefs', + ]); + }); + + it('the runtime view one fixture-line away still draws it — the rule is live', () => { + // The negative control. Standing an advisory down and going blind look + // identical from the inside, so the case above is only worth its ink beside + // one that still fires: same predicate root, same gate, opposite verdict, + // decided by the `data:` source alone. + const f = gateView(runtimeView("data.status == 'open'")) + .advisories.find((a) => a.rule === 'visibility-root-mislayered'); + expect(f, 'a wrong-layer paste on a RUNTIME view is still a wrong-layer paste').toBeDefined(); + expect(f!.severity).toBe('warning'); + }); + + it('a `record.`-rooted form now draws the advisory the OTHER way', () => { + // ADR-0089 D3's second direction, unreachable at this door until now: told + // `'runtime'`, the rule forbids `data.` and has nothing to say about + // `record.`, so a form predicate that can never match published in silence. + // Not new behaviour — the rule's existing metadata arm, finally addressed. + const result = gateView(schemaBoundForm("record.type == 'text'")); + expect(result.errors, 'still advisory-only: acceptance is untouched').toEqual([]); + const f = result.advisories.find((a) => a.rule === 'visibility-root-mislayered'); + expect(f, 'a predicate that never matches must reach the author').toBeDefined(); + expect(f!.severity).toBe('warning'); + expect(f!.hint).toMatch(/data/); + }); + + it('prescribes the root the surface actually binds when it REFUSES', () => { + // The second half of the finding: the layer default also chose the root + // quoted inside `visibility-bare-identifier`'s hint, so the loudest finding + // on this surface — the one that BLOCKS the publish — told the author to + // write `record.status` on a form that binds `data`. Same id, same + // severity, same input: only the prescription moved. + const f = gateView(schemaBoundForm('status == active')) + .errors.find((e) => e.rule === 'visibility-bare-identifier'); + expect(f, 'a bare word on the LEFT is still refused').toBeDefined(); + expect(f!.severity).toBe('error'); + expect(f!.hint).toContain('`data.status`'); + expect(f!.hint).not.toContain('`record.status`'); + }); + + it('moves NO finding across the error/advisory boundary', () => { + // The acceptance guarantee this card is bounded by, as a property over the + // schema-bound half of the family corpus: the derivation may only ever + // change which ADVISORIES are emitted. Asserted as exact error sets — each + // one is what the `'runtime'` reading produced on the same input — so a + // later edit to the layer plumbing that promoted or demoted anything goes + // red here rather than at a tenant's publish door. + const cases: Array<[string, string[]]> = [ + ["data.type == 'text'", []], + ["record.type == 'text'", []], + ['data.type == active', []], + ['status == active', ['visibility-bare-identifier']], + ['data.name == active && active', ['visibility-bare-identifier']], + ['active == data.type', ['predicate-rhs-path-shaped', 'visibility-bare-identifier']], + ["data.tpye == 'text'", ['predicate-path-unresolved']], + ["type == 'text'", ['predicate-path-unrooted']], + ['data.type == data.label', ['predicate-rhs-path-shaped']], + ["country === 'USA'", ['visibility-predicate-syntax']], + ]; + for (const [predicate, expected] of cases) { + expect( + gateView(schemaBoundForm(predicate)).errors.map((e) => e.rule).sort(), + `the refusal set for \`${predicate}\` changed — the #7815 layer derivation is ` + + `advisory-only by construction, so anything moving here is a scope breach`, + ).toEqual([...expected].sort()); + } + }); +}); diff --git a/packages/lint/src/validate-visibility-predicates.test.ts b/packages/lint/src/validate-visibility-predicates.test.ts index 01e299f843..bbcf1dcbf1 100644 --- a/packages/lint/src/validate-visibility-predicates.test.ts +++ b/packages/lint/src/validate-visibility-predicates.test.ts @@ -255,6 +255,148 @@ describe('validateVisibilityPredicates (ADR-0089 D3b)', () => { }); }); +// ───────────────────────────────────────────────────────────────────── +// #7815 — WHICH LAYER a site is on, when the caller does not say. +// +// The rule above is correct for the layer it is told. What it was told at the +// runtime publish gate was the `'runtime'` default for EVERY view, including +// schema-bound metadata forms — so a correctly `data.`-rooted form drew the +// advisory telling its author to write `record.`. These cases pin the +// derivation itself; `runtime-gate.test.ts` pins it at the door it was wrong at. +// ───────────────────────────────────────────────────────────────────── + +describe('the layer a site declares for itself (#7815)', () => { + /** + * A schema-bound metadata form, with the data source on the CONTAINER (the + * `self` rung of the `formViewSites` ladder). + */ + const metaForm = (predicate: string) => ({ + views: [{ + name: 'field_editor', + data: { provider: 'schema', schemaId: 'field' }, + sections: [{ fields: [{ field: 'notes', visibleWhen: predicate }] }], + }], + }); + + /** The same predicate on a plain runtime view — the negative control. */ + const runtimeForm = (predicate: string) => ({ + views: [{ name: 'task_form', sections: [{ fields: [{ field: 'notes', visibleWhen: predicate }] }] }], + }); + + it('a `data.`-rooted predicate on a schema-bound form is CORRECT — no advisory', () => { + // The finding this card is about. `data` IS the root that surface binds. + expect(validateVisibilityPredicates(metaForm("data.type == 'grid'"))).toEqual([]); + }); + + it('the same predicate on a plain runtime view still draws it — the rule is live', () => { + // The negative control that keeps the case above from being a walk that + // simply went blind: one character of difference in the fixture (the + // `data:` source), opposite verdicts. + const findings = validateVisibilityPredicates(runtimeForm("data.type == 'grid'")); + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_ROOT_MISLAYERED]); + expect(findings[0].severity).toBe('warning'); + }); + + it('a `record.`-rooted predicate on a schema-bound form draws it the OTHER way', () => { + // ADR-0089 D3 is bidirectional and this direction was unreachable at the + // runtime gate: told `'runtime'`, the rule forbids `data.` and says nothing + // about `record.`, so a form predicate that never matches published silent. + // No new behaviour — this is the metadata-layer arm the rule already had. + const findings = validateVisibilityPredicates(metaForm("record.type == 'grid'")); + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_ROOT_MISLAYERED]); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].message).toContain('record.'); + expect(findings[0].hint).toContain('data'); + }); + + it('derives per SITE, not per stack — one entry can carry both kinds', () => { + // `formViews.` sub-containers each declare their own `data`, so a + // stack-level layer would be wrong for one of these two no matter which + // value it took. + const stack = { + views: [{ + name: 'mixed', + object: 'account', + formViews: { + meta: { + data: { provider: 'schema', schemaId: 'field' }, + sections: [{ fields: [{ field: 'a', visibleWhen: "data.type == 'grid'" }] }], + }, + live: { + sections: [{ fields: [{ field: 'b', visibleWhen: "data.type == 'grid'" }] }], + }, + }, + }], + }; + const findings = validateVisibilityPredicates(stack); + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_ROOT_MISLAYERED]); + expect(findings[0].path).toBe('views[0].formViews.live.sections[0].fields[0]'); + }); + + it('`opts.layer` still governs every site that declares no data source', () => { + // The file-aware caller's contract is unchanged: a `*.form.ts` whose form + // carries no `data: { provider: 'schema' }` is still only reachable through + // the option, and a page component always is. + expect(validateVisibilityPredicates(runtimeForm("data.type == 'grid'"), { layer: 'metadata' })) + .toEqual([]); + expect( + validateVisibilityPredicates(runtimeForm("record.type == 'grid'"), { layer: 'metadata' }) + .map((f) => f.rule), + ).toEqual([VISIBILITY_ROOT_MISLAYERED]); + + const page = (predicate: string) => ({ + pages: [{ name: 'p', regions: [{ components: [{ type: 'element:text', visibleWhen: predicate }] }] }], + }); + expect(validateVisibilityPredicates(page("data.x == 'y'")).map((f) => f.rule)) + .toEqual([VISIBILITY_ROOT_MISLAYERED]); + expect(validateVisibilityPredicates(page("data.x == 'y'"), { layer: 'metadata' })).toEqual([]); + }); + + it('an unresolvable `schemaId` is still a schema-bound SURFACE', () => { + // The layer follows the data SOURCE, not whether the id resolves — the same + // boundary `literalRhs` draws off the same `schemaIdOf` call, so the two + // cannot disagree about which surface they are on. + expect(validateVisibilityPredicates({ + views: [{ + name: 'f', + data: { provider: 'schema', schemaId: 'no_such_schema' }, + sections: [{ fields: [{ field: 'x', visibleWhen: "data.a == 'b'" }] }], + }], + })).toEqual([]); + }); + + it('a non-schema provider is NOT a metadata form', () => { + // `schemaIdOf` reads `provider === 'schema'` only; an ObjectQL-backed data + // source is a runtime surface and keeps the runtime direction. + const findings = validateVisibilityPredicates({ + views: [{ + name: 'f', + data: { provider: 'object', object: 'account' }, + sections: [{ fields: [{ field: 'x', visibleWhen: "data.a == 'b'" }] }], + }], + }); + expect(findings.map((f) => f.rule)).toEqual([VISIBILITY_ROOT_MISLAYERED]); + }); + + it('moves NO finding across the error/advisory boundary', () => { + // The acceptance guarantee, asserted rather than argued: the derivation may + // only ever change which ADVISORIES an author hears. Every fixture here is + // schema-bound — the set the derivation moves — and every `error` on it is + // the same id, at the same path, that the `'runtime'` reading produced. + const errorsOf = (predicate: string) => + validateVisibilityPredicates(metaForm(predicate)) + .filter((f) => f.severity === 'error') + .map((f) => f.rule) + .sort(); + + expect(errorsOf("data.type == 'grid'")).toEqual([]); + expect(errorsOf("record.type == 'grid'")).toEqual([]); + expect(errorsOf('status == active')).toEqual([VISIBILITY_BARE_IDENTIFIER]); + expect(errorsOf('active == data.type')).toEqual([VISIBILITY_BARE_IDENTIFIER]); + expect(errorsOf("country === 'USA'")).toEqual([VISIBILITY_PREDICATE_SYNTAX]); + }); +}); + // ───────────────────────────────────────────────────────────────────── // `visibility-bare-identifier` — #6128 (the build-time half of #5149's // 2026-08-06 ruling; the runtime warn-once half landed as objectui#3541). @@ -475,8 +617,15 @@ describe('visibility-bare-identifier (#6128 / #5149 requirement 3)', () => { it('proves the scanner still sees — the stand-down is per IDENTIFIER', () => { // Every one of these is the same schema-bound form, so a walk that had // gone blind would report nothing here either. + // + // #7815: this pin used to read `record.status`, which is what the rule + // said here while the caller's `'runtime'` default decided the layer for a + // form that binds no `record` at all. The refusal is unchanged — same id, + // same `error`, same one finding; only the ROOT it prescribes moved to the + // one this surface actually binds. (That the pin had to change is the + // measurement: an assertion was holding the wrong prescription in place.) expect(bareFindings(metaForm('status == active')).map((f) => f.hint)) - .toEqual([expect.stringContaining('`record.status`')]); + .toEqual([expect.stringContaining('`data.status`')]); expect(bareFindings(metaForm('active == data.type'))).toHaveLength(1); expect(bareFindings(metaForm('data.type == active && active'))).toHaveLength(1); // A macro body produces no replacement finding, so nothing stands down. diff --git a/packages/lint/src/validate-visibility-predicates.ts b/packages/lint/src/validate-visibility-predicates.ts index 43ae45d491..7d5aee07fc 100644 --- a/packages/lint/src/validate-visibility-predicates.ts +++ b/packages/lint/src/validate-visibility-predicates.ts @@ -78,9 +78,9 @@ * predicate here is a wrong-layer paste that silently never matches; and * - **metadata-editing** forms (`*.form.ts` — the row under edit) bind `data`, so * a `record.`-rooted predicate there is the same bug in the other direction. - * The layer is supplied by the caller (`opts.layer`, default `'runtime'`): the - * app-lint path (`os validate` / `compile`) always lints runtime surfaces, while a - * file-aware caller linting a `*.form.ts` passes `layer: 'metadata'`. + * The layer is read off the METADATA where the metadata declares it, and taken + * from the caller (`opts.layer`, default `'runtime'`) only where it does not — + * see §Which layer a site is on (#7815). * * Scope: `views` — every form view reachable from a `views[]` entry (the entry * itself when it IS a form view, plus the container's `form` and each @@ -283,6 +283,40 @@ * `validatePredicatePathRefs` walks on, so every token this rule stops * reporting is reported by that one — as a `warning`, which is the severity * #7659 already argued for a spelling that renders correctly today. + * + * ## Which layer a site is on (#7815) + * + * `opts.layer` is the layer for sites whose binding environment the metadata + * does **not** state. A form view that declares `data: { provider: 'schema', + * schemaId }` states it: that data source is what makes the console's + * metadata-admin evaluator the thing that renders the surface, and that + * evaluator binds the row under edit as `data`. So such a site is judged at + * `metadata` whatever the caller's default is, and every other site (a plain + * runtime view, every page component) takes `opts.layer`. + * + * This is plumbing, not a change to `visibility-root-mislayered`, which was + * always correct for the layer it was told. What was wrong is that at the + * **runtime publish gate** (`authoring-rules.ts` → `validateVisibilityPredicates(stack)`, + * no options) nobody told it: every schema-bound form was judged at the + * `'runtime'` default, so a correctly `data.`-rooted form drew the advisory + * telling its author to write `record.`, and `visibility-bare-identifier`'s hint + * prescribed `record.` on a surface that binds no `record` at all. An + * advisory that is wrong on correct metadata, at the publish door, for a whole + * surface class — and nothing goes red, which is what made it survive. + * + * Derived per SITE rather than per stack because one `views[]` entry can carry + * both kinds (`formViews.` sub-containers each declare their own `data`), + * and it is derived from the same `schemaIdOf` call that decides `literalRhs`, + * so the layer verdict and the right-hand-slot stand-down can never disagree + * about which surface they are on — the reason `schemaIdOf` is shared in the + * first place (`predicate-rhs-position.ts`). + * + * ⚠️ Acceptance is untouched by the derivation. `visibility-root-mislayered` is + * `warning` in BOTH directions, so what moves is which advisories an author + * hears; the three `error` ids fire on exactly the inputs they fired on before, + * and only the ROOT quoted inside two of their hints changes (`record.x` → + * `data.x` on a schema-bound form, which is the fix, not a side effect). + * `runtime-gate.test.ts` pins that error/advisory boundary as a property. */ import { @@ -321,7 +355,17 @@ export type VisibilityLayer = 'runtime' | 'metadata'; /** Options for {@link validateVisibilityPredicates}. */ export interface VisibilityOptions { - /** Binding layer of the surface being linted. Defaults to `'runtime'`. */ + /** + * Binding layer for the sites whose layer the metadata does not state. + * Defaults to `'runtime'`. + * + * A form view that declares `data: { provider: 'schema', schemaId }` DOES + * state it — the metadata-admin evaluator renders that surface and binds the + * row under edit as `data` — so such a site is judged at `'metadata'` + * regardless of this option (#7815; see the module note §Which layer a site is + * on). Pass `'metadata'` to cover the forms that do not carry that data source + * either, which is what a file-aware caller linting a `*.form.ts` does. + */ layer?: VisibilityLayer; } @@ -836,17 +880,22 @@ function isFieldObject(entry: unknown): entry is AnyRec { * `visibility-predicate-over-budget` (#7217) and `visibility-bare-identifier` * (#6128) are `error` and the caller is expected to fail the build on them. * - * The binding-root check is layer-directional (ADR-0089 D3): pass - * `opts.layer = 'metadata'` when linting a `*.form.ts` metadata-editing form (so a - * `record.`-rooted predicate is flagged), or leave it at the `'runtime'` default for - * `*.view.ts` / `*.page.ts` surfaces (so a `data.`-rooted predicate is flagged). The - * syntax and bare-identifier checks are layer-agnostic. + * The binding-root check is layer-directional (ADR-0089 D3). A form view that + * declares `data: { provider: 'schema', schemaId }` is judged at `metadata` on + * its own say-so (#7815); for every other site pass `opts.layer = 'metadata'` + * when linting a `*.form.ts` metadata-editing form (so a `record.`-rooted + * predicate is flagged), or leave it at the `'runtime'` default for `*.view.ts` / + * `*.page.ts` surfaces (so a `data.`-rooted predicate is flagged). The syntax and + * bare-identifier checks are layer-agnostic — but the ROOT their hints prescribe + * is not, which is the second half of what #7815 fixes. */ export function validateVisibilityPredicates( stack: AnyRec, opts: VisibilityOptions = {}, ): VisibilityFinding[] { - const layer: VisibilityLayer = opts.layer ?? 'runtime'; + // The layer for every site that does not state its own — see §Which layer a + // site is on. + const declaredLayer: VisibilityLayer = opts.layer ?? 'runtime'; const findings: VisibilityFinding[] = []; // ── Views: every reachable form view's sections / groups, and their fields ── @@ -868,7 +917,16 @@ export function validateVisibilityPredicates( // A runtime `*.view.ts` predicate goes to real CEL, where a path on the // right is perfectly legal and a bare word there really is a dropped // root — so it is not in this set and its refusal is untouched. - const literalRhs = schemaIdOf(site.view) !== undefined; + // + // #7815 — and the same fact is what puts the site on the METADATA layer. + // One `schemaIdOf` call feeds both so the two verdicts cannot drift: the + // surface whose right-hand slot is a literal slot is exactly the surface + // whose row under edit is bound as `data`, because both are consequences + // of the one evaluator rendering it. `opts.layer` governs every OTHER site + // (a plain runtime view, and every page component below). + const schemaBound = schemaIdOf(site.view) !== undefined; + const literalRhs = schemaBound; + const layer: VisibilityLayer = schemaBound ? 'metadata' : declaredLayer; // `sections` (canonical) and `groups` (legacy alias → sections) both hold // FormSection objects with an optional visibility predicate + `fields`. for (const bucket of ['sections', 'groups'] as const) { @@ -912,7 +970,10 @@ export function validateVisibilityPredicates( const pageName = typeof page.name === 'string' ? page.name : undefined; const where = `page "${pageName ?? pagePath}"`; for (const walked of walkPageComponents(page, pagePath)) { - checkElement(walked.component, where, walked.path, layer, findings); + // No per-site derivation here: a page component declares no data source of + // its own, so nothing in the metadata states a layer for it and + // `opts.layer` is the only answer available (#7815). + checkElement(walked.component, where, walked.path, declaredLayer, findings); } }