fix(service-datasource)!: reconnect requires previous, and the engine read-back answers the stored driver id

.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
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any authored schema is added, removed, renamed or re-shaped: the narrowed parameter is on a host-facing service method, and the widened answer is an engine read-back. A datasource needs no metadata change, so there is nothing for a migration to rewrite. -->
+
+**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.

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.

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 } } : {}),
     }));

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',
@@ -584,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' }]);
+  });
+});

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[engine.defs.length - 1]).toMatchObject({ name: 'tree', driver: 'spike_tree', schemaMode: 'external' });
+  });
+});

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<ConnectableDatasource, 'driver' | 'schemaMode' | 'external'>;
-    } = {},
+      previous: Pick<ConnectableDatasource, 'driver' | 'schemaMode' | 'external'>;
+    },
   ): Promise<ConnectResult> {
     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 =

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;