diff --git a/.changeset/28143-install-local-datasource-holders.md b/.changeset/28143-install-local-datasource-holders.md new file mode 100644 index 00000000000..c57ff77194e --- /dev/null +++ b/.changeset/28143-install-local-datasource-holders.md @@ -0,0 +1,20 @@ +--- +'@objectstack/runtime': patch +'@objectstack/cloud-connection': patch +--- + +fix(cloud-connection): an install-local install judges who holds a datasource name with one judge that reads the engine too and fails closed, and a restart's rehydrate no longer takes over a name another holder owns + +Clause-②: yes (widening) +The widening: `@objectstack/runtime` now exports `datasourceNameHolder` and the type `DatasourceNameHolding`, and `POST /api/v1/marketplace/install-local` gains a `503 SERVICE_UNAVAILABLE` answer. +Metadata-protocol: none + +Three edges `POST /api/v1/marketplace/install-local`'s datasource-name check left open, each measured on `main`: + +- **A holder that cannot be read.** When the metadata registry's read of a declared datasource name threw, the check skipped it and the install went ahead (`200`). It now answers `503 SERVICE_UNAVAILABLE` with `details: { datasource, reading }` and writes nothing: a check that did not run is never taken for "no holder". Retry once that store answers. +- **A datasource registered on the data engine only** (the legacy `onEnable` bridge, which leaves no metadata item) was not seen: the install answered `200` and reported that pool as the package's `already-registered`. It now holds its name, `409 RESOURCE_CONFLICT` like every other holder. +- **A package installed before the check existed.** A restart's rehydrate replayed such a package's datasources over a datasource created in Setup under the same name: the Setup item was overwritten by the package's code registration, the name joined the code-defined set (the Setup row turned read-only), and the package's uninstall closed and evicted the Setup row's pool. The rehydrate now leaves a held name alone: it is not registered, recorded or connected for the package, one `error` line names it, its holder and the remedy, and the uninstall does not touch it. An uninstall also closes only the pools the package's own connect opened, never one it found already registered. + +`@objectstack/runtime` exports `datasourceNameHolder`, the one judge both the install door and the rehydrate use: the host's `default`, a metadata item not stamped with the package (a Setup row, another package's), a name in the code-datasource set the package did not record, then a driver on the data engine. + +**The remedy** for a package the rehydrate now reports: rename the package's datasource and install it again, or remove the datasource holding the name and restart. diff --git a/packages/cloud-connection/src/marketplace-install-local-datasource-holder.test.ts b/packages/cloud-connection/src/marketplace-install-local-datasource-holder.test.ts new file mode 100644 index 00000000000..f8282cab3de --- /dev/null +++ b/packages/cloud-connection/src/marketplace-install-local-datasource-holder.test.ts @@ -0,0 +1,139 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#28143] Step 2a of `POST /api/v1/marketplace/install-local` — a package + * declaring a datasource name another holder owns is refused before any write + * — judges with the runtime's ONE judge (`datasourceNameHolder`), and so: + * + * - a holder the metadata registry cannot be read for is `503 + * SERVICE_UNAVAILABLE`, never taken for "no holder": the install does not + * proceed past an outage; + * - a datasource registered on the data engine only (the legacy `onEnable` + * bridge, with no metadata item) holds its name: `409 RESOURCE_CONFLICT`, + * where the door used to install and answer that pool `already-registered`. + * + * Each refusal leaves the runtime as it found it: no ledger entry, nothing + * registered. The end-to-end refusals (the boot's datasource, the host's + * `default`) are pinned by `install-local-declared-datasources.dogfood.test.ts`. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, rmSync, readdirSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +// The install and its 2a judge reach `@objectstack/runtime` through a lazy +// `import()`; its first load is paid here, at module top — never inside a +// clocked `it` (`scripts/check-test-source-alias.mjs`). +import '@objectstack/runtime'; +import { MarketplaceInstallLocalPlugin } from './marketplace-install-local-plugin.js'; +import { installerAuthService, withInstallerGrants } from './install-local-principal.fixtures.js'; + +type Handler = (c: any) => Promise; + +const ROUTE = 'POST /api/v1/marketplace/install-local'; +const DS = 'qah_store'; + +function makeRawApp() { + const routes = new Map(); + return { + routes, + get: (p: string, h: Handler) => routes.set(`GET ${p}`, h), + post: (p: string, h: Handler) => routes.set(`POST ${p}`, h), + delete: (p: string, h: Handler) => routes.set(`DELETE ${p}`, h), + }; +} + +/** A kernel whose metadata registry and data engine the case shapes. */ +function makeCtx(rawApp: any, opts: { metadataFault?: boolean; engineDrivers?: string[] }) { + const hooks = new Map(); + const registered: any[] = []; + const drivers = new Set(opts.engineDrivers ?? []); + const services: Record = { + manifest: { register: (m: any) => registered.push({ manifest: m }) }, + auth: installerAuthService(), + objectql: withInstallerGrants({ + syncSchemas: async () => undefined, + registry: { getAllPackages: () => registered }, + getDriverByName: (name: string) => (drivers.has(name) ? { name } : undefined), + }), + metadata: { + get: async () => { + if (opts.metadataFault) throw new Error('metadata loader unavailable'); + return undefined; + }, + }, + }; + return { + registered, + ctx: { + hook: (e: string, h: any) => hooks.set(e, h), + getService: (name: string) => { + if (name === 'http-server') return { getRawApp: () => rawApp }; + const svc = services[name]; + if (svc === undefined) throw new Error(`no ${name}`); + return svc; + }, + registerService: (name: string, svc: unknown) => { services[name] = svc; }, + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + }, + fire: async () => { await hooks.get('kernel:ready')?.(); }, + }; +} + +function makeC(body: any) { + const json = vi.fn((payload: any, status?: number) => ({ payload, status: status ?? 200 })); + return { req: { url: 'http://localhost:3000/x', raw: new Request('http://localhost:3000/x'), json: async () => body, param: () => undefined }, json }; +} + +const BODY = { + manifest: { + id: 'com.example.qah', + version: '1.0.0', + datasources: [{ name: DS, label: 'QA store', driver: 'sqlite', config: { filename: ':memory:' } }], + objects: [], + }, +}; + +let dir: string; +beforeEach(() => { dir = mkdtempSync(join(tmpdir(), 'mil-ds-holder-')); }); +afterEach(() => { rmSync(dir, { recursive: true, force: true }); vi.restoreAllMocks(); }); + +async function install(opts: { metadataFault?: boolean; engineDrivers?: string[] }) { + const rawApp = makeRawApp(); + const k = makeCtx(rawApp, opts); + const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir }); + await plugin.start(k.ctx as any); + await k.fire(); + const res = await rawApp.routes.get(ROUTE)!(makeC(BODY)); + // The plugin registers its own Installed Apps UI manifest at start; only + // the package's registration is this case's. + const registered = k.registered.map((r) => r.manifest.id).filter((id) => id === 'com.example.qah'); + return { res, registered, ledger: readdirSync(dir) }; +} + +describe('install-local 2a — who holds a declared datasource name', () => { + it('a holder that cannot be read refuses 503, and nothing is written', async () => { + const { res, registered, ledger } = await install({ metadataFault: true }); + expect(res.status).toBe(503); + expect(res.payload?.error?.code).toBe('SERVICE_UNAVAILABLE'); + expect(res.payload?.error?.details).toEqual({ datasource: DS, reading: 'the metadata registry' }); + expect(registered).toEqual([]); + expect(ledger).toEqual([]); + }); + + it('a datasource registered on the engine only holds its name: 409, and nothing is written', async () => { + const { res, registered, ledger } = await install({ engineDrivers: [DS] }); + expect(res.status).toBe(409); + expect(res.payload?.error?.code).toBe('RESOURCE_CONFLICT'); + expect(res.payload?.error?.details).toEqual({ datasource: DS }); + expect(registered).toEqual([]); + expect(ledger).toEqual([]); + }); + + it('CONTROL: a name nobody holds installs', async () => { + const { res, registered } = await install({ engineDrivers: ['someone_else'] }); + expect(res.status, JSON.stringify(res.payload)).toBe(200); + expect(res.payload?.success).toBe(true); + expect(registered).toEqual(['com.example.qah']); + }); +}); diff --git a/packages/cloud-connection/src/marketplace-install-local-plugin.ts b/packages/cloud-connection/src/marketplace-install-local-plugin.ts index 3dbce3c9108..86f1411152e 100644 --- a/packages/cloud-connection/src/marketplace-install-local-plugin.ts +++ b/packages/cloud-connection/src/marketplace-install-local-plugin.ts @@ -1812,7 +1812,28 @@ export class MarketplaceInstallLocalPlugin implements Plugin { // hand the package's objects that holder's pool (the connect is // idempotent on the name), and recording the name as code would // make the holder's own datasource read-only to the admin door. + // [#28143] Judged by the runtime's ONE judge, which reads the data + // engine as well (a driver the legacy `onEnable` bridge registered + // has no metadata item), and a holder that cannot be read refuses + // `503`: a check that did not run is never taken for "no holder". const heldDatasource = await this.findHeldDatasourceName(ctx, manifest, manifestId); + if (heldDatasource && 'reading' in heldDatasource) { + ctx.logger?.warn?.( + `[MarketplaceInstallLocal] install of ${manifestId} refused: whether another holder owns its datasource ` + + `'${heldDatasource.name}' could not be judged — ${heldDatasource.reading} could not be read (${heldDatasource.error})`, + ); + return c.json({ + success: false, + error: { + code: 'SERVICE_UNAVAILABLE', + message: `Package '${manifestId}' cannot be installed now: it declares the datasource '${heldDatasource.name}', ` + + `and whether another holder already owns that name could not be checked (${heldDatasource.reading} ` + + 'could not be read). A check that did not run is never taken for "no holder". Nothing was installed ' + + 'or written. Retry once that store answers.', + details: { datasource: heldDatasource.name, reading: heldDatasource.reading }, + }, + }, 503); + } if (heldDatasource) { return c.json({ success: false, @@ -3601,37 +3622,54 @@ export class MarketplaceInstallLocalPlugin implements Plugin { /** * [#28039] The first datasource `manifest` declares whose name another - * holder already owns, with that holder: the host's `default`, or a - * datasource in the metadata service not stamped with this package (one - * created in Setup, the boot artifact's, another package's). A datasource - * this package's earlier install registered is its own, so a reinstall - * passes. `undefined` when nothing collides or nothing can be read. + * holder already owns, with that holder — or [#28143] whose holder could + * not be read, with what could not be. `undefined` when every name is free. + * + * The judge is the runtime's `datasourceNameHolder`, the one the + * rehydrate's `connectPackageDatasources` leaves a held name alone on: the + * host's `default`, a datasource item in the metadata registry not stamped + * with this package (one created in Setup, another package's), a name in + * the host's code-datasource set this package did not record, and a driver + * registered on the data engine with no metadata item. This package's own + * earlier registration is its own, so a reinstall passes. + * + * ⛔ Never `undefined` on a read fault: a holder that cannot be read is the + * `reading` answer, which the door refuses `503`. A runtime without the + * judge is that answer too. */ private findHeldDatasourceName = async ( ctx: PluginContext, manifest: unknown, manifestId: string, - ): Promise<{ name: string; holder: string } | undefined> => { + ): Promise<{ name: string; holder: string } | { name: string; reading: string; error: string } | undefined> => { const names = declaredDatasources(manifest) .map((ds) => ds?.name) .filter((name): name is string => typeof name === 'string' && name.length > 0); if (names.length === 0) return undefined; - if (names.includes('default')) return { name: 'default', holder: "the host's primary datasource" }; - let metadata: { get?: (type: string, name: string) => Promise } | undefined; - try { metadata = ctx.getService('metadata'); } catch { return undefined; } - for (const name of names) { - let item: unknown; - try { item = await metadata?.get?.('datasource', name); } catch { continue; } - if (!isPlainObject(item)) continue; - const owner = item._packageId; - if (owner === manifestId) continue; + let judge: typeof import('@objectstack/runtime')['datasourceNameHolder'] | undefined; + let loadError: unknown; + try { + const mod: any = await import('@objectstack/runtime'); + if (typeof mod?.datasourceNameHolder === 'function') judge = mod.datasourceNameHolder; + } catch (err) { + loadError = err; + } + if (!judge) { return { - name, - holder: typeof owner === 'string' && owner.length > 0 - ? `the package '${owner}'` - : item.origin === 'runtime' ? 'a datasource created in Setup' : 'a datasource', + name: names[0], + reading: "the runtime's datasource-name judge", + error: loadError !== undefined + ? String((loadError as Error)?.message ?? loadError) + : '@objectstack/runtime has no datasourceNameHolder — upgrade it alongside @objectstack/cloud-connection', }; } + for (const name of names) { + const holding = await judge(ctx, manifestId, name); + if (holding.status === 'held') return { name, holder: holding.holder }; + if (holding.status === 'unreadable') { + return { name, reading: holding.reading, error: String((holding.cause as Error)?.message ?? holding.cause) }; + } + } return undefined; }; diff --git a/packages/runtime/src/declared-datasources.test.ts b/packages/runtime/src/declared-datasources.test.ts index c4cc5283638..336e2154deb 100644 --- a/packages/runtime/src/declared-datasources.test.ts +++ b/packages/runtime/src/declared-datasources.test.ts @@ -16,23 +16,36 @@ // file `install-local-declared-datasources.dogfood.test.ts`. import { describe, it, expect, vi } from 'vitest'; -import { connectPackageDatasources } from './declared-datasources.js'; -import { CODE_DATASOURCE_NAMES_SERVICE, CodeDatasourceNames } from './code-datasource-names.js'; +import { connectPackageDatasources, datasourceNameHolder } from './declared-datasources.js'; +import { CODE_DATASOURCE_NAMES_SERVICE, CodeDatasourceNames, contributeCodeDatasourceNames } from './code-datasource-names.js'; type Verdict = { name: string; status: string; reason?: string }; /** A kernel's service registry with a metadata service, a connection service and a protocol. */ -function kernel(opts: { failFast?: string; withConnection?: boolean } = {}) { +function kernel(opts: { + failFast?: string; + withConnection?: boolean; + /** A data engine whose driver registry the connect fills, as the real one does. */ + withEngine?: boolean; + /** A `metadata.get` of this datasource name throws — a loader outage. */ + metadataFault?: string; + /** Force the connect's verdict for a name. */ + answer?: Record; +} = {}) { const services = new Map(); const items = new Map(); const states = new Map(); const cleanups = new Map Promise<{ success: boolean; removed: number; error?: string }>>(); const connectCalls: Array<{ datasources: any[]; objects?: unknown }> = []; const disconnected: string[] = []; + const drivers = new Set(); services.set('metadata', { registerInMemory: (type: string, name: string, data: unknown) => items.set(`${type}/${name}`, data), - get: async (type: string, name: string) => items.get(`${type}/${name}`), + get: async (type: string, name: string) => { + if (name === opts.metadataFault) throw new Error('metadata loader unavailable'); + return items.get(`${type}/${name}`); + }, unregister: async (type: string, name: string) => { items.delete(`${type}/${name}`); }, }); if (opts.withConnection !== false) { @@ -43,7 +56,12 @@ function kernel(opts: { failFast?: string; withConnection?: boolean } = {}) { for (const ds of input.datasources) { const v: Verdict = ds.name === opts.failFast ? { name: ds.name, status: 'failed-credentials', reason: `credential '${ds.external?.credentialsRef}' could not be resolved` } - : { name: ds.name, status: 'connected' }; + : opts.answer?.[ds.name] !== undefined + ? { name: ds.name, status: opts.answer[ds.name] } + : drivers.has(ds.name) + ? { name: ds.name, status: 'already-registered' } + : { name: ds.name, status: 'connected' }; + if (v.status === 'connected') drivers.add(ds.name); states.set(ds.name, v); results.push(v); } @@ -51,9 +69,12 @@ function kernel(opts: { failFast?: string; withConnection?: boolean } = {}) { return results; }, getConnectionState: (name: string) => states.get(name), - disconnect: async (name: string) => { disconnected.push(name); states.delete(name); }, + disconnect: async (name: string) => { disconnected.push(name); states.delete(name); drivers.delete(name); }, }); } + if (opts.withEngine) { + services.set('objectql', { getDriverByName: (name: string) => (drivers.has(name) ? { name } : undefined) }); + } services.set('protocol', { registerUninstallCleanup: (name: string, cleanup: any) => cleanups.set(name, cleanup), }); @@ -70,7 +91,7 @@ function kernel(opts: { failFast?: string; withConnection?: boolean } = {}) { logger, }; const codeNames = () => (services.get(CODE_DATASOURCE_NAMES_SERVICE) as CodeDatasourceNames | undefined)?.list() ?? []; - return { ctx: ctx as never, items, states, cleanups, connectCalls, disconnected, logger, codeNames }; + return { ctx: ctx as never, items, states, cleanups, connectCalls, disconnected, drivers, logger, codeNames }; } const STORE = { name: 'pkg_store', driver: 'sqlite', config: { filename: ':memory:' } }; @@ -178,3 +199,132 @@ describe('connectPackageDatasources — the boot artifact', () => { expect(k.items.get('datasource/pkg_store')?.origin).toBe('code'); }); }); + +describe('datasourceNameHolder — who holds a name an installed package declares', () => { + const judge = (k: ReturnType, name: string) => datasourceNameHolder(k.ctx, 'com.example.pkg', name); + + it('the host\'s default, a Setup row, another package and an engine-only driver each hold their name', async () => { + const k = kernel({ withEngine: true }); + k.items.set('datasource/setup_ds', { name: 'setup_ds', origin: 'runtime' }); + k.items.set('datasource/other_ds', { name: 'other_ds', origin: 'code', _packageId: 'com.example.other' }); + k.drivers.add('bridge_ds'); + expect(await judge(k, 'default')).toEqual({ status: 'held', holder: "the host's primary datasource" }); + expect(await judge(k, 'setup_ds')).toEqual({ status: 'held', holder: 'a datasource created in Setup' }); + expect(await judge(k, 'other_ds')).toEqual({ status: 'held', holder: "the package 'com.example.other'" }); + expect(await judge(k, 'bridge_ds')).toEqual({ status: 'held', holder: 'a driver registered on the data engine outside Setup' }); + }); + + it('a name in the code-datasource set this package did not record is held by the host\'s code', async () => { + const k = kernel(); + contributeCodeDatasourceNames(k.ctx as never, ['boot_ds']); + expect(await judge(k, 'boot_ds')).toEqual({ status: 'held', holder: 'a datasource this host registers from code' }); + }); + + it('a holder that cannot be read is unreadable, never free', async () => { + const k = kernel({ metadataFault: 'pkg_store' }); + const holding = await judge(k, 'pkg_store'); + expect(holding).toMatchObject({ status: 'unreadable', reading: 'the metadata registry' }); + + const e = kernel(); + (e.ctx as any).getService = ((orig) => (name: string) => { + if (name === 'objectql') return { getDriverByName: () => { throw new Error('engine gone'); } }; + return orig(name); + })((e.ctx as any).getService.bind(e.ctx)); + expect(await datasourceNameHolder(e.ctx, 'com.example.pkg', 'pkg_store')).toMatchObject({ + status: 'unreadable', + reading: 'the data engine', + }); + }); + + it('the package\'s own registration and pool are free: a reinstall passes', async () => { + const k = kernel({ withEngine: true }); + await connectPackageDatasources(k.ctx, { + appId: 'com.example.pkg', + datasources: [STORE], + objects: OBJECTS, + installed: true, + }); + expect(k.drivers.has('pkg_store')).toBe(true); + expect(await judge(k, 'pkg_store')).toEqual({ status: 'free' }); + // …and with its metadata registration gone, the pool it recorded is still its own. + k.items.delete('datasource/pkg_store'); + expect(await judge(k, 'pkg_store')).toEqual({ status: 'free' }); + // CONTROL: to another package, the same pool is this one's. + expect(await datasourceNameHolder(k.ctx, 'com.example.other', 'pkg_store')).toEqual({ + status: 'held', + holder: "the package 'com.example.pkg'", + }); + }); + + it('nothing anywhere: free', async () => { + expect(await judge(kernel({ withEngine: true }), 'pkg_store')).toEqual({ status: 'free' }); + }); +}); + +describe('connectPackageDatasources — a name another holder already holds (a rehydrate of an older install)', () => { + it('a Setup row\'s name is not taken over: not registered as code, not recorded, not connected, and reported', async () => { + const k = kernel({ withEngine: true }); + const setupRow = { name: 'pkg_store', origin: 'runtime', driver: 'postgres' }; + k.items.set('datasource/pkg_store', setupRow); + k.drivers.add('pkg_store'); // the Setup row's pool + const verdicts = await connectPackageDatasources(k.ctx, { + appId: 'com.example.pkg', + source: 'MarketplaceInstallLocal', + datasources: [STORE, VAULT], + objects: OBJECTS, + installed: true, + }); + expect(verdicts).toEqual([{ name: 'pkg_vault', status: 'connected' }]); + expect(k.items.get('datasource/pkg_store')).toBe(setupRow); + expect(k.codeNames()).toEqual(['pkg_vault']); + expect(k.connectCalls.map((c) => c.datasources.map((d) => d.name))).toEqual([['pkg_vault']]); + expect(k.logger.error).toHaveBeenCalledTimes(1); + const line = String(k.logger.error.mock.calls[0][0]); + expect(line).toContain("'pkg_store'"); + expect(line).toContain('a datasource created in Setup'); + + // The uninstall leaves the Setup row's pool and registration alone. + expect(await k.cleanups.get('runtime.package-datasources')!({ packageId: 'com.example.pkg' })).toEqual({ + success: true, + removed: 1, + }); + expect(k.disconnected).toEqual(['pkg_vault']); + expect(k.drivers.has('pkg_store')).toBe(true); + expect(k.items.get('datasource/pkg_store')).toBe(setupRow); + }); + + it('an engine-only holder (the legacy onEnable bridge) is not taken over either', async () => { + const k = kernel({ withEngine: true }); + k.drivers.add('pkg_store'); + const verdicts = await connectPackageDatasources(k.ctx, { + appId: 'com.example.pkg', + datasources: [STORE], + objects: OBJECTS, + installed: true, + }); + expect(verdicts).toEqual([]); + expect(k.items.has('datasource/pkg_store')).toBe(false); + expect(k.codeNames()).toEqual([]); + expect(k.connectCalls).toEqual([]); + expect(String(k.logger.error.mock.calls[0]?.[0])).toContain('a driver registered on the data engine outside Setup'); + }); + + it('a holder that cannot be read is not taken over: the name is left alone and the fault is reported', async () => { + const k = kernel({ withEngine: true, metadataFault: 'pkg_store' }); + await connectPackageDatasources(k.ctx, { appId: 'com.example.pkg', datasources: [STORE], installed: true }); + expect(k.codeNames()).toEqual([]); + expect(k.connectCalls).toEqual([]); + expect(String(k.logger.error.mock.calls[0]?.[0])).toContain('metadata loader unavailable'); + }); + + it('a pool the connect only found (already-registered) is not the package\'s to evict on uninstall', async () => { + const k = kernel({ answer: { pkg_store: 'already-registered' } }); + await connectPackageDatasources(k.ctx, { appId: 'com.example.pkg', datasources: [STORE, VAULT], installed: true }); + expect(await k.cleanups.get('runtime.package-datasources')!({ packageId: 'com.example.pkg' })).toEqual({ + success: true, + removed: 2, + }); + expect(k.disconnected).toEqual(['pkg_vault']); + expect(k.codeNames()).toEqual([]); + }); +}); diff --git a/packages/runtime/src/declared-datasources.ts b/packages/runtime/src/declared-datasources.ts index 17b4535642f..ddb9ddcbc7e 100644 --- a/packages/runtime/src/declared-datasources.ts +++ b/packages/runtime/src/declared-datasources.ts @@ -37,6 +37,16 @@ * connection service resolves `external.credentialsRef` the one way it does * for every datasource, and a credential it cannot resolve is its * `failed-credentials` verdict. + * + * [#28143] An installed package never takes over a name another holder owns. + * {@link datasourceNameHolder} is the ONE judge of "who holds this name": the + * install door refuses on it before any write (`409` held, `503` when a holder + * cannot be read), and an installed call — the rehydrate above all, which + * replays packages installed before the door judged names at all — leaves a + * held name alone and reports it, rather than registering it as code over the + * holder's registration, recording it in the code-datasource set and answering + * the holder's pool as its own. The uninstall then withdraws only what the + * package's own call recorded, and closes only the pools that call opened. */ import type { PluginContext } from '@objectstack/core'; @@ -108,11 +118,22 @@ interface ConnectionServiceLike { const PACKAGE_DATASOURCES_UNINSTALL_CLEANUP = 'runtime.package-datasources'; /** - * Per kernel (keyed by its code-datasource set), the datasource names an - * installed package's call registered — what its reinstall and its uninstall - * withdraw. A boot artifact's are never recorded: the boot owns them. + * What one installed package's call registered: the datasource names its + * reinstall and its uninstall withdraw, and of those the ones whose pool the + * connect found already registered rather than opened (`already-registered`) — + * a pool somebody else opened, which the withdrawal never closes. + */ +interface InstalledDatasources { + names: string[]; + found: Set; +} + +/** + * Per kernel (keyed by its code-datasource set), what each installed package's + * call registered ({@link InstalledDatasources}). A boot artifact's are never + * recorded: the boot owns them. */ -const INSTALLED_BY_APP = new WeakMap>(); +const INSTALLED_BY_APP = new WeakMap>(); /** The kernel's code-datasource set, which keys {@link INSTALLED_BY_APP}; `undefined` before any producer contributed. */ function kernelKey(ctx: PluginContext): object | undefined { @@ -153,15 +174,28 @@ export async function connectPackageDatasources( if (installed) await withdrawPackageDatasources(ctx, appId); if (datasources.length === 0) return []; + // [#28143] An installed package's names are judged after its own earlier + // registrations are withdrawn, so whatever still holds one is another + // holder's. A held name — or one whose holder cannot be read — is left out + // of all three steps below, and reported. + let left: ReadonlySet = new Set(); + let key: object | undefined; if (installed) { + left = await leaveHeldNames(ctx, appId, tag, ownedOf().map(({ datasource }) => datasource?.name)); const names = ownedOf() .map(({ datasource }) => datasource?.name) - .filter((name): name is string => typeof name === 'string' && name.length > 0); + .filter((name): name is string => typeof name === 'string' && name.length > 0 && !left.has(name)); contributeCodeDatasourceNames(ctx, names); - const key = kernelKey(ctx); + key = kernelKey(ctx); if (key) recordInstalled(key, appId, names); ensureDatasourceUninstallCleanup(ctx); } + const taken = left.size === 0 + // By reference when nothing is left out: the boot hands the connect the + // very list its one forward conversion produced (#23029 pins the identity). + ? datasources + : datasources.filter((ds) => !left.has(ds?.name)); + if (taken.length === 0) return []; // 1. The metadata registration — lenient: a registry that refuses leaves // the datasource unlisted, never the package unloaded. @@ -171,12 +205,12 @@ export async function connectPackageDatasources( | undefined; if (typeof metadata?.registerInMemory === 'function') { for (const { datasource: ds, owner } of ownedOf()) { - if (!ds?.name) continue; + if (!ds?.name || left.has(ds.name)) continue; metadata.registerInMemory('datasource', ds.name, applyProtection({ ...ds, origin: 'code' }, owner)); } ctx.logger.info('Registered code-defined datasources in metadata registry', { appId, - count: datasources.length, + count: taken.length, }); } } catch (err) { @@ -202,12 +236,11 @@ export async function connectPackageDatasources( } try { const results = await connection.connectDeclared({ - // By reference: the boot hands the connect the very list its one - // forward conversion produced (#23029 pins the identity). - datasources: datasources as any[], + datasources: taken as any[], objects: options.objects ?? [], mappedObjects: options.mappedObjects ?? {}, }); + if (key) recordFound(key, appId, results); const connected = results.filter((r) => r.status === 'connected'); if (connected.length > 0) { ctx.logger.info('Auto-connected declared datasources', { @@ -235,10 +268,11 @@ export async function connectPackageDatasources( // object answers `ERR_DATASOURCE_UNAVAILABLE` naming it — never a // silent driver gap. const verdicts: PackageDatasourceVerdict[] = []; - for (const ds of datasources) { + for (const ds of taken) { const state = typeof ds?.name === 'string' ? connection.getConnectionState?.(ds.name) : undefined; if (state) verdicts.push({ name: state.name, status: state.status }); } + if (key) recordFound(key, appId, verdicts); return verdicts; } } @@ -250,14 +284,179 @@ function recordInstalled(key: object, appId: string, names: string[]): void { byApp = new Map(); INSTALLED_BY_APP.set(key, byApp); } - if (names.length > 0) byApp.set(appId, [...names]); + if (names.length > 0) byApp.set(appId, { names: [...names], found: new Set() }); else byApp.delete(appId); } +/** + * [#28143] Record which of the package's datasources the connect found already + * registered (`already-registered`): a driver it did not open, and that the + * package's withdrawal therefore never closes. + */ +function recordFound(key: object, appId: string, verdicts: ReadonlyArray<{ name: string; status: string }>): void { + const record = INSTALLED_BY_APP.get(key)?.get(appId); + if (!record) return; + for (const { name, status } of verdicts) { + if (status === 'already-registered') record.found.add(name); + } +} + +/** The app id whose installed call recorded `name`, other than `appId`; `undefined` when none did. */ +function recordedByAnother(key: object | undefined, appId: string, name: string): string | undefined { + if (!key) return undefined; + for (const [other, record] of INSTALLED_BY_APP.get(key) ?? []) { + if (other !== appId && record.names.includes(name)) return other; + } + return undefined; +} + +/** A service the kernel may not have: `undefined` when it is absent. */ +function optionalService(ctx: PluginContext, name: string): T | undefined { + try { + return ctx.getService(name); + } catch { + return undefined; + } +} + +/** + * [#28143] Who holds a datasource name, as an installed package declaring it + * must see it — {@link datasourceNameHolder}'s answer. + * + * - `free`: nobody but the package itself (its own earlier registration or + * pool) holds the name. + * - `held`: another holder does, named in words for the refusal and the log. + * - `unreadable`: a place a holder lives could not be read. ⛔ Never read as + * `free`: a check that did not run is not "no holder". + */ +export type DatasourceNameHolding = + | { status: 'free' } + | { status: 'held'; holder: string } + | { status: 'unreadable'; reading: string; cause: unknown }; + +/** + * [#28143] The ONE judge of who holds a datasource name a package installed + * into the running kernel declares. The install door refuses on it before any + * write; {@link connectPackageDatasources} leaves a held name alone on every + * installed call, the rehydrate included. + * + * Every place a holder lives is read, in this order, and the first holder + * found answers: + * + * 1. the host's `default` — always held; + * 2. the metadata registry: a datasource item stamped with another package, a + * datasource created in Setup (`origin: 'runtime'`), or any other item. An + * item stamped with this package is its own earlier registration: `free`; + * 3. the code-datasource set: a name another installed package recorded, or + * one the host registers from code (the boot artifact's, whose metadata + * registration is lenient and may be missing); + * 4. the data engine: a driver registered under the name with no metadata + * item — the legacy `onEnable` bridge (ADR-0062 D8). A pool this package's + * own earlier call recorded is its own. + * + * A service the kernel does not have holds nothing; a read that throws is + * `unreadable`. + */ +export async function datasourceNameHolder( + ctx: PluginContext, + appId: string, + name: string, +): Promise { + if (name === 'default') return { status: 'held', holder: "the host's primary datasource" }; + + const metadata = optionalService<{ get?: (type: string, name: string) => Promise }>(ctx, 'metadata'); + if (typeof metadata?.get === 'function') { + let item: unknown; + try { + item = await metadata.get('datasource', name); + } catch (cause) { + return { status: 'unreadable', reading: 'the metadata registry', cause }; + } + if (item !== null && typeof item === 'object' && !Array.isArray(item)) { + const { _packageId: owner, origin } = item as { _packageId?: unknown; origin?: unknown }; + if (owner === appId) return { status: 'free' }; + return { + status: 'held', + holder: typeof owner === 'string' && owner.length > 0 + ? `the package '${owner}'` + : origin === 'runtime' ? 'a datasource created in Setup' : 'a datasource', + }; + } + } + + const key = kernelKey(ctx); + const ownNames = (key && INSTALLED_BY_APP.get(key)?.get(appId)?.names) ?? []; + const other = recordedByAnother(key, appId, name); + if (other !== undefined) return { status: 'held', holder: `the package '${other}'` }; + if (key && !ownNames.includes(name)) { + let coded: boolean; + try { + coded = typeof (key as { has?: unknown }).has === 'function' && (key as { has(n: string): boolean }).has(name); + } catch (cause) { + return { status: 'unreadable', reading: "the host's code-datasource set", cause }; + } + if (coded) return { status: 'held', holder: 'a datasource this host registers from code' }; + } + + const engine = optionalService<{ getDriverByName?: (name: string) => unknown }>(ctx, 'objectql'); + if (typeof engine?.getDriverByName === 'function') { + let driver: unknown; + try { + driver = engine.getDriverByName(name); + } catch (cause) { + return { status: 'unreadable', reading: 'the data engine', cause }; + } + if (driver && !ownNames.includes(name)) { + return { status: 'held', holder: 'a driver registered on the data engine outside Setup' }; + } + } + return { status: 'free' }; +} + +/** + * [#28143] Judge each name an installed call declares and answer the ones it + * leaves alone — held by another holder, or with a holder that cannot be read + * — after one `error` line naming each and what it means. + * + * `error`, not `warn`: the ledger says the package is installed with these + * datasources while the running kernel registers none of them as the + * package's, and nothing else looks wrong. On a rehydrate this is a package + * installed before the install door judged names; the install door itself + * refuses before it gets here. + */ +async function leaveHeldNames( + ctx: PluginContext, + appId: string, + tag: string, + declared: ReadonlyArray, +): Promise> { + const left = new Set(); + const lines: string[] = []; + for (const name of declared) { + if (typeof name !== 'string' || name.length === 0 || left.has(name)) continue; + const holding = await datasourceNameHolder(ctx, appId, name); + if (holding.status === 'free') continue; + left.add(name); + lines.push(holding.status === 'held' + ? `'${name}' is held by ${holding.holder}` + : `'${name}' could not be judged: ${holding.reading} could not be read (${(holding.cause as Error)?.message ?? String(holding.cause)})`); + } + if (lines.length > 0) { + ctx.logger.error( + `${tag} the package '${appId}' declares datasource name(s) it does not hold, and they are NOT registered, ` + + `recorded or connected for it: ${lines.join('; ')}. The package's objects bound to them are served by ` + + "whatever holds the name, and the package's uninstall leaves that holder alone. Rename the package's " + + 'datasource and install it again, or remove the datasource holding the name and restart.', + ); + } + return left; +} + /** * Withdraw every datasource an installed package's earlier call registered: * its pool disconnected (the connection service's own teardown, which evicts - * the driver and drops the retained verdict), its metadata registration + * the driver and drops the retained verdict) unless that call found the pool + * already registered by another holder, its metadata registration * removed — only a registration still stamped with this package's id — and its * name taken out of the code-datasource set. Answers the names withdrawn and * the names whose metadata registration could not be removed. @@ -267,7 +466,8 @@ async function withdrawPackageDatasources( appId: string, ): Promise<{ removed: string[]; kept: string[]; cause?: unknown }> { const key = kernelKey(ctx); - const names = (key && INSTALLED_BY_APP.get(key)?.get(appId)) ?? []; + const record = key ? INSTALLED_BY_APP.get(key)?.get(appId) : undefined; + const names = record?.names ?? []; if (!key || names.length === 0) return { removed: [], kept: [] }; let metadata: { get?: (t: string, n: string) => Promise; unregister?: (t: string, n: string, o?: { notify?: boolean }) => Promise } | undefined; @@ -287,8 +487,9 @@ async function withdrawPackageDatasources( let cause: unknown; for (const name of names) { // Never throws: a failed close is logged by the service, and the driver - // is evicted either way. - if (typeof connection?.disconnect === 'function') await connection.disconnect(name); + // is evicted either way. [#28143] Only a pool this package's connect + // opened: one it found already registered is its holder's to close. + if (typeof connection?.disconnect === 'function' && !record?.found.has(name)) await connection.disconnect(name); try { const item = await metadata?.get?.('datasource', name); if ((item as { _packageId?: unknown } | undefined)?._packageId === appId) { diff --git a/packages/runtime/src/index.ts b/packages/runtime/src/index.ts index 4918413ef82..afaf5e59076 100644 --- a/packages/runtime/src/index.ts +++ b/packages/runtime/src/index.ts @@ -178,9 +178,11 @@ export type { } from './app-artifact-handlers.js'; // [#28039] The ONE path that registers, records and connects the datasources a // package declares — called by `AppPlugin.start` and by the install-local plugin -// (`@objectstack/cloud-connection`) on install and rehydrate. -export { connectPackageDatasources } from './declared-datasources.js'; +// (`@objectstack/cloud-connection`) on install and rehydrate — and [#28143] the +// ONE judge of who holds a datasource name, which the install door refuses on. +export { connectPackageDatasources, datasourceNameHolder } from './declared-datasources.js'; export type { + DatasourceNameHolding, DeclaredDatasourceOwner, OwnedDatasources, PackageDatasourceOptions,