diff --git a/.changeset/metadata-adapter-boundary-driver-date-to-iso.md b/.changeset/metadata-adapter-boundary-driver-date-to-iso.md new file mode 100644 index 0000000000..987060b361 --- /dev/null +++ b/.changeset/metadata-adapter-boundary-driver-date-to-iso.md @@ -0,0 +1,31 @@ +--- +'@objectstack/metadata': patch +'@objectstack/metadata-protocol': patch +--- + +Five metadata adapter boundaries now emit the ISO-8601 string their declared type promises +when the driver hands them a JS `Date`, instead of asserting `as string` over it + +`MetadataRecord.createdAt` / `.updatedAt` and `MetadataHistoryRecord.recordedAt` are +declared `z.string().datetime()`, and `MetadataEvent.ts` is declared `z.string()`. Four +producers in `DatabaseLoader` (`rowToRecord`, `getHistoryRecord`, `queryHistory`) and one +in `SysMetadataRepository` (`rowToEvent`) reached those fields through an unchecked +`row. as string` cast, which is an assertion about a driver row rather than a +measurement of one — so nothing type-checked and nothing reported it. + +On Postgres and MySQL the assertion is false for both column classes involved. +`SqlDriver#formatOutput` repairs the builtin audit columns and folds declared +`Field.datetime` columns only inside its `if (this.isSqlite)` arm, and +`withPostgresCalendarDayAsText` leaves `timestamptz` / `timestamp` deliberately untouched +because those are instants. A column being declared `Field.datetime` therefore does **not** +protect it: on the production default driver both classes come out of the record read door +as a `Date`, and `.datetime()` is a refinement a `Date` fails outright. Nothing has failed +yet only because no production path parses these values today. + +The repair is producer-side, at the adapter boundary that asserts the declared type — not +a tolerant fallback in a consumer, and not a change at the driver's read door, which would +reverse a deliberate driver decision. Callers keep their existing behaviour for every other +shape: an already-canonical SQLite string passes through byte-identically, an absent column +still yields `undefined` so each caller's `?? ` chain means what it meant, and an +Invalid `Date` is handed through unchanged rather than converted, because what the shared +canonical-ISO spelling should do with that one input is still being decided. diff --git a/packages/metadata-protocol/src/sys-metadata-repository-14037-event-ts-canonicalisation.test.ts b/packages/metadata-protocol/src/sys-metadata-repository-14037-event-ts-canonicalisation.test.ts new file mode 100644 index 0000000000..6bd0121c3f --- /dev/null +++ b/packages/metadata-protocol/src/sys-metadata-repository-14037-event-ts-canonicalisation.test.ts @@ -0,0 +1,267 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14037] `MetadataEvent.ts` is declared `z.string()` — `rowToEvent`, the + * adapter that asserts that declared type over a driver row, must + * canonicalise what the live dialects actually hand it: a JS `Date`. + * + * ## The defect + * + * `rowToEvent` reached `ts` through `(row.recorded_at as string) ?? new + * Date(0).toISOString()`. `row` is `any`, so tsc saw a `string` assignment + * that never happened, and the `??` fires only on nullish — a `Date` walks + * straight past it into the declared field. + * + * `recorded_at` is a declared `Field.datetime` on `sys_metadata_history`, and + * that does NOT protect it: `SqlDriver#formatOutput` folds declared datetime + * columns (`normalizeSqliteDatetimeOutput`) only inside its + * `if (this.isSqlite)` arm, and `withPostgresCalendarDayAsText` leaves + * `timestamptz` / `timestamp` deliberately untouched. Pinned live in + * `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`. + * + * ## Why it matters downstream, not just as a type + * + * The value's one in-repo reader is `MetadataManager.applyRepoEvent`, which + * forwards it verbatim to `MetadataWatchEvent.timestamp` — declared + * `z.string().datetime()` in `packages/spec/src/system/metadata-persistence.zod.ts`. + * So the wrong shape does not stop at this package's boundary; it is carried + * into a field whose refinement a `Date` fails outright. + * + * ## Why the fixture drives a hand-made `Date` + * + * The trap the #13997 sibling in this directory names: a fixture built from a + * hand-made ISO string is already the declared shape before the adapter runs, + * so the assertion and the input share an identity and the case measures + * nothing. Every case here plants the one shape the live dialects produce, and + * carries a non-vacuity guard that the planted value really is a `Date`. + * + * ⛔ No driver dependency: `@objectstack/metadata-protocol` has none and must + * not grow one — the layering runs the other way. + * + * ## What is asserted + * + * `MetadataEventSchema` itself (`@objectstack/metadata-core`), not a + * hand-rolled regex standing in for it. + * + * §C is the #14078 NEUTRALITY pin: an Invalid `Date` must reach the consumer + * UNCHANGED, exactly as this cast passes it through today. The shared + * `canonicalIsoInstant` spelling in this same file would instead raise + * `RangeError: Invalid time value` there — measured reachable on both live + * dialects — and whether it should is the open subject of #14078, which + * #13973 is blocked on. This card imports neither answer, and §C goes red the + * moment someone swaps the contested spelling in. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +// The producer's OWN write-verb dispatch decisions (#4550 delete / #5480 +// update), so the fake engine below cannot accept a call ObjectQL refuses. +// Imported from `@objectstack/metadata-core`, not `@objectstack/objectql`: +// objectql depends on this package, so that import would close a cycle. +import { + assertEngineDeleteDispatch, + assertEngineUpdateDispatch, + assertEngineFindOnePredicate, + MetadataEventSchema, +} from '@objectstack/metadata-core'; +import { SysMetadataRepository } from './sys-metadata-repository.js'; + +interface Row { + [k: string]: unknown; +} + +/** Canonical instant text — exactly what `Date.prototype.toISOString` emits. */ +const ISO_Z = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + +/** + * The instant every case drives, as Postgres and MySQL hand it out. Non-zero + * milliseconds on purpose — `String(date)` and `date.toString()` both drop + * them, so a truncating regression stays observable rather than coinciding + * with the canonical text. + */ +const PG_INSTANT = new Date('2026-03-04T05:06:07.089Z'); + +/** What SQLite hands out for the same instant — already the declared shape. */ +const SQLITE_TEXT = '2026-03-04T05:06:07.089Z'; + +/** + * Minimal engine fake — the same shape the #13997 sibling in this directory + * uses. Stores exactly what it is handed, so a `Date` planted in a row + * survives to the read door the way a live driver's would. + */ +function makeFakeEngine() { + const rows = new Map(); + const historyRows: Row[] = []; + + const keyOf = (w: Record) => + `${String(w.type)}|${String(w.name)}|${String(w.organization_id ?? 'null')}|${String(w.state ?? 'active')}`; + + const findRow = (where: Record) => { + if (where.id !== undefined) { + for (const [k, r] of rows) if (r.id === where.id) return { key: k, row: r }; + return null; + } + const k = keyOf(where); + const r = rows.get(k); + return r ? { key: k, row: r } : null; + }; + + const matchesHistory = (h: Row, where: Record): boolean => + Object.entries(where).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + return v === undefined || h[k] === v; + }); + + return { + rows, + historyRows, + async find(table: string, opts: { where: Record; limit?: number }) { + const matched = + table === 'sys_metadata_history' + ? historyRows.filter((h) => matchesHistory(h, opts.where)) + : Array.from(rows.values()).filter((r) => { + if (opts.where.type && r.type !== opts.where.type) return false; + if ( + opts.where.organization_id !== undefined && + r.organization_id !== opts.where.organization_id + ) + return false; + if (opts.where.state && r.state !== opts.where.state) return false; + return true; + }); + // Hold the caller's bound, AFTER the filter and by PRESENCE — a double + // that silently ignores `limit` answers more rows than the real engine + // would, which is the shape `check:objectql-double-limit` exists to stop. + return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched; + }, + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + if (table === 'sys_metadata_history') + return historyRows.find((h) => matchesHistory(h, opts.where)) ?? null; + return findRow(opts.where)?.row ?? null; + }, + async insert(table: string, data: Record) { + if (table === 'sys_metadata_history') { + const h: Row = { ...data }; + if (!h.id) h.id = `h_${historyRows.length + 1}`; + historyRows.push(h); + return { id: h.id as string }; + } + const k = keyOf(data); + const row: Row = { id: `r_${rows.size + 1}`, ...data }; + rows.set(k, row); + return { id: row.id as string }; + }, + async update(_t: string, data: Record, opts: { where: Record }) { + assertEngineUpdateDispatch(data, opts); + const found = findRow(opts.where); + if (!found) throw new Error('not found'); + rows.set(found.key, { ...found.row, ...data }); + return { id: found.row.id as string }; + }, + async delete(_t: string, opts: { where: Record }) { + assertEngineDeleteDispatch(opts); + const found = findRow(opts.where); + if (!found) return { deleted: 0 }; + rows.delete(found.key); + return { deleted: 1 }; + }, + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); + }, + }; +} + +const view = (label: string) => ({ + name: 'case_grid', + label, + object: 'case', + columns: [{ field: 'name' }], +}); + +describe('#14037 — MetadataEvent.ts is canonical ISO text, whatever the dialect materialised', () => { + let engine: ReturnType; + let repo: SysMetadataRepository; + const ref = { org: 'org_alpha', type: 'view' as const, name: 'case_grid' }; + + const firstEvent = async () => { + for await (const evt of repo.history(ref)) return evt; + return null; + }; + + beforeEach(async () => { + engine = makeFakeEngine(); + repo = new SysMetadataRepository({ + engine, + organizationId: 'org_alpha', + orgLabel: 'org_alpha', + }); + await repo.put(ref, view('A'), { parentVersion: null, actor: 'usr_1' }); + }); + + describe('§A history() — recorded_at, a declared Field.datetime', () => { + it('emits a canonical ISO string when the history row carries a JS Date', async () => { + const historyRow = engine.historyRows[0]!; + historyRow.recorded_at = PG_INSTANT; + + // Non-vacuity guard: a fixture that silently degraded to a string would + // keep this file green while measuring nothing. + expect(historyRow.recorded_at).toBeInstanceOf(Date); + + const evt = await firstEvent(); + expect(evt).not.toBeNull(); + + expect(typeof evt!.ts).toBe('string'); + expect(evt!.ts).toMatch(ISO_Z); + expect(evt!.ts).toBe(PG_INSTANT.toISOString()); + + // The declared contract itself, evaluated against a driver-shaped input. + const parsed = MetadataEventSchema.safeParse(evt); + expect(parsed.success).toBe(true); + }); + + it('passes an already-canonical SQLite string through byte-identically', async () => { + engine.historyRows[0]!.recorded_at = SQLITE_TEXT; + + const evt = await firstEvent(); + + // Idempotent: the dialect that was already correct must not be reshaped. + expect(evt!.ts).toBe(SQLITE_TEXT); + }); + }); + + describe('§B the nullish arm keeps its meaning', () => { + it('still falls back to the epoch when the column is absent', async () => { + delete engine.historyRows[0]!.recorded_at; + + const evt = await firstEvent(); + + expect(evt!.ts).toBe(new Date(0).toISOString()); + }); + }); + + describe('§C #14078 neutrality — an Invalid Date is NOT converted here', () => { + /** + * ⛔ This card does not decide #14078. An Invalid `Date` is measured + * reachable on both live dialects (a MySQL zero datetime; any Postgres + * year in 275760..294276), and whether the shared canonical-ISO spelling + * should throw on it (option A) or fall back to a rendering (option B) is + * a maintainer call across four packages. Until it is ruled, this site + * hands that one shape through exactly as it does today — no new throw, + * no invented rendering. + */ + it('hands the value through unchanged instead of raising RangeError', async () => { + const invalid = new Date(NaN); + expect(Number.isNaN(invalid.getTime())).toBe(true); + // The contested spelling's `Date` arm, on this input, for contrast. + expect(() => invalid.toISOString()).toThrow(RangeError); + + engine.historyRows[0]!.recorded_at = invalid; + + const evt = await firstEvent(); + + // Unchanged — and specifically NOT the `??` fallback, which would mean + // this card had quietly chosen a rendering for the contested shape. + expect(evt!.ts).toBe(invalid as unknown as string); + }); + }); +}); diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index 5c52b34e16..a36742097b 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -122,6 +122,51 @@ function canonicalIsoInstant(value: unknown): string | undefined { return String(value); } +/** + * Canonicalise the ONE driver materialisation {@link + * SysMetadataRepository.rowToEvent} was measured to produce — a valid JS + * `Date` — into the ISO-8601 string `MetadataEvent.ts` is declared as. Every + * other shape is returned UNTOUCHED. + * + * [#14037] `rowToEvent` reaches `ts` through `(row.recorded_at as string) ?? + * …`, and `row` is `any`, so tsc sees a `string` assignment that never + * happened. `recorded_at` is a declared `Field.datetime` on + * `sys_metadata_history`, which the dialect asymmetry above does NOT protect: + * the `datetimeFields` fold sits inside `formatOutput`'s `if (this.isSqlite)` + * arm, so Postgres and MySQL hand the column out as a JS `Date`. + * `MetadataEventSchema.ts` is `z.string()` + * (`packages/metadata-core/src/types.ts`), and the value's one in-repo reader + * — `MetadataManager.applyRepoEvent`, which forwards it to + * `MetadataWatchEvent.timestamp` — is declared `z.string().datetime()`. + * + * ⚠️ Deliberately NOT {@link canonicalIsoInstant} above, and the difference is + * exactly one input shape. That spelling reaches `value.toISOString()` for ANY + * `Date`, which raises `RangeError: Invalid time value` on an Invalid `Date` + * — measured reachable on BOTH live dialects (a MySQL zero datetime; any + * Postgres year in 275760..294276) and the open subject of #14078, which + * #13973 is blocked on. Whether the shared spelling should throw there + * (option A) or fall back to a rendering (option B) is a maintainer call over + * four packages, so this repair imports NEITHER answer into a new call site: + * an Invalid `Date` is returned unchanged, exactly as this cast passes it + * through today. When #14078 rules, this helper collapses into the shared + * spelling. + * + * ⛔ NOT a tolerant fallback (#13973's standing prohibition): it teaches no + * consumer to accept an off-spec shape; it converts one measured producer + * materialisation at the producer. The `Number.isNaN(value.getTime())` guard + * is the spelling already in use at `packages/rest/src/export-format.ts` and + * `packages/rest/src/import-prepare.ts`, not a new one. + * + * A sibling copy serves the four sites in + * `packages/metadata/src/loaders/database-loader.ts`. ⛔ Neither is exported: + * widening `@objectstack/metadata-core`'s public surface for it is a separate + * decision, and #14078 consolidates this family anyway. + */ +function isoFromValidDate(value: unknown): unknown { + if (value instanceof Date && !Number.isNaN(value.getTime())) return value.toISOString(); + return value; +} + /** * Overlay-row lifecycle state. * @@ -1154,7 +1199,7 @@ export class SysMetadataRepository implements MetadataRepository { // the answer is "the platform", not a user literally named 'unknown'. actor: (row.recorded_by as string | null | undefined) ?? null, message: (row.change_note as string | undefined) ?? undefined, - ts: (row.recorded_at as string) ?? new Date(0).toISOString(), + ts: (isoFromValidDate(row.recorded_at) as string) ?? new Date(0).toISOString(), source: (row.source as string | undefined) ?? 'sys-metadata-repo', }; } diff --git a/packages/metadata/src/loaders/database-loader-14037-adapter-boundary-iso.test.ts b/packages/metadata/src/loaders/database-loader-14037-adapter-boundary-iso.test.ts new file mode 100644 index 0000000000..3ce870ca1f --- /dev/null +++ b/packages/metadata/src/loaders/database-loader-14037-adapter-boundary-iso.test.ts @@ -0,0 +1,303 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14037] The four `DatabaseLoader` adapter boundaries that assert a `string` + * over a driver timestamp column must canonicalise what the live dialects + * actually hand them: a JS `Date`. + * + * ## The defect + * + * `rowToRecord` reaches `createdAt` / `updatedAt` through + * `row.created_at as string | undefined`, and both history adapters reach + * `recordedAt` through `row.recorded_at as string`. All four are UNCHECKED + * casts — an assertion about a driver row, never a measurement of one — which + * is why tsc reported nothing. + * + * On Postgres and MySQL the assertion is false for BOTH column classes: + * `SqlDriver#formatOutput` repairs the builtin audit columns + * (`repairNaiveUtcAuditTimestamp`) and folds declared `Field.datetime` columns + * (`normalizeSqliteDatetimeOutput`) only inside its `if (this.isSqlite)` arm, + * and `withPostgresCalendarDayAsText` leaves `timestamptz` / `timestamp` + * deliberately untouched. That dialect fact is pinned live in + * `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`. + * `recorded_at` being a declared `Field.datetime` on `sys_metadata_history` + * does NOT protect it — the fold is inside the SQLite arm too. + * + * All three declarations are `z.string().datetime()` + * (`packages/spec/src/system/metadata-persistence.zod.ts`), a refinement a + * `Date` fails outright. Nothing exploded only because no production path + * parses these: `rowToRecord`'s output is consumed as an already-typed + * `MetadataRecord` with nothing revalidating it. + * + * ## Why the fixtures drive a hand-made `Date` + * + * The same trap the #13997 sibling names: a fixture built from a hand-made + * ISO string is already the declared shape before the adapter runs, so the + * assertion and the input share an identity and the case measures nothing. + * Every case here plants the one shape the live dialects produce and no + * existing fixture ever did, and each carries a non-vacuity guard asserting + * the planted value really is a `Date` before the output is read. + * + * ⛔ No driver dependency: `@objectstack/metadata` has none on `driver-sql` + * and must not grow one. The `Date` is hand-made here for the reason the + * #13567 pin states next door. + * + * ## What is asserted + * + * The declared contracts themselves — `MetadataRecordSchema.safeParse` and + * `MetadataHistoryRecordSchema.safeParse` — not a hand-rolled regex standing + * in for them. A bare `typeof` check would pass for reasons unrelated to the + * `.datetime()` refinement that is the sharp edge here. + * + * §D is the #14078 NEUTRALITY pin and is load-bearing for this card's scope: + * an Invalid `Date` must reach the consumer UNCHANGED, exactly as these casts + * pass it through today. The shared `canonicalIsoInstant` spelling would + * instead raise `RangeError: Invalid time value` there — measured reachable on + * both live dialects — and whether it should is the open subject of #14078, + * which #13973 is blocked on. This card imports neither answer, and §D goes + * red the moment someone swaps the contested spelling in. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { assertEngineFindOnePredicate } from '@objectstack/metadata-core'; +import { + MetadataRecordSchema, + MetadataHistoryRecordSchema, +} from '@objectstack/spec/system'; +import type { IDataEngine } from '@objectstack/spec/contracts'; +import { DatabaseLoader } from './database-loader.js'; + +type Row = Record; + +/** Canonical instant text — exactly what `Date.prototype.toISOString` emits. */ +const ISO_Z = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + +/** + * The instant every case drives, as Postgres and MySQL hand it out. Non-zero + * milliseconds on purpose: `String(date)` and `date.toString()` both drop + * them, so a truncating regression stays observable instead of coinciding + * with the canonical text. + */ +const PG_INSTANT = new Date('2026-03-04T05:06:07.089Z'); + +/** What SQLite hands out for the same instant — already the declared shape. */ +const SQLITE_TEXT = '2026-03-04T05:06:07.089Z'; + +/** + * Minimal engine double. Stores and returns exactly what it is handed — no + * key dropping, no coercion — so a `Date` planted in a row survives to the + * read door the way a live driver's would. + * + * Read verbs only: this file drives no write path, so there is no write-verb + * dispatch to pin (`check:engine-double-contract`). `findOne` asks the + * producer's own #4419 predicate so the double cannot accept a call ObjectQL + * refuses, and `find` applies the caller's `limit` BY PRESENCE and AFTER the + * filter (`check:objectql-double-limit`). + */ +function makeReadEngine(tables: Record) { + const matches = (r: Row, where: Record): boolean => + Object.entries(where ?? {}).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`engine double: unsupported operator ${k}`); + if (v !== null && typeof v === 'object') { + throw new Error(`engine double: unsupported operator object on ${k}`); + } + return v === undefined || r[k] === v; + }); + + const rowsOf = (table: string): Row[] => tables[table] ?? []; + + return { + async find(table: string, opts: { where: Record; limit?: number; offset?: number }) { + const matched = rowsOf(table).filter((r) => matches(r, opts?.where)); + const offset = typeof opts?.offset === 'number' ? opts.offset : 0; + const windowed = matched.slice(offset); + return typeof opts?.limit === 'number' ? windowed.slice(0, opts.limit) : windowed; + }, + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + return rowsOf(table).find((r) => matches(r, opts.where)) ?? null; + }, + async count(table: string, opts: { where: Record }) { + return rowsOf(table).filter((r) => matches(r, opts?.where)).length; + }, + } as unknown as IDataEngine; +} + +/** + * A `sys_metadata` row as the loader reads it. `metadata` is deliberately + * absent: `rowToRecord` folds a missing payload to `{}` without entering the + * stored-item conversion codec, which keeps every case below about the + * timestamp columns rather than about the payload. + */ +function metadataRow(overrides: Row = {}): Row { + return { + id: 'meta_1', + name: 'case_grid', + type: 'view', + namespace: 'default', + managed_by: 'platform', + scope: 'platform', + state: 'active', + version: 3, + checksum: 'sha256-abc', + source: 'database', + created_by: 'usr_1', + updated_by: 'usr_1', + ...overrides, + }; +} + +/** A `sys_metadata_history` row as the loader reads it. */ +function historyRow(overrides: Row = {}): Row { + return { + id: 'hist_1', + name: 'case_grid', + type: 'view', + version: 3, + operation_type: 'update', + metadata: { name: 'case_grid', object: 'case' }, + checksum: 'sha256-abc', + previous_checksum: 'sha256-prev', + change_note: 'tweak', + recorded_by: 'usr_1', + ...overrides, + }; +} + +/** + * Reach `rowToRecord` the way the loader's own callers do. It is private and + * — measured on `origin/main` — no public method returns its `createdAt` / + * `updatedAt`: `load()` reads only `record.checksum`, and `stat()` passes the + * value through `canonicalIsoInstant` before publishing it as + * `MetadataStats.mtime` (#13997). Driving `stat()` would therefore measure + * that helper, not the cast this card is about. + */ +function rowToRecordVia(loader: DatabaseLoader, row: Row) { + return (loader as unknown as { rowToRecord(r: Row): Record }).rowToRecord(row); +} + +describe('#14037 — DatabaseLoader adapter boundaries emit the declared ISO string', () => { + let tables: Record; + let loader: DatabaseLoader; + + beforeEach(() => { + tables = { sys_metadata: [], sys_metadata_history: [] }; + loader = new DatabaseLoader({ engine: makeReadEngine(tables) }); + }); + + describe('§A rowToRecord — createdAt / updatedAt, the BUILTIN audit columns', () => { + it('emits canonical ISO strings when the row carries JS Dates', () => { + const row = metadataRow({ created_at: PG_INSTANT, updated_at: PG_INSTANT }); + + // Non-vacuity: a fixture that degraded to a string would keep this file + // green while measuring the shape that was never broken. + expect(row.created_at).toBeInstanceOf(Date); + expect(row.updated_at).toBeInstanceOf(Date); + + const record = rowToRecordVia(loader, row); + + expect(typeof record.createdAt).toBe('string'); + expect(typeof record.updatedAt).toBe('string'); + expect(record.createdAt).toMatch(ISO_Z); + expect(record.updatedAt).toMatch(ISO_Z); + expect(record.createdAt).toBe(PG_INSTANT.toISOString()); + expect(record.updatedAt).toBe(PG_INSTANT.toISOString()); + + // The declared contract itself. `createdAt` / `updatedAt` are + // `z.string().datetime()` — a refinement a `Date` fails outright. + const parsed = MetadataRecordSchema.safeParse(record); + expect(parsed.success).toBe(true); + }); + + it('passes an already-canonical SQLite string through byte-identically', () => { + const row = metadataRow({ created_at: SQLITE_TEXT, updated_at: SQLITE_TEXT }); + expect(typeof row.created_at).toBe('string'); + + const record = rowToRecordVia(loader, row); + + // Idempotent: the dialect that was already correct must not be reshaped. + expect(record.createdAt).toBe(SQLITE_TEXT); + expect(record.updatedAt).toBe(SQLITE_TEXT); + }); + + it('leaves an absent column absent, so the callers keep their `??` meaning', () => { + const record = rowToRecordVia(loader, metadataRow()); + + expect(record.createdAt).toBeUndefined(); + expect(record.updatedAt).toBeUndefined(); + }); + }); + + describe('§B getHistoryRecord — recordedAt, a declared Field.datetime', () => { + it('emits a canonical ISO string when the history row carries a JS Date', async () => { + const row = historyRow({ recorded_at: PG_INSTANT }); + tables.sys_metadata_history.push(row); + expect(row.recorded_at).toBeInstanceOf(Date); + + const record = await loader.getHistoryRecord('view', 'case_grid', 3); + + expect(record).not.toBeNull(); + expect(typeof record!.recordedAt).toBe('string'); + expect(record!.recordedAt).toMatch(ISO_Z); + expect(record!.recordedAt).toBe(PG_INSTANT.toISOString()); + + const parsed = MetadataHistoryRecordSchema.safeParse(record); + expect(parsed.success).toBe(true); + }); + + it('passes an already-canonical SQLite string through byte-identically', async () => { + tables.sys_metadata_history.push(historyRow({ recorded_at: SQLITE_TEXT })); + + const record = await loader.getHistoryRecord('view', 'case_grid', 3); + + expect(record!.recordedAt).toBe(SQLITE_TEXT); + }); + }); + + describe('§C queryHistory — the same column, the other door', () => { + it('emits a canonical ISO string for every row it maps', async () => { + const rowA = historyRow({ id: 'hist_1', version: 3, recorded_at: PG_INSTANT }); + const rowB = historyRow({ id: 'hist_2', version: 2, recorded_at: PG_INSTANT }); + tables.sys_metadata_history.push(rowA, rowB); + expect(rowA.recorded_at).toBeInstanceOf(Date); + + const page = await loader.queryHistory('view', 'case_grid'); + + expect(page.records).toHaveLength(2); + for (const record of page.records) { + expect(typeof record.recordedAt).toBe('string'); + expect(record.recordedAt).toBe(PG_INSTANT.toISOString()); + expect(MetadataHistoryRecordSchema.safeParse(record).success).toBe(true); + } + }); + }); + + describe('§D #14078 neutrality — an Invalid Date is NOT converted here', () => { + /** + * ⛔ This card does not decide #14078. An Invalid `Date` is measured + * reachable on both live dialects (a MySQL zero datetime; any Postgres + * year in 275760..294276), and whether the shared canonical-ISO spelling + * should throw on it (option A) or fall back to a rendering (option B) is + * a maintainer call across four packages. Until it is ruled, these sites + * hand that one shape through exactly as they do today — no new throw, no + * invented rendering. This case is what makes that a pin rather than a + * claim. + */ + const INVALID = new Date(NaN); + + it('hands the value through unchanged instead of raising RangeError', async () => { + expect(INVALID).toBeInstanceOf(Date); + expect(Number.isNaN(INVALID.getTime())).toBe(true); + // The contested spelling's `Date` arm, on this input, for contrast. + expect(() => INVALID.toISOString()).toThrow(RangeError); + + tables.sys_metadata_history.push(historyRow({ recorded_at: INVALID })); + + const record = await loader.getHistoryRecord('view', 'case_grid', 3); + expect(record!.recordedAt).toBe(INVALID); + + const viaRecord = rowToRecordVia(loader, metadataRow({ updated_at: INVALID })); + expect(viaRecord.updatedAt).toBe(INVALID); + }); + }); +}); diff --git a/packages/metadata/src/loaders/database-loader.ts b/packages/metadata/src/loaders/database-loader.ts index dcc400b85f..d30177e632 100644 --- a/packages/metadata/src/loaders/database-loader.ts +++ b/packages/metadata/src/loaders/database-loader.ts @@ -65,6 +65,54 @@ function canonicalIsoInstant(value: unknown): string | undefined { return String(value); } +/** + * Canonicalise the ONE driver materialisation these adapter boundaries were + * measured to produce — a valid JS `Date` — into the ISO-8601 string the + * declared type promises. Every other shape is returned UNTOUCHED. + * + * [#14037] `rowToRecord` and the two history adapters below each assert a + * `string` over a driver row (`row.created_at as string | undefined`, and so + * on). On Postgres and MySQL that assertion is false: `SqlDriver#formatOutput` + * repairs the BUILTIN audit columns (`repairNaiveUtcAuditTimestamp`) and folds + * declared `Field.datetime` columns (`normalizeSqliteDatetimeOutput`) only + * inside its `if (this.isSqlite)` arm, and `withPostgresCalendarDayAsText` + * leaves `timestamptz` / `timestamp` deliberately untouched. Both column + * classes therefore arrive as a JS `Date` on the live dialects — pinned in + * `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`. + * `MetadataRecord.createdAt` / `.updatedAt` and + * `MetadataHistoryRecord.recordedAt` are declared `z.string().datetime()` + * (`packages/spec/src/system/metadata-persistence.zod.ts`) — a refinement a + * `Date` fails outright. The cast is an assertion about a driver row, never a + * measurement of one, which is why tsc reports nothing. + * + * ⚠️ Deliberately NOT {@link canonicalIsoInstant} above, and the difference is + * exactly one input shape. That spelling reaches `value.toISOString()` for ANY + * `Date`, which raises `RangeError: Invalid time value` on an Invalid `Date` + * — measured reachable on BOTH live dialects (a MySQL zero datetime; any + * Postgres year in 275760..294276) and the open subject of #14078, which + * #13973 is blocked on. Whether the shared spelling should throw there + * (option A) or fall back to a rendering (option B) is a maintainer call over + * four packages, so this repair imports NEITHER answer into five new call + * sites: an Invalid `Date` is returned unchanged, exactly as these casts pass + * it through today. When #14078 rules, this helper collapses into the shared + * spelling. + * + * ⛔ NOT a tolerant fallback (#13973's standing prohibition): it teaches no + * consumer to accept an off-spec shape; it converts one measured producer + * materialisation at the producer. The `Number.isNaN(value.getTime())` guard + * is the spelling already in use at `packages/rest/src/export-format.ts` and + * `packages/rest/src/import-prepare.ts`, not a new one. + * + * A sibling copy serves the single site in + * `packages/metadata-protocol/src/sys-metadata-repository.ts`. ⛔ Neither is + * exported: widening `@objectstack/metadata-core`'s public surface for it is a + * separate decision, and #14078 consolidates this family anyway. + */ +function isoFromValidDate(value: unknown): unknown { + if (value instanceof Date && !Number.isNaN(value.getTime())) return value.toISOString(); + return value; +} + /** * Cache configuration for `DatabaseLoader`. * @@ -743,9 +791,9 @@ export class DatabaseLoader implements MetadataLoader { source: row.source as MetadataRecord['source'], tags: row.tags ? (typeof row.tags === 'string' ? JSON.parse(row.tags as string) : row.tags as string[]) : undefined, createdBy: row.created_by as string | undefined, - createdAt: row.created_at as string | undefined, + createdAt: isoFromValidDate(row.created_at) as string | undefined, updatedBy: row.updated_by as string | undefined, - updatedAt: row.updated_at as string | undefined, + updatedAt: isoFromValidDate(row.updated_at) as string | undefined, }; } @@ -1077,7 +1125,7 @@ export class DatabaseLoader implements MetadataLoader { changeNote: row.change_note as string | undefined, organizationId: row.organization_id as string | undefined, recordedBy: row.recorded_by as string | undefined, - recordedAt: row.recorded_at as string, + recordedAt: isoFromValidDate(row.recorded_at) as string, }; } @@ -1158,7 +1206,7 @@ export class DatabaseLoader implements MetadataLoader { changeNote: row.change_note as string | undefined, organizationId: row.organization_id as string | undefined, recordedBy: row.recorded_by as string | undefined, - recordedAt: row.recorded_at as string, + recordedAt: isoFromValidDate(row.recorded_at) as string, }; }); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index c6b206d61e..04fa955d63 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1531,6 +1531,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/sys-metadata-repository-14037-event-ts-canonicalisation.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/sys-metadata-repository-14037-event-ts-canonicalisation.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/sys-metadata-repository-14037-event-ts-canonicalisation.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/sys-metadata-repository.contract.test.ts", "verb": "delete", @@ -1636,6 +1651,11 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/metadata/src/loaders/database-loader-14037-adapter-boundary-iso.test.ts", + "verb": "findOne", + "pinned": 1 + }, { "file": "packages/metadata/src/loaders/database-loader-update-id-fold-wins.test.ts", "verb": "update",