fix(metadata): collapse four isoFromValidDate call sites onto the shared canonical-ISO spelling (#16422)

.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.

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);

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();

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);
     });
   });
 });