Skip to content

feat(formula): dateField == today() now matches — AST temporal-comparison rewrite (#3183) #3205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-zhuang merged 1 commit into from
Jul 18, 2026
Merged

feat(formula): dateField == today() now matches — AST temporal-comparison rewrite (#3183) #3205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .changeset/date-equality-runtime-fix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
---
"@objectstack/formula": minor
---

feat(formula): `dateField == today()` now matches — AST temporal-comparison rewrite (#3183)

**Behavior change (the fix):** a `Field.date` compared with `==`/`!=` against a
temporal function now matches on the calendar day. Previously it **silently
returned the wrong answer** — `record.due_date == today()` was always `false`
(and `!= today()` always `true`) even for a same-day record, because a
`Field.date` reads back as a `YYYY-MM-DD` **string** (ADR-0053 Phase 1) and
cel-js's equality (`overloads.js` `isEqual`) treats a string and a timestamp as
unequal without consulting any overload.

`celEngine.evaluate` now rewrites the parsed AST: for each `==`/`!=` whose one
operand is `today()`/`daysFromNow()`/`daysAgo()`/`now()`, the **field operand**
is wrapped in `date(...)` (the stdlib coercion), then the expression is
serialized and evaluated. So `record.due_date == today()` runs as
`date(record.due_date) == today()`.

- **Per-occurrence**, not per-field: `record.d == "2026-06-20" || record.d == today()`
keeps the string-literal comparison intact while fixing the temporal one.
- **Type-blind-safe**: `date()` degrades gracefully — an already-`Date`
(`Field.datetime`) operand passes through; a non-date string or null →
`Invalid Date` → the comparison stays `false`, exactly as before. No
field-type information is needed, and no currently-correct result is worsened.
- **Cheap**: the rewrite only reserializes when such a comparison is present
(a plain-`includes` gate skips the rest), and is memoized per source string.

Applies to every interpreter site — read-time `Field.formula`, default values,
validation rules, hook conditions, and flow conditions — since all route through
`celEngine.evaluate`. RLS/sharing conditions are unaffected: they compile via
`cel-to-filter`, which already rejects function calls as a loud authoring error.

**Supersedes the #3192 advisory lint.** That build-time warning
(`checkTemporalDateEquality`) flagged `dateField == today()` as a silent-miss;
with the runtime fixed it would be a false alarm, so it (and the
`temporalEqualityFields` helper it used) is removed. Authors can now write the
natural `record.due_date == today()` directly; the `date(...)` /
`daysBetween(...) == 0` / range idioms all keep working.
122 changes: 84 additions & 38 deletions packages/formula/src/cel-engine.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, it } from 'vitest';

import { celEngine, temporalEqualityFields } from './cel-engine';
import { celEngine, rewriteTemporalEquality } from './cel-engine';
import { CEL_STDLIB_FUNCTIONS } from './validate';
import type { Expression } from '@objectstack/spec';

Expand Down Expand Up @@ -169,20 +169,23 @@ describe('celEngine', () => {
.toEqual({ ok: true, value: true });
});

it('KNOWN GAP: bare `date-string == today()` silently returns false (cel-js equality)', () => {
// Characterization guard, NOT an endorsement. cel-js's `isEqual`
// (overloads.js) hard-codes `string == X` to false and never consults a
// registered overload, so a bare `Field.date` string compared with `==`
// silently misses — independent of timezone (fails identically at UTC).
// The fix must hydrate date fields to Date in the data layer (where field
// types are known); tracked as a separate follow-up. Authors should use
// the idioms in the test above until then. If this starts returning true,
// the follow-up landed — update/remove this guard.
const now = new Date('2026-11-02T04:30:00Z');
const r = celEngine.evaluate(cel('record.due_date == today()'), {
now, timezone: 'America/New_York', record: { due_date: '2026-11-01' },
});
expect(r).toEqual({ ok: true, value: false });
it('bare `date-string == today()` now matches (the #3183 runtime fix)', () => {
// Previously the KNOWN GAP: cel-js's `isEqual` hard-codes `string == X` to
// false, so a bare `Field.date` string never equalled the Timestamp from
// today(). The engine now rewrites the field operand to `date(record.d)`
// (AST temporal-comparison rewrite, #3183), so it compares two Timestamps
// and matches on the reference-tz calendar day.
const now = new Date('2026-11-02T04:30:00Z'); // Nov 1 in NY
const ny = { now, timezone: 'America/New_York', record: { due_date: '2026-11-01' } };
expect(celEngine.evaluate(cel('record.due_date == today()'), ny))
.toEqual({ ok: true, value: true });
// The `!=` dual is now correctly false for a same-day record.
expect(celEngine.evaluate(cel('record.due_date != today()'), ny))
.toEqual({ ok: true, value: false });
// A different day still compares unequal.
expect(celEngine.evaluate(cel('record.due_date == today()'), {
now, timezone: 'America/New_York', record: { due_date: '2026-10-31' },
})).toEqual({ ok: true, value: false });
});
});

Expand Down Expand Up @@ -511,35 +514,78 @@ describe('celEngine', () => {
});
});

// #3183 — AST walk backing the date-equality guardrail. Returns field names
// compared with `==`/`!=` directly against a temporal function; the validator
// filters these by field type. AST-based, so no ReDoS on adversarial source.
describe('temporalEqualityFields (#3183)', () => {
it('finds the field on either side, for all four temporal functions', () => {
expect(temporalEqualityFields('record.due == today()')).toEqual(['due']);
expect(temporalEqualityFields('today() != record.due')).toEqual(['due']);
expect(temporalEqualityFields('record.due == daysFromNow(3)')).toEqual(['due']);
expect(temporalEqualityFields('record.due != daysAgo(7)')).toEqual(['due']);
expect(temporalEqualityFields('previous.due == now()')).toEqual(['due']);
expect(temporalEqualityFields('due == today()')).toEqual(['due']); // bare (flattened)
// #3183 — AST rewrite backing the runtime date-equality fix: wrap a field
// operand compared with `==`/`!=` against a temporal function in `date(...)`.
describe('rewriteTemporalEquality (#3183)', () => {
it('wraps the field operand on either side, for all four temporal functions', () => {
expect(rewriteTemporalEquality('record.due == today()')).toBe('date(record.due) == today()');
expect(rewriteTemporalEquality('today() != record.due')).toBe('today() != date(record.due)');
expect(rewriteTemporalEquality('record.due == daysFromNow(3)')).toBe('date(record.due) == daysFromNow(3)');
expect(rewriteTemporalEquality('record.due != daysAgo(7)')).toBe('date(record.due) != daysAgo(7)');
expect(rewriteTemporalEquality('previous.due == now()')).toBe('date(previous.due) == now()');
expect(rewriteTemporalEquality('due == today()')).toBe('date(due) == today()'); // bare (flattened)
});

it('leaves the working idioms, ordering comparisons, and non-temporal equality untouched', () => {
for (const src of [
'date(record.due) == today()', // already coerced — idempotent
'record.due >= today()', // ordering (already works)
'daysBetween(today(), record.due) == 0', // integer compare
'record.a == record.b', // no temporal
'record.due == "2026-06-20"', // string literal, no temporal
]) {
expect(rewriteTemporalEquality(src)).toBe(src);
}
});

it('rewrites per-occurrence — a mixed literal+temporal expression keeps the literal intact', () => {
expect(rewriteTemporalEquality('record.d == "2026-06-20" || record.d == today()'))
.toBe('record.d == "2026-06-20" || date(record.d) == today()');
});

it('returns nothing for the working idioms or ordering comparisons', () => {
expect(temporalEqualityFields('date(record.due) == today()')).toEqual([]);
expect(temporalEqualityFields('record.due >= today()')).toEqual([]);
expect(temporalEqualityFields('daysBetween(today(), record.due) == 0')).toEqual([]);
expect(temporalEqualityFields('record.a == record.b')).toEqual([]);
it('returns the source unchanged (no throw) on adversarial input — no ReDoS', () => {
// AST-based + a plain-`includes` gate; the parse either bails or is linear.
expect(rewriteTemporalEquality('$'.repeat(5000))).toBe('$'.repeat(5000));
expect(rewriteTemporalEquality('now('.repeat(2000))).toBe('now('.repeat(2000));
});
});

// #3183 — the end-to-end runtime behavior the rewrite delivers: a `Field.date`
// string operand now matches a temporal function under `==`/`!=`, while string
// literals and already-typed operands are unaffected.
describe('date-string == temporal runtime fix (#3183)', () => {
const now = new Date('2026-06-20T08:00:00Z');
const rec = (due: unknown) => ({ now, record: { due } });

it('a date-only string field == today() matches on the same day', () => {
expect(celEngine.evaluate(cel('record.due == today()'), rec('2026-06-20')))
.toEqual({ ok: true, value: true });
expect(celEngine.evaluate(cel('record.due == today()'), rec('2026-06-19')))
.toEqual({ ok: true, value: false });
// same-day record → `!=` is correctly false (previously silently true)
expect(celEngine.evaluate(cel('record.due != today()'), rec('2026-06-20')))
.toEqual({ ok: true, value: false });
});

it('de-duplicates and finds fields nested in a compound predicate', () => {
expect(temporalEqualityFields('record.due == today() || record.due == daysFromNow(1)')).toEqual(['due']);
expect(temporalEqualityFields('record.a == today() && b != now()').sort()).toEqual(['a', 'b']);
it('a string literal comparison is unchanged, even mixed with a temporal one', () => {
// Pre-existing behavior: string == string literal works.
expect(celEngine.evaluate(cel('record.due == "2026-06-20"'), rec('2026-06-20')))
.toEqual({ ok: true, value: true });
// Mixed: literal clause AND temporal clause both correct for a same-day record.
expect(celEngine.evaluate(cel('record.due == "2026-06-20" || record.due == today()'), rec('2026-06-20')))
.toEqual({ ok: true, value: true });
// Mixed, record on neither day: both clauses false.
expect(celEngine.evaluate(cel('record.due == "2026-06-20" || record.due == today()'), rec('2026-06-18')))
.toEqual({ ok: true, value: false });
});

it('is linear on adversarial input (the CodeQL ReDoS repros) and returns []', () => {
// These would drive the previous regex O(n²); the AST walk parses or bails fast.
expect(temporalEqualityFields('$'.repeat(5000))).toEqual([]);
expect(temporalEqualityFields('now('.repeat(2000))).toEqual([]);
it('an already-Date operand and non-date/null operands are unaffected (graceful date() coercion)', () => {
expect(celEngine.evaluate(cel('record.due == today()'), rec(new Date('2026-06-20T00:00:00Z'))))
.toEqual({ ok: true, value: true });
expect(celEngine.evaluate(cel('record.due == today()'), rec('not-a-date')))
.toEqual({ ok: true, value: false });
expect(celEngine.evaluate(cel('record.due == today()'), rec(null)))
.toEqual({ ok: true, value: false });
});
});
});
87 changes: 68 additions & 19 deletions packages/formula/src/cel-engine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* third-party plugins can't ship runaway predicates.
*/

import { Environment } from '@marcbachmann/cel-js';
import { Environment, serialize } from '@marcbachmann/cel-js';
import type { Expression } from '@objectstack/spec';

import { buildScope, registerNumericCoercions, registerStdLib } from './stdlib';
Expand Down Expand Up @@ -360,36 +360,80 @@ function fieldRefName(node: unknown): string | null {
return null;
}

/** Wrap an AST field-reference node in a `date(...)` call (the stdlib coercion). */
function wrapInDate(node: CelNode): CelNode {
return { op: 'call', args: ['date', [node]] };
}

/**
* #3183 — field names compared with `==`/`!=` DIRECTLY against a temporal
* function (`today()`/`daysFromNow()`/`daysAgo()`/`now()`), found by walking the
* cel-js AST (never a regex on the source — no ReDoS). The caller filters these
* by field type; a `Field.date` reads back as a string and cel-js equality never
* matches it against a timestamp, so such a comparison silently misses. The
* `date(…)`/`datetime(…)`/`timestamp(…)` coercions are NOT temporal calls, so the
* fixed idiom `date(record.d) == today()` yields nothing. Returns `[]` on a parse
* fault (compile() reports those) or when there is no such comparison.
* #3183 — rewrite each `<field> ==/!= <temporal>()` (either operand order) so the
* FIELD operand is coerced with `date(...)`. A `Field.date` reads back as a
* `YYYY-MM-DD` string and cel-js equality never matches a string against the
* Timestamp that `today()` etc. return, so the bare comparison silently misses;
* `date(record.d) == today()` compares two Timestamps and matches on the calendar
* day. The rewrite is:
* - **per-occurrence** — only the operand paired with a temporal call is wrapped,
* so `record.d == "2026-06-20" || record.d == today()` keeps the string-literal
* comparison intact while fixing the temporal one (no field-wide trade-off);
* - **type-blind-safe** — `date()`/`toDate` degrades gracefully (an already-`Date`
* datetime field passes through; a non-date string / null → `Invalid Date` →
* the comparison stays `false`, exactly as today), so no field-type info is
* needed and a currently-correct result is never worsened;
* - **idempotent** — `date(record.d)` is a `call`, not a field ref, so it is not
* re-wrapped.
*
* Returns the (possibly rewritten) source. Only reserializes when a rewrite
* actually happened — the ~99% case that needs no rewrite evaluates the original
* source untouched. Memoized per source string; a parse fault returns the source
* unchanged (compile()/evaluate() report it).
*/
export function temporalEqualityFields(source: string): string[] {
if (typeof source !== 'string' || !source.trim()) return [];
export function rewriteTemporalEquality(source: string): string {
if (typeof source !== 'string' || !source.trim()) return source;
const cached = temporalRewriteCache.get(source);
if (cached !== undefined) return cached;
// Cheap gate: a rewrite needs an equality operator AND a temporal call.
const gated = (source.includes('==') || source.includes('!='))
&& (source.includes('today') || source.includes('daysFromNow')
|| source.includes('daysAgo') || source.includes('now'));
if (!gated) { rememberRewrite(source, source); return source; }

let ast: unknown;
try {
ast = (recordScopeEnv ??= buildScopedEnv([])).parse(source).ast;
} catch {
return [];
rememberRewrite(source, source);
return source;
}
const out = new Set<string>();
let changed = false;
const visit = (node: unknown): void => {
if (!isCelNode(node)) return;
if ((node.op === '==' || node.op === '!=') && Array.isArray(node.args) && node.args.length === 2) {
const [left, right] = node.args;
if (isTemporalCall(left)) { const f = fieldRefName(right); if (f) out.add(f); }
if (isTemporalCall(right)) { const f = fieldRefName(left); if (f) out.add(f); }
const args = node.args as unknown[];
const [left, right] = args;
// Wrap the field operand paired with a temporal call. Guard `fieldRefName`
// so we never wrap a literal, another call, or an arithmetic sub-tree.
if (isTemporalCall(left) && isCelNode(right) && fieldRefName(right)) { args[1] = wrapInDate(right); changed = true; }
else if (isTemporalCall(right) && isCelNode(left) && fieldRefName(left)) { args[0] = wrapInDate(left); changed = true; }
}
if (Array.isArray(node.args)) for (const child of node.args) visit(child);
};
visit(ast);
return [...out];
const out = changed ? serialize(ast as Parameters<typeof serialize>[0]) : source;
rememberRewrite(source, out);
return out;
}

/** Bounded memo of source → temporal-equality-rewritten source (#3183). */
const temporalRewriteCache = new Map<string, string>();
const TEMPORAL_REWRITE_CACHE_MAX = 500;
function rememberRewrite(source: string, rewritten: string): void {
// Simple FIFO cap — expression sources are few and long-lived; this only guards
// against an unbounded set of one-off dynamic strings.
if (temporalRewriteCache.size >= TEMPORAL_REWRITE_CACHE_MAX) {
const first = temporalRewriteCache.keys().next().value;
if (first !== undefined) temporalRewriteCache.delete(first);
}
temporalRewriteCache.set(source, rewritten);
}

/** Coerce cel-js's BigInt-flavored return into spec-friendly JS values. */
Expand Down Expand Up @@ -541,8 +585,13 @@ export const celEngine: DialectEngine = {
try {
const env = buildEnv(now, ctx.timezone ?? 'UTC');
const scope = buildScope(ctx);
// #3183 — coerce a date-field operand compared with `==`/`!=` against a
// temporal function (`date(record.d) == today()`), so a `Field.date` string
// matches the Timestamp instead of silently never equalling it. No-op (and
// no reserialize) for any source without such a comparison.
const evalSource = rewriteTemporalEquality(source);
try {
const raw = env.evaluate(source, scope);
const raw = env.evaluate(evalSource, scope);
return { ok: true, value: coerce(raw) as T };
} catch (err) {
// ADR-0032 §1c — string-serialized fields make CEL raise
Expand All @@ -558,7 +607,7 @@ export const celEngine: DialectEngine = {
if (!isNumericOverloadError(err)) throw err;
const hydrated = hydrateOverloadStrings(scope) as Record<string, unknown>;
try {
const raw = env.evaluate(source, hydrated);
const raw = env.evaluate(evalSource, hydrated);
return { ok: true, value: coerce(raw) as T };
} catch {
// Hydration did not resolve it — surface the original fault, not the
Expand Down
Loading