diff --git a/.changeset/18063-transport-declares-no-transactions.md b/.changeset/18063-transport-declares-no-transactions.md new file mode 100644 index 00000000000..6d11a31ea7e --- /dev/null +++ b/.changeset/18063-transport-declares-no-transactions.md @@ -0,0 +1,29 @@ +--- +"@objectstack/spec": minor +"@objectstack/core": minor +"@objectstack/objectql": minor +"@objectstack/driver-sql": minor +"@objectstack/driver-turso": minor +--- + +feat(spec,core,objectql,driver-sql,driver-turso): a transport can declare it has no transactions, and every transaction gate reads the declaration instead of method presence (#18063) + +Maintainer ruling, decision batch #148 item 3, letter B, 「同意」 2026-09-17, verbatim and untranslated: + +> `packages/spec`: the driver contract gains a way for a transport to **declare 「no transactions」** (the dev picks the smallest spelling the existing capability/contract surface already has — a capability bit is preferred over a new key), and the engine's transaction gating reads the declaration instead of method presence. + +**`DriverCapabilities` gains one live bit, `transactionsUnsupported`.** A transport sets it to say that a handle it issued would be a FALSE SUCCESS rather than a missing feature: the caller gets a handle, the writes execute and are already durable, `rollback()` resolves and undoes nothing. Absence means `false`, exactly like `batchSchemaSync`, so a driver that declares nothing keeps the behaviour it has today. + +**⛔ This is not `DriverCapabilities.transactions` un-retired, and the difference is not cosmetic.** That key was tombstoned in 17.0.0 under ADR-0049 enforce-or-remove and STAYS tombstoned — writing it is still a compile error and still a parse refusal carrying its prescription. It claimed "I support transactions" and nothing read it; this one declares "my transport cannot honour one" and the engine dispatches on it. Reviving the name would have inverted the record's own `absence = false` convention into a tri-state, turned a documented refusal into silent acceptance of a value whose meaning had changed underneath it, and made the tombstone's published text ("no code in any repository ever read it") false. A new key costs one bit; the name costs all of that. + +**Adding a bit to a record enforce-or-remove has pruned SATISFIES that ADR rather than reversing it.** The audit removed thirty-one bits for one stated reason — no code anywhere read them — and kept the three where method presence provably cannot carry the signal. This change is the creation of the missing reader: `driverSupportsTransactions()` (exported from `@objectstack/spec`) is the one definition of the gate, and all FOUR places that used to spell `typeof driver.beginTransaction === 'function'` ask it — `ObjectQL.transaction()`, `ScopedContext.transaction`, the `ScopedContext` begin/commit/rollback trio, and `@objectstack/core`'s `engineCanRollBack`. The bit arrives WITH its reader, in the same change, which is the honest order the ADR asks for. + +**Why method presence could not carry it.** `TursoDriver extends SqlDriver`, whose `beginTransaction()` opens a real knex transaction, so the inherited method reported the libSQL REMOTE transport as transactional. It is not — `RemoteTransport`'s data methods take no `options` argument at all, so a handle cannot reach the statement that would have to join it. A subclass cannot opt out of a door it did not open. This is the mirror of `batchSchemaSync`, which exists because a subclass can inherit `syncSchemasBatch` from a base whose transport batches while its own cannot. + +**What changes for a caller.** On a datasource whose driver declares the bit, `engine.transaction()` now takes the DECLARED non-transactional path (ADR-0119 D1) instead of opening a transaction it cannot honour: the degrade warns once per datasource — naming the declaration, not a missing method — and `{ require: true }` throws `TransactionUnsupportedError` before the callback writes anything. `ScopedContext.transaction` and the discrete begin/commit/rollback trio read the same predicate; the trio's `begin` returns `null`. Both are the answers a driver with no `beginTransaction` already received. + +**`driver-turso`.** The remote face declares `transactionsUnsupported: true`; local and embedded-replica inherit `false` from the base and are untouched. `TursoDriver.beginTransaction()` publishes the inherited declaration instead of `Promise` — the annotation the earlier `any` was masking an LSP violation to avoid, dissolved rather than widened: the remote arm returns `never` (it refuses), so the only arm that still returns is the base's. `SqlDriver.beginTransaction()` keeps its narrow `Promise`; nothing in the base was widened. + +**`@objectstack/core`.** `engineCanRollBack()` — the ADR-0119 D4 gate that `@objectstack/metadata-protocol` uses for `batchData` / `updateManyData` / `deleteManyData` under `options.atomic`, and that `runMigrationJournal()` uses to decide whether to start at all — reads the same predicate. It has to: it does not open the transaction itself, it vouches that `engine.transaction()` will, and on a driver that declares the bit the engine now takes its non-transactional path. A gate still reading method presence would vouch for a runtime that is about to run the callback with no transaction, so the atomic batch would answer `rollback` over writes that stayed on disk and the journal would write `chunk_done` rows its own contract says mean "committed". What a caller sees on such a datasource instead: `batchData({ atomic: true })` refuses with `501 NOT_IMPLEMENTED` — retry without `atomic`, or probe `capabilities.transactionalBatch` on `/discovery` first — and `runMigrationJournal()` refuses with `MigrationJournalRefusal('NOT_IMPLEMENTED')` before writing a single journal row. Both are the answers a driver with no `beginTransaction` already received. + +**`RemoteTransport` loses `beginTransaction()`, `commit()` and `rollback()`.** They are a published surface, and this is **minor** rather than major on the ruling's own stated ground: that transport never honoured a transaction, so no working behaviour is withdrawn. They had already become unreachable from every caller in the repository when the driver started refusing them; they are now gone, and the declaration keeps them gone by design rather than by audit. diff --git a/content/docs/references/data/driver-nosql.mdx b/content/docs/references/data/driver-nosql.mdx index b31d7ba7e9b..fb4bb436bba 100644 --- a/content/docs/references/data/driver-nosql.mdx +++ b/content/docs/references/data/driver-nosql.mdx @@ -140,7 +140,7 @@ const result = AggregationPipelineSchema.parse(data); | :--- | :--- | :--- | :--- | | **name** | `string` | ✅ | Driver instance name | | **type** | `'nosql'` | ✅ | Driver type must be "nosql" | -| **capabilities** | `{ queryDateGranularity?: Record; autonumber?: boolean; batchSchemaSync?: boolean }` | ✅ | Driver capability flags | +| **capabilities** | `{ queryDateGranularity?: Record; autonumber?: boolean; batchSchemaSync?: boolean; transactionsUnsupported?: boolean }` | ✅ | Driver capability flags | | **connectionString** | `string` | optional | Database connection string (driver-specific format) | | **poolConfig** | `{ min: number; max: number; idleTimeoutMillis: number; connectionTimeoutMillis: number }` | optional | Connection pool configuration | | **databaseType** | `Enum<'mongodb' \| 'couchdb' \| 'dynamodb' \| 'cassandra' \| 'redis' \| 'elasticsearch' \| 'neo4j' \| 'orientdb'>` | ✅ | Specific NoSQL database type | @@ -163,6 +163,7 @@ const result = AggregationPipelineSchema.parse(data); | **queryDateGranularity** | `Record` | optional | Per-granularity native date bucketing (day/week/month/quarter/year). Missing keys fall back to in-memory bucketing. | | **autonumber** | `boolean` | optional | Driver natively generates persistent autonumber/sequence values | | **batchSchemaSync** | `boolean` | optional | Supports batched schema sync to reduce schema DDL round-trips (absence = false) | +| **transactionsUnsupported** | `boolean` | optional | Transport cannot honour transactions even though `beginTransaction` is inherited (absence = false) | | **create** | `never` | optional | [REMOVED] `DriverCapabilities.create` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. CRUD is not optional for a driver: `create`/`find`/`findOne`/`update`/`delete` are REQUIRED `IDataDriver` methods and the engine calls them unconditionally. Delete the key. | | **read** | `never` | optional | [REMOVED] `DriverCapabilities.read` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. CRUD is not optional for a driver: reads go through the REQUIRED `find`/`findOne`/`count` methods, called unconditionally. Delete the key. | | **update** | `never` | optional | [REMOVED] `DriverCapabilities.update` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. CRUD is not optional for a driver: `update`/`upsert` are REQUIRED `IDataDriver` methods, called unconditionally. Delete the key. | @@ -170,7 +171,7 @@ const result = AggregationPipelineSchema.parse(data); | **bulkCreate** | `never` | optional | [REMOVED] `DriverCapabilities.bulkCreate` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. The bulk methods (`bulkCreate`/`bulkUpdate`/`bulkDelete`) are REQUIRED `IDataDriver` methods and the engine calls them directly; wire-level batch capability is advertised by REST discovery from the live composition (#3298), never from this record. Delete the key. | | **bulkUpdate** | `never` | optional | [REMOVED] `DriverCapabilities.bulkUpdate` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. The bulk methods are REQUIRED `IDataDriver` methods and the engine calls them directly; wire-level batch capability is advertised by REST discovery from the live composition (#3298), never from this record. Delete the key. | | **bulkDelete** | `never` | optional | [REMOVED] `DriverCapabilities.bulkDelete` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. The bulk methods are REQUIRED `IDataDriver` methods and the engine calls them directly; wire-level batch capability is advertised by REST discovery from the live composition (#3298), never from this record. Delete the key. | -| **transactions** | `never` | optional | [REMOVED] `DriverCapabilities.transactions` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. Transaction use is gated on METHOD PRESENCE — `driver.beginTransaction` (`engine.transaction()`, ADR-0034 ambient transactions): a driver without the method gets the non-transactional fallback, whatever this bit claimed. Discovery's `transactionalBatch` capability is likewise derived from `engine.transaction` plus the mounted batch route, never from this bit. Delete the key. | +| **transactions** | `never` | optional | [REMOVED] `DriverCapabilities.transactions` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. Transaction use is gated on the DRIVER'S DECLARATION, no longer on METHOD PRESENCE alone: `engine.transaction()` asks `driverSupportsTransactions(driver)` — `driver.beginTransaction` present AND `transactionsUnsupported` not set (ADR-0034 ambient transactions, ADR-0119 D1). A driver without the method — or a transport that declares that live bit — gets the non-transactional fallback, whatever this bit claimed. Discovery's `transactionalBatch` capability is likewise derived from `engine.transaction` plus the mounted batch route, never from this bit. The live `transactionsUnsupported` bit is NOT this key restored and is not its opposite spelled differently: this one CLAIMED support nothing checked, that one DENIES support the engine does check, and it is written only by a transport that inherits `beginTransaction` from a base class it cannot honour. A driver with real transactions declares nothing. Delete the key. | | **savepoints** | `never` | optional | [REMOVED] `DriverCapabilities.savepoints` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. No savepoint code path exists in the engine — a capability bit for a feature the platform does not call is a false affordance, not documentation. Delete the key. | | **isolationLevels** | `never` | optional | [REMOVED] `DriverCapabilities.isolationLevels` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. Isolation is requested per transaction via `beginTransaction({ isolationLevel })`; no planner ever consulted this list to decide anything. Delete the key. | | **queryFilters** | `never` | optional | [REMOVED] `DriverCapabilities.queryFilters` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. `find()` receives the full QueryAST (`where`/`orderBy`/`limit`/`offset`) and MUST execute all of it — the "ObjectQL will filter in memory" fallback this bit's description promised was never built. Delete the key. | diff --git a/content/docs/references/data/driver-sql.mdx b/content/docs/references/data/driver-sql.mdx index 8951ce22696..cb966d506be 100644 --- a/content/docs/references/data/driver-sql.mdx +++ b/content/docs/references/data/driver-sql.mdx @@ -61,7 +61,7 @@ const result = DataTypeMappingSchema.parse(data); | :--- | :--- | :--- | :--- | | **name** | `string` | ✅ | Driver instance name | | **type** | `'sql'` | ✅ | Driver type must be "sql" | -| **capabilities** | `{ queryDateGranularity?: Record; autonumber?: boolean; batchSchemaSync?: boolean }` | ✅ | Driver capability flags | +| **capabilities** | `{ queryDateGranularity?: Record; autonumber?: boolean; batchSchemaSync?: boolean; transactionsUnsupported?: boolean }` | ✅ | Driver capability flags | | **connectionString** | `string` | optional | Database connection string (driver-specific format) | | **poolConfig** | `{ min: number; max: number; idleTimeoutMillis: number; connectionTimeoutMillis: number }` | optional | Connection pool configuration | | **dialect** | `Enum<'postgresql' \| 'mysql' \| 'sqlite' \| 'mssql' \| 'oracle' \| 'mariadb'>` | ✅ | SQL database dialect | @@ -76,6 +76,7 @@ const result = DataTypeMappingSchema.parse(data); | **queryDateGranularity** | `Record` | optional | Per-granularity native date bucketing (day/week/month/quarter/year). Missing keys fall back to in-memory bucketing. | | **autonumber** | `boolean` | optional | Driver natively generates persistent autonumber/sequence values | | **batchSchemaSync** | `boolean` | optional | Supports batched schema sync to reduce schema DDL round-trips (absence = false) | +| **transactionsUnsupported** | `boolean` | optional | Transport cannot honour transactions even though `beginTransaction` is inherited (absence = false) | | **create** | `never` | optional | [REMOVED] `DriverCapabilities.create` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. CRUD is not optional for a driver: `create`/`find`/`findOne`/`update`/`delete` are REQUIRED `IDataDriver` methods and the engine calls them unconditionally. Delete the key. | | **read** | `never` | optional | [REMOVED] `DriverCapabilities.read` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. CRUD is not optional for a driver: reads go through the REQUIRED `find`/`findOne`/`count` methods, called unconditionally. Delete the key. | | **update** | `never` | optional | [REMOVED] `DriverCapabilities.update` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. CRUD is not optional for a driver: `update`/`upsert` are REQUIRED `IDataDriver` methods, called unconditionally. Delete the key. | @@ -83,7 +84,7 @@ const result = DataTypeMappingSchema.parse(data); | **bulkCreate** | `never` | optional | [REMOVED] `DriverCapabilities.bulkCreate` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. The bulk methods (`bulkCreate`/`bulkUpdate`/`bulkDelete`) are REQUIRED `IDataDriver` methods and the engine calls them directly; wire-level batch capability is advertised by REST discovery from the live composition (#3298), never from this record. Delete the key. | | **bulkUpdate** | `never` | optional | [REMOVED] `DriverCapabilities.bulkUpdate` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. The bulk methods are REQUIRED `IDataDriver` methods and the engine calls them directly; wire-level batch capability is advertised by REST discovery from the live composition (#3298), never from this record. Delete the key. | | **bulkDelete** | `never` | optional | [REMOVED] `DriverCapabilities.bulkDelete` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. The bulk methods are REQUIRED `IDataDriver` methods and the engine calls them directly; wire-level batch capability is advertised by REST discovery from the live composition (#3298), never from this record. Delete the key. | -| **transactions** | `never` | optional | [REMOVED] `DriverCapabilities.transactions` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. Transaction use is gated on METHOD PRESENCE — `driver.beginTransaction` (`engine.transaction()`, ADR-0034 ambient transactions): a driver without the method gets the non-transactional fallback, whatever this bit claimed. Discovery's `transactionalBatch` capability is likewise derived from `engine.transaction` plus the mounted batch route, never from this bit. Delete the key. | +| **transactions** | `never` | optional | [REMOVED] `DriverCapabilities.transactions` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. Transaction use is gated on the DRIVER'S DECLARATION, no longer on METHOD PRESENCE alone: `engine.transaction()` asks `driverSupportsTransactions(driver)` — `driver.beginTransaction` present AND `transactionsUnsupported` not set (ADR-0034 ambient transactions, ADR-0119 D1). A driver without the method — or a transport that declares that live bit — gets the non-transactional fallback, whatever this bit claimed. Discovery's `transactionalBatch` capability is likewise derived from `engine.transaction` plus the mounted batch route, never from this bit. The live `transactionsUnsupported` bit is NOT this key restored and is not its opposite spelled differently: this one CLAIMED support nothing checked, that one DENIES support the engine does check, and it is written only by a transport that inherits `beginTransaction` from a base class it cannot honour. A driver with real transactions declares nothing. Delete the key. | | **savepoints** | `never` | optional | [REMOVED] `DriverCapabilities.savepoints` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. No savepoint code path exists in the engine — a capability bit for a feature the platform does not call is a false affordance, not documentation. Delete the key. | | **isolationLevels** | `never` | optional | [REMOVED] `DriverCapabilities.isolationLevels` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. Isolation is requested per transaction via `beginTransaction({ isolationLevel })`; no planner ever consulted this list to decide anything. Delete the key. | | **queryFilters** | `never` | optional | [REMOVED] `DriverCapabilities.queryFilters` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. `find()` receives the full QueryAST (`where`/`orderBy`/`limit`/`offset`) and MUST execute all of it — the "ObjectQL will filter in memory" fallback this bit's description promised was never built. Delete the key. | diff --git a/content/docs/references/data/driver.mdx b/content/docs/references/data/driver.mdx index c2bbe00bd03..70d7a73f770 100644 --- a/content/docs/references/data/driver.mdx +++ b/content/docs/references/data/driver.mdx @@ -30,6 +30,7 @@ const result = DriverCapabilitiesSchema.parse(data); | **queryDateGranularity** | `Record` | optional | Per-granularity native date bucketing (day/week/month/quarter/year). Missing keys fall back to in-memory bucketing. | | **autonumber** | `boolean` | optional | Driver natively generates persistent autonumber/sequence values | | **batchSchemaSync** | `boolean` | optional | Supports batched schema sync to reduce schema DDL round-trips (absence = false) | +| **transactionsUnsupported** | `boolean` | optional | Transport cannot honour transactions even though `beginTransaction` is inherited (absence = false) | | **create** | `never` | optional | [REMOVED] `DriverCapabilities.create` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. CRUD is not optional for a driver: `create`/`find`/`findOne`/`update`/`delete` are REQUIRED `IDataDriver` methods and the engine calls them unconditionally. Delete the key. | | **read** | `never` | optional | [REMOVED] `DriverCapabilities.read` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. CRUD is not optional for a driver: reads go through the REQUIRED `find`/`findOne`/`count` methods, called unconditionally. Delete the key. | | **update** | `never` | optional | [REMOVED] `DriverCapabilities.update` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. CRUD is not optional for a driver: `update`/`upsert` are REQUIRED `IDataDriver` methods, called unconditionally. Delete the key. | @@ -37,7 +38,7 @@ const result = DriverCapabilitiesSchema.parse(data); | **bulkCreate** | `never` | optional | [REMOVED] `DriverCapabilities.bulkCreate` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. The bulk methods (`bulkCreate`/`bulkUpdate`/`bulkDelete`) are REQUIRED `IDataDriver` methods and the engine calls them directly; wire-level batch capability is advertised by REST discovery from the live composition (#3298), never from this record. Delete the key. | | **bulkUpdate** | `never` | optional | [REMOVED] `DriverCapabilities.bulkUpdate` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. The bulk methods are REQUIRED `IDataDriver` methods and the engine calls them directly; wire-level batch capability is advertised by REST discovery from the live composition (#3298), never from this record. Delete the key. | | **bulkDelete** | `never` | optional | [REMOVED] `DriverCapabilities.bulkDelete` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. The bulk methods are REQUIRED `IDataDriver` methods and the engine calls them directly; wire-level batch capability is advertised by REST discovery from the live composition (#3298), never from this record. Delete the key. | -| **transactions** | `never` | optional | [REMOVED] `DriverCapabilities.transactions` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. Transaction use is gated on METHOD PRESENCE — `driver.beginTransaction` (`engine.transaction()`, ADR-0034 ambient transactions): a driver without the method gets the non-transactional fallback, whatever this bit claimed. Discovery's `transactionalBatch` capability is likewise derived from `engine.transaction` plus the mounted batch route, never from this bit. Delete the key. | +| **transactions** | `never` | optional | [REMOVED] `DriverCapabilities.transactions` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. Transaction use is gated on the DRIVER'S DECLARATION, no longer on METHOD PRESENCE alone: `engine.transaction()` asks `driverSupportsTransactions(driver)` — `driver.beginTransaction` present AND `transactionsUnsupported` not set (ADR-0034 ambient transactions, ADR-0119 D1). A driver without the method — or a transport that declares that live bit — gets the non-transactional fallback, whatever this bit claimed. Discovery's `transactionalBatch` capability is likewise derived from `engine.transaction` plus the mounted batch route, never from this bit. The live `transactionsUnsupported` bit is NOT this key restored and is not its opposite spelled differently: this one CLAIMED support nothing checked, that one DENIES support the engine does check, and it is written only by a transport that inherits `beginTransaction` from a base class it cannot honour. A driver with real transactions declares nothing. Delete the key. | | **savepoints** | `never` | optional | [REMOVED] `DriverCapabilities.savepoints` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. No savepoint code path exists in the engine — a capability bit for a feature the platform does not call is a false affordance, not documentation. Delete the key. | | **isolationLevels** | `never` | optional | [REMOVED] `DriverCapabilities.isolationLevels` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. Isolation is requested per transaction via `beginTransaction({ isolationLevel })`; no planner ever consulted this list to decide anything. Delete the key. | | **queryFilters** | `never` | optional | [REMOVED] `DriverCapabilities.queryFilters` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. `find()` receives the full QueryAST (`where`/`orderBy`/`limit`/`offset`) and MUST execute all of it — the "ObjectQL will filter in memory" fallback this bit's description promised was never built. Delete the key. | @@ -73,7 +74,7 @@ const result = DriverCapabilitiesSchema.parse(data); | :--- | :--- | :--- | :--- | | **name** | `string` | ✅ | Driver instance name | | **type** | `Enum<'sql' \| 'nosql' \| 'cache' \| 'search' \| 'graph' \| 'timeseries'>` | ✅ | Driver type category | -| **capabilities** | `{ queryDateGranularity?: Record; autonumber?: boolean; batchSchemaSync?: boolean }` | ✅ | Driver capability flags | +| **capabilities** | `{ queryDateGranularity?: Record; autonumber?: boolean; batchSchemaSync?: boolean; transactionsUnsupported?: boolean }` | ✅ | Driver capability flags | | **connectionString** | `string` | optional | Database connection string (driver-specific format) | | **poolConfig** | `{ min: number; max: number; idleTimeoutMillis: number; connectionTimeoutMillis: number }` | optional | Connection pool configuration | @@ -84,6 +85,7 @@ const result = DriverCapabilitiesSchema.parse(data); | **queryDateGranularity** | `Record` | optional | Per-granularity native date bucketing (day/week/month/quarter/year). Missing keys fall back to in-memory bucketing. | | **autonumber** | `boolean` | optional | Driver natively generates persistent autonumber/sequence values | | **batchSchemaSync** | `boolean` | optional | Supports batched schema sync to reduce schema DDL round-trips (absence = false) | +| **transactionsUnsupported** | `boolean` | optional | Transport cannot honour transactions even though `beginTransaction` is inherited (absence = false) | | **create** | `never` | optional | [REMOVED] `DriverCapabilities.create` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. CRUD is not optional for a driver: `create`/`find`/`findOne`/`update`/`delete` are REQUIRED `IDataDriver` methods and the engine calls them unconditionally. Delete the key. | | **read** | `never` | optional | [REMOVED] `DriverCapabilities.read` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. CRUD is not optional for a driver: reads go through the REQUIRED `find`/`findOne`/`count` methods, called unconditionally. Delete the key. | | **update** | `never` | optional | [REMOVED] `DriverCapabilities.update` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. CRUD is not optional for a driver: `update`/`upsert` are REQUIRED `IDataDriver` methods, called unconditionally. Delete the key. | @@ -91,7 +93,7 @@ const result = DriverCapabilitiesSchema.parse(data); | **bulkCreate** | `never` | optional | [REMOVED] `DriverCapabilities.bulkCreate` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. The bulk methods (`bulkCreate`/`bulkUpdate`/`bulkDelete`) are REQUIRED `IDataDriver` methods and the engine calls them directly; wire-level batch capability is advertised by REST discovery from the live composition (#3298), never from this record. Delete the key. | | **bulkUpdate** | `never` | optional | [REMOVED] `DriverCapabilities.bulkUpdate` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. The bulk methods are REQUIRED `IDataDriver` methods and the engine calls them directly; wire-level batch capability is advertised by REST discovery from the live composition (#3298), never from this record. Delete the key. | | **bulkDelete** | `never` | optional | [REMOVED] `DriverCapabilities.bulkDelete` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. The bulk methods are REQUIRED `IDataDriver` methods and the engine calls them directly; wire-level batch capability is advertised by REST discovery from the live composition (#3298), never from this record. Delete the key. | -| **transactions** | `never` | optional | [REMOVED] `DriverCapabilities.transactions` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. Transaction use is gated on METHOD PRESENCE — `driver.beginTransaction` (`engine.transaction()`, ADR-0034 ambient transactions): a driver without the method gets the non-transactional fallback, whatever this bit claimed. Discovery's `transactionalBatch` capability is likewise derived from `engine.transaction` plus the mounted batch route, never from this bit. Delete the key. | +| **transactions** | `never` | optional | [REMOVED] `DriverCapabilities.transactions` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. Transaction use is gated on the DRIVER'S DECLARATION, no longer on METHOD PRESENCE alone: `engine.transaction()` asks `driverSupportsTransactions(driver)` — `driver.beginTransaction` present AND `transactionsUnsupported` not set (ADR-0034 ambient transactions, ADR-0119 D1). A driver without the method — or a transport that declares that live bit — gets the non-transactional fallback, whatever this bit claimed. Discovery's `transactionalBatch` capability is likewise derived from `engine.transaction` plus the mounted batch route, never from this bit. The live `transactionsUnsupported` bit is NOT this key restored and is not its opposite spelled differently: this one CLAIMED support nothing checked, that one DENIES support the engine does check, and it is written only by a transport that inherits `beginTransaction` from a base class it cannot honour. A driver with real transactions declares nothing. Delete the key. | | **savepoints** | `never` | optional | [REMOVED] `DriverCapabilities.savepoints` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. No savepoint code path exists in the engine — a capability bit for a feature the platform does not call is a false affordance, not documentation. Delete the key. | | **isolationLevels** | `never` | optional | [REMOVED] `DriverCapabilities.isolationLevels` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. Isolation is requested per transaction via `beginTransaction({ isolationLevel })`; no planner ever consulted this list to decide anything. Delete the key. | | **queryFilters** | `never` | optional | [REMOVED] `DriverCapabilities.queryFilters` was removed in @objectstack/spec 17.0.0 (ADR-0049 enforce-or-remove) — no code in any repository ever read it, so its value never changed which code path ran. `find()` receives the full QueryAST (`where`/`orderBy`/`limit`/`offset`) and MUST execute all of it — the "ObjectQL will filter in memory" fallback this bit's description promised was never built. Delete the key. | diff --git a/packages/core/src/utils/migration-journal.test.ts b/packages/core/src/utils/migration-journal.test.ts index 6165b045a76..3e115809106 100644 --- a/packages/core/src/utils/migration-journal.test.ts +++ b/packages/core/src/utils/migration-journal.test.ts @@ -55,9 +55,21 @@ class FakeEngine { /** Every context object handed to `insert`, so tests can prove tx binding. */ insertContexts: unknown[] = []; private driverHasTx: boolean; - - constructor(opts: { driverHasTx?: boolean } = {}) { + /** + * [#18063] What the transport DECLARES about a handle it would issue, held + * apart from whether it publishes `beginTransaction` at all. + * + * `undefined` — the default and every pre-existing case — means the driver + * carries no opinion, which is the shape this double could ONLY produce + * before: its driver had no `supports` record, so a gate reading method + * presence and a gate reading the declaration were indistinguishable here and + * the pin below was green against both. + */ + private driverDeclaresUnsupported: boolean | undefined; + + constructor(opts: { driverHasTx?: boolean; driverDeclaresUnsupported?: boolean } = {}) { this.driverHasTx = opts.driverHasTx ?? true; + this.driverDeclaresUnsupported = opts.driverDeclaresUnsupported; } private rows(name: string): FakeRow[] { @@ -111,7 +123,17 @@ class FakeEngine { getObject(name: string): unknown { return { name }; } getDefaultDriverName(): string { return 'fake'; } getDriverByName(): unknown { - return this.driverHasTx ? { beginTransaction: () => {}, commit: () => {}, rollback: () => {} } : {}; + if (!this.driverHasTx) return {}; + // [#18063] `beginTransaction` is published on every column — the inherited + // door. Only `supports` differs. + return { + beginTransaction: () => {}, + commit: () => {}, + rollback: () => {}, + supports: this.driverDeclaresUnsupported === undefined + ? {} + : { transactionsUnsupported: this.driverDeclaresUnsupported }, + }; } async transaction(cb: (trxCtx: unknown) => Promise, baseContext?: unknown): Promise { @@ -183,6 +205,46 @@ describe('capability gate (ADR-0119 D2 item 7 / D4 probe)', () => { // A test double with no driver registry keeps the engine-level answer. expect(engineCanRollBack({ transaction: () => {} })).toBe(true); }); + + it('[#18063] the driver clause reads the DECLARATION, not method presence', () => { + // The shape neither column above can produce: `beginTransaction` present — + // INHERITED from a base class whose transport has transactions — on a + // transport that declared it cannot honour the handle. The engine takes its + // declared non-transactional path for exactly this driver, so a gate + // answering `true` here hands both callers a runtime that runs their + // callback with NO transaction: `batchData`'s atomic arm then reports a + // rollback that undid nothing, and the runner writes `chunk_done` rows its + // own header says would not mean committed. + expect(engineCanRollBack(new FakeEngine({ driverDeclaresUnsupported: true }))).toBe(false); + // LIT controls — same double, same published `beginTransaction`, the bit + // absent and the bit explicitly `false`. Both keep the transactional + // answer, so the `false` above is the declaration and not a double that + // stopped answering. + expect(engineCanRollBack(new FakeEngine({ driverDeclaresUnsupported: false }))).toBe(true); + expect(engineCanRollBack(new FakeEngine())).toBe(true); + }); + + it('[#18063] refuses to start when the transport DECLARED it cannot honour a handle', async () => { + const engine = new FakeEngine({ driverDeclaresUnsupported: true }); + const plan: MigrationPlan = { id: 'p', steps: [makeStep(3)] }; + const refusal = await runMigrationJournal(asEngine(engine), plan).then( + () => null, + (err: unknown) => err, + ); + expect(refusal).toBeInstanceOf(MigrationJournalRefusal); + expect((refusal as MigrationJournalRefusal).code).toBe('NOT_IMPLEMENTED'); + // Refused means NOTHING was written — the same bar the missing-method + // column above is held to. + expect(engine.tables.get(JOURNAL) ?? []).toHaveLength(0); + + // LIT control: the same double with the bit removed runs to completion, so + // the refusal is the declaration rather than an inert fixture. + const control = new FakeEngine(); + await expect( + runMigrationJournal(asEngine(control), { id: 'p', steps: [makeStep(3)] }), + ).resolves.toMatchObject({ status: 'completed' }); + expect(kindsOf(control)).toContain('run_done'); + }); }); // ── preflight ───────────────────────────────────────────────────────────── diff --git a/packages/core/src/utils/migration-journal.ts b/packages/core/src/utils/migration-journal.ts index 1e534e81e2c..df8d8d5009b 100644 --- a/packages/core/src/utils/migration-journal.ts +++ b/packages/core/src/utils/migration-journal.ts @@ -53,6 +53,9 @@ import { createHash, randomUUID } from 'node:crypto'; import type { IObjectQLEngine } from '@objectstack/spec/contracts'; +// [#18063] The driver clause is the DECLARATION, not bare method presence, and +// it is the same one definition the engine's own transaction entrances call. +import { driverSupportsTransactions } from '@objectstack/spec/data'; import { MIGRATION_JOURNAL_OBJECT, type MigrationJournalEvent, @@ -76,11 +79,23 @@ const DEFAULT_CHUNK_SIZE = 200; * clause and leaves one caller believing it has atomicity it does not have. * * TWO levels, both necessary. `engine.transaction()` exists but runs the - * callback with NO transaction and NO rollback when the default driver lacks - * `beginTransaction` — a declared caveat of the contract member (ADR-0119 D1), - * and one that turns "atomic" back into a lie precisely where it matters. So - * where the driver registry is inspectable the driver is checked too; where it - * is not (test doubles), the engine-level probe is all there is. + * callback with NO transaction and NO rollback when the default driver cannot + * carry one — a declared caveat of the contract member (ADR-0119 D1), and one + * that turns "atomic" back into a lie precisely where it matters. So where the + * driver registry is inspectable the driver is checked too; where it is not + * (test doubles), the engine-level probe is all there is. + * + * ⛔ The driver clause asks `driverSupportsTransactions` (`@objectstack/spec`) + * and NOT `typeof driver.beginTransaction === 'function'` (#18063). The two + * answer differently for a transport that INHERITED the method from a base + * class it cannot honour and declared `supports.transactionsUnsupported`: the + * engine takes its DECLARED non-transactional path for such a driver, so a + * presence test here would say "this runtime can roll back" about a runtime + * that is about to run the callback with no transaction at all — the + * `chunk_done` rows below, and `batchData`'s `atomic` response, would then + * report a rollback that undid nothing. Both gates read one predicate for the + * same reason the two callers read one helper: a gate that disagrees with the + * dispatch it guards is worse than no gate. * * A type predicate, not a bare boolean: every caller's next move is to CALL * `transaction`, and on the host surfaces that declare it optionally @@ -96,7 +111,15 @@ export function engineCanRollBack(engine: T): engine is T & EngineWithTransac if (typeof e?.transaction !== 'function') return false; const defaultDriverName = e.getDefaultDriverName?.(); const defaultDriver = defaultDriverName ? e.getDriverByName?.(defaultDriverName) : undefined; - return !defaultDriver || typeof (defaultDriver as { beginTransaction?: unknown }).beginTransaction === 'function'; + return ( + !defaultDriver + || driverSupportsTransactions( + defaultDriver as { + beginTransaction?: unknown; + supports?: { transactionsUnsupported?: boolean | undefined } | undefined; + }, + ) + ); } /** diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 2a17fc14ab7..22b0d5614cc 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -4770,6 +4770,13 @@ export class SqlDriver implements IDataDriver { // Subclasses whose transport batches (Turso) implement the method AND // flip this bit — the engine requires both. batchSchemaSync: false, + // [#18063] knex transactions are real here, so this stays false — spelled + // rather than left absent for the same reason `batchSchemaSync` is: this + // literal is the baseline subclasses SPREAD, and the bit is the only way + // a subclass whose transport cannot carry a handle (TursoDriver's remote + // mode) opts out of the `beginTransaction()` it inherits from this class. + // Absence would mean the same thing and say nothing. + transactionsUnsupported: false, }; } diff --git a/packages/drivers/driver-turso/src/remote-transport.ts b/packages/drivers/driver-turso/src/remote-transport.ts index 4eae72c4cd8..1dbfa99650e 100644 --- a/packages/drivers/driver-turso/src/remote-transport.ts +++ b/packages/drivers/driver-turso/src/remote-transport.ts @@ -1834,26 +1834,31 @@ export class RemoteTransport { } // =================================== - // Transactions + // Transactions — none. Deliberately. // =================================== - - // [#17690] The contract's own type. It was `Promise` — the one door of - // this family on `RemoteTransport`, whose `find`/`upsert`/`bulkUpdate` were - // already honest and are the counter-control showing the census that found - // this discriminates rather than flagging everything. Pinned both halves in - // `turso-driver-doors-declared-types.test.ts`. - async beginTransaction(): Promise { - await this.ensureConnected(); - return this.client!.transaction(); - } - - async commit(transaction: any): Promise { - await transaction.commit(); - } - - async rollback(transaction: any): Promise { - await transaction.rollback(); - } + // + // ⛔ [#18063] This transport declares NO transaction members, and re-adding + // one is the defect, not the fix. + // + // It carried three — `beginTransaction()`, `commit(t)`, `rollback(t)` — and + // they were decorative from the day they were written: not one data method on + // this class takes an `options` argument (9 data methods present, 0 with + // `options`), so a handle this transport issued could never reach a statement + // built on it. A write between `beginTransaction()` and `rollback()` executed + // on the plain connection and was ALREADY DURABLE; the rollback resolved and + // undid nothing, reporting success at every step. + // + // [#18616] closed the paths into them — `TursoDriver` refuses + // `beginTransaction()` / `commit()` / `rollback()` and any `options.transaction` + // on the remote arm — which left these three unreachable from every caller in + // the repository. [#18063] removes them, and adds the declaration that keeps + // them unreachable by design rather than by audit: + // `TursoDriver.supports.transactionsUnsupported` is true on this arm, and the + // engine gates the transactional path on that declaration. + // + // Implementing transactions here is a separate piece of work and a much + // larger one: it needs every data method to accept and thread a handle, which + // means a libSQL transport that can carry one at all. // =================================== // Schema Management diff --git a/packages/drivers/driver-turso/src/turso-driver-doors-declared-types.test.ts b/packages/drivers/driver-turso/src/turso-driver-doors-declared-types.test.ts index db8e2323600..eebc0b220e7 100644 --- a/packages/drivers/driver-turso/src/turso-driver-doors-declared-types.test.ts +++ b/packages/drivers/driver-turso/src/turso-driver-doors-declared-types.test.ts @@ -59,36 +59,44 @@ // `TursoDriver`. // // [#17690] Three more overridden doors join the driver half — `find`, -// `upsert` and `bulkUpdate` — plus `RemoteTransport.beginTransaction`, which -// lives in this package and in this same tsc program. All four nested their +// `upsert` and `bulkUpdate` — plus (at the time) `RemoteTransport.beginTransaction`, +// which lived in this package and in this same tsc program. All four nested their // `any` inside a wider type (`Promise`, `Promise>`, // `Promise[]>`, `Promise`), which is exactly why // #15267's literal-string census never named them: the characters // `Promise` were not there to match on three of the four. An instrument's // silence is only evidence if the instrument could have spoken. // -// ⛔ `TursoDriver.beginTransaction()` is the one door of that card's nine that -// is NOT pinned here, and the reason is structural rather than an omission. +// `TursoDriver.beginTransaction()` was the one door of that card's nine that +// was NOT pinned here, and the reason was structural rather than an omission. // `TursoDriver extends SqlDriver`, and `SqlDriver.beginTransaction()` publishes // `Promise` — NARROWER than the contract's `Promise`, // the honest direction, and the binding declaration for an override. Swapping -// this override onto the contract's own type therefore does not compile: +// this override onto the contract's own type therefore did not compile: // // src/turso-driver.ts(1662,18): error TS2416: Property 'beginTransaction' // in type 'TursoDriver' is not assignable to the same property in base type // 'SqlDriver'. Type 'Promise' is not assignable to type // 'Promise>'. // -// The `any` there is not masking an un-narrowed door; it is masking a genuine -// LSP violation — in remote mode this override hands back a libsql transaction -// while the inherited declaration promises a knex one. Closing it means either +// The `any` there was not masking an un-narrowed door; it was masking a genuine +// LSP violation — in remote mode the override handed back a libsql transaction +// while the inherited declaration promised a knex one. Closing it meant either // widening `SqlDriver`'s honest narrowing (measured: +14 further consumer sites // in these three driver packages alone, and a type-safety REGRESSION for every -// `driver-sql` consumer) or restructuring the remote transaction handle. Both -// are decisions above an annotation swap, so the door is left named rather than -// quietly re-masked or forced with a cast. +// `driver-sql` consumer) or restructuring the remote transaction handle. // -// `beginTransaction()` on `TursoDriver` therefore remains unasserted here. +// ⭐ [#18063] It is pinned now, and neither of those two prices was paid — the +// third option was that the remote arm stop returning a handle at all. [#18616] +// made it refuse, `refuseRemoteTransaction` returns `never`, and a `never` +// branch is assignable to any declared return type; the only arm that still +// returns is `super.beginTransaction()`. So the override republishes the base's +// own declaration, the `any` is gone, and there is no longer a value that fails +// to be a knex transaction. `RemoteTransport`'s three transaction members — +// unreachable once the driver refused — were deleted in the same change, which +// is why the pin below moved from that class's door to this one's. The +// destination is spelled as the BASE's resolved type because `knex` is not a +// dependency of this package. // // The runtime cases below drive the LOCAL face (`:memory:`); the remote face's // shapes are pinned by the `RemoteTransport` suites. @@ -96,7 +104,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import type { IDataDriver } from '@objectstack/spec/contracts'; import { TursoDriver } from './turso-driver.js'; -import { RemoteTransport } from './remote-transport.js'; +import { SqlDriver } from '@objectstack/driver-sql'; /** `any` defeats ordinary assignability checks; this is the standard detector. */ type IsAny = 0 extends 1 & T ? true : false; @@ -204,7 +212,12 @@ type TursoAggregate = Resolved; type TursoFind = Resolved; type TursoUpsert = Resolved; type TursoBulkUpdate = Resolved; -type RemoteBeginTransaction = Resolved; +// [#18063] `RemoteTransport.beginTransaction` is gone — see the header block. +// Its pin is replaced by the one it made impossible: `TursoDriver`'s own door, +// asserted against the declaration it inherits rather than against a knex type +// this package cannot name. +type SqlBeginTransaction = Resolved; +type TursoBeginTransaction = Resolved; // 1. The contract half — what `IDataDriver` already declared before this change. const contractFindOne: Equals | null> = true; @@ -237,12 +250,16 @@ const tursoUpsertHasAny: ContainsAny = false; const tursoUpsertIsContract: Equals> = true; const tursoBulkUpdateHasAny: ContainsAny = false; const tursoBulkUpdateIsContract: Equals[]> = true; -// `RemoteTransport` is not an `IDataDriver` implementer, but it is the remote -// branch of every door above, so the same two halves are owed here. The -// `IsAny` leg carries most of the weight on an `unknown` destination: -// `Equals` is already `false`. -const remoteBeginTransactionHasAny: ContainsAny = false; -const remoteBeginTransactionIsContract: Equals = true; +// [#18063] `TursoDriver.beginTransaction()`, the door the header block used to +// record as structurally unassertable. Both halves, and the destination is the +// BASE's declaration rather than a literal type: `knex` is not a dependency of +// this package (the same constraint `KnexSlice` below works around), and +// deriving the expectation from `SqlDriver` is the stronger pin anyway — it +// cannot drift from whatever the base publishes. `Equals` is the leg that +// fires on the historical regression shape, since the door's own history is a +// bare `Promise` and `Equals` is `false`. +const tursoBeginTransactionHasAny: ContainsAny = false; +const tursoBeginTransactionIsBase: Equals = true; /** * The slice of the inherited (protected) Knex instance this fixture touches. @@ -329,18 +346,20 @@ describe('TursoDriver declared return types on the doors it overrides (#15267)', expect(result === null ? 'absent' : result.name).toBe('before'); }); - // [#17690] The three further overridden doors, plus the remote branch's own - // `beginTransaction`. Both halves each: put any one annotation back and - // `ContainsAny` flips to `true` while `Equals` flips to `false` — verified by - // ablating all four, two errors apiece and nothing else. - it('pins both halves of find(), upsert(), bulkUpdate() and RemoteTransport.beginTransaction()', () => { + // [#17690] The three further overridden doors, plus the transaction door. + // [#18063] That fourth slot moved from `RemoteTransport.beginTransaction` — + // deleted with the rest of that transport's decorative transaction members — + // to `TursoDriver.beginTransaction`, which this card made assertable. Both + // halves each: put any one annotation back and `ContainsAny` flips to `true` + // while `Equals` flips to `false`. + it('pins both halves of find(), upsert(), bulkUpdate() and TursoDriver.beginTransaction()', () => { expect([contractFind, contractUpsert, contractBulkUpdate, contractBeginTransaction]).toEqual([ true, true, true, true, ]); - expect([tursoFindHasAny, tursoUpsertHasAny, tursoBulkUpdateHasAny, remoteBeginTransactionHasAny]).toEqual([ + expect([tursoFindHasAny, tursoUpsertHasAny, tursoBulkUpdateHasAny, tursoBeginTransactionHasAny]).toEqual([ false, false, false, @@ -350,7 +369,7 @@ describe('TursoDriver declared return types on the doors it overrides (#15267)', tursoFindIsContract, tursoUpsertIsContract, tursoBulkUpdateIsContract, - remoteBeginTransactionIsContract, + tursoBeginTransactionIsBase, ]).toEqual([true, true, true, true]); }); diff --git a/packages/drivers/driver-turso/src/turso-driver.test.ts b/packages/drivers/driver-turso/src/turso-driver.test.ts index baf91247cd0..4a92e31432c 100644 --- a/packages/drivers/driver-turso/src/turso-driver.test.ts +++ b/packages/drivers/driver-turso/src/turso-driver.test.ts @@ -429,10 +429,17 @@ describe('TursoDriver Capabilities', () => { expect(typeof driver.syncSchemasBatch).toBe('function'); }); - it('claims batchSchemaSync + an emptied queryDateGranularity in remote mode', () => { + it('claims batchSchemaSync, an emptied queryDateGranularity and no transactions in remote mode', () => { const remote = new TursoDriver({ url: 'libsql://test-db.turso.io', authToken: 'test-token' }); expect(remote.transportMode).toBe('remote'); - expect(ownCapabilityClaims(remote)).toEqual(['batchSchemaSync', 'queryDateGranularity']); + // [#18063] `transactionsUnsupported` joins the remote-only diff. It is a + // claim HERE and not in local mode for the reason this census exists: the + // bit is per-INSTANCE, and a value equal to the base's is not a claim. + expect(ownCapabilityClaims(remote)).toEqual([ + 'batchSchemaSync', + 'queryDateGranularity', + 'transactionsUnsupported', + ]); }); it('does NOT advertise native date-granularity in remote mode (avoids the "[object Object]" aggregate 500)', () => { diff --git a/packages/drivers/driver-turso/src/turso-driver.ts b/packages/drivers/driver-turso/src/turso-driver.ts index 4b2b21a0da4..bb55757e602 100644 --- a/packages/drivers/driver-turso/src/turso-driver.ts +++ b/packages/drivers/driver-turso/src/turso-driver.ts @@ -671,6 +671,26 @@ export class TursoDriver extends SqlDriver { ...(this.transportMode === 'remote' ? { queryDateGranularity: {} as Record } : {}), + + // [#18063] The remote transport has NO transactions, and this is the + // declaration that lets it say so. `TursoDriver extends SqlDriver`, whose + // `beginTransaction()` opens a real knex transaction, so method presence + // — the engine's gate until now — reported this face as transactional. It + // is not: `RemoteTransport`'s data methods take no `options` argument at + // all, so a handle cannot reach the statement that would have to join it; + // the write executed on the plain connection and was already durable, and + // `rollback()` resolved having undone nothing. + // + // With the bit set the engine stops opening a transaction it cannot + // honour and takes the DECLARED non-transactional path instead (ADR-0119 + // D1) — warning once, or throwing `TransactionUnsupportedError` before + // any write when the caller passed `require: true`. That is also the path + // `refuseRemoteTransaction`'s own message already tells callers to take, + // nothing could reach while the gate read method presence. + // + // Local and embedded-replica modes inherit `false` from SqlDriver: they + // run knex against a real connection and honour `options.transaction`. + transactionsUnsupported: this.transportMode === 'remote', }; } @@ -1826,28 +1846,38 @@ export class TursoDriver extends SqlDriver { // Transactions (remote mode overrides) // =================================== - // ⛔ [#17690] This door stays `Promise`, and that is a NAMED remainder - // rather than an oversight. `TursoDriver extends SqlDriver`, whose + // ⭐ [#18063] This door publishes the INHERITED declaration, and the `any` it + // used to publish is gone. The history is worth keeping because the `any` was + // a NAMED remainder, not an oversight: `TursoDriver extends SqlDriver`, whose // `beginTransaction()` publishes `Promise` — narrower than // the contract's `Promise`, the honest direction, and the binding - // declaration for an override. Swapping this onto the contract's own type - // does not compile (TS2416: `Promise` is not assignable to - // `Promise>`). So the `any` here is not masking an - // un-narrowed door — it is masking a real LSP violation: in remote mode this - // returns a libsql transaction while the inherited declaration promises a - // knex one. Closing it means widening `SqlDriver`'s honest narrowing - // (measured: +14 further consumer sites in the three driver packages, and a - // type-safety regression for every `driver-sql` consumer) or restructuring - // the remote handle. Both are above an annotation swap; the reasoning is - // recorded in `turso-driver-doors-declared-types.test.ts`. + // declaration for an override. While the remote arm RETURNED a libsql + // transaction there was no honest annotation available: the contract's own + // type does not compile against the base (TS2416), and the base's type would + // have been a lie on the remote arm. The `any` masked that real LSP + // violation, and closing it meant widening `SqlDriver`'s narrowing (measured + // at the time: +14 further consumer sites across the driver packages) or + // restructuring the remote handle — both above an annotation swap (#17690). + // + // What dissolved it is that the remote arm no longer returns anything. + // [#18616] made it REFUSE, and `refuseRemoteTransaction` returns `never`, so + // the branch is assignable to any return type; the only arm that still + // returns is `super.beginTransaction()`, whose type this now simply repeats. + // The LSP violation is not re-dressed here, it is absent: there is no longer + // a value that fails to be a knex transaction. // - // ⭐ [#18616] The remote arm now REFUSES instead of returning a handle. That - // also retires the LSP remainder above **on this arm only**: the remote - // branch no longer returns a libsql transaction against an inherited - // declaration that promises a knex one, because it returns nothing at all. - // The `Promise` annotation stays, because the LOCAL/replica arm is still - // `super.beginTransaction()` and the analysis above is unchanged for it. - override async beginTransaction(): Promise { + // And [#18063] closes the path that reached it. `supports` now declares + // `transactionsUnsupported` on the remote arm and the engine gates on the + // DECLARATION, so `engine.transaction()` takes the non-transactional path + // (ADR-0119 D1) rather than calling this and catching a 501 — the refusal + // below stays as the floor for a caller that reaches past the engine. + // ⛔ Spelled `ReturnType` and not + // `Promise`: `knex` is not a dependency of this package, so + // its types cannot be named here (`check:undeclared-dep-imports`; the same + // reason `turso-driver-doors-declared-types.test.ts` structurally types its + // knex slice). Deriving it from the base is also the stronger pin — it cannot + // drift from whatever `SqlDriver` publishes. + override async beginTransaction(): ReturnType { if (this.isRemote) { refuseRemoteTransaction( '`beginTransaction()`', diff --git a/packages/drivers/driver-turso/src/turso-transactions-unsupported-declaration.test.ts b/packages/drivers/driver-turso/src/turso-transactions-unsupported-declaration.test.ts new file mode 100644 index 00000000000..67342aa7a98 --- /dev/null +++ b/packages/drivers/driver-turso/src/turso-transactions-unsupported-declaration.test.ts @@ -0,0 +1,80 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#18063] The remote face DECLARES that it has no transactions, and the other + * two faces do not. + * + * `TursoDriver extends SqlDriver`, whose `beginTransaction()` opens a real knex + * transaction — so METHOD PRESENCE, the engine's transaction gate until now, + * reported all three faces as transactional. Remote is not: `RemoteTransport`'s + * data methods take no `options` argument at all, so a handle could never reach + * the statement that would have to join it. A subclass cannot opt out of a door + * it inherited, which is what `supports.transactionsUnsupported` is for. + * + * ⚠️ This pins the DECLARATION only. What the engine does with it is pinned in + * `packages/objectql/src/engine-transaction-declared-unsupported.test.ts`, and + * the driver-level refusals that fire when a caller reaches past the engine are + * pinned in `turso-remote-transaction-refusal.test.ts` (#18616). Three layers, + * three suites — a driver that declares honestly and an engine that ignores the + * declaration would leave both of the others green. + * + * Constructing the driver is enough: `transportMode` is resolved in the + * constructor, so no connection, no stub and no network are needed for the + * `supports` reading. The one case that connects does so to prove the base + * spread stayed intact. + */ + +import { describe, it, expect } from 'vitest'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { TursoDriver } from './turso-driver.js'; + +const remote = () => new TursoDriver({ url: 'libsql://probe.turso.io', authToken: 't' }); +const local = () => new TursoDriver({ url: ':memory:' }); +const replica = () => + new TursoDriver({ + url: ':memory:', + syncUrl: 'libsql://probe.turso.io', + authToken: 't', + sync: { onConnect: false, intervalSeconds: 0 }, + }); + +describe('[#18063] TursoDriver.supports.transactionsUnsupported', () => { + it('is true on the REMOTE face', () => { + const driver = remote(); + expect(driver.transportMode).toBe('remote'); + expect(driver.supports.transactionsUnsupported).toBe(true); + }); + + it('is false on LOCAL and REPLICA — both run knex against a real connection', () => { + // The discriminating control. If this bit were set per-CLASS rather than + // per-instance, the case above would be green for the wrong reason and + // every local Turso deployment would silently lose transactions. + const localDriver = local(); + const replicaDriver = replica(); + expect(localDriver.transportMode).toBe('local'); + expect(replicaDriver.transportMode).toBe('replica'); + expect(localDriver.supports.transactionsUnsupported).toBe(false); + expect(replicaDriver.supports.transactionsUnsupported).toBe(false); + }); + + it('inherits the value from SqlDriver rather than restating it — the base declares false', () => { + // `false` reaches the local face through `...super.supports`, which is why + // a future base-level change cannot leave this subclass behind. + // + // ⛔ Asserted with `instanceof` and not `constructor.name`: the built + // bundle renames the class to `_SqlDriver`, so a name comparison is green + // or red depending on whether the suite resolved source or dist. + expect(TursoDriver.prototype).toBeInstanceOf(SqlDriver); + expect(local().supports.transactionsUnsupported).toBe(false); + }); + + it('does not disturb the other bits the remote face already declared', () => { + const driver = remote(); + // The remote arm's existing overrides, unchanged: DDL still batches, native + // date bucketing is still withheld. + expect(driver.supports.batchSchemaSync).toBe(true); + expect(driver.supports.queryDateGranularity).toEqual({}); + // And the base's own live bit still comes through the spread. + expect(driver.supports.autonumber).toBe(true); + }); +}); diff --git a/packages/objectql/src/engine-transaction-declared-unsupported.test.ts b/packages/objectql/src/engine-transaction-declared-unsupported.test.ts new file mode 100644 index 00000000000..077ac1de317 --- /dev/null +++ b/packages/objectql/src/engine-transaction-declared-unsupported.test.ts @@ -0,0 +1,232 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#18063] The engine's transaction gate reads the DECLARATION, not bare method +// presence. +// +// The shape this exists for cannot be produced by the pre-existing doubles, and +// that is the whole point: a driver that INHERITS `beginTransaction` from a base +// class whose transport has transactions, while its own transport has none. The +// method is present, so the old gate — `if (!drv?.beginTransaction)` — answered +// "transactional", the engine opened a transaction, and the handle it minted +// reached no statement. The measured instance is the libSQL remote transport, +// whose data methods take no `options` argument at all: the write executed on +// the plain connection, was already durable, and `rollback()` resolved having +// undone nothing. +// +// A subclass cannot opt out of a door it did not open, which is why the answer +// is a declaration (`supports.transactionsUnsupported`) rather than a second +// method-presence test. `driverSupportsTransactions` in `@objectstack/spec` is +// the one definition; both engine transaction surfaces call it. +// +// ⭐ Every case below pairs with a LIT CONTROL on the same double with the bit +// removed — the difference between the two columns is the bit and nothing else, +// so a green assertion here cannot be green because the double is inert. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL, ScopedContext } from './engine.js'; +import { TransactionUnsupportedError } from './transaction-errors.js'; + +interface Recorded { + level: 'debug' | 'info' | 'warn' | 'error'; + message: string; + args: unknown[]; +} + +function recordingLogger() { + const records: Recorded[] = []; + const push = (level: Recorded['level']) => (message: string, ...args: unknown[]) => + void records.push({ level, message: String(message), args }); + return { + records, + logger: { debug: push('debug'), info: push('info'), warn: push('warn'), error: push('error') }, + at(level: Recorded['level']) { + return records.filter((r) => r.level === level); + }, + }; +} + +/** + * A driver that ALWAYS implements `beginTransaction` — the inherited door — and + * differs only in whether it declares the transport cannot honour it. + * + * `begins` counts the calls, because "the engine did not open a transaction" is + * the load-bearing observation and an absent handle alone would not prove it: + * the engine could have opened one and dropped it. + */ +function makeDriver(name: string, declareUnsupported: boolean) { + const writes: Array<{ object: string; op: string; transaction: unknown }> = []; + const rows = new Map>(); + const begins: unknown[] = []; + let nextId = 0; + const driver: any = { + name, + version: '0.0.0', + // The ONLY difference between the two columns. + supports: declareUnsupported ? { transactionsUnsupported: true } : {}, + writes, + begins, + async connect() {}, + async disconnect() {}, + async checkHealth() { return true; }, + async execute() { return null; }, + async find() { return Array.from(rows.values()); }, + async findOne(_o: string, ast: any) { + const id = ast?.where?.find?.((c: any) => c?.field === 'id')?.value; + if (id !== undefined) return rows.get(String(id)) ?? null; + for (const r of rows.values()) return r; + return null; + }, + async create(object: string, data: Record, options: any) { + writes.push({ object, op: 'create', transaction: options?.transaction }); + nextId += 1; + const id = (data.id as string) ?? `${name}_${nextId}`; + const row = { ...data, id }; + rows.set(id, row); + return row; + }, + async update(object: string, id: string, data: Record, options: any) { + writes.push({ object, op: 'update', transaction: options?.transaction }); + const row = { ...rows.get(String(id)), ...data, id }; + rows.set(String(id), row); + return row; + }, + async delete(object: string, id: string, options: any) { + writes.push({ object, op: 'delete', transaction: options?.transaction }); + return rows.delete(String(id)); + }, + async count() { return 0; }, + async bulkCreate(object: string, batch: Record[]) { + return Promise.all(batch.map((r) => this.create(object, r, undefined))); + }, + async bulkUpdate() { return []; }, + async bulkDelete() {}, + async syncSchema() {}, + // Present on BOTH columns — inherited from a base whose transport can. + async beginTransaction() { + const handle = { __trx: name, n: begins.length }; + begins.push(handle); + return handle; + }, + async commit() {}, + async rollback() {}, + }; + return driver; +} + +async function engineWith(declareUnsupported: boolean) { + const rec = recordingLogger(); + const engine = new ObjectQL({ logger: rec.logger } as any); + const driver = makeDriver('primary', declareUnsupported); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject({ name: 'thing', fields: { name: { type: 'text' } } }, '__test__'); + return { rec, engine, driver }; +} + +const scopedOf = (engine: ObjectQL): ScopedContext => + (engine as any).createContext({ userId: 'u1' }) as ScopedContext; + +describe('[#18063] engine.transaction() gates on supports.transactionsUnsupported', () => { + it('does not call beginTransaction on a declaring driver — and DOES on the same double without the bit', async () => { + const declared = await engineWith(true); + const control = await engineWith(false); + + await declared.engine.transaction(async () => 'ran'); + await control.engine.transaction(async () => 'ran'); + + // DARK: the declaration was honoured. + expect(declared.driver.begins).toHaveLength(0); + // LIT: the same double, same method, bit removed — the engine still opens + // one, so the zero above is a reading and not an inert fixture. + expect(control.driver.begins).toHaveLength(1); + }); + + it('runs the callback non-transactionally and threads NO handle to the writes', async () => { + const declared = await engineWith(true); + const control = await engineWith(false); + + await declared.engine.transaction(async () => { + await declared.engine.insert('thing', { name: 'x' }); + }); + await control.engine.transaction(async () => { + await control.engine.insert('thing', { name: 'x' }); + }); + + expect(declared.driver.writes).toHaveLength(1); + expect(declared.driver.writes[0].transaction).toBeUndefined(); + // LIT control: the write on the undeclared double carries the handle. + expect(control.driver.writes[0].transaction).toEqual({ __trx: 'primary', n: 0 }); + }); + + it('reports owned: false — there is no transaction here to own', async () => { + const { engine } = await engineWith(true); + let owned: boolean | undefined; + await engine.transaction(async (_ctx: any, info: any) => { owned = info.owned; }); + expect(owned).toBe(false); + }); + + it('warns once, and the warning names the DECLARATION rather than a missing method', async () => { + const { engine, rec } = await engineWith(true); + + await engine.transaction(async () => 1); + await engine.transaction(async () => 2); + + const warns = rec.at('warn'); + expect(warns).toHaveLength(1); + expect(warns[0].message).toContain('declares supports.transactionsUnsupported'); + // ⛔ The message an operator must NOT get: this driver publishes the method, + // so sending them to look for a missing one wastes the report. + expect(warns[0].message).not.toContain("driver 'primary' has no beginTransaction"); + // The consequence has to stay in the text — it is the reason this degrades + // loudly rather than quietly. + expect(warns[0].message).toContain('running WITHOUT transaction or rollback'); + }); + + it('throws TransactionUnsupportedError under require: true, before the callback writes anything', async () => { + const { engine, driver } = await engineWith(true); + let ran = false; + + await expect( + engine.transaction(async () => { ran = true; }, undefined, { require: true }), + ).rejects.toBeInstanceOf(TransactionUnsupportedError); + + expect(ran).toBe(false); + expect(driver.writes).toHaveLength(0); + expect(driver.begins).toHaveLength(0); + }); +}); + +describe('[#18063] ScopedContext reads the same declaration', () => { + it('degrades instead of opening one, with the LIT control opening one', async () => { + const declared = await engineWith(true); + const control = await engineWith(false); + + await expect(scopedOf(declared.engine).transaction(async () => 'ran')).resolves.toBe('ran'); + await expect(scopedOf(control.engine).transaction(async () => 'ran')).resolves.toBe('ran'); + + expect(declared.driver.begins).toHaveLength(0); + expect(control.driver.begins).toHaveLength(1); + }); + + it('the discrete trio returns null rather than a handle covering nothing', async () => { + const declared = await engineWith(true); + const control = await engineWith(false); + + const declaredOpened = await scopedOf(declared.engine).beginTransaction(); + const controlOpened = await scopedOf(control.engine).beginTransaction(); + + // `null` is the trio's declared "no transaction support" answer, and the + // caller then runs non-transactionally — the same graceful degrade a driver + // with no `beginTransaction` already got. + expect(declaredOpened).toBeNull(); + expect(controlOpened).not.toBeNull(); + expect(controlOpened!.owned).toBe(true); + }); + + it('refuses under require: true, on this surface too', async () => { + const { engine } = await engineWith(true); + await expect( + scopedOf(engine).transaction(async () => 'unreachable', { require: true }), + ).rejects.toBeInstanceOf(TransactionUnsupportedError); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 235bdf25b98..b187b43aafe 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -26,7 +26,7 @@ import type { WriteObservabilityOptions } from '@objectstack/spec/contracts'; // engine is what `metadata-protocol.validateData` returns, so letting the two // drift would put a translation layer between a verdict and its contract. import type { ValidateDataIssue, ValidateDataResponse } from '@objectstack/spec/api'; -import { parseAutonumberFormat, renderAutonumber, resolveAutonumberFormat, readAutonumberCounter, missingFieldValues, isTenancyDisabled, FILE_REFERENCE_TYPES, REFERENCE_VALUE_TYPES, referenceTargetOf, isFileIdToken, RAW_FILE_VALUES_CONTEXT_KEY, isCurrentUserDefaultToken, isNowDefaultToken, isMultiValueField } from '@objectstack/spec/data'; +import { parseAutonumberFormat, renderAutonumber, resolveAutonumberFormat, readAutonumberCounter, missingFieldValues, isTenancyDisabled, FILE_REFERENCE_TYPES, REFERENCE_VALUE_TYPES, referenceTargetOf, isFileIdToken, RAW_FILE_VALUES_CONTEXT_KEY, isCurrentUserDefaultToken, isNowDefaultToken, isMultiValueField, driverSupportsTransactions } from '@objectstack/spec/data'; // [#5158] Door 2's lowering sink — the SAME pair the protocol face (Door 1) // runs, so `FilterArray` has exactly one lowering in the product. import { @@ -14831,7 +14831,10 @@ export class ObjectQL implements IObjectQLEngine { * `OperationContext.context.transaction` and the SQL driver's per-builder * `.transacting(trx)` call. * - * - If the default driver does not support `beginTransaction`, the callback + * - If the default driver does not support transactions — no + * `beginTransaction` method, or `supports.transactionsUnsupported: true` + * from a transport that inherited one it cannot honour (#18063) — the + * callback * runs directly with the supplied base context (no rollback). This keeps * the API safe to call on drivers without ACID support (e.g. the * in-memory driver in tests). It is DECLARED behaviour (ADR-0119 D1), not @@ -14888,7 +14891,14 @@ export class ObjectQL implements IObjectQLEngine { } const driver = this.defaultDriver ? this.drivers.get(this.defaultDriver) : undefined; const drv = driver as any; - if (!drv?.beginTransaction) { + // [#18063] The gate is the DECLARATION, not bare method presence. A + // subclass inherits `beginTransaction` from a base whose transport has + // transactions while its own has none — it cannot opt out of a door it did + // not open, so presence alone routed it down the transactional path and the + // handle it produced covered nothing. `driverSupportsTransactions` is the + // one definition (`@objectstack/spec`), shared with the ScopedContext trio + // below so the engine's transaction entrances cannot drift apart. + if (!driverSupportsTransactions(drv)) { const datasource = this.defaultDriver ?? drv?.name; if (opts?.require === true) { // Fail CLOSED (#5696 point 1): the caller declared it cannot tolerate @@ -14899,7 +14909,7 @@ export class ObjectQL implements IObjectQLEngine { } // Declared degrade (ADR-0119 D1) — behaviour unchanged, but no longer // mute: the caller asked for atomicity and is not getting it (#4619). - this.warnTransactionUnsupported(datasource); + this.warnTransactionUnsupported(datasource, drv?.supports?.transactionsUnsupported === true); // `owned: false` — honest: there is no transaction here to own, and no // rollback the callback may promise on the strength of it. return callback(baseContext, { owned: false }); @@ -14961,6 +14971,14 @@ export class ObjectQL implements IObjectQLEngine { * The behaviour is unchanged and DECLARED (ADR-0119 D1: "when that driver has * no `beginTransaction` the callback runs with NO transaction and NO * rollback"). What was missing is that a caller had no way to find out — + * + * [#18063] TWO reasons now reach this degrade and the message says which. + * The second is a transport that DECLARED it cannot honour a handle while + * inheriting `beginTransaction` from a base class that can. Before the + * declaration existed such a driver was indistinguishable from a working one + * here, so the engine opened a transaction against it and the degrade — the + * honest answer — was unreachable. + * * the same shape as `batchData`'s `atomic` flag being a lie for as long as it * was (ADR-0119 D4). Tightening this into a throw would change the declared * contract and is deliberately NOT done here. @@ -14976,19 +14994,32 @@ export class ObjectQL implements IObjectQLEngine { * Once per engine instance per driver: the drivers that reach this path (test * doubles, foreign engines) reach it on EVERY call. */ - private warnTransactionUnsupported(datasource: string | undefined): void { + private warnTransactionUnsupported(datasource: string | undefined, declaredUnsupported = false): void { const name = datasource ?? ''; if (this.transactionUnsupportedReported.has(name)) return; this.transactionUnsupportedReported.add(name); + // [#18063] The two reasons reach the same degrade and must not read the + // same. Telling an operator a Turso REMOTE datasource "has no + // beginTransaction" sends them looking for a missing method on a class that + // publishes one; the fix for that reason is a different transport, not a + // different driver. + const cause = declaredUnsupported + ? `driver '${name}' declares supports.transactionsUnsupported — its transport cannot carry a ` + + 'transaction handle even though it inherits beginTransaction' + : `driver '${name}' has no beginTransaction`; + const remedy = declaredUnsupported + ? 'Point this datasource at a transport that honours transactions (for libSQL: the local or ' + + 'embedded-replica mode rather than the remote one), or have the caller fail ' + : 'Register a driver that implements beginTransaction for this datasource, or have the caller fail '; this.logger.warn( - `transaction() requested a transaction but driver '${name}' has no beginTransaction — ` + + `transaction() requested a transaction but ${cause} — ` + 'running WITHOUT transaction or rollback. Every write the callback makes commits as it executes, ' + 'so a later throw leaves the earlier ones PERSISTED even though the call rejects as if the whole ' + 'unit of work had been undone; no caller is told, and the records stay behind. ' + - 'Register a driver that implements beginTransaction for this datasource, or have the caller fail ' + + remedy + "closed itself when it cannot tolerate losing atomicity (batchData's atomic gate, ADR-0119 D4, is " + 'the pattern). Reported once per driver per engine instance.', - { datasource: name }, + { datasource: name, declaredUnsupported }, ); } @@ -15755,7 +15786,9 @@ export class ScopedContext implements IScopedContext, RunAsDerivableApi { ? engine.drivers?.get(engine.defaultDriver) : undefined; - if (!driver?.beginTransaction) { + // [#18063] Declaration, not bare method presence — see the engine surface's + // gate; one predicate serves both so they cannot answer differently. + if (!driverSupportsTransactions(driver)) { const datasource = engine.defaultDriver ?? driver?.name; if (opts?.require === true) { // Same fail-closed refusal as the engine surface (#5696 point 1). @@ -15764,7 +15797,7 @@ export class ScopedContext implements IScopedContext, RunAsDerivableApi { // No transaction support — execute directly. Declared (ADR-0119 D1), but // said out loud since #4619: the caller asked for atomicity and the // callback is about to run without any. - engine.warnTransactionUnsupported?.(datasource); + engine.warnTransactionUnsupported?.(datasource, driver?.supports?.transactionsUnsupported === true); return callback(this, { owned: false }); } @@ -15845,7 +15878,11 @@ export class ScopedContext implements IScopedContext, RunAsDerivableApi { const driver = engine.defaultDriver ? engine.drivers?.get(engine.defaultDriver) : undefined; - return driver?.beginTransaction ? driver : undefined; + // [#18063] Declaration, not bare method presence — the trio's `begin` + // returns `null` and `commit`/`rollback` abstain for a transport that + // declared it cannot honour a handle, which is the same graceful degrade a + // driver with no `beginTransaction` already gets. + return driverSupportsTransactions(driver) ? driver : undefined; } /** diff --git a/packages/spec/api-surface-declarations/contracts.txt b/packages/spec/api-surface-declarations/contracts.txt index 5362717e751..cccfda3cc7f 100644 --- a/packages/spec/api-surface-declarations/contracts.txt +++ b/packages/spec/api-surface-declarations/contracts.txt @@ -3446,6 +3446,15 @@ interface IDataDriver { temporalFilterColumnSql?(objectName: string, field: string, columnSql: string): string; /** * Begin a new database transaction. + * + * ⛔ Implementing this method is NOT by itself a claim that the transport + * honours transactions, because a subclass inherits it. A transport that + * cannot carry a handle — one whose data methods never receive + * `options.transaction` — declares `supports.transactionsUnsupported: true` + * and the engine takes the declared non-transactional path (ADR-0119 D1) + * instead of calling this. `driverSupportsTransactions()` in + * `data/driver.zod.ts` is the one predicate that answers the question. + * * @returns A transaction handle to pass via `options.transaction`. */ beginTransaction(options?: { diff --git a/packages/spec/api-surface-declarations/data.txt b/packages/spec/api-surface-declarations/data.txt index bdf043b1be4..04309e5c34c 100644 --- a/packages/spec/api-surface-declarations/data.txt +++ b/packages/spec/api-surface-declarations/data.txt @@ -12,8 +12,8 @@ # excluded: documentation drift is `check:docs`'s axis, not this one. # # entry: ./data -# exported names: 832 -# declarations: 845 +# exported names: 833 +# declarations: 846 # # GENERATED — ⛔ never hand-edited. Regenerate after a real build: # pnpm --filter @objectstack/spec build && pnpm --filter @objectstack/spec gen:api-surface-declarations @@ -5288,6 +5288,7 @@ declare const DriverCapabilitiesSchema: z.ZodObject<{ }>, z.ZodBoolean>>; autonumber: z.ZodOptional; batchSchemaSync: z.ZodOptional; + transactionsUnsupported: z.ZodOptional; create: z.ZodOptional; read: z.ZodOptional; update: z.ZodOptional; @@ -5355,6 +5356,7 @@ declare const DriverConfigSchema: z.ZodObject<{ }>, z.ZodBoolean>>; autonumber: z.ZodOptional; batchSchemaSync: z.ZodOptional; + transactionsUnsupported: z.ZodOptional; create: z.ZodOptional; read: z.ZodOptional; update: z.ZodOptional; @@ -5428,6 +5430,7 @@ declare const DriverInterfaceSchema: z.ZodObject<{ }>, z.ZodBoolean>>; autonumber: z.ZodOptional; batchSchemaSync: z.ZodOptional; + transactionsUnsupported: z.ZodOptional; create: z.ZodOptional; read: z.ZodOptional; update: z.ZodOptional; @@ -15136,6 +15139,7 @@ declare const NoSQLDriverConfigSchema: z.ZodObject<{ }>, z.ZodBoolean>>; autonumber: z.ZodOptional; batchSchemaSync: z.ZodOptional; + transactionsUnsupported: z.ZodOptional; create: z.ZodOptional; read: z.ZodOptional; update: z.ZodOptional; @@ -21573,6 +21577,7 @@ declare const SQLDriverConfigSchema: z.ZodObject<{ }>, z.ZodBoolean>>; autonumber: z.ZodOptional; batchSchemaSync: z.ZodOptional; + transactionsUnsupported: z.ZodOptional; create: z.ZodOptional; read: z.ZodOptional; update: z.ZodOptional; @@ -22883,6 +22888,14 @@ declare function driverConfigJsonSchema(schema: z.ZodType): () => Record; diff --git a/packages/spec/src/data/driver.test.ts b/packages/spec/src/data/driver.test.ts index 9ca93768772..15fef7191e0 100644 --- a/packages/spec/src/data/driver.test.ts +++ b/packages/spec/src/data/driver.test.ts @@ -6,6 +6,7 @@ import { type DriverCapabilities, type DriverInterface, type DriverOptions, + driverSupportsTransactions, } from './driver.zod'; /** @@ -48,8 +49,22 @@ const RETIRED_BITS = [ 'queryCache', ] as const; -/** The bits that survive — each with a named engine reader. */ -const LIVE_BITS = ['queryDateGranularity', 'autonumber', 'batchSchemaSync'] as const; +/** + * The bits that survive — each with a named engine reader. + * + * [#18063] `transactionsUnsupported` is the fourth, and it arrived under the + * SAME rule that removed thirty-one: a bit exists here only where method + * presence cannot carry the signal, and only WITH the reader that dispatches on + * it. `transactions` was retired because no code read it; this one is read by + * `driverSupportsTransactions`, which every transaction entrance in the engine + * now calls. It is not that key revived — see the tombstone pin below. + */ +const LIVE_BITS = [ + 'queryDateGranularity', + 'autonumber', + 'batchSchemaSync', + 'transactionsUnsupported', +] as const; describe('DriverCapabilitiesSchema', () => { it('accepts the live capability bits', () => { @@ -57,6 +72,7 @@ describe('DriverCapabilitiesSchema', () => { queryDateGranularity: { day: true, week: false, month: true, quarter: true, year: true }, autonumber: true, batchSchemaSync: true, + transactionsUnsupported: true, }; expect(() => DriverCapabilitiesSchema.parse(capabilities)).not.toThrow(); @@ -69,9 +85,12 @@ describe('DriverCapabilitiesSchema', () => { // the default forced every capability object to spell out dead weight. expect(parsed).not.toHaveProperty('batchSchemaSync'); expect(parsed).not.toHaveProperty('autonumber'); + // [#18063] Absence is the whole point for `transactionsUnsupported`: a + // driver that declares nothing keeps the transactions it has today. + expect(parsed).not.toHaveProperty('transactionsUnsupported'); }); - it('declares exactly the audited shape: 3 live bits + 31 tombstones', () => { + it('declares exactly the audited shape: 4 live bits + 31 tombstones', () => { const shape = (DriverCapabilitiesSchema as unknown as { shape: Record }).shape; const keys = Object.keys(shape).sort(); expect(keys).toEqual([...RETIRED_BITS, ...LIVE_BITS].slice().sort()); @@ -102,10 +121,18 @@ describe('[#4634] the 31 inert capability bits are tombstoned, not stripped', () ); }); - it('the transactions prescription points at method presence, not a replacement bit', () => { + it('the transactions prescription points at method presence, and says the live bit is not it restored', () => { expect(() => DriverCapabilitiesSchema.parse({ transactions: true })).toThrow( /DriverCapabilities\.transactions.*removed.*METHOD PRESENCE.*beginTransaction.*Delete the key/s, ); + // [#18063] The trap this sentence exists to close: a reader who sees a live + // `transactionsUnsupported` and concludes the retired key came back. It did + // not — the retired one CLAIMED support nothing checked, the live one + // DENIES support the engine checks — and the tombstone still refuses, so + // the two cannot be confused by writing one and getting the other. + expect(() => DriverCapabilitiesSchema.parse({ transactions: false })).toThrow( + /transactionsUnsupported.*NOT this key restored/s, + ); }); it('still parses the live bits cleanly — the tombstones reject a key, not the record', () => { @@ -156,7 +183,8 @@ describe('[#4634] tsc channel: the retired bits are unwritable in DriverCapabili const capsType = checker.getDeclaredTypeOfSymbol(capsAlias!); const props = new Map(capsType.getProperties().map((p) => [p.getName(), p])); - // Anti-vacuity: the walked shape is the audited 34-key shape. + // Anti-vacuity: the walked shape is the audited key set — 31 tombstones + // plus the live bits (4 since #18063 added `transactionsUnsupported`). expect([...props.keys()].sort()).toEqual([...RETIRED_BITS, ...LIVE_BITS].slice().sort()); const decl = capsAlias!.declarations?.[0]; @@ -382,3 +410,37 @@ describe('DriverOptions.timeout → DriverOptions.timeoutMs (#14478)', () => { expect(good.timeoutMs).toBe(5000); }); }); + +// =========================================================================== +// [#18063] driverSupportsTransactions — the one definition of the gate +// =========================================================================== + +describe('[#18063] driverSupportsTransactions', () => { + const withMethod = { beginTransaction: async () => ({}) }; + + it('is false for a driver with no beginTransaction — the pre-existing gate, unchanged', () => { + expect(driverSupportsTransactions({})).toBe(false); + expect(driverSupportsTransactions(undefined)).toBe(false); + expect(driverSupportsTransactions(null)).toBe(false); + // A non-callable member is not a door: the old gate was truthiness on the + // property, which a stray string would have passed. + expect(driverSupportsTransactions({ beginTransaction: 'yes' as unknown as () => void })).toBe(false); + }); + + it('is true for a driver with the method and no declaration — every driver today', () => { + expect(driverSupportsTransactions(withMethod)).toBe(true); + expect(driverSupportsTransactions({ ...withMethod, supports: {} })).toBe(true); + expect(driverSupportsTransactions({ ...withMethod, supports: { transactionsUnsupported: false } })).toBe(true); + }); + + it('is FALSE for a driver that inherited the method and declared it cannot honour it', () => { + // The shape this bit exists for: the method IS present — it is inherited — + // so the old method-presence gate answered `true` and the engine opened a + // transaction the transport could not carry. + expect(driverSupportsTransactions({ ...withMethod, supports: { transactionsUnsupported: true } })).toBe(false); + }); + + it('only the literal `true` denies — an absent or undefined bit is not a denial', () => { + expect(driverSupportsTransactions({ ...withMethod, supports: { transactionsUnsupported: undefined } })).toBe(true); + }); +}); diff --git a/packages/spec/src/data/driver.zod.ts b/packages/spec/src/data/driver.zod.ts index 6e0ffbb2e03..46daf837a26 100644 --- a/packages/spec/src/data/driver.zod.ts +++ b/packages/spec/src/data/driver.zod.ts @@ -147,14 +147,24 @@ const capRemoved = (key: string, mechanism: string) => * autonumber generation to the driver when set. * - `batchSchemaSync` — opt-in for `syncSchemasBatch()` even where a base * class inherits the method; the engine ANDs it with method presence. + * - `transactionsUnsupported` — opt-OUT of `beginTransaction()` for a + * transport that inherits the method from a base class it cannot honour; + * the engine ANDs its negation with method presence. * - * Everything else IS the method: transactions gate on - * `driver.beginTransaction`, aggregate pushdown on + * Everything else IS the method: aggregate pushdown gates on * `typeof driver.aggregate === 'function'`, schema sync on * `typeof driver.syncSchema === 'function'`, and the REQUIRED CRUD/bulk * methods are called unconditionally. Do not add a boolean here for behaviour * that a method's presence — or a caller that does not exist yet — already * decides; that is how thirty-one dead bits accumulated. + * + * ⭐ The reverse of that last sentence is the bar a NEW bit has to clear, and + * it is the bar `transactionsUnsupported` cleared: a bit arrives WITH the + * engine reader that dispatches on it, in the same change. Adding one to + * "document" a transport is the ADR-0078 false affordance this record was + * pruned for; adding one because method presence provably lies — a subclass + * inherits a door its own transport cannot open — is what the three survivors + * above already are. */ export const DriverCapabilitiesSchema = lazySchema(() => z.object({ // ============================================================================ @@ -223,6 +233,42 @@ export const DriverCapabilitiesSchema = lazySchema(() => z.object({ */ batchSchemaSync: z.boolean().optional().describe('Supports batched schema sync to reduce schema DDL round-trips (absence = false)'), + /** + * This transport has NO transactions — refuse rather than pretend. + * + * The mirror image of `batchSchemaSync`, and it exists for the same reason: + * a base class can publish `beginTransaction()` while the transport the + * subclass actually speaks cannot carry a transaction at all, so method + * presence lies. `batchSchemaSync` is the opt-IN for that shape; this is the + * opt-OUT. The engine reads it as `presence AND NOT this bit` — a driver + * with no `beginTransaction` is unaffected, and a driver that declares + * nothing is unaffected. + * + * Set it when handing back a handle would be a FALSE SUCCESS rather than a + * missing feature: the caller gets a handle, the writes execute and are + * already durable, `rollback()` resolves and undoes nothing. The libSQL + * remote transport is the measured instance — its data methods take no + * `options` argument at all, so a handle cannot reach the statement that + * would have to join it. + * + * What the engine does with it: `engine.transaction()` takes the DECLARED + * non-transactional path (ADR-0119 D1) instead of opening one — the + * degrade warns once per datasource, and a caller that passes + * `require: true` gets `TransactionUnsupportedError` BEFORE the callback + * writes anything. Both are the exact answers a driver with no + * `beginTransaction` already gets; this bit is only how a driver that + * INHERITED the method joins them. + * + * ⛔ Not a way to say "transactions are off right now". It describes the + * transport, is read at dispatch time, and a driver whose answer can change + * per call should not be answering here at all. + * + * Optional; absence means `false`, exactly like `batchSchemaSync` — the + * whole point is that a driver which declares nothing keeps the behaviour it + * has today. + */ + transactionsUnsupported: z.boolean().optional().describe('Transport cannot honour transactions even though `beginTransaction` is inherited (absence = false)'), + // ============================================================================ // Retired capability bits (#4634, ADR-0049 enforce-or-remove) — 17.0.0 // ============================================================================ @@ -263,11 +309,18 @@ export const DriverCapabilitiesSchema = lazySchema(() => z.object({ + '(#3298), never from this record.')), transactions: retiredKey(capRemoved('transactions', - 'Transaction use is gated on METHOD PRESENCE — `driver.beginTransaction` ' - + '(`engine.transaction()`, ADR-0034 ambient transactions): a driver without the method ' - + 'gets the non-transactional fallback, whatever this bit claimed. Discovery\'s ' + 'Transaction use is gated on the DRIVER\'S DECLARATION, no longer on METHOD PRESENCE ' + + 'alone: `engine.transaction()` asks `driverSupportsTransactions(driver)` — ' + + '`driver.beginTransaction` present AND `transactionsUnsupported` not set (ADR-0034 ' + + 'ambient transactions, ADR-0119 D1). A driver without the method — or a transport ' + + 'that declares that live bit — gets the non-transactional fallback, whatever this ' + + 'bit claimed. Discovery\'s ' + '`transactionalBatch` capability is likewise derived from `engine.transaction` plus the ' - + 'mounted batch route, never from this bit.')), + + 'mounted batch route, never from this bit. The live `transactionsUnsupported` bit is NOT ' + + 'this key restored and is not its opposite spelled differently: this one CLAIMED support ' + + 'nothing checked, that one DENIES support the engine does check, and it is written only by ' + + 'a transport that inherits `beginTransaction` from a base class it cannot honour. A driver ' + + 'with real transactions declares nothing.')), savepoints: retiredKey(capRemoved('savepoints', 'No savepoint code path exists in the engine — a capability bit for a feature the ' + 'platform does not call is a false affordance, not documentation.')), @@ -632,6 +685,12 @@ export const DriverInterfaceSchema = lazySchema(() => z.object({ /** * Begin a new database transaction. + * + * ⛔ Declaring this member is not a claim that the transport honours a + * handle — a subclass inherits it. A transport whose data methods cannot + * receive `options.transaction` declares `transactionsUnsupported` above, + * and the engine never calls this on it. + * * @param options - Isolation level and other settings. * @returns A transaction handle to be passed to subsequent operations via `options.transaction`. */ @@ -758,3 +817,41 @@ export type DriverConfigParsed = z.infer; export type PoolConfig = z.input; /** Post-parse shape of {@link PoolConfig} — defaults applied, transforms run (ADR-0122). */ export type PoolConfigParsed = z.infer; + +/** + * Can the engine open a transaction on this driver? + * + * ONE definition of the invariant, in the package that declares it, so the + * engine's three transaction entrances cannot drift apart — the same shape + * `isMultiValueField` holds for `FieldSchema.multiple`. Every caller that used + * to write `typeof driver.beginTransaction === 'function'` asks this instead. + * + * Two clauses, and they answer different questions: + * + * 1. **The method is there.** A driver with no `beginTransaction` (the + * in-memory driver, a test double, a foreign engine) has nothing to open. + * This clause is unchanged from the pre-`transactionsUnsupported` gate and + * is why the bit's absence keeps every existing driver exactly as it was. + * 2. **The transport did not deny it.** `supports.transactionsUnsupported` + * is how a subclass that INHERITED the method says its own transport + * cannot honour it. Without this clause a class cannot opt out of a door + * it did not open — which is the whole reason a declaration exists here + * rather than a second method-presence test. + * + * ⛔ The answer is NOT a promise that a `beginTransaction()` call will + * succeed — a live transport can still fail. It is the dispatch question: + * should the engine take the transactional path at all, or the DECLARED + * non-transactional one (ADR-0119 D1)? + */ +export function driverSupportsTransactions( + driver: + | { + beginTransaction?: unknown; + supports?: { transactionsUnsupported?: boolean | undefined } | undefined; + } + | null + | undefined, +): boolean { + if (typeof driver?.beginTransaction !== 'function') return false; + return driver.supports?.transactionsUnsupported !== true; +}