Skip to content

Commit 224c55b

Browse files
os-steveclaude
andcommitted
fix(driver-turso): refuse auto_number create on the remote transport (#6944)
WIP checkpoint — refusal + dedicated suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtmF8wiWUuHFcezsXhYygb
1 parent 2f3e793 commit 224c55b

2 files changed

Lines changed: 540 additions & 2 deletions

File tree

‎packages/drivers/driver-turso/src/turso-driver.ts‎

Lines changed: 157 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
*/
2121

2222
import { SqlDriver, type SqlDriverConfig } from '@objectstack/driver-sql';
23+
import { StandardErrorCode } from '@objectstack/spec/api';
2324
import type { DriverQuery } from '@objectstack/spec/contracts';
2425
import type { DriverOptions } from '@objectstack/spec/data';
2526
import type { Client } from '@libsql/client';
@@ -126,6 +127,87 @@ export interface TursoDriverConfig {
126127
client?: Client;
127128
}
128129

130+
// ── Autonumber on the remote face ────────────────────────────────────────────
131+
132+
/**
133+
* [#6944] A record number this face cannot issue — refused, not written as NULL.
134+
*
135+
* # What was measured, and where the knowledge lives
136+
*
137+
* `TursoDriver` picks its transport from `url`. Local and embedded-replica
138+
* inherit `SqlDriver.create`, which calls `fillAutoNumberFields` and issues the
139+
* number from the persistent `_objectstack_sequences` table. Remote overrides
140+
* the write path to `RemoteTransport`, which builds its own `INSERT` and never
141+
* enters that method — so on this face `auto_number` was only a column mapped to
142+
* `TEXT`, and the slot the engine deliberately left empty stayed empty. Measured
143+
* on `main` @ `2f3e79351`, remote, `case_number: { type: 'autonumber' }`:
144+
*
145+
* ```
146+
* create -> RESOLVED case_number=null
147+
* bulkCreate -> RESOLVED [null, null]
148+
* upsert -> RESOLVED case_number=null
149+
* LOCAL create-> RESOLVED case_number="CASE-00001"
150+
* ```
151+
*
152+
* The engine cannot catch this for the caller: `supports.autonumber` is `true`
153+
* here (inherited through `...super.supports`), so `engine.ts` defers generation
154+
* to the driver entirely and never runs its own fallback — see the driver table
155+
* in its `generateAutoNumbers` docblock, which already records `driver-turso` as
156+
* "inherited, no fallback path". A declared capability that boots and quietly
157+
* delivers nothing is the shape #3724 ruled on, and triage applied that ruling
158+
* here (2026-08-09): **disposition B, explicit refusal**. Implementing autonumber
159+
* on this transport (A) stays behind the appetite door for want of measured
160+
* demand — it is deferred, not overlooked.
161+
*
162+
* ⚠️ The refusal is raised HERE, on the driver, and not inside
163+
* `RemoteTransport`, because the transport cannot see what it would need to
164+
* decide: `RemoteTransport.create(object, data)` takes no schema, caches none
165+
* (`syncSchema` reads one and keeps nothing), and so cannot tell an
166+
* `auto_number` column from any other `TEXT` one. The driver can:
167+
* `registerRemoteFieldMetadata` → `SqlDriver.registerExternalObject` classifies
168+
* every field at remote schema-sync time and keys `autoNumberFields` strictly by
169+
* OBJECT name — measured populated in remote mode, tenant field and all. So the
170+
* knowledge exists exactly one layer above the statement builder, and that is
171+
* the layer that refuses.
172+
*
173+
* # Why NOT_IMPLEMENTED / 501 rather than 400
174+
*
175+
* The same two-class taxonomy this package already applies to aggregate
176+
* functions (#5907) and date buckets (#6212), and ADR-0112 for the vocabulary:
177+
* `autonumber` is a field type `@objectstack/spec` declares and this very
178+
* driver's other faces generate, so the caller's object definition is spelled
179+
* correctly and the request contains no mistake. The gap is the backend's.
180+
*
181+
* # What is deliberately NOT refused
182+
*
183+
* A record that ALREADY carries a value in the slot. `fillAutoNumberFields`
184+
* skips exactly those (`undefined` / `null` / `''` is its own generate
185+
* predicate, reused verbatim below rather than re-derived), and on this face
186+
* they are written through unchanged and correctly — measured. That is the
187+
* `isSystem` seed replay and the `preserveAudit` historical import, which
188+
* `engine.ts` exempts from its strip on purpose (#5503); refusing them would
189+
* break a path that works today and would take the two faces further apart, not
190+
* closer (#6203).
191+
*/
192+
function refuseRemoteAutonumber(object: string, fields: string[], path: string): never {
193+
const err = new Error(
194+
`Object "${object}" declares auto_number field(s) [${fields.join(', ')}] left empty for this ` +
195+
`${path}, and the Turso REMOTE transport does not generate record numbers. ` +
196+
`Generated here: none — remote writes go through \`RemoteTransport\`, which builds its own ` +
197+
`INSERT and never enters \`SqlDriver.fillAutoNumberFields\`, so on this face \`auto_number\` ` +
198+
`is only a column mapped to TEXT. The object is spelled correctly and @objectstack/spec ` +
199+
`declares \`autonumber\` as a field type this driver's local and embedded-replica faces do ` +
200+
`generate — this is a capability gap in the remote transport, not a mistake in the request, ` +
201+
`which is why it answers NOT_IMPLEMENTED/501 rather than a 400. Use the local or ` +
202+
`embedded-replica transport for objects that carry record numbers, or supply the value ` +
203+
`explicitly (a seed replay or \`preserveAudit\` import keeps its own numbers and is written ` +
204+
`unchanged). Until this change the same call RESOLVED and wrote NULL into the slot (#6944).`,
205+
) as Error & { code?: string; status?: number };
206+
err.code = StandardErrorCode.enum.NOT_IMPLEMENTED;
207+
err.status = 501;
208+
throw err;
209+
}
210+
129211
// ── Turso Driver ─────────────────────────────────────────────────────────────
130212

131213
/**
@@ -531,8 +613,51 @@ export class TursoDriver extends SqlDriver {
531613
// yielding — the opposite of the memory guarantee it was declared for. This
532614
// override went with the base method; page `find()` with `limit`/`offset`.
533615

616+
/**
617+
* [#6944] Refuse a remote write that would need a record number this face
618+
* cannot issue — see {@link refuseRemoteAutonumber} for the ruling and the
619+
* measurements.
620+
*
621+
* Called BEFORE `toRemoteWriteForms` and before any statement is built, so a
622+
* refused write costs no round trip — the rule every other refusal on this
623+
* path already follows, and the one the refusal suites assert.
624+
*
625+
* The lookup is `autoNumberFields[object]` with no table-name fallback,
626+
* unlike `fillAutoNumberFields`. That is not a shortcut: in remote mode the
627+
* only writer of this registry is `registerRemoteFieldMetadata`, whose own
628+
* docs record that it keys strictly by `object` ("never let a stray
629+
* `schema.name` shadow it"), and the Knex path that produces the other key is
630+
* unreachable here. A second lookup rule would be a second answer.
631+
*
632+
* A no-op for an object with no `auto_number` field, which is the overwhelming
633+
* majority — one map read per remote write.
634+
*/
635+
private refuseUngeneratableRemoteAutonumber(
636+
object: string,
637+
rows: Array<Record<string, any> | null | undefined>,
638+
path: string,
639+
): void {
640+
const cfgs = this.autoNumberFields[object];
641+
if (!cfgs || cfgs.length === 0) return;
642+
const empty = new Set<string>();
643+
for (const row of rows) {
644+
if (!row || typeof row !== 'object') continue;
645+
for (const cfg of cfgs) {
646+
// `fillAutoNumberFields`'s own generate predicate, verbatim: a slot
647+
// holding undefined / null / '' is one the driver would have had to
648+
// fill.
649+
const held = row[cfg.name];
650+
if (held === undefined || held === null || held === '') empty.add(cfg.name);
651+
}
652+
}
653+
if (empty.size > 0) refuseRemoteAutonumber(object, [...empty], path);
654+
}
655+
534656
override async create(object: string, data: Record<string, any>, options?: DriverOptions): Promise<any> {
535-
if (this.isRemote) return this.formatRemoteRow(object, await this.remoteTransport!.create(object, this.toRemoteWriteForms(object, data)));
657+
if (this.isRemote) {
658+
this.refuseUngeneratableRemoteAutonumber(object, [data], 'create');
659+
return this.formatRemoteRow(object, await this.remoteTransport!.create(object, this.toRemoteWriteForms(object, data)));
660+
}
536661
return super.create(object, data, options);
537662
}
538663

@@ -542,7 +667,30 @@ export class TursoDriver extends SqlDriver {
542667
}
543668

544669
override async upsert(object: string, data: Record<string, any>, conflictKeys?: string[], options?: DriverOptions): Promise<Record<string, any>> {
545-
if (this.isRemote) return this.formatRemoteRow(object, await this.remoteTransport!.upsert(object, this.toRemoteWriteForms(object, data), conflictKeys));
670+
if (this.isRemote) {
671+
// [#6944] An upsert is insert-OR-merge, and only the merge leg is safe
672+
// here: `RemoteTransport.upsert` emits
673+
// `INSERT … ON CONFLICT(<keys>) DO UPDATE`, so a row that matches keeps
674+
// the number already in its column and never needed one issued — measured
675+
// (an `id`-bearing upsert onto an existing `CASE-00042` merged and kept
676+
// it). Refusing that would break a path that works.
677+
//
678+
// The insert leg is refused where it is PROVABLE without a round trip: no
679+
// `id`/`_id` and no explicit `conflictKeys` means the transport mints a
680+
// fresh nanoid for the sole default merge key, so the statement can only
681+
// insert — measured, two such upserts produced two rows with different
682+
// ids. That is also the shape the engine sends for a new record.
683+
//
684+
// ⚠️ Residue, stated rather than papered over: an upsert that DOES carry
685+
// an id or conflict keys but matches nothing still inserts, and on that
686+
// leg the slot is still written NULL. Classifying it needs the round trip
687+
// this refusal exists to avoid, so it is left as a known gap of the same
688+
// deferred half (A) rather than answered with a probe query.
689+
const mayMerge = data?.id !== undefined || data?._id !== undefined
690+
|| (Array.isArray(conflictKeys) && conflictKeys.length > 0);
691+
if (!mayMerge) this.refuseUngeneratableRemoteAutonumber(object, [data], 'upsert');
692+
return this.formatRemoteRow(object, await this.remoteTransport!.upsert(object, this.toRemoteWriteForms(object, data), conflictKeys));
693+
}
546694
return super.upsert(object, data, conflictKeys, options);
547695
}
548696

@@ -969,6 +1117,13 @@ export class TursoDriver extends SqlDriver {
9691117

9701118
override async bulkCreate(object: string, data: any[], options?: DriverOptions): Promise<any> {
9711119
if (this.isRemote) {
1120+
// [#6944] Same refusal as `create`, and it has to be stated here rather
1121+
// than inherited: `RemoteTransport.bulkCreate` loops its OWN `create`, not
1122+
// this class's, so nothing about the single-row override reaches this
1123+
// path. Refused for the whole batch before any row is written — the
1124+
// statement is all-or-nothing on this transport too, so a partial batch is
1125+
// not a state this can leave behind.
1126+
this.refuseUngeneratableRemoteAutonumber(object, Array.isArray(data) ? data : [], 'bulkCreate');
9721127
const formatted = Array.isArray(data) ? data.map((d) => this.toRemoteWriteForms(object, d)) : data;
9731128
return this.formatRemoteRows(object, await this.remoteTransport!.bulkCreate(object, formatted));
9741129
}

0 commit comments

Comments
 (0)