Uh oh!
There was an error while loading. Please reload this page.
fix(cli): os migrate duplicates reports every holder's createdAt as canonical ISO-8601 UTC on every dialect (#13999) - #14252
Conversation
…O-8601 UTC (#13999) `DuplicateHolder.createdAt` is declared `string | null`, but the mapper built it with `String(row.created_at)`. `created_at` is a builtin audit column, so no presentation runs on the holder probe's raw-SQL path: Postgres and MySQL materialise a JS `Date` and the operator read a `Date.toString()` rendering with their local zone baked in, whole seconds and no `Z`, while SQLite printed canonical ISO-Z. One instant, two spellings, chosen by the dialect. Canonicalised at the mapper, following the `occurredAt` form already in packages/metadata-protocol/src/protocol.ts. No driver file is touched and no tolerant fallback is added. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YPiiDdw96RGS25WLctCQP
…plicates-createdat-iso
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 23 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 fced68788dde20f734d16209551e92d801be9b65 && git checkout fced68788dde20f734d16209551e92d801be9b65
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7d3b1b79c4fac1b49e96dbd0cfe41a1cf4002d16 bb642f548685350eacec4fc20d9c124b1fa2ab8c && git checkout -B drift-repro 7d3b1b79c4fac1b49e96dbd0cfe41a1cf4002d16 && git merge --no-ff bb642f548685350eacec4fc20d9c124b1fa2ab8c
node scripts/docs-audit/affected-docs.mjs --json 7d3b1b79c4fac1b49e96dbd0cfe41a1cf4002d16
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#13999
DuplicateHolder.createdAtis declaredstring | null, and the holder mapper built it withString(row.created_at).created_atis a builtin audit column — not indatetimeFields, andSqlDriver#formatOutputrepairs it only inside itsif (this.isSqlite)arm — and the holder probe reads through the raw-SQL seam, so no presentation runs on this path at all. The dialect therefore decided what the operator saw:One instant, two spellings, chosen by the dialect: the operator's local zone baked in, whole seconds instead of milliseconds, no
Z, and notDate.parse-safe for anything consuming this command's JSON.The fix, and where it deliberately does not land
Canonicalised at the mapper — a new
canonicalHolderCreatedAtinpackages/cli/src/commands/migrate/duplicates.ts, called from the one assignment site. The CLI is a leaf consumer with a declaredstring | null, so it is the side that owes the canonical spelling.packages/drivers/. Giving the driver one presented shape per dialect at the read door would repair this site for free, but it reverses a deliberate driver decision (SqlDriver.withPostgresCalendarDayAsText) and is a maintainer call on 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 as a whole. The driver pinpackages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.tswas read for this card and is neither duplicated nor edited.??hedge answers it by hiding it. Nothing downstream is taught to accept a second spelling.One correction the card could not carry
Ruling 1 cites the repo's existing correct form as
packages/metadata-protocol/src/protocol.ts:7710-7715. That range has drifted since the card was authored. Resolved by deepening history and reading the file atb1b7d6088(the tip at authoring time): the cited range is theoccurredAtmapper, which today lives atpackages/metadata-protocol/src/protocol.ts:8074-8079, byte-identical. That is the form followed here — the structural twin of this site, a leaf mapper turning a builtin audit timestamp into a declaredstring.Two arms left exactly as they were
created_atcolumn still reportscreatedAt: null, through the probe's realwithCreatedAt: falseretry.Datecarrying no time value keeps its verbatim rendering.toISOString()throws on one (RangeError: Invalid time value), andmysql2hands back exactly that for a zero date. A non-instant has no canonical spelling; a defect that was a spelling in a read-only report must not become a crashed migration command. This is the one deviation from the cited form, and it follows the other precedent in the same producer file —canonicalVersionInstantguards the identical hazard withNumber.isFiniteandMAX_TIME_VALUE.The p3 grade re-derived, since the fix shape rests on it
The card grades this p3 because
createdAthere is reported, never compared. Re-measured on this branch: the identifier appears at exactly five lines induplicates.ts—:132doc comment,:133the declaration,:325doc comment,:346the query'sAS created_atprojection,:684the assignment — and at no comparison and no sort. The only sort on holders keys onpartitionthenid. No wrong record is chosen and nothing is written. The grade stands.The test distinguishes the dialects
packages/cli/src/commands/migrate/duplicates.created-at-canonical.test.ts. Every existing pin on this command drives SQLite, which is the side that was already correct — so a SQLite-only test proves nothing about the defect, and this file exists to separate the two.Date, which is not this file's claim to make but the fact pinned against live servers in the driver file above. No runner here hosts a Postgres or a MySQL.Both legs run with the process zone forced to
Asia/Shanghai, so the canonical spelling §A1 asserts is produced while the process is demonstrably not at UTC.Ablation — the pin really catches this defect
The implementation was committed first, then the mapper reverted to
String(row.created_at)on disk (mutation confirmed by counting both the injected and the removed text: injected 1, removed 0; blob3e42719to97624e5), the file re-run, then restored and proven byte-identical to theHEADblob (git diff HEADempty, hash back to3e42719).The SQLite leg staying green under the ablation is the point: it is the measured form of "a test exercising only SQLite pins the side that was already correct".
Verification
All at
bb642f5486, the head of this branch.pnpm --filter @objectstack/cli exec vitest run --maxWorkers=2 src/commands/migrate/duplicates— Test Files 6 passed (6), Tests 35 passed (35), the wholeduplicatesfamily including the boot-heavy integration and null-seam suites.pnpm --filter @objectstack/cli exec tsc --noEmit --listFiles— exit 0, no diagnostics.--listFilesused deliberately: both edited files are proven in the tsc program (1 hit each), so this is a measurement and not a green over source nothing read.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackwere run, re-derived after the last edit and reconciled withcommin both directions — nothing derived went unrun, nothing run was underived. Exit codes captured before any pipe. 33 green; the two non-zero are both exit 3, PREREQUISITE NOT MET, which each gate's own text states is not a red and not a measurement:check-test-completeness.mjsgrades a savedturbo run testlog and none exists locally, andscripts/pm/check-half-states.mjsneeds repo-scoped REST reads this container's egress refuses (GET /rate_limit200 with 15000 left,GET /repos/...403 with no rate-limit headers). Both are recorded as NOT MEASURED.pnpm check:type-check-debt— the ratchet ran whole rather than narrowed: 27 ledger entries re-measured in 469.0s, 1217 raw tsc errors total, none above its recorded number, surplus none.pnpm lint(eslint . --no-inline-config, the repo-wide scan) — run whole, exit 0, 112s. No narrowing claimed and none needed.pnpm check:nul-bytes— OK over 7781 text files; the edited files additionally hand-scanned for raw control bytes, none found.Generated by Claude Code