diff --git a/.changeset/dispatcher-handler-ready-gate.md b/.changeset/dispatcher-handler-ready-gate.md new file mode 100644 index 0000000000..173a70e7c5 --- /dev/null +++ b/.changeset/dispatcher-handler-ready-gate.md @@ -0,0 +1,15 @@ +--- +'@objectstack/runtime': patch +'@objectstack/metadata-protocol': patch +--- + +Gate every dispatcher service domain on `handlerReady` instead of on slot occupancy (#4058 step 2). + +#4000 made the `/analytics` domain execute ADR-0076 D12's third conclusion ("consumers treat only `handlerReady: true` as a real capability"); every other domain still gated on "is a service registered", so a self-declared stub occupying `automation` / `notification` / `ai` / `file-storage` / `i18n` was called like a real implementation and its fabricated answer went out as a 200. Step 1 (#4082) made the two kinds of dev implementation distinguishable; this is the gate that reads the distinction. + +- The `/analytics`, `/automation`, `/notifications`, `/ai`, `/storage` and `/i18n` domains, the route-mount gate, discovery's `routes`/`features`, and the metadata-protocol builder's route advertisement now share one predicate (`isServiceServeable`): a slot whose occupant self-declares `handlerReady: false` is answered exactly as an empty slot is — the domain's existing 404, or the explicit 501 `/storage` and `/i18n` use. One predicate, so what is advertised and what is served cannot disagree. +- `handlerReady`, not `status`, is the test. An implementation that declares `degraded` defaults to `handlerReady: true` and keeps serving — which is why the in-memory `file-storage` and `i18n` implementations are unaffected. +- `discovery.services.*` stays presence-gated: a registered stub still reports `{ enabled: true, status: 'stub', handlerReady: false }` (with no `route`), which says strictly more than collapsing it to `unavailable` would. +- `/ai` improves for the stub case: an occupied-but-unserveable slot used to fall through to a 503 "AI service routes not yet initialized" and lose the `GET /ai/agents` empty-list answer the console polls for on every navigation. Both are restored. + +No change for a host whose services are real implementations. If you register your own stub under one of those six slots and relied on the dispatcher calling it, either drop the `handlerReady: false` self-declaration (declare `degraded` if it genuinely serves) or install the real service. Not gated, deliberately: `/data`, `/meta`, `/auth` and the security path — their dev stubs back the dev stack's own core loop, and gating them would 404 the dev stack itself. diff --git a/content/docs/kernel/services-checklist.mdx b/content/docs/kernel/services-checklist.mdx index 0f9f3a36d9..40236889f7 100644 --- a/content/docs/kernel/services-checklist.mdx +++ b/content/docs/kernel/services-checklist.mdx @@ -135,6 +135,32 @@ The discovery endpoint returns a `services` map so clients know what is availabl } ``` +#### `stub` vs `degraded` — and what the dispatcher does with each + +A service may self-declare that it is not the full thing (ADR-0076 D12), via a +`__serviceInfo: { status, handlerReady?, message? }` property on the registered +instance — see [Services → presence is not capability](./services.mdx) for how +an in-process caller reads it. The two values mean different things, and for the +HTTP surface the difference is load-bearing (#4058): + +| Declared | `handlerReady` default | Meaning | Dispatcher behaviour | +|:---|:---|:---|:---| +| `degraded` | `true` | Really does the work, with reduced capability (in-memory, no persistence, no cross-process fan-out) | **Served normally.** Route advertised, requests handled. | +| `stub` | `false` | Fabricates its answers — reports success for work that never happened | **Treated as an EMPTY slot.** Route not advertised, request answered exactly as if nothing were registered (404, or the domain's 501). | + +Only `handlerReady` is consulted, never `status` — so an implementation that +declares `degraded` but explicitly sets `handlerReady: false` is also treated as +empty. The rule is enforced by one predicate (`isServiceServeable`) shared by +the service domains, the route-mount gate, and both discovery builders, so what +is advertised and what is served cannot disagree. It applies to the +dispatcher-owned domains: `/analytics`, `/automation`, `/notifications`, `/ai`, +`/storage`, `/i18n`. + +The `services` map still reports a registered stub as +`{ enabled: true, status: "stub", handlerReady: false }` rather than collapsing +it to `unavailable` — "something is in this slot, and it is a fake" says more +than "install a plugin". + --- ## 2. data Service ✅ Implemented @@ -193,6 +219,13 @@ self-declared stub (`handlerReady: false`, ADR-0076 D12) exactly like an empty one — routes unmounted, request 404. To use analytics locally, install the real engine; `@objectstack/service-analytics` runs an InMemory strategy, so it needs no database of its own. + +**And it is no longer analytics-only** (#4058 step 2): every dispatcher-owned +domain now gates on `handlerReady`, so a slot occupied by a self-declared `stub` +— `automation`, `notification`, `ai`, wherever one is registered — answers as an +empty slot does. The implementations that really do the work in memory declare +`degraded` and keep serving. See +[`stub` vs `degraded`](#stub-vs-degraded--and-what-the-dispatcher-does-with-each). ### Features (via `@objectstack/service-analytics`) diff --git a/docs/adr/0076-objectql-core-tiering.md b/docs/adr/0076-objectql-core-tiering.md index 5dca62d51f..f756ccfcef 100644 --- a/docs/adr/0076-objectql-core-tiering.md +++ b/docs/adr/0076-objectql-core-tiering.md @@ -142,6 +142,8 @@ Decision: each capability plugin registers its routes as a **normalized handler* 2. Discovery MUST respect it: such services report **`status: 'stub'` (or `'degraded'`) with `handlerReady: false`**, never `status: 'available'`. Add the status value to the discovery schema. 3. Consumers (agents, console) treat **only `handlerReady: true` / `status: 'available'`** as a real capability; `stub`/`unavailable` ⇒ do not use for real work. *(Update #4000: the **dispatcher is a consumer too**, and was not executing this — every service domain gates on "is the slot occupied", never on `handlerReady`, so a stub in the slot was called like a real implementation. Enforced for `analytics` (`isAnalyticsServiceServeable` in `runtime/src/domains/analytics.ts`, read by the domain, the route-mount gate, and `routes`/`features` in discovery — one predicate, so advertised and served cannot disagree). The other stub-backed domains — `file-storage` / `search` / `automation` / `realtime` / `notification` / `ai` / `i18n` — still gate on presence alone; adopting the same rule across them is a per-domain call, tracked in #4058.)* + *(Update #4058 step 2 — the per-domain call, made, on top of step 1's classification. The rule is now class-wide across the dispatcher-owned domains (`/analytics`, `/automation`, `/notifications`, `/ai`, `/storage`, `/i18n`), reading ONE predicate — `isServiceServeable` in `runtime/src/service-serveable.ts`, which absorbed `isAnalyticsServiceServeable` — from the domain, the route-mount gate, discovery's `routes`/`features`, and the metadata-protocol builder's route advertisement. A slot whose occupant self-declares `handlerReady: false` gets the domain's own empty-slot answer (404, or the explicit 501 `/storage` and `/i18n` use). What makes that safe is step 1: `handlerReady` — not `status` — is the test, and the implementations that really work declare `degraded`, which defaults it to `true`, so `file-storage` / `i18n` keep serving and only the fabricating occupants (`automation` / `notification` / `ai` / `data` / `auth` / `security.*`, `handlerReady: false`) are answered as empty. Deliberately NOT gated: `/data`, `/meta`, `/auth` and the security path, whose dev stubs back the dev stack's own core loop — gating those would 404 the dev stack itself. They are honestly labelled either way; whether the fabricating class should be **registered at all** is the wider evaluation in #4093 (its A tier), where the three `security.*` stubs are also flagged against this ADR's own "a fallback may degrade features, never security semantics". `search` / `realtime` are unaffected: neither has a dispatcher domain, and the surfaceless `degraded` fallbacks (`cache`/`queue`/`job`) carry `handlerReady: false` for the separate D12 reason that no HTTP surface exists for them.)* + 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.)* diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 6e7cdb7c50..02702eb2c7 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -1408,18 +1408,29 @@ export class ObjectStackProtocolImplementation implements data: { enabled: true, status: 'available' as const, route: '/api/v1/data', provider: 'objectql' }, }; - // [#4000] The dispatcher answers a self-declared stub in the `analytics` - // slot with the same 404 an empty slot gets (`isAnalyticsServiceServeable`, - // runtime/src/domains/analytics.ts), so this builder must not advertise a - // route for it — that would be the `declared ≠ enforced` gap discovery - // exists to close. Analytics-only on purpose: every other stub-backed - // slot IS still served by its dispatcher domain, so their route - // advertisement stays presence-gated and honest. Unifying the rule - // across domains is #4058. - const analyticsUnserveable = - readServiceSelfInfo(registeredServices.get('analytics'))?.handlerReady === false; + // [#4000, #4058] The dispatcher answers a self-declared non-handler in + // one of ITS domains' slots with the same 404/501 an empty slot gets + // (`isServiceServeable`, runtime/src/service-serveable.ts), so this + // builder must not advertise a route for one — that would be the + // `declared ≠ enforced` gap discovery exists to close. + // + // Scoped to the dispatcher-owned domains on purpose. For the other + // entries in SERVICE_CONFIG the route belongs to the plugin that + // registers the service (service-storage's own `/api/v1/storage` + // routes, plugin-search, plugin-graphql, …) rather than to a dispatcher + // domain, so `handlerReady` there says nothing about whether THAT route + // is mounted, and suppressing it would be a guess. `file-storage` is + // listed because the dispatcher does own a `/storage` bridge for the + // no-plugin case; when service-storage is installed it registers a real + // (unmarked) service, so the entry never fires. + const DISPATCHER_GATED_SERVICES = new Set([ + 'analytics', 'automation', 'notification', 'ai', 'i18n', 'file-storage', + ]); + const unserveable = (serviceName: string) => + DISPATCHER_GATED_SERVICES.has(serviceName) + && readServiceSelfInfo(registeredServices.get(serviceName))?.handlerReady === false; const advertisedRoute = (serviceName: string, route?: string) => - serviceName === 'analytics' && analyticsUnserveable ? undefined : route; + unserveable(serviceName) ? undefined : route; // Check which services are actually registered for (const [serviceName, config] of Object.entries(SERVICE_CONFIG)) { diff --git a/packages/objectql/src/protocol-discovery.test.ts b/packages/objectql/src/protocol-discovery.test.ts index 8601aead43..59fcc56a75 100644 --- a/packages/objectql/src/protocol-discovery.test.ts +++ b/packages/objectql/src/protocol-discovery.test.ts @@ -194,6 +194,62 @@ describe('ObjectStackProtocolImplementation - Dynamic Service Discovery', () => expect(discovery.routes.analytics).toBeUndefined(); }); + // #4058 — the same rule for every OTHER slot whose HTTP surface is a + // dispatcher domain, now that those domains gate on `handlerReady` too. + it('should not advertise routes for self-declared stubs in the other dispatcher-owned slots', async () => { + const mockServices = new Map(); + for (const name of ['automation', 'notification', 'ai', 'i18n', 'file-storage']) { + mockServices.set(name, { __serviceInfo: { status: 'stub', message: 'dev fake' } }); + } + + protocol = new ObjectStackProtocolImplementation(engine, () => mockServices); + const discovery = await protocol.getDiscovery(); + + for (const name of ['automation', 'notification', 'ai', 'i18n', 'file-storage']) { + expect(discovery.services[name].enabled, `${name}.enabled`).toBe(true); + expect(discovery.services[name].status, `${name}.status`).toBe('stub'); + expect(discovery.services[name].handlerReady, `${name}.handlerReady`).toBe(false); + expect(discovery.services[name].route, `${name}.route`).toBeUndefined(); + } + for (const key of ['automation', 'notifications', 'ai', 'i18n', 'storage']) { + expect(discovery.routes[key], `routes.${key}`).toBeUndefined(); + } + }); + + // The limit of that rule: `degraded` means "really serves, reduced + // capability", so the route stays advertised — plugin-dev's in-memory file + // store and i18n provider are exactly this, and the dispatcher keeps serving + // them. + it('should keep advertising routes for degraded implementations that really serve', async () => { + const mockServices = new Map(); + mockServices.set('file-storage', { __serviceInfo: { status: 'degraded', message: 'in-memory' } }); + mockServices.set('i18n', { __serviceInfo: { status: 'degraded', message: 'in-memory' } }); + + protocol = new ObjectStackProtocolImplementation(engine, () => mockServices); + const discovery = await protocol.getDiscovery(); + + expect(discovery.services['file-storage'].status).toBe('degraded'); + expect(discovery.services['file-storage'].handlerReady).toBe(true); + expect(discovery.routes.storage).toBe('/api/v1/storage'); + expect(discovery.routes.i18n).toBe('/api/v1/i18n'); + }); + + // Not every SERVICE_CONFIG entry is dispatcher-owned: `search` (REST layer), + // `workflow`, `graphql` and the queue/job/cache families have their routes + // mounted by the plugin that registers the service, so `handlerReady` there + // says nothing about whether THAT route is mounted and the advertisement + // stays presence-gated. Suppressing it would be a guess, not honesty. + it('should leave non-dispatcher-owned routes presence-gated', async () => { + const mockServices = new Map(); + mockServices.set('search', { __serviceInfo: { status: 'stub', message: 'dev fake' } }); + + protocol = new ObjectStackProtocolImplementation(engine, () => mockServices); + const discovery = await protocol.getDiscovery(); + + expect(discovery.services.search.status).toBe('stub'); + expect(discovery.services.search.route).toBe('/api/v1/search'); + }); + it('should map file-storage service to storage route', async () => { const mockServices = new Map(); mockServices.set('file-storage', {}); diff --git a/packages/runtime/src/dispatcher-plugin.ts b/packages/runtime/src/dispatcher-plugin.ts index e10d44e544..b1b0ce64db 100644 --- a/packages/runtime/src/dispatcher-plugin.ts +++ b/packages/runtime/src/dispatcher-plugin.ts @@ -4,7 +4,7 @@ import { Plugin, PluginContext, IHttpServer } from '@objectstack/core'; import { looksLikeInternalErrorLeak, INTERNAL_ERROR_MESSAGE } from '@objectstack/types'; import { DispatcherErrorCode } from '@objectstack/spec/api'; import { HttpDispatcher, HttpDispatcherResult } from './http-dispatcher.js'; -import { isAnalyticsServiceServeable } from './domains/analytics.js'; +import { isServiceServeable } from './service-serveable.js'; import { validationFailureDetails, VALIDATION_FAILED_STATUS } from './validation-failure.js'; import { buildApiError } from './error-envelope.js'; import { @@ -665,7 +665,7 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu svc = await k.getServiceAsync('analytics').catch(() => undefined); } if (!svc) svc = k?.getService?.('analytics'); - return isAnalyticsServiceServeable(svc); + return isServiceServeable(svc); } catch { return false; } diff --git a/packages/runtime/src/domains/ai.ts b/packages/runtime/src/domains/ai.ts index 674061b5fa..e6989b0cb0 100644 --- a/packages/runtime/src/domains/ai.ts +++ b/packages/runtime/src/domains/ai.ts @@ -13,6 +13,7 @@ import { shouldDenyAnonymous, ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_MESSAGE, } from '@objectstack/core'; +import { isServiceServeable } from '../service-serveable.js'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; @@ -36,7 +37,13 @@ export async function handleAIRequest(deps: DomainHandlerDeps, subPath: string, // AI service not registered } - if (!aiService) { + // [#4058] A slot filled by a self-declared non-handler (`handlerReady: + // false`, ADR-0076 D12) is no AI capability, so it takes the same exits an + // empty slot takes. This is also strictly better than what such an occupant + // used to get: being truthy, it fell through to the `!routes` 503 below — + // which both reads as a fault and loses the `/ai/agents` empty-list + // courtesy the console depends on. + if (!isServiceServeable(aiService)) { // The console polls `GET /ai/agents` on every navigation to decide // whether to show AI affordances. Reporting that as a 404 turns the // normal "no AI service configured" state (the open-source default — diff --git a/packages/runtime/src/domains/analytics.ts b/packages/runtime/src/domains/analytics.ts index 90366392f7..929077c35b 100644 --- a/packages/runtime/src/domains/analytics.ts +++ b/packages/runtime/src/domains/analytics.ts @@ -8,11 +8,13 @@ * ExecutionContext and the contract `where` filter). Route registration stays * dispatcher-owned so the URL contract is stable regardless of what occupies * the slot; an empty slot answers the `handled: false` 404 below — as does a - * slot occupied by a self-declared stub (#4000, see {@link isAnalyticsServiceServeable}). + * slot occupied by a self-declared stub (#4000, see {@link isServiceServeable}, + * the shared predicate every service domain reads since #4058). */ import { CoreServiceName } from '@objectstack/spec/system'; -import { AnalyticsQueryRequestSchema, readServiceSelfInfo } from '@objectstack/spec/api'; +import { AnalyticsQueryRequestSchema } from '@objectstack/spec/api'; +import { isServiceServeable } from '../service-serveable.js'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; @@ -71,28 +73,6 @@ function assertAnalyticsQueryBody(body: unknown): void { } } -/** - * [#4000] Is the thing occupying the `analytics` slot something we may call - * like a real engine? - * - * ADR-0076 D12 gave services a way to self-declare that they are a stub or a - * degraded fallback, and its third conclusion binds *consumers*: only - * `handlerReady: true` counts as a real capability. Discovery honoured that - * from day one; the dispatcher — a consumer too — gated on mere presence, so - * anything registered in the slot got called and its fabricated rows went back - * as a 200. That is precisely the shape #3891 retired one layer up, and - * plugin-dev's analytics stub kept it alive in dev mode (retired with this - * change; this predicate is what stops the next one). - * - * `handlerReady` (not `status`) is the right test: a `degraded` implementation - * that genuinely serves requests defaults to `true` and keeps serving — only a - * self-confessed non-handler is treated as an empty slot. - */ -export function isAnalyticsServiceServeable(svc: unknown): boolean { - if (!svc) return false; - return readServiceSelfInfo(svc)?.handlerReady !== false; -} - export function createAnalyticsDomain(deps: DomainHandlerDeps): DomainRoute { return { prefix: '/analytics', @@ -113,7 +93,7 @@ export async function handleAnalyticsRequest( const analyticsService = await deps.getService(CoreServiceName.enum.analytics); // Empty slot — or a slot filled by a self-declared stub (#4000), which is // the same amount of analytics capability. 404 handled by caller. - if (!isAnalyticsServiceServeable(analyticsService)) return { handled: false }; + if (!isServiceServeable(analyticsService)) return { handled: false }; const m = method.toUpperCase(); const subPath = path.replace(/^\/+/, ''); diff --git a/packages/runtime/src/domains/automation.ts b/packages/runtime/src/domains/automation.ts index 1527754f6a..2091900dff 100644 --- a/packages/runtime/src/domains/automation.ts +++ b/packages/runtime/src/domains/automation.ts @@ -10,6 +10,7 @@ */ import { CoreServiceName } from '@objectstack/spec/system'; +import { isServiceServeable } from '../service-serveable.js'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; @@ -42,7 +43,13 @@ export function createAutomationDomain(deps: DomainHandlerDeps): DomainRoute { */ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: string, method: string, body: any, context: HttpProtocolContext, query?: any): Promise { const automationService = await deps.getService(CoreServiceName.enum.automation); - if (!automationService) return { handled: false }; + // [#4058] Empty slot — or a slot filled by a self-declared non-handler + // (`handlerReady: false`, ADR-0076 D12), which is the same amount of + // automation capability. This domain is the sharpest case for the rule: a + // stub whose `execute` returns `{ success: true }` without running anything + // answered 200, so a caller (or an agent) read "flow executed" off a flow + // that never ran. 404 handled by caller. + if (!isServiceServeable(automationService)) return { handled: false }; const m = method.toUpperCase(); const parts = path.replace(/^\/+/, '').split('/').filter(Boolean); diff --git a/packages/runtime/src/domains/i18n.ts b/packages/runtime/src/domains/i18n.ts index 2a0b1e1117..73c4f7cc7a 100644 --- a/packages/runtime/src/domains/i18n.ts +++ b/packages/runtime/src/domains/i18n.ts @@ -18,6 +18,7 @@ import { resolveLocale } from '@objectstack/core'; import { CoreServiceName, resolveObjectFieldLabels, toLocaleDescriptors } from '@objectstack/spec/system'; +import { isServiceServeable } from '../service-serveable.js'; import type { TranslationData } from '@objectstack/spec/system'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; @@ -39,7 +40,13 @@ export async function handleI18nRequest( _context: HttpProtocolContext, ): Promise { const i18nService = await deps.getService(CoreServiceName.enum.i18n); - if (!i18nService) return { handled: true, response: deps.error('i18n service not available', 501) }; + // [#4058] An empty slot and a slot filled by a self-declared non-handler + // (`handlerReady: false`, ADR-0076 D12) are the same amount of i18n. Both + // in-memory providers of this slot really translate, so both declare + // `degraded` (#4058 step 1 — `createMemoryI18n`, which plugin-dev also + // wraps) and `handlerReady` defaults to `true` for them: they keep serving. + // This gate is for an occupant that would answer with invented strings. + if (!isServiceServeable(i18nService)) return { handled: true, response: deps.error('i18n service not available', 501) }; const m = method.toUpperCase(); const parts = path.replace(/^\/+/, '').split('/').filter(Boolean); diff --git a/packages/runtime/src/domains/notifications.ts b/packages/runtime/src/domains/notifications.ts index 90a36c45ca..719ba6d352 100644 --- a/packages/runtime/src/domains/notifications.ts +++ b/packages/runtime/src/domains/notifications.ts @@ -20,6 +20,7 @@ */ import { CoreServiceName } from '@objectstack/spec/system'; +import { isServiceServeable } from '../service-serveable.js'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; @@ -41,7 +42,14 @@ export async function handleNotificationRequest( context: HttpProtocolContext, ): Promise { const service = await deps.resolveService(CoreServiceName.enum.notification, context.environmentId) as any; - if (!service || typeof service.listInbox !== 'function') return { handled: false }; + // [#4058] Three ways to have no inbox capability, one answer. The + // `listInbox` duck-type was already here and, by accident, kept the known + // fabricating stub off this surface (the dev one implements `send` / + // `sendBatch` only). `isServiceServeable` makes that explicit rather than + // incidental: a self-declared non-handler (`handlerReady: false`, ADR-0076 + // D12) is an empty slot even if it grows a `listInbox` later. A `degraded` + // inbox that really serves keeps serving. + if (!isServiceServeable(service) || typeof service.listInbox !== 'function') return { handled: false }; const userId: string | undefined = context.executionContext?.userId; if (!userId) { diff --git a/packages/runtime/src/domains/storage.ts b/packages/runtime/src/domains/storage.ts index 9a8c9c2207..ef7484290f 100644 --- a/packages/runtime/src/domains/storage.ts +++ b/packages/runtime/src/domains/storage.ts @@ -12,6 +12,7 @@ */ import { CoreServiceName } from '@objectstack/spec/system'; +import { isServiceServeable } from '../service-serveable.js'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; @@ -36,7 +37,13 @@ export async function handleStorageRequest( // already ends at the services map (and when `services` is a Map, the // legacy index access returned undefined anyway), so it is dropped here. const storageService = await deps.getService(CoreServiceName.enum['file-storage']); - if (!storageService) { + // [#4058] Same 501 for an empty slot and for a slot filled by a + // self-declared non-handler (`handlerReady: false`, ADR-0076 D12) — "not + // configured" is the honest answer to both. NOT aimed at the in-memory dev + // file store: that one really stores and really reads back, so it declares + // `degraded` (`handlerReady: true`) and still serves here. The gate exists + // for the occupant that only *claims* to store. + if (!isServiceServeable(storageService)) { return { handled: true, response: deps.error('File storage not configured', 501) }; } diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 196062859d..75bb234dff 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -2389,6 +2389,197 @@ describe('HttpDispatcher', () => { }); }); + // ═══════════════════════════════════════════════════════════════ + // [#4058] handlerReady across the REST of the service domains + // + // #4000 executed ADR-0076 D12 conclusion 3 for `analytics` alone; the other + // domains kept gating on "is the slot occupied", so a self-declared stub in + // any of them was still called like a real implementation. These pin the + // class-wide rule and, just as importantly, its limit: `degraded` — an + // implementation that really serves with reduced capability — is NOT + // affected. A single predicate decides both (`service-serveable.ts`), so the + // route/feature advertisement and the handler cannot disagree. + // ═══════════════════════════════════════════════════════════════ + + describe('handlerReady gating across service domains (D12 / #4058)', () => { + /** A fake that fabricates: every method answers, none does the work. */ + const stubbed = (methods: Record) => ({ + __serviceInfo: { status: 'stub', message: 'dev fake' }, + ...methods, + }); + /** A fake that really does the work in memory — `handlerReady` defaults true. */ + const degraded = (methods: Record) => ({ + __serviceInfo: { status: 'degraded', message: 'in-memory' }, + ...methods, + }); + + const serveOnly = (name: string, svc: unknown) => { + (kernel as any).getService = vi.fn().mockImplementation((n: string) => (n === name ? svc : null)); + (kernel as any).services = new Map([[name, svc]]); + }; + + // The sharpest case in the class: `execute` reported `{ success: true }` + // for a flow that never ran, and the domain served it as a 200 — a + // caller (or an agent) read "flow executed" off nothing happening. + it('/automation — a stub slot is an empty slot, and is never called', async () => { + const stub = stubbed({ + execute: vi.fn().mockResolvedValue({ success: true, output: undefined, durationMs: 0 }), + trigger: vi.fn().mockResolvedValue({ success: true }), + listFlows: vi.fn().mockResolvedValue([]), + registerFlow: vi.fn(), + }); + serveOnly('automation', stub); + + for (const [path, method] of [['', 'GET'], ['', 'POST'], ['trigger/x', 'POST'], ['x/trigger', 'POST']] as const) { + const result = await dispatcher.handleAutomation(path, method, { name: 'x' }, { request: {} }); + expect(result.handled, `${method} /automation/${path}`).toBe(false); + } + expect(stub.execute).not.toHaveBeenCalled(); + expect(stub.trigger).not.toHaveBeenCalled(); + expect(stub.listFlows).not.toHaveBeenCalled(); + expect(stub.registerFlow).not.toHaveBeenCalled(); + }); + + it('/automation — a degraded engine keeps serving', async () => { + const svc = degraded({ listFlows: vi.fn().mockResolvedValue(['flow_a']) }); + serveOnly('automation', svc); + + const result = await dispatcher.handleAutomation('', 'GET', {}, { request: {} }); + expect(result.handled).toBe(true); + expect(result.response?.body?.data?.flows).toEqual(['flow_a']); + }); + + // Storage answers 501 for an empty slot rather than `handled: false`, so + // "same answer as an empty slot" means the same 501 here. + it('/storage — a stub slot answers the not-configured 501 without being called', async () => { + const stub = stubbed({ upload: vi.fn(), download: vi.fn() }); + serveOnly('file-storage', stub); + + const result = await dispatcher.handleStorage('upload', 'POST', { name: 'f.txt' }, { request: {} }); + expect(result.handled).toBe(true); + expect(result.response?.status).toBe(501); + expect(stub.upload).not.toHaveBeenCalled(); + }); + + // The half that protects plugin-dev's in-memory file store: it really + // stores and really reads back, so it declares `degraded` and serves. + it('/storage — a degraded in-memory store keeps serving', async () => { + const svc = degraded({ upload: vi.fn().mockResolvedValue({ key: 'f.txt' }) }); + serveOnly('file-storage', svc); + + const result = await dispatcher.handleStorage('upload', 'POST', { name: 'f.txt' }, { request: {} }); + expect(result.handled).toBe(true); + expect(result.response?.status).toBe(200); + expect(svc.upload).toHaveBeenCalled(); + }); + + it('/i18n — a stub slot answers the not-available 501; a degraded provider serves', async () => { + const stub = stubbed({ getLocales: vi.fn().mockReturnValue(['xx']) }); + serveOnly('i18n', stub); + const stubResult = await dispatcher.handleI18n('/locales', 'GET', {}, { request: {} }); + expect(stubResult.response?.status).toBe(501); + expect(stub.getLocales).not.toHaveBeenCalled(); + + const svc = degraded({ + getLocales: vi.fn().mockReturnValue(['en']), + getDefaultLocale: vi.fn().mockReturnValue('en'), + }); + serveOnly('i18n', svc); + const okResult = await dispatcher.handleI18n('/locales', 'GET', {}, { request: {} }); + expect(okResult.response?.status).toBe(200); + expect(svc.getLocales).toHaveBeenCalled(); + }); + + // The inbox surface was already protected by accident — the `listInbox` + // duck-type kept `send`-only fakes out. Now it is protected on purpose, + // so a stub that grows a `listInbox` stays out too. + it('/notifications — a stub slot is an empty slot even when it implements listInbox', async () => { + const stub = stubbed({ + listInbox: vi.fn().mockResolvedValue({ messages: [] }), + send: vi.fn().mockResolvedValue({ success: true, messageId: 'x' }), + }); + serveOnly('notification', stub); + + const result = await dispatcher.handleNotification('', 'GET', undefined, {}, { + request: {}, executionContext: { userId: 'usr_1' }, + } as any); + expect(result.handled).toBe(false); + expect(stub.listInbox).not.toHaveBeenCalled(); + }); + + // Being truthy, a stub used to fall through to the `!routes` 503 — which + // reads as a fault AND loses the empty-list courtesy the console's + // per-navigation `GET /ai/agents` poll depends on. Treating it as an + // empty slot restores both. + it('/ai — a stub slot 404s per route and keeps the /ai/agents empty list', async () => { + const stub = stubbed({ chat: vi.fn(), listModels: vi.fn() }); + serveOnly('ai', stub); + + const chat = await dispatcher.handleAI('/ai/chat', 'POST', { messages: [] }, {}, { request: {} }); + expect(chat.handled).toBe(true); + expect(chat.response?.status).toBe(404); + expect(stub.chat).not.toHaveBeenCalled(); + + const agents = await dispatcher.handleAI('/ai/agents', 'GET', undefined, {}, { request: {} }); + expect(agents.handled).toBe(true); + expect(agents.response?.status).toBe(200); + expect(agents.response?.body).toEqual({ agents: [] }); + }); + + // Discovery must say exactly what the domains do — one predicate feeds + // both. `services.*` deliberately stays presence-gated: a registered + // stub self-reporting `status: 'stub'` says strictly more than + // `unavailable` / "install a plugin" would. + it('stops advertising routes/features for stub slots, while still reporting them as stubs', async () => { + const stubs: Record = { + 'file-storage': stubbed({ upload: vi.fn() }), + automation: stubbed({ execute: vi.fn() }), + notification: stubbed({ send: vi.fn() }), + ai: stubbed({ chat: vi.fn() }), + i18n: stubbed({ getLocales: vi.fn().mockReturnValue([]) }), + }; + (kernel as any).getService = vi.fn().mockImplementation((n: string) => stubs[n] ?? null); + (kernel as any).services = new Map(Object.entries(stubs)); + + const info = await dispatcher.getDiscoveryInfo('/api/v1'); + for (const key of ['storage', 'automation', 'notifications', 'ai', 'i18n'] as const) { + expect(info.routes[key], `routes.${key}`).toBeUndefined(); + } + for (const key of ['files', 'ai', 'notifications', 'i18n'] as const) { + expect(info.features[key], `features.${key}`).toBe(false); + } + for (const key of ['file-storage', 'automation', 'notification', 'ai', 'i18n'] as const) { + expect(info.services[key].enabled, `services.${key}.enabled`).toBe(true); + expect(info.services[key].status, `services.${key}.status`).toBe('stub'); + expect(info.services[key].handlerReady, `services.${key}.handlerReady`).toBe(false); + expect(info.services[key].route, `services.${key}.route`).toBeUndefined(); + } + }); + + it('keeps advertising routes/features for degraded slots that really serve', async () => { + const degradeds: Record = { + 'file-storage': degraded({ upload: vi.fn() }), + automation: degraded({ listFlows: vi.fn() }), + notification: degraded({ listInbox: vi.fn() }), + ai: degraded({ chat: vi.fn() }), + i18n: degraded({ getLocales: vi.fn().mockReturnValue(['en']), getDefaultLocale: vi.fn().mockReturnValue('en') }), + }; + (kernel as any).getService = vi.fn().mockImplementation((n: string) => degradeds[n] ?? null); + (kernel as any).services = new Map(Object.entries(degradeds)); + + const info = await dispatcher.getDiscoveryInfo('/api/v1'); + expect(info.routes.storage).toBe('/api/v1/storage'); + expect(info.routes.automation).toBe('/api/v1/automation'); + expect(info.routes.notifications).toBe('/api/v1/notifications'); + expect(info.routes.ai).toBe('/api/v1/ai'); + expect(info.routes.i18n).toBe('/api/v1/i18n'); + expect(info.features.files).toBe(true); + expect(info.features.i18n).toBe(true); + expect(info.services['file-storage'].status).toBe('degraded'); + expect(info.services['file-storage'].handlerReady).toBe(true); + }); + }); + // ═══════════════════════════════════════════════════════════════ // i18n across server/dev/mock environments // ═══════════════════════════════════════════════════════════════ diff --git a/packages/runtime/src/http-dispatcher.ts b/packages/runtime/src/http-dispatcher.ts index f3d0267a91..6386b4931e 100644 --- a/packages/runtime/src/http-dispatcher.ts +++ b/packages/runtime/src/http-dispatcher.ts @@ -11,7 +11,8 @@ import { apiErrorResponse } from './error-envelope.js'; import type { ExecutionContext } from '@objectstack/spec/kernel'; import { DomainHandlerRegistry, type DomainRoute, type DomainHandlerDeps } from './domain-handler-registry.js'; import * as actionExec from './action-execution.js'; -import { createAnalyticsDomain, handleAnalyticsRequest, isAnalyticsServiceServeable } from './domains/analytics.js'; +import { createAnalyticsDomain, handleAnalyticsRequest } from './domains/analytics.js'; +import { isServiceServeable } from './service-serveable.js'; import { createI18nDomain, handleI18nRequest } from './domains/i18n.js'; import { createNotificationsDomain, handleNotificationRequest } from './domains/notifications.js'; import { createSecurityDomain, handleSecurityRequest } from './domains/security.js'; @@ -863,26 +864,37 @@ export class HttpDispatcher { const hasAuth = !!authSvc; const hasSearch = !!searchSvc; - const hasFiles = !!filesSvc; - // [#4000] Mirrors the analytics domain's OWN guard - // (`isAnalyticsServiceServeable`, domains/analytics.ts): a slot filled - // by a self-declared stub takes the same `handled:false` → 404 an empty - // slot takes, so advertising the route on mere presence would promise an - // API that can only 404 — the `declared ≠ enforced` failure `hasMcp` - // below refuses by construction. Same predicate ⇒ same answer. + // [#4000, #4058] Every service whose HTTP surface is a DISPATCHER DOMAIN + // mirrors that domain's OWN guard (`isServiceServeable`, + // service-serveable.ts): a slot filled by a self-declared non-handler + // takes the same 404/501 an empty slot takes, so advertising the route + // on mere presence would promise an API that can only fail — the + // `declared ≠ enforced` failure `hasMcp` below refuses by construction. + // Same predicate ⇒ same answer. #4000 did this for `analytics` alone; + // #4058 extended it to the rest of the dispatcher-owned domains. // - // `analyticsRegistered` stays presence-only, for the services map alone: - // a registered stub is reported there as `status: 'stub', - // handlerReady: false` (D12's honest self-report), which says strictly - // more than collapsing it to `unavailable` / "install a plugin" would. - const analyticsRegistered = !!analyticsSvc; - const hasAnalytics = isAnalyticsServiceServeable(analyticsSvc); + // A `degraded` implementation keeps its route: `handlerReady` defaults + // to `true` for it and its domain keeps serving it (that is the whole + // point of the `stub` / `degraded` split — see plugin-dev's markers). + // + // `*Registered` stays presence-only, for the `services` map alone: a + // registered stub is reported there as `status: 'stub', handlerReady: + // false` (D12's honest self-report), which says strictly more than + // collapsing it to `unavailable` / "install a plugin" would. + const analyticsRegistered = !!analyticsSvc; + const filesRegistered = !!filesSvc; + const aiRegistered = !!aiSvc; + const notificationRegistered = !!notificationSvc; + const i18nRegistered = !!i18nSvc; + const automationRegistered = !!automationSvc; + const hasFiles = isServiceServeable(filesSvc); + const hasAnalytics = isServiceServeable(analyticsSvc); const hasWorkflow = !!workflowSvc; - const hasAi = !!aiSvc; - const hasNotification = !!notificationSvc; - const hasI18n = !!i18nSvc; + const hasAi = isServiceServeable(aiSvc); + const hasNotification = isServiceServeable(notificationSvc); + const hasI18n = isServiceServeable(i18nSvc); const hasUi = !!uiSvc; - const hasAutomation = !!automationSvc; + const hasAutomation = isServiceServeable(automationSvc); const hasCache = !!cacheSvc; const hasQueue = !!queueSvc; const hasJob = !!jobSvc; @@ -998,10 +1010,12 @@ export class HttpDispatcher { data: svcAvailable(routes.data, 'kernel'), // Plugin-provided — only available when a plugin registers the service auth: hasAuth ? svcAvailable(routes.auth, undefined, authSvc) : svcUnavailable('auth'), - automation: hasAutomation ? svcAvailable(routes.automation, undefined, automationSvc) : svcUnavailable('automation'), - // [#4000] Presence-gated, not serveability-gated: a registered - // stub self-reports as `stub` / `handlerReady: false` here (and - // carries no `route`, since `routes.analytics` is undefined for it). + // [#4000, #4058] Presence-gated, not serveability-gated: a + // registered stub self-reports as `stub` / `handlerReady: false` + // here (and carries no `route`, since the matching `routes.*` + // entry is undefined for it). Collapsing it to `unavailable` / + // "install a plugin" would say strictly less. + automation: automationRegistered ? svcAvailable(routes.automation, undefined, automationSvc) : svcUnavailable('automation'), analytics: analyticsRegistered ? svcAvailable(routes.analytics, undefined, analyticsSvc) : svcUnavailable('analytics'), cache: hasCache ? svcAvailable(undefined, undefined, cacheSvc) : svcUnavailable('cache'), queue: hasQueue ? svcAvailable(undefined, undefined, queueSvc) : svcUnavailable('queue'), @@ -1020,10 +1034,11 @@ export class HttpDispatcher { message: realtimeSelf?.message ?? 'In-process event bus only — no HTTP/WS realtime surface is mounted', } : svcUnavailable('realtime'), - notification: hasNotification ? svcAvailable(routes.notifications, undefined, notificationSvc) : svcUnavailable('notification'), - ai: hasAi ? svcAvailable(routes.ai, undefined, aiSvc) : svcUnavailable('ai'), - i18n: hasI18n ? svcAvailable(routes.i18n, undefined, i18nSvc) : svcUnavailable('i18n'), - 'file-storage': hasFiles ? svcAvailable(routes.storage, undefined, filesSvc) : svcUnavailable('file-storage'), + // Presence-gated for the same reason `analytics` is (#4058). + notification: notificationRegistered ? svcAvailable(routes.notifications, undefined, notificationSvc) : svcUnavailable('notification'), + ai: aiRegistered ? svcAvailable(routes.ai, undefined, aiSvc) : svcUnavailable('ai'), + i18n: i18nRegistered ? svcAvailable(routes.i18n, undefined, i18nSvc) : svcUnavailable('i18n'), + 'file-storage': filesRegistered ? svcAvailable(routes.storage, undefined, filesSvc) : svcUnavailable('file-storage'), search: hasSearch ? svcAvailable(undefined, undefined, searchSvc) : svcUnavailable('search'), }, locale, diff --git a/packages/runtime/src/service-serveable.ts b/packages/runtime/src/service-serveable.ts new file mode 100644 index 0000000000..5d0613ae26 --- /dev/null +++ b/packages/runtime/src/service-serveable.ts @@ -0,0 +1,42 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ONE predicate for "may the dispatcher call the thing in this service slot + * like a real implementation?" — ADR-0076 D12 conclusion 3, applied across the + * service domains (#4058). + * + * D12's third conclusion binds *consumers*: only `handlerReady: true` counts as + * a real capability. Discovery honoured that from #3028 on; the dispatcher — a + * consumer too — gated every domain on mere slot occupancy, so anything + * registered under a canonical name got called and its fabricated results went + * back as a 200. #4000 enforced the rule for `analytics` alone + * (`isAnalyticsServiceServeable`, since folded into this module) because the + * class-wide adoption needed a per-domain decision; #4058 made it and this is + * where the answer lives. + * + * `handlerReady` (not `status`) is the test, and that distinction is what makes + * the rule safe to apply everywhere: an implementation that self-declares + * `degraded` genuinely serves requests — `handlerReady` defaults to `true` for + * it — and keeps serving. Only a self-confessed non-handler (`handlerReady: + * false`, which is the default for `status: 'stub'`) is answered as if the slot + * were empty. A working in-memory fallback is therefore NOT collateral damage: + * it is `degraded`, and it stays on the wire. + * + * Read by the service domains, the route-mount gate, and discovery's + * `routes`/`features` — one predicate, so what is advertised and what is served + * cannot disagree. + */ + +import { readServiceSelfInfo } from '@objectstack/spec/api'; + +/** + * True when `svc` occupies its slot AND does not self-declare as a non-handler. + * + * An empty slot and a slot filled by a self-declared stub are the same amount + * of capability, so callers answer both the same way — whatever their empty-slot + * exit already is (`handled: false` → 404, or an explicit 501). + */ +export function isServiceServeable(svc: unknown): boolean { + if (!svc) return false; + return readServiceSelfInfo(svc)?.handlerReady !== false; +}