diff --git a/.changeset/27666-datasource-def-driver-id.md b/.changeset/27666-datasource-def-driver-id.md new file mode 100644 index 00000000000..44f8d637b42 --- /dev/null +++ b/.changeset/27666-datasource-def-driver-id.md @@ -0,0 +1,22 @@ +--- +'@objectstack/spec': patch +'@objectstack/service-datasource': patch +--- + +fix(service-datasource): a plugin-driver datasource connected or reconnected through the connection service keeps the engine's correctness floor (#27666) + +Clause-②: yes (narrowing) — `EngineDatasourceDef` (`@objectstack/spec/contracts`) gains an optional `driver` key, and a plugin-driver datasource connected through the datasource-connection service is now held behind the #27277 floor, so its accepted reads and writes narrow as that floor's changeset describes +Metadata-protocol: none + + + +**BREAKING** for a host whose datasource-driver factory builds plugin drivers: those datasources are now floored (a read matching more than 1000 rows answers 422 `EXTERNAL_ROW_CAP_EXCEEDED`, predicate bulk writes and `aggregate` are refused by name), as manifest-declared ones already were. + +The engine holds a plugin driver on a non-managed datasource behind a correctness floor (row cap, the query re-applied in memory, remote write refusals mapped) when the datasource definition it stores names a driver id that is not built in. Only the engine's own manifest install path carried that id. The datasource-connection service registered the definition without it, and `EngineDatasourceDef` had no such key, so a datasource created at runtime through datasource management was never floored, and one edited and reconnected lost the floor (the eviction drops the stored definition, and the connect that follows had no id to keep). Nothing said so: a driver that ignored `where` could answer rows outside the caller's sharing or row-level scope. + +- `EngineDatasourceDef` (`@objectstack/spec/contracts`) gains an optional `driver`, the datasource's driver id. An engine never reads an absent key as "built in": a caller that registers only the write-gate facts keeps the id stored before it. +- `DatasourceConnectionService.connect` registers the definition with `record.driver`. `reconnect`'s keep-old-pool restore registers the OLD record's driver id (`previous.driver`; `previous` now carries it), not the new record's, because the kept pool was built from the old record. + +A built-in driver (memory, sqlite, sqlite-wasm, postgres, mysql, mongodb, turso) and a managed datasource are unchanged: the floor engages only for a driver id a server plugin published on a datasource whose `schemaMode` is not `managed`. A host that registers no driver factory for plugin ids (the default) sees no change today. + +What to do: nothing. A host whose factory builds plugin drivers gets the floor on those datasources from the connection service's next connect. diff --git a/packages/objectql/src/driver-query-in-memory.ts b/packages/objectql/src/driver-query-in-memory.ts index 03cf61d237a..6fed9942ea6 100644 --- a/packages/objectql/src/driver-query-in-memory.ts +++ b/packages/objectql/src/driver-query-in-memory.ts @@ -3,7 +3,7 @@ /** * `applyDriverQueryInMemory` — the engine's own reading of a driver query over * rows already in memory: `where`, `orderBy`, `offset` / `limit` and `fields`, - * in that order (ADR-0015 Tier A, epic #27261 P1). + * in that order (epic #27261 Tier A, P1). * * ## Why the engine owns this * diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 68aa844759f..d3d70a82fdd 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -9247,10 +9247,13 @@ export class ObjectQL implements IObjectQLEngine { */ registerDatasourceDef(def: DatasourceDef): void { if (!def?.name) return; - // [#27277] A def that names no driver keeps the id stored before it: the - // datasource-connection service re-registers a def without `driver` after - // a connect (`EngineDatasourceDef` has no such key), and reading that - // absence as "not a plugin driver" would take the floor back off. + // [#27277] A def that names no driver keeps the id stored before it: a + // caller registering only the write-gate facts after a connect must not + // take the floor back off, so an absent key never reads as "not a plugin + // driver". It is a keep, not a source: `unregisterDriver` drops the stored + // def, so a connect that follows an eviction has nothing to keep — the + // datasource-connection service names `driver` on every registration + // ([#27666], `EngineDatasourceDef.driver`). const driver = def.driver ?? this.datasourceDefs.get(def.name)?.driver; this.datasourceDefs.set(def.name, { driver, schemaMode: def.schemaMode, external: def.external }); this.applyExternalDriverFloor(def.name); diff --git a/packages/objectql/src/external-driver-errors.ts b/packages/objectql/src/external-driver-errors.ts index 474ea7a68fa..524ef3a5f2f 100644 --- a/packages/objectql/src/external-driver-errors.ts +++ b/packages/objectql/src/external-driver-errors.ts @@ -2,7 +2,7 @@ /** * [#27277] The refusals the engine's floor over a plugin driver raises - * (`external-driver-floor.ts`, ADR-0015 Tier A, epic #27261 P1). + * (`external-driver-floor.ts`, epic #27261 Tier A, P1; ruling 6487532394 on #27277). * * All are 4xx: the caller can change the request and try again, and the * message is theirs to read. That is what keeps them apart from the third diff --git a/packages/objectql/src/external-driver-floor.test.ts b/packages/objectql/src/external-driver-floor.test.ts index 007028a5796..2715ee72f3a 100644 --- a/packages/objectql/src/external-driver-floor.test.ts +++ b/packages/objectql/src/external-driver-floor.test.ts @@ -485,3 +485,102 @@ describe('[#27277] a re-registration that names no driver keeps the floor', () = expect(ids(await engine.find('tree_node', { where: { kind: 'branch' } }))).toEqual(['n2', 'n5']); }); }); + +describe('[#27666] the datasource-connection service\'s registrations floor a plugin driver, and a reconnect keeps it', () => { + const TREE_PKG = { + id: 'tree_pkg', + name: 'Tree', + objects: [{ + name: 'tree_node', + datasource: 'tree', + external: { remoteName: 'nodes', writable: true }, + fields: { kind: { type: 'text' }, label: { type: 'text' }, weight: { type: 'number' } }, + }], + }; + /** What `DatasourceConnectionService.connect` registers after `registerDriver`: the def WITH its driver id. */ + const connectDef = (driver: string) => ({ name: 'tree', driver, schemaMode: 'external', external: { allowWrites: true } }); + + it('a runtime-origin datasource (no manifest def): driver, then the connect def naming the driver id', async () => { + const engine = new ObjectQL(); + const remote = remoteDriver('tree', TREE_ROWS); + engine.registerDriver(remote.driver); + engine.registerDatasourceDef(connectDef('spike_tree')); + engine.registerApp(TREE_PKG as any); + const held = engine.getDriverByName('tree')!; + expect(isExternalDriverFloor(held)).toBe(true); + expect(unwrapExternalDriverFloor(held)).toBe(remote.driver); + expect(ids(await engine.find('tree_node', { where: { kind: 'branch' } }))).toEqual(['n2', 'n5']); + }); + + it('a reconnect (unregisterDriver, then the connect sequence) is still floored', async () => { + const engine = new ObjectQL(); + const first = remoteDriver('tree', TREE_ROWS); + engine.registerDriver(first.driver); + engine.registerDatasourceDef(connectDef('spike_tree')); + engine.registerApp(TREE_PKG as any); + expect(isExternalDriverFloor(engine.getDriverByName('tree')!)).toBe(true); + + // `reconnect`: the eviction door drops the driver AND the stored def … + expect(engine.unregisterDriver('tree')).toBe(true); + expect(engine.getDriverByName('tree')).toBeUndefined(); + // … then `connect` registers the rebuilt pool and the def again, driver id included. + const rebuilt = remoteDriver('tree', TREE_ROWS); + engine.registerDriver(rebuilt.driver); + engine.registerDatasourceDef(connectDef('spike_tree')); + + const held = engine.getDriverByName('tree')!; + expect(isExternalDriverFloor(held)).toBe(true); + expect(unwrapExternalDriverFloor(held)).toBe(rebuilt.driver); + expect(ids(await engine.find('tree_node', { where: { kind: 'branch' } }))).toEqual(['n2', 'n5']); + expect(rebuilt.sent).toHaveLength(1); + }); + + it('the failed-rebuild restore (unregisterDriver, then the OLD pool and its def back) is still floored', async () => { + const engine = new ObjectQL(); + const old = remoteDriver('tree', TREE_ROWS); + engine.registerDriver(old.driver); + engine.registerDatasourceDef(connectDef('spike_tree')); + engine.registerApp(TREE_PKG as any); + + engine.unregisterDriver('tree'); + // `restoreOldPool`: the kept driver instance and the def it was serving under, `previous.driver` included. + engine.registerDriver(old.driver, false); + engine.registerDatasourceDef(connectDef('spike_tree')); + + const held = engine.getDriverByName('tree')!; + expect(isExternalDriverFloor(held)).toBe(true); + expect(unwrapExternalDriverFloor(held)).toBe(old.driver); + expect(ids(await engine.find('tree_node', { where: { kind: 'branch' } }))).toEqual(['n2', 'n5']); + }); + + it('control: after the eviction a re-registration that names no driver has nothing to keep, and the where-ignoring driver answers every row', async () => { + // The defect's mechanism, pinned so the positive cases above are known to + // depend on the driver id reaching `registerDatasourceDef`: the engine's + // "keep the stored id" only keeps what the eviction left, which is nothing. + const engine = new ObjectQL(); + const first = remoteDriver('tree', TREE_ROWS); + engine.registerDriver(first.driver); + engine.registerDatasourceDef(connectDef('spike_tree')); + engine.registerApp(TREE_PKG as any); + engine.unregisterDriver('tree'); + const rebuilt = remoteDriver('tree', TREE_ROWS); + engine.registerDriver(rebuilt.driver); + engine.registerDatasourceDef({ name: 'tree', schemaMode: 'external', external: { allowWrites: true } }); + expect(engine.getDriverByName('tree')).toBe(rebuilt.driver); + expect(ids(await engine.find('tree_node', { where: { kind: 'branch' } }))).toHaveLength(TREE_ROWS.length); + }); + + it('a built-in driver through the same connect and reconnect sequences stays the registered instance itself', () => { + const engine = new ObjectQL(); + const first = remoteDriver('tree', TREE_ROWS); + engine.registerDriver(first.driver); + engine.registerDatasourceDef(connectDef('sqlite')); + engine.registerApp(TREE_PKG as any); + expect(engine.getDriverByName('tree')).toBe(first.driver); + engine.unregisterDriver('tree'); + const rebuilt = remoteDriver('tree', TREE_ROWS); + engine.registerDriver(rebuilt.driver); + engine.registerDatasourceDef(connectDef('sqlite')); + expect(engine.getDriverByName('tree')).toBe(rebuilt.driver); + }); +}); diff --git a/packages/objectql/src/external-driver-floor.ts b/packages/objectql/src/external-driver-floor.ts index 49812cf4b92..06f825703ff 100644 --- a/packages/objectql/src/external-driver-floor.ts +++ b/packages/objectql/src/external-driver-floor.ts @@ -2,7 +2,7 @@ /** * [#27277] The engine's correctness floor over a plugin driver on an external - * datasource (ADR-0015 Tier A, epic #27261 P1). + * datasource (epic #27261 Tier A, P1; ruling 6487532394 on #27277). * * ## Which drivers * diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index ded28d22b40..2ab9da9c453 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -300,7 +300,7 @@ export type { PrimaryDatasourceUnresolvedReason, } from './driver-connect-errors.js'; // [#27277] The engine's floor over a plugin driver on an external datasource -// (ADR-0015 Tier A): the in-memory reading of a driver query it applies, and +// (epic #27261 Tier A): the in-memory reading of a driver query it applies, and // the two 422 refusals it raises (registered in ERROR_CODE_LEDGER), plus // [#27471] the 400 read refusal (the standard INVALID_QUERY). export { applyDriverQueryInMemory } from './driver-query-in-memory.js'; diff --git a/packages/services/service-datasource/src/__tests__/datasource-connection-service.test.ts b/packages/services/service-datasource/src/__tests__/datasource-connection-service.test.ts index 1b9117f9680..4d794d002db 100644 --- a/packages/services/service-datasource/src/__tests__/datasource-connection-service.test.ts +++ b/packages/services/service-datasource/src/__tests__/datasource-connection-service.test.ts @@ -195,8 +195,13 @@ describe('DatasourceConnectionService.connect', () => { // Driver registered under the DATASOURCE name (engine routes by driver.name). expect(engine!.drivers.has('warehouse')).toBe(true); expect(engine!.drivers.get('warehouse')!.name).toBe('warehouse'); - // Datasource definition recorded for the write gate. - expect(engine!.defs).toEqual([{ name: 'warehouse', schemaMode: 'external', external: externalDs.external }]); + // Datasource definition recorded for the write gate — and, [#27666], with + // the driver id the engine keys its plugin-driver floor on: for a + // runtime-origin datasource this registration is the only one the engine + // ever sees, so a def without `driver` would serve a plugin driver unfloored. + expect(engine!.defs).toEqual([ + { name: 'warehouse', driver: 'sqlite', schemaMode: 'external', external: externalDs.external }, + ]); // Bound external objects got read metadata synced (DDL-free). expect(engine!.synced).toEqual(['ext_customer']); }); diff --git a/packages/services/service-datasource/src/__tests__/datasource-update-rebuild.test.ts b/packages/services/service-datasource/src/__tests__/datasource-update-rebuild.test.ts index 5920cc19f93..ac42d61b0d2 100644 --- a/packages/services/service-datasource/src/__tests__/datasource-update-rebuild.test.ts +++ b/packages/services/service-datasource/src/__tests__/datasource-update-rebuild.test.ts @@ -334,13 +334,50 @@ describe('#13804 — updateDatasource rebuilds the live pool when connectivity c const state = rig.connection.getConnectionState('analytics')!; expect(state.status).toBe('failed-degraded'); // 3. Not pool-less: the datasource def the old pool was serving under is - // restored alongside the driver (`unregisterDriver` removes both). + // restored alongside the driver (`unregisterDriver` removes both) — + // [#27666] its driver id included, which is what the engine keys its + // plugin-driver floor on. expect(rig.engine.defs.get('analytics')).toMatchObject({ name: 'analytics', + driver: 'sqlite', schemaMode: 'external', }); }); + it('[#27666] the def registered after a rebuild names the NEW driver id; the def restored after a failed rebuild names the OLD one', async () => { + // The engine floors a non-managed datasource whose def names a driver id + // that is not built in. `unregisterDriver` drops the stored def, so the + // id has to travel with every registration this path makes — the rebuilt + // pool's from `next`, the kept pool's from `previous` — or a plugin + // driver is served unfloored after a reconfigure, and nothing says so. + const rig = makeRig({ + failWhen: (spec) => (spec.config as { filename?: string }).filename === '/tmp/bad.db', + }); + await rig.service.createDatasource( + draft({ driver: 'driver.port_schedule', schemaMode: 'external', external: { allowWrites: false } }), + ); + expect(rig.engine.defs.get('analytics')).toMatchObject({ driver: 'driver.port_schedule', schemaMode: 'external' }); + + // A reconfigure that changes the driver and succeeds: the def names the new id. + const rebuilt = await rig.service.updateDatasource('analytics', { + driver: 'driver.port_schedule_v2', + config: { filename: '/tmp/new.db' }, + }); + expect(rebuilt.status).toBe('ok'); + expect(rig.engine.drivers.get('analytics')!.builtFrom.driver).toBe('driver.port_schedule_v2'); + expect(rig.engine.defs.get('analytics')).toMatchObject({ driver: 'driver.port_schedule_v2', schemaMode: 'external' }); + + // A reconfigure that changes the driver and fails: the kept pool's def + // names the id it was built from, not the one that never came up. + const failed = await rig.service.updateDatasource('analytics', { + driver: 'driver.port_schedule_v3', + config: { filename: '/tmp/bad.db' }, + }); + expect(failed.status).toBe('error'); + expect(rig.engine.drivers.get('analytics')!.builtFrom.driver).toBe('driver.port_schedule_v2'); + expect(rig.engine.defs.get('analytics')).toMatchObject({ driver: 'driver.port_schedule_v2', schemaMode: 'external' }); + }); + it('a supplied secret alone forces a rebuild — rewrap-in-place changes what the ref dereferences to', async () => { const rig = makeRig({ stableSecretRef: true }); await rig.service.createDatasource( diff --git a/packages/services/service-datasource/src/datasource-connection-service.ts b/packages/services/service-datasource/src/datasource-connection-service.ts index aadc4ba25a2..42b948795bc 100644 --- a/packages/services/service-datasource/src/datasource-connection-service.ts +++ b/packages/services/service-datasource/src/datasource-connection-service.ts @@ -666,8 +666,13 @@ export class DatasourceConnectionService { : true; engine.registerDriver(engineDriver, opts.asDefault === true); if (!heldBefore) registeredByThisAttempt = engineDriver.name; + // [#27666] `driver` rides along: the engine keys its plugin-driver floor + // on the def's driver id, and this registration is the ONLY one a + // runtime-origin datasource gets (the manifest path never sees it), and + // the only one left after `reconnect`'s eviction dropped the stored def. engine.registerDatasourceDef?.({ name, + driver: record.driver, schemaMode: record.schemaMode, external: record.external as { allowWrites?: boolean } | undefined, }); @@ -830,7 +835,7 @@ export class DatasourceConnectionService { record: ConnectableDatasource, opts: { context?: DatasourceConnectContext; - previous?: Pick; + previous?: Pick; } = {}, ): Promise { const name = record.name; @@ -848,8 +853,13 @@ export class DatasourceConnectionService { const restoreOldPool = (): void => { engine.registerDriver?.(oldDriver, false); + // [#27666] The OLD record's driver id, not `record.driver`: the kept + // pool was built from `previous`, and a reconfigure may have changed the + // driver. Without it the engine would read the restored def as "not a + // plugin driver" and serve the kept pool unfloored. engine.registerDatasourceDef?.({ name, + driver: opts.previous?.driver, schemaMode: opts.previous?.schemaMode, external: opts.previous?.external as { allowWrites?: boolean } | undefined, }); diff --git a/packages/spec/src/api/error-code-ledger.zod.ts b/packages/spec/src/api/error-code-ledger.zod.ts index 06daa0eb91b..e1c7b47ea51 100644 --- a/packages/spec/src/api/error-code-ledger.zod.ts +++ b/packages/spec/src/api/error-code-ledger.zod.ts @@ -830,7 +830,8 @@ export const ERROR_CODE_LEDGER = { // ADR-0119 D1/D4 fail-closed posture). Same #8087-gate family. 'ERR_TRANSACTION_UNSUPPORTED', // [#27277] The engine's floor over a plugin driver on an external - // datasource (ADR-0015 Tier A, `external-driver-floor.ts`). Both are 422 + // datasource (epic #27261 Tier A, ruling 6487532394 on #27277; + // `external-driver-floor.ts`). Both are 422 // with their status declared on the thrown error (the // `driver-connect-errors.ts` pattern), and neither is an ADR-0015 gate // refusal: `EXTERNAL_WRITE_FORBIDDEN` (403, `@objectstack/spec/shared`) @@ -847,6 +848,15 @@ export const ERROR_CODE_LEDGER = { // driver returns, so it refuses rather than answer over a set it only // partly holds. The message prescribes narrowing the filter or mirroring // the data with DataSync. + // + // Deliberately NOT the standard `MAX_RECORDS_EXCEEDED` (400, `errors.zod.ts`: + // a caller-side limit, the request asked for more than a request may + // carry). This is a 422 on a VALID query that the remote's size makes + // unservable: the request is well-formed and its limit is fine, the + // matching set is what cannot be held, and the remedy is the data's + // shape (narrow the filter, mirror the data) rather than the request's + // form. Ruling 6487532394 on #27277 chose the code and the status; + // `standardSynonymOf` deliberately maps neither of these two rows. 'EXTERNAL_ROW_CAP_EXCEEDED', // [#15823] an `afterFind` handler REPLACED `ctx.result` with something that // is not an array, breaking `find()`'s declared `Promise`. It was the diff --git a/packages/spec/src/contracts/data-engine.test.ts b/packages/spec/src/contracts/data-engine.test.ts index 4f558a56578..76042cfb4d4 100644 --- a/packages/spec/src/contracts/data-engine.test.ts +++ b/packages/spec/src/contracts/data-engine.test.ts @@ -603,6 +603,15 @@ describe('Data Engine Contract', () => { schemaMode: 'read-only', external: { allowWrites: false, credentialsRef: 'secrets/warehouse' }, }); + // [#27666] The driver id travels with the def: the engine keys its + // plugin-driver floor on it, and the datasource-connection service's + // registration is the only one a runtime-origin datasource gets (and + // the only one left after a reconnect's eviction). Before this key the + // literal below was refused here with TS2353 while the engine already + // read and kept the value from its own manifest path. + register({ name: 'warehouse', driver: 'driver.port_schedule', schemaMode: 'external' }); + // @ts-expect-error - the driver id is a string, never a driver instance + register({ name: 'warehouse', driver: { name: 'warehouse' }, schemaMode: 'external' }); // @ts-expect-error - a datasource definition without a name registers nothing register({ schemaMode: 'read-only' }); // The widening is exactly the ruled key, not an open door: an inline diff --git a/packages/spec/src/contracts/data-engine.ts b/packages/spec/src/contracts/data-engine.ts index d725f7dc971..67bd5a90f6e 100644 --- a/packages/spec/src/contracts/data-engine.ts +++ b/packages/spec/src/contracts/data-engine.ts @@ -240,6 +240,22 @@ export interface WriteObservabilityOptions { */ export interface EngineDatasourceDef { name: string; + /** + * The driver id the datasource selects (`driver` on the authored + * datasource: a built-in id such as `postgres`, or the `driver.NAME` id a + * server plugin published). The engine reads it to decide whether the + * datasource's driver is held behind its correctness floor (a non-managed + * `schemaMode` on a driver id that is not built in): a plugin driver is + * not trusted to have applied the query, so the engine re-applies it in + * memory and refuses over the row cap. Only the engine's own manifest + * install path used to carry the id, so a datasource registered through + * this member — the datasource-connection service on a connect, and again + * after a reconnect, whose eviction drops the stored def — read as "not a + * plugin driver" and was served unfloored. Optional because a caller that + * registers only the write-gate facts keeps the id stored before it; the + * engine never reads an absent key as "built in". + */ + driver?: string; schemaMode?: string; external?: { /** Datasource-wide write gate — ADR-0015 §5.3 Gate 3. */