From 4b9db28078410f9738e7477a4617b527b6bff335 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 22:14:24 +0000 Subject: [PATCH 1/3] wip: tidy the EngineDatasourceDef.driver contract Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2fhwqSBmy1XBXfcoU1 --- .../docs/data-modeling/virtual-objects.mdx | 3 +- packages/objectql/src/engine.ts | 8 ++- .../src/external-driver-floor.test.ts | 8 ++- .../datasource-connection-service.test.ts | 55 +++++++++++++++++++ .../src/datasource-connection-service.ts | 24 ++++++-- packages/spec/src/contracts/data-engine.ts | 8 ++- 6 files changed, 92 insertions(+), 14 deletions(-) diff --git a/content/docs/data-modeling/virtual-objects.mdx b/content/docs/data-modeling/virtual-objects.mdx index 6781a8ed81b..4dcfd873454 100644 --- a/content/docs/data-modeling/virtual-objects.mdx +++ b/content/docs/data-modeling/virtual-objects.mdx @@ -631,5 +631,4 @@ separate data-sync metadata type: a sync is defined on its target, as that mappi - [Hooks](/docs/automation/hooks) and [Hook bodies](/docs/automation/hook-bodies) — the `beforeUpdate` guard. - [Views](/docs/ui/views) — the gantt and every other list view. -- ADR-0015 (federation spec) — the plugin driver is its Tier A: a driver, zero protocol - change. +- ADR-0015 (federation spec) — `schemaMode`, the `external` binding and the write gates. diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 017bca28945..543fe0beb94 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -9244,7 +9244,8 @@ export class ObjectQL implements IObjectQLEngine { * `external.allowWrites` so the write gate ({@link assertWriteAllowed}) can * enforce external-datasource ownership, and `external.credentialsRef` so * {@link listDatasourceDefs} can hand a credentials sweep the handle a - * code-declared datasource holds. Safe to call repeatedly; last write wins. + * code-declared datasource holds. Safe to call repeatedly; last write wins, + * except an absent `driver` keeps the id stored before it. */ registerDatasourceDef(def: DatasourceDef): void { if (!def?.name) return; @@ -9311,10 +9312,15 @@ export class ObjectQL implements IObjectQLEngine { * Each entry is a fresh object with a COPIED `external` block. The index * stores the caller's `external` by reference, and a reader must not be able * to reach through this accessor and mutate the write gate's own input. + * + * [#27727] The stored `driver` id is answered too: it is what decides whether + * the datasource's driver is floored, so a sweep asking "which datasources + * name a plugin driver" can be written against the contract's read-back. */ listDatasourceDefs(): DatasourceDef[] { return Array.from(this.datasourceDefs, ([name, def]) => ({ name, + ...(def.driver !== undefined ? { driver: def.driver } : {}), ...(def.schemaMode !== undefined ? { schemaMode: def.schemaMode } : {}), ...(def.external ? { external: { ...def.external } } : {}), })); diff --git a/packages/objectql/src/external-driver-floor.test.ts b/packages/objectql/src/external-driver-floor.test.ts index 2715ee72f3a..86b336532af 100644 --- a/packages/objectql/src/external-driver-floor.test.ts +++ b/packages/objectql/src/external-driver-floor.test.ts @@ -460,13 +460,15 @@ describe('[#27277] predicate writes are not offered: every write reaches the dri }); describe('[#27277] a re-registration that names no driver keeps the floor', () => { - it('the datasource-connection sequence: def with driver, driver, then a def without driver', async () => { + it('a caller registering only the write-gate facts: def with driver, driver, then a def without driver', async () => { const engine = new ObjectQL(); const remote = remoteDriver('tree', TREE_ROWS); engine.registerDatasourceDef({ name: 'tree', driver: 'spike_tree', schemaMode: 'external', external: { allowWrites: true } }); engine.registerDriver(remote.driver); - // What `datasource-connection-service.ts` registers after a connect and in - // `restoreOldPool`: no `driver` key (`EngineDatasourceDef` has none). + // A caller that registers only the write-gate facts (`schemaMode` and + // `external`) and names no `driver`. The datasource-connection service is + // not that caller: it names `driver` on every registration (the [#27666] + // block below). engine.registerDatasourceDef({ name: 'tree', schemaMode: 'external', external: { allowWrites: true } }); engine.registerApp({ id: 'tree_pkg', 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 4d794d002db..7bfe9fb3d06 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 @@ -1362,3 +1362,58 @@ describe('retained connection state (framework#3827)', () => { ]); }); }); + +describe('[#27727] reconnect requires `previous`, the record the live pool was built from', () => { + const pluginDs: ConnectableDatasource = { + name: 'tree', + driver: 'spike_tree', + schemaMode: 'external', + external: { allowWrites: true }, + }; + const ctx = { origin: 'runtime', trigger: 'runtime-admin' } as const; + + it('a call without `previous` is refused before anything is evicted or rebuilt', async () => { + const { service, engine, factory } = svc(); + await service.connect(pluginDs, { context: ctx }); + const live = engine!.drivers.get('tree'); + expect(live).toBeDefined(); + const defsBefore = engine!.defs.length; + + // @ts-expect-error — `previous` is required: without its `driver` a kept pool would be restored unfloored + await expect(service.reconnect(pluginDs, { context: ctx })).rejects.toThrow(/reconnect needs `previous`/); + await expect( + service.reconnect(pluginDs, { context: ctx, previous: { driver: '' } }), + ).rejects.toThrow(/with its `driver` id/); + + expect(engine!.drivers.get('tree')).toBe(live); + expect(engine!.defs).toHaveLength(defsBefore); + expect(factory!.create).toHaveBeenCalledTimes(1); + }); + + it('with `previous`, a failed rebuild restores the kept pool under the OLD driver id', async () => { + const engine = fakeEngine(); + let builds = 0; + const factory: IDatasourceDriverFactory = { + supports: () => true, + create: vi.fn(async () => { + builds += 1; + const driver: any = { name: 'com.fake.driver' }; + return { + driver, + connect: builds === 1 + ? async () => { driver.connected = true; } + : async () => { throw new Error('connection refused'); }, + }; + }), + }; + const { service } = svc({ engine, factory }); + await service.connect(pluginDs, { context: ctx }); + const live = engine.drivers.get('tree'); + + const next: ConnectableDatasource = { ...pluginDs, driver: 'sqlite', config: { filename: '/tmp/t.db' } }; + await service.reconnect(next, { context: ctx, previous: pluginDs }); + + expect(engine.drivers.get('tree')).toBe(live); + expect(engine.defs.at(-1)).toMatchObject({ name: 'tree', driver: 'spike_tree', schemaMode: 'external' }); + }); +}); diff --git a/packages/services/service-datasource/src/datasource-connection-service.ts b/packages/services/service-datasource/src/datasource-connection-service.ts index 42b948795bc..163483cb7b8 100644 --- a/packages/services/service-datasource/src/datasource-connection-service.ts +++ b/packages/services/service-datasource/src/datasource-connection-service.ts @@ -825,7 +825,11 @@ export class DatasourceConnectionService { * `unregisterDriver` removes the datasource def together with the driver * (the registry owns that invariant), so the keep-old-pool path must restore * the def the old pool was serving under — otherwise the write gate would - * judge writes for a datasource whose def vanished mid-failure. + * judge writes for a datasource whose def vanished mid-failure. It is + * REQUIRED ([#27727]): the restored def's `driver` is what keeps a kept + * plugin-driver pool behind the engine's correctness floor, and a restore + * that cannot name it would serve that pool unfloored, silently. A call + * without `previous.driver` is refused before anything is evicted. * * Not for the DEFAULT datasource: runtime-admin records can never be named * `default` (the name is host-reserved), and the default driver's natural- @@ -835,10 +839,18 @@ export class DatasourceConnectionService { record: ConnectableDatasource, opts: { context?: DatasourceConnectContext; - previous?: Pick; - } = {}, + previous: Pick; + }, ): Promise { const name = record.name; + if (typeof opts?.previous?.driver !== 'string' || opts.previous.driver === '') { + throw new Error( + `datasource '${name}': reconnect needs \`previous\`, the record the live pool was built from, ` + + 'with its `driver` id — without it a kept pool would be restored under a definition that names ' + + 'no driver, and a plugin driver would be served without the engine\'s correctness floor. ' + + 'Pass the stored record as it was before this update.', + ); + } const engine = this.cfg.engine(); const oldDriver = engine?.getDriverByName?.(name); if (!oldDriver || !engine || typeof engine.unregisterDriver !== 'function') { @@ -859,9 +871,9 @@ export class DatasourceConnectionService { // 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, + driver: opts.previous.driver, + schemaMode: opts.previous.schemaMode, + external: opts.previous.external as { allowWrites?: boolean } | undefined, }); }; const stillServingSuffix = diff --git a/packages/spec/src/contracts/data-engine.ts b/packages/spec/src/contracts/data-engine.ts index 67bd5a90f6e..5798a9c75e5 100644 --- a/packages/spec/src/contracts/data-engine.ts +++ b/packages/spec/src/contracts/data-engine.ts @@ -253,7 +253,9 @@ export interface EngineDatasourceDef { * 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". + * engine never reads an absent key as "built in". {@link + * IDataEngine.listDatasourceDefs} answers the stored id, so a sweep can ask + * which datasources name a plugin driver. */ driver?: string; schemaMode?: string; @@ -468,7 +470,9 @@ export interface IDataEngine { * — the engine accepts and retains the reference since #12758; before * this catch-up a caller typed against THIS contract was refused with * TS2353 for a value the runtime keeps). Distinct from registering a live - * driver connection. Safe to call repeatedly; last write wins. + * driver connection. Safe to call repeatedly; last write wins, except an + * absent `driver` keeps the id stored before it (see + * {@link EngineDatasourceDef.driver}). */ registerDatasourceDef?(def: EngineDatasourceDef): void; From f7a93d4f0ef00ec8253da25bd44dc300610c32ff Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 22:19:02 +0000 Subject: [PATCH 2/3] wip: pins for listDatasourceDefs driver and required reconnect previous Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2fhwqSBmy1XBXfcoU1 --- .../src/external-driver-floor.test.ts | 30 +++++++++++++++++++ .../datasource-connection-service.test.ts | 2 +- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/objectql/src/external-driver-floor.test.ts b/packages/objectql/src/external-driver-floor.test.ts index 86b336532af..364583395b1 100644 --- a/packages/objectql/src/external-driver-floor.test.ts +++ b/packages/objectql/src/external-driver-floor.test.ts @@ -586,3 +586,33 @@ describe('[#27666] the datasource-connection service\'s registrations floor a pl expect(engine.getDriverByName('tree')).toBe(rebuilt.driver); }); }); + +describe('[#27727] listDatasourceDefs answers the stored driver id, the key the floor is decided on', () => { + it('answers the id a registration named, and the id a later registration without one kept', () => { + const engine = new ObjectQL(); + engine.registerDatasourceDef({ name: 'tree', driver: 'spike_tree', schemaMode: 'external', external: { allowWrites: true } }); + expect(engine.listDatasourceDefs()).toEqual([ + { name: 'tree', driver: 'spike_tree', schemaMode: 'external', external: { allowWrites: true } }, + ]); + engine.registerDatasourceDef({ name: 'tree', schemaMode: 'external', external: { allowWrites: false } }); + expect(engine.listDatasourceDefs()).toEqual([ + { name: 'tree', driver: 'spike_tree', schemaMode: 'external', external: { allowWrites: false } }, + ]); + }); + + it('answers the id a package manifest declared', () => { + const engine = new ObjectQL(); + engine.registerApp({ + id: 'tree_pkg', + name: 'Tree', + datasources: [{ name: 'tree', driver: 'spike_tree', schemaMode: 'external', external: { allowWrites: true } }], + } as any); + expect(engine.listDatasourceDefs().find((d) => d.name === 'tree')?.driver).toBe('spike_tree'); + }); + + it('a definition that never named a driver is listed without the key, never as built in', () => { + const engine = new ObjectQL(); + engine.registerDatasourceDef({ name: 'bare', schemaMode: 'external' }); + expect(engine.listDatasourceDefs()).toEqual([{ name: 'bare', schemaMode: 'external' }]); + }); +}); 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 7bfe9fb3d06..f1ee36fa115 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 @@ -1414,6 +1414,6 @@ describe('[#27727] reconnect requires `previous`, the record the live pool was b await service.reconnect(next, { context: ctx, previous: pluginDs }); expect(engine.drivers.get('tree')).toBe(live); - expect(engine.defs.at(-1)).toMatchObject({ name: 'tree', driver: 'spike_tree', schemaMode: 'external' }); + expect(engine.defs[engine.defs.length - 1]).toMatchObject({ name: 'tree', driver: 'spike_tree', schemaMode: 'external' }); }); }); From ae15d7e39b788813cab09e3cf2fff6ea714bbfb4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 22:19:43 +0000 Subject: [PATCH 3/3] chore: changeset for the datasource driver contract tidy-up Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2fhwqSBmy1XBXfcoU1 --- ...727-datasource-def-driver-contract-tidy.md | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .changeset/27727-datasource-def-driver-contract-tidy.md diff --git a/.changeset/27727-datasource-def-driver-contract-tidy.md b/.changeset/27727-datasource-def-driver-contract-tidy.md new file mode 100644 index 00000000000..8528ae3a56f --- /dev/null +++ b/.changeset/27727-datasource-def-driver-contract-tidy.md @@ -0,0 +1,22 @@ +--- +'@objectstack/spec': patch +'@objectstack/objectql': patch +'@objectstack/service-datasource': patch +--- + +fix(service-datasource): `DatasourceConnectionService.reconnect` requires `previous`, and the engine's datasource read-back answers the stored driver id (#27727) + +Clause-②: yes (narrowing) — `DatasourceConnectionService.reconnect` now requires `opts.previous` with its `driver` id and refuses a call without it, and `ObjectQL.listDatasourceDefs` widens its answer by the stored `driver` key +Metadata-protocol: none + + + +**BREAKING** for a host that calls `DatasourceConnectionService.reconnect` itself without `previous`: the call is now a type error and, at runtime, rejects before anything is evicted. The in-repo caller (the datasource admin plugin's update path) already passes it. + +The engine keeps a plugin driver on a non-managed datasource behind its correctness floor only while the stored datasource definition names the driver id. `reconnect` evicts the definition with the driver, and when a rebuild fails it restores the kept pool under a definition built from `previous`. Without `previous` that definition named no driver, so a kept plugin-driver pool was served without the floor, and nothing said so. + +- `DatasourceConnectionService.reconnect`: `opts.previous` (the record the live pool was built from, including its `driver`) is required. A call without it, or with an empty `driver`, rejects with an error naming what to pass, and the live pool is left untouched. +- `ObjectQL.listDatasourceDefs` answers each definition's stored `driver` id when it has one, so a sweep can ask which datasources name a plugin driver. A definition that never named one is still listed without the key. +- `IDataEngine.registerDatasourceDef` (`@objectstack/spec/contracts`) now says what the engine already does: last write wins, except an absent `driver` keeps the id stored before it. + +What to do: a host that calls `reconnect` directly passes the stored record as it was before the update as `previous`. Nothing else changes.