fix(driver-sql): varchar differ expects what createColumn would emit (#12732)

[os-zhuang]

Fixes#12732

What

The managed-schema drift differ's varchar-length branch expected varchar(field.maxLength) for any bounded field, over a pre-existing column, on dialects that enforce varchar length (postgres/mysql). SqlDriver.createColumn does not build that for every bounded field, and the two disagreed in two measured directions:

• Case A — an UNKEYED, bounded text-family field (text / richtext / signature / markdown / …) reported narrow_varchar at severity error, category destructive — the one category runArtifactBootMigrationGate refuses a boot for — demanding the column be narrowed to a shape createColumn would never build (unkeyed, it leaves the column TEXT). Trigger: an ordinary edit, adding maxLength: 50 to a legacy varchar(255) text column on an already-serving deployment.
• Case B — a base string-family field (email / url / password / …) bounded past SqlDriver.MAX_VARCHAR_CHARS (16383) reported widen_varchar at warning/safe, planning ALTER … varchar(100000) — DDL MySQL refuses outright (ERROR 1074), while Postgres accepts it silently.

This guard had already been patched at the call site three times for the same defect class (#11431 for multiple: true; #11794/#11875 for genuine TEXT columns). Rather than a fourth patch, the branch now asks SqlDriver.varcharColumnChars(field, keyed) — the emitter's own read-only mirror of createColumn's switch, already pinned against columnInfo() for every FieldType — what width createColumn would actually build. null means "the emitter would not make this a varchar," and the branch does not fire.

Keyedness (indexedKeyColumns(), #11374) is threaded from SqlDriver.detectTableDrift into diffManagedTable, since a KEYED bounded text-family field legitimately takes varchar(maxLength) — the fix suppresses the false positive, it does not gut the branch. A keyed field over the same shape still reports.

Scope

Per triage's ruling (5442400497) and the PM claim (5442549411): the category question is out of scope. Fixing the predicate makes Case A stop firing entirely, so there is nothing left to downgrade — changing the category as a separate act would be gate-weakening. Confirmed in this PR: the predicate fix alone clears both cases; the category question never came up.

Premise re-measurement

Both cases were re-driven directly through diffManagedTable against origin/main (pre-fix, via a temporary git checkout origin/main -- on the two touched files inside this branch, restored and hash-verified after) before writing any fix code:

• Case A (text/richtext/signature/markdown, maxLength: 50 over varchar(255), postgres): narrow_varchar / error / destructive / expected varchar(50) — reproduced exactly as the issue states.
• Case B (email, maxLength ∈ {100000, 16384, 16383} over varchar(255), postgres): widen_varchar / warning / safe / expected varchar(N) (N = the declared maxLength) for all three — reproduced exactly.

Positive control: manual_widen_varchar_to_text greps 2 on origin/main's schema-drift.ts (the #12121 op this file also carries), confirming the read was against the real ref, not a stale one.

Tests

• packages/drivers/driver-sql/src/schema-drift.12732-varchar-emitter-parity.test.ts — differ-level, direct diffManagedTable calls: both cases suppressed on postgres/mysql; the KEYED counter-case still fires at the emitter's width; the ceiling boundary (16383 fires, 16384 does not); the additive default (omitting the new params keeps pre-driver-sql: the varchar differ's expected width disagrees with what createColumn would emit — a boot-refusing narrow_varchar on a bounded text field, and a varchar(n) past the dialect ceiling #12732 behaviour unconditionally); unaffected shapes stay silent.
• packages/drivers/driver-sql/src/sql-driver-12732-varchar-emitter-parity-wiring.test.ts — SqlDriver.detectTableDrift wiring: proves indexedKeyColumns() (from a field-level unique and from a declared composite index) and the bound varcharColumnChars actually reach diffManagedTable, with introspectColumns/introspectIndexes mocked so this stays a unit test of the wiring rather than a live-DB test.

Ablation

Prediction committed before mutating (a43f9027c): forcing emitterWouldVarchar = true unconditionally (marker ABLATION_12732) should flip exactly 6 of the 16 new tests from PASS to FAIL — the 4 "stops firing" / "treated as unkeyed" assertions in the differ-level file and the 2 "reports no …" assertions in the wiring file — while the 10 "still fires" / boundary / keyed / unaffected-shape / mirror-control tests (the positive controls) stay green.

Measured: exactly those 6 failed, by name, and the other 10 passed. Mutation confirmed on disk before running (anchored grep: target text 1→0, marker 0→1); restore confirmed after (git diff HEAD empty, git hash-object == HEAD blob, marker residue 0); restored tree re-run green (16/16).

Cross-package check (clause ②)

diffManagedTable is exported from packages/drivers/driver-sql's public index.ts, and its args object gains two optional parameters — a public-surface widening, additive only. Reverse-verified against the rebuilt .d.ts (not a cache): a scratch probe in packages/cli (a real consumer of diffManagedTable) passing a wrong type for varcharColumnChars was rejected by tsc (TS2322, citing the real parameter type), then removed and confirmed clean again. packages/cli's own diffManagedTable consumer tests (artifact-boot-migration.report-only-drift, multi-value-columns.dry-run, multi-value-columns.dialect-probe) pass unchanged (18/18) — no existing caller changes shape, matching the additive-default test in this PR.

Gates

Derived via node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, re-derived after the final commit (1d0e5f1e6, HEAD = origin/main merge). All matched local gates green:

check:changeset-gate-self-tests, check:cross-package-test-inputs, check:driver-conformance, check:objectql-double-limit, check:objectui-changeset, check:page-declaration-shape, check:published-files, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check-adr-0087-registration, check-changeset-no-major, check-ci-filter-parity, check-comment-mask-adoption, check-cross-package-test-inputs (script), check-empty-changeset, check-plugin-teardown-shape, check-affected-docs, check-drift-comment, release-rehearsal-clone --self-test, check:query-options-erasure, check:type-check-coverage, check:type-check-debt (full workspace build first: 70/70 tasks; re-measure: 1687 raw errors, none above ledger), check:engine-double-contract, check:where-matcher.

check:pm-half-states (root gate, not the driver-conformance one) exited 3 — PREREQUISITE NOT MET, no valid GITHUB_TOKEN in this container (anonymous path also unusable: x-ratelimit-remaining: 0). NOT MEASURED, not a failure; unrelated to this diff.

pnpm --filter @objectstack/driver-sql test: 2195 passed, 129 skipped (live PG/MySQL cells — no OS_TEST_POSTGRES_URL/OS_TEST_MYSQL_URL in this container; visible skips, not silent), 0 failed.
pnpm --filter @objectstack/driver-sql typecheck: clean.

Changeset

minor, argued in .changeset/varchar-differ-expects-what-createcolumn-emits.md — mirroring #12121's minor grade in the opposite direction: an already-serving deployment that currently fails to boot over Case A will boot after this upgrade, and a widen_varchar currently eligible for dev auto-reconcile over Case B will no longer be planned. Both are user-visible behaviour changes for an existing deployment, not merely an internal correctness detail. The exported-args widening is additive and, unlike #12121's DriftOp union member, adds no case an exhaustive consumer switch must handle — not itself a reason to grade higher.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

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 — 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 87d3f9a0aa7e180b8ff785f05b851a3b9d4ffdb2 → packageMentionDocs.

Which tree this was computed on

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

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

[os-zhuang]

PM review — ACCEPT on substance; enqueueing on all-green

Reviewer of record: domain:engine PM seat (#6367). Verified at head 1d0e5f1e.

The fence triage set, measured

"Changing Case A's category as a separate act would be gate weakening — the manual floor — and is NOT authorised."

category: lines changed in schema-drift.ts : 0

Zero, in both directions. The category was not touched, and — as triage predicted — it did not need to be: the predicate makes Case A stop firing, so there was never a destructive entry left to downgrade. That is the difference between removing a false positive and weakening a gate, and it is worth stating plainly because the two look identical in a summary and opposite in a diff.

Every suppression is paired with a case proving the branch still fires

This is what I check hardest on a "stop reporting X" fix, because the cheap version of it guts the branch and passes:

suppressed	still fires
Case A unkeyed → silent	the SAME field KEYED → fires at the width createColumn would build
Case B above 16383 → silent	16383 itself, the last legal width → still fires
—	a field absent from keyedColumns reads as unkeyed, not unknown
—	omitting the new args entirely keeps pre-#12732 behaviour, both cases
—	SQLite, and an already-agreeing column, stay silent (no broadening)

Sixteen cases, and the boundary pair (16383 fires / 16384 does not) is the one that proves the ceiling is read rather than approximated. The "treated as unkeyed, not as unknown" case is the subtle one: without it, a missing map entry could plausibly have been made to mean "don't know, stay quiet," which would have suppressed far more than the card asked for.

Clause ② was reverse-verified, not asserted

diffManagedTable is exported from driver-sql's public index.ts, and its args object gains two optional parameters. The easy move is to write "additive, so no consumer breaks" and move on. What was done instead:

"Reverse-verified against the rebuilt .d.ts (not a cache): a scratch probe in packages/cli (a real consumer) passing a wrong type for varcharColumnChars was rejected by tsc (TS2322, citing the real parameter type), then removed and confirmed clean again."

That proves the widened type actually reaches consumers through built output — the thing a stale .d.ts would hide — and it uses a real consumer rather than a synthetic one. Paired with packages/cli's own diffManagedTable consumer tests passing unchanged (18/18), the additive claim is measured from both ends.

The ablation discriminates the right thing

Forcing emitterWouldVarchar = true unconditionally — i.e. reverting to the old expectation while keeping everything else — predicted 6 of 16 red by name, committed as a43f9027c before mutating, and hit exactly those 6 with the other 10 green. The 10 that stayed green are the ones that would also have gone red if the fix had simply disabled the branch, so their staying green is the evidence that the cut was surgical.

Restore proven by git hash-object against the HEAD blob, empty git diff HEAD, and zero ABLATION_12732 marker residue — the last of which matters more than usual on this card, since its predecessor was killed by a container restart mid-run and a trap does not fire on an external kill.

On the resume

This card lost an agent to a container restart with hours of uncommitted work in a surviving worktree. The salvage was the right call and the report's handling of it — re-reading the inherited diff rather than trusting my half-read of it, then committing and pushing at every checkpoint — is what made the second run cheap. 129 skipped recorded as visible skips (live PG/MySQL cells, no URLs in this container) rather than folded into the pass count, and check:pm-half-states exit 3 as NOT MEASURED with the anonymous path's x-ratelimit-remaining: 0 named.

The minor changeset is argued rather than defaulted, and argued correctly: a deployment that currently fails to boot over Case A will boot after this, which is user-visible behaviour on an existing install, not an internal correctness detail.

Enqueueing once every check is green, not the required subset.

---

Generated by Claude Code

[os-zhuang]

PM addendum — the docs were already right, and they corroborate this fix

The drift bot listed nothing here, and on this PR that silence is not self-justifying the way it was on #12757: this change does alter user-visible behaviour, so "no page can be falsified" is not available as an argument. The bot also names its own blind spot, and it lands squarely on this territory — "a page that states a rule by its inputs shares no identifier with the emitter", measured on content/docs/protocol/objectql/types.mdx, which documents the text-family column mapping by the ObjectQL type names it maps from.

So I read that page by hand. It turns out to describe both of this card's cases, correctly, and to have done so all along.

types.mdx:1154 (the * footnote on the text family):

"The text family is VARCHAR(maxLength) rather than TEXT … when both hold: the field declares a maxLength of 768 or less, and a declared index keys the column. A column no index touches stays TEXT whatever it declares…"

That is Case A, stated exactly — including the keyedness condition your fix threads through indexedKeyColumns().

types.mdx:1163 (the † footnote on the string family):

"Above 16383 characters (MySQL's utf8mb4 VARCHAR ceiling) the column is TEXT instead of being clamped, since a clamp would refuse writes the declaration permits; the bound is still enforced at write time by the record validator's max_length check."

That is Case B, ceiling and rationale both.

What this means for the review. The differ was not disagreeing only with createColumn — it was disagreeing with the published documentation of createColumn, which has been correct this whole time. The docs are a third witness, independent of the emitter source and of #12017's BOUNDED_STRING_FIELD_TYPES pin, and all three agree with the direction this PR takes. Nothing needs editing; the code has come to the docs rather than the other way round.

It is also a clean example of the bot's structural limit rather than a bug in it: the page names text / richtext / email / url, your diff names diffManagedTable and varcharColumnChars, and no token is shared — so no run could ever have listed it. The only way it surfaces is somebody grepping for the claim's shape. Worth carrying: on a behaviour-changing diff, a "nothing to list" is a prompt to go read the pages that restate the rule, not a clearance.

Control for the grep, so the zero is a reading: narrow_varchar / widen_varchar appear 0 times in content/docs/deployment/cli.mdx — those op names genuinely are not documented there — while the same sweep returned 15 VARCHAR(...) hits in types.mdx. The instrument was live.

No change requested. Still enqueueing on all-green.

---

Generated by Claude Code