feat(formula,lint): advisory type-soundness warnings for expressions (#1928 tier 4)

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

@@ -0,0 +1,36 @@
+---
+"@objectstack/formula": minor
+"@objectstack/lint": minor
+---
+
+feat(formula,lint): advisory type-soundness warnings for formula/predicate expressions (#1928 tier 4)
+
+Closes the last open guardrail from #1928. A `Field.formula` or record-scoped
+predicate that uses a **text or boolean field with an arithmetic (`+ - * / %`)
+or ordering (`< > <= >=`) operator against a number** faults the runtime
+overload and silently evaluates to `null` (e.g. `record.title * 2`,
+`record.is_active + 1`). The build now surfaces this as a **non-blocking
+warning** with the offending field and a corrective message.
+
+Honours the ADR-0032 design law — the checker only flags what the runtime
+would also fail:
+
+- Number / currency / percent / date / datetime fields are declared `dyn`, so
+  the cases the runtime rescues never warn — `record.amount / 100` (the #1930
+  `registerOperator` fix), `record.due == today()` and numeric-string / ISO-date
+  values (the string-hydration retry), and numeric-coded `select` option values.
+- Equality (`==` / `!=`) is excluded: a heterogeneous equality is runtime-safe
+  (evaluates to `false`), never a fault.
+
+New `firstTypeMismatch(source, fieldCelTypes, scope)` export in
+`@objectstack/formula` (and an optional `fieldTypes` hint on
+`validateExpression`); `@objectstack/lint`'s `validateStackExpressions` threads
+each object's field types into every checked site:
+
+- **record-scoped** sites (`record.<field>`) — formula fields, validation rules,
+  action / hook / sharing predicates;
+- **flattened** flow / automation conditions (bare `field`) — where flow
+  variables stay `dyn` and are never flagged, and equality stays runtime-safe.
+
+Warnings are advisory in `objectstack build` / `validate` (fatal only under
+`--strict`), matching the tier-3 channel.

packages/formula/src/cel-engine.ts

@@ -167,6 +167,164 @@ export function detectBareReference(source: string): string | null {
   return firstUndeclaredReference(source);
 }
 
+/**
+ * The CEL type a field is declared as for the Tier-4 type-soundness check
+ * (#1928). Deliberately coarse: only genuinely-scalar, non-numeric-intent
+ * fields are pinned to a concrete type; everything the runtime rescues stays
+ * `dyn` and can therefore never fault. See {@link firstTypeMismatch}.
+ */
+export type FieldCelType = 'string' | 'bool' | 'dyn';
+
+/**
+ * A `no such overload` fault for an ARITHMETIC (`+ - * / %`) or ORDERING
+ * (`< > <= >=`) operator, with the two operand types captured. Equality
+ * (`==` / `!=`) is intentionally excluded: cel-js's checker faults on a
+ * heterogeneous equality (`string == int`) but the runtime evaluates it
+ * cleanly to `false` — so a fault there is NOT a runtime failure and must not
+ * warn. Linear (no nested quantifiers) — no ReDoS. Operand types are `[\w.]+`
+ * (e.g. `string`, `int`, `google.protobuf.Timestamp`); the operator token is
+ * punctuation, so the two never overlap.
+ */
+const UNSOUND_OVERLOAD_RE = /no such overload:\s*([\w.]+)\s*(<=|>=|<|>|\+|-|\*|\/|%)\s*([\w.]+)/;
+
+/**
+ * A typed environment for the soundness check. Each field carries a concrete
+ * CEL type (`string`/`bool`) or `dyn`, so cel-js's checker faults an
+ * arithmetic/ordering operator applied across incompatible types. The `scope`
+ * mirrors how the authoring site binds fields:
+ *  - `'record'`    → `record.<field>` member access, via a typed struct on the
+ *                    `record`/`previous`/`input` namespaces (formula fields,
+ *                    validations, action/hook/sharing predicates).
+ *  - `'flattened'` → bare `<field>` top-level variables (flow / automation
+ *                    conditions). Unlisted identifiers stay `dyn`
+ *                    (`unlistedVariablesAreDyn: true`) so a flow variable never
+ *                    faults — only a typed field misused does.
+ * Built per call — cheap, and only used at build time.
+ */
+function buildTypedEnv(
+  fieldCelTypes: Readonly<Record<string, FieldCelType>>,
+  scope: 'record' | 'flattened',
+): Environment {
+  if (scope === 'flattened') {
+    const env = new Environment({
+      unlistedVariablesAreDyn: true,
+      enableOptionalTypes: true,
+      limits: DEFAULT_LIMITS,
+    });
+    registerStdLib(env, () => new Date(0));
+    for (const root of SCOPE_ROOTS) {
+      try { env.registerVariable(root, 'map'); } catch { /* duplicate — ignore */ }
+    }
+    // Fields are bound bare at top level; a name that collides with a root
+    // (unlikely) is skipped by the duplicate guard.
+    for (const [name, t] of Object.entries(fieldCelTypes)) {
+      try { env.registerVariable(name, t); } catch { /* duplicate / reserved — ignore */ }
+    }
+    return env;
+  }
+  const env = new Environment({
+    unlistedVariablesAreDyn: false,
+    enableOptionalTypes: true,
+    limits: DEFAULT_LIMITS,
+  });
+  registerStdLib(env, () => new Date(0));
+  const fields: Record<string, string> = {};
+  for (const [name, t] of Object.entries(fieldCelTypes)) fields[name] = t;
+  try { env.registerType('OsRecordScope', { fields }); } catch { /* invalid field name — ignore */ }
+  // The record namespaces carry the typed struct; every other root stays a
+  // `map` (dyn members) so a reference through it never faults.
+  for (const root of ['record', 'previous', 'input']) {
+    try { env.registerVariable(root, 'OsRecordScope'); } catch { /* duplicate — ignore */ }
+  }
+  for (const root of SCOPE_ROOTS) {
+    try { env.registerVariable(root, 'map'); } catch { /* already typed above / duplicate — ignore */ }
+  }
+  return env;
+}
+
+/**
+ * The first field reference in `source` whose declared CEL type matches
+ * `celType` — best-effort attribution of an overload fault to the offending
+ * field. In `'record'` scope it looks for `record.<field>` (or `previous.`/
+ * `input.`); in `'flattened'` scope for a bare `<field>` not preceded by a dot.
+ * Returns `null` if none is found.
+ */
+function offendingField(
+  source: string,
+  fieldCelTypes: Readonly<Record<string, FieldCelType>>,
+  celType: FieldCelType,
+  scope: 'record' | 'flattened',
+): string | null {
+  for (const [name, t] of Object.entries(fieldCelTypes)) {
+    if (t !== celType) continue;
+    // Word-bounded so `amount` does not match `amount_total`; in flattened
+    // scope the leading lookbehind excludes a member ref like `previous.amount`.
+    const re = scope === 'flattened'
+      ? new RegExp(`(?<![\\w$.])${name}(?![\\w$])`)
+      : new RegExp(`(?:record|previous|input)\\.${name}(?![\\w$])`);
+    if (re.test(source)) return name;
+  }
+  return null;
+}
+
+/**
+ * Tier-4 type-soundness (#1928): detect a `record`-scoped expression that
+ * type-checks structurally but faults a runtime operator overload because a
+ * text (`string`) or boolean (`bool`) field is used with an arithmetic or
+ * ordering operator against a number. Such an expression evaluates to `null`
+ * at runtime (unless the text value happens to be numeric), so it is surfaced
+ * as a NON-blocking warning.
+ *
+ * Soundness (the ADR-0032 design law — never flag what the runtime tolerates):
+ *  - Number / currency / percent / date / datetime fields are declared `dyn`,
+ *    because the runtime rescues every mixed case for them — `registerOperator`
+ *    for `double`×`int` arithmetic and the string-hydration retry for
+ *    numeric-string / ISO-date values — so they can never fault here.
+ *  - Equality (`==` / `!=`) is excluded ({@link UNSOUND_OVERLOAD_RE}): a
+ *    heterogeneous equality is runtime-safe.
+ *
+ * Returns the operand types, the faulting operator, the concrete operand CEL
+ * type, and (best-effort) the offending field — or `null` when type-sound.
+ *
+ * `scope` selects how fields are bound: `'record'` (default) for
+ * `record.<field>` sites; `'flattened'` for bare-field flow/automation
+ * conditions.
+ */
+export function firstTypeMismatch(
+  source: string,
+  fieldCelTypes: Readonly<Record<string, FieldCelType>>,
+  scope: 'record' | 'flattened' = 'record',
+): { operator: string; operands: string; celType: FieldCelType; field: string | null } | null {
+  if (typeof source !== 'string' || !source.trim()) return null;
+  // An all-`dyn` record can never fault an overload — skip the parse entirely.
+  if (!Object.values(fieldCelTypes).some((t) => t === 'string' || t === 'bool')) return null;
+  try {
+    const env = buildTypedEnv(fieldCelTypes, scope);
+    const result = env.parse(source).check?.() as
+      | { valid?: boolean; error?: { message?: string } }
+      | undefined;
+    if (!result || result.valid !== false) return null;
+    const m = UNSOUND_OVERLOAD_RE.exec(result.error?.message ?? '');
+    if (!m) return null;
+    const operator = m[2];
+    const celType: FieldCelType | null =
+      m[1] === 'string' || m[1] === 'bool' ? (m[1] as FieldCelType)
+      : m[3] === 'string' || m[3] === 'bool' ? (m[3] as FieldCelType)
+      : null;
+    if (!celType) return null;
+    return {
+      operator,
+      operands: `${m[1]} ${operator} ${m[3]}`,
+      celType,
+      field: offendingField(source, fieldCelTypes, celType, scope),
+    };
+  } catch {
+    // A parse/other fault is the syntax checker's job (celEngine.compile); this
+    // helper only reports a clean type-soundness verdict.
+    return null;
+  }
+}
+
 /** Coerce cel-js's BigInt-flavored return into spec-friendly JS values. */
 function coerce(value: unknown): unknown {
   if (typeof value === 'bigint') {

packages/formula/src/validate.test.ts

@@ -153,6 +153,119 @@ describe('validateExpression (ADR-0032)', () => {
     });
   });
 
+  // #1928 tier 4 — a text/boolean field used with an arithmetic or ordering
+  // operator against a number faults at runtime (silent null). With per-field
+  // types the validator surfaces this as a NON-blocking warning, and — the
+  // design law — never flags a case the runtime tolerates (number/date fields,
+  // equality, string concat, null-guards).
+  describe('type-soundness warnings (#1928 tier 4)', () => {
+    const schema = {
+      objectName: 'crm_opportunity',
+      fields: ['name', 'amount', 'is_active', 'due', 'priority', 'title'] as const,
+      fieldTypes: {
+        name: 'text', title: 'textarea', amount: 'currency',
+        is_active: 'boolean', due: 'date', priority: 'select',
+      },
+      scope: 'record',
+    } as const;
+
+    it('warns (does not error) on a text field used in arithmetic against a number', () => {
+      const r = validateExpression('value', 'record.name * 2', schema);
+      expect(r.ok).toBe(true);
+      expect(r.errors).toHaveLength(0);
+      expect(r.warnings).toHaveLength(1);
+      expect(r.warnings[0].message).toMatch(/type mismatch/i);
+      expect(r.warnings[0].message).toMatch(/record\.name/);
+      expect(r.warnings[0].message).toMatch(/evaluates to null/);
+    });
+
+    it('warns on a text field ordered against a number', () => {
+      const r = validateExpression('predicate', 'record.title >= 5', schema);
+      expect(r.ok).toBe(true);
+      expect(r.warnings).toHaveLength(1);
+      expect(r.warnings[0].message).toMatch(/record\.title/);
+    });
+
+    it('warns on a boolean field used in arithmetic (always faults at runtime)', () => {
+      const r = validateExpression('value', 'record.is_active + 1', schema);
+      expect(r.ok).toBe(true);
+      expect(r.warnings).toHaveLength(1);
+      expect(r.warnings[0].message).toMatch(/boolean/i);
+      expect(r.warnings[0].message).toMatch(/record\.is_active/);
+    });
+
+    it('does NOT warn on number/currency arithmetic with an int literal (#1930 runtime fix)', () => {
+      // currency → dyn, so `amount / 100`, `amount * 2 - 50` never fault.
+      expect(validateExpression('value', 'record.amount / 100', schema).warnings).toHaveLength(0);
+      expect(validateExpression('value', 'record.amount * 2 - 50', schema).warnings).toHaveLength(0);
+    });
+
+    it('does NOT warn on a date field compared to today()/daysFromNow()', () => {
+      expect(validateExpression('predicate', 'record.due <= daysFromNow(30)', schema).warnings).toHaveLength(0);
+      expect(validateExpression('predicate', 'record.due == today()', schema).warnings).toHaveLength(0);
+    });
+
+    it('does NOT warn on a select field ordered against a number (option values may be numeric codes)', () => {
+      // select → dyn, so `priority >= 3` (a numeric-coded picklist) is not flagged.
+      expect(validateExpression('predicate', 'record.priority >= 3', schema).warnings).toHaveLength(0);
+    });
+
+    it('does NOT warn on heterogeneous equality (runtime-safe, returns false)', () => {
+      expect(validateExpression('predicate', 'record.name == 5', schema).warnings).toHaveLength(0);
+      expect(validateExpression('predicate', 'record.name != 5', schema).warnings).toHaveLength(0);
+    });
+
+    it('does NOT warn on string concatenation or a null-guard', () => {
+      expect(validateExpression('value', 'record.name + record.title', schema).warnings).toHaveLength(0);
+      expect(validateExpression('predicate', 'record.amount != null && record.amount > 0', schema).warnings).toHaveLength(0);
+    });
+
+    it('does not run without field types', () => {
+      // No fieldTypes → nothing to check.
+      expect(validateExpression('value', 'record.name * 2', { objectName: 'crm_opportunity', fields: schema.fields, scope: 'record' }).warnings).toHaveLength(0);
+    });
+  });
+
+  // #1928 tier 4 (flattened) — the same soundness check for bare-field flow /
+  // automation conditions. Fields are bound bare (`status - 1`); flow variables
+  // stay `dyn` and are never flagged.
+  describe('type-soundness warnings — flattened flow conditions (#1928 tier 4)', () => {
+    const schema = {
+      objectName: 'crm_opportunity',
+      fields: ['stage', 'amount', 'is_active', 'title'] as const,
+      fieldTypes: { stage: 'select', amount: 'currency', is_active: 'boolean', title: 'text' },
+      scope: 'flattened',
+    } as const;
+
+    it('warns on a bare text field used in arithmetic against a number', () => {
+      const r = validateExpression('predicate', 'title - 1 > 0', schema);
+      expect(r.ok).toBe(true);
+      expect(r.warnings).toHaveLength(1);
+      expect(r.warnings[0].message).toMatch(/type mismatch/i);
+      // Bare form — not `record.title`.
+      expect(r.warnings[0].message).toMatch(/`title`/);
+      expect(r.warnings[0].message).not.toMatch(/record\.title/);
+    });
+
+    it('warns on a bare boolean field used in arithmetic', () => {
+      const r = validateExpression('predicate', 'is_active + 1 > 0', schema);
+      expect(r.ok).toBe(true);
+      expect(r.warnings).toHaveLength(1);
+      expect(r.warnings[0].message).toMatch(/boolean/i);
+    });
+
+    it('does NOT flag a flow variable (unlisted → dyn) or number/date fields', () => {
+      // `expiring_count` is not a schema field → dyn → no fault.
+      expect(validateExpression('predicate', 'expiring_count * 2 > 10', schema).warnings).toHaveLength(0);
+      expect(validateExpression('predicate', 'amount / 100 > 5', schema).warnings).toHaveLength(0);
+    });
+
+    it('does NOT flag a correct bare condition, equality, or a select comparison', () => {
+      expect(validateExpression('predicate', 'stage == "closed_won" && amount > 1000', schema).warnings).toHaveLength(0);
+      expect(validateExpression('predicate', 'title == "VIP"', schema).warnings).toHaveLength(0);
+    });
+  });
+
   describe('introspection', () => {
     it('reports the dialect + scope for a field role', () => {
       expect(expectedDialect('predicate')).toBe('cel');