From ce381f88a3ee99f8ded2895a723c702f6fe589e4 Mon Sep 17 00:00:00 2001 From: zhuangjianguo Date: Mon, 31 Aug 2026 13:26:21 +0000 Subject: [PATCH 1/8] fix(objectql,service-datasource): give the driver registry an eviction door, so a deleted datasource stops draining /ready (#13578) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ObjectQL driver registry had a `registerDriver` door and no counterpart, so nothing could ever leave it. `DELETE /api/v1/datasources/:name` emptied the admin door while `GET /api/v1/ready` kept naming the deleted datasource's driver — the probe reports whatever `checkDriversHealth()` finds in that registry — leaving a process restart on every replica as the only recovery. `IObjectQLEngine` gains `unregisterDriver(name)`. The registry owns the invariant rather than each caller, because removal moves three pieces of private engine state that a caller can reach none of: the `drivers` map, the `defaultDriver` NAME (a stale one answers with a driver that is gone), and the datasource def, which has no removal door of its own. Wired into the three lifecycle paths that already funnel through teardown: datasource delete / pool teardown, failed-start rollback, and engine destroy. Eviction is per-replica, symmetric with how registration already works. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- .changeset/driver-registry-eviction.md | 37 +++ .../src/engine-driver-eviction.test.ts | 217 ++++++++++++++++++ .../src/engine-primary-datasource.test.ts | 18 +- packages/objectql/src/engine.ts | 75 ++++++ .../src/registry-eviction-readiness.test.ts | 184 +++++++++++++++ .../datasource-connection-service.test.ts | 83 +++++++ .../src/datasource-connection-service.ts | 46 ++++ .../spec/src/contracts/objectql-engine.ts | 14 ++ 8 files changed, 666 insertions(+), 8 deletions(-) create mode 100644 .changeset/driver-registry-eviction.md create mode 100644 packages/objectql/src/engine-driver-eviction.test.ts create mode 100644 packages/runtime/src/registry-eviction-readiness.test.ts diff --git a/.changeset/driver-registry-eviction.md b/.changeset/driver-registry-eviction.md new file mode 100644 index 0000000000..12cd4266cd --- /dev/null +++ b/.changeset/driver-registry-eviction.md @@ -0,0 +1,37 @@ +--- +"@objectstack/objectql": patch +"@objectstack/service-datasource": patch +"@objectstack/spec": patch +--- + +Deleting a datasource now evicts its driver from the data-engine registry, so `/api/v1/ready` recovers without a process restart + +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. diff --git a/packages/objectql/src/engine-driver-eviction.test.ts b/packages/objectql/src/engine-driver-eviction.test.ts new file mode 100644 index 0000000000..c1466cf84a --- /dev/null +++ b/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 = {}): 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']); + }); + }); +}); diff --git a/packages/objectql/src/engine-primary-datasource.test.ts b/packages/objectql/src/engine-primary-datasource.test.ts index 8ba275ce7b..22b0f348d2 100644 --- a/packages/objectql/src/engine-primary-datasource.test.ts +++ b/packages/objectql/src/engine-primary-datasource.test.ts @@ -213,14 +213,16 @@ describe('ObjectQL.resolvePrimaryDatasource() — the #13408 criterion', () => { }); it('a registered system object bound nowhere at all ⇒ no answer can be true', () => { - // ⚠️ STRUCTURAL close, not a live production state today: `registerDriver` - // makes the FIRST driver the default (`isDefault || drivers.size === 1`), - // so step 5 always answers once any driver exists, and this branch is - // reachable only with none registered. It is pinned anyway because the - // engine has no driver eviction YET — adding it is #13578's half of this - // card — and 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. + // `registerDriver` makes the FIRST driver the default (`isDefault || + // drivers.size === 1`), so step 5 always answers once any driver exists + // and this branch needs none registered. + // + // No longer only structural: #13578 landed `unregisterDriver`, and an + // eviction that removes the default is exactly how a registered system + // object stops being bound anywhere. This reading — "cannot tell", never + // a name — is what that eviction was required to preserve, and it is now + // reached from the live direction too, in + // `engine-driver-eviction.test.ts`. const engine = newEngine(); registerSys(engine, 'sys_user'); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 158532ae8e..ee562f66fb 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -5212,6 +5212,72 @@ export class ObjectQL implements IObjectQLEngine { } } + /** + * Evict a driver from the registry — the removal counterpart of + * {@link registerDriver} (#13578). + * + * ## Why the registry owns this rather than each lifecycle path + * + * #13578 named the fork explicitly: the delete path calling the registry, or + * the registry owning its own liveness. This is the second, and the reason is + * structural rather than stylistic — removing a driver is not one deletion + * but THREE pieces of private engine state that must move together, and a + * caller can reach none of them: + * + * 1. `drivers` — the Map {@link checkDriversHealth} iterates, and therefore + * the one a readiness probe reports. This is the entry #13578 watched + * survive a `DELETE /api/v1/datasources/:name` on every replica. + * 2. `defaultDriver` — a NAME, not a reference. Dropping the entry without + * clearing it leaves the default pointing at a driver that is gone, and + * {@link getDefaultDriverName} would answer with a name nothing backs. + * `engine-primary-datasource.test.ts` wrote the required reading down + * before this method existed: after an eviction that removes the default, + * the primary verdict must read "cannot tell", never a name. + * 3. `datasourceDefs` — the declarative `schemaMode` / `external` record + * {@link assertWriteAllowed} consults. It has a `registerDatasourceDef` + * door and no removal door at all, so a def outliving its driver keeps + * judging writes for a datasource that no longer exists. + * + * Only (1) is even visible from outside, and no public door removed it. So + * "every future lifecycle path remembers to clear three maps in the right + * order" is a rule with nowhere to live where it would be read — the shape + * this repo has already paid for. One primitive owns the invariant instead, + * and every path calls it exactly once. + * + * ⛔ Deliberately does NOT clear `unavailableDatasources`. That map already + * has its own door ({@link clearDatasourceUnavailable}) and its own caller, + * and the two acts are ordered OPPOSITELY on the failed-start path: a connect + * that fails after registering evicts the driver and then MARKS the + * datasource unavailable. Folding the clear in here would have eviction wipe + * the explanation the very next step writes. + * + * ⛔ Deliberately does NOT disconnect the driver. Eviction and teardown are + * separate acts: the pool's owner decides whether to close it — an ADOPTED + * host-owned instance outlives this kernel by design (ADR-0062 D5) — while + * the registry decides only whether this engine still routes to it. Folding + * them would make eviction close a pool the host still holds. + * + * @returns `true` when a driver entry was removed, `false` when the name held + * none — so an idempotent caller (a retried delete, a teardown sweep + * following a partial one) can tell a removal from a no-op. + */ + unregisterDriver(name: string): boolean { + const removed = this.drivers.delete(name); + // Cleared regardless of `removed`: a datasource that never connected has a + // def and no driver, so keying this off the driver entry would strand + // exactly the rows a failed datasource leaves behind. + this.datasourceDefs.delete(name); + if (this.defaultDriver === name) { + this.defaultDriver = null; + this.logger.info( + 'Evicted the DEFAULT driver — this engine has no default until one is registered', + { driverName: name }, + ); + } + if (removed) this.logger.info('Unregistered driver', { driverName: name }); + return removed; + } + /** * Register a Datasource *definition* (ADR-0015). * @@ -7783,6 +7849,15 @@ export class ObjectQL implements IObjectQLEngine { this.logger.error('Error disconnecting driver', e as Error, { driverName: name }); } } + + // #13578 — teardown is an eviction path too, and it used to disconnect + // every driver while leaving all of them REGISTERED. A destroyed engine + // therefore still answered `checkDriversHealth()` by pinging pools it had + // just closed, so a readiness probe racing shutdown read a list of drivers + // that were deliberately dead. Disconnect-then-evict, through the one + // primitive, so `defaultDriver` and the datasource defs go with them. + // Names are snapshotted first: `unregisterDriver` mutates the Map. + for (const name of [...this.drivers.keys()]) this.unregisterDriver(name); this.logger.info('ObjectQL engine destroyed'); } diff --git a/packages/runtime/src/registry-eviction-readiness.test.ts b/packages/runtime/src/registry-eviction-readiness.test.ts new file mode 100644 index 0000000000..d14b1ecc9e --- /dev/null +++ b/packages/runtime/src/registry-eviction-readiness.test.ts @@ -0,0 +1,184 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #13578 — the BEHAVIOURAL pin: after a datasource is deleted, `GET +// /api/v1/ready` stops naming its driver. +// +// The card was reported as an operational fact, not a structural one — "the +// admin-door list is empty on every replica … but `/ready` still names that +// datasource's driver … only a process restart clears it". A pin on +// `unregisterDriver()` alone would not have caught it: the defect was that +// nothing on the delete path CALLED the registry, and the probe reads the +// registry through two packages' worth of indirection. +// +// So this suite deliberately uses no doubles for the three things under test: +// +// - the REAL `ObjectQL` engine, which owns the driver registry; +// - the REAL `DatasourceConnectionService.disconnect()`, which is the funnel +// `DELETE /api/v1/datasources/:name` reaches via +// `removeDatasource` → `tryUnregisterPool` → `unregisterPool`; +// - the REAL `HttpDispatcher` `/ready` handler, which is the door the +// operator actually watched stay 503. +// +// `packages/runtime` is the only package that depends on all three, which is +// why the pin lives here and not beside either half. +// +// ⛔ The probe half is not re-pinned here — `http-dispatcher.ready.test.ts` +// owns the #13408 drain semantics, and this suite must not become a second +// opinion on them. What it pins is only that eviction reaches the probe. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { DatasourceConnectionService } from '@objectstack/service-datasource'; +import type { IDataDriver } from '@objectstack/spec/contracts'; +import { HttpDispatcher, type HttpDispatcherResult } from './http-dispatcher.js'; + +/** See the sibling suites: annotated so the contract, not the call site, fails first. */ +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 kernel(engine: unknown): any { + return { + getState: () => 'running', + getService: (name: string) => (name === 'data' ? engine : undefined), + getServiceAsync: async () => undefined, + }; +} + +/** + * `HttpDispatcherResult.response` is optional, so every read is a + * `possibly undefined` in a type-checked program — and this package's test + * layer IS type-checked by `check:type-check-debt`. Narrowed loudly, the shape + * lifted from `http-dispatcher.ready.test.ts` rather than invented again. + */ +function responseOf(res: HttpDispatcherResult): NonNullable { + const { response } = res; + if (!response) throw new Error('GET /ready answered no response at all'); + return response; +} + +/** What `/ready` says right now: the status, and every driver name it mentions. */ +async function ready(engine: unknown): Promise<{ status: number; named: string[] }> { + const res = await new HttpDispatcher(kernel(engine)).dispatch('GET', '/ready', undefined, undefined, {} as any); + const response = responseOf(res); + const body: any = response.body; + // Both shapes are read, deliberately: an unhealthy driver is named in + // `details.drivers` on the 503 and in `data.degraded.drivers` on the #13408 + // secondary-degraded 200. The card's symptom is "still NAMES it", so the + // assertion must not be able to pass merely because the envelope changed. + const named = [ + ...(body?.error?.details?.drivers ?? body?.details?.drivers ?? []), + ...(body?.data?.degraded?.drivers ?? []), + ]; + return { status: response.status, named: [...named].map(String).sort() }; +} + +/** The delete funnel, wired exactly as `datasource-admin-plugin` wires it. */ +function connectionServiceFor(engine: ObjectQL): DatasourceConnectionService { + return new DatasourceConnectionService({ + factory: () => undefined, + engine: () => engine, + }); +} + +describe('#13578 — DELETE of a datasource stops /ready naming its driver', () => { + it('the reported defect, end to end: after the delete funnel runs, /ready recovers without a restart', async () => { + const engine = newEngine(); + engine.registerDriver(driver('postgres_primary'), true); + engine.registerDriver(driver('stuck_mongo', { healthy: false })); + + // The state the operator observed: the app is healthy, one datasource's + // driver is stuck, and the probe names it. + const before = await ready(engine); + expect(before.status).toBe(503); + expect(before.named).toContain('stuck_mongo'); + + // `DELETE /api/v1/datasources/stuck_mongo` reaches exactly this call. + await connectionServiceFor(engine).disconnect('stuck_mongo'); + + // The recovery the card says only a process restart could produce. + const after = await ready(engine); + expect(after.named).not.toContain('stuck_mongo'); + expect(after.status).toBe(200); + }); + + it('POSITIVE CONTROL — a second stuck datasource is still named, and the healthy one survives', async () => { + // Without this, a fix that emptied the registry (or that made `/ready` + // stop reporting drivers at all) would pass the pin above. Deleting ONE + // datasource must recover exactly that one. + const engine = newEngine(); + engine.registerDriver(driver('postgres_primary'), true); + engine.registerDriver(driver('stuck_a', { healthy: false })); + engine.registerDriver(driver('stuck_b', { healthy: false })); + + expect((await ready(engine)).named).toEqual(['stuck_a', 'stuck_b']); + + await connectionServiceFor(engine).disconnect('stuck_a'); + + const after = await ready(engine); + expect(after.named).toEqual(['stuck_b']); + expect(after.status).toBe(503); + // The untouched datasources are still routable — eviction is not a reset. + expect(engine.getDriverByName('postgres_primary')).toBeDefined(); + expect(engine.getDriverByName('stuck_b')).toBeDefined(); + }); + + it('the pool is CLOSED as well as evicted — the delete does not leak the socket', async () => { + // Eviction must not become a shortcut past teardown: `disconnect()` + // resolves the driver out of the registry, so an eviction ordered before + // the close would drop the only handle that could close it. + let closed = false; + const engine = newEngine(); + engine.registerDriver(driver('primary'), true); + engine.registerDriver({ + ...driver('leaky', { healthy: false }), + disconnect: async () => { closed = true; }, + }); + + await connectionServiceFor(engine).disconnect('leaky'); + + expect(closed).toBe(true); + expect(engine.getDriverByName('leaky')).toBeUndefined(); + }); + + it('deleting the DEFAULT datasource evicts it under its NATURAL name', async () => { + // #3826: the default driver is registered under its own name, never under + // the literal 'default'. An eviction keyed on the datasource name would + // remove nothing here and report success — the exit-0-did-nothing shape. + const engine = newEngine(); + engine.registerDriver(driver('sqlite_main', { healthy: false }), true); + + expect((await ready(engine)).named).toContain('sqlite_main'); + + await connectionServiceFor(engine).disconnect('default', { asDefault: true }); + + expect((await ready(engine)).named).not.toContain('sqlite_main'); + expect(engine.getDriverByName('sqlite_main')).toBeUndefined(); + expect(engine.getDefaultDriverName()).toBeUndefined(); + }); +}); diff --git a/packages/services/service-datasource/src/__tests__/datasource-connection-service.test.ts b/packages/services/service-datasource/src/__tests__/datasource-connection-service.test.ts index f63d96e958..14d34f5e7d 100644 --- a/packages/services/service-datasource/src/__tests__/datasource-connection-service.test.ts +++ b/packages/services/service-datasource/src/__tests__/datasource-connection-service.test.ts @@ -61,6 +61,13 @@ function fakeEngine() { unavailable.delete(name); cleared.push(name); }, + // #13578 — the double implements the eviction door for the same reason it + // mirrors `registerDriver`'s skip-if-present above: a fake that silently + // lacks a member the seam declares turns every assertion about that member + // into a vacuous pass. `ConnectionEngineLike` is `Partial<…>`, so an absent + // `unregisterDriver` would make the optional call a no-op and the eviction + // pins below would hold against a service that evicted nothing. + unregisterDriver: (name) => drivers.delete(name), }; return engine; } @@ -991,6 +998,82 @@ describe('retained connection state (framework#3827)', () => { expect(engine!.unavailable.has('analytics')).toBe(false); }); + // ── #13578: the registry entries this service is responsible for removing ── + + it('#13578 — disconnect() EVICTS the driver, not merely the verdict', async () => { + // The reported defect at this seam. `disconnect()` closed the pool and + // dropped the retained state, but left the driver instance registered, so + // `checkDriversHealth()` — and through it `GET /api/v1/ready` — kept naming + // a datasource whose record the admin door had already deleted. + const { service, engine } = svc({ factory: fakeFactory() }); + await service.connect(analytics, { context: { trigger: 'runtime-admin' } }); + expect(engine!.drivers.has('analytics')).toBe(true); + + await service.disconnect('analytics'); + + expect(engine!.drivers.has('analytics')).toBe(false); + }); + + it('#13578 — evicts ONLY the named datasource', async () => { + // Positive control: an eviction that cleared the registry would pass the + // pin above and fail this one. + const { service, engine } = svc({ factory: fakeFactory() }); + await service.connect({ ...analytics, name: 'keep' }, { context: { trigger: 'runtime-admin' } }); + await service.connect({ ...analytics, name: 'drop' }, { context: { trigger: 'runtime-admin' } }); + + await service.disconnect('drop'); + + expect(engine!.drivers.has('drop')).toBe(false); + expect(engine!.drivers.has('keep')).toBe(true); + }); + + it('#13578 — a connect that throws AFTER registering rolls the registration back', async () => { + // The failed-start path the card asked to be walked. Registration happens + // partway through `attemptConnect`, so a throw after it used to return + // `failed-degraded` while leaving a live entry behind: a datasource the + // admin list reports as FAILED whose driver the readiness probe still pings + // — an orphan no door could reach. + const engine = fakeEngine(); + engine.registerDatasourceDef = () => { + throw new Error('boom — a step after registerDriver failed'); + }; + const service = new DatasourceConnectionService({ + factory: () => fakeFactory(), + engine: () => engine, + }); + + const result = await service.connect(analytics, { context: { trigger: 'runtime-admin' } }); + + expect(result.status).toBe('failed-degraded'); + expect(engine.drivers.has('analytics')).toBe(false); + // The rollback must NOT swallow the diagnosis: eviction deliberately leaves + // `unavailableDatasources` alone, and the mark is written after it. + expect(engine.unavailable.get('analytics')?.kind).toBe('failed'); + }); + + it('#13578 — the rollback never evicts a driver this attempt did not register', async () => { + // `registerDriver` keeps the incumbent when a name is already held, so + // "we called register" is not evidence that "we registered". Rolling back + // on that basis would let one datasource's failed connect evict another + // owner's live driver — a worse defect than the leak. + const engine = fakeEngine(); + engine.drivers.set('analytics', { name: 'analytics' }); + engine.registerDatasourceDef = () => { + throw new Error('boom'); + }; + const service = new DatasourceConnectionService({ + factory: () => fakeFactory(), + engine: () => engine, + }); + + // `already-registered` short-circuits before the build, so force the path + // by asking for the default-driver branch, whose guard tests for A default + // rather than for THIS name. + await service.connect(analytics, { context: { trigger: 'runtime-admin' }, asDefault: true }); + + expect(engine.drivers.get('analytics')).toEqual({ name: 'analytics' }); + }); + it('lists every retained verdict for the admin surface', async () => { const { service } = svc({ factory: fakeFactory({ connectThrows: true }) }); await service.connect({ ...analytics, name: 'a' }, { context: { trigger: 'runtime-admin' } }); diff --git a/packages/services/service-datasource/src/datasource-connection-service.ts b/packages/services/service-datasource/src/datasource-connection-service.ts index 9d6d966dbf..116bba6591 100644 --- a/packages/services/service-datasource/src/datasource-connection-service.ts +++ b/packages/services/service-datasource/src/datasource-connection-service.ts @@ -135,6 +135,7 @@ export type ConnectionEngineLike = Partial< | 'getDefaultDriverName' | 'markDatasourceUnavailable' | 'clearDatasourceUnavailable' + | 'unregisterDriver' > >; @@ -587,6 +588,13 @@ export class DatasourceConnectionService { } } + // #13578 — set to the registry key ONLY when this attempt is the thing that + // put it there, so the rollback below can never evict a driver someone else + // owns. `registerDriver` is a no-op on a name it already holds (it keeps the + // incumbent and discards the newcomer), and the `asDefault` branch's + // idempotency guard above tests for A default rather than for THIS name — so + // "we called register" is not evidence that "we registered". + let registeredByThisAttempt: string | undefined; try { const handle = await factory.create({ ...toSpec(record), ...(secret ? { secret } : {}) }); if (typeof handle?.connect === 'function') await handle.connect(); @@ -615,7 +623,17 @@ export class DatasourceConnectionService { /* frozen driver — registration may still work if name already matches */ } } + // Fails SAFE when the engine cannot answer: an engine without + // `getDriverByName` (the seam is `Partial<…>`) leaves us unable to tell + // our registration from someone else's, and evicting on a guess is the + // worse error of the two — so assume it was already held and roll nothing + // back. + const heldBefore = + typeof engine.getDriverByName === 'function' + ? engine.getDriverByName(engineDriver.name) !== undefined + : true; engine.registerDriver(engineDriver, opts.asDefault === true); + if (!heldBefore) registeredByThisAttempt = engineDriver.name; engine.registerDatasourceDef?.({ name, schemaMode: record.schemaMode, @@ -645,6 +663,18 @@ export class DatasourceConnectionService { this.logger?.info?.(`datasource '${name}': connected (driver=${record.driver}, schemaMode=${record.schemaMode ?? 'managed'})`); return { name, status: 'connected', ...(handle.ownership ? { ownership: handle.ownership } : {}) }; } catch (err) { + // #13578 — failed-start ROLLBACK, the first path the card asked to be + // walked. Registration happens partway through this block, so a throw + // after it used to return `failed-degraded` while leaving a live entry in + // the registry: a datasource the admin list reports as failed, whose + // driver the readiness probe still pings. Narrow today (the steps after + // `registerDriver` are individually guarded), but the window is real and + // the rollback is what makes "failed ⇒ not registered" true by + // construction rather than by the current arrangement of the lines. + // + // Before `handleFailure`, which records the unavailable mark: eviction + // deliberately does not touch that map, and the mark must outlive it. + if (registeredByThisAttempt) engine.unregisterDriver?.(registeredByThisAttempt); // `err` itself is handed on, not just its message: the driver package's // build output being absent is reported as `err.code === // 'ERR_MODULE_NOT_FOUND'`, and that structured signal is gone the moment @@ -686,6 +716,22 @@ export class DatasourceConnectionService { // datasource that no longer exists in that state. this.states.delete(name); engine?.clearDatasourceUnavailable?.(name); + + // #13578 — the eviction this method never did. Closing the pool above only + // ends the CONNECTION; the driver instance stayed in the engine registry, + // so `checkDriversHealth()` (and through it `GET /api/v1/ready`) kept + // naming a datasource whose record had already been deleted, with a process + // restart as the only recovery. + // + // It goes LAST, and after the disconnect: `driver` above is resolved out of + // the registry, so evicting first would leave the pool open with nothing + // left holding a handle to close it. + // + // `driverName` rather than `name`: the DEFAULT driver is registered under + // its NATURAL name (#3826), so `name` ('default') matches no registry key + // and evicting by it would silently remove nothing — the same + // exit-0-and-did-nothing shape this fix exists to remove. + if (driverName) engine?.unregisterDriver?.(driverName); } /** diff --git a/packages/spec/src/contracts/objectql-engine.ts b/packages/spec/src/contracts/objectql-engine.ts index e64619453c..11e1d15fb5 100644 --- a/packages/spec/src/contracts/objectql-engine.ts +++ b/packages/spec/src/contracts/objectql-engine.ts @@ -306,6 +306,20 @@ export interface IObjectQLEngine extends IDataEngine { // ── Boot-time wiring (AppPlugin / metadata-protocol) ───────────────── /** Register a driver; the optional second argument makes it the default. */ registerDriver(driver: IDataDriver, isDefault?: boolean): void; + /** + * Evict a driver from the registry — `registerDriver`'s removal counterpart + * (#13578). Returns `true` when an entry was removed. + * + * Declared here because its ABSENCE was the defect: the registry had a + * registration door and no eviction door, so a datasource deleted through + * the admin API left its driver instance registered and the readiness probe + * (`checkDriversHealth`) kept reporting it until the process restarted. + * + * Evicting only stops this engine routing to the driver. It does NOT + * disconnect the pool — teardown belongs to whoever owns the pool, which + * for an adopted host-owned instance is not this engine (ADR-0062 D5). + */ + unregisterDriver(name: string): boolean; /** Install the stack's datasource-mapping rules. Rule shape is engine-local; see `setDatasourceMapping` on the class. */ setDatasourceMapping(rules: unknown[]): void; /** Register an app/plugin manifest (objects, apps, metadata items) — MetadataProtocolPlugin's table-provisioning path. */ From f0ffff954f88f4e3d9a43b04bce5c5f5efae9ecd Mon Sep 17 00:00:00 2001 From: zhuangjianguo Date: Mon, 31 Aug 2026 13:47:31 +0000 Subject: [PATCH 2/8] test(service-datasource): take the ConnectionEngineLike roster pin from seven members to eight `unregisterDriver` widens the seam the datasource connection service drives the engine through, and the roster pin exists so that widening is a decision written down rather than a side effect of editing the type. Restated deliberately, with a return-type pin: the eviction door answers `boolean` so an idempotent caller can tell a removal from a no-op. Part of #13578 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- .../connection-engine-like-contract.test.ts | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/packages/services/service-datasource/src/__tests__/connection-engine-like-contract.test.ts b/packages/services/service-datasource/src/__tests__/connection-engine-like-contract.test.ts index fd436bb9d1..33854bbc03 100644 --- a/packages/services/service-datasource/src/__tests__/connection-engine-like-contract.test.ts +++ b/packages/services/service-datasource/src/__tests__/connection-engine-like-contract.test.ts @@ -19,6 +19,17 @@ import type { IDataDriver, IObjectQLEngine } from '@objectstack/spec/contracts'; import type { ConnectionEngineLike } from '../datasource-connection-service.js'; describe('ConnectionEngineLike is the contract, not a fork of it (#12010)', () => { + it('unregisterDriver answers a boolean — a no-op is distinguishable from a removal', () => { + // #13578. `void` would have made "evicted it" and "there was nothing to + // evict" indistinguishable to an idempotent caller (a retried DELETE, a + // teardown sweep after a partial one), which is the same + // exit-0-and-did-nothing shape the eviction fix exists to remove. + type Answer = ReturnType>; + const exact: [Answer] extends [boolean] ? ([boolean] extends [Answer] ? 'exact' : never) : never = + 'exact'; + expect(exact).toBe('exact'); + }); + it('the real engine contract is assignable to this view', () => { // The defect this card measured, stated as a compile: with // `registerDriver?: (driver: unknown, …)` the engine was NOT assignable @@ -63,10 +74,19 @@ describe('ConnectionEngineLike is the contract, not a fork of it (#12010)', () = expect(exact).toBe('exact'); }); - it('declares exactly the seven derived members, each identical to its contract member', () => { + it('declares exactly the eight derived members, each identical to its contract member', () => { + // #13578 added `unregisterDriver`, taking the roster from seven to eight. + // The roster is deliberately restated rather than derived from the seam: + // its whole purpose is that widening the view is a decision someone writes + // down here, not a side effect of editing the type. The eighth member is + // `registerDriver`'s removal counterpart — the service needs it because + // `disconnect()` has to evict the driver it just closed, and a datasource + // deleted from the admin door was otherwise left in the engine registry + // where the readiness probe kept reporting it. type Declared = keyof ConnectionEngineLike; type Expected = | 'registerDriver' + | 'unregisterDriver' | 'registerDatasourceDef' | 'getDriverByName' | 'syncObjectSchema' @@ -89,6 +109,7 @@ describe('ConnectionEngineLike is the contract, not a fork of it (#12010)', () = : never; const members: { [K in Expected]: Same } = { registerDriver: 'same', + unregisterDriver: 'same', registerDatasourceDef: 'same', getDriverByName: 'same', syncObjectSchema: 'same', @@ -98,7 +119,7 @@ describe('ConnectionEngineLike is the contract, not a fork of it (#12010)', () = }; expect(roster).toBe('exact'); - expect(Object.keys(members)).toHaveLength(7); + expect(Object.keys(members)).toHaveLength(8); }); it('every member stays OPTIONAL — the graceful-degradation seam', () => { From 3259302525c681a65e2d992e922e0859c92c3fa1 Mon Sep 17 00:00:00 2001 From: zhuangjianguo Date: Mon, 31 Aug 2026 13:57:31 +0000 Subject: [PATCH 3/8] docs(permissions): re-anchor the system-context census after the engine.ts insertion Pure line rot: `unregisterDriver` lands above every cited elevation-read site in packages/objectql/src/engine.ts, shifting all 11 anchors by the method's length. Rewritten by the gate's own `--fix`; no census row's meaning changes. Part of #13578 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- content/docs/permissions/system-context.mdx | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 2242da20b7..8b40db154f 100644 --- a/content/docs/permissions/system-context.mdx +++ b/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:1576` | -| 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:1451` (#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` | From 1776353f21cd649d6404fac87a04ee630ca0f258 Mon Sep 17 00:00:00 2001 From: zhuangjianguo Date: Mon, 31 Aug 2026 16:09:50 +0000 Subject: [PATCH 4/8] docs(permissions): re-anchor the system-context census after merging main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The page is routed to `merge=os-regen` in .gitattributes, so the merge took one side whole with zero conflict markers — here, this branch's side — silently dropping main's own re-anchor after #13843 grew packages/rest/src/rest-server.ts. Regenerated with the gate's own --fix, which recomputes every anchor from the actual source positions. Verified rather than assumed: 65 table rows in, 65 out; with line numbers normalised the page is identical to main's apart from the objectql/src/engine.ts anchors; and every one of those shifts matches this branch's two insertion hunks exactly (+66 for sites between them, +75 for sites after the destroy() change). No row deleted, none reworded. Part of #13578 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- content/docs/permissions/system-context.mdx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 8b40db154f..b9a102806b 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -64,7 +64,7 @@ not on any flag. ## How the flag is set `isSystem` is **server-constructed and never client-supplied**. Inbound HTTP -cannot set it (`packages/rest/src/rest-server.ts:1240`, `:1269`), and neither +cannot set it (`packages/rest/src/rest-server.ts:1302`, `:1331`), and neither can an action body (`packages/runtime/src/domains/actions.ts:404`). It is written by internal callers only, as an option on the engine call: @@ -103,7 +103,7 @@ that silently does not happen. | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:246` | | 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `read-audit.ts:556` | | 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `payload-redaction-middleware.ts:115` | -| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1272` | +| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1334` | ### 2. Write pipeline and data integrity @@ -158,7 +158,7 @@ The largest single consumer — **20 of the 109 sites**. |:--|:---|:---|:---|:---| | 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:136` | | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:399` | -| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4408`, `:5771`, `:6019`, `:6450`, `:6643` | +| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4470`, `:5833`, `:6081`, `:6512`, `:6705` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | | 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:246`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | @@ -196,10 +196,10 @@ 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: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:1451` (#3493 / #6640) | +| "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:1514` (#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` | -| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1240`, `:1269`; `domains/actions.ts:404` | +| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1302`, `:1331`; `domains/actions.ts:404` | --- From 628a0134ec3125aba14e0b13f0950c7c901ee7a8 Mon Sep 17 00:00:00 2001 From: zhuangjianguo Date: Mon, 31 Aug 2026 16:44:16 +0000 Subject: [PATCH 5/8] chore(changeset): grade @objectstack/spec as minor with a BREAKING banner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `IObjectQLEngine.unregisterDriver` is a REQUIRED member on a published interface: additive for consumers, compile-breaking for any third-party implementer. Regraded from patch to 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 so broke nobody by construction. A required member grading below that is inconsistent. Banner shape verified against #13870 rather than assumed: that changeset does pair a `minor` bump with a `**BREAKING**` line citing the launch-window convention. A strict-semver reading would say `major`; that reading is recorded as an open question for the maintainer in the PR body rather than acted on here, since uniform in-repo precedent is the operative convention and overruling it is not this PR's call. Part of #13578 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- .changeset/driver-registry-eviction.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/.changeset/driver-registry-eviction.md b/.changeset/driver-registry-eviction.md index 12cd4266cd..6dac82c687 100644 --- a/.changeset/driver-registry-eviction.md +++ b/.changeset/driver-registry-eviction.md @@ -1,11 +1,23 @@ --- "@objectstack/objectql": patch "@objectstack/service-datasource": patch -"@objectstack/spec": 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 From 3780e19e74cc59250f25d65eb5d1f3f7dd9215a4 Mon Sep 17 00:00:00 2001 From: zhuangjianguo Date: Mon, 31 Aug 2026 17:00:10 +0000 Subject: [PATCH 6/8] chore(changeset): answer the ADR-0087 question the BREAKING banner now raises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding the banner turned `check-adr-0087-registration` red, correctly: a declared-breaking changeset must state in writing whether the change needs a ledger entry. Answered with the category built for this exact shape — a published runtime TypeScript interface with no metadata surface behind it, so `objectstack migrate meta` has nothing to rewrite and the compiler is the channel that reaches consumers. Part of #13578 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- .changeset/driver-registry-eviction.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.changeset/driver-registry-eviction.md b/.changeset/driver-registry-eviction.md index 6dac82c687..c942c6335b 100644 --- a/.changeset/driver-registry-eviction.md +++ b/.changeset/driver-registry-eviction.md @@ -47,3 +47,5 @@ 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. + + From 606c88f16a8e84bd29efa96f5476338a6d7e115e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 01:09:35 +0000 Subject: [PATCH 7/8] docs(permissions): regenerate the isSystem census after merging main Discharges the `os-regen` merge-driver deferral recorded for `content/docs/permissions/system-context.mdx` by the preceding merge commit. The driver does not text-merge this page, and it kept the branch side whole. That side is correct for this branch's `engine.ts` insertions but stale for everything main landed since the branch was cut, and it silently dropped main's own contribution to the page: an 18-line block explaining what the enforced-declarations row counts, and that row's value (21 -> 22). So the page is rebased on main's version and re-anchored by the gate's own repair (`node scripts/check-system-context-census.mjs --fix`), which rewrote 11 anchors, all of them `objectql/src/engine.ts` line shifts caused by this branch. No census row was added, deleted or re-worded; the totals are unchanged from main's own green run. check-system-context-census: OK - 109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-read. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- content/docs/permissions/system-context.mdx | 28 +++++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index b9a102806b..8af558601f 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -47,7 +47,7 @@ nothing to do with elevation. | Declaration | What it is | This page? | |:---|:---|:---:| | `ExecutionContext.isSystem` — `packages/spec/src/kernel/execution-context.zod.ts:269` | The elevation flag on an operation's context | ✅ | -| `Object.isSystem` — `packages/spec/src/data/object.zod.ts:1588` | Marks a **system object** (protected from deletion; defaults its org-wide sharing to `public` when no `sharingModel` is set) | ❌ | +| `Object.isSystem` — `packages/spec/src/data/object.zod.ts:1595` | Marks a **system object** (protected from deletion; defaults its org-wide sharing to `public` when no `sharingModel` is set) | ❌ | | `EmailTemplate.isSystem` — `packages/spec/src/system/email-template.zod.ts:125` | Built-in template; tenants may override but should not delete | ❌ | | `Environment.isSystem` — `packages/spec/src/cloud/environment.zod.ts:137` | Platform-infrastructure environment, not user data | ❌ | @@ -112,7 +112,7 @@ that silently does not happen. | 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:1576` | +| 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: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` | @@ -135,7 +135,7 @@ The largest single consumer — **20 of the 109 sites**. | 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1286` (guard at `:1311`) | | 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1338` | | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` | -| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:413`, `:467`, `:471`, `:544`, `:574` | +| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:423`, `:477`, `:481`, `:554`, `:584` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | | 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:157`, `:382` | @@ -196,7 +196,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: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:1514` (#3493 / #6640) | +| "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` | | "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1302`, `:1331`; `domains/actions.ts:404` | @@ -306,7 +306,7 @@ still holds equal to the census on every pull request: | — in tests | 1013 | — | | — in non-test sources | 798 | — | | Appearances of the bare identifier `isSystem` in non-test sources | 813 | — | -| — parsed as a declaration | 21 | ✅ | +| — parsed as a declaration | 22 | ✅ | | — parsed as an object-literal / type key (producers and option objects) | 310 | — | | — parsed as a property **read** | 115 | ✅ | | — parsed in some other syntactic position (a local, a cast, a conditional) | 9 | ✅ | @@ -336,6 +336,24 @@ test files certifies nothing. ⛔ Do not re-add them to `DECLARED_COUNTS` — a self-test case in the gate refuses that by name. Re-measure them with `node scripts/isystem-census.mjs` when you want them current, and move the date. +**What the enforced declarations row counts.** Not the four field declarations +above — those are four *distinct fields* that happen to share a name, and only +the first is elevation. This row counts every position where the parser puts the +identifier in a **declaring** slot: those four, plus the structural type literals +that restate `ExecutionContext.isSystem`'s shape inline rather than importing it +(`{ isSystem: true; tenantId?: string }`, `context?: { isSystem?: boolean }`, and +the `get isSystem()` accessor on the engine's context wrapper). A restatement is +a producer's declaration of the shape it will build, never a read, so a new one +moves this count and moves nothing else on this page — the census's read +population, the anchored rows above, and the packages and files totals all stay +where they are. The most recent arrival is the scoped +seed context threaded into the org-admin permission-set lookup in +`plugins/plugin-security/src/auto-org-admin-grant.ts`, so that read resolves +against the granting organization's own catalog row rather than an +organization-less one (#11670). ⛔ Cited without a line number deliberately: an +anchor here would be refused, and rightly — this page anchors elevation +**reads**, and a declaration is not one. + Counting by hand is what made the previous edition wrong in two independent ways, so both are worth naming. Its headline said "80 distinct sites across 18 packages" while its own tables anchored **77** — the number never matched the From fc45a54aa0f0b7157ca31737af09620fc9eca54c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 01:11:29 +0000 Subject: [PATCH 8/8] docs(permissions): re-anchor the isSystem census after the second main merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discharges the `os-regen` deferral recorded by the preceding merge commit. Main's side of the page carried no prose or count change this time — its whole delta was line anchors moved by #13910 in `packages/rest`. So the gate's own repair re-derives them: 10 anchors rewritten, every one a `rest-server.ts` shift. No census row added, deleted or re-worded. check-system-context-census: OK - 109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-read. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- content/docs/permissions/system-context.mdx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 8af558601f..ed4a084ea2 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -64,7 +64,7 @@ not on any flag. ## How the flag is set `isSystem` is **server-constructed and never client-supplied**. Inbound HTTP -cannot set it (`packages/rest/src/rest-server.ts:1302`, `:1331`), and neither +cannot set it (`packages/rest/src/rest-server.ts:1389`, `:1418`), and neither can an action body (`packages/runtime/src/domains/actions.ts:404`). It is written by internal callers only, as an option on the engine call: @@ -103,7 +103,7 @@ that silently does not happen. | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:246` | | 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `read-audit.ts:556` | | 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `payload-redaction-middleware.ts:115` | -| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1334` | +| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1421` | ### 2. Write pipeline and data integrity @@ -158,7 +158,7 @@ The largest single consumer — **20 of the 109 sites**. |:--|:---|:---|:---|:---| | 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:136` | | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:399` | -| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4470`, `:5833`, `:6081`, `:6512`, `:6705` | +| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4573`, `:5936`, `:6184`, `:6615`, `:6808` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | | 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:246`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | @@ -199,7 +199,7 @@ assuming `isSystem` covers it is a documented source of bugs. | "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` | -| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1302`, `:1331`; `domains/actions.ts:404` | +| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1389`, `:1418`; `domains/actions.ts:404` | ---