Skip to content

Commit 9624761

Browse files
committed
docs(engine): correct seven out-of-package comments about formatOutput's timestamp passes
Since #13973 / PR #16619 (ADR-0053 D-F1) both of `SqlDriver#formatOutput`'s timestamp passes -- the AUDIT_TIMESTAMP_COLUMNS pass and the normalizeSqliteDatetimeOutput pass over datetimeFields -- run on every dialect. Seven out-of-package comments still described them as gated on `if (this.isSqlite)`. The rest-server.ts one was not merely stale: it warned future authors that "a declared Field.datetime is therefore NOT protected on Postgres/MySQL", which invites the tolerant consumer-side coercion ADR-0053 / #16619 expressly forbid. Corrected first, per the triage ruling on the card. Both carve-outs are preserved rather than flattened: - withPostgresCalendarDayAsText is untouched (D-F2) -- the client still hands back a Date; only "nothing folds it" was false. - the Invalid `Date` residue still stands (D-F3) -- no sentence claims the read door never hands out a Date. Comments only; no runtime behaviour changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
1 parent 56ee7ae commit 9624761

7 files changed

Lines changed: 169 additions & 81 deletions

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,20 @@
1313
* straight past it into the declared field.
1414
*
1515
* `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`.
16+
* when this landed that did NOT protect it: `SqlDriver#formatOutput` folded
17+
* declared datetime columns (`normalizeSqliteDatetimeOutput`) only inside its
18+
* `if (this.isSqlite)` arm. #13973 ([ADR-0053 D-F1]) has since lifted that fold
19+
* out of the gate — it runs on every dialect — and the pin that recorded the
20+
* asymmetry now records the canonical-text contract
21+
* (`packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`
22+
* §B, inverted on purpose).
23+
*
24+
* ⚠️ `withPostgresCalendarDayAsText` is untouched by that ruling and still
25+
* leaves `timestamptz` / `timestamp` deliberately alone ([ADR-0053 D-F2]) — the
26+
* client still hands back a `Date`; the driver folds it at its own read
27+
* boundary now. And the `Date` this file plants stays reachable: an INVALID
28+
* `Date` leaves `driver-sql` unchanged ([ADR-0053 D-F3]) and non-SQL drivers
29+
* materialise their own, so what is pinned below is a live adapter arm.
2130
*
2231
* ## Why it matters downstream, not just as a type
2332
*

packages/metadata-protocol/src/sys-metadata-repository-14938-list-drafts-updated-at.test.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,29 @@
1717
* declaration is an inline return type), and `rows` is cast `as any[]` one line
1818
* above the map, so tsc sees a `string` assignment that never happened.
1919
*
20-
* ## Why the value is a `Date` on the live dialects
20+
* ## Why a `Date` reaches this projection at all
2121
*
2222
* `updated_at` / `created_at` are the BUILTIN audit columns on `sys_metadata`
2323
* (`Field.datetime`, `packages/metadata-core/src/objects/sys-metadata.object.ts`).
24-
* `SqlDriver#formatOutput` repairs the audit columns
25-
* (`repairNaiveUtcAuditTimestamp`) and folds the declared datetime columns
24+
* When this landed, `SqlDriver#formatOutput` repaired the audit columns
25+
* (`repairNaiveUtcAuditTimestamp`) and folded the declared datetime columns
2626
* (`normalizeSqliteDatetimeOutput`) ONLY inside its `if (this.isSqlite)` arm,
27-
* and `withPostgresCalendarDayAsText` leaves `timestamptz` / `timestamp`
28-
* deliberately untouched because those are instants. That dialect fact is
29-
* pinned live in
30-
* `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`;
31-
* this file does not re-derive it and takes on no driver dependency
32-
* (`@objectstack/metadata-protocol` has none, and the layering runs the other
33-
* way) — the `Date` is hand-made here for exactly that reason.
27+
* so both live dialects handed the column through as a JS `Date`. #13973
28+
* ([ADR-0053 D-F1]) has since lifted both passes out of that gate — they run on
29+
* every dialect — and the pin that recorded the asymmetry now records the
30+
* canonical-text contract
31+
* (`packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`
32+
* §B, inverted on purpose).
33+
*
34+
* ⚠️ `withPostgresCalendarDayAsText` is untouched by that ruling and still
35+
* leaves `timestamptz` / `timestamp` deliberately alone ([ADR-0053 D-F2]) —
36+
* those are instants and the CLIENT still materialises them as a `Date`; what
37+
* changed is that the driver folds it at its own read boundary. The `Date` this
38+
* file plants therefore still reaches the projection in practice: an INVALID
39+
* `Date` leaves `driver-sql` unchanged ([ADR-0053 D-F3]) and non-SQL drivers
40+
* materialise their own. This file does not re-derive any of it and takes on no
41+
* driver dependency (`@objectstack/metadata-protocol` has none, and the
42+
* layering runs the other way) — the `Date` is hand-made here for that reason.
3443
*
3544
* ## What is asserted, and why it is not a hand-copied shape
3645
*

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

Lines changed: 42 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -90,14 +90,24 @@ import { isWritablePackage } from './package-writability.js';
9090
*
9191
* [#13997] `sys_metadata`'s `created_at` / `updated_at` are BUILTIN audit
9292
* columns; `sys_metadata_history`'s `recorded_at` is a declared
93-
* `Field.datetime`. On the live dialects BOTH arrive out of the record read
94-
* door as a JS `Date`: `SqlDriver#formatOutput` repairs the audit columns
95-
* (`repairNaiveUtcAuditTimestamp`) and folds the declared datetime columns
96-
* (`normalizeSqliteDatetimeOutput`) ONLY inside its `if (this.isSqlite)` arm,
97-
* and `withPostgresCalendarDayAsText` leaves `timestamptz` / `timestamp`
98-
* deliberately untouched because "those are instants, a `Date` is the right
99-
* materialisation for them, and `Field.datetime` depends on it". Pinned in
100-
* `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`.
93+
* `Field.datetime`. On the live dialects BOTH used to arrive out of the record
94+
* read door as a JS `Date`: `SqlDriver#formatOutput` repaired the audit columns
95+
* (`repairNaiveUtcAuditTimestamp`) and folded the declared datetime columns
96+
* (`normalizeSqliteDatetimeOutput`) ONLY inside its `if (this.isSqlite)` arm.
97+
* #13973 ([ADR-0053 D-F1]) lifted both passes out of that gate — they run on
98+
* EVERY dialect now, so the read door presents the canonical ISO-Z text.
99+
* Pinned in
100+
* `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`
101+
* §B, which was inverted on purpose to record the new contract.
102+
*
103+
* ⚠️ `withPostgresCalendarDayAsText` still leaves `timestamptz` / `timestamp`
104+
* deliberately untouched ([ADR-0053 D-F2]) — those are instants and a `Date`
105+
* remains the right materialisation for them at the CLIENT layer. What changed
106+
* is that the driver no longer lets that `Date` out of its read door. (⛔ The
107+
* clause this comment used to quote alongside it — that `Field.datetime`
108+
* "depends on" the `Date` materialisation — was checked against the tree by
109+
* #13973 and did not hold; it is gone from the driver and must not be quoted
110+
* back.)
101111
*
102112
* `MetadataItem.authoredAt` is declared `z.string()` ('ISO-8601 timestamp',
103113
* `packages/metadata-core/src/types.ts`) and `MetadataItem` is a `z.infer`, so
@@ -163,10 +173,14 @@ function canonicalIsoInstant(value: unknown): string | undefined {
163173
* [#14037] `rowToEvent` reaches `ts` through `(row.recorded_at as string) ??
164174
* …`, and `row` is `any`, so tsc sees a `string` assignment that never
165175
* happened. `recorded_at` is a declared `Field.datetime` on
166-
* `sys_metadata_history`, which the dialect asymmetry above does NOT protect:
167-
* the `datetimeFields` fold sits inside `formatOutput`'s `if (this.isSqlite)`
168-
* arm, so Postgres and MySQL hand the column out as a JS `Date`.
169-
* `MetadataEventSchema.ts` is `z.string()`
176+
* `sys_metadata_history`, and the dialect asymmetry described above did not
177+
* protect it: the `datetimeFields` fold sat inside `formatOutput`'s
178+
* `if (this.isSqlite)` arm, so Postgres and MySQL handed the column out as a JS
179+
* `Date`. #13973 ([ADR-0053 D-F1]) has since closed that asymmetry — the fold
180+
* runs on every dialect — but the cast is still an assertion rather than a
181+
* measurement, and the `Date` domain is not empty: an INVALID `Date` still
182+
* leaves `driver-sql` unchanged ([ADR-0053 D-F3]) and non-SQL drivers
183+
* materialise their own. `MetadataEventSchema.ts` is `z.string()`
170184
* (`packages/metadata-core/src/types.ts`), and the value's one in-repo reader
171185
* — `MetadataManager.applyRepoEvent`, which forwards it to
172186
* `MetadataWatchEvent.timestamp` — is declared `z.string().datetime()`.
@@ -1177,16 +1191,22 @@ export class SysMetadataRepository implements MetadataRepository {
11771191
organizationId: row.organization_id ?? null,
11781192
packageId: row.package_id ?? null,
11791193
// [#14938] `updated_at` / `created_at` are the BUILTIN audit columns,
1180-
// so on Postgres and MySQL they arrive here as a JS `Date`: the audit
1181-
// repair and the declared-datetime fold both sit inside
1182-
// `SqlDriver#formatOutput`'s `if (this.isSqlite)` arm, and
1183-
// `withPostgresCalendarDayAsText` leaves `timestamptz` / `timestamp`
1184-
// alone because those are instants. `rows` is cast `as any[]` above,
1185-
// so tsc never saw the `Date` land in a field this signature declares
1186-
// `string | null`. Canonicalised at the producer — the same adapter
1187-
// boundary `rowToItem` uses, never a tolerant `??` in the console or a
1188-
// reshape at the driver's read door (#13973's two standing
1189-
// prohibitions).
1194+
// and on Postgres and MySQL they used to arrive here as a JS `Date`:
1195+
// the audit repair and the declared-datetime fold both sat inside
1196+
// `SqlDriver#formatOutput`'s `if (this.isSqlite)` arm. #13973
1197+
// ([ADR-0053 D-F1]) lifted both out of that gate, so the read door now
1198+
// presents canonical ISO-Z text on every dialect;
1199+
// `withPostgresCalendarDayAsText` is untouched by that ruling and still
1200+
// leaves `timestamptz` / `timestamp` alone at the CLIENT parser
1201+
// ([ADR-0053 D-F2]). `rows` is cast `as any[]` above, so tsc never saw
1202+
// the `Date` land in a field this signature declares `string | null`,
1203+
// and the shape is still reachable: an INVALID `Date` leaves the driver
1204+
// unchanged ([ADR-0053 D-F3]) and non-SQL drivers materialise their own.
1205+
// Canonicalised at the producer — the same adapter boundary `rowToItem`
1206+
// uses, and ⛔ never a tolerant `??` in the console, which is the #13973
1207+
// prohibition that still stands. (Its second one — ⛔ no unilateral
1208+
// reshape at the driver's read door — was DISCHARGED by the B1 ruling,
1209+
// which made that reshape the central fix rather than a local one.)
11901210
//
11911211
// The terminal is chosen PER CALL SITE (#14078) and this one is
11921212
// `null`, not `rowToItem`'s `?? new Date(...).toISOString()`: this

packages/metadata/src/loaders/database-loader-14037-adapter-boundary-iso.test.ts

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,24 @@
1313
* casts — an assertion about a driver row, never a measurement of one — which
1414
* is why tsc reported nothing.
1515
*
16-
* On Postgres and MySQL the assertion is false for BOTH column classes:
17-
* `SqlDriver#formatOutput` repairs the builtin audit columns
18-
* (`repairNaiveUtcAuditTimestamp`) and folds declared `Field.datetime` columns
16+
* When this landed, the assertion was false on Postgres and MySQL for BOTH
17+
* column classes: `SqlDriver#formatOutput` repaired the builtin audit columns
18+
* (`repairNaiveUtcAuditTimestamp`) and folded declared `Field.datetime` columns
1919
* (`normalizeSqliteDatetimeOutput`) only inside its `if (this.isSqlite)` arm,
20-
* and `withPostgresCalendarDayAsText` leaves `timestamptz` / `timestamp`
21-
* deliberately untouched. That dialect fact is pinned live in
22-
* `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`.
23-
* `recorded_at` being a declared `Field.datetime` on `sys_metadata_history`
24-
* does NOT protect it — the fold is inside the SQLite arm too.
20+
* so `recorded_at` being a declared `Field.datetime` on `sys_metadata_history`
21+
* did not protect it either. #13973 ([ADR-0053 D-F1]) has since lifted both
22+
* passes out of that gate — they run on EVERY dialect — and the pin that
23+
* recorded the asymmetry now records the canonical-text contract
24+
* (`packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`
25+
* §B, inverted on purpose).
26+
*
27+
* ⚠️ `withPostgresCalendarDayAsText` is untouched by that ruling and still
28+
* leaves `timestamptz` / `timestamp` deliberately alone ([ADR-0053 D-F2]) — the
29+
* client still hands back a `Date`; the driver now folds it at its own read
30+
* boundary. And the `Date` domain these cases pin did not close: an INVALID
31+
* `Date` still leaves `driver-sql` unchanged ([ADR-0053 D-F3]), and non-SQL
32+
* drivers materialise their own. So these cases pin a live adapter arm, not a
33+
* historical one — what they own is the adapter's behaviour per input shape.
2534
*
2635
* All three declarations are `z.string().datetime()`
2736
* (`packages/spec/src/system/metadata-persistence.zod.ts`), a refinement a

packages/metadata/src/loaders/database-loader.ts

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,23 @@ import { migrateProjectIdToEnvironmentId } from '../migrations/migrate-project-i
3737
* `MetadataStats.mtime` is declared as.
3838
*
3939
* [#13997] `sys_metadata`'s `created_at` / `updated_at` are BUILTIN audit
40-
* columns, so no declared-field coercion reaches them and
41-
* `SqlDriver#formatOutput` repairs them only inside its `if (this.isSqlite)`
42-
* arm. On Postgres and MySQL they arrive out of the record read door as a JS
43-
* `Date` — pinned in
44-
* `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`.
40+
* columns, so no declared-field coercion reaches them; `SqlDriver#formatOutput`
41+
* repairs them through its own `AUDIT_TIMESTAMP_COLUMNS` pass, which used to
42+
* sit inside `if (this.isSqlite)` and so handed Postgres and MySQL rows out of
43+
* the record read door as a JS `Date`. #13973 ([ADR-0053 D-F1]) lifted that
44+
* pass out of the gate — it now runs on EVERY dialect, and the pin that once
45+
* recorded the asymmetry records the canonical-text contract instead
46+
* (`packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`
47+
* §B, inverted on purpose).
48+
*
4549
* `MetadataStatsSchema.mtime` is `z.string().datetime()`
4650
* (`packages/spec/src/system/metadata-persistence.zod.ts`), so a `Date` here
47-
* is a silent violation of a declared contract.
51+
* is a silent violation of a declared contract. This adapter stays because the
52+
* `Date` domain did not close: `driver-sql` still hands an INVALID `Date`
53+
* through unchanged ([ADR-0053 D-F3] — the one shape with no canonical text),
54+
* and non-SQL drivers materialise their own `Date`s. Under [ADR-0053 D-F1]'s
55+
* B1 ruling a producer-side arm like this became a NO-OP for the valid-`Date`
56+
* case, ⛔ never a conflict with it.
4857
*
4958
* ⚠️ The call below looks redundant against `MetadataRecord`'s static type and
5059
* is not: `rowToRecord` reaches its `createdAt` / `updatedAt` through an
@@ -103,18 +112,29 @@ function canonicalIsoInstant(value: unknown): string | undefined {
103112
*
104113
* [#14037] `rowToRecord` and the two history adapters below each assert a
105114
* `string` over a driver row (`row.created_at as string | undefined`, and so
106-
* on). On Postgres and MySQL that assertion is false: `SqlDriver#formatOutput`
107-
* repairs the BUILTIN audit columns (`repairNaiveUtcAuditTimestamp`) and folds
108-
* declared `Field.datetime` columns (`normalizeSqliteDatetimeOutput`) only
109-
* inside its `if (this.isSqlite)` arm, and `withPostgresCalendarDayAsText`
110-
* leaves `timestamptz` / `timestamp` deliberately untouched. Both column
111-
* classes therefore arrive as a JS `Date` on the live dialects — pinned in
112-
* `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`.
115+
* on). On Postgres and MySQL that assertion USED to be false for both column
116+
* classes: `SqlDriver#formatOutput` repaired the BUILTIN audit columns
117+
* (`repairNaiveUtcAuditTimestamp`) and folded declared `Field.datetime` columns
118+
* (`normalizeSqliteDatetimeOutput`) only inside its `if (this.isSqlite)` arm,
119+
* so both arrived as a JS `Date` on the live dialects. #13973 ([ADR-0053 D-F1])
120+
* lifted both passes out of that gate; they run on EVERY dialect now, and the
121+
* pin that recorded the asymmetry records the canonical-text contract instead
122+
* (`packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`
123+
* §B, inverted on purpose).
124+
*
125+
* ⚠️ `withPostgresCalendarDayAsText` is UNCHANGED by that ruling and still
126+
* leaves `timestamptz` / `timestamp` deliberately untouched ([ADR-0053 D-F2]):
127+
* the client library still materialises those columns as a `Date`. What moved
128+
* is where it is folded — at the driver's own read boundary, not at the parser
129+
* — so what reaches this adapter is the canonical text.
130+
*
113131
* `MetadataRecord.createdAt` / `.updatedAt` and
114132
* `MetadataHistoryRecord.recordedAt` are declared `z.string().datetime()`
115133
* (`packages/spec/src/system/metadata-persistence.zod.ts`) — a refinement a
116134
* `Date` fails outright. The cast is an assertion about a driver row, never a
117-
* measurement of one, which is why tsc reports nothing.
135+
* measurement of one, which is why tsc reports nothing — and the `Date` domain
136+
* did not close: `driver-sql` hands an INVALID `Date` through unchanged
137+
* ([ADR-0053 D-F3]) and non-SQL drivers materialise their own.
118138
*
119139
* ⚠️ Deliberately NOT {@link canonicalIsoInstant} above. That difference used
120140
* to be exactly one input shape — the Invalid `Date` on which that spelling

packages/rest/src/import-job-dto-timestamp-canonical.test.ts

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,22 @@
1717
* milliseconds dropped, the SERVER's timezone baked in, no `Z`, and not
1818
* `Date.parse`-safe for a client doing strict ISO parsing.
1919
*
20-
* Why all four, and why nothing upstream repaired them: `formatOutput`'s two
21-
* timestamp repairs — the `AUDIT_TIMESTAMP_COLUMNS` pass (`created_at`) and the
22-
* `normalizeSqliteDatetimeOutput` pass over `datetimeFields`
23-
* (`started_at` / `completed_at` / `reverted_at`, all declared `Field.datetime`
24-
* on `sys_import_job`) — both sit INSIDE `formatOutput`'s `if (this.isSqlite)`
25-
* arm. ⚠️ A declared `Field.datetime` is NOT protected on Postgres/MySQL.
20+
* Why all four, and why nothing upstream repaired them AT THE TIME:
21+
* `formatOutput`'s two timestamp repairs — the `AUDIT_TIMESTAMP_COLUMNS` pass
22+
* (`created_at`) and the `normalizeSqliteDatetimeOutput` pass over
23+
* `datetimeFields` (`started_at` / `completed_at` / `reverted_at`, all declared
24+
* `Field.datetime` on `sys_import_job`) — both sat INSIDE `formatOutput`'s
25+
* `if (this.isSqlite)` arm, so a declared `Field.datetime` was NOT protected on
26+
* Postgres/MySQL. #13973 ([ADR-0053 D-F1]) has since lifted both passes out of
27+
* that gate: they run on EVERY dialect, so `driver-sql` no longer hands this
28+
* seam a valid `Date` on any dialect.
29+
*
30+
* ⚠️ That does not make these cases redundant, and the `Date` arm they pin is
31+
* not dead code. `driver-mongodb` stamps `new Date()` and BSON round-trips it,
32+
* and `driver-sql` still hands an INVALID `Date` through unchanged — the one
33+
* shape with no canonical text to fold to ([ADR-0053 D-F3]), measured reachable
34+
* on both live dialects. What these cases own is the mapper's behaviour per
35+
* INPUT SHAPE, which is the property that outlives the dialect fact.
2636
*
2737
* ## Why the obvious pin would have proved nothing
2838
*
@@ -57,11 +67,14 @@
5767
*
5868
* ## What is deliberately NOT claimed here
5969
*
60-
* That `driver-sql` hands this seam a `Date` on Postgres. That is a fact about
61-
* `driver-sql`, measured beside the fix (`formatOutput`'s `isSqlite` bracketing)
62-
* and pinned in that package; `@objectstack/rest` must not grow a Postgres
63-
* dependency to restate it. What these tests own is the mapper's behaviour
64-
* GIVEN each input shape a driver can produce.
70+
* Which shapes `driver-sql` hands this seam, on which dialect. That is a fact
71+
* about `driver-sql` — today the canonical text on every dialect, with the
72+
* Invalid `Date` passing through ([ADR-0053 D-F1]/[D-F3]) — measured and pinned
73+
* in that package (`sql-driver-13973-canonical-iso-read-door.test.ts`);
74+
* `@objectstack/rest` must not grow a Postgres dependency to restate it, and
75+
* must not re-derive it either, which is why nothing here asserts it. What
76+
* these tests own is the mapper's behaviour GIVEN each input shape a driver can
77+
* produce.
6578
*/
6679

6780
import { describe, it, expect, afterEach } from 'vitest';

0 commit comments

Comments
 (0)