Skip to content

Commit 77a532d

Browse files
os-muskClaudeclaude
authored
fix(metadata): emit the declared ISO string at the five adapter boundaries that cast a driver Date (#14939)
* wip(#14037): narrow producer-side Date-to-ISO at the five adapter-boundary casts Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 * test(#14037): pin the five adapter boundaries against a driver Date Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 * chore(#14037): changeset + engine-double-contract pin ledger for the two new test files Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --------- Co-authored-by: Claude <elon@objectstack.ai> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 44ffa21 commit 77a532d

6 files changed

Lines changed: 719 additions & 5 deletions
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
'@objectstack/metadata': patch
3+
'@objectstack/metadata-protocol': patch
4+
---
5+
6+
Five metadata adapter boundaries now emit the ISO-8601 string their declared type promises
7+
when the driver hands them a JS `Date`, instead of asserting `as string` over it
8+
9+
`MetadataRecord.createdAt` / `.updatedAt` and `MetadataHistoryRecord.recordedAt` are
10+
declared `z.string().datetime()`, and `MetadataEvent.ts` is declared `z.string()`. Four
11+
producers in `DatabaseLoader` (`rowToRecord`, `getHistoryRecord`, `queryHistory`) and one
12+
in `SysMetadataRepository` (`rowToEvent`) reached those fields through an unchecked
13+
`row.<column> as string` cast, which is an assertion about a driver row rather than a
14+
measurement of one — so nothing type-checked and nothing reported it.
15+
16+
On Postgres and MySQL the assertion is false for both column classes involved.
17+
`SqlDriver#formatOutput` repairs the builtin audit columns and folds declared
18+
`Field.datetime` columns only inside its `if (this.isSqlite)` arm, and
19+
`withPostgresCalendarDayAsText` leaves `timestamptz` / `timestamp` deliberately untouched
20+
because those are instants. A column being declared `Field.datetime` therefore does **not**
21+
protect it: on the production default driver both classes come out of the record read door
22+
as a `Date`, and `.datetime()` is a refinement a `Date` fails outright. Nothing has failed
23+
yet only because no production path parses these values today.
24+
25+
The repair is producer-side, at the adapter boundary that asserts the declared type — not
26+
a tolerant fallback in a consumer, and not a change at the driver's read door, which would
27+
reverse a deliberate driver decision. Callers keep their existing behaviour for every other
28+
shape: an already-canonical SQLite string passes through byte-identically, an absent column
29+
still yields `undefined` so each caller's `?? <default>` chain means what it meant, and an
30+
Invalid `Date` is handed through unchanged rather than converted, because what the shared
31+
canonical-ISO spelling should do with that one input is still being decided.
Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#14037] `MetadataEvent.ts` is declared `z.string()` — `rowToEvent`, the
5+
* adapter that asserts that declared type over a driver row, must
6+
* canonicalise what the live dialects actually hand it: a JS `Date`.
7+
*
8+
* ## The defect
9+
*
10+
* `rowToEvent` reached `ts` through `(row.recorded_at as string) ?? new
11+
* Date(0).toISOString()`. `row` is `any`, so tsc saw a `string` assignment
12+
* that never happened, and the `??` fires only on nullish — a `Date` walks
13+
* straight past it into the declared field.
14+
*
15+
* `recorded_at` is a declared `Field.datetime` on `sys_metadata_history`, and
16+
* that does NOT protect it: `SqlDriver#formatOutput` folds declared datetime
17+
* columns (`normalizeSqliteDatetimeOutput`) only inside its
18+
* `if (this.isSqlite)` arm, and `withPostgresCalendarDayAsText` leaves
19+
* `timestamptz` / `timestamp` deliberately untouched. Pinned live in
20+
* `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`.
21+
*
22+
* ## Why it matters downstream, not just as a type
23+
*
24+
* The value's one in-repo reader is `MetadataManager.applyRepoEvent`, which
25+
* forwards it verbatim to `MetadataWatchEvent.timestamp` — declared
26+
* `z.string().datetime()` in `packages/spec/src/system/metadata-persistence.zod.ts`.
27+
* So the wrong shape does not stop at this package's boundary; it is carried
28+
* into a field whose refinement a `Date` fails outright.
29+
*
30+
* ## Why the fixture drives a hand-made `Date`
31+
*
32+
* The trap the #13997 sibling in this directory names: a fixture built from a
33+
* hand-made ISO string is already the declared shape before the adapter runs,
34+
* so the assertion and the input share an identity and the case measures
35+
* nothing. Every case here plants the one shape the live dialects produce, and
36+
* carries a non-vacuity guard that the planted value really is a `Date`.
37+
*
38+
* ⛔ No driver dependency: `@objectstack/metadata-protocol` has none and must
39+
* not grow one — the layering runs the other way.
40+
*
41+
* ## What is asserted
42+
*
43+
* `MetadataEventSchema` itself (`@objectstack/metadata-core`), not a
44+
* hand-rolled regex standing in for it.
45+
*
46+
* §C is the #14078 NEUTRALITY pin: an Invalid `Date` must reach the consumer
47+
* UNCHANGED, exactly as this cast passes it through today. The shared
48+
* `canonicalIsoInstant` spelling in this same file would instead raise
49+
* `RangeError: Invalid time value` there — measured reachable on both live
50+
* dialects — and whether it should is the open subject of #14078, which
51+
* #13973 is blocked on. This card imports neither answer, and §C goes red the
52+
* moment someone swaps the contested spelling in.
53+
*/
54+
55+
import { describe, it, expect, beforeEach } from 'vitest';
56+
// The producer's OWN write-verb dispatch decisions (#4550 delete / #5480
57+
// update), so the fake engine below cannot accept a call ObjectQL refuses.
58+
// Imported from `@objectstack/metadata-core`, not `@objectstack/objectql`:
59+
// objectql depends on this package, so that import would close a cycle.
60+
import {
61+
assertEngineDeleteDispatch,
62+
assertEngineUpdateDispatch,
63+
assertEngineFindOnePredicate,
64+
MetadataEventSchema,
65+
} from '@objectstack/metadata-core';
66+
import { SysMetadataRepository } from './sys-metadata-repository.js';
67+
68+
interface Row {
69+
[k: string]: unknown;
70+
}
71+
72+
/** Canonical instant text — exactly what `Date.prototype.toISOString` emits. */
73+
const ISO_Z = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/;
74+
75+
/**
76+
* The instant every case drives, as Postgres and MySQL hand it out. Non-zero
77+
* milliseconds on purpose — `String(date)` and `date.toString()` both drop
78+
* them, so a truncating regression stays observable rather than coinciding
79+
* with the canonical text.
80+
*/
81+
const PG_INSTANT = new Date('2026-03-04T05:06:07.089Z');
82+
83+
/** What SQLite hands out for the same instant — already the declared shape. */
84+
const SQLITE_TEXT = '2026-03-04T05:06:07.089Z';
85+
86+
/**
87+
* Minimal engine fake — the same shape the #13997 sibling in this directory
88+
* uses. Stores exactly what it is handed, so a `Date` planted in a row
89+
* survives to the read door the way a live driver's would.
90+
*/
91+
function makeFakeEngine() {
92+
const rows = new Map<string, Row>();
93+
const historyRows: Row[] = [];
94+
95+
const keyOf = (w: Record<string, unknown>) =>
96+
`${String(w.type)}|${String(w.name)}|${String(w.organization_id ?? 'null')}|${String(w.state ?? 'active')}`;
97+
98+
const findRow = (where: Record<string, unknown>) => {
99+
if (where.id !== undefined) {
100+
for (const [k, r] of rows) if (r.id === where.id) return { key: k, row: r };
101+
return null;
102+
}
103+
const k = keyOf(where);
104+
const r = rows.get(k);
105+
return r ? { key: k, row: r } : null;
106+
};
107+
108+
const matchesHistory = (h: Row, where: Record<string, unknown>): boolean =>
109+
Object.entries(where).every(([k, v]) => {
110+
if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`);
111+
return v === undefined || h[k] === v;
112+
});
113+
114+
return {
115+
rows,
116+
historyRows,
117+
async find(table: string, opts: { where: Record<string, unknown>; limit?: number }) {
118+
const matched =
119+
table === 'sys_metadata_history'
120+
? historyRows.filter((h) => matchesHistory(h, opts.where))
121+
: Array.from(rows.values()).filter((r) => {
122+
if (opts.where.type && r.type !== opts.where.type) return false;
123+
if (
124+
opts.where.organization_id !== undefined &&
125+
r.organization_id !== opts.where.organization_id
126+
)
127+
return false;
128+
if (opts.where.state && r.state !== opts.where.state) return false;
129+
return true;
130+
});
131+
// Hold the caller's bound, AFTER the filter and by PRESENCE — a double
132+
// that silently ignores `limit` answers more rows than the real engine
133+
// would, which is the shape `check:objectql-double-limit` exists to stop.
134+
return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched;
135+
},
136+
async findOne(table: string, opts: { where: Record<string, unknown> }) {
137+
assertEngineFindOnePredicate(table, opts);
138+
if (table === 'sys_metadata_history')
139+
return historyRows.find((h) => matchesHistory(h, opts.where)) ?? null;
140+
return findRow(opts.where)?.row ?? null;
141+
},
142+
async insert(table: string, data: Record<string, unknown>) {
143+
if (table === 'sys_metadata_history') {
144+
const h: Row = { ...data };
145+
if (!h.id) h.id = `h_${historyRows.length + 1}`;
146+
historyRows.push(h);
147+
return { id: h.id as string };
148+
}
149+
const k = keyOf(data);
150+
const row: Row = { id: `r_${rows.size + 1}`, ...data };
151+
rows.set(k, row);
152+
return { id: row.id as string };
153+
},
154+
async update(_t: string, data: Record<string, unknown>, opts: { where: Record<string, unknown> }) {
155+
assertEngineUpdateDispatch(data, opts);
156+
const found = findRow(opts.where);
157+
if (!found) throw new Error('not found');
158+
rows.set(found.key, { ...found.row, ...data });
159+
return { id: found.row.id as string };
160+
},
161+
async delete(_t: string, opts: { where: Record<string, unknown> }) {
162+
assertEngineDeleteDispatch(opts);
163+
const found = findRow(opts.where);
164+
if (!found) return { deleted: 0 };
165+
rows.delete(found.key);
166+
return { deleted: 1 };
167+
},
168+
async transaction<T>(cb: (ctx: any, info: { owned: boolean }) => Promise<T>): Promise<T> {
169+
return cb(undefined, { owned: true });
170+
},
171+
};
172+
}
173+
174+
const view = (label: string) => ({
175+
name: 'case_grid',
176+
label,
177+
object: 'case',
178+
columns: [{ field: 'name' }],
179+
});
180+
181+
describe('#14037 — MetadataEvent.ts is canonical ISO text, whatever the dialect materialised', () => {
182+
let engine: ReturnType<typeof makeFakeEngine>;
183+
let repo: SysMetadataRepository;
184+
const ref = { org: 'org_alpha', type: 'view' as const, name: 'case_grid' };
185+
186+
const firstEvent = async () => {
187+
for await (const evt of repo.history(ref)) return evt;
188+
return null;
189+
};
190+
191+
beforeEach(async () => {
192+
engine = makeFakeEngine();
193+
repo = new SysMetadataRepository({
194+
engine,
195+
organizationId: 'org_alpha',
196+
orgLabel: 'org_alpha',
197+
});
198+
await repo.put(ref, view('A'), { parentVersion: null, actor: 'usr_1' });
199+
});
200+
201+
describe('§A history() — recorded_at, a declared Field.datetime', () => {
202+
it('emits a canonical ISO string when the history row carries a JS Date', async () => {
203+
const historyRow = engine.historyRows[0]!;
204+
historyRow.recorded_at = PG_INSTANT;
205+
206+
// Non-vacuity guard: a fixture that silently degraded to a string would
207+
// keep this file green while measuring nothing.
208+
expect(historyRow.recorded_at).toBeInstanceOf(Date);
209+
210+
const evt = await firstEvent();
211+
expect(evt).not.toBeNull();
212+
213+
expect(typeof evt!.ts).toBe('string');
214+
expect(evt!.ts).toMatch(ISO_Z);
215+
expect(evt!.ts).toBe(PG_INSTANT.toISOString());
216+
217+
// The declared contract itself, evaluated against a driver-shaped input.
218+
const parsed = MetadataEventSchema.safeParse(evt);
219+
expect(parsed.success).toBe(true);
220+
});
221+
222+
it('passes an already-canonical SQLite string through byte-identically', async () => {
223+
engine.historyRows[0]!.recorded_at = SQLITE_TEXT;
224+
225+
const evt = await firstEvent();
226+
227+
// Idempotent: the dialect that was already correct must not be reshaped.
228+
expect(evt!.ts).toBe(SQLITE_TEXT);
229+
});
230+
});
231+
232+
describe('§B the nullish arm keeps its meaning', () => {
233+
it('still falls back to the epoch when the column is absent', async () => {
234+
delete engine.historyRows[0]!.recorded_at;
235+
236+
const evt = await firstEvent();
237+
238+
expect(evt!.ts).toBe(new Date(0).toISOString());
239+
});
240+
});
241+
242+
describe('§C #14078 neutrality — an Invalid Date is NOT converted here', () => {
243+
/**
244+
* ⛔ This card does not decide #14078. An Invalid `Date` is measured
245+
* reachable on both live dialects (a MySQL zero datetime; any Postgres
246+
* year in 275760..294276), and whether the shared canonical-ISO spelling
247+
* should throw on it (option A) or fall back to a rendering (option B) is
248+
* a maintainer call across four packages. Until it is ruled, this site
249+
* hands that one shape through exactly as it does today — no new throw,
250+
* no invented rendering.
251+
*/
252+
it('hands the value through unchanged instead of raising RangeError', async () => {
253+
const invalid = new Date(NaN);
254+
expect(Number.isNaN(invalid.getTime())).toBe(true);
255+
// The contested spelling's `Date` arm, on this input, for contrast.
256+
expect(() => invalid.toISOString()).toThrow(RangeError);
257+
258+
engine.historyRows[0]!.recorded_at = invalid;
259+
260+
const evt = await firstEvent();
261+
262+
// Unchanged — and specifically NOT the `??` fallback, which would mean
263+
// this card had quietly chosen a rendering for the contested shape.
264+
expect(evt!.ts).toBe(invalid as unknown as string);
265+
});
266+
});
267+
});

packages/metadata-protocol/src/sys-metadata-repository.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,51 @@ function canonicalIsoInstant(value: unknown): string | undefined {
122122
return String(value);
123123
}
124124

125+
/**
126+
* Canonicalise the ONE driver materialisation {@link
127+
* SysMetadataRepository.rowToEvent} was measured to produce — a valid JS
128+
* `Date` — into the ISO-8601 string `MetadataEvent.ts` is declared as. Every
129+
* other shape is returned UNTOUCHED.
130+
*
131+
* [#14037] `rowToEvent` reaches `ts` through `(row.recorded_at as string) ??
132+
* …`, and `row` is `any`, so tsc sees a `string` assignment that never
133+
* happened. `recorded_at` is a declared `Field.datetime` on
134+
* `sys_metadata_history`, which the dialect asymmetry above does NOT protect:
135+
* the `datetimeFields` fold sits inside `formatOutput`'s `if (this.isSqlite)`
136+
* arm, so Postgres and MySQL hand the column out as a JS `Date`.
137+
* `MetadataEventSchema.ts` is `z.string()`
138+
* (`packages/metadata-core/src/types.ts`), and the value's one in-repo reader
139+
* — `MetadataManager.applyRepoEvent`, which forwards it to
140+
* `MetadataWatchEvent.timestamp` — is declared `z.string().datetime()`.
141+
*
142+
* ⚠️ Deliberately NOT {@link canonicalIsoInstant} above, and the difference is
143+
* exactly one input shape. That spelling reaches `value.toISOString()` for ANY
144+
* `Date`, which raises `RangeError: Invalid time value` on an Invalid `Date`
145+
* — measured reachable on BOTH live dialects (a MySQL zero datetime; any
146+
* Postgres year in 275760..294276) and the open subject of #14078, which
147+
* #13973 is blocked on. Whether the shared spelling should throw there
148+
* (option A) or fall back to a rendering (option B) is a maintainer call over
149+
* four packages, so this repair imports NEITHER answer into a new call site:
150+
* an Invalid `Date` is returned unchanged, exactly as this cast passes it
151+
* through today. When #14078 rules, this helper collapses into the shared
152+
* spelling.
153+
*
154+
* ⛔ NOT a tolerant fallback (#13973's standing prohibition): it teaches no
155+
* consumer to accept an off-spec shape; it converts one measured producer
156+
* materialisation at the producer. The `Number.isNaN(value.getTime())` guard
157+
* is the spelling already in use at `packages/rest/src/export-format.ts` and
158+
* `packages/rest/src/import-prepare.ts`, not a new one.
159+
*
160+
* A sibling copy serves the four sites in
161+
* `packages/metadata/src/loaders/database-loader.ts`. ⛔ Neither is exported:
162+
* widening `@objectstack/metadata-core`'s public surface for it is a separate
163+
* decision, and #14078 consolidates this family anyway.
164+
*/
165+
function isoFromValidDate(value: unknown): unknown {
166+
if (value instanceof Date && !Number.isNaN(value.getTime())) return value.toISOString();
167+
return value;
168+
}
169+
125170
/**
126171
* Overlay-row lifecycle state.
127172
*
@@ -1154,7 +1199,7 @@ export class SysMetadataRepository implements MetadataRepository {
11541199
// the answer is "the platform", not a user literally named 'unknown'.
11551200
actor: (row.recorded_by as string | null | undefined) ?? null,
11561201
message: (row.change_note as string | undefined) ?? undefined,
1157-
ts: (row.recorded_at as string) ?? new Date(0).toISOString(),
1202+
ts: (isoFromValidDate(row.recorded_at) as string) ?? new Date(0).toISOString(),
11581203
source: (row.source as string | undefined) ?? 'sys-metadata-repo',
11591204
};
11601205
}

0 commit comments

Comments
 (0)