Give the driver registry an eviction door, so a deleted datasource stops draining /ready

.changeset/driver-registry-eviction.md

@@ -0,0 +1,51 @@
+---
+"@objectstack/objectql": patch
+"@objectstack/service-datasource": patch
+"@objectstack/spec": minor
+---
+
+Deleting a datasource now evicts its driver from the data-engine registry, so `/api/v1/ready` recovers without a process restart
+
+**BREAKING** `IObjectQLEngine` gains a REQUIRED member, shipped as `minor`
+under the repo's launch-window convention for breaking changes.
+
+`unregisterDriver(name: string): boolean` is additive for CONSUMERS — nothing
+they already call changes — but it breaks any third-party *implementer* of
+`IObjectQLEngine` at compile time, and the interface is on the published
+surface (`packages/spec/src/contracts/index.ts` re-exports it and `./contracts`
+is a published export path). Graded `minor` to match this contract's own
+precedent: the three prior changes to it all took `minor`, including one that
+added five members that were **all optional** and therefore broke nobody by
+construction. A required member grading below that would be inconsistent.
+
+The ObjectQL driver registry had a `registerDriver` door and no counterpart, so
+nothing could ever leave it. Deleting a datasource emptied the admin door while
+`GET /api/v1/ready` kept naming the deleted datasource's driver — the readiness
+probe reports whatever `checkDriversHealth()` finds in that registry — and on a
+multi-replica deployment the only recovery was restarting every process.
+
+`IObjectQLEngine` gains `unregisterDriver(name)`, the removal counterpart of
+`registerDriver`. The registry owns the invariant rather than each caller: an
+eviction has to drop the driver entry, clear the `defaultDriver` NAME when it
+pointed at the evicted driver (otherwise `getDefaultDriverName()` answers with a
+name nothing backs), and drop the datasource definition that has no removal door
+of its own. Evicting does not disconnect the pool — teardown belongs to whoever
+owns it.
+
+Three lifecycle paths now use it:
+
+- **Datasource delete / pool teardown** — `DatasourceConnectionService.disconnect()`
+  evicts after closing the pool, which is the path `DELETE /api/v1/datasources/:name`
+  reaches. The default driver is evicted under its natural registered name.
+- **Failed-start rollback** — a connect that throws after registering now rolls
+  that registration back, instead of leaving a driver the admin list reports as
+  failed and the readiness probe still pings.
+- **Engine teardown** — `destroy()` disconnects and then evicts, so a destroyed
+  engine no longer reports drivers whose pools it has already closed.
+
+Eviction is per-replica, matching how driver registration already works
+(each replica registers pools from the shared datasource records at boot);
+propagating it cluster-wide would need a broadcast channel the driver registry
+does not have today.
+
+<!-- adr-0087: not-required (no-migration-prescription) this change is purely ADDITIVE: `IObjectQLEngine` gains a member and nothing is renamed, retired or converted, so there is no authored metadata for `objectstack migrate meta` to rewrite and no migration to prescribe. The breaking half is compile-time only, against third-party IMPLEMENTERS of the interface; consumers are unaffected. Deliberately NOT claimed as runtime-interface-only: this gate classifies packages/spec/src/contracts/** as a metadata surface, so that category is false here even though the symbol is a TypeScript interface with no Zod schema behind it. -->

content/docs/permissions/system-context.mdx

@@ -109,18 +109,18 @@ that silently does not happen.
 
 | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor |
 |:--|:---|:---|:---|:---|
-| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:10712` |
-| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:10874` |
-| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9605` |
+| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:10787` |
+| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:10949` |
+| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9680` |
 | 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1664` |
-| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9642`, `readonly-strict-errors.ts:66` |
-| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5639` |
+| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9717`, `readonly-strict-errors.ts:66` |
+| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5705` |
 | 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3574`, `:3584`, `:3611` |
 | 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` |
 | 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:98` |
-| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6337` |
-| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11460` |
-| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11389` |
+| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6403` |
+| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11535` |
+| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11464` |
 
 ### 3. Sharing (`plugin-sharing`)
 
@@ -180,7 +180,7 @@ a reader tracing where elevation travels needs them.
 | # | Site | Package | What it does |
 |:--|:---|:---|:---|
 | 62 | `objectql/src/engine.ts:3406` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes |
-| 63 | `objectql/src/engine.ts:13801` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag |
+| 63 | `objectql/src/engine.ts:13876` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag |
 | 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report |
 | 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across |
 
@@ -195,7 +195,7 @@ assuming `isSystem` covers it is a documented source of bugs.
 |:---|:---|:---|
 | "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1909` (rationale at `:1819`–`1821`, #3760), `flow.zod.ts:685` |
 | "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) |
-| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9588`–`9605` |
+| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9663`–`9680` |
 | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1516` (#3493 / #6640) |
 | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` |
 | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:286` |

packages/objectql/src/engine-driver-eviction.test.ts

@@ -0,0 +1,217 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// #13578 — the registry's missing removal door.
+//
+// The observed defect was operational: `DELETE /api/v1/datasources/:name`
+// emptied the admin door on every replica while `GET /api/v1/ready` kept naming
+// the deleted datasource's driver, recoverable only by restarting every
+// process. The cause is here rather than at the probe — the driver registry had
+// a `registerDriver` and no counterpart, so nothing could ever leave it.
+//
+// These pins hold the PRIMITIVE's invariants. The behavioural pin that the
+// readiness probe actually stops naming an evicted datasource — driven through
+// the real delete funnel — is
+// `packages/runtime/src/registry-eviction-readiness.test.ts`, because only that
+// package sees the engine, the connection service and the dispatcher at once.
+
+import { describe, it, expect } from 'vitest';
+import type { IDataDriver } from '@objectstack/spec/contracts';
+import { ObjectQL } from './engine.js';
+
+/**
+ * A registrable driver double, annotated `IDataDriver` for the reason the
+ * sibling `engine-primary-datasource.test.ts` fixture states: an un-annotated
+ * literal is checked only at the call site and drifts silently as the contract
+ * grows.
+ */
+const driver = (name: string, opts: { healthy?: boolean } = {}): IDataDriver => ({
+  name,
+  version: '1.0.0',
+  supports: {},
+  connect: async () => {},
+  disconnect: async () => {},
+  checkHealth: async () => opts.healthy !== false,
+  find: async () => [],
+  findOne: async () => null,
+  create: async (_o, data) => ({ id: '1', ...data }),
+  update: async (_o, id, data) => ({ id, ...data }),
+  upsert: async (_o, data) => ({ id: '1', ...data }),
+  delete: async () => true,
+  count: async () => 0,
+  bulkCreate: async () => [],
+  bulkUpdate: async () => [],
+  bulkDelete: async () => {},
+  execute: async () => null,
+  beginTransaction: async () => ({}),
+  commit: async () => {},
+  rollback: async () => {},
+  syncSchema: async () => {},
+  dropTable: async () => {},
+});
+
+function newEngine(): ObjectQL {
+  return new ObjectQL({ logger: { debug() {}, info() {}, warn() {}, error() {} } } as any);
+}
+
+function registerSys(engine: ObjectQL, name: string, extra: Record<string, unknown> = {}): void {
+  engine.registry.registerObject(
+    { name, fields: { title: { type: 'text' } }, ...extra } as any,
+    'platform-objects',
+    undefined,
+    'own',
+  );
+}
+
+const healthNames = async (engine: ObjectQL) =>
+  (await engine.checkDriversHealth()).map((r) => r.driverName).sort();
+
+describe('#13578 — ObjectQL.unregisterDriver(), the registry\'s removal door', () => {
+  it('the entry the readiness probe reads is actually gone', async () => {
+    // The defect in one assertion: before this method existed, there was no
+    // call that could make `checkDriversHealth()` stop reporting a name.
+    const engine = newEngine();
+    engine.registerDriver(driver('primary'), true);
+    engine.registerDriver(driver('stuck', { healthy: false }));
+
+    expect(await healthNames(engine)).toEqual(['primary', 'stuck']);
+
+    expect(engine.unregisterDriver('stuck')).toBe(true);
+
+    expect(await healthNames(engine)).toEqual(['primary']);
+    expect(engine.getDriverByName('stuck')).toBeUndefined();
+  });
+
+  it('evicts ONLY the named driver — the sibling entries are untouched', async () => {
+    // The positive control. An eviction that cleared the Map, or that keyed off
+    // the wrong name, would pass the assertion above and fail this one.
+    const engine = newEngine();
+    engine.registerDriver(driver('keep_a'), true);
+    engine.registerDriver(driver('drop'));
+    engine.registerDriver(driver('keep_b'));
+
+    engine.unregisterDriver('drop');
+
+    expect(await healthNames(engine)).toEqual(['keep_a', 'keep_b']);
+    expect(engine.getDriverByName('keep_a')).toBeDefined();
+    expect(engine.getDriverByName('keep_b')).toBeDefined();
+  });
+
+  it('returns false for a name the registry never held — a no-op is distinguishable', () => {
+    // An idempotent caller (a retried DELETE, a teardown sweep after a partial
+    // one) has to be able to tell "removed" from "there was nothing there";
+    // a `void` return would have made both look identical.
+    const engine = newEngine();
+    engine.registerDriver(driver('only'), true);
+
+    expect(engine.unregisterDriver('never_registered')).toBe(false);
+    expect(engine.getDriverByName('only')).toBeDefined();
+  });
+
+  describe('the invariants a caller could not have maintained itself', () => {
+    it('evicting the DEFAULT clears the default NAME, so nothing answers with a dead one', () => {
+      // `defaultDriver` is a name, not a reference. Deleting the Map entry
+      // alone would leave `getDefaultDriverName()` answering 'main' with
+      // nothing behind it — a worse state than the leak, because callers treat
+      // that answer as a live routing target.
+      const engine = newEngine();
+      engine.registerDriver(driver('main'), true);
+      expect(engine.getDefaultDriverName()).toBe('main');
+
+      engine.unregisterDriver('main');
+
+      expect(engine.getDefaultDriverName()).toBeUndefined();
+    });
+
+    it('after evicting the default, the #13408 primary verdict reads "cannot tell", never a name', () => {
+      // `engine-primary-datasource.test.ts` wrote this requirement down before
+      // eviction existed: "an eviction that removes the default is exactly how
+      // a registered system object stops being bound anywhere. When that lands,
+      // this must already read as 'cannot tell', not as a name." This is that
+      // reading, now driven by the real eviction rather than by an engine that
+      // never had a driver.
+      const engine = newEngine();
+      engine.registerDriver(driver('sqlite'), true);
+      registerSys(engine, 'sys_user');
+      expect(engine.resolvePrimaryDatasource()).toEqual({
+        resolved: true,
+        datasource: 'sqlite',
+        witnesses: 1,
+      });
+
+      engine.unregisterDriver('sqlite');
+
+      // Unresolved ⇒ the readiness caller drains, which is the ruled
+      // fail-toward-draining direction. ⛔ Never `{ resolved: true, datasource:
+      // 'sqlite' }` read off a stale default.
+      expect(engine.resolvePrimaryDatasource()).toEqual({
+        resolved: false,
+        reason: 'system-object-unbound',
+      });
+    });
+
+    it('the datasource DEF goes with the driver — it has no removal door of its own', () => {
+      // `registerDatasourceDef` is public and there is no `unregisterDatasourceDef`,
+      // so a def outliving its driver is unreachable state: the write gate keeps
+      // judging writes against `external.allowWrites` for a datasource that no
+      // longer exists.
+      const engine = newEngine();
+      engine.registerDriver(driver('warehouse'));
+      engine.registerDatasourceDef({
+        name: 'warehouse',
+        schemaMode: 'external',
+        external: { allowWrites: false },
+      } as any);
+      expect(engine.listDatasourceDefs().map((d) => d.name)).toContain('warehouse');
+
+      engine.unregisterDriver('warehouse');
+
+      expect(engine.listDatasourceDefs().map((d) => d.name)).not.toContain('warehouse');
+    });
+
+    it('drops a def even when no driver was ever registered under the name', () => {
+      // A datasource that never connected leaves a def and no driver. Keying
+      // the def removal off "a driver was removed" would strand exactly the
+      // rows a FAILED datasource leaves behind — the population this card is
+      // about.
+      const engine = newEngine();
+      engine.registerDatasourceDef({ name: 'never_connected', schemaMode: 'external' } as any);
+      expect(engine.listDatasourceDefs().map((d) => d.name)).toContain('never_connected');
+
+      expect(engine.unregisterDriver('never_connected')).toBe(false);
+
+      expect(engine.listDatasourceDefs().map((d) => d.name)).not.toContain('never_connected');
+    });
+  });
+
+  describe('teardown is an eviction path too', () => {
+    it('destroy() leaves the registry EMPTY, not merely disconnected', async () => {
+      // Before #13578 `destroy()` disconnected every driver and left them all
+      // registered, so a destroyed engine still answered `checkDriversHealth()`
+      // by pinging pools it had just closed.
+      const engine = newEngine();
+      engine.registerDriver(driver('a'), true);
+      engine.registerDriver(driver('b'));
+      expect(await healthNames(engine)).toEqual(['a', 'b']);
+
+      await engine.destroy();
+
+      expect(await healthNames(engine)).toEqual([]);
+      expect(engine.getDefaultDriverName()).toBeUndefined();
+    });
+
+    it('destroy() still disconnects every driver before evicting it', async () => {
+      // The ordering guard: evicting first would drop the only handle able to
+      // close the pool, turning a leak of registry entries into a leak of
+      // sockets. Eviction must not be a shortcut past teardown.
+      const closed: string[] = [];
+      const engine = newEngine();
+      for (const name of ['a', 'b']) {
+        engine.registerDriver({ ...driver(name), disconnect: async () => { closed.push(name); } });
+      }
+
+      await engine.destroy();
+
+      expect(closed.sort()).toEqual(['a', 'b']);
+    });
+  });
+});