Skip to content

Commit 869fa76

Browse files
fix(service-datasource): a plugin-driver datasource connected or reconnected through the connection service keeps the engine's floor (#27715)
<!-- claude-projects-attribution: {"github_login":"marchtian"} --> _Requested by **田騏** · [project thread](https://claude.ai/code/project/chan_01VmuUhXU9SSUzqJYyXSmYSi?thread=cmsg_01VmuUhXU9SSUzqJYyXSmYSiVy7F2atKWpo5rp3YPrqddh)_ **Fable review owed.** This PR was finished on Opus while the Fable quota was out (maintainer order: finish on Opus, then wait for the contract review). It touches `packages/spec/src/**`, so it does not land until an independent full Fable review and a same-shape `CONTRACT_REVIEW_TIER` record PASS on its current head. Card #27666 (epic #27261), from the post-landing contract review of PR #27403 (6502877381, ③). ## What changes - `EngineDatasourceDef` (`@objectstack/spec/contracts`) gains an optional `driver`, the datasource's driver id. - `DatasourceConnectionService.connect` registers the def with `record.driver`. `reconnect`'s keep-old-pool restore registers the old record's driver id (`previous.driver`). Before this, a plugin-driver datasource built through the connection service was never held behind the #27277 floor, and a reconnect dropped the floor because `unregisterDriver` deletes the stored def. - The engine comment on `registerDatasourceDef` now says the stored id is a keep, not a source. - Riders from the same review (④, comment-only): the `EXTERNAL_ROW_CAP_EXCEEDED` ledger row says why it is deliberately not the standard `MAX_RECORDS_EXCEEDED`, and the "ADR-0015 Tier A" citations in objectql name the epic ruling instead. ## Evidence (local, at `df7e3c66`) - `packages/spec` `src/contracts/data-engine.test.ts`: 36/36. - `packages/objectql` `src/external-driver-floor.test.ts`: 54/54, including the unregister-then-connect sequences. - `packages/services/service-datasource`: connection-service and update-rebuild suites, 114/114. - `tsc --noEmit` clean for spec, objectql and service-datasource. - spec `check:api-surface`, `check:export-origins`, `check:docs`, `check:generated` and `check:error-code-provenance` all exit 0. Implementation: the #27666 card thread's commit (picked onto this branch after the thread stalled on the quota) plus the epic PM's changeset declaration. Clause-②: yes (narrowing) Metadata-protocol: none 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_017ihL2c5z3q4h8inrn5kwqV --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d93d9be commit 869fa76

13 files changed

Lines changed: 224 additions & 13 deletions
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
'@objectstack/spec': patch
3+
'@objectstack/service-datasource': patch
4+
---
5+
6+
fix(service-datasource): a plugin-driver datasource connected or reconnected through the connection service keeps the engine's correctness floor (#27666)
7+
8+
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
9+
Metadata-protocol: none
10+
11+
<!-- adr-0087: not-required (no-migration-prescription) No key of any authored schema is added, removed, renamed or re-shaped; the new key is on an engine-facing contract that the connection service fills itself. A datasource that now gets the floor needs no metadata change, so there is nothing for a migration to rewrite. -->
12+
13+
**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.
14+
15+
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.
16+
17+
- `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.
18+
- `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.
19+
20+
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.
21+
22+
What to do: nothing. A host whose factory builds plugin drivers gets the floor on those datasources from the connection service's next connect.

‎packages/objectql/src/driver-query-in-memory.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
/**
44
* `applyDriverQueryInMemory` — the engine's own reading of a driver query over
55
* rows already in memory: `where`, `orderBy`, `offset` / `limit` and `fields`,
6-
* in that order (ADR-0015 Tier A, epic #27261 P1).
6+
* in that order (epic #27261 Tier A, P1).
77
*
88
* ## Why the engine owns this
99
*

‎packages/objectql/src/engine.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9248,10 +9248,13 @@ export class ObjectQL implements IObjectQLEngine {
92489248
*/
92499249
registerDatasourceDef(def: DatasourceDef): void {
92509250
if (!def?.name) return;
9251-
// [#27277] A def that names no driver keeps the id stored before it: the
9252-
// datasource-connection service re-registers a def without `driver` after
9253-
// a connect (`EngineDatasourceDef` has no such key), and reading that
9254-
// absence as "not a plugin driver" would take the floor back off.
9251+
// [#27277] A def that names no driver keeps the id stored before it: a
9252+
// caller registering only the write-gate facts after a connect must not
9253+
// take the floor back off, so an absent key never reads as "not a plugin
9254+
// driver". It is a keep, not a source: `unregisterDriver` drops the stored
9255+
// def, so a connect that follows an eviction has nothing to keep — the
9256+
// datasource-connection service names `driver` on every registration
9257+
// ([#27666], `EngineDatasourceDef.driver`).
92559258
const driver = def.driver ?? this.datasourceDefs.get(def.name)?.driver;
92569259
this.datasourceDefs.set(def.name, { driver, schemaMode: def.schemaMode, external: def.external });
92579260
this.applyExternalDriverFloor(def.name);

‎packages/objectql/src/external-driver-errors.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
/**
44
* [#27277] The refusals the engine's floor over a plugin driver raises
5-
* (`external-driver-floor.ts`, ADR-0015 Tier A, epic #27261 P1).
5+
* (`external-driver-floor.ts`, epic #27261 Tier A, P1; ruling 6487532394 on #27277).
66
*
77
* All are 4xx: the caller can change the request and try again, and the
88
* message is theirs to read. That is what keeps them apart from the third

‎packages/objectql/src/external-driver-floor.test.ts‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,3 +485,102 @@ describe('[#27277] a re-registration that names no driver keeps the floor', () =
485485
expect(ids(await engine.find('tree_node', { where: { kind: 'branch' } }))).toEqual(['n2', 'n5']);
486486
});
487487
});
488+
489+
describe('[#27666] the datasource-connection service\'s registrations floor a plugin driver, and a reconnect keeps it', () => {
490+
const TREE_PKG = {
491+
id: 'tree_pkg',
492+
name: 'Tree',
493+
objects: [{
494+
name: 'tree_node',
495+
datasource: 'tree',
496+
external: { remoteName: 'nodes', writable: true },
497+
fields: { kind: { type: 'text' }, label: { type: 'text' }, weight: { type: 'number' } },
498+
}],
499+
};
500+
/** What `DatasourceConnectionService.connect` registers after `registerDriver`: the def WITH its driver id. */
501+
const connectDef = (driver: string) => ({ name: 'tree', driver, schemaMode: 'external', external: { allowWrites: true } });
502+
503+
it('a runtime-origin datasource (no manifest def): driver, then the connect def naming the driver id', async () => {
504+
const engine = new ObjectQL();
505+
const remote = remoteDriver('tree', TREE_ROWS);
506+
engine.registerDriver(remote.driver);
507+
engine.registerDatasourceDef(connectDef('spike_tree'));
508+
engine.registerApp(TREE_PKG as any);
509+
const held = engine.getDriverByName('tree')!;
510+
expect(isExternalDriverFloor(held)).toBe(true);
511+
expect(unwrapExternalDriverFloor(held)).toBe(remote.driver);
512+
expect(ids(await engine.find('tree_node', { where: { kind: 'branch' } }))).toEqual(['n2', 'n5']);
513+
});
514+
515+
it('a reconnect (unregisterDriver, then the connect sequence) is still floored', async () => {
516+
const engine = new ObjectQL();
517+
const first = remoteDriver('tree', TREE_ROWS);
518+
engine.registerDriver(first.driver);
519+
engine.registerDatasourceDef(connectDef('spike_tree'));
520+
engine.registerApp(TREE_PKG as any);
521+
expect(isExternalDriverFloor(engine.getDriverByName('tree')!)).toBe(true);
522+
523+
// `reconnect`: the eviction door drops the driver AND the stored def …
524+
expect(engine.unregisterDriver('tree')).toBe(true);
525+
expect(engine.getDriverByName('tree')).toBeUndefined();
526+
// … then `connect` registers the rebuilt pool and the def again, driver id included.
527+
const rebuilt = remoteDriver('tree', TREE_ROWS);
528+
engine.registerDriver(rebuilt.driver);
529+
engine.registerDatasourceDef(connectDef('spike_tree'));
530+
531+
const held = engine.getDriverByName('tree')!;
532+
expect(isExternalDriverFloor(held)).toBe(true);
533+
expect(unwrapExternalDriverFloor(held)).toBe(rebuilt.driver);
534+
expect(ids(await engine.find('tree_node', { where: { kind: 'branch' } }))).toEqual(['n2', 'n5']);
535+
expect(rebuilt.sent).toHaveLength(1);
536+
});
537+
538+
it('the failed-rebuild restore (unregisterDriver, then the OLD pool and its def back) is still floored', async () => {
539+
const engine = new ObjectQL();
540+
const old = remoteDriver('tree', TREE_ROWS);
541+
engine.registerDriver(old.driver);
542+
engine.registerDatasourceDef(connectDef('spike_tree'));
543+
engine.registerApp(TREE_PKG as any);
544+
545+
engine.unregisterDriver('tree');
546+
// `restoreOldPool`: the kept driver instance and the def it was serving under, `previous.driver` included.
547+
engine.registerDriver(old.driver, false);
548+
engine.registerDatasourceDef(connectDef('spike_tree'));
549+
550+
const held = engine.getDriverByName('tree')!;
551+
expect(isExternalDriverFloor(held)).toBe(true);
552+
expect(unwrapExternalDriverFloor(held)).toBe(old.driver);
553+
expect(ids(await engine.find('tree_node', { where: { kind: 'branch' } }))).toEqual(['n2', 'n5']);
554+
});
555+
556+
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 () => {
557+
// The defect's mechanism, pinned so the positive cases above are known to
558+
// depend on the driver id reaching `registerDatasourceDef`: the engine's
559+
// "keep the stored id" only keeps what the eviction left, which is nothing.
560+
const engine = new ObjectQL();
561+
const first = remoteDriver('tree', TREE_ROWS);
562+
engine.registerDriver(first.driver);
563+
engine.registerDatasourceDef(connectDef('spike_tree'));
564+
engine.registerApp(TREE_PKG as any);
565+
engine.unregisterDriver('tree');
566+
const rebuilt = remoteDriver('tree', TREE_ROWS);
567+
engine.registerDriver(rebuilt.driver);
568+
engine.registerDatasourceDef({ name: 'tree', schemaMode: 'external', external: { allowWrites: true } });
569+
expect(engine.getDriverByName('tree')).toBe(rebuilt.driver);
570+
expect(ids(await engine.find('tree_node', { where: { kind: 'branch' } }))).toHaveLength(TREE_ROWS.length);
571+
});
572+
573+
it('a built-in driver through the same connect and reconnect sequences stays the registered instance itself', () => {
574+
const engine = new ObjectQL();
575+
const first = remoteDriver('tree', TREE_ROWS);
576+
engine.registerDriver(first.driver);
577+
engine.registerDatasourceDef(connectDef('sqlite'));
578+
engine.registerApp(TREE_PKG as any);
579+
expect(engine.getDriverByName('tree')).toBe(first.driver);
580+
engine.unregisterDriver('tree');
581+
const rebuilt = remoteDriver('tree', TREE_ROWS);
582+
engine.registerDriver(rebuilt.driver);
583+
engine.registerDatasourceDef(connectDef('sqlite'));
584+
expect(engine.getDriverByName('tree')).toBe(rebuilt.driver);
585+
});
586+
});

‎packages/objectql/src/external-driver-floor.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
/**
44
* [#27277] The engine's correctness floor over a plugin driver on an external
5-
* datasource (ADR-0015 Tier A, epic #27261 P1).
5+
* datasource (epic #27261 Tier A, P1; ruling 6487532394 on #27277).
66
*
77
* ## Which drivers
88
*

‎packages/objectql/src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ export type {
300300
PrimaryDatasourceUnresolvedReason,
301301
} from './driver-connect-errors.js';
302302
// [#27277] The engine's floor over a plugin driver on an external datasource
303-
// (ADR-0015 Tier A): the in-memory reading of a driver query it applies, and
303+
// (epic #27261 Tier A): the in-memory reading of a driver query it applies, and
304304
// the two 422 refusals it raises (registered in ERROR_CODE_LEDGER), plus
305305
// [#27471] the 400 read refusal (the standard INVALID_QUERY).
306306
export { applyDriverQueryInMemory } from './driver-query-in-memory.js';

‎packages/services/service-datasource/src/__tests__/datasource-connection-service.test.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,13 @@ describe('DatasourceConnectionService.connect', () => {
195195
// Driver registered under the DATASOURCE name (engine routes by driver.name).
196196
expect(engine!.drivers.has('warehouse')).toBe(true);
197197
expect(engine!.drivers.get('warehouse')!.name).toBe('warehouse');
198-
// Datasource definition recorded for the write gate.
199-
expect(engine!.defs).toEqual([{ name: 'warehouse', schemaMode: 'external', external: externalDs.external }]);
198+
// Datasource definition recorded for the write gate — and, [#27666], with
199+
// the driver id the engine keys its plugin-driver floor on: for a
200+
// runtime-origin datasource this registration is the only one the engine
201+
// ever sees, so a def without `driver` would serve a plugin driver unfloored.
202+
expect(engine!.defs).toEqual([
203+
{ name: 'warehouse', driver: 'sqlite', schemaMode: 'external', external: externalDs.external },
204+
]);
200205
// Bound external objects got read metadata synced (DDL-free).
201206
expect(engine!.synced).toEqual(['ext_customer']);
202207
});

‎packages/services/service-datasource/src/__tests__/datasource-update-rebuild.test.ts‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,13 +334,50 @@ describe('#13804 — updateDatasource rebuilds the live pool when connectivity c
334334
const state = rig.connection.getConnectionState('analytics')!;
335335
expect(state.status).toBe('failed-degraded');
336336
// 3. Not pool-less: the datasource def the old pool was serving under is
337-
// restored alongside the driver (`unregisterDriver` removes both).
337+
// restored alongside the driver (`unregisterDriver` removes both) —
338+
// [#27666] its driver id included, which is what the engine keys its
339+
// plugin-driver floor on.
338340
expect(rig.engine.defs.get('analytics')).toMatchObject({
339341
name: 'analytics',
342+
driver: 'sqlite',
340343
schemaMode: 'external',
341344
});
342345
});
343346

347+
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 () => {
348+
// The engine floors a non-managed datasource whose def names a driver id
349+
// that is not built in. `unregisterDriver` drops the stored def, so the
350+
// id has to travel with every registration this path makes — the rebuilt
351+
// pool's from `next`, the kept pool's from `previous` — or a plugin
352+
// driver is served unfloored after a reconfigure, and nothing says so.
353+
const rig = makeRig({
354+
failWhen: (spec) => (spec.config as { filename?: string }).filename === '/tmp/bad.db',
355+
});
356+
await rig.service.createDatasource(
357+
draft({ driver: 'driver.port_schedule', schemaMode: 'external', external: { allowWrites: false } }),
358+
);
359+
expect(rig.engine.defs.get('analytics')).toMatchObject({ driver: 'driver.port_schedule', schemaMode: 'external' });
360+
361+
// A reconfigure that changes the driver and succeeds: the def names the new id.
362+
const rebuilt = await rig.service.updateDatasource('analytics', {
363+
driver: 'driver.port_schedule_v2',
364+
config: { filename: '/tmp/new.db' },
365+
});
366+
expect(rebuilt.status).toBe('ok');
367+
expect(rig.engine.drivers.get('analytics')!.builtFrom.driver).toBe('driver.port_schedule_v2');
368+
expect(rig.engine.defs.get('analytics')).toMatchObject({ driver: 'driver.port_schedule_v2', schemaMode: 'external' });
369+
370+
// A reconfigure that changes the driver and fails: the kept pool's def
371+
// names the id it was built from, not the one that never came up.
372+
const failed = await rig.service.updateDatasource('analytics', {
373+
driver: 'driver.port_schedule_v3',
374+
config: { filename: '/tmp/bad.db' },
375+
});
376+
expect(failed.status).toBe('error');
377+
expect(rig.engine.drivers.get('analytics')!.builtFrom.driver).toBe('driver.port_schedule_v2');
378+
expect(rig.engine.defs.get('analytics')).toMatchObject({ driver: 'driver.port_schedule_v2', schemaMode: 'external' });
379+
});
380+
344381
it('a supplied secret alone forces a rebuild — rewrap-in-place changes what the ref dereferences to', async () => {
345382
const rig = makeRig({ stableSecretRef: true });
346383
await rig.service.createDatasource(

‎packages/services/service-datasource/src/datasource-connection-service.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -666,8 +666,13 @@ export class DatasourceConnectionService {
666666
: true;
667667
engine.registerDriver(engineDriver, opts.asDefault === true);
668668
if (!heldBefore) registeredByThisAttempt = engineDriver.name;
669+
// [#27666] `driver` rides along: the engine keys its plugin-driver floor
670+
// on the def's driver id, and this registration is the ONLY one a
671+
// runtime-origin datasource gets (the manifest path never sees it), and
672+
// the only one left after `reconnect`'s eviction dropped the stored def.
669673
engine.registerDatasourceDef?.({
670674
name,
675+
driver: record.driver,
671676
schemaMode: record.schemaMode,
672677
external: record.external as { allowWrites?: boolean } | undefined,
673678
});
@@ -830,7 +835,7 @@ export class DatasourceConnectionService {
830835
record: ConnectableDatasource,
831836
opts: {
832837
context?: DatasourceConnectContext;
833-
previous?: Pick<ConnectableDatasource, 'schemaMode' | 'external'>;
838+
previous?: Pick<ConnectableDatasource, 'driver' | 'schemaMode' | 'external'>;
834839
} = {},
835840
): Promise<ConnectResult> {
836841
const name = record.name;
@@ -848,8 +853,13 @@ export class DatasourceConnectionService {
848853

849854
const restoreOldPool = (): void => {
850855
engine.registerDriver?.(oldDriver, false);
856+
// [#27666] The OLD record's driver id, not `record.driver`: the kept
857+
// pool was built from `previous`, and a reconfigure may have changed the
858+
// driver. Without it the engine would read the restored def as "not a
859+
// plugin driver" and serve the kept pool unfloored.
851860
engine.registerDatasourceDef?.({
852861
name,
862+
driver: opts.previous?.driver,
853863
schemaMode: opts.previous?.schemaMode,
854864
external: opts.previous?.external as { allowWrites?: boolean } | undefined,
855865
});

0 commit comments

Comments
 (0)