From 3fc4e3bf24a7bc75fdca1cbed7e36a9adc89a04b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 23:33:21 +0000 Subject: [PATCH] fix(metadata): canonicalise the timestamps `migrateSysNotificationToEvent` writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `selectLegacyRows` reads the legacy `sys_notification` table through `driver.raw`/`execute`, a door that does not run `formatOutput`. On SQLite the stamps come back as canonical ISO text and `String(row.created_at)` is the identity; on Postgres and MySQL an instant column materialises as a JS `Date`, so the migration wrote a `Date.prototype.toString` rendering — whole seconds in the migrating host's zone, milliseconds dropped — into the new inbox and receipt rows. The migration is one-way. Both `created_at` and `read_at` now go through one canonicaliser matching the repo's existing correct form. Pinned with a hand-made `Date` under a forced process zone, which breaks the SQLite identity that kept the existing cases green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- .../migration-timestamp-canonicalisation.md | 32 +++++ .../migrate-sys-notification-to-event.test.ts | 131 ++++++++++++++++++ .../migrate-sys-notification-to-event.ts | 50 ++++++- 3 files changed, 211 insertions(+), 2 deletions(-) create mode 100644 .changeset/migration-timestamp-canonicalisation.md diff --git a/.changeset/migration-timestamp-canonicalisation.md b/.changeset/migration-timestamp-canonicalisation.md new file mode 100644 index 0000000000..46762a200d --- /dev/null +++ b/.changeset/migration-timestamp-canonicalisation.md @@ -0,0 +1,32 @@ +--- +"@objectstack/metadata": patch +--- + +fix(metadata): `migrateSysNotificationToEvent` writes canonical ISO timestamps, not `Date.prototype.toString` (#13998) + +`selectLegacyRows` reads the legacy `sys_notification` table through +`driver.raw`/`execute` — a door that does not run `formatOutput`, so none of its +repairs apply. On SQLite the legacy stamps come back as canonical ISO text and +`String(row.created_at)` is the identity. On Postgres and MySQL an instant +column materialises as a JS `Date`, so the migration wrote + +``` +Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard Time) +``` + +into `created_at` on the new `sys_inbox_message` row and into `created_at` / `at` +on the new `sys_notification_receipt` row — whole seconds in the **migrating +host's** zone with the milliseconds dropped, or a value no dialect's timestamp +grammar accepts at all. The migration is one-way, so that spelling is what the +platform would carry afterwards. + +Both stamps are now canonicalised at the migration, matching the repo's existing +correct form: a `Date` is rendered with `toISOString()`, ISO text passes through +untouched. Neither column could be repaired further upstream — `created_at` is a +builtin audit column that `formatOutput` repairs only in its `if (this.isSqlite)` +arm, and `read_at` is a legacy column ADR-0030 removed from the object, so it is +not a declared `Field.datetime` either and no coercion could ever reach it. + +Pinned with a hand-made `Date` driven through the migration's read path under a +forced process zone, which is what breaks the SQLite identity that kept the +existing cases green while the defect was live. diff --git a/packages/metadata/src/migrations/migrate-sys-notification-to-event.test.ts b/packages/metadata/src/migrations/migrate-sys-notification-to-event.test.ts index ba50a34428..3d51e57b7d 100644 --- a/packages/metadata/src/migrations/migrate-sys-notification-to-event.test.ts +++ b/packages/metadata/src/migrations/migrate-sys-notification-to-event.test.ts @@ -152,3 +152,134 @@ describe('migrateSysNotificationToEvent', () => { expect(result.error).toContain('.raw'); }); }); + +// --------------------------------------------------------------------------- +// [#13998] What this migration WRITES, when the legacy row hands out a `Date`. +// +// `selectLegacyRows` reads through `driver.raw`/`execute` — a door that does +// not run `formatOutput`, so none of its repairs apply. On SQLite the legacy +// stamps come back as canonical ISO TEXT and `String(row.created_at)` is the +// IDENTITY, which is why every case above stayed green while the defect was +// live. On Postgres and MySQL an instant column materialises as a JS `Date` +// (pinned in `driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`), +// and this migration is one-way: whatever spelling lands is what the platform +// carries afterwards. +// +// `@objectstack/metadata` has no driver dependency and must not grow one — the +// layering runs the other way — so, exactly like the OCC seam's own regression +// suite, the discriminating input here is a HAND-MADE `Date`. That is the whole +// point of these cases: they break the SQLite identity the cases above rely on. +// --------------------------------------------------------------------------- + +/** The instant from the production report, kept verbatim (#13567 / #13382). */ +const REPORTED_INSTANT = '2026-08-30T10:19:25.947Z'; +/** A second instant, so the receipt's `at` cannot pass by matching `created_at`. */ +const REPORTED_READ_INSTANT = '2026-08-31T02:03:04.567Z'; + +/** Canonical audit-timestamp text — what SQLite stores and what must be written. */ +const ISO_Z = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + +/** + * Run `body` with the process pinned to `tz`, then restore. + * + * Forced rather than required so these cases are non-vacuous on any runner: + * Test Core runs at UTC, a developer runs at whatever their laptop is set to. + * Restoring rather than assuming matters because vitest reuses a worker across + * files — a leaked `TZ` would silently re-zone whatever runs next in this + * process. Mirrors `underProcessZone` in the driver-side pin. + */ +async function underProcessZone(tz: string, body: () => Promise | T): Promise { + const previous = process.env.TZ; + process.env.TZ = tz; + try { + return await body(); + } finally { + if (previous === undefined) delete process.env.TZ; + else process.env.TZ = previous; + } +} + +describe('#13998 the timestamp spelling written into the new rows', () => { + it('control — `String(Date)` is NOT the canonical spelling (the input discriminates)', async () => { + const value = new Date(REPORTED_INSTANT); + expect(value.getMilliseconds(), 'the fixture is vacuous without sub-second digits').toBe(947); + + const spelled = await underProcessZone('Asia/Shanghai', () => String(value)); + // The prefix only: the trailing `(China Standard Time)` is the one + // implementation-defined part of `toString`. + expect(spelled.startsWith('Sun Aug 30 2026 18:19:25 GMT+0800')).toBe(true); + // Whole seconds in the PROCESS zone: the milliseconds are gone. + expect(Date.parse(spelled)).toBe(value.getTime() - value.getMilliseconds()); + expect(spelled).not.toBe(REPORTED_INSTANT); + expect(spelled).not.toMatch(ISO_Z); + // …and the canonical rendering of the same instant is zone-independent. + expect(value.toISOString()).toBe(REPORTED_INSTANT); + }); + + it('canonicalises a `Date` created_at/read_at into ISO on inbox, receipt and receipt.at', async () => { + const createdAt = new Date(REPORTED_INSTANT); + const readAt = new Date(REPORTED_READ_INSTANT); + const d = fakeDriver([ + { + id: 'n1', recipient_id: 'u1', type: 'mention', title: 'You were mentioned', + body: 'hi', url: '/x', actor_name: 'Ada', is_read: 1, + // The discriminating input: what Postgres/MySQL actually hand out. + read_at: readAt, created_at: createdAt, organization_id: 'org_1', + }, + ]); + const e = fakeEngine(); + + const result = await underProcessZone('Asia/Shanghai', () => + migrateSysNotificationToEvent({ driver: d.driver, data: e.engine })); + + expect(result.status).toBe('migrated'); + expect(result.migrated).toBe(1); + + const inbox = e.inserts.find((i) => i.object === 'sys_inbox_message')!; + const receipt = e.inserts.find((i) => i.object === 'sys_notification_receipt')!; + + // Every written stamp is canonical ISO-Z text — not a `Date`, and not a + // `Date.prototype.toString` rendering carrying the migrating host's zone. + for (const [where, written] of [ + ['inbox.created_at', inbox.row.created_at], + ['receipt.created_at', receipt.row.created_at], + ['receipt.at', receipt.row.at], + ] as const) { + expect(typeof written, `${where} must be written as text`).toBe('string'); + expect(written as string, `${where} must be canonical ISO-Z`).toMatch(ISO_Z); + // The zone the OLD spelling would have baked in is absent. + expect(written as string, `${where} must not carry a GMT offset`).not.toContain('GMT'); + } + + // The instants themselves are preserved to the millisecond — the half + // `String(Date)` silently dropped. + expect(inbox.row.created_at).toBe(REPORTED_INSTANT); + expect(receipt.row.created_at).toBe(REPORTED_INSTANT); + expect(receipt.row.at).toBe(REPORTED_READ_INSTANT); + // …and `at` is the READ stamp, not `created_at` echoed back. + expect(receipt.row.at).not.toBe(receipt.row.created_at); + + // The zone was restored rather than leaked into whatever runs next. + expect(process.env.TZ).not.toBe('Asia/Shanghai'); + }); + + it('leaves canonical ISO text exactly as it found it (the SQLite path is unchanged)', async () => { + const d = fakeDriver([ + { + id: 'n1', recipient_id: 'u1', type: 'mention', title: 'hi', body: null, + url: null, actor_name: null, is_read: 1, read_at: REPORTED_READ_INSTANT, + created_at: REPORTED_INSTANT, organization_id: 'org_1', + }, + ]); + const e = fakeEngine(); + + const result = await migrateSysNotificationToEvent({ driver: d.driver, data: e.engine }); + + expect(result.status).toBe('migrated'); + const inbox = e.inserts.find((i) => i.object === 'sys_inbox_message')!; + const receipt = e.inserts.find((i) => i.object === 'sys_notification_receipt')!; + expect(inbox.row.created_at).toBe(REPORTED_INSTANT); + expect(receipt.row.created_at).toBe(REPORTED_INSTANT); + expect(receipt.row.at).toBe(REPORTED_READ_INSTANT); + }); +}); diff --git a/packages/metadata/src/migrations/migrate-sys-notification-to-event.ts b/packages/metadata/src/migrations/migrate-sys-notification-to-event.ts index 64630bf702..1f9ef52ee8 100644 --- a/packages/metadata/src/migrations/migrate-sys-notification-to-event.ts +++ b/packages/metadata/src/migrations/migrate-sys-notification-to-event.ts @@ -100,7 +100,7 @@ export async function migrateSysNotificationToEvent( const recipientId = row.recipient_id != null ? String(row.recipient_id) : null; if (!recipientId) continue; // defensive — guarded by the SELECT filter const orgId = row.organization_id != null ? String(row.organization_id) : null; - const createdAt = row.created_at != null ? String(row.created_at) : now(); + const createdAt = row.created_at != null ? canonicalTimestampText(row.created_at) : now(); const title = row.title != null ? String(row.title) : (row.type != null ? String(row.type) : 'Notification'); const isRead = row.is_read === true || row.is_read === 1 || row.is_read === '1'; // One topic for both the inbox row and the rewritten event, so the @@ -128,7 +128,7 @@ export async function migrateSysNotificationToEvent( user_id: recipientId, channel: 'inbox', state: isRead ? 'read' : 'delivered', - at: isRead && row.read_at != null ? String(row.read_at) : createdAt, + at: isRead && row.read_at != null ? canonicalTimestampText(row.read_at) : createdAt, organization_id: orgId, created_at: createdAt, }); @@ -170,6 +170,52 @@ export async function migrateSysNotificationToEvent( // Internal helpers // --------------------------------------------------------------------------- +/** + * The canonical text spelling of a timestamp read back out of the legacy table. + * + * `selectLegacyRows` reads through `driver.raw`/`execute`, which hands the + * dialect client's own materialisation straight back — that door does not run + * `formatOutput`, so none of its repairs apply here on any dialect: + * + * - `created_at` is a BUILTIN audit column, so it is never in `datetimeFields` + * and no declared-field coercion reaches it; `formatOutput` repairs it only + * inside its `if (this.isSqlite)` arm (`repairNaiveUtcAuditTimestamp` over + * `AUDIT_TIMESTAMP_COLUMNS`). + * - `read_at` is a LEGACY column ADR-0030 removed from the object, so it is + * not declared either — it can never enter `datetimeFields`, and it is not + * an audit column, so no arm of `formatOutput` could reach it even at the + * record read door. + * + * On SQLite both arrive as canonical ISO text and `String()` is the identity — + * which is why every test in this directory stayed green. On Postgres and + * MySQL an instant column materialises as a JS `Date` + * (`withPostgresCalendarDayAsText` leaves the instant types alone deliberately; + * pinned in `sql-driver-13567-audit-stamp-materialisation.test.ts`), and + * `String(date)` spells + * + * Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard Time) + * + * — whole seconds in the MIGRATING HOST's zone, with the milliseconds gone. + * This migration is one-way and this value is WRITTEN, so that spelling is what + * the platform would carry afterwards: either accepted and stored skewed and + * de-precisioned, or rejected outright, since the trailing zone name is in no + * dialect's timestamp grammar (#13998). + * + * Canonicalising HERE, at the consumer that writes, is deliberate and is the + * only shape that could also repair an already-migrated deployment (#13973 + * option A). It is not a tolerant alias: `Date` and ISO text are two + * materialisations of ONE instant, not two spellings of a key. Matches the + * repo's existing correct form at `metadata-protocol/src/protocol.ts` (the + * `occurred_at` read in `readMetadataAuditEvents`); anything that is neither a + * string nor a `Date` keeps its previous `String()` rendering unchanged rather + * than having a unit guessed for it on a one-way write path. + */ +function canonicalTimestampText(value: unknown): string { + if (typeof value === 'string') return value; + if (value instanceof Date) return value.toISOString(); + return String(value); +} + async function selectLegacyRows(driver: any): Promise { const result: any[] = await driver.raw( `SELECT id, recipient_id, type, title, body, url, actor_name, is_read, read_at, created_at, organization_id ` +