From 85c37f6334623a4190be4ac2cd13d8f73c2db048 Mon Sep 17 00:00:00 2001 From: os-steve Date: Mon, 31 Aug 2026 08:19:35 +0000 Subject: [PATCH 1/5] wip(runtime): partition /ready driver health into primary vs secondary (#13408) --- .../objectql/src/driver-connect-errors.ts | 48 ++++ .../src/engine-primary-datasource.test.ts | 225 ++++++++++++++++++ packages/objectql/src/engine.ts | 98 +++++++- packages/objectql/src/index.ts | 2 + .../runtime/src/http-dispatcher.ready.test.ts | 201 ++++++++++++++++ packages/runtime/src/http-dispatcher.ts | 210 ++++++++++++++-- 6 files changed, 762 insertions(+), 22 deletions(-) create mode 100644 packages/objectql/src/engine-primary-datasource.test.ts diff --git a/packages/objectql/src/driver-connect-errors.ts b/packages/objectql/src/driver-connect-errors.ts index c944243ea5..ffd11cf887 100644 --- a/packages/objectql/src/driver-connect-errors.ts +++ b/packages/objectql/src/driver-connect-errors.ts @@ -19,6 +19,54 @@ export interface DriverHealth { skipped?: boolean; } +/** + * Why {@link PrimaryDatasourceVerdict} could not name a primary datasource. + * + * Every member means the SAME thing to a readiness probe — *we cannot tell* — + * and the ruling on #13408 fixes what a probe must do with that answer: fall + * back to the old whole-node 503 (fail toward draining), never to "don't + * drain". They are distinguished only so the reason can be logged and reported; + * ⛔ a caller must not branch on them to keep a replica in rotation. + */ +export type PrimaryDatasourceUnresolvedReason = + /** No platform system object is registered here — nothing to read the fact off. */ + | 'no-system-objects-registered' + /** The platform's system objects are split across more than one datasource. */ + | 'system-objects-split' + /** A registered system object routes nowhere: no binding and no default driver. */ + | 'system-object-unbound' + /** The name resolved, but no driver is registered under it, so nothing probes it. */ + | 'no-driver-registered'; + +/** + * WHICH datasource carries this deployment's platform system objects — the + * machine-readable "primary/default datasource" fact the #13408 ruling requires + * (2026-08-31, 第 6 场总监席决裁批 #12), stated as a verdict rather than a + * `string | undefined` so "we could not tell" can never be mistaken for a name. + * + * ⛔ The ruling forbids deriving this from a heuristic such as "the first + * datasource registered". The one implementation is + * `ObjectQL.resolvePrimaryDatasource()`. + */ +export type PrimaryDatasourceVerdict = + | { + resolved: true; + /** The datasource name, which is also a registered driver's name. */ + datasource: string; + /** + * How many registered platform system objects agreed on it. Never 0 — a + * verdict read off nothing is `no-system-objects-registered`, not a name, + * so a caller cannot act on a vacuous agreement. + */ + witnesses: number; + } + | { + resolved: false; + reason: PrimaryDatasourceUnresolvedReason; + /** The distinct datasource names seen, when more than one disagreed. */ + candidates?: readonly string[]; + }; + /** `error.message` when it is an Error, its string form otherwise. */ function failureMessage(error: unknown): string { if (error instanceof Error) return error.message || error.name; diff --git a/packages/objectql/src/engine-primary-datasource.test.ts b/packages/objectql/src/engine-primary-datasource.test.ts new file mode 100644 index 0000000000..2138f6353f --- /dev/null +++ b/packages/objectql/src/engine-primary-datasource.test.ts @@ -0,0 +1,225 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #13408 — WHICH datasource is the primary one, i.e. the one whose failure must +// take a replica out of the load-balancer rotation. +// +// Ruled 2026-08-31 (第 6 场总监席决裁批 #12, maintainer verbatim 「同意」): +// +// 「主/默认」判据必须是一条读得出来的事实:定义为「承载平台系统对象(sys_*)的 +// 那个数据源」或等价的可机读事实,⛔ 不得用「第一个注册的」之类启发式。 +// +// These pins hold BOTH halves of that: the criterion answers from where the +// platform's system objects actually live, and it refuses to answer — rather +// than guessing — whenever that fact is not readable. The consequence of the +// refusal (drain the node) is pinned on the caller, in +// `packages/runtime/src/http-dispatcher.ready.test.ts`; here we only pin that +// the refusal happens and is not silently a name. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; + +const driver = (name: string) => ({ + name, + version: '1.0.0', + supports: {}, + connect: async () => {}, + disconnect: async () => {}, + checkHealth: async () => true, + find: async () => [], + findOne: async () => null, + create: async (_o: string, data: any) => ({ id: '1', ...data }), + update: async (_o: string, id: string, data: any) => ({ id, ...data }), + delete: async () => true, + count: async () => 0, + bulkCreate: async () => [], + bulkUpdate: async () => [], + bulkDelete: async () => {}, + execute: async () => ({}), + beginTransaction: async () => ({}), + commit: async () => {}, + rollback: async () => {}, + syncSchema: async () => {}, +}); + +function newEngine(): ObjectQL { + return new ObjectQL({ logger: { debug() {}, info() {}, warn() {}, error() {} } } as any); +} + +/** + * Register a PLATFORM object under a name that is really on + * `PLATFORM_PROVIDED_OBJECT_NAMES` — a name off that list is not evidence and + * would make every reading here vacuous. + */ +function registerSys( + engine: ObjectQL, + name: string, + extra: Record = {}, +): void { + engine.registry.registerObject( + { name, fields: { title: { type: 'text' } }, ...extra } as any, + 'platform-objects', + undefined, + 'own', + ); +} + +describe('ObjectQL.resolvePrimaryDatasource() — the #13408 criterion', () => { + describe('reads the fact off where the platform system objects live', () => { + it('names the datasource carrying sys_* in a single-datasource deployment', () => { + const engine = newEngine(); + engine.registerDriver(driver('sqlite'), true); + registerSys(engine, 'sys_user'); + registerSys(engine, 'sys_organization'); + + const verdict = engine.resolvePrimaryDatasource(); + + expect(verdict).toEqual({ resolved: true, datasource: 'sqlite', witnesses: 2 }); + }); + + it('⛔ NOT the first-registered driver, and ⛔ NOT the one flagged default', () => { + // The sharpest anti-heuristic pin the ruling asks for. `mongo` is + // registered FIRST and `pg` is the flagged DEFAULT — so a "first + // registered" heuristic answers `mongo` and a `getDefaultDriverName()` + // shortcut answers `pg`. The system objects are routed to `mongo` by a + // mapping rule, so the only answer read off WHERE THE DATA IS is `mongo`. + const engine = newEngine(); + engine.registerDriver(driver('mongo')); + engine.registerDriver(driver('pg'), true); + engine.setDatasourceMapping([{ objectPattern: 'sys_*', datasource: 'mongo' }]); + registerSys(engine, 'sys_user'); + + // The two heuristics the ruling forbids, stated so the pin is falsifiable + // rather than a coincidence of this fixture: both are LIVE and both + // disagree with the verdict. + expect(engine.getDefaultDriverName()).toBe('pg'); + + expect(engine.resolvePrimaryDatasource()).toEqual({ + resolved: true, + datasource: 'mongo', + witnesses: 1, + }); + }); + + it('a tenant/secondary datasource does not become primary by existing', () => { + const engine = newEngine(); + engine.registerDriver(driver('pg'), true); + engine.registerDriver(driver('tenant_mongo')); + registerSys(engine, 'sys_user'); + // A tenant business object on the secondary — the #13408 shape. + engine.registry.registerObject( + { name: 'tenant_lead', datasource: 'tenant_mongo', fields: { n: { type: 'text' } } } as any, + 'tenant-pkg', + undefined, + 'own', + ); + + expect(engine.resolvePrimaryDatasource()).toEqual({ + resolved: true, + datasource: 'pg', + witnesses: 1, + }); + }); + }); + + describe('ADR-0057 §3.6 lifecycle separation is not a split', () => { + it('an audit-class sys object on the telemetry datasource still leaves ONE primary', () => { + // Without the carve-out this deployment would read as `system-objects-split` + // and drain forever — for a configuration the platform recommends. + const engine = newEngine(); + engine.registerDriver(driver('pg'), true); + engine.registerDriver(driver('telemetry')); + registerSys(engine, 'sys_user'); + registerSys(engine, 'sys_audit_log', { lifecycle: { class: 'audit' } }); + + // Non-vacuity: the ledger object really IS routed away, so the carve-out + // is doing work rather than describing a case that cannot arise. + expect(engine.resolveEffectiveDatasource('sys_audit_log')).toBe('telemetry'); + + expect(engine.resolvePrimaryDatasource()).toEqual({ + resolved: true, + datasource: 'pg', + witnesses: 1, + }); + }); + + it('a `transient` sys object still votes — it stays on the primary by design', () => { + const engine = newEngine(); + engine.registerDriver(driver('pg'), true); + engine.registerDriver(driver('telemetry')); + registerSys(engine, 'sys_session', { lifecycle: { class: 'transient' } }); + + expect(engine.resolvePrimaryDatasource()).toEqual({ + resolved: true, + datasource: 'pg', + witnesses: 1, + }); + }); + }); + + describe('refuses to answer rather than guessing', () => { + it('no platform system object registered ⇒ no fact to read', () => { + const engine = newEngine(); + engine.registerDriver(driver('sqlite'), true); + engine.registry.registerObject( + { name: 'lead', fields: { n: { type: 'text' } } } as any, 'app', undefined, 'own', + ); + + expect(engine.resolvePrimaryDatasource()).toEqual({ + resolved: false, + reason: 'no-system-objects-registered', + }); + }); + + it('system objects split across datasources ⇒ ambiguous, with the candidates named', () => { + const engine = newEngine(); + engine.registerDriver(driver('pg'), true); + engine.registerDriver(driver('mongo')); + registerSys(engine, 'sys_user'); + registerSys(engine, 'sys_organization', { datasource: 'mongo' }); + + expect(engine.resolvePrimaryDatasource()).toEqual({ + resolved: false, + reason: 'system-objects-split', + candidates: ['mongo', 'pg'], + }); + }); + + it('a bound datasource with no registered driver ⇒ nothing probes it, so no name', () => { + // The silent don't-drain the ruling forbids: `checkDriversHealth()` only + // reports REGISTERED drivers, so an unregistered primary would be absent + // from the unhealthy list and read as healthy. + const engine = newEngine(); + engine.registerDriver(driver('pg'), true); + registerSys(engine, 'sys_user', { datasource: 'never_connected' }); + + expect(engine.resolvePrimaryDatasource()).toEqual({ + resolved: false, + reason: 'no-driver-registered', + candidates: ['never_connected'], + }); + }); + + it('a registered system object bound nowhere at all ⇒ no answer can be true', () => { + const engine = newEngine(); + // No default driver: nothing to fall through to at step 5. + engine.registerDriver(driver('pg')); + registerSys(engine, 'sys_user'); + + expect(engine.resolvePrimaryDatasource()).toEqual({ + resolved: false, + reason: 'system-object-unbound', + }); + }); + + it('every unresolved verdict is structurally unusable as a name', () => { + // The consequence contract: a caller cannot accidentally read a + // datasource out of a refusal, whatever the reason. + const engine = newEngine(); + engine.registerDriver(driver('sqlite'), true); + const verdict = engine.resolvePrimaryDatasource(); + + expect(verdict.resolved).toBe(false); + expect((verdict as Record).datasource).toBeUndefined(); + }); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 54c7009208..158532ae8e 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -99,6 +99,7 @@ import { type DriverHealth, type DatasourceUnavailableInfo, type DatasourceUnavailableKind, + type PrimaryDatasourceVerdict, } from './driver-connect-errors.js'; import { resolveAllowDriverConnectFailure } from '@objectstack/types'; // [#5979] The ONE shared "which read failure is benign?" predicate (#4825 @@ -146,7 +147,7 @@ import { normalizeTenancyPosture, type TenancyPosture } from '@objectstack/spec/ export type InsertManyRowOutcome = | { ok: true; record: any } | { ok: false; error: unknown }; -import { CoreServiceName, StorageNameMapping } from '@objectstack/spec/system'; +import { CoreServiceName, StorageNameMapping, PLATFORM_PROVIDED_OBJECT_NAMES } from '@objectstack/spec/system'; import { IRealtimeService, RealtimeEventPayload } from '@objectstack/spec/contracts'; import { BulkDataEventSchema, @@ -7059,6 +7060,101 @@ export class ObjectQL implements IObjectQLEngine { ); } + /** + * WHICH datasource is this deployment's PRIMARY one — the single + * implementation of the criterion ruled on #13408 (2026-08-31, 第 6 场总监席 + * 决裁批 #12, maintainer verbatim 「同意」). + * + * The ruling's words, quoted rather than paraphrased because the constraint + * is on the *kind* of answer, not only the answer: + * + * > 「主/默认」判据必须是一条读得出来的事实:定义为「承载平台系统对象(sys_*)的 + * > 那个数据源」或等价的可机读事实,⛔ 不得用「第一个注册的」之类启发式。 + * + * So the fact read here is: **where do this deployment's platform system + * objects actually live**, answered through {@link resolveDatasourceBinding} + * — the same five-step order every query routes by. Not registration order, + * not `getDefaultDriverName()` on its own. `getDefaultDriverName()` names the + * driver flagged default at registration, which is a *configuration* input to + * step 5, not evidence about where anything is stored; a deployment that + * routes `sys_*` elsewhere by an explicit binding or a `datasourceMapping` + * rule would make that answer confidently wrong, and wrong in the direction + * that keeps a dead replica in rotation. + * + * ## Who votes + * + * The curated `PLATFORM_PROVIDED_OBJECT_NAMES` registry (`@objectstack/spec`) + * intersected with what this registry actually holds — a name no package + * registered here says nothing about this deployment, and asking + * `resolveDatasourceBinding` about it would produce a step-5 answer read off + * an object that does not exist. + * + * **Minus the ADR-0057 §3.6 system ledgers** ({@link isSystemLedgerObject} — + * `audit` / `telemetry` / `event`), which are *deliberately* routed off the + * primary when a `telemetry` datasource is registered. They are excluded + * because including them would report `system-objects-split` for every + * deployment that adopted lifecycle separation — a permanent drain-always + * verdict for a configuration the platform recommends. `transient` is + * deliberately NOT excluded, matching step 3: those objects stay on the + * primary, so they are evidence about it. + * + * ## What it refuses to answer + * + * Disagreement, silence and a name with no driver behind it all return + * `resolved: false`. That is the ruling's 「错向红」 requirement: + * + * > 判据解析失败或歧义时 ⇒ fail toward draining(宁可误摘不可静默保留) + * + * ⛔ The direction lives in the CALLER, because only the caller can drain — + * this method's contract is that it never invents a name, and a caller that + * treats `resolved: false` as "no primary is unhealthy" inverts the ruling. + * The readiness caller's pins are in + * `packages/runtime/src/http-dispatcher.ready.test.ts`. + * + * Never throws for a routing reason: every failure is a verdict. (A registry + * that throws is not modelled here — that is a broken kernel, and the + * readiness caller catches it into the same drain direction.) + */ + resolvePrimaryDatasource(): PrimaryDatasourceVerdict { + const byDatasource = new Map(); + for (const objectName of PLATFORM_PROVIDED_OBJECT_NAMES) { + // Not registered in THIS deployment (OSS runtime vs cloud, plugins not + // installed) — no evidence either way, so it does not vote. + if (!this._registry.getObject(objectName)) continue; + // ADR-0057 §3.6: deliberately off the primary. See the header. + if (this.isSystemLedgerObject(objectName)) continue; + + const binding = this.resolveDatasourceBinding(objectName); + // A registered platform object that routes NOWHERE is a broken + // deployment, not a quiet abstention: there is no binding and no default + // driver, so no answer about "the primary" can be true. + if (!binding) return { resolved: false, reason: 'system-object-unbound' }; + + byDatasource.set(binding.datasource, (byDatasource.get(binding.datasource) ?? 0) + 1); + } + + if (byDatasource.size === 0) return { resolved: false, reason: 'no-system-objects-registered' }; + if (byDatasource.size > 1) { + return { + resolved: false, + reason: 'system-objects-split', + candidates: [...byDatasource.keys()].sort(), + }; + } + + const [[datasource, witnesses]] = [...byDatasource]; + // Steps 1-2 of the resolution order answer with a name even when nothing is + // registered under it (that is deliberate — `getDriver` throws loudly on + // it). A readiness probe cannot use such a name: `checkDriversHealth()` + // reports only REGISTERED drivers, so an unregistered primary would be + // absent from the unhealthy list and read as healthy — the silent + // don't-drain the ruling forbids. + if (!this.drivers.has(datasource)) { + return { resolved: false, reason: 'no-driver-registered', candidates: [datasource] }; + } + return { resolved: true, datasource, witnesses }; + } + /** * Does this object declare a media field at all? Cached per schema object — * the registry hands back the same instance per object, so this scans a diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 0b618e677d..b503262b2b 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -110,6 +110,8 @@ export type { DriverHealth, DatasourceUnavailableInfo, DatasourceUnavailableKind, + PrimaryDatasourceVerdict, + PrimaryDatasourceUnresolvedReason, } from './driver-connect-errors.js'; export type { InsertManyRowOutcome } from './engine.js'; // [#5696] Thrown by `transaction(cb, base, { require: true })` when the diff --git a/packages/runtime/src/http-dispatcher.ready.test.ts b/packages/runtime/src/http-dispatcher.ready.test.ts index 1829fdf21b..c88cc02a13 100644 --- a/packages/runtime/src/http-dispatcher.ready.test.ts +++ b/packages/runtime/src/http-dispatcher.ready.test.ts @@ -15,6 +15,25 @@ function engine(results: Array<{ driverName: string; healthy: boolean }>) { return { checkDriversHealth: vi.fn(async () => results) }; } +/** + * An engine that ALSO answers the #13408 primary-datasource criterion. + * + * Deliberately a second helper: `engine()` above stays the engine that predates + * the criterion, which is itself one of the drain cases pinned below. Folding + * the two would delete that control. + */ +function engineWithPrimary( + results: Array<{ driverName: string; healthy: boolean }>, + verdict: unknown, +) { + return { + checkDriversHealth: vi.fn(async () => results), + resolvePrimaryDatasource: vi.fn(() => verdict), + }; +} + +const primary = (datasource: string) => ({ resolved: true, datasource, witnesses: 3 }); + describe('HttpDispatcher — GET /ready readiness probe', () => { it('returns 200 when the kernel is running', async () => { const res = await new HttpDispatcher(kernel('running')).dispatch('GET', '/ready', undefined, undefined, ctx); @@ -84,6 +103,188 @@ describe('HttpDispatcher — GET /ready readiness probe', () => { }); }); +// #13408 — ruled 2026-08-31 (第 6 场总监席决裁批 #12, maintainer verbatim +// 「同意」), Option B: only the PRIMARY/default datasource's failure drains the +// node. A secondary/tenant datasource's failure is REPORTED and drains nothing. +// +// 「主/默认」判据必须是一条读得出来的事实 … ⛔ 不得用「第一个注册的」之类启发式。 +// 错向红钉为交付要件:判据解析失败或歧义时 ⇒ fail toward draining(宁可误摘不可 +// 静默保留),并有钉断言这个方向。 +// +// framework#3756 is NOT overturned: its quantified reason ("a replica that would +// fail 100% of its requests") still holds in the single-datasource deployment it +// was measured on, and the first suite below pins that that deployment's answer +// is unchanged down to the response body. +describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', () => { + describe('the single-datasource deployment is unchanged', () => { + it('the primary going down still drains, with the pre-#13408 envelope', async () => { + const res = await new HttpDispatcher( + kernel('running', engineWithPrimary([{ driverName: 'pg', healthy: false }], primary('pg'))), + ).dispatch('GET', '/ready', undefined, undefined, ctx); + + expect(res.response.status).toBe(503); + expect(res.response.body.error.message).toBe('Data driver unavailable'); + // Byte-identical to the shape #3756 shipped — an operator's alerting on + // this body must not be able to tell that the handler changed. + expect(res.response.body.error.details).toEqual({ state: 'running', drivers: ['pg'] }); + }); + + it('the all-healthy 200 body carries NO degraded key', async () => { + const res = await new HttpDispatcher( + kernel('running', engineWithPrimary([{ driverName: 'pg', healthy: true }], primary('pg'))), + ).dispatch('GET', '/ready', undefined, undefined, ctx); + + expect(res.response.status).toBe(200); + expect(res.response.body.data).toEqual({ status: 'ready', state: 'running' }); + expect(res.response.body.data).not.toHaveProperty('degraded'); + }); + + it('does not even ASK which datasource is primary while everything is healthy', () => { + // The carve-out is strict: a deployment with no unhealthy driver takes + // the identical path it took before, so the new criterion cannot + // introduce a way to LOSE a 200. + const e = engineWithPrimary([{ driverName: 'pg', healthy: true }], primary('pg')); + return new HttpDispatcher(kernel('running', e)) + .dispatch('GET', '/ready', undefined, undefined, ctx) + .then(() => { + expect(e.resolvePrimaryDatasource).not.toHaveBeenCalled(); + }); + }); + }); + + describe('a SECONDARY datasource failure is reported, not drained', () => { + it('returns 200 and names the failed driver in degraded.drivers', async () => { + // The card's own shape: one tenant mongo datasource whose driver cannot + // start, while Postgres and the app are healthy. Before #13408 this + // answered 503 on every replica and drained the whole deployment. + const res = await new HttpDispatcher( + kernel('running', engineWithPrimary( + [{ driverName: 'pg', healthy: true }, { driverName: 'tenant_mongo', healthy: false }], + primary('pg'), + )), + ).dispatch('GET', '/ready', undefined, undefined, ctx); + + expect(res.response.status).toBe(200); + expect(res.response.body.data.status).toBe('ready'); + // ⛔ The rejected fourth option — filtering the bad driver out so it + // becomes invisible — would show an EMPTY degraded list here. + expect(res.response.body.data.degraded).toEqual({ + drivers: ['tenant_mongo'], + primaryDatasource: 'pg', + }); + }); + + it('still drains when the primary is down TOO, even beside a healthy secondary', async () => { + const res = await new HttpDispatcher( + kernel('running', engineWithPrimary( + [{ driverName: 'pg', healthy: false }, { driverName: 'tenant_mongo', healthy: true }], + primary('pg'), + )), + ).dispatch('GET', '/ready', undefined, undefined, ctx); + + expect(res.response.status).toBe(503); + expect(res.response.body.error.details.drivers).toEqual(['pg']); + }); + + it('drains when BOTH are down — the primary is in the unhealthy set', async () => { + const res = await new HttpDispatcher( + kernel('running', engineWithPrimary( + [{ driverName: 'pg', healthy: false }, { driverName: 'tenant_mongo', healthy: false }], + primary('pg'), + )), + ).dispatch('GET', '/ready', undefined, undefined, ctx); + + expect(res.response.status).toBe(503); + expect(res.response.body.error.details.drivers).toEqual(['pg', 'tenant_mongo']); + }); + }); + + // ⭐ The delivery requirement of the ruling. Every way of NOT knowing which + // datasource is primary must drain, because staying in rotation requires a + // POSITIVE reading — never the absence of a negative one. Each case here is a + // deployment where a real secondary IS down and the node drains anyway. + describe('fail toward DRAINING when the criterion cannot be resolved', () => { + const secondaryDown = [ + { driverName: 'pg', healthy: true }, + { driverName: 'tenant_mongo', healthy: false }, + ]; + + it('an engine that cannot name a primary datasource at all drains', async () => { + // `engine()` — no `resolvePrimaryDatasource`. An older engine, a lite + // kernel, a non-ObjectQL data service. + const res = await new HttpDispatcher(kernel('running', engine(secondaryDown))) + .dispatch('GET', '/ready', undefined, undefined, ctx); + + expect(res.response.status).toBe(503); + expect(res.response.body.error.details).toEqual({ + state: 'running', + drivers: ['tenant_mongo'], + }); + }); + + it('a criterion probe that THROWS drains', async () => { + const res = await new HttpDispatcher(kernel('running', { + checkDriversHealth: async () => secondaryDown, + resolvePrimaryDatasource: () => { throw new Error('registry exploded'); }, + })).dispatch('GET', '/ready', undefined, undefined, ctx); + + expect(res.response.status).toBe(503); + }); + + it.each([ + ['system-objects-split', { resolved: false, reason: 'system-objects-split' }], + ['no-system-objects-registered', { resolved: false, reason: 'no-system-objects-registered' }], + ['no-driver-registered', { resolved: false, reason: 'no-driver-registered' }], + ['system-object-unbound', { resolved: false, reason: 'system-object-unbound' }], + ])('an unresolved verdict (%s) drains', async (_reason, verdict) => { + const res = await new HttpDispatcher( + kernel('running', engineWithPrimary(secondaryDown, verdict)), + ).dispatch('GET', '/ready', undefined, undefined, ctx); + + expect(res.response.status).toBe(503); + }); + + it.each([ + ['undefined', undefined], + ['null', null], + ['a bare truthy object', { resolved: true }], + ['an empty datasource name', { resolved: true, datasource: '' }], + ['a non-string datasource', { resolved: true, datasource: 42 }], + ])('a malformed verdict (%s) drains rather than being coerced into a name', async (_d, verdict) => { + const res = await new HttpDispatcher( + kernel('running', engineWithPrimary(secondaryDown, verdict)), + ).dispatch('GET', '/ready', undefined, undefined, ctx); + + expect(res.response.status).toBe(503); + }); + + it('NON-VACUITY: the same fixture returns 200 the moment the criterion resolves', async () => { + // Without this control every assertion above would still pass if the + // handler had simply stopped serving 200 at all. + const res = await new HttpDispatcher( + kernel('running', engineWithPrimary(secondaryDown, primary('pg'))), + ).dispatch('GET', '/ready', undefined, undefined, ctx); + + expect(res.response.status).toBe(200); + }); + }); + + it('the criterion is memoized with the health reading, not re-resolved per poll', async () => { + const e = engineWithPrimary( + [{ driverName: 'pg', healthy: true }, { driverName: 'tenant_mongo', healthy: false }], + primary('pg'), + ); + const dispatcher = new HttpDispatcher(kernel('running', e)); + + await dispatcher.dispatch('GET', '/ready', undefined, undefined, ctx); + await dispatcher.dispatch('GET', '/ready', undefined, undefined, ctx); + await dispatcher.dispatch('GET', '/ready', undefined, undefined, ctx); + + expect(e.checkDriversHealth).toHaveBeenCalledTimes(1); + expect(e.resolvePrimaryDatasource).toHaveBeenCalledTimes(1); + }); +}); + describe('HttpDispatcher — GET /health liveness probe', () => { // framework#3756: liveness must NOT check the database. Its failure makes the // orchestrator restart the pod, which cannot fix an unreachable database but diff --git a/packages/runtime/src/http-dispatcher.ts b/packages/runtime/src/http-dispatcher.ts index 6f9fe59768..7e848e847d 100644 --- a/packages/runtime/src/http-dispatcher.ts +++ b/packages/runtime/src/http-dispatcher.ts @@ -7,6 +7,7 @@ import { isMcpServerEnabled, looksLikeInternalErrorLeak, INTERNAL_ERROR_MESSAGE, import { measureServerTiming, allowPerfDisclosure, isPerfDisclosurePrincipal } from '@objectstack/observability'; import { CoreServiceName, serviceUnavailableMessage, inProcessServiceMessage } from '@objectstack/spec/system'; import type { IDataEngine, IObjectQLEngine } from '@objectstack/spec/contracts'; +import type { PrimaryDatasourceVerdict } from '@objectstack/objectql'; import { readServiceSelfInfo, DispatcherErrorCode, resolveDiscoveryEnvironment } from '@objectstack/spec/api'; import { apiErrorResponse } from './error-envelope.js'; import { resolveRuntimeVersion } from './runtime-version.js'; @@ -237,6 +238,43 @@ const SEARCH_IN_PROCESS_MESSAGE = + 'no dedicated search endpoint is mounted on its behalf. Cross-object search, where a host ' + 'serves it, is reported separately by capabilities.search.'; +/** + * What `/ready` learned about the data plane on one probe (#13408). + * + * The two readings travel together because they are read together and expire + * together: `primary` is only ever consulted to decide what `unhealthy` MEANS, + * so a memo holding one without the other could answer a probe from a fresh + * health reading and a stale verdict about which datasource matters. + */ +interface DriverReadiness { + /** Registered drivers that cannot serve a query right now. */ + readonly unhealthy: string[]; + /** + * Which datasource is the primary one — `undefined` when the question was + * not asked because nothing was unhealthy (see {@link HttpDispatcher} — + * `driverReadiness`), and an unresolved verdict when it was asked and could + * not be answered. + */ + readonly primary?: PrimaryDatasourceReading; +} + +/** + * The readiness handler's view of the #13408 primary-datasource criterion. + * + * Collapsed from {@link PrimaryDatasourceVerdict} to exactly two shapes on + * purpose: the ruling gives every "cannot tell" the SAME consequence — + * + * > 判据解析失败或歧义时 ⇒ fail toward draining(宁可误摘不可静默保留) + * + * — so an engine that predates the probe, a probe that throws, and a + * deployment whose system objects are genuinely split must be indistinguishable + * at the point the status code is chosen. `reason` is carried for the operator, + * ⛔ never to branch on. + */ +type PrimaryDatasourceReading = + | { readonly kind: 'resolved'; readonly datasource: string } + | { readonly kind: 'unresolved'; readonly reason: string }; + /** * The HTTP dispatch engine — translates an inbound (method, path, body, ctx) * request into a kernel response. Used directly by the framework's HTTP adapters @@ -283,8 +321,13 @@ export class HttpDispatcher { * would be a database round-trip. One second is short enough that the * verdict tracks an outage within a single probe interval and long enough * that concurrent probes collapse onto one query. + * + * Holds the whole {@link DriverReadiness} reading, not just the unhealthy + * names (#13408): the primary-datasource verdict is what decides whether + * those names drain the replica, so caching one without the other would let + * a fresh health reading be judged by a stale verdict. */ - private driverHealthMemo?: { at: number; unhealthy: string[] }; + private driverHealthMemo?: DriverReadiness & { at: number }; private static readonly DRIVER_HEALTH_TTL_MS = 1_000; /** * When `true`, scoped data-plane routes enforce a @@ -576,13 +619,32 @@ export class HttpDispatcher { }), }); // GET /ready — k8s / load-balancer readiness probe (was branch "0b2"). - // 200 only when the kernel is fully running AND the data drivers can - // serve a query. 503 while booting (idle/initializing) or shutting down - // (stopping/stopped) so a load balancer stops routing to this replica - // BEFORE in-flight requests are drained and the server closes (graceful - // rolling restart) — and 503 when a driver is down, so a replica that - // would fail 100% of its requests leaves the rotation instead of - // absorbing traffic (framework#3756). + // 200 only when the kernel is fully running AND the data plane this + // replica depends on can serve a query. 503 while booting + // (idle/initializing) or shutting down (stopping/stopped) so a load + // balancer stops routing to this replica BEFORE in-flight requests are + // drained and the server closes (graceful rolling restart). + // + // WHICH driver failures drain the replica is the #13408 ruling + // (2026-08-31, 第 6 场总监席决裁批 #12, maintainer verbatim 「同意」): + // only the PRIMARY/default datasource. A secondary/tenant datasource's + // failure is reported in the 200 body and drains nothing. + // + // ⚠️ framework#3756 is NOT overturned by that, and its comment is + // rewritten here rather than deleted. Its reasoning was a QUANTIFIED + // claim — "a replica that would fail 100% of its requests" — and that + // antecedent still holds wherever it was measured: in a + // single-datasource deployment the primary IS the only source, so this + // handler still answers 503 for exactly the case #3756 fixed. What + // #3756's reasoning never covered is the multi-datasource shape, where + // a tenant's misconfigured datasource fails SOME requests and the old + // implementation drained every replica anyway — including replicas + // serving a perfectly healthy Postgres. #13408 carves out that branch; + // it does not revisit the single-datasource one. + // + // ⛔ The rejected fourth option stays rejected: a failed driver is + // never filtered OUT of this response to make it green. It is named in + // `degraded.drivers` on the 200 precisely so alerting still sees it. this.domainRegistry.register({ prefix: '/ready', match: 'exact', methods: ['GET'], handler: async () => { @@ -596,16 +658,41 @@ export class HttpDispatcher { if (state !== 'running') { return { handled: true, response: this.error('Service not ready', 503, { state }) }; } - const unhealthy = await this.unhealthyDrivers(); - return unhealthy.length === 0 - ? { handled: true, response: this.success({ status: 'ready', state }) } - : { + const { unhealthy, primary } = await this.driverReadiness(); + if (unhealthy.length === 0) { + return { handled: true, response: this.success({ status: 'ready', state }) }; + } + // #13408: drain only when the PRIMARY datasource is the one + // that is down — or when we could not establish which + // datasource that is. Every other path below falls through to + // the 503, which is the ruled fail-toward-draining direction + // written as control flow: staying in rotation needs a POSITIVE + // reading (a resolved primary, absent from the unhealthy set), + // never the absence of a negative one. + if (primary?.kind === 'resolved' && !unhealthy.includes(primary.datasource)) { + return { handled: true, - response: this.error('Data driver unavailable', 503, { + // The healthy 200 body is untouched — `degraded` appears + // only on this branch, so a probe consumer that never + // sees a degraded secondary sees exactly what it saw + // before #13408. + response: this.success({ + status: 'ready', state, - drivers: unhealthy, + degraded: { drivers: unhealthy, primaryDatasource: primary.datasource }, }), }; + } + // Byte-identical to the pre-#13408 envelope, deliberately: this + // is still framework#3756's answer, and a single-datasource + // deployment must not be able to tell that this handler changed. + return { + handled: true, + response: this.error('Data driver unavailable', 503, { + state, + drivers: unhealthy, + }), + }; }, }); this.domainRegistry.register(createAnalyticsDomain(this.domainDeps)); @@ -639,10 +726,91 @@ export class HttpDispatcher { this.domainRegistry.register(route); } + /** + * One readiness reading of the data plane, memoized for + * {@link DRIVER_HEALTH_TTL_MS} (framework#3756 — k8s polls every few + * seconds and this must not become a database round-trip per poll). + * + * ⭐ The primary-datasource criterion is resolved ONLY when something is + * already unhealthy, and that ordering is load-bearing rather than a + * micro-optimization. It is what makes #13408 a strict carve-out: every + * deployment whose drivers are all healthy takes the identical path it took + * before, so the new criterion cannot introduce a way to LOSE a 200. The + * only transition this change can produce is 503 → 200, on the one branch + * the ruling opened. + */ + private async driverReadiness(): Promise { + const memo = this.driverHealthMemo; + if (memo && Date.now() - memo.at < HttpDispatcher.DRIVER_HEALTH_TTL_MS) { + return memo; + } + const unhealthy = await this.unhealthyDrivers(); + const reading: DriverReadiness = unhealthy.length === 0 + ? { unhealthy } + : { unhealthy, primary: this.primaryDatasource() }; + this.driverHealthMemo = { at: Date.now(), ...reading }; + return reading; + } + + /** + * WHICH datasource is the primary one, as this replica can read it — + * the #13408 criterion, consulted through the engine because the ruling + * requires 「判据的判定逻辑单点实现」 and the five-step routing order that + * decides it lives in `ObjectQL.resolvePrimaryDatasource()`. ⛔ Never + * re-derive it here: a second implementation of "which datasource carries + * `sys_*`" is exactly the drift the single-point requirement forbids. + * + * ⭐ Fails toward DRAINING — the opposite direction from + * {@link unhealthyDrivers}, and the asymmetry is the ruling, not an + * oversight: + * + * > 判据解析失败或歧义时 ⇒ fail toward draining(宁可误摘不可静默保留), + * > 并有钉断言这个方向 + * + * `unhealthyDrivers` fails open because an inconclusive HEALTH probe must + * not black-hole a working deployment. This one fails closed because it is + * only ever reached when a driver IS positively unhealthy: the question on + * the table is no longer "is anything wrong" but "may we keep serving + * anyway", and an unreadable criterion is not permission. Every failure + * class — no engine, an engine predating the probe, a throwing probe, a + * malformed verdict, a genuinely split deployment — lands on `unresolved`, + * and the handler drains on all of them. + */ + private primaryDatasource(): PrimaryDatasourceReading { + try { + // Same slot and same [#4251] argument as `unhealthyDrivers`: the + // `data` ledger entry stays the narrow `IDataEngine` view, and the + // one wider member probed is named structurally. + let engine: (IDataEngine & { resolvePrimaryDatasource?: () => PrimaryDatasourceVerdict }) + | undefined; + try { + // [#5155] Host kernel — see the `/ready` handler. + engine = (this.defaultKernel as any)?.getService?.('data'); + } catch { + return { kind: 'unresolved', reason: 'data-service-unreadable' }; + } + if (typeof engine?.resolvePrimaryDatasource !== 'function') { + return { kind: 'unresolved', reason: 'engine-cannot-name-a-primary-datasource' }; + } + const verdict = engine.resolvePrimaryDatasource(); + // `resolved === true` is required positively: a verdict that is + // undefined, malformed, or carries an empty name must not be able + // to keep a replica in rotation. + if (verdict && verdict.resolved === true + && typeof verdict.datasource === 'string' && verdict.datasource.length > 0) { + return { kind: 'resolved', datasource: verdict.datasource }; + } + const reason = verdict && verdict.resolved === false ? verdict.reason : 'malformed-verdict'; + return { kind: 'unresolved', reason }; + } catch { + return { kind: 'unresolved', reason: 'primary-datasource-probe-threw' }; + } + } + /** * Names of the data drivers that cannot serve a query right now, for - * `/ready` (framework#3756). Empty means "no reason to leave the LB - * rotation" — which includes every case where we cannot tell. + * `/ready` (framework#3756). Empty means "no driver positively reports + * itself down" — which includes every case where we cannot tell. * * Fails OPEN by design, and the asymmetry is deliberate: readiness gates * whether this replica receives ANY traffic, so an inconclusive probe must @@ -651,12 +819,13 @@ export class HttpDispatcher { * `checkDriversHealth`, or a probe that itself throws all read as ready — * exactly as they did before this check existed. Only a driver that * positively reports itself unhealthy takes the replica out. + * + * ⚠️ This answers WHICH drivers are down, never whether that should drain + * the replica — {@link primaryDatasource} answers the second question and + * fails in the OPPOSITE direction. Reading this method's fail-open posture + * as the whole readiness posture is the #13408 mistake in miniature. */ private async unhealthyDrivers(): Promise { - const memo = this.driverHealthMemo; - if (memo && Date.now() - memo.at < HttpDispatcher.DRIVER_HEALTH_TTL_MS) { - return memo.unhealthy; - } let unhealthy: string[] = []; try { // [#4251] `checkDriversHealth` is ObjectQL's (IObjectQLEngine), not @@ -681,7 +850,6 @@ export class HttpDispatcher { // The probe itself failed — inconclusive, not unhealthy. See above. unhealthy = []; } - this.driverHealthMemo = { at: Date.now(), unhealthy }; return unhealthy; } From e8bb34a66f95aa59bb4eaf1becfb922cfd36f056 Mon Sep 17 00:00:00 2001 From: os-steve Date: Mon, 31 Aug 2026 08:23:02 +0000 Subject: [PATCH 2/5] test(objectql): pin system-object-unbound to the reachable zero-driver state (#13408) --- .../objectql/src/engine-primary-datasource.test.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/objectql/src/engine-primary-datasource.test.ts b/packages/objectql/src/engine-primary-datasource.test.ts index 2138f6353f..e240e6e05e 100644 --- a/packages/objectql/src/engine-primary-datasource.test.ts +++ b/packages/objectql/src/engine-primary-datasource.test.ts @@ -200,11 +200,18 @@ 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. const engine = newEngine(); - // No default driver: nothing to fall through to at step 5. - engine.registerDriver(driver('pg')); registerSys(engine, 'sys_user'); + expect(engine.getDefaultDriverName()).toBeUndefined(); expect(engine.resolvePrimaryDatasource()).toEqual({ resolved: false, reason: 'system-object-unbound', From ae226f61922cd1a4952f1b97f19384b4d969f42c Mon Sep 17 00:00:00 2001 From: os-steve Date: Mon, 31 Aug 2026 08:26:25 +0000 Subject: [PATCH 3/5] chore: changeset for #13408 --- .changeset/ready-primary-datasource-drain.md | 65 ++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 .changeset/ready-primary-datasource-drain.md diff --git a/.changeset/ready-primary-datasource-drain.md b/.changeset/ready-primary-datasource-drain.md new file mode 100644 index 0000000000..1f94e8b6a8 --- /dev/null +++ b/.changeset/ready-primary-datasource-drain.md @@ -0,0 +1,65 @@ +--- +"@objectstack/objectql": minor +"@objectstack/runtime": minor +--- + +fix(runtime,objectql): `/api/v1/ready` drains only on the PRIMARY datasource's failure; a secondary/tenant datasource is reported, not drained (#13408) + +On a multi-datasource deployment, one datasource whose driver could not start +pinned `/api/v1/ready` to 503 on **every replica** — so a readiness-checked load +balancer drained every upstream and took the whole deployment offline, while +Postgres and the app itself were healthy (`/api/v1/health` 200, direct reads +working). One tenant's misconfiguration = total outage. Observed on a live +3-replica EE deployment and recovered only by restarting every process. + +Ruled 2026-08-31 (第 6 场总监席决裁批 #12, maintainer verbatim 「同意」), Option B: + +> `/api/v1/ready` 只在**主/默认数据源**不健康时摘流量;次要/租户数据源的故障照常 +> **上报**(`/ready` 响应 body、日志、告警)但不 drain 节点。 + +**What changed.** When a driver reports itself unhealthy, `/ready` now asks +which datasource is the deployment's primary one before choosing a status: + +- primary unhealthy, or the criterion unresolvable ⇒ **503**, unchanged envelope; +- primary healthy, only a secondary down ⇒ **200**, with the failed drivers named + in a new `degraded` block: `{ status, state, degraded: { drivers, primaryDatasource } }`. + +The failed driver is never hidden — the rejected alternative of filtering it out +of the response stays rejected. `degraded` appears **only** on that branch; an +all-healthy 200 body is byte-identical to before. + +**The criterion is a readable fact, not a heuristic.** `ObjectQL.resolvePrimaryDatasource()` +(new, exported with `PrimaryDatasourceVerdict` / `PrimaryDatasourceUnresolvedReason`) +answers *where this deployment's platform system objects actually live*, resolved +through the same five-step routing order every query uses — never registration +order, never the driver flagged default at registration. The ADR-0057 §3.6 system +ledgers (`audit` / `telemetry` / `event`) are excluded because they are +deliberately routed off the primary. Disagreement, silence, or a name with no +driver behind it return an **unresolved verdict**, never a guess. + +**Fail toward draining.** Every way of not knowing — an engine that predates the +probe, a probe that throws, a malformed verdict, a genuinely split deployment — +falls back to the old whole-node 503. Staying in rotation requires a *positive* +reading; the absence of a negative one is not permission. Pinned in both +directions, including an inversion ablation. + +**framework#3756 is not overturned.** Its quantified reason — "a replica that +would fail 100% of its requests" — still holds in the single-datasource +deployment it was measured on, where the primary *is* the only source; that +deployment's answer is unchanged down to the response body. What #3756's +reasoning never covered is the multi-datasource shape, and that is the only +branch this carves out. + +**Grade: `minor` for both, proposed rather than assumed** — this changes when a +published operational probe drains a node, so an operator whose alerting keys on +`/ready` returning 503 for *any* driver failure will see 200 + `degraded` +instead, and `degraded` is a new response field. Nothing is removed or renamed, +no declared contract key is added (the ruling explicitly declines that: 「⛔ 本裁不 +加契约键」), single-datasource behaviour is bit-identical, and no migration is +required — so `major` overstates it and `patch` understates a deliberate change +to an availability control surface. + +⚠️ Out of scope, tracked separately as **#13578**: `DELETE` of a datasource still +does not evict the stuck driver from the in-memory engine registry, so the +datasource keeps appearing in `/ready`'s report until the process restarts. That +half is a defect under either answer to this card and is queued independently. From 6a07781896ff94b84f36b4c173d4e4b41f304a8d Mon Sep 17 00:00:00 2001 From: os-steve Date: Mon, 31 Aug 2026 09:22:17 +0000 Subject: [PATCH 4/5] docs(permissions): re-anchor the system-context census after the engine.ts line shift (#13408) --- content/docs/permissions/system-context.mdx | 24 ++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 5a4eb85ac7..4948b23930 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:10616` | -| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:10778` | -| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9509` | +| 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` | | 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:9546`, `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:5638` | -| 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:3573`, `:3583`, `:3610` | +| 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` | +| 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:6336` | -| 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:11364` | -| 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:11293` | +| 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` | ### 3. Sharing (`plugin-sharing`) @@ -179,8 +179,8 @@ a reader tracing where elevation travels needs them. | # | Site | Package | What it does | |:--|:---|:---|:---| -| 62 | `objectql/src/engine.ts:3405` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | -| 63 | `objectql/src/engine.ts:13705` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | +| 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 | | 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:9492`–`9509` | +| "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 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 2aa73e83e4f01d0a1a196e27f30cf4efd09d98ed Mon Sep 17 00:00:00 2001 From: os-steve Date: Mon, 31 Aug 2026 11:03:06 +0000 Subject: [PATCH 5/5] fix(test): type the driver double and narrow the dispatch result so the hidden test layer type-checks (#13408) --- .../src/engine-primary-datasource.test.ts | 21 ++++- .../runtime/src/http-dispatcher.ready.test.ts | 90 ++++++++++++------- scripts/check-type-check-coverage.mjs | 25 +++--- 3 files changed, 91 insertions(+), 45 deletions(-) diff --git a/packages/objectql/src/engine-primary-datasource.test.ts b/packages/objectql/src/engine-primary-datasource.test.ts index e240e6e05e..8ba275ce7b 100644 --- a/packages/objectql/src/engine-primary-datasource.test.ts +++ b/packages/objectql/src/engine-primary-datasource.test.ts @@ -16,9 +16,20 @@ // the refusal happens and is not silently a name. import { describe, it, expect } from 'vitest'; +import type { IDataDriver } from '@objectstack/spec/contracts'; import { ObjectQL } from './engine.js'; -const driver = (name: string) => ({ +/** + * A registrable driver double. + * + * Annotated `IDataDriver` deliberately, rather than left as an inferred object + * literal: `registerDriver` takes the real contract, so an un-annotated double + * is checked only at the call site and silently drifts as the interface grows. + * The annotation makes THIS declaration the thing that fails when a member is + * added — which is how the first version of this fixture was found short of + * `upsert` and `dropTable`. + */ +const driver = (name: string): IDataDriver => ({ name, version: '1.0.0', supports: {}, @@ -27,18 +38,20 @@ const driver = (name: string) => ({ checkHealth: async () => true, find: async () => [], findOne: async () => null, - create: async (_o: string, data: any) => ({ id: '1', ...data }), - update: async (_o: string, id: string, data: any) => ({ id, ...data }), + 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 () => ({}), + execute: async () => null, beginTransaction: async () => ({}), commit: async () => {}, rollback: async () => {}, syncSchema: async () => {}, + dropTable: async () => {}, }); function newEngine(): ObjectQL { diff --git a/packages/runtime/src/http-dispatcher.ready.test.ts b/packages/runtime/src/http-dispatcher.ready.test.ts index c88cc02a13..7e911e15e0 100644 --- a/packages/runtime/src/http-dispatcher.ready.test.ts +++ b/packages/runtime/src/http-dispatcher.ready.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from 'vitest'; -import { HttpDispatcher } from './http-dispatcher.js'; +import { HttpDispatcher, type HttpDispatcherResult } from './http-dispatcher.js'; function kernel(state: string, dataService?: unknown): any { return { @@ -10,6 +10,34 @@ function kernel(state: string, dataService?: unknown): any { } const ctx: any = {}; +/** + * `HttpDispatcherResult.response` is OPTIONAL, so every read of it is a + * `possibly undefined` in a type-checked program — and this package's test + * layer IS type-checked, by `check:type-check-debt --re-measure` against a + * shrink-only ledger, even though `pnpm test` never sees it. That asymmetry is + * exactly how 19 fresh errors got in here behind a green `pnpm --filter + * @objectstack/runtime typecheck`: the package's own tsconfig excludes every + * `.test.ts` file, so the program that reported zero had never read this one. + * + * Narrow once, and narrow LOUDLY — the shape is lifted from the #8287 suite in + * `http-dispatcher.keys.test.ts`, deliberately rather than invented again. + * `expect(res.response).toBeDefined()` would satisfy a reader and narrow + * nothing (vitest's matchers are not assertion signatures), and a `!` would + * silence the compiler while leaving the failure to surface as `undefined is + * not an object` three lines later. A probe that answered no response at all is + * a different defect from one that answered the wrong status; this keeps them + * distinguishable. + * + * Applied to the WHOLE file, not just the #13408 suite: the reads are identical + * in kind, the repair is one call each, and leaving the older ones would bank a + * green while knowingly holding fixable errors in a file already open. + */ +function responseOf(res: HttpDispatcherResult): NonNullable { + const { response } = res; + if (!response) throw new Error('GET /ready answered no response at all'); + return response; +} + /** An engine whose `checkDriversHealth` reports the given verdicts. */ function engine(results: Array<{ driverName: string; healthy: boolean }>) { return { checkDriversHealth: vi.fn(async () => results) }; @@ -38,14 +66,14 @@ describe('HttpDispatcher — GET /ready readiness probe', () => { it('returns 200 when the kernel is running', async () => { const res = await new HttpDispatcher(kernel('running')).dispatch('GET', '/ready', undefined, undefined, ctx); expect(res.handled).toBe(true); - expect(res.response.status).toBe(200); - expect(res.response.body.data.state).toBe('running'); + expect(responseOf(res).status).toBe(200); + expect(responseOf(res).body.data.state).toBe('running'); }); it('returns 503 while booting or shutting down', async () => { for (const state of ['idle', 'initializing', 'stopping', 'stopped']) { const res = await new HttpDispatcher(kernel(state)).dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(503); + expect(responseOf(res).status).toBe(503); } }); @@ -57,7 +85,7 @@ describe('HttpDispatcher — GET /ready readiness probe', () => { kernel('running', engine([{ driverName: 'sql', healthy: true }])), ).dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(200); + expect(responseOf(res).status).toBe(200); }); it('returns 503 naming the driver when one is down, even though the kernel runs', async () => { @@ -68,9 +96,9 @@ describe('HttpDispatcher — GET /ready readiness probe', () => { ])), ).dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(503); - expect(res.response.body.error.message).toBe('Data driver unavailable'); - expect(res.response.body.error.details).toEqual({ state: 'running', drivers: ['sql'] }); + expect(responseOf(res).status).toBe(503); + expect(responseOf(res).body.error.message).toBe('Data driver unavailable'); + expect(responseOf(res).body.error.details).toEqual({ state: 'running', drivers: ['sql'] }); }); it('does not re-probe within the memo TTL — k8s polls every few seconds', async () => { @@ -91,14 +119,14 @@ describe('HttpDispatcher — GET /ready readiness probe', () => { }), ).dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(200); + expect(responseOf(res).status).toBe(200); }); it('stays ready on an engine predating checkDriversHealth', async () => { const res = await new HttpDispatcher(kernel('running', { find: async () => [] })) .dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(200); + expect(responseOf(res).status).toBe(200); }); }); }); @@ -122,11 +150,11 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', () kernel('running', engineWithPrimary([{ driverName: 'pg', healthy: false }], primary('pg'))), ).dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(503); - expect(res.response.body.error.message).toBe('Data driver unavailable'); + expect(responseOf(res).status).toBe(503); + expect(responseOf(res).body.error.message).toBe('Data driver unavailable'); // Byte-identical to the shape #3756 shipped — an operator's alerting on // this body must not be able to tell that the handler changed. - expect(res.response.body.error.details).toEqual({ state: 'running', drivers: ['pg'] }); + expect(responseOf(res).body.error.details).toEqual({ state: 'running', drivers: ['pg'] }); }); it('the all-healthy 200 body carries NO degraded key', async () => { @@ -134,9 +162,9 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', () kernel('running', engineWithPrimary([{ driverName: 'pg', healthy: true }], primary('pg'))), ).dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(200); - expect(res.response.body.data).toEqual({ status: 'ready', state: 'running' }); - expect(res.response.body.data).not.toHaveProperty('degraded'); + expect(responseOf(res).status).toBe(200); + expect(responseOf(res).body.data).toEqual({ status: 'ready', state: 'running' }); + expect(responseOf(res).body.data).not.toHaveProperty('degraded'); }); it('does not even ASK which datasource is primary while everything is healthy', () => { @@ -164,11 +192,11 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', () )), ).dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(200); - expect(res.response.body.data.status).toBe('ready'); + expect(responseOf(res).status).toBe(200); + expect(responseOf(res).body.data.status).toBe('ready'); // ⛔ The rejected fourth option — filtering the bad driver out so it // becomes invisible — would show an EMPTY degraded list here. - expect(res.response.body.data.degraded).toEqual({ + expect(responseOf(res).body.data.degraded).toEqual({ drivers: ['tenant_mongo'], primaryDatasource: 'pg', }); @@ -182,8 +210,8 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', () )), ).dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(503); - expect(res.response.body.error.details.drivers).toEqual(['pg']); + expect(responseOf(res).status).toBe(503); + expect(responseOf(res).body.error.details.drivers).toEqual(['pg']); }); it('drains when BOTH are down — the primary is in the unhealthy set', async () => { @@ -194,8 +222,8 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', () )), ).dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(503); - expect(res.response.body.error.details.drivers).toEqual(['pg', 'tenant_mongo']); + expect(responseOf(res).status).toBe(503); + expect(responseOf(res).body.error.details.drivers).toEqual(['pg', 'tenant_mongo']); }); }); @@ -215,8 +243,8 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', () const res = await new HttpDispatcher(kernel('running', engine(secondaryDown))) .dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(503); - expect(res.response.body.error.details).toEqual({ + expect(responseOf(res).status).toBe(503); + expect(responseOf(res).body.error.details).toEqual({ state: 'running', drivers: ['tenant_mongo'], }); @@ -228,7 +256,7 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', () resolvePrimaryDatasource: () => { throw new Error('registry exploded'); }, })).dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(503); + expect(responseOf(res).status).toBe(503); }); it.each([ @@ -241,7 +269,7 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', () kernel('running', engineWithPrimary(secondaryDown, verdict)), ).dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(503); + expect(responseOf(res).status).toBe(503); }); it.each([ @@ -255,7 +283,7 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', () kernel('running', engineWithPrimary(secondaryDown, verdict)), ).dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(503); + expect(responseOf(res).status).toBe(503); }); it('NON-VACUITY: the same fixture returns 200 the moment the criterion resolves', async () => { @@ -265,7 +293,7 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', () kernel('running', engineWithPrimary(secondaryDown, primary('pg'))), ).dispatch('GET', '/ready', undefined, undefined, ctx); - expect(res.response.status).toBe(200); + expect(responseOf(res).status).toBe(200); }); }); @@ -294,8 +322,8 @@ describe('HttpDispatcher — GET /health liveness probe', () => { const res = await new HttpDispatcher(kernel('running', e)) .dispatch('GET', '/health', undefined, undefined, ctx); - expect(res.response.status).toBe(200); - expect(res.response.body.data.status).toBe('ok'); + expect(responseOf(res).status).toBe(200); + expect(responseOf(res).body.data.status).toBe('ok'); expect(e.checkDriversHealth).not.toHaveBeenCalled(); }); }); diff --git a/scripts/check-type-check-coverage.mjs b/scripts/check-type-check-coverage.mjs index 6443bb68a9..eba8bac2f1 100644 --- a/scripts/check-type-check-coverage.mjs +++ b/scripts/check-type-check-coverage.mjs @@ -901,18 +901,23 @@ const TEST_DEBT = { + 'ead731756, which is what the lowering at the top of this note closed.', }, '@objectstack/runtime': { - errors: 217, - note: 'TS18048 x98 (possibly-undefined), TS2345 x26, TS18046 x20, TS2339 x16, TS2493 x15, TS2835 x11, ' - + 'TS2554 x11. LOWERED 227 -> 217 at ead731756 (#12723) -- the entry that card was filed on, where ' - + 'the slack turned out to be 10 rather than the 1 it reported: the 226 it quoted was itself a ' - + 'reading taken days earlier, which is the card\'s own point about a number that drifts. The tally ' - + 'above was taken at 227 and is NOT re-tallied here; that sweep measured per-entry TOTALS only. Src ' - + 'graduated in #4311 (declares `typecheck`); this is purely the hidden test layer. Measured 220 -> ' + errors: 206, + note: 'TS18048 x91 (possibly-undefined), TS18046 x27, TS2339 x17, TS2493 x15, TS2835 x13, TS2345 x10, ' + + 'TS7006 x8, TS6133 x6, TS2554 x4, TS2353 x4, TS2571 x3, TS2550 x2 -- RE-TALLIED at 206 (#13408). ' + + 'The previous note carried its composition from a 227-era sweep that measured per-entry TOTALS ' + + 'only and said so; this one is a fresh per-code count of the same program the ratchet measures. ' + + 'LOWERED 217 -> 206 (#13408), and the -11 is fully attributed to ONE file: ' + + 'src/http-dispatcher.ready.test.ts held 30 TS18048 reads of the optional ' + + '`HttpDispatcherResult.response` -- 19 added by that card\'s own new /ready suite and 11 that ' + + 'pre-dated it -- and all 30 were replaced by a `responseOf()` narrowing helper, the shape already ' + + 'used by the #8287 suite in src/http-dispatcher.keys.test.ts. That card found them the hard way: ' + + 'the package `typecheck` excludes test files, so its green said nothing about the 19 it had just ' + + 'added, and only this ratchet saw them. Nothing else in the package moved. Earlier lineage: 220 -> ' + '218 (5ab08428, one of only two entries that ever shrank; TS6133 x25 collapsed to x7 while ' + 'possibly-undefined grew, so that net -2 hid a much larger churn in both directions) -> 227 ' - + '(e8db1a230). The latest +9 is fully attributed and is ONE file: every one of the nine is a ' - + 'TS18048 in src/domains/meta-item-envelope.test.ts, added by #5563 / PR #5895 (the ' - + '`GET /meta/:type/:name` envelope convergence). Nothing else in the package moved.', + + '(e8db1a230, +9 all TS18048 in src/domains/meta-item-envelope.test.ts from #5563 / PR #5895) -> ' + + '217 (ead731756, #12723). Src graduated in #4311 (declares `typecheck`); this is purely the ' + + 'hidden test layer.', }, '@objectstack/cli': { errors: 144,