diff --git a/.changeset/eighty-hoops-repeat.md b/.changeset/eighty-hoops-repeat.md new file mode 100644 index 0000000000..7021869b36 --- /dev/null +++ b/.changeset/eighty-hoops-repeat.md @@ -0,0 +1,23 @@ +--- +'@objectstack/service-datasource': patch +--- + +Ledger the mounted datasource-admin routes, at the spelling they are mounted under. + +The ten admin CRUD routes under `/api/v1/datasources` — list, read, create, patch, +remove, connection probe, driver catalog and schema introspection — carried no +route-ledger entry in any of the three ledgers the platform keeps. They are mounted +the "third way" `service-storage` and `service-i18n` grew their own ledgers for: +`objectstack serve` builds a small plugin that resolves the `http.server` service and +registers straight on `IHttpServer`, so neither `RouteManager` nor +`RestServer.getRoutes()` ever sees them. + +The five `datasources` rows the REST ledger does carry are the **federation** family +(`/api/v1/datasources/:name/external/…`), which is a different, separately mounted +family in `@objectstack/rest` — not the admin family misspelled. Both are live, and +no route is renamed here: the new ledger is written at the live admin spelling, and +its conformance guard derives what it expects from the registrar rather than from a +literal in the test, so an eleventh route fails the guard instead of silently +reopening the gap. + +No runtime behaviour changes — this adds a package-internal ledger and its guard. diff --git a/packages/client/src/service-route-ledger-coverage.test.ts b/packages/client/src/service-route-ledger-coverage.test.ts index 2cdc7122d5..2a181b936b 100644 --- a/packages/client/src/service-route-ledger-coverage.test.ts +++ b/packages/client/src/service-route-ledger-coverage.test.ts @@ -7,28 +7,31 @@ * * - `packages/services/service-storage/src/storage-route-ledger.conformance.test.ts` * - `packages/services/service-i18n/src/i18n-route-ledger.conformance.test.ts` + * - `packages/services/service-datasource/src/datasource-route-ledger.conformance.test.ts` * * Same contract as the two ledger guards next door * (`route-ledger-coverage.test.ts`, `rest-route-ledger-coverage.test.ts`): * every ledger entry that names a client method must resolve to a real * function on an instantiated client. * - * Both ledgers are imported as relative SOURCE files deliberately: they are - * pure data (no imports), and a client→service package edge for them would be - * backwards — the services are where the routes are declared, so the ledgers + * All three ledgers are imported as relative SOURCE files deliberately: they + * are pure data (no imports), and a client→service package edge for them would + * be backwards — the services are where the routes are declared, so the ledgers * live there, and each package verifies its own half. That is the tranche-1 * lesson (`no runtime→client package edge`) applied verbatim: CI's per-package * test tasks build only their own dependency closure, so a real package edge * here would be unbuildable. * - * With these two, all THREE server surfaces the SDK reaches are ledgered — - * dispatcher (#3563), REST (#3587), autonomous service mounts (#3636). + * With these, all THREE server surfaces the SDK reaches are ledgered — + * dispatcher (#3563), REST (#3587), autonomous service mounts (#3636, joined by + * the datasource-admin family in #7744). */ import { describe, it, expect } from 'vitest'; import { ObjectStackClient } from './index'; import { STORAGE_ROUTE_LEDGER } from '../../services/service-storage/src/storage-route-ledger'; import { I18N_ROUTE_LEDGER } from '../../services/service-i18n/src/i18n-route-ledger'; +import { DATASOURCE_ROUTE_LEDGER } from '../../services/service-datasource/src/datasource-route-ledger'; describe('service route ledgers ↔ @objectstack/client surface', () => { const client = new ObjectStackClient({ baseUrl: 'http://localhost:9' }); @@ -58,6 +61,31 @@ describe('service route ledgers ↔ @objectstack/client surface', () => { ).toEqual([]); }); + it('every datasource-admin ledger entry naming a client method resolves to a real function', () => { + const broken = brokenIn(DATASOURCE_ROUTE_LEDGER); + expect( + broken, + `datasource-admin ledger entries claiming a client method that does not exist: ${broken.join('; ')}`, + ).toEqual([]); + }); + + it('the datasource-admin family is audited as reaching NO client method', () => { + // Said as a measurement rather than left implicit, because the assertion + // above holds vacuously while every row is `server-only` — and a guard + // that can only ever pass is the "declared but unverified" shape these + // ledgers exist to remove. What is measured here is the #7744 audit's + // actual finding: `client.datasources` reaches the FEDERATION family in + // packages/rest (ledgered there, as `datasources.external.*`), and no + // method reaches the Setup/console lifecycle family. The day the SDK + // gains a datasource-lifecycle method, this expectation is the one that + // fails and sends the author to the ledger row that must name it. + const claimed = DATASOURCE_ROUTE_LEDGER.map((e) => e.client).filter((c) => c != null); + expect(claimed).toEqual([]); + + const datasources = (client as unknown as { datasources: Record }).datasources; + expect(Object.keys(datasources)).toEqual(['external']); + }); + it('the whole `storage` namespace is backed by a ledger row', () => { // The reverse direction, scoped to the namespace this tranche audited: // a client method reaching a storage route that no ledger row backs is diff --git a/packages/services/service-datasource/src/datasource-route-ledger.conformance.test.ts b/packages/services/service-datasource/src/datasource-route-ledger.conformance.test.ts new file mode 100644 index 0000000000..5f189fa59b --- /dev/null +++ b/packages/services/service-datasource/src/datasource-route-ledger.conformance.test.ts @@ -0,0 +1,159 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Datasource-admin route-ledger conformance (#7744) — the guard that keeps the + * autonomously-mounted datasource-admin surface and `@objectstack/client` from + * drifting apart silently, mirroring the storage (#3636) and i18n (#3636) + * guards. + * + * Directions made loud here: + * + * 1. A route `registerDatasourceAdminRoutes` mounts with no ledger entry — a + * new route landed without a reviewed SDK disposition. This is the + * direction that was open when #7744 was filed: all ten routes were in it. + * 2. A ledger entry for a route the registrar no longer mounts — the ledger + * went stale. + * + * ENUMERATION IS DERIVED, NOT TRANSCRIBED, and that is the whole point of the + * file. The registrar runs against a capturing mock `IHttpServer` and its + * registration calls ARE the route set — the same seam the two tranche-3 + * ledgers use. No path literal in this file is compared against the mount: a + * hand-copied expectation would go green on a ledger that agrees with the test + * and disagrees with the server, which is exactly the class of gap #7744 + * reported (the REST ledger spelled the family `/external/tables` while the + * mount said `/remote-tables`, and nothing was comparing either to a mount). + * Add an eleventh route and this file fails until it is ledgered. + * + * The third direction — "every `sdk` row names a client method that exists" — + * lives in `packages/client/src/service-route-ledger-coverage.test.ts`, next to + * the SDK it introspects: a service→client package edge would be backwards. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { registerDatasourceAdminRoutes } from './admin-routes.js'; +import { DATASOURCE_ROUTE_LEDGER } from './datasource-route-ledger.js'; + +/** Minimal IHttpServer mock that records registrations. */ +function createMockServer() { + return { + get: vi.fn(), + post: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + }; +} + +/** + * `VERB /path` keys for every route the registrar mounts at the DEFAULT base. + * + * The context is never consulted during registration — every handler resolves + * its service per request through `resolve()`, which is what lets the family + * answer 503 rather than record a boot-time verdict about a service that may + * still register (AGENTS.md, "never record a verdict the boot can still + * contradict"). So a context that answers `undefined` for everything still + * enumerates the full surface. + */ +function enumerateDatasourceAdminRoutes(): Set { + const server = createMockServer(); + const ctx = { getService: () => undefined, logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() } }; + registerDatasourceAdminRoutes(server as never, ctx as never, '/api/v1'); + const keys = new Set(); + for (const verb of ['get', 'post', 'put', 'patch', 'delete'] as const) { + for (const call of server[verb].mock.calls) { + keys.add(`${verb.toUpperCase()} ${String(call[0])}`); + } + } + return keys; +} + +const ledgerKeys = (): Set => new Set(DATASOURCE_ROUTE_LEDGER.map((e) => e.route)); + +describe('datasource-admin route ledger ↔ registerDatasourceAdminRoutes enumeration', () => { + it('every mounted datasource-admin route has a ledger entry', () => { + const ledger = ledgerKeys(); + const missing = [...enumerateDatasourceAdminRoutes()].filter((k) => !ledger.has(k)); + expect( + missing, + `Datasource-admin routes with no datasource-route-ledger entry: ${missing.join(', ')}. ` + + 'A new route needs a reviewed disposition in datasource-route-ledger.ts (#7744).', + ).toEqual([]); + }); + + it('every ledger entry is really mounted by the registrar', () => { + const live = enumerateDatasourceAdminRoutes(); + const stale = [...ledgerKeys()].filter((k) => !live.has(k)); + expect( + stale, + `datasource-route-ledger entries the registrar no longer mounts: ${stale.join(', ')}. ` + + 'Remove or reclassify them so the ledger stays truthful.', + ).toEqual([]); + }); + + it('no route is ledgered twice', () => { + const seen = new Set(); + const dupes = DATASOURCE_ROUTE_LEDGER.map((e) => e.route).filter((r) => !seen.add(r)); + expect(dupes, `duplicate datasource-route-ledger rows: ${dupes.join(', ')}`).toEqual([]); + }); + + it('the ledger is compared against a real enumeration, not an empty one', () => { + // Absence must be loud (AGENTS.md, Route & surface ownership §3). Both + // set-difference assertions above pass vacuously if the registrar ever + // stops registering — a refactor that moves the mount elsewhere, or a mock + // whose recorded calls stop being readable, would leave this file green + // while guarding nothing. So assert the enumeration produced something, + // and that the two sides are the same size rather than merely non-conflicting. + const live = enumerateDatasourceAdminRoutes(); + expect(live.size).toBeGreaterThan(0); + expect(live.size).toBe(ledgerKeys().size); + }); + + it('carries the LIVE admin spelling of the introspection route, not the federation one', () => { + // The #7744 regression pin, stated as the card states it. `/remote-tables` + // is what `admin-routes.ts` mounts; `/external/tables` is the separate + // federation route in packages/rest, ledgered there. Reading the value out + // of the ENUMERATION rather than asserting a literal is what keeps this an + // assertion about the mount: if the mount were ever renamed, the row this + // resolves would follow it — while the first two assertions above would + // still catch the rename as a ledger diff. + const live = enumerateDatasourceAdminRoutes(); + const introspection = [...live].filter((k) => k.includes('remote-tables')); + expect(introspection).not.toEqual([]); + for (const key of introspection) { + expect( + ledgerKeys().has(key), + `${key} is mounted but unledgered — the #7744 gap, reopened.`, + ).toBe(true); + } + // And the federation spelling is NOT this ledger's business: it belongs to + // packages/rest, whose own conformance guard would fail if it moved here. + expect([...ledgerKeys()].filter((k) => k.includes('/external/'))).toEqual([]); + }); +}); + +describe('datasource-admin route ledger hygiene', () => { + it('every `sdk` entry names its client method; every non-sdk entry carries a rationale', () => { + const sdkWithout = DATASOURCE_ROUTE_LEDGER.filter((e) => e.disposition === 'sdk' && !e.client).map((e) => e.route); + expect(sdkWithout, 'sdk-disposition entries missing a client method name').toEqual([]); + + const bareNonSdk = DATASOURCE_ROUTE_LEDGER.filter((e) => e.disposition !== 'sdk' && !e.note).map((e) => e.route); + expect(bareNonSdk, 'non-sdk entries must say WHY they are not SDK surface').toEqual([]); + }); + + it('gap and mismatch counts only shrink — update the ledger (and these numbers) when closing them', () => { + // Ratchet, not aspiration. The family audited at #7744 as ten reviewed + // `server-only` rows: no client method and no CLI command reaches any of + // them, and they are mounted by `objectstack serve` rather than by + // `@objectstack/rest`. Zero is therefore the measured state, not an + // aspiration — and a `gap` row here would be a product decision to give + // the SDK a datasource-lifecycle surface, which needs its own review. + const gaps = DATASOURCE_ROUTE_LEDGER.filter((e) => e.disposition === 'gap').length; + expect(gaps).toBeLessThanOrEqual(0); + + const mismatches = DATASOURCE_ROUTE_LEDGER.filter((e) => e.disposition === 'mismatch').length; + expect(mismatches).toBeLessThanOrEqual(0); + }); +}); diff --git a/packages/services/service-datasource/src/datasource-route-ledger.ts b/packages/services/service-datasource/src/datasource-route-ledger.ts new file mode 100644 index 0000000000..b5b2cb0723 --- /dev/null +++ b/packages/services/service-datasource/src/datasource-route-ledger.ts @@ -0,0 +1,151 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Datasource-admin route ledger — the audited disposition of every HTTP route + * `registerDatasourceAdminRoutes` mounts, against what `@objectstack/client` + * can express (#7744, tranche 3 of the #3563 audit). + * + * WHY THIS EXISTS. This family had no ledger anywhere. The three ledgers that + * came before it each stop at their own package boundary, and this surface is + * outside all of them: + * + * - the dispatcher ledger (`packages/runtime/src/route-ledger.ts`) sees + * `RouteManager` routes; + * - the REST ledger (`packages/rest/src/rest-route-ledger.ts`) sees whatever + * `RestServer.getRoutes()` reports — its own routes plus the direct-mount + * registrars `mountAndRecordDirectRoutes` composes, and + * `registerDatasourceAdminRoutes` is not one of them; + * - `service-storage` and `service-i18n` carry their own ledgers (#3636) + * precisely because a service that reaches for the `http-server` service and + * registers straight on `IHttpServer` is invisible to both of the above. + * + * These routes are mounted that third way: `objectstack serve` builds a tiny + * `com.objectstack.cli.datasource-admin-routes` plugin whose `init()` resolves + * `http.server` and calls `registerDatasourceAdminRoutes(httpServer, ctx, + * '/api/v1')` (`packages/cli/src/commands/serve.ts`). So the whole Setup → + * Datasources backend — list, read, create, patch, remove, probe, driver + * catalog, schema introspection — sat in the pre-#3563 posture: mounted, + * working, and guarded by nothing. + * + * WHAT #7744 ACTUALLY FOUND, and what it is NOT. The REST ledger carries five + * `datasources` rows and every one of them is the FEDERATION family, spelled + * `/api/v1/datasources/:name/external/…`. Read quickly that looks like the + * admin family under a different name, and "reconcile the spelling" looks like + * the fix. It is not: the two spellings are different mounted routes, in + * different packages, and BOTH are live. Two of them do overlap — + * `GET /:name/remote-tables` here and `GET /:name/external/tables` there both + * reach `IExternalDatasourceService.listRemoteTables` (they diverge on `?schema=`, + * which only the federation twin forwards — #7955), as do + * `POST /:name/object-draft` and `POST /:name/external/tables/:remote/draft` + * over `generateObjectDraft`. That overlap is known and was deliberately + * reconciled rather than removed: #4249 gave the two paths ONE failure contract + * ("One operation, one failure contract now, on both paths", + * `packages/rest/src/external-datasource-routes.ts`). A ledger describes what is + * mounted; renaming a live route to close a bookkeeping gap would be an API + * break performed for the bookkeeping's benefit. So every row below carries the + * spelling the mount actually uses, and the conformance test derives its + * expectations from the registrar rather than from a literal copied into the + * test. + * + * `datasource-route-ledger.conformance.test.ts` fails when a route appears with + * no ledger entry, and when an entry names a route the registrar no longer + * mounts. The client half — every `sdk` row resolving to a real method — lives + * in `packages/client/src/service-route-ledger-coverage.test.ts`, next to the + * SDK it introspects, for the tranche-1 build-cycle reason: a service→client + * package edge would be backwards. + * + * SCOPE & SHAPE. Rows carry full wire paths at the DEFAULT base (`/api/v1`) — + * `registerDatasourceAdminRoutes`' third parameter can move the family, and the + * conformance test enumerates at the same default so the two stay comparable. + * + * WHY EVERY ROW IS `server-only`. Audited at #7744: `ObjectStackClient`'s + * `datasources` namespace contains exactly one sub-namespace, `external`, whose + * five methods reach the FEDERATION family in `packages/rest` — no client + * method reaches any route in this file, and neither does the CLI (its three + * `datasource` commands all call `/external/*`). That is consistent with how + * the family is composed: it is mounted by `objectstack serve`, not by + * `@objectstack/rest`, and its consumers are the Setup/Studio console and one + * declared metadata-type action (`test_connection`, contributed by + * `DatasourceAdminServicePlugin.init()` with + * `target: '/api/v1/datasources/${ctx.recordId}/test'`). Whether the SDK SHOULD + * gain a datasource-lifecycle surface is a product decision, and inventing one + * here — by writing ten `gap` rows — would be making that decision inside a + * bookkeeping fix. It is filed as #7954 instead; promoting any row to `sdk` + * belongs in the PR that adds the method. + * + * This module is package-internal (not exported from the index): it is the + * guard's data, not public API. It must stay import-free — the client-side + * guard imports it as a relative SOURCE file. + */ + +/** Disposition of a single datasource-admin route. Same vocabulary as the REST ledger. */ +export type DatasourceRouteDisposition = + /** Expressed by the SDK — `client` names the method (dotted path). */ + | 'sdk' + /** Should be in the SDK and is not — an open, acknowledged gap. */ + | 'gap' + /** Deliberately not SDK surface (console/Setup backends, static catalogs). */ + | 'server-only' + /** Public, unauthenticated browser-facing route. */ + | 'public' + /** Server and client disagree on the shape — needs reconciliation. */ + | 'mismatch'; + +export interface DatasourceRouteLedgerEntry { + /** `VERB /api/v1/datasources/...` — full wire path at the default base. */ + route: string; + /** Registrar family, for grouping and diff messages. */ + family: string; + disposition: DatasourceRouteDisposition; + /** Dotted method path on `ObjectStackClient` — required when disposition is `sdk`. */ + client?: string; + /** + * Name of the `@objectstack/spec/api` export declaring this route's response + * PAYLOAD — the `data` of the shared `{ success, data }` envelope. + * + * ⛔ DO NOT FILL A ROW THAT HAS NO CONFORMANCE COVERAGE — the same rule the + * REST and storage ledgers carry (#3877). A name written ahead of the test it + * points at would BE the "declared but unverified" surface the programme + * exists to remove. Every row here is unfilled today: the family's envelope + * coverage (`__tests__/envelope.conformance.test.ts`) asserts the ENVELOPE, + * not a payload schema, so no row has earned the field yet. + * + * A NAME rather than a live schema object, deliberately: this module stays + * import-free — the client-side guards compile it as a relative SOURCE file. + */ + responseSchema?: string; + /** One-line rationale. Required for every non-`sdk` disposition. */ + note?: string; +} + +export const DATASOURCE_ROUTE_LEDGER: readonly DatasourceRouteLedgerEntry[] = [ + // ── runtime datasource lifecycle (ADR-0015 Addendum §3.5) ───────────────── + // Served by `datasource-admin`; 503 SERVICE_UNAVAILABLE when that service is + // not wired, 400 DATASOURCE_ADMIN_ERROR on a refusal (#4249). + { route: 'GET /api/v1/datasources', family: 'datasource-lifecycle', disposition: 'server-only', + note: 'Setup → Datasources list: provenance (code/runtime) + the retained connect verdict per datasource (#3827). Console surface; the SDK expresses no datasource-lifecycle method.' }, + { route: 'GET /api/v1/datasources/:name', family: 'datasource-lifecycle', disposition: 'server-only', + note: 'edit-form read, credential-stripped (`config` plus a `hasSecret` flag, never `credentialsRef`). Console surface. Registered AFTER the literal `/drivers` route so that segment is never captured as a name.' }, + { route: 'POST /api/v1/datasources', family: 'datasource-lifecycle', disposition: 'server-only', + note: 'wizard "Save" — creates an `origin: runtime` datasource and answers 201 with the summary. Console surface.' }, + { route: 'PATCH /api/v1/datasources/:name', family: 'datasource-lifecycle', disposition: 'server-only', + note: 'wizard edit; runtime-origin only (a code-defined datasource is read-only). Console surface.' }, + { route: 'DELETE /api/v1/datasources/:name', family: 'datasource-lifecycle', disposition: 'server-only', + note: 'wizard delete; runtime-origin only, and refused while objects are still bound. Answers 204 with no body — the one route in this family outside the `{ success, data }` envelope, deliberately. Console surface.' }, + { route: 'POST /api/v1/datasources/test', family: 'datasource-lifecycle', disposition: 'server-only', + note: 'probes an UNSAVED draft carried inline (with an optional cleartext `secret` that never reaches the persisted draft) — the wizard\'s "Test connection" before Save. Console surface. Registered before the `:name` routes so the literal `test` segment is never captured as a name.' }, + + // ── driver catalog ──────────────────────────────────────────────────────── + { route: 'GET /api/v1/datasources/drivers', family: 'driver-catalog', disposition: 'server-only', + note: 'static `DRIVER_CATALOG` + each driver\'s JSON-Schema config; drives the Studio connection form (`packages/spec/src/data/datasource.zod.ts`). Needs no service, so it answers on every boot — the one route here that never degrades to 503.' }, + + // ── schema introspection for the Studio "sync objects" flow ─────────────── + // Served by `external-datasource`, so a refusal is EXTERNAL_DATASOURCE_ERROR + // and the 503 names that service rather than `datasource-admin` (#4225/#4249). + { route: 'GET /api/v1/datasources/:name/remote-tables', family: 'datasource-introspection', disposition: 'server-only', + note: 'lists a datasource\'s remote tables. The #7744 row: this is the LIVE admin spelling, and it is a different mounted route from the federation twin `GET /:name/external/tables` in packages/rest, which the REST ledger carries as `datasources.external.listTables`. Both reach `listRemoteTables` and share one failure contract by design (#4249); only the federation twin forwards `?schema=`, and only it is SDK-expressed.' }, + { route: 'POST /api/v1/datasources/:name/object-draft', family: 'datasource-introspection', disposition: 'server-only', + note: 'generates an ObjectStack object draft for one remote table (introspect + type-map, no persistence). Federation twin: `POST /:name/external/tables/:remote/draft` — same `generateObjectDraft` operation, and the twin is the SDK-expressed one (`datasources.external.draft`).' }, + { route: 'POST /api/v1/datasources/:name/test', family: 'datasource-introspection', disposition: 'server-only', + note: 'live round-trip against a SAVED datasource by name — distinct from `POST /datasources/test`, which probes an unsaved draft. This is the target of the declared `datasource` `test_connection` metadata-type action, contributed by DatasourceAdminServicePlugin.init(); the console renders the button from `/api/v1/meta`, so the caller is the action, not the SDK.' }, +];