diff --git a/.changeset/hono-standalone-discovery-computed.md b/.changeset/hono-standalone-discovery-computed.md new file mode 100644 index 0000000000..c6c0a46e6f --- /dev/null +++ b/.changeset/hono-standalone-discovery-computed.md @@ -0,0 +1,42 @@ +--- +"@objectstack/plugin-hono-server": patch +--- + +fix(plugin-hono-server): compute the standalone discovery `routes` from real registrations, and cede to the real owner (#4018) + +`registerStandardEndpoints` served a **fully static** discovery: a hardcoded +`routes` table listing `auth` / `packages` / `analytics` / `workflow` / +`automation` / `ai` / `notifications` / `i18n` / `storage` / `ui` regardless of +what the host actually mounted. A standalone Hono deployment therefore +advertised ten route families and 404'd on every one no plugin bridged — the +"advertise a route that doesn't exist" class ADR-0076 D12 exists to kill, and +the reason this surface disagreed with the two real discovery builders +(`HttpDispatcher.getDiscoveryInfo`, `metadata-protocol`'s `getDiscovery`), which +both compute per service at runtime. + +Two changes, no new discovery implementation to keep in sync: + +- **Single owner (D11 / OQ#9).** When `@objectstack/rest` or the runtime + dispatcher is on the kernel, this surface no longer registers + `${prefix}/discovery` — that plugin owns it. Both register during plugin + `start()`, i.e. before this `kernel:ready` hook, and Hono is + first-registration-wins, so they already shadowed this handler in every + composed deployment: the cede changes no served payload, it removes a third + one nobody read. `/.well-known/objectstack` is ceded to the dispatcher only + (REST never registers it), so a REST-without-dispatcher host keeps the + redirect. + +- **Computed, not hardcoded (D12).** When this surface does own `/discovery`, + `routes` is derived per request from the app's live route table: a family is + advertised iff a route is really registered at or under its base path. A + wildcard mounted *above* the base (global `/*` middleware, `/api/v1/*`) does + not count as a mount. + +**What changes for you.** On a standalone `HonoServerPlugin` host (no REST, no +dispatcher), `GET /api/v1/discovery` now omits every family nothing mounts — +most visibly `routes.metadata`, since `/api/v1/meta` ships with +`@objectstack/rest` / the dispatcher. Clients that read a route out of +discovery and call it stop getting a 404; `@objectstack/client` falls back to +the conventional path for any omitted key, so `client.connect()` is unaffected. +Composed deployments (`os serve`, cloud) are unchanged — the dispatcher's +service-aware discovery was already the one being served. diff --git a/docs/adr/0076-objectql-core-tiering.md b/docs/adr/0076-objectql-core-tiering.md index 626763222a..40991a0af7 100644 --- a/docs/adr/0076-objectql-core-tiering.md +++ b/docs/adr/0076-objectql-core-tiering.md @@ -143,6 +143,8 @@ Decision: each capability plugin registers its routes as a **normalized handler* This fixes the **whole class at once — without deleting any fallback** (no `/analytics` 404 regression): the analytics fallback and the dev stubs simply stop *lying*; they keep serving but are honestly labelled. It is the runtime enforcement of the D9-refinement principle (capabilities = what is actually installed, computed at runtime). +*(Update #4018: the same honesty binds the `routes` table, not only `services` — and it had a third publisher. `plugin-hono-server`'s `registerStandardEndpoints` convenience block still served a fully **static** `routes` map listing auth/packages/analytics/workflow/automation/ai/notifications/i18n/storage/ui regardless of what was mounted, so a standalone Hono host advertised ten route families and 404'd on the ones no plugin bridged. Closed on both axes: it now **cedes** `/discovery` to `@objectstack/rest` or the runtime dispatcher whenever either is on the kernel (single owner — D11 / OQ#9 worklist item 2; both register during `start()`, so first-registration-wins already shadowed this handler and the cede is behaviour-preserving), and when it does own the route it computes `routes` from the app's **live route table** — a family is advertised iff a route is really registered at or under its base. That keeps the honest answer without adding a third service-registry walk to keep in sync with the other two.)* + **Supersedes the rev.9 analytics conclusion**: the fix for the analytics fallback is to **mark it honestly (this D12)**, not "preserve-or-delete". *(Update #3891: superseded in turn for the analytics fallback specifically — honest labelling was necessary but not sufficient. The fallback's `degraded` label was accurate about capability, yet nothing in it disclosed that aggregates ran WITHOUT the caller's RLS/tenant scoping and that the contract `where` filter was ignored; an authorized caller still got a 200 with wrong (over-broad) numbers. A fallback may degrade features, never security semantics — so it was retired, and the "no `/analytics` 404 regression" goal above is deliberately abandoned for this slot: the 404 IS the honest signal. D12's marker/discovery machinery stays, and is what makes the now-empty slot report `unavailable`.)* **Execution**: framework (marker convention + `svcAvailable` respects it + discovery schema `stub` status) and console (read the honest status) land **together at the cross-repo window** — the console reads `discovery.services`, so this is a cross-repo contract change. diff --git a/packages/client/src/client.hono.test.ts b/packages/client/src/client.hono.test.ts index 72bb866214..185998928d 100644 --- a/packages/client/src/client.hono.test.ts +++ b/packages/client/src/client.hono.test.ts @@ -143,16 +143,23 @@ describe('ObjectStackClient (with Hono Server)', () => { it('should connect to hono server and discover endpoints', async () => { const client = new ObjectStackClient({ baseUrl }); await client.connect(); - + // Client should have populated discovery info expect(client['discoveryInfo']).toBeDefined(); - - // Verify endpoints from valid discovery response - // Standard: /api/v1/data, /api/v1/meta, etc. + + // The standalone hono surface advertises what it actually mounts + // (#4018): /data CRUD and the /auth/me/* helpers are registered here, + // so both are advertised and both answer. const endpoints = client['discoveryInfo']!.routes; expect(endpoints.data).toContain('/api/v1/data'); - expect(endpoints.metadata).toContain('/api/v1/meta'); expect(endpoints.auth).toContain('/api/v1/auth'); + + // `metadata` is NOT advertised on this boot, and that is the point of + // #4018: no plugin here mounts /api/v1/meta (it ships with + // @objectstack/rest / the dispatcher), so the old hardcoded table was + // promising a route that 404s. Proof the omission is honest: + expect(endpoints.metadata).toBeUndefined(); + expect((await fetch(`${baseUrl}/api/v1/meta/objects`)).status).toBe(404); }); it('should create and retrieve data via hono', async () => { diff --git a/packages/plugins/plugin-hono-server/src/hono-discovery.test.ts b/packages/plugins/plugin-hono-server/src/hono-discovery.test.ts new file mode 100644 index 0000000000..155637e193 --- /dev/null +++ b/packages/plugins/plugin-hono-server/src/hono-discovery.test.ts @@ -0,0 +1,158 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #4018 — the standard-endpoints convenience surface used to serve a fully +// STATIC discovery: a hardcoded `routes` table listing auth/packages/analytics/ +// workflow/automation/ai/notifications/i18n/storage/ui whether or not anything +// mounted them. That is the "advertise a route that 404s" class ADR-0076 D12 +// exists to kill, and it put this surface out of step with the two real +// discovery builders (the dispatcher's `getDiscoveryInfo`, the protocol's +// `getDiscovery`), which both compute per service. +// +// These tests drive the REAL Hono app the plugin builds — no mocked route +// table — so what they assert about "is this route mounted" is what a request +// would actually find. + +import { describe, it, expect } from 'vitest'; +import { HonoServerPlugin } from './hono-plugin'; + +const REST_API_PLUGIN = 'com.objectstack.rest.api'; +const RUNTIME_DISPATCHER_PLUGIN = 'com.objectstack.runtime.dispatcher'; + +/** + * Register the standard endpoints on a real Hono app and hand it back so tests + * can drive HTTP requests directly. `installedPlugins` seeds `kernel.hasPlugin`, + * which is how this surface decides whether a real discovery owner is present. + */ +function bootStandardEndpoints(installedPlugins: string[] = []) { + const plugin = new HonoServerPlugin({ port: 0 }); + const ctx: any = { + logger: { info() {}, debug() {}, warn() {}, error() {} }, + getKernel: () => ({ + hasPlugin: (name: string) => installedPlugins.includes(name), + getService: () => undefined, + }), + registerService: () => {}, + hook: () => {}, + getService: () => undefined, + }; + (plugin as any).registerDiscoveryAndCrudEndpoints(ctx); + return (plugin as any).server.getRawApp(); +} + +async function discoveryRoutes(app: any): Promise> { + const res = await app.request('http://localhost/api/v1/discovery'); + expect(res.status).toBe(200); + return (await res.json()).data.routes; +} + +describe('standalone discovery — routes are computed, never hardcoded (#4018)', () => { + it('advertises nothing for services this host does not mount', async () => { + const app = bootStandardEndpoints(); + const routes = await discoveryRoutes(app); + + // Every one of these was advertised unconditionally by the old static + // table; on a bare host each 404s, so none may be advertised now. + for (const family of [ + 'metadata', 'packages', 'analytics', 'workflow', 'automation', + 'ai', 'notifications', 'i18n', 'storage', 'ui', + ]) { + expect(routes[family], `${family} advertised but nothing mounts it`).toBeUndefined(); + } + }); + + it('advertises the families this surface really mounts, and they answer', async () => { + const app = bootStandardEndpoints(); + const routes = await discoveryRoutes(app); + + // The block mounts /data/:object CRUD and the /auth/me/* helpers, so + // both bases genuinely carry routes. (`/auth` on a bare host is only + // those helpers — the sign-in surface arrives with plugin-auth.) + expect(routes.data).toBe('/api/v1/data'); + expect(routes.auth).toBe('/api/v1/auth'); + + // Not a 404: the advertised base really has a live endpoint under it. + const res = await app.request('http://localhost/api/v1/data/thing'); + expect(res.status).not.toBe(404); + }); + + it('picks up a family once another plugin mounts it — including a wildcard', async () => { + const app = bootStandardEndpoints(); + // How plugin-auth / the dispatcher really mount: a wildcard under the + // family base, and a concrete child route. + app.all('/api/v1/i18n/*', (c: any) => c.json({})); + app.post('/api/v1/analytics/query', (c: any) => c.json({})); + + const routes = await discoveryRoutes(app); + expect(routes.i18n).toBe('/api/v1/i18n'); + expect(routes.analytics).toBe('/api/v1/analytics'); + }); + + it('reflects a mount that lands AFTER discovery is wired (not snapshotted)', async () => { + const app = bootStandardEndpoints(); + + // Sibling plugins keep registering through the rest of kernel:ready — + // i.e. after this hook wired `/discovery` — and Hono seals its matcher + // on the first request, so a mount can only ever arrive in this window. + // A table built at wiring time would miss it; one built per request + // does not. + app.get('/api/v1/workflow/definitions', (c: any) => c.json({})); + + expect((await discoveryRoutes(app)).workflow).toBe('/api/v1/workflow'); + }); + + it('does not count a wildcard mounted ABOVE the family base', async () => { + const app = bootStandardEndpoints(); + // Global middleware and a prefix-wide wildcard match every path but + // mount no family — treating them as a mount would re-advertise the + // whole table, which is the bug. + app.use('*', async (_c: any, next: any) => next()); + app.use('/api/v1/*', async (_c: any, next: any) => next()); + + const routes = await discoveryRoutes(app); + expect(routes.storage).toBeUndefined(); + expect(routes.ai).toBeUndefined(); + }); + + it('never advertises realtime — no HTTP surface exists for it (D12, #2462)', async () => { + const app = bootStandardEndpoints(); + expect((await discoveryRoutes(app)).realtime).toBeUndefined(); + }); + + it('still reports transactionalBatch=false — /batch ships with @objectstack/rest (#3298)', async () => { + const app = bootStandardEndpoints(); + const res = await app.request('http://localhost/api/v1/discovery'); + const body = await res.json(); + + expect(body.data.capabilities.transactionalBatch).toEqual({ enabled: false }); + expect((await app.request('http://localhost/api/v1/batch', { method: 'POST' })).status).toBe(404); + }); +}); + +describe('standalone discovery — single owner (ADR-0076 D11 / OQ#9)', () => { + it('cedes /discovery AND /.well-known to the dispatcher when it is installed', async () => { + const app = bootStandardEndpoints([RUNTIME_DISPATCHER_PLUGIN]); + + // The dispatcher registers both during plugin start() — before this + // kernel:ready hook — so it already served them; we must not publish a + // third payload behind it. + expect((await app.request('http://localhost/api/v1/discovery')).status).toBe(404); + expect((await app.request('http://localhost/.well-known/objectstack')).status).toBe(404); + }); + + it('cedes /discovery to @objectstack/rest but keeps /.well-known (REST never registers it)', async () => { + const app = bootStandardEndpoints([REST_API_PLUGIN]); + + expect((await app.request('http://localhost/api/v1/discovery')).status).toBe(404); + + const wellKnown = await app.request('http://localhost/.well-known/objectstack'); + expect(wellKnown.status).toBe(302); + expect(wellKnown.headers.get('location')).toBe('/api/v1/discovery'); + }); + + it('owns both when no real discovery plugin is on the kernel', async () => { + const app = bootStandardEndpoints(); + + expect((await app.request('http://localhost/api/v1/discovery')).status).toBe(200); + expect((await app.request('http://localhost/.well-known/objectstack')).status).toBe(302); + }); +}); diff --git a/packages/plugins/plugin-hono-server/src/hono-plugin.ts b/packages/plugins/plugin-hono-server/src/hono-plugin.ts index 27131cec2e..ee4dbe6619 100644 --- a/packages/plugins/plugin-hono-server/src/hono-plugin.ts +++ b/packages/plugins/plugin-hono-server/src/hono-plugin.ts @@ -7,6 +7,7 @@ import { } from '@objectstack/core'; import { RestServerConfig, + type ApiRoutes, } from '@objectstack/spec/api'; import { ADMIN_FULL_ACCESS, ORGANIZATION_ADMIN_GRANTS } from '@objectstack/spec'; import { @@ -370,6 +371,44 @@ export function annotateEffectiveApiOperations( } } +/** + * The two plugins that own a REAL, computed `/discovery` (ADR-0076 D11 / OQ#9). + * `@objectstack/rest` serves `metadata-protocol`'s registry-driven `getDiscovery()`; + * the runtime dispatcher serves `HttpDispatcher.getDiscoveryInfo()`. When either is + * on the kernel, this convenience surface cedes the route to it rather than + * publishing a third payload. + */ +const REST_API_PLUGIN = 'com.objectstack.rest.api'; +const RUNTIME_DISPATCHER_PLUGIN = 'com.objectstack.runtime.dispatcher'; + +/** + * Base-path segment for every route family this surface can advertise, keyed by + * its `ApiRoutes` field (`spec/api/discovery.zod.ts`). Typed against the spec so a + * renamed or dropped key breaks the build here instead of drifting silently. + * + * This is a path map, not a capability list — nothing here is advertised unless a + * matching route is actually registered (see `advertisableRoutes`). + * + * `realtime` is deliberately absent (ADR-0076 D12, #2462): service-realtime is an + * in-process pub/sub bus with no HTTP surface anywhere, so it could never be + * mounted under `/api/v1/realtime` and listing it would only invite a stale entry. + */ +const DISCOVERY_ROUTE_SEGMENTS: Partial> = { + data: 'data', + metadata: 'meta', + auth: 'auth', + packages: 'packages', + analytics: 'analytics', + workflow: 'workflow', + approvals: 'approvals', + automation: 'automation', + ai: 'ai', + notifications: 'notifications', + i18n: 'i18n', + storage: 'storage', + ui: 'ui', +}; + export class HonoServerPlugin implements Plugin { name = 'com.objectstack.server.hono'; type = 'server'; @@ -758,37 +797,76 @@ export class HonoServerPlugin implements Plugin { } /** - * Register discovery and basic CRUD endpoints. - * Called when `registerStandardEndpoints` is true, before the server starts listening. + * Discovery for this standalone convenience surface. Two rules, both ADR-0076: + * + * **1. Single owner (D11 / OQ#9).** `@objectstack/rest` and the runtime + * dispatcher each serve a real, computed discovery; whichever is on the kernel + * owns `${prefix}/discovery` and we do not register it. Hono is + * first-registration-wins and both of those register during plugin `start()` — + * i.e. before this `kernel:ready` hook — so they already shadowed this handler + * in every composed deployment; ceding cannot change which payload a client + * sees, it just stops us shipping a third one that nobody serves. (The + * dispatcher cedes to REST on the same `hasPlugin` predicate, without probing + * REST's `enableDiscovery`; matching it keeps the three surfaces consistent.) + * + * `/.well-known/objectstack` is ceded to the dispatcher ONLY — REST never + * registers it, so in a REST-without-dispatcher composition this redirect is + * the only thing pointing a `.well-known`-first client at `/discovery`. + * + * **2. Computed, never hardcoded (D12, #4018).** When we do own `/discovery`, + * `routes` is derived from the routes actually registered on this Hono app. + * The table used to be a hardcoded list of every ObjectStack domain — + * `/analytics`, `/workflow`, `/ai`, … — advertised whether or not anything + * mounted them, which is exactly the "advertise a route that 404s" class D12 + * exists to kill: a standalone host with no service plugins advertised the + * whole platform while serving `/data` CRUD and two `/auth/me/*` helpers. + * Both real discovery surfaces compute per service + * (`hasXxx ? route : undefined`); this one computes per registration, which on + * a bare host is the stricter and more honest question — service-registered + * does not imply route-mounted here, because nothing bridges services to HTTP + * on this surface (that bridging IS the dispatcher). */ - private registerDiscoveryAndCrudEndpoints(ctx: PluginContext) { - const rawApp = this.server.getRawApp(); - const prefix = '/api/v1'; + private registerDiscoveryEndpoints(ctx: PluginContext, rawApp: any, prefix: string) { + const kernel = ctx.getKernel() as { hasPlugin?(name: string): boolean } | undefined; + const hasPlugin = (name: string) => + typeof kernel?.hasPlugin === 'function' && kernel.hasPlugin(name); + + if (hasPlugin(RUNTIME_DISPATCHER_PLUGIN)) { + ctx.logger.info( + `/.well-known/objectstack ceded to ${RUNTIME_DISPATCHER_PLUGIN} (single owner)`, + ); + } else { + rawApp.get('/.well-known/objectstack', (c: any) => c.redirect(`${prefix}/discovery`)); + } + + const discoveryOwner = + hasPlugin(REST_API_PLUGIN) ? REST_API_PLUGIN + : hasPlugin(RUNTIME_DISPATCHER_PLUGIN) ? RUNTIME_DISPATCHER_PLUGIN + : undefined; + if (discoveryOwner) { + ctx.logger.info(`${prefix}/discovery ceded to ${discoveryOwner} (single owner)`); + return; + } + + // Built per request, not here: sibling plugins keep registering routes + // through the rest of `kernel:ready`, and the socket only opens on + // `kernel:listening` — so by the time a request can arrive the route table + // is final, while a table snapshotted now would miss every later mount. + rawApp.get(`${prefix}/discovery`, (c: any) => c.json({ data: this.buildDiscovery(prefix) })); - // Build the standard discovery response - const discovery = { + ctx.logger.info('Registered discovery endpoints', { prefix }); + } + + /** The discovery payload served when this surface owns `/discovery`. */ + private buildDiscovery(prefix: string) { + return { version: 'v1', apiName: 'ObjectStack API', - routes: { - data: `${prefix}/data`, - metadata: `${prefix}/meta`, - auth: `${prefix}/auth`, - packages: `${prefix}/packages`, - analytics: `${prefix}/analytics`, - // realtime deliberately absent (ADR-0076 D12, #2462): no - // /realtime HTTP surface is mounted anywhere — advertising - // it here made clients call a route that 404s. - workflow: `${prefix}/workflow`, - automation: `${prefix}/automation`, - ai: `${prefix}/ai`, - notifications: `${prefix}/notifications`, - i18n: `${prefix}/i18n`, - storage: `${prefix}/storage`, - ui: `${prefix}/ui`, - }, + routes: this.advertisableRoutes(prefix), capabilities: { // This standalone Hono surface registers CRUD + auth only (see - // below) — it does NOT mount the cross-object `/batch` route, + // `registerDiscoveryAndCrudEndpoints`) — it does NOT mount the + // cross-object `/batch` route, // which ships with `@objectstack/rest`. `declared === enforced` // (#3298): report `transactionalBatch: false` so a client never // drops its non-atomic fallback against a backend that lacks the @@ -797,12 +875,43 @@ export class HonoServerPlugin implements Plugin { transactionalBatch: { enabled: false }, }, }; + } - // Discovery endpoints - rawApp.get('/.well-known/objectstack', (c: any) => c.redirect(`${prefix}/discovery`)); - rawApp.get(`${prefix}/discovery`, (c: any) => c.json({ data: discovery })); + /** + * `ApiRoutes` computed from the live Hono route table: a family is advertised + * iff some route is registered AT its base path or UNDER it. + * + * `app.routes` is every registration on this app — adapter routes, `getRawApp()` + * routes (how plugin-auth mounts `${basePath}/*`), mounted sub-apps and `use()` + * middleware alike — so this sees what a request will actually hit, not what a + * parallel bookkeeping list believes. Requiring the base or a `/`-separated + * child means a wildcard ABOVE the base (global `/*` middleware, `/api/v1/*`) + * never counts as a mount, while `/api/v1/auth/*` and `/api/v1/data/:object` + * both do — and `/api/v1/me/apps` does not pass for `metadata` (`/api/v1/meta`). + */ + private advertisableRoutes(prefix: string): Partial> { + const app = this.server.getRawApp() as { routes?: Array<{ path?: string }> }; + const registered = Array.isArray(app?.routes) ? app.routes : []; + const routes: Partial> = {}; + for (const [key, segment] of Object.entries(DISCOVERY_ROUTE_SEGMENTS)) { + const base = `${prefix}/${segment}`; + const mounted = registered.some( + (r) => typeof r?.path === 'string' && (r.path === base || r.path.startsWith(`${base}/`)), + ); + if (mounted) routes[key as keyof ApiRoutes] = base; + } + return routes; + } - ctx.logger.info('Registered discovery endpoints', { prefix }); + /** + * Register discovery and basic CRUD endpoints. + * Called when `registerStandardEndpoints` is true, before the server starts listening. + */ + private registerDiscoveryAndCrudEndpoints(ctx: PluginContext) { + const rawApp = this.server.getRawApp(); + const prefix = '/api/v1'; + + this.registerDiscoveryEndpoints(ctx, rawApp, prefix); // ── Anonymous-deny gate (ADR-0056 D2, #2567) ────────────────────────── // These raw `/data/:object` routes delegate straight to ObjectQL. They