From dd2c1baa57ca892228b4ee6462b479ad998ded8b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 01:35:16 +0000 Subject: [PATCH] =?UTF-8?q?fix(formula):=20the=20ADR-0032=20=C2=A71c=20ret?= =?UTF-8?q?ry=20rewrites=20only=20the=20operands=20that=20faulted=20(#7098?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hydrateOverloadStrings` rewrote the whole scope and re-ran the whole expression on a docblock claim that it "can never change a comparison that already evaluated cleanly". The claim was false and load-bearing — it was the stated reason the hydration was allowed to be unconditional and scope-wide. The retry knows only that the WHOLE expression faulted, so every other comparison was re-interpreted against the hydrated values: record.n >= 4 && record.s == "5.0" with { n: "7", s: "5.0" } before -> { ok: true, value: false } after -> { ok: true, value: true } The author's deliberate string equality was true in evaluation 1 and was overruled silently — no fault, no log line, no red test. The coercion is now per operand POSITION, the discipline `rewriteTemporalEquality` already documents ("no field-wide trade-off") and one step stricter. The scope is never rewritten; the faulting operand is wrapped in `double(…)`/`date(…)` in place. An operand qualifies only when the operator RAISES on a string-versus-number/Timestamp pair, the counterpart is a number/Timestamp in this scope, and the operand is a §1c serialization artifact — so the docblock's guarantee now holds by construction. Measured per operator on cel-js 8.0.0: `<` `<=` `>` `>=` `+` `-` `*` `/` `%` fault and are eligible; `==`, `!=` and `in` ANSWER across types, so they already had an answer and are never rewritten. That is the root of the defect. Reach measured: this evaluator does not reach RLS — row-level security and declared sharing compile through `compileCelToFilter` / `matchesFilterCondition`, never through `celEngine.evaluate`. It does reach validation-rule predicates and `when` conditionals, `readonlyWhen`, hook conditions, automation conditions and formula fields. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BTTPxkGkjPU9u8gT57PtiJ --- .../cel-overload-retry-operand-scope.md | 107 ++++++++ packages/formula/src/cel-engine.ts | 230 +++++++++++++++--- .../src/cel-overload-retry-scope.test.ts | 179 ++++++++++++++ 3 files changed, 483 insertions(+), 33 deletions(-) create mode 100644 .changeset/cel-overload-retry-operand-scope.md create mode 100644 packages/formula/src/cel-overload-retry-scope.test.ts diff --git a/.changeset/cel-overload-retry-operand-scope.md b/.changeset/cel-overload-retry-operand-scope.md new file mode 100644 index 0000000000..213d532367 --- /dev/null +++ b/.changeset/cel-overload-retry-operand-scope.md @@ -0,0 +1,107 @@ +--- +"@objectstack/formula": patch +--- + +fix(formula): the ADR-0032 §1c retry rewrites only the operands that faulted (#7098) + +**A CEL expression could return a silently wrong boolean.** No fault, no log +line, no failing test — `{ ok: true }` with the wrong answer. If you have +compound CEL that mixes a numeric comparison with a string equality over +string-serialized fields, read the "which expressions change answer" list below: +those expressions answer differently after this fix, and the new answer is the +right one. + +## What was wrong + +When a comparison faults on a string-serialized numeric or date field +(`record.rating >= 4` where `rating` reads back as `"5.0"` — #1530 / #1534), +ADR-0032 §1c hydrates and retries. The retry hydrated the **entire scope** and +re-ran the **entire expression**, justified by a docblock claim that it + +> can never change a comparison that already evaluated cleanly — it only rescues +> one that already faulted. + +That claim was false, and it was load-bearing: it was the stated reason the +hydration was allowed to be unconditional and scope-wide. The retry knows only +that the *whole expression* faulted, not that each sub-comparison did. So: + +```text +record.n >= 4 && record.s == "5.0" with { n: "7", s: "5.0" } + before -> { ok: true, value: false } after -> { ok: true, value: true } +``` + +`record.n >= 4` faults and is correctly rescued. But `record.s` was hydrated to +the number `5` as well, so the author's deliberate string equality — **true** +when it was evaluated the first time — became `5 == "5.0"`, which CEL answers +`false` across types. The expression returned `false`, and nothing reported that +a clean answer had been overruled. + +## Which expressions change answer + +Only expressions that **already reached the §1c retry** — i.e. some operand +faulted `no such overload`. Everything that evaluates without faulting is +untouched. Within that set, an expression changes answer when it also contains: + +- **a string equality / inequality on a numeric-looking or ISO-date field** — + `record.n >= 4 && record.s == "5.0"`, and the `!=` and ternary forms. Now + answers on the string the author wrote. +- **a string membership test** — `record.s in ["5.0", "x"]`. +- **the same field compared as a number in one place and as a string in + another** — `record.n >= 4 && record.n == "7"`. Both answers are now correct + at once; previously the second was collateral damage from the first. +- **a numeric-looking string the expression RETURNS rather than compares** — + `record.n >= 4 ? record.s : "none"` returned the number `5`; it now returns + the string `"5.0"`. A `Field.formula` of type text was storing a different + value than the record held. + +One class becomes a **loud fault where it used to be silently rescued**: an +operand whose value the rewrite cannot read before deciding — bound by a +comprehension (`record.items.exists(i, i.price > 100)`), or behind a computed +index. That is the deliberate trade of this fix. Rescuing an operand we cannot +prove faulted is exactly the defect being closed, so those report the original +`no such overload` instead of guessing. The reported error is unchanged in shape +and message. + +## What replaces it + +The coercion is now **per operand position** — the same discipline +`rewriteTemporalEquality` already documents ("no field-wide trade-off"), one +step stricter. The scope is never rewritten; the faulting operand is wrapped in +`double(…)` or `date(…)` in place. An operand is rewritten only when all three +hold, which makes the docblock's guarantee true by construction rather than by +assertion: + +1. the operator **raises** on a string-versus-number/Timestamp pair instead of + answering one, so the comparison cannot have produced an answer; +2. the counterpart is a number or a Timestamp **in this scope**, read off the + values in hand rather than off a static type (every field is `dyn` under + `unlistedVariablesAreDyn`); +3. the operand's own value is a §1c serialization artifact — an entirely-numeric + string or an ISO-8601 date. A zip like `"02134"`, or free text, still faults + loudly. + +Measured per operator on cel-js 8.0.0 and pinned in the new tests: `<` `<=` `>` +`>=` `+` `-` `*` `/` `%` **fault** on a mixed pair and are eligible. `==`, `!=` +and `in` **answer** across types — CEL equality is total — so they already had +an answer and are never rewritten. That measurement is the root of the defect: +the string equality above never faulted at all. + +`Field.date` strings not matching a Timestamp under `==` remains owned by +`rewriteTemporalEquality`, which wraps them statically on the clean path, where +both sides are known from the source instead of inferred from an unrelated +conjunct's fault. + +## Reach + +`celEngine.evaluate` — the only home of this retry — does **not** reach RLS. +Row-level security compiles its `using` / `check` predicates through +`compileCelToFilter` (SQL pushdown) and `matchesFilterCondition` (write-side +post-image), and declared sharing rules do the same; neither calls this +evaluator. No access-control decision could be inverted by this. + +It does reach write-gating decisions, which is why the behaviour was not +acceptable as documented: validation-rule predicates and `when` conditionals, +`readonlyWhen`, hook `condition`s, automation/flow conditions, and formula +fields and default values. A validation rule is **fail-closed** on a fault +(#4649) — but a silently flipped boolean is not a fault, so a rule that should +have rejected a write instead read as "not violated" and let it through. diff --git a/packages/formula/src/cel-engine.ts b/packages/formula/src/cel-engine.ts index 1d2f594128..8f9d52fa23 100644 --- a/packages/formula/src/cel-engine.ts +++ b/packages/formula/src/cel-engine.ts @@ -1073,36 +1073,197 @@ function isNumericOverloadError(err: unknown): boolean { } /** - * Recursively coerce string values that faulted a CEL overload into their - * intended primitive: entirely-numeric literals → `number` (#1534), and - * ISO-8601 date / date-time strings → `Date` (cel-js `google.protobuf.Timestamp`) - * (#1530). 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. Strings that are neither (a zip like `"02134"`, free - * text) pass through untouched; if the retry still cannot type-check, the - * original loud error is preserved. + * The operators that RAISE on a string-versus-number/Timestamp operand pair + * instead of answering one. This membership is the entire basis of the §1c + * rescue below — for these operators a mixed pair cannot have produced an + * answer, so rewriting the operand cannot change one. + * + * Measured per operator on cel-js 8.0.0 (#7098), against an int literal, a + * number-valued field, a `today()` Timestamp and a Date-valued field: + * + * - `<` `<=` `>` `>=` `+` `-` `*` `/` `%` — **fault**, every shape: + * `no such overload: dyn >= int`. Listed. + * - `==` `!=` — **answer**, `false` / `true`. CEL equality is defined across + * types, so `record.s == 5` over `{ s: "5" }` is a clean `false`, not a + * fault. DELIBERATELY ABSENT: coercing an equality is exactly the defect + * this function closes — the author's string equality already had an answer. + * The separate problem that a `Field.date` string never equals a Timestamp is + * owned by {@link rewriteTemporalEquality}, which wraps it statically and + * per-occurrence on the CLEAN path, where the two sides are known from the + * source rather than guessed from an unrelated conjunct's fault. + * - `in` — **answers** too (`"7" in [1, 7]` is a clean `false`). Absent for the + * same reason. */ -function hydrateOverloadStrings(value: unknown): unknown { - if (typeof value === 'string') { - const trimmed = value.trim(); - if (trimmed.length > 0) { - if (NUMERIC_STRING_RE.test(trimmed)) { - const n = Number(trimmed); - if (Number.isFinite(n)) return n; - } else if (ISO_TEMPORAL_STRING_RE.test(trimmed)) { - const ms = Date.parse(trimmed); - if (!Number.isNaN(ms)) return new Date(ms); - } - } - return value; +const COERCIBLE_OPS: ReadonlySet = new Set([ + '<', '<=', '>', '>=', '+', '-', '*', '/', '%', +]); + +/** What an operand will actually BE at evaluation time — see {@link operandKind}. */ +type OperandKind = 'number' | 'temporal' | 'string' | 'unknown'; + +/** + * The scope path a node names, or null when it names none: `record.n` → + * `['record','n']`, a bare `status` (the flattened flow scope) → `['status']`, + * and `record.items[0].price` / `record["n"]` → the same walk through a CONSTANT + * index. Null for everything else — a call, an arithmetic sub-tree, a variable + * bound by a comprehension — which is what keeps the rewrite below to operands + * whose runtime value we can actually read before deciding. + */ +function scopePath(node: unknown): string[] | null { + if (!isCelNode(node)) return null; + if (node.op === 'id' && typeof node.args === 'string') return [node.args]; + if (node.op === '.' && Array.isArray(node.args) && node.args.length === 2) { + const [base, member] = node.args; + if (typeof member !== 'string') return null; + const head = scopePath(base); + return head ? [...head, member] : null; } - if (Array.isArray(value)) return value.map(hydrateOverloadStrings); - if (value && typeof value === 'object' && !(value instanceof Date)) { - const out: Record = {}; - for (const [k, v] of Object.entries(value)) out[k] = hydrateOverloadStrings(v); - return out; + if (node.op === '[]' && Array.isArray(node.args) && node.args.length === 2) { + const [base, index] = node.args; + if (!isCelNode(index) || index.op !== 'value') return null; + const key = index.args; + if (typeof key !== 'string' && typeof key !== 'bigint' && typeof key !== 'number') return null; + const head = scopePath(base); + return head ? [...head, String(key)] : null; } - return value; + return null; +} + +/** Resolve a {@link scopePath} against the scope; `undefined` when any hop is absent. */ +function resolveScopePath(scope: Record, path: readonly string[]): unknown { + let cur: unknown = scope; + for (const seg of path) { + if (cur == null || typeof cur !== 'object') return undefined; + cur = (cur as Record)[seg]; + } + return cur; +} + +/** The {@link OperandKind} of a concrete runtime value. */ +function valueKind(v: unknown): OperandKind { + if (typeof v === 'number' || typeof v === 'bigint') return 'number'; + if (v instanceof Date) return 'temporal'; + if (typeof v === 'string') return 'string'; + return 'unknown'; +} + +/** + * What the operand will actually be when cel-js evaluates it — read off the + * literal, off the known return type of a stdlib call, or (for a scope path) off + * the value ALREADY IN HAND in this scope. Reading the scope rather than the + * static type is what makes the "this comparison provably faulted" test exact + * under `unlistedVariablesAreDyn`, where every field is statically `dyn`. + * + * `unknown` is the safe answer and the common one: an arithmetic sub-tree, a + * comprehension variable, an absent key. An `unknown` counterpart never licenses + * a rewrite. + */ +function operandKind(node: unknown, scope: Record): OperandKind { + if (!isCelNode(node)) return 'unknown'; + if (node.op === 'value') return valueKind(node.args); + if (isTemporalCall(node)) return 'temporal'; + if (node.op === 'call' && Array.isArray(node.args) && typeof node.args[0] === 'string') { + const fn = node.args[0]; + if (fn === 'date' || fn === 'datetime') return 'temporal'; + if (fn === 'double' || fn === 'int' || fn === 'uint') return 'number'; + return 'unknown'; + } + const path = scopePath(node); + if (!path) return 'unknown'; + const resolved = resolveScopePath(scope, path); + return resolved === undefined ? 'unknown' : valueKind(resolved); +} + +/** + * The coercion this operand needs to meet `counterpart`, or null when it is not + * one ADR-0032 §1c rescues: entirely-numeric literals → `double(…)` (#1534) and + * ISO-8601 date / date-time strings → `date(…)` (#1530). Strings that are + * neither — a zip like `"02134"`, free text — return null and the original loud + * fault is preserved, exactly as before. + * + * The coercion must MATCH the counterpart: a numeric string opposite a Timestamp + * (or an ISO string opposite a number) is a genuine mismatch, not a §1c + * serialization artifact, and is left to fault. + */ +function coercionFor(value: unknown, counterpart: OperandKind): 'double' | 'date' | null { + if (typeof value !== 'string') return null; + const trimmed = value.trim(); + if (trimmed.length === 0) return null; + if (counterpart === 'number' && NUMERIC_STRING_RE.test(trimmed)) { + return Number.isFinite(Number(trimmed)) ? 'double' : null; + } + if (counterpart === 'temporal' && ISO_TEMPORAL_STRING_RE.test(trimmed)) { + return Number.isNaN(Date.parse(trimmed)) ? null : 'date'; + } + return null; +} + +/** Wrap an AST node in a one-argument stdlib call (`double(x)` / `date(x)`). */ +function wrapInCall(fn: string, node: CelNode): CelNode { + return { op: 'call', args: [fn, [node]] }; +} + +/** + * #7098 — coerce the operands that PROVABLY faulted, and only those. + * + * The predecessor of this function hydrated the whole scope and re-ran the + * expression, on a docblock claim that "it can never change a comparison that + * already evaluated cleanly". That claim was false and load-bearing: the retry + * knows only that the WHOLE expression faulted, so rewriting the scope + * re-interprets every OTHER comparison too. `record.n >= 4 && record.s == "5.0"` + * over `{ n: "7", s: "5.0" }` faults on the first conjunct, hydrates BOTH fields, + * and answers `false` — the author's deliberate string equality was `true` in + * evaluation 1, and nothing reports that it was overruled. + * + * So the rewrite is now **per-occurrence**, the same discipline + * {@link rewriteTemporalEquality} already documents ("no field-wide trade-off"), + * and one step stricter — it is per operand POSITION, so a field compared to an + * int in one conjunct and to a string literal in another keeps both answers. + * + * An operand is rewritten only where all three hold, which together make the + * docblock's guarantee true by construction rather than by assertion: + * 1. the operator is one of {@link COERCIBLE_OPS} — no string↔number/Timestamp + * overload exists, so a mixed pair cannot have produced an answer; + * 2. the counterpart is a number or a Timestamp *in this scope*, established by + * {@link operandKind} against the values in hand, not by static type; + * 3. this operand's own value is a §1c serialization artifact + * ({@link coercionFor}). + * + * Returns the rewritten source, or null when no operand qualifies — in which + * case the caller preserves the original loud error rather than guessing. That + * is the deliberate trade: shapes the walk cannot read (a comprehension + * variable, a computed index) now FAULT where they were once silently rescued, + * because a silent rescue of an operand we cannot prove faulted is precisely the + * defect this closes. + */ +function rewriteFaultedOperands(source: string, scope: Record): string | null { + let ast: unknown; + try { + ast = (recordScopeEnv ??= buildScopedEnv([])).parse(source).ast; + } catch { + return null; + } + let changed = false; + const visit = (node: unknown): void => { + if (!isCelNode(node)) return; + if (COERCIBLE_OPS.has(node.op) && Array.isArray(node.args) && node.args.length === 2) { + const args = node.args as unknown[]; + for (const side of [0, 1] as const) { + const operand = args[side]; + const path = scopePath(operand); + if (!path) continue; + const counterpart = operandKind(args[1 - side], scope); + if (counterpart !== 'number' && counterpart !== 'temporal') continue; + const fn = coercionFor(resolveScopePath(scope, path), counterpart); + if (!fn) continue; + args[side] = wrapInCall(fn, operand as CelNode); + changed = true; + } + } + if (Array.isArray(node.args)) for (const child of node.args) visit(child); + }; + visit(ast); + return changed ? serialize(ast as Parameters[0]) : null; } /** @@ -1303,14 +1464,17 @@ export const celEngine: DialectEngine = { // date/datetime fields (`end_date` → `"2026-06-20"`) on // `record.end_date <= daysFromNow(60)` (#1530), since cel-js compares the // raw string against the `google.protobuf.Timestamp` from `today()` etc. - // Hydrate those strings to number / Date 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. + // Coerce those operands — and ONLY those — and retry ONCE. #7098: the + // coercion is per operand POSITION, not scope-wide, so a comparison that + // already evaluated cleanly is never re-interpreted; the scope itself is + // never rewritten, so a numeric-looking string RETURNED by the expression + // keeps its type too. When no operand provably faulted, or the retry still + // cannot type-check, the original loud error is reported. if (!isNumericOverloadError(err)) throw err; - const hydrated = hydrateOverloadStrings(scope) as Record; + const coercedSource = rewriteFaultedOperands(evalSource, scope); + if (coercedSource === null) throw err; try { - const raw = env.evaluate(evalSource, hydrated); + const raw = env.evaluate(coercedSource, scope); return { ok: true, value: coerce(raw) as T }; } catch { // Hydration did not resolve it — surface the original fault, not the diff --git a/packages/formula/src/cel-overload-retry-scope.test.ts b/packages/formula/src/cel-overload-retry-scope.test.ts new file mode 100644 index 0000000000..81bc54de61 --- /dev/null +++ b/packages/formula/src/cel-overload-retry-scope.test.ts @@ -0,0 +1,179 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7098 — the ADR-0032 §1c retry rewrites the operands that PROVABLY faulted, + * and nothing else. + * + * The companion to `cel-overload-retry-trigger.test.ts`, which pins *when* the + * retry arms (#6679). This file pins what it is allowed to REWRITE once armed. + * + * Before the fix, `hydrateOverloadStrings` rewrote the entire scope on the + * strength of a docblock claim that it "can never change a comparison that + * already evaluated cleanly". The retry knows only that the WHOLE expression + * faulted, so that claim was false: every other comparison in the expression was + * re-interpreted against the hydrated values, and the result was a silently + * wrong answer — `{ ok: true }`, no fault, no log line. + * + * Each case below states the answer evaluation 1 gives for the sub-expression + * that must survive, so a future reader can see what "already evaluated cleanly" + * means for that row. + */ + +import { describe, expect, it } from 'vitest'; +import { celEngine } from './cel-engine'; +import type { Expression } from '@objectstack/spec'; + +const cel = (source: string): Expression => ({ dialect: 'cel', source }); + +describe('§1c retry — scope of the rewrite (#7098)', () => { + describe("the filer's two reproductions", () => { + // `record.s == "5.0"` is string == string: it type-checks and answers TRUE + // in evaluation 1. Only `record.n >= 4` faults. Scope-wide hydration made + // `record.s` the number 5, and `5 == "5.0"` is false across types. + it('keeps a deliberate string equality beside a faulting numeric compare (&&)', () => { + const r = celEngine.evaluate(cel('record.n >= 4 && record.s == "5.0"'), { + record: { n: '7', s: '5.0' }, + }); + expect(r).toEqual({ ok: true, value: true }); + }); + + it('keeps it in the ternary form too — the fault is in the condition', () => { + const r = celEngine.evaluate(cel('record.n >= 4 ? record.s == "5.0" : false'), { + record: { n: '7', s: '5.0' }, + }); + expect(r).toEqual({ ok: true, value: true }); + }); + }); + + describe('the same hazard in the other shapes that reach it', () => { + it('keeps a string membership test (`in` against a string list)', () => { + const r = celEngine.evaluate(cel('record.n >= 4 && record.s in ["5.0", "x"]'), { + record: { n: '7', s: '5.0' }, + }); + expect(r).toEqual({ ok: true, value: true }); + }); + + it('keeps a date-string equality (the ISO half of §1c)', () => { + const r = celEngine.evaluate(cel('record.n >= 4 && record.d == "2026-06-20"'), { + record: { n: '7', d: '2026-06-20' }, + }); + expect(r).toEqual({ ok: true, value: true }); + }); + + it('keeps != as well as ==', () => { + const r = celEngine.evaluate(cel('record.n >= 4 && record.s != "5.0"'), { + record: { n: '7', s: '5.0' }, + }); + expect(r).toEqual({ ok: true, value: false }); + }); + + it('keeps the SAME field readable as a string in one conjunct and a number in another', () => { + // Per operand POSITION, not per field: scope-level narrowing would still + // have had to trade one of these two answers away. + const r = celEngine.evaluate(cel('record.n >= 4 && record.n == "7"'), { + record: { n: '7' }, + }); + expect(r).toEqual({ ok: true, value: true }); + }); + + it('does not retype a numeric string the expression RETURNS', () => { + // Not a boolean at all — a `Field.formula` of type text. Scope-wide + // hydration returned the number 5, so the stored value became "5". + const r = celEngine.evaluate(cel('record.n >= 4 ? record.s : "none"'), { + record: { n: '7', s: '5.0' }, + }); + expect(r).toEqual({ ok: true, value: '5.0' }); + }); + + it('leaves a string concatenation alone (`+` is coercible, the counterpart is not)', () => { + const r = celEngine.evaluate(cel('record.n >= 4 && record.s + "!" == "5.0!"'), { + record: { n: '7', s: '5.0' }, + }); + expect(r).toEqual({ ok: true, value: true }); + }); + }); + + describe('the §1c rescue itself still works (no coverage lost)', () => { + it('rescues the numeric-literal compare (#1534)', () => { + expect(celEngine.evaluate(cel('record.rating >= 4'), { record: { rating: '5.0' } })) + .toEqual({ ok: true, value: true }); + }); + + it('rescues the temporal compare (#1530)', () => { + const r = celEngine.evaluate(cel('record.end_date <= daysFromNow(60)'), { + now: new Date('2026-06-01T00:00:00Z'), + record: { end_date: '2026-06-20' }, + }); + expect(r).toEqual({ ok: true, value: true }); + }); + + it('rescues arithmetic against an int literal', () => { + expect(celEngine.evaluate(cel('record.amount / 100'), { record: { amount: '250000.00' } })) + .toEqual({ ok: true, value: 2500 }); + }); + + it('rescues a field compared against another field that IS a number', () => { + // The counterpart is read off the scope, not off the static type — every + // field is `dyn` under `unlistedVariablesAreDyn`. + expect(celEngine.evaluate(cel('record.rating >= record.threshold'), { + record: { rating: '5.0', threshold: 4 }, + })).toEqual({ ok: true, value: true }); + }); + + it('rescues the operand on the LEFT of the literal too', () => { + expect(celEngine.evaluate(cel('100000 < record.amount'), { record: { amount: '250000.00' } })) + .toEqual({ ok: true, value: true }); + }); + + it('still answers false when the rescued compare is simply unmet', () => { + expect(celEngine.evaluate(cel('record.rating >= 4'), { record: { rating: '2.5' } })) + .toEqual({ ok: true, value: false }); + }); + }); + + describe('the operators that ANSWER instead of faulting are never rewritten', () => { + // Measured on cel-js 8.0.0: `==`, `!=` and `in` are defined ACROSS types, so + // they always had an answer and the retry has no licence to revise it. Each + // row below states that answer, and it is the same whether or not an + // unrelated conjunct faults — which is the property that was missing. + it.each([ + ['record.n == 7', false], + ['record.n != 7', true], + ['record.n in [1, 7]', false], + ] as Array<[string, boolean]>)('%s answers %s on its own', (src, expected) => { + expect(celEngine.evaluate(cel(src), { record: { n: '7' } })) + .toEqual({ ok: true, value: expected }); + }); + + it.each([ + ['record.n >= 4 && record.n == 7', false], + ['record.n >= 4 && record.n != 7', true], + ['record.n >= 4 && record.n in [1, 7]', false], + ] as Array<[string, boolean]>)('%s still answers %s beside a faulting compare', (src, expected) => { + expect(celEngine.evaluate(cel(src), { record: { n: '7' } })) + .toEqual({ ok: true, value: expected }); + }); + }); + + describe('what stays LOUD', () => { + it('does not rescue a non-numeric string', () => { + const r = celEngine.evaluate(cel('record.rating >= 4'), { record: { rating: 'high' } }); + expect(r.ok).toBe(false); + }); + + it('does not rescue a numeric string against a Timestamp (a real mismatch)', () => { + const r = celEngine.evaluate(cel('record.n <= daysFromNow(60)'), { record: { n: '7' } }); + expect(r.ok).toBe(false); + }); + + it('reports the ORIGINAL fault when no operand provably faulted', () => { + // The faulting operand is bound by a comprehension, so the walk cannot read + // its value and declines to guess. Loud beats silently wrong. + const r = celEngine.evaluate(cel('record.items.exists(i, i.price > 100)'), { + record: { items: [{ price: '250.00' }] }, + }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error.message).toMatch(/no such overload/i); + }); + }); +});