Skip to content

os migrate duplicates reports each holder's createdAt as a Date.toString() spelling on Postgres/MySQL #13999

Closed

Description

@zhuangjianguo

Found by the driver-materialisation consumer census on #13973 (class (c) — genuinely wrong on one side, lowest severity of the seven the census found). That sweep is not addressed by this card and does not close it.

The site

packages/cli/src/commands/migrate/duplicates.ts:684, building a DuplicateHolder:

const holder: DuplicateHolder = {
  id: row.holder_id == null ? null : String(row.holder_id),
  organization,
  partition: organization ?? globalTenant,
  createdAt: row.created_at == null ? null : String(row.created_at),   // <-- here
};

row comes from the CLI's own holder query, which selects the column explicitly at line 346:

const created = withCreatedAt ? `, ${quoteIdent('created_at', client)} AS created_at` : '';

DuplicateHolder.createdAt is declared string | null (line 133).

Why it is wrong on the production default driver

created_at is a builtin audit column: not in datetimeFields, and SqlDriver#formatOutput repairs it only inside if (this.isSqlite) — and this path reads through knex directly anyway, so no presentation runs at all. Pinned in packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts: the live dialects materialise it as a JS Date, SQLite as canonical ISO-Z text.

So the operator running os migrate duplicates against Postgres or MySQL sees

Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard Time)

where the same command against SQLite prints 2026-08-30T10:19:25.947Z — whole seconds, the operator's local zone baked in, no Z, and not Date.parse-safe for anything consuming the CLI's JSON output.

Why it is ranked lowest, and why it is still class (c)

createdAt here is reported, never compared — measured: the identifier appears at exactly three places in the file (:133 the declaration, :325 a doc comment, :684 this assignment) and at no comparison or sort. So no wrong record is chosen and nothing is written; the defect is confined to a spelling in an operator-facing report and in the command's JSON.

It is still class (c) rather than (b) because the value is wrong on one side, not merely untested there: string | null is declared, a Date-derived non-ISO string is delivered, and which one you get depends on the dialect.

Why this is not a ?? fallback

Per #13973's standing prohibition, the question is which side owes the canonical spelling. Here the CLI is a leaf consumer with a declared string | null, so the cheapest correct shape is canonicalise at the mapper — the repo's existing correct form is packages/metadata-protocol/src/protocol.ts:7710-7715. If the producer-side option (one presented shape per dialect at the read door) is taken for the census as a whole, this site is repaired by it for free — but that reverses a deliberate driver decision (withPostgresCalendarDayAsText) and is a maintainer call.

Re-run

rg -n 'String\(row\.created_at\)' packages/cli/src/

Backlink: #13973 (census), #13382 (the OCC seam, the same class). Neither is addressed here.

Activity

  1. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Sep 1, 2026
  2. self-assigned this
    on Sep 1, 2026
  3. os-justin commented on Sep 1, 2026

    @os-justin
    Collaborator

    Claim: PM loop round 64
    Session: session_015YPiiDdw96RGS25WLctCQP
    Branch: claude/issue-13999-duplicates-createdat-iso
    Worktree: objectstack-issue-13999
    Domain: domain:cli
    File surface: packages/cli/src/commands/migrate/duplicates.ts (the DuplicateHolder mapper, confirmed at :684 on origin/main this round) plus its test. ⛔ Zero edits to packages/drivers/** — see the fence. (stop on breach; explain in the report)
    Container & model: S but ⛔ not mechanical (the canonicalisation site is a judgement, not a gate verdict) ⇒ treated as M, mode:subagent, model: opus
    Clause-②: no — a leaf CLI consumer already declaring string | null; this delivers the declared type instead of a Date-derived spelling. ⛔ No public contract widens. ⚠️ Re-declare from your diff.
    Serial constraints cleared: no open PR touches this file (all 9 open PRs read this round). Sibling dispatches: #13860 (packages/cli/src/utils/spec-version.ts + validate/doctor/compile — same package, different files ⇒ ruling ① exemption) and #13904 (packages/rest). ⚠️ Read coupling checked: neither pins this mapper or the created_at spelling.

    Comments re-read before claiming: one. ⛔ No claim from another session.

    The fix, and the fence that decides where it lands

    packages/cli/src/commands/migrate/duplicates.ts:684 — verified present this round:

    createdAt: row.created_at == null ? null : String(row.created_at),

    DuplicateHolder.createdAt is declared string | null. On Postgres/MySQL created_at materialises as a JS Date (builtin audit column, not in datetimeFields, and this path reads through knex directly so no presentation runs), so String() yields Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard Time) — local zone baked in, whole seconds, no Z, ⛔ not Date.parse-safe for anything consuming the command's JSON. SQLite yields canonical ISO-Z. Same command, two spellings, dialect-dependent.

    ⛔ Canonicalise at the mapper — this side owes it. The repo's existing correct form is packages/metadata-protocol/src/protocol.ts:7710-7715; follow it rather than inventing a shape.

    ⛔ Do NOT repair this on the producer side. Giving the driver one presented shape per dialect at the read door would fix this site for free, but it reverses a deliberate driver decision (withPostgresCalendarDayAsText) and is a maintainer call on the #13973 census as a whole, ⛔ not this card's to take. If your reading says the mapper cannot be made correct without a driver change, stop and report — ⛔ do not widen.

    ⛔ Not a ?? fallback. Per #13973's standing prohibition the question is which side owes the canonical spelling, and a tolerant fallback answers it by hiding it.

    ⚠️ PM mechanism assumption — falsify it if the tree disagrees

    The card measures createdAt here as reported, never compared: the identifier appears at exactly three places in the file (:133 declaration, :325 doc comment, :684 this assignment) and at no comparison or sort. That is the entire basis for grading this p3 rather than higher, because it means ⛔ no wrong record is chosen and nothing is written.

    ⇒ Re-derive that count on your own tree before you rely on it. If you find a comparison or a sort on this value, ⛔ stop and report — the grade moves and so does the shape of the fix.

    Verification bar

    • The test must distinguish the dialects, not merely assert the mapper's output for one of them. A test that only exercises SQLite pins the side that was already correct and proves nothing about the defect.
    • ⚠️ Reference pin for the underlying materialisation difference: packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts — read it, ⛔ do not duplicate or edit it.
    • ⛔ A null holder must stay null; that arm is not what is broken.

    Generated by Claude Code

  4. os-justin commented on Sep 1, 2026

    @os-justin
    Collaborator

    Claim: os-dev implementer (dispatched by the PM claim above)
    Session: session_015YPiiDdw96RGS25WLctCQP (os-dev subagent seat under the round-64 PM session; the PM comment above is the dispatching claim, this one is the implementing claim)
    Branch: claude/issue-13999-duplicates-createdat-iso (pushed empty before the first edit — write route green)
    Worktree: objectstack-issue-13999, base f394614486a47ca23c3b9fb5978f2eed39f4f8d4
    File surface: packages/cli/src/commands/migrate/duplicates.ts plus a new sibling test in packages/cli/src/commands/migrate/. Zero edits to packages/drivers/ — the fence holds; the reference pin is read only.

    Comments re-read before claiming: two (triage + the PM claim). No claim from another implementer session.

    Premise re-derived on my own tree — it holds

    The site, present verbatim at packages/cli/src/commands/migrate/duplicates.ts:684:

    createdAt: row.created_at == null ? null : String(row.created_at),

    The p3 grade's basis, re-measured. grep -n 'createdAt\|created_at' over the file returns exactly five lines: :132 doc comment, :133 the string | null declaration, :325 doc comment, :346 the query's AS created_at projection, :684 this assignment. No comparison, no sort, no orderBy, no write. The card's measurement is confirmed and the grade does not move.

    One correction to carry forward: the cited line range has drifted

    Zone 1 ruling 1 names packages/metadata-protocol/src/protocol.ts:7710-7715. On today's origin/main that range is unrelated overlay-loading code. Resolved by deepening history and reading the file at b1b7d6088acc6aa2d1afe80ac559757d838ccd2e (the tip when this card was authored): :7710-7715 there is the occurredAt mapper, which today lives at packages/metadata-protocol/src/protocol.ts:8074-8079, byte-identical:

    occurredAt:
        typeof r.occurred_at === 'string'
            ? r.occurred_at
            : r.occurred_at instanceof Date
                ? r.occurred_at.toISOString()
                : String(r.occurred_at ?? ''),

    That is the form I follow — the structural twin of this site: a leaf mapper turning a builtin audit timestamp into a declared string. Not a ?? fallback; not a driver change.


    Generated by Claude Code

  5. os-justin commented on Sep 1, 2026

    @os-justin
    Collaborator

    os-dev-report

    {
      "issue": 13999,
      "status": "done",
      "branch": "claude/issue-13999-duplicates-createdat-iso",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/14252",
      "premise_still_valid": true,
      "summary": "Canonicalised at the mapper: new exported `canonicalHolderCreatedAt` in packages/cli/src/commands/migrate/duplicates.ts, called from the single assignment at the site formerly spelled `String(row.created_at)`. A `string` passes through, a JS `Date` becomes `toISOString()`, `null` stays `null`. Zero edits to packages/drivers (the driver pin was read only); no `??` fallback; no public contract widens. PM MECHANISM ASSUMPTION RE-DERIVED AND HOLDING: `createdAt`/`created_at` appears at exactly five lines (:132 doc, :133 declaration, :325 doc, :346 the query projection, :684 the assignment) and at no comparison and no sort - the only holder sort keys on partition then id - so the p3 grade and the fix shape stand. ONE CARD CORRECTION: ruling 1's cited form `protocol.ts:7710-7715` has drifted; resolved by deepening history and reading the file at b1b7d6088 (the tip at card-authoring time), where that range is the `occurredAt` mapper, today at protocol.ts:8074-8079 byte-identical - that is the form followed. ONE DELIBERATE DEVIATION from that form, declared rather than silent: a `Date` carrying no time value keeps its verbatim rendering, because `toISOString()` throws RangeError on one and mysql2 produces exactly that for a zero date - turning a spelling defect in a read-only report into a crashed migration command would be a regression. It follows the other precedent in the same producer file (`canonicalVersionInstant` guards the identical hazard). NOT WIDENED, flagged for your call rather than filed: the epoch-milliseconds input arm (`String(n)` yields a bare integer) is left as the cited form leaves it - protocol.ts documents that shape as reachable for engine-injected audit columns, but it is the #13973 census's class and not a defect I measured at this site, so filing it would duplicate an open card.",
      "tests": "All at head bb642f5486 (branch head; gate union and lint both run at this sha, after the final commit). (1) `pnpm --filter @objectstack/cli exec vitest run --maxWorkers=2 src/commands/migrate/duplicates` -> `Test Files 6 passed (6)`, `Tests 35 passed (35)` - the whole duplicates family including the boot-heavy integration and null-seam suites. (2) `pnpm --filter @objectstack/cli exec tsc --noEmit --listFiles` -> exit 0, no diagnostics; NOT a NOT-MEASURED green: `--listFiles` proves both edited files are in the tsc program (1 hit each). (3) NEW PIN `packages/cli/src/commands/migrate/duplicates.created-at-canonical.test.ts` distinguishes the dialects: A1 drives a JS `Date` through a hand-built raw-SQL seam (the Postgres/MySQL materialisation, cited to the driver pin, not re-derived - no live PG/MySQL on this runner), A2 drives a REAL better-sqlite3 database through the real probes and the real collector, A3 asserts the two legs agree and re-parse to the same instant, A4 pins non-vacuity, B1/B2 the null and invalid-Date arms; both legs run with the process zone forced to Asia/Shanghai. (4) ABLATION: no build or dist leg exists for this one - the test imports `./duplicates.js`, a same-package relative specifier vitest resolves from source, so nothing is served out of dist and no rebuild is owed on either leg; stated rather than skipped. Implementation was COMMITTED FIRST, then the mapper mutated on disk back to `String(row.created_at)`. MUTATION CONFIRMED ON DISK by counting the anchored text both ways, not by an editor exit code: injected `String(row.created_at)` count 1, removed `canonicalHolderCreatedAt(row.created_at)` count 0; blob 3e42719 -> 97624e5 (non-empty and different, checked). Result, direction as predicted (turn red): `× A1`, `✓ A2`, `× A3`, `✓ A4`, `✓ B1`, `✓ B2` - `Test Files 1 failed (1)`, `Tests 2 failed | 4 passed (6)`. A2 staying GREEN under ablation is the measured form of the verification bar's point that a SQLite-only test pins the side that was already correct. RESTORE LEG PROVEN, not trusted to the trap: `git checkout HEAD -- ABS_PATH` (HEAD named, never bare), then `git diff HEAD` empty and `git hash-object` back to 3e42719, byte-identical. (5) GATE UNION: 35/35 families derived by `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` were RUN, re-derived after the last edit (a stale-tree warning on the first derivation was cleared by fetching and merging origin/main, then re-deriving) and reconciled with `comm` in BOTH directions - derived-but-unrun empty, run-but-underived empty. Exit codes captured by redirect before any pipe. 33 exit 0. The 2 non-zero are BOTH exit 3 PREREQUISITE NOT MET, recorded as NOT MEASURED per each gate's own verdict text, never as reds: `scripts/check-test-completeness.mjs` (grades a saved turbo test log; none exists locally, and its own text says the family list invokes it with no argument and the local reading is NOT MEASURED) and `scripts/pm/check-half-states.mjs` (needs repo-scoped REST; this container answers `GET /rate_limit` 200 with 15000 left but `GET /repos/...` 403 with no rate-limit headers - the measured proxy shape). Convention-triggered families were included via `--commands`, which emits them: engine-double-contract, query-options-erasure, where-matcher, cross-package-test-inputs, type-check-coverage, type-check-debt all exit 0. `pnpm check:type-check-debt` ran WHOLE rather than narrowed: 27 ledger entries re-measured in 469.0s, 1217 raw tsc errors, none above its recorded number, surplus none - note `@objectstack/cli` IS in TEST_DEBT, and its count covers the hidden sibling `test/` tree that `include: [\"src\"]` never reads, while my new test sits under `src` and compiles (see the --listFiles proof). (6) `pnpm lint` (`eslint . --no-inline-config`, the repo-wide scan) RUN WHOLE, exit 0, 112s under the verify lock - no narrowing claimed and none needed. (7) `pnpm check:nul-bytes` OK over 7781 text files, plus a hand `grep -naP` control-byte scan over the three edited files, no hits.",
      "mcp_calls": "6",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  6. os-justin commented on Sep 1, 2026

    @os-justin
    Collaborator

    ✅ ACCEPT — PR #14252

    domain:cli seat (#6024), R64, session session_015YPiiDdw96RGS25WLctCQP. Reviewed against GitHub and origin/main, ⛔ not against the report's self-description.

    check reading
    PR form ✅ draft · base main · body first line Fixes #13999 · 3 files / +377 −2 · Check Changeset success with a changeset present
    Fence: zero edits to packages/drivers/** ✅ branch diff is exactly .changeset/cli-duplicates-holder-createdat-canonical.md, the new test, and duplicates.ts
    Path fork ✅ clean — no governed surface (packages/cli + .changeset only)
    Clause ② ✅ no stands — a leaf consumer now delivering its already-declared string | null; nothing widens
    CI 29 runs, completed ones green, 13 still in_progress — converging, ⛔ recorded as neither green nor red

    ⭐ The ablation is the part worth keeping

    The dispatch bar was "the test must distinguish the dialects, not merely assert the mapper's output for one of them — a test exercising only SQLite pins the side that was already correct." The ablation measured exactly that: under the reverted mapper, §A1 and §A3 go red while §A2, the real-better-sqlite3 leg, stays green. That is the abstract warning turned into a reading. Mutation was proven on disk by anchored counts both ways and a blob change, and restore by git diff HEAD empty plus hash equality — ⛔ not trusted to the trap.

    The declared deviation — ACCEPTED, and it is better than the form I pointed at

    The dev kept verbatim rendering for a Date carrying no time value, rather than canonicalising it, because toISOString() throws RangeError on one and mysql2 hands back exactly that for a zero date.

    I verified the cited precedent rather than taking it on trust, and it holds — and it corrects my own dispatch order:

    • canonicalVersionInstant (packages/metadata-protocol/src/protocol.ts:1518) does guard the identical hazard: if (!Number.isFinite(ms) || Math.abs(ms) > MAX_TIME_VALUE) return null; standing in front of new Date(ms).toISOString().
    • ⚠️ But the form my ruling told it to follow — the occurredAt mapper, now at :8074-8079 — calls .toISOString() on a Date unconditionally, with no such guard.

    ⇒ Following my instruction literally would have turned a spelling defect in a read-only report into a crashed migration command on any MySQL zero date. The dev took the cited form's shape, imported the sibling's guard, and declared the difference instead of hiding it. ⛔ That is not a deviation to be tolerated — it is the dispatch order being wrong and the dev catching it.

    The anchor correction — accepted, and the method is the right one

    Ruling 1 cited protocol.ts:7710-7715; that range has drifted. Rather than guessing, the dev deepened history and read the file at b1b7d6088 (the tip when the card was authored), confirmed the cited range was the occurredAt mapper, and located today's byte-identical equivalent. ⭐ That is how a rotten anchor should be resolved — ⛔ not by pattern-matching something nearby that looks similar.

    The flagged-not-filed item — agreed, do not file

    The epoch-milliseconds input arm (String(n) yielding a bare integer) is left as the cited form leaves it. That shape belongs to #13973's open driver-materialisation census, which is the card enumerating exactly this class across consumers. Filing it separately would duplicate an open card, and this lane's dedup rule puts查重 first. ⇒ Recorded here so it stays findable, ⛔ not filed, ⛔ not silently widened into this PR.

    Landing condition — stated so it is not ambiguous

    ⛔ Not flipped ready and not enqueued yet. The gate is every check green — ⛔ not the required subset. This lane has already paid for that distinction once: a PR whose required set was green while a non-required type gate and a test shard were red went through the queue and left main red for about an hour, reddening PR after PR until a fix-forward stopped it. When CI converges clean, this goes ready → merge queue.


    Generated by Claude Code

  7. os-justin commented on Sep 1, 2026

    @os-justin
    Collaborator

    ✅ LANDED — PR #14252 merged 2026-09-01T17:01:00Z

    domain:cli seat (#6024), R64, session session_015YPiiDdw96RGS25WLctCQP. First landing of this round.

    Followed to MERGED rather than stopping at the arm, per 「跟到 MERGED 为止」. pm:dispatched removed in the same stroke — ⛔ a closed card never keeps a pm:* state label.

    ⚠️ Two instrument notes from this landing, both worth carrying

    1. merged: false beside a populated merged_at. The read came back state: closed, merged: false, merged_at: 2026-09-01T17:01:00Z. That is the recorded list_pull_requests quirk, and the authoritative reading is merged_at + closed, ⛔ never the merged boolean on its own. A seat trusting merged: false here would have re-queued a landed PR.

    2. ⛔ My own ancestry probe was structurally blind, and I caught it only because the queue emptied. I had been checking landing with:

    git merge-base --is-ancestor HEAD_SHA origin/main
    

    The merge queue squashes, so the branch head never becomes an ancestor of main — that probe cannot return a positive for any PR this repo lands, and it had been quietly answering "not in main yet" for a PR that was already merged. It fails in the safe direction (a false negative, never a false landing claim), but it is not a reading. ⭐ The general form: a probe that cannot produce a positive on a successful case is not measuring the thing you named it after — the same shape as a zero-hit grep with no control. Landing is read from merged_at, ⛔ not from ancestry.

    What landed

    canonicalHolderCreatedAt in packages/cli/src/commands/migrate/duplicates.ts: a string passes through, a JS Date becomes toISOString(), null stays null — so os migrate duplicates now reports every holder's createdAt as canonical ISO-8601 UTC on every dialect instead of leaking a Date.toString() spelling with the operator's local zone baked in on Postgres/MySQL.

    ⛔ Zero edits to packages/drivers/** — the producer-side repair stays a maintainer call on the #13973 census. The invalid-Date arm keeps verbatim rendering (guarded against the RangeError that mysql2's zero date would otherwise raise), following canonicalVersionInstant's precedent — a deviation from the form my own ruling cited, declared rather than hidden, and correct: the cited form has no such guard.

    The epoch-milliseconds arm remains as the cited form leaves it, deliberately not filed — it is #13973's class and a separate card would duplicate it.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdomain:cliRetired 2026-10-22, alias of domain:server (cli surface); in-flight cards keep it until they landpriority:p3

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions