Skip to content

Commit 572b914

Browse files
committed
feat(formula): warn on date field == temporal-function silent-miss (#3183)
A `Field.date` deserializes as a "YYYY-MM-DD" string (ADR-0053 Phase 1), and cel-js's equality hard-codes `string == <timestamp>` to false, so `record.due_date == today()` compiles clean but silently never matches. cel-js offers no operator-layer hook to fix the comparison, so this adds a build-time advisory warning (the established ADR-0032 guardrail strategy) that turns the silent runtime miss into a loud, self-correcting build signal. `validateExpression` reuses the shared `ExprSchemaHint.fieldTypes` (the same per-field type map the #1928 tier-4 soundness check already threads through `@objectstack/lint`) to flag `==`/`!=` between a `date` field and today()/daysFromNow()/daysAgo()/now(), with a message pointing at the working idioms (date(...), a range, or daysBetween). Warning severity — the write/validation path may carry a real Date, so it never fails the build. Restricted to `type: 'date'` (unambiguously a string); `datetime` excluded to avoid false positives. Ordering operators already work (they fault→hydrate) and are not flagged. Complements the #1874 flow-scheduling lint, which targets a different root cause (record-change triggers firing only on the exact write day). A runtime fix (normalizing the peer of a temporal operand in the data layer) stays tracked in #3183 — a naive "hydrate date fields to Date" version would trade this silent-miss for another (breaking `dateField == "literal"`). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SuiM565BZ3TR1VD3prMguB
1 parent 1c7ab8b commit 572b914

4 files changed

Lines changed: 237 additions & 2 deletions

File tree

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
"@objectstack/formula": minor
3+
---
4+
5+
feat(formula): warn when a `date` field is compared to a temporal function with `==`/`!=` (#3183)
6+
7+
A `Field.date` deserializes as a `YYYY-MM-DD` **string** (ADR-0053 Phase 1), and
8+
cel-js's equality hard-codes `string == <timestamp>` to `false` — it returns
9+
`false` for a string left operand without ever consulting a registered overload,
10+
and refuses cross-type object equality (`@marcbachmann/cel-js` `overloads.js`
11+
`isEqual`). So the most natural "is it due today" predicate —
12+
13+
```cel
14+
record.due_date == today() // silently false, even when due_date IS today
15+
record.due_date != today() // silently true for a same-day record
16+
```
17+
18+
— compiles clean, throws nothing, and silently never matches. Same silent-miss
19+
family as #1928; **timezone-independent** (fails identically at UTC) and
20+
cross-cutting (formulas, validation, RLS, flow/action/sharing/hook predicates).
21+
22+
cel-js gives no operator-layer hook to fix the comparison, so this adds a
23+
**build-time advisory warning** (the established ADR-0032 guardrail strategy)
24+
rather than a runtime behavior change. `validateExpression` reuses the shared
25+
`ExprSchemaHint.fieldTypes` (the same per-field type map the #1928 tier-4
26+
soundness check already threads through `@objectstack/lint`) to flag a `==`/`!=`
27+
between a `date` field (`record.`/`previous.`/bare) and
28+
`today()`/`daysFromNow()`/`daysAgo()`/`now()`, with a self-correcting message
29+
pointing at the working idioms: `date(record.d) == today()`, a range
30+
(`>= … && <= …`), or `daysBetween(today(), record.d) == 0`.
31+
32+
Warning severity — never fails the build (the write/validation path may carry a
33+
real `Date`). Restricted to `type: 'date'` (unambiguously a string); `datetime`
34+
is excluded to avoid false positives. Ordering operators (`>=`/`<=`/`<`/`>`)
35+
already work — cel-js *throws* for them, tripping the engine's existing
36+
string-hydration retry — so they are not flagged.
37+
38+
A runtime fix (normalizing the peer of a temporal operand in the data layer)
39+
remains tracked in #3183; a naive "hydrate date fields to `Date`" version would
40+
trade this silent-miss for another (breaking `dateField == "2026-06-20"`), so it
41+
needs its own design.

packages/formula/src/validate.test.ts

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,12 @@ describe('validateExpression (ADR-0032)', () => {
200200
expect(validateExpression('value', 'record.amount * 2 - 50', schema).warnings).toHaveLength(0);
201201
});
202202

203-
it('does NOT warn on a date field compared to today()/daysFromNow()', () => {
203+
it('does NOT warn on a date field with an ORDERING comparison (they hydrate at runtime)', () => {
204+
// Ordering ops fault → the engine's string-hydration retry fires → they work.
205+
// (Equality `==`/`!=` does NOT — that is the #3183 silent-miss, covered in its
206+
// own block below; this tier-4 check leaves it to the #3183 guardrail.)
204207
expect(validateExpression('predicate', 'record.due <= daysFromNow(30)', schema).warnings).toHaveLength(0);
205-
expect(validateExpression('predicate', 'record.due == today()', schema).warnings).toHaveLength(0);
208+
expect(validateExpression('predicate', 'record.due >= today()', schema).warnings).toHaveLength(0);
206209
});
207210

208211
it('does NOT warn on a select field ordered against a number (option values may be numeric codes)', () => {
@@ -320,3 +323,61 @@ describe('inferExpressionType — coarse value-type of a formula', () => {
320323
expect(inferExpressionType('undeclared_field + 1')).toBe('unknown'); // bare ref, no fields given
321324
});
322325
});
326+
327+
// #3183 — a `Field.date` reads back as a "YYYY-MM-DD" string, and cel-js's
328+
// equality hard-codes `string == <timestamp>` to false, so `record.due_date ==
329+
// today()` silently never matches. Advisory warning that guides the author to a
330+
// working idiom. Derived from the shared `fieldTypes` hint (type === 'date').
331+
describe('temporal date-equality guardrail (#3183)', () => {
332+
const schema = {
333+
objectName: 'task',
334+
fields: ['due_date', 'status'] as const,
335+
fieldTypes: { due_date: 'date', status: 'text' },
336+
scope: 'record',
337+
} as const;
338+
const dateWarns = (src: string, s: Record<string, unknown> = schema) =>
339+
validateExpression('predicate', src, s as never).warnings
340+
.filter(w => /calendar-day \(date\) field/.test(w.message));
341+
342+
it('warns on `dateField == today()` (field on the left)', () => {
343+
const r = validateExpression('predicate', 'record.due_date == today()', schema);
344+
expect(r.ok).toBe(true); // advisory — never fails the build
345+
expect(r.errors).toHaveLength(0);
346+
const w = r.warnings.filter(x => /calendar-day \(date\) field/.test(x.message));
347+
expect(w).toHaveLength(1);
348+
expect(w[0].message).toMatch(/date\(record\.due_date\) == today\(\)/);
349+
});
350+
351+
it('warns on `today() == dateField` and on daysFromNow/daysAgo/now', () => {
352+
expect(dateWarns('today() == record.due_date')).toHaveLength(1);
353+
expect(dateWarns('record.due_date == daysFromNow(3)')).toHaveLength(1);
354+
expect(dateWarns('record.due_date != daysAgo(7)')).toHaveLength(1);
355+
expect(dateWarns('record.due_date == now()')).toHaveLength(1);
356+
});
357+
358+
it('warns on a bare date-field ref (flattened flow-condition scope)', () => {
359+
const flat = { objectName: 'task', fields: ['due_date'], fieldTypes: { due_date: 'date' } };
360+
expect(dateWarns('due_date == today()', flat)).toHaveLength(1);
361+
});
362+
363+
it('does NOT warn on the working idioms', () => {
364+
expect(dateWarns('date(record.due_date) == today()')).toHaveLength(0);
365+
expect(dateWarns('record.due_date >= today() && record.due_date <= today()')).toHaveLength(0);
366+
expect(dateWarns('daysBetween(today(), record.due_date) == 0')).toHaveLength(0);
367+
});
368+
369+
it('does NOT warn on ordering comparisons (they fault→hydrate and work)', () => {
370+
expect(dateWarns('record.due_date >= today()')).toHaveLength(0);
371+
expect(dateWarns('record.due_date < daysFromNow(30)')).toHaveLength(0);
372+
});
373+
374+
it('does NOT warn on a non-date field, or when fieldTypes is absent', () => {
375+
expect(dateWarns('record.status == today()')).toHaveLength(0); // status is text, not date
376+
expect(dateWarns('record.due_date == today()', { objectName: 'task', fields: ['due_date'], scope: 'record' }))
377+
.toHaveLength(0); // no fieldTypes → check skipped
378+
});
379+
380+
it('de-duplicates repeated references to the same field', () => {
381+
expect(dateWarns('record.due_date == today() || record.due_date == daysFromNow(1)')).toHaveLength(1);
382+
});
383+
});

packages/formula/src/validate.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,56 @@ function typeSoundnessWarning(
144144
};
145145
}
146146

147+
/**
148+
* #3183 — flag `==`/`!=` between a calendar-day (`date`) field and a temporal
149+
* function (`today()`/`daysFromNow()`/`daysAgo()`/`now()`). A `Field.date` reads
150+
* back as a `YYYY-MM-DD` string (ADR-0053 Phase 1), and cel-js's equality
151+
* (`overloads.js` `isEqual`) treats a string and a timestamp as unequal without
152+
* consulting any overload, so the comparison silently never matches. Advisory
153+
* `warning` only — on the write/validation path the value may be a real `Date` —
154+
* and no-op unless `schema.fieldTypes` marks the referenced field `date`. Derives
155+
* the date fields from the shared `fieldTypes` hint (no separate plumbing).
156+
*/
157+
function checkTemporalDateEquality(
158+
source: string,
159+
schema: ExprSchemaHint | undefined,
160+
warnings: ExprValidationError[],
161+
): void {
162+
const fieldTypes = schema?.fieldTypes;
163+
if (!fieldTypes) return;
164+
const seen = new Set<string>();
165+
const flag = (field: string): void => {
166+
if (fieldTypes[field] !== 'date' || seen.has(field)) return;
167+
seen.add(field);
168+
warnings.push({
169+
source,
170+
message:
171+
`\`${field}\` is a calendar-day (date) field, stored as a "YYYY-MM-DD" string, so comparing it to a ` +
172+
`timestamp function with \`==\`/\`!=\` silently never matches (CEL treats a string and a timestamp as ` +
173+
`unequal). Wrap the field to coerce it — \`date(record.${field}) == today()\` — or use a range ` +
174+
`(\`record.${field} >= today() && record.${field} <= today()\`) or \`daysBetween(today(), record.${field}) == 0\`.`,
175+
});
176+
};
177+
let m: RegExpExecArray | null;
178+
TEMPORAL_EQ_LEFT_RE.lastIndex = 0;
179+
while ((m = TEMPORAL_EQ_LEFT_RE.exec(source)) !== null) flag(m[1]);
180+
TEMPORAL_EQ_RIGHT_RE.lastIndex = 0;
181+
while ((m = TEMPORAL_EQ_RIGHT_RE.exec(source)) !== null) flag(m[1]);
182+
}
183+
147184
/** A bare `{x}` that is NOT part of a `{{x}}` mustache hole. */
148185
const SINGLE_BRACE_RE = /(?:^|[^{])\{\s*([A-Za-z_$][\w.$]*)\s*\}(?!\})/;
149186
/** `record.<field>` / `previous.<field>` head references for field-existence. */
150187
const RECORD_REF_RE = /\b(?:record|previous)\.([A-Za-z_$][\w$]*)/g;
188+
// #3183 — a date-field reference (`record.`/`previous.`/bare) compared with
189+
// `==`/`!=` against a calendar-day temporal function, in either operand order.
190+
// Bounded quantifiers only (`[\w$]*`, a single `[^)]*`) → linear-time, no ReDoS.
191+
// The `date(...)`/`datetime(...)`/`timestamp(...)` coercions are NOT in the
192+
// temporal set, so the recommended idiom `date(record.d) == today()` is not matched.
193+
const TEMPORAL_EQ_LEFT_RE =
194+
/(?:record\.|previous\.)?([A-Za-z_$][\w$]*)\s*(?:==|!=)\s*(?:today|daysFromNow|daysAgo|now)\s*\(/g;
195+
const TEMPORAL_EQ_RIGHT_RE =
196+
/(?:today|daysFromNow|daysAgo|now)\s*\([^)]*\)\s*(?:==|!=)\s*(?:record\.|previous\.)?([A-Za-z_$][\w$]*)/g;
151197

152198
/** The dialect a field role expects (Decision 2). */
153199
export function expectedDialect(role: FieldRole): 'cel' | 'template' {
@@ -310,6 +356,10 @@ export function validateExpression(
310356
} else {
311357
checkFieldExistence(source, schema, errors);
312358
checkRoleCatalog(source, schema, errors);
359+
// #3183 — date-field `==`/`!=` a temporal function silently never matches.
360+
// Scope-independent (wrong in both record and flattened sites), so run it
361+
// outside the scope branch below.
362+
checkTemporalDateEquality(source, schema, warnings);
313363
if (schema?.scope === 'record') {
314364
// In a `record`-scoped site a bare top-level identifier is a silent bug —
315365
// it must be `record.<field>` (#1928). Hard error.

packages/lint/src/validate-expressions.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,4 +472,87 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
472472
expect(issues.some(i => /columnName/.test(i.message))).toBe(false);
473473
});
474474
});
475+
476+
// #3183 — `date` field `== today()` silently never matches (cel-js equality).
477+
// The stack validator threads each object's field types into the shared
478+
// validator, which flags the pattern with an advisory (non-blocking) warning.
479+
describe('date-field equality guardrail (#3183)', () => {
480+
const objects = [{
481+
name: 'task',
482+
fields: { due_date: { type: 'date' }, title: { type: 'text' } },
483+
}];
484+
const dateWarns = (issues: { message: string }[]) =>
485+
issues.filter(i => /calendar-day \(date\) field/.test(i.message));
486+
487+
it('warns on a formula field comparing a date field to today()', () => {
488+
const issues = validateStackExpressions({
489+
objects: [{
490+
name: 'task',
491+
fields: {
492+
due_date: { type: 'date' },
493+
is_due_today: { type: 'formula', formula: 'record.due_date == today()' },
494+
},
495+
}],
496+
});
497+
const w = dateWarns(issues);
498+
expect(w).toHaveLength(1);
499+
expect(w[0].severity).toBe('warning');
500+
expect(w[0].where).toMatch(/object 'task'.*formula/);
501+
});
502+
503+
it('warns on a validation-rule predicate, and never fails the build (advisory)', () => {
504+
const issues = validateStackExpressions({
505+
objects: [{
506+
name: 'task',
507+
fields: { due_date: { type: 'date' } },
508+
validations: [{ name: 'due_check', expression: 'record.due_date != today()' }],
509+
}],
510+
});
511+
const w = dateWarns(issues);
512+
expect(w).toHaveLength(1);
513+
expect(w.every(i => i.severity === 'warning')).toBe(true);
514+
});
515+
516+
it('warns on a flattened flow condition using a bare date field', () => {
517+
const issues = validateStackExpressions({
518+
objects,
519+
flows: [{
520+
name: 'due_flow',
521+
nodes: [
522+
{ id: 'start', type: 'start', config: { objectName: 'task' } },
523+
{ id: 'check', type: 'decision', config: { condition: 'due_date == today()' } },
524+
],
525+
edges: [],
526+
}],
527+
});
528+
expect(dateWarns(issues)).toHaveLength(1);
529+
});
530+
531+
it('does not warn on the working idioms or ordering comparisons', () => {
532+
const issues = validateStackExpressions({
533+
objects: [{
534+
name: 'task',
535+
fields: {
536+
due_date: { type: 'date' },
537+
due_soon: { type: 'formula', formula: 'record.due_date >= today() && record.due_date <= daysFromNow(7)' },
538+
is_today: { type: 'formula', formula: 'date(record.due_date) == today()' },
539+
},
540+
}],
541+
});
542+
expect(dateWarns(issues)).toHaveLength(0);
543+
});
544+
545+
it('does not warn on a datetime field (may deserialize as a real instant)', () => {
546+
const issues = validateStackExpressions({
547+
objects: [{
548+
name: 'task',
549+
fields: {
550+
signed_at: { type: 'datetime' },
551+
just_signed: { type: 'formula', formula: 'record.signed_at == now()' },
552+
},
553+
}],
554+
});
555+
expect(dateWarns(issues)).toHaveLength(0);
556+
});
557+
});
475558
});

0 commit comments

Comments
 (0)