diff --git a/.changeset/iso-from-valid-date-family-collapse.md b/.changeset/iso-from-valid-date-family-collapse.md new file mode 100644 index 0000000000..e5ecf7c10a --- /dev/null +++ b/.changeset/iso-from-valid-date-family-collapse.md @@ -0,0 +1,108 @@ +--- +"@objectstack/metadata": minor +"@objectstack/metadata-protocol": patch +--- + +fix(metadata): four `isoFromValidDate` call sites collapse onto the shared canonical-ISO spelling; `MetadataHistoryRecord.recordedAt` gets the terminal value it never had (#16422) + +## What was wrong + +`#14037`/`#14038` landed a narrow per-site helper, `isoFromValidDate`, beside +the shared `canonicalIsoInstant` spelling. It rewrote exactly one shape — a +valid JS `Date` becomes ISO text — and handed **every other input back +untouched**. Four adapter boundaries used it, and each fed a field declared +`z.string()` or `z.string().datetime()`: + +| site | declared as | +|:--|:--| +| `SysMetadataRepository.rowToEvent` → `MetadataEvent.ts` | `z.string()` | +| `DatabaseLoader.rowToRecord` → `MetadataRecord.createdAt` / `.updatedAt` | `z.string().datetime().optional()` | +| `DatabaseLoader.getHistoryRecord` → `MetadataHistoryRecord.recordedAt` | `z.string().datetime()` — **required** | +| `DatabaseLoader.queryHistory` → the same field, the other door | `z.string().datetime()` — **required** | + +So a `null`, a `number`, an opaque column and an Invalid `Date` all arrived at a +field declared `string`, each wearing an `as string` / `as string | undefined` +cast that asserted the opposite. Measured over the seven inputs that +distinguish the two helpers, the declared schemas refused **21 of 35** produced +values. + +`recordedAt` was the sharp end: a REQUIRED `z.string().datetime()` for which +none of the three available answers was legal — the visible text +`"Invalid Date"` fails the refinement, `undefined` fails the required field, and +the pass-through fed it the `Date` object, which fails both. + +## What it does now + +Those four sites read `canonicalIsoInstant`, whose return type **is** +`string | undefined`, so all four casts are deleted rather than restated. Both +sibling definitions of `isoFromValidDate` are gone. The terminal value is chosen +per site, from the site's own declared schema: + +- `MetadataRecord.createdAt` / `.updatedAt` are `.optional()` → `undefined`, the + branch an absent column already took. ⛔ No default is invented for a field the + schema lets be absent. +- `MetadataHistoryRecord.recordedAt` is required → the **epoch**, via a named + `recordedAtFallback()` shared by both history doors. ⛔ Not `new Date()`: a + `now` stamp is a plausible-looking recording instant nobody measured, and it + sorts a version recorded years ago to the top of a newest-first timeline. The + epoch invents no fact and sorts to the oldest end. It is also the answer the + sibling reader of this same `sys_metadata_history.recorded_at` column already + gives (`rowToEvent` and `history()`, both `?? new Date(0).toISOString()`). + +Schema refusals over the same seven inputs: **21 → 8**. The eight that remain +are a `number` and an opaque object at four sites — shapes no driver is measured +to materialise for these columns. They now arrive as the declared *type* (a +string) that simply is not a valid datetime, so the producer's bug stays visible +instead of being papered over. + +## One behaviour change worth reading twice — and it is why this is `minor` + +`DatabaseLoader.stat()` computes `record.updatedAt ?? record.createdAt`. An +Invalid `updated_at` used to WIN that `??` — a `Date` is truthy and not nullish — +so a row with an unreadable `updated_at` and a good `created_at` published +`new Date()` as its `mtime`. It now folds to `undefined` one step earlier and +loses the `??`, so the row publishes its `created_at`: a stored instant in place +of a fabricated one, and exactly the "same `?? DEFAULT` chain an absent column +takes" that `#14078`'s own ruling text prescribes for the shape. + +⚠️ **The old answer was LEGAL.** `new Date().toISOString()` satisfies +`MetadataStats.mtime`'s `z.string().datetime()` perfectly well, and the +pre-existing pin asserted exactly that. So this one site is **not** the repair of +a violation — it is one legal published answer replaced by a different legal +published answer on a published read verb. Nothing was refused before and is +permitted now; a consumer simply receives a different instant. + +## Why the two levels differ + +- **`@objectstack/metadata` — `minor`.** Its four repaired sites, on their own, + are the "repairing an implementation that silently violated its own already + published declared type" case: the values that changed there are ones + `MetadataRecordSchema` / `MetadataHistoryRecordSchema` already refused, and + nothing a consumer legitimately received has moved. But this package also + carries `stat()`, and that site changes a **legal** published answer, which the + paragraph above measures. The level is per package, so the four repaired sites + ride along at `minor`. +- **`@objectstack/metadata-protocol` — `patch`.** Neither of its two sites moves + a legal published answer. `rowToEvent` only stops emitting values + `MetadataEventSchema` refused (a `Date`, a `number`, an opaque object in a + field declared `z.string()`), and `listCommits` is byte-identical on all seven + probe inputs. + +⛔ No declared type narrowed, no export was added or removed (neither helper was +ever exported), and no envelope or accept set moved — so this is `minor` by the +changed-answer row, not a breaking change, and it carries no ADR-0087 +disposition. + +## What deliberately did NOT collapse + +`listCommits` in `@objectstack/metadata-protocol` keeps its copy. Its docblock +promises callers the RAW value back for a non-`Date`, and the shared spelling +rewrites the whole domain: swapping it in would ERASE an Invalid `Date` from the +response (`undefined` — the one answer ADR-0053 D-F3 refuses, because it silently +drops a value that is on disk) and hand a `number` or an opaque object to the +commit-timeline sort as `String(value)` rather than verbatim. Measured, that site +is byte-identical on all seven inputs before and after this change. + +`SqlDriver`'s same-named helper is not part of this family at all: it takes +`Date` (not `unknown`), both its call sites narrow with `instanceof Date` first, +and it is the PRODUCER-side fold ADR-0053 D-F3 governs. It is untouched. diff --git a/packages/metadata-protocol/src/protocol-14038-list-commits-created-at-iso.test.ts b/packages/metadata-protocol/src/protocol-14038-list-commits-created-at-iso.test.ts index 99554acd99..be053845a0 100644 --- a/packages/metadata-protocol/src/protocol-14038-list-commits-created-at-iso.test.ts +++ b/packages/metadata-protocol/src/protocol-14038-list-commits-created-at-iso.test.ts @@ -52,10 +52,19 @@ * now total, answering `undefined` for the shape. This card's route is * unchanged: `isoFromValidDate` in `protocol.ts` converts the ONE measured * shape (a valid `Date`) and returns every other shape — including an Invalid - * `Date` — UNCHANGED, which is what `listCommits` promises its callers. §D - * below stays the pin on that promise: it goes red the moment anyone swaps - * the other spelling into this site, now the separately-tracked consolidation - * decision #16422. + * `Date` — UNCHANGED, which is what `listCommits` promises its callers. + * + * ⚠️ #16422 has now RULED the consolidation, and this site was held OUT of it + * on the strength of that promise. The card collapsed the family's other four + * call sites into `canonicalIsoInstant` and deleted both sibling definitions; + * `protocol.ts` keeps its copy, and §D below is no longer a placeholder for a + * pending decision but the standing pin on a decided one. Measured across the + * seven inputs that distinguish the two helpers, this site is byte-identical + * before and after that card — the swap here would have ERASED an Invalid + * `Date` from the response (`undefined`, the one answer [ADR-0053 D-F3] + * refuses) and handed a `number` or an opaque object to + * `compareAuditInstants` as `String(value)` instead of verbatim, reordering + * rows this seam deliberately leaves alone. * * ## Reverse verification, direction predicted BEFORE running * @@ -180,7 +189,12 @@ describe('[#14038] listCommits emits the ISO-8601 string createdAt is declared a * invented rendering. This case is what makes that a PIN rather than * a claim: it goes red the moment `canonicalIsoInstant` (or any * spelling that reaches `.toISOString()` unconditionally) is swapped - * into `listCommits`. The consolidation is #16422. + * into `listCommits`. + * + * ⚠️ #16422 ruled the consolidation and held this site OUT of it, so + * this pin now guards a DECIDED contract rather than an open one. It + * stays exactly as written — the only pin of the three that did not + * need rewriting, because the behaviour it asserts did not move. */ it('hands the value through unchanged instead of raising RangeError', async () => { const invalid = new Date(NaN); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 5b194ac438..3d7ca6399e 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -1811,18 +1811,33 @@ function compareAuditInstants(a: unknown, b: unknown): number { * declared `string` return type. * * ⚠️ Deliberately NOT the `canonicalIsoInstant` spelling next door in - * `sys-metadata-repository.ts` / `database-loader.ts` (#14037's sibling - * sites). That difference used to be exactly one input shape — the Invalid + * `sys-metadata-repository.ts` / `database-loader.ts` — which is, since + * #16422, the ONLY spelling at #14037's sibling sites. That difference used + * to be exactly one input shape — the Invalid * `Date` on which that spelling raised `RangeError: Invalid time value`, * measured reachable on BOTH live dialects (a MySQL zero datetime; any * Postgres year in 275760..294276). #14078 has since RULED it (option B, * 2026-09-02): that arm is now total and answers `undefined` for the shape. * - * ⛔ They are still not ONE spelling, and this copy has the strongest reason - * of the three not to be collapsed — see the paragraph below on what - * `listCommits` promises its callers for a non-`Date` value. The - * consolidation is tracked as **#16422**; #14078 ruled only the five arms - * that THREW. + * ⛔ They are still not ONE spelling, and **#16422 ruled that this copy is the + * one that stays**. That card collapsed the family's other four call sites — + * `rowToEvent` in `sys-metadata-repository.ts` and the three adapter + * boundaries in `database-loader.ts` — into `canonicalIsoInstant` and deleted + * both sibling definitions of this spelling. This site was held out, for the + * reason the last paragraph below states: `listCommits` promises its callers + * the RAW value back for a non-`Date`, and `canonicalIsoInstant` rewrites the + * whole domain. Measured on the seven inputs that distinguish the two + * helpers, swapping it in here moves three: an Invalid `Date` would be ERASED + * from the response (`undefined` — the one answer [ADR-0053 D-F3] refuses, + * because it silently drops a value that is on disk), and a `number` and an + * opaque object would reach {@link compareAuditInstants} as `String(value)` + * rather than verbatim, reordering rows this seam deliberately leaves alone. + * + * ⇒ The family is now two DELIBERATE helpers, not one pending merge: the + * shared domain rewrite at the sites whose declared field is a + * `z.string().datetime()` and whose caller carries a terminal value, and this + * narrow one-shape conversion at the site whose declared contract is + * pass-through. ⛔ Do not collapse it without superseding that ruling. * * ⛔ NOT a tolerant fallback (#13973's standing prohibition): it teaches no * consumer to accept an off-spec shape; it converts the one measured @@ -1837,8 +1852,9 @@ function compareAuditInstants(a: unknown, b: unknown): number { * valid `Date` — an absent/opaque column must still reach `sort`'s fallback * branch and any in-process reader exactly as before. Consolidating the * family's near-identical copies was expected to be #14078's call; that - * ruling covered only the five arms that threw, so the consolidation is - * tracked separately as #16422. + * ruling covered only the five arms that threw, and #16422 then ruled this + * promise the reason to keep this copy rather than the obstacle to removing + * it. §D of `protocol-14038-list-commits-created-at-iso.test.ts` is the pin. */ function isoFromValidDate(value: unknown): unknown { if (value instanceof Date && !Number.isNaN(value.getTime())) return value.toISOString(); 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 index b32bdd746e..482e4ec945 100644 --- 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 @@ -52,15 +52,21 @@ * `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. #14078 has since - * ruled (option B, 2026-09-02) and `canonicalIsoInstant` in this same file is - * now TOTAL — it answers `undefined` for that shape rather than raising - * `RangeError: Invalid time value`. The two helpers still differ across the - * REST of the input domain, so §C keeps its job unchanged: it goes red the - * moment someone swaps the other spelling into this site, which is now the - * separately-tracked consolidation decision #16422 rather than an open - * ruling. + * §C WAS the #14078 neutrality pin — "an Invalid `Date` must reach the + * consumer UNCHANGED, exactly as this cast passes it through" — written to go + * red the moment anyone swapped the shared spelling into this site. #16422 + * made that swap DELIBERATELY, so §C is rewritten as the RULED pin rather + * than kept or deleted: it now asserts the terminal value the ruling chose, + * and it still goes red if anyone reverts to the pass-through, because the + * shape that reaches `MetadataEvent.ts` under that spelling is a `Date` + * object in a field declared `z.string()`. + * + * ⚠️ The rewrite is the point, not a formality. The neutrality pin existed so + * the swap could not happen by accident; its evidence — the seven-input + * before/after matrix in #16422's PR — is what discharges it. `rowToEvent` + * now reads `canonicalIsoInstant(row.recorded_at) ?? new Date(0).toISOString()`, + * the same spelling and the same terminal value `history()` already used for + * `authoredAt` off this very column. */ import { describe, it, expect, beforeEach } from 'vitest'; @@ -250,29 +256,57 @@ describe('#14037 — MetadataEvent.ts is canonical ISO text, whatever the dialec }); }); - describe('§C #14078 neutrality — an Invalid Date is NOT converted here', () => { + describe('§C [#16422] RULED — an Invalid Date takes the epoch, the branch an absent column takes', () => { /** - * ⛔ 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. + * This section was the #14078 NEUTRALITY pin: it asserted that this site + * hands an Invalid `Date` through UNCHANGED, and it was written to go red + * on exactly the swap #16422 then performed. It is rewritten, not + * deleted, because the swap was deliberate and now has its own evidence. + * + * What the ruling decided, per call site: `rowToEvent` reads + * `canonicalIsoInstant(row.recorded_at) ?? new Date(0).toISOString()`. An + * Invalid `Date` folds to `undefined` — #14078's own total `Date` arm — + * and therefore takes the `??` branch an ABSENT column already took (§B), + * which is also the answer `history()` gives for `authoredAt` off this + * same column. + * + * ⛔ Why the old behaviour could not stay: `MetadataEvent.ts` is declared + * `z.string()` (`@objectstack/metadata-core`) and its one in-repo reader + * forwards it to `MetadataWatchEvent.timestamp`, a `z.string().datetime()`. + * The pass-through put a `Date` OBJECT in that field, so + * `MetadataEventSchema` refused the event this adapter produced — asserted + * below rather than described, by parsing the same fixture both ways. */ - it('hands the value through unchanged instead of raising RangeError', async () => { + it('answers the epoch and produces an event the declared schema accepts', async () => { const invalid = new Date(NaN); expect(Number.isNaN(invalid.getTime())).toBe(true); - // The contested spelling's `Date` arm, on this input, for contrast. + // Non-vacuity: the shape really is the one with no canonical text. 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); + // The ruled terminal value — the same one §B's absent column takes. + expect(evt!.ts).toBe(new Date(0).toISOString()); + expect(typeof evt!.ts).toBe('string'); + + // ⛔ And specifically NOT the retired pass-through, which is what the + // neutrality version of this section asserted. + expect(evt!.ts).not.toBe(invalid as unknown as string); + expect(evt!.ts).not.toBeInstanceOf(Date); + + // The declared contract, which the pass-through could not satisfy. + const parsed = MetadataEventSchema.safeParse(evt); + expect(parsed.success, JSON.stringify((parsed as { error?: { issues: unknown } }).error?.issues)).toBe(true); + }); + + it('rejects the retired shape, so a revert to pass-through cannot pass silently', () => { + // The exact object the pass-through used to emit, checked against the + // declared schema in isolation. This is why the swap was not cosmetic. + const passThrough = { seq: 1, op: 'update', ref, hash: null, parentHash: null, + actor: null, ts: new Date(NaN), source: 'sys-metadata-repo' }; + expect(MetadataEventSchema.safeParse(passThrough).success).toBe(false); }); }); }); diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index 11c3269c89..ce0567fc46 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -143,8 +143,9 @@ import { isWritablePackage } from './package-writability.js'; * * The terminal value is chosen **per call site**, and this one's is * `undefined`: every caller already carries such a chain — `getByHash` and - * `rowToItem` end in `?? new Date(...).toISOString()`, `listDrafts` (#14938) - * in `?? null` — the branch an absent column takes at each of them today. + * `rowToItem` end in `?? new Date(...).toISOString()`, `rowToEvent` (#16422) + * in `?? new Date(0).toISOString()`, `listDrafts` (#14938) in `?? null` — + * the branch an absent column takes at each of them today. * The ruling assigns `undefined` exactly where "the field is optional and the * caller already carries a `?? default` chain". ⛔ NOT the visible text * `"Invalid Date"` — the fields fed from here are read by machines @@ -152,6 +153,20 @@ import { isWritablePackage } from './package-writability.js'; * `z.string().datetime()` field), so that text would move the failure to a zod * refusal at the consumer instead of removing it. ⛔ And NOT a blanket `''`: a silent blank is the shape that * hides the producer's bug. + * + * ## [#16422] This file now has ONE spelling + * + * #14037's per-site `isoFromValidDate` stood beside this one until #16422 and + * is gone: it rewrote a single shape (valid `Date` -> ISO) and handed every + * other input back untouched, so `rowToEvent` fed a `Date`, a `number` and an + * opaque column straight into `MetadataEvent.ts`, declared `z.string()`. + * Measured across seven inputs, `MetadataEventSchema` refused four of them. + * The two helpers were never interchangeable — #14078 aligned them on the + * Invalid-`Date` shape ALONE — so the collapse was a per-call-site decision + * with a terminal value each, not the mechanical swap the retired docblocks + * implied. `listCommits` in `protocol.ts` keeps its own copy on purpose: it + * promises its callers the RAW value back, a contract this domain rewrite + * would reverse. */ function canonicalIsoInstant(value: unknown): string | undefined { if (value === null || value === undefined) return undefined; @@ -164,62 +179,6 @@ 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`, and the dialect asymmetry described above did not - * protect it: the `datetimeFields` fold sat inside `formatOutput`'s - * `if (this.isSqlite)` arm, so Postgres and MySQL handed the column out as a JS - * `Date`. #13973 ([ADR-0053 D-F1]) has since closed that asymmetry — the fold - * runs on every dialect — but the cast is still an assertion rather than a - * measurement, and the `Date` domain is not empty: an INVALID `Date` still - * leaves `driver-sql` unchanged ([ADR-0053 D-F3]) and non-SQL drivers - * materialise their own. `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. That difference used - * to be exactly one input shape — the Invalid `Date` on which that spelling - * raised `RangeError: Invalid time value`, measured reachable on BOTH live - * dialects (a MySQL zero datetime; any Postgres year in 275760..294276). - * #14078 has since RULED it (option B, 2026-09-02): that arm is now total and - * answers `undefined` for the shape, so the two agree on it. - * - * ⛔ They are still not ONE spelling, which is why #14078 did not collapse - * this helper into it. `canonicalIsoInstant` returns `string | undefined` and - * rewrites the whole domain (nullish -> `undefined`; anything neither `Date` - * nor string -> `String(value)`), while this one returns `unknown` and hands - * every non-valid-`Date` shape back UNTOUCHED. The consolidation is its own - * decision — **#16422** — because it moves six call sites for `null`, for a - * `number` and for an opaque column, one of which (`MetadataHistoryRecord - * .recordedAt`, a REQUIRED `z.string().datetime()`) has no terminal value - * either half of the #14078 ruling supplies. §C of - * `sys-metadata-repository-14037-event-ts-canonicalisation.test.ts` pins the - * behaviour this paragraph describes. - * - * ⛔ 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. * @@ -1278,7 +1237,16 @@ 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: (isoFromValidDate(row.recorded_at) as string) ?? new Date(0).toISOString(), + // [#16422] The shared spelling, not the per-site `isoFromValidDate` this + // card retired. Two things change at this site and both were measured: + // the `as string` cast is gone (`canonicalIsoInstant` RETURNS + // `string | undefined`, so the declared type is now a measurement), and + // an Invalid `Date` folds to `undefined` and takes the epoch below — + // the branch an absent column already took, and the same answer + // `history()` gives for `authoredAt` off this very column. Before the + // collapse that shape reached `MetadataEvent.ts` — declared `z.string()` + // — as a `Date` object, and `MetadataEventSchema` refused the event. + ts: canonicalIsoInstant(row.recorded_at) ?? 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 index 2183d87266..2fc7d86ce7 100644 --- 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 @@ -58,15 +58,22 @@ * 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. #14078 has since ruled (option B, 2026-09-02) and the - * shared `canonicalIsoInstant` spelling is now TOTAL — it answers `undefined` - * for that shape rather than raising `RangeError: Invalid time value`. The two - * helpers still differ across the REST of the input domain, so §D keeps its - * job unchanged: it goes red the moment someone swaps the other spelling in, - * which is now the separately-tracked consolidation decision #16422 rather - * than an open ruling. + * §D WAS the #14078 neutrality pin — "an Invalid `Date` must reach the + * consumer UNCHANGED, exactly as these casts pass it through" — written to go + * red the moment anyone swapped the shared spelling in. #16422 made that swap + * DELIBERATELY, so §D is rewritten as the RULED pin rather than kept or + * deleted, and it now asserts the terminal value chosen at each boundary. + * They are NOT one value: `MetadataRecord.createdAt` / `.updatedAt` are + * `.optional()` and take `undefined`, while `MetadataHistoryRecord.recordedAt` + * is REQUIRED and takes the epoch from `recordedAtFallback()` — the site this + * card was filed for, and the one with no legal terminal value at all before + * the change. + * + * ⚠️ The casts those lines carried (`as string | undefined`, `as string`) are + * gone, not restated: `canonicalIsoInstant` RETURNS `string | undefined`, so + * the declared type is a measurement now. §A/§B/§C are unchanged — a valid + * `Date` and a canonical string were never shapes the two helpers disagreed + * on. */ import { describe, it, expect, beforeEach } from 'vitest'; @@ -283,32 +290,81 @@ describe('#14037 — DatabaseLoader adapter boundaries emit the declared ISO str }); }); - describe('§D #14078 neutrality — an Invalid Date is NOT converted here', () => { + describe('§D [#16422] RULED — the terminal value per site, and the schemas now accept it', () => { /** - * ⛔ 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. + * This section was the #14078 NEUTRALITY pin, asserting these boundaries + * hand an Invalid `Date` through UNCHANGED. It was written to go red on + * exactly the swap #16422 then performed, and is rewritten rather than + * deleted because that swap was deliberate and carries its own evidence. + * + * The card exists because `MetadataHistoryRecord.recordedAt` is a REQUIRED + * `z.string().datetime()` for which NONE of the three candidate answers + * was legal: the visible text `"Invalid Date"` fails the refinement, + * `undefined` fails the required field, and the pass-through fed it a + * `Date` object, which fails both. The third answer is a caller-side + * default — `recordedAtFallback()`, the epoch — and this is its pin. + * + * `MetadataRecord.createdAt` / `.updatedAt` are `.optional()`, so their + * terminal value is `undefined` and no default is invented there. Two + * different answers on purpose; a single one would have been the tell that + * nobody followed each site to its declared schema. */ const INVALID = new Date(NaN); - it('hands the value through unchanged instead of raising RangeError', async () => { + it('recordedAt: the epoch, and MetadataHistoryRecordSchema now accepts the record', async () => { expect(INVALID).toBeInstanceOf(Date); expect(Number.isNaN(INVALID.getTime())).toBe(true); - // The contested spelling's `Date` arm, on this input, for contrast. + // Non-vacuity: the shape really is the one with no canonical text. 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); + expect(record!.recordedAt).toBe(new Date(0).toISOString()); + // ⛔ NOT the retired pass-through, which is what this section used to + // assert and what the declared schema refused. + expect(record!.recordedAt).not.toBe(INVALID as unknown as string); + + const parsed = MetadataHistoryRecordSchema.safeParse(record); + expect(parsed.success, JSON.stringify((parsed as { error?: { issues: unknown } }).error?.issues)).toBe(true); + }); + + it('queryHistory takes the same terminal value — the two doors do not drift', async () => { + tables.sys_metadata_history.push(historyRow({ recorded_at: INVALID })); + + const { records } = await loader.queryHistory('view', 'case_grid'); + + expect(records[0].recordedAt).toBe(new Date(0).toISOString()); + expect(MetadataHistoryRecordSchema.safeParse(records[0]).success).toBe(true); + }); + + it('createdAt / updatedAt: `undefined`, because the declared field is optional', () => { + const viaRecord = rowToRecordVia(loader, metadataRow({ created_at: INVALID, updated_at: INVALID })); + + expect(viaRecord.updatedAt).toBeUndefined(); + expect(viaRecord.createdAt).toBeUndefined(); + // ⛔ Specifically NOT the epoch: inventing a creation instant for a + // field the schema lets be absent would be a fabricated fact. + expect(viaRecord.updatedAt).not.toBe(new Date(0).toISOString()); + + expect(MetadataRecordSchema.safeParse(viaRecord).success).toBe(true); + }); + + it('a `null` column reaches the same terminal values — not just the Invalid `Date`', async () => { + // The neutrality version measured ONE shape. The collapse moved four, + // and `null` is the one a reader is most likely to assume was already + // handled: it used to arrive as a literal `null` in fields declared + // `string | undefined`, which both schemas refused. + tables.sys_metadata_history.push(historyRow({ recorded_at: null })); + const record = await loader.getHistoryRecord('view', 'case_grid', 3); + expect(record!.recordedAt).toBe(new Date(0).toISOString()); + expect(MetadataHistoryRecordSchema.safeParse(record).success).toBe(true); + + const viaRecord = rowToRecordVia(loader, metadataRow({ created_at: null, updated_at: null })); + expect(viaRecord.createdAt).toBeUndefined(); + expect(viaRecord.updatedAt).toBeUndefined(); + expect(MetadataRecordSchema.safeParse(viaRecord).success).toBe(true); }); }); }); diff --git a/packages/metadata/src/loaders/database-loader-14078-invalid-date-total-arm.test.ts b/packages/metadata/src/loaders/database-loader-14078-invalid-date-total-arm.test.ts index 44a346e050..0fdc531459 100644 --- a/packages/metadata/src/loaders/database-loader-14078-invalid-date-total-arm.test.ts +++ b/packages/metadata/src/loaders/database-loader-14078-invalid-date-total-arm.test.ts @@ -33,14 +33,30 @@ * cell, it would produce a zod refusal at the consumer — the same 500 moved * one layer out. ⛔ And not `''`, the silent blank the ruling forbids. * - * ## The composition this file also pins + * ## The composition this file also pins — REWRITTEN by #16422 * - * `stat()` reads `record.updatedAt` out of `rowToRecord`, which canonicalises - * through the SEPARATE `isoFromValidDate` helper (#14037) — and that helper - * hands an Invalid `Date` through UNCHANGED by design. So the bad value - * reaches `canonicalIsoInstant` as a `Date`, not as text, and this arm is the - * one that has to be total. §C states that as an assertion so the two helpers - * cannot drift apart unnoticed. + * `stat()` reads `record.updatedAt` out of `rowToRecord`. That step used to + * canonicalise through the SEPARATE `isoFromValidDate` helper (#14037), which + * handed an Invalid `Date` through UNCHANGED, so the bad value reached + * `canonicalIsoInstant` here as a `Date` and THIS arm was the one that had to + * be total. #16422 collapsed `rowToRecord` onto `canonicalIsoInstant` itself + * and deleted that helper, so the fold now happens one step EARLIER and + * `stat()` receives `undefined`. + * + * ⚠️ Two consequences, both asserted below rather than described: + * + * 1. `stat()`'s arm is still live and still total — it is the nullish arm + * that answers now, not the `Date` arm. §A's first case is unchanged and + * still publishes a `mtime` the declared schema accepts. + * 2. `record.updatedAt ?? record.createdAt` resolves DIFFERENTLY for a row + * whose `updated_at` is unreadable and whose `created_at` is good. The + * Invalid `Date` used to win that `??` — a `Date` is truthy and not + * nullish — and the row published `new Date()`; it now loses it and the + * row publishes `created_at`. That is a real, deliberate behaviour change + * on a published read surface: a stored instant replacing a fabricated + * one, and exactly the "same chain an absent column takes" that #14078's + * own ruling text prescribes for the shape. §A's second case is rewritten + * to assert it, ⛔ not left to fail and ⛔ not deleted. */ import { describe, it, expect, beforeEach } from 'vitest'; @@ -144,17 +160,46 @@ describe('[#14078] DatabaseLoader.stat — an Invalid Date yields the caller fal expect(parsed.success, JSON.stringify((parsed as { error?: { issues: unknown } }).error?.issues)).toBe(true); }); - it('falls back for updated_at alone, without letting `??` mistake it for absent', async () => { + it('[#16422] an unreadable updated_at now falls through to created_at, a stored instant', async () => { const bad = new Date(NaN); assertOldSpellingWouldThrow(bad); - // `canonicalIsoInstant(record.updatedAt ?? record.createdAt)` — an - // Invalid `Date` is NOT nullish, so `??` does not fall through to - // `created_at`. The guard, not the `??`, is what makes this total. + // BEFORE #16422 this case asserted the opposite: `rowToRecord` handed + // the Invalid `Date` straight through, so `record.updatedAt ?? + // record.createdAt` saw a truthy, non-nullish `Date`, the `??` did not + // fall through, and `stat` published `new Date()`. + // + // AFTER #16422 `rowToRecord` folds it to `undefined` — #14078's own + // ruled answer for the shape — so the `??` DOES fall through and the + // row publishes its `created_at`. The change is deliberate and is the + // better answer: `new Date()` claimed the record had been modified at + // read time, a fact nobody measured, while `created_at` is an instant + // actually on disk. ⛔ It is not a widening: an Invalid `updated_at` is + // indistinguishable from an absent one to every reader of `mtime`, and + // #14078's ruling text prescribes exactly "the same `?? ` chain + // an absent column takes". const stats = await loaderFor(metadataRow({ updated_at: bad, created_at: PG_INSTANT })).stat('view', 'case_grid'); - expect(stats!.mtime).not.toBe(PG_INSTANT.toISOString()); + expect(stats!.mtime).toBe(PG_INSTANT.toISOString()); + expect(stats!.mtime).toMatch(ISO_Z); + expect(MetadataStatsSchema.safeParse(stats).success).toBe(true); + }); + + it('[#16422] with BOTH columns unreadable the caller default still answers', async () => { + const bad = new Date(NaN); + assertOldSpellingWouldThrow(bad); + + // Nothing to fall through to, so `stat`'s own `?? new Date().toISOString()` + // is what answers — the arm that has to be total, still reached, just by + // the nullish input rather than by the `Date` one. + const before = Date.now(); + const stats = await loaderFor(metadataRow({ updated_at: bad, created_at: bad })).stat('view', 'case_grid'); + const after = Date.now(); + expect(stats!.mtime).toMatch(ISO_Z); + const stamped = Date.parse(stats!.mtime!); + expect(stamped).toBeGreaterThanOrEqual(before); + expect(stamped).toBeLessThanOrEqual(after); expect(MetadataStatsSchema.safeParse(stats).success).toBe(true); }); }); @@ -171,23 +216,39 @@ describe('[#14078] DatabaseLoader.stat — an Invalid Date yields the caller fal }); }); - describe('§C the composition with #14037 `isoFromValidDate`, stated as an assertion', () => { - it('hands the Invalid Date to this arm as a Date — the other helper does NOT convert it', async () => { + describe('§C [#16422] the composition after the collapse, stated as an assertion', () => { + it('rowToRecord absorbs the Invalid Date FIRST — this arm then answers for `undefined`', async () => { const bad = new Date(NaN); const loader = loaderFor(metadataRow({ updated_at: bad })); - // `rowToRecord` is the step before `stat`'s own; it routes through the - // separate `isoFromValidDate` helper, which passes this shape through - // untouched. If that ever changes, the arm under test stops being the - // one that has to be total and this file should be re-read. + // `rowToRecord` is the step before `stat`'s own. Until #16422 it routed + // through the separate `isoFromValidDate` helper and passed this shape + // through untouched, so `stat` received a `Date`. It now routes through + // `canonicalIsoInstant` itself, so `stat` receives `undefined` and the + // arm under test is reached by the NULLISH input instead. Asserted, not + // assumed: if the fold ever moves again, this is where it shows. const record = (loader as unknown as { rowToRecord(r: Row): Record }) .rowToRecord(metadataRow({ updated_at: bad })); - expect(record.updatedAt).toBe(bad); - expect(record.updatedAt).toBeInstanceOf(Date); + expect(record.updatedAt).toBeUndefined(); + expect(record.updatedAt).not.toBeInstanceOf(Date); - // …and the total arm is what turns that into a contract-satisfying stat. + // …and `stat`'s own `?? new Date().toISOString()` is what turns that + // into a contract-satisfying stat. Still total, still live, ⛔ not dead: + // a non-SQL driver materialises its own `Date`s and `stat` is reachable + // with a `Date` from every row shape `rowToRecord` does not own. const stats = await loader.stat('view', 'case_grid'); expect(MetadataStatsSchema.safeParse(stats).success).toBe(true); }); + + it('the `Date` arm of this helper is still exercised directly, by the collapsed sites', () => { + // ⛔ The point of the assertion above is NOT that the `Date` arm became + // unnecessary. #16422 moved it: `rowToRecord` is now the caller that + // hands a `Date` to `canonicalIsoInstant`, so the arm runs one frame + // earlier and its output is what `stat` reads. + const loader = loaderFor(metadataRow({})); + const record = (loader as unknown as { rowToRecord(r: Row): Record }) + .rowToRecord(metadataRow({ updated_at: PG_INSTANT })); + expect(record.updatedAt).toBe(PG_INSTANT.toISOString()); + }); }); }); diff --git a/packages/metadata/src/loaders/database-loader.test.ts b/packages/metadata/src/loaders/database-loader.test.ts index 8f7e58f950..0dd22855b8 100644 --- a/packages/metadata/src/loaders/database-loader.test.ts +++ b/packages/metadata/src/loaders/database-loader.test.ts @@ -1529,10 +1529,14 @@ describe('MetadataManager auto-configuration', () => { * `driver-sql` hands an INVALID `Date` through unchanged ([ADR-0053 D-F3]), * and non-SQL drivers materialise their own. * - * ⚠️ `rowToRecord` reaches `createdAt` / `updatedAt` through an unchecked - * `row.created_at as string | undefined` cast, so the `string` in - * `MetadataRecord` is an assertion about a driver row and never a measurement - * of one — which is why tsc reported nothing. + * ⚠️ `rowToRecord` USED to reach `createdAt` / `updatedAt` through an + * unchecked `row.created_at as string | undefined` cast, so the `string` in + * `MetadataRecord` was an assertion about a driver row and never a measurement + * of one — which is why tsc reported nothing. #16422 replaced that cast with + * `canonicalIsoInstant`, whose return type IS `string | undefined`, so the + * shape reaching `stat()` from that adapter is measured now. The cases below + * are unaffected: a valid `Date` and an already-canonical string were never + * shapes the two spellings disagreed on. * * ## Why the double is overridden rather than replaced * diff --git a/packages/metadata/src/loaders/database-loader.ts b/packages/metadata/src/loaders/database-loader.ts index 57ee5739bd..5b5926c2d7 100644 --- a/packages/metadata/src/loaders/database-loader.ts +++ b/packages/metadata/src/loaders/database-loader.ts @@ -55,11 +55,15 @@ import { migrateProjectIdToEnvironmentId } from '../migrations/migrate-project-i * B1 ruling a producer-side arm like this became a NO-OP for the valid-`Date` * case, ⛔ never a conflict with it. * - * ⚠️ The call below looks redundant against `MetadataRecord`'s static type and - * is not: `rowToRecord` reaches its `createdAt` / `updatedAt` through an - * unchecked `row.created_at as string | undefined` cast, so the `string` there - * is an assertion about a driver row, never a measurement of one. ⛔ Do not - * "simplify" this away without fixing that cast. + * ⚠️ The call in `stat()` looks redundant against `MetadataRecord`'s static + * type. Since #16422 it very nearly IS: `rowToRecord` reaches `createdAt` / + * `updatedAt` through this same helper, so `stat()` sees the declared + * `string | undefined` as a measurement rather than the unchecked + * `row.created_at as string | undefined` cast that used to stand there. It is + * kept because `stat()` is reachable from `rowToRecord`'s output AND is the + * boundary that publishes `mtime`, and because idempotence is what makes the + * second application free — ⛔ do not "simplify" it away on the strength of + * the static type alone, which is what made the old cast invisible. * * ⛔ NOT a tolerant fallback: it converts the one per-dialect materialisation * the driver genuinely produces into the single declared spelling, at the @@ -83,8 +87,7 @@ import { migrateProjectIdToEnvironmentId } from '../migrations/migrate-project-i * error; the spelling this replaced (`String(value)`) served the visible text * `"Invalid Date"` instead. * - * The terminal value is chosen **per call site**, and this one's is - * `undefined`: the one caller (`stat`) already ends in + * The terminal value is chosen **per call site**. `stat` ends in * `?? new Date().toISOString()`, the branch an absent column takes today. The * ruling assigns `undefined` exactly where "the field is optional and the * caller already carries a `?? default` chain". ⛔ NOT the visible text @@ -93,6 +96,33 @@ import { migrateProjectIdToEnvironmentId } from '../migrations/migrate-project-i * `packages/spec`), so that text would move the failure to a zod refusal at * the consumer instead of removing it. ⛔ And NOT a blanket `''`: a silent blank is the shape that * hides the producer's bug. + * + * ## [#16422] Four more call sites — the collapse, and what it was not + * + * #14037's per-site `isoFromValidDate` stood beside this helper until #16422. + * It rewrote a single shape (valid `Date` -> ISO) and handed every other input + * back UNTOUCHED, so the four adapter boundaries fed a `Date`, a `null`, a + * `number` and an opaque column straight into fields declared + * `z.string().datetime()`. Measured across seven inputs, `MetadataRecordSchema` + * refused four and `MetadataHistoryRecordSchema` five. + * + * ⛔ The collapse was NOT the mechanical swap the retired docblocks implied. + * The two helpers were never interchangeable — #14078 aligned them on the + * Invalid-`Date` shape ALONE — so it moved every site for `null`, for a + * `number` and for an opaque column, and each needed its own terminal value: + * `undefined` for the two `.optional()` fields on `MetadataRecord`, the epoch + * for the REQUIRED `MetadataHistoryRecord.recordedAt` ({@link + * recordedAtFallback}). `listCommits` in `@objectstack/metadata-protocol` + * keeps a copy of the retired spelling on purpose: it promises its callers the + * RAW value back, a contract this domain rewrite would reverse. + * + * ⚠️ One composed behaviour moved with it, deliberately: `stat()` computes + * `record.updatedAt ?? record.createdAt`, and an Invalid `updated_at` is now + * `undefined` by the time that `??` runs instead of a truthy `Date`. So a row + * with an unreadable `updated_at` and a good `created_at` publishes + * `created_at` as its `mtime` where it used to publish `new Date()` — a stored + * instant in place of a fabricated one, and exactly the "same chain an absent + * column takes" that #14078's own ruling text prescribes for the shape. */ function canonicalIsoInstant(value: unknown): string | undefined { if (value === null || value === undefined) return undefined; @@ -106,69 +136,33 @@ function canonicalIsoInstant(value: unknown): string | undefined { } /** - * 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 USED to be false for both column - * classes: `SqlDriver#formatOutput` repaired the BUILTIN audit columns - * (`repairNaiveUtcAuditTimestamp`) and folded declared `Field.datetime` columns - * (`normalizeSqliteDatetimeOutput`) only inside its `if (this.isSqlite)` arm, - * so both arrived as a JS `Date` on the live dialects. #13973 ([ADR-0053 D-F1]) - * lifted both passes out of that gate; they run on EVERY dialect now, and the - * pin that recorded the asymmetry records the canonical-text contract instead - * (`packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts` - * §B, inverted on purpose). - * - * ⚠️ `withPostgresCalendarDayAsText` is UNCHANGED by that ruling and still - * leaves `timestamptz` / `timestamp` deliberately untouched ([ADR-0053 D-F2]): - * the client library still materialises those columns as a `Date`. What moved - * is where it is folded — at the driver's own read boundary, not at the parser - * — so what reaches this adapter is the canonical text. - * - * `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 — and the `Date` domain - * did not close: `driver-sql` hands an INVALID `Date` through unchanged - * ([ADR-0053 D-F3]) and non-SQL drivers materialise their own. + * The caller-side terminal value for `MetadataHistoryRecord.recordedAt` when + * the stored column canonicalises to nothing. * - * ⚠️ Deliberately NOT {@link canonicalIsoInstant} above. That difference used - * to be exactly one input shape — the Invalid `Date` on which that spelling - * raised `RangeError: Invalid time value`, measured reachable on BOTH live - * dialects (a MySQL zero datetime; any Postgres year in 275760..294276). - * #14078 has since RULED it (option B, 2026-09-02): that arm is now total and - * answers `undefined` for the shape, so the two agree on it. + * [#16422] `recordedAt` is a REQUIRED `z.string().datetime()` + * (`packages/spec/src/system/metadata-persistence.zod.ts`), which is what made + * it the one site in this family with no ruled-valid answer: the visible text + * `"Invalid Date"` fails the refinement, `undefined` fails the required field, + * and the pass-through this card retired fed it the `Date` object, which fails + * both. A caller-side default is the third answer, and this is it. * - * ⛔ They are still not ONE spelling, which is why #14078 did not collapse - * this helper into it. `canonicalIsoInstant` returns `string | undefined` and - * rewrites the whole domain (nullish -> `undefined`; anything neither `Date` - * nor string -> `String(value)`), while this one returns `unknown` and hands - * every non-valid-`Date` shape back UNTOUCHED. The consolidation is its own - * decision — **#16422** — because it moves six call sites for `null`, for a - * `number` and for an opaque column, one of which (`MetadataHistoryRecord - * .recordedAt`, a REQUIRED `z.string().datetime()` fed twice from here) has no - * terminal value either half of the #14078 ruling supplies. §D of - * `database-loader-14037-adapter-boundary-iso.test.ts` pins the behaviour this - * paragraph describes. + * ⛔ NOT `new Date()`. A history row records WHEN a version was recorded; a + * `now` stamp is a plausible-looking fact nobody measured, it sorts a version + * recorded years ago to the top of a newest-first timeline, and it is + * indistinguishable at every reader from a real recording instant. The epoch + * is inert on both counts — it sorts to the oldest end and no reader can + * mistake `1970-01-01T00:00:00.000Z` for a measurement. * - * ⛔ 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. + * It is also the answer the sibling reader of this same + * `sys_metadata_history.recorded_at` column already gives: `rowToEvent` and + * `history()` in `@objectstack/metadata-protocol`'s `sys-metadata-repository.ts` + * both end in `?? new Date(0).toISOString()`. * - * 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. + * A function rather than a module constant so the two history doors share ONE + * spelling that a grep finds, and so no caller can mutate a shared `Date`. */ -function isoFromValidDate(value: unknown): unknown { - if (value instanceof Date && !Number.isNaN(value.getTime())) return value.toISOString(); - return value; +function recordedAtFallback(): string { + return new Date(0).toISOString(); } /** @@ -864,9 +858,19 @@ 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: isoFromValidDate(row.created_at) as string | undefined, + // [#16422] The shared spelling, not the per-site `isoFromValidDate` this + // card retired. `canonicalIsoInstant` RETURNS `string | undefined`, + // which is exactly what `MetadataRecord.createdAt` / `.updatedAt` are + // declared as (`z.string().datetime().optional()`), so the `as string | + // undefined` cast that stood here — an assertion about a driver row, + // never a measurement of one — is gone rather than restated. The + // terminal value is `undefined`, the branch an absent column already + // took: both fields are `.optional()`, and both readers (`load()`, which + // reads only `checksum`, and `stat()`, whose `?? new Date().toISOString()` + // chain is unchanged) already carry it. + createdAt: canonicalIsoInstant(row.created_at), updatedBy: row.updated_by as string | undefined, - updatedAt: isoFromValidDate(row.updated_at) as string | undefined, + updatedAt: canonicalIsoInstant(row.updated_at), }; } @@ -1198,7 +1202,20 @@ 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: isoFromValidDate(row.recorded_at) as string, + // [#16422] `MetadataHistoryRecord.recordedAt` is a REQUIRED + // `z.string().datetime()`, so it is the one site in this family with no + // ruled-valid terminal value before this card: the visible text + // `"Invalid Date"` fails the refinement, `undefined` fails the required + // field, and the pre-collapse pass-through fed it a `Date` object, which + // fails both. The third answer is a CALLER-side default, and it is the + // epoch rather than `new Date()`: `now` would stamp a plausible but + // false recording instant on a version recorded years ago and sort it to + // the top of a newest-first history, while the epoch sorts an unreadable + // stamp to the oldest end and invents no fact. It is also the answer the + // sibling reader of this very column already gives (`rowToEvent` and + // `history()` in `sys-metadata-repository.ts`, both `?? new + // Date(0).toISOString()`). See {@link recordedAtFallback}. + recordedAt: canonicalIsoInstant(row.recorded_at) ?? recordedAtFallback(), }; } @@ -1279,7 +1296,10 @@ 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: isoFromValidDate(row.recorded_at) as string, + // [#16422] The other door onto the same column — same shared spelling, + // same caller-side terminal value. See `getHistoryRecord` above for + // why the epoch and not `now`, and {@link recordedAtFallback}. + recordedAt: canonicalIsoInstant(row.recorded_at) ?? recordedAtFallback(), }; });