Skip to content

Five more adapter-boundary sites cast a driver Date into a declared ISO-string timestamp — MetadataEvent.ts, MetadataHistoryRecord.recordedAt, MetadataRecord.createdAt/updatedAt #14037

Description

@zhuangjianguo

Measured while implementing #13997, in the same two files that card names. Not addressed by it: #13997 canonicalises the authoredAt and MetadataStats.mtime producers only, and deliberately stops at those two declared fields. This card carries the rest of the population. Member of the #13973 census family.

The mechanism, restated once

SqlDriver#formatOutput repairs the builtin audit columns (repairNaiveUtcAuditTimestamp) and folds declared Field.datetime columns (normalizeSqliteDatetimeOutput) only inside its if (this.isSqlite) arm — measured at packages/drivers/driver-sql/src/sql-driver.ts, where the datetimeFields loop sits at brace depth 2 inside the isSqlite arm opened at 15879 and closed at 15968. withPostgresCalendarDayAsText leaves the instant types alone on purpose: "timestamptz / timestamp are deliberately untouched: those are instants, a Date is the right materialisation for them, and Field.datetime depends on it."

So on Postgres and MySQL both column classes — builtin audit columns and declared Field.datetime columns — come out of the record read door as a JS Date. Pinned live in packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts.

⚠️ Worth stating explicitly because #13997 and its triage both assumed otherwise: the declared-Field.datetime half is also SQLite-only. A column being declared Field.datetime does not protect it.

The sites

Each reads a driver row and lands the value in a field declared as an ISO-8601 string. All five are unchecked casts, which is why tsc reports nothing — the string is an assertion about a driver row, never a measurement of one.

site expression declared as
packages/metadata-protocol/src/sys-metadata-repository.ts:1118 (rowToEvent) ts: (row.recorded_at as string) ?? new Date(0).toISOString() MetadataEvent.tsz.string(), packages/metadata-core/src/types.ts:147
packages/metadata/src/loaders/database-loader.ts:996 recordedAt: row.recorded_at as string MetadataHistoryRecord.recordedAtz.string().datetime(), packages/spec/src/system/metadata-persistence.zod.ts:452
packages/metadata/src/loaders/database-loader.ts:1077 recordedAt: row.recorded_at as string same as above
packages/metadata/src/loaders/database-loader.ts:711 (rowToRecord) createdAt: row.created_at as string | undefined MetadataRecord.createdAtz.string().datetime(), same file line 139
packages/metadata/src/loaders/database-loader.ts:713 (rowToRecord) updatedAt: row.updated_at as string | undefined MetadataRecord.updatedAtz.string().datetime(), same file line 141

recorded_at is declared Field.datetime on sys_metadata_history (packages/metadata-core/src/objects/sys-metadata-history.object.ts:175), so it is in datetimeFields and is still straight-through on the live dialects, per the mechanism above.

One lower-confidence neighbour, listed but not claimed: sys-metadata-repository.ts:1056 maps updatedAt: row.updated_at ?? row.created_at ?? null onto an ad-hoc shape with no schema I could locate. Worth a look when this card is picked up; it may be fine.

⚠️ Three of the five are declared z.string().datetime()stricter than the z.string() #13997 dealt with. A Date fails that refinement outright, so any path that ever parses these would reject on the production default driver.

Why nothing reports it

The same two reasons #13997 names, and they hold here unchanged: the row is untyped (or cast), and the declaring schemas are not parsed on these paths. rowToRecord's output in particular is consumed as a typed MetadataRecord throughout database-loader.ts with nothing revalidating it.

Suggested shape (not a decision)

The same route #13997 took: canonicalise at the producer, at the adapter boundary that asserts the declared type. That PR adds a local canonicalIsoInstant helper to each of these two files already, so four of the five sites are a call away. ⛔ Not a tolerant ?? fallback in a consumer (#13973 standing prohibition), and ⛔ not route B (normalising at the driver read door), which reverses a deliberate driver decision and is the maintainer's call for the whole census.

If these land, the two duplicated local helpers are probably worth promoting to one shared export beside the MetadataItem declaration in @objectstack/metadata-core — both packages already depend on it. That consolidation is deliberately left to whoever takes this card rather than done speculatively in #13997.

Re-run

rg -n 'as string' packages/metadata/src/loaders/database-loader.ts | rg 'recorded_at|created_at|updated_at'
rg -n 'recorded_at|created_at|updated_at' packages/metadata-protocol/src/sys-metadata-repository.ts

Backlinks: #13973 (census), #13997 (the two sites already repaired). Neither is addressed here.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions