fix(formula): hydrate string-serialized numeric fields in CEL comparisons (#1534)

.changeset/fix-numeric-string-cel-coercion.md

@@ -0,0 +1,9 @@
+---
+"@objectstack/formula": patch
+---
+
+fix(formula): hydrate string-serialized numeric fields in CEL comparisons (#1534)
+
+Numeric fields that serialize as strings — `Field.rating(allowHalf)` → `"5.0"`, `Field.currency(scale)` → `"250000.00"`, `Field.percent` — made comparisons like `record.rating >= 4` fault under strict CEL with `no such overload: dyn >= int`. In flow decision/edge conditions this silently dead-ended the run (no edge matched), and in objectql `applyFormulaPlan` it swallowed to `null`.
+
+The CEL engine now retries an evaluation **once** with purely-numeric strings hydrated to numbers, but only after a `no such overload` fault — so a comparison that already type-checks is never re-interpreted (a zip like `"02134"` stays a string in `record.zip == "02134"`). Because both the automation condition path (`service-automation` `evaluateCondition`) and the objectql formula path route through `ExpressionEngine.evaluate`, both are fixed consistently. A genuinely non-numeric operand (e.g. `record.rating >= 4` where `rating` is `"high"`) still faults loudly rather than being silently rescued.

packages/formula/src/cel-engine.test.ts

@@ -82,4 +82,69 @@ describe('celEngine', () => {
     expect(r.ok).toBe(true);
     if (r.ok) expect(typeof r.value === 'string' || typeof r.value === 'number').toBe(true);
   });
+
+  // ADR-0032 §1c — string-serialized numeric fields (#1530, #1534).
+  describe('numeric-string field hydration', () => {
+    it('compares a rating that serializes as "5.0" against an int literal', () => {
+      const r = celEngine.evaluate(cel('record.rating >= 4'), {
+        record: { rating: '5.0' },
+      });
+      expect(r).toEqual({ ok: true, value: true });
+    });
+
+    it('compares a currency string against an int literal', () => {
+      const r = celEngine.evaluate(cel('record.amount > 100000'), {
+        record: { amount: '250000.00' },
+      });
+      expect(r).toEqual({ ok: true, value: true });
+    });
+
+    it('returns false (not a fault) when the hydrated compare is unmet', () => {
+      const r = celEngine.evaluate(cel('record.rating >= 4'), {
+        record: { rating: '2.5' },
+      });
+      expect(r).toEqual({ ok: true, value: false });
+    });
+
+    it('compares a percent string against a number literal', () => {
+      const r = celEngine.evaluate(cel('record.completion >= 0.8'), {
+        record: { completion: '0.95' },
+      });
+      expect(r).toEqual({ ok: true, value: true });
+    });
+
+    it('hydrates within a compound predicate (the real flow-condition shape)', () => {
+      const r = celEngine.evaluate(
+        cel('record.rating >= 4 && record.status == "new"'),
+        { record: { rating: '5.0', status: 'new' } },
+      );
+      expect(r).toEqual({ ok: true, value: true });
+    });
+
+    it('hydrates nested numeric strings (e.g. previous.* transition gates)', () => {
+      const r = celEngine.evaluate(cel('record.amount > previous.amount'), {
+        record: { amount: '600000.00' },
+        previous: { amount: '500000.00' },
+      });
+      expect(r).toEqual({ ok: true, value: true });
+    });
+
+    it('leaves genuine string equality untouched (no spurious coercion)', () => {
+      // string == string already type-checks, so the retry path never runs
+      // and a numeric-looking string stays a string.
+      const r = celEngine.evaluate(cel('record.zip == "02134"'), {
+        record: { zip: '02134' },
+      });
+      expect(r).toEqual({ ok: true, value: true });
+    });
+
+    it('does not coerce non-numeric strings', () => {
+      // "high" is not a number literal, so the compare still faults loudly
+      // rather than being silently rescued.
+      const r = celEngine.evaluate(cel('record.rating >= 4'), {
+        record: { rating: 'high' },
+      });
+      expect(r.ok).toBe(false);
+    });
+  });
 });

packages/formula/src/cel-engine.ts

@@ -59,6 +59,58 @@ function coerce(value: unknown): unknown {
   return value;
 }
 
+/**
+ * A string that is *entirely* a JS number literal: optional sign, integer
+ * and/or fractional part, optional exponent. Deliberately strict — `"5.0"`,
+ * `"250000.00"`, `"-3"`, `"1e3"` match; `"5px"`, `"0x10"`, `" "`, `""`,
+ * `"1,000"`, `"v2"` do not.
+ */
+// The fractional part is a single optional `(?:\.\d*)?` group anchored by the
+// literal `.` — never the ambiguous `\d+\.?\d*`, whose adjacent unbounded
+// quantifiers (`\d+\d*` when the dot is absent) backtrack polynomially on long
+// digit runs (CodeQL ReDoS). This matches the same strings without the hazard.
+const NUMERIC_STRING_RE = /^[+-]?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?$/;
+
+/**
+ * cel-js raises `no such overload: dyn <op> int` (and kin) when a comparison
+ * or arithmetic operator sees a `string` on one side and a number on the
+ * other. ADR-0032 §1c — numeric fields that serialize as strings (`Field.rating`
+ * → `"5.0"`, `Field.currency` → `"250000.00"`, `Field.percent`) trip this in
+ * flow conditions / formulas (#1530, #1534) even though the schema and the
+ * build-time validator treat them as numeric.
+ */
+function isNumericOverloadError(err: unknown): boolean {
+  const message = err instanceof Error ? err.message : String(err);
+  return /no such overload/i.test(message);
+}
+
+/**
+ * Recursively coerce string values that are *entirely* numeric literals into
+ * numbers. Used only on the {@link isNumericOverloadError} retry path, so it
+ * can never change a comparison that already evaluated cleanly — it only
+ * rescues one that already faulted. Dates and non-numeric strings pass through
+ * untouched (a zip like `"02134"` only changes if the surrounding expression
+ * already faulted, in which case the original loud error is preserved when the
+ * retry still cannot type-check).
+ */
+function hydrateNumericStrings(value: unknown): unknown {
+  if (typeof value === 'string') {
+    const trimmed = value.trim();
+    if (trimmed.length > 0 && NUMERIC_STRING_RE.test(trimmed)) {
+      const n = Number(trimmed);
+      if (Number.isFinite(n)) return n;
+    }
+    return value;
+  }
+  if (Array.isArray(value)) return value.map(hydrateNumericStrings);
+  if (value && typeof value === 'object' && !(value instanceof Date)) {
+    const out: Record<string, unknown> = {};
+    for (const [k, v] of Object.entries(value)) out[k] = hydrateNumericStrings(v);
+    return out;
+  }
+  return value;
+}
+
 function classifyError(err: unknown): EvalResult<never> {
   const message = err instanceof Error ? err.message : String(err);
   let kind: 'parse' | 'type' | 'runtime' | 'bounds' = 'runtime';
@@ -114,8 +166,27 @@ export const celEngine: DialectEngine = {
     try {
       const env = buildEnv(now);
       const scope = buildScope(ctx);
-      const raw = env.evaluate(source, scope);
-      return { ok: true, value: coerce(raw) as T };
+      try {
+        const raw = env.evaluate(source, scope);
+        return { ok: true, value: coerce(raw) as T };
+      } catch (err) {
+        // ADR-0032 §1c — string-serialized numeric fields (`rating` → `"5.0"`,
+        // `amount` → `"250000.00"`) make `record.rating >= 4` raise CEL's
+        // `no such overload: dyn >= int`. Hydrate purely-numeric strings to
+        // numbers and retry ONCE. This only runs after a fault, so a comparison
+        // that already evaluated cleanly is never re-interpreted; if the retry
+        // still cannot type-check, the original loud error is reported (#1534).
+        if (!isNumericOverloadError(err)) throw err;
+        const hydrated = hydrateNumericStrings(scope) as Record<string, unknown>;
+        try {
+          const raw = env.evaluate(source, hydrated);
+          return { ok: true, value: coerce(raw) as T };
+        } catch {
+          // Hydration did not resolve it — surface the original fault, not the
+          // retry's, so the message reflects what the author actually wrote.
+          throw err;
+        }
+      }
     } catch (err) {
       return classifyError(err);
     }

packages/services/service-automation/src/engine.test.ts

@@ -376,6 +376,59 @@ describe('AutomationEngine', () => {
             expect(() => engine.evaluateCondition({ dialect: 'cel', source: '{record.rating} >= 4' }, vars))
                 .toThrow(/source:|template braces|bare CEL/);
         });
+
+        // ADR-0032 §1c / #1534 — numeric fields that serialize as strings
+        // (`Field.rating` → `"5.0"`, `Field.currency` → `"250000.00"`) used to
+        // fault under strict CEL (`no such overload: dyn >= int`) and silently
+        // dead-end the flow at the decision node. The condition must now compare
+        // as a number so the matching edge is taken.
+        it('takes a decision edge gated on a string-serialized numeric field — #1534', async () => {
+            const vars = new Map<string, unknown>([['record', { rating: '5.0', amount: '250000.00' }]]);
+            expect(engine.evaluateCondition({ dialect: 'cel', source: 'record.rating >= 4' }, vars)).toBe(true);
+            expect(engine.evaluateCondition({ dialect: 'cel', source: 'record.amount > 100000' }, vars)).toBe(true);
+            expect(engine.evaluateCondition({ dialect: 'cel', source: 'record.rating >= 4' },
+                new Map([['record', { rating: '2.5' }]]))).toBe(false);
+        });
+
+        it('routes through a decision instead of dead-ending when rating is "5.0" — #1534', async () => {
+            const executed: string[] = [];
+            engine.registerNodeExecutor({
+                type: 'decision',
+                async execute() { return { success: true }; },
+            });
+            engine.registerNodeExecutor({
+                type: 'assignment',
+                async execute(node) {
+                    executed.push(node.id);
+                    return { success: true };
+                },
+            });
+
+            engine.registerFlow('hot_lead', {
+                name: 'hot_lead',
+                label: 'Hot Lead Routing',
+                type: 'record_change',
+                nodes: [
+                    { id: 'start', type: 'start', label: 'Start' },
+                    { id: 'check', type: 'decision', label: 'Check' },
+                    { id: 'hot', type: 'assignment', label: 'Hot' },
+                    { id: 'cold', type: 'assignment', label: 'Cold' },
+                    { id: 'end', type: 'end', label: 'End' },
+                ],
+                edges: [
+                    { id: 'e1', source: 'start', target: 'check' },
+                    { id: 'e2', source: 'check', target: 'hot', condition: 'record.rating >= 4' },
+                    { id: 'e3', source: 'check', target: 'cold', condition: 'record.rating < 4' },
+                    { id: 'e4', source: 'hot', target: 'end' },
+                    { id: 'e5', source: 'cold', target: 'end' },
+                ],
+            });
+
+            const result = await engine.execute('hot_lead', { record: { rating: '5.0' }, object: 'crm_lead' });
+            expect(result.success).toBe(true);
+            expect(executed).toContain('hot');
+            expect(executed).not.toContain('cold');
+        });
     });
 
     describe('Durable Suspend / Resume (ADR-0019)', () => {