diff --git a/.changeset/lucky-pugs-shave.md b/.changeset/lucky-pugs-shave.md new file mode 100644 index 0000000000..3d33937e7d --- /dev/null +++ b/.changeset/lucky-pugs-shave.md @@ -0,0 +1,19 @@ +--- +'@objectstack/metadata-protocol': patch +'@objectstack/metadata': patch +--- + +Canonicalise driver-materialised timestamps at the metadata adapter boundaries + +`MetadataItem.authoredAt` is declared `z.string()` ('ISO-8601 timestamp') and +`MetadataStats.mtime` is declared `z.string().datetime()`, but three producers +adapted a driver row into those declared types without converting the value. +`created_at` / `updated_at` are builtin audit columns and `recorded_at` is a +declared `Field.datetime`; `SqlDriver#formatOutput` repairs both only inside its +`if (this.isSqlite)` arm, so on Postgres and MySQL a JS `Date` landed in a field +every consumer reads as a `string`. + +`SysMetadataRepository#get` / `#getByHash` and `DatabaseLoader#stat` now emit +canonical ISO-8601 text on every dialect, matching the sibling producers that +already spelled it correctly. Values that were already canonical (SQLite) pass +through byte-identically. diff --git a/packages/metadata-protocol/src/sys-metadata-repository-13997-authored-at-canonicalisation.test.ts b/packages/metadata-protocol/src/sys-metadata-repository-13997-authored-at-canonicalisation.test.ts new file mode 100644 index 0000000000..d0d9a3e5af --- /dev/null +++ b/packages/metadata-protocol/src/sys-metadata-repository-13997-authored-at-canonicalisation.test.ts @@ -0,0 +1,258 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13997] `MetadataItem.authoredAt` is declared `z.string()` — the two + * adapter sites that pass a driver row straight through must canonicalise it. + * + * ## The defect + * + * `MetadataItem.authoredAt` is declared `z.string().describe('ISO-8601 + * timestamp')` (`packages/metadata-core/src/types.ts`), and `MetadataItem` is + * a `z.infer`, so the field is `string` to every consumer. Two producers in + * this file adapted a driver row into that declared type WITHOUT converting + * the timestamp: + * + * - `getByHash()` — `recorded_at`, a declared `Field.datetime` on + * `sys_metadata_history`; + * - `rowToItem()` (reached by `get()`) — `updated_at` / `created_at`, the + * BUILTIN audit columns. + * + * On Postgres and MySQL both arrive out of the record read door as a JS + * `Date`: `SqlDriver#formatOutput` repairs the audit columns and folds + * declared `datetime` columns only inside its `if (this.isSqlite)` arm, and + * `withPostgresCalendarDayAsText` leaves `timestamptz` / `timestamp` + * deliberately untouched. That dialect fact is pinned live in + * `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`. + * + * ## Why nothing reported it, and what that costs THIS file + * + * Two independent reasons. `row` is `any`, so tsc saw a `string` assignment + * that never happened. And `MetadataItemSchema` — the runtime validator that + * would have caught it — is parsed nowhere on a production path: its only + * `.parse` call sites in the repo are its own unit test + * (`packages/metadata-core/test/types.test.ts`), which feeds a hand-made + * **string**. + * + * ⚠️ That is the trap this file exists to break. A fixture built from a + * hand-made string proves nothing here, because the value under test is + * already the declared shape before the adapter runs — the assertion and the + * input share an identity. **Every case below drives a hand-made `Date`**, the + * one shape the live dialects produce and no existing fixture ever did, and + * §A's non-vacuity guard asserts the input really is a `Date` before reading + * the output. Without that guard a fixture that silently degraded to a string + * would keep this file green while measuring nothing. + * + * ⛔ No driver dependency: `@objectstack/metadata-protocol` has none and must + * not grow one — the layering runs the other way. The `Date` is hand-made here + * for exactly the reason the #13567 pin states for the OCC seam next door. + * + * ## What is asserted + * + * The declared contract itself, via `MetadataItemSchema.safeParse` — not a + * hand-rolled regex standing in for it. This is the schema's first evaluation + * against a driver-shaped input in this repo; a bare `toThrow()` or a + * `typeof` check would each pass for reasons unrelated to the defect. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +// The producer's OWN write-verb dispatch decisions (#4550 delete / #5480 +// update), so the fake engine below cannot accept a call ObjectQL refuses. +// Imported from `@objectstack/metadata-core`, not `@objectstack/objectql`: +// objectql depends on this package, so that import would close a cycle. +import { + assertEngineDeleteDispatch, + assertEngineUpdateDispatch, + assertEngineFindOnePredicate, + MetadataItemSchema, +} from '@objectstack/metadata-core'; +import { SysMetadataRepository } from './sys-metadata-repository.js'; + +interface Row { + [k: string]: unknown; +} + +/** Canonical instant text — exactly what `Date.prototype.toISOString` emits. */ +const ISO_Z = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + +/** + * The instant every case drives, as the live dialects hand it out: a JS + * `Date`. Carries non-zero milliseconds on purpose — `String(date)` and + * `date.toString()` both drop them, so a truncating regression stays + * observable rather than coinciding with the canonical text. + */ +const PG_INSTANT = new Date('2026-03-04T05:06:07.089Z'); + +/** + * Minimal engine fake. Deliberately stores exactly what it is handed — no key + * dropping, no coercion — so a `Date` planted in a row survives to the read + * door the way a live driver's would. + */ +function makeFakeEngine() { + const rows = new Map(); + const historyRows: Row[] = []; + + const keyOf = (w: Record) => + `${String(w.type)}|${String(w.name)}|${String(w.organization_id ?? 'null')}|${String(w.state ?? 'active')}`; + + const findRow = (where: Record) => { + if (where.id !== undefined) { + for (const [k, r] of rows) if (r.id === where.id) return { key: k, row: r }; + return null; + } + const k = keyOf(where); + const r = rows.get(k); + return r ? { key: k, row: r } : null; + }; + + const matchesHistory = (h: Row, where: Record): boolean => + Object.entries(where).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + return v === undefined || h[k] === v; + }); + + return { + rows, + historyRows, + async find(table: string, opts: { where: Record; limit?: number }) { + const matched = + table === 'sys_metadata_history' + ? historyRows.filter((h) => matchesHistory(h, opts.where)) + : Array.from(rows.values()).filter((r) => { + if (opts.where.type && r.type !== opts.where.type) return false; + if ( + opts.where.organization_id !== undefined && + r.organization_id !== opts.where.organization_id + ) + return false; + if (opts.where.state && r.state !== opts.where.state) return false; + return true; + }); + // Hold the caller's bound, AFTER the filter and by PRESENCE — a double + // that silently ignores `limit` answers more rows than the real engine + // would, which is the shape `check:objectql-double-limit` exists to stop. + return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched; + }, + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + if (table === 'sys_metadata_history') + return historyRows.find((h) => matchesHistory(h, opts.where)) ?? null; + return findRow(opts.where)?.row ?? null; + }, + async insert(table: string, data: Record) { + if (table === 'sys_metadata_history') { + const h: Row = { ...data }; + if (!h.id) h.id = `h_${historyRows.length + 1}`; + historyRows.push(h); + return { id: h.id as string }; + } + const k = keyOf(data); + const row: Row = { id: `r_${rows.size + 1}`, ...data }; + rows.set(k, row); + return { id: row.id as string }; + }, + async update(_t: string, data: Record, opts: { where: Record }) { + assertEngineUpdateDispatch(data, opts); + const found = findRow(opts.where); + if (!found) throw new Error('not found'); + rows.set(found.key, { ...found.row, ...data }); + return { id: found.row.id as string }; + }, + async delete(_t: string, opts: { where: Record }) { + assertEngineDeleteDispatch(opts); + const found = findRow(opts.where); + if (!found) return { deleted: 0 }; + rows.delete(found.key); + return { deleted: 1 }; + }, + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); + }, + }; +} + +const view = (label: string) => ({ + name: 'case_grid', + label, + object: 'case', + columns: [{ field: 'name' }], +}); + +describe('#13997 — authoredAt is canonical ISO-8601 text, whatever the dialect materialised', () => { + let engine: ReturnType; + let repo: SysMetadataRepository; + const ref = { org: 'org_alpha', type: 'view' as const, name: 'case_grid' }; + + beforeEach(() => { + engine = makeFakeEngine(); + repo = new SysMetadataRepository({ + engine, + organizationId: 'org_alpha', + orgLabel: 'org_alpha', + }); + }); + + describe('§A get() — the builtin audit columns, via rowToItem', () => { + it('emits a canonical ISO string when the row carries a JS Date', async () => { + await repo.put(ref, view('A'), { parentVersion: null, actor: 'usr_1' }); + + // Restate the row the way Postgres/MySQL hand it out. Mutating the + // stored row rather than the returned copy is what makes the READ path + // — the adapter under test — see the `Date`. + const stored = Array.from(engine.rows.values())[0]!; + stored.updated_at = PG_INSTANT; + stored.created_at = PG_INSTANT; + + // Non-vacuity guard: if the fixture ever degrades to a string this file + // would keep passing while testing the shape that was never broken. + expect(stored.updated_at).toBeInstanceOf(Date); + + const item = await repo.get(ref); + expect(item).not.toBeNull(); + + expect(typeof item!.authoredAt).toBe('string'); + expect(item!.authoredAt).toMatch(ISO_Z); + expect(item!.authoredAt).toBe(PG_INSTANT.toISOString()); + + // The declared contract itself, evaluated against a driver-shaped input. + const parsed = MetadataItemSchema.safeParse(item); + expect(parsed.success).toBe(true); + }); + + it('passes an already-canonical SQLite string through byte-identically', async () => { + await repo.put(ref, view('A'), { parentVersion: null, actor: 'usr_1' }); + + const canonical = '2026-03-04T05:06:07.089Z'; + const stored = Array.from(engine.rows.values())[0]!; + stored.updated_at = canonical; + + expect(typeof stored.updated_at).toBe('string'); + + const item = await repo.get(ref); + // Idempotent: the dialect that was already correct must not be reshaped. + expect(item!.authoredAt).toBe(canonical); + }); + }); + + describe('§B getByHash() — recorded_at, a declared Field.datetime', () => { + it('emits a canonical ISO string when the history row carries a JS Date', async () => { + const put = await repo.put(ref, view('A'), { parentVersion: null, actor: 'usr_1' }); + const hash = put.version; + + const historyRow = engine.historyRows[0]!; + historyRow.recorded_at = PG_INSTANT; + + // Same non-vacuity guard as §A, for the other column and the other door. + expect(historyRow.recorded_at).toBeInstanceOf(Date); + + const item = await repo.getByHash(ref, hash); + expect(item).not.toBeNull(); + + expect(typeof item!.authoredAt).toBe('string'); + expect(item!.authoredAt).toMatch(ISO_Z); + expect(item!.authoredAt).toBe(PG_INSTANT.toISOString()); + + const parsed = MetadataItemSchema.safeParse(item); + expect(parsed.success).toBe(true); + }); + }); +}); diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index 17cb9b8206..1a6fbc4c25 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -84,6 +84,44 @@ import type { IObjectQLEngine } from '@objectstack/core'; // door too (that shared-rule argument is the module's whole reason to exist). import { isWritablePackage } from './package-writability.js'; +/** + * Canonicalise a driver-materialised timestamp into the ISO-8601 string the + * declared output type of this adapter promises. + * + * [#13997] `sys_metadata`'s `created_at` / `updated_at` are BUILTIN audit + * columns; `sys_metadata_history`'s `recorded_at` is a declared + * `Field.datetime`. On the live dialects BOTH arrive out of the record read + * door as a JS `Date`: `SqlDriver#formatOutput` repairs the audit columns + * (`repairNaiveUtcAuditTimestamp`) and folds the declared datetime columns + * (`normalizeSqliteDatetimeOutput`) ONLY inside its `if (this.isSqlite)` arm, + * and `withPostgresCalendarDayAsText` leaves `timestamptz` / `timestamp` + * deliberately untouched because "those are instants, a `Date` is the right + * materialisation for them, and `Field.datetime` depends on it". Pinned in + * `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`. + * + * `MetadataItem.authoredAt` is declared `z.string()` ('ISO-8601 timestamp', + * `packages/metadata-core/src/types.ts`) and `MetadataItem` is a `z.infer`, so + * the field is `string` to every consumer. The producer owes the canonical + * spelling, and this is the adapter boundary that asserts the declared type — + * hence here, and not at the driver's read door (which would reverse that + * deliberate driver decision and belongs to the whole census, not this fix). + * + * ⛔ NOT a tolerant fallback: it does not teach a consumer to accept an + * off-spec shape. It converts the one per-dialect materialisation the driver + * genuinely produces into the single declared spelling, at the producer. The + * `Date` arm is the SAME spelling `auditMetaItem` already applies to + * `sys_metadata_audit.occurred_at` in `protocol.ts` — one shape, not a third. + * + * Absent column -> `undefined`, so each caller's existing `?? ` chain + * keeps exactly its current meaning. + */ +function canonicalIsoInstant(value: unknown): string | undefined { + if (value === null || value === undefined) return undefined; + if (value instanceof Date) return value.toISOString(); + if (typeof value === 'string') return value; + return String(value); +} + /** * Overlay-row lifecycle state. * @@ -414,7 +452,9 @@ export class SysMetadataRepository implements MetadataRepository { // that as the string 'unknown' invents an identity the column never // held, which is the same declared-≠-actual defect on the read side. authoredBy: ((row as any).recorded_by as string | null | undefined) ?? null, - authoredAt: (row as any).recorded_at ?? new Date(0).toISOString(), + // [#13997] `recorded_at` is a declared `Field.datetime`, so on Postgres + // and MySQL it materialises as a JS `Date` — see `canonicalIsoInstant`. + authoredAt: canonicalIsoInstant((row as any).recorded_at) ?? new Date(0).toISOString(), message: (row as any).change_note ?? undefined, seq: ((row as any).event_seq as number) ?? 0, }; @@ -1749,7 +1789,9 @@ export class SysMetadataRepository implements MetadataRepository { // #4556 — `updated_by` / `created_by` are lookup('sys_user') too; // absent means absent, not a user called 'unknown'. authoredBy: (row.updated_by as string | null | undefined) ?? (row.created_by as string | null | undefined) ?? null, - authoredAt: row.updated_at ?? row.created_at ?? new Date().toISOString(), + // [#13997] The builtin audit columns materialise as a JS `Date` on the + // live dialects; `authoredAt` is declared `z.string()`. + authoredAt: canonicalIsoInstant(row.updated_at ?? row.created_at) ?? new Date().toISOString(), message: undefined, seq: this.seqCounter, }; diff --git a/packages/metadata/src/loaders/database-loader.test.ts b/packages/metadata/src/loaders/database-loader.test.ts index 97597ff898..ae917c7ce0 100644 --- a/packages/metadata/src/loaders/database-loader.test.ts +++ b/packages/metadata/src/loaders/database-loader.test.ts @@ -1491,3 +1491,85 @@ describe('MetadataManager auto-configuration', () => { expect(result).toEqual({ name: 'account', label: 'Account' }); }); }); + +/** + * [#13997] `MetadataStats.mtime` is declared `z.string().datetime()` — `stat()` + * adapts a driver row into it and must canonicalise the timestamp. + * + * ## The defect + * + * `MetadataStatsSchema.mtime` is `z.string().datetime()` + * (`packages/spec/src/system/metadata-persistence.zod.ts`) — stricter than the + * sibling `MetadataItem.authoredAt`, which is a bare `z.string()`. `stat()` + * built it from `record.updatedAt ?? record.createdAt`, and `created_at` / + * `updated_at` are BUILTIN audit columns: not in `datetimeFields`, and + * `SqlDriver#formatOutput` repairs them only inside its `if (this.isSqlite)` + * arm. On Postgres and MySQL they arrive as a JS `Date`, pinned live in + * `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`. + * + * ⚠️ `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. + * + * ## Why the double is overridden rather than replaced + * + * `createMockDriver` above stores and returns plain objects, so it cannot + * produce the one shape that discriminates this defect. Overriding `findOne` + * on the existing double keeps every other guarantee that double already + * carries instead of introducing a second, looser one. + */ +describe('#13997 — stat() emits canonical ISO text whatever the dialect materialised', () => { + /** The instant the live dialects hand out, with observable milliseconds. */ + const PG_INSTANT = new Date('2026-03-04T05:06:07.089Z'); + const ISO_Z = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + + /** A `sys_metadata` row as a live dialect returns it: `updated_at` is a Date. */ + function rowWithStamp(stamp: unknown): Record { + return { + id: 'r_1', + name: 'account', + type: 'object', + metadata: JSON.stringify({ name: 'account' }), + checksum: 'sha256:abc', + version: 1, + created_at: stamp, + updated_at: stamp, + }; + } + + it('canonicalises a JS Date into the declared ISO string', async () => { + const driver = createMockDriver(); + // Cache off: `stat()` memoises, and a cached hit would read back the value + // this assertion is about without re-running the adapter under test. + const loader = new DatabaseLoader({ driver, cache: { enabled: false } }); + + const row = rowWithStamp(PG_INSTANT); + // Non-vacuity guard — a fixture that degraded to a string would leave this + // test green while measuring the shape that was never broken. + expect(row.updated_at).toBeInstanceOf(Date); + driver.findOne = vi.fn().mockResolvedValue(row); + + const stats = await loader.stat('object', 'account'); + + expect(stats).not.toBeNull(); + expect(typeof stats!.mtime).toBe('string'); + expect(stats!.mtime).toMatch(ISO_Z); + expect(stats!.mtime).toBe(PG_INSTANT.toISOString()); + }); + + it('passes an already-canonical SQLite string through byte-identically', async () => { + const driver = createMockDriver(); + const loader = new DatabaseLoader({ driver, cache: { enabled: false } }); + + const canonical = '2026-03-04T05:06:07.089Z'; + const row = rowWithStamp(canonical); + expect(typeof row.updated_at).toBe('string'); + driver.findOne = vi.fn().mockResolvedValue(row); + + const stats = await loader.stat('object', 'account'); + + // Idempotent: the dialect that was already correct must not be reshaped. + expect(stats!.mtime).toBe(canonical); + }); +}); diff --git a/packages/metadata/src/loaders/database-loader.ts b/packages/metadata/src/loaders/database-loader.ts index f376c6bbe2..92b0b5e829 100644 --- a/packages/metadata/src/loaders/database-loader.ts +++ b/packages/metadata/src/loaders/database-loader.ts @@ -32,6 +32,39 @@ import { LRUCache } from '../utils/lru-cache.js'; import { isMissingTableError, isSchemaAlreadyExistsError } from '@objectstack/types'; import { migrateProjectIdToEnvironmentId } from '../migrations/migrate-project-id-to-environment-id.js'; +/** + * Canonicalise a driver-materialised timestamp into the ISO-8601 string + * `MetadataStats.mtime` is declared as. + * + * [#13997] `sys_metadata`'s `created_at` / `updated_at` are BUILTIN audit + * columns, so no declared-field coercion reaches them and + * `SqlDriver#formatOutput` repairs them only inside its `if (this.isSqlite)` + * arm. On Postgres and MySQL they arrive out of the record read door as a JS + * `Date` — pinned in + * `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`. + * `MetadataStatsSchema.mtime` is `z.string().datetime()` + * (`packages/spec/src/system/metadata-persistence.zod.ts`), so a `Date` here + * is a silent violation of a declared contract. + * + * ⚠️ 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. + * + * ⛔ NOT a tolerant fallback: it converts the one per-dialect materialisation + * the driver genuinely produces into the single declared spelling, at the + * producer — the same shape the sibling adapters in + * `@objectstack/metadata-protocol` apply. Absent column -> `undefined`, so the + * caller's existing `?? ` chain keeps its current meaning. + */ +function canonicalIsoInstant(value: unknown): string | undefined { + if (value === null || value === undefined) return undefined; + if (value instanceof Date) return value.toISOString(); + if (typeof value === 'string') return value; + return String(value); +} + /** * Cache configuration for `DatabaseLoader`. * @@ -914,7 +947,7 @@ export class DatabaseLoader implements MetadataLoader { const stats: MetadataStats = { size: metadataStr.length, - mtime: record.updatedAt ?? record.createdAt ?? new Date().toISOString(), + mtime: canonicalIsoInstant(record.updatedAt ?? record.createdAt) ?? new Date().toISOString(), format: 'json', etag: record.checksum, }; diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 741162ba41..3d2d1bad3b 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1466,6 +1466,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/sys-metadata-repository-13997-authored-at-canonicalisation.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/sys-metadata-repository-13997-authored-at-canonicalisation.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/sys-metadata-repository-13997-authored-at-canonicalisation.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/sys-metadata-repository.contract.test.ts", "verb": "delete",