Skip to content

Serve canonical ISO-8601 for the import-job DTO's four timestamps on Postgres/MySQL - #14076

Merged
zhuangjianguo merged 4 commits into
mainfrom
claude/issue-13994-import-job-dto-timestamps
Sep 1, 2026
Merged

Serve canonical ISO-8601 for the import-job DTO's four timestamps on Postgres/MySQL#14076
zhuangjianguo merged 4 commits into
mainfrom
claude/issue-13994-import-job-dto-timestamps

Conversation

@zhuangjianguo

@zhuangjianguo zhuangjianguo commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes #13994

Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L (durable attribution copy — a body edit rewrites the footer below)

importJobToProgress — the mapper behind GET /data/import/jobs/:jobId, /results and the history list — rendered its four timestamp fields through String(value). On Postgres and MySQL those columns arrive as JS Dates, so String ran Date.prototype.toString and the REST contract served

Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard Time)   <- what the API served
2026-08-30T10:19:25.947Z                                  <- what it promises

Milliseconds dropped, the server's timezone baked into the value, no Z, and not Date.parse-safe for a client doing strict ISO parsing.

Route A as ruled: the repair is spelled at the mapper. No driver read-door change, no tolerant ?? fallback, and the ? presence-guards are untouched.

What was measured before choosing (every line number re-derived)

The four sites are at packages/rest/src/rest-server.ts:524, 538, 539, 540 — the card cited :520/:533/:534/:536. The model form is at packages/metadata-protocol/src/protocol.ts:7746-7750; the card said :7710-7715 and triage measured :7623-7627, so both had rotted again.

Neither timestamp repair runs on Postgres/MySQL — brace-depth evidence. In SqlDriver#formatOutput (packages/drivers/driver-sql/src/sql-driver.ts:15861), if (this.isSqlite) { opens at :15879 (depth 1 → 2) and closes at :15968 (depth 2 → 1). Inside it: the AUDIT_TIMESTAMP_COLUMNS loop at :15946 (depth_before=2) and the datetimeFields lookup at :15960 (depth_before=2) with its normalizeSqliteDatetimeOutput loop at :15962-15964. So a declared Field.datetime is not protected on Postgres/MySQL — re-verified on the current tree, as the lane's earlier finding said.

A refinement to the card's classification. The card says created_at is "a builtin audit column — not in datetimeFields". It is in fact both: it is in AUDIT_TIMESTAMP_COLUMNS (sql-driver.ts:269) and declared Field.datetime on the object (packages/platform-objects/src/audit/sys-import-job.object.ts:124). The conclusion is unchanged and slightly strengthened — it is doubly unrepaired on PG/MySQL, since both passes sit in the same isSqlite arm.

This read path does reach formatOutput — it is not one of the bypass doors. loadImportJob (rest-server.ts:7923) calls p.findData, which lands on SqlDriver#findfindRows, and find runs this.formatOutput(object, row) over every row for every dialect (sql-driver.ts:5665-5670), unlike execute / aggregate / distinct. So the card's mechanism is right for the right reason: formatOutput runs, and the two repairs inside it are what the dialect gate withholds.

The site is the only one. The card's re-run expression and a widened sweep for any String(...) over a date-ish column across packages/rest/src/ both return exactly these four lines.

Why the String() was deleted-shaped but not deleted

Triage's sharpest observation is recorded and correct: JSON.stringify would already have serialised a bare Date correctly through toJSON(), so the explicit String() is one layer too many, not one too few. Deleting it is nevertheless the wrong repair, because it changes the emitted value's static type from string to string | Date — and that widens a declared contract which three independent declarations spell as string:

declaration spelling
packages/spec/src/api/export.zod.ts:466-486ImportJobProgressSchema (the output of ImportJobApiContracts.getImportJobProgress) createdAt: z.string(), the other three z.string().optional(), each documented "(ISO 8601)"
packages/client/src/index.ts:5485, 6236 — the SDK's getImportJobProgress returns ImportJobProgress, a z.input of the above
objectui packages/types/src/data.ts:1022-1050ImportJobProgressInfo all four string, each doc-commented "ISO-8601"

The declaration was right; the emitted value was wrong. So the declaration does not move, and no consumer is touched.

No consumer is currently tolerating the broken spelling. Sweeping objectui and the SDK for a lenient parse of these four fields found none — the Date.parse / new Date(...) sites there belong to unrelated createdAts (comments, notifications, conversations). On Postgres/MySQL a client now receives the same instant spelled correctly, with the milliseconds it previously lost; on SQLite nothing changes at all.

The spelling

The four sites go through one module-private helper carrying the same three branches as the two landed normalisers — auditMetaItem's occurredAt in protocol.ts and canonicalIsoInstant in sys-metadata-repository.ts (#13997, which landed into this branch mid-flight). One spelling repo-wide, not a new variant. The only difference from canonicalIsoInstant is its nullish arm, and that difference is forced by the call sites: it returns undefined so each caller's own ?? DEFAULT chain keeps its meaning, whereas these four sites are the DTO's last step and the required createdAt field's absent-value spelling ('') is folded in, exactly as the String(row?.created_at ?? '') it replaces produced.

Verification

A pin on ISO-text fixtures would prove nothing — that identity is exactly what kept this green while the defect was live, including in this package's own real-engine SQLite integration test. packages/rest/src/import-job-dto-timestamp-canonical.test.ts therefore drives real Dates through the real route handlers under a forced non-UTC process zone, and asserts:

  • canonical ISO-Z on all four fields, with the four stamps distinct from each other (and each carrying a distinct non-zero millisecond component), so no field can pass by echoing another and a millisecond-dropping spelling cannot pass by accident;
  • a non-vacuity controlString(new Date(...)) really is non-canonical under the forced zone, and the three swept zones really do produce three different broken spellings;
  • timezone independence — one answer across Asia/Shanghai, America/New_York and UTC;
  • an idempotence control — the already-canonical SQLite shape comes back byte-identical, which is what shows the pin discriminates rather than being globally sensitive;
  • the presence semantics are unchanged — a job that has not started still omits startedAt / completedAt / revertedAt entirely, and an absent created_at is still '';
  • a full safeParse of each response against the spec's own ImportJobProgressSchema / ImportJobSummarySchema. Because the judgement here is about a value, a green parse is the assertion (not merely the absence of unknown keys) — and this limb is what refuses the String()-deletion route.

Both mappers are covered: importJobToSummary re-reads importJobToProgress's output, and the list route is pinned too.

Ablation

Predicted direction: red on the Date cases, green on the controls. No rebuild leg applies — the pin imports ./rest-server relatively, within the same package, so the subject does not cross a package wall through dist.

The mutant is the pre-fix file taken byte-exact from origin/main (git checkout BASE -- ABSOLUTE_PATH), rather than hand-written mutant text. That choice was forced by a first attempt whose shell quoting silently ate a '' and produced String(row?.created_at ?? ): the run went red on a PARSE_ERROR having collected zero tests, which would have read as a behavioural failure. That first reading is void and is not reported as evidence; the rerun carries an explicit guard that refuses any leg failing to collect.

leg blob on-disk proof result
mutated 902319cd (= origin/main, ≠ HEAD) 4 × String(row.*_at) present, 0 × canonicalIsoStamp 4 failed | 2 passed
restored aab40fd3 (= HEAD blob) git diff HEAD empty, git status clean 6 passed

The mutated leg failing 4 of 6 while the two control cases stay green is the discrimination claim, measured rather than asserted. Restore was pinned to HEAD with absolute paths under trap … EXIT INT TERM, and proved by blob equality plus an empty git diff HEAD — not by an exit code.

Gates

Change set re-derived after the final commit (node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, 4 paths, no stale-tree warning); the derived family grew from 39 to 54 when the census repair added a content/docs/** path, and the 15 added docs gates were run too. All exit codes captured before any pipe. Union re-run on final head c680e23b22:

  • pnpm --filter @objectstack/rest exec vitest run165 files / 2772 tests passed
  • pnpm --filter @objectstack/rest typecheck — clean; its own verdict line: check:test-typecheck: OK — @objectstack/rest's test layer compiles under packages/rest/tsconfig.test.json. The new pin is in that program (confirmed by tsc -p tsconfig.test.json --listFiles, 1 hit), so this is a real reading over it and not a green over source nothing read.
  • pnpm lint (eslint . --no-inline-config, whole repo) — exit 0
  • pnpm check:type-check-coverage — OK, counts unchanged (11 DEBT / 309 frozen)
  • 51 further derived gates green.
  • NOT MEASURED (three gates, each self-declaring PREREQUISITE NOT MET, exit 3 — recorded as neither pass nor red): check:dual-build-cjs-loads and check:type-check-debt both need a full built closure, and check-test-completeness needs a saved turbo run test log its local invocation is not given. CI owns all three.

One gate genuinely went red, and it was mine

check-system-context-census reported 16 problems. Measured as caused here rather than inherited: with rest-server.ts reverted to origin/main the census is exit 0, and with this change exit 1. The cause is pure line rot — the helper adds a net +56 lines near the top of the file (60 insertions − 4 deletions), and every isSystem anchor the census page cites in that file moved by exactly that offset. Repaired with the gate's own documented remedy for line rot (--fix): 10 anchors re-pointed, uniformly +56, no ledger row added or deleted and no prose changed. The census is exit 0 again.

Recorded finding, not folded in

The shared canonical spelling throws on an Invalid Date. All three copies (auditMetaItem, canonicalIsoInstant, and this one) call .toISOString() on any Date, and that raises RangeError: Invalid time value for an invalid one — where the previous String(v) served "Invalid Date". So for that one input shape the change trades a wrong-looking string for a 500. No driver in the measured input domain produces it (the enumeration in canonicalVersionInstant's docblock: Date, canonical ISO string, epoch-ms number, nullish), and deviating unilaterally would have been the fourth variant the ruling forbids, so the spelling is unchanged here. Raised for the maintainer as a property of the shared spelling rather than of this site.

A correction to an earlier revision of this body

An earlier revision claimed the pnpm --filter direction memo in the dispatch instructions was backwards. That claim was wrong and is withdrawn. Re-measured precisely in this worktree (pnpm 10.31.0): '@objectstack/rest^...' selects 35 packages including @objectstack/driver-sql (a dependency of rest) and excluding @objectstack/client (a dependent) ⇒ dependencies; '...^@objectstack/rest' selects 18 including client and excluding driver-sqldependents. That confirms the memo rather than contradicting it. The one genuine wrinkle worth passing on: under this pnpm version both forms still include the named package itself, so ^ does not exclude it here — which is why building 'PKG^...' also rebuilds PKG.

Generated by Claude Code

…estamps

`importJobToProgress` rendered `created_at`, `started_at`, `completed_at` and
`reverted_at` through `String(value)`. On Postgres and MySQL those columns are
materialised as JS `Date`s, so `String` ran `Date.prototype.toString`: the
REST contract served `"Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard
Time)"` where it promises `"2026-08-30T10:19:25.947Z"` — milliseconds dropped,
the server's timezone baked in, no `Z`, not `Date.parse`-safe.

Nothing upstream repaired it: `formatOutput`'s `AUDIT_TIMESTAMP_COLUMNS` pass
and its `normalizeSqliteDatetimeOutput` pass over `datetimeFields` both sit
inside the `if (this.isSqlite)` arm, so a declared `Field.datetime` is not
protected on Postgres/MySQL. SQLite returns canonical ISO text, where
`String()` was an identity — which is what kept every test green.

The four sites now share the three-branch normaliser already landed in
`@objectstack/metadata-protocol` (string passthrough -> `instanceof Date` ->
`toISOString()` -> last-resort `String(v ?? '')`). Presence semantics are
untouched. The new pin drives real `Date`s through the real routes under a
forced non-UTC process zone, with a non-vacuity control and an idempotence
control for the already-canonical shape.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
… repair

#13997's `canonicalIsoInstant` landed in `@objectstack/metadata-protocol`
while this branch was open. Cross-reference it beside `auditMetaItem`'s
`occurredAt` form and record why the nullish arm differs here: these four
sites are the DTO's last step, and the required `createdAt` field's
absent-value spelling (`''`) is folded in rather than left to a caller's
`?? <default>` chain.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
…r insertion

`check:check-system-context-census` went red with 16 problems: the
`canonicalIsoStamp` helper adds a net +56 lines near the top of
`rest-server.ts`, so every `isSystem` anchor the census page cites in that
file rotted by exactly that offset.

Repaired with the gate's own documented remedy for pure line rot
(`check-system-context-census.mjs --fix`): 10 anchors re-pointed, uniformly
+56, no ledger row added or deleted and no prose touched. Measured as caused
here rather than inherited — the census is exit 0 against the pre-fix
`rest-server.ts` and exit 1 with it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/rest, touching 8 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/protocol/objectql/state-machine.mdx (via /import/jobs/:jobId (route, bridged from symbol importJobToProgress — its registrar handler names it))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json a7002ce5ac9acf6a4d556713c587a770f1848c26packageMentionDocs.

Which tree this was computed on

This run read content/docs from 304b9f6d47ae6e1b8731fdd21f95f69e2b2c350c — the merge of head c680e23b2299bb141e2639deb061008f13e1decc into base a7002ce5ac9acf6a4d556713c587a770f1848c26, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 304b9f6d47ae6e1b8731fdd21f95f69e2b2c350c && git checkout 304b9f6d47ae6e1b8731fdd21f95f69e2b2c350c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin a7002ce5ac9acf6a4d556713c587a770f1848c26 c680e23b2299bb141e2639deb061008f13e1decc && git checkout -B drift-repro a7002ce5ac9acf6a4d556713c587a770f1848c26 && git merge --no-ff c680e23b2299bb141e2639deb061008f13e1decc

node scripts/docs-audit/affected-docs.mjs --json a7002ce5ac9acf6a4d556713c587a770f1848c26

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs a7002ce5ac9acf6a4d556713c587a770f1848c26 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 1, 2026 04:38
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Sep 1, 2026
Merged via the queue into main with commit ddea371 Sep 1, 2026
38 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-13994-import-job-dto-timestamps branch September 1, 2026 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants