From 85bfc99eab30b24e5995eae83f987ddd192afd1c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 14:38:39 +0000 Subject: [PATCH] fix(objectql): a CEL `defaultValue` stores the declared type's contract shape, not a raw `Date` (#7373) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `applyFieldDefaults` produces a default three ways and only two honoured the stored-value contract: the `NOW()` token routes through `resolveNowDefault`, a literal is checked against `valueSchemaFor(def, 'stored')` at author time (#7127) — and the expression envelope's result was assigned verbatim. The temporal stdlib returns a JS `Date` (ADR-0053 D1: `today()`/`daysFromNow(n)`/ `daysAgo(n)` are UTC-midnight of the reference-tz calendar day, `now()` the raw instant), so `{ dialect: 'cel', source: 'daysFromNow(7)' }` on a `datetime` put a `Date` OBJECT in the column while `valueSchemaFor` names an ISO-8601 STRING. `validateRecord` accepts a `Date` on `date`/`datetime` by explicit decision, so nothing refused the write and the divergence was silent — while `os migrate value-shapes` walks stored values against that same schema and reports such a row as a violation by the platform's own scan. The expression branch now routes a `Date` result through the SAME per-type table the `NOW()` token uses, rather than growing a second copy of the contract: `datetime` → `YYYY-MM-DDTHH:MM:SS.sssZ`, `date` → `YYYY-MM-DD`, `time` → `HH:MM:SS[.fff]`. Storage on SQL and MongoDB is byte-identical to before: `SqlDriver.formatInput` already coerced a `Date` through `canonicalUtcDatetime`/`toDateOnly`, and mongodb's `storageDatetimeValue`/`storageDateValue` do the same. What changes is the memory driver, which applies its temporal canon to filter comparands only (`coerceTemporalValue`) and stored writes as handed. Same declaration, different stored shape per datasource — the split #4597 / #4560 closed for the `NOW()` token, reappearing on the CEL branch and closed the same way, engine-side. Normalization rather than refusal: refusing a `Date` here would make the rule depend on WHO wrote the value — `validateRecord` accepts one from any caller, temporal types are not in ADR-0104's strict value-shape block, and the documented envelope (#7244) stores correctly on SQL today. Non-`Date` results pass through untouched, as does an `Invalid Date` (the totality the driver canons keep). No day can shift: the ADR-0053 `Date` is UTC-midnight OF the reference-tz day and is read back with UTC getters. Pins in `engine-cel-default-temporal-shape.test.ts` assert the stored value against `valueSchemaFor` itself, and assert the TYPE as well as the text — `JSON.stringify` renders a `Date` as its ISO string, which is what made the original defect read as correct. Reverse-verified: the four subject pins fail against the pre-fix assignment; the four controls (`NOW()` token, literals, non-date CEL results, caller-supplied values) pass in both states. Refs #7373, #7244, ADR-0053, ADR-0104. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012tjfhdVGuYU9KH7SNbor56 --- .../cel-default-temporal-storage-shape.md | 45 +++ .../engine-cel-default-temporal-shape.test.ts | 263 ++++++++++++++++++ packages/objectql/src/engine.ts | 86 +++++- 3 files changed, 388 insertions(+), 6 deletions(-) create mode 100644 .changeset/cel-default-temporal-storage-shape.md create mode 100644 packages/objectql/src/engine-cel-default-temporal-shape.test.ts diff --git a/.changeset/cel-default-temporal-storage-shape.md b/.changeset/cel-default-temporal-storage-shape.md new file mode 100644 index 0000000000..b931a6044c --- /dev/null +++ b/.changeset/cel-default-temporal-storage-shape.md @@ -0,0 +1,45 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql): a CEL `defaultValue` stores the declared type's contract shape instead of a raw `Date` (#7373) + +`applyFieldDefaults` produces a default three ways, and only two of them +honoured the stored-value contract. The `NOW()` token routes through +`resolveNowDefault`, which emits the form the declared type stores; a literal is +checked against `valueSchemaFor(def, 'stored')` at author time (#7127); the +expression envelope's result was assigned **verbatim**. The temporal stdlib +returns a JS `Date` — ADR-0053 D1 fixes `today()` / `daysFromNow(n)` / +`daysAgo(n)` as UTC-midnight of the reference-tz calendar day, and `now()` as +the raw instant — so `{ dialect: 'cel', source: 'daysFromNow(7)' }` on a +`datetime` put a `Date` **object** in the column while `valueSchemaFor` names an +ISO-8601 **string**. Nothing refused the write (`validateRecord` accepts a +`Date` on `date`/`datetime` by explicit decision), so the divergence was silent +— and `os migrate value-shapes`, which walks stored values against that same +schema, reports such a row as a violation by the platform's own scan. + +The expression branch now routes a `Date` result through the same per-type table +the `NOW()` token uses: `datetime` stores `YYYY-MM-DDTHH:MM:SS.sssZ`, `date` +stores `YYYY-MM-DD`, `time` stores `HH:MM:SS[.fff]`. One table, both branches — +not a second copy of the contract. + +**Storage on SQL and MongoDB is byte-identical to before.** Handed a `Date`, +`SqlDriver.formatInput` already coerced it through `canonicalUtcDatetime` +(`toISOString()`) and `toDateOnly`, and mongodb's `storageDatetimeValue` / +`storageDateValue` do the same, so those backends already stored exactly what +the engine now produces. What changes is the memory driver, which applies its +temporal canon to filter comparands only (`coerceTemporalValue`) and stored +writes as handed: it kept the `Date` object. Same declaration, different stored +shape per datasource — the split #4597 / #4560 closed for the `NOW()` token, +reappearing on the CEL branch and now closed the same way, engine-side, so one +answer serves every driver. + +Normalization rather than refusal, because refusing a `Date` here would make the +rule depend on who wrote the value: `validateRecord` accepts one from any +caller, temporal types are not in ADR-0104's strict value-shape block, and the +documented envelope (#7244) stores correctly on SQL today. Non-`Date` results +pass through untouched — a CEL default's result type is otherwise a runtime +concern — as does an `Invalid Date`, keeping the totality the driver canons +have. No calendar day can shift: ADR-0053 D1's `Date` is UTC-midnight *of* the +reference-tz day and is read back with UTC getters, the same `getUTC*` the ADR +names for the driver filter path. diff --git a/packages/objectql/src/engine-cel-default-temporal-shape.test.ts b/packages/objectql/src/engine-cel-default-temporal-shape.test.ts new file mode 100644 index 0000000000..4c1ea9a3e4 --- /dev/null +++ b/packages/objectql/src/engine-cel-default-temporal-shape.test.ts @@ -0,0 +1,263 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7373 — a CEL `defaultValue` stores the DECLARED TYPE's contract shape, not + * the raw `Date` the temporal stdlib returns. + * + * `applyFieldDefaults` produces a default three ways, and only two of them + * honoured the stored-value contract: the `NOW()` token routes through + * `resolveNowDefault`, a literal is checked against `valueSchemaFor(def, + * 'stored')` at author time (#7127) — and the expression envelope's result was + * assigned verbatim. ADR-0053 D1 makes `today()` / `daysFromNow(n)` / + * `daysAgo(n)` return a **JS `Date`** (UTC-midnight of the reference-tz + * calendar day) and `now()` the raw instant, so + * `{ dialect: 'cel', source: 'daysFromNow(7)' }` on a `datetime` put a `Date` + * OBJECT in the column while `valueSchemaFor` names an ISO-8601 string. The + * platform's own `os migrate value-shapes` scan reports such a row as a + * violation. + * + * The assertions below check the stored value against `valueSchemaFor` itself + * rather than against a hand-copied string shape: the contract is what was + * violated, so the contract — not a restatement of it — is what pins the fix. + * `JSON.stringify` renders a `Date` as its ISO string, which is exactly why + * the original defect read as correct; every pin here therefore asserts the + * TYPE as well as the text. + * + * The driver is a store-as-handed stub, which is the memory driver's observable + * behaviour (it applies its temporal canon to filter comparands only) and the + * one backend where the defect is visible: `SqlDriver.formatInput` coerces a + * `Date` through `canonicalUtcDatetime`/`toDateOnly` at the wire and mongodb's + * `storageDatetimeValue`/`storageDateValue` do the same, so those two already + * stored the shape this change now produces engine-side for everyone. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { valueSchemaFor } from '@objectstack/spec/data'; +import { ObjectQL } from './engine.js'; + +const cel = (source: string) => ({ dialect: 'cel' as const, source }); + +/** + * One object carrying every branch that matters side by side, so a control and + * its subject are defaulted by the SAME insert and cannot drift apart through + * two differently-configured rigs. + */ +const DEFAULTED = { + name: 'cel_default_probe', + label: 'CEL Default Probe', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + + // ── subjects: CEL defaults whose result is a `Date` ────────────── + dt_now: { name: 'dt_now', label: 'dt now', type: 'datetime' as const, defaultValue: cel('now()') }, + dt_days: { name: 'dt_days', label: 'dt days', type: 'datetime' as const, defaultValue: cel('daysFromNow(7)') }, + dt_today: { name: 'dt_today', label: 'dt today', type: 'datetime' as const, defaultValue: cel('today()') }, + d_today: { name: 'd_today', label: 'd today', type: 'date' as const, defaultValue: cel('today()') }, + d_days: { name: 'd_days', label: 'd days', type: 'date' as const, defaultValue: cel('daysAgo(3)') }, + t_now: { name: 't_now', label: 't now', type: 'time' as const, defaultValue: cel('now()') }, + + // ── controls: the `NOW()` token, byte-identical before and after ── + tok_dt: { name: 'tok_dt', label: 'tok dt', type: 'datetime' as const, defaultValue: 'NOW()' }, + tok_d: { name: 'tok_d', label: 'tok d', type: 'date' as const, defaultValue: 'NOW()' }, + tok_t: { name: 'tok_t', label: 'tok t', type: 'time' as const, defaultValue: 'NOW()' }, + + // ── controls: literals, untouched by this path ──────────────────── + lit_txt: { name: 'lit_txt', label: 'lit txt', type: 'text' as const, defaultValue: 'plain' }, + lit_dt: { + name: 'lit_dt', label: 'lit dt', type: 'datetime' as const, + defaultValue: '2020-01-02T03:04:05.678Z', + }, + + // ── controls: CEL results that are NOT dates, passed through ────── + cel_str: { name: 'cel_str', label: 'cel str', type: 'text' as const, defaultValue: cel("'hello'") }, + cel_num: { name: 'cel_num', label: 'cel num', type: 'number' as const, defaultValue: cel('1 + 2') }, + cel_bool: { name: 'cel_bool', label: 'cel bool', type: 'boolean' as const, defaultValue: cel('true') }, + }, +}; + +/** + * A driver that stores exactly what the engine hands it. Deliberately WITHOUT + * the temporal coercion the SQL/mongodb drivers apply on write: those repair a + * `Date` at the wire, which is precisely what hid this defect on SQL-backed + * stores. Storing as-handed is what makes the engine's own output observable. + */ +function makeStoreAsHandedDriver() { + const rows = new Map>(); + let nextId = 0; + const driver = { + name: 'memory', + version: '0.0.0', + supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, + async find() { return Array.from(rows.values()).map((r) => ({ ...r })); }, + async findOne() { for (const r of rows.values()) return { ...r }; return null; }, + async create(_object: string, data: Record) { + nextId += 1; + const id = (data.id as string) ?? `r_${nextId}`; + const row = { ...data, id }; + rows.set(id, row); + return { ...row }; + }, + async update(_object: string, id: string, data: Record) { + const cur = rows.get(id); + if (!cur) return null; + const next = { ...cur, ...data, id }; + rows.set(id, next); + return { ...next }; + }, + async updateMany() { return 0; }, + async upsert(object: string, data: Record) { return this.create(object, data); }, + async delete(_object: string, id: string) { return rows.delete(id); }, + async count() { return rows.size; }, + async bulkCreate(object: string, batch: Record[]) { + const out: Record[] = []; + for (const r of batch) out.push(await this.create(object, r)); + return out; + }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, rows }; +} + +async function makeEngine() { + const engine = new ObjectQL(); + const rig = makeStoreAsHandedDriver(); + engine.registerDriver(rig.driver as never, true); + await engine.init(); + engine.registry.registerObject(DEFAULTED as never); + return { engine, ...rig }; +} + +/** The stored row after one defaults-only insert. */ +async function insertDefaulted( + context?: Record, +): Promise> { + const { engine, rows } = await makeEngine(); + await (engine as unknown as { + insert(o: string, d: unknown, opts?: unknown): Promise; + }).insert('cel_default_probe', {}, context ? { context } : undefined); + return Array.from(rows.values())[0]; +} + +/** Assert a stored value satisfies its field's own ADR-0104 stored contract. */ +function expectStoredShape(value: unknown, type: string): void { + const parsed = valueSchemaFor({ type }, 'stored').safeParse(value); + expect( + parsed.success ? null : `${type}: ${parsed.error.issues[0]?.message} (got ${Object.prototype.toString.call(value)})`, + ).toBeNull(); +} + +// A fixed instant whose UTC calendar day and its Los_Angeles calendar day are +// DIFFERENT days: 2026-08-10T05:00Z is 2026-08-09 22:00 in America/Los_Angeles. +// Every reference-tz assertion below turns on that gap. +const PINNED_NOW = new Date('2026-08-10T05:00:00.000Z'); + +describe('#7373 — a CEL `defaultValue` stores the declared type\'s contract shape', () => { + beforeEach(() => { + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(PINNED_NOW); + }); + afterEach(() => { vi.useRealTimers(); }); + + it('stores an ISO-8601 STRING on `datetime`, never a `Date` object', async () => { + const row = await insertDefaulted(); + + for (const field of ['dt_now', 'dt_days', 'dt_today'] as const) { + // The defect precisely: the value was a `Date`, which JSON.stringify + // renders as the right text — so assert the type, not just the text. + expect(row[field]).not.toBeInstanceOf(Date); + expect(typeof row[field]).toBe('string'); + expectStoredShape(row[field], 'datetime'); + } + + // …and the instants themselves are the ones CEL computed. + expect(row.dt_now).toBe('2026-08-10T05:00:00.000Z'); + expect(row.dt_days).toBe('2026-08-17T00:00:00.000Z'); // UTC-midnight calendar day + 7 + }); + + it('stores `YYYY-MM-DD` on `date`, never a `Date` object', async () => { + const row = await insertDefaulted(); + + for (const field of ['d_today', 'd_days'] as const) { + expect(row[field]).not.toBeInstanceOf(Date); + expect(typeof row[field]).toBe('string'); + expectStoredShape(row[field], 'date'); + } + + expect(row.d_today).toBe('2026-08-10'); + expect(row.d_days).toBe('2026-08-07'); + }); + + it('stores a wall clock on `time`, never a `Date` object', async () => { + const row = await insertDefaulted(); + expect(row.t_now).not.toBeInstanceOf(Date); + expectStoredShape(row.t_now, 'time'); + expect(row.t_now).toBe('05:00:00'); + }); + + /** + * The day-shift guard. ADR-0053 D1 fixes `today()` as UTC-midnight OF the + * reference-tz calendar day, so the serialization must read the parts back + * with UTC getters — the same `getUTC*` the ADR names for the driver filter + * path. Reading them in LOCAL time is the move that shifts a day, and this + * is the case that would catch it: at the pinned instant the UTC day is the + * 10th while the Los_Angeles day is the 9th. + */ + it('keeps the REFERENCE-TZ calendar day on `date` — no off-by-one', async () => { + const row = await insertDefaulted({ isSystem: true, timezone: 'America/Los_Angeles' }); + + expect(row.d_today).toBe('2026-08-09'); // the LA day, not the UTC 10th + expectStoredShape(row.d_today, 'date'); + + // The same reference day, one week out, on a `datetime`: still the LA day + // at UTC-midnight, so the calendar arithmetic and the serialization agree. + expect(row.dt_days).toBe('2026-08-16T00:00:00.000Z'); + }); + + it('leaves the `NOW()` token byte-identical (control)', async () => { + const row = await insertDefaulted(); + + // Exactly `resolveNowDefault`'s table — unchanged by this fix, which is the + // point: the CEL branch now shares that table rather than owning a copy. + expect(row.tok_dt).toBe('2026-08-10T05:00:00.000Z'); + expect(row.tok_d).toBe('2026-08-10'); + expect(row.tok_t).toBe('05:00:00'); + for (const [f, t] of [['tok_dt', 'datetime'], ['tok_d', 'date'], ['tok_t', 'time']] as const) { + expect(row[f]).not.toBeInstanceOf(Date); + expectStoredShape(row[f], t); + } + }); + + it('leaves LITERAL defaults untouched (control)', async () => { + const row = await insertDefaulted(); + expect(row.lit_txt).toBe('plain'); + expect(row.lit_dt).toBe('2020-01-02T03:04:05.678Z'); + }); + + it('passes NON-date CEL results through unchanged (control)', async () => { + const row = await insertDefaulted(); + + // Normalization is scoped to temporal FORM; a CEL default's result type is + // otherwise a runtime concern and must not be rewritten. + expect(row.cel_str).toBe('hello'); + expect(row.cel_num).toBe(3); + expect(row.cel_bool).toBe(true); + }); + + it('does not touch a value the caller supplied explicitly', async () => { + const { engine, rows } = await makeEngine(); + const explicit = new Date('2001-02-03T04:05:06.007Z'); + await (engine as unknown as { + insert(o: string, d: unknown): Promise; + }).insert('cel_default_probe', { dt_now: explicit }); + const row = Array.from(rows.values())[0]; + + // Defaults apply only to an omitted/null slot. A caller-supplied `Date` is + // the drivers' business (SQL/mongodb coerce it at the wire), not this + // path's — narrowing the change to what the issue measured. + expect(row.dt_now).toBe(explicit); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 980716a574..5d25647e90 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -2571,11 +2571,19 @@ export class ObjectQL implements IObjectQLEngine { * that opts into `NOW()` gets the instant, which is what the SQL column * DEFAULT gives it today. * - * `now` is the caller's per-insert snapshot, so two defaulted fields on one - * record cannot straddle a millisecond boundary. - */ - private resolveNowDefault(fieldType: unknown, now: Date): string { - const iso = now.toISOString(); // YYYY-MM-DDTHH:MM:SS.sssZ + * `instant` is the caller's per-insert `now` snapshot when the `NOW()` token + * is what asked, so two defaulted fields on one record cannot straddle a + * millisecond boundary. + * + * It is NOT only the token's, though (#7373): this is the one place that + * knows what shape a declared type stores an instant in, so the CEL branch's + * {@link normalizeExpressionDefault} routes its own `Date` results through + * the SAME table rather than growing a second copy of it. Hence the + * parameter is an arbitrary `instant`, not "now" — `daysFromNow(7)` is a + * week out and takes the identical per-type treatment. + */ + private resolveNowDefault(fieldType: unknown, instant: Date): string { + const iso = instant.toISOString(); // YYYY-MM-DDTHH:MM:SS.sssZ if (fieldType === 'date') return iso.slice(0, 10); if (fieldType === 'time') { const timeOfDay = iso.slice(11, 23); // HH:MM:SS.fff @@ -2587,6 +2595,69 @@ export class ObjectQL implements IObjectQLEngine { return iso; } + /** + * Put a CEL `defaultValue`'s evaluated result into the storage shape the + * field's declared type contracts for (#7373). + * + * The counterpart the expression branch was missing. `applyFieldDefaults` + * has three ways to produce a default and, before this, only two of them + * honoured the stored-value contract: the `NOW()` token routed through + * {@link resolveNowDefault}, a literal is checked against + * `valueSchemaFor(def, 'stored')` at AUTHOR time (#7127), and the expression + * envelope's result was assigned verbatim. But the temporal stdlib returns a + * JS `Date` — ADR-0053 D1 fixes `today()` / `daysFromNow(n)` / `daysAgo(n)` + * as UTC-midnight of the reference-tz calendar day, and `now()` as the raw + * instant — while `valueSchemaFor` says a stored instant is an ISO-8601 + * STRING. So `{ dialect: 'cel', source: 'daysFromNow(7)' }` on a `datetime` + * put a `Date` object in the column: a value the platform's own + * `os migrate value-shapes` scan reports as a violation. + * + * Why NORMALIZE rather than refuse the `Date` — the two shapes this could + * have taken, decided on measurement: + * + * - **The drivers already agree with this answer.** Handed a `Date`, + * `SqlDriver.formatInput` coerces it through `canonicalUtcDatetime` + * (`toISOString()`) and `toDateOnly` (`YYYY-MM-DD`); mongodb's + * `storageDatetimeValue` keeps the BSON `Date`, which is what parsing the + * ISO string produces anyway, and its `storageDateValue` collapses to the + * same `YYYY-MM-DD`. Normalizing here is therefore byte-identical on + * every SQL and mongodb-backed store and changes exactly one thing: the + * memory driver, which applies its temporal canon to filter comparands + * only (`coerceTemporalValue`) and stores writes as handed. That is the + * whole defect, and this is the smallest change that closes it. + * - **Refusal would single out one writer.** `validateRecord` accepts a + * `Date` on `date`/`datetime` from ANY caller by explicit decision + * (`if (value instanceof Date) return null`), and temporal types are not + * in ADR-0104's strict value-shape block at all. Refusing a `Date` only + * when a CEL default produced it would make the rule depend on who wrote + * the value rather than on what the value is — and would break the + * documented envelope (#7244) on precisely the SQL backends where it + * stores correctly today. + * + * This is the `NOW()` crack of #4597 / #4560 in its third form: a value the + * SQL driver silently repaired at the wire and non-SQL datasources did not, + * so one declaration stored two shapes depending on the datasource. Resolved + * engine-side, one answer serves every driver; the driver coercions stay as + * defence in depth for writes that bypass the engine. + * + * No day can shift here. ADR-0053 D1's `Date` is UTC-midnight OF the + * reference-tz calendar day, and {@link resolveNowDefault} reads it back + * with `toISOString()` — UTC getters, the same `getUTC*` reading the ADR + * names for the driver filter path. Reading those parts in LOCAL time is the + * move that would shift a day, and nothing here does it. + * + * Non-`Date` results (a string, a number, a bool, a list) pass through + * untouched: this normalizes temporal FORM, it does not police the contract, + * and a CEL default is documented as "result type is a runtime concern". + * An `Invalid Date` passes through too — the same totality the driver + * canons keep, so a value nothing can interpret is never silently rewritten + * into a wrong one. + */ + private normalizeExpressionDefault(fieldType: unknown, value: unknown): unknown { + if (!(value instanceof Date) || Number.isNaN(value.getTime())) return value; + return this.resolveNowDefault(fieldType, value); + } + /** * Build a HookContext.api: a ScopedContext that hooks can use to * read/write other objects within the same execution context. @@ -2751,7 +2822,10 @@ export class ObjectQL implements IObjectQLEngine { extra: { object }, }); if (result.ok) { - out[f.name] = result.value as unknown; + // Normalized to the declared type's stored shape, never assigned + // verbatim: the temporal stdlib returns a `Date` and the contract + // names an ISO-8601 string (#7373 — {@link normalizeExpressionDefault}). + out[f.name] = this.normalizeExpressionDefault(f.type, result.value); } else { this.logger.warn('Failed to evaluate default expression', { object, field: f.name, error: result.error,