From 0a9f297ea7143067f306c7a2312ff42668b9ea1e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 07:31:34 +0000 Subject: [PATCH] fix(plugin-dev,runtime): retire the dev analytics stub; /analytics gates on handlerReady, not presence (#4000) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #3891/#3989 retired the degraded analytics shim, making an empty `analytics` slot the honest signal: routes unmounted, request 404, discovery `unavailable`. plugin-dev refilled that slot in dev mode and the retired shape came back one layer down — the dispatcher gates on "is a service registered", so the stub was called like a real engine and its fabricated rows went out as a 200. Dev mode is explicit opt-in and the fake was empty, so this was never the security hole the shim was; the defect is that "the capability is present" meant something different in dev than in production. Both halves are fixed, because deleting the stub alone would leave the gate that let it through: - plugin-dev no longer registers an `analytics` dev stub (`NO_DEV_STUB_SERVICES` — the slot stays empty, not even the shapeless `{ _dev: true }` fallback). Every other dev stub is unchanged. - ADR-0076 D12's third conclusion ("consumers treat only `handlerReady: true` as a real capability") is now executed by the dispatcher, which is a consumer too. `isAnalyticsServiceServeable` is read by the `/analytics` domain, the route-mount gate, and discovery's `routes`/`features` — one predicate, so what is advertised and what is served cannot disagree. `handlerReady`, not `status`, is the test: a `degraded` implementation that genuinely serves keeps serving. - The metadata-protocol discovery builder stops advertising `routes.analytics` (and the service entry's `route`) for a stub, for the same reason. Analytics only: the other stub-backed domains ARE still served by their dispatcher domains, so their presence-gated advertisement stays honest. - `discovery.services.analytics` still reports a registered stub as `status: 'stub', handlerReady: false` — strictly more informative than collapsing it to `unavailable`. To use analytics in dev, install the real engine: @objectstack/service-analytics runs an InMemory strategy. `os serve`/`os dev` are unaffected — `analytics` is in ALWAYS_ON_CAPABILITIES, so the real engine is already installed there. Out of scope, filed as #4058: the other dispatcher domains (storage, automation, i18n, notifications, ai) still gate on presence alone, and plugin-dev's remaining stubs still occupy those slots. Adopting the same rule there is a per-domain call — some of those stubs are working in-memory implementations (`degraded`), not fabricated data (`stub`), and the single `_dev: true` marker cannot tell them apart. ADR-0076 D12 annotated accordingly. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UvcnzFr5XBuvvAd7mosG54 --- .changeset/retire-dev-analytics-stub.md | 13 ++++ content/docs/kernel/services-checklist.mdx | 7 ++ docs/adr/0076-objectql-core-tiering.md | 4 +- packages/metadata-protocol/src/protocol.ts | 20 +++++- .../objectql/src/protocol-discovery.test.ts | 19 ++++++ .../plugins/plugin-dev/src/dev-plugin.test.ts | 16 +++-- packages/plugins/plugin-dev/src/dev-plugin.ts | 31 ++++++--- .../src/dispatcher-plugin.routes.test.ts | 25 +++++++ packages/runtime/src/dispatcher-plugin.ts | 19 ++++-- packages/runtime/src/domains/analytics.ts | 31 ++++++++- packages/runtime/src/http-dispatcher.test.ts | 67 +++++++++++++++++++ packages/runtime/src/http-dispatcher.ts | 21 +++++- 12 files changed, 240 insertions(+), 33 deletions(-) create mode 100644 .changeset/retire-dev-analytics-stub.md diff --git a/.changeset/retire-dev-analytics-stub.md b/.changeset/retire-dev-analytics-stub.md new file mode 100644 index 0000000000..3a79af0483 --- /dev/null +++ b/.changeset/retire-dev-analytics-stub.md @@ -0,0 +1,13 @@ +--- +'@objectstack/plugin-dev': patch +'@objectstack/runtime': patch +--- + +Retire the dev-mode `analytics` stub, and make the dispatcher gate `/analytics` on `handlerReady` rather than on service presence (#4000). + +Retiring the degraded analytics shim (#3891) made an empty `analytics` slot the honest signal: `/api/v1/analytics/*` 404s and discovery reports `unavailable`. `plugin-dev` refilled that slot with a stub, which re-created the retired shape in dev mode — the dispatcher gated on "is a service registered", so the stub was called like a real engine and its empty result came back as a 200. + +- `plugin-dev` no longer registers an `analytics` dev stub; the slot stays empty (`NO_DEV_STUB_SERVICES`). Every other dev stub is unchanged. +- The `/analytics` domain, its route-mount gate, and discovery's `routes`/`features` now share one predicate (`isAnalyticsServiceServeable`): a service that self-declares `handlerReady: false` (ADR-0076 D12 — `__serviceInfo`, or plugin-dev's legacy `_dev: true`) is treated as an empty slot. A `degraded` implementation that genuinely serves requests keeps serving; `discovery.services.analytics` still reports a registered stub as `status: 'stub'`, which says more than `unavailable` would. + +FROM → TO for dev setups that relied on the stub answering `POST /api/v1/analytics/query` with `{ rows: [], fields: [] }`: install the real engine — `@objectstack/service-analytics` runs an InMemory strategy and needs no database of its own. Nothing else changes; hosts that already install it (including `os serve`, where `analytics` is in `ALWAYS_ON_CAPABILITIES`) are unaffected. diff --git a/content/docs/kernel/services-checklist.mdx b/content/docs/kernel/services-checklist.mdx index 15f1128dff..0f9f3a36d9 100644 --- a/content/docs/kernel/services-checklist.mdx +++ b/content/docs/kernel/services-checklist.mdx @@ -186,6 +186,13 @@ caller's `ExecutionContext` — aggregates ran without RLS/tenant scoping — an silently ignored the contract `where` filter. Without `@objectstack/service-analytics`, `/api/v1/analytics/*` now answers **404** and discovery reports `analytics: { enabled: false, status: "unavailable" }`. + +**This holds in dev mode too** (#4000): `plugin-dev` no longer registers an +`analytics` dev stub, and the dispatcher treats a slot filled by any +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. ### Features (via `@objectstack/service-analytics`) diff --git a/docs/adr/0076-objectql-core-tiering.md b/docs/adr/0076-objectql-core-tiering.md index 626763222a..c81bf097c0 100644 --- a/docs/adr/0076-objectql-core-tiering.md +++ b/docs/adr/0076-objectql-core-tiering.md @@ -132,14 +132,14 @@ Decision: each capability plugin registers its routes as a **normalized handler* **Root cause of agents being misled.** Several plugins register stub / dev / fallback services under canonical names, and the discovery builder reports *any* present service as fully real: `runtime/http-dispatcher.ts`'s `svcAvailable` hardcodes `{ enabled: true, status: 'available', handlerReady: true }` for every registered service — it **ignores stub markers** (its own comment even says "handlerReady:false … may be served by a stub", but the code never computes it). So `discovery.services.*` claims capabilities that are only stubbed, and consumers (AI agents, the console) trust them. A dev AI stub advertised this way has already confused an agent. **Inventory of current fakes / mis-reports:** -- `plugin-dev` registers ~8 dev stubs — `storage` / `search` / `automation` / `graphql` / `analytics` / `realtime` / `notification` / `ai` (they already carry a `_dev: true` marker that nothing respects). +- `plugin-dev` registers ~8 dev stubs — `storage` / `search` / `automation` / `graphql` / `analytics` / `realtime` / `notification` / `ai` (they already carry a `_dev: true` marker that nothing respects). *(Update #4000: the `analytics` one is **gone** — after the fallback's retirement (#3891/#3989) it was the last thing refilling that slot, and refilling it re-created the retired shape in dev: the dispatcher gated on service presence, so the stub was called like an engine and answered 200 with fabricated rows. The remaining stubs are unchanged, and the class-wide question they raise is tracked separately — see conclusion 3 below.)* - `ObjectQLPlugin` registers a ~66-line `analytics` **fallback** (the D10 note — deliberate, but it reports as fully available). - `http-dispatcher.ts` `svcAvailable` — the hardcode above. **Decision — honest capabilities:** 1. A registered service that is a stub / dev / fallback MUST self-identify with a **standard marker** (standardise one; `_dev` is the existing precedent). 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. +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.)* 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). diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 4cd661f22a..6e7cdb7c50 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -1408,6 +1408,19 @@ 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; + const advertisedRoute = (serviceName: string, route?: string) => + serviceName === 'analytics' && analyticsUnserveable ? undefined : route; + // Check which services are actually registered for (const [serviceName, config] of Object.entries(SERVICE_CONFIG)) { if (registeredServices.has(serviceName)) { @@ -1420,7 +1433,7 @@ export class ObjectStackProtocolImplementation implements services[serviceName] = { enabled: true, status: self?.status ?? (noHttpSurface ? ('degraded' as const) : ('available' as const)), - route: config.route, + route: advertisedRoute(serviceName, config.route), provider: config.plugin, ...(noHttpSurface || self?.handlerReady !== undefined ? { handlerReady: noHttpSurface ? false : self?.handlerReady } @@ -1460,10 +1473,11 @@ export class ObjectStackProtocolImplementation implements // Add routes for available plugin services. Services without an HTTP // surface (config.route undefined) advertise no route (D12, #2462). for (const [serviceName, config] of Object.entries(SERVICE_CONFIG)) { - if (registeredServices.has(serviceName) && config.route) { + const route = advertisedRoute(serviceName, config.route); + if (registeredServices.has(serviceName) && route) { const routeKey = serviceToRouteKey[serviceName]; if (routeKey) { - optionalRoutes[routeKey] = config.route; + optionalRoutes[routeKey] = route; } } } diff --git a/packages/objectql/src/protocol-discovery.test.ts b/packages/objectql/src/protocol-discovery.test.ts index da139598e0..8601aead43 100644 --- a/packages/objectql/src/protocol-discovery.test.ts +++ b/packages/objectql/src/protocol-discovery.test.ts @@ -175,6 +175,25 @@ describe('ObjectStackProtocolImplementation - Dynamic Service Discovery', () => expect(discovery.routes.analytics).toBe('/api/v1/analytics'); }); + // #4000 — the same rule one step further: a stub occupying the slot is not + // the capability either. The dispatcher answers it with the empty-slot 404, + // so advertising a route here would re-tell the lie #3891 removed. The + // service entry itself still self-reports as a stub — that says more than + // `unavailable` would. + it('should not advertise the analytics route for a self-declared stub', async () => { + const mockServices = new Map(); + mockServices.set('analytics', { _dev: true, query: async () => ({ rows: [], fields: [] }) }); + + protocol = new ObjectStackProtocolImplementation(engine, () => mockServices); + const discovery = await protocol.getDiscovery(); + + expect(discovery.services.analytics.enabled).toBe(true); + expect(discovery.services.analytics.status).toBe('stub'); + expect(discovery.services.analytics.handlerReady).toBe(false); + expect(discovery.services.analytics.route).toBeUndefined(); + expect(discovery.routes.analytics).toBeUndefined(); + }); + it('should map file-storage service to storage route', async () => { const mockServices = new Map(); mockServices.set('file-storage', {}); diff --git a/packages/plugins/plugin-dev/src/dev-plugin.test.ts b/packages/plugins/plugin-dev/src/dev-plugin.test.ts index d9a7550eec..72d9e39c27 100644 --- a/packages/plugins/plugin-dev/src/dev-plugin.test.ts +++ b/packages/plugins/plugin-dev/src/dev-plugin.test.ts @@ -67,7 +67,7 @@ describe('DevPlugin', () => { await expect(plugin.init(ctx)).resolves.not.toThrow(); }); - it('should register contract-compliant dev stubs for all core services', async () => { + it('should register contract-compliant dev stubs for every core service except analytics', async () => { const registeredServices = new Map(); const ctx: any = { logger: { @@ -159,12 +159,14 @@ describe('DevPlugin', () => { expect(execResult.success).toBe(true); expect(Array.isArray(await automation.listFlows())).toBe(true); - // ── Verify IAnalyticsService contract ── - const analytics = registeredServices.get('analytics'); - const analyticsResult = await analytics.query({ cube: 'test' }); - expect(Array.isArray(analyticsResult.rows)).toBe(true); - expect(Array.isArray(analyticsResult.fields)).toBe(true); - expect(Array.isArray(await analytics.getMeta())).toBe(true); + // ── analytics: deliberately NOT stubbed (#4000) ── + // #3891/#3989 retired the degraded analytics shim so an empty slot is the + // honest signal (route unmounted → 404, discovery `unavailable`). A dev + // stub refilled the slot and the dispatcher, gating on presence alone, + // served its fabricated rows with a 200 — the retired shape, one layer + // down. The slot stays empty; install @objectstack/service-analytics. + expect(registeredServices.has('analytics')).toBe(false); + expect(stubLog[0]).not.toContain('analytics'); // ── Verify IRealtimeService contract ── const realtime = registeredServices.get('realtime'); diff --git a/packages/plugins/plugin-dev/src/dev-plugin.ts b/packages/plugins/plugin-dev/src/dev-plugin.ts index 6cc68578b1..57d88d9781 100644 --- a/packages/plugins/plugin-dev/src/dev-plugin.ts +++ b/packages/plugins/plugin-dev/src/dev-plugin.ts @@ -97,16 +97,6 @@ function createAutomationStub() { } -/** IAnalyticsService — dev stub returning empty results */ -function createAnalyticsStub() { - return { - _dev: true, _serviceName: 'analytics', - async query() { return { rows: [], fields: [] }; }, - async getMeta() { return []; }, - async generateSql() { return { sql: '', params: [] }; }, - }; -} - /** IRealtimeService — in-memory pub/sub stub */ function createRealtimeStub() { const subs = new Map(); @@ -266,7 +256,6 @@ const DEV_STUB_FACTORIES: Record Record> = { 'file-storage': createStorageStub, 'search': createSearchStub, 'automation': createAutomationStub, - 'analytics': createAnalyticsStub, 'realtime': createRealtimeStub, 'notification': createNotificationStub, 'ai': createAIStub, @@ -281,6 +270,24 @@ const DEV_STUB_FACTORIES: Record Record> = { 'security.fieldMasker': createSecurityFieldMaskerStub, }; +/** + * Core service slots that deliberately get NO dev stub — not even the + * shapeless `{ _dev: true }` fallback the registration loop uses for slots + * without a factory. + * + * `analytics` (#4000): #3891/#3989 retired the degraded analytics shim, making + * an unoccupied slot the honest signal — `/analytics/*` is not mounted, the + * request 404s, and discovery reports `unavailable`. Registering a dev stub + * re-filled that slot and re-created the retired shape one layer down: the + * dispatcher gates on service presence, so a stub was called like a real + * engine and answered 200 with fabricated rows. Dev mode is explicit opt-in + * and the fake was empty, so this was never the security hole the shim was — + * but "the capability is present" must mean the same thing in dev as in + * production. To use analytics locally, install the real engine: + * `@objectstack/service-analytics` runs an InMemory strategy. + */ +const NO_DEV_STUB_SERVICES = new Set(['analytics']); + /** * Dev Plugin Options * @@ -657,11 +664,13 @@ export class DevPlugin implements Plugin { // dev stub (implementing the interface from packages/spec/src/contracts/) // so that the full kernel service map is populated and downstream code // receives correct return types (arrays, booleans, objects — not undefined). + // Exception: NO_DEV_STUB_SERVICES slots stay empty on purpose (#4000). const stubNames: string[] = []; for (const svc of CORE_SERVICE_NAMES) { if (!enabled(svc)) continue; + if (NO_DEV_STUB_SERVICES.has(svc)) continue; try { ctx.getService(svc); // Already registered by a real plugin — skip diff --git a/packages/runtime/src/dispatcher-plugin.routes.test.ts b/packages/runtime/src/dispatcher-plugin.routes.test.ts index ec74c72e53..c5f587f836 100644 --- a/packages/runtime/src/dispatcher-plugin.routes.test.ts +++ b/packages/runtime/src/dispatcher-plugin.routes.test.ts @@ -155,6 +155,31 @@ describe('createDispatcherPlugin — HTTP route registration', () => { for (const r of ANALYTICS_ROUTES) expect(routes).toContain(r); }); + // [#4000] "Registered" is not the test — `handlerReady` is (ADR-0076 D12). + // plugin-dev used to fill this slot with a stub, which mounted the three + // routes and served its fabricated rows with a 200. The stub is retired, + // and the mount gate now reads the same predicate the domain does, so the + // wire surface can't advertise routes that could only 404. + it('does NOT mount /analytics when the registered analytics service self-declares as a stub', async () => { + const { server, routes } = makeFakeServer(); + const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false }); + await plugin.start?.(ctxWithServices(server, { + analytics: { _dev: true, query: async () => ({ rows: [], fields: [] }) }, + })); + + for (const r of ANALYTICS_ROUTES) expect(routes).not.toContain(r); + }); + + it('DOES mount /analytics for a degraded-but-serving analytics service', async () => { + const { server, routes } = makeFakeServer(); + const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false }); + await plugin.start?.(ctxWithServices(server, { + analytics: { __serviceInfo: { status: 'degraded' }, query: async () => ({ rows: [] }) }, + })); + + for (const r of ANALYTICS_ROUTES) expect(routes).toContain(r); + }); + it('mounts /analytics unconditionally on a multi-tenant host (kernel-resolver wired)', async () => { // Host-global mounts, per-project services: presence is a per-request // question the analytics domain answers (`handled:false` → 404), so the diff --git a/packages/runtime/src/dispatcher-plugin.ts b/packages/runtime/src/dispatcher-plugin.ts index 61c268cfe0..e10d44e544 100644 --- a/packages/runtime/src/dispatcher-plugin.ts +++ b/packages/runtime/src/dispatcher-plugin.ts @@ -4,6 +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 { validationFailureDetails, VALIDATION_FAILED_STATUS } from './validation-failure.js'; import { buildApiError } from './error-envelope.js'; import { @@ -648,14 +649,23 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu // Route via dispatch() (not handleAnalytics directly) so the host // dispatcher's project-aware kernel swap runs first — the // per-project kernel owns the `analytics` service. + // + // [#4000] "Registered" is not the test — `handlerReady` is. A + // service that self-declares as a stub (ADR-0076 D12) occupies the + // slot without being the capability, and the domain answers it with + // the same `handled:false` 404 an empty slot gets; mounting routes + // that can only 404 would re-advertise a capability that isn't + // there. Same predicate on both, so the wire surface and the + // handler can't disagree. const analyticsInstalled = dispatcher.isMultiTenantHost() || await (async () => { const k: any = kernel; try { + let svc: unknown; if (k && typeof k.getServiceAsync === 'function') { - const svc = await k.getServiceAsync('analytics').catch(() => undefined); - if (svc) return true; + svc = await k.getServiceAsync('analytics').catch(() => undefined); } - return !!k?.getService?.('analytics'); + if (!svc) svc = k?.getService?.('analytics'); + return isAnalyticsServiceServeable(svc); } catch { return false; } @@ -690,7 +700,8 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu }); } else { ctx.logger?.info?.( - '[dispatcher] /analytics not mounted — no `analytics` service is registered. ' + + '[dispatcher] /analytics not mounted — no `analytics` service is registered ' + + '(or the registered one self-declares as a stub). ' + 'Install @objectstack/service-analytics to enable the analytics API.', ); } diff --git a/packages/runtime/src/domains/analytics.ts b/packages/runtime/src/domains/analytics.ts index 58e877d04f..90366392f7 100644 --- a/packages/runtime/src/domains/analytics.ts +++ b/packages/runtime/src/domains/analytics.ts @@ -7,11 +7,12 @@ * the degraded ObjectQL fallback was retired (#3891: it dropped the caller's * 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. + * 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}). */ import { CoreServiceName } from '@objectstack/spec/system'; -import { AnalyticsQueryRequestSchema } from '@objectstack/spec/api'; +import { AnalyticsQueryRequestSchema, readServiceSelfInfo } from '@objectstack/spec/api'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; @@ -70,6 +71,28 @@ 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', @@ -88,7 +111,9 @@ export async function handleAnalyticsRequest( query?: any, ): Promise { const analyticsService = await deps.getService(CoreServiceName.enum.analytics); - if (!analyticsService) return { handled: false }; // 404 handled by caller if unhandled + // 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 }; const m = method.toUpperCase(); const subPath = path.replace(/^\/+/, ''); diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 7add8b98fa..196062859d 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -600,6 +600,51 @@ describe('HttpDispatcher', () => { expect(result.handled).toBe(false); }); + // [#4000] ADR-0076 D12 conclusion 3 binds consumers: only + // `handlerReady: true` is a real capability. The dispatcher gated + // on presence alone, so anything occupying the slot got called and + // its fabricated rows went back as a 200 — the shape #3891 retired + // one layer up, kept alive in dev by plugin-dev's analytics stub + // (retired with this change). A stub slot is an empty slot. + it('returns unhandled when the analytics slot holds a self-declared stub, without calling it', async () => { + const stub = { + _dev: true, + query: vi.fn().mockResolvedValue({ rows: [], fields: [] }), + getMeta: vi.fn().mockResolvedValue([]), + generateSql: vi.fn().mockResolvedValue({ sql: '', params: [] }), + }; + (kernel as any).getService = vi.fn().mockResolvedValue(stub); + + for (const [sub, method] of [['query', 'POST'], ['meta', 'GET'], ['sql', 'POST']] as const) { + const result = await dispatcher.handleAnalytics(sub, method, { cube: 'leads', measures: ['count'] }, { request: {} }); + expect(result.handled, `${method} /analytics/${sub}`).toBe(false); + } + expect(stub.query).not.toHaveBeenCalled(); + expect(stub.getMeta).not.toHaveBeenCalled(); + expect(stub.generateSql).not.toHaveBeenCalled(); + }); + + // The same gate read through the standard descriptor, and its other + // half: `degraded` means "working, but partial" — `handlerReady` + // defaults to true there, so it keeps serving. Only a self-confessed + // non-handler is treated as an empty slot. + it('honours __serviceInfo: stub 404s, degraded still serves', async () => { + const make = (info: Record) => ({ + __serviceInfo: info, + query: vi.fn().mockResolvedValue({ rows: [], fields: [] }), + }); + + const stub = make({ status: 'stub', message: 'dev fake' }); + (kernel as any).getService = vi.fn().mockResolvedValue(stub); + expect((await dispatcher.handleAnalytics('query', 'POST', { cube: 'leads', measures: ['count'] }, { request: {} })).handled).toBe(false); + expect(stub.query).not.toHaveBeenCalled(); + + const degraded = make({ status: 'degraded' }); + (kernel as any).getService = vi.fn().mockResolvedValue(degraded); + expect((await dispatcher.handleAnalytics('query', 'POST', { cube: 'leads', measures: ['count'] }, { request: {} })).handled).toBe(true); + expect(degraded.query).toHaveBeenCalled(); + }); + it('should return unhandled for unknown analytics sub-path', async () => { const mockAnalytics = { query: vi.fn() }; (kernel as any).getService = vi.fn().mockResolvedValue(mockAnalytics); @@ -2309,6 +2354,28 @@ describe('HttpDispatcher', () => { expect(info.routes.analytics).toBe('/api/v1/analytics'); }); + // [#4000] The other half of the same marker: a stub occupying the + // analytics slot must not have its route advertised, because the + // dispatcher now answers it with the empty-slot 404. Reporting the + // service itself stays maximally informative — `stub` / + // `handlerReady: false` says more than `unavailable` would. + it('stops advertising the analytics route for a stub, while still reporting it as a stub', async () => { + (kernel as any).getService = vi.fn().mockImplementation((name: string) => + name === 'analytics' ? { _dev: true, query: vi.fn() } : null, + ); + + const info = await dispatcher.getDiscoveryInfo('/api/v1'); + expect(info.routes.analytics).toBeUndefined(); + expect(info.features.analytics).toBe(false); + expect(info.services.analytics.enabled).toBe(true); + expect(info.services.analytics.status).toBe('stub'); + expect(info.services.analytics.handlerReady).toBe(false); + // …and the advertisement matches what the route actually does: + // `handled: false`, which the caller answers with the 404. + const result = await dispatcher.dispatch('POST', '/analytics/query', { cube: 'leads', measures: ['count'] }, {}, { request: {} }); + expect(result.handled).toBe(false); + }); + it('keeps reporting unmarked services as available', async () => { (kernel as any).getService = vi.fn().mockImplementation((name: string) => { if (name === 'workflow') return { getConfig: vi.fn() }; diff --git a/packages/runtime/src/http-dispatcher.ts b/packages/runtime/src/http-dispatcher.ts index f78c0fc41c..f3d0267a91 100644 --- a/packages/runtime/src/http-dispatcher.ts +++ b/packages/runtime/src/http-dispatcher.ts @@ -11,7 +11,7 @@ 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 } from './domains/analytics.js'; +import { createAnalyticsDomain, handleAnalyticsRequest, isAnalyticsServiceServeable } from './domains/analytics.js'; import { createI18nDomain, handleI18nRequest } from './domains/i18n.js'; import { createNotificationsDomain, handleNotificationRequest } from './domains/notifications.js'; import { createSecurityDomain, handleSecurityRequest } from './domains/security.js'; @@ -864,7 +864,19 @@ export class HttpDispatcher { const hasAuth = !!authSvc; const hasSearch = !!searchSvc; const hasFiles = !!filesSvc; - const hasAnalytics = !!analyticsSvc; + // [#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. + // + // `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); const hasWorkflow = !!workflowSvc; const hasAi = !!aiSvc; const hasNotification = !!notificationSvc; @@ -987,7 +999,10 @@ export class HttpDispatcher { // 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'), - analytics: hasAnalytics ? svcAvailable(routes.analytics, undefined, analyticsSvc) : svcUnavailable('analytics'), + // [#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). + 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'), job: hasJob ? svcAvailable(undefined, undefined, jobSvc) : svcUnavailable('job'),