fix(metadata): emit the declared ISO string at the five adapter boundaries that cast a driver Date - #14939
Conversation
…ndary casts Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…two new test files Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
📓 Docs Drift Check5 anchor(s) derived from 2 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 2aa6f79e2e858aabd1fea891c409cb3872e38fdf && git checkout 2aa6f79e2e858aabd1fea891c409cb3872e38fdf
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7317cf266e9682edc830ccac6fe78e66b3ddfc32 2a96397bcb692f4e77687255447a6457a779017a && git checkout -B drift-repro 7317cf266e9682edc830ccac6fe78e66b3ddfc32 && git merge --no-ff 2a96397bcb692f4e77687255447a6457a779017a
node scripts/docs-audit/affected-docs.mjs --json 7317cf266e9682edc830ccac6fe78e66b3ddfc32 |
Fixes #14037
Five metadata adapter boundaries asserted
as stringover a driver timestamp column that the live dialects hand out as a JSDate. They now emit the ISO-8601 string their declared type promises, at the producer.The sharp edge, first
Three of the five land in fields declared
z.string().datetime(), notz.string(). That is not "the type is imprecise" — it is a refinement that rejects outright. The only reason nothing is red today is that no production path parses these values:rowToRecord's output is consumed as an already-typedMetadataRecordthroughoutdatabase-loader.tswith nothing revalidating it. The day anyone adds a.parse()on one of these paths, the production default driver fails it. Measured during this card: no such parse exists yet, so the p2 grade stands and no severity re-grade is owed.The five sites, re-verified on
origin/main431979e67The card's line numbers had drifted; the sites themselves are exactly the five it names.
metadata-protocol/src/sys-metadata-repository.ts:1157(rowToEvent)ts: (row.recorded_at as string) ?? new Date(0).toISOString()MetadataEvent.ts—z.string(),metadata-core/src/types.ts:147metadata/src/loaders/database-loader.ts:746(rowToRecord)createdAt: row.created_at as string | undefinedMetadataRecord.createdAt—z.string().datetime(),spec/src/system/metadata-persistence.zod.ts:139metadata/src/loaders/database-loader.ts:748(rowToRecord)updatedAt: row.updated_at as string | undefinedMetadataRecord.updatedAt— same file,:141metadata/src/loaders/database-loader.ts:1080(getHistoryRecord)recordedAt: row.recorded_at as stringMetadataHistoryRecord.recordedAt— same file,:452metadata/src/loaders/database-loader.ts:1161(queryHistory)recordedAt: row.recorded_at as stringAll five are unchecked casts, which is why tsc reported nothing: the
stringis an assertion about a driver row, never a measurement of one.Why both column classes are affected
SqlDriver#formatOutputrepairs the BUILTIN audit columns (repairNaiveUtcAuditTimestamp) and folds declaredField.datetimecolumns (normalizeSqliteDatetimeOutput) only inside itsif (this.isSqlite)arm — verified atsql-driver.ts:16048, where both loops sit.withPostgresCalendarDayAsTextleavestimestamptz/timestampalone on purpose, because those are instants. So a column being declaredField.datetimedoes not protect it, and on Postgres and MySQL both classes come out of the record read door as aDate. That dialect asymmetry is pinned live inpackages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts.The route, and the two prohibitions kept
Producer-side canonicalisation at the adapter boundary that asserts the declared type — the route #13997 established. Not a tolerant
??fallback in a consumer (#13973's standing prohibition), and not route B at the driver's read door, which would reverse a deliberate driver decision and belongs to the whole census rather than to this card.The obvious spelling was to call the
canonicalIsoInstanthelper that #13997 already left in both of these files, which would have made four of five sites a one-word change. This PR deliberately does not do that, and the reason is a live adjudication rather than taste.canonicalIsoInstant(and itscanonicalIsoStampsibling inrest-server.ts, andauditMetaIteminprotocol.ts) reachesvalue.toISOString()for anyDate, which raisesRangeError: Invalid time valueon an InvalidDate. #14078 measured that input to be reachable on both live dialects — a MySQL zero datetime returnsmysql2'sINVALID_DATEconstant by name, and any Postgres year in 275760..294276 materialises throughpostgres-dateasnew Date(NaN). Whether the shared spelling should throw there (option A) or fall back to a rendering (option B) is a maintainer call across four packages, and #13973 ispm:blockedon it.So adopting it here would have imported option A's consequence into five new call sites while the question is open, turning a silently-wrong field into an uncaught 500 on a read path. Instead each file gets a narrow local helper that converts only a valid
Dateand returns every other shape untouched:An Invalid
Datetherefore reaches the consumer exactly as it does today — no new throw, no invented rendering, nothing for #14078 to un-decide. TheNumber.isNaN(value.getTime())guard is the spelling already in use atrest/src/export-format.ts:291andrest/src/import-prepare.ts:115, not a new one. When #14078 rules, both helpers collapse into the shared spelling; each docblock says so.What adopting
canonicalIsoInstantwould have changed, site by site, for the Invalid-Dateinput only (every other input is identical either way):database-loader.tsrowToRecordcreatedAt/updatedAt— would throw insiderowToRecord, takingload()andstat()with it.database-loader.tsgetHistoryRecordrecordedAt— would throw on the history read.database-loader.tsqueryHistoryrecordedAt— would throw on the history page.sys-metadata-repository.tsrowToEventts— would throw inside thehistory()generator and insidewatch's replay.Two new test cases pin the neutral behaviour, so this stays a measured decision rather than a claim.
⛔ Two things this PR does not do, both deliberate: it does not decide #14078, and it does not promote the helper to a shared export in
@objectstack/metadata-core. The card floated that consolidation and triage allowed it in scope, but it would widen a package's public surface for a spelling #14078 is about to consolidate anyway — the smaller, reversible move is two locals with docblocks pointing at each other.Behaviour preserved for every non-
Dateshapeundefined, so each caller's existing?? defaultchain keeps exactly its current meaning — includingrowToEvent's epoch fallback (pinned).stat()'smtime, the one in-repo consumer ofrowToRecord's timestamps, reads throughcanonicalIsoInstantand now receives a string it returns unchanged.Measurements the card asked for
metadata-core/src/**,metadata-fs/src/repository.ts,metadata/src/metadata-manager.ts) is falsified: zero hits in all three, andmetadata-fs/src/repository.tsandmetadata-manager.tscontain noas stringcast at all, while the same grep shape returned hits in the two real files.DatabaseLoader's seam isIDataDriver/IDataEngineandSysMetadataRepository's is an engine, so both suites drive a hand-madeDatein process, which is also what keeps these packages free of a driver dependency.Date. None — measured, not assumed.MetadataEvent.tshas exactly one in-repo reader,MetadataManager.applyRepoEventatmetadata-manager.ts:2933, which forwards it toMetadataWatchEvent.timestamp, itself declaredz.string().datetime()— so converting repairs that consumer rather than breaking it.recordedAt's only readers (rollback,diff) touch.metadataand never the timestamp.rowToRecord's timestamps never leave the loader:load()reads onlyrecord.checksumandstat()passes them throughcanonicalIsoInstant. No stop condition triggered.Out of scope, filed rather than fixed
The card's lower-confidence neighbour is a real sixth site of the same class, and the declaration it could not locate is an inline TypeScript return type rather than a Zod schema:
listDraftsdeclaresupdatedAt: string | nullatsys-metadata-repository.ts:1108and emitsrow.updated_at ?? row.created_at ?? nullat:1140, withrowscastas any[]one line above. Triage ruled that site explicitly out of this card with "file a separate card if it is real", so it is filed as #14938 and untouched here. #14938 is not addressed by this PR.Verification
Gate union re-run at the final commit
2a96397bc, derived bynode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandswith no path arguments, on the final six-path change set: 47 commands, 43 exit 0, 0 findings. Four exited 3 (PREREQUISITE NOT MET, recorded as NOT MEASURED, each by its own printed verdict):check-test-completenessandpm/check-half-stateshave no local input,check:dual-build-cjs-loadsandcheck:type-check-debtneed a whole-repo build.check:engine-double-contractreported the two new test files as new pinned coverage the ledger did not record;--writeadded 4 rows, 0 lost.pnpm --filter @objectstack/metadata --filter @objectstack/metadata-protocol test— 691/691 and 2339 passed, 10 skipped.pnpm --filter @objectstack/metadata-protocol typecheck— clean;tsc --listFilesconfirms the new test file is in the program (1 hit, against a control hit for theMetadataItem.authoredAtis declaredz.string()but receives a JSDateon Postgres/MySQL — a silent declared-contract violation, because the schema is parsed only in its own test #13997 sibling).packages/metadatadeclares notypecheckscript and carries a DEBT ledger entry of 89; a directtsc --noEmitthere reports exactly 89, none naming either changed file. That is a corroborating reading, not the ratchet's re-measure, which needs the whole closure built.pnpm lint— whole repo,eslint . --no-inline-config, clean in 95s. No narrowing claimed.node scripts/pm/check-governed-merges.mjs --teston the final file list — 0 of 6 paths governed.Ablation. Both helper bodies were mutated to a bare
return value;— the pre-fix behaviour at all five sites in one edit per file — with the mutation confirmed on disk in both directions (guard-line count 0 in each file; whole-treegit diff --statshowing the two deletions) and both packages rebuilt;scripts/ablation-dist-preflight.mjs --absentconfirmed the guard had left bothdist/trees. Predicted direction was stated before the run and matched: 3 of 7 red inmetadata, 1 of 4 red inmetadata-protocol, with the SQLite-passthrough, absent-column and #14078-neutrality cases correctly staying green because the mutation does not change them. Restore leg:git checkout HEAD --on absolute paths, proven by an emptygit diff HEAD, a whole-treegit status --porcelainwith no output, and agit hash-objectmatch against each path's HEAD blob; both packages rebuilt and the preflight confirmed the guard back indist/; the restored run is 7/7 and 4/4 green.🤖 Generated with Claude Code
https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
Generated by Claude Code