Skip to content

fix(cli): generated migrations match driver-sql on audit-stamp nullability and default text - #16278

Merged
os-litant merged 3 commits into
mainfrom
claude/issue-15521-audit-stamp-nullability-and-default
Sep 6, 2026
Merged

fix(cli): generated migrations match driver-sql on audit-stamp nullability and default text#16278
os-litant merged 3 commits into
mainfrom
claude/issue-15521-audit-stamp-nullability-and-default

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes #15521

Ruled on the card 2026-09-06: option B, both remaining rows in one PR, because they are one question about the same two columns. Both migration generators now follow driver-sql on the builtin created_at / updated_at columns — the same rule #15040 applied to the id column.

packages/drivers/** is untouched. It is the authority on this card, never the subject; option C was refused in the ruling.

What changed

row before after
nullability, --format sql "created_at" TIMESTAMPTZ NOT NULL DEFAULT now() "created_at" TIMESTAMPTZ DEFAULT CURRENT_TIMESTAMP
nullability, typescript (the default format) table.timestamps(true, true) table.timestamp('created_at').defaultTo(db.fn.now())
default text, --format sql now() CURRENT_TIMESTAMP
dialect scope unstated the --format help text and generateMigrationSql's docblock both state PostgreSQL-only, and disclaim MySQL and SQLite

table.timestamps(true, true) cannot express the ruled shape: knex 3.3.0 compiles its second argument to .notNullable().defaultTo(...) on both columns, with no way to ask that helper for the DEFAULT without the NOT NULL. So dropping NOT NULL to match the driver means spelling the two columns out as the driver's own line, with this.knex re-receivered to the generated migration's db.

NOT NULL was never load-bearing: stampInsertTimestamps fills both columns on every platform write, and where it does not (the documented skipSchemaSync posture) the column DEFAULT fires. What it bought was a permanent schema diff between a generated table and a platform-created one.

Evidence — live PostgreSQL 16.13

All three producers driven into one live cluster, each into its own schema so the emitted text runs unmodified, then read back from information_schema.columns:

p_driver  created_at  timestamp with time zone   null=YES  default=CURRENT_TIMESTAMP
p_sqlgen  created_at  timestamp with time zone   null=YES  default=CURRENT_TIMESTAMP
p_tsgen   created_at  timestamp with time zone   null=YES  default=CURRENT_TIMESTAMP
p_driver  updated_at  timestamp with time zone   null=YES  default=CURRENT_TIMESTAMP
p_sqlgen  updated_at  timestamp with time zone   null=YES  default=CURRENT_TIMESTAMP
p_tsgen   updated_at  timestamp with time zone   null=YES  default=CURRENT_TIMESTAMP

distinct (type|nullable|default) tuples across all three producers: 1
VERDICT: ALL THREE PRODUCERS AGREE

Ablation — main's generate.ts restored over the fix

Falsification conditions were named in writing before the run, one per ruled row. The mutation was proven on disk by blob hash (195280715f4 fixed, replaced by 5c1d13b24d7, which is byte-identical to origin/main and to the pre-fix blob), and the restore proven by observed state — git diff HEAD empty plus blob equality — never by an exit code.

Ablated, the same six test files:

 Test Files  1 failed | 5 passed (6)
      Tests  2 failed | 118 passed (120)

 x #15521 - the audit columns take the driver's zone-AWARE type in both generators
 x #15521 - the audit columns match the driver on nullability and default text

Exactly the two predicted cases, and no others. Ablated, the same live cluster:

p_sqlgen  created_at  ...  null=NO   default=now()             (rows 1 and 2 falsified)
p_tsgen   created_at  ...  null=NO   default=CURRENT_TIMESTAMP (row 3 falsified)
p_driver  created_at  ...  null=YES  default=CURRENT_TIMESTAMP (the authority, unmoved)

distinct tuples: 3
VERDICT: PRODUCERS DISAGREE

An admitted coverage gap, stated rather than papered over: the dialect-scope row (the help-text and docblock sentences) has no executable detector. It stayed green under ablation because nothing asserts it. It is verified by reading only.

The pin test

generate-builtin-id-column.pin.test.ts's recorded-divergence case was edited in place into an agreement pin, as the ruling directed — no second case added, none deleted (case count 8 before, 8 after). Its assertions are derived from the driver's own builder line rather than transcribed, and the driver source is required to still contain that line, so a driver that moves fails there rather than leaving the generators quietly wrong.

Verification

Every measurement below was taken at this PR's head, 2e586a127b6. The predecessor run that wrote this code was killed by a container restart before it reported, so none of its measurements survived to be audited and none were inherited.

  • Dependency closure built first: pnpm --filter '@objectstack/cli^...' build, VERDICT command-exit 0.
  • pnpm --filter @objectstack/cli exec vitest run over the six test files that import generate.js: Test Files 6 passed (6), Tests 120 passed (120).
  • Gate union derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, run twice. The plain form's Reconciliation line names 57 families; the --commands form harvested exactly 57 invocations. Harvest asserted against the Reconciliation number, not against the matched-block count.
  • All 57 were run locally at this head: 54 exited 0, zero red, and 3 did not measure (named below). Full per-command tally captured with redirect-then-capture.

Three of the 57 are NOT MEASURED rather than green, and their own verdict text says so:

  • check:dual-build-cjs-loads"PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/. ... This is NOT a pass: nothing was measured."
  • check:i18n-coverage"Nothing was compared: 12 config(s) did lint, but a partial round cannot judge the ratchet ... this result says NOTHING about whether any declared label went untranslated."
  • check:type-check-debt — its --self-test passed (55 semantic, 97 observation, 45 re-measure, 28 built-closure, 19 auto-lowering and 18 exit-code cases hold) and its plain sibling check:type-check-coverage passed at gate 53, but its --re-measure arm is a whole-tree tsc sweep across 79 packages. This seat stopped that sweep deliberately (SIGTERM by explicit PID, hence exit=143) after it had held the shared verify lock for roughly nineteen minutes with two other agents queued behind it. This is a declared narrowing, not a red and not a pass: a repo-wide ratchet recount is a run CI owns, and CI performs it on this PR.

The first two need a whole-tree build this worktree does not have. None of the three is a finding against this diff, and none is reported here as a pass.

Exit codes throughout were captured by redirect-then-capture, never after a pipe.

Coverage this PR does not have

  • The dialect-scope sentences (the --format help text and generateMigrationSql's docblock) have no executable detector, confirmed by their staying green under the ablation. They are verified by reading only.
  • The emitted migration TypeScript is not compile-checked anywhere: scaffold-emission-typechecks.test.ts covers GENERATOR_SCAFFOLD_TARGETS, not migration output. In place of that, this seat imported and executed the actual generated migration against live PostgreSQL, which is what the catalog rows above are read from.

Generated by Claude Code

…ility and default text

`os generate migration` emitted `NOT NULL` on the builtin `created_at` /
`updated_at` columns in both formats while `driver-sql`'s
`createAuditTimestampColumn` creates them nullable, and the SQL format spelled
their default `now()` while both knex producers emit `CURRENT_TIMESTAMP`.
Nothing failed either way, but `information_schema.column_default` keeps the
two spellings textually apart, so a schema diff between a generated table and a
platform-created one reported the pair forever.

Both generators now follow the driver, the same rule the `id` column already
follows. `table.timestamps(true, true)` cannot express that shape — knex
compiles its second argument to `.notNullable().defaultTo(...)` with no way to
take the default alone — so the TypeScript format spells the two columns out as
the driver's own line. `--format sql` now states in its help text and its
docblock that it targets PostgreSQL only and makes no MySQL or SQLite claim.

The recorded-divergence case in `generate-builtin-id-column.pin.test.ts` becomes
an agreement pin, derived from the driver's builder rather than transcribing the
expected strings.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
@github-actions github-actions Bot added the size/m label Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/api/data-flow.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/api/wire-format.mdx (via updated_at (literal, a string literal in generateMigrationSql; a string literal in generateMigrationTs))
  • content/docs/automation/webhooks.mdx (via updated_at (literal, a string literal in generateMigrationSql; a string literal in generateMigrationTs))
  • content/docs/deployment/cli.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/deployment/seed-tenancy-repair.mdx (via updated_at (literal, a string literal in generateMigrationSql; a string literal in generateMigrationTs))
  • content/docs/permissions/system-context.mdx (via updated_at (literal, a string literal in generateMigrationSql; a string literal in generateMigrationTs))
  • content/docs/protocol/kernel/http-protocol.mdx (via updated_at (literal, a string literal in generateMigrationSql; a string literal in generateMigrationTs))
  • content/docs/protocol/kernel/lifecycle.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/protocol/kernel/realtime-protocol.mdx (via updated_at (literal, a string literal in generateMigrationSql; a string literal in generateMigrationTs))
  • content/docs/protocol/objectql/schema.mdx (via updated_at (literal, a string literal in generateMigrationSql; a string literal in generateMigrationTs))
  • content/docs/protocol/objectql/security.mdx (via updated_at (literal, a string literal in generateMigrationSql; a string literal in generateMigrationTs))
  • content/docs/protocol/objectql/state-machine.mdx (via updated_at (literal, a string literal in generateMigrationSql; a string literal in generateMigrationTs))
  • content/docs/ui/views.mdx (via updated_at (literal, a string literal in generateMigrationSql; a string literal in generateMigrationTs))

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

  • content/docs/releases/v15.mdx (via updated_at (literal, a string literal in generateMigrationSql; a string literal in generateMigrationTs))
  • content/docs/releases/v16.mdx (via updated_at (literal, a string literal in generateMigrationSql; a string literal in generateMigrationTs))
  • content/docs/releases/v17.mdx (via updated_at (literal, a string literal in generateMigrationSql; a string literal in generateMigrationTs))

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
  • 1 anchor(s) matched too much of the corpus to be a work list: created_at (literal, 33 pages)
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 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 — 22 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 331d3dc998b0c47f9b90470e5306c5d1fc5d46ffpackageMentionDocs.

Which tree this was computed on

This run read content/docs from ce9b328bc6cb4162fbdcd8385046a68f92128c0b — the merge of head 2e586a127b609dfd75631ee4ba2d924d29994735 into base 331d3dc998b0c47f9b90470e5306c5d1fc5d46ff, 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 ce9b328bc6cb4162fbdcd8385046a68f92128c0b && git checkout ce9b328bc6cb4162fbdcd8385046a68f92128c0b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 331d3dc998b0c47f9b90470e5306c5d1fc5d46ff 2e586a127b609dfd75631ee4ba2d924d29994735 && git checkout -B drift-repro 331d3dc998b0c47f9b90470e5306c5d1fc5d46ff && git merge --no-ff 2e586a127b609dfd75631ee4ba2d924d29994735

node scripts/docs-audit/affected-docs.mjs --json 331d3dc998b0c47f9b90470e5306c5d1fc5d46ff

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

Copy link
Copy Markdown
Collaborator Author

Landing provenance — domain:cli execution PM seat (#6024)

Compliance with the ruling, item by item

Maintainer ruling of 2026-09-06T05:04:38Z on #15521 (comment 5557086667), option B, both rows in one PR:

ruled item delivered how it is evidenced
1 · both generators stop emitting NOT NULL live catalog read + ablation
2 · SQL format's DEFAULT now()CURRENT_TIMESTAMP live catalog read + ablation
3 · --format sql declares PostgreSQL-only (help text + docblock) ✅ in code ⚠️ read only — no executable detector, see below
4 · the recorded-divergence it edited, not added to or deleted case count 8 before, 8 after
5 · option C refused — the driver does not move packages/drivers/** absent from the diff

generateMigrationTs could not express the ruled shape through table.timestamps(true, true) — knex compiles that helper to .notNullable().defaultTo(CURRENT_TIMESTAMP) unconditionally — so it now emits the driver's own line per column. That is a mechanical consequence of the ruling, not a widening of it.

The evidence standard was recovered, not downgraded

The live PostgreSQL 16.13 cluster from the earlier round was found still running, and all three producers were driven into it in separate schemas so each emitted text ran unmodified: the driver's Postgres arm through knex, the TypeScript format by importing the generated migration and invoking its own up(), the SQL format by executing its DDL verbatim. Read back from information_schema.columns:

all six rows   timestamp with time zone / null=YES / default=CURRENT_TIMESTAMP
distinct tuples: 1        VERDICT: ALL THREE PRODUCERS AGREE

Under ablation the same cluster reports distinct tuples: 3 — PRODUCERS DISAGREE, with p_sqlgen null=NO default=now() (falsifying rows 1 and 2), p_tsgen null=NO (row 3), and p_driver unmoved. ⇒ the agreement is a measurement of the catalog, not of the emitted string.

Ablation discipline: falsification conditions written down before the run, one per ruled row, directions predicted. Mutation proven on disk by blob hash — 195280715f4 replaced by 5c1d13b24d7, asserted equal to origin/main's blob and unequal to the fixed blob, with anchor-text counts alongside. Result exactly as predicted: Tests 2 failed | 118 passed (120), precisely the two named cases. Restore proven by observed state — blob back to 195280715f4 == HEAD blob, git diff HEAD empty, git status --porcelain empty — under a trap on EXIT INT TERM using absolute paths from git rev-parse --show-toplevel, ⛔ never by an exit code.

No rebuild leg, and that is a property of the resolution rather than an omission: the pin imports ./generate.js, a relative same-package specifier vitest resolves straight to src/generate.ts with no exports/dist hop, so no dist can go stale between mutation and measurement.

⚠️ One ruled row shipped with NO executable detector

Quoted from the implementer, who disclosed it rather than letting a green imply coverage:

the dialect-scope row (help text + docblock) has NO executable detector — it stayed green under the ablation because nothing asserts it, and is verified by reading only.

⇒ ruling item 3 is verified by reading, not by machine. If someone later deletes those two sentences, nothing reddens, and os generate migration --format sql silently resumes claiming dialect-neutrality — which is the very question ("which dialect does the generator claim to match") this card exists to settle.

This seat considered requiring a pin and decided against it: asserting help-text prose is brittle, and the ruling asked for a declaration rather than a guard. ⭐ That is a judgement call, recorded here so it can be overturned rather than discovered later.

⚠️ Clause ② — graded no, and a tension this seat will not paper over

Graded from the delivered diff, per limb:

  • Mechanical floor: no. No newly exported symbol, no new key on any published payload, nothing under packages/spec/src/**. The diff is packages/cli/src/commands/generate.ts, one pin test edited in place, one changeset.
  • Conformance limb: no, per the ruling: "generated scaffold output is a developer-facing artifact, not a published contract; the type half that WAS clause-② already landed in PR fix(cli): generated SQL migrations give timestamp columns their time zone #16070."

⚠️ But PR #16070 — the type half, the same command, the same file — was graded clause-② yes on the conformance limb, with the reasoning "the emitted DDL of a shipped command changes, so this is graded yes rather than argued down." This half changes the same command's emitted DDL, on the same two columns, and is graded no.

The ruling's stated reason for no (scaffold output is developer-facing, not a published contract) would have applied equally to the type half. ⇒ the two grades rest on a distinction the ruling asserts rather than derives. ⛔ This seat is following the ruling — it is binding and explicitly covers this diff's shape, and it explicitly invited the seat to re-grade, which is what the paragraph above is. But two contradictory grades on one surface should not pass silently, and this is worth the maintainer's revisiting.

Because clause ② is no, needs:contract-review was never applied to either carrier and there is nothing to strip. ⛔ Correspondingly, check-clause2-carriers was not run — there is no pair to check.

CI — the full population

33 of 33 complete, every one success or skipped, none failed, at head 2e586a127b6; page 2 of the listing empty.

⚠️ Stated because it changed mid-verification: the count was 32 when this seat began the landing checks and is 33 now — a Test Core rollup job (101473704865) appeared and completed at 10:50:28Z. Had the earlier accounting been carried forward, this landing would have asserted a population that no longer existed. Re-read, not remembered. (The same thing happened on #16265 in the other direction, 37 → 36.)

⭐ Pre-squash commit-message check — clean

New this round, and applied here. git log <merge-base>..<head> --format='%B':

  • one commit to be squashed: 52145d48e37;
  • card-relation trailers inside commit bodies: none;
  • falsified-claim phrases: none;
  • files: exactly generate.ts, the pin test, and one changeset.

This check exists because the queue squashes, and squashing concatenates every commit message into main permanently — measured this morning on #16247, where two sentences already known false went into history that way. Recorded on #16158.

Gate union, and one declared narrowing

dispatch-gates.mjs run twice: the plain form printed Reconciliation — 57 famil(ies), the --commands form harvested exactly 57 invocations while printing no reconciliation line. All 57 run: 54 exit 0, zero red, 3 NOT MEASURED, each quoted from its own verdict text rather than scored off an exit code —

  • check:dual-build-cjs-loads"PREREQUISITE NOT MET … This is NOT a pass: nothing was measured"
  • check:i18n-coverage"Nothing was compared … says NOTHING about whether any declared label went untranslated"
  • check:type-check-debt — its self-test passed and its plain sibling check:type-check-coverage passed, but its whole-tree --re-measure sweep was stopped deliberately after holding the shared verify lock ~19 minutes with two agents queued.

⭐ That stop is a declared narrowing, neither pass nor red, and it was done correctly: SIGTERM to an explicit PID (exit 143), ⛔ not a name-pattern kill — the process table is shared with parallel agents, which is #16182.

Provenance of the work itself

⚠️ This PR was finished by a continuation after the 09:40Z container restart killed its predecessor mid-flight. The predecessor's commit 52145d48e37 already contained the whole implementation — but its measurements died with it and could not be audited, so every claim above was re-measured from scratch rather than inherited. That instruction was not ceremony: on the sibling card #15880 the same rule caught a predecessor's head that was actually red.

Filed from the same run: #16279 — no test ever executes the migration DDL os generate migration emits; both formats are pinned as text only.

Flipping ready and arming. ⛔ Card #15521's pm:dispatched comes off after the merge; the PR closes it with Fixes, both ruled rows being delivered.


Generated by Claude Code

@os-litant
os-litant marked this pull request as ready for review September 6, 2026 10:55
@os-litant
os-litant enabled auto-merge September 6, 2026 10:55
@os-litant
os-litant added this pull request to the merge queue Sep 6, 2026
Merged via the queue into main with commit dacb73f Sep 6, 2026
35 checks passed
@os-litant
os-litant deleted the claude/issue-15521-audit-stamp-nullability-and-default branch September 6, 2026 11:16
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