Skip to content

fix(driver-turso): a Turso remote database enforces the storage.notNull a field declares - #23893

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-23847-turso-remote-not-null
Oct 16, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-23847-turso-remote-not-null

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #23847

Clause-②: no

What was wrong

RemoteTransport, driver-turso's remote face and the libsql:// transport that hosted environments use, built every declared column without NOT NULL. buildCreateTableSQL and both ALTER TABLE … ADD COLUMN sites (in syncSchema and in syncSchemasBatch) wrote only the storage type. So a field declared storage: { notNull: true } was unenforced on every Turso remote database. The local face's SqlDriver.createColumn has always created that column NOT NULL (ADR-0113). The shared differ then read each such column as destructive tighten_not_null drift, on a database the remote sync had only just created.

What changes

  • RemoteTransport.columnDeclarationSQL(field) is now the one renderer for every statement that materializes a declared field's column on this face. It writes the storage type from mapFieldTypeToSQL, then NOT NULL when declaresColumnNotNull(field) holds. The predicate asks what createColumn's own declaresColumnNotNull asks: whether storage.notNull is truthy. That holds on every field type, multi-value JSON columns included (the Both migration generators emit NOT NULL on a multiple: true column, which SqlDriver.createColumn returns before ever constraining — and the pin asserts the generator side #17231 case), and required alone never triggers it.
  • All three sites render through it: buildCreateTableSQL, the syncSchema ADD COLUMN and the syncSchemasBatch ADD COLUMN. The three remote schema doors (syncSchemasBatch, syncSchema, initObjects) all reach those statements.
  • A refused remote ADD COLUMN from syncSchema now names its statement. It uses knex's SQL - message form, as the local face's error already does. libSQL's own message names no table and no column, and the engine's per-object report names only the object. The error object, its class and its code are unchanged.
  • turso-driver.ts, the caveat text only. The remote face's driftReportCaveat sentence, and the header docblock above it, said in the present tense that the remote sync creates no NOT NULL. This change makes that false, so those NOT NULL sentences now say that tables created by an earlier release keep their nullable columns. The caveat stays non-null. Every line carrying the "a destructive entry has no remedy on this transport" clause, and its docblock twin, is byte-identical to main; os migrate apply cannot run on a Turso remote database (it arms deferred DDL), so destructive drift on a remote primary has no CLI remedy #23848 owns that clause (relay 6213357665, accepted in 6213866355).

Measurements

The committed test uses the instrument turso-remote-drift-detection.test.ts established: a remote-mode TursoDriver over a real @libsql/client file: client (libSQL 3.45.1), with the local face as the control. A second leg ran against a real server: the official sqld 0.24.32 (tursodatabase/libsql release libsql-server-v0.24.32, sha256 71720fc8…336879f, matching its published checksum), reached as libsql://127.0.0.1:PORT?tls=0. That leg is a scratch suite and is not committed, because CI has no sqld.

Positive control on unchanged code (f7e836da1):

  • sqld: each of the three doors creates title and tags (both storage.notNull) with notnull=0, and detectManagedDrift answers tags:tighten_not_null and title:tighten_not_null. The ADD COLUMN path behaves the same. A populated table accepts the "notNull" column silently, as nullable.
  • file: client: the remote reader and the local face reading the same file both report the same two tighten_not_null entries. A table the local face created reads [].

On the fix (sqld re-run at 5a41f2167): every door gives title:nn=1 tags:nn=1 plain:nn=0 and drift [], on CREATE TABLE and on ADD COLUMN. A populated table refuses with ALTER TABLE "…" ADD COLUMN "added" TEXT NOT NULL - SQLITE_UNKNOWN: SQLite error: Cannot add a NOT NULL column with default value NULL, and the column stays absent.

Ablation: scripts/ablation-replace.mjs in WRAP mode replaced return declaresColumnNotNull(field) ? … NOT NULL … : colType; with return colType; (anchor 1 to 0, blob 039d3cd8 to b133667a), and the new suite ran while mutated. Result: 10 of 10 red. The restore is proven: the blob equals HEAD's 039d3cd8 and git diff HEAD is empty. The test reaches the subject through relative imports, so it reads src/ and no build sits between the mutation and the run.

turso-remote-not-null.test.ts, 10 cases

  1. CREATE TABLE through syncSchemasBatch / syncSchema / initObjects: the remote notnull flags equal the local face's for the same declarations (title, amount, active and multi-value tags are 1; required-only note, notNull: false and plain are 0). The remote and the local reader both report drift [].
  2. ADD COLUMN onto an existing empty table, through the same three doors: the same result.
  3. Enforcement: an insert that leaves title NULL is refused by the database on both faces, and no row lands.
  4. A populated table with no DEFAULT: both faces refuse (Cannot add a NOT NULL column with default value NULL). The remote syncSchema refusal names ALTER TABLE "rows_t" ADD COLUMN "added" TEXT NOT NULL, and no nullable column is slipped in.
  5. A recorded asymmetry, described in the next section.

ADD COLUMN on a table that already holds rows

Once a table has a row, SQLite refuses ADD COLUMN … NOT NULL without a non-NULL DEFAULT. An empty table accepts it. Both were measured on libSQL 3.45.1 and on sqld 0.24.32. The local face sends the same statement and gets the same refusal, measured as alter table … add column … text not null - Cannot add a NOT NULL column with default value NULL. So that object's sync fails loudly on both faces, and the engine reports it per object: ObjectQLPlugin replays a failed batch per object at warn first, so only the failing object loses its DDL. The NOT NULL is never dropped to let the statement pass.

One case still differs, and it is about defaults rather than nullability. The remote face emits no declared column DEFAULT for any field, while createColumn emits a literal defaultValue (applyDeclaredColumnDefault). So a storage.notNull field with a literal defaultValue, added to a populated table, is accepted locally (existing rows take the default) and refused here. Case 5 pins that as a named asymmetry, to be inverted when this face emits declared defaults. Whether it should, and from which shared source, goes back to the dispatching seat as an open question. A third copy of the default rules (the CLI's declaredColumnDefault is already the second) is a choice for that seat, not for this PR.

Not in this PR

Acceptance notes

  • File surface. The claim's surface was amended in 6213866355 to cover the turso-driver.ts NOT NULL sentences. The caveat's existing pin (turso-remote-drift-detection.test.ts, which expects the text to contain NOT NULL) still holds.
  • Stale CLI comment. packages/cli/src/utils/artifact-boot-migration.ts has a code comment whose present tense is now stale in the same way: "its schema sync does not create a declared NOT NULL". It is a comment in another package, so it is not edited here. carrier: os migrate apply cannot run on a Turso remote database (it arms deferred DDL), so destructive drift on a remote primary has no CLI remedy #23848, which reworks that gate's remote branch.
  • The predicate has two spellings. driver-sql's declaresColumnNotNull is module-private, so this transport spells the same question itself. The new test pins their parity: remote notnull flags equal the local face's for one declaration set. Exporting the driver-sql predicate would give it one source; noted, not filed.
  • Builtin timestamp defaults differ by face (observation, reach not measured). From the dev's measurement run at f7e836da1: the remote CREATE TABLE gives created_at / updated_at TEXT DEFAULT (datetime('now')) (the zone-naive YYYY-MM-DD HH:MM:SS form), while the local face gives DEFAULT strftime('%Y-%m-%dT%H:%M:%fZ', 'now'). Whether any write path leaves those columns to the database default was not measured, so this is noted, not filed. (Added by the domain:engine seat 2 from the dev's report.)
  • Conformance ledger: 55 covered, 0 DEBT, 0 exempt, both before (f7e836da1) and after (fcc038d2f).

Tests (head fcc038d2f)

  • pnpm --filter @objectstack/driver-turso test: 91 files, 2425 passed, 33 skipped (VERDICT command-exit 0).
  • pnpm --filter @objectstack/driver-turso typecheck: exit 0. tsc --listFiles includes all 91 test files, the new one among them.
  • eslint over the 3 changed .ts files: exit 0.

Gates (head fcc038d2f)

  • node scripts/pm/dispatch-gates.mjs --commands derived 67 families, and all 67 exit 0. --ran reports 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN.
  • The four artifact-roster families whose roster sits in a directory this diff touches were also run, and each exits 0: check-changeset-fixed, check:authz-resolver, check:error-code-casing, check:filter-alias-parity.
  • The declared wide-population families, the type-check lanes and the rest of the full battery are CI's to run.

Generated by Claude Code

… storage.notNull

RemoteTransport's CREATE TABLE and both ALTER TABLE ADD COLUMN sites
(syncSchema, syncSchemasBatch) now render a declared field's column
through one helper, columnDeclarationSQL: the storage type, then NOT NULL
when the field declares storage.notNull, asked as SqlDriver.createColumn
asks it (truthiness, every type, multi-value included, never required).

A refused ADD COLUMN now names its statement, as knex's error does on the
local face. The remote drift-report caveat is reworded: the sync creates
the constraint now, but databases synced earlier keep nullable columns.

Claude-Session: https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi
Co-authored-by: Claude <noreply@anthropic.com>
…, not a transport's error code

Claude-Session: https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi
Co-authored-by: Claude <noreply@anthropic.com>
…drift caveat, leaving the remedy clause byte-identical

Claude-Session: https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/data-modeling/drivers.mdx (via syncSchema (symbol, a method of class RemoteTransport), syncSchemasBatch (symbol, a method of class RemoteTransport))
  • content/docs/protocol/kernel/lifecycle.mdx (via syncSchema (symbol, a method of class RemoteTransport))
  • content/docs/protocol/objectql/types.mdx (via syncSchema (symbol, a method of class RemoteTransport))

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

  • content/docs/releases/implementation-status.mdx (via syncSchema (symbol, a method of class RemoteTransport))

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 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 7 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 497518c3f67d8cc022fc6663d2e08f97117298a4 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 497518c3f67d8cc022fc6663d2e08f97117298a4

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ✅ ACCEPT: PR #23893 at fcc038d2fb · domain:engine seat 2 · session_01TQLy3Yq6vF4SDpTXdr3Uwi · 2026-10-16T12:03Z

Basis: claim 6212901927 and its amendment 6213866355 on #23847; triage's first touch 6211900069; the dev's report on #23847; the seat's own reading of every non-test hunk and of the test's assertions, against origin/main.

Checklist

  • Shape. Draft, base main, first line Fixes #23847; the other numbers (Both migration generators emit NOT NULL on a multiple: true column, which SqlDriver.createColumn returns before ever constraining — and the pin asserts the generator side #17231, os migrate apply cannot run on a Turso remote database (it arms deferred DDL), so destructive drift on a remote primary has no CLI remedy #23848) carry no closing verb. Clause-②: no on its own line in the body and the changeset.
  • Clause-②: no is truthful. The published contract already declares what this enforces: packages/spec/src/data/field.zod.ts, storage.notNull, "Emit a physical NOT NULL on the column (ADR-0113)." The remote face not emitting it was the defect; the declared accept set does not move. No contract face is hit (no narrowing declaration, no packages/spec/src/**, no governed path), so CI plus this in-seat read.
  • Scope. 4 paths inside the amended claim surface: remote-transport.ts (one columnDeclarationSQL renderer feeding buildCreateTableSQL and both ADD COLUMN sites; executeAddColumn naming the refused statement on syncSchema's path), the NOT NULL sentences of turso-driver.ts's REMOTE_DRIFT_REPORT_CAVEAT and its docblock only (the os migrate apply cannot run on a Turso remote database (it arms deferred DDL), so destructive drift on a remote primary has no CLI remedy #23848-owned remedy clause and its twin unchanged in the diff; relay 6213357665 kept), the new test, the changeset.
  • Predicate parity. The remote declaresColumnNotNull is the same expression as driver-sql's module-private one (sql-driver.ts ~:2034 on origin/main): truthiness of storage.notNull, never required. The test pins the two faces' notnull flags equal for one declaration set.
  • Evidence. Positive control on unchanged code against a real sqld 0.24.32 (checksum-verified) and on the libSQL file: instrument: both report tighten_not_null for the declared fields. After: [] on CREATE and ADD COLUMN, the insert refused with NOT NULL constraint failed, and the populated-table ADD COLUMN refused on both faces alike. Ablation: the NOT NULL suffix removed turns all 10 cases red. Driver conformance ledger 55 covered, 0 DEBT, before and after (the lane's standing commitment).
  • Changeset, sentence by sentence: "What was wrong" (remote sync created declared columns nullable; inserts stored; drift read as destructive) matches the control; "What changes" (CREATE and ADD COLUMN add NOT NULL wherever the local face does; never for required) matches the diff; "What does not change" (existing columns untouched, no data at rest migrated, the caveat reworded, the boot gate still does not act; a populated-table ADD COLUMN refused on both faces; the literal-defaultValue asymmetry) matches the test's cases. patch: a fix bringing the remote face to the declared contract.
  • Gates on the head. ci-failure.mjs --pr 23893: GREEN, 40 of 40. Six skips are on check-expected-skips.mjs's roster; Auto Label and Check PR Size read skipped only on the 11:47 run that this seat's body edit fired, and both read success on the 11:42 run of the same head.
  • Landing checks. check-governed-merges --pr 23893: NOT governed, 353 changed lines. mergeable_state clean; git merge-tree against current origin/main (497518c3f) is clean.

The dev's open question, answered A by this seat (not escalated): keep the shipped behaviour. The remote face does not emit declared column DEFAULTs, so a storage.notNull field with a literal defaultValue added to a populated remote table is refused loudly and names the statement. No in-repo producer declares storage.notNull (the dev's grep over platform-objects, plugins and examples), and the external case is unmeasured. B (one shared default renderer, exported from driver-sql) is the shape to take if a real pull is measured; C (a remote-only default rule) is not taken.

Out of scope, one line each:

Next: ready, then auto-merge through the queue.


Generated by Claude Code

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