diff --git a/.changeset/artifact-unbound-form-predicate-root-notice.md b/.changeset/artifact-unbound-form-predicate-root-notice.md new file mode 100644 index 0000000000..4909c8cbc1 --- /dev/null +++ b/.changeset/artifact-unbound-form-predicate-root-notice.md @@ -0,0 +1,50 @@ +--- +"@objectstack/metadata-core": patch +"@objectstack/metadata": patch +--- + +feat(metadata-core,metadata): warn the operator when a pre-current-era artifact carries form-view predicates that fault open (#12915) + +A form-view predicate binds `record` (+ `previous`, `parent`) in runtime record +forms, or `data` in metadata-editing forms. The contract states the failure mode +beside the vocabulary: **a bare identifier is unbound, the predicate faults, and +`visibleWhen`'s fault fallback is `true`** — so a field the predicate was +authored to hide renders for everyone. + +That is quiet alone and lethal in combination with the authoring pattern it +serves. Measured on a real deployment: an artifact built by released +`@objectstack/cli` 17.1.0 authors +`{ field: 'disqualification_reason', required: true, visibleWhen: 'status == "unqualified"' }` +— the era's working spelling. On a 17.2 runtime the predicate faults open, the +conditionally hidden field renders, and its unconditional `required: true` +blocks **every** record creation through the console, while the same payload +POSTs 201 through REST. Nothing refused and nothing logged, so the operator — +the only person who can rebuild the artifact — had no signal at all. + +The framework artifact door now emits **one deduped `warn` line per artifact** +naming the authored `engines.protocol` floor and the runtime spec version, how +many predicates on which views (with the first path as an anchor), the +fault-open consequence, and the remedy (`os build`). It rides the same funnel +that already carries the forward-conversion summaries, so both SaaS shapes are +covered: a single-DB multi-org runtime warns once at boot, and per-tenant-DB +kernels each warn at their own. + +**No behaviour change.** No refusal, no rewrite, no schema or contract edit — +the predicate keeps faulting open exactly as before, and the artifact bytes are +untouched. Rewriting a bare root to `record.` is a separate, deferred ADR-0087 +conversion. + +**Scoped to legacy artifacts by construction.** The notice fires only inside the +versioned window the forward conversion already opens (declared floor below the +running spec, or undeclared), read off that pass's own verdict rather than +recomputed. An artifact declaring the current or a newer floor gets zero notices +from this feature even when it carries bare roots — the boundary that keeps a +notice about legacy artifacts out of contract territory. + +Detection is exported from `@objectstack/metadata-core` as +`detectUnboundFormViewPredicateRoots` (with `BOUND_FORM_VIEW_PREDICATE_ROOTS`) +so other composed artifact doors can reuse one policy rather than fork it. It is +pure, read-only, and tuned to prefer silence over a false accusation: string +literals are stripped before the scan, only root position counts, call targets +are not roots, comprehension macros (whose iteration variable is locally bound) +are skipped whole, and AST-only envelopes pass. diff --git a/packages/metadata-core/src/form-predicate-root-policy.test.ts b/packages/metadata-core/src/form-predicate-root-policy.test.ts new file mode 100644 index 0000000000..3d59ec63e3 --- /dev/null +++ b/packages/metadata-core/src/form-predicate-root-policy.test.ts @@ -0,0 +1,269 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Unit pins for the unbound form-view predicate root detector (#12915 scope C). + * + * The detector's product requirement is asymmetric, and so is this suite: a + * MISSED exotic predicate costs one un-warned artifact, while a FALSE POSITIVE + * on a healthy current artifact trains operators to ignore the channel. So the + * negative cases below (string literals, calls, member access, comprehension + * macros, every bound root) carry as much weight as the positive one, and each + * exists because a naive "identifier not in the vocabulary" scan gets it wrong. + */ + +import { describe, it, expect } from 'vitest'; +import { + BOUND_FORM_VIEW_PREDICATE_ROOTS, + detectUnboundFormViewPredicateRoots, + unboundRootsInCelSource, +} from './form-predicate-root-policy.js'; + +/** The repro artifact's shape: one form view, one section, one gated field. */ +function definitionWithFieldPredicate(predicate: unknown, object = 'crm_lead'): unknown { + return { + manifest: { id: 'app.test', engines: { protocol: '^17.0.0-rc.1' } }, + views: [ + { + form: { + type: 'simple', + data: { object }, + sections: [ + { + name: 'main', + fields: [ + { field: 'name', required: true }, + { field: 'disqualification_reason', required: true, visibleWhen: predicate }, + ], + }, + ], + }, + }, + ], + }; +} + +const CEL = (source: string) => ({ dialect: 'cel', source }); + +describe('the bound vocabulary comes from the contract, not from this module', () => { + it('is exactly record / previous / parent / data', () => { + // `packages/spec/src/ui/view.zod.ts`, `FormFieldSchema.visibleWhen` and + // `FormSectionSchema.visibleWhen`: "Root: `record` (+ `previous`, + // `parent`) in runtime forms, or `data` in metadata forms." + expect([...BOUND_FORM_VIEW_PREDICATE_ROOTS]).toEqual(['record', 'previous', 'parent', 'data']); + }); + + it('excludes `current_user`, which the same prose calls unbound at field level', () => { + expect(BOUND_FORM_VIEW_PREDICATE_ROOTS).not.toContain('current_user'); + expect(unboundRootsInCelSource("current_user.id == record.owner")).toEqual(['current_user']); + }); +}); + +describe('unboundRootsInCelSource — the judgement under the traversal', () => { + it('flags the era spelling from the real incident', () => { + expect(unboundRootsInCelSource('status == "unqualified"')).toEqual(['status']); + }); + + it('says nothing about a predicate rooted at any bound identifier', () => { + for (const root of BOUND_FORM_VIEW_PREDICATE_ROOTS) { + expect(unboundRootsInCelSource(`${root}.status == "unqualified"`), root).toEqual([]); + } + }); + + it('does not mistake a MEMBER named like a root for a root', () => { + // A record field that happens to be called `status`, `data` or `features` + // is member access, not a scope root. + expect(unboundRootsInCelSource('record.status == "unqualified"')).toEqual([]); + expect(unboundRootsInCelSource('record.data.parent.previous != null')).toEqual([]); + expect(unboundRootsInCelSource('record.features.beta')).toEqual([]); + }); + + it('does not read identifier-shaped text inside string literals', () => { + // The load-bearing false-positive case: quoted prose mentioning a field. + expect(unboundRootsInCelSource('record.note == "status unqualified"')).toEqual([]); + expect(unboundRootsInCelSource("record.note == 'company == acme'")).toEqual([]); + expect(unboundRootsInCelSource('record.note == "it\'s status"')).toEqual([]); + // A literal is not a hiding place either way round: a real bare root + // beside a decoy literal is still reported, exactly once. + expect(unboundRootsInCelSource('status == "status"')).toEqual(['status']); + }); + + it('does not treat a call target as a scope root', () => { + expect(unboundRootsInCelSource('has(record.owner)')).toEqual([]); + expect(unboundRootsInCelSource('size(record.tags) > 0')).toEqual([]); + expect(unboundRootsInCelSource('has (record.owner)')).toEqual([]); + expect(unboundRootsInCelSource('int(record.amount) > 100')).toEqual([]); + // …but an unbound root INSIDE a call argument is still a fault-open root. + expect(unboundRootsInCelSource('has(status)')).toEqual(['status']); + }); + + it('declines to judge a comprehension macro at all (its variable is locally bound)', () => { + // `t` is bound by the macro. A tokenizer cannot tell that from an unbound + // root, so the whole predicate is skipped — silence over a wrong accusation. + expect(unboundRootsInCelSource("record.tags.exists(t, t == 'vip')")).toEqual([]); + expect(unboundRootsInCelSource('record.lines.all(l, l.qty > 0)')).toEqual([]); + expect(unboundRootsInCelSource('record.lines.map(l, l.qty).size() > 0')).toEqual([]); + expect(unboundRootsInCelSource('record.lines.filter(l, l.ok).size() > 0')).toEqual([]); + expect(unboundRootsInCelSource('record.tags.exists_one(t, t == 1)')).toEqual([]); + }); + + it('does not report CEL literals or reserved words as roots', () => { + expect(unboundRootsInCelSource('true')).toEqual([]); + expect(unboundRootsInCelSource('record.owner != null && true')).toEqual([]); + expect(unboundRootsInCelSource("'vip' in record.tags")).toEqual([]); + }); + + it('does not read a number as an identifier', () => { + expect(unboundRootsInCelSource('record.amount > 1e5')).toEqual([]); + expect(unboundRootsInCelSource('record.amount > 100')).toEqual([]); + }); + + it('reports each distinct unbound root once, in source order', () => { + expect(unboundRootsInCelSource('status == "x" && company != "" && status != "y"')) + .toEqual(['status', 'company']); + }); + + it('answers identically on a second call (no leaked regex state)', () => { + const source = 'status == "unqualified"'; + expect(unboundRootsInCelSource(source)).toEqual(unboundRootsInCelSource(source)); + }); +}); + +describe('detectUnboundFormViewPredicateRoots — traversal', () => { + it('reports the field predicate with its path, view identity and source', () => { + const findings = detectUnboundFormViewPredicateRoots( + definitionWithFieldPredicate(CEL('status == "unqualified"')), + ); + expect(findings).toEqual([ + { + path: 'views[0].form.sections[0].fields[1].visibleWhen', + view: 'crm_lead', + root: 'status', + source: 'status == "unqualified"', + }, + ]); + }); + + it('reports nothing for the same artifact spelled with the `record.` root', () => { + expect( + detectUnboundFormViewPredicateRoots( + definitionWithFieldPredicate(CEL('record.status == "unqualified"')), + ), + ).toEqual([]); + }); + + it('reads the bare-string shorthand and the deprecated `visibleOn` alias', () => { + // Both reach this scan because it runs BEFORE the parse that normalizes them. + expect(detectUnboundFormViewPredicateRoots( + definitionWithFieldPredicate('status == "unqualified"'), + )).toHaveLength(1); + + const withAlias: any = definitionWithFieldPredicate(CEL('record.x')); + const field = withAlias.views[0].form.sections[0].fields[1]; + delete field.visibleWhen; + field.visibleOn = CEL('status == "unqualified"'); + const findings = detectUnboundFormViewPredicateRoots(withAlias); + expect(findings).toHaveLength(1); + expect(findings[0]!.path).toBe('views[0].form.sections[0].fields[1].visibleOn'); + }); + + it('passes an opaque predicate: AST-only, and any non-CEL dialect', () => { + expect(detectUnboundFormViewPredicateRoots( + definitionWithFieldPredicate({ dialect: 'cel', ast: { kind: 'binary' } }), + )).toEqual([]); + expect(detectUnboundFormViewPredicateRoots( + definitionWithFieldPredicate({ dialect: 'template', source: 'status' }), + )).toEqual([]); + }); + + it('walks section predicates, the legacy `groups` bucket, and sub-fields at depth', () => { + const findings = detectUnboundFormViewPredicateRoots({ + views: [ + { + form: { + data: { object: 'crm_lead' }, + groups: [ + { + visibleWhen: CEL('stage == "closed"'), + fields: [ + { + field: 'lines', + type: 'repeater', + fields: [ + { field: 'note', visibleWhen: CEL('type == "formula"') }, + { field: 'ok', visibleWhen: CEL('data.type == "formula"') }, + ], + }, + ], + }, + ], + }, + }, + ], + }); + expect(findings.map((f) => f.path)).toEqual([ + 'views[0].form.groups[0].visibleWhen', + 'views[0].form.groups[0].fields[0].fields[0].visibleWhen', + ]); + expect(findings.map((f) => f.root)).toEqual(['stage', 'type']); + }); + + it('walks the keyed `formViews` map as well as the default `form` arm', () => { + const findings = detectUnboundFormViewPredicateRoots({ + views: [ + { + list: { data: { object: 'crm_lead' } }, + formViews: { + edit: { + data: { object: 'crm_lead' }, + sections: [{ fields: [{ field: 'a', visibleWhen: CEL('status == "x"') }] }], + }, + }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0]!.path).toBe('views[0].formViews.edit.sections[0].fields[0].visibleWhen'); + expect(findings[0]!.view).toBe('crm_lead'); + }); + + it('reads an independent form ViewItem, and skips a list ViewItem', () => { + const formItem = { + views: [ + { + name: 'crm_lead.edit', + object: 'crm_lead', + viewKind: 'form', + config: { sections: [{ fields: [{ field: 'a', visibleWhen: CEL('status == "x"') }] }] }, + }, + ], + }; + const findings = detectUnboundFormViewPredicateRoots(formItem); + expect(findings).toHaveLength(1); + expect(findings[0]!.view).toBe('crm_lead.edit'); + expect(findings[0]!.path).toBe('views[0].config.sections[0].fields[0].visibleWhen'); + + const listItem = { views: [{ ...formItem.views[0], viewKind: 'list' }] }; + expect(detectUnboundFormViewPredicateRoots(listItem)).toEqual([]); + }); + + it('is silent — never throwing — on shapes it cannot read', () => { + expect(detectUnboundFormViewPredicateRoots(undefined)).toEqual([]); + expect(detectUnboundFormViewPredicateRoots(null)).toEqual([]); + expect(detectUnboundFormViewPredicateRoots('not a definition')).toEqual([]); + expect(detectUnboundFormViewPredicateRoots([])).toEqual([]); + expect(detectUnboundFormViewPredicateRoots({ manifest: {} })).toEqual([]); + expect(detectUnboundFormViewPredicateRoots({ views: 'nope' })).toEqual([]); + expect(detectUnboundFormViewPredicateRoots({ views: [null, 7, 'x'] })).toEqual([]); + // Legacy bare-string field entries name a field and carry no predicate. + expect(detectUnboundFormViewPredicateRoots({ + views: [{ form: { sections: [{ fields: ['title', 'status', null] }] } }], + })).toEqual([]); + }); + + it('does not mutate the definition it reads', () => { + const definition = definitionWithFieldPredicate(CEL('status == "unqualified"')); + const before = JSON.stringify(definition); + detectUnboundFormViewPredicateRoots(definition); + expect(JSON.stringify(definition)).toBe(before); + }); +}); diff --git a/packages/metadata-core/src/form-predicate-root-policy.ts b/packages/metadata-core/src/form-predicate-root-policy.ts new file mode 100644 index 0000000000..0d828bb8f3 --- /dev/null +++ b/packages/metadata-core/src/form-predicate-root-policy.ts @@ -0,0 +1,331 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Detection policy for form-view predicates whose ROOT identifier is not bound + * on the surface that evaluates them (#12915 scope C). + * + * ## The degradation this makes audible + * + * A form-view predicate binds a fixed scope: `record` (plus `previous`, the + * saved record, and `parent` for master-detail line items) in runtime record + * forms, and `data` — the row under edit, at every depth, repeater rows + * included — in metadata-editing forms. The contract states the failure mode + * beside the vocabulary (`packages/spec/src/ui/view.zod.ts`, + * `FormFieldSchema.visibleWhen` / `FormSectionSchema.visibleWhen`): **a bare + * identifier is UNBOUND, the predicate faults, and `visibleWhen`'s fault + * fallback is `true`** — so a field the predicate was authored to hide renders + * for everyone. + * + * That is quiet on its own, and lethal in combination with the authoring + * pattern it exists to serve. Measured on a real deployment: an artifact built + * 2026-08-05 by released `@objectstack/cli` 17.1.0 authors + * `{ field: 'disqualification_reason', required: true, visibleWhen: + * { dialect: 'cel', source: 'status == "unqualified"' } }` — the era's working + * spelling. On a 17.2 runtime the predicate faults open, the conditionally + * hidden field renders, and its unconditional `required: true` (authored to be + * gated by that visibility) blocks **every** record creation through the + * console. The same payload POSTed to the REST door returns 201: server-side + * validation is correct, and only the already-built artifact degrades. Nothing + * refuses, nothing logs, and the operator — the one person who can rebuild the + * artifact — has no signal at all. + * + * ## What this module is, and deliberately is not + * + * It is a **detector**: given an artifact's stack definition, report every + * form-view predicate naming an unbound root. Its caller turns that into one + * deduped operator-facing boot line at the artifact door. + * + * - **Not a rewriter.** Prefixing a bare root with `record.` is the ADR-0087 + * conversion (#12915 scope A), which needs a field-name-aware guard and is + * deferred by the maintainer ruling of 2026-08-28 (「同意C」) with an + * explicit start line. Nothing here mutates the definition. + * - **Not a refusal.** No parse changes, no gate, no behaviour change. A + * faulting predicate keeps faulting open exactly as it does today; the only + * difference is that the operator is told. + * - **Not a validator of the current surface.** Its caller applies it only + * inside the versioned window `applyArtifactForwardConversions` already + * opens (declared floor below the running spec, or undeclared). An artifact + * authored against the current surface answers to the strict parse and gets + * nothing from here — which is what keeps a *notice about legacy artifacts* + * out of contract territory. + * + * ## Precision: prefer missing an exotic predicate over crying wolf + * + * A notice that fires on a healthy, current, correctly-authored artifact is + * worse than no notice, because the next one is ignored. Every judgement call + * below therefore resolves toward silence: + * + * - **String literals are stripped before the scan**, so `record.note == + * "status unqualified"` cannot false-positive on identifier-shaped text + * inside quotes. + * - **Only ROOT position counts.** An identifier preceded by `.` is a member + * access, so `record.status`, `record.data`, and `record.parent.x` name one + * root (`record`) and nothing else. + * - **A name followed by `(` is a call, not a scope root** — `has(record.x)`, + * `size(record.tags)` and every other CEL builtin drop out without needing + * to be enumerated. + * - **A comprehension macro makes the whole predicate opaque and it is + * skipped.** `record.tags.exists(t, t == 'x')` binds `t` locally, and a + * tokenizer cannot tell that from an unbound root — so predicates + * containing `.all(` / `.exists(` / `.exists_one(` / `.map(` / `.filter(` + * are not judged at all. + * - **An AST-only envelope passes.** `{ dialect: 'cel', ast }` with no + * `source` is opaque at this layer — the same posture the spec's own + * `features.*` root scanner takes. + * - **Per-option `visibleWhen` is out of scope**, deliberately: options are + * evaluated by a *different* evaluator (`resolveCascadingOptions`, ADR-0068) + * which binds `current_user` as well, so this vocabulary would be the wrong + * yardstick there. + * + * A tokenizer rather than a CEL parse is the established shape for exactly + * this question in this codebase: the spec's own enforced + * `checkFormViewPredicateFeaturesRoot` scans the source string the same way, + * for the same reason (parse time sees the source, and a detector with no + * dependencies cannot itself fail to resolve). A real CEL parse lives in + * `@objectstack/formula`, which this package cannot reach — `metadata-core` + * depends on `@objectstack/spec` and nothing else, and spec exposes no parse. + */ + +/** + * The identifiers a form-view predicate may name in ROOT position. + * + * Sourced from the contract prose on `FormFieldSchema.visibleWhen` and + * `FormSectionSchema.visibleWhen` (`packages/spec/src/ui/view.zod.ts`): + * `record` + `previous` + `parent` in runtime record forms, `data` in + * metadata-editing forms (and inside a repeater, where `data` is the ROW but + * is still spelled `data`). The union of both surfaces is used because an + * artifact's `views` collection carries both kinds and the definition does not + * say which renderer will read a given form — the union is the direction that + * stays silent on a healthy artifact. + * + * ⚠️ `current_user` is deliberately ABSENT: the contract states it is unbound + * at field and section level and that such a predicate faults open. It is + * bound only for per-option `visibleWhen`, which this scan does not visit. + */ +export const BOUND_FORM_VIEW_PREDICATE_ROOTS: readonly string[] = [ + 'record', + 'previous', + 'parent', + 'data', +]; + +/** One form-view predicate naming a root that is not bound where it evaluates. */ +export interface UnboundFormPredicateRoot { + /** Dotted path into the stack definition, e.g. `views[3].form.sections[0].fields[2].visibleWhen`. */ + path: string; + /** Operator-legible identity of the view carrying it (view name, else its object). */ + view: string; + /** The root identifier that is not bound on this surface. */ + root: string; + /** The predicate's CEL source, verbatim. */ + source: string; +} + +/** CEL string literals (both quote styles, with escapes) — stripped before the root scan. */ +const CEL_STRING_LITERAL_RE = /'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"/g; + +/** + * A comprehension macro binds its own iteration variable, which is + * indistinguishable from an unbound root to a tokenizer. Predicates containing + * one are skipped whole rather than guessed at. + */ +const CEL_COMPREHENSION_MACRO_RE = /\.\s*(?:all|exists|exists_one|map|filter)\s*\(/; + +/** CEL literals and reserved words that can stand in root position without naming a scope. */ +const CEL_RESERVED_WORDS: ReadonlySet = new Set([ + 'true', 'false', 'null', 'in', + // The CEL reserved-word list — none may be an identifier, so none is a root. + 'as', 'break', 'const', 'continue', 'else', 'for', 'function', 'if', 'import', + 'let', 'loop', 'package', 'namespace', 'return', 'var', 'void', 'while', +]); + +/** The predicate keys a form view carries. `visibleOn` is the ADR-0089 alias, + * folded onto `visibleWhen` AT PARSE — so a raw artifact, which reaches this + * scan before any parse, can still spell either. */ +const FORM_PREDICATE_KEYS: readonly string[] = ['visibleWhen', 'visibleOn']; + +/** + * Root identifiers named by one CEL source, minus every bound root, reserved + * word and call target. Order-preserving and de-duplicated. + * + * Exported for the pinned false-positive cases in this module's test — the + * traversal below is the product surface, this is the judgement under it. + */ +export function unboundRootsInCelSource(source: string): string[] { + const stripped = source.replace(CEL_STRING_LITERAL_RE, ''); + if (CEL_COMPREHENSION_MACRO_RE.test(stripped)) return []; + + // Built per call rather than shared at module scope: a `g`-flagged literal + // carries `lastIndex` between calls, and a detector that answers differently + // on its second invocation is the exact class of bug this module exists to + // report. + const rootIdentifier = /(?:^|[^.\w$])([A-Za-z_$][\w$]*)/g; + const roots: string[] = []; + const seen = new Set(); + let match: RegExpExecArray | null; + while ((match = rootIdentifier.exec(stripped)) !== null) { + const identifier = match[1]!; + // Peek past whitespace: a name applied to an argument list is a call. + let after = match.index + match[0].length; + while (after < stripped.length && /\s/.test(stripped[after]!)) after += 1; + if (stripped[after] === '(') continue; + if (CEL_RESERVED_WORDS.has(identifier)) continue; + if (BOUND_FORM_VIEW_PREDICATE_ROOTS.includes(identifier)) continue; + if (seen.has(identifier)) continue; + seen.add(identifier); + roots.push(identifier); + } + return roots; +} + +/** The CEL source of one predicate slot, or `null` when there is nothing to judge. */ +function readCelSource(predicate: unknown): string | null { + // Bare-string shorthand for `{ dialect: 'cel', source }` — normalized away by + // `objectstack compile`, so a built artifact should not carry it; read it + // anyway, because a hand-written or third-party artifact may. + if (typeof predicate === 'string') return predicate.trim() === '' ? null : predicate; + if (predicate === null || typeof predicate !== 'object' || Array.isArray(predicate)) return null; + const { dialect, source } = predicate as { dialect?: unknown; source?: unknown }; + if (dialect !== 'cel') return null; + if (typeof source !== 'string' || source.trim() === '') return null; + return source; +} + +function isPlainObject(value: unknown): value is Record { + return value !== null && typeof value === 'object' && !Array.isArray(value); +} + +function scanPredicateSlot( + predicate: unknown, + path: string, + view: string, + out: UnboundFormPredicateRoot[], +): void { + const source = readCelSource(predicate); + if (source === null) return; + for (const root of unboundRootsInCelSource(source)) { + out.push({ path, view, root, source }); + } +} + +/** + * One form-view field entry, and its sub-fields at any depth (composite / + * repeater / record types nest). A legacy bare-string entry (`'title'`) names + * a field and carries no predicate. + */ +function scanFormField( + field: unknown, + path: string, + view: string, + out: UnboundFormPredicateRoot[], +): void { + if (!isPlainObject(field)) return; + for (const key of FORM_PREDICATE_KEYS) { + scanPredicateSlot(field[key], `${path}.${key}`, view, out); + } + const subFields = field.fields; + if (Array.isArray(subFields)) { + subFields.forEach((sub, index) => scanFormField(sub, `${path}.fields[${index}]`, view, out)); + } +} + +function scanFormSection( + section: unknown, + path: string, + view: string, + out: UnboundFormPredicateRoot[], +): void { + if (!isPlainObject(section)) return; + for (const key of FORM_PREDICATE_KEYS) { + scanPredicateSlot(section[key], `${path}.${key}`, view, out); + } + const fields = section.fields; + if (Array.isArray(fields)) { + fields.forEach((field, index) => scanFormField(field, `${path}.fields[${index}]`, view, out)); + } +} + +/** + * One form view. Sections live under `sections`, or under the legacy `groups` + * alias that the parse folds onto `sections` — both are read here for the same + * reason `visibleOn` is: this scan runs BEFORE the parse. + */ +function scanFormView( + formView: unknown, + path: string, + view: string, + out: UnboundFormPredicateRoot[], +): void { + if (!isPlainObject(formView)) return; + for (const bucket of ['sections', 'groups'] as const) { + const sections = formView[bucket]; + if (!Array.isArray(sections)) continue; + sections.forEach((section, index) => + scanFormSection(section, `${path}.${bucket}[${index}]`, view, out), + ); + } +} + +/** The object a view arm targets (`.data.object`), when it declares one. */ +function readArmObject(arm: unknown): string | undefined { + if (!isPlainObject(arm)) return undefined; + const data = arm.data; + if (!isPlainObject(data)) return undefined; + const object = data.object; + return typeof object === 'string' && object !== '' ? object : undefined; +} + +/** Operator-legible identity for one `views[]` entry. */ +function viewLabel(entry: Record, index: number): string { + const name = entry.name; + if (typeof name === 'string' && name !== '') return name; + const object = entry.object; + if (typeof object === 'string' && object !== '') return object; + return readArmObject(entry.form) ?? readArmObject(entry.list) ?? `views[${index}]`; +} + +/** + * Every form-view predicate in one stack definition whose root identifier is + * not bound where it evaluates. + * + * `definition` is the **stack definition** — the shape with `manifest`, + * `objects`, `views`, … at the top; for an environment-artifact envelope, pass + * the envelope's `metadata` block. Same input as + * `applyArtifactForwardConversions`, so a door can hand both the same value. + * + * Pure and read-only: never throws, never mutates, never gates. An empty array + * means "nothing to say", which is also the answer for a definition this + * cannot read — silence is the safe direction (see the module doc). + * + * Both `views[]` shapes are visited: the aggregated container + * (`{ list, form, listViews, formViews }`, identified by the ABSENCE of + * `viewKind`) and the independent ViewItem (`{ name, object, viewKind, config }`). + * A ViewItem whose `viewKind` is not `'form'` carries no form-view predicate + * and is skipped. + */ +export function detectUnboundFormViewPredicateRoots(definition: unknown): UnboundFormPredicateRoot[] { + const out: UnboundFormPredicateRoot[] = []; + if (!isPlainObject(definition)) return out; + const views = definition.views; + if (!Array.isArray(views)) return out; + + views.forEach((entry, index) => { + if (!isPlainObject(entry)) return; + const label = viewLabel(entry, index); + + if (entry.viewKind !== undefined) { + if (entry.viewKind !== 'form') return; + scanFormView(entry.config, `views[${index}].config`, label, out); + return; + } + + scanFormView(entry.form, `views[${index}].form`, label, out); + const formViews = entry.formViews; + if (!isPlainObject(formViews)) return; + for (const [key, formView] of Object.entries(formViews)) { + scanFormView(formView, `views[${index}].formViews.${key}`, label, out); + } + }); + + return out; +} diff --git a/packages/metadata-core/src/index.ts b/packages/metadata-core/src/index.ts index f68f18927a..d1ada19a6c 100644 --- a/packages/metadata-core/src/index.ts +++ b/packages/metadata-core/src/index.ts @@ -19,6 +19,11 @@ export * from './protocol-handshake.js'; // an artifact whose declared `engines.protocol` floor predates the running // spec, so within-line key retirements do not brick already-built artifacts. export * from './artifact-forward-conversion.js'; +// #12915 scope C — the read-only sibling of the conversion above: report +// form-view predicates whose ROOT identifier is unbound (and therefore fault +// OPEN) on artifacts inside the same versioned window. Detection only; the +// rewriting conversion is scope A, deferred by the 2026-08-28 ruling. +export * from './form-predicate-root-policy.js'; export * from './objects/index.js'; // [#5619] The ObjectQL WRITE-VERB dispatch predicates (#4550 delete / #5480 diff --git a/packages/metadata/src/__fixtures__/hotcrm-17.1-built-bare-root-predicates.artifact.json b/packages/metadata/src/__fixtures__/hotcrm-17.1-built-bare-root-predicates.artifact.json new file mode 100644 index 0000000000..d7a3ff5c3d --- /dev/null +++ b/packages/metadata/src/__fixtures__/hotcrm-17.1-built-bare-root-predicates.artifact.json @@ -0,0 +1,85 @@ +{ + "manifest": { + "id": "app.objectstack.hotcrm", + "namespace": "crm", + "defaultDatasource": "default", + "version": "2.2.2", + "type": "app", + "scope": "project", + "name": "HotCRM", + "description": "Era fixture for #12915 — the lead form view as an artifact built by released @objectstack/cli 17.1.0 spelled it: visibility predicates rooted at a BARE field identifier, gating fields that are unconditionally required.", + "engines": { + "protocol": "^17.0.0-rc.1" + } + }, + "views": [ + { + "list": { + "label": "All Leads", + "type": "grid", + "data": { "provider": "object", "object": "crm_lead" }, + "columns": [ + { "field": "name" }, + { "field": "company" }, + { "field": "status" } + ] + }, + "form": { + "type": "simple", + "data": { "provider": "object", "object": "crm_lead" }, + "sections": [ + { + "name": "lead", + "label": "Lead", + "columns": 2, + "fields": [ + { "field": "name", "required": true }, + { "field": "company", "required": true }, + { "field": "email", "required": true }, + { "field": "status" }, + { + "field": "disqualification_reason", + "required": true, + "visibleWhen": { "dialect": "cel", "source": "status == \"unqualified\"" } + }, + { + "field": "duplicate_of_type", + "required": true, + "visibleWhen": { "dialect": "cel", "source": "status == \"duplicate\"" } + }, + { + "field": "duplicate_of_lead", + "required": true, + "visibleWhen": { "dialect": "cel", "source": "duplicate_of_type == \"lead\"" } + }, + { + "field": "note", + "visibleWhen": { "dialect": "cel", "source": "record.note != \"status unqualified\"" } + } + ] + } + ] + } + }, + { + "form": { + "type": "simple", + "data": { "provider": "object", "object": "crm_case" }, + "sections": [ + { + "name": "case", + "label": "Case", + "fields": [ + { "field": "subject", "required": true }, + { + "field": "resolution", + "required": true, + "visibleWhen": { "dialect": "cel", "source": "record.status == \"closed\"" } + } + ] + } + ] + } + } + ] +} diff --git a/packages/metadata/src/plugin-unbound-form-predicate-roots.test.ts b/packages/metadata/src/plugin-unbound-form-predicate-roots.test.ts new file mode 100644 index 0000000000..8908a75cba --- /dev/null +++ b/packages/metadata/src/plugin-unbound-form-predicate-roots.test.ts @@ -0,0 +1,203 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The artifact-ingestion door warns — once per artifact, operator-visibly — + * when a pre-current-era artifact carries form-view predicates whose ROOT + * identifier is unbound and therefore faults OPEN (#12915 scope C, maintainer + * ruling 2026-08-28 「同意C」). + * + * `__fixtures__/hotcrm-17.1-built-bare-root-predicates.artifact.json` reproduces + * the measured shape: a lead form view whose `disqualification_reason` / + * `duplicate_of_type` / `duplicate_of_lead` entries are unconditionally + * `required: true` and gated by `visibleWhen` predicates rooted at a BARE field + * identifier — the 17.1 era's working spelling. On a 17.2 runtime each faults, + * visibility fails open, the fields render, and their `required: true` + * dead-ends console record creation while the same payload POSTs 201 through + * REST. Nothing refused and nothing logged; this suite pins the log. + * + * The suite is deliberately weighted toward the SILENT directions — a notice + * that fires on a healthy or current artifact trains operators to ignore the + * channel, which costs more than the missed warning it was meant to prevent. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { resolveInstalledSpecVersion } from '@objectstack/metadata-core'; +import { MetadataPlugin } from './plugin.js'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const FIXTURE_PATH = join(HERE, '__fixtures__/hotcrm-17.1-built-bare-root-predicates.artifact.json'); + +/** Fresh parse per test — `_parseAndRegisterArtifact` mutates items in place. */ +function loadFixture(): any { + return JSON.parse(readFileSync(FIXTURE_PATH, 'utf8')); +} + +function fakeCtx() { + return { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: vi.fn(), + getService: vi.fn(() => undefined), + trigger: vi.fn(), + } as any; +} + +function newPlugin(): any { + return new MetadataPlugin({ watch: false, config: { bootstrap: 'lazy' } }); +} + +/** Just the notices this feature emits — never the #12772 conversion summaries. */ +function unboundRootWarnings(ctx: any): string[] { + return (ctx.logger.warn.mock.calls as any[]) + .map((call) => String(call[0])) + .filter((message) => message.includes('root identifier is NOT bound')); +} + +describe('artifact door — unbound form-predicate roots are announced to the operator (#12915)', () => { + it('the fixture carries the incident shape (premise guard)', () => { + const fixture = loadFixture(); + // If a later edit ever launders these away, every assertion below stops + // testing the incident while staying green. + expect(fixture.manifest.engines.protocol).toBe('^17.0.0-rc.1'); + const leadFields = fixture.views[0].form.sections[0].fields; + const gated = leadFields.filter( + (f: any) => f?.required === true && f?.visibleWhen?.dialect === 'cel', + ); + expect(gated.map((f: any) => f.field)).toEqual([ + 'disqualification_reason', + 'duplicate_of_type', + 'duplicate_of_lead', + ]); + // Bare roots: no `record.` prefix anywhere in those three sources. + for (const field of gated) expect(field.visibleWhen.source).not.toContain('record.'); + // …and the fixture also carries the two SILENT controls: a `record.`-rooted + // predicate whose string literal contains identifier-shaped text, and a + // second view that is entirely healthy. + expect(leadFields.at(-1).visibleWhen.source).toBe('record.note != "status unqualified"'); + expect(fixture.views[1].form.sections[0].fields[1].visibleWhen.source) + .toBe('record.status == "closed"'); + }); + + it('emits ONE notice naming the affected view, the version evidence and the rebuild remedy', async () => { + const plugin = newPlugin(); + const ctx = fakeCtx(); + + await plugin._parseAndRegisterArtifact(ctx, loadFixture(), 'fixture-bare-roots'); + + const warnings = unboundRootWarnings(ctx); + expect(warnings).toHaveLength(1); + const notice = warnings[0]!; + + // Version evidence — the authored floor and the runtime it is below. + expect(notice).toContain('fixture-bare-roots'); + expect(notice).toContain('17.0.0'); + expect(notice).toContain(String(resolveInstalledSpecVersion())); + // Volume, the roots seen, and the bound vocabulary to compare against. + expect(notice).toContain('3 form-view predicate(s)'); + expect(notice).toContain("'status'"); + expect(notice).toContain("'duplicate_of_type'"); + expect(notice).toContain("'record', 'previous', 'parent', 'data'"); + // Which view, with the first path as the anchor. + expect(notice).toContain('1 view(s): crm_lead'); + expect(notice).toContain('views[0].form.sections[0].fields[4].visibleWhen'); + // The consequence in one clause, and the prescription. + expect(notice).toContain('fails OPEN'); + expect(notice).toContain("'os build'"); + // The healthy sibling view is NOT named. + expect(notice).not.toContain('crm_case'); + }); + + it('does not repeat on re-ingestion — the HMR watcher replays the same artifact', async () => { + const plugin = newPlugin(); + const ctx = fakeCtx(); + + await plugin._parseAndRegisterArtifact(ctx, loadFixture(), 'fixture-bare-roots'); + await plugin._parseAndRegisterArtifact(ctx, loadFixture(), 'fixture-bare-roots'); + await plugin._parseAndRegisterArtifact(ctx, loadFixture(), 'fixture-bare-roots'); + + expect(unboundRootWarnings(ctx)).toHaveLength(1); + }); + + it('says NOTHING about an artifact authored against the current surface', async () => { + // The boundary that keeps this out of contract territory: the notice + // lives inside the versioned window #12772 built, so an artifact + // declaring the current floor gets zero notices even carrying the very + // same bare-root predicates. Derived from the installed spec rather than + // hardcoded, so the pin cannot rot into vacuity on the next spec bump. + const current = resolveInstalledSpecVersion(); + expect(current, 'spec version must resolve for this pin to mean anything').toBeTruthy(); + + const fixture = loadFixture(); + fixture.manifest.engines.protocol = `^${current}`; + + const plugin = newPlugin(); + const ctx = fakeCtx(); + await plugin._parseAndRegisterArtifact(ctx, fixture, 'fixture-current-floor'); + + expect(unboundRootWarnings(ctx)).toEqual([]); + }); + + it('says NOTHING about an old artifact whose predicates all carry a bound root', async () => { + const fixture = loadFixture(); + for (const field of fixture.views[0].form.sections[0].fields) { + if (field.visibleWhen) field.visibleWhen.source = `record.${field.visibleWhen.source}`; + } + // Still old, still gated-required — only the root is now bound. + expect(fixture.manifest.engines.protocol).toBe('^17.0.0-rc.1'); + + const plugin = newPlugin(); + const ctx = fakeCtx(); + await plugin._parseAndRegisterArtifact(ctx, fixture, 'fixture-record-rooted'); + + expect(unboundRootWarnings(ctx)).toEqual([]); + }); + + it('does not cry wolf on identifier-shaped text inside a string literal', async () => { + // Reduced to the single control so a failure here reads as "the literal + // stripping broke", not "something in the fixture changed". + const fixture = loadFixture(); + fixture.views = [ + { + form: { + type: 'simple', + data: { provider: 'object', object: 'crm_lead' }, + sections: [{ + name: 'lead', + fields: [{ + field: 'note', + visibleWhen: { dialect: 'cel', source: 'record.note == "status unqualified"' }, + }], + }], + }, + }, + ]; + + const plugin = newPlugin(); + const ctx = fakeCtx(); + await plugin._parseAndRegisterArtifact(ctx, fixture, 'fixture-literal-control'); + + expect(unboundRootWarnings(ctx)).toEqual([]); + }); + + it('changes NO behaviour: the artifact still registers and its predicates are untouched', async () => { + const plugin = newPlugin(); + const ctx = fakeCtx(); + const pristine = loadFixture(); + + const total = await plugin._parseAndRegisterArtifact(ctx, loadFixture(), 'fixture-bare-roots'); + expect(total).toBeGreaterThan(0); + + // No refusal (the call above would have thrown) and no rewrite: the + // registered view carries the authored source verbatim, bare root and + // all. Rewriting is #12915 scope A, deferred by the same ruling. + const registered: any = await plugin.manager.get('view', 'crm_lead'); + expect(registered).toBeDefined(); + const authored = pristine.views[0].form.sections[0].fields; + const stored = registered.form.sections[0].fields; + for (const [index, field] of authored.entries()) { + expect(stored[index].visibleWhen?.source).toBe(field.visibleWhen?.source); + } + }); +}); diff --git a/packages/metadata/src/plugin.ts b/packages/metadata/src/plugin.ts index 6c74ef4633..469d2ddd39 100644 --- a/packages/metadata/src/plugin.ts +++ b/packages/metadata/src/plugin.ts @@ -15,6 +15,9 @@ import { SysMetadataAuditObject, SysViewDefinitionObject, applyArtifactForwardConversions, + detectUnboundFormViewPredicateRoots, + BOUND_FORM_VIEW_PREDICATE_ROOTS, + type ArtifactForwardConversionResult, } from '@objectstack/metadata-core'; // `SysMetadataObject` + `SysMetadataHistoryObject` are the customer overlay @@ -256,12 +259,18 @@ export class MetadataPlugin implements Plugin { private lastParsedMetadata?: Record; /** - * Once-per-process dedupe for artifact forward-conversion summaries - * (`conversionId|label`, #12772). The artifact watcher replays - * `_parseAndRegisterArtifact` on every file change, so without this a dev - * loop over a legacy artifact would re-announce the same conversions on - * every reload — the same shape `Protocol.storedConversionWarned` guards - * on the stored-row pass, which this surfacing is modeled on. + * Once-per-process dedupe for the summaries the versioned artifact window + * emits. The artifact watcher replays `_parseAndRegisterArtifact` on every + * file change, so without this a dev loop over a legacy artifact would + * re-announce the same finding on every reload — the same shape + * `Protocol.storedConversionWarned` guards on the stored-row pass, which + * this surfacing is modeled on. + * + * Two key families share the set, because they share the replay: + * `|