Uh oh!
There was an error while loading. Please reload this page.
fix(metadata): canonicalise the timestamps migrateSysNotificationToEvent writes - #14024
Conversation
…vent` writes `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
📓 Docs Drift Check2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 12 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin c98582dd0e0827bc597c495fd557b1dbd2730b24 && git checkout c98582dd0e0827bc597c495fd557b1dbd2730b24
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c54d4d3d7d0a7f3e6ec848dd0cb415ee35ebbb37 3fc4e3bf24a7bc75fdca1cbed7e36a9adc89a04b && git checkout -B drift-repro c54d4d3d7d0a7f3e6ec848dd0cb415ee35ebbb37 && git merge --no-ff 3fc4e3bf24a7bc75fdca1cbed7e36a9adc89a04b
node scripts/docs-audit/affected-docs.mjs --json c54d4d3d7d0a7f3e6ec848dd0cb415ee35ebbb37 |
Uh oh!
There was an error while loading. Please reload this page.
Part of #13998 — the code half only. The data half of that card (whether any deployment has already run this migration against Postgres or MySQL, and therefore whether a backfill is owed) is unanswered and is a maintainer floor, so this PR deliberately does not carry a closing keyword. Reading below.
All gates and tests quoted here were run at
3fc4e3bf24, which is this branch's head.What was wrong
selectLegacyRowsreads the legacysys_notificationtable throughdriver.raw/execute, and the migration then wroteinto
created_aton the newsys_inbox_messagerow and intocreated_at/aton the newsys_notification_receiptrow. On Postgres and MySQL an instant column materialises as a JSDate, soString(...)spells— whole seconds in the migrating host's zone, milliseconds dropped, and a trailing zone name that is in no dialect's timestamp grammar. This migration is one-way, so that spelling is what the platform carries afterwards.
The fix
One canonicaliser, applied at both sites, matching the repo's existing correct form (
metadata-protocol/src/protocol.ts, theoccurred_atread inreadMetadataAuditEvents):Route A of #13973 (canonicalise at the migration), as triaged — not the driver-side read-door normalisation, and not a tolerant
??fallback. ADateand ISO text are two materialisations of one instant, not two spellings of a key. Anything that is neither a string nor aDatekeeps its previousString()rendering unchanged rather than having a unit guessed for it on a one-way write path; widening that would be a second, unevidenced decision.Why neither column could be repaired further upstream
This read path bypasses
formatOutputentirely —execute()returnsawait builderuntouched, the same class of bypasssql-driver.ts:3964-3965already names foraggregateanddistinct. Even at the record read door the two columns diverge by different gates, and neither one closes:created_atis a builtin audit column, so it is never indatetimeFieldsand no declared-field coercion reaches it;formatOutputrepairs it only inside itsif (this.isSqlite)arm (repairNaiveUtcAuditTimestampoverAUDIT_TIMESTAMP_COLUMNS = ['created_at', 'updated_at'],sql-driver.ts:269).read_atis a legacy column ADR-0030 removed from the object, so it is not a declaredField.datetimeeither —datetimeFieldsis built from declared datetime columns, andread_atappears nowhere inpackages/platform-objects/srctoday. No arm offormatOutputcould reach it on any dialect.The pin, and why it is the real content here
The defect was invisible because this migration's tests drive SQLite/memory shapes, where the legacy stamps are already canonical ISO text and
String(row.created_at)is the identity. The new cases break that identity by feeding a hand-madeDate— the discriminating input — through the migration's read path under a forced process zone.@objectstack/metadatahas no driver dependency and must not grow one, so a hand-madeDateis the right instrument, exactly as the OCC seam's own suite uses one.They live in the existing test file and reuse its already-pinned engine doubles, so
check:engine-double-contract's ledger is untouched.Ablation, on the committed tree, no rebuild leg (the pin imports
./migrate-sys-notification-to-event.jsrelatively, so vitest resolves source;dist/is not on this path — stated rather than fabricated):String(row.created_at)/String(row.read_at)count 2, removedcanonicalTimestampText(row.count 0 — and by blob:1f9ef52ee8tofb4534c3e5Tests 1 failed | 7 passed (8), failing precisely on the new case withAssertionError: inbox.created_at must be canonical ISO-Z: expected 'Sun Aug 30 2026 18:19:25 GMT+0800 (Ch…' to match /^\d{4}-\d{2}-\d{2}T…/— the production spelling, reproducedtrap ... EXIT INT TERMwith absolute paths, proven by an emptygit diff HEAD(0 bytes from--name-only), a cleangit status, and a restored blob matching the HEAD blob1f9ef52ee8Verification
pnpm --filter '@objectstack/metadata^...' build—VERDICT command-exit 0pnpm --filter @objectstack/metadata test—Test Files 38 passed (38),Tests 628 passed (628)node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackat3fc4e3bf24(34 commands, path-derived plus the test-file convention kind): 33 exit 0, 1 NOT MEASURED —node scripts/check-test-completeness.mjsexits 3 with no test-run log to read, which its own output states is not a red and not a findingpnpm check:type-check-debtre-measured 29 ledger entries at this head and reportssurplus: none — every entry sits exactly at its measurement, so any new error is red, so the new test code adds zero tsc errors to@objectstack/metadata's frozen 89pnpm check:dual-build-cjs-loadsandpnpm check:type-check-debtboth needed a built workspace (turbo run build --filter='./packages/*' --filter='./packages/*/*', 70/70 successful) and were re-run green afterwards rather than left at their exit-3 prerequisite stateESLint — a declared narrowing, measured on three counts rather than skipped. Run on the changed files only,
--no-inline-config --format json: 0 errors, 0 warnings.--print-configresolves a real config for both.tsfiles (6 and 5 active rules), and ESLint itself reports the changeset asFile ignored because no matching configuration was supplied.--format json: 3 result objects, 2 linted, 1 ignored.--print-configshows noparserOptions.projecton either file, andeslint.config.mjs:328records — measured with a positive control — that this reponever enables type-aware linting ... for ANY file. Every file's verdict therefore depends only on its own bytes and the shared config, and this diff changes neither the config nor any other file.Readings the card asked for
docs/handoff/adr-0030-notification-convergence.mdlabels it "Data migration (not auto-run)" and hands operators a manual cut-over sequence), and it is not in thesys_migrationledger — the well-known ids are onlyadr-0104-file-referencesandadr-0104-value-shapes. Meanwhile it is published:migrateSysNotificationToEventappears in seven package CHANGELOGs. What narrows the population is the finding below: no driver in this repo defines.raw, so the documented call errors out before touching data. That leaves only an operator who passed a knex-like handle of their own, which the repo cannot see either way.OS_TEST_POSTGRES_URLandOS_TEST_MYSQL_URLare unset and nothing is listening in this container, so no live dialect was reachable. The fix is safe under either shape: it removes the input to both, since the value written is canonical ISO text on every dialect. The JavaScript half that needs no server — thatString(Date)drops the milliseconds and bakes the process zone — is measured in the pin.read_atshare the defect, and by which gate. Yes, and by a different gate thancreated_at— the two bullets above. Both are covered by the fix.packages/metadata/src/migrations/returns the two lines in this file and nothing else; widened toString\((row|r|legacy|old)\.[A-Za-z_]*_at\bacross all ofpackages, the other hits are read-side sites already accounted for by the [finding] Sweep: which consumers compare or format a value whose runtime type differs between the Date-materialising drivers and the ISO-text ones #13973 census family (objectql/engine.ts,rest-server.ts,platform-objects/system/migration-flag.ts,metadata-protocol/protocol.ts,cli/commands/migrate/duplicates.ts) plusservices/service-queue/src/db-queue-adapter.ts:262. The three other files in this migrations directory contain no_atreference at all.Out-of-scope finding, filed not folded
#14023 — every migration in
packages/metadata/src/migrations/requiresdriver.raw(...), and no driver in this repo defines that method; they all exposeexecute(). Deduped against the backlog first. Different defect class, so it is not repaired here, and #14023 remains open on its own terms.Generated by Claude Code
Generated by Claude Code