Skip to content

Commit 826edb1

Browse files
committed
feat(formula,lint): extend tier-4 type-soundness to flattened flow conditions (#1928)
`firstTypeMismatch` gains a `scope` param: in addition to record-scoped sites (`record.<field>`), it now covers bare-field flow/automation conditions (`status - 1`, `is_active + 1`). The flattened env binds each field as a top-level typed variable with `unlistedVariablesAreDyn: true`, so flow variables stay `dyn` and are never flagged; equality stays excluded. `validateExpression` runs the check in the flattened branch too, so the lint's flow-condition validation surfaces the same advisory warning. Message uses the bare `field` form in flattened scope, `record.field` in record scope. Tests: formula 219 (+4 flattened cases), lint 232 (+2 flow-condition cases). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hnji7EEYR2mGt6pY53a8Bm
1 parent 5143d9d commit 826edb1

5 files changed

Lines changed: 169 additions & 37 deletions

File tree

.changeset/tier4-type-soundness-warnings.md

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,15 @@ would also fail:
2222
- Equality (`==` / `!=`) is excluded: a heterogeneous equality is runtime-safe
2323
(evaluates to `false`), never a fault.
2424

25-
New `firstTypeMismatch` export in `@objectstack/formula` (and an optional
26-
`fieldTypes` hint on `validateExpression`); `@objectstack/lint`'s
27-
`validateStackExpressions` threads each object's field types into every
28-
record-scoped site (formula fields, validation rules, action / hook / sharing
29-
predicates). Warnings are advisory in `objectstack build` / `validate`
30-
(fatal only under `--strict`), matching the tier-3 channel.
25+
New `firstTypeMismatch(source, fieldCelTypes, scope)` export in
26+
`@objectstack/formula` (and an optional `fieldTypes` hint on
27+
`validateExpression`); `@objectstack/lint`'s `validateStackExpressions` threads
28+
each object's field types into every checked site:
29+
30+
- **record-scoped** sites (`record.<field>`) — formula fields, validation rules,
31+
action / hook / sharing predicates;
32+
- **flattened** flow / automation conditions (bare `field`) — where flow
33+
variables stay `dyn` and are never flagged, and equality stays runtime-safe.
34+
35+
Warnings are advisory in `objectstack build` / `validate` (fatal only under
36+
`--strict`), matching the tier-3 channel.

packages/formula/src/cel-engine.ts

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -188,13 +188,40 @@ export type FieldCelType = 'string' | 'bool' | 'dyn';
188188
const UNSOUND_OVERLOAD_RE = /no such overload:\s*([\w.]+)\s*(<=|>=|<|>|\+|-|\*|\/|%)\s*([\w.]+)/;
189189

190190
/**
191-
* A `record`-typed environment where each field carries a concrete CEL type
192-
* (`string`/`bool`) or `dyn`. Member access (`record.<field>`) then resolves to
193-
* the field's type, so cel-js's checker faults an arithmetic/ordering operator
194-
* applied across incompatible types. Built per call — cheap, and only used at
195-
* build time.
191+
* A typed environment for the soundness check. Each field carries a concrete
192+
* CEL type (`string`/`bool`) or `dyn`, so cel-js's checker faults an
193+
* arithmetic/ordering operator applied across incompatible types. The `scope`
194+
* mirrors how the authoring site binds fields:
195+
* - `'record'` → `record.<field>` member access, via a typed struct on the
196+
* `record`/`previous`/`input` namespaces (formula fields,
197+
* validations, action/hook/sharing predicates).
198+
* - `'flattened'` → bare `<field>` top-level variables (flow / automation
199+
* conditions). Unlisted identifiers stay `dyn`
200+
* (`unlistedVariablesAreDyn: true`) so a flow variable never
201+
* faults — only a typed field misused does.
202+
* Built per call — cheap, and only used at build time.
196203
*/
197-
function buildTypedRecordEnv(fieldCelTypes: Readonly<Record<string, FieldCelType>>): Environment {
204+
function buildTypedEnv(
205+
fieldCelTypes: Readonly<Record<string, FieldCelType>>,
206+
scope: 'record' | 'flattened',
207+
): Environment {
208+
if (scope === 'flattened') {
209+
const env = new Environment({
210+
unlistedVariablesAreDyn: true,
211+
enableOptionalTypes: true,
212+
limits: DEFAULT_LIMITS,
213+
});
214+
registerStdLib(env, () => new Date(0));
215+
for (const root of SCOPE_ROOTS) {
216+
try { env.registerVariable(root, 'map'); } catch { /* duplicate — ignore */ }
217+
}
218+
// Fields are bound bare at top level; a name that collides with a root
219+
// (unlikely) is skipped by the duplicate guard.
220+
for (const [name, t] of Object.entries(fieldCelTypes)) {
221+
try { env.registerVariable(name, t); } catch { /* duplicate / reserved — ignore */ }
222+
}
223+
return env;
224+
}
198225
const env = new Environment({
199226
unlistedVariablesAreDyn: false,
200227
enableOptionalTypes: true,
@@ -216,19 +243,26 @@ function buildTypedRecordEnv(fieldCelTypes: Readonly<Record<string, FieldCelType
216243
}
217244

218245
/**
219-
* The first `record.<field>` (or `previous.`/`input.`) reference in `source`
220-
* whose declared CEL type matches `celType` — best-effort attribution of an
221-
* overload fault to the offending field. Returns `null` if none is found.
246+
* The first field reference in `source` whose declared CEL type matches
247+
* `celType` — best-effort attribution of an overload fault to the offending
248+
* field. In `'record'` scope it looks for `record.<field>` (or `previous.`/
249+
* `input.`); in `'flattened'` scope for a bare `<field>` not preceded by a dot.
250+
* Returns `null` if none is found.
222251
*/
223252
function offendingField(
224253
source: string,
225254
fieldCelTypes: Readonly<Record<string, FieldCelType>>,
226255
celType: FieldCelType,
256+
scope: 'record' | 'flattened',
227257
): string | null {
228258
for (const [name, t] of Object.entries(fieldCelTypes)) {
229259
if (t !== celType) continue;
230-
// Word-bounded so `amount` does not match `amount_total`.
231-
if (new RegExp(`(?:record|previous|input)\\.${name}(?![\\w$])`).test(source)) return name;
260+
// Word-bounded so `amount` does not match `amount_total`; in flattened
261+
// scope the leading lookbehind excludes a member ref like `previous.amount`.
262+
const re = scope === 'flattened'
263+
? new RegExp(`(?<![\\w$.])${name}(?![\\w$])`)
264+
: new RegExp(`(?:record|previous|input)\\.${name}(?![\\w$])`);
265+
if (re.test(source)) return name;
232266
}
233267
return null;
234268
}
@@ -251,16 +285,21 @@ function offendingField(
251285
*
252286
* Returns the operand types, the faulting operator, the concrete operand CEL
253287
* type, and (best-effort) the offending field — or `null` when type-sound.
288+
*
289+
* `scope` selects how fields are bound: `'record'` (default) for
290+
* `record.<field>` sites; `'flattened'` for bare-field flow/automation
291+
* conditions.
254292
*/
255293
export function firstTypeMismatch(
256294
source: string,
257295
fieldCelTypes: Readonly<Record<string, FieldCelType>>,
296+
scope: 'record' | 'flattened' = 'record',
258297
): { operator: string; operands: string; celType: FieldCelType; field: string | null } | null {
259298
if (typeof source !== 'string' || !source.trim()) return null;
260299
// An all-`dyn` record can never fault an overload — skip the parse entirely.
261300
if (!Object.values(fieldCelTypes).some((t) => t === 'string' || t === 'bool')) return null;
262301
try {
263-
const env = buildTypedRecordEnv(fieldCelTypes);
302+
const env = buildTypedEnv(fieldCelTypes, scope);
264303
const result = env.parse(source).check?.() as
265304
| { valid?: boolean; error?: { message?: string } }
266305
| undefined;
@@ -277,7 +316,7 @@ export function firstTypeMismatch(
277316
operator,
278317
operands: `${m[1]} ${operator} ${m[3]}`,
279318
celType,
280-
field: offendingField(source, fieldCelTypes, celType),
319+
field: offendingField(source, fieldCelTypes, celType, scope),
281320
};
282321
} catch {
283322
// A parse/other fault is the syntax checker's job (celEngine.compile); this

packages/formula/src/validate.test.ts

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,12 +220,49 @@ describe('validateExpression (ADR-0032)', () => {
220220
expect(validateExpression('predicate', 'record.amount != null && record.amount > 0', schema).warnings).toHaveLength(0);
221221
});
222222

223-
it('does not run without field types, or in a flattened (flow) scope', () => {
223+
it('does not run without field types', () => {
224224
// No fieldTypes → nothing to check.
225225
expect(validateExpression('value', 'record.name * 2', { objectName: 'crm_opportunity', fields: schema.fields, scope: 'record' }).warnings).toHaveLength(0);
226-
// Flattened flow conditions reference fields bare and carry flow variables;
227-
// the typed check is intentionally record-scope only.
228-
expect(validateExpression('predicate', 'name * 2', { ...schema, scope: 'flattened' }).warnings).toHaveLength(0);
226+
});
227+
});
228+
229+
// #1928 tier 4 (flattened) — the same soundness check for bare-field flow /
230+
// automation conditions. Fields are bound bare (`status - 1`); flow variables
231+
// stay `dyn` and are never flagged.
232+
describe('type-soundness warnings — flattened flow conditions (#1928 tier 4)', () => {
233+
const schema = {
234+
objectName: 'crm_opportunity',
235+
fields: ['stage', 'amount', 'is_active', 'title'] as const,
236+
fieldTypes: { stage: 'select', amount: 'currency', is_active: 'boolean', title: 'text' },
237+
scope: 'flattened',
238+
} as const;
239+
240+
it('warns on a bare text field used in arithmetic against a number', () => {
241+
const r = validateExpression('predicate', 'title - 1 > 0', schema);
242+
expect(r.ok).toBe(true);
243+
expect(r.warnings).toHaveLength(1);
244+
expect(r.warnings[0].message).toMatch(/type mismatch/i);
245+
// Bare form — not `record.title`.
246+
expect(r.warnings[0].message).toMatch(/`title`/);
247+
expect(r.warnings[0].message).not.toMatch(/record\.title/);
248+
});
249+
250+
it('warns on a bare boolean field used in arithmetic', () => {
251+
const r = validateExpression('predicate', 'is_active + 1 > 0', schema);
252+
expect(r.ok).toBe(true);
253+
expect(r.warnings).toHaveLength(1);
254+
expect(r.warnings[0].message).toMatch(/boolean/i);
255+
});
256+
257+
it('does NOT flag a flow variable (unlisted → dyn) or number/date fields', () => {
258+
// `expiring_count` is not a schema field → dyn → no fault.
259+
expect(validateExpression('predicate', 'expiring_count * 2 > 10', schema).warnings).toHaveLength(0);
260+
expect(validateExpression('predicate', 'amount / 100 > 5', schema).warnings).toHaveLength(0);
261+
});
262+
263+
it('does NOT flag a correct bare condition, equality, or a select comparison', () => {
264+
expect(validateExpression('predicate', 'stage == "closed_won" && amount > 1000', schema).warnings).toHaveLength(0);
265+
expect(validateExpression('predicate', 'title == "VIP"', schema).warnings).toHaveLength(0);
229266
});
230267
});
231268

packages/formula/src/validate.ts

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,33 @@ function toCelFieldTypes(fieldTypes: Readonly<Record<string, string>>): Record<s
117117
return out;
118118
}
119119

120+
/**
121+
* #1928 tier 4 — a NON-blocking warning for a text/boolean field used with an
122+
* arithmetic/ordering operator against a number (a silent-null bug), or `null`
123+
* when the expression is type-sound. `scope` selects `record.<field>` vs bare
124+
* field binding, and shapes the referenced form in the message.
125+
*/
126+
function typeSoundnessWarning(
127+
source: string,
128+
fieldTypes: Readonly<Record<string, string>>,
129+
scope: 'record' | 'flattened',
130+
): ExprValidationError | null {
131+
const mismatch = firstTypeMismatch(source, toCelFieldTypes(fieldTypes), scope);
132+
if (!mismatch) return null;
133+
const held = mismatch.celType === 'bool' ? 'a boolean' : 'text';
134+
const ref = mismatch.field
135+
? (scope === 'record' ? `\`record.${mismatch.field}\`` : `\`${mismatch.field}\``)
136+
: null;
137+
const subject = ref ? `${ref} holds ${held}` : `${held === 'a boolean' ? 'a boolean' : 'a text'} field`;
138+
return {
139+
source,
140+
message:
141+
`type mismatch \`${mismatch.operands}\` — ${subject} but is used with \`${mismatch.operator}\` ` +
142+
`against a number. This faults at runtime, so the expression silently evaluates to null ` +
143+
`(unless the value happens to be numeric). Use a number field, or drop the arithmetic/comparison.`,
144+
};
145+
}
146+
120147
/** A bare `{x}` that is NOT part of a `{{x}}` mustache hole. */
121148
const SINGLE_BRACE_RE = /(?:^|[^{])\{\s*([A-Za-z_$][\w.$]*)\s*\}(?!\})/;
122149
/** `record.<field>` / `previous.<field>` head references for field-existence. */
@@ -303,20 +330,8 @@ export function validateExpression(
303330
// value happens to be numeric, so this is a warning, not an error. Only
304331
// runs when there is no bare-ref error (the typed check needs the
305332
// canonical `record.<field>` form).
306-
const mismatch = firstTypeMismatch(source, toCelFieldTypes(schema.fieldTypes));
307-
if (mismatch) {
308-
const held = mismatch.celType === 'bool' ? 'a boolean' : 'text';
309-
const subject = mismatch.field
310-
? `\`record.${mismatch.field}\` holds ${held}`
311-
: `${held === 'a boolean' ? 'a boolean' : 'a text'} field`;
312-
warnings.push({
313-
source,
314-
message:
315-
`type mismatch \`${mismatch.operands}\` — ${subject} but is used with \`${mismatch.operator}\` ` +
316-
`against a number. This faults at runtime, so the expression silently evaluates to null ` +
317-
`(unless the value happens to be numeric). Use a number field, or drop the arithmetic/comparison.`,
318-
});
319-
}
333+
const w = typeSoundnessWarning(source, schema.fieldTypes, 'record');
334+
if (w) warnings.push(w);
320335
}
321336
} else if (schema?.fields && schema.fields.length > 0) {
322337
// Flattened flow/automation condition: the record's fields ARE bound at
@@ -337,6 +352,14 @@ export function validateExpression(
337352
});
338353
}
339354
}
355+
// #1928 tier 4 — the same type-soundness check, for bare-field conditions:
356+
// a text/boolean field compared/arithmetic'd against a number faults at
357+
// runtime. Flow variables stay `dyn` (never flagged); equality is
358+
// runtime-safe (never flagged). Advisory only.
359+
if (schema.fieldTypes) {
360+
const w = typeSoundnessWarning(source, schema.fieldTypes, 'flattened');
361+
if (w) warnings.push(w);
362+
}
340363
}
341364
}
342365
return { ok: errors.length === 0, errors, warnings };

packages/lint/src/validate-expressions.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,33 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
283283
expect(w).toHaveLength(1);
284284
expect(w[0].message).toMatch(/record\.title/);
285285
});
286+
287+
it('warns on a flattened flow condition doing arithmetic on a bare text field', () => {
288+
const issues = validateStackExpressions({
289+
objects: [{ name: 'crm_opportunity', fields: { title: { type: 'text' }, amount: { type: 'currency' } } }],
290+
flows: [{
291+
name: 'opp_flow',
292+
nodes: [{ id: 'start', type: 'start', config: { objectName: 'crm_opportunity', condition: 'title * 2 > 10' } }],
293+
edges: [],
294+
}],
295+
});
296+
const w = issues.filter(i => i.severity === 'warning');
297+
expect(w).toHaveLength(1);
298+
expect(w[0].message).toMatch(/type mismatch/i);
299+
expect(w[0].message).toMatch(/`title`/);
300+
});
301+
302+
it('does not flag a numeric flow condition or a flow variable', () => {
303+
const issues = validateStackExpressions({
304+
objects: [{ name: 'crm_opportunity', fields: { amount: { type: 'currency' } } }],
305+
flows: [{
306+
name: 'opp_flow',
307+
nodes: [{ id: 'start', type: 'start', config: { objectName: 'crm_opportunity' } }],
308+
edges: [{ id: 'e1', source: 'start', target: 'end', condition: 'amount / 100 > 5 && expiring_count * 2 > 3' }],
309+
}],
310+
});
311+
expect(issues).toHaveLength(0);
312+
});
286313
});
287314

288315
describe('action visible/disabled predicates (record-scoped) — #2183 class', () => {

0 commit comments

Comments
 (0)