Skip to content

fix(driver-sql): give the SQLite builtin audit columns the canonical ISO-8601 DEFAULT - #11401

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-11321-sqlite-audit-default-canonical
Aug 23, 2026
Merged

fix(driver-sql): give the SQLite builtin audit columns the canonical ISO-8601 DEFAULT#11401
os-zhuang merged 2 commits into
mainfrom
claude/issue-11321-sqlite-audit-default-canonical

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes#11321

On SQLite the builtin created_at/updated_at audit columns defaulted to an unqualified CURRENT_TIMESTAMP — zone-naive, space-separated, second-precision — while a declared Field.datetime with defaultValue: 'NOW()'in the same table already got the canonical ISO-8601 form. One table, two spellings of one conceptual value.

The SQLite branch of createAuditTimestampColumn now routes through nowColumnDefault('datetime'), the existing single source for "what does NOW() mean in DDL on this dialect", rather than restating the expression — so the two cannot drift apart again. Postgres and MySQL are untouched (knex.fn.now() on Postgres is a real zone-aware TIMESTAMP; MySQL keeps the now(3) precision match from #11224).

The question that gates this card: does schema-drift read column defaults?

Answered no, by reading the code and by measuring on live in-memory SQLite (better-sqlite3) through the real detectManagedDrift entry point, against a table carrying the OLD default — i.e. exactly what every already-deployed SQLite database holds.

introspected created_at.defaultValue = "CURRENT_TIMESTAMP" <- drift CAN see it
when.defaultValue = "strftime('%Y-%m-%dT%H:%M:%fZ', 'now')"
detectManagedDrift(...) = [] <- and reports nothing

The zero is positive-controlled, because a bare empty array is equally satisfied by an inert harness. Same table, same call, after adding an orphan column and a current_user default:

drift kinds = ["unmapped_column:orphan_col", "default_mismatch:owner"]
entries naming created_at / updated_at = []

default_mismatch — the drift kind that does read column defaults — fires in the same call, and still says nothing about the audit columns. Two independent guards explain it: BUILTIN_COLUMNS skips created_at/updated_at in both of diffManagedTable's loops, and the only default_mismatch producer is the #4560 runtime-token check, for which isAppResolvedDefaultToken('NOW()') is pinned false in packages/spec. applyDeclaredColumnDefault's own docblock states the same invariant independently: "there is no general declared-literal-vs-physical comparison".

So no existing SQLite deployment starts reporting drift on its audit columns. Both call sites are CREATE TABLE only — initObjects' alterTable branch adds declared fields and never the audit columns — so existing tables keep their old default, and formatOutput's repairNaiveUtcAuditTimestamp folds already-written naive rows to canonical on read. That is the same disposition nowColumnDefault already documents for declared fields.

A second site, named here because it is not in the card

rebuildSqliteTablePatched — the whole-table rebuild SQLite drift reconciliation uses — re-emitted the audit default itself, as knex.fn.now():

if(c.name==='created_at'||c.name==='updated_at'){col.defaultTo(this.knex.fn.now());}

That method runs only under if (this.isSqlite) in applyMigrationEntries. Fixing only createAuditTimestampColumn would therefore have shipped a fix that silently reverts itself: a canonically-created table would fall back to CURRENT_TIMESTAMP the first time any unrelated drift (a relaxed NOT NULL, an orphaned column) triggered a rebuild. Fixed in the same change, and pinned by §4 — whose reverse-verification leg (below) turns red alone, so it is a real guard and not an untested rider. This extends the card's declared region beyond createAuditTimestampColumn (~12452) and its call sites; it is disjoint from #11324's introspectForeignKeys region and from both pm:on-hold symbol sets.

Reachability — measured, and wider than the card states

The card says the DEFAULT only fires on writes that bypass the driver. Measured, that is not the whole population: the driver's own create() door reaches it on a documented deployment posture.

stampInsertTimestamps gates on tablesWithTimestamps, which only DDL-running paths populate. On skipSchemaSync / OS_SKIP_SCHEMA_SYNC=1, registerObjectMetadata (the DDL-free registration door) deliberately sets updatedAtColumnState='presumed' and never touches that set — so it is empty, the stamp returns early, and the insert falls through to the column DEFAULT. One table, one row per boot posture, before the fix:

boot 1 (initObjects ran) created_at "2026-08-23T14:54:17.791Z" canonical
boot 2 (skipSchemaSync) created_at "2026-08-23 14:54:17" NAIVE
both rows when "…T14:54:17.79…Z" canonical

The declared NOW() sibling is canonical on both postures — because its canonical shape lives in the column DEFAULT rather than in an app-side stamp. That asymmetry is the argument for fixing this in DDL, and §5 pins it closed.

Existing pins on the old DDL

None. A repo-wide sweep for CURRENT_TIMESTAMP found no test asserting the SQLite audit-column DDL. The one dialect-gate assertion (sql-driver-user-datetime-default-format.test.ts:202) covers Postgres/MySQL only. Nothing was rewritten to match new behaviour.

The opposite is true and worth stating: two live-matrix suites already spell the SQLite audit default as strftime('%Y-%m-%dT%H:%M:%fZ','now') in their hand-written-migration fixtures, each with a docblock claiming those columns take "the SAME physical types createAuditTimestampColumn produces"sql-driver-11176-bulk-and-merge-updated-at.test.ts and sql-driver-timestamps-without-ddl.test.ts. That claim held on MySQL and was false on SQLite; the fixtures modelled a more correct table than the driver built. This change makes the driver agree with them.

Tests

packages/drivers/driver-sql/src/sql-driver-11321-sqlite-audit-default-canonical.test.ts — six legs on real better-sqlite3 through the driver's own initObjects. §1 and §2 assert the audit columns agree with the declared NOW() sibling in the same table rather than matching a hard-coded literal, so a future respelling of the canonical expression moves both sides together and still fails if only one moves.

Reverse verification — direction predicted before each leg, then observed

legmutationpredictedobserved
Aremove the SQLite branch from createAuditTimestampColumn§1 §2 §5 red; §4 red for an inverted reason (before becomes CURRENT_TIMESTAMP while the still-fixed rebuild returns canonical, so after === before fails); §3 §6 greenexactly that — 4 failed, 2 passed
Brestore knex.fn.now() in rebuildSqliteTablePatched only§4 red alone; all others greenexactly that — 1 failed, 5 passed

§3 staying green under leg A is the point of that prediction: its subject is drift's indifference to the default, which the fix never changed.

Each leg proved the mutation on disk at the text it meant to change (createAuditTimestampColumn canonical line 1→0, 119 bytes removed for A; rebuild canonical 1→0 and knex.fn.now 0→1 for B), and each restore was proved byte-identical by git hash-object against the HEAD blob, not by an insertion count. Both mutation scripts installed a bash trap on EXIT, INT and TERM whose action was the git checkout HEAD -- restore, so a foreground-cap SIGTERM landing mid-mutation cannot leave the tree mutated. No build/dist step is involved: the suite imports ../src/index.js, a relative path vitest compiles from source, so the mutation is in the code under test with nothing to rebuild.

Gates

Union derived from the actual diff via node scripts/pm/dispatch-gates.mjs with no paths passed, re-derived unchanged at the final commit 1bf96b8a6. All 20 green, each exit status captured before any pipe:

changeset-gate-self-tests 0 · driver-conformance 0 · objectui-changeset 0 · published-files 0 · slot-lookup 0 · test-source-alias 0 · type-source-resolution 0 · adr-0087-registration 0 · changeset-no-major 0 · ci-filter-parity 0 · empty-changeset 0 · plugin-teardown-shape 0 · affected-docs 0 · query-options-erasure 0 · type-check-coverage 0 · type-check-debt 0 · engine-double-contract 0 · cross-package-test-inputs 0 · where-matcher 0 · nul-bytes 0

check:type-check-debt ran against a built workspace closure and printed its own verdict: "--re-measure: OK — 33 ledger entr(ies) re-measured in 278.4s, 1897 raw tsc error(s) total, none above its recorded number."

Package-level, at the same commit: pnpm --filter @objectstack/driver-sql typecheck clean (tsc --noEmit, script name echoed), and the full suite 116 files passed | 7 skipped, 1854 tests passed | 92 skipped — the skips are the live-dialect PG/MySQL cells, which CI runs.

Repo-wide pnpm lint (eslint . --no-inline-config) run in full — exit 0 in 99s. Not a narrowing.

Deliberately not done


Generated by Claude Code

…ISO-8601 DEFAULT
`createAuditTimestampColumn`'s non-MySQL branch was
`table.timestamp(name).defaultTo(this.knex.fn.now())`, which on SQLite compiles
to an unqualified `CURRENT_TIMESTAMP` — a zone-naive, space-separated,
second-precision string. A declared `Field.datetime` with `defaultValue:
'NOW()'` in the SAME table already got the canonical ISO-8601 form from
`nowColumnDefault`, so one table carried two spellings of one conceptual value.
Route the SQLite branch through `nowColumnDefault('datetime')` — the existing
single source for "what does NOW() mean in DDL on this dialect" — rather than
restating the expression, so the two cannot drift apart again.
`rebuildSqliteTablePatched` carried the same `knex.fn.now()` for the audit
columns. That method is SQLite-only, so leaving it would have silently REVERTED
a canonically-created table the moment any unrelated drift triggered a rebuild.
Fixed together; a rebuild must hand back the column `initObjects` would build.
Postgres is untouched (`knex.fn.now()` there is a real zone-aware TIMESTAMP);
MySQL keeps `now(3)` from #11224.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
…drift's indifference to it
Six legs on real better-sqlite3 through the driver's own initObjects:
1. emitted DDL — audit columns and a declared Field.datetime NOW() sibling
carry the SAME default expression (compared to the sibling, not to a
literal, so a future respelling moves both sides together);
2. a DEFAULT-firing insert stores a canonical instant in every column;
3. the blast-radius question — a table still carrying the OLD
CURRENT_TIMESTAMP default reports NO drift, WITH a positive control
proving the default-reading dimension is live in the same call;
4. a drift-triggered SQLite rebuild hands back the canonical default;
5. on a skipSchemaSync boot the driver's own create() door — which reaches
the column DEFAULT because tablesWithTimestamps is empty — now stores
canonical;
6. dialect gate: Postgres keeps CURRENT_TIMESTAMP, MySQL keeps
CURRENT_TIMESTAMP(3), neither leaks strftime. Compiled offline.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-sql, touching 3 documentable anchor(s).

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

  • content/docs/data-modeling/drivers.mdx(via SqlDriver (symbol))
  • content/docs/data-modeling/index.mdx(via SqlDriver (symbol))
  • content/docs/plugins/packages.mdx(via SqlDriver (symbol))
  • content/docs/protocol/kernel/index.mdx(via SqlDriver (symbol))
  • content/docs/protocol/kernel/lifecycle.mdx(via SqlDriver (symbol))
  • content/docs/protocol/objectql/query-syntax.mdx(via SqlDriver (symbol))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx(via SqlDriver (symbol))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 9 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 365e3340f5c7807eb0f59af115f93476e508a094packageMentionDocs.

Which tree this was computed on

This run read content/docs from 2f80ff3bdb0ca7aeacb0ecdad70a53ecd93c04aa — the merge of head 1bf96b8a699d3ab1a281ccf6898b5e7ef0fdfabc into base 365e3340f5c7807eb0f59af115f93476e508a094, 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 2f80ff3bdb0ca7aeacb0ecdad70a53ecd93c04aa && git checkout 2f80ff3bdb0ca7aeacb0ecdad70a53ecd93c04aa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 365e3340f5c7807eb0f59af115f93476e508a094 1bf96b8a699d3ab1a281ccf6898b5e7ef0fdfabc && git checkout -B drift-repro 365e3340f5c7807eb0f59af115f93476e508a094 && git merge --no-ff 1bf96b8a699d3ab1a281ccf6898b5e7ef0fdfabc
node scripts/docs-audit/affected-docs.mjs --json 365e3340f5c7807eb0f59af115f93476e508a094

⚠️ 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 365e3340f5c7807eb0f59af115f93476e508a094 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 23, 2026
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Docs Drift Check — chased, no edit owed. Recording the disposition so a reviewer doesn't re-derive it. This bot has produced real findings for this seat (#11205 invalidated four shipped claims), so "advisory" is not grounds to skip it — and unlike the last two driver-sql PRs, this one changes DDL, so a doc stating what the audit columns default to would genuinely be falsified.

One page does discuss SQLite datetime defaults and explicitly contrasts them with a bare CURRENT_TIMESTAMPprotocol/objectql/types.mdx:425–453. Read in full, it is scoped to defaultValue: 'NOW()' on declared temporal fields, not to the builtin audit columns:

On SQLite all three types use strftime(…, 'now') expressions that emit the canonical form directly.

That sentence was true before this PR and stays true after — it is about declared fields, which already had the canonical form. So the row is a coarse SqlDriver symbol hit, not a falsified claim. The other five hand-written rows carry no default/DDL claim at all, and the read-only releases/v17.mdx names no audit-column default (CURRENT_TIMESTAMP|strftime|created_at.*default over that file → nothing), so there is nothing to file there either.

Two things worth stating rather than filing.

First, that page's own Callout already documents the exact disposition this PR relies on for existing tables:

A DDL default only governs newly created columns. A column created before this convention keeps its legacy default and can still emit a zone-naive value on a defaulted insert; the read path repairs those to canonical form, so find() stays uniform without a data migration.

The PR reached that same posture independently via repairNaiveUtcAuditTimestamp. Documented policy and implemented behaviour agree, which is the argument for not inventing a migration here — and it is a second, independent source for the "no migration owed" conclusion beyond the drift measurement.

Second, and the reason this is not purely a miss: the section opens by saying the driver resolves NOW() against the UTC clock on every dialect. A reader could reasonably carry that inference to the audit columns — and on SQLite that inference was wrong until this PR. Nothing in the prose asserted it, so no sentence is being corrected; but this change removes a latent misreading rather than creating one. Same direction as the test fixtures noted in the PR body, which already spelled the audit default canonically and were describing a more correct table than the driver built.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 23, 2026 15:59
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

ACCEPT — engine seat. Marked ready for review, then enqueued (that order deliberately: ready_for_review discards auto-merge and any queue slot).

Green read by job name

34 check runs, 0 still running, none non-green; Build Docs and Console Pin Gate skipped for cause. ⚠️ Skipped is not red — worth restating on this file, since a red Console Pin Gate has been a real signal since #11146. Temporal Conformance (live PG + MySQL), Build Core, Lint & Repo Gates, Check Changeset and all four Type Check jobs read completed: success individually.

The gating question was answered the right way

I made one question blocking before any edit: does schema-drift read column defaults? — because if it did, this narrow correctness fix would make every already-deployed SQLite database start reporting drift on created_at/updated_at, turning a two-line change into a fleet-wide false alarm.

Answered no, and answered as a measurement rather than a code reading alone. detectManagedDrift returns [] against a table carrying the OLD default CURRENT_TIMESTAMP — but a bare empty array is equally satisfied by an inert harness, so the zero was positive-controlled in the same call: adding an orphan column and a current_user default produced ["unmapped_column:orphan_col", "default_mismatch:owner"]. The drift kind that does read defaults demonstrably fires, and still says nothing about the audit columns. Two independent code guards explain it (BUILTIN_COLUMNS skipping them in both of diffManagedTable's loops at schema-drift.ts:402/:528, and isAppResolvedDefaultToken('NOW()') pinned false in packages/spec), and applyDeclaredColumnDefault's docblock states the same invariant independently.

That is the difference between "I looked and saw nothing" and "I proved the instrument was live and it saw nothing."

The second site — accepted region extension, and the PR would have been worthless without it

rebuildSqliteTablePatched re-emitted the audit default itself as knex.fn.now(). Fixing only createAuditTimestampColumn would have shipped a fix that silently reverts itself: a canonically-created table falls back to CURRENT_TIMESTAMP the first time any unrelated drift — a relaxed NOT NULL, an orphaned column — triggers a rebuild.

⚠️ This extends beyond the region my dispatch order authorised (createAuditTimestampColumn ~12452 and its call sites). Accepting it deliberately, for three reasons: it was named prominently in the PR body rather than folded in quietly; it is measured disjoint from #11324's introspectForeignKeys region and from both pm:on-hold symbol sets; and reverse-verification leg B turns §4 red alone, which proves it is a real guard rather than an untested rider riding along. This is the protocol's declared-addition path working as intended — the rule exists to stop silent scope growth, not to force a fix to ship broken.

Reachability was measured, not inherited — and the card was understated

I asked for this explicitly because this seat has twice today published an inherited scope claim dressed as a measurement. The card said the DEFAULT only fires on writes that bypass the driver. Measured, the driver's own create() door reaches it under skipSchemaSync / OS_SKIP_SCHEMA_SYNC=1: stampInsertTimestamps gates on tablesWithTimestamps, which only DDL-running paths populate, and registerObjectMetadata — the DDL-free registration door that exists for exactly that posture — sets updatedAtColumnState='presumed' and never touches that set. Two boot postures against one file-backed database:

boot 1 (initObjects ran) created_at "2026-08-23T14:54:17.791Z" canonical
boot 2 (skipSchemaSync) created_at "2026-08-23 14:54:17" NAIVE

The declared NOW() sibling is canonical on both, because its canonical shape lives in the column DEFAULT rather than an app-side stamp. That asymmetry is the argument for fixing this in DDL.

Smaller things that were right

  • Tests assert agreement with the sibling, not a hard-coded literal — so a future respelling of the canonical expression moves both sides together, and the pin still fails if only one moves.
  • No existing pin was quietly rewritten. A repo-wide sweep found nothing asserting the old SQLite audit DDL. The reverse was reported as a signal: two live-matrix suites already spell that default canonically in fixtures whose docblocks claim those columns take "the SAME physical types createAuditTimestampColumn produces" — false on SQLite until now. The fixtures modelled a more correct table than the driver built; this makes the driver agree with them.
  • The card's own naming error was reported rather than worked around — it calls the helper defaultDatetimeSql; the real name on main is nowColumnDefault.
  • A third control beyond the two I required: the symbols the diff does touch, through the same filter (nowColumnDefault 5, createAuditTimestampColumn 1). That is the control proving the filter can see this diff — and it is what made the BRE false-zero unmistakable when Control B read 0/0/0 instead of 1/1/1.
  • No migration and no drift exemption invented. Existing tables keep the old default; nothing reports it, and repairNaiveUtcAuditTimestamp folds already-written naive rows to canonical on read — the same disposition types.mdx's own Callout documents for declared fields.

needs:contract-review does not apply: packages/spec is not in this diff and no interface shape changed.


Generated by Claude Code

@os-zhuang
os-zhuang added this pull request to the merge queueAug 23, 2026
Merged via the queue into main with commit 5dd3bc9Aug 23, 2026
35 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-11321-sqlite-audit-default-canonical branch August 23, 2026 16:17
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-zhuang@claude