Skip to content

Commit b110578

Browse files
claude[bot]claude
andauthored
fix(metadata): collapse four isoFromValidDate call sites onto the shared canonical-ISO spelling (#16422) (#17198)
* fix(metadata): collapse four isoFromValidDate call sites onto the shared canonical-ISO spelling (#16422) WIP checkpoint before the verification lap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU * fix(metadata): collapse four isoFromValidDate call sites onto the shared canonical-ISO spelling (#16422) The per-site `isoFromValidDate` helper rewrote exactly one shape (a valid JS `Date` becomes ISO text) and handed every other input back untouched, so four adapter boundaries fed a `null`, a `number`, an opaque column and an Invalid `Date` into fields declared `z.string()` / `z.string().datetime()`, each behind an `as string` cast asserting the opposite. Measured over the seven inputs that distinguish the two helpers, the declared schemas refused 21 of 35 values. Those four sites now read `canonicalIsoInstant`, whose return type IS `string | undefined`, so all four casts are deleted rather than restated, and both sibling definitions of the retired helper are gone. The terminal value is chosen per site from that site's declared schema: `undefined` for the two `.optional()` fields on `MetadataRecord`, and the epoch (`recordedAtFallback`) for the REQUIRED `MetadataHistoryRecord.recordedAt`, which had no legal answer at all before this change. Refusals over the same inputs: 21 -> 8. `listCommits` in metadata-protocol keeps its copy on purpose — it promises callers the RAW value back, and the shared spelling would erase an Invalid `Date` from the response and reorder the commit timeline. That site is byte-identical on all seven inputs. `SqlDriver`'s same-named helper takes `Date`, not `unknown`, and is the producer-side fold ADR-0053 D-F3 governs; it is not part of this family and is untouched. The three neutrality pins are dispositioned individually: two rewritten as ruled pins, the `listCommits` one kept verbatim because its behaviour did not move. A fourth section the card did not name (`database-loader-14078`'s composition pin) is rewritten too, and records the one composed behaviour that changed: `stat()`'s `updatedAt ?? createdAt` now falls through for an unreadable `updated_at`, publishing a stored `created_at` instead of a fabricated `new Date()`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU * chore(changeset): regrade @objectstack/metadata to minor for the stat() answer change (#16422) Seat ruling on the delivery: the `patch` argument holds for the four repaired sites — those change only values `MetadataRecordSchema` / `MetadataHistoryRecordSchema` already refused, so nothing a consumer legitimately received has moved. It does not cover `DatabaseLoader.stat()`. A row with an unreadable `updated_at` and a good `created_at` previously published `new Date().toISOString()` as its `mtime`, and that answer was LEGAL — it satisfies `MetadataStats.mtime`'s `z.string().datetime()`, and the pre-existing pin asserted exactly that. So that 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, which the ladder puts at `minor`. The level is per package, so the four repaired sites ride along. `@objectstack/metadata-protocol` stays `patch`: `rowToEvent` only stops emitting values `MetadataEventSchema` refused, and `listCommits` is byte-identical on all seven probe inputs. Front-matter and prose are moved together — the body now states which site carries the level, why the old answer was legal, and why the sibling package does not take it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0f38ab0 commit b110578

9 files changed

Lines changed: 493 additions & 212 deletions
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
---
2+
"@objectstack/metadata": minor
3+
"@objectstack/metadata-protocol": patch
4+
---
5+
6+
fix(metadata): four `isoFromValidDate` call sites collapse onto the shared canonical-ISO spelling; `MetadataHistoryRecord.recordedAt` gets the terminal value it never had (#16422)
7+
8+
## What was wrong
9+
10+
`#14037`/`#14038` landed a narrow per-site helper, `isoFromValidDate`, beside
11+
the shared `canonicalIsoInstant` spelling. It rewrote exactly one shape — a
12+
valid JS `Date` becomes ISO text — and handed **every other input back
13+
untouched**. Four adapter boundaries used it, and each fed a field declared
14+
`z.string()` or `z.string().datetime()`:
15+
16+
| site | declared as |
17+
|:--|:--|
18+
| `SysMetadataRepository.rowToEvent``MetadataEvent.ts` | `z.string()` |
19+
| `DatabaseLoader.rowToRecord``MetadataRecord.createdAt` / `.updatedAt` | `z.string().datetime().optional()` |
20+
| `DatabaseLoader.getHistoryRecord``MetadataHistoryRecord.recordedAt` | `z.string().datetime()`**required** |
21+
| `DatabaseLoader.queryHistory` → the same field, the other door | `z.string().datetime()`**required** |
22+
23+
So a `null`, a `number`, an opaque column and an Invalid `Date` all arrived at a
24+
field declared `string`, each wearing an `as string` / `as string | undefined`
25+
cast that asserted the opposite. Measured over the seven inputs that
26+
distinguish the two helpers, the declared schemas refused **21 of 35** produced
27+
values.
28+
29+
`recordedAt` was the sharp end: a REQUIRED `z.string().datetime()` for which
30+
none of the three available answers was legal — the visible text
31+
`"Invalid Date"` fails the refinement, `undefined` fails the required field, and
32+
the pass-through fed it the `Date` object, which fails both.
33+
34+
## What it does now
35+
36+
Those four sites read `canonicalIsoInstant`, whose return type **is**
37+
`string | undefined`, so all four casts are deleted rather than restated. Both
38+
sibling definitions of `isoFromValidDate` are gone. The terminal value is chosen
39+
per site, from the site's own declared schema:
40+
41+
- `MetadataRecord.createdAt` / `.updatedAt` are `.optional()``undefined`, the
42+
branch an absent column already took. ⛔ No default is invented for a field the
43+
schema lets be absent.
44+
- `MetadataHistoryRecord.recordedAt` is required → the **epoch**, via a named
45+
`recordedAtFallback()` shared by both history doors. ⛔ Not `new Date()`: a
46+
`now` stamp is a plausible-looking recording instant nobody measured, and it
47+
sorts a version recorded years ago to the top of a newest-first timeline. The
48+
epoch invents no fact and sorts to the oldest end. It is also the answer the
49+
sibling reader of this same `sys_metadata_history.recorded_at` column already
50+
gives (`rowToEvent` and `history()`, both `?? new Date(0).toISOString()`).
51+
52+
Schema refusals over the same seven inputs: **21 → 8**. The eight that remain
53+
are a `number` and an opaque object at four sites — shapes no driver is measured
54+
to materialise for these columns. They now arrive as the declared *type* (a
55+
string) that simply is not a valid datetime, so the producer's bug stays visible
56+
instead of being papered over.
57+
58+
## One behaviour change worth reading twice — and it is why this is `minor`
59+
60+
`DatabaseLoader.stat()` computes `record.updatedAt ?? record.createdAt`. An
61+
Invalid `updated_at` used to WIN that `??` — a `Date` is truthy and not nullish —
62+
so a row with an unreadable `updated_at` and a good `created_at` published
63+
`new Date()` as its `mtime`. It now folds to `undefined` one step earlier and
64+
loses the `??`, so the row publishes its `created_at`: a stored instant in place
65+
of a fabricated one, and exactly the "same `?? DEFAULT` chain an absent column
66+
takes" that `#14078`'s own ruling text prescribes for the shape.
67+
68+
⚠️ **The old answer was LEGAL.** `new Date().toISOString()` satisfies
69+
`MetadataStats.mtime`'s `z.string().datetime()` perfectly well, and the
70+
pre-existing pin asserted exactly that. So this one site is **not** the repair of
71+
a violation — it is one legal published answer replaced by a different legal
72+
published answer on a published read verb. Nothing was refused before and is
73+
permitted now; a consumer simply receives a different instant.
74+
75+
## Why the two levels differ
76+
77+
- **`@objectstack/metadata``minor`.** Its four repaired sites, on their own,
78+
are the "repairing an implementation that silently violated its own already
79+
published declared type" case: the values that changed there are ones
80+
`MetadataRecordSchema` / `MetadataHistoryRecordSchema` already refused, and
81+
nothing a consumer legitimately received has moved. But this package also
82+
carries `stat()`, and that site changes a **legal** published answer, which the
83+
paragraph above measures. The level is per package, so the four repaired sites
84+
ride along at `minor`.
85+
- **`@objectstack/metadata-protocol``patch`.** Neither of its two sites moves
86+
a legal published answer. `rowToEvent` only stops emitting values
87+
`MetadataEventSchema` refused (a `Date`, a `number`, an opaque object in a
88+
field declared `z.string()`), and `listCommits` is byte-identical on all seven
89+
probe inputs.
90+
91+
⛔ No declared type narrowed, no export was added or removed (neither helper was
92+
ever exported), and no envelope or accept set moved — so this is `minor` by the
93+
changed-answer row, not a breaking change, and it carries no ADR-0087
94+
disposition.
95+
96+
## What deliberately did NOT collapse
97+
98+
`listCommits` in `@objectstack/metadata-protocol` keeps its copy. Its docblock
99+
promises callers the RAW value back for a non-`Date`, and the shared spelling
100+
rewrites the whole domain: swapping it in would ERASE an Invalid `Date` from the
101+
response (`undefined` — the one answer ADR-0053 D-F3 refuses, because it silently
102+
drops a value that is on disk) and hand a `number` or an opaque object to the
103+
commit-timeline sort as `String(value)` rather than verbatim. Measured, that site
104+
is byte-identical on all seven inputs before and after this change.
105+
106+
`SqlDriver`'s same-named helper is not part of this family at all: it takes
107+
`Date` (not `unknown`), both its call sites narrow with `instanceof Date` first,
108+
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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,19 @@
5252
* now total, answering `undefined` for the shape. This card's route is
5353
* unchanged: `isoFromValidDate` in `protocol.ts` converts the ONE measured
5454
* shape (a valid `Date`) and returns every other shape — including an Invalid
55-
* `Date` — UNCHANGED, which is what `listCommits` promises its callers. §D
56-
* below stays the pin on that promise: it goes red the moment anyone swaps
57-
* the other spelling into this site, now the separately-tracked consolidation
58-
* decision #16422.
55+
* `Date` — UNCHANGED, which is what `listCommits` promises its callers.
56+
*
57+
* ⚠️ #16422 has now RULED the consolidation, and this site was held OUT of it
58+
* on the strength of that promise. The card collapsed the family's other four
59+
* call sites into `canonicalIsoInstant` and deleted both sibling definitions;
60+
* `protocol.ts` keeps its copy, and §D below is no longer a placeholder for a
61+
* pending decision but the standing pin on a decided one. Measured across the
62+
* seven inputs that distinguish the two helpers, this site is byte-identical
63+
* before and after that card — the swap here would have ERASED an Invalid
64+
* `Date` from the response (`undefined`, the one answer [ADR-0053 D-F3]
65+
* refuses) and handed a `number` or an opaque object to
66+
* `compareAuditInstants` as `String(value)` instead of verbatim, reordering
67+
* rows this seam deliberately leaves alone.
5968
*
6069
* ## Reverse verification, direction predicted BEFORE running
6170
*
@@ -180,7 +189,12 @@ describe('[#14038] listCommits emits the ISO-8601 string createdAt is declared a
180189
* invented rendering. This case is what makes that a PIN rather than
181190
* a claim: it goes red the moment `canonicalIsoInstant` (or any
182191
* spelling that reaches `.toISOString()` unconditionally) is swapped
183-
* into `listCommits`. The consolidation is #16422.
192+
* into `listCommits`.
193+
*
194+
* ⚠️ #16422 ruled the consolidation and held this site OUT of it, so
195+
* this pin now guards a DECIDED contract rather than an open one. It
196+
* stays exactly as written — the only pin of the three that did not
197+
* need rewriting, because the behaviour it asserts did not move.
184198
*/
185199
it('hands the value through unchanged instead of raising RangeError', async () => {
186200
const invalid = new Date(NaN);

packages/metadata-protocol/src/protocol.ts

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1811,18 +1811,33 @@ function compareAuditInstants(a: unknown, b: unknown): number {
18111811
* declared `string` return type.
18121812
*
18131813
* ⚠️ Deliberately NOT the `canonicalIsoInstant` spelling next door in
1814-
* `sys-metadata-repository.ts` / `database-loader.ts` (#14037's sibling
1815-
* sites). That difference used to be exactly one input shape — the Invalid
1814+
* `sys-metadata-repository.ts` / `database-loader.ts` — which is, since
1815+
* #16422, the ONLY spelling at #14037's sibling sites. That difference used
1816+
* to be exactly one input shape — the Invalid
18161817
* `Date` on which that spelling raised `RangeError: Invalid time value`,
18171818
* measured reachable on BOTH live dialects (a MySQL zero datetime; any
18181819
* Postgres year in 275760..294276). #14078 has since RULED it (option B,
18191820
* 2026-09-02): that arm is now total and answers `undefined` for the shape.
18201821
*
1821-
* ⛔ They are still not ONE spelling, and this copy has the strongest reason
1822-
* of the three not to be collapsed — see the paragraph below on what
1823-
* `listCommits` promises its callers for a non-`Date` value. The
1824-
* consolidation is tracked as **#16422**; #14078 ruled only the five arms
1825-
* that THREW.
1822+
* ⛔ They are still not ONE spelling, and **#16422 ruled that this copy is the
1823+
* one that stays**. That card collapsed the family's other four call sites —
1824+
* `rowToEvent` in `sys-metadata-repository.ts` and the three adapter
1825+
* boundaries in `database-loader.ts` — into `canonicalIsoInstant` and deleted
1826+
* both sibling definitions of this spelling. This site was held out, for the
1827+
* reason the last paragraph below states: `listCommits` promises its callers
1828+
* the RAW value back for a non-`Date`, and `canonicalIsoInstant` rewrites the
1829+
* whole domain. Measured on the seven inputs that distinguish the two
1830+
* helpers, swapping it in here moves three: an Invalid `Date` would be ERASED
1831+
* from the response (`undefined` — the one answer [ADR-0053 D-F3] refuses,
1832+
* because it silently drops a value that is on disk), and a `number` and an
1833+
* opaque object would reach {@link compareAuditInstants} as `String(value)`
1834+
* rather than verbatim, reordering rows this seam deliberately leaves alone.
1835+
*
1836+
* ⇒ The family is now two DELIBERATE helpers, not one pending merge: the
1837+
* shared domain rewrite at the sites whose declared field is a
1838+
* `z.string().datetime()` and whose caller carries a terminal value, and this
1839+
* narrow one-shape conversion at the site whose declared contract is
1840+
* pass-through. ⛔ Do not collapse it without superseding that ruling.
18261841
*
18271842
* ⛔ NOT a tolerant fallback (#13973's standing prohibition): it teaches no
18281843
* consumer to accept an off-spec shape; it converts the one measured
@@ -1837,8 +1852,9 @@ function compareAuditInstants(a: unknown, b: unknown): number {
18371852
* valid `Date` — an absent/opaque column must still reach `sort`'s fallback
18381853
* branch and any in-process reader exactly as before. Consolidating the
18391854
* family's near-identical copies was expected to be #14078's call; that
1840-
* ruling covered only the five arms that threw, so the consolidation is
1841-
* tracked separately as #16422.
1855+
* ruling covered only the five arms that threw, and #16422 then ruled this
1856+
* promise the reason to keep this copy rather than the obstacle to removing
1857+
* it. §D of `protocol-14038-list-commits-created-at-iso.test.ts` is the pin.
18421858
*/
18431859
function isoFromValidDate(value: unknown): unknown {
18441860
if (value instanceof Date && !Number.isNaN(value.getTime())) return value.toISOString();

packages/metadata-protocol/src/sys-metadata-repository-14037-event-ts-canonicalisation.test.ts

Lines changed: 56 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,21 @@
5252
* `MetadataEventSchema` itself (`@objectstack/metadata-core`), not a
5353
* hand-rolled regex standing in for it.
5454
*
55-
* §C is the #14078 NEUTRALITY pin: an Invalid `Date` must reach the consumer
56-
* UNCHANGED, exactly as this cast passes it through today. #14078 has since
57-
* ruled (option B, 2026-09-02) and `canonicalIsoInstant` in this same file is
58-
* now TOTAL — it answers `undefined` for that shape rather than raising
59-
* `RangeError: Invalid time value`. The two helpers still differ across the
60-
* REST of the input domain, so §C keeps its job unchanged: it goes red the
61-
* moment someone swaps the other spelling into this site, which is now the
62-
* separately-tracked consolidation decision #16422 rather than an open
63-
* ruling.
55+
* §C WAS the #14078 neutrality pin — "an Invalid `Date` must reach the
56+
* consumer UNCHANGED, exactly as this cast passes it through" — written to go
57+
* red the moment anyone swapped the shared spelling into this site. #16422
58+
* made that swap DELIBERATELY, so §C is rewritten as the RULED pin rather
59+
* than kept or deleted: it now asserts the terminal value the ruling chose,
60+
* and it still goes red if anyone reverts to the pass-through, because the
61+
* shape that reaches `MetadataEvent.ts` under that spelling is a `Date`
62+
* object in a field declared `z.string()`.
63+
*
64+
* ⚠️ The rewrite is the point, not a formality. The neutrality pin existed so
65+
* the swap could not happen by accident; its evidence — the seven-input
66+
* before/after matrix in #16422's PR — is what discharges it. `rowToEvent`
67+
* now reads `canonicalIsoInstant(row.recorded_at) ?? new Date(0).toISOString()`,
68+
* the same spelling and the same terminal value `history()` already used for
69+
* `authoredAt` off this very column.
6470
*/
6571

6672
import { describe, it, expect, beforeEach } from 'vitest';
@@ -250,29 +256,57 @@ describe('#14037 — MetadataEvent.ts is canonical ISO text, whatever the dialec
250256
});
251257
});
252258

253-
describe('§C #14078 neutrality — an Invalid Date is NOT converted here', () => {
259+
describe('§C [#16422] RULED — an Invalid Date takes the epoch, the branch an absent column takes', () => {
254260
/**
255-
* ⛔ This card does not decide #14078. An Invalid `Date` is measured
256-
* reachable on both live dialects (a MySQL zero datetime; any Postgres
257-
* year in 275760..294276), and whether the shared canonical-ISO spelling
258-
* should throw on it (option A) or fall back to a rendering (option B) is
259-
* a maintainer call across four packages. Until it is ruled, this site
260-
* hands that one shape through exactly as it does today — no new throw,
261-
* no invented rendering.
261+
* This section was the #14078 NEUTRALITY pin: it asserted that this site
262+
* hands an Invalid `Date` through UNCHANGED, and it was written to go red
263+
* on exactly the swap #16422 then performed. It is rewritten, not
264+
* deleted, because the swap was deliberate and now has its own evidence.
265+
*
266+
* What the ruling decided, per call site: `rowToEvent` reads
267+
* `canonicalIsoInstant(row.recorded_at) ?? new Date(0).toISOString()`. An
268+
* Invalid `Date` folds to `undefined` — #14078's own total `Date` arm —
269+
* and therefore takes the `??` branch an ABSENT column already took (§B),
270+
* which is also the answer `history()` gives for `authoredAt` off this
271+
* same column.
272+
*
273+
* ⛔ Why the old behaviour could not stay: `MetadataEvent.ts` is declared
274+
* `z.string()` (`@objectstack/metadata-core`) and its one in-repo reader
275+
* forwards it to `MetadataWatchEvent.timestamp`, a `z.string().datetime()`.
276+
* The pass-through put a `Date` OBJECT in that field, so
277+
* `MetadataEventSchema` refused the event this adapter produced — asserted
278+
* below rather than described, by parsing the same fixture both ways.
262279
*/
263-
it('hands the value through unchanged instead of raising RangeError', async () => {
280+
it('answers the epoch and produces an event the declared schema accepts', async () => {
264281
const invalid = new Date(NaN);
265282
expect(Number.isNaN(invalid.getTime())).toBe(true);
266-
// The contested spelling's `Date` arm, on this input, for contrast.
283+
// Non-vacuity: the shape really is the one with no canonical text.
267284
expect(() => invalid.toISOString()).toThrow(RangeError);
268285

269286
engine.historyRows[0]!.recorded_at = invalid;
270287

271288
const evt = await firstEvent();
272289

273-
// Unchanged — and specifically NOT the `??` fallback, which would mean
274-
// this card had quietly chosen a rendering for the contested shape.
275-
expect(evt!.ts).toBe(invalid as unknown as string);
290+
// The ruled terminal value — the same one §B's absent column takes.
291+
expect(evt!.ts).toBe(new Date(0).toISOString());
292+
expect(typeof evt!.ts).toBe('string');
293+
294+
// ⛔ And specifically NOT the retired pass-through, which is what the
295+
// neutrality version of this section asserted.
296+
expect(evt!.ts).not.toBe(invalid as unknown as string);
297+
expect(evt!.ts).not.toBeInstanceOf(Date);
298+
299+
// The declared contract, which the pass-through could not satisfy.
300+
const parsed = MetadataEventSchema.safeParse(evt);
301+
expect(parsed.success, JSON.stringify((parsed as { error?: { issues: unknown } }).error?.issues)).toBe(true);
302+
});
303+
304+
it('rejects the retired shape, so a revert to pass-through cannot pass silently', () => {
305+
// The exact object the pass-through used to emit, checked against the
306+
// declared schema in isolation. This is why the swap was not cosmetic.
307+
const passThrough = { seq: 1, op: 'update', ref, hash: null, parentHash: null,
308+
actor: null, ts: new Date(NaN), source: 'sys-metadata-repo' };
309+
expect(MetadataEventSchema.safeParse(passThrough).success).toBe(false);
276310
});
277311
});
278312
});

0 commit comments

Comments
 (0)