Skip to content

Commit b745157

Browse files
authored
feat(metadata-core,metadata): warn when a pre-current-era artifact carries fault-open form-view predicates (#12989)
* wip: unbound form-predicate-root detection policy + door wiring * wip: tests + era fixture for the unbound-root boot notice * wip: fixture view data needs an explicit provider * wip: changeset
1 parent d028b37 commit b745157

7 files changed

Lines changed: 1029 additions & 6 deletions
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@objectstack/metadata-core": patch
3+
"@objectstack/metadata": patch
4+
---
5+
6+
feat(metadata-core,metadata): warn the operator when a pre-current-era artifact carries form-view predicates that fault open (#12915)
7+
8+
A form-view predicate binds `record` (+ `previous`, `parent`) in runtime record
9+
forms, or `data` in metadata-editing forms. The contract states the failure mode
10+
beside the vocabulary: **a bare identifier is unbound, the predicate faults, and
11+
`visibleWhen`'s fault fallback is `true`** — so a field the predicate was
12+
authored to hide renders for everyone.
13+
14+
That is quiet alone and lethal in combination with the authoring pattern it
15+
serves. Measured on a real deployment: an artifact built by released
16+
`@objectstack/cli` 17.1.0 authors
17+
`{ field: 'disqualification_reason', required: true, visibleWhen: 'status == "unqualified"' }`
18+
— the era's working spelling. On a 17.2 runtime the predicate faults open, the
19+
conditionally hidden field renders, and its unconditional `required: true`
20+
blocks **every** record creation through the console, while the same payload
21+
POSTs 201 through REST. Nothing refused and nothing logged, so the operator —
22+
the only person who can rebuild the artifact — had no signal at all.
23+
24+
The framework artifact door now emits **one deduped `warn` line per artifact**
25+
naming the authored `engines.protocol` floor and the runtime spec version, how
26+
many predicates on which views (with the first path as an anchor), the
27+
fault-open consequence, and the remedy (`os build`). It rides the same funnel
28+
that already carries the forward-conversion summaries, so both SaaS shapes are
29+
covered: a single-DB multi-org runtime warns once at boot, and per-tenant-DB
30+
kernels each warn at their own.
31+
32+
**No behaviour change.** No refusal, no rewrite, no schema or contract edit —
33+
the predicate keeps faulting open exactly as before, and the artifact bytes are
34+
untouched. Rewriting a bare root to `record.` is a separate, deferred ADR-0087
35+
conversion.
36+
37+
**Scoped to legacy artifacts by construction.** The notice fires only inside the
38+
versioned window the forward conversion already opens (declared floor below the
39+
running spec, or undeclared), read off that pass's own verdict rather than
40+
recomputed. An artifact declaring the current or a newer floor gets zero notices
41+
from this feature even when it carries bare roots — the boundary that keeps a
42+
notice about legacy artifacts out of contract territory.
43+
44+
Detection is exported from `@objectstack/metadata-core` as
45+
`detectUnboundFormViewPredicateRoots` (with `BOUND_FORM_VIEW_PREDICATE_ROOTS`)
46+
so other composed artifact doors can reuse one policy rather than fork it. It is
47+
pure, read-only, and tuned to prefer silence over a false accusation: string
48+
literals are stripped before the scan, only root position counts, call targets
49+
are not roots, comprehension macros (whose iteration variable is locally bound)
50+
are skipped whole, and AST-only envelopes pass.
Lines changed: 269 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,269 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Unit pins for the unbound form-view predicate root detector (#12915 scope C).
5+
*
6+
* The detector's product requirement is asymmetric, and so is this suite: a
7+
* MISSED exotic predicate costs one un-warned artifact, while a FALSE POSITIVE
8+
* on a healthy current artifact trains operators to ignore the channel. So the
9+
* negative cases below (string literals, calls, member access, comprehension
10+
* macros, every bound root) carry as much weight as the positive one, and each
11+
* exists because a naive "identifier not in the vocabulary" scan gets it wrong.
12+
*/
13+
14+
import { describe, it, expect } from 'vitest';
15+
import {
16+
BOUND_FORM_VIEW_PREDICATE_ROOTS,
17+
detectUnboundFormViewPredicateRoots,
18+
unboundRootsInCelSource,
19+
} from './form-predicate-root-policy.js';
20+
21+
/** The repro artifact's shape: one form view, one section, one gated field. */
22+
function definitionWithFieldPredicate(predicate: unknown, object = 'crm_lead'): unknown {
23+
return {
24+
manifest: { id: 'app.test', engines: { protocol: '^17.0.0-rc.1' } },
25+
views: [
26+
{
27+
form: {
28+
type: 'simple',
29+
data: { object },
30+
sections: [
31+
{
32+
name: 'main',
33+
fields: [
34+
{ field: 'name', required: true },
35+
{ field: 'disqualification_reason', required: true, visibleWhen: predicate },
36+
],
37+
},
38+
],
39+
},
40+
},
41+
],
42+
};
43+
}
44+
45+
const CEL = (source: string) => ({ dialect: 'cel', source });
46+
47+
describe('the bound vocabulary comes from the contract, not from this module', () => {
48+
it('is exactly record / previous / parent / data', () => {
49+
// `packages/spec/src/ui/view.zod.ts`, `FormFieldSchema.visibleWhen` and
50+
// `FormSectionSchema.visibleWhen`: "Root: `record` (+ `previous`,
51+
// `parent`) in runtime forms, or `data` in metadata forms."
52+
expect([...BOUND_FORM_VIEW_PREDICATE_ROOTS]).toEqual(['record', 'previous', 'parent', 'data']);
53+
});
54+
55+
it('excludes `current_user`, which the same prose calls unbound at field level', () => {
56+
expect(BOUND_FORM_VIEW_PREDICATE_ROOTS).not.toContain('current_user');
57+
expect(unboundRootsInCelSource("current_user.id == record.owner")).toEqual(['current_user']);
58+
});
59+
});
60+
61+
describe('unboundRootsInCelSource — the judgement under the traversal', () => {
62+
it('flags the era spelling from the real incident', () => {
63+
expect(unboundRootsInCelSource('status == "unqualified"')).toEqual(['status']);
64+
});
65+
66+
it('says nothing about a predicate rooted at any bound identifier', () => {
67+
for (const root of BOUND_FORM_VIEW_PREDICATE_ROOTS) {
68+
expect(unboundRootsInCelSource(`${root}.status == "unqualified"`), root).toEqual([]);
69+
}
70+
});
71+
72+
it('does not mistake a MEMBER named like a root for a root', () => {
73+
// A record field that happens to be called `status`, `data` or `features`
74+
// is member access, not a scope root.
75+
expect(unboundRootsInCelSource('record.status == "unqualified"')).toEqual([]);
76+
expect(unboundRootsInCelSource('record.data.parent.previous != null')).toEqual([]);
77+
expect(unboundRootsInCelSource('record.features.beta')).toEqual([]);
78+
});
79+
80+
it('does not read identifier-shaped text inside string literals', () => {
81+
// The load-bearing false-positive case: quoted prose mentioning a field.
82+
expect(unboundRootsInCelSource('record.note == "status unqualified"')).toEqual([]);
83+
expect(unboundRootsInCelSource("record.note == 'company == acme'")).toEqual([]);
84+
expect(unboundRootsInCelSource('record.note == "it\'s status"')).toEqual([]);
85+
// A literal is not a hiding place either way round: a real bare root
86+
// beside a decoy literal is still reported, exactly once.
87+
expect(unboundRootsInCelSource('status == "status"')).toEqual(['status']);
88+
});
89+
90+
it('does not treat a call target as a scope root', () => {
91+
expect(unboundRootsInCelSource('has(record.owner)')).toEqual([]);
92+
expect(unboundRootsInCelSource('size(record.tags) > 0')).toEqual([]);
93+
expect(unboundRootsInCelSource('has (record.owner)')).toEqual([]);
94+
expect(unboundRootsInCelSource('int(record.amount) > 100')).toEqual([]);
95+
// …but an unbound root INSIDE a call argument is still a fault-open root.
96+
expect(unboundRootsInCelSource('has(status)')).toEqual(['status']);
97+
});
98+
99+
it('declines to judge a comprehension macro at all (its variable is locally bound)', () => {
100+
// `t` is bound by the macro. A tokenizer cannot tell that from an unbound
101+
// root, so the whole predicate is skipped — silence over a wrong accusation.
102+
expect(unboundRootsInCelSource("record.tags.exists(t, t == 'vip')")).toEqual([]);
103+
expect(unboundRootsInCelSource('record.lines.all(l, l.qty > 0)')).toEqual([]);
104+
expect(unboundRootsInCelSource('record.lines.map(l, l.qty).size() > 0')).toEqual([]);
105+
expect(unboundRootsInCelSource('record.lines.filter(l, l.ok).size() > 0')).toEqual([]);
106+
expect(unboundRootsInCelSource('record.tags.exists_one(t, t == 1)')).toEqual([]);
107+
});
108+
109+
it('does not report CEL literals or reserved words as roots', () => {
110+
expect(unboundRootsInCelSource('true')).toEqual([]);
111+
expect(unboundRootsInCelSource('record.owner != null && true')).toEqual([]);
112+
expect(unboundRootsInCelSource("'vip' in record.tags")).toEqual([]);
113+
});
114+
115+
it('does not read a number as an identifier', () => {
116+
expect(unboundRootsInCelSource('record.amount > 1e5')).toEqual([]);
117+
expect(unboundRootsInCelSource('record.amount > 100')).toEqual([]);
118+
});
119+
120+
it('reports each distinct unbound root once, in source order', () => {
121+
expect(unboundRootsInCelSource('status == "x" && company != "" && status != "y"'))
122+
.toEqual(['status', 'company']);
123+
});
124+
125+
it('answers identically on a second call (no leaked regex state)', () => {
126+
const source = 'status == "unqualified"';
127+
expect(unboundRootsInCelSource(source)).toEqual(unboundRootsInCelSource(source));
128+
});
129+
});
130+
131+
describe('detectUnboundFormViewPredicateRoots — traversal', () => {
132+
it('reports the field predicate with its path, view identity and source', () => {
133+
const findings = detectUnboundFormViewPredicateRoots(
134+
definitionWithFieldPredicate(CEL('status == "unqualified"')),
135+
);
136+
expect(findings).toEqual([
137+
{
138+
path: 'views[0].form.sections[0].fields[1].visibleWhen',
139+
view: 'crm_lead',
140+
root: 'status',
141+
source: 'status == "unqualified"',
142+
},
143+
]);
144+
});
145+
146+
it('reports nothing for the same artifact spelled with the `record.` root', () => {
147+
expect(
148+
detectUnboundFormViewPredicateRoots(
149+
definitionWithFieldPredicate(CEL('record.status == "unqualified"')),
150+
),
151+
).toEqual([]);
152+
});
153+
154+
it('reads the bare-string shorthand and the deprecated `visibleOn` alias', () => {
155+
// Both reach this scan because it runs BEFORE the parse that normalizes them.
156+
expect(detectUnboundFormViewPredicateRoots(
157+
definitionWithFieldPredicate('status == "unqualified"'),
158+
)).toHaveLength(1);
159+
160+
const withAlias: any = definitionWithFieldPredicate(CEL('record.x'));
161+
const field = withAlias.views[0].form.sections[0].fields[1];
162+
delete field.visibleWhen;
163+
field.visibleOn = CEL('status == "unqualified"');
164+
const findings = detectUnboundFormViewPredicateRoots(withAlias);
165+
expect(findings).toHaveLength(1);
166+
expect(findings[0]!.path).toBe('views[0].form.sections[0].fields[1].visibleOn');
167+
});
168+
169+
it('passes an opaque predicate: AST-only, and any non-CEL dialect', () => {
170+
expect(detectUnboundFormViewPredicateRoots(
171+
definitionWithFieldPredicate({ dialect: 'cel', ast: { kind: 'binary' } }),
172+
)).toEqual([]);
173+
expect(detectUnboundFormViewPredicateRoots(
174+
definitionWithFieldPredicate({ dialect: 'template', source: 'status' }),
175+
)).toEqual([]);
176+
});
177+
178+
it('walks section predicates, the legacy `groups` bucket, and sub-fields at depth', () => {
179+
const findings = detectUnboundFormViewPredicateRoots({
180+
views: [
181+
{
182+
form: {
183+
data: { object: 'crm_lead' },
184+
groups: [
185+
{
186+
visibleWhen: CEL('stage == "closed"'),
187+
fields: [
188+
{
189+
field: 'lines',
190+
type: 'repeater',
191+
fields: [
192+
{ field: 'note', visibleWhen: CEL('type == "formula"') },
193+
{ field: 'ok', visibleWhen: CEL('data.type == "formula"') },
194+
],
195+
},
196+
],
197+
},
198+
],
199+
},
200+
},
201+
],
202+
});
203+
expect(findings.map((f) => f.path)).toEqual([
204+
'views[0].form.groups[0].visibleWhen',
205+
'views[0].form.groups[0].fields[0].fields[0].visibleWhen',
206+
]);
207+
expect(findings.map((f) => f.root)).toEqual(['stage', 'type']);
208+
});
209+
210+
it('walks the keyed `formViews` map as well as the default `form` arm', () => {
211+
const findings = detectUnboundFormViewPredicateRoots({
212+
views: [
213+
{
214+
list: { data: { object: 'crm_lead' } },
215+
formViews: {
216+
edit: {
217+
data: { object: 'crm_lead' },
218+
sections: [{ fields: [{ field: 'a', visibleWhen: CEL('status == "x"') }] }],
219+
},
220+
},
221+
},
222+
],
223+
});
224+
expect(findings).toHaveLength(1);
225+
expect(findings[0]!.path).toBe('views[0].formViews.edit.sections[0].fields[0].visibleWhen');
226+
expect(findings[0]!.view).toBe('crm_lead');
227+
});
228+
229+
it('reads an independent form ViewItem, and skips a list ViewItem', () => {
230+
const formItem = {
231+
views: [
232+
{
233+
name: 'crm_lead.edit',
234+
object: 'crm_lead',
235+
viewKind: 'form',
236+
config: { sections: [{ fields: [{ field: 'a', visibleWhen: CEL('status == "x"') }] }] },
237+
},
238+
],
239+
};
240+
const findings = detectUnboundFormViewPredicateRoots(formItem);
241+
expect(findings).toHaveLength(1);
242+
expect(findings[0]!.view).toBe('crm_lead.edit');
243+
expect(findings[0]!.path).toBe('views[0].config.sections[0].fields[0].visibleWhen');
244+
245+
const listItem = { views: [{ ...formItem.views[0], viewKind: 'list' }] };
246+
expect(detectUnboundFormViewPredicateRoots(listItem)).toEqual([]);
247+
});
248+
249+
it('is silent — never throwing — on shapes it cannot read', () => {
250+
expect(detectUnboundFormViewPredicateRoots(undefined)).toEqual([]);
251+
expect(detectUnboundFormViewPredicateRoots(null)).toEqual([]);
252+
expect(detectUnboundFormViewPredicateRoots('not a definition')).toEqual([]);
253+
expect(detectUnboundFormViewPredicateRoots([])).toEqual([]);
254+
expect(detectUnboundFormViewPredicateRoots({ manifest: {} })).toEqual([]);
255+
expect(detectUnboundFormViewPredicateRoots({ views: 'nope' })).toEqual([]);
256+
expect(detectUnboundFormViewPredicateRoots({ views: [null, 7, 'x'] })).toEqual([]);
257+
// Legacy bare-string field entries name a field and carry no predicate.
258+
expect(detectUnboundFormViewPredicateRoots({
259+
views: [{ form: { sections: [{ fields: ['title', 'status', null] }] } }],
260+
})).toEqual([]);
261+
});
262+
263+
it('does not mutate the definition it reads', () => {
264+
const definition = definitionWithFieldPredicate(CEL('status == "unqualified"'));
265+
const before = JSON.stringify(definition);
266+
detectUnboundFormViewPredicateRoots(definition);
267+
expect(JSON.stringify(definition)).toBe(before);
268+
});
269+
});

0 commit comments

Comments
 (0)