Skip to content

fix(cli): generated migrations give the table's own id the driver's column shape - #15518

Merged
os-litant merged 1 commit into
mainfrom
claude/issue-15040-generated-migration-id-column-shape
Sep 4, 2026
Merged

fix(cli): generated migrations give the table's own id the driver's column shape#15518
os-litant merged 1 commit into
mainfrom
claude/issue-15040-generated-migration-id-column-shape

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes #15040

Both migration generators in packages/cli/src/commands/generate.ts hardcoded the table's own primary key as a UUID. The platform's SQL driver emits table.string('id').primary() for that column — knex's varchar(255), SqlDriver.DEFAULT_STRING_VARCHAR_CHARS. A platform id is a string, not a uuid, so on Postgres the generated table refuses the platform's first insert with 22P02 invalid input syntax for type uuid.

Two literal lines, one per generator. No restructuring.

What changed

before after
generateMigrationSql "id" UUID PRIMARY KEY DEFAULT gen_random_uuid(), "id" VARCHAR(255) PRIMARY KEY,
generateMigrationTs table.uuid('id').primary().defaultTo(db.fn.uuid()); table.string('id').primary();

The DEFAULT half is the quiet one, and it is what the fix is really for

The loud failure announces itself: 22P02 on the first insert, so nobody ships on top of the generated table silently. The default does not.

Measured: SqlDriver.create() always supplies the id itself — _id, else a caller-supplied id, else a minted one — so DEFAULT gen_random_uuid() never fires for a platform write. It fires only for an out-of-band insert, and hands that row a 36-character uuid this platform's id generator would never mint. One table would then hold two incompatible id shapes, with nothing anywhere saying so. The driver emits no database-side default for id at all, and neither generator does now.

On Postgres knex.fn.uuid() compiles to (gen_random_uuid()), so the two generators were emitting one and the same wrong default in two spellings.

The file already contradicted itself

generate.ts states, a few hundred lines above the two generators, that a reference column takes the width of the target's id column because the driver emits table.string('id').primary() — the derivation #14828 applied to the lookup / master_detail / user / tree rows. Those comments were already right. Only the code was wrong, and this change makes the code agree with prose that had been correct all along; no comment needed correcting.

Measured, not quoted

The 22P02 refusal itself is quoted from sql-driver.ts's own prose, not driven against a live Postgres — there is no database in this container. What is driven here is the emitted shape, on both sides, compiled offline through knex's pg dialect (knex.schema.createTable(...).toSQL(), a pure compile with no connection):

driver-sql (postgres arm)   "id" varchar(255)
generateMigrationTs before  "id" uuid default (gen_random_uuid())
generateMigrationTs after   "id" varchar(255)

The after row is byte-identical to the driver's own id column.

Third disagreement, measured and NOT folded in: the audit-stamp columns

Triage asked for created_at / updated_at to be decided in the same pass rather than left half-done. Measured; it is a third disagreement, and this PR deliberately does not touch it.

driver-sql   table.timestamp(name).defaultTo(knex.fn.now())     -> nullable
sql gen      "created_at" TIMESTAMP NOT NULL DEFAULT now()      -> NOT NULL
ts gen       table.timestamps(true, true)                       -> NOT NULL

table.timestamps(true, true) compiles to .notNullable().defaultTo(CURRENT_TIMESTAMP) on both columns (knex 3.3.0, knex/lib/schema/tablebuilder.js). Compiled through the same offline pg probe:

driver  "created_at" timestamptz default CURRENT_TIMESTAMP
ts gen  "created_at" timestamptz not null default CURRENT_TIMESTAMP

So the disagreement is nullability, not type — unlike the id column, where the type itself was wrong. It is not obviously a wrong value on either side: the driver stamps both columns on every write, which makes NOT NULL arguably the truer constraint, and the driver's DDL is dialect-branched (datetime(3) on MySQL, a canonical ISO default on SQLite, per createAuditTimestampColumn) in a way a Postgres-flavoured generated migration does not try to reproduce. Which side moves is a decision, not a correction, so nothing here changes those lines. The measurement is recorded as the last case of the new pin, explicitly as a record rather than a ruling, so it cannot change shape unnoticed.

The pin reads the authority instead of transcribing it

packages/cli/src/commands/generate-builtin-id-column.pin.test.ts. A pin asserting varchar(255) would re-create this very defect one layer up — the whole shape of this card is "the generator disagrees with the driver". So the width is read from DEFAULT_STRING_VARCHAR_CHARS where it is declared, and the typescript generator's emitted line is compared against the driver's own call byte for byte. Both extractions carry non-vacuity controls, and every "must not" assertion carries an anti-vacuity control proving the predicate fires on the shape that was there before.

It needs no new declared inputs: packages/drivers/driver-sql/src/sql-driver.ts is already listed under @objectstack/cli#test in turbo.json, and pnpm check:cross-package-test-inputs passes unchanged (26 package(s) read outside themselves, all declared).

Two legs of ablation, one per generator, because the two lines live in different functions and a pin covering only one would look identical from the outside.

Clause-②: no, re-declared from the delivered diff

A scaffolding generator's emitted DDL is corrected toward what the platform's driver already emits. No exported symbol, no accepted key or value, no schema and no wire shape moves. Changeset is patch on that basis: per Check Changeset's WHICH LEVEL prose, minor is for a purely additive widening of a published package's public surface, and a fix( that changes no public surface stays patch.

Verification

All readings at bffde48fc99, on a clean tree, in the worktree objectstack-issue-15040. Heavy runs went through scripts/pm/os-verify-lock.sh; every verdict below is quoted from the gate's or the lock's own verdict line, never from a bare $?.

  • @objectstack/cli full suitepnpm --filter @objectstack/cli exec vitest run --maxWorkers=2. Test Files 244 passed, Tests 2872 passed | 6 expected fail | 11 skipped, plus the five *.e2e.test.ts suites re-run after turbo run build satisfied their requireBuiltCli prerequisite: Test Files 5 passed (5), Tests 11 passed (11). The first run reported those five as failed suites for one reason — packages/cli is not built: .../dist/commands/serve.js does not exist — which is a prerequisite, not a finding, so it was satisfied and re-measured rather than reported.
  • pnpm --filter @objectstack/cli typecheckVERDICT command-exit 0; check:test-typecheck: OK — @objectstack/cli's test layer compiles under packages/cli/tsconfig.test.json. The new pin really is in the program: tsc --noEmit --listFiles names generate-builtin-id-column.pin.test.ts and commands/generate.ts, so "typecheck is clean" covers the file this PR adds rather than merely being true.
  • Gate union — derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands from the change set the script takes itself (3 paths). 49 commands, all exit 0. Four first came back PREREQUISITE NOT MET / NOTHING was measured (check:dual-build-cjs-loads, check:i18n, check:i18n-coverage, check:i18n-walk-parity) — read as NOT MEASURED, satisfied with a workspace build, and re-run green. The five roster families the derivation flags as scoring silent for structural reasons with a roster under one of these paths were run too, not read as clearance: check-changeset-fixed, check:authz-resolver, check:error-code-casing, check:filter-alias-parity, check:swallow-census-controls — all exit 0.
  • ⚠️ Declared limitation. The derivation printed STALE TREE — this branch is behind origin/main, and four files the answer derives from changed across that range (check-dispatcher-error-vocabulary.mjs, check-type-check-coverage.mjs, engine-double-contract.pinned.json, measure-self-test-floor.mjs). The family list itself was byte-identical when re-derived after a fresh git fetch origin main, and three of those four gates ran green here from this tree's copies — but the reading is from this tree, not from origin/main. CI runs the farm against the merge and is the stronger reading.

Ablation — two legs, one per generator, proved on disk

Predicted before running: restoring uuid in one generator reds that generator's cases while the other generator's stay green. Both legs behaved that way, and distinctly.

No rebuild is involved and none is owed: the pin imports ./generate.js relatively, so vitest reads src/commands/generate.ts itself rather than anything in dist/. That is not an assumption here — each leg mutated only the source and the pin went red without a build, which is what proves the resolution path.

leg mutation, proved on disk red green
SQL removed-text count 1 -> 0, injected marker 1 the sql generator emits the driver's width, neither generator gives the id a uuid type, neither generator gives the id a database-side default the typescript generator emits the driver's own line, byte for byte
TS removed-text count 1 -> 0, injected marker 1 the typescript generator emits the driver's own line, byte for byte, and the same two shared cases the sql generator emits the driver's width

Both legs: Tests 3 failed | 48 passed (51), and in both Test Files 1 failed | 2 passedgenerate-field-type-vocabulary.pin.test.ts and generate-multiple-json-column.pin.test.ts stayed green through both mutations. That is the case for the new pin existing: the two pins already driving these generators do not cover the builtin id column, so either regression could have landed under a green suite.

Restore was git checkout HEAD -- ABSOLUTE_PATH from a trap, and is proved rather than assumed: blob hash 06db8cc9cc53cccc3e5aa27ac50417e0717655b1 equal to the HEAD blob and git diff HEAD empty, checked after each leg. An empty hash is treated as failure, not as "nothing to compare".


Generated by Claude Code

…olumn shape

Both migration generators hardcoded the primary key as a UUID:

  generateMigrationSql   '  "id" UUID PRIMARY KEY DEFAULT gen_random_uuid(),'
  generateMigrationTs    "    table.uuid('id').primary().defaultTo(db.fn.uuid());"

driver-sql emits `table.string('id').primary()` for that column — knex's
varchar(255), SqlDriver.DEFAULT_STRING_VARCHAR_CHARS. A platform id is a
string, not a uuid, so on Postgres the generated table refused the platform's
first insert with `22P02 invalid input syntax for type uuid`.

The DEFAULT is the quieter half and the reason this is worth correcting rather
than working around: the driver emits no database-side default, because its
insert path always supplies the id itself. `gen_random_uuid()` therefore only
ever fired for an out-of-band insert, handing that row a 36-character uuid the
platform's generator would never mint — one table holding two incompatible id
shapes, silently.

Both generators now emit the driver's own answer. The correction also closes a
contradiction inside generate.ts, whose prose already stated that a reference
column takes the width of the target's id column *because* the driver emits
`table.string('id').primary()`, a few hundred lines above the two lines that
emitted uuid.

generate-builtin-id-column.pin.test.ts reads the width from the driver's own
DEFAULT_STRING_VARCHAR_CHARS rather than transcribing 255, so the generators
cannot drift away from the driver again without a named failure. Its last case
records — deliberately without correcting — a third disagreement measured in
the same pass: the audit-stamp columns. driver-sql emits them nullable
(`table.timestamp(name).defaultTo(knex.fn.now())`); both generators emit them
NOT NULL. Which side moves is not this change's to decide.

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 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

3 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/deployment/cli.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/protocol/kernel/lifecycle.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
What this run could not see
  • 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 20032594f4a9ac8c78e2d6a3eedbe8e5aeae4911packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 20032594f4a9ac8c78e2d6a3eedbe8e5aeae4911

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

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