Skip to content

fix(driver-sql): name the real remedy when a bounded field sits over a stale TEXT column - #13073

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-12999-stale-text-column-remedy
Aug 29, 2026
Merged

fix(driver-sql): name the real remedy when a bounded field sits over a stale TEXT column#13073
os-zhuang merged 2 commits into
mainfrom
claude/issue-12999-stale-text-column-remedy

Conversation

@claude

@claudeclaudeBot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Fixes#12999

One message served two causes and was true of only one of them.

The defect

explainUnkeyableTextColumn (packages/drivers/driver-sql/src/sql-driver.ts) turns MySQL's
ER_BLOB_KEY_WITHOUT_LENGTH / ER_TOO_LONG_KEY index refusal into operator-readable advice. It rendered
every such refusal as "the field declares no maxLength … Declare maxLength on the field(s) so the
column is emitted as varchar(n) and can be keyed"
.

paththe factwhat the message said
createthe field really declares no usable bound"declare maxLength"correct
upgradethe field does declare a usable bound; the COLUMN predates it and is still TEXT"declare maxLength"both halves false, and a no-op

The additive sync adds columns and indexes and deliberately never rewrites a column's type (#3728). So once
a release adds a bound to a previously unbounded keyed field — #12978 did exactly that for five
sys_notification_* objects — the field declares a perfectly usable maxLength while the physical column
is still TEXT. Every boot re-attempts the missing index, the refusal re-fires, and the message tells the
operator to do the thing they already did. In production that reads as the release that just landed being
broken, which is the worst possible reading of a correct change.

What changed

A second branch, selected per column, on a criterion that needs both halves:

the physical column is TEXT andkeyableTextLength says a fresh create would have emitted
varchar(n) for the field's declared bound.

Both inputs were already in hand on the failure path: the columnInfo() read this method already performs,
and the driver's managedObjectFields registration (reached through the existing declaredFieldsFor
helper, so there is one derivation and two readers).

The criterion needs the physical half and cannot be shortened to "the declaration is bounded", because
ER_TOO_LONG_KEY can refuse a composite whose parts are each individually keyable — the 3072-byte
budget is per KEY, not per column, as MAX_KEYABLE_VARCHAR_CHARS already records.

The new message names the column, the bound it already declares, that re-declaring changes nothing, and the
remedy that does apply: convert the column to varchar(n)by hand, with a backup taken first, restating
the FULL column definition on MySQL
(MODIFY does not repeat a NOT NULL and silently drops a DEFAULT it
does not restate) — after which the next boot creates the index. The wording reuses the #12121 mirror case
already in schema-drift.ts rather than inventing one, and the two MySQL MODIFY hazards it names are the
two this same file already derived on its datetime and time widenings. A composite key that mixes a stale
column with a genuinely unbounded one names both dispositions, so an operator is not sent down one route
for both columns.

Fences, each honoured and each pinned

  • Only a branch was added. The CREATE-path message is byte-identical; the diff on that return statement
    is zero lines. A field that really declares no usable bound still gets it, and so does a bound wider
    than a utf8mb4 key part can hold (768 characters) — a fresh create emits TEXT for that one too, so it is
    not a stale column, and the 768-character ceiling is the fact that operator needs.
  • The refusal stays loud. Nothing was silenced or downgraded to warn. The index genuinely was not
    created, the declared uniqueness genuinely is not enforced, and the message still says so; a dedicated
    test asserts the loudness clauses survive on the new branch.
  • The additive sync still never rewrites the column. No ALTER … MODIFY was added. That statement needs
    an exclusive metadata lock on the table (documented at the top of this file), which makes it destructive,
    hard to roll back, and a deliberate manual floor — the new message says so explicitly, so the remedy
    cannot be misread as something the driver will eventually do for you.

Verification

Union re-run on the final commit 52a242ecc (after merging origin/main, which touched this same file):

whatverdict line
pnpm --filter @objectstack/driver-sql typecheck && … testos-verify-lock: VERDICT command-exit 0Test Files 146 passed | 9 skipped (155), Tests 2229 passed | 131 skipped (2360)
pnpm lint (repo-wide eslint . --no-inline-config, not narrowed)os-verify-lock: VERDICT command-exit 0
25 derived gate families (node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, re-derived on the merged head)all EXIT=0; e.g. check:keyed-text-bounds printed 148 keyed text-family columns judged, 148 bounded

tsc --noEmit --listFiles reports 2 hits for the two changed TypeScript files, so the green typecheck
really did read both — this package's tsconfig.json does not exclude *.test.ts.

Pins in both directions

Seven cases in packages/drivers/driver-sql/src/sql-driver-12999-stale-text-remedy.test.ts, on SQLite so
they run in Test Core. The stale fixture is performed, not described: the object is booted twice, first
unbounded and then bounded, and the suite asserts the physical column is still text afterwards — so the
premise the whole card rests on is measured rather than assumed.

  • new branch: the remedy text, and the loudness clauses that must survive it;
  • counter-pin: an unbounded field still gets the original declare-maxLength message;
  • counter-pin: a bound past the 768-character key ceiling is not a stale column and keeps that message;
  • counter-pin: a table this driver holds no declaration for (external/federated, or a shard registered under
    its base name) degrades to that message rather than guessing;
  • mixed composite: both dispositions named;
  • the helper still declines a failure that is not one of the two MySQL codes.

Ablation — two legs, mutation proven on disk, restore proven byte-identical

Both legs mutate sql-driver.ts under a trap … EXIT INT TERM with absolute paths, count the anchored text
before and after, and restore with git checkout HEAD -- ABSOLUTE_PATH verified against the HEAD blob hash
40fa55994cb457e5b3d53a0703ac9167f4a01b63and an empty git diff HEAD. The suite resolves
../src/index.js, i.e. package source, so no rebuild sits between the mutation and the measurement.

  • Leg A — delete the new branch (staleBounds.size > 0 becomes > 999999; anchor 1 to 0, injection 0 to
    1, blob 5e0533865…): Tests 2 failed | 5 passed (7). The two branch-selection cases red; the three
    counter-pins and the loudness case stay green, which is correct — they assert text common to both
    messages or the CREATE message itself.
  • Leg B — break the CREATE-path message the counter-pins guard (varchar(n) becomes VARCHAR(N);
    anchor 1 to 0, injection 0 to 1, blob d03d9ca16…): Tests 1 failed | 6 passed (7), the failing one being
    the unbounded-field counter-pin. So the counter-pin family can fail, and is not decorative.
  • Disclosure: the first attempt at leg B was a no-op — the anchor was spelled with a bare backtick
    while the source carries an escaped one, so it matched 0 times, the tree was never mutated, and the suite
    passed. The anchor counts caught it; that reading was discarded and the leg re-run with a
    backtick-free anchor. Recorded because a silent re-run to a landing mutation would have reproduced the
    same defect one layer up.

Declared narrowings (what was not run here, and why)

  • No live MySQL is reachable in this container (mysql://127.0.0.1:3306 refused, no client binary), so
    the end-to-end MySQL boot in which the server itself raises the refusal was not exercised locally. The
    live cells in sql-driver-keyed-text-mysql.test.ts own that path; the one live assertion that reads this
    message text pins the CREATE path over an unbounded field, which this change leaves untouched by
    construction and which leg B proves is still asserted. Temporal Conformance (live PG + MySQL) runs it.
  • check:type-check-debt --re-measure and check:dual-build-cjs-loads need the whole workspace built and
    are left to CI's required jobs; driver-sql carries no debt ledger entry and its typecheck is clean.

Clause ② self-assessment — no

Judged against the actual diff, not the card: the change moves diagnostic message text on a failure path
and nothing else. No accept/reject behaviour moves (no input is newly accepted or newly refused; the same
failures fail, identically, and the same throw carries them), no exported surface widens (the method is
protected, no export, schema, DDL or wire shape changes), and no new error code is stamped. The only
observable delta is which of two English paragraphs an operator reads for a refusal that already happened.

Changeset — grade A-, argued

.changeset/driver-sql-stale-text-column-remedy.md, patch on @objectstack/driver-sql.

It earns the grade on the three things a changeset is read for. Bump correctness: diagnostic text only,
no surface moves, so patch is right and minor would over-signal. What an upgrader has to do: an
operator hitting this now has the manual conversion named in full, restate step included — the clause whose
omission would have turned a harmless no-op into an operation that loses column defaults. Why, not just
what:
it states the mechanism (additive sync never rewrites a column type) rather than the symptom, so the
next reader can tell which of their deployments is affected.

Marked down from A because the release-notes reader cannot see the new text itself — the entry describes the
message rather than quoting it, and a quoted before/after would let someone recognise the string from their
own boot log. Quoting it in full would have doubled the entry's length for a string that is only reachable
on MySQL, so the trade was made deliberately rather than overlooked.

Not addressed here


Generated by Claude Code

…a stale TEXT column
`explainUnkeyableTextColumn` rendered every ER_BLOB_KEY_WITHOUT_LENGTH /
ER_TOO_LONG_KEY index refusal as "the field declares no `maxLength` — declare
one". True at CREATE time; false in both halves on the UPGRADE path, where the
additive sync never rewrites a column's type, so a field that HAS since
declared a usable bound still sits over a TEXT column and the operator is told
to redo what they already did — once per boot, in production.
Adds a second branch selected per column on "physical column is TEXT AND
keyableTextLength() would have emitted varchar(n)", using the columnInfo() read
this method already performs and the driver's managedObjectFields. It names the
declared bound, says re-declaring changes nothing, and gives the manual remedy
in full: convert by hand, backup first, restate the FULL column definition on
MySQL (MODIFY does not repeat NOT NULL and drops a DEFAULT it does not
restate), then let the next boot create the index.
The CREATE-path message is byte-identical, a bound past the 768-character key
ceiling deliberately keeps it, the refusal stays loud, and the sync still never
rewrites the column (that ALTER needs an exclusive metadata lock).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-sql, touching 2 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 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 269167fefd8f7ce6f5c3ee7cbffb5be73ede426epackageMentionDocs.

Which tree this was computed on

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

⚠️ 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 269167fefd8f7ce6f5c3ee7cbffb5be73ede426e → 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 29, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 29, 2026 03:19
@os-zhuang
os-zhuang enabled auto-merge August 29, 2026 03:19
@os-zhuang
os-zhuang added this pull request to the merge queueAug 29, 2026
Merged via the queue into main with commit ebcc34eAug 29, 2026
34 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-12999-stale-text-column-remedy branch August 29, 2026 03:42
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