Skip to content

Commit c9d254a

Browse files
os-zhuangclaude
andauthored
feat(datasource,runtime): teardown through the one datasource path, honoring adopted pools (#3993); ADR-0062 status flip (#3992) (#3999)
DatasourceConnectionService owns the disconnect half symmetrically now: disconnect(name, { asDefault }) resolves the default under its natural name, disconnectAll() closes exactly the 'connected' pools this service opened, and the new DatasourceDriverHandle.ownership discriminator ('factory' | 'host') keeps kernel teardown away from adopted instances — createPrebuiltDriverFactory stamps 'host', so a cloud LRU eviction can never pull a shared pool from under its other consumers. Wired at the kernel's real teardown phase (destroy(); the Plugin contract has no stop()) via DefaultDatasourcePlugin and DatasourceAdminServicePlugin. ADR-0062: header and D1 notes now record the completed cross-repo convergence (cloud#915) with the escape-hatch classification and the parity guard's shifted role; D5 gains the teardown amendment. Closes #3992. Closes #3993. Claude-Session: https://claude.ai/code/session_01GK5nd3gtehcAhrF6Zzp2on Co-authored-by: Claude <noreply@anthropic.com>
1 parent d6bfb3d commit c9d254a

11 files changed

Lines changed: 254 additions & 8 deletions
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
"@objectstack/service-datasource": minor
3+
"@objectstack/runtime": minor
4+
---
5+
6+
feat(datasource,runtime): kernel teardown disconnects through the one datasource path — and never closes an adopted pool (#3993)
7+
8+
After the #3826 connect convergence, ADR-0062 D5's "owns connect/disconnect"
9+
was half-true: nothing disconnected the `default` (or a declared datasource's
10+
pool) on graceful shutdown. `DriverPlugin` never had teardown, `ObjectQLPlugin`
11+
teardown never touched drivers, and the kernel's actual teardown phase is
12+
`destroy()` — the Plugin contract has no `stop()`, so stray `stop` methods were
13+
never called by anything.
14+
15+
The disconnect half now mirrors the connect half:
16+
17+
- **`DatasourceConnectionService.disconnect(name, { asDefault })`** resolves
18+
the default under its NATURAL name (the same #3826 rule that makes
19+
`drivers.get('default')` impossible — the old lookup could never have found
20+
it), and honours a new ownership discriminator recorded at connect time.
21+
- **`disconnectAll()`** closes exactly the pools THIS service opened —
22+
`'connected'` states only. `already-registered` drivers belong to whoever
23+
registered them (an `onEnable` bridge, the default's idempotent replay) and
24+
are never touched.
25+
- **`DatasourceDriverHandle.ownership: 'factory' | 'host'`** is the
26+
discriminator. `createPrebuiltDriverFactory` stamps its handles `'host'`:
27+
an ADOPTED instance's pool outlives the kernel (the cloud control-plane
28+
driver doubles as every environment kernel's proxy base; per-environment
29+
drivers are registry-cached across kernel rebuilds), so kernel teardown —
30+
including a cloud LRU eviction's `kernel.shutdown()` — clears the retained
31+
verdict but NEVER closes the pool. Factory-built instances disconnect as
32+
before there was a before.
33+
- **`DefaultDatasourcePlugin.destroy()`** and
34+
**`DatasourceAdminServicePlugin.destroy()`** wire the sweep at the kernel's
35+
real teardown phase, best-effort (a failed disconnect never masks shutdown).
36+
37+
A welcome side effect: a file-backed `sqlite-wasm` default with
38+
`persist: 'on-disconnect'` now actually flushes on graceful shutdown.
39+
40+
Also flips ADR-0062's status to reflect the completed convergence (#3992):
41+
D1 is fully implemented across both repos since cloud#915; the remaining
42+
`DriverPlugin` uses are documented named-auxiliary/escape-hatch cases, and the
43+
degraded-boot parity guard stays with its role shifted to "the escape hatches
44+
must not drift".

docs/adr/0062-external-datasource-runtime.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# ADR-0062: External Datasource Runtime — connection lifecycle, credentials, visibility & query completeness
22

3-
**Status**: Accepted (2026-06-22) — D2–D8 implemented (`service-datasource` connection service + opt-in-safe gate + fail-closed connect policy; native-SQL declines external per D6; D7 lint in `validate-expressions.ts`). **D1 implemented for every open-core boot path** (#3826: the standalone/artifact `default` and the CLI serve fallback are declared definitions connected through the one service — see the status notes under D1); the cloud repo's own stack composition is the last remaining second site, and the parity guard stays until it converges.
3+
**Status**: Accepted (2026-06-22) — D1–D8 implemented. **D1 fully implemented across both repos** (#3826, closed): every open-core boot path (#3968) and every cloud composition (cloud#915 — control-plane preset, objectos `artifact-kernel-factory`, `DefaultEnvironmentKernelFactory`) connects its `default` through the one `DatasourceConnectionService` path; the remaining `DriverPlugin` uses are documented named-auxiliary/escape-hatch cases, and the degraded-boot parity guard stays to pin them (see the status notes under D1). D5's teardown half landed with #3993: kernel teardown disconnects through the same service, honouring adopted (host-owned) instances — see the amendment under D5.
44

55
**Supersedes the runtime portions of**: ADR-0015 §18 addendum (kept as the historical record). ADR-0015 remains the canonical spec/binding decision; this ADR is the canonical *runtime* decision.
66

@@ -68,7 +68,7 @@ Introduce a single service that, given a datasource definition, builds a driver
6868
>
6969
> **Config-load fallback converged too (#3826, third pass).** `createStorageDriver` is gone: the CLI's serve fallback (a host `objectstack.config.ts` with objects but no driver plugin) now emits a definition via `resolveStorageDefinition` and hands it to the same `DefaultDatasourcePlugin`. `mysql` joined the shared factory for it; the dev loosen-only self-heal (#2186) rides as `config.autoMigrate` and the CLI's wasm persistence mode as `config.persist` — host-composition passthroughs the factory honours, never part of the app-facing datasource spec. `turso`/libSQL keeps its loud typed failure at *resolution* (nothing is constructed to fail later). The `telemetry` sibling datasource deliberately stays a pre-built `DriverPlugin` (the documented escape hatch for named auxiliary drivers): it is best-effort, dev-oriented, and its own `resolveSqliteDriver` step-down check replaces the old primary-resolution coupling.
7070
>
71-
> **Remaining second site: the cloud repo's own compositions** — the real primary-driver sites are `environment-kernel-factory.ts` (each environment's per-tenant driver) and the control-plane preset fed by `cloud-stack.ts`'s `buildControlDriver`, which also owns the `turso` driver. Until they converge, `packages/runtime/src/degraded-boot-parity.test.ts` remains load-bearing: it pins both connect paths to the same operator-visible contract (fail-fast by default, identical `OS_ALLOW_DRIVER_CONNECT_FAILURE` parsing, `DEGRADED BOOT` on stderr). #3741#3758 was exactly the miss it exists to catch.
71+
> **Convergence completed — the cloud compositions landed (cloud#915), no second site remains.** The cloud repo's three primary-driver sites — the control-plane preset (fed by `cloud-stack.ts`'s `buildControlDriver`, whose instance doubles as every environment kernel's proxy base), `artifact-kernel-factory.ts` (the objectos per-tenant hot path), and both `DefaultEnvironmentKernelFactory` paths — now boot through `DefaultDatasourcePlugin` with their pre-built instances adopted via `createPrebuiltDriverFactory`. Every remaining `DriverPlugin` use is a **documented named-auxiliary / escape-hatch case**, not a default path: the framework's `telemetry` sibling, the cloud proxy `cloud` datasource (`registerAsDefault: false`), the objectos host routing shell, the artifact factory's `'cloud'` alias metadata registration, and test injection. `packages/runtime/src/degraded-boot-parity.test.ts` **stays load-bearing after the convergence**, with its role shifted from "the unconverged second implementation must not drift" to "the escape hatches must not drift": as long as `ObjectQLEngine.init()` can throw a connect verdict at all (the boot re-verification, pre-built `DriverPlugin` drivers), that verdict must match the service's. #3741 → #3758 was exactly the miss it exists to catch.
7272
>
7373
> **The convergence seam (#3826, fourth pass).** Two properties of the cloud composition made "just declare it" impossible: its driver kinds live outside open-core (`turso`), and its instances are *pooled beyond one kernel* (the control-plane driver doubles as the proxy base of every environment kernel; per-environment drivers are cached across kernel rebuilds — reconstruction per boot would multiply pools). So `DefaultDatasourcePlugin` now accepts an **injected `IDatasourceDriverFactory`** (defaulting to the shared open-core factory), and `createPrebuiltDriverFactory` wraps an already-built instance as a factory — the "adopt an existing driver" entry point this ADR's first pass found missing, landed *as a factory* so the connect orchestration (policy-free init connect, `bootCritical` verdict, shared escape hatch, start() replay into retained state) stays this one implementation. Construction and pooling remain host concerns; only the verdict converges. The `@objectstack/verify` dogfood harness also boots through the declared default now (not the `DriverPlugin` escape hatch), making the §Risk mitigation — "behind the dogfood gate" — actually true for the converged path.
7474
@@ -108,6 +108,8 @@ Code-defined datasources surface in `GET /api/v1/datasources`, `GET /api/v1/meta
108108
>
109109
> **A `DatasourceConnectPolicy` denial is not a connect failure** and stays metadata-only, unchanged. A multi-tenant host that blocks egress for a tenant's plan is making a deliberate decision about a datasource it knows it is refusing; fail-fasting there would turn a policy verdict into a boot outage for every tenant on the shared runtime. The fail-fast set covers *failures* — unreachable, unauthenticated, unsupported — not *refusals*.
110110
111+
> **Amendment (#3993) — teardown is one implementation too, and it distinguishes owned from adopted.** After the D1 connect convergence, "owns connect/disconnect" was half-true: nothing disconnected the `default` (or a declared datasource's pool) on graceful shutdown — `DriverPlugin` never had teardown, `ObjectQLPlugin`'s teardown never touched drivers, and the kernel's actual teardown phase is **`destroy()`** (the Plugin contract has no `stop()`; stray `stop` methods were never called). Now `DatasourceConnectionService` owns the disconnect half symmetrically: `disconnect(name, { asDefault })` resolves the default under its **natural name** (the same #3826 rule that makes `drivers.get('default')` impossible), and `disconnectAll()` — wired from `DefaultDatasourcePlugin.destroy()` and `DatasourceAdminServicePlugin.destroy()` — closes **exactly the pools this service opened** (`'connected'` states; `already-registered` drivers belong to whoever registered them). The factory handle carries the discriminator: **`ownership: 'host'`** (set by `createPrebuiltDriverFactory`) marks an ADOPTED instance whose pool outlives the kernel — the cloud constraint: the control-plane driver doubles as every environment kernel's proxy base, per-environment drivers are registry-cached across kernel rebuilds, and an LRU eviction (`kernel.shutdown()`) closing a shared pool would pull it from under every other consumer. For those, teardown clears the retained verdict and leaves the pool to its host. A welcome side effect: a file-backed `sqlite-wasm` default with `persist: 'on-disconnect'` now actually flushes on graceful shutdown.
112+
111113
### D6 — Native-analytics SQL honors the remote table/columns
112114

113115
The analytics native-SQL strategy compiles its own `FROM "<table>"` / column references outside the driver (ADR-0015 §18 noted this). It must resolve an external object's physical table (`remoteName`/`remoteSchema`) and columns (`columnMap`) the same way `SqlDriver` now does — reusing the driver's resolution (e.g. an exposed `physicalTableFor(object)` / `physicalColumnFor(object, field)`), not a second copy. Until then, analytics over external objects stays disabled rather than silently querying the wrong table.

packages/runtime/src/default-datasource-plugin.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,37 @@ describe('DefaultDatasourcePlugin — the default datasource as a declaration (#
194194
try { await (kernel as any)?.stop?.(); } catch { /* noop */ }
195195
}, BOOT_TIMEOUT);
196196

197+
it('kernel teardown disconnects an OWNED (factory-built) default through the one service (#3993)', async () => {
198+
const kernel = await assemble({});
199+
await kernel.bootstrap();
200+
const engine = kernel.getService<any>('data');
201+
const drv = engine.getDriverByName(engine.getDefaultDriverName());
202+
let disconnects = 0;
203+
const orig = drv.disconnect?.bind(drv);
204+
drv.disconnect = async () => { disconnects += 1; return orig?.(); };
205+
await (kernel as any).shutdown();
206+
expect(disconnects).toBeGreaterThan(0);
207+
}, BOOT_TIMEOUT);
208+
209+
it('kernel teardown NEVER closes an ADOPTED (host-owned) default — the pool outlives the kernel (#3993)', async () => {
210+
// The cloud shape: the instance is shared beyond this kernel (proxy base /
211+
// registry cache). An LRU eviction shutting the kernel down must not pull
212+
// the pool from under every other consumer.
213+
const { createPrebuiltDriverFactory } = await import('@objectstack/service-datasource');
214+
const { InMemoryDriver } = await import('@objectstack/driver-memory');
215+
const hostBuilt = new InMemoryDriver() as any;
216+
let disconnects = 0;
217+
const orig = hostBuilt.disconnect?.bind(hostBuilt);
218+
hostBuilt.disconnect = async () => { disconnects += 1; return orig?.(); };
219+
const kernel = await assemble({
220+
driver: 'turso',
221+
factory: createPrebuiltDriverFactory(hostBuilt, { driverId: 'turso' }),
222+
});
223+
await kernel.bootstrap();
224+
await (kernel as any).shutdown();
225+
expect(disconnects).toBe(0);
226+
}, BOOT_TIMEOUT);
227+
197228
it("rejects an app bundle that declares a datasource named 'default' (host-reserved name)", async () => {
198229
const kernel = await assemble({
199230
bundle: {

packages/runtime/src/default-datasource-plugin.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ export class DefaultDatasourcePlugin implements Plugin {
9898
private readonly def: DefaultDatasourceDefinition;
9999
private readonly dev?: boolean;
100100
private readonly factory?: IDatasourceDriverFactory;
101+
/** The init()-time local connection service — held for destroy()'s teardown. */
102+
private connection?: DatasourceConnectionService;
101103

102104
constructor(def: DefaultDatasourceDefinition, opts: DefaultDatasourcePluginOptions = {}) {
103105
this.def = def;
@@ -131,6 +133,7 @@ export class DefaultDatasourcePlugin implements Plugin {
131133
// packages.
132134
logger: ctx.logger as unknown as ConstructorParameters<typeof DatasourceConnectionService>[0]['logger'],
133135
});
136+
this.connection = connection;
134137

135138
// Throws on failure (bootCritical ⇒ fail-fast per ADR-0062 D5), aborting
136139
// bootstrap exactly like the engine-level guard did — same escape hatch,
@@ -243,4 +246,24 @@ export class DefaultDatasourcePlugin implements Plugin {
243246
ctx.logger.debug('[DefaultDatasourcePlugin] metadata service unavailable — default not listed', { error: e });
244247
}
245248
}
249+
250+
/**
251+
* Kernel teardown (ADR-0062 D5, #3993): the default disconnects through the
252+
* SAME service that connected it — one teardown implementation, mirroring
253+
* the one connect implementation. The service resolves the driver the way
254+
* the default was registered (natural name via `asDefault`) and honours
255+
* ownership: a host-owned ADOPTED instance (`createPrebuiltDriverFactory`,
256+
* the cloud compositions — pools shared beyond this kernel) is never
257+
* closed here; only the retained verdict is cleared. Note the kernel's
258+
* teardown phase is `destroy()` — `stop()` exists nowhere in the Plugin
259+
* contract and is never called.
260+
*/
261+
destroy = async () => {
262+
try {
263+
await this.connection?.disconnect('default', { asDefault: true });
264+
} catch {
265+
// Teardown is best-effort — the kernel is going away either way, and a
266+
// failed disconnect must not mask the real shutdown path.
267+
}
268+
};
246269
}

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,80 @@ describe('createPrebuiltDriverFactory — through DatasourceConnectionService (t
112112
expect(driver.calls).toContain('connect');
113113
});
114114

115+
it("stamps the handle 'host'-owned, and kernel teardown leaves the adopted pool alone (#3993)", async () => {
116+
// The cloud constraint: the adopted instance's pool outlives this kernel
117+
// (proxy base / registry cache). disconnect() must clear the retained
118+
// verdict WITHOUT closing the pool.
119+
const driver = stubDriver('turso');
120+
const factory = createPrebuiltDriverFactory(driver, { driverId: 'turso' });
121+
const handle: any = await factory.create({ driver: 'turso', config: {} });
122+
expect(handle.ownership).toBe('host');
123+
124+
const { svc } = service(factory);
125+
const result = await svc.connect(
126+
{ name: 'default', driver: 'turso', config: {}, origin: 'code', bootCritical: true },
127+
{ asDefault: true, context: { origin: 'code', trigger: 'declared-auto' } },
128+
);
129+
expect(result.ownership).toBe('host');
130+
expect(svc.getConnectionState('default')?.ownership).toBe('host');
131+
132+
await svc.disconnect('default', { asDefault: true });
133+
expect(driver.calls).not.toContain('disconnect'); // the pool belongs to the host
134+
expect(svc.getConnectionState('default')).toBeUndefined(); // the verdict is gone
135+
});
136+
137+
it('a FACTORY-built default IS disconnected at teardown — natural-name resolution via asDefault (#3993)', async () => {
138+
// Without ownership the instance was built for this connect: teardown may
139+
// close it. `asDefault` must resolve the driver under its natural name —
140+
// `getDriverByName('default')` can never find it (#3826).
141+
const driver = stubDriver('sql');
142+
const factoryBuilt = {
143+
supports: () => true,
144+
create: () => ({
145+
connect: async () => { await driver.connect(); },
146+
disconnect: async () => { await driver.disconnect(); },
147+
driver,
148+
}),
149+
};
150+
const { svc } = service(factoryBuilt as any);
151+
await svc.connect(
152+
{ name: 'default', driver: 'sqlite', config: {}, origin: 'code', bootCritical: true },
153+
{ asDefault: true, context: { origin: 'code', trigger: 'declared-auto' } },
154+
);
155+
await svc.disconnect('default', { asDefault: true });
156+
expect(driver.calls).toContain('disconnect');
157+
expect(svc.getConnectionState('default')).toBeUndefined();
158+
});
159+
160+
it("disconnectAll() closes only what THIS service opened — 'connected' states, never 'already-registered' (#3993)", async () => {
161+
const opened = stubDriver('sql');
162+
const factoryBuilt = {
163+
supports: () => true,
164+
create: () => ({ disconnect: async () => { await opened.disconnect(); }, driver: opened }),
165+
};
166+
const { svc, drivers } = service(factoryBuilt as any);
167+
// A driver someone ELSE registered (the D8 onEnable escape hatch): the
168+
// idempotency guard records `already-registered` — not ours to close.
169+
const foreign = stubDriver('warehouse');
170+
drivers.set('warehouse', foreign);
171+
const pre = await svc.connect(
172+
{ name: 'warehouse', driver: 'sqlite', config: {}, autoConnect: true },
173+
{ context: { origin: 'code', trigger: 'declared-auto' } },
174+
);
175+
expect(pre.status).toBe('already-registered');
176+
// A pool this service opened.
177+
await svc.connect(
178+
{ name: 'analytics', driver: 'sqlite', config: {}, autoConnect: true },
179+
{ context: { origin: 'code', trigger: 'declared-auto' } },
180+
);
181+
182+
await svc.disconnectAll();
183+
expect(opened.calls).toContain('disconnect');
184+
expect(foreign.calls).not.toContain('disconnect');
185+
expect(svc.getConnectionState('analytics')).toBeUndefined();
186+
expect(svc.getConnectionState('warehouse')).toBeDefined(); // untouched
187+
});
188+
115189
it('a failing adopted instance takes the SAME bootCritical fail-fast verdict', async () => {
116190
const driver = stubDriver('turso', { failConnect: true });
117191
const { svc } = service(createPrebuiltDriverFactory(driver, { driverId: 'turso' }));

packages/services/service-datasource/src/contracts/datasource-driver-factory.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,19 @@ export interface DatasourceConnectionSpec {
3737
pool?: Record<string, unknown>;
3838
}
3939

40+
/**
41+
* Who owns the TEARDOWN of the driver instance behind a handle (ADR-0062 D5,
42+
* #3993):
43+
* - `'factory'` (the default when absent) — the factory built this instance
44+
* for this connect; the connection service may disconnect it when the
45+
* kernel tears down.
46+
* - `'host'` — an adopted, pre-built instance (`createPrebuiltDriverFactory`)
47+
* whose lifecycle outlives the kernel (a pool shared across environment
48+
* kernels, a control-plane driver doubling as a proxy base). Kernel
49+
* teardown must NEVER disconnect it; the host does, on its own schedule.
50+
*/
51+
export type DatasourceDriverOwnership = 'factory' | 'host';
52+
4053
/**
4154
* A live (or lazily-connecting) driver handle. Intentionally structural and
4255
* fully optional so any concrete driver satisfies it — the admin service uses
@@ -47,6 +60,8 @@ export interface DatasourceDriverHandle {
4760
connect?(): Promise<void>;
4861
/** Close the connection / pool. */
4962
disconnect?(): Promise<void>;
63+
/** Teardown ownership of the underlying instance — see {@link DatasourceDriverOwnership}. */
64+
ownership?: DatasourceDriverOwnership;
5065
/** Cheap liveness round-trip (preferred for probes). */
5166
ping?(): Promise<unknown>;
5267
/** Introspect the live schema (fallback probe when `ping` is absent). */

0 commit comments

Comments
 (0)