Skip to content

Commit 77b91bd

Browse files
Elon Muskclaude
andauthored
fix(service-datasource): derive ConnectionEngineLike from the engine contract, and stop promising registerDriver accepts any value (#13391)
* fix(service-datasource): derive ConnectionEngineLike from the engine contract (#12010) Replace the seven hand-declared engine members on the exported `ConnectionEngineLike` with `Partial<Pick<IObjectQLEngine, ...>>` — the #4251 B3 move `datasource-admin-plugin.ts` already made for its sibling `DataEngineLike`. The load-bearing half is `registerDriver`, which the seam declared as `(driver: unknown, isDefault?: boolean) => void` against the contract's `(driver: IDataDriver, isDefault?: boolean) => void`. Under `strictFunctionTypes` the real engine was therefore not assignable to this view, and the exported type promised the engine accepts any value as a driver. Adds `connection-engine-like-contract.test.ts`, whose assertions fail if the type is ever hand-written back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WkdHQwHr2KQmaX7P1BHzi * test(runtime): narrow the degraded-boot parity engine double's getDriverByName The consumer half of the ConnectionEngineLike derivation. This double answers `unknown` where the derived seam member now answers the contract's `IDataDriver | undefined`, which `check:type-check-debt` measured as @objectstack/runtime TEST_DEBT 217 -> 218 (the package's own `typecheck` excludes `**/*.test.ts`, so nothing else in the local family sees it). Same triage as the three service-datasource fixtures: the double keeps its minimal stand-ins and narrows on the way out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WkdHQwHr2KQmaX7P1BHzi --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 83546d9 commit 77b91bd

7 files changed

Lines changed: 259 additions & 51 deletions
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
"@objectstack/service-datasource": patch
3+
---
4+
5+
fix(service-datasource): derive `ConnectionEngineLike` from the engine contract instead of re-declaring it, and stop promising `registerDriver` accepts any value (#12010)
6+
7+
`ConnectionEngineLike` — the exported view `DatasourceConnectionService` drives
8+
the ObjectQL `'data'` engine through — hand-declared seven engine members. The
9+
#12010 inventory measured what that cost, and one half of it was unsafe rather
10+
than merely duplicated.
11+
12+
**The unsafe half.** The seam declared
13+
`registerDriver?: (driver: unknown, isDefault?: boolean) => void`,
14+
while the engine contract declares `registerDriver(driver: IDataDriver,
15+
isDefault?: boolean): void`. Under `strictFunctionTypes` that made the real
16+
engine **not assignable** to this view, and it told every consumer of the
17+
exported type that the engine accepts *any* value as a driver — which it does
18+
not, so a mis-shaped driver reaching `registerDriver` was a runtime problem the
19+
type was structured not to see. The parameter is now the contract's
20+
`IDataDriver`, which repairs both halves at once: the seam stops over-promising
21+
AND the engine becomes assignable to it.
22+
23+
**The duplicated half.** Three members (`registerDatasourceDef`,
24+
`markDatasourceUnavailable`, `clearDatasourceUnavailable`) were declared by no
25+
contract at all when the card was filed — real `ObjectQL` methods, called
26+
across a package boundary, meeting no compiler on the producer side. #12248
27+
adjudicated all three onto `IDataEngine`, and #12482 followed with
28+
`syncObjectSchema`. All seven members are now **derived**
29+
(`Partial<Pick<IObjectQLEngine, …>>`) rather than re-written, the same #4251 B3
30+
move `datasource-admin-plugin.ts` already made for its sibling `DataEngineLike`
31+
one file over. Drift now lands as a build error here instead of a silent
32+
disagreement.
33+
34+
`Partial` is preserved deliberately: `registerDriver` is required on the
35+
contract, while this service treats its absence as graceful degradation (the
36+
datasource is left metadata-only, `'skipped-no-infra'`). Making it required
37+
would change what a lightweight kernel does at boot.
38+
39+
No runtime behaviour changes. The one cast the factory escape hatch still needs
40+
(`DatasourceDriverHandle.driver` is declared `unknown`, open to any host-built
41+
driver) moved to the single call site that constructs the value, and the
42+
`disconnect()` path dropped its cast entirely now that `getDriverByName` answers
43+
the contract's `IDataDriver | undefined`.
44+
45+
**For hosts:** if you implement `ConnectionEngineLike` directly, its members are
46+
now the engine contract's members. An implementation whose `registerDriver`
47+
takes a wider parameter (`unknown`, `any`) is still accepted; one that returns a
48+
narrower value from `getDriverByName` than `IDataDriver | undefined` is not, and
49+
should answer the contract type.

packages/runtime/src/degraded-boot-parity.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
3232
import { ObjectQL } from '@objectstack/objectql';
3333
import { DatasourceConnectionService } from '@objectstack/service-datasource';
34+
import type { IDataDriver } from '@objectstack/spec/contracts';
3435

3536
const ENV = 'OS_ALLOW_DRIVER_CONNECT_FAILURE';
3637

@@ -85,7 +86,12 @@ async function bootDatasource(): Promise<Error | undefined> {
8586
}) as any,
8687
engine: () => ({
8788
registerDriver: (d: any) => drivers.set(d.name, d),
88-
getDriverByName: (n: string) => drivers.get(n),
89+
// [#12010] The double stores a bare `{ name: 'd' }` stand-in, while
90+
// `ConnectionEngineLike.getDriverByName` is now derived from the engine
91+
// contract and answers `IDataDriver | undefined`. Narrowing on the way
92+
// out keeps the double as loose as this parity test needs it without
93+
// re-widening the seam.
94+
getDriverByName: (n: string) => drivers.get(n) as IDataDriver | undefined,
8995
}),
9096
logger: { warn() {} },
9197
});
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#12010] The seam pin for `ConnectionEngineLike`.
5+
*
6+
* `ConnectionEngineLike` is the exported view `DatasourceConnectionService`
7+
* drives the ObjectQL `'data'` engine through. It used to re-declare seven
8+
* engine members by hand; it is now derived from {@link IObjectQLEngine}. The
9+
* assertions below are the ones that FAIL if it is ever hand-written back.
10+
*
11+
* Enforced by `tsc --noEmit` (this package's `typecheck` includes `src`, and
12+
* `src/__tests__` is inside it), not by the vitest run: vitest strips types.
13+
* The `expect`s exist so the file is also a real, running test — but the
14+
* measurement that matters here is the typecheck.
15+
*/
16+
17+
import { describe, expect, it } from 'vitest';
18+
import type { IDataDriver, IObjectQLEngine } from '@objectstack/spec/contracts';
19+
import type { ConnectionEngineLike } from '../datasource-connection-service.js';
20+
21+
describe('ConnectionEngineLike is the contract, not a fork of it (#12010)', () => {
22+
it('the real engine contract is assignable to this view', () => {
23+
// The defect this card measured, stated as a compile: with
24+
// `registerDriver?: (driver: unknown, …)` the engine was NOT assignable
25+
// here, because `unknown` is not assignable to the parameter's
26+
// `IDataDriver` under `strictFunctionTypes`. Cast-free by construction.
27+
const asConnectionEngine = (engine: IObjectQLEngine): ConnectionEngineLike => engine;
28+
expect(typeof asConnectionEngine).toBe('function');
29+
});
30+
31+
it('registerDriver takes a DRIVER, not any value', () => {
32+
type RegisterDriver = NonNullable<ConnectionEngineLike['registerDriver']>;
33+
type DriverParam = Parameters<RegisterDriver>[0];
34+
// Exactly the contract's parameter — not a supertype of it. A widening
35+
// back to `unknown` collapses the second leg.
36+
const exact: [DriverParam] extends [IDataDriver]
37+
? [IDataDriver] extends [DriverParam]
38+
? 'exact'
39+
: never
40+
: never = 'exact';
41+
expect(exact).toBe('exact');
42+
});
43+
44+
it('refuses a value that is not a driver, at the call site', () => {
45+
const engine = {} as ConnectionEngineLike;
46+
// @ts-expect-error - a bare `{ name }` is not an `IDataDriver`. This call
47+
// compiled before #12010, which is precisely the hole: the seam promised
48+
// the engine accepts any value as a driver, and it does not.
49+
engine.registerDriver?.({ name: 'com.example.not-a-driver' });
50+
expect(engine).toBeTruthy();
51+
});
52+
53+
it('getDriverByName answers the contract driver, not `unknown`', () => {
54+
type Answer = ReturnType<NonNullable<ConnectionEngineLike['getDriverByName']>>;
55+
// `unknown` would satisfy neither leg; this is what lets `disconnect()`
56+
// reach `driver.disconnect` without the local re-derivation it used to
57+
// cast through.
58+
const exact: [Answer] extends [IDataDriver | undefined]
59+
? [IDataDriver | undefined] extends [Answer]
60+
? 'exact'
61+
: never
62+
: never = 'exact';
63+
expect(exact).toBe('exact');
64+
});
65+
66+
it('declares exactly the seven derived members, each identical to its contract member', () => {
67+
type Declared = keyof ConnectionEngineLike;
68+
type Expected =
69+
| 'registerDriver'
70+
| 'registerDatasourceDef'
71+
| 'getDriverByName'
72+
| 'syncObjectSchema'
73+
| 'getDefaultDriverName'
74+
| 'markDatasourceUnavailable'
75+
| 'clearDatasourceUnavailable';
76+
// Both directions: a member added by hand fails the first leg, a member
77+
// dropped fails the second.
78+
const roster: [Declared] extends [Expected]
79+
? [Expected] extends [Declared]
80+
? 'exact'
81+
: never
82+
: never = 'exact';
83+
84+
type Same<K extends Expected> =
85+
NonNullable<ConnectionEngineLike[K]> extends NonNullable<IObjectQLEngine[K]>
86+
? NonNullable<IObjectQLEngine[K]> extends NonNullable<ConnectionEngineLike[K]>
87+
? 'same'
88+
: never
89+
: never;
90+
const members: { [K in Expected]: Same<K> } = {
91+
registerDriver: 'same',
92+
registerDatasourceDef: 'same',
93+
getDriverByName: 'same',
94+
syncObjectSchema: 'same',
95+
getDefaultDriverName: 'same',
96+
markDatasourceUnavailable: 'same',
97+
clearDatasourceUnavailable: 'same',
98+
};
99+
100+
expect(roster).toBe('exact');
101+
expect(Object.keys(members)).toHaveLength(7);
102+
});
103+
104+
it('every member stays OPTIONAL — the graceful-degradation seam', () => {
105+
// `registerDriver` is REQUIRED on the contract; `Partial<…>` is what keeps
106+
// `if (!factory || !engine?.registerDriver) → 'skipped-no-infra'` a live
107+
// runtime branch rather than dead code.
108+
type AllOptional = {
109+
[K in keyof ConnectionEngineLike]-?: undefined extends ConnectionEngineLike[K] ? true : false;
110+
}[keyof ConnectionEngineLike];
111+
const optional: AllOptional extends true ? 'optional' : never = 'optional';
112+
expect(optional).toBe('optional');
113+
});
114+
});

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
type ConnectionEngineLike,
99
} from '../datasource-connection-service.js';
1010
import type { IDatasourceDriverFactory } from '../contracts/datasource-driver-factory.js';
11+
import type { IDataDriver } from '@objectstack/spec/contracts';
1112
import type { DatasourceConnectPolicy } from '../contracts/connect-policy.js';
1213
import {
1314
GENERIC_CONNECT_FAILURE_REMEDY,
@@ -44,7 +45,12 @@ function fakeEngine() {
4445
registerDatasourceDef: (def) => {
4546
defs.push(def);
4647
},
47-
getDriverByName: (name) => drivers.get(name),
48+
// [#12010] The double deliberately stores MINIMAL stand-ins (a bare
49+
// `{ name }` is how these tests simulate an `onEnable`-registered
50+
// driver), while the derived seam member answers the contract's
51+
// `IDataDriver | undefined`. Narrowing on the way out keeps the double
52+
// loose where it is meant to be loose without re-widening the seam.
53+
getDriverByName: (name) => drivers.get(name) as IDataDriver | undefined,
4854
syncObjectSchema: async (name) => {
4955
synced.push(name);
5056
},

packages/services/service-datasource/src/__tests__/datasource-pool-support.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ import {
6464
} from '../datasource-connection-service.js';
6565
import { DatasourceAdminService, type StoredDatasource } from '../datasource-admin-service.js';
6666
import type { IDatasourceDriverFactory } from '../contracts/datasource-driver-factory.js';
67+
import type { IDataDriver } from '@objectstack/spec/contracts';
6768

6869
describe('#5714 — which driver arms read a declared `pool`', () => {
6970
it('names the two sqlite arms, `memory` and `turso` as unable to honour it (#5931 / #7243)', () => {
@@ -304,7 +305,12 @@ function fakeEngine() {
304305
drivers,
305306
registerDriver: (driver: any) => { drivers.set(driver.name, driver); },
306307
registerDatasourceDef: () => {},
307-
getDriverByName: (name) => drivers.get(name),
308+
// [#12010] The double deliberately stores MINIMAL stand-ins (a bare
309+
// `{ name }` is how these tests simulate an `onEnable`-registered
310+
// driver), while the derived seam member answers the contract's
311+
// `IDataDriver | undefined`. Narrowing on the way out keeps the double
312+
// loose where it is meant to be loose without re-widening the seam.
313+
getDriverByName: (name) => drivers.get(name) as IDataDriver | undefined,
308314
};
309315
return engine;
310316
}

packages/services/service-datasource/src/__tests__/prebuilt-driver-factory.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
1212
import { createPrebuiltDriverFactory } from '../prebuilt-driver-factory.js';
1313
import { DatasourceConnectionService } from '../datasource-connection-service.js';
14+
import type { IDataDriver } from '@objectstack/spec/contracts';
1415

1516
// The fail-fast case below asserts the default (no-escape-hatch) verdict.
1617
const ENV = 'OS_ALLOW_DRIVER_CONNECT_FAILURE';
@@ -91,7 +92,12 @@ describe('createPrebuiltDriverFactory — through DatasourceConnectionService (t
9192
drivers.set(d.name, d);
9293
if (isDefault) defaultName = d.name;
9394
},
94-
getDriverByName: (n: string) => drivers.get(n),
95+
// [#12010] The double deliberately stores MINIMAL stand-ins (a bare
96+
// `{ name }` is how these tests simulate an `onEnable`-registered
97+
// driver), while the derived seam member answers the contract's
98+
// `IDataDriver | undefined`. Narrowing on the way out keeps the double
99+
// loose where it is meant to be loose without re-widening the seam.
100+
getDriverByName: (n: string) => drivers.get(n) as IDataDriver | undefined,
95101
getDefaultDriverName: () => defaultName,
96102
}),
97103
logger: { warn() {} },

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

Lines changed: 68 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import {
4545
} from './datasource-pool-support.js';
4646
import { connectFailureRemedy } from './connect-failure-remedy.js';
4747
import type { Logger } from './logger.js';
48+
import type { IDataDriver, IObjectQLEngine } from '@objectstack/spec/contracts';
4849

4950
/** A datasource definition this service can connect (code- or runtime-origin). */
5051
export interface ConnectableDatasource {
@@ -85,48 +86,57 @@ export interface DatasourceBoundObject {
8586
datasource?: string;
8687
}
8788

88-
/** Engine surface this service drives (the ObjectQL `'data'` engine). */
89-
export interface ConnectionEngineLike {
90-
registerDriver?: (driver: unknown, isDefault?: boolean) => void;
91-
registerDatasourceDef?: (def: {
92-
name: string;
93-
schemaMode?: string;
94-
external?: { allowWrites?: boolean };
95-
}) => void;
96-
getDriverByName?: (name: string) => unknown;
97-
/**
98-
* Register read metadata (DDL-free) for a federated object so its physical
99-
* remote table/columns resolve for queries. Idempotent; called per bound
100-
* external object after the driver is registered, because boot schema-sync
101-
* ran before this driver existed (ADR-0015 §18; matches what the legacy
102-
* `onEnable` bridge does manually).
103-
*/
104-
syncObjectSchema?: (objectName: string) => Promise<void>;
105-
/**
106-
* Name of the engine's DEFAULT driver, when one is set. Used by the
107-
* `asDefault` connect path's idempotency guard (#3826): the default driver
108-
* keeps its natural name, so `getDriverByName('default')` can never detect a
109-
* prior registration.
110-
*/
111-
getDefaultDriverName?: () => string | undefined;
112-
/**
113-
* Tell the engine a datasource was *declared* but is not connected, and why
114-
* (framework#3828). Without this the engine cannot distinguish "the app
115-
* misspelled a datasource name" from "the host's policy refused it" from "it
116-
* failed to connect and the operator set OS_ALLOW_DRIVER_CONNECT_FAILURE" —
117-
* all three used to surface as the same bare `is not registered`.
118-
*
119-
* `publicDetail` is the only part safe to echo to an end user; the operator
120-
* -facing reason stays in the logs and the datasource-admin list.
121-
*/
122-
markDatasourceUnavailable?: (info: {
123-
name: string;
124-
kind: 'blocked' | 'failed';
125-
publicDetail?: string;
126-
}) => void;
127-
/** Drop a previous {@link markDatasourceUnavailable} record (reconnect / removal). */
128-
clearDatasourceUnavailable?: (name: string) => void;
129-
}
89+
/**
90+
* Engine surface this service drives (the ObjectQL `'data'` engine), DERIVED
91+
* from the published contracts rather than re-declared structurally — the
92+
* #4251 B3 sweep pattern, applied here exactly as `datasource-admin-plugin.ts`
93+
* applied it to its own `DataEngineLike` one file over, under #11493's ruling.
94+
*
95+
* [#12010] Every member was hand-written here until this change, and the
96+
* inventory that filed that card measured what it cost. Three of them —
97+
* `registerDatasourceDef`, `markDatasourceUnavailable`,
98+
* `clearDatasourceUnavailable` — were declared by NO contract at all: real
99+
* `ObjectQL` methods, called across a package boundary, meeting no compiler on
100+
* the producer side, so drift landed silently in this consumer. #12248
101+
* adjudicated all three onto {@link IDataEngine} and #12482 followed with
102+
* `syncObjectSchema`; deriving is what makes the next drift a build error here
103+
* instead of a re-declaration that quietly disagrees.
104+
*
105+
* One member was not merely a duplicate but wrong in the unsafe direction.
106+
* `registerDriver?: (driver: unknown, isDefault?: boolean) => void` promised
107+
* this seam accepts *any* value as a driver. The engine does not —
108+
* {@link IObjectQLEngine.registerDriver} takes an {@link IDataDriver} — so
109+
* under `strictFunctionTypes` the real engine was measurably NOT assignable to
110+
* this view, and a mis-shaped driver reaching `registerDriver` was a runtime
111+
* problem the type was structured not to see. Deriving repairs both halves at
112+
* once; `__tests__/connection-engine-like-contract.test.ts` pins them.
113+
*
114+
* Anchored on {@link IObjectQLEngine} rather than {@link IDataEngine} for one
115+
* member: `registerDriver` is declared only on the full engine contract. The
116+
* two describe the same instance — `packages/objectql`'s plugin registers one
117+
* object under both the `'data'` and `'objectql'` slots — but the `'data'`
118+
* slot's declared type is the data plane alone, which is why
119+
* `datasource-admin-plugin.ts` still reaches this type through a cast.
120+
*
121+
* `Partial<…>` is load-bearing, not shorthand — the same reason the sibling
122+
* `DataEngineLike` carries it. `registerDriver` is REQUIRED on the contract,
123+
* while this service treats its absence as a graceful-degradation signal
124+
* (`if (!factory || !engine?.registerDriver)` leaves the datasource
125+
* metadata-only, status `'skipped-no-infra'`). Making it required here would
126+
* change what a lightweight kernel does at boot.
127+
*/
128+
export type ConnectionEngineLike = Partial<
129+
Pick<
130+
IObjectQLEngine,
131+
| 'registerDriver'
132+
| 'registerDatasourceDef'
133+
| 'getDriverByName'
134+
| 'syncObjectSchema'
135+
| 'getDefaultDriverName'
136+
| 'markDatasourceUnavailable'
137+
| 'clearDatasourceUnavailable'
138+
>
139+
>;
130140

131141
/** Secret dereference surface (the `SecretBinder.resolve`, Phase 2 / D3). */
132142
export interface ConnectionSecretResolver {
@@ -588,10 +598,19 @@ export class DatasourceConnectionService {
588598
// `default` goes through the engine's default-driver fallback, never
589599
// `drivers.get('default')`, and the natural name keeps logs/lookups
590600
// byte-for-byte with the pre-#3826 boot.
591-
const engineDriver = (handle.driver ?? handle) as { name?: string };
601+
// [#12010] `DatasourceDriverHandle.driver` is declared `unknown` — the
602+
// factory escape hatch is open to any host-built driver — so a cast is
603+
// unavoidable somewhere on this path. It belongs HERE, at the one call
604+
// site that constructs the value, not widened into the exported seam
605+
// type where it told every consumer the engine accepts any value as a
606+
// driver.
607+
const engineDriver = (handle.driver ?? handle) as IDataDriver;
592608
if (!opts.asDefault) {
593609
try {
594-
engineDriver.name = name;
610+
// `IDataDriver.name` is `readonly`, and the engine routes by
611+
// `driver.name === <datasource>`, so the stamp goes through a
612+
// writable view (a frozen driver throws and is tolerated below).
613+
(engineDriver as { name?: string }).name = name;
595614
} catch {
596615
/* frozen driver — registration may still work if name already matches */
597616
}
@@ -648,9 +667,11 @@ export class DatasourceConnectionService {
648667
async disconnect(name: string, opts: { asDefault?: boolean } = {}): Promise<void> {
649668
const engine = this.cfg.engine();
650669
const driverName = opts.asDefault ? engine?.getDefaultDriverName?.() : name;
651-
const driver = (driverName ? engine?.getDriverByName?.(driverName) : undefined) as
652-
| { disconnect?: () => Promise<void> }
653-
| undefined;
670+
// [#12010] Cast-free: `getDriverByName` now answers the contract's
671+
// `IDataDriver | undefined` instead of a locally re-declared `unknown`.
672+
// The `typeof … === 'function'` guard below stays — a host-built driver in
673+
// the registry satisfies the contract only structurally.
674+
const driver = driverName ? engine?.getDriverByName?.(driverName) : undefined;
654675
if (this.states.get(name)?.ownership === 'host') {
655676
this.logger?.debug?.(`datasource '${name}': adopted (host-owned) instance — pool left to the host, clearing state only`);
656677
} else if (typeof driver?.disconnect === 'function') {

0 commit comments

Comments
 (0)