Formula guardrail: cel-js arithmetic silently returns null (double × int + bare identifiers)

[os-zhuang]

Summary

Two formula-authoring footguns let an AI write a reasonable-looking Field.formula that passes objectstack build but silently evaluates to null at runtime — the failure class our guardrails are meant to eliminate. Both were found during 9.5.x release-prep testing of example-crm (fixed there in #1927; this issue tracks the systemic prevention).

1. No double × int arithmetic overload (primary)

cel-js (@marcbachmann/cel-js) types a record field number as double and a bare integer literal as int, and has no mixed arithmetic overload:

record.amount / 100              → ERR  no such overload: dyn<double> / int   → null
record.amount * 2                → ERR  no such overload: dyn<double> * int    → null
record.amount * record.probability / 100.0   → OK  (float literal)
record.amount >= 4               → OK   (cel-js DOES have mixed *comparison* overloads)

So amount / 100, price * 2, total - fee (int literal) all silently null. Only float literals (100.0, 2.0) work. Integer literals in arithmetic are ubiquitous → high-frequency footgun.

Constraint: cel-js rejects registering operator overloads — env.registerFunction('_/_(double, int): double', …) → Invalid signature. So there is no trivial overload-registration fix.

Options

• (a) Build-time lint — in validate-expressions.ts (ADR-0032), flag a number-typed field used as an arithmetic operand against an int literal; message: "use a float literal (e.g. / 100.0)". Cheapest, matches the advisory-warning guardrail strategy.
• (b) Engine AST promotion — in the CEL engine, walk the parsed AST and promote int literals that are arithmetic operands to double. Must NOT touch int-typed function args (e.g. daysFromNow(30)). More robust (fixes silently), more invasive, needs tests.
• (a) and (b) are complementary.

2. Bare identifiers in formula expressions (secondary)

Formula scope exposes only {record, previous, input, os} (see buildScope, packages/formula/src/stdlib.ts) — no field flattening. So:

cel`status == "converted"`        → null   (bare `status`)
cel`record.status == "converted"` → OK

validate-expressions.ts doesn't flag bare identifiers that fail to resolve. A lint that requires record./input./os.-qualified references in formula expressions would catch it at build.

Acceptance

• An AI-authored expected_revenue = amount * probability / 100 is either rejected at build with an actionable message, or compiles to a working double formula.
• Bare-identifier formula references are flagged at build.

Refs: #1927 (example-crm fixes), ADR-0032. Same "build-green / runtime-silent" family as #1877 / #1870 / #1876.

[os-zhuang]

Feasibility verified (cel-js 7.6.1) — both compile-time AND runtime fixes work

Spiked both approaches against the pinned cel-js. Yes, these bugs can be caught at metadata-compile time — and there's an even cleaner runtime fix that removes the author burden entirely.

A. Compile-time type check (answers "can the build catch it?") ✅

The build env currently uses unlistedVariablesAreDyn: true, so every field is dyn and nothing type-checks. If the build-time validator instead declares record/input/previous with the schema's real field types and runs cel-js check():

const env = new Environment({ unlistedVariablesAreDyn: false })
  .registerType('Rec', { fields: { amount: 'double', probability: 'double', status: 'string' } })
  .registerVariable('record', 'Rec');
env.parse('record.amount / 100').check();      // → { valid: false, error: 'no such overload: double / int' }
env.parse('record.amount / 100.0').check();    // → { valid: true }
env.parse('record.amont / 100.0').check();     // → { valid: false, error: 'No such key: amont' }

So a typed check() catches a superset of what we hand-fixed in #1927:

• double / int arithmetic overload → flagged
• unknown/typo field (amont) → flagged
• bare identifiers (status instead of record.status) → flagged as undeclared variable (with unlistedVariablesAreDyn:false)

This is a build-time-only environment (runtime intentionally stays dyn + string-hydration retry), wired into validate-expressions.ts (ADR-0032) which already has the object schema. Needs a field-type → CEL-type map (number/currency/percent→double, text/select→string, checkbox→bool, date/datetime→google.protobuf.Timestamp, lookup→nested type or dyn).

B. Runtime fix via registerOperator (makes the common case Just Work) ✅

My earlier note said cel-js rejects operator overloads — that was the wrong API (registerFunction('_/_…')). The correct API is registerOperator('double / int', impl):

for (const op of ['/','*','+','-','%'])
  for (const sig of [`double ${op} int`, `int ${op} double`])
    env.registerOperator(sig, NUMERIC_IMPL[op]);

env.evaluate('record.amount / 100', { record:{ amount:120000 } });                       // → 1200
env.evaluate('record.amount * record.probability / 100', { record:{amount:120000,probability:70} }); // → 84000
env.evaluate('7 / 2', {});                                                                // → 3  (int/int untouched — overload only on MIXED)

~10 lines in buildEnv/registerStdLib (cel-engine.ts). amount / 100 then works with no float-literal burden, and pure int/int keeps integer-division semantics. Composes with the existing string-hydration retry.

Recommendation

Ship both: B removes the footgun for authors (no .0 ritual), A turns the genuinely wrong cases (typos, type mismatches, bare identifiers) into actionable build errors. With B in place, the / 100.0 workaround in #1927's expected_revenue can revert to plain / 100.

[os-zhuang]

Strategy: validate every expression, tiered by what can be checked soundly

Re-evaluated against the mission (all metadata is AI-authored → catch mistakes at build). The goal is yes, validate every expression site. But there's a hard design law, learned the hard way:

The build-time checker may only reject what the runtime evaluator would also fail. Anything stricter is a false positive — and since the authors are AIs generating the whole ecosystem, a false-positive gate breaks the very builds it's meant to protect.

So coverage is staged by soundness, not by ambition. Three findings pin the tiers:

1. Per-site scope differs. Flow/automation conditions flatten the record's fields to top-level (status is valid); Field.formula and validation predicates bind only the record namespace (status is a silent-null bug). A single rule is wrong for both.
2. Real-typed checking false-positives. Declaring fields with true types makes record.lead_score != null fault (no such overload: double != null) — null-guards are everywhere and correct at runtime. So fields must be dyn in the checker, not their real types.
3. Flow conditions also carry flow variables (expiring_deals, new_opportunity) that aren't schema-knowable, so a bare identifier there can't be told apart from a typo.

Tiers

Tier	Catches	Soundness	Status
1. Bare ref in record-scope (formula + validation)	bare amount → null	zero false positives (verified)	#1931 (hard error)
2. record.X typos at all sites	record.amont	already sound (regex existence)	shipped
3. Bare-field typos in flow conditions	staus == "done"	needs flow-var awareness → can't hard-gate; edit-distance warning only	proposed, advisory
4. Full type-soundness (string vs number, date misuse)	wrong-type compares	high false-positive surface; must mirror runtime exactly + bake as warnings	deferred

Tiers 1+2 are the dominant AI-error classes (silent-null formulas, silent-skip rules) and are now hard build errors with zero false positives. Tiers 3–4 are real but need a warning channel (two-tier guardrail) before they can gate — validateStackExpressions currently hard-fails on every issue, so adding severity: warning is the prerequisite. Recommend tackling that next if we want flow-condition typo coverage.

[os-zhuang]

Tier 3 implemented in #1933 (advisory did-you-mean warnings for likely field typos in flow conditions; non-blocking). Tiers now shipped: 1 (record-scope bare refs, hard error — #1931), 2 (record.X existence, regex — pre-existing), 3 (flow-condition typos, warning — #1933). Tier 4 (full type-soundness) remains deferred — it needs the checker to mirror runtime string-hydration/optional-type semantics exactly and a longer warning bake. Runtime arithmetic fix: #1930.